diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java index c247a6d7fed..f957cb0c492 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java @@ -1560,12 +1560,15 @@ static class JwkSetUriConfig { @Bean SecurityFilterChain filterChain(HttpSecurity http) throws Exception { // @formatter:off + DefaultBearerTokenResolver defaultBearerTokenResolver = new DefaultBearerTokenResolver(); + defaultBearerTokenResolver.setAllowUriQueryParameter(true); http .authorizeRequests() .requestMatchers("/requires-read-scope").access("hasAuthority('SCOPE_message:read')") .anyRequest().authenticated() .and() .oauth2ResourceServer() + .bearerTokenResolver(defaultBearerTokenResolver) .jwt() .jwkSetUri(this.jwkSetUri); return http.build(); diff --git a/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml b/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml index 3f81363d270..aac12989e91 100644 --- a/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml +++ b/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml @@ -25,10 +25,15 @@ + + + + - + diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java index d238e870178..d0694ae0817 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java @@ -53,8 +53,8 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver { @Override public String resolve(final HttpServletRequest request) { final String authorizationHeaderToken = resolveFromAuthorizationHeader(request); - final String parameterToken = isParameterTokenSupportedForRequest(request) - ? resolveFromRequestParameters(request) : null; + final String parameterToken = resolveFromRequestParameters(request); + if (authorizationHeaderToken != null) { if (parameterToken != null) { BearerTokenError error = BearerTokenErrors @@ -63,15 +63,12 @@ public String resolve(final HttpServletRequest request) { } return authorizationHeaderToken; } - if (parameterToken != null && isParameterTokenEnabledForRequest(request)) { - if (!StringUtils.hasText(parameterToken)) { - BearerTokenError error = BearerTokenErrors - .invalidRequest("The requested token parameter is an empty string"); - throw new OAuth2AuthenticationException(error); - } - return parameterToken; + if (parameterToken != null && !StringUtils.hasText(parameterToken)) { + BearerTokenError error = BearerTokenErrors + .invalidRequest("The requested token parameter is an empty string"); + throw new OAuth2AuthenticationException(error); } - return null; + return parameterToken; } /** @@ -122,7 +119,10 @@ private String resolveFromAuthorizationHeader(HttpServletRequest request) { return matcher.group("token"); } - private static String resolveFromRequestParameters(HttpServletRequest request) { + private String resolveFromRequestParameters(HttpServletRequest request) { + if (!isParameterTokenEnabledForRequest(request)) { + return null; + } String[] values = request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME); if (values == null || values.length == 0) { return null; @@ -134,10 +134,6 @@ private static String resolveFromRequestParameters(HttpServletRequest request) { throw new OAuth2AuthenticationException(error); } - private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) { - return isFormEncodedRequest(request) || isGetRequest(request); - } - private static boolean isGetRequest(HttpServletRequest request) { return HttpMethod.GET.name().equals(request.getMethod()); } diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java index bd07c59b747..ec7675acd04 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java @@ -77,18 +77,18 @@ private String token(ServerHttpRequest request) { } return authorizationHeaderToken; } - if (parameterToken != null && isParameterTokenSupportedForRequest(request)) { - if (!StringUtils.hasText(parameterToken)) { - BearerTokenError error = BearerTokenErrors - .invalidRequest("The requested token parameter is an empty string"); - throw new OAuth2AuthenticationException(error); - } - return parameterToken; + if (parameterToken != null && !StringUtils.hasText(parameterToken)) { + BearerTokenError error = BearerTokenErrors + .invalidRequest("The requested token parameter is an empty string"); + throw new OAuth2AuthenticationException(error); } - return null; + return parameterToken; } - private static String resolveAccessTokenFromRequest(ServerHttpRequest request) { + private String resolveAccessTokenFromRequest(ServerHttpRequest request) { + if (!isParameterTokenSupportedForRequest(request)) { + return null; + } List parameterTokens = request.getQueryParams().get("access_token"); if (CollectionUtils.isEmpty(parameterTokens)) { return null; diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java index f04fb69a3df..6c62d7c27fb 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java @@ -110,6 +110,7 @@ public void resolveWhenHeaderWithInvalidCharactersIsPresentThenAuthenticationExc @Test public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowFormEncodedBodyParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.setMethod("POST"); @@ -121,6 +122,7 @@ public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenti @Test public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowUriQueryParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.setMethod("GET"); @@ -133,6 +135,7 @@ public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthent // gh-10326 @Test public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowUriQueryParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); request.addParameter("access_token", "token1", "token2"); @@ -143,6 +146,7 @@ public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthentic // gh-10326 @Test public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowFormEncodedBodyParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("POST"); request.setContentType("application/x-www-form-urlencoded"); @@ -261,6 +265,25 @@ public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResol assertThat(this.resolver.resolve(request)).isNull(); } + // gh-16038 + @Test + void resolveWhenRequestContainsTwoAccessTokenFormParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", "token1", "token2"); + assertThat(this.resolver.resolve(request)).isNull(); + } + + // gh-16038 + @Test + void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + request.addParameter("access_token", "token1", "token2"); + assertThat(this.resolver.resolve(request)).isNull(); + } + @Test public void resolveWhenQueryParameterIsPresentAndEmptyStringThenTokenIsNotResolved() { this.resolver.setAllowUriQueryParameter(true); diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java index 1f4b17697fd..3e474018243 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java @@ -157,6 +157,7 @@ public void resolveWhenHeaderWithInvalidCharactersIsPresentAndNotSubscribedThenN @Test public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { // @formatter:off + this.converter.setAllowUriQueryParameter(true); MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") .queryParam("access_token", TEST_TOKEN) .header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN); @@ -205,6 +206,7 @@ public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResol @Test void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() { + this.converter.setAllowUriQueryParameter(true); MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") .queryParam("access_token", TEST_TOKEN, TEST_TOKEN); assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request)) @@ -217,6 +219,14 @@ void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationExc } + // gh-16038 + @Test + void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") + .queryParam("access_token", TEST_TOKEN, TEST_TOKEN); + assertThat(convertToToken(request)).isNull(); + } + private BearerTokenAuthenticationToken convertToToken(MockServerHttpRequest.BaseBuilder request) { return convertToToken(request.build()); }