Skip to content

Commit

Permalink
Conditionally resolve bearer token from request parameters
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pneuschwander authored and sjohnr committed Oct 13, 2021
1 parent 88c64b3 commit 6db58cb
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ public void postWhenUsingDefaultsWithBearerTokenAsFormParameterThenIgnoresToken(
this.spring.register(JwkSetUriConfig.class).autowire();
// engage csrf
// @formatter:off
this.mvc.perform(post("/").with(bearerToken("token").asParam()))
this.mvc.perform(post("/").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken("token").asParam()))
.andExpect(status().isForbidden())
.andExpect(header().doesNotExist(HttpHeaders.WWW_AUTHENTICATE));
// @formatter:on
Expand All @@ -370,7 +370,7 @@ public void postWhenUsingDefaultsWithBearerTokenAsFormParameterThenIgnoresToken(
public void postWhenCsrfDisabledWithBearerTokenAsFormParameterThenIgnoresToken() throws Exception {
this.spring.register(CsrfDisabledConfig.class).autowire();
// @formatter:off
this.mvc.perform(post("/").with(bearerToken("token").asParam()))
this.mvc.perform(post("/").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken("token").asParam()))
.andExpect(status().isUnauthorized())
.andExpect(header().string(HttpHeaders.WWW_AUTHENTICATE, "Bearer"));
// @formatter:on
Expand Down Expand Up @@ -536,7 +536,7 @@ public void postWhenUsingDefaultsWithValidBearerTokenAndNoCsrfTokenThenOk() thro
mockRestOperations(jwks("Default"));
String token = this.token("ValidNoScopes");
// @formatter:off
this.mvc.perform(post("/authenticated").with(bearerToken(token)))
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken(token)))
.andExpect(status().isOk())
.andExpect(content().string("test-subject"));
// @formatter:on
Expand All @@ -558,7 +558,7 @@ public void postWhenUsingDefaultsWithExpiredBearerTokenAndNoCsrfThenInvalidToken
mockRestOperations(jwks("Default"));
String token = this.token("Expired");
// @formatter:off
this.mvc.perform(post("/authenticated").with(bearerToken(token)))
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).with(bearerToken(token)))
.andExpect(status().isUnauthorized())
.andExpect(invalidTokenHeader("An error occurred while attempting to decode the Jwt"));
// @formatter:on
Expand Down Expand Up @@ -626,7 +626,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyThenEitherHeaderOrReq
this.mvc.perform(get("/authenticated").with(bearerToken(JWT_TOKEN)))
.andExpect(status().isOk())
.andExpect(content().string(JWT_SUBJECT));
this.mvc.perform(post("/authenticated").param("access_token", JWT_TOKEN))
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).param("access_token", JWT_TOKEN))
.andExpect(status().isOk())
.andExpect(content().string(JWT_SUBJECT));
// @formatter:on
Expand Down Expand Up @@ -659,6 +659,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyAndRequestContainsTwo
given(decoder.decode(anyString())).willReturn(JWT);
// @formatter:off
MockHttpServletRequestBuilder request = post("/authenticated")
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
.param("access_token", JWT_TOKEN)
.with(bearerToken(JWT_TOKEN))
.with(csrf());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ public void getWhenBearerTokenInTwoParametersThenInvalidRequest() throws Excepti
public void postWhenBearerTokenAsFormParameterThenIgnoresToken() throws Exception {
this.spring.configLocations(xml("JwkSetUri")).autowire();
this.mvc.perform(post("/") // engage csrf
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
.param("access_token", "token")).andExpect(status().isForbidden())
.andExpect(header().string(HttpHeaders.WWW_AUTHENTICATE, "Bearer")); // different
// from
Expand Down Expand Up @@ -451,7 +452,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyThenEitherHeaderOrReq
// @formatter:off
this.mvc.perform(get("/authenticated").header("Authorization", "Bearer token"))
.andExpect(status().isNotFound());
this.mvc.perform(post("/authenticated").param("access_token", "token"))
this.mvc.perform(post("/authenticated").header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE).param("access_token", "token"))
.andExpect(status().isNotFound());
// @formatter:on
}
Expand All @@ -477,6 +478,7 @@ public void requestWhenBearerTokenResolverAllowsRequestBodyAndRequestContainsTwo
this.spring.configLocations(xml("MockJwtDecoder"), xml("AllowBearerTokenInBody")).autowire();
// @formatter:off
MockHttpServletRequestBuilder request = post("/authenticated")
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
.param("access_token", "token")
.header("Authorization", "Bearer token")
.with(csrf());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,6 +22,7 @@
import javax.servlet.http.HttpServletRequest;

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

@Override
public String resolve(HttpServletRequest request) {
String authorizationHeaderToken = resolveFromAuthorizationHeader(request);
String parameterToken = resolveFromRequestParameters(request);
public String resolve(final HttpServletRequest request) {
final String authorizationHeaderToken = resolveFromAuthorizationHeader(request);
final String parameterToken = isParameterTokenSupportedForRequest(request)
? resolveFromRequestParameters(request) : null;
if (authorizationHeaderToken != null) {
if (parameterToken != null) {
BearerTokenError error = BearerTokenErrors
final BearerTokenError error = BearerTokenErrors
.invalidRequest("Found multiple bearer tokens in the request");
throw new OAuth2AuthenticationException(error);
}
return authorizationHeaderToken;
}
if (parameterToken != null && isParameterTokenSupportedForRequest(request)) {
if (parameterToken != null && isParameterTokenEnabledForRequest(request)) {
return parameterToken;
}
return null;
Expand Down Expand Up @@ -124,8 +126,15 @@ private static String resolveFromRequestParameters(HttpServletRequest request) {
throw new OAuth2AuthenticationException(error);
}

private boolean isParameterTokenSupportedForRequest(HttpServletRequest request) {
return ((this.allowFormEncodedBodyParameter && "POST".equals(request.getMethod()))
private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) {
return (("POST".equals(request.getMethod())
&& MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()))
|| "GET".equals(request.getMethod()));
}

private boolean isParameterTokenEnabledForRequest(final HttpServletRequest request) {
return ((this.allowFormEncodedBodyParameter && "POST".equals(request.getMethod())
&& MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()))
|| (this.allowUriQueryParameter && "GET".equals(request.getMethod())));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -126,14 +126,38 @@ public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthent
.withMessageContaining("Found multiple bearer tokens in the request");
}

// gh-10326
@Test
public void resolveWhenRequestContainsTwoAccessTokenParametersThenAuthenticationExceptionIsThrown() {
public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("GET");
request.addParameter("access_token", "token1", "token2");
assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> this.resolver.resolve(request))
.withMessageContaining("Found multiple bearer tokens in the request");
}

// gh-10326
@Test
public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("POST");
request.setContentType("application/x-www-form-urlencoded");
request.addParameter("access_token", "token1", "token2");
assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> this.resolver.resolve(request))
.withMessageContaining("Found multiple bearer tokens in the request");
}

// gh-10326
@Test
public void resolveWhenParameterIsPresentInMultipartRequestAndFormParameterSupportedThenTokenIsNotResolved() {
this.resolver.setAllowFormEncodedBodyParameter(true);
MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("POST");
request.setContentType("multipart/form-data");
request.addParameter("access_token", TEST_TOKEN);
assertThat(this.resolver.resolve(request)).isNull();
}

@Test
public void resolveWhenFormParameterIsPresentAndSupportedThenTokenIsResolved() {
this.resolver.setAllowFormEncodedBodyParameter(true);
Expand Down

0 comments on commit 6db58cb

Please sign in to comment.