Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

False positives on multiple nested abilities definitions #564

Merged
merged 1 commit into from May 10, 2012

Conversation

Projects
None yet
3 participants
Contributor

flop commented Feb 29, 2012

Hi Ryan,

I just found a nasty problem when you have multiple definitions for a nested ability, the can? method always return true. For example :

# ability.rb
can :read, Comment, :post => {:published => true}
can [:read, :update], Comment, :post => {:author_id => user.id}

# checking ability
can? :read, @post => Comment

I found out that the rule.rb class is using Hash.shift on the subject. Doing that removes the content from the subject. So when the second rule is tested the subject is empty and the condition matching return true.
In this commit, I simply propose to replace Hash.shift by Enumerable.first as it's not destructive on the subject Hash.

Florent

Contributor

flop commented Mar 5, 2012

@ryanb, any news on merging and releasing this one ? It might lead to serious security issues as 'can?' always return true in those situations.

@Sija Sija pushed a commit to Sija/cancan that referenced this pull request Mar 19, 2012

Sijawusz Pur Rahnama Merged PR #564 664e0b4
Collaborator

jeremyf commented May 10, 2012

[verified] Tests pass, looks good.

@ryanb ryanb added a commit that referenced this pull request May 10, 2012

@ryanb ryanb Merge pull request #564 from flop/master
False positives on multiple nested abilities definitions
b73bd06

@ryanb ryanb merged commit b73bd06 into ryanb:master May 10, 2012

Contributor

flop commented May 10, 2012

Thank you both for reviewing and merging

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