v5-0002-Add-owners-to-roles.patch

From 83dea225a5baac7efd05e9b835055c031a290c2d Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 11 Jan 2022 11:15:02 -0800
Subject: [PATCH v5 2/5] 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/catalog/aclchk.c                  |  59 ++++++-
 src/backend/catalog/pg_shdepend.c             |   5 +
 src/backend/catalog/system_views.sql          |   1 +
 src/backend/commands/alter.c                  |   3 +
 src/backend/commands/user.c                   | 148 +++++++++++++++++-
 src/backend/nodes/copyfuncs.c                 |   1 +
 src/backend/nodes/equalfuncs.c                |   1 +
 src/backend/parser/gram.y                     |  23 +++
 src/bin/psql/describe.c                       |  12 ++
 src/include/catalog/pg_authid.h               |   1 +
 src/include/commands/user.h                   |   2 +
 src/include/nodes/parsenodes.h                |   1 +
 src/include/utils/acl.h                       |   1 +
 .../unsafe_tests/expected/rolenames.out       |   6 +-
 .../modules/unsafe_tests/sql/rolenames.sql    |   3 +-
 src/test/regress/expected/create_role.out     | 136 ++++++++++++++--
 src/test/regress/expected/oidjoins.out        |   1 +
 src/test/regress/expected/privileges.out      |   9 +-
 src/test/regress/expected/rules.out           |   1 +
 src/test/regress/sql/create_role.sql          |  67 +++++++-
 src/test/regress/sql/privileges.sql           |  10 +-
 21 files changed, 470 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 1dd03a8e51..e5c7324340 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3385,6 +3385,9 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_PUBLICATION:
 						msg = gettext_noop("permission denied for publication %s");
 						break;
+					case OBJECT_ROLE:
+						msg = gettext_noop("permission denied for role %s");
+						break;
 					case OBJECT_ROUTINE:
 						msg = gettext_noop("permission denied for routine %s");
 						break;
@@ -3429,7 +3432,6 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_DOMCONSTRAINT:
 					case OBJECT_PUBLICATION_NAMESPACE:
 					case OBJECT_PUBLICATION_REL:
-					case OBJECT_ROLE:
 					case OBJECT_RULE:
 					case OBJECT_TABCONSTRAINT:
 					case OBJECT_TRANSFORM:
@@ -3511,6 +3513,9 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_PUBLICATION:
 						msg = gettext_noop("must be owner of publication %s");
 						break;
+					case OBJECT_ROLE:
+						msg = gettext_noop("must be owner of role %s");
+						break;
 					case OBJECT_ROUTINE:
 						msg = gettext_noop("must be owner of routine %s");
 						break;
@@ -3569,7 +3574,6 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_DOMCONSTRAINT:
 					case OBJECT_PUBLICATION_NAMESPACE:
 					case OBJECT_PUBLICATION_REL:
-					case OBJECT_ROLE:
 					case OBJECT_TRANSFORM:
 					case OBJECT_TSPARSER:
 					case OBJECT_TSTEMPLATE:
@@ -5430,6 +5434,57 @@ pg_statistics_object_ownercheck(Oid stat_oid, Oid roleid)
 	return has_privs_of_role(roleid, ownerId);
 }
 
+/*
+ * Ownership check for a role (specified by OID)
+ */
+bool
+pg_role_ownercheck(Oid owned_role_oid, Oid owner_roleid)
+{
+	HeapTuple		tuple;
+	Form_pg_authid	authform;
+	Oid				owner_oid;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(owner_roleid))
+		return true;
+
+	/* Otherwise, look up the owner of the role */
+	tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(owned_role_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("role with OID %u does not exist",
+						owned_role_oid)));
+	authform = (Form_pg_authid) GETSTRUCT(tuple);
+	owner_oid = authform->rolowner;
+
+	/*
+	 * Roles must necessarily have owners.  Even the bootstrap user has an
+	 * owner.  (It owns itself).  Other roles must form a proper tree.
+	 */
+	if (!OidIsValid(owner_oid))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("role \"%s\" with OID %u has invalid owner",
+						authform->rolname.data, authform->oid)));
+	if (authform->oid != BOOTSTRAP_SUPERUSERID &&
+		authform->rolowner == authform->oid)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("role \"%s\" with OID %u owns itself",
+						authform->rolname.data, authform->oid)));
+	if (authform->oid == BOOTSTRAP_SUPERUSERID &&
+		authform->rolowner != BOOTSTRAP_SUPERUSERID)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("role \"%s\" with OID %u owned by role with OID %u",
+						authform->rolname.data, authform->oid,
+						authform->rolowner)));
+	ReleaseSysCache(tuple);
+
+	return (owner_oid == owner_roleid);
+}
+
 /*
  * Check whether specified role has CREATEROLE privilege (or is a superuser)
  *
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 3e8fa008b9..52f69ad76e 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -61,6 +61,7 @@
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
 #include "commands/typecmds.h"
+#include "commands/user.h"
 #include "miscadmin.h"
 #include "storage/lmgr.h"
 #include "utils/acl.h"
@@ -1578,6 +1579,10 @@ shdepReassignOwned(List *roleids, Oid newrole)
 					AlterSubscriptionOwner_oid(sdepForm->objid, newrole);
 					break;
 
+				case AuthIdRelationId:
+					AlterRoleOwner_oid(sdepForm->objid, newrole);
+					break;
+
 					/* Generic alter owner cases */
 				case CollationRelationId:
 				case ConversionRelationId:
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 701ff38f76..af08b69864 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -17,6 +17,7 @@
 CREATE VIEW pg_roles AS
     SELECT
         rolname,
+        pg_get_userbyid(rolowner) AS rolowner,
         rolsuper,
         rolinherit,
         rolcreaterole,
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 1f64c8aa51..e6c7c84c87 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -840,6 +840,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 3b512a84b3..a4e2223a2d 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -55,6 +55,8 @@ static void AddRoleMems(const char *rolename, Oid roleid,
 static void DelRoleMems(const char *rolename, Oid roleid,
 						List *memberSpecs, List *memberIds,
 						bool admin_opt);
+static void AlterRoleOwner_internal(HeapTuple tup, Relation rel,
+									Oid newOwnerId);
 
 
 /* Check if current user has createrole privileges */
@@ -77,6 +79,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	Datum		new_record[Natts_pg_authid];
 	bool		new_record_nulls[Natts_pg_authid];
 	Oid			roleid;
+	Oid			owner_uid;
+	Oid			saved_uid;
+	int			save_sec_context;
 	ListCell   *item;
 	ListCell   *option;
 	char	   *password = NULL;	/* user password */
@@ -108,6 +113,16 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dvalidUntil = NULL;
 	DefElem    *dbypassRLS = NULL;
 
+	GetUserIdAndSecContext(&saved_uid, &save_sec_context);
+
+	/*
+	 * Who is supposed to own the new role?
+	 */
+	if (stmt->authrole)
+		owner_uid = get_rolespec_oid(stmt->authrole, false);
+	else
+		owner_uid = saved_uid;
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -254,6 +269,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 					 errmsg("must be superuser to create superusers")));
+		if (!superuser_arg(owner_uid))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("must be superuser to own superusers")));
 	}
 	else if (isreplication)
 	{
@@ -310,6 +329,19 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 				 errmsg("role \"%s\" already exists",
 						stmt->role)));
 
+	/*
+	 * If the requested authorization is different from the current user,
+	 * temporarily set the current user so that the object(s) will be created
+	 * with the correct ownership.
+	 *
+	 * (The setting will be restored at the end of this routine, or in case of
+	 * error, transaction abort will clean things up.)
+	 */
+	if (saved_uid != owner_uid)
+		SetUserIdAndSecContext(owner_uid,
+							   save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
+
+
 	/* Convert validuntil to internal form */
 	if (validUntil)
 	{
@@ -345,6 +377,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_uid);
 	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 +455,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	 */
 	CatalogTupleInsert(pg_authid_rel, tuple);
 
+	recordDependencyOnOwner(AuthIdRelationId, roleid, owner_uid);
+
 	/*
 	 * Advance command counter so we can see new record; else tests in
 	 * AddRoleMems may fail.
@@ -478,6 +513,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	 */
 	table_close(pg_authid_rel, NoLock);
 
+	/* Reset current user and security context */
+	SetUserIdAndSecContext(saved_uid, save_sec_context);
+
 	return roleid;
 }
 
@@ -1078,8 +1116,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 +1714,110 @@ 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_OBJECT),
+				 errmsg("role \"%s\" does not exist", name)));
+
+	authform = (Form_pg_authid) GETSTRUCT(tup);
+	roleid = authform->oid;
+
+	AlterRoleOwner_internal(tup, rel, newOwnerId);
+
+	ObjectAddressSet(address, AuthIdRelationId, roleid);
+
+	ReleaseSysCache(tup);
+
+	table_close(rel, RowExclusiveLock);
+
+	return address;
+}
+
+void
+AlterRoleOwner_oid(Oid roleOid, Oid newOwnerId)
+{
+	HeapTuple	tup;
+	Relation	rel;
+
+	rel = table_open(AuthIdRelationId, RowExclusiveLock);
+
+	tup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleOid));
+	if (!HeapTupleIsValid(tup))
+		elog(ERROR, "cache lookup failed for role %u", roleOid);
+
+	AlterRoleOwner_internal(tup, rel, newOwnerId);
+
+	ReleaseSysCache(tup);
+
+	table_close(rel, RowExclusiveLock);
+}
+
+static void
+AlterRoleOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId)
+{
+	Form_pg_authid authForm;
+
+	Assert(tup->t_tableOid == AuthIdRelationId);
+	Assert(RelationGetRelid(rel) == AuthIdRelationId);
+
+	authForm = (Form_pg_authid) GETSTRUCT(tup);
+
+	/*
+	 * If the new owner is the same as the existing owner, consider the
+	 * command to have succeeded.  This is for dump restoration purposes.
+	 */
+	if (authForm->rolowner != newOwnerId)
+	{
+		/* Otherwise, must be owner of the existing object */
+		if (!pg_role_ownercheck(authForm->oid, GetUserId()))
+			aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_ROLE,
+						   NameStr(authForm->rolname));
+
+		/* Must be able to become new owner */
+		check_is_member_of_role(GetUserId(), newOwnerId);
+
+		/*
+		 * must have CREATEROLE rights
+		 *
+		 * NOTE: This is different from most other alter-owner checks in that
+		 * the current user is checked for create privileges instead of the
+		 * destination owner.  This is consistent with the CREATE case for
+		 * roles.  Because superusers will always have this right, we need no
+		 * special case for them.
+		 */
+		if (!have_createrole_privilege())
+			aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_ROLE,
+						   NameStr(authForm->rolname));
+
+		/* Only the bootstrap superuser is allowed to own itself. */
+		if (newOwnerId != BOOTSTRAP_SUPERUSERID && authForm->oid == newOwnerId)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("role may not own itself")));
+
+		authForm->rolowner = newOwnerId;
+		CatalogTupleUpdate(rel, &tup->t_self, tup);
+
+		/* Update owner dependency reference */
+		changeDependencyOnOwner(AuthIdRelationId, authForm->oid, newOwnerId);
+	}
+
+	InvokeObjectPostAlterHook(AuthIdRelationId,
+							  authForm->oid, 0);
+}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 456d563f34..cb0794e873 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4519,6 +4519,7 @@ _copyCreateRoleStmt(const CreateRoleStmt *from)
 
 	COPY_SCALAR_FIELD(stmt_type);
 	COPY_STRING_FIELD(role);
+	COPY_NODE_FIELD(authrole);
 	COPY_NODE_FIELD(options);
 
 	return newnode;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 53beef1488..05dd05b170 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2128,6 +2128,7 @@ _equalCreateRoleStmt(const CreateRoleStmt *a, const CreateRoleStmt *b)
 {
 	COMPARE_SCALAR_FIELD(stmt_type);
 	COMPARE_STRING_FIELD(role);
+	COMPARE_NODE_FIELD(authrole);
 	COMPARE_NODE_FIELD(options);
 
 	return true;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 879018377b..ef56f87d66 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1077,9 +1077,20 @@ CreateRoleStmt:
 					CreateRoleStmt *n = makeNode(CreateRoleStmt);
 					n->stmt_type = ROLESTMT_ROLE;
 					n->role = $3;
+					n->authrole = NULL;
 					n->options = $5;
 					$$ = (Node *)n;
 				}
+			|
+			CREATE ROLE RoleId AUTHORIZATION RoleSpec opt_with OptRoleList
+				{
+					CreateRoleStmt *n = makeNode(CreateRoleStmt);
+					n->stmt_type = ROLESTMT_ROLE;
+					n->role = $3;
+					n->authrole = $5;
+					n->options = $7;
+					$$ = (Node *)n;
+				}
 		;
 
 
@@ -1218,6 +1229,10 @@ CreateOptRoleElem:
 				{
 					$$ = makeDefElem("addroleto", (Node *)$3, @1);
 				}
+			| OWNER RoleSpec
+				{
+					$$ = makeDefElem("owner", (Node *)$2, @1);
+				}
 		;
 
 
@@ -9585,6 +9600,14 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec
 					n->newowner = $6;
 					$$ = (Node *)n;
 				}
+			| ALTER ROLE name OWNER TO RoleSpec
+				{
+					AlterOwnerStmt *n = makeNode(AlterOwnerStmt);
+					n->objectType = OBJECT_ROLE;
+					n->object = (Node *) makeString($3);
+					n->newowner = $6;
+					$$ = (Node *)n;
+				}
 			| ALTER ROUTINE function_with_argtypes OWNER TO RoleSpec
 				{
 					AlterOwnerStmt *n = makeNode(AlterOwnerStmt);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 8587b19160..fcf641ec41 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3496,6 +3496,12 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 		appendPQExpBufferStr(&buf, "\n, r.rolbypassrls");
 	}
 
+	if (pset.sversion >= 150000)
+	{
+		appendPQExpBufferStr(&buf, "\n, r.rolowner");
+		ncols++;
+	}
+
 	appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n");
 
 	if (!showSystem && !pattern)
@@ -3516,6 +3522,8 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows);
 
 	printTableAddHeader(&cont, gettext_noop("Role name"), true, align);
+	if (pset.sversion >= 150000)
+		printTableAddHeader(&cont, gettext_noop("Owner"), true, align);
 	printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
 	/* ignores implicit memberships from superuser & pg_database_owner */
 	printTableAddHeader(&cont, gettext_noop("Member of"), true, align);
@@ -3527,6 +3535,10 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
 	{
 		printTableAddCell(&cont, PQgetvalue(res, i, 0), false, false);
 
+		if (pset.sversion >= 150000)
+			printTableAddCell(&cont, PQgetvalue(res, i, (verbose ? 12 : 11)),
+							  false, false);
+
 		resetPQExpBuffer(&buf);
 		if (strcmp(PQgetvalue(res, i, 1), "t") == 0)
 			add_role_attribute(&buf, _("Superuser"));
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index 4b65e39a1f..3af0f3908c 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..c32127e41e 100644
--- a/src/include/commands/user.h
+++ b/src/include/commands/user.h
@@ -33,5 +33,7 @@ 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);
+extern void AlterRoleOwner_oid(Oid roleOid, Oid newOwnerId);
 
 #endif							/* USER_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 413e7c85a1..06ad44569b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2622,6 +2622,7 @@ typedef struct CreateRoleStmt
 	NodeTag		type;
 	RoleStmtType stmt_type;		/* ROLE/USER/GROUP */
 	char	   *role;			/* role name */
+	RoleSpec   *authrole;		/* the owner of the created role */
 	List	   *options;		/* List of DefElem nodes */
 } CreateRoleStmt;
 
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 1ce4c5556e..42f85d0e84 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -316,6 +316,7 @@ extern bool pg_extension_ownercheck(Oid ext_oid, Oid roleid);
 extern bool pg_publication_ownercheck(Oid pub_oid, Oid roleid);
 extern bool pg_subscription_ownercheck(Oid sub_oid, Oid roleid);
 extern bool pg_statistics_object_ownercheck(Oid stat_oid, Oid roleid);
+extern bool pg_role_ownercheck(Oid owned_role_oid, Oid owner_roleid);
 extern bool has_createrole_privilege(Oid roleid);
 extern bool has_bypassrls_privilege(Oid roleid);
 
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 4e67d72760..66d4c29cf7 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_admin;
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_nosuch_superuser SUPERUSER;
@@ -11,14 +12,98 @@ CREATE ROLE regress_nosuch_replication REPLICATION;
 ERROR:  must be superuser to create replication users
 CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
 ERROR:  must be superuser to create bypassrls users
+-- fail, only superusers can own superusers
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_admin SUPERUSER;
+ERROR:  must be superuser to own superusers
+-- ok, superuser can create superusers belonging to other superusers
+CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_super SUPERUSER;
+-- ok, superuser can create users with these privileges for normal role
+CREATE ROLE regress_nosuch_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS;
+CREATE ROLE regress_nosuch_replication AUTHORIZATION regress_role_admin REPLICATION;
+CREATE ROLE regress_nosuch_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS;
+\du+ regress_nosuch_superuser
+                                           List of roles
+        Role name         |       Owner        |       Attributes        | Member of | Description 
+--------------------------+--------------------+-------------------------+-----------+-------------
+ regress_nosuch_superuser | regress_role_super | Superuser, Cannot login | {}        | 
+
+\du+ regress_nosuch_replication_bypassrls
+                                                        List of roles
+              Role name               |       Owner        |              Attributes               | Member of | Description 
+--------------------------------------+--------------------+---------------------------------------+-----------+-------------
+ regress_nosuch_replication_bypassrls | regress_role_admin | Cannot login, Replication, Bypass RLS | {}        | 
+
+\du+ regress_nosuch_replication
+                                             List of roles
+         Role name          |       Owner        |        Attributes         | Member of | Description 
+----------------------------+--------------------+---------------------------+-----------+-------------
+ regress_nosuch_replication | regress_role_admin | Cannot login, Replication | {}        | 
+
+\du+ regress_nosuch_bypassrls
+                                           List of roles
+        Role name         |       Owner        |        Attributes        | Member of | Description 
+--------------------------+--------------------+--------------------------+-----------+-------------
+ regress_nosuch_bypassrls | regress_role_admin | Cannot login, Bypass RLS | {}        | 
+
+-- fail, roles are not allowed to own themselves
+ALTER ROLE regress_nosuch_bypassrls OWNER TO regress_nosuch_bypassrls;
+ERROR:  role may not own itself
 -- ok, having CREATEROLE is enough to create users with these privileges
+SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
-CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo';
-CREATE ROLE regress_password_null PASSWORD NULL;
+CREATE ROLE regress_encrypted_password PASSWORD NULL;
+CREATE ROLE regress_password_null
+  CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo'
+  IN ROLE regress_createdb, regress_createrole, regress_login;
+\du+ regress_createdb
+                                       List of roles
+    Role name     |       Owner        |       Attributes        | Member of | Description 
+------------------+--------------------+-------------------------+-----------+-------------
+ regress_createdb | regress_role_admin | Create DB, Cannot login | {}        | 
+
+\du+ regress_createrole
+                                         List of roles
+     Role name      |       Owner        |        Attributes         | Member of | Description 
+--------------------+--------------------+---------------------------+-----------+-------------
+ regress_createrole | regress_role_admin | Create role, Cannot login | {}        | 
+
+\du+ regress_login
+                               List of roles
+   Role name   |       Owner        | Attributes | Member of | Description 
+---------------+--------------------+------------+-----------+-------------
+ regress_login | regress_role_admin |            | {}        | 
+
+\du+ regress_inherit
+                                 List of roles
+    Role name    |       Owner        |  Attributes  | Member of | Description 
+-----------------+--------------------+--------------+-----------+-------------
+ regress_inherit | regress_role_admin | Cannot login | {}        | 
+
+\du+ regress_connection_limit
+                                      List of roles
+        Role name         |       Owner        |  Attributes   | Member of | Description 
+--------------------------+--------------------+---------------+-----------+-------------
+ regress_connection_limit | regress_role_admin | Cannot login +| {}        | 
+                          |                    | 5 connections |           | 
+
+\du+ regress_encrypted_password
+                                      List of roles
+         Role name          |       Owner        |  Attributes  | Member of | Description 
+----------------------------+--------------------+--------------+-----------+-------------
+ regress_encrypted_password | regress_role_admin | Cannot login | {}        | 
+
+\du+ regress_password_null
+                                                              List of roles
+       Role name       |       Owner        |       Attributes       |                      Member of                      | Description 
+-----------------------+--------------------+------------------------+-----------------------------------------------------+-------------
+ regress_password_null | regress_role_admin | Create role, Create DB+| {regress_createdb,regress_createrole,regress_login} | 
+                       |                    | 2 connections          |                                                     | 
+
 -- ok, backwards compatible noise words should be ignored
 CREATE ROLE regress_noiseword SYSID 12345;
 NOTICE:  SYSID can no longer be specified
@@ -84,16 +169,24 @@ CREATE ROLE regress_read_server_files IN ROLE pg_read_server_files;
 CREATE ROLE regress_write_server_files IN ROLE pg_write_server_files;
 CREATE ROLE regress_execute_server_program IN ROLE pg_execute_server_program;
 CREATE ROLE regress_signal_backend IN ROLE pg_signal_backend;
--- fail, creation of these roles failed above so they do not now exist
+-- fail, cannot take ownership of these objects from regress_createrole
 SET SESSION AUTHORIZATION regress_role_admin;
+ALTER ROLE regress_plainrole OWNER TO regress_role_admin;
+ERROR:  must be owner of role regress_plainrole
+REASSIGN OWNED BY regress_plainrole TO regress_role_admin;
+ERROR:  permission denied to reassign objects
+-- superuser can do it, though
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_plainrole OWNER TO regress_role_admin;
+REASSIGN OWNED BY regress_plainrole TO regress_role_admin;
+-- ok, superuser roles can drop superuser roles they own
+SET SESSION AUTHORIZATION regress_role_super;
 DROP ROLE regress_nosuch_superuser;
-ERROR:  role "regress_nosuch_superuser" does not exist
+-- ok, non-superuser roles can drop non-superuser roles they own
+SET SESSION AUTHORIZATION regress_role_admin;
 DROP ROLE regress_nosuch_replication_bypassrls;
-ERROR:  role "regress_nosuch_replication_bypassrls" does not exist
 DROP ROLE regress_nosuch_replication;
-ERROR:  role "regress_nosuch_replication" does not exist
 DROP ROLE regress_nosuch_bypassrls;
-ERROR:  role "regress_nosuch_bypassrls" does not exist
 DROP ROLE regress_nosuch_super;
 ERROR:  role "regress_nosuch_super" does not exist
 DROP ROLE regress_nosuch_dbowner;
@@ -103,9 +196,23 @@ ERROR:  role "regress_nosuch_recursive" does not exist
 DROP ROLE regress_nosuch_admin_recursive;
 ERROR:  role "regress_nosuch_admin_recursive" does not exist
 DROP ROLE regress_plainrole;
--- ok, should be able to drop non-superuser roles we created
-DROP ROLE regress_createdb;
+-- fail, cannot drop roles that own other roles
 DROP ROLE regress_createrole;
+ERROR:  role "regress_createrole" cannot be dropped because some objects depend on it
+DETAIL:  owner of role regress_rolecreator
+owner of role regress_tenant
+owner of role regress_read_all_data
+owner of role regress_write_all_data
+owner of role regress_monitor
+owner of role regress_read_all_settings
+owner of role regress_read_all_stats
+owner of role regress_stat_scan_tables
+owner of role regress_read_server_files
+owner of role regress_write_server_files
+owner of role regress_execute_server_program
+owner of role regress_signal_backend
+-- ok, should be able to drop these non-superuser roles
+DROP ROLE regress_createdb;
 DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
@@ -130,6 +237,10 @@ DROP ROLE regress_tenant;
 ERROR:  role "regress_tenant" cannot be dropped because some objects depend on it
 DETAIL:  owner of table tenant_table
 owner of view tenant_view
+-- fail, role still owns other roles
+DROP ROLE regress_createrole;
+ERROR:  role "regress_createrole" cannot be dropped because some objects depend on it
+DETAIL:  owner of role regress_tenant
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
 ERROR:  must be superuser to drop superusers
@@ -141,5 +252,12 @@ DROP INDEX tenant_idx;
 DROP TABLE tenant_table;
 DROP VIEW tenant_view;
 DROP ROLE regress_tenant;
+DROP ROLE regress_createrole;
+-- fail, cannot drop role with remaining privileges
+DROP ROLE regress_role_admin;
+ERROR:  role "regress_role_admin" cannot be dropped because some objects depend on it
+DETAIL:  privileges for database regression
+-- ok, can drop role if we revoke privileges first
+REVOKE CREATE ON DATABASE regression FROM regress_role_admin;
 DROP ROLE regress_role_admin;
 DROP ROLE regress_role_super;
diff --git a/src/test/regress/expected/oidjoins.out b/src/test/regress/expected/oidjoins.out
index 215eb899be..266a30a85b 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 291e21d7a6..9ce619fd5f 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;
 CREATE USER regress_priv_user8;
 CREATE USER regress_priv_user9;
 CREATE USER regress_priv_user10;
@@ -2356,7 +2358,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/expected/rules.out b/src/test/regress/expected/rules.out
index b58b062b10..6bce5a005d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1482,6 +1482,7 @@ pg_replication_slots| SELECT l.slot_name,
    FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, temporary, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn, wal_status, safe_wal_size, two_phase)
      LEFT JOIN pg_database d ON ((l.datoid = d.oid)));
 pg_roles| SELECT pg_authid.rolname,
+    pg_get_userbyid(pg_authid.rolowner) AS rolowner,
     pg_authid.rolsuper,
     pg_authid.rolinherit,
     pg_authid.rolcreaterole,
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 292dc08797..091b116dd6 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_admin;
 
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
@@ -9,14 +10,45 @@ CREATE ROLE regress_nosuch_replication_bypassrls REPLICATION BYPASSRLS;
 CREATE ROLE regress_nosuch_replication REPLICATION;
 CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
 
+-- fail, only superusers can own superusers
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_admin SUPERUSER;
+
+-- ok, superuser can create superusers belonging to other superusers
+CREATE ROLE regress_nosuch_superuser AUTHORIZATION regress_role_super SUPERUSER;
+
+-- ok, superuser can create users with these privileges for normal role
+CREATE ROLE regress_nosuch_replication_bypassrls AUTHORIZATION regress_role_admin REPLICATION BYPASSRLS;
+CREATE ROLE regress_nosuch_replication AUTHORIZATION regress_role_admin REPLICATION;
+CREATE ROLE regress_nosuch_bypassrls AUTHORIZATION regress_role_admin BYPASSRLS;
+
+\du+ regress_nosuch_superuser
+\du+ regress_nosuch_replication_bypassrls
+\du+ regress_nosuch_replication
+\du+ regress_nosuch_bypassrls
+
+-- fail, roles are not allowed to own themselves
+ALTER ROLE regress_nosuch_bypassrls OWNER TO regress_nosuch_bypassrls;
+
 -- ok, having CREATEROLE is enough to create users with these privileges
+SET SESSION AUTHORIZATION regress_role_admin;
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
-CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo';
-CREATE ROLE regress_password_null PASSWORD NULL;
+CREATE ROLE regress_encrypted_password PASSWORD NULL;
+CREATE ROLE regress_password_null
+  CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo'
+  IN ROLE regress_createdb, regress_createrole, regress_login;
+
+\du+ regress_createdb
+\du+ regress_createrole
+\du+ regress_login
+\du+ regress_inherit
+\du+ regress_connection_limit
+\du+ regress_encrypted_password
+\du+ regress_password_null
 
 -- ok, backwards compatible noise words should be ignored
 CREATE ROLE regress_noiseword SYSID 12345;
@@ -86,9 +118,22 @@ CREATE ROLE regress_write_server_files IN ROLE pg_write_server_files;
 CREATE ROLE regress_execute_server_program IN ROLE pg_execute_server_program;
 CREATE ROLE regress_signal_backend IN ROLE pg_signal_backend;
 
--- fail, creation of these roles failed above so they do not now exist
+-- fail, cannot take ownership of these objects from regress_createrole
 SET SESSION AUTHORIZATION regress_role_admin;
+ALTER ROLE regress_plainrole OWNER TO regress_role_admin;
+REASSIGN OWNED BY regress_plainrole TO regress_role_admin;
+
+-- superuser can do it, though
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_plainrole OWNER TO regress_role_admin;
+REASSIGN OWNED BY regress_plainrole TO regress_role_admin;
+
+-- ok, superuser roles can drop superuser roles they own
+SET SESSION AUTHORIZATION regress_role_super;
 DROP ROLE regress_nosuch_superuser;
+
+-- ok, non-superuser roles can drop non-superuser roles they own
+SET SESSION AUTHORIZATION regress_role_admin;
 DROP ROLE regress_nosuch_replication_bypassrls;
 DROP ROLE regress_nosuch_replication;
 DROP ROLE regress_nosuch_bypassrls;
@@ -98,9 +143,11 @@ DROP ROLE regress_nosuch_recursive;
 DROP ROLE regress_nosuch_admin_recursive;
 DROP ROLE regress_plainrole;
 
--- ok, should be able to drop non-superuser roles we created
-DROP ROLE regress_createdb;
+-- fail, cannot drop roles that own other roles
 DROP ROLE regress_createrole;
+
+-- ok, should be able to drop these non-superuser roles
+DROP ROLE regress_createdb;
 DROP ROLE regress_login;
 DROP ROLE regress_inherit;
 DROP ROLE regress_connection_limit;
@@ -124,6 +171,9 @@ DROP ROLE regress_signal_backend;
 -- fail, role still owns database objects
 DROP ROLE regress_tenant;
 
+-- fail, role still owns other roles
+DROP ROLE regress_createrole;
+
 -- fail, cannot drop ourself nor superusers
 DROP ROLE regress_role_super;
 DROP ROLE regress_role_admin;
@@ -134,5 +184,12 @@ DROP INDEX tenant_idx;
 DROP TABLE tenant_table;
 DROP VIEW tenant_view;
 DROP ROLE regress_tenant;
+DROP ROLE regress_createrole;
+
+-- fail, cannot drop role with remaining privileges
+DROP ROLE regress_role_admin;
+
+-- ok, can drop role if we revoke privileges first
+REVOKE CREATE ON DATABASE regression FROM regress_role_admin;
 DROP ROLE regress_role_admin;
 DROP ROLE regress_role_super;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c8c545b64c..dcf84f91fd 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -30,8 +30,10 @@ 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;
 CREATE USER regress_priv_user8;
 CREATE USER regress_priv_user9;
 CREATE USER regress_priv_user10;
@@ -1426,8 +1428,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)