From 4c2e94cb92155efe55398aff79ea0f371dabbca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Mon, 24 Feb 2025 13:55:52 +0100 Subject: [PATCH] Try to make integration test for user verification work --- crates/matrix-sdk-base/src/client.rs | 8 +- crates/matrix-sdk-base/src/sliding_sync.rs | 10 +- .../matrix-sdk-crypto/src/olm/signing/mod.rs | 2 +- .../matrix-sdk-ui/src/notification_client.rs | 3 +- crates/matrix-sdk/src/sliding_sync/builder.rs | 12 +-- crates/matrix-sdk/src/sliding_sync/client.rs | 97 ++++++++++++++++--- crates/matrix-sdk/src/sliding_sync/mod.rs | 6 +- crates/matrix-sdk/tests/integration/client.rs | 47 ++++----- .../src/tests/e2ee.rs | 69 ++++++++++++- 9 files changed, 192 insertions(+), 62 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 10facda94..ac948b988 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -409,8 +409,8 @@ impl BaseClient { limited: bool, events: Vec>, ignore_state_events: bool, - #[cfg(feature = "e2e-encryption")] ignore_verification_requests: bool, - #[cfg(not(feature = "e2e-encryption"))] _ignore_verification_requests: bool, + #[cfg(feature = "e2e-encryption")] ignore_verification_events: bool, + #[cfg(not(feature = "e2e-encryption"))] _ignore_verification_events: bool, prev_batch: Option, push_rules: &Ruleset, user_ids: &mut BTreeSet, @@ -498,7 +498,7 @@ impl BaseClient { SyncMessageLikeEvent::Original(original_event), ) => match &original_event.content.msgtype { MessageType::VerificationRequest(_) => { - if !ignore_verification_requests { + if !ignore_verification_events { Box::pin(self.handle_verification_event(e, room.room_id())) .await?; } @@ -506,7 +506,7 @@ impl BaseClient { _ => (), }, _ if e.event_type().to_string().starts_with("m.key.verification") => { - if !ignore_verification_requests { + if !ignore_verification_events { Box::pin(self.handle_verification_event(e, room.room_id())) .await?; } diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 404b95abf..5214b98a3 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -124,14 +124,14 @@ impl BaseClient { /// sync. /// * `previous_events_provider` - Timeline events prior to the current /// sync. - /// * `ignore_verification_requests` - Whether verification requests found + /// * `ignore_verification_events` - Whether verification requests found /// in the response should be ignored. #[instrument(skip_all, level = "trace")] pub async fn process_sliding_sync( &self, response: &http::Response, previous_events_provider: &PEP, - ignore_verification_requests: bool, + ignore_verification_events: bool, ) -> Result { let http::Response { // FIXME not yet supported by sliding sync. see @@ -190,7 +190,7 @@ impl BaseClient { &mut room_info_notable_updates, &mut notifications, &mut ambiguity_cache, - ignore_verification_requests, + ignore_verification_events, ) .await?; @@ -362,7 +362,7 @@ impl BaseClient { room_info_notable_updates: &mut BTreeMap, notifications: &mut BTreeMap>, ambiguity_cache: &mut AmbiguityCache, - ignore_verification_requests: bool, + ignore_verification_events: bool, ) -> Result<( RoomInfo, Option, @@ -445,7 +445,7 @@ impl BaseClient { room_data.limited, room_data.timeline.clone(), true, - ignore_verification_requests, + ignore_verification_events, room_data.prev_batch.clone(), &push_rules, &mut user_ids, diff --git a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs index 061ada9ff..7df2cf358 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs @@ -415,7 +415,7 @@ impl PrivateCrossSigningIdentity { Ok((master, self_signing, user_signing)) } - pub(crate) async fn to_public_identity(&self) -> Result { + pub async fn to_public_identity(&self) -> Result { let (master, self_signing, user_signing) = self.public_keys().await?; let identity = OwnUserIdentityData::new(master, self_signing, user_signing)?; diff --git a/crates/matrix-sdk-ui/src/notification_client.rs b/crates/matrix-sdk-ui/src/notification_client.rs index 0af1ff281..9298a3a5e 100644 --- a/crates/matrix-sdk-ui/src/notification_client.rs +++ b/crates/matrix-sdk-ui/src/notification_client.rs @@ -427,7 +427,8 @@ impl NotificationClient { assign!(http::request::AccountData::default(), { enabled: Some(true) }), ) .add_list(invites) - .ignore_verification_requests() + // TODO: re-enable once we verified the test fails without it + // .ignore_verification_events() .build() .await?; diff --git a/crates/matrix-sdk/src/sliding_sync/builder.rs b/crates/matrix-sdk/src/sliding_sync/builder.rs index 3cfd3d4ba..504b80568 100644 --- a/crates/matrix-sdk/src/sliding_sync/builder.rs +++ b/crates/matrix-sdk/src/sliding_sync/builder.rs @@ -35,7 +35,7 @@ pub struct SlidingSyncBuilder { #[cfg(feature = "e2e-encryption")] share_pos: bool, #[cfg(feature = "e2e-encryption")] - ignore_verification_requests: bool, + ignore_verification_events: bool, } impl SlidingSyncBuilder { @@ -59,7 +59,7 @@ impl SlidingSyncBuilder { #[cfg(feature = "e2e-encryption")] share_pos: false, #[cfg(feature = "e2e-encryption")] - ignore_verification_requests: false, + ignore_verification_events: false, }) } } @@ -233,8 +233,8 @@ impl SlidingSyncBuilder { /// otherwise those would be processed several times and the verification /// flow will be broken. #[cfg(feature = "e2e-encryption")] - pub fn ignore_verification_requests(mut self) -> Self { - self.ignore_verification_requests = true; + pub fn ignore_verification_events(mut self) -> Self { + self.ignore_verification_events = true; self } @@ -308,9 +308,9 @@ impl SlidingSyncBuilder { poll_timeout: self.poll_timeout, network_timeout: self.network_timeout, #[cfg(feature = "e2e-encryption")] - ignore_verification_requests: self.ignore_verification_requests, + ignore_verification_events: self.ignore_verification_events, #[cfg(not(feature = "e2e-encryption"))] - ignore_verification_requests: false, + ignore_verification_events: false, })) } } diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index ce4e7f7ed..3988217d4 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -168,21 +168,21 @@ pub(crate) struct SlidingSyncResponseProcessor<'a> { to_device_events: Vec>, response: Option, rooms: &'a BTreeMap, - ignore_verification_requests: bool, + ignore_verification_events: bool, } impl<'a> SlidingSyncResponseProcessor<'a> { pub fn new( client: Client, rooms: &'a BTreeMap, - ignore_verification_requests: bool, + ignore_verification_events: bool, ) -> Self { Self { client, to_device_events: Vec::new(), response: None, rooms, - ignore_verification_requests, + ignore_verification_events, } } @@ -219,7 +219,7 @@ impl<'a> SlidingSyncResponseProcessor<'a> { .process_sliding_sync( response, &SlidingSyncPreviousEventsProvider(self.rooms), - self.ignore_verification_requests, + self.ignore_verification_events, ) .await?, ); @@ -270,20 +270,22 @@ mod tests { use assert_matches::assert_matches; use matrix_sdk_base::notification_settings::RoomNotificationMode; use matrix_sdk_test::async_test; - use ruma::{assign, room_id, serde::Raw}; + use ruma::{assign, device_id, owned_device_id, room_id, serde::Raw, user_id, MilliSecondsSinceUnixEpoch, SecondsSinceUnixEpoch}; + use ruma::events::key::verification::VerificationMethod; + use ruma::events::room::message::{KeyVerificationRequestEventContent, MessageType, RoomMessageEventContent}; use serde_json::json; + use tokio::sync::Mutex; use wiremock::{ matchers::{method, path}, Mock, ResponseTemplate, }; - - use super::{get_supported_versions, Version, VersionBuilder}; - use crate::{ - error::Result, - sliding_sync::{http, VersionBuilderError}, - test_utils::logged_in_client_with_server, - SlidingSyncList, SlidingSyncMode, - }; + use matrix_sdk_test::event_factory::EventFactory; + use super::{get_supported_versions, SlidingSyncResponseProcessor, Version, VersionBuilder}; + use crate::{error::Result, sliding_sync::{http, VersionBuilderError}, test_utils::logged_in_client_with_server, SlidingSyncList, SlidingSyncMode, SlidingSyncRoom}; + use crate::crypto::{Account, DeviceData}; + use crate::crypto::olm::PrivateCrossSigningIdentity; + use crate::crypto::store::Changes; + use crate::test_utils::mocks::MatrixMockServer; #[test] fn test_version_builder_none() { @@ -465,4 +467,73 @@ mod tests { Ok(()) } + + // This works, but needs to make `to_public_identity` public + #[async_test] + async fn test_ignore_verification_events() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_id = room_id!("!r0:matrix.org"); + let user_id = user_id!("@other:localhost"); + + let sliding_sync = client.sliding_sync("notifications") + .expect("Should be able to create a sliding sync builder") + .add_list( + SlidingSyncList::builder("all") + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=10)), + ) + .ignore_verification_events() + .build() + .await + .expect("Should be able to build the sliding sync"); + + sliding_sync.subscribe_to_rooms(&[room_id], None, false); + + let verification_request_content = KeyVerificationRequestEventContent::new("Test".to_owned(), vec![VerificationMethod::QrCodeScanV1], owned_device_id!("BOBDEVICE"), user_id.to_owned()); + let event = EventFactory::new().room(room_id).sender(user_id).event(RoomMessageEventContent::new(MessageType::VerificationRequest(verification_request_content.clone()))).into_event(); + + let room = SlidingSyncRoom::new(room_id.to_owned(), None, vec![event]); + + let rooms = BTreeMap::from([(room_id.to_owned(), room)]); + let mut processor = SlidingSyncResponseProcessor::new(client.clone(), &rooms, true); + + let mut room = ruma::api::client::sync::sync_events::v5::response::Room::new(); + let mut f = EventFactory::new(); + f.set_next_ts(MilliSecondsSinceUnixEpoch::now().get().into()); + let event = f.room(room_id).sender(user_id).event(RoomMessageEventContent::new(MessageType::VerificationRequest(verification_request_content))).into_raw_sync(); + room.timeline = vec![event]; + + let mut response = ruma::api::client::sync::sync_events::v5::Response::new("0".to_owned()); + response.rooms.insert(room_id.to_owned(), room); + + let olm_machine = client.olm_machine_for_testing().await.clone().expect("Got olm machine"); + + let alice_account = Account::with_device_id(user_id, device_id!("ALIDEVICE")); + let private_identity = PrivateCrossSigningIdentity::new(user_id.to_owned()); + + let bob_account = + Account::with_device_id(alice_account.user_id(), device_id!("BOBDEVICE")); + + let private_identity = PrivateCrossSigningIdentity::new(user_id.to_owned()); + let identity = private_identity.to_public_identity().await.unwrap(); + + let master_key = private_identity.master_public_key().await.unwrap(); + let master_key = master_key.get_first_key().unwrap().to_owned(); + + let alice_device = DeviceData::from_account(&alice_account); + let bob_device = DeviceData::from_account(&bob_account); + + let mut changes = Changes::default(); + changes.identities.new.push(identity.clone().into()); + changes.devices.new.push(bob_device.clone()); + + olm_machine.store().save_changes(changes).await.expect("Changes should be saved"); + + processor.handle_room_response(&response).await.expect("Should be able to handle room response"); + + let requests = olm_machine.get_verification_requests(user_id); + + assert!(requests.is_empty()); + } } diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 99f146c71..f75bc6ba4 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -121,8 +121,8 @@ pub(super) struct SlidingSyncInner { /// types. internal_channel: Sender, - /// Ignore any verification requests received in the sync. - ignore_verification_requests: bool, + /// Ignore any verification events received in the sync. + ignore_verification_events: bool, } impl SlidingSync { @@ -291,7 +291,7 @@ impl SlidingSync { let mut response_processor = SlidingSyncResponseProcessor::new( self.inner.client.clone(), rooms, - self.inner.ignore_verification_requests, + self.inner.ignore_verification_events, ); #[cfg(feature = "e2e-encryption")] diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index aafaaa3fa..58c50663d 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -2,14 +2,9 @@ use std::{collections::BTreeMap, time::Duration}; use assert_matches2::{assert_let, assert_matches}; use eyeball_im::VectorDiff; -use futures_util::FutureExt; -use matrix_sdk::{ - authentication::matrix::{MatrixSession, MatrixSessionTokens}, - config::{RequestConfig, StoreConfig, SyncSettings}, - sync::RoomUpdate, - test_utils::no_retry_test_client_with_server, - Client, MemoryStore, SessionMeta, StateChanges, StateStore, -}; +use futures_util::{pin_mut, FutureExt}; +use js_int::UInt; +use matrix_sdk::{authentication::matrix::{MatrixSession, MatrixSessionTokens}, config::{RequestConfig, StoreConfig, SyncSettings}, sync::RoomUpdate, test_utils::no_retry_test_client_with_server, Client, MemoryStore, SessionMeta, SlidingSyncList, SlidingSyncMode, StateChanges, StateStore}; use matrix_sdk_base::{sync::RoomUpdates, RoomState}; use matrix_sdk_test::{ async_test, sync_state_event, @@ -24,33 +19,29 @@ use matrix_sdk_test::{ }, GlobalAccountDataTestEvent, JoinedRoomBuilder, SyncResponseBuilder, DEFAULT_TEST_ROOM_ID, }; -use ruma::{ - api::client::{ - directory::{ - get_public_rooms, - get_public_rooms_filtered::{self, v3::Request as PublicRoomsFilterRequest}, - }, - uiaa, +use ruma::{api::client::{ + directory::{ + get_public_rooms, + get_public_rooms_filtered::{self, v3::Request as PublicRoomsFilterRequest}, }, - assign, device_id, - directory::Filter, - event_id, - events::{ - direct::{DirectEventContent, OwnedDirectUserIdentifier}, - AnyInitialStateEvent, - }, - room_id, - serde::Raw, - user_id, OwnedUserId, -}; + uiaa, +}, assign, device_id, directory::Filter, event_id, events::{ + direct::{DirectEventContent, OwnedDirectUserIdentifier}, + AnyInitialStateEvent, +}, owned_device_id, owned_event_id, owned_user_id, room_id, serde::Raw, user_id, MilliSecondsSinceUnixEpoch, OwnedUserId}; +use ruma::events::{AnySyncMessageLikeEvent, AnySyncTimelineEvent, MessageLikeUnsigned, OriginalSyncMessageLikeEvent}; +use ruma::events::key::verification::VerificationMethod; +use ruma::events::room::message::{KeyVerificationRequestEventContent, MessageType, RoomMessageEventContent, SyncRoomMessageEvent}; use serde_json::{json, Value as JsonValue}; use stream_assert::{assert_next_matches, assert_pending}; +use tokio_stream::StreamExt; use tokio_stream::wrappers::BroadcastStream; use wiremock::{ matchers::{header, method, path, path_regex}, Mock, Request, ResponseTemplate, }; - +use matrix_sdk::test_utils::mocks::MatrixMockServer; +use matrix_sdk_test::event_factory::EventFactory; use crate::{logged_in_client_with_server, mock_sync}; #[async_test] @@ -1249,7 +1240,7 @@ async fn test_rooms_stream() { assert!(client.get_room(room_id_3).is_some()); // We receive 3 diffs… - assert_let!(Some(diffs) = rooms_stream.next().await); + assert_let!(Some(diffs) = futures_util::StreamExt::next(&mut rooms_stream).await); assert_eq!(diffs.len(), 3); // … which map to the new rooms! diff --git a/testing/matrix-sdk-integration-testing/src/tests/e2ee.rs b/testing/matrix-sdk-integration-testing/src/tests/e2ee.rs index 1e3f47b83..88ab4c05d 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/e2ee.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/e2ee.rs @@ -31,9 +31,76 @@ use matrix_sdk::{ }; use similar_asserts::assert_eq; use tracing::{debug, warn}; - +use matrix_sdk::ruma::event_id; +use matrix_sdk_ui::notification_client::{NotificationClient, NotificationProcessSetup}; +use matrix_sdk_ui::sync_service::SyncService; use crate::helpers::{SyncTokenAwareClient, TestClientBuilder}; +// I'm pretty ure I'm getting the verification request as an UTD +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn test_mutual_sas_verification_with_notification_client_ignores_verification_events() -> Result<()> { + let encryption_settings = + EncryptionSettings { auto_enable_cross_signing: true, ..Default::default() }; + let alice = SyncTokenAwareClient::new( + TestClientBuilder::new("alice") + .use_sqlite() + .encryption_settings(encryption_settings) + .build() + .await?, + ); + let bob_inner_client = TestClientBuilder::new("bob") + .use_sqlite() + .encryption_settings(encryption_settings) + .build() + .await?; + let bob = SyncTokenAwareClient::new( + bob_inner_client.clone(), + ); + + warn!("alice's device: {}", alice.device_id().unwrap()); + warn!("bob's device: {}", bob.device_id().unwrap()); + + let invite = vec![bob.user_id().unwrap().to_owned()]; + let request = assign!(CreateRoomRequest::new(), { + invite, + is_direct: true, + }); + + let alice_room = alice.create_room(request).await?; + alice_room.enable_encryption().await?; + let room_id = alice_room.room_id(); + + warn!("alice has created and enabled encryption in the room"); + + bob.sync_once().await?; + bob.get_room(room_id).unwrap().join().await?; + + alice.sync_once().await?; + bob.sync_once().await?; + + warn!("alice and bob are both aware of each other in the e2ee room"); + + let alice_bob_identity = alice + .encryption() + .get_user_identity(bob.user_id().unwrap()) + .await? + .expect("alice knows bob's identity"); + + warn!("alice has found bob's identity"); + + let alice_verification_request = alice_bob_identity.request_verification().await?; + + let sync_service = Arc::new(SyncService::builder(bob_inner_client.clone()).build().await.expect("Wat")); + let notification_client = NotificationClient::new(bob_inner_client.clone(), NotificationProcessSetup::SingleProcess { sync_service }).await.expect("couldn't create notification client"); + let _ = notification_client.get_notification_with_sliding_sync(room_id, event_id!("$1")).await; + + let verification = bob_inner_client.encryption().get_verification_request(alice_verification_request.own_user_id(), alice_verification_request.flow_id()).await; + // This should fail if the ignore_verification_events parameter is true + assert!(verification.is_none()); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_mutual_sas_verification() -> Result<()> { let encryption_settings =