Hi,
On Fri, Jan 17, 2025 at 10:03:17AM +0100, Laurenz Albe wrote:
> 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.
Thanks! And sorry for the long delay...
> > --- 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.
Right, I had already changed that in my tree but never sent out an
updated version with this.
> Should we say "creating, altering or dropping extensions" to make the
> connection to
> the respective commands obvious?
Alright, did so.
> > --- 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.
Done.
> > - : 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.
We actually do, I think I modelled it after other predefined roles,
e.g.:
|src/backend/commands/subscriptioncmds.c:
errdetail("Only roles with privileges of the \"%s\" role may create
subscriptions.",
|src/backend/commands/subscriptioncmds.c-
"pg_create_subscription")));
|--
|src/backend/commands/copy.c:
errdetail("Only roles with privileges of the \"%s\" role may COPY to or from an
external program.",
|src/backend/commands/copy.c-
"pg_execute_server_program"),
|--
|src/backend/commands/copy.c:
errdetail("Only roles with privileges of the \"%s\" role may COPY from a file.",
|src/backend/commands/copy.c-
"pg_read_server_files"),
|--
|src/backend/commands/copy.c:
errdetail("Only roles with privileges of the \"%s\" role may COPY to a file.",
|src/backend/commands/copy.c-
"pg_write_server_files"),
However, those are all errdetail, while the previous "Must be superuser
to [...]" is errhint, and that error message has different hints based
on whether the extension is trusted or not...
> 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.")));
Alright, I think it makes more sense to have it like that than the
above, so changed it to that.
New version 3 attached.
Michael
>From 57e02d95ace2a390ea1e40266735529f313b31ec Mon Sep 17 00:00:00 2001
From: Michael Banck <[email protected]>
Date: Thu, 31 Oct 2024 22:36:12 +0100
Subject: [PATCH v3] Add new pg_manage_extensions predefined role.
This allows any role that is granted this new predefined role to CREATE, UPDATE
or DROP extensions, no matter whether they are trusted or not.
---
doc/src/sgml/user-manag.sgml | 11 +++++++++++
src/backend/commands/extension.c | 11 ++++++-----
src/include/catalog/pg_authid.dat | 5 +++++
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index ed18704a9c2..2a44f41da4f 100644
--- 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, altering or
+ dropping extensions, even if the extensions are untrusted or the user
+ does not have <literal>CREATE</literal> rights on the database.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="predefined-role-pg-monitor" xreflabel="pg_monitor">
<term><varname>pg_monitor</varname></term>
<term><varname>pg_read_all_settings</varname></term>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index d9bb4ce5f1e..2426c3e0eda 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -996,13 +996,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.
*/
if (control->superuser && !superuser())
{
- if (extension_is_trusted(control))
+ if (extension_is_trusted(control) || has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_EXTENSIONS))
switch_to_superuser = true;
else if (from_version == NULL)
ereport(ERROR,
@@ -1011,7 +1012,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
control->name),
control->trusted
? errhint("Must have CREATE privilege on current database to create this extension.")
- : errhint("Must be superuser to create this extension.")));
+ : errhint("Must be superuser or member of pg_manage_extensions to create this extension.")));
else
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1019,7 +1020,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
control->name),
control->trusted
? errhint("Must have CREATE privilege on current database to update this extension.")
- : errhint("Must be superuser to update this extension.")));
+ : errhint("Must be superuser or member of pg_manage_extensions to update this extension.")));
}
filename = get_extension_script_filename(control, from_version, version);
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index eb4dab5c6aa..0e3eb20e2b9 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -104,5 +104,10 @@
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '8801', oid_symbol => 'ROLE_PG_MANAGE_EXTENSIONS',
+ rolname => 'pg_manage_extensions', rolsuper => 'f', rolinherit => 't',
+ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+ rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+ rolpassword => '_null_', rolvaliduntil => '_null_' },
]
--
2.39.5
