pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedNew default role- 'pg_read_all_data'
25+ messages / 8 participants
[nested] [flat]
* New default role- 'pg_read_all_data'
@ 2020-08-28 00:30 Stephen Frost <sfrost@snowman.net>
2020-08-28 01:17 ` Re: New default role- 'pg_read_all_data' Steven Pousty <steve.pousty@gmail.com>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-10-29 11:27 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
0 siblings, 3 replies; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 00:30 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Greetings,
There's no shortage of requests and responses regarding how to have a
'read all of the data' role in PG, with various hacks involving "GRANT
ALL" and "ALTER DEFAULT PRIVILEGES" to "solve" this, neither of which
really works long term ("GRANT ALL" is one-time, and "ALTER DEFAULT"
only helps for the roles that exist today).
Now that we have the default role system, we can provide a proper
solution to this oft-requested capability.
This patch adds a default role to meet specifically that use-case, in
the long-term, by explicitly allowing SELECT rights on all relations,
and USAGE rights on all schemas, for roles who are members of the new
'pg_read_all_data' role.
No effort is made to prevent a user who has this role from writing data-
that's up to the admin, but this will allow someone to use pg_dump or
pg_dumpall in a much more reliable manner to make sure that the entire
database is able to be exported for the purpose of backups, upgrades, or
other common use-cases, without having to have that same user be a PG
superuser.
This role is given the Bypass RLS right, though to use it effectively, a
user would need to pass '--role=pg_read_all_data' to pg_dump/pg_dumpall,
since role attributes are not checked as part of role membership.
Thoughts?
Will add to the September CF.
Thanks,
Stephen
Attachments:
pg_rorole_v1.patch (text/x-diff, 6.3K) download
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 829decd883..09b3cd6cb8 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -517,6 +517,12 @@ DROP ROLE doomed_role;
</row>
</thead>
<tbody>
+ <row>
+ <entry>pg_read_all_data</entry>
+ <entry>Read all data (tables, sequences), even without having explicit
+ GRANT rights to the object. Implicitly includes USAGE rights on all
+ schemas.</entry>
+ </row>
<row>
<entry>pg_read_all_settings</entry>
<entry>Read all configuration variables, even those normally visible only to
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index c626161408..aca1deeca8 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3849,6 +3849,15 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
ReleaseSysCache(tuple);
+ /*
+ * Check if ACL_SELECT is being checked and, if so, and not set already
+ * as part of the result, then check if the user is a member of the
+ * pg_read_all_data role, which allows read access to all relations.
+ */
+ if (mask & ACL_SELECT && !(result & ACL_SELECT) &&
+ has_privs_of_role(roleid, DEFAULT_ROLE_READ_ALL_DATA))
+ result |= ACL_SELECT;
+
return result;
}
@@ -4175,6 +4184,14 @@ pg_namespace_aclmask(Oid nsp_oid, Oid roleid,
ReleaseSysCache(tuple);
+ /*
+ * Check if ACL_USAGE is being checked and, if so, and not set already
+ * as part of the result, then check if the user is a member of the
+ * pg_read_all_data role, which allows usage access to all schemas.
+ */
+ if (mask & ACL_USAGE && !(result & ACL_USAGE) &&
+ has_privs_of_role(roleid, DEFAULT_ROLE_READ_ALL_DATA))
+ result |= ACL_USAGE;
return result;
}
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 7c08851550..0dc4b2baec 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -25,6 +25,11 @@
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '3435', oid_symbol => 'DEFAULT_ROLE_READ_ALL_DATA',
+ rolname => 'pg_read_all_data', rolsuper => 'f', rolinherit => 't',
+ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+ rolreplication => 'f', rolbypassrls => 't', rolconnlimit => '-1',
+ rolpassword => '_null_', rolvaliduntil => '_null_' },
{ oid => '3374', oid_symbol => 'DEFAULT_ROLE_READ_ALL_SETTINGS',
rolname => 'pg_read_all_settings', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 3ec22c20ea..411525bcd1 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -24,8 +24,10 @@ CREATE USER regress_priv_user2;
CREATE USER regress_priv_user3;
CREATE USER regress_priv_user4;
CREATE USER regress_priv_user5;
+CREATE USER regress_priv_user6;
CREATE USER regress_priv_user5; -- duplicate
ERROR: role "regress_priv_user5" already exists
+GRANT pg_read_all_data TO regress_priv_user6;
CREATE GROUP regress_priv_group1;
CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4;
@@ -129,6 +131,21 @@ SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) );
------+------
(0 rows)
+SET SESSION AUTHORIZATION regress_priv_user6;
+SELECT * FROM atest1; -- ok
+ a | b
+---+-----
+ 1 | two
+ 1 | two
+(2 rows)
+
+SELECT * FROM atest2; -- ok
+ col1 | col2
+------+------
+(0 rows)
+
+INSERT INTO atest2 VALUES ('foo', true); -- fail
+ERROR: permission denied for table atest2
SET SESSION AUTHORIZATION regress_priv_user3;
SELECT session_user, current_user;
session_user | current_user
@@ -1674,6 +1691,12 @@ SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes
t
(1 row)
+SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes
+ has_schema_privilege
+----------------------
+ t
+(1 row)
+
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no
has_schema_privilege
----------------------
@@ -2045,7 +2068,8 @@ DROP USER regress_priv_user3;
DROP USER regress_priv_user4;
DROP USER regress_priv_user5;
DROP USER regress_priv_user6;
-ERROR: role "regress_priv_user6" does not exist
+DROP USER regress_priv_user7; -- does not exist
+ERROR: role "regress_priv_user7" does not exist
-- permissions with LOCK TABLE
CREATE USER regress_locktable_user;
CREATE TABLE lock_table (a int);
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 3550f61587..74f6f62c33 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -28,8 +28,11 @@ CREATE USER regress_priv_user2;
CREATE USER regress_priv_user3;
CREATE USER regress_priv_user4;
CREATE USER regress_priv_user5;
+CREATE USER regress_priv_user6;
CREATE USER regress_priv_user5; -- duplicate
+GRANT pg_read_all_data TO regress_priv_user6;
+
CREATE GROUP regress_priv_group1;
CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
@@ -94,6 +97,10 @@ GRANT ALL ON atest1 TO PUBLIC; -- fail
SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) );
SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) );
+SET SESSION AUTHORIZATION regress_priv_user6;
+SELECT * FROM atest1; -- ok
+SELECT * FROM atest2; -- ok
+INSERT INTO atest2 VALUES ('foo', true); -- fail
SET SESSION AUTHORIZATION regress_priv_user3;
SELECT session_user, current_user;
@@ -988,6 +995,7 @@ ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_priv_user2;
CREATE SCHEMA testns2;
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes
+SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no
ALTER DEFAULT PRIVILEGES REVOKE USAGE ON SCHEMAS FROM regress_priv_user2;
@@ -1211,6 +1219,7 @@ DROP USER regress_priv_user3;
DROP USER regress_priv_user4;
DROP USER regress_priv_user5;
DROP USER regress_priv_user6;
+DROP USER regress_priv_user7; -- does not exist
-- permissions with LOCK TABLE
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 01:17 ` Steven Pousty <steve.pousty@gmail.com>
2 siblings, 0 replies; 25+ messages in thread
From: Steven Pousty @ 2020-08-28 01:17 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Aug 27, 2020 at 5:30 PM Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> There's no shortage of requests and responses regarding how to have a
> 'read all of the data' role in PG, with various hacks involving "GRANT
> ALL" and "ALTER DEFAULT PRIVILEGES" to "solve" this, neither of which
> really works long term ("GRANT ALL" is one-time, and "ALTER DEFAULT"
> only helps for the roles that exist today).
>
> Now that we have the default role system, we can provide a proper
> solution to this oft-requested capability.
>
> This patch adds a default role to meet specifically that use-case, in
> the long-term, by explicitly allowing SELECT rights on all relations,
> and USAGE rights on all schemas, for roles who are members of the new
> 'pg_read_all_data' role.
>
> No effort is made to prevent a user who has this role from writing data-
> that's up to the admin, but this will allow someone to use pg_dump or
> pg_dumpall in a much more reliable manner to make sure that the entire
> database is able to be exported for the purpose of backups, upgrades, or
> other common use-cases, without having to have that same user be a PG
> superuser.
>
> This role is given the Bypass RLS right, though to use it effectively, a
> user would need to pass '--role=pg_read_all_data' to pg_dump/pg_dumpall,
> since role attributes are not checked as part of role membership.
>
>
This will be much appreciated from an app developers perspective. It makes
life so much easier to "do the right thing" in terms of giving read only
webapps the right permissions.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 10:13 ` Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2 siblings, 2 replies; 25+ messages in thread
From: Magnus Hagander @ 2020-08-28 10:13 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Fri, Aug 28, 2020 at 2:30 AM Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> There's no shortage of requests and responses regarding how to have a
> 'read all of the data' role in PG, with various hacks involving "GRANT
> ALL" and "ALTER DEFAULT PRIVILEGES" to "solve" this, neither of which
> really works long term ("GRANT ALL" is one-time, and "ALTER DEFAULT"
> only helps for the roles that exist today).
>
> Now that we have the default role system, we can provide a proper
> solution to this oft-requested capability.
>
> This patch adds a default role to meet specifically that use-case, in
> the long-term, by explicitly allowing SELECT rights on all relations,
> and USAGE rights on all schemas, for roles who are members of the new
> 'pg_read_all_data' role.
>
> No effort is made to prevent a user who has this role from writing data-
> that's up to the admin, but this will allow someone to use pg_dump or
> pg_dumpall in a much more reliable manner to make sure that the entire
> database is able to be exported for the purpose of backups, upgrades, or
> other common use-cases, without having to have that same user be a PG
> superuser.
>
> This role is given the Bypass RLS right, though to use it effectively, a
> user would need to pass '--role=pg_read_all_data' to pg_dump/pg_dumpall,
> since role attributes are not checked as part of role membership.
>
> Thoughts?
>
Without having actually looked at the code, definite +1 for this feature.
It's much requested...
But, should we also have a pg_write_all_data to go along with it?
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/;
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
@ 2020-08-28 12:16 ` Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
1 sibling, 1 reply; 25+ messages in thread
From: Georgios Kokolatos @ 2020-08-28 12:16 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Stephen Frost <sfrost@snowman.net>
Thank you for the patch.
My high level review comment:
The patch seems to be implementing a useful and requested feature.
The patch applies cleanly and passes the basic regress tests. Also the commitfest bot is happy.
A first pass at the code, has not revealed any worthwhile comments.
Please allow me for a second and more thorough pass. The commitfest has hardly started after all.
Also allow me a series of genuine questions:
What would the behaviour be with REVOKE?
In a sequence similar to:
GRANT ALL ON ...
REVOKE pg_read_all_data FROM ...
What privileges would the user be left with? Would it be possible to end up in the same privilege only with a GRANT command?
Does the above scenario even make sense?
Regards,
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
@ 2020-08-28 12:43 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 12:51 ` Re: New default role- 'pg_read_all_data' Isaac Morland <isaac.morland@gmail.com>
2020-08-28 12:53 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:06 ` Re: New default role- 'pg_read_all_data' gkokolatos@pm.me
0 siblings, 3 replies; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 12:43 UTC (permalink / raw)
To: Georgios Kokolatos <gkokolatos@protonmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
Greetings,
* Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
> The patch seems to be implementing a useful and requested feature.
> The patch applies cleanly and passes the basic regress tests. Also the commitfest bot is happy.
>
> A first pass at the code, has not revealed any worthwhile comments.
> Please allow me for a second and more thorough pass. The commitfest has hardly started after all.
Great, thanks!
> Also allow me a series of genuine questions:
>
> What would the behaviour be with REVOKE?
> In a sequence similar to:
> GRANT ALL ON ...
GRANT ALL would be independently GRANT'ing rights to some role and
therefore unrelated.
> REVOKE pg_read_all_data FROM ...
This would simply REVOKE that role from the user. Privileges
independently GRANT'd directly to the user wouldn't be affected. Nor
would other role membership.
> What privileges would the user be left with? Would it be possible to end up in the same privilege only with a GRANT command?
I'm not sure what's being asked here.
> Does the above scenario even make sense?
I definitely believe it makes sense for a given role/user to be a member
of pg_read_all_data and to be a member of other roles, or to have other
privileges GRANT'd directly to them.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 12:51 ` Isaac Morland <isaac.morland@gmail.com>
2020-08-28 12:54 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2 siblings, 1 reply; 25+ messages in thread
From: Isaac Morland @ 2020-08-28 12:51 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Fri, 28 Aug 2020 at 08:43, Stephen Frost <sfrost@snowman.net> wrote:
> This would simply REVOKE that role from the user. Privileges
> independently GRANT'd directly to the user wouldn't be affected. Nor
> would other role membership.
>
> > What privileges would the user be left with? Would it be possible to end
> up in the same privilege only with a GRANT command?
>
What about:
REVOKE SELECT ON [table] FROM pg_read_all_data;
I guess what I’m really asking is whether pg_read_all_data is automatically
granted SELECT on all newly-created relations, or if the permission
checking system always returns TRUE when asked if pg_read_all_data can
select from a relation? I’m guessing it’s the latter so that it would be
ineffective to revoke select privilege as I think this is more useful, but
I’d like to be sure and the documentation should be explicit on this point.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:51 ` Re: New default role- 'pg_read_all_data' Isaac Morland <isaac.morland@gmail.com>
@ 2020-08-28 12:54 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 13:24 ` Re: New default role- 'pg_read_all_data' Isaac Morland <isaac.morland@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 12:54 UTC (permalink / raw)
To: Isaac Morland <isaac.morland@gmail.com>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Greetings,
* Isaac Morland (isaac.morland@gmail.com) wrote:
> On Fri, 28 Aug 2020 at 08:43, Stephen Frost <sfrost@snowman.net> wrote:
> > This would simply REVOKE that role from the user. Privileges
> > independently GRANT'd directly to the user wouldn't be affected. Nor
> > would other role membership.
> >
> > > What privileges would the user be left with? Would it be possible to end
> > up in the same privilege only with a GRANT command?
>
> What about:
>
> REVOKE SELECT ON [table] FROM pg_read_all_data;
Wouldn't have any effect, and I think that's correct.
> I guess what I’m really asking is whether pg_read_all_data is automatically
> granted SELECT on all newly-created relations, or if the permission
> checking system always returns TRUE when asked if pg_read_all_data can
> select from a relation? I’m guessing it’s the latter so that it would be
> ineffective to revoke select privilege as I think this is more useful, but
> I’d like to be sure and the documentation should be explicit on this point.
Yes, it's the latter. I'm not really sure about the documentation
change you're contemplating- have a specific suggestion?
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:51 ` Re: New default role- 'pg_read_all_data' Isaac Morland <isaac.morland@gmail.com>
2020-08-28 12:54 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 13:24 ` Isaac Morland <isaac.morland@gmail.com>
0 siblings, 0 replies; 25+ messages in thread
From: Isaac Morland @ 2020-08-28 13:24 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Fri, 28 Aug 2020 at 08:54, Stephen Frost <sfrost@snowman.net> wrote:
>
> Yes, it's the latter. I'm not really sure about the documentation
> change you're contemplating- have a specific suggestion?
>
Sorry, I was discussing this as if it was an abstract idea, not a concrete
patch. I've just taken a look at the patch and I think the documentation in
doc/src/sgml/user-manag.sgml is OK.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 12:53 ` Magnus Hagander <magnus@hagander.net>
2 siblings, 0 replies; 25+ messages in thread
From: Magnus Hagander @ 2020-08-28 12:53 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Fri, Aug 28, 2020 at 2:43 PM Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> * Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
> > The patch seems to be implementing a useful and requested feature.
> > The patch applies cleanly and passes the basic regress tests. Also the
> commitfest bot is happy.
> >
> > A first pass at the code, has not revealed any worthwhile comments.
> > Please allow me for a second and more thorough pass. The commitfest has
> hardly started after all.
>
> Great, thanks!
>
> > Also allow me a series of genuine questions:
> >
> > What would the behaviour be with REVOKE?
> > In a sequence similar to:
> > GRANT ALL ON ...
>
> GRANT ALL would be independently GRANT'ing rights to some role and
> therefore unrelated.
>
> > REVOKE pg_read_all_data FROM ...
>
> This would simply REVOKE that role from the user. Privileges
> independently GRANT'd directly to the user wouldn't be affected. Nor
> would other role membership.
>
> > What privileges would the user be left with? Would it be possible to end
> up in the same privilege only with a GRANT command?
>
> I'm not sure what's being asked here.
>
I think the core thing to remember here is that SQL permissions are always
additive, that's what confuses some cases.
That is, REVOKE something FROM role only removes this particular additive
permission. It doesn't make sure the role doesn't have the same permission
*through some other means*.
Sometime it would be really useful to be able to do e.g. "DENY DELETE ON
importanttable FROM sfrost", which would then override any DELETE
permissions he'd be getting from anywhere else. To be able to say
"everybody except x". But that's not something that's in SQL permissions
AFAIK :)
> Does the above scenario even make sense?
>
> I definitely believe it makes sense for a given role/user to be a member
> of pg_read_all_data and to be a member of other roles, or to have other
> privileges GRANT'd directly to them.
>
Yeah, for example another role might have a combination of read and write
permissions, and those would then remain for the user if pg_read_all_data
is removed.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/;
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 13:06 ` gkokolatos@pm.me
2020-08-28 13:18 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2 siblings, 1 reply; 25+ messages in thread
From: gkokolatos@pm.me @ 2020-08-28 13:06 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, 28 August 2020 15:43, Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> - Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
>
> > The patch seems to be implementing a useful and requested feature.
> > The patch applies cleanly and passes the basic regress tests. Also the commitfest bot is happy.
> > A first pass at the code, has not revealed any worthwhile comments.
> > Please allow me for a second and more thorough pass. The commitfest has hardly started after all.
>
> Great, thanks!
>
> > Also allow me a series of genuine questions:
> > What would the behaviour be with REVOKE?
> > In a sequence similar to:
> > GRANT ALL ON ...
>
> GRANT ALL would be independently GRANT'ing rights to some role and
> therefore unrelated.
>
> > REVOKE pg_read_all_data FROM ...
>
> This would simply REVOKE that role from the user. Privileges
> independently GRANT'd directly to the user wouldn't be affected. Nor
> would other role membership.
>
> > What privileges would the user be left with? Would it be possible to end up in the same privilege only with a GRANT command?
>
> I'm not sure what's being asked here.
You are correct. My phrasing is not clear. Please be patient and allow me to try again.
I was playing around with the code and I was trying a bit the opposite of what you have submitted in the test file.
You have, (snipped):
GRANT pg_read_all_data TO regress_priv_user6;
SET SESSION AUTHORIZATION regress_priv_user6;
SELECT * FROM atest1; -- ok
INSERT INTO atest2 VALUES ('foo', true); -- fail
I was expecting:
REVOKE pg_read_all_data FROM regress_priv_user6;
SET SESSION AUTHORIZATION regress_priv_user6;
SELECT * FROM atest1; -- fail
INSERT INTO atest2 VALUES ('foo', true); -- ok
My expectation was not met since in my manual test (unless I made a mistake which is entirely possible), the SELECT above did not fail. The insert did succeed though.
The first question: Was my expectation wrong?
The second question: Is there a privilege that can be granted to regress_priv_user6 that will not permit the select operation but will permit the insert operation? If no, should there be one?
I hope I am clearer now.
Thank you again for your patience.
>
> > Does the above scenario even make sense?
>
> I definitely believe it makes sense for a given role/user to be a member
> of pg_read_all_data and to be a member of other roles, or to have other
> privileges GRANT'd directly to them.
>
> Thanks,
>
> Stephen
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 13:06 ` Re: New default role- 'pg_read_all_data' gkokolatos@pm.me
@ 2020-08-28 13:18 ` Stephen Frost <sfrost@snowman.net>
0 siblings, 0 replies; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 13:18 UTC (permalink / raw)
To: gkokolatos@pm.me; +Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Greetings,
* gkokolatos@pm.me (gkokolatos@pm.me) wrote:
> On Friday, 28 August 2020 15:43, Stephen Frost <sfrost@snowman.net> wrote:
> > > What privileges would the user be left with? Would it be possible to end up in the same privilege only with a GRANT command?
> >
> > I'm not sure what's being asked here.
>
> You are correct. My phrasing is not clear. Please be patient and allow me to try again.
>
> I was playing around with the code and I was trying a bit the opposite of what you have submitted in the test file.
>
> You have, (snipped):
>
> GRANT pg_read_all_data TO regress_priv_user6;
>
> SET SESSION AUTHORIZATION regress_priv_user6;
> SELECT * FROM atest1; -- ok
> INSERT INTO atest2 VALUES ('foo', true); -- fail
>
>
> I was expecting:
> REVOKE pg_read_all_data FROM regress_priv_user6;
Are you sure this REVOKE was successful..?
> SET SESSION AUTHORIZATION regress_priv_user6;
> SELECT * FROM atest1; -- fail
> INSERT INTO atest2 VALUES ('foo', true); -- ok
=# create role r1;
CREATE ROLE
=*# grant pg_read_all_data to r1;
GRANT ROLE
=*# create table t1 (c1 int);
CREATE TABLE
=*# set role r1;
=*> select * from t1;
c1
----
(0 rows)
=*> reset role;
RESET
=*# revoke pg_read_all_data from r1;
REVOKE ROLE
=*# set role r1;
SET
=*> select * from t1;
ERROR: permission denied for table t1
Seems to be working as expected here.
> My expectation was not met since in my manual test (unless I made a mistake which is entirely possible), the SELECT above did not fail. The insert did succeed though.
That the INSERT worked seems pretty odd- could you post the exact
changes you've made to the regression tests, or the exact script where
you aren't seeing what you expect? I've not been able to reproduce the
GRANT allowing a user to INSERT into a table.
> The first question: Was my expectation wrong?
If there aren't any other privileges involved, then REVOKE'ing the role
from a user should prevent that user from being able to SELECT from the
table.
> The second question: Is there a privilege that can be granted to regress_priv_user6 that will not permit the select operation but will permit the insert operation? If no, should there be one?
GRANT INSERT ON atest1 TO regress_prive_user6; would allow just
INSERT'ing.
Magnus also brought up the idea of a 'write_all_data' role, but that's
pretty independent of this, imv. Not against adding it, if we can agree
as to what it means, exactly, but we should probably discuss over in
that sub-thread.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
@ 2020-08-28 12:38 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
1 sibling, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 12:38 UTC (permalink / raw)
To: Magnus Hagander <magnus@hagander.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Greetings,
* Magnus Hagander (magnus@hagander.net) wrote:
> Without having actually looked at the code, definite +1 for this feature.
> It's much requested...
Thanks.
> But, should we also have a pg_write_all_data to go along with it?
Perhaps, but could certainly be a different patch, and it'd need to be
better defined, it seems to me... read_all is pretty straight-forward
(the general goal being "make pg_dumpall/pg_dump work"), what would
write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
Doesn't seem like you could just declare it to be 'allow pg_restore'
either, as that might include creating untrusted functions, et al.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 12:50 ` Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
0 siblings, 1 reply; 25+ messages in thread
From: Magnus Hagander @ 2020-08-28 12:50 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> * Magnus Hagander (magnus@hagander.net) wrote:
> > Without having actually looked at the code, definite +1 for this feature.
> > It's much requested...
>
> Thanks.
>
> > But, should we also have a pg_write_all_data to go along with it?
>
> Perhaps, but could certainly be a different patch, and it'd need to be
> better defined, it seems to me... read_all is pretty straight-forward
> (the general goal being "make pg_dumpall/pg_dump work"), what would
> write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
>
Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
system catalogs.
I'd say insert/update/delete yes.
TRUNCATE is always an outlier.Given it's generally classified as DDL, I
wouldn't include it.
Doesn't seem like you could just declare it to be 'allow pg_restore'
> either, as that might include creating untrusted functions, et al.
>
No definitely not. That wouldn't be the usecase at all :)
(and fwiw to me the main use case for read_all_data also isn't pg_dump,
because most people using pg_dump are already db owner or higher in my
experience. But it is nice that it helps with that too)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/;
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
@ 2020-08-28 13:26 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 13:58 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
2020-08-30 23:20 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
0 siblings, 2 replies; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 13:26 UTC (permalink / raw)
To: Magnus Hagander <magnus@hagander.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Greetings,
* Magnus Hagander (magnus@hagander.net) wrote:
> On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
> > * Magnus Hagander (magnus@hagander.net) wrote:
> > > Without having actually looked at the code, definite +1 for this feature.
> > > It's much requested...
> >
> > Thanks.
> >
> > > But, should we also have a pg_write_all_data to go along with it?
> >
> > Perhaps, but could certainly be a different patch, and it'd need to be
> > better defined, it seems to me... read_all is pretty straight-forward
> > (the general goal being "make pg_dumpall/pg_dump work"), what would
> > write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
>
> Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
> system catalogs.
>
> I'd say insert/update/delete yes.
>
> TRUNCATE is always an outlier.Given it's generally classified as DDL, I
> wouldn't include it.
Alright, that seems like it'd be pretty easy. We already have a check
in pg_class_aclmask to disallow modification of system catalogs w/o
being a superuser, so we should be alright to add a similar check for
insert/update/delete just below where I added the SELECT check.
> > Doesn't seem like you could just declare it to be 'allow pg_restore'
> > either, as that might include creating untrusted functions, et al.
>
> No definitely not. That wouldn't be the usecase at all :)
Good. :)
> (and fwiw to me the main use case for read_all_data also isn't pg_dump,
> because most people using pg_dump are already db owner or higher in my
> experience. But it is nice that it helps with that too)
Glad to have confirmation that there's other use-cases this helps with.
I'll post an updated patch with that added in a day or two.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 13:58 ` Gilles Darold <gilles@darold.net>
2020-08-28 14:52 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
1 sibling, 1 reply; 25+ messages in thread
From: Gilles Darold @ 2020-08-28 13:58 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; Magnus Hagander <magnus@hagander.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Le 28/08/2020 à 15:26, Stephen Frost a écrit :
> Greetings,
>
> * Magnus Hagander (magnus@hagander.net) wrote:
>> On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
>>> * Magnus Hagander (magnus@hagander.net) wrote:
>>>> Without having actually looked at the code, definite +1 for this feature.
>>>> It's much requested...
>>> Thanks.
>>>
>>>> But, should we also have a pg_write_all_data to go along with it?
>>> Perhaps, but could certainly be a different patch, and it'd need to be
>>> better defined, it seems to me... read_all is pretty straight-forward
>>> (the general goal being "make pg_dumpall/pg_dump work"), what would
>>> write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
>> Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
>> system catalogs.
>>
>> I'd say insert/update/delete yes.
>>
>> TRUNCATE is always an outlier.Given it's generally classified as DDL, I
>> wouldn't include it.
> Alright, that seems like it'd be pretty easy. We already have a check
> in pg_class_aclmask to disallow modification of system catalogs w/o
> being a superuser, so we should be alright to add a similar check for
> insert/update/delete just below where I added the SELECT check.
>
>>> Doesn't seem like you could just declare it to be 'allow pg_restore'
>>> either, as that might include creating untrusted functions, et al.
>> No definitely not. That wouldn't be the usecase at all :)
> Good. :)
>
>> (and fwiw to me the main use case for read_all_data also isn't pg_dump,
>> because most people using pg_dump are already db owner or higher in my
>> experience. But it is nice that it helps with that too)
> Glad to have confirmation that there's other use-cases this helps with.
>
> I'll post an updated patch with that added in a day or two.
>
> Thanks,
>
> Stephen
Hi,
Looking at this thread I was thinking about the FDW. Does role
pg_read_all_data will allow to execute pg_dump with
--include-foreign-data option? If this is the case how about priviledge
on fdw and foreign server? If this is the behavior we want I guess that
the modification should be applied to pg_foreign_data_wrapper_aclmask()
and pg_foreign_server_aclmask() too.
Best regards,
--
Gilles Darold
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 13:58 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
@ 2020-08-28 14:52 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 15:21 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
0 siblings, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 14:52 UTC (permalink / raw)
To: Gilles Darold <gilles@darold.net>; +Cc: Magnus Hagander <magnus@hagander.net>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Greetings,
* Gilles Darold (gilles@darold.net) wrote:
> Le 28/08/2020 à 15:26, Stephen Frost a écrit :
> >* Magnus Hagander (magnus@hagander.net) wrote:
> >>On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
> >>>* Magnus Hagander (magnus@hagander.net) wrote:
> >>>>Without having actually looked at the code, definite +1 for this feature.
> >>>>It's much requested...
> >>>Thanks.
> >>>
> >>>>But, should we also have a pg_write_all_data to go along with it?
> >>>Perhaps, but could certainly be a different patch, and it'd need to be
> >>>better defined, it seems to me... read_all is pretty straight-forward
> >>>(the general goal being "make pg_dumpall/pg_dump work"), what would
> >>>write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
> >>Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
> >>system catalogs.
> >>
> >>I'd say insert/update/delete yes.
> >>
> >>TRUNCATE is always an outlier.Given it's generally classified as DDL, I
> >>wouldn't include it.
> >Alright, that seems like it'd be pretty easy. We already have a check
> >in pg_class_aclmask to disallow modification of system catalogs w/o
> >being a superuser, so we should be alright to add a similar check for
> >insert/update/delete just below where I added the SELECT check.
> >
> >>>Doesn't seem like you could just declare it to be 'allow pg_restore'
> >>>either, as that might include creating untrusted functions, et al.
> >>No definitely not. That wouldn't be the usecase at all :)
> >Good. :)
> >
> >>(and fwiw to me the main use case for read_all_data also isn't pg_dump,
> >>because most people using pg_dump are already db owner or higher in my
> >>experience. But it is nice that it helps with that too)
> >Glad to have confirmation that there's other use-cases this helps with.
> >
> >I'll post an updated patch with that added in a day or two.
>
> Looking at this thread I was thinking about the FDW. Does role
> pg_read_all_data will allow to execute pg_dump with --include-foreign-data
> option? If this is the case how about priviledge on fdw and foreign server?
> If this is the behavior we want I guess that the modification should be
> applied to pg_foreign_data_wrapper_aclmask() and pg_foreign_server_aclmask()
> too.
Using an FDW will often also require having a user mapping and there's
no way for that to be accomplished through only GRANT'ing a default
role, so I don't think we should mix this specific role with the FDW
permissions system.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 13:58 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
2020-08-28 14:52 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-28 15:21 ` Gilles Darold <gilles@darold.net>
2020-08-28 17:25 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
0 siblings, 1 reply; 25+ messages in thread
From: Gilles Darold @ 2020-08-28 15:21 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Le 28/08/2020 à 16:52, Stephen Frost a écrit :
> Greetings,
>
> * Gilles Darold (gilles@darold.net) wrote:
>> Le 28/08/2020 à 15:26, Stephen Frost a écrit :
>>> * Magnus Hagander (magnus@hagander.net) wrote:
>>>> On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
>>>>> * Magnus Hagander (magnus@hagander.net) wrote:
>>>>>> Without having actually looked at the code, definite +1 for this feature.
>>>>>> It's much requested...
>>>>> Thanks.
>>>>>
>>>>>> But, should we also have a pg_write_all_data to go along with it?
>>>>> Perhaps, but could certainly be a different patch, and it'd need to be
>>>>> better defined, it seems to me... read_all is pretty straight-forward
>>>>> (the general goal being "make pg_dumpall/pg_dump work"), what would
>>>>> write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
>>>> Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
>>>> system catalogs.
>>>>
>>>> I'd say insert/update/delete yes.
>>>>
>>>> TRUNCATE is always an outlier.Given it's generally classified as DDL, I
>>>> wouldn't include it.
>>> Alright, that seems like it'd be pretty easy. We already have a check
>>> in pg_class_aclmask to disallow modification of system catalogs w/o
>>> being a superuser, so we should be alright to add a similar check for
>>> insert/update/delete just below where I added the SELECT check.
>>>
>>>>> Doesn't seem like you could just declare it to be 'allow pg_restore'
>>>>> either, as that might include creating untrusted functions, et al.
>>>> No definitely not. That wouldn't be the usecase at all :)
>>> Good. :)
>>>
>>>> (and fwiw to me the main use case for read_all_data also isn't pg_dump,
>>>> because most people using pg_dump are already db owner or higher in my
>>>> experience. But it is nice that it helps with that too)
>>> Glad to have confirmation that there's other use-cases this helps with.
>>>
>>> I'll post an updated patch with that added in a day or two.
>> Looking at this thread I was thinking about the FDW. Does role
>> pg_read_all_data will allow to execute pg_dump with --include-foreign-data
>> option? If this is the case how about priviledge on fdw and foreign server?
>> If this is the behavior we want I guess that the modification should be
>> applied to pg_foreign_data_wrapper_aclmask() and pg_foreign_server_aclmask()
>> too.
> Using an FDW will often also require having a user mapping and there's
> no way for that to be accomplished through only GRANT'ing a default
> role, so I don't think we should mix this specific role with the FDW
> permissions system.
I'm fine with that, perhaps it should be mentioned in the documentation
that foreign tables are not covered by this role.
--
Gilles Darold
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 13:58 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
2020-08-28 14:52 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 15:21 ` Re: New default role- 'pg_read_all_data' Gilles Darold <gilles@darold.net>
@ 2020-08-28 17:25 ` Stephen Frost <sfrost@snowman.net>
0 siblings, 0 replies; 25+ messages in thread
From: Stephen Frost @ 2020-08-28 17:25 UTC (permalink / raw)
To: Gilles Darold <gilles@darold.net>; +Cc: pgsql-hackers@lists.postgresql.org
Greetings,
* Gilles Darold (gilles@darold.net) wrote:
> Le 28/08/2020 à 16:52, Stephen Frost a écrit :
> >Using an FDW will often also require having a user mapping and there's
> >no way for that to be accomplished through only GRANT'ing a default
> >role, so I don't think we should mix this specific role with the FDW
> >permissions system.
>
> I'm fine with that, perhaps it should be mentioned in the documentation that
> foreign tables are not covered by this role.
We could say it doesn't GRANT CONNECT rights on databases, or EXECUTE on
functions too, but that doesn't seem like a terribly good approach for
the documentation to take- instead we document specifically what IS
included, which seems sufficiently clear to me.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-30 23:20 ` Stephen Frost <sfrost@snowman.net>
2020-08-31 14:17 ` Re: New default role- 'pg_read_all_data' gkokolatos@pm.me
1 sibling, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-08-30 23:20 UTC (permalink / raw)
To: Magnus Hagander <magnus@hagander.net>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Greetings,
* Stephen Frost (sfrost@snowman.net) wrote:
> * Magnus Hagander (magnus@hagander.net) wrote:
> > On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost <sfrost@snowman.net> wrote:
> > > * Magnus Hagander (magnus@hagander.net) wrote:
> > > > Without having actually looked at the code, definite +1 for this feature.
> > > > It's much requested...
> > >
> > > Thanks.
> > >
> > > > But, should we also have a pg_write_all_data to go along with it?
> > >
> > > Perhaps, but could certainly be a different patch, and it'd need to be
> > > better defined, it seems to me... read_all is pretty straight-forward
> > > (the general goal being "make pg_dumpall/pg_dump work"), what would
> > > write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
> >
> > Well, it's pg_write_all_*data*, so it certainly wouldn't be alter table or
> > system catalogs.
> >
> > I'd say insert/update/delete yes.
> >
> > TRUNCATE is always an outlier.Given it's generally classified as DDL, I
> > wouldn't include it.
>
> Alright, that seems like it'd be pretty easy. We already have a check
> in pg_class_aclmask to disallow modification of system catalogs w/o
> being a superuser, so we should be alright to add a similar check for
> insert/update/delete just below where I added the SELECT check.
>
> > > Doesn't seem like you could just declare it to be 'allow pg_restore'
> > > either, as that might include creating untrusted functions, et al.
> >
> > No definitely not. That wouldn't be the usecase at all :)
>
> Good. :)
>
> > (and fwiw to me the main use case for read_all_data also isn't pg_dump,
> > because most people using pg_dump are already db owner or higher in my
> > experience. But it is nice that it helps with that too)
>
> Glad to have confirmation that there's other use-cases this helps with.
>
> I'll post an updated patch with that added in a day or two.
Here's that updated patch, comments welcome.
Thanks!
Stephen
Attachments:
pg_rorole_v2.patch (text/x-diff, 9.0K) download
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 829decd883..1213125bfd 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -517,6 +517,20 @@ DROP ROLE doomed_role;
</row>
</thead>
<tbody>
+ <row>
+ <entry>pg_read_all_data</entry>
+ <entry>Read all data (tables, views, sequences), as if having SELECT
+ rights on those objects, and USAGE rights on all schemas, even without
+ having it explicitly. This role also has <literal>BYPASSRLS</literal>
+ set for it.</entry>
+ </row>
+ <row>
+ <entry>pg_write_all_data</entry>
+ <entry>Write all data (tables, views, sequences), as if having INSERT,
+ UPDATE, and DELETE rights on those objects, and USAGE rights on all
+ schemas, even without having it explicitly. This role also has
+ <literal>BYPASSRLS</literal> set for it.</entry>
+ </row>
<row>
<entry>pg_read_all_settings</entry>
<entry>Read all configuration variables, even those normally visible only to
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index c626161408..3e6d060554 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3849,6 +3849,26 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
ReleaseSysCache(tuple);
+ /*
+ * Check if ACL_SELECT is being checked and, if so, and not set already
+ * as part of the result, then check if the user is a member of the
+ * pg_read_all_data role, which allows read access to all relations.
+ */
+ if (mask & ACL_SELECT && !(result & ACL_SELECT) &&
+ has_privs_of_role(roleid, DEFAULT_ROLE_READ_ALL_DATA))
+ result |= ACL_SELECT;
+
+ /*
+ * Check if ACL_INSERT, ACL_UPDATE, or ACL_DELETE is being checked
+ * and, if so, and not set already as part of the result, then check
+ * if the user is a member of the pg_write_all_data role, which
+ * allows INSERT/UPDATE/DELETE access to all relations.
+ */
+ if (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE) &&
+ !(result & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) &&
+ has_privs_of_role(roleid, DEFAULT_ROLE_WRITE_ALL_DATA))
+ result |= (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE));
+
return result;
}
@@ -4175,6 +4195,16 @@ pg_namespace_aclmask(Oid nsp_oid, Oid roleid,
ReleaseSysCache(tuple);
+ /*
+ * Check if ACL_USAGE is being checked and, if so, and not set already
+ * as part of the result, then check if the user is a member of the
+ * pg_read_all_data or pg_write_all_data roles, which allow usage
+ * access to all schemas.
+ */
+ if (mask & ACL_USAGE && !(result & ACL_USAGE) &&
+ (has_privs_of_role(roleid, DEFAULT_ROLE_READ_ALL_DATA) ||
+ has_privs_of_role(roleid, DEFAULT_ROLE_WRITE_ALL_DATA)))
+ result |= ACL_USAGE;
return result;
}
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 7c08851550..3f72474146 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -25,6 +25,16 @@
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '9274', oid_symbol => 'DEFAULT_ROLE_READ_ALL_DATA',
+ rolname => 'pg_read_all_data', rolsuper => 'f', rolinherit => 't',
+ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+ rolreplication => 'f', rolbypassrls => 't', rolconnlimit => '-1',
+ rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '9275', oid_symbol => 'DEFAULT_ROLE_WRITE_ALL_DATA',
+ rolname => 'pg_write_all_data', rolsuper => 'f', rolinherit => 't',
+ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+ rolreplication => 'f', rolbypassrls => 't', rolconnlimit => '-1',
+ rolpassword => '_null_', rolvaliduntil => '_null_' },
{ oid => '3374', oid_symbol => 'DEFAULT_ROLE_READ_ALL_SETTINGS',
rolname => 'pg_read_all_settings', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 3ec22c20ea..090e90dadb 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -12,6 +12,7 @@ DROP ROLE IF EXISTS regress_priv_user3;
DROP ROLE IF EXISTS regress_priv_user4;
DROP ROLE IF EXISTS regress_priv_user5;
DROP ROLE IF EXISTS regress_priv_user6;
+DROP ROLE IF EXISTS regress_priv_user7;
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
lo_unlink
-----------
@@ -26,6 +27,10 @@ CREATE USER regress_priv_user4;
CREATE USER regress_priv_user5;
CREATE USER regress_priv_user5; -- duplicate
ERROR: role "regress_priv_user5" already exists
+CREATE USER regress_priv_user6;
+CREATE USER regress_priv_user7;
+GRANT pg_read_all_data TO regress_priv_user6;
+GRANT pg_write_all_data TO regress_priv_user7;
CREATE GROUP regress_priv_group1;
CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4;
@@ -129,6 +134,29 @@ SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) );
------+------
(0 rows)
+SET SESSION AUTHORIZATION regress_priv_user6;
+SELECT * FROM atest1; -- ok
+ a | b
+---+-----
+ 1 | two
+ 1 | two
+(2 rows)
+
+SELECT * FROM atest2; -- ok
+ col1 | col2
+------+------
+(0 rows)
+
+INSERT INTO atest2 VALUES ('foo', true); -- fail
+ERROR: permission denied for table atest2
+SET SESSION AUTHORIZATION regress_priv_user7;
+SELECT * FROM atest1; -- fail
+ERROR: permission denied for table atest1
+SELECT * FROM atest2; -- fail
+ERROR: permission denied for table atest2
+INSERT INTO atest2 VALUES ('foo', true); -- ok
+UPDATE atest2 SET col2 = true; -- ok
+DELETE FROM atest2; -- ok
SET SESSION AUTHORIZATION regress_priv_user3;
SELECT session_user, current_user;
session_user | current_user
@@ -1674,6 +1702,12 @@ SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes
t
(1 row)
+SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes
+ has_schema_privilege
+----------------------
+ t
+(1 row)
+
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no
has_schema_privilege
----------------------
@@ -2045,7 +2079,9 @@ DROP USER regress_priv_user3;
DROP USER regress_priv_user4;
DROP USER regress_priv_user5;
DROP USER regress_priv_user6;
-ERROR: role "regress_priv_user6" does not exist
+DROP USER regress_priv_user7;
+DROP USER regress_priv_user8; -- does not exist
+ERROR: role "regress_priv_user8" does not exist
-- permissions with LOCK TABLE
CREATE USER regress_locktable_user;
CREATE TABLE lock_table (a int);
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 3550f61587..9919e8c490 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -16,6 +16,7 @@ DROP ROLE IF EXISTS regress_priv_user3;
DROP ROLE IF EXISTS regress_priv_user4;
DROP ROLE IF EXISTS regress_priv_user5;
DROP ROLE IF EXISTS regress_priv_user6;
+DROP ROLE IF EXISTS regress_priv_user7;
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
@@ -29,6 +30,11 @@ CREATE USER regress_priv_user3;
CREATE USER regress_priv_user4;
CREATE USER regress_priv_user5;
CREATE USER regress_priv_user5; -- duplicate
+CREATE USER regress_priv_user6;
+CREATE USER regress_priv_user7;
+
+GRANT pg_read_all_data TO regress_priv_user6;
+GRANT pg_write_all_data TO regress_priv_user7;
CREATE GROUP regress_priv_group1;
CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
@@ -94,6 +100,17 @@ GRANT ALL ON atest1 TO PUBLIC; -- fail
SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) );
SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) );
+SET SESSION AUTHORIZATION regress_priv_user6;
+SELECT * FROM atest1; -- ok
+SELECT * FROM atest2; -- ok
+INSERT INTO atest2 VALUES ('foo', true); -- fail
+
+SET SESSION AUTHORIZATION regress_priv_user7;
+SELECT * FROM atest1; -- fail
+SELECT * FROM atest2; -- fail
+INSERT INTO atest2 VALUES ('foo', true); -- ok
+UPDATE atest2 SET col2 = true; -- ok
+DELETE FROM atest2; -- ok
SET SESSION AUTHORIZATION regress_priv_user3;
SELECT session_user, current_user;
@@ -988,6 +1005,7 @@ ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_priv_user2;
CREATE SCHEMA testns2;
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes
+SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes
SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no
ALTER DEFAULT PRIVILEGES REVOKE USAGE ON SCHEMAS FROM regress_priv_user2;
@@ -1211,6 +1229,8 @@ DROP USER regress_priv_user3;
DROP USER regress_priv_user4;
DROP USER regress_priv_user5;
DROP USER regress_priv_user6;
+DROP USER regress_priv_user7;
+DROP USER regress_priv_user8; -- does not exist
-- permissions with LOCK TABLE
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 10:13 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 12:38 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Re: New default role- 'pg_read_all_data' Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-30 23:20 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-08-31 14:17 ` gkokolatos@pm.me
0 siblings, 0 replies; 25+ messages in thread
From: gkokolatos@pm.me @ 2020-08-31 14:17 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: Magnus Hagander <magnus@hagander.net>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, 31 August 2020 02:20, Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> - Stephen Frost (sfrost@snowman.net) wrote:
>
> > - Magnus Hagander (magnus@hagander.net) wrote:
> >
> > > On Fri, Aug 28, 2020 at 2:38 PM Stephen Frost sfrost@snowman.net wrote:
> > >
> > > > - Magnus Hagander (magnus@hagander.net) wrote:
> > > >
> > > > > Without having actually looked at the code, definite +1 for this feature.
> > > > > It's much requested...
> > > >
> > > > Thanks.
> > > >
> > > > > But, should we also have a pg_write_all_data to go along with it?
> > > >
> > > > Perhaps, but could certainly be a different patch, and it'd need to be
> > > > better defined, it seems to me... read_all is pretty straight-forward
> > > > (the general goal being "make pg_dumpall/pg_dump work"), what would
> > > > write mean? INSERT? DELETE? TRUNCATE? ALTER TABLE? System catalogs?
> > >
> > > Well, it's pg_write_all_data, so it certainly wouldn't be alter table or
> > > system catalogs.
> > > I'd say insert/update/delete yes.
> > > TRUNCATE is always an outlier.Given it's generally classified as DDL, I
> > > wouldn't include it.
> >
> > Alright, that seems like it'd be pretty easy. We already have a check
> > in pg_class_aclmask to disallow modification of system catalogs w/o
> > being a superuser, so we should be alright to add a similar check for
> > insert/update/delete just below where I added the SELECT check.
> >
> > > > Doesn't seem like you could just declare it to be 'allow pg_restore'
> > > > either, as that might include creating untrusted functions, et al.
> > >
> > > No definitely not. That wouldn't be the usecase at all :)
> >
> > Good. :)
> >
> > > (and fwiw to me the main use case for read_all_data also isn't pg_dump,
> > > because most people using pg_dump are already db owner or higher in my
> > > experience. But it is nice that it helps with that too)
> >
> > Glad to have confirmation that there's other use-cases this helps with.
> > I'll post an updated patch with that added in a day or two.
>
> Here's that updated patch, comments welcome.
Thank you for the updated patch!
Had a quick look on it and nothing stands out.
Also this sub-thread seems to have clearly responded on my early thoughts regarding invoking. Adding part of that subthread bellow:
>> My expectation was not met since in my manual test (unless I made a mistake which is entirely possible), the SELECT above did not fail. The insert did succeed though.
> That the INSERT worked seems pretty odd- could you post the exact
> changes you've made to the regression tests, or the exact script where
> you aren't seeing what you expect? I've not been able to reproduce the
> GRANT allowing a user to INSERT into a table.
>> The first question: Was my expectation wrong?
> If there aren't any other privileges involved, then REVOKE'ing the role
> from a user should prevent that user from being able to SELECT from the
> table.
>> The second question: Is there a privilege that can be granted to regress_priv_user6 that will not permit the select operation but will permit the insert operation? If no, should there be one?
As discussed above, while I was struggling to formulate the thought, Magnus had already proposed pg_write_all_data and the community had reached a consensus on what it actually means.
Please find attached a minimal test case and the output of it.
It is obvious that I was confused and added confusion to the thread. Permissions are additive and autonomous. Now it is rather clear to me what the expectations are and how the patch should behave.
To add to my embarrassment, the REVOKE operation emitted a warning which I had clearly missed.
Apologies.
//Georgios
>
> Thanks!
>
> Stephen
Attachments:
pg_rorole_v2_minimal_test.out (application/octet-stream, 979B) download
pg_rorole_v2_minimal_test.sql (application/sql, 903B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-10-29 11:27 ` Georgios Kokolatos <gkokolatos@protonmail.com>
2020-10-29 14:19 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2 siblings, 1 reply; 25+ messages in thread
From: Georgios Kokolatos @ 2020-10-29 11:27 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Stephen Frost <sfrost@snowman.net>
Hi,
this patch is in "Ready for committer" state and the Cfbot is happy.
Is there any committer that is available for taking a look at it?
Cheers,
//Georgios - CFM 2020-11
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-10-29 11:27 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
@ 2020-10-29 14:19 ` Stephen Frost <sfrost@snowman.net>
2020-11-23 22:08 ` Re: New default role- 'pg_read_all_data' Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
0 siblings, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-10-29 14:19 UTC (permalink / raw)
To: Georgios Kokolatos <gkokolatos@protonmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
Greetings,
* Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
> this patch is in "Ready for committer" state and the Cfbot is happy.
Glad that's still the case. :)
> Is there any committer that is available for taking a look at it?
If there aren't any objections or further comments, I'll take another
look through it and will commit it during the upcoming CF.
Thanks!
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-10-29 11:27 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-10-29 14:19 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2020-11-23 22:08 ` Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
2020-11-23 22:31 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
0 siblings, 1 reply; 25+ messages in thread
From: Anastasia Lubennikova @ 2020-11-23 22:08 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; Georgios Kokolatos <gkokolatos@protonmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On 29.10.2020 17:19, Stephen Frost wrote:
> Greetings,
>
> * Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
>> this patch is in "Ready for committer" state and the Cfbot is happy.
> Glad that's still the case. :)
>
>> Is there any committer that is available for taking a look at it?
> If there aren't any objections or further comments, I'll take another
> look through it and will commit it during the upcoming CF.
>
> Thanks!
>
> Stephen
CFM reminder. Just in case you forgot about this thread)
The commitfest is heading to the end. And there was a plenty of time for
anyone to object.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-10-29 11:27 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-10-29 14:19 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-11-23 22:08 ` Re: New default role- 'pg_read_all_data' Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
@ 2020-11-23 22:31 ` Stephen Frost <sfrost@snowman.net>
2021-03-04 14:47 ` Re: New default role- 'pg_read_all_data' gkokolatos@pm.me
0 siblings, 1 reply; 25+ messages in thread
From: Stephen Frost @ 2020-11-23 22:31 UTC (permalink / raw)
To: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>; +Cc: Georgios Kokolatos <gkokolatos@protonmail.com>; pgsql-hackers@lists.postgresql.org
Greetings,
* Anastasia Lubennikova (a.lubennikova@postgrespro.ru) wrote:
> On 29.10.2020 17:19, Stephen Frost wrote:
> >* Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
> >>this patch is in "Ready for committer" state and the Cfbot is happy.
> >Glad that's still the case. :)
> >
> >>Is there any committer that is available for taking a look at it?
> >If there aren't any objections or further comments, I'll take another
> >look through it and will commit it during the upcoming CF.
> >
> >Thanks!
> >
> >Stephen
>
> CFM reminder. Just in case you forgot about this thread)
> The commitfest is heading to the end. And there was a plenty of time for
> anyone to object.
Thanks, I've not forgotten, but it's a bit complicated given that I've
another patch in progress to rename default roles to be predefined
roles which conflicts with this one. Hopefully we'll have a few
comments on that and I can get it committed and this one updated with
the new naming. I'd rather not commit this one and then immediately
commit changes over top of it.
Thanks again!
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: New default role- 'pg_read_all_data'
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-10-29 11:27 ` Re: New default role- 'pg_read_all_data' Georgios Kokolatos <gkokolatos@protonmail.com>
2020-10-29 14:19 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-11-23 22:08 ` Re: New default role- 'pg_read_all_data' Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
2020-11-23 22:31 ` Re: New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
@ 2021-03-04 14:47 ` gkokolatos@pm.me
0 siblings, 0 replies; 25+ messages in thread
From: gkokolatos@pm.me @ 2021-03-04 14:47 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>; pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Hi,
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, November 23, 2020 11:31 PM, Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
> - Anastasia Lubennikova (a.lubennikova@postgrespro.ru) wrote:
>
> > On 29.10.2020 17:19, Stephen Frost wrote:
> >
> > > - Georgios Kokolatos (gkokolatos@protonmail.com) wrote:
> > >
> > > > this patch is in "Ready for committer" state and the Cfbot is happy.
> > > > Glad that's still the case. :)
> > >
> > > > Is there any committer that is available for taking a look at it?
> > > > If there aren't any objections or further comments, I'll take another
> > > > look through it and will commit it during the upcoming CF.
> > >
> > > Thanks!
> > > Stephen
> >
> > CFM reminder. Just in case you forgot about this thread)
> > The commitfest is heading to the end. And there was a plenty of time for
> > anyone to object.
>
> Thanks, I've not forgotten, but it's a bit complicated given that I've
> another patch in progress to rename default roles to be predefined
> roles which conflicts with this one. Hopefully we'll have a few
> comments on that and I can get it committed and this one updated with
> the new naming. I'd rather not commit this one and then immediately
> commit changes over top of it.
May I enquire about the status of the current?
//Georgios
>
> Thanks again!
>
> Stephen
^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2021-03-04 14:47 UTC | newest]
Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 00:30 New default role- 'pg_read_all_data' Stephen Frost <sfrost@snowman.net>
2020-08-28 01:17 ` Steven Pousty <steve.pousty@gmail.com>
2020-08-28 10:13 ` Magnus Hagander <magnus@hagander.net>
2020-08-28 12:16 ` Georgios Kokolatos <gkokolatos@protonmail.com>
2020-08-28 12:43 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 12:51 ` Isaac Morland <isaac.morland@gmail.com>
2020-08-28 12:54 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 13:24 ` Isaac Morland <isaac.morland@gmail.com>
2020-08-28 12:53 ` Magnus Hagander <magnus@hagander.net>
2020-08-28 13:06 ` gkokolatos@pm.me
2020-08-28 13:18 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 12:38 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 12:50 ` Magnus Hagander <magnus@hagander.net>
2020-08-28 13:26 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 13:58 ` Gilles Darold <gilles@darold.net>
2020-08-28 14:52 ` Stephen Frost <sfrost@snowman.net>
2020-08-28 15:21 ` Gilles Darold <gilles@darold.net>
2020-08-28 17:25 ` Stephen Frost <sfrost@snowman.net>
2020-08-30 23:20 ` Stephen Frost <sfrost@snowman.net>
2020-08-31 14:17 ` gkokolatos@pm.me
2020-10-29 11:27 ` Georgios Kokolatos <gkokolatos@protonmail.com>
2020-10-29 14:19 ` Stephen Frost <sfrost@snowman.net>
2020-11-23 22:08 ` Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
2020-11-23 22:31 ` Stephen Frost <sfrost@snowman.net>
2021-03-04 14:47 ` gkokolatos@pm.me
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox