Re: SET ROLE documentation improvement

On Fri, Mar 22, 2024 at 03:58:59PM -0400, Robert Haas wrote:
> On Fri, Nov 10, 2023 at 12:41 PM Nathan Bossart
> <nathandbossart@gmail.com> wrote:
>> 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."

+1.  IMHO these kinds of special mentions of SUPERUSER tend to be
redundant, and, as evidenced by this thread, confusing.  I'll update the
patch.

> 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.

+1

>> 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.

Cool.

> 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.

That seems like a resonable idea, although it might require a few rounds of
wordsmithing.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com