commit 08d4fc31447487181f59e7be54121fc681f3a5b4
Author: Ori Rabin <orabin@redhat.com>
Date:   Sun Jul 26 15:13:54 2015 +0300

    Fixes #11188 - EnsureNotUsedBy checks for unscoped associations before destroying

diff --git a/app/helpers/layout_helper.rb b/app/helpers/layout_helper.rb
index c1c3a50a7..7dd55f141 100644
--- a/app/helpers/layout_helper.rb
+++ b/app/helpers/layout_helper.rb
@@ -86,7 +86,7 @@ module LayoutHelper
   # add hidden field for options[:disabled]
   def multiple_selects(f, attr, associations, selected_ids, options = {}, html_options = {})
     options.merge!(:size => "col-md-10")
-    authorized = authorized_associations(associations).all
+    authorized = AssociationAuthorizer.authorized_associations(associations).all
     unauthorized = selected_ids.blank? ? [] : selected_ids - authorized.map(&:id)
     field(f, attr, options) do
       attr_ids = (attr.to_s.singularize+"_ids").to_sym
@@ -443,26 +443,6 @@ module LayoutHelper
 
   private
 
-  def authorized_associations(associations)
-    if associations.included_modules.include?(Authorizable)
-      if associations.respond_to?(:klass)
-        associations.authorized(authorized_associations_permission_name(associations.klass), associations.klass)
-      else
-        associations.authorized(authorized_associations_permission_name(associations), associations)
-      end
-    else
-      associations
-    end
-  end
-
-  def authorized_associations_permission_name(klass)
-    permission = "view_#{klass.to_s.underscore.pluralize}"
-    unless Permission.where(:name => permission).present?
-      raise Foreman::Exception.new(N_('unknown permission %s'), permission)
-    end
-    permission
-  end
-
   def table_css_classes(classes = '')
     "table table-bordered table-striped table-condensed " + classes
   end
diff --git a/app/models/smart_proxy.rb b/app/models/smart_proxy.rb
index 4536ae0fc..aed7ef4d8 100644
--- a/app/models/smart_proxy.rb
+++ b/app/models/smart_proxy.rb
@@ -8,7 +8,7 @@ class SmartProxy < ActiveRecord::Base
 
   attr_accessible :name, :url, :location_ids, :organization_ids
   validates_lengths_from_database
-  before_destroy EnsureNotUsedBy.new(:hosts, :hostgroups, :subnets, :domains, :puppet_ca_hosts, :puppet_ca_hostgroups, :realms)
+  before_destroy EnsureNotUsedBy.new(:hosts, :hostgroups, :subnets, :domains, [:puppet_ca_hosts, :hosts], [:puppet_ca_hostgroups, :hostgroups], :realms)
   #TODO check if there is a way to look into the tftp_id too
   # maybe with a predefined sql
   has_and_belongs_to_many :features
diff --git a/app/models/user.rb b/app/models/user.rb
index 14053f967..d88ebccc7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -17,7 +17,7 @@ class User < ActiveRecord::Base
   attr_protected :password_hash, :password_salt, :admin
   attr_accessor :password, :password_confirmation
   after_save :ensure_default_role
-  before_destroy EnsureNotUsedBy.new(:direct_hosts), :ensure_hidden_users_are_not_deleted, :ensure_last_admin_is_not_deleted
+  before_destroy EnsureNotUsedBy.new([:direct_hosts, :hosts]), :ensure_hidden_users_are_not_deleted, :ensure_last_admin_is_not_deleted
 
   belongs_to :auth_source
   belongs_to :default_organization, :class_name => 'Organization'
diff --git a/app/services/association_authorizer.rb b/app/services/association_authorizer.rb
new file mode 100644
index 000000000..9247fa1a3
--- /dev/null
+++ b/app/services/association_authorizer.rb
@@ -0,0 +1,26 @@
+class AssociationAuthorizer
+  def self.authorized_associations(associations,  klass_name = nil, should_raise_exception = true)
+    if associations.included_modules.include?(Authorizable)
+      associations_klass = associations
+      if associations.respond_to?(:klass)
+        associations_klass = associations.klass
+      end
+      klass_name ||= associations_klass
+      permission = view_permission_name(klass_name, should_raise_exception)
+      associations.authorized(permission, associations_klass) if permission
+    else
+      associations
+    end
+  end
+
+  def self.view_permission_name(klass, should_raise_exception)
+    permission = "view_#{klass.to_s.underscore.pluralize}"
+    if Permission.where(:name => permission).present?
+      permission
+    elsif should_raise_exception
+      raise Foreman::Exception.new(N_('unknown permission %s'), permission)
+    else
+      false
+    end
+  end
+end
diff --git a/lib/core_extensions.rb b/lib/core_extensions.rb
index 731d9d6d3..280d58683 100644
--- a/lib/core_extensions.rb
+++ b/lib/core_extensions.rb
@@ -38,10 +38,19 @@ class ActiveRecord::Base
     end
 
     def before_destroy(record)
-      klasses.each do |klass|
-        record.send(klass.to_sym).each do |what|
-          what = what.to_label unless what.is_a? String
-          record.errors.add :base, _("%{record} is used by %{what}") % { :record => record, :what => what }
+      klasses.each do |klass, klass_name = klass|
+        record.association(klass.to_sym).association_scope.each do |what|
+          error_message = _("%{record} is used by %{what}")
+          unless what.is_a? String
+            authorized_associations = AssociationAuthorizer.authorized_associations(record.class.reflect_on_association(klass.to_sym).klass, klass_name, false)
+            if !authorized_associations.respond_to?(:to_a) || authorized_associations.to_a.include?(what)
+              what = what.to_label
+            else
+              what = _(what.class.name)
+              error_message = _("%{record} is being used by a hidden %{what} resource")
+            end
+          end
+          record.errors.add :base, error_message % { :record => record, :what => what }
         end
       end
       if record.errors.empty?
diff --git a/test/unit/association_authorizer_test.rb b/test/unit/association_authorizer_test.rb
new file mode 100644
index 000000000..698471237
--- /dev/null
+++ b/test/unit/association_authorizer_test.rb
@@ -0,0 +1,43 @@
+require 'test_helper'
+
+class AssociationAuthorizerTest < ActiveSupport::TestCase
+  def setup
+    @hostgroup = FactoryGirl.create(:hostgroup)
+    @host = FactoryGirl.create(:host, :managed, :hostgroup => @hostgroup)
+    @user = FactoryGirl.create(:user)
+  end
+
+  test "user with permissions can view host" do
+    role = FactoryGirl.create(:role, :name => 'can_view_host')
+    role.add_permissions!(['view_hosts'])
+    @user.update_attribute :roles, [role]
+
+    as_user @user do
+      authorized = AssociationAuthorizer.authorized_associations(Hostgroup.reflect_on_association(:hosts).klass, :hosts)
+      assert authorized.include?(@host)
+    end
+  end
+
+  test "user without permissions can't view host" do
+    as_user @user do
+      authorized = AssociationAuthorizer.authorized_associations(Hostgroup.reflect_on_association(:hosts).klass, :hosts)
+      refute authorized.include?(@host)
+    end
+  end
+
+  test "authorized_associations should raise unknown permission exception when should_raise_exception is true" do
+    assert_raise(Foreman::Exception) do
+      AssociationAuthorizer.view_permission_name('non_existing_permission', true)
+    end
+  end
+
+  test "authorized_associations should return false for unknown permission when should_raise_exception is false" do
+    permission = AssociationAuthorizer.view_permission_name('non_existing_permission', false)
+    assert_equal false, permission
+  end
+
+  test "authorized_associations should return permission if it exists" do
+    permission = AssociationAuthorizer.view_permission_name(:host, false)
+    assert_equal "view_hosts", permission
+  end
+end
diff --git a/test/unit/ensure_not_used_by_test.rb b/test/unit/ensure_not_used_by_test.rb
new file mode 100644
index 000000000..0c85e0f13
--- /dev/null
+++ b/test/unit/ensure_not_used_by_test.rb
@@ -0,0 +1,68 @@
+require 'test_helper'
+
+class EnsureNotUsedByTest < ActiveSupport::TestCase
+  def setup
+    @org1 = FactoryGirl.create(:organization)
+    @org2 = FactoryGirl.create(:organization)
+    @user = FactoryGirl.build(:user, :with_mail, :organizations => [@org1])
+    @user.save(:validate => false)
+    role = FactoryGirl.create(:role, :name => 'can_view_host')
+    role.add_permissions!(['view_hosts'])
+    @user.update_attribute :roles, [role]
+  end
+
+  test "hostgroup should not be deleted if used by host in user org" do
+    hostgroup = FactoryGirl.create(:hostgroup, :organizations => [@org1, @org2])
+    host = FactoryGirl.create(:host, :managed, :hostgroup => hostgroup, :organization => @org1)
+
+    as_user @user do
+      in_taxonomy @org1 do
+        refute hostgroup.destroy
+        assert_equal "#{hostgroup.name} is used by #{host.name}", hostgroup.errors.full_messages.first
+      end
+    end
+  end
+
+  test "hostgroup should not be deleted if used by host in different org" do
+    hostgroup = FactoryGirl.create(:hostgroup, :organizations => [@org1, @org2])
+    FactoryGirl.create(:host, :hostgroup => hostgroup, :organization => @org2)
+
+    as_user @user do
+      in_taxonomy @org1 do
+        refute hostgroup.destroy
+        assert_equal "#{hostgroup.name} is being used by a hidden Host::Managed resource", hostgroup.errors.full_messages.first
+      end
+    end
+  end
+
+  test "hostgroup should not be deleted if used by host" do
+    hostgroup = FactoryGirl.create(:hostgroup, :organizations => [@org1, @org2])
+    FactoryGirl.create(:host, :hostgroup => hostgroup, :organization => @org2)
+
+    as_user  FactoryGirl.create(:user, :with_mail) do
+      in_taxonomy @org1 do
+        refute hostgroup.destroy
+        assert_equal "#{hostgroup.name} is being used by a hidden Host::Managed resource", hostgroup.errors.full_messages.first
+      end
+    end
+  end
+
+  test "hostgroup should be deleted if not used by host" do
+    hostgroup = FactoryGirl.create(:hostgroup, :organizations => [@org1, @org2])
+    FactoryGirl.create(:host, :organization => @org2)
+
+    as_user @user do
+      assert hostgroup.destroy
+    end
+  end
+
+  test "host using hostgroup should not be shown to user without permissions" do
+    hostgroup = FactoryGirl.create(:hostgroup)
+    FactoryGirl.create(:host, :managed, :hostgroup => hostgroup)
+
+    as_user  FactoryGirl.create(:user, :with_mail) do
+      refute hostgroup.destroy
+      assert_equal "#{hostgroup.name} is being used by a hidden Host::Managed resource", hostgroup.errors.full_messages.first
+    end
+  end
+end
