Breaks the bound of grants with DDL

src/backend/distributed/commands/dependencies.c

@@ -44,7 +44,7 @@ static int ObjectAddressComparator(const void *a, const void *b);
 static void EnsureDependenciesExistOnAllNodes(const ObjectAddress *target);
 static void EnsureRequiredObjectSetExistOnAllNodes(const ObjectAddress *target,
 												   RequiredObjectSet requiredObjectSet);
-static List * GetDependencyCreateDDLCommands(const ObjectAddress *dependency);
+static List * GetDependencyCreateDDLCommands(const ObjectAddress *dependency,bool fetchGrantStatements);
 static bool ShouldPropagateObject(const ObjectAddress *address);
 static char * DropTableIfExistsCommand(Oid relationId);
 
@@ -164,7 +164,7 @@ EnsureRequiredObjectSetExistOnAllNodes(const ObjectAddress *target,
 	ObjectAddress *object = NULL;
 	foreach_ptr(object, objectsToBeCreated)
 	{
-		List *dependencyCommands = GetDependencyCreateDDLCommands(object);
+		List *dependencyCommands = GetDependencyCreateDDLCommands(object,true);
 		ddlCommands = list_concat(ddlCommands, dependencyCommands);
 
 		/* create a new list with objects that actually created commands */
@@ -432,7 +432,7 @@ GetDistributableDependenciesForObject(const ObjectAddress *target)
 		 * in nodes, but we utilize logic it follows to choose the objects that could
 		 * be distributed
 		 */
-		List *dependencyCommands = GetDependencyCreateDDLCommands(dependency);
+		List *dependencyCommands = GetDependencyCreateDDLCommands(dependency,true);
 
 		/* create a new list with dependencies that actually created commands */
 		if (list_length(dependencyCommands) > 0)
@@ -465,7 +465,7 @@ DropTableIfExistsCommand(Oid relationId)
  * commands to execute on a worker to create the object.
  */
 static List *
-GetDependencyCreateDDLCommands(const ObjectAddress *dependency)
+GetDependencyCreateDDLCommands(const ObjectAddress *dependency, bool fetchGrantStatements)
 {
 	switch (getObjectClass(dependency))
 	{
@@ -605,7 +605,7 @@ GetDependencyCreateDDLCommands(const ObjectAddress *dependency)
 
 		case OCLASS_ROLE:
 		{
-			return GenerateCreateOrAlterRoleCommand(dependency->objectId);
+			return GenerateCreateOrAlterRoleCommand(dependency->objectId,fetchGrantStatements);
 		}
 
 		case OCLASS_SCHEMA:
@@ -680,15 +680,15 @@ GetDependencyCreateDDLCommands(const ObjectAddress *dependency)
 List *
 GetAllDependencyCreateDDLCommands(const List *dependencies)
 {
-	List *commands = NIL;
+	List *ddlCommands = NIL;
 
 	ObjectAddress *dependency = NULL;
 	foreach_ptr(dependency, dependencies)
 	{
-		commands = list_concat(commands, GetDependencyCreateDDLCommands(dependency));
+		ddlCommands = list_concat(ddlCommands, GetDependencyCreateDDLCommands(dependency,false));
 	}
 
-	return commands;
+	return ddlCommands;
 }
 
 

src/backend/distributed/commands/role.c

@@ -56,6 +56,12 @@
 #include "distributed/version_compat.h"
 #include "distributed/worker_transaction.h"
 
+typedef struct GrantRoleStmts
+{
+    List *adminStmts;
+    List *otherStmts;
+} GrantRoleStmts;
+
 static const char * ExtractEncryptedPassword(Oid roleOid);
 static const char * CreateAlterRoleIfExistsCommand(AlterRoleStmt *stmt);
 static const char * CreateAlterRoleSetIfExistsCommand(AlterRoleSetStmt *stmt);
@@ -66,7 +72,7 @@ static DefElem * makeDefElemInt(char *name, int value);
 static DefElem * makeDefElemBool(char *name, bool value);
 static List * GenerateRoleOptionsList(HeapTuple tuple);
 static List * GenerateGrantRoleStmtsFromOptions(RoleSpec *roleSpec, List *options);
-static List * GenerateGrantRoleStmtsOfRole(Oid roleid);
+static GrantRoleStmts GenerateGrantRoleStmtsOfRole(Oid roleid);
 static List * GenerateSecLabelOnRoleStmts(Oid roleid, char *rolename);
 static void EnsureSequentialModeForRoleDDL(void);
 
@@ -513,8 +519,10 @@ GenerateRoleOptionsList(HeapTuple tuple)
  * the pg_authid table.
  */
 List *
-GenerateCreateOrAlterRoleCommand(Oid roleOid)
+GenerateCreateOrAlterRoleCommand(Oid roleOid, bool fetchGrantStmts)
 {
+
+	elog(NOTICE, "Generating create or alter role command for role %u with fetchGrantStmts %s", roleOid,fetchGrantStmts ? "true" : "false");
 	HeapTuple roleTuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleOid));
 	Form_pg_authid role = ((Form_pg_authid) GETSTRUCT(roleTuple));
 	char *rolename = pstrdup(NameStr(role->rolname));
@@ -563,12 +571,19 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid)
 
 	if (EnableCreateRolePropagation)
 	{
-		List *grantRoleStmts = GenerateGrantRoleStmtsOfRole(roleOid);
+		if(fetchGrantStmts){
+			elog(NOTICE, "Fetching grant statements for role %s", rolename);
+			List *grantRoleStmtList = NIL;
+			GrantRoleStmts grantRoleStmts= GenerateGrantRoleStmtsOfRole(roleOid);
+
+			grantRoleStmtList = list_concat(grantRoleStmts.adminStmts, grantRoleStmts.otherStmts);
+
 			Node *stmt = NULL;
-		foreach_ptr(stmt, grantRoleStmts)
+			foreach_ptr(stmt, grantRoleStmtList)
 			{
 				completeRoleList = lappend(completeRoleList, DeparseTreeNode(stmt));
 			}
+		}
 
 		/*
 		 * append SECURITY LABEL ON ROLE commands for this specific user
@@ -578,7 +593,7 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid)
 		 * SecLabel stmts to be run in the new node.
 		 */
 		List *secLabelOnRoleStmts = GenerateSecLabelOnRoleStmts(roleOid, rolename);
-		stmt = NULL;
+		Node *stmt = NULL;
 		foreach_ptr(stmt, secLabelOnRoleStmts)
 		{
 			completeRoleList = lappend(completeRoleList, DeparseTreeNode(stmt));
@@ -868,12 +883,14 @@ GenerateGrantRoleStmtsFromOptions(RoleSpec *roleSpec, List *options)
  * GenerateGrantRoleStmtsOfRole generates the GrantRoleStmts for the memberships
  * of the role whose oid is roleid.
  */
-static List *
+static GrantRoleStmts
 GenerateGrantRoleStmtsOfRole(Oid roleid)
 {
     Relation pgAuthMembers = table_open(AuthMemRelationId, AccessShareLock);
     HeapTuple tuple = NULL;
-	List *stmts = NIL;
+
+    List *adminStmts = NIL;
+    List *otherStmts = NIL;
 
     ScanKeyData skey[1];
 
@@ -916,7 +933,6 @@ GenerateGrantRoleStmtsOfRole(Oid roleid)
         grantRoleStmt->grantor = grantorRole;
 
 #if PG_VERSION_NUM >= PG_VERSION_16
-
         /* inherit option is always included */
         DefElem *inherit_opt;
         if (membership->inherit_option)
@@ -945,14 +961,73 @@ GenerateGrantRoleStmtsOfRole(Oid roleid)
 #else
         grantRoleStmt->admin_opt = membership->admin_option;
 #endif
-
+        if (membership->admin_option)
-		stmts = lappend(stmts, grantRoleStmt);
+        {
+            adminStmts = lappend(adminStmts, grantRoleStmt);
+        }
+        else
+        {
+            otherStmts = lappend(otherStmts, grantRoleStmt);
+        }
     }
 
     systable_endscan(scan);
     table_close(pgAuthMembers, AccessShareLock);
 
-	return stmts;
+    GrantRoleStmts result;
+    result.adminStmts = adminStmts;
+    result.otherStmts = otherStmts;
+
+    return result;
+}
+
+
+/**/
+List * GenerateGrantRoleStmts()
+{
+    Relation pgAuthMembers = table_open(AuthMemRelationId, AccessShareLock);
+    HeapTuple tuple = NULL;
+    List *adminStmts = NIL;
+    List *otherStmts = NIL;
+
+    SysScanDesc scan = systable_beginscan(pgAuthMembers, InvalidOid, false,
+                                          NULL, 0, NULL);
+
+    while (HeapTupleIsValid(tuple = systable_getnext(scan)))
+    {
+        Form_pg_auth_members membership = (Form_pg_auth_members) GETSTRUCT(tuple);
+
+        ObjectAddress *roleAddress = palloc0(sizeof(ObjectAddress));
+        ObjectAddressSet(*roleAddress, AuthIdRelationId, membership->grantor);
+		elog(NOTICE, "Role name: %s", GetUserNameFromId(membership->roleid, true));
+		elog(NOTICE, "Member name: %s", GetUserNameFromId(membership->member, true));
+        if (!IsAnyObjectDistributed(list_make1(roleAddress)))
+        {
+            /* we only need to propagate the grant if the grantor is distributed */
+            continue;
+        }
+
+		//log role name
+		elog(NOTICE, "Role name fetched: %s", GetUserNameFromId(membership->roleid, true));
+		elog(NOTICE, "Member name fetched: %s", GetUserNameFromId(membership->member, true));
+		GrantRoleStmts grantRoleStmts = GenerateGrantRoleStmtsOfRole(membership->roleid);
+		adminStmts = list_concat(adminStmts, grantRoleStmts.adminStmts);
+		otherStmts = list_concat(otherStmts, grantRoleStmts.otherStmts);
+    }
+
+    systable_endscan(scan);
+    table_close(pgAuthMembers, AccessShareLock);
+
+    List *allGrantStatements =  list_concat(adminStmts, otherStmts);
+
+	Node *stmt = NULL;
+	List *grantStatements = NIL;
+	foreach_ptr(stmt, allGrantStatements)
+	{
+		grantStatements = lappend(grantStatements, DeparseTreeNode(stmt));
+	}
+
+	return grantStatements;
 }
 
 

src/backend/distributed/metadata/dependency.c

@@ -1828,13 +1828,6 @@ ExpandRolesToGroups(Oid roleid)
 		ObjectAddressSet(definition->data.address, AuthIdRelationId, membership->roleid);
 
 		roles = lappend(roles, definition);
-
-		DependencyDefinition *definition1 = palloc0(sizeof(DependencyDefinition));
-		definition1->mode = DependencyObjectAddress;
-		ObjectAddressSet(definition1->data.address, AuthIdRelationId,
-						 membership->grantor);
-
-		roles = lappend(roles, definition1);
 	}
 
 	systable_endscan(scanDescriptor);

src/backend/distributed/metadata/metadata_sync.c

@@ -5027,6 +5027,11 @@ SendDependencyCreationCommands(MetadataSyncContext *context)
 		List *ddlCommands = GetAllDependencyCreateDDLCommands(list_make1(dependency));
 		SendOrCollectCommandListToActivatedNodes(context, ddlCommands);
 	}
+
+	List *grantRoleCommands = GenerateGrantRoleStmts();
+	SendOrCollectCommandListToActivatedNodes(context, grantRoleCommands);
+
+
 	MemoryContextSwitchTo(oldContext);
 
 	if (!MetadataSyncCollectsCommands(context))

src/include/distributed/commands.h

@@ -515,7 +515,7 @@ extern List * PreprocessDropRoleStmt(Node *stmt, const char *queryString,
 extern List * PreprocessGrantRoleStmt(Node *stmt, const char *queryString,
 									  ProcessUtilityContext processUtilityContext);
 extern List * PostprocessGrantRoleStmt(Node *stmt, const char *queryString);
-extern List * GenerateCreateOrAlterRoleCommand(Oid roleOid);
+extern List * GenerateCreateOrAlterRoleCommand(Oid roleOid,bool fetchGrantStatements);
 extern List * CreateRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool
 										  isPostprocess);
 
@@ -524,6 +524,7 @@ extern List * RenameRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool
 
 extern void UnmarkRolesDistributed(List *roles);
 extern List * FilterDistributedRoles(List *roles);
+extern List * GenerateGrantRoleStmts(void);
 
 /* schema.c - forward declarations */
 extern List * PostprocessCreateSchemaStmt(Node *node, const char *queryString);

src/test/regress/sql/granted_by_support.sql

@@ -21,6 +21,12 @@ grant role3 to role1 with admin option GRANTED BY role4;
 grant "role5'_test" to role1 with admin option;
 grant "role5'_test" to role3 with admin option GRANTED BY role1;
 
+SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option
+        FROM pg_auth_members
+        WHERE member::regrole::text in
+            ('role1','role2','role3','role4','"role5''_test"')
+        order by member::regrole::text, roleid::regrole::text;
+
 
 select result FROM run_command_on_all_nodes(
     $$
@@ -35,8 +41,14 @@ select result FROM run_command_on_all_nodes(
     $$
 );
 
+set citus.log_remote_commands = on;
+--set citus.grep_remote_commands = '%GRANT%';
+
 select 1 from citus_add_node ('localhost',:worker_2_port);
 
+reset citus.log_remote_commands;
+reset citus.grep_remote_commands;
+
 --clean all resources
 drop role role1,role2,role3,role4,"role5'_test";
 
@@ -52,3 +64,24 @@ select result FROM run_command_on_all_nodes(
     ) t
     $$
 );
+
+
+GRANT role2 TO  role3 WITH INHERIT TRUE, ADMIN OPTION GRANTED BY role1;;
+GRANT role2 TO  role3 WITH INHERIT TRUE, ADMIN OPTION GRANTED BY role1;;
+GRANT role3 TO  role4 WITH INHERIT TRUE, ADMIN OPTION GRANTED BY postgres;; x
+GRANT role3 TO  role4 WITH INHERIT TRUE, ADMIN OPTION GRANTED BY postgres;;
+GRANT role2 TO  role3 WITH INHERIT TRUE, ADMIN OPTION GRANTED BY role1;; x
+GRANT role4 TO  "role5'_test" WITH INHERIT TRUE, ADMIN OPTION GRANTED BY postgres;; x
+GRANT role2 TO  "role5'_test" WITH INHERIT TRUE GRANTED BY role3;; x
+GRANT role3 TO  "role5'_test" WITH INHERIT TRUE GRANTED BY role4 ; x
+
+
+ "role5'_test" | role2         | role3         | f x
+ "role5'_test" | role3         | role4         | f x
+ "role5'_test" | role4         | postgres      | t x
+ role1         | "role5'_test" | postgres      | t
+ role1         | role2         | postgres      | t
+ role1         | role3         | role4         | t
+ role1         | role4         | "role5'_test" | t
+ role3         | role2         | role1         | t x
+ role4         | role3         | postgres      | t x