From 3c0fef59b547359ac3e5824c6b56694c3cb7b8dd Mon Sep 17 00:00:00 2001 From: Steve Riesenberg <5248162+sjohnr@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:12:44 -0600 Subject: [PATCH] Polish gh-16039 Closes gh-16038 --- .../OAuth2ResourceServerConfigurerTests.java | 2 +- .../web/DefaultBearerTokenResolver.java | 113 ++++++++++-------- ...verBearerTokenAuthenticationConverter.java | 2 +- .../web/DefaultBearerTokenResolverTests.java | 19 ++- ...arerTokenAuthenticationConverterTests.java | 2 +- 5 files changed, 80 insertions(+), 58 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java index 493a7f0eaf..903df193d9 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 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. 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 96aa1be570..f00305843a 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 @@ -52,23 +52,77 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver { @Override public String resolve(final HttpServletRequest request) { - final String authorizationHeaderToken = resolveFromAuthorizationHeader(request); - final String parameterToken = resolveFromRequestParameters(request); + // @formatter:off + return resolveToken( + resolveFromAuthorizationHeader(request), + resolveAccessTokenFromQueryString(request), + resolveAccessTokenFromBody(request) + ); + // @formatter:on + } + + private static String resolveToken(String... accessTokens) { + if (accessTokens == null || accessTokens.length == 0) { + return null; + } - if (authorizationHeaderToken != null) { - if (parameterToken != null) { + String accessToken = null; + for (String token : accessTokens) { + if (accessToken == null) { + accessToken = token; + } + else if (token != null) { BearerTokenError error = BearerTokenErrors .invalidRequest("Found multiple bearer tokens in the request"); throw new OAuth2AuthenticationException(error); } - return authorizationHeaderToken; } - if (parameterToken != null && parameterToken.isBlank()) { + + if (accessToken != null && accessToken.isBlank()) { BearerTokenError error = BearerTokenErrors .invalidRequest("The requested token parameter is an empty string"); throw new OAuth2AuthenticationException(error); } - return parameterToken; + + return accessToken; + } + + private String resolveFromAuthorizationHeader(HttpServletRequest request) { + String authorization = request.getHeader(this.bearerTokenHeaderName); + if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) { + return null; + } + + Matcher matcher = authorizationPattern.matcher(authorization); + if (!matcher.matches()) { + BearerTokenError error = BearerTokenErrors.invalidToken("Bearer token is malformed"); + throw new OAuth2AuthenticationException(error); + } + + return matcher.group("token"); + } + + private String resolveAccessTokenFromQueryString(HttpServletRequest request) { + if (!this.allowUriQueryParameter || !HttpMethod.GET.name().equals(request.getMethod())) { + return null; + } + + return resolveToken(request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME)); + } + + private String resolveAccessTokenFromBody(HttpServletRequest request) { + if (!this.allowFormEncodedBodyParameter + || !MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()) + || HttpMethod.GET.name().equals(request.getMethod())) { + return null; + } + + String queryString = request.getQueryString(); + if (queryString != null && queryString.contains(ACCESS_TOKEN_PARAMETER_NAME)) { + return null; + } + + return resolveToken(request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME)); } /** @@ -106,49 +160,4 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver { this.bearerTokenHeaderName = bearerTokenHeaderName; } - private String resolveFromAuthorizationHeader(HttpServletRequest request) { - String authorization = request.getHeader(this.bearerTokenHeaderName); - if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) { - return null; - } - Matcher matcher = authorizationPattern.matcher(authorization); - if (!matcher.matches()) { - BearerTokenError error = BearerTokenErrors.invalidToken("Bearer token is malformed"); - throw new OAuth2AuthenticationException(error); - } - return matcher.group("token"); - } - - 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; - } - if (values.length == 1) { - return values[0]; - } - BearerTokenError error = BearerTokenErrors.invalidRequest("Found multiple bearer tokens in the request"); - throw new OAuth2AuthenticationException(error); - } - - 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); - } - - private boolean isParameterTokenEnabledForRequest(HttpServletRequest request) { - return ((this.allowFormEncodedBodyParameter && isFormEncodedRequest(request) && !isGetRequest(request) - && !hasAccessTokenInQueryString(request)) || (this.allowUriQueryParameter && isGetRequest(request))); - } - } diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java index ec7675acd0..c3c7699f8e 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. 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 6c62d7c27f..db8c6292bd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -237,6 +237,19 @@ public class DefaultBearerTokenResolverTests { assertThat(this.resolver.resolve(request)).isNull(); } + @Test + public void resolveWhenPostAndQueryParameterIsSupportedAndFormParameterIsPresentThenTokenIsNotResolved() { + this.resolver.setAllowUriQueryParameter(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(); @@ -267,7 +280,7 @@ public class DefaultBearerTokenResolverTests { // gh-16038 @Test - void resolveWhenRequestContainsTwoAccessTokenFormParametersAndSupportIsDisabledThenTokenIsNotResolved() { + public void resolveWhenRequestContainsTwoAccessTokenFormParametersAndSupportIsDisabledThenTokenIsNotResolved() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("POST"); request.setContentType("application/x-www-form-urlencoded"); @@ -277,7 +290,7 @@ public class DefaultBearerTokenResolverTests { // gh-16038 @Test - void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { + public void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); request.addParameter("access_token", "token1", "token2"); diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java index 3e47401824..32349f121d 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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.