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
