Polish Authorization Consent Deny Request

Issue spring-projectsgh-470

Author: jgrandja

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java

@@ -32,6 +32,7 @@
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
 import org.springframework.security.crypto.keygen.StringKeyGenerator;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
@@ -329,19 +330,6 @@ private Authentication authenticateAuthorizationConsent(Authentication authentic
 		Set<String> currentAuthorizedScopes = currentAuthorizationConsent != null ?
 				currentAuthorizationConsent.getScopes() : Collections.emptySet();
 
-		if (authorizedScopes.isEmpty() && currentAuthorizedScopes.isEmpty() &&
-				authorizationCodeRequestAuthentication.getAdditionalParameters().isEmpty()) {
-			// Authorization consent denied
-			this.authorizationService.remove(authorization);
-			throwError(OAuth2ErrorCodes.ACCESS_DENIED, OAuth2ParameterNames.CLIENT_ID,
-					authorizationCodeRequestAuthentication, registeredClient, authorizationRequest);
-		}
-
-		if (requestedScopes.contains(OidcScopes.OPENID)) {
-			// 'openid' scope is auto-approved as it does not require consent
-			authorizedScopes.add(OidcScopes.OPENID);
-		}
-
 		if (!currentAuthorizedScopes.isEmpty()) {
 			for (String requestedScope : requestedScopes) {
 				if (currentAuthorizedScopes.contains(requestedScope)) {
@@ -350,6 +338,11 @@ private Authentication authenticateAuthorizationConsent(Authentication authentic
 			}
 		}
 
+		if (!authorizedScopes.isEmpty() && requestedScopes.contains(OidcScopes.OPENID)) {
+			// 'openid' scope is auto-approved as it does not require consent
+			authorizedScopes.add(OidcScopes.OPENID);
+		}
+
 		OAuth2AuthorizationConsent.Builder authorizationConsentBuilder;
 		if (currentAuthorizationConsent != null) {
 			authorizationConsentBuilder = OAuth2AuthorizationConsent.from(currentAuthorizationConsent);
@@ -371,6 +364,19 @@ private Authentication authenticateAuthorizationConsent(Authentication authentic
 			this.authorizationConsentCustomizer.accept(authorizationConsentAuthenticationContext);
 		}
 
+		Set<GrantedAuthority> authorities = new HashSet<>();
+		authorizationConsentBuilder.authorities(authorities::addAll);
+
+		if (authorities.isEmpty()) {
+			// Authorization consent denied (or revoked)
+			if (currentAuthorizationConsent != null) {
+				this.authorizationConsentService.remove(currentAuthorizationConsent);
+			}
+			this.authorizationService.remove(authorization);
+			throwError(OAuth2ErrorCodes.ACCESS_DENIED, OAuth2ParameterNames.CLIENT_ID,
+					authorizationCodeRequestAuthentication, registeredClient, authorizationRequest);
+		}
+
 		OAuth2AuthorizationConsent authorizationConsent = authorizationConsentBuilder.build();
 		if (!authorizationConsent.equals(currentAuthorizationConsent)) {
 			this.authorizationConsentService.save(authorizationConsent);

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java

@@ -868,7 +868,52 @@ private void assertAuthorizationConsentRequestWithAuthorizationCodeResult(
 	}
 
 	@Test
-	public void authenticateWhenConsentRequestWithPreviouslyApprovedThenAuthorizationConsentUpdated() {
+	public void authenticateWhenConsentRequestApproveNoneAndRevokePreviouslyApprovedThenAuthorizationConsentRemoved() {
+		String previouslyApprovedScope = "message.read";
+		String requestedScope = "message.write";
+		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
+				.scopes(scopes -> {
+					scopes.clear();
+					scopes.add(previouslyApprovedScope);
+					scopes.add(requestedScope);
+				})
+				.build();
+		when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
+				.thenReturn(registeredClient);
+		OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient)
+				.principalName(this.principal.getName())
+				.build();
+		OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(OAuth2AuthorizationRequest.class.getName());
+		OAuth2AuthorizationCodeRequestAuthenticationToken authentication =
+				authorizationConsentRequestAuthentication(registeredClient, this.principal)
+						.scopes(new HashSet<>())	// No scopes approved
+						.build();
+		when(this.authorizationService.findByToken(eq(authentication.getState()), eq(STATE_TOKEN_TYPE)))
+				.thenReturn(authorization);
+		OAuth2AuthorizationConsent previousAuthorizationConsent =
+				OAuth2AuthorizationConsent.withId(authorization.getRegisteredClientId(), authorization.getPrincipalName())
+						.scope(previouslyApprovedScope)
+						.build();
+		when(this.authorizationConsentService.findById(eq(authorization.getRegisteredClientId()), eq(authorization.getPrincipalName())))
+				.thenReturn(previousAuthorizationConsent);
+
+		// Revoke all (including previously approved)
+		this.authenticationProvider.setAuthorizationConsentCustomizer((authorizationConsentContext) ->
+				authorizationConsentContext.getAuthorizationConsent().authorities(Set::clear));
+
+		assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
+				.isInstanceOf(OAuth2AuthorizationCodeRequestAuthenticationException.class)
+				.satisfies(ex ->
+						assertAuthenticationException((OAuth2AuthorizationCodeRequestAuthenticationException) ex,
+								OAuth2ErrorCodes.ACCESS_DENIED, OAuth2ParameterNames.CLIENT_ID, authorizationRequest.getRedirectUri())
+				);
+
+		verify(this.authorizationConsentService).remove(eq(previousAuthorizationConsent));
+		verify(this.authorizationService).remove(eq(authorization));
+	}
+
+	@Test
+	public void authenticateWhenConsentRequestApproveSomeAndPreviouslyApprovedThenAuthorizationConsentUpdated() {
 		String previouslyApprovedScope = "message.read";
 		String requestedScope = "message.write";
 		String otherPreviouslyApprovedScope = "other.scope";
@@ -923,7 +968,7 @@ public void authenticateWhenConsentRequestWithPreviouslyApprovedThenAuthorizatio
 	}
 
 	@Test
-	public void authenticateWhenConsentRequestApproveNoneButPreviouslyApprovedThenAuthorizationConsentNotUpdated() {
+	public void authenticateWhenConsentRequestApproveNoneAndPreviouslyApprovedThenAuthorizationConsentNotUpdated() {
 		String previouslyApprovedScope = "message.read";
 		String requestedScope = "message.write";
 		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()