database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

When changing owner of a subscription, do not throw an error.

Browse Source 
Errors will be caught when the connection is actually used.

Restore uses multiple DDL commands to restore a subscription, so
checks of the intermediate state risk restore errors. In the future we
could address this with a more careful restoration order, but the
DDL-time errors are merely for convenience.

Addresses finding 2 in the report from the linked discussion.

Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Discussion: https://postgr.es/m/e103ae8daf74485e0c0ebde297fae735d38f54d1.camel@j-davis.com
Backpatch-through: 19
This commit is contained in:
Jeff Davis
2026-08-05 13:19:21 -07:00
parent 45b479a836
commit a1e39c501d
3 changed files with 16 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/backend/commands/subscriptioncmds.c
View File

@@ -2790,25 +2790,15 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
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.
* The privileges will be checked before the connection is actually used,
* so it does not need to be done here. Avoid unnecessary risk of errors
* here, which could interfere with restore.
*
* However, it is convenient to check if a user mapping exists, and raise
* a WARNING if not.
*/
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);
}
GetUserMappingExtended(newOwnerId, form->subserver, WARNING);
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, &tup->t_self, tup);

5
src/test/regress/expected/subscription.out
View File

@@ -191,6 +191,11 @@ RESET SESSION AUTHORIZATION;
DROP SERVER test_server CASCADE;
ERROR: cannot drop server test_server because subscription regress_testsub6 depends on it
HINT: Drop subscription regress_testsub6 first.
-- ok, USAGE privilege on server not checked for OWNER TO, but warn
-- about user mapping
ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2;
WARNING: user mapping not found for user "regress_subscription_user2", server "test_server"
ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3;
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
-- ok, lacks USAGE on test_server, but replacing connection anyway

4
src/test/regress/sql/subscription.sql
View File

@@ -136,6 +136,10 @@ RESET SESSION AUTHORIZATION;
-- fail, subscription depends on the server and cannot be dropped by CASCADE
DROP SERVER test_server CASCADE;
-- ok, USAGE privilege on server not checked for OWNER TO, but warn
-- about user mapping
ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2;
ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3;
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;