Re: Additional role attributes && superuser review

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Thu, Dec 31, 2015 at 4:26 PM, Noah Misch <noah@leadboat.com> wrote:
> > The proposed pg_replication role introduces abstraction that could, as you
> > hope, spare a DBA from studying sets of functions to grant together.  The
> > pg_rotate_logfile role, however, does not shield the DBA from complexity.
> > Being narrowly tied to a specific function, it's just a suboptimal spelling of
> > GRANT.  The gap in GRANT has distorted the design for these predefined roles.
> > I do not anticipate a sound design discussion about specific predefined roles
> > so long as the state of GRANT clouds the matter.
>
> As the patch stands, the system roles that are just close to synonyms
> of GRANT/REVOKE are the following:
> - pg_file_settings, which works just on the system view
> pg_file_settings and the function pg_show_all_file_settings().
> - pg_rotate_logfile as mentioned already.

The above are fine, I believe.

> - pg_signal_backend, which is just checked once in pg_signal_backend

This is a bit more complicated.  pg_signal_backend() isn't directly
exposed, pg_cancel_backend() and pg_termiante_backend() are.  I'm not
saying that doesn't mean we should, necessairly, keep the
pg_signal_backend role, just that it's more than just a single function.

Further, pg_terminate_backend and pg_cancel_backend are callable by
anyone today.  We'd have to invent new functions or change user-visible
behavior in an unfortunate way- we don't want *anyone* to be able to
call those functions on *anyone* unless they've been specifically
granted that access.  Today, you can cancel and/or terminate your own
backends and superusers can cancel and/or termiante anyone's.  The point
of pg_signal_backend was to allow non-superusers to cancel and/or
terminate any non-superuser-started backends.  With the default role
approach, we can provide exactly the intended semantics and not break
backwards compatibility for those functions.

> Based on those concerns, it looks clear that they should be shaved off
> from the patch.

I'd like to hear back regarding the summary email that I sent to Noah
and the follow-up I sent to Robert, as they have time to reply, of
course.  It's certainly easy enough to remove those default roles if
that's the consensus though.

> >> > To summarize, I think the right next step is to resume designing pg_dump
> >> > support for system object ACLs.  I looked over your other two patches and will
> >> > unshelve those reviews when their time comes.
> >>
> >> To be clear, I don't believe the two patches are particularly involved
> >> with each other and don't feel that one needs to wait for the other.
> >
> > Patch 2/3 could stand without patch 3/3, but not vice-versa.  It's patch 2/3
> > that makes pg_dumpall skip ^pg_ roles, and that must be in place no later than
> > the first patch that adds a predefined ^pg_ role.
>
> I am a bit confused by this statement, and I disagree with Stephen's
> point as well. It seems to me that 2/3 exists *because* 3/3 is here.
> Why would we want to restrict the role names that can be used if we
> are not going to introduce some system roles that are created at
> bootstrap?

My comment was, evidently, not anywhere near clear enough.  The two
patches I was referring to were the pg_dump-support-for-catalog-ACLs and
the default-roles patches.  Apologies for the confusion there.

Thanks!

Stephen