From 9cdf5e2f0c70709916ecda625923cb363bae125a Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Thu, 17 Oct 2024 08:27:32 +0300 Subject: [PATCH 01/17] create a function in room_member handler to check if a user is a member of a room checked if a user is in a room before tagging it --- synapse/handlers/room_member.py | 6 ++++++ synapse/rest/client/tags.py | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 75c60e3c34d..31eac159867 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1189,6 +1189,12 @@ async def update_membership_locked( outlier=outlier, origin_server_ts=origin_server_ts, ) + async def check_user_membership(self, user_id, room_id) -> None: + rooms_for_user = await self.store.get_rooms_for_user(user_id) + if room_id not in rooms_for_user: + raise AuthError( + 403, f"You are not member of the room {room_id}" + ) async def _should_perform_remote_join( self, diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index 554bcb95dd8..f87675f3213 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -50,6 +50,7 @@ def __init__(self, hs: "HomeServer"): super().__init__() self.auth = hs.get_auth() self.store = hs.get_datastores().main + self.room_member_handler = hs.get_room_member_handler() async def on_GET( self, request: SynapseRequest, user_id: str, room_id: str @@ -85,6 +86,9 @@ async def on_PUT( 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 is member of the room and raise error if not + await self.room_member_handler.check_user_membership(user_id=user_id, room_id=room_id) body = parse_json_object_from_request(request) From 6e1d05fcccdb6887d2eed83ea2c042d521fe5f4d Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Thu, 17 Oct 2024 08:41:31 +0300 Subject: [PATCH 02/17] added changelog --- changelog.d/17839.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17839.bugfix diff --git a/changelog.d/17839.bugfix b/changelog.d/17839.bugfix new file mode 100644 index 00000000000..22882197e63 --- /dev/null +++ b/changelog.d/17839.bugfix @@ -0,0 +1 @@ +fixed checking if user is member of a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file From a1bfccece63d9315f826118392fdc162be905cf4 Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Thu, 17 Oct 2024 08:47:40 +0300 Subject: [PATCH 03/17] fixed changelog typo --- changelog.d/17839.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17839.bugfix b/changelog.d/17839.bugfix index 22882197e63..919dcb4e871 100644 --- a/changelog.d/17839.bugfix +++ b/changelog.d/17839.bugfix @@ -1 +1 @@ -fixed checking if user is member of a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file +Checking if user is member of a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file From bbbe344fa91b4d796d037d341608ce0ae437f898 Mon Sep 17 00:00:00 2001 From: Lama Date: Thu, 24 Oct 2024 08:17:23 +0300 Subject: [PATCH 04/17] Update synapse/rest/client/tags.py Co-authored-by: Eric Eastwood --- synapse/rest/client/tags.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index f87675f3213..307b4e8f8cb 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -87,7 +87,9 @@ async def on_PUT( if user_id != requester.user.to_string(): raise AuthError(403, "Cannot add tags for other users.") - # check if the user is member of the room and raise error if not + # 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_user_membership(user_id=user_id, room_id=room_id) body = parse_json_object_from_request(request) From f13dc0e4589bb8bb31fa5c920af39590bf26ed92 Mon Sep 17 00:00:00 2001 From: Lama Date: Thu, 24 Oct 2024 08:17:57 +0300 Subject: [PATCH 05/17] Update changelog.d/17839.bugfix Co-authored-by: Eric Eastwood --- changelog.d/17839.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17839.bugfix b/changelog.d/17839.bugfix index 919dcb4e871..57667a6df5d 100644 --- a/changelog.d/17839.bugfix +++ b/changelog.d/17839.bugfix @@ -1 +1 @@ -Checking if user is member of a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file +Check if user has membership in a room before tagging it. Contributed by Lama Alosaimi. \ No newline at end of file From cbf476d074c9ebcad80334c72bed5f692f281eef Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Thu, 24 Oct 2024 08:40:21 +0300 Subject: [PATCH 06/17] reuse function get_local_current_membership_for_user_in_room instead of looping on users's rooms fixed lint issues --- synapse/handlers/room_member.py | 13 +++++++------ synapse/rest/client/tags.py | 3 +-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 31eac159867..65b3a596523 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1189,12 +1189,13 @@ async def update_membership_locked( outlier=outlier, origin_server_ts=origin_server_ts, ) - async def check_user_membership(self, user_id, room_id) -> None: - rooms_for_user = await self.store.get_rooms_for_user(user_id) - if room_id not in rooms_for_user: - raise AuthError( - 403, f"You are not member of the room {room_id}" - ) + + async def check_user_membership(self, user_id: str, room_id: str) -> None: + result: Optional[Tuple[Optional[str], Optional[str]]] = await self.store.get_local_current_membership_for_user_in_room(user_id=user_id, room_id=room_id) + + if result is None: + raise AuthError(403, f"You are not a member of the room {room_id}") + async def _should_perform_remote_join( self, diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index 307b4e8f8cb..03e0354526c 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -50,7 +50,6 @@ def __init__(self, hs: "HomeServer"): super().__init__() self.auth = hs.get_auth() self.store = hs.get_datastores().main - self.room_member_handler = hs.get_room_member_handler() async def on_GET( self, request: SynapseRequest, user_id: str, room_id: str @@ -79,6 +78,7 @@ def __init__(self, hs: "HomeServer"): 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 @@ -86,7 +86,6 @@ async def on_PUT( 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. From 56a1e7263e4f4c0063e36fc3bb2e5fcae9186540 Mon Sep 17 00:00:00 2001 From: Lama Date: Fri, 25 Oct 2024 07:03:52 +0300 Subject: [PATCH 07/17] Update synapse/handlers/room_member.py Co-authored-by: Eric Eastwood --- synapse/handlers/room_member.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 65b3a596523..3a3fb5e1c6f 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1191,7 +1191,7 @@ async def update_membership_locked( ) async def check_user_membership(self, user_id: str, room_id: str) -> None: - result: Optional[Tuple[Optional[str], Optional[str]]] = await self.store.get_local_current_membership_for_user_in_room(user_id=user_id, room_id=room_id) + result = await self.store.get_local_current_membership_for_user_in_room(user_id=user_id, room_id=room_id) if result is None: raise AuthError(403, f"You are not a member of the room {room_id}") From c83c70fa6dc5d77fa7df684f06e74fd805593b18 Mon Sep 17 00:00:00 2001 From: Lama Date: Fri, 25 Oct 2024 07:04:08 +0300 Subject: [PATCH 08/17] Update synapse/handlers/room_member.py Co-authored-by: Eric Eastwood --- synapse/handlers/room_member.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 3a3fb5e1c6f..3f907663ccf 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1194,7 +1194,7 @@ async def check_user_membership(self, user_id: str, room_id: str) -> None: result = await self.store.get_local_current_membership_for_user_in_room(user_id=user_id, room_id=room_id) if result is None: - raise AuthError(403, f"You are not a member of the room {room_id}") + raise AuthError(403, "User %s has no membership in room %s" % (user_id, room_id)) async def _should_perform_remote_join( From bcb026e52d8cf7c659cbb4a8daaf6c687e905e9b Mon Sep 17 00:00:00 2001 From: Lama Date: Fri, 25 Oct 2024 20:43:50 +0300 Subject: [PATCH 09/17] Update synapse/handlers/room_member.py Co-authored-by: Eric Eastwood --- synapse/handlers/room_member.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 3f907663ccf..ae7487dec6e 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1190,7 +1190,17 @@ async def update_membership_locked( origin_server_ts=origin_server_ts, ) - async def check_user_membership(self, user_id: str, room_id: str) -> None: + 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: From 58ca73cdbffdc4c7a0f55e597737a3f5bf6969ea Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Fri, 25 Oct 2024 21:10:31 +0300 Subject: [PATCH 10/17] rename check_user_membership to meaningful name --- synapse/handlers/room_member.py | 15 ++++++++++----- synapse/rest/client/tags.py | 4 +++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index ae7487dec6e..96f94228005 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1190,7 +1190,9 @@ async def update_membership_locked( origin_server_ts=origin_server_ts, ) - async def check_for_any_membership_in_room(self, *, user_id: str, room_id: str) -> None: + 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. @@ -1201,11 +1203,14 @@ async def check_for_any_membership_in_room(self, *, user_id: str, room_id: str) 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: - raise AuthError(403, "User %s has no membership in room %s" % (user_id, room_id)) + result = await self.store.get_local_current_membership_for_user_in_room( + user_id=user_id, room_id=room_id + ) + if result is None: + raise AuthError( + 403, "User %s has no membership in room %s" % (user_id, room_id) + ) async def _should_perform_remote_join( self, diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index 03e0354526c..b6648f3499f 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -89,7 +89,9 @@ async def on_PUT( # 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_user_membership(user_id=user_id, room_id=room_id) + 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) From 61b0ace9e24917a4b16d97eeedb8a57977c228f2 Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Sun, 27 Oct 2024 08:56:36 +0300 Subject: [PATCH 11/17] added test cases for tags api fixed the query about user membership in a room --- synapse/handlers/room_member.py | 6 +-- tests/rest/client/test_tags.py | 82 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 tests/rest/client/test_tags.py diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 96f94228005..70cbbc352be 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1207,10 +1207,8 @@ async def check_for_any_membership_in_room( user_id=user_id, room_id=room_id ) - if result is None: - raise AuthError( - 403, "User %s has no membership in room %s" % (user_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, diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py new file mode 100644 index 00000000000..52540d6f554 --- /dev/null +++ b/tests/rest/client/test_tags.py @@ -0,0 +1,82 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright 2014-2016 OpenMarket Ltd +# Copyright (C) 2023 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: +# . +# +# Originally licensed under the Apache License, Version 2.0: +# . +# +# [This file includes modifications made by New Vector Limited] +# +# + +"""Tests REST events for /tags paths.""" + +from twisted.test.proto_helpers import MemoryReactor + +from synapse.rest.client import room, tags +from synapse.server import HomeServer +from synapse.types import UserID +from synapse.util import Clock + +from tests import unittest + +PATH_PREFIX = "/_matrix/client/api/v1" + + +class RoomTaggingTestCase(unittest.HomeserverTestCase): + """Tests /user/$user_id/rooms/$room_id/tags/$tag REST API.""" + + user_id = "@sid:red" + servlets = [room.register_servlets, tags.register_servlets] + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + hs = self.setup_test_homeserver("red") + self.room_member_handler = hs.get_room_member_handler() + return hs + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.room_id = self.helper.create_room_as(self.user_id) + + def test_put_tag_checks_room_membership(self) -> None: + tag = "test_tag" + # Make the request + channel = self.make_request( + "PUT", + f"/user/{self.user_id}/rooms/{self.room_id}/tags/{tag}", + content={"order": 0.5}, + access_token=self.get_success( + self.hs.get_auth_handler().create_access_token_for_user_id( + self.user_id, device_id=None, valid_until_ms=None + ) + ), + ) + # Check that the request was successful + self.assertEqual(channel.code, 200, channel.result) + + def test_put_tag_fails_if_not_in_room(self) -> None: + room_id = "!nonexistent:test" + tag = "test_tag" + + # Make the request + channel = self.make_request( + "PUT", + f"/user/{self.user_id}/rooms/{room_id}/tags/{tag}", + content={"order": 0.5}, + access_token=self.get_success( + self.hs.get_auth_handler().create_access_token_for_user_id( + self.user_id, device_id=None, valid_until_ms=None + ) + ), + ) + # Check that the request failed with the correct error + self.assertEqual(channel.code, 403, channel.result) \ No newline at end of file From b9190e986bda754ce69847b23b2d647724d5478f Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Sun, 27 Oct 2024 10:15:32 +0300 Subject: [PATCH 12/17] removed unused instances --- tests/rest/client/test_tags.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index 52540d6f554..112cded7ad7 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -41,7 +41,6 @@ class RoomTaggingTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: hs = self.setup_test_homeserver("red") - self.room_member_handler = hs.get_room_member_handler() return hs def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: From b945409ee4dc8a5a8ce6a9ccc9c14d65bc0a05b6 Mon Sep 17 00:00:00 2001 From: Lama Date: Tue, 29 Oct 2024 06:31:15 +0300 Subject: [PATCH 13/17] Update tests/rest/client/test_tags.py Co-authored-by: Eric Eastwood --- tests/rest/client/test_tags.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index 112cded7ad7..a1419a1bc86 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -1,8 +1,7 @@ # # This file is licensed under the Affero General Public License (AGPL) version 3. # -# Copyright 2014-2016 OpenMarket Ltd -# Copyright (C) 2023 New Vector, Ltd +# 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 @@ -12,12 +11,6 @@ # See the GNU Affero General Public License for more details: # . # -# Originally licensed under the Apache License, Version 2.0: -# . -# -# [This file includes modifications made by New Vector Limited] -# -# """Tests REST events for /tags paths.""" From 4b01b38c100d543142543a8abff6155b0c39b94e Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Tue, 29 Oct 2024 07:34:03 +0300 Subject: [PATCH 14/17] refactoring test_tags.py --- tests/rest/client/test_tags.py | 70 +++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index a1419a1bc86..1da8b94b7d9 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -14,61 +14,71 @@ """Tests REST events for /tags paths.""" -from twisted.test.proto_helpers import MemoryReactor +from http import HTTPStatus -from synapse.rest.client import room, tags -from synapse.server import HomeServer -from synapse.types import UserID -from synapse.util import Clock +import synapse.rest.admin +from synapse.rest.client import login, room, tags from tests import unittest -PATH_PREFIX = "/_matrix/client/api/v1" - class RoomTaggingTestCase(unittest.HomeserverTestCase): """Tests /user/$user_id/rooms/$room_id/tags/$tag REST API.""" - user_id = "@sid:red" - servlets = [room.register_servlets, tags.register_servlets] + servlets = [ + room.register_servlets, + tags.register_servlets, + login.register_servlets, + synapse.rest.admin.register_servlets_for_client_rest_resource, + ] - def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: - hs = self.setup_test_homeserver("red") - return hs + 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. - def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: - self.room_id = self.helper.create_room_as(self.user_id) + Args: + None - def test_put_tag_checks_room_membership(self) -> None: + Returns: + None + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login( + user1_id, "pass" + ) # login(username: str, password: str) -> str + 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/{self.user_id}/rooms/{self.room_id}/tags/{tag}", + f"/user/{user1_id}/rooms/{room_id}/tags/{tag}", content={"order": 0.5}, - access_token=self.get_success( - self.hs.get_auth_handler().create_access_token_for_user_id( - self.user_id, device_id=None, valid_until_ms=None - ) - ), + access_token=user1_tok, ) # Check that the request was successful - self.assertEqual(channel.code, 200, channel.result) + 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. + + Args: + None + + Returns: + None + """ + 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/{self.user_id}/rooms/{room_id}/tags/{tag}", + f"/user/{user1_id}/rooms/{room_id}/tags/{tag}", content={"order": 0.5}, - access_token=self.get_success( - self.hs.get_auth_handler().create_access_token_for_user_id( - self.user_id, device_id=None, valid_until_ms=None - ) - ), + access_token=user1_tok, ) # Check that the request failed with the correct error - self.assertEqual(channel.code, 403, channel.result) \ No newline at end of file + self.assertEqual(channel.code, HTTPStatus.FORBIDDEN, channel.result) From fb086690b39d1d8022ee50f64dbdf2af84aec647 Mon Sep 17 00:00:00 2001 From: Lama Date: Tue, 29 Oct 2024 22:45:17 +0300 Subject: [PATCH 15/17] Update tests/rest/client/test_tags.py Co-authored-by: Eric Eastwood --- tests/rest/client/test_tags.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index 1da8b94b7d9..5b7ed4cb5b0 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -45,9 +45,10 @@ def test_put_tag_checks_room_membership(self) -> None: user1_id = self.register_user("user1", "pass") user1_tok = self.login( user1_id, "pass" - ) # login(username: str, password: str) -> str + ) room_id = self.helper.create_room_as(user1_id, tok=user1_tok) tag = "test_tag" + # Make the request channel = self.make_request( "PUT", From 38ee114dc519ab1440ff33348a8716192d29161d Mon Sep 17 00:00:00 2001 From: Lama Alosaimi Date: Wed, 30 Oct 2024 07:34:35 +0300 Subject: [PATCH 16/17] removed unnecessary docstring --- tests/rest/client/test_tags.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index 5b7ed4cb5b0..00856aa8170 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -35,17 +35,9 @@ class RoomTaggingTestCase(unittest.HomeserverTestCase): 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. - - Args: - None - - Returns: - None """ user1_id = self.register_user("user1", "pass") - user1_tok = self.login( - user1_id, "pass" - ) + user1_tok = self.login(user1_id, "pass") room_id = self.helper.create_room_as(user1_id, tok=user1_tok) tag = "test_tag" @@ -62,12 +54,6 @@ def test_put_tag_checks_room_membership(self) -> None: 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. - - Args: - None - - Returns: - None """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") From bff600f0f92de1e6e34d66690dcd8fd0441094ec Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Oct 2024 11:26:36 -0500 Subject: [PATCH 17/17] Add test for room that exists but no membership --- tests/rest/client/test_tags.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_tags.py b/tests/rest/client/test_tags.py index 00856aa8170..5d596409e18 100644 --- a/tests/rest/client/test_tags.py +++ b/tests/rest/client/test_tags.py @@ -53,13 +53,37 @@ def test_put_tag_checks_room_membership(self) -> None: 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. + 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",