From 432b910207f41fab3012c79f12e9feb4c7cf3d74 Mon Sep 17 00:00:00 2001 From: shalinilohia50 <46928246+shalinilohia50@users.noreply.github.com> Date: Tue, 20 Aug 2024 12:38:52 +0530 Subject: [PATCH] Restrict dropping user created for another database (#2863) Earlier, a user was able to drop user/role that belonged to another database. With this commit, a user can only drop the role/user that belongs to the same database with sufficient privileges. Issues Resolved Task: BABEL-5173 Signed-off-by: Shalini Lohia lshalini@amazon.com --- contrib/babelfishpg_tsql/src/pl_handler.c | 7 +- .../JDBC/expected/restrict_drop_user_role.out | 115 +++++++++++++++++- test/JDBC/input/restrict_drop_user_role.mix | 78 +++++++++++- 3 files changed, 196 insertions(+), 4 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 9e6988d2e6..e7b09bb63b 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3290,14 +3290,18 @@ bbf_ProcessUtility(PlannedStmt *pstmt, int role_oid; int rolename_len; bool is_tsql_db_principal = false; + bool is_psql_db_principal = false; + Oid dbowner; user_name = get_physical_user_name(db_name, rolspec->rolename, false); db_owner_name = get_db_owner_name(db_name); + dbowner = get_role_oid(db_owner_name, false); role_oid = get_role_oid(user_name, true); rolename_len = strlen(rolspec->rolename); is_tsql_db_principal = OidIsValid(role_oid) && ((drop_user && is_user(role_oid)) || (drop_role && is_role(role_oid))); + is_psql_db_principal = OidIsValid(role_oid) && !is_tsql_db_principal; /* If user is dbo or role is db_owner, restrict dropping */ if ((drop_user && rolename_len == 3 && strncmp(rolspec->rolename, "dbo", 3) == 0) || @@ -3311,7 +3315,8 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * must be database owner to drop user/role */ if ((!stmt->missing_ok && !is_tsql_db_principal) || - !is_member_of_role(GetUserId(), get_role_oid(db_owner_name, false))) + !is_member_of_role(GetUserId(), dbowner) || + (is_tsql_db_principal && !is_member_of_role(dbowner, role_oid)) || is_psql_db_principal) 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_type, rolspec->rolename))); diff --git a/test/JDBC/expected/restrict_drop_user_role.out b/test/JDBC/expected/restrict_drop_user_role.out index fa3bcc0426..f3d8f79a97 100644 --- a/test/JDBC/expected/restrict_drop_user_role.out +++ b/test/JDBC/expected/restrict_drop_user_role.out @@ -211,11 +211,25 @@ drop user if exists fake_user; go -- both of the following statements should be executed successfully since user/role exists +drop role if exists no_priv_user3; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the role 'no_priv_user3', because it does not exist or you do not have permission.)~~ + + drop user if exists no_priv_user3; go drop user if exists no_priv_role3; go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'no_priv_role3', because it does not exist or you do not have permission.)~~ + + +drop role if exists no_priv_role3; +go select count(*) from sys.database_principals where name like '%no_priv_%3'; go @@ -251,14 +265,14 @@ drop user if exists pguser1; go ~~ERROR (Code: 33557097)~~ -~~ERROR (Message: permission denied to drop role)~~ +~~ERROR (Message: Cannot drop the user 'pguser1', because it does not exist or you do not have permission.)~~ drop role if exists pguser1; go ~~ERROR (Code: 33557097)~~ -~~ERROR (Message: permission denied to drop role)~~ +~~ERROR (Message: Cannot drop the role 'pguser1', because it does not exist or you do not have permission.)~~ -- psql @@ -300,3 +314,100 @@ go drop database restrict_user_db1 go + +-- psql +-- 3.5 - Try dropping user created for another database +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'multi-db'; +GO +SELECT pg_reload_conf(); +GO +~~START~~ +bool +t +~~END~~ + +-- Wait to sync with another session +SELECT pg_sleep(1); +GO +~~START~~ +void + +~~END~~ + + +-- tsql +create database restrict_user_db1; +go + +create database restrict_user_db1_restrict_user_db1; +go + +create login restrict_user_l1 with password = '12345678' +go + +use restrict_user_db1 +go + +create user restrict_user_db1_user1 for login restrict_user_l1; +go + +use restrict_user_db1_restrict_user_db1 +go + +drop user user1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'user1', because it does not exist or you do not have permission.)~~ + + +drop user if exists user1; +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot drop the user 'user1', because it does not exist or you do not have permission.)~~ + + +use master +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'restrict_user_l1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go +~~START~~ +bool +~~END~~ + + +-- tsql +use restrict_user_db1 +go + +drop user restrict_user_db1_user1; +go + +use master +go + +drop login restrict_user_l1; +go + +drop database restrict_user_db1; +go + +drop database restrict_user_db1_restrict_user_db1; +go + +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'single-db'; +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO +~~START~~ +void + +~~END~~ + diff --git a/test/JDBC/input/restrict_drop_user_role.mix b/test/JDBC/input/restrict_drop_user_role.mix index 6bac815143..a013061551 100644 --- a/test/JDBC/input/restrict_drop_user_role.mix +++ b/test/JDBC/input/restrict_drop_user_role.mix @@ -143,12 +143,18 @@ drop user if exists fake_user; go -- both of the following statements should be executed successfully since user/role exists +drop role if exists no_priv_user3; +go + drop user if exists no_priv_user3; go drop user if exists no_priv_role3; go +drop role if exists no_priv_role3; +go + select count(*) from sys.database_principals where name like '%no_priv_%3'; go @@ -200,4 +206,74 @@ drop login no_priv_login3 go drop database restrict_user_db1 -go \ No newline at end of file +go + +-- 3.5 - Try dropping user created for another database +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'multi-db'; +GO +SELECT pg_reload_conf(); +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO + +-- tsql +create database restrict_user_db1; +go + +create database restrict_user_db1_restrict_user_db1; +go + +create login restrict_user_l1 with password = '12345678' +go + +use restrict_user_db1 +go + +create user restrict_user_db1_user1 for login restrict_user_l1; +go + +use restrict_user_db1_restrict_user_db1 +go + +drop user user1; +go + +drop user if exists user1; +go + +use master +go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'restrict_user_l1' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go + +-- tsql +use restrict_user_db1 +go + +drop user restrict_user_db1_user1; +go + +use master +go + +drop login restrict_user_l1; +go + +drop database restrict_user_db1; +go + +drop database restrict_user_db1_restrict_user_db1; +go + +-- psql +ALTER SYSTEM SET babelfishpg_tsql.migration_mode = 'single-db'; +GO +-- Wait to sync with another session +SELECT pg_sleep(1); +GO \ No newline at end of file