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 16, 2018
1 parent 1492fb7 commit 7328fb4
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 22 deletions.
9 changes: 6 additions & 3 deletions app/models/miq_product_feature.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
class MiqProductFeature < ApplicationRecord
SUPER_ADMIN_FEATURE = "everything".freeze
ADMIN_FEATURE = "miq_report_superadmin".freeze
TENANT_ADMIN_FEATURE = "rbac_tenant".freeze
SUPER_ADMIN_FEATURE = "everything".freeze
REPORT_ADMIN_FEATURE = "miq_report_superadmin".freeze
REQUEST_ADMIN_FEATURE = "miq_request_superadmin".freeze
ADMIN_FEATURE = REPORT_ADMIN_FEATURE
TENANT_ADMIN_FEATURE = "rbac_tenant".freeze

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 tenant_admin_user?
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?, :tenant_admin_user?, :self_service?, :limited_self_service?,
delegate :super_admin_user?, :admin_user?, :tenant_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 @@ -1609,8 +1613,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
4 changes: 2 additions & 2 deletions lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ def scope_for_user_role_group(klass, scope, miq_group, user, managed_filters)
scope.where(:id => klass == User ? user.id : miq_group.id)
else
# hide creating admin group / roles from tenant administrators
if user_or_group.miq_user_role&.tenant_admin_user?
scope = scope.with_roles_excluding([MiqProductFeature::SUPER_ADMIN_FEATURE, MiqProductFeature::ADMIN_FEATURE])
unless user_or_group.miq_user_role&.super_admin_user?
scope = scope.with_roles_excluding(MiqProductFeature::SUPER_ADMIN_FEATURE)
end

if MiqUserRole != klass
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 @@ -951,7 +951,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)

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

it 'can see all groups except for group with roles EvmRole-super_administrator amd EvmRole-administrator' do
Expand Down
21 changes: 14 additions & 7 deletions spec/models/miq_user_role_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,33 +172,40 @@
expect(MiqUserRole.count).to eq(1)
end

let(:super_admin_role) { FactoryGirl.create(:miq_user_role, :features => MiqProductFeature::SUPER_ADMIN_FEATURE) }
let(:tenant_admin_role) { FactoryGirl.create(:miq_user_role, :features => MiqProductFeature::TENANT_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 7328fb4

Please sign in to comment.