On 21.01.22 04:08, Masahiko Sawada wrote:
I think the superuser check in AlterSubscription() might no longer be
appropriate. Subscriptions can now be owned by non-superusers. Please
check that.
IIUC we don't allow non-superuser to own the subscription yet. We
still have the following superuser checks:
In CreateSubscription():
if (!superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to create subscriptions")));
and in AlterSubscriptionOwner_internal();
/* 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.")));
Also, doing superuser check here seems to be consistent with
pg_replication_origin_advance() which is another way to skip
transactions and also requires superuser permission.
I'm referring to commit a2ab9c06ea15fbcb2bfde570986a06b37f52bcca. You
still have to be superuser to create a subscription, but you can change
the owner to a nonprivileged user and it will observe table permissions
on the subscriber.
Assuming my understanding of that commit is correct, I think it would be
sufficient in your patch to check that the current user is the owner of
the subscription.