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

Per-query level verify_pundit_policy_scoped #728

Open
MyklClason opened this issue Mar 18, 2022 · 1 comment
Open

Per-query level verify_pundit_policy_scoped #728

MyklClason opened this issue Mar 18, 2022 · 1 comment

Comments

@MyklClason
Copy link

Have similar issue as this one, but have a solution in mind: #663

The issue is to be explicit and potentially excessive. Though we can make use of lazy query logic to prevent doing too much work: Basically given this:

records = Record.all # Lazy, so doesn't run here unless you try to view the result
records.pluck(:name) # This actually runs the database query

We want it to raise an error unless it has something like one of these two:

records = policy_scope(Record.all)
records.pluck(:name)

or

records = Record.all
policy_scope(records).pluck(:name)

This might be as simple as policy_scope setting a "pundit_policy_scoped" flag on Record.all (or worst case, a global/instance variable or just using the cache) and to_sql (or another method that is called when actually sending the query to the database) raising an error if the flag isn't set. We can skip doing both unless policy_scope is defined.

Even if it's not an official solution, it would be good to have a code example that allows for it.

@dgmstuart
Copy link
Collaborator

Hi - you say you have a solution, but I don't think I understand what that solution you're suggesting is?

Overriding calls to Record.all feels both unnecessarily intrusive (it would require monkeypatching ActiveRecord?) and also insufficient (eg. calls to Record.where(...) would not be covered).

Using global state is also not something we'd be keen to do.

In general I think this feature is not possible: you're never going to be able to lock down all database queries because Ruby is just too dynamic and Rails provides so many different ways to access the database: there will always be some workaround which will slip through.

If you really need more strict locking down of access like this, then I'm afraid Ruby and Rails are probably not good tools for achieving that.

But in any case, I think it's a mistake to think of verify_policy_scoped and verify_authorized_features as a way to systematically ensure that Pundit is applied everywhere: as the documentation hopefully makes clear, they're just intended to be a reminder to the developer to prevent accidentally forgetting to add any scoping/authorizing.

Perhaps we need to make that clearer in the documentation: you're not the first person to try to use these methods in the way you're describing.

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

3 participants