diff --git a/Cargo.lock b/Cargo.lock index 992b994..a859612 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "am-cloud-client" -version = "0.2.2" +version = "0.3.0" dependencies = [ "am-cloud-types", "am-core-types", @@ -30,7 +30,7 @@ dependencies = [ [[package]] name = "am-cloud-types" -version = "0.2.2" +version = "0.3.0" dependencies = [ "am-core-types", "anyhow", @@ -47,7 +47,7 @@ dependencies = [ [[package]] name = "am-core-types" -version = "0.2.2" +version = "0.3.0" dependencies = [ "chrono", "serde", @@ -149,7 +149,7 @@ checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" [[package]] name = "atomicmemory" -version = "0.2.2" +version = "0.3.0" dependencies = [ "am-cloud-client", "am-cloud-types", diff --git a/Cargo.toml b/Cargo.toml index 6ecbe5b..886ff94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ lto = "thin" codegen-units = 1 [workspace.package] -version = "0.2.2" +version = "0.3.0" edition = "2024" rust-version = "1.88" license = "Apache-2.0" @@ -63,7 +63,7 @@ indicatif = "0.18" wiremock = "0.6.5" clap = { version = "4", features = ["derive", "env"] } fs4 = { version = "0.12.0", features = ["sync"] } -am-core-types = { path = "crates/core-types", version = "0.2.2" } -am-cloud-types = { path = "crates/cloud-types", version = "0.2.2" } -am-cloud-client = { path = "crates/cloud-client", version = "0.2.2" } -atomicmemory = { path = "crates/cli", version = "0.2.2" } +am-core-types = { path = "crates/core-types", version = "0.3.0" } +am-cloud-types = { path = "crates/cloud-types", version = "0.3.0" } +am-cloud-client = { path = "crates/cloud-client", version = "0.3.0" } +atomicmemory = { path = "crates/cli", version = "0.3.0" } diff --git a/crates/cli/README.md b/crates/cli/README.md index 8fd89b0..ce2fdfa 100644 --- a/crates/cli/README.md +++ b/crates/cli/README.md @@ -208,12 +208,19 @@ machines therefore keep independent credentials. ### Token fallback -Paste a dashboard session JWT when browser OAuth is unavailable: +On a remote or headless host, prefer device login (refreshable Cloud session): + +```bash +am auth login --device +``` + +Paste a dashboard session JWT only for a short session (no refresh token): ```bash am auth login --token "eyJ..." ``` +`--no-browser` uses the same device flow as `--device`. ## Defaults | Setting | Default | diff --git a/crates/cli/src/auth/claims.rs b/crates/cli/src/auth/claims.rs index 1bceb4d..1edfdf6 100644 --- a/crates/cli/src/auth/claims.rs +++ b/crates/cli/src/auth/claims.rs @@ -48,7 +48,7 @@ pub fn token_has_active_org(id_token: &str) -> bool { pub fn missing_org_login_hint() -> &'static str { "Session has no active organization — run `am init` to bootstrap a personal workspace, \ - or `am auth login --token ` from memory.dev with an org selected." + `am auth login --device`, or `am auth login --token ` from memory.dev with an org selected." } pub fn decode_id_token(id_token: &str) -> Result { diff --git a/crates/cli/src/auth/clerk_oauth.rs b/crates/cli/src/auth/clerk_oauth.rs index 1fc2fe7..db7208e 100644 --- a/crates/cli/src/auth/clerk_oauth.rs +++ b/crates/cli/src/auth/clerk_oauth.rs @@ -3,13 +3,13 @@ use anyhow::{Result, bail}; use crate::config::ConfigFile; -use crate::environment::{Environment, is_production_api_url}; +use crate::environment::{Environment, is_first_party_cloud_api_url, is_production_api_url}; /// Accept a stored/env OAuth value only if it is not the shipped production /// credential. /// -/// Reaching this point means the base URL is NOT the production origin, so the -/// production issuer/client_id must never be used: an older CLI seeded them +/// Reaching this point means the base URL is NOT a first-party Cloud origin, so +/// the production issuer/client_id must never be used: an older CLI seeded them /// into `config.toml`, and reading them back would hand the production identity /// to an arbitrary `--base-url` — the bearer token is then attached to that /// origin. Fail closed instead and require explicit configuration. @@ -19,7 +19,7 @@ fn usable_for_custom_origin(value: Option, shipped_production: &str) -> /// Public OAuth `client_id` for end-user login (PKCE). Never uses `CLERK_SECRET_KEY`. /// -/// Production API URL: CLI `--client-id` → baked prod preset → config → env. +/// First-party API URL (prod / Dev / staging): CLI `--client-id` → baked preset → … /// Custom API URL: `--client-id` → config → env (fail closed — never use prod OAuth). pub fn resolve_public_client_id( config: &ConfigFile, @@ -29,7 +29,7 @@ pub fn resolve_public_client_id( if let Some(id) = flag_override { return Ok(id); } - if is_production_api_url(base_url) { + if is_first_party_cloud_api_url(base_url) { return Ok(Environment::PROD_OAUTH_CLIENT_ID.to_string()); } if let Some(id) = usable_for_custom_origin( @@ -49,7 +49,10 @@ pub fn resolve_public_client_id( bail!( "browser login is not configured in this CLI build yet.\n\ \n\ - Sign in via the web console and run:\n\ + Preferred on remote/headless hosts (refreshable):\n\ + am auth login --device\n\ + \n\ + Short paste session from the web console:\n\ am auth login --token \n\ \n\ Or run `am auth doctor` to diagnose OAuth configuration." @@ -62,14 +65,16 @@ pub fn resolve_public_client_id( Set issuer and client_id in config.toml, or run:\n\ am auth login --issuer --client-id \n\ \n\ - Or sign in via the web console:\n\ + Or use device login on remote hosts:\n\ + am auth login --device\n\ + Or sign in via the web console (short paste, no refresh):\n\ am auth login --token " ) } /// Resolve the OAuth issuer for a Cloud API base URL. /// -/// Production API URL: CLI `--issuer` → baked prod preset. +/// First-party API URL (prod / Dev / staging): CLI `--issuer` → baked preset. /// Custom API URL: `--issuer` → env → config (fail closed). /// /// The production issuer and client_id are shipped as ONE pair. Consulting a @@ -85,7 +90,7 @@ fn resolve_issuer( if let Some(issuer) = flag_override.filter(|s| !s.is_empty()) { return Ok(issuer); } - if is_production_api_url(base_url) { + if is_first_party_cloud_api_url(base_url) { return Ok(Environment::PROD_OAUTH_ISSUER.to_string()); } if let Some(issuer) = usable_for_custom_origin( @@ -117,10 +122,26 @@ pub fn resolve_oauth_pair( Ok((issuer, client_id)) } +/// Refuse blank `--issuer` / `--client-id` overrides before any login work. +/// +/// Both login paths persist overrides into the global `[oauth]` table. A blank +/// value is not "no override": persisted, it replaces a working pair for every +/// profile and every later command. Omitting the flag is the way to use the +/// configured pair. +pub fn reject_blank_oauth_overrides(issuer: Option<&str>, client_id: Option<&str>) -> Result<()> { + for (flag, value) in [("--issuer", issuer), ("--client-id", client_id)] { + if value.is_some_and(|value| value.trim().is_empty()) { + bail!("{flag} must not be blank; omit it to use the configured OAuth pair"); + } + } + Ok(()) +} + pub fn invalid_client_help() -> &'static str { "The OAuth client_id in this CLI build is not accepted by Clerk (invalid_client).\n\ Run `am auth doctor` to diagnose (checks env overrides and Clerk registration).\n\ - Fallback: am auth login --token " + Remote/headless: am auth login --device\n\ + Short paste: am auth login --token " } #[cfg(test)] @@ -213,6 +234,49 @@ mod tests { assert_eq!(client_id, "staging-client"); } + #[test] + fn blank_oauth_overrides_are_refused_and_omitted_ones_allowed() { + for (issuer, client_id, flag) in [ + (Some(""), None, "--issuer"), + (Some(" "), None, "--issuer"), + (None, Some(""), "--client-id"), + (None, Some(" \t"), "--client-id"), + ] { + let err = reject_blank_oauth_overrides(issuer, client_id) + .expect_err("blank override must be refused") + .to_string(); + assert!(err.contains(&format!("{flag} must not be blank")), "{err}"); + } + reject_blank_oauth_overrides(None, None).unwrap(); + reject_blank_oauth_overrides(Some("https://clerk.example"), Some("client")).unwrap(); + } + + #[test] + fn first_party_dev_url_uses_shipped_oauth_pair_without_config() { + // ATO-2321: api.dev is first-party; whoami/doctor must not demand a + // non-production issuer in config.toml (prod issuer there was filtered). + let config = ConfigFile::default(); + let (issuer, client_id) = + resolve_oauth_pair(&config, "https://api.dev.atomicstrata.ai", None, None).unwrap(); + assert_eq!(issuer, Environment::PROD_OAUTH_ISSUER); + assert_eq!(client_id, Environment::PROD_OAUTH_CLIENT_ID); + } + + #[test] + fn first_party_dev_url_ignores_stale_custom_config_pair() { + let config = ConfigFile { + oauth: crate::config::OAuthDefaults { + issuer: Some("https://clerk.custom.example".into()), + client_id: Some("stale-client".into()), + }, + ..Default::default() + }; + let (issuer, client_id) = + resolve_oauth_pair(&config, "https://api.dev.atomicstrata.ai/", None, None).unwrap(); + assert_eq!(issuer, Environment::PROD_OAUTH_ISSUER); + assert_eq!(client_id, Environment::PROD_OAUTH_CLIENT_ID); + } + #[test] fn custom_origin_refuses_the_shipped_production_pair_from_config() { // `default_config()` used to seed config.toml with the production diff --git a/crates/cli/src/auth/device_login.rs b/crates/cli/src/auth/device_login.rs index 567801a..5a8c426 100644 --- a/crates/cli/src/auth/device_login.rs +++ b/crates/cli/src/auth/device_login.rs @@ -7,10 +7,15 @@ use anyhow::{Context, Result, bail}; use reqwest::Url; use tokio::time::sleep; +use crate::auth::claims::decode_id_token; +use crate::auth::clerk_oauth::{reject_blank_oauth_overrides, resolve_oauth_pair}; use crate::auth::http; use crate::auth::login_feedback::LoginFeedback; +use crate::auth::origin::same_origin; use crate::auth::setup::setup_default_project; -use crate::config::{OAuthTokens, load_config, store_oauth, store_profile_base_url}; +use crate::config::{ + ConfigFile, OAuthTokens, load_config, store_oauth, store_profile_base_url, update_config, +}; use crate::output::message; use crate::progress::ProgressReporter; @@ -21,8 +26,13 @@ pub struct DeviceLoginOptions { pub profile: String, pub base_url: String, pub client_id: Option, + /// `--issuer` override. Persisted on success, like browser login, so the + /// stored session stays usable and refreshable on a custom origin. + pub issuer: Option, pub quiet: bool, pub verbose: bool, + /// Skip interactive default-project selection after a successful login. + pub skip_project_select: bool, } pub async fn run_device_login( @@ -32,6 +42,15 @@ pub async fn run_device_login( ) -> Result<()> { let feedback = LoginFeedback::detect(opts.verbose, opts.quiet); let step_id = progress_step.unwrap_or("identity"); + // Every later command authorizes and refreshes this session through the + // OAuth pair resolved for its origin. Resolve it before the flow starts: + // otherwise login could store a session no subsequent command can use. + let (expected_issuer, _) = device_oauth_pair( + load_config()?, + &opts.base_url, + opts.issuer.as_deref(), + opts.client_id.as_deref(), + )?; let base = Url::parse(&opts.base_url).context("parse cloud base_url")?; let http = http::client()?; @@ -103,19 +122,13 @@ pub async fn run_device_login( if resp.status().is_success() { let token: DeviceTokenResponse = resp.json().await.context("decode device token")?; - store_oauth( - &opts.profile, - OAuthTokens { - id_token: token.id_token, - refresh_token: token.refresh_token, - expires_at: Some(chrono::Utc::now().timestamp() + token.expires_in as i64), - issuer: load_config().ok().and_then(|c| c.oauth.issuer), - api_origin: None, - }, - &opts.base_url, - )?; + let tokens = oauth_tokens_from_device_response(token, &expected_issuer)?; + persist_oauth_overrides(opts.issuer.clone(), opts.client_id.clone())?; + store_oauth(&opts.profile, tokens, &opts.base_url)?; store_profile_base_url(&opts.profile, &opts.base_url)?; - setup_default_project(&opts.profile, false, Some(&opts.base_url)).await?; + if !opts.skip_project_select { + setup_default_project(&opts.profile, false, Some(&opts.base_url)).await?; + } if feedback.show_success() { message(!opts.quiet, "Device login complete."); } @@ -141,15 +154,124 @@ pub async fn run_device_login( bail!("device login timed out waiting for activation") } +/// Resolve the OAuth pair a device session will be used and refreshed with. +/// +/// Later commands resolve the pair from configuration alone, and login +/// persists `--issuer` / `--client-id` into configuration, so apply the +/// overrides to a copy and resolve exactly as a later command would. An +/// override that resolution then ignores must be refused before the flow +/// starts: first-party origins always use the shipped pair, +/// `ATOMICMEMORY_OAUTH_ISSUER` outranks configuration, and the shipped pair is +/// refused on a custom origin. Any of those would yield a session that every +/// later command rejects. +fn device_oauth_pair( + mut config: ConfigFile, + base_url: &str, + issuer: Option<&str>, + client_id: Option<&str>, +) -> Result<(String, String)> { + reject_blank_oauth_overrides(issuer, client_id)?; + if let Some(issuer) = issuer { + config.oauth.issuer = Some(issuer.to_string()); + } + if let Some(client_id) = client_id { + config.oauth.client_id = Some(client_id.to_string()); + } + let (resolved_issuer, resolved_client_id) = resolve_oauth_pair(&config, base_url, None, None)?; + if let Some(issuer) = issuer + && !same_origin(issuer, &resolved_issuer) + { + bail!( + "--issuer {issuer} would not be used after login: commands against {base_url} \ + resolve the issuer {resolved_issuer} (first-party origins use the shipped \ + OAuth pair, and ATOMICMEMORY_OAUTH_ISSUER takes precedence over configuration).\n\ + Drop --issuer, or unset ATOMICMEMORY_OAUTH_ISSUER, so the session stays usable." + ); + } + if let Some(client_id) = client_id + && client_id != resolved_client_id + { + bail!( + "--client-id {client_id} would not be used after login: commands against \ + {base_url} resolve the client id {resolved_client_id}, so the session could not \ + be refreshed.\nDrop --client-id for this origin." + ); + } + Ok((resolved_issuer, resolved_client_id)) +} + +/// Persist explicit `--issuer` / `--client-id` overrides, as browser login does. +/// +/// Refresh and authorization resolve the OAuth pair from configuration, so a +/// pair supplied only on the login command line must outlive this process. +fn persist_oauth_overrides(issuer: Option, client_id: Option) -> Result<()> { + if issuer.is_none() && client_id.is_none() { + return Ok(()); + } + update_config(|cfg| { + if let Some(issuer) = issuer { + cfg.oauth.issuer = Some(issuer); + } + if let Some(client_id) = client_id { + cfg.oauth.client_id = Some(client_id); + } + Ok(()) + }) +} + +/// Map a successful device-token response into stored OAuth credentials. +/// +/// Device login is the durable VPS path: a missing `refresh_token` must fail +/// closed rather than store an id-token-only session that cannot survive +/// process restarts (ATO-2321). A token from a different issuer than the one +/// this origin resolves to is refused for the same reason: origin checks would +/// reject every later use of it. +fn oauth_tokens_from_device_response( + token: DeviceTokenResponse, + expected_issuer: &str, +) -> Result { + let refresh_token = match token.refresh_token { + Some(value) if !value.is_empty() => value, + _ => bail!( + "device token response omitted refresh_token — Cloud must return a refreshable \ + session for `am auth login --device` (see ATO-2321 / am-cloud-api device token).\n\ + Browser login on a local host still stores a refreshable session; paste login \ + (`--token`) remains short-lived." + ), + }; + let issuer = match decode_id_token(&token.id_token) + .ok() + .and_then(|claims| claims.iss) + { + Some(iss) if same_origin(&iss, expected_issuer) => iss, + Some(iss) => bail!( + "device login returned a session issued by {iss}, but this Cloud origin expects \ + {expected_issuer}; refusing to store a session no later command could use.\n\ + Pass `--issuer ` if this origin uses a different identity provider." + ), + None => expected_issuer.to_string(), + }; + Ok(OAuthTokens { + id_token: token.id_token, + refresh_token: Some(refresh_token), + expires_at: Some(chrono::Utc::now().timestamp() + token.expires_in as i64), + issuer: Some(issuer), + api_origin: None, + }) +} + #[cfg(test)] mod tests { use std::sync::{Arc, Mutex}; + use am_cloud_types::DeviceTokenResponse; use axum::extract::State; use axum::http::{HeaderMap, StatusCode, header}; use axum::response::{IntoResponse, Response}; use axum::routing::post; use axum::{Json, Router}; + use base64::Engine; + use base64::engine::general_purpose::URL_SAFE_NO_PAD; use super::*; @@ -217,9 +339,11 @@ mod tests { DeviceLoginOptions { profile: "test".into(), base_url: format!("http://{address}"), - client_id: None, + client_id: Some("test-client".into()), + issuer: Some("https://issuer.test".into()), quiet: true, verbose: false, + skip_project_select: true, }, None, None, @@ -246,4 +370,221 @@ mod tests { let fb = LoginFeedback::for_test(false, false, true); assert!(fb.concise_tty()); } + + fn synthetic_id_token(iss: &str) -> String { + let payload = format!(r#"{{"sub":"user_1","iss":"{iss}","exp":9999999999}}"#); + let encoded = URL_SAFE_NO_PAD.encode(payload.as_bytes()); + format!("hdr.{encoded}.sig") + } + + #[test] + fn device_response_persists_refresh_token_and_jwt_issuer() { + let issuer = "https://clerk.atomicstrata.ai"; + let tokens = oauth_tokens_from_device_response( + DeviceTokenResponse { + id_token: synthetic_id_token(issuer), + refresh_token: Some("refresh-from-cloud".into()), + token_type: "Bearer".into(), + expires_in: 3600, + }, + issuer, + ) + .unwrap(); + assert_eq!(tokens.refresh_token.as_deref(), Some("refresh-from-cloud")); + assert_eq!(tokens.issuer.as_deref(), Some(issuer)); + assert!(tokens.expires_at.is_some()); + } + + #[test] + fn first_party_origin_without_overrides_uses_the_shipped_pair() { + let (issuer, client_id) = device_oauth_pair( + ConfigFile::default(), + "https://api.dev.atomicstrata.ai", + None, + None, + ) + .unwrap(); + assert_eq!(issuer, crate::environment::Environment::PROD_OAUTH_ISSUER); + assert_eq!( + client_id, + crate::environment::Environment::PROD_OAUTH_CLIENT_ID + ); + } + + #[test] + fn first_party_origin_refuses_overrides_later_commands_ignore() { + let err = device_oauth_pair( + ConfigFile::default(), + "https://api.atomicstrata.ai", + Some("https://clerk.custom.example"), + None, + ) + .expect_err("first-party origins ignore a configured issuer") + .to_string(); + assert!(err.contains("would not be used after login"), "{err}"); + + let err = device_oauth_pair( + ConfigFile::default(), + "https://api.atomicstrata.ai", + None, + Some("custom-client"), + ) + .expect_err("first-party origins ignore a configured client id") + .to_string(); + assert!(err.contains("would not be used after login"), "{err}"); + } + + #[test] + fn custom_origin_uses_overrides_it_will_keep_resolving() { + let (issuer, client_id) = device_oauth_pair( + ConfigFile::default(), + "https://api.custom.example", + Some("https://clerk.custom.example"), + Some("custom-client"), + ) + .unwrap(); + assert_eq!(issuer, "https://clerk.custom.example"); + assert_eq!(client_id, "custom-client"); + } + + #[test] + fn custom_origin_refuses_the_shipped_pair_as_overrides() { + use crate::environment::Environment; + assert!( + device_oauth_pair( + ConfigFile::default(), + "https://api.custom.example", + Some(Environment::PROD_OAUTH_ISSUER), + Some(Environment::PROD_OAUTH_CLIENT_ID), + ) + .is_err(), + "the shipped pair is filtered on custom origins, so it cannot be persisted for one" + ); + } + + #[test] + fn device_response_from_another_issuer_is_refused() { + let err = oauth_tokens_from_device_response( + DeviceTokenResponse { + id_token: synthetic_id_token("https://clerk.other.example"), + refresh_token: Some("refresh".into()), + token_type: "Bearer".into(), + expires_in: 3600, + }, + "https://clerk.atomicstrata.ai", + ) + .expect_err("a session no later command could authorize must not be stored") + .to_string(); + assert!(err.contains("refusing to store"), "{err}"); + assert!(err.contains("--issuer"), "{err}"); + } + + #[test] + fn device_response_without_iss_records_the_resolved_issuer() { + let tokens = oauth_tokens_from_device_response( + DeviceTokenResponse { + id_token: "hdr.e30.sig".into(), + refresh_token: Some("refresh".into()), + token_type: "Bearer".into(), + expires_in: 3600, + }, + "https://clerk.atomicstrata.ai", + ) + .unwrap(); + assert_eq!( + tokens.issuer.as_deref(), + Some("https://clerk.atomicstrata.ai") + ); + } + + #[test] + fn device_response_without_refresh_token_fails_closed() { + let err = oauth_tokens_from_device_response( + DeviceTokenResponse { + id_token: synthetic_id_token("https://clerk.example"), + refresh_token: None, + token_type: "Bearer".into(), + expires_in: 3600, + }, + "https://clerk.example", + ) + .expect_err("id-token-only device response must not be stored") + .to_string(); + assert!( + err.contains("omitted refresh_token"), + "unexpected error: {err}" + ); + } + + #[test] + fn empty_refresh_token_is_treated_as_missing() { + let err = oauth_tokens_from_device_response( + DeviceTokenResponse { + id_token: synthetic_id_token("https://clerk.example"), + refresh_token: Some(String::new()), + token_type: "Bearer".into(), + expires_in: 3600, + }, + "https://clerk.example", + ) + .unwrap_err() + .to_string(); + assert!(err.contains("omitted refresh_token")); + } + + #[tokio::test] + async fn successful_device_poll_refuses_id_token_only_cloud_response() { + #[derive(Clone, Default)] + struct Fixture; + + async fn authorize_ok() -> Response { + Json(serde_json::json!({ + "device_code": "device-code", + "user_code": "user-code", + "verification_uri": "https://example.com/activate", + "verification_uri_complete": "https://example.com/activate?code=user-code", + "expires_in": 600, + "interval": 1 + })) + .into_response() + } + + async fn token_id_only() -> Response { + Json(serde_json::json!({ + "id_token": "hdr.e30.sig", + "token_type": "Bearer", + "expires_in": 3600 + })) + .into_response() + } + + let app = Router::new() + .route("/api/oauth/device/authorize", post(authorize_ok)) + .route("/api/oauth/device/token", post(token_id_only)) + .with_state(Fixture); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let err = run_device_login( + DeviceLoginOptions { + profile: "test-no-refresh".into(), + base_url: format!("http://{address}"), + client_id: Some("test-client".into()), + issuer: Some("https://issuer.test".into()), + quiet: true, + verbose: false, + skip_project_select: true, + }, + None, + None, + ) + .await + .expect_err("must fail closed when Cloud omits refresh_token"); + server.abort(); + assert!( + err.to_string().contains("omitted refresh_token"), + "unexpected error: {err}" + ); + } } diff --git a/crates/cli/src/auth/doctor.rs b/crates/cli/src/auth/doctor.rs index 6bc0d2d..dbfd0b5 100644 --- a/crates/cli/src/auth/doctor.rs +++ b/crates/cli/src/auth/doctor.rs @@ -11,7 +11,7 @@ use crate::auth::token::discover_metadata; use crate::config::{ DEFAULT_OAUTH_CALLBACK_PORT, ensure_config_initialized, load_config, resolve_profile, }; -use crate::environment::is_production_api_url; +use crate::environment::{is_first_party_cloud_api_url, is_production_api_url}; const API_HEALTH_TIMEOUT: Duration = Duration::from_secs(10); @@ -68,7 +68,7 @@ pub async fn run_doctor( "ATOMICMEMORY_OAUTH_CLIENT_ID={env_id} is set but ignored on production — login uses shipped client {client_id}. Run: unset ATOMICMEMORY_OAUTH_CLIENT_ID" )); } - if !is_production_api_url(&api_base) { + if !is_first_party_cloud_api_url(&api_base) { hints.push( "Using a custom Cloud API URL — OAuth issuer and client_id must be configured explicitly." .into(), @@ -136,7 +136,11 @@ pub async fn run_doctor( and the API JWT audience includes {client_id}." )); } - hints.push("Until fixed, use `am auth login --token `.".into()); + hints.push( + "Until fixed, use `am auth login --device` (remote/VPS) or \ + `am auth login --token ` (short paste)." + .into(), + ); } let api_health_ok = match probe_api_health(&api_base).await { diff --git a/crates/cli/src/auth/ensure_org.rs b/crates/cli/src/auth/ensure_org.rs index 665d010..d28db01 100644 --- a/crates/cli/src/auth/ensure_org.rs +++ b/crates/cli/src/auth/ensure_org.rs @@ -70,6 +70,7 @@ pub async fn ensure_org_context( "no organization available and org bootstrap is not deployed on {} \ (POST /api/onboarding/ensure → 404).\n\ • Ensure org bootstrap is deployed on the Cloud API, then re-run `am init`\n\ + • Or finish device login: `am auth login --device`\n\ • Or paste a dashboard JWT with an org selected: `am auth login --token `\n\ • Or finish onboarding at memory.dev, then `am auth login --token `", profile.base_url diff --git a/crates/cli/src/auth/login.rs b/crates/cli/src/auth/login.rs index 1ae8de3..d4bee1b 100644 --- a/crates/cli/src/auth/login.rs +++ b/crates/cli/src/auth/login.rs @@ -12,7 +12,9 @@ use axum::routing::get; use tokio::sync::{Mutex, oneshot}; use crate::auth::auth_wait::wait_for_oneshot; -use crate::auth::clerk_oauth::{invalid_client_help, resolve_oauth_pair, resolve_public_client_id}; +use crate::auth::clerk_oauth::{ + invalid_client_help, reject_blank_oauth_overrides, resolve_oauth_pair, resolve_public_client_id, +}; use crate::auth::doctor::{DoctorOverrides, require_login_ready}; use crate::auth::login_feedback::LoginFeedback; use crate::auth::pkce::{generate_pkce_pair, generate_state}; @@ -26,11 +28,26 @@ use crate::progress::ProgressReporter; const CALLBACK_TIMEOUT: Duration = Duration::from_secs(120); +/// Next steps when loopback browser OAuth cannot finish on this host. +pub fn headless_login_next_steps(open_error: Option<&str>) -> String { + let mut msg = String::new(); + if let Some(err) = open_error { + msg.push_str(&format!("Could not open a browser ({err}).\n")); + } + msg.push_str( + "Browser OAuth uses a loopback callback on this machine, so opening the authorize \ + URL on another host cannot finish login.\n\ + On a remote or headless host, use device login (stores a refreshable Cloud session):\n\ + am auth login --device\n\ + Short paste session (no refresh token): am auth login --token ", + ); + msg +} + #[derive(Debug, Clone)] pub struct LoginOptions { pub profile: String, pub port: Option, - pub no_browser: bool, pub issuer: Option, pub client_id: Option, pub skip_project_select: bool, @@ -50,6 +67,9 @@ pub async fn run_login( ) -> Result<()> { let feedback = LoginFeedback::detect(opts.verbose, opts.quiet); let step_id = progress_step.unwrap_or("identity"); + // Before any config write: browser login persists overrides into the + // global [oauth] table, so a blank one would replace a working pair. + reject_blank_oauth_overrides(opts.issuer.as_deref(), opts.client_id.as_deref())?; let mut config = load_config()?; if let Some(issuer) = opts.issuer.clone() { @@ -127,33 +147,15 @@ pub async fn run_login( if feedback.show_authorize_url() { eprintln!("Authorize URL:\n{authorize_url}\n"); } - if opts.no_browser { - if feedback.show_recovery_hints() { - eprintln!( - "Open that URL in your browser (private window works if a shared session misroutes)." - ); - } else if feedback.concise_tty() { - eprintln!("Open the authorize URL from `am auth login --verbose` if needed."); - } - } else if let Err(err) = open::that(authorize_url.as_str()) { - // Failing the login here would strand the user: on a plain interactive - // TTY show_authorize_url() is false, so the URL was never printed and - // a bare "open browser" error leaves nothing to act on. The callback - // server is already listening, so print the URL unconditionally (even - // under --quiet — login cannot proceed without it) and keep waiting. - eprintln!("Could not open a browser ({err})."); - if !feedback.show_authorize_url() { - eprintln!("Authorize URL:\n{authorize_url}\n"); - } - // The redirect targets 127.0.0.1 on THIS machine, so a browser on - // another device would send the callback to its own loopback and this - // process would wait until timeout. Remote/headless users need the - // token fallback instead. - eprintln!( - "Open that URL in a browser on this machine to continue. On a remote or headless \ - host, cancel and run `am auth login --token ` from the web console." - ); - } else if feedback.concise_tty() { + // Loopback OAuth only works when a browser on THIS host hits redirect_uri. + // A failed open would leave the process stranded — exit with the supported + // remote path instead of waiting on 127.0.0.1. (`--no-browser` never reaches + // here: it selects the device flow before browser login starts.) + if let Err(err) = open::that(authorize_url.as_str()) { + server.abort(); + bail!("{}", headless_login_next_steps(Some(&err.to_string()))); + } + if feedback.concise_tty() { eprintln!("Complete sign-in in your browser…"); } else if feedback.show_waiting_message() { eprintln!("Waiting for browser login on {redirect_uri} …"); @@ -173,11 +175,7 @@ pub async fn run_login( progress, step_id, CALLBACK_TIMEOUT, - if opts.no_browser { - "waiting for authorization" - } else { - "waiting for browser" - }, + "waiting for browser", ) .await .map_err(|err| { @@ -185,13 +183,15 @@ pub async fn run_login( anyhow::anyhow!( "{err} — no callback received at {redirect_uri}.\n\ Paste the Authorize URL printed above into a private/incognito window and approve access.\n\ - Fallback: am auth login --token " + Remote/headless: am auth login --device\n\ + Short paste: am auth login --token " ) } else { anyhow::anyhow!( "{err} — no callback received at {redirect_uri}.\n\ Re-run with --verbose for the authorize URL and recovery steps.\n\ - Fallback: am auth login --token " + Remote/headless: am auth login --device\n\ + Short paste: am auth login --token " ) } })?; @@ -209,7 +209,8 @@ pub async fn run_login( "oauth error: {error} — {description}\n\ Omit --no-org for now, or enable the user:org:read scope on the \ Atomic Strata Cloud CLI OAuth app in Clerk Dashboard.\n\ - Fallback: am auth login --token " + Remote/headless: am auth login --device\n\ + Short paste: am auth login --token " ); } bail!("oauth error: {error} — {description}"); diff --git a/crates/cli/src/auth/origin.rs b/crates/cli/src/auth/origin.rs index b9cd8ea..eafc9f0 100644 --- a/crates/cli/src/auth/origin.rs +++ b/crates/cli/src/auth/origin.rs @@ -63,8 +63,8 @@ pub fn check_token_origin( None => bail!( "stored session predates Cloud-origin binding, so the origin it belongs to \ is unknown.\n\ - Run `am auth login` against {target_base_url} (or `am auth login --token …`) \ - to re-establish it." + Run `am auth login` against {target_base_url} (or `am auth login --device` / \ + `am auth login --token …`) to re-establish it." ), } diff --git a/crates/cli/src/auth/token.rs b/crates/cli/src/auth/token.rs index 934082a..25dac77 100644 --- a/crates/cli/src/auth/token.rs +++ b/crates/cli/src/auth/token.rs @@ -248,10 +248,9 @@ async fn complete_bearer_token( return Ok(tokens.id_token); } - let refresh = tokens - .refresh_token - .clone() - .ok_or_else(|| anyhow!("session expired — run `am auth login`"))?; + let Some(refresh) = tokens.refresh_token.clone() else { + return Err(unrefreshable_session_error(&tokens)); + }; let meta = discover_metadata(&issuer).await?; let refreshed = match refresh_tokens(&meta.token_endpoint, &client_id, &refresh).await { Ok(tokens) => tokens, @@ -306,14 +305,59 @@ fn is_refresh_rejection(err: &anyhow::Error) -> bool { }) } -fn token_fresh(tokens: &OAuthTokens) -> bool { - match tokens.expires_at { - Some(exp) => Utc::now().timestamp() + 60 < exp, - None => decode_id_token(&tokens.id_token) +/// Seconds before `exp` at which a refreshable OAuth session is treated as +/// stale so refresh can run before the access/id token dies. +/// +/// Applied only when a refresh token is present. Paste-login Clerk session +/// JWTs have ~60s lifetimes and no refresh token; requiring this skew there +/// rejected a just-issued JWT immediately (ATO-2308). +const REFRESH_SKEW_SECS: i64 = 60; + +fn token_expiry(tokens: &OAuthTokens) -> Option { + tokens.expires_at.or_else(|| { + decode_id_token(&tokens.id_token) .ok() - .and_then(|c| c.exp) - .map(|exp| Utc::now().timestamp() + 60 < exp) - .unwrap_or(true), + .and_then(|claims| claims.exp) + }) +} + +/// Whether the stored id/access token can be used as a bearer without refresh. +/// +/// Refreshable sessions use a lead-time skew so refresh runs before expiry. +/// Unrefreshable sessions (pasted Clerk JWTs) are fresh for their full +/// remaining lifetime — still-unexpired is enough. +fn token_fresh(tokens: &OAuthTokens) -> bool { + let Some(exp) = token_expiry(tokens) else { + // No expiry claim: treat as usable (caller still subject to API 401). + return true; + }; + let now = Utc::now().timestamp(); + if tokens.refresh_token.is_some() { + now + REFRESH_SKEW_SECS < exp + } else { + now < exp + } +} + +fn unrefreshable_session_error(tokens: &OAuthTokens) -> anyhow::Error { + match token_expiry(tokens) { + Some(exp) if Utc::now().timestamp() >= exp => anyhow!( + "session JWT is expired — paste a fresh Clerk session JWT \ + (`am auth login --token`) or run browser `am auth login`" + ), + Some(exp) => { + // Remaining life was shorter than REFRESH_SKEW_SECS would demand, + // but this path is for tokens without a refresh token — after + // ATO-2308 they are accepted while `now < exp`, so this branch is + // defensive (clock race / future skew changes). + let remaining = (exp - Utc::now().timestamp()).max(0); + anyhow!( + "session JWT has only {remaining}s remaining and cannot be refreshed \ + (pasted tokens have no refresh token) — paste a fresh JWT or run \ + `am auth login`" + ) + } + None => anyhow!("session expired — run `am auth login`"), } } @@ -545,6 +589,36 @@ mod tests { assert_eq!(session.tokens.id_token, "header.payload.sig"); } + #[test] + fn device_session_on_dev_api_authorizes_without_config_oauth() { + // ATO-2321: after device login against api.dev, whoami must not bail on + // "requires explicit OAuth issuer" when config.toml has empty [oauth]. + let mut config = ConfigFile::default(); + config.profiles.insert( + "qa-dev".into(), + crate::config::ProfileConfig { + base_url: Some("https://api.dev.atomicstrata.ai".into()), + ..Default::default() + }, + ); + let mut creds = CredentialsFile::default(); + creds.oauth.insert( + "qa-dev".into(), + OAuthTokens { + id_token: "header.payload.sig".into(), + refresh_token: Some("refresh".into()), + expires_at: Some(Utc::now().timestamp() + 3600), + issuer: Some(Environment::PROD_OAUTH_ISSUER.into()), + api_origin: Some("https://api.dev.atomicstrata.ai".into()), + }, + ); + let session = + authorize_stored_session(&config, &creds, "qa-dev", "https://api.dev.atomicstrata.ai") + .expect("first-party Dev must resolve OAuth without config"); + assert_eq!(session.issuer, Environment::PROD_OAUTH_ISSUER); + assert_eq!(session.client_id, Environment::PROD_OAUTH_CLIENT_ID); + } + #[test] fn stored_session_is_refused_for_a_shared_issuer_on_another_origin() { // Both origins use the SAME configured issuer, so only the recorded @@ -622,4 +696,104 @@ mod tests { let err = anyhow!("token refresh").context("operation timed out"); assert!(!is_refresh_rejection(&err)); } + + /// Synthetic unsigned JWT for expiry tests (decode path only; no verify). + fn synthetic_jwt(iat: i64, exp: i64) -> String { + use base64::Engine; + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + let payload = format!(r#"{{"sub":"user_test","iat":{iat},"exp":{exp}}}"#); + let encoded = URL_SAFE_NO_PAD.encode(payload.as_bytes()); + format!("hdr.{encoded}.sig") + } + + fn paste_tokens(iat: i64, exp: i64) -> OAuthTokens { + OAuthTokens { + id_token: synthetic_jwt(iat, exp), + refresh_token: None, + expires_at: Some(exp), + issuer: Some(Environment::PROD_OAUTH_ISSUER.into()), + api_origin: Some(Environment::PROD_BASE_URL.into()), + } + } + + fn oauth_tokens(exp: i64, with_refresh: bool) -> OAuthTokens { + OAuthTokens { + id_token: synthetic_jwt(exp - 3600, exp), + refresh_token: with_refresh.then(|| "refresh-token".into()), + expires_at: Some(exp), + issuer: Some(Environment::PROD_OAUTH_ISSUER.into()), + api_origin: Some(Environment::PROD_BASE_URL.into()), + } + } + + #[test] + fn sixty_second_clerk_paste_jwt_is_fresh_without_refresh() { + // ATO-2308: Clerk dashboard session JWTs often have exp-iat = 60 and + // no refresh token. The old `now + 60 < exp` skew rejected them at once. + let now = Utc::now().timestamp(); + let tokens = paste_tokens(now, now + 60); + assert!( + token_fresh(&tokens), + "just-issued 60s-lifetime paste JWT must be usable for project setup" + ); + } + + #[test] + fn sixty_second_paste_jwt_fresh_via_claim_when_expires_at_unset() { + let now = Utc::now().timestamp(); + let mut tokens = paste_tokens(now, now + 60); + tokens.expires_at = None; + assert!(token_fresh(&tokens)); + } + + #[test] + fn already_expired_paste_jwt_is_not_fresh() { + let now = Utc::now().timestamp(); + let tokens = paste_tokens(now - 120, now - 1); + assert!(!token_fresh(&tokens)); + let err = unrefreshable_session_error(&tokens).to_string(); + assert!( + err.contains("session JWT is expired"), + "expired paste must say the JWT is expired, got: {err}" + ); + assert!( + !err.contains("cannot be refreshed"), + "expired path must not use the remaining-life / skew message: {err}" + ); + } + + #[test] + fn oauth_with_refresh_applies_skew_before_expiry() { + let now = Utc::now().timestamp(); + // 30s remaining: without skew this would still be valid; with refresh + // skew we must refresh early instead of using the dying access token. + let with_refresh = oauth_tokens(now + 30, true); + assert!( + !token_fresh(&with_refresh), + "refreshable OAuth must treat token inside the {REFRESH_SKEW_SECS}s skew as stale" + ); + let long_lived = oauth_tokens(now + 3600, true); + assert!(token_fresh(&long_lived)); + } + + #[test] + fn unrefreshable_error_distinguishes_remaining_life_from_expired() { + let now = Utc::now().timestamp(); + let still_valid = paste_tokens(now, now + 15); + let err = unrefreshable_session_error(&still_valid).to_string(); + assert!( + err.contains("remaining") && err.contains("cannot be refreshed"), + "unexpired-but-unusable path must mention remaining life, got: {err}" + ); + assert!(!err.contains("session JWT is expired")); + } + + #[test] + fn oauth_without_refresh_inside_skew_window_still_usable() { + // Same remaining life as the refresh skew case, but paste/no-refresh: + // still-unexpired must win so short-lived Clerk JWTs work. + let now = Utc::now().timestamp(); + let tokens = oauth_tokens(now + 30, false); + assert!(token_fresh(&tokens)); + } } diff --git a/crates/cli/src/commands/auth.rs b/crates/cli/src/commands/auth.rs index 5e8697b..27c0307 100644 --- a/crates/cli/src/commands/auth.rs +++ b/crates/cli/src/commands/auth.rs @@ -4,6 +4,7 @@ use anyhow::Result; use clap::Subcommand; use crate::auth::claims::decode_id_token; +use crate::auth::device_login::{DeviceLoginOptions, run_device_login}; use crate::auth::doctor::{DoctorOverrides, report_ok, run_doctor}; use crate::auth::login::{LoginOptions, run_login}; use crate::auth::token::valid_bearer_token; @@ -14,6 +15,35 @@ use crate::config::{ }; use crate::output::{emit, message}; +/// How `am auth login` authenticates after flag parsing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AuthLoginMethod { + /// Paste a dashboard session JWT (`--token`). + Token, + /// OAuth device flow (headless / remote VPS; refreshable). + Device, + /// Browser OAuth with loopback callback. + Browser, +} + +/// Pick the login path from mutually exclusive / headless-routing flags. +/// +/// `--device` and `--no-browser` both select device flow: loopback OAuth cannot +/// finish when the authorize URL is opened on another machine. +pub fn select_auth_login_method( + has_token: bool, + device: bool, + no_browser: bool, +) -> AuthLoginMethod { + if has_token { + AuthLoginMethod::Token + } else if device || no_browser { + AuthLoginMethod::Device + } else { + AuthLoginMethod::Browser + } +} + #[derive(Debug, Subcommand)] pub enum AuthCommand { /// Log in via browser (OAuth2 PKCE loopback against Clerk) @@ -27,9 +57,13 @@ pub enum AuthCommand { /// Loopback callback port (default 9876; must match Clerk redirect URI) #[arg(long)] port: Option, - #[arg(long)] + /// Skip opening a browser; use OAuth device flow instead (same as `--device`) + #[arg(long, conflicts_with_all = ["token", "port", "fresh", "no_org"])] no_browser: bool, - /// Paste a Clerk session JWT instead of browser OAuth (works immediately) + /// Authenticate via OAuth device flow (preferred for remote / headless hosts) + #[arg(long, conflicts_with_all = ["token", "port", "fresh", "no_org"])] + device: bool, + /// Paste a Clerk session JWT instead of browser OAuth (short-lived; no refresh) #[arg(long)] token: Option, /// Skip interactive default-project selection after login @@ -78,6 +112,7 @@ pub async fn run(cmd: AuthCommand, global: &GlobalOptions) -> Result<()> { client_id, port, no_browser, + device, token, skip_project_select, no_org, @@ -87,38 +122,64 @@ pub async fn run(cmd: AuthCommand, global: &GlobalOptions) -> Result<()> { if let Some(url) = global.base_url.as_deref() { store_profile_base_url(&profile_name, url)?; } - if let Some(jwt) = token { - return run_login_token( - &profile_name, - jwt, - skip_project_select, - global.base_url.as_deref(), - ) - .await; + let method = select_auth_login_method(token.is_some(), device, no_browser); + match method { + AuthLoginMethod::Token => { + let jwt = token.expect("token method requires --token"); + run_login_token( + &profile_name, + jwt, + skip_project_select, + global.base_url.as_deref(), + ) + .await + } + AuthLoginMethod::Device => { + let resolved = resolve_profile( + Some(&profile_name), + global.base_url.as_deref(), + global.environment, + )?; + run_device_login( + DeviceLoginOptions { + profile: profile_name, + base_url: resolved.base_url, + client_id, + issuer, + quiet: global.quiet, + verbose: global.verbose > 0, + skip_project_select, + }, + None, + None, + ) + .await + } + AuthLoginMethod::Browser => { + let resolved = resolve_profile( + Some(&profile_name), + global.base_url.as_deref(), + global.environment, + )?; + run_login( + LoginOptions { + profile: profile_name, + port, + issuer, + client_id, + skip_project_select, + base_url: Some(resolved.base_url), + org_scope: !no_org, + fresh_login: fresh, + verbose: global.verbose > 0, + quiet: global.quiet, + }, + None, + None, + ) + .await + } } - let resolved = resolve_profile( - Some(&profile_name), - global.base_url.as_deref(), - global.environment, - )?; - run_login( - LoginOptions { - profile: profile_name, - port, - no_browser, - issuer, - client_id, - skip_project_select, - base_url: Some(resolved.base_url), - org_scope: !no_org, - fresh_login: fresh, - verbose: global.verbose > 0, - quiet: global.quiet, - }, - None, - None, - ) - .await } AuthCommand::Doctor { base_url, @@ -168,3 +229,175 @@ pub async fn run(cmd: AuthCommand, global: &GlobalOptions) -> Result<()> { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::Cli; + use crate::cli::Command; + use clap::Parser; + + #[test] + fn select_method_prefers_token_then_device_then_browser() { + assert_eq!( + select_auth_login_method(true, false, false), + AuthLoginMethod::Token + ); + assert_eq!( + select_auth_login_method(false, true, false), + AuthLoginMethod::Device + ); + assert_eq!( + select_auth_login_method(false, false, true), + AuthLoginMethod::Device + ); + assert_eq!( + select_auth_login_method(false, true, true), + AuthLoginMethod::Device + ); + assert_eq!( + select_auth_login_method(false, false, false), + AuthLoginMethod::Browser + ); + } + + #[test] + fn auth_login_device_flag_parses() { + let cli = Cli::try_parse_from(["am", "auth", "login", "--device"]).unwrap(); + match cli.command { + Command::Auth(AuthCommand::Login { + device, + no_browser, + token, + .. + }) => { + assert!(device); + assert!(!no_browser); + assert!(token.is_none()); + } + other => panic!("expected auth login --device, got {other:?}"), + } + } + + #[test] + fn auth_login_no_browser_routes_like_device() { + let cli = Cli::try_parse_from(["am", "auth", "login", "--no-browser"]).unwrap(); + match cli.command { + Command::Auth(AuthCommand::Login { + device, + no_browser, + token, + .. + }) => { + assert!(!device); + assert!(no_browser); + assert!(token.is_none()); + assert_eq!( + select_auth_login_method(token.is_some(), device, no_browser), + AuthLoginMethod::Device + ); + } + other => panic!("expected auth login --no-browser, got {other:?}"), + } + } + + #[test] + fn auth_login_token_parses_as_token_method() { + let cli = Cli::try_parse_from(["am", "auth", "login", "--token", "eyJ.test"]).unwrap(); + match cli.command { + Command::Auth(AuthCommand::Login { + device, + no_browser, + token, + .. + }) => { + assert!(!device); + assert!(!no_browser); + assert_eq!(token.as_deref(), Some("eyJ.test")); + assert_eq!( + select_auth_login_method(token.is_some(), device, no_browser), + AuthLoginMethod::Token + ); + } + other => panic!("expected auth login --token, got {other:?}"), + } + } + + #[test] + fn auth_login_default_is_browser() { + let cli = Cli::try_parse_from(["am", "auth", "login"]).unwrap(); + match cli.command { + Command::Auth(AuthCommand::Login { + device, + no_browser, + token, + .. + }) => { + assert_eq!( + select_auth_login_method(token.is_some(), device, no_browser), + AuthLoginMethod::Browser + ); + } + other => panic!("expected auth login, got {other:?}"), + } + } + + #[test] + fn no_browser_rejects_the_same_flags_as_device() { + // Both spellings select the device flow, which cannot honor browser-only + // options, so both must refuse them rather than silently ignore them. + for spelling in ["--device", "--no-browser"] { + for extra in [ + &["--fresh"][..], + &["--no-org"][..], + &["--port", "9999"][..], + &["--token", "x"][..], + ] { + let mut args = vec!["am", "auth", "login", spelling]; + args.extend_from_slice(extra); + assert!( + Cli::try_parse_from(&args).is_err(), + "{args:?} must be rejected, not silently ignored" + ); + } + } + } + + #[test] + fn device_flow_accepts_oauth_pair_overrides() { + for spelling in ["--device", "--no-browser"] { + Cli::try_parse_from([ + "am", + "auth", + "login", + spelling, + "--issuer", + "https://clerk.custom.example", + "--client-id", + "custom-client", + ]) + .unwrap_or_else(|err| panic!("{spelling} with overrides must parse: {err}")); + } + } + + #[test] + fn auth_login_rejects_device_with_token() { + let err = Cli::try_parse_from(["am", "auth", "login", "--device", "--token", "x"]) + .expect_err("device conflicts with token"); + let msg = err.to_string(); + assert!( + msg.contains("cannot be used with") || msg.contains("conflict"), + "unexpected clap error: {msg}" + ); + } + + #[test] + fn headless_hint_points_at_device_flow() { + use crate::auth::login::headless_login_next_steps; + let msg = headless_login_next_steps(Some("No such file or directory")); + assert!(msg.contains("Could not open a browser")); + assert!(msg.contains("am auth login --device")); + assert!(msg.contains("am auth login --token")); + assert!(!msg.contains("cancel and run")); + } +} diff --git a/crates/cli/src/commands/connect_project.rs b/crates/cli/src/commands/connect_project.rs index d599996..8aff0bb 100644 --- a/crates/cli/src/commands/connect_project.rs +++ b/crates/cli/src/commands/connect_project.rs @@ -922,9 +922,10 @@ async fn ensure_authenticated( io::stdin().is_terminal(), ) { bail!( - "sign-in required — run `am auth login --token ` first, \ - or `am init --device` to sign in with a device code. Browser sign-in \ - needs an interactive terminal, and --yes never opens one." + "sign-in required — run `am auth login --device` (preferred on remote/VPS), \ + `am auth login --token ` for a short paste session, \ + or `am init --device`. Browser sign-in needs an interactive terminal, \ + and --yes never opens one." ); } @@ -935,8 +936,10 @@ async fn ensure_authenticated( profile: cloud_profile.to_string(), base_url: cloud_api_url.to_string(), client_id: None, + issuer: None, quiet: global.quiet, verbose: global.verbose > 0, + skip_project_select: true, }, Some(progress), Some("identity"), @@ -948,7 +951,6 @@ async fn ensure_authenticated( LoginOptions { profile: cloud_profile.to_string(), port: None, - no_browser: false, issuer: None, client_id: None, skip_project_select: true, diff --git a/crates/cli/src/commands/init.rs b/crates/cli/src/commands/init.rs index 367d248..3b954d3 100644 --- a/crates/cli/src/commands/init.rs +++ b/crates/cli/src/commands/init.rs @@ -529,9 +529,10 @@ async fn ensure_init_authenticated(input: InitAuthInput<'_>) -> Result<()> { if !may_run_init_login(allow_prompts, use_device, io::stdin().is_terminal()) { bail!( - "sign-in required — run `am auth login --token ` first, \ - or `am init --device` to sign in with a device code. Browser sign-in \ - needs an interactive terminal, and --yes never opens one." + "sign-in required — run `am auth login --device` (preferred on remote/VPS), \ + `am auth login --token ` for a short paste session, \ + or `am init --device`. Browser sign-in needs an interactive terminal, \ + and --yes never opens one." ); } @@ -542,8 +543,10 @@ async fn ensure_init_authenticated(input: InitAuthInput<'_>) -> Result<()> { profile: cloud_profile.to_string(), base_url: cloud_api_url.to_string(), client_id: None, + issuer: None, quiet: global.quiet, verbose: global.verbose > 0, + skip_project_select: true, }, Some(progress), Some("identity"), @@ -562,7 +565,6 @@ async fn ensure_init_authenticated(input: InitAuthInput<'_>) -> Result<()> { LoginOptions { profile: cloud_profile.to_string(), port: None, - no_browser: false, issuer: None, client_id: None, skip_project_select: true, diff --git a/crates/cli/src/environment.rs b/crates/cli/src/environment.rs index 4e0e558..1c9c9ce 100644 --- a/crates/cli/src/environment.rs +++ b/crates/cli/src/environment.rs @@ -11,10 +11,27 @@ pub const ENV_CORE_IMAGE: &str = "ATOMICMEMORY_CORE_IMAGE"; /// Hostnames treated as production Cloud API endpoints (exact match, lowercase). pub const PROD_API_HOSTS: [&str; 1] = ["api.atomicstrata.ai"]; +/// Hostnames treated as first-party Atomic Strata Cloud API endpoints. +/// +/// These share the shipped Clerk OAuth pair (`PROD_OAUTH_*`). Dev/staging are +/// not production for health/tier labels, but they are first-party for OAuth +/// baking so `am auth whoami` / `doctor` work against `--base-url +/// https://api.dev…` / `https://api.staging…` without requiring a +/// non-production issuer in config.toml (ATO-2321). +pub const FIRST_PARTY_API_HOSTS: [&str; 3] = [ + "api.atomicstrata.ai", + "api.dev.atomicstrata.ai", + "api.staging.atomicstrata.ai", +]; + /// Sanctioned API hostname → memory web hostname for automatic browser open. -const SANCTIONED_MEMORY_WEB_HOSTS: [(&str, &str); 2] = [ +const SANCTIONED_MEMORY_WEB_HOSTS: [(&str, &str); 3] = [ ("api.atomicstrata.ai", "memory.atomicstrata.ai"), ("api.dev.atomicstrata.ai", "memory.dev.atomicstrata.ai"), + ( + "api.staging.atomicstrata.ai", + "memory.staging.atomicstrata.ai", + ), ]; /// Named Cloud tier — production preset only in the public CLI. @@ -74,14 +91,8 @@ pub fn parse_api_base_url(raw: &str) -> Result { Ok(url) } -/// True when the URL is the canonical production Cloud API origin. -/// -/// This gates whether the shipped production OAuth identity is used and, -/// downstream, whether a bearer token is attached to the request. Matching on -/// hostname alone would treat `http://api.atomicstrata.ai` (cleartext, so the -/// token is exposed to anyone on path) and non-default ports as production, so -/// the scheme and port must be canonical too. -pub fn is_production_api_url(raw: &str) -> bool { +/// True when `url` is HTTPS on the default port with a host in `hosts`. +fn is_canonical_https_api_host(raw: &str, hosts: &[&str]) -> bool { let Ok(url) = parse_api_base_url(raw) else { return false; }; @@ -94,9 +105,24 @@ pub fn is_production_api_url(raw: &str) -> bool { } url.host_str() .map(str::to_ascii_lowercase) - .is_some_and(|host| PROD_API_HOSTS.contains(&host.as_str())) + .is_some_and(|host| hosts.contains(&host.as_str())) +} + +/// True when the URL is the canonical production Cloud API origin. +/// +/// Matching on hostname alone would treat `http://api.atomicstrata.ai` +/// (cleartext, so the token is exposed to anyone on path) and non-default +/// ports as production, so the scheme and port must be canonical too. +pub fn is_production_api_url(raw: &str) -> bool { + is_canonical_https_api_host(raw, &PROD_API_HOSTS) } +/// True when the URL is a first-party Atomic Strata Cloud API origin (prod, +/// Dev, or staging). Gates the shipped Clerk OAuth pair — not production-only +/// policy. +pub fn is_first_party_cloud_api_url(raw: &str) -> bool { + is_canonical_https_api_host(raw, &FIRST_PARTY_API_HOSTS) +} /// True when the URL targets a remote Cloud API (HTTPS, not loopback). /// /// Used to honor dashboard shell exports (`ATOMICMEMORY_API_URL` + `amc_` key) @@ -319,6 +345,28 @@ mod tests { fn is_production_api_url_rejects_custom_hosts() { assert!(!is_production_api_url("https://api.staging.example.com")); assert!(!is_production_api_url("http://127.0.0.1:8080")); + // Dev/staging are first-party for OAuth, but not the production origin. + assert!(!is_production_api_url("https://api.dev.atomicstrata.ai")); + assert!(!is_production_api_url( + "https://api.staging.atomicstrata.ai" + )); + } + + #[test] + fn is_first_party_cloud_api_url_accepts_prod_dev_and_staging() { + assert!(is_first_party_cloud_api_url("https://api.atomicstrata.ai")); + assert!(is_first_party_cloud_api_url( + "https://api.dev.atomicstrata.ai/" + )); + assert!(is_first_party_cloud_api_url( + "https://api.staging.atomicstrata.ai" + )); + assert!(!is_first_party_cloud_api_url( + "https://api.staging.example.com" + )); + assert!(!is_first_party_cloud_api_url( + "http://api.dev.atomicstrata.ai" + )); } #[test] @@ -379,6 +427,9 @@ mod tests { let dev = memory_web_origin("https://api.dev.atomicstrata.ai").unwrap(); assert_eq!(dev, "https://memory.dev.atomicstrata.ai/"); + + let staging = memory_web_origin("https://api.staging.atomicstrata.ai").unwrap(); + assert_eq!(staging, "https://memory.staging.atomicstrata.ai/"); } #[test] diff --git a/crates/cli/tests/device_login_oauth.rs b/crates/cli/tests/device_login_oauth.rs new file mode 100644 index 0000000..73ddd47 --- /dev/null +++ b/crates/cli/tests/device_login_oauth.rs @@ -0,0 +1,256 @@ +//! Device login must leave a session that later commands can authorize. + +#![cfg(unix)] + +#[path = "support/local_token.rs"] +mod support; + +use std::path::Path; +use support::{Fixture, MEMBER}; + +fn binary() -> &'static Path { + Path::new(env!("CARGO_BIN_EXE_am")) +} + +fn stderr(output: &std::process::Output) -> String { + String::from_utf8_lossy(&output.stderr).into_owned() +} + +/// A custom Cloud origin with no OAuth pair in configuration. +async fn custom_origin_without_oauth_pair() -> Fixture { + let fixture = Fixture::new().await; + fixture.edit("config.toml", |value| { + value.as_table_mut().unwrap().remove("oauth"); + }); + fixture.edit("credentials.toml", |value| { + value.as_table_mut().unwrap().remove("oauth"); + }); + fixture +} + +fn device_requests(fixture: &Fixture) -> usize { + fixture + .requests() + .iter() + .filter(|r| r.path.starts_with("/api/oauth/device/")) + .count() +} + +fn stored_session(fixture: &Fixture, profile: &str) -> Option { + let text = std::fs::read_to_string(fixture.config.join("credentials.toml")).unwrap(); + let value: toml::Value = text.parse().unwrap(); + value.get("oauth")?.get(profile).cloned() +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn blank_oauth_overrides_are_rejected_without_changing_credentials() { + for route in ["--device", "--no-browser"] { + for flag in ["--issuer", "--client-id"] { + for value in ["", " "] { + let fixture = Fixture::new().await; + let config_before = std::fs::read(fixture.config.join("config.toml")).unwrap(); + let credentials_before = + std::fs::read(fixture.config.join("credentials.toml")).unwrap(); + let login = fixture.run( + binary(), + &["auth", "login", route, flag, value, "--skip-project-select"], + None, + ); + assert!( + !login.status.success(), + "{route} {flag} {value:?} must fail" + ); + assert!( + stderr(&login).contains(&format!("{flag} must not be blank")), + "{}", + stderr(&login) + ); + assert_eq!(device_requests(&fixture), 0); + assert_eq!( + std::fs::read(fixture.config.join("config.toml")).unwrap(), + config_before + ); + assert_eq!( + std::fs::read(fixture.config.join("credentials.toml")).unwrap(), + credentials_before + ); + let whoami = fixture.run(binary(), &["auth", "whoami"], None); + assert!(whoami.status.success(), "{}", stderr(&whoami)); + } + } + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn device_login_overrides_keep_the_session_usable_afterwards() { + let fixture = custom_origin_without_oauth_pair().await; + let base = fixture.base.clone(); + let login = fixture.run( + binary(), + &[ + "auth", + "login", + "--device", + "--issuer", + &base, + "--client-id", + "fixture_client", + "--skip-project-select", + ], + None, + ); + assert!(login.status.success(), "{}", stderr(&login)); + + let config: toml::Value = std::fs::read_to_string(fixture.config.join("config.toml")) + .unwrap() + .parse() + .unwrap(); + assert_eq!(config["oauth"]["issuer"].as_str(), Some(base.as_str())); + assert_eq!( + config["oauth"]["client_id"].as_str(), + Some("fixture_client") + ); + + // A later command resolves the OAuth pair from configuration, not the + // login command line. Before the fix this failed on a custom origin. + let whoami = fixture.run(binary(), &["auth", "whoami"], None); + assert!(whoami.status.success(), "{}", stderr(&whoami)); + assert!( + String::from_utf8_lossy(&whoami.stdout).contains(MEMBER), + "{}", + String::from_utf8_lossy(&whoami.stdout) + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn device_login_without_a_resolvable_oauth_pair_fails_before_the_flow() { + let fixture = custom_origin_without_oauth_pair().await; + let login = fixture.run( + binary(), + &["auth", "login", "--device", "--skip-project-select"], + None, + ); + assert!(!login.status.success(), "must fail without an OAuth pair"); + assert!( + stderr(&login).contains("requires explicit OAuth configuration"), + "must fail on the missing pair, not something else: {}", + stderr(&login) + ); + assert_eq!( + device_requests(&fixture), + 0, + "no device flow may start: {:?}", + fixture.requests() + ); + assert!(stored_session(&fixture, "local").is_none()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn device_login_refuses_a_session_from_another_issuer() { + let fixture = custom_origin_without_oauth_pair().await; + fixture.api.lock().unwrap().device_iss = Some("https://other-issuer.example".into()); + let base = fixture.base.clone(); + let login = fixture.run( + binary(), + &[ + "auth", + "login", + "--device", + "--issuer", + &base, + "--client-id", + "fixture_client", + "--skip-project-select", + ], + None, + ); + assert!( + !login.status.success(), + "foreign-issuer session must be refused" + ); + assert!( + stderr(&login).contains("refusing to store"), + "{}", + stderr(&login) + ); + assert!(stored_session(&fixture, "local").is_none()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn device_login_refuses_an_issuer_override_the_environment_would_shadow() { + // ATOMICMEMORY_OAUTH_ISSUER outranks configuration, so a persisted + // --issuer would never be used; the resulting session could not be + // authorized by any later command. + let fixture = custom_origin_without_oauth_pair().await; + let base = fixture.base.clone(); + let login = fixture.run_with_envs( + binary(), + &[ + "auth", + "login", + "--device", + "--issuer", + &base, + "--client-id", + "fixture_client", + "--skip-project-select", + ], + None, + &[("ATOMICMEMORY_OAUTH_ISSUER", "https://env-issuer.example")], + ); + assert!( + !login.status.success(), + "a shadowed --issuer must be refused" + ); + assert!( + stderr(&login).contains("would not be used after login"), + "{}", + stderr(&login) + ); + assert_eq!( + device_requests(&fixture), + 0, + "must fail before the device flow: {:?}", + fixture.requests() + ); + assert!(stored_session(&fixture, "local").is_none()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn blank_oauth_overrides_are_rejected_for_browser_login_too() { + // Browser login persists overrides into the same global [oauth] table + // before opening the browser, so it must refuse blank ones first. + for flag in ["--issuer", "--client-id"] { + for value in ["", " "] { + let fixture = Fixture::new().await; + let config_before = std::fs::read(fixture.config.join("config.toml")).unwrap(); + let credentials_before = + std::fs::read(fixture.config.join("credentials.toml")).unwrap(); + let login = fixture.run( + binary(), + &["auth", "login", flag, value, "--skip-project-select"], + None, + ); + assert!( + !login.status.success(), + "browser {flag} {value:?} must fail" + ); + assert!( + stderr(&login).contains(&format!("{flag} must not be blank")), + "{}", + stderr(&login) + ); + assert!(fixture.requests().is_empty(), "{:?}", fixture.requests()); + assert_eq!( + std::fs::read(fixture.config.join("config.toml")).unwrap(), + config_before + ); + assert_eq!( + std::fs::read(fixture.config.join("credentials.toml")).unwrap(), + credentials_before + ); + let whoami = fixture.run(binary(), &["auth", "whoami"], None); + assert!(whoami.status.success(), "{}", stderr(&whoami)); + } + } +} diff --git a/crates/cli/tests/support/local_token.rs b/crates/cli/tests/support/local_token.rs index 0fdf16c..e9039dc 100644 --- a/crates/cli/tests/support/local_token.rs +++ b/crates/cli/tests/support/local_token.rs @@ -37,6 +37,8 @@ pub struct Request { pub struct Api { pub requests: Vec, pub fail_first_discovery: bool, + /// `iss` claim on device-flow id_tokens (None omits the claim). + pub device_iss: Option, } pub struct Fixture { @@ -196,6 +198,11 @@ impl Drop for Fixture { } } +fn token_with_iss(iss: &str) -> String { + let payload = format!(r#"{{"sub":"user_member","iss":"{iss}","exp":4102444800}}"#); + format!("hdr.{}.sig", URL_SAFE_NO_PAD.encode(payload.as_bytes())) +} + fn token() -> String { let payload = URL_SAFE_NO_PAD.encode(br#"{"sub":"user_member","exp":4102444800}"#); format!("hdr.{payload}.sig") @@ -242,6 +249,18 @@ async fn handle( } json!({"authorization_endpoint":format!("{base}/authorize"), "token_endpoint":format!("{base}/oauth/token")}) } + "/api/oauth/device/authorize" => json!({ + "device_code":"fixture_device_code", + "user_code":"FIX-TURE", + "verification_uri":format!("{base}/activate"), + "verification_uri_complete":format!("{base}/activate?code=FIX-TURE"), + "expires_in":600, + "interval":1 + }), + "/api/oauth/device/token" => { + let id_token = api.device_iss.as_deref().map_or_else(token, token_with_iss); + json!({"id_token":id_token, "refresh_token":"fixture_device_refresh", "token_type":"Bearer", "expires_in":3600}) + } "/oauth/token" => { json!({"id_token":token(), "refresh_token":"fixture_refresh", "expires_in":3600}) }