Skip to content

Fix ability precedence #647

Open
wants to merge 1 commit into from

4 participants

@andhapp
Collaborator
andhapp commented Jun 14, 2012

The gist of this pull request can be explained via the following example:

can :manage, User, :id => user.id
cannot :destroy, User

The cannot should override and not allow the user to destroy any user. This also fixes the case where only one cannot rule is used in the ability model.

@andhapp andhapp Fix issue when only one cannot rule is defined, although not sure if …
…that'll make a valid ability model; Also, fix the ability precedence issue, that ensures the cannot rule defined at the end will overwrite any of the can rules. This is the default behaviour.
32af911
@ryanb
Owner
ryanb commented Jun 19, 2012

This seems like a pretty significant change and I don't want to risk breaking existing Ability behavior for a minor release. Let's consider this in the 2.0 release instead.

@andhapp
Collaborator
andhapp commented Jun 19, 2012

@ryanb: That's fair.

@tardate
tardate commented Oct 31, 2012

Just fyi:

I just updated a site from cancan 1.5.1 to 1.6.8 and immediately ran into the regressed behaviour. NB: using the AR adapter with postgres.

I'm using this pull request now and so far it has fixed that issue and haven't run into any others (yet).

The issue - :cannot abilities cause all permissions to be ignored for accessible_by - seems really serious. If people's specs aren't up to scratch, its the kind of bug that can leave them exposing supposedly secure information without knowing it. I'd vote for the fix to be fast-tracked for the next possible release.

@davidakachaos

Should this still be open? Or can this be closed now?

@tardate
tardate commented Oct 4, 2013

hmm, this pull request never got merged. I just took a look at HEAD, and it's not clear that this issue is properly addressed yet. Looks like need to cross-port the necessary tests to be sure first. Anyone know different?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.