Fix issues where users can sometimes not see themselves from RBAC

app/models/miq_group.rb

@@ -62,10 +62,16 @@ def settings=(new_settings)
     super(indifferent_settings)
   end
 
-  def self.with_roles_excluding(identifier)
-    where.not(:id => MiqGroup.unscope(:select).joins(:miq_product_features)
-                             .where(:miq_product_features => {:identifier => identifier})
-                             .select(:id))
+  def self.with_roles_excluding(identifier, allowed_ids: nil)
+    scope = where.not(
+      :id => MiqGroup
+        .unscope(:select)
+        .joins(:miq_product_features)
+        .where(:miq_product_features => {:identifier => identifier})
+        .select(:id)
+    )
+    scope = scope.or(where(:id => allowed_ids)) if allowed_ids.present?
+    scope
   end
 
   def self.next_sequence

app/models/miq_user_role.rb

@@ -65,10 +65,16 @@ def limited_self_service?(klass = nil)
     (settings || {}).fetch_path(:restrictions, restriction_type(klass)) == :user
   end
 
-  def self.with_roles_excluding(identifier)
-    where.not(:id => MiqUserRole.unscope(:select).joins(:miq_product_features)
-                                .where(:miq_product_features => {:identifier => identifier})
-                                .select(:id))
+  def self.with_roles_excluding(identifier, allowed_ids: nil)
+    scope = where.not(
+      :id => MiqUserRole
+        .unscope(:select)
+        .joins(:miq_product_features)
+        .where(:miq_product_features => {:identifier => identifier})
+        .select(:id)
+    )
+    scope = scope.or(where(:id => allowed_ids)) if allowed_ids.present?
+    scope
   end
 
   def self.seed

app/models/user.rb

@@ -61,10 +61,16 @@ class User < ApplicationRecord
 
   scope :in_all_regions, ->(id) { where(:userid => User.default_scoped.where(:id => id).select(:userid)) }
 
-  def self.with_roles_excluding(identifier)
-    where.not(:id => User.unscope(:select).joins(:miq_groups => :miq_product_features)
-                             .where(:miq_product_features => {:identifier => identifier})
-                             .select(:id))
+  def self.with_roles_excluding(identifier, allowed_ids: nil)
+    scope = where.not(
+      :id => User
+        .unscope(:select)
+        .joins(:miq_groups => :miq_product_features)
+        .where(:miq_product_features => {:identifier => identifier})
+        .select(:id)
+    )
+    scope = scope.or(where(:id => allowed_ids)) if allowed_ids.present?
+    scope
   end
 
   def self.scope_by_tenant?

lib/rbac/filterer.rb

@@ -628,17 +628,34 @@ def scope_to_cloud_tenant(scope, user, miq_group)
     def scope_for_user_role_group(klass, scope, miq_group, user, managed_filters)
       user_or_group = miq_group || user
 
-      if user_or_group.try!(:self_service?) && MiqUserRole != klass
+      if user_or_group.try!(:self_service?) && klass != MiqUserRole
         scope.where(:id => klass == User ? user.id : miq_group.id)
       else
         role = user_or_group.miq_user_role
-        # hide creating admin group / roles from non-super administrators
+
+        # Exclude users/groups/roles tied to the super admin user if the current user isn't also
+        # a super admin user. This is to prevent creation / escalation to super admin.
         unless role&.super_admin_user?
-          scope = scope.with_roles_excluding(MiqProductFeature::SUPER_ADMIN_FEATURE)
+          # In the case that the user is not currently in a super admin group, but can _become_
+          #   a super admin by virtue of being in multiple groups, then they will be filtered by
+          #   with_roles_excluding. The allowed_ids option ensures the user can still see themselves.
+          #
+          # TODO: Determine if this same logic should apply to MiqUserRole as well
+          with_roles_excluding_options =
+            if klass == User
+              {:allowed_ids => user.id}
+            elsif klass == MiqGroup
+              {:allowed_ids => miq_group.id}
+            else
+              {}
+            end
+
+          scope = scope.with_roles_excluding(MiqProductFeature::SUPER_ADMIN_FEATURE, **with_roles_excluding_options)
         end
 
-        if MiqUserRole != klass
+        if klass != MiqUserRole
           filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, managed_filters))
+
           # Non tenant admins can only see their own groups. Note - a super admin is also a tenant admin
           scope = scope.with_groups(user.miq_group_ids) unless role&.tenant_admin_user?
         end

spec/lib/rbac/filterer_spec.rb

@@ -127,6 +127,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
   let(:default_tenant)     { Tenant.seed }
 
   let(:admin_user)         { FactoryBot.create(:user, :role => "super_administrator") }
+  let(:admin_group)        { admin_user.miq_groups.find_by(:description => "EvmGroup-super_administrator") }
 
   let(:owner_tenant)       { FactoryBot.create(:tenant) }
   let(:owner_group)        { FactoryBot.create(:miq_group, :tenant => owner_tenant) }
@@ -1286,56 +1287,142 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
         end
       end
 
-      it "returns users from current user's groups" do
-        other_user.miq_groups << group
-        get_rbac_results_for_and_expect_objects(User, [user, other_user])
-      end
+      context "with other users in the group" do
+        before do
+          other_user.miq_groups << group
+        end
+
+        it "returns users from current user's groups" do
+          get_rbac_results_for_and_expect_objects(User, [user, other_user])
+        end
 
-      it "returns user's groups" do
-        _expected_groups = [group, other_group] # this will create more groups than 2
-        get_rbac_results_for_and_expect_objects(MiqGroup, user.miq_groups)
+        it "returns user's groups" do
+          get_rbac_results_for_and_expect_objects(MiqGroup, [group])
+        end
       end
 
-      context "with self-service user" do
+      context "with a user in a shared group" do
+        let(:shared_group) { FactoryBot.create(:miq_group, :tenant => default_tenant) }
+
         before do
-          allow_any_instance_of(MiqGroup).to receive_messages(:self_service? => true)
+          other_user.miq_groups << shared_group
+          user.miq_groups << shared_group
         end
 
-        it "returns only the current user" do
-          get_rbac_results_for_and_expect_objects(User, [user])
+        context "and the current_group is the shared group" do
+          before { user.current_group = shared_group }
+
+          it "returns users from all of the current user's groups" do
+            get_rbac_results_for_and_expect_objects(User, [user, other_user])
+          end
+
+          it "returns user's groups" do
+            get_rbac_results_for_and_expect_objects(MiqGroup, [group, shared_group])
+          end
         end
 
-        it "returns only the current group" do
-          get_rbac_results_for_and_expect_objects(MiqGroup, [user.current_group])
+        context "and the user's current_group is not the shared group" do
+          before { user.current_group = group }
+
+          it "returns users from all of the current user's groups" do
+            get_rbac_results_for_and_expect_objects(User, [user, other_user])
+          end
+
+          it "returns user's groups" do
+            get_rbac_results_for_and_expect_objects(MiqGroup, [group, shared_group])
+          end
         end
       end
 
-      context 'with EvmRole-tenant_administrator' do
-        let(:rbac_tenant) do
-          FactoryBot.create(:miq_product_feature, :identifier => MiqProductFeature::TENANT_ADMIN_FEATURE)
+      context "with a user in a super_administrator group" do
+        before do
+          admin_user # ensure we create another user as an admin user
+          other_user # ensure we create another user in another regular group
+
+          user.miq_groups << admin_group
+          user.miq_groups << other_group
         end
 
-        let(:tenant_administrator_user_role) do
-          FactoryBot.create(:miq_user_role, :name => MiqUserRole::DEFAULT_TENANT_ROLE_NAME, :miq_product_features => [rbac_tenant])
+        context "and the current_group is the super_administrator group" do
+          before { user.current_group = admin_group }
+
+          it "returns all users" do
+            get_rbac_results_for_and_expect_objects(User, User.all.to_a)
+          end
+
+          it "returns user's groups" do
+            get_rbac_results_for_and_expect_objects(MiqGroup, MiqGroup.all.to_a)
+          end
+        end
+
+        context "and the current_group is not the super_administrator group" do
+          before { user.current_group = group }
+
+          it "returns users from all of the current user's groups except admins" do
+            get_rbac_results_for_and_expect_objects(User, [user, other_user])
+          end
+
+          it "returns user's groups except admins" do
+            get_rbac_results_for_and_expect_objects(MiqGroup, [group, other_group])
+          end
         end
+      end
+
+      context "with a self_service user" do
+        let(:user)  { FactoryBot.create(:user, :role => "user_self_service") }
+        let(:group) { user.miq_groups.detect { |g| g.description == "EvmGroup-user_self_service" } }
 
-        let!(:super_administrator_user_role) do
-          FactoryBot.create(:miq_user_role, :role => "super_administrator")
+        before do
+          other_user # ensure we create another user in another group
         end
 
-        let(:group) do
-          FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => tenant_administrator_user_role)
+        it "returns only the current user" do
+          get_rbac_results_for_and_expect_objects(User, [user])
         end
 
-        let!(:user_role) do
-          FactoryBot.create(:miq_user_role, :role => "user")
+        it "returns only the current group" do
+          get_rbac_results_for_and_expect_objects(MiqGroup, [group])
         end
 
-        let!(:other_group) do
-          FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => user_role)
+        context "that is in multiple groups" do
+          before { user.miq_groups << other_group }
+
+          context "and the current_group is the self_service group" do
+            before { user.current_group = group }
+
+            it "returns only the current user" do
+              get_rbac_results_for_and_expect_objects(User, [user])
+            end
+
+            it "returns only the current group" do
+              get_rbac_results_for_and_expect_objects(MiqGroup, [group])
+            end
+          end
+
+          context "and the current_group is not the self_service group" do
+            before { user.current_group = other_group }
+
+            it "returns users from all of the current user's groups" do
+              get_rbac_results_for_and_expect_objects(User, [user, other_user])
+            end
+
+            it "returns user's groups" do
+              get_rbac_results_for_and_expect_objects(MiqGroup, [other_group])
+            end
+          end
         end
+      end
 
+      context 'with EvmRole-tenant_administrator' do
+        let(:rbac_tenant) { FactoryBot.create(:miq_product_feature, :identifier => MiqProductFeature::TENANT_ADMIN_FEATURE) }
+        let(:tenant_administrator_user_role) { FactoryBot.create(:miq_user_role, :name => MiqUserRole::DEFAULT_TENANT_ROLE_NAME, :miq_product_features => [rbac_tenant]) }
+        let!(:super_administrator_user_role) { FactoryBot.create(:miq_user_role, :role => "super_administrator") }
+        let(:group) { FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => tenant_administrator_user_role) }
+        let!(:user_role) { FactoryBot.create(:miq_user_role, :role => "user") }
+        let!(:other_group) { FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => user_role) }
         let!(:user) { FactoryBot.create(:user, :miq_groups => [group]) }
+        let(:super_admin_group) { FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => super_administrator_user_role) }
+        let!(:super_admin_user) { FactoryBot.create(:user, :miq_groups => [super_admin_group]) }
 
         it 'can see all roles except for EvmRole-super_administrator' do
           expect(MiqUserRole.count).to eq(3)
@@ -1358,12 +1445,6 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
           get_rbac_results_for_and_expect_objects(MiqGroup, [another_tenant_group, default_group_for_tenant])
         end
 
-        let(:super_admin_group) do
-          FactoryBot.create(:miq_group, :tenant => default_tenant, :miq_user_role => super_administrator_user_role)
-        end
-
-        let!(:super_admin_user) { FactoryBot.create(:user, :miq_groups => [super_admin_group]) }
-
         it 'can see all users except for user with group with role EvmRole-super_administrator' do
           expect(User.count).to eq(2)
           get_rbac_results_for_and_expect_objects(User, [user])