Re: DROP OWNED BY fails to clean out pg_init_privs grants - Mailing list pgsql-hackers

From Tom Lane
Subject Re: DROP OWNED BY fails to clean out pg_init_privs grants
Date
Msg-id 2720580.1718476858@sss.pgh.pa.us
Whole thread Raw
In response to Re: DROP OWNED BY fails to clean out pg_init_privs grants  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> The semantics I've implemented on top of that are:
> * ALTER OWNER does not touch pg_init_privs entries.
> * REASSIGN OWNED replaces pg_init_privs references with the new
> role, whether the references are as grantor or grantee.
> * DROP OWNED removes pg_init_privs mentions of the doomed role
> (as grantor or grantee), removing the pg_init_privs entry
> altogether if it goes to zero clauses.  (This is what happened
> already, but only if if a SHARED_DEPENDENCY_INITACL entry existed.)

> I'm not terribly thrilled with this, because it's still possible
> to get into a state where a pg_init_privs entry is based on
> an owning role that's no longer the owner: you just have to use
> ALTER OWNER directly rather than via REASSIGN OWNED.  While
> I don't think the backend has much problem with that, it probably
> will confuse pg_dump to some extent.

I poked at this some more, and I'm now moderately convinced that
this is a good place to stop for v17, primarily because ALTER OWNER
doesn't touch pg_init_privs in the older branches either.  Thus,
while we've not made things better for pg_dump, at least we've not
introduced a new behavior it will have to account for in the future.

I experimented with this test scenario (to set up, do
"make install" in src/test/modules/test_pg_dump):

-----
create role regress_dump_test_role;
create user test_super superuser;
create user joe;
create database tpd;
\c tpd test_super
create extension test_pg_dump;
alter function regress_pg_dump_schema.test_func() owner to joe;
\df+ regress_pg_dump_schema.test_func()
-----

The \df+ will correctly show that test_func() is owned by joe
and has ACL

|      Access privileges       |
+------------------------------+
| =X/joe                      +|
| joe=X/joe                   +|
| regress_dump_test_role=X/joe |

Now, if you pg_dump this database, what you get is

CREATE EXTENSION IF NOT EXISTS test_pg_dump WITH SCHEMA public;
...
--
-- Name: FUNCTION test_func(); Type: ACL; Schema: regress_pg_dump_schema; Owner: joe
--

REVOKE ALL ON FUNCTION regress_pg_dump_schema.test_func() FROM PUBLIC;
REVOKE ALL ON FUNCTION regress_pg_dump_schema.test_func() FROM test_super;
REVOKE ALL ON FUNCTION regress_pg_dump_schema.test_func() FROM regress_dump_test_role;
GRANT ALL ON FUNCTION regress_pg_dump_schema.test_func() TO joe;
GRANT ALL ON FUNCTION regress_pg_dump_schema.test_func() TO PUBLIC;
GRANT ALL ON FUNCTION regress_pg_dump_schema.test_func() TO regress_dump_test_role;

So pg_dump realizes that the privileges are not what they were,
but it's fairly confused about what to do about it.  And it really
can't get that right without better modeling (er, more than none at
all) of the ownership of extension-member objects.  If you restore
this dump as postgres, you'll find that test_func is now owned by
postgres and has ACL

|         Access privileges         |
+-----------------------------------+
| postgres=X/postgres              +|
| joe=X/postgres                   +|
| =X/postgres                      +|
| regress_dump_test_role=X/postgres |

The grantees are okay, more or less, but we've totally failed to
replicate the owner/grantor.  But this is exactly the same as what
you get if you do the experiment in v16 or before.  Given the lack
of complaints about that, I think it's okay to stop here for now.
We've at least made REASSIGN OWNED work better.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Jelte Fennema-Nio
Date:
Subject: Re: RFC: adding pytest as a supported test framework
Next
From: Ilya Gladyshev
Date:
Subject: Re: CREATE INDEX CONCURRENTLY on partitioned index