Skip to content
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

Issue with scopes after 5.6.0 update #1588

Open
colinbruce opened this issue Sep 15, 2022 · 4 comments
Open

Issue with scopes after 5.6.0 update #1588

colinbruce opened this issue Sep 15, 2022 · 4 comments

Comments

@colinbruce
Copy link

Steps to reproduce

What we need to do to see your problem or bug?

We have an doorkeeper configuration with no default values

Each application, that we create manually, will have one or more of the scopes that is taken from the optional_scopes array

Is this a valid way of configuring a Doorkeeper application?

We have been using the oauth_application.scopes to validate requests inside the application and don't really want users to have to understand the meaning of use_case_one, use_case_two, etc when requesting data via swagger docs
We believe this was introduced in the #1558 PR because we are not submitting a scope name in the request and have no default scopes.

Expected behavior

In the 5.5.4, and prior, release an application could authorise from swagger docs without submitting an expected scope

Actual behavior

We now get an {"error":"invalid_scope","error_description":"The requested scope is invalid, unknown, or malformed."} error.

System configuration

You can help us to understand your problem if you will share some very
useful information about your project environment (don't forget to
remove any confidential data if it exists).

Doorkeeper initializer:

# config/initializers/doorkeeper.rb
Doorkeeper.configure do
  orm :active_record
  optional_scopes :use_case_one, :use_case_two, :use_case_three, :use_case_four
  enforce_configured_scopes
  api_only
  access_token_expires_in 5.minutes
  grant_flows %w[client_credentials]
end

Ruby version:
3.1.2

Gemfile.lock:

Gemfile.lock content
  <massive snip>
    doorkeeper (5.6.0)
      railties (>= 5)
  <massive snip>
@nbulaj
Copy link
Member

nbulaj commented Sep 16, 2022

Thanks @colinbruce for reporting this. Will check a little bit later when have time.
We can also ping @enrico to check the changes and the issue

@enrico
Copy link
Contributor

enrico commented Oct 26, 2022

sorry for responding late, I just saw this.

@colinbruce : I'm not fully not clear on what "We have been using the oauth_application.scopes to validate requests inside the application" means...

I think the old behavior represents a security hole, that is if you have an application specifiying the list of scopes allowed, and you did not request any scope when authenticating, the default behavior of returning the default scope (even if NOT present in the Application list of allowed scopes) is not what we want.

curious: what's stopping you from listing all your optional scopes inside a default_scope clause instead?

default_scopes :use_case_one, :use_case_two, :use_case_three, :use_case_four

this way the caller does not need to request a specific scope, and any token he/she gets should have all 4 allowed scopes...

@colinbruce
Copy link
Author

Sorry for the even longer delay in replying @enrico - my GitHub notifications have been swamped!

Thanks for the steer, I will need to test but I think your solution works.

The issue for us was we didn't want callers to have all four scopes by default. But I think that we misunderstood the default scope meaning.

Running tests now, if I create a new application with:

Doorkeeper::Application.create(name: 'New_application', scopes: ['use_case_one'])

And then access the service and call for data protected by "use_case_four" scope, I get the error I expect.

We assumed that if use_case_four was a default scope all callers would be able to access it 🤔

I will re-read the docs and test further but it is looking like this will work fine 👍

@stale
Copy link

stale bot commented May 21, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants