https://www.aeolusproject.org/redmine/issues/3158
Use the new derived_permissions model for permission checks -- remove no-longer-needed
helper methods that added in the joins for the non-denormalized model.
---
src/app/models/catalog.rb | 17 --------
src/app/models/deployable.rb | 20 ----------
src/app/models/deployment.rb | 17 --------
src/app/models/instance.rb | 20 ----------
src/app/models/permissioned_object.rb | 41 +++++++------------
src/app/models/pool.rb | 15 -------
src/app/models/provider_account.rb | 14 -------
.../controllers/deployables_controller_spec.rb | 2 +
.../controllers/deployments_controller_spec.rb | 2 +-
9 files changed, 18 insertions(+), 130 deletions(-)
diff --git a/src/app/models/catalog.rb b/src/app/models/catalog.rb
index 5ab51c4..4ea3df1 100644
--- a/src/app/models/catalog.rb
+++ b/src/app/models/catalog.rb
@@ -62,23 +62,6 @@ class Catalog < ActiveRecord::Base
def self.additional_privilege_target_types
[Deployable]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [ {:pool => :permissions},
- {:pool_family => :permissions} ]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_pools.user_id=:user and
- permissions_pools.role_id in (:role_ids)) or
- (permissions_pool_families.user_id=:user and
- permissions_pool_families.role_id in (:role_ids))"
- end
PRESET_FILTERS_OPTIONS = [
{:title => "catalogs.preset_filters.belongs_to_default_pool", :id =>
"belongs_to_default_pool", :query =>
includes(:pool).where("pools.name" => "Default")}
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index 02c53ff..201123f 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -56,26 +56,6 @@ class Deployable < ActiveRecord::Base
def perm_ancestors
super + catalogs + catalogs.collect{|c| c.pool}.uniq + [pool_family]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [ {:catalogs => [:permissions,
- {:pool => :permissions}]},
- {:pool_family => :permissions} ]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_catalogs.user_id=:user and
- permissions_catalogs.role_id in (:role_ids)) or
- (permissions_pools.user_id=:user and
- permissions_pools.role_id in (:role_ids)) or
- (permissions_pool_families.user_id=:user and
- permissions_pool_families.role_id in (:role_ids))"
- end
def valid_deployable_xml?
begin
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 559f027..2b01d63 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -133,23 +133,6 @@ class Deployment < ActiveRecord::Base
def self.additional_privilege_target_types
[Instance]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [ {:pool => :permissions},
- {:pool_family => :permissions} ]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_pools.user_id=:user and
- permissions_pools.role_id in (:role_ids)) or
- (permissions_pool_families.user_id=:user and
- permissions_pool_families.role_id in (:role_ids))"
- end
def get_action_list(user=nil)
# FIXME: how do actions and states interact for deployments?
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 84f1224..8258118 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -150,26 +150,6 @@ class Instance < ActiveRecord::Base
ancestors << deployment unless deployment.nil?
ancestors += [pool, pool_family]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [ {:deployment => :permissions},
- {:pool => :permissions},
- {:pool_family => :permissions} ]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_deployments.user_id=:user and
- permissions_deployments.role_id in (:role_ids)) or
- (permissions_pools.user_id=:user and
- permissions_pools.role_id in (:role_ids)) or
- (permissions_pool_families.user_id=:user and
- permissions_pool_families.role_id in (:role_ids))"
- end
def get_action_list(user=nil)
# return empty list rather than nil
diff --git a/src/app/models/permissioned_object.rb
b/src/app/models/permissioned_object.rb
index 3f083ea..27f28e1 100644
--- a/src/app/models/permissioned_object.rb
+++ b/src/app/models/permissioned_object.rb
@@ -19,26 +19,20 @@ module PermissionedObject
def has_privilege(user, action, target_type=nil)
return false if user.nil? or action.nil?
target_type = self.class.default_privilege_target_type if target_type.nil?
- perm_ancestors.each do |obj|
- return true if obj and obj.permissions.find(:first,
- :include => [:role => :privileges],
- :conditions =>
- ["permissions.user_id=:user and
- privileges.target_type=:target_type and
- privileges.action=:action",
- { :user => user.id,
- :target_type => target_type.name,
- :action => action}])
- end
- return false
+ derived_permissions.includes(:role => :privileges).where(
+ ["derived_permissions.user_id=:user and
+ privileges.target_type=:target_type and
+ privileges.action=:action",
+ { :user => user.id,
+ :target_type => target_type.name,
+ :action => action}]).any?
end
# Returns the list of objects to check for permissions on -- by default
# this object plus the Base permission object
# At the moment, the retuned list is not necessarily ordered
- # FIXME: remove self once has_privilege stops using this method
def perm_ancestors
- [self, BasePermissionObject.general_permission_scope]
+ [BasePermissionObject.general_permission_scope]
end
# Returns the list of objects to generate derived permissions for
# -- by default just this object
@@ -101,23 +95,18 @@ module PermissionedObject
def self.default_privilege_target_type
self
end
- def self.list_for_user_include
- [:permissions]
- end
- def self.list_for_user_conditions
- "permissions.user_id=:user and
- permissions.role_id in (:role_ids)"
- end
def self.list_for_user(user, action,
target_type=self.default_privilege_target_type)
return where("1=0") if user.nil? or action.nil? or target_type.nil?
if BasePermissionObject.general_permission_scope.has_privilege(user, action,
target_type)
scoped
else
- role_ids = Role.includes(:privileges).where("privileges.target_type"
=> target_type, "privileges.action" => action).collect {|r| r.id}
- include_clause = self.list_for_user_include
- conditions_hash = {:user => user.id, :target_type => target_type.name,
:action => action, :role_ids => role_ids}
- conditions_str = self.list_for_user_conditions
- includes(include_clause).where(conditions_str, conditions_hash)
+ includes([:derived_permissions => {:role => :privileges}]).
+ where("derived_permissions.user_id=:user and
+ privileges.target_type=:target_type and
+ privileges.action=:action",
+ {:user => user.id,
+ :target_type => target_type.name,
+ :action => action})
end
end
end
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 9110481..f8daa58 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -165,21 +165,6 @@ class Pool < ActiveRecord::Base
def self.additional_privilege_target_types
[Deployment, Instance, Catalog, Quota]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [ {:pool_family => :permissions} ]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_pool_families.user_id=:user and
- permissions_pool_families.role_id in (:role_ids))"
- end
-
private
diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
index 0ebc848..7fbc584 100644
--- a/src/app/models/provider_account.rb
+++ b/src/app/models/provider_account.rb
@@ -115,20 +115,6 @@ class ProviderAccount < ActiveRecord::Base
def self.additional_privilege_target_types
[Quota]
end
- class << self
- alias orig_list_for_user_include list_for_user_include
- alias orig_list_for_user_conditions list_for_user_conditions
- end
-
- def self.list_for_user_include
- orig_list_for_user_include + [{ :provider => :permissions}]
- end
-
- def self.list_for_user_conditions
- "(#{orig_list_for_user_conditions}) or
- (permissions_providers.user_id=:user and
- permissions_providers.role_id in (:role_ids))"
- end
def destroyable?
instances.empty? || instances.all? { |i| i.destroyable? }
diff --git a/src/spec/controllers/deployables_controller_spec.rb
b/src/spec/controllers/deployables_controller_spec.rb
index 3e08905..e952dbf 100644
--- a/src/spec/controllers/deployables_controller_spec.rb
+++ b/src/spec/controllers/deployables_controller_spec.rb
@@ -83,6 +83,7 @@ describe DeployablesController do
context "deletion successfully" do
before do
Deployable.any_instance.stub(:destroy).and_return(true)
+ Deployable.any_instance.stub(:has_privilege).and_return(true)
end
it "deletes a deployable and redirect to deployables#index" do
@@ -99,6 +100,7 @@ describe DeployablesController do
context "deletion fails" do
before do
Deployable.any_instance.stub(:destroy).and_return(false)
+ Deployable.any_instance.stub(:has_privilege).and_return(true)
end
it "not delete a deployable and redirect to deployables#show" do
diff --git a/src/spec/controllers/deployments_controller_spec.rb
b/src/spec/controllers/deployments_controller_spec.rb
index 54b1477..4629ad7 100644
--- a/src/spec/controllers/deployments_controller_spec.rb
+++ b/src/spec/controllers/deployments_controller_spec.rb
@@ -74,7 +74,7 @@ describe DeploymentsController do
describe "#destroy" do
before do
- @deployment = Factory.build(:deployment)
+ @deployment = Factory.create(:deployment)
Deployment.stub!(:find).and_return([@deployment])
delete :multi_destroy, :deployments_selected => [@deployment.id], :format
=> :json
end
--
1.7.6.5