From 3d960d88b38c93fa82be2e98e57c9f146460f056 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 18 Mar 2026 07:53:13 -0500 Subject: [PATCH 01/11] Add MSC3820 comment context to `RoomVersion` attributes (#19577) Spawning from https://github.com/element-hq/synapse/pull/19424#discussion_r2855303614 --- changelog.d/19577.misc | 1 + synapse/api/room_versions.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changelog.d/19577.misc diff --git a/changelog.d/19577.misc b/changelog.d/19577.misc new file mode 100644 index 0000000000..d26790e490 --- /dev/null +++ b/changelog.d/19577.misc @@ -0,0 +1 @@ +Add MSC3820 comment context to `RoomVersion` attributes. diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index 2f98d7a8a8..13c7758638 100644 --- a/synapse/api/room_versions.py +++ b/synapse/api/room_versions.py @@ -86,9 +86,9 @@ class RoomVersion: # MSC2209: Check 'notifications' key while verifying # m.room.power_levels auth rules. limit_notifications_power_levels: bool - # No longer include the creator in m.room.create events. + # MSC3820: No longer include the creator in m.room.create events (room version 11) implicit_room_creator: bool - # Apply updated redaction rules algorithm from room version 11. + # MSC3820: Apply updated redaction rules algorithm from room version 11 updated_redaction_rules: bool # Support the 'restricted' join rule. restricted_join_rule: bool From 8201e58767333aca988b42ef0fee633d329a5b23 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Wed, 18 Mar 2026 16:29:36 +0200 Subject: [PATCH 02/11] Update and stabilize mutual rooms support (MSC2666) (#19511) Updates the error codes to match MSC2666 changes (user ID query param validation + proper errcode for requesting rooms with self), added the new `count` field, and stabilized the endpoint. --- changelog.d/19511.feature | 1 + synapse/config/experimental.py | 3 -- synapse/rest/client/mutual_rooms.py | 35 ++++++++++------ synapse/rest/client/versions.py | 2 +- synapse/types/__init__.py | 58 ++++++++++++++++++++++++-- tests/rest/client/test_mutual_rooms.py | 54 ++++++++++++++---------- 6 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 changelog.d/19511.feature diff --git a/changelog.d/19511.feature b/changelog.d/19511.feature new file mode 100644 index 0000000000..bb9a38b9c1 --- /dev/null +++ b/changelog.d/19511.feature @@ -0,0 +1 @@ +Update and stabilize support for [MSC2666](https://github.com/matrix-org/matrix-spec-proposals/pull/2666): Get rooms in common with another user. Contributed by @tulir @ Beeper. diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 41efff1d8f..0e96b1dc6a 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -422,9 +422,6 @@ class ExperimentalConfig(Config): # previously calculated push actions. self.msc2654_enabled: bool = experimental.get("msc2654_enabled", False) - # MSC2666: Query mutual rooms between two users. - self.msc2666_enabled: bool = experimental.get("msc2666_enabled", False) - # MSC2815 (allow room moderators to view redacted event content) self.msc2815_enabled: bool = experimental.get("msc2815_enabled", False) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index a6a913db34..d6783c15da 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -29,7 +29,7 @@ from synapse.api.errors import Codes, SynapseError from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_strings_from_args from synapse.http.site import SynapseRequest -from synapse.types import JsonDict +from synapse.types import JsonDict, UserID from ._base import client_patterns @@ -65,13 +65,10 @@ def _parse_mutual_rooms_batch_token_args(args: dict[bytes, list[bytes]]) -> str class UserMutualRoomsServlet(RestServlet): """ - GET /uk.half-shot.msc2666/user/mutual_rooms?user_id={user_id}&from={token} HTTP/1.1 + GET /mutual_rooms?user_id={user_id}&from={token} HTTP/1.1 """ - PATTERNS = client_patterns( - "/uk.half-shot.msc2666/user/mutual_rooms$", - releases=(), # This is an unstable feature - ) + PATTERNS = [*client_patterns("/mutual_rooms$", releases=("v1",))] def __init__(self, hs: "HomeServer"): super().__init__() @@ -82,7 +79,9 @@ class UserMutualRoomsServlet(RestServlet): # twisted.web.server.Request.args is incorrectly defined as Any | None args: dict[bytes, list[bytes]] = request.args # type: ignore - user_ids = parse_strings_from_args(args, "user_id", required=True) + user_ids = parse_strings_from_args( + args, "user_id", required=True, encoding="utf-8" + ) from_batch = _parse_mutual_rooms_batch_token_args(args) if len(user_ids) > 1: @@ -93,13 +92,19 @@ class UserMutualRoomsServlet(RestServlet): ) user_id = user_ids[0] + if not UserID.is_valid_strict(user_id): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "Invalid user_id query parameter", + errcode=Codes.INVALID_PARAM, + ) requester = await self.auth.get_user_by_req(request) if user_id == requester.user.to_string(): raise SynapseError( HTTPStatus.BAD_REQUEST, "You cannot request a list of shared rooms with yourself", - errcode=Codes.UNKNOWN, + errcode=Codes.INVALID_PARAM, ) # Sort here instead of the database function, so that we don't expose @@ -109,6 +114,7 @@ class UserMutualRoomsServlet(RestServlet): frozenset((requester.user.to_string(), user_id)) ) ) + total_count = len(rooms) if from_batch: # A from_batch token was provided, so cut off any rooms where the ID is @@ -123,7 +129,7 @@ class UserMutualRoomsServlet(RestServlet): if len(rooms) <= MUTUAL_ROOMS_BATCH_LIMIT: # We've reached the end of the list, don't return a batch token - return 200, {"joined": rooms} + return 200, {"joined": rooms, "count": total_count} rooms = rooms[:MUTUAL_ROOMS_BATCH_LIMIT] # We use urlsafe unpadded base64 encoding for the batch token in order to @@ -135,11 +141,14 @@ class UserMutualRoomsServlet(RestServlet): # in the room ID. In the event that some silly user does that, don't let # them paginate further. if next_batch == from_batch: - return 200, {"joined": rooms} + return 200, {"joined": rooms, "count": total_count} - return 200, {"joined": list(rooms), "next_batch": next_batch} + return 200, { + "joined": rooms, + "next_batch": next_batch, + "count": total_count, + } def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: - if hs.config.experimental.msc2666_enabled: - UserMutualRoomsServlet(hs).register(http_server) + UserMutualRoomsServlet(hs).register(http_server) diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index f8d7a1a4d9..904da86794 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -144,7 +144,7 @@ class VersionsRestServlet(RestServlet): # Implements additional endpoints as described in MSC2432 "org.matrix.msc2432": True, # Implements additional endpoints as described in MSC2666 - "uk.half-shot.msc2666.query_mutual_rooms": self.config.experimental.msc2666_enabled, + "uk.half-shot.msc2666.query_mutual_rooms.stable": True, # Whether new rooms will be set to encrypted or not (based on presets). "io.element.e2ee_forced.public": self.e2ee_forced_public, "io.element.e2ee_forced.private": self.e2ee_forced_private, diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index fb1f1192b7..02889795bb 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -343,7 +343,7 @@ class DomainSpecificString(metaclass=abc.ABCMeta): # possible for invalid data to exist in room-state, etc. parse_and_validate_server_name(obj.domain) return True - except Exception: + except (SynapseError, ValueError): return False __repr__ = to_string @@ -355,6 +355,29 @@ class UserID(DomainSpecificString): SIGIL = "@" + @classmethod + def is_valid_strict(cls, s: str) -> bool: + """ + Parses the input string and attempts to ensure it is a valid and compliant user + ID according to https://spec.matrix.org/v1.17/appendices/#historical-user-ids. + + This should be used with care: there are existing non-compliant user IDs in the + wild with empty or non-ASCII localparts, which will be rejected by this method. + """ + if len(s.encode("utf-8")) > 255: + return False + try: + obj = cls.from_string(s) + if not is_compliant_user_id_localpart(obj.localpart): + return False + # Apply additional validation to the domain. This is only done + # during is_valid (and not part of from_string) since it is + # possible for invalid data to exist in room-state, etc. + parse_and_validate_server_name(obj.domain) + return True + except (SynapseError, ValueError): + return False + @attr.s(slots=True, frozen=True, repr=False) class RoomAlias(DomainSpecificString): @@ -453,12 +476,17 @@ GUEST_USER_ID_PATTERN = re.compile(r"^\d+$") def contains_invalid_mxid_characters(localpart: str) -> bool: - """Check for characters not allowed in an mxid or groupid localpart + """ + Check for characters not allowed in a modern user ID localpart. + + This is primarily used for new registrations and MUST NOT be used to validate + existing user IDs, as there are real users whose user IDs don't follow this + character set. + + See https://spec.matrix.org/v1.17/appendices/#user-identifiers Args: localpart: the localpart to be checked - use_extended_character_set: True to use the extended allowed characters - from MSC4009. Returns: True if there are any naughty characters @@ -466,6 +494,28 @@ def contains_invalid_mxid_characters(localpart: str) -> bool: return any(c not in MXID_LOCALPART_ALLOWED_CHARACTERS for c in localpart) +def is_compliant_user_id_localpart(localpart: str) -> bool: + """ + Validates that the given user ID localpart is within the "compliant" range, + i.e. not empty and all characters are between U+0021 and U+007E inclusive. + See https://spec.matrix.org/v1.17/appendices/#historical-user-ids + + To check if a localpart is non-historical, use contains_invalid_mxid_characters instead. + + This should be used with care: there are existing non-compliant user IDs in the + wild with empty or non-ASCII localparts, which will be rejected by this method. + + Args: + localpart: the localpart to be checked + + Returns: + True if the localpart is compliant, False otherwise + """ + if not localpart: + return False + return all(0x21 <= ord(c) <= 0x7E for c in localpart) + + UPPER_CASE_PATTERN = re.compile(b"[A-Z_]") # the following is a pattern which matches '=', and bytes which are not allowed in a mxid diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index f78c67fcd9..cbaca17cdd 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -43,12 +43,6 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): mutual_rooms.register_servlets, ] - def default_config(self) -> dict: - config = super().default_config() - experimental = config.setdefault("experimental_features", {}) - experimental.setdefault("msc2666_enabled", True) - return config - def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() return self.setup_test_homeserver(config=config) @@ -62,27 +56,12 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): ) -> FakeChannel: return self.make_request( "GET", - "/_matrix/client/unstable/uk.half-shot.msc2666/user/mutual_rooms" + "/_matrix/client/v1/mutual_rooms" f"?user_id={quote(other_user)}" + (f"&from={quote(since_token)}" if since_token else ""), access_token=token, ) - @unittest.override_config({"experimental_features": {"msc2666_enabled": False}}) - def test_mutual_rooms_no_experimental_flag(self) -> None: - """ - The endpoint should 404 if the experimental flag is not enabled. - """ - # Register a user. - u1 = self.register_user("user1", "pass") - u1_token = self.login(u1, "pass") - - # Check that we're unable to query the endpoint due to the endpoint - # being unrecognised. - channel = self._get_mutual_rooms(u1_token, "@not-used:test") - self.assertEqual(404, channel.code, channel.result) - self.assertEqual("M_UNRECOGNIZED", channel.json_body["errcode"], channel.result) - def test_shared_room_list_public(self) -> None: """ A room should show up in the shared list of rooms between two users @@ -129,6 +108,7 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 1) + self.assertEqual(channel.json_body["count"], 1) self.assertEqual(channel.json_body["joined"][0], room_id_one) # Create another room and invite user2 to it @@ -142,6 +122,7 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 2) + self.assertEqual(channel.json_body["count"], 2) for room_id_id in channel.json_body["joined"]: self.assertIn(room_id_id, [room_id_one, room_id_two]) @@ -167,11 +148,13 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(channel.json_body["joined"], room_ids[0:10]) + self.assertEqual(channel.json_body["count"], 15) self.assertIn("next_batch", channel.json_body) channel = self._get_mutual_rooms(u1_token, u2, channel.json_body["next_batch"]) self.assertEqual(200, channel.code, channel.result) self.assertEqual(channel.json_body["joined"], room_ids[10:20]) + self.assertEqual(channel.json_body["count"], 15) self.assertNotIn("next_batch", channel.json_body) def test_shared_room_list_pagination_one_page(self) -> None: @@ -180,6 +163,7 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(channel.json_body["joined"], room_ids) + self.assertEqual(channel.json_body["count"], 10) self.assertNotIn("next_batch", channel.json_body) def test_shared_room_list_pagination_invalid_token(self) -> None: @@ -209,6 +193,7 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 1) + self.assertEqual(channel.json_body["count"], 1) self.assertEqual(channel.json_body["joined"][0], room) self.helper.leave(room, user=u1, tok=u1_token) @@ -217,11 +202,13 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 0) + self.assertEqual(channel.json_body["count"], 0) # Check user2's view of shared rooms with user1 channel = self._get_mutual_rooms(u2_token, u1) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 0) + self.assertEqual(channel.json_body["count"], 0) def test_shared_room_list_nonexistent_user(self) -> None: u1 = self.register_user("user1", "pass") @@ -232,4 +219,27 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): channel = self._get_mutual_rooms(u1_token, "@meow:example.com") self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 0) + self.assertEqual(channel.json_body["count"], 0) self.assertNotIn("next_batch", channel.json_body) + + def test_shared_room_list_invalid_user(self) -> None: + u1 = self.register_user("user1", "pass") + u1_token = self.login(u1, "pass") + + channel = self._get_mutual_rooms(u1_token, "@:example.com") + self.assertEqual(400, channel.code, channel.result) + self.assertEqual( + "M_INVALID_PARAM", channel.json_body["errcode"], channel.result + ) + + channel = self._get_mutual_rooms(u1_token, "@" + "a" * 255 + ":example.com") + self.assertEqual(400, channel.code, channel.result) + self.assertEqual( + "M_INVALID_PARAM", channel.json_body["errcode"], channel.result + ) + + channel = self._get_mutual_rooms(u1_token, "@🐈️:example.com") + self.assertEqual(400, channel.code, channel.result) + self.assertEqual( + "M_INVALID_PARAM", channel.json_body["errcode"], channel.result + ) From 261bfb786fd70c5d62237db291b090d606e9f723 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 18 Mar 2026 09:50:09 -0600 Subject: [PATCH 03/11] Fix zeroing out remote quarantined media count (#19559) Just something I noticed while working on https://github.com/element-hq/synapse/pull/19558 We start the function by setting `total_media_quarantined` to zero, then we do work on the `media_ids`, add the number affected, zero it out (**bug**), do work on `hashes`, add the number of affected rows, then return `total_media_quarantined`. ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --- changelog.d/19559.bugfix | 1 + synapse/storage/databases/main/room.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/19559.bugfix diff --git a/changelog.d/19559.bugfix b/changelog.d/19559.bugfix new file mode 100644 index 0000000000..f64a84e8e0 --- /dev/null +++ b/changelog.d/19559.bugfix @@ -0,0 +1 @@ +Fix quarantine media admin APIs sometimes returning inaccurate counts for remote media. \ No newline at end of file diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 633df07736..7ac88e4c2a 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1217,7 +1217,6 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): txn.execute(sql, [quarantined_by] + sql_args) total_media_quarantined += txn.rowcount if txn.rowcount > 0 else 0 - total_media_quarantined = 0 if hashes: sql_many_clause_sql, sql_many_clause_args = make_in_list_sql_clause( txn.database_engine, "sha256", hashes From edf5ce277adda9e0028decd0b9e131cd686b18d6 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 18 Mar 2026 19:47:17 +0100 Subject: [PATCH 04/11] Allow using HTTP/2 over plaintext when introspecting tokens with MAS (#19586) --- changelog.d/19586.feature | 1 + docs/usage/configuration/config_documentation.md | 2 ++ schema/synapse-config.schema.yaml | 7 +++++++ synapse/api/auth/mas.py | 1 + synapse/config/mas.py | 2 ++ 5 files changed, 13 insertions(+) create mode 100644 changelog.d/19586.feature diff --git a/changelog.d/19586.feature b/changelog.d/19586.feature new file mode 100644 index 0000000000..a10bd2d2ef --- /dev/null +++ b/changelog.d/19586.feature @@ -0,0 +1 @@ +Introduce a [configuration option](https://element-hq.github.io/synapse/latest/usage/configuration/config_documentation.html#matrix_authentication_service) to allow using HTTP/2 over plaintext when Synapse connects to Matrix Authentication Service. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 1335def585..48f33d5427 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -653,6 +653,8 @@ This setting has the following sub-options: * `endpoint` (string): The URL where Synapse can reach MAS. This *must* have the `discovery` and `oauth` resources mounted. Defaults to `"http://localhost:8080"`. +* `force_http2` (boolean): Force HTTP/2 over plaintext (H2C) when connecting to MAS. MAS supports this natively, but a reverse proxy between Synapse and MAS may not. Defaults to `false`. + * `secret` (string|null): A shared secret that will be used to authenticate requests from and to MAS. * `secret_path` (string|null): Alternative to `secret`, reading the shared secret from a file. The file should be a plain text file, containing only the secret. Synapse reads the secret from the given file once at startup. diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index dbf7d7acb7..aaa306cee0 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -677,6 +677,13 @@ properties: and `oauth` resources mounted. default: http://localhost:8080 + force_http2: + type: boolean + description: >- + Force HTTP/2 over plaintext (H2C) when connecting to MAS. MAS supports + this natively, but a reverse proxy between Synapse and MAS may not. + default: false + secret: type: ["string", "null"] description: >- diff --git a/synapse/api/auth/mas.py b/synapse/api/auth/mas.py index 79c15a5329..95c6d62a9d 100644 --- a/synapse/api/auth/mas.py +++ b/synapse/api/auth/mas.py @@ -111,6 +111,7 @@ class MasDelegatedAuth(BaseAuth): self._rust_http_client = HttpClient( reactor=hs.get_reactor(), user_agent=self._http_client.user_agent.decode("utf8"), + http2_only=self._config.force_http2, ) self._server_metadata = RetryOnExceptionCachedCall[ServerMetadata]( self._load_metadata diff --git a/synapse/config/mas.py b/synapse/config/mas.py index 104ba17ab7..6973e9ae58 100644 --- a/synapse/config/mas.py +++ b/synapse/config/mas.py @@ -36,6 +36,7 @@ from ._base import Config, ConfigError, RootConfig class MasConfigModel(ParseModel): enabled: StrictBool = False endpoint: AnyHttpUrl = AnyHttpUrl("http://localhost:8080") + force_http2: StrictBool = False secret: StrictStr | None = Field(default=None) # We set `strict=False` to allow `str` instances. secret_path: FilePath | None = Field(default=None, strict=False) @@ -82,6 +83,7 @@ class MasConfig(Config): self.enabled = parsed.enabled self.endpoint = parsed.endpoint + self.force_http2 = parsed.force_http2 self._secret = parsed.secret self._secret_path = parsed.secret_path From 90697637a8f37faf898350318a231813a942bd74 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 19:16:14 +0000 Subject: [PATCH 05/11] Bump actions/setup-go from 6.2.0 to 6.3.0 in the minor-and-patches group (#19564) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/latest_deps.yml | 2 +- .github/workflows/tests.yml | 2 +- .github/workflows/twisted_trunk.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/latest_deps.yml b/.github/workflows/latest_deps.yml index 2d945c2096..450be20956 100644 --- a/.github/workflows/latest_deps.yml +++ b/.github/workflows/latest_deps.yml @@ -207,7 +207,7 @@ jobs: - name: Prepare Complement's Prerequisites run: synapse/.ci/scripts/setup_complement_prerequisites.sh - - uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 + - uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0 with: cache-dependency-path: complement/go.sum go-version-file: complement/go.mod diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a03d27472d..987046b741 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -715,7 +715,7 @@ jobs: - name: Prepare Complement's Prerequisites run: synapse/.ci/scripts/setup_complement_prerequisites.sh - - uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 + - uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0 with: cache-dependency-path: complement/go.sum go-version-file: complement/go.mod diff --git a/.github/workflows/twisted_trunk.yml b/.github/workflows/twisted_trunk.yml index d38e38ebcb..74b4f11722 100644 --- a/.github/workflows/twisted_trunk.yml +++ b/.github/workflows/twisted_trunk.yml @@ -180,7 +180,7 @@ jobs: - name: Prepare Complement's Prerequisites run: synapse/.ci/scripts/setup_complement_prerequisites.sh - - uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 + - uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0 with: cache-dependency-path: complement/go.sum go-version-file: complement/go.mod From 9fe7cbfe7f69a69fb0f445f16a9753a611c24e58 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 19:24:28 +0000 Subject: [PATCH 06/11] Bump actions/upload-artifact from 6.0.0 to 7.0.0 (#19565) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/docker.yml | 2 +- .github/workflows/docs-pr.yaml | 2 +- .github/workflows/latest_deps.yml | 2 +- .github/workflows/release-artifacts.yml | 6 +++--- .github/workflows/tests.yml | 4 ++-- .github/workflows/twisted_trunk.yml | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index b1dd8a75e7..a796f9cf2a 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -108,7 +108,7 @@ jobs: touch "${{ runner.temp }}/digests/${digest#sha256:}" - name: Upload digest - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: digests-${{ matrix.suffix }} path: ${{ runner.temp }}/digests/* diff --git a/.github/workflows/docs-pr.yaml b/.github/workflows/docs-pr.yaml index 41f3664336..cba1293743 100644 --- a/.github/workflows/docs-pr.yaml +++ b/.github/workflows/docs-pr.yaml @@ -39,7 +39,7 @@ jobs: cp book/welcome_and_overview.html book/index.html - name: Upload Artifact - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: book path: book diff --git a/.github/workflows/latest_deps.yml b/.github/workflows/latest_deps.yml index 450be20956..a85bad9d74 100644 --- a/.github/workflows/latest_deps.yml +++ b/.github/workflows/latest_deps.yml @@ -172,7 +172,7 @@ jobs: if: ${{ always() }} run: /sytest/scripts/tap_to_gha.pl /logs/results.tap - name: Upload SyTest logs - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 if: ${{ always() }} with: name: Sytest Logs - ${{ job.status }} - (${{ join(matrix.*, ', ') }}) diff --git a/.github/workflows/release-artifacts.yml b/.github/workflows/release-artifacts.yml index 4343cb482f..125d659f4e 100644 --- a/.github/workflows/release-artifacts.yml +++ b/.github/workflows/release-artifacts.yml @@ -99,7 +99,7 @@ jobs: echo "ARTIFACT_NAME=${DISTRO#*:}" >> "$GITHUB_OUTPUT" - name: Upload debs as artifacts - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: debs-${{ steps.artifact-name.outputs.ARTIFACT_NAME }} path: debs/* @@ -150,7 +150,7 @@ jobs: # musl: (TODO: investigate). CIBW_TEST_SKIP: pp3*-* *musl* - - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: Wheel-${{ matrix.os }} path: ./wheelhouse/*.whl @@ -171,7 +171,7 @@ jobs: - name: Build sdist run: python -m build --sdist - - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: Sdist path: dist/*.tar.gz diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 987046b741..35c633b8ed 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -561,7 +561,7 @@ jobs: if: ${{ always() }} run: /sytest/scripts/tap_to_gha.pl /logs/results.tap - name: Upload SyTest logs - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 if: ${{ always() }} with: name: Sytest Logs - ${{ job.status }} - (${{ join(matrix.job.*, ', ') }}) @@ -658,7 +658,7 @@ jobs: PGPASSWORD: postgres PGDATABASE: postgres - name: "Upload schema differences" - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 if: ${{ failure() && !cancelled() && steps.run_tester_script.outcome == 'failure' }} with: name: Schema dumps diff --git a/.github/workflows/twisted_trunk.yml b/.github/workflows/twisted_trunk.yml index 74b4f11722..e39d64cfb2 100644 --- a/.github/workflows/twisted_trunk.yml +++ b/.github/workflows/twisted_trunk.yml @@ -145,7 +145,7 @@ jobs: if: ${{ always() }} run: /sytest/scripts/tap_to_gha.pl /logs/results.tap - name: Upload SyTest logs - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 if: ${{ always() }} with: name: Sytest Logs - ${{ job.status }} - (${{ join(matrix.*, ', ') }}) From f490c49c8580c6b404dd3dc311da30cc3cca9529 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 19 Mar 2026 10:54:04 +0000 Subject: [PATCH 07/11] Bump pyasn1 from 0.6.2 to 0.6.3 (#19584) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index 681a183b42..36d3035277 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2076,14 +2076,14 @@ psycopg2 = "*" [[package]] name = "pyasn1" -version = "0.6.2" +version = "0.6.3" description = "Pure-Python implementation of ASN.1 types and DER/BER/CER codecs (X.208)" optional = false python-versions = ">=3.8" groups = ["main"] files = [ - {file = "pyasn1-0.6.2-py3-none-any.whl", hash = "sha256:1eb26d860996a18e9b6ed05e7aae0e9fc21619fcee6af91cca9bad4fbea224bf"}, - {file = "pyasn1-0.6.2.tar.gz", hash = "sha256:9b59a2b25ba7e4f8197db7686c09fb33e658b98339fadb826e9512629017833b"}, + {file = "pyasn1-0.6.3-py3-none-any.whl", hash = "sha256:a80184d120f0864a52a073acc6fc642847d0be408e7c7252f31390c0f4eadcde"}, + {file = "pyasn1-0.6.3.tar.gz", hash = "sha256:697a8ecd6d98891189184ca1fa05d1bb00e2f84b5977c481452050549c8a72cf"}, ] [[package]] From 9edbf56969af7a4aa894237fa58bb1042947754b Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 19 Mar 2026 12:52:40 +0000 Subject: [PATCH 08/11] Prevent sending registration emails if registration is disabled (#19585) --- changelog.d/19585.misc | 1 + synapse/rest/client/register.py | 9 +++++++ tests/rest/client/test_register.py | 40 +++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 changelog.d/19585.misc diff --git a/changelog.d/19585.misc b/changelog.d/19585.misc new file mode 100644 index 0000000000..621cf782f1 --- /dev/null +++ b/changelog.d/19585.misc @@ -0,0 +1 @@ +Prevent sending registration emails if registration is disabled. \ No newline at end of file diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index fdd2f1985a..73832ba7a8 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -86,6 +86,7 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): self.server_name = hs.hostname self.identity_handler = hs.get_identity_handler() self.config = hs.config + self._registration_enabled = hs.config.registration.enable_registration if self.hs.config.email.can_verify_email: self.registration_mailer = Mailer( @@ -109,6 +110,14 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): raise SynapseError( 400, "Email-based registration has been disabled on this server" ) + + if not self._registration_enabled: + raise SynapseError( + 403, + "Registration is disabled on this homeserver", + Codes.FORBIDDEN, + ) + body = parse_json_object_from_request(request) assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) diff --git a/tests/rest/client/test_register.py b/tests/rest/client/test_register.py index 2c0396a3de..f66c56a6b1 100644 --- a/tests/rest/client/test_register.py +++ b/tests/rest/client/test_register.py @@ -22,7 +22,7 @@ import datetime import importlib.resources as importlib_resources import os -from typing import Any +from typing import Any, cast from unittest.mock import AsyncMock from twisted.internet.testing import MemoryReactor @@ -66,6 +66,17 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): hs.get_send_email_handler()._sendmail = AsyncMock() return hs + def _get_sendmail_mock(self) -> AsyncMock: + """ + Cast the homeserver's `_sendmail` object as an `AsyncMock`. + + `_sendmail` is an `AsyncMock` (see `make_homeserver`) but this type + information doesn't make it through the test harness. Thus we need to + cast the object again. + """ + sendmail = self.hs.get_send_email_handler()._sendmail + return cast(AsyncMock, sendmail) + def test_POST_appservice_registration_valid(self) -> None: user_id = "@as_user_kermit:test" as_token = "i_am_an_app_service" @@ -747,6 +758,33 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.assertIsNotNone(channel.json_body.get("sid")) + @unittest.override_config( + { + "public_baseurl": "https://test_server", + "email": { + "smtp_host": "mail_server", + "smtp_port": 2525, + "notif_from": "sender@host", + }, + } + ) + def test_request_token_allowed_when_email_flow_is_advertised(self) -> None: + sendmail = self._get_sendmail_mock() + sendmail.reset_mock() + + channel = self.make_request( + "POST", + b"register/email/requestToken", + { + "client_secret": "foobar", + "email": "test@example.com", + "send_attempt": 1, + }, + ) + self.assertEqual(200, channel.code, channel.result) + self.assertIsNotNone(channel.json_body.get("sid")) + sendmail.assert_awaited_once() + @unittest.override_config( { "public_baseurl": "https://test_server", From 2c412ba24ae013c657128f6ff2256a4e23b181ae Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:30:03 +0000 Subject: [PATCH 09/11] complement.sh: ensure old complement checkout files are deleted; remove -N `wget` flag (#19592) --- changelog.d/19592.misc | 1 + scripts-dev/complement.sh | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 changelog.d/19592.misc diff --git a/changelog.d/19592.misc b/changelog.d/19592.misc new file mode 100644 index 0000000000..e3fa3a5416 --- /dev/null +++ b/changelog.d/19592.misc @@ -0,0 +1 @@ +Ensure old Complement test files are removed when downloading a Complement checkout via `./scripts-dev/complement.sh`. \ No newline at end of file diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index a8a361fd4a..cca87d42a9 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -160,8 +160,18 @@ main() { if [[ -z "$COMPLEMENT_DIR" ]]; then COMPLEMENT_REF=${COMPLEMENT_REF:-main} echo "COMPLEMENT_DIR not set. Fetching Complement checkout from ${COMPLEMENT_REF}..." - wget -Nq https://github.com/matrix-org/complement/archive/${COMPLEMENT_REF}.tar.gz + + # Download the Complement checkout at the specified ref. + wget -q https://github.com/matrix-org/complement/archive/${COMPLEMENT_REF}.tar.gz + + # Delete the existing complement checkout. Otherwise we'll end up with stale + # test files after they're deleted server-side, and `tar` will not delete + # old files. + rm -rf complement-${COMPLEMENT_REF} + + # Extract the checkout. tar -xzf ${COMPLEMENT_REF}.tar.gz + COMPLEMENT_DIR=complement-${COMPLEMENT_REF} echo "Checkout available at 'complement-${COMPLEMENT_REF}'" fi From b4282b82d0dbc3ba34954d442a581ad9331e465a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 20 Mar 2026 16:33:43 +0000 Subject: [PATCH 10/11] Updates for experimental MSC4388 support (sign-in with QR code) (#19573) --- changelog.d/19573.feature | 1 + synapse/config/experimental.py | 4 +- synapse/rest/client/rendezvous.py | 7 ++++ synapse/rest/client/versions.py | 2 - tests/rest/client/test_msc4388_rendezvous.py | 42 ++++++++++++++------ 5 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 changelog.d/19573.feature diff --git a/changelog.d/19573.feature b/changelog.d/19573.feature new file mode 100644 index 0000000000..69eca83681 --- /dev/null +++ b/changelog.d/19573.feature @@ -0,0 +1 @@ +Updated experimental support for [MSC4388: Secure out-of-band channel for sign in with QR](https://github.com/matrix-org/matrix-spec-proposals/pull/4388). diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 0e96b1dc6a..ff9b394eb9 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -538,9 +538,9 @@ class ExperimentalConfig(Config): # See: https://github.com/element-hq/synapse/issues/19433 msc4388_mode = experimental.get("msc4388_mode", "off") - if msc4388_mode not in ["off", "public", "authenticated"]: + if msc4388_mode not in ["off", "open", "authenticated"]: raise ConfigError( - "msc4388_mode must be one of 'off', 'public' or 'authenticated'", + "msc4388_mode must be one of 'off', 'open' or 'authenticated'", ("experimental", "msc4388_mode"), ) self.msc4388_enabled: bool = msc4388_mode != "off" diff --git a/synapse/rest/client/rendezvous.py b/synapse/rest/client/rendezvous.py index bd9205fc5f..246788609e 100644 --- a/synapse/rest/client/rendezvous.py +++ b/synapse/rest/client/rendezvous.py @@ -20,6 +20,7 @@ # import logging +from http import HTTPStatus from http.client import TEMPORARY_REDIRECT from typing import TYPE_CHECKING, Any @@ -81,6 +82,12 @@ class MSC4388CreateRendezvousServlet(RestServlet): hs.config.experimental.msc4388_requires_authentication ) + async def on_GET(self, request: SynapseRequest) -> tuple[int, Any]: + if self.require_authentication: + # This will raise if the user is not authenticated + await self.auth.get_user_by_req(request) + return HTTPStatus.OK, {"create_available": True} + async def on_POST(self, request: SynapseRequest) -> tuple[int, Any]: if self.require_authentication: # This will raise if the user is not authenticated diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 904da86794..7bf4b12e8b 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -189,8 +189,6 @@ class VersionsRestServlet(RestServlet): is not None ) ), - # MSC4388: Secure out-of-band channel for sign in with QR - "io.element.msc4388": (self.config.experimental.msc4388_enabled), # MSC4140: Delayed events "org.matrix.msc4140": bool(self.config.server.max_event_delay_ms), # Simplified sliding sync diff --git a/tests/rest/client/test_msc4388_rendezvous.py b/tests/rest/client/test_msc4388_rendezvous.py index f16bb3f344..913a41dedb 100644 --- a/tests/rest/client/test_msc4388_rendezvous.py +++ b/tests/rest/client/test_msc4388_rendezvous.py @@ -131,6 +131,10 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): } ) def test_off(self) -> None: + # Discovery endpoint should return 404 + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 404) + # Create session should also fail channel = self.make_request("POST", rz_endpoint, {}, access_token=None) self.assertEqual(channel.code, 404) @@ -143,7 +147,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -156,6 +160,11 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): - Deleting the data - Sequence token handling """ + # Discovery should return 200 + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 200) + self.assertTrue(channel.json_body.get("create_available")) + # We can post arbitrary data to the endpoint channel = self.make_request( "POST", @@ -268,7 +277,11 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): self.setup_mock_oauth() alice_token = self.register_oauth_user("alice", "device1") - # This should fail without authentication: + # Discovery should fail due to lack of authentication + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 401) + + # Creating a session should fail without authentication: channel = self.make_request( "POST", rz_endpoint, @@ -277,6 +290,11 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): ) self.assertEqual(channel.code, 401) + # Discovery should succeed with authentication + channel = self.make_request("GET", rz_endpoint, {}, access_token=alice_token) + self.assertEqual(channel.code, 200) + self.assertTrue(channel.json_body.get("create_available")) + # This should work as we are now authenticated channel = self.make_request( "POST", @@ -288,7 +306,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): rendezvous_id = channel.json_body["id"] sequence_token = channel.json_body["sequence_token"] expires_in_ms = channel.json_body["expires_in_ms"] - self.assertGreater(expires_in_ms, 0) + self.assertEqual(expires_in_ms, 120000) session_endpoint = rz_endpoint + f"/{rendezvous_id}" @@ -302,7 +320,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["data"], "foo=bar") self.assertEqual(channel.json_body["sequence_token"], sequence_token) - self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms) + self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 100) # We can update the data without authentication channel = self.make_request( @@ -325,7 +343,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["data"], "foo=baz") self.assertEqual(channel.json_body["sequence_token"], new_sequence_token) - self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 200) + self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 300) # We can delete the data without authentication channel = self.make_request( @@ -355,7 +373,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -402,7 +420,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -473,7 +491,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -531,7 +549,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -585,7 +603,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -648,7 +666,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -698,7 +716,7 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) From 40d699b1d4d7855ffb892723eac90cd34f22aa6f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 20 Mar 2026 13:34:26 -0600 Subject: [PATCH 11/11] Stable support for MSC4284 policy servers (#19503) Fixes https://github.com/element-hq/synapse/issues/19494 MSC4284 policy servers This: * removes the old `/check` (recommendation) support because it's from an older design. Policy servers should have updated to `/sign` by now. We also remove optionality around the policy server's public key because it was only optional to support `/check`. * supports the stable `m.room.policy` state event and `/sign` endpoints, falling back to unstable if required. Note the changes between unstable and stable: * Stable `/sign` uses errors instead of an empty signatures block to indicate refusal. * Stable `m.room.policy` nests the public key in an object with explicit key algorithm (always ed25519 for now) * does *not* introduce tests that the above fallback to unstable works. If it breaks, we're not going to be sad about an early transition. Tests can be added upon request, though. * fixes a bug where the policy server was asked to sign policy server state events (the events were correctly skipped in `is_event_allowed`, but `ask_policy_server_to_sign_event` didn't do the same). * fixes a bug where the original event sender's signature can be deleted if the sending server is the same as the policy server. * proxies Matrix-shaped errors from the policy server to the Client-Server API as `SynapseError`s (a new capability of the stable API). Membership event handling (from the issue) is expected to be a different PR due to the size of changes involved (tracked by https://github.com/element-hq/synapse/issues/19587). ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: turt2live <1190097+turt2live@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Eric Eastwood --- changelog.d/19503.bugfix.1 | 1 + changelog.d/19503.bugfix.2 | 1 + changelog.d/19503.feature | 1 + synapse/api/constants.py | 2 + synapse/federation/federation_client.py | 75 +------ synapse/federation/transport/client.py | 57 +++--- synapse/handlers/room_policy.py | 251 ++++++++++++++++-------- synapse/types/handlers/policy_server.py | 16 -- tests/handlers/test_room_policy.py | 96 ++++----- 9 files changed, 250 insertions(+), 250 deletions(-) create mode 100644 changelog.d/19503.bugfix.1 create mode 100644 changelog.d/19503.bugfix.2 create mode 100644 changelog.d/19503.feature delete mode 100644 synapse/types/handlers/policy_server.py diff --git a/changelog.d/19503.bugfix.1 b/changelog.d/19503.bugfix.1 new file mode 100644 index 0000000000..45e3ad603b --- /dev/null +++ b/changelog.d/19503.bugfix.1 @@ -0,0 +1 @@ +Fix [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Servers implementation to skip signing `org.matrix.msc4284.policy` and `m.room.policy` state events. \ No newline at end of file diff --git a/changelog.d/19503.bugfix.2 b/changelog.d/19503.bugfix.2 new file mode 100644 index 0000000000..8a423ba785 --- /dev/null +++ b/changelog.d/19503.bugfix.2 @@ -0,0 +1 @@ +Correctly apply [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Server signatures to events when the sender and policy server have the same server name. \ No newline at end of file diff --git a/changelog.d/19503.feature b/changelog.d/19503.feature new file mode 100644 index 0000000000..20db8aeaae --- /dev/null +++ b/changelog.d/19503.feature @@ -0,0 +1 @@ +Add stable support for [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Servers. diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 21139a3867..eb9e6cc39b 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -158,6 +158,8 @@ class EventTypes: PollStart: Final = "m.poll.start" + RoomPolicy: Final = "m.room.policy" + class ToDeviceEventTypes: RoomKeyRequest: Final = "m.room_key_request" diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index ba738ad65e..55151ca549 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -72,7 +72,6 @@ from synapse.http.types import QueryParams from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, tag_args, trace from synapse.metrics import SERVER_NAME_LABEL from synapse.types import JsonDict, StrCollection, UserID, get_domain_from_id -from synapse.types.handlers.policy_server import RECOMMENDATION_OK, RECOMMENDATION_SPAM from synapse.util.async_helpers import concurrently_execute from synapse.util.caches.expiringcache import ExpiringCache from synapse.util.duration import Duration @@ -438,72 +437,16 @@ class FederationClient(FederationBase): return None - @trace - @tag_args - async def get_pdu_policy_recommendation( - self, destination: str, pdu: EventBase, timeout: int | None = None - ) -> str: - """Requests that the destination server (typically a policy server) - check the event and return its recommendation on how to handle the - event. - - If the policy server could not be contacted or the policy server - returned an unknown recommendation, this returns an OK recommendation. - This type fixing behaviour is done because the typical caller will be - in a critical call path and would generally interpret a `None` or similar - response as "weird value; don't care; move on without taking action". We - just frontload that logic here. - - - Args: - destination: The remote homeserver to ask (a policy server) - pdu: The event to check - timeout: How long to try (in ms) the destination for before - giving up. None indicates no timeout. - - Returns: - The policy recommendation, or RECOMMENDATION_OK if the policy server was - uncontactable or returned an unknown recommendation. - """ - - logger.debug( - "get_pdu_policy_recommendation for event_id=%s from %s", - pdu.event_id, - destination, - ) - - try: - res = await self.transport_layer.get_policy_recommendation_for_pdu( - destination, pdu, timeout=timeout - ) - recommendation = res.get("recommendation") - if not isinstance(recommendation, str): - raise InvalidResponseError("recommendation is not a string") - if recommendation not in (RECOMMENDATION_OK, RECOMMENDATION_SPAM): - logger.warning( - "get_pdu_policy_recommendation: unknown recommendation: %s", - recommendation, - ) - return RECOMMENDATION_OK - return recommendation - except Exception as e: - logger.warning( - "get_pdu_policy_recommendation: server %s responded with error, assuming OK recommendation: %s", - destination, - e, - ) - return RECOMMENDATION_OK - @trace @tag_args async def ask_policy_server_to_sign_event( self, destination: str, pdu: EventBase, timeout: int | None = None - ) -> JsonDict | None: + ) -> JsonDict: """Requests that the destination server (typically a policy server) sign the event as not spam. If the policy server could not be contacted or the policy server - returned an error, this returns no signature. + returned an error, that error is raised. Args: destination: The remote homeserver to ask (a policy server) @@ -519,17 +462,9 @@ class FederationClient(FederationBase): pdu.event_id, destination, ) - try: - return await self.transport_layer.ask_policy_server_to_sign_event( - destination, pdu, timeout=timeout - ) - except Exception as e: - logger.warning( - "ask_policy_server_to_sign_event: server %s responded with error: %s", - destination, - e, - ) - return None + return await self.transport_layer.ask_policy_server_to_sign_event( + destination, pdu, timeout=timeout + ) @trace @tag_args diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 35d3c30c69..5d5212ef96 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -47,6 +47,7 @@ from synapse.api.urls import ( ) from synapse.events import EventBase, make_event_from_dict from synapse.federation.units import Transaction +from synapse.http.client import is_unknown_endpoint from synapse.http.matrixfederationclient import ByteParser, LegacyJsonSendParser from synapse.http.types import QueryParams from synapse.types import JsonDict, UserID @@ -141,33 +142,6 @@ class TransportLayerClient: destination, path=path, timeout=timeout, try_trailing_slash_on_400=True ) - async def get_policy_recommendation_for_pdu( - self, destination: str, event: EventBase, timeout: int | None = None - ) -> JsonDict: - """Requests the policy recommendation for the given pdu from the given policy server. - - Args: - destination: The host name of the remote homeserver checking the event. - event: The event to check. - timeout: How long to try (in ms) the destination for before giving up. - None indicates no timeout. - - Returns: - The full recommendation object from the remote server. - """ - logger.debug( - "get_policy_recommendation_for_pdu dest=%s, event_id=%s", - destination, - event.event_id, - ) - return await self.client.post_json( - destination=destination, - path=f"/_matrix/policy/unstable/org.matrix.msc4284/event/{event.event_id}/check", - data=event.get_pdu_json(), - ignore_backoff=True, - timeout=timeout, - ) - async def ask_policy_server_to_sign_event( self, destination: str, event: EventBase, timeout: int | None = None ) -> JsonDict: @@ -186,13 +160,28 @@ class TransportLayerClient: The signature from the policy server, structured in the same was as the 'signatures' JSON in the event e.g { "$policy_server_via_domain" : { "ed25519:policy_server": "signature_base64" }} """ - return await self.client.post_json( - destination=destination, - path="/_matrix/policy/unstable/org.matrix.msc4284/sign", - data=event.get_pdu_json(), - ignore_backoff=True, - timeout=timeout, - ) + # Try stable first, then fall back to unstable if unsupported. All other errors + # are just errors. + try: + return await self.client.post_json( + destination=destination, + path="/_matrix/policy/v1/sign", + data=event.get_pdu_json(), + ignore_backoff=True, + timeout=timeout, + ) + except HttpResponseException as ex: + if is_unknown_endpoint(ex): + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + return await self.client.post_json( + destination=destination, + path="/_matrix/policy/unstable/org.matrix.msc4284/sign", + data=event.get_pdu_json(), + ignore_backoff=True, + timeout=timeout, + ) + raise async def backfill( self, destination: str, room_id: str, event_tuples: Collection[str], limit: int diff --git a/synapse/handlers/room_policy.py b/synapse/handlers/room_policy.py index 0663a36714..57163c4344 100644 --- a/synapse/handlers/room_policy.py +++ b/synapse/handlers/room_policy.py @@ -17,13 +17,14 @@ import logging from typing import TYPE_CHECKING +import attr from signedjson.key import decode_verify_key_bytes from unpaddedbase64 import decode_base64 -from synapse.api.errors import SynapseError +from synapse.api.constants import EventTypes +from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.crypto.keyring import VerifyJsonRequest from synapse.events import EventBase -from synapse.types.handlers.policy_server import RECOMMENDATION_OK from synapse.util.stringutils import parse_and_validate_server_name if TYPE_CHECKING: @@ -31,10 +32,18 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -POLICY_SERVER_EVENT_TYPE = "org.matrix.msc4284.policy" POLICY_SERVER_KEY_ID = "ed25519:policy_server" +@attr.s(slots=True, auto_attribs=True) +class PolicyServerInfo: + # name of the server. + server_name: str + + # the unpadded base64-encoded Ed25519 public key of the server. + public_key: str + + class RoomPolicyHandler: def __init__(self, hs: "HomeServer"): self._hs = hs @@ -43,6 +52,86 @@ class RoomPolicyHandler: self._event_auth_handler = hs.get_event_auth_handler() self._federation_client = hs.get_federation_client() + def _is_policy_server_state_event(self, event: EventBase) -> bool: + state_key = event.get_state_key() + if state_key is not None and state_key == "": + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + # Note: we can probably drop this whole function when we remove unstable support + return event.type in [EventTypes.RoomPolicy, "org.matrix.msc4284.policy"] + return False + + async def _get_policy_server(self, room_id: str) -> PolicyServerInfo | None: + """Get the policy server's name and Ed25519 public key for the room, if set. + + If the `m.room.policy` state event is invalid, then a policy server is not set. It + can be invalid if: + - The room doesn't have an `m.room.policy` state event with empty state key. + - The policy state event is missing the `via` or `public_keys` field. + - The policy state event's public keys is missing an `ed25519` key. + - The via server is not a valid server name. + - The via server is not in the room. + - The via server is Synapse itself. + + TODO: Remove unstable MSC4284 support - https://github.com/element-hq/synapse/issues/19502 + This function also checks for the unstable `org.matrix.msc4284.policy` state event. + + Args: + room_id: The room ID to get the policy server for. + + Returns: + A tuple of policy server name and its Ed25519 public key (unpadded base64). + Both values will be None if no policy server is configured or the configration + is invalid. + """ + policy_event = await self._storage_controllers.state.get_current_state_event( + room_id, EventTypes.RoomPolicy, "" + ) + public_key = None + if not policy_event: + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + policy_event = ( + await self._storage_controllers.state.get_current_state_event( + room_id, "org.matrix.msc4284.policy", "" + ) + ) + if not policy_event: + return None # neither stable or unstable configured + + # Unstable configured, grab its public key + public_key = policy_event.content.get("public_key", None) + else: + # Stable configured, grab its public key + public_keys = policy_event.content.get("public_keys") + if isinstance(public_keys, dict): + ed25519_key = public_keys.get("ed25519") + if isinstance(ed25519_key, str): + public_key = ed25519_key + + if public_key is None or not isinstance(public_key, str): + return None # no public key means no policy server + + policy_server = policy_event.content.get("via", "") + if policy_server is None or not isinstance(policy_server, str): + return None # no policy server + + if policy_server == self._hs.hostname: + return None # Synapse itself can't be a policy server (currently) + + try: + parse_and_validate_server_name(policy_server) + except ValueError: + return None # invalid policy server + + is_in_room = await self._event_auth_handler.is_host_in_room( + room_id, policy_server + ) + if not is_in_room: + return None # policy server not in room + + return PolicyServerInfo(policy_server, public_key) + async def is_event_allowed(self, event: EventBase) -> bool: """Check if the given event is allowed in the room by the policy server. @@ -62,58 +151,32 @@ class RoomPolicyHandler: Returns: bool: True if the event is allowed in the room, False otherwise. """ - if event.type == POLICY_SERVER_EVENT_TYPE and event.state_key is not None: + if self._is_policy_server_state_event(event): return True # always allow policy server change events - policy_event = await self._storage_controllers.state.get_current_state_event( - event.room_id, POLICY_SERVER_EVENT_TYPE, "" + policy_server = await self._get_policy_server(event.room_id) + if policy_server is None: + return True # no policy server configured, so allow + + # Check if the event has been signed with the public key in the policy server + # state event. If it is, the event is valid according to the policy server and + # we don't need to request a fresh signature. + valid = await self._verify_policy_server_signature( + event, policy_server.server_name, policy_server.public_key ) - if not policy_event: - return True # no policy server == default allow - - policy_server = policy_event.content.get("via", "") - if policy_server is None or not isinstance(policy_server, str): - return True # no policy server == default allow - - if policy_server == self._hs.hostname: - return True # Synapse itself can't be a policy server (currently) + if valid: + return True # valid signature == allow + # We couldn't save the HTTP hit, so do that hit. try: - parse_and_validate_server_name(policy_server) - except ValueError: - return True # invalid policy server == default allow - - is_in_room = await self._event_auth_handler.is_host_in_room( - event.room_id, policy_server - ) - if not is_in_room: - return True # policy server not in room == default allow - - # Check if the event has been signed with the public key in the policy server state event. - # If it is, we can save an HTTP hit. - # We actually want to get the policy server state event BEFORE THE EVENT rather than - # the current state value, else changing the public key will cause all of these checks to fail. - # However, if we are checking outlier events (which we will due to is_event_allowed being called - # near the edges at _check_sigs_and_hash) we won't know the state before the event, so the - # only safe option is to use the current state - public_key = policy_event.content.get("public_key", None) - if public_key is not None and isinstance(public_key, str): - valid = await self._verify_policy_server_signature( - event, policy_server, public_key - ) - if valid: - return True - # fallthrough to hit /check manually - - # At this point, the server appears valid and is in the room, so ask it to check - # the event. - recommendation = await self._federation_client.get_pdu_policy_recommendation( - policy_server, event - ) - if recommendation != RECOMMENDATION_OK: + await self.ask_policy_server_to_sign_event(event, verify=True) + except Exception as ex: + # We probably caught either a refusal to sign, an invalid signature, or + # some other transient or network error. These are all rejection cases. + logger.warning("Failed to get a signature from the policy server: %s", ex) return False - return True # default allow + return True # passed all verifications and checks, so allow async def _verify_policy_server_signature( self, event: EventBase, policy_server: str, public_key: str @@ -140,47 +203,71 @@ class RoomPolicyHandler: """Ask the policy server to sign this event. The signature is added to the event signatures block. Does nothing if there is no policy server state event in the room. If the policy server - refuses to sign the event (as it's marked as spam) does nothing. + refuses to sign the event (as it's marked as spam), an error is raised. Args: - event: The event to sign - verify: If True, verify that the signature is correctly signed by the public_key in the - policy server state event. + event: The event to sign. + verify: If True, verify that the signature is correctly signed by the policy server's + defined public key. Raises: - if verify=True and the policy server signed the event with an invalid signature. Does - not raise if the policy server refuses to sign the event. + When the policy server refuses to sign the event, or when verify is True and the + signature is invalid. """ - policy_event = await self._storage_controllers.state.get_current_state_event( - event.room_id, POLICY_SERVER_EVENT_TYPE, "" - ) - if not policy_event: + if self._is_policy_server_state_event(event): + # per spec, policy servers aren't asked to sign `m.room.policy` state events + # with empty state keys return - policy_server = policy_event.content.get("via", None) - if policy_server is None or not isinstance(policy_server, str): - return - # Only ask to sign events if the policy state event has a public_key (so they can be subsequently verified) - public_key = policy_event.content.get("public_key", None) - if public_key is None or not isinstance(public_key, str): + + policy_server = await self._get_policy_server(event.room_id) + if policy_server is None: return # Ask the policy server to sign this event. # We set a smallish timeout here as we don't want to block event sending too long. - signature = await self._federation_client.ask_policy_server_to_sign_event( - policy_server, - event, - timeout=3000, - ) - if ( - # the policy server returns {} if it refuses to sign the event. - signature and len(signature) > 0 - ): - event.signatures.update(signature) - if verify: - is_valid = await self._verify_policy_server_signature( - event, policy_server, public_key + try: + signature = await self._federation_client.ask_policy_server_to_sign_event( + policy_server.server_name, + event, + timeout=3000, + ) + # TODO: We can *probably* remove this when we remove unstable MSC4284 support. + # The server *should* be returning either a signature or an error, but there could + # also be implementation bugs. Whoever reads this when removing unstable MSC4284 + # stuff, make a decision on whether to remove this bit. + # https://github.com/element-hq/synapse/issues/19502 + if not signature or len(signature) == 0: + raise SynapseError( + 403, + "This event has been rejected as probable spam by the policy server", + Codes.FORBIDDEN, + ) + + # Note: if the policy server and event sender are the same server, the sender + # might not have added policy server signatures to the event for whatever reason. + # When this happens, we don't want to obliterate the event's existing signatures + # because the event will fail authorization. This is why we add defaults rather + # than simply `update` the signatures on the event. + # + # This situation can happen if the homeserver and policy server parts are + # logically the same server, but run by different software. For example, Synapse + # will not ask "itself" for a policy server signature, even if its server name + # is the designated policy server, so it could send an event outwards that other + # servers need to manually fetch signatures for. This is the code that allows + # those events to continue working (because they're legally sent, even if missing + # the policy server signature). + event.signatures.setdefault(policy_server.server_name, {}).update( + signature.get(policy_server.server_name, {}) + ) + except HttpResponseException as ex: + # re-wrap HTTP errors as `SynapseError` so they can be proxied to clients directly + raise ex.to_synapse_error() from ex + + if verify: + is_valid = await self._verify_policy_server_signature( + event, policy_server.server_name, policy_server.public_key + ) + if not is_valid: + raise SynapseError( + 500, + f"policy server {policy_server.server_name} failed to sign event correctly", ) - if not is_valid: - raise SynapseError( - 500, - f"policy server {policy_server} failed to sign event correctly", - ) diff --git a/synapse/types/handlers/policy_server.py b/synapse/types/handlers/policy_server.py deleted file mode 100644 index bfc09dabf4..0000000000 --- a/synapse/types/handlers/policy_server.py +++ /dev/null @@ -1,16 +0,0 @@ -# -# This file is licensed under the Affero General Public License (AGPL) version 3. -# -# Copyright (C) 2025 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: -# . -# - -RECOMMENDATION_OK = "ok" -RECOMMENDATION_SPAM = "spam" diff --git a/tests/handlers/test_room_policy.py b/tests/handlers/test_room_policy.py index ff212ab06e..9adf0e38c2 100644 --- a/tests/handlers/test_room_policy.py +++ b/tests/handlers/test_room_policy.py @@ -19,7 +19,8 @@ from signedjson.key import encode_verify_key_base64, get_verify_key from twisted.internet.testing import MemoryReactor -from synapse.api.errors import SynapseError +from synapse.api.constants import EventTypes +from synapse.api.errors import HttpResponseException, SynapseError from synapse.crypto.event_signing import compute_event_signature from synapse.events import EventBase, make_event_from_dict from synapse.handlers.room_policy import POLICY_SERVER_KEY_ID @@ -27,7 +28,6 @@ from synapse.rest import admin from synapse.rest.client import filter, login, room, sync from synapse.server import HomeServer from synapse.types import JsonDict, UserID -from synapse.types.handlers.policy_server import RECOMMENDATION_OK, RECOMMENDATION_SPAM from synapse.util.clock import Clock from tests import unittest @@ -49,13 +49,9 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): # mock out the federation transport client self.mock_federation_transport_client = mock.Mock( spec=[ - "get_policy_recommendation_for_pdu", "ask_policy_server_to_sign_event", ] ) - self.mock_federation_transport_client.get_policy_recommendation_for_pdu = ( - mock.AsyncMock() - ) self.mock_federation_transport_client.ask_policy_server_to_sign_event = ( mock.AsyncMock() ) @@ -106,25 +102,6 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) - # Prepare the policy server mock to decide spam vs not spam on those events - self.call_count = 0 - - async def get_policy_recommendation_for_pdu( - destination: str, - pdu: EventBase, - timeout: int | None = None, - ) -> JsonDict: - self.call_count += 1 - self.assertEqual(destination, self.OTHER_SERVER_NAME) - if pdu.event_id == self.spammy_event.event_id: - return {"recommendation": RECOMMENDATION_SPAM} - elif pdu.event_id == self.not_spammy_event.event_id: - return {"recommendation": RECOMMENDATION_OK} - else: - self.fail("Unexpected event ID") - - self.mock_federation_transport_client.get_policy_recommendation_for_pdu.side_effect = get_policy_recommendation_for_pdu - # Mock policy server actions on signing events async def policy_server_signs_event( destination: str, pdu: EventBase, timeout: int | None = None @@ -157,7 +134,9 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): async def policy_server_event_sign_error( destination: str, pdu: EventBase, timeout: int | None = None ) -> JsonDict | None: - return None + raise HttpResponseException( + 500, "Internal Server Error", b'{"errcode": "M_UNKNOWN"}' + ) self.policy_server_signs_event = policy_server_signs_event self.policy_server_refuses_to_sign_event = policy_server_refuses_to_sign_event @@ -174,14 +153,16 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): self.hs, self.room_id, policy_user_id, "join" ) ) - content = { + content: JsonDict = { "via": self.OTHER_SERVER_NAME, } if public_key is not None: - content["public_key"] = public_key + content["public_keys"] = { + "ed25519": public_key, + } self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, content, tok=self.creator_token, state_key="", @@ -192,12 +173,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): # case where a room doesn't use a policy server. ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_empty_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { # empty content (no `via`) }, @@ -207,12 +187,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_nonstring_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": 42, # should be a server name }, @@ -222,12 +201,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_self_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { # We ignore events when the policy server is ourselves (for now?) "via": (UserID.from_string(self.creator)).domain, @@ -238,12 +216,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_invalid_server_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": "|this| is *not* a (valid) server name.com", }, @@ -253,12 +230,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_not_in_room_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": f"x.{self.OTHER_SERVER_NAME}", }, @@ -268,14 +244,30 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) + + def test_missing_public_key_event_set(self) -> None: + """ + Tests that a missing public key in the `m.room.policy` state event (an invalid + configuration) is treated as though there is no policy server configured, thus + allowing all events. + """ + self._add_policy_server_to_room() # no public_key + + ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) + self.assertEqual(ok, True) def test_spammy_event_is_spam(self) -> None: - self._add_policy_server_to_room() + verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) + self._add_policy_server_to_room(public_key=verify_key_str) + + # Explicitly configure the policy server mock to refuse to sign the event. + self.mock_federation_transport_client.ask_policy_server_to_sign_event.return_value = False ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, False) - self.assertEqual(self.call_count, 1) + + # Ensure we actually contacted the policy server once for this event. + self.mock_federation_transport_client.ask_policy_server_to_sign_event.assert_awaited_once() def test_signed_event_is_not_spam(self) -> None: verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) @@ -306,8 +298,6 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(event)) self.assertEqual(ok, True) - # Make sure we did not make an HTTP hit to get_policy_recommendation_for_pdu - self.assertEqual(self.call_count, 0) def test_ask_policy_server_to_sign_event_ok(self) -> None: verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) @@ -348,8 +338,15 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) self.mock_federation_transport_client.ask_policy_server_to_sign_event.side_effect = self.policy_server_refuses_to_sign_event - self.get_success( - self.handler.ask_policy_server_to_sign_event(event, verify=True) + fail = self.get_failure( + self.handler.ask_policy_server_to_sign_event(event, verify=True), + SynapseError, + ) + self.assertIsInstance(fail.value, SynapseError) + self.assertEqual(fail.value.code, 403) + self.assertEqual( + fail.value.msg, + "This event has been rejected as probable spam by the policy server", ) self.assertEqual(len(event.signatures), 0) @@ -370,9 +367,12 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) self.mock_federation_transport_client.ask_policy_server_to_sign_event.side_effect = self.policy_server_event_sign_error - self.get_success( - self.handler.ask_policy_server_to_sign_event(event, verify=True) + fail = self.get_failure( + self.handler.ask_policy_server_to_sign_event(event, verify=True), + SynapseError, ) + self.assertIsInstance(fail.value, SynapseError) + self.assertEqual(fail.value.code, 500) self.assertEqual(len(event.signatures), 0) def test_ask_policy_server_to_sign_event_wrong_sig(self) -> None: