From c464be40ac12c409dc76a528c5975342a2aa9042 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 Jul 2026 22:05:19 +0100 Subject: [PATCH 1/4] Implement MSC4233: remember which server a user knocked through Fixes rescinding and denying knocks over federation (#18030): - Remember the server that fulfilled our /send_knock in a new local_knock_via_servers table. - Route a rescission of the knock (make_leave/send_leave) through that server, instead of only rescinding locally. - Send a leave/ban denying a knock to the knocking user's server (the same mechanism as invite rescissions), and accept such an event as an out-of-band retraction on the knocking server when it references the knock in its auth events and comes from the server we knocked through (or the denying user's own server). Gated behind the experimental msc4233_enabled flag. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Uk8aPxHn3BHCe52L226jdG --- changelog.d/18030.feature | 1 + synapse/config/experimental.py | 4 + synapse/federation/federation_client.py | 27 +- synapse/federation/sender/__init__.py | 30 +- synapse/handlers/federation.py | 24 +- synapse/handlers/federation_event.py | 43 +- synapse/handlers/room_member.py | 37 +- synapse/storage/databases/main/roommember.py | 41 ++ .../delta/94/07_local_knock_via_servers.sql | 27 + .../test_federation_knock_msc4233.py | 597 ++++++++++++++++++ 10 files changed, 804 insertions(+), 27 deletions(-) create mode 100644 changelog.d/18030.feature create mode 100644 synapse/storage/schema/main/delta/94/07_local_knock_via_servers.sql create mode 100644 tests/federation/test_federation_knock_msc4233.py diff --git a/changelog.d/18030.feature b/changelog.d/18030.feature new file mode 100644 index 00000000000..cbe505f43b1 --- /dev/null +++ b/changelog.d/18030.feature @@ -0,0 +1 @@ +Add experimental support for [MSC4233](https://github.com/matrix-org/matrix-spec-proposals/pull/4233): remember which server a user knocked through, so that knocks can be rescinded and denied over federation. diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index f99f7b139ec..8b18409bf2c 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -256,6 +256,10 @@ def read_config( # MSC4222: Adding `state_after` to sync v2 self.msc4222_enabled: bool = experimental.get("msc4222_enabled", False) + # MSC4233: Remembering which server a user knocked through, so that + # knocks can be rescinded and denied over federation. + self.msc4233_enabled: bool = experimental.get("msc4233_enabled", False) + # MSC4076: Add `disable_badge_count`` to pusher configuration self.msc4076_enabled: bool = experimental.get("msc4076_enabled", False) diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 5b334543256..0cf3ed8593a 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -126,6 +126,15 @@ class SendJoinResult: servers_in_room: AbstractSet[str] +@attr.s(slots=True, frozen=True, auto_attribs=True) +class SendKnockResult: + # The response body from the remote server, of the form + # {"knock_room_state": [, ...]}. + response: JsonDict + # The server which fulfilled the knock (i.e. answered our /send_knock). + origin: str + + class FederationClient(FederationBase): def __init__(self, hs: "HomeServer"): super().__init__(hs) @@ -1437,7 +1446,9 @@ async def _do_send_leave(self, destination: str, pdu: EventBase) -> JsonDict: # content. return resp[1] - async def send_knock(self, destinations: list[str], pdu: EventBase) -> JsonDict: + async def send_knock( + self, destinations: list[str], pdu: EventBase + ) -> "SendKnockResult": """Attempts to send a knock event to a given list of servers. Iterates through the list until one attempt succeeds. @@ -1450,20 +1461,18 @@ async def send_knock(self, destinations: list[str], pdu: EventBase) -> JsonDict: pdu: The event to be sent. Returns: - The remote homeserver return some state from the room. The response - dictionary is in the form: - - {"knock_room_state": [, ...]} - - The list of state events may be empty. + A SendKnockResult holding the remote homeserver's response (some + state from the room, possibly empty) and the name of the server + that fulfilled the knock. Raises: SynapseError: If the chosen remote server returns a 3xx/4xx code. RuntimeError: If no servers were reachable. """ - async def send_request(destination: str) -> JsonDict: - return await self._do_send_knock(destination, pdu) + async def send_request(destination: str) -> "SendKnockResult": + response = await self._do_send_knock(destination, pdu) + return SendKnockResult(response=response, origin=destination) return await self._try_destination_list( "send_knock", destinations, send_request diff --git a/synapse/federation/sender/__init__.py b/synapse/federation/sender/__init__.py index f7240c2f7f9..807726da400 100644 --- a/synapse/federation/sender/__init__.py +++ b/synapse/federation/sender/__init__.py @@ -669,18 +669,23 @@ async def handle_event(event: EventBase) -> None: ) return - # If we've rescinded an invite then we want to tell the - # other server. + # If we've rescinded an invite, or denied a knock + # (MSC4233), then we want to tell the other server. + msc4233_enabled = self.hs.config.experimental.msc4233_enabled if ( event.type == EventTypes.Member - and event.membership == Membership.LEAVE and event.sender != event.state_key + and ( + event.membership == Membership.LEAVE + or (msc4233_enabled and event.membership == Membership.BAN) + ) ): # We check if this leave event is rescinding an invite - # by looking if there is an invite event for the user in - # the auth events. It could otherwise be a kick or - # unban, which we don't want to send (if the user wasn't - # already in the room). + # (or denying a knock) by looking if there is an invite + # (or knock) event for the user in the auth events. It + # could otherwise be a kick or unban, which we don't + # want to send (if the user wasn't already in the + # room). auth_events = await self.store.get_events_as_list( event.auth_event_ids() ) @@ -688,7 +693,16 @@ async def handle_event(event: EventBase) -> None: if ( auth_event.type == EventTypes.Member and auth_event.state_key == event.state_key - and auth_event.membership == Membership.INVITE + and ( + ( + event.membership == Membership.LEAVE + and auth_event.membership == Membership.INVITE + ) + or ( + msc4233_enabled + and auth_event.membership == Membership.KNOCK + ) + ) ): destinations = set(destinations) destinations.add(get_domain_from_id(event.state_key)) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index ba83d4fd268..4ba94c45e54 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -886,7 +886,18 @@ async def do_knock( # Send the signed event back to the room, and potentially receive some # further information about the room in the form of partial state events - knock_response = await self.federation_client.send_knock(target_hosts, event) + send_knock_result = await self.federation_client.send_knock(target_hosts, event) + knock_response = send_knock_result.response + + # Remember which server fulfilled the knock, so that we can route a + # rescission of the knock through it later, and know which server to + # trust for an out-of-band retraction of the knock (MSC4233). + await self.store.store_local_knock_via_server( + user_id=knockee, + room_id=event.room_id, + via_server=send_knock_result.origin, + knock_event_id=event.event_id, + ) # Store any stripped room state events in the "unsigned" key of the event. # This is a bit of a hack and is cribbing off of invites. Basically we @@ -1162,6 +1173,17 @@ async def on_invite_request( return event + async def do_remotely_rescind_knock( + self, target_hosts: Iterable[str], room_id: str, user_id: str, content: JsonDict + ) -> tuple[EventBase, int]: + """Rescind a knock on a remote room: the same make_leave/send_leave + dance as rejecting an invite, routed through the server(s) given + (normally the server the knock was fulfilled through, per MSC4233). + """ + return await self.do_remotely_reject_invite( + target_hosts, room_id, user_id, content + ) + async def do_remotely_reject_invite( self, target_hosts: Iterable[str], room_id: str, user_id: str, content: JsonDict ) -> tuple[EventBase, int]: diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 7cbc52d350e..2ba321eacdd 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -256,12 +256,17 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: room_id, self.server_name ) if not is_in_room: - # Check if this is a leave event rescinding an invite + # Check if this is a leave event rescinding an invite, or denying + # a knock (MSC4233) + msc4233_enabled = self._config.experimental.msc4233_enabled if ( pdu.type == EventTypes.Member - and pdu.membership == Membership.LEAVE and pdu.state_key != pdu.sender and self._is_mine_id(pdu.state_key) + and ( + pdu.membership == Membership.LEAVE + or (msc4233_enabled and pdu.membership == Membership.BAN) + ) ): ( membership, @@ -270,7 +275,8 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: pdu.state_key, pdu.room_id ) if ( - membership == Membership.INVITE + pdu.membership == Membership.LEAVE + and membership == Membership.INVITE and membership_event_id and membership_event_id in pdu.auth_event_ids() # The invite should be in the auth events of the rescission. @@ -292,6 +298,37 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: context = EventContext.for_outlier(self._storage_controllers) await self.persist_events_and_notify(room_id, [(pdu, context)]) return + elif ( + msc4233_enabled + and membership == Membership.KNOCK + and membership_event_id + and membership_event_id + in pdu.auth_event_ids() # The knock should be in the auth events of the denial. + ): + # A local user's knock on this remote room is being denied + # (or the user banned). + # + # We cannot fully auth the denial event, but the sender has + # demonstrated knowledge of our knock by referencing it in + # the auth events, and the event's signature (checked on + # receipt) proves it comes from the sender's server. On top + # of that, only accept the event from the server the knock + # was fulfilled through, or from the denying user's own + # server. + # + # Technically we cannot verify that the sender has the + # power level required to deny the knock, but the same + # holds for invite rescission above. + via_server = await self._store.get_local_knock_via_server( + pdu.state_key, pdu.room_id + ) + if origin == via_server or origin == get_domain_from_id(pdu.sender): + # Handle the denial event + pdu.internal_metadata.outlier = True + pdu.internal_metadata.out_of_band_membership = True + context = EventContext.for_outlier(self._storage_controllers) + await self.persist_events_and_notify(room_id, [(pdu, context)]) + return logger.info( "Ignoring PDU from %s as we're not in the room", diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 5152d0b522e..5893ef82bde 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -2086,13 +2086,38 @@ async def remote_rescind_knock( Implements RoomMemberHandler.remote_rescind_knock """ - # TODO: We don't yet support rescinding knocks over federation - # as we don't know which homeserver to send it to. An obvious - # candidate is the remote homeserver we originally knocked through, - # however we don't currently store that information. - - # Just rescind the knock locally knock_event = await self.store.get_event(knock_event_id) + + if self.hs.config.experimental.msc4233_enabled: + # MSC4233: route the rescission through the server the knock was + # fulfilled through, which we remembered at knock time. + via_server = await self.store.get_local_knock_via_server( + knock_event.state_key, knock_event.room_id + ) + if via_server is not None: + try: + ( + event, + stream_id, + ) = await self.federation_handler.do_remotely_rescind_knock( + [via_server], + knock_event.room_id, + knock_event.state_key, + content, + ) + return event.event_id, stream_id + except Exception as e: + # If we can't reach the remote server, fall back to + # rescinding the knock locally, as we would have done + # before MSC4233. + logger.warning( + "Failed to rescind knock on %s via %s: %s", + knock_event.room_id, + via_server, + e, + ) + + # Rescind the knock locally return await self._generate_local_out_of_band_leave( knock_event, txn_id, requester, content ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 736f3e4c781..c759f8613ac 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -685,6 +685,47 @@ async def get_local_current_membership_for_user_in_room( return results + async def store_local_knock_via_server( + self, user_id: str, room_id: str, via_server: str, knock_event_id: str + ) -> None: + """Record the remote server that fulfilled a local user's knock on a + remote room, so that a later rescission can be routed through it + (MSC4233). Overwrites any previous record for the user/room pair. + + Args: + user_id: The ID of the knocking (local) user. + room_id: The ID of the room that was knocked on. + via_server: The remote server that answered our /send_knock. + knock_event_id: The event ID of the knock membership event. + """ + await self.db_pool.simple_upsert( + table="local_knock_via_servers", + keyvalues={"user_id": user_id, "room_id": room_id}, + values={"via_server": via_server, "knock_event_id": knock_event_id}, + desc="store_local_knock_via_server", + ) + + async def get_local_knock_via_server( + self, user_id: str, room_id: str + ) -> str | None: + """Retrieve the remote server that fulfilled a local user's knock on a + remote room, if we have a record of one (MSC4233). + + Args: + user_id: The ID of the knocking (local) user. + room_id: The ID of the room that was knocked on. + + Returns: + The server name, or None if no knock route is recorded. + """ + return await self.db_pool.simple_select_one_onecol( + table="local_knock_via_servers", + keyvalues={"user_id": user_id, "room_id": room_id}, + retcol="via_server", + allow_none=True, + desc="get_local_knock_via_server", + ) + async def get_users_server_still_shares_room_with( self, user_ids: Collection[str] ) -> set[str]: diff --git a/synapse/storage/schema/main/delta/94/07_local_knock_via_servers.sql b/synapse/storage/schema/main/delta/94/07_local_knock_via_servers.sql new file mode 100644 index 00000000000..42a0d6766ab --- /dev/null +++ b/synapse/storage/schema/main/delta/94/07_local_knock_via_servers.sql @@ -0,0 +1,27 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2026 New Vector, Ltd +-- +-- This program is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as +-- published by the Free Software Foundation, either version 3 of the +-- License, or (at your option) any later version. +-- +-- See the GNU Affero General Public License for more details: +-- . + +-- Records the remote server that fulfilled a local user's knock on a remote +-- room (i.e. the server that answered our /send_knock request), so that we can +-- route a subsequent rescission of the knock through the same server, and +-- so that we know which server to trust for out-of-band retractions of the +-- knock (MSC4233). +CREATE TABLE local_knock_via_servers ( + user_id TEXT NOT NULL, + room_id TEXT NOT NULL, + -- The server the knock was fulfilled through. + via_server TEXT NOT NULL, + -- The event ID of the knock membership event. + knock_event_id TEXT NOT NULL, + CONSTRAINT local_knock_via_servers_uniqueness UNIQUE (user_id, room_id) +); diff --git a/tests/federation/test_federation_knock_msc4233.py b/tests/federation/test_federation_knock_msc4233.py new file mode 100644 index 00000000000..4f01fd1d058 --- /dev/null +++ b/tests/federation/test_federation_knock_msc4233.py @@ -0,0 +1,597 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 New Vector, Ltd +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . +# + +"""Tests for MSC4233: remembering which server a user knocked through, so +that knocks can be rescinded and denied over federation.""" + +import logging +import time +import urllib.parse +from http import HTTPStatus +from typing import Any, Callable, TypeVar +from unittest.mock import Mock + +import attr + +from twisted.internet.testing import MemoryReactor + +from synapse.api.constants import EventContentFields, EventTypes, Membership +from synapse.api.room_versions import RoomVersion, RoomVersions +from synapse.events import EventBase, builder +from synapse.events.snapshot import EventContext +from synapse.events.utils import strip_event +from synapse.http.matrixfederationclient import ByteParser +from synapse.http.types import QueryParams +from synapse.rest import admin +from synapse.rest.client import knock, login, room +from synapse.server import HomeServer +from synapse.types import JsonDict +from synapse.util.clock import Clock + +from tests import unittest +from tests.test_utils.event_builders import make_test_event +from tests.utils import test_timeout + +logger = logging.getLogger(__name__) + +T = TypeVar("T") + + +@attr.s(slots=True, auto_attribs=True) +class RemoteRoomKnockResult: + remote_room_id: str + room_version: RoomVersion + remote_room_creator_user_id: str + local_user1_id: str + local_user1_tok: str + room_create_event: EventBase + knock_event_id: str + + +class KnockViaServerTestCase(unittest.FederatingHomeserverTestCase): + """ + Tests for the knocking server's side of MSC4233: + + - the server that fulfilled our /send_knock is remembered + - a rescission of the knock is routed through that server + - a denial of the knock sent to us by that server is accepted as an + out-of-band retraction of the knock + """ + + servlets = [ + admin.register_servlets, + knock.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def default_config(self) -> JsonDict: + conf = super().default_config() + conf["experimental_features"] = {"msc4233_enabled": True} + return conf + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + self.federation_http_client = Mock() + return self.setup_test_homeserver( + federation_http_client=self.federation_http_client + ) + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + + self.store = self.hs.get_datastores().main + + def _knock_on_remote_room(self) -> RemoteRoomKnockResult: + """Have a local user knock on a (mocked) remote room via the remote + server, and return the details of the knock.""" + + local_user1_id = self.register_user("user1", "pass") + local_user1_tok = self.login(local_user1_id, "pass") + + room_creator_user_id = f"@remote-user:{self.OTHER_SERVER_NAME}" + remote_room_id = f"!remote-room:{self.OTHER_SERVER_NAME}" + room_version = RoomVersions.V10 + + room_create_event = make_test_event( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": remote_room_id, + "sender": room_creator_user_id, + "depth": 1, + "origin_server_ts": 1, + "type": EventTypes.Create, + "state_key": "", + "content": { + EventContentFields.ROOM_CREATOR: room_creator_user_id, + EventContentFields.ROOM_VERSION: room_version.identifier, + }, + "auth_events": [], + "prev_events": [], + } + ), + room_version=room_version, + ) + + # A knock event template, as the remote server would return from + # /make_knock. + knock_event_template = { + "room_id": remote_room_id, + "sender": local_user1_id, + "depth": 2, + "origin_server_ts": 2, + "type": EventTypes.Member, + "state_key": local_user1_id, + "content": {"membership": Membership.KNOCK}, + "auth_events": [room_create_event.event_id], + "prev_events": [room_create_event.event_id], + } + + async def get_json( + destination: str, + path: str, + args: QueryParams | None = None, + retry_on_dns_fail: bool = True, + timeout: int | None = None, + ignore_backoff: bool = False, + try_trailing_slash_on_400: bool = False, + parser: ByteParser[T] | None = None, + ) -> JsonDict | T: + if path.startswith( + f"/_matrix/federation/v1/make_knock/{urllib.parse.quote_plus(remote_room_id)}/{urllib.parse.quote_plus(local_user1_id)}" + ): + return { + "event": knock_event_template, + "room_version": room_version.identifier, + } + + raise NotImplementedError( + f"Unmocked `get_json(...)` endpoint: {destination}{path}" + ) + + self.federation_http_client.get_json.side_effect = get_json + + # Record which server(s) we called /send_knock on. + send_knock_destinations: list[str] = [] + + async def put_json( + destination: str, + path: str, + args: QueryParams | None = None, + data: JsonDict | None = None, + json_data_callback: Callable[[], JsonDict] | None = None, + long_retries: bool = False, + timeout: int | None = None, + ignore_backoff: bool = False, + backoff_on_404: bool = False, + try_trailing_slash_on_400: bool = False, + parser: ByteParser[T] | None = None, + backoff_on_all_error_codes: bool = False, + ) -> JsonDict | T: + if path.startswith( + f"/_matrix/federation/v1/send_knock/{urllib.parse.quote_plus(remote_room_id)}/" + ): + send_knock_destinations.append(destination) + return {"knock_room_state": [strip_event(room_create_event)]} + + raise NotImplementedError( + f"Unmocked `put_json(...)` endpoint: {destination}{path} with body {data}" + ) + + self.federation_http_client.put_json.side_effect = put_json + + # User1 knocks on the remote room + channel = self.make_request( + "POST", + f"/_matrix/client/v3/knock/{urllib.parse.quote_plus(remote_room_id)}?server_name={self.OTHER_SERVER_NAME}", + {}, + access_token=local_user1_tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # Reset the mocks now that the knock has completed + self.federation_http_client.get_json.side_effect = None + self.federation_http_client.put_json.side_effect = None + + self.assertEqual(send_knock_destinations, [self.OTHER_SERVER_NAME]) + + # Our local membership should now be an (out-of-band) knock + membership, knock_event_id = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + local_user1_id, remote_room_id + ) + ) + self.assertEqual(membership, Membership.KNOCK) + assert knock_event_id is not None + + return RemoteRoomKnockResult( + remote_room_id=remote_room_id, + room_version=room_version, + remote_room_creator_user_id=room_creator_user_id, + local_user1_id=local_user1_id, + local_user1_tok=local_user1_tok, + room_create_event=room_create_event, + knock_event_id=knock_event_id, + ) + + def test_knock_remembers_via_server(self) -> None: + """The server that fulfilled our /send_knock is recorded.""" + knock_result = self._knock_on_remote_room() + + via_server = self.get_success( + self.store.get_local_knock_via_server( + knock_result.local_user1_id, knock_result.remote_room_id + ) + ) + self.assertEqual(via_server, self.OTHER_SERVER_NAME) + + def test_rescind_knock_routed_through_via_server(self) -> None: + """Rescinding a knock does the make_leave/send_leave dance through the + server the knock was fulfilled through.""" + knock_result = self._knock_on_remote_room() + remote_room_id = knock_result.remote_room_id + local_user1_id = knock_result.local_user1_id + + # A leave event template, as the remote server would return from + # /make_leave. + leave_event_template = { + "room_id": remote_room_id, + "sender": local_user1_id, + "depth": 3, + "origin_server_ts": 3, + "type": EventTypes.Member, + "state_key": local_user1_id, + "content": {"membership": Membership.LEAVE}, + "auth_events": [ + knock_result.room_create_event.event_id, + knock_result.knock_event_id, + ], + "prev_events": [knock_result.knock_event_id], + } + + make_leave_destinations: list[str] = [] + send_leave_destinations: list[str] = [] + + async def get_json( + destination: str, + path: str, + args: QueryParams | None = None, + retry_on_dns_fail: bool = True, + timeout: int | None = None, + ignore_backoff: bool = False, + try_trailing_slash_on_400: bool = False, + parser: ByteParser[T] | None = None, + ) -> JsonDict | T: + if path.startswith( + f"/_matrix/federation/v1/make_leave/{urllib.parse.quote_plus(remote_room_id)}/{urllib.parse.quote_plus(local_user1_id)}" + ): + make_leave_destinations.append(destination) + return { + "event": leave_event_template, + "room_version": knock_result.room_version.identifier, + } + + raise NotImplementedError( + f"Unmocked `get_json(...)` endpoint: {destination}{path}" + ) + + self.federation_http_client.get_json.side_effect = get_json + + async def put_json( + destination: str, + path: str, + args: QueryParams | None = None, + data: JsonDict | None = None, + json_data_callback: Callable[[], JsonDict] | None = None, + long_retries: bool = False, + timeout: int | None = None, + ignore_backoff: bool = False, + backoff_on_404: bool = False, + try_trailing_slash_on_400: bool = False, + parser: ByteParser[T] | None = None, + backoff_on_all_error_codes: bool = False, + ) -> JsonDict | T: + if path.startswith( + f"/_matrix/federation/v2/send_leave/{urllib.parse.quote_plus(remote_room_id)}/" + ): + send_leave_destinations.append(destination) + return {} + + raise NotImplementedError( + f"Unmocked `put_json(...)` endpoint: {destination}{path} with body {data}" + ) + + self.federation_http_client.put_json.side_effect = put_json + + # User1 rescinds the knock + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{urllib.parse.quote_plus(remote_room_id)}/leave", + {}, + access_token=knock_result.local_user1_tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # The rescission should have been routed through the server we + # knocked through. + self.assertEqual(make_leave_destinations, [self.OTHER_SERVER_NAME]) + self.assertEqual(send_leave_destinations, [self.OTHER_SERVER_NAME]) + + # And our local membership should now be leave. + membership, _ = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + local_user1_id, remote_room_id + ) + ) + self.assertEqual(membership, Membership.LEAVE) + + def test_rescind_knock_falls_back_to_local_leave(self) -> None: + """If the server we knocked through is unreachable, the knock is still + rescinded locally.""" + knock_result = self._knock_on_remote_room() + + async def get_json(*args: Any, **kwargs: Any) -> JsonDict: + raise RuntimeError("server unreachable") + + self.federation_http_client.get_json.side_effect = get_json + + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{urllib.parse.quote_plus(knock_result.remote_room_id)}/leave", + {}, + access_token=knock_result.local_user1_tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + membership, _ = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + knock_result.local_user1_id, knock_result.remote_room_id + ) + ) + self.assertEqual(membership, Membership.LEAVE) + + def _send_denial_over_federation( + self, knock_result: RemoteRoomKnockResult, auth_events: list[str] + ) -> None: + """Send a leave event denying the knock to our server, as the remote + server we knocked through.""" + deny_event = make_test_event( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": knock_result.remote_room_id, + "sender": knock_result.remote_room_creator_user_id, + "depth": 3, + "origin_server_ts": 3, + "type": EventTypes.Member, + "state_key": knock_result.local_user1_id, + "content": {"membership": Membership.LEAVE}, + "auth_events": auth_events, + "prev_events": [knock_result.knock_event_id], + } + ), + room_version=knock_result.room_version, + ) + + channel = self.make_signed_federation_request( + "PUT", + "/_matrix/federation/v1/send/txn_deny_knock", + {"pdus": [deny_event.get_pdu_json()]}, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + def test_denied_knock_accepted_from_via_server(self) -> None: + """A leave event for our knocked user, referencing the knock in its + auth events and sent by the server we knocked through, retracts the + knock.""" + knock_result = self._knock_on_remote_room() + + self._send_denial_over_federation( + knock_result, + auth_events=[ + knock_result.room_create_event.event_id, + knock_result.knock_event_id, + ], + ) + + # The knock should (eventually - the PDU is processed in the + # background) be retracted. + with test_timeout(3, "Denial of the knock was not processed"): + while True: + membership, _ = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + knock_result.local_user1_id, knock_result.remote_room_id + ) + ) + if membership == Membership.LEAVE: + break + time.sleep(0.1) + + def test_denied_knock_ignored_without_knock_in_auth_events(self) -> None: + """A leave event for our knocked user which does not reference the + knock in its auth events is ignored.""" + knock_result = self._knock_on_remote_room() + + self._send_denial_over_federation( + knock_result, + auth_events=[knock_result.room_create_event.event_id], + ) + + # Pump the reactor to let the (ignored) PDU get processed. + self.pump(1) + + membership, _ = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + knock_result.local_user1_id, knock_result.remote_room_id + ) + ) + self.assertEqual(membership, Membership.KNOCK) + + +class DenyKnockFederationSendTestCase(unittest.FederatingHomeserverTestCase): + """ + Tests for the resident server's side of MSC4233: a leave event denying a + knock is sent to the knocking user's server, which is otherwise not in + the room. + """ + + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def default_config(self) -> JsonDict: + conf = super().default_config() + conf["experimental_features"] = {"msc4233_enabled": True} + # Federation sending is disabled by default in the test environment + # so we need to enable it like this. + conf["federation_sender_instances"] = ["master"] + return conf + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + self.federation_http_client = Mock() + return self.setup_test_homeserver( + federation_http_client=self.federation_http_client + ) + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + self.store = homeserver.get_datastores().main + + # We're not going to be properly signing events as our remote + # homeserver is fake, therefore disable event signature checks. + async def approve_all_signature_checking( + room_version: RoomVersion, + pdu: EventBase, + record_failure_callback: Any = None, + ) -> EventBase: + return pdu + + homeserver.get_federation_server()._check_sigs_and_hash = ( # type: ignore[method-assign] + approve_all_signature_checking + ) + + async def _check_event_auth( + origin: str | None, event: EventBase, context: EventContext + ) -> None: + pass + + homeserver.get_federation_event_handler()._check_event_auth = _check_event_auth # type: ignore[method-assign] + + return super().prepare(reactor, clock, homeserver) + + def test_deny_sends_leave_to_knocking_server(self) -> None: + """Kicking a remote user whose membership is knock sends the leave + event to their (otherwise uninvolved) server.""" + user_id = self.register_user("u1", "pass") + user_token = self.login("u1", "pass") + + fake_knocking_user_id = f"@user:{self.OTHER_SERVER_NAME}" + + # Create a knockable room + room_id = self.helper.create_room_as( + "u1", + is_public=False, + room_version=RoomVersions.V10.identifier, + tok=user_token, + ) + self.helper.send_state( + room_id, + EventTypes.JoinRules, + {"join_rule": "knock"}, + tok=user_token, + ) + + # Collect the PDUs that our server sends out, per destination. + sent_pdus_by_destination: dict[str, list[JsonDict]] = {} + + async def put_json( + destination: str, + path: str, + args: QueryParams | None = None, + data: JsonDict | None = None, + json_data_callback: Callable[[], JsonDict] | None = None, + long_retries: bool = False, + timeout: int | None = None, + ignore_backoff: bool = False, + backoff_on_404: bool = False, + try_trailing_slash_on_400: bool = False, + parser: ByteParser[T] | None = None, + backoff_on_all_error_codes: bool = False, + ) -> JsonDict | T: + if path.startswith("/_matrix/federation/v1/send/") and data is not None: + pdus = data.get("pdus", []) + sent_pdus_by_destination.setdefault(destination, []).extend(pdus) + return {} + + raise NotImplementedError( + f"Unmocked `put_json(...)` endpoint: {destination}{path}" + ) + + self.federation_http_client.put_json.side_effect = put_json + + # The remote user knocks on the room, over federation. + channel = self.make_signed_federation_request( + "GET", + "/_matrix/federation/v1/make_knock/%s/%s?ver=%s" + % ( + room_id, + fake_knocking_user_id, + RoomVersions.V10.identifier, + ), + ) + self.assertEqual(200, channel.code, channel.result) + knock_event = channel.json_body["event"] + + signed_knock_event = builder.create_local_event_from_event_dict( + self.clock, + self.hs.hostname, + self.hs.signing_key, + room_version=RoomVersions.V10, + event_dict=knock_event, + ) + channel = self.make_signed_federation_request( + "PUT", + "/_matrix/federation/v1/send_knock/%s/%s" + % (room_id, signed_knock_event.event_id), + signed_knock_event.get_pdu_json(self.clock.time_msec()), + ) + self.assertEqual(200, channel.code, channel.result) + + # The local user denies the knock. + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{urllib.parse.quote_plus(room_id)}/kick", + {"user_id": fake_knocking_user_id}, + access_token=user_token, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + # The leave event should be sent to the knocking user's server, even + # though it has no other users in the room. + with test_timeout(3, "Leave event was not sent to the knocking server"): + while True: + leave_pdus = [ + pdu + for pdu in sent_pdus_by_destination.get( + self.OTHER_SERVER_NAME, [] + ) + if pdu.get("type") == EventTypes.Member + and pdu.get("state_key") == fake_knocking_user_id + and pdu.get("content", {}).get("membership") == Membership.LEAVE + ] + if leave_pdus: + break + time.sleep(0.1) + self.pump(0.1) From 962148d40441377d59587d2ada6ab1a444d82b56 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 Jul 2026 22:45:58 +0100 Subject: [PATCH 2/4] Stamp replaced knock into unsigned on an out-of-band denial Outliers never gain replaces_state, so clients could not tell a denied knock (prev_content.membership == knock) from a plain kick. Reflect the replaced knock event in the denial's unsigned ourselves. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Uk8aPxHn3BHCe52L226jdG --- synapse/handlers/federation_event.py | 12 ++++++++++++ .../federation/test_federation_knock_msc4233.py | 16 +++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 2ba321eacdd..d0556f11e60 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -323,6 +323,18 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: pdu.state_key, pdu.room_id ) if origin == via_server or origin == get_domain_from_id(pdu.sender): + # As an outlier the event will never gain a + # `replaces_state`, so reflect the replaced knock in + # `unsigned` ourselves: clients use `prev_content` to + # distinguish a denied knock from a plain kick. + knock_event = await self._store.get_event( + membership_event_id, allow_none=True + ) + if knock_event is not None: + pdu.unsigned["replaces_state"] = knock_event.event_id + pdu.unsigned["prev_content"] = knock_event.content + pdu.unsigned["prev_sender"] = knock_event.sender + # Handle the denial event pdu.internal_metadata.outlier = True pdu.internal_metadata.out_of_band_membership = True diff --git a/tests/federation/test_federation_knock_msc4233.py b/tests/federation/test_federation_knock_msc4233.py index 4f01fd1d058..6900d870318 100644 --- a/tests/federation/test_federation_knock_msc4233.py +++ b/tests/federation/test_federation_knock_msc4233.py @@ -405,9 +405,10 @@ def test_denied_knock_accepted_from_via_server(self) -> None: # The knock should (eventually - the PDU is processed in the # background) be retracted. + leave_event_id = None with test_timeout(3, "Denial of the knock was not processed"): while True: - membership, _ = self.get_success( + membership, leave_event_id = self.get_success( self.store.get_local_current_membership_for_user_in_room( knock_result.local_user1_id, knock_result.remote_room_id ) @@ -416,6 +417,19 @@ def test_denied_knock_accepted_from_via_server(self) -> None: break time.sleep(0.1) + # The stored leave should carry the replaced knock in `unsigned`, so + # that clients can tell a denied knock from a plain kick. + assert leave_event_id is not None + leave_event = self.get_success(self.store.get_event(leave_event_id)) + self.assertEqual( + leave_event.unsigned.get("prev_content", {}).get("membership"), + Membership.KNOCK, + ) + self.assertEqual( + leave_event.unsigned.get("replaces_state"), + knock_result.knock_event_id, + ) + def test_denied_knock_ignored_without_knock_in_auth_events(self) -> None: """A leave event for our knocked user which does not reference the knock in its auth events is ignored.""" From 6e4ff005590e11fee63254188757f0e05ee15e1e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 14 Jul 2026 01:51:44 +0100 Subject: [PATCH 3/4] Support denying a knock with a ban over federation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSC2403 gives a ban the same retracting effect as a kick. The sender and receive sides already handled BAN; the storage layer did not — extend the out-of-band membership assertions and the sliding-sync snapshot handling (a ban denying a knock inherits the knock's metadata, like a leave). Adds sender- and receive-side ban unit tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Uk8aPxHn3BHCe52L226jdG --- synapse/storage/databases/main/events.py | 15 +- .../test_federation_knock_msc4233.py | 135 +++++++++++++++--- 2 files changed, 121 insertions(+), 29 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 84b38f4bf2a..89904aa6b27 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -3129,13 +3129,15 @@ def _store_room_members_txn( and event.internal_metadata.is_out_of_band_membership() ): # The only sort of out-of-band-membership events we expect to see here - # are remote invites/knocks and LEAVE events corresponding to - # rejected/retracted invites and rescinded knocks. + # are remote invites/knocks, LEAVE events corresponding to + # rejected/retracted invites and rescinded knocks, and LEAVE/BAN + # events corresponding to denied knocks (MSC4233). assert event.type == EventTypes.Member assert event.membership in ( Membership.INVITE, Membership.KNOCK, Membership.LEAVE, + Membership.BAN, ) self.db_pool.simple_upsert_txn( @@ -3170,10 +3172,11 @@ def _store_room_members_txn( "event_stream_ordering": event.internal_metadata.stream_ordering, "event_instance_name": event.internal_metadata.instance_name, } - if event.membership == Membership.LEAVE: - # Inherit the meta data from the remote invite/knock. When using - # sliding sync filters, this will prevent the room from - # disappearing/appearing just because you left the room. + if event.membership in (Membership.LEAVE, Membership.BAN): + # Inherit the meta data from the remote invite/knock (a BAN here is + # a denied knock, MSC4233). When using sliding sync filters, this + # will prevent the room from disappearing/appearing just because + # you left the room. pass elif event.membership in (Membership.INVITE, Membership.KNOCK): extra_insert_values = ( diff --git a/tests/federation/test_federation_knock_msc4233.py b/tests/federation/test_federation_knock_msc4233.py index 6900d870318..b96629a313b 100644 --- a/tests/federation/test_federation_knock_msc4233.py +++ b/tests/federation/test_federation_knock_msc4233.py @@ -361,10 +361,13 @@ async def get_json(*args: Any, **kwargs: Any) -> JsonDict: self.assertEqual(membership, Membership.LEAVE) def _send_denial_over_federation( - self, knock_result: RemoteRoomKnockResult, auth_events: list[str] + self, + knock_result: RemoteRoomKnockResult, + auth_events: list[str], + membership: str = Membership.LEAVE, ) -> None: - """Send a leave event denying the knock to our server, as the remote - server we knocked through.""" + """Send a leave (or ban) event denying the knock to our server, as the + remote server we knocked through.""" deny_event = make_test_event( self.add_hashes_and_signatures_from_other_server( { @@ -374,7 +377,7 @@ def _send_denial_over_federation( "origin_server_ts": 3, "type": EventTypes.Member, "state_key": knock_result.local_user1_id, - "content": {"membership": Membership.LEAVE}, + "content": {"membership": membership}, "auth_events": auth_events, "prev_events": [knock_result.knock_event_id], } @@ -430,6 +433,42 @@ def test_denied_knock_accepted_from_via_server(self) -> None: knock_result.knock_event_id, ) + def test_banned_knock_accepted_from_via_server(self) -> None: + """A ban event for our knocked user (deny + ban, per MSC2403 a ban has + the same retracting effect), referencing the knock in its auth events + and sent by the server we knocked through, retracts the knock and + leaves us banned.""" + knock_result = self._knock_on_remote_room() + + self._send_denial_over_federation( + knock_result, + auth_events=[ + knock_result.room_create_event.event_id, + knock_result.knock_event_id, + ], + membership=Membership.BAN, + ) + + ban_event_id = None + with test_timeout(3, "Ban of the knocked user was not processed"): + while True: + membership, ban_event_id = self.get_success( + self.store.get_local_current_membership_for_user_in_room( + knock_result.local_user1_id, knock_result.remote_room_id + ) + ) + if membership == Membership.BAN: + break + time.sleep(0.1) + + # The stored ban should carry the replaced knock in `unsigned`. + assert ban_event_id is not None + ban_event = self.get_success(self.store.get_event(ban_event_id)) + self.assertEqual( + ban_event.unsigned.get("prev_content", {}).get("membership"), + Membership.KNOCK, + ) + def test_denied_knock_ignored_without_knock_in_auth_events(self) -> None: """A leave event for our knocked user which does not reference the knock in its auth events is ignored.""" @@ -505,10 +544,15 @@ async def _check_event_auth( return super().prepare(reactor, clock, homeserver) - def test_deny_sends_leave_to_knocking_server(self) -> None: - """Kicking a remote user whose membership is knock sends the leave - event to their (otherwise uninvolved) server.""" - user_id = self.register_user("u1", "pass") + def _setup_room_with_pending_federated_knock( + self, + ) -> tuple[str, str, str, dict[str, list[JsonDict]]]: + """Create a knockable room, receive a federated knock into it, and + start collecting outbound PDUs. + + Returns (room_id, admin_token, knocking_user_id, sent_pdus_by_destination). + """ + self.register_user("u1", "pass") user_token = self.login("u1", "pass") fake_knocking_user_id = f"@user:{self.OTHER_SERVER_NAME}" @@ -583,6 +627,40 @@ async def put_json( ) self.assertEqual(200, channel.code, channel.result) + return room_id, user_token, fake_knocking_user_id, sent_pdus_by_destination + + def _expect_membership_sent_to_knocking_server( + self, + sent_pdus_by_destination: dict[str, list[JsonDict]], + knocking_user_id: str, + membership: str, + ) -> None: + """Wait until a membership event of the given kind for the knocking + user has been sent to their (otherwise uninvolved) server.""" + with test_timeout(3, f"{membership} event was not sent to the knocking server"): + while True: + pdus = [ + pdu + for pdu in sent_pdus_by_destination.get(self.OTHER_SERVER_NAME, []) + if pdu.get("type") == EventTypes.Member + and pdu.get("state_key") == knocking_user_id + and pdu.get("content", {}).get("membership") == membership + ] + if pdus: + break + time.sleep(0.1) + self.pump(0.1) + + def test_deny_sends_leave_to_knocking_server(self) -> None: + """Kicking a remote user whose membership is knock sends the leave + event to their (otherwise uninvolved) server.""" + ( + room_id, + user_token, + fake_knocking_user_id, + sent_pdus_by_destination, + ) = self._setup_room_with_pending_federated_knock() + # The local user denies the knock. channel = self.make_request( "POST", @@ -594,18 +672,29 @@ async def put_json( # The leave event should be sent to the knocking user's server, even # though it has no other users in the room. - with test_timeout(3, "Leave event was not sent to the knocking server"): - while True: - leave_pdus = [ - pdu - for pdu in sent_pdus_by_destination.get( - self.OTHER_SERVER_NAME, [] - ) - if pdu.get("type") == EventTypes.Member - and pdu.get("state_key") == fake_knocking_user_id - and pdu.get("content", {}).get("membership") == Membership.LEAVE - ] - if leave_pdus: - break - time.sleep(0.1) - self.pump(0.1) + self._expect_membership_sent_to_knocking_server( + sent_pdus_by_destination, fake_knocking_user_id, Membership.LEAVE + ) + + def test_ban_sends_ban_to_knocking_server(self) -> None: + """Banning a remote user whose membership is knock (deny + ban) sends + the ban event to their (otherwise uninvolved) server.""" + ( + room_id, + user_token, + fake_knocking_user_id, + sent_pdus_by_destination, + ) = self._setup_room_with_pending_federated_knock() + + # The local user bans the knocker. + channel = self.make_request( + "POST", + f"/_matrix/client/v3/rooms/{urllib.parse.quote_plus(room_id)}/ban", + {"user_id": fake_knocking_user_id}, + access_token=user_token, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + self._expect_membership_sent_to_knocking_server( + sent_pdus_by_destination, fake_knocking_user_id, Membership.BAN + ) From fb3d9792519f8345d3069460525634046a9d3c2a Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 14 Jul 2026 16:54:17 +0100 Subject: [PATCH 4/4] fix changelog --- changelog.d/{18030.feature => 19956.feature} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{18030.feature => 19956.feature} (100%) diff --git a/changelog.d/18030.feature b/changelog.d/19956.feature similarity index 100% rename from changelog.d/18030.feature rename to changelog.d/19956.feature