Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use $nor for joining cannot rules #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johnnyshields
Copy link

@johnnyshields johnnyshields commented Jul 31, 2022

Previously this gem used a series of '$ne' operators for cannot rules. I have changed it to put then all inside a $nor block. This has the advantage that it works with more complex nested behaviors, such as "$nor" => [{ name: { "$in" => ["Bob", "Jane"] }]

Also includes some refactoring which makes it cleaner / easier to read.

As a result, this PR also changes the behavior of one spec which I think was incorrect previously:

    # ORIGINAL spec
    it 'returns the correct records when a mix of can and cannot rules in defined ability' do
      @ability.can :manage, MongoidProject, title: 'Sir'
      @ability.cannot :destroy, MongoidProject

      sir = MongoidProject.create(title: 'Sir')
      MongoidProject.create(title: 'Lord')
      MongoidProject.create(title: 'Dude')

      expect(MongoidProject.accessible_by(@ability, :destroy).to_a).to eq([sir])  #=> I think should be []
    end

Here I think it should actually be no results, i.e. [], because the .cannot rule should take precedence over the .can. I think this makes much more sense from a security standpoint, etc.

I have changed it to:

    # NEW spec
    it 'returns the correct records when a mix of can and cannot rules in defined ability' do
      @ability.can :manage, MongoidProject
      @ability.cannot :destroy, MongoidProject, title: { '$in' => %w[Lord Dude] }

      sir = MongoidProject.create(title: 'Sir')
      MongoidProject.create(title: 'Lord')
      MongoidProject.create(title: 'Dude')

      expect(MongoidProject.accessible_by(@ability, :destroy).to_a).to eq([sir])
    end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant