psql: Add role's membership options to the \du+ command

Started by Pavel Luzanovover 3 years ago78 messageshackers
Jump to latest
#1Pavel Luzanov
p.luzanov@postgrespro.ru

When you include one role in another, you can specify three options:
ADMIN, INHERIT (added in e3ce2de0) and SET (3d14e171).

For example.

CREATE ROLE alice LOGIN;

GRANT pg_read_all_settings TO alice WITH ADMIN TRUE, INHERIT TRUE, SET TRUE;
GRANT pg_stat_scan_tables TO alice WITH ADMIN FALSE, INHERIT FALSE, SET
FALSE;
GRANT pg_read_all_stats TO alice WITH ADMIN FALSE, INHERIT TRUE, SET FALSE;

For information about the options, you need to look in the pg_auth_members:

SELECT roleid::regrole, admin_option, inherit_option, set_option
FROM pg_auth_members
WHERE member = 'alice'::regrole;
        roleid        | admin_option | inherit_option | set_option
----------------------+--------------+----------------+------------
 pg_read_all_settings | t            | t              | t
 pg_stat_scan_tables  | f            | f              | f
 pg_read_all_stats    | f            | t              | f
(3 rows)

I think it would be useful to be able to get this information with a
psql command
like \du (and \dg). With proposed patch the \du command still only lists
the roles of which alice is a member:

\du alice
                                     List of roles
 Role name | Attributes |                          Member of
-----------+------------+--------------------------------------------------------------
 alice     |            |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}

But the \du+ command adds information about the selected ADMIN, INHERIT
and SET options:

\du+ alice
                                    List of roles
 Role name | Attributes |                   Member of                  
| Description
-----------+------------+-----------------------------------------------+-------------
 alice     |            | pg_read_all_settings WITH ADMIN, INHERIT, SET+|
           |            | pg_read_all_stats WITH INHERIT               +|
           |            | pg_stat_scan_tables                           |

One more change. The roles in the "Member of" column are sorted for both
\du+ and \du for consistent output.

Any comments are welcome.

--
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Attachments:

add_role_membership_options_to_du.patchtext/x-patch; charset=UTF-8; name=add_role_membership_options_to_du.patchDownload+59-11
#2Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#1)
Re: psql: Add role's membership options to the \du+ command

Added the patch to the open commitfest:
https://commitfest.postgresql.org/42/4116/

Feel free to reject if it's not interesting.

--
Pavel Luzanov

#3David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#1)
Re: psql: Add role's membership options to the \du+ command

On Mon, Jan 9, 2023 at 9:09 AM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

When you include one role in another, you can specify three options:
ADMIN, INHERIT (added in e3ce2de0) and SET (3d14e171).

For example.

CREATE ROLE alice LOGIN;

GRANT pg_read_all_settings TO alice WITH ADMIN TRUE, INHERIT TRUE, SET
TRUE;
GRANT pg_stat_scan_tables TO alice WITH ADMIN FALSE, INHERIT FALSE, SET
FALSE;
GRANT pg_read_all_stats TO alice WITH ADMIN FALSE, INHERIT TRUE, SET FALSE;

For information about the options, you need to look in the pg_auth_members:

SELECT roleid::regrole, admin_option, inherit_option, set_option
FROM pg_auth_members
WHERE member = 'alice'::regrole;
roleid | admin_option | inherit_option | set_option
----------------------+--------------+----------------+------------
pg_read_all_settings | t | t | t
pg_stat_scan_tables | f | f | f
pg_read_all_stats | f | t | f
(3 rows)

I think it would be useful to be able to get this information with a
psql command
like \du (and \dg). With proposed patch the \du command still only lists
the roles of which alice is a member:

\du alice
List of roles
Role name | Attributes | Member of

-----------+------------+--------------------------------------------------------------
alice | |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}

But the \du+ command adds information about the selected ADMIN, INHERIT
and SET options:

\du+ alice
List of roles
Role name | Attributes | Member of
| Description

-----------+------------+-----------------------------------------------+-------------
alice | | pg_read_all_settings WITH ADMIN, INHERIT, SET+|
| | pg_read_all_stats WITH INHERIT +|
| | pg_stat_scan_tables |

One more change. The roles in the "Member of" column are sorted for both
\du+ and \du for consistent output.

Any comments are welcome.

Yeah, I noticed the lack too, then went a bit too far afield with trying to
compose a graph of the roles. I'm still working on that but at this point
it probably won't be something I try to get committed to psql. Something
more limited like this does need to be included.

One thing I did was name the situation where none of the grants are true -
EMPTY. So: pg_stat_scan_tables WITH EMPTY.

I'm not too keen on the idea of converting the existing array into a
newline separated string. I would try hard to make the modification here
purely additional. If users really want to build up queries on their own
they should be using the system catalog. So concise human readability
should be the goal here. Keeping those two things in mind I would add a
new text[] column to the views with the following possible values in each
cell the meaning of which should be self-evident or this probably isn't a
good approach...

ais
ai
as
a
is
i
s
empty

That said, I do find the newline based output to be quite useful in the
graph query I'm writing and so wouldn't be disappointed if we changed over
to that. I'd probably stick with abbreviations though. Another thing I
did with the graph was have both "member" and "memberof" columns in the
output. In short, every grant row in pg_auth_members appears twice, once
in each column, so the role being granted membership and the role into
which membership is granted both have visibility when you filter on them.
For the role graph I took this idea and extended out to an entire chain of
roles (and also broke out user and group separately) but I think doing the
direct-grant only here would still be a big improvement.

postgres=# \dgS+ pg_read_all_settings
List of roles
Role name | Attributes | Member of | Members | Description
----------------------+--------------+-----------+-------------
pg_read_all_settings | Cannot login | {} | { pg_monitor } |

David J.

#4Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#3)
Re: psql: Add role's membership options to the \du+ command

On 24.01.2023 20:16, David G. Johnston wrote:

Yeah, I noticed the lack too, then went a bit too far afield with
trying to compose a graph of the roles.  I'm still working on that but
at this point it probably won't be something I try to get committed to
psql.  Something more limited like this does need to be included.

Glad to hear that you're working on it.

I'm not too keen on the idea of converting the existing array into a
newline separated string.  I would try hard to make the modification
here purely additional.

I agree with all of your arguments. A couple of months I tried to find
an acceptable variant in the background.
But apparently tried not very hard ))

In the end, the variant proposed in the patch seemed to me worthy to
show and
start a discussion. But I'm not sure that this is the best choice.

Another thing I did with the graph was have both "member" and
"memberof" columns in the output.  In short, every grant row in
pg_auth_members appears twice, once in each column, so the role being
granted membership and the role into which membership is granted both
have visibility when you filter on them.  For the role graph I took
this idea and extended out to an entire chain of roles (and also broke
out user and group separately) but I think doing the direct-grant only
here would still be a big improvement.

It will be interesting to see the result.

--
Pavel Luzanov

#5David Zhang
david.zhang@highgo.ca
In reply to: Pavel Luzanov (#1)
Re: psql: Add role's membership options to the \du+ command

Thanks a lot for the improvement, and it will definitely help provide
more very useful information.

I noticed the document psql-ref.sgml has been updated for both `du+` and
`dg+`, but only `du` and `\du+` are covered in regression test. Is that
because `dg+` is treated exactly the same as `du+` from testing point of
view?

The reason I am asking this question is that I notice that `pg_monitor`
also has the detailed information, so not sure if more test cases required.

postgres=# \duS+
List of roles
          Role name          | Attributes                        
|                   Member of                   | Description
-----------------------------+------------------------------------------------------------+-----------------------------------------------+-------------
 alice |                                                            |
pg_read_all_settings WITH ADMIN, INHERIT, SET |
 pg_checkpoint               | Cannot login
|                                               |
 pg_database_owner           | Cannot login
|                                               |
 pg_execute_server_program   | Cannot login
|                                               |
 pg_maintain                 | Cannot login
|                                               |
 pg_monitor                  | Cannot
login                                               |
pg_read_all_settings WITH INHERIT, SET       +|
|                                                            |
pg_read_all_stats WITH INHERIT, SET          +|
|                                                            |
pg_stat_scan_tables WITH INHERIT, SET         |

Best regards,

David

Show quoted text

On 2023-01-09 8:09 a.m., Pavel Luzanov wrote:

When you include one role in another, you can specify three options:
ADMIN, INHERIT (added in e3ce2de0) and SET (3d14e171).

For example.

CREATE ROLE alice LOGIN;

GRANT pg_read_all_settings TO alice WITH ADMIN TRUE, INHERIT TRUE, SET
TRUE;
GRANT pg_stat_scan_tables TO alice WITH ADMIN FALSE, INHERIT FALSE,
SET FALSE;
GRANT pg_read_all_stats TO alice WITH ADMIN FALSE, INHERIT TRUE, SET
FALSE;

For information about the options, you need to look in the
pg_auth_members:

SELECT roleid::regrole, admin_option, inherit_option, set_option
FROM pg_auth_members
WHERE member = 'alice'::regrole;
        roleid        | admin_option | inherit_option | set_option
----------------------+--------------+----------------+------------
 pg_read_all_settings | t            | t              | t
 pg_stat_scan_tables  | f            | f              | f
 pg_read_all_stats    | f            | t              | f
(3 rows)

I think it would be useful to be able to get this information with a
psql command
like \du (and \dg). With proposed patch the \du command still only lists
the roles of which alice is a member:

\du alice
                                     List of roles
 Role name | Attributes |                          Member of
-----------+------------+--------------------------------------------------------------

 alice     |            |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}

But the \du+ command adds information about the selected ADMIN, INHERIT
and SET options:

\du+ alice
                                    List of roles
 Role name | Attributes |                   Member
of                   | Description
-----------+------------+-----------------------------------------------+-------------

 alice     |            | pg_read_all_settings WITH ADMIN, INHERIT, SET+|
           |            | pg_read_all_stats WITH INHERIT               +|
           |            | pg_stat_scan_tables                           |

One more change. The roles in the "Member of" column are sorted for both
\du+ and \du for consistent output.

Any comments are welcome.

#6David G. Johnston
david.g.johnston@gmail.com
In reply to: David Zhang (#5)
Re: psql: Add role's membership options to the \du+ command

On Fri, Feb 10, 2023 at 2:08 PM David Zhang <david.zhang@highgo.ca> wrote:

I noticed the document psql-ref.sgml has been updated for both `du+` and
`dg+`, but only `du` and `\du+` are covered in regression test. Is that
because `dg+` is treated exactly the same as `du+` from testing point of
view?

Yes.

The reason I am asking this question is that I notice that `pg_monitor`
also has the detailed information, so not sure if more test cases required.

Of course it does. Why does that bother you? And what does that have to
do with the previous paragraph?

David J.

#7David Zhang
david.zhang@highgo.ca
In reply to: David G. Johnston (#6)
Re: psql: Add role's membership options to the \du+ command

On 2023-02-10 2:27 p.m., David G. Johnston wrote:

On Fri, Feb 10, 2023 at 2:08 PM David Zhang <david.zhang@highgo.ca> wrote:

I noticed the document psql-ref.sgml has been updated for both
`du+` and
`dg+`, but only `du` and `\du+` are covered in regression test. Is
that
because `dg+` is treated exactly the same as `du+` from testing
point of
view?

Yes.

The reason I am asking this question is that I notice that
`pg_monitor`
also has the detailed information, so not sure if more test cases
required.

Of course it does.  Why does that bother you?  And what does that have
to do with the previous paragraph?

There is a default built-in role `pg_monitor` and the behavior changed
after the patch. If `\dg+` and `\du+` is treated as the same, and `make
check` all pass, then I assume there is no test case to verify the
output of `duS+`. My point is should we consider add a test case?

Before patch the output for `pg_monitor`,

postgres=# \duS+
List of roles
          Role name          | Attributes                         |
Member of                           | Description
-----------------------------+------------------------------------------------------------+--------------------------------------------------------------+-------------
 alice |                                                            |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables} |
 pg_checkpoint               | Cannot
login                                               |
{}                                                           |
 pg_database_owner           | Cannot
login                                               |
{}                                                           |
 pg_execute_server_program   | Cannot
login                                               |
{}                                                           |
 pg_maintain                 | Cannot
login                                               |
{}                                                           |
 pg_monitor                  | Cannot
login                                               |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables} |
 pg_read_all_data            | Cannot
login                                               |
{}                                                           |
 pg_read_all_settings        | Cannot
login                                               |
{}                                                           |
 pg_read_all_stats           | Cannot
login                                               |
{}                                                           |
 pg_read_server_files        | Cannot
login                                               |
{}                                                           |
 pg_signal_backend           | Cannot
login                                               |
{}                                                           |
 pg_stat_scan_tables         | Cannot
login                                               |
{}                                                           |
 pg_use_reserved_connections | Cannot
login                                               |
{}                                                           |
 pg_write_all_data           | Cannot
login                                               |
{}                                                           |
 pg_write_server_files       | Cannot
login                                               |
{}                                                           |
 ubuntu                      | Superuser, Create role, Create DB,
Replication, Bypass RLS |
{}                                                           |

After patch the output for `pg_monitor`,

postgres=# \duS+
List of roles
          Role name          | Attributes                        
|                   Member of                   | Description
-----------------------------+------------------------------------------------------------+-----------------------------------------------+-------------
 alice |                                                            |
pg_read_all_settings WITH ADMIN, INHERIT, SET+|
|                                                            |
pg_read_all_stats WITH INHERIT               +|
|                                                            |
pg_stat_scan_tables                           |
 pg_checkpoint               | Cannot login
|                                               |
 pg_database_owner           | Cannot login
|                                               |
 pg_execute_server_program   | Cannot login
|                                               |
 pg_maintain                 | Cannot login
|                                               |
 pg_monitor                  | Cannot
login                                               |
pg_read_all_settings WITH INHERIT, SET       +|
|                                                            |
pg_read_all_stats WITH INHERIT, SET          +|
|                                                            |
pg_stat_scan_tables WITH INHERIT, SET         |
 pg_read_all_data            | Cannot login
|                                               |
 pg_read_all_settings        | Cannot login
|                                               |
 pg_read_all_stats           | Cannot login
|                                               |
 pg_read_server_files        | Cannot login
|                                               |
 pg_signal_backend           | Cannot login
|                                               |
 pg_stat_scan_tables         | Cannot login
|                                               |
 pg_use_reserved_connections | Cannot login
|                                               |
 pg_write_all_data           | Cannot login
|                                               |
 pg_write_server_files       | Cannot login
|                                               |
 ubuntu                      | Superuser, Create role, Create DB,
Replication, Bypass RLS |                                               |

Best regards,

David

Show quoted text

David J.

#8David G. Johnston
david.g.johnston@gmail.com
In reply to: David Zhang (#7)
Re: psql: Add role's membership options to the \du+ command

On Wed, Feb 15, 2023 at 2:31 PM David Zhang <david.zhang@highgo.ca> wrote:

There is a default built-in role `pg_monitor` and the behavior changed
after the patch. If `\dg+` and `\du+` is treated as the same, and `make
check` all pass, then I assume there is no test case to verify the output
of `duS+`. My point is should we consider add a test case?

I mean, either you accept the change in how this meta-command presents its
information or you don't. I don't see how a test case is particularly
beneficial. Or, at least the pg_monitor role is not special in this
regard. Alice changed too and you don't seem to be including it in your
complaint.

David J.

#9David Zhang
david.zhang@highgo.ca
In reply to: David G. Johnston (#8)
Re: psql: Add role's membership options to the \du+ command

On 2023-02-15 1:37 p.m., David G. Johnston wrote:

On Wed, Feb 15, 2023 at 2:31 PM David Zhang <david.zhang@highgo.ca> wrote:

There is a default built-in role `pg_monitor` and the behavior
changed after the patch. If `\dg+` and `\du+` is treated as the
same, and `make check` all pass, then I assume there is no test
case to verify the output of `duS+`. My point is should we
consider add a test case?

I mean, either you accept the change in how this meta-command presents
its information or you don't.  I don't see how a test case is
particularly beneficial.  Or, at least the pg_monitor role is not
special in this regard.  Alice changed too and you don't seem to be
including it in your complaint.

Good improvement, +1.

#10Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#8)
Re: psql: Add role's membership options to the \du+ command

On 16.02.2023 00:37, David G. Johnston wrote:

I mean, either you accept the change in how this meta-command presents
its information or you don't.

Yes, that's the main issue of this patch.

On the one hand, it would be nice to see the membership options with the
psql command.

On the other hand, I don't have an exact understanding of how best to do
it. That's why I proposed a variant for discussion. It is quite possible
that if there is no consensus, it would be better to leave it as is, and
get information by queries to the system catalog.

-----
Pavel Luzanov

#11Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#10)
Re: psql: Add role's membership options to the \du+ command

Hello,

On the one hand, it would be nice to see the membership options with
the psql command.

After playing with cf5eb37c and e5b8a4c0 I think something must be made
with \du command.

postgres@demo(16.0)=# CREATE ROLE admin LOGIN CREATEROLE;
CREATE ROLE
postgres@demo(16.0)=# \c - admin
You are now connected to database "demo" as user "admin".
admin@demo(16.0)=> SET createrole_self_grant = 'SET, INHERIT';
SET
admin@demo(16.0)=> CREATE ROLE bob LOGIN;
CREATE ROLE
admin@demo(16.0)=> \du

                                   List of roles
 Role name | Attributes                         | Member of
-----------+------------------------------------------------------------+-----------
 admin     | Create role                                               
| {bob,bob}
 bob |                                                            | {}
 postgres  | Superuser, Create role, Create DB, Replication, Bypass RLS
| {}

We see two bob roles in the 'Member of' column.Strange? But this is correct.

admin@demo(16.0)=> select roleid::regrole, member::regrole, * from
pg_auth_members where roleid = 'bob'::regrole;
 roleid | member |  oid  | roleid | member | grantor | admin_option |
inherit_option | set_option
--------+--------+-------+--------+--------+---------+--------------+----------------+------------
 bob    | admin  | 16713 |  16712 |  16711 |      10 | t            |
f              | f
 bob    | admin  | 16714 |  16712 |  16711 |   16711 | f            |
t              | t
(2 rows)

First 'grant bob to admin' command issued immediately after creating
role bob by superuser(grantor=10). Second command issues by admin role
and set membership options SET and INHERIT.

If we don't ready to display membership options with \du+ may be at
least we must group records in 'Member of' column for \du command?

-----
Pavel Luzanov

#12David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#11)
Re: psql: Add role's membership options to the \du+ command

On Fri, Feb 17, 2023 at 4:02 AM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

List of roles
Role name | Attributes |
Member of

-----------+------------------------------------------------------------+-----------
admin | Create role |
{bob,bob}
bob | |
{}
postgres | Superuser, Create role, Create DB, Replication, Bypass RLS |
{}

First 'grant bob to admin' command issued immediately after creating role
bob by superuser(grantor=10). Second command issues by admin role and set
membership options SET and INHERIT.
If we don't ready to display membership options with \du+ may be at least
we must group records in 'Member of' column for \du command?

I agree that these views should GROUP BY roleid and use bool_or(*_option)
to produce their result. Their purpose is to communicate the current
effective state to the user, not facilitate full inspection of the
configuration, possibly to aid in issuing GRANT and REVOKE commands.

One thing I found, and I plan to bring this up independently once I've
collected my thoughts, is that pg_has_role() uses the terminology "USAGE"
and "MEMBER" for "INHERIT" and "SET" respectively.

It's annoying that "member" has been overloaded here. And the choice of
USAGE just seems arbitrary (though I haven't researched it) given the
related syntax.

https://www.postgresql.org/docs/15/functions-info.html

#13Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#12)
Re: psql: Add role's membership options to the \du+ command

On 17.02.2023 19:53, David G. Johnston wrote:

On Fri, Feb 17, 2023 at 4:02 AM Pavel Luzanov
<p.luzanov@postgrespro.ru> wrote:

                                   List of roles
 Role name | Attributes                         | Member of
-----------+------------------------------------------------------------+-----------
 admin     | Create
role                                                | {bob,bob}
 bob | | {}
 postgres  | Superuser, Create role, Create DB, Replication,
Bypass RLS | {}

First 'grant bob to admin' command issued immediately after
creating role bob by superuser(grantor=10). Second command issues
by admin role and set membership options SET and INHERIT.

If we don't ready to display membership options with \du+ may be
at least we must group records in 'Member of' column for \du command?

I agree that these views should GROUP BY roleid and use
bool_or(*_option) to produce their result.

Ok, I'll try in the next few days. But what presentation format to use?

1. bob(admin_option=t inherit_option=t set_option=f) -- it seems very long
2. bob(ai) -- short, but will it be clear?
3. something else?

Their purpose is to communicate the current effective state to the
user, not facilitate full inspection of the configuration, possibly to
aid in issuing GRANT and REVOKE commands.

This can help in issuing GRANT command, but not REVOKE. Revoking a
role's membership is now very similar to revoking privileges. Only the
role that granted membership can revoke that membership. So for REVOKE
you need to know who granted membership, but this information will not
be available after grouping.

One thing I found, and I plan to bring this up independently once I've
collected my thoughts, is that pg_has_role() uses the terminology
"USAGE" and "MEMBER" for "INHERIT" and "SET" respectively.

It's annoying that "member" has been overloaded here.  And the choice
of USAGE just seems arbitrary (though I haven't researched it) given
the related syntax.

https://www.postgresql.org/docs/15/functions-info.html

I didn't even know this function existed. But I see that it was changed
in 3d14e171 with updated documentation:
https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS
Maybe that's enough.

--
Pavel Luzanov
Postgres Professional:https://postgrespro.com

#14David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#13)
Re: psql: Add role's membership options to the \du+ command

On Tue, Feb 21, 2023 at 2:14 PM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

On 17.02.2023 19:53, David G. Johnston wrote:

On Fri, Feb 17, 2023 at 4:02 AM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

List of roles
Role name | Attributes |
Member of

-----------+------------------------------------------------------------+-----------
admin | Create role |
{bob,bob}
bob | |
{}
postgres | Superuser, Create role, Create DB, Replication, Bypass RLS |
{}

First 'grant bob to admin' command issued immediately after creating role
bob by superuser(grantor=10). Second command issues by admin role and set
membership options SET and INHERIT.
If we don't ready to display membership options with \du+ may be at least
we must group records in 'Member of' column for \du command?

I agree that these views should GROUP BY roleid and use bool_or(*_option)
to produce their result.

Ok, I'll try in the next few days. But what presentation format to use?

This is the format I've gone for (more-or-less) in my RoleGraph view (I'll
be sharing it publicly in the near future).

bob from grantor (a, s, i) \n
adam from postgres (a, s, i) \n
emily from postgres (empty)
I don't think first-letter mnemonics will be an issue - you need to learn
the syntax anyway. And it is already what we do for object grants besides.

Based upon prior comments going for something like the following is
undesirable: bob=asi/grantor

So I converted the "/" into "from" and stuck the permissions on the end
instead of in the middle (makes reading the "from" fragment cleaner).

To be clear, this is going away from grouping but trades verbosity and
deviation from what is done today for better information. If we are going
to break this I suppose we might as well break it thoroughly.

I didn't even know this function existed. But I see that it was changed in
3d14e171 with updated documentation:

https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS
Maybe that's enough.

I think that should probably have ADMIN as one of the options as well.
Also curious what it reports for an empty membership.

David J.

#15Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#14)
Re: psql: Add role's membership options to the \du+ command

On 22.02.2023 00:34, David G. Johnston wrote:

This is the format I've gone for (more-or-less) in my RoleGraph view
(I'll be sharing it publicly in the near future).

bob from grantor (a, s, i) \n
adam from postgres (a, s, i) \n
emily from postgres (empty)

I think this is a good compromise.

Based upon prior comments going for something like the following is
undesirable: bob=asi/grantor

Agree. Membership options are not the ACL (although they have
similarities). Therefore, showing them as a ACL-like column will be
confusing.

So, please find attached the second version of the patch. It implements
suggested display format and small refactoring of existing code for \du
command.
As a non-native writer, I have doubts about the documentation part.

--
Pavel Luzanov
Postgres Professional:https://postgrespro.com

Attachments:

v2-0001-psql-du-shows-membership-options.patchtext/x-patch; charset=UTF-8; name=v2-0001-psql-du-shows-membership-options.patchDownload+111-17
#16Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#15)
Re: psql: Add role's membership options to the \du+ command

Next version (v3) addresses complains from cfbot. Changed only tests.

--
Pavel Luzanov
Postgres Professional: https://postgrespro.com

Attachments:

v3-0001-psql-du-shows-memberships-options.patchtext/x-patch; charset=UTF-8; name=v3-0001-psql-du-shows-memberships-options.patchDownload+118-17
#17Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#14)
Re: psql: Add role's membership options to the \du+ command

Hello,

On 22.02.2023 00:34, David G. Johnston wrote:

I didn't even know this function existed. But I see that it was
changed in 3d14e171 with updated documentation:
https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS
Maybe that's enough.

I think that should probably have ADMIN as one of the options as well.
Also curious what it reports for an empty membership.

I've been experimenting for a few days and I want to admit that this is
a very difficult and not obvious topic.
I'll try to summarize what I think.

1.
About ADMIN value for pg_has_role.
Implementation of ADMIN value will be different from USAGE and SET.
To be True, USAGE value requires the full chain of memberships to have
INHERIT option.
Similar with SET: the full chain of memberships must have SET option.
But for ADMIN, only last member in the chain must have ADMIN option and
all previous members
must have INHERIT (to administer directly) or SET option (to switch to
role, last in the chain).
Therefore, it is not obvious to me that the function needs the ADMIN value.

2.
pg_has_role function description starts with: Does user have privilege
for role?
    - This is not exact: function works not only with users, but with
NOLOGIN roles too.
    - Term "privilege": this term used for ACL columns, such usage may
be confusing,
      especially after adding INHERIT and SET in addition to ADMIN option.

3.
It is possible to grant membership with all three options turned off:
    grant a to b with admin false, inherit false, set false;
But such membership is completely useless (if i didn't miss something).
May be such grants must be prohibited. At least this may be documented
in the GRANT command.

4.
Since v16 it is possible to grant membership from one role to another
several times with different grantors.
And only grantor can revoke membership.
    - This is not documented anywhere.
    - Current behavior of \du command with duplicated roles in "Member
of" column strongly confusing.
      This is one of the goals of the discussion patch.

I think to write about this to pgsql-docs additionally to this topic.

--
Pavel Luzanov
Postgres Professional:https://postgrespro.com

#18David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#17)
Re: psql: Add role's membership options to the \du+ command

On Fri, Mar 3, 2023 at 4:01 AM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

Hello,

On 22.02.2023 00:34, David G. Johnston wrote:

I didn't even know this function existed. But I see that it was changed in
3d14e171 with updated documentation:

https://www.postgresql.org/docs/devel/functions-info.html#FUNCTIONS-INFO-ACCESS
Maybe that's enough.

I think that should probably have ADMIN as one of the options as well.
Also curious what it reports for an empty membership.

I've been experimenting for a few days and I want to admit that this is a
very difficult and not obvious topic.
I'll try to summarize what I think.

1.
About ADMIN value for pg_has_role.
Implementation of ADMIN value will be different from USAGE and SET.
To be True, USAGE value requires the full chain of memberships to have
INHERIT option.
Similar with SET: the full chain of memberships must have SET option.
But for ADMIN, only last member in the chain must have ADMIN option and
all previous members
must have INHERIT (to administer directly) or SET option (to switch to
role, last in the chain).
Therefore, it is not obvious to me that the function needs the ADMIN value.

Or you can SET to some role that then has an unbroken INHERIT chain to the
administered role.

ADMIN basically implies SET/USAGE but it doesn't work the other way around.

I'd be fine with "pg_can_admin_role" being a newly created function that
provides this true/false answer but it seems indisputable that today there
is no core-provided means to answer the question "can one role get ADMIN
rights on another role". Modifying \du to show this seems out-of-scope but
the pg_has_role function already provides that question for INHERIT and SET
so it is at least plausible to extend it to include ADMIN, even if the
phrase "has role" seems a bit of a misnomer. I do cover this aspect with
the Role Graph pseudo-extension but given the presence and ease-of-use of a
boolean-returning function this seems like a natural addition. We've also
survived quite long without it - this isn't a new concept in v16, just a
bit refined.

2.
pg_has_role function description starts with: Does user have privilege for
role?
- This is not exact: function works not only with users, but with
NOLOGIN roles too.
- Term "privilege": this term used for ACL columns, such usage may be
confusing,
especially after adding INHERIT and SET in addition to ADMIN option.

Yes, it missed the whole "there are only roles now" memo. I don't have an
issue with using privilege here though - you have to use the GRANT command
which "defines access privileges". Otherwise "membership option" or maybe
just "option" would need to be explored.

3.
It is possible to grant membership with all three options turned off:
grant a to b with admin false, inherit false, set false;
But such membership is completely useless (if i didn't miss something).
May be such grants must be prohibited. At least this may be documented in
the GRANT command.

I have no issue with prohibiting the "empty membership" if someone wants to
code that up.

4.
Since v16 it is possible to grant membership from one role to another
several times with different grantors.
And only grantor can revoke membership.
- This is not documented anywhere.

Yeah, a pass over the GRANTED BY actual operation versus documentation
seems warranted.

- Current behavior of \du command with duplicated roles in "Member of"
column strongly confusing.
This is one of the goals of the discussion patch.

This indeed needs to be fixed, one way (include grantor) or the other
(du-duplicate), with the current proposal of including grantor getting my
vote.

I think to write about this to pgsql-docs additionally to this topic.

I wouldn't bother starting yet another thread in this area right now, this
one can absorb some related changes as well as the subject line item.

David J.

#19Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#18)
Re: psql: Add role's membership options to the \du+ command

On 03.03.2023 19:21, David G. Johnston wrote:

I'd be fine with "pg_can_admin_role" being a newly created function
that provides this true/false answer but it seems indisputable that
today there is no core-provided means to answer the question "can one
role get ADMIN rights on another role".  Modifying \du to show this
seems out-of-scope but the pg_has_role function already provides that
question for INHERIT and SET so it is at least plausible to extend it
to include ADMIN, even if the phrase "has role" seems a bit of a
misnomer.  I do cover this aspect with the Role Graph pseudo-extension
but given the presence and ease-of-use of a boolean-returning function
this seems like a natural addition.  We've also survived quite long
without it - this isn't a new concept in v16, just a bit refined.

I must admit that I am slowly coming to the same conclusions that you
have already outlined in previous messages.

Indeed, adding ADMIN to pg_has_role looks logical. The function will
show whether one role can manage another directly or indirectly (via SET
ROLE).
Adding ADMIN will lead to the question of naming other values. It is
more reasonable to have INHERIT instead of USAGE.
And it is not very clear whether (except for backward compatibility) a
separate MEMBER value is needed at all.

I wouldn't bother starting yet another thread in this area right now,
this one can absorb some related changes as well as the subject line item.

I agree.

--
Pavel Luzanov
Postgres Professional:https://postgrespro.com

#20David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#19)
Re: psql: Add role's membership options to the \du+ command

On Mon, Mar 6, 2023 at 12:43 AM Pavel Luzanov <p.luzanov@postgrespro.ru>
wrote:

Indeed, adding ADMIN to pg_has_role looks logical. The function will show
whether one role can manage another directly or indirectly (via SET ROLE).

FWIW I've finally gotten to publishing my beta version of the Role Graph
for PostgreSQL pseudo-extension I'd been talking about:

https://github.com/polobo/RoleGraphForPostgreSQL

The administration column basically determines all this via a recursive
CTE. I'm pondering how to incorporate some of this core material into it
now for both cross-validation purposes and ease-of-use.

I handle EMPTY explicitly in the Role Graph but as I noted somewhere in my
comments, it really shouldn't be possible to leave the database in that
state. Do we need to bug Robert on this directly or do you plan to have a
go at it?

Adding ADMIN will lead to the question of naming other values. It is more

reasonable to have INHERIT instead of USAGE.

And it is not very clear whether (except for backward compatibility) a

separate MEMBER value is needed at all.

There is an appeal to breaking things thoroughly here and removing both
MEMBER and USAGE terms while adding ADMIN, SET, INHERIT.

However, I am against that. Most everyday usage of this would only likely
care about SET and INHERIT even going forward - administration of roles is
distinct from how those roles generally behave at runtime. Breaking the
later because we improved upon the former doesn't seem defensible. Thus,
while adding ADMIN makes sense, keeping MEMBER (a.k.a., SET) and USAGE
(a.k.a., INHERIT) is my suggested way forward.

I'll be looking over your v3 patch sometime this week, if not today.

David J.

#21David G. Johnston
david.g.johnston@gmail.com
In reply to: David G. Johnston (#20)
#22Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#21)
#23Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#20)
#24Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#22)
#25Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#24)
#26Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#25)
#27David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#26)
#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#27)
#29David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#28)
#30Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#28)
#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#30)
#32Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#31)
#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#32)
#34David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#33)
#35Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#34)
#36Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Robert Haas (#35)
#37Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#34)
#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Luzanov (#37)
#39David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#38)
#40Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#39)
#41Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Pavel Luzanov (#40)
#42Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Kyotaro Horiguchi (#41)
#43Jonathan S. Katz
jkatz@postgresql.org
In reply to: Pavel Luzanov (#40)
#44David G. Johnston
david.g.johnston@gmail.com
In reply to: Jonathan S. Katz (#43)
#45Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#44)
#46Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#45)
#47David G. Johnston
david.g.johnston@gmail.com
In reply to: Jonathan S. Katz (#46)
#48Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#47)
#49Jonathan S. Katz
jkatz@postgresql.org
In reply to: Pavel Luzanov (#48)
#50Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Jonathan S. Katz (#49)
#51David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#50)
#52Jonathan S. Katz
jkatz@postgresql.org
In reply to: David G. Johnston (#51)
#53Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jonathan S. Katz (#52)
#54David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#53)
#55Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#54)
#56Jonathan S. Katz
jkatz@postgresql.org
In reply to: David G. Johnston (#54)
#57Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#55)
#58Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#54)
#59David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#58)
#60Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#59)
#61David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Luzanov (#60)
#62Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: David G. Johnston (#61)
#63Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#62)
#64Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Luzanov (#63)
#65Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Tom Lane (#64)
#66Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#65)
#67Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Tom Lane (#64)
#68Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Luzanov (#67)
#69David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#68)
#70Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Tom Lane (#68)
#71Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Luzanov (#70)
#72Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Pavel Luzanov (#67)
#73Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Pavel Luzanov (#66)
#74Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#73)
#75Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#74)
#76David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#75)
#77Pavel Luzanov
p.luzanov@postgrespro.ru
In reply to: Tom Lane (#75)
#78Jonathan S. Katz
jkatz@postgresql.org
In reply to: Pavel Luzanov (#77)