Skip to content

Commit

Permalink
Allow tenant admins to see all groups within the scope of their tenant
Browse files Browse the repository at this point in the history
Manual back port of ManageIQ#17768 to gaprindashvili
This had to be done because of the change in master to rely on product features instead of role name
to determine whether a user is an admin, tenant admin or super admin

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1613387
  • Loading branch information
gtanzillo committed Aug 8, 2018
1 parent a63ea70 commit c1c047a
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 15 deletions.
2 changes: 1 addition & 1 deletion app/models/miq_group.rb
Expand Up @@ -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?
Expand Down
10 changes: 7 additions & 3 deletions 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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/models/user.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion lib/rbac/filterer.rb
Expand Up @@ -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)
Expand Down
39 changes: 31 additions & 8 deletions spec/lib/rbac/filterer_spec.rb
Expand Up @@ -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 role EvmRole-super_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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions spec/models/miq_user_role_spec.rb
Expand Up @@ -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") }
Expand Down

0 comments on commit c1c047a

Please sign in to comment.