Evaluate URI query parameter only if enabled

Issue gh-16038
This commit is contained in:
Jonah Klöckner 2024-11-05 02:30:03 -06:00 committed by Steve Riesenberg
parent 857ef6fe08
commit da94fbe431
No known key found for this signature in database
GPG Key ID: 3D0169B18AB8F0A9
6 changed files with 62 additions and 25 deletions

View File

@ -1560,12 +1560,15 @@ public class OAuth2ResourceServerConfigurerTests {
@Bean @Bean
SecurityFilterChain filterChain(HttpSecurity http) throws Exception { SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// @formatter:off // @formatter:off
DefaultBearerTokenResolver defaultBearerTokenResolver = new DefaultBearerTokenResolver();
defaultBearerTokenResolver.setAllowUriQueryParameter(true);
http http
.authorizeRequests() .authorizeRequests()
.requestMatchers("/requires-read-scope").access("hasAuthority('SCOPE_message:read')") .requestMatchers("/requires-read-scope").access("hasAuthority('SCOPE_message:read')")
.anyRequest().authenticated() .anyRequest().authenticated()
.and() .and()
.oauth2ResourceServer() .oauth2ResourceServer()
.bearerTokenResolver(defaultBearerTokenResolver)
.jwt() .jwt()
.jwkSetUri(this.jwkSetUri); .jwkSetUri(this.jwkSetUri);
return http.build(); return http.build();

View File

@ -25,10 +25,15 @@
<c:property-placeholder local-override="true"/> <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> <http>
<intercept-url pattern="/**" access="authenticated"/> <intercept-url pattern="/**" access="authenticated"/>
<intercept-url pattern="/requires-read-scope" access="hasAuthority('SCOPE_message:read')"/> <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}"/> <jwt jwk-set-uri="${jwk-set-uri:https://idp.example.org}"/>
</oauth2-resource-server> </oauth2-resource-server>
</http> </http>

View File

@ -53,8 +53,8 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
@Override @Override
public String resolve(final HttpServletRequest request) { public String resolve(final HttpServletRequest request) {
final String authorizationHeaderToken = resolveFromAuthorizationHeader(request); final String authorizationHeaderToken = resolveFromAuthorizationHeader(request);
final String parameterToken = isParameterTokenSupportedForRequest(request) final String parameterToken = resolveFromRequestParameters(request);
? resolveFromRequestParameters(request) : null;
if (authorizationHeaderToken != null) { if (authorizationHeaderToken != null) {
if (parameterToken != null) { if (parameterToken != null) {
BearerTokenError error = BearerTokenErrors BearerTokenError error = BearerTokenErrors
@ -63,15 +63,12 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
} }
return authorizationHeaderToken; return authorizationHeaderToken;
} }
if (parameterToken != null && isParameterTokenEnabledForRequest(request)) { if (parameterToken != null && parameterToken.isBlank()) {
if (!StringUtils.hasText(parameterToken)) { BearerTokenError error = BearerTokenErrors
BearerTokenError error = BearerTokenErrors .invalidRequest("The requested token parameter is an empty string");
.invalidRequest("The requested token parameter is an empty string"); throw new OAuth2AuthenticationException(error);
throw new OAuth2AuthenticationException(error);
}
return parameterToken;
} }
return null; return parameterToken;
} }
/** /**
@ -122,7 +119,10 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
return matcher.group("token"); 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); String[] values = request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME);
if (values == null || values.length == 0) { if (values == null || values.length == 0) {
return null; return null;
@ -134,10 +134,6 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
throw new OAuth2AuthenticationException(error); throw new OAuth2AuthenticationException(error);
} }
private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) {
return isFormEncodedRequest(request) || isGetRequest(request);
}
private static boolean isGetRequest(HttpServletRequest request) { private static boolean isGetRequest(HttpServletRequest request) {
return HttpMethod.GET.name().equals(request.getMethod()); return HttpMethod.GET.name().equals(request.getMethod());
} }

View File

@ -77,18 +77,18 @@ public class ServerBearerTokenAuthenticationConverter implements ServerAuthentic
} }
return authorizationHeaderToken; return authorizationHeaderToken;
} }
if (parameterToken != null && isParameterTokenSupportedForRequest(request)) { if (parameterToken != null && !StringUtils.hasText(parameterToken)) {
if (!StringUtils.hasText(parameterToken)) { BearerTokenError error = BearerTokenErrors
BearerTokenError error = BearerTokenErrors .invalidRequest("The requested token parameter is an empty string");
.invalidRequest("The requested token parameter is an empty string"); throw new OAuth2AuthenticationException(error);
throw new OAuth2AuthenticationException(error);
}
return parameterToken;
} }
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"); List<String> parameterTokens = request.getQueryParams().get("access_token");
if (CollectionUtils.isEmpty(parameterTokens)) { if (CollectionUtils.isEmpty(parameterTokens)) {
return null; return null;

View File

@ -110,6 +110,7 @@ public class DefaultBearerTokenResolverTests {
@Test @Test
public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenticationExceptionIsThrown() { public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenticationExceptionIsThrown() {
this.resolver.setAllowFormEncodedBodyParameter(true);
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.addHeader("Authorization", "Bearer " + TEST_TOKEN);
request.setMethod("POST"); request.setMethod("POST");
@ -121,6 +122,7 @@ public class DefaultBearerTokenResolverTests {
@Test @Test
public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() {
this.resolver.setAllowUriQueryParameter(true);
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.addHeader("Authorization", "Bearer " + TEST_TOKEN);
request.setMethod("GET"); request.setMethod("GET");
@ -133,6 +135,7 @@ public class DefaultBearerTokenResolverTests {
// gh-10326 // gh-10326
@Test @Test
public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() { public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() {
this.resolver.setAllowUriQueryParameter(true);
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("GET"); request.setMethod("GET");
request.addParameter("access_token", "token1", "token2"); request.addParameter("access_token", "token1", "token2");
@ -143,6 +146,7 @@ public class DefaultBearerTokenResolverTests {
// gh-10326 // gh-10326
@Test @Test
public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() { public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() {
this.resolver.setAllowFormEncodedBodyParameter(true);
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("POST"); request.setMethod("POST");
request.setContentType("application/x-www-form-urlencoded"); request.setContentType("application/x-www-form-urlencoded");
@ -261,6 +265,25 @@ public class DefaultBearerTokenResolverTests {
assertThat(this.resolver.resolve(request)).isNull(); 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 @Test
public void resolveWhenQueryParameterIsPresentAndEmptyStringThenTokenIsNotResolved() { public void resolveWhenQueryParameterIsPresentAndEmptyStringThenTokenIsNotResolved() {
this.resolver.setAllowUriQueryParameter(true); this.resolver.setAllowUriQueryParameter(true);

View File

@ -157,6 +157,7 @@ public class ServerBearerTokenAuthenticationConverterTests {
@Test @Test
public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() {
// @formatter:off // @formatter:off
this.converter.setAllowUriQueryParameter(true);
MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/") MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/")
.queryParam("access_token", TEST_TOKEN) .queryParam("access_token", TEST_TOKEN)
.header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN); .header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN);
@ -205,6 +206,7 @@ public class ServerBearerTokenAuthenticationConverterTests {
@Test @Test
void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() { void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() {
this.converter.setAllowUriQueryParameter(true);
MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/") MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/")
.queryParam("access_token", TEST_TOKEN, TEST_TOKEN); .queryParam("access_token", TEST_TOKEN, TEST_TOKEN);
assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request)) assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
@ -217,6 +219,14 @@ public class ServerBearerTokenAuthenticationConverterTests {
} }
// 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) { private BearerTokenAuthenticationToken convertToToken(MockServerHttpRequest.BaseBuilder<?> request) {
return convertToToken(request.build()); return convertToToken(request.build());
} }