Check if user is in room before being able to tag it (#17839)

Fix #17819

changelog.d/17839.bugfix

@@ -0,0 +1 @@
+Check if user has membership in a room before tagging it. Contributed by Lama Alosaimi.

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,

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)
 

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:
+# <https://www.gnu.org/licenses/agpl-3.0.html>.
+#
+
+"""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)