Skip to content

The authorize request should fail if scope request parameter is not provided #288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
anoopgarlapati opened this issue May 10, 2021 · 4 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@anoopgarlapati
Copy link
Contributor

Currently a client performing authorize request can omit the scope parameter and receive an access token without any scopes. This is neither useful nor the expected behavior for many authorization servers.
As per RFC 6749 section 3.3:

If the client omits the scope parameter when requesting
authorization, the authorization server MUST either process the
request using a pre-defined default value or fail the request
indicating an invalid scope. The authorization server SHOULD
document its scope requirements and default value (if defined).

As per the above criterion, the default behavior provided by the project should be changed to reject the request if scope parameter is not provided. Ability to customize this default behavior for using a pre-defined value as an alternative behavior suggested in the spec would be provided by custom configuration that would be delivered in #139.

@anoopgarlapati anoopgarlapati added the type: enhancement A general enhancement label May 10, 2021
@jgrandja
Copy link
Collaborator

@anoopgarlapati Would you be interested in submitting a PR for this?

@anoopgarlapati
Copy link
Contributor Author

@jgrandja Yes, I'm interested in submitting a PR for this.

@jgrandja
Copy link
Collaborator

jgrandja commented Jun 9, 2021

@anoopgarlapati I've been giving this some further thought and I now feel that we should not add this scope validation.

There are use cases where the scope parameter will not be used at all and the authorities assigned to the access token will be derived from other sources, e.g. authorities from user, resource indicators #279, custom data source, etc.

If we add this scope validation then the above use cases will not work out-of-the-box, which will be very limiting. Those applications will need to customize/disable the scope validation. This won't be a good user experience since the validation is quite restrictive.

How can we address these parts in the spec?

If the client omits the scope parameter when requesting
authorization, the authorization server MUST either process the
request using a pre-defined default value or fail the request
indicating an invalid scope
. The authorization server SHOULD
document its scope requirements and default value (if defined).

Part 1:

process the request using a pre-defined default value

This responsibility really belongs in the "product" and not the "framework". The provider product that leverages the framework will define these kinds of "policies" and document in their reference. Default values are not necessarily statically defined but could be advanced and derived from multiple sources at runtime. Based on this alone, it makes sense that the provider product owns this responsibility. The framework should only provide extension points to allow the product to derive the default scopes, which is already possible via the OAuth2TokenCustomizer.

Part 2:

fail the request indicating an invalid scope

Applying this as the default behaviour will be very restrictive. However, we can easily achieve this behaviour by allowing applications to customize the OAuth2AuthorizationEndpointFilter to allow for custom validation logic. The application would simply configure a custom validator to enforce the scope parameter.

I've updated the issue description in #139:

Allow custom validation of Authorization Request parameters, e.g. scope, redirect_uri, etc.

What do you think?

@anoopgarlapati
Copy link
Contributor Author

@jgrandja I agree with both the parts. It does seem to be too restrictive to reject the request as default behavior and as long as it is easily customizable it should not be much of a hassle for product teams to define this behavior if they want.
I am closing the issue and linked merge request.

@jgrandja jgrandja removed this from the 0.1.2 milestone Jun 11, 2021
@jgrandja jgrandja added the status: declined A suggestion or change that we don't feel we should currently apply label Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
2 participants