Restrict dropping of dbo and db_owner

Signed-off-by: ANJU BHARTI <[email protected]>
babelfish-for-postgresql · Aug 15, 2024 · ba94f77 · ba94f77
1 parent b946273
commit ba94f77
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 15 deletions.
diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c
@@ -3285,9 +3285,21 @@ bbf_ProcessUtility(PlannedStmt *pstmt,
 								{
 									RoleSpec   *rolspec = lfirst(item);
 									char	   *user_name;
+									char       *db_principal;
 
 									user_name = get_physical_user_name(db_name, rolspec->rolename, false);
-
+									role_name = rolspec->rolename;
+									if (drop_user)
+										db_principal = "user";
+									else
+										db_principal = "role";
+
+									/* If user is dbo or role is db_owner, restrict dropping */
+									if ((drop_user && strncmp(role_name, "dbo", 3) == 0) || (drop_role && strncmp(role_name, "db_owner", 8) == 0))
+										ereport(ERROR,
+												(errcode(ERRCODE_CHECK_VIOLATION),
+												errmsg("Cannot drop the %s '%s'.", db_principal, role_name)));
+
 									/*
 									 * If a role has members, do not drop it.
 									 * Note that here we don't handle invalid
@@ -3431,17 +3443,24 @@ bbf_ProcessUtility(PlannedStmt *pstmt,
 									 errmsg("Could not drop login '%s' as the user is currently logged in.", role_name)));
 					}
 					/* If user/role, check for current_user's privileges */
-					else if (drop_user || drop_role)
-					{
-						const char *db_owner_name;
+						else if (drop_user || drop_role)
+						{
+							const char *db_owner_name;
+							char       *db_principal;
+							int			role_oid = get_role_oid(role_name, true);
 
-						/* must be database owner to drop user/role*/
-						db_owner_name = get_db_owner_name(get_cur_db_name());
-						if (!has_privs_of_role(GetUserId(),get_role_oid(db_owner_name, false)))
-							ereport(ERROR,
-									(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-									 errmsg("User does not have permission to perform this action.")));
-					}
+							if (drop_user)
+								db_principal = "user";
+							else
+								db_principal = "role";
+
+							/* must be database owner to drop user/role */
+							db_owner_name = get_db_owner_name(get_cur_db_name());
+							if (!OidIsValid(role_oid) || !is_member_of_role(GetUserId(), get_role_oid(db_owner_name, false)))
+								ereport(ERROR,
+										(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+										errmsg("Cannot drop the %s '%s', because it does not exist or you do not have permission.", db_principal, role_name)));
+						}
 
 					/*
 					 * We have performed all the permissions checks.

diff --git a/test/JDBC/expected/restrict_drop_user_role.out b/test/JDBC/expected/restrict_drop_user_role.out
@@ -41,14 +41,14 @@ drop role dont_drop_role
 go
 ~~ERROR (Code: 33557097)~~
 
-~~ERROR (Message: User does not have permission to perform this action.)~~
+~~ERROR (Message: Cannot drop the role 'master_dont_drop_role', because it does not exist or you do not have permission.)~~
 
 
 drop user no_priv_user2
 go
 ~~ERROR (Code: 33557097)~~
 
-~~ERROR (Message: User does not have permission to perform this action.)~~
+~~ERROR (Message: Cannot drop the user 'master_no_priv_user2', because it does not exist or you do not have permission.)~~
 
 
 -- tsql
@@ -81,14 +81,14 @@ drop role dont_drop_role
 go
 ~~ERROR (Code: 33557097)~~
 
-~~ERROR (Message: User does not have permission to perform this action.)~~
+~~ERROR (Message: Cannot drop the role 'master_dont_drop_role', because it does not exist or you do not have permission.)~~
 
 
 drop user no_priv_user2
 go
 ~~ERROR (Code: 33557097)~~
 
-~~ERROR (Message: User does not have permission to perform this action.)~~
+~~ERROR (Message: Cannot drop the user 'master_no_priv_user2', because it does not exist or you do not have permission.)~~
 
 
 -- tsql
@@ -114,6 +114,39 @@ go
 drop user no_priv_user1
 go
 
+-- error
+-- try to drop non existing role/user
+drop role fake_role
+go
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: Cannot drop the role 'restrict_user_db1_fake_role', because it does not exist or you do not have permission.)~~
+
+
+drop user fake_user
+go
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: Cannot drop the user 'restrict_user_db1_fake_user', because it does not exist or you do not have permission.)~~
+
+
+-- should deny
+-- try to drop dbo user, db_owner role
+drop role db_owner
+go
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: Cannot drop the role 'db_owner'.)~~
+
+
+drop user dbo
+go
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: Cannot drop the user 'dbo'.)~~
+
+
+
 create role dont_drop_role
 go
 

diff --git a/test/JDBC/input/restrict_drop_user_role.mix b/test/JDBC/input/restrict_drop_user_role.mix
@@ -78,6 +78,23 @@ go
 drop user no_priv_user1
 go
 
+-- error
+-- try to drop non existing role/user
+drop role fake_role
+go
+
+drop user fake_user
+go
+
+-- should deny
+-- try to drop dbo user, db_owner role
+drop role db_owner
+go
+
+drop user dbo
+go
+
+
 create role dont_drop_role
 go