Fixes #21343 - support multiple orgs supported for non-admin users

* Fixes #21343 - support multiple orgs supported for non-admins

This adds a full support for taxonomies in API for non-admin users. It
fixes the issue with dirty associations module that only track _ids
change. It also makes the nil a valid value for organization_id and
location_id parameters which set "Any context" explictly, so user can
override default context to any. Finally it updates the org admin role
to have permissions to see and edit organizations. That required an
enforcement of taxonomies that are being set as parent as well as
taxonomy filters being searchable by taxonomy_id. So the filter for
e.g. organzations can be correctly scoped for org admin too.

app/controllers/api/base_controller.rb

@@ -278,6 +278,11 @@ def find_nested_object
 
     def not_found_if_nested_id_exists
       allowed_nested_id.each do |obj_id|
+        # this method does not reliably work when you have multiple parameters and some of them can be nil
+        # find_nested_object in such case returns nil (since org and loc can be nil for any context),
+        # but it detects other paramter which can have value set
+        # therefore we always skip these
+        next if [ 'organization_id', 'location_id' ].include?(obj_id)
         if params[obj_id].present?
           not_found _("%{resource_name} not found by id '%{id}'") % { :resource_name => obj_id.humanize, :id => params[obj_id] }
           return
@@ -348,6 +353,13 @@ def parent_resource_details
       end
 
       return nil if parent_name.nil? || parent_class.nil?
+      # for admin we don't want to add any context condition, that would fail for hosts since we'd add join to
+      # taxonomy table without any condition, inner join would return no host in this case
+      return nil if User.current.admin? && [ Organization, Location ].include?(parent_class) && parent_id.blank?
+      # for taxonomies, nil is valid value which indicates, we need to search in Users all taxonomies
+      return [parent_name, User.current.my_organizations] if parent_class == Organization && parent_id.blank?
+      return [parent_name, User.current.my_locations] if parent_class == Location && parent_id.blank?
+
       parent_scope = scope_for(parent_class, :permission => "#{parent_permission(action_permission)}_#{parent_name.pluralize}")
       parent_scope = select_by_resource_id_scope(parent_scope, parent_class, parent_id)
       [parent_name, parent_scope]

app/controllers/concerns/application_shared.rb

@@ -34,43 +34,54 @@ def set_taxonomy
       determined_taxonomy = nil
 
       if api_request? # API request
-        if user.admin? && !params.has_key?("#{taxonomy}_id") # admin always uses any context, they can limit the scope with explicit parameters such as organization_id(s)
-          determined_taxonomy = nil
-        elsif params.has_key?("#{taxonomy}_id") # admin and non-admin who specified explicit context
-          determined_taxonomy = available.where(:id => params["#{taxonomy}_id"]).first
+        if params.has_key?("#{taxonomy}_id") # admin and non-admin who specified context explicitly
+          if params["#{taxonomy}_id"].blank? # the key is present and explicitly set to nil which indicates "any" context
+            determined_taxonomy = nil
+          else
+            determined_taxonomy = available.where(:id => params["#{taxonomy}_id"]).first
 
-          # in case admin asked for taxonomy that does not exist or is not accessible, we reply with 404
-          if determined_taxonomy.nil?
-            not_found _("%{taxonomy} with id %{id} not found") % { :taxonomy => taxonomy.capitalize, :id => params["#{taxonomy}_id"] }
-            return false
+            # in case user asked for taxonomy that does not exist or is not accessible, we reply with 404
+            if determined_taxonomy.nil?
+              not_found _("%{taxonomy} with id %{id} not found") % { :taxonomy => taxonomy.capitalize, :id => params["#{taxonomy}_id"] }
+              return false
+            end
           end
-        elsif request.session["#{taxonomy}_id"].present? # non-admin who didn't not specify explicit context
-          determined_taxonomy = find_session_taxonomy(taxonomy, user)
-        else # non-admin user who didn't specify explicit context and does not have anything in session
+        elsif session.has_key?("#{taxonomy}_id") # session with taxonomy explicitly set to id or nil (any contexxt)
+          if session["#{taxonomy}_id"].present?
+            determined_taxonomy = find_session_taxonomy(taxonomy, user) # specific id
+          else
+            determined_taxonomy = nil # explicitly set any context
+          end
+        else # user who didn't specify explicit context and does not have anything in session
           determined_taxonomy = find_default_taxonomy(user, taxonomy)
         end
       else # UI request
-        if request.session["#{taxonomy}_id"].present?
+        if session["#{taxonomy}_id"].present?
           determined_taxonomy = find_session_taxonomy(taxonomy, user)
         elsif !user.admin? && available.count == 1
           determined_taxonomy = available.first
         end
       end
 
-      store_taxonomy(taxonomy, determined_taxonomy) if determined_taxonomy.present?
+      set_current_taxonomy(taxonomy, determined_taxonomy)
+      store_taxonomy(taxonomy, determined_taxonomy) unless api_request?
     end
   end
 
+  # determined_taxonomy can be nil, which means any context
   def store_taxonomy(taxonomy, determined_taxonomy)
+    # session can't store nil, so we use empty string to represent any context
+    session["#{taxonomy}_id"] = determined_taxonomy.try(:id) || ''
+  end
+
+  def set_current_taxonomy(taxonomy, determined_taxonomy)
     taxonomy.classify.constantize.send 'current=', determined_taxonomy
-    request.session["#{taxonomy}_id"] = determined_taxonomy.id
   end
 
-  def store_default_taxonomies(user)
-    ['location', 'organization'].each do |taxonomy|
-      default_taxonomy = find_default_taxonomy(user, taxonomy)
-      store_taxonomy(taxonomy, default_taxonomy) if default_taxonomy.present?
-    end
+  def store_default_taxonomy(user, taxonomy)
+    default_taxonomy = find_default_taxonomy(user, taxonomy)
+    set_current_taxonomy(taxonomy, default_taxonomy)
+    store_taxonomy(taxonomy, default_taxonomy)
   end
 
   # we want to be explicit to keep this readable
@@ -81,7 +92,7 @@ def find_default_taxonomy(user, taxonomy)
 
     if default_taxonomy.present? && available.include?(default_taxonomy)
       default_taxonomy
-    elsif available.count == 1
+    elsif available.count == 1 && !user.admin?
       available.first
     else
       # no available default taxonomy and user is either admin or user with more taxonomies, nil represents "Any Context"
@@ -91,11 +102,16 @@ def find_default_taxonomy(user, taxonomy)
 
   def find_session_taxonomy(taxonomy, user)
     available = user.send("my_#{taxonomy.pluralize}")
-    determined_taxonomy = available.where(:id => request.session["#{taxonomy}_id"]).first
+    determined_taxonomy = available.where(:id => session["#{taxonomy}_id"]).first
     # warn user if taxonomy stored in session does not exist and delete it from session (probably taxonomy has been deleted meanwhile)
     if determined_taxonomy.nil?
-      warning _("%s you had selected as your context has been deleted") % taxonomy.capitalize unless api_request?
-      request.session["#{taxonomy}_id"] = nil
+      if api_request?
+        not_found _("%{taxonomy} stored in session with id %{id} not found") % { :taxonomy => taxonomy.capitalize, :id => params["#{taxonomy}_id"] }
+        return false
+      else
+        warning _("%s you had selected as your context has been deleted") % taxonomy.capitalize
+      end
+      session.delete("#{taxonomy}_id")
       determined_taxonomy = find_default_taxonomy(user, taxonomy)
     end
     determined_taxonomy

app/controllers/concerns/foreman/controller/taxonomies_controller.rb

@@ -108,7 +108,8 @@ def clear
 
   def clear_current_taxonomy_from_session
     taxonomy_class.current = nil
-    session[taxonomy_id] = nil
+    # session can't store nil, so we use empty string to represent any context
+    session[taxonomy_id] = ''
     TopbarSweeper.expire_cache
   end
 

app/controllers/users_controller.rb

@@ -158,7 +158,8 @@ def login_user(user)
     session[:user]         = user.id
     uri                    = session.to_hash.with_indifferent_access[:original_uri]
     session[:original_uri] = nil
-    store_default_taxonomies(user)
+    store_default_taxonomy(user, 'organization') unless session.has_key?(:organization_id)
+    store_default_taxonomy(user, 'location') unless session.has_key?(:location_id)
     TopbarSweeper.expire_cache
     user.post_successful_login
     redirect_to (uri || hosts_path)

app/models/concerns/taxonomix.rb

@@ -146,8 +146,9 @@ def scope_by_taxable_ids(scope)
 
   def set_current_taxonomy
     if self.new_record? && self.errors.empty?
-      self.locations     << Location.current     if add_current_location?
-      self.organizations << Organization.current if add_current_organization?
+      # we need to use _ids methods so that DirtyAssociations is correctly saved
+      self.location_ids += [ Location.current.id ] if add_current_location?
+      self.organization_ids += [ Organization.current.id ] if add_current_organization?
     end
   end
 

app/models/taxonomies/location.rb

@@ -15,6 +15,8 @@ class Location < Taxonomy
 
   scope :completer_scope, ->(opts) { my_locations }
 
+  scoped_search :on => :id, :validator => ScopedSearch::Validators::INTEGER, :rename => 'location_id', :only_explicit => true
+
   scope :my_locations, lambda { |user = User.current|
     conditions = user.admin? ? {} : sanitize_sql_for_conditions([" (taxonomies.id in (?))", user.location_and_child_ids])
     where(conditions)

app/models/taxonomies/organization.rb

@@ -15,6 +15,8 @@ class Organization < Taxonomy
 
   scope :completer_scope, ->(opts) { my_organizations }
 
+  scoped_search :on => :id, :validator => ScopedSearch::Validators::INTEGER, :rename => 'organization_id', :only_explicit => true
+
   scope :my_organizations, lambda { |user = User.current|
     conditions = user.admin? ? {} : sanitize_sql_for_conditions([" (taxonomies.id in (?))", user.organization_and_child_ids])
     where(conditions)

app/models/taxonomy.rb

@@ -27,6 +27,9 @@ class Taxonomy < ApplicationRecord
   has_many :subnets, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Subnet'
 
   validate :check_for_orphans, :unless => Proc.new {|t| t.new_record?}
+  # the condition for parent_id != 0 is required because of our tests, should validate macros fill in attribute with values and it set 0 to this one
+  # which would lead to an error when we ask for parent object
+  validate :parent_id_does_not_escalate, :if => Proc.new { |t| t.ancestry_changed? && t.parent_id != 0 && t.parent.present? }
   validates :name, :presence => true, :uniqueness => {:scope => [:ancestry, :type], :case_sensitive => false}
 
   def self.inherited(child)
@@ -241,4 +244,11 @@ def assign_taxonomy_to_user
     return if User.current.nil? || User.current.admin
     TaxableTaxonomy.create(:taxonomy_id => self.id, :taxable_id => User.current.id, :taxable_type => 'User')
   end
+
+  def parent_id_does_not_escalate
+    unless User.current.can?("edit_#{self.class.to_s.underscore.pluralize}", self.parent)
+      errors.add :parent_id, _("Missing a permission to edit parent %s") % self.class.to_s
+      return false
+    end
+  end
 end

app/views/domains/index.html.erb

@@ -2,7 +2,7 @@
 
 <% title_actions new_link(_("Create Domain")), help_button %>
 
-<table class="<%= table_css_classes 'table-two-pane table-fixed' %>">
+<table class="<%= table_css_classes 'table-two-pane table-fixed' %>" id="domains_list">
   <thead>
     <tr>
       <th class="col-md-8"><%= sort :name, :as => s_("Domain|Fullname") %></th>

app/views/taxonomies/_form.html.erb

@@ -53,7 +53,8 @@
     <div class="tab-content stacked-content col-md-9">
       <div class="tab-pane active" id="primary">
         <%= base_errors_for taxonomy %>
-        <%= select_f(f, :parent_id, taxonomy.class.completer_scope(nil).where("id NOT IN (#{taxonomy.subtree_ids.join(',')})").order(:title), :id, :title, { :include_blank => true },
+        <% taxonomy_name = taxonomy.class.to_s.underscore %>
+        <%= select_f(f, :parent_id, taxonomy.class.completer_scope(nil).authorized("edit_#{taxonomy_name.pluralize}").where("id NOT IN (#{taxonomy.subtree_ids.join(',')})").order(:title), :id, :title, { :include_blank => true },
                      { :label => _('Parent'), :onchange => 'parent_taxonomy_changed(this);',
                        :help_inline => :indicator,
                        :'data-url' => (controller_name == 'organizations' ? parent_taxonomy_selected_organization_path(taxonomy.id) :

db/seeds.d/020-roles_list.rb

@@ -6,7 +6,7 @@ def seeded_roles
       {
         Role::MANAGER => { :permissions => base_manage_permissions + view_permissions + manage_organizations_permissions,
                            :description => 'Role granting all available permissions. With this role, user is able to do everything that admin can except for changing settings.' },
-        Role::ORG_ADMIN => { :permissions => base_manage_permissions + view_permissions - [:view_organizations],
+        Role::ORG_ADMIN => { :permissions => base_manage_permissions + view_permissions,
                              :description => 'Role granting all permissions except for managing organizations. It can be used to delegate administration of specific organization to a user. In order to create such role, clone this role and assign desired organizations' },
         'Edit partition tables' => { :permissions => [:view_ptables, :create_ptables, :edit_ptables, :destroy_ptables], :description => 'Role granting permissions required for managin partition tables' },
         'View hosts' => { :permissions => [:view_hosts],
@@ -48,12 +48,19 @@ def role_names
     end
 
     def base_manage_permissions
-      PermissionsList.permissions.reject { |resource, name| name.start_with?('view_') }.map { |p| p.last.to_sym } - manage_organizations_permissions
+      PermissionsList.permissions.reject { |resource, name| name.start_with?('view_') }.map { |p| p.last.to_sym } - manage_organizations_permissions - role_managements_permissions
     end
 
     def manage_organizations_permissions
       [
-        :create_organizations, :edit_organizations, :destroy_organizations, :assign_organizations
+        :create_organizations, :destroy_organizations
+      ]
+    end
+
+    def role_managements_permissions
+      [
+        :create_roles, :edit_roles, :destroy_roles,
+        :create_filters, :edit_filters, :destroy_filters,
       ]
     end