Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (1.0)
Subject: Re: Monitoring roles patch
From: Dave Page <dpage@pgadmin.org>
In-Reply-To: <20170325155520.GW9812@tamriel.snowman.net>
Date: Sat, 25 Mar 2017 16:30:24 +0000
Cc: Robert Haas <robertmhaas@gmail.com>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <0C373C5B-2291-4608-A349-4F57D694F533@pgadmin.org>
References: 
 <CA+OCxoyRdsc1xyLfF9s698gUGyPXBs4CvJ+0Gwo8U65NmYJ7pw@mail.gmail.com>
 <CA+TgmobveXzVPytFOO3AkmsM+Y=YEG+UXcCvgwAt+SgYj85OCA@mail.gmail.com>
 <CA+OCxox9c9-1=bXxA6-gr-iWZbD5zdHYMz=cpC034RKB7zX40Q@mail.gmail.com>
 <c99cde7f-0aa7-ebaa-b9f4-ee75522554a4@2ndquadrant.com>
 <20170322131730.GD9812@tamriel.snowman.net>
 <d3e2e808-50fd-daca-fc07-8863930cae7d@2ndquadrant.com>
 <20170324123031.GD9812@tamriel.snowman.net>
 <CA+TgmobXyfkR2nL6jSYHCNHe6ovAw13eyY3ga39DN=J4Dd=idQ@mail.gmail.com>
 <CA+OCxoyuW71uA8TqLpiOk33gB38fwZAUL=1L5wWcGCTu2SAiwg@mail.gmail.com>
 <CA+TgmoadTvGh9_O71iQ8vjEePhSwtPo148We15+k568hXHMEJw@mail.gmail.com>
 <20170325155520.GW9812@tamriel.snowman.net>
To: Stephen Frost <sfrost@snowman.net>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Hi

> On 25 Mar 2017, at 15:55, Stephen Frost <sfrost@snowman.net> wrote:
>=20
> Robert,
>=20
> * Robert Haas (robertmhaas@gmail.com) wrote:
>>> On Fri, Mar 24, 2017 at 12:46 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>> On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas <robertmhaas@gmail.com> w=
rote:
>>>> That's possible, but it really depends on the tool, not on core
>>>> PostgreSQL.  The tool should be the one providing the script, because
>>>> the tool is what knows its own permissions requirements.  Users who
>>>> care about security won't want to grant every privilege given by a
>>>> pg_monitor role to a tool that only needs a subset of those
>>>> privileges.
>>>=20
>>> The upshot of this would be that every time there's a database server
>>> upgrade that changes the permissions required somehow, the user has to
>>> login to every server they have and run a script. It is no longer a
>>> one-time thing, which makes it vastly more painful to deal with
>>> upgrades.
>>=20
>> So, I might be all wet here, but I would have expected that changes on
>> the TOOL side would be vastly more frequent.  I mean, we do not change
>> what a certain builtin permission does very often.  If we add
>> pg_read_all_settings, what is the likelihood that the remit of that
>> role is ever going to change?  I would judge that was is vastly more
>> likely is that a new version of some tool would start needing that
>> privilege (or some other) where it didn't before.  If that happens,
>> and the script is on the tool side, then you just add a new line to
>> the script.  If the script is built into initdb, then you've got to
>> wait for the next major release before you can update the definition
>> of pg_monitor - and maybe argue with other tool authors with different
>> requirements.
>=20
> The expectation here is that the pg_monitor role will include all of the
> reasonable privileges in a given release that a monitor tool should be
> using.

Exactly.

>=20
> While it's likely that monitoring tools will not all be able to work
> with every function they would then have access to on day 1, I'm sure
> the goal for all of them will be to reach the point where they are using
> all of the functions and tables they then have access to.

Right - and we have discussed here, in at least one developer meeting, and =
with the tools team at EDB what should be included, so I'm confident we hav=
e a solid starting point that would be generally useful.

>=20
> Moving forward, the privileges being added in future versions of PG (the
> ones you point out as happening at initdb-time) would be only those
> which are associated with new features in those later versions of PG.
> Those new features aren't going to be back-patched and therefore it
> wouldn't be possible for the tool to provide a script to GRANT access to
> those new features which would work on older versions of PG, meaning
> that the script you suggest would necessairly be PG-version specific.

Indeed.

Another issue with having the admin run a script is that any new databases =
created from template0 or the default template1, may all need the script to=
 be run at that point. Admin tools that can automatically and immediately s=
tart monitoring new databases may need manual admin/dbowner intervention wh=
ich could be extremely painful in large environments.

I believe this and other reasons we've described are exactly why other DBMS=
' do what we're proposing.

>=20
> In other words, I don't believe this is a valid concern.
>=20
>> So I'm fine with this:
>>=20
>> -    if (tblspcOid !=3D MyDatabaseTableSpace)
>> +    if (tblspcOid !=3D MyDatabaseTableSpace &&
>> +        !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
>>=20
>> But I don't like this:
>>=20
>> +GRANT EXECUTE ON FUNCTION pgstattuple(text) TO pg_read_all_stats;
>=20
> I believe we understood that to be the case.  I don't object to the
> clarification, of course.
>=20
>> My position is that execute permission on a function is already
>> grantable, so granting it by default to a built-in role is just
>> removing flexibility which would otherwise be available to the user.
>=20
> To remove flexibility would require that we remove the ability to
> independently GRANT that right.  We are not doing that.  Nor are we
> taking anything away from the user by added a new default role- we
> already claimed the pg_ namespace for roles in 9.6.

Right - and if a user doesn't like it, they can easily just not use the def=
ault role and create their own alternative instead.

From a usability perspective, I cannot see any downsides to the default rol=
es as proposed. They take nothing away from the users that don't want/need =
them, and add significant convenience for those that do.

--=20
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers