On Fri, Nov 10, 2023 at 11:11 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. If we changed it to
>
> Note that when a superuser chooses to SET ROLE to a non-superuser role,
> they lose their superuser privileges, except for the privilege to
> change to another role again using SET ROLE or RESET ROLE.
>
> it almost seems to imply that a non-superuser role could obtain the ability
> to switch to any role if they first SET ROLE to a superuser. In practice,
> that's true because they could just give the session role SUPERUSER, but I
> don't think that's the intent of this section.
>
> I thought about changing it to something like
>
> Note that when a superuser chooses to SET ROLE to a non-superuser role,
> they lose their superuser privileges. However, if the current session
> user is a superuser, they retain the ability to set the current user
> identifier to any role via SET ROLE and RESET ROLE.
>
> but it seemed weird to me to single out superusers here when it's always
> true that the current session user retains the ability to SET ROLE to any
> role they have the SET option on. That is already covered above in the
> "Description" section, so I don't really see the need to belabor the point
> by adding qualifications to the "Notes" section. ISTM the point of these
> couple of paragraphs in the "Notes" section is to explain the effects on
> privileges for schemas, tables, etc.
>
> 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>
>
> --
> I have Reviewed the patch. Patch applies neatly without any issues. Documentation build was successful and there was
noSpell-check issue also. I did not find any issues. The patch looks good to me.
>
>Thanks and Regards,
>Shubham Khanna.