-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add for_all_actions composite matcher to permit/forbid_mass_assignment_of #24
Comments
In 2018 I proposed adding a composite matcher Testing permitted attributes for all actions by default would be a breaking change though, which could be disruptive. If there is value in checking all actions by default, perhaps we could consider adding new matchers such as |
Some related thoughts as this feature is worked through: Given that Pundit internally falls back from an That is: Given a policy that implements It should pass. Why? Because that configuration of a pundit policy truly does permit the slug attribute for create actions. The current matchers fail because they don't implement the same fallback rules that pundit actually does. |
I'd also add some additional caution/concern regarding the potential implementation of (I have a vested interest in this capability because we're using Pundit to authorize our Graphiti api, so the "actions" for attribute guards are |
@jasonkarns Yes, I think your reasoning is solid for us to change the behaviour of A question remains: do we need a fallback composite matcher for testing the default |
The readme current states the following:
However if an attribute is permitted or forbidden in all
permitted_attributes
methods in the policy, the spec code can become quite long winded. Here's an example from the readme:It would be useful to check do this test in a single statement. I propose that we add a
for_all_actions
compose matcher to do just that:This would also work for
forbid_mass_assignment_of
.The text was updated successfully, but these errors were encountered: