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;