public inbox for [email protected]  
help / color / mirror / Atom feed
From: Laurenz Albe <[email protected]>
To: Michael Banck <[email protected]>
To: Jelte Fennema-Nio <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] New predefined role pg_manage_extensions
Date: Fri, 17 Jan 2025 10:03:17 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAGECzQQ2HB85N9PjTAdDTpFCciQmpeE2PcXbc8EKhSF=RPi3fA@mail.gmail.com>
	<[email protected]>
	<[email protected]>

On Thu, 2024-10-31 at 22:47 +0100, Michael Banck wrote:
> Even though there has not been a lot of discussion on this, here is a
> rebased patch.  I have also added it to the upcoming commitfest.

I had a look at the patch.

> --- a/doc/src/sgml/user-manag.sgml
> +++ b/doc/src/sgml/user-manag.sgml
> @@ -669,6 +669,17 @@ GRANT pg_signal_backend TO admin_user;
>       </listitem>
>      </varlistentry>
>  
> +    <varlistentry id="predefined-role-pg-manage-extensions" xreflabel="pg_manage_extensions">
> +     <term><varname>pg_manage_extensions</varname></term>
> +     <listitem>
> +      <para>
> +       <literal>pg_manage_extensions</literal> allows creating, removing or
> +       updating extensions, even if the extensions are untrusted or the user is
> +       not the database owner.
> +      </para>
> +     </listitem>
> +    </varlistentry>
> +

The inaccuracy of the database owner has already been mentioned.

Should we say "creating, altering or dropping extensions" to make the connection to
the respective commands obvious?

> --- a/src/backend/commands/extension.c
> +++ b/src/backend/commands/extension.c
> @@ -994,13 +994,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
>     ListCell   *lc2;
>  
>     /*
> -    * Enforce superuser-ness if appropriate.  We postpone these checks until
> -    * here so that the control flags are correctly associated with the right
> +    * Enforce superuser-ness/membership of the pg_manage_extensions
> +    * predefined role if appropriate.  We postpone these checks until here
> +    * so that the control flags are correctly associated with the right
>      * script(s) if they happen to be set in secondary control files.
>      */

This is just an attempt to improve the English:

  If appropriate, enforce superuser-ness or membership of the predefined role
  pg_manage_extensions.

> -                    : errhint("Must be superuser to create this extension.")));
> +                    : errhint("Only roles with privileges of the \"%s\" role are allowed to create this extension.", "pg_manage_extensions")));

I don't see the point of breaking out the role name from the message.
We don't do that in other places.
Besides, I think that the mention of the superuser should be retained.
Moreover, I think that commit 8d9978a717 pretty much established that we
should not quote names if they contain underscores.
Perhaps:

  errhint("Must be superuser or member of pg_manage_extensions to create this extension.")));

Yours,
Laurenz Albe






view thread (27+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] New predefined role pg_manage_extensions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox