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.
