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

Use security_invoker views for temporary views created in the new schema #179

Closed
ben-pr-p opened this issue Oct 6, 2023 · 2 comments · Fixed by #189
Closed

Use security_invoker views for temporary views created in the new schema #179

ben-pr-p opened this issue Oct 6, 2023 · 2 comments · Fixed by #189
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request

Comments

@ben-pr-p
Copy link

ben-pr-p commented Oct 6, 2023

First, thanks for introducing this library - it's great to have, and I'm planning on contributing some extra tooling around it, specifically an integration with Kysely.

I think there's a bug (I haven't verified it, but just believe it's there from reading the code), that the views on the new schema that point to the tables in the old schema are created using the default security setting, which is similar to security definer.

This means that during the transition phase, row level security on the underlying tables will be bypassed by the views created by pgroll.

In order to make sure that accessing the old schema and new schema are the same, create view view_name with (security_invoker = true) as .... should be used.

See this article on the PG 15 release for more info: https://www.depesz.com/2022/03/22/waiting-for-postgresql-15-add-support-for-security-invoker-views/

I think that pgroll should also have some documented limitation on this potential "gotcha" in versions before Postgres 15, and potentially even error if there's no way around it (if there is an RLS policy on a table and the version is 14, which I see is supported).

Thanks again for all your work, super excited for what it means for the ecosystem!

@exekias
Copy link
Member

exekias commented Oct 6, 2023

Thank you for bringing this up! I agree we should document better this and other limitations, happy to take a first stab at that.

Adding security_invoker to pgroll views makes sense to me, and should not be too complicated. We will need to retrieve the Postgres version first, as I believe this would be the first conditional clause we add based on it.

Also, really looking forward to knowing more about your plans for Kysely integration, this is great to hear! It seems there is enough interest on integrating ORMs & query builders across the board, and it’s something we want to look into: #166

@exekias exekias added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request labels Oct 6, 2023
@andrew-farries andrew-farries self-assigned this Oct 16, 2023
@andrew-farries
Copy link
Collaborator

@ben-pr-p FYI we have a PR up that addresses the issue you raised with security_invoker views: #189.

andrew-farries added a commit that referenced this issue Oct 18, 2023
Ensure that views (in Postgres 15 and 16) are created with
`security_invoker = true`.

This ensures that any row-level security on the underlying table is
applied according to the permissions of the invoker of the view rather
than its owner.

Postgres 14 does not support the creation of views with
`security_invoker = true`.

See:
*
https://www.depesz.com/2022/03/22/waiting-for-postgresql-15-add-support-for-security-invoker-views/
*
https://www.cybertec-postgresql.com/en/view-permissions-and-row-level-security-in-postgresql/

Closes #179
andrew-farries added a commit that referenced this issue Nov 8, 2023
Add a section covering the limitations of `pgroll` and row level
security policies in Postgres 14.

[[Direct
link](https://github.com/xataio/pgroll/blob/document-rls/docs/README.md#supported-postgres-versions)]

Relates to #179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants