-
Notifications
You must be signed in to change notification settings - Fork 59
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
Support for Pundit Policy Namespacing? #101
Comments
I would also be hugely in favor of this. Namespaced policies would fit perfectly with namespaced resources/controllers imo. @valscion Would this just require a processor change? |
Hi, thanks for opening an issue! Sorry for taking so long to reply — I've been on two weeks vacation and just came back to work today. It seems like I see @Subtletree has created a proof of concept for automatic namespacing based on resource class names in #102. I think it would be better if we didn't look at the resource class names and then use the namespace from that. It would be nice to see some design around an explicit policy class definition, for example by using a special resource class method as originally proposed in #8 if that would fit your use case. |
Hope you had a good break @valscion 😄 I think that's a good idea to step back to a design stage and look at actual use cases for namespaced resources/policies. If there are use cases for specifying a policy override per resource then that sounds like the way to go. If it ends up being that namespaced policies are always wanted for namespaced resources then I think automatic namespacing should be the way to go. This is how jsonapi-resources handles matching controllers to resources. Namespaced controllers are automatically matched to namespaced resources via the controller class name. Another option would be to have a separate |
Hmm. Yeah. My use case will break if namespacing is applied automatically, as our resources and controllers are namespaced under So this will need to be somehow configurable. And the implementation in this gem should be simple, or it won't be worth the maintenance cost. What is simple, is of course subjective — that's why throwing out some ideas and designs here can be useful |
With Pundit 2 supported in #104, could we not author a Enabling this behaviour is as simple as specifying this new I'm thinking ... drying up some of the existing I'm very interested in this use case. Maybe I'm the only one! 😄 |
As long as it's configurable and as straightforward as possible to do this, why not 😊. I don't want the policy finding for namespaced policies to be only possible via class name namespacing. I'm open to see some code examples on how JA could support this :) a gist of an example use case would be nice to lead the conversation forward |
Interesting, that may add more complexity. Lazy me will argue that in following with Rails' convention over configuration, a Would you ever mix this class path resolution I describe with this configurable variation you yearn for? Or would only one of these techniques be activated at a time? EDITApologies -- I just noticed you mentioning in a comment above that you'd need to subtract the I'll fork and tinker. |
One approach could be to have some sort of class method to define the namespace in one place, and then let the policy finding work as it did before. The class method would in that case have to be on the resource level, if I've understood your use case correctly. module Api
class MyResource
some_special_method_from_jsonapi_authorization "ApiPolicyNamespace"
end
end Another option would be to apply some configuration on app-level wide, say some sort of mapping from a module namespace to a policy namespace, but I think this might be confusing for users. # config/initializers/jsonapi_authorization.rb
JSONAPI::Authorization.configure do |config|
config.foobar_policy_stuff_mapping_of_some_sort = {
"Api::V1::*" => "Api::V1::*Policy"
}
end ☝️ this seems like it could easily get confusing. And if all of the resources would have to be defined like this: # config/initializers/jsonapi_authorization.rb
JSONAPI::Authorization.configure do |config|
config.foobar_policy_stuff_mapping_of_some_sort = {
"Api::V1::FooResource" => "Api::V1::FooPolicy"
}
end ...it would be easy to forget to do this. Making something "easy to forget" would not match the core design principle of So the proposed solution would have to be something that would protect users from accidentally forgetting to do something. Take my contrived second example above: The implementation could yell out loud when a mapping was configured yet some resource didn't have an explicit mapping configured — this way, users would no longer accidentally forget to define the policy class for a resource. |
Perhaps I have overlooked it but it seems that the way this gem is designed we are unable to take advantage of Pundit's Policy Namespacing feature.
We have a motivation similar to #8 in which users can access our apis under different contexts, such as an admin or a user, and thus should receive different policies and different scopes. The namespacing seemed a good fit as we could pass the PolicyFinder an array containing the context and the object instead just the object and it will resolve policy classes as follows:
Was this intentional? Am I missing something?
The text was updated successfully, but these errors were encountered: