fix: Add is_requester_admin parameter to upgrade room callback (#21)

This is for consistency with the room create API.

synapse/handlers/room.py

@@ -220,7 +220,18 @@ async def upgrade_room(
         if old_room is None:
             raise NotFoundError("Unknown room id %s" % (old_room_id,))
 
-        await self._third_party_event_rules.on_upgrade_room(requester, new_version)
+        if (
+            self._server_notices_mxid is not None
+            and user_id == self._server_notices_mxid
+        ):
+            # allow the server notices mxid to create rooms
+            is_requester_admin = True
+        else:
+            is_requester_admin = await self.auth.is_server_admin(requester)
+
+        await self._third_party_event_rules.on_upgrade_room(
+            requester, new_version, is_requester_admin=is_requester_admin
+        )
 
         new_room_id = self._generate_room_id()
 

synapse/module_api/callbacks/third_party_event_rules_callbacks.py

@@ -55,7 +55,7 @@
 ON_THREEPID_BIND_CALLBACK = Callable[[str, str, str], Awaitable]
 ON_ADD_USER_THIRD_PARTY_IDENTIFIER_CALLBACK = Callable[[str, str, str], Awaitable]
 ON_REMOVE_USER_THIRD_PARTY_IDENTIFIER_CALLBACK = Callable[[str, str, str], Awaitable]
-ON_UPGRADE_ROOM_CALLBACK = Callable[[Requester, RoomVersion], Awaitable]
+ON_UPGRADE_ROOM_CALLBACK = Callable[[Requester, RoomVersion, bool], Awaitable]
 
 
 def load_legacy_third_party_event_rules(hs: "HomeServer") -> None:
@@ -605,17 +605,18 @@ async def on_remove_user_third_party_identifier(
                 )
 
     async def on_upgrade_room(
-        self, requester: Requester, room_version: RoomVersion
+        self, requester: Requester, room_version: RoomVersion, is_requester_admin: bool
     ) -> None:
         """Intercept requests to upgrade a room to maybe deny it (via an exception).
 
         Args:
             requester
             room_version: The RoomVersion requested for the upgrade.
+            is_requester_admin: If the requester is an admin
         """
         for callback in self._on_upgrade_room_callbacks:
             try:
-                await callback(requester, room_version)
+                await callback(requester, room_version, is_requester_admin)
             except Exception as e:
                 logger.exception(
                     "Failed to run module API callback %s: %s", callback, e

tests/rest/client/test_third_party_rules.py

@@ -106,9 +106,12 @@ def __init__(self, config: Dict, module_api: "ModuleApi") -> None:
         )
 
     async def on_upgrade_room(
-        self, requester: Requester, room_version: RoomVersion
+        self, requester: Requester, room_version: RoomVersion, is_requester_admin: bool
     ) -> None:
-        if room_version.identifier not in self.allowed_room_versions:
+        if (
+            not is_requester_admin
+            and room_version.identifier not in self.allowed_room_versions
+        ):
             raise SynapseError(
                 400,
                 "You can not upgrade room to that version",
@@ -510,6 +513,15 @@ def upgrade_room_to_version(
         )
         upgrade_room_to_version(room_id_2, "11", tok=self.tok, expect_code=400)
 
+        # Room not in configured list as admin
+        admin_user_id = self.register_user("admin_kermit", "monkey", admin=True)
+
+        admin_tok = self.login("admin_kermit", "monkey")
+        room_id_3 = self.helper.create_room_as(
+            admin_user_id, room_version="9", tok=admin_tok
+        )
+        upgrade_room_to_version(room_id_3, "11", tok=admin_tok, expect_code=200)
+
     def test_sent_event_end_up_in_room_state(self) -> None:
         """Tests that a state event sent by a module while processing another state event
         doesn't get dropped from the state of the room. This is to guard against a bug