From 4d22832f4ae9a3d08b03f0fbacbb3d6e1c22cd80 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Mon, 27 Sep 2021 10:09:45 -0700 Subject: [PATCH v1 2/4] Add owners to roles All roles now have owners. By default, roles belong to the role that created them, and initdb-time roles are owned by POSTGRES. This is a preparatory patch for changing how CREATEROLE works. --- src/backend/commands/alter.c | 3 ++ src/backend/commands/user.c | 45 ++++++++++++++++++- src/backend/parser/gram.y | 12 +++++ src/include/catalog/pg_authid.h | 1 + src/include/commands/user.h | 1 + .../unsafe_tests/expected/rolenames.out | 6 ++- .../modules/unsafe_tests/sql/rolenames.sql | 3 +- src/test/regress/expected/create_role.out | 32 ++++++++++++- src/test/regress/expected/oidjoins.out | 1 + src/test/regress/expected/privileges.out | 9 +++- src/test/regress/sql/create_role.sql | 8 +++- src/test/regress/sql/privileges.sql | 13 +++++- 12 files changed, 127 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index c47d54e96b..979034ab2f 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -839,6 +839,9 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt) case OBJECT_DATABASE: return AlterDatabaseOwner(strVal(stmt->object), newowner); + case OBJECT_ROLE: + return AlterRoleOwner(strVal(stmt->object), newowner); + case OBJECT_SCHEMA: return AlterSchemaOwner(strVal(stmt->object), newowner); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index aa69821be4..815c7095ec 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -77,6 +77,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) Datum new_record[Natts_pg_authid]; bool new_record_nulls[Natts_pg_authid]; Oid roleid; + Oid owner; ListCell *item; ListCell *option; char *password = NULL; /* user password */ @@ -108,6 +109,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dvalidUntil = NULL; DefElem *dbypassRLS = NULL; + /* Make more flexible later */ + owner = GetUserId(); + /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) { @@ -345,6 +349,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DirectFunctionCall1(namein, CStringGetDatum(stmt->role)); new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(issuper); + new_record[Anum_pg_authid_rolowner - 1] = ObjectIdGetDatum(owner); new_record[Anum_pg_authid_rolinherit - 1] = BoolGetDatum(inherit); new_record[Anum_pg_authid_rolcreaterole - 1] = BoolGetDatum(createrole); new_record[Anum_pg_authid_rolcreatedb - 1] = BoolGetDatum(createdb); @@ -422,6 +427,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) */ CatalogTupleInsert(pg_authid_rel, tuple); + recordDependencyOnOwner(AuthIdRelationId, roleid, owner); + /* * Advance command counter so we can see new record; else tests in * AddRoleMems may fail. @@ -1078,8 +1085,9 @@ DropRole(DropRoleStmt *stmt) systable_endscan(sscan); /* - * Remove any comments or security labels on this role. + * Remove any dependencies, comments or security labels on this role. */ + deleteSharedDependencyRecordsFor(AuthIdRelationId, roleid, 0); DeleteSharedComments(roleid, AuthIdRelationId); DeleteSharedSecurityLabel(roleid, AuthIdRelationId); @@ -1675,3 +1683,38 @@ DelRoleMems(const char *rolename, Oid roleid, */ table_close(pg_authmem_rel, NoLock); } + +/* + * Change role owner + */ +ObjectAddress +AlterRoleOwner(const char *name, Oid newOwnerId) +{ + Oid roleid; + HeapTuple tup; + Relation rel; + ObjectAddress address; + Form_pg_authid authform; + + rel = table_open(AuthIdRelationId, RowExclusiveLock); + + tup = SearchSysCache1(AUTHNAME, CStringGetDatum(name)); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_SCHEMA), + errmsg("role \"%s\" does not exist", name))); + + authform = (Form_pg_authid) GETSTRUCT(tup); + roleid = authform->oid; + + elog(WARNING, "AlterRoleOwner_internal not yet implemented"); + // AlterRoleOwner_internal(tup, rel, newOwnerId); + + ObjectAddressSet(address, AuthIdRelationId, roleid); + + ReleaseSysCache(tup); + + table_close(rel, RowExclusiveLock); + + return address; +} diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 08f1bf1031..965c903c71 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -1194,6 +1194,10 @@ CreateOptRoleElem: { $$ = makeDefElem("addroleto", (Node *)$3, @1); } + | OWNER RoleSpec + { + $$ = makeDefElem("owner", (Node *)$2, @1); + } ; @@ -9490,6 +9494,14 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec n->newowner = $6; $$ = (Node *)n; } + | ALTER ROLE RoleSpec OWNER TO RoleSpec + { + AlterOwnerStmt *n = makeNode(AlterOwnerStmt); + n->objectType = OBJECT_ROLE; + n->object = (Node *) $3; + n->newowner = $6; + $$ = (Node *)n; + } | ALTER ROUTINE function_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index 2d7115e31d..cce43388d8 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -32,6 +32,7 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284 { Oid oid; /* oid */ NameData rolname; /* name of role */ + Oid rolowner BKI_DEFAULT(POSTGRES) BKI_LOOKUP(pg_authid); /* owner of this role */ bool rolsuper; /* read this field via superuser() only! */ bool rolinherit; /* inherit privileges from other roles? */ bool rolcreaterole; /* allowed to create more roles? */ diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 0b7a3cd65f..b49fd2b2c5 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -33,5 +33,6 @@ extern ObjectAddress RenameRole(const char *oldname, const char *newname); extern void DropOwnedObjects(DropOwnedStmt *stmt); extern void ReassignOwnedObjects(ReassignOwnedStmt *stmt); extern List *roleSpecsToIds(List *memberNames); +extern ObjectAddress AlterRoleOwner(const char *name, Oid newOwnerId); #endif /* USER_H */ diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out index eb608fdc2e..8b79a63b80 100644 --- a/src/test/modules/unsafe_tests/expected/rolenames.out +++ b/src/test/modules/unsafe_tests/expected/rolenames.out @@ -1086,6 +1086,10 @@ REVOKE pg_read_all_settings FROM regress_role_haspriv; \c DROP SCHEMA test_roles_schema; DROP OWNED BY regress_testrol0, "Public", "current_role", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE; -DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- fails with owner of role regress_role_haspriv +ERROR: role "regress_testrol2" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_role_haspriv +owner of role regress_role_nopriv DROP ROLE "Public", "None", "current_role", "current_user", "session_user", "user"; DROP ROLE regress_role_haspriv, regress_role_nopriv; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- ok now diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql index adac36536d..95a54ce70d 100644 --- a/src/test/modules/unsafe_tests/sql/rolenames.sql +++ b/src/test/modules/unsafe_tests/sql/rolenames.sql @@ -499,6 +499,7 @@ REVOKE pg_read_all_settings FROM regress_role_haspriv; DROP SCHEMA test_roles_schema; DROP OWNED BY regress_testrol0, "Public", "current_role", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE; -DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- fails with owner of role regress_role_haspriv DROP ROLE "Public", "None", "current_role", "current_user", "session_user", "user"; DROP ROLE regress_role_haspriv, regress_role_nopriv; +DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; -- ok now diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out index e6afb72bf7..385ee74342 100644 --- a/src/test/regress/expected/create_role.out +++ b/src/test/regress/expected/create_role.out @@ -103,9 +103,34 @@ ERROR: role "regress_role_17" does not exist DROP ROLE regress_role_19; ERROR: role "regress_role_19" does not exist DROP ROLE regress_role_20; +-- fail, cannot drop roles that own other roles +DROP ROLE regress_role_7; +ERROR: role "regress_role_7" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_role_21 +owner of role regress_role_22 +owner of role regress_role_23 +owner of role regress_role_24 +owner of role regress_role_25 +owner of role regress_role_26 +owner of role regress_role_27 +owner of role regress_role_28 +owner of role regress_role_29 +owner of role regress_role_30 +owner of role regress_role_31 +owner of role regress_role_32 +owner of role regress_role_33 +owner of role regress_role_34 +owner of role regress_role_35 +owner of role regress_role_36 +owner of role regress_role_37 +owner of role regress_role_38 +owner of role regress_role_39 +owner of role regress_role_40 +owner of role regress_role_41 +owner of role regress_role_42 +owner of role regress_role_43 -- ok, should be able to drop non-superuser roles we created DROP ROLE regress_role_6; -DROP ROLE regress_role_7; DROP ROLE regress_role_8; DROP ROLE regress_role_9; DROP ROLE regress_role_10; @@ -130,6 +155,10 @@ DROP ROLE regress_role_22; ERROR: role "regress_role_22" cannot be dropped because some objects depend on it DETAIL: owner of table regress_tbl_22 owner of view regress_view_22 +-- fail, role still owns other roles +DROP ROLE regress_role_7; +ERROR: role "regress_role_7" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_role_22 -- fail, cannot drop ourself nor superusers DROP ROLE regress_role_super; ERROR: must be superuser to drop superusers @@ -141,5 +170,6 @@ DROP INDEX regress_idx_22; DROP TABLE regress_tbl_22; DROP VIEW regress_view_22; DROP ROLE regress_role_22; +DROP ROLE regress_role_7; DROP ROLE regress_role_1; DROP ROLE regress_role_super; diff --git a/src/test/regress/expected/oidjoins.out b/src/test/regress/expected/oidjoins.out index 1461e947cd..beaf942f59 100644 --- a/src/test/regress/expected/oidjoins.out +++ b/src/test/regress/expected/oidjoins.out @@ -194,6 +194,7 @@ NOTICE: checking pg_database {dattablespace} => pg_tablespace {oid} NOTICE: checking pg_db_role_setting {setdatabase} => pg_database {oid} NOTICE: checking pg_db_role_setting {setrole} => pg_authid {oid} NOTICE: checking pg_tablespace {spcowner} => pg_authid {oid} +NOTICE: checking pg_authid {rolowner} => pg_authid {oid} NOTICE: checking pg_auth_members {roleid} => pg_authid {oid} NOTICE: checking pg_auth_members {member} => pg_authid {oid} NOTICE: checking pg_auth_members {grantor} => pg_authid {oid} diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 83cff902f3..c4456cadce 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -27,8 +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_user6 CREATEROLE; +SET SESSION AUTHORIZATION regress_priv_user6; CREATE USER regress_priv_user7; +RESET SESSION AUTHORIZATION; GRANT pg_read_all_data TO regress_priv_user6; GRANT pg_write_all_data TO regress_priv_user7; CREATE GROUP regress_priv_group1; @@ -2327,7 +2329,12 @@ 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" cannot be dropped because some objects depend on it +DETAIL: owner of role regress_priv_user7 +SET SESSION AUTHORIZATION regress_priv_user6; DROP USER regress_priv_user7; +RESET SESSION AUTHORIZATION; +DROP USER regress_priv_user6; DROP USER regress_priv_user8; -- does not exist ERROR: role "regress_priv_user8" does not exist -- permissions with LOCK TABLE diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql index 8429b54595..728208f1e1 100644 --- a/src/test/regress/sql/create_role.sql +++ b/src/test/regress/sql/create_role.sql @@ -98,9 +98,11 @@ DROP ROLE regress_role_17; DROP ROLE regress_role_19; DROP ROLE regress_role_20; +-- fail, cannot drop roles that own other roles +DROP ROLE regress_role_7; + -- ok, should be able to drop non-superuser roles we created DROP ROLE regress_role_6; -DROP ROLE regress_role_7; DROP ROLE regress_role_8; DROP ROLE regress_role_9; DROP ROLE regress_role_10; @@ -124,6 +126,9 @@ DROP ROLE regress_role_32; -- fail, role still owns database objects DROP ROLE regress_role_22; +-- fail, role still owns other roles +DROP ROLE regress_role_7; + -- fail, cannot drop ourself nor superusers DROP ROLE regress_role_super; DROP ROLE regress_role_1; @@ -134,5 +139,6 @@ DROP INDEX regress_idx_22; DROP TABLE regress_tbl_22; DROP VIEW regress_view_22; DROP ROLE regress_role_22; +DROP ROLE regress_role_7; DROP ROLE regress_role_1; DROP ROLE regress_role_super; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 3d1a1db987..bd2d67691c 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -29,9 +29,14 @@ CREATE USER regress_priv_user2; 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_user6 CREATEROLE; + +SET SESSION AUTHORIZATION regress_priv_user6; CREATE USER regress_priv_user7; +RESET SESSION AUTHORIZATION; GRANT pg_read_all_data TO regress_priv_user6; GRANT pg_write_all_data TO regress_priv_user7; @@ -1389,8 +1394,14 @@ DROP USER regress_priv_user2; DROP USER regress_priv_user3; DROP USER regress_priv_user4; DROP USER regress_priv_user5; + DROP USER regress_priv_user6; + +SET SESSION AUTHORIZATION regress_priv_user6; DROP USER regress_priv_user7; +RESET SESSION AUTHORIZATION; + +DROP USER regress_priv_user6; DROP USER regress_priv_user8; -- does not exist -- 2.21.1 (Apple Git-122.3)