From f135e694fbee6780c2a2f82440a3d2acee673883 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 21:41:21 +0000 Subject: [PATCH 1/2] Login: remember accounts on the device and offer them as a picker After someone has signed in, the login screen shows their account the next time instead of an empty username field, like Microsoft's "Pick an account". One click continues as that account; "Use another account" shows today's form, and each tile can be forgotten. - The SPA keeps the accounts in localStorage (forge.accounts, web/src/ lib/accounts.ts): the identifier, the IdP's display name, the method with its realm and the last use, newest first, at most five, one entry per account whatever its case. Never a password, a token or the session. Every access is wrapped; with storage blocked the picker is simply not shown. The server lists no accounts, so /auth/identify still answers from public realm configuration alone. - A password sign-in is remembered when it succeeds. An OIDC sign-in leaves a pending marker in sessionStorage before the browser leaves, and the first /auth/me after the round trip turns it into an entry; coming back to the login screen drops it. - A tile continues at the password step (identifier-first: once /auth/identify confirms the identifier still signs in with a password; combined: the form with the username filled in), or at the IdP of an OIDC realm the login options still list. An OIDC account whose realm is gone is routed by /auth/identify, so it cannot loop. - GET /auth/oidc/login takes an optional login_hint, trimmed, at most 256 bytes and without control characters (anything else is dropped, logged at debug level with its size only), and passes it to the authorize request. Tiles and the identifier-first form send it, so the IdP does not ask for the account again. - remember_accounts, a new auth setting (default true), turns the picker off for kiosks and shared computers: GET /auth/login-options reports it, and with it off the screen keeps none and forgets those kept before. It is an auth setting rather than a forge.toml key, like login_mode, with its checkbox under Admin -> Settings -> Sign-in. - /auth/me reports the IdP's display name for an OIDC session; the callback now links the session to the identity that signed in. Fixes #115 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJbsBcmhipReXByB1cF --- ...17c32fcbc0ff479922e253804baef69719131.json | 28 ++ crates/core/src/oidc/flow.rs | 43 ++- crates/core/tests/oidc_flow.rs | 14 +- crates/db/src/external_identity_dao.rs | 12 + crates/server/src/auth/mod.rs | 106 +++++- crates/server/src/auth/settings.rs | 34 +- crates/server/tests/auth_settings.rs | 33 +- crates/server/tests/oidc_routes.rs | 70 ++++ docs/changes/2.2.0/115.feature.md | 9 + docs/deployment.md | 21 ++ web/e2e/specs/auth.spec.ts | 28 ++ web/src/api/auth.ts | 17 +- web/src/api/settings.ts | 2 + web/src/lib/accounts.test.ts | 168 +++++++++ web/src/lib/accounts.ts | 223 ++++++++++++ web/src/routes/Login.test.tsx | 284 ++++++++++++++- web/src/routes/Login.tsx | 330 ++++++++++++++++-- web/src/routes/admin/settings/Settings.tsx | 17 +- .../routes/admin/settings/settings.test.tsx | 7 + web/src/styles/forge-ui.css | 12 + 20 files changed, 1394 insertions(+), 64 deletions(-) create mode 100644 .sqlx/query-7a8126aeef8d481b8abc9e97ba017c32fcbc0ff479922e253804baef69719131.json create mode 100644 docs/changes/2.2.0/115.feature.md create mode 100644 web/src/lib/accounts.test.ts create mode 100644 web/src/lib/accounts.ts diff --git a/.sqlx/query-7a8126aeef8d481b8abc9e97ba017c32fcbc0ff479922e253804baef69719131.json b/.sqlx/query-7a8126aeef8d481b8abc9e97ba017c32fcbc0ff479922e253804baef69719131.json new file mode 100644 index 00000000..ee0c70e6 --- /dev/null +++ b/.sqlx/query-7a8126aeef8d481b8abc9e97ba017c32fcbc0ff479922e253804baef69719131.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT display_name FROM external_identity WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "display_name", + "type_info": "Text", + "origin": { + "Table": { + "table": "external_identity", + "name": "display_name" + } + } + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + true + ] + }, + "hash": "7a8126aeef8d481b8abc9e97ba017c32fcbc0ff479922e253804baef69719131" +} diff --git a/crates/core/src/oidc/flow.rs b/crates/core/src/oidc/flow.rs index 85f1e865..bdad2d3f 100644 --- a/crates/core/src/oidc/flow.rs +++ b/crates/core/src/oidc/flow.rs @@ -175,12 +175,16 @@ pub struct ValidatedLogin { /// /// `redirect_to` is the (already open-redirect-validated by the caller) relative /// path to send the user to after login; it is persisted so the completing -/// instance — possibly a *different* one — can honor it. +/// instance — possibly a *different* one — can honor it. `login_hint` (already +/// validated by the caller) is the account the user means to sign in as; it +/// goes to the IdP as the authorize request's `login_hint`, so the IdP can +/// preselect that account instead of asking for it again. pub async fn begin_login( pool: &PgPool, meta: &OidcMetadataClient, client: &ClientParams, redirect_to: &str, + login_hint: Option<&str>, ) -> Result { let discovery = meta.discovery().await?; @@ -215,6 +219,7 @@ pub async fn begin_login( &state, &nonce, &code_challenge, + login_hint, ) .map_err(|e| FlowError::TokenResponse(format!("bad authorize endpoint: {e}")))?; @@ -438,6 +443,7 @@ fn build_authorize_url( state: &str, nonce: &str, code_challenge: &str, + login_hint: Option<&str>, ) -> Result { let mut url = Url::parse(authorization_endpoint)?; { @@ -451,6 +457,9 @@ fn build_authorize_url( q.append_pair("nonce", nonce); q.append_pair("code_challenge", code_challenge); q.append_pair("code_challenge_method", "S256"); + if let Some(hint) = login_hint { + q.append_pair("login_hint", hint); + } } Ok(url.to_string()) } @@ -525,6 +534,7 @@ mod tests { "the-state", "the-nonce", "the-challenge", + None, ) .unwrap(); let parsed = Url::parse(&url).unwrap(); @@ -539,6 +549,37 @@ mod tests { assert_eq!(pairs["redirect_uri"], client.redirect_url); // The secret is never in the authorize URL. assert!(!url.contains("shh")); + // Without a hint, the IdP gets none. + assert!(!pairs.contains_key("login_hint")); + } + + #[test] + fn authorize_url_carries_the_login_hint_encoded() { + let client = ClientParams { + client_id: "client-abc".into(), + auth: ClientAuth::Secret("shh".into()), + redirect_url: "https://forge.test/auth/oidc/callback".into(), + scopes: vec!["openid".into()], + leeway_seconds: 60, + }; + let url = build_authorize_url( + "https://idp.test/authorize?tenant=t1", + &client, + "s", + "n", + "c", + Some("alice+ci@corp.example&prompt=none"), + ) + .unwrap(); + let parsed = Url::parse(&url).unwrap(); + let pairs: Vec<(String, String)> = parsed.query_pairs().into_owned().collect(); + // One parameter, encoded: it cannot add parameters of its own. + let hints: Vec<_> = pairs.iter().filter(|(k, _)| k == "login_hint").collect(); + assert_eq!(hints.len(), 1, "{url}"); + assert_eq!(hints[0].1, "alice+ci@corp.example&prompt=none"); + assert!(!pairs.iter().any(|(k, _)| k == "prompt"), "{url}"); + // The endpoint's own parameters are kept. + assert!(pairs.iter().any(|(k, v)| k == "tenant" && v == "t1")); } #[test] diff --git a/crates/core/tests/oidc_flow.rs b/crates/core/tests/oidc_flow.rs index 0411d0d6..762c18fd 100644 --- a/crates/core/tests/oidc_flow.rs +++ b/crates/core/tests/oidc_flow.rs @@ -196,7 +196,7 @@ async fn begin_login_persists_flow_and_builds_authorize_url() { let pool = pool().await; let idp = MockIdp::start().await; - let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/packages") + let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/packages", None) .await .expect("begin_login"); @@ -233,7 +233,7 @@ async fn complete_login_happy_path_then_replay_fails() { let n = unique(); let sub = format!("sub-{n}"); - let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/home") + let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/home", None) .await .expect("begin"); @@ -310,7 +310,7 @@ async fn complete_login_authenticates_with_a_client_assertion_instead_of_a_secre ..idp.client() }; - let begun = begin_login(&pool, &idp.meta(), &client, "/") + let begun = begin_login(&pool, &idp.meta(), &client, "/", None) .await .expect("begin"); let nonce = peek_nonce(&pool, &begun.state).await; @@ -365,7 +365,7 @@ async fn complete_login_fails_without_calling_the_idp_when_the_assertion_cannot_ auth: ClientAuth::Assertion(std::sync::Arc::new(FixedAssertion(Err("no keys")))), ..idp.client() }; - let begun = begin_login(&pool, &idp.meta(), &client, "/") + let begun = begin_login(&pool, &idp.meta(), &client, "/", None) .await .expect("begin"); @@ -395,7 +395,7 @@ async fn complete_login_rejects_nonce_mismatch() { let idp = MockIdp::start().await; let n = unique(); - let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/") + let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/", None) .await .expect("begin"); @@ -451,7 +451,7 @@ async fn complete_login_rejects_other_browser_without_consuming_flow() { let pool = pool().await; let idp = MockIdp::start().await; let n = unique(); - let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/") + let begun = begin_login(&pool, &idp.meta(), &idp.client(), "/", None) .await .expect("begin"); @@ -1156,7 +1156,7 @@ async fn run_login( username: &str, roles: Value, ) -> oidc::ValidatedLogin { - let begun = begin_login(pool, &idp.meta(), &idp.client(), "/") + let begun = begin_login(pool, &idp.meta(), &idp.client(), "/", None) .await .expect("begin"); let nonce = peek_nonce(pool, &begun.state).await; diff --git a/crates/db/src/external_identity_dao.rs b/crates/db/src/external_identity_dao.rs index c445e34e..d8ecdc36 100644 --- a/crates/db/src/external_identity_dao.rs +++ b/crates/db/src/external_identity_dao.rs @@ -72,6 +72,18 @@ pub async fn user_has_identity(pool: &PgPool, user_id: i64) -> sqlx::Result sqlx::Result> { + let name = sqlx::query_scalar!( + "SELECT display_name FROM external_identity WHERE id = $1", + id, + ) + .fetch_optional(pool) + .await?; + Ok(name.flatten()) +} + /// Find the identity row for `(issuer, subject)`, if any. pub async fn find_by_issuer_subject( pool: &PgPool, diff --git a/crates/server/src/auth/mod.rs b/crates/server/src/auth/mod.rs index 9bf3faa8..5be7de71 100644 --- a/crates/server/src/auth/mod.rs +++ b/crates/server/src/auth/mod.rs @@ -15,8 +15,9 @@ //! * `POST /auth/identify` — identifier-first routing, //! decided ONLY by public realm config (anti-enumeration). //! * `GET /auth/login-options` — what the login screen should render. -//! * `GET /auth/oidc/login` — start the auth-code+PKCE flow (302 → IdP); -//! 404 when no enabled oidc realm exists. +//! * `GET /auth/oidc/login` — start the auth-code+PKCE flow (302 → IdP, +//! with the optional `login_hint` the login screen sends); 404 when no +//! enabled oidc realm exists. //! * `GET /auth/oidc/callback` — complete it, set the session cookie, 302 to a //! validated relative target. //! * `POST /auth/logout` — revoke the session + clear the cookie (CSRF @@ -55,7 +56,7 @@ use forge_core::oidc::{ use forge_core::session::{csrf_token_matches, IssuedSession, SessionService}; use forge_db::model::{AppUser, Session}; use forge_db::token_dao::{self, NewToken}; -use forge_db::user_dao; +use forge_db::{external_identity_dao, user_dao}; use serde::{Deserialize, Serialize}; use crate::config::OidcConfig; @@ -177,6 +178,29 @@ pub struct LoginQuery { /// Optional post-login relative path; sanitized to defeat open redirects. #[serde(default)] pub redirect_to: Option, + /// Optional account the user means to sign in as (the identifier typed + /// on the login screen, or a remembered account's): passed to the IdP as + /// `login_hint` once [`sanitize_login_hint`] accepts it. + #[serde(default)] + pub login_hint: Option, +} + +/// Longest accepted `login_hint` (bytes): an email address or UPN fits many +/// times over. +pub const MAX_LOGIN_HINT_BYTES: usize = 256; + +/// The `login_hint` to pass to the IdP: `raw` trimmed, when it is not empty, +/// at most [`MAX_LOGIN_HINT_BYTES`] and free of control characters. Anything +/// else is dropped rather than refused: the hint only saves the user typing +/// their account at the IdP again, and the sign-in works without it. +fn sanitize_login_hint(raw: Option<&str>) -> Option<&str> { + let hint = raw.map(str::trim).filter(|h| !h.is_empty())?; + if hint.len() > MAX_LOGIN_HINT_BYTES || hint.chars().any(char::is_control) { + // It is an email address: never above debug, and only its size. + tracing::debug!(bytes = hint.len(), "oidc login_hint dropped"); + return None; + } + Some(hint) } /// Build the `/auth` router. Always mounted: sessions exist for local @@ -250,8 +274,9 @@ async fn oidc_login( }; let rt = &orr.runtime; let redirect_to = sanitize_redirect_to(q.redirect_to.as_deref()); + let login_hint = sanitize_login_hint(q.login_hint.as_deref()); - match begin_login(&state.pool, &rt.meta, &rt.client, &redirect_to).await { + match begin_login(&state.pool, &rt.meta, &rt.client, &redirect_to, login_hint).await { // Bind the flow to this browser: the callback must present the cookie. Ok(begun) => ( StatusCode::FOUND, @@ -448,8 +473,26 @@ async fn oidc_callback_inner( forge_core::authz::invalidate_user(&state.cache, user.id).await; } + // Link the session to the identity that signed in (`/auth/me` reports its + // display name). Display only, so a failed lookup leaves it unlinked. + let identity_id = external_identity_dao::find_by_issuer_subject( + &state.pool, + &validated.issuer, + &validated.subject, + ) + .await + .unwrap_or_else(|e| { + tracing::warn!(error = %e, "oidc identity lookup failed"); + None + }) + .map(|identity| identity.id); + // Create a fresh session (login always mints a new row — fixation-proof). - let issued = match rt.sessions.create_for_user(user.id, None, None).await { + let issued = match rt + .sessions + .create_for_user(user.id, identity_id, None) + .await + { Ok(s) => s, Err(e) => { tracing::error!(error = %e, "session creation failed"); @@ -662,6 +705,10 @@ async fn require_session_csrf( struct AuthMe { user_id: i64, username: String, + /// The name the identity provider gave for the user, for a session an + /// OIDC sign-in opened (`null` otherwise, and when it sent none). Display + /// only: the login screen's remembered accounts show it. + display_name: Option, is_admin: bool, /// Per-session double-submit CSRF secret (echo it in `X-CSRF-Token`). csrf_token: String, @@ -687,9 +734,20 @@ async fn me(State(state): State, headers: HeaderMap) -> Response { method: forge_core::auth::AuthMethod::Session, }) .await; + let display_name = match session.external_identity_id { + Some(id) => external_identity_dao::display_name(&state.pool, id) + .await + .unwrap_or_else(|e| { + // Display only: answer without it rather than fail the session. + tracing::warn!(error = %e, "display name lookup failed"); + None + }), + None => None, + }; Json(AuthMe { user_id: u.id, username: u.username, + display_name, is_admin, csrf_token: session.csrf_secret, }) @@ -1147,6 +1205,7 @@ async fn password_login( Json(AuthMe { user_id: principal.user_id, username, + display_name: None, is_admin: state .authz .is_global_admin(&forge_core::auth::Principal::from_user(principal)) @@ -1412,6 +1471,10 @@ struct LoginOptions { password: bool, /// Enabled OIDC realm buttons (at most one). oidc: Vec, + /// Whether the login screen remembers the accounts signed in on the + /// device and offers them (the `remember_accounts` auth setting). The + /// accounts are kept in the browser only; the server never lists any. + remember_accounts: bool, } /// `GET /auth/login-options` — public, unauthenticated: the login screen asks @@ -1432,16 +1495,12 @@ async fn login_options(State(state): State) -> Response { }] }) .unwrap_or_default(); + let settings = &state.auth_settings.current().await.settings; Json(LoginOptions { - mode: state - .auth_settings - .current() - .await - .settings - .login_mode - .as_str(), + mode: settings.login_mode.as_str(), password: snap.chain.is_some(), oidc, + remember_accounts: settings.remember_accounts, }) .into_response() } @@ -1588,6 +1647,29 @@ mod tests { assert!(clear.contains("Path=/;")); } + #[test] + fn login_hint_is_bounded_and_printable() { + assert_eq!(sanitize_login_hint(None), None); + assert_eq!(sanitize_login_hint(Some("")), None); + assert_eq!(sanitize_login_hint(Some(" ")), None); + assert_eq!( + sanitize_login_hint(Some(" alice@corp.example ")), + Some("alice@corp.example") + ); + // Not ASCII-only: a UPN may hold any printable character. + assert_eq!( + sanitize_login_hint(Some("jürgen@bücher.example")), + Some("jürgen@bücher.example") + ); + for bad in ["alice\n@corp.example", "alice\u{0}", "a\tb", "\u{7f}x"] { + assert_eq!(sanitize_login_hint(Some(bad)), None, "{bad:?}"); + } + let fits = "a".repeat(MAX_LOGIN_HINT_BYTES); + assert_eq!(sanitize_login_hint(Some(&fits)), Some(fits.as_str())); + let long = "a".repeat(MAX_LOGIN_HINT_BYTES + 1); + assert_eq!(sanitize_login_hint(Some(&long)), None); + } + #[test] fn clear_cookie_expires_immediately() { let v = clear_cookie_value(true); diff --git a/crates/server/src/auth/settings.rs b/crates/server/src/auth/settings.rs index 20d4dd01..c904d512 100644 --- a/crates/server/src/auth/settings.rs +++ b/crates/server/src/auth/settings.rs @@ -1,7 +1,8 @@ //! Sign-in and anonymous-access policy, set at runtime (#294). //! -//! The anonymous mode, the login screen's mode, password Basic -//! authentication, the session lifetimes and the password lockout are policy +//! The anonymous mode, the login screen's mode and its remembered accounts, +//! password Basic authentication, the session lifetimes and the password +//! lockout are policy //! an administrator changes while Forge runs, and every instance must //! apply the same values. They are kept in `forge_setting` under //! [`AUTH_SETTINGS_KEY`] as one JSON document ([`AuthSettings`]), changed @@ -20,8 +21,9 @@ //! (lockout limits). //! //! An install that never saved them uses [`AuthSettings::default`]: no -//! anonymous access, the identifier-first login screen, no password Basic -//! authentication, 8 h / 7 d sessions and 10 failures per 5 minutes. When the +//! anonymous access, the identifier-first login screen remembering accounts, +//! no password Basic authentication, 8 h / 7 d sessions and 10 failures per 5 +//! minutes. When the //! row cannot be read, an instance keeps the settings it has (the defaults //! before the first read) and tries again after the TTL. @@ -137,6 +139,10 @@ pub struct AuthSettings { pub login_max_failures: u32, /// Length (seconds) of the failure window and the lockout. pub login_failure_window_seconds: u64, + /// Whether the login screen remembers the accounts signed in on a + /// device and offers them for the next sign-in. The browser keeps them; + /// `false` (a kiosk, a shared terminal) has it keep and show none. + pub remember_accounts: bool, } impl Default for AuthSettings { @@ -149,6 +155,7 @@ impl Default for AuthSettings { session_absolute_ttl_seconds: 7 * 24 * 60 * 60, login_max_failures: 10, login_failure_window_seconds: 300, + remember_accounts: true, } } } @@ -216,6 +223,10 @@ impl AuthSettings { "login_failure_window_seconds", self.login_failure_window_seconds != other.login_failure_window_seconds, ); + diff( + "remember_accounts", + self.remember_accounts != other.remember_accounts, + ); keys } } @@ -233,6 +244,7 @@ pub struct AuthSettingsUpdate { pub session_absolute_ttl_seconds: Option, pub login_max_failures: Option, pub login_failure_window_seconds: Option, + pub remember_accounts: Option, } impl AuthSettingsUpdate { @@ -260,6 +272,9 @@ impl AuthSettingsUpdate { if let Some(window) = self.login_failure_window_seconds { next.login_failure_window_seconds = window; } + if let Some(remember) = self.remember_accounts { + next.remember_accounts = remember; + } next.validate()?; Ok(next) } @@ -496,11 +511,14 @@ mod tests { assert_eq!(settings.validate().unwrap(), AnonPolicy::Off); assert!(!settings.allow_password_basic); assert_eq!(settings.login_mode, LoginMode::IdentifierFirst); - // An empty or partial stored document takes the defaults. + assert!(settings.remember_accounts); + // An empty or partial stored document takes the defaults, so a row + // saved before `remember_accounts` existed remembers accounts. let stored: AuthSettings = serde_json::from_str(r#"{"anonymous_mode":"all","removed_later":1}"#).unwrap(); assert_eq!(stored.anonymous_mode, AnonymousMode::All); assert_eq!(stored.session_idle_ttl_seconds, 8 * 60 * 60); + assert!(stored.remember_accounts); } #[test] @@ -531,11 +549,17 @@ mod tests { let update: AuthSettingsUpdate = serde_json::from_value(serde_json::json!({ "anonymous_mode": " All", "login_mode": "combined", + "remember_accounts": false, })) .unwrap(); let next = update.apply_to(¤t).unwrap(); assert_eq!(next.anonymous_mode, AnonymousMode::All); assert_eq!(next.login_mode, LoginMode::Combined); + assert!(!next.remember_accounts); + assert_eq!( + current.changed_keys(&next).last(), + Some(&"remember_accounts") + ); assert!(next.allow_password_basic, "left out, kept"); assert_eq!(next.validate().unwrap(), AnonPolicy::All); } diff --git a/crates/server/tests/auth_settings.rs b/crates/server/tests/auth_settings.rs index aab27422..d23d793a 100644 --- a/crates/server/tests/auth_settings.rs +++ b/crates/server/tests/auth_settings.rs @@ -87,18 +87,24 @@ async fn read_index(srv: &TestServer, repo: &str, basic: Option<(&str, &str)>) - req.send().await.unwrap().status().as_u16() } -/// The login screen's mode (`GET /auth/login-options`). -async fn login_mode(srv: &TestServer) -> String { - let options: Value = srv - .client() +/// What the login screen renders (`GET /auth/login-options`). +async fn login_options(srv: &TestServer) -> Value { + srv.client() .get(format!("{}/auth/login-options", srv.base_url)) .send() .await .unwrap() .json() .await - .unwrap(); - options["mode"].as_str().expect("mode").to_string() + .unwrap() +} + +/// The login screen's mode (`GET /auth/login-options`). +async fn login_mode(srv: &TestServer) -> String { + login_options(srv).await["mode"] + .as_str() + .expect("mode") + .to_string() } /// Admins read and change the settings; a change names only what it changes, @@ -119,6 +125,7 @@ async fn admins_read_and_change_the_settings_and_wrong_values_are_refused() { "session_absolute_ttl_seconds": 604800, "login_max_failures": 10, "login_failure_window_seconds": 300, + "remember_accounts": true, }) ); @@ -147,6 +154,20 @@ async fn admins_read_and_change_the_settings_and_wrong_values_are_refused() { assert_eq!(get_settings(&srv).await, saved); assert_eq!(login_mode(&srv).await, "combined", "applied at once"); + // The login screen remembers accounts until an administrator turns it + // off (a kiosk); the screen learns it from the login options. + assert_eq!(login_options(&srv).await["remember_accounts"], true); + let resp = put_settings(&srv, json!({"remember_accounts": false})).await; + assert_eq!(resp.status(), 200); + let saved: Value = resp.json().await.unwrap(); + assert_eq!(saved["remember_accounts"], false); + assert_eq!(saved["login_mode"], "combined", "left out, kept"); + assert_eq!( + login_options(&srv).await["remember_accounts"], + false, + "applied at once" + ); + for (body, message) in [ ( json!({"anonymous_mode": "allowlist"}), diff --git a/crates/server/tests/oidc_routes.rs b/crates/server/tests/oidc_routes.rs index 2ebb928c..c8cfff85 100644 --- a/crates/server/tests/oidc_routes.rs +++ b/crates/server/tests/oidc_routes.rs @@ -354,6 +354,74 @@ async fn login_rejects_open_redirect_target() { assert_eq!(row.redirect_to, "/", "absolute URL normalised to root"); } +/// The `login_hint` query parameter of an authorize URL, if any. +fn login_hint_from_url(url: &str) -> Option { + url::Url::parse(url) + .expect("authorize url") + .query_pairs() + .find(|(k, _)| k == "login_hint") + .map(|(_, v)| v.into_owned()) +} + +/// `login_hint` (the account typed on the login screen, or a remembered +/// one's) reaches the IdP's authorize request so it can preselect the +/// account; one that is too long or holds control characters is dropped, and +/// the sign-in still starts without it. +#[tokio::test] +async fn login_passes_a_valid_login_hint_to_the_idp() { + let _serial = SERIAL.lock().await; + let idp = MockIdp::start().await; + let srv = start_with( + common::pool().await, + ServerOpts { + bootstrap_admin: Some(( + format!("admin{}@example.com", common::unique()), + "adminpw".into(), + )), + oidc_block: Some(idp.oidc_block()), + ..Default::default() + }, + ) + .await; + let start = |query: String| { + let url = format!("{}/auth/oidc/login?{query}", srv.base_url); + let authorize = format!("{}/authorize", idp.issuer); + async move { + let resp = no_redirect_client().get(url).send().await.unwrap(); + assert_eq!(resp.status(), 302, "the sign-in starts"); + let location = resp.headers().get("location").unwrap().to_str().unwrap(); + assert!(location.starts_with(&authorize), "{location}"); + login_hint_from_url(location) + } + }; + + // Encoded on the way in and out: `&` and `+` stay part of the hint. + assert_eq!( + start("login_hint=alice%2Bci%40corp.example%26prompt%3Dnone&redirect_to=/browse".into()) + .await + .as_deref(), + Some("alice+ci@corp.example&prompt=none") + ); + assert_eq!(start("redirect_to=/browse".into()).await, None, "none sent"); + assert_eq!(start("login_hint=%20%20".into()).await, None, "blank"); + assert_eq!( + start("login_hint=alice%0A%40corp.example".into()).await, + None, + "a control character" + ); + let at_bound = format!("{}@x", "a".repeat(254)); + assert_eq!( + start(format!("login_hint={at_bound}")).await.as_deref(), + Some(at_bound.as_str()), + "256 bytes" + ); + assert_eq!( + start(format!("login_hint={}@x", "a".repeat(255))).await, + None, + "257 bytes" + ); +} + // ── callback happy path + /admin/me ────────────────────────────────────────── #[tokio::test] @@ -1004,6 +1072,8 @@ async fn auth_me_returns_identity_and_csrf_token() { .await .unwrap(); assert_eq!(body["username"], json!(username)); + // The IdP's `name` claim, for the login screen's remembered account. + assert_eq!(body["display_name"], json!(format!("User {username}"))); assert_eq!(body["is_admin"], json!(false), "no admin role claim"); assert!(!csrf.is_empty(), "csrf token present"); } diff --git a/docs/changes/2.2.0/115.feature.md b/docs/changes/2.2.0/115.feature.md new file mode 100644 index 00000000..0f85f4f0 --- /dev/null +++ b/docs/changes/2.2.0/115.feature.md @@ -0,0 +1,9 @@ +The login screen remembers the accounts signed in on a device and offers them +the next time, like Microsoft's "Pick an account": one click continues at the +password step, or at the identity provider, which now gets the account as +`login_hint` and does not ask for it again (the email typed into the +identifier-first form is passed the same way). **Use another account** shows +the usual form, and an account can be forgotten. The browser keeps at most +five, never a password or a session. Kiosks and shared computers turn it off +with **Remember accounts on the login screen** under **Admin → Settings** +(`remember_accounts` in `/admin/settings/auth`). diff --git a/docs/deployment.md b/docs/deployment.md index 0d9197e5..92f209f2 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -532,6 +532,7 @@ wrong. | --- | --- | --- | | `anonymous_mode` | `off` | `off` or `all`: who is served without credentials. A repository is read anonymously only when it also allows it (**Allow anonymous read**, or a grant of the `anonymous` role). Under `all`, signed-in users also read what anonymous callers may. To let a network read without credentials, have the load balancer name a service account through a [trusted-header realm](#trusted-header-realms). | | `login_mode` | `identifier_first` | The login screen: the username first (then a password prompt or the identity provider whose login domains match), or `combined` (username and password, plus the OIDC button). | +| `remember_accounts` | `true` | The login screen offers the accounts signed in on the device before, so signing in again is one click (see below). `false` for kiosks and shared computers. | | `allow_password_basic` | `false` | Accept a user's password over HTTP Basic on the feeds (see below). | | `session_idle_ttl_seconds` | `28800` (8 h) | A browser session ends this long after its last use. | | `session_absolute_ttl_seconds` | `604800` (7 d) | A browser session ends this long after sign-in. New lifetimes apply to sessions signed in afterwards; an open session's idle window moves to the new idle timeout on its next use. | @@ -542,6 +543,26 @@ A configuration that still sets one of these (`auth.anonymous_mode`, `FORGE_AUTH__ALLOW_PASSWORD_BASIC`, …, or the `oidc.session_*` lifetimes) is refused at startup, with a message naming the keys to remove. +#### Remembered accounts + +After someone signs in, the login screen shows their account the next time +instead of an empty username field, like Microsoft's "Pick an account". A +click on it continues as that account: a password account at the password +step, an OIDC account at its identity provider, which gets the username as +`login_hint` and so does not ask for the account again (the username typed +into the identifier-first form is passed the same way). **Use another +account** shows the usual form, and each account's ✕ forgets it. Signing out +keeps the account; the browser keeps at most five, the latest first. + +The accounts are kept in the browser only (`localStorage`): the username, the +name the identity provider gave, the realm and when it was last used, never a +password, a token or the session. The server never lists accounts, so +`POST /auth/identify` answers as before, from the realms' configuration alone. +An account whose OIDC realm was disabled or renamed is routed like a typed +username. With `remember_accounts` off (**Admin → Settings → Remember accounts +on the login screen**), the login screen keeps none and forgets those a +browser kept, the next time it is opened there. + #### Password Basic auth `allow_password_basic` is off by default: the web UI signs in through its diff --git a/web/e2e/specs/auth.spec.ts b/web/e2e/specs/auth.spec.ts index 3ffd6664..2aaa106a 100644 --- a/web/e2e/specs/auth.spec.ts +++ b/web/e2e/specs/auth.spec.ts @@ -121,6 +121,34 @@ test('the admin route guard redirects a non-admin away from /manage (hard nav)', await expect(page.getByRole('heading', { name: /^browse$/i })).toBeVisible() }) +test('a remembered account signs in again from its tile after sign-out', async ({ + page, +}) => { + const f = facts() + await uiSignIn(page, f.adminUser, f.adminPass) + await expect(page.getByRole('heading', { name: /^browse$/i })).toBeVisible() + await page.getByRole('button', { name: /sign out/i }).click() + await expect(page).toHaveURL(/\/login/) + + // The login screen offers the account instead of an empty username field. + const accounts = page.getByRole('list', { name: 'Remembered accounts' }) + const escaped = f.adminUser.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + const tile = accounts.getByRole('button', { name: new RegExp(`^${escaped}`) }) + await expect(tile).toBeVisible() + await expect(page.getByLabel(/^username$/i)).toHaveCount(0) + // What the browser keeps names the account; it works as no credential. + const stored = await page.evaluate(() => window.localStorage.getItem('forge.accounts')) + expect(stored).toContain(f.adminUser) + expect(stored).not.toContain(f.adminPass) + + // One click reaches the password step for that account. + await tile.click() + const passwordField = page.getByLabel(/^password$/i) + await passwordField.fill(f.adminPass) + await page.getByRole('button', { name: /^sign in$/i }).click() + await expect(page.getByRole('heading', { name: /^browse$/i })).toBeVisible() +}) + test('sign out invalidates the session and returns to login', async ({ page, }) => { diff --git a/web/src/api/auth.ts b/web/src/api/auth.ts index f9620c8a..7403cf3f 100644 --- a/web/src/api/auth.ts +++ b/web/src/api/auth.ts @@ -1,5 +1,6 @@ import { queryOptions } from '@tanstack/react-query' +import { completePendingSignIn } from '~/lib/accounts' import { getCsrfToken, setSession } from '~/lib/auth' import { httpClient } from '~/lib/httpClient' @@ -35,6 +36,11 @@ export interface LoginOptions { /** Whether any password-capable realm is enabled (renders the password form). */ password: boolean oidc: OidcLoginOption[] + /** + * Whether the screen remembers the accounts signed in on this device and + * offers them (`~/lib/accounts`). Off, it keeps and shows none. + */ + remember_accounts: boolean } /** `POST /auth/identify` response — where to send this username. */ @@ -49,6 +55,8 @@ export interface IdentifyResponse { export interface LoginResponse { user_id: number username: string + /** Always null for a password sign-in (only an IdP gives a name). */ + display_name?: string | null is_admin: boolean csrf_token: string } @@ -57,6 +65,8 @@ export interface LoginResponse { export interface AuthSession { user_id: number username: string + /** The identity provider's name for the user, for an OIDC session. */ + display_name?: string | null is_admin: boolean csrf_token: string } @@ -112,7 +122,9 @@ export function fetchAuthSession( * An OIDC sign-in ends with a server redirect that only sets the httpOnly * cookie, and a token stored by an earlier session can be stale, so the app * shell calls this once per load: without the current token every mutating - * request (and logout) would be refused. Best-effort — any failure resolves + * request (and logout) would be refused. It is also the first identity the + * SPA sees after an OIDC sign-in, so it remembers that account for the login + * screen (`completePendingSignIn`). Best-effort — any failure resolves * to `null` and leaves the stored token alone (a 401 clears it). A 401 is * not reported as an expired session: the app shell calls this only once * `/admin/me` answered, and a user the proxy in front signs in (a @@ -124,6 +136,9 @@ export async function syncAuthSession( try { const session = await fetchAuthSession(signal, true) if (getCsrfToken() !== session.csrf_token) setSession(session.csrf_token) + // The first identity after an OIDC round trip this tab started: remember + // the account for the login screen. + completePendingSignIn(session) return session } catch { return null diff --git a/web/src/api/settings.ts b/web/src/api/settings.ts index a88fdfce..0ed5d13a 100644 --- a/web/src/api/settings.ts +++ b/web/src/api/settings.ts @@ -21,6 +21,8 @@ export type LoginMode = 'identifier_first' | 'combined' export interface AuthSettings { anonymous_mode: AnonymousMode login_mode: LoginMode + /** The login screen offers the accounts signed in on the device before. */ + remember_accounts: boolean /** Accept a user's password over HTTP Basic on the feeds. */ allow_password_basic: boolean session_idle_ttl_seconds: number diff --git a/web/src/lib/accounts.test.ts b/web/src/lib/accounts.test.ts new file mode 100644 index 00000000..d840d389 --- /dev/null +++ b/web/src/lib/accounts.test.ts @@ -0,0 +1,168 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + MAX_ACCOUNTS, + beginOidcSignIn, + clearPendingSignIn, + completePendingSignIn, + forgetAccount, + forgetAllAccounts, + listAccounts, + rememberAccount, +} from './accounts' + +/** + * The remembered-accounts store: newest first, at most five, one entry per + * account whatever the case of its identifier, nothing but the identifier, + * name, method and realm kept, and every access survives blocked storage. + * An OIDC sign-in is remembered from the pending marker the login screen + * leaves, by the first `/auth/me` after the round trip. + */ +describe('remembered accounts', () => { + beforeEach(() => { + localStorage.clear() + sessionStorage.clear() + }) + + afterEach(() => { + vi.restoreAllMocks() + localStorage.clear() + sessionStorage.clear() + }) + + it('keeps the newest first and updates an account signed in again', () => { + rememberAccount({ identifier: 'alice@corp.example', method: 'password', realm: 'local' }, 1) + rememberAccount({ identifier: 'bob@corp.example', method: 'oidc', realm: 'corp' }, 2) + expect(listAccounts().map((a) => a.identifier)).toEqual([ + 'bob@corp.example', + 'alice@corp.example', + ]) + + // The same account, typed differently, replaces its entry and moves up. + rememberAccount({ identifier: 'Alice@Corp.example', method: 'oidc', realm: 'corp' }, 3) + expect(listAccounts()).toEqual([ + { identifier: 'Alice@Corp.example', method: 'oidc', realm: 'corp', last_used: 3 }, + { identifier: 'bob@corp.example', method: 'oidc', realm: 'corp', last_used: 2 }, + ]) + }) + + it(`keeps at most ${MAX_ACCOUNTS}, dropping the oldest`, () => { + for (let i = 1; i <= MAX_ACCOUNTS + 2; i++) { + rememberAccount({ identifier: `user${i}@corp.example`, method: 'password' }, i) + } + const kept = listAccounts().map((a) => a.identifier) + expect(kept).toHaveLength(MAX_ACCOUNTS) + expect(kept[0]).toBe(`user${MAX_ACCOUNTS + 2}@corp.example`) + expect(kept).not.toContain('user1@corp.example') + expect(kept).not.toContain('user2@corp.example') + }) + + it('forgets one account, or all of them', () => { + rememberAccount({ identifier: 'alice@corp.example', method: 'password' }, 1) + rememberAccount({ identifier: 'bob@corp.example', method: 'password' }, 2) + forgetAccount('ALICE@corp.example') + expect(listAccounts().map((a) => a.identifier)).toEqual(['bob@corp.example']) + + beginOidcSignIn('corp') + forgetAllAccounts() + expect(listAccounts()).toEqual([]) + expect(localStorage.getItem('forge.accounts')).toBeNull() + // A pending sign-in goes too: nothing is remembered afterwards. + completePendingSignIn({ username: 'carol@corp.example' }) + expect(listAccounts()).toEqual([]) + }) + + it('stores nothing that works as a credential', () => { + rememberAccount({ + identifier: 'alice@corp.example', + display_name: 'Alice Example', + method: 'password', + realm: 'local', + // A caller passing more than an entry holds (a session, a password). + ...({ csrf_token: 'csrf-secret', password: 'hunter2' } as object), + }) + const raw = localStorage.getItem('forge.accounts') ?? '' + expect(raw).not.toContain('csrf-secret') + expect(raw).not.toContain('hunter2') + expect(Object.keys(JSON.parse(raw)[0]).sort()).toEqual([ + 'display_name', + 'identifier', + 'last_used', + 'method', + 'realm', + ]) + }) + + it('reads a damaged or foreign value as no accounts', () => { + for (const raw of ['not json', '{"identifier":"x"}', '42', 'null']) { + localStorage.setItem('forge.accounts', raw) + expect(listAccounts()).toEqual([]) + } + localStorage.setItem( + 'forge.accounts', + JSON.stringify([ + { identifier: 'ok@corp.example', method: 'oidc', realm: 'corp', last_used: 1 }, + { identifier: 'x', method: 'token', last_used: 2 }, + { identifier: '', method: 'password', last_used: 3 }, + { identifier: 'y'.repeat(300), method: 'password', last_used: 4 }, + 'alice', + ]), + ) + expect(listAccounts().map((a) => a.identifier)).toEqual(['ok@corp.example']) + }) + + it('works without storage: nothing is listed and nothing throws', () => { + const blocked = () => { + throw new DOMException('blocked', 'SecurityError') + } + vi.spyOn(Storage.prototype, 'getItem').mockImplementation(blocked) + vi.spyOn(Storage.prototype, 'setItem').mockImplementation(blocked) + vi.spyOn(Storage.prototype, 'removeItem').mockImplementation(blocked) + + expect(() => + rememberAccount({ identifier: 'alice@corp.example', method: 'password' }), + ).not.toThrow() + expect(listAccounts()).toEqual([]) + expect(() => forgetAccount('alice@corp.example')).not.toThrow() + expect(() => forgetAllAccounts()).not.toThrow() + expect(() => beginOidcSignIn('corp')).not.toThrow() + expect(() => completePendingSignIn({ username: 'alice@corp.example' })).not.toThrow() + }) + + describe('an OIDC sign-in', () => { + it('is remembered by the first identity after the round trip, once', () => { + beginOidcSignIn('corp', 1_000) + completePendingSignIn({ username: 'alice@corp.example', display_name: 'Alice' }, 2_000) + expect(listAccounts()).toEqual([ + { + identifier: 'alice@corp.example', + display_name: 'Alice', + method: 'oidc', + realm: 'corp', + last_used: 2_000, + }, + ]) + // Later identities (every app load) do not touch it again. + forgetAccount('alice@corp.example') + completePendingSignIn({ username: 'alice@corp.example' }, 3_000) + expect(listAccounts()).toEqual([]) + }) + + it('is not remembered when it is abandoned or too old', () => { + beginOidcSignIn('corp', 1_000) + clearPendingSignIn() + completePendingSignIn({ username: 'alice@corp.example' }, 2_000) + expect(listAccounts()).toEqual([]) + + beginOidcSignIn('corp', 1_000) + completePendingSignIn({ username: 'alice@corp.example' }, 1_000 + 11 * 60 * 1000) + expect(listAccounts()).toEqual([]) + }) + + it('without a display name keeps none', () => { + beginOidcSignIn('corp', 1_000) + completePendingSignIn({ username: 'alice@corp.example', display_name: null }, 1_500) + expect(listAccounts()[0]).not.toHaveProperty('display_name') + }) + }) +}) diff --git a/web/src/lib/accounts.ts b/web/src/lib/accounts.ts new file mode 100644 index 00000000..5922934b --- /dev/null +++ b/web/src/lib/accounts.ts @@ -0,0 +1,223 @@ +/** + * Remembered accounts: the accounts signed in on this device, which the login + * screen offers as a picker (like Microsoft's "Pick an account") so the next + * sign-in is one click. + * + * Kept in localStorage under `forge.accounts`, newest first, at most + * {@link MAX_ACCOUNTS}. An entry holds what the picker shows and how to + * continue: the identifier, the identity provider's display name, the sign-in + * method with its realm, and when it was last used. **Never** a password, a + * token, the session or anything else that works as a credential: an entry + * only saves typing the identifier again. The server never lists accounts, + * so `POST /auth/identify` still answers from public realm configuration + * alone. + * + * Every access is wrapped: with storage blocked (a private window, a policy) + * the list reads as empty and nothing is kept, so the picker is simply not + * shown. A stored value that is not a list of entries reads as empty too. + * + * An OIDC sign-in ends with a server redirect, so the login screen cannot + * record its entry: it leaves a pending sign-in in sessionStorage (this tab + * only, {@link beginOidcSignIn}), and the first `GET /auth/me` after the + * round trip turns it into an entry ({@link completePendingSignIn}). + */ + +const ACCOUNTS_KEY = 'forge.accounts' +const PENDING_KEY = 'forge.accounts.pending' + +/** The most accounts kept; signing in with a sixth drops the oldest. */ +export const MAX_ACCOUNTS = 5 + +/** Longest identifier, display name or realm name kept. */ +const MAX_FIELD_CHARS = 256 + +/** + * A pending OIDC sign-in older than this is dropped: the server's flow ends + * after 10 minutes, so a later session was not opened by it. + */ +const PENDING_TTL_MS = 10 * 60 * 1000 + +/** How a remembered account signs in. */ +export type AccountMethod = 'password' | 'oidc' + +/** One remembered account. */ +export interface RememberedAccount { + /** What the user signs in with: the username (usually an email address). */ + identifier: string + /** The name the identity provider gave for the user, when it gave one. */ + display_name?: string + method: AccountMethod + /** The realm that signed the account in, when the screen learned it. */ + realm?: string + /** When the account last signed in (epoch milliseconds). */ + last_used: number +} + +/** An OIDC sign-in this tab started (sessionStorage). */ +interface PendingSignIn { + realm: string + started: number +} + +/** + * The key two identifiers of one account share: Forge keeps usernames + * trimmed, NFC-normalised and lowercased, so `Alice@Corp.com` and + * `alice@corp.com` are one account. + */ +function accountKey(identifier: string): string { + return identifier.trim().normalize('NFC').toLowerCase() +} + +function boundedString(v: unknown): v is string { + return typeof v === 'string' && v.trim() !== '' && v.length <= MAX_FIELD_CHARS +} + +/** `raw` as an entry, or null when it is not one (tampered, another version). */ +function parseAccount(raw: unknown): RememberedAccount | null { + if (typeof raw !== 'object' || raw === null) return null + const r = raw as Record + if (!boundedString(r.identifier)) return null + if (r.method !== 'password' && r.method !== 'oidc') return null + if (typeof r.last_used !== 'number' || !Number.isFinite(r.last_used)) return null + const account: RememberedAccount = { + identifier: r.identifier.trim(), + method: r.method, + last_used: r.last_used, + } + if (boundedString(r.display_name)) account.display_name = r.display_name.trim() + if (boundedString(r.realm)) account.realm = r.realm + return account +} + +/** The remembered accounts, newest first (empty when storage is unavailable). */ +export function listAccounts(): RememberedAccount[] { + let raw: string | null + try { + raw = localStorage.getItem(ACCOUNTS_KEY) + } catch { + return [] + } + if (!raw) return [] + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return [] + } + if (!Array.isArray(parsed)) return [] + const seen = new Set() + return parsed + .map(parseAccount) + .filter((a): a is RememberedAccount => a !== null) + .sort((a, b) => b.last_used - a.last_used) + .filter((a) => { + const key = accountKey(a.identifier) + if (seen.has(key)) return false + seen.add(key) + return true + }) + .slice(0, MAX_ACCOUNTS) +} + +function store(accounts: RememberedAccount[]): void { + try { + if (accounts.length === 0) localStorage.removeItem(ACCOUNTS_KEY) + else localStorage.setItem(ACCOUNTS_KEY, JSON.stringify(accounts)) + } catch { + /* storage unavailable: nothing is remembered */ + } +} + +/** + * Remember a sign-in: the account moves to the front with `now` as its last + * use, replacing its earlier entry (same identifier, any case). The oldest + * accounts beyond {@link MAX_ACCOUNTS} are dropped. A value too long to keep + * is not remembered. + */ +export function rememberAccount( + account: Omit, + now: number = Date.now(), +): void { + const entry = parseAccount({ ...account, last_used: now }) + if (!entry) return + const key = accountKey(entry.identifier) + const others = listAccounts().filter((a) => accountKey(a.identifier) !== key) + store([entry, ...others].slice(0, MAX_ACCOUNTS)) +} + +/** Forget one account (its "Forget" on the login screen). */ +export function forgetAccount(identifier: string): void { + const key = accountKey(identifier) + store(listAccounts().filter((a) => accountKey(a.identifier) !== key)) +} + +/** Forget every account (also what remembering turned off asks for). */ +export function forgetAllAccounts(): void { + store([]) + clearPendingSignIn() +} + +/** + * Note that this tab is leaving for the identity provider of `realm`, so the + * session the round trip opens is remembered as an OIDC account of that + * realm ({@link completePendingSignIn}). + */ +export function beginOidcSignIn(realm: string, now: number = Date.now()): void { + const pending: PendingSignIn = { realm, started: now } + try { + sessionStorage.setItem(PENDING_KEY, JSON.stringify(pending)) + } catch { + /* storage unavailable: the account is not remembered */ + } +} + +/** Drop this tab's pending OIDC sign-in (it failed, or another one replaced it). */ +export function clearPendingSignIn(): void { + try { + sessionStorage.removeItem(PENDING_KEY) + } catch { + /* ignore */ + } +} + +function takePendingSignIn(now: number): PendingSignIn | null { + let raw: string | null + try { + raw = sessionStorage.getItem(PENDING_KEY) + } catch { + return null + } + if (!raw) return null + clearPendingSignIn() + try { + const p = JSON.parse(raw) as Partial + if (!boundedString(p.realm) || typeof p.started !== 'number') return null + const age = now - p.started + return age >= 0 && age <= PENDING_TTL_MS ? { realm: p.realm, started: p.started } : null + } catch { + return null + } +} + +/** + * Called with the session of the first `GET /auth/me` of an app load: when + * this tab started an OIDC sign-in a moment ago, remember the account it + * signed in as (by its Forge username, which the identity provider gets back + * as the login hint next time). + */ +export function completePendingSignIn( + session: { username: string; display_name?: string | null }, + now: number = Date.now(), +): void { + const pending = takePendingSignIn(now) + if (!pending) return + rememberAccount( + { + identifier: session.username, + display_name: session.display_name ?? undefined, + method: 'oidc', + realm: pending.realm, + }, + now, + ) +} diff --git a/web/src/routes/Login.test.tsx b/web/src/routes/Login.test.tsx index 3a8d2122..cd276b6b 100644 --- a/web/src/routes/Login.test.tsx +++ b/web/src/routes/Login.test.tsx @@ -9,6 +9,7 @@ import { } from '@tanstack/react-router' import { makeQueryClient } from '~/lib/queryClient' +import { listAccounts, rememberAccount } from '~/lib/accounts' import { clearSession, getCsrfToken, isAuthenticated } from '~/lib/auth' import { assignLocation } from '~/lib/navigation' import type { LoginOptions } from '~/api/auth' @@ -31,7 +32,11 @@ vi.mock('~/lib/navigation', () => ({ assignLocation: vi.fn() })) * - a failed OIDC callback (`?error=`) shows a generic message per * code, never the raw parameter, with a "Try again" link that clears it; * - there is **no** registry-token sign-in: the UI always authenticates with - * username+password or via a realm. + * username+password or via a realm; + * - accounts signed in on this device are remembered and offered as tiles + * (unless the server turns remembering off): a tile continues at the + * password step or at the IdP with `login_hint`, an OIDC account whose + * realm is gone is routed by `/auth/identify`, and a tile can be forgotten. */ const readerMe: Me = { @@ -44,6 +49,7 @@ const identifierFirst: LoginOptions = { mode: 'identifier_first', password: true, oidc: [], + remember_accounts: true, } const combined: LoginOptions = { @@ -52,6 +58,7 @@ const combined: LoginOptions = { oidc: [ { name: 'corp', label: 'Corp SSO', login_url: '/auth/oidc/login?realm=corp' }, ], + remember_accounts: true, } function json(body: unknown, status = 200): Response { @@ -132,11 +139,15 @@ describe('Login', () => { fetchMock.mockReset() vi.mocked(assignLocation).mockReset() clearSession() + localStorage.clear() + sessionStorage.clear() }) afterEach(() => { vi.unstubAllGlobals() clearSession() + localStorage.clear() + sessionStorage.clear() }) describe('identifier-first mode', () => { @@ -206,7 +217,7 @@ describe('Login', () => { expect(isAuthenticated()).toBe(false) }) - it('navigates the browser to the IdP on an oidc_redirect decision, carrying redirect_to', async () => { + it('navigates the browser to the IdP on an oidc_redirect decision, carrying redirect_to and login_hint', async () => { const user = userEvent.setup() mockAuthApi({ options: identifierFirst, @@ -223,9 +234,10 @@ describe('Login', () => { await user.type(await screen.findByLabelText('Username'), 'bob@example.com') await user.click(screen.getByRole('button', { name: 'Continue' })) + // The IdP gets the email typed here, so it does not ask for it again. await waitFor(() => expect(assignLocation).toHaveBeenCalledWith( - '/auth/oidc/login?realm=corp&redirect_to=%2Fmanage%2Frepositories', + '/auth/oidc/login?realm=corp&redirect_to=%2Fmanage%2Frepositories&login_hint=bob%40example.com', ), ) // No password step was revealed. @@ -444,6 +456,272 @@ describe('Login', () => { }) }) + describe('remembered accounts', () => { + const alice = { + identifier: 'alice@corp.example', + method: 'password' as const, + realm: 'local', + } + const bob = { + identifier: 'bob@corp.example', + display_name: 'Bob Builder', + method: 'oidc' as const, + realm: 'corp', + } + const aliceSession = { + user_id: 2, + username: 'alice@corp.example', + display_name: null, + is_admin: false, + csrf_token: 'csrf-abc', + } + /** An account's tile: the button naming it that is not its "Forget". */ + const tile = (text: string) => + screen.findByRole('button', { + name: (name) => name.includes(text) && !name.startsWith('Forget'), + }) + + it('remembers a password sign-in and offers it next time', async () => { + const user = userEvent.setup() + mockAuthApi({ + options: identifierFirst, + onIdentify: () => json({ mode: 'password', realm: 'local' }), + onLogin: () => json(aliceSession), + }) + + const router = await renderLogin() + await user.type(await screen.findByLabelText('Username'), 'alice@corp.example') + await user.click(screen.getByRole('button', { name: 'Continue' })) + await user.type(await screen.findByLabelText('Password'), 'hunter2') + await user.click(screen.getByRole('button', { name: 'Sign in' })) + await waitFor(() => expect(router.state.location.pathname).toBe('/')) + + // The account, never the password or the session. + expect(listAccounts()).toEqual([ + { ...alice, last_used: expect.any(Number) }, + ]) + expect(localStorage.getItem('forge.accounts')).not.toMatch(/hunter2|csrf-abc/) + + // Back at the login screen, it offers the account. + await router.navigate({ to: '/login' }) + expect(await tile('alice@corp.example')).toBeInTheDocument() + expect(screen.getByText('Pick an account to continue.')).toBeInTheDocument() + expect(screen.queryByLabelText('Username')).not.toBeInTheDocument() + }) + + it('continues a password account at the password step', async () => { + const user = userEvent.setup() + rememberAccount(alice) + let identified: unknown = null + let loggedIn: unknown = null + mockAuthApi({ + options: identifierFirst, + onIdentify: (body) => { + identified = body + return json({ mode: 'password', realm: 'local' }) + }, + onLogin: (body) => { + loggedIn = body + return json(aliceSession) + }, + }) + + const router = await renderLogin() + await user.click(await tile('alice@corp.example')) + + // Routed like the identifier typed into the form, then step 2. + await waitFor(() => expect(identified).toEqual({ username: 'alice@corp.example' })) + expect(await screen.findByText('alice@corp.example')).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Change' })).toBeInTheDocument() + await user.type(screen.getByLabelText('Password'), 'hunter2') + await user.click(screen.getByRole('button', { name: 'Sign in' })) + + await waitFor(() => + expect(loggedIn).toEqual({ username: 'alice@corp.example', password: 'hunter2' }), + ) + await waitFor(() => expect(router.state.location.pathname).toBe('/')) + }) + + it('continues an OIDC account at its identity provider with login_hint', async () => { + const user = userEvent.setup() + rememberAccount(bob) + mockAuthApi({ options: combined }) + + await renderLogin('/login?redirect=%2Ftokens') + // The tile shows the IdP's name for the account, and its identifier. + const bobTile = await tile('Bob Builder') + expect(bobTile).toHaveTextContent(/Bob Builder\s*bob@corp\.example\s*SSO/) + await user.click(bobTile) + + expect(assignLocation).toHaveBeenCalledWith( + '/auth/oidc/login?realm=corp&redirect_to=%2Ftokens&login_hint=bob%40corp.example', + ) + // The session the round trip opens is remembered as this realm's. + expect(sessionStorage.getItem('forge.accounts.pending')).toMatch(/"realm":"corp"/) + // The browser is leaving: no second flow can start. + expect(bobTile).toBeDisabled() + }) + + it('routes an OIDC account whose realm is gone through /auth/identify, without a loop', async () => { + const user = userEvent.setup() + rememberAccount({ ...bob, realm: 'retired' }) + let identified: unknown = null + mockAuthApi({ + options: identifierFirst, + onIdentify: (body) => { + identified = body + return json({ mode: 'password', realm: 'local' }) + }, + }) + + await renderLogin() + await user.click(await tile('bob@corp.example')) + + await waitFor(() => expect(identified).toEqual({ username: 'bob@corp.example' })) + expect(await screen.findByLabelText('Password')).toBeInTheDocument() + expect(assignLocation).not.toHaveBeenCalled() + }) + + it('follows /auth/identify to the realm that now takes the account', async () => { + const user = userEvent.setup() + rememberAccount({ ...bob, realm: 'retired' }) + mockAuthApi({ + options: identifierFirst, + onIdentify: () => + json({ + mode: 'oidc_redirect', + realm: 'corp-new', + login_url: '/auth/oidc/login', + }), + }) + + await renderLogin() + await user.click(await tile('bob@corp.example')) + + await waitFor(() => + expect(assignLocation).toHaveBeenCalledWith( + '/auth/oidc/login?login_hint=bob%40corp.example', + ), + ) + expect(sessionStorage.getItem('forge.accounts.pending')).toMatch(/"realm":"corp-new"/) + }) + + it('fills the form of combined mode for a password account', async () => { + const user = userEvent.setup() + rememberAccount(alice) + mockAuthApi({ options: combined }) + + await renderLogin() + await user.click(await tile('alice@corp.example')) + + expect(await screen.findByLabelText('Username')).toHaveValue('alice@corp.example') + expect(screen.getByLabelText('Password')).toHaveFocus() + expect(screen.getByRole('button', { name: 'Login with Corp SSO' })).toBeInTheDocument() + }) + + it('forgets an account, and shows the form once none is left', async () => { + const user = userEvent.setup() + rememberAccount(alice, 1) + rememberAccount(bob, 2) + mockAuthApi({ options: identifierFirst }) + + await renderLogin() + await user.click( + await screen.findByRole('button', { name: 'Forget bob@corp.example' }), + ) + expect(screen.queryByRole('button', { name: /Bob Builder/ })).not.toBeInTheDocument() + expect(listAccounts().map((a) => a.identifier)).toEqual(['alice@corp.example']) + + await user.click(screen.getByRole('button', { name: 'Forget alice@corp.example' })) + expect(await screen.findByLabelText('Username')).toBeInTheDocument() + expect(listAccounts()).toEqual([]) + }) + + it('"Use another account" shows the form, and comes back to the accounts', async () => { + const user = userEvent.setup() + rememberAccount(alice) + mockAuthApi({ options: identifierFirst }) + + await renderLogin() + await user.click(await screen.findByRole('button', { name: 'Use another account' })) + expect(await screen.findByLabelText('Username')).toHaveValue('') + + await user.click(screen.getByRole('button', { name: 'Back to your accounts' })) + expect(await tile('alice@corp.example')).toBeInTheDocument() + }) + + it('shows the accounts after a session expired too', async () => { + rememberAccount(alice) + mockAuthApi({ options: identifierFirst }) + + await renderLogin('/login?expired=true&redirect=%2Ftokens') + + expect(await screen.findByText(/your session expired/i)).toBeInTheDocument() + expect(await tile('alice@corp.example')).toBeInTheDocument() + }) + + it('starts at the form for an invited user', async () => { + rememberAccount(alice) + mockAuthApi({ options: identifierFirst }) + + await renderLogin('/login?username=carol%40corp.example&invited=true') + + expect(await screen.findByLabelText('Username')).toHaveValue('carol@corp.example') + expect(screen.queryByRole('button', { name: /alice@corp\.example/ })).toBeNull() + }) + + it('keeps and shows nothing when the server turns remembering off', async () => { + const user = userEvent.setup() + rememberAccount(alice) + mockAuthApi({ + options: { ...identifierFirst, remember_accounts: false }, + onIdentify: () => json({ mode: 'password', realm: 'local' }), + onLogin: () => json(aliceSession), + }) + + const router = await renderLogin() + + // No tiles, and the accounts kept before are forgotten. + expect(await screen.findByLabelText('Username')).toBeInTheDocument() + expect(screen.queryByRole('list', { name: 'Remembered accounts' })).toBeNull() + await waitFor(() => expect(listAccounts()).toEqual([])) + + // A sign-in is not remembered either. + await user.type(screen.getByLabelText('Username'), 'alice@corp.example') + await user.click(screen.getByRole('button', { name: 'Continue' })) + await user.type(await screen.findByLabelText('Password'), 'hunter2') + await user.click(screen.getByRole('button', { name: 'Sign in' })) + await waitFor(() => expect(router.state.location.pathname).toBe('/')) + expect(listAccounts()).toEqual([]) + }) + + it('neither offers nor forgets the accounts while the options cannot be loaded', async () => { + rememberAccount(alice) + mockAuthApi({ options: undefined }) + + await renderLogin() + + expect(await screen.findByRole('status')).toHaveTextContent( + /password sign-in is available/i, + ) + expect(screen.getByLabelText('Username')).toBeInTheDocument() + expect(listAccounts()).toHaveLength(1) + }) + + it('drops an OIDC sign-in that came back here instead of opening a session', async () => { + sessionStorage.setItem( + 'forge.accounts.pending', + JSON.stringify({ realm: 'corp', started: Date.now() }), + ) + mockAuthApi({ options: combined }) + + await renderLogin('/login?error=oidc_failed') + + await screen.findByRole('alert') + expect(sessionStorage.getItem('forge.accounts.pending')).toBeNull() + }) + }) + describe('OIDC callback errors', () => { it('says the identity provider sign-in did not complete (oidc_failed)', async () => { mockAuthApi({ options: combined }) diff --git a/web/src/routes/Login.tsx b/web/src/routes/Login.tsx index 5b70d246..a07cb453 100644 --- a/web/src/routes/Login.tsx +++ b/web/src/routes/Login.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useEffect, useState } from 'react' import { useQuery, useQueryClient } from '@tanstack/react-query' import { createFileRoute, @@ -16,12 +16,30 @@ import { type OidcLoginOption, } from '~/api/auth' import { ApiError } from '~/lib/httpClient' +import { + beginOidcSignIn, + clearPendingSignIn, + forgetAccount, + forgetAllAccounts, + listAccounts, + rememberAccount, + type RememberedAccount, +} from '~/lib/accounts' import { setSession } from '~/lib/auth' import { assignLocation } from '~/lib/navigation' import { safeReturnTo } from '~/lib/returnTo' import { searchFlag } from '~/lib/search' import { AuthCard } from '~/components/AuthCard' -import { Button, Code, Icon, Notice, Skeleton, TextField } from '~/components/ui' +import { + Avatar, + Button, + Code, + Icon, + Notice, + Skeleton, + StatusBadge, + TextField, +} from '~/components/ui' interface LoginSearch { /** @@ -88,6 +106,18 @@ const passwordFallbackOptions: LoginOptions = { mode: 'combined', password: true, oidc: [], + // Unknown while the options cannot be read: the remembered accounts are + // neither offered nor forgotten. + remember_accounts: false, +} + +/** + * The account a remembered-account tile continues with in the mode's form: + * its identifier, and the password realm `/auth/identify` routed it to. + */ +interface PickedAccount { + identifier: string + realm?: string } export const Route = createFileRoute('/login')({ @@ -107,8 +137,12 @@ export const Route = createFileRoute('/login')({ }) /** - * Sign-in screen. On load it fetches `GET /auth/login-options` and renders one - * of two layouts: + * Sign-in screen. On load it fetches `GET /auth/login-options`. When it + * remembers accounts (`remember_accounts`) and this device signed some in + * before (`~/lib/accounts`), it first offers them as tiles: one click + * continues as that account (the password step, or the identity provider + * with the account as `login_hint`), "Use another account" shows the form. + * The form is one of two layouts: * * - **identifier-first** (default): step 1 asks only for the username and * calls `POST /auth/identify`; depending on the realm decision step 2 shows @@ -118,7 +152,9 @@ export const Route = createFileRoute('/login')({ * enabled OIDC realm. * * A successful password login sets the `forge_session` cookie; we persist the - * returned CSRF token (session mode) and navigate to the `redirect` target. + * returned CSRF token (session mode), remember the account, and navigate to + * the `redirect` target. An OIDC sign-in is remembered once the session it + * opens reaches the app (`completePendingSignIn`). * A failed OIDC callback lands here with `?error=`: a short, generic * message is shown above the form with a "Try again" link that clears it. * Signing in with a registry token is intentionally not offered: the UI always @@ -138,14 +174,40 @@ function Login() { }) const loginOptions = options.data ?? (options.isError ? passwordFallbackOptions : undefined) + const remember = loginOptions?.remember_accounts === true + const [accounts, setAccounts] = useState(listAccounts) + const [picked, setPicked] = useState(null) + const [another, setAnother] = useState(false) + // An invited user's username starts the form; otherwise the remembered + // accounts come first, until one is picked or "Use another account". + const canPick = remember && accounts.length > 0 && !username + const showPicker = canPick && !another && picked === null + + // Back here, an OIDC sign-in this tab started did not open a session (it + // failed, or the user came back): it must not be remembered later. + useEffect(() => clearPendingSignIn(), []) + + // Remembering turned off by an administrator (a kiosk, a shared + // terminal): keep no account on this device. + const rememberOff = options.data?.remember_accounts === false + useEffect(() => { + if (rememberOff) forgetAllAccounts() + }, [rememberOff]) + + function forget(identifier: string) { + forgetAccount(identifier) + setAccounts(listAccounts()) + } return ( {error && ( @@ -187,13 +249,48 @@ function Login() { {!loginOptions ? ( + ) : showPicker ? ( + setAnother(true)} + /> ) : loginOptions.mode === 'combined' ? ( - + ) : ( - + )} - {loginOptions?.mode !== 'combined' && ( + {canPick && !showPicker && ( + + )} + + {loginOptions?.mode !== 'combined' && !showPicker && (

Forge checks which sign-in your account uses. With your organization’s single sign-on you continue at its sign-in page; @@ -207,12 +304,32 @@ function Login() { /** * Append the post-login destination to an OIDC login URL so the server can * send the browser back after the IdP round-trip - * (`/auth/oidc/login?redirect_to=...`). + * (`/auth/oidc/login?redirect_to=...`), and the account the user means as + * `login_hint`, which the server passes to the IdP so it does not ask for the + * account again. */ -function oidcUrl(loginUrl: string, redirect?: string): string { - if (!redirect) return loginUrl +function oidcUrl(loginUrl: string, redirect?: string, loginHint?: string): string { + const params: string[] = [] + if (redirect) params.push(`redirect_to=${encodeURIComponent(safeReturnTo(redirect))}`) + if (loginHint) params.push(`login_hint=${encodeURIComponent(loginHint)}`) + if (params.length === 0) return loginUrl const sep = loginUrl.includes('?') ? '&' : '?' - return `${loginUrl}${sep}redirect_to=${encodeURIComponent(safeReturnTo(redirect))}` + return `${loginUrl}${sep}${params.join('&')}` +} + +/** + * Leave for the identity provider of `realm`. When the screen remembers + * accounts, the session the round trip opens is remembered as this realm's + * (`beginOidcSignIn`). + */ +function startOidcSignIn( + loginUrl: string, + realm: string, + { redirect, loginHint, remember }: { redirect?: string; loginHint?: string; remember: boolean }, +): void { + if (remember) beginOidcSignIn(realm) + else clearPendingSignIn() + assignLocation(oidcUrl(loginUrl, redirect, loginHint)) } /** @@ -237,14 +354,18 @@ function signInErrorMessage(err: unknown): string { return 'Could not sign in. Is the server reachable?' } -/** Shared hook: perform `POST /auth/login` and enter session mode. */ -function usePasswordLogin(redirect?: string) { +/** + * Shared hook: perform `POST /auth/login` and enter session mode. With + * `remember`, the account is remembered on this device for the next sign-in + * (with `realm`, the password realm `/auth/identify` named, when known). + */ +function usePasswordLogin(redirect: string | undefined, remember: boolean) { const router = useRouter() const queryClient = useQueryClient() const [error, setError] = useState(null) const [submitting, setSubmitting] = useState(false) - async function signIn(username: string, password: string): Promise { + async function signIn(username: string, password: string, realm?: string): Promise { setSubmitting(true) setError(null) try { @@ -255,6 +376,16 @@ function usePasswordLogin(redirect?: string) { // identity fresh. queryClient.clear() setSession(session.csrf_token) + // This sign-in replaces any OIDC one this tab left unfinished. + clearPendingSignIn() + if (remember) { + rememberAccount({ + identifier: username, + display_name: session.display_name ?? undefined, + method: 'password', + realm, + }) + } // Stay "submitting" while the app loads; this screen is leaving (the // reset below only matters if a guard sends us back here). await router.navigate({ href: safeReturnTo(redirect) }) @@ -281,20 +412,29 @@ function FormError({ children }: { children: React.ReactNode }) { /** * Identifier-first (UX A): step 1 = username -> `POST /auth/identify`; the * realm decision either reveals the password field (step 2, username shown - * with a back link) or navigates the browser to the IdP. + * with a back link) or navigates the browser to the IdP, with the username + * as its login hint. With `initialRealm` (a remembered account `identify` + * already routed to a password realm) it starts at step 2. */ function IdentifierFirstLogin({ redirect, + remember, initialUsername, + initialRealm, }: { redirect?: string + remember: boolean initialUsername?: string + initialRealm?: string }) { - const [step, setStep] = useState<'identify' | 'password'>('identify') + const [step, setStep] = useState<'identify' | 'password'>( + initialRealm !== undefined ? 'password' : 'identify', + ) const [username, setUsername] = useState(initialUsername ?? '') + const [realm, setRealm] = useState(initialRealm) const [password, setPassword] = useState('') const { signIn, error, setError, submitting, setSubmitting } = - usePasswordLogin(redirect) + usePasswordLogin(redirect, remember) async function onIdentify(e: React.FormEvent) { e.preventDefault() @@ -308,11 +448,16 @@ function IdentifierFirstLogin({ try { const decision = await identify(trimmed) if (decision.mode === 'oidc_redirect' && decision.login_url) { - assignLocation(oidcUrl(decision.login_url, redirect)) + startOidcSignIn(decision.login_url, decision.realm, { + redirect, + loginHint: trimmed, + remember, + }) // The browser is leaving: keep "submitting" (and Continue disabled) // until it does, so a second click cannot start another flow. return } + setRealm(decision.realm) setStep('password') setSubmitting(false) } catch (err) { @@ -327,7 +472,7 @@ function IdentifierFirstLogin({ setError('Enter your password.') return } - void signIn(username.trim(), password) + void signIn(username.trim(), password, realm) } if (step === 'identify') { @@ -383,20 +528,26 @@ function IdentifierFirstLogin({ /** * Combined (UX B): username+password always shown (when a password realm is - * enabled) plus one button per OIDC entry. No identify step. + * enabled) plus one button per OIDC entry. No identify step. A remembered + * account picked for it fills the username and puts the cursor in the + * password (`focusPassword`). */ function CombinedLogin({ options, redirect, + remember, initialUsername, + focusPassword = false, }: { options: LoginOptions redirect?: string + remember: boolean initialUsername?: string + focusPassword?: boolean }) { const [username, setUsername] = useState(initialUsername ?? '') const [password, setPassword] = useState('') - const { signIn, error, setError, submitting } = usePasswordLogin(redirect) + const { signIn, error, setError, submitting } = usePasswordLogin(redirect, remember) function onSubmit(e: React.FormEvent) { e.preventDefault() @@ -415,7 +566,7 @@ function CombinedLogin({ label="Username" type="text" autoComplete="username" - autoFocus + autoFocus={!focusPassword} value={username} onChange={(e) => setUsername(e.target.value)} /> @@ -423,6 +574,7 @@ function CombinedLogin({ label="Password" type="password" autoComplete="current-password" + autoFocus={focusPassword} value={password} onChange={(e) => setPassword(e.target.value)} /> @@ -442,7 +594,7 @@ function CombinedLogin({ )} {options.oidc.map((idp) => ( - + ))} ) @@ -451,17 +603,139 @@ function CombinedLogin({ function OidcButton({ idp, redirect, + remember, }: { idp: OidcLoginOption redirect?: string + remember: boolean }) { return ( ) } + +/** + * The remembered accounts, newest first, as tiles (like Microsoft's "Pick an + * account"). A tile continues as its account: + * + * - an OIDC account whose realm the login options still offer goes straight + * to that realm's identity provider, the identifier as `login_hint`; + * - a password account goes to the password step (`onPassword`): in + * combined mode the form with the username filled in, in identifier-first + * mode once `/auth/identify` confirms the identifier still signs in with a + * password (a domain an OIDC realm took over since goes to its identity + * provider instead, as does every account once no password realm is + * left); + * - an OIDC account whose realm is gone or disabled is routed by + * `/auth/identify`, like the identifier typed into the form, so it never + * loops back to a realm that is not offered. + * + * Each tile's ✕ forgets its account; "Use another account" shows the form. + */ +function AccountPicker({ + accounts, + options, + redirect, + onPassword, + onForget, + onAnother, +}: { + accounts: RememberedAccount[] + options: LoginOptions + redirect?: string + onPassword: (account: PickedAccount) => void + onForget: (identifier: string) => void + onAnother: () => void +}) { + // The identifier being continued with, while its request runs or the + // browser leaves for the identity provider. + const [busy, setBusy] = useState(null) + const [error, setError] = useState(null) + + async function pick(account: RememberedAccount) { + const loginHint = account.identifier + setError(null) + setBusy(account.identifier) + if (account.method === 'oidc') { + const idp = options.oidc.find((o) => o.name === account.realm) + if (idp) { + // The browser is leaving: stay busy until it does. + startOidcSignIn(idp.login_url, idp.name, { redirect, loginHint, remember: true }) + return + } + } else if (options.mode === 'combined' && options.password) { + onPassword({ identifier: account.identifier }) + return + } + try { + const decision = await identify(account.identifier) + if (decision.mode === 'oidc_redirect' && decision.login_url) { + startOidcSignIn(decision.login_url, decision.realm, { + redirect, + loginHint, + remember: true, + }) + return + } + onPassword({ identifier: account.identifier, realm: decision.realm }) + } catch (err) { + setError(signInErrorMessage(err)) + setBusy(null) + } + } + + return ( +

+
    + {accounts.map((account) => { + const name = account.display_name ?? account.identifier + return ( +
  • + + +
    • + ) + })} +
+ {error && {error}} + +
+ ) +} diff --git a/web/src/routes/admin/settings/Settings.tsx b/web/src/routes/admin/settings/Settings.tsx index b75df6b9..a051010b 100644 --- a/web/src/routes/admin/settings/Settings.tsx +++ b/web/src/routes/admin/settings/Settings.tsx @@ -213,6 +213,7 @@ function AnonymousRepositories() { interface SignInDraft { login_mode: LoginMode + remember_accounts: boolean allow_password_basic: boolean session_idle_ttl_seconds: string session_absolute_ttl_seconds: string @@ -241,6 +242,7 @@ function validateSignIn( return { body: { login_mode: draft.login_mode, + remember_accounts: draft.remember_accounts, allow_password_basic: draft.allow_password_basic, session_idle_ttl_seconds: idle!, session_absolute_ttl_seconds: absolute!, @@ -256,12 +258,16 @@ function durationHint(raw: string): string | null { return n === null ? null : `= ${formatDuration(n)}` } -/** The login screen, password Basic, session lifetimes and the lockout. */ +/** + * The login screen and its remembered accounts, password Basic, session + * lifetimes and the lockout. + */ function SignInForm({ settings }: { settings: AuthSettings }) { const update = useUpdateAuthSettings() const realms = useRealms() const [draft, setDraft] = useState({ login_mode: settings.login_mode, + remember_accounts: settings.remember_accounts, allow_password_basic: settings.allow_password_basic, session_idle_ttl_seconds: String(settings.session_idle_ttl_seconds), session_absolute_ttl_seconds: String(settings.session_absolute_ttl_seconds), @@ -319,6 +325,15 @@ function SignInForm({ settings }: { settings: AuthSettings }) { + set('remember_accounts', checked)} + description="After a sign-in, the browser keeps the account (its username, name and how it signs in, never a password or a session) and the login screen offers it next time, so signing in again is one click. Turn it off for kiosks and shared computers: their login screens then forget the accounts they kept." + /> + { name: 'Accept passwords over HTTP Basic on the feeds', }), ) + const remember = within(signIn).getByRole('checkbox', { + name: 'Remember accounts on the login screen', + }) + expect(remember).toBeChecked() + await u.click(remember) await u.click(within(signIn).getByRole('button', { name: 'Save sign-in' })) await waitFor(() => expect(puts()).toEqual([ { login_mode: 'combined', + remember_accounts: false, allow_password_basic: true, session_idle_ttl_seconds: 600, session_absolute_ttl_seconds: 604800, diff --git a/web/src/styles/forge-ui.css b/web/src/styles/forge-ui.css index 871433a6..0f73a434 100644 --- a/web/src/styles/forge-ui.css +++ b/web/src/styles/forge-ui.css @@ -453,4 +453,16 @@ .fg-toasts { right: var(--space-4); bottom: var(--space-4); } } .fg-dialog { max-height: calc(100dvh - 2 * var(--space-4)); overflow-y: auto; } + + /* ---------- App additions: remembered accounts (login screen) ---------- */ + .fg-accounts { display: grid; gap: var(--space-2); margin: 0; padding: 0; list-style: none; } + .fg-account { display: flex; align-items: center; gap: var(--space-1); padding-right: var(--space-1); border: 1px solid var(--line); border-radius: var(--radius-md); background: var(--surface); transition: background-color 120ms ease-out, border-color 120ms ease-out; } + .fg-account:hover { border-color: var(--line-strong); background: var(--surface-hover); } + .fg-account-pick { display: flex; flex: 1; align-items: center; gap: var(--space-3); min-width: 0; padding: 10px 12px; border: 0; border-radius: var(--radius-md); background: transparent; color: var(--ink); font: inherit; text-align: left; cursor: pointer; } + .fg-account-pick:disabled { cursor: progress; } + .fg-account-pick:focus-visible { outline: 2px solid var(--focus); outline-offset: 2px; } + .fg-account-who { display: grid; flex: 1; min-width: 0; } + .fg-account-name, .fg-account-id { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } + .fg-account-name { font-size: 14px; line-height: 20px; font-weight: 600; } + .fg-account-id { color: var(--ink-muted); font-size: 12.5px; line-height: 18px; } } From 790b2b946efecbd6e3f465ea45c6057e5845ead8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 21:49:40 +0000 Subject: [PATCH 2/2] Login: rename the fallback login options so CodeQL stops reading them as a password CodeQL's js/clear-text-storage-of-sensitive-data took passwordFallbackOptions for a password by its name and followed it through options.oidc[].name into the pending OIDC sign-in that beginOidcSignIn keeps in sessionStorage. What is stored there is a realm name, which /auth/login-options publishes; the constant is the login options while those cannot be read, and is now named so. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJbsBcmhipReXByB1cF --- web/src/routes/Login.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/src/routes/Login.tsx b/web/src/routes/Login.tsx index a07cb453..8c8e83bc 100644 --- a/web/src/routes/Login.tsx +++ b/web/src/routes/Login.tsx @@ -102,7 +102,13 @@ function parseClaim(raw: unknown): string | undefined { : undefined } -const passwordFallbackOptions: LoginOptions = { +/** + * The login options while `GET /auth/login-options` cannot be read: the + * username and password form alone. (Not named after the password: static + * analysis takes such a name for a credential, and the realm names read from + * these options reach storage as a pending OIDC sign-in.) + */ +const fallbackLoginOptions: LoginOptions = { mode: 'combined', password: true, oidc: [], @@ -173,7 +179,7 @@ function Login() { retry: false, }) const loginOptions = - options.data ?? (options.isError ? passwordFallbackOptions : undefined) + options.data ?? (options.isError ? fallbackLoginOptions : undefined) const remember = loginOptions?.remember_accounts === true const [accounts, setAccounts] = useState(listAccounts) const [picked, setPicked] = useState(null)