From dad746d31fe01434865dd1aaf0e8db1c9f716669 Mon Sep 17 00:00:00 2001
From: Jason Frey <jfrey@redhat.com>
Date: Tue, 23 Jan 2018 13:46:49 -0500
Subject: [PATCH] Refactor Authenticator to remove const manipulations

After https://github.com/ManageIQ/manageiq/pull/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
---
 app/models/authenticator.rb      | 39 +++++++++++++++++---------------
 app/models/authenticator/base.rb |  4 ++++
 lib/vmdb/config/validator.rb     | 18 ++-------------
 3 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/app/models/authenticator.rb b/app/models/authenticator.rb
index d9d28513600..b7e432a2efe 100644
--- a/app/models/authenticator.rb
+++ b/app/models/authenticator.rb
@@ -1,35 +1,38 @@
 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_class(config[:mode])&.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
+      authenticator.validate_config
+    else
+      [:mode, "authentication type, #{config[:mode].inspect}, invalid. Should be one of: #{valid_modes.join(", ")}"]
     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
diff --git a/app/models/authenticator/base.rb b/app/models/authenticator/base.rb
index 7958c8cfea9..5ce64ac33e8 100644
--- a/app/models/authenticator/base.rb
+++ b/app/models/authenticator/base.rb
@@ -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
diff --git a/lib/vmdb/config/validator.rb b/lib/vmdb/config/validator.rb
index f2c77c44199..7f09f932f58 100644
--- a/lib/vmdb/config/validator.rb
+++ b/lib/vmdb/config/validator.rb
@@ -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