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

[4.x] RLS without central user (role) having bypassrls #1288

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

JustinElst
Copy link

Original issue

Description

I am using a managed Postgres solution where the admin role does not have and can not get BYPASSRLS.
That leads to the behaviour that when using the 'central connection' i don't have access to tenant data (including 'tennant_user').
That makes the management of tenants impossible.

So I thought of a solution giving the tenancy.rls.user.username (database tenant_user) the rsl policies already implemented by the package.
AND giving the central user a policy with full access:

CREATE POLICY {$table}_central_rls_policy ON {$table} AS PERMISSIVE TO {$centralUserName} USING (true);

This worked for me :)

Why this should be added

I have implemented this in the CreateUserWithRLSPolicies command and 'TableRLSManager' class.
This could use some more refactoring and possibly detecting if central user has BYPASSRLS permissions and ommiting the central policy.
But this is to give you an idea of what I did with this.
What do you think?

Original @stancl reply:

Thanks for the detailed writeup! I have some plans for refactoring the RLS feature, so I think I could take a look at this alongside that. This seems like an implementation detail so there shouldn't be any breaking changes.

Would the ideal behavior be basically checking whether the central user has BYPASSRLS perms when creating the policies, and if it doesn't, append the policies with the AS PERMISSIVE TO ...?

Will just need to find the right spot for this, to not run the check multiple times ideally but also to make this work well with the hashing logic.

@JustinElst JustinElst changed the title RLS without central user (role) having bypassrls [4.x] RLS without central user (role) having bypassrls Jan 9, 2025
@stancl
Copy link
Member

stancl commented Jan 9, 2025

Thanks for the PR! Can you revert the formatting changes?

@stancl
Copy link
Member

stancl commented Jan 9, 2025

Ah nevermind, I see that there's an extra loop now.

@stancl
Copy link
Member

stancl commented Jan 9, 2025

Currently in the process of testing an alternative implementation for this btw, so no need to bother fixing phpstan for now.

@stancl
Copy link
Member

stancl commented Jan 9, 2025

Could you provide some concrete reproduction steps for the bug first? ALTER ROLE (central user) NOBYPASSRLS in our tests doesn't seem to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants