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

From f3f8376f5fca815c8e1f75b74c781a27d5904124 Mon Sep 17 00:00:00 2001
From: Ashutosh Sharma <ashu.coek88@gmail.com>
Date: Tue, 18 Feb 2025 09:11:50 +0000
Subject: [PATCH] Add role dependency check before dropping the role

Before dropping a role, check for any dependencies. If dependencies
exist, raise an error to prevent the role from being dropped, and
include a list of dependent roles in the error message.

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               | 113 ++++++++++++++++++++++
 src/test/regress/expected/create_role.out |  29 ++++++
 src/test/regress/sql/create_role.sql      |  15 +++
 3 files changed, 157 insertions(+)

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 0db174e6f10..7d86f3373ad 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -124,6 +124,116 @@ have_createrole_privilege(void)
 	return has_createrole_privilege(GetUserId());
 }
 
+/*
+ * check_drop_role_dependency
+ *
+ * Check if there are any dependencies related to the role that is being
+ * dropped.
+ *
+ * This function scans the pg_auth_members table to find roles that are either
+ * admins of the role being dropped, or have the role being dropped as an admin
+ * member. If both conditions are true, it indicates that there are dependencies
+ * on the role being dropped. In such cases, an error is raised 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 *cell;
+		StringInfoData dependent_roles_list;
+
+		/* Initialize StringInfo to build the list string */
+		initStringInfo(&dependent_roles_list);
+
+		foreach(cell, admin_members_of_drop_role)
+		{
+			Oid role_oid = lfirst_oid(cell);
+
+			if (dependent_roles_list.len > 0)
+				appendStringInfo(&dependent_roles_list, ", ");
+
+			appendStringInfo(&dependent_roles_list, "%s",
+							 GetUserNameFromId(role_oid, false));
+		}
+
+		ereport(ERROR,
+				(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+				 errmsg("role \"%s\" cannot be dropped because some role(s) depend on it",
+					   GetUserNameFromId(roleid, false)),
+				 errdetail("Dependent role(s): %s", dependent_roles_list.data)));
+
+		pfree(dependent_roles_list.data);
+	}
+}
 
 /*
  * CREATE ROLE
@@ -1178,6 +1288,9 @@ DropRole(DropRoleStmt *stmt)
 					 errdetail("Only roles with the %s attribute and the %s option on role \"%s\" may drop this role.",
 							   "CREATEROLE", "ADMIN", NameStr(roleform->rolname))));
 
+		/* Check for role dependencies before dropping the role. */
+		check_drop_role_dependency(pg_auth_members_rel, roleid);
+
 		/* DROP hook for the role being removed */
 		InvokeObjectDropHook(AuthIdRelationId, roleid, 0);
 
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 46d4f9efe99..4f051821991 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -225,6 +225,34 @@ 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 role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_createrole;
+ERROR:  role "regress_createrole" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_login;
+ERROR:  role "regress_login" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_inherit;
+ERROR:  role "regress_inherit" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_connection_limit;
+ERROR:  role "regress_connection_limit" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_encrypted_password;
+ERROR:  role "regress_encrypted_password" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+DROP ROLE regress_password_null;
+ERROR:  role "regress_password_null" cannot be dropped because some role(s) depend on it
+DETAIL:  Dependent role(s): regress_role_admin
+-- 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 +263,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