can :read, :all followed by a can :read, Controller, :some_condition.

[mkrogh]

Hi
I'm having a problem with the :all option.

When I have the following ability setup:

can :read, :all
can :manage, SomeController, :user_id => user.id
can :read, AnotherController, :some_condition => 0..2

Then cancan will short circuit (or at least not "respect" the last :read).
Using the debug method described in https://github.com/ryanb/cancan/wiki/Debugging-Abilities cancan says that all AnotherController records can be read.

If I remove the first can :read, :all, then cancan works as expected. So it seems like the :all is way to greedy...

Is this a bug, or do I really have to specify a can :read for all my individual controllers?

(Running ruby 1.9.2 and cancan 1.5.1)

[ryanb]

The can calls are additive, so if you are granting can :read, :all permission then adding further can calls will only grant additional behavior. The can call never takes away behavior. For that you can use cannot, but it doesn't look like the negative context makes sense here. The best current solution is to mention each other class in the first line.

can :read, [Article, Comment, Product, ...]

The main reason for this behavior is so you can accomplish "or" conditions easily. For example if you want someone to read either projects he owns or projects which are public you could do this:

can :read, Project, :user_id => user.id
can :read, Project, :public => true

But I see your point here, this does not make as much sense for the :all keyword, it would be better if there were ways to override. I'm marking this for further discussion and will consider changing this in CanCan 2.0.

[ryanb]

Hmm, thinking about it some more, the cannot method can be used here like this.

can :read, :all
cannot :read, Project
can :manage, Project, :user_id => user.id

I will still consider changing the behavior of :all in CanCan 2.0, but see if that works for now.

[mkrogh]

Ahh I see thanks for the answer.

I totally missed that they are additive, which makes sense for the scenario you describe.

It can as it is be solved using:
cannot :read, Controller, ["condition_field > ?", user.upper_bound] do |controller|
controller.condition_field > user.upper_bound
end

(Needed to dig up: https://github.com/ryanb/cancan/wiki/Defining-Abilities-with-Blocks)

But it would be quite nice, if the :all keyword was just a catch all.

[ryanb]

I've been thinking about this, and I think it more has to do with the conditions and not the :all keyword. For example, what if we did this.

can :read, Project
can :read, Project, :user_id => user.id

Here you would assume the second definition would override the first. So I think I need to treat conditions a little special. If a can? check matches a rule with a condition, it should then only match other rules where conditions are applied. This would get the behavior you expect above while still maintaining the additive functionality I mentioned earlier.

Same goes for the new attribute additions in CanCan 2.0. If one doesn't define attributes, but later does, it should override it.

can :update, :projects
can :update, :projects, [:name, :price]

Here you would think the name/price would override the earlier definition since it's more defined. I'll mark this for being added to CanCan 2.0.

[mkrogh]

Hmm that would make sense. This would mean something like three levels of "specificity" (or two if can :read, :all is meant to be exactly the same as can :read, Project).

I presume that the following would be equivalent:

 can :read, Project, :user_id => user.id
 can :read, Project

And:

 can :read, Project
 can :read, Project, :user_id => user.id

Back to the suggestion, as you said it would allow one to define a :all block, and later trump it with a more specific rule. However I am wondering if it would make sense also to add an except condition like so:

 can :read, :all, :except => [Project] 

This would allow for the behavior in the original "issue", and might be practical in some cases.

[ryanb]

To stay consistent with the current cannot behavior it would need to be order dependent, so in this case.

can :read, Project, :user_id => user.id
can :read, Project

Would not reach the top condition because it starts on the bottom and stops at the first passing rule it finds. This allows you to override rules if you have multiple roles.

if user.role? :moderator
  can :read, Project, :user_id => user.id
end
if user.role? :admin
  can :access, :all
end

I'm open to reconsidering how this works, ultimately I want to find the most intuitive behavior.

Either way, I will be encouraging testing the Ability class more in 2.0 because authorization needs to be tight and secure, and complex rules like this need to be well tested.

As for the :except option, I can't add this because it conflicts with the current use of hashes for conditions. You can use cannot for this.

can :read, :all
cannot :read, Project

[mkrogh]

Ahh ok,
And with that comment I finally realized how the rules are applied. I was just about to ask how the rules then could be additive, but then it hit me that, as you state, it just accepts the first authorization rule that passes.

As for the can + cannot vs :except, no worries it was just a suggestion.

[ryanb]

There's more information about how this works on the Ability Precedence page. I will be expanding that page to mention the hash conditions changes we talked about here once CanCan 2.0 is out.