From 3ae80b0de4abeff2bdbbb9d8735057e6b301a670 Mon Sep 17 00:00:00 2001 From: Lama Date: Wed, 30 Oct 2024 19:55:23 +0300 Subject: [PATCH] Check if user is in room before being able to tag it (#17839) Fix #17819 --- changelog.d/17839.bugfix | 1 + synapse/handlers/room_member.py | 20 +++++++ synapse/rest/client/tags.py | 7 +++ tests/rest/client/test_tags.py | 95 +++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 changelog.d/17839.bugfix create mode 100644 tests/rest/client/test_tags.py diff --git a/changelog.d/17839.bugfix b/changelog.d/17839.bugfix new file mode 100644 index 0000000000..57667a6df5 --- /dev/null +++ b/changelog.d/17839.bugfix @@ -0,0 +1 @@ +Check if user has membership in a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 75c60e3c34..70cbbc352b 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1190,6 +1190,26 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): origin_server_ts=origin_server_ts, ) + async def check_for_any_membership_in_room( + self, *, user_id: str, room_id: str + ) -> None: + """ + Check if the user has any membership in the room and raise error if not. + + Args: + user_id: The user to check. + room_id: The room to check. + + Raises: + AuthError if the user doesn't have any membership in the room. + """ + result = await self.store.get_local_current_membership_for_user_in_room( + user_id=user_id, room_id=room_id + ) + + if result is None or result == (None, None): + raise AuthError(403, f"User {user_id} has no membership in room {room_id}") + async def _should_perform_remote_join( self, user_id: str, diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index 554bcb95dd..b6648f3499 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -78,6 +78,7 @@ class TagServlet(RestServlet): super().__init__() self.auth = hs.get_auth() self.handler = hs.get_account_data_handler() + self.room_member_handler = hs.get_room_member_handler() async def on_PUT( self, request: SynapseRequest, user_id: str, room_id: str, tag: str @@ -85,6 +86,12 @@ class TagServlet(RestServlet): requester = await self.auth.get_user_by_req(request) if user_id != requester.user.to_string(): raise AuthError(403, "Cannot add tags for other users.") + # Check if the user has any membership in the room and raise error if not. + # Although it's not harmful for users to tag random rooms, it's just superfluous + # data we don't need to track or allow. + await self.room_member_handler.check_for_any_membership_in_room( + user_id=user_id, room_id=room_id + ) body = parse_json_object_from_request(request) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py new file mode 100644 index 0000000000..5d596409e1 --- /dev/null +++ b/tests/rest/client/test_tags.py @@ -0,0 +1,95 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2024 New Vector, Ltd +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . +# + +"""Tests REST events for /tags paths.""" + +from http import HTTPStatus + +import synapse.rest.admin +from synapse.rest.client import login, room, tags + +from tests import unittest + + +class RoomTaggingTestCase(unittest.HomeserverTestCase): + """Tests /user/$user_id/rooms/$room_id/tags/$tag REST API.""" + + servlets = [ + room.register_servlets, + tags.register_servlets, + login.register_servlets, + synapse.rest.admin.register_servlets_for_client_rest_resource, + ] + + def test_put_tag_checks_room_membership(self) -> None: + """ + Test that a user can add a tag to a room if they have membership to the room. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + tag = "test_tag" + + # Make the request + channel = self.make_request( + "PUT", + f"/user/{user1_id}/rooms/{room_id}/tags/{tag}", + content={"order": 0.5}, + access_token=user1_tok, + ) + # Check that the request was successful + self.assertEqual(channel.code, HTTPStatus.OK, channel.result) + + def test_put_tag_fails_if_not_in_room(self) -> None: + """ + Test that a user cannot add a tag to a room if they don't have membership to the + room. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + # Create the room with user2 (user1 has no membership in the room) + room_id = self.helper.create_room_as(user2_id, tok=user2_tok) + tag = "test_tag" + + # Make the request + channel = self.make_request( + "PUT", + f"/user/{user1_id}/rooms/{room_id}/tags/{tag}", + content={"order": 0.5}, + access_token=user1_tok, + ) + # Check that the request failed with the correct error + self.assertEqual(channel.code, HTTPStatus.FORBIDDEN, channel.result) + + def test_put_tag_fails_if_room_does_not_exist(self) -> None: + """ + Test that a user cannot add a tag to a room if the room doesn't exist (therefore + no membership in the room.) + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + room_id = "!nonexistent:test" + tag = "test_tag" + + # Make the request + channel = self.make_request( + "PUT", + f"/user/{user1_id}/rooms/{room_id}/tags/{tag}", + content={"order": 0.5}, + access_token=user1_tok, + ) + # Check that the request failed with the correct error + self.assertEqual(channel.code, HTTPStatus.FORBIDDEN, channel.result)