Re: ALTER ROLE documentation improvement
On Thu, Jan 18, 2024 at 02:44:35PM -0700, David G. Johnston wrote: > LGTM too. I didn't go looking for anything else related to this, but the > proposed changes all look needed. Committed. Thanks! -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: ALTER ROLE documentation improvement
On Sun, Jan 14, 2024 at 6:59 PM Nathan Bossart wrote: > On Sun, Jan 14, 2024 at 04:17:41PM +0530, vignesh C wrote: > > The attached v3 version patch has the changes for the same. > > LGTM. I'll wait a little while longer for additional feedback, but if none > materializes, I'll commit this soon. > > LGTM too. I didn't go looking for anything else related to this, but the proposed changes all look needed. David J.
Re: ALTER ROLE documentation improvement
On Sun, Jan 14, 2024 at 04:17:41PM +0530, vignesh C wrote: > The attached v3 version patch has the changes for the same. LGTM. I'll wait a little while longer for additional feedback, but if none materializes, I'll commit this soon. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: ALTER ROLE documentation improvement
On Tue, 26 Sept 2023 at 04:38, Nathan Bossart wrote:
>
> On Fri, Sep 15, 2023 at 02:25:38PM -0700, Yurii Rashkovskii wrote:
> > Thank you for the feedback. I've updated the glossary and updated the
> > terminology to be consistent. Please see the new patch attached.
>
> Thanks for the new version of the patch.
>
> This user owns all system catalog tables in each database. It is also
> the role
> from which all granted permissions originate. Because of these things,
> this
> - role may not be dropped.
> + role may not be dropped. This role must always be a superuser, it can't
> be changed
> + to be a non-superuser.
>
> I think we should move this note to the sentence just below that mentions
> its superuserness. Maybe it could look something like this:
>
> This role also behaves as a normal database superuser, and its
> superuser status cannot be revoked.
Modified
> + Database superusers can change any of these settings for any role, except
> for
> + changing SUPERUSER to NOSUPERUSER
> + for a bootstrap
> superuser.
>
> nitpick: s/a bootstrap superuser/the bootstrap superuser
Modified
> #: commands/user.c:871
> #, c-format
> -msgid "The bootstrap user must have the %s attribute."
> +msgid "The bootstrap superuser must have the %s attribute."
> msgstr "Der Bootstrap-Benutzer muss das %s-Attribut haben."
>
> No need to update the translation files. Those are updated separately in
> the pgtranslation repo.
Removed the translation changes
The attached v3 version patch has the changes for the same.
Regards,
Vignesh
From 0b92a0abfbd759a295eb8d5ec80d9203486a0e46 Mon Sep 17 00:00:00 2001
From: Yurii Rashkovskii
Date: Fri, 15 Sep 2023 11:41:46 -0700
Subject: [PATCH v3] Improve ALTER ROLE documentation to document current
behavior.
Previously, this was possible (assuming current_user is a bootstrap user):
```
ALTER ROLE current_user NOSUPERUSER
```
However, as of 16.0 this is no longer allowed:
```
ERROR: permission denied to alter role
DETAIL: The bootstrap user must have the SUPERUSER attribute.
```
Also, update the term across the board to use "bootstrap superuser"
---
doc/src/sgml/glossary.sgml | 3 ++-
doc/src/sgml/ref/alter_role.sgml | 4 +++-
doc/src/sgml/user-manag.sgml | 2 +-
src/backend/commands/user.c | 2 +-
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/glossary.sgml b/doc/src/sgml/glossary.sgml
index 5815fa4471..5839094fce 100644
--- a/doc/src/sgml/glossary.sgml
+++ b/doc/src/sgml/glossary.sgml
@@ -247,7 +247,8 @@
This role also behaves as a normal
- database superuser.
+ database superuser,
+ and its superuser status cannot be revoked.
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index ab1ee45d54..361bae27c3 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -69,7 +69,9 @@ ALTER ROLE { role_specification | A
GRANT and
REVOKE for that.)
Attributes not mentioned in the command retain their previous settings.
- Database superusers can change any of these settings for any role.
+ Database superusers can change any of these settings for any role, except for
+ changing SUPERUSER to NOSUPERUSER
+ for the bootstrap superuser.
Non-superuser roles having CREATEROLE privilege can
change most of these properties, but only for non-superuser and
non-replication roles for which they have been granted
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 92a299d2d3..1c011ac62b 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -350,7 +350,7 @@ ALTER ROLE myname SET enable_indexscan TO off;
options. Thus, the fact that privileges are not inherited by default nor
is SET ROLE granted by default is a safeguard against
accidents, not a security feature. Also note that, because this automatic
- grant is granted by the bootstrap user, it cannot be removed or changed by
+ grant is granted by the bootstrap superuser, it cannot be removed or changed by
the CREATEROLE user; however, any superuser could
revoke it, modify it, and/or issue additional such grants to other
CREATEROLE users. Whichever CREATEROLE
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 7e81589711..7a9c177b21 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -868,7 +868,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("permission denied to alter role"),
- errdetail("The bootstrap user must have the %s attribute.",
+ errdetail("The bootstrap superuser must have the %s attribute.",
"SUPERUSER")));
new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(should_be_super);
--
2.34.1
Re: ALTER ROLE documentation improvement
On Fri, Sep 15, 2023 at 02:25:38PM -0700, Yurii Rashkovskii wrote: > Thank you for the feedback. I've updated the glossary and updated the > terminology to be consistent. Please see the new patch attached. Thanks for the new version of the patch. This user owns all system catalog tables in each database. It is also the role from which all granted permissions originate. Because of these things, this - role may not be dropped. + role may not be dropped. This role must always be a superuser, it can't be changed + to be a non-superuser. I think we should move this note to the sentence just below that mentions its superuserness. Maybe it could look something like this: This role also behaves as a normal database superuser, and its superuser status cannot be revoked. + Database superusers can change any of these settings for any role, except for + changing SUPERUSER to NOSUPERUSER + for a bootstrap superuser. nitpick: s/a bootstrap superuser/the bootstrap superuser #: commands/user.c:871 #, c-format -msgid "The bootstrap user must have the %s attribute." +msgid "The bootstrap superuser must have the %s attribute." msgstr "Der Bootstrap-Benutzer muss das %s-Attribut haben." No need to update the translation files. Those are updated separately in the pgtranslation repo. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: ALTER ROLE documentation improvement
On Fri, Sep 15, 2023 at 1:53 PM Nathan Bossart wrote: > On Fri, Sep 15, 2023 at 11:46:35AM -0700, Yurii Rashkovskii wrote: > > It appears that 16.0 improved some of the checks in ALTER ROLE. > Previously, > > it was possible to do the following (assuming current_user is a bootstrap > > user): > > > > ``` > > ALTER ROLE current_user NOSUPERUSER > > ``` > > > > As of 16.0, this produces an error: > > > > ``` > > ERROR: permission denied to alter role > > DETAIL: The bootstrap user must have the SUPERUSER attribute. > > ``` > > > > The attached patch documents this behavior by providing a bit more > > clarification to the following statement: > > > > "Database superusers can change any of these settings for any role." > > I think this could also be worth a mention in the glossary [0]. BTW the > glossary calls this role the "bootstrap superuser", but the DETAIL message > calls it the "bootstrap user". Perhaps we should standardize on one name. > > [0] https://www.postgresql.org/docs/devel/glossary.html > > Thank you for the feedback. I've updated the glossary and updated the terminology to be consistent. Please see the new patch attached. -- Y. 0001-Improve-ALTER-ROLE-documentation-to-document-current-v2.patch Description: Binary data
Re: ALTER ROLE documentation improvement
On Fri, Sep 15, 2023 at 11:46:35AM -0700, Yurii Rashkovskii wrote: > It appears that 16.0 improved some of the checks in ALTER ROLE. Previously, > it was possible to do the following (assuming current_user is a bootstrap > user): > > ``` > ALTER ROLE current_user NOSUPERUSER > ``` > > As of 16.0, this produces an error: > > ``` > ERROR: permission denied to alter role > DETAIL: The bootstrap user must have the SUPERUSER attribute. > ``` > > The attached patch documents this behavior by providing a bit more > clarification to the following statement: > > "Database superusers can change any of these settings for any role." I think this could also be worth a mention in the glossary [0]. BTW the glossary calls this role the "bootstrap superuser", but the DETAIL message calls it the "bootstrap user". Perhaps we should standardize on one name. [0] https://www.postgresql.org/docs/devel/glossary.html -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
ALTER ROLE documentation improvement
Hi, It appears that 16.0 improved some of the checks in ALTER ROLE. Previously, it was possible to do the following (assuming current_user is a bootstrap user): ``` ALTER ROLE current_user NOSUPERUSER ``` As of 16.0, this produces an error: ``` ERROR: permission denied to alter role DETAIL: The bootstrap user must have the SUPERUSER attribute. ``` The attached patch documents this behavior by providing a bit more clarification to the following statement: "Database superusers can change any of these settings for any role." -- Y. 0001-Improve-ALTER-ROLE-documentation-to-document-current.patch Description: Binary data
