From 7f06ba0ee93eebf4482a7eb5d5d25e9d8a072f9d Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 23 Mar 2026 09:33:04 -0700 Subject: chore: Refactor CLI auth flows and token storage (#3317) This PR eplaces the binary `is_hub_sync()` auth routing with an explicit `SyncAuth` enum that classifies the client's authentication state at runtime. This fixes a class of bugs where CLI session tokens were silently mis-stored or used with the wrong auth scheme during Hub migration. --- crates/atuin-client/src/auth.rs | 46 ++++++++++--- crates/atuin-client/src/settings.rs | 125 ++++++++++++++++++++++++++++-------- 2 files changed, 136 insertions(+), 35 deletions(-) (limited to 'crates/atuin-client/src') diff --git a/crates/atuin-client/src/auth.rs b/crates/atuin-client/src/auth.rs index 1e638c21..8ea4b8ab 100644 --- a/crates/atuin-client/src/auth.rs +++ b/crates/atuin-client/src/auth.rs @@ -18,7 +18,13 @@ static APP_USER_AGENT: &str = concat!("atuin/", env!("CARGO_PKG_VERSION")); /// Result of an auth operation that may require 2FA. pub enum AuthResponse { /// Operation succeeded; for login/register, contains the session token. - Success { session: String }, + /// `auth_type` indicates the kind of token: `Some("hub")` for Hub API + /// tokens (prefixed `atapi_`), `Some("cli")` for legacy CLI session + /// tokens. `None` when the server didn't include the field (old servers). + Success { + session: String, + auth_type: Option, + }, /// Two-factor authentication is required; the caller should prompt for a /// TOTP code and retry with it. TwoFactorRequired, @@ -153,6 +159,7 @@ impl AuthClient for LegacyAuthClient { Ok(AuthResponse::Success { session: resp.session, + auth_type: resp.auth.or(Some("cli".into())), }) } @@ -160,6 +167,7 @@ impl AuthClient for LegacyAuthClient { let resp = crate::api_client::register(&self.address, username, email, password).await?; Ok(AuthResponse::Success { session: resp.session, + auth_type: resp.auth.or(Some("cli".into())), }) } @@ -273,6 +281,7 @@ impl AuthClient for HubAuthClient { let login: LoginResponse = resp.json().await?; return Ok(AuthResponse::Success { session: login.session, + auth_type: login.auth, }); } @@ -316,6 +325,7 @@ impl AuthClient for HubAuthClient { let reg: RegisterResponse = resp.json().await?; return Ok(AuthResponse::Success { session: reg.session, + auth_type: reg.auth, }); } @@ -332,10 +342,19 @@ impl AuthClient for HubAuthClient { new_password: &str, totp_code: Option<&str>, ) -> Result { - let hub_token = self - .hub_token - .as_deref() - .ok_or_else(|| eyre::eyre!("Not logged in to Hub"))?; + let hub_token = self.hub_token.as_deref().ok_or_else(|| { + eyre::eyre!( + "Not logged in to Atuin Hub. \ + Please run 'atuin login' to authenticate." + ) + })?; + + if !hub_token.starts_with("atapi_") { + bail!( + "Your Hub session token is invalid. \ + Please run 'atuin login' to re-authenticate with Atuin Hub." + ); + } ensure_crypto_provider(); let url = make_url(&self.address, "/api/v0/account/password")?; @@ -385,10 +404,19 @@ impl AuthClient for HubAuthClient { password: &str, totp_code: Option<&str>, ) -> Result { - let hub_token = self - .hub_token - .as_deref() - .ok_or_else(|| eyre::eyre!("Not logged in to Hub"))?; + let hub_token = self.hub_token.as_deref().ok_or_else(|| { + eyre::eyre!( + "Not logged in to Atuin Hub. \ + Please run 'atuin login' to authenticate." + ) + })?; + + if !hub_token.starts_with("atapi_") { + bail!( + "Your Hub session token is invalid. \ + Please run 'atuin login' to re-authenticate with Atuin Hub." + ); + } ensure_crypto_provider(); let url = make_url(&self.address, "/api/v0/account")?; diff --git a/crates/atuin-client/src/settings.rs b/crates/atuin-client/src/settings.rs index becf72db..5b18d9ea 100644 --- a/crates/atuin-client/src/settings.rs +++ b/crates/atuin-client/src/settings.rs @@ -383,6 +383,45 @@ pub enum SyncProtocol { Auto, } +/// Resolved authentication state for sync operations. +/// +/// Determined at runtime by examining which tokens are available and what +/// server the client is configured to talk to. Operations use this to pick +/// the right auth header and endpoint style. +#[cfg(feature = "sync")] +#[derive(Debug, Clone)] +pub enum SyncAuth { + /// Self-hosted Rust server. Uses `Authorization: Token ` and + /// legacy endpoints. + Legacy { token: String }, + /// Hub with a valid Hub API token (`atapi_*`). Uses + /// `Authorization: Bearer ` and v0 endpoints. + Hub { token: String }, + /// Targeting Hub but only has a CLI session token. Uses + /// `Authorization: Token ` against compat/record endpoints. + /// Sync, password change, and account deletion still work, but the user + /// should be nudged to run `atuin login` for full Hub auth. + HubViaCli { token: String }, + /// Not authenticated at all. Contains an actionable user-facing message. + NotLoggedIn { reason: String }, +} + +#[cfg(feature = "sync")] +impl SyncAuth { + /// Convert into the auth token type used by the API client. + /// + /// Returns an error with an actionable message for `NotLoggedIn`. + pub fn into_auth_token(self) -> Result { + use crate::api_client::AuthToken; + match self { + SyncAuth::Legacy { token } => Ok(AuthToken::Token(token)), + SyncAuth::Hub { token } => Ok(AuthToken::Bearer(token)), + SyncAuth::HubViaCli { token } => Ok(AuthToken::Token(token)), + SyncAuth::NotLoggedIn { reason } => Err(eyre!(reason)), + } + } +} + #[derive(Clone, Debug, Deserialize, Default, Serialize)] pub struct Keys { pub scroll_exits: bool, @@ -1239,40 +1278,74 @@ impl Settings { } } - /// Returns the best available auth token for sync operations. - /// - /// Token priority when using Hub sync: - /// 1. Hub token (Bearer) - enables unified Hub auth - /// 2. CLI session token (Token) - fallback if Hub token revoked - /// - /// For legacy/self-hosted sync, only CLI session token is used. - /// - /// Hub tokens are preferred when available because they provide unified - /// authentication across CLI and Hub features, and users can manage them - /// via the Hub web interface. + /// Examines the configured sync target and available tokens to determine + /// the correct auth strategy. Also performs cleanup of mis-stored tokens + /// (e.g. a CLI token incorrectly saved in the Hub session slot). #[cfg(feature = "sync")] - pub async fn sync_auth_token(&self) -> Result { - use crate::api_client::AuthToken; + pub async fn resolve_sync_auth(&self) -> SyncAuth { + let meta = match Self::meta_store().await { + Ok(m) => m, + Err(e) => { + return SyncAuth::NotLoggedIn { + reason: format!("Failed to open meta store: {e}"), + }; + } + }; - let meta = Self::meta_store().await?; + if !self.is_hub_sync() { + // Self-hosted / legacy server + return match meta.session_token().await { + Ok(Some(token)) => SyncAuth::Legacy { token }, + _ => SyncAuth::NotLoggedIn { + reason: "Not logged in. Run 'atuin login' to authenticate \ + with your sync server." + .into(), + }, + }; + } - // Try Hub token first if we're using Hub sync - if self.is_hub_sync() - && let Some(hub_token) = meta.hub_session_token().await? - { - return Ok(AuthToken::Bearer(hub_token)); + // Targeting Hub — check for a valid Hub API token first + if let Ok(Some(hub_token)) = meta.hub_session_token().await { + if hub_token.starts_with("atapi_") { + return SyncAuth::Hub { token: hub_token }; + } + + // A non-atapi_ token in the hub_session slot is a mis-stored CLI + // token (from the migration-fallback bug). Move it to the CLI + // session slot if that slot is empty, then clear hub_session + // only if the move succeeded. + if let Ok(None) = meta.session_token().await { + if meta.save_session(&hub_token).await.is_ok() { + let _ = meta.delete_hub_session().await; + } + } else { + // CLI slot already has a token; just clear the bad hub_session + let _ = meta.delete_hub_session().await; + } + // Fall through to check CLI token below } - // Fall back to CLI session token - match meta.session_token().await? { - Some(token) => Ok(AuthToken::Token(token)), - None => Err(eyre!( - "Not logged in - no Hub session or CLI session found. \ - Run 'atuin login' or 'atuin register' to authenticate." - )), + // No valid Hub token — check for a CLI session token + match meta.session_token().await { + Ok(Some(token)) => SyncAuth::HubViaCli { token }, + _ => SyncAuth::NotLoggedIn { + reason: "Not logged in. Run 'atuin login' or 'atuin register' \ + to authenticate." + .into(), + }, } } + /// Returns the appropriate auth token for sync operations. + /// + /// Delegates to [`resolve_sync_auth`] and converts the result to an + /// `AuthToken`. Callers that need to distinguish between auth states + /// (e.g. to show different UI) should call `resolve_sync_auth` directly. + #[cfg(feature = "sync")] + pub async fn sync_auth_token(&self) -> Result { + self.resolve_sync_auth().await.into_auth_token() + } + #[cfg(feature = "check-update")] async fn needs_update_check(&self) -> Result { let last_check = Settings::last_version_check().await?; -- cgit v1.3.1