From 015a06256b22c331299d533ef66d6525c1a106f1 Mon Sep 17 00:00:00 2001 From: Nikita Bulai Date: Fri, 31 Jan 2020 14:09:32 +0300 Subject: [PATCH] Introduce polymorphic resource owner --- CHANGELOG.md | 2 + .../doorkeeper/authorizations_controller.rb | 2 +- lib/doorkeeper.rb | 1 + lib/doorkeeper/config.rb | 10 + lib/doorkeeper/models/access_grant_mixin.rb | 12 +- lib/doorkeeper/models/access_token_mixin.rb | 104 +- .../models/concerns/resource_ownerable.rb | 47 + lib/doorkeeper/oauth/authorization/code.rb | 13 +- lib/doorkeeper/oauth/authorization/token.rb | 2 +- .../oauth/authorization_code_request.rb | 9 +- lib/doorkeeper/oauth/base_request.rb | 4 +- .../oauth/password_access_token_request.rb | 2 +- lib/doorkeeper/oauth/refresh_token_request.rb | 13 +- .../orm/active_record/mixins/access_grant.rb | 9 +- .../orm/active_record/mixins/access_token.rb | 6 +- .../confidential_applications_generator.rb | 2 +- .../polymorphic_resource_owner_generator.rb | 34 + .../add_owner_to_application_migration.rb.erb | 2 + ...ious_refresh_token_to_access_tokens.rb.erb | 2 + .../templates/enable_pkce_migration.rb.erb | 2 + ...olymorphic_resource_owner_migration.rb.erb | 11 + .../doorkeeper/templates/initializer.rb | 17 + .../doorkeeper/templates/migration.rb.erb | 2 + .../authorizations_controller_spec.rb | 9 +- ...20151223192035_create_doorkeeper_tables.rb | 4 +- spec/dummy/db/schema.rb | 2 + spec/factories.rb | 2 +- ...lymorphic_resource_owner_generator_spec.rb | 28 + spec/lib/config_spec.rb | 22 +- .../oauth/authorization_code_request_spec.rb | 21 +- spec/lib/oauth/base_request_spec.rb | 10 +- .../oauth/client_credentials/creator_spec.rb | 12 +- spec/lib/oauth/code_request_spec.rb | 2 +- spec/lib/oauth/code_response_spec.rb | 6 +- .../password_access_token_request_spec.rb | 23 +- spec/lib/oauth/token_request_spec.rb | 21 +- spec/lib/stale_records_cleaner_spec.rb | 23 +- spec/models/doorkeeper/access_grant_spec.rb | 22 +- spec/models/doorkeeper/access_token_spec.rb | 986 +++++++++--------- spec/models/doorkeeper/application_spec.rb | 638 ++++++------ spec/requests/endpoints/token_spec.rb | 6 +- .../flows/authorization_code_errors_spec.rb | 5 +- .../requests/flows/authorization_code_spec.rb | 7 +- spec/requests/flows/refresh_token_spec.rb | 24 +- spec/requests/flows/revoke_token_spec.rb | 3 + .../helpers/access_token_request_helper.rb | 1 + .../helpers/authorization_request_helper.rb | 8 +- spec/support/helpers/config_helper.rb | 2 +- .../shared/controllers_shared_context.rb | 4 +- spec/support/shared/models_shared_examples.rb | 10 +- 50 files changed, 1300 insertions(+), 909 deletions(-) create mode 100644 lib/doorkeeper/models/concerns/resource_ownerable.rb create mode 100644 lib/generators/doorkeeper/polymorphic_resource_owner_generator.rb create mode 100644 lib/generators/doorkeeper/templates/enable_polymorphic_resource_owner_migration.rb.erb create mode 100644 spec/generators/polymorphic_resource_owner_generator_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index a9caba0bf..a7d56ab12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/app/controllers/doorkeeper/authorizations_controller.rb b/app/controllers/doorkeeper/authorizations_controller.rb index 7bb5518dd..bbf8bc01f 100644 --- a/app/controllers/doorkeeper/authorizations_controller.rb +++ b/app/controllers/doorkeeper/authorizations_controller.rb @@ -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 diff --git a/lib/doorkeeper.rb b/lib/doorkeeper.rb index ff3028a87..2ef3927d2 100644 --- a/lib/doorkeeper.rb +++ b/lib/doorkeeper.rb @@ -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" diff --git a/lib/doorkeeper/config.rb b/lib/doorkeeper/config.rb index cc7d04593..94f76a601 100644 --- a/lib/doorkeeper/config.rb +++ b/lib/doorkeeper/config.rb @@ -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) @@ -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 diff --git a/lib/doorkeeper/models/access_grant_mixin.rb b/lib/doorkeeper/models/access_grant_mixin.rb index 6fba84104..00bba556b 100644 --- a/lib/doorkeeper/models/access_grant_mixin.rb +++ b/lib/doorkeeper/models/access_grant_mixin.rb @@ -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? @@ -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. diff --git a/lib/doorkeeper/models/access_token_mixin.rb b/lib/doorkeeper/models/access_token_mixin.rb index 565dd21a3..96a510553 100644 --- a/lib/doorkeeper/models/access_token_mixin.rb +++ b/lib/doorkeeper/models/access_token_mixin.rb @@ -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 @@ -64,11 +65,12 @@ 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 @@ -76,7 +78,7 @@ def revoke_all_for(application_id, resource_owner, clock = Time) # # @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 @@ -84,14 +86,8 @@ def revoke_all_for(application_id, resource_owner, clock = Time) # @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 @@ -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`) @@ -180,20 +176,27 @@ 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 @@ -201,15 +204,14 @@ def find_or_create_for(application, resource_owner_id, scopes, expires_in, use_r # # @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 @@ -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 ## @@ -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 @@ -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. @@ -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 diff --git a/lib/doorkeeper/models/concerns/resource_ownerable.rb b/lib/doorkeeper/models/concerns/resource_ownerable.rb new file mode 100644 index 000000000..6765334d1 --- /dev/null +++ b/lib/doorkeeper/models/concerns/resource_ownerable.rb @@ -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 diff --git a/lib/doorkeeper/oauth/authorization/code.rb b/lib/doorkeeper/oauth/authorization/code.rb index 38a46b09d..4923bd9c9 100644 --- a/lib/doorkeeper/oauth/authorization/code.rb +++ b/lib/doorkeeper/oauth/authorization/code.rb @@ -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 diff --git a/lib/doorkeeper/oauth/authorization/token.rb b/lib/doorkeeper/oauth/authorization/token.rb index 6b8af3ae3..99e827dff 100644 --- a/lib/doorkeeper/oauth/authorization/token.rb +++ b/lib/doorkeeper/oauth/authorization/token.rb @@ -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, diff --git a/lib/doorkeeper/oauth/authorization_code_request.rb b/lib/doorkeeper/oauth/authorization_code_request.rb index c1af3af73..19028ce24 100644 --- a/lib/doorkeeper/oauth/authorization_code_request.rb +++ b/lib/doorkeeper/oauth/authorization_code_request.rb @@ -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 diff --git a/lib/doorkeeper/oauth/base_request.rb b/lib/doorkeeper/oauth/base_request.rb index e0bd8be0b..8c00bfc04 100644 --- a/lib/doorkeeper/oauth/base_request.rb +++ b/lib/doorkeeper/oauth/base_request.rb @@ -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), diff --git a/lib/doorkeeper/oauth/password_access_token_request.rb b/lib/doorkeeper/oauth/password_access_token_request.rb index fcf4a7731..926b13352 100644 --- a/lib/doorkeeper/oauth/password_access_token_request.rb +++ b/lib/doorkeeper/oauth/password_access_token_request.rb @@ -25,7 +25,7 @@ def initialize(server, client, resource_owner, parameters = {}) private def before_successful_response - find_or_create_access_token(client, resource_owner.id, scopes, server) + find_or_create_access_token(client, resource_owner, scopes, server) super end diff --git a/lib/doorkeeper/oauth/refresh_token_request.rb b/lib/doorkeeper/oauth/refresh_token_request.rb index ac5d61f8c..e15d2773a 100644 --- a/lib/doorkeeper/oauth/refresh_token_request.rb +++ b/lib/doorkeeper/oauth/refresh_token_request.rb @@ -54,13 +54,20 @@ def create_access_token end def access_token_attributes - { + attrs = { application_id: refresh_token.application_id, - resource_owner_id: refresh_token.resource_owner_id, scopes: scopes.to_s, expires_in: access_token_expires_in, use_refresh_token: true, - }.tap do |attributes| + } + + if Doorkeeper.config.polymorphic_resource_owner? + attrs[:resource_owner] = refresh_token.resource_owner + else + attrs[:resource_owner_id] = refresh_token.resource_owner_id + end + + attrs.tap do |attributes| if refresh_token_revoked_on_use? attributes[:previous_refresh_token] = refresh_token.refresh_token end diff --git a/lib/doorkeeper/orm/active_record/mixins/access_grant.rb b/lib/doorkeeper/orm/active_record/mixins/access_grant.rb index e68eca4d1..98afe79bf 100644 --- a/lib/doorkeeper/orm/active_record/mixins/access_grant.rb +++ b/lib/doorkeeper/orm/active_record/mixins/access_grant.rb @@ -13,8 +13,13 @@ module AccessGrant optional: true, inverse_of: :access_grants - validates :resource_owner_id, - :application_id, + if Doorkeeper.config.polymorphic_resource_owner? + belongs_to :resource_owner, polymorphic: true, optional: false + else + validates :resource_owner_id, presence: true + end + + validates :application_id, :token, :expires_in, :redirect_uri, diff --git a/lib/doorkeeper/orm/active_record/mixins/access_token.rb b/lib/doorkeeper/orm/active_record/mixins/access_token.rb index 6edd60f6b..9b81c60a4 100644 --- a/lib/doorkeeper/orm/active_record/mixins/access_token.rb +++ b/lib/doorkeeper/orm/active_record/mixins/access_token.rb @@ -13,6 +13,10 @@ module AccessToken inverse_of: :access_tokens, optional: true + if Doorkeeper.config.polymorphic_resource_owner? + belongs_to :resource_owner, polymorphic: true, optional: true + end + validates :token, presence: true, uniqueness: { case_sensitive: true } validates :refresh_token, uniqueness: { case_sensitive: true }, if: :use_refresh_token? @@ -36,7 +40,7 @@ module AccessToken # active Access Tokens for Resource Owner # def active_for(resource_owner) - where(resource_owner_id: resource_owner.id, revoked_at: nil) + by_resource_owner(resource_owner).where(revoked_at: nil) end def refresh_token_revoked_on_use? diff --git a/lib/generators/doorkeeper/confidential_applications_generator.rb b/lib/generators/doorkeeper/confidential_applications_generator.rb index 5497314e7..10cd0ac39 100644 --- a/lib/generators/doorkeeper/confidential_applications_generator.rb +++ b/lib/generators/doorkeeper/confidential_applications_generator.rb @@ -12,7 +12,7 @@ class ConfidentialApplicationsGenerator < ::Rails::Generators::Base source_root File.expand_path("templates", __dir__) desc "Add confidential column to Doorkeeper applications" - def pkce + def confidential_applications migration_template( "add_confidential_to_applications.rb.erb", "db/migrate/add_confidential_to_applications.rb", diff --git a/lib/generators/doorkeeper/polymorphic_resource_owner_generator.rb b/lib/generators/doorkeeper/polymorphic_resource_owner_generator.rb new file mode 100644 index 000000000..bcd4b3aa1 --- /dev/null +++ b/lib/generators/doorkeeper/polymorphic_resource_owner_generator.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "rails/generators" +require "rails/generators/active_record" + +module Doorkeeper + # Generates migration with polymorphic resource owner required + # database columns for Doorkeeper Access Token and Access Grant + # models. + # + class PolymorphicResourceOwnerGenerator < ::Rails::Generators::Base + include ::Rails::Generators::Migration + source_root File.expand_path("templates", __dir__) + desc "Provide support for polymorphic Resource Owner." + + def polymorphic_resource_owner + migration_template( + "enable_polymorphic_resource_owner_migration.rb.erb", + "db/migrate/enable_polymorphic_resource_owner.rb", + migration_version: migration_version, + ) + end + + def self.next_migration_number(dirname) + ActiveRecord::Generators::Base.next_migration_number(dirname) + end + + private + + def migration_version + "[#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}]" + end + end +end diff --git a/lib/generators/doorkeeper/templates/add_owner_to_application_migration.rb.erb b/lib/generators/doorkeeper/templates/add_owner_to_application_migration.rb.erb index 542a17b02..487b7ee81 100644 --- a/lib/generators/doorkeeper/templates/add_owner_to_application_migration.rb.erb +++ b/lib/generators/doorkeeper/templates/add_owner_to_application_migration.rb.erb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddOwnerToApplication < ActiveRecord::Migration<%= migration_version %> def change add_column :oauth_applications, :owner_id, :integer, null: true diff --git a/lib/generators/doorkeeper/templates/add_previous_refresh_token_to_access_tokens.rb.erb b/lib/generators/doorkeeper/templates/add_previous_refresh_token_to_access_tokens.rb.erb index 9dc447512..e29d2e254 100644 --- a/lib/generators/doorkeeper/templates/add_previous_refresh_token_to_access_tokens.rb.erb +++ b/lib/generators/doorkeeper/templates/add_previous_refresh_token_to_access_tokens.rb.erb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddPreviousRefreshTokenToAccessTokens < ActiveRecord::Migration<%= migration_version %> def change add_column( diff --git a/lib/generators/doorkeeper/templates/enable_pkce_migration.rb.erb b/lib/generators/doorkeeper/templates/enable_pkce_migration.rb.erb index 0b9d627ca..a2153ab34 100644 --- a/lib/generators/doorkeeper/templates/enable_pkce_migration.rb.erb +++ b/lib/generators/doorkeeper/templates/enable_pkce_migration.rb.erb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class EnablePkce < ActiveRecord::Migration<%= migration_version %> def change add_column :oauth_access_grants, :code_challenge, :string, null: true diff --git a/lib/generators/doorkeeper/templates/enable_polymorphic_resource_owner_migration.rb.erb b/lib/generators/doorkeeper/templates/enable_polymorphic_resource_owner_migration.rb.erb new file mode 100644 index 000000000..b3bf92056 --- /dev/null +++ b/lib/generators/doorkeeper/templates/enable_polymorphic_resource_owner_migration.rb.erb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class EnablePolymorphicResourceOwner < ActiveRecord::Migration<%= migration_version %> + def change + add_column :oauth_access_tokens, :resource_owner_type, :string + add_column :oauth_access_grants, :resource_owner_type, :string, null: false + + add_index :oauth_access_tokens, [:resource_owner_id, :resource_owner_type] + add_index :oauth_access_grants, [:resource_owner_id, :resource_owner_type] + end +end diff --git a/lib/generators/doorkeeper/templates/initializer.rb b/lib/generators/doorkeeper/templates/initializer.rb index 91ad438e4..1053636e9 100644 --- a/lib/generators/doorkeeper/templates/initializer.rb +++ b/lib/generators/doorkeeper/templates/initializer.rb @@ -58,6 +58,23 @@ # end # end + # Enables polymorphic Resource Owner association for Access Tokens and Access Grants. + # By default this option is disabled. + # + # Make sure you properly setup you database and have all the required columns (run + # `bundle exec rails generate doorkeeper:enable_polymorphic_resource_owner` and execute Rails + # migrations). + # + # If this option enabled, Doorkeeper will store not only Resource Owner primary key + # value, but also it's type (class name). See "Polymorphic Associations" section of + # Rails guides: https://guides.rubyonrails.org/association_basics.html#polymorphic-associations + # + # [NOTE] If you apply this option on already existing project don't forget to manually + # update `resource_owner_type` column in the database and fix migration template as it will + # set NOT NULL constraint for Access Grants table. + # + # use_polymorphic_resource_owner + # If you are planning to use Doorkeeper in Rails 5 API-only application, then you might # want to use API mode that will skip all the views management and change the way how # Doorkeeper responds to a requests. diff --git a/lib/generators/doorkeeper/templates/migration.rb.erb b/lib/generators/doorkeeper/templates/migration.rb.erb index cdb49e3be..c6b51ce7d 100644 --- a/lib/generators/doorkeeper/templates/migration.rb.erb +++ b/lib/generators/doorkeeper/templates/migration.rb.erb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class CreateDoorkeeperTables < ActiveRecord::Migration<%= migration_version %> def change create_table :oauth_applications do |t| diff --git a/spec/controllers/authorizations_controller_spec.rb b/spec/controllers/authorizations_controller_spec.rb index d3939c232..0f6eb4a2c 100644 --- a/spec/controllers/authorizations_controller_spec.rb +++ b/spec/controllers/authorizations_controller_spec.rb @@ -16,7 +16,14 @@ def query_params let(:client) { FactoryBot.create :application } let(:user) { User.create!(name: "Joe", password: "sekret") } - let(:access_token) { FactoryBot.build :access_token, resource_owner_id: user.id, application_id: client.id, scopes: "default" } + + let(:access_token) do + FactoryBot.build :access_token, + resource_owner_id: user.id, + resource_owner_type: user.class.name, + application_id: client.id, + scopes: "default" + end before do Doorkeeper.configure do diff --git a/spec/dummy/db/migrate/20151223192035_create_doorkeeper_tables.rb b/spec/dummy/db/migrate/20151223192035_create_doorkeeper_tables.rb index c04c12f47..3220b983e 100644 --- a/spec/dummy/db/migrate/20151223192035_create_doorkeeper_tables.rb +++ b/spec/dummy/db/migrate/20151223192035_create_doorkeeper_tables.rb @@ -18,7 +18,7 @@ def change add_index :oauth_applications, :uid, unique: true create_table :oauth_access_grants do |t| - t.references :resource_owner, null: false + t.references :resource_owner, null: false, polymorphic: true t.references :application, null: false t.string :token, null: false t.integer :expires_in, null: false @@ -36,7 +36,7 @@ def change ) create_table :oauth_access_tokens do |t| - t.references :resource_owner, index: true + t.references :resource_owner, index: true, polymorphic: true t.references :application, null: false # If you use a custom token generator you may need to change this column diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 8310180f6..d025ff316 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -14,6 +14,7 @@ create_table "oauth_access_grants", force: :cascade do |t| t.integer "resource_owner_id", null: false + t.string "resource_owner_type" # [NOTE] null: false skipped to allow test pass t.integer "application_id", null: false t.string "token", null: false t.integer "expires_in", null: false @@ -30,6 +31,7 @@ create_table "oauth_access_tokens", force: :cascade do |t| t.integer "resource_owner_id" + t.string "resource_owner_type" t.integer "application_id" t.string "token", null: false t.string "refresh_token" diff --git a/spec/factories.rb b/spec/factories.rb index 2ab7b4b3e..53830cd03 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -26,5 +26,5 @@ # do not name this factory :user, otherwise it will conflict with factories # from applications that use doorkeeper factories in their own tests - factory :doorkeeper_testing_user, class: :user + factory :doorkeeper_testing_user, class: :user, aliases: [:resource_owner] end diff --git a/spec/generators/polymorphic_resource_owner_generator_spec.rb b/spec/generators/polymorphic_resource_owner_generator_spec.rb new file mode 100644 index 000000000..a9924bd68 --- /dev/null +++ b/spec/generators/polymorphic_resource_owner_generator_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require "spec_helper" +require "generators/doorkeeper/polymorphic_resource_owner_generator" + +describe "Doorkeeper::PolymorphicResourceOwnerGenerator" do + include GeneratorSpec::TestCase + + tests Doorkeeper::PolymorphicResourceOwnerGenerator + destination ::File.expand_path("../tmp/dummy", __FILE__) + + describe "after running the generator" do + before :each do + prepare_destination + end + + it "creates a migration with a version specifier" do + stub_const("ActiveRecord::VERSION::MAJOR", 5) + stub_const("ActiveRecord::VERSION::MINOR", 0) + + run_generator + + assert_migration "db/migrate/enable_polymorphic_resource_owner.rb" do |migration| + assert migration.include?("ActiveRecord::Migration[5.0]\n") + end + end + end +end diff --git a/spec/lib/config_spec.rb b/spec/lib/config_spec.rb index 435fea7ef..d180b8df8 100644 --- a/spec/lib/config_spec.rb +++ b/spec/lib/config_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Doorkeeper, "configuration" do - subject { Doorkeeper.configuration } + subject { Doorkeeper.config } describe "resource_owner_authenticator" do it "sets the block that is accessible via authenticate_resource_owner" do @@ -328,7 +328,7 @@ describe "enable_application_owner" do it "is disabled by default" do - expect(Doorkeeper.configuration.enable_application_owner?).not_to eq(true) + expect(Doorkeeper.config.enable_application_owner?).not_to eq(true) end context "when enabled without confirmation" do @@ -344,7 +344,7 @@ end it "Doorkeeper.configuration.confirm_application_owner? returns false" do - expect(Doorkeeper.configuration.confirm_application_owner?).not_to eq(true) + expect(Doorkeeper.config.confirm_application_owner?).not_to eq(true) end end @@ -361,14 +361,14 @@ end it "Doorkeeper.configuration.confirm_application_owner? returns true" do - expect(Doorkeeper.configuration.confirm_application_owner?).to eq(true) + expect(Doorkeeper.config.confirm_application_owner?).to eq(true) end end end describe "realm" do it "is 'Doorkeeper' by default" do - expect(Doorkeeper.configuration.realm).to eq("Doorkeeper") + expect(Doorkeeper.config.realm).to eq("Doorkeeper") end it "can change the value" do @@ -383,7 +383,7 @@ describe "grant_flows" do it "is set to all grant flows by default" do - expect(Doorkeeper.configuration.grant_flows) + expect(Doorkeeper.config.grant_flows) .to eq(%w[authorization_code client_credentials]) end @@ -454,13 +454,13 @@ end it "raises an exception when configuration is not set" do - old_config = Doorkeeper.configuration + old_config = Doorkeeper.config Doorkeeper.module_eval do @config = nil end expect do - Doorkeeper.configuration + Doorkeeper.config end.to raise_error Doorkeeper::MissingConfiguration Doorkeeper.module_eval do @@ -527,13 +527,13 @@ end end - it { expect(Doorkeeper.configuration.base_controller).to eq("ApplicationController") } + it { expect(Doorkeeper.config.base_controller).to eq("ApplicationController") } end end describe "base_metal_controller" do context "default" do - it { expect(Doorkeeper.configuration.base_metal_controller).to eq("ActionController::API") } + it { expect(Doorkeeper.config.base_metal_controller).to eq("ActionController::API") } end context "custom" do @@ -662,7 +662,7 @@ class FakeCustomModel; end describe "handle_auth_errors" do it "is set to render by default" do - expect(Doorkeeper.configuration.handle_auth_errors).to eq(:render) + expect(Doorkeeper.config.handle_auth_errors).to eq(:render) end it "can change the value" do Doorkeeper.configure do diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index d2d979a30..79677bd07 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/spec/lib/oauth/authorization_code_request_spec.rb @@ -12,7 +12,12 @@ } end - let(:grant) { FactoryBot.create :access_grant } + let(:resource_owner) { FactoryBot.create :resource_owner } + let(:grant) do + FactoryBot.create :access_grant, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name + end let(:client) { grant.application } let(:redirect_uri) { client.redirect_uri } let(:params) { { redirect_uri: redirect_uri } } @@ -100,8 +105,11 @@ end FactoryBot.create( - :access_token, application_id: client.id, - resource_owner_id: grant.resource_owner_id, scopes: grant.scopes.to_s, + :access_token, + application_id: client.id, + resource_owner_id: grant.resource_owner_id, + resource_owner_type: grant.resource_owner_type, + scopes: grant.scopes.to_s, ) expect { subject.authorize }.to_not(change { Doorkeeper::AccessToken.count }) @@ -117,8 +125,11 @@ end FactoryBot.create( - :access_token, application_id: client.id, - resource_owner_id: grant.resource_owner_id, scopes: grant.scopes.to_s, + :access_token, + application_id: client.id, + resource_owner_id: grant.resource_owner_id, + resource_owner_type: grant.resource_owner_type, + scopes: grant.scopes.to_s, ) allow_any_instance_of(Doorkeeper::AccessToken).to receive(:reusable?).and_return(false) diff --git a/spec/lib/oauth/base_request_spec.rb b/spec/lib/oauth/base_request_spec.rb index 07092ddd7..949bbf482 100644 --- a/spec/lib/oauth/base_request_spec.rb +++ b/spec/lib/oauth/base_request_spec.rb @@ -117,10 +117,12 @@ end describe "#find_or_create_access_token" do + let(:resource_owner) { FactoryBot.build_stubbed(:resource_owner) } + it "returns an instance of AccessToken" do result = subject.find_or_create_access_token( client, - "1", + resource_owner, "public", server, ) @@ -140,7 +142,7 @@ result = subject.find_or_create_access_token( client, - "1", + resource_owner, "public", server, ) @@ -161,7 +163,7 @@ result = subject.find_or_create_access_token( client, - "1", + resource_owner, "public", server, ) @@ -169,7 +171,7 @@ result = subject.find_or_create_access_token( client, - "1", + resource_owner, "private", server, ) diff --git a/spec/lib/oauth/client_credentials/creator_spec.rb b/spec/lib/oauth/client_credentials/creator_spec.rb index 7af80bd82..9a3de5002 100644 --- a/spec/lib/oauth/client_credentials/creator_spec.rb +++ b/spec/lib/oauth/client_credentials/creator_spec.rb @@ -20,7 +20,7 @@ class Doorkeeper::OAuth::ClientCredentialsRequest context "when reuse_access_token is true" do context "when expiration is disabled" do it "returns the existing valid token" do - allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) + allow(Doorkeeper.config).to receive(:reuse_access_token).and_return(true) existing_token = subject.call(client, scopes) result = subject.call(client, scopes) @@ -34,8 +34,8 @@ class Doorkeeper::OAuth::ClientCredentialsRequest let!(:existing_token) { subject.call(client, scopes, expires_in: 1000) } before do - allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) - allow(Doorkeeper.configuration).to receive(:token_reuse_limit).and_return(50) + allow(Doorkeeper.config).to receive(:reuse_access_token).and_return(true) + allow(Doorkeeper.config).to receive(:token_reuse_limit).and_return(50) allow_any_instance_of(Doorkeeper::AccessToken).to receive(:expires_in_seconds).and_return(600) end @@ -48,7 +48,7 @@ class Doorkeeper::OAuth::ClientCredentialsRequest context "and when revoke_previous_client_credentials_token is true" do before do - allow(Doorkeeper.configuration).to receive(:revoke_previous_client_credentials_token).and_return(false) + allow(Doorkeeper.config).to receive(:revoke_previous_client_credentials_token).and_return(false) end it "does not revoke the existing valid token" do @@ -60,8 +60,8 @@ class Doorkeeper::OAuth::ClientCredentialsRequest context "when existing token has crossed token_reuse_limit" do it "returns a new token" do - allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) - allow(Doorkeeper.configuration).to receive(:token_reuse_limit).and_return(50) + allow(Doorkeeper.config).to receive(:reuse_access_token).and_return(true) + allow(Doorkeeper.config).to receive(:token_reuse_limit).and_return(50) existing_token = subject.call(client, scopes, expires_in: 1000) allow_any_instance_of(Doorkeeper::AccessToken).to receive(:expires_in_seconds).and_return(400) diff --git a/spec/lib/oauth/code_request_spec.rb b/spec/lib/oauth/code_request_spec.rb index 6ea638e2b..62bc012b6 100644 --- a/spec/lib/oauth/code_request_spec.rb +++ b/spec/lib/oauth/code_request_spec.rb @@ -24,7 +24,7 @@ pre_auth end - let(:owner) { double :owner, id: 8900 } + let(:owner) { FactoryBot.create(:resource_owner) } subject do described_class.new(pre_auth, owner) diff --git a/spec/lib/oauth/code_response_spec.rb b/spec/lib/oauth/code_response_spec.rb index ab1fc6f07..93725b4b8 100644 --- a/spec/lib/oauth/code_response_spec.rb +++ b/spec/lib/oauth/code_response_spec.rb @@ -15,8 +15,12 @@ ) end + let :owner do + FactoryBot.create(:resource_owner) + end + let :auth do - Doorkeeper::OAuth::Authorization::Token.new(pre_auth, double(id: 1)).tap do |c| + Doorkeeper::OAuth::Authorization::Token.new(pre_auth, owner).tap do |c| c.issue_token allow(c.token).to receive(:expires_in_seconds).and_return(3600) end diff --git a/spec/lib/oauth/password_access_token_request_spec.rb b/spec/lib/oauth/password_access_token_request_spec.rb index dce06b6e9..d16da7a10 100644 --- a/spec/lib/oauth/password_access_token_request_spec.rb +++ b/spec/lib/oauth/password_access_token_request_spec.rb @@ -15,7 +15,7 @@ ) end let(:client) { FactoryBot.create(:application) } - let(:owner) { double :owner, id: 99 } + let(:owner) { FactoryBot.create(:resource_owner) } before do allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true) @@ -62,7 +62,12 @@ end it "creates token even when there is already one (default)" do - FactoryBot.create(:access_token, application_id: client.id, resource_owner_id: owner.id) + FactoryBot.create( + :access_token, + application_id: client.id, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name, + ) expect do subject.authorize @@ -71,7 +76,12 @@ it "skips token creation if there is already one reusable" do allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) - FactoryBot.create(:access_token, application_id: client.id, resource_owner_id: owner.id) + FactoryBot.create( + :access_token, + application_id: client.id, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name, + ) expect do subject.authorize @@ -80,7 +90,12 @@ it "creates token when there is already one but non reusable" do allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) - FactoryBot.create(:access_token, application_id: client.id, resource_owner_id: owner.id) + FactoryBot.create( + :access_token, + application_id: client.id, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name, + ) allow_any_instance_of(Doorkeeper::AccessToken).to receive(:reusable?).and_return(false) expect do diff --git a/spec/lib/oauth/token_request_spec.rb b/spec/lib/oauth/token_request_spec.rb index 301ea26d1..433f88862 100644 --- a/spec/lib/oauth/token_request_spec.rb +++ b/spec/lib/oauth/token_request_spec.rb @@ -8,7 +8,7 @@ end let :pre_auth do - server = Doorkeeper.configuration + server = Doorkeeper.config allow(server).to receive(:default_scopes).and_return(Doorkeeper::OAuth::Scopes.from_string("public")) allow(server).to receive(:grant_flows).and_return(Doorkeeper::OAuth::Scopes.from_string("implicit")) @@ -26,7 +26,7 @@ end let :owner do - double :owner, id: 7866 + FactoryBot.create(:doorkeeper_testing_user) end subject do @@ -116,9 +116,13 @@ it "creates a new token if scopes do not match" do allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true) FactoryBot.create( - :access_token, application_id: pre_auth.client.id, - resource_owner_id: owner.id, scopes: "", + :access_token, + application_id: pre_auth.client.id, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name, + scopes: "", ) + expect do subject.authorize end.to change { Doorkeeper::AccessToken.count }.by(1) @@ -131,7 +135,7 @@ FactoryBot.create( :access_token, application_id: pre_auth.client.id, - resource_owner_id: owner.id, scopes: "public", + resource_owner_id: owner.id, resource_owner_type: owner.class.name, scopes: "public", ) expect { subject.authorize }.not_to(change { Doorkeeper::AccessToken.count }) @@ -143,8 +147,11 @@ allow(application.scopes).to receive(:all?).and_return(true) FactoryBot.create( - :access_token, application_id: pre_auth.client.id, - resource_owner_id: owner.id, scopes: "public", + :access_token, + application_id: pre_auth.client.id, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name, + scopes: "public", ) allow_any_instance_of(Doorkeeper::AccessToken).to receive(:reusable?).and_return(false) diff --git a/spec/lib/stale_records_cleaner_spec.rb b/spec/lib/stale_records_cleaner_spec.rb index af4b9295a..6f09c352c 100644 --- a/spec/lib/stale_records_cleaner_spec.rb +++ b/spec/lib/stale_records_cleaner_spec.rb @@ -10,6 +10,7 @@ access_grant: Doorkeeper::AccessGrant, } end + let(:resource_owner) { FactoryBot.create(:resource_owner) } context "when ORM has no cleaner class" do it "raises an error" do @@ -30,7 +31,10 @@ context "with revoked record" do before do - FactoryBot.create model_name, revoked_at: Time.current - 1.minute + FactoryBot.create model_name, + revoked_at: Time.current - 1.minute, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "removes the record" do @@ -40,7 +44,9 @@ context "with record revoked in the future" do before do - FactoryBot.create model_name, revoked_at: Time.current + 1.minute + FactoryBot.create model_name, revoked_at: Time.current + 1.minute, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "keeps the record" do @@ -50,7 +56,9 @@ context "with unrevoked record" do before do - FactoryBot.create model_name, revoked_at: nil + FactoryBot.create model_name, revoked_at: nil, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "keeps the record" do @@ -66,7 +74,10 @@ context "with record that is expired" do before do - FactoryBot.create model_name, created_at: expiry_border - 1.minute + FactoryBot.create model_name, + created_at: expiry_border - 1.minute, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "removes the record" do @@ -76,7 +87,9 @@ context "with record that is not expired" do before do - FactoryBot.create model_name, created_at: expiry_border + 1.minute + FactoryBot.create model_name, created_at: expiry_border + 1.minute, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "keeps the record" do diff --git a/spec/models/doorkeeper/access_grant_spec.rb b/spec/models/doorkeeper/access_grant_spec.rb index 6b83bba94..737eac6f5 100644 --- a/spec/models/doorkeeper/access_grant_spec.rb +++ b/spec/models/doorkeeper/access_grant_spec.rb @@ -3,10 +3,18 @@ require "spec_helper" describe Doorkeeper::AccessGrant do + let(:resource_owner) { FactoryBot.create(:resource_owner) } let(:client) { FactoryBot.build_stubbed(:application) } let(:clazz) { Doorkeeper::AccessGrant } - subject { FactoryBot.build(:access_grant, application: client) } + subject do + FactoryBot.build( + :access_grant, + application: client, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + ) + end it { expect(subject).to be_valid } @@ -17,7 +25,12 @@ end context "with hashing enabled" do - let(:grant) { FactoryBot.create :access_grant } + let(:resource_owner) { FactoryBot.create(:resource_owner) } + let(:grant) do + FactoryBot.create :access_grant, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name + end include_context "with token hashing enabled" it "holds a volatile plaintext token when created" do @@ -117,12 +130,12 @@ end describe ".revoke_all_for" do - let(:resource_owner) { double(id: 100) } let(:application) { FactoryBot.create :application } let(:default_attributes) do { application: application, resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, } end @@ -148,9 +161,10 @@ end it "matches resource owner" do + other_resource_owner = FactoryBot.create(:resource_owner) access_grant_for_different_owner = FactoryBot.create( :access_grant, - default_attributes.merge(resource_owner_id: 90), + default_attributes.merge(resource_owner_id: other_resource_owner.id), ) described_class.revoke_all_for application.id, resource_owner diff --git a/spec/models/doorkeeper/access_token_spec.rb b/spec/models/doorkeeper/access_token_spec.rb index 69b3c623f..4f2dcb822 100644 --- a/spec/models/doorkeeper/access_token_spec.rb +++ b/spec/models/doorkeeper/access_token_spec.rb @@ -2,621 +2,649 @@ require "spec_helper" -module Doorkeeper - describe AccessToken do - let(:clazz) { Doorkeeper::AccessToken } - subject { FactoryBot.build(:access_token) } +RSpec.describe Doorkeeper::AccessToken do + subject { FactoryBot.build(:access_token) } - it { expect(subject).to be_valid } + it { expect(subject).to be_valid } - it_behaves_like "an accessible token" - it_behaves_like "a revocable token" - it_behaves_like "a unique token" do - let(:factory_name) { :access_token } - end + it_behaves_like "an accessible token" + it_behaves_like "a revocable token" + it_behaves_like "a unique token" do + let(:factory_name) { :access_token } + end + + module CustomGeneratorArgs + def self.generate; end + end + + describe "#generate_token" do + it "generates a token using the default method" do + FactoryBot.create :access_token - module CustomGeneratorArgs - def self.generate; end + token = FactoryBot.create :access_token + expect(token.token).to be_a(String) end - describe :generate_token do - it "generates a token using the default method" do - FactoryBot.create :access_token + context "with hashing enabled" do + let(:token) { FactoryBot.create :access_token } + include_context "with token hashing enabled" - token = FactoryBot.create :access_token - expect(token.token).to be_a(String) - end + it "holds a volatile plaintext token when created" do + expect(token.plaintext_token).to be_a(String) + expect(token.token) + .to eq(hashed_or_plain_token_func.call(token.plaintext_token)) - context "with hashing enabled" do - let(:token) { FactoryBot.create :access_token } - include_context "with token hashing enabled" + # Finder method only finds the hashed token + loaded = described_class.find_by(token: token.token) + expect(loaded).to eq(token) + expect(loaded.plaintext_token).to be_nil + expect(loaded.token).to eq(token.token) + end - it "holds a volatile plaintext token when created" do - expect(token.plaintext_token).to be_a(String) - expect(token.token) - .to eq(hashed_or_plain_token_func.call(token.plaintext_token)) + it "does not find_by plain text tokens" do + expect(described_class.find_by(token: token.plaintext_token)).to be_nil + end - # Finder method only finds the hashed token - loaded = clazz.find_by(token: token.token) - expect(loaded).to eq(token) - expect(loaded.plaintext_token).to be_nil - expect(loaded.token).to eq(token.token) - end + describe "with having a plain text token" do + let(:plain_text_token) { "plain text token" } + let(:access_token) { FactoryBot.create :access_token } - it "does not find_by plain text tokens" do - expect(clazz.find_by(token: token.plaintext_token)).to be_nil + before do + # Assume we have a plain text token from before activating the option + access_token.update_column(:token, plain_text_token) end - describe "with having a plain text token" do - let(:plain_text_token) { "plain text token" } - let(:access_token) { FactoryBot.create :access_token } + context "without fallback lookup" do + it "does not provide lookups with either through by_token" do + expect(described_class.by_token(plain_text_token)).to eq(nil) + expect(described_class.by_token(access_token.token)).to eq(nil) - before do - # Assume we have a plain text token from before activating the option - access_token.update_column(:token, plain_text_token) + # And it does not touch the token + access_token.reload + expect(access_token.token).to eq(plain_text_token) end + end - context "without fallback lookup" do - it "does not provide lookups with either through by_token" do - expect(clazz.by_token(plain_text_token)).to eq(nil) - expect(clazz.by_token(access_token.token)).to eq(nil) - - # And it does not touch the token - access_token.reload - expect(access_token.token).to eq(plain_text_token) + context "with fallback lookup" do + include_context "with token hashing and fallback lookup enabled" + + it "upgrades a plain token when falling back to it" do + # Side-effect: This will automatically upgrade the token + expect(described_class).to receive(:upgrade_fallback_value).and_call_original + expect(described_class.by_token(plain_text_token)) + .to have_attributes( + resource_owner_id: access_token.resource_owner_id, + application_id: access_token.application_id, + scopes: access_token.scopes, + ) + + # Will find subsequently by hashing the token + expect(described_class.by_token(plain_text_token)) + .to have_attributes( + resource_owner_id: access_token.resource_owner_id, + application_id: access_token.application_id, + scopes: access_token.scopes, + ) + + # Not all the ORM support :id PK + if access_token.respond_to?(:id) + expect(described_class.by_token(plain_text_token).id).to eq(access_token.id) end - end - context "with fallback lookup" do - include_context "with token hashing and fallback lookup enabled" - - it "upgrades a plain token when falling back to it" do - # Side-effect: This will automatically upgrade the token - expect(clazz).to receive(:upgrade_fallback_value).and_call_original - expect(clazz.by_token(plain_text_token)) - .to have_attributes( - resource_owner_id: access_token.resource_owner_id, - application_id: access_token.application_id, - scopes: access_token.scopes, - ) - - # Will find subsequently by hashing the token - expect(clazz.by_token(plain_text_token)) - .to have_attributes( - resource_owner_id: access_token.resource_owner_id, - application_id: access_token.application_id, - scopes: access_token.scopes, - ) - - # Not all the ORM support :id PK - if access_token.respond_to?(:id) - expect(clazz.by_token(plain_text_token).id).to eq(access_token.id) - end - - # And it modifies the token value - access_token.reload - expect(access_token.token).not_to eq(plain_text_token) - expect(clazz.find_by(token: plain_text_token)).to eq(nil) - expect(clazz.find_by(token: access_token.token)).not_to be_nil - end + # And it modifies the token value + access_token.reload + expect(access_token.token).not_to eq(plain_text_token) + expect(described_class.find_by(token: plain_text_token)).to eq(nil) + expect(described_class.find_by(token: access_token.token)).not_to be_nil end end end + end - it "generates a token using a custom object" do - eigenclass = class << CustomGeneratorArgs; self; end - eigenclass.class_eval do - remove_method :generate - end - module CustomGeneratorArgs - def self.generate(opts = {}) - "custom_generator_token_#{opts[:resource_owner_id]}" - end - end - - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + it "generates a token using a custom object" do + eigenclass = class << CustomGeneratorArgs; self; end + eigenclass.class_eval do + remove_method :generate + end + module CustomGeneratorArgs + def self.generate(opts = {}) + id = opts[:resource_owner_id] || opts[:resource_owner]&.id + "custom_generator_token_#{id}" end + end - token = FactoryBot.create :access_token - expect(token.token).to match(/custom_generator_token_\d+/) + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "allows the custom generator to access the application details" do - eigenclass = class << CustomGeneratorArgs; self; end - eigenclass.class_eval do - remove_method :generate - end + owner = FactoryBot.create :resource_owner + token = FactoryBot.create :access_token, + resource_owner_id: owner.id, + resource_owner_type: owner.class.name - module CustomGeneratorArgs - def self.generate(opts = {}) - "custom_generator_token_#{opts[:application].name}" - end - end + expect(token.token).to match(/custom_generator_token_\d+/) + end + + it "allows the custom generator to access the application details" do + eigenclass = class << CustomGeneratorArgs; self; end + eigenclass.class_eval do + remove_method :generate + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + module CustomGeneratorArgs + def self.generate(opts = {}) + "custom_generator_token_#{opts[:application].name}" end + end - token = FactoryBot.create :access_token - expect(token.token).to match(/custom_generator_token_Application \d+/) + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "allows the custom generator to access the scopes" do - eigenclass = class << CustomGeneratorArgs; self; end - eigenclass.class_eval do - remove_method :generate - end - module CustomGeneratorArgs - def self.generate(opts = {}) - "custom_generator_token_#{opts[:scopes].count}_#{opts[:scopes]}" - end - end + token = FactoryBot.create :access_token + expect(token.token).to match(/custom_generator_token_Application \d+/) + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + it "allows the custom generator to access the scopes" do + eigenclass = class << CustomGeneratorArgs; self; end + eigenclass.class_eval do + remove_method :generate + end + module CustomGeneratorArgs + def self.generate(opts = {}) + "custom_generator_token_#{opts[:scopes].count}_#{opts[:scopes]}" end + end - token = FactoryBot.create :access_token, scopes: "public write" - - expect(token.token).to eq "custom_generator_token_2_public write" + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "allows the custom generator to access the expiry length" do - eigenclass = class << CustomGeneratorArgs; self; end - eigenclass.class_eval do - remove_method :generate - end - module CustomGeneratorArgs - def self.generate(opts = {}) - "custom_generator_token_#{opts[:expires_in]}" - end - end + token = FactoryBot.create :access_token, scopes: "public write" + + expect(token.token).to eq "custom_generator_token_2_public write" + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + it "allows the custom generator to access the expiry length" do + eigenclass = class << CustomGeneratorArgs; self; end + eigenclass.class_eval do + remove_method :generate + end + module CustomGeneratorArgs + def self.generate(opts = {}) + "custom_generator_token_#{opts[:expires_in]}" end + end - token = FactoryBot.create :access_token - expect(token.token).to eq "custom_generator_token_7200" + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "allows the custom generator to access the created time" do - module CustomGeneratorArgs - def self.generate(opts = {}) - "custom_generator_token_#{opts[:created_at].to_i}" - end - end + token = FactoryBot.create :access_token + expect(token.token).to eq "custom_generator_token_7200" + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + it "allows the custom generator to access the created time" do + module CustomGeneratorArgs + def self.generate(opts = {}) + "custom_generator_token_#{opts[:created_at].to_i}" end + end - token = FactoryBot.create :access_token - created_at = token.created_at - expect(token.token).to eq "custom_generator_token_#{created_at.to_i}" + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "raises an error if the custom object does not support generate" do - module NoGenerate - end + token = FactoryBot.create :access_token + created_at = token.created_at + expect(token.token).to eq "custom_generator_token_#{created_at.to_i}" + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::NoGenerate" - end + it "raises an error if the custom object does not support generate" do + module NoGenerate + end - expect { FactoryBot.create :access_token }.to( - raise_error(Doorkeeper::Errors::UnableToGenerateToken), - ) + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "NoGenerate" end - it "raises original error if something went wrong in custom generator" do - eigenclass = class << CustomGeneratorArgs; self; end - eigenclass.class_eval do - remove_method :generate - end + expect { FactoryBot.create :access_token }.to( + raise_error(Doorkeeper::Errors::UnableToGenerateToken), + ) + end - module CustomGeneratorArgs - def self.generate(_opts = {}) - raise LoadError, "custom behaviour" - end - end + it "raises original error if something went wrong in custom generator" do + eigenclass = class << CustomGeneratorArgs; self; end + eigenclass.class_eval do + remove_method :generate + end - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::CustomGeneratorArgs" + module CustomGeneratorArgs + def self.generate(_opts = {}) + raise LoadError, "custom behaviour" end + end - expect { FactoryBot.create :access_token }.to( - raise_error(LoadError), - ) + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "CustomGeneratorArgs" end - it "raises an error if the custom object does not exist" do - Doorkeeper.configure do - orm DOORKEEPER_ORM - access_token_generator "Doorkeeper::NotReal" - end + expect { FactoryBot.create :access_token }.to( + raise_error(LoadError), + ) + end - expect { FactoryBot.create :access_token }.to( - raise_error(Doorkeeper::Errors::TokenGeneratorNotFound, /NotReal/), - ) + it "raises an error if the custom object does not exist" do + Doorkeeper.configure do + orm DOORKEEPER_ORM + access_token_generator "Doorkeeper::NotReal" end + + expect { FactoryBot.create :access_token }.to( + raise_error(Doorkeeper::Errors::TokenGeneratorNotFound, /NotReal/), + ) end + end - describe :refresh_token do - it "has empty refresh token if it was not required" do - token = FactoryBot.create :access_token - expect(token.refresh_token).to be_nil - end + describe "refresh_token" do + it "has empty refresh token if it was not required" do + token = FactoryBot.create :access_token + expect(token.refresh_token).to be_nil + end - it "generates a refresh token if it was requested" do - token = FactoryBot.create :access_token, use_refresh_token: true - expect(token.refresh_token).not_to be_nil - end + it "generates a refresh token if it was requested" do + token = FactoryBot.create :access_token, use_refresh_token: true + expect(token.refresh_token).not_to be_nil + end + + it "is not valid if token exists" do + token1 = FactoryBot.create :access_token, use_refresh_token: true + token2 = FactoryBot.create :access_token, use_refresh_token: true + token2.refresh_token = token1.refresh_token + expect(token2).not_to be_valid + end - it "is not valid if token exists" do - token1 = FactoryBot.create :access_token, use_refresh_token: true - token2 = FactoryBot.create :access_token, use_refresh_token: true + it "expects database to raise an error if refresh tokens are the same" do + token1 = FactoryBot.create :access_token, use_refresh_token: true + token2 = FactoryBot.create :access_token, use_refresh_token: true + expect do token2.refresh_token = token1.refresh_token - expect(token2).not_to be_valid - end + token2.save(validate: false) + end.to raise_error(uniqueness_error) + end - it "expects database to raise an error if refresh tokens are the same" do - token1 = FactoryBot.create :access_token, use_refresh_token: true - token2 = FactoryBot.create :access_token, use_refresh_token: true - expect do - token2.refresh_token = token1.refresh_token - token2.save(validate: false) - end.to raise_error(uniqueness_error) - end + context "with hashing enabled" do + include_context "with token hashing enabled" + let(:token) { FactoryBot.create :access_token, use_refresh_token: true } - context "with hashing enabled" do - include_context "with token hashing enabled" - let(:token) { FactoryBot.create :access_token, use_refresh_token: true } + it "holds a volatile refresh token when created" do + expect(token.plaintext_refresh_token).to be_a(String) + expect(token.refresh_token) + .to eq(hashed_or_plain_token_func.call(token.plaintext_refresh_token)) - it "holds a volatile refresh token when created" do - expect(token.plaintext_refresh_token).to be_a(String) - expect(token.refresh_token) - .to eq(hashed_or_plain_token_func.call(token.plaintext_refresh_token)) + # Finder method only finds the hashed token + loaded = described_class.find_by(refresh_token: token.refresh_token) + expect(loaded).to eq(token) + expect(loaded.plaintext_refresh_token).to be_nil + expect(loaded.refresh_token).to eq(token.refresh_token) + end - # Finder method only finds the hashed token - loaded = clazz.find_by(refresh_token: token.refresh_token) - expect(loaded).to eq(token) - expect(loaded.plaintext_refresh_token).to be_nil - expect(loaded.refresh_token).to eq(token.refresh_token) - end + it "does not find_by plain text refresh tokens" do + expect(described_class.find_by(refresh_token: token.plaintext_refresh_token)).to be_nil + end + + describe "with having a plain text token" do + let(:plain_refresh_token) { "plain refresh token" } + let(:access_token) { FactoryBot.create :access_token } - it "does not find_by plain text refresh tokens" do - expect(clazz.find_by(refresh_token: token.plaintext_refresh_token)).to be_nil + before do + # Assume we have a plain text token from before activating the option + access_token.update_column(:refresh_token, plain_refresh_token) end - describe "with having a plain text token" do - let(:plain_refresh_token) { "plain refresh token" } - let(:access_token) { FactoryBot.create :access_token } + context "without fallback lookup" do + it "does not provide lookups with either through by_token" do + expect(described_class.by_refresh_token(plain_refresh_token)).to eq(nil) + expect(described_class.by_refresh_token(access_token.refresh_token)).to eq(nil) - before do - # Assume we have a plain text token from before activating the option - access_token.update_column(:refresh_token, plain_refresh_token) + # And it does not touch the token + access_token.reload + expect(access_token.refresh_token).to eq(plain_refresh_token) end + end - context "without fallback lookup" do - it "does not provide lookups with either through by_token" do - expect(clazz.by_refresh_token(plain_refresh_token)).to eq(nil) - expect(clazz.by_refresh_token(access_token.refresh_token)).to eq(nil) - - # And it does not touch the token - access_token.reload - expect(access_token.refresh_token).to eq(plain_refresh_token) + context "with fallback lookup" do + include_context "with token hashing and fallback lookup enabled" + + it "upgrades a plain token when falling back to it" do + # Side-effect: This will automatically upgrade the token + expect(described_class).to receive(:upgrade_fallback_value).and_call_original + expect(described_class.by_refresh_token(plain_refresh_token)) + .to have_attributes( + token: access_token.token, + resource_owner_id: access_token.resource_owner_id, + application_id: access_token.application_id, + ) + + # Will find subsequently by hashing the token + expect(described_class.by_refresh_token(plain_refresh_token)) + .to have_attributes( + token: access_token.token, + resource_owner_id: access_token.resource_owner_id, + application_id: access_token.application_id, + ) + + # Not all the ORM support :id PK + if access_token.respond_to?(:id) + expect(described_class.by_refresh_token(plain_refresh_token).id).to eq(access_token.id) end - end - context "with fallback lookup" do - include_context "with token hashing and fallback lookup enabled" - - it "upgrades a plain token when falling back to it" do - # Side-effect: This will automatically upgrade the token - expect(clazz).to receive(:upgrade_fallback_value).and_call_original - expect(clazz.by_refresh_token(plain_refresh_token)) - .to have_attributes( - token: access_token.token, - resource_owner_id: access_token.resource_owner_id, - application_id: access_token.application_id, - ) - - # Will find subsequently by hashing the token - expect(clazz.by_refresh_token(plain_refresh_token)) - .to have_attributes( - token: access_token.token, - resource_owner_id: access_token.resource_owner_id, - application_id: access_token.application_id, - ) - - # Not all the ORM support :id PK - if access_token.respond_to?(:id) - expect(clazz.by_refresh_token(plain_refresh_token).id).to eq(access_token.id) - end - - # And it modifies the token value - access_token.reload - expect(access_token.refresh_token).not_to eq(plain_refresh_token) - expect(clazz.find_by(refresh_token: plain_refresh_token)).to eq(nil) - expect(clazz.find_by(refresh_token: access_token.refresh_token)).not_to be_nil - end + # And it modifies the token value + access_token.reload + expect(access_token.refresh_token).not_to eq(plain_refresh_token) + expect(described_class.find_by(refresh_token: plain_refresh_token)).to eq(nil) + expect(described_class.find_by(refresh_token: access_token.refresh_token)).not_to be_nil end end end end + end - describe "validations" do - it "is valid without resource_owner_id" do - # For client credentials flow - subject.resource_owner_id = nil - expect(subject).to be_valid - end + describe "validations" do + it "is valid without resource_owner_id" do + # For client credentials flow + subject.resource_owner_id = nil + expect(subject).to be_valid + end - it "is valid without application_id" do - # For resource owner credentials flow - subject.application_id = nil - expect(subject).to be_valid - end + it "is valid without application_id" do + # For resource owner credentials flow + subject.application_id = nil + expect(subject).to be_valid end + end - describe "#same_credential?" do - context "with default parameters" do - let(:resource_owner_id) { 100 } - let(:application) { FactoryBot.create :application } - let(:default_attributes) do - { application: application, resource_owner_id: resource_owner_id } - end - let(:access_token1) { FactoryBot.create :access_token, default_attributes } + describe "#same_credential?" do + context "with default parameters" do + let(:resource_owner) { FactoryBot.create(:resource_owner) } + let(:resource_owner_id) { resource_owner.id } + let(:application) { FactoryBot.create :application } + let(:default_attributes) do + { + application: application, + resource_owner_id: resource_owner_id, + resource_owner_type: resource_owner.class.name, + } + end + let(:access_token1) { FactoryBot.create :access_token, default_attributes } - context "the second token has the same owner and same app" do - let(:access_token2) { FactoryBot.create :access_token, default_attributes } - it "success" do - expect(access_token1.same_credential?(access_token2)).to be_truthy - end + context "the second token has the same owner and same app" do + let(:access_token2) { FactoryBot.create :access_token, default_attributes } + it "success" do + expect(access_token1.same_credential?(access_token2)).to be_truthy end + end - context "the second token has same owner and different app" do - let(:other_application) { FactoryBot.create :application } - let(:access_token2) do - FactoryBot.create :access_token, - application: other_application, - resource_owner_id: resource_owner_id - end + context "the second token has same owner and different app" do + let(:other_application) { FactoryBot.create :application } + let(:access_token2) do + FactoryBot.create :access_token, + application: other_application, + resource_owner_id: resource_owner_id, + resource_owner_type: resource_owner.class.name + end - it "fail" do - expect(access_token1.same_credential?(access_token2)).to be_falsey - end + it "fail" do + expect(access_token1.same_credential?(access_token2)).to be_falsey end + end - context "the second token has different owner and different app" do - let(:other_application) { FactoryBot.create :application } - let(:access_token2) do - FactoryBot.create :access_token, application: other_application, resource_owner_id: 42 - end + context "the second token has different owner and different app" do + let(:other_application) { FactoryBot.create :application } + let(:access_token2) do + FactoryBot.create :access_token, + application: other_application, + resource_owner_id: resource_owner.id + 1 + end - it "fail" do - expect(access_token1.same_credential?(access_token2)).to be_falsey - end + it "fail" do + expect(access_token1.same_credential?(access_token2)).to be_falsey end + end - context "the second token has different owner and same app" do - let(:access_token2) do - FactoryBot.create :access_token, application: application, resource_owner_id: 42 - end + context "the second token has different owner and same app" do + let(:access_token2) do + FactoryBot.create :access_token, + application: application, + resource_owner_id: resource_owner.id + 1 + end - it "fail" do - expect(access_token1.same_credential?(access_token2)).to be_falsey - end + it "fail" do + expect(access_token1.same_credential?(access_token2)).to be_falsey end end end + end - describe "#acceptable?" do - context "a token that is not accessible" do - let(:token) { FactoryBot.create(:access_token, created_at: 6.hours.ago) } + describe "#acceptable?" do + context "a token that is not accessible" do + let(:token) { FactoryBot.create(:access_token, created_at: 6.hours.ago) } - it "should return false" do - expect(token.acceptable?(nil)).to be false - end + it "should return false" do + expect(token.acceptable?(nil)).to be false end + end - context "a token that has the incorrect scopes" do - let(:token) { FactoryBot.create(:access_token) } + context "a token that has the incorrect scopes" do + let(:token) { FactoryBot.create(:access_token) } - it "should return false" do - expect(token.acceptable?(["public"])).to be false - end + it "should return false" do + expect(token.acceptable?(["public"])).to be false end + end - context "a token is acceptable with the correct scopes" do - let(:token) do - token = FactoryBot.create(:access_token) - token[:scopes] = "public" - token - end + context "a token is acceptable with the correct scopes" do + let(:token) do + token = FactoryBot.create(:access_token) + token[:scopes] = "public" + token + end - it "should return true" do - expect(token.acceptable?(["public"])).to be true - end + it "should return true" do + expect(token.acceptable?(["public"])).to be true end end + end - describe ".revoke_all_for" do - let(:resource_owner) { double(id: 100) } - let(:application) { FactoryBot.create :application } - let(:default_attributes) do - { application: application, resource_owner_id: resource_owner.id } - end + describe ".revoke_all_for" do + let(:resource_owner) { FactoryBot.create :resource_owner } + let(:application) { FactoryBot.create :application } + let(:default_attributes) do + { + application: application, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + } + end - it "revokes all tokens for given application and resource owner" do - FactoryBot.create :access_token, default_attributes - AccessToken.revoke_all_for application.id, resource_owner - AccessToken.all.each do |token| - expect(token).to be_revoked - end + it "revokes all tokens for given application and resource owner" do + FactoryBot.create :access_token, default_attributes + described_class.revoke_all_for application.id, resource_owner + described_class.all.each do |token| + expect(token).to be_revoked end + end - it "matches application" do - access_token_for_different_app = FactoryBot.create( - :access_token, - default_attributes.merge(application: FactoryBot.create(:application)), - ) + it "matches application" do + access_token_for_different_app = FactoryBot.create( + :access_token, + default_attributes.merge(application: FactoryBot.create(:application)), + ) - AccessToken.revoke_all_for application.id, resource_owner + described_class.revoke_all_for application.id, resource_owner - expect(access_token_for_different_app.reload).not_to be_revoked - end + expect(access_token_for_different_app.reload).not_to be_revoked + end - it "matches resource owner" do - access_token_for_different_owner = FactoryBot.create( - :access_token, - default_attributes.merge(resource_owner_id: 90), - ) + it "matches resource owner" do + access_token_for_different_owner = FactoryBot.create( + :access_token, + default_attributes.merge(resource_owner_id: resource_owner.id + 1), + ) - AccessToken.revoke_all_for application.id, resource_owner + described_class.revoke_all_for application.id, resource_owner - expect(access_token_for_different_owner.reload).not_to be_revoked - end + expect(access_token_for_different_owner.reload).not_to be_revoked end + end - describe ".matching_token_for" do - let(:resource_owner_id) { 100 } - let(:application) { FactoryBot.create :application } - let(:scopes) { Doorkeeper::OAuth::Scopes.from_string("public write") } - let(:default_attributes) do - { - application: application, - resource_owner_id: resource_owner_id, - scopes: scopes.to_s, - } - end + describe ".matching_token_for" do + let(:resource_owner) { FactoryBot.create :resource_owner } + let(:resource_owner_id) { resource_owner.id } + let(:application) { FactoryBot.create :application } + let(:scopes) { Doorkeeper::OAuth::Scopes.from_string("public write") } + let(:default_attributes) do + { + application: application, + resource_owner_id: resource_owner_id, + resource_owner_type: resource_owner.class.name, + scopes: scopes.to_s, + } + end - before do - default_scopes_exist(*scopes.all) - end + before do + default_scopes_exist(*scopes.all) + end - it "returns only one token" do - token = FactoryBot.create :access_token, default_attributes - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to eq(token) - end + it "returns only one token" do + token = FactoryBot.create :access_token, default_attributes + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to eq(token) + end - it "accepts resource owner as object" do - resource_owner = double(to_key: true, id: 100) - token = FactoryBot.create :access_token, default_attributes - last_token = AccessToken.matching_token_for(application, resource_owner, scopes) - expect(last_token).to eq(token) - end + it "accepts resource owner as object" do + token = FactoryBot.create :access_token, default_attributes + last_token = described_class.matching_token_for(application, resource_owner, scopes) + expect(last_token).to eq(token) + end - it "accepts nil as resource owner" do - token = FactoryBot.create :access_token, default_attributes.merge(resource_owner_id: nil) - last_token = AccessToken.matching_token_for(application, nil, scopes) - expect(last_token).to eq(token) - end + it "accepts nil as resource owner" do + token = FactoryBot.create :access_token, + default_attributes.merge(resource_owner_id: nil, resource_owner_type: nil) + last_token = described_class.matching_token_for(application, nil, scopes) + expect(last_token).to eq(token) + end - it "excludes revoked tokens" do - FactoryBot.create :access_token, default_attributes.merge(revoked_at: 1.day.ago) - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes revoked tokens" do + FactoryBot.create :access_token, default_attributes.merge(revoked_at: 1.day.ago) + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with a different application" do - FactoryBot.create :access_token, default_attributes.merge(application: FactoryBot.create(:application)) - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with a different application" do + FactoryBot.create :access_token, default_attributes.merge(application: FactoryBot.create(:application)) + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with a different resource owner" do - FactoryBot.create :access_token, default_attributes.merge(resource_owner_id: 2) - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with a different resource owner" do + FactoryBot.create :access_token, default_attributes.merge(resource_owner_id: resource_owner.id + 1) + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with fewer scopes" do - FactoryBot.create :access_token, default_attributes.merge(scopes: "public") - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with fewer scopes" do + FactoryBot.create :access_token, default_attributes.merge(scopes: "public") + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with different scopes" do - FactoryBot.create :access_token, default_attributes.merge(scopes: "public email") - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with different scopes" do + FactoryBot.create :access_token, default_attributes.merge(scopes: "public email") + last_token = described_class.matching_token_for(application, resource_owner, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with additional scopes" do - FactoryBot.create :access_token, default_attributes.merge(scopes: "public write email") - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with additional scopes" do + FactoryBot.create :access_token, default_attributes.merge(scopes: "public write email") + last_token = described_class.matching_token_for(application, resource_owner, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with scopes that are not present in server scopes" do - FactoryBot.create :access_token, default_attributes.merge( - application: application, scopes: "public read", - ) - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with scopes that are not present in server scopes" do + FactoryBot.create :access_token, default_attributes.merge( + application: application, scopes: "public read", + ) + last_token = described_class.matching_token_for(application, resource_owner, scopes) + expect(last_token).to be_nil + end - it "excludes tokens with scopes that are not present in application scopes" do - application = FactoryBot.create :application, scopes: "private read" - FactoryBot.create :access_token, default_attributes.merge( - application: application, - ) - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to be_nil - end + it "excludes tokens with scopes that are not present in application scopes" do + application = FactoryBot.create :application, scopes: "private read" + FactoryBot.create :access_token, default_attributes.merge( + application: application, + ) + last_token = described_class.matching_token_for(application, resource_owner, scopes) + expect(last_token).to be_nil + end - it "does not match token if empty scope requested and token/app scopes present" do - application = FactoryBot.create :application, scopes: "sample:scope" - app_params = { - application_id: application.id, scopes: "sample:scope", - resource_owner_id: 100, - } - FactoryBot.create :access_token, app_params - empty_scopes = Doorkeeper::OAuth::Scopes.from_string("") - last_token = AccessToken.matching_token_for(application, 100, empty_scopes) - expect(last_token).to be_nil - end + it "does not match token if empty scope requested and token/app scopes present" do + application = FactoryBot.create :application, scopes: "sample:scope" + app_params = { + application_id: application.id, scopes: "sample:scope", + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + } + FactoryBot.create :access_token, app_params + empty_scopes = Doorkeeper::OAuth::Scopes.from_string("") + last_token = described_class.matching_token_for(application, resource_owner.id, empty_scopes) + expect(last_token).to be_nil + end - it "matches token if empty scope requested and no token scopes present" do - empty_scopes = Doorkeeper::OAuth::Scopes.from_string("") - token = FactoryBot.create :access_token, default_attributes.merge(scopes: empty_scopes) - last_token = AccessToken.matching_token_for(application, 100, empty_scopes) - expect(last_token).to eq(token) - end + it "matches token if empty scope requested and no token scopes present" do + empty_scopes = Doorkeeper::OAuth::Scopes.from_string("") + token = FactoryBot.create :access_token, default_attributes.merge(scopes: empty_scopes) + last_token = described_class.matching_token_for(application, resource_owner.id, empty_scopes) + expect(last_token).to eq(token) + end - it "returns the last matching token" do - FactoryBot.create :access_token, default_attributes.merge(created_at: 1.day.ago) - matching_token = FactoryBot.create :access_token, default_attributes - FactoryBot.create :access_token, default_attributes.merge(scopes: "public") + it "returns the last matching token" do + FactoryBot.create :access_token, default_attributes.merge(created_at: 1.day.ago) + matching_token = FactoryBot.create :access_token, default_attributes + FactoryBot.create :access_token, default_attributes.merge(scopes: "public") - last_token = AccessToken.matching_token_for(application, resource_owner_id, scopes) - expect(last_token).to eq(matching_token) - end + last_token = described_class.matching_token_for(application, resource_owner_id, scopes) + expect(last_token).to eq(matching_token) + end + end + + describe "#as_json" do + let(:token) { FactoryBot.create(:access_token) } + let(:token_hash) do + { + resource_owner_id: token.resource_owner_id, + scope: token.scopes, + expires_in: token.expires_in_seconds, + application: { uid: token.application.uid }, + created_at: token.created_at.to_i, + } end - describe "#as_json" do - it "returns as_json hash" do - token = FactoryBot.create :access_token - token_hash = { - resource_owner_id: token.resource_owner_id, - scope: token.scopes, - expires_in: token.expires_in_seconds, - application: { uid: token.application.uid }, - created_at: token.created_at.to_i, - } - expect(token.as_json).to eq token_hash + it "returns as_json hash" do + hash = token_hash + + if Doorkeeper.configuration.polymorphic_resource_owner? + hash[:resource_owner_type] = token.resource_owner_type end + + expect(token.as_json).to match(hash) end end end diff --git a/spec/models/doorkeeper/application_spec.rb b/spec/models/doorkeeper/application_spec.rb index db60fe7a9..24a7b1736 100644 --- a/spec/models/doorkeeper/application_spec.rb +++ b/spec/models/doorkeeper/application_spec.rb @@ -3,401 +3,439 @@ require "spec_helper" require "bcrypt" -module Doorkeeper - describe Application do - let(:clazz) { Doorkeeper::Application } - let(:require_owner) { Doorkeeper.configuration.instance_variable_set("@confirm_application_owner", true) } - let(:unset_require_owner) { Doorkeeper.configuration.instance_variable_set("@confirm_application_owner", false) } - let(:new_application) { FactoryBot.build(:application) } +describe Doorkeeper::Application do + let(:require_owner) { Doorkeeper.config.instance_variable_set("@confirm_application_owner", true) } + let(:unset_require_owner) { Doorkeeper.config.instance_variable_set("@confirm_application_owner", false) } + let(:new_application) { FactoryBot.build(:application) } - let(:uid) { SecureRandom.hex(8) } - let(:secret) { SecureRandom.hex(8) } + let(:uid) { SecureRandom.hex(8) } + let(:secret) { SecureRandom.hex(8) } - it "is invalid without a name" do - new_application.name = nil - expect(new_application).not_to be_valid - end + it "is invalid without a name" do + new_application.name = nil + expect(new_application).not_to be_valid + end - it "is invalid without determining confidentiality" do - new_application.confidential = nil - expect(new_application).not_to be_valid - end + it "is invalid without determining confidentiality" do + new_application.confidential = nil + expect(new_application).not_to be_valid + end - it "generates uid on create" do - expect(new_application.uid).to be_nil - new_application.save - expect(new_application.uid).not_to be_nil - end + it "generates uid on create" do + expect(new_application.uid).to be_nil + new_application.save + expect(new_application.uid).not_to be_nil + end - it "generates uid on create if an empty string" do - new_application.uid = "" - new_application.save - expect(new_application.uid).not_to be_blank - end + it "generates uid on create if an empty string" do + new_application.uid = "" + new_application.save + expect(new_application.uid).not_to be_blank + end - it "generates uid on create unless one is set" do - new_application.uid = uid - new_application.save - expect(new_application.uid).to eq(uid) - end + it "generates uid on create unless one is set" do + new_application.uid = uid + new_application.save + expect(new_application.uid).to eq(uid) + end - it "is invalid without uid" do - new_application.save - new_application.uid = nil - expect(new_application).not_to be_valid - end + it "is invalid without uid" do + new_application.save + new_application.uid = nil + expect(new_application).not_to be_valid + end - it "checks uniqueness of uid" do - app1 = FactoryBot.create(:application) - app2 = FactoryBot.create(:application) - app2.uid = app1.uid - expect(app2).not_to be_valid - end + it "checks uniqueness of uid" do + app1 = FactoryBot.create(:application) + app2 = FactoryBot.create(:application) + app2.uid = app1.uid + expect(app2).not_to be_valid + end - it "expects database to throw an error when uids are the same" do - app1 = FactoryBot.create(:application) - app2 = FactoryBot.create(:application) - app2.uid = app1.uid - expect { app2.save!(validate: false) }.to raise_error(uniqueness_error) - end + it "expects database to throw an error when uids are the same" do + app1 = FactoryBot.create(:application) + app2 = FactoryBot.create(:application) + app2.uid = app1.uid + expect { app2.save!(validate: false) }.to raise_error(uniqueness_error) + end - it "generate secret on create" do - expect(new_application.secret).to be_nil - new_application.save - expect(new_application.secret).not_to be_nil - end + it "generate secret on create" do + expect(new_application.secret).to be_nil + new_application.save + expect(new_application.secret).not_to be_nil + end - it "generate secret on create if is blank string" do - new_application.secret = "" - new_application.save - expect(new_application.secret).not_to be_blank - end + it "generate secret on create if is blank string" do + new_application.secret = "" + new_application.save + expect(new_application.secret).not_to be_blank + end - it "generate secret on create unless one is set" do - new_application.secret = secret - new_application.save - expect(new_application.secret).to eq(secret) - end + it "generate secret on create unless one is set" do + new_application.secret = secret + new_application.save + expect(new_application.secret).to eq(secret) + end - it "is invalid without secret" do - new_application.save - new_application.secret = nil - expect(new_application).not_to be_valid - end + it "is invalid without secret" do + new_application.save + new_application.secret = nil + expect(new_application).not_to be_valid + end - context "application_owner is enabled" do - before do - Doorkeeper.configure do - orm DOORKEEPER_ORM - enable_application_owner - end + context "application_owner is enabled" do + before do + Doorkeeper.configure do + orm DOORKEEPER_ORM + enable_application_owner end + end - context "application owner is not required" do - before(:each) do - unset_require_owner - end + context "application owner is not required" do + before(:each) do + unset_require_owner + end - it "is valid given valid attributes" do - expect(new_application).to be_valid - end + it "is valid given valid attributes" do + expect(new_application).to be_valid end + end - context "application owner is required" do - before(:each) do - require_owner - @owner = FactoryBot.build_stubbed(:doorkeeper_testing_user) - end + context "application owner is required" do + before(:each) do + require_owner + @owner = FactoryBot.build_stubbed(:doorkeeper_testing_user) + end - it "is invalid without an owner" do - expect(new_application).not_to be_valid - end + it "is invalid without an owner" do + expect(new_application).not_to be_valid + end - it "is valid with an owner" do - new_application.owner = @owner - expect(new_application).to be_valid - end + it "is valid with an owner" do + new_application.owner = @owner + expect(new_application).to be_valid end end + end - context "redirect URI" do - context "when grant flows allow blank redirect URI" do - before do - Doorkeeper.configure do - grant_flows %w[password client_credentials] - end + context "redirect URI" do + context "when grant flows allow blank redirect URI" do + before do + Doorkeeper.configure do + grant_flows %w[password client_credentials] end + end - it "is valid without redirect_uri" do - new_application.save - new_application.redirect_uri = nil - expect(new_application).to be_valid - end + it "is valid without redirect_uri" do + new_application.save + new_application.redirect_uri = nil + expect(new_application).to be_valid end + end - context "when grant flows require redirect URI" do - before do - Doorkeeper.configure do - grant_flows %w[password client_credentials authorization_code] - end + context "when grant flows require redirect URI" do + before do + Doorkeeper.configure do + grant_flows %w[password client_credentials authorization_code] end + end - it "is invalid without redirect_uri" do - new_application.save - new_application.redirect_uri = nil - expect(new_application).not_to be_valid - end + it "is invalid without redirect_uri" do + new_application.save + new_application.redirect_uri = nil + expect(new_application).not_to be_valid end + end - context "when blank URI option disabled" do - before do - Doorkeeper.configure do - grant_flows %w[password client_credentials] - allow_blank_redirect_uri false - end + context "when blank URI option disabled" do + before do + Doorkeeper.configure do + grant_flows %w[password client_credentials] + allow_blank_redirect_uri false end + end - it "is invalid without redirect_uri" do - new_application.save - new_application.redirect_uri = nil - expect(new_application).not_to be_valid - end + it "is invalid without redirect_uri" do + new_application.save + new_application.redirect_uri = nil + expect(new_application).not_to be_valid end end + end - context "with hashing enabled" do - include_context "with application hashing enabled" - let(:app) { FactoryBot.create :application } - let(:default_strategy) { Doorkeeper::SecretStoring::Sha256Hash } + context "with hashing enabled" do + include_context "with application hashing enabled" + let(:app) { FactoryBot.create :application } + let(:default_strategy) { Doorkeeper::SecretStoring::Sha256Hash } - it "uses SHA256 to avoid additional dependencies" do - # Ensure token was generated - app.validate - expect(app.secret).to eq(default_strategy.transform_secret(app.plaintext_secret)) - end + it "uses SHA256 to avoid additional dependencies" do + # Ensure token was generated + app.validate + expect(app.secret).to eq(default_strategy.transform_secret(app.plaintext_secret)) + end - context "when bcrypt strategy is configured" do - # In this text context, we have bcrypt loaded so `bcrypt_present?` - # will always be true - before do - Doorkeeper.configure do - hash_application_secrets using: "Doorkeeper::SecretStoring::BCrypt" - end + context "when bcrypt strategy is configured" do + # In this text context, we have bcrypt loaded so `bcrypt_present?` + # will always be true + before do + Doorkeeper.configure do + hash_application_secrets using: "Doorkeeper::SecretStoring::BCrypt" end + end - it "holds a volatile plaintext and BCrypt secret" do - expect(app.secret_strategy).to eq Doorkeeper::SecretStoring::BCrypt - expect(app.plaintext_secret).to be_a(String) - expect(app.secret).not_to eq(app.plaintext_secret) - expect { ::BCrypt::Password.create(app.secret) }.not_to raise_error - end + it "holds a volatile plaintext and BCrypt secret" do + expect(app.secret_strategy).to eq Doorkeeper::SecretStoring::BCrypt + expect(app.plaintext_secret).to be_a(String) + expect(app.secret).not_to eq(app.plaintext_secret) + expect { ::BCrypt::Password.create(app.secret) }.not_to raise_error end + end - it "does not fallback to plain lookup by default" do - lookup = clazz.by_uid_and_secret(app.uid, app.secret) - expect(lookup).to eq(nil) + it "does not fallback to plain lookup by default" do + lookup = described_class.by_uid_and_secret(app.uid, app.secret) + expect(lookup).to eq(nil) - lookup = clazz.by_uid_and_secret(app.uid, app.plaintext_secret) - expect(lookup).to eq(app) - end + lookup = described_class.by_uid_and_secret(app.uid, app.plaintext_secret) + expect(lookup).to eq(app) + end - context "with fallback enabled" do - include_context "with token hashing and fallback lookup enabled" + context "with fallback enabled" do + include_context "with token hashing and fallback lookup enabled" - it "provides plain and hashed lookup" do - lookup = clazz.by_uid_and_secret(app.uid, app.secret) - expect(lookup).to eq(app) + it "provides plain and hashed lookup" do + lookup = described_class.by_uid_and_secret(app.uid, app.secret) + expect(lookup).to eq(app) - lookup = clazz.by_uid_and_secret(app.uid, app.plaintext_secret) - expect(lookup).to eq(app) - end + lookup = described_class.by_uid_and_secret(app.uid, app.plaintext_secret) + expect(lookup).to eq(app) end + end - it "does not provide access to secret after loading" do - lookup = clazz.by_uid_and_secret(app.uid, app.plaintext_secret) - expect(lookup.plaintext_secret).to be_nil - end + it "does not provide access to secret after loading" do + lookup = described_class.by_uid_and_secret(app.uid, app.plaintext_secret) + expect(lookup.plaintext_secret).to be_nil end + end - describe "destroy related models on cascade" do - before(:each) do - new_application.save - end + describe "destroy related models on cascade" do + before(:each) do + new_application.save + end - it "should destroy its access grants" do - FactoryBot.create(:access_grant, application: new_application) - expect { new_application.destroy }.to change { Doorkeeper::AccessGrant.count }.by(-1) - end + let(:resource_owner) { FactoryBot.create(:resource_owner) } - it "should destroy its access tokens" do - FactoryBot.create(:access_token, application: new_application) - FactoryBot.create(:access_token, application: new_application, revoked_at: Time.now.utc) - expect do - new_application.destroy - end.to change { Doorkeeper::AccessToken.count }.by(-2) - end + it "should destroy its access grants" do + FactoryBot.create( + :access_grant, + application: new_application, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + ) + + expect { new_application.destroy }.to change { Doorkeeper::AccessGrant.count }.by(-1) end - describe "#ordered_by" do - let(:applications) { FactoryBot.create_list(:application, 5) } + it "should destroy its access tokens" do + FactoryBot.create(:access_token, application: new_application) + FactoryBot.create(:access_token, application: new_application, revoked_at: Time.now.utc) + expect do + new_application.destroy + end.to change { Doorkeeper::AccessToken.count }.by(-2) + end + end - context "when a direction is not specified" do - it "calls order with a default order of asc" do - names = applications.map(&:name).sort - expect(Application.ordered_by(:name).map(&:name)).to eq(names) - end + describe "#ordered_by" do + let(:applications) { FactoryBot.create_list(:application, 5) } + + context "when a direction is not specified" do + it "calls order with a default order of asc" do + names = applications.map(&:name).sort + expect(described_class.ordered_by(:name).map(&:name)).to eq(names) end + end - context "when a direction is specified" do - it "calls order with specified direction" do - names = applications.map(&:name).sort.reverse - expect(Application.ordered_by(:name, :desc).map(&:name)).to eq(names) - end + context "when a direction is specified" do + it "calls order with specified direction" do + names = applications.map(&:name).sort.reverse + expect(described_class.ordered_by(:name, :desc).map(&:name)).to eq(names) end end + end - describe "#redirect_uri=" do - context "when array of valid redirect_uris" do - it "should join by newline" do - new_application.redirect_uri = ["http://localhost/callback1", "http://localhost/callback2"] - expect(new_application.redirect_uri).to eq("http://localhost/callback1\nhttp://localhost/callback2") - end + describe "#redirect_uri=" do + context "when array of valid redirect_uris" do + it "should join by newline" do + new_application.redirect_uri = ["http://localhost/callback1", "http://localhost/callback2"] + expect(new_application.redirect_uri).to eq("http://localhost/callback1\nhttp://localhost/callback2") end - context "when string of valid redirect_uris" do - it "should store as-is" do - new_application.redirect_uri = "http://localhost/callback1\nhttp://localhost/callback2" - expect(new_application.redirect_uri).to eq("http://localhost/callback1\nhttp://localhost/callback2") - end + end + context "when string of valid redirect_uris" do + it "should store as-is" do + new_application.redirect_uri = "http://localhost/callback1\nhttp://localhost/callback2" + expect(new_application.redirect_uri).to eq("http://localhost/callback1\nhttp://localhost/callback2") end end + end - describe "#renew_secret" do - let(:app) { FactoryBot.create :application } + describe "#renew_secret" do + let(:app) { FactoryBot.create :application } - it "should generate a new secret" do - old_secret = app.secret - app.renew_secret - expect(old_secret).not_to eq(app.secret) - end + it "should generate a new secret" do + old_secret = app.secret + app.renew_secret + expect(old_secret).not_to eq(app.secret) end + end - describe "#authorized_for" do - let(:resource_owner) { double(:resource_owner, id: 10) } + describe "#authorized_for" do + let(:resource_owner) { FactoryBot.create(:resource_owner) } + let(:other_resource_owner) { FactoryBot.create(:resource_owner) } - it "is empty if the application is not authorized for anyone" do - expect(Application.authorized_for(resource_owner)).to be_empty - end + it "is empty if the application is not authorized for anyone" do + expect(described_class.authorized_for(resource_owner)).to be_empty + end - it "returns only application for a specific resource owner" do - FactoryBot.create(:access_token, resource_owner_id: resource_owner.id + 1) - token = FactoryBot.create(:access_token, resource_owner_id: resource_owner.id) - expect(Application.authorized_for(resource_owner)).to eq([token.application]) - end + it "returns only application for a specific resource owner" do + FactoryBot.create( + :access_token, + resource_owner_id: other_resource_owner.id, + resource_owner_type: other_resource_owner.class.name, + ) + token = FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + ) + expect(described_class.authorized_for(resource_owner)).to eq([token.application]) + end - it "excludes revoked tokens" do - FactoryBot.create(:access_token, resource_owner_id: resource_owner.id, revoked_at: 2.days.ago) - expect(Application.authorized_for(resource_owner)).to be_empty - end + it "excludes revoked tokens" do + FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + revoked_at: 2.days.ago, + ) + expect(described_class.authorized_for(resource_owner)).to be_empty + end - it "returns all applications that have been authorized" do - token1 = FactoryBot.create(:access_token, resource_owner_id: resource_owner.id) - token2 = FactoryBot.create(:access_token, resource_owner_id: resource_owner.id) - expect(Application.authorized_for(resource_owner)).to eq([token1.application, token2.application]) - end + it "returns all applications that have been authorized" do + token1 = FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + ) + token2 = FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + ) + expect(described_class.authorized_for(resource_owner)) + .to eq([token1.application, token2.application]) + end - it "returns only one application even if it has been authorized twice" do - application = FactoryBot.create(:application) - FactoryBot.create(:access_token, resource_owner_id: resource_owner.id, application: application) - FactoryBot.create(:access_token, resource_owner_id: resource_owner.id, application: application) - expect(Application.authorized_for(resource_owner)).to eq([application]) - end + it "returns only one application even if it has been authorized twice" do + application = FactoryBot.create(:application) + FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + application: application, + ) + FactoryBot.create( + :access_token, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + application: application, + ) + expect(described_class.authorized_for(resource_owner)).to eq([application]) end + end - describe "#revoke_tokens_and_grants_for" do - it "revokes all access tokens and access grants" do - application_id = 42 - resource_owner = double - expect(Doorkeeper::AccessToken) - .to receive(:revoke_all_for).with(application_id, resource_owner) - expect(Doorkeeper::AccessGrant) - .to receive(:revoke_all_for).with(application_id, resource_owner) + describe "#revoke_tokens_and_grants_for" do + it "revokes all access tokens and access grants" do + application_id = 42 + resource_owner = double + expect(Doorkeeper::AccessToken) + .to receive(:revoke_all_for).with(application_id, resource_owner) + expect(Doorkeeper::AccessGrant) + .to receive(:revoke_all_for).with(application_id, resource_owner) - Application.revoke_tokens_and_grants_for(application_id, resource_owner) - end + described_class.revoke_tokens_and_grants_for(application_id, resource_owner) end + end - describe "#by_uid_and_secret" do - context "when application is private/confidential" do - it "finds the application via uid/secret" do + describe "#by_uid_and_secret" do + context "when application is private/confidential" do + it "finds the application via uid/secret" do + app = FactoryBot.create :application + authenticated = described_class.by_uid_and_secret(app.uid, app.secret) + expect(authenticated).to eq(app) + end + context "when secret is wrong" do + it "should not find the application" do app = FactoryBot.create :application - authenticated = Application.by_uid_and_secret(app.uid, app.secret) - expect(authenticated).to eq(app) - end - context "when secret is wrong" do - it "should not find the application" do - app = FactoryBot.create :application - authenticated = Application.by_uid_and_secret(app.uid, "bad") - expect(authenticated).to eq(nil) - end + authenticated = described_class.by_uid_and_secret(app.uid, "bad") + expect(authenticated).to eq(nil) end end + end - context "when application is public/non-confidential" do - context "when secret is blank" do - it "should find the application" do - app = FactoryBot.create :application, confidential: false - authenticated = Application.by_uid_and_secret(app.uid, nil) - expect(authenticated).to eq(app) - end + context "when application is public/non-confidential" do + context "when secret is blank" do + it "should find the application" do + app = FactoryBot.create :application, confidential: false + authenticated = described_class.by_uid_and_secret(app.uid, nil) + expect(authenticated).to eq(app) end - context "when secret is wrong" do - it "should not find the application" do - app = FactoryBot.create :application, confidential: false - authenticated = Application.by_uid_and_secret(app.uid, "bad") - expect(authenticated).to eq(nil) - end + end + context "when secret is wrong" do + it "should not find the application" do + app = FactoryBot.create :application, confidential: false + authenticated = described_class.by_uid_and_secret(app.uid, "bad") + expect(authenticated).to eq(nil) end end end + end - describe "#confidential?" do - subject { FactoryBot.create(:application, confidential: confidential).confidential? } + describe "#confidential?" do + subject { FactoryBot.create(:application, confidential: confidential).confidential? } - context "when application is private/confidential" do - let(:confidential) { true } - it { expect(subject).to eq(true) } - end + context "when application is private/confidential" do + let(:confidential) { true } + it { expect(subject).to eq(true) } + end - context "when application is public/non-confidential" do - let(:confidential) { false } - it { expect(subject).to eq(false) } - end + context "when application is public/non-confidential" do + let(:confidential) { false } + it { expect(subject).to eq(false) } end + end - describe "#as_json" do - let(:app) { FactoryBot.create :application, secret: "123123123" } + describe "#as_json" do + let(:app) { FactoryBot.create :application, secret: "123123123" } - before do - allow(Doorkeeper.configuration) - .to receive(:application_secret_strategy).and_return(Doorkeeper::SecretStoring::Plain) - end + before do + allow(Doorkeeper.configuration) + .to receive(:application_secret_strategy).and_return(Doorkeeper::SecretStoring::Plain) + end - it "includes plaintext secret" do - expect(app.as_json).to include("secret" => "123123123") - end + it "includes plaintext secret" do + expect(app.as_json).to include("secret" => "123123123") + end - it "respects custom options" do - expect(app.as_json(except: :secret)).not_to include("secret") - expect(app.as_json(only: :id)).to match("id" => app.id) - end + it "respects custom options" do + expect(app.as_json(except: :secret)).not_to include("secret") + expect(app.as_json(only: :id)).to match("id" => app.id) + end - # AR specific - if DOORKEEPER_ORM == :active_record - it "correctly works with #to_json" do - ActiveRecord::Base.include_root_in_json = true - expect(app.to_json(include_root_in_json: true)).to match(/application.+?:\{/) - ActiveRecord::Base.include_root_in_json = false - end + # AR specific + if DOORKEEPER_ORM == :active_record + it "correctly works with #to_json" do + ActiveRecord::Base.include_root_in_json = true + expect(app.to_json(include_root_in_json: true)).to match(/application.+?:\{/) + ActiveRecord::Base.include_root_in_json = false end end end diff --git a/spec/requests/endpoints/token_spec.rb b/spec/requests/endpoints/token_spec.rb index a4cd053ee..806fd2e83 100644 --- a/spec/requests/endpoints/token_spec.rb +++ b/spec/requests/endpoints/token_spec.rb @@ -5,7 +5,11 @@ describe "Token endpoint" do before do client_exists - authorization_code_exists application: @client, scopes: "public" + create_resource_owner + authorization_code_exists application: @client, + scopes: "public", + resource_owner_id: @resource_owner.id, + resource_owner_type: @resource_owner.class.name end it "respond with correct headers" do diff --git a/spec/requests/flows/authorization_code_errors_spec.rb b/spec/requests/flows/authorization_code_errors_spec.rb index 5e4d4248b..1580ec44e 100644 --- a/spec/requests/flows/authorization_code_errors_spec.rb +++ b/spec/requests/flows/authorization_code_errors_spec.rb @@ -50,7 +50,10 @@ describe "Authorization Code Flow Errors", "after authorization" do before do client_exists - authorization_code_exists application: @client + create_resource_owner + authorization_code_exists application: @client, + resource_owner_id: @resource_owner.id, + resource_owner_type: @resource_owner.class.name end it "returns :invalid_grant error when posting an already revoked grant code" do diff --git a/spec/requests/flows/authorization_code_spec.rb b/spec/requests/flows/authorization_code_spec.rb index 0d5ee3200..a8ffd4f09 100644 --- a/spec/requests/flows/authorization_code_spec.rb +++ b/spec/requests/flows/authorization_code_spec.rb @@ -182,6 +182,7 @@ def authorize(redirect_url) access_token_exists application: @client, expires_in: -100, # even expired token resource_owner_id: @resource_owner.id, + resource_owner_type: @resource_owner.class.name, scopes: "public write" visit authorization_endpoint_url(client: @client, scope: "public write") @@ -506,8 +507,12 @@ def authorize(redirect_url) end context "issuing a refresh token" do + let(:resource_owner) { FactoryBot.create(:resource_owner) } + before do - authorization_code_exists application: @client + authorization_code_exists application: @client, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "second of simultaneous client requests get an error for revoked acccess token" do diff --git a/spec/requests/flows/refresh_token_spec.rb b/spec/requests/flows/refresh_token_spec.rb index 79548c50d..f989591e5 100644 --- a/spec/requests/flows/refresh_token_spec.rb +++ b/spec/requests/flows/refresh_token_spec.rb @@ -12,9 +12,13 @@ client_exists end + let(:resource_owner) { FactoryBot.create(:resource_owner) } + context "issuing a refresh token" do before do - authorization_code_exists application: @client + authorization_code_exists application: @client, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name end it "client gets the refresh token and refreshes it" do @@ -43,7 +47,8 @@ @token = FactoryBot.create( :access_token, application: @client, - resource_owner_id: 1, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end @@ -110,7 +115,8 @@ FactoryBot.create( :access_token, application: @client, - resource_owner_id: 1, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end @@ -119,7 +125,8 @@ FactoryBot.create( :access_token, application: public_client, - resource_owner_id: 1, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end @@ -185,14 +192,15 @@ end create_resource_owner _another_token = post password_token_endpoint_url( - client: @client, resource_owner: @resource_owner, + client: @client, resource_owner: resource_owner, ) - last_token.update_attribute :created_at, 5.seconds.ago + last_token.update(created_at: 5.seconds.ago) @token = FactoryBot.create( :access_token, application: @client, - resource_owner_id: @resource_owner.id, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) @token.update_attribute :expires_in, -100 @@ -226,7 +234,7 @@ def last_token Doorkeeper::AccessToken.last_authorized_token_for( - @client.id, @resource_owner.id, + @client.id, resource_owner, ) end end diff --git a/spec/requests/flows/revoke_token_spec.rb b/spec/requests/flows/revoke_token_spec.rb index a6ffcbc33..ba30a9b33 100644 --- a/spec/requests/flows/revoke_token_spec.rb +++ b/spec/requests/flows/revoke_token_spec.rb @@ -15,6 +15,7 @@ :access_token, application: client_application, resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end @@ -106,6 +107,7 @@ :access_token, application: nil, resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end @@ -130,6 +132,7 @@ :access_token, application: client_application, resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, use_refresh_token: true, ) end diff --git a/spec/support/helpers/access_token_request_helper.rb b/spec/support/helpers/access_token_request_helper.rb index 15af9db33..e78843747 100644 --- a/spec/support/helpers/access_token_request_helper.rb +++ b/spec/support/helpers/access_token_request_helper.rb @@ -5,6 +5,7 @@ def client_is_authorized(client, resource_owner, access_token_attributes = {}) attributes = { application: client, resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, }.merge(access_token_attributes) FactoryBot.create(:access_token, attributes) end diff --git a/spec/support/helpers/authorization_request_helper.rb b/spec/support/helpers/authorization_request_helper.rb index e4e888f7c..e8350f201 100644 --- a/spec/support/helpers/authorization_request_helper.rb +++ b/spec/support/helpers/authorization_request_helper.rb @@ -3,19 +3,19 @@ module AuthorizationRequestHelper def resource_owner_is_authenticated(resource_owner = nil) resource_owner ||= User.create!(name: "Joe", password: "sekret") - Doorkeeper.configuration.instance_variable_set(:@authenticate_resource_owner, proc { resource_owner }) + Doorkeeper.config.instance_variable_set(:@authenticate_resource_owner, proc { resource_owner }) end def resource_owner_is_not_authenticated - Doorkeeper.configuration.instance_variable_set(:@authenticate_resource_owner, proc { redirect_to("/sign_in") }) + Doorkeeper.config.instance_variable_set(:@authenticate_resource_owner, proc { redirect_to("/sign_in") }) end def default_scopes_exist(*scopes) - Doorkeeper.configuration.instance_variable_set(:@default_scopes, Doorkeeper::OAuth::Scopes.from_array(scopes)) + Doorkeeper.config.instance_variable_set(:@default_scopes, Doorkeeper::OAuth::Scopes.from_array(scopes)) end def optional_scopes_exist(*scopes) - Doorkeeper.configuration.instance_variable_set(:@optional_scopes, Doorkeeper::OAuth::Scopes.from_array(scopes)) + Doorkeeper.config.instance_variable_set(:@optional_scopes, Doorkeeper::OAuth::Scopes.from_array(scopes)) end def client_should_be_authorized(client) diff --git a/spec/support/helpers/config_helper.rb b/spec/support/helpers/config_helper.rb index 6c86e53be..5b4f0da77 100644 --- a/spec/support/helpers/config_helper.rb +++ b/spec/support/helpers/config_helper.rb @@ -4,7 +4,7 @@ module ConfigHelper def config_is_set(setting, value = nil, &block) setting_ivar = "@#{setting}" value = block_given? ? block : value - Doorkeeper.configuration.instance_variable_set(setting_ivar, value) + Doorkeeper.config.instance_variable_set(setting_ivar, value) end end diff --git a/spec/support/shared/controllers_shared_context.rb b/spec/support/shared/controllers_shared_context.rb index 3f9f8af98..be9ca0307 100644 --- a/spec/support/shared/controllers_shared_context.rb +++ b/spec/support/shared/controllers_shared_context.rb @@ -40,13 +40,13 @@ shared_context "authenticated resource owner" do before do user = double(:resource, id: 1) - allow(Doorkeeper.configuration).to receive(:authenticate_resource_owner) { proc { user } } + allow(Doorkeeper.config).to receive(:authenticate_resource_owner) { proc { user } } end end shared_context "not authenticated resource owner" do before do - allow(Doorkeeper.configuration).to receive(:authenticate_resource_owner) { proc { redirect_to "/" } } + allow(Doorkeeper.config).to receive(:authenticate_resource_owner) { proc { redirect_to "/" } } end end diff --git a/spec/support/shared/models_shared_examples.rb b/spec/support/shared/models_shared_examples.rb index 8e00d9a53..a2b77303c 100644 --- a/spec/support/shared/models_shared_examples.rb +++ b/spec/support/shared/models_shared_examples.rb @@ -31,20 +31,22 @@ shared_examples "a unique token" do describe :token do + let(:owner) { FactoryBot.create(:resource_owner) } + it "is generated before validation" do expect { subject.valid? }.to change { subject.token }.from(nil) end it "is not valid if token exists" do - token1 = FactoryBot.create factory_name - token2 = FactoryBot.create factory_name + token1 = FactoryBot.create factory_name, resource_owner_id: owner.id, resource_owner_type: owner.class.name + token2 = FactoryBot.create factory_name, resource_owner_id: owner.id, resource_owner_type: owner.class.name token2.token = token1.token expect(token2).not_to be_valid end it "expects database to throw an error when tokens are the same" do - token1 = FactoryBot.create factory_name - token2 = FactoryBot.create factory_name + token1 = FactoryBot.create factory_name, resource_owner_id: owner.id, resource_owner_type: owner.class.name + token2 = FactoryBot.create factory_name, resource_owner_id: owner.id, resource_owner_type: owner.class.name token2.token = token1.token expect do token2.save!(validate: false)