From 5a7fa5a19494ac194242a6d85257c8b77ccde153 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 26 Aug 2020 12:27:19 +0100 Subject: [PATCH 1/5] Remove unused is_guest parameter from get_room_data --- synapse/handlers/message.py | 10 +++------- synapse/rest/client/v1/room.py | 1 - tests/rest/client/test_retention.py | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 02d624268bee..ec8632adc153 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,7 +104,6 @@ async def get_room_data( room_id event_type state_key - is_guest Returns: The path data content. Raises: @@ -130,6 +124,8 @@ async def get_room_data( [membership_event_id], StateFilter.from_types([key]) ) data = room_state[membership_event_id].get(key) + else: + 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="" ) ) From f36d9a9abdb361f8453e84447fe52c7fe8db3bab Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 26 Aug 2020 12:29:26 +0100 Subject: [PATCH 2/5] Changelog --- changelog.d/8174.misc | 1 + 1 file changed, 1 insertion(+) 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..44baadd02725 --- /dev/null +++ b/changelog.d/8174.misc @@ -0,0 +1 @@ +Remove unused `is_guest` parameter from `MessageHandler.get_room_data`. \ No newline at end of file From 79d11b1269e18fb49d33f4fbffb80b4df82ad90d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 26 Aug 2020 14:17:01 +0100 Subject: [PATCH 3/5] Clarify what exceptions get_room_data can raise and why --- synapse/handlers/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index ec8632adc153..55c3386c5cdf 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -107,7 +107,7 @@ async def get_room_data( Returns: The path data content. Raises: - SynapseError if something went wrong. + SynapseError or AuthError if the user is not in the room """ ( membership, From dacfae69385bed0be0930ae267a41456cd86f7f8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 26 Aug 2020 14:25:23 +0100 Subject: [PATCH 4/5] Add log line and explanation --- synapse/handlers/message.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 55c3386c5cdf..9d0c38f4df02 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -125,6 +125,14 @@ async def get_room_data( ) 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 From c4a4a553b44ac659f765f1e655bdc19430d9ae0d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 26 Aug 2020 15:05:18 +0100 Subject: [PATCH 5/5] Update changelog --- changelog.d/8174.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/8174.misc b/changelog.d/8174.misc index 44baadd02725..a39e9eab46cd 100644 --- a/changelog.d/8174.misc +++ b/changelog.d/8174.misc @@ -1 +1 @@ -Remove unused `is_guest` parameter from `MessageHandler.get_room_data`. \ No newline at end of file +Remove unused `is_guest` parameter from, and add safeguard to, `MessageHandler.get_room_data`. \ No newline at end of file