Issue #687: cancan inserting "AND (NULL)" at the end of sql #765

Merged
merged 1 commit into from Oct 25, 2012

Projects

None yet

3 participants

@jonsgreen
Collaborator

Ensure that empty conditions does not trigger unmergeable conditions

@jonsgreen jonsgreen Issue #687: cancan inserting "AND (NULL)" at the end of sql
Ensure that empty conditions does not trigger unmergeable conditions
f5b3fcd
@jonsgreen
Collaborator

I was a bit unsure about adding a user model/table to the specs but wanted the new spec to reflect the issue.

Also, I suppose that CanCan users really should be discouraged from writing rules like:

can :manage, :all
can :manage, Article, :user_id => user.id

since really the condition on Article will get overridden by the lack of conditions above as somewhat explained in Ability Precedence.

I am wondering though if really CanCan should be handling the logic in this case differently? Intuitively should only Articles that belong to users be accessible with these rules?

@ryanb ryanb merged commit 4dcd544 into ryanb:master Oct 25, 2012
@ryanb
Owner
ryanb commented Oct 25, 2012

Pulled in thanks. The way CanCan rules work is it will go to the next rule up the list if one fails. Since can :manage, :all should catch everything it would makes sense that everything still passes. The main exception to this is the cannot method. I'm open to changing this in 2.0 if it isn't intuitive though.

@korobkov

Please, bump the version with this fix included (now I've to use master from source rather than versioned gem)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment