Merge pull request #20494 from NickLaMuro/fix-authentication-status-v…

…irtual-delegate

[AuthenticationMixin] Fix .authentication_status

(cherry picked from commit a1ee4dd)

app/models/mixins/authentication_mixin.rb

@@ -2,10 +2,59 @@ module AuthenticationMixin
   extend ActiveSupport::Concern
 
   included do
+    # There are some dirty, dirty Ruby/Rails reasons why this method needs to
+    # exist like this, and I will try and explain:
+    #
+    # First, we need the extra +.where+ here because if this is used in a
+    # nested SELECT statement (aka: virtual_delegate), the +resource_type+
+    # check is dropped on the floor and not included in the subquery.  As a
+    # result, it will just pickup the first record to match the
+    # relationship_id column and that could be associated with any OTHER object
+    # with an Authentication record.
+    #
+    # For example, you get back an Authentication record for an EMS record when
+    # you are looking up one for a Host.
+    #
+    # Secondly, the reason this method exists, and is FIRST (prior to the
+    # +has_one+ that uses it) is it needs to be defined prior to the
+    # ActiveRecord::Relation method that calls it.  Also, it needs to be a
+    # method so the proper local variable, +authentication_mixin_relation+, can
+    # be defined with the class that the +resource_type+ needs to match and
+    # remain in scope for the Proc below.  If it is a class variable or done in
+    # other fashion, it will be overwritten whenever this module is included
+    # elsewhere, or is called against ActiveRecord::Relation, and not the class
+    # we are mixing into.
+    #
+    # Finally, the use of a prepared statement is also for some reason required
+    # over the Hash syntax since otherwise the following is ERROR is produced
+    # when doing a nested SELECT:
+    #
+    #     PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters,
+    #     but prepared statement "" requires 1 (ActiveRecord::StatementInvalid)
+    #
+    # FIXME:  If we handle this in `virtual_attributes`, then this can be
+    # deleted and returned to the following proc on the has one:
+    #
+    #     has_one :authentication_status_severity_level,
+    #             -> { order(Authentication::STATUS_SEVERITY_AREL.desc) }
+    #             # ...
+    #
+    # But keep the test that was added ;)
+    #
+    def self.authentication_status_severity_level_filter
+      # required to be done here so it is in scope of the Proc below
+      authentication_mixin_relation = name
+
+      proc do
+        where('"authentications"."resource_type" = ?', authentication_mixin_relation)
+          .order(Authentication::STATUS_SEVERITY_AREL.desc)
+      end
+    end
+
     has_many :authentications, :as => :resource, :dependent => :destroy, :autosave => true
 
     has_one  :authentication_status_severity_level,
-             -> { order(Authentication::STATUS_SEVERITY_AREL.desc) },
+             authentication_status_severity_level_filter,
              :as         => :resource,
              :inverse_of => :resource,
              :class_name => "Authentication"

spec/models/mixins/authentication_mixin_spec.rb

@@ -785,6 +785,28 @@ def self.name; "TestClass"; end
       end
     end
 
+    context "with an unrelated and existing valid authentication" do
+      let(:id_start_point)        { [Host.order(:id).last&.id.to_i, ExtManagementSystem.order(:id).last&.id.to_i].max }
+      let(:host)                  { FactoryBot.create(:host, :id => ext_management_system.id) }
+      let(:ext_management_system) { FactoryBot.create(:ext_management_system, :authtype => "default", :id => id_start_point + 1) }
+
+      before { host }
+
+      it "is nil with sql" do
+        expect(virtual_column_sql_value(Host, "authentication_status")).to be_nil
+      end
+
+      it "is 'None' with pure ruby (via relations)" do
+        expect(host.authentication_status).to eq("None")
+      end
+
+      it "is 'None' when accessed via ruby, but fetched via sql" do
+        fetched_host = Host.select(:id, :authentication_status).first
+        expect(fetched_host.attributes[:authentication_status]).to be_nil
+        expect(fetched_host.authentication_status).to eq("None")
+      end
+    end
+
     context "with a valid authentication" do
       before { valid_auth }