fix: improve appservice authentication and thread pagination UX #923

Merged
tcpipuk merged 4 commits from tom/fixes into main 2025-08-12 07:06:31 +00:00

4 commits

Author SHA1 Message Date
583cb924f1 refactor: address code review feedback for auth and pagination improvements
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m8s
Checks / Prefligit / prefligit (pull_request) Successful in 19s
Release Docker Image / define-variables (push) Successful in 4s
Documentation / Build and Deploy Documentation (push) Successful in 34s
Checks / Prefligit / prefligit (push) Successful in 34s
Checks / Rust / Format (push) Successful in 39s
Checks / Rust / Clippy (push) Successful in 4m33s
Checks / Rust / Cargo Test (push) Successful in 4m57s
Release Docker Image / build-image (linux/amd64, release, linux-amd64, base) (push) Successful in 14m12s
Release Docker Image / build-image (linux/arm64, release, linux-arm64, base) (push) Successful in 14m31s
Release Docker Image / merge (push) Successful in 12s
- Extract duplicated thread/message pagination functions to shared utils module
- Refactor pagination token parsing to use Option combinators instead of defaults
- Split access token generation from assignment for clearer error handling
- Add appservice token collision detection at startup and registration
- Allow appservice re-registration with same token (for config updates)
- Simplify thread relation chunk building using iterator chaining
- Fix saturating_inc edge case in relation queries with explicit filtering
- Add concise comments explaining non-obvious behaviour choices
2025-08-12 05:29:41 +01:00
9286838d23 fix(relations): improve thread pagination and include root event
All checks were successful
Release Docker Image / define-variables (push) Successful in 5s
Checks / Prefligit / prefligit (push) Successful in 21s
Checks / Rust / Format (push) Successful in 55s
Checks / Rust / Clippy (push) Successful in 4m33s
Checks / Rust / Cargo Test (push) Successful in 5m20s
Release Docker Image / build-image (linux/amd64, release, linux-amd64, base) (push) Successful in 14m52s
Release Docker Image / build-image (linux/arm64, release, linux-arm64, base) (push) Successful in 15m10s
Release Docker Image / merge (push) Successful in 30s
Checks / Prefligit / prefligit (pull_request) Successful in 26s
Documentation / Build and Deploy Documentation (pull_request) Successful in 35s
Replace unreliable PduCount pagination tokens with ShortEventId throughout
the relations and messages endpoints. ShortEventId provides stable, unique
identifiers that persist across server restarts and database operations.

Key improvements:
- Add token parsing helpers that try ShortEventId first, fall back to
  PduCount for backwards compatibility
- Include thread root event when paginating backwards to thread start
- Fix off-by-one error in get_relations that was returning the starting
  event in results
- Only return next_batch/prev_batch tokens when more events are available,
  preventing clients from making unnecessary requests at thread boundaries
- Ensure consistent token format between /relations, /messages, and /sync
  endpoints for interoperability

This fixes duplicate events when scrolling at thread boundaries and ensures
the thread root message is visible when viewing a thread, matching expected
client behaviour.
2025-08-10 19:12:56 +01:00
d1ebcfaf0b fix(auth): prevent token collisions and optimise lookups
Ensures access tokens are unique across both user and appservice tables to
prevent authentication ambiguity and potential security issues.

Changes:
- On startup, automatically logout any user devices using tokens that
  conflict with appservice tokens (resolves in favour of appservices)
  and log a warning with affected user/device details
- When creating new user tokens, check for conflicts with appservice tokens
  and generate a new token if a collision would occur
- When registering new appservices, reject registration if the token is
  already in use by a user device
- Use futures::select_ok to race token lookups concurrently for better
  performance (adapted from tuwunel commit 066097a8)

This fix-forward approach resolves existing token collisions on startup
whilst preventing new ones from being created, without breaking existing
valid authentications.

The find_token optimisation is adapted from tuwunel (matrix-construct/tuwunel)
commit 066097a8: "Optimize user and appservice token queries" by Jason Volk.
2025-08-10 17:10:06 +01:00
e820551f62 fix(appservice): create sender_localpart user during appservice startup
Fixes #813: Application services were unable to work because their sender_localpart
user was never created in the database, preventing authentication.

This fix ensures the appservice user account is created when:
- The server starts up and loads existing appservices from the database
- A new appservice is registered via the admin command

Additionally, if an appservice user has been accidentally deactivated, it will be
automatically reactivated when the appservice starts.

The solution centralises all appservice startup logic into a single `start_appservice`
helper method, eliminating code duplication between the registration and startup paths.
2025-08-10 17:10:06 +01:00