Bug 2060541 - Recheck auth status on startup, weekly

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()

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_check_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)]

components/fxa-client/src/internal/oauth.rs

@@ -119,7 +119,7 @@ impl FirefoxAccount {
 
     /// Check whether user is authorized using our refresh token.
     pub fn check_authorization_status(&mut self) -> Result<IntrospectInfo> {
-        let resp = match self.state.refresh_token() {
+        let resp = match self.state.refresh_token_for_reauth() {
             Some(refresh_token) => {
                 self.auth_circuit_breaker.check()?;
                 self.client

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::{
@@ -42,6 +43,16 @@ impl StateManager {
     }
 
     pub fn refresh_token(&self) -> Option<&RefreshToken> {
+        if self.persisted_state.logged_out_from_auth_issues {
+            None
+        } else {
+            self.persisted_state.refresh_token.as_ref()
+        }
+    }
+
+    /// Gets the refresh token if it is available, even if we are in the
+    /// `logged_out_from_auth_issues` state.
+    pub fn refresh_token_for_reauth(&self) -> Option<&RefreshToken> {
         self.persisted_state.refresh_token.as_ref()
     }
 
@@ -74,6 +85,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_check_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_check_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 +245,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_check_time = None;
         self.flow_store.clear();
     }
 
@@ -220,9 +257,10 @@ impl StateManager {
     ///
     ///   * `current_device_id`
     ///   * `device_capabilities`
+    ///   * `last_auth_check_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 +271,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
         }

components/fxa-client/src/internal/state_persistence.rs

@@ -110,6 +110,7 @@ pub(crate) struct StateV2 {
     pub(crate) server_local_device_info: Option<LocalDevice>,
     #[serde(default)]
     pub(crate) logged_out_from_auth_issues: bool,
+    pub(crate) last_auth_check_time: Option<u64>,
 }
 
 #[cfg(test)]

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)]

components/fxa-client/src/state_machine/transitions.rs

@@ -24,11 +24,34 @@ 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) => {
+                            error_support::report_error!(
+                                "fxaclient-authissues-recheck-succeeded",
+                                "Recheck of auth status succeeded despite being in AuthIssues state"
+                            );
+                            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 +166,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 +205,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 +224,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 +242,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  ─────────────────────────────────

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}"),
-        },
+        }
     }
 }