From 0777f60f13d8f43f0d62ce47407a53232cf8949c Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Thu, 26 Aug 2021 15:42:27 -0700 Subject: [PATCH v6 18/19] Adding 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. --- 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/oidjoins.out | 1 + src/test/regress/expected/privileges.out | 9 +++- src/test/regress/sql/privileges.sql | 13 +++++- 10 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 29249498a9..6cf1a9175a 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((Value *) stmt->object), newowner); + case OBJECT_ROLE: + return AlterRoleOwner(strVal((Value *) stmt->object), newowner); + case OBJECT_SCHEMA: return AlterSchemaOwner(strVal((Value *) 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 39a2849eba..aca6856f3d 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -1195,6 +1195,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/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/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)