diff --git a/app/models/miq_group.rb b/app/models/miq_group.rb index 035cf7a9a5cb..0d59b3ccbc0c 100644 --- a/app/models/miq_group.rb +++ b/app/models/miq_group.rb @@ -217,7 +217,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? diff --git a/app/models/miq_user_role.rb b/app/models/miq_user_role.rb index 27aba92dadd9..0f95219fbaa0 100644 --- a/app/models/miq_user_role.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 621ef9a05ab3..a3b4af5115de 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -31,7 +31,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 @@ -264,7 +264,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.seed diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index d689d8a82b75..5050f9af6aa6 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -531,7 +531,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) diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index b92e6abdcac1..9159397fb430 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -695,14 +695,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 @@ -711,7 +734,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 @@ -741,7 +764,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 diff --git a/spec/models/miq_user_role_spec.rb b/spec/models/miq_user_role_spec.rb index e4e14b34dcc2..b20599387842 100644 --- a/spec/models/miq_user_role_spec.rb +++ b/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") }