Skip to content

Commit 156f71f

Browse files
pneuschwanderpneuschwander
committed
Conditionally resolve bearer token from request parameters
Before this commit, the DefaultBearerTokenResolver unconditionally resolved the request parameters to check whether multiple tokens are present in the request and reject those requests as invalid. This commit changes this behaviour to resolve the request parameters only if parameter token is supported for the specific request according to spec (RFC 6750). Closes gh-10326
1 parent 7d81a52 commit 156f71f

File tree

4 files changed

+52
-16
lines changed

4 files changed

+52
-16
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ public void postWhenUsingDefaultsWithBearerTokenAsFormParameterThenIgnoresToken(
360360
this.spring.register(JwkSetUriConfig.class).autowire();
361361
// engage csrf
362362
// @formatter:off
363-
this.mvc.perform(post("/").with(bearerToken("token").asParam()))
363+
this.mvc.perform(post("/").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken("token").asParam()))
364364
.andExpect(status().isForbidden())
365365
.andExpect(header().doesNotExist(HttpHeaders.WWW_AUTHENTICATE));
366366
// @formatter:on
@@ -370,7 +370,7 @@ public void postWhenUsingDefaultsWithBearerTokenAsFormParameterThenIgnoresToken(
370370
public void postWhenCsrfDisabledWithBearerTokenAsFormParameterThenIgnoresToken() throws Exception {
371371
this.spring.register(CsrfDisabledConfig.class).autowire();
372372
// @formatter:off
373-
this.mvc.perform(post("/").with(bearerToken("token").asParam()))
373+
this.mvc.perform(post("/").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken("token").asParam()))
374374
.andExpect(status().isUnauthorized())
375375
.andExpect(header().string(HttpHeaders.WWW_AUTHENTICATE, "Bearer"));
376376
// @formatter:on
@@ -536,7 +536,7 @@ public void postWhenUsingDefaultsWithValidBearerTokenAndNoCsrfTokenThenOk() thro
536536
mockRestOperations(jwks("Default"));
537537
String token = this.token("ValidNoScopes");
538538
// @formatter:off
539-
this.mvc.perform(post("/authenticated").with(bearerToken(token)))
539+
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken(token)))
540540
.andExpect(status().isOk())
541541
.andExpect(content().string("test-subject"));
542542
// @formatter:on
@@ -558,7 +558,7 @@ public void postWhenUsingDefaultsWithExpiredBearerTokenAndNoCsrfThenInvalidToken
558558
mockRestOperations(jwks("Default"));
559559
String token = this.token("Expired");
560560
// @formatter:off
561-
this.mvc.perform(post("/authenticated").with(bearerToken(token)))
561+
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken(token)))
562562
.andExpect(status().isUnauthorized())
563563
.andExpect(invalidTokenHeader("An error occurred while attempting to decode the Jwt"));
564564
// @formatter:on
@@ -626,7 +626,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyThenEitherHeaderOrReq
626626
this.mvc.perform(get("/authenticated").with(bearerToken(JWT_TOKEN)))
627627
.andExpect(status().isOk())
628628
.andExpect(content().string(JWT_SUBJECT));
629-
this.mvc.perform(post("/authenticated").param("access_token", JWT_TOKEN))
629+
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).param("access_token", JWT_TOKEN))
630630
.andExpect(status().isOk())
631631
.andExpect(content().string(JWT_SUBJECT));
632632
// @formatter:on
@@ -659,6 +659,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyAndRequestContainsTwo
659659
given(decoder.decode(anyString())).willReturn(JWT);
660660
// @formatter:off
661661
MockHttpServletRequestBuilder request = post("/authenticated")
662+
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
662663
.param("access_token", JWT_TOKEN)
663664
.with(bearerToken(JWT_TOKEN))
664665
.with(csrf());

‎config/src/test/java/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ public void getWhenBearerTokenInTwoParametersThenInvalidRequest() throws Excepti
261261
public void postWhenBearerTokenAsFormParameterThenIgnoresToken() throws Exception {
262262
this.spring.configLocations(xml("JwkSetUri")).autowire();
263263
this.mvc.perform(post("/") // engage csrf
264+
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
264265
.param("access_token", "token")).andExpect(status().isForbidden())
265266
.andExpect(header().string(HttpHeaders.WWW_AUTHENTICATE, "Bearer")); // different
266267
// from
@@ -451,7 +452,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyThenEitherHeaderOrReq
451452
// @formatter:off
452453
this.mvc.perform(get("/authenticated").header("Authorization", "Bearer token"))
453454
.andExpect(status().isNotFound());
454-
this.mvc.perform(post("/authenticated").param("access_token", "token"))
455+
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).param("access_token", "token"))
455456
.andExpect(status().isNotFound());
456457
// @formatter:on
457458
}
@@ -477,6 +478,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyAndRequestContainsTwo
477478
this.spring.configLocations(xml("MockJwtDecoder"), xml("AllowBearerTokenInBody")).autowire();
478479
// @formatter:off
479480
MockHttpServletRequestBuilder request = post("/authenticated")
481+
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
480482
.param("access_token", "token")
481483
.header("Authorization", "Bearer token")
482484
.with(csrf());

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java

+17-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import javax.servlet.http.HttpServletRequest;
2323

2424
import org.springframework.http.HttpHeaders;
25+
import org.springframework.http.MediaType;
2526
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
2627
import org.springframework.security.oauth2.server.resource.BearerTokenError;
2728
import org.springframework.security.oauth2.server.resource.BearerTokenErrors;
@@ -47,18 +48,19 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
4748
private String bearerTokenHeaderName = HttpHeaders.AUTHORIZATION;
4849

4950
@Override
50-
public String resolve(HttpServletRequest request) {
51-
String authorizationHeaderToken = resolveFromAuthorizationHeader(request);
52-
String parameterToken = resolveFromRequestParameters(request);
51+
public String resolve(final HttpServletRequest request) {
52+
final String authorizationHeaderToken = resolveFromAuthorizationHeader(request);
53+
final String parameterToken = isParameterTokenSupportedForRequest(request)
54+
? resolveFromRequestParameters(request) : null;
5355
if (authorizationHeaderToken != null) {
5456
if (parameterToken != null) {
55-
BearerTokenError error = BearerTokenErrors
57+
final BearerTokenError error = BearerTokenErrors
5658
.invalidRequest("Found multiple bearer tokens in the request");
5759
throw new OAuth2AuthenticationException(error);
5860
}
5961
return authorizationHeaderToken;
6062
}
61-
if (parameterToken != null && isParameterTokenSupportedForRequest(request)) {
63+
if (parameterToken != null && isParameterTokenEnabledForRequest(request)) {
6264
return parameterToken;
6365
}
6466
return null;
@@ -124,8 +126,15 @@ private static String resolveFromRequestParameters(HttpServletRequest request) {
124126
throw new OAuth2AuthenticationException(error);
125127
}
126128

127-
private boolean isParameterTokenSupportedForRequest(HttpServletRequest request) {
128-
return ((this.allowFormEncodedBodyParameter && "POST".equals(request.getMethod()))
129+
private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) {
130+
return (("POST".equals(request.getMethod())
131+
&& MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()))
132+
|| "GET".equals(request.getMethod()));
133+
}
134+
135+
private boolean isParameterTokenEnabledForRequest(final HttpServletRequest request) {
136+
return ((this.allowFormEncodedBodyParameter && "POST".equals(request.getMethod())
137+
&& MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()))
129138
|| (this.allowUriQueryParameter && "GET".equals(request.getMethod())));
130139
}
131140

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java

+26-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -126,14 +126,38 @@ public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthent
126126
.withMessageContaining("Found multiple bearer tokens in the request");
127127
}
128128

129+
// gh-10326
129130
@Test
130-
public void resolveWhenRequestContainsTwoAccessTokenParametersThenAuthenticationExceptionIsThrown() {
131+
public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() {
131132
MockHttpServletRequest request = new MockHttpServletRequest();
133+
request.setMethod("GET");
134+
request.addParameter("access_token", "token1", "token2");
135+
assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> this.resolver.resolve(request))
136+
.withMessageContaining("Found multiple bearer tokens in the request");
137+
}
138+
139+
// gh-10326
140+
@Test
141+
public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() {
142+
MockHttpServletRequest request = new MockHttpServletRequest();
143+
request.setMethod("POST");
144+
request.setContentType("application/x-www-form-urlencoded");
132145
request.addParameter("access_token", "token1", "token2");
133146
assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> this.resolver.resolve(request))
134147
.withMessageContaining("Found multiple bearer tokens in the request");
135148
}
136149

150+
// gh-10326
151+
@Test
152+
public void resolveWhenParameterIsPresentInMultipartRequestAndFormParameterSupportedThenTokenIsNotResolved() {
153+
this.resolver.setAllowFormEncodedBodyParameter(true);
154+
MockHttpServletRequest request = new MockHttpServletRequest();
155+
request.setMethod("POST");
156+
request.setContentType("multipart/form-data");
157+
request.addParameter("access_token", TEST_TOKEN);
158+
assertThat(this.resolver.resolve(request)).isNull();
159+
}
160+
137161
@Test
138162
public void resolveWhenFormParameterIsPresentAndSupportedThenTokenIsResolved() {
139163
this.resolver.setAllowFormEncodedBodyParameter(true);

0 commit comments

Comments
 (0)