Policy Finder `find` does not strip namespace.

arvind0598 commented 1 year ago

This seems to be brought up before, most closely in #697.

Background

I'm using Pundit via the Pundit Adapter in Active Admin.
My policies are namespaced under active_admin, resulting in policy classes like ActiveAdmin::BookPolicy.
I am using STI on the Book class, with types like RedBook and BlueBook. Both these classes have the self.policy_class defined to point back to the BookPolicy.

The Pundit adapter uses the namespace method and sends an array with the namespace and a resource to Pundit - that eventually ends up calling PolicyFinder::find method with the array.

Expected Behaviour

I should be able to share policies between various "STI siblings", even if the policy is namespaced.

Actual Behaviour

This is not possible, at least when Pundit is abstracted through the Pundit Adapter.

Issue

When attempting to authorize an action on RedBook, I've tried these three scenarios:

When self.policy_class is not defined - Pundit looks for ActiveAdmin::RedBookPolicy and fails [EXPECTED]
When self.policy_class returns BookPolicy - rails says this class is not found [EXPECTED]
When self.policy_class returns ActiveAdmin::BookPolicy - the find method does not strip the namespace, ending up with ActiveAdmin::ActiveAdmin::BookPolicy.

So right now I'm not able to share the policy at all - I'd expect the third case to go through.

More Info

I'm not able to override methods because of the adapter abstraction. This seems to have been the solution most people used in the other issues. I think find should be able to understand whether this statement is actually required before executing it-

     if subject.is_a?(Array)
        modules = subject.dup
        last = modules.pop
        context = modules.map { |x| find_class_name(x) }.join("::")
        [context, find(last)].join("::") #this line might need to be conditional?

I'm aware that I can create more policies for each type of Book, but my goal is to have a single policy that governs all Books.

arvind0598 commented 1 year ago

Tried two more things:

def self.policy_class
  "BookPolicy" # finds correctly
end

def self.policy_class
  :book_policy # does not work
end

I believe this is undocumented behaviour. Do we want to add this in here?

Burgestrand commented 3 months ago

Reading this, it seems like it boiled down to:

class Parent; end

class Child < Parent
  def self.policy_class = ParentPolicy
end

class NamespacedChild < Parent
  def self.policy_class = ActiveAdmin::ParentPolicy
end

def show
  authorize([:active_admin, Child]) #=> ActiveAdmin::ParentPolicy
  authorize([:active_admin, NamespacedChild]) #=> ActiveAdmin::ActiveAdmin::ParentPolicy
end

From what I can tell, this is how it's supposed to work judging by the test: https://github.com/varvet/pundit/blob/55f64cbe09a9da2f21988021c6a1d5848d312059/spec/policy_finder_spec.rb#L52-L58

In the test we're making sure that the namespace is added and that we end up with Project::PostPolicy, even though the instance method returns PostPolicy.

I realise this issue is old. I'm closing this mostly because I don't expect a reply. If I misunderstood the issue feel free to reopen. A failing example goes a long way!

varvet / pundit