Skip to content

Commit 1006d9a

Browse files
authored
Restrict DROP USER/ROLE from non-dbo user (#2859) (#2865) (#2883)
Earlier, any user was able to drop user/role, irrespective of whether that user has required privileges or not. With this commit, Only dbo and members of db_owner will have the permission to drop user/role. Additionally, this restricts dropping internal database principal such as dbo and db_owner, it restricts dropping non-Babelfish roles from TDS endpoint. Task: BABEL-5173 Authored-by: ANJU BHARTI <abanju@amazon.com>
1 parent 21c072a commit 1006d9a

File tree

3 files changed

+734
-1
lines changed

3 files changed

+734
-1
lines changed

contrib/babelfishpg_tsql/src/pl_handler.c

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2808,15 +2808,49 @@ bbf_ProcessUtility(PlannedStmt *pstmt,
28082808
{
28092809
RoleSpec *rolspec = lfirst(item);
28102810
char *user_name;
2811+
const char *db_principal_type = drop_user ? "user" : "role";
2812+
const char *db_owner_name;
2813+
int role_oid;
2814+
int rolename_len;
2815+
bool is_tsql_db_principal = false;
2816+
bool is_psql_db_principal = false;
2817+
Oid dbowner;
28112818

28122819
user_name = get_physical_user_name(db_name, rolspec->rolename);
28132820

2821+
db_owner_name = get_db_owner_name(db_name);
2822+
dbowner = get_role_oid(db_owner_name, false);
2823+
role_oid = get_role_oid(user_name, true);
2824+
rolename_len = strlen(rolspec->rolename);
2825+
is_tsql_db_principal = OidIsValid(role_oid) &&
2826+
((drop_user && is_user(role_oid)) ||
2827+
(drop_role && is_role(role_oid)));
2828+
is_psql_db_principal = OidIsValid(role_oid) && !is_tsql_db_principal;
2829+
2830+
/* If user is dbo or role is db_owner, restrict dropping */
2831+
if ((drop_user && rolename_len == 3 && strncmp(rolspec->rolename, "dbo", 3) == 0) ||
2832+
(drop_role && rolename_len == 8 && strncmp(rolspec->rolename, "db_owner", 8) == 0))
2833+
ereport(ERROR,
2834+
(errcode(ERRCODE_CHECK_VIOLATION),
2835+
errmsg("Cannot drop the %s '%s'.", db_principal_type, rolspec->rolename)));
2836+
2837+
/*
2838+
* Check for current_user's privileges
2839+
* must be database owner to drop user/role
2840+
*/
2841+
if ((!stmt->missing_ok && !is_tsql_db_principal) ||
2842+
!is_member_of_role(GetUserId(), dbowner) ||
2843+
(is_tsql_db_principal && !is_member_of_role(dbowner, role_oid)) || is_psql_db_principal)
2844+
ereport(ERROR,
2845+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
2846+
errmsg("Cannot drop the %s '%s', because it does not exist or you do not have permission.", db_principal_type, rolspec->rolename)));
2847+
28142848
/*
28152849
* If a role has members, do not drop it.
28162850
* Note that here we don't handle invalid
28172851
* roles.
28182852
*/
2819-
if (drop_role && !is_empty_role(get_role_oid(user_name, true)))
2853+
if (drop_role && !is_empty_role(role_oid))
28202854
ereport(ERROR,
28212855
(errcode(ERRCODE_CHECK_VIOLATION),
28222856
errmsg("The role has members. It must be empty before it can be dropped.")));

0 commit comments

Comments
 (0)