From b2a459edfe645747744402f23de041e9c0a3cd93 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 26 Nov 2021 14:02:01 +0100 Subject: [PATCH] Fix GRANTED BY support in REVOKE ROLE statements Commit 6aaaa76bb added support for the GRANTED BY clause in GRANT and REVOKE statements, but missed adding support for checking the role in the REVOKE ROLE case. Fix by checking that the parsed role matches the CURRENT_ROLE/CURRENT_USER requirement, and also add some tests for it. Backpatch to v14 where GRANTED BY support was introduced. Discussion: https://postgr.es/m/B7F6699A-A984-4943-B9BF-CEB84C003527@yesql.se Backpatch-through: 14 --- src/backend/commands/user.c | 11 +++++++++++ src/backend/parser/gram.y | 2 ++ src/test/regress/expected/privileges.out | 9 +++++++++ src/test/regress/sql/privileges.sql | 8 ++++++++ 4 files changed, 30 insertions(+) diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index aa69821be4..c8c0dd0dd5 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1259,7 +1259,18 @@ GrantRole(GrantRoleStmt *stmt) ListCell *item; if (stmt->grantor) + { grantor = get_rolespec_oid(stmt->grantor, false); + + /* + * Currently, this clause is only for SQL compatibility, not very + * interesting otherwise. + */ + if (grantor != GetUserId()) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("grantor must be current user"))); + } else grantor = GetUserId(); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index a6d0cefa6b..86ce33bd97 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -7196,6 +7196,7 @@ RevokeRoleStmt: n->admin_opt = false; n->granted_roles = $2; n->grantee_roles = $4; + n->grantor = $5; n->behavior = $6; $$ = (Node*)n; } @@ -7206,6 +7207,7 @@ RevokeRoleStmt: n->admin_opt = true; n->granted_roles = $5; n->grantee_roles = $7; + n->grantor = $8; n->behavior = $9; $$ = (Node*)n; } diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 9b91865dcc..1d7ce4d0ea 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -29,6 +29,7 @@ CREATE USER regress_priv_user5; -- duplicate ERROR: role "regress_priv_user5" already exists CREATE USER regress_priv_user6; CREATE USER regress_priv_user7; +CREATE ROLE regress_priv_role; GRANT pg_read_all_data TO regress_priv_user6; GRANT pg_write_all_data TO regress_priv_user7; CREATE GROUP regress_priv_group1; @@ -44,6 +45,14 @@ CREATE FUNCTION leak(integer,integer) RETURNS boolean LANGUAGE internal IMMUTABLE STRICT; -- but deliberately not LEAKPROOF ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1; -- test owner privileges +GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY CURRENT_ROLE; +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY foo; -- error +ERROR: role "foo" does not exist +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- error +ERROR: grantor must be current user +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_USER; +REVOKE regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_ROLE; +DROP ROLE regress_priv_role; SET SESSION AUTHORIZATION regress_priv_user1; SELECT session_user, current_user; session_user | current_user diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 6353a1cb8c..5c84df1a13 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -32,6 +32,7 @@ CREATE USER regress_priv_user5; CREATE USER regress_priv_user5; -- duplicate CREATE USER regress_priv_user6; CREATE USER regress_priv_user7; +CREATE ROLE regress_priv_role; GRANT pg_read_all_data TO regress_priv_user6; GRANT pg_write_all_data TO regress_priv_user7; @@ -53,6 +54,13 @@ ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1; -- test owner privileges +GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY CURRENT_ROLE; +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY foo; -- error +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- error +REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_USER; +REVOKE regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_ROLE; +DROP ROLE regress_priv_role; + SET SESSION AUTHORIZATION regress_priv_user1; SELECT session_user, current_user;