Merge branch 'fix-verification-done-wait'

Author: poljar

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

@@ -1568,6 +1568,7 @@ pub(crate) mod test {
         event_id,
         events::{
             dummy::ToDeviceDummyEventContent,
+            key::verification::VerificationMethod,
             room::{
                 encrypted::ToDeviceRoomEncryptedEventContent,
                 message::{MessageType, RoomMessageEventContent},
@@ -2143,25 +2144,17 @@ pub(crate) mod test {
         assert_eq!(alice_sas.emoji(), bob_sas.emoji());
         assert_eq!(alice_sas.decimals(), bob_sas.decimals());
 
-        let event = bob_sas
-            .confirm()
-            .await
-            .unwrap()
-            .0
-            .map(|r| request_to_event(bob.user_id(), &r))
-            .unwrap();
+        let contents = bob_sas.confirm().await.unwrap().0;
+        assert!(contents.len() == 1);
+        let event = request_to_event(bob.user_id(), &contents[0]);
         alice.handle_verification_event(&event).await;
 
         assert!(!alice_sas.is_done());
         assert!(!bob_sas.is_done());
 
-        let event = alice_sas
-            .confirm()
-            .await
-            .unwrap()
-            .0
-            .map(|r| request_to_event(alice.user_id(), &r))
-            .unwrap();
+        let contents = alice_sas.confirm().await.unwrap().0;
+        assert!(contents.len() == 1);
+        let event = request_to_event(alice.user_id(), &contents[0]);
 
         assert!(alice_sas.is_done());
         assert!(bob_device.verified());
@@ -2174,4 +2167,172 @@ pub(crate) mod test {
         assert!(bob_sas.is_done());
         assert!(alice_device.verified());
     }
+
+    #[tokio::test]
+    async fn interactive_verification_started_from_request() {
+        let (alice, bob) = get_machine_pair_with_setup_sessions().await;
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+        let bob_device = alice.get_device(bob.user_id(), bob.device_id()).await.unwrap().unwrap();
+
+        assert!(!bob_device.verified());
+
+        // Alice sends a verification request with her desired methods to Bob
+        let (alice_ver_req, request) =
+            bob_device.request_verification_with_methods(vec![VerificationMethod::SasV1]).await;
+
+        // ----------------------------------------------------------------------------
+        // On Bobs's device:
+        let event = request_to_event(alice.user_id(), &request);
+        bob.handle_verification_event(&event).await;
+        let flow_id = alice_ver_req.flow_id().as_str();
+
+        let verification_request = bob.get_verification_request(alice.user_id(), flow_id).unwrap();
+
+        // Bob accepts the request, sending a Ready request
+        let accept_request =
+            verification_request.accept_with_methods(vec![VerificationMethod::SasV1]).unwrap();
+        // And also immediately sends a start request
+        let (_, start_request_from_bob) = verification_request.start_sas().await.unwrap().unwrap();
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+
+        // Alice receives the Ready
+        let event = request_to_event(bob.user_id(), &accept_request);
+        alice.handle_verification_event(&event).await;
+
+        let verification_request = alice.get_verification_request(bob.user_id(), flow_id).unwrap();
+
+        // And also immediately sends a start request
+        let (alice_sas, start_request_from_alice) =
+            verification_request.start_sas().await.unwrap().unwrap();
+
+        // Now alice receives Bob's start:
+        let event = request_to_event(bob.user_id(), &start_request_from_bob);
+        alice.handle_verification_event(&event).await;
+
+        // Since Alice's user id is lexicographically smaller than Bob's, Alice does not
+        // do anything with the request, however.
+        assert!(alice.user_id() < bob.user_id());
+
+        // ----------------------------------------------------------------------------
+        // On Bob's device:
+
+        // Bob receives Alice's start:
+        let event = request_to_event(alice.user_id(), &start_request_from_alice);
+        bob.handle_verification_event(&event).await;
+
+        let bob_sas = bob
+            .get_verification(alice.user_id(), alice_sas.flow_id().as_str())
+            .unwrap()
+            .sas_v1()
+            .unwrap();
+
+        assert!(alice_sas.emoji().is_none());
+        assert!(bob_sas.emoji().is_none());
+
+        // ... and accepts it
+        let event = bob_sas.accept().map(|r| request_to_event(bob.user_id(), &r)).unwrap();
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+
+        // Alice receives the Accept request:
+        alice.handle_verification_event(&event).await;
+
+        // Alice sends a key
+        let msgs = alice.verification_machine.outgoing_messages();
+        assert!(msgs.len() == 1);
+        let msg = msgs.first().unwrap();
+        let event = outgoing_request_to_event(alice.user_id(), msg);
+        alice.verification_machine.mark_request_as_sent(&msg.request_id);
+
+        // ----------------------------------------------------------------------------
+        // On Bob's device:
+
+        // And bob receive's it:
+        bob.handle_verification_event(&event).await;
+
+        // Now bob sends a key
+        let msgs = bob.verification_machine.outgoing_messages();
+        assert!(msgs.len() == 1);
+        let msg = msgs.first().unwrap();
+        let event = outgoing_request_to_event(bob.user_id(), msg);
+        bob.verification_machine.mark_request_as_sent(&msg.request_id);
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+
+        // And alice receives it
+        alice.handle_verification_event(&event).await;
+
+        // As a result, both devices now can show emojis/decimals
+        assert!(alice_sas.emoji().is_some());
+        assert!(bob_sas.emoji().is_some());
+
+        // ----------------------------------------------------------------------------
+        // On Bob's device:
+
+        assert_eq!(alice_sas.emoji(), bob_sas.emoji());
+        assert_eq!(alice_sas.decimals(), bob_sas.decimals());
+
+        // Bob first confirms that the emojis match and sends the MAC...
+        let contents = bob_sas.confirm().await.unwrap().0;
+        assert!(contents.len() == 1);
+        let event = request_to_event(bob.user_id(), &contents[0]);
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+
+        // ...which alice receives
+        alice.handle_verification_event(&event).await;
+
+        assert!(!alice_sas.is_done());
+        assert!(!bob_sas.is_done());
+
+        // Now alice confirms that the emojis match and sends...
+        let contents = alice_sas.confirm().await.unwrap().0;
+        assert!(contents.len() == 2);
+        // ... her own MAC...
+        let event_mac = request_to_event(alice.user_id(), &contents[0]);
+        // ... and a Done message
+        let event_done = request_to_event(alice.user_id(), &contents[1]);
+
+        // ----------------------------------------------------------------------------
+        // On Bob's device:
+
+        // Bob receives the MAC message
+        bob.handle_verification_event(&event_mac).await;
+
+        // Bob verifies that the MAC is valid and also sends a "done" message.
+        let msgs = bob.verification_machine.outgoing_messages();
+        eprintln!("{:?}", msgs);
+        assert!(msgs.len() == 1);
+        let event = msgs.first().map(|r| outgoing_request_to_event(bob.user_id(), r)).unwrap();
+
+        let alice_device =
+            bob.get_device(alice.user_id(), alice.device_id()).await.unwrap().unwrap();
+
+        assert!(!bob_sas.is_done());
+        assert!(!alice_device.verified());
+        // And Bob receives the Done message of alice.
+        bob.handle_verification_event(&event_done).await;
+
+        assert!(bob_sas.is_done());
+        assert!(alice_device.verified());
+
+        // ----------------------------------------------------------------------------
+        // On Alice's device:
+
+        assert!(!alice_sas.is_done());
+        assert!(!bob_device.verified());
+        // Alices receives the done message
+        eprintln!("{:?}", event);
+        alice.handle_verification_event(&event).await;
+
+        assert!(alice_sas.is_done());
+        assert!(bob_device.verified());
+    }
 }

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

@@ -471,6 +471,18 @@ impl VerificationMachine {
 
                             if s.is_done() {
                                 self.mark_sas_as_done(s, content).await?;
+                            } else {
+                                // Even if we are not done (yet), there might be content to send
+                                // out, e.g. in the case where we are done with our side of the
+                                // verification process, but the other side has not yet sent their
+                                // "done".
+                                if let Some(content) = content {
+                                    self.queue_up_content(
+                                        s.other_user_id(),
+                                        s.other_device_id(),
+                                        content,
+                                    );
+                                }
                             }
                         } else {
                             flow_id_mismatch();
@@ -630,12 +642,16 @@ mod test {
         assert!(bob.emoji().is_some());
         assert_eq!(alice.emoji(), bob.emoji());
 
-        let request = alice.confirm().await.unwrap().0.unwrap();
+        let mut requests = alice.confirm().await.unwrap().0;
+        assert!(requests.len() == 1);
+        let request = requests.pop().unwrap();
         let content = OutgoingContent::try_from(request).unwrap();
         let content = MacContent::try_from(&content).unwrap().into();
         bob.receive_any_event(alice.user_id(), &content);
 
-        let request = bob.confirm().await.unwrap().0.unwrap();
+        let mut requests = bob.confirm().await.unwrap().0;
+        assert!(requests.len() == 1);
+        let request = requests.pop().unwrap();
         let content = OutgoingContent::try_from(request).unwrap();
         let content = MacContent::try_from(&content).unwrap().into();
         alice.receive_any_event(bob.user_id(), &content);

crates/matrix-sdk-crypto/src/verification/mod.rs

@@ -31,6 +31,10 @@ use matrix_sdk_common::locks::Mutex;
 #[cfg(feature = "qrcode")]
 pub use qrcode::{QrVerification, ScanError};
 pub use requests::VerificationRequest;
+#[cfg(feature = "qrcode")]
+use ruma::events::key::verification::done::{
+    KeyVerificationDoneEventContent, ToDeviceKeyVerificationDoneEventContent,
+};
 use ruma::{
     api::client::r0::keys::upload_signatures::Request as SignatureUploadRequest,
     events::{
@@ -39,7 +43,6 @@ use ruma::{
                 CancelCode, KeyVerificationCancelEventContent,
                 ToDeviceKeyVerificationCancelEventContent,
             },
-            done::{KeyVerificationDoneEventContent, ToDeviceKeyVerificationDoneEventContent},
             Relation,
         },
         AnyMessageEventContent, AnyToDeviceEventContent,
@@ -243,6 +246,7 @@ pub struct Done {
 }
 
 impl Done {
+    #[cfg(feature = "qrcode")]
     pub fn as_content(&self, flow_id: &FlowId) -> OutgoingContent {
         match flow_id {
             FlowId::ToDevice(t) => AnyToDeviceEventContent::KeyVerificationDone(
@@ -715,6 +719,12 @@ pub(crate) mod test {
         let sender = sender.to_owned();
 
         match content {
+            AnyToDeviceEventContent::KeyVerificationRequest(c) => {
+                AnyToDeviceEvent::KeyVerificationRequest(ToDeviceEvent { sender, content: c })
+            }
+            AnyToDeviceEventContent::KeyVerificationReady(c) => {
+                AnyToDeviceEvent::KeyVerificationReady(ToDeviceEvent { sender, content: c })
+            }
             AnyToDeviceEventContent::KeyVerificationKey(c) => {
                 AnyToDeviceEvent::KeyVerificationKey(ToDeviceEvent { sender, content: c })
             }
@@ -727,6 +737,9 @@ pub(crate) mod test {
             AnyToDeviceEventContent::KeyVerificationMac(c) => {
                 AnyToDeviceEvent::KeyVerificationMac(ToDeviceEvent { sender, content: c })
             }
+            AnyToDeviceEventContent::KeyVerificationDone(c) => {
+                AnyToDeviceEvent::KeyVerificationDone(ToDeviceEvent { sender, content: c })
+            }
 
             _ => unreachable!(),
         }

crates/matrix-sdk-crypto/src/verification/sas/inner_sas.rs

@@ -228,27 +228,27 @@ impl InnerSas {
         (InnerSas::Cancelled(sas), Some(content))
     }
 
-    pub fn confirm(self) -> (InnerSas, Option<OutgoingContent>) {
+    pub fn confirm(self) -> (InnerSas, Vec<OutgoingContent>) {
         match self {
             InnerSas::KeyReceived(s) => {
                 let sas = s.confirm();
                 let content = sas.as_content();
-                (InnerSas::Confirmed(sas), Some(content))
+                (InnerSas::Confirmed(sas), vec![content])
             }
             InnerSas::MacReceived(s) => {
                 if s.started_from_request {
                     let sas = s.confirm_and_wait_for_done();
-                    let content = sas.as_content();
+                    let contents = vec![sas.as_content(), sas.done_content()];
 
-                    (InnerSas::WaitingForDone(sas), Some(content))
+                    (InnerSas::WaitingForDone(sas), contents)
                 } else {
                     let sas = s.confirm();
                     let content = sas.as_content();
 
-                    (InnerSas::Done(sas), Some(content))
+                    (InnerSas::Done(sas), vec![content])
                 }
             }
-            _ => (self, None),
+            _ => (self, Vec::new()),
         }
     }
 
@@ -335,10 +335,7 @@ impl InnerSas {
             },
             AnyVerificationContent::Done(c) => match self {
                 InnerSas::WaitingForDone(s) => match s.into_done(sender, c) {
-                    Ok(s) => {
-                        let content = s.done_content();
-                        (InnerSas::Done(s), Some(content))
-                    }
+                    Ok(s) => (InnerSas::Done(s), None),
                     Err(s) => {
                         let content = s.as_content();
                         (InnerSas::Cancelled(s), Some(content))