stffn / declarative_authorization

An unmaintained authorization plugin for Rails. Please fork to support current versions of Rails
MIT License
1.24k stars 233 forks source link

Authorization::AuthorizationUsageError with ActiveRecord::Associations::CollectionProxy #193

Closed igor04 closed 10 years ago

igor04 commented 10 years ago

Seems like in rails > 4.0.2 where is ActiveRecord::Associations::CollectionProxy we have issue with authorization (with rails rails ~ 3.2 all is fine)

Message:

Authorization::AuthorizationUsageError: Error occurred while validating
attribute #user on #<ActiveRecord::Associations::CollectionProxy[]>: 
undefined method `user' for #<ActiveRecord::Associations::CollectionProxy[]>. 

Please check your authorization rules and ensure the attribute is correctly
spelled and corresponds to a method on the model you are authorizing for.

Rule like: if_attribute :object => {:collection => {:user => is { user } }}

In Requirements i found Rails >= 2.2, including 3 .. are you planing support rails 4?

stffn commented 10 years ago

Are you sure that it works on Rails 3? Shouldn't it be "is { current_user }"?

igor04 commented 10 years ago

in Requirements and Examples "is { user }" - the same as current_user in controllers

yes, in Rails 3 it works, but in Rails 4 it doesn't, and I think that's because: in 3 we have object.collection.class - Array in 4 it is object.collection.class - ActiveRecord::Associations::CollectionProxy

bwlang commented 10 years ago

I think I also have the same issue. this works for me with rails 4.0.0

if_attribute :roles => intersects_with { user.roles }

but with rails 4.0.1-4.0.4 invalid sql is generated:

PG::UndefinedParameter: ERROR: there is no parameter $1 LINE 1: ...r_roles"."role_id" WHERE "user_roles"."user_id" = $1)) OR (1...

igor04 commented 10 years ago

I resolved this issue for myself with creating scope method and used it in authorization rule

instead of:

if_attribute :object => {:collection => {:user => is { user } }}

replaced to:

if_attribute :object => {:collection => {:user_ids =>  contain { user.id } }}

where user_ids is scope like pluck :user_id

maletor commented 10 years ago

This is great. @igor04's fix works for me. Would love to see first class support for Rails 4 though.

zeiv commented 10 years ago

@igor04, would you mind posting the full trace, if you can? I've added a couple tests for this and have it working in Rails 4.0.2, but it's still breaking in 4.1.4.

zeiv commented 10 years ago

This test is now passing through Rails 4.1.4, does this cover what you were trying to do?

 def test_with_nested_has_many
   reader = Authorization::Reader::DSLReader.new
   reader.parse %{
     authorization do
       role :test_role do
         has_permission_on :companies, :to => :read do
           if_attribute :branches => { :test_attrs => { :attr => is { user.test_attr_value } } }
         end
       end
     end
   }
   Authorization::Engine.instance(reader)

   allowed_company = Company.create!
   allowed_company.branches.create!.test_attrs.create!(:attr => 1)
   allowed_company.branches.create!.test_attrs.create!(:attr => 2)

   prohibited_company = Company.create!
   prohibited_company.branches.create!.test_attrs.create!(:attr => 3)

   user = MockUser.new(:test_role, :test_attr_value => 1)
   prohibited_user = MockUser.new(:test_role, :test_attr_value => 4)
   assert_equal 1, Company.with_permissions_to(:read, :user => user).length
   assert_equal 0, Company.with_permissions_to(:read, :user => prohibited_user).length

   Company.delete_all
   Branch.delete_all
   TestAttr.delete_all
 end
igor04 commented 10 years ago

@zeiv yes, it looks like this test covers it, the problem was with checking attribute from collection, because collection was accepted as object.

I think, it also will be good leave some Company with empty branches or test_attrs for covering behaviour for empty collection