Pundit: Best practices for authorizing a collection action?

Created on 22 Dec 2013  路  4Comments  路  Source: varvet/pundit

This is really just a question of aesthetics, but I'm convinced I'm not using Pundit effectively here. I'm having trouble explaining this clearly, but hopefully I'll get my message across:

I'd like to be able to authorize whether or not a user can access #index on a model. In my particular case, let's say I have Essays and Topics -- a topic has_many essays. If you created a topic, you can see what essays others have written on that topic (this would be the EssaysController#index action). I want to have an access-denied exception if a user tries to see what essays have been written about a subject she did not create.

I've implemented this by doing:

class EssaysController
  def index
    @topic = Topic.find(params[:topic_id])
    authorize @topic, :list_essays?

    @essays = @topic.essays
  end
end

class TopicPolicy
  def list_essays?
    user == topic.creator
  end
end

What I don't like about this is that I'm calling #authorize in my _essay_ controller, but it's going to talk to the _topic_ policy. Is there a cleaner way of doing this?

_Edit_: Another unsavory aspect to this method is that it doesn't work well if I call verify_policy_scoped after index actions; my implementation doesn't necessarily need to call policy_scoped.

I am totally conscious of the fact that this is a relatively benign issue, but I'd rather use my tools in the best possible way to produce easy-to-follow code.

Most helpful comment

For what its worth, I've had the same question many times and not found a clear answer. It does seem that best practice for authorizing index actions is conspicuously missing from the Pundit doc.

All 4 comments

Perhaps you could consider trying something like this (un-tested example):

class Topic < ActiveRecord::Base
  belongs_to :user
  has_many :essays
end

class Essay < ActiveRecord::Base
  belongs_to :topic
end

class ApplicationController < ActionController::Base
  after_filter :verify_policy_scoped, only: :index
end

class EssaysController < ApplicationController
  def index
    @essays = policy_scrope(Essay.where(topic_id: params[:topic_id]))
  end
end

class EssayPolicy
  class Scope < Struct.new(:user, :scope)
    def resolve
      scope.joins(:topic).where(topics: { user_id: user.id })
    end
  end
end

If you instead want to raise a Pundit::NotAuthorizedError if there are no essays accessible to the user you could implement something like:

class ApplicationPolicy
  def scope
    Pundit.policy_scope!(user, record.class)
  end

  def index?
    scope.exists?
  end
end

class EssayPolicy < ApplicationPolicy
  # (Inherit from ApplicationPolicy)
end

class EssaysController < ApplicationController
  def index
    # ...
    authorize @essays
  end
end

Note that if you need to allow access to the index view before it is populated with at least one Essay the above approach would not work though.

@ramhoj Thanks for giving me a hand! Your second example is the first thing I had come up with, but I don't like that @essays cannot be empty. The first example is also not what I had in mind because it'll be difficult for the user to tell the difference between their accessing a topic with no essays and their accessing a topic that they did not create, since in both cases I return an empty set of essays. I'd prefer it if throwing an authorization exception and merely returning an empty set could be separate notions.

Furthermore, I'd like to able to use #policy on a topic in my views, and having to do

policy(Essays.where(topic_id: @topic.id)).index?

is less desirable than doing

policy(@topic).list_essays?

Or do I misunderstand your examples?

If I'm asking too much of Pundit here (and I think I am), then feel free to close this issue. Now that I've looked at my code again, I'm fairly satisfied with the legibility of this list_essays? pattern.

I'm closing this since it seems there's no better way to solve my problem.

For what its worth, I've had the same question many times and not found a clear answer. It does seem that best practice for authorizing index actions is conspicuously missing from the Pundit doc.

Was this page helpful?
0 / 5 - 0 ratings