From cb0283fa4b381322cd915227f2fab1f491567e2d Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Mon, 5 Nov 2018 22:40:06 +0100 Subject: [PATCH] Ensure access token isn't resolved from query for form-encoded requests --- .../web/DefaultBearerTokenResolver.java | 23 ++++++- .../web/DefaultBearerTokenResolverTests.java | 67 ++++++++++++++++++- 2 files changed, 86 insertions(+), 4 deletions(-) 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 c4532fa0297..5ed5e86e946 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 @@ -21,7 +21,9 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +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.BearerTokenErrorCodes; @@ -36,6 +38,8 @@ */ public final class DefaultBearerTokenResolver implements BearerTokenResolver { + private static final String ACCESS_TOKEN_PARAMETER_NAME = "access_token"; + private static final Pattern authorizationPattern = Pattern.compile("^Bearer (?[a-zA-Z0-9-._~+/]+)=*$"); private boolean allowFormEncodedBodyParameter = false; @@ -104,7 +108,7 @@ private static String resolveFromAuthorizationHeader(HttpServletRequest request) } private static String resolveFromRequestParameters(HttpServletRequest request) { - String[] values = request.getParameterValues("access_token"); + String[] values = request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME); if (values == null || values.length == 0) { return null; } @@ -121,7 +125,20 @@ private static String resolveFromRequestParameters(HttpServletRequest request) { } private boolean isParameterTokenSupportedForRequest(HttpServletRequest request) { - return ((this.allowFormEncodedBodyParameter && "POST".equals(request.getMethod())) - || (this.allowUriQueryParameter && "GET".equals(request.getMethod()))); + return ((this.allowFormEncodedBodyParameter && isFormEncodedRequest(request) && !isGetRequest(request) + && !hasAccessTokenInQueryString(request)) || (this.allowUriQueryParameter && isGetRequest(request))); + } + + private static boolean isGetRequest(HttpServletRequest request) { + return HttpMethod.GET.name().equals(request.getMethod()); + } + + private static boolean isFormEncodedRequest(HttpServletRequest request) { + return MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()); + } + + private static boolean hasAccessTokenInQueryString(HttpServletRequest request) { + return (request.getQueryString() != null) && request.getQueryString().contains(ACCESS_TOKEN_PARAMETER_NAME); } + } 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 32518f4d1a6..f5420f10fb9 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 @@ -101,6 +101,7 @@ public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthent MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.setMethod("GET"); + request.setQueryString("access_token=" + TEST_TOKEN); request.addParameter("access_token", TEST_TOKEN); assertThatCode(() -> this.resolver.resolve(request)).isInstanceOf(OAuth2AuthenticationException.class) @@ -117,7 +118,7 @@ public void resolveWhenRequestContainsTwoAccessTokenParametersThenAuthentication } @Test - public void resolveWhenFormParameterIsPresentAndSupportedThenTokenIsResolved() { + public void resolveWhenPostAndFormParameterIsPresentAndSupportedThenTokenIsResolved() { this.resolver.setAllowFormEncodedBodyParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); @@ -128,6 +129,67 @@ public void resolveWhenFormParameterIsPresentAndSupportedThenTokenIsResolved() { assertThat(this.resolver.resolve(request)).isEqualTo(TEST_TOKEN); } + @Test + public void resolveWhenPutAndFormParameterIsPresentAndSupportedThenTokenIsResolved() { + this.resolver.setAllowFormEncodedBodyParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("PUT"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isEqualTo(TEST_TOKEN); + } + + @Test + public void resolveWhenPatchAndFormParameterIsPresentAndSupportedThenTokenIsResolved() { + this.resolver.setAllowFormEncodedBodyParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("PATCH"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isEqualTo(TEST_TOKEN); + } + + @Test + public void resolveWhenDeleteAndFormParameterIsPresentAndSupportedThenTokenIsResolved() { + this.resolver.setAllowFormEncodedBodyParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("DELETE"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isEqualTo(TEST_TOKEN); + } + + @Test + public void resolveWhenGetAndFormParameterIsPresentAndSupportedThenTokenIsNotResolved() { + this.resolver.setAllowFormEncodedBodyParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isNull(); + } + + @Test + public void resolveWhenPostAndFormParameterIsSupportedAndQueryParameterIsPresentThenTokenIsNotResolved() { + this.resolver.setAllowFormEncodedBodyParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContentType("application/x-www-form-urlencoded"); + request.setQueryString("access_token=" + TEST_TOKEN); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isNull(); + } + @Test public void resolveWhenFormParameterIsPresentAndNotSupportedThenTokenIsNotResolved() { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -144,6 +206,7 @@ public void resolveWhenQueryParameterIsPresentAndSupportedThenTokenIsResolved() MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); + request.setQueryString("access_token=" + TEST_TOKEN); request.addParameter("access_token", TEST_TOKEN); assertThat(this.resolver.resolve(request)).isEqualTo(TEST_TOKEN); @@ -153,8 +216,10 @@ public void resolveWhenQueryParameterIsPresentAndSupportedThenTokenIsResolved() public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResolved() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); + request.setQueryString("access_token=" + TEST_TOKEN); request.addParameter("access_token", TEST_TOKEN); assertThat(this.resolver.resolve(request)).isNull(); } + }