fixup! sdk: fix identity reset not actually disabling backups when no…

…t enabled locally, resulting in conflicts and failing to correctly setup the newly reset session

crates/matrix-sdk/src/encryption/backups/mod.rs

@@ -163,7 +163,11 @@ impl Backups {
         result
     }
 
-    /// Disable and delete the currently active backup.
+    /// Disable and delete the currently active backup only if previously
+    /// enabled before, oterwise an error will be returned.
+    ///
+    /// For a more aggresive variant see [`Backups::disable_and_delete`] which
+    /// will delete the remote backup without checking the local state.
     ///
     /// # Examples
     ///
@@ -200,7 +204,6 @@ impl Backups {
                 info!("Backup successfully deleted");
 
                 olm_machine.backup_machine().disable_backup().await?;
-                self.set_state(BackupState::Unknown);
 
                 info!("Backup successfully disabled and deleted");
 
@@ -213,9 +216,60 @@ impl Backups {
 
         let result = future.await;
 
-        if result.is_err() {
-            self.set_state(BackupState::Unknown);
-        }
+        self.set_state(BackupState::Unknown);
+
+        result
+    }
+
+    /// Completely disable and delete the active backup both locally
+    /// and from the backend no matter if previously setup locally
+    /// or not.
+    ///
+    /// ⚠️ This method is mainly used when resetting the crypto identity
+    /// and for most other use cases its safer [`Backups::disable`] counterpart
+    /// should be used.
+    ///
+    /// It will fetch the current backup version from the backend and delete it
+    /// before proceeding to disabling local backups as well
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// # use matrix_sdk::{Client, encryption::backups::BackupState};
+    /// # use url::Url;
+    /// # async {
+    /// # let homeserver = Url::parse("http://example.com")?;
+    /// # let client = Client::new(homeserver).await?;
+    /// let backups = client.encryption().backups();
+    /// backups.disable_and_delete().await?;
+    ///
+    /// assert_eq!(backups.state(), BackupState::Unknown);
+    /// # anyhow::Ok(()) };
+    /// ```
+    pub async fn disable_and_delete(&self) -> Result<(), Error> {
+        let _guard = self.client.locks().backup_modify_lock.lock().await;
+
+        self.set_state(BackupState::Disabling);
+
+        // Create a future so we can catch errors and go back to the `Unknown` state.
+        let future = async {
+            let response = self.get_current_version().await?;
+
+            if let Some(response) = response {
+                self.delete_backup_from_server(response.version).await?;
+            }
+
+            let olm_machine = self.client.olm_machine().await;
+            let olm_machine = olm_machine.as_ref().ok_or(Error::NoOlmMachine)?;
+
+            olm_machine.backup_machine().disable_backup().await?;
+
+            Ok(())
+        };
+
+        let result = future.await;
+
+        self.set_state(BackupState::Unknown);
 
         result
     }
@@ -423,39 +477,6 @@ impl Backups {
         Ok(())
     }
 
-    /// Deletes the current backup version from the server
-    pub async fn delete_backup(&self) -> Result<(), Error> {
-        let _guard = self.client.locks().backup_modify_lock.lock().await;
-
-        self.set_state(BackupState::Disabling);
-
-        // Create a future so we can catch errors and go back to the `Unknown` state.
-        let future = async {
-            let response = self.get_current_version().await?;
-
-            if let Some(response) = response {
-                self.delete_backup_from_server(response.version).await?;
-            }
-
-            let olm_machine = self.client.olm_machine().await;
-            let olm_machine = olm_machine.as_ref().ok_or(Error::NoOlmMachine)?;
-
-            olm_machine.backup_machine().disable_backup().await?;
-
-            self.set_state(BackupState::Unknown);
-
-            Ok(())
-        };
-
-        let result = future.await;
-
-        if result.is_err() {
-            self.set_state(BackupState::Unknown);
-        }
-
-        result
-    }
-
     /// Set the state of the backup.
     fn set_state(&self, new_state: BackupState) {
         let old_state = self.client.inner.e2ee.backup_state.global_state.set(new_state);

crates/matrix-sdk/src/encryption/recovery/mod.rs

@@ -394,7 +394,7 @@ impl Recovery {
     /// # anyhow::Ok(()) };
     /// ```
     pub async fn reset_identity(&self) -> Result<Option<IdentityResetHandle>> {
-        self.client.encryption().backups().delete_backup().await?; // 1.
+        self.client.encryption().backups().disable_and_delete().await?; // 1.
 
         // 2. (We can't delete account data events)
         self.client.account().set_account_data(SecretStorageDisabledContent {}).await?;

crates/matrix-sdk/tests/integration/encryption/recovery.rs

@@ -779,19 +779,53 @@ async fn test_reset_identity() {
     assert_eq!(client.encryption().backups().state(), BackupState::Enabled);
     assert_eq!(client.encryption().recovery().state(), RecoveryState::Enabled);
 
+    let did_delete_backup = Arc::new(Mutex::new(false));
+
     // Disabling backups
     Mock::given(method("GET"))
         .and(path("_matrix/client/r0/room_keys/version"))
         .and(header("authorization", "Bearer 1234"))
-        .respond_with(ResponseTemplate::new(404).set_body_json(json!({
-            "errcode": "M_NOT_FOUND",
-            "error": "No current backup version"
-        })))
+        .respond_with({
+            let did_delete_backup = did_delete_backup.clone();
+            move |_: &wiremock::Request| {
+                if did_delete_backup.lock().unwrap().clone() {
+                    ResponseTemplate::new(404).set_body_json(json!({
+                        "errcode": "M_NOT_FOUND",
+                        "error": "No current backup version"
+                    }))
+                } else {
+                    ResponseTemplate::new(200).set_body_json(json!({
+                        "algorithm": "m.megolm_backup.v1.curve25519-aes-sha2",
+                        "auth_data": {
+                            "public_key": "hdx5rSn94rBuvJI5cwnhKAVmFyZgfJjk7vwEBD6mIHc",
+                            "signatures": {}
+                        },
+                        "count": 1,
+                        "etag": "1",
+                        "version": "1"
+                    }))
+                }
+            }
+        })
         .expect(3)
         .named("room_keys/version GET")
         .mount(&server)
         .await;
 
+    Mock::given(method("DELETE"))
+        .and(path("_matrix/client/r0/room_keys/version/1"))
+        .and(header("authorization", "Bearer 1234"))
+        .respond_with({
+            let did_delete_backup = did_delete_backup.clone();
+            move |_: &wiremock::Request| {
+                *did_delete_backup.lock().unwrap() = true;
+                ResponseTemplate::new(200).set_body_json(json!({}))
+            }
+        })
+        .expect(1)
+        .mount(&server)
+        .await;
+
     // Disabling recovery
     Mock::given(method("PUT"))
         .and(path(format!(