Skip to content

Commit

Permalink
Introduce polymorphic resource owner
Browse files Browse the repository at this point in the history
  • Loading branch information
nbulaj committed Feb 6, 2020
1 parent ba3c959 commit 015a062
Show file tree
Hide file tree
Showing 50 changed files with 1,300 additions and 909 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ User-visible changes worth mentioning.

## master

- [#1355] Allow to enable polymorphic Resource Owner association for Access Tokens/Grants
(`polymorphic_resource_owner` configuration option).
- [#1356] Invalidate duplicated scopes for Access Tokens and Grants in database.
- [#1357] Fix `Doorkeeper::OAuth::PreAuthorization#as_json` method causing
`Stack level too deep` error with AMS (fix #1312).
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/doorkeeper/authorizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def render_error
def matching_token?
AccessToken.matching_token_for(
pre_auth.client,
current_resource_owner.id,
current_resource_owner,
pre_auth.scopes,
)
end
Expand Down
1 change: 1 addition & 0 deletions lib/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
require "doorkeeper/models/concerns/revocable"
require "doorkeeper/models/concerns/accessible"
require "doorkeeper/models/concerns/secret_storable"
require "doorkeeper/models/concerns/resource_ownerable"

require "doorkeeper/models/access_grant_mixin"
require "doorkeeper/models/access_token_mixin"
Expand Down
10 changes: 10 additions & 0 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ def api_only
@config.instance_variable_set(:@api_only, true)
end

# Enables polymorphic Resource Owner association for Access Grant and
# Access Token models. Requires additional database columns to be setup.
def use_polymorphic_resource_owner
@config.instance_variable_set(:@polymorphic_resource_owner, true)
end

# Forbids creating/updating applications with arbitrary scopes that are
# not in configuration, i.e. `default_scopes` or `optional_scopes`.
# (disabled by default)
Expand Down Expand Up @@ -476,6 +482,10 @@ def enable_application_owner?
option_set? :enable_application_owner
end

def polymorphic_resource_owner?
option_set? :polymorphic_resource_owner
end

def confirm_application_owner?
option_set? :confirm_application_owner
end
Expand Down
12 changes: 7 additions & 5 deletions lib/doorkeeper/models/access_grant_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module AccessGrantMixin
include Models::Orderable
include Models::SecretStorable
include Models::Scopes
include Models::ResourceOwnerable

# never uses pkce, if pkce migrations were not generated
def uses_pkce?
Expand Down Expand Up @@ -43,11 +44,12 @@ def by_token(token)
# instance of the Resource Owner model
#
def revoke_all_for(application_id, resource_owner, clock = Time)
where(
application_id: application_id,
resource_owner_id: resource_owner.id,
revoked_at: nil,
).update_all(revoked_at: clock.now.utc)
by_resource_owner(resource_owner)
.where(
application_id: application_id,
revoked_at: nil,
)
.update_all(revoked_at: clock.now.utc)
end

# Implements PKCE code_challenge encoding without base64 padding as described in the spec.
Expand Down
104 changes: 69 additions & 35 deletions lib/doorkeeper/models/access_token_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module AccessTokenMixin
include Models::Orderable
include Models::SecretStorable
include Models::Scopes
include Models::ResourceOwnerable

module ClassMethods
# Returns an instance of the Doorkeeper::AccessToken with
Expand Down Expand Up @@ -64,34 +65,29 @@ def by_previous_refresh_token(previous_refresh_token)
# instance of the Resource Owner model
#
def revoke_all_for(application_id, resource_owner, clock = Time)
where(
application_id: application_id,
resource_owner_id: resource_owner.id,
revoked_at: nil,
).update_all(revoked_at: clock.now.utc)
by_resource_owner(resource_owner)
.where(
application_id: application_id,
revoked_at: nil,
)
.update_all(revoked_at: clock.now.utc)
end

# Looking for not revoked Access Token with a matching set of scopes
# that belongs to specific Application and Resource Owner.
#
# @param application [Doorkeeper::Application]
# Application instance
# @param resource_owner_or_id [ActiveRecord::Base, Integer]
# @param resource_owner [ActiveRecord::Base, Integer]
# Resource Owner model instance or it's ID
# @param scopes [String, Doorkeeper::OAuth::Scopes]
# set of scopes
#
# @return [Doorkeeper::AccessToken, nil] Access Token instance or
# nil if matching record was not found
#
def matching_token_for(application, resource_owner_or_id, scopes)
resource_owner_id = if resource_owner_or_id.respond_to?(:to_key)
resource_owner_or_id.id
else
resource_owner_or_id
end

tokens = authorized_tokens_for(application.try(:id), resource_owner_id)
def matching_token_for(application, resource_owner, scopes)
tokens = authorized_tokens_for(application&.id, resource_owner)
find_matching_token(tokens, application, scopes)
end

Expand Down Expand Up @@ -169,7 +165,7 @@ def scopes_match?(token_scopes, param_scopes, app_scopes)
#
# @param application [Doorkeeper::Application]
# Application instance
# @param resource_owner_id [ActiveRecord::Base, Integer]
# @param resource_owner [ActiveRecord::Base, Integer]
# Resource Owner model instance or it's ID
# @param scopes [#to_s]
# set of scopes (any object that responds to `#to_s`)
Expand All @@ -180,36 +176,42 @@ def scopes_match?(token_scopes, param_scopes, app_scopes)
#
# @return [Doorkeeper::AccessToken] existing record or a new one
#
def find_or_create_for(application, resource_owner_id, scopes, expires_in, use_refresh_token)
def find_or_create_for(application, resource_owner, scopes, expires_in, use_refresh_token)
if Doorkeeper.config.reuse_access_token
access_token = matching_token_for(application, resource_owner_id, scopes)
access_token = matching_token_for(application, resource_owner, scopes)

return access_token if access_token&.reusable?
end

create!(
application_id: application.try(:id),
resource_owner_id: resource_owner_id,
attributes = {
application_id: application&.id,
scopes: scopes.to_s,
expires_in: expires_in,
use_refresh_token: use_refresh_token,
)
}

if Doorkeeper.config.polymorphic_resource_owner?
attributes[:resource_owner] = resource_owner
else
attributes[:resource_owner_id] = resource_owner_id_for(resource_owner)
end

create!(attributes)
end

# Looking for not revoked Access Token records that belongs to specific
# Application and Resource Owner.
#
# @param application_id [Integer]
# ID of the Application model instance
# @param resource_owner_id [Integer]
# @param resource_owner [Integer]
# ID of the Resource Owner model instance
#
# @return [Doorkeeper::AccessToken] array of matching AccessToken objects
#
def authorized_tokens_for(application_id, resource_owner_id)
where(
def authorized_tokens_for(application_id, resource_owner)
by_resource_owner(resource_owner).where(
application_id: application_id,
resource_owner_id: resource_owner_id,
revoked_at: nil,
)
end
Expand All @@ -219,15 +221,16 @@ def authorized_tokens_for(application_id, resource_owner_id)
#
# @param application_id [Integer]
# ID of the Application model instance
# @param resource_owner_id [Integer]
# @param resource_owner [ActiveRecord::Base, Integer]
# ID of the Resource Owner model instance
#
# @return [Doorkeeper::AccessToken, nil] matching AccessToken object or
# nil if nothing was found
#
def last_authorized_token_for(application_id, resource_owner_id)
authorized_tokens_for(application_id, resource_owner_id)
.ordered_by(:created_at, :desc).first
def last_authorized_token_for(application_id, resource_owner)
authorized_tokens_for(application_id, resource_owner)
.ordered_by(:created_at, :desc)
.first
end

##
Expand Down Expand Up @@ -268,7 +271,11 @@ def as_json(_options = {})
expires_in: expires_in_seconds,
application: { uid: application.try(:uid) },
created_at: created_at.to_i,
}
}.tap do |json|
if Doorkeeper.configuration.polymorphic_resource_owner?
json[:resource_owner_type] = resource_owner_type
end
end
end

# Indicates whether the token instance have the same credential
Expand All @@ -280,7 +287,22 @@ def as_json(_options = {})
#
def same_credential?(access_token)
application_id == access_token.application_id &&
same_resource_owner?(access_token)
end

# Indicates whether the token instance have the same credential
# as the other Access Token.
#
# @param access_token [Doorkeeper::AccessToken] other token
#
# @return [Boolean] true if credentials are same of false in other cases
#
def same_resource_owner?(access_token)
if Doorkeeper.configuration.polymorphic_resource_owner?
resource_owner == access_token.resource_owner
else
resource_owner_id == access_token.resource_owner_id
end
end

# Indicates if token is acceptable for specific scopes.
Expand Down Expand Up @@ -362,16 +384,28 @@ def generate_refresh_token
def generate_token
self.created_at ||= Time.now.utc

@raw_token = token_generator.generate(
@raw_token = token_generator.generate(attributes_for_token_generator)
secret_strategy.store_secret(self, :token, @raw_token)
@raw_token
end

# Set of attributes that would be passed to token generator to
# generate unique token based on them.
#
# @return [Hash] set of attributes
#
def attributes_for_token_generator
{
resource_owner_id: resource_owner_id,
scopes: scopes,
application: application,
expires_in: expires_in,
created_at: created_at,
)

secret_strategy.store_secret(self, :token, @raw_token)
@raw_token
}.tap do |attributes|
if Doorkeeper.config.polymorphic_resource_owner?
attributes[:resource_owner] = resource_owner
end
end
end

def token_generator
Expand Down
47 changes: 47 additions & 0 deletions lib/doorkeeper/models/concerns/resource_ownerable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

module Doorkeeper
module Models
module ResourceOwnerable
extend ActiveSupport::Concern

module ClassMethods
# Searches for record by Resource Owner considering Doorkeeper
# configuration for resource owner association.
#
# @param resource_owner [ActiveRecord::Base, Integer]
# resource owner
#
# @return [Doorkeeper::AccessGrant, Doorkeeper::AccessToken]
# collection of records
#
def by_resource_owner(resource_owner)
if Doorkeeper.configuration.polymorphic_resource_owner?
where(resource_owner: resource_owner)
else
where(resource_owner_id: resource_owner_id_for(resource_owner))
end
end

protected

# Backward compatible way to retrieve resource owner itself (if
# polymorphic association enabled) or just it's ID.
#
# @param resource_owner [ActiveRecord::Base, Integer]
# resource owner
#
# @return [ActiveRecord::Base, Integer]
# instance of Resource Owner or it's ID
#
def resource_owner_id_for(resource_owner)
if resource_owner.respond_to?(:to_key)
resource_owner.id
else
resource_owner
end
end
end
end
end
end
13 changes: 10 additions & 3 deletions lib/doorkeeper/oauth/authorization/code.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,20 @@ def authorization_code_expires_in
end

def access_grant_attributes
pkce_attributes.merge(
attributes = {
application_id: pre_auth.client.id,
resource_owner_id: resource_owner.id,
expires_in: authorization_code_expires_in,
redirect_uri: pre_auth.redirect_uri,
scopes: pre_auth.scopes.to_s,
)
}

if Doorkeeper.config.polymorphic_resource_owner?
attributes[:resource_owner] = resource_owner
else
attributes[:resource_owner_id] = resource_owner.id
end

pkce_attributes.merge(attributes)
end

def pkce_attributes
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/oauth/authorization/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def issue_token

@token = configuration.access_token_model.find_or_create_for(
pre_auth.client,
resource_owner.id,
resource_owner,
pre_auth.scopes,
self.class.access_token_expires_in(configuration, context),
false,
Expand Down
9 changes: 8 additions & 1 deletion lib/doorkeeper/oauth/authorization_code_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,20 @@ def before_successful_response

grant.revoke

resource_owner = if Doorkeeper.config.polymorphic_resource_owner?
grant.resource_owner
else
grant.resource_owner_id
end

find_or_create_access_token(
grant.application,
grant.resource_owner_id,
resource_owner,
grant.scopes,
server,
)
end

super
end

Expand Down
4 changes: 2 additions & 2 deletions lib/doorkeeper/oauth/base_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def valid?
error.nil?
end

def find_or_create_access_token(client, resource_owner_id, scopes, server)
def find_or_create_access_token(client, resource_owner, scopes, server)
context = Authorization::Token.build_context(client, grant_type, scopes)
@access_token = server_config.access_token_model.find_or_create_for(
client,
resource_owner_id,
resource_owner,
scopes,
Authorization::Token.access_token_expires_in(server, context),
Authorization::Token.refresh_token_enabled?(server, context),
Expand Down
Loading

1 comment on commit 015a062

@amex-doug
Copy link

Choose a reason for hiding this comment

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

@nbulaj Do you have any examples for how one might implement resource_owner_authenticator for a polymorphic resource owner?

Without this option enabled, it looks something like this:

  resource_owner_authenticator do
    current_user || warden.authenticate!(scope: :user)
  end

However, with the polymorphic, you would need a way to determine the resource type. It could be by client id, it could be by an additional query parameter. Any suggestions?

Please sign in to comment.