varvet / pundit

Minimal authorization through OO design and pure Ruby classes
MIT License
8.28k stars 629 forks source link

Overriding both authorize and policy is required for namespacing #723

Open sedubois opened 2 years ago

sedubois commented 2 years ago

The README explains how to namespace policies, however this does not seem to take into account direct calls to policy (e.g. calling policy(record).show? from some view). Before the merge of #697, it was possible to only override policy and authorize would automatically be properly scoped as well. Now it has apparently become necessary to override both authorize and policy in Pundit::Authorization. It still works, but is just now more cumbersome, and also required some figuring out when performing the upgrade to Pundit 2.2.0.

Opening this issue as suggested in #697.

dgmstuart commented 2 years ago

I'm inclined to say that the fact that authorize was implemented using policy was an implementation detail, and that it's not something which should be relied on.

However, you're absolutely right that there's nothing written in the documentation about how to namespace direct calls to policy and how to override calls to policy in the AdminController example. Before doing that, we'd need to add some tests for namespacing those calls - it's not something we have time to do right now, but it's something we'd like to see.

igorlvicente commented 2 years ago

I want to tackle this issue but I suspect I have not quite understood what it is to be done. @dgmstuart, can you explain better what you have in mind? @sedubois, can you tell me whether what @dgmstuart is suggesting will solve the problem you - or others - have?

mattzollinhofer commented 2 years ago

I'm inclined to say that the fact that authorize was implemented using policy was an implementation detail, and that it's not something which should be relied on.

I can understand the perspective the policy was an implementation detail, but it was the implicit link between a custom policy's use for authorize calls and view's usage of custom policy so it's a little bit of a difficult balance for me to understand.

I opened a new issue (#740) trying to more clearly call this detail out as it's not necessarily related to namespacing in my case.

Burgestrand commented 4 months ago

Now with Pundit::Context I believe we could provide namespacing by overriding the pundit context in the controller: https://github.com/varvet/pundit/blob/7e59b98a72850a02c0a10ec17fff425cff18d31a/lib/pundit/authorization.rb#L19-L24

There's no explicit implementation for that, but I'm thinking something like:

def pundit = super.with_namespace(...)