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

Doorkeeper::AccessToken.find_or_create_for with empty scopes raises NoMethodError #1699

Closed
pdany1116 opened this issue Mar 12, 2024 · 6 comments · Fixed by #1714
Closed

Doorkeeper::AccessToken.find_or_create_for with empty scopes raises NoMethodError #1699

pdany1116 opened this issue Mar 12, 2024 · 6 comments · Fixed by #1714

Comments

@pdany1116
Copy link

Steps to reproduce

Doorkeeper::AccessToken.find_or_create_for raises NoMethodError when searching with empty scopes.

application = Doorkeeper::Application.create(name: "test_application") # => Doorkeeper::Application
Doorkeeper::AccessToken.create_for(application: application, resource_owner: 42, scopes: 'public') # => Doorkeeper::AccessToken
Doorkeeper::AccessToken.find_or_create_for(application: application, resource_owner: 42, scopes: '') # => NoMethodError

Expected behavior

A new Doorkeeper::AccessToken should be created with empty scopes.

Actual behavior

NoMethodError: undefined method `sort' for "":String
from /usr/local/bundle/gems/doorkeeper-5.6.6/lib/doorkeeper/models/access_token_mixin.rb:155:in `scopes_match?'

System configuration

Ruby version 3.0.6
Doorkeeper version 5.6.6

@pdany1116
Copy link
Author

Hi, thanks for this awesome and useful gem! I can look into the issue and propose some tests + fix. But before getting to work, I want to make sure this is a valid case and I am not missing anything.

Thank you!

@nbulaj
Copy link
Member

nbulaj commented Mar 12, 2024

Hey @pdany1116 . Try to use Doorkeeper::OAuth::Scopes.from_string("") instead.

Also even plain one works without issues for me:

image

Looks like depends from configuration, but try to use above advise.

UPD: yes, requires reuse_access_token config option to be enabled

@pdany1116
Copy link
Author

pdany1116 commented Mar 12, 2024

Indeed, I have the reuse_access_token config option enabled.

Works with Doorkeeper::OAuth::Scopes.from_string(""), thanks!

@nbulaj
Copy link
Member

nbulaj commented Mar 13, 2024

Can we close this issue? Or do you think it makes sense to add some extra protection into #find_or_create_for method?

def find_or_create_for(application:, resource_owner:, scopes:, **token_attributes)
  scopes = Doorkeeper::OAuth::Scopes.from_string(scopes) if scopes.is_a?(String)
  # ...
end

@pdany1116
Copy link
Author

I think it makes sense to add extra protection, I was not expecting this to be the problem, maybe other devs will encounter the same issue and be confused.

I would like to contribute if possible 😄

@nbulaj
Copy link
Member

nbulaj commented Mar 13, 2024

Yeah sure, will be great @pdany1116 , thanks a lot! Please refer CONTRIBUTING guide when open a PR 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants