From e63c17b14ba967208a8f6dcebcf37736bc9e32fb Mon Sep 17 00:00:00 2001
From: Sean Quah <seanq@element.io>
Date: Tue, 10 May 2022 19:41:22 +0100
Subject: [PATCH] Fix room upgrades creating an empty room when auth fails

Signed-off-by: Sean Quah <seanq@element.io>
---
 synapse/handlers/room.py | 64 ++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 25 deletions(-)

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,