v2-0001-Add-role-dependency-check-before-dropping-the-role.patch

From e1848dddef48d0e33d45ee6966d529a4c2cf52bb Mon Sep 17 00:00:00 2001
From: Ashutosh Sharma <ashu.coek88@gmail.com>
Date: Thu, 6 Mar 2025 10:19:06 +0000
Subject: [PATCH] Add role dependency check before dropping the role

Before dropping a role, check for any existing dependencies. If
dependencies are found, raise an error to prevent the role from being
dropped, and and provide a detailed error message outlining the role
dependencies.

Additionally, update the existing test case in create_role.sql to
verify the new functionality and ensure proper handling of role
dependencies.
---
 src/backend/commands/user.c               | 131 ++++++++++++++++++++++
 src/test/regress/expected/create_role.out |  32 ++++++
 src/test/regress/sql/create_role.sql      |  15 +++
 3 files changed, 178 insertions(+)

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8ae510c623b..db3207428c2 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -125,6 +125,131 @@ have_createrole_privilege(void)
 	return has_createrole_privilege(GetUserId());
 }
 
+/*
+ * check_drop_role_dependency
+ *
+ * Check if there are any dependencies associated with the role being dropped.
+ *
+ * This function scans the pg_auth_members table to find: 1) roles that are
+ * admins of the role being dropped, and 2) roles where the role being dropped
+ * is an admin. If both conditions are met, it means the roles that are admins
+ * of the role being dropped rely on it for admin privileges on roles where the
+ * role being dropped is an admin.
+ *
+ * For example, if userA creates userB, and userB creates userC, both userB and
+ * userA would have admin privileges on userC. UserB has admin privileges on
+ * userC because it created userC, while userA inherits those admin privileges
+ * from userB. This means userA relies on userB for admin privileges on userC.
+ * Therefore, if userB is dropped, userA will lose its admin privileges on
+ * userC.
+ *
+ * If such a dependency exists, this function raises an error to prevent the
+ * role from being dropped.
+ *
+ * pg_auth_members_rel: relation object representing the pg_auth_members system
+ * catalog.
+ * roleid: Oid of the role that is being dropped.
+ */
+static void
+check_drop_role_dependency(Relation pg_auth_members_rel, Oid roleid)
+{
+	ScanKeyData	scankey;
+	SysScanDesc	sscan;
+	HeapTuple	tmp_tuple;
+	List	   *admin_members_of_drop_role = NULL;  			/* List of roles that are admin members of the role to drop */
+	List	   *roles_with_drop_role_as_admin_member = NULL;	/* List of roles where the role to drop is an admin member */
+
+	ScanKeyInit(&scankey,
+				Anum_pg_auth_members_roleid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(roleid));
+
+	sscan = systable_beginscan(pg_auth_members_rel,
+							   AuthMemRoleMemIndexId,
+							   true, NULL, 1, &scankey);
+
+	while (HeapTupleIsValid(tmp_tuple = systable_getnext(sscan)))
+	{
+		Form_pg_auth_members authmem_form;
+		authmem_form = (Form_pg_auth_members) GETSTRUCT(tmp_tuple);
+
+		if (authmem_form->admin_option)
+		{
+			if (!admin_members_of_drop_role)
+				admin_members_of_drop_role =
+						list_make1_oid(authmem_form->member);
+			else
+				admin_members_of_drop_role =
+						lappend_oid(admin_members_of_drop_role,
+									authmem_form->member);
+		}
+	}
+
+	systable_endscan(sscan);
+
+	ScanKeyInit(&scankey,
+				Anum_pg_auth_members_member,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(roleid));
+
+	sscan = systable_beginscan(pg_auth_members_rel,
+							   AuthMemRoleMemIndexId,
+							   true, NULL, 1, &scankey);
+
+	while (HeapTupleIsValid(tmp_tuple = systable_getnext(sscan)))
+	{
+		Form_pg_auth_members authmem_form;
+		authmem_form = (Form_pg_auth_members) GETSTRUCT(tmp_tuple);
+
+		if (authmem_form->admin_option)
+		{
+			if (!roles_with_drop_role_as_admin_member)
+				roles_with_drop_role_as_admin_member =
+							list_make1_oid(authmem_form->roleid);
+			else
+				roles_with_drop_role_as_admin_member =
+							lappend_oid(roles_with_drop_role_as_admin_member,
+										authmem_form->roleid);
+		}
+	}
+
+	systable_endscan(sscan);
+
+	/* If there are dependencies, raise an error */
+	if (admin_members_of_drop_role && roles_with_drop_role_as_admin_member)
+	{
+		ListCell *dependent_role;
+		ListCell *referenced_role;
+		StringInfoData detail_log;
+
+		/* Initialize StringInfo to build the detailed error log message */
+		initStringInfo(&detail_log);
+
+		foreach(dependent_role, admin_members_of_drop_role)
+		{
+			Oid dependent_role_oid = lfirst_oid(dependent_role);
+
+			foreach(referenced_role, roles_with_drop_role_as_admin_member)
+			{
+				Oid referenced_role_oid = lfirst_oid(referenced_role);
+
+				if (detail_log.len > 0)
+					appendStringInfoChar(&detail_log, '\n');
+
+				appendStringInfo(&detail_log, "role %s inherits ADMIN privileges on role %s through role %s",
+								 GetUserNameFromId(dependent_role_oid, false),
+								 GetUserNameFromId(referenced_role_oid, false),
+								 GetUserNameFromId(roleid, false));
+			}
+		}
+
+		ereport(ERROR,
+				(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+				 errmsg("role \"%s\" cannot be dropped because some objects depend on it",
+					   GetUserNameFromId(roleid, false)),
+				 errdetail("%s", detail_log.data)));
+	}
+}
 
 /*
  * CREATE ROLE
@@ -1190,6 +1315,12 @@ DropRole(DropRoleStmt *stmt)
 		 */
 		LockSharedObject(AuthIdRelationId, roleid, 0, AccessExclusiveLock);
 
+		/*
+		 * Check if there are any existing dependencies on the role before dropping
+		 * it.
+		 */
+		check_drop_role_dependency(pg_auth_members_rel, roleid);
+
 		/*
 		 * If there is a pg_auth_members entry that has one of the roles to be
 		 * dropped as the roleid or member, it should be silently removed, but
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 46d4f9efe99..c16fabd2a95 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -225,6 +225,37 @@ REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
 DROP ROLE regress_replication_bypassrls;
 DROP ROLE regress_replication;
 DROP ROLE regress_bypassrls;
+-- should fail, as some roles have dependency on these roles
+DROP ROLE regress_createdb;
+ERROR:  role "regress_createdb" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_createdb
+DROP ROLE regress_createrole;
+ERROR:  role "regress_createrole" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_createrole
+role regress_role_admin inherits ADMIN privileges on role regress_rolecreator through role regress_createrole
+role regress_role_admin inherits ADMIN privileges on role regress_tenant through role regress_createrole
+role regress_role_admin inherits ADMIN privileges on role regress_tenant2 through role regress_createrole
+DROP ROLE regress_login;
+ERROR:  role "regress_login" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_login
+DROP ROLE regress_inherit;
+ERROR:  role "regress_inherit" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_inherit
+DROP ROLE regress_connection_limit;
+ERROR:  role "regress_connection_limit" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_connection_limit
+DROP ROLE regress_encrypted_password;
+ERROR:  role "regress_encrypted_password" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_encrypted_password
+DROP ROLE regress_password_null;
+ERROR:  role "regress_password_null" cannot be dropped because some objects depend on it
+DETAIL:  role regress_role_admin inherits ADMIN privileges on role regress_adminroles through role regress_password_null
+-- remove any dependencies associated with the role before dropping it
+-- should pass
+RESET SESSION AUTHORIZATION;
+REVOKE regress_createdb, regress_createrole, regress_login,
+	   regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null
+FROM regress_role_admin;
 DROP ROLE regress_createdb;
 DROP ROLE regress_createrole;
 DROP ROLE regress_login;
@@ -235,6 +266,7 @@ DROP ROLE regress_password_null;
 DROP ROLE regress_noiseword;
 DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
+SET SESSION AUTHORIZATION regress_role_admin;
 -- fail, cannot drop ourself, nor superusers or roles we lack ADMIN for
 DROP ROLE regress_role_super;
 ERROR:  permission denied to drop role
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 4491a28a8ae..312495c7df8 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -183,6 +183,20 @@ REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
 DROP ROLE regress_replication_bypassrls;
 DROP ROLE regress_replication;
 DROP ROLE regress_bypassrls;
+-- should fail, as some roles have dependency on these roles
+DROP ROLE regress_createdb;
+DROP ROLE regress_createrole;
+DROP ROLE regress_login;
+DROP ROLE regress_inherit;
+DROP ROLE regress_connection_limit;
+DROP ROLE regress_encrypted_password;
+DROP ROLE regress_password_null;
+-- remove any dependencies associated with the role before dropping it
+-- should pass
+RESET SESSION AUTHORIZATION;
+REVOKE regress_createdb, regress_createrole, regress_login,
+	   regress_inherit, regress_connection_limit, regress_encrypted_password, regress_password_null
+FROM regress_role_admin;
 DROP ROLE regress_createdb;
 DROP ROLE regress_createrole;
 DROP ROLE regress_login;
@@ -193,6 +207,7 @@ DROP ROLE regress_password_null;
 DROP ROLE regress_noiseword;
 DROP ROLE regress_inroles;
 DROP ROLE regress_adminroles;
+SET SESSION AUTHORIZATION regress_role_admin;
 
 -- fail, cannot drop ourself, nor superusers or roles we lack ADMIN for
 DROP ROLE regress_role_super;
-- 
2.17.1