Skip to content

Commit 43245a5

Browse files
willemvdwillemvd
committed
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 <[email protected]>
1 parent dffe22a commit 43245a5

13 files changed

+372
-50
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,8 @@ public Authentication authenticate(Authentication authentication) throws Authent
283283
? currentAuthorizationConsent.getScopes() : null;
284284

285285
return new OAuth2AuthorizationConsentAuthenticationToken(authorizationRequest.getAuthorizationUri(),
286-
registeredClient.getClientId(), principal, state, currentAuthorizedScopes, null);
286+
registeredClient.getClientId(), principal, state, authorizationRequest.getScopes(),
287+
currentAuthorizedScopes, null);
287288
}
288289

289290
OAuth2TokenContext tokenContext = createAuthorizationCodeTokenContext(authorizationCodeRequestAuthentication,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
142142
OAuth2AuthorizationRequest authorizationRequest = authorization
143143
.getAttribute(OAuth2AuthorizationRequest.class.getName());
144144
Set<String> requestedScopes = authorizationRequest.getScopes();
145-
Set<String> authorizedScopes = new HashSet<>(authorizationConsentAuthentication.getScopes());
145+
Set<String> authorizedScopes = new HashSet<>(authorizationConsentAuthentication.getRequestedScopes());
146146
if (!requestedScopes.containsAll(authorizedScopes)) {
147147
throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE, authorizationConsentAuthentication,
148148
registeredClient, authorizationRequest);
@@ -354,7 +354,7 @@ private static void throwError(OAuth2Error error, String parameterName,
354354
String state = (authorizationRequest != null) ? authorizationRequest.getState()
355355
: authorizationConsentAuthentication.getState();
356356
Set<String> requestedScopes = (authorizationRequest != null) ? authorizationRequest.getScopes()
357-
: authorizationConsentAuthentication.getScopes();
357+
: authorizationConsentAuthentication.getRequestedScopes();
358358

359359
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken(
360360
authorizationConsentAuthentication.getAuthorizationUri(),

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

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ public class OAuth2AuthorizationConsentAuthenticationToken extends AbstractAuthe
4949

5050
private final String state;
5151

52-
private final Set<String> scopes;
52+
private final Set<String> authorizedScopes;
53+
54+
private final Set<String> requestedScopes;
5355

5456
private final Map<String, Object> additionalParameters;
5557

@@ -60,12 +62,29 @@ public class OAuth2AuthorizationConsentAuthenticationToken extends AbstractAuthe
6062
* @param clientId the client identifier
6163
* @param principal the {@code Principal} (Resource Owner)
6264
* @param state the state
63-
* @param scopes the requested (or authorized) scope(s)
65+
* @param requestedScopes the requested scope(s)
6466
* @param additionalParameters the additional parameters
6567
*/
6668
public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, String clientId,
67-
Authentication principal, String state, @Nullable Set<String> scopes,
69+
Authentication principal, String state, @Nullable Set<String> requestedScopes,
6870
@Nullable Map<String, Object> additionalParameters) {
71+
this(authorizationUri, clientId, principal, state, requestedScopes, null, additionalParameters);
72+
}
73+
74+
/**
75+
* Constructs an {@code OAuth2AuthorizationConsentAuthenticationToken} using the
76+
* provided parameters.
77+
* @param authorizationUri the authorization URI
78+
* @param clientId the client identifier
79+
* @param principal the {@code Principal} (Resource Owner)
80+
* @param state the state
81+
* @param requestedScopes the requested scope(s)
82+
* @param authorizedScopes the authorized scope(s)
83+
* @param additionalParameters the additional parameters
84+
*/
85+
public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, String clientId,
86+
Authentication principal, String state, @Nullable Set<String> requestedScopes,
87+
@Nullable Set<String> authorizedScopes, @Nullable Map<String, Object> additionalParameters) {
6988
super(Collections.emptyList());
7089
Assert.hasText(authorizationUri, "authorizationUri cannot be empty");
7190
Assert.hasText(clientId, "clientId cannot be empty");
@@ -75,7 +94,10 @@ public OAuth2AuthorizationConsentAuthenticationToken(String authorizationUri, St
7594
this.clientId = clientId;
7695
this.principal = principal;
7796
this.state = state;
78-
this.scopes = Collections.unmodifiableSet((scopes != null) ? new HashSet<>(scopes) : Collections.emptySet());
97+
this.requestedScopes = Collections
98+
.unmodifiableSet((requestedScopes != null) ? new HashSet<>(requestedScopes) : Collections.emptySet());
99+
this.authorizedScopes = Collections
100+
.unmodifiableSet((authorizedScopes != null) ? new HashSet<>(authorizedScopes) : Collections.emptySet());
79101
this.additionalParameters = Collections.unmodifiableMap(
80102
(additionalParameters != null) ? new HashMap<>(additionalParameters) : Collections.emptyMap());
81103
setAuthenticated(true);
@@ -116,12 +138,19 @@ public String getState() {
116138
}
117139

118140
/**
119-
* Returns the requested (or authorized) scope(s).
120-
* @return the requested (or authorized) scope(s), or an empty {@code Set} if not
121-
* available
141+
* Returns the requested scope(s).
142+
* @return the requested scope(s), or an empty {@code Set} if not available
143+
*/
144+
public Set<String> getRequestedScopes() {
145+
return this.requestedScopes;
146+
}
147+
148+
/**
149+
* Returns the authorized scope(s).
150+
* @return the authorized scope(s), or an empty {@code Set} if not available
122151
*/
123-
public Set<String> getScopes() {
124-
return this.scopes;
152+
public Set<String> getAuthorizedScopes() {
153+
return this.authorizedScopes;
125154
}
126155

127156
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
122122
}
123123

124124
Set<String> requestedScopes = authorization.getAttribute(OAuth2ParameterNames.SCOPE);
125-
Set<String> authorizedScopes = new HashSet<>(deviceAuthorizationConsentAuthentication.getScopes());
125+
Set<String> authorizedScopes = new HashSet<>(deviceAuthorizationConsentAuthentication.getRequestedScopes());
126126
if (!requestedScopes.containsAll(authorizedScopes)) {
127127
throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE);
128128
}

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

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
package org.springframework.security.oauth2.server.authorization.authentication;
1717

1818
import java.io.Serial;
19-
import java.util.Collections;
20-
import java.util.HashSet;
2119
import java.util.Map;
2220
import java.util.Set;
2321

@@ -42,8 +40,6 @@ public class OAuth2DeviceAuthorizationConsentAuthenticationToken extends OAuth2A
4240

4341
private final String userCode;
4442

45-
private final Set<String> requestedScopes;
46-
4743
/**
4844
* Constructs an {@code OAuth2DeviceAuthorizationConsentAuthenticationToken} using the
4945
* provided parameters.
@@ -52,16 +48,15 @@ public class OAuth2DeviceAuthorizationConsentAuthenticationToken extends OAuth2A
5248
* @param principal the {@code Principal} (Resource Owner)
5349
* @param userCode the user code associated with the device authorization response
5450
* @param state the state
55-
* @param authorizedScopes the authorized scope(s)
51+
* @param requestedScopes the requested scope(s)
5652
* @param additionalParameters the additional parameters
5753
*/
5854
public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationUri, String clientId,
59-
Authentication principal, String userCode, String state, @Nullable Set<String> authorizedScopes,
55+
Authentication principal, String userCode, String state, @Nullable Set<String> requestedScopes,
6056
@Nullable Map<String, Object> additionalParameters) {
61-
super(authorizationUri, clientId, principal, state, authorizedScopes, additionalParameters);
57+
super(authorizationUri, clientId, principal, state, requestedScopes, null, additionalParameters);
6258
Assert.hasText(userCode, "userCode cannot be empty");
6359
this.userCode = userCode;
64-
this.requestedScopes = null;
6560
setAuthenticated(false);
6661
}
6762

@@ -79,11 +74,9 @@ public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationU
7974
public OAuth2DeviceAuthorizationConsentAuthenticationToken(String authorizationUri, String clientId,
8075
Authentication principal, String userCode, String state, @Nullable Set<String> requestedScopes,
8176
@Nullable Set<String> authorizedScopes) {
82-
super(authorizationUri, clientId, principal, state, authorizedScopes, null);
77+
super(authorizationUri, clientId, principal, state, requestedScopes, authorizedScopes, null);
8378
Assert.hasText(userCode, "userCode cannot be empty");
8479
this.userCode = userCode;
85-
this.requestedScopes = Collections
86-
.unmodifiableSet((requestedScopes != null) ? new HashSet<>(requestedScopes) : Collections.emptySet());
8780
setAuthenticated(true);
8881
}
8982

@@ -95,12 +88,4 @@ public String getUserCode() {
9588
return this.userCode;
9689
}
9790

98-
/**
99-
* Returns the requested scopes.
100-
* @return the requested scopes
101-
*/
102-
public Set<String> getRequestedScopes() {
103-
return this.requestedScopes;
104-
}
105-
10691
}

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
191191
if (this.logger.isTraceEnabled()) {
192192
this.logger.trace("Authorization consent is required");
193193
}
194-
sendAuthorizationConsent(request, response,
195-
(OAuth2AuthorizationCodeRequestAuthenticationToken) authentication,
196-
authorizationConsentAuthenticationToken);
194+
sendAuthorizationConsent(request, response, authorizationConsentAuthenticationToken);
197195
return;
198196
}
199197

@@ -287,13 +285,12 @@ public void setConsentPage(String consentPage) {
287285
}
288286

289287
private void sendAuthorizationConsent(HttpServletRequest request, HttpServletResponse response,
290-
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication,
291288
OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthentication) throws IOException {
292289

293290
String clientId = authorizationConsentAuthentication.getClientId();
294291
Authentication principal = (Authentication) authorizationConsentAuthentication.getPrincipal();
295-
Set<String> requestedScopes = authorizationCodeRequestAuthentication.getScopes();
296-
Set<String> authorizedScopes = authorizationConsentAuthentication.getScopes();
292+
Set<String> requestedScopes = authorizationConsentAuthentication.getRequestedScopes();
293+
Set<String> authorizedScopes = authorizationConsentAuthentication.getAuthorizedScopes();
297294
String state = authorizationConsentAuthentication.getState();
298295

299296
if (hasConsentUri()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ private void sendAuthorizationConsent(HttpServletRequest request, HttpServletRes
256256
String clientId = authorizationConsentAuthentication.getClientId();
257257
Authentication principal = (Authentication) authorizationConsentAuthentication.getPrincipal();
258258
Set<String> requestedScopes = authorizationConsentAuthentication.getRequestedScopes();
259-
Set<String> authorizedScopes = authorizationConsentAuthentication.getScopes();
259+
Set<String> authorizedScopes = authorizationConsentAuthentication.getAuthorizedScopes();
260260
String state = authorizationConsentAuthentication.getState();
261261
String userCode = authorizationConsentAuthentication.getUserCode();
262262

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,9 @@ public void authenticateWhenRequireAuthorizationConsentThenReturnAuthorizationCo
506506
assertThat(authenticationResult.getClientId()).isEqualTo(registeredClient.getClientId());
507507
assertThat(authenticationResult.getPrincipal()).isEqualTo(this.principal);
508508
assertThat(authenticationResult.getAuthorizationUri()).isEqualTo(authorizationRequest.getAuthorizationUri());
509-
assertThat(authenticationResult.getScopes()).isEmpty();
509+
assertThat(authenticationResult.getAuthorizedScopes()).isEmpty();
510+
assertThat(authenticationResult.getRequestedScopes())
511+
.contains(registeredClient.getScopes().toArray(new String[0]));
510512
assertThat(authenticationResult.getState()).isEqualTo(state);
511513
assertThat(authenticationResult.isAuthenticated()).isTrue();
512514
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ public void authenticateWhenAuthorizationConsentDoesNotExistThenReturnAuthorizat
262262
assertThat(authenticationResult.getUserCode()).isEqualTo(USER_CODE);
263263
assertThat(authenticationResult.getState()).hasSize(44);
264264
assertThat(authenticationResult.getRequestedScopes()).hasSameElementsAs(registeredClient.getScopes());
265-
assertThat(authenticationResult.getScopes()).isEmpty();
265+
assertThat(authenticationResult.getAuthorizedScopes()).isEmpty();
266266

267267
ArgumentCaptor<OAuth2Authorization> authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class);
268268
verify(this.authorizationService).findByToken(USER_CODE,
@@ -365,7 +365,7 @@ public void authenticateWhenAuthorizationConsentExistsAndRequestedScopesDoNotMat
365365
assertThat(authenticationResult.getUserCode()).isEqualTo(USER_CODE);
366366
assertThat(authenticationResult.getState()).hasSize(44);
367367
assertThat(authenticationResult.getRequestedScopes()).hasSameElementsAs(registeredClient.getScopes());
368-
assertThat(authenticationResult.getScopes()).containsExactly("previous");
368+
assertThat(authenticationResult.getAuthorizedScopes()).containsExactly("previous");
369369

370370
ArgumentCaptor<OAuth2Authorization> authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class);
371371
verify(this.authorizationService).findByToken(USER_CODE,

0 commit comments

Comments
 (0)