Skip to content

Commit

Permalink
Refactor Authenticator to remove const manipulations
Browse files Browse the repository at this point in the history
After #16867, the
DescendantLoader now works properly with non-AR models such as the
Authenticator.  Much of the code in the Authenticator, such as
require_nested and force_load_authenticator_for is just workarounds
over the issues from DescendantLoader, so these can be removed.
Additionally, the only caller of the authenticator_class was the
validator, so this commit moves the validation code into the
Authenticator class directly, allow us to make all of those
authenticator_class methods private.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1537299
  • Loading branch information
Fryguy committed Jan 23, 2018
1 parent 104416d commit ced98f2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 34 deletions.
40 changes: 22 additions & 18 deletions app/models/authenticator.rb
Original file line number Diff line number Diff line change
@@ -1,35 +1,39 @@
module Authenticator
require_nested :Base
require_nested :Database
require_nested :Httpd
require_nested :Ldap

def self.for(config, username = nil)
if username == 'admin'
Database.new(config)
else
authenticator_class(config[:mode]).new(config)
authenticator = authenticator_class(config[:mode])
authenticator && authenticator.new(config)
end
end

def self.authenticator_class(name)
authenticator_classes_cache[name.to_s]
end
def self.validate_config(config)
return [] if config[:mode] == "none"

def self.authenticator_classes_cache
@authenticator_classes_cache ||= Concurrent::Hash.new do |hash, name|
hash[name] = authenticator_class_for(name) || force_load_authenticator_for(name)
authenticator = self.for(config)
if authenticator
[:mode, "authentication type, #{config[:mode].inspect}, invalid. Should be one of: #{valid_modes.join(", ")}"]
else
authenticator.validate_config
end
end

def self.force_load_authenticator_for(name)
# try to constantize in case the subclass is not loaded yet, but it still exists
# this is happens with authenticators from provider gems
subclass = "Authenticator::#{name.camelize}".safe_constantize
subclass if subclass && subclass.authenticates_for.include?(name)
private_class_method def self.valid_modes
Base.subclasses.flat_map(&:authenticates_for).uniq << "none"
end

private_class_method def self.authenticator_class(name)
authenticator_classes_cache[name.to_s]
end

private_class_method def self.authenticator_classes_cache
@authenticator_classes_cache ||= Hash.new do |hash, name|
hash[name] = authenticator_class_for(name)
end
end

def self.authenticator_class_for(name)
private_class_method def self.authenticator_class_for(name)
Base.subclasses.find do |subclass|
subclass.authenticates_for.include?(name)
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/authenticator/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ def initialize(config)
@config = config
end

def validate_config
self.class.validate_config(config)
end

def uses_stored_password?
false
end
Expand Down
18 changes: 2 additions & 16 deletions lib/vmdb/config/validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,9 @@ def webservices(data)
return valid, errors
end

AUTH_TYPES = %w(ldap ldaps httpd amazon database none)

def authentication(data)
valid, errors = true, []

return valid, errors if data.mode == 'none'

authenticator = Authenticator.authenticator_class(data.mode)

if authenticator
errors = authenticator.validate_config(data)
valid = errors.empty?
else
valid = false
errors << [:mode, "authentication type, \"#{data.mode}\", invalid. Should be one of: #{Authenticator::Base.subclasses.map(&:authenticates_for).join(", ")}"]
end

errors = Authenticator.validate_config(data)
valid = errors.empty?
return valid, errors
end

Expand Down

0 comments on commit ced98f2

Please sign in to comment.