Re: SET ROLE documentation improvement

On Fri, Nov 10, 2023 at 12:41 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:
> On Tue, Sep 26, 2023 at 08:33:25AM -0700, Yurii Rashkovskii wrote:
> > This is a good start, indeed. I've amended my patch to include it.
>
> Thanks for the new patch.
>
> Looking again, I'm kind of hesitant to add too much qualification to this
> note about losing superuser privileges.

The note in question is:

  <para>
   Note that when a superuser chooses to <command>SET ROLE</command> to a
   non-superuser role, they lose their superuser privileges.
  </para>

It's not entirely clear to me what the point of this note is. I think
we could consider removing it entirely, on the theory that it's just
poorly-stated special case of what's already been said in the
description, i.e. "permissions checking for SQL commands is carried
out as though the named role were the one that had logged in
originally" and "The specified <replaceable
class="parameter">role_name</replaceable> must be a role that the
current session user is a member of."

I think it's also possible that what the author of this paragraph
meant was that role attributes like CREATEDB, CREATEROLE, REPLICATION,
and SUPERUSER follow the current user, not the session user. If we
think that was the point of this paragraph, we could make it say that
more clearly. However, I'm not sure that really needs to be mentioned,
because "permissions checking for SQL commands is carried out as
though the named role were the one that had logged in originally"
seems to cover that ground along with everything else.

> I still think we should update the existing note about privileges for
> SET/RESET ROLE to something like the following:
>
> diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
> index 13bad1bf66..c91a95f5af 100644
> --- a/doc/src/sgml/ref/set_role.sgml
> +++ b/doc/src/sgml/ref/set_role.sgml
> @@ -41,8 +41,10 @@ RESET ROLE
>    </para>
>
>    <para>
> -   The specified <replaceable class="parameter">role_name</replaceable>
> -   must be a role that the current session user is a member of.
> +   The current session user must have the <literal>SET</option> for the
> +   specified <replaceable class="parameter">role_name</replaceable>, either
> +   directly or indirectly via a chain of memberships with the
> +   <literal>SET</literal> option.
>     (If the session user is a superuser, any role can be selected.)
>    </para>

This is a good change; I should have done this when SET was added.

Another change we could consider is revising "permissions checking for
SQL commands is carried out as though the named role were the one that
had logged in originally" to mention that SET ROLE and SET SESSION
AUTHORIZATION are exceptions.

--
Robert Haas
EDB: http://www.enterprisedb.com