From e0d6244beb0165417a2817f8b36c828ad22f8dbd Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Wed, 26 Aug 2020 15:07:35 +0100
Subject: [PATCH] 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.
---
 changelog.d/8174.misc               |  1 +
 synapse/handlers/message.py         | 20 ++++++++++++--------
 synapse/rest/client/v1/room.py      |  1 -
 tests/rest/client/test_retention.py |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)
 create mode 100644 changelog.d/8174.misc

diff --git a/changelog.d/8174.misc b/changelog.d/8174.misc
new file mode 100644
index 000000000000..a39e9eab46cd
--- /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 02d624268bee..9d0c38f4df02 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -95,12 +95,7 @@ def __init__(self, hs):
             )
 
     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 @@ async def get_room_data(
             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 @@ async def get_room_data(
                 [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 11da8bc0371f..88245fc177dd 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -171,7 +171,6 @@ async def on_GET(self, request, room_id, event_type, state_key):
             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 d4e7fa129334..7d3773ff7835 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -178,7 +178,7 @@ def _test_retention_event_purged(self, room_id: str, increment: float):
         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=""
             )
         )