From a33007b5748e62d539d32140f9116e3f29335c21 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 6 Jan 2015 16:32:21 -0800 Subject: [PATCH 4/4] RLS support for ON CONFLICT UPDATE Row-Level Security policies may apply to UPDATE commands or INSERT commands only. UPDATE RLS policies can have both USING() security barrier quals, and CHECK options (INSERT RLS policies may only have CHECK options, though). It is necessary to carefully consider the behavior of RLS policies in the context of INSERT with ON CONFLICT UPDATE, since ON CONFLICT UPDATE is more or less a new top-level command, conceptually quite different to two separate statements (an INSERT and an UPDATE). The approach taken is to "bunch together" both sets of policies, and to enforce them in 3 different places against three different slots (3 different stages of query processing in the executor). Note that UPDATE policy USING() barrier quals are always treated as CHECK options. It is thought that silently failing when USING() barrier quals are not satisfied is a more surprising outcome, even if it is closer to the existing behavior of UPDATE statements. This is because the user's intent to UPDATE one particular row based on simple criteria is quite clear with ON CONFLICT UPDATE. The 3 places that RLS policies are enforced are: * Against row actually inserted, after insertion proceeds successfully (INSERT-applicable policies only). * Against row in target table that caused conflict. The implementation is careful not to leak the contents of that row in diagnostic messages (INSERT-applicable *and* UPDATE-applicable policies). * Against the version of the row added by to the relation after ExecUpdate() is called (INSERT-applicable *and* UPDATE-applicable policies). --- doc/src/sgml/ref/alter_policy.sgml | 7 ++- doc/src/sgml/ref/create_policy.sgml | 37 ++++++++++-- src/backend/executor/execMain.c | 25 +++++--- src/backend/executor/nodeModifyTable.c | 53 ++++++++++++++++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/rewrite/rewriteHandler.c | 2 + src/backend/rewrite/rowsecurity.c | 94 ++++++++++++++++++++++++++----- src/include/executor/executor.h | 3 +- src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/rowsecurity.out | 90 +++++++++++++++++++++++++++++ src/test/regress/sql/rowsecurity.sql | 73 ++++++++++++++++++++++++ 14 files changed, 357 insertions(+), 32 deletions(-) diff --git a/doc/src/sgml/ref/alter_policy.sgml b/doc/src/sgml/ref/alter_policy.sgml index 6d03db5..65cd85c 100644 --- a/doc/src/sgml/ref/alter_policy.sgml +++ b/doc/src/sgml/ref/alter_policy.sgml @@ -93,8 +93,11 @@ ALTER POLICY name ON documentation for + INSERT with ON CONFLICT UPDATE). + The USING expression applies to records which are being retrieved from the + table. diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml index 868a6c1..f17192e 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -70,11 +70,12 @@ CREATE POLICY name ON INSERT with ON CONFLICT UPDATE). + Further, for commands which can have both USING and WITH CHECK policies (ALL + and UPDATE), if no WITH CHECK policy is defined then the USING policy will + be used for both what rows are visible (normal USING case) and which rows + will be allowed to be added (WITH CHECK case). @@ -255,6 +256,19 @@ CREATE POLICY name ON + + Note that INSERT with ON CONFLICT + UPDATE requires that an INSERT policy WITH + CHECK expression also passes for both any existing tuple in the target + table that necessitates that the UPDATE path be + taken, and the final tuple added back into the relation. + INSERT policies are separately combined using + OR, and this distinct set of policy expressions must + always pass, regardless of whether any or all UPDATE + policies also pass (in the same tuple check). However, successfully + inserted tuples are not subject to UPDATE policy + enforcement. + @@ -263,7 +277,9 @@ CREATE POLICY name ON Using UPDATE for a policy means that it will apply - to UPDATE commands. As UPDATE + to UPDATE commands (or auxiliary ON + CONFLICT UPDATE clauses of INSERT + commands). As UPDATE involves pulling an existing record and then making changes to some portion (but possibly not all) of the record, the UPDATE policy accepts both a USING expression and @@ -279,6 +295,15 @@ CREATE POLICY name ON USING and WITH CHECK cases. + + Note that INSERT with ON CONFLICT + UPDATE requires that an UPDATE policy + USING expression always be treated as a WITH CHECK + expression. This UPDATE policy must + always pass, regardless of whether any + INSERT policy also passes in the same + tuple check. + diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 9ef4179..6b6d4dd 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1699,7 +1699,8 @@ ExecConstraints(ResultRelInfo *resultRelInfo, */ void ExecWithCheckOptions(ResultRelInfo *resultRelInfo, - TupleTableSlot *slot, EState *estate) + TupleTableSlot *slot, bool detail, + bool onlyInsert, EState *estate) { Relation rel = resultRelInfo->ri_RelationDesc; TupleDesc tupdesc = RelationGetDescr(rel); @@ -1724,6 +1725,15 @@ ExecWithCheckOptions(ResultRelInfo *resultRelInfo, ExprState *wcoExpr = (ExprState *) lfirst(l2); /* + * INSERT ... ON CONFLICT UPDATE callers may require that not all WITH + * CHECK OPTIONs associated with resultRelInfo are enforced at all + * stages of query processing. (UPDATE-related policies are not + * enforced in respect of a successfully inserted tuple). + */ + if (onlyInsert && wco->commandType == CMD_UPDATE) + continue; + + /* * WITH CHECK OPTION checks are intended to ensure that the new tuple * is visible (in the case of a view) or that it passes the * 'with-check' policy (in the case of row security). @@ -1734,16 +1744,17 @@ ExecWithCheckOptions(ResultRelInfo *resultRelInfo, */ if (!ExecQual((List *) wcoExpr, econtext, false)) { - char *val_desc; + char *val_desc = NULL; Bitmapset *modifiedCols; modifiedCols = GetUpdatedColumns(resultRelInfo, estate); modifiedCols = bms_union(modifiedCols, GetInsertedColumns(resultRelInfo, estate)); - val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), - slot, - tupdesc, - modifiedCols, - 64); + if (detail) + val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + slot, + tupdesc, + modifiedCols, + 64); ereport(ERROR, (errcode(ERRCODE_WITH_CHECK_OPTION_VIOLATION), diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 5a2ac21..ac95eb9 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -455,7 +455,8 @@ vlock: /* Check any WITH CHECK OPTION constraints */ if (resultRelInfo->ri_WithCheckOptions != NIL) - ExecWithCheckOptions(resultRelInfo, slot, estate); + ExecWithCheckOptions(resultRelInfo, slot, true, spec == SPEC_INSERT, + estate); /* Process RETURNING if present */ if (resultRelInfo->ri_projectReturning) @@ -949,7 +950,7 @@ lreplace:; /* Check any WITH CHECK OPTION constraints */ if (resultRelInfo->ri_WithCheckOptions != NIL) - ExecWithCheckOptions(resultRelInfo, slot, estate); + ExecWithCheckOptions(resultRelInfo, slot, true, false, estate); /* Process RETURNING if present */ if (resultRelInfo->ri_projectReturning) @@ -1135,6 +1136,54 @@ ExecLockUpdateTuple(ResultRelInfo *resultRelInfo, slot = EvalPlanQualNext(&onConflict->mt_epqstate); + /* + * For RLS with ON CONFLICT UPDATE, security quals are always + * treated as WITH CHECK options, even when there were separate + * security quals and explicit WITH CHECK options (ordinarily, + * security quals are only treated as WITH CHECK options when there + * are no explicit WITH CHECK options). Also, CHECK OPTIONs + * (originating either explicitly, or implicitly as security quals) + * for both UPDATE and INSERT policies (or ALL policies) are + * checked (as CHECK OPTIONs) at three different points for three + * distinct but related tuples/slots in the context of ON CONFLICT + * UPDATE. There are three relevant ExecWithCheckOptions() calls: + * + * * After successful insertion, within ExecInsert(), against the + * inserted tuple. This only includes INSERT-applicable policies. + * + * * Here, after row locking but before calling ExecUpdate(), on + * the existing tuple in the target relation (which we cannot leak + * details of). This is conceptually like a security barrier qual + * for the purposes of the auxiliary update, although unlike + * regular updates that require security barrier quals we prefer to + * raise an error (by treating the security barrier quals as CHECK + * OPTIONS) rather than silently not affect rows, because the + * intent to update seems clear and unambiguous for ON CONFLICT + * UPDATE. This includes both INSERT-applicable and + * UPDATE-applicable policies. + * + * * On the final tuple created by the update within ExecUpdate (if + * any). This is also subject to INSERT policy enforcement, unlike + * conventional ExecUpdate() calls for UPDATE statements -- it + * includes both INSERT-applicable and UPDATE-applicable policies. + */ + if (resultRelInfo->ri_WithCheckOptions != NIL) + { + TupleTableSlot *opts; + + /* Construct temp slot for locked tuple from target */ + opts = MakeSingleTupleTableSlot(slot->tts_tupleDescriptor); + ExecStoreTuple(copyTuple, opts, InvalidBuffer, false); + + /* + * Check, but without leaking contents of tuple; user only + * supplied one conflicting value or composition of values, and + * not the entire tuple. + */ + ExecWithCheckOptions(resultRelInfo, opts, false, false, + estate); + } + if (!TupIsNull(slot)) *returning = ExecUpdate(&tuple.t_data->t_ctid, NULL, slot, planSlot, &onConflict->mt_epqstate, diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 9368ab2..b05cd2b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2083,6 +2083,7 @@ _copyWithCheckOption(const WithCheckOption *from) COPY_STRING_FIELD(viewname); COPY_NODE_FIELD(qual); + COPY_SCALAR_FIELD(commandType); COPY_SCALAR_FIELD(cascaded); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 30c164a..33129fb 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2379,6 +2379,7 @@ _equalWithCheckOption(const WithCheckOption *a, const WithCheckOption *b) { COMPARE_STRING_FIELD(viewname); COMPARE_NODE_FIELD(qual); + COMPARE_SCALAR_FIELD(commandType); COMPARE_SCALAR_FIELD(cascaded); return true; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index de08bb4..9d775d4 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2349,6 +2349,7 @@ _outWithCheckOption(StringInfo str, const WithCheckOption *node) WRITE_STRING_FIELD(viewname); WRITE_NODE_FIELD(qual); + WRITE_ENUM_FIELD(commandType, CmdType); WRITE_BOOL_FIELD(cascaded); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 319db56..f9910f3 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -272,6 +272,7 @@ _readWithCheckOption(void) READ_STRING_FIELD(viewname); READ_NODE_FIELD(qual); + READ_ENUM_FIELD(commandType, CmdType); READ_BOOL_FIELD(cascaded); READ_DONE(); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 8ef80d6..36d40cf 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1767,6 +1767,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown) List *quals = NIL; wco = (WithCheckOption *) makeNode(WithCheckOption); + wco->commandType = parsetree->commandType; quals = lcons(wco->qual, quals); activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs); @@ -2935,6 +2936,7 @@ rewriteTargetView(Query *parsetree, Relation view) wco->viewname = pstrdup(RelationGetRelationName(view)); wco->qual = NULL; wco->cascaded = cascaded; + wco->commandType = viewquery->commandType; parsetree->withCheckOptions = lcons(wco, parsetree->withCheckOptions); diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 7669130..09f1ac3 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -56,12 +56,14 @@ #include "utils/syscache.h" #include "tcop/utility.h" -static List *pull_row_security_policies(CmdType cmd, Relation relation, - Oid user_id); +static List *pull_row_security_policies(CmdType cmd, bool onConflict, + Relation relation, Oid user_id); static void process_policies(List *policies, int rt_index, Expr **final_qual, Expr **final_with_check_qual, - bool *hassublinks); + bool *hassublinks, + Expr **spec_with_check_eval, + bool onConflict); static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id); /* @@ -88,6 +90,7 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) Expr *rowsec_with_check_expr = NULL; Expr *hook_expr = NULL; Expr *hook_with_check_expr = NULL; + Expr *hook_spec_with_check_expr = NULL; List *rowsec_policies; List *hook_policies = NIL; @@ -149,8 +152,9 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) /* Grab the built-in policies which should be applied to this relation. */ rel = heap_open(rte->relid, NoLock); - rowsec_policies = pull_row_security_policies(root->commandType, rel, - user_id); + rowsec_policies = pull_row_security_policies(root->commandType, + root->specClause == SPEC_INSERT, + rel, user_id); /* * Check if this is only the default-deny policy. @@ -168,7 +172,9 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) /* Now that we have our policies, build the expressions from them. */ process_policies(rowsec_policies, rt_index, &rowsec_expr, - &rowsec_with_check_expr, &hassublinks); + &rowsec_with_check_expr, &hassublinks, + &hook_spec_with_check_expr, + root->specClause == SPEC_INSERT); /* * Also, allow extensions to add their own policies. @@ -198,7 +204,9 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) /* Build the expression from any policies returned. */ process_policies(hook_policies, rt_index, &hook_expr, - &hook_with_check_expr, &hassublinks); + &hook_with_check_expr, &hassublinks, + &hook_spec_with_check_expr, + root->specClause == SPEC_INSERT); } /* @@ -230,6 +238,7 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) rowsec_with_check_expr; wco->cascaded = false; + wco->commandType = root->commandType; root->withCheckOptions = lcons(wco, root->withCheckOptions); } @@ -244,6 +253,23 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) hook_with_check_expr; wco->cascaded = false; + wco->commandType = root->commandType; + root->withCheckOptions = lcons(wco, root->withCheckOptions); + } + + /* + * Also add the expression, if any, returned from the extension that + * applies to auxiliary UPDATE within ON CONFLICT UPDATE. + */ + if (hook_spec_with_check_expr) + { + WithCheckOption *wco; + + wco = (WithCheckOption *) makeNode(WithCheckOption); + wco->viewname = RelationGetRelationName(rel); + wco->qual = (Node *) hook_spec_with_check_expr; + wco->cascaded = false; + wco->commandType = CMD_UPDATE; root->withCheckOptions = lcons(wco, root->withCheckOptions); } } @@ -288,7 +314,8 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) * */ static List * -pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id) +pull_row_security_policies(CmdType cmd, bool onConflict, Relation relation, + Oid user_id) { List *policies = NIL; ListCell *item; @@ -322,7 +349,9 @@ pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id) if (policy->polcmd == ACL_INSERT_CHR && check_role_for_policy(policy->roles, user_id)) policies = lcons(policy, policies); - break; + if (!onConflict) + break; + /* FALL THRU */ case CMD_UPDATE: if (policy->polcmd == ACL_UPDATE_CHR && check_role_for_policy(policy->roles, user_id)) @@ -384,26 +413,41 @@ pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id) */ static void process_policies(List *policies, int rt_index, Expr **qual_eval, - Expr **with_check_eval, bool *hassublinks) + Expr **with_check_eval, bool *hassublinks, + Expr **spec_with_check_eval, bool onConflict) { ListCell *item; List *quals = NIL; List *with_check_quals = NIL; + List *conflict_update_quals = NIL; /* * Extract the USING and WITH CHECK quals from each of the policies - * and add them to our lists. + * and add them to our lists. CONFLICT UPDATE quals are always treated + * as CHECK OPTIONS. */ foreach(item, policies) { RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item); if (policy->qual != NULL) - quals = lcons(copyObject(policy->qual), quals); + { + if (!onConflict || policy->polcmd != ACL_UPDATE_CHR) + quals = lcons(copyObject(policy->qual), quals); + else + conflict_update_quals = lcons(copyObject(policy->qual), quals); + } if (policy->with_check_qual != NULL) - with_check_quals = lcons(copyObject(policy->with_check_qual), - with_check_quals); + { + if (!onConflict || policy->polcmd != ACL_UPDATE_CHR) + with_check_quals = lcons(copyObject(policy->with_check_qual), + with_check_quals); + else + conflict_update_quals = + lcons(copyObject(policy->with_check_qual), + conflict_update_quals); + } if (policy->hassublinks) *hassublinks = true; @@ -420,6 +464,10 @@ process_policies(List *policies, int rt_index, Expr **qual_eval, /* * If we end up with only USING quals, then use those as * WITH CHECK quals also. + * + * For the INSERT with ON CONFLICT UPDATE case, we always enforce that the + * UPDATE's USING quals are treated like WITH CHECK quals, enforced against + * the target relation's tuple in multiple places. */ if (with_check_quals == NIL) with_check_quals = copyObject(quals); @@ -453,6 +501,24 @@ process_policies(List *policies, int rt_index, Expr **qual_eval, else *with_check_eval = (Expr*) linitial(with_check_quals); + /* + * For INSERT with ON CONFLICT UPDATE, *both* sets of WITH CHECK options + * (from any INSERT policy and any UPDATE policy) are enforced. + * + * These are handled separately because enforcement of each type of WITH + * CHECK option is based on the point in query processing of INSERT ... ON + * CONFLICT UPDATE. The INSERT path does not enforce UPDATE related CHECK + * OPTIONs. + */ + if (conflict_update_quals != NIL) + { + if (list_length(conflict_update_quals) > 1) + *spec_with_check_eval = makeBoolExpr(AND_EXPR, + conflict_update_quals, -1); + else + *spec_with_check_eval = (Expr*) linitial(conflict_update_quals); + } + return; } diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 8100bd8..91ba93e 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -194,7 +194,8 @@ extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids); extern void ExecConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern void ExecWithCheckOptions(ResultRelInfo *resultRelInfo, - TupleTableSlot *slot, EState *estate); + TupleTableSlot *slot, bool detail, bool onlyInsert, + EState *estate); extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index fe4c69a..c23a8fe 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -886,6 +886,7 @@ typedef struct WithCheckOption NodeTag type; char *viewname; /* name of view that specified the WCO */ Node *qual; /* constraint qual to check */ + CmdType commandType; /* select|insert|update|delete */ bool cascaded; /* true = WITH CASCADED CHECK OPTION */ } WithCheckOption; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 44e8dab..eb234b5 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -1193,6 +1193,96 @@ NOTICE: f_leak => yyyyyy (3 rows) -- +-- INSERT ... ON CONFLICT UPDATE and Row-level security +-- +-- Would fail with unique violation, but with ON CONFLICT fails as row is +-- locked for update (notably, existing/target row is not leaked): +INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my fourth manga') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Can't insert new violating tuple, either: +INSERT INTO document VALUES (22, 11, 2, 'rls_regress_user2', 'mediocre novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; +ERROR: new row violates WITH CHECK OPTION for "document" +-- INSERT path is taken here, so UPDATE targelist doesn't matter: +INSERT INTO document VALUES (33, 22, 1, 'rls_regress_user1', 'okay science fiction') + ON CONFLICT (did) UPDATE SET dauthor = 'rls_regress_user3' RETURNING *; + did | cid | dlevel | dauthor | dtitle +-----+-----+--------+-------------------+---------------------- + 33 | 22 | 1 | rls_regress_user1 | okay science fiction +(1 row) + +-- Update path will now taken for same query, so UPDATE targelist now matters +-- (this is the same query as the last, but now fails): +INSERT INTO document VALUES (33, 22, 1, 'rls_regress_user1', 'okay science fiction') + ON CONFLICT (did) UPDATE SET dauthor = 'rls_regress_user3' RETURNING *; +ERROR: new row violates WITH CHECK OPTION for "document" +SET SESSION AUTHORIZATION rls_regress_user0; +DROP POLICY p1 ON document; +CREATE POLICY p1 ON document FOR SELECT USING (true); +CREATE POLICY p2 ON document FOR INSERT WITH CHECK (dauthor = current_user); +CREATE POLICY p3 ON document FOR UPDATE + USING (cid = (SELECT cid from category WHERE cname = 'novel')) + WITH CHECK (dauthor = current_user); +SET SESSION AUTHORIZATION rls_regress_user1; +-- Again, would fail with unique violation, but with ON CONFLICT fails as row is +-- locked for update (notably, existing/target row is not leaked, which is what +-- failed to satisfy WITH CHECK options - not row proposed for insertion by +-- user): +INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my fourth manga') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Again, can't insert new violating tuple, either (unsuccessfully inserted tuple +-- values are reported here, though) +-- +-- Violates actual CHECK OPTION within UPDATE: +INSERT INTO document VALUES (2, 11, 1, 'rls_regress_user2', 'my first novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, dauthor = EXCLUDED.dauthor; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Violates USING qual for UPDATE policy p3, interpreted here as CHECK OPTION. +-- +-- UPDATE path is taken, but UPDATE fails purely because *existing* row to be +-- updated is not a "novel"/cid 11 (row is not leaked, even though we have +-- SELECT privileges sufficient to see the row in this instance): +INSERT INTO document VALUES (33, 11, 1, 'rls_regress_user1', 'Some novel, replaces sci-fi') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Fine (we UPDATE): +INSERT INTO document VALUES (2, 11, 1, 'rls_regress_user1', 'my first novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle RETURNING *; + did | cid | dlevel | dauthor | dtitle +-----+-----+--------+-------------------+---------------- + 2 | 11 | 2 | rls_regress_user1 | my first novel +(1 row) + +-- Fine (we INSERT, so "cid = 33" isn't evaluated): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33 RETURNING *; + did | cid | dlevel | dauthor | dtitle +-----+-----+--------+-------------------+------------------ + 78 | 11 | 1 | rls_regress_user1 | some other novel +(1 row) + +-- Fail (same query, but we UPDATE, so "cid = 33" is evaluated at end of +-- UPDATE): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33 RETURNING *; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Fail (we UPDATE, so dauthor assignment is evaluated at end of UPDATE): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, dauthor = 'rls_regress_user2'; +ERROR: new row violates WITH CHECK OPTION for "document" +-- Don't fail because INSERT doesn't satisfy WITH CHECK option that originated +-- as a barrier/USING() qual from the UPDATE. Note that the UPDATE path +-- *isn't* taken, and so UPDATE-related policy does not apply: +INSERT INTO document VALUES (88, 33, 1, 'rls_regress_user1', 'technology book, can only insert') + ON CONFLICT (did) UPDATE SET dtitle = upper(EXCLUDED.dtitle) RETURNING *; + did | cid | dlevel | dauthor | dtitle +-----+-----+--------+-------------------+---------------------------------- + 88 | 33 | 1 | rls_regress_user1 | technology book, can only insert +(1 row) + +-- -- ROLE/GROUP -- SET SESSION AUTHORIZATION rls_regress_user0; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index ed7adbf..5c660d5 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -436,6 +436,79 @@ DELETE FROM only t1 WHERE f_leak(b) RETURNING oid, *, t1; DELETE FROM t1 WHERE f_leak(b) RETURNING oid, *, t1; -- +-- INSERT ... ON CONFLICT UPDATE and Row-level security +-- + +-- Would fail with unique violation, but with ON CONFLICT fails as row is +-- locked for update (notably, existing/target row is not leaked): +INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my fourth manga') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; + +-- Can't insert new violating tuple, either: +INSERT INTO document VALUES (22, 11, 2, 'rls_regress_user2', 'mediocre novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; + +-- INSERT path is taken here, so UPDATE targelist doesn't matter: +INSERT INTO document VALUES (33, 22, 1, 'rls_regress_user1', 'okay science fiction') + ON CONFLICT (did) UPDATE SET dauthor = 'rls_regress_user3' RETURNING *; + +-- Update path will now taken for same query, so UPDATE targelist now matters +-- (this is the same query as the last, but now fails): +INSERT INTO document VALUES (33, 22, 1, 'rls_regress_user1', 'okay science fiction') + ON CONFLICT (did) UPDATE SET dauthor = 'rls_regress_user3' RETURNING *; + +SET SESSION AUTHORIZATION rls_regress_user0; +DROP POLICY p1 ON document; + +CREATE POLICY p1 ON document FOR SELECT USING (true); +CREATE POLICY p2 ON document FOR INSERT WITH CHECK (dauthor = current_user); +CREATE POLICY p3 ON document FOR UPDATE + USING (cid = (SELECT cid from category WHERE cname = 'novel')) + WITH CHECK (dauthor = current_user); + +SET SESSION AUTHORIZATION rls_regress_user1; + +-- Again, would fail with unique violation, but with ON CONFLICT fails as row is +-- locked for update (notably, existing/target row is not leaked, which is what +-- failed to satisfy WITH CHECK options - not row proposed for insertion by +-- user): +INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my fourth manga') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; + +-- Again, can't insert new violating tuple, either (unsuccessfully inserted tuple +-- values are reported here, though) +-- +-- Violates actual CHECK OPTION within UPDATE: +INSERT INTO document VALUES (2, 11, 1, 'rls_regress_user2', 'my first novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, dauthor = EXCLUDED.dauthor; + +-- Violates USING qual for UPDATE policy p3, interpreted here as CHECK OPTION. +-- +-- UPDATE path is taken, but UPDATE fails purely because *existing* row to be +-- updated is not a "novel"/cid 11 (row is not leaked, even though we have +-- SELECT privileges sufficient to see the row in this instance): +INSERT INTO document VALUES (33, 11, 1, 'rls_regress_user1', 'Some novel, replaces sci-fi') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle; +-- Fine (we UPDATE): +INSERT INTO document VALUES (2, 11, 1, 'rls_regress_user1', 'my first novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle RETURNING *; +-- Fine (we INSERT, so "cid = 33" isn't evaluated): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33 RETURNING *; +-- Fail (same query, but we UPDATE, so "cid = 33" is evaluated at end of +-- UPDATE): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33 RETURNING *; +-- Fail (we UPDATE, so dauthor assignment is evaluated at end of UPDATE): +INSERT INTO document VALUES (78, 11, 1, 'rls_regress_user1', 'some other novel') + ON CONFLICT (did) UPDATE SET dtitle = EXCLUDED.dtitle, dauthor = 'rls_regress_user2'; +-- Don't fail because INSERT doesn't satisfy WITH CHECK option that originated +-- as a barrier/USING() qual from the UPDATE. Note that the UPDATE path +-- *isn't* taken, and so UPDATE-related policy does not apply: +INSERT INTO document VALUES (88, 33, 1, 'rls_regress_user1', 'technology book, can only insert') + ON CONFLICT (did) UPDATE SET dtitle = upper(EXCLUDED.dtitle) RETURNING *; + +-- -- ROLE/GROUP -- SET SESSION AUTHORIZATION rls_regress_user0; -- 1.9.1