ryanb / cancan

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

Combining conditionless & sql/block rules #685

Open hungrything opened 12 years ago

hungrything commented 12 years ago
cannot :read, Foo
can :read, Foo, ["bar_id = ?", user.bar_id] do |foo|
  foo.bar_id == user.bar_id
end

yields undefined method '%' for ["bar_id = ?", 2]

while

cannot :read, Foo, "true" do |foo|
  true
end
can :read, Foo, ["bar_id = ?", user.bar_id] do |foo|
  foo.bar_id == user.bar_id
end

succeeds.

cancan 1.6.8, activerecord 3.2.6

swobspace commented 12 years ago

I have a similar bug with 1.6.8 and 3.2.6 (see #704). Can you try downgrade to 1.6.7 and check it again? 1.6.7 works for me.

Wolfgang

fillman commented 12 years ago

Passing conditions with a block is not a good way to declare rules in cancan. What do you really expect from it?

module CanCan
  # This class is used internally and should only be called through Ability.
  # it holds the information about a "can" call made on Ability and provides
  # helpful methods to determine permission checking and conditions hash generation.
  class Rule # :nodoc:

    ...

    def initialize(base_behavior, action, subject, conditions, block)
      raise Error, "You are not able to supply a block with a hash of conditions in #{action} #{subject} ability. Use either one." if conditions.kind_of?(Hash) && !block.nil?
      ...
    end
hungrything commented 12 years ago

@fillman I'm not sure what you mean. Your code quote is about hash conditions with a block, which is a stated limitation of cancan. Thus we can only pass sql conditions with a block. This has a clear intended use case: Fetching a set of readable objects for display, then determining other allowed actions per item, while iterating. I ran into this using a fairly plain rails_admin setup.

@swobspace yup, 1.6.7 works. could be something that changed around the truthiness sql, 't'='t'. I just use true in my workaround... hmm...

psteininger commented 11 years ago

I'm running into the same issue currently. I have:

can :read, Location
can :manage, Location do |location|
   user.role? :curator, location.city
end

The problem seems to be with this controler_resource.rb:

 def load_collection?
    resource_base.respond_to?(:accessible_by) && !current_ability.has_block?(authorization_action, resource_class)
 end

this means that if I have a block defined then I cannot load a collection. This seems a bit restrictive. I'll try to get a better logic in place and submit a PR

psteininger commented 11 years ago

The issue is caused by an overlap of rules for one action (in this case :index). If I have:

can :read, Location
can :manage, Location do |location|
   user.role? :curator, location.city
end

then :manage includes :index. I tried modifying the gem, but it felt like the fix may affect other areas. So here is a viable workaround:

can :manage, Location
cannot [:create, :update, :destroy], Location do |location|
   !user.role? :curator, location.city
end

basically I add all permissions, but exclude the ones that are only available to :curators. I hope this helps someone in need :).

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!