From 7c1c011942ebeb7ab781ccb3640922290159a98c Mon Sep 17 00:00:00 2001 From: Georg Date: Tue, 20 Feb 2024 17:15:58 +0100 Subject: [PATCH 01/17] Add HAProxy example for single port operation (#16768) --- changelog.d/16768.doc | 1 + docs/reverse_proxy.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 changelog.d/16768.doc diff --git a/changelog.d/16768.doc b/changelog.d/16768.doc new file mode 100644 index 0000000000..4f574c2ac6 --- /dev/null +++ b/changelog.d/16768.doc @@ -0,0 +1 @@ +Add HAProxy example for single port operation to reverse proxy documentation. Contributed by Georg Pfuetzenreuter (@tacerus). diff --git a/docs/reverse_proxy.md b/docs/reverse_proxy.md index de72fbde96..7128af114e 100644 --- a/docs/reverse_proxy.md +++ b/docs/reverse_proxy.md @@ -186,6 +186,25 @@ Example configuration, if using a UNIX socket. The configuration lines regarding backend matrix server matrix unix@/run/synapse/main_public.sock ``` +Example configuration when using a single port for both client and federation traffic. +``` +frontend https + bind *:443,[::]:443 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1 + http-request set-header X-Forwarded-Proto https if { ssl_fc } + http-request set-header X-Forwarded-Proto http if !{ ssl_fc } + http-request set-header X-Forwarded-For %[src] + + acl matrix-host hdr(host) -i matrix.example.com matrix.example.com:443 + acl matrix-sni ssl_fc_sni matrix.example.com + acl matrix-path path_beg /_matrix + acl matrix-path path_beg /_synapse/client + + use_backend matrix if matrix-host matrix-path + use_backend matrix if matrix-sni + +backend matrix + server matrix 127.0.0.1:8008 +``` [Delegation](delegate.md) example: ``` From 0c55c76da807c8ca0c37b7394594d3757ffab8a6 Mon Sep 17 00:00:00 2001 From: kegsay <7190048+kegsay@users.noreply.github.com> Date: Tue, 20 Feb 2024 17:14:50 +0000 Subject: [PATCH 02/17] Better complement docs (#16946) --- changelog.d/16946.doc | 1 + docker/complement/README.md | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 changelog.d/16946.doc diff --git a/changelog.d/16946.doc b/changelog.d/16946.doc new file mode 100644 index 0000000000..8037a65450 --- /dev/null +++ b/changelog.d/16946.doc @@ -0,0 +1 @@ +Improve the documentation around running Complement tests with new configuration parameters. \ No newline at end of file diff --git a/docker/complement/README.md b/docker/complement/README.md index 62682219e8..1ce8412818 100644 --- a/docker/complement/README.md +++ b/docker/complement/README.md @@ -30,3 +30,14 @@ Consult `scripts-dev/complement.sh` in the repository root for a real example. [complement]: https://github.com/matrix-org/complement [complementEnv]: https://github.com/matrix-org/complement/pull/382 + +## How to modify homeserver.yaml for Complement tests + +It's common for MSCs to be gated behind a feature flag like this: +```yaml +experimental_features: + faster_joins: true +``` +To modify this for the Complement image, modify `./conf/workers-shared-extra.yaml.j2`. Despite the name, +this will affect non-worker mode as well. Remember to _rebuild_ the image (so don't use `-e` if using +`complement.sh`). From e0b19a4777559110b46ae5268a82b654acddf8c6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:34:03 +0000 Subject: [PATCH 03/17] Bump dawidd6/action-download-artifact from 3.0.0 to 3.1.1 (#16933) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/docs-pr-netlify.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs-pr-netlify.yaml b/.github/workflows/docs-pr-netlify.yaml index 4c0a7989a9..c42077d3af 100644 --- a/.github/workflows/docs-pr-netlify.yaml +++ b/.github/workflows/docs-pr-netlify.yaml @@ -14,7 +14,7 @@ jobs: # There's a 'download artifact' action, but it hasn't been updated for the workflow_run action # (https://github.com/actions/download-artifact/issues/60) so instead we get this mess: - name: 📥 Download artifact - uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d # v3.0.0 + uses: dawidd6/action-download-artifact@72aaadce3bc708349fc665eee3785cbb1b6e51d0 # v3.1.1 with: workflow: docs-pr.yaml run_id: ${{ github.event.workflow_run.id }} From 91694907da1a9484464fe40ca1401dc3e3272e11 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:35:05 +0000 Subject: [PATCH 04/17] Bump furo from 2023.9.10 to 2024.1.29 (#16939) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 834d6512d7..3fa49250c9 100644 --- a/poetry.lock +++ b/poetry.lock @@ -559,13 +559,13 @@ dev = ["Sphinx", "coverage", "flake8", "lxml", "lxml-stubs", "memory-profiler", [[package]] name = "furo" -version = "2023.9.10" +version = "2024.1.29" description = "A clean customisable Sphinx documentation theme." optional = false python-versions = ">=3.8" files = [ - {file = "furo-2023.9.10-py3-none-any.whl", hash = "sha256:513092538537dc5c596691da06e3c370714ec99bc438680edc1debffb73e5bfc"}, - {file = "furo-2023.9.10.tar.gz", hash = "sha256:5707530a476d2a63b8cad83b4f961f3739a69f4b058bcf38a03a39fa537195b2"}, + {file = "furo-2024.1.29-py3-none-any.whl", hash = "sha256:3548be2cef45a32f8cdc0272d415fcb3e5fa6a0eb4ddfe21df3ecf1fe45a13cf"}, + {file = "furo-2024.1.29.tar.gz", hash = "sha256:4d6b2fe3f10a6e36eb9cc24c1e7beb38d7a23fc7b3c382867503b7fcac8a1e02"}, ] [package.dependencies] @@ -3434,4 +3434,4 @@ user-search = ["pyicu"] [metadata] lock-version = "2.0" python-versions = "^3.8.0" -content-hash = "053bda662e95c273f4eda41d7ece8de0e404783ac66d54cdbedc396e196fb63a" +content-hash = "e4ca55af1dcb6b28b8064b7551008fd16f6cdfa9cb9bf90d18c6b47766b56ae6" diff --git a/pyproject.toml b/pyproject.toml index e1ffb9ffef..7735f08268 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -372,7 +372,7 @@ optional = true sphinx = {version = "^6.1", python = "^3.8"} sphinx-autodoc2 = {version = ">=0.4.2,<0.6.0", python = "^3.8"} myst-parser = {version = "^1.0.0", python = "^3.8"} -furo = ">=2022.12.7,<2024.0.0" +furo = ">=2022.12.7,<2025.0.0" [build-system] From f2c5f1564ef73550178366263f7b9214577dc0a6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:36:35 +0000 Subject: [PATCH 05/17] Bump types-netaddr from 0.10.0.20240106 to 1.2.0.20240219 (#16938) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index 3fa49250c9..d183efd39e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3070,13 +3070,13 @@ referencing = "*" [[package]] name = "types-netaddr" -version = "0.10.0.20240106" +version = "1.2.0.20240219" description = "Typing stubs for netaddr" optional = false python-versions = ">=3.8" files = [ - {file = "types-netaddr-0.10.0.20240106.tar.gz", hash = "sha256:7cc6c16bc76f57faf4a042184f748a05e9642b189caf7fe7e36c07cb87c057b3"}, - {file = "types_netaddr-0.10.0.20240106-py3-none-any.whl", hash = "sha256:0acd8116293b06abe66484cf033c2d597f039326c28e3df83b8abd5709f6c65d"}, + {file = "types-netaddr-1.2.0.20240219.tar.gz", hash = "sha256:984e70ad838218d3032f37f05a7e294f7b007fe274ec9d774265c8c06698395f"}, + {file = "types_netaddr-1.2.0.20240219-py3-none-any.whl", hash = "sha256:b26144e878acb8a1a9008e6997863714db04f8029a0f7f6bfe483c977d21b522"}, ] [[package]] From 5ce949804755465050800d248523b40641d48faf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:37:32 +0000 Subject: [PATCH 06/17] Bump JasonEtco/create-an-issue from 2.9.1 to 2.9.2 (#16934) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/latest_deps.yml | 2 +- .github/workflows/twisted_trunk.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/latest_deps.yml b/.github/workflows/latest_deps.yml index a754515c9d..b9e9a401b9 100644 --- a/.github/workflows/latest_deps.yml +++ b/.github/workflows/latest_deps.yml @@ -226,7 +226,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd # v2.9.1 + - uses: JasonEtco/create-an-issue@1b14a70e4d8dc185e5cc76d3bec9eab20257b2c5 # v2.9.2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/.github/workflows/twisted_trunk.yml b/.github/workflows/twisted_trunk.yml index b7b93b3561..76609c2118 100644 --- a/.github/workflows/twisted_trunk.yml +++ b/.github/workflows/twisted_trunk.yml @@ -207,7 +207,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd # v2.9.1 + - uses: JasonEtco/create-an-issue@1b14a70e4d8dc185e5cc76d3bec9eab20257b2c5 # v2.9.2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: From 3778cea776e2d9ec0685801c1d37031057c53820 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:38:38 +0000 Subject: [PATCH 07/17] Bump pyopenssl from 23.3.0 to 24.0.0 (#16937) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index d183efd39e..9257d2ccfa 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2081,17 +2081,17 @@ tests = ["hypothesis (>=3.27.0)", "pytest (>=3.2.1,!=3.3.0)"] [[package]] name = "pyopenssl" -version = "23.3.0" +version = "24.0.0" description = "Python wrapper module around the OpenSSL library" optional = false python-versions = ">=3.7" files = [ - {file = "pyOpenSSL-23.3.0-py3-none-any.whl", hash = "sha256:6756834481d9ed5470f4a9393455154bc92fe7a64b7bc6ee2c804e78c52099b2"}, - {file = "pyOpenSSL-23.3.0.tar.gz", hash = "sha256:6b2cba5cc46e822750ec3e5a81ee12819850b11303630d575e98108a079c2b12"}, + {file = "pyOpenSSL-24.0.0-py3-none-any.whl", hash = "sha256:ba07553fb6fd6a7a2259adb9b84e12302a9a8a75c44046e8bb5d3e5ee887e3c3"}, + {file = "pyOpenSSL-24.0.0.tar.gz", hash = "sha256:6aa33039a93fffa4563e655b61d11364d01264be8ccb49906101e02a334530bf"}, ] [package.dependencies] -cryptography = ">=41.0.5,<42" +cryptography = ">=41.0.5,<43" [package.extras] docs = ["sphinx (!=5.2.0,!=5.2.0.post0,!=7.2.5)", "sphinx-rtd-theme"] From 4ad70f115b670a8ef964afb3ab9fbb30ecafc3c2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:40:34 +0000 Subject: [PATCH 08/17] Bump anyhow from 1.0.79 to 1.0.80 (#16935) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b1568dc7d8..d41a498fc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,9 +13,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.79" +version = "1.0.80" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "080e9890a082662b09c1ad45f567faeeb47f22b5fb23895fbe1e651e718e25ca" +checksum = "5ad32ce52e4161730f7098c077cd2ed6229b5804ccf99e5366be1ab72a98b4e1" [[package]] name = "arc-swap" From 8de3283ebe520e945ccfb66242f4795ecdd0d1f4 Mon Sep 17 00:00:00 2001 From: Twilight Sparkle <19155609+Twi1ightSparkle@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:36:41 +0000 Subject: [PATCH 09/17] Add docs on upgrading from a very old version (#16951) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/16951.doc | 1 + docs/upgrade.md | 20 ++++++++++++++++++++ docs/usage/administration/admin_faq.md | 5 +++++ 3 files changed, 26 insertions(+) create mode 100644 changelog.d/16951.doc diff --git a/changelog.d/16951.doc b/changelog.d/16951.doc new file mode 100644 index 0000000000..a7fb33979d --- /dev/null +++ b/changelog.d/16951.doc @@ -0,0 +1 @@ +Add docs on upgrading from a very old version. \ No newline at end of file diff --git a/docs/upgrade.md b/docs/upgrade.md index 7f67ef8806..640fed3ae3 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -97,6 +97,26 @@ v1.61.0. +## Upgrading from a very old version + +You need to read all of the upgrade notes for each version between your current +version and the latest so that you can update your dependencies, environment, +config files, etc. if necessary. But you do not need to perform an +upgrade to each individual version that was missed. + +We do not have a list of which versions must be installed. Instead, we recommend +that you upgrade through each incompatible database schema version, which would +give you the ability to roll back the maximum number of versions should anything +go wrong. See [Rolling back to older versions](#rolling-back-to-older-versions) +above. + +Additionally, new versions of Synapse will occasionally run database migrations +and background updates to update the database. Synapse will not start until +database migrations are complete. You should wait until background updates from +each upgrade are complete before moving on to the next upgrade, to avoid +stacking them up. You can monitor the currently running background updates with +[the Admin API](usage/administration/admin_api/background_updates.html#status). + # Upgrading to v1.100.0 ## Minimum supported Rust version diff --git a/docs/usage/administration/admin_faq.md b/docs/usage/administration/admin_faq.md index 5c9ee7d0aa..092dcc1c84 100644 --- a/docs/usage/administration/admin_faq.md +++ b/docs/usage/administration/admin_faq.md @@ -120,6 +120,11 @@ for file in $source_directory/*; do done ``` +How do I upgrade from a very old version of Synapse to the latest? +--- +See [this](../../upgrade.html#upgrading-from-a-very-old-version) section in the +upgrade docs. + Manually resetting passwords --- Users can reset their password through their client. Alternatively, a server admin From 274f289a52a0097784d7179c1d912dbc3cfda3ee Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:12:10 +0000 Subject: [PATCH 10/17] Ignore notification counts from rooms you've left (#16954) Co-authored-by: reivilibre --- changelog.d/16954.bugfix | 1 + .../databases/main/event_push_actions.py | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 changelog.d/16954.bugfix diff --git a/changelog.d/16954.bugfix b/changelog.d/16954.bugfix new file mode 100644 index 0000000000..7e5ad69094 --- /dev/null +++ b/changelog.d/16954.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.100.0 where notifications from rooms you've left would continue to be counted. \ No newline at end of file diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index d7aa8a0ee0..56c549ae66 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -404,7 +404,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, notif_count, e.stream_ordering, e.thread_id, last_receipt_stream_ordering, ev.stream_ordering AS receipt_stream_ordering FROM event_push_summary AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -472,7 +476,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -514,7 +522,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id From 8a05304222db744c48c7640a34d5b35e5ecdc22e Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 5 Mar 2024 12:27:27 +0000 Subject: [PATCH 11/17] Revert "Improve DB performance of calculating badge counts for push. (#16756)" (#16979) --- changelog.d/16979.misc | 1 + synapse/push/push_tools.py | 8 +- .../databases/main/event_push_actions.py | 253 ++++++++---------- 3 files changed, 115 insertions(+), 147 deletions(-) create mode 100644 changelog.d/16979.misc diff --git a/changelog.d/16979.misc b/changelog.d/16979.misc new file mode 100644 index 0000000000..a8c3b33ab2 --- /dev/null +++ b/changelog.d/16979.misc @@ -0,0 +1 @@ +Revert https://github.com/element-hq/synapse/pull/16756, which caused incorrect notification counts on mobile clients. diff --git a/synapse/push/push_tools.py b/synapse/push/push_tools.py index 76c7ab6477..1ef881f702 100644 --- a/synapse/push/push_tools.py +++ b/synapse/push/push_tools.py @@ -29,11 +29,17 @@ from synapse.storage.databases.main import DataStore async def get_badge_count(store: DataStore, user_id: str, group_by_room: bool) -> int: invites = await store.get_invited_rooms_for_local_user(user_id) + joins = await store.get_rooms_for_user(user_id) badge = len(invites) room_to_count = await store.get_unread_counts_by_room_for_user(user_id) - for _room_id, notify_count in room_to_count.items(): + for room_id, notify_count in room_to_count.items(): + # room_to_count may include rooms which the user has left, + # ignore those. + if room_id not in joins: + continue + if notify_count == 0: continue diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index d7aa8a0ee0..3a5666cd9b 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -358,6 +358,10 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas This function is intentionally not cached because it is called to calculate the unread badge for push notifications and thus the result is expected to change. + Note that this function assumes the user is a member of the room. Because + summary rows are not removed when a user leaves a room, the caller must + filter out those results from the result. + Returns: A map of room ID to notification counts for the given user. """ @@ -370,170 +374,127 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas def _get_unread_counts_by_room_for_user_txn( self, txn: LoggingTransaction, user_id: str ) -> Dict[str, int]: - # To get the badge count of all rooms we need to make three queries: - # 1. Fetch all counts from `event_push_summary`, discarding any stale - # rooms. - # 2. Fetch all notifications from `event_push_actions` that haven't - # been rotated yet. - # 3. Fetch all notifications from `event_push_actions` for the stale - # rooms. - # - # The "stale room" scenario generally happens when there is a new read - # receipt that hasn't yet been processed to update the - # `event_push_summary` table. When that happens we ignore the - # `event_push_summary` table for that room and calculate the count - # manually from `event_push_actions`. - - # We need to only take into account read receipts of these types. - receipt_types_clause, receipt_types_args = make_in_list_sql_clause( + receipt_types_clause, args = make_in_list_sql_clause( self.database_engine, "receipt_type", (ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE), ) + args.extend([user_id, user_id]) - # Step 1, fetch all counts from `event_push_summary` for the user. This - # is slightly convoluted as we also need to pull out the stream ordering - # of the most recent receipt of the user in the room (either a thread - # aware receipt or thread unaware receipt) in order to determine - # whether the row in `event_push_summary` is stale. Hence the outer - # GROUP BY and odd join condition against `receipts_linearized`. - sql = f""" - SELECT room_id, notif_count, stream_ordering, thread_id, last_receipt_stream_ordering, - MAX(receipt_stream_ordering) - FROM ( - SELECT e.room_id, notif_count, e.stream_ordering, e.thread_id, last_receipt_stream_ordering, - ev.stream_ordering AS receipt_stream_ordering - FROM event_push_summary AS e - INNER JOIN local_current_membership USING (user_id, room_id) - LEFT JOIN receipts_linearized AS r ON ( - e.user_id = r.user_id - AND e.room_id = r.room_id - AND (e.thread_id = r.thread_id OR r.thread_id IS NULL) - AND {receipt_types_clause} - ) - LEFT JOIN events AS ev ON (r.event_id = ev.event_id) - WHERE e.user_id = ? and notif_count > 0 - ) AS es - GROUP BY room_id, notif_count, stream_ordering, thread_id, last_receipt_stream_ordering + receipts_cte = f""" + WITH all_receipts AS ( + SELECT room_id, thread_id, MAX(event_stream_ordering) AS max_receipt_stream_ordering + FROM receipts_linearized + LEFT JOIN events USING (room_id, event_id) + WHERE + {receipt_types_clause} + AND user_id = ? + GROUP BY room_id, thread_id + ) """ - txn.execute( - sql, - receipt_types_args - + [ - user_id, - ], - ) + receipts_joins = """ + LEFT JOIN ( + SELECT room_id, thread_id, + max_receipt_stream_ordering AS threaded_receipt_stream_ordering + FROM all_receipts + WHERE thread_id IS NOT NULL + ) AS threaded_receipts USING (room_id, thread_id) + LEFT JOIN ( + SELECT room_id, thread_id, + max_receipt_stream_ordering AS unthreaded_receipt_stream_ordering + FROM all_receipts + WHERE thread_id IS NULL + ) AS unthreaded_receipts USING (room_id) + """ + # First get summary counts by room / thread for the user. We use the max receipt + # stream ordering of both threaded & unthreaded receipts to compare against the + # summary table. + # + # PostgreSQL and SQLite differ in comparing scalar numerics. + if isinstance(self.database_engine, PostgresEngine): + # GREATEST ignores NULLs. + max_clause = """GREATEST( + threaded_receipt_stream_ordering, + unthreaded_receipt_stream_ordering + )""" + else: + # MAX returns NULL if any are NULL, so COALESCE to 0 first. + max_clause = """MAX( + COALESCE(threaded_receipt_stream_ordering, 0), + COALESCE(unthreaded_receipt_stream_ordering, 0) + )""" + + sql = f""" + {receipts_cte} + SELECT eps.room_id, eps.thread_id, notif_count + FROM event_push_summary AS eps + {receipts_joins} + WHERE user_id = ? + AND notif_count != 0 + AND ( + (last_receipt_stream_ordering IS NULL AND stream_ordering > {max_clause}) + OR last_receipt_stream_ordering = {max_clause} + ) + """ + txn.execute(sql, args) + + seen_thread_ids = set() room_to_count: Dict[str, int] = defaultdict(int) - stale_room_ids = set() - for row in txn: - room_id = row[0] - notif_count = row[1] - stream_ordering = row[2] - _thread_id = row[3] - last_receipt_stream_ordering = row[4] - receipt_stream_ordering = row[5] - if last_receipt_stream_ordering is None: - if receipt_stream_ordering is None: - room_to_count[room_id] += notif_count - elif stream_ordering > receipt_stream_ordering: - room_to_count[room_id] += notif_count - else: - # The latest read receipt from the user is after all the rows for - # this room in `event_push_summary`. We ignore them, and - # calculate the count from `event_push_actions` in step 3. - pass - elif last_receipt_stream_ordering == receipt_stream_ordering: - room_to_count[room_id] += notif_count - else: - # The row is stale if `last_receipt_stream_ordering` is set and - # *doesn't* match the latest receipt from the user. - stale_room_ids.add(room_id) + for room_id, thread_id, notif_count in txn: + room_to_count[room_id] += notif_count + seen_thread_ids.add(thread_id) - # Discard any stale rooms from `room_to_count`, as we will recalculate - # them in step 3. - for room_id in stale_room_ids: - room_to_count.pop(room_id, None) + # Now get any event push actions that haven't been rotated using the same OR + # join and filter by receipt and event push summary rotated up to stream ordering. + sql = f""" + {receipts_cte} + SELECT epa.room_id, epa.thread_id, COUNT(CASE WHEN epa.notif = 1 THEN 1 END) AS notif_count + FROM event_push_actions AS epa + {receipts_joins} + WHERE user_id = ? + AND epa.notif = 1 + AND stream_ordering > (SELECT stream_ordering FROM event_push_summary_stream_ordering) + AND (threaded_receipt_stream_ordering IS NULL OR stream_ordering > threaded_receipt_stream_ordering) + AND (unthreaded_receipt_stream_ordering IS NULL OR stream_ordering > unthreaded_receipt_stream_ordering) + GROUP BY epa.room_id, epa.thread_id + """ + txn.execute(sql, args) - # Step 2, basically the same query, except against `event_push_actions` - # and only fetching rows inserted since the last rotation. - rotated_upto_stream_ordering = self.db_pool.simple_select_one_onecol_txn( - txn, - table="event_push_summary_stream_ordering", - keyvalues={}, - retcol="stream_ordering", + for room_id, thread_id, notif_count in txn: + # Note: only count push actions we have valid summaries for with up to date receipt. + if thread_id not in seen_thread_ids: + continue + room_to_count[room_id] += notif_count + + thread_id_clause, thread_ids_args = make_in_list_sql_clause( + self.database_engine, "epa.thread_id", seen_thread_ids ) + # Finally re-check event_push_actions for any rooms not in the summary, ignoring + # the rotated up-to position. This handles the case where a read receipt has arrived + # but not been rotated meaning the summary table is out of date, so we go back to + # the push actions table. sql = f""" - SELECT room_id, thread_id - FROM ( - SELECT e.room_id, e.stream_ordering, e.thread_id, - ev.stream_ordering AS receipt_stream_ordering - FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) - LEFT JOIN receipts_linearized AS r ON ( - e.user_id = r.user_id - AND e.room_id = r.room_id - AND (e.thread_id = r.thread_id OR r.thread_id IS NULL) - AND {receipt_types_clause} - ) - LEFT JOIN events AS ev ON (r.event_id = ev.event_id) - WHERE e.user_id = ? and notif > 0 - AND e.stream_ordering > ? - ) AS es - GROUP BY room_id, stream_ordering, thread_id - HAVING stream_ordering > COALESCE(MAX(receipt_stream_ordering), 0) + {receipts_cte} + SELECT epa.room_id, COUNT(CASE WHEN epa.notif = 1 THEN 1 END) AS notif_count + FROM event_push_actions AS epa + {receipts_joins} + WHERE user_id = ? + AND NOT {thread_id_clause} + AND epa.notif = 1 + AND (threaded_receipt_stream_ordering IS NULL OR stream_ordering > threaded_receipt_stream_ordering) + AND (unthreaded_receipt_stream_ordering IS NULL OR stream_ordering > unthreaded_receipt_stream_ordering) + GROUP BY epa.room_id """ - txn.execute( - sql, - receipt_types_args + [user_id, rotated_upto_stream_ordering], - ) - for room_id, _thread_id in txn: - # Again, we ignore any stale rooms. - if room_id not in stale_room_ids: - # For event push actions it is one notification per row. - room_to_count[room_id] += 1 + args.extend(thread_ids_args) + txn.execute(sql, args) - # Step 3, if we have stale rooms then we need to recalculate the counts - # from `event_push_actions`. Again, this is basically the same query as - # above except without a lower bound on stream ordering and only against - # a specific set of rooms. - if stale_room_ids: - room_id_clause, room_id_args = make_in_list_sql_clause( - self.database_engine, - "e.room_id", - stale_room_ids, - ) - - sql = f""" - SELECT room_id, thread_id - FROM ( - SELECT e.room_id, e.stream_ordering, e.thread_id, - ev.stream_ordering AS receipt_stream_ordering - FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) - LEFT JOIN receipts_linearized AS r ON ( - e.user_id = r.user_id - AND e.room_id = r.room_id - AND (e.thread_id = r.thread_id OR r.thread_id IS NULL) - AND {receipt_types_clause} - ) - LEFT JOIN events AS ev ON (r.event_id = ev.event_id) - WHERE e.user_id = ? and notif > 0 - AND {room_id_clause} - ) AS es - GROUP BY room_id, stream_ordering, thread_id - HAVING stream_ordering > COALESCE(MAX(receipt_stream_ordering), 0) - """ - txn.execute( - sql, - receipt_types_args + [user_id] + room_id_args, - ) - for room_id, _ in txn: - room_to_count[room_id] += 1 + for room_id, notif_count in txn: + room_to_count[room_id] += notif_count return room_to_count From 571ca0c004a1a93ec4e04bf5d0b8a6f5a705762c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 5 Mar 2024 14:47:35 +0000 Subject: [PATCH 12/17] 1.102.0 --- CHANGES.md | 7 +++++++ changelog.d/16979.misc | 1 - debian/changelog | 6 ++++++ pyproject.toml | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) delete mode 100644 changelog.d/16979.misc diff --git a/CHANGES.md b/CHANGES.md index cb67babbac..59799ad394 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,10 @@ +# Synapse 1.102.0 (2024-03-05) + +### Internal Changes + +- Revert https://github.com/element-hq/synapse/pull/16756, which caused incorrect notification counts on mobile clients since v1.100.0. ([\#16979](https://github.com/element-hq/synapse/issues/16979)) + + # Synapse 1.102.0rc1 (2024-02-20) ### Features diff --git a/changelog.d/16979.misc b/changelog.d/16979.misc deleted file mode 100644 index a8c3b33ab2..0000000000 --- a/changelog.d/16979.misc +++ /dev/null @@ -1 +0,0 @@ -Revert https://github.com/element-hq/synapse/pull/16756, which caused incorrect notification counts on mobile clients. diff --git a/debian/changelog b/debian/changelog index a2170c1220..df0ab0dcd8 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.102.0) stable; urgency=medium + + * New Synapse release 1.102.0. + + -- Synapse Packaging team Tue, 05 Mar 2024 14:47:03 +0000 + matrix-synapse-py3 (1.102.0~rc1) stable; urgency=medium * New Synapse release 1.102.0rc1. diff --git a/pyproject.toml b/pyproject.toml index e1ffb9ffef..fe51b46b75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,7 +96,7 @@ module-name = "synapse.synapse_rust" [tool.poetry] name = "matrix-synapse" -version = "1.102.0rc1" +version = "1.102.0" description = "Homeserver for the Matrix decentralised comms protocol" authors = ["Matrix.org Team and Contributors "] license = "AGPL-3.0-or-later" From 1dee1b72ec80546c817e13ccf238ff80e5e94d4e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 5 Mar 2024 15:13:32 +0000 Subject: [PATCH 13/17] Switch #16979 changelog type from internal change to bugfix --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 59799ad394..2e7e4be5ac 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,6 @@ # Synapse 1.102.0 (2024-03-05) -### Internal Changes +### Bugfixes - Revert https://github.com/element-hq/synapse/pull/16756, which caused incorrect notification counts on mobile clients since v1.100.0. ([\#16979](https://github.com/element-hq/synapse/issues/16979)) From ab80b3412e314b25827bcc5c61628e024c8cf571 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 5 Mar 2024 16:02:54 +0000 Subject: [PATCH 14/17] Revert "Ignore notification counts from rooms you've left" (#16981) --- changelog.d/16954.bugfix | 1 - .../databases/main/event_push_actions.py | 18 +++--------------- 2 files changed, 3 insertions(+), 16 deletions(-) delete mode 100644 changelog.d/16954.bugfix diff --git a/changelog.d/16954.bugfix b/changelog.d/16954.bugfix deleted file mode 100644 index 7e5ad69094..0000000000 --- a/changelog.d/16954.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a bug introduced in v1.100.0 where notifications from rooms you've left would continue to be counted. \ No newline at end of file diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index 56c549ae66..d7aa8a0ee0 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -404,11 +404,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, notif_count, e.stream_ordering, e.thread_id, last_receipt_stream_ordering, ev.stream_ordering AS receipt_stream_ordering FROM event_push_summary AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -476,11 +472,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -522,11 +514,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id From 4af33015af280b4716e812e47d5631fcac088128 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 6 Mar 2024 16:00:20 +0100 Subject: [PATCH 15/17] Fix joining remote rooms when a `on_new_event` callback is registered (#16973) Since Synapse 1.76.0, any module which registers a `on_new_event` callback would brick the ability to join remote rooms. This is because this callback tried to get the full state of the room, which would end up in a deadlock. Related: https://github.com/matrix-org/synapse-auto-accept-invite/issues/18 The following module would brick the ability to join remote rooms: ```python from typing import Any, Dict, Literal, Union import logging from synapse.module_api import ModuleApi, EventBase logger = logging.getLogger(__name__) class MyModule: def __init__(self, config: None, api: ModuleApi): self._api = api self._config = config self._api.register_third_party_rules_callbacks( on_new_event=self.on_new_event, ) async def on_new_event(self, event: EventBase, _state_map: Any) -> None: logger.info(f"Received new event: {event}") @staticmethod def parse_config(_config: Dict[str, Any]) -> None: return None ``` This is technically a breaking change, as we are now passing partial state on the `on_new_event` callback. However, this callback was broken for federated rooms since 1.76.0, and local rooms have full state anyway, so it's unlikely that it would change anything. --- changelog.d/16973.bugfix | 1 + docs/modules/third_party_rules_callbacks.md | 4 ++++ .../third_party_event_rules_callbacks.py | 23 ++++++++----------- synapse/storage/controllers/state.py | 9 ++++++-- 4 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 changelog.d/16973.bugfix diff --git a/changelog.d/16973.bugfix b/changelog.d/16973.bugfix new file mode 100644 index 0000000000..a0e3a12304 --- /dev/null +++ b/changelog.d/16973.bugfix @@ -0,0 +1 @@ +Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. diff --git a/docs/modules/third_party_rules_callbacks.md b/docs/modules/third_party_rules_callbacks.md index d06bff25eb..b97e28db11 100644 --- a/docs/modules/third_party_rules_callbacks.md +++ b/docs/modules/third_party_rules_callbacks.md @@ -142,6 +142,10 @@ Called after sending an event into a room. The module is passed the event, as we as the state of the room _after_ the event. This means that if the event is a state event, it will be included in this state. +The state map may not be complete if Synapse hasn't yet loaded the full state +of the room. This can happen for events in rooms that were just joined from +a remote server. + Note that this callback is called when the event has already been processed and stored into the room, which means this callback cannot be used to deny persisting the event. To deny an incoming event, see [`check_event_for_spam`](spam_checker_callbacks.md#check_event_for_spam) instead. diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py index 7a3255aeef..9f7a04372d 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py @@ -366,7 +366,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_threepid_can_be_invited_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_threepid_can_be_invited_callbacks: try: @@ -399,7 +399,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_visibility_can_be_modified_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_visibility_can_be_modified_callbacks: try: @@ -427,7 +427,13 @@ class ThirdPartyEventRulesModuleApiCallbacks: return event = await self.store.get_event(event_id) - state_events = await self._get_state_map_for_room(event.room_id) + + # We *don't* want to wait for the full state here, because waiting for full + # state will persist event, which in turn will call this method. + # This would end up in a deadlock. + state_events = await self._storage_controllers.state.get_current_state( + event.room_id, await_full_state=False + ) for callback in self._on_new_event_callbacks: try: @@ -490,17 +496,6 @@ class ThirdPartyEventRulesModuleApiCallbacks: ) return True - async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]: - """Given a room ID, return the state events of that room. - - Args: - room_id: The ID of the room. - - Returns: - A dict mapping (event type, state key) to state event. - """ - return await self._storage_controllers.state.get_current_state(room_id) - async def on_profile_update( self, user_id: str, new_profile: ProfileInfo, by_admin: bool, deactivation: bool ) -> None: diff --git a/synapse/storage/controllers/state.py b/synapse/storage/controllers/state.py index 99666d79a9..22d93a561c 100644 --- a/synapse/storage/controllers/state.py +++ b/synapse/storage/controllers/state.py @@ -562,10 +562,15 @@ class StateStorageController: @trace @tag_args async def get_current_state( - self, room_id: str, state_filter: Optional[StateFilter] = None + self, + room_id: str, + state_filter: Optional[StateFilter] = None, + await_full_state: bool = True, ) -> StateMap[EventBase]: """Same as `get_current_state_ids` but also fetches the events""" - state_map_ids = await self.get_current_state_ids(room_id, state_filter) + state_map_ids = await self.get_current_state_ids( + room_id, state_filter, await_full_state + ) event_map = await self.stores.main.get_events(list(state_map_ids.values())) From 696cc9e802f63ba8657856d85f6982f49de14f27 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 8 Mar 2024 04:33:46 -0500 Subject: [PATCH 16/17] Stabilize support for Retry-After header (MSC4014) (#16947) --- changelog.d/16947.feature | 1 + synapse/api/errors.py | 5 ++--- synapse/config/experimental.py | 9 --------- tests/api/test_errors.py | 8 ++------ tests/rest/client/test_login.py | 3 --- 5 files changed, 5 insertions(+), 21 deletions(-) create mode 100644 changelog.d/16947.feature diff --git a/changelog.d/16947.feature b/changelog.d/16947.feature new file mode 100644 index 0000000000..efd0dbddb2 --- /dev/null +++ b/changelog.d/16947.feature @@ -0,0 +1 @@ +Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b44088f9b3..dd4a1ae706 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -517,8 +517,6 @@ class InvalidCaptchaError(SynapseError): class LimitExceededError(SynapseError): """A client has sent too many requests and is being throttled.""" - include_retry_after_header = False - def __init__( self, limiter_name: str, @@ -526,9 +524,10 @@ class LimitExceededError(SynapseError): retry_after_ms: Optional[int] = None, errcode: str = Codes.LIMIT_EXCEEDED, ): + # Use HTTP header Retry-After to enable library-assisted retry handling. headers = ( {"Retry-After": str(math.ceil(retry_after_ms / 1000))} - if self.include_retry_after_header and retry_after_ms is not None + if retry_after_ms is not None else None ) super().__init__(code, "Too Many Requests", errcode, headers=headers) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d43d9da956..0bd3befdc2 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -25,7 +25,6 @@ from typing import TYPE_CHECKING, Any, Optional import attr import attr.validators -from synapse.api.errors import LimitExceededError from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.config import ConfigError from synapse.config._base import Config, RootConfig @@ -415,14 +414,6 @@ class ExperimentalConfig(Config): "msc4010_push_rules_account_data", False ) - # MSC4041: Use HTTP header Retry-After to enable library-assisted retry handling - # - # This is a bit hacky, but the most reasonable way to *alway* include the - # headers. - LimitExceededError.include_retry_after_header = experimental.get( - "msc4041_enabled", False - ) - self.msc4028_push_encrypted_events = experimental.get( "msc4028_push_encrypted_events", False ) diff --git a/tests/api/test_errors.py b/tests/api/test_errors.py index 25fa93b9d8..efa3addf00 100644 --- a/tests/api/test_errors.py +++ b/tests/api/test_errors.py @@ -33,18 +33,14 @@ class LimitExceededErrorTestCase(unittest.TestCase): self.assertIn("needle", err.debug_context) self.assertNotIn("needle", serialised) - # Create a sub-class to avoid mutating the class-level property. - class LimitExceededErrorHeaders(LimitExceededError): - include_retry_after_header = True - def test_limit_exceeded_header(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=100) + err = LimitExceededError(limiter_name="test", retry_after_ms=100) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 100) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "1") def test_limit_exceeded_rounding(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=3001) + err = LimitExceededError(limiter_name="test", retry_after_ms=3001) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 3001) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "4") diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py index 3d3a7b0aa7..3a1f150082 100644 --- a/tests/rest/client/test_login.py +++ b/tests/rest/client/test_login.py @@ -177,7 +177,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): # rc_login dict here, we need to set this manually as well "account": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_address(self) -> None: @@ -229,7 +228,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): # rc_login dict here, we need to set this manually as well "address": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account(self) -> None: @@ -278,7 +276,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): "address": {"per_second": 10000, "burst_count": 10000}, "failed_attempts": {"per_second": 0.17, "burst_count": 5}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account_failed_attempts(self) -> None: From 48f59d3806477d575350fa4dc093e02cf0eca901 Mon Sep 17 00:00:00 2001 From: Alexander Fechler <141915399+afechler@users.noreply.github.com> Date: Mon, 11 Mar 2024 17:08:04 +0100 Subject: [PATCH 17/17] deactivated flag refactored to filter deactivated users. (#16874) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/16874.feature | 1 + docs/admin_api/user_admin_api.md | 14 ++++++ synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/users.py | 21 +++++++- synapse/storage/databases/main/__init__.py | 9 ++-- tests/rest/admin/test_user.py | 56 ++++++++++++++++++++-- 6 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 changelog.d/16874.feature diff --git a/changelog.d/16874.feature b/changelog.d/16874.feature new file mode 100644 index 0000000000..6e5afdde3b --- /dev/null +++ b/changelog.d/16874.feature @@ -0,0 +1 @@ +Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 9dc600b875..9736fe3021 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -164,6 +164,7 @@ Body parameters: Other allowed options are: `bot` and `support`. ## List Accounts +### List Accounts (V2) This API returns all local user accounts. By default, the response is ordered by ascending user ID. @@ -287,6 +288,19 @@ The following fields are returned in the JSON response body: *Added in Synapse 1.93:* the `locked` query parameter and response field. +### List Accounts (V3) + +This API returns all local user accounts (see v2). In contrast to v2, the query parameter `deactivated` is handled differently. + +``` +GET /_synapse/admin/v3/users +``` + +**Parameters** +- `deactivated` - Optional flag to filter deactivated users. If `true`, only deactivated users are returned. + If `false`, deactivated users are excluded from the query. When the flag is absent (the default), + users are not filtered by deactivation status. + ## Query current sessions for a user This API returns information about the active sessions for a specific user. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 0e413f02ab..07e0fb71f2 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -109,6 +109,7 @@ from synapse.rest.admin.users import ( UserReplaceMasterCrossSigningKeyRestServlet, UserRestServletV2, UsersRestServletV2, + UsersRestServletV3, UserTokenRestServlet, WhoisRestServlet, ) @@ -289,6 +290,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UserTokenRestServlet(hs).register(http_server) UserRestServletV2(hs).register(http_server) UsersRestServletV2(hs).register(http_server) + UsersRestServletV3(hs).register(http_server) UserMediaStatisticsRestServlet(hs).register(http_server) LargestRoomsStatistics(hs).register(http_server) EventReportDetailRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 0229e87f43..a9645e4af7 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -23,7 +23,7 @@ import hmac import logging import secrets from http import HTTPStatus -from typing import TYPE_CHECKING, Dict, List, Optional, Tuple +from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Union import attr @@ -118,7 +118,8 @@ class UsersRestServletV2(RestServlet): errcode=Codes.INVALID_PARAM, ) - deactivated = parse_boolean(request, "deactivated", default=False) + deactivated = self._parse_parameter_deactivated(request) + locked = parse_boolean(request, "locked", default=False) admins = parse_boolean(request, "admins") @@ -182,6 +183,22 @@ class UsersRestServletV2(RestServlet): return HTTPStatus.OK, ret + def _parse_parameter_deactivated(self, request: SynapseRequest) -> Optional[bool]: + """ + Return None (no filtering) if `deactivated` is `true`, otherwise return `False` + (exclude deactivated users from the results). + """ + return None if parse_boolean(request, "deactivated") else False + + +class UsersRestServletV3(UsersRestServletV2): + PATTERNS = admin_patterns("/users$", "v3") + + def _parse_parameter_deactivated( + self, request: SynapseRequest + ) -> Union[bool, None]: + return parse_boolean(request, "deactivated") + class UserRestServletV2(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]*)$", "v2") diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index 394985d87f..bf779587d9 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -176,7 +176,7 @@ class DataStore( user_id: Optional[str] = None, name: Optional[str] = None, guests: bool = True, - deactivated: bool = False, + deactivated: Optional[bool] = None, admins: Optional[bool] = None, order_by: str = UserSortOrder.NAME.value, direction: Direction = Direction.FORWARDS, @@ -232,8 +232,11 @@ class DataStore( if not guests: filters.append("is_guest = 0") - if not deactivated: - filters.append("deactivated = 0") + if deactivated is not None: + if deactivated: + filters.append("deactivated = 1") + else: + filters.append("deactivated = 0") if not locked: filters.append("locked IS FALSE") diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 9265854223..c5da1e9686 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -503,7 +503,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): channel = self.make_request( "GET", - self.url + "?deactivated=true", + f"{self.url}?deactivated=true", {}, access_token=self.admin_user_tok, ) @@ -982,6 +982,56 @@ class UsersListTestCase(unittest.HomeserverTestCase): self.assertEqual(1, channel.json_body["total"]) self.assertFalse(channel.json_body["users"][0]["admin"]) + def test_filter_deactivated_users(self) -> None: + """ + Tests whether the various values of the query parameter `deactivated` lead to the + expected result set. + """ + users_url_v3 = self.url.replace("v2", "v3") + + # Register an additional non admin user + user_id = self.register_user("user", "pass", admin=False) + + # Deactivate that user, requesting erasure. + deactivate_account_handler = self.hs.get_deactivate_account_handler() + self.get_success( + deactivate_account_handler.deactivate_account( + user_id, erase_data=True, requester=create_requester(user_id) + ) + ) + + # Query all users + channel = self.make_request( + "GET", + users_url_v3, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(2, channel.json_body["total"]) + + # Query deactivated users + channel = self.make_request( + "GET", + f"{users_url_v3}?deactivated=true", + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(1, channel.json_body["total"]) + self.assertEqual("@user:test", channel.json_body["users"][0]["name"]) + + # Query non-deactivated users + channel = self.make_request( + "GET", + f"{users_url_v3}?deactivated=false", + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(1, channel.json_body["total"]) + self.assertEqual("@admin:test", channel.json_body["users"][0]["name"]) + @override_config( { "experimental_features": { @@ -1130,7 +1180,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): # They should appear in the list users API, marked as not erased. channel = self.make_request( "GET", - self.url + "?deactivated=true", + f"{self.url}?deactivated=true", access_token=self.admin_user_tok, ) users = {user["name"]: user for user in channel.json_body["users"]} @@ -1194,7 +1244,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): dir: The direction of ordering to give the server """ - url = self.url + "?deactivated=true&" + url = f"{self.url}?deactivated=true&" if order_by is not None: url += "order_by=%s&" % (order_by,) if dir is not None and dir in ("b", "f"):