Fixes #9947 - restrict user taxonomies if none is set

app/models/concerns/taxonomix.rb

@@ -27,8 +27,8 @@ module ClassMethods
     # default inner_method includes children (subtree_ids)
     def with_taxonomy_scope(loc = Location.current, org = Organization.current, inner_method = :subtree_ids)
       self.which_ancestry_method = inner_method
-      self.which_location        = loc
-      self.which_organization    = org
+      self.which_location        = Location.expand(loc)
+      self.which_organization    = Organization.expand(org)
       scope =  block_given? ? yield : where('1=1')
       scope = scope.where(:id => taxable_ids) if taxable_ids
       scope.readonly(false)
@@ -47,13 +47,17 @@ def used_taxonomy_ids
     def used_location_ids
       enforce_default
       return [] unless which_location && SETTINGS[:locations_enabled]
-      (which_location.send(which_ancestry_method) + which_location.ancestor_ids).uniq
+      get_taxonomy_ids(which_location, which_ancestry_method)
     end
 
     def used_organization_ids
       enforce_default
       return [] unless which_organization && SETTINGS[:organizations_enabled]
-      (which_organization.send(which_ancestry_method) + which_organization.ancestor_ids).uniq
+      get_taxonomy_ids(which_organization, which_ancestry_method)
+    end
+
+    def get_taxonomy_ids(taxonomy, method)
+      Array(taxonomy).map { |t| t.send(method) + t.ancestor_ids }.flatten.uniq
     end
 
     # default scope is not called if we just use #scoped therefore we have to enforce quering
@@ -65,14 +69,14 @@ def enforce_default
     end
 
     def taxable_ids(loc = which_location, org = which_organization, inner_method = which_ancestry_method)
-      if SETTINGS[:locations_enabled] && loc
+      if SETTINGS[:locations_enabled] && loc.present?
         inner_ids_loc = if Location.ignore?(self.to_s)
                           self.pluck("#{table_name}.id")
                         else
                           inner_select(loc, inner_method)
                         end
       end
-      if SETTINGS[:organizations_enabled] && org
+      if SETTINGS[:organizations_enabled] && org.present?
         inner_ids_org = if Organization.ignore?(self.to_s)
                           self.pluck("#{table_name}.id")
                         else
@@ -87,10 +91,17 @@ def taxable_ids(loc = which_location, org = which_organization, inner_method = w
       inner_ids
     end
 
+    # taxonomy can be either specific taxonomy object or array of these objects
+    # it can also be an empty array that means all taxonomies (user is not assigned to any)
     def inner_select(taxonomy, inner_method = which_ancestry_method)
       # always include ancestor_ids in inner select
-      taxonomy_ids = (taxonomy.send(inner_method) + taxonomy.ancestor_ids).uniq
-      TaxableTaxonomy.where(:taxable_type => self.name, :taxonomy_id => taxonomy_ids).pluck(:taxable_id).compact.uniq
+      conditions = { :taxable_type => self.name }
+      if taxonomy.present?
+        taxonomy_ids = get_taxonomy_ids(taxonomy, inner_method)
+        conditions.merge!(:taxonomy_id => taxonomy_ids)
+      end
+
+      TaxableTaxonomy.where(conditions).pluck(:taxable_id).compact.uniq
     end
 
     def admin_ids

app/models/host/managed.rb

@@ -68,11 +68,11 @@ class Jail < ::Safemode::Jail
   attr_reader :cached_host_params
 
   default_scope lambda {
-    org = Organization.current
-    loc = Location.current
+    org = Organization.expand(Organization.current)
+    loc = Location.expand(Location.current)
     conditions = {}
-    conditions[:organization_id] = org.subtree_ids if org
-    conditions[:location_id]     = loc.subtree_ids if loc
+    conditions[:organization_id] = Array(org).map {|o| o.subtree_ids }.flatten.uniq if org.present?
+    conditions[:location_id]     = Array(loc).map {|l| l.subtree_ids }.flatten.uniq if loc.present?
     where(conditions)
   }
 

app/models/report.rb

@@ -33,7 +33,7 @@ class Report < ActiveRecord::Base
 
   # returns reports for hosts in the User's filter set
   scope :my_reports, lambda {
-    unless User.current.admin? and Organization.current.nil? and Location.current.nil?
+    if !User.current.admin? || Organization.expand(Organization.current).present? || Location.expand(Location.current).present?
       where(:reports => {:host_id => Host.authorized(:view_hosts, Host)})
     end
   }

app/models/taxonomy.rb

@@ -85,6 +85,21 @@ def self.ignore?(taxable_type)
     false
   end
 
+  # if taxonomy e.g. organization was not set by current context (e.g. Any organization)
+  # then we have to compute what this context mean for current user (in what organizations
+  # is he assigned to)
+  #
+  # if user is not assigned to any organization then empty array is returned which means
+  # that we should use all organizations
+  #
+  # if user is admin we we return the original value since it does not need any additional scoping
+  def self.expand(value)
+    if value.blank? && User.current.present? && !User.current.admin?
+      value = self.send("my_#{self.to_s.underscore.pluralize}").all
+    end
+    value
+  end
+
   def ignore?(taxable_type)
     if ignore_types.empty?
       false

test/unit/host_test.rb

@@ -435,6 +435,32 @@ def teardown
     assert host.save!
   end
 
+  test 'host can be searched in multiple taxonomies' do
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization)
+    org3 = FactoryGirl.create(:organization)
+    user = FactoryGirl.create(:user, :organizations => [org1, org2])
+    host1 = FactoryGirl.create(:host, :organization => org1)
+    host2 = FactoryGirl.create(:host, :organization => org2)
+    host3 = FactoryGirl.create(:host, :organization => org3)
+    hosts = nil
+
+    assert_nil Organization.current
+    as_user(user) do
+      hosts = Host::Managed.all
+    end
+    assert_includes hosts, host1
+    assert_includes hosts, host2
+    refute_includes hosts, host3
+
+    as_user(:one) do
+      hosts = Host::Managed.all
+    end
+    assert_includes hosts, host1
+    assert_includes hosts, host2
+    assert_includes hosts, host3
+  end
+
   context "location or organizations are not enabled" do
     before do
       SETTINGS[:locations_enabled] = false

test/unit/taxonomix_test.rb

@@ -47,4 +47,118 @@ def setup
     assert_includes @dummy.organizations, taxonomies(:organization1)
     assert_empty @dummy.locations
   end
+
+  test ".with_taxonomy_scope expands organizations and locations to actual values" do
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization)
+    org3 = FactoryGirl.create(:organization)
+    user = FactoryGirl.create(:user, :organizations => [org1, org2])
+    dummy_class = @dummy.class
+
+    as_user(user) do
+      dummy_class.with_taxonomy_scope(nil, nil)
+    end
+
+    assert_empty dummy_class.which_location
+    assert_includes dummy_class.which_organization, org1
+    assert_includes dummy_class.which_organization, org2
+    refute_includes dummy_class.which_organization, org3
+  end
+
+  test ".used_location_ids can work with array of locations" do
+    loc1 = FactoryGirl.create(:location)
+    loc2 = FactoryGirl.create(:location, :parent_id => loc1)
+    loc3 = FactoryGirl.create(:location, :parent_id => loc2)
+    loc4 = FactoryGirl.create(:location)
+    dummy_class = @dummy.class
+    dummy_class.which_ancestry_method = :subtree_ids
+
+    dummy_class.which_location = []
+    used_locations = dummy_class.used_location_ids
+    assert_empty used_locations
+
+    dummy_class.which_location = [loc2]
+    used_locations = dummy_class.used_location_ids
+    assert_includes used_locations, loc1.id
+    assert_includes used_locations, loc2.id
+    assert_includes used_locations, loc3.id
+    refute_includes used_locations, loc4.id
+
+    dummy_class.which_location = [loc2, loc4]
+    used_locations = dummy_class.used_location_ids
+    assert_includes used_locations, loc1.id
+    assert_includes used_locations, loc2.id
+    assert_includes used_locations, loc3.id
+    assert_includes used_locations, loc4.id
+  end
+
+  test ".used_organization_ids can work with array of organizations" do
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization, :parent_id => org1)
+    org3 = FactoryGirl.create(:organization, :parent_id => org2)
+    org4 = FactoryGirl.create(:organization)
+    dummy_class = @dummy.class
+    dummy_class.which_ancestry_method = :subtree_ids
+
+    dummy_class.which_organization = []
+    used_organizations = dummy_class.used_organization_ids
+    assert_empty used_organizations
+
+    dummy_class.which_organization = [org2]
+    used_organizations = dummy_class.used_organization_ids
+    assert_includes used_organizations, org1.id
+    assert_includes used_organizations, org2.id
+    assert_includes used_organizations, org3.id
+    refute_includes used_organizations, org4.id
+
+    dummy_class.which_organization = [org2, org4]
+    used_organizations = dummy_class.used_organization_ids
+    assert_includes used_organizations, org1.id
+    assert_includes used_organizations, org2.id
+    assert_includes used_organizations, org3.id
+    assert_includes used_organizations, org4.id
+  end
+
+  test ".taxable_ids can work with empty array returning nil" do
+    dummy_class = @dummy.class
+    assert_nil dummy_class.taxable_ids([], [])
+  end
+
+  test ".taxable_ids (and .inner_select) can work with array of taxonomies" do
+    loc1 = FactoryGirl.create(:location)
+    loc2 = FactoryGirl.create(:location, :parent_id => loc1)
+    loc3 = FactoryGirl.create(:location, :parent_id => loc2)
+    loc4 = FactoryGirl.create(:location)
+    org = FactoryGirl.create(:organization)
+    env1 = FactoryGirl.create(:environment, :organizations => [org], :locations => [loc2])
+    env2 = FactoryGirl.create(:environment, :organizations => [org])
+    env3 = FactoryGirl.create(:environment, :locations => [loc2])
+    env4 = FactoryGirl.create(:environment, :locations => [loc4])
+    env5 = FactoryGirl.create(:environment, :locations => [loc1])
+    env6 = FactoryGirl.create(:environment, :locations => [loc3])
+
+    taxable_ids = Environment.taxable_ids([loc2, loc4], org, :subtree_ids)
+    visible = [ env1 ]
+    invisible = [ env2, env3, env4, env5, env6 ]
+    visible.each { |env| assert_includes taxable_ids, env.id }
+    invisible.each { |env| refute_includes taxable_ids, env.id }
+
+    taxable_ids = Environment.taxable_ids([], org, :subtree_ids)
+    visible = [ env1, env2 ]
+    invisible = [ env3, env4, env5, env6 ]
+    visible.each { |env| assert_includes taxable_ids, env.id }
+    invisible.each { |env| refute_includes taxable_ids, env.id }
+
+    taxable_ids = Environment.taxable_ids(loc2, [], :subtree_ids)
+    visible = [ env1, env3, env5, env6 ]
+    invisible = [ env2, env4 ]
+    visible.each { |env| assert_includes taxable_ids, env.id }
+    invisible.each { |env| refute_includes taxable_ids, env.id }
+
+    taxable_ids = Environment.taxable_ids([loc2, loc4], [], :subtree_ids)
+    visible = [ env1, env3, env4, env5, env6 ]
+    invisible = [ env2 ]
+    visible.each { |env| assert_includes taxable_ids, env.id }
+    invisible.each { |env| refute_includes taxable_ids, env.id }
+  end
 end

test/unit/taxonomy_test.rb

@@ -18,4 +18,64 @@ def setup
   test '.organizations_enabled' do
     assert Taxonomy.organizations_enabled
   end
+
+  test 'expand return [] for admin if no taxonomy set' do
+    as_admin do
+      assert_empty Taxonomy.expand(nil)
+    end
+  end
+
+  test 'expand return [] for admin if empty set of taxonomies set' do
+    as_admin do
+      assert_empty Taxonomy.expand([])
+    end
+  end
+
+  test 'expand return the specified taxonomy for admin' do
+    org = FactoryGirl.build(:organization)
+    as_admin do
+      assert_equal org, Taxonomy.expand(org)
+    end
+  end
+
+  test 'does not expand if no user set' do
+    org1 = FactoryGirl.build(:organization)
+    org2 = FactoryGirl.build(:organization)
+    assert_equal nil, Taxonomy.expand(nil)
+    assert_equal [], Taxonomy.expand([])
+    assert_equal org1, Taxonomy.expand(org1)
+    assert_equal [org1, org2], Taxonomy.expand([org1, org2])
+  end
+
+  test 'for non admin user, nil is expanded to user assigned taxonomies' do
+    # we have to run on specific taxonomy because my_* is defined only in Organization and Location
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization)
+    FactoryGirl.create(:organization) # this one won't be expanded
+    user = FactoryGirl.create(:user, :organizations => [org1, org2])
+    as_user(user) do
+      assert_equal [org1, org2], Organization.expand(nil)
+      assert_equal [org1, org2], Organization.expand([])
+    end
+  end
+
+  test 'for non admin user, nil is expanded to [] if user is not assigned to any org' do
+    # we have to run on specific taxonomy because my_* is defined only in Organization and Location
+    user = FactoryGirl.create(:user)
+    as_user(user) do
+      assert_equal [], Organization.expand(nil)
+      assert_equal [], Organization.expand([])
+    end
+  end
+
+  test 'for non admin user, expand return the specified taxonomy' do
+    # we have to run on specific taxonomy because my_* is defined only in Organization and Location
+    org1 = FactoryGirl.create(:organization)
+    org2 = FactoryGirl.create(:organization)
+    user = FactoryGirl.create(:user, :organizations => [org1, org2])
+    as_user(user) do
+      assert_equal org1, Organization.expand(org1)
+      assert_equal [org1, org2], Organization.expand([org1, org2])
+    end
+  end
 end