Fix device authorization flow without requested scopes#19257
Open
therepanic wants to merge 1 commit into
Open
Conversation
The main issue is inconsistent behavior. A `device_authorization` request without a `scope` is always accepted, but then, during the `consent` phase, if `authorities` is empty, we always get an `access_denied` error. However, would not it make sense to continue processing when both `scope` and `authorities` are empty? The `scope` parameter itself is optional according to [RFC-8628](https://datatracker.ietf.org/doc/html/rfc8628#section-3.1). There is also a note attached to it that refers to [Section 3.3 in RFC-6749](https://datatracker.ietf.org/doc/html/rfc6749#section-3.3). It states that if `scope` is empty, then we must either handle it with a default value or throw an error. Closes: spring-projectsgh-19238 Ref: spring-projectsgh-19256 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
Contributor
Author
|
I renamed the PR to avoid being tied to the commit title in case we decide to change direction. |
2 tasks
therepanic
commented
Jun 2, 2026
Comment on lines
+207
to
+209
| if (!authorities.isEmpty()) { | ||
| OAuth2AuthorizationConsent authorizationConsent = authorizationConsentBuilder.build(); | ||
| if (currentAuthorizationConsent == null || !authorizationConsent.equals(currentAuthorizationConsent)) { |
Contributor
Author
There was a problem hiding this comment.
If left unprocessed, the following OAuth2AuthorizationConsent.Builder#build assertiong will be executed
public OAuth2AuthorizationConsent build() {
Assert.notEmpty(this.authorities, "authorities cannot be empty");
return new OAuth2AuthorizationConsent(this.registeredClientId, this.principalName, this.authorities);
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The main issue is inconsistent behavior. A
device_authorizationrequest without ascopeis always accepted, but then, during theconsentphase, ifauthoritiesis empty, we always get anaccess_deniederror. However, would not it make sense to continue processing when bothscopeandauthoritiesare empty?The
scopeparameter itself is optional according to RFC-8628. There is also a note attached to it that refers to Section 3.3 in RFC-6749. It states that ifscopeis empty, then we must either handle it with a default value or throw an error.And here’s the interesting part. I propose two solutions to the problem; I’ve implemented the first one in the current commit, and in the future we can pivot to the other solution. The first is to directly handle the case at the
consentstage whenscopeandauthoritiesare empty and allow the event to proceed. The second is to throw an error on adevice_authorizationrequest ifscopeis empty.I think the first solution is better because it’s also easier to maintain. With the first solution, the test breaks, but that’s because it handles all cases and is abstract, so I refined it for the case where
authoritiesis empty butscopeis not empty. The second solution breaks the specific test caseOAuth2DeviceAuthorizationRequestAuthenticationProviderTests#authenticateWhenNoScopesRequestedThenReturnDeviceCodeAndUserCode.Closes: gh-19238
Ref: gh-19256