pgsql: Allow GRANT on pg_log_backend_memory_contexts().

On Tue, Oct 26, 2021 at 08:41:11PM +0000, Jeff Davis wrote:

Allow GRANT on pg_log_backend_memory_contexts().

Remove superuser check, allowing any user granted permissions on
pg_log_backend_memory_contexts() to log the memory contexts of any
backend.

Note that this could allow a privileged non-superuser to log the
memory contexts of a superuser backend, but as discussed, that does
not seem to be a problem.

but will not be sent to the client regardless of
<xref linkend="guc-client-min-messages"/>.
- Only superusers can request to log the memory contexts.
</para></entry>
I don't think that the documentation part of this commit is correct.
This sentence should have been changed to something like the
following:
"This function is restricted to superusers by default, but other users
can be granted EXECUTE to run the function."

Thanks,
--
Michael

On Wed, Oct 27, 2021 at 9:45 AM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Oct 26, 2021 at 08:41:11PM +0000, Jeff Davis wrote:

Allow GRANT on pg_log_backend_memory_contexts().

Remove superuser check, allowing any user granted permissions on
pg_log_backend_memory_contexts() to log the memory contexts of any
backend.

Note that this could allow a privileged non-superuser to log the
memory contexts of a superuser backend, but as discussed, that does
not seem to be a problem.

but will not be sent to the client regardless of
<xref linkend="guc-client-min-messages"/>.
- Only superusers can request to log the memory contexts.
</para></entry>
I don't think that the documentation part of this commit is correct.
This sentence should have been changed to something like the
following:
"This function is restricted to superusers by default, but other users
can be granted EXECUTE to run the function."

+1 and the above statement looks good. Apart from that I have one more
suggestion that I earlier made: have the function produce the error
when no permissions were granted (just to ensure we have the error
message covered) including has_function_privilege() case.

Attached a patch with the above documentation change and the error
case added. Please review it.

Regards,
Bharath Rupireddy.

On Wed, Oct 27, 2021 at 10:27:18AM +0530, Bharath Rupireddy wrote:

+1 and the above statement looks good. Apart from that I have one more
suggestion that I earlier made: have the function produce the error
when no permissions were granted (just to ensure we have the error
message covered) including has_function_privilege() case.

has_function_privilege() makes sure of the same thing, so I think that
what Jeff has done for this part is just but fine. There is no need
for more duplication in the tests.
--
Michael

On Wed, Oct 27, 2021 at 10:33 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Oct 27, 2021 at 10:27:18AM +0530, Bharath Rupireddy wrote:

+1 and the above statement looks good. Apart from that I have one more
suggestion that I earlier made: have the function produce the error
when no permissions were granted (just to ensure we have the error
message covered) including has_function_privilege() case.

has_function_privilege() makes sure of the same thing, so I think that
what Jeff has done for this part is just but fine. There is no need
for more duplication in the tests.

Fair enough.

Regards,
Bharath Rupireddy.

On Wed, 2021-10-27 at 13:15 +0900, Michael Paquier wrote:

I don't think that the documentation part of this commit is correct.
This sentence should have been changed to something like the
following:
"This function is restricted to superusers by default, but other
users
can be granted EXECUTE to run the function."

Above the table of functions it already says:

"Use of these functions is restricted to superusers by default but
access may be granted to others using GRANT, with noted exceptions."

It looks like several people missed that, so perhaps we should get rid
of that statement at the top, and move it in to each function
description?

Regards,
Jeff Davis

On Wed, Oct 27, 2021 at 08:12:23AM -0700, Jeff Davis wrote:

It looks like several people missed that, so perhaps we should get rid
of that statement at the top, and move it in to each function
description?

Oops, you are right, sorry about the useless noise. With a table that
includes all the functions and a description at the top, it looks
fine. Thanks!
--
Michael

On 2021-Oct-27, Jeff Davis wrote:

Above the table of functions it already says:

"Use of these functions is restricted to superusers by default but
access may be granted to others using GRANT, with noted exceptions."

It looks like several people missed that, so perhaps we should get rid
of that statement at the top, and move it in to each function
description?

Looking at the output in the website, I agree it looks OK as you left
it and no further patching is necessary.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/