Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy evaluate Doorkeeper config when loading files and executing initializers #1627

Merged
merged 3 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 0 additions & 26 deletions Appraisals

This file was deleted.

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ User-visible changes worth mentioning.
- [#1626] Remove deprecated `active_record_options` config option.
- [#1631] Fix regression with redirect behavior after token lookup optimizations (redirect to app URI when found).
- [#1630] Special case unique index creation for refresh_token on SQL Server.
- [#1627] Lazy evaluate Doorkeeper config when loading files and executing initializers.

## 5.6.2

Expand Down
3 changes: 0 additions & 3 deletions bin/console
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,5 @@ ActiveRecord::Base.establish_connection(
# Load database schema
load File.expand_path("../spec/dummy/db/schema.rb", __dir__)

# Call engine #to_prepare block
Doorkeeper.setup

require "irb"
IRB.start(__FILE__)
77 changes: 72 additions & 5 deletions lib/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ module Models
autoload :Expirable, "doorkeeper/models/concerns/expirable"
autoload :ExpirationTimeSqlMath, "doorkeeper/models/concerns/expiration_time_sql_math"
autoload :Orderable, "doorkeeper/models/concerns/orderable"
autoload :PolymorphicResourceOwner, "doorkeeper/models/concerns/polymorphic_resource_owner"
autoload :Scopes, "doorkeeper/models/concerns/scopes"
autoload :Reusable, "doorkeeper/models/concerns/reusable"
autoload :ResourceOwnerable, "doorkeeper/models/concerns/resource_ownerable"
Expand All @@ -113,11 +114,77 @@ module SecretStoring
autoload :BCrypt, "doorkeeper/secret_storing/bcrypt"
end

def self.authenticate(request, methods = Doorkeeper.config.access_token_methods)
OAuth::Token.authenticate(request, *methods)
end
class << self
attr_reader :orm_adapter

def configure(&block)
@config = Config::Builder.new(&block).build
setup
@config
end

# @return [Doorkeeper::Config] configuration instance
#
def configuration
@config || configure
end

def configured?
!@config.nil?
end

alias config configuration

def setup
setup_orm_adapter
run_orm_hooks
config.clear_cache!

# Deprecated, will be removed soon
unless configuration.orm == :active_record
setup_orm_models
setup_application_owner
end
end

def setup_orm_adapter
@orm_adapter = "doorkeeper/orm/#{configuration.orm}".classify.constantize
rescue NameError => e
raise e, "ORM adapter not found (#{configuration.orm})", <<-ERROR_MSG.strip_heredoc
[DOORKEEPER] ORM adapter not found (#{configuration.orm}), or there was an error
trying to load it.

def self.gem_version
::Gem::Version.new(::Doorkeeper::VERSION::STRING)
You probably need to add the related gem for this adapter to work with
doorkeeper.
ERROR_MSG
end

def run_orm_hooks
if @orm_adapter.respond_to?(:run_hooks)
@orm_adapter.run_hooks
else
::Kernel.warn <<~MSG.strip_heredoc
[DOORKEEPER] ORM "#{configuration.orm}" should move all it's setup logic under `#run_hooks` method for
the #{@orm_adapter.name}. Later versions of Doorkeeper will no longer support `setup_orm_models` and
`setup_application_owner` API.
MSG
end
end

def setup_orm_models
@orm_adapter.initialize_models!
end

def setup_application_owner
@orm_adapter.initialize_application_owner!
end

def authenticate(request, methods = Doorkeeper.config.access_token_methods)
OAuth::Token.authenticate(request, *methods)
end

def gem_version
::Gem::Version.new(::Doorkeeper::VERSION::STRING)
end
end
end
70 changes: 0 additions & 70 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,81 +5,11 @@
require "doorkeeper/config/validations"

module Doorkeeper
# Defines a MissingConfiguration error for a missing Doorkeeper configuration
#
class MissingConfiguration < StandardError
def initialize
super("Configuration for doorkeeper missing. Do you have doorkeeper initializer?")
end
end

# Doorkeeper option DSL could be reused in extensions to build their own
# configurations. To use the Option DSL gems need to define `builder_class` method
# that returns configuration Builder class. This exception raises when they don't
# define it.
#
class MissingConfigurationBuilderClass < StandardError; end

class << self
attr_reader :orm_adapter

def configure(&block)
@config = Config::Builder.new(&block).build
end

# @return [Doorkeeper::Config] configuration instance
#
def configuration
@config || (raise MissingConfiguration)
end

alias config configuration

def setup
setup_orm_adapter
run_orm_hooks
config.clear_cache!

# Deprecated, will be removed soon
unless configuration.orm == :active_record
setup_orm_models
setup_application_owner
end
end

def setup_orm_adapter
@orm_adapter = "doorkeeper/orm/#{configuration.orm}".classify.constantize
rescue NameError => e
raise e, "ORM adapter not found (#{configuration.orm})", <<-ERROR_MSG.strip_heredoc
[DOORKEEPER] ORM adapter not found (#{configuration.orm}), or there was an error
trying to load it.

You probably need to add the related gem for this adapter to work with
doorkeeper.
ERROR_MSG
end

def run_orm_hooks
if @orm_adapter.respond_to?(:run_hooks)
@orm_adapter.run_hooks
else
::Kernel.warn <<~MSG.strip_heredoc
[DOORKEEPER] ORM "#{configuration.orm}" should move all it's setup logic under `#run_hooks` method for
the #{@orm_adapter.name}. Later versions of Doorkeeper will no longer support `setup_orm_models` and
`setup_application_owner` API.
MSG
end
end

def setup_orm_models
@orm_adapter.initialize_models!
end

def setup_application_owner
@orm_adapter.initialize_application_owner!
end
end

class Config
# Default Doorkeeper configuration builder
class Builder < AbstractBuilder
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/config/abstract_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class AbstractBuilder
#
def initialize(config = Config.new, &block)
@config = config
instance_eval(&block)
instance_eval(&block) if block_given?
end

# Builds and validates configuration.
Expand Down
10 changes: 6 additions & 4 deletions lib/doorkeeper/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

module Doorkeeper
class Engine < Rails::Engine
initializer "doorkeeper.params.filter" do |app|
parameters = %w[client_secret authentication_token access_token refresh_token]
parameters << "code" if Doorkeeper.config.grant_flows.include?("authorization_code")
app.config.filter_parameters << /^(#{Regexp.union(parameters)})$/
initializer "doorkeeper.params.filter", after: :load_config_initializers do |app|
if Doorkeeper.configured?
parameters = %w[client_secret authentication_token access_token refresh_token]
parameters << "code" if Doorkeeper.config.grant_flows.include?("authorization_code")
app.config.filter_parameters << /^(#{Regexp.union(parameters)})$/
end
end

initializer "doorkeeper.routes" do
Expand Down
30 changes: 30 additions & 0 deletions lib/doorkeeper/models/concerns/polymorphic_resource_owner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true

module Doorkeeper
module Models
module PolymorphicResourceOwner
module ForAccessGrant
extend ActiveSupport::Concern

included do
if Doorkeeper.config.polymorphic_resource_owner?
belongs_to :resource_owner, polymorphic: true, optional: false
else
validates :resource_owner_id, presence: true
end
end
end

module ForAccessToken
extend ActiveSupport::Concern

included do
if Doorkeeper.config.polymorphic_resource_owner?
belongs_to :resource_owner, polymorphic: true, optional: true
end
end
end
end
end
end

11 changes: 10 additions & 1 deletion lib/doorkeeper/orm/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ module Mixins
end

def self.run_hooks
# nop
initialize_configured_associations
end

def self.initialize_configured_associations
if Doorkeeper.config.enable_application_owner?
Doorkeeper.config.application_model.include ::Doorkeeper::Models::Ownership
end

Doorkeeper.config.access_grant_model.include ::Doorkeeper::Models::PolymorphicResourceOwner::ForAccessGrant
Doorkeeper.config.access_token_model.include ::Doorkeeper::Models::PolymorphicResourceOwner::ForAccessToken
end
end
end
Expand Down
6 changes: 0 additions & 6 deletions lib/doorkeeper/orm/active_record/mixins/access_grant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ module AccessGrant
optional: true,
inverse_of: :access_grants

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,
Expand Down
4 changes: 0 additions & 4 deletions lib/doorkeeper/orm/active_record/mixins/access_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ 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?

Expand Down
4 changes: 0 additions & 4 deletions lib/doorkeeper/orm/active_record/mixins/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ module Application

include ::Doorkeeper::ApplicationMixin

if Doorkeeper.config.enable_application_owner?
include ::Doorkeeper::Models::Ownership
end

has_many :access_grants,
foreign_key: :application_id,
dependent: :delete_all,
Expand Down
7 changes: 6 additions & 1 deletion lib/doorkeeper/rails/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def generate_routes!(options)
map_route(:authorizations, :authorization_routes)
map_route(:tokens, :token_routes)
map_route(:tokens, :revoke_routes)
map_route(:tokens, :introspect_routes) unless Doorkeeper.config.allow_token_introspection.is_a?(FalseClass)
map_route(:tokens, :introspect_routes) if introspection_routes?
map_route(:applications, :application_routes)
map_route(:authorized_applications, :authorized_applications_routes)
map_route(:token_info, :token_info_routes)
Expand Down Expand Up @@ -100,6 +100,11 @@ def authorized_applications_routes(mapping)
def native_authorization_code_route
Doorkeeper.configuration.native_authorization_code_route
end

def introspection_routes?
Doorkeeper.configured? &&
!Doorkeeper.config.allow_token_introspection.is_a?(FalseClass)
end
end
end
end
2 changes: 0 additions & 2 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
factory :application, class: "Doorkeeper::Application" do
sequence(:name) { |n| "Application #{n}" }
redirect_uri { "https://app.com/callback" }

factory :application_with_owner, class: "ApplicationWithOwner"
end

# do not name this factory :user, otherwise it will conflict with factories
Expand Down