From 27e0c2a167a7bf2a38bc071db2aa457f4ae083e4 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 13:13:48 -0700 Subject: [PATCH 1/7] add a column `suspended` to table `users` --- synapse/_scripts/synapse_port_db.py | 2 +- synapse/storage/schema/__init__.py | 5 ++++- .../schema/main/delta/85/01_add_suspended.sql | 14 ++++++++++++++ synapse/types/__init__.py | 2 ++ 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 synapse/storage/schema/main/delta/85/01_add_suspended.sql diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 15507372a4c..1e56f46911d 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -127,7 +127,7 @@ "redactions": ["have_censored"], "room_stats_state": ["is_federatable"], "rooms": ["is_public", "has_auth_chain_index"], - "users": ["shadow_banned", "approved", "locked"], + "users": ["shadow_banned", "approved", "locked", "suspended"], "un_partial_stated_event_stream": ["rejection_status_changed"], "users_who_share_rooms": ["share_private"], "per_user_experimental_features": ["enabled"], diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index c0b925444fe..54b7ad833a6 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -19,7 +19,7 @@ # # -SCHEMA_VERSION = 84 # remember to update the list below when updating +SCHEMA_VERSION = 85 # remember to update the list below when updating """Represents the expectations made by the codebase about the database schema This should be incremented whenever the codebase changes its requirements on the @@ -132,6 +132,9 @@ Changes in SCHEMA_VERSION = 83 - The event_txn_id is no longer used. + +Changes in SCHEMA_VERSION = 85 + - Add a column `suspended` to the `users` table """ diff --git a/synapse/storage/schema/main/delta/85/01_add_suspended.sql b/synapse/storage/schema/main/delta/85/01_add_suspended.sql new file mode 100644 index 00000000000..807aad374fa --- /dev/null +++ b/synapse/storage/schema/main/delta/85/01_add_suspended.sql @@ -0,0 +1,14 @@ +-- +-- 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: +-- . + +ALTER TABLE users ADD COLUMN suspended BOOLEAN DEFAULT FALSE NOT NULL; \ No newline at end of file diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index a88982a04c2..509a2d3a0f9 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -1156,6 +1156,7 @@ class UserInfo: user_type: User type (None for normal user, 'support' and 'bot' other options). approved: If the user has been "approved" to register on the server. locked: Whether the user's account has been locked + suspended: Whether the user's account is currently suspended """ user_id: UserID @@ -1171,6 +1172,7 @@ class UserInfo: is_shadow_banned: bool approved: bool locked: bool + suspended: bool class UserProfile(TypedDict): From ebff1473474bd20dfa7aefbd3f8a79d7c51c6272 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 13:14:50 -0700 Subject: [PATCH 2/7] add functionality to get/set suspended status --- .../storage/databases/main/registration.py | 54 ++++++++++++++++++- tests/storage/test_registration.py | 2 +- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index d939ade4271..1dc23c392ab 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -236,7 +236,8 @@ def get_user_by_id_txn(txn: LoggingTransaction) -> Optional[UserInfo]: consent_server_notice_sent, appservice_id, creation_ts, user_type, deactivated, COALESCE(shadow_banned, FALSE) AS shadow_banned, COALESCE(approved, TRUE) AS approved, - COALESCE(locked, FALSE) AS locked + COALESCE(locked, FALSE) AS locked, + COALESCE(suspended, FALSE) AS suspended FROM users WHERE name = ? """, @@ -261,6 +262,7 @@ def get_user_by_id_txn(txn: LoggingTransaction) -> Optional[UserInfo]: shadow_banned, approved, locked, + suspended, ) = row return UserInfo( @@ -277,6 +279,7 @@ def get_user_by_id_txn(txn: LoggingTransaction) -> Optional[UserInfo]: user_type=user_type, approved=bool(approved), locked=bool(locked), + suspended=bool(suspended), ) return await self.db_pool.runInteraction( @@ -1180,6 +1183,26 @@ async def get_user_locked_status(self, user_id: str) -> bool: # Convert the potential integer into a boolean. return bool(res) + @cached() + async def get_user_suspended_status(self, user_id: str) -> bool: + """ + Determine whether the user's account is suspended. + Args: + user_id: The user ID of the user in question + Returns: + True if the user's account is suspended, false if not. + """ + + res = await self.db_pool.simple_select_one_onecol( + table="users", + keyvalues={"name": user_id}, + retcol="suspended", + allow_none=True, + desc="get_user_suspended", + ) + + return bool(res) + async def get_threepid_validation_session( self, medium: Optional[str], @@ -2206,6 +2229,35 @@ def set_user_deactivated_status_txn( self._invalidate_cache_and_stream(txn, self.get_user_by_id, (user_id,)) txn.call_after(self.is_guest.invalidate, (user_id,)) + async def set_user_suspended_status(self, user_id: str, suspended: bool) -> None: + """ + Set whether the user's account is suspended in the `users` table. + + Args: + user_id: The user ID of the user in question + suspended: True if the user is suspended, false if not + """ + await self.db_pool.runInteraction( + "set_user_suspended_status", + self.set_user_suspended_status_txn, + user_id, + suspended, + ) + + def set_user_suspended_status_txn( + self, txn: LoggingTransaction, user_id: str, suspended: bool + ) -> None: + self.db_pool.simple_update_one_txn( + txn=txn, + table="users", + keyvalues={"name": user_id}, + updatevalues={"suspended": 1 if suspended else 0}, + ) + self._invalidate_cache_and_stream( + txn, self.get_user_suspended_status, (user_id,) + ) + self._invalidate_cache_and_stream(txn, self.get_user_by_id, (user_id,)) + async def set_user_locked_status(self, user_id: str, locked: bool) -> None: """Set the `locked` property for the provided user to the provided value. diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py index 505465d529e..14e3871dc1c 100644 --- a/tests/storage/test_registration.py +++ b/tests/storage/test_registration.py @@ -43,7 +43,6 @@ def test_register(self) -> None: self.assertEqual( UserInfo( - # TODO(paul): Surely this field should be 'user_id', not 'name' user_id=UserID.from_string(self.user_id), is_admin=False, is_guest=False, @@ -57,6 +56,7 @@ def test_register(self) -> None: locked=False, is_shadow_banned=False, approved=True, + suspended=False, ), (self.get_success(self.store.get_user_by_id(self.user_id))), ) From 92c4bffb2485379e59aa75152dcec530d5682d43 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 13:15:09 -0700 Subject: [PATCH 3/7] prohibit suspended users from invting, joining, or knocking --- synapse/handlers/room_member.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 601d37341b6..655c78e1505 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -752,6 +752,36 @@ async def update_membership_locked( and requester.user.to_string() == self._server_notices_mxid ) + requester_suspended = await self.store.get_user_suspended_status( + requester.user.to_string() + ) + if action == Membership.INVITE and requester_suspended: + raise SynapseError( + 403, + "Sending invites while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + + if target.to_string() != requester.user.to_string(): + target_suspended = await self.store.get_user_suspended_status( + target.to_string() + ) + else: + target_suspended = requester_suspended + + if action == Membership.JOIN and target_suspended: + raise SynapseError( + 403, + "Joining rooms while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + if action == Membership.KNOCK and target_suspended: + raise SynapseError( + 403, + "Knocking on rooms while account is suspended is not allowed.", + Codes.USER_ACCOUNT_SUSPENDED, + ) + if ( not self.allow_per_room_profiles and not is_requester_server_notices_user ) or requester.shadow_banned: From 6e3668540ed0199e4566a0bf5d005b353e8c2800 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 13:15:12 -0700 Subject: [PATCH 4/7] tests --- tests/rest/client/test_rooms.py | 69 +++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 1364615085e..f7cd6d603e3 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -48,7 +48,16 @@ from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.rest import admin -from synapse.rest.client import account, directory, login, profile, register, room, sync +from synapse.rest.client import ( + account, + directory, + knock, + login, + profile, + register, + room, + sync, +) from synapse.server import HomeServer from synapse.types import JsonDict, RoomAlias, UserID, create_requester from synapse.util import Clock @@ -733,7 +742,7 @@ def test_post_room_no_keys(self) -> None: self.assertEqual(HTTPStatus.OK, channel.code, channel.result) self.assertTrue("room_id" in channel.json_body) assert channel.resource_usage is not None - self.assertEqual(32, channel.resource_usage.db_txn_count) + self.assertEqual(33, channel.resource_usage.db_txn_count) def test_post_room_initial_state(self) -> None: # POST with initial_state config key, expect new room id @@ -746,7 +755,7 @@ def test_post_room_initial_state(self) -> None: self.assertEqual(HTTPStatus.OK, channel.code, channel.result) self.assertTrue("room_id" in channel.json_body) assert channel.resource_usage is not None - self.assertEqual(34, channel.resource_usage.db_txn_count) + self.assertEqual(35, channel.resource_usage.db_txn_count) def test_post_room_visibility_key(self) -> None: # POST with visibility config key, expect new room id @@ -1154,6 +1163,7 @@ class RoomJoinTestCase(RoomBase): admin.register_servlets, login.register_servlets, room.register_servlets, + knock.register_servlets, ] def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: @@ -1167,6 +1177,8 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.room2 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) self.room3 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) + self.store = hs.get_datastores().main + def test_spam_checker_may_join_room_deprecated(self) -> None: """Tests that the user_may_join_room spam checker callback is correctly called and blocks room joins when needed. @@ -1317,6 +1329,57 @@ async def user_may_join_room( expect_additional_fields=return_value[1], ) + def test_suspended_user_cannot_join_room(self) -> None: + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user2, True)) + + channel = self.make_request( + "POST", f"/join/{self.room1}", access_token=self.tok2 + ) + self.assertEqual(channel.code, 403) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + channel = self.make_request( + "POST", f"/rooms/{self.room1}/join", access_token=self.tok2 + ) + self.assertEqual(channel.code, 403) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + def test_suspended_user_cannot_knock_on_room(self) -> None: + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user2, True)) + + channel = self.make_request( + "POST", + f"/_matrix/client/v3/knock/{self.room1}", + access_token=self.tok2, + content={}, + shorthand=False, + ) + self.assertEqual(channel.code, 403) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + + def test_suspended_user_cannot_invite_to_room(self) -> None: + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user1, True)) + + # first user invites second user + channel = self.make_request( + "POST", + f"/rooms/{self.room1}/invite", + access_token=self.tok1, + content={"user_id": self.user2}, + ) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED" + ) + class RoomAppserviceTsParamTestCase(unittest.HomeserverTestCase): servlets = [ From 665c851ee4421fe3f0a8bc1ddb2636e75faac3f8 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 13:24:17 -0700 Subject: [PATCH 5/7] newsfragment --- changelog.d/17051.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17051.feature diff --git a/changelog.d/17051.feature b/changelog.d/17051.feature new file mode 100644 index 00000000000..b8e9870965d --- /dev/null +++ b/changelog.d/17051.feature @@ -0,0 +1 @@ +Add preliminary support for MSC3823 - Account Suspension. \ No newline at end of file From 6b13af9f85eb62bbe6accd4b316c4db17bf0bb36 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 4 Apr 2024 14:01:59 -0700 Subject: [PATCH 6/7] fix postgres compatibility --- synapse/storage/databases/main/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 1dc23c392ab..a3acb068c1a 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -2251,7 +2251,7 @@ def set_user_suspended_status_txn( txn=txn, table="users", keyvalues={"name": user_id}, - updatevalues={"suspended": 1 if suspended else 0}, + updatevalues={"suspended": suspended}, ) self._invalidate_cache_and_stream( txn, self.get_user_suspended_status, (user_id,) From 6e7b8afafa6a50be0e9d8a2b9a5e25e5592c918f Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 29 Apr 2024 12:36:22 -0700 Subject: [PATCH 7/7] requested changes --- changelog.d/17051.feature | 2 +- synapse/storage/databases/main/registration.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/changelog.d/17051.feature b/changelog.d/17051.feature index b8e9870965d..1c41f49f7d3 100644 --- a/changelog.d/17051.feature +++ b/changelog.d/17051.feature @@ -1 +1 @@ -Add preliminary support for MSC3823 - Account Suspension. \ No newline at end of file +Add preliminary support for [MSC3823](https://github.com/matrix-org/matrix-spec-proposals/pull/3823) - Account Suspension. \ No newline at end of file diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index afdf6e7536b..df7f8a43b70 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -237,7 +237,7 @@ def get_user_by_id_txn(txn: LoggingTransaction) -> Optional[UserInfo]: deactivated, COALESCE(shadow_banned, FALSE) AS shadow_banned, COALESCE(approved, TRUE) AS approved, COALESCE(locked, FALSE) AS locked, - COALESCE(suspended, FALSE) AS suspended + suspended FROM users WHERE name = ? """, @@ -1190,7 +1190,8 @@ async def get_user_suspended_status(self, user_id: str) -> bool: Args: user_id: The user ID of the user in question Returns: - True if the user's account is suspended, false if not. + True if the user's account is suspended, false if it is not suspended or + if the user ID cannot be found. """ res = await self.db_pool.simple_select_one_onecol(