From 7833171bcb8298d6acd550383a3d5036a89d2a89 Mon Sep 17 00:00:00 2001 From: bytesized Date: Wed, 26 Aug 2026 15:41:13 -0700 Subject: [PATCH] Bug 2060541 - Recheck auth status on startup, weekly --- components/fxa-client/src/auth.rs | 14 +++++ components/fxa-client/src/internal/mod.rs | 14 +++++ .../fxa-client/src/internal/state_manager.rs | 36 ++++++++++-- .../src/internal/state_persistence.rs | 1 + .../fxa-client/src/state_machine/helpers.rs | 13 +++++ .../src/state_machine/transitions.rs | 57 +++++++++++++++---- examples/fxa-client/src/main.rs | 45 ++++++++++++--- 7 files changed, 158 insertions(+), 22 deletions(-) diff --git a/components/fxa-client/src/auth.rs b/components/fxa-client/src/auth.rs index ecbb7107f68..7a8f6d3cffb 100644 --- a/components/fxa-client/src/auth.rs +++ b/components/fxa-client/src/auth.rs @@ -112,6 +112,20 @@ impl FirefoxAccount { self.internal.lock().on_auth_issues() } + /// Reset the timer indicating time since last auth issues were encountered. + /// + /// **💾 This method alters the persisted account state.** + /// + /// Call this if we have encountered the [FxaRustAuthState.AuthIssues] state as a result of a + /// failure happening (i.e. not as a result of initialization simply loading that state from a + /// previous failure). + /// Most likely, this should not need to be called externally except in testing since the state + /// machine's `transition` function should generally call the internal version of this function + /// when necessary. + pub fn reset_auth_recheck_timer(&self) { + self.internal.lock().reset_auth_recheck_timer() + } + /// Used by the application to test auth token issues pub fn simulate_temporary_auth_token_issue(&self) { self.internal.lock().simulate_temporary_auth_token_issue() diff --git a/components/fxa-client/src/internal/mod.rs b/components/fxa-client/src/internal/mod.rs index 2c24db44f5d..5d2b8367e43 100644 --- a/components/fxa-client/src/internal/mod.rs +++ b/components/fxa-client/src/internal/mod.rs @@ -92,6 +92,7 @@ impl FirefoxAccount { last_seen_profile: None, access_token_cache: HashMap::new(), logged_out_from_auth_issues: false, + last_auth_recheck_time: None, }) } @@ -272,6 +273,19 @@ impl FirefoxAccount { pub fn simulate_permanent_auth_token_issue(&mut self) { self.state.simulate_permanent_auth_token_issue() } + + /// Checks if enough time has passed since the last auth attempt that we should try checking the + /// auth again. + pub fn should_recheck_auth(&self) -> bool { + self.state.should_recheck_auth() + } + + /// Set the last time we re-checked our authentication after a failure to the current time. + /// + /// **💾 This method alters the persisted account state.** + pub fn reset_auth_recheck_timer(&mut self) { + self.state.reset_auth_recheck_timer(); + } } #[derive(Debug, Clone, Deserialize, Serialize)] diff --git a/components/fxa-client/src/internal/state_manager.rs b/components/fxa-client/src/internal/state_manager.rs index 04c1776d266..eff0558c357 100644 --- a/components/fxa-client/src/internal/state_manager.rs +++ b/components/fxa-client/src/internal/state_manager.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use std::collections::{HashMap, HashSet}; +use std::time::SystemTime; use crate::{ internal::{ @@ -74,6 +75,31 @@ impl StateManager { self.persisted_state.server_local_device_info = Some(local_device) } + /// Checks if enough time has passed since the last auth attempt that we should try checking the + /// auth again. + pub fn should_recheck_auth(&self) -> bool { + let last_auth_time: u64 = self.persisted_state.last_auth_recheck_time.unwrap_or(0); + // Authentication interval is one week + let next_auth_time = last_auth_time + (7 * 24 * 60 * 60); + // This should only return an error if `now()` is before the epoch. This is an unexpected + // case and we will just return `false` if this happens (presumably resulting in a recheck + // not being made). + if let Ok(now) = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) { + let now: u64 = now.as_secs(); + if next_auth_time <= now { + return true; + } + } + false + } + + pub fn reset_auth_recheck_timer(&mut self) { + // Attempt to reset the timer that indicates how long until we recheck the auth issues + if let Ok(now) = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) { + self.persisted_state.last_auth_recheck_time = Some(now.as_secs()); + } + } + /// Clear out the last known LocalDevice info. This means that the next call to /// `ensure_capabilities()` will re-send our capabilities to the server /// @@ -209,6 +235,7 @@ impl StateManager { self.persisted_state.session_token = None; self.persisted_state.logged_out_from_auth_issues = false; self.persisted_state.last_seen_profile = None; + self.persisted_state.last_auth_recheck_time = None; self.flow_store.clear(); } @@ -220,9 +247,10 @@ impl StateManager { /// /// * `current_device_id` /// * `device_capabilities` + /// * `last_auth_recheck_time` /// * `last_handled_command` + /// * `refresh_token` pub fn on_auth_issues(&mut self) { - self.persisted_state.refresh_token = None; self.persisted_state.scoped_keys = HashMap::new(); self.persisted_state.commands_data = HashMap::new(); self.persisted_state.access_token_cache = HashMap::new(); @@ -233,10 +261,10 @@ impl StateManager { } pub fn get_auth_state(&self) -> FxaRustAuthState { - if self.persisted_state.refresh_token.is_some() { - FxaRustAuthState::Connected - } else if self.persisted_state.logged_out_from_auth_issues { + if self.persisted_state.logged_out_from_auth_issues { FxaRustAuthState::AuthIssues + } else if self.persisted_state.refresh_token.is_some() { + FxaRustAuthState::Connected } else { FxaRustAuthState::Disconnected } diff --git a/components/fxa-client/src/internal/state_persistence.rs b/components/fxa-client/src/internal/state_persistence.rs index 8f37087764d..97f824d3bae 100644 --- a/components/fxa-client/src/internal/state_persistence.rs +++ b/components/fxa-client/src/internal/state_persistence.rs @@ -110,6 +110,7 @@ pub(crate) struct StateV2 { pub(crate) server_local_device_info: Option, #[serde(default)] pub(crate) logged_out_from_auth_issues: bool, + pub(crate) last_auth_recheck_time: Option, } #[cfg(test)] diff --git a/components/fxa-client/src/state_machine/helpers.rs b/components/fxa-client/src/state_machine/helpers.rs index 5e9670426e1..9bf8d6d3088 100644 --- a/components/fxa-client/src/state_machine/helpers.rs +++ b/components/fxa-client/src/state_machine/helpers.rs @@ -249,6 +249,19 @@ impl<'a> RetryingAccount<'a> { } } } + + /// Checks if enough time has passed since the last auth attempt that we should try checking the + /// auth again. + pub fn should_recheck_auth(&self) -> bool { + self.inner.should_recheck_auth() + } + + /// Set the last time we re-checked our authentication after a failure to the current time. + /// + /// **💾 This method alters the persisted account state.** + pub fn reset_auth_recheck_timer(&mut self) { + self.inner.reset_auth_recheck_timer(); + } } #[cfg(test)] diff --git a/components/fxa-client/src/state_machine/transitions.rs b/components/fxa-client/src/state_machine/transitions.rs index d2e3f69f1a5..25a4c43dc9e 100644 --- a/components/fxa-client/src/state_machine/transitions.rs +++ b/components/fxa-client/src/state_machine/transitions.rs @@ -24,11 +24,28 @@ pub fn transition( // ── From Uninitialized ────────────────────────────────────────── (S::Uninitialized, E::Initialize { device_config }) => match account.get_auth_state() { FxaRustAuthState::Disconnected => Ok(S::Disconnected), - FxaRustAuthState::AuthIssues => Ok(S::AuthIssues), + FxaRustAuthState::AuthIssues => { + // This probably indicates that the user is not authorized but there are various + // corner cases where we might have gotten something wrong. For example, a bug in an + // older browser version that we've since fixed or an FxA server bug. + // Because of this, we will recheck the authorization status from time to time. + if account.should_recheck_auth() { + account.reset_auth_recheck_timer(); + match account.check_authorization_status() { + Ok(true) => Ok(S::Connected), + _ => Ok(S::AuthIssues), + } + } else { + Ok(S::AuthIssues) + } + } FxaRustAuthState::Connected => { match account.finish_initialize(&device_config.capabilities) { Ok(()) => Ok(S::Connected), - Err(cause) => Err(StateMachineErr::new(cause, S::AuthIssues)), + Err(cause) => { + account.reset_auth_recheck_timer(); + Err(StateMachineErr::new(cause, S::AuthIssues)) + } } } }, @@ -143,12 +160,18 @@ pub fn transition( let active = account .check_authorization_status() .to_state_machine_err(|| S::Connected)?; - Ok(if active { S::Connected } else { S::AuthIssues }) + if active { + Ok(S::Connected) + } else { + account.reset_auth_recheck_timer(); + Ok(S::AuthIssues) + } } (S::Connected, E::CallGetProfile) => { - account - .get_profile() - .to_state_machine_err(|| S::AuthIssues)?; + account.get_profile().to_state_machine_err(|| { + account.reset_auth_recheck_timer(); + S::AuthIssues + })?; Ok(S::Connected) } ( @@ -176,7 +199,10 @@ pub fn transition( // the device record (push subscription, commands, etc) against the new token. account .handle_web_channel_password_change(&json_payload) - .to_state_machine_err(|| S::AuthIssues)?; + .to_state_machine_err(|| { + account.reset_auth_recheck_timer(); + S::AuthIssues + })?; Ok(S::Connected) } @@ -192,7 +218,10 @@ pub fn transition( let scope_refs: Vec<&str> = scopes.iter().map(String::as_str).collect(); let oauth_url = account .begin_oauth_flow(&service, &scope_refs, &entrypoint) - .to_state_machine_err(|| S::AuthIssues)?; + .to_state_machine_err(|| { + account.reset_auth_recheck_timer(); + S::AuthIssues + })?; Ok(S::Authenticating { oauth_url, initial_state: FxaRustAuthState::AuthIssues, @@ -207,14 +236,22 @@ pub fn transition( // session token recovers us; device re-registration will be handled inside the inner call. account .handle_web_channel_password_change(&json_payload) - .to_state_machine_err(|| S::AuthIssues)?; + .to_state_machine_err(|| { + account.reset_auth_recheck_timer(); + S::AuthIssues + })?; Ok(S::Connected) } (S::AuthIssues, E::CheckAuthorizationStatus) => { let active = account .check_authorization_status() .to_state_machine_err(|| S::AuthIssues)?; - Ok(if active { S::Connected } else { S::AuthIssues }) + if active { + Ok(S::Connected) + } else { + account.reset_auth_recheck_timer(); + Ok(S::AuthIssues) + } } // ── Other transitions ───────────────────────────────── diff --git a/examples/fxa-client/src/main.rs b/examples/fxa-client/src/main.rs index 609905dbd08..47af7895998 100644 --- a/examples/fxa-client/src/main.rs +++ b/examples/fxa-client/src/main.rs @@ -7,7 +7,9 @@ mod send_tab; use clap::{Parser, Subcommand, ValueEnum}; use cli_support::fxa_creds::{self, CliFxa, WELL_KNOWN_SCOPES}; -use fxa_client::{FxaConfig, FxaServer}; +use fxa_client::{ + DeviceCapability, DeviceConfig, DeviceType, FxaConfig, FxaEvent, FxaServer, FxaState, +}; static CLIENT_ID: &str = "a2270f727f45f648"; @@ -75,6 +77,8 @@ enum Command { /// List the clients attached to the account (uses session-token auth). AttachedClients, Disconnect, + /// Force the user into the FxaState::AuthIssues state + ForceAuthIssues, } fn main() -> Result<()> { @@ -92,7 +96,12 @@ fn main() -> Result<()> { println!("The account state managed by this utility can be used by many app-services demos and examples."); println!("Run with `help` or `--help` for more"); print_status(&fxa); - return Ok(()); + + // Even though we are ostensibly just printing the status, sometimes the process of just + // initializing can change the state. This happens, for example, if we are in the + // `AuthIssues` state and the timer has expired to re-check the auth, which is + // successful this time. + return fxa.persist(); } Some(Command::Login { scopes }) => { let scope_refs: Vec<&str> = if scopes.is_empty() { @@ -134,6 +143,10 @@ fn main() -> Result<()> { account.disconnect(); } Command::Login { .. } => unreachable!(), + Command::ForceAuthIssues => { + account.reset_auth_recheck_timer(); + account.on_auth_issues(); + } } } } @@ -164,13 +177,29 @@ impl Cli { fn print_status(fxa: &CliFxa) { match fxa.account() { None => println!("Not logged in"), - Some(account) => match account.check_authorization_status() { - Ok(status) if status.active => { - println!("Account is logged in and authorized by the server") + Some(account) => { + let mut state: FxaState = account.get_state(); + if state == FxaState::Uninitialized { + state = account + .process_event(FxaEvent::Initialize { + device_config: DeviceConfig { + name: "test-device".to_owned(), + device_type: DeviceType::Mobile, + capabilities: vec![DeviceCapability::SendTab], + }, + }) + .unwrap(); + } + println!("Account currently in state: {state}"); + + match account.check_authorization_status() { + Ok(status) if status.active => { + println!("Account is logged in and authorized by the server") + } + Ok(_) => println!("Account is logged in but not authorized by the server"), + Err(e) => println!("Account logged in but account status failed: {e}"), } - Ok(_) => println!("Account is logged in but not authorized by the server"), - Err(e) => println!("Account logged in but account status failed: {e}"), - }, + } } }