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

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 65bb73395891d54e97ccae4b2e53b303eec15ef8..b9cca41a7d3615ae358bfb95c4d41bf7928082c0 100644 (file)
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1319,7 +1319,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 fe2af568c9133a8bf3c734cbe142d327e8ad6624..1b7494f0df8dffe9de75884d404c8a7529206bf3 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7181,6 +7181,7 @@ RevokeRoleStmt:
                    n->admin_opt = false;
                    n->granted_roles = $2;
                    n->grantee_roles = $4;
+                   n->grantor = $5;
                    n->behavior = $6;
                    $$ = (Node*)n;
                }
@@ -7191,6 +7192,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 83cff902f31e0f4e361a16789097384fb70db19d..d106733dcc637397d32f74da1a278ca76ee06e86 100644 (file)
--- 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 3d1a1db9870831ec1d0ec8fad94b3d23e2190cce..5903437a0999dace8bf8384a91749eb563aad380 100644 (file)
--- 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;