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

Properly fall back to default_scopes when no scope is specified. #632

Merged
merged 1 commit into from
Apr 18, 2015
Merged

Properly fall back to default_scopes when no scope is specified. #632

merged 1 commit into from
Apr 18, 2015

Conversation

knu
Copy link
Contributor

@knu knu commented Apr 15, 2015

The parameter scopes is always an array which evaluates as truthy, so the intended fallback has never worked.

This bug caused Doorkeeper to allow clients to access to protected resources under the default scopes regardless of the scopes their token has.

The parameter `scopes` is always an array which evaluates as truthy, so
the intended fallback has never worked.

This bug caused Doorkeeper to allow clients to access to protected
resources under the default scopes regardless of the scopes their token
has.
@@ -4,7 +4,7 @@ module Helpers
extend ActiveSupport::Concern

def doorkeeper_authorize!(*scopes)
@_doorkeeper_scopes = scopes || Doorkeeper.configuration.default_scopes
@_doorkeeper_scopes = scopes.presence || Doorkeeper.configuration.default_scopes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [88/80]

@knu
Copy link
Contributor Author

knu commented Apr 15, 2015

I found a problem in the RSpec configuration where Doorkeeper's configuration is cleared before running each example, making what's in spec/dummy/config/initializers/doorkeeper.rb completely ignored.
This is the reason for the ugly Doorkeeper.configuration.instance_eval in the added spec.

@knu
Copy link
Contributor Author

knu commented Apr 15, 2015

Looks like scope.presence is used in doorkeeper_for.rb in the 1.4-stable branch, so I guess this is new in the 2.x series.

@tute
Copy link
Contributor

tute commented Apr 18, 2015

This is great, thank you very much. This was discussed (and not addressed) in #567 (comment). I'm very happy to see it fixed.

tute added a commit that referenced this pull request Apr 18, 2015
Properly fall back to default_scopes when no scope is specified.
@tute tute merged commit e21c283 into doorkeeper-gem:master Apr 18, 2015
@knu knu deleted the check_with_default_scopes branch February 12, 2016 07:06
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 this pull request may close these issues.

3 participants