public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: David G. Johnston <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Ing. Marijo Kristo <[email protected]>
Cc: PostgreSQL Bug List <[email protected]>
Subject: Re: Re: Re: Revoke Connect Privilege from Database not working
Date: Thu, 13 Nov 2025 10:47:14 -0600
Message-ID: <aRYLkTpazxKhnS_w@nathan> (raw)
In-Reply-To: <CAKFQuwbpC5w6sUq8gZQATrviZUT4bYpxW+=2uH6sWWMg7fWjzg@mail.gmail.com>
References: <CAKFQuwa7m2smqqpgPetw=i8Aj-xqg9Zjc5Z2aX3AUwNh96WnXw@mail.gmail.com>
<[email protected]>
<CAKFQuwbB-ZKtN_p_y5sWa2MrTuy5=pRNPWSj1Ud4HHvTuhb54w@mail.gmail.com>
<[email protected]>
<CAKFQuwbpC5w6sUq8gZQATrviZUT4bYpxW+=2uH6sWWMg7fWjzg@mail.gmail.com>
On Mon, Apr 07, 2025 at 09:22:45AM -0700, David G. Johnston wrote:
> On Mon, Apr 7, 2025 at 9:06 AM Tom Lane <[email protected]> wrote:
>> I believe what's going on there is explained by the rule that
>> "grants and revokes done by a superuser are done as if issued
>> by the object owner". So here, what would be revoked is
>> test_user=c/postgres, which isn't the privilege at issue.
>> Include GRANTED BY in the REVOKE to override the default
>> choice of grantor.
>
> The command in question did include "granted by" which is why this is a
> bug. The explicit granted by specification is being ignored if the
> invoking user is a superuser.
This is admittedly a half-formed idea, but perhaps we could have whatever'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.
--
nathan
From 7949dc2dd3bda2b93f0125311fea1d8fa60e6263 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 13 Nov 2025 09:29:35 -0600
Subject: [PATCH v1 1/1] GRANTED BY
---
src/backend/catalog/aclchk.c | 89 +++++++++++++++++-------
src/backend/utils/adt/acl.c | 2 +-
src/include/utils/acl.h | 2 +
src/include/utils/aclchk_internal.h | 1 +
src/test/regress/expected/privileges.out | 2 +-
5 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index cd139bd65a6..9ee3ff9e494 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -397,19 +397,15 @@ ExecuteGrantStmt(GrantStmt *stmt)
if (stmt->grantor)
{
- Oid grantor;
-
- grantor = get_rolespec_oid(stmt->grantor, false);
-
- /*
- * Currently, this clause is only for SQL compatibility, not very
- * interesting otherwise.
- */
- if (grantor != GetUserId())
+ istmt.grantor = get_rolespec_oid(stmt->grantor, false);
+ if (!has_privs_of_role(GetUserId(), istmt.grantor))
ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("grantor must be current user")));
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must inherit privileges of role \"%s\"",
+ GetUserNameFromId(istmt.grantor, false))));
}
+ else
+ istmt.grantor = InvalidOid;
/*
* Turn the regular GrantStmt into the InternalGrant form.
@@ -1538,6 +1534,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
istmt.all_privs = true;
istmt.privileges = ACL_NO_RIGHTS;
istmt.col_privs = NIL;
+ istmt.grantor = InvalidOid;
istmt.grantees = list_make1_oid(roleid);
istmt.grant_option = false;
istmt.behavior = DROP_CASCADE;
@@ -1694,9 +1691,17 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
merged_acl = aclconcat(old_rel_acl, old_acl);
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), col_privileges,
- merged_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(merged_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(col_privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), col_privileges,
+ merged_acl, ownerId,
+ &grantorId, &avail_goptions);
pfree(merged_acl);
@@ -1967,9 +1972,17 @@ ExecGrant_Relation(InternalGrant *istmt)
ObjectType objtype;
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), this_privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(this_privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), this_privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
switch (pg_class_tuple->relkind)
{
@@ -2182,9 +2195,17 @@ 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,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
nameDatum = SysCacheGetAttrNotNull(cacheid, tuple,
get_object_attnum_name(classid));
@@ -2337,9 +2358,17 @@ ExecGrant_Largeobject(InternalGrant *istmt)
}
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), istmt->privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
/*
* Restrict the privileges to what we can actually grant, and emit the
@@ -2483,9 +2512,17 @@ ExecGrant_Parameter(InternalGrant *istmt)
}
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), istmt->privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
/*
* Restrict the privileges to what we can actually grant, and emit the
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index fbcd64a2609..bf078888b0f 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -1473,7 +1473,7 @@ aclmask(const Acl *acl, Oid roleid, Oid ownerId,
* This is exactly like aclmask() except that we consider only privileges
* held *directly* by roleid, not those inherited via role membership.
*/
-static AclMode
+AclMode
aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
AclMode mask, AclMaskHow how)
{
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 01ae5b719fd..50d3b42bfcc 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -207,6 +207,8 @@ extern bool aclequal(const Acl *left_acl, const Acl *right_acl);
extern AclMode aclmask(const Acl *acl, Oid roleid, Oid ownerId,
AclMode mask, AclMaskHow how);
+extern AclMode aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
+ AclMode mask, AclMaskHow how);
extern int aclmembers(const Acl *acl, Oid **roleids);
extern bool has_privs_of_role(Oid member, Oid role);
diff --git a/src/include/utils/aclchk_internal.h b/src/include/utils/aclchk_internal.h
index 62af290779a..051a9630256 100644
--- a/src/include/utils/aclchk_internal.h
+++ b/src/include/utils/aclchk_internal.h
@@ -36,6 +36,7 @@ typedef struct
bool all_privs;
AclMode privileges;
List *col_privs;
+ Oid grantor;
List *grantees;
bool grant_option;
DropBehavior behavior;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/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_user5; -- 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
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v1-0001-GRANTED-BY.patch (7.3K, 2-v1-0001-GRANTED-BY.patch)
download | inline diff:
From 7949dc2dd3bda2b93f0125311fea1d8fa60e6263 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 13 Nov 2025 09:29:35 -0600
Subject: [PATCH v1 1/1] GRANTED BY
---
src/backend/catalog/aclchk.c | 89 +++++++++++++++++-------
src/backend/utils/adt/acl.c | 2 +-
src/include/utils/acl.h | 2 +
src/include/utils/aclchk_internal.h | 1 +
src/test/regress/expected/privileges.out | 2 +-
5 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index cd139bd65a6..9ee3ff9e494 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -397,19 +397,15 @@ ExecuteGrantStmt(GrantStmt *stmt)
if (stmt->grantor)
{
- Oid grantor;
-
- grantor = get_rolespec_oid(stmt->grantor, false);
-
- /*
- * Currently, this clause is only for SQL compatibility, not very
- * interesting otherwise.
- */
- if (grantor != GetUserId())
+ istmt.grantor = get_rolespec_oid(stmt->grantor, false);
+ if (!has_privs_of_role(GetUserId(), istmt.grantor))
ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("grantor must be current user")));
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must inherit privileges of role \"%s\"",
+ GetUserNameFromId(istmt.grantor, false))));
}
+ else
+ istmt.grantor = InvalidOid;
/*
* Turn the regular GrantStmt into the InternalGrant form.
@@ -1538,6 +1534,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
istmt.all_privs = true;
istmt.privileges = ACL_NO_RIGHTS;
istmt.col_privs = NIL;
+ istmt.grantor = InvalidOid;
istmt.grantees = list_make1_oid(roleid);
istmt.grant_option = false;
istmt.behavior = DROP_CASCADE;
@@ -1694,9 +1691,17 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
merged_acl = aclconcat(old_rel_acl, old_acl);
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), col_privileges,
- merged_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(merged_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(col_privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), col_privileges,
+ merged_acl, ownerId,
+ &grantorId, &avail_goptions);
pfree(merged_acl);
@@ -1967,9 +1972,17 @@ ExecGrant_Relation(InternalGrant *istmt)
ObjectType objtype;
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), this_privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(this_privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), this_privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
switch (pg_class_tuple->relkind)
{
@@ -2182,9 +2195,17 @@ 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,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
nameDatum = SysCacheGetAttrNotNull(cacheid, tuple,
get_object_attnum_name(classid));
@@ -2337,9 +2358,17 @@ ExecGrant_Largeobject(InternalGrant *istmt)
}
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), istmt->privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
/*
* Restrict the privileges to what we can actually grant, and emit the
@@ -2483,9 +2512,17 @@ ExecGrant_Parameter(InternalGrant *istmt)
}
/* Determine ID to do the grant as, and available grant options */
- select_best_grantor(GetUserId(), istmt->privileges,
- old_acl, ownerId,
- &grantorId, &avail_goptions);
+ if (OidIsValid(istmt->grantor))
+ {
+ grantorId = istmt->grantor;
+ avail_goptions = aclmask_direct(old_acl, grantorId, ownerId,
+ ACL_GRANT_OPTION_FOR(istmt->privileges),
+ ACLMASK_ALL);
+ }
+ else
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
/*
* Restrict the privileges to what we can actually grant, and emit the
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index fbcd64a2609..bf078888b0f 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -1473,7 +1473,7 @@ aclmask(const Acl *acl, Oid roleid, Oid ownerId,
* This is exactly like aclmask() except that we consider only privileges
* held *directly* by roleid, not those inherited via role membership.
*/
-static AclMode
+AclMode
aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
AclMode mask, AclMaskHow how)
{
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 01ae5b719fd..50d3b42bfcc 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -207,6 +207,8 @@ extern bool aclequal(const Acl *left_acl, const Acl *right_acl);
extern AclMode aclmask(const Acl *acl, Oid roleid, Oid ownerId,
AclMode mask, AclMaskHow how);
+extern AclMode aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
+ AclMode mask, AclMaskHow how);
extern int aclmembers(const Acl *acl, Oid **roleids);
extern bool has_privs_of_role(Oid member, Oid role);
diff --git a/src/include/utils/aclchk_internal.h b/src/include/utils/aclchk_internal.h
index 62af290779a..051a9630256 100644
--- a/src/include/utils/aclchk_internal.h
+++ b/src/include/utils/aclchk_internal.h
@@ -36,6 +36,7 @@ typedef struct
bool all_privs;
AclMode privileges;
List *col_privs;
+ Oid grantor;
List *grantees;
bool grant_option;
DropBehavior behavior;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/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_user5; -- 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
--
2.39.5 (Apple Git-154)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Re: Re: Revoke Connect Privilege from Database not working
In-Reply-To: <aRYLkTpazxKhnS_w@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox