ryanb / cancan

Authorization Gem for Ruby on Rails.
MIT License
6.26k stars 784 forks source link

can with a block will execute the action on the class as well? #782

Open denniss opened 11 years ago

denniss commented 11 years ago

def initialize(staff) staff ||= Staff.new can :manage, Store do |store| store.staff_privileges.select(&:owner?).map(&:staff_id).include? staff.id end end

I am not sure why staff.can? :manage would return true here because I thought the above block should only get executed on the instance of store and not on the class itself

staff = Staff.first staff.can? :manage, Store #true staff.can? :manage, Store.first #false, because there is no staff_privileges associated to this store

the8472 commented 11 years ago

When executed on a class you can read it as the following: "might this user be able to manage some random instance of this class, not necessarily all instances?"

E.g. if you provide a create-button then the user might be able to create something in general, but is banned from assigning specific values, thus might fail when the instance is checked.

petersooley commented 11 years ago

I'm as confused as @denniss.

Why is this:

can :manage, Store

the same as this:

can :manage, Store do |store|
    # check instance
end

when it comes to running can? on a class and not an instance?

I should be able to authorize instance actions without authorizing class actions carte blanche.

I understand that there is a difference between authorizing a class and an instance when using can?. I'm just wondering why I can't define abilities for instance variables without also defining them at the class level.

In any case, the point will be moot by CanCan 2.0. But until then, has anyone figured out a workaround?

(And amen to @ryanb for making cancan.)

the8472 commented 11 years ago

I'm just wondering why I can't define abilities for instance variables without also defining them at the class level.

Because you can't check them at the class level... you don't have an instance after all.

Maybe you can think of it as similar to the .any? test on enumerations.

camallen commented 11 years ago

Hi @psoots, @denniss and @the8472,

I was surprised to find the blocks ability assignment adds the class level permissions even when there is no instance to evaluate!

I seem to have worked around the issue by explicitly adding a whitelist testing the instance and blacklist testing the class _in that order_. The whitelist instance check will add explicit instance level permission (if the instance exists and the block returns true) whereas the blacklist will add a class level permission which overrides the whitelist, thereby adding the required instance and class level permissions

This seems to be working for me, YMMV.

Some code using your example above (note the last ability will override the first):

_whitelist:_

def owner_ids
  store.staff_privileges.select(&:owner?).map(&:staff_id)
end

can :manage, Store do |store|
  owner_ids.include? staff.id
end 

and

_blacklist:_

cannot :manage, Store do |store|
  !(owner_ids.include? staff.id)
end 
xhoy commented 10 years ago

Thanks for your submission! The ryanb/cancan repository has been inactive since Sep 06, 2013. Since only Ryan himself has commit permissions, the CanCan project is on a standstill.

CanCan has many open issues, including missing support for Rails 4. To keep CanCan alive, an active fork exists at cancancommunity/cancancan. The new gem is cancancan. More info is available at #994.

If your pull request or issue is still applicable, it would be really appreciated if you resubmit it to CanCanCan.

We hope to see you on the other side!