From 43245a57dd569e2288fe602dd5c7dc71a6ef7577 Mon Sep 17 00:00:00 2001 From: Willem van Dreumel <3583529+willemvd@users.noreply.github.com> Date: Sun, 31 Aug 2025 23:24:22 +0200 Subject: [PATCH] PAR using requested scopes on consent PAR was missing the scopes when giving consent. Making consent authentications distinguish between requested and already authorized scopes. Fixes gh-2175 Signed-off-by: Willem van Dreumel <3583529+willemvd@users.noreply.github.com> --- ...tionCodeRequestAuthenticationProvider.java | 3 +- ...rizationConsentAuthenticationProvider.java | 4 +- ...thorizationConsentAuthenticationToken.java | 47 ++- ...rizationConsentAuthenticationProvider.java | 2 +- ...thorizationConsentAuthenticationToken.java | 23 +- .../OAuth2AuthorizationEndpointFilter.java | 9 +- ...Auth2DeviceVerificationEndpointFilter.java | 2 +- ...odeRequestAuthenticationProviderTests.java | 4 +- ...rificationAuthenticationProviderTests.java | 4 +- .../OAuth2AuthorizationCodeGrantTests.java | 305 ++++++++++++++++++ ...Auth2AuthorizationEndpointFilterTests.java | 9 +- ...DeviceVerificationEndpointFilterTests.java | 2 +- ...onConsentAuthenticationConverterTests.java | 8 +- 13 files changed, 372 insertions(+), 50 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java index 81042cbf5..56396e7e6 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java @@ -283,7 +283,8 @@ public Authentication authenticate(Authentication authentication) throws Authent ? currentAuthorizationConsent.getScopes() : null; return new OAuth2AuthorizationConsentAuthenticationToken(authorizationRequest.getAuthorizationUri(), - registeredClient.getClientId(), principal, state, currentAuthorizedScopes, null); + registeredClient.getClientId(), principal, state, authorizationRequest.getScopes(), + currentAuthorizedScopes, null); } OAuth2TokenContext tokenContext = createAuthorizationCodeTokenContext(authorizationCodeRequestAuthentication, diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationProvider.java index 131b58ce6..ee1ee5b94 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationProvider.java @@ -142,7 +142,7 @@ public Authentication authenticate(Authentication authentication) throws Authent OAuth2AuthorizationRequest authorizationRequest = authorization .getAttribute(OAuth2AuthorizationRequest.class.getName()); Set requestedScopes = authorizationRequest.getScopes(); - Set authorizedScopes = new HashSet<>(authorizationConsentAuthentication.getScopes()); + Set authorizedScopes = new HashSet<>(authorizationConsentAuthentication.getRequestedScopes()); if (!requestedScopes.containsAll(authorizedScopes)) { throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE, authorizationConsentAuthentication, registeredClient, authorizationRequest); @@ -354,7 +354,7 @@ private static void throwError(OAuth2Error error, String parameterName, String state = (authorizationRequest != null) ? authorizationRequest.getState() : authorizationConsentAuthentication.getState(); Set requestedScopes = (authorizationRequest != null) ? authorizationRequest.getScopes() - : authorizationConsentAuthentication.getScopes(); + : authorizationConsentAuthentication.getRequestedScopes(); OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( authorizationConsentAuthentication.getAuthorizationUri(), diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationToken.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationToken.java index f278bfeda..edddf4868 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationToken.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationConsentAuthenticationToken.java @@ -49,7 +49,9 @@ public class OAuth2AuthorizationConsentAuthenticationToken extends AbstractAuthe private final String state; - private final Set scopes; + private final Set authorizedScopes; + + private final Set requestedScopes; private final Map additionalParameters; @@ -60,12 +62,29 @@ public class OAuth2AuthorizationConsentAuthenticationToken extends AbstractAuthe * @param clientId the client identifier * @param principal the {@code Principal} (Resource Owner) * @param state the state - * @param scopes the requested (or authorized) scope(s) + * @param requestedScopes the requested scope(s) * @param additionalParameters the additional parameters */ public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, String clientId, - Authentication principal, String state, @Nullable Set scopes, + Authentication principal, String state, @Nullable Set requestedScopes, @Nullable Map additionalParameters) { + this(authorizationUri, clientId, principal, state, requestedScopes, null, additionalParameters); + } + + /** + * Constructs an {@code OAuth2AuthorizationConsentAuthenticationToken} using the + * provided parameters. + * @param authorizationUri the authorization URI + * @param clientId the client identifier + * @param principal the {@code Principal} (Resource Owner) + * @param state the state + * @param requestedScopes the requested scope(s) + * @param authorizedScopes the authorized scope(s) + * @param additionalParameters the additional parameters + */ + public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, String clientId, + Authentication principal, String state, @Nullable Set requestedScopes, + @Nullable Set authorizedScopes, @Nullable Map additionalParameters) { super(Collections.emptyList()); Assert.hasText(authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(clientId, "clientId cannot be empty"); @@ -75,7 +94,10 @@ public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, St this.clientId = clientId; this.principal = principal; this.state = state; - this.scopes = Collections.unmodifiableSet((scopes != null) ? new HashSet<>(scopes) : Collections.emptySet()); + this.requestedScopes = Collections + .unmodifiableSet((requestedScopes != null) ? new HashSet<>(requestedScopes) : Collections.emptySet()); + this.authorizedScopes = Collections + .unmodifiableSet((authorizedScopes != null) ? new HashSet<>(authorizedScopes) : Collections.emptySet()); this.additionalParameters = Collections.unmodifiableMap( (additionalParameters != null) ? new HashMap<>(additionalParameters) : Collections.emptyMap()); setAuthenticated(true); @@ -116,12 +138,19 @@ public String getState() { } /** - * Returns the requested (or authorized) scope(s). - * @return the requested (or authorized) scope(s), or an empty {@code Set} if not - * available + * Returns the requested scope(s). + * @return the requested scope(s), or an empty {@code Set} if not available + */ + public Set getRequestedScopes() { + return this.requestedScopes; + } + + /** + * Returns the authorized scope(s). + * @return the authorized scope(s), or an empty {@code Set} if not available */ - public Set getScopes() { - return this.scopes; + public Set getAuthorizedScopes() { + return this.authorizedScopes; } /** diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationProvider.java index 45fea1207..ccfc1e4f6 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationProvider.java @@ -122,7 +122,7 @@ public Authentication authenticate(Authentication authentication) throws Authent } Set requestedScopes = authorization.getAttribute(OAuth2ParameterNames.SCOPE); - Set authorizedScopes = new HashSet<>(deviceAuthorizationConsentAuthentication.getScopes()); + Set authorizedScopes = new HashSet<>(deviceAuthorizationConsentAuthentication.getRequestedScopes()); if (!requestedScopes.containsAll(authorizedScopes)) { throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationToken.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationToken.java index 3e3dd0390..7732d3d3b 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationToken.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationConsentAuthenticationToken.java @@ -16,8 +16,6 @@ package org.springframework.security.oauth2.server.authorization.authentication; import java.io.Serial; -import java.util.Collections; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -42,8 +40,6 @@ public class OAuth2DeviceAuthorizationConsentAuthenticationToken extends OAuth2A private final String userCode; - private final Set requestedScopes; - /** * Constructs an {@code OAuth2DeviceAuthorizationConsentAuthenticationToken} using the * provided parameters. @@ -52,16 +48,15 @@ public class OAuth2DeviceAuthorizationConsentAuthenticationToken extends OAuth2A * @param principal the {@code Principal} (Resource Owner) * @param userCode the user code associated with the device authorization response * @param state the state - * @param authorizedScopes the authorized scope(s) + * @param requestedScopes the requested scope(s) * @param additionalParameters the additional parameters */ public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationUri, String clientId, - Authentication principal, String userCode, String state, @Nullable Set authorizedScopes, + Authentication principal, String userCode, String state, @Nullable Set requestedScopes, @Nullable Map additionalParameters) { - super(authorizationUri, clientId, principal, state, authorizedScopes, additionalParameters); + super(authorizationUri, clientId, principal, state, requestedScopes, null, additionalParameters); Assert.hasText(userCode, "userCode cannot be empty"); this.userCode = userCode; - this.requestedScopes = null; setAuthenticated(false); } @@ -79,11 +74,9 @@ public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationU public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationUri, String clientId, Authentication principal, String userCode, String state, @Nullable Set requestedScopes, @Nullable Set authorizedScopes) { - super(authorizationUri, clientId, principal, state, authorizedScopes, null); + super(authorizationUri, clientId, principal, state, requestedScopes, authorizedScopes, null); Assert.hasText(userCode, "userCode cannot be empty"); this.userCode = userCode; - this.requestedScopes = Collections - .unmodifiableSet((requestedScopes != null) ? new HashSet<>(requestedScopes) : Collections.emptySet()); setAuthenticated(true); } @@ -95,12 +88,4 @@ public String getUserCode() { return this.userCode; } - /** - * Returns the requested scopes. - * @return the requested scopes - */ - public Set getRequestedScopes() { - return this.requestedScopes; - } - } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index d89a6f108..efaf0c289 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -191,9 +191,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse if (this.logger.isTraceEnabled()) { this.logger.trace("Authorization consent is required"); } - sendAuthorizationConsent(request, response, - (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication, - authorizationConsentAuthenticationToken); + sendAuthorizationConsent(request, response, authorizationConsentAuthenticationToken); return; } @@ -287,13 +285,12 @@ public void setConsentPage(String consentPage) { } private void sendAuthorizationConsent(HttpServletRequest request, HttpServletResponse response, - OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication, OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthentication) throws IOException { String clientId = authorizationConsentAuthentication.getClientId(); Authentication principal = (Authentication) authorizationConsentAuthentication.getPrincipal(); - Set requestedScopes = authorizationCodeRequestAuthentication.getScopes(); - Set authorizedScopes = authorizationConsentAuthentication.getScopes(); + Set requestedScopes = authorizationConsentAuthentication.getRequestedScopes(); + Set authorizedScopes = authorizationConsentAuthentication.getAuthorizedScopes(); String state = authorizationConsentAuthentication.getState(); if (hasConsentUri()) { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java index 8f91a6562..3af3996be 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java @@ -256,7 +256,7 @@ private void sendAuthorizationConsent(HttpServletRequest request, HttpServletRes String clientId = authorizationConsentAuthentication.getClientId(); Authentication principal = (Authentication) authorizationConsentAuthentication.getPrincipal(); Set requestedScopes = authorizationConsentAuthentication.getRequestedScopes(); - Set authorizedScopes = authorizationConsentAuthentication.getScopes(); + Set authorizedScopes = authorizationConsentAuthentication.getAuthorizedScopes(); String state = authorizationConsentAuthentication.getState(); String userCode = authorizationConsentAuthentication.getUserCode(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java index 21bf3fe00..e40678e2e 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java @@ -506,7 +506,9 @@ public void authenticateWhenRequireAuthorizationConsentThenReturnAuthorizationCo assertThat(authenticationResult.getClientId()).isEqualTo(registeredClient.getClientId()); assertThat(authenticationResult.getPrincipal()).isEqualTo(this.principal); assertThat(authenticationResult.getAuthorizationUri()).isEqualTo(authorizationRequest.getAuthorizationUri()); - assertThat(authenticationResult.getScopes()).isEmpty(); + assertThat(authenticationResult.getAuthorizedScopes()).isEmpty(); + assertThat(authenticationResult.getRequestedScopes()) + .contains(registeredClient.getScopes().toArray(new String[0])); assertThat(authenticationResult.getState()).isEqualTo(state); assertThat(authenticationResult.isAuthenticated()).isTrue(); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java index fd6a54d60..eca82e779 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java @@ -262,7 +262,7 @@ public void authenticateWhenAuthorizationConsentDoesNotExistThenReturnAuthorizat assertThat(authenticationResult.getUserCode()).isEqualTo(USER_CODE); assertThat(authenticationResult.getState()).hasSize(44); assertThat(authenticationResult.getRequestedScopes()).hasSameElementsAs(registeredClient.getScopes()); - assertThat(authenticationResult.getScopes()).isEmpty(); + assertThat(authenticationResult.getAuthorizedScopes()).isEmpty(); ArgumentCaptor authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class); verify(this.authorizationService).findByToken(USER_CODE, @@ -365,7 +365,7 @@ public void authenticateWhenAuthorizationConsentExistsAndRequestedScopesDoNotMat assertThat(authenticationResult.getUserCode()).isEqualTo(USER_CODE); assertThat(authenticationResult.getState()).hasSize(44); assertThat(authenticationResult.getRequestedScopes()).hasSameElementsAs(registeredClient.getScopes()); - assertThat(authenticationResult.getScopes()).containsExactly("previous"); + assertThat(authenticationResult.getAuthorizedScopes()).containsExactly("previous"); ArgumentCaptor authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class); verify(this.authorizationService).findByToken(USER_CODE, diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java index 55a2bdbe7..6525424f9 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java @@ -232,6 +232,9 @@ public class OAuth2AuthorizationCodeGrantTests { @Autowired private OAuth2AuthorizationService authorizationService; + @Autowired + private OAuth2AuthorizationConsentService authorizationConsentService; + @Autowired private JwtDecoder jwtDecoder; @@ -689,6 +692,38 @@ public void requestWhenRequiresConsentThenDisplaysConsentPage() throws Exception assertThat(consentPage).contains(scopeCheckbox("message.write")); } + @Test + public void requestWhenRequiresConsentThenDisplaysConsentPageWithOnlyNewScope() throws Exception { + this.spring.register(AuthorizationServerConfiguration.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + OAuth2AuthorizationConsent authorizationConsent = OAuth2AuthorizationConsent + .withId(registeredClient.getId(), "user") + .scope("message.write") + .build(); + + this.authorizationConsentService.save(authorizationConsent); + + String consentPage = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParams(getAuthorizationRequestParameters(registeredClient)) + .with(user("user"))) + .andExpect(status().is2xxSuccessful()) + .andReturn() + .getResponse() + .getContentAsString(); + + assertThat(consentPage).contains("Consent required"); + assertThat(consentPage).contains(scopeCheckbox("message.read")); + assertThat(consentPage).contains(disabledScopeCheckbox("message.write")); + } + @Test public void requestWhenConsentRequestThenReturnAccessTokenResponse() throws Exception { this.spring.register(AuthorizationServerConfiguration.class).autowire(); @@ -780,6 +815,47 @@ public void requestWhenCustomConsentPageConfiguredThenRedirect() throws Exceptio assertThat(authorization).isNotNull(); } + @Test + public void requestWhenCustomConsentPageConfiguredThenRedirectWithAllScopes() throws Exception { + this.spring.register(AuthorizationServerConfigurationCustomConsentPage.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + OAuth2AuthorizationConsent authorizationConsent = OAuth2AuthorizationConsent + .withId(registeredClient.getId(), "user") + .scope("message.write") + .build(); + + this.authorizationConsentService.save(authorizationConsent); + + MvcResult mvcResult = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParams(getAuthorizationRequestParameters(registeredClient)) + .with(user("user"))) + .andExpect(status().is3xxRedirection()) + .andReturn(); + String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); + assertThat(redirectedUrl).matches("http://localhost/oauth2/consent\\?scope=.+&client_id=.+&state=.+"); + + String locationHeader = URLDecoder.decode(redirectedUrl, StandardCharsets.UTF_8.name()); + UriComponents uriComponents = UriComponentsBuilder.fromUriString(locationHeader).build(); + MultiValueMap redirectQueryParams = uriComponents.getQueryParams(); + + assertThat(uriComponents.getPath()).isEqualTo(consentPage); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.SCOPE)).isEqualTo("message.read message.write"); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.CLIENT_ID)) + .isEqualTo(registeredClient.getClientId()); + + String state = extractParameterFromRedirectUri(redirectedUrl, "state"); + OAuth2Authorization authorization = this.authorizationService.findByToken(state, STATE_TOKEN_TYPE); + assertThat(authorization).isNotNull(); + } + @Test public void requestWhenCustomConsentCustomizerConfiguredThenUsed() throws Exception { this.spring.register(AuthorizationServerConfigurationCustomConsentRequest.class).autowire(); @@ -1076,6 +1152,202 @@ public void requestWhenPushedAuthorizationRequestThenReturnAccessTokenResponse() .isEqualTo(true); } + @Test + public void requestWhenPushedAuthorizationRequestAndRequiresConsentThenDisplaysConsentPage() throws Exception { + this.spring.register(AuthorizationServerConfigurationWithPushedAuthorizationRequests.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + MvcResult mvcResult = this.mvc + .perform(post("/oauth2/par").params(getAuthorizationRequestParameters(registeredClient)) + .param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE) + .param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256") + .header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient))) + .andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store"))) + .andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache"))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.request_uri").isNotEmpty()) + .andExpect(jsonPath("$.expires_in").isNotEmpty()) + .andReturn(); + + String requestUri = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.request_uri"); + + String consentPage = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .queryParam(OAuth2ParameterNames.REQUEST_URI, requestUri) + .with(user("user"))) + .andExpect(status().is2xxSuccessful()) + .andReturn() + .getResponse() + .getContentAsString(); + + assertThat(consentPage).contains("Consent required"); + assertThat(consentPage).contains(scopeCheckbox("message.read")); + assertThat(consentPage).contains(scopeCheckbox("message.write")); + } + + @Test + public void requestPushedAuthorizationRequestAndRequiresConsentThenDisplaysConsentPageWithOnlyNewScope() + throws Exception { + this.spring.register(AuthorizationServerConfigurationWithPushedAuthorizationRequests.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + OAuth2AuthorizationConsent authorizationConsent = OAuth2AuthorizationConsent + .withId(registeredClient.getId(), "user") + .scope("message.write") + .build(); + + this.authorizationConsentService.save(authorizationConsent); + + MvcResult mvcResult = this.mvc + .perform(post("/oauth2/par").params(getAuthorizationRequestParameters(registeredClient)) + .param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE) + .param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256") + .header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient))) + .andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store"))) + .andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache"))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.request_uri").isNotEmpty()) + .andExpect(jsonPath("$.expires_in").isNotEmpty()) + .andReturn(); + + String requestUri = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.request_uri"); + + String consentPage = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .queryParam(OAuth2ParameterNames.REQUEST_URI, requestUri) + .with(user("user"))) + .andExpect(status().is2xxSuccessful()) + .andReturn() + .getResponse() + .getContentAsString(); + + assertThat(consentPage).contains("Consent required"); + assertThat(consentPage).contains(scopeCheckbox("message.read")); + assertThat(consentPage).contains(disabledScopeCheckbox("message.write")); + } + + @Test + public void requestWhenWhenPushedAuthorizationRequestAndCustomConsentPageConfiguredThenRedirect() throws Exception { + this.spring.register(AuthorizationServerConfigurationWithPushedAuthorizationRequestsAndCustomConsentPage.class) + .autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + MvcResult mvcResult = this.mvc + .perform(post("/oauth2/par").params(getAuthorizationRequestParameters(registeredClient)) + .param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE) + .param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256") + .header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient))) + .andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store"))) + .andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache"))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.request_uri").isNotEmpty()) + .andExpect(jsonPath("$.expires_in").isNotEmpty()) + .andReturn(); + + String requestUri = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.request_uri"); + + mvcResult = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .queryParam(OAuth2ParameterNames.REQUEST_URI, requestUri) + .with(user("user"))) + .andExpect(status().is3xxRedirection()) + .andReturn(); + String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); + assertThat(redirectedUrl).matches("http://localhost/oauth2/consent\\?scope=.+&client_id=.+&state=.+"); + + String locationHeader = URLDecoder.decode(redirectedUrl, StandardCharsets.UTF_8.name()); + UriComponents uriComponents = UriComponentsBuilder.fromUriString(locationHeader).build(); + MultiValueMap redirectQueryParams = uriComponents.getQueryParams(); + + assertThat(uriComponents.getPath()).isEqualTo(consentPage); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.SCOPE)).isEqualTo("message.read message.write"); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.CLIENT_ID)) + .isEqualTo(registeredClient.getClientId()); + + String state = extractParameterFromRedirectUri(redirectedUrl, "state"); + OAuth2Authorization authorization = this.authorizationService.findByToken(state, STATE_TOKEN_TYPE); + assertThat(authorization).isNotNull(); + } + + @Test + public void requestWhenWhenPushedAuthorizationRequestAndCustomConsentPageConfiguredThenRedirectWithAllScopes() + throws Exception { + this.spring.register(AuthorizationServerConfigurationWithPushedAuthorizationRequestsAndCustomConsentPage.class) + .autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add("message.read"); + scopes.add("message.write"); + }).clientSettings(ClientSettings.builder().requireAuthorizationConsent(true).build()).build(); + this.registeredClientRepository.save(registeredClient); + + OAuth2AuthorizationConsent authorizationConsent = OAuth2AuthorizationConsent + .withId(registeredClient.getId(), "user") + .scope("message.write") + .build(); + + this.authorizationConsentService.save(authorizationConsent); + + MvcResult mvcResult = this.mvc + .perform(post("/oauth2/par").params(getAuthorizationRequestParameters(registeredClient)) + .param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE) + .param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256") + .header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient))) + .andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store"))) + .andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache"))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.request_uri").isNotEmpty()) + .andExpect(jsonPath("$.expires_in").isNotEmpty()) + .andReturn(); + + String requestUri = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.request_uri"); + + mvcResult = this.mvc + .perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .queryParam(OAuth2ParameterNames.REQUEST_URI, requestUri) + .with(user("user"))) + .andExpect(status().is3xxRedirection()) + .andReturn(); + String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); + assertThat(redirectedUrl).matches("http://localhost/oauth2/consent\\?scope=.+&client_id=.+&state=.+"); + + String locationHeader = URLDecoder.decode(redirectedUrl, StandardCharsets.UTF_8.name()); + UriComponents uriComponents = UriComponentsBuilder.fromUriString(locationHeader).build(); + MultiValueMap redirectQueryParams = uriComponents.getQueryParams(); + + assertThat(uriComponents.getPath()).isEqualTo(consentPage); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.SCOPE)).isEqualTo("message.read message.write"); + assertThat(redirectQueryParams.getFirst(OAuth2ParameterNames.CLIENT_ID)) + .isEqualTo(registeredClient.getClientId()); + + String state = extractParameterFromRedirectUri(redirectedUrl, "state"); + OAuth2Authorization authorization = this.authorizationService.findByToken(state, STATE_TOKEN_TYPE); + assertThat(authorization).isNotNull(); + } + private static String generateDPoPProof(String tokenEndpointUri) { // @formatter:off Map publicJwk = TestJwks.DEFAULT_EC_JWK @@ -1132,6 +1404,12 @@ private static String scopeCheckbox(String scope) { "", scope); } + private static String disabledScopeCheckbox(String scope) { + return MessageFormat.format( + "", + scope); + } + private String extractParameterFromRedirectUri(String redirectUri, String param) throws UnsupportedEncodingException { String locationHeader = URLDecoder.decode(redirectUri, StandardCharsets.UTF_8.name()); @@ -1506,4 +1784,31 @@ SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) th } + @EnableWebSecurity + @Configuration(proxyBeanMethods = false) + static class AuthorizationServerConfigurationWithPushedAuthorizationRequestsAndCustomConsentPage + extends AuthorizationServerConfiguration { + + // @formatter:off + @Bean + SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { + OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = + OAuth2AuthorizationServerConfigurer.authorizationServer(); + http + .securityMatcher(authorizationServerConfigurer.getEndpointsMatcher()) + .with(authorizationServerConfigurer, (authorizationServer) -> + authorizationServer + .pushedAuthorizationRequestEndpoint(Customizer.withDefaults()) + .authorizationEndpoint((authorizationEndpoint) -> + authorizationEndpoint.consentPage(consentPage)) + ) + .authorizeHttpRequests((authorize) -> + authorize.anyRequest().authenticated() + ); + return http.build(); + } + // @formatter:on + + } + } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index 77327f4e8..c19a55149 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -502,7 +502,8 @@ public void doFilterWhenAuthorizationRequestConsentRequiredWithCustomConsentUriT }).build(); // No scopes previously approved OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( - AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, new HashSet<>(), null); + AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, requestedScopes, + new HashSet<>(), null); authorizationConsentAuthenticationResult.setAuthenticated(true); given(this.authenticationManager.authenticate(any())).willReturn(authorizationConsentAuthenticationResult); @@ -530,7 +531,8 @@ public void doFilterWhenAuthorizationRequestConsentRequiredThenConsentResponse() }).build(); // No scopes previously approved OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( - AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, new HashSet<>(), null); + AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, requestedScopes, + new HashSet<>(), null); authorizationConsentAuthenticationResult.setAuthenticated(true); given(this.authenticationManager.authenticate(any())).willReturn(authorizationConsentAuthenticationResult); @@ -561,7 +563,8 @@ public void doFilterWhenAuthorizationRequestConsentRequiredWithPreviouslyApprove scopes.addAll(requestedScopes); }).build(); OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( - AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, approvedScopes, null); + AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, STATE, registeredClient.getScopes(), + approvedScopes, null); authorizationConsentAuthenticationResult.setAuthenticated(true); given(this.authenticationManager.authenticate(any())).willReturn(authorizationConsentAuthenticationResult); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java index f755e75c3..f29114beb 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java @@ -215,7 +215,7 @@ public void doFilterWhenDeviceAuthorizationConsentRequestThenSuccess() throws Ex assertThat(deviceAuthorizationConsentAuthentication.getPrincipal()) .isInstanceOf(TestingAuthenticationToken.class); assertThat(deviceAuthorizationConsentAuthentication.getUserCode()).isEqualTo(USER_CODE); - assertThat(deviceAuthorizationConsentAuthentication.getScopes()).containsExactly("scope-1", "scope-2"); + assertThat(deviceAuthorizationConsentAuthentication.getRequestedScopes()).containsExactly("scope-1", "scope-2"); assertThat(deviceAuthorizationConsentAuthentication.getAdditionalParameters()).containsExactly( Map.entry("custom-param-1", "custom-value-1"), Map.entry("custom-param-2", new String[] { "custom-value-1", "custom-value-2" })); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java index c7ec48aee..fa816b112 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverterTests.java @@ -232,7 +232,7 @@ public void convertWhenMissingPrincipalThenReturnDeviceAuthorizationConsentAuthe assertThat(authentication.getClientId()).isEqualTo(CLIENT_ID); assertThat(authentication.getPrincipal()).isInstanceOf(AnonymousAuthenticationToken.class); assertThat(authentication.getUserCode()).isEqualTo(USER_CODE); - assertThat(authentication.getScopes()).isEmpty(); + assertThat(authentication.getRequestedScopes()).isEmpty(); assertThat(authentication.getAdditionalParameters()).isEmpty(); } @@ -254,7 +254,7 @@ public void convertWhenMissingScopeThenReturnDeviceAuthorizationConsentAuthentic assertThat(authentication.getClientId()).isEqualTo(CLIENT_ID); assertThat(authentication.getPrincipal()).isInstanceOf(TestingAuthenticationToken.class); assertThat(authentication.getUserCode()).isEqualTo(USER_CODE); - assertThat(authentication.getScopes()).isEmpty(); + assertThat(authentication.getRequestedScopes()).isEmpty(); assertThat(authentication.getAdditionalParameters()).isEmpty(); } @@ -280,7 +280,7 @@ public void convertWhenAllParametersThenReturnDeviceAuthorizationConsentAuthenti assertThat(authentication.getClientId()).isEqualTo(CLIENT_ID); assertThat(authentication.getPrincipal()).isInstanceOf(TestingAuthenticationToken.class); assertThat(authentication.getUserCode()).isEqualTo(USER_CODE); - assertThat(authentication.getScopes()).containsExactly("message.read", "message.write"); + assertThat(authentication.getRequestedScopes()).containsExactly("message.read", "message.write"); assertThat(authentication.getAdditionalParameters()).containsExactly(Map.entry("param-1", "value-1"), Map.entry("param-2", new String[] { "value-1", "value-2" })); } @@ -303,7 +303,7 @@ public void convertWhenNonNormalizedUserCodeThenReturnDeviceAuthorizationConsent assertThat(authentication.getClientId()).isEqualTo(CLIENT_ID); assertThat(authentication.getPrincipal()).isInstanceOf(TestingAuthenticationToken.class); assertThat(authentication.getUserCode()).isEqualTo(USER_CODE); - assertThat(authentication.getScopes()).isEmpty(); + assertThat(authentication.getRequestedScopes()).isEmpty(); assertThat(authentication.getAdditionalParameters()).isEmpty(); }