Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr
Date
Msg-id CAOuzzgpp0dmOFjWC4JDvk57ZQGm8umCrFdR1at4b80xuF0XChw@mail.gmail.com
Whole thread Raw
In response to Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
Greeting,

On Wed, Oct 13, 2021 at 04:14 Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
On Wed, Oct 13, 2021 at 1:24 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Oct 13, 2021 at 11:15:16AM +0530, Bharath Rupireddy wrote:
> > IMO, we can just retain the "if (!superuser())" check in the
> > pg_log_backend_memory_contexts as is. This would be more meaningful as
> > the error "must be superuser to use raw page functions" explicitly
> > says that a superuser is allowed. Whereas if we revoke the permissions
> > in system_views.sql, then the error we get is not meaningful as the
> > error "permission denied for function pg_log_backend_memory_contexts"
> > says that permissions denied and the user will have to look at the
> > documentation for what permissions this function requires.
>
> I don't really buy this argument with the "superuser" error message.
> When removing hardcoded superuser(), we just close the gap by adding
> in the documentation that the function execution can be granted
> afterwards.  And nobody has complained about the difference in error
> message AFAIK.  That's about extensibility.

I'm not against removing superuser() check in the
pg_log_backend_memory_contexts. However, there are a lot of functions
with the "must be superuser to XXXXX" kind of error [1]. I'm worried
if someone proposes to change these as well with what we do for
pg_log_backend_memory_contexts.

brin_page_type
brin_page_items
brin_metapage_info
brin_revmap_data
bt_page_stats_internal
bt_page_items_internal
bt_page_items_bytea
bt_metap
fsm_page_contents
gin_metapage_info
gin_page_opaque_info
and the list goes on.

Yes, would generally be good to change at least some of those also, perhaps all of them. 

Not sure I see what the argument here is. We should really be trying to move away from explicit superuser checks. 

Thanks.

Stephen

pgsql-hackers by date:

Previous
From: Sergey Shinderuk
Date:
Subject: Re: Bug in DefineRange() with multiranges
Next
From: "tanghy.fnst@fujitsu.com"
Date:
Subject: RE: Added schema level support for publication.