Fixes #5929 - Taxonomy filter obey permissions

With this patch you can assign permissions like assign_organizations and
assign_locations to particular user so that they can then assign
taxonomies
only from set of taxonomies granted by their filters.

Global users would be still able to assign any taxonomy to a resource as
long as they have appropriate assign permissions. They can also leave
the resource global.

app/helpers/taxonomy_helper.rb

@@ -121,7 +121,7 @@ def location_selects(f, selected_ids, options = {}, options_html = {})
   def taxonomy_selects(f, selected_ids, taxonomy, label, options = {}, options_html = {})
     options[:disabled] = Array.wrap(options[:disabled])
     options[:label]    ||= _(label)
-    multiple_selects f, label.downcase, taxonomy, selected_ids, options, options_html
+    multiple_selects f, label.downcase, taxonomy.authorized("assign_#{label.downcase}", taxonomy), selected_ids, options, options_html
   end
 
 end

app/models/concerns/dirty_associations.rb

@@ -0,0 +1,63 @@
+# ActiveModel::Dirty does not support associations but it's sometime it'd be useful
+# so you could ask like this
+#   @user.role_ids_was
+#
+# This concern can be used exactly for this. Note that it does not detect changes to
+# associated object but just change of the collection itself.
+#
+# Also note one difference to built-in attribute dirty cache for new records. With
+# this concern we don't wait on saving the record which results in following behavior
+#   @user = User.new
+#   @user.role_ids = [8]
+#   @user.role_ids = [8, 9]
+#   @user.role_ids_was # => [8]
+module DirtyAssociations
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    # usage:
+    #   class Model
+    #     dirty_has_many_associations :organizations, :locations
+    def dirty_has_many_associations(*args)
+      args.each do |association|
+        association_ids = association.to_s.singularize + '_ids'
+
+        # result for :organizations
+        #   def organization_ids_with_change_detection=(organizations)
+        #     organizations ||= []
+        #     @organization_ids_changed = organizations.uniq.select(&:present?).map(&:to_i).sort != organization_ids.sort
+        #     @organization_ids_was = organization_ids.clone
+        #     self.organization_ids_without_change_detection = organizations
+        #   end
+        define_method "#{association_ids}_with_change_detection=" do |collection|
+          collection ||= [] # in API, #{association}_ids is converted to nil if user sent empty array
+          instance_variable_set("@#{association_ids}_changed", collection.uniq.select(&:present?).map(&:to_i).sort != self.send(association_ids).sort)
+          instance_variable_set("@#{association_ids}_was", self.send(association_ids).clone)
+          self.send("#{association_ids}_without_change_detection=", collection)
+        end
+        alias_method_chain("#{association_ids}=", :change_detection)
+
+        # result for :organizations
+        #   def organization_ids_changed?
+        #     @organization_ids_changed
+        #   end
+        define_method "#{association_ids}_changed?" do
+          instance_variable_get("@#{association_ids}_changed")
+        end
+
+        # result for :organizations
+        #   def organization_ids_was
+        #     @organization_ids_was ||= organization_ids
+        #   end
+        define_method "#{association_ids}_was" do
+          value = instance_variable_get("@#{association_ids}_was")
+          if value.nil?
+            instance_variable_set("@#{association_ids}_was", self.send(association_ids))
+          else
+            value
+          end
+        end
+      end
+    end
+  end
+end

app/models/concerns/taxonomix.rb

@@ -1,5 +1,6 @@
 module Taxonomix
   extend ActiveSupport::Concern
+  include DirtyAssociations
 
   included do
     taxonomy_join_table = "taxable_taxonomies"
@@ -14,6 +15,10 @@ module Taxonomix
     scoped_search :in => :locations, :on => :id, :rename => :location_id, :complete_value => true
     scoped_search :in => :organizations, :on => :name, :rename => :organization, :complete_value => true
     scoped_search :in => :organizations, :on => :id, :rename => :organization_id, :complete_value => true
+
+    dirty_has_many_associations :organizations, :locations
+
+    validate :ensure_taxonomies_not_escalated, :if => Proc.new { User.current.nil? || !User.current.admin? }
   end
 
   module ClassMethods
@@ -147,6 +152,34 @@ def children_of_selected_organization_ids
     children_of_selected_taxonomy_ids(:organizations)
   end
 
+  # Only administrator can choose empty taxonomies or taxonomies that they aren't assigned to,
+  # other users can select only taxonomies they are granted to assign
+  # and they can't leave the selection empty (which would mean global resource)
+  #
+  # we skip checking if user is global or it's not a new record and taxonomies were not changed
+  # for existing records it would block saving global objects if taxonomies were not changed
+  # for new records this would allow to create global objects for non global users
+  def ensure_taxonomies_not_escalated
+    taxonomies = []
+    taxonomies << Organization if SETTINGS[:organizations_enabled]
+    taxonomies << Location if SETTINGS[:locations_enabled]
+
+    taxonomies.each do |taxonomy|
+      assoc_base = taxonomy.to_s.downcase
+      assoc = assoc_base.pluralize
+      key = assoc_base + '_ids'
+
+      next if (User.current.nil? || User.current.send("#{assoc}").empty?) || (!new_record? && !self.send("#{key}_changed?"))
+
+      allowed = taxonomy.authorized("assign_#{assoc}", taxonomy).pluck(:id).to_set
+      tried = self.send(key).to_set
+
+      if tried.empty? || !tried.subset?(allowed)
+        errors.add key, _('Invalid %s selection, you must select at least one of yours') % _(assoc)
+      end
+    end
+  end
+
   protected
 
   def taxonomy_foreign_key_conditions

app/models/user.rb

@@ -5,6 +5,7 @@ class User < ActiveRecord::Base
   include Authorizable
   include Foreman::ThreadSession::UserModel
   include Taxonomix
+  include DirtyAssociations
   audited :except => [:last_login_on, :password, :password_hash, :password_salt, :password_confirmation], :allow_mass_assignment => true
   self.auditing_enabled = !Foreman.in_rake?('db:migrate')
 
@@ -107,6 +108,8 @@ def self.name_format
     end
   }
 
+  dirty_has_many_associations :roles
+
   def can?(permission, subject = nil)
     if self.admin?
       true
@@ -317,22 +320,6 @@ def can_change_admin_flag?
     self.admin?
   end
 
-  def role_ids_with_change_detection=(roles)
-    roles ||= [] # in API, role_ids is converted to nil if user sent empty array
-    @role_ids_changed = roles.uniq.select(&:present?).map(&:to_i).sort != role_ids.sort
-    @role_ids_was = role_ids.clone
-    self.role_ids_without_change_detection = roles
-  end
-  alias_method_chain(:role_ids=, :change_detection)
-
-  def role_ids_changed?
-    @role_ids_changed
-  end
-
-  def role_ids_was
-    @role_ids_was ||= role_ids
-  end
-
   def editing_self?(options = {})
     options[:controller].to_s == 'users' &&
       options[:action] =~ /edit|update/ &&

test/lib/dirty_associations_test.rb

@@ -0,0 +1,28 @@
+require 'test_helper'
+
+class DummyDirtyAssociationsModel
+  def organization_ids
+    @organization_ids ||= []
+  end
+  attr_writer :organization_ids
+
+  include DirtyAssociations
+  dirty_has_many_associations :organizations
+end
+
+class DirtyAssociationsTest < ActiveSupport::TestCase
+  def setup
+    @tester = DummyDirtyAssociationsModel.new
+    @tester.organization_ids = [1, 2]
+  end
+
+  test "value change sets change flag" do
+    assert @tester.organization_ids_changed?
+  end
+
+  test "value change stores previous value" do
+    assert_equal @tester.organization_ids_was, []
+    @tester.organization_ids = [3]
+    assert_equal @tester.organization_ids_was, [1, 2]
+  end
+end

test/test_helper.rb

@@ -84,7 +84,7 @@ def set_session_user
 
     def as_user user
       saved_user   = User.current
-      User.current = users(user)
+      User.current = user.is_a?(User) ? user : users(user)
       result = yield
       User.current = saved_user
       result
@@ -103,6 +103,18 @@ def in_taxonomy(taxonomy)
       result
     end
 
+    def disable_taxonomies
+      org_settings = SETTINGS[:organizations_enabled]
+      SETTINGS[:organizations_enabled] = false
+      loc_settings = SETTINGS[:locations_enabled]
+      SETTINGS[:locations_enabled] = false
+      result = yield
+    ensure
+      SETTINGS[:organizations_enabled] = org_settings
+      SETTINGS[:locations_enabled] = loc_settings
+      return result
+    end
+
     def setup_users
       User.current = users :admin
       user = User.find_by_login("one")

test/unit/user_test.rb

@@ -391,6 +391,72 @@ def setup_user operation
     refute admin.admin?
   end
 
+  test "admin can assign arbitrary taxonomies" do
+    as_admin do
+      user = FactoryGirl.build(:user)
+      org1 = FactoryGirl.create(:organization)
+      org2 = FactoryGirl.create(:organization)
+      user.organization_ids = [org1.id, org2.id]
+      assert user.save
+    end
+  end
+
+  test "user can set only subset of his taxonomies" do
+    # superset, one of two, another of two, both
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization)
+    org3 = FactoryGirl.create(:organization)
+    loc1 = FactoryGirl.create(:location)
+
+    user = FactoryGirl.build(:user)
+    user.organizations = [org1, org2]
+    user.locations = [loc1]
+    user.save
+    Organization.expects(:authorized).with('assign_organizations', Organization).returns(Organization.where(:id => [org1, org2])).times(4)
+    Location.expects(:authorized).with('assign_locations', Location).returns(Location.where(:id => [loc1])).times(4)
+
+    as_user user do
+      # org subset
+      new_user = FactoryGirl.build(:user)
+      new_user.organization_ids = [org1.id]
+      new_user.location_ids = [loc1.id]
+      assert new_user.save
+
+      # org subset
+      new_user = FactoryGirl.build(:user)
+      new_user.organization_ids = [org2.id]
+      new_user.location_ids = [loc1.id]
+      assert new_user.save
+
+      # org same set
+      new_user = FactoryGirl.build(:user)
+      new_user.organization_ids = [org1.id, org2.id]
+      new_user.location_ids = [loc1.id]
+      assert new_user.save
+
+      # org superset
+      new_user = FactoryGirl.build(:user)
+      new_user.organization_ids = [org1.id, org3.id]
+      new_user.location_ids = [loc1.id]
+      refute new_user.save
+      assert_not_empty new_user.errors[:organization_ids]
+    end
+  end
+
+  test "user can't set empty taxonomies set if he's assigned to some" do
+    user = FactoryGirl.create(:user)
+    org1 = FactoryGirl.create(:organization)
+    user.organizations << org1
+
+    as_user user do
+      # empty set
+      new_user = FactoryGirl.build(:user)
+      refute new_user.save
+      assert_not_empty new_user.errors[:organization_ids]
+      assert_empty new_user.errors[:location_ids]
+    end
+  end
+
   test ".find_or_create_external_user" do
     count = User.count
     # existing user