From 9bddf5705327aef24ae6e65ab1699091bf729b6e Mon Sep 17 00:00:00 2001 From: eaydingol <60466783+eaydingol@users.noreply.github.com> Date: Fri, 7 Feb 2025 10:56:08 +0300 Subject: [PATCH 1/6] Add changelog for 12.1.7 (#7889) Add changelog entries for 12.1.7 --------- Co-authored-by: Onur Tirtir (cherry picked from commit bae20578d4523380963961466afa7919eda3cd07) --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 775950ff0..9775de149 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,14 @@ * Fixes a bug that breaks router updates on distributed tables when a reference table is used in the subquery (#7897) +### citus v12.1.7 (Feb 6, 2025) ### + +* Fixes a crash that happens because of unsafe catalog access when re-assigning + the global pid after `application_name` changes (#7791) + +* Prevents crashes when another extension skips executing the + `ClientAuthentication_hook` of Citus. (#7836) + ### citus v13.0.1 (February 4th, 2025) ### * Drops support for PostgreSQL 14 (#7753) From 88904eda9703267cf0aef65389046f9d666045d7 Mon Sep 17 00:00:00 2001 From: naisila Date: Thu, 20 Mar 2025 12:20:59 +0300 Subject: [PATCH 2/6] Update changelog for 13.0.3 (cherry picked from commit bbe0539df22ed2669a23b070798e19eac912ee1d) --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9775de149..451e6e433 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### citus v13.0.3 (March 20th, 2025) ### + +* Fixes a version bump issue in 13.0.2 + ### citus v13.0.2 (March 12th, 2025) ### * Fixes a crash in columnar custom scan that happens when a columnar table is From a7e686c1066154d1e29355dae497fdb76ebc1ce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Thu, 27 Mar 2025 10:39:43 +0100 Subject: [PATCH 3/6] Make sure to prevent INSERT INTO ... SELECT queries involving subfield or sublink (#7912) DESCRIPTION: Makes sure to prevent `INSERT INTO ... SELECT` queries involving subfield or sublink, to avoid crashes The following query was crashing the backend: ``` INSERT INTO field_indirection_test_1 ( int_col, ct1_col.int_1,ct1_col.int_2 ) SELECT 0, 1, 2; -- crash ``` En passant, added more tests with sublink in distributed_types and found another query with wrong behavior: ``` INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; ERROR: could not find a conversion path from type 23 to 17619 -- not the expected ERROR ``` Fixed them by using `strip_implicit_coercions()` on target entry expression before checking for the presence of a subscript or fieldstore, else we fail to find the existing ones and wrongly accept to execute unsafe query. --- .../planner/insert_select_planner.c | 6 ++- .../regress/expected/distributed_types.out | 47 ++++++++++++++++++- src/test/regress/sql/distributed_types.sql | 27 +++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index ce61bd0ae..db5c3d4ff 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -1083,9 +1083,11 @@ ReorderInsertSelectTargetLists(Query *originalQuery, RangeTblEntry *insertRte, AttrNumber originalAttrNo = get_attnum(insertRelationId, oldInsertTargetEntry->resname); + /* we need to explore the underlying expression */ + Node *expr = strip_implicit_coercions((Node *) oldInsertTargetEntry->expr); + /* see transformInsertRow() for the details */ - if (IsA(oldInsertTargetEntry->expr, SubscriptingRef) || - IsA(oldInsertTargetEntry->expr, FieldStore)) + if (IsA(expr, SubscriptingRef) || IsA(expr, FieldStore)) { ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg( diff --git a/src/test/regress/expected/distributed_types.out b/src/test/regress/expected/distributed_types.out index ab16b6d8d..703614d61 100644 --- a/src/test/regress/expected/distributed_types.out +++ b/src/test/regress/expected/distributed_types.out @@ -386,6 +386,12 @@ HINT: Use the column name to insert or update the composite type as a single va INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) VALUES (0, 1); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) SELECT 0, 1, 2; +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. +INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) SELECT 0, 1; +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. CREATE TYPE ct2 as (int_2 int, text_1 text, int_1 int); CREATE TABLE field_indirection_test_2 (int_col int, ct2_col ct2, ct1_col ct1); SELECT create_distributed_table('field_indirection_test_2', 'int_col'); @@ -399,10 +405,17 @@ INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2) +SELECT * FROM (VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5)) qq(int_1, int_col2, text_1, int_2); +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. -- not supported (field indirection in update) UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = ('text2', 10) WHERE int_col=4; ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = (SELECT 'text2', 10) WHERE int_col=4; +ERROR: inserting or modifying composite type fields is not supported +HINT: Use the column name to insert or update the composite type as a single value CREATE TYPE two_ints as (if1 int, if2 int); CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array domain[]); @@ -416,22 +429,36 @@ SELECT create_distributed_table('domain_indirection_test', 'f1'); INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. UPDATE domain_indirection_test SET domain_array[0].if2 = 5; ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); +ERROR: inserting or modifying composite type fields is not supported +HINT: Use the column name to insert or update the composite type as a single value -- below are supported as we don't do any field indirection INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) VALUES ('(1, "text1", 2)', 3, '(4, 5)'), ('(6, "text2", 7)', 8, '(9, 10)'); +INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) +SELECT * FROM (VALUES ('(1, "text1", 2)'::ct2, 3, '(4, 5)'::ct1), ('(6, "text2", 7)'::ct2, 8, '(9, 10)'::ct1)) qq(ct2_col, int_col, ct1_col); UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = ('(10, "text10", 20)', '(40, 50)') WHERE int_col=8; +UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = (SELECT '(10, "text10", 20)'::ct2, '(40, 50)'::ct1) WHERE int_col=8; SELECT * FROM field_indirection_test_2 ORDER BY 1,2,3; int_col | ct2_col | ct1_col --------------------------------------------------------------------- + 3 | (1," text1",2) | (4,5) 3 | (1," text1",2) | (4,5) 8 | (10," text10",20) | (40,50) -(2 rows) + 8 | (10," text10",20) | (40,50) +(4 rows) -- test different ddl propagation modes SET citus.create_object_propagation TO deferred; @@ -583,3 +610,21 @@ SET client_min_messages TO error; -- suppress cascading objects dropping DROP SCHEMA type_tests CASCADE; DROP SCHEMA type_tests2 CASCADE; DROP USER typeuser; +RESET client_min_messages; +CREATE SCHEMA issue_4704; +SET search_path TO issue_4704; +-- https://github.com/citusdata/citus/issues/4704 +-- known to have crash citus before +CREATE TYPE comptype as (r float8, i float8); +CREATE DOMAIN dcomptypea as comptype[]; +CREATE TABLE dcomptable (d1 dcomptypea unique); +SELECT create_distributed_table('dcomptable', 'd1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +insert into dcomptable values (array[row(1,2)]::dcomptypea); +SET client_min_messages TO error; -- suppress cascading objects dropping +DROP TABLE dcomptable; +DROP SCHEMA issue_4704 CASCADE; diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index d8a4b6247..205faf9c7 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -245,6 +245,8 @@ SELECT create_distributed_table('field_indirection_test_1', 'int_col'); -- not supported (field indirection in single row insert) INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) VALUES (0, 1, 2); INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) VALUES (0, 1); +INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) SELECT 0, 1, 2; +INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) SELECT 0, 1; CREATE TYPE ct2 as (int_2 int, text_1 text, int_1 int); CREATE TABLE field_indirection_test_2 (int_col int, ct2_col ct2, ct1_col ct1); @@ -253,9 +255,12 @@ SELECT create_distributed_table('field_indirection_test_2', 'int_col'); -- not supported (field indirection in multi row insert) INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2) VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5); +INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2) +SELECT * FROM (VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5)) qq(int_1, int_col2, text_1, int_2); -- not supported (field indirection in update) UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = ('text2', 10) WHERE int_col=4; +UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = (SELECT 'text2', 10) WHERE int_col=4; CREATE TYPE two_ints as (if1 int, if2 int); CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); @@ -264,13 +269,19 @@ SELECT create_distributed_table('domain_indirection_test', 'f1'); -- not supported (field indirection to underlying composite type) INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); +INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); +INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; UPDATE domain_indirection_test SET domain_array[0].if2 = 5; +UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); -- below are supported as we don't do any field indirection INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) VALUES ('(1, "text1", 2)', 3, '(4, 5)'), ('(6, "text2", 7)', 8, '(9, 10)'); +INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) +SELECT * FROM (VALUES ('(1, "text1", 2)'::ct2, 3, '(4, 5)'::ct1), ('(6, "text2", 7)'::ct2, 8, '(9, 10)'::ct1)) qq(ct2_col, int_col, ct1_col); UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = ('(10, "text10", 20)', '(40, 50)') WHERE int_col=8; +UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = (SELECT '(10, "text10", 20)'::ct2, '(40, 50)'::ct1) WHERE int_col=8; SELECT * FROM field_indirection_test_2 ORDER BY 1,2,3; @@ -355,3 +366,19 @@ SET client_min_messages TO error; -- suppress cascading objects dropping DROP SCHEMA type_tests CASCADE; DROP SCHEMA type_tests2 CASCADE; DROP USER typeuser; + +RESET client_min_messages; +CREATE SCHEMA issue_4704; +SET search_path TO issue_4704; + +-- https://github.com/citusdata/citus/issues/4704 +-- known to have crash citus before +CREATE TYPE comptype as (r float8, i float8); +CREATE DOMAIN dcomptypea as comptype[]; +CREATE TABLE dcomptable (d1 dcomptypea unique); +SELECT create_distributed_table('dcomptable', 'd1'); +insert into dcomptable values (array[row(1,2)]::dcomptypea); + +SET client_min_messages TO error; -- suppress cascading objects dropping +DROP TABLE dcomptable; +DROP SCHEMA issue_4704 CASCADE; From 1dc60e38bb304c9d28521d8554e6d41ca425a5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Fri, 4 Apr 2025 10:54:16 +0200 Subject: [PATCH 4/6] Propagates GRANT/REVOKE rights on table columns (#7918) This commit adds support for GRANT/REVOKE on table columns. It extends propagated DDL according to this logic: https://github.com/citusdata/citus/tree/main/src/backend/distributed#ddl * Unchanged pre-existing behavior related to splitting ddl per relation during propagation. * Changed the way ACL are checked in some cases (see `EnsureTablePermissions()` and associated commits) * Rewrite `pg_get_table_grants` to include column grants as well * Add missing `pfree()` in `pg_get_table_grants()` Fixes https://github.com/citusdata/citus/issues/7287 Also check a box in https://github.com/citusdata/citus/issues/4812 --- src/backend/distributed/commands/grant.c | 45 +- .../distributed/deparser/citus_ruleutils.c | 173 ++-- .../distributed/metadata/metadata_utility.c | 24 +- .../distributed/metadata/node_metadata.c | 2 +- .../distributed/operations/stage_protocol.c | 2 +- src/backend/distributed/utils/resource_lock.c | 8 +- src/include/distributed/metadata_utility.h | 2 +- .../expected/grant_on_table_propagation.out | 930 ++++++++++++++++++ .../expected/grant_on_table_propagation_0.out | 930 ++++++++++++++++++ .../multi_multiuser_master_protocol.out | 3 +- .../multi_multiuser_master_protocol_0.out | 3 +- src/test/regress/multi_1_schedule | 1 + .../sql/grant_on_table_propagation.sql | 444 +++++++++ .../sql/multi_multiuser_master_protocol.sql | 2 +- 14 files changed, 2488 insertions(+), 81 deletions(-) create mode 100644 src/test/regress/expected/grant_on_table_propagation.out create mode 100644 src/test/regress/expected/grant_on_table_propagation_0.out create mode 100644 src/test/regress/sql/grant_on_table_propagation.sql diff --git a/src/backend/distributed/commands/grant.c b/src/backend/distributed/commands/grant.c index c4278cee1..3cc18c155 100644 --- a/src/backend/distributed/commands/grant.c +++ b/src/backend/distributed/commands/grant.c @@ -17,6 +17,7 @@ #include "distributed/citus_ruleutils.h" #include "distributed/commands.h" #include "distributed/commands/utility_hook.h" +#include "distributed/deparser.h" #include "distributed/metadata/distobject.h" #include "distributed/metadata_cache.h" #include "distributed/version_compat.h" @@ -32,7 +33,6 @@ static List * CollectGrantTableIdList(GrantStmt *grantStmt); * needed during the worker node portion of DDL execution before returning the * DDLJobs in a List. If no distributed table is involved, this returns NIL. * - * NB: So far column level privileges are not supported. */ List * PreprocessGrantStmt(Node *node, const char *queryString, @@ -70,9 +70,12 @@ PreprocessGrantStmt(Node *node, const char *queryString, return NIL; } + EnsureCoordinator(); + /* deparse the privileges */ if (grantStmt->privileges == NIL) { + /* this is used for table level only */ appendStringInfo(&privsString, "ALL"); } else @@ -88,18 +91,44 @@ PreprocessGrantStmt(Node *node, const char *queryString, { appendStringInfoString(&privsString, ", "); } + + if (priv->priv_name) + { + appendStringInfo(&privsString, "%s", priv->priv_name); + } + /* + * ALL can only be set alone. + * And ALL is not added as a keyword in priv_name by parser, but + * because there are column(s) defined, a grantStmt->privileges is + * defined. So we need to handle this special case here (see if + * condition above). + */ + else if (isFirst) + { + /* this is used for column level only */ + appendStringInfo(&privsString, "ALL"); + } + /* + * Instead of relying only on the syntax check done by Postgres and + * adding an assert here, add a default ERROR if ALL is not first + * and no priv_name is defined. + */ + else + { + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("Cannot parse GRANT/REVOKE privileges"))); + } + isFirst = false; if (priv->cols != NIL) { - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("grant/revoke on column list is currently " - "unsupported"))); + StringInfoData colsString; + initStringInfo(&colsString); + + AppendColumnNameList(&colsString, priv->cols); + appendStringInfo(&privsString, "%s", colsString.data); } - - Assert(priv->priv_name != NULL); - - appendStringInfo(&privsString, "%s", priv->priv_name); } } diff --git a/src/backend/distributed/deparser/citus_ruleutils.c b/src/backend/distributed/deparser/citus_ruleutils.c index bdba2bf6e..d590495a6 100644 --- a/src/backend/distributed/deparser/citus_ruleutils.c +++ b/src/backend/distributed/deparser/citus_ruleutils.c @@ -83,6 +83,8 @@ static void AppendStorageParametersToString(StringInfo stringBuffer, static const char * convert_aclright_to_string(int aclright); static void simple_quote_literal(StringInfo buf, const char *val); static void AddVacuumParams(ReindexStmt *reindexStmt, StringInfo buffer); +static void process_acl_items(Acl *acl, const char *relationName, + const char *attributeName, List **defs); /* @@ -1110,9 +1112,8 @@ pg_get_indexclusterdef_string(Oid indexRelationId) /* * pg_get_table_grants returns a list of sql statements which recreate the - * permissions for a specific table. + * permissions for a specific table, including attributes privileges. * - * This function is modeled after aclexplode(), don't change too heavily. */ List * pg_get_table_grants(Oid relationId) @@ -1136,6 +1137,8 @@ pg_get_table_grants(Oid relationId) errmsg("relation with OID %u does not exist", relationId))); } + Form_pg_class classForm = (Form_pg_class) GETSTRUCT(classTuple); + AttrNumber nattrs = classForm->relnatts; Datum aclDatum = SysCacheGetAttr(RELOID, classTuple, Anum_pg_class_relacl, &isNull); @@ -1163,80 +1166,132 @@ pg_get_table_grants(Oid relationId) /* iterate through the acl datastructure, emit GRANTs */ Acl *acl = DatumGetAclP(aclDatum); - AclItem *aidat = ACL_DAT(acl); - int offtype = -1; - int i = 0; - while (i < ACL_NUM(acl)) + process_acl_items(acl, relationName, NULL, &defs); + + /* if we have a detoasted copy, free it */ + if ((Pointer) acl != DatumGetPointer(aclDatum)) + pfree(acl); + } + + resetStringInfo(&buffer); + + /* lookup all attribute level grants */ + for (AttrNumber attNum = 1; attNum <= nattrs; attNum++) + { + HeapTuple attTuple = SearchSysCache2(ATTNUM, ObjectIdGetDatum(relationId), + Int16GetDatum(attNum)); + if (!HeapTupleIsValid(attTuple)) { - AclItem *aidata = NULL; - AclMode priv_bit = 0; + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("attribute with OID %u does not exist", + attNum))); + } - offtype++; + Form_pg_attribute thisAttribute = (Form_pg_attribute) GETSTRUCT(attTuple); - if (offtype == N_ACL_RIGHTS) + /* ignore dropped columns */ + if (thisAttribute->attisdropped) + { + ReleaseSysCache(attTuple); + continue; + } + + Datum aclAttDatum = SysCacheGetAttr(ATTNUM, attTuple, Anum_pg_attribute_attacl, + &isNull); + if (!isNull) + { + /* iterate through the acl datastructure, emit GRANTs */ + Acl *acl = DatumGetAclP(aclAttDatum); + + process_acl_items(acl, relationName, NameStr(thisAttribute->attname), &defs); + + /* if we have a detoasted copy, free it */ + if ((Pointer) acl != DatumGetPointer(aclAttDatum)) + pfree(acl); + } + ReleaseSysCache(attTuple); + } + + relation_close(relation, NoLock); + return defs; +} + + +/* + * Helper function to process ACL items. + * If attributeName is NULL, the function emits table-level GRANT commands; + * otherwise it emits column-level GRANT commands. + * This function was modeled after aclexplode(), previously in pg_get_table_grants(). + */ +static void +process_acl_items(Acl *acl, const char *relationName, const char *attributeName, + List **defs) +{ + AclItem *aidat = ACL_DAT(acl); + int i = 0; + int offtype = -1; + StringInfoData buffer; + + initStringInfo(&buffer); + + while (i < ACL_NUM(acl)) + { + offtype++; + if (offtype == N_ACL_RIGHTS) + { + offtype = 0; + i++; + if (i >= ACL_NUM(acl)) /* done */ { - offtype = 0; - i++; - if (i >= ACL_NUM(acl)) /* done */ - { - break; - } + break; + } + } + + AclItem *aidata = &aidat[i]; + AclMode priv_bit = 1 << offtype; + + if (ACLITEM_GET_PRIVS(*aidata) & priv_bit) + { + const char *roleName = NULL; + const char *withGrant = ""; + + if (aidata->ai_grantee != 0) + { + roleName = quote_identifier(GetUserNameFromId(aidata->ai_grantee, false)); + } + else + { + roleName = "PUBLIC"; } - aidata = &aidat[i]; - priv_bit = 1 << offtype; - - if (ACLITEM_GET_PRIVS(*aidata) & priv_bit) + if ((ACLITEM_GET_GOPTIONS(*aidata) & priv_bit) != 0) { - const char *roleName = NULL; - const char *withGrant = ""; - - if (aidata->ai_grantee != 0) - { - - HeapTuple htup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(aidata->ai_grantee)); - if (HeapTupleIsValid(htup)) - { - Form_pg_authid authForm = ((Form_pg_authid) GETSTRUCT(htup)); - - roleName = quote_identifier(NameStr(authForm->rolname)); - - ReleaseSysCache(htup); - } - else - { - elog(ERROR, "cache lookup failed for role %u", aidata->ai_grantee); - } - } - else - { - roleName = "PUBLIC"; - } - - if ((ACLITEM_GET_GOPTIONS(*aidata) & priv_bit) != 0) - { - withGrant = " WITH GRANT OPTION"; - } + withGrant = " WITH GRANT OPTION"; + } + if (attributeName) + { + appendStringInfo(&buffer, "GRANT %s(%s) ON %s TO %s%s", + convert_aclright_to_string(priv_bit), + quote_identifier(attributeName), + relationName, + roleName, + withGrant); + } + else + { appendStringInfo(&buffer, "GRANT %s ON %s TO %s%s", convert_aclright_to_string(priv_bit), relationName, roleName, withGrant); - - defs = lappend(defs, pstrdup(buffer.data)); - - resetStringInfo(&buffer); } + *defs = lappend(*defs, pstrdup(buffer.data)); + resetStringInfo(&buffer); } } - - resetStringInfo(&buffer); - - relation_close(relation, NoLock); - return defs; - /* *INDENT-ON* */ } diff --git a/src/backend/distributed/metadata/metadata_utility.c b/src/backend/distributed/metadata/metadata_utility.c index fad263abd..6d8ed0808 100644 --- a/src/backend/distributed/metadata/metadata_utility.c +++ b/src/backend/distributed/metadata/metadata_utility.c @@ -2428,14 +2428,32 @@ UpdateNoneDistTableMetadata(Oid relationId, char replicationModel, uint32 coloca /* - * Check that the current user has `mode` permissions on relationId, error out - * if not. Superusers always have such permissions. + * Check that the current user has `mode` permissions on relationId. + * If not, also check relationId's attributes with `mask`, error out + * privileges are not defined. + * ACL mask is used because we assume that user has enough privilege + * to distribute a table when either ACL_INSERT on the TABLE or + * ACL_INSERT on ALL attributes. + * In other situations, having a single attribute privilege is enough. + * Superusers always have such permissions. */ void -EnsureTablePermissions(Oid relationId, AclMode mode) +EnsureTablePermissions(Oid relationId, AclMode mode, AclMaskHow mask) { AclResult aclresult = pg_class_aclcheck(relationId, GetUserId(), mode); + if (aclresult == ACLCHECK_OK) + { + return; + } + + /* + * Also check the attributes: for example "GRANT ALL(a)" has no table level + * right but user is still allowed to lock table as needed. PostgreSQL will + * still enforce ACL later so it's safe. + */ + aclresult = pg_attribute_aclcheck_all(relationId, GetUserId(), mode, mask); + if (aclresult != ACLCHECK_OK) { aclcheck_error(aclresult, OBJECT_TABLE, get_rel_name(relationId)); diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index d031f4d2c..c2a2abd60 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -1522,7 +1522,7 @@ get_shard_id_for_distribution_column(PG_FUNCTION_ARGS) } Oid relationId = PG_GETARG_OID(0); - EnsureTablePermissions(relationId, ACL_SELECT); + EnsureTablePermissions(relationId, ACL_SELECT, ACLMASK_ANY); if (!IsCitusTable(relationId)) { diff --git a/src/backend/distributed/operations/stage_protocol.c b/src/backend/distributed/operations/stage_protocol.c index 9881d8775..976f1549f 100644 --- a/src/backend/distributed/operations/stage_protocol.c +++ b/src/backend/distributed/operations/stage_protocol.c @@ -108,7 +108,7 @@ master_create_empty_shard(PG_FUNCTION_ARGS) Oid relationId = ResolveRelationId(relationNameText, false); - EnsureTablePermissions(relationId, ACL_INSERT); + EnsureTablePermissions(relationId, ACL_INSERT, ACLMASK_ALL); CheckDistributedTable(relationId); /* diff --git a/src/backend/distributed/utils/resource_lock.c b/src/backend/distributed/utils/resource_lock.c index 3f50b682e..012dc1079 100644 --- a/src/backend/distributed/utils/resource_lock.c +++ b/src/backend/distributed/utils/resource_lock.c @@ -222,10 +222,12 @@ lock_shard_resources(PG_FUNCTION_ARGS) * on the executor. However, for INSERTs, the user might have only * INSERTs granted, so add a special case for it. */ - AclMode aclMask = ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE; + AclMode aclMode = ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE; + AclMaskHow aclMaskHow = ACLMASK_ANY; + if (lockMode == RowExclusiveLock) { - aclMask |= ACL_INSERT; + aclMode |= ACL_INSERT; } for (int shardIdIndex = 0; shardIdIndex < shardIdCount; shardIdIndex++) @@ -254,7 +256,7 @@ lock_shard_resources(PG_FUNCTION_ARGS) if (!SkipAdvisoryLockPermissionChecks) { - EnsureTablePermissions(relationId, aclMask); + EnsureTablePermissions(relationId, aclMode, aclMaskHow); } LockShardResource(shardId, lockMode); diff --git a/src/include/distributed/metadata_utility.h b/src/include/distributed/metadata_utility.h index 737e1283b..38c13eb51 100644 --- a/src/include/distributed/metadata_utility.h +++ b/src/include/distributed/metadata_utility.h @@ -400,7 +400,7 @@ extern bool ShouldPropagateAnyObject(List *addresses); /* Remaining metadata utility functions */ extern Oid TableOwnerOid(Oid relationId); extern char * TableOwner(Oid relationId); -extern void EnsureTablePermissions(Oid relationId, AclMode mode); +extern void EnsureTablePermissions(Oid relationId, AclMode mode, AclMaskHow mask); extern void EnsureTableOwner(Oid relationId); extern void EnsureHashDistributedTable(Oid relationId); extern void EnsureHashOrSingleShardDistributedTable(Oid relationId); diff --git a/src/test/regress/expected/grant_on_table_propagation.out b/src/test/regress/expected/grant_on_table_propagation.out new file mode 100644 index 000000000..55a55fac3 --- /dev/null +++ b/src/test/regress/expected/grant_on_table_propagation.out @@ -0,0 +1,930 @@ +-- PostgreSQL 17 introduces MAINTAIN privilege, this change the default output +-- of some test queries. +-- +-- Alternative "grant_on_table_propagation_0.out" and this comment can be +-- removed when support for PostgreSQL versions < 17 is dropped. +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 17 AS server_version_ge_17; + server_version_ge_17 +--------------------------------------------------------------------- + t +(1 row) + +-- +-- GRANT_ON_TABLE_PROPAGATION +-- +SET citus.shard_replication_factor TO 1; +CREATE SCHEMA grant_on_table; +SET search_path TO grant_on_table; +-- create some simple tables: 1 local on all nodes and 2 managed by citus +-- null_privs ACL must not be updated in anyway. +CREATE TABLE dist_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL/mixed + , null_privs text + ); +SELECT create_distributed_table('dist_table', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE ref_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL + , null_privs text + ); +SELECT create_reference_table('ref_table'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +SELECT result FROM run_command_on_all_nodes('CREATE TABLE grant_on_table.local_table (id int GENERATED BY DEFAULT AS IDENTITY primary key , test_a int, test_r text, test_w text, test_mix int, null_privs text)'); + result +--------------------------------------------------------------------- + CREATE TABLE + CREATE TABLE + CREATE TABLE +(3 rows) + +-- queries used to check privileges: +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''dist_table'', ''ref_table'', ''local_table'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.dist_table''::regclass, ''grant_on_table.ref_table''::regclass, ''grant_on_table.local_table''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +-- create some users +CREATE USER grant_user_0; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_0; +CREATE USER grant_user_1; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_1; +-- this one should not be granted anything: +CREATE USER nogrant_user; +GRANT USAGE ON SCHEMA grant_on_table TO nogrant_user; +-- +-- tests related to columns ACL +-- +-- +-- when executing a table level grant, "postgres" is add/listed in pg_class.relacl +-- but nothing happens with revoke, as a result pg_class.relacl is not stable. +-- in order to have immutable cleanup results in those tests, init the +-- "postgres" special case +GRANT SELECT ON ref_table TO grant_user_0; +REVOKE SELECT ON ref_table FROM grant_user_0; +-- +-- check we are able to propagate a single attribute privilege +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (test_r) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT test_r FROM ref_table; + test_r +--------------------------------------------------------------------- +(0 rows) + +-- not granted: +SELECT test_a FROM ref_table; +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (test_r) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we are able to propagate a privilege to multiple attributes, users and tables at once +-- we use only INSERT/UPDATE +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (test_a) VALUES (1); +ERROR: permission denied for table ref_table +RESET ROLE; +-- we would prefer not have to grant INSERT (id) as expected in the standard but +-- Citus rewrite queries with such attributes, which prevent standard to be applied. +GRANT INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (test_a) VALUES (1); +-- granted: +UPDATE ref_table SET test_w = 2, test_mix = 2; +-- not granted: +INSERT INTO ref_table (test_w) VALUES (1); +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- granted: +INSERT INTO dist_table (test_a) VALUES (1); +-- granted: +UPDATE dist_table SET test_w = 3, test_mix = 3; +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") +(24 rows) + +-- cleanup +REVOKE INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table FROM grant_user_0, grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we are able to propagate a table privilege associated with an attribute level privilege +-- we use only SELECT/DELETE +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (test_r, test_mix), DELETE ON ref_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT test_r, test_mix FROM ref_table; + test_r | test_mix +--------------------------------------------------------------------- + | 2 +(1 row) + +-- granted: +DELETE FROM ref_table; +-- not granted: +SELECT test_a FROM ref_table; +ERROR: permission denied for table ref_table +-- not granted: +UPDATE ref_table SET null_privs = 3; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,"{postgres=arwdDxtm/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") + 57637 | (ref_table,"{postgres=arwdDxtm/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") + 57638 | (ref_table,"{postgres=arwdDxtm/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57636 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57637 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57638 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") +(6 rows) + +-- cleanup +REVOKE SELECT (test_r, test_mix), DELETE ON ref_table FROM grant_user_0, grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we also propagate system columns +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT ctid, xmin FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (ctid, xmin) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT ctid, xmin FROM ref_table; + ctid | xmin +--------------------------------------------------------------------- +(0 rows) + +-- not granted: +SELECT ctid, test_a FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) +(6 rows) + +-- cleanup +REVOKE SELECT (ctid, xmin) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we correctly propagate ALL, which has a few special cases +-- we use only ALL/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (id) VALUES (13); +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted yet: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT ALL (id) ON ref_table TO grant_user_0; +GRANT ALL (id, test_mix) ON ref_table TO grant_user_1; +-- ALL cannot be mixed with other privs +-- should error: +GRANT SELECT (null_privs), ALL (null_privs) ON ref_table TO nogrant_user; +ERROR: syntax error at or near "ALL" +-- should error: +GRANT ALL (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +ERROR: syntax error at or near "," +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (id) VALUES (13); +SET ROLE grant_user_1; +-- granted: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +-- not granted: +INSERT INTO ref_table (null_privs) VALUES (3); +ERROR: permission denied for table ref_table +-- granted: +SELECT id, test_mix FROM ref_table; + id | test_mix +--------------------------------------------------------------------- + 13 | + 9 | 3 +(2 rows) + +-- not granted: +SELECT null_privs FROM ref_table; +ERROR: permission denied for table ref_table +-- not granted: +DELETE FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57637 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57638 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) +(6 rows) + +-- cleanup +REVOKE ALL (id) ON ref_table FROM grant_user_0; +REVOKE ALL (id, test_mix) ON ref_table FROM grant_user_1; +TRUNCATE ref_table; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we correctly propagate when mixed with local table, but only on the +-- current local table, not others +-- we use only INSERT/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- NOTE: +-- test special case: ALL TABLES IN SCHEMA is not supposed to be correct +-- but is accepted by PostgreSQL - non documented feature +GRANT SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table TO grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +GRANT INSERT (test_mix) ON local_table TO grant_user_0; +-- check we can propagate also when mixed with distributed table: +GRANT SELECT (test_r, test_mix) ON local_table, dist_table TO grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; + id +--------------------------------------------------------------------- + 1 +(1 row) + +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM dist_table +UNION ALL SELECT test_r, test_mix FROM local_table; + test_r | test_mix +--------------------------------------------------------------------- + | 3 +(1 row) + +RESET ROLE; +-- check on coordinator and workers +-- we pay special attention to local_table privileges here: +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57637 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57638 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57637 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57638 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.local_table,id,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.local_table,test_mix,"{grant_user_0=a/postgres,grant_user_1=r/postgres}") + 57636 | (grant_on_table.local_table,test_r,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) +(15 rows) + +-- cleanup +REVOKE SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table FROM grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +REVOKE INSERT (test_mix) ON local_table FROM grant_user_0; +-- check we can propagate also when mixed with distributed table: +REVOKE SELECT (test_r, test_mix) ON local_table, dist_table FROM grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check TRUNCATE is not propagated (inccorect grammar) +-- also ensure no privs are propagated at all with "partially" incorrect grammar +-- we use only TRUNCATE/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT TRUNCATE (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +ERROR: invalid privilege type TRUNCATE for column +SET ROLE grant_user_0; +-- still not granted: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +-- still not granted: +TRUNCATE ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- no cleanup required +-- +-- check we do not propage from a worker +-- we use only SELECT +-- +\c - - - :worker_1_port +SET search_path TO grant_on_table; +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0; +ERROR: operation is not allowed on this node +HINT: Connect to the coordinator and run it again. +\c - - - :master_port +SET search_path TO grant_on_table; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- no cleanup required +-- +-- check we do propagate WITH GRANT OPTION +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- grant with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0 WITH GRANT OPTION; +SET ROLE grant_user_0; +-- grant using a role with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_1; +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM ref_table; + test_r | test_mix +--------------------------------------------------------------------- +(0 rows) + +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57636 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57637 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57638 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") +(6 rows) + +-- cleanup and further checks: +SET ROLE grant_user_0; +-- revoke as grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_1; +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) +(6 rows) + +-- revoke only grant options from grant_user_0: +REVOKE GRANT OPTION FOR SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) +(6 rows) + +-- revoke select from grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxtm/postgres}) + 57637 | (ref_table,{postgres=arwdDxtm/postgres}) + 57638 | (ref_table,{postgres=arwdDxtm/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE dist_table, ref_table; +SELECT result FROM run_command_on_all_nodes('DROP TABLE grant_on_table.local_table'); + result +--------------------------------------------------------------------- + DROP TABLE + DROP TABLE + DROP TABLE +(3 rows) + +RESET client_min_messages; +-- +-- check we propagate privileges when GRANTed before distributed and when adding a node +-- we use only SELECT +-- +-- test propagation on columns when distributing the table after GRANT has been executed +CREATE TABLE grant_table_propagated (id int primary key); +GRANT SELECT (id) ON grant_table_propagated TO grant_user_0; +SELECT create_distributed_table('grant_table_propagated', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SET ROLE grant_user_0; +-- granted: +SELECT id FROM grant_table_propagated; + id +--------------------------------------------------------------------- +(0 rows) + +RESET ROLE; +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated,) + 57637 | (grant_table_propagated,) + 57638 | (grant_table_propagated,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated FROM grant_user_0; +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated,) + 57637 | (grant_table_propagated,) + 57638 | (grant_table_propagated,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated; +RESET client_min_messages; +-- similar test but adding a node after the fact +-- remove one of the worker nodes: +SELECT citus_remove_node('localhost', :worker_2_port); + citus_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE grant_table_propagated_after (id int primary key); +SET citus.shard_replication_factor TO 1; +SELECT create_distributed_table('grant_table_propagated_after', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SET citus.shard_replication_factor TO 2; +GRANT SELECT (id) ON grant_table_propagated_after TO grant_user_0; +-- add back the worker node +SELECT FROM citus_add_node('localhost', :worker_2_port); +-- +(1 row) + +-- granted: +SELECT id FROM grant_table_propagated_after; + id +--------------------------------------------------------------------- +(0 rows) + +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated_after'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated_after''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated_after,) + 57637 | (grant_table_propagated_after,) + 57638 | (grant_table_propagated_after,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated_after,) + 57637 | (grant_table_propagated_after,) + 57638 | (grant_table_propagated_after,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated_after; +RESET client_min_messages; +-- global cleanup +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP SCHEMA grant_on_table CASCADE; +DROP ROLE grant_user_0, grant_user_1; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/expected/grant_on_table_propagation_0.out b/src/test/regress/expected/grant_on_table_propagation_0.out new file mode 100644 index 000000000..adebacd19 --- /dev/null +++ b/src/test/regress/expected/grant_on_table_propagation_0.out @@ -0,0 +1,930 @@ +-- PostgreSQL 17 introduces MAINTAIN privilege, this change the default output +-- of some test queries. +-- +-- Alternative "grant_on_table_propagation_0.out" and this comment can be +-- removed when support for PostgreSQL versions < 17 is dropped. +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 17 AS server_version_ge_17; + server_version_ge_17 +--------------------------------------------------------------------- + f +(1 row) + +-- +-- GRANT_ON_TABLE_PROPAGATION +-- +SET citus.shard_replication_factor TO 1; +CREATE SCHEMA grant_on_table; +SET search_path TO grant_on_table; +-- create some simple tables: 1 local on all nodes and 2 managed by citus +-- null_privs ACL must not be updated in anyway. +CREATE TABLE dist_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL/mixed + , null_privs text + ); +SELECT create_distributed_table('dist_table', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE ref_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL + , null_privs text + ); +SELECT create_reference_table('ref_table'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +SELECT result FROM run_command_on_all_nodes('CREATE TABLE grant_on_table.local_table (id int GENERATED BY DEFAULT AS IDENTITY primary key , test_a int, test_r text, test_w text, test_mix int, null_privs text)'); + result +--------------------------------------------------------------------- + CREATE TABLE + CREATE TABLE + CREATE TABLE +(3 rows) + +-- queries used to check privileges: +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''dist_table'', ''ref_table'', ''local_table'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.dist_table''::regclass, ''grant_on_table.ref_table''::regclass, ''grant_on_table.local_table''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +-- create some users +CREATE USER grant_user_0; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_0; +CREATE USER grant_user_1; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_1; +-- this one should not be granted anything: +CREATE USER nogrant_user; +GRANT USAGE ON SCHEMA grant_on_table TO nogrant_user; +-- +-- tests related to columns ACL +-- +-- +-- when executing a table level grant, "postgres" is add/listed in pg_class.relacl +-- but nothing happens with revoke, as a result pg_class.relacl is not stable. +-- in order to have immutable cleanup results in those tests, init the +-- "postgres" special case +GRANT SELECT ON ref_table TO grant_user_0; +REVOKE SELECT ON ref_table FROM grant_user_0; +-- +-- check we are able to propagate a single attribute privilege +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (test_r) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT test_r FROM ref_table; + test_r +--------------------------------------------------------------------- +(0 rows) + +-- not granted: +SELECT test_a FROM ref_table; +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (test_r) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we are able to propagate a privilege to multiple attributes, users and tables at once +-- we use only INSERT/UPDATE +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (test_a) VALUES (1); +ERROR: permission denied for table ref_table +RESET ROLE; +-- we would prefer not have to grant INSERT (id) as expected in the standard but +-- Citus rewrite queries with such attributes, which prevent standard to be applied. +GRANT INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (test_a) VALUES (1); +-- granted: +UPDATE ref_table SET test_w = 2, test_mix = 2; +-- not granted: +INSERT INTO ref_table (test_w) VALUES (1); +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- granted: +INSERT INTO dist_table (test_a) VALUES (1); +-- granted: +UPDATE dist_table SET test_w = 3, test_mix = 3; +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.dist_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.dist_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.dist_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.dist_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.ref_table,id,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57637 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57638 | (grant_on_table.ref_table,test_a,"{grant_user_0=a/postgres,grant_user_1=a/postgres}") + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57636 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57637 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") + 57638 | (grant_on_table.ref_table,test_w,"{grant_user_0=w/postgres,grant_user_1=w/postgres}") +(24 rows) + +-- cleanup +REVOKE INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table FROM grant_user_0, grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we are able to propagate a table privilege associated with an attribute level privilege +-- we use only SELECT/DELETE +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (test_r, test_mix), DELETE ON ref_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT test_r, test_mix FROM ref_table; + test_r | test_mix +--------------------------------------------------------------------- + | 2 +(1 row) + +-- granted: +DELETE FROM ref_table; +-- not granted: +SELECT test_a FROM ref_table; +ERROR: permission denied for table ref_table +-- not granted: +UPDATE ref_table SET null_privs = 3; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,"{postgres=arwdDxt/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") + 57637 | (ref_table,"{postgres=arwdDxt/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") + 57638 | (ref_table,"{postgres=arwdDxt/postgres,grant_user_0=d/postgres,grant_user_1=d/postgres}") +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57636 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57637 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") + 57638 | (grant_on_table.ref_table,test_r,"{grant_user_0=r/postgres,grant_user_1=r/postgres}") +(6 rows) + +-- cleanup +REVOKE SELECT (test_r, test_mix), DELETE ON ref_table FROM grant_user_0, grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we also propagate system columns +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT ctid, xmin FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT SELECT (ctid, xmin) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT ctid, xmin FROM ref_table; + ctid | xmin +--------------------------------------------------------------------- +(0 rows) + +-- not granted: +SELECT ctid, test_a FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,ctid,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,xmin,{grant_user_0=r/postgres}) +(6 rows) + +-- cleanup +REVOKE SELECT (ctid, xmin) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we correctly propagate ALL, which has a few special cases +-- we use only ALL/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (id) VALUES (13); +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted yet: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT ALL (id) ON ref_table TO grant_user_0; +GRANT ALL (id, test_mix) ON ref_table TO grant_user_1; +-- ALL cannot be mixed with other privs +-- should error: +GRANT SELECT (null_privs), ALL (null_privs) ON ref_table TO nogrant_user; +ERROR: syntax error at or near "ALL" +-- should error: +GRANT ALL (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +ERROR: syntax error at or near "," +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (id) VALUES (13); +SET ROLE grant_user_1; +-- granted: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +-- not granted: +INSERT INTO ref_table (null_privs) VALUES (3); +ERROR: permission denied for table ref_table +-- granted: +SELECT id, test_mix FROM ref_table; + id | test_mix +--------------------------------------------------------------------- + 13 | + 9 | 3 +(2 rows) + +-- not granted: +SELECT null_privs FROM ref_table; +ERROR: permission denied for table ref_table +-- not granted: +DELETE FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57637 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57638 | (grant_on_table.ref_table,id,"{grant_user_0=arwx/postgres,grant_user_1=arwx/postgres}") + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_1=arwx/postgres}) +(6 rows) + +-- cleanup +REVOKE ALL (id) ON ref_table FROM grant_user_0; +REVOKE ALL (id, test_mix) ON ref_table FROM grant_user_1; +TRUNCATE ref_table; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check we correctly propagate when mixed with local table, but only on the +-- current local table, not others +-- we use only INSERT/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- NOTE: +-- test special case: ALL TABLES IN SCHEMA is not supposed to be correct +-- but is accepted by PostgreSQL - non documented feature +GRANT SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table TO grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +GRANT INSERT (test_mix) ON local_table TO grant_user_0; +-- check we can propagate also when mixed with distributed table: +GRANT SELECT (test_r, test_mix) ON local_table, dist_table TO grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; + id +--------------------------------------------------------------------- + 1 +(1 row) + +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM dist_table +UNION ALL SELECT test_r, test_mix FROM local_table; + test_r | test_mix +--------------------------------------------------------------------- + | 3 +(1 row) + +RESET ROLE; +-- check on coordinator and workers +-- we pay special attention to local_table privileges here: +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.dist_table,id,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57637 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57638 | (grant_on_table.dist_table,test_mix,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57637 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57638 | (grant_on_table.dist_table,test_r,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.local_table,id,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.local_table,test_mix,"{grant_user_0=a/postgres,grant_user_1=r/postgres}") + 57636 | (grant_on_table.local_table,test_r,{grant_user_1=r/postgres}) + 57636 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,id,{grant_user_0=r/postgres}) +(15 rows) + +-- cleanup +REVOKE SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table FROM grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +REVOKE INSERT (test_mix) ON local_table FROM grant_user_0; +-- check we can propagate also when mixed with distributed table: +REVOKE SELECT (test_r, test_mix) ON local_table, dist_table FROM grant_user_1; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- +-- check TRUNCATE is not propagated (inccorect grammar) +-- also ensure no privs are propagated at all with "partially" incorrect grammar +-- we use only TRUNCATE/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +GRANT TRUNCATE (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +ERROR: invalid privilege type TRUNCATE for column +SET ROLE grant_user_0; +-- still not granted: +SELECT test_r FROM ref_table; +ERROR: permission denied for table ref_table +-- still not granted: +TRUNCATE ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- no cleanup required +-- +-- check we do not propage from a worker +-- we use only SELECT +-- +\c - - - :worker_1_port +SET search_path TO grant_on_table; +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0; +ERROR: operation is not allowed on this node +HINT: Connect to the coordinator and run it again. +\c - - - :master_port +SET search_path TO grant_on_table; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- no cleanup required +-- +-- check we do propagate WITH GRANT OPTION +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +SET ROLE grant_user_1; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +ERROR: permission denied for table ref_table +RESET ROLE; +-- grant with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0 WITH GRANT OPTION; +SET ROLE grant_user_0; +-- grant using a role with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_1; +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM ref_table; + test_r | test_mix +--------------------------------------------------------------------- +(0 rows) + +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57637 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57638 | (grant_on_table.ref_table,test_mix,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57636 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57637 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") + 57638 | (grant_on_table.ref_table,test_r,"{grant_user_0=r*/postgres,grant_user_1=r/grant_user_0}") +(6 rows) + +-- cleanup and further checks: +SET ROLE grant_user_0; +-- revoke as grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_1; +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_0=r*/postgres}) + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r*/postgres}) +(6 rows) + +-- revoke only grant options from grant_user_0: +REVOKE GRANT OPTION FOR SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_mix,{grant_user_0=r/postgres}) + 57636 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.ref_table,test_r,{grant_user_0=r/postgres}) +(6 rows) + +-- revoke select from grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (dist_table,) + 57637 | (dist_table,) + 57638 | (dist_table,) + 57636 | (local_table,) + 57637 | (local_table,) + 57638 | (local_table,) + 57636 | (ref_table,{postgres=arwdDxt/postgres}) + 57637 | (ref_table,{postgres=arwdDxt/postgres}) + 57638 | (ref_table,{postgres=arwdDxt/postgres}) +(9 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE dist_table, ref_table; +SELECT result FROM run_command_on_all_nodes('DROP TABLE grant_on_table.local_table'); + result +--------------------------------------------------------------------- + DROP TABLE + DROP TABLE + DROP TABLE +(3 rows) + +RESET client_min_messages; +-- +-- check we propagate privileges when GRANTed before distributed and when adding a node +-- we use only SELECT +-- +-- test propagation on columns when distributing the table after GRANT has been executed +CREATE TABLE grant_table_propagated (id int primary key); +GRANT SELECT (id) ON grant_table_propagated TO grant_user_0; +SELECT create_distributed_table('grant_table_propagated', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SET ROLE grant_user_0; +-- granted: +SELECT id FROM grant_table_propagated; + id +--------------------------------------------------------------------- +(0 rows) + +RESET ROLE; +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated,) + 57637 | (grant_table_propagated,) + 57638 | (grant_table_propagated,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.grant_table_propagated,id,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated FROM grant_user_0; +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated,) + 57637 | (grant_table_propagated,) + 57638 | (grant_table_propagated,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated; +RESET client_min_messages; +-- similar test but adding a node after the fact +-- remove one of the worker nodes: +SELECT citus_remove_node('localhost', :worker_2_port); + citus_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE grant_table_propagated_after (id int primary key); +SET citus.shard_replication_factor TO 1; +SELECT create_distributed_table('grant_table_propagated_after', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SET citus.shard_replication_factor TO 2; +GRANT SELECT (id) ON grant_table_propagated_after TO grant_user_0; +-- add back the worker node +SELECT FROM citus_add_node('localhost', :worker_2_port); +-- +(1 row) + +-- granted: +SELECT id FROM grant_table_propagated_after; + id +--------------------------------------------------------------------- +(0 rows) + +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated_after'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated_after''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated_after,) + 57637 | (grant_table_propagated_after,) + 57638 | (grant_table_propagated_after,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) + 57637 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) + 57638 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) +(3 rows) + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; +:verify_grant_table ; + nodeport | unnest +--------------------------------------------------------------------- + 57636 | (grant_table_propagated_after,) + 57637 | (grant_table_propagated_after,) + 57638 | (grant_table_propagated_after,) +(3 rows) + +:verify_grant_attributes ; + nodeport | unnest +--------------------------------------------------------------------- +(0 rows) + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated_after; +RESET client_min_messages; +-- global cleanup +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP SCHEMA grant_on_table CASCADE; +DROP ROLE grant_user_0, grant_user_1; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/expected/multi_multiuser_master_protocol.out b/src/test/regress/expected/multi_multiuser_master_protocol.out index 9d08bf454..422d30858 100644 --- a/src/test/regress/expected/multi_multiuser_master_protocol.out +++ b/src/test/regress/expected/multi_multiuser_master_protocol.out @@ -313,9 +313,8 @@ SELECT * FROM trivial_full_access ORDER BY id; (4 rows) RESET ROLE; --- verify column level grants are not supported +-- verify column level grants are supported GRANT UPDATE (id) ON trivial_postgres TO read_access; -ERROR: grant/revoke on column list is currently unsupported DROP TABLE trivial_full_access; DROP TABLE trivial_postgres; DROP TABLE stage_full_access; diff --git a/src/test/regress/expected/multi_multiuser_master_protocol_0.out b/src/test/regress/expected/multi_multiuser_master_protocol_0.out index f8422ad67..8fb2a87e4 100644 --- a/src/test/regress/expected/multi_multiuser_master_protocol_0.out +++ b/src/test/regress/expected/multi_multiuser_master_protocol_0.out @@ -304,9 +304,8 @@ SELECT * FROM trivial_full_access ORDER BY id; (4 rows) RESET ROLE; --- verify column level grants are not supported +-- verify column level grants are supported GRANT UPDATE (id) ON trivial_postgres TO read_access; -ERROR: grant/revoke on column list is currently unsupported DROP TABLE trivial_full_access; DROP TABLE trivial_postgres; DROP TABLE stage_full_access; diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index cfff00942..39a9e4070 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -57,6 +57,7 @@ test: multi_metadata_attributes test: multi_read_from_secondaries +test: grant_on_table_propagation test: grant_on_database_propagation test: alter_database_propagation diff --git a/src/test/regress/sql/grant_on_table_propagation.sql b/src/test/regress/sql/grant_on_table_propagation.sql new file mode 100644 index 000000000..754870c83 --- /dev/null +++ b/src/test/regress/sql/grant_on_table_propagation.sql @@ -0,0 +1,444 @@ +-- PostgreSQL 17 introduces MAINTAIN privilege, this change the default output +-- of some test queries. +-- +-- Alternative "grant_on_table_propagation_0.out" and this comment can be +-- removed when support for PostgreSQL versions < 17 is dropped. + +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 17 AS server_version_ge_17; + +-- +-- GRANT_ON_TABLE_PROPAGATION +-- +SET citus.shard_replication_factor TO 1; +CREATE SCHEMA grant_on_table; +SET search_path TO grant_on_table; + +-- create some simple tables: 1 local on all nodes and 2 managed by citus +-- null_privs ACL must not be updated in anyway. +CREATE TABLE dist_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL/mixed + , null_privs text + ); +SELECT create_distributed_table('dist_table', 'id'); + +CREATE TABLE ref_table (id bigint GENERATED BY DEFAULT AS IDENTITY primary key + , test_a int -- test for INSERT + , test_r text -- test for SELECT + , test_w text -- test for UPDATE + , test_mix int -- test for ALL + , null_privs text + ); +SELECT create_reference_table('ref_table'); + +SELECT result FROM run_command_on_all_nodes('CREATE TABLE grant_on_table.local_table (id int GENERATED BY DEFAULT AS IDENTITY primary key , test_a int, test_r text, test_w text, test_mix int, null_privs text)'); + +-- queries used to check privileges: +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''dist_table'', ''ref_table'', ''local_table'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.dist_table''::regclass, ''grant_on_table.ref_table''::regclass, ''grant_on_table.local_table''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +-- create some users +CREATE USER grant_user_0; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_0; +CREATE USER grant_user_1; +GRANT USAGE ON SCHEMA grant_on_table TO grant_user_1; +-- this one should not be granted anything: +CREATE USER nogrant_user; +GRANT USAGE ON SCHEMA grant_on_table TO nogrant_user; + +-- +-- tests related to columns ACL +-- + +-- +-- when executing a table level grant, "postgres" is add/listed in pg_class.relacl +-- but nothing happens with revoke, as a result pg_class.relacl is not stable. +-- in order to have immutable cleanup results in those tests, init the +-- "postgres" special case +GRANT SELECT ON ref_table TO grant_user_0; +REVOKE SELECT ON ref_table FROM grant_user_0; + +-- +-- check we are able to propagate a single attribute privilege +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +RESET ROLE; +GRANT SELECT (test_r) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT test_r FROM ref_table; +-- not granted: +SELECT test_a FROM ref_table; +SET ROLE grant_user_1; +-- not granted: +SELECT test_r FROM ref_table; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (test_r) ON ref_table FROM grant_user_0; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check we are able to propagate a privilege to multiple attributes, users and tables at once +-- we use only INSERT/UPDATE +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (test_a) VALUES (1); +RESET ROLE; +-- we would prefer not have to grant INSERT (id) as expected in the standard but +-- Citus rewrite queries with such attributes, which prevent standard to be applied. +GRANT INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (test_a) VALUES (1); +-- granted: +UPDATE ref_table SET test_w = 2, test_mix = 2; +-- not granted: +INSERT INTO ref_table (test_w) VALUES (1); +SET ROLE grant_user_1; +-- granted: +INSERT INTO dist_table (test_a) VALUES (1); +-- granted: +UPDATE dist_table SET test_w = 3, test_mix = 3; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE INSERT (id, test_a), UPDATE (test_w, test_mix) ON ref_table, dist_table FROM grant_user_0, grant_user_1; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check we are able to propagate a table privilege associated with an attribute level privilege +-- we use only SELECT/DELETE +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +RESET ROLE; +GRANT SELECT (test_r, test_mix), DELETE ON ref_table TO grant_user_0, grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT test_r, test_mix FROM ref_table; +-- granted: +DELETE FROM ref_table; +-- not granted: +SELECT test_a FROM ref_table; +-- not granted: +UPDATE ref_table SET null_privs = 3; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (test_r, test_mix), DELETE ON ref_table FROM grant_user_0, grant_user_1; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check we also propagate system columns +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT ctid, xmin FROM ref_table; +RESET ROLE; +GRANT SELECT (ctid, xmin) ON ref_table TO grant_user_0; +SET ROLE grant_user_0; +-- granted: +SELECT ctid, xmin FROM ref_table; +-- not granted: +SELECT ctid, test_a FROM ref_table; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (ctid, xmin) ON ref_table FROM grant_user_0; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check we correctly propagate ALL, which has a few special cases +-- we use only ALL/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +INSERT INTO ref_table (id) VALUES (13); +SET ROLE grant_user_1; +-- not granted yet: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +RESET ROLE; +GRANT ALL (id) ON ref_table TO grant_user_0; +GRANT ALL (id, test_mix) ON ref_table TO grant_user_1; +-- ALL cannot be mixed with other privs +-- should error: +GRANT SELECT (null_privs), ALL (null_privs) ON ref_table TO nogrant_user; +-- should error: +GRANT ALL (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +SET ROLE grant_user_0; +-- granted: +INSERT INTO ref_table (id) VALUES (13); +SET ROLE grant_user_1; +-- granted: +INSERT INTO ref_table (id, test_mix) VALUES (9, 3); +-- not granted: +INSERT INTO ref_table (null_privs) VALUES (3); +-- granted: +SELECT id, test_mix FROM ref_table; +-- not granted: +SELECT null_privs FROM ref_table; +-- not granted: +DELETE FROM ref_table; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE ALL (id) ON ref_table FROM grant_user_0; +REVOKE ALL (id, test_mix) ON ref_table FROM grant_user_1; +TRUNCATE ref_table; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check we correctly propagate when mixed with local table, but only on the +-- current local table, not others +-- we use only INSERT/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; +RESET ROLE; +-- NOTE: +-- test special case: ALL TABLES IN SCHEMA is not supposed to be correct +-- but is accepted by PostgreSQL - non documented feature +GRANT SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table TO grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +GRANT INSERT (test_mix) ON local_table TO grant_user_0; +-- check we can propagate also when mixed with distributed table: +GRANT SELECT (test_r, test_mix) ON local_table, dist_table TO grant_user_1; +SET ROLE grant_user_0; +-- granted: +SELECT id FROM ref_table +UNION ALL SELECT id FROM dist_table +UNION ALL SELECT id FROM local_table; +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM dist_table +UNION ALL SELECT test_r, test_mix FROM local_table; +RESET ROLE; + +-- check on coordinator and workers +-- we pay special attention to local_table privileges here: +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (id) ON ALL TABLES IN SCHEMA grant_on_table FROM grant_user_0; +-- check non propagation for local table (we'll just check ACL later, no INSERT testing) +REVOKE INSERT (test_mix) ON local_table FROM grant_user_0; +-- check we can propagate also when mixed with distributed table: +REVOKE SELECT (test_r, test_mix) ON local_table, dist_table FROM grant_user_1; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- +-- check TRUNCATE is not propagated (inccorect grammar) +-- also ensure no privs are propagated at all with "partially" incorrect grammar +-- we use only TRUNCATE/SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r FROM ref_table; +RESET ROLE; +GRANT TRUNCATE (null_privs), SELECT (null_privs) ON ref_table TO nogrant_user; +SET ROLE grant_user_0; +-- still not granted: +SELECT test_r FROM ref_table; +-- still not granted: +TRUNCATE ref_table; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- no cleanup required + +-- +-- check we do not propage from a worker +-- we use only SELECT +-- +\c - - - :worker_1_port +SET search_path TO grant_on_table; +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0; +\c - - - :master_port +SET search_path TO grant_on_table; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- no cleanup required + +-- +-- check we do propagate WITH GRANT OPTION +-- we use only SELECT +-- +SET ROLE grant_user_0; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +SET ROLE grant_user_1; +-- not granted yet: +SELECT test_r, test_mix FROM ref_table; +RESET ROLE; +-- grant with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_0 WITH GRANT OPTION; +SET ROLE grant_user_0; +-- grant using a role with grant option +GRANT SELECT (test_r, test_mix) ON ref_table TO grant_user_1; +SET ROLE grant_user_1; +-- granted: +SELECT test_r, test_mix FROM ref_table; +RESET ROLE; + +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup and further checks: +SET ROLE grant_user_0; +-- revoke as grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_1; +RESET ROLE; +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- revoke only grant options from grant_user_0: +REVOKE GRANT OPTION FOR SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- revoke select from grant_user_0: +REVOKE SELECT (test_r, test_mix) ON ref_table FROM grant_user_0; +-- check on coordinator and workers +:verify_grant_table ; +:verify_grant_attributes ; + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE dist_table, ref_table; +SELECT result FROM run_command_on_all_nodes('DROP TABLE grant_on_table.local_table'); +RESET client_min_messages; + +-- +-- check we propagate privileges when GRANTed before distributed and when adding a node +-- we use only SELECT +-- +-- test propagation on columns when distributing the table after GRANT has been executed +CREATE TABLE grant_table_propagated (id int primary key); +GRANT SELECT (id) ON grant_table_propagated TO grant_user_0; +SELECT create_distributed_table('grant_table_propagated', 'id'); +SET ROLE grant_user_0; +-- granted: +SELECT id FROM grant_table_propagated; +RESET ROLE; + +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated FROM grant_user_0; + +:verify_grant_table ; +:verify_grant_attributes ; + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated; +RESET client_min_messages; + +-- similar test but adding a node after the fact +-- remove one of the worker nodes: +SELECT citus_remove_node('localhost', :worker_2_port); +CREATE TABLE grant_table_propagated_after (id int primary key); +SET citus.shard_replication_factor TO 1; +SELECT create_distributed_table('grant_table_propagated_after', 'id'); +SET citus.shard_replication_factor TO 2; +GRANT SELECT (id) ON grant_table_propagated_after TO grant_user_0; +-- add back the worker node +SELECT FROM citus_add_node('localhost', :worker_2_port); + +-- granted: +SELECT id FROM grant_table_propagated_after; + +-- check on coordinator and workers +\set verify_grant_table 'SELECT nodeport, unnest(result::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((relname, relacl) order by 1) FROM pg_class WHERE relname IN (''grant_table_propagated_after'') $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +\set verify_grant_attributes 'SELECT nodeport, unnest(coalesce(nullif(result, ''''), ''{}'')::text[]) FROM run_command_on_all_nodes($$SELECT array_agg((attrelid::regclass, attname, attacl) order by 1, 2) FROM pg_attribute WHERE attrelid IN (''grant_on_table.grant_table_propagated_after''::regclass) AND attacl IS NOT NULL $$) join pg_dist_node using (nodeid) ORDER BY 2, 1' + +:verify_grant_table ; +:verify_grant_attributes ; + +-- cleanup +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; + +:verify_grant_table ; +:verify_grant_attributes ; + +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP TABLE grant_table_propagated_after; +RESET client_min_messages; + +-- global cleanup +-- prevent useless messages on DROP objects. +SET client_min_messages TO ERROR; +DROP SCHEMA grant_on_table CASCADE; +DROP ROLE grant_user_0, grant_user_1; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/multi_multiuser_master_protocol.sql b/src/test/regress/sql/multi_multiuser_master_protocol.sql index e13605fff..1510794c8 100644 --- a/src/test/regress/sql/multi_multiuser_master_protocol.sql +++ b/src/test/regress/sql/multi_multiuser_master_protocol.sql @@ -121,7 +121,7 @@ SELECT * FROM trivial_postgres ORDER BY id; SELECT * FROM trivial_full_access ORDER BY id; RESET ROLE; --- verify column level grants are not supported +-- verify column level grants are supported GRANT UPDATE (id) ON trivial_postgres TO read_access; DROP TABLE trivial_full_access; From f084b79a4b53cc84495eb987b6a38c5731a6e115 Mon Sep 17 00:00:00 2001 From: manaldush Date: Fri, 4 Apr 2025 16:03:41 +0300 Subject: [PATCH 5/6] AddressSanitizer: stack-use-after-scope on address in CreateBackgroundJob (#7949) Var jobTypeName is created on stack and its value over pointer is used in heap_form_tuple, so we have stack use out of scope. Issue was detected with adress sanitizer. Fixes #7943. --- src/backend/distributed/metadata/metadata_utility.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/metadata/metadata_utility.c b/src/backend/distributed/metadata/metadata_utility.c index 6d8ed0808..0c3dbbda3 100644 --- a/src/backend/distributed/metadata/metadata_utility.c +++ b/src/backend/distributed/metadata/metadata_utility.c @@ -3024,6 +3024,8 @@ CreateBackgroundJob(const char *jobType, const char *description) /* insert new job */ Datum values[Natts_pg_dist_background_job] = { 0 }; bool isnull[Natts_pg_dist_background_job] = { 0 }; + + NameData jobTypeName = { 0 }; memset(isnull, true, sizeof(isnull)); int64 jobId = GetNextBackgroundJobsJobId(); @@ -3036,7 +3038,6 @@ CreateBackgroundJob(const char *jobType, const char *description) if (jobType) { - NameData jobTypeName = { 0 }; namestrcpy(&jobTypeName, jobType); InitFieldValue(Anum_pg_dist_background_job_job_type, values, isnull, NameGetDatum(&jobTypeName)); From 0e6127c4f60b26da18ac8df48d2b6a8de5ed9d4c Mon Sep 17 00:00:00 2001 From: manaldush Date: Fri, 4 Apr 2025 16:27:56 +0300 Subject: [PATCH 6/6] AddressSanitizer: stack-use-after-scope on distributed_planner:HasUnresolvedExternParamsWalker (#7948) Var externParamPlaceholder is created on stack, and its address is used for paramFetch. Postgres code return address of externParamPlaceholder var to externParam, then code flow go out of scope and dereference pointer on stack out of scope. Fixes https://github.com/citusdata/citus/issues/7941. --------- Co-authored-by: Onur Tirtir --- src/backend/distributed/planner/distributed_planner.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/backend/distributed/planner/distributed_planner.c b/src/backend/distributed/planner/distributed_planner.c index ac7754cb9..7f8f827ea 100644 --- a/src/backend/distributed/planner/distributed_planner.c +++ b/src/backend/distributed/planner/distributed_planner.c @@ -2549,21 +2549,20 @@ HasUnresolvedExternParamsWalker(Node *expression, ParamListInfo boundParams) /* check whether parameter is available (and valid) */ if (boundParams && paramId > 0 && paramId <= boundParams->numParams) { - ParamExternData *externParam = NULL; + Oid paramType = InvalidOid; /* give hook a chance in case parameter is dynamic */ if (boundParams->paramFetch != NULL) { ParamExternData externParamPlaceholder; - externParam = (*boundParams->paramFetch)(boundParams, paramId, false, - &externParamPlaceholder); + paramType = (*boundParams->paramFetch)(boundParams, paramId, false, + &externParamPlaceholder)->ptype; } else { - externParam = &boundParams->params[paramId - 1]; + paramType = boundParams->params[paramId - 1].ptype; } - Oid paramType = externParam->ptype; if (OidIsValid(paramType)) { return false;