CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-6cancancancancan
stevec
stevec

Reputation: 52758

How does cancancan set the model instance, and how can I inspect it?

I noticed users could access an action they shouldn't be able to access.

I debugged in the rails console with something like

user = User.first 
physician = Physician.first 
ability = Ability.new(user)
ability.can?(:send_message, physician)
# => false

The above says that user can't access the send_message action for that physician, which is the desired behaviour, yet I know they can in the app!

I think this narrows down the cause to a problem with cancancan loading the wrong model instance for some reason. And that's hinted to in the cancan docs too:

Note: this assumes that the model instance is being loaded properly.

But the problem is I'm not sure how to diagnose the problem from here, since the console says it should be working. I don't know how to view the model instance that cancancan has set, and I don't know what else to try.

Any ideas?

Update

I did manage to work around this by using authorize! :send_message, physician in the controller, but since I only stumbled upon this behaviour by chance, I think it's much more important to figure out why the wrong model instance was being loaded (especially so I can see if that was happening elsewhere too).

I figured out why it was probably happening (but I still don't know how to disagnose it)

I think this was happening because I had many custom actions, and some had @physician = Physician.find(current_user.physician.id) (i.e they're the current user), whereas others were more like @physician = Physician.find_by_id(physician_params[:id]). I'm not sure how cancan sets the instance model, but I do know it's not psychic, so it wouldn't know whether to set it to the current user's physician instance, or the physician instance for the physician id passed in.

What remains?

How does cancancan set the model instance for custom methods (I presume it tries something, and if that doesn't work, tries something else, etc etc)?

Small notes that help:

When I returned slug it breaks this behaviour and I can edit all pokemons.

Upvotes: 2

Views: 1507

Answers (2)

stevec
stevec

Reputation: 52758

Leaving my notes here in case they are helpful to anyone else.

TL;DR, there are a lot of nuanced assumptions cancancan makes, which you won't know about from the outset. I discovered many of them by thoroughly reading the comments in the cancancan readme, code, and defining abilities docs

So here goes..

How cancancan works

  • if you call authorize! in the controller action itself, cancancan will look for an instance variable in each controller action.
  • if you instead simply add load_and_authorize_resource at the start of your controller, that will do two things:
    1. Load an instance variable that cancancan thinks should be loaded, and
    2. Checks for authorization on that model instance
  • Note that for custom actions, load_and_authorize_resource will still try to load a model instance, but how does it know what to load? It doesn't, it guesses, which, for me, I do not like, so be aware of that.
    • For me, I prefer to do the work of load_and_authorize_resource myself in two separate steps, so I know exactly what's going on.
      1. Ensure @article is generated via a before action for each controller action (or @articles for index action)
      2. Simply have a line at the top of the controller saying load_and_authorize_resource after the before action that sets the model instance
        • Note that the only difference is now the developer is responsible for loading the right model instance, and cancancan is not trying to guess it. I prefer this approach because it only takes one mistake to accidentally allow access where it shouldn't be granted.
  • Also remember that load_and_authorize_resource should always go after any before actions that set the model instance variable

Random notes that may also help

  • The name of the instance variable depends on the action. If we have an articles controller, then:

    • For the index action, authorize looks for @articles
    • For all other actions, authorize looks for @article
      • It then checks to see if the user is allowed access to that resource.

  • load_and_authorize_resource checks to see if the model instance is set, and if not, it sets one. So if you have a before action that creates @article/@articles, then load_and_authorize_resource won't do it for you (i.e. it won't overwrite it), but if you didn't set one, cancan will try to set one. See comments from the source code:

A resource is not loaded if the instance variable is already set. This makes it easy to override the behavior through a before_action on certain actions.

  • An ability rule will override a previous one. (see here for an example)
  • Just one last thing, never use current_user in ability.rb, it will error silently (!!), so be sure to use user instead :)

Upvotes: 5

Sergio Tulentsev
Sergio Tulentsev

Reputation: 230521

Here's what is happening: https://github.com/CanCanCommunity/cancancan/blob/585e5ea54c900c6afd536f143cde962ccdf68607/lib/cancan/controller_additions.rb#L342-L355


    # Creates and returns the current user's ability and caches it. If you
    # want to override how the Ability is defined then this is the place.
    # Just define the method in the controller to change behavior.
    #
    #   def current_ability
    #     # instead of Ability.new(current_user)
    #     @current_ability ||= UserAbility.new(current_account)
    #   end
    #
    # Notice it is important to cache the ability object so it is not
    # recreated every time.
    def current_ability
      @current_ability ||= ::Ability.new(current_user)
    end

Upvotes: 2

Related Questions