From 9fcde408fc41416865c83ee7f63a3820b0e34327 Mon Sep 17 00:00:00 2001 From: Sagnik Ghosh Date: Sun, 18 Oct 2026 17:53:24 +0530 Subject: [PATCH] Stop refreshing a dead login token (CE-20) A daemon holding a refresh token the server refused kept refreshing it on every telemetry flush, so those installs stayed logged out and their authorship data never arrived. Make the refusal terminal on the client: - Persist a refused device refresh token (invalid_grant / login_required) with the credentials and stop refreshing until `autter login` writes new ones; remember it in-process if persisting fails. - Tell the user once: the next interactive command shows the server's error_description, whoami/doctor/debug show LoginRefused, and the daemon logs the refusal once instead of per queue per flush. - Write refreshed credentials compare-and-set on the refresh token that was sent, and wait 10 s (was 2 s) for credentials.lock. - Return the parsed OAuthError (error, error_code, login_required) from exchange_token instead of a string. Transient failures (network, 5xx, temporarily_unavailable, access_denied) are still retried. Co-Authored-By: Claude Opus 5.5 --- src/api/client.rs | 507 ++++++++++++++++++++++++++++++--- src/auth/client.rs | 92 ++++-- src/auth/credentials.rs | 160 ++++++++++- src/auth/notice.rs | 112 +++++++- src/auth/state.rs | 6 +- src/auth/types.rs | 275 +++++++++++++++++- src/commands/blame.rs | 2 +- src/commands/debug.rs | 19 +- src/commands/doctor.rs | 18 ++ src/commands/login.rs | 12 +- src/commands/whoami.rs | 7 + src/daemon/telemetry_worker.rs | 8 +- src/file_changes/mod.rs | 2 +- 13 files changed, 1136 insertions(+), 84 deletions(-) diff --git a/src/api/client.rs b/src/api/client.rs index 1b16c5d..00d7f82 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -1,3 +1,4 @@ +use crate::auth::types::{RefreshFailure, StoredCredentials, TokenRequestError}; use crate::auth::{CredentialStore, OAuthClient}; use crate::config; use crate::error::AutterError; @@ -5,6 +6,7 @@ use crate::git::repository::{current_git_committer_identity_resolution, parse_gi use crate::http; use once_cell::sync::Lazy; use std::sync::Mutex; +use std::sync::atomic::{AtomicI64, Ordering}; use url::Url; /// Global mutex to prevent multiple threads from refreshing simultaneously. @@ -13,12 +15,33 @@ use url::Url; /// because device-flow refresh tokens rotate on every use. static REFRESH_LOCK: Lazy> = Lazy::new(|| Mutex::new(())); +/// How long to wait for a sibling process's refresh before refreshing without +/// `credentials.lock`. A refresh round trip is normally well under 3 s. +const REFRESH_FILE_LOCK_WAIT: std::time::Duration = std::time::Duration::from_secs(10); + +/// The last refresh token this process saw refused, and why. Backs up the +/// persisted refusal when writing it failed (keyring locked, disk full). +static REFUSED_REFRESH_TOKEN: Lazy>> = + Lazy::new(|| Mutex::new(None)); + +/// `at` of the refusal this process last logged, so it is logged once. +static LOGGED_REFUSAL_AT: AtomicI64 = AtomicI64::new(0); + #[derive(Debug, Clone, PartialEq, Eq)] pub enum AuthLoadIssue { NoCredentials, CredentialStore(String), RefreshExpired, RefreshFailed(String), + /// The server refused the stored refresh token for good. Nothing retries + /// until `autter login` stores new credentials. + LoginRequired(RefreshFailure), +} + +impl AuthLoadIssue { + pub fn is_login_required(&self) -> bool { + matches!(self, Self::LoginRequired(_)) + } } impl std::fmt::Display for AuthLoadIssue { @@ -28,6 +51,9 @@ impl std::fmt::Display for AuthLoadIssue { Self::CredentialStore(error) => write!(f, "could not read credentials: {error}"), Self::RefreshExpired => write!(f, "refresh token expired"), Self::RefreshFailed(error) => write!(f, "access-token refresh failed: {error}"), + Self::LoginRequired(failure) => { + write!(f, "sign-in refused by the server: {}", failure.message()) + } } } } @@ -37,17 +63,15 @@ impl std::fmt::Display for AuthLoadIssue { /// Uses in-process Mutex for thread safety during token refresh. fn try_load_auth_token() -> Result { let store = CredentialStore::new(); - - let creds = match store.load() { - Ok(Some(c)) => c, - Ok(None) => return Err(AuthLoadIssue::NoCredentials), - Err(error) => return Err(AuthLoadIssue::CredentialStore(error)), - }; - - // If refresh token expired, can't authenticate - if creds.is_refresh_token_expired() { - return Err(AuthLoadIssue::RefreshExpired); + let result = load_auth_token(&store); + if let Err(AuthLoadIssue::LoginRequired(failure)) = &result { + report_login_required(failure); } + result +} + +fn load_auth_token(store: &CredentialStore) -> Result { + let creds = load_refreshable_credentials(store)?; // Fast path: if access token is valid (with 5 min buffer), use it directly if !creds.is_access_token_expired(300) { @@ -67,61 +91,174 @@ fn try_load_auth_token() -> Result { // we still have on disk. Wait for a sibling refresh, then reload. let _file_lock = acquire_refresh_file_lock(); - // Re-check credentials after acquiring lock - another thread/process may have refreshed + let client = OAuthClient::new(); + refresh_stored_credentials(store, |refresh_token| { + client.refresh_access_token(refresh_token) + }) + // Mutex / file-lock guards are released when dropped +} + +/// Load credentials that may be used or refreshed. A login the server already +/// refused, or one whose refresh token has expired, never reaches the network. +fn load_refreshable_credentials( + store: &CredentialStore, +) -> Result { let creds = match store.load() { Ok(Some(c)) => c, Ok(None) => return Err(AuthLoadIssue::NoCredentials), Err(error) => return Err(AuthLoadIssue::CredentialStore(error)), }; - + if let Some(failure) = creds + .refresh_failed + .clone() + .or_else(|| refused_in_process(&creds.refresh_token)) + { + return Err(AuthLoadIssue::LoginRequired(failure)); + } if creds.is_refresh_token_expired() { return Err(AuthLoadIssue::RefreshExpired); } + Ok(creds) +} + +/// The refresh itself, run under the refresh locks. `refresh` performs the +/// network call (injected so tests can count calls without a server). +fn refresh_stored_credentials( + store: &CredentialStore, + refresh: impl Fn(&str) -> Result, +) -> Result { + // Re-check credentials after acquiring lock - another thread/process may have refreshed + let mut creds = load_refreshable_credentials(store)?; // Check again if access token is now valid (another thread/process may have refreshed) if !creds.is_access_token_expired(300) { return Ok(creds.access_token); } - // Still expired - we need to refresh - let client = OAuthClient::new(); - match client.refresh_access_token(&creds.refresh_token) { - Ok(new_creds) => { - // The current process can use the fresh token even if persisting it - // fails, but surface that failure so the next process is diagnosable. - if let Err(error) = store.store(&new_creds) { - tracing::warn!(%error, "could not persist refreshed credentials"); + // Still expired - we need to refresh. At most one retry, with a token a + // sibling process rotated in while this one was refreshing. + let mut retried = false; + loop { + let error = match refresh(&creds.refresh_token) { + Ok(new_creds) => { + persist_refreshed_credentials(store, &creds.refresh_token, &new_creds); + return Ok(new_creds.access_token); + } + Err(error) => error, + }; + + // Lost the rotation race: another process already swapped the + // stored refresh token. Reload and use that session if it is live. + if let Ok(Some(reloaded)) = store.load() + && !reloaded.is_login_required() + && !reloaded.is_refresh_token_expired() + { + if !reloaded.is_access_token_expired(300) { + return Ok(reloaded.access_token); } - Ok(new_creds.access_token) + if !retried && reloaded.refresh_token != creds.refresh_token { + retried = true; + creds = reloaded; + continue; + } + } + + // A device token the server refused for good: stop here. PATs share + // the refresh grant but are not rotated device sessions, so they keep + // today's retry behavior. + if error.requires_login() + && !creds.refresh_token.starts_with(PAT_PREFIX) + && let Some(oauth_error) = error.oauth_error() + && let Some(failure) = record_refused_login( + store, + &creds.refresh_token, + RefreshFailure::from_oauth_error(oauth_error), + ) + { + return Err(AuthLoadIssue::LoginRequired(failure)); } + + // Transient (network, 5xx, temporarily_unavailable, access_denied): + // keep the credentials and retry on a later call. + tracing::warn!(%error, "could not refresh autter access token"); + return Err(AuthLoadIssue::RefreshFailed(format!( + "Token refresh failed: {error}" + ))); + } +} + +/// Write rotated credentials unless the store has moved on (compare-and-set +/// on the refresh token that was sent). The current process can use the fresh +/// token even if persisting it fails, but surface that failure so the next +/// process is diagnosable. +fn persist_refreshed_credentials( + store: &CredentialStore, + sent_refresh_token: &str, + new_creds: &StoredCredentials, +) { + match store.store_if_current(sent_refresh_token, new_creds) { + Ok(true) => {} + Ok(false) => tracing::info!( + "stored credentials changed during refresh; keeping the newer stored session" + ), + Err(error) => tracing::warn!(%error, "could not persist refreshed credentials"), + } +} + +/// Persist the refusal with the credentials so every process stops refreshing +/// them. If that write fails, this process remembers it instead, so the loop +/// cannot restart. Returns `None` when the store already holds other +/// credentials (a sibling refresh or `autter login` landed meanwhile): those +/// are not refused. +fn record_refused_login( + store: &CredentialStore, + refresh_token: &str, + failure: RefreshFailure, +) -> Option { + match store.mark_refresh_failed(refresh_token, failure.clone()) { + Ok(recorded) => recorded, Err(error) => { - // Lost the rotation race: another process already swapped the - // stored refresh token. Reload and use that session if it is live. - if let Ok(Some(reloaded)) = store.load() { - if !reloaded.is_refresh_token_expired() && !reloaded.is_access_token_expired(300) { - return Ok(reloaded.access_token); - } - if reloaded.refresh_token != creds.refresh_token - && !reloaded.is_refresh_token_expired() - && let Ok(new_creds) = client.refresh_access_token(&reloaded.refresh_token) - { - if let Err(store_error) = store.store(&new_creds) { - tracing::warn!(%store_error, "could not persist refreshed credentials"); - } - return Ok(new_creds.access_token); - } + tracing::warn!(%error, "could not persist the refused login; remembering it in this process"); + if let Ok(mut refused) = REFUSED_REFRESH_TOKEN.lock() { + *refused = Some((refresh_token.to_string(), failure.clone())); } - tracing::warn!(%error, "could not refresh autter access token"); - Err(AuthLoadIssue::RefreshFailed(error)) + Some(failure) } } - // Mutex / file-lock guards are released when dropped +} + +fn refused_in_process(refresh_token: &str) -> Option { + let refused = REFUSED_REFRESH_TOKEN.lock().ok()?; + match refused.as_ref() { + Some((token, failure)) if token == refresh_token => Some(failure.clone()), + _ => None, + } +} + +/// Log a refused login once per process (the daemon would otherwise log it on +/// every flush tick) and tell an interactive user once. +fn report_login_required(failure: &RefreshFailure) { + if first_report_in_process(failure) { + tracing::warn!( + error = %failure.error, + error_code = failure.error_code.as_deref().unwrap_or("none"), + reason = %failure.message(), + "autter login refused by the server; not refreshing again until `autter login` stores new credentials" + ); + } + crate::auth::notice::announce_login_refused(failure); +} + +/// True the first time this process reports `failure`. +fn first_report_in_process(failure: &RefreshFailure) -> bool { + LOGGED_REFUSAL_AT.swap(failure.at, Ordering::Relaxed) != failure.at } /// Cross-process lock around refresh-token rotation. /// /// Returns `None` in tests, when the lock file cannot be created, or if a -/// sibling still holds the lock after a short wait (caller then reloads). +/// sibling still holds the lock after [`REFRESH_FILE_LOCK_WAIT`]. The caller +/// then refreshes without it; the compare-and-set write keeps that safe. fn acquire_refresh_file_lock() -> Option { if cfg!(test) || std::env::var_os("AUTTER_TEST_DB_PATH").is_some() { return None; @@ -133,13 +270,17 @@ fn acquire_refresh_file_lock() -> Option { if let Some(parent) = path.parent() { let _ = std::fs::create_dir_all(parent); } - for _ in 0..10 { + let deadline = std::time::Instant::now() + REFRESH_FILE_LOCK_WAIT; + loop { if let Some(lock) = crate::utils::LockFile::try_acquire(&path) { return Some(lock); } - std::thread::sleep(std::time::Duration::from_millis(200)); + if std::time::Instant::now() >= deadline { + tracing::debug!("credentials.lock still held; refreshing without it"); + return None; + } + std::thread::sleep(std::time::Duration::from_millis(100)); } - None } /// Prefix marking a Personal Access Token (mirrors the backend). @@ -541,6 +682,12 @@ impl ApiClient { self.context.auth_issue.as_ref() } + /// The auth issue worth a log line on each flush. A refused login is left + /// out: the auth loader logs it once, and it cannot change before `autter login`. + pub fn auth_issue_to_log(&self) -> Option<&AuthLoadIssue> { + self.auth_issue().filter(|issue| !issue.is_login_required()) + } + /// Check if an API key is configured pub fn has_api_key(&self) -> bool { self.context.api_key.is_some() @@ -693,6 +840,280 @@ mod tests { assert_eq!(final_count, 5); } + // ============= Dead refresh token (CE-20) ============= + + use crate::auth::credential_backend::{CredentialBackend, MockBackend}; + use crate::auth::types::OAuthError; + use std::cell::Cell; + + fn creds(refresh_token: &str, access_token: &str, access_expires_in: i64) -> StoredCredentials { + let now = chrono::Utc::now().timestamp(); + StoredCredentials { + access_token: access_token.to_string(), + refresh_token: refresh_token.to_string(), + access_token_expires_at: now + access_expires_in, + refresh_token_expires_at: now + 86400 * 90, + refresh_failed: None, + } + } + + /// Stored credentials whose access token has expired, so the next load refreshes. + fn store_needing_refresh(refresh_token: &str) -> CredentialStore { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&creds(refresh_token, "expired-access", -60)) + .unwrap(); + store + } + + fn refusal(status: u16, json: &str) -> TokenRequestError { + TokenRequestError::Refused { + status, + error: serde_json::from_str::(json).unwrap(), + } + } + + const DEAD_TOKEN_BODY: &str = r#"{"error":"invalid_grant","error_description":"Refresh token is not valid. Run `autter login` to sign in again.","error_code":"refresh_token_invalid","login_required":true}"#; + + #[test] + fn dead_refresh_token_is_not_refreshed_again_until_login() { + let store = store_needing_refresh("rt-ce20-dead"); + let calls = Cell::new(0); + let refresh = |token: &str| { + calls.set(calls.get() + 1); + if token == "rt-ce20-fresh" { + Ok(creds("rt-ce20-fresh-2", "fresh-access", 3600)) + } else { + Err(refusal(400, DEAD_TOKEN_BODY)) + } + }; + + let Err(AuthLoadIssue::LoginRequired(failure)) = + refresh_stored_credentials(&store, refresh) + else { + panic!("a dead token must end in LoginRequired"); + }; + assert_eq!(calls.get(), 1); + assert_eq!(failure.error_code.as_deref(), Some("refresh_token_invalid")); + assert!(failure.message().contains("autter login")); + // The refusal is persisted with the credentials for every process. + assert_eq!( + store.load().unwrap().unwrap().refresh_failed, + Some(failure.clone()) + ); + + // Every later attempt (daemon ticks, other commands) stops before the network. + for _ in 0..5 { + assert_eq!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::LoginRequired(failure.clone())) + ); + assert_eq!( + load_refreshable_credentials(&store).unwrap_err(), + AuthLoadIssue::LoginRequired(failure.clone()) + ); + } + assert_eq!(calls.get(), 1); + + // `autter login` writes new credentials; refreshing resumes. + store + .store(&creds("rt-ce20-fresh", "login-access", -60)) + .unwrap(); + assert_eq!( + refresh_stored_credentials(&store, refresh), + Ok("fresh-access".to_string()) + ); + assert_eq!(calls.get(), 2); + let stored = store.load().unwrap().unwrap(); + assert_eq!(stored.refresh_token, "rt-ce20-fresh-2"); + assert!(!stored.is_login_required()); + } + + #[test] + fn refused_credentials_skip_even_a_valid_access_token() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + let mut stored = creds("rt-ce20-refused-live", "still-valid", 3600); + stored.refresh_failed = Some(RefreshFailure::from_oauth_error( + &serde_json::from_str(DEAD_TOKEN_BODY).unwrap(), + )); + store.store(&stored).unwrap(); + + assert!(matches!( + load_auth_token(&store), + Err(AuthLoadIssue::LoginRequired(_)) + )); + } + + #[test] + fn older_server_invalid_grant_is_terminal_too() { + let store = store_needing_refresh("rt-ce20-old-server"); + let calls = Cell::new(0); + let refresh = |_: &str| { + calls.set(calls.get() + 1); + Err(refusal(400, r#"{"error":"invalid_grant"}"#)) + }; + assert!(matches!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::LoginRequired(_)) + )); + assert!(matches!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::LoginRequired(_)) + )); + assert_eq!(calls.get(), 1); + } + + #[test] + fn transient_refresh_errors_are_retried() { + let transient = [ + TokenRequestError::Transport("Failed to connect to server: timed out".to_string()), + TokenRequestError::Transport("HTTP 502: Bad Gateway".to_string()), + refusal(503, r#"{"error":"temporarily_unavailable"}"#), + refusal(400, r#"{"error":"temporarily_unavailable"}"#), + refusal(400, r#"{"error":"access_denied"}"#), + refusal(500, r#"{"error":"server_error"}"#), + ]; + for (index, error) in transient.into_iter().enumerate() { + let store = store_needing_refresh(&format!("rt-ce20-transient-{index}")); + let calls = Cell::new(0); + let refresh = |_: &str| { + calls.set(calls.get() + 1); + Err(error.clone()) + }; + for _ in 0..2 { + assert!(matches!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::RefreshFailed(_)) + )); + } + assert_eq!(calls.get(), 2, "{error:?} must be retried"); + assert!(!store.load().unwrap().unwrap().is_login_required()); + } + } + + #[test] + fn refused_pat_is_not_marked_dead() { + let store = store_needing_refresh("autter_pat_ce20"); + let calls = Cell::new(0); + let refresh = |_: &str| { + calls.set(calls.get() + 1); + Err(refusal(400, DEAD_TOKEN_BODY)) + }; + for _ in 0..2 { + assert!(matches!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::RefreshFailed(_)) + )); + } + assert_eq!(calls.get(), 2); + assert!(!store.load().unwrap().unwrap().is_login_required()); + } + + #[test] + fn superseded_by_a_sibling_uses_the_sibling_session() { + // Another process rotated rt-1 -> rt-2 while this one was refreshing + // rt-1; the server calls rt-1 superseded. That is not a dead login. + let store = store_needing_refresh("rt-ce20-sibling-1"); + let refresh = |token: &str| { + assert_eq!(token, "rt-ce20-sibling-1"); + store + .store(&creds("rt-ce20-sibling-2", "sibling-access", 3600)) + .unwrap(); + Err(refusal( + 400, + r#"{"error":"invalid_grant","error_code":"refresh_token_superseded","login_required":true}"#, + )) + }; + assert_eq!( + refresh_stored_credentials(&store, refresh), + Ok("sibling-access".to_string()) + ); + let stored = store.load().unwrap().unwrap(); + assert_eq!(stored.refresh_token, "rt-ce20-sibling-2"); + assert!(!stored.is_login_required()); + } + + #[test] + fn refreshed_credentials_do_not_overwrite_a_concurrent_login() { + let store = store_needing_refresh("rt-ce20-cas-1"); + let refresh = |_: &str| { + // `autter login` lands while this refresh is in flight. + store + .store(&creds("rt-ce20-cas-login", "login-access", 3600)) + .unwrap(); + Ok(creds("rt-ce20-cas-2", "refreshed-access", 3600)) + }; + assert_eq!( + refresh_stored_credentials(&store, refresh), + Ok("refreshed-access".to_string()) + ); + assert_eq!( + store.load().unwrap().unwrap().refresh_token, + "rt-ce20-cas-login" + ); + } + + #[test] + fn refusal_is_remembered_when_it_cannot_be_persisted() { + let backend = MockBackend::new(); + backend + .store(&serde_json::to_string(&creds("rt-ce20-disk-full", "expired", -60)).unwrap()) + .unwrap(); + let store = CredentialStore::with_backend(Box::new(backend.fail_store("disk full"))); + let calls = Cell::new(0); + let refresh = |_: &str| { + calls.set(calls.get() + 1); + Err(refusal(400, DEAD_TOKEN_BODY)) + }; + for _ in 0..3 { + assert!(matches!( + refresh_stored_credentials(&store, refresh), + Err(AuthLoadIssue::LoginRequired(_)) + )); + } + assert_eq!(calls.get(), 1); + } + + #[test] + fn a_refusal_is_logged_once_per_process() { + let failure = RefreshFailure { + at: 1_760_000_123, + error: "invalid_grant".to_string(), + error_code: None, + description: None, + }; + assert!(first_report_in_process(&failure)); + assert!(!first_report_in_process(&failure)); + assert!(!first_report_in_process(&failure)); + // A new login that dies again is a new refusal. + let later = RefreshFailure { + at: failure.at + 1, + ..failure + }; + assert!(first_report_in_process(&later)); + } + + #[test] + fn login_required_is_not_logged_per_flush() { + let refused = ApiClient::new(ApiContext { + auth_issue: Some(AuthLoadIssue::LoginRequired(RefreshFailure { + at: 1, + error: "invalid_grant".to_string(), + error_code: None, + description: None, + })), + ..ApiContext::without_auth(Some("https://example.com".to_string())) + }); + assert!(refused.auth_issue().is_some()); + assert!(refused.auth_issue_to_log().is_none()); + + let transient = ApiClient::new(ApiContext { + auth_issue: Some(AuthLoadIssue::RefreshFailed("timed out".to_string())), + ..ApiContext::without_auth(Some("https://example.com".to_string())) + }); + assert!(transient.auth_issue_to_log().is_some()); + } + // ============= encode_for_header Tests ============= #[test] diff --git a/src/auth/client.rs b/src/auth/client.rs index 20f6d81..71bd527 100644 --- a/src/auth/client.rs +++ b/src/auth/client.rs @@ -1,5 +1,7 @@ use crate::api::client::ApiContext; -use crate::auth::types::{DeviceAuthResponse, OAuthError, StoredCredentials, TokenResponse}; +use crate::auth::types::{ + DeviceAuthResponse, OAuthError, StoredCredentials, TokenRequestError, TokenResponse, +}; use crate::config; use crate::http; use std::thread; @@ -55,33 +57,30 @@ impl OAuthClient { }) } - /// Common token exchange logic - POST to /worker/oauth/token with given body - fn exchange_token(&self, body: serde_json::Value) -> Result { + /// Common token exchange logic - POST to /worker/oauth/token with given body. + /// A refusal comes back parsed so callers can tell a dead grant from an outage. + fn exchange_token( + &self, + body: serde_json::Value, + ) -> Result { let url = format!("{}/worker/oauth/token", self.base_url); let (_agent, request) = ApiContext::http_post(&url, Some(30)); let request = request.set("Content-Type", "application/json"); - let response = http::send_with_body(request, &body.to_string()) - .map_err(|e| format!("Failed to connect to server: {}", e))?; + let response = http::send_with_body(request, &body.to_string()).map_err(|e| { + TokenRequestError::Transport(format!("Failed to connect to server: {}", e)) + })?; - let response_body = response - .as_str() - .map_err(|e| format!("Invalid response encoding: {}", e))?; + let response_body = response.as_str().map_err(|e| { + TokenRequestError::Transport(format!("Invalid response encoding: {}", e)) + })?; if response.status_code != 200 { - let error: OAuthError = serde_json::from_str(response_body).unwrap_or(OAuthError { - error: "unknown_error".to_string(), - error_description: None, - }); - - let msg = error - .error_description - .unwrap_or_else(|| error.error.clone()); - return Err(msg); + return Err(parse_token_error(response.status_code, response_body)); } let token_response: TokenResponse = serde_json::from_str(response_body) - .map_err(|e| format!("Invalid token response: {}", e))?; + .map_err(|e| TokenRequestError::Transport(format!("Invalid token response: {}", e)))?; let now = chrono::Utc::now().timestamp(); Ok(StoredCredentials { @@ -89,6 +88,7 @@ impl OAuthClient { refresh_token: token_response.refresh_token, access_token_expires_at: now + token_response.expires_in as i64, refresh_token_expires_at: now + token_response.refresh_expires_in as i64, + refresh_failed: None, }) } @@ -176,6 +176,7 @@ impl OAuthClient { refresh_token: token_response.refresh_token, access_token_expires_at: now + token_response.expires_in as i64, refresh_token_expires_at: now + token_response.refresh_expires_in as i64, + refresh_failed: None, }); } @@ -225,7 +226,10 @@ impl OAuthClient { } /// Refresh the access token using a refresh token - pub fn refresh_access_token(&self, refresh_token: &str) -> Result { + pub fn refresh_access_token( + &self, + refresh_token: &str, + ) -> Result { let body = serde_json::json!({ "grant_type": "refresh_token", "refresh_token": refresh_token, @@ -233,7 +237,6 @@ impl OAuthClient { }); self.exchange_token(body) - .map_err(|e| format!("Token refresh failed: {}", e)) } /// Exchange a Personal Access Token (PAT) for credentials. @@ -310,6 +313,23 @@ impl OAuthClient { } } +/// Turn a non-200 token-endpoint reply into a typed error. Anything that is +/// not an OAuth error body (a proxy page, an empty 5xx) counts as transport. +fn parse_token_error(status: u16, body: &str) -> TokenRequestError { + match serde_json::from_str::(body) { + Ok(error) => TokenRequestError::Refused { status, error }, + Err(_) => { + let detail = body.trim(); + let detail = if detail.is_empty() { + "no details".to_string() + } else { + detail.chars().take(200).collect() + }; + TokenRequestError::Transport(format!("HTTP {status}: {detail}")) + } + } +} + impl Default for OAuthClient { fn default() -> Self { Self::new() @@ -458,6 +478,37 @@ mod tests { assert!(error.error_description.is_none()); } + #[test] + fn test_parse_token_error_keeps_the_oauth_fields() { + let error = parse_token_error( + 400, + r#"{"error":"invalid_grant","error_description":"Refresh token is not valid. Run `autter login` to sign in again.","error_code":"refresh_token_invalid","login_required":true}"#, + ); + let TokenRequestError::Refused { status, error } = &error else { + panic!("expected a refusal, got {error:?}"); + }; + assert_eq!(*status, 400); + assert_eq!(error.error, "invalid_grant"); + assert_eq!(error.error_code.as_deref(), Some("refresh_token_invalid")); + assert_eq!(error.login_required, Some(true)); + } + + #[test] + fn test_parse_token_error_treats_non_oauth_pages_as_transport() { + let error = parse_token_error(502, "Bad Gateway"); + assert_eq!( + error, + TokenRequestError::Transport("HTTP 502: Bad Gateway".to_string()) + ); + assert!(!error.requires_login()); + + let error = parse_token_error(503, " "); + assert_eq!( + error, + TokenRequestError::Transport("HTTP 503: no details".to_string()) + ); + } + // ============= Device Auth Response Parsing Tests ============= #[test] @@ -512,6 +563,7 @@ mod tests { refresh_token: "test".to_string(), access_token_expires_at: now + expires_in as i64, refresh_token_expires_at: now + refresh_expires_in as i64, + refresh_failed: None, }; // Access token should expire in about 1 hour diff --git a/src/auth/credentials.rs b/src/auth/credentials.rs index 856c27e..feb80f1 100644 --- a/src/auth/credentials.rs +++ b/src/auth/credentials.rs @@ -1,7 +1,7 @@ #[cfg(all(not(test), feature = "keyring"))] use crate::auth::credential_backend::KeyringBackend; use crate::auth::credential_backend::{CredentialBackend, FileBackend}; -use crate::auth::types::StoredCredentials; +use crate::auth::types::{RefreshFailure, StoredCredentials}; #[cfg(not(test))] use crate::config::Config; use std::path::PathBuf; @@ -141,6 +141,7 @@ impl CredentialStore { refresh_token: creds.refresh_token, access_token_expires_at: 0, refresh_token_expires_at: creds.refresh_token_expires_at, + refresh_failed: creds.refresh_failed, }; self.store(&migrated)?; eprintln!("Migrated stored credentials to API-only authentication."); @@ -150,6 +151,50 @@ impl CredentialStore { Ok(Some(creds)) } + /// Store refreshed credentials only if the store still holds + /// `sent_refresh_token`, the token the refresh used. Returns `Ok(false)` + /// without writing when a sibling refresh, `autter login` or `autter logout` + /// changed the store first: overwriting would put back an older session. + /// + /// Callers hold the refresh locks, so this mainly guards a process whose + /// wait for `credentials.lock` timed out. + pub fn store_if_current( + &self, + sent_refresh_token: &str, + creds: &StoredCredentials, + ) -> Result { + match self.load()? { + Some(current) if current.refresh_token == sent_refresh_token => { + self.store(creds)?; + Ok(true) + } + _ => Ok(false), + } + } + + /// Persist that the server refused `refresh_token` for good, if it is + /// still the stored one. An earlier refusal of the same token is kept, so + /// its timestamp (which keys the one-time notice) stays stable. Returns the + /// refusal now on record, or `None` when the store moved on to other + /// credentials (which must not be marked). + pub fn mark_refresh_failed( + &self, + refresh_token: &str, + failure: RefreshFailure, + ) -> Result, String> { + match self.load()? { + Some(mut current) if current.refresh_token == refresh_token => { + if let Some(existing) = current.refresh_failed { + return Ok(Some(existing)); + } + current.refresh_failed = Some(failure.clone()); + self.store(¤t)?; + Ok(Some(failure)) + } + _ => Ok(None), + } + } + /// Clear stored credentials pub fn clear(&self) -> Result<(), String> { self.backend.clear() @@ -212,6 +257,7 @@ mod tests { refresh_token: "test_refresh_token_67890".to_string(), access_token_expires_at: chrono::Utc::now().timestamp() + 3600, refresh_token_expires_at: chrono::Utc::now().timestamp() + 86400 * 90, + refresh_failed: None, } } @@ -258,6 +304,7 @@ mod tests { refresh_token: "opaque-refresh-token".to_string(), access_token_expires_at: chrono::Utc::now().timestamp() + 3600, refresh_token_expires_at: chrono::Utc::now().timestamp() + 86400, + refresh_failed: None, }; store.store(&legacy).unwrap(); @@ -281,6 +328,7 @@ mod tests { refresh_token: String::new(), access_token_expires_at: 1, refresh_token_expires_at: 1, + refresh_failed: None, }) .unwrap(); @@ -303,6 +351,116 @@ mod tests { assert_eq!(loaded.access_token, "new_access_token"); } + // ============= Compare-and-set writes (CE-20) ============= + + fn credentials_with_refresh_token(refresh_token: &str) -> StoredCredentials { + StoredCredentials { + refresh_token: refresh_token.to_string(), + ..make_test_credentials() + } + } + + fn refusal(at: i64) -> RefreshFailure { + RefreshFailure { + at, + error: "invalid_grant".to_string(), + error_code: Some("refresh_token_invalid".to_string()), + description: Some("Refresh token is not valid.".to_string()), + } + } + + #[test] + fn test_store_if_current_writes_over_the_token_that_was_sent() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&credentials_with_refresh_token("rt-1")) + .unwrap(); + + let rotated = credentials_with_refresh_token("rt-2"); + assert!(store.store_if_current("rt-1", &rotated).unwrap()); + assert_eq!(store.load().unwrap().unwrap().refresh_token, "rt-2"); + } + + #[test] + fn test_store_if_current_keeps_a_newer_sibling_rotation() { + // Three processes refreshed rt-1 at once. The first wrote rt-2; the + // later ones must not put back their own (now older) rotation. + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&credentials_with_refresh_token("rt-2")) + .unwrap(); + + let late = credentials_with_refresh_token("rt-2b"); + assert!(!store.store_if_current("rt-1", &late).unwrap()); + assert_eq!(store.load().unwrap().unwrap().refresh_token, "rt-2"); + } + + #[test] + fn test_store_if_current_does_not_resurrect_a_logout() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + assert!( + !store + .store_if_current("rt-1", &credentials_with_refresh_token("rt-2")) + .unwrap() + ); + assert!(store.load().unwrap().is_none()); + } + + #[test] + fn test_mark_refresh_failed_marks_only_the_refused_token() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&credentials_with_refresh_token("rt-new")) + .unwrap(); + + // The store moved on (login or sibling rotation): nothing is marked. + assert_eq!( + store.mark_refresh_failed("rt-old", refusal(1)).unwrap(), + None + ); + assert!(!store.load().unwrap().unwrap().is_login_required()); + + assert_eq!( + store.mark_refresh_failed("rt-new", refusal(1)).unwrap(), + Some(refusal(1)) + ); + let stored = store.load().unwrap().unwrap(); + assert_eq!(stored.refresh_failed, Some(refusal(1))); + assert_eq!(stored.refresh_token, "rt-new"); + } + + #[test] + fn test_mark_refresh_failed_keeps_the_first_refusal() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&credentials_with_refresh_token("rt-1")) + .unwrap(); + store.mark_refresh_failed("rt-1", refusal(10)).unwrap(); + + assert_eq!( + store.mark_refresh_failed("rt-1", refusal(20)).unwrap(), + Some(refusal(10)) + ); + assert_eq!( + store.load().unwrap().unwrap().refresh_failed, + Some(refusal(10)) + ); + } + + #[test] + fn test_login_store_clears_the_refusal() { + let store = CredentialStore::with_backend(Box::new(MockBackend::new())); + store + .store(&credentials_with_refresh_token("rt-1")) + .unwrap(); + store.mark_refresh_failed("rt-1", refusal(1)).unwrap(); + + store + .store(&credentials_with_refresh_token("rt-login")) + .unwrap(); + assert!(!store.load().unwrap().unwrap().is_login_required()); + } + #[test] fn test_load_nonexistent_with_mock() { let store = CredentialStore::with_backend(Box::new(MockBackend::new())); diff --git a/src/auth/notice.rs b/src/auth/notice.rs index 6cb9a5a..1034700 100644 --- a/src/auth/notice.rs +++ b/src/auth/notice.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; use crate::auth::CredentialStore; +use crate::auth::types::RefreshFailure; /// Minimum seconds between notices across processes (24 hours). const NOTICE_INTERVAL_SECS: i64 = 24 * 60 * 60; @@ -76,6 +77,14 @@ fn notice_stamp_path() -> PathBuf { .join("logged-out-notice-at") } +/// Holds the `at` of the last refused login the user was told about. +fn login_refused_notice_stamp_path() -> PathBuf { + crate::mdm::utils::home_dir() + .join(".autter") + .join("internal") + .join("login-refused-notice-at") +} + fn sync_blocked_stamp_path() -> PathBuf { crate::mdm::utils::home_dir() .join(".autter") @@ -280,10 +289,9 @@ pub fn cloud_sync_attention() -> Option { } let creds = CredentialStore::new().load().ok().flatten(); - let auth_blocked = creds - .as_ref() - .is_none_or(|credentials| credentials.is_refresh_token_expired()) - || sync_auth_blocked_recently(); + let auth_blocked = creds.as_ref().is_none_or(|credentials| { + credentials.is_refresh_token_expired() || credentials.is_login_required() + }) || sync_auth_blocked_recently(); if auth_blocked { return Some(CloudSyncAttention::AuthBlocked); @@ -537,11 +545,29 @@ fn read_pending_sync_counts() -> Option { }) } -fn print_sync_attention_message(attention: CloudSyncAttention, pending: PendingSyncCounts) { +/// The server's refusal of the stored login, if it refused it. +fn stored_login_refusal() -> Option { + CredentialStore::new().load().ok().flatten()?.refresh_failed +} + +fn print_sync_attention_message( + attention: CloudSyncAttention, + pending: PendingSyncCounts, + refusal: Option<&RefreshFailure>, +) { eprintln!(); match attention { CloudSyncAttention::AuthBlocked => { - eprintln!("\x1b[1;33m⚠ Cloud sync is paused — your autter login has expired.\x1b[0m"); + if let Some(refusal) = refusal { + eprintln!( + "\x1b[1;33m⚠ Cloud sync is paused — the autter server refused your saved login.\x1b[0m" + ); + eprintln!("\x1b[1;33m {}\x1b[0m", refusal.message()); + } else { + eprintln!( + "\x1b[1;33m⚠ Cloud sync is paused — your autter login has expired.\x1b[0m" + ); + } if pending.total() > 0 { eprintln!( "\x1b[1;33m {} are stored locally and will upload once you're back in.\x1b[0m", @@ -599,6 +625,16 @@ fn eprint_sync_attention(rate_limited: bool) { let Some(attention) = cloud_sync_attention() else { return; }; + let refusal = stored_login_refusal(); + // A login the server just refused is news: say so on the next command even + // inside the 24-hour reminder window, but only once per refusal. + if attention == CloudSyncAttention::AuthBlocked + && let Some(refusal) = &refusal + && !refusal_already_announced(&login_refused_notice_stamp_path(), refusal) + { + announce_login_refused(refusal); + return; + } if rate_limited { if NOTICE_EMITTED.load(Ordering::Relaxed) || recently_notified() { return; @@ -613,7 +649,45 @@ fn eprint_sync_attention(rate_limited: bool) { } let pending = pending_sync_counts(); - print_sync_attention_message(attention, pending); + print_sync_attention_message(attention, pending, refusal.as_ref()); +} + +/// Tell an interactive user, once per refusal, that the server refused their +/// saved login, with the server's own reason. The daemon has no terminal, so +/// its refusals reach the user through the next interactive command instead +/// (see [`maybe_warn_logged_out`]). +pub fn announce_login_refused(refusal: &RefreshFailure) { + if cfg!(test) || !std::io::stdout().is_terminal() { + return; + } + let stamp = login_refused_notice_stamp_path(); + if refusal_already_announced(&stamp, refusal) { + return; + } + if NOTICE_EMITTED + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + { + return; + } + record_refusal_announced(&stamp, refusal); + record_notified(); + print_sync_attention_message( + CloudSyncAttention::AuthBlocked, + pending_sync_counts(), + Some(refusal), + ); +} + +fn refusal_already_announced(stamp: &std::path::Path, refusal: &RefreshFailure) -> bool { + stamp_time(stamp) == Some(refusal.at) +} + +fn record_refusal_announced(stamp: &std::path::Path, refusal: &RefreshFailure) { + if let Some(parent) = stamp.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(stamp, refusal.at.to_string()); } /// Rate-limited reminder on interactive git/autter commands (once per 24 hours). @@ -755,6 +829,30 @@ mod tests { assert!(summary.contains("Last metrics batch received")); } + #[test] + fn login_refusal_is_announced_once_per_refusal() { + let dir = tempfile::tempdir().unwrap(); + let stamp = dir.path().join("login-refused-notice-at"); + let refusal = RefreshFailure { + at: 1_760_000_000, + error: "invalid_grant".to_string(), + error_code: Some("refresh_token_invalid".to_string()), + description: Some("Refresh token is not valid.".to_string()), + }; + + assert!(!refusal_already_announced(&stamp, &refusal)); + record_refusal_announced(&stamp, &refusal); + assert!(refusal_already_announced(&stamp, &refusal)); + assert!(refusal_already_announced(&stamp, &refusal.clone())); + + // A new login that is refused later is news again. + let later = RefreshFailure { + at: refusal.at + 3600, + ..refusal + }; + assert!(!refusal_already_announced(&stamp, &later)); + } + #[test] fn notice_interval_parse_roundtrip() { // The stamp file format is a bare unix timestamp; make sure the diff --git a/src/auth/state.rs b/src/auth/state.rs index e2de48b..729e169 100644 --- a/src/auth/state.rs +++ b/src/auth/state.rs @@ -7,6 +7,8 @@ pub enum AuthState { LoggedIn, SyncBlocked, RefreshExpired, + /// The server refused the stored login for good; carries its reason. + LoginRefused(String), Error(String), } @@ -41,7 +43,9 @@ pub fn collect_auth_status() -> AuthStatus { }, Ok(Some(creds)) => { let identity = extract_identity_from_access_token(&creds.access_token); - let state = if creds.is_refresh_token_expired() { + let state = if let Some(refusal) = &creds.refresh_failed { + AuthState::LoginRefused(refusal.message()) + } else if creds.is_refresh_token_expired() { AuthState::RefreshExpired } else if super::notice::sync_auth_blocked_recently() { AuthState::SyncBlocked diff --git a/src/auth/types.rs b/src/auth/types.rs index fe94f54..b614844 100644 --- a/src/auth/types.rs +++ b/src/auth/types.rs @@ -13,6 +13,11 @@ pub struct StoredCredentials { pub access_token_expires_at: i64, /// Unix timestamp when the refresh token expires pub refresh_token_expires_at: i64, + /// Set once the server refuses `refresh_token` for good (`invalid_grant`). + /// Every process then stops refreshing until `autter login` stores new + /// credentials, which never carry this field. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub refresh_failed: Option, } /// Custom Debug implementation that redacts sensitive token values @@ -23,11 +28,54 @@ impl fmt::Debug for StoredCredentials { .field("refresh_token", &"[REDACTED]") .field("access_token_expires_at", &self.access_token_expires_at) .field("refresh_token_expires_at", &self.refresh_token_expires_at) + .field("refresh_failed", &self.refresh_failed) .finish() } } +/// The server's terminal refusal of a stored refresh token. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RefreshFailure { + /// Unix timestamp of the refusal. + pub at: i64, + /// OAuth `error`, normally `invalid_grant`. + pub error: String, + /// Server `error_code` (e.g. `refresh_token_invalid`); older servers omit it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error_code: Option, + /// Server `error_description`, shown to the user as-is. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, +} + +impl RefreshFailure { + pub fn from_oauth_error(error: &OAuthError) -> Self { + Self { + at: chrono::Utc::now().timestamp(), + error: error.error.clone(), + error_code: error.error_code.clone(), + description: error.error_description.clone(), + } + } + + /// What to tell the user: the server's own wording when it sent one. + pub fn message(&self) -> String { + match self.description.as_deref().map(str::trim) { + Some(description) if !description.is_empty() => description.to_string(), + _ => format!( + "The server no longer accepts this login ({}). Run `autter login` to sign in again.", + self.error + ), + } + } +} + impl StoredCredentials { + /// True once the server refused the refresh token; only `autter login` fixes it. + pub fn is_login_required(&self) -> bool { + self.refresh_failed.is_some() + } + /// Check if the access token is expired or will expire within the given buffer (seconds) pub fn is_access_token_expired(&self, buffer_secs: i64) -> bool { let now = chrono::Utc::now().timestamp(); @@ -63,11 +111,77 @@ pub struct TokenResponse { pub refresh_expires_in: u64, } -/// OAuth error response -#[derive(Debug, Deserialize)] +/// OAuth error response (RFC 6749 §5.2, plus the autter server's extensions) +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct OAuthError { pub error: String, + #[serde(default)] pub error_description: Option, + /// Why a refresh token was refused: `refresh_token_invalid`, `device_revoked`, + /// `refresh_token_expired` or `refresh_token_superseded`. + #[serde(default)] + pub error_code: Option, + /// The server's verdict that only a new `autter login` can recover. + #[serde(default)] + pub login_required: Option, +} + +impl OAuthError { + /// Whether this refusal is terminal for the token that was sent. Servers + /// before `login_required` existed only send `invalid_grant`. + pub fn requires_login(&self) -> bool { + self.login_required.unwrap_or(self.error == "invalid_grant") + } +} + +impl fmt::Display for OAuthError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.error_description.as_deref() { + Some(description) if !description.trim().is_empty() => f.write_str(description), + _ => f.write_str(&self.error), + } + } +} + +/// Why a request to the token endpoint failed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TokenRequestError { + /// No OAuth answer: network failure, unreadable body, or a non-OAuth + /// error page (e.g. a proxy 502). Retry later. + Transport(String), + /// The server answered with an OAuth error body. + Refused { status: u16, error: OAuthError }, +} + +impl TokenRequestError { + /// The parsed OAuth error, when the server sent one. + pub fn oauth_error(&self) -> Option<&OAuthError> { + match self { + Self::Transport(_) => None, + Self::Refused { error, .. } => Some(error), + } + } + + /// True only for a client-error refusal that the server says needs a new + /// login. Network errors, 5xx, `temporarily_unavailable` and + /// `access_denied` stay retryable. + pub fn requires_login(&self) -> bool { + match self { + Self::Transport(_) => false, + Self::Refused { status, error } => { + (400..500).contains(status) && error.requires_login() + } + } + } +} + +impl fmt::Display for TokenRequestError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Transport(message) => f.write_str(message), + Self::Refused { error, .. } => write!(f, "{error}"), + } + } } #[cfg(test)] @@ -80,6 +194,7 @@ mod tests { refresh_token: "test_refresh_token".to_string(), access_token_expires_at: access_expires_at, refresh_token_expires_at: refresh_expires_at, + refresh_failed: None, } } @@ -199,4 +314,160 @@ mod tests { assert!(debug_output.contains("1234567890")); assert!(debug_output.contains("9876543210")); } + + // ============= OAuth error parsing (CE-20) ============= + + fn parse_oauth_error(json: &str) -> OAuthError { + serde_json::from_str(json).unwrap() + } + + #[test] + fn test_current_server_refusal_parses_every_field() { + let error = parse_oauth_error( + r#"{"error":"invalid_grant","error_description":"Refresh token is not valid. Run `autter login` to sign in again.","error_code":"refresh_token_invalid","login_required":true}"#, + ); + assert_eq!(error.error, "invalid_grant"); + assert_eq!(error.error_code.as_deref(), Some("refresh_token_invalid")); + assert_eq!(error.login_required, Some(true)); + assert!(error.requires_login()); + assert_eq!( + error.to_string(), + "Refresh token is not valid. Run `autter login` to sign in again." + ); + } + + #[test] + fn test_every_refusal_code_requires_login() { + for code in [ + "refresh_token_invalid", + "device_revoked", + "refresh_token_expired", + "refresh_token_superseded", + ] { + let error = parse_oauth_error(&format!( + r#"{{"error":"invalid_grant","error_code":"{code}","login_required":true}}"# + )); + assert!(error.requires_login(), "{code} should require login"); + } + } + + #[test] + fn test_older_server_invalid_grant_alone_requires_login() { + let error = parse_oauth_error(r#"{"error":"invalid_grant"}"#); + assert!(error.error_code.is_none()); + assert!(error.login_required.is_none()); + assert!(error.requires_login()); + assert_eq!(error.to_string(), "invalid_grant"); + + let error = parse_oauth_error(r#"{"error":"invalid_grant","login_required":null}"#); + assert!(error.requires_login()); + } + + #[test] + fn test_login_required_flag_decides_when_present() { + assert!( + parse_oauth_error(r#"{"error":"unauthorized_client","login_required":true}"#) + .requires_login() + ); + assert!( + !parse_oauth_error(r#"{"error":"invalid_grant","login_required":false}"#) + .requires_login() + ); + } + + #[test] + fn test_transient_oauth_errors_do_not_require_login() { + for error in ["temporarily_unavailable", "access_denied", "server_error"] { + let error = parse_oauth_error(&format!(r#"{{"error":"{error}"}}"#)); + assert!( + !error.requires_login(), + "{} should be retryable", + error.error + ); + } + } + + fn refused(status: u16, json: &str) -> TokenRequestError { + TokenRequestError::Refused { + status, + error: parse_oauth_error(json), + } + } + + #[test] + fn test_token_request_error_is_terminal_only_for_client_refusals() { + assert!(refused(400, r#"{"error":"invalid_grant"}"#).requires_login()); + assert!(refused(401, r#"{"error":"invalid_grant"}"#).requires_login()); + // A 5xx is an outage, whatever its body says. + assert!(!refused(503, r#"{"error":"invalid_grant"}"#).requires_login()); + assert!(!refused(400, r#"{"error":"temporarily_unavailable"}"#).requires_login()); + assert!(!refused(400, r#"{"error":"access_denied"}"#).requires_login()); + assert!( + !TokenRequestError::Transport("Failed to connect to server: timed out".to_string()) + .requires_login() + ); + } + + #[test] + fn test_token_request_error_exposes_parsed_oauth_error() { + let error = refused( + 400, + r#"{"error":"invalid_grant","error_code":"device_revoked","login_required":true}"#, + ); + let oauth = error.oauth_error().unwrap(); + assert_eq!(oauth.error_code.as_deref(), Some("device_revoked")); + assert!( + TokenRequestError::Transport("x".to_string()) + .oauth_error() + .is_none() + ); + } + + // ============= Persisted refusal ============= + + #[test] + fn test_refresh_failure_message_prefers_server_description() { + let failure = RefreshFailure::from_oauth_error(&parse_oauth_error( + r#"{"error":"invalid_grant","error_description":"Device was revoked.","error_code":"device_revoked"}"#, + )); + assert_eq!(failure.message(), "Device was revoked."); + assert_eq!(failure.error_code.as_deref(), Some("device_revoked")); + + let bare = + RefreshFailure::from_oauth_error(&parse_oauth_error(r#"{"error":"invalid_grant"}"#)); + assert!(bare.message().contains("invalid_grant")); + assert!(bare.message().contains("autter login")); + } + + #[test] + fn test_credentials_without_refusal_field_still_parse() { + // Credentials written by earlier CLI versions. + let creds: StoredCredentials = serde_json::from_str( + r#"{"access_token":"a","refresh_token":"r","access_token_expires_at":1,"refresh_token_expires_at":2}"#, + ) + .unwrap(); + assert!(creds.refresh_failed.is_none()); + assert!(!creds.is_login_required()); + } + + #[test] + fn test_live_credentials_serialize_without_refusal_field() { + let json = serde_json::to_string(&make_credentials(1, 2)).unwrap(); + assert!(!json.contains("refresh_failed")); + } + + #[test] + fn test_refused_credentials_roundtrip() { + let mut creds = make_credentials(1, 2); + creds.refresh_failed = Some(RefreshFailure { + at: 42, + error: "invalid_grant".to_string(), + error_code: Some("refresh_token_superseded".to_string()), + description: Some("Refresh token was replaced.".to_string()), + }); + let loaded: StoredCredentials = + serde_json::from_str(&serde_json::to_string(&creds).unwrap()).unwrap(); + assert!(loaded.is_login_required()); + assert_eq!(loaded.refresh_failed, creds.refresh_failed); + } } diff --git a/src/commands/blame.rs b/src/commands/blame.rs index 007a6de..cdc372a 100644 --- a/src/commands/blame.rs +++ b/src/commands/blame.rs @@ -1425,7 +1425,7 @@ fn output_json_format( .load() .ok() .flatten() - .map(|creds| !creds.is_refresh_token_expired()) + .map(|creds| !creds.is_refresh_token_expired() && !creds.is_login_required()) .unwrap_or(false); let current_user = repo.effective_author_identity().formatted(); diff --git a/src/commands/debug.rs b/src/commands/debug.rs index b791c0c..7eef5cc 100644 --- a/src/commands/debug.rs +++ b/src/commands/debug.rs @@ -334,6 +334,10 @@ fn build_debug_report(options: DebugOptions) -> String { let _ = writeln!(out, "Status: credentials expired (refresh token expired)"); let _ = writeln!(out, " fix: run `autter login` to sign in again"); } + AuthState::LoginRefused(reason) => { + let _ = writeln!(out, "Status: sign-in refused by the server ({})", reason); + let _ = writeln!(out, " fix: run `autter login` to sign in again"); + } AuthState::Error(err) => { let _ = writeln!(out, "Status: ", err); let _ = writeln!( @@ -360,7 +364,10 @@ fn build_debug_report(options: DebugOptions) -> String { let _ = writeln!(out, "User ID: {}", user_id); } else if matches!( &auth_info.state, - AuthState::LoggedIn | AuthState::SyncBlocked | AuthState::RefreshExpired + AuthState::LoggedIn + | AuthState::SyncBlocked + | AuthState::RefreshExpired + | AuthState::LoginRefused(_) ) { let _ = writeln!(out, "User ID: "); } @@ -368,7 +375,10 @@ fn build_debug_report(options: DebugOptions) -> String { let _ = writeln!(out, "Email: {}", email); } else if matches!( &auth_info.state, - AuthState::LoggedIn | AuthState::SyncBlocked | AuthState::RefreshExpired + AuthState::LoggedIn + | AuthState::SyncBlocked + | AuthState::RefreshExpired + | AuthState::LoginRefused(_) ) { let _ = writeln!(out, "Email: "); } @@ -376,7 +386,10 @@ fn build_debug_report(options: DebugOptions) -> String { let _ = writeln!(out, "Name: {}", name); } else if matches!( &auth_info.state, - AuthState::LoggedIn | AuthState::SyncBlocked | AuthState::RefreshExpired + AuthState::LoggedIn + | AuthState::SyncBlocked + | AuthState::RefreshExpired + | AuthState::LoginRefused(_) ) { let _ = writeln!(out, "Name: "); } diff --git a/src/commands/doctor.rs b/src/commands/doctor.rs index 7ffb0bc..fe82db6 100644 --- a/src/commands/doctor.rs +++ b/src/commands/doctor.rs @@ -808,6 +808,24 @@ fn check_auth() -> (DoctorCheck, bool) { }, has_api_key, ), + AuthState::LoginRefused(reason) => ( + DoctorCheck { + section: SECTION_ACCOUNT, + name, + status: if Config::get().notes_backend_kind().uses_http() { + DoctorStatus::Failed + } else { + DoctorStatus::Warning + }, + summary: format!("the server refused the saved login: {}", reason), + details, + remediation: Some( + "run `autter login` to sign in again (ignore this in local-only mode)" + .to_string(), + ), + }, + has_api_key, + ), AuthState::Error(err) => ( DoctorCheck { section: SECTION_ACCOUNT, diff --git a/src/commands/login.rs b/src/commands/login.rs index f344709..41a275c 100644 --- a/src/commands/login.rs +++ b/src/commands/login.rs @@ -16,7 +16,17 @@ pub enum LoginOutcome { pub fn run_device_login() -> Result { let store = CredentialStore::new(); - if let Ok(Some(creds)) = store.load() + let stored = store.load().ok().flatten(); + if let Some(refusal) = stored + .as_ref() + .and_then(|creds| creds.refresh_failed.as_ref()) + { + // Refused credentials are never "already logged in", whatever their expiry says. + eprintln!( + "The server refused your saved login: {}\nStarting a new sign-in...\n", + refusal.message() + ); + } else if let Some(creds) = &stored && !creds.is_refresh_token_expired() { if !crate::auth::notice::sync_auth_blocked_recently() { diff --git a/src/commands/whoami.rs b/src/commands/whoami.rs index 4d15a85..a8d96ab 100644 --- a/src/commands/whoami.rs +++ b/src/commands/whoami.rs @@ -54,6 +54,13 @@ pub fn handle_whoami(args: &[String]) { auth_failed = true; } } + AuthState::LoginRefused(reason) => { + println!("Auth state: sign-in refused by the server ({})", reason); + println!("Fix: run `autter login`"); + if api_ctx.api_key.is_none() { + auth_failed = true; + } + } AuthState::Error(err) => { println!("Auth state: error ({})", err); if api_ctx.api_key.is_none() { diff --git a/src/daemon/telemetry_worker.rs b/src/daemon/telemetry_worker.rs index 76c6b39..77525a3 100644 --- a/src/daemon/telemetry_worker.rs +++ b/src/daemon/telemetry_worker.rs @@ -395,7 +395,7 @@ fn flush_commit_summaries() { let home_client = ApiClient::new(ApiContext::new(None)); if !home_client.is_logged_in() && !home_client.has_api_key() { note_durable_sync_unauthenticated("commit summaries", pending); - if let Some(issue) = home_client.auth_issue() { + if let Some(issue) = home_client.auth_issue_to_log() { tracing::warn!(reason = %issue, pending, "commit summaries: sync authentication unavailable"); } return; @@ -637,7 +637,7 @@ fn flush_stored_metrics() { let client = ApiClient::new(ApiContext::new(None)); if !client.is_logged_in() && !client.has_api_key() { note_durable_sync_unauthenticated("metrics", pending as i64); - if let Some(issue) = client.auth_issue() { + if let Some(issue) = client.auth_issue_to_log() { tracing::warn!(reason = %issue, pending, "metrics: durable sync authentication unavailable"); } return; @@ -1075,7 +1075,7 @@ pub fn flush_notes() { if !client.is_logged_in() && !client.has_api_key() { note_durable_sync_unauthenticated("notes", pending_count); - if let Some(issue) = client.auth_issue() { + if let Some(issue) = client.auth_issue_to_log() { tracing::warn!(reason = %issue, pending = pending_count, "notes: sync authentication unavailable"); } return; @@ -1367,7 +1367,7 @@ fn flush_cas_queue() { let (auth_client, enabled) = cas_client(); if !enabled { note_durable_sync_unauthenticated("cas_transcripts", pending); - if let Some(issue) = auth_client.auth_issue() { + if let Some(issue) = auth_client.auth_issue_to_log() { tracing::warn!(reason = %issue, pending, "cas: sync authentication unavailable"); } return; diff --git a/src/file_changes/mod.rs b/src/file_changes/mod.rs index 3e31e66..ce522ec 100644 --- a/src/file_changes/mod.rs +++ b/src/file_changes/mod.rs @@ -104,7 +104,7 @@ pub fn flush_pending_to_cloud() { "file_changes", pending_count, ); - if let Some(issue) = default_client.auth_issue() { + if let Some(issue) = default_client.auth_issue_to_log() { tracing::warn!(reason = %issue, pending = pending_count, "file-changes: sync authentication unavailable"); } return;