From ccdd64bf3af5ba8982bb61ecb38b16714fdeab73 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 24 Jun 2026 19:30:13 +0200 Subject: [PATCH] Add the account-mismatch interstitial for id_token_hint/login_hint When continuing an authorization grant whose requested identity (a verified id_token_hint target or an untrusted login_hint) does not match the active session, divert to a new grant-scoped /account-selection screen rather than straight to consent. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/handlers/src/lib.rs | 5 + .../src/oauth2/authorization/consent.rs | 33 + .../src/oauth2/authorization/id_token_hint.rs | 57 +- .../handlers/src/oauth2/authorization/mod.rs | 35 +- .../oauth2/authorization/select_account.rs | 837 ++++++++++++++++++ crates/handlers/src/views/shared.rs | 41 +- crates/router/src/endpoints.rs | 47 + crates/templates/src/context.rs | 52 ++ crates/templates/src/lib.rs | 11 +- templates/pages/account_selection.html | 43 + translations/en.json | 20 +- 11 files changed, 1151 insertions(+), 30 deletions(-) create mode 100644 crates/handlers/src/oauth2/authorization/select_account.rs create mode 100644 templates/pages/account_selection.html diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index 19a0afbaf..439e9ff02 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -479,6 +479,11 @@ where get(self::oauth2::authorization::consent::get) .post(self::oauth2::authorization::consent::post), ) + .route( + mas_router::SelectAccount::route(), + get(self::oauth2::authorization::select_account::get) + .post(self::oauth2::authorization::select_account::post), + ) .route( mas_router::CompatLoginSsoComplete::route(), get(self::compat::login_sso_complete::get).post(self::compat::login_sso_complete::post), diff --git a/crates/handlers/src/oauth2/authorization/consent.rs b/crates/handlers/src/oauth2/authorization/consent.rs index 5d93d98a1..74b70560d 100644 --- a/crates/handlers/src/oauth2/authorization/consent.rs +++ b/crates/handlers/src/oauth2/authorization/consent.rs @@ -145,6 +145,24 @@ pub(crate) async fn get( .record_browser_session(&clock, &session) .await; + // Trusted-mismatch guard: for a grant with a verified `id_token_hint` + // target, OIDC Core forbids returning a token for a different user. If the + // active session is for someone else, divert to the account-selection + // screen rather than rendering consent. We only enforce the *trusted* case; + // an untrusted `login_hint` stays advisory (enforcing it here would loop + // consent → select-account → "continue" → consent → …). + if grant + .target_user_id + .is_some_and(|target| target != session.user.id) + { + repo.save().await?; + return Ok(( + cookie_jar, + url_builder.redirect(&mas_router::SelectAccount::continue_grant(grant_id)), + ) + .into_response()); + } + let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let session_counts = count_user_sessions_for_limiting(&mut repo, &session.user).await?; @@ -250,6 +268,21 @@ pub(crate) async fn post( .record_browser_session(&clock, &browser_session) .await; + // Trusted-mismatch guard (see the GET handler): never fulfil a grant with a + // verified `id_token_hint` target as a different user. Divert to the + // account-selection screen instead. Untrusted `login_hint` stays advisory. + if grant + .target_user_id + .is_some_and(|target| target != browser_session.user.id) + { + repo.save().await?; + return Ok(( + cookie_jar, + url_builder.redirect(&mas_router::SelectAccount::continue_grant(grant_id)), + ) + .into_response()); + } + let client = repo .oauth2_client() .lookup(grant.client_id) diff --git a/crates/handlers/src/oauth2/authorization/id_token_hint.rs b/crates/handlers/src/oauth2/authorization/id_token_hint.rs index f4e103336..a3afc6458 100644 --- a/crates/handlers/src/oauth2/authorization/id_token_hint.rs +++ b/crates/handlers/src/oauth2/authorization/id_token_hint.rs @@ -8,17 +8,70 @@ use std::collections::HashMap; -use mas_data_model::{BrowserSession, Clock, User}; +use mas_data_model::{AuthorizationGrant, BrowserSession, Clock, User}; use mas_jose::{ claims::{self, TimeOptions}, jwt::Jwt, }; use mas_keystore::Keystore; use mas_router::UrlBuilder; -use mas_storage::{BoxRepository, RepositoryAccess, RepositoryError}; +use mas_storage::{BoxRepository, RepositoryAccess, RepositoryError, user::UserEmailRepository}; use serde_json::Value; use ulid::Ulid; +use crate::views::shared::{LoginHint, parse_login_hint}; + +/// Decide whether the active browser session matches the identity the client +/// requested on the grant. +/// +/// The match key is the **user**, not the session: a stale `sid` whose user is +/// still logged in is a match. The trust ladder mirrors §4 of the design: +/// +/// * a verified `id_token_hint` target (`grant.target_user_id`) is **trusted** +/// and compared by user id; +/// * otherwise an untrusted `grant.login_hint` is parsed and compared against +/// the *current* session's own data only (we never look up the hinted +/// account): an `mxid:` hint matches on localpart + homeserver, a bare email +/// matches iff the current user owns that email; +/// * an unparseable or absent hint is treated as no constraint (a match). +/// +/// Returns `Ok(true)` when there is no mismatch (proceed to consent), +/// `Ok(false)` when the active session is a different account than requested. +pub(crate) async fn session_matches_requested_identity( + repo: &mut BoxRepository, + homeserver: &str, + grant: &AuthorizationGrant, + session: &BrowserSession, +) -> Result { + // Trusted target wins: compare by user id. + if let Some(target) = grant.target_user_id { + return Ok(session.user.id == target); + } + + // Otherwise fall back to the untrusted `login_hint`, compared only against + // the current session's own data. + let Some(login_hint) = &grant.login_hint else { + return Ok(true); + }; + + match parse_login_hint(login_hint, homeserver) { + // `parse_login_hint` only resolves mxids on our own homeserver, so + // comparing localparts is enough. + LoginHint::Mxid(mxid) => Ok(mxid.localpart() == session.user.username), + LoginHint::Email(email) => { + // Look up the email on the *current* user only — their own data. + let found = repo + .user_email() + .find(&session.user, email.as_ref()) + .await?; + Ok(found.is_some()) + } + // Unparseable hint (including an mxid on another homeserver): no + // constraint we can act on, treat as a match. + LoginHint::None => Ok(true), + } +} + /// The resolved outcome of verifying an `id_token_hint`. pub(crate) struct ResolvedHint { /// The user identified by the token's `sub` claim. diff --git a/crates/handlers/src/oauth2/authorization/mod.rs b/crates/handlers/src/oauth2/authorization/mod.rs index 16d80c51d..09e308ffd 100644 --- a/crates/handlers/src/oauth2/authorization/mod.rs +++ b/crates/handlers/src/oauth2/authorization/mod.rs @@ -5,7 +5,7 @@ // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial // Please see LICENSE files in the repository root for full details. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, sync::Arc}; use axum::{ extract::State, @@ -16,6 +16,7 @@ use hyper::StatusCode; use mas_axum_utils::{GenericError, InternalError, SessionInfoExt, cookies::CookieJar}; use mas_data_model::{AuthorizationCode, BoxClock, BoxRng, Pkce}; use mas_keystore::Keystore; +use mas_matrix::HomeserverConnection; use mas_router::{PostAuthAction, UrlBuilder}; use mas_storage::{ BoxRepository, @@ -38,6 +39,7 @@ use crate::{BoundActivityTracker, PreferredLanguage, impl_from_error_for_route}; mod callback; pub(crate) mod consent; mod id_token_hint; +pub(crate) mod select_account; #[derive(Debug, Error)] pub enum RouteError { @@ -122,6 +124,7 @@ pub(crate) async fn get( State(templates): State, State(url_builder): State, State(key_store): State, + State(homeserver): State>, activity_tracker: BoundActivityTracker, mut repo: BoxRepository, cookie_jar: CookieJar, @@ -319,14 +322,34 @@ pub(crate) async fn get( Some(user_session) => { // TODO: better support for prompt=create when we have a session - repo.save().await?; - activity_tracker .record_browser_session(&clock, &user_session) .await; - url_builder - .redirect(&mas_router::Consent(grant.id)) - .into_response() + + // If the client requested a specific identity (via a verified + // `id_token_hint` target or an untrusted `login_hint`) that + // doesn't match the active session, divert to the + // account-mismatch interstitial. Consent re-enforces this + // authoritatively; this branch is only early UX routing. + let matches = id_token_hint::session_matches_requested_identity( + &mut repo, + homeserver.homeserver(), + &grant, + &user_session, + ) + .await?; + + repo.save().await?; + + if matches { + url_builder + .redirect(&mas_router::Consent(grant.id)) + .into_response() + } else { + url_builder + .redirect(&mas_router::SelectAccount::continue_grant(grant.id)) + .into_response() + } } }; diff --git a/crates/handlers/src/oauth2/authorization/select_account.rs b/crates/handlers/src/oauth2/authorization/select_account.rs new file mode 100644 index 000000000..1706d363b --- /dev/null +++ b/crates/handlers/src/oauth2/authorization/select_account.rs @@ -0,0 +1,837 @@ +// Copyright 2026 Element Creations Ltd. +// +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +// Please see LICENSE files in the repository root for full details. + +//! The account-mismatch interstitial. +//! +//! Shown while continuing an authorization grant whose requested identity (a +//! verified `id_token_hint` target, or an untrusted `login_hint`) doesn't match +//! the active browser session. It is always scoped to a grant — never reachable +//! as a standalone endpoint — which keeps it from being a generic phishing +//! surface. + +use std::sync::Arc; + +use axum::{ + extract::{Form, State}, + response::{Html, IntoResponse, Response}, +}; +use axum_extra::extract::Query; +use hyper::StatusCode; +use mas_axum_utils::{ + GenericError, InternalError, SessionInfoExt, + cookies::CookieJar, + csrf::{CsrfExt, ProtectedForm}, +}; +use mas_data_model::{AuthorizationGrantStage, BoxClock, BoxRng}; +use mas_matrix::HomeserverConnection; +use mas_router::{PostAuthAction, UrlBuilder}; +use mas_storage::{ + BoxRepository, oauth2::OAuth2AuthorizationGrantRepository, user::BrowserSessionRepository, +}; +use mas_templates::{SelectAccountContext, TemplateContext, Templates}; +use oauth2_types::errors::{ClientError, ClientErrorCode}; +use serde::Deserialize; +use thiserror::Error; +use ulid::Ulid; + +use super::{callback::CallbackDestination, id_token_hint::session_matches_requested_identity}; +use crate::{BoundActivityTracker, PreferredLanguage, impl_from_error_for_route}; + +#[derive(Debug, Error)] +pub enum RouteError { + #[error(transparent)] + Internal(Box), + + #[error(transparent)] + Csrf(#[from] mas_axum_utils::csrf::CsrfError), + + #[error("Authorization grant not found")] + GrantNotFound, + + #[error("Authorization grant {0} already used")] + GrantNotPending(Ulid), +} + +impl_from_error_for_route!(mas_templates::TemplateError); +impl_from_error_for_route!(mas_storage::RepositoryError); +impl_from_error_for_route!(super::callback::IntoCallbackDestinationError); +impl_from_error_for_route!(super::callback::CallbackDestinationError); + +impl IntoResponse for RouteError { + fn into_response(self) -> Response { + match self { + Self::Internal(e) => InternalError::new(e).into_response(), + e @ Self::GrantNotFound => GenericError::new(StatusCode::NOT_FOUND, e).into_response(), + e @ Self::GrantNotPending(_) => { + GenericError::new(StatusCode::CONFLICT, e).into_response() + } + e @ Self::Csrf(_) => GenericError::new(StatusCode::BAD_REQUEST, e).into_response(), + } + } +} + +/// Build the requested-identity display string and trust flag for the grant. +/// +/// For a trusted target we look up the resolved user (allowed — the hint was +/// cryptographically verified) and show its mxid. For an untrusted `login_hint` +/// we echo the client-supplied string verbatim, never looking it up. +async fn requested_identity( + repo: &mut BoxRepository, + homeserver: &dyn HomeserverConnection, + grant: &mas_data_model::AuthorizationGrant, +) -> Result, RouteError> { + if let Some(target) = grant.target_user_id { + let user = repo.user().lookup(target).await?; + return Ok(user.map(|user| (homeserver.mxid(&user.username), true))); + } + + Ok(grant + .login_hint + .clone() + .map(|hint| (login_hint_display(&hint), false))) +} + +/// Turn a raw `login_hint` into something friendlier to echo, while still being +/// purely the client's own input (no MAS-side lookup). +fn login_hint_display(hint: &str) -> String { + hint.strip_prefix("mxid:").unwrap_or(hint).to_owned() +} + +#[tracing::instrument( + name = "handlers.oauth2.authorization.select_account.get", + fields(grant.id = tracing::field::Empty), + skip_all, +)] +pub(crate) async fn get( + mut rng: BoxRng, + clock: BoxClock, + PreferredLanguage(locale): PreferredLanguage, + State(templates): State, + State(url_builder): State, + State(homeserver): State>, + activity_tracker: BoundActivityTracker, + mut repo: BoxRepository, + cookie_jar: CookieJar, + Query(params): Query, +) -> Result { + let action = params.post_auth_action(); + let PostAuthAction::ContinueAuthorizationGrant { id: grant_id } = *action else { + // Other flows (device-code, manage-account) don't populate a requested + // identity yet, so the account-selection screen is a no-op passthrough + // for them; just continue the flow. + return Ok((cookie_jar, action.go_next(&url_builder)).into_response()); + }; + tracing::Span::current().record("grant.id", tracing::field::display(grant_id)); + + let (session_info, cookie_jar) = cookie_jar.session_info(); + let maybe_session = session_info.load_active_session(&mut repo).await?; + + let grant = repo + .oauth2_authorization_grant() + .lookup(grant_id) + .await? + .ok_or(RouteError::GrantNotFound)?; + + if !matches!(grant.stage, AuthorizationGrantStage::Pending) { + return Err(RouteError::GrantNotPending(grant.id)); + } + + // No active session: nothing to mismatch against, send them to login + // continuing the grant. + let Some(session) = maybe_session else { + let login = mas_router::Login::and_continue_grant(grant_id); + return Ok((cookie_jar, url_builder.redirect(&login)).into_response()); + }; + + activity_tracker + .record_browser_session(&clock, &session) + .await; + + // Defensive: if the session actually matches (or there is no constraint), + // don't show the interstitial — go straight to consent. Entry routing + // should already prevent landing here on a match. + if session_matches_requested_identity(&mut repo, homeserver.homeserver(), &grant, &session) + .await? + { + repo.save().await?; + return Ok(( + cookie_jar, + url_builder.redirect(&mas_router::Consent(grant_id)), + ) + .into_response()); + } + + let Some((requested, trusted)) = requested_identity(&mut repo, &*homeserver, &grant).await? + else { + // Only reachable when a trusted target vanished since authorize time + // (an absent hint counted as a match above). Per OIDC Core the grant + // can never be fulfilled as a different user, so return + // `login_required` to the client rather than redirecting to consent, + // which would divert straight back here. + let callback_destination = CallbackDestination::try_from(&grant)?; + repo.save().await?; + let response = callback_destination.go( + &templates, + &locale, + ClientError::from(ClientErrorCode::LoginRequired), + )?; + return Ok((cookie_jar, response).into_response()); + }; + + let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); + let current_username = session.user.username.clone(); + + repo.save().await?; + + let ctx = SelectAccountContext::new(requested, trusted, current_username) + .with_csrf(csrf_token.form_value()) + .with_language(locale); + + let content = templates.render_select_account(&ctx)?; + + Ok((cookie_jar, Html(content)).into_response()) +} + +#[derive(Deserialize)] +#[serde(rename_all = "snake_case", tag = "action")] +pub(crate) enum FormAction { + /// Sign in as the requested account (sign out the current one). + Switch, + /// Continue as the current account (untrusted hint only). + Continue, + /// Abandon the flow and return an error to the client. + Cancel, +} + +#[tracing::instrument( + name = "handlers.oauth2.authorization.select_account.post", + fields(grant.id = tracing::field::Empty), + skip_all, +)] +pub(crate) async fn post( + clock: BoxClock, + PreferredLanguage(locale): PreferredLanguage, + State(templates): State, + State(url_builder): State, + activity_tracker: BoundActivityTracker, + mut repo: BoxRepository, + cookie_jar: CookieJar, + Query(params): Query, + Form(form): Form>, +) -> Result { + let action = params.post_auth_action(); + let PostAuthAction::ContinueAuthorizationGrant { id: grant_id } = *action else { + // Other flows (device-code, manage-account) don't populate a requested + // identity yet, so the account-selection screen is a no-op passthrough + // for them; just continue the flow. + return Ok((cookie_jar, action.go_next(&url_builder)).into_response()); + }; + tracing::Span::current().record("grant.id", tracing::field::display(grant_id)); + + let form = cookie_jar.verify_form(&clock, form)?; + + let (session_info, cookie_jar) = cookie_jar.session_info(); + + let grant = repo + .oauth2_authorization_grant() + .lookup(grant_id) + .await? + .ok_or(RouteError::GrantNotFound)?; + + if !matches!(grant.stage, AuthorizationGrantStage::Pending) { + return Err(RouteError::GrantNotPending(grant.id)); + } + + match form { + FormAction::Continue => { + // "Continue as current" only exists for an untrusted hint. For a + // trusted target (a verified `id_token_hint`) OIDC Core forbids + // returning a token for a different user, and the screen never + // offers this action — so a crafted POST must not be allowed to + // complete the grant as the current account. Divert back to the + // account-selection screen instead of proceeding to consent. + if grant.target_user_id.is_some() { + repo.save().await?; + return Ok(( + cookie_jar, + url_builder.redirect(&mas_router::SelectAccount::new(action.clone())), + ) + .into_response()); + } + + // Untrusted hint only: the user explicitly chooses to keep their + // current account. Proceed to consent (it re-enforces the match). + repo.save().await?; + Ok(( + cookie_jar, + url_builder.redirect(&mas_router::Consent(grant_id)), + ) + .into_response()) + } + + FormAction::Switch => { + // Sign out the current session (mirroring logout), then send the + // user to login continuing the grant. Slice 1's welcome-back handles + // a trusted target; a generic pre-fill handles the untrusted case, + // for which we carry the original `login_hint` along. + if let Some(session_id) = session_info.current_session_id() { + let maybe_session = repo.browser_session().lookup(session_id).await?; + if let Some(session) = maybe_session + && session.finished_at.is_none() + { + activity_tracker + .record_browser_session(&clock, &session) + .await; + repo.browser_session().finish(&clock, session).await?; + } + } + + repo.save().await?; + + let cookie_jar = + cookie_jar.update_session_info(&session_info.mark_session_ended(clock.now())); + + let mut login = mas_router::Login::and_continue_grant(grant_id); + // Only the untrusted case needs the hint forwarded; for a trusted + // target the resolved identity is already on the grant. + if grant.target_user_id.is_none() + && let Some(login_hint) = grant.login_hint + { + login = login.with_login_hint(login_hint); + } + + Ok((cookie_jar, url_builder.redirect(&login)).into_response()) + } + + FormAction::Cancel => { + // Bail out: return an `access_denied` error to the client. + let callback_destination = CallbackDestination::try_from(&grant)?; + repo.save().await?; + let response = callback_destination.go( + &templates, + &locale, + ClientError::from(ClientErrorCode::AccessDenied), + )?; + Ok((cookie_jar, response).into_response()) + } + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use chrono::Duration; + use hyper::{ + Request, StatusCode, + header::{CONTENT_TYPE, LOCATION}, + }; + use mas_data_model::Clock; + use mas_iana::jose::JsonWebSignatureAlg; + use mas_jose::{ + claims, + jwt::{JsonWebSignatureHeader, Jwt}, + }; + use mas_router::SimpleRoute; + use mas_storage::user::{UserPasswordRepository, UserRepository}; + use serde_json::Value; + use sqlx::PgPool; + use zeroize::Zeroizing; + + use crate::test_utils::{CookieHelper, RequestBuilderExt, ResponseExt, TestState, setup}; + + /// Register a public `OAuth2` client we can run an authorization flow + /// against. + async fn provision_client(state: &TestState) -> String { + let request = + Request::post(mas_router::OAuth2RegistrationEndpoint::PATH).json(serde_json::json!({ + "client_uri": "https://example.com/", + "redirect_uris": ["https://example.com/redirect"], + "response_types": ["code"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "none", + })); + let response = state.request(request).await; + response.assert_status(StatusCode::CREATED); + let response: oauth2_types::registration::ClientRegistrationResponse = response.json(); + response.client_id + } + + /// Provision a user with a password. + async fn user_with_password(state: &TestState, username: &str) -> mas_data_model::User { + let mut rng = state.rng(); + let mut repo = state.repository().await.unwrap(); + let user = repo + .user() + .add(&mut rng, &state.clock, username.to_owned()) + .await + .unwrap(); + let (version, hash) = state + .password_manager + .hash(&mut rng, Zeroizing::new("hunter2".to_owned())) + .await + .unwrap(); + repo.user_password() + .add(&mut rng, &state.clock, &user, version, hash, None) + .await + .unwrap(); + repo.save().await.unwrap(); + user + } + + /// Log in as the given user via `POST /login`, returning a cookie helper + /// holding the active session cookie. + async fn login(state: &TestState, username: &str) -> CookieHelper { + let cookies = CookieHelper::new(); + + let request = cookies.with_cookies(Request::get("/login").empty()); + let response = state.request(request).await; + cookies.save_cookies(&response); + let csrf_token = response + .body() + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + + let request = cookies.with_cookies(Request::post("/login").form(serde_json::json!({ + "csrf": csrf_token, + "username": username, + "password": "hunter2", + }))); + let response = state.request(request).await; + cookies.save_cookies(&response); + response.assert_status(StatusCode::SEE_OTHER); + + cookies + } + + /// Mint a MAS-signed ID-token-shaped JWT for the given subject, with an + /// already-expired `exp` (a realistic stale `id_token_hint`). + fn mint_hint(state: &TestState, sub: &str) -> String { + let issuer = state.url_builder.oidc_issuer().to_string(); + let mut payload: HashMap = HashMap::new(); + claims::ISS.insert(&mut payload, issuer).unwrap(); + claims::SUB.insert(&mut payload, sub.to_owned()).unwrap(); + claims::IAT + .insert(&mut payload, state.clock.now() - Duration::hours(2)) + .unwrap(); + claims::EXP + .insert(&mut payload, state.clock.now() - Duration::hours(1)) + .unwrap(); + + let key = state + .key_store + .signing_key_for_algorithm(&JsonWebSignatureAlg::Rs256) + .unwrap(); + let signer = key + .params() + .signing_key_for_alg(&JsonWebSignatureAlg::Rs256) + .unwrap(); + let header = JsonWebSignatureHeader::new(JsonWebSignatureAlg::Rs256); + let jwt: Jwt<'static, _> = + Jwt::sign_with_rng(&mut state.rng(), header, payload, &signer).unwrap(); + jwt.into_string() + } + + fn authorize_url(client_id: &str, extra: &str) -> String { + format!( + "{}?response_type=code&client_id={client_id}&redirect_uri=https://example.com/redirect&scope=openid&state=somestate&{extra}", + mas_router::OAuth2AuthorizationEndpoint::PATH, + ) + } + + /// Find the grant id the authorize handler redirected to, regardless of + /// whether it landed on consent or the account-selection screen. + fn redirect_path(response: &hyper::Response) -> String { + response + .headers() + .get(LOCATION) + .unwrap() + .to_str() + .unwrap() + .to_owned() + } + + /// Logged in as bob, a trusted hint for alice → account-selection screen + /// naming alice, with NO "continue as bob" option. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_trusted_mismatch_no_continue(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + + let location = redirect_path(&response); + assert!( + location.contains("/account-selection"), + "expected account-selection, got {location}" + ); + + // Follow the redirect to render the interstitial. + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + response.assert_header_value(CONTENT_TYPE, "text/html; charset=utf-8"); + let body = response.body(); + assert!( + body.contains("@alice:"), + "should name the trusted target alice, body: {body}" + ); + assert!( + !body.contains("Continue as bob"), + "trusted mismatch must not offer continue-as-current, body: {body}" + ); + } + + /// Logged in as bob, an untrusted `login_hint` for alice → + /// account-selection screen WITH a "continue as bob" option; choosing to + /// continue redirects to consent. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_untrusted_mismatch_continue(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let url = authorize_url(&client_id, "login_hint=mxid%3A%40alice%3Aexample.com"); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + + let location = redirect_path(&response); + assert!( + location.contains("/account-selection"), + "expected account-selection, got {location}" + ); + + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + let body = response.body(); + assert!( + body.contains("Continue as bob"), + "untrusted mismatch must offer continue-as-current, body: {body}" + ); + // Echoes the client-supplied hint. + assert!( + body.contains("@alice:example.com"), + "should echo the login_hint, body: {body}" + ); + + // Extract CSRF and POST "continue". + let csrf_token = body + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + let request = cookies.with_cookies(Request::post(&location).form(serde_json::json!({ + "csrf": csrf_token, + "action": "continue", + }))); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + assert!( + redirect_path(&response).contains("/consent/"), + "continue-as-current should land on consent" + ); + } + + /// Logged in as alice, a trusted hint for alice (a match) → straight to + /// consent, no interstitial. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_match_goes_to_consent(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "alice").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + assert!( + redirect_path(&response).contains("/consent/"), + "a matching session should skip straight to consent" + ); + } + + /// "Sign in as target" POST clears the session cookie and redirects to + /// /login continuing the grant. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_switch_clears_session(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + let body = response.body(); + let csrf_token = body + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + + let request = cookies.with_cookies(Request::post(&location).form(serde_json::json!({ + "csrf": csrf_token, + "action": "switch", + }))); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + + // It should redirect to /login continuing the grant... + let dest = redirect_path(&response); + assert!(dest.starts_with("/login"), "expected /login, got {dest}"); + + // ...and forget the session cookie (Set-Cookie with an emptied/expired + // session jar). + let set_cookie = response + .headers() + .get_all(hyper::header::SET_COOKIE) + .iter() + .map(|v| v.to_str().unwrap()) + .collect::>() + .join("\n"); + assert!( + set_cookie.contains("session"), + "the switch response should rewrite the session cookie, got: {set_cookie}" + ); + } + + /// Extract the grant id from the `id` query parameter of an + /// `/account-selection?kind=...&id=` location. + fn grant_id_from_selection(location: &str) -> &str { + let query = location + .split_once('?') + .expect("expected an account-selection location with a query") + .1; + query + .split('&') + .find_map(|pair| pair.strip_prefix("id=")) + .expect("expected an `id` query parameter") + } + + /// Trusted mismatch: hitting `/consent/{grant_id}` directly while logged in + /// as the wrong user (session=bob, target=alice) must NOT render consent — + /// it diverts back to the account-selection screen. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_consent_trusted_mismatch_diverts(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + let grant_id = grant_id_from_selection(&location); + + // Hit consent directly, bypassing the interstitial. + let consent = format!("/consent/{grant_id}"); + let request = cookies.with_cookies(Request::get(&consent).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + let dest = redirect_path(&response); + assert!( + dest.contains("/account-selection"), + "trusted mismatch at consent must divert to account-selection, got {dest}" + ); + } + + /// Trusted grant + POST `action=continue` to the account-selection screen + /// must be rejected: it must NOT reach consent (the screen never offers + /// this action for a trusted target). + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_trusted_continue_rejected(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + + // Render the interstitial to grab a valid CSRF token. + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + let body = response.body(); + let csrf_token = body + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + + // A crafted `continue` POST with a valid CSRF token must not complete + // the trusted grant. + let request = cookies.with_cookies(Request::post(&location).form(serde_json::json!({ + "csrf": csrf_token, + "action": "continue", + }))); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + let dest = redirect_path(&response); + assert!( + !dest.contains("/consent/"), + "a trusted `continue` must not reach consent, got {dest}" + ); + assert!( + dest.contains("/account-selection"), + "a trusted `continue` should be sent back to account-selection, got {dest}" + ); + } + + /// Untrusted mismatch (`login_hint` only) at consent must still proceed — + /// it must NOT be diverted (guards against an interstitial ↔ consent + /// loop). + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_consent_untrusted_mismatch_proceeds(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let url = authorize_url(&client_id, "login_hint=mxid%3A%40alice%3Aexample.com"); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + let grant_id = grant_id_from_selection(&location); + + let consent = format!("/consent/{grant_id}"); + let request = cookies.with_cookies(Request::get(&consent).empty()); + let response = state.request(request).await; + // Renders consent (200) rather than diverting back to account-selection. + response.assert_status(StatusCode::OK); + let dest = response.headers().get(LOCATION); + assert!( + dest.is_none(), + "untrusted mismatch at consent must not be diverted" + ); + } + + /// Match (session=alice, target=alice) at consent proceeds as before: + /// the authorize handler sends a matching session straight to consent. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_consent_match_proceeds(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "alice").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + assert!( + location.contains("/consent/"), + "a matching session should land on consent, got {location}" + ); + + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + let dest = response.headers().get(LOCATION); + assert!(dest.is_none(), "a matching session must render consent"); + } + + /// "Cancel" POST returns an `access_denied` error to the client callback. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_cancel_access_denied(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + + let alice = user_with_password(&state, "alice").await; + user_with_password(&state, "bob").await; + let client_id = provision_client(&state).await; + let cookies = login(&state, "bob").await; + + let hint = mint_hint(&state, &alice.sub); + let url = authorize_url(&client_id, &format!("id_token_hint={hint}")); + let request = cookies.with_cookies(Request::get(&url).empty()); + let response = state.request(request).await; + let location = redirect_path(&response); + + let request = cookies.with_cookies(Request::get(&location).empty()); + let response = state.request(request).await; + let body = response.body(); + let csrf_token = body + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + + let request = cookies.with_cookies(Request::post(&location).form(serde_json::json!({ + "csrf": csrf_token, + "action": "cancel", + }))); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + let dest = redirect_path(&response); + assert!( + dest.starts_with("https://example.com/redirect") + && dest.contains("error=access_denied"), + "cancel should redirect to the client callback with access_denied, got {dest}" + ); + assert!( + dest.contains("state=somestate"), + "cancel callback should carry the grant state, got {dest}" + ); + } +} diff --git a/crates/handlers/src/views/shared.rs b/crates/handlers/src/views/shared.rs index b0c7b8ac1..6b6c32f1d 100644 --- a/crates/handlers/src/views/shared.rs +++ b/crates/handlers/src/views/shared.rs @@ -1,3 +1,4 @@ +// Copyright 2025, 2026 Element Creations Ltd. // Copyright 2024, 2025 New Vector Ltd. // Copyright 2021-2024 The Matrix.org Foundation C.I.C. // @@ -123,28 +124,34 @@ pub(crate) struct QueryLoginHint { } impl QueryLoginHint { - /// Parse a `login_hint` - /// - /// Returns `LoginHint::MXID` for valid mxid 'mxid:@john.doe:example.com' - /// - /// Returns `LoginHint::Email` for valid email 'john.doe@example.com' - /// - /// Otherwise returns `LoginHint::None` + /// Parse the `login_hint` query parameter, if any pub fn parse_login_hint(&self, homeserver: &str) -> LoginHint<'_> { let Some(login_hint) = &self.login_hint else { return LoginHint::None; }; - if let Some(value) = login_hint.strip_prefix("mxid:") - && let Ok(mxid) = <&UserId>::try_from(value) - && mxid.server_name() == homeserver - { - LoginHint::Mxid(mxid) - } else if let Ok(email) = lettre::Address::from_str(login_hint) { - LoginHint::Email(email) - } else { - LoginHint::None - } + parse_login_hint(login_hint, homeserver) + } +} + +/// Parse a `login_hint` +/// +/// Returns `LoginHint::Mxid` for a valid mxid on this homeserver +/// ('mxid:@john.doe:example.com') +/// +/// Returns `LoginHint::Email` for a valid email ('john.doe@example.com') +/// +/// Otherwise returns `LoginHint::None` +pub(crate) fn parse_login_hint<'a>(login_hint: &'a str, homeserver: &str) -> LoginHint<'a> { + if let Some(value) = login_hint.strip_prefix("mxid:") + && let Ok(mxid) = <&UserId>::try_from(value) + && mxid.server_name() == homeserver + { + LoginHint::Mxid(mxid) + } else if let Ok(email) = lettre::Address::from_str(login_hint) { + LoginHint::Email(email) + } else { + LoginHint::None } } diff --git a/crates/router/src/endpoints.rs b/crates/router/src/endpoints.rs index 239f24b25..899604830 100644 --- a/crates/router/src/endpoints.rs +++ b/crates/router/src/endpoints.rs @@ -1,3 +1,4 @@ +// Copyright 2025, 2026 Element Creations Ltd. // Copyright 2024, 2025 New Vector Ltd. // Copyright 2022-2024 The Matrix.org Foundation C.I.C. // @@ -588,6 +589,52 @@ impl Route for Consent { } } +/// `GET|POST /account-selection` +/// +/// The account-mismatch interstitial shown while continuing a post-auth action +/// (currently only an authorization grant) whose requested identity (a verified +/// `id_token_hint` target, or an untrusted `login_hint`) doesn't match the +/// active session. The action it should continue is carried as a query +/// parameter, mirroring the `Login`/`Register` routes. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SelectAccount { + #[serde(flatten)] + post_auth_action: PostAuthAction, +} + +impl SelectAccount { + /// Build a `SelectAccount` route for the given post-auth action. + #[must_use] + pub const fn new(action: PostAuthAction) -> Self { + Self { + post_auth_action: action, + } + } + + /// Build a `SelectAccount` route continuing the given authorization grant. + #[must_use] + pub const fn continue_grant(id: Ulid) -> Self { + Self::new(PostAuthAction::continue_grant(id)) + } + + /// Get the post-auth action this route should continue. + #[must_use] + pub fn post_auth_action(&self) -> &PostAuthAction { + &self.post_auth_action + } +} + +impl Route for SelectAccount { + type Query = PostAuthAction; + fn route() -> &'static str { + "/account-selection" + } + + fn query(&self) -> Option<&Self::Query> { + Some(&self.post_auth_action) + } +} + /// `GET|POST /_matrix/client/v3/login` pub struct CompatLogin; diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 63c848d02..ed3fe0156 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -678,6 +678,58 @@ impl WelcomeBackContext { } } +/// Context used by the `pages/account_selection.html` template +/// +/// Rendered when continuing an authorization grant whose requested identity +/// doesn't match the active session: a verified `id_token_hint` target +/// (`trusted = true`, the resolved username) or an untrusted `login_hint` +/// (`trusted = false`, the raw client-supplied string echoed back). +/// +/// For a trusted mismatch the only options are "sign in as the requested user" +/// or "cancel" — per OIDC Core, MAS must not return a token for a different +/// user. An untrusted mismatch additionally offers "continue as the current +/// user", since `login_hint` is purely advisory. +#[derive(Serialize)] +pub struct SelectAccountContext { + /// The requested identity, as a display string. For a trusted target this + /// is the resolved username; for an untrusted hint it is the raw + /// client-supplied `login_hint` string echoed verbatim. + requested: String, + /// Whether the requested identity came from a verified `id_token_hint`. + trusted: bool, + /// The username of the currently-active session. + current_username: String, +} + +impl TemplateContext for SelectAccountContext { + fn sample( + _now: chrono::DateTime, + _rng: &mut R, + _locales: &[DataLocale], + ) -> BTreeMap + where + Self: Sized, + { + sample_list(vec![SelectAccountContext { + requested: "@alice:example.com".to_owned(), + trusted: true, + current_username: "bob".to_owned(), + }]) + } +} + +impl SelectAccountContext { + /// Construct a context for the account-mismatch interstitial. + #[must_use] + pub fn new(requested: String, trusted: bool, current_username: String) -> Self { + Self { + requested, + trusted, + current_username, + } + } +} + /// Fields of the registration form #[derive(Serialize, Deserialize, Debug, Clone, Copy, Hash, PartialEq, Eq)] #[serde(rename_all = "snake_case")] diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index dda5b09f8..4e281a638 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -49,10 +49,10 @@ pub use self::{ RegisterStepsDisplayNameContext, RegisterStepsDisplayNameFormField, RegisterStepsEmailInUseContext, RegisterStepsRegistrationTokenContext, RegisterStepsRegistrationTokenFormField, RegisterStepsVerifyEmailContext, - RegisterStepsVerifyEmailFormField, SiteBranding, SiteConfigExt, SiteFeatures, - TemplateContext, UpstreamExistingLinkContext, UpstreamRegister, UpstreamRegisterFormField, - UpstreamSuggestLink, WelcomeBackContext, WithCaptcha, WithCsrf, WithLanguage, - WithOptionalSession, WithSession, + RegisterStepsVerifyEmailFormField, SelectAccountContext, SiteBranding, SiteConfigExt, + SiteFeatures, TemplateContext, UpstreamExistingLinkContext, UpstreamRegister, + UpstreamRegisterFormField, UpstreamSuggestLink, WelcomeBackContext, WithCaptcha, WithCsrf, + WithLanguage, WithOptionalSession, WithSession, }, forms::{FieldError, FormError, FormField, FormState, ToFormState}, }; @@ -375,6 +375,9 @@ register_templates! { /// Render the streamlined "welcome back" re-authentication page pub fn render_welcome_back(WithLanguage>) { "pages/login/welcome_back.html" } + /// Render the account-mismatch interstitial + pub fn render_select_account(WithLanguage>) { "pages/account_selection.html" } + /// Render the registration page pub fn render_register(WithLanguage>) { "pages/register/index.html" } diff --git a/templates/pages/account_selection.html b/templates/pages/account_selection.html new file mode 100644 index 000000000..3efba94ab --- /dev/null +++ b/templates/pages/account_selection.html @@ -0,0 +1,43 @@ +{# +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +-#} + +{% extends "base.html" %} + +{% block content %} +
+
+ {{ icon.user_profile_solid() }} +
+ +
+

{{ _("mas.select_account.headline") }}

+

+ {{ _("mas.select_account.description", current=current_username, requested=requested) }} +

+
+
+ +
+
+ + + + + {% if not trusted %} + + {% endif %} + + +
+
+{% endblock content %} diff --git a/translations/en.json b/translations/en.json index e0ce33465..c59f3b49e 100644 --- a/translations/en.json +++ b/translations/en.json @@ -6,7 +6,7 @@ }, "cancel": "Cancel", "@cancel": { - "context": "pages/consent.html:81:11-29, pages/device_consent.html:179:13-31, pages/device_link.html:45:33-51, pages/login/welcome_back.html:59:13-31, pages/policy_violation.html:70:15-33" + "context": "pages/account_selection.html:39:11-29, pages/consent.html:81:11-29, pages/device_consent.html:179:13-31, pages/device_link.html:45:33-51, pages/login/welcome_back.html:59:13-31, pages/policy_violation.html:70:15-33" }, "continue": "Continue", "@continue": { @@ -746,6 +746,24 @@ "description": "Displayed when the 'openid' scope is requested" } }, + "select_account": { + "continue_as": "Continue as %(username)s", + "@continue_as": { + "context": "pages/account_selection.html:34:13-75" + }, + "description": "You are signed in as %(current)s, but the application requested %(requested)s.", + "@description": { + "context": "pages/account_selection.html:19:11-93" + }, + "headline": "Choose an account", + "@headline": { + "context": "pages/account_selection.html:17:27-59" + }, + "sign_in_as": "Sign in as %(username)s", + "@sign_in_as": { + "context": "pages/account_selection.html:29:11-65" + } + }, "upstream_oauth2": { "link_mismatch": { "heading": "This upstream account is already linked to another account.",