PAR uses requested scopes on consent

PAR was missing the requested scopes when giving consent. Making consent authentications distinguish between requested and already authorized scopes.

Closes gh-2182

Signed-off-by: Willem van Dreumel <willem.vandreumel@edsn.nl>

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

@@ -20,7 +20,9 @@ import java.time.Instant;
 import java.util.Arrays;
 import java.util.Base64;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
@@ -282,8 +284,13 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
 			Set<String> currentAuthorizedScopes = (currentAuthorizationConsent != null)
 					? currentAuthorizationConsent.getScopes() : null;
 
+			Map<String, Object> additionalParameters = new HashMap<>();
+			if (pushedAuthorization != null) {
+				additionalParameters.put(OAuth2ParameterNames.SCOPE, authorizationRequest.getScopes());
+			}
+
 			return new OAuth2AuthorizationConsentAuthenticationToken(authorizationRequest.getAuthorizationUri(),
-					registeredClient.getClientId(), principal, state, currentAuthorizedScopes, null);
+					registeredClient.getClientId(), principal, state, currentAuthorizedScopes, additionalParameters);
 		}
 
 		OAuth2TokenContext tokenContext = createAuthorizationCodeTokenContext(authorizationCodeRequestAuthentication,

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java

@@ -292,10 +292,20 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte
 
 		String clientId = authorizationConsentAuthentication.getClientId();
 		Authentication principal = (Authentication) authorizationConsentAuthentication.getPrincipal();
-		Set<String> requestedScopes = authorizationCodeRequestAuthentication.getScopes();
 		Set<String> authorizedScopes = authorizationConsentAuthentication.getScopes();
 		String state = authorizationConsentAuthentication.getState();
 
+		Set<String> requestedScopes;
+		String requestUri = (String) authorizationCodeRequestAuthentication.getAdditionalParameters()
+			.get(OAuth2ParameterNames.REQUEST_URI);
+		if (StringUtils.hasText(requestUri)) {
+			requestedScopes = (Set<String>) authorizationConsentAuthentication.getAdditionalParameters()
+				.get(OAuth2ParameterNames.SCOPE);
+		}
+		else {
+			requestedScopes = authorizationCodeRequestAuthentication.getScopes();
+		}
+
 		if (hasConsentUri()) {
 			String redirectUri = UriComponentsBuilder.fromUriString(resolveConsentUri(request))
 				.queryParam(OAuth2ParameterNames.SCOPE, String.join(" ", requestedScopes))

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 class OAuth2AuthorizationCodeGrantTests {
 		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();
@@ -746,6 +781,47 @@ public class OAuth2AuthorizationCodeGrantTests {
 			.andReturn();
 	}
 
+	@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);
+		UriComponents uriComponents = UriComponentsBuilder.fromUriString(locationHeader).build();
+		MultiValueMap<String, String> 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 requestWhenCustomConsentPageConfiguredThenRedirect() throws Exception {
 		this.spring.register(AuthorizationServerConfigurationCustomConsentPage.class).autowire();
@@ -1076,6 +1152,202 @@ public class OAuth2AuthorizationCodeGrantTests {
 			.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 requestWhenPushedAuthorizationRequestAndRequiresConsentThenDisplaysConsentPageWithOnlyNewScope()
+			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 requestWhenPushedAuthorizationRequestAndCustomConsentPageConfiguredThenRedirect() 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<String, String> 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 requestWhenPushedAuthorizationRequestAndCustomConsentPageConfiguredThenRedirectWithAllScopes()
+			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);
+		UriComponents uriComponents = UriComponentsBuilder.fromUriString(locationHeader).build();
+		MultiValueMap<String, String> 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<String, Object> publicJwk = TestJwks.DEFAULT_EC_JWK
@@ -1120,8 +1392,8 @@ public class OAuth2AuthorizationCodeGrantTests {
 	private static String getAuthorizationHeader(RegisteredClient registeredClient) throws Exception {
 		String clientId = registeredClient.getClientId();
 		String clientSecret = registeredClient.getClientSecret();
-		clientId = URLEncoder.encode(clientId, StandardCharsets.UTF_8.name());
-		clientSecret = URLEncoder.encode(clientSecret, StandardCharsets.UTF_8.name());
+		clientId = URLEncoder.encode(clientId, StandardCharsets.UTF_8);
+		clientSecret = URLEncoder.encode(clientSecret, StandardCharsets.UTF_8);
 		String credentialsString = clientId + ":" + clientSecret;
 		byte[] encodedBytes = Base64.getEncoder().encode(credentialsString.getBytes(StandardCharsets.UTF_8));
 		return "Basic " + new String(encodedBytes, StandardCharsets.UTF_8);
@@ -1132,6 +1404,12 @@ public class OAuth2AuthorizationCodeGrantTests {
 				"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">", scope);
 	}
 
+	private static String disabledScopeCheckbox(String scope) {
+		return MessageFormat.format(
+				"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" id=\"{0}\" checked disabled>",
+				scope);
+	}
+
 	private String extractParameterFromRedirectUri(String redirectUri, String param)
 			throws UnsupportedEncodingException {
 		String locationHeader = URLDecoder.decode(redirectUri, StandardCharsets.UTF_8.name());
@@ -1506,4 +1784,31 @@ public class OAuth2AuthorizationCodeGrantTests {
 
 	}
 
+	@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
+
+	}
+
 }