diff --git a/services/commtest/tests/identity_device_list_tests.rs b/services/commtest/tests/identity_device_list_tests.rs --- a/services/commtest/tests/identity_device_list_tests.rs +++ b/services/commtest/tests/identity_device_list_tests.rs @@ -97,7 +97,10 @@ #[tokio::test] async fn test_update_device_list_rpc() { // Register user with primary device - let primary_device = register_user_device(None, None).await; + let mut primary_account = MockOlmAccount::new(); + let primary_device_keys = primary_account.public_keys(); + let primary_device = + register_user_device(Some(&primary_device_keys), None).await; let mut auth_client = get_auth_client( &service_addr::IDENTITY_GRPC.to_string(), primary_device.user_id.clone(), @@ -120,6 +123,14 @@ assert!(initial_device_list.len() == 1, "Expected single device"); let primary_device_id = initial_device_list[0].clone(); + // migrate to signed device lists + migrate_device_list( + &mut auth_client, + &initial_device_list, + &mut primary_account, + ) + .await; + // perform update by adding a new device let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); let devices_payload = vec![primary_device_id, "device2".to_string()]; @@ -170,13 +181,32 @@ .await .expect("Couldn't connect to identity service"); - // Perform unsigned update (add a new device) + // perform a migration to signed device list + let migration_update: DeviceListHistoryItem = { + let latest_device_list = &[primary_device_id.to_string()]; + let updated_device_list = migrate_device_list( + &mut auth_client, + latest_device_list, + &mut primary_account, + ) + .await; + + ( + updated_device_list.cur_primary_signature.clone(), + updated_device_list.into_raw().devices, + ) + }; + + // Perform first update (add a new device) let first_update: DeviceListHistoryItem = { - let update_payload = - SignedDeviceList::from_raw_unsigned(&RawDeviceList::new(vec![ + let update_payload = SignedDeviceList::create_signed( + &RawDeviceList::new(vec![ primary_device_id.to_string(), "device2".to_string(), - ])); + ]), + &mut primary_account, + None, + ); let update_request = UpdateDeviceListRequest::from(&update_payload); auth_client .update_device_list(update_request) @@ -189,7 +219,7 @@ ) }; - // now perform a update (remove a device), but sign the device list + // now perform another update (remove a device) let second_update: DeviceListHistoryItem = { let update_payload = SignedDeviceList::create_signed( &RawDeviceList::new(vec![primary_device_id.to_string()]), @@ -238,6 +268,7 @@ let expected_devices_lists: Vec = vec![ (None, vec![primary_device_id.to_string()]), // auto-generated during registration + migration_update, first_update, second_update, ]; @@ -555,3 +586,19 @@ .map(|signed| signed.into_raw()) .collect() } + +async fn migrate_device_list( + client: &mut ChainedInterceptedAuthClient, + last_device_list: &[String], + signing_account: &mut MockOlmAccount, +) -> SignedDeviceList { + let raw_list = RawDeviceList::new(Vec::from(last_device_list)); + let signed_list = + SignedDeviceList::create_signed(&raw_list, signing_account, None); + client + .update_device_list(UpdateDeviceListRequest::from(&signed_list)) + .await + .expect("Failed to perform signed device list migration"); + + signed_list +} diff --git a/services/identity/src/device_list.rs b/services/identity/src/device_list.rs --- a/services/identity/src/device_list.rs +++ b/services/identity/src/device_list.rs @@ -382,6 +382,22 @@ is_added != is_removed } + pub fn new_flow_migration_validator( + previous_device_list: &[&str], + new_device_list: &[&str], + calling_device_id: &str, + ) -> bool { + // new primary must be the calling device + if new_device_list.first() != Some(&calling_device_id) { + return false; + } + + // no device added or removed, only reorder allowed + let previous_set: HashSet<_> = previous_device_list.iter().collect(); + let new_set: HashSet<_> = new_device_list.iter().collect(); + previous_set == new_set + } + #[cfg(test)] mod tests { use super::*; diff --git a/services/identity/src/grpc_services/authenticated.rs b/services/identity/src/grpc_services/authenticated.rs --- a/services/identity/src/grpc_services/authenticated.rs +++ b/services/identity/src/grpc_services/authenticated.rs @@ -920,13 +920,58 @@ ) .await?; + let is_new_flow_user = self + .db_client + .get_user_login_flow(&user_id) + .await? + .is_signed_device_list_flow(); + let new_list = SignedDeviceList::try_from(request.into_inner())?; let update = DeviceListUpdate::try_from(new_list)?; - let validator = - crate::device_list::validation::update_device_list_rpc_validator; + + let validator = if is_new_flow_user { + // regular device list update + Some(crate::device_list::validation::update_device_list_rpc_validator) + } else { + // new flow migration + let Some(current_device_list) = + self.db_client.get_current_device_list(&user_id).await? + else { + tracing::warn!("User {} does not have valid device list. New flow migration impossible.", redact_sensitive_data(&user_id)); + return Err(tonic::Status::aborted( + tonic_status_messages::DEVICE_LIST_ERROR, + )); + }; + + let calling_device_id = &device_id; + let previous_device_ids: Vec<&str> = current_device_list + .device_ids + .iter() + .map(AsRef::as_ref) + .collect(); + let new_device_ids: Vec<&str> = + update.devices.iter().map(AsRef::as_ref).collect(); + + let is_valid = + crate::device_list::validation::new_flow_migration_validator( + &previous_device_ids, + &new_device_ids, + calling_device_id, + ); + if !is_valid { + return Err( + crate::error::Error::DeviceList( + crate::error::DeviceListError::InvalidDeviceListUpdate, + ) + .into(), + ); + } + // we've already validated it, no further validator required + None + }; self .db_client - .apply_devicelist_update(&user_id, update, Some(validator), true) + .apply_devicelist_update(&user_id, update, validator, true) .await?; Ok(Response::new(Empty {}))