From 648b90412b801708b15dcb23ce63757451733072 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Sun, 19 Jul 2026 11:26:41 -0700 Subject: [PATCH v1] Fix dump/restore of server-based subscriptions. Defer connection validation until the time the connection is actually used. A server-based subscription generates the connection string based on the subscription owner and available user mapping, which may change between the time the subscription is created and when it's actually used. In particular, dump/restore first creates the subscription and then changes the owner. The creation must succeed even if the user mapping doesn't exist for the restoring user, or if there's some other problem with the connection string which might be fine after the owner is set properly. This change also affects ALTER SUBSCRIPTION ... OWNER TO, which previously checked for a user mapping, and now does not (until the connection is used). While that could be made to work with dump/restore, it would make subscriptions dependent on the ACLs for foreign servers, and move subscription creation to a later phase. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com Discussion: https://postgr.es/m/20260710192533.4f.noahmisch%40microsoft.com Backpatch-through: 19 --- doc/src/sgml/ref/create_subscription.sgml | 7 ++-- src/backend/commands/subscriptioncmds.c | 49 ++++++++++------------ src/backend/foreign/foreign.c | 28 +++++++++---- src/include/foreign/foreign.h | 1 + src/test/regress/expected/subscription.out | 11 +++-- src/test/regress/sql/subscription.sql | 14 ++++--- 6 files changed, 63 insertions(+), 47 deletions(-) diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 81fbf3487a4..82f960fa027 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -83,10 +83,9 @@ CREATE SUBSCRIPTION subscription_name A foreign server to use for the connection. The server's foreign data wrapper must have a connection_function - registered, and a user mapping for the subscription owner on the server - must exist. Additionally, the subscription owner must have - USAGE privileges on - servername. + registered. When the connection is used, the subscription owner must + have USAGE privileges on + servername and a user mapping must exist. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 630d2498fa0..195bd939708 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -805,11 +805,26 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); - /* make sure a user mapping exists */ - GetUserMapping(owner, server->serverid); - serverid = server->serverid; - conninfo = ForeignServerConnectionString(owner, server); + + if (opts.connect) + { + /* make sure a user mapping exists */ + GetUserMapping(owner, server->serverid); + + conninfo = ForeignServerConnectionString(owner, server); + } + else + { + /* + * If connect = false, don't check the connection information + * (necessary for dump/restore, which creates the subscription as + * the restoring user first and then changes the owner). However, + * still check that the server's FDW at least supports a + * connection function. + */ + GetForeignServerConnectionFunction(server); + } } else { @@ -819,8 +834,9 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, conninfo = stmt->conninfo; } - /* Check the connection info string. */ - walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser()); + /* Check the connection info string, if we need one. */ + if (conninfo) + walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser()); publications = stmt->publication; @@ -2904,27 +2920,6 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) aclcheck_error(aclresult, OBJECT_DATABASE, get_database_name(MyDatabaseId)); - /* - * If the subscription uses a server, check that the new owner has USAGE - * privileges on the server and that a user mapping exists. Note: does not - * re-check the resulting connection string. - */ - if (OidIsValid(form->subserver)) - { - ForeignServer *server = GetForeignServer(form->subserver); - - aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, newOwnerId, ACL_USAGE); - if (aclresult != ACLCHECK_OK) - ereport(ERROR, - errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("new subscription owner \"%s\" does not have permission on foreign server \"%s\"", - GetUserNameFromId(newOwnerId, false), - server->servername)); - - /* make sure a user mapping exists */ - GetUserMapping(newOwnerId, server->serverid); - } - form->subowner = newOwnerId; CatalogTupleUpdate(rel, &tup->t_self, tup); diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c index 821d45c1e11..8de081a7377 100644 --- a/src/backend/foreign/foreign.c +++ b/src/backend/foreign/foreign.c @@ -194,15 +194,12 @@ GetForeignServerByName(const char *srvname, bool missing_ok) /* - * Retrieve connection string from server's FDW. - * - * NB: leaks into CurrentMemoryContext. + * Get the connection function from a server's FDW. */ -char * -ForeignServerConnectionString(Oid userid, ForeignServer *server) +Oid +GetForeignServerConnectionFunction(ForeignServer *server) { ForeignDataWrapper *fdw; - Datum connection_datum; fdw = GetForeignDataWrapper(server->fdwid); @@ -213,7 +210,24 @@ ForeignServerConnectionString(Oid userid, ForeignServer *server) fdw->fdwname), errdetail("Foreign data wrapper must be defined with CONNECTION specified."))); - connection_datum = OidFunctionCall3(fdw->fdwconnection, + return fdw->fdwconnection; +} + + +/* + * Retrieve connection string from server's FDW. + * + * NB: leaks into CurrentMemoryContext. + */ +char * +ForeignServerConnectionString(Oid userid, ForeignServer *server) +{ + Datum connection_datum; + Oid connection_function; + + connection_function = GetForeignServerConnectionFunction(server); + + connection_datum = OidFunctionCall3(connection_function, ObjectIdGetDatum(userid), ObjectIdGetDatum(server->serverid), PointerGetDatum(NULL)); diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h index 92a55214fee..f0d2d1f6ad5 100644 --- a/src/include/foreign/foreign.h +++ b/src/include/foreign/foreign.h @@ -70,6 +70,7 @@ extern ForeignServer *GetForeignServerExtended(Oid serverid, uint16 flags); extern ForeignServer *GetForeignServerByName(const char *srvname, bool missing_ok); +extern Oid GetForeignServerConnectionFunction(ForeignServer *server); extern char *ForeignServerConnectionString(Oid userid, ForeignServer *server); extern UserMapping *GetUserMapping(Oid userid, Oid serverid); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index d201ad764f0..5daa48a1c35 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -173,10 +173,6 @@ ERROR: permission denied for foreign server test_server RESET SESSION AUTHORIZATION; GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; --- fail, need user mapping -CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false); -ERROR: user mapping not found for user "regress_subscription_user3", server "test_server" -CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); -- fail, need CONNECTION clause CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false); ERROR: foreign data wrapper "test_fdw" does not support subscription connections @@ -184,10 +180,12 @@ DETAIL: Foreign data wrapper must be defined with CONNECTION specified. RESET SESSION AUTHORIZATION; ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; SET SESSION AUTHORIZATION regress_subscription_user3; +-- ok, user mapping is not needed with connect = false CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = 'dummy', connect = false); 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. +CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); RESET SESSION AUTHORIZATION; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; @@ -209,6 +207,11 @@ 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. DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server; +-- ok, changing owner does not require server access or a user mapping +RESET SESSION AUTHORIZATION; +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3; +SET SESSION AUTHORIZATION regress_subscription_user3; -- ok, test_server lacks user mapping, but replacing connection anyway BEGIN; ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret'; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 86c402c59aa..f53a59326c9 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -120,11 +120,6 @@ RESET SESSION AUTHORIZATION; GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; --- fail, need user mapping -CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false); - -CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); - -- fail, need CONNECTION clause CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false); @@ -132,9 +127,12 @@ RESET SESSION AUTHORIZATION; ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; SET SESSION AUTHORIZATION regress_subscription_user3; +-- ok, user mapping is not needed with connect = false CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = 'dummy', connect = false); +CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); + RESET SESSION AUTHORIZATION; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; @@ -159,6 +157,12 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server; +-- ok, changing owner does not require server access or a user mapping +RESET SESSION AUTHORIZATION; +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3; +SET SESSION AUTHORIZATION regress_subscription_user3; + -- ok, test_server lacks user mapping, but replacing connection anyway BEGIN; ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret'; -- 2.43.0