diff --git a/docs/superpowers/plans/2026-04-23-oidc-login-implementation.md b/docs/superpowers/plans/2026-04-23-oidc-login-implementation.md new file mode 100644 index 00000000..a854d2ea --- /dev/null +++ b/docs/superpowers/plans/2026-04-23-oidc-login-implementation.md @@ -0,0 +1,487 @@ +# OIDC Login For MAS Servers Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Add MAS/OIDC login to `robrix2` so an existing account on `matrix.org`, `alvin.meldry.com`, or any MSC2965-capable homeserver can sign in through the system browser and return to the app via a loopback callback. + +**Architecture:** Reuse the existing homeserver capability probe and the existing login worker/persistence pipeline instead of building a parallel auth stack. The implementation has four seams: a shared homeserver capability model, auth-session persistence that supports both Matrix password sessions and OAuth sessions, an OIDC worker module under `src/login/`, and a LoginScreen state machine that branches between password login and MAS browser login. Logout and token-refresh persistence must be updated in the same patch so the new session type is survivable after restart. + +**Tech Stack:** Rust 2024, Makepad 2.0 `script_mod!`, `matrix-sdk` OAuth API on `project-robius/matrix-rust-sdk@space_room_suggested`, `matrix_sdk::utils::local_server`, `robius_open`, existing `SyncService` / `finalize_authenticated_client()` pipeline. + +--- + +## Constraints + +- Do **not** run `cargo fmt` or `rustfmt`. +- Do **not** add dependencies. +- Do **not** commit before user testing. +- Keep the existing password login and legacy SSO flows working. +- Keep MAS detection aligned with the current register flow: accept both stable `m.authentication` and unstable `org.matrix.msc2965.authentication`. + +## Task 1: Extract Shared Homeserver Capability State + +**Files:** +- Create: `src/homeserver.rs` +- Modify: `src/lib.rs` +- Modify: `src/register/mod.rs` +- Modify: `src/register/register_screen.rs` +- Modify: `src/sliding_sync.rs` +- Test: `src/homeserver.rs` + +**Step 1: Write the failing tests** + +Add tests in `src/homeserver.rs` for the shared classifier surface before adding production code: + +```rust +#[test] +fn login_mode_prefers_mas_oidc_when_mas_is_advertised() { + let caps = HsCapabilities { + base_url: "https://matrix.org".into(), + is_mas_native_oidc: true, + registration_enabled: false, + uiaa_probe: None, + sso_providers: Vec::new(), + mas_signup_url: Some("https://issuer/register".into()), + mas_issuer_url: Some("https://issuer".into()), + }; + assert_eq!(login_mode(&caps), LoginMode::MasOidc); +} + +#[test] +fn login_mode_falls_back_to_password_for_non_mas_servers() { + let caps = HsCapabilities { + base_url: "https://palpo.example".into(), + is_mas_native_oidc: false, + registration_enabled: true, + uiaa_probe: None, + sso_providers: Vec::new(), + mas_signup_url: None, + mas_issuer_url: None, + }; + assert_eq!(login_mode(&caps), LoginMode::Password); +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cargo test login_mode_ --lib` +Expected: FAIL because `src/homeserver.rs`, `LoginMode`, and `login_mode()` do not exist yet. + +**Step 3: Write minimal implementation** + +Create `src/homeserver.rs` and move the shared capability structs there: + +```rust +#[derive(Clone, Debug)] +pub struct HsCapabilities { + pub base_url: String, + pub is_mas_native_oidc: bool, + pub registration_enabled: bool, + pub uiaa_probe: Option, + pub sso_providers: Vec, + pub mas_signup_url: Option, + pub mas_issuer_url: Option, +} + +#[derive(Clone, Debug)] +pub enum CapabilityProbeAction { + Discovered { requested_url: String, caps: Box }, + Failed { requested_url: String, error: String }, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LoginMode { + MasOidc, + Password, +} + +pub fn login_mode(caps: &HsCapabilities) -> LoginMode { + if caps.is_mas_native_oidc { + LoginMode::MasOidc + } else { + LoginMode::Password + } +} +``` + +Update `src/register/mod.rs` to import `HsCapabilities` from `crate::homeserver`, keep `RegisterMode`, and remove `CapabilitiesDiscovered` / `DiscoveryFailed` from `RegisterAction`. + +Update `src/sliding_sync.rs` to: + +```rust +Cx::post_action(crate::homeserver::CapabilityProbeAction::Discovered { + requested_url, + caps: Box::new(caps), +}); +``` + +Also capture `mas_issuer_url` next to the existing `mas_signup_url` in `discover_homeserver_capabilities()`: + +```rust +let (mas, mas_signup_url, mas_issuer_url) = + ["m.authentication", "org.matrix.msc2965.authentication"] + .iter() + .find_map(|key| { + let issuer = body.get(*key)?.get("issuer").and_then(|v| v.as_str())?; + let issuer = issuer.trim_end_matches('/').to_string(); + Some(( + true, + Some(format!("{issuer}/register")), + Some(issuer), + )) + }) + .unwrap_or((false, None, None)); +``` + +Update `src/register/register_screen.rs` to react to `CapabilityProbeAction` instead of `RegisterAction::CapabilitiesDiscovered` / `DiscoveryFailed`. + +**Step 4: Run test to verify it passes** + +Run: `cargo test login_mode_ --lib` +Expected: PASS + +**Step 5: Run narrow compile verification** + +Run: `cargo build` +Expected: PASS + +## Task 2: Generalize Persisted Sessions For Matrix And OAuth + +**Files:** +- Modify: `src/persistence/matrix_state.rs` +- Modify: `src/sliding_sync.rs` +- Modify: `src/logout/logout_state_machine.rs` +- Test: `src/persistence/matrix_state.rs` + +**Step 1: Write the failing tests** + +Add a round-trip serialization test in `src/persistence/matrix_state.rs`: + +```rust +#[test] +fn persisted_auth_session_round_trips_oauth_variant() { + let persisted = PersistedAuthSession::OAuth { + client_id: "client-id".into(), + user_session: matrix_sdk::authentication::oauth::UserSession { + meta: SessionMeta { + user_id: user_id!("@alice:example.org").to_owned(), + device_id: device_id!("DEVICEID").to_owned(), + }, + tokens: SessionTokens { + access_token: "access".into(), + refresh_token: Some("refresh".into()), + }, + }, + }; + + let json = serde_json::to_string(&persisted).unwrap(); + let restored: PersistedAuthSession = serde_json::from_str(&json).unwrap(); + assert_eq!(restored.user_id().as_str(), "@alice:example.org"); +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cargo test persisted_auth_session_round_trips_oauth_variant --lib` +Expected: FAIL because `PersistedAuthSession` does not exist. + +**Step 3: Write minimal implementation** + +Replace `FullSessionPersisted.user_session: MatrixSession` with a new serializable enum: + +```rust +#[derive(Debug, Serialize, Deserialize)] +pub enum PersistedAuthSession { + Matrix(MatrixSession), + OAuth { + client_id: String, + user_session: matrix_sdk::authentication::oauth::UserSession, + }, +} +``` + +Expose helpers: + +```rust +impl PersistedAuthSession { + fn user_id(&self) -> &UserId { ... } + fn into_auth_session(self) -> matrix_sdk::authentication::AuthSession { ... } +} +``` + +Update `save_session()` to use `client.session()` instead of `client.matrix_auth().session()`: + +```rust +let auth_session = match client.session().ok_or_else(|| anyhow!("..."))? { + AuthSession::Matrix(session) => PersistedAuthSession::Matrix(session), + AuthSession::OAuth(session) => PersistedAuthSession::OAuth { + client_id: session.client_id.to_string(), + user_session: session.user, + }, +}; +``` + +Update `restore_session()` to match the persisted enum and call `client.restore_session(...)`. + +Update `handle_session_changes()` in `src/sliding_sync.rs` so `SessionChange::TokensRefreshed` re-saves the session: + +```rust +Ok(SessionChange::TokensRefreshed) => { + if let Err(e) = persistence::save_session(&client, client_session.clone()).await { + warning!("Failed to persist refreshed session tokens: {e}"); + } +} +``` + +Update `perform_server_logout()` in `src/logout/logout_state_machine.rs` to branch on `client.auth_api()`: + +```rust +match client.auth_api() { + Some(AuthApi::Matrix(_)) => client.matrix_auth().logout().await, + Some(AuthApi::OAuth(_)) => client.oauth().logout().await, + None => Ok(()), +} +``` + +**Step 4: Run test to verify it passes** + +Run: `cargo test persisted_auth_session_round_trips_oauth_variant --lib` +Expected: PASS + +**Step 5: Run focused regression checks** + +Run: `cargo test login_failure_modal_is_suppressed_while_register_flow_is_active --lib` +Expected: PASS + +Run: `cargo build` +Expected: PASS + +## Task 3: Add OIDC Worker Flow And Cancellation + +**Files:** +- Modify: `src/login/mod.rs` +- Create: `src/login/oidc_login.rs` +- Modify: `src/sliding_sync.rs` +- Test: `src/login/oidc_login.rs` + +**Step 1: Write the failing tests** + +Add pure error-mapping tests in `src/login/oidc_login.rs`: + +```rust +#[test] +fn map_oidc_error_handles_cancelled_login() { + let msg = map_oidc_error(&OidcLoginError::Cancelled); + assert!(msg.contains("cancel")); +} + +#[test] +fn map_oidc_error_handles_missing_dynamic_registration() { + let msg = map_oidc_error(&OidcLoginError::DynamicRegistrationNotSupported); + assert!(msg.contains("third-party sign-in apps")); +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cargo test map_oidc_error_ --lib` +Expected: FAIL because `src/login/oidc_login.rs` and `OidcLoginError` do not exist. + +**Step 3: Write minimal implementation** + +Add `src/login/oidc_login.rs` with a worker-facing function: + +```rust +pub async fn start_oidc_login( + homeserver_url: String, + proxy: Option, +) -> anyhow::Result<(Client, ClientSessionPersisted, String)> { ... } +``` + +Implementation requirements: + +- Build the client with the existing `build_client()` helper and the provided homeserver/proxy. +- Spawn `LocalServerBuilder::new().spawn().await?` and keep both the redirect URI and shutdown handle. +- Build OAuth authorization with: + +```rust +let OAuthAuthorizationData { url, state } = client + .oauth() + .login( + redirect_uri, + None, + Some(oidc_client_registration_data().into()), + None, + ) + .build() + .await?; +``` + +- Open the returned URL with `robius_open::Uri::new(url.as_str()).open()`. +- Wait with `tokio::select!` across: + - loopback callback + - explicit cancel signal + - timeout +- On cancel or timeout, shut down the local server and call `client.oauth().abort_login(&state).await`. +- On success, call `client.oauth().finish_login(query.into()).await?`. +- Return `(client, client_session, fallback_user_id)` where the fallback user ID comes from `client.user_id()` after `finish_login()`. + +Add a new `MatrixRequest`: + +```rust +StartOidcLogin { + homeserver_url: String, + proxy: Option, +} +``` + +and a cancel request: + +```rust +CancelOidcLogin +``` + +The worker should translate outcomes into `LoginAction`: + +- `OidcLoginStarted` +- `OidcLoginCancelled` +- `OidcLoginFailed(String)` +- existing `LoginAction::Status` +- existing `LoginAction::LoginSuccess` through `finalize_authenticated_client()` + +**Step 4: Run test to verify it passes** + +Run: `cargo test map_oidc_error_ --lib` +Expected: PASS + +**Step 5: Run compile verification** + +Run: `cargo build` +Expected: PASS + +## Task 4: Add LoginScreen MAS/OIDC Branching + +**Files:** +- Modify: `src/login/login_screen.rs` +- Modify: `resources/i18n/en.json` +- Modify: `resources/i18n/zh-CN.json` +- Test: `src/login/login_screen.rs` + +**Step 1: Write the failing tests** + +Add pure helper tests near the existing login screen tests: + +```rust +#[test] +fn capability_probe_is_required_when_login_mode_is_unknown() { + assert!(should_probe_homeserver(None, false)); +} + +#[test] +fn capability_probe_is_not_required_while_oidc_login_is_in_flight() { + assert!(!should_probe_homeserver(Some(LoginMode::MasOidc), true)); +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cargo test capability_probe_is_required_when_login_mode_is_unknown --lib` +Expected: FAIL because `should_probe_homeserver()` and `LoginMode` are not wired into the login screen. + +**Step 3: Write minimal implementation** + +Extend `LoginScreen` state with: + +```rust +#[rust] login_mode: Option, +#[rust] discovery_pending: bool, +#[rust] last_discovery_input_url: Option, +#[rust] oidc_in_flight: bool, +``` + +Add a hidden MAS card to the DSL, for example: + +```rust +oidc_card := View { + visible: false + width: 275, height: Fit, + flow: Down, + spacing: 8 + + oidc_info_label := Label { text: "..." } + oidc_continue_button := RobrixIconButton { text: "Continue in browser" } +} +``` + +Behavior: + +- When homeserver input changes, clear `login_mode`, hide the OIDC card, reset CTA state. +- If mode is unknown and the user clicks the primary login button, run `MatrixRequest::DiscoverHomeserverCapabilities`. +- On `CapabilityProbeAction::Discovered`: + - `LoginMode::MasOidc`: show the OIDC card, hide password submit affordance, keep SSO buttons hidden. + - `LoginMode::Password`: keep existing username/password flow active. +- On `oidc_continue_button` click: + - validate proxy settings + - dispatch `MatrixRequest::StartOidcLogin { homeserver_url, proxy }` + - set `oidc_in_flight = true` +- On cancel button while OIDC is active: + - dispatch `MatrixRequest::CancelOidcLogin` +- On `LoginAction::OidcLoginCancelled` / `OidcLoginFailed`: + - clear `oidc_in_flight` + - keep the MAS card visible for retry + +Add i18n strings for: + +- `login.button.next` +- `login.button.continue_in_browser` +- `login.oidc.mas_info` +- `login.oidc.waiting` +- `login.status.checking_homeserver.title` +- `login.status.checking_homeserver.body` + +**Step 4: Run test to verify it passes** + +Run: `cargo test capability_probe_is_required_when_login_mode_is_unknown --lib` +Expected: PASS + +**Step 5: Run runtime parse verification** + +Run: `cargo run` +Expected: App starts without Makepad script errors; no `login_screen.rs` type/scope parse failures in the first screen. + +## Task 5: End-To-End Verification + +**Files:** +- Modify as needed from previous tasks only + +**Step 1: Run unit tests** + +Run: `cargo test login_mode_ persisted_auth_session_round_trips_oauth_variant map_oidc_error_ capability_probe_is_required_when_login_mode_is_unknown --lib` +Expected: PASS + +**Step 2: Run full build** + +Run: `cargo build` +Expected: PASS + +**Step 3: Run manual smoke verification** + +Run: `cargo run` + +Manual checks: + +1. Enter `http://127.0.0.1:8128` or another non-MAS homeserver, confirm password login still uses the current path. +2. Enter `https://matrix.org` or `https://alvin.meldry.com`, confirm the MAS card appears and `Continue in browser` opens the system browser. +3. Cancel the browser flow before callback, confirm the UI returns to retryable idle state. +4. Complete OIDC login on a MAS server, confirm the app transitions into the main UI. +5. Relaunch the app, confirm the restored session skips the login screen. +6. Logout from an OIDC session, confirm the app returns cleanly to the login screen. + +**Step 4: Report for user testing** + +Do not commit. Present: + +- the plan file path +- the files changed +- build/test evidence +- the exact manual flows still needing user validation on target MAS servers diff --git a/resources/i18n/en.json b/resources/i18n/en.json index 61a65ab7..784f7b9f 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -72,6 +72,14 @@ "login.proxy_settings.error.invalid_port": "Proxy port must be a number between 1 and 65535.", "login.button.login": "Login", "login.button.create_account": "Create account", + "login.button.continue_in_browser": "Continue in browser", + "login.button.cancel_oidc": "Cancel sign-in", + "login.oidc.info_title": "Browser sign-in required", + "login.oidc.info_body": "This server uses a secure web sign-in. We'll open your browser — after you log in there, this app finishes automatically.", + "login.oidc.waiting_body": "Finish signing in on your browser, then return to Robrix.", + "login.oidc.cancelled": "Sign-in was cancelled. Click Login again to retry.", + "login.status.checking_homeserver.title": "Checking homeserver...", + "login.status.checking_homeserver.body": "Detecting how this server handles sign-in.", "login.sso.prompt": "Or, login with an SSO provider:", "login.account_prompt.no_account": "Don't have an account?", "login.account_prompt.already_have": "Already have an account?", diff --git a/resources/i18n/zh-CN.json b/resources/i18n/zh-CN.json index be3dea67..a554bcb9 100644 --- a/resources/i18n/zh-CN.json +++ b/resources/i18n/zh-CN.json @@ -72,6 +72,14 @@ "login.proxy_settings.error.invalid_port": "代理端口必须是 1 到 65535 之间的数字。", "login.button.login": "登录", "login.button.create_account": "创建账号", + "login.button.continue_in_browser": "在浏览器中继续", + "login.button.cancel_oidc": "取消登录", + "login.oidc.info_title": "需要浏览器登录", + "login.oidc.info_body": "该服务器使用网页安全登录。我们会打开系统浏览器——你在浏览器里完成登录后，本应用会自动继续。", + "login.oidc.waiting_body": "请在浏览器中完成登录，然后回到 Robrix。", + "login.oidc.cancelled": "登录已取消。再次点击登录可重试。", + "login.status.checking_homeserver.title": "正在检查服务器...", + "login.status.checking_homeserver.body": "正在识别该服务器的登录方式。", "login.sso.prompt": "或者，使用 SSO 提供商登录：", "login.account_prompt.no_account": "还没有账号？", "login.account_prompt.already_have": "已经有账号了？", diff --git a/src/homeserver.rs b/src/homeserver.rs new file mode 100644 index 00000000..bfb9ef51 --- /dev/null +++ b/src/homeserver.rs @@ -0,0 +1,67 @@ +//! Shared homeserver capability discovery state used by login and register. + +use makepad_widgets::ActionDefaultRef; +use matrix_sdk::ruma::api::client::uiaa::UiaaInfo; + +/// Homeserver capabilities discovered before branching into auth/register flows. +#[derive(Clone, Debug)] +pub struct HsCapabilities { + /// Normalized base URL the client will use. + pub base_url: String, + /// True iff the server advertises `m.authentication.issuer` in + /// `.well-known/matrix/client` (MSC2965 / MAS delegation). + pub is_mas_native_oidc: bool, + /// True iff `POST /_matrix/client/v3/register` with empty body returns + /// 401 with parseable UIAA flows (NOT 403 M_FORBIDDEN). + pub registration_enabled: bool, + /// Optional UIAA probe result (empty when server requires MAS). + pub uiaa_probe: Option, + /// Identity providers harvested from `/_matrix/client/v3/login`. + pub sso_providers: Vec, + /// URL to open in the system browser for MAS self-registration. + pub mas_signup_url: Option, + /// OAuth issuer discovered from `.well-known`, used by MAS login. + pub mas_issuer_url: Option, +} + +/// Minimal info per identity provider. Full matrix-sdk type is not +/// used because we only need name + id at this phase. +#[derive(Clone, Debug)] +pub struct IdentityProviderSummary { + pub id: String, + pub name: String, + pub icon_url: Option, +} + +/// Shared capability probe result action. +#[derive(Clone, Debug, Default)] +pub enum CapabilityProbeAction { + /// `requested_url` is echoed so screens can drop out-of-order responses. + Discovered { requested_url: String, caps: Box }, + /// `requested_url` is echoed so screens can drop out-of-order responses. + Failed { requested_url: String, error: String }, + #[default] + None, +} + +impl ActionDefaultRef for CapabilityProbeAction { + fn default_ref() -> &'static Self { + static DEFAULT: CapabilityProbeAction = CapabilityProbeAction::None; + &DEFAULT + } +} + +/// Outcome classification of capability discovery for login. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LoginMode { + MasOidc, + Password, +} + +pub fn login_mode(caps: &HsCapabilities) -> LoginMode { + if caps.is_mas_native_oidc { + LoginMode::MasOidc + } else { + LoginMode::Password + } +} diff --git a/src/lib.rs b/src/lib.rs index 497b7ebe..067925b4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,6 +47,8 @@ pub mod i18n; /// Login screen pub mod login; +/// Shared homeserver capability discovery state. +pub mod homeserver; /// Account registration flow pub mod register; /// Logout confirmation and state management diff --git a/src/login/login_screen.rs b/src/login/login_screen.rs index c6ba3237..1bb05c63 100644 --- a/src/login/login_screen.rs +++ b/src/login/login_screen.rs @@ -3,8 +3,8 @@ use std::ops::Not; use makepad_widgets::*; use url::Url; -use crate::{app::AppState, i18n::{AppLanguage, tr_fmt, tr_key}, sliding_sync::{submit_async_request, AccountSwitchAction, LoginByPassword, LoginRequest, MatrixRequest}}; -use crate::register::RegisterAction; +use crate::{app::AppState, homeserver::{login_mode, CapabilityProbeAction, LoginMode}, i18n::{AppLanguage, tr_fmt, tr_key}, sliding_sync::{submit_async_request, AccountSwitchAction, LoginByPassword, LoginRequest, MatrixRequest}}; +use crate::register::{validation::normalize_homeserver_url, RegisterAction}; use super::login_status_modal::{LoginStatusModalAction, LoginStatusModalWidgetExt}; @@ -16,6 +16,17 @@ fn should_show_login_failure_modal( !suppress_login_failure_modal && last_failure_message_shown != Some(error) } +/// Whether the login_button click should trigger a homeserver capability +/// probe before attempting to log in. +/// +/// Pure predicate so the decision can be unit-tested without driving a +/// LoginScreen instance: we probe whenever we haven't yet classified this +/// homeserver into Password vs MasOidc, and no OIDC flow is already in +/// flight (re-probing mid-OAuth would clobber the session we're building). +fn should_probe_homeserver(login_mode: Option, oidc_in_flight: bool) -> bool { + login_mode.is_none() && !oidc_in_flight +} + script_mod! { use mod.prelude.widgets.* use mod.widgets.* @@ -218,6 +229,64 @@ script_mod! { text: "Login" } + // MAS (OIDC) login branch. Hidden by default; the Rust + // side flips visibility on CapabilityProbeAction::Discovered + // when login_mode resolves to MasOidc. Mirrors the register + // screen's "browser sign-in" affordance so the two entry + // points feel consistent. + oidc_card := View { + visible: false + width: 275, height: Fit, + flow: Down, + spacing: 8, + margin: Inset{top: 5, bottom: 10} + + oidc_info_title := Label { + width: Fill, height: Fit + draw_text +: { + color: (COLOR_TEXT) + text_style: TITLE_TEXT {font_size: 11.0} + } + text: "Browser sign-in required" + } + + oidc_info_body := Label { + width: Fill, height: Fit + draw_text +: { + color: #8C8C8C + text_style: REGULAR_TEXT {font_size: 10.0} + } + text: "" + } + + oidc_continue_button := RobrixIconButton { + width: 275, + height: 40 + padding: 10 + align: Align{x: 0.5, y: 0.5} + text: "Continue in browser" + } + + oidc_status_label := Label { + visible: false + width: Fill, height: Fit + draw_text +: { + color: (COLOR_TEXT) + text_style: REGULAR_TEXT {font_size: 10.0} + } + text: "" + } + + oidc_cancel_button := RobrixIconButton { + visible: false + width: 275, + height: 40 + padding: 10 + align: Align{x: 0.5, y: 0.5} + text: "Cancel sign-in" + } + } + LineH { width: 275 margin: Inset{bottom: -5} @@ -586,6 +655,20 @@ pub struct LoginScreen { /// Boolean to indicate if we're in "add account" mode (adding another Matrix account). #[rust] adding_account: bool, #[rust] use_proxy_enabled: bool, + /// Classified login flavor for the current homeserver input, once a + /// capability probe has completed. None while unresolved or after the + /// user edits the homeserver field. + #[rust] login_mode: Option, + /// Normalized URL we last dispatched a probe for. Used to (a) drop + /// out-of-order probe responses from superseded clicks, and (b) detect + /// when the user has edited the homeserver field since the last probe. + #[rust] last_discovery_input_url: Option, + /// True between probe-dispatch and probe-result. Blocks duplicate probes + /// from rapid clicking and keeps the button's "Checking..." label honest. + #[rust] discovery_pending: bool, + /// True while the OIDC browser flow is in flight. Blocks re-probes and + /// re-entry into start_oidc_login from duplicate clicks. + #[rust] oidc_in_flight: bool, } impl LoginScreen { @@ -786,6 +869,54 @@ impl LoginScreen { Ok(Some(proxy_url)) } + fn clear_homeserver_classification(&mut self) { + self.login_mode = None; + self.last_discovery_input_url = None; + self.discovery_pending = false; + } + + fn show_password_login_branch(&mut self, cx: &mut Cx) { + self.view.view(cx, ids!(oidc_card)).set_visible(cx, false); + self.view.text_input(cx, ids!(user_id_input)).set_visible(cx, true); + self.view.text_input(cx, ids!(password_input)).set_visible(cx, true); + self.view.button(cx, ids!(login_button)).set_visible(cx, true); + self.view.view(cx, ids!(sso_view)).set_visible(cx, true); + self.view.label(cx, ids!(sso_prompt_label)).set_visible(cx, true); + self.view.label(cx, ids!(oidc_info_title)) + .set_text(cx, tr_key(self.app_language, "login.oidc.info_title")); + self.view.label(cx, ids!(oidc_info_body)) + .set_text(cx, tr_key(self.app_language, "login.oidc.info_body")); + self.view.button(cx, ids!(oidc_continue_button)) + .set_text(cx, tr_key(self.app_language, "login.button.continue_in_browser")); + self.view.button(cx, ids!(oidc_continue_button)).set_visible(cx, true); + self.view.label(cx, ids!(oidc_status_label)).set_visible(cx, false); + self.view.button(cx, ids!(oidc_cancel_button)).set_visible(cx, false); + } + + fn show_oidc_login_branch(&mut self, cx: &mut Cx) { + self.view.button(cx, ids!(login_button)).set_visible(cx, false); + self.view.text_input(cx, ids!(user_id_input)).set_visible(cx, false); + self.view.text_input(cx, ids!(password_input)).set_visible(cx, false); + self.view.view(cx, ids!(sso_view)).set_visible(cx, false); + self.view.label(cx, ids!(sso_prompt_label)).set_visible(cx, false); + self.view.label(cx, ids!(oidc_info_title)) + .set_text(cx, tr_key(self.app_language, "login.oidc.info_title")); + self.view.label(cx, ids!(oidc_info_body)) + .set_text(cx, tr_key(self.app_language, "login.oidc.info_body")); + self.view.button(cx, ids!(oidc_continue_button)) + .set_text(cx, tr_key(self.app_language, "login.button.continue_in_browser")); + self.view.button(cx, ids!(oidc_continue_button)).set_visible(cx, true); + self.view.label(cx, ids!(oidc_status_label)).set_visible(cx, false); + self.view.button(cx, ids!(oidc_cancel_button)).set_visible(cx, false); + self.view.view(cx, ids!(oidc_card)).set_visible(cx, true); + } + + fn reset_oidc_screen_state(&mut self, cx: &mut Cx) { + self.oidc_in_flight = false; + self.clear_homeserver_classification(); + self.show_password_login_branch(cx); + } + } impl ScriptHook for LoginScreen { @@ -853,6 +984,14 @@ impl WidgetMatchEvent for LoginScreen { self.set_use_proxy_enabled(cx, enabled); } + if homeserver_input.changed(actions).is_some() { + self.clear_homeserver_classification(); + if !self.oidc_in_flight { + self.show_password_login_branch(cx); + self.redraw(cx); + } + } + if self.view.button(cx, ids!(proxy_settings_save_button)).clicked(actions) { match self.build_proxy_url_from_form(cx) { Ok(proxy_url) => { @@ -891,10 +1030,10 @@ impl WidgetMatchEvent for LoginScreen { if cancel_button.clicked(actions) { self.adding_account = false; self.reset_sso_state(cx); + self.reset_oidc_screen_state(cx); // Reset the UI back to normal login mode self.view.label(cx, ids!(title)).set_text(cx, tr_key(self.app_language, "login.title.login_to_robrix")); cancel_button.set_visible(cx, false); - self.view.view(cx, ids!(sso_view)).set_visible(cx, true); mode_toggle_button.set_visible(cx, true); cx.action(LoginAction::CancelAddAccount); self.redraw(cx); @@ -926,6 +1065,65 @@ impl WidgetMatchEvent for LoginScreen { let user_id = user_id_input.text().trim().to_owned(); let password = password_input.text(); let homeserver = homeserver_input.text().trim().to_owned(); + + // Defensive backstop for cases where the homeserver field was + // updated programmatically rather than through a Changed action. + // Compare normalized URLs so `matrix.org` and + // `https://matrix.org` count as the same probe target. + let normalized_homeserver = homeserver + .is_empty() + .not() + .then(|| normalize_homeserver_url(&homeserver).ok()) + .flatten(); + if self.last_discovery_input_url.as_deref() != normalized_homeserver.as_deref() { + self.clear_homeserver_classification(); + } + + // Defensive guard: in MAS mode the login_button should be hidden + // and Continue-in-browser is the active CTA. If a stale click + // reaches here, drop it rather than submit password-auth to a + // server that rejects it. + if matches!(self.login_mode, Some(LoginMode::MasOidc)) { + return; + } + + // If the user typed a homeserver we haven't classified yet, run a + // capability probe before deciding between password and OIDC paths. + // An empty input means "use the default (matrix-client.matrix.org)" — + // preserve the existing zero-latency password path there rather than + // adding a probe round-trip. + if !homeserver.is_empty() + && !self.discovery_pending + && should_probe_homeserver(self.login_mode, self.oidc_in_flight) + { + if let Ok(normalized) = normalize_homeserver_url(&homeserver) { + self.discovery_pending = true; + self.last_discovery_input_url = Some(normalized.clone()); + self.view.button(cx, ids!(login_button)).set_text( + cx, + tr_key(self.app_language, "login.status.checking_homeserver.title"), + ); + login_status_modal_inner.set_title( + cx, + tr_key(self.app_language, "login.status.checking_homeserver.title"), + ); + login_status_modal_inner.set_status( + cx, + tr_key(self.app_language, "login.status.checking_homeserver.body"), + ); + login_status_modal_inner.button_ref(cx) + .set_text(cx, tr_key(self.app_language, "login.status.cancel")); + login_status_modal.open(cx); + submit_async_request(MatrixRequest::DiscoverHomeserverCapabilities { + url: normalized, + }); + self.redraw(cx); + return; + } + // normalize failed: fall through so the existing password path + // surfaces a usable error to the user. + } + if user_id.is_empty() { login_status_modal_inner.set_title(cx, tr_key(self.app_language, "login.status.missing_user_id.title")); login_status_modal_inner.set_status(cx, tr_key(self.app_language, "login.status.missing_user_id.body")); @@ -970,7 +1168,54 @@ impl WidgetMatchEvent for LoginScreen { login_status_modal.open(cx); self.redraw(cx); } - + + // "Continue in browser" click — only reachable when login_mode resolved + // to MasOidc (otherwise the card is hidden). Kick off the worker's + // OAuth flow via StartOidcLogin; oidc_in_flight will flip on when the + // worker posts LoginAction::OidcLoginStarted. + if self.view.button(cx, ids!(oidc_continue_button)).clicked(actions) + && !self.oidc_in_flight + { + if !matches!(self.login_mode, Some(LoginMode::MasOidc)) { + return; + } + let homeserver = homeserver_input.text().trim().to_owned(); + match self.build_proxy_url_from_form(cx) { + Ok(proxy) => { + if let Err(e) = crate::proxy_config::save_proxy_url(proxy.as_deref()) { + warning!("Failed to persist proxy configuration from login screen: {e}"); + } + submit_async_request(MatrixRequest::StartOidcLogin { + homeserver_url: homeserver, + proxy, + is_add_account: self.adding_account, + }); + self.redraw(cx); + } + Err(proxy_validation_error) => { + login_status_modal_inner.set_title( + cx, + tr_key(self.app_language, "login.status.invalid_proxy.title"), + ); + let error_text = tr_fmt(self.app_language, "login.status.invalid_proxy.body", &[ + ("error", proxy_validation_error.as_str()), + ]); + login_status_modal_inner.set_status(cx, &error_text); + login_status_modal_inner.button_ref(cx) + .set_text(cx, tr_key(self.app_language, "login.status.okay")); + login_status_modal.open(cx); + self.redraw(cx); + } + } + } + + // Cancel an in-flight OIDC flow. Worker drops the cancel branch of + // its tokio::select!, calls abort_login, and posts OidcLoginCancelled + // which we handle below to restore the oidc_card to ready state. + if self.view.button(cx, ids!(oidc_cancel_button)).clicked(actions) { + submit_async_request(MatrixRequest::CancelOidcLogin); + } + let provider_brands = ["apple", "facebook", "github", "gitlab", "google", "twitter"]; let button_set: &[&[LiveId]] = ids_array!( apple_button, @@ -988,15 +1233,67 @@ impl WidgetMatchEvent for LoginScreen { if let Some(RegisterAction::NavigateToLogin) = action.downcast_ref() { self.suppress_login_failure_modal = false; self.last_failure_message_shown = None; + self.reset_oidc_screen_state(cx); login_status_modal.close(cx); } + // Capability probe result for the homeserver input. We share this + // action with RegisterScreen via crate::homeserver; filter on + // requested_url so a probe fired from Register doesn't drive us + // and vice versa. + match action.downcast_ref::() { + Some(CapabilityProbeAction::Discovered { requested_url, caps }) => { + if self.last_discovery_input_url.as_deref() != Some(requested_url.as_str()) { + continue; + } + self.discovery_pending = false; + self.view.button(cx, ids!(login_button)) + .set_text(cx, tr_key(self.app_language, "login.button.login")); + let resolved = login_mode(caps.as_ref()); + self.login_mode = Some(resolved); + match resolved { + LoginMode::MasOidc => { + self.show_oidc_login_branch(cx); + login_status_modal.close(cx); + } + LoginMode::Password => { + self.show_password_login_branch(cx); + login_status_modal.close(cx); + } + } + self.redraw(cx); + continue; + } + Some(CapabilityProbeAction::Failed { requested_url, error }) => { + if self.last_discovery_input_url.as_deref() != Some(requested_url.as_str()) { + continue; + } + self.clear_homeserver_classification(); + self.show_password_login_branch(cx); + self.view.button(cx, ids!(login_button)) + .set_text(cx, tr_key(self.app_language, "login.button.login")); + login_status_modal_inner.set_title( + cx, + tr_key(self.app_language, "login.status.login_failed"), + ); + login_status_modal_inner.set_status(cx, error); + login_status_modal_inner.button_ref(cx) + .set_text(cx, tr_key(self.app_language, "login.status.okay")); + login_status_modal.open(cx); + self.redraw(cx); + continue; + } + Some(CapabilityProbeAction::None) | None => {} + } + // Handle login-related actions received from background async tasks. match action.downcast_ref() { Some(LoginAction::ShowLoginScreen) => { self.suppress_login_failure_modal = false; self.last_failure_message_shown = None; + self.reset_oidc_screen_state(cx); login_status_modal.close(cx); + self.redraw(cx); } Some(LoginAction::CliAutoLogin { user_id, homeserver }) => { self.last_failure_message_shown = None; @@ -1031,6 +1328,7 @@ impl WidgetMatchEvent for LoginScreen { self.suppress_login_failure_modal = false; self.last_failure_message_shown = None; self.adding_account = false; + self.reset_oidc_screen_state(cx); user_id_input.set_text(cx, ""); password_input.set_text(cx, ""); homeserver_input.set_text(cx, ""); @@ -1074,6 +1372,7 @@ impl WidgetMatchEvent for LoginScreen { self.suppress_login_failure_modal = false; self.adding_account = true; self.reset_sso_state(cx); + self.reset_oidc_screen_state(cx); // Update UI to "add account" mode self.view.label(cx, ids!(title)).set_text(cx, tr_key(self.app_language, "settings.account.button.add_another_account")); cancel_button.set_visible(cx, true); @@ -1086,6 +1385,7 @@ impl WidgetMatchEvent for LoginScreen { self.suppress_login_failure_modal = false; self.adding_account = false; self.reset_sso_state(cx); + self.reset_oidc_screen_state(cx); user_id_input.set_text(cx, ""); password_input.set_text(cx, ""); homeserver_input.set_text(cx, ""); @@ -1096,6 +1396,46 @@ impl WidgetMatchEvent for LoginScreen { login_status_modal.close(cx); self.redraw(cx); } + Some(LoginAction::OidcLoginStarted) => { + // Worker has launched the browser; flip the oidc_card to + // its waiting state and expose Cancel. + self.oidc_in_flight = true; + self.show_oidc_login_branch(cx); + self.view.button(cx, ids!(oidc_continue_button)).set_visible(cx, false); + let status = self.view.label(cx, ids!(oidc_status_label)); + status.set_text(cx, tr_key(self.app_language, "login.oidc.waiting_body")); + status.set_visible(cx, true); + let cancel = self.view.button(cx, ids!(oidc_cancel_button)); + cancel.set_text(cx, tr_key(self.app_language, "login.button.cancel_oidc")); + cancel.set_visible(cx, true); + login_status_modal.close(cx); + self.redraw(cx); + } + Some(LoginAction::OidcLoginCancelled) => { + // Restore the idle MAS branch so the user can retry. Use + // login.oidc.cancelled as a soft hint in the info body so + // they know why they're back here without a modal popup. + self.oidc_in_flight = false; + self.show_oidc_login_branch(cx); + self.view.label(cx, ids!(oidc_info_body)) + .set_text(cx, tr_key(self.app_language, "login.oidc.cancelled")); + self.redraw(cx); + } + Some(LoginAction::OidcLoginFailed(error)) => { + // Same idle reset as cancel, but surface the error via + // the login_status_modal so it's unmissable. + self.oidc_in_flight = false; + self.show_oidc_login_branch(cx); + login_status_modal_inner.set_title( + cx, + tr_key(self.app_language, "login.status.login_failed"), + ); + login_status_modal_inner.set_status(cx, error); + login_status_modal_inner.button_ref(cx) + .set_text(cx, tr_key(self.app_language, "login.status.okay")); + login_status_modal.open(cx); + self.redraw(cx); + } _ => { } } @@ -1209,13 +1549,27 @@ pub enum LoginAction { NavigateToRegister, /// Request to cancel adding an account and return to the previous screen. CancelAddAccount, + /// Posted by the OIDC worker once the browser-based auth flow has been + /// launched and robrix2 is waiting for the loopback callback. + /// LoginScreen uses this to swap the "Continue in browser" button for the + /// "Waiting for callback..." + Cancel affordance. + OidcLoginStarted, + /// Posted when the OIDC flow was aborted — either via in-app Cancel, via + /// the browser's `error=access_denied` redirect, or via the 3-minute + /// total timeout. LoginScreen returns to the MAS branch ready-for-retry. + OidcLoginCancelled, + /// Posted when OIDC failed at any post-click stage (metadata discovery, + /// dynamic registration, authorize build, browser open, token exchange). + /// Payload is user-displayable; technical details go to logs. + OidcLoginFailed(String), #[default] None, } #[cfg(test)] mod tests { - use super::should_show_login_failure_modal; + use super::{should_probe_homeserver, should_show_login_failure_modal}; + use crate::homeserver::LoginMode; #[test] fn login_failure_modal_is_suppressed_while_register_flow_is_active() { @@ -1231,4 +1585,20 @@ mod tests { fn fresh_login_failure_message_is_shown_when_not_suppressed() { assert!(should_show_login_failure_modal(false, Some("old"), "boom")); } + + #[test] + fn capability_probe_is_required_when_login_mode_is_unknown() { + assert!(should_probe_homeserver(None, false)); + } + + #[test] + fn capability_probe_is_not_required_when_mode_already_classified() { + assert!(!should_probe_homeserver(Some(LoginMode::Password), false)); + assert!(!should_probe_homeserver(Some(LoginMode::MasOidc), false)); + } + + #[test] + fn capability_probe_is_not_required_while_oidc_login_is_in_flight() { + assert!(!should_probe_homeserver(None, true)); + } } diff --git a/src/login/mod.rs b/src/login/mod.rs index 13d5b39b..6f1cbc2b 100644 --- a/src/login/mod.rs +++ b/src/login/mod.rs @@ -2,6 +2,7 @@ use makepad_widgets::ScriptVm; pub mod login_screen; pub mod login_status_modal; +pub mod oidc_login; pub fn script_mod(vm: &mut ScriptVm) { login_status_modal::script_mod(vm); diff --git a/src/login/oidc_login.rs b/src/login/oidc_login.rs new file mode 100644 index 00000000..95630143 --- /dev/null +++ b/src/login/oidc_login.rs @@ -0,0 +1,298 @@ +//! OIDC (MAS) login orchestration for existing Matrix accounts. +//! +//! This module owns the server-side OAuth 2.0 authorization-code flow that +//! lets robrix2 sign users into MAS-delegated homeservers (matrix.org, +//! alvin.meldry.com, any MSC2965 server). The UI-facing entry point is +//! `MatrixRequest::StartOidcLogin` in the Matrix worker; on the worker +//! side, the spawned task calls `start_oidc_login()` here and funnels the +//! outcome back into the normal `LoginRequest::LoginByOidcSuccess` pipeline +//! (shared with password + SSO paths). +//! +//! Error variants are only added when an actual construction site lands — +//! the project disallows `#[allow(dead_code)]` (see `feedback_no_allow_dead_code`). + +use std::time::Duration; + +use makepad_widgets::Cx; +use matrix_sdk::{ + Client, + authentication::oauth::{ + ClientRegistrationData, OAuthError, + error::{OAuthAuthorizationCodeError, OAuthClientRegistrationError}, + registration::{ApplicationType, ClientMetadata, Localized, OAuthGrantType}, + }, + ruma::{OwnedUserId, serde::Raw}, + utils::local_server::LocalServerBuilder, +}; +use robius_open::Uri; +use tokio::sync::oneshot; +use url::Url; + +use crate::{ + login::login_screen::LoginAction, + persistence::ClientSessionPersisted, + sliding_sync::build_client_for_oidc, +}; + +/// The total time we're willing to wait for the user to finish the browser +/// auth flow before giving up and releasing the loopback server. +const OIDC_FLOW_TIMEOUT: Duration = Duration::from_secs(180); + +/// Every terminal state an OIDC login can reach. +#[derive(Debug)] +pub enum OidcLoginError { + /// The flow was aborted before the homeserver issued tokens. Possible + /// triggers: in-app Cancel, browser `error=access_denied`, loopback + /// server closing without a redirect. All collapse to one message + /// because the corrective action is identical — click Continue again. + Cancelled, + + /// 3-minute total timeout elapsed without a browser callback. + Timeout, + + /// Dynamic client registration (MSC2966) is unsupported by this server. + /// The message tells the user the failure is server-side. + DynamicRegistrationNotSupported, + + /// `build_client()` (sqlite setup, homeserver discovery) failed before + /// the OAuth flow even started. + ClientBuild(String), + + /// OAuth server metadata discovery (`.well-known/openid-configuration`) + /// failed. Distinct from "not MAS" because capability probe already + /// classified this homeserver as MAS — so this is network/server sick. + ServerMetadata(String), + + /// Loopback HTTP server failed to bind (ports exhausted, firewall block). + LoopbackServer(String), + + /// `OAuth::login().build().await` failed for a reason other than one of + /// the specific cases above (malformed metadata, scope mismatch, etc). + AuthorizeBuild(String), + + /// `robius_open` couldn't launch the system browser. + BrowserOpen(String), + + /// Token exchange (`finish_login`) or session load failed — the user + /// completed the browser step but robrix2 couldn't finalize the session. + FinishLogin(String), + + /// Catch-all for invariant-violating conditions that should never happen + /// (e.g., `client.user_id()` returning None after a successful login). + Other(String), +} + +/// Translate an `OidcLoginError` into a sentence safe to show in the UI. +/// +/// Pattern mirrors `map_register_error()` in sliding_sync.rs: technical +/// details go to `log!` / `error!` at the construction site; this function +/// is deliberately terse and friendly. +pub fn map_oidc_error(err: &OidcLoginError) -> String { + match err { + OidcLoginError::Cancelled => { + "Sign-in was cancelled.".to_string() + } + OidcLoginError::Timeout => { + "Sign-in timed out. Please try again.".to_string() + } + OidcLoginError::DynamicRegistrationNotSupported => { + "This server doesn't support third-party sign-in apps yet.".to_string() + } + OidcLoginError::ClientBuild(e) => { + format!("Couldn't prepare the sign-in client: {e}") + } + OidcLoginError::ServerMetadata(e) => { + format!("Couldn't discover sign-in server: {e}") + } + OidcLoginError::LoopbackServer(e) => { + format!("Couldn't start local sign-in server: {e}") + } + OidcLoginError::AuthorizeBuild(e) => { + format!("Couldn't build sign-in URL: {e}") + } + OidcLoginError::BrowserOpen(e) => { + format!("Couldn't open your browser: {e}") + } + OidcLoginError::FinishLogin(e) => { + format!("Sign-in failed at the last step: {e}") + } + OidcLoginError::Other(e) => { + format!("Sign-in failed: {e}") + } + } +} + +/// Classify an `OAuthError` returned by `OAuth::login().build()`. +/// +/// MSC2966 "not supported" gets its own user message; everything else is +/// bucketed into ServerMetadata (discovery path) or AuthorizeBuild (authorize +/// path) so the ops class is visible in logs. +fn classify_auth_build_error(err: OAuthError) -> OidcLoginError { + match &err { + OAuthError::ClientRegistration(OAuthClientRegistrationError::NotSupported) => { + OidcLoginError::DynamicRegistrationNotSupported + } + OAuthError::Discovery(_) => { + OidcLoginError::ServerMetadata(err.to_string()) + } + _ => OidcLoginError::AuthorizeBuild(err.to_string()), + } +} + +/// Classify a `matrix_sdk::Error` returned by `OAuth::finish_login()`. +/// +/// Browser-side cancellation (MAS redirects with `error=access_denied`) lands +/// here as `OAuthError::AuthorizationCode(_::Cancelled)`; collapse it to our +/// shared Cancelled variant so the UI state machine stays simple. +fn classify_finish_error(err: matrix_sdk::Error) -> OidcLoginError { + if let matrix_sdk::Error::OAuth(oauth_err) = &err { + if let OAuthError::AuthorizationCode(ref ac) = **oauth_err { + if matches!(ac, OAuthAuthorizationCodeError::Cancelled) { + return OidcLoginError::Cancelled; + } + } + } + OidcLoginError::FinishLogin(err.to_string()) +} + +/// Build the `ClientRegistrationData` describing Robrix to the MAS server. +/// +/// Dynamic registration happens per-flow (no client_id caching yet) because +/// the redirect URI carries a fresh loopback port each run. Caching is an +/// Open Question to be answered in Phase 3c once the basic flow is stable. +fn client_registration_data(redirect_uri: &Url) -> Result { + let client_uri = Url::parse("https://github.com/project-robius/robrix") + .map_err(|e| OidcLoginError::Other(format!("client URI parse: {e}")))?; + let mut metadata = ClientMetadata::new( + ApplicationType::Native, + vec![OAuthGrantType::AuthorizationCode { + redirect_uris: vec![redirect_uri.clone()], + }], + Localized::new(client_uri, None), + ); + metadata.client_name = Some(Localized::new("Robrix".to_owned(), None)); + Raw::new(&metadata) + .map(ClientRegistrationData::new) + .map_err(|e| OidcLoginError::Other(format!("metadata serialize: {e}"))) +} + +/// End-to-end MAS OIDC login. +/// +/// Flow: +/// 1. Build a fresh `Client` keyed to `homeserver_url` + `proxy`. +/// 2. Spawn a loopback HTTP server on 127.0.0.1 to catch the redirect. +/// 3. Ask MAS for an authorization URL (triggers dynamic client registration). +/// 4. Open the URL in the system browser and post `OidcLoginStarted`. +/// 5. Race loopback callback vs in-app Cancel vs 3-minute timeout. +/// 6. Exchange the authorization code for tokens via `finish_login`. +/// +/// On success the function returns `(client, client_session, user_id)` — +/// the caller pipes those into `LoginRequest::LoginByOidcSuccess` so the +/// normal post-login pipeline (save_session, sync service startup, main +/// UI navigation) runs unchanged. +pub async fn start_oidc_login( + homeserver_url: String, + proxy: Option, + cancel_rx: oneshot::Receiver<()>, +) -> Result<(Client, ClientSessionPersisted, OwnedUserId), OidcLoginError> { + // 1. Build client. Trim to None when the probe handed us an empty URL so + // build_client falls back to its usual homeserver inference. + let homeserver = { + let trimmed = homeserver_url.trim(); + if trimmed.is_empty() { None } else { Some(trimmed.to_owned()) } + }; + let (client, client_session) = build_client_for_oidc(homeserver, proxy).await + .map_err(|e| OidcLoginError::ClientBuild(e.to_string()))?; + + // 2. Spawn loopback server. Dropping `redirect_handle` later in this + // function tears the server down; all early-return paths also release + // it because `redirect_handle` goes out of scope. + let (redirect_uri, redirect_handle) = LocalServerBuilder::new().spawn().await + .map_err(|e| OidcLoginError::LoopbackServer(e.to_string()))?; + + // 3. Kick off the authorization flow. Dynamic registration happens here + // (client not yet registered), and it's where MSC2966-unsupported + // servers fail fast. + let registration_data = client_registration_data(&redirect_uri)?; + let auth_data = client.oauth() + .login(redirect_uri.clone(), None, Some(registration_data), None) + .build() + .await + .map_err(classify_auth_build_error)?; + + // 4. Launch the browser. On failure, abort_login to release the CSRF + // state held inside matrix-sdk so the next retry starts fresh. + if let Err(e) = Uri::new(auth_data.url.as_str()).open() { + client.oauth().abort_login(&auth_data.state).await; + return Err(OidcLoginError::BrowserOpen(format!("{e:?}"))); + } + + // 5. Tell the UI the loopback is live + browser has launched. + Cx::post_action(LoginAction::OidcLoginStarted); + + // 6. Race: loopback callback | explicit cancel | 3-min timeout. + // Every non-success path calls abort_login + returns an error; the + // redirect_handle drop then tears the loopback down. + let query = tokio::select! { + q = redirect_handle => match q { + Some(query) => query, + None => { + client.oauth().abort_login(&auth_data.state).await; + return Err(OidcLoginError::Cancelled); + } + }, + _ = cancel_rx => { + client.oauth().abort_login(&auth_data.state).await; + return Err(OidcLoginError::Cancelled); + } + _ = tokio::time::sleep(OIDC_FLOW_TIMEOUT) => { + client.oauth().abort_login(&auth_data.state).await; + return Err(OidcLoginError::Timeout); + } + }; + + // 7. Token exchange. `query.into()` converts `QueryString` → `UrlOrQuery` + // via the From impl that ships under the `local-server` feature. + client.oauth().finish_login(query.into()).await + .map_err(classify_finish_error)?; + + // 8. Resolve user_id for the caller's account-manager bookkeeping. + let user_id = client.user_id() + .ok_or_else(|| OidcLoginError::Other( + "no user_id after finish_login — matrix-sdk invariant violated".to_string(), + ))? + .to_owned(); + + Ok((client, client_session, user_id)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn map_oidc_error_handles_cancelled_login() { + let msg = map_oidc_error(&OidcLoginError::Cancelled); + assert!(msg.to_lowercase().contains("cancel"), "got: {msg}"); + } + + #[test] + fn map_oidc_error_handles_missing_dynamic_registration() { + let msg = map_oidc_error(&OidcLoginError::DynamicRegistrationNotSupported); + assert!(msg.contains("third-party sign-in apps"), "got: {msg}"); + } + + #[test] + fn map_oidc_error_handles_timeout_is_retriable() { + let msg = map_oidc_error(&OidcLoginError::Timeout); + assert!(msg.to_lowercase().contains("try again"), "got: {msg}"); + } + + #[test] + fn map_oidc_error_surfaces_inner_finish_login_text() { + let msg = map_oidc_error(&OidcLoginError::FinishLogin( + "token endpoint 502".to_string(), + )); + assert!(msg.contains("token endpoint 502"), "got: {msg}"); + } +} diff --git a/src/logout/logout_state_machine.rs b/src/logout/logout_state_machine.rs index 9d2c4bde..4830ec30 100644 --- a/src/logout/logout_state_machine.rs +++ b/src/logout/logout_state_machine.rs @@ -88,6 +88,7 @@ use std::time::{Duration, Instant}; use tokio::sync::{Mutex, Notify}; use anyhow::{anyhow, Result}; use makepad_widgets::{Cx, log}; +use matrix_sdk::authentication::AuthApi; use crate::home::navigation_tab_bar::NavigationBarAction; use crate::persistence::{delete_latest_user_id, skip_app_state_restore_once}; @@ -498,13 +499,36 @@ impl LogoutStateMachine { let Some(client) = get_client() else { return Err(LogoutError::Unrecoverable(UnrecoverableError::ComponentsCleared)); }; - - match tokio::time::timeout( - self.config.server_logout_timeout, - client.matrix_auth().logout() - ).await { - Ok(Ok(_)) => Ok(()), - Ok(Err(e)) => Err(LogoutError::Recoverable(RecoverableError::ServerLogoutFailed(e.to_string()))), + + // Dispatch by the active auth kind: password/Matrix sessions revoke via + // matrix_auth().logout(); OIDC sessions revoke tokens at the MAS issuer + // via oauth().logout(). The two return types differ, so normalize to + // Result<(), String> under a single timeout. + let logout_future: std::pin::Pin> + Send>> = + match client.auth_api() { + Some(AuthApi::Matrix(_)) => { + let client = client.clone(); + Box::pin(async move { + client.matrix_auth().logout().await + .map(|_| ()) + .map_err(|e| e.to_string()) + }) + } + Some(AuthApi::OAuth(_)) => { + let client = client.clone(); + Box::pin(async move { + client.oauth().logout().await + .map_err(|e| e.to_string()) + }) + } + // No live auth API means there is nothing to revoke server-side + // (e.g., session already torn down); succeed silently. + _ => return Ok(()), + }; + + match tokio::time::timeout(self.config.server_logout_timeout, logout_future).await { + Ok(Ok(())) => Ok(()), + Ok(Err(msg)) => Err(LogoutError::Recoverable(RecoverableError::ServerLogoutFailed(msg))), Err(_) => Err(LogoutError::Recoverable(RecoverableError::Timeout("Server logout timed out".to_string()))), } } diff --git a/src/persistence/matrix_state.rs b/src/persistence/matrix_state.rs index 635562bc..c11280eb 100644 --- a/src/persistence/matrix_state.rs +++ b/src/persistence/matrix_state.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use anyhow::{anyhow, bail}; use makepad_widgets::{log, warning, Cx}; use matrix_sdk::{ - authentication::matrix::MatrixSession, + authentication::{AuthSession, matrix::MatrixSession, oauth::{ClientId, OAuthSession, UserSession}}, ruma::{OwnedUserId, UserId}, sliding_sync, Client, @@ -45,8 +45,8 @@ pub struct FullSessionPersisted { /// The data to re-build the client. pub client_session: ClientSessionPersisted, - /// The Matrix user session. - pub user_session: MatrixSession, + /// The persisted auth session. + pub user_session: PersistedAuthSession, /// The latest sync token. /// @@ -68,6 +68,40 @@ pub struct FullSessionPersisted { pub sliding_sync_version: SlidingSyncVersion, } +/// Persisted OAuth session payload. +#[derive(Debug, Serialize, Deserialize)] +pub struct PersistedOAuthSession { + pub client_id: String, + pub user_session: UserSession, +} + +/// Persisted auth session, backward-compatible with old matrix-only files. +#[derive(Debug, Serialize, Deserialize)] +#[serde(untagged)] +pub enum PersistedAuthSession { + Matrix(MatrixSession), + OAuth(PersistedOAuthSession), +} + +impl PersistedAuthSession { + fn user_id(&self) -> &UserId { + match self { + PersistedAuthSession::Matrix(session) => session.meta.user_id.as_ref(), + PersistedAuthSession::OAuth(session) => session.user_session.meta.user_id.as_ref(), + } + } + + fn into_auth_session(self) -> AuthSession { + match self { + PersistedAuthSession::Matrix(session) => AuthSession::Matrix(session), + PersistedAuthSession::OAuth(session) => AuthSession::OAuth(Box::new(OAuthSession { + client_id: ClientId::new(session.client_id), + user: session.user_session, + })), + } + } +} + /// A serializable duplicate of [`sliding_sync::Version`]. #[derive(Debug, Default, Serialize, Deserialize)] pub enum SlidingSyncVersion { @@ -197,16 +231,16 @@ pub async fn restore_session( let client = client_builder.build().await?; let sliding_sync_version = sliding_sync_version.into(); client.set_sliding_sync_version(sliding_sync_version); - let status_str = format!("Authenticating previous login session for {}...", user_session.meta.user_id); + let restored_user_id = user_session.user_id().to_owned(); + let status_str = format!("Authenticating previous login session for {}...", restored_user_id); log!("{status_str}"); Cx::post_action(LoginAction::Status { title: "Authenticating session".into(), status: status_str, }); - // Restore the Matrix user session. - client.restore_session(user_session).await?; - save_latest_user_id(&user_id).await?; + client.restore_session(user_session.into_auth_session()).await?; + save_latest_user_id(&restored_user_id).await?; Ok((client, sync_token, client_session)) } @@ -222,14 +256,23 @@ pub async fn save_session( client_session: ClientSessionPersisted, ) -> anyhow::Result<()> { let user_session = client - .matrix_auth() .session() .ok_or_else(|| anyhow!("A logged-in client should have a session"))?; - save_latest_user_id(&user_session.meta.user_id).await?; + let user_session = match user_session { + AuthSession::Matrix(session) => PersistedAuthSession::Matrix(session), + AuthSession::OAuth(session) => PersistedAuthSession::OAuth(PersistedOAuthSession { + client_id: session.client_id.to_string(), + user_session: session.user, + }), + other => bail!("Unsupported auth session variant for persistence: {other:?}"), + }; + + let persisted_user_id = user_session.user_id().to_owned(); + save_latest_user_id(&persisted_user_id).await?; let sliding_sync_version = client.sliding_sync_version().into(); // Save that user's session. - let session_file = session_file_path(&user_session.meta.user_id); + let session_file = session_file_path(&persisted_user_id); let serialized_session = serde_json::to_string(&FullSessionPersisted { client_session, user_session, @@ -332,3 +375,35 @@ pub async fn delete_session(user_id: &UserId) -> anyhow::Result { Ok(false) } } + +#[cfg(test)] +mod tests { + use matrix_sdk::{ + SessionMeta, SessionTokens, + authentication::oauth::UserSession, + ruma::{device_id, user_id}, + }; + + use super::{PersistedAuthSession, PersistedOAuthSession}; + + #[test] + fn persisted_auth_session_round_trips_oauth_variant() { + let persisted = PersistedAuthSession::OAuth(PersistedOAuthSession { + client_id: "client-id".into(), + user_session: UserSession { + meta: SessionMeta { + user_id: user_id!("@alice:example.org").to_owned(), + device_id: device_id!("DEVICEID").to_owned(), + }, + tokens: SessionTokens { + access_token: "access".into(), + refresh_token: Some("refresh".into()), + }, + }, + }); + + let json = serde_json::to_string(&persisted).unwrap(); + let restored: PersistedAuthSession = serde_json::from_str(&json).unwrap(); + assert_eq!(restored.user_id().as_str(), "@alice:example.org"); + } +} diff --git a/src/register/mod.rs b/src/register/mod.rs index c71c8356..3ca19925 100644 --- a/src/register/mod.rs +++ b/src/register/mod.rs @@ -15,45 +15,9 @@ pub fn script_mod(vm: &mut ScriptVm) { register_screen::script_mod(vm); } -use matrix_sdk::ruma::api::client::uiaa::UiaaInfo; +use crate::homeserver::HsCapabilities; use makepad_widgets::ActionDefaultRef; -/// Homeserver capabilities discovered before register branching. -#[derive(Clone, Debug)] -pub struct HsCapabilities { - /// Normalized base URL the client will use. - pub base_url: String, - /// True iff the server advertises `m.authentication.issuer` in - /// `.well-known/matrix/client` (MSC2965 / MAS delegation). - pub is_mas_native_oidc: bool, - /// True iff `POST /_matrix/client/v3/register` with empty body returns - /// 401 with parseable UIAA flows (NOT 403 M_FORBIDDEN). - pub registration_enabled: bool, - /// Optional UIAA probe result (empty when server requires MAS). - pub uiaa_probe: Option, - /// Identity providers harvested from `/_matrix/client/v3/login`. - /// Phase 1 populates but does not render; Phase 4 renders buttons. - pub sso_providers: Vec, - - /// URL to open in the system browser for MAS self-registration. - /// Derived as `/register` when a MAS issuer is discovered in - /// `.well-known` `m.authentication` (stable) or - /// `org.matrix.msc2965.authentication` (unstable). None for non-MAS - /// servers. Intentionally does NOT use MSC2965's `account` field — - /// that URL is for logged-in account management and loops when - /// opened unauthenticated. - pub mas_signup_url: Option, -} - -/// Minimal info per identity provider. Full matrix-sdk type is not -/// used because we only need name + id at this phase. -#[derive(Clone, Debug)] -pub struct IdentityProviderSummary { - pub id: String, - pub name: String, - pub icon_url: Option, -} - /// Outcome classification of capability discovery. /// /// Derived from `HsCapabilities` by `mode()` below; used for UI display. @@ -89,10 +53,6 @@ impl HsCapabilities { pub enum RegisterAction { /// User clicked the back button on RegisterScreen. NavigateToLogin, - /// `requested_url` is echoed so the screen can drop out-of-order responses. - CapabilitiesDiscovered { requested_url: String, caps: Box }, - /// `requested_url` is echoed so the screen can drop out-of-order responses. - DiscoveryFailed { requested_url: String, error: String }, /// User submitted the RegistrationForm; first POST is in flight. RegistrationSubmitted, /// Registration completed and the client has been persisted via @@ -111,3 +71,38 @@ impl ActionDefaultRef for RegisterAction { &DEFAULT } } + +#[cfg(test)] +mod homeserver_login_mode_tests { + use crate::homeserver::{HsCapabilities, IdentityProviderSummary, LoginMode, login_mode}; + + #[test] + fn login_mode_prefers_mas_oidc_when_mas_is_advertised() { + let caps = HsCapabilities { + base_url: "https://matrix.org".into(), + is_mas_native_oidc: true, + registration_enabled: false, + uiaa_probe: None, + sso_providers: Vec::::new(), + mas_signup_url: Some("https://issuer/register".into()), + mas_issuer_url: Some("https://issuer".into()), + }; + + assert_eq!(login_mode(&caps), LoginMode::MasOidc); + } + + #[test] + fn login_mode_falls_back_to_password_for_non_mas_servers() { + let caps = HsCapabilities { + base_url: "https://palpo.example".into(), + is_mas_native_oidc: false, + registration_enabled: true, + uiaa_probe: None, + sso_providers: Vec::::new(), + mas_signup_url: None, + mas_issuer_url: None, + }; + + assert_eq!(login_mode(&caps), LoginMode::Password); + } +} diff --git a/src/register/register_screen.rs b/src/register/register_screen.rs index 1bb06936..24e82685 100644 --- a/src/register/register_screen.rs +++ b/src/register/register_screen.rs @@ -11,8 +11,9 @@ use makepad_widgets::*; +use crate::homeserver::{CapabilityProbeAction, HsCapabilities}; use crate::login::login_screen::LoginAction; -use crate::register::{HsCapabilities, RegisterAction, RegisterMode}; +use crate::register::{RegisterAction, RegisterMode}; use crate::register::validation::{normalize_homeserver_url, HomeserverUrlError}; use crate::sliding_sync::{submit_async_request, MatrixRequest}; @@ -441,8 +442,8 @@ impl WidgetMatchEvent for RegisterScreen { } for action in actions { - match action.downcast_ref::() { - Some(RegisterAction::CapabilitiesDiscovered { requested_url, caps }) => { + match action.downcast_ref::() { + Some(CapabilityProbeAction::Discovered { requested_url, caps }) => { // Drop out-of-order response from a superseded Next click. if self.last_discovery_input_url.as_deref() != Some(requested_url.as_str()) { continue; @@ -501,7 +502,7 @@ impl WidgetMatchEvent for RegisterScreen { } self.last_discovery = Some(caps.clone()); } - Some(RegisterAction::DiscoveryFailed { requested_url, error }) => { + Some(CapabilityProbeAction::Failed { requested_url, error }) => { if self.last_discovery_input_url.as_deref() != Some(requested_url.as_str()) { continue; } @@ -513,6 +514,12 @@ impl WidgetMatchEvent for RegisterScreen { self.last_discovery = None; self.last_discovery_input_url = None; } + _ => {} + } + } + + for action in actions { + match action.downcast_ref::() { Some(RegisterAction::RegistrationSubmitted) => {} Some(RegisterAction::RegistrationSuccess) => { // Full reset: the same widget instance is reused on re-entry diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 375af757..62757021 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -35,7 +35,7 @@ use robius_open::Uri; use ruma::{OwnedRoomOrAliasId, RoomId, events::tag::Tags}; use tokio::{ runtime::Handle, - sync::{broadcast, mpsc::{Sender, UnboundedReceiver, UnboundedSender}, watch, Notify}, task::JoinHandle, time::error::Elapsed, + sync::{broadcast, mpsc::{Sender, UnboundedReceiver, UnboundedSender}, oneshot, watch, Notify}, task::JoinHandle, time::error::Elapsed, }; use url::Url; use std::{borrow::Cow, cmp::{max, min}, future::Future, hash::{BuildHasherDefault, DefaultHasher}, iter::Peekable, ops::{Deref, DerefMut, Not}, path::{ Path, PathBuf }, sync::{Arc, LazyLock, Mutex, atomic::{AtomicBool, Ordering}}, time::Duration}; @@ -45,7 +45,7 @@ use crate::{ account_manager::{self, Account}, app::{AppStateAction, RoomFilterRemoteSearchAction}, app_data_dir, avatar_cache::AvatarUpdate, event_preview::{BeforeText, TextPreview, text_preview_of_raw_timeline_event, text_preview_of_timeline_item}, home::{ add_room::{CreatableSpacesAction, CreateRoomAction, CreateRoomContext, KnockResultAction}, invite_screen::{JoinRoomResultAction, LeaveRoomResultAction}, link_preview::{LinkPreviewData, LinkPreviewDataNonNumeric, LinkPreviewRateLimitResponse}, room_screen::{ActionResponseResultAction, InviteResultAction, ReportRoomResultAction, TimelineUpdate}, rooms_list::{self, InvitedRoomInfo, InviterInfo, JoinedRoomInfo, RoomsListUpdate, build_room_search_text, enqueue_rooms_list_update}, rooms_list_header::RoomsListHeaderAction, tombstone_footer::SuccessorRoomDetails - }, login::login_screen::LoginAction, logout::{logout_confirm_modal::LogoutAction, logout_state_machine::{LogoutConfig, is_logout_in_progress, logout_with_state_machine}}, media_cache::{MediaCacheEntry, MediaCacheEntryRef}, persistence::{self, ClientSessionPersisted, load_app_state, take_skip_app_state_restore_once}, profile::{ + }, homeserver::{CapabilityProbeAction, HsCapabilities, IdentityProviderSummary}, login::login_screen::LoginAction, logout::{logout_confirm_modal::LogoutAction, logout_state_machine::{LogoutConfig, is_logout_in_progress, logout_with_state_machine}}, media_cache::{MediaCacheEntry, MediaCacheEntryRef}, persistence::{self, ClientSessionPersisted, load_app_state, take_skip_app_state_restore_once}, profile::{ user_profile::UserProfile, user_profile_cache::{UserProfileUpdate, enqueue_user_profile_update}, }, room::{FetchedRoomAvatar, FetchedRoomPreview, RoomPreviewAction}, shared::{ @@ -512,12 +512,32 @@ async fn login( } Ok((client, None, is_add_account, client_session)) } + LoginRequest::LoginByOidcSuccess(client, client_session, is_add_account) => { + // Mirrors the SSO arm: the OIDC worker already performed + // finish_login, so the client is fully authenticated. We only + // need to persist and return — finalize_authenticated_client in + // the outer loop handles account-manager + rooms-list status. + if let Err(e) = persistence::save_session(&client, client_session.clone()).await { + error!("Failed to save session state to storage: {e:?}"); + } + Ok((client, None, is_add_account, client_session)) + } LoginRequest::HomeserverLoginTypesQuery(_) => { bail!("LoginRequest::HomeserverLoginTypesQuery not handled earlier"); } } } +/// Thin wrapper around `build_client` that exposes just what the OIDC worker +/// needs, without leaking the private `Cli` type across module boundaries. +pub(crate) async fn build_client_for_oidc( + homeserver: Option, + proxy: Option, +) -> std::result::Result<(Client, ClientSessionPersisted), ClientBuildError> { + let cli = Cli { homeserver, proxy, ..Default::default() }; + build_client(&cli, app_data_dir()).await +} + /// Which direction to paginate in. /// @@ -712,11 +732,32 @@ pub enum MatrixRequest { Login(LoginRequest), /// Probe a homeserver's registration capabilities. /// Sent from RegisterScreen's Next button; result arrives via - /// `RegisterAction::CapabilitiesDiscovered` / `DiscoveryFailed`. + /// `CapabilityProbeAction::Discovered` / `Failed`. DiscoverHomeserverCapabilities { /// Already-normalized homeserver URL (has scheme, no trailing slash). url: String, }, + /// Begin the OIDC (MAS) login flow for an already-existing account on a + /// MAS-delegated homeserver. `homeserver_url` is the normalized URL from + /// capability discovery; `proxy` mirrors the password login's optional + /// per-request proxy override. + /// + /// Outcome dispatch: + /// - `LoginAction::OidcLoginStarted` fires once the loopback server is + /// live and the system browser has been opened. + /// - On success, `LoginAction::LoginSuccess` fires after + /// `finalize_authenticated_client()` persists the session. + /// - Cancellation (in-app Cancel, browser `error=access_denied`, or + /// 3-min timeout) dispatches `LoginAction::OidcLoginCancelled`. + /// - Any other failure dispatches `LoginAction::OidcLoginFailed(msg)`. + StartOidcLogin { + homeserver_url: String, + proxy: Option, + is_add_account: bool, + }, + /// Abort the in-flight OIDC login. Posted by LoginScreen's Cancel button. + /// No-op if no OIDC login is currently in flight. + CancelOidcLogin, /// Register a new account on a UIAA server using the single-stage /// `m.login.dummy` flow. `homeserver_url` is the already-normalized URL /// from capability discovery. @@ -1292,6 +1333,12 @@ pub enum LoginRequest{ LoginByPassword(LoginByPassword), Register(RegisterAccount), LoginBySSOSuccess(Client, ClientSessionPersisted, bool), + /// Sent by the OIDC worker task after `OAuth::finish_login()` returns + /// successfully. The payload mirrors `LoginBySSOSuccess` — already-built + /// client + its session bundle + `is_add_account`. The main login + /// handler just persists the session and returns it to the outer loop, + /// so sync-service startup is shared with password/SSO flows. + LoginByOidcSuccess(Client, ClientSessionPersisted, bool), LoginByCli, HomeserverLoginTypesQuery(String), @@ -1337,6 +1384,7 @@ async fn matrix_worker_task( let is_add_account = match &login_request { LoginRequest::LoginByPassword(lpw) => lpw.is_add_account, LoginRequest::LoginBySSOSuccess(_, _, is_add) => *is_add, + LoginRequest::LoginByOidcSuccess(_, _, is_add) => *is_add, _ => false, }; @@ -1389,13 +1437,13 @@ async fn matrix_worker_task( let requested_url = url.clone(); match discover_homeserver_capabilities(&url).await { Ok(caps) => { - Cx::post_action(crate::register::RegisterAction::CapabilitiesDiscovered { + Cx::post_action(CapabilityProbeAction::Discovered { requested_url, caps: Box::new(caps), }); } Err(e) => { - Cx::post_action(crate::register::RegisterAction::DiscoveryFailed { + Cx::post_action(CapabilityProbeAction::Failed { requested_url, error: e.to_string(), }); @@ -1404,6 +1452,53 @@ async fn matrix_worker_task( }); } + MatrixRequest::StartOidcLogin { homeserver_url, proxy, is_add_account } => { + let (flow_id, cancel_rx) = match try_start_oidc_flow() { + Ok(flow) => flow, + Err(msg) => { + warning!("{msg}"); + continue; + } + }; + + let login_sender = login_sender.clone(); + tokio::spawn(async move { + let outcome = crate::login::oidc_login::start_oidc_login( + homeserver_url, + proxy, + cancel_rx, + ).await; + + match outcome { + Ok((client, client_session, user_id)) => { + log!("OIDC login succeeded for {user_id}; forwarding to login pipeline."); + if let Err(e) = login_sender.send( + LoginRequest::LoginByOidcSuccess(client, client_session, is_add_account) + ).await { + error!("Failed to forward OIDC login result: {e:?}"); + Cx::post_action(LoginAction::OidcLoginFailed( + "BUG: couldn't hand OIDC login result to the login pipeline.".to_string(), + )); + } + } + Err(crate::login::oidc_login::OidcLoginError::Cancelled) => { + Cx::post_action(LoginAction::OidcLoginCancelled); + } + Err(e) => { + error!("OIDC login failed: {e:?}"); + let msg = crate::login::oidc_login::map_oidc_error(&e); + Cx::post_action(LoginAction::OidcLoginFailed(msg)); + } + } + + finish_oidc_flow(flow_id); + }); + } + + MatrixRequest::CancelOidcLogin => { + cancel_active_oidc_flow(); + } + MatrixRequest::RegisterViaUiaa { username, password, homeserver_url } => { Cx::post_action(crate::register::RegisterAction::RegistrationSubmitted); let register_request = LoginRequest::Register(RegisterAccount { @@ -3705,6 +3800,78 @@ fn get_room_timeline(room_id: &RoomId) -> Option> { /// The logged-in Matrix client, which can be freely and cheaply cloned. static CLIENT: Mutex> = Mutex::new(None); +struct ActiveOidcFlow { + flow_id: u64, + cancel_tx: oneshot::Sender<()>, +} + +#[derive(Default)] +struct OidcFlowSlot { + next_flow_id: u64, + active_flow: Option, +} + +impl OidcFlowSlot { + fn try_start_flow(&mut self) -> std::result::Result<(u64, oneshot::Receiver<()>), &'static str> { + if self.active_flow.is_some() { + return Err("OIDC login already in progress"); + } + + self.next_flow_id += 1; + let flow_id = self.next_flow_id; + let (cancel_tx, cancel_rx) = oneshot::channel(); + self.active_flow = Some(ActiveOidcFlow { flow_id, cancel_tx }); + Ok((flow_id, cancel_rx)) + } + + fn finish_flow(&mut self, flow_id: u64) { + if self + .active_flow + .as_ref() + .is_some_and(|active| active.flow_id == flow_id) + { + self.active_flow = None; + } + } + + fn cancel_active_flow(&mut self) -> bool { + if let Some(active) = self.active_flow.take() { + let _ = active.cancel_tx.send(()); + true + } else { + false + } + } + + #[cfg(test)] + fn has_active_flow(&self) -> bool { + self.active_flow.is_some() + } +} + +/// Single active OIDC flow slot. +/// +/// We keep this generation-scoped rather than storing a bare sender so that a +/// late cleanup from an older flow cannot drop the cancel handle for a newer +/// loopback server. That race would make the browser land on `127.0.0.1` +/// after the local listener had already been torn down. +static OIDC_FLOW_SLOT: Mutex = Mutex::new(OidcFlowSlot { + next_flow_id: 0, + active_flow: None, +}); + +fn try_start_oidc_flow() -> std::result::Result<(u64, oneshot::Receiver<()>), &'static str> { + OIDC_FLOW_SLOT.lock().unwrap().try_start_flow() +} + +fn finish_oidc_flow(flow_id: u64) { + OIDC_FLOW_SLOT.lock().unwrap().finish_flow(flow_id); +} + +fn cancel_active_oidc_flow() -> bool { + OIDC_FLOW_SLOT.lock().unwrap().cancel_active_flow() +} + pub fn get_client() -> Option { CLIENT.lock().unwrap().clone() } @@ -3984,7 +4151,7 @@ async fn start_matrix_client_login_and_sync(rt: Handle) { let mut initial_client_opt = new_login_opt; loop { - let (client, sync_service, logged_in_user_id) = 'login_loop: loop { + let (client, sync_service, logged_in_user_id, client_session) = 'login_loop: loop { let (client, _sync_token, validate_session, session) = match initial_client_opt.take() { Some(login) => login, None => { @@ -4050,7 +4217,7 @@ async fn start_matrix_client_login_and_sync(rt: Handle) { let account = account_manager::Account { client: client.clone(), user_id: logged_in_user_id.clone(), - session, + session: session.clone(), display_name: None, avatar_url: None, }; @@ -4100,14 +4267,14 @@ async fn start_matrix_client_login_and_sync(rt: Handle) { } }; - break 'login_loop (client, sync_service, logged_in_user_id); + break 'login_loop (client, sync_service, logged_in_user_id, session); }; let (session_reset_sender, mut session_reset_receiver) = tokio::sync::mpsc::unbounded_channel::(); // Listen for session changes, e.g., when the access token becomes invalid. let session_change_handler_task = - handle_session_changes(client.clone(), session_reset_sender); + handle_session_changes(client.clone(), client_session.clone(), session_reset_sender); // Signal login success now that SyncService::build() has already succeeded (inside // 'login_loop), which is the only step that can fail with an invalid/expired token. @@ -4266,7 +4433,7 @@ async fn start_matrix_client_login_and_sync(rt: Handle) { REQUEST_SENDER.lock().unwrap().replace(sender); // Restore session for the switched account match persistence::restore_session(Some(switch_user_id.clone())).await { - Ok((client, _sync_token, _session)) => { + Ok((client, _sync_token, session)) => { // Store the client CLIENT.lock().unwrap().replace(client.clone()); @@ -4303,7 +4470,7 @@ async fn start_matrix_client_login_and_sync(rt: Handle) { let (session_reset_sender, mut session_reset_receiver) = tokio::sync::mpsc::unbounded_channel::(); let session_change_handler_task = - handle_session_changes(client.clone(), session_reset_sender); + handle_session_changes(client.clone(), session.clone(), session_reset_sender); let mut matrix_worker_task_handle = rt.spawn(matrix_worker_task(receiver, login_sender)); let mut room_list_service_task = rt.spawn(room_list_service_loop(room_list_service)); @@ -5127,6 +5294,7 @@ fn is_invalid_token_error(e: &sync_service::Error) -> bool { /// so the user is prompted to log in again. fn handle_session_changes( client: Client, + client_session: ClientSessionPersisted, session_reset_sender: UnboundedSender, ) -> JoinHandle<()> { let mut receiver = client.subscribe_to_session_changes(); @@ -5152,7 +5320,14 @@ fn handle_session_changes( // for every rejected request, but one re-login prompt suffices. break; } - Ok(SessionChange::TokensRefreshed) => {} + Ok(SessionChange::TokensRefreshed) => { + // OAuth refresh lands new access/refresh tokens inside the client; + // save_session() re-reads them via client.session() and rewrites the + // on-disk FullSessionPersisted so a restart picks up the fresh pair. + if let Err(e) = persistence::save_session(&client, client_session.clone()).await { + warning!("Failed to persist refreshed session tokens: {e}"); + } + } Err(broadcast::error::RecvError::Lagged(n)) => { warning!("Session change receiver lagged, missed {n} messages."); } @@ -6254,6 +6429,7 @@ impl UserPowerLevels { /// Shuts down the current Tokio runtime completely and takes ownership to ensure proper cleanup. pub fn shutdown_background_tasks() { + cancel_active_oidc_flow(); if let Some(runtime) = TOKIO_RUNTIME.lock().unwrap().take() { runtime.shutdown_background(); } @@ -6262,6 +6438,7 @@ pub fn shutdown_background_tasks() { pub async fn clear_app_state(config: &LogoutConfig) -> Result<()> { // Clear resources normally, allowing them to be properly dropped // This prevents memory leaks when users logout and login again without closing the app + cancel_active_oidc_flow(); CLIENT.lock().unwrap().take(); SYNC_SERVICE.lock().unwrap().take(); REQUEST_SENDER.lock().unwrap().take(); @@ -6292,8 +6469,7 @@ pub async fn clear_app_state(config: &LogoutConfig) -> Result<()> { /// response bodies are read as text and parsed via `serde_json::from_str`. async fn discover_homeserver_capabilities( raw_url: &str, -) -> anyhow::Result { - use crate::register::{HsCapabilities, IdentityProviderSummary}; +) -> anyhow::Result { use serde_json::Value; let http = matrix_sdk::reqwest::Client::builder() @@ -6310,7 +6486,7 @@ async fn discover_homeserver_capabilities( // Step 1: .well-known (lenient — default base_url = raw_url on failure). let wk_url = format!("{raw_url}/.well-known/matrix/client"); - let (base_url, is_mas, mas_signup_url) = match http.get(&wk_url).send().await { + let (base_url, is_mas, mas_signup_url, mas_issuer_url) = match http.get(&wk_url).send().await { Ok(resp) if resp.status().is_success() => { let body = body_json(resp).await; let base = body @@ -6327,17 +6503,18 @@ async fn discover_homeserver_capabilities( // `account` field is for post-login account management (requires a // session) — opening it while unauthenticated loops between // /account/ and /login, so we do NOT use it here. - let (mas, mas_signup_url) = ["m.authentication", "org.matrix.msc2965.authentication"] + let (mas, mas_signup_url, mas_issuer_url) = ["m.authentication", "org.matrix.msc2965.authentication"] .iter() .find_map(|key: &&str| { let issuer = body.get(*key)?.get("issuer").and_then(|v: &Value| v.as_str())?; - let signup = format!("{}/register", issuer.trim_end_matches('/')); - Some((true, Some(signup))) + let issuer = issuer.trim_end_matches('/').to_string(); + let signup = format!("{issuer}/register"); + Some((true, Some(signup), Some(issuer))) }) - .unwrap_or((false, None)); - (base, mas, mas_signup_url) + .unwrap_or((false, None, None)); + (base, mas, mas_signup_url, mas_issuer_url) } - _ => (raw_url.trim_end_matches('/').to_string(), false, None), + _ => (raw_url.trim_end_matches('/').to_string(), false, None, None), }; // Step 2: versions — liveness (fatal if unreachable). @@ -6417,12 +6594,13 @@ async fn discover_homeserver_capabilities( uiaa_probe, sso_providers, mas_signup_url, + mas_issuer_url, }) } #[cfg(test)] mod tests { - use super::worker_shutdown_is_unexpected; + use super::{OidcFlowSlot, worker_shutdown_is_unexpected}; #[test] fn worker_shutdown_is_not_unexpected_during_logout() { @@ -6438,4 +6616,30 @@ mod tests { fn worker_shutdown_is_unexpected_without_controlled_teardown() { assert!(worker_shutdown_is_unexpected(false, false)); } + + #[test] + fn oidc_flow_slot_rejects_duplicate_start_until_cleared() { + let mut slot = OidcFlowSlot::default(); + + let _first = slot.try_start_flow().unwrap(); + assert!(slot.try_start_flow().is_err()); + + assert!(slot.cancel_active_flow()); + assert!(slot.try_start_flow().is_ok()); + } + + #[test] + fn oidc_flow_slot_finish_is_scoped_to_matching_generation() { + let mut slot = OidcFlowSlot::default(); + + let (first_id, _first_rx) = slot.try_start_flow().unwrap(); + assert!(slot.cancel_active_flow()); + + let (second_id, _second_rx) = slot.try_start_flow().unwrap(); + slot.finish_flow(first_id); + assert!(slot.has_active_flow()); + + slot.finish_flow(second_id); + assert!(!slot.has_active_flow()); + } }