diff --git a/src/commands/dev/mod.rs b/src/commands/dev/mod.rs index f7101d1..47c1828 100644 --- a/src/commands/dev/mod.rs +++ b/src/commands/dev/mod.rs @@ -15,7 +15,7 @@ use std::io::IsTerminal; use std::path::{Path, PathBuf}; -use std::time::{Duration, SystemTime}; +use std::time::Duration; use anyhow::{Context, Result, anyhow}; use base64::Engine; @@ -27,8 +27,7 @@ use rand::rngs::OsRng; use reqwest::blocking::Client; use super::{ - DEFAULT_API_BASE, DEFAULT_DISPATCH_BASE, ENV_API_URL, ENV_DISPATCH_URL, ok_mark, resolve_base, - session_expired, warn_prefix, + DEFAULT_DISPATCH_BASE, ENV_DISPATCH_URL, ok_mark, resolve_base, session_expired, warn_prefix, }; use crate::{api, credentials, http}; @@ -149,7 +148,6 @@ fn open_tunnel( ) -> Result<(tunnel::TunnelHandle, tokio::runtime::Runtime)> { let mut creds = credentials::load_or_login_hint()?; let dispatch_base = resolve_base(ENV_DISPATCH_URL, DEFAULT_DISPATCH_BASE); - let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); let client = http::client(Duration::from_secs(15))?; let module_id = module_meta::read_module_id(module_dir)?; let local_url = local_url.unwrap_or(DEFAULT_LOCAL_URL); @@ -159,7 +157,7 @@ fn open_tunnel( ok_mark(), style(&dispatch_base).cyan().dim() ); - let token = match mint_tunnel_token(&client, &dispatch_base, &api_base, &mut creds) { + let token = match mint_tunnel_token(&client, &dispatch_base, &mut creds) { Ok(t) => t, Err(api::ApiError::Unauthenticated) => return Err(session_expired()), Err(e) => { @@ -221,38 +219,17 @@ fn open_tunnel( /// Access tokens are short (15 min per the platform's TokenConfig). /// `mirrorstack dev` is a long-running command, so we silently refresh /// the access token via the stored refresh token (30-day TTL) on a 401 -/// and retry the mint once. The refresh endpoint rotates the refresh -/// token too, so the rotated pair is persisted back to credentials. -/// If the refresh ALSO 401s (revoked / expired session), the original -/// Unauthenticated bubbles up and the caller surfaces session_expired. +/// and retry the mint once via [`credentials::with_refresh_retry`]. The +/// refresh endpoint rotates the refresh token too, so the rotated pair is +/// persisted back to credentials. If the refresh ALSO 401s (revoked / +/// expired session), Unauthenticated bubbles up and the caller surfaces +/// session_expired. fn mint_tunnel_token( client: &Client, dispatch_base: &str, - api_base: &str, creds: &mut credentials::Credentials, ) -> Result { - match api::tunnel_token(client, dispatch_base, &creds.access_token) { - Ok(t) => Ok(t), - Err(api::ApiError::Unauthenticated) => { - let refreshed = api::refresh_session(client, api_base, &creds.refresh_token)?; - creds.access_token = refreshed.access_token; - creds.refresh_token = refreshed.refresh_token; - // expires_at is informational only on the credentials struct; - // match the platform's 15-minute access TTL. - creds.expires_at = SystemTime::now() + Duration::from_secs(15 * 60); - if let Err(e) = credentials::save(creds) { - // Save failure isn't fatal for the in-process retry (we - // hold the new access token in `creds`), but log so the - // user sees the warning if the next run re-prompts login. - eprintln!( - "{} refreshed session but failed to save credentials: {e:#}", - warn_prefix() - ); - } - api::tunnel_token(client, dispatch_base, &creds.access_token) - } - Err(e) => Err(e), - } + credentials::with_refresh_retry(creds, |tok| api::tunnel_token(client, dispatch_base, tok)) } /// Build a user-facing error for the `module_dev_mode_off` rpc.err. The diff --git a/src/commands/module/mod.rs b/src/commands/module/mod.rs index fe4b7ce..90c33c5 100644 --- a/src/commands/module/mod.rs +++ b/src/commands/module/mod.rs @@ -73,7 +73,7 @@ pub fn run(args: ModuleArgs) -> Result<()> { fn init(args: InitArgs) -> Result<()> { let theme = ColorfulTheme::default(); - let creds = credentials::load_or_login_hint()?; + let mut creds = credentials::load_or_login_hint()?; let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); let apps_base = resolve_base(ENV_APPS_API_URL, DEFAULT_APPS_API_BASE); let web_base = resolve_base(ENV_WEB_URL, DEFAULT_WEB_BASE); @@ -82,11 +82,12 @@ fn init(args: InitArgs) -> Result<()> { // The platform stores ownership by user id, but the CLI surfaces the // full namespaced `@/` — so we refuse to POST until the // caller has claimed a username, and point them at the web flow. - let identity = match api::me(&client, &api_base, &creds.access_token) { - Ok(id) => id, - Err(ApiError::Unauthenticated) => return Err(session_expired()), - Err(e) => return Err(e.into()), - }; + let identity = + match credentials::with_refresh_retry(&mut creds, |tok| api::me(&client, &api_base, tok)) { + Ok(id) => id, + Err(ApiError::Unauthenticated) => return Err(session_expired()), + Err(e) => return Err(e.into()), + }; let Some(username) = identity.slug.as_deref().filter(|s| !s.is_empty()) else { return Err(anyhow!( "no username set on this account. Visit {web_base}/me to claim one before creating modules." @@ -116,7 +117,9 @@ fn init(args: InitArgs) -> Result<()> { // the common "I forgot I made this last week" case before we POST. // Reserved/invalid still surface server-side from the POST below. let pre_check = with_spinner("Checking availability…", || { - api::get_module(&client, &apps_base, &creds.access_token, &slug) + credentials::with_refresh_retry(&mut creds, |tok| { + api::get_module(&client, &apps_base, tok, &slug) + }) }); // Capture the platform-assigned module ID so scaffolding can substitute // it into Config.ID and the table prefix. Sourced from whichever branch @@ -151,15 +154,17 @@ fn init(args: InitArgs) -> Result<()> { } let create_result = with_spinner("Creating module…", || { - api::create_module( - &client, - &apps_base, - &creds.access_token, - &CreateModuleInput { - name: &name, - slug: &slug, - }, - ) + credentials::with_refresh_retry(&mut creds, |tok| { + api::create_module( + &client, + &apps_base, + tok, + &CreateModuleInput { + name: &name, + slug: &slug, + }, + ) + }) }); match create_result { @@ -172,7 +177,7 @@ fn init(args: InitArgs) -> Result<()> { // time we POST'd. Re-fetch so scaffold still has a real // module ID to substitute. print_already_exists(username, &slug, None); - refetch_module_id(&client, &apps_base, &creds.access_token, username, &slug)? + refetch_module_id(&client, &apps_base, &mut creds, username, &slug)? } Err(ApiError::Server { code, message, .. }) => { return Err(anyhow!( @@ -201,12 +206,12 @@ fn init(args: InitArgs) -> Result<()> { fn refetch_module_id( client: &reqwest::blocking::Client, apps_base: &str, - access_token: &str, + creds: &mut credentials::Credentials, username: &str, slug: &str, ) -> Result { let refetch = with_spinner("Resolving existing module…", || { - api::get_module(client, apps_base, access_token, slug) + credentials::with_refresh_retry(creds, |tok| api::get_module(client, apps_base, tok, slug)) }); match refetch { Ok(Some(m)) => Ok(m.id), diff --git a/src/commands/whoami.rs b/src/commands/whoami.rs index 5b87f25..224a0e8 100644 --- a/src/commands/whoami.rs +++ b/src/commands/whoami.rs @@ -1,9 +1,12 @@ //! `mirrorstack whoami` — print the authenticated user. //! //! Reads the access token from the credentials file and calls -//! GET /v1/auth/me. If the file is missing or the server returns 401, -//! tells the user to run `mirrorstack login`. Auto-refresh on token -//! expiry is a follow-up — for v1 the user re-runs login. +//! GET /v1/auth/me. `load_or_login_hint` auto-refreshes a known-expired +//! access token from the stored refresh token before the call, and the +//! call itself is wrapped in `with_refresh_retry` so a server-side 401 +//! (clock skew, server-side revocation) triggers one refresh + retry. The +//! `session expired` message is only shown when that refresh also fails — +//! i.e. the refresh token is gone — at which point the user re-runs login. use std::time::Duration; @@ -20,11 +23,11 @@ use super::{DEFAULT_API_BASE, ENV_API_URL, resolve_base}; pub struct WhoamiArgs {} pub fn run(_args: WhoamiArgs) -> Result<()> { - let creds = credentials::load_or_login_hint()?; + let mut creds = credentials::load_or_login_hint()?; let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); let client = http::client(Duration::from_secs(15))?; - match api::me(&client, &api_base, &creds.access_token) { + match credentials::with_refresh_retry(&mut creds, |tok| api::me(&client, &api_base, tok)) { Ok(id) => { println!("{}", id.email); if !id.name.is_empty() { diff --git a/src/credentials/mod.rs b/src/credentials/mod.rs index fc422bc..8c9c7c5 100644 --- a/src/credentials/mod.rs +++ b/src/credentials/mod.rs @@ -7,13 +7,24 @@ use std::fs; use std::io::{self, Write}; use std::path::PathBuf; -use std::time::SystemTime; +use std::time::{Duration, SystemTime}; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use tempfile::NamedTempFile; use thiserror::Error; +use crate::api::{self, ApiError}; +use crate::commands::{DEFAULT_API_BASE, ENV_API_URL, resolve_base, warn_prefix}; +use crate::http; + +/// Local access-token lifetime re-derived on every refresh. The platform's +/// `TokenConfig` mints 15-minute access tokens; the refresh response's +/// `expires_at` is informational only, so we recompute the TTL locally to +/// keep `expires_at` honest on disk. Mirrors `login`'s use of the +/// exchange's `expires_in`. +const ACCESS_TTL: Duration = Duration::from_secs(15 * 60); + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct Credentials { pub access_token: String, @@ -76,12 +87,89 @@ pub fn load() -> Result { } } +/// Exchange the stored refresh token for a fresh access + refresh pair, +/// persist the rotated pair, and return the new credentials. +/// +/// `expires_at` is re-derived locally from [`ACCESS_TTL`] (the refresh +/// response's `expires_at` is informational only). The platform rotates +/// the refresh token on every refresh, so the rotated value MUST be saved +/// back — callers that drop the return value still get the new tokens on +/// disk. +/// +/// Saving is best-effort: a save failure is surfaced as a non-fatal +/// warning (the in-memory credentials are still valid for the rest of the +/// command) rather than failing the whole operation. A refresh that 401s +/// returns `ApiError::Unauthenticated` via the `?` — the refresh token +/// itself is gone, so the caller should fall back to the login hint. +pub fn refresh_and_save(refresh_token: &str) -> Result { + let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); + let client = http::client(std::time::Duration::from_secs(15)) + .context("credentials: build HTTP client for refresh")?; + let refreshed = api::refresh_session(&client, &api_base, refresh_token)?; + let creds = Credentials { + access_token: refreshed.access_token, + refresh_token: refreshed.refresh_token, + expires_at: SystemTime::now() + ACCESS_TTL, + }; + if let Err(e) = save(&creds) { + eprintln!( + "{} refreshed session but failed to save credentials: {e:#}", + warn_prefix() + ); + } + Ok(creds) +} + +/// Run an authenticated call, transparently refreshing the access token +/// and retrying ONCE on a 401. +/// +/// `call` is handed the current access token and runs against it. If it +/// returns `Ok` or any error other than [`ApiError::Unauthenticated`], +/// that result is returned unchanged. On `Unauthenticated`, this attempts +/// [`refresh_and_save`]: on success the rotated credentials are written +/// back through `creds` and `call` runs exactly one more time (its result +/// — even a second `Unauthenticated` — is returned as-is). If the refresh +/// itself fails (revoked / expired session), the original `Unauthenticated` +/// is returned so the caller's terminal arm surfaces the login hint. +/// +/// At most one refresh + one retry — never a loop. +pub fn with_refresh_retry( + creds: &mut Credentials, + mut call: impl FnMut(&str) -> Result, +) -> Result { + match call(&creds.access_token) { + Err(ApiError::Unauthenticated) => {} + other => return other, + } + match refresh_and_save(&creds.refresh_token) { + Ok(new) => { + *creds = new; + call(&creds.access_token) + } + Err(_) => Err(ApiError::Unauthenticated), + } +} + /// Wrap [`load`] with the auth-required command idiom: a missing file /// becomes a "run `mirrorstack login`" hint instead of the raw error. /// Used by every command that needs an authenticated session. +/// +/// When the stored access token is already past its `expires_at`, this +/// proactively refreshes from the stored refresh token before returning, +/// so callers start with a live token. A refresh failure here is +/// swallowed: the (stale) credentials are returned and the command's own +/// on-401 retry (see [`with_refresh_retry`]) gets the next attempt at +/// recovery before surfacing the login hint. pub fn load_or_login_hint() -> Result { match load() { - Ok(c) => Ok(c), + Ok(c) => { + if c.expires_at <= SystemTime::now() { + if let Ok(refreshed) = refresh_and_save(&c.refresh_token) { + return Ok(refreshed); + } + } + Ok(c) + } Err(LoadError::NotFound) => Err(anyhow::anyhow!( "not signed in. Run `mirrorstack login` to sign in." )), diff --git a/src/credentials/tests.rs b/src/credentials/tests.rs index 67f238f..feb0098 100644 --- a/src/credentials/tests.rs +++ b/src/credentials/tests.rs @@ -76,4 +76,116 @@ fn credentials_lifecycle() { // delete() is idempotent — no error when called against missing file. delete().expect("delete idempotent"); + + // --- with_refresh_retry: refresh-on-401 paths --- + // These exercise the network refresh, so they live inside this single + // serial test (it already owns the per-platform config-dir env var that + // `refresh_and_save`'s `save()` writes through). The api-base env var is + // also set here, where no other test races on it. + + use crate::api::ApiError; + use mockito::Server; + + // Refresh-then-retry succeeds: first call 401s, refresh mints a new + // pair, the retried call sees the rotated token and succeeds. The + // rotated pair is written back through `creds`. + { + let mut server = Server::new(); + let _refresh = server + .mock("POST", "/v1/auth/sessions/refresh") + .with_status(200) + .with_body( + r#"{"access_token":"AT2","refresh_token":"RT2","expires_at":"2026-01-01T00:00:00Z"}"#, + ) + .create(); + unsafe { std::env::set_var("MIRRORSTACK_API_URL", server.url()) }; + + let mut creds = Credentials { + access_token: "AT1".into(), + refresh_token: "RT1".into(), + expires_at: SystemTime::now() + Duration::from_secs(900), + }; + let mut attempts = 0; + let out = with_refresh_retry(&mut creds, |tok| { + attempts += 1; + if tok == "AT1" { + Err(ApiError::Unauthenticated) + } else { + Ok(tok.to_string()) + } + }); + assert_eq!(out.unwrap(), "AT2"); + assert_eq!(attempts, 2, "exactly one retry"); + assert_eq!(creds.access_token, "AT2"); + assert_eq!(creds.refresh_token, "RT2", "rotated refresh persisted"); + } + + // Refresh itself 401s (revoked/expired session): the original + // Unauthenticated is returned and the call is NOT retried. + { + let mut server = Server::new(); + let _refresh = server + .mock("POST", "/v1/auth/sessions/refresh") + .with_status(401) + .create(); + unsafe { std::env::set_var("MIRRORSTACK_API_URL", server.url()) }; + + let mut creds = Credentials { + access_token: "AT1".into(), + refresh_token: "RT-gone".into(), + expires_at: SystemTime::now() + Duration::from_secs(900), + }; + let mut attempts = 0; + let out = with_refresh_retry(&mut creds, |_tok| { + attempts += 1; + Err::<(), _>(ApiError::Unauthenticated) + }); + assert!(matches!(out, Err(ApiError::Unauthenticated))); + assert_eq!(attempts, 1, "no retry when refresh fails"); + } + + unsafe { std::env::remove_var("MIRRORSTACK_API_URL") }; +} + +/// Happy path needs no network: the call succeeds on the first token, so +/// the refresh branch is never entered. +#[test] +fn with_refresh_retry_passes_through_success() { + let mut creds = Credentials { + access_token: "AT".into(), + refresh_token: "RT".into(), + expires_at: SystemTime::now() + Duration::from_secs(900), + }; + let mut attempts = 0; + let out = with_refresh_retry(&mut creds, |tok| { + attempts += 1; + Ok::<_, crate::api::ApiError>(tok.to_string()) + }); + assert_eq!(out.unwrap(), "AT"); + assert_eq!(attempts, 1); + assert_eq!(creds.access_token, "AT", "unchanged on success"); +} + +/// A non-401 error short-circuits without touching the refresh path, so +/// no network is needed. +#[test] +fn with_refresh_retry_propagates_non_401_without_refresh() { + let mut creds = Credentials { + access_token: "AT".into(), + refresh_token: "RT".into(), + expires_at: SystemTime::now() + Duration::from_secs(900), + }; + let mut attempts = 0; + let out: Result<(), _> = with_refresh_retry(&mut creds, |_tok| { + attempts += 1; + Err(crate::api::ApiError::Unexpected { + status: 503, + body: "down".into(), + }) + }); + assert!(matches!( + out, + Err(crate::api::ApiError::Unexpected { status: 503, .. }) + )); + assert_eq!(attempts, 1, "no refresh, no retry on non-401"); }