[identity] Verify new flow migration device list update

Summary:
Address [[ https://linear.app/comm/issue/ENG-9694/oldnew-flow-based-verification-for-updatedevicelist-rpc | ENG-9694 ]]

> - For old flows: Should allow reordering primary device under the following conditions:
>   - Previous device list was unsigned.
>   - Reorder only makes the calling device primary (only one swap, no more changes).
> - Otherwise, if previous device list was signed, it should act as before

Depends on D14087

Test Plan:
- Commtest
- Manual testing during development of device list migration handler

Reviewers: kamil

Reviewed By: kamil

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D14088

services/commtest/tests/identity_device_list_tests.rs

@@ -97,7 +97,10 @@ async fn test_device_list_rotation() {
 #[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 @@ async fn test_update_device_list_rpc() {
   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 @@ async fn test_device_list_signatures() {
   .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 @@ async fn test_device_list_signatures() {
     )
   };
 
-  // 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 @@ async fn test_device_list_signatures() {
 
   let expected_devices_lists: Vec<DeviceListHistoryItem> = vec![
     (None, vec![primary_device_id.to_string()]), // auto-generated during registration
+    migration_update,
     first_update,
     second_update,
   ];
@@ -555,3 +586,19 @@ async fn get_raw_device_list_history(
     .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
+}

services/identity/src/device_list.rs

@@ -382,6 +382,22 @@ pub mod validation {
     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::*;

services/identity/src/grpc_services/authenticated.rs

@@ -920,13 +920,58 @@ impl IdentityClientService for AuthenticatedService {
       )
       .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 {}))