Re: Re: Revoke Connect Privilege from Database not working

Hi, here is a full reproducer. Also revoking with the granted by clause does not work. #clean initialization postgres=# create database testdb owner postgres; CREATE DATABASE postgres=# create user test_admin createrole; CREATE ROLE postgres=# alter user test_admin with password 'test1234'; ALTER ROLE postgres=# grant connect on database testdb to test_admin with grant option; GRANT #create user and grant connect privilege with test_admin postgres=# set role test_admin; SET postgres=> create user test_user password 'testuserpw'; CREATE ROLE postgres=> grant connect on database testdb to test_user; GRANT #generate the failure by granting test_admin superuser privileges postgres=> reset role; RESET postgres=# alter user test_admin superuser; ALTER ROLE postgres=# set role test_admin; SET postgres=# revoke connect on database testdb from test_user; REVOKE postgres=# drop user test_user; ERROR: role "test_user" cannot be dropped because some objects depend on it DETAIL: privileges for database testdb #test also with "granted by clause" postgres=# revoke connect on database testdb from test_user granted by "test_admin"; REVOKE postgres=# drop user test_user; ERROR: role "test_user" cannot be dropped because some objects depend on it DETAIL: privileges for database testdb #fix by removing superuser privilege from test_admin postgres=# reset role; RESET postgres=# alter user test_admin nosuperuser; ALTER ROLE postgres=# set role test_admin; SET postgres=> revoke connect on database testdb from test_user; REVOKE postgres=> drop role test_user; DROP ROLE Best Regards Marijo Kristo David G. Johnston <david.g.johnston@gmail.com> schrieb am 7. Apr. 2025 um 15:42: On Monday, April 7, 2025, Ing. Marijo Kristo < marijo.kristo@icloud.com > wrote: Seems like a bug to me. Can someone else verifiy this ? It would help greatly if you create a reproducer that starts from a clean install, creates the roles and database, and demonstrates the issue. postgres=# \du vault_admin; List of roles Role name | Attributes -------------+---------------- -------- vault_admin | Superuser, Create role postgres=# set role vault_admin; You are setting role to another role that has superuser which is basically pointless. Use “granted by” in your revoke command. If that works this isn’t a bug. David J.

On Mon, Apr 7, 2025 at 7:27 AM Ing. Marijo Kristo <marijo.kristo@icloud.com>
wrote:

Hi,
here is a full reproducer. Also revoking with the granted by clause does
not work.

#clean initialization
postgres=# create database testdb owner postgres;
CREATE DATABASE
postgres=# create user test_admin createrole;
CREATE ROLE
postgres=# alter user test_admin with password 'test1234';
ALTER ROLE
postgres=# grant connect on database testdb to test_admin with grant
option;
GRANT

#create user and grant connect privilege with test_admin
postgres=# set role test_admin;
SET
postgres=> create user test_user password 'testuserpw';
CREATE ROLE
postgres=> grant connect on database testdb to test_user;
GRANT

#generate the failure by granting test_admin superuser privileges
postgres=> reset role;
RESET
postgres=# alter user test_admin superuser;
ALTER ROLE
postgres=# set role test_admin;
SET
postgres=# revoke connect on database testdb from test_user;
REVOKE
postgres=# drop user test_user;
ERROR: role "test_user" cannot be dropped because some objects depend on
it
DETAIL: privileges for database testdb

#test also with "granted by clause"
postgres=# revoke connect on database testdb from test_user granted by
"test_admin";
REVOKE

On master, confirmed that after this command the privilege:

test_user=c/test_admin (on database testdb) still exists. That seems like
a bug. Its at least a POLA violation and I cannot figure out how to read
the revoke reference page in a way that explains it.

David J.

# revokescript.psql
create database testdb:v;
create user test_admin:v createrole;
grant connect on database testdb:v to test_admin:v with grant option;
set role test_admin:v;
create user test_user:v password 'testuserpw';
grant connect on database testdb:v to test_user:v;
reset role;
alter user test_admin:v superuser;
set role test_admin:v;
revoke connect on database testdb:v from test_user:v granted by
test_admin:v;
\l+ testdb:v
drop user test_user:v;

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On master, confirmed that after this command the privilege:
test_user=c/test_admin (on database testdb) still exists. That seems like
a bug. Its at least a POLA violation and I cannot figure out how to read
the revoke reference page in a way that explains it.

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.

IIRC, said rule was invented before we had the GRANTED BY
syntax. It probably doesn't make as much sense today,
but I'd be very afraid of breaking peoples' work flows
by changing it.

regards, tom lane

On Mon, Apr 7, 2025 at 9:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On master, confirmed that after this command the privilege:
test_user=c/test_admin (on database testdb) still exists. That seems

like

a bug. Its at least a POLA violation and I cannot figure out how to read
the revoke reference page in a way that explains it.

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.

revoke connect on database testdb:v
from test_user:v
---------------
granted by test_admin:v;
---^^^^^^^^^

So if we stick with status quo behavior we'd need to write the following
because the ignoring part is a POLA violation:

If a superuser chooses to issue a GRANT or REVOKE command, the command is
performed as though it were issued by the owner of the affected object, and
the granted by clause is ignored.

David J.

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 <tgl@sss.pgh.pa.us> 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

Nathan Bossart <nathandbossart@gmail.com> writes:

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.

Motivated by the discussion at [1]/messages/by-id/85cd06c6-7b2e-483e-b05d-d5ff87b0168d@garret.ru, 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]: /messages/by-id/85cd06c6-7b2e-483e-b05d-d5ff87b0168d@garret.ru

On Tue, Jan 20, 2026 at 06:05:41PM -0500, Tom Lane wrote:

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.

Yeah, I think doing most of the work in select_best_grantor() is obviously
better. I recall wondering whether we should check for INHERIT or SET
privilege (or both) on the grantor role, and IIRC I settled on INHERIT
because select_best_grantor() searches through roles we have INHERIT on.

Would you like to handle docs/tests/committing, or shall I?

--
nathan

Nathan Bossart <nathandbossart@gmail.com> writes:

Yeah, I think doing most of the work in select_best_grantor() is obviously
better. I recall wondering whether we should check for INHERIT or SET
privilege (or both) on the grantor role, and IIRC I settled on INHERIT
because select_best_grantor() searches through roles we have INHERIT on.

Yeah, I mentally had that point as something to check on. Clearly,
if you have SET ROLE privilege then you can become the target role
and then issue the GRANT, so if we define GRANTED BY like that
then we're not allowing anything that can't be done today. However,
it feels like INHERIT is a more natural test to make, since AIUI
that is what permits "automatic" use of a role's privileges, and that
seems to be what we'd be doing here.

I'd be interested to hear Robert's opinion on this, or somebody
else who worked on the SET/INHERIT splitup.

Also cc'ing Peter, who put in the current effectively-a-noise-clause
behavior of GRANTED BY, to see if he has standards-compliance or
other concerns about changing this.

Would you like to handle docs/tests/committing, or shall I?

I'm willing to push it forward if we have consensus to do it.

regards, tom lane

On Wed, Jan 21, 2026 at 11:57:01AM -0500, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

Yeah, I think doing most of the work in select_best_grantor() is obviously
better. I recall wondering whether we should check for INHERIT or SET
privilege (or both) on the grantor role, and IIRC I settled on INHERIT
because select_best_grantor() searches through roles we have INHERIT on.

Yeah, I mentally had that point as something to check on. Clearly,
if you have SET ROLE privilege then you can become the target role
and then issue the GRANT, so if we define GRANTED BY like that
then we're not allowing anything that can't be done today. However,
it feels like INHERIT is a more natural test to make, since AIUI
that is what permits "automatic" use of a role's privileges, and that
seems to be what we'd be doing here.

Agreed.

I'd be interested to hear Robert's opinion on this, or somebody
else who worked on the SET/INHERIT splitup.

Also cc'ing Peter, who put in the current effectively-a-noise-clause
behavior of GRANTED BY, to see if he has standards-compliance or
other concerns about changing this.

Robert/Peter, do you have any thoughts about expanding GRANT/REVOKE GRANTED
BY like this? I think it would've helped with a couple of reports received
during this development cycle, and IMHO it'd be a nice little feature for
v19.

--
nathan

I went ahead and tried adding docs, tests, and a commit message. The
documentation for these commands might need a revamp. They seem to meander
a bit, probably due to decades of organic development. But that's probably
not this patch's problem.

--
nathan

Nathan Bossart <nathandbossart@gmail.com> writes:

I went ahead and tried adding docs, tests, and a commit message.

Thanks! I was about to conclude that "silence means assent" and
do that work, but you beat me to it. Your changes look fine,
except that where you have

+   ... A role can only attribute a grant
+   to another role if they possess the privileges of that role.

the word "possess" seems a little ambiguous --- it's not clear whether
it means SET or INHERIT privileges. The grammar nerd in me doesn't
like "they" either. How about s/they possess/it inherits/ ?
(likewise in revoke.sgml)

The
documentation for these commands might need a revamp. They seem to meander
a bit, probably due to decades of organic development. But that's probably
not this patch's problem.

Agreed, seems like a task for another day.

regards, tom lane

On Wed, Mar 18, 2026 at 12:36:17PM -0400, Tom Lane wrote:

Thanks! I was about to conclude that "silence means assent" and
do that work, but you beat me to it. Your changes look fine,
except that where you have

Thanks for reviewing.

+   ... A role can only attribute a grant
+   to another role if they possess the privileges of that role.

the word "possess" seems a little ambiguous --- it's not clear whether
it means SET or INHERIT privileges. The grammar nerd in me doesn't
like "they" either. How about s/they possess/it inherits/ ?
(likewise in revoke.sgml)

This crossed my mind when copy/pasting from the "GRANT on Roles" section,
but I obviously didn't do anything about it. Fixed in the attached.

--
nathan

Nathan Bossart <nathandbossart@gmail.com> writes:

On Wed, Mar 18, 2026 at 12:36:17PM -0400, Tom Lane wrote:

the word "possess" seems a little ambiguous --- it's not clear whether
it means SET or INHERIT privileges. The grammar nerd in me doesn't
like "they" either. How about s/they possess/it inherits/ ?
(likewise in revoke.sgml)

This crossed my mind when copy/pasting from the "GRANT on Roles" section,
but I obviously didn't do anything about it. Fixed in the attached.

v4 LGTM.

regards, tom lane

On Wed, Mar 18, 2026 at 01:58:55PM -0400, Tom Lane wrote:

v4 LGTM.

Thanks, committed.

--
nathan