From 8d612eca462c29a873fc7a2af8365bdcce472f02 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Sun, 26 Jan 2025 22:14:55 +0000 Subject: [PATCH] crypto: break up split_devices_for_user handle the separate flags with separate methods. --- .../group_sessions/share_strategy.rs | 97 ++++++++++++++++--- 1 file changed, 85 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index ce954e559..4b952d120 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -353,8 +353,10 @@ fn is_session_overshared_for_user( should_rotate } -/// Result type for [`split_devices_for_user_for_identity_based_strategy`] and -/// [`split_devices_for_user`]. +/// Result type for [`split_devices_for_user_for_all_devices_strategy`], +/// [`split_devices_for_user_for_error_on_verified_user_problem_strategy`], +/// [`split_devices_for_user_for_identity_based_strategy`], +/// [`split_devices_for_user_for_only_trusted_devices`]. /// /// A partitioning of the devices for a given user. #[derive(Default)] @@ -398,6 +400,60 @@ fn split_devices_for_user( device_owner_identity: &Option, only_allow_trusted_devices: bool, error_on_verified_user_problem: bool, +) -> DeviceBasedRecipientDevices { + if only_allow_trusted_devices { + let recipient_devices = split_devices_for_user_for_only_trusted_devices( + user_devices, + own_identity, + device_owner_identity, + ); + DeviceBasedRecipientDevices::Devices(recipient_devices) + } else if error_on_verified_user_problem { + split_devices_for_user_for_error_on_verified_user_problem_strategy( + user_devices, + own_identity, + device_owner_identity, + ) + } else { + let recipient_devices = split_devices_for_user_for_all_devices_strategy(user_devices); + DeviceBasedRecipientDevices::Devices(recipient_devices) + } +} + +/// Partition the list of a user's devices according to whether they should +/// receive the key, for [`CollectStrategy::DeviceBasedStrategy`] with +/// neither `error_on_verified_user_problem` nor `only_allow_trusted_devices`. +fn split_devices_for_user_for_all_devices_strategy( + user_devices: HashMap, +) -> RecipientDevicesForUser { + let (left, right) = user_devices.into_values().partition_map(|d| { + if d.is_blacklisted() { + Either::Right((d, WithheldCode::Blacklisted)) + } else { + Either::Left(d) + } + }); + + RecipientDevicesForUser { allowed_devices: left, denied_devices_with_code: right } +} + +/// Partition the list of a user's devices according to whether they should +/// receive the key, for [`CollectStrategy::DeviceBasedStrategy`] with +/// `error_on_verified_user_problem` (but not `only_allow_trusted_devices`) +/// +/// This function returns one of two values: +/// +/// * A list of the devices that should cause the transmission to fail due to +/// being unsigned. In this case, we don't bother to return the rest of the +/// devices, because we assume transmission will fail. +/// +/// * Otherwise, returns a [`RecipientDevicesForUser`] which lists, separately, +/// the devices that should receive the room key, and those that should +/// receive a withheld code. +fn split_devices_for_user_for_error_on_verified_user_problem_strategy( + user_devices: HashMap, + own_identity: &Option, + device_owner_identity: &Option, ) -> DeviceBasedRecipientDevices { let mut recipient_devices = RecipientDevicesForUser::default(); @@ -411,16 +467,11 @@ fn split_devices_for_user( } else if d.local_trust_state() == LocalTrust::Ignored { // Ignore the trust state of that device and share recipient_devices.allowed_devices.push(d); - } else if only_allow_trusted_devices && !d.is_verified(own_identity, device_owner_identity) - { - recipient_devices.denied_devices_with_code.push((d, WithheldCode::Unverified)); - } else if error_on_verified_user_problem - && is_unsigned_device_of_verified_user( - own_identity.as_ref(), - device_owner_identity.as_ref(), - &d, - ) - { + } else if is_unsigned_device_of_verified_user( + own_identity.as_ref(), + device_owner_identity.as_ref(), + &d, + ) { unsigned_devices_of_verified_users .get_or_insert_with(Vec::default) .push(d.device_id().to_owned()) @@ -472,6 +523,28 @@ fn split_devices_for_user_for_identity_based_strategy( } } +/// Partition the list of a user's devices according to whether they should +/// receive the key, for [`CollectStrategy::DeviceBasedStrategy`] with +/// `only_allow trusted_devices`. +fn split_devices_for_user_for_only_trusted_devices( + user_devices: HashMap, + own_identity: &Option, + device_owner_identity: &Option, +) -> RecipientDevicesForUser { + let (left, right) = user_devices.into_values().partition_map(|d| { + match ( + d.local_trust_state(), + d.is_cross_signing_trusted(own_identity, device_owner_identity), + ) { + (LocalTrust::BlackListed, _) => Either::Right((d, WithheldCode::Blacklisted)), + (LocalTrust::Ignored | LocalTrust::Verified, _) => Either::Left(d), + (LocalTrust::Unset, false) => Either::Right((d, WithheldCode::Unverified)), + (LocalTrust::Unset, true) => Either::Left(d), + } + }); + RecipientDevicesForUser { allowed_devices: left, denied_devices_with_code: right } +} + fn is_unsigned_device_of_verified_user( own_identity: Option<&OwnUserIdentityData>, device_owner_identity: Option<&UserIdentityData>,