aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/backend/executor/nodeModifyTable.c7
-rw-r--r--src/backend/rewrite/rowsecurity.c85
-rw-r--r--src/test/regress/expected/rowsecurity.out58
-rw-r--r--src/test/regress/sql/rowsecurity.sql52
4 files changed, 152 insertions, 50 deletions
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 2a5fec8d017..5005d8c0d12 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2861,13 +2861,14 @@ lmerge_matched:
* UPDATE/DELETE RLS policies. If those checks fail, we throw an
* error.
*
- * The WITH CHECK quals are applied in ExecUpdate() and hence we need
- * not do anything special to handle them.
+ * The WITH CHECK quals for UPDATE RLS policies are applied in
+ * ExecUpdateAct() and hence we need not do anything special to handle
+ * them.
*
* NOTE: We must do this after WHEN quals are evaluated, so that we
* check policies only when they matter.
*/
- if (resultRelInfo->ri_WithCheckOptions)
+ if (resultRelInfo->ri_WithCheckOptions && commandType != CMD_NOTHING)
{
ExecWithCheckOptions(commandType == CMD_UPDATE ?
WCO_RLS_MERGE_UPDATE_CHECK : WCO_RLS_MERGE_DELETE_CHECK,
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 5c3fe4eda28..b1620e4625d 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -394,7 +394,11 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
* and set them up so that we can enforce the appropriate policy depending
* on the final action we take.
*
- * We already fetched the SELECT policies above.
+ * We already fetched the SELECT policies above, to check existing rows,
+ * but we must also check that new rows created by UPDATE actions are
+ * visible, if SELECT rights are required for this relation. We don't do
+ * this for INSERT actions, since an INSERT command would only do this
+ * check if it had a RETURNING list, and MERGE does not support RETURNING.
*
* We don't push the UPDATE/DELETE USING quals to the RTE because we don't
* really want to apply them while scanning the relation since we don't
@@ -410,16 +414,20 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
*/
if (commandType == CMD_MERGE)
{
- List *merge_permissive_policies;
- List *merge_restrictive_policies;
+ List *merge_update_permissive_policies;
+ List *merge_update_restrictive_policies;
+ List *merge_delete_permissive_policies;
+ List *merge_delete_restrictive_policies;
+ List *merge_insert_permissive_policies;
+ List *merge_insert_restrictive_policies;
/*
* Fetch the UPDATE policies and set them up to execute on the
* existing target row before doing UPDATE.
*/
get_policies_for_relation(rel, CMD_UPDATE, user_id,
- &merge_permissive_policies,
- &merge_restrictive_policies);
+ &merge_update_permissive_policies,
+ &merge_update_restrictive_policies);
/*
* WCO_RLS_MERGE_UPDATE_CHECK is used to check UPDATE USING quals on
@@ -427,23 +435,59 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
*/
add_with_check_options(rel, rt_index,
WCO_RLS_MERGE_UPDATE_CHECK,
- merge_permissive_policies,
- merge_restrictive_policies,
+ merge_update_permissive_policies,
+ merge_update_restrictive_policies,
withCheckOptions,
hasSubLinks,
true);
+ /* Enforce the WITH CHECK clauses of the UPDATE policies */
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_UPDATE_CHECK,
+ merge_update_permissive_policies,
+ merge_update_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ false);
+
+ /*
+ * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure
+ * that the updated row is visible when executing an UPDATE action, if
+ * SELECT rights are required for this relation.
+ */
+ if (perminfo->requiredPerms & ACL_SELECT)
+ {
+ List *merge_select_permissive_policies;
+ List *merge_select_restrictive_policies;
+
+ get_policies_for_relation(rel, CMD_SELECT, user_id,
+ &merge_select_permissive_policies,
+ &merge_select_restrictive_policies);
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_UPDATE_CHECK,
+ merge_select_permissive_policies,
+ merge_select_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ true);
+ }
+
/*
- * Same with DELETE policies.
+ * Fetch the DELETE policies and set them up to execute on the
+ * existing target row before doing DELETE.
*/
get_policies_for_relation(rel, CMD_DELETE, user_id,
- &merge_permissive_policies,
- &merge_restrictive_policies);
+ &merge_delete_permissive_policies,
+ &merge_delete_restrictive_policies);
+ /*
+ * WCO_RLS_MERGE_DELETE_CHECK is used to check DELETE USING quals on
+ * the existing target row.
+ */
add_with_check_options(rel, rt_index,
WCO_RLS_MERGE_DELETE_CHECK,
- merge_permissive_policies,
- merge_restrictive_policies,
+ merge_delete_permissive_policies,
+ merge_delete_restrictive_policies,
withCheckOptions,
hasSubLinks,
true);
@@ -454,22 +498,13 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
* withCheckOptions.
*/
get_policies_for_relation(rel, CMD_INSERT, user_id,
- &merge_permissive_policies,
- &merge_restrictive_policies);
+ &merge_insert_permissive_policies,
+ &merge_insert_restrictive_policies);
add_with_check_options(rel, rt_index,
WCO_RLS_INSERT_CHECK,
- merge_permissive_policies,
- merge_restrictive_policies,
- withCheckOptions,
- hasSubLinks,
- false);
-
- /* Enforce the WITH CHECK clauses of the UPDATE policies */
- add_with_check_options(rel, rt_index,
- WCO_RLS_UPDATE_CHECK,
- merge_permissive_policies,
- merge_restrictive_policies,
+ merge_insert_permissive_policies,
+ merge_insert_restrictive_policies,
withCheckOptions,
hasSubLinks,
false);
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 4e549766184..97ca9bf72c5 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2121,10 +2121,10 @@ ALTER TABLE document ADD COLUMN dnotes text DEFAULT '';
CREATE POLICY p1 ON document FOR SELECT USING (true);
-- one may insert documents only authored by them
CREATE POLICY p2 ON document FOR INSERT WITH CHECK (dauthor = current_user);
--- one may only update documents in 'novel' category
+-- one may only update documents in 'novel' category and new dlevel must be > 0
CREATE POLICY p3 ON document FOR UPDATE
USING (cid = (SELECT cid from category WHERE cname = 'novel'))
- WITH CHECK (dauthor = current_user);
+ WITH CHECK (dlevel > 0);
-- one may only delete documents in 'manga' category
CREATE POLICY p4 ON document FOR DELETE
USING (cid = (SELECT cid from category WHERE cname = 'manga'));
@@ -2148,12 +2148,12 @@ SELECT * FROM document;
(14 rows)
SET SESSION AUTHORIZATION regress_rls_bob;
--- Fails, since update violates WITH CHECK qual on dauthor
+-- Fails, since update violates WITH CHECK qual on dlevel
MERGE INTO document d
USING (SELECT 1 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
- UPDATE SET dnotes = dnotes || ' notes added by merge1 ', dauthor = 'regress_rls_alice';
+ UPDATE SET dnotes = dnotes || ' notes added by merge1 ', dlevel = 0;
ERROR: new row violates row-level security policy for table "document"
-- Should be OK since USING and WITH CHECK quals pass
MERGE INTO document d
@@ -2161,12 +2161,12 @@ USING (SELECT 1 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
UPDATE SET dnotes = dnotes || ' notes added by merge2 ';
--- Even when dauthor is updated explicitly, but to the existing value
+-- Even when dlevel is updated explicitly, but to the existing value
MERGE INTO document d
USING (SELECT 1 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
- UPDATE SET dnotes = dnotes || ' notes added by merge3 ', dauthor = 'regress_rls_bob';
+ UPDATE SET dnotes = dnotes || ' notes added by merge3 ', dlevel = 1;
-- There is a MATCH for did = 3, but UPDATE's USING qual does not allow
-- updating an item in category 'science fiction'
MERGE INTO document d
@@ -2205,6 +2205,14 @@ WHEN MATCHED AND dnotes <> '' THEN
WHEN MATCHED THEN
DELETE;
ERROR: target row violates row-level security policy (USING expression) for table "document"
+-- OK if DELETE is replaced with DO NOTHING
+MERGE INTO document d
+USING (SELECT 4 as sdid) s
+ON did = s.sdid
+WHEN MATCHED AND dnotes <> '' THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge '
+WHEN MATCHED THEN
+ DO NOTHING;
SELECT * FROM document WHERE did = 4;
did | cid | dlevel | dauthor | dtitle | dnotes
-----+-----+--------+-----------------+----------------+--------
@@ -2253,30 +2261,53 @@ WHEN MATCHED THEN
WHEN NOT MATCHED THEN
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
-- drop and create a new SELECT policy which prevents us from reading
--- any document except with category 'magna'
+-- any document except with category 'novel'
RESET SESSION AUTHORIZATION;
DROP POLICY p1 ON document;
CREATE POLICY p1 ON document FOR SELECT
- USING (cid = (SELECT cid from category WHERE cname = 'manga'));
+ USING (cid = (SELECT cid from category WHERE cname = 'novel'));
SET SESSION AUTHORIZATION regress_rls_bob;
-- MERGE can no longer see the matching row and hence attempts the
-- NOT MATCHED action, which results in unique key violation
MERGE INTO document d
-USING (SELECT 1 as sdid) s
+USING (SELECT 7 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
UPDATE SET dnotes = dnotes || ' notes added by merge5 '
WHEN NOT MATCHED THEN
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
ERROR: duplicate key value violates unique constraint "document_pkey"
+-- UPDATE action fails if new row is not visible
+MERGE INTO document d
+USING (SELECT 1 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge6 ',
+ cid = (SELECT cid from category WHERE cname = 'technology');
+ERROR: new row violates row-level security policy for table "document"
+-- but OK if new row is visible
+MERGE INTO document d
+USING (SELECT 1 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge7 ',
+ cid = (SELECT cid from category WHERE cname = 'novel');
+-- OK to insert a new row that is not visible
+MERGE INTO document d
+USING (SELECT 13 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge8 '
+WHEN NOT MATCHED THEN
+ INSERT VALUES (13, 44, 1, 'regress_rls_bob', 'new manga');
RESET SESSION AUTHORIZATION;
-- drop the restrictive SELECT policy so that we can look at the
-- final state of the table
DROP POLICY p1 ON document;
-- Just check everything went per plan
SELECT * FROM document;
- did | cid | dlevel | dauthor | dtitle | dnotes
------+-----+--------+-------------------+----------------------------------+-----------------------------------------------------------------------
+ did | cid | dlevel | dauthor | dtitle | dnotes
+-----+-----+--------+-------------------+----------------------------------+----------------------------------------------------------------------------------------------
3 | 22 | 2 | regress_rls_bob | my science fiction |
5 | 44 | 2 | regress_rls_bob | my second manga |
6 | 22 | 1 | regress_rls_carol | great science fiction |
@@ -2290,8 +2321,9 @@ SELECT * FROM document;
78 | 33 | 1 | regress_rls_bob | some technology novel |
79 | 33 | 1 | regress_rls_bob | technology book, can only insert |
12 | 11 | 1 | regress_rls_bob | another novel |
- 1 | 11 | 1 | regress_rls_bob | my first novel | notes added by merge2 notes added by merge3 notes added by merge4
-(14 rows)
+ 1 | 11 | 1 | regress_rls_bob | my first novel | notes added by merge2 notes added by merge3 notes added by merge4 notes added by merge7
+ 13 | 44 | 1 | regress_rls_bob | new manga |
+(15 rows)
--
-- ROLE/GROUP
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 3d664538a69..dec7340538c 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -821,10 +821,10 @@ ALTER TABLE document ADD COLUMN dnotes text DEFAULT '';
CREATE POLICY p1 ON document FOR SELECT USING (true);
-- one may insert documents only authored by them
CREATE POLICY p2 ON document FOR INSERT WITH CHECK (dauthor = current_user);
--- one may only update documents in 'novel' category
+-- one may only update documents in 'novel' category and new dlevel must be > 0
CREATE POLICY p3 ON document FOR UPDATE
USING (cid = (SELECT cid from category WHERE cname = 'novel'))
- WITH CHECK (dauthor = current_user);
+ WITH CHECK (dlevel > 0);
-- one may only delete documents in 'manga' category
CREATE POLICY p4 ON document FOR DELETE
USING (cid = (SELECT cid from category WHERE cname = 'manga'));
@@ -833,12 +833,12 @@ SELECT * FROM document;
SET SESSION AUTHORIZATION regress_rls_bob;
--- Fails, since update violates WITH CHECK qual on dauthor
+-- Fails, since update violates WITH CHECK qual on dlevel
MERGE INTO document d
USING (SELECT 1 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
- UPDATE SET dnotes = dnotes || ' notes added by merge1 ', dauthor = 'regress_rls_alice';
+ UPDATE SET dnotes = dnotes || ' notes added by merge1 ', dlevel = 0;
-- Should be OK since USING and WITH CHECK quals pass
MERGE INTO document d
@@ -847,12 +847,12 @@ ON did = s.sdid
WHEN MATCHED THEN
UPDATE SET dnotes = dnotes || ' notes added by merge2 ';
--- Even when dauthor is updated explicitly, but to the existing value
+-- Even when dlevel is updated explicitly, but to the existing value
MERGE INTO document d
USING (SELECT 1 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
- UPDATE SET dnotes = dnotes || ' notes added by merge3 ', dauthor = 'regress_rls_bob';
+ UPDATE SET dnotes = dnotes || ' notes added by merge3 ', dlevel = 1;
-- There is a MATCH for did = 3, but UPDATE's USING qual does not allow
-- updating an item in category 'science fiction'
@@ -892,6 +892,15 @@ WHEN MATCHED AND dnotes <> '' THEN
WHEN MATCHED THEN
DELETE;
+-- OK if DELETE is replaced with DO NOTHING
+MERGE INTO document d
+USING (SELECT 4 as sdid) s
+ON did = s.sdid
+WHEN MATCHED AND dnotes <> '' THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge '
+WHEN MATCHED THEN
+ DO NOTHING;
+
SELECT * FROM document WHERE did = 4;
-- Switch to regress_rls_carol role and try the DELETE again. It should succeed
@@ -941,24 +950,49 @@ WHEN NOT MATCHED THEN
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
-- drop and create a new SELECT policy which prevents us from reading
--- any document except with category 'magna'
+-- any document except with category 'novel'
RESET SESSION AUTHORIZATION;
DROP POLICY p1 ON document;
CREATE POLICY p1 ON document FOR SELECT
- USING (cid = (SELECT cid from category WHERE cname = 'manga'));
+ USING (cid = (SELECT cid from category WHERE cname = 'novel'));
SET SESSION AUTHORIZATION regress_rls_bob;
-- MERGE can no longer see the matching row and hence attempts the
-- NOT MATCHED action, which results in unique key violation
MERGE INTO document d
-USING (SELECT 1 as sdid) s
+USING (SELECT 7 as sdid) s
ON did = s.sdid
WHEN MATCHED THEN
UPDATE SET dnotes = dnotes || ' notes added by merge5 '
WHEN NOT MATCHED THEN
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
+-- UPDATE action fails if new row is not visible
+MERGE INTO document d
+USING (SELECT 1 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge6 ',
+ cid = (SELECT cid from category WHERE cname = 'technology');
+
+-- but OK if new row is visible
+MERGE INTO document d
+USING (SELECT 1 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge7 ',
+ cid = (SELECT cid from category WHERE cname = 'novel');
+
+-- OK to insert a new row that is not visible
+MERGE INTO document d
+USING (SELECT 13 as sdid) s
+ON did = s.sdid
+WHEN MATCHED THEN
+ UPDATE SET dnotes = dnotes || ' notes added by merge8 '
+WHEN NOT MATCHED THEN
+ INSERT VALUES (13, 44, 1, 'regress_rls_bob', 'new manga');
+
RESET SESSION AUTHORIZATION;
-- drop the restrictive SELECT policy so that we can look at the
-- final state of the table
generated by cgit v1.2.3 (git 2.25.1) at 2025-05-30 16:39:47 +0000