From 9b1be64005e0e78694f9b6965075b35cfa77072e Mon Sep 17 00:00:00 2001 From: Mark Dilger 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 name RENAME TO < You must own the subscription to use ALTER SUBSCRIPTION. 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. 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)