Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy Finder find does not strip namespace. #750

Closed
arvind0598 opened this issue Nov 20, 2022 · 2 comments
Closed

Policy Finder find does not strip namespace. #750

arvind0598 opened this issue Nov 20, 2022 · 2 comments

Comments

@arvind0598
Copy link

arvind0598 commented Nov 20, 2022

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:

  1. When self.policy_class is not defined - Pundit looks for ActiveAdmin::RedBookPolicy and fails [EXPECTED]
  2. When self.policy_class returns BookPolicy - rails says this class is not found [EXPECTED]
  3. 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
Copy link
Author

arvind0598 commented Nov 21, 2022

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
Copy link
Member

Burgestrand commented Mar 28, 2024

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:

context "with an array of a symbol and a class with a specified policy class" do
it "returns the associated namespaced policy" do
object = described_class.new([:project, Customer::Post])
expect(object.policy).to eq Project::PostPolicy
end
end

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!

@Burgestrand Burgestrand closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants