Ensure that pg_auth_members.grantor is always valid.

Previously, "GRANT foo TO bar" or "GRANT foo TO bar GRANTED BY baz"
would record the OID of the grantor in pg_auth_members.grantor, but
that role could later be dropped without modifying or removing the
pg_auth_members record. That's not great, because we typically try
to avoid dangling references in catalog data.

Now, a role grant depends on the grantor, and the grantor can't be
dropped without removing the grant or changing the grantor.  "DROP
OWNED BY" will remove the grant, just as it does for other kinds of
privileges. "REASSIGN OWNED BY" will not, again just like what we do
in other cases involving privileges.

pg_auth_members now has an OID column, because that is needed in order
for dependencies to work. It also now has an index on the grantor
column, because otherwise dropping a role would require a sequential
scan of the entire table to see whether the role's OID is in use as
a grantor. That probably wouldn't be too large a problem in practice,
but it seems better to have an index just in case.

A follow-on patch is planned with the goal of more thoroughly
rationalizing the behavior of role grants. This patch is just trying
to do enough to make sure that the data we store in the catalogs is at
some basic level valid.

Patch by me, reviewed by Stephen Frost

Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com

4 files changed, 80 insertions, 2 deletions

diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 4e67d727603..c2465d0f492 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -103,9 +103,21 @@ ERROR:  role "regress_nosuch_recursive" does not exist
 DROP ROLE regress_nosuch_admin_recursive;
 ERROR:  role "regress_nosuch_admin_recursive" does not exist
 DROP ROLE regress_plainrole;
+-- fail, can't drop regress_createrole yet, due to outstanding grants
+DROP ROLE regress_createrole;
+ERROR:  role "regress_createrole" cannot be dropped because some objects depend on it
+DETAIL:  privileges for membership of role regress_read_all_data in role pg_read_all_data
+privileges for membership of role regress_write_all_data in role pg_write_all_data
+privileges for membership of role regress_monitor in role pg_monitor
+privileges for membership of role regress_read_all_settings in role pg_read_all_settings
+privileges for membership of role regress_read_all_stats in role pg_read_all_stats
+privileges for membership of role regress_stat_scan_tables in role pg_stat_scan_tables
+privileges for membership of role regress_read_server_files in role pg_read_server_files
+privileges for membership of role regress_write_server_files in role pg_write_server_files
+privileges for membership of role regress_execute_server_program in role pg_execute_server_program
+privileges for membership of role regress_signal_backend in role pg_signal_backend
 -- ok, should be able to drop non-superuser roles we created
 DROP ROLE regress_createdb;
-DROP ROLE regress_createrole;
 DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
@@ -125,6 +137,8 @@ DROP ROLE regress_read_server_files;
 DROP ROLE regress_write_server_files;
 DROP ROLE regress_execute_server_program;
 DROP ROLE regress_signal_backend;
+-- ok, dropped the other roles first so this is ok now
+DROP ROLE regress_createrole;
 -- fail, role still owns database objects
 DROP ROLE regress_tenant;
 ERROR:  role "regress_tenant" cannot be dropped because some objects depend on it
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index e10dd6f9ae5..65b4a22ebc5 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -33,6 +33,38 @@ CREATE USER regress_priv_user8;
 CREATE USER regress_priv_user9;
 CREATE USER regress_priv_user10;
 CREATE ROLE regress_priv_role;
+-- test GRANTED BY with DROP OWNED and REASSIGN OWNED
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user3 GRANTED BY regress_priv_user2;
+DROP ROLE regress_priv_user2; -- fail, dependency
+ERROR:  role "regress_priv_user2" cannot be dropped because some objects depend on it
+DETAIL:  privileges for membership of role regress_priv_user3 in role regress_priv_user1
+REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4;
+DROP ROLE regress_priv_user2; -- still fail, REASSIGN OWNED doesn't help
+ERROR:  role "regress_priv_user2" cannot be dropped because some objects depend on it
+DETAIL:  privileges for membership of role regress_priv_user3 in role regress_priv_user1
+DROP OWNED BY regress_priv_user2;
+DROP ROLE regress_priv_user2; -- ok now, DROP OWNED does the job
+-- test that removing granted role or grantee role removes dependency
+GRANT regress_priv_user1 TO regress_priv_user3 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user4 GRANTED BY regress_priv_user3;
+DROP ROLE regress_priv_user3; -- should fail, dependency
+ERROR:  role "regress_priv_user3" cannot be dropped because some objects depend on it
+DETAIL:  privileges for membership of role regress_priv_user4 in role regress_priv_user1
+DROP ROLE regress_priv_user4; -- ok
+DROP ROLE regress_priv_user3; -- ok now
+GRANT regress_priv_user1 TO regress_priv_user5 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user6 GRANTED BY regress_priv_user5;
+DROP ROLE regress_priv_user5; -- should fail, dependency
+ERROR:  role "regress_priv_user5" cannot be dropped because some objects depend on it
+DETAIL:  privileges for membership of role regress_priv_user6 in role regress_priv_user1
+DROP ROLE regress_priv_user1, regress_priv_user5; -- ok, despite order
+-- recreate the roles we just dropped
+CREATE USER regress_priv_user1;
+CREATE USER regress_priv_user2;
+CREATE USER regress_priv_user3;
+CREATE USER regress_priv_user4;
+CREATE USER regress_priv_user5;
 GRANT pg_read_all_data TO regress_priv_user6;
 GRANT pg_write_all_data TO regress_priv_user7;
 GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 292dc087975..b696628238a 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -98,9 +98,11 @@ DROP ROLE regress_nosuch_recursive;
 DROP ROLE regress_nosuch_admin_recursive;
 DROP ROLE regress_plainrole;
 
+-- fail, can't drop regress_createrole yet, due to outstanding grants
+DROP ROLE regress_createrole;
+
 -- ok, should be able to drop non-superuser roles we created
 DROP ROLE regress_createdb;
-DROP ROLE regress_createrole;
 DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
@@ -121,6 +123,9 @@ DROP ROLE regress_write_server_files;
 DROP ROLE regress_execute_server_program;
 DROP ROLE regress_signal_backend;
 
+-- ok, dropped the other roles first so this is ok now
+DROP ROLE regress_createrole;
+
 -- fail, role still owns database objects
 DROP ROLE regress_tenant;
 
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 6d1fd3391a2..66834e32a7e 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -37,6 +37,33 @@ CREATE USER regress_priv_user9;
 CREATE USER regress_priv_user10;
 CREATE ROLE regress_priv_role;
 
+-- test GRANTED BY with DROP OWNED and REASSIGN OWNED
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user3 GRANTED BY regress_priv_user2;
+DROP ROLE regress_priv_user2; -- fail, dependency
+REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4;
+DROP ROLE regress_priv_user2; -- still fail, REASSIGN OWNED doesn't help
+DROP OWNED BY regress_priv_user2;
+DROP ROLE regress_priv_user2; -- ok now, DROP OWNED does the job
+
+-- test that removing granted role or grantee role removes dependency
+GRANT regress_priv_user1 TO regress_priv_user3 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user4 GRANTED BY regress_priv_user3;
+DROP ROLE regress_priv_user3; -- should fail, dependency
+DROP ROLE regress_priv_user4; -- ok
+DROP ROLE regress_priv_user3; -- ok now
+GRANT regress_priv_user1 TO regress_priv_user5 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user6 GRANTED BY regress_priv_user5;
+DROP ROLE regress_priv_user5; -- should fail, dependency
+DROP ROLE regress_priv_user1, regress_priv_user5; -- ok, despite order
+
+-- recreate the roles we just dropped
+CREATE USER regress_priv_user1;
+CREATE USER regress_priv_user2;
+CREATE USER regress_priv_user3;
+CREATE USER regress_priv_user4;
+CREATE USER regress_priv_user5;
+
 GRANT pg_read_all_data TO regress_priv_user6;
 GRANT pg_write_all_data TO regress_priv_user7;
 GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;