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

Introduce polymorphic resource owner #1355

Merged
merged 1 commit into from
Feb 7, 2020
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
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)
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
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
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
{
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
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
extend ActiveSupport::Concern

module ClassMethods
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
# 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