Re: Read access for pg_monitor to pg_replication_origin_status view

Hi,

> > $ git diff
> > diff --git a/src/backend/catalog/system_views.sql
> > b/src/backend/catalog/system_views.sql
> > index c16061f8f00..97ee72a9cfc 100644
> > --- a/src/backend/catalog/system_views.sql
> > +++ b/src/backend/catalog/system_views.sql
> > @@ -1494,9 +1494,6 @@ GRANT EXECUTE ON FUNCTION
> > pg_ls_archive_statusdir() TO pg_monitor;
> > GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor;
> > GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
> >
> > -GRANT EXECUTE ON FUNCTION pg_replication_origin_progress(text,
> > boolean) TO pg_monitor;
> > -GRANT EXECUTE ON FUNCTION
> > pg_replication_origin_session_progress(boolean) TO pg_monitor;
> > -
> > GRANT pg_read_all_settings TO pg_monitor;
> > GRANT pg_read_all_stats TO pg_monitor;
> > GRANT pg_stat_scan_tables TO pg_monitor;
>
>
> Agreed on this part. The two functions aren't needed to be granted.
>
> But, pg_show_replication_origin_status() should be allowed
> pg_read_all_stats, not pg_monitor. pg_monitor is just a union role of
> actual privileges.

I placed that GRANT on purpose to `pg_monitor`, separated from the
`pg_read_all_stats` role, because it doesn't match the description for
that role.

```
Read all pg_stat_* views and use various statistics related
extensions, even those normally visible only to superusers.
```

I have no problem adding it to this ROLE, but we'd have to amend the
doc for default-roles to reflect that SELECT for this view is also
granted to `pg_read_all_stats`.

> Another issue would be how to control output of
> pg_show_replication_origin_status(). Most of functions that needs
> pg_read_all_stats privileges are filtering sensitive columns in each
> row, instead of hiding the existence of rows. Maybe the view
> pg_replication_origin_status should show only local_id and hide other
> columns from non-pg_read_all_stats users.

I think that the output from `pg_show_replication_origin_status()`
doesn't expose any data that `pg_read_all_stats` or `pg_monitor`
shouldn't be able to read. Removing or obfuscating `external_id`
and/or `remote_lsn` would make the view somehow pointless, in
particular for monitoring and diagnostic tools.

I'll upload new patches shortly following Michael's suggestions.

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services