From 3c73117597ba0269de5c20b28a8804056ea38970 Mon Sep 17 00:00:00 2001 From: gurkanindibay Date: Wed, 17 Jan 2024 10:29:23 +0300 Subject: [PATCH] Adds metadata sync support for grant on parameter --- src/backend/distributed/commands/parameter.c | 120 ++++++++++++++++++ .../distributed/metadata/metadata_sync.c | 50 ++++++-- src/include/distributed/grant_utils.h | 34 +++++ .../grant_on_parameter_propagation.out | 68 +++++++++- src/test/regress/multi_1_schedule | 2 +- .../sql/grant_on_parameter_propagation.sql | 14 ++ 6 files changed, 275 insertions(+), 13 deletions(-) create mode 100644 src/include/distributed/grant_utils.h diff --git a/src/backend/distributed/commands/parameter.c b/src/backend/distributed/commands/parameter.c index 84c6e358b..3e1c91a59 100644 --- a/src/backend/distributed/commands/parameter.c +++ b/src/backend/distributed/commands/parameter.c @@ -1,10 +1,24 @@ #include "postgres.h" + +#include "access/genam.h" #include "catalog/namespace.h" +#include "catalog/pg_parameter_acl.h" #include "commands/defrem.h" #include "distributed/metadata_sync.h" #include "distributed/deparser.h" #include "distributed/commands.h" +#include "distributed/grant_utils.h" +#include "distributed/listutils.h" + +#include "utils/acl.h" +#include "utils/builtins.h" +#include "utils/syscache.h" + +static List *GenerateGrantOnParameterFromAclItem(char *parameterName, AclItem *aclItem); +static bool HasAclGrantOption(AclItem *aclItem,AclMode aclMode); +static void CheckPermissionsAndGrants(AclItem *aclItem, AclMode modes[], int numModes); +static void CheckAndAppendQuery(List **queries, AclItem *aclItem, Oid granteeOid, char *parameterName, AclMode mode, char *modeStr); List * @@ -25,3 +39,109 @@ PostprocessGrantParameterStmt(Node *node, const char *queryString) return NontransactionalNodeDDLTaskList(REMOTE_NODES, commands); } + + +/* + * GenerateGrantOnParameterFromAclItem generates a query string for replicating a users permissions + * on a database. + */ +static List * +GenerateGrantOnParameterFromAclItem(char *parameterName, AclItem *aclItem) +{ + /* + * seems unlikely but we check if there is a grant option in the list without the actual permission + */ + CheckPermissionsAndGrants(aclItem, (AclMode[]) {ACL_SET, ACL_ALTER_SYSTEM}, 2); + Oid granteeOid = aclItem->ai_grantee; + List *queries = NIL; + + queries = lappend(queries, GenerateSetRoleQuery(aclItem->ai_grantor)); + + CheckAndAppendQuery(&queries, aclItem, granteeOid, parameterName, ACL_SET, "SET"); + CheckAndAppendQuery(&queries, aclItem, granteeOid, parameterName, ACL_ALTER_SYSTEM, "ALTER SYSTEM"); + + queries = lappend(queries, "RESET ROLE"); + + return queries; +} + +static void CheckAndAppendQuery(List **queries, AclItem *aclItem, Oid granteeOid, char *parameterName, AclMode mode, char *modeStr) { + AclResult aclresult = pg_parameter_aclcheck(parameterName, granteeOid, mode); + if (aclresult == ACLCHECK_OK) + { + char *query = DeparseTreeNode((Node *) GenerateGrantStmtForRightsWithObjectName( + OBJECT_PARAMETER_ACL, granteeOid, parameterName, + modeStr, + HasAclGrantOption(aclItem, mode))); + + // remove the semicolon at the end of the query since it is already + // appended in metadata_sync phase + query[strlen(query) - 1] = '\0'; + + *queries = lappend(*queries, query); + } +} + +static void CheckPermissionsAndGrants(AclItem *aclItem, AclMode modes[], int numModes) { + AclMode permissions = ACLITEM_GET_PRIVS(*aclItem) & ACL_ALL_RIGHTS_PARAMETER_ACL; + AclMode grants = ACLITEM_GET_GOPTIONS(*aclItem) & ACL_ALL_RIGHTS_PARAMETER_ACL; + + for (int i = 0; i < numModes; i++) { + AclMode mode = modes[i]; + Assert(!(grants & mode) || (permissions & mode)); + } +} + +static bool HasAclGrantOption(AclItem *aclItem,AclMode aclMode) +{ + return (aclItem->ai_privs & ACL_GRANT_OPTION_FOR(aclMode)) != 0; +} + +List * GrantOnParameters(void) +{ + /* Open pg_shdescription catalog */ + Relation paramPermissionRelation = table_open(ParameterAclRelationId, AccessShareLock); + + + int scanKeyCount = 0; + bool indexOk = false; + SysScanDesc scan = systable_beginscan(paramPermissionRelation, InvalidOid, + indexOk, NULL, scanKeyCount,NULL); + HeapTuple tuple; + List *commands = NIL; + while ((tuple = systable_getnext(scan)) != NULL) + { + + bool isNull = false; + + TupleDesc tupdesc = RelationGetDescr(paramPermissionRelation); + + Datum aclDatum = heap_getattr(tuple, Anum_pg_parameter_acl_paracl, tupdesc, + &isNull); + Datum parameterNameDatum = heap_getattr(tuple, Anum_pg_parameter_acl_parname, tupdesc, + &isNull); + + char *parameterName = TextDatumGetCString(parameterNameDatum); + + Acl *acl = DatumGetAclPCopy(aclDatum); + AclItem *aclDat = ACL_DAT(acl); + int aclNum = ACL_NUM(acl); + + + + for (int i = 0; i < aclNum; i++) + { + commands = list_concat(commands, + GenerateGrantOnParameterFromAclItem( + parameterName, &aclDat[i])); + } + + + } + + /* End the scan and close the catalog */ + systable_endscan(scan); + table_close(paramPermissionRelation, AccessShareLock); + + return commands; +} diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index 842a45519..c5266bff4 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -88,6 +88,7 @@ #include "distributed/tenant_schema_metadata.h" #include "distributed/utils/array_type.h" #include "distributed/utils/function.h" +#include "distributed/grant_utils.h" #include "distributed/version_compat.h" #include "distributed/worker_manager.h" #include "distributed/worker_protocol.h" @@ -115,11 +116,6 @@ static bool SyncNodeMetadataSnapshotToNode(WorkerNode *workerNode, bool raiseOnE static void DropMetadataSnapshotOnNode(WorkerNode *workerNode); static char * CreateSequenceDependencyCommand(Oid relationId, Oid sequenceId, char *columnName); -static GrantStmt * GenerateGrantStmtForRights(ObjectType objectType, - Oid roleOid, - Oid objectId, - char *permission, - bool withGrantOption); static List * GetObjectsForGrantStmt(ObjectType objectType, Oid objectId); static AccessPriv * GetAccessPrivObjectForGrantStmt(char *permission); static List * GenerateGrantOnSchemaQueriesFromAclItem(Oid schemaOid, @@ -130,7 +126,6 @@ static List * GenerateGrantOnFunctionQueriesFromAclItem(Oid schemaOid, static List * GrantOnSequenceDDLCommands(Oid sequenceOid); static List * GenerateGrantOnSequenceQueriesFromAclItem(Oid sequenceOid, AclItem *aclItem); -static char * GenerateSetRoleQuery(Oid roleOid); static void MetadataSyncSigTermHandler(SIGNAL_ARGS); static void MetadataSyncSigAlrmHandler(SIGNAL_ARGS); @@ -2159,18 +2154,51 @@ GenerateGrantOnDatabaseFromAclItem(Oid databaseOid, AclItem *aclItem) * The field `objects` of GrantStmt doesn't have a common structure for all types. * Make sure you have added your object type to GetObjectsForGrantStmt. */ -static GrantStmt * +GrantStmt * GenerateGrantStmtForRights(ObjectType objectType, Oid roleOid, Oid objectId, char *permission, bool withGrantOption) { + return BaseGenerateGrantStmtForRights(objectType,roleOid,objectId,NULL,permission,withGrantOption); +} + +GrantStmt * +GenerateGrantStmtForRightsWithObjectName(ObjectType objectType, + Oid roleOid, + char *objectName, + char *permission, + bool withGrantOption) +{ + return BaseGenerateGrantStmtForRights(objectType,roleOid,InvalidOid,objectName,permission,withGrantOption); +} + + +GrantStmt * +BaseGenerateGrantStmtForRights(ObjectType objectType, + Oid roleOid, + Oid objectId, + char *objectName, + char *permission, + bool withGrantOption) +{ + + //either objectId or objectName should be valid + Assert(objectId != InvalidOid || objectName != NULL); + GrantStmt *stmt = makeNode(GrantStmt); stmt->is_grant = true; stmt->targtype = ACL_TARGET_OBJECT; stmt->objtype = objectType; - stmt->objects = GetObjectsForGrantStmt(objectType, objectId); + if (objectId != InvalidOid) + { + stmt->objects = GetObjectsForGrantStmt(objectType, objectId); + } + else + { + stmt->objects = list_make1(makeString(objectName)); + } stmt->privileges = list_make1(GetAccessPrivObjectForGrantStmt(permission)); stmt->grantees = list_make1(GetRoleSpecObjectForUser(roleOid)); stmt->grant_option = withGrantOption; @@ -2179,6 +2207,7 @@ GenerateGrantStmtForRights(ObjectType objectType, } + /* * GetObjectsForGrantStmt takes an object type and object id and returns the 'objects' * field to be used when creating GrantStmt. We have only one object here (the one with @@ -2230,6 +2259,7 @@ GetObjectsForGrantStmt(ObjectType objectType, Oid objectId) return list_make1(makeString(get_database_name(objectId))); } + default: { elog(ERROR, "unsupported object type for GRANT"); @@ -2563,7 +2593,7 @@ SetLocalEnableMetadataSync(bool state) } -static char * +char * GenerateSetRoleQuery(Oid roleOid) { StringInfo buf = makeStringInfo(); @@ -4682,6 +4712,8 @@ PropagateNodeWideObjectsCommandList(void) List *alterRoleSetCommands = GenerateAlterRoleSetCommandForRole(InvalidOid); ddlCommands = list_concat(ddlCommands, alterRoleSetCommands); } + List *grantOnParameterCommands = GrantOnParameters(); + ddlCommands = list_concat(ddlCommands, grantOnParameterCommands); return ddlCommands; } diff --git a/src/include/distributed/grant_utils.h b/src/include/distributed/grant_utils.h new file mode 100644 index 000000000..8f9e24d8f --- /dev/null +++ b/src/include/distributed/grant_utils.h @@ -0,0 +1,34 @@ +/*------------------------------------------------------------------------- + * + * grant_utils.h + * + * Routines for grant operations. + * + *------------------------------------------------------------------------- + */ +#ifndef CITUS_GRANT_UTILS_H +#define CITUS_GRANT_UTILS_H +#include "postgres.h" +#include "nodes/parsenodes.h" + +extern List * GrantOnParameters(void); +extern char * GenerateSetRoleQuery(Oid roleOid); +extern GrantStmt * GenerateGrantStmtForRights(ObjectType objectType, + Oid roleOid, + Oid objectId, + char *permission, + bool withGrantOption); +extern GrantStmt *GenerateGrantStmtForRightsWithObjectName(ObjectType objectType, + Oid roleOid, + char *objectName, + char *permission, + bool withGrantOption); +extern GrantStmt *BaseGenerateGrantStmtForRights(ObjectType objectType, + Oid roleOid, + Oid objectId, + char *objectName, + char *permission, + bool withGrantOption); + + +#endif /* CITUS_GRANT_UTILS_H */ diff --git a/src/test/regress/expected/grant_on_parameter_propagation.out b/src/test/regress/expected/grant_on_parameter_propagation.out index 091fcbf7d..ab32b2cd0 100644 --- a/src/test/regress/expected/grant_on_parameter_propagation.out +++ b/src/test/regress/expected/grant_on_parameter_propagation.out @@ -169,13 +169,75 @@ SELECT check_parameter_privileges(ARRAY['grant_param_user4'],ARRAY['max_connecti (f,grant_param_user4,shared_buffers,"ALTER SYSTEM") (6 rows) +--test metadata_sync +SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +GRANT SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_user3,"grant_param_user5-\!" WITH GRANT OPTION GRANTED BY CURRENT_USER; +SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']); + check_parameter_privileges +--------------------------------------------------------------------- + (t,grant_param_user3,max_connections,SET) + (t,grant_param_user3,max_connections,SET) + (t,grant_param_user3,max_connections,"ALTER SYSTEM") + (t,grant_param_user3,max_connections,"ALTER SYSTEM") + (t,grant_param_user3,shared_buffers,SET) + (t,grant_param_user3,shared_buffers,SET) + (t,grant_param_user3,shared_buffers,"ALTER SYSTEM") + (t,grant_param_user3,shared_buffers,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",max_connections,SET) + (t,"grant_param_user5-\\!",max_connections,SET) + (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",shared_buffers,SET) + (t,"grant_param_user5-\\!",shared_buffers,SET) + (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM") +(16 rows) + +SELECT 1 FROM citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']); + check_parameter_privileges +--------------------------------------------------------------------- + (t,grant_param_user3,max_connections,SET) + (t,grant_param_user3,max_connections,SET) + (t,grant_param_user3,max_connections,SET) + (t,grant_param_user3,max_connections,"ALTER SYSTEM") + (t,grant_param_user3,max_connections,"ALTER SYSTEM") + (t,grant_param_user3,max_connections,"ALTER SYSTEM") + (t,grant_param_user3,shared_buffers,SET) + (t,grant_param_user3,shared_buffers,SET) + (t,grant_param_user3,shared_buffers,SET) + (t,grant_param_user3,shared_buffers,"ALTER SYSTEM") + (t,grant_param_user3,shared_buffers,"ALTER SYSTEM") + (t,grant_param_user3,shared_buffers,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",max_connections,SET) + (t,"grant_param_user5-\\!",max_connections,SET) + (t,"grant_param_user5-\\!",max_connections,SET) + (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",shared_buffers,SET) + (t,"grant_param_user5-\\!",shared_buffers,SET) + (t,"grant_param_user5-\\!",shared_buffers,SET) + (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM") + (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM") +(24 rows) + +REVOKE SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers FROM grant_param_user3,"grant_param_user5-\!" cascade; --clean all resources DROP USER grant_param_user1; DROP USER grant_param_user2; DROP USER grant_param_user3; -ERROR: role "grant_param_user3" cannot be dropped because some objects depend on it -DETAIL: privileges for parameter max_connections -privileges for parameter shared_buffers DROP USER grant_param_user4; DROP USER "grant_param_user5-\!"; reset citus.log_remote_commands; diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 726c64e32..b98207b20 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -38,6 +38,7 @@ test: create_single_shard_table test: create_drop_database_propagation test: create_drop_database_propagation_pg15 test: create_drop_database_propagation_pg16 +test: grant_on_parameter_propagation # don't parallelize single_shard_table_udfs to make sure colocation ids are sequential test: single_shard_table_udfs test: schema_based_sharding @@ -60,7 +61,6 @@ test: alter_database_propagation test: citus_shards test: reassign_owned -test: grant_on_parameter_propagation # ---------- # multi_citus_tools tests utility functions written for citus tools diff --git a/src/test/regress/sql/grant_on_parameter_propagation.sql b/src/test/regress/sql/grant_on_parameter_propagation.sql index 1a4a98db0..d20831941 100644 --- a/src/test/regress/sql/grant_on_parameter_propagation.sql +++ b/src/test/regress/sql/grant_on_parameter_propagation.sql @@ -47,6 +47,20 @@ GRANT ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_us SELECT check_parameter_privileges(ARRAY['grant_param_user4'],ARRAY['max_connections','shared_buffers'], ARRAY['ALTER SYSTEM']); +--test metadata_sync + +SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); +GRANT SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_user3,"grant_param_user5-\!" WITH GRANT OPTION GRANTED BY CURRENT_USER; + +SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']); + +SELECT 1 FROM citus_add_node('localhost', :worker_2_port); + +SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']); + +REVOKE SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers FROM grant_param_user3,"grant_param_user5-\!" cascade; + + --clean all resources DROP USER grant_param_user1; DROP USER grant_param_user2;