Browse Source

Polish BasicAuthenticationConverter

This reverts to the old behavior from BasicAuthenticationFilter.
Specifically, if a token has an empty password, it still parses a username
and an empty String password.

Issue gh-7025
Rob Winch 6 years ago
parent
commit
ad2f999c25

+ 19 - 25
web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationConverter.java

@@ -17,7 +17,7 @@ package org.springframework.security.web.authentication.www;
 
 
 import static org.springframework.http.HttpHeaders.AUTHORIZATION;
 import static org.springframework.http.HttpHeaders.AUTHORIZATION;
 
 
-import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.Base64;
 
 
@@ -25,7 +25,6 @@ import javax.servlet.http.HttpServletRequest;
 
 
 import org.springframework.security.authentication.AuthenticationDetailsSource;
 import org.springframework.security.authentication.AuthenticationDetailsSource;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.BadCredentialsException;
-import org.springframework.security.authentication.InternalAuthenticationServiceException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.web.authentication.AuthenticationConverter;
 import org.springframework.security.web.authentication.AuthenticationConverter;
 import org.springframework.security.web.authentication.WebAuthenticationDetailsSource;
 import org.springframework.security.web.authentication.WebAuthenticationDetailsSource;
@@ -47,7 +46,7 @@ public class BasicAuthenticationConverter implements AuthenticationConverter {
 
 
 	private AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource;
 	private AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource;
 
 
-	private String credentialsCharset = StandardCharsets.UTF_8.name();
+	private Charset credentialsCharset = StandardCharsets.UTF_8;
 
 
 	public BasicAuthenticationConverter() {
 	public BasicAuthenticationConverter() {
 		this(new WebAuthenticationDetailsSource());
 		this(new WebAuthenticationDetailsSource());
@@ -58,16 +57,16 @@ public class BasicAuthenticationConverter implements AuthenticationConverter {
 		this.authenticationDetailsSource = authenticationDetailsSource;
 		this.authenticationDetailsSource = authenticationDetailsSource;
 	}
 	}
 
 
-	public String getCredentialsCharset() {
-		return credentialsCharset;
+	public Charset getCredentialsCharset() {
+		return this.credentialsCharset;
 	}
 	}
 
 
-	public void setCredentialsCharset(String credentialsCharset) {
+	public void setCredentialsCharset(Charset credentialsCharset) {
 		this.credentialsCharset = credentialsCharset;
 		this.credentialsCharset = credentialsCharset;
 	}
 	}
 
 
 	public AuthenticationDetailsSource<HttpServletRequest, ?> getAuthenticationDetailsSource() {
 	public AuthenticationDetailsSource<HttpServletRequest, ?> getAuthenticationDetailsSource() {
-		return authenticationDetailsSource;
+		return this.authenticationDetailsSource;
 	}
 	}
 
 
 	public void setAuthenticationDetailsSource(
 	public void setAuthenticationDetailsSource(
@@ -88,34 +87,29 @@ public class BasicAuthenticationConverter implements AuthenticationConverter {
 			return null;
 			return null;
 		}
 		}
 
 
-		byte[] base64Token = header.substring(6).getBytes();
+		byte[] base64Token = header.substring(6).getBytes(StandardCharsets.UTF_8);
 		byte[] decoded;
 		byte[] decoded;
 		try {
 		try {
 			decoded = Base64.getDecoder().decode(base64Token);
 			decoded = Base64.getDecoder().decode(base64Token);
-		} catch (IllegalArgumentException e) {
-			throw new BadCredentialsException("Failed to decode basic authentication token");
 		}
 		}
-
-		String token;
-		try {
-			token = new String(decoded, getCredentialsCharset(request));
-		} catch (UnsupportedEncodingException e) {
-			throw new InternalAuthenticationServiceException(e.getMessage(), e);
+		catch (IllegalArgumentException e) {
+			throw new BadCredentialsException(
+					"Failed to decode basic authentication token");
 		}
 		}
 
 
-		String[] tokens = token.split(":");
-		if (tokens.length != 2) {
-			throw new BadCredentialsException("Invalid basic authentication token");
-		}
+		String token = new String(decoded, getCredentialsCharset(request));
 
 
-		UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken(tokens[0],
-				tokens[1]);
-		authentication.setDetails(authenticationDetailsSource.buildDetails(request));
+		int delim = token.indexOf(":");
 
 
-		return authentication;
+		if (delim == -1) {
+			throw new BadCredentialsException("Invalid basic authentication token");
+		}
+		UsernamePasswordAuthenticationToken result  = new UsernamePasswordAuthenticationToken(token.substring(0, delim), token.substring(delim + 1));
+		result.setDetails(this.authenticationDetailsSource.buildDetails(request));
+		return result;
 	}
 	}
 
 
-	protected String getCredentialsCharset(HttpServletRequest request) {
+	protected Charset getCredentialsCharset(HttpServletRequest request) {
 		return getCredentialsCharset();
 		return getCredentialsCharset();
 	}
 	}
 
 

+ 12 - 0
web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationConverterTests.java

@@ -99,4 +99,16 @@ public class BasicAuthenticationConverterTests {
 		converter.convert(request);
 		converter.convert(request);
 	}
 	}
 
 
+	@Test
+	public void convertWhenEmptyPassword() {
+		String token = "rod:";
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
+		UsernamePasswordAuthenticationToken authentication = converter.convert(request);
+
+		verify(authenticationDetailsSource).buildDetails(any());
+		assertThat(authentication).isNotNull();
+		assertThat(authentication.getName()).isEqualTo("rod");
+		assertThat(authentication.getCredentials()).isEqualTo("");
+	}
 }
 }