Re: Default Roles (was: Additional role attributes) - Mailing list pgsql-hackers

On Wed, May 13, 2015 at 12:07 PM, Stephen Frost <sfrost@snowman.net> wrote:
> All,
>
> This patch gets smaller and smaller.
>
> Upon reflection I realized that, with default roles, it's entirely
> unnecssary to change how the permission checks happen today- we can
> simply add checks to them to be looking at role membership also.  That's
> removed the last of my concerns regarding any API breakage for existing
> use-cases and has greatly simplified things overall.
>
> This does change the XLOG functions to require pg_monitor, as discussed
> on the other thread where it was pointed out by Heikki that the XLOG
> location information could be used to extract sensitive information
> based on what happens during compression.  Adding docs explaining that
> is on my to-do list for tomorrow.
>
> * Stephen Frost (sfrost@snowman.net) wrote:
>> Andres suggested that we drop the REPLICATION role attribute and just
>> use membership in pg_replication instead.  That's turned out quite
>> fantastically as we can now handle upgrades without breaking anything-
>> CREATE ROLE and ALTER ROLE still accept the attribute but simply grant
>> pg_replication to the role instead, and postinit.c has been changed to
>> check role membership similar to other pg_hba role membership checks
>> when a replication connection comes in.  Hat's off to Andres for his
>> suggestion.
>
> It's also unnecessary to change how the REPLICATION role attribute
> functions today.  This patch does add the pg_replication role, but it's
> only allowed to execute the various pg_logical and friends functions and
> not to actually connect as a REPLICATION user.  Connecting as a
> REPLICATION user allows you to stream the entire contents of the
> backend, after all, so it makes sense to have that be independent.
>
> I added another default role which allows the user to view
> pg_show_file_settings, as that seemed useful to me.  The diffstat for
> that being something like 4 additions without docs and maybe 10 with.
> More documentation would probably be good though and I'll look at adding
> to it.
>
> Most of the rest of what I've done has simply been reverting back to
> what we had.  The patch is certainly far easier to verify by reading
> through it now, as the changes are right next to each other, and the
> regression output changes are much smaller.
>
> Thoughts?  Comments?  Suggestions?

he documents of the functions which the corresponding default roles
are added by this patch need to be updated. For example, the description
of pg_xlog_replay_pause() says "Pauses recovery immediately (restricted
to superusers).". I think that the description needs to mention
the corresponding default role "pg_replay". Otherwise, it's difficult for
users to see which default role is related to the function they want to use.
Or probably we can add the table explaining all the relationships between
default roles and corresponding operations. And it's useful.

Why do we allow users to change the attributes of default roles?
For example, ALTER ROLE default_role or GRANT ... TO default_role.
Those changes are not dumped by pg_dumpall. So if users change
the attributes for some reasons but they disappear via pg_dumpall,
maybe the system goes into unexpected state.

I think that it's better to allow the roles with pg_monitor to
execute pgstattuple functions. They are usually used for monitoring.
Thought?

Regards,

-- 
Fujii Masao