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

Overriding both authorize and policy is required for namespacing #723

Open
sedubois opened this issue Feb 20, 2022 · 4 comments
Open

Overriding both authorize and policy is required for namespacing #723

sedubois opened this issue Feb 20, 2022 · 4 comments

Comments

@sedubois
Copy link

sedubois commented Feb 20, 2022

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

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

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

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

Now with Pundit::Context I believe we could provide namespacing by overriding the pundit context in the controller:

def pundit
@pundit ||= Pundit::Context.new(
user: pundit_user,
policy_cache: Pundit::CacheStore::LegacyStore.new(policies)
)
end

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

def pundit = super.with_namespace(...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants