Merge branch '1.1.x'

ddubson · Oct 16, 2023 · 1ed0b3a · 1ed0b3a
2 parents 88d604f + ae016cd
commit 1ed0b3a
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 1 deletion.
diff --git a/...uth2/server/authorization/web/authentication/ClientSecretPostAuthenticationConverter.java b/...uth2/server/authorization/web/authentication/ClientSecretPostAuthenticationConverter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-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.
@@ -23,6 +23,7 @@
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
+import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken;
@@ -69,6 +70,17 @@ public Authentication convert(HttpServletRequest request) {
 			throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
 		}
 
+		String queryString = request.getQueryString();
+		if (StringUtils.hasText(queryString) &&
+				(queryString.contains(OAuth2ParameterNames.CLIENT_ID) ||
+						queryString.contains(OAuth2ParameterNames.CLIENT_SECRET))) {
+			OAuth2Error error = new OAuth2Error(
+					OAuth2ErrorCodes.INVALID_REQUEST,
+					"Client credentials MUST NOT be included in the request URI.",
+					null);
+			throw new OAuth2AuthenticationException(error);
+		}
+
 		Map<String, Object> additionalParameters = OAuth2EndpointUtils.getParametersIfMatchesAuthorizationCodeGrantRequest(request,
 				OAuth2ParameterNames.CLIENT_ID,
 				OAuth2ParameterNames.CLIENT_SECRET);

diff --git a/...er/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java b/...er/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java
@@ -61,6 +61,7 @@
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.OAuth2AccessToken;
+import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.jose.TestJwks;
 import org.springframework.security.oauth2.server.authorization.JdbcOAuth2AuthorizationService;
@@ -101,6 +102,7 @@
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+import org.springframework.web.util.UriComponentsBuilder;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
@@ -234,6 +236,37 @@ public void requestWhenTokenRequestPostsClientCredentialsThenTokenResponse() thr
 		verify(jwtCustomizer).customize(any());
 	}
 
+	// gh-1378
+	@Test
+	public void requestWhenTokenRequestWithClientCredentialsInQueryParamThenInvalidRequest() throws Exception {
+		this.spring.register(AuthorizationServerConfiguration.class).autowire();
+
+		RegisteredClient registeredClient = TestRegisteredClients.registeredClient2().build();
+		this.registeredClientRepository.save(registeredClient);
+
+		String tokenEndpointUri = UriComponentsBuilder.fromUriString(DEFAULT_TOKEN_ENDPOINT_URI)
+				.queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
+				.toUriString();
+
+		this.mvc.perform(post(tokenEndpointUri)
+						.param(OAuth2ParameterNames.CLIENT_SECRET, registeredClient.getClientSecret())
+						.param(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.CLIENT_CREDENTIALS.getValue())
+						.param(OAuth2ParameterNames.SCOPE, "scope1 scope2"))
+				.andExpect(status().isBadRequest())
+				.andExpect(jsonPath("$.error").value(OAuth2ErrorCodes.INVALID_REQUEST));
+
+		tokenEndpointUri = UriComponentsBuilder.fromUriString(DEFAULT_TOKEN_ENDPOINT_URI)
+				.queryParam(OAuth2ParameterNames.CLIENT_SECRET, registeredClient.getClientSecret())
+				.toUriString();
+
+		this.mvc.perform(post(tokenEndpointUri)
+						.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
+						.param(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.CLIENT_CREDENTIALS.getValue())
+						.param(OAuth2ParameterNames.SCOPE, "scope1 scope2"))
+				.andExpect(status().isBadRequest())
+				.andExpect(jsonPath("$.error").value(OAuth2ErrorCodes.INVALID_REQUEST));
+	}
+
 	@Test
 	public void requestWhenTokenRequestPostsClientCredentialsAndRequiresUpgradingThenClientSecretUpgraded() throws Exception {
 		this.spring.register(AuthorizationServerConfigurationCustomPasswordEncoder.class).autowire();

diff --git a/...server/authorization/web/authentication/ClientSecretPostAuthenticationConverterTests.java b/...server/authorization/web/authentication/ClientSecretPostAuthenticationConverterTests.java
@@ -79,6 +79,31 @@ public void convertWhenMultipleClientSecretsThenInvalidRequestError() {
 				.isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
 	}
 
+	// gh-1378
+	@Test
+	public void convertWhenClientCredentialsInQueryParamThenInvalidRequestError() {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter(OAuth2ParameterNames.CLIENT_ID, "client-1");
+		request.addParameter(OAuth2ParameterNames.CLIENT_SECRET, "client-secret");
+		request.setQueryString("client_id=client-1");
+		assertThatThrownBy(() -> this.converter.convert(request))
+				.isInstanceOf(OAuth2AuthenticationException.class)
+				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
+				.satisfies(error -> {
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
+					assertThat(error.getDescription()).isEqualTo("Client credentials MUST NOT be included in the request URI.");
+				});
+
+		request.setQueryString("client_secret=client-secret");
+		assertThatThrownBy(() -> this.converter.convert(request))
+				.isInstanceOf(OAuth2AuthenticationException.class)
+				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
+				.satisfies(error -> {
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
+					assertThat(error.getDescription()).isEqualTo("Client credentials MUST NOT be included in the request URI.");
+				});
+	}
+
 	@Test
 	public void convertWhenPostWithValidCredentialsThenReturnClientAuthenticationToken() {
 		MockHttpServletRequest request = new MockHttpServletRequest();