Skip to content

[4.x] Make forcing RLS configurable #1293

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

Merged
merged 20 commits into from
Jun 5, 2025
Merged

[4.x] Make forcing RLS configurable #1293

merged 20 commits into from
Jun 5, 2025

Conversation

lukinovec
Copy link
Contributor

@lukinovec lukinovec commented Jan 14, 2025

This PR adds the $forceRls static property to the tenants:rls command.

If $forceRls is true (default), the table owners won't be able to query their own tables (unless the owners have the BYPASSRLS privilege). If the property is set to false, table owners will bypass RLS, allowing them to query the owned tables.

(follow up on #1288)


Additional context:

  • By default we expect the central user to have the BYPASSRLS perm
  • If that's not the case, this setting can be set to false and everything should still work normally — table owners bypass RLS by virtue of being table owners
  • When forceRls is true, table owners do not automatically bypass RLS policies unless they have the BYPASSRLS perm
  • In custom setups where additional users may be involved, or the central user isn't the user creating tables, additional care should be taken when customizing this
  • The setting is true by default for more safety (even if in the most standard cases there's no change in behavior regardless of how this setting is configured)

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.71%. Comparing base (e74e1f9) to head (3c6e5b3).
Report is 1 commits behind head on may25.

Additional details and impacted files
@@            Coverage Diff            @@
##              may25    #1293   +/-   ##
=========================================
  Coverage     84.71%   84.71%           
- Complexity     1078     1079    +1     
=========================================
  Files           178      178           
  Lines          3120     3121    +1     
=========================================
+ Hits           2643     2644    +1     
  Misses          477      477           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheDeadCode
Copy link

Any news on this? <3

@stancl stancl marked this pull request as draft March 18, 2025 20:28
@lukinovec lukinovec marked this pull request as ready for review March 19, 2025 10:49
@lukinovec lukinovec force-pushed the configurable-force-rls branch from b7dda96 to e4a32e1 Compare March 19, 2025 12:41
@stancl stancl changed the base branch from master to may25 May 29, 2025 16:12
@stancl stancl requested a review from Copilot May 30, 2025 14:34
Copilot

This comment was marked as outdated.

@stancl stancl requested a review from Copilot June 5, 2025 02:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds the configurable static property $forceRls to control whether table owners bypass RLS policies and updates tests to validate the behavior with different RLS configurations.

  • Introduces the $forceRls property in CreateUserWithRLSPolicies to force RLS policy enforcement.
  • Updates tests in TraitManagerTest, TableManagerTest, and PolicyTest to account for configurable RLS behavior using additional test parameters.
  • Adds new tests to verify that even table owners and users without BYPASSRLS privilege are correctly affected when forceRls is enabled.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/RLS/TraitManagerTest.php Updated test signatures and configuration for testing implicit RLS and forceRls behavior.
tests/RLS/TableManagerTest.php Modified tests to include RLS forcing scenarios and added comprehensive tests for RLS logic.
tests/RLS/PolicyTest.php Revised tests to incorporate forceRls parameter and validate tenant session variable failures.
src/Commands/CreateUserWithRLSPolicies.php Introduced the $forceRls property with documentation and corresponding logic in the command.

@stancl stancl merged commit 2057e1e into may25 Jun 5, 2025
8 checks passed
@stancl stancl deleted the configurable-force-rls branch June 5, 2025 03:06
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.

3 participants