Ensure access token isn't resolved from query for form-encoded requests

Closes gh-5668

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -22,6 +22,7 @@ import java.util.regex.Pattern;
 import jakarta.servlet.http.HttpServletRequest;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
 import org.springframework.security.oauth2.server.resource.BearerTokenError;
@@ -38,6 +39,8 @@ import org.springframework.util.StringUtils;
  */
 public final class DefaultBearerTokenResolver implements BearerTokenResolver {
 
+	private static final String ACCESS_TOKEN_PARAMETER_NAME = "access_token";
+
 	private static final Pattern authorizationPattern = Pattern.compile("^Bearer (?<token>[a-zA-Z0-9-._~+/]+=*)$",
 			Pattern.CASE_INSENSITIVE);
 
@@ -115,7 +118,7 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
 	}
 
 	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;
 		}
@@ -127,15 +130,24 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver {
 	}
 
 	private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) {
-		return (("POST".equals(request.getMethod())
-				&& MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()))
-				|| "GET".equals(request.getMethod()));
+		return isFormEncodedRequest(request) || 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);
 	}
 
-	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())));
+	private boolean isParameterTokenEnabledForRequest(HttpServletRequest request) {
+		return ((this.allowFormEncodedBodyParameter && isFormEncodedRequest(request) && !isGetRequest(request)
+				&& !hasAccessTokenInQueryString(request)) || (this.allowUriQueryParameter && isGetRequest(request)));
 	}
 
 }

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

@@ -121,6 +121,7 @@ public class DefaultBearerTokenResolverTests {
 		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);
 		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> this.resolver.resolve(request))
 				.withMessageContaining("Found multiple bearer tokens in the request");
@@ -159,7 +160,7 @@ public class DefaultBearerTokenResolverTests {
 	}
 
 	@Test
-	public void resolveWhenFormParameterIsPresentAndSupportedThenTokenIsResolved() {
+	public void resolveWhenPostAndFormParameterIsPresentAndSupportedThenTokenIsResolved() {
 		this.resolver.setAllowFormEncodedBodyParameter(true);
 		MockHttpServletRequest request = new MockHttpServletRequest();
 		request.setMethod("POST");
@@ -168,6 +169,67 @@ public class DefaultBearerTokenResolverTests {
 		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();
@@ -182,6 +244,7 @@ public class DefaultBearerTokenResolverTests {
 		this.resolver.setAllowUriQueryParameter(true);
 		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);
 	}
@@ -190,6 +253,7 @@ public class DefaultBearerTokenResolverTests {
 	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();
 	}