Re: [PATCH] Add reloption for views to enable RLS

On Wed, 2022-02-02 at 18:23 +0100, Christoph Heiss wrote:
> > Huh?  "duff" has no permission to insert into "tab"!
> That really should not happen, thanks for finding that and helping me
> investigating on how to fix that!
>
> This is now solved by checking the security_invoker property on the view
> in rewriteTargetView().
>
> I've also added a testcase for this in v4 to catch that in future.

I tested it, and the patch works fine now.

Some little comments:

> --- a/src/backend/rewrite/rewriteHandler.c
> +++ b/src/backend/rewrite/rewriteHandler.c
> @@ -3242,9 +3243,13 @@ rewriteTargetView(Query *parsetree, Relation view)
> 0);
>
> /*
> - * Mark the new target RTE for the permissions checks that we want to
> - * enforce against the view owner, as distinct from the query caller. At
> - * the relation level, require the same INSERT/UPDATE/DELETE permissions
> + * If the view has security_invoker set, mark the new target RTE for the
> + * permissions checks that we want to enforce against the query caller, as
> + * distince from the view owner.

Typo: distince

diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 509e930fc7..fea893569f 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -261,15 +261,26 @@ CREATE VIEW mysecview3 WITH (security_barrier=false)
AS SELECT * FROM tbl1 WHERE a < 0;
CREATE VIEW mysecview4 WITH (security_barrier)
AS SELECT * FROM tbl1 WHERE a <> 0;
-CREATE VIEW mysecview5 WITH (security_barrier=100) -- Error
+CREATE VIEW mysecview5 WITH (security_invoker=true)
+ AS SELECT * FROM tbl1 WHERE a = 100;
+CREATE VIEW mysecview6 WITH (security_invoker=false)
AS SELECT * FROM tbl1 WHERE a > 100;
+CREATE VIEW mysecview7 WITH (security_invoker)
+ AS SELECT * FROM tbl1 WHERE a < 100;
+CREATE VIEW mysecview8 WITH (security_barrier=100) -- Error
+ AS SELECT * FROM tbl1 WHERE a <> 100;
ERROR: invalid value for boolean option "security_barrier": 100
-CREATE VIEW mysecview6 WITH (invalid_option) -- Error
+CREATE VIEW mysecview9 WITH (security_invoker=100) -- Error
+ AS SELECT * FROM tbl1 WHERE a = 100;
+ERROR: invalid value for boolean option "security_invoker": 100
+CREATE VIEW mysecview10 WITH (invalid_option) -- Error

I see no reasons to remove two of the existing tests.

+++ b/src/test/regress/expected/rowsecurity.out
@@ -8,9 +8,11 @@ DROP USER IF EXISTS regress_rls_alice;
DROP USER IF EXISTS regress_rls_bob;
DROP USER IF EXISTS regress_rls_carol;
DROP USER IF EXISTS regress_rls_dave;
+DROP USER IF EXISTS regress_rls_grace;

But the name has to start with "e"!

I also see no reason to split a small patch like this into three parts.

In the attached, I dealt with the above and went over the comments.
How do you like it?

Yours,
Laurenz Albe
>