* Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Mar 22, 2017 at 7:48 AM, Dave Page <dp...@pgadmin.org> wrote: > >> I'd be inclined to skip the rest of > >> this. If an individual user wants to grant that bundle of privileges > >> to a role, they can do it with or without pg_monitor. > > > > GRANT cannot be used in all cases, as some of the functions changed > > have hard-coded superuser checks. In those cases, I've added > > pg_monitor membership to the permission checks in the C code. > > Oh. Well, why not just control access using the permissions checking > mechanism entirely, without hardcoding any OID?
I've not had the chance yet to review this (though I have been planning to), but at least in most cases where we have hard-coded checks it's because the GRANT system doesn't allow the kind of fine-grained access we've implemented at the C level. pg_stat_activity is a great example of this where, if you aren't a superuser or a member of the other role, you can still query the table but you can't see the 'query' column for those other connections. I did specifically ask for explicit roles to be made to enable such capability and that the pg_monitor role be GRANT'd those roles instead of hacking the pg_monitor OID into those checks, but it seems like that's not been done yet. > > The reason for having the role is to minimise the amount of work > > required by the user to setup a role for the purposes of monitoring > > the server. This is a practice which is seen in other DBMSs for > > usability. > > > > With the patch, complex monitoring systems can easily be setup with > > something like: > > > > CREATE ROLE monitoring_user LOGIN; > > GRANT pg_monitor TO monitoring_role; > > > > Without, the users setting up their monitoring system will have to run > > a much more complex set of GRANTs, almost certainly requiring > > scripting. > > But that script is easy to provide, probably not very long, and could > be bundled in an extension if it's helpful. I'm wary of committing > ourselves to a specific decision about what pg_monitor includes; that > seems like it could result in endless future litigation every time > somebody wants to make a change. In contrast, nobody's going to have > any question at all about the remit of pg_read_all_settings. I tend to agree with Dave on this. While I understand this concern and risk of litigation, I believe that comes down to, really, making sure that we spell out exactly what pg_monitor is intended for and that the privileges it has will change over time. If users are not comfortable with that, then they can use provided script to create their own 'monitor' role- that is specifically why I want to have nothing hard-coded to pg_monitor though, because otherwise you couldn't have such a script able to GRANT out a subset of what pg_monitor allows. Thanks! Stephen
signature.asc
Description: Digital signature
