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.


Reply via email to