Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
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: 2021-10-13 09:06:47
Message-ID: CALj2ACU0GJ1hBRDR1F5FCjxaaw=8z0o0um-oBu5OHBziFruesA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 13, 2021 at 2:19 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> Greeting,
>
> On Wed, Oct 13, 2021 at 04:14 Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> On Wed, Oct 13, 2021 at 1:24 PM Michael Paquier <michael(at)paquier(dot)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.

Hm. Let's deal with it separately, if required.

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

I will remove the superuser() for pg_log_backend_memory_context alone
here in the next version of patch.

Regards,
Bharath Rupireddy.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2021-10-13 09:48:08 Re: RFC: compression dictionaries for JSONB
Previous Message tanghy.fnst@fujitsu.com 2021-10-13 08:57:00 RE: Added schema level support for publication.