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 @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; use std::str::FromStr; +use std::time::{SystemTime, UNIX_EPOCH}; use commtest::identity::device::{ login_user_device, logout_user_device, register_user_device, DEVICE_TYPE, @@ -125,9 +126,10 @@ let primary_device_id = initial_device_list[0].clone(); // perform update by adding a new device + let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); let raw_update_payload = json!({ "devices": [primary_device_id, "device2"], - "timestamp": 123456789, + "timestamp": now.as_millis(), }); let update_payload = json!({ "rawDeviceList": serde_json::to_string(&raw_update_payload).unwrap(), diff --git a/services/identity/src/constants.rs b/services/identity/src/constants.rs --- a/services/identity/src/constants.rs +++ b/services/identity/src/constants.rs @@ -160,6 +160,10 @@ pub const NONCE_LENGTH: usize = 17; pub const NONCE_TTL_DURATION: Duration = Duration::from_secs(120); // seconds +// Device list + +pub const DEVICE_LIST_TIMESTAMP_VALID_FOR: Duration = Duration::from_secs(300); + // Workflows in progress pub const WORKFLOWS_IN_PROGRESS_TTL_DURATION: Duration = 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 @@ -114,11 +114,15 @@ impl DeviceListRow { /// Generates new device list row from given devices - fn new(user_id: impl Into, device_ids: Vec) -> Self { + fn new( + user_id: impl Into, + device_ids: Vec, + timestamp: Option>, + ) -> Self { Self { user_id: user_id.into(), device_ids, - timestamp: Utc::now(), + timestamp: timestamp.unwrap_or_else(Utc::now), } } } @@ -778,7 +782,7 @@ let put_device_operation = TransactWriteItem::builder().put(put_device).build(); - Ok(Some(put_device_operation)) + Ok((Some(put_device_operation), None)) }) .await?; @@ -829,7 +833,7 @@ let operation = TransactWriteItem::builder().delete(delete_device).build(); - Ok(Some(operation)) + Ok((Some(operation), None)) }) .await?; @@ -843,7 +847,8 @@ update: DeviceListUpdate, ) -> Result { let DeviceListUpdate { - devices: new_list, .. + devices: new_list, + timestamp, } = update; self .transact_update_devicelist(user_id, |current_list, _| { @@ -864,7 +869,7 @@ debug!("Applying device list update. Difference: {:?}", difference); *current_list = new_list; - Ok(None) + Ok((None, Some(timestamp))) }) .await } @@ -881,12 +886,17 @@ // It receives two arguments: // 1. A mutable reference to the current device list (ordered device IDs). // 2. Details (full data) of the current devices (unordered). - // The closure should return a transactional DDB - // operation to be performed when updating the device list. + // The closure should return a two-element tuple: + // - (optional) transactional DDB operation to be performed + // when updating the device list. + // - (optional) new device list timestamp. Defaults to `Utc::now()`. action: impl FnOnce( &mut Vec, Vec, - ) -> Result, Error>, + ) -> Result< + (Option, Option>), + Error, + >, ) -> Result { let previous_timestamp = get_current_devicelist_timestamp(self, user_id).await?; @@ -898,8 +908,13 @@ .unwrap_or_default(); // Perform the update action, then generate new device list - let operation = action(&mut device_ids, current_devices_data)?; - let new_device_list = DeviceListRow::new(user_id, device_ids); + let (operation, timestamp) = action(&mut device_ids, current_devices_data)?; + + crate::device_list::verify_device_list_timestamp( + previous_timestamp.as_ref(), + timestamp.as_ref(), + )?; + let new_device_list = DeviceListRow::new(user_id, device_ids, timestamp); // Update timestamp in users table let timestamp_update_operation = device_list_timestamp_update_operation( diff --git a/services/identity/src/device_list.rs b/services/identity/src/device_list.rs new file mode 100644 --- /dev/null +++ b/services/identity/src/device_list.rs @@ -0,0 +1,88 @@ +use chrono::{DateTime, Duration, Utc}; + +use crate::{ + constants::DEVICE_LIST_TIMESTAMP_VALID_FOR, error::DeviceListError, +}; + +/// Returns `true` if given timestamp is valid. The timestamp is considered +/// valid under the following condition: +/// - `new_timestamp` is greater than `previous_timestamp` (if provided) +/// - `new_timestamp` is not older than [`DEVICE_LIST_TIMESTAMP_VALID_FOR`] +/// +/// Note: For Identity-managed device lists, the timestamp can be `None`. +/// Verification is then skipped +fn is_new_timestamp_valid( + previous_timestamp: Option<&DateTime>, + new_timestamp: Option<&DateTime>, +) -> bool { + let Some(new_timestamp) = new_timestamp else { + return true; + }; + + if let Some(previous_timestamp) = previous_timestamp { + if new_timestamp < previous_timestamp { + return false; + } + } + + let timestamp_valid_duration = + Duration::from_std(DEVICE_LIST_TIMESTAMP_VALID_FOR) + .expect("FATAL - Invalid duration constant provided"); + + Utc::now().signed_duration_since(new_timestamp) < timestamp_valid_duration +} + +/// Returns error if new timestamp is invalid. The timestamp is considered +/// valid under the following condition: +/// - `new_timestamp` is greater than `previous_timestamp` (if provided) +/// - `new_timestamp` is not older than [`DEVICE_LIST_TIMESTAMP_VALID_FOR`] +/// +/// Note: For Identity-managed device lists, the timestamp can be `None`. +/// Verification is then skipped +pub fn verify_device_list_timestamp( + previous_timestamp: Option<&DateTime>, + new_timestamp: Option<&DateTime>, +) -> Result<(), DeviceListError> { + if !is_new_timestamp_valid(previous_timestamp, new_timestamp) { + return Err(DeviceListError::InvalidDeviceListUpdate); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_timestamp_validation() { + let valid_timestamp = Utc::now() - Duration::milliseconds(100); + let previous_timestamp = Utc::now() - Duration::seconds(10); + let too_old_timestamp = previous_timestamp - Duration::seconds(1); + let expired_timestamp = Utc::now() - Duration::minutes(20); + + assert!( + verify_device_list_timestamp( + Some(&previous_timestamp), + Some(&valid_timestamp) + ) + .is_ok(), + "Valid timestamp should pass verification" + ); + assert!( + verify_device_list_timestamp( + Some(&previous_timestamp), + Some(&too_old_timestamp) + ) + .is_err(), + "Timestamp older than previous, should fail verification" + ); + assert!( + verify_device_list_timestamp(None, Some(&expired_timestamp)).is_err(), + "Expired timestamp should fail verification" + ); + assert!( + verify_device_list_timestamp(None, None).is_ok(), + "No provided timestamp should pass" + ); + } +} diff --git a/services/identity/src/main.rs b/services/identity/src/main.rs --- a/services/identity/src/main.rs +++ b/services/identity/src/main.rs @@ -10,6 +10,7 @@ mod cors; mod database; pub mod ddb_utils; +mod device_list; pub mod error; mod grpc_services; mod grpc_utils;