Re: User with BYPASSRLS privilege can't change password

Wolfgang Walther <walther@technowledgy.de> writes:
> Tom Lane:
>> so AFAICS it's impossible to get there.  If it isn't impossible,
>> we have a much bigger hole with respect to issuper.

> Yes, you're right. I read the || as &&. And also missed the ! in else if 
> (!have_createrole_privilege()) btw. :)

Actually the right way to deal with this potential confusion is to
add a comment, as below.  I fixed the docs too.

> I guess the error message "must be superuser to alter replication users" 
> led me on the wrong path. I would be more precise as "must be superuser 
> to alter replication users or change replication attribute" to cover the 
> change-non-replication-to-replication user case, I think. The same thing 
> for superusers.

I'd be okay with changing the error text in HEAD, but less so in the back
branches, since that'd cause thrashing of translatable strings.

            regards, tom lane

diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index aef30521bc..5aa5648ae7 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -71,7 +71,9 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
    Attributes not mentioned in the command retain their previous settings.
    Database superusers can change any of these settings for any role.
    Roles having <literal>CREATEROLE</literal> privilege can change any of these
-   settings, but only for non-superuser and non-replication roles.
+   settings except <literal>SUPERUSER</literal>, <literal>REPLICATION</literal>,
+   and <literal>BYPASSRLS</literal>; but only for non-superuser and
+   non-replication roles.
    Ordinary roles can only change their own password.
   </para>

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 9ce9a66921..293e7e4c0c 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -709,8 +709,10 @@ AlterRole(AlterRoleStmt *stmt)
     roleid = authform->oid;

     /*
-     * To mess with a superuser you gotta be superuser; else you need
-     * createrole, or just want to change your own password
+     * To mess with a superuser or replication role in any way you gotta be
+     * superuser.  We also insist on superuser to change the BYPASSRLS
+     * property.  Otherwise, if you don't have createrole, you're only allowed
+     * to change your own password.
      */
     if (authform->rolsuper || issuper >= 0)
     {
@@ -726,7 +728,7 @@ AlterRole(AlterRoleStmt *stmt)
                     (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                      errmsg("must be superuser to alter replication users")));
     }
-    else if (authform->rolbypassrls || bypassrls >= 0)
+    else if (bypassrls >= 0)
     {
         if (!superuser())
             ereport(ERROR,
@@ -735,11 +737,11 @@ AlterRole(AlterRoleStmt *stmt)
     }
     else if (!have_createrole_privilege())
     {
+        /* We already checked issuper, isreplication, and bypassrls */
         if (!(inherit < 0 &&
               createrole < 0 &&
               createdb < 0 &&
               canlogin < 0 &&
-              isreplication < 0 &&
               !dconnlimit &&
               !rolemembers &&
               !validUntil &&