Skip to content

Commit

Permalink
Merge 08f8e8a into f2142e1
Browse files Browse the repository at this point in the history
  • Loading branch information
danielsbird committed Sep 29, 2021
2 parents f2142e1 + 08f8e8a commit 24b6d29
Show file tree
Hide file tree
Showing 18 changed files with 328 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@ User-visible changes worth mentioning.

## main

- [#1529] Redirect_uri is optional in token request if it was not present in authorization request.
- [#PR ID] Add your PR description here.

## 5.5.3
Expand Down
18 changes: 18 additions & 0 deletions lib/doorkeeper/config.rb
Expand Up @@ -437,6 +437,24 @@ def configure_secrets_for(type, using:, fallback:)
end
end)

# Allows the client to not pass a redirect_uri parameter to the
# authorization endpoint during the authorization code flow and implicit
# flow. If the client does not pass a redirect_uri parameter to the
# authorization endpoint then then Doorkeeper will redirect the resource
# owner to the redirection endpoint specified when the application was
# created, as per Section 3.1.2 of the OAuth 2.0 specification:
# https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.2
#
# By default this option is false, meaning that the client must pass a
# redirect_uri parameter to the authorization endpoint.
#
# This option should only be set to true if your application does not have
# a NOT_NULL constraint applied to the redirect_uri column of its
# oauth_access_grants database table.
#
option :redirect_uri_optional_during_authorization,
default: false

attr_reader :reuse_access_token,
:token_secret_fallback_strategy,
:application_secret_fallback_strategy
Expand Down
13 changes: 11 additions & 2 deletions lib/doorkeeper/oauth/authorization_code_request.rb
Expand Up @@ -57,7 +57,7 @@ def pkce_supported?
def validate_params
@missing_param = if grant&.uses_pkce? && code_verifier.blank?
:code_verifier
elsif redirect_uri.blank?
elsif grant&.redirect_uri.present? && redirect_uri.blank?
:redirect_uri
end

Expand All @@ -75,9 +75,18 @@ def validate_grant
end

def validate_redirect_uri
# 4.1.3. Access Token Request
# redirect_uri
# REQUIRED, if the "redirect_uri" parameter was included in the
# authorization request as described in Section 4.1.1, and their
# values MUST be identical.
#
# @see https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3
return true if redirect_uri.blank? && grant.redirect_uri.blank?

Helpers::URIChecker.valid_for_authorization?(
redirect_uri,
grant.redirect_uri,
grant.redirect_uri.presence || client.redirect_uri,
)
end

Expand Down
8 changes: 5 additions & 3 deletions lib/doorkeeper/oauth/code_response.rb
Expand Up @@ -38,12 +38,14 @@ def body
end

def redirect_uri
if URIChecker.oob_uri?(pre_auth.redirect_uri)
uri = pre_auth.redirect_uri.presence || pre_auth.client.redirect_uri

if URIChecker.oob_uri?(uri)
auth.oob_redirect
elsif response_on_fragment
Authorization::URIBuilder.uri_with_fragment(pre_auth.redirect_uri, body)
Authorization::URIBuilder.uri_with_fragment(uri, body)
else
Authorization::URIBuilder.uri_with_query(pre_auth.redirect_uri, body)
Authorization::URIBuilder.uri_with_query(uri, body)
end
end
end
Expand Down
9 changes: 8 additions & 1 deletion lib/doorkeeper/oauth/pre_authorization.rb
Expand Up @@ -97,7 +97,14 @@ def validate_resource_owner_authorize_for_client
end

def validate_redirect_uri
return false if redirect_uri.blank?
# 4.1.1. Authorization Request
# redirect_uri
# OPTIONAL. As described in Section 3.1.2.
#
# @see https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.1
return true if redirect_uri.blank? &&
client.redirect_uri.present? &&
Doorkeeper.config.redirect_uri_optional_during_authorization

Helpers::URIChecker.valid_for_authorization?(
redirect_uri,
Expand Down
23 changes: 22 additions & 1 deletion lib/doorkeeper/orm/active_record/mixins/access_grant.rb
Expand Up @@ -23,9 +23,18 @@ module AccessGrant
validates :application_id,
:token,
:expires_in,
:redirect_uri,
presence: true

validates :redirect_uri,
presence: true,
on: :create,
unless: :redirect_uri_optional_during_authorization?

validates :redirect_uri,
presence: true,
on: :update,
if: :validate_redirect_uri_on_update?

validates :token, uniqueness: { case_sensitive: true }

before_validation :generate_token, on: :create
Expand Down Expand Up @@ -54,6 +63,18 @@ def generate_token
@raw_token = Doorkeeper::OAuth::Helpers::UniqueToken.generate
secret_strategy.store_secret(self, :token, @raw_token)
end

def redirect_uri_optional_during_authorization?
Doorkeeper.config.redirect_uri_optional_during_authorization
end

def redirect_uri_required_during_authorization?
!redirect_uri_optional_during_authorization?
end

def validate_redirect_uri_on_update?
redirect_uri_required_during_authorization? && redirect_uri_changed?
end
end

module ClassMethods
Expand Down
17 changes: 17 additions & 0 deletions lib/generators/doorkeeper/templates/initializer.rb
Expand Up @@ -485,6 +485,23 @@
# If you need to block the request at all, then configure your routes.rb or web-server
# like nginx to forbid the request.

# Allows the client to not pass a redirect_uri parameter to the
# authorization endpoint during the authorization code flow and implicit
# flow. If the client does not pass a redirect_uri parameter to the
# authorization endpoint then then Doorkeeper will redirect the resource
# owner to the redirection endpoint specified when the application was
# created, as per Section 3.1.2 of the OAuth 2.0 specification:
# https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.2
#
# By default this option is false, meaning that the client must pass a
# redirect_uri parameter to the authorization endpoint.
#
# This option should only be set to true if your application does not have
# a NOT_NULL constraint applied to the redirect_uri column of its
# oauth_access_grants database table.
#
# redirect_uri_optional_during_authorization false

# WWW-Authenticate Realm (default: "Doorkeeper").
#
# realm "Doorkeeper"
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/doorkeeper/templates/migration.rb.erb
Expand Up @@ -23,7 +23,7 @@ class CreateDoorkeeperTables < ActiveRecord::Migration<%= migration_version %>
t.references :application, null: false
t.string :token, null: false
t.integer :expires_in, null: false
t.text :redirect_uri, null: false
t.text :redirect_uri
t.datetime :created_at, null: false
t.datetime :revoked_at
t.string :scopes, null: false, default: ''
Expand Down
17 changes: 17 additions & 0 deletions spec/dummy/config/initializers/doorkeeper.rb
Expand Up @@ -161,6 +161,23 @@
# If you need to block the request at all, then configure your routes.rb or web-server
# like nginx to forbid the request.

# Allows the client to not pass a redirect_uri parameter to the
# authorization endpoint during the authorization code flow and implicit
# flow. If the client does not pass a redirect_uri parameter to the
# authorization endpoint then then Doorkeeper will redirect the resource
# owner to the redirection endpoint specified when the application was
# created, as per Section 3.1.2 of the OAuth 2.0 specification:
# https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.2
#
# By default this option is false, meaning that the client must pass a
# redirect_uri parameter to the authorization endpoint.
#
# This option should only be set to true if your application does not have
# a NOT_NULL constraint applied to the redirect_uri column of its
# oauth_access_grants database table.
#
# redirect_uri_optional_during_authorization false

# WWW-Authenticate Realm (default "Doorkeeper").
realm "Doorkeeper"
end
Expand Up @@ -22,7 +22,7 @@ def change
t.references :application, null: false
t.string :token, null: false
t.integer :expires_in, null: false
t.text :redirect_uri, null: false
t.text :redirect_uri
t.datetime :created_at, null: false
t.datetime :revoked_at
t.string :scopes, null: false, default: ""
Expand Down
2 changes: 1 addition & 1 deletion spec/dummy/db/schema.rb
Expand Up @@ -18,7 +18,7 @@
t.integer "application_id", null: false
t.string "token", null: false
t.integer "expires_in", null: false
t.text "redirect_uri", null: false
t.text "redirect_uri"
t.datetime "created_at", null: false
t.datetime "revoked_at"
t.string "scopes"
Expand Down
15 changes: 15 additions & 0 deletions spec/lib/config_spec.rb
Expand Up @@ -810,6 +810,21 @@ class FakeCustomModel; end
end
end

describe "redirect_uri_optional_during_authorization" do
it "is false by default" do
expect(config.redirect_uri_optional_during_authorization).to be(false)
end

it "can be set to true" do
Doorkeeper.configure do
orm DOORKEEPER_ORM
redirect_uri_optional_during_authorization true
end

expect(config.redirect_uri_optional_during_authorization).to be(true)
end
end

describe "options deprecation" do
it "prints a warning message when an option is deprecated" do
expect(Kernel).to receive(:warn).with(
Expand Down

0 comments on commit 24b6d29

Please sign in to comment.