Ability Scopes with Mongoid. Bug? #707

Closed
al opened this Issue Aug 1, 2012 · 2 comments

Projects

None yet

2 participants

al commented Aug 1, 2012

I am encountering an issue using Mongoid and scopes in rules.

Apologies for the length of this but context is required I think.

I have a User model and Profile model. In between there is a ProfileUser model which contains an additional permission attribute which may either be manager or collaborator.

Because this is Mongoid and we can't have relations :through other relations, I have implemented this little method on User which simulates a join to Profile through ProfileUser. It accepts a block which allows you to refine the conditions of the ProfileUser we are interested in.

class User
  ...

  def profiles(&block)
    pu = profile_users.scoped
    pu = pu.instance_eval(&block) if block_given?
    Profile.where(:id.in => pu.only(:profile_id).map(&:profile_id))
  end
end

So I can now say for example @user.profiles { where( ... the join document meets some condition ... ) }.

With respect to authorisation, the intention is to allow a User to do anything with a Profile if they are a manager of it. If they are a collaborator they just have read access. Else they have no access. My ability.rb is as follows:

class Ability
  include CanCan::Ability

  def initialize(user)
    user ||= User.new
    ...
    can [:read], Profile, user.profiles do |profile|
      user.profiles { where(profile_id: profile.id, :permission.in => ['manager', 'collaborator']) }.exists?
    end
    can [:manage], Profile, user.profiles { where(permission: 'manager') } do |profile|
      user.profiles { where(profile_id: profile.id, permission: 'manager') }.exists?
    end
    ...
  end
end

Call the first rule A and the second rule B.

As you can see I attempt to use the User#profiles method as a scope argument to change the behaviour of accessible_by. For the rule A I say the scope is just the all the user's Profile. For rule B I add the extra constraint that the user must be a manager of the Profile.

That's all fine up to the point I try to use load_resource for a collection action:

class ProfilesController < ApplicationController
  before_filter :authenticate_user!
  load_and_authorize_resource

  respond_to :html

  def index
    respond_with @profiles
  end

  ...
end

The scope does not work and instead @profile contains all the Profile.

I have looked into the CanCan code and the issue arises from CanCan::ModelAdapters::MongoidAdapter#database_records. What is happening is my current_user happens to not have manager permissions on any ProfileUser, thus for rule B rule.conditions.empty? => true and consequently execution falls through to line 42 and all documents are returned.

My question is finally, Should execution not be allowed to continue using rule A (for which in the case of this user rule.conditions.empty? => false), and thus return only the documents to which he has access via A?

I guess I could work around this by modifying rule B to be more specific (replacing :manage with [:update, :destroy] and leaving rule A to handle :read) but my understanding of the ability DSL is that this shouldn't be necessary.

I've been looking at it for some time, and my guess is that the process_can_rules variable just needs to be removed so that rules are always processed. However, I think there is another problem.

If I supply scopes on my rules then the rule.conditions are going to be Mongoid::Criteria, however the code seems to assume that if there are multiple rules then the conditions are all Hash, it will attempt to use Origin methods on them (or) and will error out.

So, is this a bug or ...? Any advice appreciated.

Can you try the most recent version of cancan (currently 1.6.10) to see if your issue has been resolved?

This is one of the oldest CanCan issues with no discussion. CanCan is struggling right now to implement support for rails 4, and the issue count is nearing 200. It would be a big help if we could close a few old issues and get the issue count down. Thanks!

al commented Jul 19, 2013

Okay, I've spent a little while trying to reproduce in a minimal project using 1.6.10 and so far it seems okay, so I think this can be closed.

@al al closed this Jul 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment