Faster joins: fix rejected events becoming un-rejected during resync (#13413)

Make sure that we re-check the auth rules during state resync, otherwise
rejected events get un-rejected.
This commit is contained in:
Richard van der Hoff 2022-08-01 11:20:05 +01:00 committed by GitHub
parent d548d8f18d
commit 23768ccb4d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 6 deletions

1
changelog.d/13413.bugfix Normal file
View file

@ -0,0 +1 @@
Faster room joins: fix a bug which caused rejected events to become un-rejected during state syncing.

View file

@ -581,6 +581,13 @@ class FederationEventHandler:
event.event_id, event.event_id,
) )
return return
# since the state at this event has changed, we should now re-evaluate
# whether it should have been rejected. We must already have all of the
# auth events (from last time we went round this path), so there is no
# need to pass the origin.
await self._check_event_auth(None, event, context)
await self._store.update_state_for_partial_state_event(event, context) await self._store.update_state_for_partial_state_event(event, context)
self._state_storage_controller.notify_event_un_partial_stated( self._state_storage_controller.notify_event_un_partial_stated(
event.event_id event.event_id
@ -1624,13 +1631,15 @@ class FederationEventHandler:
) )
async def _check_event_auth( async def _check_event_auth(
self, origin: str, event: EventBase, context: EventContext self, origin: Optional[str], event: EventBase, context: EventContext
) -> None: ) -> None:
""" """
Checks whether an event should be rejected (for failing auth checks). Checks whether an event should be rejected (for failing auth checks).
Args: Args:
origin: The host the event originates from. origin: The host the event originates from. This is used to fetch
any missing auth events. It can be set to None, but only if we are
sure that we already have all the auth events.
event: The event itself. event: The event itself.
context: context:
The event context. The event context.
@ -1876,7 +1885,7 @@ class FederationEventHandler:
event.internal_metadata.soft_failed = True event.internal_metadata.soft_failed = True
async def _load_or_fetch_auth_events_for_event( async def _load_or_fetch_auth_events_for_event(
self, destination: str, event: EventBase self, destination: Optional[str], event: EventBase
) -> Collection[EventBase]: ) -> Collection[EventBase]:
"""Fetch this event's auth_events, from database or remote """Fetch this event's auth_events, from database or remote
@ -1892,12 +1901,19 @@ class FederationEventHandler:
Args: Args:
destination: where to send the /event_auth request. Typically the server destination: where to send the /event_auth request. Typically the server
that sent us `event` in the first place. that sent us `event` in the first place.
If this is None, no attempt is made to load any missing auth events:
rather, an AssertionError is raised if there are any missing events.
event: the event whose auth_events we want event: the event whose auth_events we want
Returns: Returns:
all of the events listed in `event.auth_events_ids`, after deduplication all of the events listed in `event.auth_events_ids`, after deduplication
Raises: Raises:
AssertionError if some auth events were missing and no `destination` was
supplied.
AuthError if we were unable to fetch the auth_events for any reason. AuthError if we were unable to fetch the auth_events for any reason.
""" """
event_auth_event_ids = set(event.auth_event_ids()) event_auth_event_ids = set(event.auth_event_ids())
@ -1909,6 +1925,13 @@ class FederationEventHandler:
) )
if not missing_auth_event_ids: if not missing_auth_event_ids:
return event_auth_events.values() return event_auth_events.values()
if destination is None:
# this shouldn't happen: destination must be set unless we know we have already
# persisted the auth events.
raise AssertionError(
"_load_or_fetch_auth_events_for_event() called with no destination for "
"an event with missing auth_events"
)
logger.info( logger.info(
"Event %s refers to unknown auth events %s: fetching auth chain", "Event %s refers to unknown auth events %s: fetching auth chain",

View file

@ -419,13 +419,15 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore):
# anything that was rejected should have the same state as its # anything that was rejected should have the same state as its
# predecessor. # predecessor.
if context.rejected: if context.rejected:
assert context.state_group == context.state_group_before_event state_group = context.state_group_before_event
else:
state_group = context.state_group
self.db_pool.simple_update_txn( self.db_pool.simple_update_txn(
txn, txn,
table="event_to_state_groups", table="event_to_state_groups",
keyvalues={"event_id": event.event_id}, keyvalues={"event_id": event.event_id},
updatevalues={"state_group": context.state_group}, updatevalues={"state_group": state_group},
) )
self.db_pool.simple_delete_one_txn( self.db_pool.simple_delete_one_txn(
@ -440,7 +442,7 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore):
txn.call_after( txn.call_after(
self._get_state_group_for_event.prefill, self._get_state_group_for_event.prefill,
(event.event_id,), (event.event_id,),
context.state_group, state_group,
) )