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

From 0a27010622339b7f80e7d622add73137c69a97c0 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Wed, 27 Oct 2021 07:34:18 -0700
Subject: [PATCH v2 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/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                   | 142 +++++++++++++++++-
 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     | 133 ++++++++++++++--
 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          |  64 +++++++-
 src/test/regress/sql/privileges.sql           |  13 +-
 21 files changed, 461 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index ce0a4ff14e..ddd205d656 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 role_oid, Oid roleid)
+{
+	HeapTuple		tuple;
+	Form_pg_authid	authform;
+	Oid				owner_oid;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	/* Otherwise, look up the owner of the role */
+	tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(role_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("role with OID %u does not exist",
+						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 == 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 8453d8fefd..88e479ca21 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"
@@ -1557,6 +1558,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 55f6e3711d..b03763695c 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 40044070cf..eb407cfc4c 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 aa69821be4..259968f1d4 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,104 @@ 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));
+
+		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 82464c9889..f406763d71 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4514,6 +4514,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 f537d3eb96..0d2c32121b 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 d0eb80e69c..08fd20e239 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1060,9 +1060,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;
+				}
 		;
 
 
@@ -1201,6 +1212,10 @@ CreateOptRoleElem:
 				{
 					$$ = makeDefElem("addroleto", (Node *)$3, @1);
 				}
+			| OWNER RoleSpec
+				{
+					$$ = makeDefElem("owner", (Node *)$2, @1);
+				}
 		;
 
 
@@ -9497,6 +9512,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 006661412e..e051cab6f9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3801,6 +3801,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)
@@ -3837,6 +3843,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);
@@ -3848,6 +3856,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 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..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 49123e28a4..0d5cfd3eb9 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2621,6 +2621,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 af771c901d..ec9d480d67 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 role_oid, Oid 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 57010bbb58..e1ce5f3a66 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_1 CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_1;
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_1;
 CREATE ROLE regress_role_2 SUPERUSER;
@@ -11,14 +12,95 @@ CREATE ROLE regress_role_4 REPLICATION;
 ERROR:  must be superuser to create replication users
 CREATE ROLE regress_role_5 BYPASSRLS;
 ERROR:  must be superuser to create bypassrls users
+-- fail, only superusers can own superusers
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_role_2 AUTHORIZATION regress_role_1 SUPERUSER;
+ERROR:  must be superuser to own superusers
+-- ok, superuser can create superusers belonging to other superusers
+CREATE ROLE regress_role_2 AUTHORIZATION regress_role_super SUPERUSER;
+-- ok, superuser can create users with these privileges for normal role
+CREATE ROLE regress_role_3 AUTHORIZATION regress_role_1 REPLICATION BYPASSRLS;
+CREATE ROLE regress_role_4 AUTHORIZATION regress_role_1 REPLICATION;
+CREATE ROLE regress_role_5 AUTHORIZATION regress_role_1 BYPASSRLS;
+\du+ regress_role_2
+                                      List of roles
+   Role name    |       Owner        |       Attributes        | Member of | Description 
+----------------+--------------------+-------------------------+-----------+-------------
+ regress_role_2 | regress_role_super | Superuser, Cannot login | {}        | 
+
+\du+ regress_role_3
+                                           List of roles
+   Role name    |     Owner      |              Attributes               | Member of | Description 
+----------------+----------------+---------------------------------------+-----------+-------------
+ regress_role_3 | regress_role_1 | Cannot login, Replication, Bypass RLS | {}        | 
+
+\du+ regress_role_4
+                                     List of roles
+   Role name    |     Owner      |        Attributes         | Member of | Description 
+----------------+----------------+---------------------------+-----------+-------------
+ regress_role_4 | regress_role_1 | Cannot login, Replication | {}        | 
+
+\du+ regress_role_5
+                                    List of roles
+   Role name    |     Owner      |        Attributes        | Member of | Description 
+----------------+----------------+--------------------------+-----------+-------------
+ regress_role_5 | regress_role_1 | Cannot login, Bypass RLS | {}        | 
+
 -- ok, having CREATEROLE is enough to create users with these privileges
+SET SESSION AUTHORIZATION regress_role_1;
 CREATE ROLE regress_role_6 CREATEDB;
 CREATE ROLE regress_role_7 CREATEROLE;
 CREATE ROLE regress_role_8 LOGIN;
 CREATE ROLE regress_role_9 INHERIT;
 CREATE ROLE regress_role_10 CONNECTION LIMIT 5;
-CREATE ROLE regress_role_11 ENCRYPTED PASSWORD 'foo';
-CREATE ROLE regress_role_12 PASSWORD NULL;
+CREATE ROLE regress_role_11 PASSWORD NULL;
+CREATE ROLE regress_role_12
+  CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo'
+  IN ROLE regress_role_6, regress_role_7, regress_role_8;
+\du+ regress_role_6
+                                    List of roles
+   Role name    |     Owner      |       Attributes        | Member of | Description 
+----------------+----------------+-------------------------+-----------+-------------
+ regress_role_6 | regress_role_1 | Create DB, Cannot login | {}        | 
+
+\du+ regress_role_7
+                                     List of roles
+   Role name    |     Owner      |        Attributes         | Member of | Description 
+----------------+----------------+---------------------------+-----------+-------------
+ regress_role_7 | regress_role_1 | Create role, Cannot login | {}        | 
+
+\du+ regress_role_8
+                             List of roles
+   Role name    |     Owner      | Attributes | Member of | Description 
+----------------+----------------+------------+-----------+-------------
+ regress_role_8 | regress_role_1 |            | {}        | 
+
+\du+ regress_role_9
+                              List of roles
+   Role name    |     Owner      |  Attributes  | Member of | Description 
+----------------+----------------+--------------+-----------+-------------
+ regress_role_9 | regress_role_1 | Cannot login | {}        | 
+
+\du+ regress_role_10
+                               List of roles
+    Role name    |     Owner      |  Attributes   | Member of | Description 
+-----------------+----------------+---------------+-----------+-------------
+ regress_role_10 | regress_role_1 | Cannot login +| {}        | 
+                 |                | 5 connections |           | 
+
+\du+ regress_role_11
+                               List of roles
+    Role name    |     Owner      |  Attributes  | Member of | Description 
+-----------------+----------------+--------------+-----------+-------------
+ regress_role_11 | regress_role_1 | Cannot login | {}        | 
+
+\du+ regress_role_12
+                                                      List of roles
+    Role name    |     Owner      |       Attributes       |                   Member of                    | Description 
+-----------------+----------------+------------------------+------------------------------------------------+-------------
+ regress_role_12 | regress_role_1 | Create role, Create DB+| {regress_role_6,regress_role_7,regress_role_8} | 
+                 |                | 2 connections          |                                                | 
+
 -- ok, backwards compatible noise words should be ignored
 CREATE ROLE regress_role_13 SYSID 12345;
 NOTICE:  SYSID can no longer be specified
@@ -84,16 +166,24 @@ CREATE ROLE regress_role_29 IN ROLE pg_read_server_files;
 CREATE ROLE regress_role_30 IN ROLE pg_write_server_files;
 CREATE ROLE regress_role_31 IN ROLE pg_execute_server_program;
 CREATE ROLE regress_role_32 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_role_7
 SET SESSION AUTHORIZATION regress_role_1;
+ALTER ROLE regress_role_20 OWNER TO regress_role_1;
+ERROR:  must be owner of role regress_role_20
+REASSIGN OWNED BY regress_role_20 TO regress_role_1;
+ERROR:  permission denied to reassign objects
+-- superuser can do it, though
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_role_20 OWNER TO regress_role_1;
+REASSIGN OWNED BY regress_role_20 TO regress_role_1;
+-- ok, superuser roles can drop superuser roles they own
+SET SESSION AUTHORIZATION regress_role_super;
 DROP ROLE regress_role_2;
-ERROR:  role "regress_role_2" does not exist
+-- ok, non-superuser roles can drop non-superuser roles they own
+SET SESSION AUTHORIZATION regress_role_1;
 DROP ROLE regress_role_3;
-ERROR:  role "regress_role_3" does not exist
 DROP ROLE regress_role_4;
-ERROR:  role "regress_role_4" does not exist
 DROP ROLE regress_role_5;
-ERROR:  role "regress_role_5" does not exist
 DROP ROLE regress_role_14;
 ERROR:  role "regress_role_14" does not exist
 DROP ROLE regress_role_15;
@@ -103,9 +193,23 @@ 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;
--- ok, should be able to drop non-superuser roles we created
-DROP ROLE regress_role_6;
+-- 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
+-- ok, should be able to drop these non-superuser roles
+DROP ROLE regress_role_6;
 DROP ROLE regress_role_8;
 DROP ROLE regress_role_9;
 DROP ROLE regress_role_10;
@@ -130,6 +234,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 +249,12 @@ 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;
+-- fail, cannot drop role with remaining privileges
+DROP ROLE regress_role_1;
+ERROR:  role "regress_role_1" 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_1;
 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 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 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/expected/rules.out b/src/test/regress/expected/rules.out
index 2fa00a3c29..c452cc433c 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 e00893de4e..8a4f177574 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_1 CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_1;
 
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_1;
@@ -9,14 +10,42 @@ CREATE ROLE regress_role_3 REPLICATION BYPASSRLS;
 CREATE ROLE regress_role_4 REPLICATION;
 CREATE ROLE regress_role_5 BYPASSRLS;
 
+-- fail, only superusers can own superusers
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_role_2 AUTHORIZATION regress_role_1 SUPERUSER;
+
+-- ok, superuser can create superusers belonging to other superusers
+CREATE ROLE regress_role_2 AUTHORIZATION regress_role_super SUPERUSER;
+
+-- ok, superuser can create users with these privileges for normal role
+CREATE ROLE regress_role_3 AUTHORIZATION regress_role_1 REPLICATION BYPASSRLS;
+CREATE ROLE regress_role_4 AUTHORIZATION regress_role_1 REPLICATION;
+CREATE ROLE regress_role_5 AUTHORIZATION regress_role_1 BYPASSRLS;
+
+\du+ regress_role_2
+\du+ regress_role_3
+\du+ regress_role_4
+\du+ regress_role_5
+
 -- ok, having CREATEROLE is enough to create users with these privileges
+SET SESSION AUTHORIZATION regress_role_1;
 CREATE ROLE regress_role_6 CREATEDB;
 CREATE ROLE regress_role_7 CREATEROLE;
 CREATE ROLE regress_role_8 LOGIN;
 CREATE ROLE regress_role_9 INHERIT;
 CREATE ROLE regress_role_10 CONNECTION LIMIT 5;
-CREATE ROLE regress_role_11 ENCRYPTED PASSWORD 'foo';
-CREATE ROLE regress_role_12 PASSWORD NULL;
+CREATE ROLE regress_role_11 PASSWORD NULL;
+CREATE ROLE regress_role_12
+  CREATEDB CREATEROLE LOGIN INHERIT CONNECTION LIMIT 2 ENCRYPTED PASSWORD 'foo'
+  IN ROLE regress_role_6, regress_role_7, regress_role_8;
+
+\du+ regress_role_6
+\du+ regress_role_7
+\du+ regress_role_8
+\du+ regress_role_9
+\du+ regress_role_10
+\du+ regress_role_11
+\du+ regress_role_12
 
 -- ok, backwards compatible noise words should be ignored
 CREATE ROLE regress_role_13 SYSID 12345;
@@ -86,9 +115,22 @@ CREATE ROLE regress_role_30 IN ROLE pg_write_server_files;
 CREATE ROLE regress_role_31 IN ROLE pg_execute_server_program;
 CREATE ROLE regress_role_32 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_role_7
 SET SESSION AUTHORIZATION regress_role_1;
+ALTER ROLE regress_role_20 OWNER TO regress_role_1;
+REASSIGN OWNED BY regress_role_20 TO regress_role_1;
+
+-- superuser can do it, though
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_role_20 OWNER TO regress_role_1;
+REASSIGN OWNED BY regress_role_20 TO regress_role_1;
+
+-- ok, superuser roles can drop superuser roles they own
+SET SESSION AUTHORIZATION regress_role_super;
 DROP ROLE regress_role_2;
+
+-- ok, non-superuser roles can drop non-superuser roles they own
+SET SESSION AUTHORIZATION regress_role_1;
 DROP ROLE regress_role_3;
 DROP ROLE regress_role_4;
 DROP ROLE regress_role_5;
@@ -98,9 +140,11 @@ DROP ROLE regress_role_17;
 DROP ROLE regress_role_19;
 DROP ROLE regress_role_20;
 
--- ok, should be able to drop non-superuser roles we created
-DROP ROLE regress_role_6;
+-- fail, cannot drop roles that own other roles
 DROP ROLE regress_role_7;
+
+-- ok, should be able to drop these non-superuser roles
+DROP ROLE regress_role_6;
 DROP ROLE regress_role_8;
 DROP ROLE regress_role_9;
 DROP ROLE regress_role_10;
@@ -124,6 +168,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 +181,12 @@ 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;
+
+-- fail, cannot drop role with remaining privileges
+DROP ROLE regress_role_1;
+
+-- ok, can drop role if we revoke privileges first
+REVOKE CREATE ON DATABASE regression FROM regress_role_1;
 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)