-
-
Notifications
You must be signed in to change notification settings - Fork 804
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
Ransack 4.0 allowlist be an optional config #1482
Comments
Another reason to offer an opt out mechanism is the performance degradation reported in #1462 |
The old behaviour is quite simple to implement and is actually mentioned in one of the tests implementing the new requirement. But this is not mentioned in the official documentation. |
@Tmpv Am i missing something, I added that to ApplicationRecord and I get undefined method |
There's also so many other access control mechanisms that work on a different layer than models that developers might choose to utilize. Strong params, explicitly defined filters in ActiveAdmin (and I promise that so many application will only use Ransack as a dependency of ActiveAdmin), form objects with params validation, etc. While giving the option to perform the validation inside models is great, there really should be a simple way to opt out of it. |
Furthermore, if there are gems which define AR models (e.g. mobility defines them dynamically), they might inherit directly from |
This allowlist enforcement actually needs to be able to be turned off. I understand the security protections trying to be put in place by this change. But for many apps it's not even needed, internal only apps that aren't even internet accessible don't need additional any additional attention. I can understand default requiring this but a simple config of some kind to turn it off would be needed as well.
TLDR: This was a breaking change and instituted quite abruptly, and so everyone probably is just pinning to the older version.
The text was updated successfully, but these errors were encountered: