Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1viKnN-003OKZ-0X for pgsql-bugs@arkaria.postgresql.org; Tue, 20 Jan 2026 23:05:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1viKnM-003sQz-0s for pgsql-bugs@arkaria.postgresql.org; Tue, 20 Jan 2026 23:05:56 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1viKnL-003sQr-37 for pgsql-bugs@lists.postgresql.org; Tue, 20 Jan 2026 23:05:56 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1viKnJ-001beq-1S for pgsql-bugs@lists.postgresql.org; Tue, 20 Jan 2026 23:05:56 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 60KN5fmb1933587; Tue, 20 Jan 2026 18:05:41 -0500 From: Tom Lane To: Nathan Bossart cc: "David G. Johnston" , "Ing. Marijo Kristo" , PostgreSQL Bug List Subject: Re: Revoke Connect Privilege from Database not working In-reply-to: References: <3467676.1744041977@sss.pgh.pa.us> Comments: In-reply-to Nathan Bossart message dated "Thu, 13 Nov 2025 10:47:14 -0600" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1933370.1768950290.0@sss.pgh.pa.us> Date: Tue, 20 Jan 2026 18:05:41 -0500 Message-ID: <1933586.1768950341@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1933370.1768950290.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable Nathan Bossart writes: > This is admittedly a half-formed idea, but perhaps we could have whateve= r's > specified in GRANTED BY override select_best_grantor(), like in the > attached patch. I've no idea if this is the intention of the standard, = but > it should at least address the reported issue. FWIW I recently received= an > independent report about the same thing. = Motivated by the discussion at [1], I'd started on the same idea, but arrived at a rather different refactorization. I think this way is nicer (less duplicated logic). Either way, we need to address the docs and probably add more regression tests. regards, tom lane [1] https://www.postgresql.org/message-id/flat/85cd06c6-7b2e-483e-b05d-d5f= f87b0168d%40garret.ru ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v2-0001-GRANTED-BY.patch"; charset="us-ascii" Content-ID: <1933370.1768950290.2@sss.pgh.pa.us> Content-Description: v2-0001-GRANTED-BY.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index a431fc0926f..e31d22ebf7d 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -98,6 +98,7 @@ typedef struct AclMode privileges; List *grantees; bool grant_option; + RoleSpec *grantor; DropBehavior behavior; } InternalDefaultACL; = @@ -395,22 +396,6 @@ ExecuteGrantStmt(GrantStmt *stmt) const char *errormsg; AclMode all_privileges; = - if (stmt->grantor) - { - Oid grantor; - - grantor =3D get_rolespec_oid(stmt->grantor, false); - - /* - * Currently, this clause is only for SQL compatibility, not very - * interesting otherwise. - */ - if (grantor !=3D GetUserId()) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("grantor must be current user"))); - } - /* * Turn the regular GrantStmt into the InternalGrant form. */ @@ -438,6 +423,7 @@ ExecuteGrantStmt(GrantStmt *stmt) istmt.col_privs =3D NIL; /* may get filled below */ istmt.grantees =3D NIL; /* filled below */ istmt.grant_option =3D stmt->grant_option; + istmt.grantor =3D stmt->grantor; istmt.behavior =3D stmt->behavior; = /* @@ -960,6 +946,7 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, Alt= erDefaultPrivilegesStmt *s /* privileges to be filled below */ iacls.grantees =3D NIL; /* filled below */ iacls.grant_option =3D action->grant_option; + iacls.grantor =3D action->grantor; iacls.behavior =3D action->behavior; = /* @@ -1486,6 +1473,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid= objid) iacls.privileges =3D ACL_NO_RIGHTS; iacls.grantees =3D list_make1_oid(roleid); iacls.grant_option =3D false; + iacls.grantor =3D NULL; iacls.behavior =3D DROP_CASCADE; = /* Do it */ @@ -1542,6 +1530,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid= objid) istmt.col_privs =3D NIL; istmt.grantees =3D list_make1_oid(roleid); istmt.grant_option =3D false; + istmt.grantor =3D NULL; istmt.behavior =3D DROP_CASCADE; = ExecGrantStmt_oids(&istmt); @@ -1696,7 +1685,7 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid= , const char *relname, merged_acl =3D aclconcat(old_rel_acl, old_acl); = /* Determine ID to do the grant as, and available grant options */ - select_best_grantor(GetUserId(), col_privileges, + select_best_grantor(istmt->grantor, col_privileges, merged_acl, ownerId, &grantorId, &avail_goptions); = @@ -1969,7 +1958,7 @@ ExecGrant_Relation(InternalGrant *istmt) ObjectType objtype; = /* Determine ID to do the grant as, and available grant options */ - select_best_grantor(GetUserId(), this_privileges, + select_best_grantor(istmt->grantor, this_privileges, old_acl, ownerId, &grantorId, &avail_goptions); = @@ -2184,7 +2173,7 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, = AclMode default_privs, } = /* Determine ID to do the grant as, and available grant options */ - select_best_grantor(GetUserId(), istmt->privileges, + select_best_grantor(istmt->grantor, istmt->privileges, old_acl, ownerId, &grantorId, &avail_goptions); = @@ -2339,7 +2328,7 @@ ExecGrant_Largeobject(InternalGrant *istmt) } = /* Determine ID to do the grant as, and available grant options */ - select_best_grantor(GetUserId(), istmt->privileges, + select_best_grantor(istmt->grantor, istmt->privileges, old_acl, ownerId, &grantorId, &avail_goptions); = @@ -2485,7 +2474,7 @@ ExecGrant_Parameter(InternalGrant *istmt) } = /* Determine ID to do the grant as, and available grant options */ - select_best_grantor(GetUserId(), istmt->privileges, + select_best_grantor(istmt->grantor, istmt->privileges, old_acl, ownerId, &grantorId, &avail_goptions); = diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 3a6905f9546..90fa49eacb7 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -5451,6 +5451,10 @@ select_best_admin(Oid member, Oid role) /* * Select the effective grantor ID for a GRANT or REVOKE operation. * + * If the GRANT/REVOKE has an explicit GRANTED BY clause, we always use + * exactly that role (which may result in granting/revoking no privileges= ). + * Otherwise, we seek a "best" grantor, starting with the current user. + * * The grantor must always be either the object owner or some role that h= as * been explicitly granted grant options. This ensures that all granted * privileges appear to flow from the object owner, and there are never @@ -5463,25 +5467,44 @@ select_best_admin(Oid member, Oid role) * role has 'em all. In this case we pick a role with the largest number * of desired options. Ties are broken in favor of closer ancestors. * - * roleId: the role attempting to do the GRANT/REVOKE + * grantedBy: the GRANTED BY clause of GRANT/REVOKE, or NULL if none * privileges: the privileges to be granted/revoked * acl: the ACL of the object in question * ownerId: the role owning the object in question * *grantorId: receives the OID of the role to do the grant as - * *grantOptions: receives the grant options actually held by grantorId - * - * If no grant options exist, we set grantorId to roleId, grantOptions to= 0. + * *grantOptions: receives grant options actually held by grantorId (mayb= e 0) */ void -select_best_grantor(Oid roleId, AclMode privileges, +select_best_grantor(const RoleSpec *grantedBy, AclMode privileges, const Acl *acl, Oid ownerId, Oid *grantorId, AclMode *grantOptions) { + Oid roleId =3D GetUserId(); AclMode needed_goptions =3D ACL_GRANT_OPTION_FOR(privileges); List *roles_list; int nrights; ListCell *l; = + /* + * If we have GRANTED BY, resolve it and verify current user is allowed = to + * specify that role. + */ + if (grantedBy) + { + Oid grantor =3D get_rolespec_oid(grantedBy, false); + + if (!has_privs_of_role(roleId, grantor)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must inherit privileges of role \"%s\"", + GetUserNameFromId(grantor, false)))); + /* Use exactly that grantor, whether it has privileges or not */ + *grantorId =3D grantor; + *grantOptions =3D aclmask_direct(acl, grantor, ownerId, + needed_goptions, ACLMASK_ALL); + return; + } + /* * The object owner is always treated as having all grant options, so if * roleId is the owner it's easy. Also, if roleId is a superuser it's diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes= .h index 646d6ced763..b12f21d22e9 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2613,7 +2613,7 @@ typedef struct GrantStmt /* privileges =3D=3D NIL denotes ALL PRIVILEGES */ List *grantees; /* list of RoleSpec nodes */ bool grant_option; /* grant or revoke grant option */ - RoleSpec *grantor; + RoleSpec *grantor; /* GRANTED BY clause, or NULL if none */ DropBehavior behavior; /* drop behavior (for REVOKE) */ } GrantStmt; = diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index ec01fd581cf..9da62a7aa76 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -223,7 +223,7 @@ extern void check_rolespec_name(const RoleSpec *role, = const char *detail_msg); extern HeapTuple get_rolespec_tuple(const RoleSpec *role); extern char *get_rolespec_name(const RoleSpec *role); = -extern void select_best_grantor(Oid roleId, AclMode privileges, +extern void select_best_grantor(const RoleSpec *grantedBy, AclMode privil= eges, const Acl *acl, Oid ownerId, Oid *grantorId, AclMode *grantOptions); = diff --git a/src/include/utils/aclchk_internal.h b/src/include/utils/aclch= k_internal.h index 38317e2ed37..fa0b65fbba7 100644 --- a/src/include/utils/aclchk_internal.h +++ b/src/include/utils/aclchk_internal.h @@ -38,6 +38,7 @@ typedef struct List *col_privs; List *grantees; bool grant_option; + RoleSpec *grantor; DropBehavior behavior; } InternalGrant; = diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/e= xpected/privileges.out index daafaa94fde..997c4b68f47 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -321,7 +321,7 @@ SELECT pg_get_acl(0, 0, 0); -- null (1 row) = GRANT TRUNCATE ON atest2 TO regress_priv_user4 GRANTED BY regress_priv_us= er5; -- error -ERROR: grantor must be current user +ERROR: must inherit privileges of role "regress_priv_user5" SET SESSION AUTHORIZATION regress_priv_user2; SELECT session_user, current_user; session_user | current_user = ------- =_aaaaaaaaaa0--