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

Configuration to enforce application scopes #1010

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ User-visible changes worth mentioning.
- [#1048] Remove deprecated `Doorkeeper#configured?`, `Doorkeeper#database_installed?`, and
`Doorkeeper#installed?` method
- [#1031] Allow public clients to authenticate without `client_secret`. Define an app as either public or private/confidential
- [#1010] Add configuration to enforce known application scopes

## 4.3.1

Expand Down
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ en:
relative_uri: 'must be an absolute URI.'
secured_uri: 'must be an HTTPS/SSL URI.'
forbidden_uri: 'is forbidden by the server.'
scopes:
invalid_scope: 'The requested scope is invalid, unknown, or malformed.'

doorkeeper:
applications:
Expand Down
12 changes: 12 additions & 0 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ def reuse_access_token
def api_only
@config.instance_variable_set(:@api_only, true)
end

# Forbids creating/updating applications with arbitrary scopes that are
# not in configuration, i.e. `default_scopes` or `optional_scopes`.
# (Disabled by default)
def enforce_configured_scopes
@config.instance_variable_set("@enforce_configured_scopes", true)
end
end

module Option
Expand Down Expand Up @@ -280,6 +287,11 @@ def pkce_without_secret_enabled?
@enable_pkce_without_secret ||= false
end

def enforce_configured_scopes?
@enforce_configured_scopes ||= false
!!@enforce_configured_scopes
end

def enable_application_owner?
@enable_application_owner ||= false
!!@enable_application_owner
Expand Down
13 changes: 13 additions & 0 deletions lib/doorkeeper/orm/active_record/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class Application < ActiveRecord::Base
validates :redirect_uri, redirect_uri: true
validates :confidential, inclusion: { in: [true, false] }

validate :validate_configured_scopes, if: :enforce_scopes?

before_validation :generate_uid, :generate_secret, on: :create

has_many :authorized_tokens, -> { where(revoked_at: nil) }, class_name: 'AccessToken'
Expand Down Expand Up @@ -41,5 +43,16 @@ def generate_uid
def generate_secret
self.secret = UniqueToken.generate if secret.blank?
end

def validate_configured_scopes
if !scopes.blank? &&
!ScopeChecker.valid?(scopes.to_s, Doorkeeper.configuration.scopes)
errors.add(:scopes, :invalid_scope)
end
end

def enforce_scopes?
Doorkeeper.configuration.enforce_configured_scopes?
end
end
end
5 changes: 5 additions & 0 deletions lib/generators/doorkeeper/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@
#
# use_refresh_token

# Forbids creating/updating applications with arbitrary scopes that are
# not in configuration, i.e. `default_scopes` or `optional_scopes`.
# (Disabled by default)
# enforce_configured_scopes

# Provide support for an owner to be assigned to each registered application (disabled by default)
# Optional parameter confirmation: true (default false) if you want to enforce ownership of
# a registered application
Expand Down
5 changes: 5 additions & 0 deletions spec/dummy/config/initializers/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
# Issue access tokens with refresh token (disabled by default)
use_refresh_token

# Forbids creating/updating applications with arbitrary scopes that are
# not in configuration, i.e. `default_scopes` or `optional_scopes`.
# (Disabled by default)
# enforce_configured_scopes

# Provide support for an owner to be assigned to each registered application (disabled by default)
# Optional parameter confirmation: true (default false) if you want to enforce ownership of
# a registered application
Expand Down
67 changes: 67 additions & 0 deletions spec/requests/applications/applications_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,73 @@
click_button 'Submit'
i_should_see 'Whoops! Check your form for possible errors'
end

scenario "adding app ignoring bad scope" do
config_is_set("enforce_configured_scopes", false)

fill_in "doorkeeper_application[name]", with: "My Application"
fill_in "doorkeeper_application[redirect_uri]",
with: "https://example.com"
fill_in "doorkeeper_application[scopes]", with: "blahblah"

click_button "Submit"
i_should_see "Application created"
i_should_see "My Application"
end

scenario "adding app validating bad scope" do
config_is_set("enforce_configured_scopes", true)

fill_in "doorkeeper_application[name]", with: "My Application"
fill_in "doorkeeper_application[redirect_uri]",
with: "https://example.com"
fill_in "doorkeeper_application[scopes]", with: "blahblah"

click_button "Submit"
i_should_see "Whoops! Check your form for possible errors"
end

scenario "adding app validating scope, blank scope is accepted" do
config_is_set("enforce_configured_scopes", true)
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a test with multiple scopes to be sure everything is OK


fill_in "doorkeeper_application[name]", with: "My Application"
fill_in "doorkeeper_application[redirect_uri]",
with: "https://example.com"
fill_in "doorkeeper_application[scopes]", with: ""

click_button "Submit"
i_should_see "Application created"
i_should_see "My Application"
end

scenario "adding app validating scope, multiple scopes configured" do
config_is_set("enforce_configured_scopes", true)
scopes = Doorkeeper::OAuth::Scopes.from_array(%w(read write admin))
config_is_set("optional_scopes", scopes)

fill_in "doorkeeper_application[name]", with: "My Application"
fill_in "doorkeeper_application[redirect_uri]",
with: "https://example.com"
fill_in "doorkeeper_application[scopes]", with: "read write"

click_button "Submit"
i_should_see "Application created"
i_should_see "My Application"
end

scenario "adding app validating scope, bad scope with multiple scopes configured" do
config_is_set("enforce_configured_scopes", true)
scopes = Doorkeeper::OAuth::Scopes.from_array(%w(read write admin))
config_is_set("optional_scopes", scopes)

fill_in "doorkeeper_application[name]", with: "My Application"
fill_in "doorkeeper_application[redirect_uri]",
with: "https://example.com"
fill_in "doorkeeper_application[scopes]", with: "read blah"

click_button "Submit"
i_should_see "Whoops! Check your form for possible errors"
end
end
end

Expand Down