From 59710437e4a885252de5e5555fbcf42d223b092c Mon Sep 17 00:00:00 2001 From: Melvyn Laïly Date: Fri, 26 Apr 2024 10:43:52 +0200 Subject: Return the search terms as search highlights for SQLite instead of nothing (#17000) Fixes https://github.com/element-hq/synapse/issues/16999 and https://github.com/element-hq/element-android/pull/8729 by returning the search terms as search highlights. --- synapse/storage/databases/main/search.py | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'synapse/storage/databases') diff --git a/synapse/storage/databases/main/search.py b/synapse/storage/databases/main/search.py index 4a0afb50ac..20fcfd3122 100644 --- a/synapse/storage/databases/main/search.py +++ b/synapse/storage/databases/main/search.py @@ -470,6 +470,8 @@ class SearchStore(SearchBackgroundUpdateStore): count_args = args count_clauses = clauses + sqlite_highlights: List[str] = [] + if isinstance(self.database_engine, PostgresEngine): search_query = search_term sql = """ @@ -486,7 +488,7 @@ class SearchStore(SearchBackgroundUpdateStore): """ count_args = [search_query] + count_args elif isinstance(self.database_engine, Sqlite3Engine): - search_query = _parse_query_for_sqlite(search_term) + search_query, sqlite_highlights = _parse_query_for_sqlite(search_term) sql = """ SELECT rank(matchinfo(event_search)) as rank, room_id, event_id @@ -531,9 +533,11 @@ class SearchStore(SearchBackgroundUpdateStore): event_map = {ev.event_id: ev for ev in events} - highlights = None + highlights: Collection[str] = [] if isinstance(self.database_engine, PostgresEngine): highlights = await self._find_highlights_in_postgres(search_query, events) + else: + highlights = sqlite_highlights count_sql += " GROUP BY room_id" @@ -597,6 +601,8 @@ class SearchStore(SearchBackgroundUpdateStore): count_args = list(args) count_clauses = list(clauses) + sqlite_highlights: List[str] = [] + if pagination_token: try: origin_server_ts_str, stream_str = pagination_token.split(",") @@ -647,7 +653,7 @@ class SearchStore(SearchBackgroundUpdateStore): CROSS JOIN events USING (event_id) WHERE """ - search_query = _parse_query_for_sqlite(search_term) + search_query, sqlite_highlights = _parse_query_for_sqlite(search_term) args = [search_query] + args count_sql = """ @@ -694,9 +700,11 @@ class SearchStore(SearchBackgroundUpdateStore): event_map = {ev.event_id: ev for ev in events} - highlights = None + highlights: Collection[str] = [] if isinstance(self.database_engine, PostgresEngine): highlights = await self._find_highlights_in_postgres(search_query, events) + else: + highlights = sqlite_highlights count_sql += " GROUP BY room_id" @@ -892,19 +900,25 @@ def _tokenize_query(query: str) -> TokenList: return tokens -def _tokens_to_sqlite_match_query(tokens: TokenList) -> str: +def _tokens_to_sqlite_match_query(tokens: TokenList) -> Tuple[str, List[str]]: """ Convert the list of tokens to a string suitable for passing to sqlite's MATCH. Assume sqlite was compiled with enhanced query syntax. + Returns the sqlite-formatted query string and the tokenized search terms + that can be used as highlights. + Ref: https://www.sqlite.org/fts3.html#full_text_index_queries """ match_query = [] + highlights = [] for token in tokens: if isinstance(token, str): match_query.append(token) + highlights.append(token) elif isinstance(token, Phrase): match_query.append('"' + " ".join(token.phrase) + '"') + highlights.append(" ".join(token.phrase)) elif token == SearchToken.Not: # TODO: SQLite treats NOT as a *binary* operator. Hopefully a search # term has already been added before this. @@ -916,11 +930,14 @@ def _tokens_to_sqlite_match_query(tokens: TokenList) -> str: else: raise ValueError(f"unknown token {token}") - return "".join(match_query) + return "".join(match_query), highlights -def _parse_query_for_sqlite(search_term: str) -> str: +def _parse_query_for_sqlite(search_term: str) -> Tuple[str, List[str]]: """Takes a plain unicode string from the user and converts it into a form that can be passed to sqllite's matchinfo(). + + Returns the converted query string and the tokenized search terms + that can be used as highlights. """ return _tokens_to_sqlite_match_query(_tokenize_query(search_term)) -- cgit 1.5.1 From 89fc579329d7c81c040b1c178099860e7de37bed Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 26 Apr 2024 10:52:24 +0100 Subject: Fix filtering of rooms when supplying the `destination` query parameter to `/_synapse/admin/v1/federation/destinations//rooms` (#17077) --- changelog.d/17077.bugfix | 1 + synapse/storage/databases/main/transactions.py | 1 + tests/rest/admin/test_federation.py | 67 ++++++++++++++++++++++++-- 3 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 changelog.d/17077.bugfix (limited to 'synapse/storage/databases') diff --git a/changelog.d/17077.bugfix b/changelog.d/17077.bugfix new file mode 100644 index 0000000000..7d8ea37406 --- /dev/null +++ b/changelog.d/17077.bugfix @@ -0,0 +1 @@ +Fixes a bug introduced in v1.52.0 where the `destination` query parameter for the [Destination Rooms Admin API](https://element-hq.github.io/synapse/v1.105/usage/administration/admin_api/federation.html#destination-rooms) failed to actually filter returned rooms. \ No newline at end of file diff --git a/synapse/storage/databases/main/transactions.py b/synapse/storage/databases/main/transactions.py index 08e0241f68..770802483c 100644 --- a/synapse/storage/databases/main/transactions.py +++ b/synapse/storage/databases/main/transactions.py @@ -660,6 +660,7 @@ class TransactionWorkerStore(CacheInvalidationWorkerStore): limit=limit, retcols=("room_id", "stream_ordering"), order_direction=order, + keyvalues={"destination": destination}, ), ) return rooms, count diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py index c1d88f0176..c2015774a1 100644 --- a/tests/rest/admin/test_federation.py +++ b/tests/rest/admin/test_federation.py @@ -778,20 +778,81 @@ class DestinationMembershipTestCase(unittest.HomeserverTestCase): self.assertEqual(number_rooms, len(channel.json_body["rooms"])) self._check_fields(channel.json_body["rooms"]) - def _create_destination_rooms(self, number_rooms: int) -> None: - """Create a number rooms for destination + def test_room_filtering(self) -> None: + """Tests that rooms are correctly filtered""" + + # Create two rooms on the homeserver. Each has a different remote homeserver + # participating in it. + other_destination = "other.destination.org" + room_ids_self_dest = self._create_destination_rooms(2, destination=self.dest) + room_ids_other_dest = self._create_destination_rooms( + 1, destination=other_destination + ) + + # Ask for the rooms that `self.dest` is participating in. + channel = self.make_request("GET", self.url, access_token=self.admin_user_tok) + self.assertEqual(200, channel.code, msg=channel.json_body) + + # Verify that we received only the rooms that `self.dest` is participating in. + # This assertion method name is a bit misleading. It does check that both lists + # contain the same items, and the same counts. + self.assertCountEqual( + [r["room_id"] for r in channel.json_body["rooms"]], room_ids_self_dest + ) + self.assertEqual(channel.json_body["total"], len(room_ids_self_dest)) + + # Ask for the rooms that `other_destination` is participating in. + channel = self.make_request( + "GET", + self.url.replace(self.dest, other_destination), + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + + # Verify that we received only the rooms that `other_destination` is + # participating in. + self.assertCountEqual( + [r["room_id"] for r in channel.json_body["rooms"]], room_ids_other_dest + ) + self.assertEqual(channel.json_body["total"], len(room_ids_other_dest)) + + def _create_destination_rooms( + self, + number_rooms: int, + destination: Optional[str] = None, + ) -> List[str]: + """ + Create the given number of rooms. The given `destination` homeserver will + be recorded as a participant. Args: number_rooms: Number of rooms to be created + destination: The domain of the homeserver that will be considered + as a participant in the rooms. + + Returns: + The IDs of the rooms that have been created. """ + room_ids = [] + + # If no destination was provided, default to `self.dest`. + if destination is None: + destination = self.dest + for _ in range(number_rooms): room_id = self.helper.create_room_as( self.admin_user, tok=self.admin_user_tok ) + room_ids.append(room_id) + self.get_success( - self.store.store_destination_rooms_entries((self.dest,), room_id, 1234) + self.store.store_destination_rooms_entries( + (destination,), room_id, 1234 + ) ) + return room_ids + def _check_fields(self, content: List[JsonDict]) -> None: """Checks that the expected room attributes are present in content -- cgit 1.5.1 From 37558d5e4cd22ec8f120d2c0fbb8c9842d6dd131 Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 1 May 2024 09:45:17 -0700 Subject: Add support for MSC3823 - Account Suspension (#17051) --- changelog.d/17051.feature | 1 + synapse/_scripts/synapse_port_db.py | 2 +- synapse/handlers/room_member.py | 30 ++++++++++ synapse/storage/databases/main/registration.py | 55 ++++++++++++++++- synapse/storage/schema/__init__.py | 5 +- .../schema/main/delta/85/01_add_suspended.sql | 14 +++++ synapse/types/__init__.py | 2 + tests/rest/client/test_rooms.py | 69 +++++++++++++++++++++- tests/storage/test_registration.py | 2 +- 9 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 changelog.d/17051.feature create mode 100644 synapse/storage/schema/main/delta/85/01_add_suspended.sql (limited to 'synapse/storage/databases') diff --git a/changelog.d/17051.feature b/changelog.d/17051.feature new file mode 100644 index 0000000000..1c41f49f7d --- /dev/null +++ b/changelog.d/17051.feature @@ -0,0 +1 @@ +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/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 15507372a4..1e56f46911 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -127,7 +127,7 @@ BOOLEAN_COLUMNS = { "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/handlers/room_member.py b/synapse/handlers/room_member.py index 601d37341b..655c78e150 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -752,6 +752,36 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): 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: diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 29bf47befc..df7f8a43b7 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -236,7 +236,8 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore): 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, + suspended FROM users WHERE name = ? """, @@ -261,6 +262,7 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore): shadow_banned, approved, locked, + suspended, ) = row return UserInfo( @@ -277,6 +279,7 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore): user_type=user_type, approved=bool(approved), locked=bool(locked), + suspended=bool(suspended), ) return await self.db_pool.runInteraction( @@ -1180,6 +1183,27 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore): # 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 it is not suspended or + if the user ID cannot be found. + """ + + 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], @@ -2213,6 +2237,35 @@ class RegistrationBackgroundUpdateStore(RegistrationWorkerStore): 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": suspended}, + ) + 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/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 039aa91b92..0dc5d24249 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 @@ -136,6 +136,9 @@ Changes in SCHEMA_VERSION = 83 Changes in SCHEMA_VERSION = 84 - No longer assumes that `event_auth_chain_links` holds transitive links, and so read operations must do graph traversal. + +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 0000000000..807aad374f --- /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 a88982a04c..509a2d3a0f 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): diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index b796163dcb..d398cead1c 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -48,7 +48,16 @@ from synapse.appservice import ApplicationService 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 @@ class RoomsCreateTestCase(RoomBase): 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 @@ class RoomsCreateTestCase(RoomBase): 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 @@ class RoomJoinTestCase(RoomBase): 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 @@ class RoomJoinTestCase(RoomBase): 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 = [ diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py index 505465d529..14e3871dc1 100644 --- a/tests/storage/test_registration.py +++ b/tests/storage/test_registration.py @@ -43,7 +43,6 @@ class RegistrationStoreTestCase(HomeserverTestCase): 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 @@ class RegistrationStoreTestCase(HomeserverTestCase): locked=False, is_shadow_banned=False, approved=True, + suspended=False, ), (self.get_success(self.store.get_user_by_id(self.user_id))), ) -- cgit 1.5.1