Skip to content

Commit

Permalink
Remove dependency on commons-codec by using java.util.Base64
Browse files Browse the repository at this point in the history
  • Loading branch information
j3graham authored and jzheaux committed Jun 9, 2022
1 parent d1cb236 commit f3c96fa
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 54 deletions.
6 changes: 0 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,6 @@ updateDependenciesSettings {
alphaBetaVersions()
snapshotVersions()
addRule { components ->
components.withModule("commons-codec:commons-codec") { selection ->
ModuleComponentIdentifier candidate = selection.getCandidate();
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {
selection.reject("commons-codec updates break saml tests");
}
}
components.withModule("org.python:jython") { selection ->
ModuleComponentIdentifier candidate = selection.getCandidate();
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {
Expand Down
1 change: 0 additions & 1 deletion dependencies/spring-security-dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ dependencies {
api "com.squareup.okhttp3:mockwebserver:3.14.9"
api "com.squareup.okhttp3:okhttp:3.14.9"
api "com.unboundid:unboundid-ldapsdk:4.0.14"
api "commons-codec:commons-codec:1.15"
api "commons-collections:commons-collections:3.2.2"
api "io.mockk:mockk:1.12.4"
api "jakarta.annotation:jakarta.annotation-api:2.1.0"
Expand Down
1 change: 0 additions & 1 deletion messaging/spring-security-messaging.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ dependencies {
optional 'jakarta.servlet:jakarta.servlet-api'

testImplementation project(path: ':spring-security-core', configuration: 'tests')
testImplementation 'commons-codec:commons-codec'
testImplementation "org.assertj:assertj-core"
testImplementation "org.junit.jupiter:junit-jupiter-api"
testImplementation "org.junit.jupiter:junit-jupiter-params"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@

import java.io.ByteArrayOutputStream;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Base64;
import java.util.function.Function;
import java.util.zip.Inflater;
import java.util.zip.InflaterOutputStream;

import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.codec.CodecPolicy;
import org.apache.commons.codec.binary.Base64;

import org.springframework.http.HttpMethod;
import org.springframework.security.saml2.core.Saml2Error;
Expand All @@ -47,7 +47,11 @@
*/
public final class Saml2AuthenticationTokenConverter implements AuthenticationConverter {

private static Base64 BASE64 = new Base64(0, new byte[] { '\n' }, false, CodecPolicy.STRICT);
// MimeDecoder allows extra line-breaks as well as other non-alphabet values.
// This matches the behaviour of the commons-codec decoder.
private static final Base64.Decoder BASE64 = Base64.getMimeDecoder();

private static final Base64Checker BASE_64_CHECKER = new Base64Checker();

private final RelyingPartyRegistrationResolver relyingPartyRegistrationResolver;

Expand Down Expand Up @@ -110,6 +114,7 @@ private String inflateIfRequired(HttpServletRequest request, byte[] b) {

private byte[] samlDecode(String base64EncodedPayload) {
try {
BASE_64_CHECKER.checkAcceptable(base64EncodedPayload);
return BASE64.decode(base64EncodedPayload);
}
catch (Exception ex) {
Expand All @@ -132,4 +137,58 @@ private String samlInflate(byte[] b) {
}
}

static class Base64Checker {

private static final int[] values = genValueMapping();

Base64Checker() {

}

private static int[] genValueMapping() {
byte[] alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
.getBytes(StandardCharsets.ISO_8859_1);

int[] values = new int[256];
Arrays.fill(values, -1);
for (int i = 0; i < alphabet.length; i++) {
values[alphabet[i] & 0xff] = i;
}
return values;
}

boolean isAcceptable(String s) {
int goodChars = 0;
int lastGoodCharVal = -1;

// count number of characters from Base64 alphabet
for (int i = 0; i < s.length(); i++) {
int val = values[0xff & s.charAt(i)];
if (val != -1) {
lastGoodCharVal = val;
goodChars++;
}
}

// in cases of an incomplete final chunk, ensure the unused bits are zero
switch (goodChars % 4) {
case 0:
return true;
case 2:
return (lastGoodCharVal & 0b1111) == 0;
case 3:
return (lastGoodCharVal & 0b11) == 0;
default:
return false;
}
}

void checkAcceptable(String ins) {
if (!isAcceptable(ins)) {
throw new IllegalArgumentException("Unaccepted Encoding");
}
}

}

}
1 change: 0 additions & 1 deletion web/spring-security-web.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ dependencies {
provided 'jakarta.servlet:jakarta.servlet-api'

testImplementation project(path: ':spring-security-core', configuration: 'tests')
testImplementation 'commons-codec:commons-codec'
testImplementation 'io.projectreactor:reactor-test'
testImplementation 'jakarta.xml.bind:jakarta.xml.bind-api'
testImplementation 'org.hamcrest:hamcrest'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.security.test.web;

import java.util.Base64;

import org.springframework.util.DigestUtils;

public final class CodecTestUtils {

private CodecTestUtils() {
}

public static String encodeBase64(String unencoded) {
return Base64.getEncoder().encodeToString(unencoded.getBytes());
}

public static String encodeBase64(byte[] unencoded) {
return Base64.getEncoder().encodeToString(unencoded);
}

public static String decodeBase64(String encoded) {
return new String(Base64.getDecoder().decode(encoded));
}

public static boolean isBase64(byte[] arrayOctet) {
try {
Base64.getMimeDecoder().decode(arrayOctet);
return true;

}
catch (Exception ex) {
return false;
}
}

public static String md5Hex(String data) {
return DigestUtils.md5DigestAsHex(data.getBytes());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import java.util.Date;

import jakarta.servlet.http.Cookie;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.codec.digest.DigestUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -33,6 +31,7 @@
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.test.web.CodecTestUtils;
import org.springframework.util.StringUtils;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -76,7 +75,7 @@ void udsWillReturnNull() {
}

private long determineExpiryTimeFromBased64EncodedToken(String validToken) {
String cookieAsPlainText = new String(Base64.decodeBase64(validToken.getBytes()));
String cookieAsPlainText = CodecTestUtils.decodeBase64(validToken);
String[] cookieTokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, ":");
if (cookieTokens.length == 3) {
try {
Expand All @@ -92,9 +91,9 @@ private String generateCorrectCookieContentForToken(long expiryTime, String user
// format is:
// username + ":" + expiryTime + ":" + Md5Hex(username + ":" + expiryTime + ":" +
// password + ":" + key)
String signatureValue = DigestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
String signatureValue = CodecTestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
String tokenValue = username + ":" + expiryTime + ":" + signatureValue;
return new String(Base64.encodeBase64(tokenValue.getBytes()));
return CodecTestUtils.encodeBase64(tokenValue);
}

@Test
Expand Down Expand Up @@ -134,7 +133,7 @@ public void autoLoginReturnsNullForExpiredCookieAndClearsCookie() {
@Test
public void autoLoginReturnsNullAndClearsCookieIfMissingThreeTokensInCookieValue() {
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
new String(Base64.encodeBase64("x".getBytes())));
CodecTestUtils.encodeBase64("x"));
MockHttpServletRequest request = new MockHttpServletRequest();
request.setCookies(cookie);
MockHttpServletResponse response = new MockHttpServletResponse();
Expand Down Expand Up @@ -175,7 +174,7 @@ public void autoLoginClearsCookieIfSignatureBlocksDoesNotMatchExpectedValue() {
@Test
public void autoLoginClearsCookieIfTokenDoesNotContainANumberInCookieValue() {
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
new String(Base64.encodeBase64("username:NOT_A_NUMBER:signature".getBytes())));
CodecTestUtils.encodeBase64("username:NOT_A_NUMBER:signature"));
MockHttpServletRequest request = new MockHttpServletRequest();
request.setCookies(cookie);
MockHttpServletResponse response = new MockHttpServletResponse();
Expand Down Expand Up @@ -275,7 +274,7 @@ public void loginSuccessNormalWithNonUserDetailsBasedPrincipalSetsExpectedCookie
assertThat(Long.parseLong(expiryTime) > expectedExpiryTime - 10000).isTrue();
assertThat(cookie).isNotNull();
assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds());
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue();
}

Expand All @@ -289,7 +288,7 @@ public void loginSuccessNormalWithUserDetailsBasedPrincipalSetsExpectedCookie()
Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
assertThat(cookie).isNotNull();
assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds());
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue();
}

Expand All @@ -315,7 +314,7 @@ public void negativeValidityPeriodIsSetOnCookieButExpiryTimeRemainsAtTwoWeeks()
assertThat(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())
- System.currentTimeMillis() > AbstractRememberMeServices.TWO_WEEKS_S - 50).isTrue();
assertThat(cookie.getMaxAge()).isEqualTo(-1);
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.springframework.security.web.authentication.www;

import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.codec.binary.Base64;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand All @@ -28,6 +27,7 @@
import org.springframework.security.authentication.AuthenticationDetailsSource;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.test.web.CodecTestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -56,7 +56,7 @@ public void setup() {
public void testNormalOperation() {
String token = "rod:koala";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand All @@ -67,7 +67,7 @@ public void testNormalOperation() {
public void requestWhenAuthorizationSchemeInMixedCaseThenAuthenticates() {
String token = "rod:koala";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "BaSiC " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "BaSiC " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand All @@ -87,7 +87,7 @@ public void testWhenUnsupportedAuthorizationHeaderThenIgnored() {
public void testWhenInvalidBasicAuthorizationTokenThenError() {
String token = "NOT_A_VALID_TOKEN_AS_MISSING_COLON";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.converter.convert(request));
}

Expand All @@ -102,7 +102,7 @@ public void testWhenInvalidBase64ThenError() {
public void convertWhenEmptyPassword() {
String token = "rod:";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand Down
Loading

0 comments on commit f3c96fa

Please sign in to comment.