Remove unused parameter from, and add safeguard in, get_room_data (#8174)

Small cleanup PR.

* Removed the unused `is_guest` argument
* Added a safeguard to a (currently) impossible code path, fixing static checking at the same time.

4 files changed, 14 insertions, 10 deletions

diff --git a/changelog.d/8174.misc b/changelog.d/8174.misc
new file mode 100644
index 0000000000..a39e9eab46
--- /dev/null
+++ b/changelog.d/8174.misc
@@ -0,0 +1 @@
+Remove unused `is_guest` parameter from, and add safeguard to, `MessageHandler.get_room_data`.
\ No newline at end of file
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 02d624268b..9d0c38f4df 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -95,12 +95,7 @@ class MessageHandler(object):
             )
 
     async def get_room_data(
-        self,
-        user_id: str,
-        room_id: str,
-        event_type: str,
-        state_key: str,
-        is_guest: bool,
+        self, user_id: str, room_id: str, event_type: str, state_key: str,
     ) -> dict:
         """ Get data from a room.
 
@@ -109,11 +104,10 @@ class MessageHandler(object):
             room_id
             event_type
             state_key
-            is_guest
         Returns:
             The path data content.
         Raises:
-            SynapseError if something went wrong.
+            SynapseError or AuthError if the user is not in the room
         """
         (
             membership,
@@ -130,6 +124,16 @@ class MessageHandler(object):
                 [membership_event_id], StateFilter.from_types([key])
             )
             data = room_state[membership_event_id].get(key)
+        else:
+            # check_user_in_room_or_world_readable, if it doesn't raise an AuthError, should
+            # only ever return a Membership.JOIN/LEAVE object
+            #
+            # Safeguard in case it returned something else
+            logger.error(
+                "Attempted to retrieve data from a room for a user that has never been in it. "
+                "This should not have happened."
+            )
+            raise SynapseError(403, "User not in room", errcode=Codes.FORBIDDEN)
 
         return data
 
diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 11da8bc037..88245fc177 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -171,7 +171,6 @@ class RoomStateEventRestServlet(TransactionRestServlet):
             room_id=room_id,
             event_type=event_type,
             state_key=state_key,
-            is_guest=requester.is_guest,
         )
 
         if not data:
diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
index d4e7fa1293..7d3773ff78 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -178,7 +178,7 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         message_handler = self.hs.get_message_handler()
         create_event = self.get_success(
             message_handler.get_room_data(
-                self.user_id, room_id, EventTypes.Create, state_key="", is_guest=False
+                self.user_id, room_id, EventTypes.Create, state_key=""
             )
         )