Polish gh-15819

Closes gh-15818

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

@@ -16,16 +16,12 @@
 
 package org.springframework.security.oauth2.server.resource.web.server.authentication;
 
-import static org.springframework.security.oauth2.server.resource.BearerTokenErrors.invalidRequest;
-
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
-import reactor.util.function.Tuple2;
-import reactor.util.function.Tuples;
 
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
@@ -38,6 +34,7 @@ import org.springframework.security.oauth2.server.resource.BearerTokenErrors;
 import org.springframework.security.oauth2.server.resource.authentication.BearerTokenAuthenticationToken;
 import org.springframework.security.web.server.authentication.ServerAuthenticationConverter;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -53,73 +50,89 @@ import org.springframework.web.server.ServerWebExchange;
  */
 public class ServerBearerTokenAuthenticationConverter implements ServerAuthenticationConverter {
 
-	public static final String ACCESS_TOKEN_NAME = "access_token";
-	public static final String MULTIPLE_BEARER_TOKENS_ERROR_MSG = "Found multiple bearer tokens in the request";
+	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);
 
-	private boolean allowUriQueryParameter = false;
-
 	private boolean allowFormEncodedBodyParameter = false;
 
+	private boolean allowUriQueryParameter = false;
+
 	private String bearerTokenHeaderName = HttpHeaders.AUTHORIZATION;
 
 	@Override
 	public Mono<Authentication> convert(ServerWebExchange exchange) {
-		return Mono.defer(() -> token(exchange)).map(token -> {
-			if (token.isEmpty()) {
-				BearerTokenError error = invalidTokenError();
-				throw new OAuth2AuthenticationException(error);
-			}
-			return new BearerTokenAuthenticationToken(token);
+		return Mono.defer(() -> {
+			ServerHttpRequest request = exchange.getRequest();
+			// @formatter:off
+			return Flux.merge(resolveFromAuthorizationHeader(request.getHeaders()),
+						resolveAccessTokenFromQueryString(request),
+						resolveAccessTokenFromBody(exchange))
+				.collectList()
+				.flatMap(ServerBearerTokenAuthenticationConverter::resolveToken)
+				.map(BearerTokenAuthenticationToken::new);
+			// @formatter:on
 		});
 	}
 
-	private Mono<String> token(ServerWebExchange exchange) {
-		final ServerHttpRequest request = exchange.getRequest();
-
-		return Flux.merge(resolveFromAuthorizationHeader(request.getHeaders()).map(s -> Tuples.of(s, TokenSource.HEADER)),
-						  resolveAccessTokenFromRequest(request).map(s -> Tuples.of(s, TokenSource.QUERY_PARAMETER)),
-						  resolveAccessTokenFromBody(exchange).map(s -> Tuples.of(s, TokenSource.BODY_PARAMETER)))
-				   .collectList()
-				   .mapNotNull(tokenTuples -> {
-					   switch (tokenTuples.size()) {
-						   case 0:
-							   return null;
-						   case 1:
-							   return getTokenIfSupported(tokenTuples.get(0), request);
-						   default:
-							   BearerTokenError error = invalidRequest(MULTIPLE_BEARER_TOKENS_ERROR_MSG);
-							   throw new OAuth2AuthenticationException(error);
-					   }
-				   });
+	private static Mono<String> resolveToken(List<String> accessTokens) {
+		if (CollectionUtils.isEmpty(accessTokens)) {
+			return Mono.empty();
+		}
+
+		if (accessTokens.size() > 1) {
+			BearerTokenError error = BearerTokenErrors.invalidRequest("Found multiple bearer tokens in the request");
+			return Mono.error(new OAuth2AuthenticationException(error));
+		}
+
+		String accessToken = accessTokens.get(0);
+		if (!StringUtils.hasText(accessToken)) {
+			BearerTokenError error = BearerTokenErrors
+				.invalidRequest("The requested token parameter is an empty string");
+			return Mono.error(new OAuth2AuthenticationException(error));
+		}
+
+		return Mono.just(accessToken);
 	}
 
-	private static Mono<String> resolveAccessTokenFromRequest(ServerHttpRequest request) {
-		List<String> parameterTokens = request.getQueryParams().get(ACCESS_TOKEN_NAME);
-		if (CollectionUtils.isEmpty(parameterTokens)) {
+	private Mono<String> resolveFromAuthorizationHeader(HttpHeaders headers) {
+		String authorization = headers.getFirst(this.bearerTokenHeaderName);
+		if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) {
 			return Mono.empty();
 		}
-		if (parameterTokens.size() == 1) {
-			return Mono.just(parameterTokens.get(0));
+
+		Matcher matcher = authorizationPattern.matcher(authorization);
+		if (!matcher.matches()) {
+			BearerTokenError error = BearerTokenErrors.invalidToken("Bearer token is malformed");
+			throw new OAuth2AuthenticationException(error);
 		}
 
-		BearerTokenError error = invalidRequest(MULTIPLE_BEARER_TOKENS_ERROR_MSG);
-		throw new OAuth2AuthenticationException(error);
+		return Mono.just(matcher.group("token"));
+	}
+
+	private Flux<String> resolveAccessTokenFromQueryString(ServerHttpRequest request) {
+		if (!this.allowUriQueryParameter || !HttpMethod.GET.equals(request.getMethod())) {
+			return Flux.empty();
+		}
 
+		return resolveTokens(request.getQueryParams());
 	}
 
-	private String getTokenIfSupported(Tuple2<String, TokenSource> tokenTuple, ServerHttpRequest request) {
-		switch (tokenTuple.getT2()) {
-			case HEADER:
-				return tokenTuple.getT1();
-			case QUERY_PARAMETER:
-				return isParameterTokenSupportedForRequest(request) ? tokenTuple.getT1() : null;
-			case BODY_PARAMETER:
-				return isBodyParameterTokenSupportedForRequest(request) ? tokenTuple.getT1() : null;
-			default:
-				throw new IllegalArgumentException();
+	private Flux<String> resolveAccessTokenFromBody(ServerWebExchange exchange) {
+		ServerHttpRequest request = exchange.getRequest();
+		if (!this.allowFormEncodedBodyParameter
+				|| !MediaType.APPLICATION_FORM_URLENCODED.equals(request.getHeaders().getContentType())
+				|| !HttpMethod.POST.equals(request.getMethod())) {
+			return Flux.empty();
 		}
+
+		return exchange.getFormData().flatMapMany(ServerBearerTokenAuthenticationConverter::resolveTokens);
+	}
+
+	private static Flux<String> resolveTokens(MultiValueMap<String, String> parameters) {
+		List<String> accessTokens = parameters.get(ACCESS_TOKEN_PARAMETER_NAME);
+		return CollectionUtils.isEmpty(accessTokens) ? Flux.empty() : Flux.fromIterable(accessTokens);
 	}
 
 	/**
@@ -158,59 +171,4 @@ public class ServerBearerTokenAuthenticationConverter implements ServerAuthentic
 		this.allowFormEncodedBodyParameter = allowFormEncodedBodyParameter;
 	}
 
-	private Mono<String> resolveFromAuthorizationHeader(HttpHeaders headers) {
-		String authorization = headers.getFirst(this.bearerTokenHeaderName);
-		if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) {
-			return Mono.empty();
-		}
-		Matcher matcher = authorizationPattern.matcher(authorization);
-		if (!matcher.matches()) {
-			BearerTokenError error = invalidTokenError();
-			throw new OAuth2AuthenticationException(error);
-		}
-		return Mono.just(matcher.group("token"));
-	}
-
-	private static BearerTokenError invalidTokenError() {
-		return BearerTokenErrors.invalidToken("Bearer token is malformed");
-	}
-
-	private Mono<String> resolveAccessTokenFromBody(ServerWebExchange exchange) {
-		if (!allowFormEncodedBodyParameter) {
-			return Mono.empty();
-		}
-
-		final ServerHttpRequest request = exchange.getRequest();
-
-		if (request.getMethod() == HttpMethod.POST &&
-				MediaType.APPLICATION_FORM_URLENCODED.equalsTypeAndSubtype(request.getHeaders().getContentType())) {
-
-			return exchange.getFormData().mapNotNull(formData -> {
-				if (formData.isEmpty()) {
-					return null;
-				}
-				final List<String> tokens = formData.get(ACCESS_TOKEN_NAME);
-				if (tokens == null) {
-					return null;
-				}
-				if (tokens.size() > 1) {
-					BearerTokenError error = invalidRequest(MULTIPLE_BEARER_TOKENS_ERROR_MSG);
-					throw new OAuth2AuthenticationException(error);
-				}
-				return formData.getFirst(ACCESS_TOKEN_NAME);
-			});
-		}
-		return Mono.empty();
-	}
-
-	private boolean isBodyParameterTokenSupportedForRequest(ServerHttpRequest request) {
-		return this.allowFormEncodedBodyParameter && HttpMethod.POST == request.getMethod();
-	}
-
-	private boolean isParameterTokenSupportedForRequest(ServerHttpRequest request) {
-		return this.allowUriQueryParameter && HttpMethod.GET.equals(request.getMethod());
-	}
-
-	private enum TokenSource {HEADER, QUERY_PARAMETER, BODY_PARAMETER}
-
 }

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

@@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
 import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
 import org.springframework.mock.web.server.MockServerWebExchange;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
@@ -32,9 +33,6 @@ import org.springframework.security.oauth2.server.resource.authentication.Bearer
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.springframework.http.HttpHeaders.AUTHORIZATION;
-import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED;
-import static org.springframework.mock.http.server.reactive.MockServerHttpRequest.post;
 
 /**
  * @author Rob Winch
@@ -159,6 +157,7 @@ public class ServerBearerTokenAuthenticationConverterTests {
 
 	@Test
 	public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() {
+		this.converter.setAllowUriQueryParameter(true);
 		// @formatter:off
 		this.converter.setAllowUriQueryParameter(true);
 		MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/")
@@ -170,6 +169,54 @@ public class ServerBearerTokenAuthenticationConverterTests {
 		// @formatter:on
 	}
 
+	@Test
+	public void resolveWhenValidHeaderIsPresentTogetherWithBodyParameterThenAuthenticationExceptionIsThrown() {
+		this.converter.setAllowFormEncodedBodyParameter(true);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN)
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN);
+
+		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
+			.withMessageContaining("Found multiple bearer tokens in the request");
+	}
+
+	@Test
+	public void resolveWhenValidHeaderIsPresentTogetherWithBodyParameterAndQueryParameterThenAuthenticationExceptionIsThrown() {
+		this.converter.setAllowUriQueryParameter(true);
+		this.converter.setAllowFormEncodedBodyParameter(true);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN)
+			.queryParam("access_token", TEST_TOKEN)
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN);
+
+		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
+			.withMessageContaining("Found multiple bearer tokens in the request");
+	}
+
+	@Test
+	public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() {
+		this.converter.setAllowUriQueryParameter(true);
+		MockServerHttpRequest request = MockServerHttpRequest.get("/")
+			.queryParam("access_token", "token1", "token2")
+			.build();
+
+		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
+			.withMessageContaining("Found multiple bearer tokens in the request");
+	}
+
+	@Test
+	public void resolveWhenRequestContainsTwoAccessTokenBodyParametersThenAuthenticationExceptionIsThrown() {
+		this.converter.setAllowFormEncodedBodyParameter(true);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN + "&access_token=" + TEST_TOKEN);
+
+		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
+			.withMessageContaining("Found multiple bearer tokens in the request");
+	}
+
 	@Test
 	public void resolveWhenQueryParameterIsPresentAndSupportedThenTokenIsResolved() {
 		this.converter.setAllowUriQueryParameter(true);
@@ -208,7 +255,7 @@ public class ServerBearerTokenAuthenticationConverterTests {
 	}
 
 	@Test
-	void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() {
+	public void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() {
 		this.converter.setAllowUriQueryParameter(true);
 		MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/")
 			.queryParam("access_token", TEST_TOKEN, TEST_TOKEN);
@@ -223,109 +270,74 @@ public class ServerBearerTokenAuthenticationConverterTests {
 	}
 
 	@Test
-	void resolveWhenBodyParameterIsPresentThenTokenIsResolved() {
+	public void resolveWhenBodyParameterIsPresentThenTokenIsResolved() {
 		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN);
 
 		assertThat(convertToToken(request).getToken()).isEqualTo(TEST_TOKEN);
 	}
 
-
 	@Test
-	void resolveWhenBodyParameterIsPresentButNotAllowedThenTokenIsNotResolved() {
+	public void resolveWhenBodyParameterIsPresentButNotAllowedThenTokenIsNotResolved() {
 		this.converter.setAllowFormEncodedBodyParameter(false);
-		MockServerHttpRequest request = post("/").contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN);
 
 		assertThat(convertToToken(request)).isNull();
 	}
 
 	@Test
-	void resolveWhenBodyParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() {
+	public void resolveWhenBodyParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() {
 		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN + "&access_token=" + TEST_TOKEN);
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN + "&access_token=" + TEST_TOKEN);
 
-		assertThatExceptionOfType(OAuth2AuthenticationException.class)
-				.isThrownBy(() -> convertToToken(request))
-				.satisfies(ex -> {
-					BearerTokenError error = (BearerTokenError) ex.getError();
-					assertThat(error.getDescription()).isEqualTo("Found multiple bearer tokens in the request");
-					assertThat(error.getErrorCode()).isEqualTo(BearerTokenErrorCodes.INVALID_REQUEST);
-					assertThat(error.getUri()).isEqualTo("https://tools.ietf.org/html/rfc6750#section-3.1");
-					assertThat(error.getHttpStatus()).isEqualTo(HttpStatus.BAD_REQUEST);
-				});
+		assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request))
+			.satisfies((ex) -> {
+				BearerTokenError error = (BearerTokenError) ex.getError();
+				assertThat(error.getDescription()).isEqualTo("Found multiple bearer tokens in the request");
+				assertThat(error.getErrorCode()).isEqualTo(BearerTokenErrorCodes.INVALID_REQUEST);
+				assertThat(error.getUri()).isEqualTo("https://tools.ietf.org/html/rfc6750#section-3.1");
+				assertThat(error.getHttpStatus()).isEqualTo(HttpStatus.BAD_REQUEST);
+			});
 	}
 
 	@Test
-	void resolveBodyContainsOtherParameterAsWellThenTokenIsResolved() {
+	public void resolveBodyContainsOtherParameterAsWellThenTokenIsResolved() {
 		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN + "&other_param=value");
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("access_token=" + TEST_TOKEN + "&other_param=value");
 
 		assertThat(convertToToken(request).getToken()).isEqualTo(TEST_TOKEN);
 	}
 
 	@Test
-	void resolveWhenNoBodyParameterThenTokenIsNotResolved() {
+	public void resolveWhenNoBodyParameterThenTokenIsNotResolved() {
 		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest.BaseBuilder<?> request = post("/").contentType(APPLICATION_FORM_URLENCODED);
+		MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED);
 
 		assertThat(convertToToken(request)).isNull();
 	}
 
 	@Test
-	void resolveWhenWrongBodyParameterThenTokenIsNotResolved() {
+	public void resolveWhenWrongBodyParameterThenTokenIsNotResolved() {
 		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").contentType(APPLICATION_FORM_URLENCODED)
-												 .body("other_param=value");
+		MockServerHttpRequest request = MockServerHttpRequest.post("/")
+			.contentType(MediaType.APPLICATION_FORM_URLENCODED)
+			.body("other_param=value");
 
 		assertThat(convertToToken(request)).isNull();
 	}
 
-	@Test
-	void resolveWhenValidHeaderIsPresentTogetherWithBodyParameterThenAuthenticationExceptionIsThrown() {
-		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").header(AUTHORIZATION, "Bearer " + TEST_TOKEN)
-												 .contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN);
-
-		assertThatExceptionOfType(OAuth2AuthenticationException.class)
-				.isThrownBy(() -> convertToToken(request))
-				.withMessageContaining("Found multiple bearer tokens in the request");
-	}
-
-	@Test
-	void resolveWhenValidQueryParameterIsPresentTogetherWithBodyParameterThenAuthenticationExceptionIsThrown() {
-		this.converter.setAllowUriQueryParameter(true);
-		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").queryParam("access_token", TEST_TOKEN)
-												 .contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN);
-
-		assertThatExceptionOfType(OAuth2AuthenticationException.class)
-				.isThrownBy(() -> convertToToken(request))
-				.withMessageContaining("Found multiple bearer tokens in the request");
-	}
-
-	@Test
-	void resolveWhenValidQueryParameterIsPresentTogetherWithBodyParameterAndValidHeaderThenAuthenticationExceptionIsThrown() {
-		this.converter.setAllowUriQueryParameter(true);
-		this.converter.setAllowFormEncodedBodyParameter(true);
-		MockServerHttpRequest request = post("/").header(AUTHORIZATION, "Bearer " + TEST_TOKEN)
-												 .queryParam("access_token", TEST_TOKEN)
-												 .contentType(APPLICATION_FORM_URLENCODED)
-												 .body("access_token=" + TEST_TOKEN);
-
-		assertThatExceptionOfType(OAuth2AuthenticationException.class)
-				.isThrownBy(() -> convertToToken(request))
-				.withMessageContaining("Found multiple bearer tokens in the request");
-	}
-
 	// gh-16038
 	@Test
-	void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() {
+	public void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() {
 		MockServerHttpRequest.BaseBuilder<?> request = MockServerHttpRequest.get("/")
 			.queryParam("access_token", TEST_TOKEN, TEST_TOKEN);
 		assertThat(convertToToken(request)).isNull();