Bug 2061224 - Make Disconnect idempotent

CHANGELOG.md

@@ -10,6 +10,10 @@
 - Add address metadata APIs for importing records already persisted elsewhere: `add_address_with_meta`, `add_many_addresses_with_meta`, `update_address_with_meta` and `add_many_address_tombstones`, with the bulk variants isolating per-record failures. `AddressMeta` carries the guid, timestamps and `sync_change_counter`, so a record keeps whether it still has changes pending upload.
 - Add `Store::addresses_bridged_engine()`, exposing the existing address sync engine through `mozIBridgedSyncEngine` so Firefox Desktop can drive address sync.
 
+### Fxa Client
+- The `CheckAuthorizationStatus` and `Disconnect` events are now valid from all states except `Uninitialized`.
+  In the cases where the failed before, they're now no-ops.
+
 ### Nimbus
 
 - `NimbusClient::get_available_firefox_labs()` now includes detailed debug level logging for each processed lab. ([#7482](https://github.com/mozilla/application-services/pull/7482))

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

@@ -170,7 +170,10 @@ mod driver_tests {
             .unwrap();
         assert_eq!(account.get_state(), FxaState::Disconnected);
 
-        let result = account.process_event(FxaEvent::Disconnect);
+        let result = account.process_event(FxaEvent::CompleteOAuthFlow {
+            code: "test".into(),
+            state: "test".into(),
+        });
 
         match result {
             Err(Error::InvalidStateTransition(_)) => {}

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

@@ -228,6 +228,13 @@ pub fn transition(
             error_support::debug!("Ignoring `CheckAuthorizationStatus` from {from_state:?}");
             Ok(from_state)
         }
+        (S::Disconnected, FxaEvent::Disconnect) => {
+            // Ignore Disconnect from the Disconnected state.
+            //
+            // This makes it idempotent and safe to send from any state.
+            // https://bugzilla.mozilla.org/show_bug.cgi?id=2061224
+            Ok(S::Disconnected)
+        }
 
         // ── Invalid (state, event) pair ─────────────────────────────────
         (state, event) => Err(StateMachineErr::Fatal(Box::new(
@@ -319,12 +326,40 @@ mod tests {
     }
 
     #[test]
-    fn disconnected_invalid_event_returns_fatal_invalid_state_transition() {
+    fn disconnect_is_idempotent() {
         nss_as::ensure_initialized();
+        // `Disconnect` should be valid from all states and always result in the user being
+        // disconnected.
         let mut account = mock_account();
         let mut wrapper = RetryingAccount::new(&mut account);
-        let result = transition(&mut wrapper, FxaState::Disconnected, FxaEvent::Disconnect);
-        assert_fatal_invalid_transition(result);
+
+        assert_eq!(
+            transition(&mut wrapper, FxaState::Connected, FxaEvent::Disconnect).unwrap(),
+            FxaState::Disconnected
+        );
+
+        assert_eq!(
+            transition(&mut wrapper, FxaState::AuthIssues, FxaEvent::Disconnect).unwrap(),
+            FxaState::Disconnected
+        );
+
+        assert_eq!(
+            transition(
+                &mut wrapper,
+                FxaState::Authenticating {
+                    oauth_url: "test".into(),
+                    initial_state: FxaRustAuthState::Disconnected
+                },
+                FxaEvent::Disconnect
+            )
+            .unwrap(),
+            FxaState::Disconnected
+        );
+
+        assert_eq!(
+            transition(&mut wrapper, FxaState::Disconnected, FxaEvent::Disconnect).unwrap(),
+            FxaState::Disconnected
+        );
     }
 
     fn assert_handled_lands_at(