Page MenuHomePhorge
Files F32559931

D10404.1767290061.diff
No OneTemporary
Actions

Size
6 KB
Referenced Files
None
Subscribers
None

D10404.1767290061.diff
View Options

diff --git a/services/identity/src/database/device_list.rs b/services/identity/src/database/device_list.rs
--- a/services/identity/src/database/device_list.rs
+++ b/services/identity/src/database/device_list.rs
@@ -718,7 +718,7 @@
) -> Result<(), Error> {
let user_id: String = user_id.into();
self
- .transact_update_devicelist(&user_id, |ref mut device_ids| {
+ .transact_update_devicelist(&user_id, |device_ids, mut devices_data| {
let new_device = DeviceRow::from_device_key_upload(
&user_id,
device_key_upload,
@@ -737,6 +737,10 @@
}
device_ids.push(new_device.device_id.clone());
+ // Reorder devices (determine primary device again)
+ devices_data.push(new_device.clone());
+ migration::reorder_device_list(&user_id, device_ids, &devices_data);
+
// Put new device
let put_device = Put::builder()
.table_name(devices_table::NAME)
@@ -765,7 +769,7 @@
let user_id: String = user_id.into();
let device_id = device_id.as_ref();
self
- .transact_update_devicelist(&user_id, |ref mut device_ids| {
+ .transact_update_devicelist(&user_id, |device_ids, mut devices_data| {
let device_exists = device_ids.iter().any(|id| id == device_id);
if !device_exists {
warn!(
@@ -778,6 +782,10 @@
device_ids.retain(|id| id != device_id);
+ // Reorder devices (determine primary device again)
+ devices_data.retain(|d| d.device_id != device_id);
+ migration::reorder_device_list(&user_id, device_ids, &devices_data);
+
// Delete device DDB operation
let delete_device = Delete::builder()
.table_name(devices_table::NAME)
@@ -810,10 +818,14 @@
// The closure performing a transactional update of the device list. It receives a mutable
// reference to the current device list. The closure should return a transactional DDB
// operation to be performed when updating the device list.
- action: impl FnOnce(&mut Vec<String>) -> Result<TransactWriteItem, Error>,
+ action: impl FnOnce(
+ &mut Vec<String>,
+ Vec<DeviceRow>,
+ ) -> Result<TransactWriteItem, Error>,
) -> Result<(), Error> {
let previous_timestamp =
get_current_devicelist_timestamp(self, user_id).await?;
+ let current_devices_data = self.get_current_devices(user_id).await?;
let mut device_ids = self
.get_current_device_list(user_id)
.await?
@@ -821,7 +833,7 @@
.unwrap_or_default();
// Perform the update action, then generate new device list
- let operation = action(&mut device_ids)?;
+ let operation = action(&mut device_ids, current_devices_data)?;
let new_device_list = DeviceListRow::new(user_id, device_ids);
// Update timestamp in users table
@@ -1032,9 +1044,80 @@
// of managing the device list.
mod migration {
use std::cmp::Ordering;
+ use tracing::{debug, error, info};
use super::*;
+ pub(super) fn reorder_device_list(
+ user_id: &str,
+ list: &mut [String],
+ devices_data: &[DeviceRow],
+ ) {
+ if !validate_device_list_match(list, devices_data) {
+ error!("Found corrupt device list for user (userID={})!", user_id);
+ return;
+ }
+
+ let Some(first_device) = list.first() else {
+ debug!("Skipping device list rotation. Nothing to reorder.");
+ return;
+ };
+ let Some(primary_device) = determine_primary_device(devices_data) else {
+ info!(
+ "No valid primary device found for user (userID={}).\
+ Skipping device list reorder.",
+ user_id
+ );
+ return;
+ };
+
+ if first_device == &primary_device.device_id {
+ debug!("Skipping device list reorder. Primary device is already first");
+ return;
+ }
+
+ // swap primary device with the first one
+ let Some(primary_device_idx) = list
+ .iter()
+ .position(|id| id == &primary_device.device_id) else {
+ error!(
+ "Primary device not found in device list (userID={})",
+ user_id
+ );
+ return;
+ };
+ list.swap(0, primary_device_idx);
+ info!("Reordered device list for user (userID={})", user_id);
+ }
+
+ // checks if device list matches given devices data
+ fn validate_device_list_match(
+ list: &[String],
+ devices_data: &[DeviceRow],
+ ) -> bool {
+ if list.len() != devices_data.len() {
+ error!("Device list length mismatch!");
+ return false;
+ }
+
+ let devices_map = devices_data
+ .iter()
+ .map(|device| (&device.device_id, device))
+ .collect::<HashMap<_, _>>();
+
+ for device_id in list {
+ if !devices_map.contains_key(device_id) {
+ error!(
+ "Device list contains unknown device (deviceID={})",
+ device_id
+ );
+ return false;
+ }
+ }
+
+ true
+ }
+
/// Returns reference to primary device (if any) from given list of devices
/// or None if there's no valid primary device.
fn determine_primary_device(devices: &[DeviceRow]) -> Option<&DeviceRow> {
@@ -1065,6 +1148,47 @@
use super::*;
use chrono::Duration;
+ #[test]
+ fn reorder_skis_no_devices() {
+ let mut list = vec![];
+ reorder_device_list("", &mut list, &[]);
+ assert_eq!(list, Vec::<String>::new());
+ }
+
+ #[test]
+ fn reorder_skips_single_device() {
+ let mut list = vec!["test".into()];
+ let devices_data =
+ vec![create_test_device("test", DeviceType::Web, 0, Utc::now())];
+
+ reorder_device_list("", &mut list, &devices_data);
+ assert_eq!(list, vec!["test"]);
+ }
+
+ #[test]
+ fn reorder_skips_for_valid_list() {
+ let mut list = vec!["mobile".into(), "web".into()];
+ let devices_data = vec![
+ create_test_device("mobile", DeviceType::Android, 1, Utc::now()),
+ create_test_device("web", DeviceType::Web, 0, Utc::now()),
+ ];
+
+ reorder_device_list("", &mut list, &devices_data);
+ assert_eq!(list, vec!["mobile", "web"]);
+ }
+
+ #[test]
+ fn reorder_swaps_primary_device_when_possible() {
+ let mut list = vec!["web".into(), "mobile".into()];
+ let devices_data = vec![
+ create_test_device("web", DeviceType::Web, 0, Utc::now()),
+ create_test_device("mobile", DeviceType::Android, 1, Utc::now()),
+ ];
+
+ reorder_device_list("", &mut list, &devices_data);
+ assert_eq!(list, vec!["mobile", "web"]);
+ }
+
#[test]
fn determine_primary_device_returns_none_for_empty_list() {
let devices = vec![];

File Metadata

Mime Type
text/plain
Expires
Thu, Jan 1, 5:54 PM (14 h, 38 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5877037
Default Alt Text
D10404.1767290061.diff (6 KB)

Event Timeline