Fix trusted devices key rotation

playwright/tests/sso_trusted.spec.ts

@@ -29,14 +29,19 @@ export async function startTrusted(test: Test, page: Page) {
 
 export async function trustedUnlock(test: Test, page: Page) {
     await test.step('Unlock', async () => {
-        await page.getByRole('button', { name: users.user1.name, exact: true }).click();
-        await page.getByRole('menuitem', { name: 'Log out' }).click();
-
         await landing(test, page, users.user1, { noReset: true });
         await expect(page).toHaveTitle(/Vaults/);
     });
 }
 
+export async function logOutUnlockTrusted(test: Test, page: Page) {
+    await test.step('Log out', async () => {
+        await page.getByRole('button', { name: users.user1.name, exact: true }).click();
+        await page.getByRole('menuitem', { name: 'Log out' }).click();
+    });
+    await trustedUnlock(test, page);
+}
+
 test('Trusted', async ({ browser, page }) => {
     // No change to onboarding
     await logNewUser(test, page, users.user1);
@@ -61,7 +66,7 @@ test('Trusted', async ({ browser, page }) => {
             await utils.checkNotification(page, 'Device Trusted');
         });
 
-        await trustedUnlock(test, page);
+        await logOutUnlockTrusted(test, page);
     });
 
     const context2 = await browser.newContext();
@@ -90,6 +95,31 @@ test('Trusted', async ({ browser, page }) => {
             await utils.checkNotification(page2, 'Device Trusted');
         });
 
+        await logOutUnlockTrusted(test, page2);
+    });
+
+    let newPassword = "TotoNewPassword";
+    await test.step('Rotate key', async () => {
+        await page.getByRole('button', { name: users.user1.name }).click();
+        await page.getByRole('menuitem', { name: 'Account settings' }).click();
+        await page.getByRole('link', { name: 'Security' }).click();
+        await page.getByRole('link', { name: 'Master password' }).click();
+
+        await expect(page.getByRole('heading', { name: 'Change master password' })).toBeVisible();
+        await page.getByRole('textbox', { name: 'Current master password * (required)' }).fill(users.user1.password);
+        await page.getByRole('textbox', { name: 'New master password * (required)', exact: true }).fill(newPassword);
+        await page.getByRole('textbox', { name: 'Confirm new master password * (required)', exact: true }).fill(newPassword);
+        await page.getByRole('checkbox', { name: 'Check known data breaches for this password' }).uncheck();
+        await page.getByRole('checkbox', { name: "Also rotate my account's encryption key" }).check();
+        await page.getByRole('button', { name: 'Yes' }).click();
+        await page.getByRole('button', { name: 'Change master password' }).click();
+
+        await expect(page.getByRole('heading', { name: 'Log in', exact: true })).toBeVisible();
+        await expect(page2.getByRole('heading', { name: 'Log in', exact: true })).toBeVisible();
+    });
+
+    await test.step('Still trusted', async () => {
+        await trustedUnlock(test, page);
         await trustedUnlock(test, page2);
     });
 
@@ -97,7 +127,7 @@ test('Trusted', async ({ browser, page }) => {
         await page.getByRole('link', { name: 'Settings' }).click();
         await page.getByRole('button', { name: 'Deauthorise sessions' }).click();;
         await expect(page.getByRole('heading', { name: 'Deauthorise sessions' })).toBeVisible();
-        await page.getByRole('textbox', { name: 'Master password * (required)', exact: true }).fill(users.user1.password);
+        await page.getByRole('textbox', { name: 'Master password * (required)', exact: true }).fill(newPassword);
         await page.getByRole('button', { name: 'Deauthorise sessions' }).click();
     });
 

src/api/core/accounts.rs

@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 
 use chrono::Utc;
 use rocket::{
@@ -68,6 +68,7 @@ pub fn routes() -> Vec<rocket::Route> {
         get_device,
         post_device_lost_trust,
         post_device_token,
+        post_devices_untrust,
         put_device_token,
         put_clear_device_token,
         post_clear_device_token,
@@ -834,6 +835,9 @@ struct RotateAccountUnlockData {
     emergency_access_unlock_data: Vec<UpdateEmergencyAccessData>,
     master_password_unlock_data: MasterPasswordUnlockData,
     organization_account_recovery_unlock_data: Vec<UpdateResetPasswordData>,
+
+    #[serde(default)]
+    device_key_unlock_data: Vec<UpdateDeviceKeysData>,
 }
 
 #[derive(Deserialize)]
@@ -848,6 +852,14 @@ struct MasterPasswordUnlockData {
     master_key_encrypted_user_key: String,
 }
 
+#[derive(Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct UpdateDeviceKeysData {
+    device_id: DeviceId,
+    encrypted_user_key: String,
+    encrypted_public_key: String,
+}
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct RotateAccountKeys {
@@ -863,9 +875,11 @@ struct RotateAccountData {
     sends: Vec<SendData>,
 }
 
+#[allow(clippy::too_many_arguments)]
 fn validate_keydata(
     data: &KeyData,
     existing_ciphers: &[Cipher],
+    existing_devices: &[Device],
     existing_folders: &[Folder],
     existing_emergency_access: &[EmergencyAccess],
     existing_memberships: &[Membership],
@@ -931,6 +945,15 @@ fn validate_keydata(
         err!("All existing reset password keys must be included in the rotation")
     }
 
+    // Check that we're correctly rotating all the user's trusted device
+    let existing_trusted_device_ids =
+        existing_devices.iter().filter(|d| d.is_trusted()).map(|c| &c.uuid).collect::<HashSet<&DeviceId>>();
+    let provided_device_ids =
+        data.account_unlock_data.device_key_unlock_data.iter().map(|rp| &rp.device_id).collect::<HashSet<&DeviceId>>();
+    if !provided_device_ids.is_superset(&existing_trusted_device_ids) {
+        err!("All existing trusted device keys must be included in the rotation")
+    }
+
     // Check that we're correctly rotating all the user's sends
     let existing_send_ids = existing_sends.iter().map(|s| &s.uuid).collect::<HashSet<&SendId>>();
     let provided_send_ids = data.account_data.sends.iter().filter_map(|s| s.id.as_ref()).collect::<HashSet<&SendId>>();
@@ -961,6 +984,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, conn: DbConn, nt:
     // TODO: Ideally we'd do everything after this point in a single transaction.
 
     let mut existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await;
+    let existing_devices = Device::find_by_user(user_id, &conn).await;
     let mut existing_folders = Folder::find_by_user(user_id, &conn).await;
     let mut existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await;
     let mut existing_memberships = Membership::find_by_user(user_id, &conn).await;
@@ -971,6 +995,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, conn: DbConn, nt:
     validate_keydata(
         &data,
         &existing_ciphers,
+        &existing_devices,
         &existing_folders,
         &existing_emergency_access,
         &existing_memberships,
@@ -1016,6 +1041,20 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, conn: DbConn, nt:
         membership.save(&conn).await?;
     }
 
+    // Update device
+    let mut device_updates: HashMap<DeviceId, UpdateDeviceKeysData> =
+        data.account_unlock_data.device_key_unlock_data.into_iter().map(|d| (d.device_id.clone(), d)).collect();
+    for mut device in existing_devices {
+        if let Some(update) = device_updates.remove(&device.uuid) {
+            device.encrypted_public_key = Some(update.encrypted_public_key);
+            device.encrypted_user_key = Some(update.encrypted_user_key);
+            device.save(true, &conn).await?;
+        } else if device.is_trusted() {
+            device.lost_trust();
+            device.save(true, &conn).await?;
+        }
+    }
+
     // Update send data
     for send_data in data.account_data.sends {
         let Some(send) = send_data.id.as_ref().and_then(|id| existing_sends.iter_mut().find(|s| &s.uuid == id)) else {
@@ -1568,15 +1607,40 @@ async fn get_all_devices(headers: Headers, conn: DbConn) -> JsonResult {
 }
 
 #[post("/devices/lost-trust")]
-async fn post_device_lost_trust(headers: Headers, conn: DbConn) -> JsonResult {
+async fn post_device_lost_trust(headers: Headers, conn: DbConn) -> EmptyResult {
     let mut device = headers.device;
 
-    device.encrypted_user_key = None;
-    device.encrypted_public_key = None;
-    device.encrypted_private_key = None;
+    if device.lost_trust() {
+        device.save(true, &conn).await?;
+    }
+
+    Ok(())
+}
+
+#[derive(Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct DevicesUntrusted {
+    devices: Vec<DeviceId>,
+}
+
+#[post("/devices/untrust", data = "<data>")]
+async fn post_devices_untrust(headers: Headers, data: Json<DevicesUntrusted>, conn: DbConn) -> EmptyResult {
+    let untrusted = data.into_inner();
+    if !untrusted.devices.is_empty() {
+        for device_id in untrusted.devices {
+            warn!("Cannot find user {} device {} to untrust", device_id, headers.user.email);
+
+            if let Some(mut device) = Device::find_by_uuid_and_user(&device_id, &headers.user.uuid, &conn).await {
+                if device.lost_trust() {
                     device.save(true, &conn).await?;
+                }
+            } else {
+                warn!("Cannot find user {} device {} to untrust", device_id, headers.user.email);
+            }
+        }
+    }
 
-    Ok(Json(json!({})))
+    Ok(())
 }
 
 #[get("/devices/identifier/<device_id>")]

src/db/models/device.rs

@@ -77,6 +77,14 @@ impl Device {
         self.encrypted_user_key.is_some() && self.encrypted_public_key.is_some() && self.encrypted_private_key.is_some()
     }
 
+    pub fn lost_trust(&mut self) -> bool {
+        let lost = self.is_trusted();
+        self.encrypted_private_key = None;
+        self.encrypted_public_key = None;
+        self.encrypted_user_key = None;
+        lost
+    }
+
     pub fn to_json(&self) -> Value {
         json!({
             "id": self.uuid,