`KeyStore.get_server_keys_json` orders the `server_keys_json` rows of a
key by `ts_added_ms` and each row overwrites the previous one, so the
last notary to answer wins even when its `ts_valid_until_ms` is lower
than an earlier answer's. When that answer is stale, `StoreKeyFetcher`
returns a key below `minimum_valid_until_ts` and event signature
verification fails although a long enough key is stored.
The query now orders by `ts_valid_until_ms, ts_added_ms`, so the
longest-valid entry wins and the most recent one still wins a tie.
`tests/crypto/test_keyring.py` stores two notary responses for one key,
the longer-lived one first, and checks that the lookup and
`verify_json_for_server` use it. Not covered:
`get_server_keys_json_for_remote` and
`get_all_server_keys_json_for_remote`, which still prefer the most
recently added row.
Fixes#20358
### 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 (ruff check, ruff format and mypy run on the changed files)
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>
Part of: MSC4354 (Experimental Feature tracking issue #19409)
Follows: #20165 (Sticky Event federation backlog + catch-up
transactions)
The MSC says that we need to tell a newly-joining server about all of
our sticky events (yes, even the ones before the server joined the
room).
This PR introduces the mechanism, building on the 'Sticky Event backlog'
mechanism introduced in #20165, to make that happen.
Most of the logic is taken from the original Sticky Events PR #18968,
but the Sticky Event backlog system did not exist back then.
(The benefit of having the persistent Sticky Event backlog is that we
don't need to rely on the federation senders not crashing to make sure
we get around to sending the Sticky Events out.)
- Add NEW_SERVER_JOINED replication command. (This is a cross-worker
'wake-up call' to check for a sticky event backlog)
We send it when persisting a state change that causes a new server to be
in the room.
We handle it by waking up federation senders for the destination,
including checking the sticky event backlog tables.
- When a server newly joins a room, we insert a sticky event backlog row
for that server, if there are any Sticky Events needing to be sent.
---------
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>
Follows: #18375
This bugfix is a prerequisite for MSC4354 (Sticky Events, Experimental
Feature tracking issue #19409) changes.
- Add a failing test for join->join being newly_joined
- Fix the join->join transition being treated as a new join
(when prev_events didn't coincidentally mask the problem)
Rationale:
`event_edges` / `prev_events` does NOT link a membership event
to its previous state.
(It could even be someone else's membership!)
In most real-world cases, this would generate a `prev_membership` of
NULL
for every normal membership change.
It appears as though the author intended some kind of 'previous state'
relationship, but if you fix the first branch of the `UNION` query
to use `current_state_delta_stream` for this (which is the source of
state transitions), then it looks very redundant with the second
branch.
It looks like the real reason we want to use the snapshots table is for
out-of-band memberships (outlier memberships).
So we continue using that for those, but ONLY for those.
The `SUSPICIOUS` if block is removed because it appears to have been
papering over this issue by deduplicating the spurious `NULL` ->
non-null transitions,
but when you remove those duplicates at the source, it causes more
trouble
because it doesn't know that what it's 'deduplicating' is actually the
same membership transition.
This means if you join, then out-of-band leave (e.g. last member on the
server
to leave), that room would appear in `newly_joined` on a new initial
sync.
(Tests did in fact catch this, too.)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
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: MSC4354
Part of: #19641
Follows: #20232 (which applies already-persisted redactions to Sticky
Events being persisted)
In some cases we batch events together during persistence.
If one of the events in the batch is a redaction of another event, we
should apply it, which provides the same benefit as #20232 (namely: we
avoid potentially dodgy event content being persisted & the event
naturally loses its stickiness
so is not written to `sticky_events`)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes#20309.
For endpoints that validate the request body with Pydantic, a validation
failure put Pydantic's raw verbose error text into the `error` field of
the standard error response, e.g.:
```json
{
"errcode": "M_MISSING_PARAM",
"error": "1 validation error for PutBody\\nroom_id\\n Field required [type=missing, input_value={}, input_type=dict]\\n For further information visit https://errors.pydantic.dev/2.12/v/missing"
}
```
This leaks internal details clients cannot act upon (model class names,
raw input values, pydantic.dev URLs). This PR adds a
`_format_pydantic_error` helper in `synapse/http/servlet.py` that builds
a short client-friendly message from the structured
`ValidationError.errors()` data instead of `str(e)`:
- `{}` against a model requiring `room_id` -> `Missing required field
'room_id'.`
- `{"room_id": 123}` -> `'room_id': Input should be a valid string.`
- nested models use dotted paths, e.g. `Missing required field
'put.room_id'.`
- multiple errors are joined: `Missing required field 'name'; Missing
required field 'put'.`
The `errcode` selection logic (`M_MISSING_PARAM` / `M_INVALID_PARAM` /
`M_BAD_JSON`) is unchanged.
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>
Fixes https://github.com/element-hq/synapse/issues/18793
When the last local user leaves a room, Synapse clears
`current_state_events` and writes `null` `event_id` entries into the
`current_state_delta_stream`. The incremental sync code for MSC4222's
state_after skipped those `null` deltas entirely, and
`get_current_state_ids` also returned nothing (table already cleared).
Result: clients using `state_after` never learned about the leave.
Fix by falling back to state groups (which are preserved) to look up the
actual state at `end_token` for any cleared state keys.
### 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).
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: Paul Chobert <paul@chobert.fr>
Co-authored-by: Quentin Gliech <quenting@element.io>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Several clients, including Element Web/Desktop and Element X, have
already implemented
[MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426)
User Status profile fields. However, we have only implemented sending
profile fields down sync for local users
([MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429)/[MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262)),
since federation support for pushing profile field updates from server
to server has not yet been implemented
([MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259)).
This has created user confusion. The before mentioned clients use the
profile and/or profile field fetch endpoints to fetch missing profile
information, regardless of the user being local or remote. Since the
sync response isn't delivering these profiles, the client does a
fallback fetch, causing a one time update of the profile information
over federation.
This causes stale data to be cached in the client for the remote user,
which in the case of EW for example is never cleared.
This filtering should be temporary until MSC4259 is implemented and the
remote profile data also syncs via the profile update stream to clients.
I suggest making this not configurable so we don't have to maintain a
configuration property, as hopefully this would only be a case for a few
months at most.
### 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.
Ayesha Waseem