Fixes#19561
`GET /_synapse/admin/v1/statistics/database/rooms` returns a 500 when
`pg_stats` has no common `room_id` values for one of the tables it
checks. This happens in two cases:
- The table is empty or not yet analyzed, so `pg_stats` has no row for
it.
- No value is more common than the others, so `most_common_vals` is
`NULL`. `event_edges.room_id` hits this on servers where all rows were
written after schema 72 stopped filling the column.
The change skips the table in both cases, since it adds nothing to the
estimate.
Thanks to @christianarielli for finding the cause and suggesting the
fix. They're credited as co-author on the commit.
The new test runs only on Postgres, as the endpoint doesn't support
SQLite. It creates a room, runs `ANALYZE` while `event_push_actions` is
empty and `event_edges.room_id` is all `NULL`, and checks for a 200 with
the room listed. I ran it against Postgres 17.
`event_edges.room_id` isn't written any more, so dropping that table
from the list might make sense. I left that out to keep this PR small.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Christian Arielli <903609+christianarielli@users.noreply.github.com>
Co-authored-by: Eric Eastwood <erice@element.io>
This fixes the test `ThreadedMemoryReactorClock` to support
`removeSystemEventTrigger`, which in turn allows the shutdown triggers
to run when `HomeServer.shutdown()` is called in the tests. Previously,
`ThreadedMemoryReactorClock.removeSystemEventTrigger` raised a
`NotImplementedError`,
`cleanup_test_reactor_system_event_triggers` cleared the triggers by
hand so that a homeserver could be garbage collected. `shutdown()` now
removes them itself, so the helper goes.
We can also remove all the `tearDown` implementations that tried to
manually trigger the shutdown as its not longer necessary to do so
manually. They also didn't work, as they indexed `reactor.triggers` as
`[eventType][phase]` when the keys are `[phase][eventType]`
`DeviceHandler.get_user_ids_changed`, which the sliding sync e2ee
extension and `GET /keys/changes` use, adds a member to
`newly_joined_or_invited_or_knocked_users` only when the previous
membership was none of join, invite or knock. A user going from invite
to join is skipped, so other members are never told to refresh the
devices of a user whose device list changed while they were only
invited. Classic sync does report them.
A change to join from any membership other than join is now counted.
join -> join (a profile change) is still ignored.
`test_extension_e2ee.py` covers invite -> join, invite -> join -> leave
in one window and a profile change, `test_keys.py` covers invite -> join
through `/keys/changes`. Not covered: device list changes made while a
user is only invited (#3504), which are still not announced until the
user joins.
Fixes#20486
Follows on from #20289 and #20433. Properly fixes handling of concurrent
requests (on single workers).
This tries to fix two cases:
1. We are *computing* responses for multiple syncs at the same time,
which currently causes the cache to get cross-contaminated.
2. Two syncs concurrent syncs on the wire (but don't overlap during
processing). When the second request is processed we correctly forget
any entries from the first request, however we currently still update
the cache with members returned on the second request. However, there is
no guarantee that the second request is what the client actually reads.
We use three rules to handle concurrent requests:
1. A response only uses cached members recorded at or before the
request's `since`. Members recorded after it cannot be from an ancestor
sync response.
2. Once a response has been built, we forget all already cached members
recorded after its `since`. This handles the client reusing an older
`since` token. Doing it at the end of the build rather than the start
also forgets members recorded by responses built at the same time.
3. We track the highest position seen for the device (`_newest`), from
the `since` of each request and the `next_batch` of each built response.
A response only records its members if its `next_batch` is after that
position. This handles responses that end at the same position, or that
finish out of order.
With these rules, when concurrent requests happen we throw away all
existing cached members and don't record any further members until one
sync has advanced sufficiently such that there are no longer any
concurrent sync requests.
Note that ideally we'd have some sort of per-connection identity so that
we can track this properly (like in sliding sync). However, that is a
bigger change. Instead, we note that we just need to detect concurrent
requests, and not update the cache in those cases.
Also note, can only detect concurrent requests that go to the same
worker.
### Pull Request Checklist
A notary key query that leaves out minimum_valid_until_ts is rejected
with errcode M_MISSING_PARAM and an error naming that field as required.
Callers that follow the server-server API, where the timestamp is
optional and the server is supposed to use the current time, cannot
complete the query. The criteria model typed minimum_valid_until_ts as
an integer or null but gave it no default, so validation treated the
field as required and failed before the query ran.
An explicit null was already a separate accepted value, used for
half-lifetime cache behaviour, and was not the same as omitting the
field.
The field now defaults to null, so an omitted timestamp validates. After
parsing, each key whose criteria did not include the field is replaced
with a copy that sets minimum_valid_until_ts to the current time in
milliseconds. Explicit null is left unchanged, and an empty key-id map
is left unchanged so a request for every cached key still ignores
validity. Tests cover the omitted timestamp against cache lifetime,
mixed criteria including zero and null, non-integer timestamps, and a
body without server_keys, and a bugfix newsfragment records the default.
Ran `python3 -m pytest -q tests/rest/key/v2/test_remote_key_resource.py`
in a Linux container (`python:3.12`) for the synapse RemoteKey change; 7
tests passed.
The tests in `tests/rest/key/v2/test_remote_key_resource.py` fail
without the fix in the synapse RemoteKey change (checked by reverting
the source changes).
Fixes#20466
---------
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This PR should be reviewable on a commit-by-commit basis. If you would
prefer that it is broken down into multiple PRs then I can do so.
Fixes to match latest MSC4388:
- Return `200` with `create_available=false` for `GET` when configured
in `authenticated` mode and no access token presented
- Return `401` for a `GET` where an invalid/expired token is presented
- Return `404` for `DELETE` where session has expired but not yet
evicted
Features:
- Rate limiting which is configurable via
`experimental_features.msc4388_rc_create` (for `POST` and is per
requester for authenticated requests or per client IP for
unauthenticated requests) and
`experimental_features.msc4388_rc_requests` (for `GET`/`PUT`/`DELETE`).
- Allow the session TTL to be configured via the
`experimental_features.msc4388_session_ttl`. As per spec this is between
2 and 5 minutes.
- Concurrent sessions are unlimited for authenticated requesters
(subject to the rate limits).
- Unauthenticated requesters have a maximum concurrent sessions limit
(default 1,000) which is configurable by
`experimental_features.msc4388_max_unauthenticated_sessions`.
Misc:
- Code and comment cleanup
The experimental feature is tracked by
https://github.com/element-hq/synapse/issues/19433 and I will update it
with the new config options when this PR merges.
Follow-up to https://github.com/element-hq/synapse/pull/20407, which
added the `excluded_user_ids` option without enforcing it. This makes
legacy `/sync` respect it: users listed in a room's `excluded_user_ids`
no longer get that room through the synthetic join. They can still read
it, as it is world-readable.
The cached lookup now maps room ID to config ID, so the per-user filter
runs in memory and adds no database query to `/sync`.
`get_server_announcement_room_ids()` is kept as a wrapper, so the
stacked PRs are unaffected.
Known limitation, documented in the config docs: changing the list does
not update rooms that clients already have. A newly excluded user keeps
the room, which stops updating. A user removed from the list gets new
events without the room's state until their next initial sync. I think
that's an okay limitation for now, as we don't really expect this list
to be used for real users
---------
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
> [!NOTE]
> Epic: [Efficient Server
Announcements](https://github.com/element-hq/synapse/issues/20240)
> Follows: https://github.com/element-hq/synapse/pull/20372
This PR first tests that every user can read the server announcements
through the "history" endpoints like
- [x] `GET /rooms/{roomId}/messages`
- [x] `GET /rooms/{roomId}/context/{eventId}`
- [x] `GET /rooms/{roomId}/event/{eventId}`
- [x] `GET /rooms/{roomId}/relations/{eventId}`
- [x] `GET /rooms/{roomId}/threads`
- [x] `GET /rooms/{roomId}/initialSync`
This already works for users who have never been in the room, because
announcement rooms are world-readable.
It doesn't for a sender who was removed from the room's config: Synapse
still uses their `leave` membership and shows them the room as it was
when they left.
This PR then treats a user who left a configured announcement room like
a user who has never been in it.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Quentin Gliech <quenting@element.io>
- For the dedicated delayed event scheduling endpoint, drop support for
setting the sticky duration in a query parameter, as MSC4354 is slated
to use only a body parameter for that
- Restrict the sticky duration for delayed events to being strictly
positive, analogously to #20350
Fixes#19257.
When an identity server returns a non-2xx response to
`/_matrix/identity/v2/store-invite` during a third-party invite,
`ask_id_server_for_third_party_invite` lets the `HttpResponseException`
escape. The client receives a 500 `M_UNKNOWN` error and the real cause
(for example, `M_THREEPID_IN_USE`) is lost.
This change catches `HttpResponseException` and converts it with
`to_synapse_error()`, which matches how the other identity server calls
in `IdentityHandler` (bind, `requestToken`) already handle error
responses.
A new test in `ThreepidInviteTestCase` makes the identity server return
a 400 `M_THREEPID_IN_USE` error and checks that the client receives the
same code and errcode. The test fails on `develop` with `500 != 400` and
passes with this change.
Broke by #20289. Hopefully fixeselement-hq/element-web#35567.
One subtle change in #20289 was that we started to filter rooms with
`full_state` set through the cache. This *should* be safe since none of
the membership events should have been previously sent down the sync.
However, there is a bug with the lazy-loaded member cache when two
concurrent syncs for the same device happens. The updates to the cache
from the handling of one request are immediately seen by the other
request, polluting the cache.
This is a relatively common scenario in Element Web when a user reloads
the page. The sync from before the reload is still being processed when
Synapse receives the initial sync from after the reload.
The full fix to correctly handle overlapping requests is a separate PR,
but this is a stop-gap measure that should help Element Web.
This includes the following:
- Implements ['Implicit Forward
Pagination'](https://github.com/matrix-org/matrix-spec/issues/2378).
Remove the extension's `since` and `next_batch` fields. The position is
now tracked via the global `pos`, with per-room connection state
recording how far each room has been sent so that `limit` paginates
without holding `pos` back and lost responses are re-delivered.
- Discussed in
https://github.com/matrix-org/matrix-spec-proposals/pull/4480#discussion_r4165887827
- Honour the `lists` and `rooms` fields.
- Send all unexpired sticky events when a room enters scope for the
first time or is newly joined, and catch up on events sent while a room
was out of scope when it re-enters.
- Enabling the extension partway through a connection returns all
unexpired sticky events for in-scope rooms, and re-enabling it after
disabling it returns the unexpired sticky events sent in the meantime.
I tried to split this up into standalone commits though the bulk of the
change is in the third commit.
---------
Co-authored-by: Olivier 'reivilibre <git.oo@emunest.net>
## What the MSC says
From MSC4140's [Auth rule
evaluation](https://github.com/matrix-org/matrix-spec-proposals/blob/b7cb09664fed684388ba656459d08cfaf786c1d1/proposals/4140-delayed-events-futures.md#auth-rule-evaluation):
> The server SHOULD also check the event could be sent when it is
scheduled and reject the request if not. In particular, servers SHOULD
check the sender is in the room (or otherwise authorized to send the
event) to avoid storing events for rooms the user is not in.
## This PR
- Checks the event against the room's auth rules as if it were sent now,
and returns the same 403 `M_FORBIDDEN` that `/send` and `/state` would.
- Changes to the sender's own membership are exempt: a join or knock can
be sent from outside the room, which is how this PR reads the MSC's "or
otherwise authorized". They are checked by the auth rules when they are
sent.
Here are a few examples:
| Delayed event | Before | After |
| ----------------------------------------------------------------- |
--------------------------------------- |
---------------------------------------- |
| `m.room.message` from outside the room | 200 `delay_id`, fails at send
time | 403 `M_FORBIDDEN` |
| Kick of another member from outside the room | 200 `delay_id`, fails
at send time | 403 `M_FORBIDDEN` |
| `m.room.name` from a member whose power level is too low | 200
`delay_id`, fails at send time | 403 `M_FORBIDDEN` |
| **Exception**: join of a public room from outside it | 200 `delay_id`,
joins at send time | unchanged |
| Join from outside the room, then a message due after it | both
accepted, message sent once joined | join accepted, message 403
`M_FORBIDDEN` |
Part of: MSC4354 (Sticky Events, Experimental Feature tracking issue
#19409)
When a user is the sender of a sticky event that comes down sync, that
user is now in scope for having their lazy-loaded membership sent down
in sync.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Depends on: https://github.com/matrix-org/complement/pull/914
`displayname` and `avatar_url` already did not permit nulls, seemingly
from #8223.
The specification at v1.15 does not permit `null` for those fields
either, so this seems consistent with historical specification.
This only changes behaviour for 'custom' fields.
---------
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Closes#20116
> [!NOTE]
> This PR Follows #20115 which already implemented this for
`m.device_list_update`
> User IDs with localparts containing characters outside the range
U+0021 to U+007E, or with
> an empty localpart, are considered non-compliant. For current room
versions, servers must
> still accept events using such user IDs over federation; however they
SHOULD NOT forward
> such user IDs to clients when referenced outside the context of an
event. For example,
> device list updates from non-compliant user IDs would be dropped by
the receiving server.
>
> — [Spec
1.19](https://spec.matrix.org/v1.19/appendices/#historical-user-ids)
This PR prevents from forwarding non compliant user ids for the
following events:
- `m.signing_key_update` 8c3c35e95d
- `m.presence` 26a8dfe862
- `m.typing` 1110cf2d73
- `m.receipt` 9e2da8b0a6
- `m.direct_to_device` 9541232a30
These things are not needed now that MSC2965 has stabilised. In any
case, they
should never have been exposed without a config flag in the first place.
This speeds up the /createRoom calls a tiny bit by not checking the
senders push rules.
This helps speed up a lot of test runs in Element Web which
runs a lot of tests in one-person rooms (and creates a heck of a lot of rooms)
## Summary
- default `request_token_inhibit_3pid_errors` to `true` to prevent
account enumeration
- document the new default and the explicit opt-out
- add an upgrade note and regression coverage for the default
Fixes#18415.
Part of #19414 (Support Matrix 1.16).
⚠️ Before merging, wait for the following PRs to be merged first:
- https://github.com/element-hq/synapse/pull/20081
- https://github.com/element-hq/synapse/pull/20169
- https://github.com/element-hq/synapse/pull/20171
---
Since Matrix 1.16, the
[spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
defines the stable identifiers for
[MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222):
the `use_state_after` query parameter on `GET /_matrix/client/v3/sync`,
and the `state_after` room response field:
> **`use_state_after`** — `boolean` — Controls whether to receive state
changes between the previous sync and the start of the timeline, or
between the previous sync and the end of the timeline. If this is set to
`true`, servers MUST respond with the state between the previous sync
and the end of the timeline in `state_after` and MUST omit `state`. […]
>
> **`state_after`** — […] Required if `use_state_after` was set to true
in the request, even if it is empty. Added in `v1.16`
>
> — [Matrix
Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
Synapse so far only implemented the unstable identifiers
(`org.matrix.msc4222.use_state_after` /
`org.matrix.msc4222.state_after`), gated behind the `msc4222_enabled`
experimental flag and the per-user experimental features admin API.
This PR:
- accepts the stable `use_state_after` query parameter, unconditionally,
and responds with the stable `state_after` field. The unstable query
parameter is kept for a transition period; clients opting in with it get
the unstable response field name back, so unstable-era clients see an
unchanged wire format. If both parameters are set, the stable one wins.
- removes the `msc4222_enabled` experimental flag and the `msc4222`
per-user feature from the experimental features admin API, since the
behaviour is no longer gated.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
TLDR: `PUT` on a server-default push rule now applies the same existence
check that `GET` already does. A rule that is not served cannot be
modified.
Synapse ships some server-default push rules behind experimental feature
flags, so the set of server-default rules a user has depends on the
server's configuration. Most flags add rules that only exist while the
flag is on.
[MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210)
works the other way: it removes the three legacy mention rules while its
flag is on. The flags that change the set are:
- `msc1767_enabled`:
[MSC1767](https://github.com/matrix-org/matrix-spec-proposals/pull/1767)
extensible events rules
- `msc3381_polls_enabled`:
[MSC3381](https://github.com/matrix-org/matrix-spec-proposals/pull/3381)
poll rules
- `msc3664_enabled`:
[MSC3664](https://github.com/matrix-org/matrix-spec-proposals/pull/3664)
reply rule
- `msc4028_push_encrypted_events`:
[MSC4028](https://github.com/matrix-org/matrix-spec-proposals/pull/4028)
encrypted event rule
- `msc4306_enabled`:
[MSC4306](https://github.com/matrix-org/matrix-spec-proposals/pull/4306)
thread subscription rules
- `msc4210_enabled`: removes `.m.rule.contains_display_name`,
`.m.rule.contains_user_name` and `.m.rule.roomnotif`
`GET /pushrules/` only lists the rules that exist for that
configuration. `GET` on a single rule that is not in the set returns
`404 M_NOT_FOUND`. The same set is used to evaluate notifications, and
to build the `m.push_rules` account data in `/sync`.
`PUT .../enabled` and `PUT .../actions` did not use that set. They
accepted any rule ID that Synapse knows about, whether or not the server
currently serves it. Clients could modify a rule the server had just
said did not exist. The request returned `200`, the change was stored,
and it was then ignored. Both endpoints now return `404 M_NOT_FOUND` for
such a rule, as [the spec
requires](https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3pushrulesglobalkindruleidenabled)
for a rule that does not exist.
Before (`msc4210_enabled: true`):
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
200 {}
```
After:
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
404 {"errcode": "M_NOT_FOUND", "error": "Unknown rule"}
```
This is a no-op for default configurations. All of these flags are off
by default, and rules that are served are still modifiable as before.
Use a new, dedicated endpoint for scheduling delayed events. This should
deprecate the `delay` query parameter on the `/send` and `/state`
endpoints.
Co-authored-by: Paul Chobert <paul@chobert.fr>
Co-authored-by: Eric Eastwood <erice@element.io>
In a similar vein to #20315, this attempts to speed up Synapse when
delayed events aren't in use. Here we skip doing any work if the room
event stream has not advanced.
Fixes: #19661
Part of: MSC4354
First introduced some helpers for querying a sharded stream, which were moved to
https://github.com/element-hq/synapse/pull/20108.
- SQL builder for 'give me rows between two sharded tokens'
- a function to return the 'new' from_token after a limited read
Then make the sticky events portions of both oldschool and sliding sync use
sharded tokens.
This prevents a 'torn sync' problem where, because the events and sticky
events streams are both sharded with the same writers:
1. persisting a sticky event leads to an event row and a `sticky_events`
row being created for event `e` (on writer `W`).
2. Suppose another writer is still persisting, so the position of the
stream for writer W advances, but the global
['linear'](https://element-hq.github.io/synapse/latest/development/synapse_architecture/streams.html?highlight=%E2%80%9Clinear%E2%80%9D#current-stream-id)
(I have been calling this 'baseline') position does not advance yet.
3. sync is sharding-aware for the regular events stream, so `e` appears
as a real-time timeline event, thus coming down sync
4. since sticky events were not sharding-aware, `e` is not detected as a
sticky event in this sync response. The sticky event position doesn't
advance in the sync token, either.
5. (assume the position advances because other writers have finished
writing and so the 'linear' position advances for the sticky events
stream)
6. the client then syncs again, using the new sync token. This time, `e`
appears in the sticky event section.
If we make sticky events sharding-aware in sync, then `e` would appear
for consideration in both the timeline section **and** the sticky event
section in the first sync.
It then gets deduplicated out of the sticky section since it is
appearing in the timeline section in the same sync.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
In `/sync` there is a mode called lazy loaded (LL) members. In this mode
`/sync` does not return all memberships in the room, instead it only
returns a) membership changes, and b) the memberships of senders of
events in the timeline (if not previously sent).
Synapse implements a lazy-loaded member cache to avoid repeatedly
sending down the same memberships when a single user sends multiple
events. It does this by storing for each user/device the memberships it
has previously returned in an LRU cache. Note that it is fine for the
server to return the same membership, clients handle that fine, but it
is a waste of bandwidth.
The current lazy-loaded membership cache has three intertwined flaws:
1. It does not handle retries or clients re-using old tokens.
2. It incorrectly applies the membership cache to membership changes,
not just memberships added by lazy-loading.
3. It does not correctly handle multiple "sync streams" from the same
device.
In this PR we explicitly fix the first two issues in two commits, and
the third issue is mitigated:
1. Alongside each membership in the LL cache we store the `next_batch`
position of the response that carried it. A client that processed a
response syncs from its `next_batch` next. When a request arrives with a
`since` token, we delete every membership recorded after that token, as
they were sent in responses the client never processed. Memberships
recorded at or before `since` are kept.
2. We always return memberships that have changed, even if they appear
in the LL member cache.
These changes also half mitigate the issue of running multiple `/sync`
loops per-device (though we do not see this in the wild), as we'd at
least always return the changed membership states.
Fixes#19978 and #20278, supersedes #20279
Fixes#15871
### What was broken
Sending `{"auth": null}` to any endpoint that requires user-interactive
auth (for example `POST /keys/device_signing/upload`, `DELETE
/devices/{deviceId}` or `POST /register`) returned a 500 instead of the
usual 401 with the available flows. A non-object `auth` on endpoints
that do not validate the body with a model (such as `/register`) could
also 500, e.g. `{"auth": ["session"]}`.
### Root cause
`AuthHandler.check_ui_auth` did `clientdict.pop("auth", {})`, so the
`{}` default only applies when the key is missing. An explicit `null`
came through as `None` and the following `"session" in authdict` raised
`TypeError`. `get_session_id` had the same assumption that `auth` is a
dict.
### Pull Request Checklist
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Signed-off-by: Ankit Jha <ankit.jha@tradomate.one>
