[GAPRINDASHVILI] Allow tenant admins to see all groups within the scope of their tenant

app/models/miq_group.rb

@@ -251,7 +251,7 @@ def self.non_tenant_groups_in_my_region
 
   def self.with_current_user_groups(user = nil)
     current_user = user || User.current_user
-    current_user.admin_user? ? all : where(:id => current_user.miq_group_ids)
+    current_user.tenant_admin_user? ? all : where(:id => current_user.miq_group_ids)
   end
 
   def single_group_users?

app/models/miq_user_role.rb

@@ -1,7 +1,7 @@
 class MiqUserRole < ApplicationRecord
-  SUPER_ADMIN_ROLE_NAME = "EvmRole-super_administrator"
-  ADMIN_ROLE_NAME       = "EvmRole-administrator"
-  DEFAULT_TENANT_ROLE_NAME = "EvmRole-tenant_administrator"
+  SUPER_ADMIN_ROLE_NAME    = "EvmRole-super_administrator".freeze
+  ADMIN_ROLE_NAME          = "EvmRole-administrator".freeze
+  DEFAULT_TENANT_ROLE_NAME = "EvmRole-tenant_administrator".freeze
 
   has_many                :entitlements, :dependent => :restrict_with_exception
   has_many                :miq_groups, :through => :entitlements
@@ -112,6 +112,10 @@ def admin_user?
     name == SUPER_ADMIN_ROLE_NAME || name == ADMIN_ROLE_NAME
   end
 
+  def tenant_admin_user?
+    name == SUPER_ADMIN_ROLE_NAME || name == ADMIN_ROLE_NAME || name == DEFAULT_TENANT_ROLE_NAME
+  end
+
   def self.default_tenant_role
     find_by(:name => DEFAULT_TENANT_ROLE_NAME)
   end

app/models/user.rb

@@ -32,7 +32,7 @@ class User < ApplicationRecord
 
   delegate   :miq_user_role, :current_tenant, :get_filters, :has_filters?, :get_managed_filters, :get_belongsto_filters,
              :to => :current_group, :allow_nil => true
-  delegate   :super_admin_user?, :admin_user?, :self_service?, :limited_self_service?, :disallowed_roles,
+  delegate   :super_admin_user?, :admin_user?, :tenant_admin_user?, :self_service?, :limited_self_service?, :disallowed_roles,
              :to => :miq_user_role, :allow_nil => true
 
   validates_presence_of   :name, :userid
@@ -285,7 +285,7 @@ def self.current_user
 
   def self.with_current_user_groups(user = nil)
     user ||= current_user
-    user.admin_user? ? all : includes(:miq_groups).where(:miq_groups => {:id => user.miq_group_ids})
+    user.tenant_admin_user? ? all : includes(:miq_groups).where(:miq_groups => {:id => user.miq_group_ids})
   end
 
   def self.missing_user_features(db_user)

lib/rbac/filterer.rb

@@ -519,7 +519,8 @@ def scope_for_user_role_group(klass, scope, miq_group, user, managed_filters)
 
         if MiqUserRole != klass
           filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, managed_filters))
-          scope = scope.with_current_user_groups(user)
+          # Non tenant admins can only see their own groups. Note - a super admin is also a tenant admin
+          scope = scope.with_current_user_groups(user) unless user.tenant_admin_user?
         end
 
         scope_by_ids(scope, filtered_ids)

spec/lib/rbac/filterer_spec.rb

@@ -937,14 +937,37 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
 
         let!(:user) { FactoryGirl.create(:user, :miq_groups => [group]) }
 
-        it 'can see all roles expect to EvmRole-super_administrator' do
-          expect(MiqUserRole.count).to eq(3)
-          get_rbac_results_for_and_expect_objects(MiqUserRole, [tenant_administrator_user_role])
+        let!(:user_role) do
+          FactoryGirl.create(:miq_user_role, :role => "user")
         end
 
-        it 'can see all groups expect to group with role EvmRole-super_administrator' do
-          expect(MiqUserRole.count).to eq(3)
-          get_rbac_results_for_and_expect_objects(MiqGroup, [group])
+        let!(:other_group) do
+          FactoryGirl.create(:miq_group, :tenant => default_tenant, :miq_user_role => user_role)
+        end
+
+        it 'can see all roles except for EvmRole-super_administrator' do
+          expect(MiqUserRole.count).to eq(4)
+          get_rbac_results_for_and_expect_objects(MiqUserRole, [tenant_administrator_user_role, user_role])
+        end
+
+        it 'can see all groups except for group with roles EvmRole-super_administrator amd EvmRole-administrator' do
+          expect(MiqUserRole.count).to eq(4)
+          default_group_for_tenant = user.current_tenant.miq_groups.where(:group_type => "tenant").first
+          default_group_for_tenant.miq_user_role = MiqUserRole.default_tenant_role
+          default_group_for_tenant.save!
+          super_admin_group
+          get_rbac_results_for_and_expect_objects(MiqGroup, [group, other_group, default_group_for_tenant])
+        end
+
+        it 'can see all groups in the current tenant only' do
+          another_tenant = FactoryGirl.create(:tenant)
+          another_tenant_group = FactoryGirl.create(:miq_group, :miq_user_role => user_role, :tenant => another_tenant)
+          group.tenant = another_tenant
+
+          default_group_for_tenant = user.current_tenant.miq_groups.where(:group_type => "tenant").first
+          default_group_for_tenant.miq_user_role = MiqUserRole.default_tenant_role
+          default_group_for_tenant.save!
+          get_rbac_results_for_and_expect_objects(MiqGroup, [another_tenant_group, default_group_for_tenant])
         end
 
         let(:super_admin_group) do
@@ -953,7 +976,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
 
         let!(:super_admin_user) { FactoryGirl.create(:user, :miq_groups => [super_admin_group]) }
 
-        it 'can see all users expect to user with group with role EvmRole-super_administrator' do
+        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])
         end
@@ -983,7 +1006,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
               h.metric_rollups << FactoryGirl.create(:metric_rollup_host_hr,
                                                      :timestamp                  => t,
                                                      :cpu_usage_rate_average     => v,
-                                                     :cpu_ready_delta_summation  => v * 1000, # Multiply by a factor of 1000 to maake it more realistic and enable testing virtual col v_pct_cpu_ready_delta_summation
+                                                     :cpu_ready_delta_summation  => v * 1000, # Multiply by a factor of 1000 to make it more realistic and enable testing virtual col v_pct_cpu_ready_delta_summation
                                                      :sys_uptime_absolute_latest => v
                                                     )
             end

spec/models/miq_user_role_spec.rb

@@ -200,6 +200,24 @@
     end
   end
 
+  describe "#tenant_admin" do
+    it "detects tenant_admin" do
+      expect(FactoryGirl.build(:miq_user_role, :role => "tenant_administrator")).to be_tenant_admin_user
+    end
+
+    it "detects admin" do
+      expect(FactoryGirl.build(:miq_user_role, :role => "administrator")).to be_tenant_admin_user
+    end
+
+    it "detects super_admin" do
+      expect(FactoryGirl.build(:miq_user_role, :role => "super_administrator")).to be_tenant_admin_user
+    end
+
+    it "does not detect regular role" do
+      expect(FactoryGirl.build(:miq_user_role)).not_to be_tenant_admin_user
+    end
+  end
+
   describe "#destroy" do
     subject { miq_group.entitlement.miq_user_role }
     let!(:miq_group) { FactoryGirl.create(:miq_group, :role => "EvmRole-administrator") }