From bcb930afc97d123444a6996c9f987c4f9052eb39 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Tue, 10 May 2022 19:41:22 +0100 Subject: [PATCH 01/10] Fix room upgrades creating an empty room when auth fails Signed-off-by: Sean Quah --- changelog.d/12696.bugfix | 1 + synapse/handlers/room.py | 64 ++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 25 deletions(-) create mode 100644 changelog.d/12696.bugfix diff --git a/changelog.d/12696.bugfix b/changelog.d/12696.bugfix new file mode 100644 index 000000000000..e410184a22af --- /dev/null +++ b/changelog.d/12696.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where an empty room would be created when a user with an insufficient power level tried to upgrade a room. diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 604eb6ec154a..ef4d83799ef7 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -33,6 +33,7 @@ import attr from typing_extensions import TypedDict +import synapse.events.snapshot from synapse.api.constants import ( EventContentFields, EventTypes, @@ -196,6 +197,31 @@ async def upgrade_room( 400, "An upgrade for this room is currently in progress" ) + # Check whether the user has the power level to carry out the upgrade. + # `check_auth_rules_from_context` will check that they are in the room and have + # the required power level to send the tombstone event. + ( + tombstone_event, + tombstone_context, + ) = await self.event_creation_handler.create_event( + requester, + { + "type": EventTypes.Tombstone, + "state_key": "", + "room_id": old_room_id, + "sender": user_id, + "content": { + "body": "This room has been replaced", + "replacement_room": None, # Use a placeholder value for now + }, + }, + ) + old_room_version = await self.store.get_room_version(old_room_id) + validate_event_for_room_version(old_room_version, tombstone_event) + await self._event_auth_handler.check_auth_rules_from_context( + old_room_version, tombstone_event, tombstone_context + ) + # Upgrade the room # # If this user has sent multiple upgrade requests for the same room @@ -207,18 +233,29 @@ async def upgrade_room( requester, old_room_id, new_version, # args for _upgrade_room + tombstone_event, + tombstone_context, ) return ret async def _upgrade_room( - self, requester: Requester, old_room_id: str, new_version: RoomVersion + self, + requester: Requester, + old_room_id: str, + new_version: RoomVersion, + tombstone_event: EventBase, + tombstone_context: synapse.events.snapshot.EventContext, ) -> str: """ Args: requester: the user requesting the upgrade old_room_id: the id of the room to be replaced new_versions: the version to upgrade the room to + tombstone_event: a template for the tombstone event to send to the old room. + `content["replacement_room"]` will be filled in the the id of the new + room. + tombstone_context: the context for the tombstone event Raises: ShadowBanError if the requester is shadow-banned. @@ -237,30 +274,7 @@ async def _upgrade_room( ) logger.info("Creating new room %s to replace %s", new_room_id, old_room_id) - - # we create and auth the tombstone event before properly creating the new - # room, to check our user has perms in the old room. - ( - tombstone_event, - tombstone_context, - ) = await self.event_creation_handler.create_event( - requester, - { - "type": EventTypes.Tombstone, - "state_key": "", - "room_id": old_room_id, - "sender": user_id, - "content": { - "body": "This room has been replaced", - "replacement_room": new_room_id, - }, - }, - ) - old_room_version = await self.store.get_room_version(old_room_id) - validate_event_for_room_version(old_room_version, tombstone_event) - await self._event_auth_handler.check_auth_rules_from_context( - old_room_version, tombstone_event, tombstone_context - ) + tombstone_event.content["replacement_room"] = new_room_id await self.clone_existing_room( requester, From ad30ac975707e82d17e4707ddb04ce5428dcca28 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 11 May 2022 15:30:00 +0100 Subject: [PATCH 02/10] Accept `Collection`s instead of `List`s in `EventCreationHandler.create_event` Signed-off-by: Sean Quah --- synapse/events/builder.py | 10 +++++----- synapse/handlers/message.py | 14 +++++++------- synapse/storage/databases/main/event_federation.py | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 98c203ada03b..05690e6a9fc2 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Tuple, Union import attr from signedjson.types import SigningKey @@ -101,8 +101,8 @@ def is_state(self) -> bool: async def build( self, - prev_event_ids: List[str], - auth_event_ids: Optional[List[str]], + prev_event_ids: Collection[str], + auth_event_ids: Optional[Collection[str]], depth: Optional[int] = None, ) -> EventBase: """Transform into a fully signed and hashed event @@ -135,8 +135,8 @@ async def build( auth_events = await self._store.add_event_hashes(auth_event_ids) prev_events = await self._store.add_event_hashes(prev_event_ids) else: - auth_events = auth_event_ids - prev_events = prev_event_ids + auth_events = list(auth_event_ids) + prev_events = list(prev_event_ids) # Otherwise, progress the depth as normal if depth is None: diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index c28b792e6fe2..147836659483 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -17,7 +17,7 @@ import logging import random from http import HTTPStatus -from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Tuple +from typing import TYPE_CHECKING, Any, Collection, Dict, List, Mapping, Optional, Tuple from canonicaljson import encode_canonical_json @@ -487,9 +487,9 @@ async def create_event( event_dict: dict, txn_id: Optional[str] = None, allow_no_prev_events: bool = False, - prev_event_ids: Optional[List[str]] = None, - auth_event_ids: Optional[List[str]] = None, - state_event_ids: Optional[List[str]] = None, + prev_event_ids: Optional[Collection[str]] = None, + auth_event_ids: Optional[Collection[str]] = None, + state_event_ids: Optional[Collection[str]] = None, require_consent: bool = True, outlier: bool = False, historical: bool = False, @@ -901,9 +901,9 @@ async def create_new_client_event( builder: EventBuilder, requester: Optional[Requester] = None, allow_no_prev_events: bool = False, - prev_event_ids: Optional[List[str]] = None, - auth_event_ids: Optional[List[str]] = None, - state_event_ids: Optional[List[str]] = None, + prev_event_ids: Optional[Collection[str]] = None, + auth_event_ids: Optional[Collection[str]] = None, + state_event_ids: Optional[Collection[str]] = None, depth: Optional[int] = None, ) -> Tuple[EventBase, EventContext]: """Create a new event for a local client diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 471022470843..2464471b5748 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -788,7 +788,7 @@ def get_insertion_event_backward_extremities_in_room_txn(txn, room_id): room_id, ) - async def get_max_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: + async def get_max_depth_of(self, event_ids: Iterable[str]) -> Tuple[str, int]: """Returns the event ID and depth for the event that has the max depth from a set of event IDs Args: @@ -817,7 +817,7 @@ async def get_max_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: return max_depth_event_id, current_max_depth - async def get_min_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: + async def get_min_depth_of(self, event_ids: Iterable[str]) -> Tuple[str, int]: """Returns the event ID and depth for the event that has the min depth from a set of event IDs Args: From d9797540558848acbf7378001ee415536060db8c Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 11 May 2022 15:38:23 +0100 Subject: [PATCH 03/10] Don't try to modify an event that has already been signed --- synapse/handlers/room.py | 110 ++++++++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 31 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index ef4d83799ef7..ab13ddf03068 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -197,29 +197,16 @@ async def upgrade_room( 400, "An upgrade for this room is currently in progress" ) - # Check whether the user has the power level to carry out the upgrade. - # `check_auth_rules_from_context` will check that they are in the room and have - # the required power level to send the tombstone event. - ( - tombstone_event, - tombstone_context, - ) = await self.event_creation_handler.create_event( + # Before starting the room upgrade, check whether the user has the power level + # to carry out the upgrade, by creating a dummy tombstone event. The auth checks + # will check for room membership and the required power level to send the + # tombstone. We reuse the prev events and auth events from the dummy event for + # the real tombstone. That way if the dummy event passes the auth checks, the + # real tombstone should pass auth checks too. + tombstone_event, _ = await self._create_tombstone_and_check_auth( requester, - { - "type": EventTypes.Tombstone, - "state_key": "", - "room_id": old_room_id, - "sender": user_id, - "content": { - "body": "This room has been replaced", - "replacement_room": None, # Use a placeholder value for now - }, - }, - ) - old_room_version = await self.store.get_room_version(old_room_id) - validate_event_for_room_version(old_room_version, tombstone_event) - await self._event_auth_handler.check_auth_rules_from_context( - old_room_version, tombstone_event, tombstone_context + old_room_id, + new_room_id=None, ) # Upgrade the room @@ -233,8 +220,7 @@ async def upgrade_room( requester, old_room_id, new_version, # args for _upgrade_room - tombstone_event, - tombstone_context, + tombstone_auth_template=tombstone_event, ) return ret @@ -244,18 +230,16 @@ async def _upgrade_room( requester: Requester, old_room_id: str, new_version: RoomVersion, - tombstone_event: EventBase, - tombstone_context: synapse.events.snapshot.EventContext, + tombstone_auth_template: EventBase, ) -> str: """ Args: requester: the user requesting the upgrade old_room_id: the id of the room to be replaced new_versions: the version to upgrade the room to - tombstone_event: a template for the tombstone event to send to the old room. - `content["replacement_room"]` will be filled in the the id of the new - room. - tombstone_context: the context for the tombstone event + tombstone_auth_template: a template for the tombstone event to send to the + old room. `tombstone_auth_template`'s prev and auth events will be used + to create the tombstone event. Raises: ShadowBanError if the requester is shadow-banned. @@ -274,7 +258,16 @@ async def _upgrade_room( ) logger.info("Creating new room %s to replace %s", new_room_id, old_room_id) - tombstone_event.content["replacement_room"] = new_room_id + + ( + tombstone_event, + tombstone_context, + ) = await self._create_tombstone_and_check_auth( + requester, + old_room_id, + new_room_id, + copy_auth_from_event=tombstone_auth_template, + ) await self.clone_existing_room( requester, @@ -317,6 +310,61 @@ async def _upgrade_room( return new_room_id + async def _create_tombstone_and_check_auth( + self, + requester: Requester, + old_room_id: str, + new_room_id: Optional[str], + copy_auth_from_event: Optional[EventBase] = None, + ) -> Tuple[EventBase, synapse.events.snapshot.EventContext]: + """Creates a tombstone event for a room upgrade and checks that it can be sent. + + Args: + requester: the user requesting the upgrade + old_room_id: the id of the room to be replaced + new_room_id: the id of the replacement room, or `None` if not known yet + copy_auth_from_event: an event whose prev and auth events are to be used for + the tombstone event, if provided + + Returns: + A tuple containing the tombstone event and its context. + + Raises: + AuthError if the requester cannot send the tombstone event, eg. if they are + not in the room or do not have the required power level. + """ + user_id = requester.user.to_string() + + ( + tombstone_event, + tombstone_context, + ) = await self.event_creation_handler.create_event( + requester, + { + "type": EventTypes.Tombstone, + "state_key": "", + "room_id": old_room_id, + "sender": user_id, + "content": { + "body": "This room has been replaced", + "replacement_room": new_room_id, + }, + }, + prev_event_ids=( + copy_auth_from_event.prev_event_ids() if copy_auth_from_event else None + ), + auth_event_ids=( + copy_auth_from_event.auth_event_ids() if copy_auth_from_event else None + ), + ) + old_room_version = await self.store.get_room_version(old_room_id) + validate_event_for_room_version(old_room_version, tombstone_event) + await self._event_auth_handler.check_auth_rules_from_context( + old_room_version, tombstone_event, tombstone_context + ) + + return tombstone_event, tombstone_context + async def _update_upgraded_room_pls( self, requester: Requester, From 09c68bbafc3c7d118884fc65f63f1ca103affb8b Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 11 May 2022 17:02:49 +0100 Subject: [PATCH 04/10] Fix regression: an upgrade of a non-existant room should 404 instead of 403 --- synapse/handlers/room.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index ab13ddf03068..d45a8377c0fe 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -197,6 +197,12 @@ async def upgrade_room( 400, "An upgrade for this room is currently in progress" ) + # Check whether the room exists and 404 if it doesn't. + # We could go straight for the auth check, but that will raise a 403 instead. + old_room = await self.store.get_room(old_room_id) + if old_room is None: + raise NotFoundError("Unknown room id %s" % (old_room_id,)) + # Before starting the room upgrade, check whether the user has the power level # to carry out the upgrade, by creating a dummy tombstone event. The auth checks # will check for room membership and the required power level to send the @@ -220,6 +226,7 @@ async def upgrade_room( requester, old_room_id, new_version, # args for _upgrade_room + old_room, tombstone_auth_template=tombstone_event, ) @@ -230,6 +237,7 @@ async def _upgrade_room( requester: Requester, old_room_id: str, new_version: RoomVersion, + old_room: Dict[str, Any], tombstone_auth_template: EventBase, ) -> str: """ @@ -237,6 +245,8 @@ async def _upgrade_room( requester: the user requesting the upgrade old_room_id: the id of the room to be replaced new_versions: the version to upgrade the room to + old_room: a dict containing room information for the room the be replaced, + as returned by `RoomWorkerStore.get_room`. tombstone_auth_template: a template for the tombstone event to send to the old room. `tombstone_auth_template`'s prev and auth events will be used to create the tombstone event. @@ -248,12 +258,9 @@ async def _upgrade_room( assert self.hs.is_mine_id(user_id), "User must be our own: %s" % (user_id,) # start by allocating a new room id - r = await self.store.get_room(old_room_id) - if r is None: - raise NotFoundError("Unknown room id %s" % (old_room_id,)) new_room_id = await self._generate_room_id( creator_id=user_id, - is_public=r["is_public"], + is_public=old_room["is_public"], room_version=new_version, ) From 19cfeb06201d37f937c4008d3e3b7fb20f419db4 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 11 May 2022 17:04:35 +0100 Subject: [PATCH 05/10] Fix typo --- synapse/handlers/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index d45a8377c0fe..b73016bf0f78 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -245,7 +245,7 @@ async def _upgrade_room( requester: the user requesting the upgrade old_room_id: the id of the room to be replaced new_versions: the version to upgrade the room to - old_room: a dict containing room information for the room the be replaced, + old_room: a dict containing room information for the room to be replaced, as returned by `RoomWorkerStore.get_room`. tombstone_auth_template: a template for the tombstone event to send to the old room. `tombstone_auth_template`'s prev and auth events will be used From d0ea9dde3adead2ac84dcbd69a6f246a21150c8a Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 13 May 2022 15:20:46 +0100 Subject: [PATCH 06/10] Split RoomCreationHandler._generate_room_id() into two methods --- synapse/handlers/room.py | 28 +++++++++++++++---- .../test_sharded_event_persister.py | 13 +-------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index b73016bf0f78..82ff5dd05dbc 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -258,7 +258,7 @@ async def _upgrade_room( assert self.hs.is_mine_id(user_id), "User must be our own: %s" % (user_id,) # start by allocating a new room id - new_room_id = await self._generate_room_id( + new_room_id = await self._generate_and_create_room_id( creator_id=user_id, is_public=old_room["is_public"], room_version=new_version, @@ -847,7 +847,7 @@ async def create_room( visibility = config.get("visibility", "private") is_public = visibility == "public" - room_id = await self._generate_room_id( + room_id = await self._generate_and_create_room_id( creator_id=user_id, is_public=is_public, room_version=room_version, @@ -1159,7 +1159,26 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int: return last_sent_stream_id - async def _generate_room_id( + def _generate_room_id(self) -> str: + """Generates a random room ID. + + Room IDs look like "!opaque_id:domain" and are case-sensitive as per the spec + at https://spec.matrix.org/v1.2/appendices/#room-ids-and-event-ids. + + Does not check for collisions with existing rooms or prevent future calls from + returning the same room ID. To ensure the uniqueness of a new room ID, use + `_generate_and_create_room_id` instead. + + Synapse's room IDs are 18 [a-zA-Z] characters long, which comes out to around + 102 bits. + + Returns: + A random room ID of the form "!opaque_id:domain". + """ + random_string = stringutils.random_string(18) + return RoomID(random_string, self.hs.hostname).to_string() + + async def _generate_and_create_room_id( self, creator_id: str, is_public: bool, @@ -1170,8 +1189,7 @@ async def _generate_room_id( attempts = 0 while attempts < 5: try: - random_string = stringutils.random_string(18) - gen_room_id = RoomID(random_string, self.hs.hostname).to_string() + gen_room_id = self._generate_room_id() await self.store.store_room( room_id=gen_room_id, room_creator_user_id=creator_id, diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index 5f142e84c359..6d198c19a364 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -64,21 +64,10 @@ def _create_room(self, room_id: str, user_id: str, tok: str): # We control the room ID generation by patching out the # `_generate_room_id` method - async def generate_room( - creator_id: str, is_public: bool, room_version: RoomVersion - ): - await self.store.store_room( - room_id=room_id, - room_creator_user_id=creator_id, - is_public=is_public, - room_version=room_version, - ) - return room_id - with patch( "synapse.handlers.room.RoomCreationHandler._generate_room_id" ) as mock: - mock.side_effect = generate_room + mock.side_effect = lambda: room_id self.helper.create_room_as(user_id, tok=tok) def test_basic(self): From fc3659fdaab482a2e0b89f33f1f5835aed695ef4 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 13 May 2022 15:21:23 +0100 Subject: [PATCH 07/10] Re-fix the bug --- synapse/handlers/room.py | 132 +++++++++++++-------------------------- 1 file changed, 44 insertions(+), 88 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 82ff5dd05dbc..c0206b7ac9be 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -203,16 +203,31 @@ async def upgrade_room( if old_room is None: raise NotFoundError("Unknown room id %s" % (old_room_id,)) - # Before starting the room upgrade, check whether the user has the power level - # to carry out the upgrade, by creating a dummy tombstone event. The auth checks - # will check for room membership and the required power level to send the - # tombstone. We reuse the prev events and auth events from the dummy event for - # the real tombstone. That way if the dummy event passes the auth checks, the - # real tombstone should pass auth checks too. - tombstone_event, _ = await self._create_tombstone_and_check_auth( + new_room_id = self._generate_room_id() + + # Check whether the user has the power level to carry out the upgrade. + # `check_auth_rules_from_context` will check that they are in the room and have + # the required power level to send the tombstone event. + ( + tombstone_event, + tombstone_context, + ) = await self.event_creation_handler.create_event( requester, - old_room_id, - new_room_id=None, + { + "type": EventTypes.Tombstone, + "state_key": "", + "room_id": old_room_id, + "sender": user_id, + "content": { + "body": "This room has been replaced", + "replacement_room": new_room_id, + }, + }, + ) + old_room_version = await self.store.get_room_version(old_room_id) + validate_event_for_room_version(old_room_version, tombstone_event) + await self._event_auth_handler.check_auth_rules_from_context( + old_room_version, tombstone_event, tombstone_context ) # Upgrade the room @@ -225,9 +240,11 @@ async def upgrade_room( self._upgrade_room, requester, old_room_id, - new_version, # args for _upgrade_room - old_room, - tombstone_auth_template=tombstone_event, + old_room, # args for _upgrade_room + new_room_id, + new_version, + tombstone_event, + tombstone_context, ) return ret @@ -236,20 +253,22 @@ async def _upgrade_room( self, requester: Requester, old_room_id: str, - new_version: RoomVersion, old_room: Dict[str, Any], - tombstone_auth_template: EventBase, + new_room_id: str, + new_version: RoomVersion, + tombstone_event: EventBase, + tombstone_context: synapse.events.snapshot.EventContext, ) -> str: """ Args: requester: the user requesting the upgrade old_room_id: the id of the room to be replaced - new_versions: the version to upgrade the room to old_room: a dict containing room information for the room to be replaced, as returned by `RoomWorkerStore.get_room`. - tombstone_auth_template: a template for the tombstone event to send to the - old room. `tombstone_auth_template`'s prev and auth events will be used - to create the tombstone event. + new_room_id: the id of the replacement room + new_version: the version to upgrade the room to + tombstone_event: the tombstone event to send to the old room + tombstone_context: the context for the tombstone event Raises: ShadowBanError if the requester is shadow-banned. @@ -257,23 +276,15 @@ async def _upgrade_room( user_id = requester.user.to_string() assert self.hs.is_mine_id(user_id), "User must be our own: %s" % (user_id,) - # start by allocating a new room id - new_room_id = await self._generate_and_create_room_id( - creator_id=user_id, - is_public=old_room["is_public"], - room_version=new_version, - ) - logger.info("Creating new room %s to replace %s", new_room_id, old_room_id) - ( - tombstone_event, - tombstone_context, - ) = await self._create_tombstone_and_check_auth( - requester, - old_room_id, - new_room_id, - copy_auth_from_event=tombstone_auth_template, + # create the new room. may raise a `StoreError` in the exceedingly unlikely + # event of a room ID collision. + await self.store.store_room( + room_id=new_room_id, + room_creator_user_id=user_id, + is_public=old_room["is_public"], + room_version=new_version, ) await self.clone_existing_room( @@ -317,61 +328,6 @@ async def _upgrade_room( return new_room_id - async def _create_tombstone_and_check_auth( - self, - requester: Requester, - old_room_id: str, - new_room_id: Optional[str], - copy_auth_from_event: Optional[EventBase] = None, - ) -> Tuple[EventBase, synapse.events.snapshot.EventContext]: - """Creates a tombstone event for a room upgrade and checks that it can be sent. - - Args: - requester: the user requesting the upgrade - old_room_id: the id of the room to be replaced - new_room_id: the id of the replacement room, or `None` if not known yet - copy_auth_from_event: an event whose prev and auth events are to be used for - the tombstone event, if provided - - Returns: - A tuple containing the tombstone event and its context. - - Raises: - AuthError if the requester cannot send the tombstone event, eg. if they are - not in the room or do not have the required power level. - """ - user_id = requester.user.to_string() - - ( - tombstone_event, - tombstone_context, - ) = await self.event_creation_handler.create_event( - requester, - { - "type": EventTypes.Tombstone, - "state_key": "", - "room_id": old_room_id, - "sender": user_id, - "content": { - "body": "This room has been replaced", - "replacement_room": new_room_id, - }, - }, - prev_event_ids=( - copy_auth_from_event.prev_event_ids() if copy_auth_from_event else None - ), - auth_event_ids=( - copy_auth_from_event.auth_event_ids() if copy_auth_from_event else None - ), - ) - old_room_version = await self.store.get_room_version(old_room_id) - validate_event_for_room_version(old_room_version, tombstone_event) - await self._event_auth_handler.check_auth_rules_from_context( - old_room_version, tombstone_event, tombstone_context - ) - - return tombstone_event, tombstone_context - async def _update_upgraded_room_pls( self, requester: Requester, From 15d0939ba11fb6d86fc5ed44cbb1f007a57a8204 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 13 May 2022 15:13:29 +0100 Subject: [PATCH 08/10] Revert "Accept `Collection`s instead of `List`s in `EventCreationHandler.create_event`" This reverts commit ad30ac975707e82d17e4707ddb04ce5428dcca28. --- synapse/events/builder.py | 10 +++++----- synapse/handlers/message.py | 14 +++++++------- synapse/storage/databases/main/event_federation.py | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 05690e6a9fc2..98c203ada03b 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union import attr from signedjson.types import SigningKey @@ -101,8 +101,8 @@ def is_state(self) -> bool: async def build( self, - prev_event_ids: Collection[str], - auth_event_ids: Optional[Collection[str]], + prev_event_ids: List[str], + auth_event_ids: Optional[List[str]], depth: Optional[int] = None, ) -> EventBase: """Transform into a fully signed and hashed event @@ -135,8 +135,8 @@ async def build( auth_events = await self._store.add_event_hashes(auth_event_ids) prev_events = await self._store.add_event_hashes(prev_event_ids) else: - auth_events = list(auth_event_ids) - prev_events = list(prev_event_ids) + auth_events = auth_event_ids + prev_events = prev_event_ids # Otherwise, progress the depth as normal if depth is None: diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 147836659483..c28b792e6fe2 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -17,7 +17,7 @@ import logging import random from http import HTTPStatus -from typing import TYPE_CHECKING, Any, Collection, Dict, List, Mapping, Optional, Tuple +from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Tuple from canonicaljson import encode_canonical_json @@ -487,9 +487,9 @@ async def create_event( event_dict: dict, txn_id: Optional[str] = None, allow_no_prev_events: bool = False, - prev_event_ids: Optional[Collection[str]] = None, - auth_event_ids: Optional[Collection[str]] = None, - state_event_ids: Optional[Collection[str]] = None, + prev_event_ids: Optional[List[str]] = None, + auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, require_consent: bool = True, outlier: bool = False, historical: bool = False, @@ -901,9 +901,9 @@ async def create_new_client_event( builder: EventBuilder, requester: Optional[Requester] = None, allow_no_prev_events: bool = False, - prev_event_ids: Optional[Collection[str]] = None, - auth_event_ids: Optional[Collection[str]] = None, - state_event_ids: Optional[Collection[str]] = None, + prev_event_ids: Optional[List[str]] = None, + auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, ) -> Tuple[EventBase, EventContext]: """Create a new event for a local client diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 2464471b5748..471022470843 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -788,7 +788,7 @@ def get_insertion_event_backward_extremities_in_room_txn(txn, room_id): room_id, ) - async def get_max_depth_of(self, event_ids: Iterable[str]) -> Tuple[str, int]: + async def get_max_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: """Returns the event ID and depth for the event that has the max depth from a set of event IDs Args: @@ -817,7 +817,7 @@ async def get_max_depth_of(self, event_ids: Iterable[str]) -> Tuple[str, int]: return max_depth_event_id, current_max_depth - async def get_min_depth_of(self, event_ids: Iterable[str]) -> Tuple[str, int]: + async def get_min_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: """Returns the event ID and depth for the event that has the min depth from a set of event IDs Args: From 3139557eb2f325fffe2a779babedc12969d5ee7a Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 13 May 2022 14:15:56 +0100 Subject: [PATCH 09/10] Remove unused room upgrade Linearizer --- synapse/handlers/room.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index c0206b7ac9be..f8db4d92c7c4 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -78,7 +78,6 @@ create_requester, ) from synapse.util import stringutils -from synapse.util.async_helpers import Linearizer from synapse.util.caches.response_cache import ResponseCache from synapse.util.stringutils import parse_and_validate_server_name from synapse.visibility import filter_events_for_client @@ -152,9 +151,6 @@ def __init__(self, hs: "HomeServer"): self._replication = hs.get_replication_data_handler() - # linearizer to stop two upgrades happening at once - self._upgrade_linearizer = Linearizer("room_upgrade_linearizer") - # If a user tries to update the same room multiple times in quick # succession, only process the first attempt and return its result to # subsequent requests From 0af099738e3362f3f080db7a938eb21a4ecb4898 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 13 May 2022 15:27:13 +0100 Subject: [PATCH 10/10] Remove unused import --- tests/replication/test_sharded_event_persister.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index 6d198c19a364..a7ca68069e86 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -14,7 +14,6 @@ import logging from unittest.mock import patch -from synapse.api.room_versions import RoomVersion from synapse.rest import admin from synapse.rest.client import login, room, sync from synapse.storage.util.id_generators import MultiWriterIdGenerator