feat(matrix): add OIDC authentication via Matrix Authentication Service#730
Merged
feat(matrix): add OIDC authentication via Matrix Authentication Service#730
Conversation
…ce (#711) Modern Matrix homeservers (including matrix.org since April 2025) use Matrix Authentication Service (MAS) which implements OAuth 2.0/OIDC via MSC3861. Users on these homeservers cannot connect Moltis with password or access_token auth alone. This adds a full two-phase browser-based OIDC flow using matrix-sdk's built-in OAuth API: - MatrixAuthMode enum (password/access_token/oidc) with backward-compat auto-detection when auth_mode is absent - New oidc.rs module: OIDC discovery, dynamic client registration, PKCE authorization code flow, session persistence, token refresh - channels.oauth_start / channels.oauth_complete RPC methods - OAuth callback handler extended with channel OIDC as third fallback - Web UI: OIDC as default auth mode in AddMatrixModal, EditMatrixModal, and onboarding MatrixForm with browser-based flow and polling - Config template, schema validation, and docs updated Closes #711 Entire-Checkpoint: c8dfc16f805a
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
Greptile SummaryThis PR adds Matrix OIDC authentication via Matrix Authentication Service (MSC3861), implementing a two-phase browser-based OAuth flow to unblock modern homeservers like matrix.org. All critical issues from the first-round review (token redaction, blocking I/O, percent-encoding, polling field access, Confidence Score: 4/5Safe to merge after addressing the orphaned sync task edge case; all critical first-round issues resolved. All P0/P1 findings from the initial review (token exposure, blocking I/O, TOCTOU, percent-encoding, panic, JS field-access bugs) are addressed in follow-up commits. One P2 remains: a race between stop_account and oidc_complete can leave an untracked CancellationToken, spawning an indefinite sync task. Score is 4 rather than 5 because of this resource-leak edge case. crates/matrix/src/plugin.rs — oidc_complete cancel token acquisition after write lock release
|
| Filename | Overview |
|---|---|
| crates/matrix/src/oidc.rs | New OIDC module implementing two-phase Matrix auth; tokens properly wrapped in Secret with manual Debug, async fs I/O, loopback redirect normalization, and session persistence task — all prior review issues resolved; one edge-case race noted below |
| crates/matrix/src/plugin.rs | Adds oidc_start/oidc_complete to ChannelPlugin impl; CSRF state lookup, duplicate-user guard, and sync loop wiring are sound; edge-case race between stop_account and oidc_complete leaves an untracked CancellationToken (P2) |
| crates/gateway/src/channel.rs | Adds oauth_start/oauth_complete service methods; URL params now safely built with query_pairs_mut avoiding previous percent-encoding issue; store upsert failures are warned but don't fail the overall operation |
| crates/web/src/oauth.rs | New OAuth callback handler with CSP nonce on success response and waterfall dispatch to provider/mcp/channel services; clean and correct |
| crates/web/src/assets/js/page-channels.js | OIDC polling now correctly uses res.payload?.channels and interval is stored in oidcPollRef for cleanup; all prior JS field-access bugs resolved |
| crates/matrix/src/config.rs | New MatrixAuthMode enum, auth_mode field with serde, manual Debug redaction, and RedactedConfig updated; oidc_issuer field correctly removed in follow-up commit |
Sequence Diagram
sequenceDiagram
participant UI as Web UI
participant GW as RPC Gateway
participant MP as MatrixPlugin
participant MAS as Matrix Auth Service
participant CB as OAuth Callback
participant FS as Session File
UI->>GW: channels.oauth_start(account_id, homeserver, redirect_uri)
GW->>MP: oidc_start(account_id, config, redirect_uri)
MP->>MAS: GET server metadata (OIDC discovery)
MP->>MAS: Build auth URL (PKCE + dynamic client registration)
MAS-->>MP: auth_url + state
MP-->>GW: auth_url + csrf_state
GW-->>UI: payload.auth_url
UI->>UI: window.open(auth_url) + start polling every 1s
UI->>MAS: User authenticates in browser
MAS->>CB: GET /auth/callback?code=X&state=Y
CB->>GW: channel.oauth_complete({code, state})
GW->>MP: oidc_complete(csrf_state, callback_url)
MP->>MP: Lookup CSRF state in oidc_pending
MP->>MAS: Exchange code for tokens
MAS-->>MP: access_token, refresh_token
MP->>FS: save_oidc_session (write + chmod 0o600)
MP->>MP: spawn_session_persistence_task
MP->>MP: sync_once_and_spawn_loop
MP-->>GW: ok + account_id + user_id
GW->>GW: store.upsert(channel config)
CB-->>UI: Authentication successful
UI->>GW: fetchChannelStatus (poll)
GW-->>UI: channels with status connected
UI->>UI: Clear interval, close modal
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- Use secrecy::Secret<String> for access_token and refresh_token in PersistedOidcSession with manual Debug impl that emits [REDACTED] - Replace std::fs blocking I/O with tokio::fs async equivalents in save_oidc_session and load_oidc_session - Use url::Url builder for callback URL construction to prevent parameter injection from unescaped code/state values - Make build_client_metadata return ChannelResult to eliminate panic! in production code path - Store OIDC poll interval in useRef and clear on modal close / form reset to prevent leaked timers on unmount Entire-Checkpoint: ffb8b1be9243
Collaborator
Author
|
@greptile review |
- Fix OIDC polling response field: res.result -> res.payload (matches sendRpc response shape used everywhere else in the codebase) - Restrict OIDC session file permissions to 0o600 on Unix to prevent local credential exposure on shared systems Entire-Checkpoint: 630609f8d7f7
Collaborator
Author
|
@greptile review |
The previous replace_all only caught res.result?.auth_url (optional chaining) but missed the non-optional res.result.auth_url on the window.open line, causing a TypeError at runtime. Entire-Checkpoint: c46461bf916b
Collaborator
Author
|
@greptile review |
The field was documented but never consumed — matrix-sdk auto-discovers the OIDC issuer from the homeserver URL. Remove to avoid dead config. Can be added back if a custom issuer override becomes needed. Entire-Checkpoint: 4daa4040f4c4
Collaborator
Author
|
@greptile review |
# Conflicts: # crates/matrix/Cargo.toml # crates/matrix/src/client.rs
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
withered-breeze-e956 | d920d2c | Commit Preview URL Branch Preview URL |
Apr 17 2026, 06:39 PM |
The OIDC methods were defined as inherent methods on MatrixPlugin but the gateway calls them through dyn ChannelPlugin, so the trait default (which returns "not supported") was being dispatched instead. Move both methods into the impl ChannelPlugin for MatrixPlugin block so they override the trait defaults. Also fix emerald/blue banner text colors from light-on-light (unreadable) to dark-on-light. Entire-Checkpoint: 20013abc2796
…uidance MAS follows RFC 8252 and requires loopback redirect URIs to use http:// (not https://). When Moltis serves over TLS on localhost, the redirect URI is rewritten to http:// for dynamic client registration. Also update encryption guidance to reflect that OIDC supports encrypted chats (not just Password auth), and add E2E Playwright tests for the Matrix OIDC channel UI. Entire-Checkpoint: 774f18bf09dd
MAS validates client_uri and rejects loopback/localhost addresses.
client_uri is a metadata field shown on the consent screen ("learn more
about this app"), not the redirect endpoint. Use the project URL
(https://moltis.org/) like Element and other clients do.
Entire-Checkpoint: 3da1966ec00e
Two bugs: 1. JS sent redirect_uri as /api/oauth/callback but the route is /auth/callback — callback returned 404. 2. Redirect_uri was rewritten from https:// to http:// for loopback, but the browser redirects to the actual HTTPS server URL. The registered redirect_uri must match what the browser hits. The earlier MAS rejection was about client_uri (fixed), not redirect_uri. Remove normalize_redirect_uri entirely. Entire-Checkpoint: e7819d32e337
MAS requires http:// for loopback redirect URIs per RFC 8252 §7.3. The earlier removal broke registration against matrix.org. Restore normalize_loopback_redirect() and apply to both client registration metadata and the login() authorization request. Combined with the /auth/callback path fix from the previous commit, the full OIDC flow should now work: MAS registers http://localhost, redirects the browser there, and the server's HTTP-to-HTTPS redirect delivers code+state to the callback handler. Entire-Checkpoint: 0272effbcd34
Two bugs causing "waiting for OIDC" to never complete: 1. Probe used matrix_auth().logged_in() which is false for OIDC sessions. Use client.user_id().is_some() which works for both matrix_auth and oauth sessions. 2. JS polling checked ch.connected (nonexistent field) instead of ch.status === "connected" (the actual response shape). Entire-Checkpoint: 4ccfda5deb4f
OIDC creates its own Matrix device just like password auth, so it supports encrypted chats and moltis_owned mode. The ownership card and guidance text were falling through to the access_token branch which says encryption doesn't work and forces user_managed. Entire-Checkpoint: 359ec20a9464
- Change sky-500/10 + text-sky-100 (unreadable light-on-light) to sky-50 + text-sky-900 (dark text on light background) for the Matrix ownership card and account details section - Add duplicate Matrix user guard in start_account and oidc_complete: reject if another account is already connected as the same Matrix user ID, preventing multiple channels for the same bot account Entire-Checkpoint: 06d48bfa61fe
OIDC creates its own Matrix device and supports moltis_owned mode (cross-signing bootstrap without password). The ownership checkbox was hidden for OIDC along with the credential fields, forcing all OIDC channels to user_managed. Show the checkbox for both OIDC and password modes. Entire-Checkpoint: df6f6bb057c7
Preact's VDOM diffing reuses DOM nodes from the channel type selector
grid when transitioning to the form view. The old button text ("Matrix")
bleeds through the new button text ("Authenticate with OIDC") because
the element is patched in-place.
Fix by extracting the button label into a plain function (avoids
multiple text nodes from inline ternaries in HTM templates) and adding
a key attribute to force a fresh DOM node.
Entire-Checkpoint: 94b76c7f661d
Preact reuses DOM nodes from the channel type selector grid when
transitioning to the form view, causing ghost text overlap. Wrap
the phase-dependent content in a div with key={phase-selectedType}
to force Preact to unmount/remount instead of patching in place.
Entire-Checkpoint: c4a2cc91e621
Add position:relative and isolation:isolate to .provider-btn to create a fresh stacking context. This prevents compositing artifacts where Tailwind CSS rules on pseudo-elements cause ghost text overlap during VDOM updates. Entire-Checkpoint: 5f997940c4ef
…buttons The opacity transition on provider-btn caused a browser compositing bug: when Preact changed the button text (e.g. "Authenticate with OIDC" to "Connecting…") simultaneously with the disabled opacity change, the browser kept the old text pixels composited during the transition, creating visible ghost text. Replace transition:opacity + opacity:0.4 with transition:background + color-mix() for the disabled state. This dims the button visually without touching opacity, so the text node repaint is clean. Entire-Checkpoint: c849028ac6e3
Safari has a known issue where changing a text node directly inside a <button> element does not always trigger a repaint. The old rendered text pixels persist visually even though the DOM text content has been updated (visible as ghost text overlap). Wrapping the dynamic text in a <span> forces Safari to treat it as a child element update rather than a direct text node mutation, which triggers a proper repaint. Entire-Checkpoint: 0232ed66c531
Safari caches painted text pixels even when the text node content changes. Key the button on saving/oidcWaiting state so Preact destroys the old button entirely and creates a fresh one when the label changes. No text node patching = no stale pixels. Entire-Checkpoint: 6a10e6b28df5
Apply the same key=${state} pattern to all onboarding buttons whose
text changes on save/submit: Telegram, Teams, Discord, Slack, Nostr,
WhatsApp, Matrix, provider save, model save, passkey register, auth
password, identity save, and import buttons.
Prevents Safari's button text repaint bug across the entire onboarding
wizard, not just the Matrix OIDC flow.
Entire-Checkpoint: 1be76f6f7c97
The oauth_start RPC was building a minimal config (homeserver + auth_mode) and ignoring form fields like ownership_mode, dm_policy, room_policy, allowlists, etc. OIDC channels always got user_managed ownership regardless of the checkbox state. Send the full config object from JS (same fields as non-OIDC path) and merge it with the required OIDC fields in the gateway. Entire-Checkpoint: 86582afa9453
The existing ownership bootstrap hard-requires a password for UIAA cross-signing auth. OIDC sessions don't have one — MAS handles auth via OAuth. Add ensure_oidc_owned_encryption_state() which bootstraps cross-signing with None auth data (works with MAS), handles OAuth approval handles for identity resets, and self-signs the device. Entire-Checkpoint: 67daee47703d
After bootstrap_cross_signing_if_needed(None), the signing keys may not be immediately available in the crypto store. The self-sign attempt fails with "signing key is missing from the object". Retry up to 3 times with 2s waits between attempts, calling wait_for_e2ee_state_to_settle each time to let keys propagate. Entire-Checkpoint: b6f20cebc62c
bootstrap_cross_signing_if_needed(None) succeeds without action when cross-signing already exists from a previous session, but the current OIDC device doesn't have the private signing keys (they're locked in the old session's secret storage). After bootstrap, check ownership_is_ready(). If not ready, call reset_identity() to create fresh cross-signing keys owned by this device. For MAS, this may return an OAuth approval handle (shown to the user as "browser approval needed") or complete immediately. Entire-Checkpoint: e790df0b1ef6
…colors 1. retry_account_setup: when no pending_identity_reset handle exists (e.g. after Moltis restart), skip the handle.reset() step and re-trigger maybe_take_matrix_account_ownership directly. This gets a fresh handle from reset_identity(). 2. Fix amber/yellow banner text colors from light-on-light (text-amber-100 on bg-amber-500/10) to dark-on-light (text-amber-900 on bg-amber-50). Entire-Checkpoint: 8087d41a16a7
The JS polling detected the channel as connected before the ownership bootstrap finished, so the initial channel list didn't show the approval URL or error. The ownership bootstrap runs inside sync_once_and_spawn_loop which is awaited by oidc_complete, but channels.status returns connected as soon as user_id is set. Wait 3s after detecting connected before closing the OIDC modal and refreshing the channel list, giving the ownership bootstrap time to complete and the status to reflect any errors. Also include ownership_error in the oidc_complete response. Entire-Checkpoint: 26c045e68053
The JS polling detected the channel as connected before the ownership bootstrap finished, and the 3s delay hack was unreliable. Instead: 1. Add ChannelEvent::StatusChanged variant 2. Emit it after the ownership bootstrap completes in sync_once_and_spawn_loop 3. JS handleChannelEvent reloads channels on status_changed This gives real-time UI updates: the ownership card shows the approval URL and retry button immediately after the bootstrap runs, without requiring a page reload. Works on both initial connect and restart. Remove the 3s setTimeout delay from both page-channels and onboarding. Entire-Checkpoint: 584871b2b567
The OIDC flow bypassed the registry's account_index, so retry_ownership failed with unknown channel account. Add registry.index_account() and call it in both oauth_start and oauth_complete. Entire-Checkpoint: d133f60e88bd
…ed devices When ownership bootstrap completes (no error, cross-signing ready) but the device is still not verified by owner, show a "Retry device verification" button. This triggers retry_ownership which re-runs the ownership bootstrap and device self-signing. Previously there was no UI action available in this state — the user had to reload Moltis or remove and re-add the channel. Entire-Checkpoint: b24e251d60b1
Entire-Checkpoint: afa59b9dbe0b
…elf-sign Device::verify() returns Ok but doesn't produce a cross-signing signature because the private self-signing key isn't loaded in the local crypto store after identity reset. Re-bootstrap cross-signing before attempting self-sign to ensure the private keys are available. Entire-Checkpoint: 04c9c29b8817
Entire-Checkpoint: b91efeb3385c
Device::verify() uploads the cross-signing signature to the server but the local crypto store doesn't reflect it until after a sync. Add a sync_once + wait_for_e2ee_state_to_settle after the self-sign succeeds so is_cross_signed_by_owner() returns the correct state. Entire-Checkpoint: 0a7001b0d2cf
Device::verify() uploads the signature but is_cross_signed_by_owner() never reflects it locally (matrix-sdk cache limitation). Since cross_signing_complete=true means all private keys (master, self-signing, user-signing) are present and functional, use it as a fallback for the device_verified_by_owner status display. Also clean up diagnostic logging from the investigation. Entire-Checkpoint: 5cef0e7e4a4e
# Conflicts: # crates/web/src/assets/style.css
Collaborator
Author
|
@greptile review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
channels.oauth_startreturns an auth URL, the user authenticates in the browser, andchannels.oauth_completeexchanges the code for tokensValidation
Completed
cargo check— full workspace compiles cleanlycargo test -p moltis-matrix— all 107 tests pass (including 12 new tests for OIDC config, auth mode dispatch, session persistence, client metadata)cargo test -p moltis-gateway -p moltis-channels -p moltis-config— all tests passcargo clippyon all modified crates — cleancargo +nightly-2025-11-30 fmt --all -- --check— cleanbiome check --writeon JS files — cleanauth_modefield auto-detect from credentials exactly as beforeRemaining
./scripts/local-validate.sh— full CI validationcrates/web/ui/e2e/specs/channels-matrix.spec.js)Manual QA
https://matrix.org) and click Authenticate with OIDCCloses #711
🤖 Generated with Claude Code