From 060ba6392271ce31f4ce77d17121a8c07187612c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 23 May 2024 13:07:42 -0700 Subject: [PATCH 1/7] prevent suspended users from sending messages, changing profile data and redacting messages other than their own --- synapse/handlers/message.py | 11 +++++++++++ synapse/rest/client/profile.py | 26 ++++++++++++++++++++++++++ synapse/rest/client/room.py | 14 ++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index ccaa5508ff0..06c8452bb59 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -651,6 +651,17 @@ async def create_event( """ await self.auth_blocking.check_auth_blocking(requester=requester) + if event_dict["type"] == EventTypes.Message: + requester_suspended = await self.store.get_user_suspended_status( + requester.user.to_string() + ) + if requester_suspended: + raise SynapseError( + 403, + "Sending messages while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + if event_dict["type"] == EventTypes.Create and event_dict["state_key"] == "": room_version_id = event_dict["content"]["room_version"] maybe_room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version_id) diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 0323f6afa15..c1a80c5c3d5 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -108,6 +108,19 @@ async def on_PUT( propagate = _read_propagate(self.hs, request) + requester_suspended = ( + await self.hs.get_datastores().main.get_user_suspended_status( + requester.user.to_string() + ) + ) + + if requester_suspended: + raise SynapseError( + 403, + "Updating displayname while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + await self.profile_handler.set_displayname( user, requester, new_name, is_admin, propagate=propagate ) @@ -167,6 +180,19 @@ async def on_PUT( propagate = _read_propagate(self.hs, request) + requester_suspended = ( + await self.hs.get_datastores().main.get_user_suspended_status( + requester.user.to_string() + ) + ) + + if requester_suspended: + raise SynapseError( + 403, + "Updating avatar URL while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + await self.profile_handler.set_avatar_url( user, requester, new_avatar_url, is_admin, propagate=propagate ) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index fb4d44211e3..65712df9cab 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -1109,6 +1109,20 @@ async def _do( ) -> Tuple[int, JsonDict]: content = parse_json_object_from_request(request) + requester_suspended = await self._store.get_user_suspended_status( + requester.user.to_string() + ) + + if requester_suspended: + event = await self._store.get_event(event_id, allow_none=True) + if event: + if event.sender != requester.user.to_string(): + raise SynapseError( + 403, + "Only events created by the requester may be redacted while account is suspended.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + # Ensure the redacts property in the content matches the one provided in # the URL. room_version = await self._store.get_room_version(room_id) From 245e28f816cc4270e71c0dd58555a21323b97cf2 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 31 May 2024 11:26:40 -0700 Subject: [PATCH 2/7] add admin API to suspend users + add experimental config option --- synapse/config/experimental.py | 4 ++++ synapse/rest/admin/__init__.py | 3 +++ synapse/rest/admin/users.py | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 749452ce934..9886e3852e3 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -436,3 +436,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.msc4115_membership_on_events = experimental.get( "msc4115_membership_on_events", False ) + + self.msc3823_account_suspension = experimental.get( + "msc3823_account_suspension", False + ) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 6da1d791686..cdaee174518 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -101,6 +101,7 @@ ResetPasswordRestServlet, SearchUsersRestServlet, ShadowBanRestServlet, + SuspendAccountRestServlet, UserAdminServlet, UserByExternalId, UserByThreePid, @@ -327,6 +328,8 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: BackgroundUpdateRestServlet(hs).register(http_server) BackgroundUpdateStartJobRestServlet(hs).register(http_server) ExperimentalFeaturesRestServlet(hs).register(http_server) + if hs.config.experimental.msc3823_account_suspension: + SuspendAccountRestServlet(hs).register(http_server) def register_servlets_for_client_rest_resource( diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 5bf12c49795..801ad2b646e 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -732,6 +732,33 @@ async def on_POST( return HTTPStatus.OK, {"id_server_unbind_result": id_server_unbind_result} +class SuspendAccountRestServlet(RestServlet): + PATTERNS = admin_patterns("/suspend/(?P[^/]*)$") + + def __init__(self, hs: "HomeServer"): + self.auth = hs.get_auth() + self.is_mine = hs.is_mine + self.store = hs.get_datastores().main + + async def on_PUT( + self, request: SynapseRequest, target_user_id: str + ) -> Tuple[int, JsonDict]: + requester = await self.auth.get_user_by_req(request) + await assert_user_is_admin(self.auth, requester) + + if not self.is_mine(UserID.from_string(target_user_id)): + raise SynapseError(HTTPStatus.BAD_REQUEST, "Can only suspend local users") + + if not await self.store.get_user_by_id(target_user_id): + raise NotFoundError("User not found") + + body = parse_json_object_from_request(request, allow_empty_body=True) + suspend = body.get("suspend", False) + await self.store.set_user_suspended_status(target_user_id, suspend) + + return HTTPStatus.OK, {f"user_{target_user_id}_suspended": suspend} + + class AccountValidityRenewServlet(RestServlet): PATTERNS = admin_patterns("/account_validity/validity$") From 07bf06935ff8737166eeecd29d7bdddc759b3884 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 31 May 2024 11:26:44 -0700 Subject: [PATCH 3/7] tests --- tests/rest/admin/test_user.py | 84 +++++++++++++++++++++++++ tests/rest/client/test_rooms.py | 105 ++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index c5da1e96869..16bb4349f54 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -37,6 +37,7 @@ from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError from synapse.api.room_versions import RoomVersions from synapse.media.filepath import MediaFilePaths +from synapse.rest import admin from synapse.rest.client import ( devices, login, @@ -5005,3 +5006,86 @@ def test_success(self) -> None: ) assert timestamp is not None self.assertGreater(timestamp, self.clock.time_msec()) + + +class UserSuspensionTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + admin.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.admin = self.register_user("thomas", "hackme", True) + self.admin_tok = self.login("thomas", "hackme") + + self.bad_user = self.register_user("teresa", "hackme") + self.bad_user_tok = self.login("teresa", "hackme") + + self.store = hs.get_datastores().main + + @override_config({"experimental_features": {"msc3823_account_suspension": True}}) + def test_suspend_user(self) -> None: + # test that suspending user works + channel = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.bad_user}", + {"suspend": True}, + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body, {f"user_{self.bad_user}_suspended": True}) + + res = self.get_success(self.store.get_user_suspended_status(self.bad_user)) + self.assertEqual(True, res) + + # test that un-suspending user works + channel2 = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.bad_user}", + {"suspend": False}, + access_token=self.admin_tok, + ) + self.assertEqual(channel2.code, 200) + self.assertEqual(channel2.json_body, {f"user_{self.bad_user}_suspended": False}) + + res2 = self.get_success(self.store.get_user_suspended_status(self.bad_user)) + self.assertEqual(False, res2) + + # test that trying to un-suspend user who isn't suspended doesn't cause problems + channel3 = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.bad_user}", + {"suspend": False}, + access_token=self.admin_tok, + ) + self.assertEqual(channel3.code, 200) + self.assertEqual(channel3.json_body, {f"user_{self.bad_user}_suspended": False}) + + res3 = self.get_success(self.store.get_user_suspended_status(self.bad_user)) + self.assertEqual(False, res3) + + # test that trying to suspend user who is already suspended doesn't cause problems + channel4 = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.bad_user}", + {"suspend": True}, + access_token=self.admin_tok, + ) + self.assertEqual(channel4.code, 200) + self.assertEqual(channel4.json_body, {f"user_{self.bad_user}_suspended": True}) + + res4 = self.get_success(self.store.get_user_suspended_status(self.bad_user)) + self.assertEqual(True, res4) + + channel5 = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.bad_user}", + {"suspend": True}, + access_token=self.admin_tok, + ) + self.assertEqual(channel5.code, 200) + self.assertEqual(channel5.json_body, {f"user_{self.bad_user}_suspended": True}) + + res5 = self.get_success(self.store.get_user_suspended_status(self.bad_user)) + self.assertEqual(True, res5) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index d398cead1c0..c559dfda834 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -3819,3 +3819,108 @@ def test_no_outliers(self) -> None: # Make sure the outlier event is not returned self.assertNotEqual(channel.json_body["event_id"], outlier_event.event_id) + + +class UserSuspensionTests(unittest.HomeserverTestCase): + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + profile.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.user1 = self.register_user("thomas", "hackme") + self.tok1 = self.login("thomas", "hackme") + + self.user2 = self.register_user("teresa", "hackme") + self.tok2 = self.login("teresa", "hackme") + + self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) + self.store = hs.get_datastores().main + + def test_suspended_user_cannot_send_message_to_room(self) -> None: + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user1, True)) + + channel = self.make_request( + "PUT", + f"/rooms/{self.room1}/send/m.room.message/1", + access_token=self.tok1, + content={"body": "hello", "msgtype": "m.text"}, + ) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + def test_suspended_user_cannot_change_profile_data(self) -> None: + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user1, True)) + + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/profile/{self.user1}/avatar_url", + access_token=self.tok1, + content={"avatar_url": "mxc://matrix.org/wefh34uihSDRGhw34"}, + shorthand=False, + ) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + channel2 = self.make_request( + "PUT", + f"/_matrix/client/v3/profile/{self.user1}/displayname", + access_token=self.tok1, + content={"displayname": "something offensive"}, + shorthand=False, + ) + self.assertEqual( + channel2.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + def test_suspended_user_cannot_redact_messages_other_than_their_own(self) -> None: + # first user sends message + self.make_request("POST", f"/rooms/{self.room1}/join", access_token=self.tok2) + res = self.helper.send_event( + self.room1, + "m.room.message", + {"body": "hello", "msgtype": "m.text"}, + tok=self.tok2, + ) + event_id = res["event_id"] + + # second user sends message + self.make_request("POST", f"/rooms/{self.room1}/join", access_token=self.tok1) + res2 = self.helper.send_event( + self.room1, + "m.room.message", + {"body": "bad_message", "msgtype": "m.text"}, + tok=self.tok1, + ) + event_id2 = res2["event_id"] + + # set the second user as suspended + self.get_success(self.store.set_user_suspended_status(self.user1, True)) + + # second user can't redact first user's message + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/rooms/{self.room1}/redact/{event_id}/1", + access_token=self.tok1, + content={"reason": "bogus"}, + shorthand=False, + ) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + # but can redact their own + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/rooms/{self.room1}/redact/{event_id2}/1", + access_token=self.tok1, + content={"reason": "bogus"}, + shorthand=False, + ) + self.assertEqual(channel.code, 200) From fe4edc1cb2c20ea8fa4ab2fdcaffcd9169d18df3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 31 May 2024 11:43:35 -0700 Subject: [PATCH 4/7] newsfragment --- changelog.d/17255.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17255.feature diff --git a/changelog.d/17255.feature b/changelog.d/17255.feature new file mode 100644 index 00000000000..4093de1146a --- /dev/null +++ b/changelog.d/17255.feature @@ -0,0 +1 @@ +Add support for [MSC823](https://github.com/matrix-org/matrix-spec-proposals/pull/3823) - Account suspension. \ No newline at end of file From 7975a81e12a621bc58ff24f537563a0b02db08b8 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 31 May 2024 12:04:56 -0700 Subject: [PATCH 5/7] lint --- synapse/config/experimental.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 01c06748dd8..21cba919d98 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -443,7 +443,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.msc3823_account_suspension = experimental.get( "msc3823_account_suspension", False ) - + self.msc3916_authenticated_media_enabled = experimental.get( "msc3916_authenticated_media_enabled", False ) From 4b4180bba5cb069b1c87d2bc3dd95dcca3510e27 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 6 Jun 2024 11:50:25 -0700 Subject: [PATCH 6/7] add pydantic model + comment --- synapse/rest/admin/users.py | 16 ++++++++++++++-- synapse/rest/client/room.py | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 801ad2b646e..1fed638cd27 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -27,11 +27,13 @@ import attr +from synapse._pydantic_compat import HAS_PYDANTIC_V2 from synapse.api.constants import Direction, UserTypes from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.servlet import ( RestServlet, assert_params_in_dict, + parse_and_validate_json_object_from_request, parse_boolean, parse_enum, parse_integer, @@ -46,6 +48,7 @@ assert_user_is_admin, ) from synapse.rest.client._base import client_patterns +from synapse.rest.models import RequestBodyModel from synapse.storage.databases.main.registration import ExternalIDReuseException from synapse.storage.databases.main.stats import UserSortOrder from synapse.types import JsonDict, JsonMapping, UserID @@ -53,6 +56,12 @@ if TYPE_CHECKING: from synapse.server import HomeServer +if TYPE_CHECKING or HAS_PYDANTIC_V2: + from pydantic.v1 import StrictBool +else: + from pydantic import StrictBool + + logger = logging.getLogger(__name__) @@ -740,6 +749,9 @@ def __init__(self, hs: "HomeServer"): self.is_mine = hs.is_mine self.store = hs.get_datastores().main + class PutBody(RequestBodyModel): + suspend: StrictBool + async def on_PUT( self, request: SynapseRequest, target_user_id: str ) -> Tuple[int, JsonDict]: @@ -752,8 +764,8 @@ async def on_PUT( if not await self.store.get_user_by_id(target_user_id): raise NotFoundError("User not found") - body = parse_json_object_from_request(request, allow_empty_body=True) - suspend = body.get("suspend", False) + body = parse_and_validate_json_object_from_request(request, self.PutBody) + suspend = body.suspend await self.store.set_user_suspended_status(target_user_id, suspend) return HTTPStatus.OK, {f"user_{target_user_id}_suspended": suspend} diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 65712df9cab..ff5759bd4af 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -1119,7 +1119,7 @@ async def _do( if event.sender != requester.user.to_string(): raise SynapseError( 403, - "Only events created by the requester may be redacted while account is suspended.", + "You can only redact your own events while account is suspended.", Codes.USER_ACCOUNT_SUSPENDED, ) From f93d2618bfd178f65dced8e57e18dcaec7e3dded Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 21 Jun 2024 14:00:16 -0700 Subject: [PATCH 7/7] fix import changed by merge --- synapse/rest/admin/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 1fed638cd27..c770b7c8280 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -48,10 +48,10 @@ assert_user_is_admin, ) from synapse.rest.client._base import client_patterns -from synapse.rest.models import RequestBodyModel from synapse.storage.databases.main.registration import ExternalIDReuseException from synapse.storage.databases.main.stats import UserSortOrder from synapse.types import JsonDict, JsonMapping, UserID +from synapse.types.rest import RequestBodyModel if TYPE_CHECKING: from synapse.server import HomeServer