crypto: Fix UserIdentity::is_verified to take into account our own id…

…entity

The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate
before version 0.7.2 doesn't take into account the verification status
of the user's own identity while performing the check and may as a result
return a value contrary to what is implied by its name and documentation.

This patch fixes this and adds a regression test.

The method itself is not used internally and as such has not a larger
impact.

Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Signed-off-by: Damir Jelić <poljar@termina.org.uk>

crates/matrix-sdk-crypto/src/identities/user.rs

@@ -226,7 +226,11 @@ impl Deref for UserIdentity {
 impl UserIdentity {
     /// Is this user identity verified.
     pub fn is_verified(&self) -> bool {
-        self.own_identity.as_ref().is_some_and(|o| o.is_identity_signed(&self.inner).is_ok())
+        self.own_identity.as_ref().is_some_and(|own_identity| {
+            // The identity of another user is verified iff our own identity is verified and
+            // if our own identity has signed the other user's identity.
+            own_identity.is_verified() && own_identity.is_identity_signed(&self.inner).is_ok()
+        })
     }
 
     /// Manually verify this user.
@@ -778,7 +782,7 @@ pub(crate) mod tests {
 
     use assert_matches::assert_matches;
     use matrix_sdk_test::async_test;
-    use ruma::{device_id, user_id};
+    use ruma::{device_id, user_id, UserId};
     use serde_json::{json, Value};
     use tokio::sync::Mutex;
 
@@ -788,10 +792,15 @@ pub(crate) mod tests {
     };
     use crate::{
         identities::{manager::testing::own_key_query, Device},
+        machine::tests::{
+            get_machine_pair, mark_alice_identity_as_verified_test_helper,
+            setup_cross_signing_for_machine_test_helper,
+        },
         olm::{Account, PrivateCrossSigningIdentity},
-        store::{CryptoStoreWrapper, MemoryStore},
+        store::{Changes, CryptoStoreWrapper, MemoryStore},
         types::{CrossSigningKey, MasterPubkey, SelfSigningPubkey, Signatures, UserSigningPubkey},
         verification::VerificationMachine,
+        OlmMachine,
     };
 
     #[test]
@@ -1006,4 +1015,84 @@ pub(crate) mod tests {
             [second_device_id]
         );
     }
+
+    async fn get_machine_pair_with_signed_identities(
+        alice: &UserId,
+        bob: &UserId,
+    ) -> (OlmMachine, OlmMachine) {
+        let (alice, bob, _) = get_machine_pair(alice, bob, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;
+
+        (alice, bob)
+    }
+
+    #[async_test]
+    async fn test_other_user_is_verified_if_my_identity_is_verified_and_they_are_cross_signed() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+        let (alice, bob) =
+            get_machine_pair_with_signed_identities(alice_user_id, bob_user_id).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");
+    }
+
+    #[async_test]
+    async fn test_other_user_is_not_verified_if_they_are_not_cross_signed() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(!bobs_alice_identity.is_verified(), "Alice's identity should not be considered verified since Bob has not signed it.");
+    }
+
+    #[async_test]
+    async fn test_other_user_is_not_verified_if_my_identity_is_not_verified() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+
+        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");
+
+        bobs_own_identity.mark_as_unverified();
+
+        bob.store()
+            .save_changes(Changes {
+                identities: crate::store::IdentityChanges {
+                    changed: vec![bobs_own_identity.inner.clone().into()],
+                    ..Default::default()
+                },
+                ..Default::default()
+            })
+            .await
+            .unwrap();
+
+        assert!(!bobs_own_identity.is_verified(), "Bob's identity should not be verified anymore.");
+        assert!(
+            !bobs_alice_identity.is_verified(),
+            "Alice's identity should not be verified either."
+        );
+    }
 }

crates/matrix-sdk-crypto/src/machine.rs

@@ -3223,7 +3223,7 @@ pub(crate) mod tests {
         );
     }
 
-    async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
+    pub async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
         let CrossSigningBootstrapRequests { upload_signing_keys_req: alice_upload_signing, .. } =
             alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request");
 
@@ -3344,7 +3344,7 @@ pub(crate) mod tests {
         bob.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
     }
 
-    async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
+    pub async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
         let alice_device =
             bob.get_device(alice.user_id(), alice.device_id(), None).await.unwrap().unwrap();