Page MenuHomePhabricator
Files F3518876

D14088.id46457.diff
No OneTemporary
Actions

D14088.id46457.diff
View Options

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()];
@@ -189,7 +200,23 @@
)
};
- // now perform a update (remove a device), but sign the device list
+ // perform a migration to signed device list
+ let migration_update: DeviceListHistoryItem = {
+ let latest_device_list = &first_update.1;
+ 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,
+ )
+ };
+
+ // now perform a update (remove a device), but with device list signed
let second_update: DeviceListHistoryItem = {
let update_payload = SignedDeviceList::create_signed(
&RawDeviceList::new(vec![primary_device_id.to_string()]),
@@ -239,6 +266,7 @@
let expected_devices_lists: Vec<DeviceListHistoryItem> = vec![
(None, vec![primary_device_id.to_string()]), // auto-generated during registration
first_update,
+ migration_update,
second_update,
];
let actual_device_lists: Vec<DeviceListHistoryItem> = device_list_history
@@ -555,3 +583,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,27 @@
is_added != is_removed
}
+ pub fn new_flow_migration_validator(
+ previous_device_list: &[&str],
+ new_device_list: &[&str],
+ calling_device_id: &str,
+ ) -> bool {
+ // primary device must be changed
+ if !primary_device_changed(previous_device_list, new_device_list) {
+ return false;
+ }
+
+ // new primary must be the calling device
+ if new_device_list.first() != Some(&calling_device_id) {
+ return false;
+ }
+
+ // no device added or removed, just reorder
+ 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 {}))

File Metadata

Mime Type
text/plain
Expires
Mon, Dec 23, 8:56 PM (19 h, 4 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2696441
Default Alt Text
D14088.id46457.diff (5 KB)

Event Timeline