Revert "Avoid race condition between "GRANT role" and "DROP ROLE"".

This reverts commit 98fc31d6499163a0a781aa6f13582a07f09cd7c6.
That change allowed DROP OWNED BY to drop grants of the target
role to other roles, arguing that nobody would need those
privileges anymore.  But that's not so: if you're not superuser,
you still need admin privilege on the target role so you can
drop it.

It's not clear whether or how the dependency-based approach
to solving the original problem can be adapted to keep these
grants.  Since v18 release is fast approaching, the sanest
thing to do seems to be to revert this patch for now.  The
race-condition problem is low severity and not worth taking
risks for.

I didn't force a catversion bump in 98fc31d64, so I won't do
so here either.

Reported-by: Dipesh Dhameliya <dipeshdhameliya125@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CABgZEgczOFicCJoqtrH9gbYMe_BV3Hq8zzCBRcMgmU6LRsihUA@mail.gmail.com
Backpatch-through: 18

diff --git a/doc/src/sgml/ref/drop_owned.sgml b/doc/src/sgml/ref/drop_owned.sgml
index 46e1c229ec0fb762686f0c36b6c4522e135248f3..efda01a39e88bc7a008d4f0df7e826f377b6a91f 100644 (file)
--- a/doc/src/sgml/ref/drop_owned.sgml
+++ b/doc/src/sgml/ref/drop_owned.sgml
@@ -33,7 +33,7 @@ DROP OWNED BY { <replaceable class="parameter">name</replaceable> | CURRENT_ROLE
    database that are owned by one of the specified roles. Any
    privileges granted to the given roles on objects in the current
    database or on shared objects (databases, tablespaces, configuration
-   parameters, or other roles) will also be revoked.
+   parameters) will also be revoked.
   </para>
  </refsect1>
 

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 0d638e29d0066415abab9a5a898beff3a17c3a8e..5ce8ffa863a725eca8ad96f4ee1c0f9ba51e5232 100644 (file)
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -30,7 +30,6 @@
 #include "commands/defrem.h"
 #include "commands/seclabel.h"
 #include "commands/user.h"
-#include "lib/qunique.h"
 #include "libpq/crypt.h"
 #include "miscadmin.h"
 #include "storage/lmgr.h"
@@ -490,7 +489,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
         * Advance command counter so we can see new record; else tests in
         * AddRoleMems may fail.
         */
-       CommandCounterIncrement();
+       if (addroleto || adminmembers || rolemembers)
+               CommandCounterIncrement();
 
        /* Default grant. */
        InitGrantRoleOptions(&popt);
@@ -1904,8 +1904,7 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
                else
                {
                        Oid                     objectId;
-                       Oid                *newmembers = (Oid *) palloc(3 * sizeof(Oid));
-                       int                     nnewmembers;
+                       Oid                *newmembers = palloc(sizeof(Oid));
 
                        /*
                         * The values for these options can be taken directly from 'popt'.
@@ -1947,22 +1946,12 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
                                                                        new_record, new_record_nulls);
                        CatalogTupleInsert(pg_authmem_rel, tuple);
 
-                       /*
-                        * Record dependencies on the roleid, member, and grantor, as if a
-                        * pg_auth_members entry were an object ACL.
-                        * updateAclDependencies() requires an input array that is
-                        * palloc'd (it will free it), sorted, and de-duped.
-                        */
-                       newmembers[0] = roleid;
-                       newmembers[1] = memberid;
-                       newmembers[2] = grantorId;
-                       qsort(newmembers, 3, sizeof(Oid), oid_cmp);
-                       nnewmembers = qunique(newmembers, 3, sizeof(Oid), oid_cmp);
-
+                       /* updateAclDependencies wants to pfree array inputs */
+                       newmembers[0] = grantorId;
                        updateAclDependencies(AuthMemRelationId, objectId,
                                                                  0, InvalidOid,
                                                                  0, NULL,
-                                                                 nnewmembers, newmembers);
+                                                                 1, newmembers);
                }
 
                /* CCI after each change, in case there are duplicates in list */

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index a8997a3eea5d47a2e67df3591536b6d7bae4f6ec..823b824c203f9bbc73ad6122baa17c97a809b709 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -113,36 +113,6 @@ CREATE USER regress_priv_user2;
 CREATE USER regress_priv_user3;
 CREATE USER regress_priv_user4;
 CREATE USER regress_priv_user5;
--- DROP OWNED should also act on granted and granted-to roles
-GRANT regress_priv_user1 TO regress_priv_user2;
-GRANT regress_priv_user2 TO regress_priv_user3;
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
-       roleid       |       member       
---------------------+--------------------
- regress_priv_user1 | regress_priv_user2
- regress_priv_user2 | regress_priv_user3
-(2 rows)
-
-REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4;  -- no effect
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
-       roleid       |       member       
---------------------+--------------------
- regress_priv_user1 | regress_priv_user2
- regress_priv_user2 | regress_priv_user3
-(2 rows)
-
-DROP OWNED BY regress_priv_user2;  -- removes both grants
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
- roleid | member 
---------+--------
-(0 rows)
-
 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/privileges.sql b/src/test/regress/sql/privileges.sql
index e3f047fff1946a215146d11c969d6d775976768d..75fe68df333c128ccb72bcd2f592e5189a1dcb91 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -90,21 +90,6 @@ CREATE USER regress_priv_user3;
 CREATE USER regress_priv_user4;
 CREATE USER regress_priv_user5;
 
--- DROP OWNED should also act on granted and granted-to roles
-GRANT regress_priv_user1 TO regress_priv_user2;
-GRANT regress_priv_user2 TO regress_priv_user3;
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
-REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4;  -- no effect
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
-DROP OWNED BY regress_priv_user2;  -- removes both grants
-SELECT roleid::regrole, member::regrole FROM pg_auth_members
-  WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
-  ORDER BY roleid::regrole::text;
-
 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;