From 902987c80d1c5528bb9afbdb9d4f487e0cc8da97 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 5 Apr 2021 09:28:25 -0400 Subject: [PATCH 01/17] Optionally pass in the event context to handle_new_event. --- synapse/handlers/federation.py | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5ea8a7b6034b..8611d9259dd3 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1994,13 +1994,36 @@ async def _handle_new_event( self, origin: str, event: EventBase, + context: Optional[EventContext] = None, state: Optional[Iterable[EventBase]] = None, auth_events: Optional[MutableStateMap[EventBase]] = None, backfilled: bool = False, ) -> EventContext: - context = await self._prep_event( - origin, event, state=state, auth_events=auth_events, backfilled=backfilled - ) + """ + Process an event. + + Args: + origin: The host the event originates from. + event: The event itself. + context: The event context, if available. Otherwise this is calculated + from state and auth_events. + state: The state events to calculate the event context from. This is + ignored if context is provided. + auth_events: The auth events to calculate the event context from. This is + ignored if context is provided. + backfilled: True if the event was backfilled. + + Returns: + The event context. + """ + if not context: + context = await self._prep_event( + origin, + event, + state=state, + auth_events=auth_events, + backfilled=backfilled, + ) try: if ( From 861f40ab212820daf2ddd0f882230b83c0fb6a45 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 5 Apr 2021 12:59:56 -0400 Subject: [PATCH 02/17] Check whether a user can join a restricted room during a /send_join request. --- synapse/handlers/federation.py | 42 ++++++++++++++++++++++++++++++--- synapse/handlers/room_member.py | 4 ++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 8611d9259dd3..3d06050ace8d 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1667,7 +1667,45 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: # would introduce the danger of backwards-compatibility problems. event.internal_metadata.send_on_behalf_of = origin - context = await self._handle_new_event(origin, event) + # Calculate the event context. + context = await self._prep_event( + origin, event, state=None, auth_events=None, backfilled=False + ) + + # Get the current state at the to-be created event. + prev_state_ids = await context.get_prev_state_ids() + + # Check if the user is already in the room or invited to the room. + user_id = event.state_key + prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) + newly_joined = True + is_invite = False + if prev_member_event_id: + prev_member_event = await self.store.get_event(prev_member_event_id) + newly_joined = prev_member_event.membership != Membership.JOIN + is_invite = prev_member_event.membership == Membership.INVITE + + # We retrieve the room member handler here as to not cause a cyclic dependency + member_handler = self.hs.get_room_member_handler() + + # If the member is not already in the room, and not invited, check if + # they should be allowed access via membership in a space. + if ( + newly_joined + and not is_invite + and not await member_handler.can_join_without_invite( + prev_state_ids, + event.room_version, + user_id, + ) + ): + raise SynapseError( + 400, + "You do not belong to any of the required spaces to join this room.", + ) + + # Persist the event. + await self._handle_new_event(origin, event, context) logger.debug( "on_send_join_request: After _handle_new_event: %s, sigs: %s", @@ -1675,8 +1713,6 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: event.signatures, ) - prev_state_ids = await context.get_prev_state_ids() - state_ids = list(prev_state_ids.values()) auth_chain = await self.store.get_auth_chain(event.room_id, state_ids) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 894ef859f4d4..6f098b29e762 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -179,7 +179,7 @@ async def ratelimit_invite( await self._invites_per_user_limiter.ratelimit(requester, invitee_user_id) - async def _can_join_without_invite( + async def can_join_without_invite( self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str ) -> bool: """ @@ -303,7 +303,7 @@ async def _local_membership_update( if ( newly_joined and not user_is_invited - and not await self._can_join_without_invite( + and not await self.can_join_without_invite( prev_state_ids, event.room_version, user_id ) ): From 94bdb01a90f09a67b2d83d1af43be060676f354c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 7 Apr 2021 13:39:48 -0400 Subject: [PATCH 03/17] Newsfragment --- changelog.d/9763.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9763.feature diff --git a/changelog.d/9763.feature b/changelog.d/9763.feature new file mode 100644 index 000000000000..c2c74f13d578 --- /dev/null +++ b/changelog.d/9763.feature @@ -0,0 +1 @@ +Add experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. From 683d02d6ff2fd28e219deb59a22d20959c13997d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 9 Apr 2021 12:12:18 -0400 Subject: [PATCH 04/17] Clarify comment. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/handlers/federation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 3d06050ace8d..647360d0cb33 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1672,7 +1672,7 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: origin, event, state=None, auth_events=None, backfilled=False ) - # Get the current state at the to-be created event. + # Get the state before the new event. prev_state_ids = await context.get_prev_state_ids() # Check if the user is already in the room or invited to the room. From bb6d71cc6a32899472405339de145872f9ef6bdd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 5 Apr 2021 09:28:25 -0400 Subject: [PATCH 05/17] Calculate the context before handling the event. --- synapse/handlers/federation.py | 63 ++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 67888898ffb5..7462a2217e5e 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -808,7 +808,8 @@ async def _process_received_pdu( logger.debug("Processing event: %s", event) try: - await self._handle_new_event(origin, event, state=state) + context = await self._prep_event(origin, event, state=state) + await self._handle_new_event(event, context) except AuthError as e: raise FederationError("ERROR", e.code, e.msg, affected=event.event_id) @@ -1024,10 +1025,12 @@ async def backfill( # non-outliers assert not event.internal_metadata.is_outlier() + context = await self._prep_event(dest, event, backfilled=True) + # We store these one at a time since each event depends on the # previous to work out the state. # TODO: We can probably do something more clever here. - await self._handle_new_event(dest, event, backfilled=True) + await self._handle_new_event(event, context, backfilled=True) return events @@ -1667,7 +1670,11 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: # would introduce the danger of backwards-compatibility problems. event.internal_metadata.send_on_behalf_of = origin - context = await self._handle_new_event(origin, event) + # Calculate the event context and persist the event. + context = await self._prep_event( + origin, event, state=None, auth_events=None, backfilled=False + ) + context = await self._handle_new_event(event, context) logger.debug( "on_send_join_request: After _handle_new_event: %s, sigs: %s", @@ -1879,7 +1886,8 @@ async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: event.internal_metadata.outlier = False - await self._handle_new_event(origin, event) + context = await self._prep_event(origin, event) + await self._handle_new_event(event, context) logger.debug( "on_send_leave_request: After _handle_new_event: %s, sigs: %s", @@ -1992,16 +2000,21 @@ async def get_min_depth_for_context(self, context: str) -> int: async def _handle_new_event( self, - origin: str, event: EventBase, - state: Optional[Iterable[EventBase]] = None, - auth_events: Optional[MutableStateMap[EventBase]] = None, + context: EventContext, backfilled: bool = False, ) -> EventContext: - context = await self._prep_event( - origin, event, state=state, auth_events=auth_events, backfilled=backfilled - ) + """ + Process an event. + + Args: + event: The event itself. + context: The event context. + backfilled: True if the event was backfilled. + Returns: + The event context. + """ try: if ( not event.internal_metadata.is_outlier() @@ -2182,10 +2195,31 @@ async def _prep_event( self, origin: str, event: EventBase, - state: Optional[Iterable[EventBase]], - auth_events: Optional[MutableStateMap[EventBase]], - backfilled: bool, + state: Optional[Iterable[EventBase]] = None, + auth_events: Optional[MutableStateMap[EventBase]] = None, + backfilled: bool = False, ) -> EventContext: + """ + Prepare an event for sending over federation. + + Args: + origin: The host the event originates from. + event: The event itself. + state: The state events to calculate the event context from. + auth_events: + Map from (event_type, state_key) to event + + Normally, our calculated auth_events based on the state of the room + at the event's position in the DAG, though occasionally (eg if the + event is an outlier), may be the auth events claimed by the remote + server. + + Also NB that this function adds entries to it. + backfilled: True if the event was backfilled. + + Returns: + The event context. + """ context = await self.state_handler.compute_event_context(event, old_state=state) if not auth_events: @@ -2471,7 +2505,8 @@ async def _update_auth_events_and_context_for_auth( logger.debug( "do_auth %s missing_auth: %s", event.event_id, e.event_id ) - await self._handle_new_event(origin, e, auth_events=auth) + context = await self._prep_event(origin, e, auth_events=auth) + await self._handle_new_event(e, context) if e.event_id in event_auth_events: auth_events[(e.type, e.state_key)] = e From af7a67995fb7504d7fcd174661bab17c369bad68 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 12 Apr 2021 11:18:59 -0400 Subject: [PATCH 06/17] Move additional auth checks into do_auth. --- synapse/handlers/federation.py | 42 ++++++++++++++++++++++------------ tests/test_federation.py | 6 +++-- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 7462a2217e5e..3b8a66a48eda 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -2240,18 +2240,14 @@ async def _prep_event( if c and c.type == EventTypes.Create: auth_events[(c.type, c.state_key)] = c - context = await self.do_auth(origin, event, context, auth_events=auth_events) - - if not context.rejected: - await self._check_for_soft_fail(event, state, backfilled) - - if event.type == EventTypes.GuestAccess and not context.rejected: - await self.maybe_kick_guest_users(event) - - # If we are going to send this event over federation we precaclculate - # the joined hosts. - if event.internal_metadata.get_send_on_behalf_of(): - await self.event_creation_handler.cache_joined_hosts_for_event(event) + context = await self.do_auth( + origin, + event, + context, + state=state, + auth_events=auth_events, + backfilled=backfilled, + ) return context @@ -2370,14 +2366,18 @@ async def do_auth( origin: str, event: EventBase, context: EventContext, + state: Optional[Iterable[EventBase]], auth_events: MutableStateMap[EventBase], + backfilled: bool, ) -> EventContext: """ Args: - origin: - event: - context: + origin: The host the event originates from. + event: The event itself. + context: The event context. + state: The state events to calculate the event context from. This is + ignored if context is provided. auth_events: Map from (event_type, state_key) to event @@ -2387,6 +2387,7 @@ async def do_auth( server. Also NB that this function adds entries to it. + backfilled: True if the event was backfilled. Returns: updated context object """ @@ -2414,6 +2415,17 @@ async def do_auth( logger.warning("Failed auth resolution for %r because %s", event, e) context.rejected = RejectedReason.AUTH_ERROR + if not context.rejected: + await self._check_for_soft_fail(event, state, backfilled) + + if event.type == EventTypes.GuestAccess and not context.rejected: + await self.maybe_kick_guest_users(event) + + # If we are going to send this event over federation we precaclculate + # the joined hosts. + if event.internal_metadata.get_send_on_behalf_of(): + await self.event_creation_handler.cache_joined_hosts_for_event(event) + return context async def _update_auth_events_and_context_for_auth( diff --git a/tests/test_federation.py b/tests/test_federation.py index 8928597d17db..7943b1e3fefa 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -76,8 +76,10 @@ def setUp(self): ) self.handler = self.homeserver.get_federation_handler() - self.handler.do_auth = lambda origin, event, context, auth_events: succeed( - context + self.handler.do_auth = ( + lambda origin, event, context, state, auth_events, backfilled: succeed( + context + ) ) self.client = self.homeserver.get_federation_client() self.client._check_sigs_and_hash_and_fetch = lambda dest, pdus, **k: succeed( From b25de51b514badb97137fc2018dfbd56d1093b78 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 12 Apr 2021 14:14:08 -0400 Subject: [PATCH 07/17] Move do_auth call to handle event. --- synapse/handlers/federation.py | 82 +++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 3b8a66a48eda..084227dad528 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -808,8 +808,10 @@ async def _process_received_pdu( logger.debug("Processing event: %s", event) try: - context = await self._prep_event(origin, event, state=state) - await self._handle_new_event(event, context) + context, auth_events = await self._prep_event(event, state=state) + await self._handle_new_event( + origin, event, context, auth_events=auth_events, state=state + ) except AuthError as e: raise FederationError("ERROR", e.code, e.msg, affected=event.event_id) @@ -1025,12 +1027,14 @@ async def backfill( # non-outliers assert not event.internal_metadata.is_outlier() - context = await self._prep_event(dest, event, backfilled=True) + context, context_auth_events = await self._prep_event(event) # We store these one at a time since each event depends on the # previous to work out the state. # TODO: We can probably do something more clever here. - await self._handle_new_event(event, context, backfilled=True) + await self._handle_new_event( + dest, event, context, auth_events=context_auth_events, backfilled=True + ) return events @@ -1671,10 +1675,12 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: event.internal_metadata.send_on_behalf_of = origin # Calculate the event context and persist the event. - context = await self._prep_event( - origin, event, state=None, auth_events=None, backfilled=False + context, auth_events = await self._prep_event( + event, state=None, auth_events=None + ) + context = await self._handle_new_event( + origin, event, context, auth_events=auth_events, backfilled=False ) - context = await self._handle_new_event(event, context) logger.debug( "on_send_join_request: After _handle_new_event: %s, sigs: %s", @@ -1886,8 +1892,8 @@ async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: event.internal_metadata.outlier = False - context = await self._prep_event(origin, event) - await self._handle_new_event(event, context) + context, auth_events = await self._prep_event(event) + await self._handle_new_event(origin, event, context, auth_events=auth_events) logger.debug( "on_send_leave_request: After _handle_new_event: %s, sigs: %s", @@ -2000,21 +2006,42 @@ async def get_min_depth_for_context(self, context: str) -> int: async def _handle_new_event( self, + origin: str, event: EventBase, context: EventContext, + auth_events: MutableStateMap[EventBase], + state: Optional[Iterable[EventBase]] = None, backfilled: bool = False, ) -> EventContext: """ Process an event. Args: + origin: The host the event originates from. event: The event itself. context: The event context. + state: The state events used to auth the event. + auth_events: + Map from (event_type, state_key) to event + + Normally, our calculated auth_events based on the state of the room + at the event's position in the DAG, though occasionally (eg if the + event is an outlier), may be the auth events claimed by the remote + server. backfilled: True if the event was backfilled. Returns: The event context. """ + context = await self.do_auth( + origin, + event, + context, + state=state, + auth_events=auth_events, + backfilled=backfilled, + ) + try: if ( not event.internal_metadata.is_outlier() @@ -2054,11 +2081,17 @@ async def _handle_new_events( async def prep(ev_info: _NewEventInfo): event = ev_info.event with nested_logging_context(suffix=event.event_id): - res = await self._prep_event( - origin, + res, auth_events = await self._prep_event( event, state=ev_info.state, auth_events=ev_info.auth_events, + ) + res = await self.do_auth( + origin, + event, + res, + state=ev_info.state, + auth_events=auth_events, backfilled=backfilled, ) return res @@ -2193,17 +2226,14 @@ async def _persist_auth_tree( async def _prep_event( self, - origin: str, event: EventBase, state: Optional[Iterable[EventBase]] = None, auth_events: Optional[MutableStateMap[EventBase]] = None, - backfilled: bool = False, - ) -> EventContext: + ) -> Tuple[EventContext, MutableStateMap[EventBase]]: """ Prepare an event for sending over federation. Args: - origin: The host the event originates from. event: The event itself. state: The state events to calculate the event context from. auth_events: @@ -2215,10 +2245,9 @@ async def _prep_event( server. Also NB that this function adds entries to it. - backfilled: True if the event was backfilled. Returns: - The event context. + The event context and auth events. """ context = await self.state_handler.compute_event_context(event, old_state=state) @@ -2240,16 +2269,7 @@ async def _prep_event( if c and c.type == EventTypes.Create: auth_events[(c.type, c.state_key)] = c - context = await self.do_auth( - origin, - event, - context, - state=state, - auth_events=auth_events, - backfilled=backfilled, - ) - - return context + return context, auth_events async def _check_for_soft_fail( self, event: EventBase, state: Optional[Iterable[EventBase]], backfilled: bool @@ -2511,14 +2531,16 @@ async def _update_auth_events_and_context_for_auth( (e.type, e.state_key): e for e in remote_auth_chain if e.event_id in auth_ids or e.type == EventTypes.Create - } + } # type: MutableStateMap[EventBase] e.internal_metadata.outlier = True logger.debug( "do_auth %s missing_auth: %s", event.event_id, e.event_id ) - context = await self._prep_event(origin, e, auth_events=auth) - await self._handle_new_event(e, context) + context, auth = await self._prep_event(e, auth_events=auth) + await self._handle_new_event( + origin, e, context, auth_events=auth + ) if e.event_id in event_auth_events: auth_events[(e.type, e.state_key)] = e From 778391ab2753e4f9306fbd5b40f2ddd5afc26330 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 12 Apr 2021 15:15:12 -0400 Subject: [PATCH 08/17] Newsfragment --- changelog.d/9800.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9800.misc diff --git a/changelog.d/9800.misc b/changelog.d/9800.misc new file mode 100644 index 000000000000..c2c74f13d578 --- /dev/null +++ b/changelog.d/9800.misc @@ -0,0 +1 @@ +Add experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. From 0b4f1a831948b35024e209e06d1d3571b8cd33b6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 13 Apr 2021 08:02:09 -0400 Subject: [PATCH 09/17] Rename methods and update docstrings. --- synapse/handlers/federation.py | 76 +++++++++++++++++++++------------- tests/test_federation.py | 2 +- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 084227dad528..85c5cd281f0d 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -104,7 +104,7 @@ @attr.s(slots=True) class _NewEventInfo: - """Holds information about a received event, ready for passing to _handle_new_events + """Holds information about a received event, ready for passing to _auth_and_persist_events Attributes: event: the received event @@ -808,8 +808,10 @@ async def _process_received_pdu( logger.debug("Processing event: %s", event) try: - context, auth_events = await self._prep_event(event, state=state) - await self._handle_new_event( + context, auth_events = await self._calculate_event_context( + event, state=state + ) + await self._auth_and_persist_event( origin, event, context, auth_events=auth_events, state=state ) except AuthError as e: @@ -1014,7 +1016,9 @@ async def backfill( ) if ev_infos: - await self._handle_new_events(dest, room_id, ev_infos, backfilled=True) + await self._auth_and_persist_events( + dest, room_id, ev_infos, backfilled=True + ) # Step 2: Persist the rest of the events in the chunk one by one events.sort(key=lambda e: e.depth) @@ -1027,12 +1031,12 @@ async def backfill( # non-outliers assert not event.internal_metadata.is_outlier() - context, context_auth_events = await self._prep_event(event) + context, context_auth_events = await self._calculate_event_context(event) # We store these one at a time since each event depends on the # previous to work out the state. # TODO: We can probably do something more clever here. - await self._handle_new_event( + await self._auth_and_persist_event( dest, event, context, auth_events=context_auth_events, backfilled=True ) @@ -1368,7 +1372,7 @@ async def get_event(event_id: str): event_infos.append(_NewEventInfo(event, None, auth)) - await self._handle_new_events( + await self._auth_and_persist_events( destination, room_id, event_infos, @@ -1675,15 +1679,15 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: event.internal_metadata.send_on_behalf_of = origin # Calculate the event context and persist the event. - context, auth_events = await self._prep_event( + context, auth_events = await self._calculate_event_context( event, state=None, auth_events=None ) - context = await self._handle_new_event( + context = await self._auth_and_persist_event( origin, event, context, auth_events=auth_events, backfilled=False ) logger.debug( - "on_send_join_request: After _handle_new_event: %s, sigs: %s", + "on_send_join_request: After _auth_and_persist_event: %s, sigs: %s", event.event_id, event.signatures, ) @@ -1892,11 +1896,13 @@ async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: event.internal_metadata.outlier = False - context, auth_events = await self._prep_event(event) - await self._handle_new_event(origin, event, context, auth_events=auth_events) + context, auth_events = await self._calculate_event_context(event) + await self._auth_and_persist_event( + origin, event, context, auth_events=auth_events + ) logger.debug( - "on_send_leave_request: After _handle_new_event: %s, sigs: %s", + "on_send_leave_request: After _auth_and_persist_event: %s, sigs: %s", event.event_id, event.signatures, ) @@ -2004,7 +2010,7 @@ async def get_persisted_pdu( async def get_min_depth_for_context(self, context: str) -> int: return await self.store.get_min_depth(context) - async def _handle_new_event( + async def _auth_and_persist_event( self, origin: str, event: EventBase, @@ -2014,12 +2020,15 @@ async def _handle_new_event( backfilled: bool = False, ) -> EventContext: """ - Process an event. + Process an event by performing auth checks and then persisting to the database. Args: origin: The host the event originates from. event: The event itself. - context: The event context. + context: + The event context. + + NB that this function potentially modifies it. state: The state events used to auth the event. auth_events: Map from (event_type, state_key) to event @@ -2033,7 +2042,7 @@ async def _handle_new_event( Returns: The event context. """ - context = await self.do_auth( + context = await self._check_event_auth( origin, event, context, @@ -2063,7 +2072,7 @@ async def _handle_new_event( return context - async def _handle_new_events( + async def _auth_and_persist_events( self, origin: str, room_id: str, @@ -2081,12 +2090,12 @@ async def _handle_new_events( async def prep(ev_info: _NewEventInfo): event = ev_info.event with nested_logging_context(suffix=event.event_id): - res, auth_events = await self._prep_event( + res, auth_events = await self._calculate_event_context( event, state=ev_info.state, auth_events=ev_info.auth_events, ) - res = await self.do_auth( + res = await self._check_event_auth( origin, event, res, @@ -2224,14 +2233,14 @@ async def _persist_auth_tree( room_id, [(event, new_event_context)] ) - async def _prep_event( + async def _calculate_event_context( self, event: EventBase, state: Optional[Iterable[EventBase]] = None, auth_events: Optional[MutableStateMap[EventBase]] = None, ) -> Tuple[EventContext, MutableStateMap[EventBase]]: """ - Prepare an event for sending over federation. + Calculate the context and auth events for a given event. Args: event: The event itself. @@ -2381,7 +2390,7 @@ async def on_get_missing_events( return missing_events - async def do_auth( + async def _check_event_auth( self, origin: str, event: EventBase, @@ -2391,11 +2400,15 @@ async def do_auth( backfilled: bool, ) -> EventContext: """ + Checks whether an event should be rejected (for failing auth checks). Args: origin: The host the event originates from. event: The event itself. - context: The event context. + context: + The event context. + + NB that this function potentially modifies it. state: The state events to calculate the event context from. This is ignored if context is provided. auth_events: @@ -2408,8 +2421,9 @@ async def do_auth( Also NB that this function adds entries to it. backfilled: True if the event was backfilled. + Returns: - updated context object + The updated context object. """ room_version = await self.store.get_room_version_id(event.room_id) room_version_obj = KNOWN_ROOM_VERSIONS[room_version] @@ -2455,7 +2469,7 @@ async def _update_auth_events_and_context_for_auth( context: EventContext, auth_events: MutableStateMap[EventBase], ) -> EventContext: - """Helper for do_auth. See there for docs. + """Helper for _check_event_auth. See there for docs. Checks whether a given event has the expected auth events. If it doesn't then we talk to the remote server to compare state to see if @@ -2535,10 +2549,14 @@ async def _update_auth_events_and_context_for_auth( e.internal_metadata.outlier = True logger.debug( - "do_auth %s missing_auth: %s", event.event_id, e.event_id + "_check_event_auth %s missing_auth: %s", + event.event_id, + e.event_id, + ) + context, auth = await self._calculate_event_context( + e, auth_events=auth ) - context, auth = await self._prep_event(e, auth_events=auth) - await self._handle_new_event( + await self._auth_and_persist_event( origin, e, context, auth_events=auth ) diff --git a/tests/test_federation.py b/tests/test_federation.py index 7943b1e3fefa..07bc3a21edbb 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -76,7 +76,7 @@ def setUp(self): ) self.handler = self.homeserver.get_federation_handler() - self.handler.do_auth = ( + self.handler._check_event_auth = ( lambda origin, event, context, state, auth_events, backfilled: succeed( context ) From 9df28bedec89b39fe8c70720a46027759d864406 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 07:48:00 -0400 Subject: [PATCH 10/17] Move auth events modifications to auth checks. --- synapse/handlers/federation.py | 107 ++++++++++----------------------- 1 file changed, 33 insertions(+), 74 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 85c5cd281f0d..1ed11a5d7a59 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -808,12 +808,10 @@ async def _process_received_pdu( logger.debug("Processing event: %s", event) try: - context, auth_events = await self._calculate_event_context( - event, state=state - ) - await self._auth_and_persist_event( - origin, event, context, auth_events=auth_events, state=state + context = await self.state_handler.compute_event_context( + event, old_state=state ) + await self._auth_and_persist_event(origin, event, context, state=state) except AuthError as e: raise FederationError("ERROR", e.code, e.msg, affected=event.event_id) @@ -1031,14 +1029,12 @@ async def backfill( # non-outliers assert not event.internal_metadata.is_outlier() - context, context_auth_events = await self._calculate_event_context(event) + context = await self.state_handler.compute_event_context(event) # We store these one at a time since each event depends on the # previous to work out the state. # TODO: We can probably do something more clever here. - await self._auth_and_persist_event( - dest, event, context, auth_events=context_auth_events, backfilled=True - ) + await self._auth_and_persist_event(dest, event, context, backfilled=True) return events @@ -1679,11 +1675,9 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: event.internal_metadata.send_on_behalf_of = origin # Calculate the event context and persist the event. - context, auth_events = await self._calculate_event_context( - event, state=None, auth_events=None - ) + context = await self.state_handler.compute_event_context(event) context = await self._auth_and_persist_event( - origin, event, context, auth_events=auth_events, backfilled=False + origin, event, context, backfilled=False ) logger.debug( @@ -1896,10 +1890,8 @@ async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: event.internal_metadata.outlier = False - context, auth_events = await self._calculate_event_context(event) - await self._auth_and_persist_event( - origin, event, context, auth_events=auth_events - ) + context = await self.state_handler.compute_event_context(event) + await self._auth_and_persist_event(origin, event, context) logger.debug( "on_send_leave_request: After _auth_and_persist_event: %s, sigs: %s", @@ -2015,8 +2007,8 @@ async def _auth_and_persist_event( origin: str, event: EventBase, context: EventContext, - auth_events: MutableStateMap[EventBase], state: Optional[Iterable[EventBase]] = None, + auth_events: Optional[MutableStateMap[EventBase]] = None, backfilled: bool = False, ) -> EventContext: """ @@ -2090,17 +2082,15 @@ async def _auth_and_persist_events( async def prep(ev_info: _NewEventInfo): event = ev_info.event with nested_logging_context(suffix=event.event_id): - res, auth_events = await self._calculate_event_context( - event, - state=ev_info.state, - auth_events=ev_info.auth_events, + res = await self.state_handler.compute_event_context( + event, old_state=ev_info.state ) res = await self._check_event_auth( origin, event, res, state=ev_info.state, - auth_events=auth_events, + auth_events=ev_info.auth_events, backfilled=backfilled, ) return res @@ -2233,53 +2223,6 @@ async def _persist_auth_tree( room_id, [(event, new_event_context)] ) - async def _calculate_event_context( - self, - event: EventBase, - state: Optional[Iterable[EventBase]] = None, - auth_events: Optional[MutableStateMap[EventBase]] = None, - ) -> Tuple[EventContext, MutableStateMap[EventBase]]: - """ - Calculate the context and auth events for a given event. - - Args: - event: The event itself. - state: The state events to calculate the event context from. - auth_events: - Map from (event_type, state_key) to event - - Normally, our calculated auth_events based on the state of the room - at the event's position in the DAG, though occasionally (eg if the - event is an outlier), may be the auth events claimed by the remote - server. - - Also NB that this function adds entries to it. - - Returns: - The event context and auth events. - """ - context = await self.state_handler.compute_event_context(event, old_state=state) - - if not auth_events: - prev_state_ids = await context.get_prev_state_ids() - auth_events_ids = self.auth.compute_auth_events( - event, prev_state_ids, for_verification=True - ) - auth_events_x = await self.store.get_events(auth_events_ids) - auth_events = {(e.type, e.state_key): e for e in auth_events_x.values()} - - # This is a hack to fix some old rooms where the initial join event - # didn't reference the create event in its auth events. - if event.type == EventTypes.Member and not event.auth_event_ids(): - if len(event.prev_event_ids()) == 1 and event.depth < 5: - c = await self.store.get_event( - event.prev_event_ids()[0], allow_none=True - ) - if c and c.type == EventTypes.Create: - auth_events[(c.type, c.state_key)] = c - - return context, auth_events - async def _check_for_soft_fail( self, event: EventBase, state: Optional[Iterable[EventBase]], backfilled: bool ) -> None: @@ -2396,7 +2339,7 @@ async def _check_event_auth( event: EventBase, context: EventContext, state: Optional[Iterable[EventBase]], - auth_events: MutableStateMap[EventBase], + auth_events: Optional[MutableStateMap[EventBase]], backfilled: bool, ) -> EventContext: """ @@ -2428,6 +2371,24 @@ async def _check_event_auth( room_version = await self.store.get_room_version_id(event.room_id) room_version_obj = KNOWN_ROOM_VERSIONS[room_version] + if not auth_events: + prev_state_ids = await context.get_prev_state_ids() + auth_events_ids = self.auth.compute_auth_events( + event, prev_state_ids, for_verification=True + ) + auth_events_x = await self.store.get_events(auth_events_ids) + auth_events = {(e.type, e.state_key): e for e in auth_events_x.values()} + + # This is a hack to fix some old rooms where the initial join event + # didn't reference the create event in its auth events. + if event.type == EventTypes.Member and not event.auth_event_ids(): + if len(event.prev_event_ids()) == 1 and event.depth < 5: + c = await self.store.get_event( + event.prev_event_ids()[0], allow_none=True + ) + if c and c.type == EventTypes.Create: + auth_events[(c.type, c.state_key)] = c + try: context = await self._update_auth_events_and_context_for_auth( origin, event, context, auth_events @@ -2553,9 +2514,7 @@ async def _update_auth_events_and_context_for_auth( event.event_id, e.event_id, ) - context, auth = await self._calculate_event_context( - e, auth_events=auth - ) + context = await self.state_handler.compute_event_context(e) await self._auth_and_persist_event( origin, e, context, auth_events=auth ) From c8e2d1f086e8c9b9b35da334b9a38cec619ca9b1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 08:19:33 -0400 Subject: [PATCH 11/17] Reduce the overall diff. --- synapse/handlers/federation.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 1ed11a5d7a59..76273e59ea52 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1674,11 +1674,8 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: # would introduce the danger of backwards-compatibility problems. event.internal_metadata.send_on_behalf_of = origin - # Calculate the event context and persist the event. context = await self.state_handler.compute_event_context(event) - context = await self._auth_and_persist_event( - origin, event, context, backfilled=False - ) + context = await self._auth_and_persist_event(origin, event, context) logger.debug( "on_send_join_request: After _auth_and_persist_event: %s, sigs: %s", @@ -2506,7 +2503,7 @@ async def _update_auth_events_and_context_for_auth( (e.type, e.state_key): e for e in remote_auth_chain if e.event_id in auth_ids or e.type == EventTypes.Create - } # type: MutableStateMap[EventBase] + } e.internal_metadata.outlier = True logger.debug( From 9fe4b0ae69565f06ed9bc8d0faed34ec1287254e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 08:26:27 -0400 Subject: [PATCH 12/17] Tweak news file since a release happened. --- changelog.d/9800.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9800.misc b/changelog.d/9800.misc index c2c74f13d578..9404ad2fc047 100644 --- a/changelog.d/9800.misc +++ b/changelog.d/9800.misc @@ -1 +1 @@ -Add experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. +Update experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. From 4e3de96b61ace30dcd1181da3a5277da35ba759e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 09:44:58 -0400 Subject: [PATCH 13/17] Clarify comments. --- synapse/handlers/federation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 76273e59ea52..fb87add843ab 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -2018,7 +2018,9 @@ async def _auth_and_persist_event( The event context. NB that this function potentially modifies it. - state: The state events used to auth the event. + state: + The state events used to auth the event. If this is not provided + the current state events will be used. auth_events: Map from (event_type, state_key) to event @@ -2349,8 +2351,9 @@ async def _check_event_auth( The event context. NB that this function potentially modifies it. - state: The state events to calculate the event context from. This is - ignored if context is provided. + state: + The state events used to auth the event. If this is not provided + the current state events will be used. auth_events: Map from (event_type, state_key) to event From acc63f69f5d89ab2e21b796b8265267dac1d21c6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 09:55:55 -0400 Subject: [PATCH 14/17] Update changelog due to release. --- changelog.d/9763.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9763.feature b/changelog.d/9763.feature index c2c74f13d578..9404ad2fc047 100644 --- a/changelog.d/9763.feature +++ b/changelog.d/9763.feature @@ -1 +1 @@ -Add experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. +Update experimental support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083): restricting room access via group membership. From 0f3e3739aa8c9bdffc9dde98d19f52f2974548fb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 09:56:11 -0400 Subject: [PATCH 15/17] Rename changelog. --- changelog.d/{9800.misc => 9800.feature} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9800.misc => 9800.feature} (100%) diff --git a/changelog.d/9800.misc b/changelog.d/9800.feature similarity index 100% rename from changelog.d/9800.misc rename to changelog.d/9800.feature From 808386d8e71061c0255d4aa6b67feae97862ae8b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 09:58:18 -0400 Subject: [PATCH 16/17] Remove unused return value. --- synapse/handlers/federation.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b29ec0bbfcea..f3a4b1b92326 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -2040,7 +2040,7 @@ async def _auth_and_persist_event( state: Optional[Iterable[EventBase]] = None, auth_events: Optional[MutableStateMap[EventBase]] = None, backfilled: bool = False, - ) -> EventContext: + ) -> None: """ Process an event by performing auth checks and then persisting to the database. @@ -2062,9 +2062,6 @@ async def _auth_and_persist_event( event is an outlier), may be the auth events claimed by the remote server. backfilled: True if the event was backfilled. - - Returns: - The event context. """ context = await self._check_event_auth( origin, @@ -2094,8 +2091,6 @@ async def _auth_and_persist_event( ) raise - return context - async def _auth_and_persist_events( self, origin: str, From f5f9421e7a9bb5c39ec96d6bb2773564bbd12ad0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 11:37:38 -0400 Subject: [PATCH 17/17] Create a separate handler to avoid import cycles. --- synapse/handlers/event_auth.py | 82 +++++++++++++++++++++++++++++++++ synapse/handlers/federation.py | 6 +-- synapse/handlers/room_member.py | 62 ++----------------------- synapse/server.py | 5 ++ 4 files changed, 92 insertions(+), 63 deletions(-) create mode 100644 synapse/handlers/event_auth.py diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py new file mode 100644 index 000000000000..06da1a93d9f1 --- /dev/null +++ b/synapse/handlers/event_auth.py @@ -0,0 +1,82 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from typing import TYPE_CHECKING + +from synapse.api.constants import EventTypes, JoinRules +from synapse.api.room_versions import RoomVersion +from synapse.types import StateMap + +if TYPE_CHECKING: + from synapse.server import HomeServer + + +class EventAuthHandler: + def __init__(self, hs: "HomeServer"): + self._store = hs.get_datastore() + + async def can_join_without_invite( + self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str + ) -> bool: + """ + Check whether a user can join a room without an invite. + + When joining a room with restricted joined rules (as defined in MSC3083), + the membership of spaces must be checked during join. + + Args: + state_ids: The state of the room as it currently is. + room_version: The room version of the room being joined. + user_id: The user joining the room. + + Returns: + True if the user can join the room, false otherwise. + """ + # This only applies to room versions which support the new join rule. + if not room_version.msc3083_join_rules: + return True + + # If there's no join rule, then it defaults to public (so this doesn't apply). + join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None) + if not join_rules_event_id: + return True + + # If the join rule is not restricted, this doesn't apply. + join_rules_event = await self._store.get_event(join_rules_event_id) + if join_rules_event.content.get("join_rule") != JoinRules.MSC3083_RESTRICTED: + return True + + # If allowed is of the wrong form, then only allow invited users. + allowed_spaces = join_rules_event.content.get("allow", []) + if not isinstance(allowed_spaces, list): + return False + + # Get the list of joined rooms and see if there's an overlap. + joined_rooms = await self._store.get_rooms_for_user(user_id) + + # Pull out the other room IDs, invalid data gets filtered. + for space in allowed_spaces: + if not isinstance(space, dict): + continue + + space_id = space.get("space") + if not isinstance(space_id, str): + continue + + # The user was joined to one of the spaces specified, they can join + # this room! + if space_id in joined_rooms: + return True + + # The user was not in any of the required spaces. + return False diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index f3a4b1b92326..ec0c23bf7a90 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -147,6 +147,7 @@ def __init__(self, hs: "HomeServer"): self.is_mine_id = hs.is_mine_id self.spam_checker = hs.get_spam_checker() self.event_creation_handler = hs.get_event_creation_handler() + self.event_auth_handler = hs.get_event_auth_handler() self._message_handler = hs.get_message_handler() self._server_notices_mxid = hs.config.server_notices_mxid self.config = hs.config @@ -1690,15 +1691,12 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: newly_joined = prev_member_event.membership != Membership.JOIN is_invite = prev_member_event.membership == Membership.INVITE - # We retrieve the room member handler here as to not cause a cyclic dependency - member_handler = self.hs.get_room_member_handler() - # If the member is not already in the room, and not invited, check if # they should be allowed access via membership in a space. if ( newly_joined and not is_invite - and not await member_handler.can_join_without_invite( + and not await self.event_auth_handler.can_join_without_invite( prev_state_ids, event.room_version, user_id, diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 6f098b29e762..2471c84ab392 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -20,7 +20,7 @@ from typing import TYPE_CHECKING, Iterable, List, Optional, Tuple from synapse import types -from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules, Membership +from synapse.api.constants import AccountDataTypes, EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, @@ -29,7 +29,6 @@ SynapseError, ) from synapse.api.ratelimiting import Ratelimiter -from synapse.api.room_versions import RoomVersion from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.types import JsonDict, Requester, RoomAlias, RoomID, StateMap, UserID @@ -65,6 +64,7 @@ def __init__(self, hs: "HomeServer"): self.profile_handler = hs.get_profile_handler() self.event_creation_handler = hs.get_event_creation_handler() self.account_data_handler = hs.get_account_data_handler() + self.event_auth_handler = hs.get_event_auth_handler() self.member_linearizer = Linearizer(name="member") @@ -179,62 +179,6 @@ async def ratelimit_invite( await self._invites_per_user_limiter.ratelimit(requester, invitee_user_id) - async def can_join_without_invite( - self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str - ) -> bool: - """ - Check whether a user can join a room without an invite. - - When joining a room with restricted joined rules (as defined in MSC3083), - the membership of spaces must be checked during join. - - Args: - state_ids: The state of the room as it currently is. - room_version: The room version of the room being joined. - user_id: The user joining the room. - - Returns: - True if the user can join the room, false otherwise. - """ - # This only applies to room versions which support the new join rule. - if not room_version.msc3083_join_rules: - return True - - # If there's no join rule, then it defaults to public (so this doesn't apply). - join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None) - if not join_rules_event_id: - return True - - # If the join rule is not restricted, this doesn't apply. - join_rules_event = await self.store.get_event(join_rules_event_id) - if join_rules_event.content.get("join_rule") != JoinRules.MSC3083_RESTRICTED: - return True - - # If allowed is of the wrong form, then only allow invited users. - allowed_spaces = join_rules_event.content.get("allow", []) - if not isinstance(allowed_spaces, list): - return False - - # Get the list of joined rooms and see if there's an overlap. - joined_rooms = await self.store.get_rooms_for_user(user_id) - - # Pull out the other room IDs, invalid data gets filtered. - for space in allowed_spaces: - if not isinstance(space, dict): - continue - - space_id = space.get("space") - if not isinstance(space_id, str): - continue - - # The user was joined to one of the spaces specified, they can join - # this room! - if space_id in joined_rooms: - return True - - # The user was not in any of the required spaces. - return False - async def _local_membership_update( self, requester: Requester, @@ -303,7 +247,7 @@ async def _local_membership_update( if ( newly_joined and not user_is_invited - and not await self.can_join_without_invite( + and not await self.event_auth_handler.can_join_without_invite( prev_state_ids, event.room_version, user_id ) ): diff --git a/synapse/server.py b/synapse/server.py index cfb55c230ded..f8746bfa8650 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -78,6 +78,7 @@ from synapse.handlers.directory import DirectoryHandler from synapse.handlers.e2e_keys import E2eKeysHandler from synapse.handlers.e2e_room_keys import E2eRoomKeysHandler +from synapse.handlers.event_auth import EventAuthHandler from synapse.handlers.events import EventHandler, EventStreamHandler from synapse.handlers.federation import FederationHandler from synapse.handlers.groups_local import GroupsLocalHandler, GroupsLocalWorkerHandler @@ -743,6 +744,10 @@ def get_account_data_handler(self) -> AccountDataHandler: def get_space_summary_handler(self) -> SpaceSummaryHandler: return SpaceSummaryHandler(self) + @cache_in_self + def get_event_auth_handler(self) -> EventAuthHandler: + return EventAuthHandler(self) + @cache_in_self def get_external_cache(self) -> ExternalCache: return ExternalCache(self)