Merge pull request #8281 from martinpovolny/role_rbac_fixes

Role RBAC tree and feature fixes
ManageIQ · Apr 28, 2016 · 91b798c · 91b798c
2 parents 5ee02a0 + 010dd5a
commit 91b798c
Show file tree

Hide file tree

Showing 3 changed files with 143 additions and 134 deletions.
diff --git a/app/controllers/ops_controller/ops_rbac.rb b/app/controllers/ops_controller/ops_rbac.rb
@@ -1185,7 +1185,7 @@ def rbac_role_set_form_vars
     @edit[:new][:name] = @record.name
     vmr = @record.settings.fetch_path(:restrictions, :vms) if @record.settings
     @edit[:new][:vm_restriction] = vmr || :none
-    @edit[:new][:features] = rbac_expand_features(@record.feature_identifiers, MiqProductFeature.feature_root).sort
+    @edit[:new][:features] = rbac_expand_features(@record.feature_identifiers).sort
 
     @edit[:current] = copy_hash(@edit[:new])
 
@@ -1194,19 +1194,49 @@ def rbac_role_set_form_vars
   end
 
   # Get array of total set of features from the children of selected features
-  def rbac_expand_features(ids, node) # Selected IDS and node to check
-    if ids.include?(node)   # This node is selected, return all children
-      return [node] + MiqProductFeature.feature_all_children(node)
-    else                    # Node is not selected, check this nodes direct children
-      MiqProductFeature.feature_children(node).flat_map { |n| rbac_expand_features(ids, n) }
+  def rbac_expand_features(selected, node = nil)
+    node ||= MiqProductFeature.feature_root
+    if selected.include?(node)
+      [node] + MiqProductFeature.feature_all_children(node)
+    else
+      MiqProductFeature.feature_children(node).flat_map { |n| rbac_expand_features(selected, n) }
     end
   end
 
   # Get array of all fully selected parent or leaf node features
-  def rbac_compact_features(ids, node)  # Selected IDS and node to check
-    return [node] if ids.include?(node) # This feature is selected, return this node
+  def rbac_compact_features(selected, node = nil)
+    node ||= MiqProductFeature.feature_root
+    return [node] if selected.include?(node)
     MiqProductFeature.feature_children(node, false).flat_map do |n|
-      rbac_compact_features(ids, n)
+      rbac_compact_features(selected, n)
+    end
+  end
+
+  # Yield all features for given tree node a section or feature
+  #
+  # a. special case _tab_all_vm_rules
+  # b. section node /^_tab_/
+  #   return all features below this section and
+  #   recursively below any nested sections
+  #   and nested features recursively
+  # c. feature node
+  #   return nested features recursively
+  #
+  def recurse_sections_and_features(node)
+    if node =~ /_tab_all_vm_rules$/
+      MiqProductFeature.feature_children('all_vm_rules').each do |feature|
+        kids = MiqProductFeature.feature_all_children(feature)
+        yield feature, [feature] + kids
+      end
+    elsif node =~ /^_tab_/
+      section_id = node.split('_tab_').last.to_sym
+      Menu::Manager.section(section_id).features_recursive.each do |f|
+        kids = MiqProductFeature.feature_all_children(f)
+        yield f, [f] + kids
+      end
+    else
+      kids = MiqProductFeature.feature_all_children(node)
+      yield node, [node] + kids
     end
   end
 
@@ -1218,36 +1248,21 @@ def rbac_role_get_form_vars
     if params[:check]
       node = params[:id].split("__").last # Get the feature of the checked node
       if params[:check] == "0"  # Unchecked
-        if node.starts_with?("_tab_") # Remove all features under this tab
-          tab_features_for_node(node).each do |f|
-            @edit[:new][:features] -= ([f] + MiqProductFeature.feature_all_children(f)) # Remove the feature + children
-            rbac_role_remove_parent(f)  # Remove all parents above the unchecked tab feature
-          end
-        else  # Remove the unchecked feature
-          @edit[:new][:features] -= ([node] + MiqProductFeature.feature_all_children(node))
-          rbac_role_remove_parent(node) # Remove all parents above the unchecked node
+        recurse_sections_and_features(node) do |feature, all|
+          @edit[:new][:features] -= all # remove the feature + children
+          rbac_role_remove_parent(feature) # remove all parents above the unchecked tab feature
         end
       else                      # Checked
-        if node.starts_with?("_tab_") # Add all features under this tab
-          tab_features_for_node(node).each do |f|
-            @edit[:new][:features] += ([f] + MiqProductFeature.feature_all_children(f))
-            rbac_role_add_parent(f) # Add any parents above the checked tab feature that have all children checked
-          end
-        else  # Add the checked feature
-          @edit[:new][:features] += ([node] + MiqProductFeature.feature_all_children(node))
-          rbac_role_add_parent(node)  # Add any parents above the checked node that have all children checked
+        recurse_sections_and_features(node) do |feature, all|
+          @edit[:new][:features] += all # remove the feature + children
+          rbac_role_add_parent(feature) # remove all parents above the unchecked tab feature
         end
       end
     end
     @edit[:new][:features].uniq!
     @edit[:new][:features].sort!
   end
 
-  def tab_features_for_node(node)
-    feature_id = node.split('_tab_')
-    node =~ /_tab_all_vm_rules$/ ? MiqProductFeature.feature_children(feature_id) : Menu::Manager.section(feature_id)
-  end
-
   # Walk the features tree, removing features up to the top
   def rbac_role_remove_parent(node)
     return unless parent = MiqProductFeature.feature_parent(node) # Intentional single =, using parent var below
@@ -1278,10 +1293,9 @@ def rbac_role_set_record_vars(role)
 
   def set_role_features(role)
     role.miq_product_features =
-      MiqProductFeature.find_all_by_identifier(rbac_compact_features(@edit[:new][:features], MiqProductFeature.feature_root))
+      MiqProductFeature.find_all_by_identifier(rbac_compact_features(@edit[:new][:features]))
   end
 
-
   # Validate some of the role fields
   def rbac_role_validate?
     valid = true

diff --git a/db/fixtures/miq_product_features.yml b/db/fixtures/miq_product_features.yml
@@ -643,80 +643,75 @@
       :feature_type: control
       :identifier: flavor_tag
 
-# Compute top level
-- :name: Compute
-  :description: Compute menu
+# EmsInfra
+- :name: Infrastructure Providers
+  :description: Everything under Infrastructure Providers
   :feature_type: node
-  :identifier: compute
+  :identifier: ems_infra
   :children:
-  - :name: Infrastructure Providers
-    :description: Everything under Infrastructure Providers
-    :feature_type: node
-    :identifier: ems_infra
+  - :name: View
+    :description: View Infrastructure Providers
+    :feature_type: view
+    :identifier: ems_infra_view
     :children:
-    - :name: View
-      :description: View Infrastructure Providers
+    - :name: List
+      :description: Display Lists of Infrastructure Providers
       :feature_type: view
-      :identifier: ems_infra_view
-      :children:
-      - :name: List
-        :description: Display Lists of Infrastructure Providers
-        :feature_type: view
-        :identifier: ems_infra_show_list
-      - :name: Show
-        :description: Display Individual Infrastructure Providers
-        :feature_type: view
-        :identifier: ems_infra_show
-      - :name: Timeline
-        :description: Display Timelines for Infrastructure Providers
-        :feature_type: view
-        :identifier: ems_infra_timeline
-    - :name: Operate
-      :description: Perform Operations on Infrastructure Providers
+      :identifier: ems_infra_show_list
+    - :name: Show
+      :description: Display Individual Infrastructure Providers
+      :feature_type: view
+      :identifier: ems_infra_show
+    - :name: Timeline
+      :description: Display Timelines for Infrastructure Providers
+      :feature_type: view
+      :identifier: ems_infra_timeline
+  - :name: Operate
+    :description: Perform Operations on Infrastructure Providers
+    :feature_type: control
+    :identifier: ems_infra_control
+    :children:
+    - :name: Discover
+      :description: Discover Infrastructure Providers
       :feature_type: control
-      :identifier: ems_infra_control
-      :children:
-      - :name: Discover
-        :description: Discover Infrastructure Providers
-        :feature_type: control
-        :identifier: ems_infra_discover
-      - :name: Edit Tags
-        :description: Edit Tags of Infrastructure Providers
-        :feature_type: control
-        :identifier: ems_infra_tag
-      - :name: Manage Policies
-        :description: Manage Policies of Infrastructure Providers
-        :feature_type: control
-        :identifier: ems_infra_protect
-      - :name: Refresh
-        :description: Refresh Infrastructure Providers
-        :feature_type: control
-        :identifier: ems_infra_refresh
-    - :name: Modify
-      :description: Modify Infrastructure Providers
+      :identifier: ems_infra_discover
+    - :name: Edit Tags
+      :description: Edit Tags of Infrastructure Providers
+      :feature_type: control
+      :identifier: ems_infra_tag
+    - :name: Manage Policies
+      :description: Manage Policies of Infrastructure Providers
+      :feature_type: control
+      :identifier: ems_infra_protect
+    - :name: Refresh
+      :description: Refresh Infrastructure Providers
+      :feature_type: control
+      :identifier: ems_infra_refresh
+  - :name: Modify
+    :description: Modify Infrastructure Providers
+    :feature_type: admin
+    :identifier: ems_infra_admin
+    :children:
+    - :name: Remove
+      :description: Remove Infrastructure Providers from the VMDB
       :feature_type: admin
-      :identifier: ems_infra_admin
-      :children:
-      - :name: Remove
-        :description: Remove Infrastructure Providers from the VMDB
-        :feature_type: admin
-        :identifier: ems_infra_delete
-      - :name: Edit
-        :description: Edit an Infrastructure Provider
-        :feature_type: admin
-        :identifier: ems_infra_edit
-      - :name: Add
-        :description: Add an Infrastructure Provider
-        :feature_type: admin
-        :identifier: ems_infra_new
-      - :name: Scale
-        :description: Scale an Infrastructure Provider
-        :feature_type: admin
-        :identifier: ems_infra_scale
-      - :name: Scale Down
-        :description: Scale an Infrastructure Provider down
-        :feature_type: admin
-        :identifier: ems_infra_scaledown
+      :identifier: ems_infra_delete
+    - :name: Edit
+      :description: Edit an Infrastructure Provider
+      :feature_type: admin
+      :identifier: ems_infra_edit
+    - :name: Add
+      :description: Add an Infrastructure Provider
+      :feature_type: admin
+      :identifier: ems_infra_new
+    - :name: Scale
+      :description: Scale an Infrastructure Provider
+      :feature_type: admin
+      :identifier: ems_infra_scale
+    - :name: Scale Down
+      :description: Scale an Infrastructure Provider down
+      :feature_type: admin
+      :identifier: ems_infra_scaledown
 
 # Datacenters
 - :name: Datacenters
@@ -3141,7 +3136,7 @@
       :identifier: orchestration_stack_retire
 
 # EmsContainer
-- :name: Providers
+- :name: Container Providers
   :description: Everything under Containers Providers
   :feature_type: node
   :identifier: ems_container
@@ -3256,8 +3251,19 @@
         :feature_type: admin
         :identifier: ems_middleware_new
 
+  # Middleware Topology
+  - :name: Middleware Topology
+    :description: Middleware Topology
+    :feature_type: node
+    :identifier: middleware_topology
+    :children:
+      - :name: View
+        :description: View Middleware Topology
+        :feature_type: view
+        :identifier: middleware_topology_view
+
   # MiddlewareServer
-  - :name: MiddlewareServer
+  - :name: Middleware Server
     :description: Everything under Middleware Servers
     :feature_type: node
     :identifier: middleware_server
@@ -3319,7 +3325,7 @@
         :identifier: middleware_server_stop
 
   # MiddlewareDeployment
-  - :name: MiddlewareDeployment
+  - :name: Middleware Deployment
     :description: Everything under Middleware Deployments
     :feature_type: node
     :identifier: middleware_deployment
@@ -3542,6 +3548,17 @@
       :feature_type: control
       :identifier: network_port_tag
 
+# Network Topology
+- :name: Network Topology
+  :description: Network Topology
+  :feature_type: node
+  :identifier: network_topology
+  :children:
+    - :name: View
+      :description: View Network Topology
+      :feature_type: view
+      :identifier: network_topology_view
+
 # Cloud Subnet
 - :name: Cloud Subnets
   :description: Everything under Cloud Subnets
@@ -4133,27 +4150,16 @@
         :feature_type: view
         :identifier: container_topology_view
 
-  # Middleware Topology
-  - :name: Middleware Topology
-    :description: Middleware Topology
-    :feature_type: node
-    :identifier: middleware_topology
-    :children:
-      - :name: View
-        :description: View Middleware Topology
-        :feature_type: view
-        :identifier: middleware_topology_view
-
-  # Network Topology
-  - :name: Network Topology
-    :description: Network Topology
+  # Container Dashboard
+  - :name: Containers Dashboard
+    :description: Containers Dashboard
     :feature_type: node
-    :identifier: network_topology
+    :identifier: container_dashboard
     :children:
-      - :name: View
-        :description: View Network Topology
-        :feature_type: view
-        :identifier: network_topology_view
+    - :name: View
+      :description: View Containers Dashboard
+      :feature_type: view
+      :identifier: container_dashboard_view
 
 # Foreman
 - :name: Configuration Management
@@ -4790,14 +4796,3 @@
           :description: Revert to selected snapshot on Templates
           :feature_type: control
           :identifier: miq_template_snapshot_revert
-
-# Container Dashboard
-- :name: Containers Dashboard
-  :description: Containers Dashboard
-  :feature_type: node
-  :identifier: container_dashboard
-  :children:
-  - :name: View
-    :description: View Containers Dashboard
-    :feature_type: view
-    :identifier: container_dashboard_view
diff --git a/spec/models/miq_product_feature_spec.rb b/spec/models/miq_product_feature_spec.rb
@@ -2,7 +2,7 @@
 require 'pathname'
 
 describe MiqProductFeature do
-  let(:expected_feature_count) { 1018 }
+  let(:expected_feature_count) { 1017 }
 
   # - container_dashboard
   # - miq_report_widget_editor