From b45c372a559202295ca5dc352ad57f70f8778f81 Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Tue, 28 Jul 2026 17:46:47 +0530 Subject: [PATCH v3] Check foreign server permissions for ALTER SUBSCRIPTION ... ADD/DROP/SET/REFRESH PUBLICATION When a subscription is created using a foreign server, ALTER SUBSCRIPTION ... REFRESH PUBLICATION, ADD PUBLICATION, DROP PUBLICATION, and SET PUBLICATION may connect to the publisher to fetch the updated relation list. However, AlterSubscription() calls GetSubscription() with conninfo_aclcheck set to false, which skips the ACL check on the associated foreign server. As a result, these commands can successfully connect to the publisher even when the user does not have permission to access the server. Fix this by enabling conninfo_aclcheck for these ALTER SUBSCRIPTION operations so that access permissions on the foreign server are validated before establishing the connection. --- src/backend/commands/subscriptioncmds.c | 22 +++++++++++++++++++++- src/test/regress/expected/subscription.out | 7 +++++++ src/test/regress/sql/subscription.sql | 8 ++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 013ac46db07..02a82d5cf6a 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1618,6 +1618,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, Datum values[Natts_pg_subscription]; HeapTuple tup; Oid subid; + bool orig_conninfo_aclchk = false; bool orig_conninfo_needed = true; bool update_tuple = false; bool update_failover = false; @@ -1729,6 +1730,24 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, orig_conninfo_needed = false; } + /* + * ALTER SUBSCRIPTION ... REFRESH PUBLICATION and ALTER SUBSCRIPTION ... + * ADD/DROP/SET PUBLICATION with refresh = true connect to the publisher + * to fetch the updated publication information. Permissions on the + * original foreign server must be validated. + */ + else if (stmt->kind == ALTER_SUBSCRIPTION_REFRESH_PUBLICATION) + { + orig_conninfo_aclchk = true; + } + else if (stmt->kind == ALTER_SUBSCRIPTION_ADD_PUBLICATION || + stmt->kind == ALTER_SUBSCRIPTION_DROP_PUBLICATION || + stmt->kind == ALTER_SUBSCRIPTION_SET_PUBLICATION) + { + if (opts.refresh) + orig_conninfo_aclchk = true; + } + /* * Skip ACL checks on the subscription's foreign server, if any. If * changing the server (or replacing it with a raw connection), then the @@ -1736,7 +1755,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, * there's no need to do an additional ACL check here; that will be done * by the subscription worker. */ - sub = GetSubscription(subid, false, orig_conninfo_needed, false); + sub = GetSubscription(subid, false, orig_conninfo_needed, + orig_conninfo_aclchk); retain_dead_tuples = sub->retaindeadtuples; origin = sub->origin; diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 1bb785f4f9f..4e8a67392ed 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -193,6 +193,13 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server WARNING: subscription was created, but is not connected HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and alter the subscription to refresh publications. RESET SESSION AUTHORIZATION; +REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; +SET SESSION AUTHORIZATION regress_subscription_user3; +-- fail, need USAGE privileges on the server +ALTER SUBSCRIPTION regress_testsub6 REFRESH PUBLICATION; +ERROR: subscription owner "regress_subscription_user3" does not have permission on foreign server "test_server" +RESET SESSION AUTHORIZATION; +GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3; GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2; CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo'); ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index f19740fdfb8..aa0b3d722ff 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -140,6 +140,14 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = 'dummy', connect = false); RESET SESSION AUTHORIZATION; +REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; +SET SESSION AUTHORIZATION regress_subscription_user3; + +-- fail, need USAGE privileges on the server +ALTER SUBSCRIPTION regress_testsub6 REFRESH PUBLICATION; + +RESET SESSION AUTHORIZATION; +GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3; GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2; CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo'); ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password; -- 2.34.1