diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 5b869e91c17..375fb527af2 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1778,6 +1778,8 @@ ReassignOwnedObjects(ReassignOwnedStmt *stmt) * roleSpecsToIds * * Given a list of RoleSpecs, generate a list of role OIDs in the same order. + * Each role is locked with AccessShareLock to prevent concurrent DROP ROLE + * from removing it between resolution and the caller's catalog update. * * ROLESPEC_PUBLIC is not allowed. */ @@ -1792,7 +1794,24 @@ roleSpecsToIds(List *memberNames) RoleSpec *rolespec = lfirst_node(RoleSpec, l); Oid roleid; - roleid = get_rolespec_oid(rolespec, false); + if (rolespec->roletype == ROLESPEC_CSTRING) + roleid = RoleNameGetOid(rolespec->rolename, + AccessShareLock, false, + NULL, NULL); + else + { + roleid = get_rolespec_oid(rolespec, false); + LockSharedObject(AuthIdRelationId, roleid, 0, + AccessShareLock); + + /* Recheck that the role still exists after locking. */ + if (!SearchSysCacheExists1(AUTHOID, ObjectIdGetDatum(roleid))) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("role \"%s\" does not exist", + get_rolespec_name(rolespec)))); + } + result = lappend_oid(result, roleid); } return result; diff --git a/src/test/isolation/expected/role-membership-drop-member.out b/src/test/isolation/expected/role-membership-drop-member.out new file mode 100644 index 00000000000..5b267ea32c2 --- /dev/null +++ b/src/test/isolation/expected/role-membership-drop-member.out @@ -0,0 +1,75 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin s1_grant s2_drop_member s1_commit s2_check_orphans +step s1_begin: BEGIN; +step s1_grant: GRANT regress_role_group TO regress_role_member; +step s2_drop_member: DROP ROLE regress_role_member; +step s1_commit: COMMIT; +step s2_drop_member: <... completed> +step s2_check_orphans: + SELECT count(*) + FROM pg_auth_members m + LEFT JOIN pg_authid ra ON m.roleid = ra.oid + LEFT JOIN pg_authid me ON m.member = me.oid + LEFT JOIN pg_authid gr ON m.grantor = gr.oid + WHERE ra.oid IS NULL OR me.oid IS NULL OR gr.oid IS NULL; + +count +----- + 0 +(1 row) + + +starting permutation: s1_begin s1_alter_add s2_drop_member s1_commit s2_check_orphans +step s1_begin: BEGIN; +step s1_alter_add: ALTER GROUP regress_role_group ADD USER regress_role_member; +step s2_drop_member: DROP ROLE regress_role_member; +step s1_commit: COMMIT; +step s2_drop_member: <... completed> +step s2_check_orphans: + SELECT count(*) + FROM pg_auth_members m + LEFT JOIN pg_authid ra ON m.roleid = ra.oid + LEFT JOIN pg_authid me ON m.member = me.oid + LEFT JOIN pg_authid gr ON m.grantor = gr.oid + WHERE ra.oid IS NULL OR me.oid IS NULL OR gr.oid IS NULL; + +count +----- + 0 +(1 row) + + +starting permutation: s1_begin s1_create_role s2_drop_member s1_commit s2_check_orphans +step s1_begin: BEGIN; +step s1_create_role: CREATE ROLE regress_role_new ROLE regress_role_member; +step s2_drop_member: DROP ROLE regress_role_member; +step s1_commit: COMMIT; +step s2_drop_member: <... completed> +step s2_check_orphans: + SELECT count(*) + FROM pg_auth_members m + LEFT JOIN pg_authid ra ON m.roleid = ra.oid + LEFT JOIN pg_authid me ON m.member = me.oid + LEFT JOIN pg_authid gr ON m.grantor = gr.oid + WHERE ra.oid IS NULL OR me.oid IS NULL OR gr.oid IS NULL; + +count +----- + 0 +(1 row) + + +starting permutation: s1_begin s1_drop_owned s2_drop_member s1_commit +step s1_begin: BEGIN; +step s1_drop_owned: DROP OWNED BY regress_role_member; +step s2_drop_member: DROP ROLE regress_role_member; +step s1_commit: COMMIT; +step s2_drop_member: <... completed> + +starting permutation: s1_begin s1_reassign_owned s2_drop_member s1_commit +step s1_begin: BEGIN; +step s1_reassign_owned: REASSIGN OWNED BY regress_role_member TO regress_role_group; +step s2_drop_member: DROP ROLE regress_role_member; +step s1_commit: COMMIT; +step s2_drop_member: <... completed> diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index b8ebe92553c..8fb8b52b77f 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -128,3 +128,4 @@ test: matview-write-skew test: lock-nowait test: for-portion-of test: ddl-dependency-locking +test: role-membership-drop-member diff --git a/src/test/isolation/specs/role-membership-drop-member.spec b/src/test/isolation/specs/role-membership-drop-member.spec new file mode 100644 index 00000000000..989fe996249 --- /dev/null +++ b/src/test/isolation/specs/role-membership-drop-member.spec @@ -0,0 +1,51 @@ +# Test that role membership commands properly lock the grantee/member +# role to prevent concurrent DROP ROLE from creating orphaned pg_auth_members +# entries, or from operating on a stale OID. + +setup +{ + CREATE ROLE regress_role_group; + CREATE ROLE regress_role_member; +} + +teardown +{ + DROP ROLE IF EXISTS regress_role_group; + DROP ROLE IF EXISTS regress_role_member; + DROP ROLE IF EXISTS regress_role_new; +} + +session s1 +step s1_begin { BEGIN; } +step s1_grant { GRANT regress_role_group TO regress_role_member; } +step s1_alter_add { ALTER GROUP regress_role_group ADD USER regress_role_member; } +step s1_create_role { CREATE ROLE regress_role_new ROLE regress_role_member; } +step s1_drop_owned { DROP OWNED BY regress_role_member; } +step s1_reassign_owned { REASSIGN OWNED BY regress_role_member TO regress_role_group; } +step s1_commit { COMMIT; } + +session s2 +step s2_drop_member { DROP ROLE regress_role_member; } +step s2_check_orphans { + SELECT count(*) + FROM pg_auth_members m + LEFT JOIN pg_authid ra ON m.roleid = ra.oid + LEFT JOIN pg_authid me ON m.member = me.oid + LEFT JOIN pg_authid gr ON m.grantor = gr.oid + WHERE ra.oid IS NULL OR me.oid IS NULL OR gr.oid IS NULL; +} + +# GRANT role TO member - concurrent DROP of the member +permutation s1_begin s1_grant s2_drop_member s1_commit s2_check_orphans + +# ALTER GROUP ADD USER - concurrent DROP of the member +permutation s1_begin s1_alter_add s2_drop_member s1_commit s2_check_orphans + +# CREATE ROLE ... ROLE member - concurrent DROP of the member +permutation s1_begin s1_create_role s2_drop_member s1_commit s2_check_orphans + +# DROP OWNED BY role - concurrent DROP of the role +permutation s1_begin s1_drop_owned s2_drop_member s1_commit + +# REASSIGN OWNED BY role - concurrent DROP of the role +permutation s1_begin s1_reassign_owned s2_drop_member s1_commit -- 2.34.1