Skip to content

Commit

Permalink
separate report and request admins
Browse files Browse the repository at this point in the history
Admins have 2 types of escalated privileges
report and request

This separates the types (with 2 different product features)
  • Loading branch information
kbrock committed Aug 14, 2018
1 parent 1b470b5 commit 3a5bba6
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 22 deletions.
4 changes: 3 additions & 1 deletion app/models/miq_product_feature.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class MiqProductFeature < ApplicationRecord
SUPER_ADMIN_FEATURE = "everything".freeze
ADMIN_FEATURE = "miq_report_superadmin".freeze
REPORT_ADMIN_FEATURE = "miq_report_superadmin".freeze
REQUEST_ADMIN_FEATURE = "miq_request_superadmin".freeze
ADMIN_FEATURE = REPORT_ADMIN_FEATURE
acts_as_tree

has_and_belongs_to_many :miq_user_roles, :join_table => :miq_roles_features
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class MiqReport < ApplicationRecord
IMPORT_CLASS_NAMES = %w(MiqReport).freeze

scope :for_user, lambda { |user|
if user.admin_user?
if user.report_admin_user?
all
else
where(
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_report/import_export.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def import_from_hash(report, options = nil)
# if report exists
if options[:overwrite]
# if report exists delete and create new
if user.admin_user? || user.current_group_id == rep.miq_group_id
if user.report_admin_user? || user.current_group_id == rep.miq_group_id
msg = "Overwriting Report: [#{report["name"]}]"
rep.attributes = report
result = {:message => "Replaced Report: [#{report["name"]}]", :level => :info, :status => :update}
Expand Down
4 changes: 2 additions & 2 deletions app/models/miq_report_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MiqReportResult < ApplicationRecord
end
}
scope :for_user, lambda { |user|
for_groups(user.admin_user? ? nil : user.miq_group_ids)
for_groups(user.report_admin_user? ? nil : user.miq_group_ids)
}

before_save do
Expand Down Expand Up @@ -357,7 +357,7 @@ def self.with_report(report_id = nil)

def self.with_current_user_groups
current_user = User.current_user
current_user.admin_user? ? all : where(:miq_group_id => current_user.miq_group_ids)
current_user.report_admin_user? ? all : where(:miq_group_id => current_user.miq_group_ids)
end

def self.with_chargeback
Expand Down
10 changes: 8 additions & 2 deletions app/models/miq_user_role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,14 @@ def super_admin_user?
allows?(:identifier => MiqProductFeature::SUPER_ADMIN_FEATURE)
end

def admin_user?
allows_any?(:identifiers => [MiqProductFeature::SUPER_ADMIN_FEATURE, MiqProductFeature::ADMIN_FEATURE])
def report_admin_user?
allows_any?(:identifiers => [MiqProductFeature::SUPER_ADMIN_FEATURE, MiqProductFeature::REPORT_ADMIN_FEATURE])
end

alias admin_user? report_admin_user?

def request_admin_user?
allows_any?(:identifiers => [MiqProductFeature::SUPER_ADMIN_FEATURE, MiqProductFeature::REQUEST_ADMIN_FEATURE])
end

def self.default_tenant_role
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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?,
delegate :super_admin_user?, :admin_user?, :self_service?, :limited_self_service?, :report_admin_user?,
:to => :miq_user_role, :allow_nil => true

validates_presence_of :name, :userid
Expand Down
8 changes: 6 additions & 2 deletions db/fixtures/miq_product_features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@
:description: Edit a Request
:feature_type: admin
:identifier: miq_request_edit
- :name: Request Admin
:description: Edit other user's requests
:feature_type: admin
:identifier: miq_request_superadmin

# Catalog Items
- :name: Catalogs Explorer
Expand Down Expand Up @@ -1603,8 +1607,8 @@
:feature_type: node
:identifier: miq_report_menu_editor
# Special Admin Functionality
- :name: Admin
:description: Special Admin Functionality
- :name: Report Admin
:description: See other user's reports
:feature_type: admin
:identifier: miq_report_superadmin
- :name: Import / Export
Expand Down
1 change: 1 addition & 0 deletions db/fixtures/miq_user_roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
- miq_report
- miq_report_superadmin
- miq_request
- miq_request_superadmin
- miq_template
- orchestration_stack
- physical_server
Expand Down
8 changes: 4 additions & 4 deletions lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -506,15 +506,15 @@ def scope_for_user_role_group(klass, scope, miq_group, user, managed_filters)
if user_or_group.try!(:self_service?) && MiqUserRole != klass
scope.where(:id => klass == User ? user.id : miq_group.id)
else
# hide creating admin group / roles from tenant administrators
unless user_or_group.miq_user_role&.admin_user?
scope = scope.with_roles_excluding([MiqProductFeature::SUPER_ADMIN_FEATURE, MiqProductFeature::ADMIN_FEATURE])
# hide creating admin group / roles from non-super administrators
unless user_or_group.miq_user_role&.super_admin_user?
scope = scope.with_roles_excluding(MiqProductFeature::SUPER_ADMIN_FEATURE)
end

if MiqUserRole != klass
filtered_ids = pluck_ids(get_managed_filter_object_ids(scope, managed_filters))
# Non admins can only see their own groups
scope = scope.with_groups(user.miq_group_ids) unless user_or_group.miq_user_role&.admin_user?
scope = scope.with_groups(user.miq_group_ids) unless user_or_group.miq_user_role&.super_admin_user?
end

scope_by_ids(scope, filtered_ids)
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/rbac/filterer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)

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])
get_rbac_results_for_and_expect_objects(MiqUserRole, [tenant_administrator_user_role, administrator_user_role])
end

it 'can see all groups expect to group with role EvmRole-super_administrator' do
Expand Down
22 changes: 15 additions & 7 deletions spec/models/miq_user_role_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,31 +172,39 @@
expect(MiqUserRole.count).to eq(1)
end

let(:super_admin_role) { FactoryGirl.create(:miq_user_role, :features => MiqProductFeature::SUPER_ADMIN_FEATURE) }
let(:report_admin_role) { FactoryGirl.create(:miq_user_role, :features => MiqProductFeature::REPORT_ADMIN_FEATURE) }
let(:request_admin_role) { FactoryGirl.create(:miq_user_role, :features => MiqProductFeature::REQUEST_ADMIN_FEATURE) }
let(:regular_role) { FactoryGirl.create(:miq_user_role) }

describe "#super_admin_user?" do
it "detects super admin" do
expect(FactoryGirl.create(:miq_user_role, :role => "super_administrator")).to be_super_admin_user
expect(super_admin_role).to be_super_admin_user
end

it "detects admin" do
expect(FactoryGirl.create(:miq_user_role, :role => "administrator")).not_to be_super_admin_user
expect(report_admin_role).not_to be_super_admin_user
end

it "detects non-admin" do
expect(FactoryGirl.create(:miq_user_role)).not_to be_super_admin_user
expect(regular_role).not_to be_super_admin_user
end
end

describe "#admin_user?" do
describe "#admin_user?", "#report_admin_user?" do
it "detects super admin" do
expect(FactoryGirl.create(:miq_user_role, :role => "super_administrator")).to be_admin_user
expect(super_admin_role).to be_admin_user
expect(super_admin_role).to be_report_admin_user
end

it "detects admin" do
expect(FactoryGirl.create(:miq_user_role, :role => "administrator")).to be_admin_user
expect(report_admin_role).to be_admin_user
expect(report_admin_role).to be_report_admin_user
end

it "detects non-admin" do
expect(FactoryGirl.create(:miq_user_role)).not_to be_admin_user
expect(regular_role).not_to be_admin_user
expect(regular_role).not_to be_report_admin_user
end
end

Expand Down

0 comments on commit 3a5bba6

Please sign in to comment.