SET ROLE documentation improvement

Started by Yurii Rashkovskiiover 2 years ago22 messages
#1Yurii Rashkovskii
yrashk@gmail.com
1 attachment(s)

Hi,

I believe SET ROLE documentation makes a slightly incomplete statement
about what happens when a superuser uses SET ROLE.

The documentation reading suggests that the superuser would lose all their
privileges. However, they still retain the ability to use `SET ROLE` again.

The attached patch adds this bit to the documentation.

--
Y.

Attachments:

0001-Minor-improvement-to-SET-ROLE-documentation.patchapplication/octet-stream; name=0001-Minor-improvement-to-SET-ROLE-documentation.patchDownload
From 087dbd31aed42ca7b265b072702faf622779bfb6 Mon Sep 17 00:00:00 2001
From: Yurii Rashkovskii <yrashk@gmail.com>
Date: Fri, 15 Sep 2023 11:18:48 -0700
Subject: [PATCH] Minor improvement to SET ROLE documentation.

This helps indicating that even though `SET ROLE` made by a superuser
will make that superuser to lose their privileges, there's still one
they retain, and that is the ability to `SET ROLE` again.
---
 doc/src/sgml/ref/set_role.sgml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
index 13bad1bf66..71de393db6 100644
--- a/doc/src/sgml/ref/set_role.sgml
+++ b/doc/src/sgml/ref/set_role.sgml
@@ -88,7 +88,9 @@ RESET ROLE
 
   <para>
    Note that when a superuser chooses to <command>SET ROLE</command> to a
-   non-superuser role, they lose their superuser privileges.
+   non-superuser role, they lose their superuser privileges, except for
+   the privilege to change to another role again using <command>SET ROLE</command>
+   or <command>RESET ROLE</command>.
   </para>
 
   <para>
-- 
2.33.0

#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Yurii Rashkovskii (#1)
Re: SET ROLE documentation improvement

On Fri, Sep 15, 2023 at 11:26:16AM -0700, Yurii Rashkovskii wrote:

I believe SET ROLE documentation makes a slightly incomplete statement
about what happens when a superuser uses SET ROLE.

The documentation reading suggests that the superuser would lose all their
privileges. However, they still retain the ability to use `SET ROLE` again.

The attached patch adds this bit to the documentation.

IMO this is arguably covered by the following note:

The specified <replaceable class="parameter">role_name</replaceable>
must be a role that the current session user is a member of.
(If the session user is a superuser, any role can be selected.)

But I don't see a big issue with clarifying things further as you propose.

I think another issue is that the aforementioned note doesn't mention the
new SET option added in 3d14e17.

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

#3Yurii Rashkovskii
yrashk@gmail.com
In reply to: Nathan Bossart (#2)
Re: SET ROLE documentation improvement

On Fri, Sep 15, 2023 at 1:47 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

On Fri, Sep 15, 2023 at 11:26:16AM -0700, Yurii Rashkovskii wrote:

I believe SET ROLE documentation makes a slightly incomplete statement
about what happens when a superuser uses SET ROLE.

The documentation reading suggests that the superuser would lose all

their

privileges. However, they still retain the ability to use `SET ROLE`

again.

The attached patch adds this bit to the documentation.

IMO this is arguably covered by the following note:

The specified <replaceable class="parameter">role_name</replaceable>
must be a role that the current session user is a member of.
(If the session user is a superuser, any role can be selected.)

I agree that this may be considered sufficient coverage, but I believe that
giving contextual clarification goes a long way to help people understand.
Documentation reading can be challenging.

But I don't see a big issue with clarifying things further as you propose.

I think another issue is that the aforementioned note doesn't mention the
new SET option added in 3d14e17.

How do you think we should word it in that note to make it useful?

--
Y.

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Yurii Rashkovskii (#3)
Re: SET ROLE documentation improvement

On Fri, Sep 15, 2023 at 02:36:16PM -0700, Yurii Rashkovskii wrote:

On Fri, Sep 15, 2023 at 1:47 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

I think another issue is that the aforementioned note doesn't mention the
new SET option added in 3d14e17.

How do you think we should word it in that note to make it useful?

Maybe something like this:

The current session user must have the SET option for the specified
role_name, either directly or indirectly via a chain of memberships
with the SET option.

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

#5Yurii Rashkovskii
yrashk@gmail.com
In reply to: Nathan Bossart (#4)
1 attachment(s)
Re: SET ROLE documentation improvement

On Mon, Sep 25, 2023 at 3:09 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

On Fri, Sep 15, 2023 at 02:36:16PM -0700, Yurii Rashkovskii wrote:

On Fri, Sep 15, 2023 at 1:47 PM Nathan Bossart <nathandbossart@gmail.com

wrote:

I think another issue is that the aforementioned note doesn't mention

the

new SET option added in 3d14e17.

How do you think we should word it in that note to make it useful?

Maybe something like this:

The current session user must have the SET option for the specified
role_name, either directly or indirectly via a chain of memberships
with the SET option.

This is a good start, indeed. I've amended my patch to include it.

--
Y.

Attachments:

V2-0001-Minor-improvement-to-SET-ROLE-documentation.patchapplication/octet-stream; name=V2-0001-Minor-improvement-to-SET-ROLE-documentation.patchDownload
From 923a44997b38399d9e8a3ffaaa9e5b606c948b22 Mon Sep 17 00:00:00 2001
From: Yurii Rashkovskii <yrashk@gmail.com>
Date: Fri, 15 Sep 2023 11:18:48 -0700
Subject: [PATCH] Minor improvement to SET ROLE documentation.

This helps indicating that even though `SET ROLE` made by a superuser
will make that superuser to lose their privileges, there's still one
they retain, and that is the ability to `SET ROLE` again.
---
 doc/src/sgml/ref/set_role.sgml | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
index 13bad1bf66..808a99e8fc 100644
--- a/doc/src/sgml/ref/set_role.sgml
+++ b/doc/src/sgml/ref/set_role.sgml
@@ -42,9 +42,12 @@ RESET ROLE
 
   <para>
    The specified <replaceable class="parameter">role_name</replaceable>
-   must be a role that the current session user is a member of.
-   (If the session user is a superuser, any role can be selected.)
-  </para>
+   must be a role that the current session user is a member of. The current
+   session user must have the SET option for the specified
+   <replaceable class="parameter">role_name</replaceable>, either directly
+   or indirectly via a chain of memberships with the SET option. (If the
+   session user is a superuser, any role can be selected.)
+   </para>
 
   <para>
    The <literal>SESSION</literal> and <literal>LOCAL</literal> modifiers act the same
@@ -88,7 +91,9 @@ RESET ROLE
 
   <para>
    Note that when a superuser chooses to <command>SET ROLE</command> to a
-   non-superuser role, they lose their superuser privileges.
+   non-superuser role, they lose their superuser privileges, except for
+   the privilege to change to another role again using <command>SET ROLE</command>
+   or <command>RESET ROLE</command>.
   </para>
 
   <para>
-- 
2.33.0

#6Nathan Bossart
nathandbossart@gmail.com
In reply to: Yurii Rashkovskii (#5)
Re: SET ROLE documentation improvement

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>

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

#7Shubham Khanna
khannashubham1197@gmail.com
In reply to: Nathan Bossart (#6)
Re: SET ROLE documentation improvement

On Fri, Nov 10, 2023 at 11:11 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:

Show quoted text

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 no Spell-check issue also. I did not find any issues. The patch looks good to me.

Thanks and Regards,
Shubham Khanna.

#8Robert Haas
robertmhaas@gmail.com
In reply to: Nathan Bossart (#6)
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

#9Nathan Bossart
nathandbossart@gmail.com
In reply to: Robert Haas (#8)
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

#10Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#9)
Re: SET ROLE documentation improvement

On Fri, Mar 22, 2024 at 03:45:06PM -0500, Nathan Bossart wrote:

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:

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.

Actually, shouldn't this one be back-patched to v16? If so, I'd do that
one separately from the other changes we are discussing.

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

#11Robert Haas
robertmhaas@gmail.com
In reply to: Nathan Bossart (#10)
Re: SET ROLE documentation improvement

On Fri, Mar 22, 2024 at 4:51 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

Actually, shouldn't this one be back-patched to v16? If so, I'd do that
one separately from the other changes we are discussing.

Sure, that seems fine.

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

#12Nathan Bossart
nathandbossart@gmail.com
In reply to: Robert Haas (#11)
Re: SET ROLE documentation improvement

On Sat, Mar 23, 2024 at 08:37:20AM -0400, Robert Haas wrote:

On Fri, Mar 22, 2024 at 4:51 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

Actually, shouldn't this one be back-patched to v16? If so, I'd do that
one separately from the other changes we are discussing.

Sure, that seems fine.

Committed that part.

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

#13Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Nathan Bossart (#12)
Re: SET ROLE documentation improvement

On 24 Mar 2024, at 23:34, Nathan Bossart <nathandbossart@gmail.com> wrote:

Committed that part.

Hi Nathan and Yurii!

Can I ask you please to help me with determining status of CF item [0]https://commitfest.postgresql.org/47/4572/. Is it committed or there's something to move to next CF?

Thanks!

Best regards, Andrey Borodin.

[0]: https://commitfest.postgresql.org/47/4572/

#14Michael Paquier
michael@paquier.xyz
In reply to: Andrey M. Borodin (#13)
Re: SET ROLE documentation improvement

On Tue, Apr 09, 2024 at 09:21:39AM +0300, Andrey M. Borodin wrote:

Can I ask you please to help me with determining status of CF item
[0]. Is it committed or there's something to move to next CF?

Only half of the patch has been applied as of 3330a8d1b792. Yurii and
Nathan, could you follow up with the rest? Moving the patch to the
next CF makes sense, and the last thread update is rather recent.
--
Michael

#15Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#14)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 10:33:15AM +0900, Michael Paquier wrote:

On Tue, Apr 09, 2024 at 09:21:39AM +0300, Andrey M. Borodin wrote:

Can I ask you please to help me with determining status of CF item
[0]. Is it committed or there's something to move to next CF?

Only half of the patch has been applied as of 3330a8d1b792. Yurii and
Nathan, could you follow up with the rest? Moving the patch to the
next CF makes sense, and the last thread update is rather recent.

AFAICT there are two things remaining:

* Remove the "they lose their superuser privileges" note.
* Note that SET ROLE and SET SESSION AUTHORIZATION are exceptions.

While I think these are good changes, I don't sense any urgency here, so
I'm treating this as v18 material at this point.

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

#16Robert Haas
robertmhaas@gmail.com
In reply to: Nathan Bossart (#15)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 10:03 AM Nathan Bossart
<nathandbossart@gmail.com> wrote:

On Thu, Apr 11, 2024 at 10:33:15AM +0900, Michael Paquier wrote:

On Tue, Apr 09, 2024 at 09:21:39AM +0300, Andrey M. Borodin wrote:

Can I ask you please to help me with determining status of CF item
[0]. Is it committed or there's something to move to next CF?

Only half of the patch has been applied as of 3330a8d1b792. Yurii and
Nathan, could you follow up with the rest? Moving the patch to the
next CF makes sense, and the last thread update is rather recent.

AFAICT there are two things remaining:

* Remove the "they lose their superuser privileges" note.
* Note that SET ROLE and SET SESSION AUTHORIZATION are exceptions.

While I think these are good changes, I don't sense any urgency here, so
I'm treating this as v18 material at this point.

I suggest that we close the existing CF entry as committed and
somebody can start a new one for whatever remains. I think that will
be less confusing.

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

#17Nathan Bossart
nathandbossart@gmail.com
In reply to: Robert Haas (#16)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 11:36:52AM -0400, Robert Haas wrote:

I suggest that we close the existing CF entry as committed and
somebody can start a new one for whatever remains. I think that will
be less confusing.

Done: https://commitfest.postgresql.org/48/4923/.

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

#18Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#17)
1 attachment(s)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 11:48:30AM -0500, Nathan Bossart wrote:

On Thu, Apr 11, 2024 at 11:36:52AM -0400, Robert Haas wrote:

I suggest that we close the existing CF entry as committed and
somebody can start a new one for whatever remains. I think that will
be less confusing.

Done: https://commitfest.postgresql.org/48/4923/.

While it's fresh on my mind, I very hastily hacked together a draft of what
I believe is the remaining work.

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

Attachments:

v3-0001-further-improvements-to-SET-ROLE-docs.patchtext/x-diff; charset=us-asciiDownload
From bb3aa06b6e55b403489afafcc8b7608516fd7b40 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Thu, 11 Apr 2024 12:01:21 -0500
Subject: [PATCH v3 1/1] further improvements to SET ROLE docs

---
 doc/src/sgml/ref/set_role.sgml | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
index 083e6dc6ea..9557bb77ab 100644
--- a/doc/src/sgml/ref/set_role.sgml
+++ b/doc/src/sgml/ref/set_role.sgml
@@ -37,7 +37,10 @@ RESET ROLE
    written as either an identifier or a string literal.
    After <command>SET ROLE</command>, permissions checking for SQL commands
    is carried out as though the named role were the one that had logged
-   in originally.
+   in originally.  Note that <command>SET ROLE</command> and
+   <command>SET SESSION AUTHORIZATION</command> are exceptions; permissions
+   checks for those continue to use the current session user and the initial
+   session user (the <firstterm>authenticated user</firstterm>), respectively.
   </para>
 
   <para>
@@ -88,11 +91,6 @@ RESET ROLE
    exercised either with or without <literal>SET ROLE</literal>.
   </para>
 
-  <para>
-   Note that when a superuser chooses to <command>SET ROLE</command> to a
-   non-superuser role, they lose their superuser privileges.
-  </para>
-
   <para>
    <command>SET ROLE</command> has effects comparable to
    <link linkend="sql-set-session-authorization"><command>SET SESSION AUTHORIZATION</command></link>, but the privilege
-- 
2.25.1

#19Robert Haas
robertmhaas@gmail.com
In reply to: Nathan Bossart (#18)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 1:03 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

On Thu, Apr 11, 2024 at 11:48:30AM -0500, Nathan Bossart wrote:

On Thu, Apr 11, 2024 at 11:36:52AM -0400, Robert Haas wrote:

I suggest that we close the existing CF entry as committed and
somebody can start a new one for whatever remains. I think that will
be less confusing.

Done: https://commitfest.postgresql.org/48/4923/.

While it's fresh on my mind, I very hastily hacked together a draft of what
I believe is the remaining work.

That looks fine to me. And if others agree, I think it's fine to just
commit this now, post-freeze. It's only a doc change, and a
back-patchable one if you want to go that route.

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

#20Nathan Bossart
nathandbossart@gmail.com
In reply to: Robert Haas (#19)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 01:38:37PM -0400, Robert Haas wrote:

On Thu, Apr 11, 2024 at 1:03 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

While it's fresh on my mind, I very hastily hacked together a draft of what
I believe is the remaining work.

That looks fine to me. And if others agree, I think it's fine to just
commit this now, post-freeze. It's only a doc change, and a
back-patchable one if you want to go that route.

No objections here. I'll give this a few days for others to comment. I'm
not particularly interested in back-patching this since it's arguably not
fixing anything that's incorrect, but if anyone really wants me to, I will.

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

#21Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#20)
Re: SET ROLE documentation improvement

On Thu, Apr 11, 2024 at 02:21:49PM -0500, Nathan Bossart wrote:

No objections here. I'll give this a few days for others to comment. I'm
not particularly interested in back-patching this since it's arguably not
fixing anything that's incorrect, but if anyone really wants me to, I will.

HEAD looks fine based on what I'm reading in the patch. If there are
more voices in favor of a backpatch, it could always happen later.
--
Michael

#22Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#21)
Re: SET ROLE documentation improvement

On Fri, Apr 12, 2024 at 09:54:24AM +0900, Michael Paquier wrote:

On Thu, Apr 11, 2024 at 02:21:49PM -0500, Nathan Bossart wrote:

No objections here. I'll give this a few days for others to comment. I'm
not particularly interested in back-patching this since it's arguably not
fixing anything that's incorrect, but if anyone really wants me to, I will.

HEAD looks fine based on what I'm reading in the patch. If there are
more voices in favor of a backpatch, it could always happen later.

Committed.

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