Evaluate URI query parameter only if enabled

Issue spring-projectsgh-16038

Author: jonah1und1

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();

config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml

@@ -25,10 +25,15 @@
 
 	<c:property-placeholder local-override="true"/>
 
+	<b:bean id="bearerTokenResolver"
+			class="org.springframework.security.oauth2.server.resource.web.DefaultBearerTokenResolver">
+		<b:property name="allowUriQueryParameter" value="true"/>
+	</b:bean>
+
 	<http>
 		<intercept-url pattern="/**" access="authenticated"/>
 		<intercept-url pattern="/requires-read-scope" access="hasAuthority('SCOPE_message:read')"/>
-		<oauth2-resource-server>
+		<oauth2-resource-server bearer-token-resolver-ref="bearerTokenResolver">
 			<jwt jwk-set-uri="${jwk-set-uri:https://idp.example.org}"/>
 		</oauth2-resource-server>
 	</http>

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 && parameterToken.isBlank()) {
+			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());
 	}

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<String> parameterTokens = request.getQueryParams().get("access_token");
 		if (CollectionUtils.isEmpty(parameterTokens)) {
 			return null;

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);

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());
 	}