Re: New default role- 'pg_read_all_data' - Mailing list pgsql-hackers

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, 31 August 2020 02:20, Stephen Frost <sfrost@snowman.net> wrote:

> Greetings,
>
> -   Stephen Frost (sfrost@snowman.net) wrote:
>
> > -   Magnus Hagander (magnus@hagander.net) wrote:
> >
> > > On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost sfrost@snowman.net wrote:
> > >
> > > > -   Magnus Hagander (magnus@hagander.net) wrote:
> > > >
> > > > > Without having actually looked at the code, definite +1 for this feature.
> > > > > It's much requested...
> > > >
> > > > Thanks.
> > > >
> > > > > But, should we also have a pg_write_all_data to go along with it?
> > > >
> > > > Perhaps, but could certainly be a different patch, and it'd need to be
> > > > better defined, it seems to me... read_all is pretty straight-forward
> > > > (the general goal being "make pg_dumpall/pg_dump work"), what would
> > > > write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
> > >
> > > Well, it's pg_write_all_data, so it certainly wouldn't be alter table or
> > > system catalogs.
> > > I'd say insert/update/delete yes.
> > > TRUNCATE is always an outlier.Given it's generally classified as DDL, I
> > > wouldn't include it.
> >
> > Alright, that seems like it'd be pretty easy. We already have a check
> > in pg_class_aclmask to disallow modification of system catalogs w/o
> > being a superuser, so we should be alright to add a similar check for
> > insert/update/delete just below where I added the SELECT check.
> >
> > > > Doesn't seem like you could just declare it to be 'allow pg_restore'
> > > > either, as that might include creating untrusted functions, et al.
> > >
> > > No definitely not. That wouldn't be the usecase at all :)
> >
> > Good. :)
> >
> > > (and fwiw to me the main use case for read_all_data also isn't pg_dump,
> > > because most people using pg_dump are already db owner or higher in my
> > > experience. But it is nice that it helps with that too)
> >
> > Glad to have confirmation that there's other use-cases this helps with.
> > I'll post an updated patch with that added in a day or two.
>
> Here's that updated patch, comments welcome.

Thank you for the updated patch!

Had a quick look on it and nothing stands out.

Also this sub-thread seems to have clearly responded on my early thoughts regarding invoking. Adding part of that
subthreadbellow: 


>> My expectation was not met since in my manual test (unless I made a mistake which is entirely possible), the SELECT
abovedid not fail. The insert did succeed though. 

> That the INSERT worked seems pretty odd- could you post the exact
> changes you've made to the regression tests, or the exact script where
> you aren't seeing what you expect?  I've not been able to reproduce the
> GRANT allowing a user to INSERT into a table.

>>  The first question: Was my expectation wrong?

> If there aren't any other privileges involved, then REVOKE'ing the role
> from a user should prevent that user from being able to SELECT from the
> table.

>> The second question: Is there a privilege that can be granted to regress_priv_user6 that will not permit the select
operationbut will permit the insert operation? If no, should there be one? 

As discussed above, while I was struggling to formulate the thought, Magnus had already proposed pg_write_all_data and
thecommunity had reached a consensus on what it actually means. 

Please find attached a minimal test case and the output of it.

It is obvious that I was confused and added confusion to the thread. Permissions are additive and autonomous. Now it is
ratherclear to me what the expectations are and how the patch should behave. 

To add to my embarrassment, the REVOKE operation emitted a warning which I had clearly missed.

Apologies.

//Georgios

>
> Thanks!
>
> Stephen