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.
