Re: incorrect pg_dump output due to not handling dropped roles correctly

Greetings,

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> On Wed, Jan 29, 2020 at 14:46 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > > In particular- should we really be setting default privileges for objects
> > > that are being created by an extension, at all? The more I think about
> > it,
> > > the less I like it, particularly when we start to think about the trusted
> > > extension work that Tom’s doing.
> >
> > > In short, I’m leaning towards the argument that this was a missing check
> > in
> > > the default ACL code to explicitly *not* add default privileges on
> > objects
> > > that are being created by an extension. With such a check, the entire
> > > wouldn’t end up in pg_init_privs and the issue that started this thread
> > > wouldn’t happen.
> >
> > Hmm. I think we'd still wish pg_upgrade to preserve whatever privileges
> > have been granted on extension objects, but that doesn't necessarily
> > have to happen through pg_init_privs. I've forgotten the details ---
> > would the change you suggest be enough to make upgrade work correctly,
> > or would we need additional hacking?
>
> Err, privileges on upgrade *are* preserved, even with this. Guess I wasn’t
> clear.
>
> Right now:
>
> If someone sets up default privileges for a user, and that user then
> creates an extension, the default privs are applied to all of the
> individual objects that are created by the extension, and then get recorded
> in pg_init_privs, as if it was the extension setting up those privileges.
>
> Having those privileges end up in pg_init_privs is pretty clearly wrong
> because only those privileges that are installed by the extension should
> end up in init privs, because all OTHER privileges are what will be dumped
> out.
>
> However, I’m not sure that those objects, which are inside the extension,
> should really be having default privs applied to them in the first place!
> If I have default privs set up to grant EXECUTE to some user on functions,
> and then I install pg adminpack, is it really right for all the functions
> in pg adminpack to now have execute rights granted to whomever this user
> is- even when the extension revoked the grant to public because the
> function is unsafe for general users to have access to..?
>
> I feel like default privs should apply when the user is EXPLICITLY creating
> a specific object, not when it’s being created as part of an extension, and
> therefore we should have a check in the creation system, where the default
> privs are handled, to basically make the default privs code be a no-op when
> the object is being created by an extension. Users will have to go grant
> access to those objects in the extension explicitly instead, but that seems
> perfectly reasonable to me.
>
> Currently traveling and so not actually looking at the code, but that’s my
> recollection of things.

Back from said travel now and reviewed the thread and discussion and
this looks like exactly what's happening- so, does someone want to
comment on this, or..?

Specifically, if one does:

postgres=# create role test;
CREATE ROLE
postgres=# alter default privileges in schema public grant all privileges on tables to test;
ALTER DEFAULT PRIVILEGES
postgres=# create extension pg_stat_statements;
CREATE EXTENSION

Should the pg_stat_statements view, that's actually created by the
extension as part of its install, end up with the default privileges of
"grant all privileges on tables to test"?

At this point, my vote is 'no, that is a bug that we should fix.', but
it's also clearly a behavior change and maybe others have a different
opinion..? If not, it should at least be a relatively easy fix..

Thanks,

Stephen