From 1f7e4c8b6be22e81b281625d1a014b6d83735211 Mon Sep 17 00:00:00 2001 From: Sergio Arbeo Date: Thu, 4 Oct 2012 19:52:28 +0200 Subject: [PATCH] Solves problem when authorizing new action. Given two models Category and Projects. A Category has_many projects and Project belongs_to a category. Furthermore, projects are shallow nested resources in a category. Let's say that a user can edit certain category's projects (and only one category can be edited by each user [1]), this is expressed with the following line in Ability model: can :new, :projects, category_id: user.category_id Given the old implementation, we get that any user can 'new' (though not 'create') a project in any category: ```ruby def assign_attributes(resource) resource.send("#{parent_name}=", parent_resource) if @options[:singleton] && parent_resource initial_attributes.each do |attr_name, value| resource.send("#{attr_name}=", value) end resource end ``` In this case, category_id in project would get overwritten inside the initial_attributes loop and authorization would pass. I consider this a buggy behaviour. [1] User belongs_to a category, and a Category has many users. On the other hand, there might be users without any category. Conflicts: spec/cancan/controller_resource_spec.rb --- lib/cancan/controller_resource.rb | 3 ++- spec/cancan/controller_resource_spec.rb | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/cancan/controller_resource.rb b/lib/cancan/controller_resource.rb index 65a2240..0a8d6f6 100644 --- a/lib/cancan/controller_resource.rb +++ b/lib/cancan/controller_resource.rb @@ -85,10 +85,11 @@ module CanCan end def assign_attributes(resource) - resource.send("#{parent_name}=", parent_resource) if @options[:singleton] && parent_resource initial_attributes.each do |attr_name, value| resource.send("#{attr_name}=", value) end + + resource.send("#{parent_name}=", parent_resource) if @options[:singleton] && parent_resource resource end diff --git a/spec/cancan/controller_resource_spec.rb b/spec/cancan/controller_resource_spec.rb index 70d6bcc..9c9fc4c 100644 --- a/spec/cancan/controller_resource_spec.rb +++ b/spec/cancan/controller_resource_spec.rb @@ -284,10 +284,26 @@ describe CanCan::ControllerResource do CanCan::ControllerResource.new(@controller, :category, :load => true).process CanCan::ControllerResource.new(@controller, :project, :load => true, :through => :category).process project = @controller.instance_variable_get(:@project) - project.new_record?.should eq(true) + project.should be_new_record project.name.should eq('foo') end + it "does not overrides an attribute if it is based on parent resource" do + user = double('user') + user.should_receive(:category_id).and_return nil + @ability = Ability.new user + @ability.can :new, :projects, :category_id => user.category_id + category = Category.create! + @controller.instance_variable_set(:@category, category) + + @params.merge! :action => 'new', :category_id => category.id + CanCan::ControllerResource.new(@controller, :category, :load => true).process + CanCan::ControllerResource.new(@controller, :project, :load => true, :through => :category, :singleton => true).process + project = @controller.instance_variable_get(:@project) + project.category_id.should_not be_nil + project.category.should eq(category) + end + it "authorizes nested resource through parent association on index action" do pending @params.merge!(:action => "index")