From bed00bb7669d2768a7e13715bc89b5c34670225f Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 14 Apr 2026 18:44:58 +0200 Subject: [PATCH 1/7] Allow resigning of events with a new signing key (#19668) This adds a way to re-sign all locally-created events with a new signing key, which is useful when rotating server signing keys. This doesn't trigger automatically, instead needs to be triggered when needed via the admin API. c.f. https://github.com/matrix-org/internal-config/issues/1670#issuecomment-4206020126 for internal discussion. --------- Co-authored-by: Kegan Dougall Co-authored-by: Erik Johnston --- changelog.d/19668.feature | 1 + .../admin_api/background_updates.md | 3 + poetry.lock | 46 ++--- pyproject.toml | 5 +- synapse/crypto/event_signing.py | 55 ++++- synapse/rest/admin/background_updates.py | 19 ++ .../databases/main/events_bg_updates.py | 193 +++++++++++++++++- synapse/types/storage/__init__.py | 2 + tests/crypto/test_event_signing.py | 135 +++++++++++- tests/storage/test_events_bg_updates.py | 168 ++++++++++++++- 10 files changed, 594 insertions(+), 33 deletions(-) create mode 100644 changelog.d/19668.feature diff --git a/changelog.d/19668.feature b/changelog.d/19668.feature new file mode 100644 index 0000000000..ab3585c172 --- /dev/null +++ b/changelog.d/19668.feature @@ -0,0 +1 @@ +Add a way to re-sign local events with a new signing key. diff --git a/docs/usage/administration/admin_api/background_updates.md b/docs/usage/administration/admin_api/background_updates.md index 7b75ee5587..7a78b8964b 100644 --- a/docs/usage/administration/admin_api/background_updates.md +++ b/docs/usage/administration/admin_api/background_updates.md @@ -107,3 +107,6 @@ The following JSON body parameters are available: - `job_name` - A string which job to run. Valid values are: - `populate_stats_process_rooms` - Recalculate the stats for all rooms. - `regenerate_directory` - Recalculate the [user directory](../../../user_directory.md) if it is stale or out of sync. + - `event_resign` - Re-sign all locally-sent events with the current signing key. This is useful after rotating the server's signing key to ensure all historical events are signed with the new key. Optional additional parameters: + - `old_key` - Only re-sign events whose signature verifies against this key. Format: `"ed25519:key_id base64_public_key"` (e.g. `"ed25519:my_old_key XGX0JRS2Af3be3knz2fBiRbApjm2Dh61gXDJA8kcJNI"`). + - `before_ts` - Only re-sign events with a `received_ts` less than this value (milliseconds since the epoch). diff --git a/poetry.lock b/poetry.lock index 9df82f50a6..ef5c13684d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -31,7 +31,7 @@ description = "The ultimate Python library in building OAuth and OpenID Connect optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"jwt\" or extra == \"oidc\"" +markers = "extra == \"oidc\" or extra == \"jwt\" or extra == \"all\"" files = [ {file = "authlib-1.6.9-py2.py3-none-any.whl", hash = "sha256:f08b4c14e08f0861dc18a32357b33fbcfd2ea86cfe3fe149484b4d764c4a0ac3"}, {file = "authlib-1.6.9.tar.gz", hash = "sha256:d8f2421e7e5980cc1ddb4e32d3f5fa659cfaf60d8eaf3281ebed192e4ab74f04"}, @@ -531,7 +531,7 @@ description = "XML bomb protection for Python stdlib modules" optional = true python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "defusedxml-0.7.1-py2.py3-none-any.whl", hash = "sha256:a352e7e428770286cc899e2542b6cdaedb2b4953ff269a210103ec58f6198a61"}, {file = "defusedxml-0.7.1.tar.gz", hash = "sha256:1bb3032db185915b62d7c6209c5a8792be6a32ab2fedacc84e01b52c51aa3e69"}, @@ -556,7 +556,7 @@ description = "XPath 1.0/2.0/3.0/3.1 parsers and selectors for ElementTree and l optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "elementpath-4.8.0-py3-none-any.whl", hash = "sha256:5393191f84969bcf8033b05ec4593ef940e58622ea13cefe60ecefbbf09d58d9"}, {file = "elementpath-4.8.0.tar.gz", hash = "sha256:5822a2560d99e2633d95f78694c7ff9646adaa187db520da200a8e9479dc46ae"}, @@ -606,7 +606,7 @@ description = "Python wrapper for hiredis" optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"redis\"" +markers = "extra == \"redis\" or extra == \"all\"" files = [ {file = "hiredis-3.3.1-cp310-cp310-macosx_10_15_universal2.whl", hash = "sha256:f525734382a47f9828c9d6a1501522c78d5935466d8e2be1a41ba40ca5bb922b"}, {file = "hiredis-3.3.1-cp310-cp310-macosx_10_15_x86_64.whl", hash = "sha256:6e2e1024f0a021777740cb7c633a0efb2c4a4bc570f508223a8dcbcf79f99ef9"}, @@ -930,7 +930,7 @@ description = "Jaeger Python OpenTracing Tracer implementation" optional = true python-versions = ">=3.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "jaeger-client-4.8.0.tar.gz", hash = "sha256:3157836edab8e2c209bd2d6ae61113db36f7ee399e66b1dcbb715d87ab49bfe0"}, ] @@ -1122,7 +1122,7 @@ description = "A strictly RFC 4510 conforming LDAP V3 pure Python client library optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"matrix-synapse-ldap3\"" +markers = "extra == \"matrix-synapse-ldap3\" or extra == \"all\"" files = [ {file = "ldap3-2.9.1-py2.py3-none-any.whl", hash = "sha256:5869596fc4948797020d3f03b7939da938778a0f9e2009f7a072ccf92b8e8d70"}, {file = "ldap3-2.9.1.tar.gz", hash = "sha256:f3e7fc4718e3f09dda568b57100095e0ce58633bcabbed8667ce3f8fbaa4229f"}, @@ -1239,7 +1239,7 @@ description = "Powerful and Pythonic XML processing library combining libxml2/li optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"url-preview\"" +markers = "extra == \"url-preview\" or extra == \"all\"" files = [ {file = "lxml-6.0.2-cp310-cp310-macosx_10_9_universal2.whl", hash = "sha256:e77dd455b9a16bbd2a5036a63ddbd479c19572af81b624e79ef422f929eef388"}, {file = "lxml-6.0.2-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:5d444858b9f07cefff6455b983aea9a67f7462ba1f6cbe4a21e8bf6791bf2153"}, @@ -1553,7 +1553,7 @@ description = "An LDAP3 auth provider for Synapse" optional = true python-versions = ">=3.10" groups = ["main"] -markers = "extra == \"all\" or extra == \"matrix-synapse-ldap3\"" +markers = "extra == \"matrix-synapse-ldap3\" or extra == \"all\"" files = [ {file = "matrix_synapse_ldap3-0.4.0-py3-none-any.whl", hash = "sha256:bf080037230d2af5fd3639cb87266de65c1cad7a68ea206278c5b4bf9c1a17f3"}, {file = "matrix_synapse_ldap3-0.4.0.tar.gz", hash = "sha256:cff52ba780170de5e6e8af42863d2648ee23f3bf0a9fea6db52372f9fc00be2b"}, @@ -1834,7 +1834,7 @@ description = "OpenTracing API for Python. See documentation at http://opentraci optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "opentracing-2.4.0.tar.gz", hash = "sha256:a173117e6ef580d55874734d1fa7ecb6f3655160b8b8974a2a1e98e5ec9c840d"}, ] @@ -2032,7 +2032,7 @@ description = "psycopg2 - Python-PostgreSQL Database Adapter" optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"postgres\"" +markers = "extra == \"postgres\" or extra == \"all\"" files = [ {file = "psycopg2-2.9.11-cp310-cp310-win_amd64.whl", hash = "sha256:103e857f46bb76908768ead4e2d0ba1d1a130e7b8ed77d3ae91e8b33481813e8"}, {file = "psycopg2-2.9.11-cp311-cp311-win_amd64.whl", hash = "sha256:210daed32e18f35e3140a1ebe059ac29209dd96468f2f7559aa59f75ee82a5cb"}, @@ -2050,7 +2050,7 @@ description = ".. image:: https://travis-ci.org/chtd/psycopg2cffi.svg?branch=mas optional = true python-versions = "*" groups = ["main"] -markers = "platform_python_implementation == \"PyPy\" and (extra == \"all\" or extra == \"postgres\")" +markers = "platform_python_implementation == \"PyPy\" and (extra == \"postgres\" or extra == \"all\")" files = [ {file = "psycopg2cffi-2.9.0.tar.gz", hash = "sha256:7e272edcd837de3a1d12b62185eb85c45a19feda9e62fa1b120c54f9e8d35c52"}, ] @@ -2066,7 +2066,7 @@ description = "A Simple library to enable psycopg2 compatability" optional = true python-versions = "*" groups = ["main"] -markers = "platform_python_implementation == \"PyPy\" and (extra == \"all\" or extra == \"postgres\")" +markers = "platform_python_implementation == \"PyPy\" and (extra == \"postgres\" or extra == \"all\")" files = [ {file = "psycopg2cffi-compat-1.1.tar.gz", hash = "sha256:d25e921748475522b33d13420aad5c2831c743227dc1f1f2585e0fdb5c914e05"}, ] @@ -2348,7 +2348,7 @@ description = "A development tool to measure, monitor and analyze the memory beh optional = true python-versions = ">=3.6" groups = ["main"] -markers = "extra == \"all\" or extra == \"cache-memory\"" +markers = "extra == \"cache-memory\" or extra == \"all\"" files = [ {file = "Pympler-1.0.1-py3-none-any.whl", hash = "sha256:d260dda9ae781e1eab6ea15bacb84015849833ba5555f141d2d9b7b7473b307d"}, {file = "Pympler-1.0.1.tar.gz", hash = "sha256:993f1a3599ca3f4fcd7160c7545ad06310c9e12f70174ae7ae8d4e25f6c5d3fa"}, @@ -2480,7 +2480,7 @@ description = "Python implementation of SAML Version 2 Standard" optional = true python-versions = ">=3.9,<4.0" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "pysaml2-7.5.0-py3-none-any.whl", hash = "sha256:bc6627cc344476a83c757f440a73fda1369f13b6fda1b4e16bca63ffbabb5318"}, {file = "pysaml2-7.5.0.tar.gz", hash = "sha256:f36871d4e5ee857c6b85532e942550d2cf90ea4ee943d75eb681044bbc4f54f7"}, @@ -2505,7 +2505,7 @@ description = "Extensions to the standard Python datetime module" optional = true python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "python-dateutil-2.9.0.post0.tar.gz", hash = "sha256:37dd54208da7e1cd875388217d5e00ebd4179249f90fb72437e91a35459a0ad3"}, {file = "python_dateutil-2.9.0.post0-py2.py3-none-any.whl", hash = "sha256:a8b2bc7bffae282281c8140a97d3aa9c14da0b136dfe83f850eea9a5f7470427"}, @@ -2533,7 +2533,7 @@ description = "World timezone definitions, modern and historical" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "pytz-2026.1.post1-py2.py3-none-any.whl", hash = "sha256:f2fd16142fda348286a75e1a524be810bb05d444e5a081f37f7affc635035f7a"}, {file = "pytz-2026.1.post1.tar.gz", hash = "sha256:3378dde6a0c3d26719182142c56e60c7f9af7e968076f31aae569d72a0358ee1"}, @@ -2938,7 +2938,7 @@ description = "Python client for Sentry (https://sentry.io)" optional = true python-versions = ">=3.6" groups = ["main"] -markers = "extra == \"all\" or extra == \"sentry\"" +markers = "extra == \"sentry\" or extra == \"all\"" files = [ {file = "sentry_sdk-2.57.0-py2.py3-none-any.whl", hash = "sha256:812c8bf5ff3d2f0e89c82f5ce80ab3a6423e102729c4706af7413fd1eb480585"}, {file = "sentry_sdk-2.57.0.tar.gz", hash = "sha256:4be8d1e71c32fb27f79c577a337ac8912137bba4bcbc64a4ec1da4d6d8dc5199"}, @@ -3138,7 +3138,7 @@ description = "Tornado IOLoop Backed Concurrent Futures" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "threadloop-1.0.2-py2-none-any.whl", hash = "sha256:5c90dbefab6ffbdba26afb4829d2a9df8275d13ac7dc58dccb0e279992679599"}, {file = "threadloop-1.0.2.tar.gz", hash = "sha256:8b180aac31013de13c2ad5c834819771992d350267bddb854613ae77ef571944"}, @@ -3154,7 +3154,7 @@ description = "Python bindings for the Apache Thrift RPC system" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "thrift-0.22.0.tar.gz", hash = "sha256:42e8276afbd5f54fe1d364858b6877bc5e5a4a5ed69f6a005b94ca4918fe1466"}, ] @@ -3229,7 +3229,7 @@ description = "Tornado is a Python web framework and asynchronous networking lib optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "tornado-6.5.5-cp39-abi3-macosx_10_9_universal2.whl", hash = "sha256:487dc9cc380e29f58c7ab88f9e27cdeef04b2140862e5076a66fb6bb68bb1bfa"}, {file = "tornado-6.5.5-cp39-abi3-macosx_10_9_x86_64.whl", hash = "sha256:65a7f1d46d4bb41df1ac99f5fcb685fb25c7e61613742d5108b010975a9a6521"}, @@ -3361,7 +3361,7 @@ description = "non-blocking redis client for python" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"redis\"" +markers = "extra == \"redis\" or extra == \"all\"" files = [ {file = "txredisapi-1.4.11-py3-none-any.whl", hash = "sha256:ac64d7a9342b58edca13ef267d4fa7637c1aa63f8595e066801c1e8b56b22d0b"}, {file = "txredisapi-1.4.11.tar.gz", hash = "sha256:3eb1af99aefdefb59eb877b1dd08861efad60915e30ad5bf3d5bf6c5cedcdbc6"}, @@ -3622,7 +3622,7 @@ description = "An XML Schema validator and decoder" optional = true python-versions = ">=3.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "xmlschema-2.5.1-py3-none-any.whl", hash = "sha256:ec2b2a15c8896c1fcd14dcee34ca30032b99456c3c43ce793fdb9dca2fb4b869"}, {file = "xmlschema-2.5.1.tar.gz", hash = "sha256:4f7497de6c8b6dc2c28ad7b9ed6e21d186f4afe248a5bea4f54eedab4da44083"}, @@ -3756,4 +3756,4 @@ url-preview = ["lxml"] [metadata] lock-version = "2.1" python-versions = ">=3.10.0,<4.0.0" -content-hash = "ce9ac9da9e7ffaf24b3e1e7892342ba486e7af4ea25385f875d0f3a2d5c5d133" +content-hash = "ef0540b89c417a69668f551688bd0974256ea7a580044f3954a76bdf0d8fe7c9" diff --git a/pyproject.toml b/pyproject.toml index 5fa7dade52..c156d4f899 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,10 @@ dependencies = [ # We require 2.0.0 for immutabledict support. "canonicaljson>=2.0.0,<3.0.0", # we use the type definitions added in signedjson 1.1. - "signedjson>=1.1.0,<2.0.0", + # 1.1.0 erroneously removed decode_verify_key_base64 (reintroduced in 1.1.1). + # 1.1.1 is mispackaged (importlib-metadata dependency without minimum version bound) + # 1.1.2, 1.1.3 and 1.1.4 were all released on the same day, so no good reason to use the older version. + "signedjson>=1.1.4,<2.0.0", # validating SSL certs for IP addresses requires service_identity 18.1. "service-identity>=18.1.0", # Twisted 18.9 introduces some logger improvements that the structured diff --git a/synapse/crypto/event_signing.py b/synapse/crypto/event_signing.py index d13d5d04c3..d789c06a9c 100644 --- a/synapse/crypto/event_signing.py +++ b/synapse/crypto/event_signing.py @@ -27,7 +27,7 @@ from typing import Any, Callable from canonicaljson import encode_canonical_json from signedjson.sign import sign_json -from signedjson.types import SigningKey +from signedjson.types import SigningKey, VerifyKey from unpaddedbase64 import decode_base64, encode_base64 from synapse.api.errors import Codes, SynapseError @@ -35,7 +35,7 @@ from synapse.api.room_versions import RoomVersion from synapse.events import EventBase from synapse.events.utils import prune_event, prune_event_dict from synapse.logging.opentracing import trace -from synapse.types import JsonDict +from synapse.types import JsonDict, UserID logger = logging.getLogger(__name__) @@ -192,3 +192,54 @@ def add_hashes_and_signatures( event_dict["signatures"] = compute_event_signature( room_version, event_dict, signature_name=signature_name, signing_key=signing_key ) + + +def resign_event( + ev: EventBase, + server_name: str, + signing_key: SigningKey, + time_now: int | None = None, +) -> JsonDict: + """Re-sign the provided event with the given signing key. Any existing signatures on the event + for this server_name are removed. + + If there has been no signature for this event by this server_name, the event is still re-signed. + If there have been signatures on this event by this server_name, the event is not re-checked for + validity. As such, only events that have valid signatures should be passed into this function + e.g. from the event_json table in the database. + """ + event_dict = ev.get_pdu_json(time_now=time_now) + event_dict["signatures"].pop( + server_name, None + ) # remove existing signatures for this server_name + event_dict["signatures"].update( + compute_event_signature( + ev.room_version, + event_dict, + server_name, + signing_key, + ) + ) + return event_dict + + +def event_needs_resigning( + ev: EventBase, server_name: str, verify_key: VerifyKey +) -> bool: + """Check if this event needs re-signing. + + This returns True if all of the following are True: + - the event `sender` domain matches the `server_name` provided. + - the event has not been already signed with this `verify_key`. + """ + sender = UserID.from_string(ev.sender) + if sender.domain != server_name: + return False + want_key_id = verify_key.alg + ":" + verify_key.version + signed_with_current_key_id = ev.signatures.get(server_name, {}).get( + want_key_id, None + ) + if signed_with_current_key_id: + return False + + return True diff --git a/synapse/rest/admin/background_updates.py b/synapse/rest/admin/background_updates.py index 96190c416d..e693a0afd3 100644 --- a/synapse/rest/admin/background_updates.py +++ b/synapse/rest/admin/background_updates.py @@ -18,6 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # +import json import logging from http import HTTPStatus from typing import TYPE_CHECKING @@ -150,6 +151,24 @@ class BackgroundUpdateStartJobRestServlet(RestServlet): "populate_user_directory_process_users", ), ] + elif job_name == "event_resign": + old_key = body.get("old_key") + if old_key is not None and not isinstance(old_key, str): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "'old_key' must be a string", + ) + before_ts = body.get("before_ts") + if before_ts is not None and not isinstance(before_ts, int): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "'before_ts' must be an integer", + ) + progress = { + "old_key": old_key, + "before_ts": before_ts, + } + jobs = [("event_resign", json.dumps(progress), "")] else: raise SynapseError(HTTPStatus.BAD_REQUEST, "Invalid job_name") diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 934cd157ca..d2623f0760 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -23,6 +23,8 @@ import logging from typing import TYPE_CHECKING, cast import attr +from signedjson.key import decode_verify_key_base64, get_verify_key +from signedjson.sign import SignatureVerifyException, verify_signed_json from synapse.api.constants import ( MAX_DEPTH, @@ -31,7 +33,12 @@ from synapse.api.constants import ( RelationTypes, ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS +from synapse.crypto.event_signing import ( + event_needs_resigning, + resign_event, +) from synapse.events import EventBase, make_event_from_dict +from synapse.events.utils import prune_event_dict from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause from synapse.storage.database import ( DatabasePool, @@ -39,6 +46,7 @@ from synapse.storage.database import ( LoggingTransaction, make_tuple_comparison_clause, ) +from synapse.storage.databases.main.cache import CacheInvalidationWorkerStore from synapse.storage.databases.main.events import ( SLIDING_SYNC_RELEVANT_STATE_SET, PersistEventsStore, @@ -48,6 +56,7 @@ from synapse.storage.databases.main.events import ( ) from synapse.storage.databases.main.events_worker import ( DatabaseCorruptionError, + EventRedactBehaviour, InvalidEventError, ) from synapse.storage.databases.main.state_deltas import StateDeltasStore @@ -112,7 +121,9 @@ class _JoinedRoomStreamOrderingUpdate: most_recent_bump_stamp: int | None -class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseStore): +class EventsBackgroundUpdatesStore( + StreamWorkerStore, StateDeltasStore, CacheInvalidationWorkerStore, SQLBaseStore +): def __init__( self, database: DatabasePool, @@ -346,6 +357,11 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS _BackgroundUpdates.FIXUP_MAX_DEPTH_CAP, self.fixup_max_depth_cap_bg_update ) + self.db_pool.updates.register_background_update_handler( + _BackgroundUpdates.EVENT_RESIGN, + self._resign_events, + ) + # We want this to run on the main database at startup before we start processing # events. # @@ -1370,7 +1386,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) # Iterate the parent IDs and invalidate caches. - self._invalidate_cache_and_stream_bulk( # type: ignore[attr-defined] + self._invalidate_cache_and_stream_bulk( txn, self.get_relations_for_event, # type: ignore[attr-defined] { @@ -1381,7 +1397,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS for r in relations_to_insert }, ) - self._invalidate_cache_and_stream_bulk( # type: ignore[attr-defined] + self._invalidate_cache_and_stream_bulk( txn, self.get_thread_summary, # type: ignore[attr-defined] {(r[1],) for r in relations_to_insert}, @@ -2713,6 +2729,177 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS return num_rooms + async def _resign_events(self, progress: dict, batch_size: int) -> int: + """Retroactively re-sign events signed with a different key than the + current signing key. + + Optional progress parameters: + old_key: If set, only re-sign events whose signature can be + verified with this key. Format: "algorithm:key_id base64key" + (e.g. "ed25519:my_old_key XGX0JRS2Af3be3k..."). + before_ts: If set, only re-sign events with a received_ts less + than this value (milliseconds since epoch). + """ + + # Read optional filter parameters from progress. These are set once + # when the job is created and preserved across batches. + old_key_str: str | None = progress.get("old_key") + before_ts: int | None = progress.get("before_ts") + + # Parse the old verify key if provided. + old_verify_key = None + if old_key_str is not None: + parts = old_key_str.split(" ", 1) + if len(parts) == 2: + key_id, key_base64 = parts + alg, _, version = key_id.partition(":") + old_verify_key = decode_verify_key_base64(alg, version, key_base64) + else: + raise ValueError( + f"Invalid old_key format: expected 'algorithm:version base64key', got {old_key_str!r}" + ) + + # Load the next set of candidate events to re-sign. + # Returns the event IDs and the highest stream position for those events. + # If no event IDs are returned, this signals the background update is complete. + def _fetch_next_events_txn( + txn: LoggingTransaction, + ) -> tuple[list[str], int]: + # Start from the minimum 32-bit integer to ensure we cover events + # with negative stream orderings (e.g. from backfill). + last_stream_pos: int = progress.get("last_stream_pos", -(1 << 31)) + + sql = """ + SELECT event_id, stream_ordering FROM events + WHERE stream_ordering > ? AND sender LIKE ? + """ + args: list[object] = [ + last_stream_pos, + f"%:{self.hs.hostname}", + ] + + if before_ts is not None: + sql += " AND received_ts < ?" + args.append(before_ts) + + sql += " ORDER BY stream_ordering ASC LIMIT ?" + args.append(batch_size) + + txn.execute(sql, args) + event_rows: list[tuple[str, int]] = txn.fetchall() + if not event_rows: + return [], last_stream_pos + + last_stream_pos = event_rows[-1][1] + return [row[0] for row in event_rows], last_stream_pos + + next_event_ids, max_stream_pos = await self.db_pool.runInteraction( + "_resign_events._fetch_next_events", + _fetch_next_events_txn, + ) + logger.debug( + "Resign[num_checking=%d,sp=%d]", len(next_event_ids), max_stream_pos + ) + + if not next_event_ids: + await self.db_pool.updates._end_background_update( + _BackgroundUpdates.EVENT_RESIGN + ) + return 0 + + next_events = await self.get_events_as_list( + next_event_ids, + redact_behaviour=EventRedactBehaviour.as_is, + ) + current_verify_key = get_verify_key(self.hs.signing_key) + + # Re-sign any events that need it. + # A list of event IDs and their newly signed event dicts. + resigned_events: list[tuple[str, JsonDict]] = [] + for event in next_events: + if not event_needs_resigning(event, self.hs.hostname, current_verify_key): + continue + + # If old_key is set, only re-sign events whose signature verifies + # with the provided old key. + if old_verify_key is not None: + old_key_id = f"{old_verify_key.alg}:{old_verify_key.version}" + server_sigs = event.signatures.get(self.hs.hostname, {}) + if old_key_id not in server_sigs: + # Event wasn't signed with this key ID at all, skip. + continue + + # Verify the signature is genuinely from this key. We prune + # first since signatures are computed over the redacted form. + pruned = prune_event_dict(event.room_version, event.get_pdu_json()) + try: + verify_signed_json(pruned, self.hs.hostname, old_verify_key) + except SignatureVerifyException: + # In this case, the key ID was right but the signature doesn't match + # the public key we had. We definitely need to log about this. + logger.warning( + "Event %s has a signature for key %s that does not " + "verify — skipping", + event.event_id, + old_key_id, + ) + continue + + event_dict = resign_event(event, self.hs.hostname, self.hs.signing_key) + resigned_events.append((event.event_id, event_dict)) + + # Atomically write the new stream pos progress with the new signatures, + # else we may update the pos and crash before writing the new + # signatures, thus not re-signing at all! + def _write_events_txn( + txn: LoggingTransaction, + events_to_write: list[tuple[str, JsonDict]], + max_stream_pos: int, + ) -> None: + if events_to_write: + self.db_pool.simple_update_many_txn( + txn, + "event_json", + key_names=["event_id"], + key_values=[[event_id] for event_id, _ in events_to_write], + value_names=["json"], + value_values=[ + [json_encoder.encode(event_dict)] + for _, event_dict in events_to_write + ], + ) + # Always update the progress even if we re-sign nothing. + self.db_pool.updates._background_update_progress_txn( + txn, + _BackgroundUpdates.EVENT_RESIGN, + progress={ + "last_stream_pos": max_stream_pos, + "old_key": old_key_str, + "before_ts": before_ts, + }, + ) + + # Invalidate the event cache for re-signed events so that other + # workers also pick up the new signatures. + for event_id, _ in events_to_write: + self.invalidate_get_event_cache_after_txn(txn, event_id) + self._send_invalidation_to_replication( + txn, "_get_event_cache", (event_id,) + ) + + await self.db_pool.runInteraction( + "_resign_events._write_events_txn", + _write_events_txn, + resigned_events, + max_stream_pos, + ) + + logger.info("Re-signed %d events", len(resigned_events)) + + # Even if we don't re-sign them, we need to let the background updater + # know we're still churning through the events. + return len(next_event_ids) + def _resolve_stale_data_in_sliding_sync_tables( txn: LoggingTransaction, diff --git a/synapse/types/storage/__init__.py b/synapse/types/storage/__init__.py index 992c36caba..6b857aeb0e 100644 --- a/synapse/types/storage/__init__.py +++ b/synapse/types/storage/__init__.py @@ -66,3 +66,5 @@ class _BackgroundUpdates: FIXUP_MAX_DEPTH_CAP = "fixup_max_depth_cap" REDACTIONS_RECHECK_BG_UPDATE = "redactions_recheck" + + EVENT_RESIGN = "event_resign" diff --git a/tests/crypto/test_event_signing.py b/tests/crypto/test_event_signing.py index 9cdc1604da..334ff64bc2 100644 --- a/tests/crypto/test_event_signing.py +++ b/tests/crypto/test_event_signing.py @@ -19,12 +19,23 @@ # # -from signedjson.key import decode_signing_key_base64 +from typing import TypedDict + +from signedjson.key import ( + decode_signing_key_base64, + generate_signing_key, + get_verify_key, +) from signedjson.types import SigningKey from synapse.api.room_versions import RoomVersions -from synapse.crypto.event_signing import add_hashes_and_signatures -from synapse.events import make_event_from_dict +from synapse.crypto.event_signing import ( + add_hashes_and_signatures, + event_needs_resigning, + resign_event, +) +from synapse.events import EventBase, make_event_from_dict +from synapse.types import JsonDict from tests import unittest @@ -107,3 +118,121 @@ class EventSigningTestCase(unittest.TestCase): "Ay4aj2b5oJ1k8INYZ9n3KnszCflM0emwcmQQ7vxpbdc" "Sv9bkJxIZdWX1IJllcZLq89+D3sSabE+vqPtZs9akDw", ) + + +class EventResigningTestCase(unittest.TestCase): + def setUp(self) -> None: + self.signing_key: SigningKey = decode_signing_key_base64( + KEY_ALG, KEY_VER, SIGNING_KEY_SEED + ) + + def test_resign(self) -> None: + event_dict: JsonDict = { + "content": {"body": "Here is the message content"}, + "event_id": "$fffff:" + HOSTNAME, + "origin_server_ts": 1000000, + "type": "m.room.message", + "room_id": "!r:" + HOSTNAME, + "sender": "@u:" + HOSTNAME, + "signatures": {}, + "unsigned": {"age_ts": 1000000}, + } + add_hashes_and_signatures( + RoomVersions.V1, event_dict, HOSTNAME, self.signing_key + ) + event = make_event_from_dict(event_dict) + self.assertIn(HOSTNAME, event.signatures) + self.assertIn(KEY_NAME, event.signatures[HOSTNAME]) + signature = event.signatures[HOSTNAME][KEY_NAME] + + # Re-sign with a different key + signing_key_2: SigningKey = generate_signing_key("2") + key_name_2 = "ed25519:2" + + resigned_event = resign_event(event, HOSTNAME, signing_key_2) + self.assertIn(HOSTNAME, resigned_event["signatures"]) + self.assertIn(key_name_2, resigned_event["signatures"][HOSTNAME]) + self.assertEqual( + len(resigned_event["signatures"][HOSTNAME]), 1 + ) # the previous signature was removed. + self.assertNotEqual( + signature, resigned_event["signatures"][HOSTNAME][key_name_2] + ) # different signatures + + # Repeat but with an event without any signatures. + event_dict = { + "content": {"body": "Here is the message content"}, + "event_id": "$fffff:" + HOSTNAME, + "origin_server_ts": 1000000, + "type": "m.room.message", + "room_id": "!r:" + HOSTNAME, + "sender": "@u:" + HOSTNAME, + "signatures": {}, + "unsigned": {"age_ts": 1000000}, + } + event = make_event_from_dict(event_dict) + resigned_event = resign_event(event, HOSTNAME, signing_key_2) + self.assertIn(HOSTNAME, resigned_event["signatures"]) + self.assertIn(key_name_2, resigned_event["signatures"][HOSTNAME]) + self.assertEqual(len(resigned_event["signatures"][HOSTNAME]), 1) + + def test_event_needs_resigning(self) -> None: + event_that_needs_resigning_dict: JsonDict = { + "content": {"body": "Here is the message content"}, + "event_id": "$fffff:" + HOSTNAME, + "origin_server_ts": 1000000, + "type": "m.room.message", + "room_id": "!r:" + HOSTNAME, + "sender": "@u:" + HOSTNAME, + "unsigned": {"age_ts": 1000000}, + } + internal_metadata: JsonDict = {} + event_that_needs_resigning = make_event_from_dict( + event_that_needs_resigning_dict, + RoomVersions.V1, + internal_metadata, + ) + self.assertEqual( + event_needs_resigning( + event_that_needs_resigning, HOSTNAME, get_verify_key(self.signing_key) + ), + True, + ) + + class TestCase(TypedDict): + name: str + event: EventBase + + events_that_dont_need_resigning: list[TestCase] = [ + { + "name": "sender domain isn't ours", + "event": make_event_from_dict( + {**event_that_needs_resigning_dict, "sender": "@u:somewhereelse"}, + RoomVersions.V1, + internal_metadata, + ), + }, + { + "name": "already signed with this key", + "event": make_event_from_dict( + { + **event_that_needs_resigning_dict, + "signatures": { + HOSTNAME: { + KEY_NAME: "thisisntchecked", + }, + }, + }, + RoomVersions.V1, + internal_metadata, + ), + }, + ] + for test_case in events_that_dont_need_resigning: + self.assertEqual( + event_needs_resigning( + test_case["event"], HOSTNAME, get_verify_key(self.signing_key) + ), + False, + test_case["name"], + ) diff --git a/tests/storage/test_events_bg_updates.py b/tests/storage/test_events_bg_updates.py index a5b53de77f..aceacec98e 100644 --- a/tests/storage/test_events_bg_updates.py +++ b/tests/storage/test_events_bg_updates.py @@ -14,17 +14,23 @@ # +import json + +import signedjson.key from canonicaljson import encode_canonical_json from twisted.internet.testing import MemoryReactor from synapse.api.constants import MAX_DEPTH from synapse.api.room_versions import RoomVersion, RoomVersions +from synapse.rest import admin +from synapse.rest.client import login, room from synapse.server import HomeServer +from synapse.storage.background_updates import BackgroundUpdater from synapse.types.storage import _BackgroundUpdates from synapse.util.clock import Clock -from tests.unittest import HomeserverTestCase +from tests.unittest import HomeserverTestCase, override_config class TestFixupMaxDepthCapBgUpdate(HomeserverTestCase): @@ -287,3 +293,163 @@ class TestRedactionsRecheckBgUpdate(HomeserverTestCase): self.assertTrue(self._get_recheck("$redact5:test")) self.assertFalse(self._get_recheck("$redact6:test")) self.assertTrue(self._get_recheck("$redact7:test")) + + +class TestResignEventsBgUpdate(HomeserverTestCase): + """Test the background update that re-signs events.""" + + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + self.updates: BackgroundUpdater = self.hs.get_datastores().main.db_pool.updates + self.store = self.hs.get_datastores().main + self.db_pool = self.store.db_pool + + self.room_id = "!testroom:example.com" + + @override_config({"caches": {"global_factor": 1}, "event_cache_size": "999"}) + def test_events_are_resigned_after_bg_update_runs(self) -> None: + """Test that the background update correctly re-signs existing events with the + new key""" + + # Ensure all background updates have finished running + self.wait_for_background_updates() + + # Set up a room with a local and remote user in it. + self.register_user("user", "pass") + token = self.login("user", "pass") + + # Create new room + room_id = self.helper.create_room_as( + "user", room_version=RoomVersions.V12.identifier, tok=token + ) + + # Send a message + body = self.helper.send(room_id, body="Test", tok=token) + + old_event = self.get_success(self.store.get_event(body["event_id"])) + old_key_id = f"{self.hs.signing_key.alg}:{self.hs.signing_key.version}" + + # Ensure the message event is in the cache so that we test the cache is + # invalidated properly + res = self.store._get_event_cache.get_local((old_event.event_id,)) + self.assertEqual(res.event, old_event, "Event not cached as expected.") # type: ignore + + # Ensure message event is signed with original signing key + self.assertIn( + old_key_id, old_event.signatures[self.hs.config.server.server_name] + ) + + # Generate a new signing key + self.hs.signing_key = signedjson.key.generate_signing_key("new-test-key") + + # Reinsert the background update as it was already run at the start of + # the test. + self.get_success( + self.db_pool.simple_insert( + table="background_updates", + values={ + "update_name": "event_resign", + "progress_json": "{}", + }, + ) + ) + self.updates.start_doing_background_updates() + # Ensure the background updates have finished running + self.wait_for_background_updates() + + # Get the event from the database again + new_event = self.get_success(self.store.get_event(body["event_id"])) + new_key_id = f"{self.hs.signing_key.alg}:{self.hs.signing_key.version}" + + # Ensure message event is signed with new signing key, and not with the original + # signing key + self.assertNotIn( + old_key_id, new_event.signatures[self.hs.config.server.server_name] + ) + self.assertIn( + new_key_id, new_event.signatures[self.hs.config.server.server_name] + ) + + @override_config({"caches": {"global_factor": 1}, "event_cache_size": "999"}) + def test_old_key_filter(self) -> None: + """Test that old_key parameter causes only events whose signature + verifies against the provided key to be re-signed.""" + + self.wait_for_background_updates() + + self.register_user("user2", "pass") + token = self.login("user2", "pass") + + room_id = self.helper.create_room_as( + "user2", room_version=RoomVersions.V12.identifier, tok=token + ) + body = self.helper.send(room_id, body="Test old_key", tok=token) + + old_signing_key = self.hs.signing_key + old_key_id = f"{old_signing_key.alg}:{old_signing_key.version}" + old_verify_key = signedjson.key.get_verify_key(old_signing_key) + old_key_param = ( + f"{old_verify_key.alg}:{old_verify_key.version} " + f"{signedjson.key.encode_verify_key_base64(old_verify_key)}" + ) + + # Generate a new signing key + self.hs.signing_key = signedjson.key.generate_signing_key("new-test-key-2") + + # Generate a different key but reuse the same key ID/version, to + # ensure we're filtering on the actual public key, not just the ID. + wrong_key = signedjson.key.generate_signing_key(old_signing_key.version) + wrong_verify_key = signedjson.key.get_verify_key(wrong_key) + wrong_key_param = ( + f"{old_verify_key.alg}:{old_verify_key.version} " + f"{signedjson.key.encode_verify_key_base64(wrong_verify_key)}" + ) + + # Insert BG update with old_key filter pointing to a WRONG key + self.get_success( + self.db_pool.simple_insert( + table="background_updates", + values={ + "update_name": "event_resign", + "progress_json": json.dumps({"old_key": wrong_key_param}), + }, + ) + ) + self.updates.start_doing_background_updates() + self.wait_for_background_updates() + + # Event should NOT have been re-signed (wrong key) + event_after = self.get_success(self.store.get_event(body["event_id"])) + self.assertIn( + old_key_id, event_after.signatures[self.hs.config.server.server_name] + ) + + # Now insert BG update with the CORRECT old key + self.get_success( + self.db_pool.simple_insert( + table="background_updates", + values={ + "update_name": "event_resign", + "progress_json": json.dumps({"old_key": old_key_param}), + }, + ) + ) + self.updates.start_doing_background_updates() + self.wait_for_background_updates() + + # Event should now be re-signed + new_event = self.get_success(self.store.get_event(body["event_id"])) + new_key_id = f"{self.hs.signing_key.alg}:{self.hs.signing_key.version}" + self.assertNotIn( + old_key_id, new_event.signatures[self.hs.config.server.server_name] + ) + self.assertIn( + new_key_id, new_event.signatures[self.hs.config.server.server_name] + ) From 52c05c5ca418058436137cb4e8e3af01120e6890 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 15 Apr 2026 15:53:23 +0000 Subject: [PATCH 2/7] Introduce `spam_checker_spammy` internal event metadata. (#19453) Follows: #19365 Part of: MSC4354 Sticky Events (experimental feature #19409) This PR introduces a `spam_checker_spammy` flag, analogous to `policy_server_spammy`, as an explicit flag that an event was decided to be spammy by a spam-checker module. The original Sticky Events PR (#18968) just reused `policy_server_spammy`, but it didn't sit right with me because we (at least appear to be experimenting with features that) allow users to opt-in to seeing `policy_server_spammy` events (presumably for moderation purposes). Keeping these flags separate felt best, therefore. As for why we need this flag: soon soft-failed status won't be permanent, at least for sticky events. The spam checker modules currently work by making events soft-failed. We want to prevent spammy events from getting reconsidered/un-soft-failed, so it seems like we need a flag to track spam-checker spamminess *separately* from soft-failed. Should be commit-by-commit friendly, but is also small. --------- Signed-off-by: Olivier 'reivilibre --- changelog.d/19453.misc | 1 + rust/src/events/internal_metadata.rs | 24 +++ synapse/federation/federation_base.py | 6 +- .../storage/databases/main/sticky_events.py | 13 +- synapse/synapse_rust/events.pyi | 13 ++ tests/module_api/test_spamchecker.py | 188 +++++++++++++++++- .../client/sliding_sync/test_rooms_meta.py | 3 +- tests/storage/test_roommember.py | 4 +- tests/storage/test_sliding_sync_tables.py | 4 +- tests/storage/test_sticky_events.py | 152 ++++++++++++++ tests/test_utils/event_injection.py | 24 ++- 11 files changed, 416 insertions(+), 16 deletions(-) create mode 100644 changelog.d/19453.misc diff --git a/changelog.d/19453.misc b/changelog.d/19453.misc new file mode 100644 index 0000000000..48744c3a7a --- /dev/null +++ b/changelog.d/19453.misc @@ -0,0 +1 @@ +Introduce `spam_checker_spammy` internal event metadata. \ No newline at end of file diff --git a/rust/src/events/internal_metadata.rs b/rust/src/events/internal_metadata.rs index a53c1e771b..503722955e 100644 --- a/rust/src/events/internal_metadata.rs +++ b/rust/src/events/internal_metadata.rs @@ -55,6 +55,7 @@ enum EventInternalMetadataData { SoftFailed(bool), ProactivelySend(bool), PolicyServerSpammy(bool), + SpamCheckerSpammy(bool), Redacted(bool), TxnId(Box), DelayId(Box), @@ -105,6 +106,13 @@ impl EventInternalMetadataData { .to_owned() .into_any(), ), + EventInternalMetadataData::SpamCheckerSpammy(o) => ( + pyo3::intern!(py, "spam_checker_spammy"), + o.into_pyobject(py) + .unwrap_infallible() + .to_owned() + .into_any(), + ), EventInternalMetadataData::Redacted(o) => ( pyo3::intern!(py, "redacted"), o.into_pyobject(py) @@ -173,6 +181,11 @@ impl EventInternalMetadataData { .extract() .with_context(|| format!("'{key_str}' has invalid type"))?, ), + "spam_checker_spammy" => EventInternalMetadataData::SpamCheckerSpammy( + value + .extract() + .with_context(|| format!("'{key_str}' has invalid type"))?, + ), "redacted" => EventInternalMetadataData::Redacted( value .extract() @@ -468,6 +481,17 @@ impl EventInternalMetadata { set_property!(self, PolicyServerSpammy, obj); } + #[getter] + fn get_spam_checker_spammy(&self) -> PyResult { + Ok(get_property_opt!(self, SpamCheckerSpammy) + .copied() + .unwrap_or(false)) + } + #[setter] + fn set_spam_checker_spammy(&mut self, obj: bool) { + set_property!(self, SpamCheckerSpammy, obj); + } + #[getter] fn get_redacted(&self) -> PyResult { let bool = get_property!(self, Redacted)?; diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 04ba5b86db..fe0710a0bf 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -177,7 +177,9 @@ class FederationBase: # Note: we don't redact the event so admins can inspect the event after the # fact. Other processes may redact the event, but that won't be applied to # the database copy of the event until the server's config requires it. - return pdu + # + # We also *don't* return early here as we would still like to evaluate + # `spam_checker_spammy`, for completeness. spam_check = await self._spam_checker_module_callbacks.check_event_for_spam(pdu) @@ -194,6 +196,8 @@ class FederationBase: # using the event in prev_events). redacted_event = prune_event(pdu) redacted_event.internal_metadata.soft_failed = True + # Mark this as spam so we don't re-evaluate soft-failure status. + redacted_event.internal_metadata.spam_checker_spammy = True return redacted_event return pdu diff --git a/synapse/storage/databases/main/sticky_events.py b/synapse/storage/databases/main/sticky_events.py index 38b84443df..eee6b92415 100644 --- a/synapse/storage/databases/main/sticky_events.py +++ b/synapse/storage/databases/main/sticky_events.py @@ -302,10 +302,14 @@ class StickyEventsWorkerStore(StateGroupWorkerStore, CacheInvalidationWorkerStor Skips inserting events: - if they are considered spammy by the policy server; (unsure if correct, track: https://github.com/matrix-org/matrix-spec-proposals/pull/4354#discussion_r2727593350) + - if they are considered spammy by a Synapse spam checker module; - if they are rejected; - if they are outliers (they should be reconsidered for insertion when de-outliered); or - if they are not sticky (e.g. if the stickiness expired). + Note: Soft-failed sticky events ARE inserted, as their soft-failed status + could be re-evaluated later. + Skipping the insertion of these types of 'invalid' events is useful for performance reasons because they would fill up the table yet we wouldn't show them to clients anyway. @@ -321,7 +325,12 @@ class StickyEventsWorkerStore(StateGroupWorkerStore, CacheInvalidationWorkerStor sticky_events: list[tuple[EventBase, int]] = [] for ev in events: # MSC: Note: policy servers and other similar antispam techniques still apply to these events. - if ev.internal_metadata.policy_server_spammy: + # We don't filter out soft-failed events altogether (in case they get re-evaluated later), + # so filter out `spam_checker_spammy` events specifically as we don't want to re-evaluate _those_ later. + if ( + ev.internal_metadata.policy_server_spammy + or ev.internal_metadata.spam_checker_spammy + ): continue # We shouldn't be passed rejected events, but if we do, we filter them out too. if ev.rejected_reason is not None: @@ -332,7 +341,7 @@ class StickyEventsWorkerStore(StateGroupWorkerStore, CacheInvalidationWorkerStor sticky_duration = ev.sticky_duration() if sticky_duration is None: continue - # Calculate the end time as start_time + effecitve sticky duration + # Calculate the end time as start_time + effective sticky duration expires_at = min(ev.origin_server_ts, now_ms) + sticky_duration.as_millis() # Filter out already expired sticky events if expires_at <= now_ms: diff --git a/synapse/synapse_rust/events.pyi b/synapse/synapse_rust/events.pyi index e6753bbbad..29432bdd56 100644 --- a/synapse/synapse_rust/events.pyi +++ b/synapse/synapse_rust/events.pyi @@ -38,6 +38,19 @@ class EventInternalMetadata: policy_server_spammy: bool """whether the policy server indicated that this event is spammy""" + spam_checker_spammy: bool + """Whether a spam checker module indicated that this event is spammy + + Note that spam checkers also cause the event to be marked as soft-failed. + + This flags exists for two reasons: + 1. as debugging information + 2. to prevent the soft-failed re-evaluation of spammy events + (the re-evaluation behaviour originates from MSC4354 Sticky Events) + + Note that historical spammy events won't have this flag. + """ + txn_id: str """The transaction ID, if it was set when the event was created.""" delay_id: str diff --git a/tests/module_api/test_spamchecker.py b/tests/module_api/test_spamchecker.py index 42ef969ce0..572f7bdbcf 100644 --- a/tests/module_api/test_spamchecker.py +++ b/tests/module_api/test_spamchecker.py @@ -12,22 +12,33 @@ # . # # +from http import HTTPStatus from typing import Literal from twisted.internet.testing import MemoryReactor -from synapse.api.constants import EventContentFields, EventTypes +from synapse.api.constants import ( + EventContentFields, + EventTypes, + Membership, +) +from synapse.api.room_versions import RoomVersions from synapse.config.server import DEFAULT_ROOM_VERSION +from synapse.events import make_event_from_dict +from synapse.module_api import EventBase from synapse.rest import admin, login, room, room_upgrade_rest_servlet from synapse.server import HomeServer from synapse.types import Codes, JsonDict from synapse.util.clock import Clock +from tests import unittest from tests.server import FakeChannel from tests.unittest import HomeserverTestCase class SpamCheckerTestCase(HomeserverTestCase): + """Tests for the spam checker module API.""" + servlets = [ room.register_servlets, admin.register_servlets, @@ -284,3 +295,178 @@ class SpamCheckerTestCase(HomeserverTestCase): self.assertEqual(channel.code, 403) self.assertEqual(channel.json_body["errcode"], Codes.FORBIDDEN) + + +class FederatedEventSpamCheckMetadataTestCase(unittest.FederatingHomeserverTestCase): + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + self._module_api = hs.get_module_api() + self._store = hs.get_datastores().main + self._storage_controllers = hs.get_storage_controllers() + self._federation_event_handler = hs.get_federation_event_handler() + self._federation_server = hs.get_federation_server() + self._state_handler = hs.get_state_handler() + self._persistence_controller = hs.get_storage_controllers().persistence + + # Create a room + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + self.room_id = self.helper.create_room_as( + user1_id, + tok=user1_tok, + is_public=True, + room_version=RoomVersions.V10.identifier, + ) + + # Prepare a join for the 'remote' user + state_map = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + forward_extremity_event_ids = self.get_success( + self.hs.get_datastores().main.get_latest_event_ids_in_room(self.room_id) + ) + self.remote_user_id = f"@remoteuser:{self.OTHER_SERVER_NAME}" + self.remote_user_join_event = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": self.remote_user_id, + "state_key": self.remote_user_id, + "depth": 1000, + "origin_server_ts": 1, + "type": EventTypes.Member, + "content": {"membership": Membership.JOIN}, + "auth_events": [ + state_map[(EventTypes.Create, "")].event_id, + state_map[(EventTypes.JoinRules, "")].event_id, + ], + "prev_events": list(forward_extremity_event_ids), + } + ), + room_version=RoomVersions.V10, + ) + + # Send the join + self.get_success( + self._federation_event_handler.on_receive_pdu( + self.OTHER_SERVER_NAME, self.remote_user_join_event + ) + ) + + # Check the join made it to the 'local' view of the room + self.helper.get_event( + room_id=self.room_id, + event_id=self.remote_user_join_event.event_id, + tok=user1_tok, + expect_code=HTTPStatus.OK, + ) + + def test_federated_events_with_spam_checker_metadata(self) -> None: + """ + Simulates receiving spammy and non-spammy events over federation, + then checks their `spam_checker_spammy` flag is set properly. + """ + + async def check_event_for_spam(event: EventBase) -> Literal["NOT_SPAM"] | Codes: + if event.type == EventTypes.Message: + if "ham" not in event.content["body"]: + return Codes.FORBIDDEN + return "NOT_SPAM" + + # Register a spam checker callback that only allows messages with 'ham' + self._module_api.register_spam_checker_callbacks( + check_event_for_spam=check_event_for_spam + ) + + # Prepare a spammy and a non-spammy event. + forward_extremity_event_ids = self.get_success( + self._store.get_latest_event_ids_in_room(self.room_id) + ) + state_map = self.get_success( + self._storage_controllers.state.get_current_state(self.room_id) + ) + spammy_event = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": self.remote_user_id, + "depth": 2000, + "origin_server_ts": 2, + "type": EventTypes.Message, + "content": {"body": "this is spam", "msgtype": "m.text"}, + "auth_events": [ + state_map[(EventTypes.Create, "")].event_id, + state_map[(EventTypes.JoinRules, "")].event_id, + state_map[(EventTypes.Member, self.remote_user_id)].event_id, + ], + "prev_events": list(forward_extremity_event_ids), + } + ), + room_version=RoomVersions.V10, + ) + non_spammy_event = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.room_id, + "sender": self.remote_user_id, + "depth": 2000, + "origin_server_ts": 2, + "type": EventTypes.Message, + "content": {"body": "delicious ham", "msgtype": "m.text"}, + "auth_events": [ + state_map[(EventTypes.Create, "")].event_id, + state_map[(EventTypes.JoinRules, "")].event_id, + state_map[(EventTypes.Member, self.remote_user_id)].event_id, + ], + "prev_events": list(forward_extremity_event_ids), + } + ), + room_version=RoomVersions.V10, + ) + + # Receive these events over federation + # We need to let the federation server have them because it will + # invoke `_check_sigs_and_hash` which invokes the spam checker. + self.get_success( + self._federation_server._handle_received_pdu( + self.OTHER_SERVER_NAME, spammy_event + ) + ) + self.get_success( + self._federation_server._handle_received_pdu( + self.OTHER_SERVER_NAME, non_spammy_event + ) + ) + + # Retrieve the events from the database + retrieved_spammy_event = self.get_success( + self._store.get_event(spammy_event.event_id, allow_rejected=True) + ) + retrieved_non_spammy_event = self.get_success( + self._store.get_event(non_spammy_event.event_id, allow_rejected=True) + ) + + # Assert the spammy flags (and soft-failed flags, for good measure) are set properly + self.assertTrue( + retrieved_spammy_event.internal_metadata.spam_checker_spammy, + "Spammy inbound event should be marked as spam_checker_spammy!", + ) + self.assertTrue( + retrieved_spammy_event.internal_metadata.is_soft_failed(), + "Spammy inbound event should be soft-failed.", + ) + + self.assertFalse( + retrieved_non_spammy_event.internal_metadata.spam_checker_spammy, + "Non-spammy inbound event should not be marked as spam_checker_spammy!", + ) + self.assertFalse( + retrieved_non_spammy_event.internal_metadata.is_soft_failed(), + "Non-spammy inbound event should not be soft-failed.", + ) diff --git a/tests/rest/client/sliding_sync/test_rooms_meta.py b/tests/rest/client/sliding_sync/test_rooms_meta.py index 9e3f8aaf94..4ff07b4d26 100644 --- a/tests/rest/client/sliding_sync/test_rooms_meta.py +++ b/tests/rest/client/sliding_sync/test_rooms_meta.py @@ -12,6 +12,7 @@ # . # import logging +from typing import Any from parameterized import parameterized, parameterized_class @@ -966,7 +967,7 @@ class SlidingSyncRoomsMetaTestCase(SlidingSyncBase): creator = "@user:other" room_id = "!foo:other" room_version = RoomVersions.V10 - shared_kwargs = { + shared_kwargs: dict[str, Any] = { "room_id": room_id, "room_version": room_version.identifier, } diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index f8d64e8ce6..711c9ba4ac 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -20,7 +20,7 @@ # # import logging -from typing import cast +from typing import Any, cast from twisted.internet.testing import MemoryReactor @@ -238,7 +238,7 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase): creator = "@user:other" room_id = "!foo:other" room_version = RoomVersions.V10 - shared_kwargs = { + shared_kwargs: dict[str, Any] = { "room_id": room_id, "room_version": room_version.identifier, } diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index f5bbd49663..39226ff9be 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -18,7 +18,7 @@ # # import logging -from typing import cast +from typing import Any, cast import attr from parameterized import parameterized @@ -873,7 +873,7 @@ class SlidingSyncTablesTestCase(SlidingSyncTablesTestCaseBase): creator = "@user:other" room_id = "!foo:other" room_version = RoomVersions.V10 - shared_kwargs = { + shared_kwargs: dict[str, Any] = { "room_id": room_id, "room_version": room_version.identifier, } diff --git a/tests/storage/test_sticky_events.py b/tests/storage/test_sticky_events.py index 60243cb2f4..e77b362f52 100644 --- a/tests/storage/test_sticky_events.py +++ b/tests/storage/test_sticky_events.py @@ -30,6 +30,7 @@ from synapse.util.clock import Clock from synapse.util.duration import Duration from tests import unittest +from tests.test_utils.event_injection import inject_event from tests.utils import USE_POSTGRES_FOR_TESTS @@ -276,3 +277,154 @@ class StickyEventsTestCase(unittest.HomeserverTestCase): ) self.assertEqual(len(updates), 1) self.assertEqual(updates[0].event_id, event_non_outlier.event_id) + + def test_soft_failed_events_are_tracked(self) -> None: + """ + Tests that sticky events marked as soft_failed ARE inserted + into the sticky_events table, as their soft-failed status can be re-evaluated later, + as per MSC4354. + """ + user_id = self.register_user("testuser", "pass") + token = self.login(user_id, "pass") + room_id = self.helper.create_room_as(user_id, tok=token) + + start_id = self.store.get_max_sticky_events_stream_id() + + # Create and persist a sticky event that is soft-failed + soft_failed_sticky_event = self.get_success( + inject_event( + self.hs, + room_id=room_id, + sender=user_id, + type=EventTypes.Message, + content={"body": "spam checker spammy message", "msgtype": "m.text"}, + internal_metadata={"soft_failed": True}, + # Corresponds to StickyEvent.EVENT_FIELD_NAME + msc4354_sticky=StickyEventField( + duration_ms=Duration(minutes=1).as_millis() + ), + ) + ) + + end_id = self.store.get_max_sticky_events_stream_id() + + updates = self.get_success( + self.store.get_updated_sticky_events( + from_id=start_id, to_id=end_id, limit=10 + ) + ) + + self.assertEqual(len(updates), 1) + self.assertEqual(updates[0].event_id, soft_failed_sticky_event.event_id) + + def test_policy_server_spammy_events_are_not_tracked(self) -> None: + """ + Tests that sticky events marked as policy_server_spammy are NOT inserted + into the sticky_events table, as they are exempt from the soft-failed + re-evaluation logic. + """ + user_id = self.register_user("testuser", "pass") + token = self.login(user_id, "pass") + room_id = self.helper.create_room_as(user_id, tok=token) + + start_id = self.store.get_max_sticky_events_stream_id() + + # Create and persist a sticky event that is marked policy_server_spammy + # N.B. policy_server_spammy events are always soft-failed too + _spammy_sticky_event = self.get_success( + inject_event( + self.hs, + room_id=room_id, + sender=user_id, + type=EventTypes.Message, + content={"body": "spam checker spammy message", "msgtype": "m.text"}, + internal_metadata={"soft_failed": True, "policy_server_spammy": True}, + # Corresponds to StickyEvent.EVENT_FIELD_NAME + msc4354_sticky=StickyEventField( + duration_ms=Duration(minutes=1).as_millis() + ), + ) + ) + + # Also insert a valid sticky event as a canary for the test setup + valid_sticky_event = self.get_success( + inject_event( + self.hs, + room_id=room_id, + sender=user_id, + type=EventTypes.Message, + content={"body": "normal sticky", "msgtype": "m.text"}, + # Corresponds to StickyEvent.EVENT_FIELD_NAME + msc4354_sticky=StickyEventField( + duration_ms=Duration(minutes=1).as_millis() + ), + ) + ) + + end_id = self.store.get_max_sticky_events_stream_id() + + # Verify only the regular event was inserted + updates = self.get_success( + self.store.get_updated_sticky_events( + from_id=start_id, to_id=end_id, limit=10 + ) + ) + + self.assertEqual(len(updates), 1) + self.assertEqual(updates[0].event_id, valid_sticky_event.event_id) + + def test_spam_checker_spammy_events_are_not_tracked(self) -> None: + """ + Tests that sticky events marked as spam_checker_spammy are NOT inserted + into the sticky_events table, as they are exempt from the soft-failed + re-evaluation logic. + """ + user_id = self.register_user("testuser", "pass") + token = self.login(user_id, "pass") + room_id = self.helper.create_room_as(user_id, tok=token) + + start_id = self.store.get_max_sticky_events_stream_id() + + # Create and persist a sticky event that is marked spam_checker_spammy + # N.B. spam_checker_spammy events are always soft-failed too + _spammy_sticky_event = self.get_success( + inject_event( + self.hs, + room_id=room_id, + sender=user_id, + type=EventTypes.Message, + content={"body": "spam checker spammy message", "msgtype": "m.text"}, + internal_metadata={"soft_failed": True, "spam_checker_spammy": True}, + # Corresponds to StickyEvent.EVENT_FIELD_NAME + msc4354_sticky=StickyEventField( + duration_ms=Duration(minutes=1).as_millis() + ), + ) + ) + + # Also insert a valid sticky event as a canary for the test setup + valid_sticky_event = self.get_success( + inject_event( + self.hs, + room_id=room_id, + sender=user_id, + type=EventTypes.Message, + content={"body": "normal sticky", "msgtype": "m.text"}, + # Corresponds to StickyEvent.EVENT_FIELD_NAME + msc4354_sticky=StickyEventField( + duration_ms=Duration(minutes=1).as_millis() + ), + ) + ) + + end_id = self.store.get_max_sticky_events_stream_id() + + # Verify only the valid sticky event was inserted + updates = self.get_success( + self.store.get_updated_sticky_events( + from_id=start_id, to_id=end_id, limit=10 + ) + ) + + self.assertEqual(len(updates), 1) + self.assertEqual(updates[0].event_id, valid_sticky_event.event_id) diff --git a/tests/test_utils/event_injection.py b/tests/test_utils/event_injection.py index a90fc5884d..36f6baa123 100644 --- a/tests/test_utils/event_injection.py +++ b/tests/test_utils/event_injection.py @@ -18,7 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # -from typing import Any +from typing import Any, Mapping import synapse.server from synapse.api.constants import EventTypes @@ -63,6 +63,8 @@ async def inject_event( hs: synapse.server.HomeServer, room_version: str | None = None, prev_event_ids: list[str] | None = None, + *, + internal_metadata: Mapping[str, Any] | None = None, **kwargs: Any, ) -> EventBase: """Inject a generic event into a room @@ -72,9 +74,12 @@ async def inject_event( room_version: the version of the room we're inserting into. if not specified, will be looked up prev_event_ids: prev_events for the event. If not specified, will be looked up + internal_metadata: Dict representing the event's internal metadata; see `EventBase.internal_metadata` kwargs: fields for the event to be created """ - event, context = await create_event(hs, room_version, prev_event_ids, **kwargs) + event, context = await create_event( + hs, room_version, prev_event_ids, internal_metadata=internal_metadata, **kwargs + ) persistence = hs.get_storage_controllers().persistence assert persistence is not None @@ -88,8 +93,11 @@ async def create_event( hs: synapse.server.HomeServer, room_version: str | None = None, prev_event_ids: list[str] | None = None, + *, + internal_metadata: Mapping[str, Any] | None = None, **kwargs: Any, ) -> tuple[EventBase, EventContext]: + internal_metadata = internal_metadata or {} if room_version is None: room_version = await hs.get_datastores().main.get_room_version_id( kwargs["room_id"] @@ -106,11 +114,13 @@ async def create_event( ) # Copy over writable internal_metadata, if set - # Dev note: This isn't everything that's writable. `for k,v` doesn't work here :( - if kwargs.get("internal_metadata", {}).get("soft_failed", False): - event.internal_metadata.soft_failed = True - if kwargs.get("internal_metadata", {}).get("policy_server_spammy", False): - event.internal_metadata.policy_server_spammy = True + if internal_metadata: + for key, value in internal_metadata.items(): + # Note: this calls the relevant `#[setter]` function in the (Rust) event class' + # internal metadata struct. + # Will reject unknown keys with exceptions. + # This is desirable for our test suite anyway. + setattr(event.internal_metadata, key, value) context = await unpersisted_context.persist(event) From 943da0ace86cc1d608696ce23960269f75d87f5f Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 15 Apr 2026 15:55:46 +0000 Subject: [PATCH 3/7] Fix database migrations failing on platforms where SQLite is configured with `SQLITE_DBCONFIG_DEFENSIVE` by default, such as macOS. (#19690) Fixes: #19616 This caused 2+ people trouble now, so worth batting away with a low-effort change if we can. Only seen on macOS so far, but nothing stops SQLite being configured in defensive mode by default on other platforms, so it is not necessarily entirely specific to macOS. We *could* also do this for Python < 3.12 but it'd be more effort and I don't know if it's worth it. (For context @kegsay says the interpreter with this problem was installed through `pyenv install`.) --------- Signed-off-by: Olivier 'reivilibre --- changelog.d/19690.bugfix | 1 + docs/development/contributing_guide.md | 15 --------------- synapse/storage/engines/sqlite.py | 15 +++++++++++++++ 3 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 changelog.d/19690.bugfix diff --git a/changelog.d/19690.bugfix b/changelog.d/19690.bugfix new file mode 100644 index 0000000000..3aa0dd11f3 --- /dev/null +++ b/changelog.d/19690.bugfix @@ -0,0 +1 @@ +Fix database migrations failing on platforms where SQLite is configured with `SQLITE_DBCONFIG_DEFENSIVE` by default, such as macOS. \ No newline at end of file diff --git a/docs/development/contributing_guide.md b/docs/development/contributing_guide.md index 7e0dd4059d..ab506b5f05 100644 --- a/docs/development/contributing_guide.md +++ b/docs/development/contributing_guide.md @@ -309,21 +309,6 @@ works without further arguments). Your Postgres account needs to be able to create databases; see the postgres docs for [`ALTER ROLE`](https://www.postgresql.org/docs/current/sql-alterrole.html). -### Running the tests on MacOS - -To run the unit tests with SQLite on MacOS you will need to swap out the default MacOS -python interpreter with the homebrew version. This is due to the default using a -locked down version of Python and sqlite3. After installing python from homebrew: - -``` - poetry env remove --all - poetry env use /opt/homebrew/bin/python3.13 - poetry install - poetry run trial tests -``` - -This example uses python 3.13, but choose whichever version you want. - ## Run the integration tests ([Sytest](https://github.com/matrix-org/sytest)). The integration tests are a more comprehensive suite of tests. They diff --git a/synapse/storage/engines/sqlite.py b/synapse/storage/engines/sqlite.py index 3b1b19c00e..316e188dc9 100644 --- a/synapse/storage/engines/sqlite.py +++ b/synapse/storage/engines/sqlite.py @@ -21,6 +21,7 @@ import platform import sqlite3 import struct +import sys import threading from typing import TYPE_CHECKING, Any, Mapping @@ -90,6 +91,20 @@ class Sqlite3Engine(BaseDatabaseEngine[sqlite3.Connection, sqlite3.Cursor]): # We need to import here to avoid an import loop. from synapse.storage.prepare_database import prepare_database + if sys.version_info >= (3, 12): + # Opportunistically disable the SQLITE_DBCONFIG_DEFENSIVE + # flag on the database, as some of our database migrations + # alter the schema and this is forbidden in defensive mode. + # + # This is only known to be necessary on macOS, though SQLite can + # in theory be configured to be defensive by default on any + # platform. + # + # The constant for this is only exposed in the sqlite3 module + # on Python >= 3.12. + assert isinstance(db_conn.conn, sqlite3.Connection) + db_conn.conn.setconfig(sqlite3.SQLITE_DBCONFIG_DEFENSIVE, False) + if self._is_in_memory: # In memory databases need to be rebuilt each time. Ideally we'd # reuse the same connection as we do when starting up, but that From 71781de7074390cc7ca532505eca8f8abbdb7ca2 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 Apr 2026 09:47:08 +0100 Subject: [PATCH 4/7] Add a `FilteredEvent` type to handle per-user data on events (#19640) When we return events to clients we need to annotate them with the membership of the user at the time of the event, in the `unsigned` section. We already check the membership at the event during the visibility checks, and so we annotate events there. However, since this a per-user field we end up having to clone the event in question. Instead, let's add a `FilteredEvent` class that is returned by the visibility checks, which allows returning the membership without editing the event. This has three benefits: 1. Avoids the clones of the event. 2. Allows us to statically check that we have filtered events before returning them to clients. 3. We no longer edit `unsigned` data after event deserialization, this makes it easier to port the event class to Rust. The last benefit is why we're doing this *now*, however IMV it shouldn't affect whether we want this change or not. Reviewable commit-by-commit --------- Co-authored-by: Olivier 'reivilibre' --- changelog.d/19640.misc | 1 + synapse/app/admin_cmd.py | 11 +- synapse/appservice/api.py | 4 +- synapse/events/utils.py | 107 +++++++++++++++++--- synapse/handlers/admin.py | 29 ++++-- synapse/handlers/events.py | 17 ++-- synapse/handlers/initial_sync.py | 28 +++-- synapse/handlers/message.py | 8 +- synapse/handlers/pagination.py | 19 ++-- synapse/handlers/relations.py | 32 +++--- synapse/handlers/room.py | 40 ++++---- synapse/handlers/search.py | 47 ++++----- synapse/handlers/sliding_sync/__init__.py | 35 ++++--- synapse/handlers/sliding_sync/extensions.py | 2 +- synapse/handlers/sync.py | 97 ++++++++++-------- synapse/handlers/thread_subscriptions.py | 10 +- synapse/notifier.py | 5 +- synapse/push/mailer.py | 6 +- synapse/rest/admin/events.py | 5 +- synapse/rest/admin/rooms.py | 8 +- synapse/rest/client/notifications.py | 3 +- synapse/rest/client/room.py | 9 +- synapse/rest/client/sync.py | 28 +++-- synapse/types/handlers/sliding_sync.py | 3 +- synapse/visibility.py | 34 ++----- tests/events/test_auto_accept_invites.py | 8 +- tests/events/test_utils.py | 3 +- tests/handlers/test_admin.py | 12 ++- tests/handlers/test_sync.py | 28 ++--- tests/rest/client/test_retention.py | 11 +- tests/test_visibility.py | 31 ++---- 31 files changed, 414 insertions(+), 267 deletions(-) create mode 100644 changelog.d/19640.misc diff --git a/changelog.d/19640.misc b/changelog.d/19640.misc new file mode 100644 index 0000000000..f13d8f6753 --- /dev/null +++ b/changelog.d/19640.misc @@ -0,0 +1 @@ +Add a `FilteredEvent` class that saves us copying events. diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 0614c805da..67189e91e7 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -34,6 +34,7 @@ from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig from synapse.config.logger import setup_logging from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.handlers.admin import ExfiltrationWriter from synapse.server import HomeServer from synapse.storage.database import DatabasePool, LoggingDatabaseConnection @@ -150,14 +151,14 @@ class FileExfiltrationWriter(ExfiltrationWriter): if list(os.listdir(self.base_directory)): raise Exception("Directory must be empty") - def write_events(self, room_id: str, events: list[EventBase]) -> None: + def write_events(self, room_id: str, filtered_events: list[FilteredEvent]) -> None: room_directory = os.path.join(self.base_directory, "rooms", room_id) os.makedirs(room_directory, exist_ok=True) events_file = os.path.join(room_directory, "events") with open(events_file, "a") as f: - for event in events: - json.dump(event.get_pdu_json(), fp=f) + for filtered_event in filtered_events: + json.dump(filtered_event.event.get_pdu_json(), fp=f) def write_state( self, room_id: str, event_id: str, state: StateMap[EventBase] @@ -175,7 +176,7 @@ class FileExfiltrationWriter(ExfiltrationWriter): def write_invite( self, room_id: str, event: EventBase, state: StateMap[EventBase] ) -> None: - self.write_events(room_id, [event]) + self.write_events(room_id, [FilteredEvent.state(event)]) # We write the invite state somewhere else as they aren't full events # and are only a subset of the state at the event. @@ -191,7 +192,7 @@ class FileExfiltrationWriter(ExfiltrationWriter): def write_knock( self, room_id: str, event: EventBase, state: StateMap[EventBase] ) -> None: - self.write_events(room_id, [event]) + self.write_events(room_id, [FilteredEvent.state(event)]) # We write the knock state somewhere else as they aren't full events # and are only a subset of the state at the event. diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index d4e9d50b96..66c962e17d 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -40,7 +40,7 @@ from synapse.appservice import ( TransactionUnusedFallbackKeys, ) from synapse.events import EventBase -from synapse.events.utils import SerializeEventConfig +from synapse.events.utils import FilteredEvent, SerializeEventConfig from synapse.http.client import SimpleHttpClient, is_unknown_endpoint from synapse.logging import opentracing from synapse.metrics import SERVER_NAME_LABEL @@ -545,7 +545,7 @@ class ApplicationServiceApi(SimpleHttpClient): ) -> list[JsonDict]: time_now = self.clock.time_msec() return await self._event_serializer.serialize_events( - list(events), + [FilteredEvent(event=e, membership=None) for e in events], time_now, config=SerializeEventConfig( as_client_event=True, diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 76ebac8b17..ff0476f5fb 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -41,6 +41,7 @@ from synapse.api.constants import ( MAX_PDU_SIZE, EventContentFields, EventTypes, + EventUnsignedContentFields, RelationTypes, ) from synapse.api.errors import Codes, SynapseError @@ -416,6 +417,50 @@ def format_event_for_client_v2_without_room_id(d: JsonDict) -> JsonDict: return d +@attr.s(slots=True, frozen=True, auto_attribs=True) +class FilteredEvent: + """An event annotated with per-user data for client serialization. + + Produced by filter_and_transform_events_for_client. Carries the user's + membership at the time of the event so serialization can inject it into + unsigned.membership (MSC4115) without cloning the underlying event. + """ + + event: "EventBase" + """The event to be serialized.""" + + membership: str | None + """The user whose requesting the event's membership at the time of the + event was sent. + + This is None if we didn't compute the membership. In Synapse this happens a) + when returning state events to state endpoints, or b) when the event is + returned to an admin. + + According to the spec we don't have to include the membership for any events + if we don't want to, especially if its expensive to compute. In practice + clients really only care about events in the room timeline so that in + encrypted room they can determine if they should be able to decrypt the + event or not. + """ + + @classmethod + def state(cls, event: "EventBase") -> "FilteredEvent": + """Wrap a state event with no per-user membership annotation. + + The event must be a state event (i.e. have a state_key). + """ + assert event.is_state(), ( + f"FilteredEvent.state() called with non-state event {event.event_id}" + ) + return cls(event=event, membership=None) + + @classmethod + def admin_override(cls, event: "EventBase") -> "FilteredEvent": + """Wrap an event that bypasses visibility filtering due to admin privileges.""" + return cls(event=event, membership=None) + + @attr.s(slots=True, frozen=True, auto_attribs=True) class SerializeEventConfig: as_client_event: bool = True @@ -435,6 +480,9 @@ class SerializeEventConfig: # only server admins can see through other configuration. For example, # whether an event was soft failed by the server. include_admin_metadata: bool = False + # Whether MSC4354 (sticky events) is enabled. When True, the sticky TTL + # will be computed and included in the unsigned section of sticky events. + msc4354_enabled: bool = False @only_event_fields.validator def _validate_only_event_fields( @@ -461,6 +509,7 @@ def _serialize_event( time_now_ms: int, *, config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, + membership: str | None = None, ) -> JsonDict: """Serialize event for clients @@ -468,6 +517,8 @@ def _serialize_event( e time_now_ms config: Event serialization config + membership: The requesting user's membership at the time of the event, + to be injected into unsigned.membership (MSC4115). Returns: The serialized event dictionary. @@ -564,6 +615,23 @@ def _serialize_event( if e.internal_metadata.policy_server_spammy: d["unsigned"]["io.element.synapse.policy_server_spammy"] = True + if config.msc4354_enabled: + sticky_duration = e.sticky_duration() + if sticky_duration: + expires_at = ( + # min() ensures that the origin server can't lie about the time and + # send the event 'in the future', as that would allow them to exceed + # the 1 hour limit on stickiness duration. + min(e.origin_server_ts, time_now_ms) + sticky_duration.as_millis() + ) + if expires_at > time_now_ms: + d["unsigned"][EventUnsignedContentFields.STICKY_TTL] = ( + expires_at - time_now_ms + ) + + if membership is not None: + d["unsigned"][EventUnsignedContentFields.MEMBERSHIP] = membership + return d @@ -577,13 +645,15 @@ class EventClientSerializer: def __init__(self, hs: "HomeServer") -> None: self._store = hs.get_datastores().main self._auth = hs.get_auth() + self._config = hs.config + self._clock = hs.get_clock() self._add_extra_fields_to_unsigned_client_event_callbacks: list[ ADD_EXTRA_FIELDS_TO_UNSIGNED_CLIENT_EVENT_CALLBACK ] = [] async def serialize_event( self, - event: JsonDict | EventBase, + event: JsonDict | FilteredEvent, time_now: int, *, config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, @@ -605,7 +675,7 @@ class EventClientSerializer: The serialized event """ # To handle the case of presence events and the like - if not isinstance(event, EventBase): + if not isinstance(event, FilteredEvent): return event # Force-enable server admin metadata because the only time an event with @@ -617,11 +687,16 @@ class EventClientSerializer: ): config = make_config_for_admin(config) - serialized_event = _serialize_event(event, time_now, config=config) + if self._config.experimental.msc4354_enabled: + config = attr.evolve(config, msc4354_enabled=True) + + serialized_event = _serialize_event( + event.event, time_now, config=config, membership=event.membership + ) # If the event was redacted, fetch the redaction event from the database # and include it in the serialized event's unsigned section. - redacted_by: str | None = event.internal_metadata.redacted_by + redacted_by: str | None = event.event.internal_metadata.redacted_by if redacted_by is not None: serialized_event.setdefault("unsigned", {})["redacted_by"] = redacted_by if redaction_map is not None: @@ -648,7 +723,7 @@ class EventClientSerializer: new_unsigned = {} for callback in self._add_extra_fields_to_unsigned_client_event_callbacks: - u = await callback(event) + u = await callback(event.event) new_unsigned.update(u) if new_unsigned: @@ -666,9 +741,9 @@ class EventClientSerializer: # Check if there are any bundled aggregations to include with the event. if bundle_aggregations: - if event.event_id in bundle_aggregations: + if event.event.event_id in bundle_aggregations: await self._inject_bundled_aggregations( - event, + event.event, time_now, config, bundle_aggregations, @@ -720,7 +795,7 @@ class EventClientSerializer: # `sender` of the edit; however MSC3925 proposes extending it to the whole # of the edit, which is what we do here. serialized_aggregations[RelationTypes.REPLACE] = await self.serialize_event( - event_aggregations.replace, + FilteredEvent(event=event_aggregations.replace, membership=None), time_now, config=config, ) @@ -730,7 +805,7 @@ class EventClientSerializer: thread = event_aggregations.thread serialized_latest_event = await self.serialize_event( - thread.latest_event, + FilteredEvent(event=thread.latest_event, membership=None), time_now, config=config, bundle_aggregations=bundled_aggregations, @@ -755,7 +830,7 @@ class EventClientSerializer: @trace async def serialize_events( self, - events: Collection[JsonDict | EventBase], + events: Collection[JsonDict | FilteredEvent], time_now: int, *, config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, @@ -780,11 +855,13 @@ class EventClientSerializer: ) # Batch-fetch all redaction events in one go rather than one per event. - redaction_ids = { - e.internal_metadata.redacted_by - for e in events - if isinstance(e, EventBase) and e.internal_metadata.redacted_by is not None - } + redaction_ids: set[str] = set() + for e in events: + base = e.event if isinstance(e, FilteredEvent) else e + if isinstance(base, EventBase): + redacted_by = base.internal_metadata.redacted_by + if redacted_by is not None: + redaction_ids.add(redacted_by) redaction_map = ( await self._store.get_events(redaction_ids) if redaction_ids else {} ) diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py index 2fb0e5814f..d2c1f98d7c 100644 --- a/synapse/handlers/admin.py +++ b/synapse/handlers/admin.py @@ -33,6 +33,7 @@ import attr from synapse.api.constants import Direction, EventTypes, Membership from synapse.api.errors import SynapseError from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.types import ( JsonMapping, Requester, @@ -251,32 +252,40 @@ class AdminHandler: topological=last_event.depth, ) - events = await filter_and_transform_events_for_client( + filtered_events = await filter_and_transform_events_for_client( self._storage_controllers, user_id, events, ) - writer.write_events(room_id, events) + writer.write_events(room_id, filtered_events) # Update the extremity tracking dicts - for event in events: + for filtered_event in filtered_events: # Check if we have any prev events that haven't been # processed yet, and add those to the appropriate dicts. - unseen_events = set(event.prev_event_ids()) - written_events + unseen_events = ( + set(filtered_event.event.prev_event_ids()) - written_events + ) if unseen_events: - event_to_unseen_prevs[event.event_id] = unseen_events + event_to_unseen_prevs[filtered_event.event.event_id] = ( + unseen_events + ) for unseen in unseen_events: unseen_to_child_events.setdefault(unseen, set()).add( - event.event_id + filtered_event.event.event_id ) # Now check if this event is an unseen prev event, if so # then we remove this event from the appropriate dicts. - for child_id in unseen_to_child_events.pop(event.event_id, []): - event_to_unseen_prevs[child_id].discard(event.event_id) + for child_id in unseen_to_child_events.pop( + filtered_event.event.event_id, [] + ): + event_to_unseen_prevs[child_id].discard( + filtered_event.event.event_id + ) - written_events.add(event.event_id) + written_events.add(filtered_event.event.event_id) logger.info( "Written %d events in room %s", len(written_events), room_id @@ -511,7 +520,7 @@ class ExfiltrationWriter(metaclass=abc.ABCMeta): """Interface used to specify how to write exported data.""" @abc.abstractmethod - def write_events(self, room_id: str, events: list[EventBase]) -> None: + def write_events(self, room_id: str, events: list[FilteredEvent]) -> None: """Write a batch of events for a room.""" raise NotImplementedError() diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index f6517def9c..2518716bc7 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -25,8 +25,7 @@ from typing import TYPE_CHECKING, Iterable from synapse.api.constants import EduTypes, EventTypes, Membership, PresenceState from synapse.api.errors import AuthError, SynapseError -from synapse.events import EventBase -from synapse.events.utils import SerializeEventConfig +from synapse.events.utils import FilteredEvent, SerializeEventConfig from synapse.handlers.presence import format_user_presence_state from synapse.storage.databases.main.events_worker import EventRedactBehaviour from synapse.streams.config import PaginationConfig @@ -102,19 +101,19 @@ class EventStreamHandler: # joined room, we need to send down presence for those users. to_add: list[JsonDict] = [] for event in events: - if not isinstance(event, EventBase): + if not isinstance(event, FilteredEvent): continue - if event.type == EventTypes.Member: - if event.membership != Membership.JOIN: + if event.event.type == EventTypes.Member: + if event.event.membership != Membership.JOIN: continue # Send down presence. - if event.state_key == requester.user.to_string(): + if event.event.state_key == requester.user.to_string(): # Send down presence for everyone in the room. users: Iterable[str] = await self.store.get_users_in_room( - event.room_id + event.event.room_id ) else: - users = [event.state_key] + users = [event.event.state_key] states = await presence_handler.get_states(users) to_add.extend( @@ -155,7 +154,7 @@ class EventHandler: room_id: str | None, event_id: str, show_redacted: bool = False, - ) -> EventBase | None: + ) -> FilteredEvent | None: """Retrieve a single specified event on behalf of a user. The event will be transformed in a user-specific and time-specific way, e.g. having unsigned metadata added or being erased depending on who is accessing. diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 45477412ba..591a0aefd3 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -30,7 +30,7 @@ from synapse.api.constants import ( Membership, ) from synapse.api.errors import SynapseError -from synapse.events.utils import SerializeEventConfig +from synapse.events.utils import FilteredEvent, SerializeEventConfig from synapse.events.validator import EventValidator from synapse.handlers.presence import format_user_presence_state from synapse.handlers.receipts import ReceiptEventSource @@ -186,7 +186,7 @@ class InitialSyncHandler: invite_event = await self.store.get_event(event.event_id) d["invite"] = await self._event_serializer.serialize_event( - invite_event, + FilteredEvent.state(event=invite_event), time_now, config=serializer_options, ) @@ -225,7 +225,7 @@ class InitialSyncHandler: ) ).addErrback(unwrapFirstError) - messages = await filter_and_transform_events_for_client( + filtered_messages = await filter_and_transform_events_for_client( self._storage_controllers, user_id, messages, @@ -240,7 +240,7 @@ class InitialSyncHandler: d["messages"] = { "chunk": ( await self._event_serializer.serialize_events( - messages, + filtered_messages, time_now=time_now, config=serializer_options, ) @@ -250,7 +250,7 @@ class InitialSyncHandler: } d["state"] = await self._event_serializer.serialize_events( - current_state.values(), + [FilteredEvent.state(e) for e in current_state.values()], time_now=time_now, config=serializer_options, ) @@ -382,7 +382,9 @@ class InitialSyncHandler: room_id, limit=pagin_config.limit, end_token=stream_token ) - messages = await filter_and_transform_events_for_client( + filtered_messages: list[ + FilteredEvent + ] = await filter_and_transform_events_for_client( self._storage_controllers, requester.user.to_string(), messages, @@ -402,7 +404,7 @@ class InitialSyncHandler: "chunk": ( # Don't bundle aggregations as this is a deprecated API. await self._event_serializer.serialize_events( - messages, time_now, config=serialize_options + filtered_messages, time_now, config=serialize_options ) ), "start": await start_token.to_string(self.store), @@ -411,7 +413,9 @@ class InitialSyncHandler: "state": ( # Don't bundle aggregations as this is a deprecated API. await self._event_serializer.serialize_events( - room_state.values(), time_now, config=serialize_options + [FilteredEvent.state(e) for e in room_state.values()], + time_now, + config=serialize_options, ) ), "presence": [], @@ -435,7 +439,7 @@ class InitialSyncHandler: serialize_options = SerializeEventConfig(requester=requester) # Don't bundle aggregations as this is a deprecated API. state = await self._event_serializer.serialize_events( - current_state.values(), + [FilteredEvent.state(e) for e in current_state.values()], time_now, config=serialize_options, ) @@ -494,7 +498,9 @@ class InitialSyncHandler: ).addErrback(unwrapFirstError) ) - messages = await filter_and_transform_events_for_client( + filtered_messages: list[ + FilteredEvent + ] = await filter_and_transform_events_for_client( self._storage_controllers, requester.user.to_string(), messages, @@ -510,7 +516,7 @@ class InitialSyncHandler: "chunk": ( # Don't bundle aggregations as this is a deprecated API. await self._event_serializer.serialize_events( - messages, time_now, config=serialize_options + filtered_messages, time_now, config=serialize_options ) ), "start": await start_token.to_string(self.store), diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 80b037e98a..0aa0a16127 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -61,7 +61,11 @@ from synapse.events.snapshot import ( UnpersistedEventContext, UnpersistedEventContextBase, ) -from synapse.events.utils import SerializeEventConfig, maybe_upsert_event_field +from synapse.events.utils import ( + FilteredEvent, + SerializeEventConfig, + maybe_upsert_event_field, +) from synapse.events.validator import EventValidator from synapse.handlers.directory import DirectoryHandler from synapse.handlers.worker_lock import NEW_EVENT_DURING_PURGE_LOCK_NAME @@ -261,7 +265,7 @@ class MessageHandler: room_state = room_state_events[membership_event_id] events = await self._event_serializer.serialize_events( - room_state.values(), + [FilteredEvent.state(e) for e in room_state.values()], self.clock.time_msec(), config=SerializeEventConfig(requester=requester), ) diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 7b9c829056..8cbe4b63c8 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -29,6 +29,7 @@ from synapse.api.constants import Direction, EventTypes, Membership from synapse.api.errors import SynapseError from synapse.api.filtering import Filter from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.handlers.relations import BundledAggregations from synapse.handlers.worker_lock import NEW_EVENT_DURING_PURGE_LOCK_NAME from synapse.logging.opentracing import trace @@ -79,7 +80,7 @@ class GetMessagesResult: Everything needed to serialize a `/messages` response. """ - messages_chunk: list[EventBase] + messages_chunk: list[FilteredEvent] """ A list of room events. @@ -684,16 +685,18 @@ class PaginationHandler: events = await event_filter.filter(events) if not use_admin_priviledge: - events = await filter_and_transform_events_for_client( + filtered_events = await filter_and_transform_events_for_client( self._storage_controllers, user_id, events, is_peeking=(member_event_id is None), ) + else: + filtered_events = [FilteredEvent.admin_override(e) for e in events] # if after the filter applied there are no more events # return immediately - but there might be more in next_token batch - if not events: + if not filtered_events: return GetMessagesResult( messages_chunk=[], bundled_aggregations={}, @@ -703,16 +706,16 @@ class PaginationHandler: ) state = None - if event_filter and event_filter.lazy_load_members and len(events) > 0: + if event_filter and event_filter.lazy_load_members and len(filtered_events) > 0: # TODO: remove redundant members # FIXME: we also care about invite targets etc. state_filter = StateFilter.from_types( - (EventTypes.Member, event.sender) for event in events + (EventTypes.Member, event.event.sender) for event in filtered_events ) state_ids = await self._state_storage_controller.get_state_ids_for_event( - events[0].event_id, state_filter=state_filter + filtered_events[0].event.event_id, state_filter=state_filter ) if state_ids: @@ -720,11 +723,11 @@ class PaginationHandler: state = list(state_dict.values()) aggregations = await self._relations_handler.get_bundled_aggregations( - events, user_id + filtered_events, user_id ) return GetMessagesResult( - messages_chunk=events, + messages_chunk=filtered_events, bundled_aggregations=aggregations, state=state, start_token=from_token, diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py index d7d3002fbe..ee4f8d672e 100644 --- a/synapse/handlers/relations.py +++ b/synapse/handlers/relations.py @@ -33,7 +33,7 @@ import attr from synapse.api.constants import Direction, EventTypes, RelationTypes from synapse.api.errors import SynapseError from synapse.events import EventBase, relation_from_event -from synapse.events.utils import SerializeEventConfig +from synapse.events.utils import FilteredEvent, SerializeEventConfig from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.logging.opentracing import trace from synapse.storage.databases.main.relations import ThreadsNextBatch, _RelatedEvent @@ -139,7 +139,7 @@ class RelationsHandler: # not passing them in here we should get a better cache hit rate). related_events, next_token = await self._main_store.get_relations_for_event( event_id=event_id, - event=event, + event=event.event, room_id=room_id, relation_type=relation_type, event_type=event_type, @@ -154,7 +154,9 @@ class RelationsHandler: [e.event_id for e in related_events] ) - events = await filter_and_transform_events_for_client( + filtered_events: list[ + FilteredEvent + ] = await filter_and_transform_events_for_client( self._storage_controllers, user_id, events, @@ -164,14 +166,14 @@ class RelationsHandler: # The relations returned for the requested event do include their # bundled aggregations. aggregations = await self.get_bundled_aggregations( - events, requester.user.to_string() + filtered_events, requester.user.to_string() ) now = self._clock.time_msec() serialize_options = SerializeEventConfig(requester=requester) return_value: JsonDict = { "chunk": await self._event_serializer.serialize_events( - events, + filtered_events, now, bundle_aggregations=aggregations, config=serialize_options, @@ -389,7 +391,7 @@ class RelationsHandler: potential_events, _ = await self._main_store.get_relations_for_event( room_id, event_id, - event, + event.event, RelationTypes.THREAD, direction=Direction.FORWARDS, ) @@ -417,7 +419,7 @@ class RelationsHandler: potential_events[-1].event_id, ) continue - latest_thread_event = event + latest_thread_event = event.event results[event_id] = _ThreadAggregation( latest_event=latest_thread_event, @@ -432,12 +434,12 @@ class RelationsHandler: @trace async def get_bundled_aggregations( - self, events: Iterable[EventBase], user_id: str + self, filtered_events: Iterable[FilteredEvent], user_id: str ) -> dict[str, BundledAggregations]: """Generate bundled aggregations for events. Args: - events: The iterable of events to calculate bundled aggregations for. + filtered_events: The iterable of filtered events to calculate bundled aggregations for. user_id: The user requesting the bundled aggregations. Returns: @@ -453,7 +455,9 @@ class RelationsHandler: events_by_id = {} # A map of event ID to the relation in that event, if there is one. relations_by_id: dict[str, str] = {} - for event in events: + for filtered_event in filtered_events: + event = filtered_event.event + # State events do not get bundled aggregations. if event.is_state(): continue @@ -599,7 +603,9 @@ class RelationsHandler: # Limit the returned threads to those the user has participated in. events = [event for event in events if participated[event.event_id]] - events = await filter_and_transform_events_for_client( + filtered_events: list[ + FilteredEvent + ] = await filter_and_transform_events_for_client( self._storage_controllers, user_id, events, @@ -607,12 +613,12 @@ class RelationsHandler: ) aggregations = await self.get_bundled_aggregations( - events, requester.user.to_string() + filtered_events, requester.user.to_string() ) now = self._clock.time_msec() serialized_events = await self._event_serializer.serialize_events( - events, now, bundle_aggregations=aggregations + filtered_events, now, bundle_aggregations=aggregations ) return_value: JsonDict = {"chunk": serialized_events} diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 1c3489a00e..9074d7916b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -67,7 +67,7 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.event_auth import validate_event_for_room_version from synapse.events import EventBase from synapse.events.snapshot import UnpersistedEventContext -from synapse.events.utils import copy_and_fixup_power_levels_contents +from synapse.events.utils import FilteredEvent, copy_and_fixup_power_levels_contents from synapse.handlers.relations import BundledAggregations from synapse.rest.admin._base import assert_user_is_admin from synapse.streams import EventSource @@ -109,9 +109,9 @@ FIVE_MINUTES_IN_MS = 5 * 60 * 1000 @attr.s(slots=True, frozen=True, auto_attribs=True) class EventContext: - events_before: list[EventBase] - event: EventBase - events_after: list[EventBase] + events_before: list[FilteredEvent] + event: FilteredEvent + events_after: list[FilteredEvent] state: list[EventBase] aggregations: dict[str, BundledAggregations] start: str @@ -1916,9 +1916,9 @@ class RoomContextHandler: # The user is peeking if they aren't in the room already is_peeking = not is_user_in_room - async def filter_evts(events: list[EventBase]) -> list[EventBase]: + async def filter_evts(events: list[EventBase]) -> list[FilteredEvent]: if use_admin_priviledge: - return events + return [FilteredEvent.admin_override(e) for e in events] return await filter_and_transform_events_for_client( self._storage_controllers, user.to_string(), @@ -1946,31 +1946,33 @@ class RoomContextHandler: events_before = await event_filter.filter(events_before) events_after = await event_filter.filter(events_after) - events_before = await filter_evts(events_before) - events_after = await filter_evts(events_after) + filtered_events_before = await filter_evts(events_before) + filtered_events_after = await filter_evts(events_after) # filter_evts can return a pruned event in case the user is allowed to see that # there's something there but not see the content, so use the event that's in # `filtered` rather than the event we retrieved from the datastore. - event = filtered[0] + filtered_event = filtered[0] # Fetch the aggregations. aggregations = await self._relations_handler.get_bundled_aggregations( - itertools.chain(events_before, (event,), events_after), + itertools.chain( + filtered_events_before, (filtered_event,), filtered_events_after + ), user.to_string(), ) - if events_after: - last_event_id = events_after[-1].event_id + if filtered_events_after: + last_event_id = filtered_events_after[-1].event.event_id else: last_event_id = event_id if event_filter and event_filter.lazy_load_members: state_filter = StateFilter.from_lazy_load_member_list( - ev.sender + ev.event.sender for ev in itertools.chain( - events_before, - (event,), - events_after, + filtered_events_before, + (filtered_event,), + filtered_events_after, ) ) else: @@ -1993,9 +1995,9 @@ class RoomContextHandler: token = StreamToken.START return EventContext( - events_before=events_before, - event=event, - events_after=events_after, + events_before=filtered_events_before, + event=filtered_event, + events_after=filtered_events_after, state=state_events, aggregations=aggregations, start=await token.copy_and_replace( diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index 56c047b0e8..30e072d011 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -29,8 +29,7 @@ from unpaddedbase64 import decode_base64, encode_base64 from synapse.api.constants import EventTypes, Membership from synapse.api.errors import NotFoundError, SynapseError from synapse.api.filtering import Filter -from synapse.events import EventBase -from synapse.events.utils import SerializeEventConfig +from synapse.events.utils import FilteredEvent, SerializeEventConfig from synapse.types import JsonDict, Requester, StrCollection, StreamKeyType, UserID from synapse.types.state import StateFilter from synapse.visibility import filter_and_transform_events_for_client @@ -48,7 +47,7 @@ class _SearchResult: # A mapping of event ID to the rank of that event. rank_map: dict[str, int] # A list of the resulting events. - allowed_events: list[EventBase] + allowed_events: list[FilteredEvent] # A map of room ID to results. room_groups: dict[str, JsonDict] # A set of event IDs to highlight. @@ -355,12 +354,12 @@ class SearchHandler: state_results = {} if include_state: - for room_id in {e.room_id for e in search_result.allowed_events}: + for room_id in {e.event.room_id for e in search_result.allowed_events}: state = await self._storage_controllers.state.get_current_state(room_id) state_results[room_id] = list(state.values()) aggregations = await self._relations_handler.get_bundled_aggregations( - # Generate an iterable of EventBase for all the events that will be + # Generate an iterable of FilteredEvent for all the events that will be # returned, including contextual events. itertools.chain( # The events_before and events_after for each context. @@ -396,14 +395,14 @@ class SearchHandler: results = [ { - "rank": search_result.rank_map[e.event_id], + "rank": search_result.rank_map[e.event.event_id], "result": await self._event_serializer.serialize_event( e, time_now, bundle_aggregations=aggregations, config=serialize_options, ), - "context": contexts.get(e.event_id, {}), + "context": contexts.get(e.event.event_id, {}), } for e in search_result.allowed_events ] @@ -417,7 +416,9 @@ class SearchHandler: if state_results: rooms_cat_res["state"] = { room_id: await self._event_serializer.serialize_events( - state_events, time_now, config=serialize_options + [FilteredEvent.state(e) for e in state_events], + time_now, + config=serialize_options, ) for room_id, state_events in state_results.items() } @@ -485,19 +486,19 @@ class SearchHandler: filtered_events, ) - events.sort(key=lambda e: -rank_map[e.event_id]) + events.sort(key=lambda e: -rank_map[e.event.event_id]) allowed_events = events[: search_filter.limit] for e in allowed_events: rm = room_groups.setdefault( - e.room_id, {"results": [], "order": rank_map[e.event_id]} + e.event.room_id, {"results": [], "order": rank_map[e.event.event_id]} ) - rm["results"].append(e.event_id) + rm["results"].append(e.event.event_id) s = sender_group.setdefault( - e.sender, {"results": [], "order": rank_map[e.event_id]} + e.event.sender, {"results": [], "order": rank_map[e.event.event_id]} ) - s["results"].append(e.event_id) + s["results"].append(e.event.event_id) return ( _SearchResult( @@ -549,7 +550,7 @@ class SearchHandler: highlights = set() - room_events: list[EventBase] = [] + room_events: list[FilteredEvent] = [] i = 0 pagination_token = batch_token @@ -595,11 +596,11 @@ class SearchHandler: pagination_token = results[-1]["pagination_token"] for event in room_events: - group = room_groups.setdefault(event.room_id, {"results": []}) - group["results"].append(event.event_id) + group = room_groups.setdefault(event.event.room_id, {"results": []}) + group["results"].append(event.event.event_id) if room_events and len(room_events) >= search_filter.limit: - last_event_id = room_events[-1].event_id + last_event_id = room_events[-1].event.event_id pagination_token = results_map[last_event_id]["pagination_token"] # We want to respect the given batch group and group keys so @@ -632,7 +633,7 @@ class SearchHandler: async def _calculate_event_contexts( self, user: UserID, - allowed_events: list[EventBase], + allowed_events: list[FilteredEvent], before_limit: int, after_limit: int, include_profile: bool, @@ -658,7 +659,7 @@ class SearchHandler: contexts = {} for event in allowed_events: res = await self.store.get_events_around( - event.room_id, event.event_id, before_limit, after_limit + event.event.room_id, event.event.event_id, before_limit, after_limit ) logger.info( @@ -692,14 +693,14 @@ class SearchHandler: if include_profile: senders = { - ev.sender + ev.event.sender for ev in itertools.chain(events_before, [event], events_after) } if events_after: - last_event_id = events_after[-1].event_id + last_event_id = events_after[-1].event.event_id else: - last_event_id = event.event_id + last_event_id = event.event.event_id state_filter = StateFilter.from_types( [(EventTypes.Member, sender) for sender in senders] @@ -718,6 +719,6 @@ class SearchHandler: if s.type == EventTypes.Member and s.state_key in senders } - contexts[event.event_id] = context + contexts[event.event.event_id] = context return contexts diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 6feb6c292e..1cc587d4a7 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -23,7 +23,7 @@ from typing_extensions import assert_never from synapse.api.constants import Direction, EventTypes, Membership from synapse.events import EventBase -from synapse.events.utils import strip_event +from synapse.events.utils import FilteredEvent, strip_event from synapse.handlers.relations import BundledAggregations from synapse.handlers.sliding_sync.extensions import SlidingSyncExtensionHandler from synapse.handlers.sliding_sync.room_lists import ( @@ -679,7 +679,7 @@ class SlidingSyncHandler: # membership. Currently, we have to make all of these optional because # `invite`/`knock` rooms only have `stripped_state`. See # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1653045932 - timeline_events: list[EventBase] = [] + timeline_events: list[FilteredEvent] = [] bundled_aggregations: dict[str, BundledAggregations] | None = None limited: bool | None = None prev_batch_token: StreamToken | None = None @@ -739,7 +739,7 @@ class SlidingSyncHandler: # Use `stream_ordering` for updates else paginate_room_events_by_stream_ordering ) - timeline_events, new_room_key, limited = await pagination_method( + raw_timeline_events, new_room_key, limited = await pagination_method( room_id=room_id, # The bounds are reversed so we can paginate backwards # (from newer to older events) starting at to_bound. @@ -752,13 +752,13 @@ class SlidingSyncHandler: # We want to return the events in ascending order (the last event is the # most recent). - timeline_events.reverse() + raw_timeline_events.reverse() # Make sure we don't expose any events that the client shouldn't see timeline_events = await filter_and_transform_events_for_client( self.storage_controllers, user.to_string(), - timeline_events, + raw_timeline_events, is_peeking=room_membership_for_user_at_to_token.membership != Membership.JOIN, filter_send_to_client=True, @@ -778,12 +778,17 @@ class SlidingSyncHandler: if from_token is not None: for timeline_event in reversed(timeline_events): # This fields should be present for all persisted events - assert timeline_event.internal_metadata.stream_ordering is not None - assert timeline_event.internal_metadata.instance_name is not None + assert ( + timeline_event.event.internal_metadata.stream_ordering + is not None + ) + assert ( + timeline_event.event.internal_metadata.instance_name is not None + ) persisted_position = PersistedEventPosition( - instance_name=timeline_event.internal_metadata.instance_name, - stream=timeline_event.internal_metadata.stream_ordering, + instance_name=timeline_event.event.internal_metadata.instance_name, + stream=timeline_event.event.internal_metadata.stream_ordering, ) if persisted_position.persisted_after( from_token.stream_token.room_key @@ -1061,13 +1066,13 @@ class SlidingSyncHandler: if timeline_events is not None: for timeline_event in timeline_events: # Anyone who sent a message is relevant - timeline_membership.add(timeline_event.sender) + timeline_membership.add(timeline_event.event.sender) # We also care about invite, ban, kick, targets, # etc. - if timeline_event.type == EventTypes.Member: + if timeline_event.event.type == EventTypes.Member: timeline_membership.add( - timeline_event.state_key + timeline_event.event.state_key ) # The client needs to know the membership of everyone in @@ -1480,7 +1485,7 @@ class SlidingSyncHandler: self, room_id: str, to_token: StreamToken, - timeline: list[EventBase], + timeline: list[FilteredEvent], check_outside_timeline: bool, ) -> int | None: """Get a bump stamp for the room, if we have a bump event and it has @@ -1500,8 +1505,8 @@ class SlidingSyncHandler: # those matches. We iterate backwards and take the stream ordering # of the first event that matches the bump event types. for timeline_event in reversed(timeline): - if timeline_event.type in SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES: - new_bump_stamp = timeline_event.internal_metadata.stream_ordering + if timeline_event.event.type in SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES: + new_bump_stamp = timeline_event.event.internal_metadata.stream_ordering # All persisted events have a stream ordering assert new_bump_stamp is not None diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 9b7a01df14..4a324e9661 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -761,7 +761,7 @@ class SlidingSyncExtensionHandler: # in the timeline to avoid bloating and blowing up the sync response # as the number of users in the room increases. (this behavior is part of the spec) initial_rooms_and_event_ids = [ - (room_id, event.event_id) + (room_id, event.event.event_id) for room_id in initial_rooms if room_id in actual_room_response_map for event in actual_room_response_map[room_id].timeline_events diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index c8ef5e2aa6..c88f703ae9 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -43,6 +43,7 @@ from synapse.api.filtering import FilterCollection from synapse.api.presence import UserPresenceState from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.handlers.relations import BundledAggregations from synapse.logging import issue9533_logger from synapse.logging.context import current_context @@ -123,7 +124,7 @@ class SyncConfig: @attr.s(slots=True, frozen=True, auto_attribs=True) class TimelineBatch: prev_batch: StreamToken - events: Sequence[EventBase] + events: Sequence[FilteredEvent] limited: bool # A mapping of event ID to the bundled aggregations for the above events. # This is only calculated if limited is true. @@ -148,7 +149,7 @@ class JoinedSyncResult: state: StateMap[EventBase] ephemeral: list[JsonDict] account_data: list[JsonDict] - sticky: list[EventBase] + sticky: list[FilteredEvent] unread_notifications: JsonDict unread_thread_notifications: JsonDict summary: JsonDict | None @@ -699,6 +700,7 @@ class SyncHandler: log_kv({"limited": limited}) + filtered_recents: list[FilteredEvent] if potential_recents: recents = await sync_config.filter_collection.filter_room_timeline( potential_recents @@ -725,29 +727,32 @@ class SyncHandler: ) ) - recents = await filter_and_transform_events_for_client( + filtered_recents = await filter_and_transform_events_for_client( self._storage_controllers, sync_config.user.to_string(), recents, always_include_ids=current_state_ids, ) - log_kv({"recents_after_visibility_filtering": len(recents)}) + log_kv({"recents_after_visibility_filtering": len(filtered_recents)}) else: - recents = [] + filtered_recents = [] if not limited or block_all_timeline: prev_batch_token = upto_token - if recents: - assert recents[0].internal_metadata.stream_ordering + if filtered_recents: + assert filtered_recents[0].event.internal_metadata.stream_ordering room_key = RoomStreamToken( - stream=recents[0].internal_metadata.stream_ordering - 1 + stream=filtered_recents[ + 0 + ].event.internal_metadata.stream_ordering + - 1 ) prev_batch_token = upto_token.copy_and_replace( StreamKeyType.ROOM, room_key ) return TimelineBatch( - events=recents, prev_batch=prev_batch_token, limited=False + events=filtered_recents, prev_batch=prev_batch_token, limited=False ) filtering_factor = 2 @@ -764,7 +769,7 @@ class SyncHandler: elif since_token and not newly_joined_room: since_key = since_token.room_key - while limited and len(recents) < timeline_limit and max_repeat: + while limited and len(filtered_recents) < timeline_limit and max_repeat: # For initial `/sync`, we want to view a historical section of the # timeline; to fetch events by `topological_ordering` (best # representation of the room DAG as others were seeing it at the time). @@ -835,26 +840,35 @@ class SyncHandler: ) ) - loaded_recents = await filter_and_transform_events_for_client( + loaded_filtered_recents: list[ + FilteredEvent + ] = await filter_and_transform_events_for_client( self._storage_controllers, sync_config.user.to_string(), loaded_recents, always_include_ids=current_state_ids, ) - log_kv({"loaded_recents_after_client_filtering": len(loaded_recents)}) + log_kv( + { + "loaded_recents_after_client_filtering": len( + loaded_filtered_recents + ) + } + ) - loaded_recents.extend(recents) - recents = loaded_recents + loaded_filtered_recents.extend(filtered_recents) + filtered_recents = loaded_filtered_recents max_repeat -= 1 - if len(recents) > timeline_limit: + if len(filtered_recents) > timeline_limit: limited = True - recents = recents[-timeline_limit:] - assert recents[0].internal_metadata.stream_ordering + filtered_recents = filtered_recents[-timeline_limit:] + assert filtered_recents[0].event.internal_metadata.stream_ordering room_key = RoomStreamToken( - stream=recents[0].internal_metadata.stream_ordering - 1 + stream=filtered_recents[0].event.internal_metadata.stream_ordering + - 1 ) prev_batch_token = upto_token.copy_and_replace(StreamKeyType.ROOM, room_key) @@ -865,12 +879,12 @@ class SyncHandler: if limited or newly_joined_room: bundled_aggregations = ( await self._relations_handler.get_bundled_aggregations( - recents, sync_config.user.to_string() + filtered_recents, sync_config.user.to_string() ) ) return TimelineBatch( - events=recents, + events=filtered_recents, prev_batch=prev_batch_token, # Also mark as limited if this is a new room or there has been a gap # (to force client to paginate the gap). @@ -976,8 +990,8 @@ class SyncHandler: # ...or ones which are in the timeline... for ev in batch.events: - if ev.type == EventTypes.Member: - existing_members.add(ev.state_key) + if ev.event.type == EventTypes.Member: + existing_members.add(ev.event.state_key) # ...and then ensure any missing ones get included in state. missing_hero_event_ids = [ @@ -1084,32 +1098,34 @@ class SyncHandler: first_event_by_sender_map = {} for event in batch.events: # Build the map from user IDs to the first timeline event they sent. - if event.sender not in first_event_by_sender_map: - first_event_by_sender_map[event.sender] = event + if event.event.sender not in first_event_by_sender_map: + first_event_by_sender_map[event.event.sender] = event.event # When using `state_after`, there is no special treatment with # regards to state also being in the `timeline`. Always fetch # relevant membership regardless of whether the state event is in # the `timeline`. if sync_config.use_state_after: - members_to_fetch.add(event.sender) + members_to_fetch.add(event.event.sender) # For `state`, the client is supposed to do a flawed re-construction # of state over time by starting with the given `state` and layering # on state from the `timeline` as you go (flawed because state # resolution). In this case, we only need their membership in # `state` when their membership isn't already in the `timeline`. - elif (EventTypes.Member, event.sender) not in timeline_state: - members_to_fetch.add(event.sender) + elif (EventTypes.Member, event.event.sender) not in timeline_state: + members_to_fetch.add(event.event.sender) # FIXME: we also care about invite targets etc. - if event.is_state(): - timeline_state[(event.type, event.state_key)] = event.event_id + if event.event.is_state(): + timeline_state[(event.event.type, event.event.state_key)] = ( + event.event.event_id + ) else: timeline_state = { - (event.type, event.state_key): event.event_id + (event.event.type, event.event.state_key): event.event.event_id for event in batch.events - if event.is_state() + if event.event.is_state() } # Now calculate the state to return in the sync response for the room. @@ -1340,7 +1356,7 @@ class SyncHandler: # timeline, but that is good enough here. state_at_timeline_start = ( await self._state_storage_controller.get_state_ids_for_event( - batch.events[0].event_id, + batch.events[0].event.event_id, state_filter=state_filter, await_full_state=await_full_state, ) @@ -1470,10 +1486,10 @@ class SyncHandler: prev_event_id = last_event_id_prev_batch for e in batch.events: - if e.prev_event_ids() != [prev_event_id]: + if e.event.prev_event_ids() != [prev_event_id]: is_linear_timeline = False break - prev_event_id = e.event_id + prev_event_id = e.event.event_id if is_linear_timeline and not batch.limited: state_ids: StateMap[str] = {} @@ -1487,7 +1503,7 @@ class SyncHandler: state_ids = ( await self._state_storage_controller.get_state_ids_for_event( - batch.events[0].event_id, + batch.events[0].event.event_id, # we only want members! state_filter=StateFilter.from_types( (EventTypes.Member, member) @@ -1501,7 +1517,7 @@ class SyncHandler: if batch: state_at_timeline_start = ( await self._state_storage_controller.get_state_ids_for_event( - batch.events[0].event_id, + batch.events[0].event.event_id, state_filter=state_filter, await_full_state=await_full_state, ) @@ -2854,7 +2870,7 @@ class SyncHandler: # if there are membership changes in the timeline, or # if membership has changed during a gappy sync, or # if this is an initial sync. - any(ev.type == EventTypes.Member for ev in batch.events) + any(ev.event.type == EventTypes.Member for ev in batch.events) or ( # XXX: this may include false positives in the form of LL # members which have snuck into state @@ -2870,7 +2886,7 @@ class SyncHandler: if room_builder.rtype == "joined": unread_notifications: dict[str, int] = {} - sticky_events: list[EventBase] = [] + sticky_events: list[FilteredEvent] = [] if sticky_event_ids: # As per MSC4354: # Remove sticky events that are already in the timeline, else we will needlessly duplicate @@ -2880,7 +2896,7 @@ class SyncHandler: # This is particularly important given the risk of sticky events spam since # anyone can send sticky events, so halving the bandwidth on average for each sticky # event is helpful. - timeline_event_id_set = {ev.event_id for ev in batch.events} + timeline_event_id_set = {ev.event.event_id for ev in batch.events} # Must preserve sticky event stream order sticky_event_ids = [ e for e in sticky_event_ids if e not in timeline_event_id_set @@ -3144,7 +3160,8 @@ class SyncResultBuilder: if self.since_token: for joined_sync in self.joined: it = itertools.chain( - joined_sync.state.values(), joined_sync.timeline.events + joined_sync.state.values(), + (e.event for e in joined_sync.timeline.events), ) for event in it: if event.type == EventTypes.Member: diff --git a/synapse/handlers/thread_subscriptions.py b/synapse/handlers/thread_subscriptions.py index 539672c7fe..29cb045d00 100644 --- a/synapse/handlers/thread_subscriptions.py +++ b/synapse/handlers/thread_subscriptions.py @@ -53,7 +53,7 @@ class ThreadSubscriptionsHandler: raise NotFoundError("No such thread root") return await self.store.get_subscription_for_thread( - user_id.to_string(), event.room_id, thread_root_event_id + user_id.to_string(), event.event.room_id, thread_root_event_id ) async def subscribe_user_to_thread( @@ -103,7 +103,7 @@ class ThreadSubscriptionsHandler: ) if autosub_cause_event is None: raise NotFoundError("Automatic subscription event not found") - relation = relation_from_event(autosub_cause_event) + relation = relation_from_event(autosub_cause_event.event) if ( relation is None or relation.rel_type != RelationTypes.THREAD @@ -115,7 +115,9 @@ class ThreadSubscriptionsHandler: errcode=Codes.MSC4306_NOT_IN_THREAD, ) - automatic_event_orderings = EventOrderings.from_event(autosub_cause_event) + automatic_event_orderings = EventOrderings.from_event( + autosub_cause_event.event + ) else: automatic_event_orderings = None @@ -174,7 +176,7 @@ class ThreadSubscriptionsHandler: outcome = await self.store.unsubscribe_user_from_thread( user_id.to_string(), - event.room_id, + event.event.room_id, thread_root_event_id, ) diff --git a/synapse/notifier.py b/synapse/notifier.py index 93d438def7..f1cec74462 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -41,6 +41,7 @@ from twisted.internet.defer import Deferred from synapse.api.constants import EduTypes, EventTypes, HistoryVisibility, Membership from synapse.api.errors import AuthError from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.handlers.presence import format_user_presence_state from synapse.logging import issue9533_logger from synapse.logging.context import PreserveLoggingContext @@ -210,7 +211,7 @@ class _NotifierUserStream: @attr.s(slots=True, frozen=True, auto_attribs=True) class EventStreamResult: - events: list[JsonDict | EventBase] + events: list[JsonDict | FilteredEvent] start_token: StreamToken end_token: StreamToken @@ -765,7 +766,7 @@ class Notifier: # The events fetched from each source are a JsonDict, EventBase, or # UserPresenceState, but see below for UserPresenceState being # converted to JsonDict. - events: list[JsonDict | EventBase] = [] + events: list[JsonDict | FilteredEvent] = [] end_token = from_token for keyname, source in self.event_sources.sources.get_sources(): diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index d18630e80b..1ebbc6d4f3 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -543,8 +543,10 @@ class Mailer: results.events_before + [notif_event], ) - for event in the_events: - messagevars = await self._get_message_vars(notif, event, room_state_ids) + for filtered_event in the_events: + messagevars = await self._get_message_vars( + notif, filtered_event.event, room_state_ids + ) if messagevars is not None: ret["messages"].append(messagevars) diff --git a/synapse/rest/admin/events.py b/synapse/rest/admin/events.py index 8da7a67820..1c311b0471 100644 --- a/synapse/rest/admin/events.py +++ b/synapse/rest/admin/events.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING from synapse.api.errors import NotFoundError from synapse.events.utils import ( + FilteredEvent, SerializeEventConfig, format_event_raw, ) @@ -66,7 +67,9 @@ class EventRestServlet(RestServlet): ) res = { "event": await self._event_serializer.serialize_event( - event, self._clock.time_msec(), config=config + FilteredEvent.admin_override(event), + self._clock.time_msec(), + config=config, ) } diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index a886859ffa..61511b9360 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -29,6 +29,7 @@ from synapse.api.constants import Direction, EventTypes, JoinRules, Membership from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError from synapse.api.filtering import Filter from synapse.events.utils import ( + FilteredEvent, SerializeEventConfig, ) from synapse.handlers.pagination import ( @@ -529,7 +530,9 @@ class RoomStateRestServlet(RestServlet): ) events = await self.store.get_events(event_ids.values()) now = self.clock.time_msec() - room_state = await self._event_serializer.serialize_events(events.values(), now) + room_state = await self._event_serializer.serialize_events( + [FilteredEvent.state(e) for e in events.values()], now + ) ret = {"state": room_state} return HTTPStatus.OK, ret @@ -897,7 +900,8 @@ class RoomEventContextServlet(RestServlet): bundle_aggregations=event_context.aggregations, ), "state": await self._event_serializer.serialize_events( - event_context.state, time_now + [FilteredEvent.state(e) for e in event_context.state], + time_now, ), "start": event_context.start, "end": event_context.end, diff --git a/synapse/rest/client/notifications.py b/synapse/rest/client/notifications.py index 2420e9fffb..f80a43b297 100644 --- a/synapse/rest/client/notifications.py +++ b/synapse/rest/client/notifications.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING from synapse.api.constants import ReceiptTypes from synapse.events.utils import ( + FilteredEvent, SerializeEventConfig, format_event_for_client_v2_without_room_id, ) @@ -111,7 +112,7 @@ class NotificationsServlet(RestServlet): "ts": pa.received_ts, "event": ( await self._event_serializer.serialize_event( - notif_events[pa.event_id], + FilteredEvent(event=notif_events[pa.event_id], membership=None), now, config=serialize_options, ) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 65d9c130ef..83664814a6 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -53,6 +53,7 @@ from synapse.api.errors import ( from synapse.api.filtering import Filter from synapse.events.utils import ( EventClientSerializer, + FilteredEvent, SerializeEventConfig, format_event_for_client_v2, ) @@ -286,7 +287,7 @@ class RoomStateEventRestServlet(RestServlet): if format == "event": event = await self._event_serializer.serialize_event( - data, + FilteredEvent.state(data), self.clock.time_msec(), config=SerializeEventConfig( event_format=format_event_for_client_v2, @@ -866,7 +867,9 @@ async def encode_messages_response( serialized_result[ "state" ] = await serialize_deps.event_serializer.serialize_events( - get_messages_result.state, time_now, config=serialize_options + [FilteredEvent.state(e) for e in get_messages_result.state], + time_now, + config=serialize_options, ) return serialized_result @@ -1172,7 +1175,7 @@ class RoomEventContextServlet(RestServlet): config=serializer_options, ), "state": await self._event_serializer.serialize_events( - event_context.state, + [FilteredEvent.state(e) for e in event_context.state], time_now, config=serializer_options, ), diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 710d097eab..c3cf0dc3c4 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -18,7 +18,6 @@ # [This file includes modifications made by New Vector Limited] # # -import itertools import logging from collections import defaultdict from typing import TYPE_CHECKING, Any, Mapping @@ -31,6 +30,7 @@ from synapse.api.filtering import FilterCollection from synapse.api.presence import UserPresenceState from synapse.api.ratelimiting import Ratelimiter from synapse.events.utils import ( + FilteredEvent, SerializeEventConfig, format_event_for_client_v2_without_room_id, format_event_raw, @@ -448,7 +448,9 @@ class SyncRestServlet(RestServlet): invited = {} for room in rooms: invite = await self._event_serializer.serialize_event( - room.invite, time_now, config=serialize_options + FilteredEvent.state(event=room.invite), + time_now, + config=serialize_options, ) unsigned = dict(invite.get("unsigned", {})) invite["unsigned"] = unsigned @@ -484,7 +486,9 @@ class SyncRestServlet(RestServlet): knocked = {} for room in rooms: knock = await self._event_serializer.serialize_event( - room.knock, time_now, config=serialize_options + FilteredEvent.state(event=room.knock), + time_now, + config=serialize_options, ) # Extract the `unsigned` key from the knock event. @@ -574,7 +578,7 @@ class SyncRestServlet(RestServlet): state_events = state_dict.values() - for event in itertools.chain(state_events, timeline_events): + for event in state_events: # We've had bug reports that events were coming down under the # wrong room. if event.room_id != room.room_id: @@ -584,9 +588,21 @@ class SyncRestServlet(RestServlet): room.room_id, event.room_id, ) + for filtered_event in timeline_events: + # We've had bug reports that events were coming down under the + # wrong room. + if filtered_event.event.room_id != room.room_id: + logger.warning( + "Event %r is under room %r instead of %r", + filtered_event.event.event_id, + room.room_id, + filtered_event.event.room_id, + ) serialized_state = await self._event_serializer.serialize_events( - state_events, time_now, config=serialize_options + [FilteredEvent.state(e) for e in state_events], + time_now, + config=serialize_options, ) serialized_timeline = await self._event_serializer.serialize_events( timeline_events, @@ -974,7 +990,7 @@ class SlidingSyncRestServlet(RestServlet): ): serialized_required_state = ( await self.event_serializer.serialize_events( - room_result.required_state, + [FilteredEvent.state(e) for e in room_result.required_state], time_now, config=serialize_options, ) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 694b3e1645..1a84bf1ff8 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -34,6 +34,7 @@ from pydantic import ConfigDict from synapse.api.constants import EventTypes from synapse.events import EventBase +from synapse.events.utils import FilteredEvent from synapse.types import ( DeviceListUpdates, JsonDict, @@ -185,7 +186,7 @@ class SlidingSyncResult: # Should be empty for invite/knock rooms with `stripped_state` required_state: list[EventBase] # Should be empty for invite/knock rooms with `stripped_state` - timeline_events: list[EventBase] + timeline_events: list[FilteredEvent] bundled_aggregations: dict[str, "BundledAggregations"] | None # Optional because it's only relevant to invite/knock rooms stripped_state: list[JsonDict] diff --git a/synapse/visibility.py b/synapse/visibility.py index 5ba2a14a24..fc3e9dfa49 100644 --- a/synapse/visibility.py +++ b/synapse/visibility.py @@ -31,14 +31,13 @@ import attr from synapse.api.constants import ( EventTypes, - EventUnsignedContentFields, HistoryVisibility, JoinRules, Membership, ) from synapse.events import EventBase from synapse.events.snapshot import EventContext -from synapse.events.utils import clone_event, prune_event +from synapse.events.utils import FilteredEvent, prune_event from synapse.logging.opentracing import trace from synapse.storage.controllers import StorageControllers from synapse.storage.databases.main import DataStore @@ -82,7 +81,7 @@ async def filter_and_transform_events_for_client( is_peeking: bool = False, always_include_ids: frozenset[str] = frozenset(), filter_send_to_client: bool = True, -) -> list[EventBase]: +) -> list[FilteredEvent]: """ Check which events a user is allowed to see. If the user can see the event but its sender asked for their data to be erased, prune the content of the event. @@ -102,8 +101,8 @@ async def filter_and_transform_events_for_client( also be called to check whether a user can see the state at a given point. Returns: - The filtered events. The `unsigned` data is annotated with the membership state - of `user_id` at each event. + The filtered events, wrapped in FilteredEvent with the requesting user's + membership at each event annotated for use during serialization (MSC4115). """ # Filter out events that have been soft failed so that we don't relay them # to clients, unless they're a server admin and want that to happen. @@ -176,7 +175,7 @@ async def filter_and_transform_events_for_client( room_id ] = await storage.main.get_retention_policy_for_room(room_id) - def allowed(event: EventBase) -> EventBase | None: + def allowed(event: EventBase) -> FilteredEvent | None: state_after_event = event_id_to_state.get(event.event_id) filtered = _check_client_allowed_to_see_event( user_id=user_id, @@ -233,28 +232,9 @@ async def filter_and_transform_events_for_client( else Membership.LEAVE ) - # Copy the event before updating the unsigned data: this shouldn't be persisted - # to the cache! - cloned = clone_event(filtered) - cloned.unsigned[EventUnsignedContentFields.MEMBERSHIP] = user_membership - if storage.main.config.experimental.msc4354_enabled: - sticky_duration = cloned.sticky_duration() - if sticky_duration: - now_ms = storage.main.clock.time_msec() - expires_at = ( - # min() ensures that the origin server can't lie about the time and - # send the event 'in the future', as that would allow them to exceed - # the 1 hour limit on stickiness duration. - min(cloned.origin_server_ts, now_ms) + sticky_duration.as_millis() - ) - if expires_at > now_ms: - cloned.unsigned[EventUnsignedContentFields.STICKY_TTL] = ( - expires_at - now_ms - ) + return FilteredEvent(event=filtered, membership=user_membership) - return cloned - - # Check each event: gives an iterable of None or (a modified) EventBase. + # Check each event: gives an iterable of None or a FilteredEvent. filtered_events = map(allowed, events) # Turn it into a list and remove None entries before returning. diff --git a/tests/events/test_auto_accept_invites.py b/tests/events/test_auto_accept_invites.py index 72ade45758..e0ebdf0bca 100644 --- a/tests/events/test_auto_accept_invites.py +++ b/tests/events/test_auto_accept_invites.py @@ -380,7 +380,7 @@ class AutoAcceptInvitesTestCase(FederatingHomeserverTestCase): join_updates, _ = sync_join(self, inviting_user_id) # Assert that the last event in the room was not a member event for the target user. self.assertEqual( - join_updates[0].timeline.events[-1].content["membership"], "invite" + join_updates[0].timeline.events[-1].event.content["membership"], "invite" ) @override_config( @@ -423,7 +423,7 @@ class AutoAcceptInvitesTestCase(FederatingHomeserverTestCase): join_updates, b = sync_join(self, inviting_user_id) # Assert that the last event in the room was not a member event for the target user. self.assertEqual( - join_updates[0].timeline.events[-1].content["membership"], "invite" + join_updates[0].timeline.events[-1].event.content["membership"], "invite" ) @override_config( @@ -466,7 +466,7 @@ class AutoAcceptInvitesTestCase(FederatingHomeserverTestCase): join_updates, b = sync_join(self, inviting_user_id) # Assert that the last event in the room was not a member event for the target user. self.assertEqual( - join_updates[0].timeline.events[-1].content["membership"], "invite" + join_updates[0].timeline.events[-1].event.content["membership"], "invite" ) @override_config( @@ -509,7 +509,7 @@ class AutoAcceptInvitesTestCase(FederatingHomeserverTestCase): join_updates, b = sync_join(self, inviting_user_id) # Assert that the last event in the room was not a member event for the target user. self.assertEqual( - join_updates[0].timeline.events[-1].content["membership"], "invite" + join_updates[0].timeline.events[-1].event.content["membership"], "invite" ) diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index af44b5dec1..12ef42866d 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -28,6 +28,7 @@ from synapse.api.constants import EventContentFields from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.events.utils import ( + FilteredEvent, PowerLevelsContent, SerializeEventConfig, _split_field, @@ -655,7 +656,7 @@ class SerializeEventTestCase(HomeserverTestCase): ) -> JsonDict: return self.get_success( self._event_serializer.serialize_event( - ev, + FilteredEvent(event=ev, membership=None), 1479807801915, config=SerializeEventConfig( only_event_fields=fields, diff --git a/tests/handlers/test_admin.py b/tests/handlers/test_admin.py index 49bd3ba3f4..a368363d7e 100644 --- a/tests/handlers/test_admin.py +++ b/tests/handlers/test_admin.py @@ -81,7 +81,8 @@ class ExfiltrateData(unittest.HomeserverTestCase): # Check that the right number of events were written counter = Counter( - (event.type, getattr(event, "state_key", None)) for event in written_events + (event.event.type, getattr(event.event, "state_key", None)) + for event in written_events ) self.assertEqual(counter[(EventTypes.Message, None)], 2) self.assertEqual(counter[(EventTypes.Member, self.user1)], 1) @@ -119,7 +120,8 @@ class ExfiltrateData(unittest.HomeserverTestCase): # Check that the right number of events were written counter = Counter( - (event.type, getattr(event, "state_key", None)) for event in written_events + (event.event.type, getattr(event.event, "state_key", None)) + for event in written_events ) self.assertEqual(counter[(EventTypes.Message, None)], 1) self.assertEqual(counter[(EventTypes.Member, self.user1)], 1) @@ -151,7 +153,8 @@ class ExfiltrateData(unittest.HomeserverTestCase): # Check that the right number of events were written counter = Counter( - (event.type, getattr(event, "state_key", None)) for event in written_events + (event.event.type, getattr(event.event, "state_key", None)) + for event in written_events ) self.assertEqual(counter[(EventTypes.Message, None)], 2) self.assertEqual(counter[(EventTypes.Member, self.user1)], 1) @@ -192,7 +195,8 @@ class ExfiltrateData(unittest.HomeserverTestCase): # Check that the right number of events were written counter = Counter( - (event.type, getattr(event, "state_key", None)) for event in written_events + (event.event.type, getattr(event.event, "state_key", None)) + for event in written_events ) self.assertEqual(counter[(EventTypes.Message, None)], 2) self.assertEqual(counter[(EventTypes.Member, self.user1)], 1) diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index 18ec2ca6b6..b9dee1c954 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -307,7 +307,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(len(alice_sync_result.joined), 1) self.assertEqual(alice_sync_result.joined[0].room_id, room_id) last_room_creation_event_id = ( - alice_sync_result.joined[0].timeline.events[-1].event_id + alice_sync_result.joined[0].timeline.events[-1].event.event_id ) # Eve, a ne'er-do-well, registers. @@ -402,7 +402,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) ) last_room_creation_event_id = ( - initial_sync_result.joined[0].timeline.events[-1].event_id + initial_sync_result.joined[0].timeline.events[-1].event.event_id ) # Send a state event, and a regular event, both using the same prev ID @@ -437,7 +437,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(room_sync.room_id, room_id) self.assertTrue(room_sync.timeline.limited) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e3_event, e4_event], ) self.assertEqual( @@ -476,7 +476,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) ) last_room_creation_event_id = ( - initial_sync_result.joined[0].timeline.events[-1].event_id + initial_sync_result.joined[0].timeline.events[-1].event.event_id ) # Send a state event, and a regular event, both using the same prev ID @@ -521,7 +521,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(room_sync.room_id, room_id) self.assertTrue(room_sync.timeline.limited) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e3_event], ) self.assertEqual( @@ -563,7 +563,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) ) last_room_creation_event_id = ( - initial_sync_result.joined[0].timeline.events[-1].event_id + initial_sync_result.joined[0].timeline.events[-1].event.event_id ) # Send a state event, and a regular event, both using the same prev ID @@ -593,7 +593,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(room_sync.room_id, room_id) self.assertTrue(room_sync.timeline.limited) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e3_event], ) @@ -632,7 +632,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(room_sync.room_id, room_id) self.assertFalse(room_sync.timeline.limited) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e4_event], ) @@ -701,7 +701,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) ) last_room_creation_event_id = ( - initial_sync_result.joined[0].timeline.events[-1].event_id + initial_sync_result.joined[0].timeline.events[-1].event.event_id ) # Send a state event, and a regular event, both using the same prev ID @@ -728,7 +728,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): room_sync = initial_sync_result.joined[0] self.assertEqual(room_sync.room_id, room_id) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e3_event], ) if self.use_state_after: @@ -757,7 +757,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): self.assertEqual(room_sync.room_id, room_id) self.assertFalse(room_sync.timeline.limited) self.assertEqual( - [e.event_id for e in room_sync.timeline.events], + [e.event.event_id for e in room_sync.timeline.events], [e4_event, e5_event], ) @@ -855,7 +855,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): # The last three events in the timeline should be those leading up to the # leave self.assertEqual( - [e.event_id for e in sync_room_result.timeline.events[-3:]], + [e.event.event_id for e in sync_room_result.timeline.events[-3:]], [before_message_event, before_state_event, leave_event], ) @@ -947,7 +947,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) event_ids = [] for event in sync_result.joined[0].timeline.events: - event_ids.append(event.event_id) + event_ids.append(event.event.event_id) self.assertNotIn(call_event.event_id, event_ids) # it will come down in a private room, though @@ -995,7 +995,7 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): ) priv_event_ids = [] for event in private_sync_result.joined[0].timeline.events: - priv_event_ids.append(event.event_id) + priv_event_ids.append(event.event.event_id) self.assertIn(private_call_event.event_id, priv_event_ids) diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py index 82a3b5b337..1f8d9154ca 100644 --- a/tests/rest/client/test_retention.py +++ b/tests/rest/client/test_retention.py @@ -23,6 +23,7 @@ from unittest.mock import Mock from twisted.internet.testing import MemoryReactor from synapse.api.constants import EventTypes +from synapse.events.utils import FilteredEvent from synapse.rest import admin from synapse.rest.client import login, room from synapse.server import HomeServer @@ -173,7 +174,9 @@ class RetentionTestCase(unittest.HomeserverTestCase): # We should only get one event back. self.assertEqual(len(filtered_events), 1, filtered_events) # That event should be the second, not outdated event. - self.assertEqual(filtered_events[0].event_id, valid_event_id, filtered_events) + self.assertEqual( + filtered_events[0].event.event_id, valid_event_id, filtered_events + ) def _test_retention_event_purged(self, room_id: str, increment: float) -> None: """Run the following test scenario to test the message retention policy support: @@ -253,7 +256,11 @@ class RetentionTestCase(unittest.HomeserverTestCase): assert event is not None time_now = self.clock.time_msec() - serialized = self.get_success(self.serializer.serialize_event(event, time_now)) + serialized = self.get_success( + self.serializer.serialize_event( + FilteredEvent(event=event, membership=None), time_now + ) + ) return serialized diff --git a/tests/test_visibility.py b/tests/test_visibility.py index b50faa2a49..9a5efbdd39 100644 --- a/tests/test_visibility.py +++ b/tests/test_visibility.py @@ -22,7 +22,7 @@ from unittest.mock import patch from twisted.test.proto_helpers import MemoryReactor -from synapse.api.constants import AccountDataTypes, EventUnsignedContentFields +from synapse.api.constants import AccountDataTypes from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.events.snapshot import EventContext @@ -341,7 +341,7 @@ class FilterEventsForServerAdminsTestCase(HomeserverTestCase): ) self.assertEqual( [e.event_id for e in [self.regular_event]], - [e.event_id for e in filtered_events], + [e.event.event_id for e in filtered_events], ) def test_see_soft_failed_events(self) -> None: @@ -380,7 +380,7 @@ class FilterEventsForServerAdminsTestCase(HomeserverTestCase): ) self.assertEqual( [e.event_id for e in [self.regular_event, self.soft_failed_event]], - [e.event_id for e in filtered_events], + [e.event.event_id for e in filtered_events], ) def test_see_policy_server_spammy_events(self) -> None: @@ -427,7 +427,7 @@ class FilterEventsForServerAdminsTestCase(HomeserverTestCase): ) self.assertEqual( [e.event_id for e in [self.regular_event, self.spammy_event]], - [e.event_id for e in filtered_events], + [e.event.event_id for e in filtered_events], ) def test_see_soft_failed_and_policy_server_spammy_events(self) -> None: @@ -477,7 +477,7 @@ class FilterEventsForServerAdminsTestCase(HomeserverTestCase): e.event_id for e in [self.regular_event, self.soft_failed_event, self.spammy_event] ], - [e.event_id for e in filtered_events], + [e.event.event_id for e in filtered_events], ) @@ -559,14 +559,11 @@ class FilterEventsForClientTestCase(HomeserverTestCase): # and messages sent between the two, but not before or after. self.assertEqual( [e.event_id for e in [join_event, during_event, leave_event]], - [e.event_id for e in joiner_filtered_events], + [e.event.event_id for e in joiner_filtered_events], ) self.assertEqual( ["join", "join", "leave"], - [ - e.unsigned[EventUnsignedContentFields.MEMBERSHIP] - for e in joiner_filtered_events - ], + [e.membership for e in joiner_filtered_events], ) # The resident user should see all the events. @@ -581,14 +578,11 @@ class FilterEventsForClientTestCase(HomeserverTestCase): after_event, ] ], - [e.event_id for e in resident_filtered_events], + [e.event.event_id for e in resident_filtered_events], ) self.assertEqual( ["join", "join", "join", "join", "join"], - [ - e.unsigned[EventUnsignedContentFields.MEMBERSHIP] - for e in resident_filtered_events - ], + [e.membership for e in resident_filtered_events], ) @@ -651,15 +645,12 @@ class FilterEventsOutOfBandEventsForClientTestCase( ) ) self.assertEqual( - [e.event_id for e in filtered_events], + [e.event.event_id for e in filtered_events], [e.event_id for e in [invite_event, reject_event]], ) self.assertEqual( ["invite", "leave"], - [ - e.unsigned[EventUnsignedContentFields.MEMBERSHIP] - for e in filtered_events - ], + [e.membership for e in filtered_events], ) # other users should see neither From 2d015f78ea7719688ba4e62a2f37632fb02a9a54 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 Apr 2026 10:59:39 +0100 Subject: [PATCH 5/7] Convert `EventInternalMetadata` to use `Arc>` (#19669) This moves the reference counting from PyO3 into standard Rust types, allowing the class to be used natively from Rust without needing a Python runtime. --- changelog.d/19669.misc | 1 + rust/src/events/internal_metadata.rs | 535 +++++++++++++++++++-------- 2 files changed, 374 insertions(+), 162 deletions(-) create mode 100644 changelog.d/19669.misc diff --git a/changelog.d/19669.misc b/changelog.d/19669.misc new file mode 100644 index 0000000000..c0355c398b --- /dev/null +++ b/changelog.d/19669.misc @@ -0,0 +1 @@ +Convert `EventInternalMetadata` to use `Arc>`. diff --git a/rust/src/events/internal_metadata.rs b/rust/src/events/internal_metadata.rs index 503722955e..21d3b8c435 100644 --- a/rust/src/events/internal_metadata.rs +++ b/rust/src/events/internal_metadata.rs @@ -32,12 +32,16 @@ //! attributes, but for small number of keys is actually faster than using a //! hash or btree map. -use std::{num::NonZeroI64, ops::Deref}; +use std::{ + num::NonZeroI64, + ops::Deref, + sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}, +}; use anyhow::Context; use log::warn; use pyo3::{ - exceptions::PyAttributeError, + exceptions::{PyAttributeError, PyRuntimeError}, pybacked::PyBackedStr, pyclass, pymethods, types::{PyAnyMethods, PyDict, PyDictMethods, PyString}, @@ -235,19 +239,6 @@ macro_rules! get_property_opt { }; } -/// Helper macro to find the given field in internal metadata, raising an -/// attribute error if not found. -macro_rules! get_property { - ($self:expr, $name:ident) => { - get_property_opt!($self, $name).ok_or_else(|| { - PyAttributeError::new_err(format!( - "'EventInternalMetadata' has no attribute '{}'", - stringify!($name), - )) - }) - }; -} - /// Helper macro to set the give field. macro_rules! set_property { ($self:expr, $name:ident, $obj:expr) => { @@ -262,27 +253,240 @@ macro_rules! set_property { }; } -#[pyclass] +/// The inner data for `EventInternalMetadata`, containing all fields and +/// business logic with pure Rust types. #[derive(Clone)] -pub struct EventInternalMetadata { +struct EventInternalMetadataInner { /// The fields of internal metadata. This functions as a mapping. data: Vec, /// The stream ordering of this event. None, until it has been persisted. - #[pyo3(get, set)] - stream_ordering: Option, - #[pyo3(get, set)] - instance_name: Option, + pub stream_ordering: Option, + pub instance_name: Option, /// The event ID of the redaction event, if this event has been redacted. /// This is set dynamically at load time and is not persisted to the database. - #[pyo3(get, set)] - redacted_by: Option, + pub redacted_by: Option, /// whether this event is an outlier (ie, whether we have the state at that /// point in the DAG) - #[pyo3(get, set)] - outlier: bool, + pub outlier: bool, +} + +impl EventInternalMetadataInner { + pub fn is_outlier(&self) -> bool { + self.outlier + } + + /// Whether this event is an out-of-band membership. + /// + /// OOB memberships are a special case of outlier events: they are + /// membership events for federated rooms that we aren't full members of. + /// Examples include invites received over federation, and rejections for + /// such invites. + /// + /// The concept of an OOB membership is needed because these events need to + /// be processed as if they're new regular events (e.g. updating membership + /// state in the database, relaying to clients via /sync, etc) despite being + /// outliers. + /// + /// See also + /// https://element-hq.github.io/synapse/develop/development/room-dag-concepts.html#out-of-band-membership-events. + /// + /// (Added in synapse 0.99.0, so may be unreliable for events received + /// before that) + pub fn is_out_of_band_membership(&self) -> bool { + get_property_opt!(self, OutOfBandMembership) + .copied() + .unwrap_or(false) + } + + /// Whether this server should send the event on behalf of another server. + /// This is used by the federation "send_join" API to forward the initial + /// join event for a server in the room. + /// + /// returns a str with the name of the server this event is sent on behalf + /// of. + pub fn get_send_on_behalf_of(&self) -> Option<&str> { + get_property_opt!(self, SendOnBehalfOf).map(|a| a.deref()) + } + + /// Whether the redaction event needs to be rechecked when fetching + /// from the database. + /// + /// Starting in room v3 redaction events are accepted up front, and later + /// checked to see if the redacter and redactee's domains match. + /// + /// If the sender of the redaction event is allowed to redact any event + /// due to auth rules, then this will always return false. + pub fn need_to_check_redaction(&self) -> bool { + get_property_opt!(self, RecheckRedaction) + .copied() + .unwrap_or(false) + } + + /// Whether the event has been soft failed. + /// + /// Soft failed events should be handled as usual, except: + /// 1. They should not go down sync or event streams, or generally sent to + /// clients. + /// 2. They should not be added to the forward extremities (and therefore + /// not to current state). + pub fn is_soft_failed(&self) -> bool { + get_property_opt!(self, SoftFailed) + .copied() + .unwrap_or(false) + } + + /// Whether the event, if ours, should be sent to other clients and servers. + /// + /// This is used for sending dummy events internally. Servers and clients + /// can still explicitly fetch the event. + pub fn should_proactively_send(&self) -> bool { + get_property_opt!(self, ProactivelySend) + .copied() + .unwrap_or(true) + } + + /// Whether the event has been redacted. + /// + /// This is used for efficiently checking whether an event has been marked + /// as redacted without needing to make another database call. + pub fn is_redacted(&self) -> bool { + get_property_opt!(self, Redacted).copied().unwrap_or(false) + } + + /// Whether this event can trigger a push notification + pub fn is_notifiable(&self) -> bool { + !self.outlier || self.is_out_of_band_membership() + } + + pub fn get_out_of_band_membership(&self) -> Option { + get_property_opt!(self, OutOfBandMembership).copied() + } + + pub fn get_recheck_redaction(&self) -> Option { + get_property_opt!(self, RecheckRedaction).copied() + } + + pub fn get_soft_failed(&self) -> Option { + get_property_opt!(self, SoftFailed).copied() + } + + pub fn get_proactively_send(&self) -> Option { + get_property_opt!(self, ProactivelySend).copied() + } + + pub fn get_policy_server_spammy(&self) -> bool { + get_property_opt!(self, PolicyServerSpammy) + .copied() + .unwrap_or(false) + } + + pub fn get_redacted(&self) -> Option { + get_property_opt!(self, Redacted).copied() + } + + pub fn get_txn_id(&self) -> Option<&str> { + get_property_opt!(self, TxnId).map(|s| s.deref()) + } + + pub fn get_delay_id(&self) -> Option<&str> { + get_property_opt!(self, DelayId).map(|s| s.deref()) + } + + pub fn get_token_id(&self) -> Option { + get_property_opt!(self, TokenId).copied() + } + + pub fn get_device_id(&self) -> Option<&str> { + get_property_opt!(self, DeviceId).map(|s| s.deref()) + } + + pub fn set_out_of_band_membership(&mut self, obj: bool) { + set_property!(self, OutOfBandMembership, obj); + } + + pub fn set_send_on_behalf_of(&mut self, obj: String) { + set_property!(self, SendOnBehalfOf, obj.into_boxed_str()); + } + + pub fn set_recheck_redaction(&mut self, obj: bool) { + set_property!(self, RecheckRedaction, obj); + } + + pub fn set_soft_failed(&mut self, obj: bool) { + set_property!(self, SoftFailed, obj); + } + + pub fn set_proactively_send(&mut self, obj: bool) { + set_property!(self, ProactivelySend, obj); + } + + pub fn set_policy_server_spammy(&mut self, obj: bool) { + set_property!(self, PolicyServerSpammy, obj); + } + + fn get_spam_checker_spammy(&self) -> bool { + get_property_opt!(self, SpamCheckerSpammy) + .copied() + .unwrap_or(false) + } + + fn set_spam_checker_spammy(&mut self, obj: bool) { + set_property!(self, SpamCheckerSpammy, obj); + } + + pub fn set_redacted(&mut self, obj: bool) { + set_property!(self, Redacted, obj); + } + + pub fn set_txn_id(&mut self, obj: String) { + set_property!(self, TxnId, obj.into_boxed_str()); + } + + pub fn set_delay_id(&mut self, obj: String) { + set_property!(self, DelayId, obj.into_boxed_str()); + } + + pub fn set_token_id(&mut self, obj: i64) { + set_property!(self, TokenId, obj); + } + + pub fn set_device_id(&mut self, obj: String) { + set_property!(self, DeviceId, obj.into_boxed_str()); + } +} + +#[pyclass(frozen)] +#[derive(Clone)] +pub struct EventInternalMetadata { + inner: Arc>, +} + +impl EventInternalMetadata { + fn read_inner(&self) -> PyResult> { + self.inner + .read() + .map_err(|_| PyRuntimeError::new_err("EventInternalMetadata lock poisoned")) + } + + /// Get a write lock on the inner data. + /// + /// Note that callers should be careful not to panic while holding the write + /// lock, as this will poison the lock.q + fn write_inner(&self) -> PyResult> { + self.inner + .write() + .map_err(|_| PyRuntimeError::new_err("EventInternalMetadata lock poisoned")) + } +} + +/// Helper to convert `None` to an `AttributeError` for a property getter. +fn attr_err(val: Option, name: &str) -> PyResult { + val.ok_or_else(|| { + PyAttributeError::new_err(format!("'EventInternalMetadata' has no attribute '{name}'",)) + }) } #[pymethods] @@ -304,25 +508,33 @@ impl EventInternalMetadata { data.shrink_to_fit(); Ok(EventInternalMetadata { - data, - stream_ordering: None, - instance_name: None, - redacted_by: None, - outlier: false, + inner: Arc::new(RwLock::new(EventInternalMetadataInner { + data, + stream_ordering: None, + instance_name: None, + redacted_by: None, + outlier: false, + })), }) } - fn copy(&self) -> Self { - self.clone() + fn copy(&self) -> PyResult { + let guard = self.read_inner()?; + Ok(EventInternalMetadata { + inner: Arc::new(RwLock::new(guard.clone())), + }) } - /// Get a dict holding the data stored in the `internal_metadata` column in the database. + /// Get a dict holding the data stored in the `internal_metadata` column in + /// the database. /// - /// Note that `outlier` and `stream_ordering` are stored in separate columns so are not returned here. + /// Note that `outlier` and `stream_ordering` are stored in separate columns + /// so are not returned here. fn get_dict(&self, py: Python<'_>) -> PyResult> { + let guard = self.read_inner()?; let dict = PyDict::new(py); - for entry in &self.data { + for entry in &guard.data { let (key, value) = entry.to_python_pair(py); dict.set_item(key, value)?; } @@ -330,219 +542,218 @@ impl EventInternalMetadata { Ok(dict.into()) } - fn is_outlier(&self) -> bool { - self.outlier + fn is_outlier(&self) -> PyResult { + Ok(self.read_inner()?.is_outlier()) } - /// Whether this event is an out-of-band membership. - /// - /// OOB memberships are a special case of outlier events: they are - /// membership events for federated rooms that we aren't full members of. - /// Examples include invites received over federation, and rejections for - /// such invites. - /// - /// The concept of an OOB membership is needed because these events need to - /// be processed as if they're new regular events (e.g. updating membership - /// state in the database, relaying to clients via /sync, etc) despite being - /// outliers. - /// - /// See also - /// https://element-hq.github.io/synapse/develop/development/room-dag-concepts.html#out-of-band-membership-events. - /// - /// (Added in synapse 0.99.0, so may be unreliable for events received - /// before that) - fn is_out_of_band_membership(&self) -> bool { - get_property_opt!(self, OutOfBandMembership) - .copied() - .unwrap_or(false) + fn is_out_of_band_membership(&self) -> PyResult { + Ok(self.read_inner()?.is_out_of_band_membership()) } - /// Whether this server should send the event on behalf of another server. - /// This is used by the federation "send_join" API to forward the initial - /// join event for a server in the room. - /// - /// returns a str with the name of the server this event is sent on behalf - /// of. - fn get_send_on_behalf_of(&self) -> Option<&str> { - let s = get_property_opt!(self, SendOnBehalfOf); - s.map(|a| a.deref()) + fn get_send_on_behalf_of(&self) -> PyResult> { + Ok(self + .read_inner()? + .get_send_on_behalf_of() + .map(|s| s.to_owned())) } - /// Whether the redaction event needs to be rechecked when fetching - /// from the database. - /// - /// Starting in room v3 redaction events are accepted up front, and later - /// checked to see if the redacter and redactee's domains match. - /// - /// If the sender of the redaction event is allowed to redact any event - /// due to auth rules, then this will always return false. - fn need_to_check_redaction(&self) -> bool { - get_property_opt!(self, RecheckRedaction) - .copied() - .unwrap_or(false) + fn need_to_check_redaction(&self) -> PyResult { + Ok(self.read_inner()?.need_to_check_redaction()) } - /// Whether the event has been soft failed. - /// - /// Soft failed events should be handled as usual, except: - /// 1. They should not go down sync or event streams, or generally sent to - /// clients. - /// 2. They should not be added to the forward extremities (and therefore - /// not to current state). - fn is_soft_failed(&self) -> bool { - get_property_opt!(self, SoftFailed) - .copied() - .unwrap_or(false) + fn is_soft_failed(&self) -> PyResult { + Ok(self.read_inner()?.is_soft_failed()) } - /// Whether the event, if ours, should be sent to other clients and servers. - /// - /// This is used for sending dummy events internally. Servers and clients - /// can still explicitly fetch the event. - fn should_proactively_send(&self) -> bool { - get_property_opt!(self, ProactivelySend) - .copied() - .unwrap_or(true) + fn should_proactively_send(&self) -> PyResult { + Ok(self.read_inner()?.should_proactively_send()) } - /// Whether the event has been redacted. - /// - /// This is used for efficiently checking whether an event has been marked - /// as redacted without needing to make another database call. - fn is_redacted(&self) -> bool { - get_property_opt!(self, Redacted).copied().unwrap_or(false) + fn is_redacted(&self) -> PyResult { + Ok(self.read_inner()?.is_redacted()) } - /// Whether this event can trigger a push notification - fn is_notifiable(&self) -> bool { - !self.outlier || self.is_out_of_band_membership() + fn is_notifiable(&self) -> PyResult { + Ok(self.read_inner()?.is_notifiable()) } - // ** The following are the getters and setters of the various properties ** + #[getter] + fn get_stream_ordering(&self) -> PyResult> { + Ok(self.read_inner()?.stream_ordering) + } + #[setter] + fn set_stream_ordering(&self, val: Option) -> PyResult<()> { + self.write_inner()?.stream_ordering = val; + Ok(()) + } + + #[getter] + fn get_instance_name(&self) -> PyResult> { + Ok(self.read_inner()?.instance_name.clone()) + } + #[setter] + fn set_instance_name(&self, val: Option) -> PyResult<()> { + self.write_inner()?.instance_name = val; + Ok(()) + } + + #[getter] + fn get_redacted_by(&self) -> PyResult> { + Ok(self.read_inner()?.redacted_by.clone()) + } + #[setter] + fn set_redacted_by(&self, val: Option) -> PyResult<()> { + self.write_inner()?.redacted_by = val; + Ok(()) + } + + #[getter] + fn get_outlier(&self) -> PyResult { + Ok(self.read_inner()?.outlier) + } + #[setter] + fn set_outlier(&self, val: bool) -> PyResult<()> { + self.write_inner()?.outlier = val; + Ok(()) + } #[getter] fn get_out_of_band_membership(&self) -> PyResult { - let bool = get_property!(self, OutOfBandMembership)?; - Ok(*bool) + attr_err( + self.read_inner()?.get_out_of_band_membership(), + "out_of_band_membership", + ) } #[setter] - fn set_out_of_band_membership(&mut self, obj: bool) { - set_property!(self, OutOfBandMembership, obj); + fn set_out_of_band_membership(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_out_of_band_membership(obj); + Ok(()) } #[getter(send_on_behalf_of)] - fn getter_send_on_behalf_of(&self) -> PyResult<&str> { - let s = get_property!(self, SendOnBehalfOf)?; - Ok(s) + fn getter_send_on_behalf_of(&self) -> PyResult { + let guard = self.read_inner()?; + attr_err( + guard.get_send_on_behalf_of().map(|s| s.to_owned()), + "send_on_behalf_of", + ) } #[setter] - fn set_send_on_behalf_of(&mut self, obj: String) { - set_property!(self, SendOnBehalfOf, obj.into_boxed_str()); + fn set_send_on_behalf_of(&self, obj: String) -> PyResult<()> { + self.write_inner()?.set_send_on_behalf_of(obj); + Ok(()) } #[getter] fn get_recheck_redaction(&self) -> PyResult { - let bool = get_property!(self, RecheckRedaction)?; - Ok(*bool) + attr_err( + self.read_inner()?.get_recheck_redaction(), + "recheck_redaction", + ) } #[setter] - fn set_recheck_redaction(&mut self, obj: bool) { - set_property!(self, RecheckRedaction, obj); + fn set_recheck_redaction(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_recheck_redaction(obj); + Ok(()) } #[getter] fn get_soft_failed(&self) -> PyResult { - let bool = get_property!(self, SoftFailed)?; - Ok(*bool) + attr_err(self.read_inner()?.get_soft_failed(), "soft_failed") } #[setter] - fn set_soft_failed(&mut self, obj: bool) { - set_property!(self, SoftFailed, obj); + fn set_soft_failed(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_soft_failed(obj); + Ok(()) } #[getter] fn get_proactively_send(&self) -> PyResult { - let bool = get_property!(self, ProactivelySend)?; - Ok(*bool) + attr_err( + self.read_inner()?.get_proactively_send(), + "proactively_send", + ) } #[setter] - fn set_proactively_send(&mut self, obj: bool) { - set_property!(self, ProactivelySend, obj); + fn set_proactively_send(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_proactively_send(obj); + Ok(()) } #[getter] fn get_policy_server_spammy(&self) -> PyResult { - Ok(get_property_opt!(self, PolicyServerSpammy) - .copied() - .unwrap_or(false)) + Ok(self.read_inner()?.get_policy_server_spammy()) } #[setter] - fn set_policy_server_spammy(&mut self, obj: bool) { - set_property!(self, PolicyServerSpammy, obj); + fn set_policy_server_spammy(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_policy_server_spammy(obj); + Ok(()) } #[getter] fn get_spam_checker_spammy(&self) -> PyResult { - Ok(get_property_opt!(self, SpamCheckerSpammy) - .copied() - .unwrap_or(false)) + Ok(self.read_inner()?.get_spam_checker_spammy()) } #[setter] - fn set_spam_checker_spammy(&mut self, obj: bool) { - set_property!(self, SpamCheckerSpammy, obj); + fn set_spam_checker_spammy(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_spam_checker_spammy(obj); + Ok(()) } #[getter] fn get_redacted(&self) -> PyResult { - let bool = get_property!(self, Redacted)?; - Ok(*bool) + attr_err(self.read_inner()?.get_redacted(), "redacted") } #[setter] - fn set_redacted(&mut self, obj: bool) { - set_property!(self, Redacted, obj); + fn set_redacted(&self, obj: bool) -> PyResult<()> { + self.write_inner()?.set_redacted(obj); + Ok(()) } /// The transaction ID, if it was set when the event was created. #[getter] - fn get_txn_id(&self) -> PyResult<&str> { - let s = get_property!(self, TxnId)?; - Ok(s) + fn get_txn_id(&self) -> PyResult { + let guard = self.read_inner()?; + attr_err(guard.get_txn_id().map(|s| s.to_owned()), "txn_id") } #[setter] - fn set_txn_id(&mut self, obj: String) { - set_property!(self, TxnId, obj.into_boxed_str()); + fn set_txn_id(&self, obj: String) -> PyResult<()> { + self.write_inner()?.set_txn_id(obj); + Ok(()) } /// The delay ID, set only if the event was a delayed event. #[getter] - fn get_delay_id(&self) -> PyResult<&str> { - let s = get_property!(self, DelayId)?; - Ok(s) + fn get_delay_id(&self) -> PyResult { + let guard = self.read_inner()?; + attr_err(guard.get_delay_id().map(|s| s.to_owned()), "delay_id") } #[setter] - fn set_delay_id(&mut self, obj: String) { - set_property!(self, DelayId, obj.into_boxed_str()); + fn set_delay_id(&self, obj: String) -> PyResult<()> { + self.write_inner()?.set_delay_id(obj); + Ok(()) } /// The access token ID of the user who sent this event, if any. #[getter] fn get_token_id(&self) -> PyResult { - let r = get_property!(self, TokenId)?; - Ok(*r) + attr_err(self.read_inner()?.get_token_id(), "token_id") } #[setter] - fn set_token_id(&mut self, obj: i64) { - set_property!(self, TokenId, obj); + fn set_token_id(&self, obj: i64) -> PyResult<()> { + self.write_inner()?.set_token_id(obj); + Ok(()) } /// The device ID of the user who sent this event, if any. #[getter] - fn get_device_id(&self) -> PyResult<&str> { - let s = get_property!(self, DeviceId)?; - Ok(s) + fn get_device_id(&self) -> PyResult { + let guard = self.read_inner()?; + attr_err(guard.get_device_id().map(|s| s.to_owned()), "device_id") } #[setter] - fn set_device_id(&mut self, obj: String) { - set_property!(self, DeviceId, obj.into_boxed_str()); + fn set_device_id(&self, obj: String) -> PyResult<()> { + self.write_inner()?.set_device_id(obj); + Ok(()) } } From e1b56313de110ded3a9311ac9811d671f33e69a8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 16 Apr 2026 04:31:10 -0600 Subject: [PATCH 6/7] Add upgrade notes for new quarantined_media_changes stream (#19694) Fixes https://github.com/element-hq/synapse/issues/19692 Introduced by https://github.com/element-hq/synapse/pull/19558 --------- Co-authored-by: Eric Eastwood --- changelog.d/19694.feature | 1 + docs/upgrade.md | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 changelog.d/19694.feature diff --git a/changelog.d/19694.feature b/changelog.d/19694.feature new file mode 100644 index 0000000000..f4c8f042bf --- /dev/null +++ b/changelog.d/19694.feature @@ -0,0 +1 @@ +Add a ["Listing quarantined media changes" Admin API](https://element-hq.github.io/synapse/latest/admin_api/media_admin_api.html#listing-quarantined-media-changes) for retrieving a paginated record of when media became (un)quarantined. \ No newline at end of file diff --git a/docs/upgrade.md b/docs/upgrade.md index aeae82c114..44ab342d5a 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -117,6 +117,19 @@ each upgrade are complete before moving on to the next upgrade, to avoid stacking them up. You can monitor the currently running background updates with [the Admin API](usage/administration/admin_api/background_updates.html#status). +# Upgrading to v1.152.0 + +## Workers which quarantine media must be stream writers + +A new [`quarantined_media_changes` stream writer](./workers.md#the-quarantined_media_changes-stream) is +introduced. Existing deployments which route the `/quarantine_media` endpoints to a +worker (instead of the main process) *must* also add those workers to the +`quarantined_media_changes` stream writer list. Quarantining media will not work without +this. + +If your deployment does not use workers, or instead uses the main process for +quarantining media, you do not need to make any changes to your configuration. + # Upgrading to v1.150.0 ## Removal of the `systemd` pip extra From 964ca65ebb110203b449dccb64d7669e68dfbb3e Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 16 Apr 2026 11:19:28 +0000 Subject: [PATCH 7/7] Support MSC4450: Identity Provider selection for User-Interactive Authentication with Legacy Single Sign-On. (#19693) Closes: #19688 Part of: MSC4450 whose Experimental Feature tracking issue is #19691 Add an unstable, namespaced `idp_id` query parameter to `fallback/web` \ This allows clients to specify the identity provider they'd like to log in with for SSO when they have multiple upstream IdPs associated with their account. Previously, Synapse would just pick one arbitrarily. But this was undesirable as you may want to use a different one at that point in time. When logging in, the user is able to choose when IdP they use - during UIA (which uses fallback auth mechanism) they should be able to do the same. ----- Signed-off-by: Olivier 'reivilibre Co-authored-by: Andrew Morgan Co-authored-by: Eric Eastwood --- changelog.d/19693.feature | 1 + synapse/config/experimental.py | 6 ++ synapse/handlers/auth.py | 35 ++++++--- synapse/rest/client/auth.py | 21 ++++- tests/rest/client/test_auth.py | 140 +++++++++++++++++++++++++++++++++ 5 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 changelog.d/19693.feature diff --git a/changelog.d/19693.feature b/changelog.d/19693.feature new file mode 100644 index 0000000000..efe65b8c49 --- /dev/null +++ b/changelog.d/19693.feature @@ -0,0 +1 @@ +Support [MSC4450: Identity Provider selection for User-Interactive Authentication with Legacy Single Sign-On](https://github.com/matrix-org/matrix-spec-proposals/pull/4450). \ No newline at end of file diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index e77412a797..702c7e3246 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -604,3 +604,9 @@ class ExperimentalConfig(Config): # Note that sticky events persisted before this feature is enabled will not be # considered sticky by the local homeserver. self.msc4354_enabled: bool = experimental.get("msc4354_enabled", False) + + # MSC4450: Identity Provider selection for User-Interactive Authentication + # with Legacy Single Sign-On (`m.login.sso`) + # Tracked in: https://github.com/element-hq/synapse/issues/19691 + # Note that this is only applicable to legacy auth, not MAS integration (OAuth 2.0). + self.msc4450_enabled: bool = experimental.get("msc4450_enabled", False) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index b5c0cbdba2..1c8e108ea8 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -1738,13 +1738,17 @@ class AuthHandler: else: return False - async def start_sso_ui_auth(self, request: SynapseRequest, session_id: str) -> str: + async def start_sso_ui_auth( + self, request: SynapseRequest, session_id: str, preferred_idp_id: str | None + ) -> str: """ Get the HTML for the SSO redirect confirmation page. Args: request: The incoming HTTP request session_id: The user interactive authentication session ID. + preferred_idp_id: The ID of the identity provider to use. If `None` one will + be picked unpredictably from those the user has already signed in with. Returns: The HTML to render. @@ -1768,15 +1772,26 @@ class AuthHandler: # it not being offered. raise SynapseError(400, "User has no SSO identities") - # for now, just pick one - idp_id, sso_auth_provider = next(iter(idps.items())) - if len(idps) > 0: - logger.warning( - "User %r has previously logged in with multiple SSO IdPs; arbitrarily " - "picking %r", - user_id_to_verify, - idp_id, - ) + if preferred_idp_id is not None: + # Use the idp specified by the client. + sso_auth_provider = idps.get(preferred_idp_id) + if sso_auth_provider is None: + raise SynapseError( + 400, + f"Unknown preferred Identity Provider: '{preferred_idp_id}'", + errcode=Codes.INVALID_PARAM, + ) + else: + idp_id, sso_auth_provider = next(iter(idps.items())) + if len(idps) > 0: + # We arbitrarily picked an IdP from multiple potential + # candidates. This may be undesirable for the user. + logger.warning( + "User %r has previously logged in with multiple SSO IdPs; arbitrarily " + "picking %r", + user_id_to_verify, + idp_id, + ) redirect_url = await sso_auth_provider.handle_redirect_request( request, None, session_id diff --git a/synapse/rest/client/auth.py b/synapse/rest/client/auth.py index f325499044..566c9c98c5 100644 --- a/synapse/rest/client/auth.py +++ b/synapse/rest/client/auth.py @@ -20,13 +20,13 @@ # import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Optional from twisted.web.server import Request from synapse.api.auth.mas import MasDelegatedAuth from synapse.api.constants import LoginType -from synapse.api.errors import LoginError, SynapseError +from synapse.api.errors import Codes, LoginError, SynapseError from synapse.api.urls import CLIENT_API_PREFIX from synapse.http.server import HttpServer, respond_with_html, respond_with_redirect from synapse.http.servlet import RestServlet, parse_string @@ -61,12 +61,27 @@ class AuthRestServlet(RestServlet): hs.config.registration.registration_token_template ) self.success_template = hs.config.registration.fallback_success_template + self._msc4450_enabled = hs.config.experimental.msc4450_enabled async def on_GET(self, request: SynapseRequest, stagetype: str) -> None: session = parse_string(request, "session") if not session: raise SynapseError(400, "No session supplied") + # MSC4450 query parameter which allows clients to specify the Identity Provider + # they wish to use for legacy SSO during User-Interactive Authentication. + idp_id: Optional[str] = None + + if self._msc4450_enabled: + idp_id = parse_string(request, "io.element.idp_id") + + if idp_id is not None and stagetype != LoginType.SSO: + raise SynapseError( + 400, + Codes.INVALID_PARAM, + "idp_id can only be specified for the `m.login.sso` auth type", + ) + # We support the unstable (`org.matrix.cross_signing_reset`) name from MSC4312 until # enough clients have adopted the stable name (`m.oauth`). # Note: `org.matrix.cross_signing_reset` *is* the stable name of the *action* in the @@ -118,7 +133,7 @@ class AuthRestServlet(RestServlet): elif stagetype == LoginType.SSO: # Display a confirmation page which prompts the user to # re-authenticate with their SSO provider. - html = await self.auth_handler.start_sso_ui_auth(request, session) + html = await self.auth_handler.start_sso_ui_auth(request, session, idp_id) elif stagetype == LoginType.REGISTRATION_TOKEN: html = self.registration_token_template.render( diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index ffaf0e5a32..c266937a65 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -43,6 +43,62 @@ from tests.rest.client.utils import TEST_OIDC_CONFIG, TEST_OIDC_ISSUER from tests.server import FakeChannel from tests.unittest import override_config, skip_unless +TEST_MULTIPLE_OIDC_IDP_ID1 = "friendface" +TEST_MULTIPLE_OIDC_IDP_ID2 = "potatocloud" +TEST_MULTIPLE_OIDC_IDP_ID3 = "svnswitch" + +TEST_MULTIPLE_OIDC_ISSUER1 = "https://friendface.invalid/" +TEST_MULTIPLE_OIDC_ISSUER2 = "https://potato.invalid/" +TEST_MULTIPLE_OIDC_ISSUER3 = "https://subversionswitch.invalid/" + +TEST_MULTIPLE_OIDC_PROVIDERS = [ + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID1, + "idp_name": "FriendFace", + "idp_icon": "mxc://example.invalid/friendface", + "idp_brand": "friendface", + "issuer": TEST_MULTIPLE_OIDC_ISSUER1, + "client_id": "id-for-friendface", + "client_secret": "secret-for-friendface", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER1}oidc/jwks", + }, + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID2, + "idp_name": "Potato Cloud", + "idp_icon": "mxc://example.invalid/potatocloud", + "idp_brand": "potato", + "issuer": TEST_MULTIPLE_OIDC_ISSUER2, + "client_id": "id-for-potato", + "client_secret": "secret-for-potato", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER2}oidc/jwks", + }, + { + "idp_id": TEST_MULTIPLE_OIDC_IDP_ID3, + "idp_name": "SubversionSwitch", + "idp_icon": "mxc://example.invalid/svnswitch", + "idp_brand": "svnswitch", + "issuer": TEST_MULTIPLE_OIDC_ISSUER3, + "client_id": "id-for-svnswitch", + "client_secret": "secret-for-svnswitch", + "scopes": ["openid"], + "discover": False, + "authorization_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/auth", + "token_endpoint": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/token", + "jwks_uri": f"{TEST_MULTIPLE_OIDC_ISSUER3}oidc/jwks", + }, +] +""" +`oidc_providers` config example for multiple OIDC providers. +""" + class DummyRecaptchaChecker(UserInteractiveAuthChecker): def __init__(self, hs: HomeServer) -> None: @@ -506,6 +562,90 @@ class UIAuthTests(unittest.HomeserverTestCase): body={"auth": {"session": session_id}}, ) + @skip_unless(HAS_OIDC, "requires OIDC") + @override_config( + { + "oidc_providers": TEST_MULTIPLE_OIDC_PROVIDERS, + "experimental_features": {"msc4450_enabled": True}, + } + ) + def test_msc4450_select_idp_id(self) -> None: + """ + Test for MSC4450: Identity Provider selection for + User-Interactive Authentication with Legacy Single Sign-On. + + We configure 3 OIDC providers and then check that we can select + which one to redirect to for User-Interactive Authentication. + """ + + # Attach the user to the OIDC providers manually + for idp_id in ( + TEST_MULTIPLE_OIDC_IDP_ID1, + TEST_MULTIPLE_OIDC_IDP_ID2, + ): + self.get_success( + self.hs.get_datastores().main.record_user_external_id( + # `oidc-` is a magic prefix needed for OIDC providers + f"oidc-{idp_id}", + # arbitrary/opaque provider-specific external ID, doesn't really matter + "some.ext.user.id", + self.user, + ) + ) + + # Start a user-interactive authentication session by + # calling the device deletion API + channel = self.delete_device( + self.user_tok, self.device_id, HTTPStatus.UNAUTHORIZED + ) + auth_session = channel.json_body["session"] + flows = channel.json_body["flows"] + self.assertCountEqual( + flows, [{"stages": ["m.login.sso"]}, {"stages": ["m.login.password"]}] + ) + + # Try to start the User-Interactive Auth against both identity providers. + # Make sure we get the identity provider that we ask for. + # + # We need to try against both to be sure that Synapse doesn't just conveniently happen to + # arbitrarily select the identity provider we test. + for idp_id, provider_config in ( + ( + TEST_MULTIPLE_OIDC_IDP_ID1, + TEST_MULTIPLE_OIDC_PROVIDERS[0], + ), + ( + TEST_MULTIPLE_OIDC_IDP_ID2, + TEST_MULTIPLE_OIDC_PROVIDERS[1], + ), + ): + endpoint = f"/_matrix/client/v3/auth/m.login.sso/fallback/web?session={auth_session}&io.element.idp_id=oidc-{idp_id}" + channel = self.make_request("GET", endpoint) + self.assertEqual( + channel.code, + HTTPStatus.OK, + f"Failed to use the {endpoint} endpoint as part of the UIA flow for idp_id={idp_id} : response_body={channel.text_body}", + ) + + # This 'Continue with ...' text is templated by `synapse/res/templates/sso_auth_confirm.html` + self.assertIn( + f"Continue with {provider_config['idp_name']}", + channel.text_body, + ) + + self.assertIn(provider_config["authorization_endpoint"], channel.text_body) + + # Test that we can't use the 3rd OIDC provider as we're not + # registered with it + channel = self.make_request( + "GET", + f"/_matrix/client/v3/auth/m.login.sso/fallback/web?session={auth_session}&io.element.idp_id=oidc-{TEST_MULTIPLE_OIDC_IDP_ID3}", + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.json_body) + self.assertEqual( + channel.json_body["errcode"], Codes.INVALID_PARAM, channel.json_body + ) + @skip_unless(HAS_OIDC, "requires OIDC") @override_config({"oidc_config": TEST_OIDC_CONFIG}) def test_does_not_offer_password_for_sso_user(self) -> None: