v1-0002-Allow-subscription-ownership-by-non-superusers.patch

From 9b1be64005e0e78694f9b6965075b35cfa77072e Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Sun, 26 Sep 2021 18:24:07 -0700
Subject: [PATCH v1 2/3] Allow subscription ownership by non-superusers

Non-superusers can already end up owning subscriptions if superuser
is revoked from a subscription owner, so quit the pretense and
simply allow the ownership transfer overtly by ALTER SUBSCRIPTION.

The prior situation did more to give a false sense that
subscriptions would never be owned by non-superusers than it did to
actually prevent that scenario from arising.
---
 doc/src/sgml/ref/alter_subscription.sgml   |  2 +-
 src/backend/commands/subscriptioncmds.c    |  9 ++-------
 src/test/regress/expected/subscription.out | 12 ++++--------
 src/test/regress/sql/subscription.sql      | 11 ++++-------
 4 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index b1fd73f776..bc52339eba 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -46,7 +46,7 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
   <para>
    You must own the subscription to use <command>ALTER SUBSCRIPTION</command>.
    To alter the owner, you must also be a direct or indirect member of the
-   new owning role. The new owner has to be a superuser.
+   new owning role.
   </para>
 
   <para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 6a5d192128..d75183cd12 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1476,13 +1476,8 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION,
 					   NameStr(form->subname));
 
-	/* New owner must be a superuser */
-	if (!superuser_arg(newOwnerId))
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("permission denied to change owner of subscription \"%s\"",
-						NameStr(form->subname)),
-				 errhint("The owner of a subscription must be a superuser.")));
+	/* Must be able to assign ownership to the target role */
+	check_is_member_of_role(GetUserId(), newOwnerId);
 
 	form->subowner = newOwnerId;
 	CatalogTupleUpdate(rel, &tup->t_self, tup);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index a05e6f1795..9b4dac9c0e 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -137,16 +137,12 @@ HINT:  Available values: local, remote_write, remote_apply, on, off.
 
 -- rename back to keep the rest simple
 ALTER SUBSCRIPTION regress_testsub_foo RENAME TO regress_testsub;
--- fail - new owner must be superuser
+-- superuser can assign the ownership to a non-superuser
 ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
-ERROR:  permission denied to change owner of subscription "regress_testsub"
-HINT:  The owner of a subscription must be a superuser.
-ALTER ROLE regress_subscription_user2 SUPERUSER;
--- now it works
-ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
--- revoke superuser from new owner
-ALTER ROLE regress_subscription_user2 NOSUPERUSER;
 SET SESSION AUTHORIZATION regress_subscription_user2;
+-- fail - not a member of the target role
+ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user;
+ERROR:  must be member of role "regress_subscription_user"
 -- fail - non-superuser owner cannot change connection parameter
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=somethingelse';
 ERROR:  must be superuser to alter the connection for a subscription
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 8f66709441..6c68d547d1 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -99,17 +99,14 @@ ALTER SUBSCRIPTION regress_testsub_foo SET (synchronous_commit = foobar);
 -- rename back to keep the rest simple
 ALTER SUBSCRIPTION regress_testsub_foo RENAME TO regress_testsub;
 
--- fail - new owner must be superuser
-ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
-ALTER ROLE regress_subscription_user2 SUPERUSER;
--- now it works
+-- superuser can assign the ownership to a non-superuser
 ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
 
--- revoke superuser from new owner
-ALTER ROLE regress_subscription_user2 NOSUPERUSER;
-
 SET SESSION AUTHORIZATION regress_subscription_user2;
 
+-- fail - not a member of the target role
+ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user;
+
 -- fail - non-superuser owner cannot change connection parameter
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=somethingelse';
 
-- 
2.21.1 (Apple Git-122.3)