Fix scope mapping

Issue gh-12101
spring-projects · Oct 28, 2022 · 7500458 · 7500458
1 parent 5dbfa57
commit 7500458
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 117 deletions.
diff --git a/...ramework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java b/...ramework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -27,7 +27,6 @@
 import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
 import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 import org.springframework.web.client.ResponseErrorHandler;
 import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestOperations;
@@ -78,20 +77,12 @@ public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRe
 					"An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null);
 			throw new OAuth2AuthorizationException(oauth2Error, ex);
 		}
-
-		OAuth2AccessTokenResponse tokenResponse = response.getBody();
-
-		if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) {
-			// As per spec, in Section 5.1 Successful Access Token Response
-			// https://tools.ietf.org/html/rfc6749#section-5.1
-			// If AccessTokenResponse.scope is empty, then default to the scope
-			// originally requested by the client in the Token Request
-			tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse)
-					.scopes(authorizationCodeGrantRequest.getClientRegistration().getScopes())
-					.build();
-		}
-
-		return tokenResponse;
+		// As per spec, in Section 5.1 Successful Access Token Response
+		// https://tools.ietf.org/html/rfc6749#section-5.1
+		// If AccessTokenResponse.scope is empty, then we assume all requested scopes were
+		// granted.
+		// However, we use the explicit scopes returned in the response (if any).
+		return response.getBody();
 	}
 
 	/**

diff --git a/...ramework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java b/...ramework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -27,7 +27,6 @@
 import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
 import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 import org.springframework.web.client.ResponseErrorHandler;
 import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestOperations;
@@ -78,20 +77,12 @@ public OAuth2AccessTokenResponse getTokenResponse(OAuth2ClientCredentialsGrantRe
 					"An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null);
 			throw new OAuth2AuthorizationException(oauth2Error, ex);
 		}
-
-		OAuth2AccessTokenResponse tokenResponse = response.getBody();
-
-		if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) {
-			// As per spec, in Section 5.1 Successful Access Token Response
-			// https://tools.ietf.org/html/rfc6749#section-5.1
-			// If AccessTokenResponse.scope is empty, then default to the scope
-			// originally requested by the client in the Token Request
-			tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse)
-					.scopes(clientCredentialsGrantRequest.getClientRegistration().getScopes())
-					.build();
-		}
-
-		return tokenResponse;
+		// As per spec, in Section 5.1 Successful Access Token Response
+		// https://tools.ietf.org/html/rfc6749#section-5.1
+		// If AccessTokenResponse.scope is empty, then we assume all requested scopes were
+		// granted.
+		// However, we use the explicit scopes returned in the response (if any).
+		return response.getBody();
 	}
 
 	/**

diff --git a/...g/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java b/...g/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -27,7 +27,6 @@
 import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
 import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 import org.springframework.web.client.ResponseErrorHandler;
 import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestOperations;
@@ -78,20 +77,12 @@ public OAuth2AccessTokenResponse getTokenResponse(OAuth2PasswordGrantRequest pas
 					"An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null);
 			throw new OAuth2AuthorizationException(oauth2Error, ex);
 		}
-
-		OAuth2AccessTokenResponse tokenResponse = response.getBody();
-
-		if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) {
-			// As per spec, in Section 5.1 Successful Access Token Response
-			// https://tools.ietf.org/html/rfc6749#section-5.1
-			// If AccessTokenResponse.scope is empty, then default to the scope
-			// originally requested by the client in the Token Request
-			tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse)
-					.scopes(passwordGrantRequest.getClientRegistration().getScopes())
-					.build();
-		}
-
-		return tokenResponse;
+		// As per spec, in Section 5.1 Successful Access Token Response
+		// https://tools.ietf.org/html/rfc6749#section-5.1
+		// If AccessTokenResponse.scope is empty, then we assume all requested scopes were
+		// granted.
+		// However, we use the explicit scopes returned in the response (if any).
+		return response.getBody();
 	}
 
 	/**

diff --git a/...ecurity/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java b/...ecurity/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -79,15 +79,7 @@ public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2AuthorizationCodeG
 					})
 					.body(body)
 					.exchange()
-					.flatMap(response -> response.body(oauth2AccessTokenResponse()))
-					.map(response -> {
-						if (response.getAccessToken().getScopes().isEmpty()) {
-							response = OAuth2AccessTokenResponse.withResponse(response)
-								.scopes(authorizationExchange.getAuthorizationRequest().getScopes())
-								.build();
-						}
-						return response;
-					});
+					.flatMap(response -> response.body(oauth2AccessTokenResponse()));
 		});
 	}
 

diff --git a/...ecurity/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java b/...ecurity/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -82,15 +82,7 @@ public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2ClientCredentialsG
 											null
 								)));
 						}
-						return response.body(oauth2AccessTokenResponse()); })
-					.map(response -> {
-						if (response.getAccessToken().getScopes().isEmpty()) {
-							response = OAuth2AccessTokenResponse.withResponse(response)
-								.scopes(authorizationGrantRequest.getClientRegistration().getScopes())
-								.build();
-						}
-						return response;
-					});
+						return response.body(oauth2AccessTokenResponse()); });
 		});
 	}
 

diff --git a/...amework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java b/...amework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -79,18 +79,6 @@ public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2PasswordGrantReque
 									.then(Mono.error(new OAuth2AuthorizationException(oauth2Error)));
 						}
 						return response.body(oauth2AccessTokenResponse());
-					})
-					.map(tokenResponse -> {
-						if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) {
-							// As per spec, in Section 5.1 Successful Access Token Response
-							// https://tools.ietf.org/html/rfc6749#section-5.1
-							// If AccessTokenResponse.scope is empty, then default to the scope
-							// originally requested by the client in the Token Request
-							tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse)
-									.scopes(passwordGrantRequest.getClientRegistration().getScopes())
-									.build();
-						}
-						return tokenResponse;
 					});
 		});
 	}

diff --git a/...ork/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java b/...ork/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -218,7 +218,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe
 	}
 
 	@Test
-	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() {
+	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() {
 		String accessTokenSuccessResponse = "{\n" +
 				"	\"access_token\": \"access-token-1234\",\n" +
 				"   \"token_type\": \"bearer\",\n" +
@@ -230,7 +230,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessToke
 		OAuth2AccessTokenResponse accessTokenResponse =
 				this.tokenResponseClient.getTokenResponse(this.authorizationCodeGrantRequest());
 
-		assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read", "write");
+		assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty();
 	}
 
 	@Test

diff --git a/...ork/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java b/...ork/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -222,7 +222,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe
 	}
 
 	@Test
-	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() {
+	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() {
 		String accessTokenSuccessResponse = "{\n" +
 				"	\"access_token\": \"access-token-1234\",\n" +
 				"   \"token_type\": \"bearer\",\n" +
@@ -235,7 +235,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessToke
 
 		OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(clientCredentialsGrantRequest);
 
-		assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read", "write");
+		assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty();
 	}
 
 	@Test

diff --git a/...ingframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java b/...ingframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -86,11 +86,14 @@ public void getTokenResponseWhenRequestIsNullThenThrowIllegalArgumentException()
 
 	@Test
 	public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() throws Exception {
-		String accessTokenSuccessResponse = "{\n" +
-				"	\"access_token\": \"access-token-1234\",\n" +
-				"   \"token_type\": \"bearer\",\n" +
-				"   \"expires_in\": \"3600\"\n" +
-				"}\n";
+		// @formatter:off
+		String accessTokenSuccessResponse = "{\n"
+			+ "   \"access_token\": \"access-token-1234\",\n"
+			+ "   \"token_type\": \"bearer\",\n"
+			+ "   \"expires_in\": \"3600\",\n"
+			+ "   \"scope\": \"read write\"\n"
+			+ "}\n";
+		// @formatter:on
 		this.server.enqueue(jsonResponse(accessTokenSuccessResponse));
 
 		Instant expiresAtBefore = Instant.now().plusSeconds(3600);
@@ -123,11 +126,14 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t
 
 	@Test
 	public void getTokenResponseWhenClientAuthenticationPostThenFormParametersAreSent() throws Exception {
-		String accessTokenSuccessResponse = "{\n" +
-				"	\"access_token\": \"access-token-1234\",\n" +
-				"   \"token_type\": \"bearer\",\n" +
-				"   \"expires_in\": \"3600\"\n" +
-				"}\n";
+		// @formatter:off
+		String accessTokenSuccessResponse = "{\n"
+			+ "   \"access_token\": \"access-token-1234\",\n"
+			+ "   \"token_type\": \"bearer\",\n"
+			+ "   \"expires_in\": \"3600\",\n"
+			+ "   \"scope\": \"read\"\n"
+			+ "}\n";
+		// @formatter:on
 		this.server.enqueue(jsonResponse(accessTokenSuccessResponse));
 
 		ClientRegistration clientRegistration = this.clientRegistrationBuilder
@@ -186,6 +192,22 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe
 		assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read");
 	}
 
+	@Test
+	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() {
+		// @formatter:off
+		String accessTokenSuccessResponse = "{\n"
+			+ "   \"access_token\": \"access-token-1234\",\n"
+			+ "   \"token_type\": \"bearer\",\n"
+			+ "   \"expires_in\": \"3600\"\n"
+			+ "}\n";
+		// @formatter:on
+		this.server.enqueue(jsonResponse(accessTokenSuccessResponse));
+		OAuth2PasswordGrantRequest passwordGrantRequest = new OAuth2PasswordGrantRequest(
+				this.clientRegistrationBuilder.build(), this.username, this.password);
+		OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(passwordGrantRequest);
+		assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty();
+	}
+
 	@Test
 	public void getTokenResponseWhenErrorResponseThenThrowOAuth2AuthorizationException() {
 		String accessTokenErrorResponse = "{\n" +

diff --git a/...ramework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java b/...ramework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -88,11 +88,14 @@ public void getTokenResponseWhenRequestIsNullThenThrowIllegalArgumentException()
 
 	@Test
 	public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() throws Exception {
-		String accessTokenSuccessResponse = "{\n" +
-				"	\"access_token\": \"access-token-1234\",\n" +
-				"   \"token_type\": \"bearer\",\n" +
-				"   \"expires_in\": \"3600\"\n" +
-				"}\n";
+		// @formatter:off
+		String accessTokenSuccessResponse = "{\n"
+			+ "   \"access_token\": \"access-token-1234\",\n"
+			+ "   \"token_type\": \"bearer\",\n"
+			+ "   \"expires_in\": \"3600\",\n"
+			+ "   \"scope\": \"read write\"\n"
+			+ "}\n";
+		// @formatter:on
 		this.server.enqueue(jsonResponse(accessTokenSuccessResponse));
 
 		Instant expiresAtBefore = Instant.now().plusSeconds(3600);
@@ -121,6 +124,26 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t
 		assertThat(accessTokenResponse.getRefreshToken().getTokenValue()).isEqualTo(this.refreshToken.getTokenValue());
 	}
 
+	@Test
+	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasOriginalScope() {
+		// @formatter:off
+		String accessTokenSuccessResponse = "{\n"
+				+ "   \"access_token\": \"access-token-1234\",\n"
+				+ "   \"token_type\": \"bearer\",\n"
+				+ "   \"expires_in\": \"3600\"\n"
+				+ "}\n";
+		// @formatter:on
+		this.server.enqueue(jsonResponse(accessTokenSuccessResponse));
+		ClientRegistration clientRegistration = this.clientRegistrationBuilder
+				.clientAuthenticationMethod(ClientAuthenticationMethod.POST).build();
+		OAuth2RefreshTokenGrantRequest refreshTokenGrantRequest = new OAuth2RefreshTokenGrantRequest(clientRegistration,
+				this.accessToken, this.refreshToken);
+		OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient
+				.getTokenResponse(refreshTokenGrantRequest);
+		assertThat(accessTokenResponse.getAccessToken().getScopes())
+				.containsExactly(this.accessToken.getScopes().toArray(new String[0]));
+	}
+
 	@Test
 	public void getTokenResponseWhenClientAuthenticationPostThenFormParametersAreSent() throws Exception {
 		String accessTokenSuccessResponse = "{\n" +

diff --git a/...ty/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java b/...ty/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -226,7 +226,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenReturnAccessToke
 	}
 
 	@Test
-	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseUsingRequestedScope() {
+	public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseWithNoScopes() {
 		String accessTokenSuccessResponse = "{\n" +
 				"	\"access_token\": \"access-token-1234\",\n" +
 				"   \"token_type\": \"bearer\",\n" +
@@ -239,7 +239,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAcce
 
 		OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(authorizationCodeGrantRequest()).block();
 
-		assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("openid", "profile", "email", "address");
+		assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty();
 	}
 
 	private OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest() {