Skip to content
This repository has been archived by the owner on Dec 12, 2021. It is now read-only.

cc2.0: Problems with multiple rules for same controller #727

Open
gamov opened this issue Aug 22, 2012 · 9 comments
Open

cc2.0: Problems with multiple rules for same controller #727

gamov opened this issue Aug 22, 2012 · 9 comments
Labels

Comments

@gamov
Copy link

gamov commented Aug 22, 2012

Some of my findings using cc2.0:
1 . Craps out with multiple rules going through the same nested association:

Sale.joins(:order_request => :business_site).where({:order_request => {:business_site => {:business_id => 12}}} | {:order_request => {:business_site => {:name => '3'}}} )

works fine in the console (Sale belong_to OrderRequest belong_to BusinessSite belong_to Business)
When I write the follow rule:
can :access, :sales, {:order_request => {:business_site => {:business_id => 3}}}
Sales index display fine but if I duplicate the rule (or add any other rules referencing :business_site, I get the following malformed SQL:

SELECT "sales".* FROM "sales" INNER JOIN "order_requests" ON "order_requests"."id" = "sales"."order_request_id" INNER JOIN "business_sites" ON "business_sites"."id" = "order_requests"."business_site_id" WHERE (("order_requests"."business_sites" = '---
:business_id: 3
') OR ("order_requests"."business_sites" = '---
:business_id: 3
'))

The debugger tells me that it's the '@model_class.send(:sanitize_sql, conditions)' line in ActiveRecordAdapter that returns the malformed SQL. Indeed:

Sale.send(:sanitize_sql, order_request: {business_site: {business_id: 4}})
# => "\"order_request\".\"business_site\" = '---\n:business_id: 4\n'"

Update: This is actually a misuse of sanitize_sql, it should be at most 1 level deep. In that case, order_request should be removed.

2 . cc shouldn't duplicate same rules
As we see, cc OR'ed the two rules conditions (as stated in the doc). However, when the rule is the same, it shouldn't, should it?

3 . Multiple overlapping roles
I found out of those issues when I was adding different roles to a user. Those roles abilities overlap for some controllers. This means I can't be sure that all role configurations will work together... I'm wondering what should I do to have a predictable behavior.

Ruby: 1.9.3 / Rails 3.0.20

@mikepack
Copy link
Collaborator

CanCan can't handle complex SQL very well, due to the nature of constructing queries. Specifically, joining through numerous tables. I'm not sure if @ryanb has plans to dig deep into this issue but this might help in the meantime:

The longer your can declaration, the deeper the query. Though it's unideal, try denormalizing your dataset a bit. This might be too long:

can :access, :sales, {:order_request => {:business_site => {:business_id => 3}}}

If you denormalized your data so that business_id lived on your sale model, your authorization declaration could be simplified:

can :access, :sales, {:business_id => 3}

@graywh
Copy link

graywh commented Sep 29, 2012

You can use through associations instead of denormalizing

class Sales < ActiveModel::Base
  belongs_to :order_request
  has_one :business_site, :through => :order_request
  has_one :business, :through => :business_site
end

so the ability is similar

can :access, :sales, :business_site => { :business_id => 3 }

Alternatively, you could also do

can :access, :sales, :business => { :id => 3 }

@gamov
Copy link
Author

gamov commented Oct 3, 2012

Thanks for your comments @mikepack and @graywh.
At this point of time, I don't want to denormalize.
I've just tried using :through associations but cc fails the same.
In the meantime, I'll add unless/if clauses in my ability files where needed... :o(

@graywh
Copy link

graywh commented Oct 3, 2012

Sorry, I tried entering that without looking at how I'd done it before. I've updated my comment.

@mikepack
Copy link
Collaborator

mikepack commented Oct 3, 2012

@graywh belongs_to does not have a :through option. I believe you're looking for has_one. This is a good thought, but it doesn't ultimately eliminate the join which is the cause of the issue. Denormalizing would be the only way to eliminate the join.

@graywh
Copy link

graywh commented Oct 3, 2012

It does use joins, but it works for me with 1.6.8. Did something change in 2.0 to break that?

And thanks for the correction.

@graywh
Copy link

graywh commented Oct 3, 2012

To clarify, with Cancan 1.6.8 (or branch 2.0) and ActiveRecord 3.2.8, all the through associations, and these abilities

can :manage, Sale, :business => { :id => 3 }
can :manage, Sale, :business => { :name => '3' }

it generates the following SQL with Sale.accessible_by

SELECT "sales".* FROM "sales"
INNER JOIN "order_requests" ON "order_requests"."id" = "sales"."order_request_id"
INNER JOIN "business_sites" ON "business_sites"."id" = "order_requests"."business_site_id"
INNER JOIN "businesses" ON "businesses"."id" = "business_sites"."business_id"
WHERE (("businesses"."id" = 3) OR ("businesses"."id" = 3))

@gamov
Copy link
Author

gamov commented Jun 11, 2013

@graywh have you tried setting both ability declarations to :business => { :id => 3}

@xhoy
Copy link

xhoy commented Jul 1, 2014

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!

gamov pushed a commit to gamov/cancan that referenced this issue Apr 4, 2016
If conditions are the same, we don’t merge them.
ryanb#727
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants