diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 08401adec76..10a48dd8974 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -19,6 +19,12 @@ All notable changes to this project will be documented in this file. to network mode after a cache miss. ([#5930](https://github.com/matrix-org/matrix-rust-sdk/pull/5930)) +### Bug Fixes + +- Fix the redecryption of events in timelines built using the + `TimelineFocus` of `PinnedEvents`, `Thread`, `Event`. + ([#5955](https://github.com/matrix-org/matrix-rust-sdk/pull/5955)) + ## [0.16.0] - 2025-12-04 ### Features diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs index 3e67cedc169..5c257d69e2d 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs @@ -210,7 +210,7 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> { let event_id = deserialized.event_id().to_owned(); let txn_id = deserialized.transaction_id().map(ToOwned::to_owned); - if let Some(action @ TimelineAction::HandleAggregation { .. }) = TimelineAction::from_event( + let timeline_action = TimelineAction::from_event( deserialized, raw_event, room_data_provider, @@ -219,31 +219,52 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> { None, None, ) - .await - { - let encryption_info = event.kind.encryption_info().cloned(); + .await; - let sender_profile = room_data_provider.profile_from_user_id(&sender).await; + match timeline_action { + Some(action @ TimelineAction::AddItem { .. }) + | Some(action @ TimelineAction::HandleAggregation { .. }) => { + let encryption_info = event.kind.encryption_info().cloned(); + let sender_profile = room_data_provider.profile_from_user_id(&sender).await; + let mut ctx = TimelineEventContext { + sender, + sender_profile, + timestamp, + // These are not used when handling an aggregation. + read_receipts: Default::default(), + is_highlighted: false, + flow: Flow::Remote { + event_id: event_id.clone(), + raw_event: event.raw().clone(), + encryption_info, + txn_id, + position, + }, + // This field is not used when handling an aggregation. + should_add_new_items: false, + }; - let ctx = TimelineEventContext { - sender, - sender_profile, - timestamp, - // These are not used when handling an aggregation. - read_receipts: Default::default(), - is_highlighted: false, - flow: Flow::Remote { - event_id: event_id.clone(), - raw_event: event.raw().clone(), - encryption_info, - txn_id, - position, - }, - // This field is not used when handling an aggregation. - should_add_new_items: false, - }; + // FIXME: Continuation of the hackjob to get UTDs for focused timelines + // working from `handle_remote_aggregations()`. + if let TimelineAction::AddItem { .. } = action + && let TimelineItemPosition::UpdateAt { timeline_item_index } = position + && let Some(event) = self.items.get(timeline_item_index) + && event + .as_event() + .map(|e| { + e.content().is_unable_to_decrypt() && e.event_id() == Some(&event_id) + }) + .unwrap_or_default() + { + // Except when this is an UTD transitioning into a decrypted event. + ctx.should_add_new_items = true; + } - TimelineEventHandler::new(self, ctx).handle_event(date_divider_adjuster, action).await; + TimelineEventHandler::new(self, ctx) + .handle_event(date_divider_adjuster, action) + .await; + } + None => {} } } @@ -321,6 +342,40 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> { &mut date_divider_adjuster, ) .await; + } else if let Some(event_id) = event.event_id() + && let Some(meta) = + self.items.all_remote_events().get_by_event_id(&event_id) + && let Some(timeline_item_index) = meta.timeline_item_index + { + // FIXME: This branch is a complete hackjob. + // + // The reason being is that this branch is here to handle UTD -> Decrypted + // event remplacements for focused timelines. But this transition should + // naturally happen the same way it happens for unfocused timelines. + // + // Why it doesn't work here? Because the event cache fires out a + // VectorDiff::Set with an index that matches to the cache's view of the + // timeline, which is unfiltered, while the focused timeline will only show + // i.e. pinned events. + // + // The `test_pinned_events_are_decrypted_after_recovering` integration test + // showcases this. The event cache fires out the `Set` with an index of 7, + // but the timeline with the PinnedEvents focus has only 4 items. + // + // This hackjob continues in the `handle_remote_aggregation()` method as we + // can't just handle any `TimelineAction::AddItem` due to: + // https://github.com/matrix-org/matrix-rust-sdk/pull/4645 + // + // Doing so breaks the `test_new_pinned_events_are_not_added_on_sync` test. + // + // Relevant issue: https://github.com/matrix-org/matrix-rust-sdk/issues/5954. + self.handle_remote_aggregation( + event, + TimelineItemPosition::UpdateAt { timeline_item_index }, + room_data_provider, + &mut date_divider_adjuster, + ) + .await; } else { warn!( event_index, diff --git a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs index 7582733cf86..7d1c0f338c4 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs @@ -25,13 +25,15 @@ use matrix_sdk::{ Client, Room, RoomState, assert_next_with_timeout, config::SyncSettings, deserialized_responses::{VerificationLevel, VerificationState}, - encryption::{EncryptionSettings, backups::BackupState}, + encryption::{ + BackupDownloadStrategy, EncryptionSettings, backups::BackupState, recovery::RecoveryState, + }, room::{ edit::EditedContent, reply::{EnforceThread, Reply}, }, ruma::{ - MilliSecondsSinceUnixEpoch, OwnedEventId, RoomId, UserId, + MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, RoomId, UserId, api::client::room::create_room::v3::{Request as CreateRoomRequest, RoomPreset}, events::{ InitialStateEvent, @@ -45,7 +47,7 @@ use matrix_sdk::{ }, }, }; -use matrix_sdk_test::TestResult; +use matrix_sdk_test::{TestError, TestResult}; use matrix_sdk_ui::{ Timeline, notification_client::NotificationClient, @@ -65,7 +67,7 @@ use tokio::{ }; use tracing::{debug, warn}; -use crate::helpers::TestClientBuilder; +use crate::helpers::{TestClientBuilder, wait_for_room}; /// Checks that there a timeline update, and returns the EventTimelineItem. /// @@ -345,8 +347,7 @@ async fn test_stale_local_echo_time_abort_edit() { async fn test_enabling_backups_retries_decryption() { let encryption_settings = EncryptionSettings { auto_enable_backups: true, - backup_download_strategy: - matrix_sdk::encryption::BackupDownloadStrategy::AfterDecryptionFailure, + backup_download_strategy: BackupDownloadStrategy::AfterDecryptionFailure, ..Default::default() }; let alice = TestClientBuilder::new("alice") @@ -1081,3 +1082,188 @@ async fn test_local_echo_to_send_event_has_encryption_info() -> TestResult { Ok(()) } + +async fn prepare_room_with_pinned_events( + alice: &Client, + recovery_passphrase: &str, + number_of_normal_events: usize, +) -> Result<(OwnedRoomId, OwnedEventId), TestError> { + let sync_service = SyncService::builder(alice.clone()).build().await?; + sync_service.start().await; + + alice.encryption().wait_for_e2ee_initialization_tasks().await; + alice.encryption().recovery().enable().with_passphrase(recovery_passphrase).await?; + + debug!("Creating room…"); + let room = alice + .create_room(assign!(CreateRoomRequest::new(), { + is_direct: true, + initial_state: vec![], + preset: Some(RoomPreset::PrivateChat) + })) + .await?; + + room.enable_encryption().await?; + + // Send an event to the encrypted room and pin it. + let room_id = room.room_id().to_owned(); + let result = + room.send(RoomMessageEventContent::text_plain("It's a secret to everybody")).await?; + let event_id = result.response.event_id; + + let timeline = room.timeline().await?; + timeline.pin_event(&event_id).await?; + + // Now send a bunch of normal events, this ensures that our pinned event isn't + // in the main timeline when we restore things. + for i in 0..number_of_normal_events { + room.send(RoomMessageEventContent::text_plain(format!("Normal event {i}"))).await?; + } + + sync_service.stop().await; + + Ok((room_id, event_id)) +} + +async fn test_pinned_events_are_decrypted_after_recovering_with_event_count( + event_count: usize, +) -> TestResult { + const RECOVERY_PASSPHRASE: &str = "I am error"; + + let encryption_settings = EncryptionSettings { + auto_enable_cross_signing: true, + auto_enable_backups: true, + backup_download_strategy: BackupDownloadStrategy::AfterDecryptionFailure, + }; + + // Set up sync for user Alice, and create a room. + let alice = TestClientBuilder::new("alice") + .encryption_settings(encryption_settings) + .use_sqlite() + .build() + .await?; + let user_id = alice.user_id().expect("We should have a user ID by now"); + + let (room_id, event_id) = + prepare_room_with_pinned_events(&alice, RECOVERY_PASSPHRASE, event_count).await?; + + // Now `another_alice` comes into play. + let another_alice = TestClientBuilder::with_exact_username(user_id.localpart().to_owned()) + .encryption_settings(encryption_settings) + .use_sqlite() + .build() + .await?; + + // Alright, we're done with the original Alice. + drop(alice); + + // No rooms as of yet, we have not synced with the server as of yet. + assert!(another_alice.rooms().is_empty()); + another_alice.event_cache().subscribe()?; + + let sync_service = SyncService::builder(another_alice.clone()).build().await?; + // We need to subscribe to the room, otherwise we won't request the + // `m.room.pinned_events` stat event. + // + // Additionally if we subscribe to the room after we already synced, we'll won't + // receive the event, likely due to a Synapse bug. + sync_service.room_list_service().subscribe_to_rooms(&[&room_id]).await; + sync_service.start().await; + another_alice.encryption().wait_for_e2ee_initialization_tasks().await; + + // Let's get the room. + let room = wait_for_room(&another_alice, &room_id).await; + + assert!(room.latest_encryption_state().await?.is_encrypted(), "The room should be encrypted"); + + // Let's see if the pinned event is there. + let pinned_events = room.load_pinned_events().await?.unwrap_or_default(); + assert!( + pinned_events.contains(&event_id), + "The pinned event should be found in the pinned events state event" + ); + + // Let's see if the event is there, it should be a UTD. + let event = room.event(&event_id, Default::default()).await?; + assert!(event.kind.is_utd()); + + // Alright, let's now get to the timeline with a PinnedEvents focus. + let pinned_timeline = room + .timeline_builder() + .with_focus(TimelineFocus::PinnedEvents { + max_events_to_load: 100, + max_concurrent_requests: 10, + }) + .build() + .await?; + + let (items, mut stream) = pinned_timeline.subscribe_filter_map(|i| i.as_event().cloned()).await; + + // If we don't have any items as of yet, wait on the stream. + if items.is_empty() { + let _ = assert_next_with_timeout!(stream, 5000); + } + + // Alright, let's get the event from the timeline. + let item = pinned_timeline + .item_by_event_id(&event_id) + .await + .expect("We should have access to the pinned event"); + + // Still a UTD. + assert!( + item.content().is_unable_to_decrypt(), + "The pinned event should be a UTD as we didn't recover yet" + ); + assert_eq!(pinned_timeline.items().await.len(), 2); + + // Let's now recover. + another_alice.encryption().recovery().recover(RECOVERY_PASSPHRASE).await?; + assert_eq!(another_alice.encryption().recovery().state(), RecoveryState::Enabled); + + // The next update for the timeline should replace the UTD item with a decrypted + // value. + let next_item = assert_next_with_timeout!(stream, 5000); + + assert_let!(VectorDiff::Set { index: 0, value } = next_item); + let content = value.content(); + + // And we're not a UTD anymore. + assert!(!content.is_unable_to_decrypt()); + let message = content.as_message().expect("The pinned event should be a message"); + assert_eq!(message.body(), "It's a secret to everybody"); + + // And we check that we don't have any more items in the timeline, the UTD item + // was indeed replaced. + let items = pinned_timeline.items().await; + assert_eq!(items.len(), 2); + + Ok(()) +} + +/// Test that pinned UTD events, once decrypted by R2D2 (the redecryptor), get +/// replaced in the timeline with the decrypted variant. +/// +/// We do this by first creating the pinned events on one Client, called +/// `alice`. Then another client object is created, called `another_alice`. +/// `another_alice` initially doesn't have access to the room history. +/// +/// Only once `another_alice` recovers things and gets access to the backup can +/// she download the room key to decrypt the pinned event. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn test_pinned_events_are_decrypted_after_recovering_with_event_in_timeline() -> TestResult { + test_pinned_events_are_decrypted_after_recovering_with_event_count(0).await +} + +/// Test that pinned UTD events, once decrypted by R2D2 (the redecryptor), get +/// replaced in the timeline with the decrypted variant even if the pinened UTD +/// event isn't part of the main timeline and thus wasn't put into the event +/// cache by the main timeline backpaginating. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +// FIXME: This test is ignored because R2D2 can't decrypt this pinned event as it's never put into +// the event cache. +#[ignore] +async fn test_pinned_events_are_decrypted_after_recovering_with_event_not_in_timeline() -> TestResult +{ + test_pinned_events_are_decrypted_after_recovering_with_event_count(30).await +}