Polish gh-1552

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2024 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.
@@ -19,6 +19,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 
 import org.springframework.lang.Nullable;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
@@ -28,13 +29,14 @@ import org.springframework.util.Assert;
 
 /**
  * An {@link OAuth2AuthenticationContext} that holds an {@link OAuth2AuthorizationCodeRequestAuthenticationToken} and additional information
- * and is used when validating the OAuth 2.0 Authorization Request used in the Authorization Code Grant.
+ * and is used when validating the OAuth 2.0 Authorization Request parameters, as well as, determining if authorization consent is required.
  *
  * @author Joe Grandja
  * @since 0.4.0
  * @see OAuth2AuthenticationContext
  * @see OAuth2AuthorizationCodeRequestAuthenticationToken
  * @see OAuth2AuthorizationCodeRequestAuthenticationProvider#setAuthenticationValidator(Consumer)
+ * @see OAuth2AuthorizationCodeRequestAuthenticationProvider#setAuthorizationConsentRequired(Predicate)
  */
 public final class OAuth2AuthorizationCodeRequestAuthenticationContext implements OAuth2AuthenticationContext {
 	private final Map<Object, Object> context;
@@ -66,22 +68,24 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationContext implement
 	}
 
 	/**
-	 * Returns the {@link OAuth2AuthorizationRequest oauth2 authorization request}.
+	 * Returns the {@link OAuth2AuthorizationRequest authorization request}.
 	 *
 	 * @return the {@link OAuth2AuthorizationRequest}
+	 * @since 1.3
 	 */
 	@Nullable
-	public OAuth2AuthorizationRequest getOAuth2AuthorizationRequest() {
+	public OAuth2AuthorizationRequest getAuthorizationRequest() {
 		return get(OAuth2AuthorizationRequest.class);
 	}
 
 	/**
-	 * Returns the {@link OAuth2AuthorizationConsent oauth2 authorization consent}.
+	 * Returns the {@link OAuth2AuthorizationConsent authorization consent}.
 	 *
 	 * @return the {@link OAuth2AuthorizationConsent}
+	 * @since 1.3
 	 */
 	@Nullable
-	public OAuth2AuthorizationConsent getOAuth2AuthorizationConsent() {
+	public OAuth2AuthorizationConsent getAuthorizationConsent() {
 		return get(OAuth2AuthorizationConsent.class);
 	}
 
@@ -116,22 +120,22 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationContext implement
 		}
 
 		/**
-		 * Sets the {@link OAuth2AuthorizationRequest oauth2 authorization request}.
+		 * Sets the {@link OAuth2AuthorizationRequest authorization request}.
 		 *
 		 * @param authorizationRequest the {@link OAuth2AuthorizationRequest}
 		 * @return the {@link Builder} for further configuration
-		 * @since 1.3.0
+		 * @since 1.3
 		 */
 		public Builder authorizationRequest(OAuth2AuthorizationRequest authorizationRequest) {
 			return put(OAuth2AuthorizationRequest.class, authorizationRequest);
 		}
 
 		/**
-		 * Sets the {@link OAuth2AuthorizationConsent oauth2 authorization consent}.
+		 * Sets the {@link OAuth2AuthorizationConsent authorization consent}.
 		 *
 		 * @param authorizationConsent the {@link OAuth2AuthorizationConsent}
 		 * @return the {@link Builder} for further configuration
-		 * @since 1.3.0
+		 * @since 1.3
 		 */
 		public Builder authorizationConsent(OAuth2AuthorizationConsent authorizationConsent) {
 			return put(OAuth2AuthorizationConsent.class, authorizationConsent);

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

@@ -81,7 +81,8 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 	private OAuth2TokenGenerator<OAuth2AuthorizationCode> authorizationCodeGenerator = new OAuth2AuthorizationCodeGenerator();
 	private Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> authenticationValidator =
 			new OAuth2AuthorizationCodeRequestAuthenticationValidator();
-	private Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> requiresAuthorizationConsent;
+	private Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> authorizationConsentRequired =
+			OAuth2AuthorizationCodeRequestAuthenticationProvider::isAuthorizationConsentRequired;
 
 	/**
 	 * Constructs an {@code OAuth2AuthorizationCodeRequestAuthenticationProvider} using the provided parameters.
@@ -98,7 +99,6 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 		this.registeredClientRepository = registeredClientRepository;
 		this.authorizationService = authorizationService;
 		this.authorizationConsentService = authorizationConsentService;
-		this.requiresAuthorizationConsent = this::requireAuthorizationConsent;
 	}
 
 	@Override
@@ -117,11 +117,10 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 			this.logger.trace("Retrieved registered client");
 		}
 
-		OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext =
+		OAuth2AuthorizationCodeRequestAuthenticationContext.Builder authenticationContextBuilder =
 				OAuth2AuthorizationCodeRequestAuthenticationContext.with(authorizationCodeRequestAuthentication)
-						.registeredClient(registeredClient)
-						.build();
-		this.authenticationValidator.accept(authenticationContext);
+						.registeredClient(registeredClient);
+		this.authenticationValidator.accept(authenticationContextBuilder.build());
 
 		if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.AUTHORIZATION_CODE)) {
 			if (this.logger.isDebugEnabled()) {
@@ -170,23 +169,15 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 				.state(authorizationCodeRequestAuthentication.getState())
 				.additionalParameters(authorizationCodeRequestAuthentication.getAdditionalParameters())
 				.build();
+		authenticationContextBuilder.authorizationRequest(authorizationRequest);
 
 		OAuth2AuthorizationConsent currentAuthorizationConsent = this.authorizationConsentService.findById(
 				registeredClient.getId(), principal.getName());
-
-		OAuth2AuthorizationCodeRequestAuthenticationContext.Builder authenticationContextBuilder =
-				OAuth2AuthorizationCodeRequestAuthenticationContext.with(authorizationCodeRequestAuthentication)
-				.registeredClient(registeredClient)
-				.authorizationRequest(authorizationRequest);
-
 		if (currentAuthorizationConsent != null) {
 			authenticationContextBuilder.authorizationConsent(currentAuthorizationConsent);
 		}
 
-		OAuth2AuthorizationCodeRequestAuthenticationContext contextWithAuthorizationRequestAndAuthorizationConsent =
-				authenticationContextBuilder.build();
-
-		if (requiresAuthorizationConsent.test(contextWithAuthorizationRequestAndAuthorizationConsent)) {
+		if (this.authorizationConsentRequired.test(authenticationContextBuilder.build())) {
 			String state = DEFAULT_STATE_GENERATOR.generateKey();
 			OAuth2Authorization authorization = authorizationBuilder(registeredClient, principal, authorizationRequest)
 					.attribute(OAuth2ParameterNames.STATE, state)
@@ -280,47 +271,44 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 	}
 
 	/**
-	 * Sets the {@link Predicate} used to determine if authorization consent is required.
+	 * Sets the {@code Predicate} used to determine if authorization consent is required.
 	 *
 	 * <p>
 	 * The {@link OAuth2AuthorizationCodeRequestAuthenticationContext} gives the predicate access to the {@link OAuth2AuthorizationCodeRequestAuthenticationToken},
 	 * as well as, the following context attributes:
-	 * {@link OAuth2AuthorizationCodeRequestAuthenticationContext#getRegisteredClient()} containing {@link RegisteredClient} used to make the request.
-	 * {@link OAuth2AuthorizationCodeRequestAuthenticationContext#getOAuth2AuthorizationRequest()} containing {@link OAuth2AuthorizationRequest}.
-	 * {@link OAuth2AuthorizationCodeRequestAuthenticationContext#getOAuth2AuthorizationConsent()} containing {@link OAuth2AuthorizationConsent} granted in the request.
+	 * <ul>
+	 * <li>The {@link RegisteredClient} associated with the authorization request.</li>
+	 * <li>The {@link OAuth2AuthorizationRequest} containing the authorization request parameters.</li>
+	 * <li>The {@link OAuth2AuthorizationConsent} previously granted to the {@link RegisteredClient}, or {@code null} if not available.</li>
+	 * </ul>
 	 *
-	 * @param requiresAuthorizationConsent the {@link Predicate} that determines if authorization consent is required.
-	 * @since 1.3.0
+	 * @param authorizationConsentRequired the {@code Predicate} used to determine if authorization consent is required
+	 * @since 1.3
 	 */
-	public void setRequiresAuthorizationConsent(Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> requiresAuthorizationConsent) {
-		Assert.notNull(requiresAuthorizationConsent, "requiresAuthorizationConsent cannot be null");
-		this.requiresAuthorizationConsent = requiresAuthorizationConsent;
+	public void setAuthorizationConsentRequired(Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> authorizationConsentRequired) {
+		Assert.notNull(authorizationConsentRequired, "authorizationConsentRequired cannot be null");
+		this.authorizationConsentRequired = authorizationConsentRequired;
 	}
 
-	private boolean requireAuthorizationConsent(OAuth2AuthorizationCodeRequestAuthenticationContext context) {
-		RegisteredClient registeredClient = context.getRegisteredClient();
-		if (!registeredClient.getClientSettings().isRequireAuthorizationConsent()) {
+	private static boolean isAuthorizationConsentRequired(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) {
+		if (!authenticationContext.getRegisteredClient().getClientSettings().isRequireAuthorizationConsent()) {
 			return false;
 		}
-
-		OAuth2AuthorizationRequest authorizationRequest = context.getOAuth2AuthorizationRequest();
 		// 'openid' scope does not require consent
-		if (authorizationRequest.getScopes().contains(OidcScopes.OPENID) &&
-				authorizationRequest.getScopes().size() == 1) {
+		if (authenticationContext.getAuthorizationRequest().getScopes().contains(OidcScopes.OPENID) &&
+				authenticationContext.getAuthorizationRequest().getScopes().size() == 1) {
 			return false;
 		}
 
-		OAuth2AuthorizationConsent authorizationConsent = context.getOAuth2AuthorizationConsent();
-		if (authorizationConsent != null &&
-				authorizationConsent.getScopes().containsAll(authorizationRequest.getScopes())) {
+		if (authenticationContext.getAuthorizationConsent() != null &&
+				authenticationContext.getAuthorizationConsent().getScopes().containsAll(authenticationContext.getAuthorizationRequest().getScopes())) {
 			return false;
 		}
 
 		return true;
 	}
 
-	private static OAuth2Authorization.Builder authorizationBuilder(RegisteredClient registeredClient,
-			Authentication principal,
+	private static OAuth2Authorization.Builder authorizationBuilder(RegisteredClient registeredClient, Authentication principal,
 			OAuth2AuthorizationRequest authorizationRequest) {
 		return OAuth2Authorization.withRegisteredClient(registeredClient)
 				.principalName(principal.getName())

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2023 the original author or authors.
+ * Copyright 2020-2024 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.
@@ -73,7 +73,6 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 	private OAuth2AuthorizationConsentService authorizationConsentService;
 	private OAuth2AuthorizationCodeRequestAuthenticationProvider authenticationProvider;
 	private TestingAuthenticationToken principal;
-	private Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> requiresAuthorizationConsent;
 
 	@BeforeEach
 	public void setUp() {
@@ -132,10 +131,10 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 	}
 
 	@Test
-	public void setRequiresAuthorizationConsentWhenNullThenThrowIllegalArgumentException() {
-		assertThatThrownBy(() -> this.authenticationProvider.setRequiresAuthorizationConsent(null))
+	public void setAuthorizationConsentRequiredWhenNullThenThrowIllegalArgumentException() {
+		assertThatThrownBy(() -> this.authenticationProvider.setAuthorizationConsentRequired(null))
 				.isInstanceOf(IllegalArgumentException.class)
-				.hasMessage("requiresAuthorizationConsent cannot be null");
+				.hasMessage("authorizationConsentRequired cannot be null");
 	}
 
 	@Test
@@ -453,64 +452,12 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 	}
 
 	@Test
-	public void authenticateWhenRequireAuthorizationConsentAndRequiresAuthorizationConsentPredicateTrueThenReturnAuthorizationConsent() {
-		this.authenticationProvider.setRequiresAuthorizationConsent((authenticationContext) -> true);
-
-		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
-				.clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build())
-				.build();
-		when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
-				.thenReturn(registeredClient);
-
-		String redirectUri = registeredClient.getRedirectUris().toArray(new String[0])[0];
-		OAuth2AuthorizationCodeRequestAuthenticationToken authentication =
-				new OAuth2AuthorizationCodeRequestAuthenticationToken(
-						AUTHORIZATION_URI, registeredClient.getClientId(), principal,
-						redirectUri, STATE, registeredClient.getScopes(), null);
-
-		OAuth2AuthorizationConsentAuthenticationToken authenticationResult =
-				(OAuth2AuthorizationConsentAuthenticationToken) this.authenticationProvider.authenticate(authentication);
-
-		ArgumentCaptor<OAuth2Authorization> authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class);
-		verify(this.authorizationService).save(authorizationCaptor.capture());
-		OAuth2Authorization authorization = authorizationCaptor.getValue();
-
-		OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(OAuth2AuthorizationRequest.class.getName());
-		assertThat(authorizationRequest.getGrantType()).isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE);
-		assertThat(authorizationRequest.getResponseType()).isEqualTo(OAuth2AuthorizationResponseType.CODE);
-		assertThat(authorizationRequest.getAuthorizationUri()).isEqualTo(authentication.getAuthorizationUri());
-		assertThat(authorizationRequest.getClientId()).isEqualTo(registeredClient.getClientId());
-		assertThat(authorizationRequest.getRedirectUri()).isEqualTo(authentication.getRedirectUri());
-		assertThat(authorizationRequest.getScopes()).isEqualTo(authentication.getScopes());
-		assertThat(authorizationRequest.getState()).isEqualTo(authentication.getState());
-		assertThat(authorizationRequest.getAdditionalParameters()).isEqualTo(authentication.getAdditionalParameters());
-
-		assertThat(authorization.getRegisteredClientId()).isEqualTo(registeredClient.getId());
-		assertThat(authorization.getPrincipalName()).isEqualTo(this.principal.getName());
-		assertThat(authorization.getAuthorizationGrantType()).isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE);
-		assertThat(authorization.<Authentication>getAttribute(Principal.class.getName())).isEqualTo(this.principal);
-		String state = authorization.getAttribute(OAuth2ParameterNames.STATE);
-		assertThat(state).isNotNull();
-		assertThat(state).isNotEqualTo(authentication.getState());
-
-		assertThat(authenticationResult.getClientId()).isEqualTo(registeredClient.getClientId());
-		assertThat(authenticationResult.getPrincipal()).isEqualTo(this.principal);
-		assertThat(authenticationResult.getAuthorizationUri()).isEqualTo(authorizationRequest.getAuthorizationUri());
-		assertThat(authenticationResult.getScopes()).isEmpty();
-		assertThat(authenticationResult.getState()).isEqualTo(state);
-		assertThat(authenticationResult.isAuthenticated()).isTrue();
-	}
-
-	@Test
-	public void authenticateWhenRequireAuthorizationConsentAndRequiresAuthorizationConsentPredicateFalseThenAuthorizationConsentNotRequired() {
-		this.authenticationProvider.setRequiresAuthorizationConsent((authenticationContext) -> false);
-
+	public void authenticateWhenRequireAuthorizationConsentAndOnlyOpenidScopeRequestedThenAuthorizationConsentNotRequired() {
 		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
 				.clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build())
 				.scopes(scopes -> {
 					scopes.clear();
 					scopes.add(OidcScopes.OPENID);
-					scopes.add(OidcScopes.EMAIL);
 				})
 				.build();
 		when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
@@ -529,18 +476,21 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 	}
 
 	@Test
-	public void authenticateWhenRequireAuthorizationConsentAndOnlyOpenidScopeRequestedThenAuthorizationConsentNotRequired() {
+	public void authenticateWhenRequireAuthorizationConsentAndAllPreviouslyApprovedThenAuthorizationConsentNotRequired() {
 		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
 				.clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build())
-				.scopes(scopes -> {
-					scopes.clear();
-					scopes.add(OidcScopes.OPENID);
-				})
 				.build();
 		when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
 				.thenReturn(registeredClient);
 
-		String redirectUri = registeredClient.getRedirectUris().toArray(new String[0])[1];
+		OAuth2AuthorizationConsent.Builder builder =
+				OAuth2AuthorizationConsent.withId(registeredClient.getId(), this.principal.getName());
+		registeredClient.getScopes().forEach(builder::scope);
+		OAuth2AuthorizationConsent previousAuthorizationConsent = builder.build();
+		when(this.authorizationConsentService.findById(eq(registeredClient.getId()), eq(this.principal.getName())))
+				.thenReturn(previousAuthorizationConsent);
+
+		String redirectUri = registeredClient.getRedirectUris().toArray(new String[0])[2];
 		OAuth2AuthorizationCodeRequestAuthenticationToken authentication =
 				new OAuth2AuthorizationCodeRequestAuthenticationToken(
 						AUTHORIZATION_URI, registeredClient.getClientId(), principal,
@@ -553,21 +503,16 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 	}
 
 	@Test
-	public void authenticateWhenRequireAuthorizationConsentAndAllPreviouslyApprovedThenAuthorizationConsentNotRequired() {
-		RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
-				.clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build())
-				.build();
+	public void authenticateWhenCustomAuthorizationConsentRequiredThenUsed() {
+		@SuppressWarnings("unchecked")
+		Predicate<OAuth2AuthorizationCodeRequestAuthenticationContext> authorizationConsentRequired = mock(Predicate.class);
+		this.authenticationProvider.setAuthorizationConsentRequired(authorizationConsentRequired);
+
+		RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
 		when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
 				.thenReturn(registeredClient);
 
-		OAuth2AuthorizationConsent.Builder builder =
-				OAuth2AuthorizationConsent.withId(registeredClient.getId(), this.principal.getName());
-		registeredClient.getScopes().forEach(builder::scope);
-		OAuth2AuthorizationConsent previousAuthorizationConsent = builder.build();
-		when(this.authorizationConsentService.findById(eq(registeredClient.getId()), eq(this.principal.getName())))
-				.thenReturn(previousAuthorizationConsent);
-
-		String redirectUri = registeredClient.getRedirectUris().toArray(new String[0])[2];
+		String redirectUri = registeredClient.getRedirectUris().toArray(new String[0])[1];
 		OAuth2AuthorizationCodeRequestAuthenticationToken authentication =
 				new OAuth2AuthorizationCodeRequestAuthenticationToken(
 						AUTHORIZATION_URI, registeredClient.getClientId(), principal,
@@ -577,6 +522,8 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests {
 				(OAuth2AuthorizationCodeRequestAuthenticationToken) this.authenticationProvider.authenticate(authentication);
 
 		assertAuthorizationCodeRequestWithAuthorizationCodeResult(registeredClient, authentication, authenticationResult);
+
+		verify(authorizationConsentRequired).test(any());
 	}
 
 	@Test