diff --git a/doc/src/sgml/ref/drop_owned.sgml b/doc/src/sgml/ref/drop_owned.sgml index efda01a39e88..46e1c229ec0f 100644 --- a/doc/src/sgml/ref/drop_owned.sgml +++ b/doc/src/sgml/ref/drop_owned.sgml @@ -33,7 +33,7 @@ DROP OWNED BY { name | 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) will also be revoked. + parameters, or other roles) will also be revoked. diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 0db174e6f106..0c84886e82ec 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -30,6 +30,7 @@ #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" @@ -489,7 +490,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) * Advance command counter so we can see new record; else tests in * AddRoleMems may fail. */ - if (addroleto || adminmembers || rolemembers) + if (addroleto || adminmembers || rolemembers || !superuser()) CommandCounterIncrement(); /* Default grant. */ @@ -1904,7 +1905,8 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid, else { Oid objectId; - Oid *newmembers = palloc(sizeof(Oid)); + Oid *newmembers = (Oid *) palloc(3 * sizeof(Oid)); + int nnewmembers; /* * The values for these options can be taken directly from 'popt'. @@ -1946,12 +1948,22 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid, new_record, new_record_nulls); CatalogTupleInsert(pg_authmem_rel, tuple); - /* updateAclDependencies wants to pfree array inputs */ - newmembers[0] = grantorId; + /* + * 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(AuthMemRelationId, objectId, 0, InvalidOid, 0, NULL, - 1, newmembers); + nnewmembers, newmembers); } /* CCI after each change, in case there are duplicates in list */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index b993b05cc220..b9b22c47be6d 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -986,6 +986,13 @@ dumpRoleMembership(PGconn *conn) total; bool dump_grantors; bool dump_grant_options; + int i_role; + int i_member; + int i_grantor; + int i_roleid; + int i_memberid; + int i_grantorid; + int i_admin_option; int i_inherit_option; int i_set_option; @@ -995,6 +1002,10 @@ dumpRoleMembership(PGconn *conn) * they didn't have ADMIN OPTION on the role, or a user that no longer * existed. To avoid dump and restore failures, don't dump the grantor * when talking to an old server version. + * + * Also, in older versions the roleid and/or member could be role OIDs + * that no longer exist. If we find such cases, print a warning and skip + * the entry. */ dump_grantors = (PQserverVersion(conn) >= 160000); @@ -1006,8 +1017,10 @@ dumpRoleMembership(PGconn *conn) /* Generate and execute query. */ printfPQExpBuffer(buf, "SELECT ur.rolname AS role, " "um.rolname AS member, " - "ug.oid AS grantorid, " "ug.rolname AS grantor, " + "a.roleid AS roleid, " + "a.member AS memberid, " + "a.grantor AS grantorid, " "a.admin_option"); if (dump_grant_options) appendPQExpBufferStr(buf, ", a.inherit_option, a.set_option"); @@ -1016,8 +1029,15 @@ dumpRoleMembership(PGconn *conn) "LEFT JOIN %s um on um.oid = a.member " "LEFT JOIN %s ug on ug.oid = a.grantor " "WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')" - "ORDER BY 1,2,4", role_catalog, role_catalog, role_catalog); + "ORDER BY 1,2,3", role_catalog, role_catalog, role_catalog); res = executeQuery(conn, buf->data); + i_role = PQfnumber(res, "role"); + i_member = PQfnumber(res, "member"); + i_grantor = PQfnumber(res, "grantor"); + i_roleid = PQfnumber(res, "roleid"); + i_memberid = PQfnumber(res, "memberid"); + i_grantorid = PQfnumber(res, "grantorid"); + i_admin_option = PQfnumber(res, "admin_option"); i_inherit_option = PQfnumber(res, "inherit_option"); i_set_option = PQfnumber(res, "set_option"); @@ -1041,24 +1061,32 @@ dumpRoleMembership(PGconn *conn) total = PQntuples(res); while (start < total) { - char *role = PQgetvalue(res, start, 0); + char *role = PQgetvalue(res, start, i_role); int i; bool *done; int remaining; int prev_remaining = 0; rolename_hash *ht; + /* If we hit a null roleid, we're done (nulls sort to the end). */ + if (PQgetisnull(res, start, i_role)) + { + /* translator: %s represents a numeric role OID */ + pg_log_warning("found dangling pg_auth_members entry for role %s", + PQgetvalue(res, start, i_roleid)); + break; + } + /* All memberships for a single role should be adjacent. */ for (end = start; end < total; ++end) { char *otherrole; - otherrole = PQgetvalue(res, end, 0); + otherrole = PQgetvalue(res, end, i_role); if (strcmp(role, otherrole) != 0) break; } - role = PQgetvalue(res, start, 0); remaining = end - start; done = pg_malloc0(remaining * sizeof(bool)); ht = rolename_create(remaining, NULL); @@ -1098,10 +1126,30 @@ dumpRoleMembership(PGconn *conn) if (done[i - start]) continue; - member = PQgetvalue(res, i, 1); - grantorid = PQgetvalue(res, i, 2); - grantor = PQgetvalue(res, i, 3); - admin_option = PQgetvalue(res, i, 4); + /* Complain about, then ignore, entries with dangling OIDs. */ + if (PQgetisnull(res, i, i_member)) + { + /* translator: %s represents a numeric role OID */ + pg_log_warning("found dangling pg_auth_members entry for role %s", + PQgetvalue(res, i, i_memberid)); + done[i - start] = true; + --remaining; + continue; + } + if (PQgetisnull(res, i, i_grantor)) + { + /* translator: %s represents a numeric role OID */ + pg_log_warning("found dangling pg_auth_members entry for role %s", + PQgetvalue(res, i, i_grantorid)); + done[i - start] = true; + --remaining; + continue; + } + + member = PQgetvalue(res, i, i_member); + grantor = PQgetvalue(res, i, i_grantor); + grantorid = PQgetvalue(res, i, i_grantorid); + admin_option = PQgetvalue(res, i, i_admin_option); if (dump_grant_options) set_option = PQgetvalue(res, i, i_set_option); diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 6b01313101b6..a76256405fe2 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -113,6 +113,36 @@ 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 60e7443bf592..d195aaf13774 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -90,6 +90,21 @@ 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;