Use feature for admin

app/models/miq_group.rb

@@ -15,11 +15,11 @@ class MiqGroup < ApplicationRecord
   has_many   :miq_widget_sets, :as => :owner, :dependent => :destroy
   has_many   :miq_product_features, :through => :miq_user_role
 
-  virtual_column :miq_user_role_name, :type => :string,  :uses => :miq_user_role
+  virtual_delegate :name, :to => :miq_user_role, :allow_nil => true, :prefix => true
   virtual_column :read_only,          :type => :boolean
   virtual_has_one :sui_product_features, :class_name => "Array"
 
-  delegate :self_service?, :limited_self_service?, :disallowed_roles, :to => :miq_user_role, :allow_nil => true
+  delegate :self_service?, :limited_self_service?, :to => :miq_user_role, :allow_nil => true
 
   validates :description, :presence => true, :unique_within_region => { :match_case => false }
   validate :validate_default_tenant, :on => :update, :if => :tenant_id_changed?
@@ -60,8 +60,10 @@ def settings=(new_settings)
     super(indifferent_settings)
   end
 
-  def self.with_allowed_roles_for(user_or_group)
-    includes(:miq_user_role).where.not({:miq_user_roles => {:name => user_or_group.disallowed_roles}})
+  def self.with_roles_excluding(identifier)
+    where.not(:id => MiqGroup.joins(:miq_product_features)
+                             .where(:miq_product_features => {:identifier => identifier})
+                             .select(:id))
   end
 
   def self.next_sequence
@@ -183,10 +185,6 @@ def get_belongsto_filters
     entitlement.try(:get_belongsto_filters) || []
   end
 
-  def miq_user_role_name
-    miq_user_role.try(:name)
-  end
-
   def system_group?
     group_type == SYSTEM_GROUP
   end
@@ -251,9 +249,9 @@ def self.non_tenant_groups_in_my_region
     in_my_region.non_tenant_groups
   end
 
-  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)
+  # parallel to User.with_groups - only show these groups
+  def self.with_groups(miq_group_ids)
+    where(:id => miq_group_ids)
   end
 
   def single_group_users?

app/models/miq_product_feature.rb

@@ -1,4 +1,8 @@
 class MiqProductFeature < ApplicationRecord
+  SUPER_ADMIN_FEATURE = "everything".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
@@ -107,7 +111,7 @@ def self.seed_features(path = FIXTURE_PATH)
     features = all.to_a.index_by(&:identifier)
     seen     = seed_from_hash(YAML.load_file(fixture_yaml), seen, nil, features)
 
-    root_feature = MiqProductFeature.find_by(:identifier => 'everything')
+    root_feature = MiqProductFeature.find_by(:identifier => SUPER_ADMIN_FEATURE)
     Dir.glob(path.join("*.yml")).each do |fixture|
       seed_from_hash(YAML.load_file(fixture), seen, root_feature)
     end

app/models/miq_report.rb

@@ -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(

app/models/miq_report/import_export.rb

@@ -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}

app/models/miq_report_result.rb

@@ -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
@@ -329,7 +329,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

app/models/miq_user_role.rb

@@ -1,6 +1,4 @@
 class MiqUserRole < ApplicationRecord
-  SUPER_ADMIN_ROLE_NAME = "EvmRole-super_administrator"
-  ADMIN_ROLE_NAME       = "EvmRole-administrator"
   DEFAULT_TENANT_ROLE_NAME = "EvmRole-tenant_administrator"
 
   has_many                :entitlements, :dependent => :restrict_with_exception
@@ -64,12 +62,10 @@ def limited_self_service?
     (settings || {}).fetch_path(:restrictions, :vms) == :user
   end
 
-  def disallowed_roles
-    !super_admin_user? && Rbac::Filterer::DISALLOWED_ROLES_FOR_USER_ROLE[name]
-  end
-
-  def self.with_allowed_roles_for(user_or_group)
-    where.not(:name => user_or_group.disallowed_roles)
+  def self.with_roles_excluding(identifier)
+    where.not(:id => MiqUserRole.joins(:miq_product_features)
+                                .where(:miq_product_features => {:identifier => identifier})
+                                .select(:id))
   end
 
   def self.seed
@@ -107,11 +103,17 @@ def vm_restriction
   end
 
   def super_admin_user?
-    name == SUPER_ADMIN_ROLE_NAME
+    allows?(:identifier => MiqProductFeature::SUPER_ADMIN_FEATURE)
   end
 
-  def admin_user?
-    name == SUPER_ADMIN_ROLE_NAME || name == ADMIN_ROLE_NAME
+  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

app/models/user.rb

@@ -25,14 +25,15 @@ class User < ApplicationRecord
   belongs_to :current_group, :class_name => "MiqGroup"
   has_and_belongs_to_many :miq_groups
   scope      :superadmins, lambda {
-    joins(:miq_groups => :miq_user_role).where(:miq_user_roles => {:name => MiqUserRole::SUPER_ADMIN_ROLE_NAME })
+    joins(:miq_groups => {:miq_user_role => :miq_product_features})
+      .where(:miq_product_features => {:identifier => MiqProductFeature::SUPER_ADMIN_FEATURE })
   }
 
   virtual_has_many :active_vms, :class_name => "VmOrTemplate"
 
   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?, :self_service?, :limited_self_service?, :report_admin_user?,
              :to => :miq_user_role, :allow_nil => true
 
   validates_presence_of   :name, :userid
@@ -50,8 +51,10 @@ class User < ApplicationRecord
 
   scope :with_same_userid, ->(id) { where(:userid => User.find(id).userid) }
 
-  def self.with_allowed_roles_for(user_or_group)
-    includes(:miq_groups => :miq_user_role).where.not(:miq_user_roles => {:name => user_or_group.disallowed_roles})
+  def self.with_roles_excluding(identifier)
+    where.not(:id => User.joins(:miq_groups => :miq_product_features)
+                             .where(:miq_product_features => {:identifier => identifier})
+                             .select(:id))
   end
 
   def self.scope_by_tenant?
@@ -285,9 +288,9 @@ def self.current_user
     Thread.current[:user] ||= find_by_userid(current_userid)
   end
 
-  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})
+  # parallel to MiqGroup.with_groups - only show users with these groups
+  def self.with_groups(miq_group_ids)
+    includes(:miq_groups).where(:miq_groups => {:id => miq_group_ids})
   end
 
   def self.missing_user_features(db_user)

db/fixtures/miq_product_features.yml

@@ -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
@@ -1598,6 +1602,11 @@
     :description: Edit Report Menus Accordion
     :feature_type: node
     :identifier: miq_report_menu_editor
+# Special Admin Functionality
+  - :name: Report Admin
+    :description: See other user's reports
+    :feature_type: admin
+    :identifier: miq_report_superadmin
   - :name: Import / Export
     :description: Import / Export Accordion
     :feature_type: node

db/fixtures/miq_user_roles.yml

@@ -63,7 +63,9 @@
   - policy_simulation
   - policy_log
   - miq_report
+  - miq_report_superadmin
   - miq_request
+  - miq_request_superadmin
   - miq_template
   - orchestration_stack
   - physical_rack

lib/rbac/filterer.rb

@@ -76,13 +76,6 @@ class Filterer
       VmOrTemplate
     ) + NETWORK_MODELS_FOR_BELONGSTO_FILTER
 
-    # key: MiqUserRole#name - user's role
-    # value:
-    #   array - disallowed roles for the user's role
-    DISALLOWED_ROLES_FOR_USER_ROLE = {
-      'EvmRole-tenant_administrator' => %w(EvmRole-super_administrator EvmRole-administrator)
-    }.freeze
-
     # key: descendant::klass
     # value:
     #   if it is a symbol/method_name:
@@ -164,7 +157,7 @@ def self.accessible_tenant_ids_strategy(klass)
     # @option options :where_clause  []
     # @option options :sub_filter
     # @option options :include_for_find [Array<Symbol>]
-    # @option options :filter
+    # @option options :filter       [MiqExpression] (optional)
 
     # @option options :user         [User]     (default: current_user)
     # @option options :userid       [String]   User#userid (not user_id)
@@ -521,13 +514,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
-        if user_or_group.disallowed_roles
-          scope = scope.with_allowed_roles_for(user_or_group)
+        # 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))
-          scope = scope.with_current_user_groups(user)
+          # Non admins can only see their own groups
+          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)

spec/factories/miq_user_role.rb

@@ -12,6 +12,7 @@
     name { |ur| ur.role ? "EvmRole-#{ur.role}" : generate(:miq_user_role_name) }
 
     after(:build) do |user, evaluator|
+      e_features = evaluator.features
       if evaluator.role.present?
         @system_roles ||= YAML.load_file(MiqUserRole::FIXTURE_YAML)
         seeded_role = @system_roles.detect { |role| role[:name] == "EvmRole-#{evaluator.role}" }
@@ -20,10 +21,18 @@
           user.read_only = seeded_role[:read_only]
           user.settings = seeded_role[:settings]
         end
+        if e_features.blank?
+          # admins now using a feature instead of a roll
+          if evaluator.role == "super_administrator"
+            e_features = MiqProductFeature::SUPER_ADMIN_FEATURE
+          elsif evaluator.role == "administrator"
+            e_features = MiqProductFeature::ADMIN_FEATURE
+          end
+        end
       end
 
-      if evaluator.features.present?
-        user.miq_product_features = Array.wrap(evaluator.features).map do |f|
+      if e_features.present?
+        user.miq_product_features = Array.wrap(e_features).map do |f|
           if f.kind_of?(MiqProductFeature) # TODO: remove class reference
             f
           else

spec/lib/rbac/filterer_spec.rb

@@ -882,11 +882,11 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
         end
 
         let!(:super_administrator_user_role) do
-          FactoryGirl.create(:miq_user_role, :name => MiqUserRole::SUPER_ADMIN_ROLE_NAME)
+          FactoryGirl.create(:miq_user_role, :role => "super_administrator")
         end
 
         let!(:administrator_user_role) do
-          FactoryGirl.create(:miq_user_role, :name => MiqUserRole::ADMIN_ROLE_NAME)
+          FactoryGirl.create(:miq_user_role, :role => "administrator")
         end
 
         let(:group) do
@@ -897,7 +897,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

spec/lib/task_helpers/exports/roles_spec.rb

@@ -10,7 +10,7 @@
     [{"name"                => "EvmRole-super_administrator",
       "read_only"           => true,
       "settings"            => nil,
-      "feature_identifiers" => []}]
+      "feature_identifiers" => ["everything"]}]
   end
 
   let(:export_dir) do

spec/models/miq_report_result_spec.rb

@@ -64,7 +64,7 @@
       end
 
       it "returns report all results, admin user logged" do
-        admin_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-administrator", :read_only => false)
+        admin_role = FactoryGirl.create(:miq_user_role, :role => "administrator", :read_only => false)
         User.current_user.current_group.miq_user_role = admin_role
         report_result = MiqReportResult.with_current_user_groups
         expected_reports = [@report_result1, @report_result2, @report_result3, @report_result_nil_report_id]

spec/models/miq_user_role_spec.rb

@@ -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.build(: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.build(: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.build(: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.build(: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.build(: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.build(: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
 

spec/models/user/user_ldap_methods_spec.rb

@@ -105,7 +105,7 @@ def create_super_admin_group
     FactoryGirl.create(
       :miq_group,
       :description   => "EvmGroup-super_administrator",
-      :miq_user_role => FactoryGirl.create(:miq_user_role, :name => "EvmRole-super_administrator")
+      :miq_user_role => FactoryGirl.create(:miq_user_role, :role => "super_administrator")
     )
   end