Pundit: Best way to pass an extra object to authorize method

Created on 4 Aug 2014  路  6Comments  路  Source: varvet/pundit

I have nested routes, like: /foos/1/users:

# users_controller.rb
def index
  @foo=Foo.find(params[:id])
  @users = @foo.users
end

I want to reference @foo in my policy, so I've modified my user_policy initialize method to take another argument:

  # user_policy.rb
  def initialize(current_user, user, foo=nil)
    @current_user = current_user
    @user = user
    @foo = foo
  end

  def index?
    current_user.foos,include?(foo) 
  end

Is this the way we're supposed to do this sort of thing?

Also I suspect this technique is wrong because I can't use the authorize method, and instead have to do the following:

  after_action :verify_authorized, except: [:index]

  def index
    @users = @foo.users
    raise Pundit::NotAuthorizedError unless UserPolicy.new(current_user, @users, @foo).index?
  end

I'd prefer to use authorize method in there but the following all fail:

authorize current_user, @users, @foo
# throws an argument error

authorize @users, @foo
# throws a type error

Is this the wrong way to do things?

Most helpful comment

Thanks to @chrisalley, for the proposed solution, here's what I did to get it working:

1) In user model:

    # models/user.rb
    attr_accessor :current_foo

2) In users_controller

    # controllers/users_controller.rb
    before_action :find_foo, :set_current_foo
    ...
    def find_foo
      @foo = Foo.find(params[:foo_id])
    end

    def set_current_foo
      current_user.current_foo = @foo if current_user
    end

3) In my policy I have the following:

    def has_foo?
      current_user.foos.include?(current_user.current_foo)
    end

    def new?
      has_foo?
    end

This allows me to go back to using authorize @user in my controllers. Thanks to @chrisalley.

All 6 comments

I also would like to know how to do it. Actually, we are using it that way :

raise Pundit::NotAuthorizedError unless UserPolicy.new(current_user, @users, @foo).index?

but as @murdoch pointed out, this is not convenient, especially when we want to handle error within the application controller :

def user_not_authorized(exception)
  policy_name = exception.policy.class.to_s.underscore
  render json: { type: 'error', message: t("auth.#{ policy_name }.#{ exception.query }") }, status: :unauthorized
end

as it will force us to do it within the controller's action.

Also, why can't we specify a specific Policy within a specific controller, such as for example :

class GroupsController < ApplicationController
  def update
    @group = get_group(params[:id])
    authorize @group, :update_item?, with: AntoherPolicy
  end
end

Right now, if I'm inside GroupsController and want to check for a specific conditions on antoher object, I can't using only authorize. Ideas?

I came up with a semi-workable solution to my particular problem. YMMV. It involves adding a method called :view_users? to my FooPolicy class:

  def view_users?
    current_user.foos.include?(foo)
  end

And now I can call authorize in my UsersController like so:

@ users = @foo.users
authorize @foo, :view_users?

It works, although it does still leave me wondering what's the best way to pass additional arguments to the authorize method.

Since no one seems to have any solution to the problem of passing multiple values, I came up with a workable solution as well. It's a bit smelly but works well (sure need to be improved though) :

# application_controller.rb
class ApplicationController < ActionController::Base
  include Pundit
  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  def user_not_authorized(exception)
    policy_name = exception.policy.class.to_s.underscore
    respond_to do |format|
      format.json { render json: { type: 'error', message: t("auth.#{ policy_name }.#{ exception.query }") }, status: :unauthorized }
      format.html { redirect_to root_url }
    end
  end

  def authorize_with_multiple(*args, query, policy)
    pundit_policy = policy.new(current_user, *args)
    unless pundit_policy.public_send(query)
      error = NotAuthorizedError.new("not allowed")
      error.query, error.policy = query, pundit_policy

      raise error
    end
  end
end

Then, all I need to do is call this method when I want to authorize using multiple object :

class GroupsController < ApplicationController
  def update
    @group  = get_group(params[:id])
    @friend = get_friend(params[:friend])

    authorize_with_multiple *[@group, @friend], :can?, AnotherPolicy

    # whatever you need to do then
  end
end

Finally, the policy would looks like that :

class AnotherPolicy < ApplicationPolicy
  attr_reader :current_user, :group, :friend

  def initialize(current_user, *args)
    @user = current_user
    @group, @friend = *args
  end

  def can?
    # Can now use group and friend object
  end
end

You could reference the class manually in your controller and add the additional parameter to the method call:

UserPolicy.new(current_user, user, foo).index?

However this goes against the simplicity of authorising one object at a time. It also causes problems when using Pundit's permit matcher in RSpec.

I prefer to check additional objects inside the policy file. These would be attributes of either the current user or the record being authorised. The additional objects should be (conceptually) related to the current user or to the record being authorised in some way - otherwise why do you need to check their properties?

In the first example, there is a many-to-many relationship between users and foos. You could create a virtual attribute in the user model such as "current_foo" (based on the foo_id in the URL) and assign this to the current user before the authorize method is called. Then in the policy file, simply check the user's current foo:

def index?
  current_user.foos.include?(current_user.current_foo)
end

While there is extra work in setting the additional attribute, this approach keeps the authorize method call in the controller clean and focused on authorising just one record at a time for the current user.

Thanks to @chrisalley, for the proposed solution, here's what I did to get it working:

1) In user model:

    # models/user.rb
    attr_accessor :current_foo

2) In users_controller

    # controllers/users_controller.rb
    before_action :find_foo, :set_current_foo
    ...
    def find_foo
      @foo = Foo.find(params[:foo_id])
    end

    def set_current_foo
      current_user.current_foo = @foo if current_user
    end

3) In my policy I have the following:

    def has_foo?
      current_user.foos.include?(current_user.current_foo)
    end

    def new?
      has_foo?
    end

This allows me to go back to using authorize @user in my controllers. Thanks to @chrisalley.

It's so much simpler in Laravel, https://laravel.com/docs/5.8/authorization#writing-policies , you just pass any number of arguments to policy method and that's all.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mattruzicka picture mattruzicka  路  5Comments

gabrielecirulli picture gabrielecirulli  路  4Comments

lnikkila picture lnikkila  路  5Comments

mackshkatz picture mackshkatz  路  4Comments

jdwolk picture jdwolk  路  3Comments