Dear Shlok, > While testing another patch, I found a bug related to subscriptions > created using a foreign server.
Good catch. > This happens because AlterSubscription() calls: > GetSubscription(subid, false, orig_conninfo_needed, false) > with conninfo_aclcheck set to false, so the ACL check on the > associated foreign server is skipped. Agree. There is an ongoing discussion to weaken the validation at DDL phase [1], but IIUC it does not match with this issue. REFRESH PUBLICATION command needs to refer the conninfo to connect to the publisher, so validation is mandatory. > The attached patch fixes this by enabling conninfo_aclcheck for > these ALTER SUBSCRIPTION operations, ensuring that permissions on > the foreign server are validated before connecting to the publisher. > Thoughts? I think for ADD/DROP/SET PUBLICATION commands, ACL check is not needed non-refresh case. For these cases, backends won't directly connect to the publisher server: apply worker will check privileges and raise the ERROR. Can you set orig_conninfo_aclchk to true only for REFRESH case? [1]: https://www.postgresql.org/message-id/e96efe16fb47fcca4ae0cd0157fb6bc662470712.camel%40j-davis.com Best regards, Hayato Kuroda FUJITSU LIMITED
