From 821bf21bd7c1f7f30becc20c85a894ad66656272 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 19 Jun 2019 09:14:57 -0500 Subject: [PATCH] ServerBearerTokenAuthenticationConverter Handles Empty Tokens Previously ServerBearerTokenAuthenticationConverter would throw an IllegalArgumentException when the access token in a URI was empty String. It also incorrectly provided HttpStatus.BAD_REQUEST for an empty String access token in the headers. This changes ServerBearerTokenAuthenticationConverter to consistently throw a OAuth2AuthenticationException with an HttpStatus.UNAUTHORIZED Fixes gh-7011 --- ...verBearerTokenAuthenticationConverter.java | 24 +++++++++---- ...arerTokenAuthenticationConverterTests.java | 36 +++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverter.java index 78adc4f3eca..b3aa15a8291 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverter.java @@ -50,8 +50,14 @@ public class ServerBearerTokenAuthenticationConverter private boolean allowUriQueryParameter = false; public Mono convert(ServerWebExchange exchange) { - return Mono.justOrEmpty(this.token(exchange.getRequest())) - .map(BearerTokenAuthenticationToken::new); + return Mono.justOrEmpty(token(exchange.getRequest())) + .map(token -> { + if (token.isEmpty()) { + BearerTokenError error = invalidTokenError(); + throw new OAuth2AuthenticationException(error); + } + return new BearerTokenAuthenticationToken(token); + }); } private String token(ServerHttpRequest request) { @@ -90,11 +96,8 @@ private static String resolveFromAuthorizationHeader(HttpHeaders headers) { if (StringUtils.startsWithIgnoreCase(authorization, "bearer")) { Matcher matcher = authorizationPattern.matcher(authorization); - if ( !matcher.matches() ) { - BearerTokenError error = new BearerTokenError(BearerTokenErrorCodes.INVALID_TOKEN, - HttpStatus.BAD_REQUEST, - "Bearer token is malformed", - "https://tools.ietf.org/html/rfc6750#section-3.1"); + if (!matcher.matches() ) { + BearerTokenError error = invalidTokenError(); throw new OAuth2AuthenticationException(error); } @@ -103,6 +106,13 @@ private static String resolveFromAuthorizationHeader(HttpHeaders headers) { return null; } + private static BearerTokenError invalidTokenError() { + return new BearerTokenError(BearerTokenErrorCodes.INVALID_TOKEN, + HttpStatus.UNAUTHORIZED, + "Bearer token is malformed", + "https://tools.ietf.org/html/rfc6750#section-3.1"); + } + private boolean isParameterTokenSupportedForRequest(ServerHttpRequest request) { return this.allowUriQueryParameter && HttpMethod.GET.equals(request.getMethod()); } diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverterTests.java index 677bc40c005..1a36a2eef8f 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/ServerBearerTokenAuthenticationConverterTests.java @@ -19,15 +19,19 @@ import org.junit.Before; import org.junit.Test; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.server.resource.BearerTokenAuthenticationToken; +import org.springframework.security.oauth2.server.resource.BearerTokenError; +import org.springframework.security.oauth2.server.resource.BearerTokenErrorCodes; import java.util.Base64; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.catchThrowableOfType; /** * @author Rob Winch @@ -52,6 +56,21 @@ public void resolveWhenValidHeaderIsPresentThenTokenIsResolved() { assertThat(convertToToken(request).getToken()).isEqualTo(TEST_TOKEN); } + // gh-7011 + @Test + public void resolveWhenValidHeaderIsEmptyStringThenTokenIsResolved() { + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest + .get("/") + .header(HttpHeaders.AUTHORIZATION, "Bearer "); + + OAuth2AuthenticationException expected = catchThrowableOfType(() -> convertToToken(request), + OAuth2AuthenticationException.class); + BearerTokenError error = (BearerTokenError) expected.getError(); + assertThat(error.getErrorCode()).isEqualTo(BearerTokenErrorCodes.INVALID_TOKEN); + assertThat(error.getUri()).isEqualTo("https://tools.ietf.org/html/rfc6750#section-3.1"); + assertThat(error.getHttpStatus()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + @Test public void resolveWhenLowercaseHeaderIsPresentThenTokenIsResolved() { MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest @@ -123,6 +142,23 @@ public void resolveWhenQueryParameterIsPresentAndSupportedThenTokenIsResolved() assertThat(convertToToken(request).getToken()).isEqualTo(TEST_TOKEN); } + // gh-7011 + @Test + public void resolveWhenQueryParameterIsEmptyAndSupportedThenOAuth2AuthenticationException() { + this.converter.setAllowUriQueryParameter(true); + + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest + .get("/") + .queryParam("access_token", ""); + + OAuth2AuthenticationException expected = catchThrowableOfType(() -> convertToToken(request), + OAuth2AuthenticationException.class); + BearerTokenError error = (BearerTokenError) expected.getError(); + assertThat(error.getErrorCode()).isEqualTo(BearerTokenErrorCodes.INVALID_TOKEN); + assertThat(error.getUri()).isEqualTo("https://tools.ietf.org/html/rfc6750#section-3.1"); + assertThat(error.getHttpStatus()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + @Test public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResolved() { MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest