Thread: 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

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 <literal>SUPERUSER</literal> to <literal>NOSUPERUSER</literal>
+   for a <glossterm linkend="glossary-bootstrap-superuser">bootstrap superuser</glossterm>.

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

On Tue, 26 Sept 2023 at 04:38, Nathan Bossart <nathandbossart@gmail.com> 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 <literal>SUPERUSER</literal> to <literal>NOSUPERUSER</literal>
> +   for a <glossterm linkend="glossary-bootstrap-superuser">bootstrap superuser</glossterm>.
>
> 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

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

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