Use policy class names in nested resourceful policy lookups to avoid causing naming collisions

[jmschneider]

We often create multiple namespaces within our rails application and load different policies depending on the namespace. It would be nice if we could structure our policy classes to mirror our route resources. For example, we might have a routes like:

authenticated :admin do
  scope module: :admin do
    draw :admin
  end
end

authenticated :customer do
  scope module: :customer do
    draw :customer
  end
end

And setup our controllers with something like:

class ApplicationController < ActionController::Base
  # Normal global stuff here
end

class Admin::Controller < ApplicationController
  def authorize(record, query = nil)
    super([:admin, record], query)
  end
end

class Customer::Controller < ApplicationController
  def authorize(record, query = nil)
    super([:customer, record], query)
  end
end

This all works great! The trouble I am having is when I want to use policies on nested resources. To continue our example, I could have a few models:

class User < ApplicationRecord
  has_many :program_applications
end

class Program < ApplicationRecord
  has_many :program_applications
end

class ProgramApplication < ApplicationRecord
  belongs_to :program
  belongs_to :user
end

My business rules dictate that each user should only have one active application for each program. For my routes, I use nested resources as ProgramApplication is a logic child of Application. What I would like to do is look up the policy using [@program, ProgramApplication] like this:

class Customer::Programs::ProgramApplicationsController < Customer::Programs::Controller
  def new
    authorize [@program, ProgramApplication]
  end
end

Based on previous code, this should resolve to authorize([:customer, [@program, ProgramApplication]], nil) which looks for a class called Customer::Program::ProgramApplicationPolicy. Functionally, this policy works great. I can create a policy class like:

class Customer::Program::ProgramApplicationPolicy << Customer::Policy
  def initializer(user, record)
    @user = user
    @program, @record = record
  end

  def create?
    @user.program_applications.exists?(program: @program, status: :active)
  end
end

The problem is it expects a module called Customer::Program. This breaks any reference to my Program model from any controller within my Customer namespace. My initial thought would be to use the @program's policy class in the namespace instead (i.e. Customer::ProgramPolicy::ProgramApplicationPolicy).

[jmschneider]

I did a little spelunking in the source code and now realize authorize([:customer, [@program, ProgramApplication]]) providing [@program, ProgramApplication] as the record may be an unintended side effect of how Pundit::PolicyFinder recursively resolves an array record and Pundit::Context just passes the last item of the outermost array. I do think this functionality can be awesome to use though.

[Burgestrand]

Hi! Just letting you know I've seen this. I'll read through it some time this week or next 🙂

[Burgestrand]

Yes, I too believe that the policy name lookup behaviour here is unintentional; when you've got both a namespace-by-array and a record with additional context that is intended as your record.

Technically it might even be considered a bug, but I'm sure somebody in the wild is relying on this behaviour so changing it isn't likely.

On the [@program, ProgramApplication] approach:

Specifically for new and create what we've sometimes done is to authorize on an actual record, like so:

def new
  program_application = ProgramApplication.new(program: @program)
  authorize [:customer, :programs, program_application]
end

Through this, then you have the same context as if you had when you passed an array for a record:

class Customer::Programs::ProgramApplicationPolicy < Customer::Policy
  def create?
    @user.program_applications.exists?(program: @record.program, status: :active)
  end
end

On the namespacing

Overriding authorize in multiple levels of inheritance to provide namespacing might require a slightly different approach:

class ApplicationController
  def authorize(record, ...) = super(pundit_namespace(record), ...)
  def pundit_namespace(record) = record
end

class Customer::Controller < ApplicationController
  def pundit_namespace(record) = [:customer, record]
end

class Customer::Programs::Controller < Customer::Controller
  def pundit_namespace(record) = [:customer, :programs, record]
end

PS: I've got a possible upcoming change I'm thinking about that would make for a more official API around namespacing, but I'm still undecided:

class Customer::Controller < ApplicationController
  def pundit = super.with_namespace(Customer)
end

class Customer::Programs::Controller < Customer::Controller
  def pundit = super.with_namespace(Customer::Programs)
end

[jmschneider]

I forgot to mention it in my other message, but I love the namespacing suggestions and the potential new API to more officially support it. I end up needing namespacing in almost every application I build.

[jmschneider]

Hi @Burgestrand

Thanks for taking the time to follow up on this! I figured there wouldn't be much support for this workflow once I realized the behavior fell somewhere between unintended functionality and a bug. I do actually think it is a useful feature so I will do my best to organize my thoughts on why it is useful while fully understanding it is unlikely to be enhanced. I apologize if it is a bit all over the place.

I think the crux of my approach is trying to mirror the resourceful design within Rails in Pundit policies. In Rails, it is very common have nested resources/resource relationships and load resources from an outside in approach. I think some of the suggested solutions for Pundit policies flip that to and inside out approach which, while it works, doesn't match the mental model elsewhere in Rails.

I have had to use the approach you suggested a bit to work around these issues:

def new
  program_application = policy_scope(ProgramApplication).new(program: @program)
  authorize [:customer, :programs, program_application]
end

This moves some of the important context for the policy to an attribute on the record (@record.program) rather than in the initializer data. To compare it to nested resource routes:

# we typically wouldn't set the @program_application first and set the @program from that
# /programs/program_application/edit
def edit
  @program_application = policy_scope(ProgramApplication).find(params[:id)
  @program = @program_application.program
end

# /programs/program_application/new?program_id=3
def new
  @program_application = policy_scope(ProgramApplication).new(program: params[:program_id])
  @program = @program_application.program
end

# more likely, we'd find the @program first and then find the @program_application from within that scope
# /programs/3/program_application/edit
def set_program_application
  @program = policy_scope(Program).find(params[:program_id])
  @program_application = policy_scope(@program.program_applications).find(params[:id)
end

# /programs/3/program_application/new
def new
  @program = policy_scope(Program).find(params[:program_id])
  @program_application = policy_scope(@program.program_applications).new
end

Additionally, this approach is it can be a bit verbose if we need to check policies in views as well.

<% if policy(policy_scope(ProgramApplication).new(program: @program).new? %>
  <%= link_to "Apply", [:new, @program, ProgramApplication] %>
<% end %>

Realistically what usually ends up happening is I add a custom method to the Customer::ProgramPolicy:

class Customer::ProgramPolicy
  def apply?
    @user.program_applications.exists?(program: @record, status: :active)
  end
end

class Customer::Programs::ProgramApplicationsController < Customer::Programs::Controller
  def new
    authorize @program, :apply?
  end
end

<% if policy(@program).apply? %>
  <%= link_to "Apply", [:new, @program, ProgramApplication] %>
<% end %>

This works fine but shifts our policies away from the resourceful way we tend to think about our models and routes. Similar to any time a non-resourceful route gets added, this can get bloated with all the different actions for other associations and the inverse actions (unenroll?). Being able to mirror that resourceful relationship from the data model to the routes, link helpers, and policies is helpful when thinking about our application.

# routes/customer.rb
resources :programs do
  resource :program_application, module: :programs
end

# views/*.html.erb
<% if policy([@program, ProgramApplication]).new? %>
  <%= link_to "Apply", [:new, @program, ProgramApplication] %>
<% end %>

<% if policy([@program, application]).edit? %>
  <%= link_to "Edit", [:edit, @program, application] %>
<% end %>

<% if policy([@program, application]).destroy? %>
  <%= link_to "Delete", [@program, application], "data-turbo-method": :delete, "data-turbo-confirm": "Are you sure?" %>
<% end %>

Again, thank you for taking the time to read and consider my silly unintended behavior 🙃