diff options
author | Richard van der Hoff <richard@matrix.org> | 2024-04-18 19:14:44 +0100 |
---|---|---|
committer | Richard van der Hoff <richard@matrix.org> | 2024-04-22 20:51:40 +0100 |
commit | b9a49e8f8551b0283321cc63a7b4117608d4751b (patch) | |
tree | 496933bfea77c3fee178e44a8ba6857d7c02d0f2 | |
parent | Add a test for client event filtering (diff) | |
download | synapse-b9a49e8f8551b0283321cc63a7b4117608d4751b.tar.xz |
Return membership on each event returned to the client
-rw-r--r-- | synapse/api/constants.py | 7 | ||||
-rw-r--r-- | synapse/config/experimental.py | 4 | ||||
-rw-r--r-- | synapse/events/utils.py | 23 | ||||
-rw-r--r-- | synapse/handlers/admin.py | 6 | ||||
-rw-r--r-- | synapse/handlers/events.py | 7 | ||||
-rw-r--r-- | synapse/handlers/initial_sync.py | 7 | ||||
-rw-r--r-- | synapse/handlers/pagination.py | 1 | ||||
-rw-r--r-- | synapse/handlers/relations.py | 3 | ||||
-rw-r--r-- | synapse/handlers/room.py | 1 | ||||
-rw-r--r-- | synapse/handlers/search.py | 20 | ||||
-rw-r--r-- | synapse/handlers/sync.py | 2 | ||||
-rw-r--r-- | synapse/notifier.py | 1 | ||||
-rw-r--r-- | synapse/push/mailer.py | 5 | ||||
-rw-r--r-- | synapse/visibility.py | 59 | ||||
-rw-r--r-- | tests/events/test_utils.py | 13 | ||||
-rw-r--r-- | tests/rest/client/test_retention.py | 7 | ||||
-rw-r--r-- | tests/test_visibility.py | 93 |
17 files changed, 221 insertions, 38 deletions
diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 98884b4967..0a9123c56b 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -234,6 +234,13 @@ class EventContentFields: TO_DEVICE_MSGID: Final = "org.matrix.msgid" +class EventUnsignedContentFields: + """Fields found inside the 'unsigned' data on events""" + + # Requesting user's membership, per MSC4115 + MSC4115_MEMBERSHIP: Final = "io.element.msc4115.membership" + + class RoomTypes: """Understood values of the room_type field of m.room.create events.""" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 353ae23f91..23f9e77aa8 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -422,3 +422,7 @@ class ExperimentalConfig(Config): "MSC4108 requires MSC3861 to be enabled", ("experimental", "msc4108_delegation_endpoint"), ) + + self.msc4115_membership_on_events = experimental.get( + "msc4115_membership_on_events", False + ) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index e0613d0dbc..8b7c5885db 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -49,7 +49,7 @@ from synapse.api.errors import Codes, SynapseError from synapse.api.room_versions import RoomVersion from synapse.types import JsonDict, Requester -from . import EventBase +from . import EventBase, make_event_from_dict if TYPE_CHECKING: from synapse.handlers.relations import BundledAggregations @@ -82,8 +82,6 @@ def prune_event(event: EventBase) -> EventBase: """ pruned_event_dict = prune_event_dict(event.room_version, event.get_dict()) - from . import make_event_from_dict - pruned_event = make_event_from_dict( pruned_event_dict, event.room_version, event.internal_metadata.get_dict() ) @@ -101,6 +99,25 @@ def prune_event(event: EventBase) -> EventBase: return pruned_event +def clone_event(event: EventBase) -> EventBase: + """Take a copy of the event. + + This is mostly useful because it does a *shallow* copy of the `unsigned` data, + which means it can then be updated without corrupting the in-memory cache. + """ + new_event = make_event_from_dict( + event.get_dict(), event.room_version, event.internal_metadata.get_dict() + ) + + # copy the internal fields + new_event.internal_metadata.stream_ordering = ( + event.internal_metadata.stream_ordering + ) + new_event.internal_metadata.outlier = event.internal_metadata.outlier + + return new_event + + def prune_event_dict(room_version: RoomVersion, event_dict: JsonDict) -> JsonDict: """Redacts the event_dict in the same way as `prune_event`, except it operates on dicts rather than event objects diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py index 360614e25b..702d40332c 100644 --- a/synapse/handlers/admin.py +++ b/synapse/handlers/admin.py @@ -42,6 +42,7 @@ class AdminHandler: self._device_handler = hs.get_device_handler() self._storage_controllers = hs.get_storage_controllers() self._state_storage_controller = self._storage_controllers.state + self._hs_config = hs.config self._msc3866_enabled = hs.config.experimental.msc3866.enabled async def get_whois(self, user: UserID) -> JsonMapping: @@ -217,7 +218,10 @@ class AdminHandler: ) events = await filter_events_for_client( - self._storage_controllers, user_id, events + self._storage_controllers, + user_id, + events, + msc4115_membership_on_events=self._hs_config.experimental.msc4115_membership_on_events, ) writer.write_events(room_id, events) diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index c3fee74a98..09d553cff1 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -148,6 +148,7 @@ class EventHandler: def __init__(self, hs: "HomeServer"): self.store = hs.get_datastores().main self._storage_controllers = hs.get_storage_controllers() + self._config = hs.config async def get_event( self, @@ -189,7 +190,11 @@ class EventHandler: is_peeking = not is_user_in_room filtered = await filter_events_for_client( - self._storage_controllers, user.to_string(), [event], is_peeking=is_peeking + self._storage_controllers, + user.to_string(), + [event], + is_peeking=is_peeking, + msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events, ) if not filtered: diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index bcc5b285ac..d99fc4bec0 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -221,7 +221,10 @@ class InitialSyncHandler: ).addErrback(unwrapFirstError) messages = await filter_events_for_client( - self._storage_controllers, user_id, messages + self._storage_controllers, + user_id, + messages, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) start_token = now_token.copy_and_replace(StreamKeyType.ROOM, token) @@ -380,6 +383,7 @@ class InitialSyncHandler: requester.user.to_string(), messages, is_peeking=is_peeking, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) start_token = StreamToken.START.copy_and_replace(StreamKeyType.ROOM, token) @@ -494,6 +498,7 @@ class InitialSyncHandler: requester.user.to_string(), messages, is_peeking=is_peeking, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) start_token = now_token.copy_and_replace(StreamKeyType.ROOM, token) diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index cd3a9088cd..6617105cdb 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -623,6 +623,7 @@ class PaginationHandler: user_id, events, is_peeking=(member_event_id is None), + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) # if after the filter applied there are no more events diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py index 931ac0c813..c5cee8860b 100644 --- a/synapse/handlers/relations.py +++ b/synapse/handlers/relations.py @@ -95,6 +95,7 @@ class RelationsHandler: self._event_handler = hs.get_event_handler() self._event_serializer = hs.get_event_client_serializer() self._event_creation_handler = hs.get_event_creation_handler() + self._config = hs.config async def get_relations( self, @@ -163,6 +164,7 @@ class RelationsHandler: user_id, events, is_peeking=(member_event_id is None), + msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events, ) # The relations returned for the requested event do include their @@ -608,6 +610,7 @@ class RelationsHandler: user_id, events, is_peeking=(member_event_id is None), + msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events, ) aggregations = await self.get_bundled_aggregations( diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 5e81a51638..51739a2653 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1476,6 +1476,7 @@ class RoomContextHandler: user.to_string(), events, is_peeking=is_peeking, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) event = await self.store.get_event( diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index 19c5a2f257..fdbe98de3b 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -480,7 +480,10 @@ class SearchHandler: filtered_events = await search_filter.filter([r["event"] for r in results]) events = await filter_events_for_client( - self._storage_controllers, user.to_string(), filtered_events + self._storage_controllers, + user.to_string(), + filtered_events, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) events.sort(key=lambda e: -rank_map[e.event_id]) @@ -579,7 +582,10 @@ class SearchHandler: filtered_events = await search_filter.filter([r["event"] for r in results]) events = await filter_events_for_client( - self._storage_controllers, user.to_string(), filtered_events + self._storage_controllers, + user.to_string(), + filtered_events, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) room_events.extend(events) @@ -664,11 +670,17 @@ class SearchHandler: ) events_before = await filter_events_for_client( - self._storage_controllers, user.to_string(), res.events_before + self._storage_controllers, + user.to_string(), + res.events_before, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) events_after = await filter_events_for_client( - self._storage_controllers, user.to_string(), res.events_after + self._storage_controllers, + user.to_string(), + res.events_after, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) context: JsonDict = { diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index a6d54ee4b8..8ff45a3353 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -596,6 +596,7 @@ class SyncHandler: sync_config.user.to_string(), recents, always_include_ids=current_state_ids, + msc4115_membership_on_events=self.hs_config.experimental.msc4115_membership_on_events, ) log_kv({"recents_after_visibility_filtering": len(recents)}) else: @@ -681,6 +682,7 @@ class SyncHandler: sync_config.user.to_string(), loaded_recents, always_include_ids=current_state_ids, + msc4115_membership_on_events=self.hs_config.experimental.msc4115_membership_on_events, ) loaded_recents = [] diff --git a/synapse/notifier.py b/synapse/notifier.py index e87333a80a..7c1cd3b5f2 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -721,6 +721,7 @@ class Notifier: user.to_string(), new_events, is_peeking=is_peeking, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) elif keyname == StreamKeyType.PRESENCE: now = self.clock.time_msec() diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index f1ffc8115f..d5079652e4 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -513,7 +513,10 @@ class Mailer: } the_events = await filter_events_for_client( - self._storage_controllers, user_id, results.events_before + self._storage_controllers, + user_id, + results.events_before, + msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events, ) the_events.append(notif_event) diff --git a/synapse/visibility.py b/synapse/visibility.py index 4be4ac81e7..17ec15f42c 100644 --- a/synapse/visibility.py +++ b/synapse/visibility.py @@ -36,10 +36,15 @@ from typing import ( import attr -from synapse.api.constants import EventTypes, HistoryVisibility, Membership +from synapse.api.constants import ( + EventTypes, + EventUnsignedContentFields, + HistoryVisibility, + Membership, +) from synapse.events import EventBase from synapse.events.snapshot import EventContext -from synapse.events.utils import prune_event +from synapse.events.utils import clone_event, prune_event from synapse.logging.opentracing import trace from synapse.storage.controllers import StorageControllers from synapse.storage.databases.main import DataStore @@ -77,6 +82,7 @@ async def filter_events_for_client( is_peeking: bool = False, always_include_ids: FrozenSet[str] = frozenset(), filter_send_to_client: bool = True, + msc4115_membership_on_events: bool = False, ) -> List[EventBase]: """ Check which events a user is allowed to see. If the user can see the event but its @@ -95,9 +101,12 @@ async def filter_events_for_client( filter_send_to_client: Whether we're checking an event that's going to be sent to a client. This might not always be the case since this function can also be called to check whether a user can see the state at a given point. + msc4115_membership_on_events: Whether to include the requesting user's + membership in the "unsigned" data, per MSC4115. Returns: - The filtered events. + The filtered events. If `msc4115_membership_on_events` is true, the `unsigned` + data is annotated with the membership state of `user_id` at each event. """ # Filter out events that have been soft failed so that we don't relay them # to clients. @@ -134,7 +143,8 @@ async def filter_events_for_client( ) def allowed(event: EventBase) -> Optional[EventBase]: - return _check_client_allowed_to_see_event( + state_after_event = event_id_to_state.get(event.event_id) + filtered = _check_client_allowed_to_see_event( user_id=user_id, event=event, clock=storage.main.clock, @@ -142,13 +152,45 @@ async def filter_events_for_client( sender_ignored=event.sender in ignore_list, always_include_ids=always_include_ids, retention_policy=retention_policies[room_id], - state=event_id_to_state.get(event.event_id), + state=state_after_event, is_peeking=is_peeking, sender_erased=erased_senders.get(event.sender, False), ) + if filtered is None: + return None + + if not msc4115_membership_on_events: + return filtered + + # Annotate the event with the user's membership after the event. + # + # Normally we just look in `state_after_event`, but if the event is an outlier + # we won't have such a state. The only outliers that are returned here are the + # user's own membership event, so we can just inspect that. + + user_membership_event: Optional[EventBase] + if event.type == EventTypes.Member and event.state_key == user_id: + user_membership_event = event + elif state_after_event is not None: + user_membership_event = state_after_event.get((EventTypes.Member, user_id)) + else: + # unreachable! + raise Exception("Missing state for event that is not user's own membership") + + user_membership = ( + user_membership_event.membership + if user_membership_event + else Membership.LEAVE + ) - # Check each event: gives an iterable of None or (a potentially modified) - # EventBase. + # Copy the event before updating the unsigned data: this shouldn't be persisted + # to the cache! + cloned = clone_event(filtered) + cloned.unsigned[EventUnsignedContentFields.MSC4115_MEMBERSHIP] = user_membership + + return cloned + + # Check each event: gives an iterable of None or (a modified) EventBase. filtered_events = map(allowed, events) # Turn it into a list and remove None entries before returning. @@ -412,8 +454,7 @@ def _check_membership( state: StateMap[EventBase], is_peeking: bool, ) -> _CheckMembershipReturn: - """Check whether the user can see the event due to their membership - """ + """Check whether the user can see the event due to their membership""" # If the event is the user's own membership event, use the 'most joined' # membership membership = None diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index cf81bcf52c..975c285ec7 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -32,6 +32,7 @@ from synapse.events.utils import ( PowerLevelsContent, SerializeEventConfig, _split_field, + clone_event, copy_and_fixup_power_levels_contents, maybe_upsert_event_field, prune_event, @@ -611,6 +612,18 @@ class PruneEventTestCase(stdlib_unittest.TestCase): ) +class CloneEventTestCase(stdlib_unittest.TestCase): + def test_unsigned_is_copied(self) -> None: + original = make_event_from_dict( + {"type": "A", "event_id": "$test:domain", "unsigned": {"a": 1, "b": 2}} + ) + cloned = clone_event(original) + cloned.unsigned["b"] = 3 + + self.assertEqual(original.unsigned, {"a": 1, "b": 2}) + self.assertEqual(cloned.unsigned, {"a": 1, "b": 3}) + + class SerializeEventTestCase(stdlib_unittest.TestCase): def serialize(self, ev: EventBase, fields: Optional[List[str]]) -> JsonDict: return serialize_event( diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py index 09a5d64349..ceae40498e 100644 --- a/tests/rest/client/test_retention.py +++ b/tests/rest/client/test_retention.py @@ -163,7 +163,12 @@ class RetentionTestCase(unittest.HomeserverTestCase): ) self.assertEqual(2, len(events), "events retrieved from database") filtered_events = self.get_success( - filter_events_for_client(storage_controllers, self.user_id, events) + filter_events_for_client( + storage_controllers, + self.user_id, + events, + msc4115_membership_on_events=True, + ) ) # We should only get one event back. diff --git a/tests/test_visibility.py b/tests/test_visibility.py index c67a169ebd..1f7bc81ef6 100644 --- a/tests/test_visibility.py +++ b/tests/test_visibility.py @@ -21,6 +21,7 @@ import logging from typing import Optional from unittest.mock import patch +from synapse.api.constants import EventUnsignedContentFields from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.events.snapshot import EventContext @@ -29,6 +30,7 @@ from synapse.rest.client import login, room from synapse.server import HomeServer from synapse.types import create_requester from synapse.visibility import filter_events_for_client, filter_events_for_server + from tests import unittest from tests.test_utils.event_injection import inject_event, inject_member_event from tests.unittest import HomeserverTestCase @@ -286,22 +288,22 @@ class FilterEventsForClientTestCase(HomeserverTestCase): room_id = self.helper.create_room_as("resident", tok=resident_token) self.get_success( - inject_visibility_event(self.hs, room_id, "@resident:hs", "joined") + inject_visibility_event(self.hs, room_id, "@resident:test", "joined") ) before_event = self.get_success( - inject_message_event(self.hs, room_id, "@resident:hs", body="before") + inject_message_event(self.hs, room_id, "@resident:test", body="before") ) join_event = self.get_success( - inject_member_event(self.hs, room_id, "@joiner:hs", "join") + inject_member_event(self.hs, room_id, "@joiner:test", "join") ) during_event = self.get_success( - inject_message_event(self.hs, room_id, "@resident:hs", body="during") + inject_message_event(self.hs, room_id, "@resident:test", body="during") ) leave_event = self.get_success( - inject_member_event(self.hs, room_id, "@joiner:hs", "leave") + inject_member_event(self.hs, room_id, "@joiner:test", "leave") ) after_event = self.get_success( - inject_message_event(self.hs, room_id, "@resident:hs", body="after") + inject_message_event(self.hs, room_id, "@resident:test", body="after") ) # We have to reload the events from the db, to ensure that prev_content is @@ -322,17 +324,64 @@ class FilterEventsForClientTestCase(HomeserverTestCase): ] ] - filtered_events = self.get_success( + # Now run the events through the filter, and check that we can see the events + # we expect, and that the membership prop is as expected. + # + # We deliberately do the queries for both users upfront; this simulates + # concurrent queries on the server, and helps ensure that we aren't + # accidentally serving the same event object (with the same unsigned.membership + # property) to both users. + joiner_filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@joiner:test", + events_to_filter, + msc4115_membership_on_events=True, + ) + ) + resident_filtered_events = self.get_success( filter_events_for_client( self.hs.get_storage_controllers(), - "@joiner:hs", + "@resident:test", events_to_filter, + msc4115_membership_on_events=True, ) ) + # The joiner should be able to seem the join and leave, + # 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 filtered_events], + [e.event_id for e in joiner_filtered_events], + ) + self.assertEqual( + ["join", "join", "leave"], + [ + e.unsigned[EventUnsignedContentFields.MSC4115_MEMBERSHIP] + for e in joiner_filtered_events + ], + ) + + # The resident user should see all the events. + self.assertEqual( + [ + e.event_id + for e in [ + before_event, + join_event, + during_event, + leave_event, + after_event, + ] + ], + [e.event_id for e in resident_filtered_events], + ) + self.assertEqual( + ["join", "join", "join", "join", "join"], + [ + e.unsigned[EventUnsignedContentFields.MSC4115_MEMBERSHIP] + for e in resident_filtered_events + ], ) @@ -387,15 +436,24 @@ class FilterEventsOutOfBandEventsForClientTestCase( ) # the invited user should be able to see both the invite and the rejection + filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@user:test", + [invite_event, reject_event], + msc4115_membership_on_events=True, + ) + ) self.assertEqual( - self.get_success( - filter_events_for_client( - self.hs.get_storage_controllers(), - "@user:test", - [invite_event, reject_event], - ) - ), - [invite_event, reject_event], + [e.event_id for e in filtered_events], + [e.event_id for e in [invite_event, reject_event]], + ) + self.assertEqual( + ["invite", "leave"], + [ + e.unsigned[EventUnsignedContentFields.MSC4115_MEMBERSHIP] + for e in filtered_events + ], ) # other users should see neither @@ -405,6 +463,7 @@ class FilterEventsOutOfBandEventsForClientTestCase( self.hs.get_storage_controllers(), "@other:test", [invite_event, reject_event], + msc4115_membership_on_events=True, ) ), [], |