ensure on device removal that the member state and the local user context are updated with the new rotated user keys generation

michaelkamphausen · HerbCaudill · commit 9e5324aed2eb · 2024-09-24T14:27:45.000+02:00
diff --git a/packages/auth/src/connection/test/authentication.test.ts b/packages/auth/src/connection/test/authentication.test.ts
@@ -17,6 +17,7 @@ import {
 } from 'util/testing/index.js'
 import { describe, expect, it } from 'vitest'
 import type { InviteeDeviceContext } from '../types.js'
+import { createDevice } from 'device/createDevice.js'
 
 describe('connection', () => {
   describe('authentication', () => {
@@ -248,7 +249,7 @@ describe('connection', () => {
 
         // Bob invites and admits his phone
 
-        const phone = bob.phone!
+        let phone = bob.phone!
 
         {
           const { seed } = bob.team.inviteDevice()
@@ -279,6 +280,12 @@ describe('connection', () => {
         {
           // Bob invites his phone again
 
+          // The phone needs a new deviceId, otherwise the connection will immediately
+          // fail with DEVICE_REMOVED error after being established. It also gets a new
+          // device key as the old one could be compromised.
+          phone = createDevice({ userId: bob.userId, deviceName: phone.deviceName })
+          bob.phone = phone
+
           const { seed } = bob.team.inviteDevice()
           const phoneContext: InviteeDeviceContext = {
             userName: bob.userName,
diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts
@@ -137,9 +137,12 @@ export class Team extends EventEmitter<TeamEvents> {
     }
 
     this.state = this.store.getState()
+    this.checkForNewUserKeysGeneration()
 
     // Wire up event listeners
     this.on('updated', () => {
+      this.checkForNewUserKeysGeneration()
+
       // If we're admin, check for pending key rotations
       this.checkForPendingKeyRotations()
     })
@@ -807,6 +810,15 @@ export class Team extends EventEmitter<TeamEvents> {
     if (isForServer) device.keys = newKeys // (a server plays the role of both a user and a device)
   }
 
+  private checkForNewUserKeysGeneration() {
+    const { user } = this.context
+    const latestUserKeys = this.allUserKeys().at(-1)
+
+    if (latestUserKeys && user.keys.generation < latestUserKeys.generation) {
+      user.keys = latestUserKeys
+    }
+  }
+
   private checkForPendingKeyRotations() {
     // Only admins can rotate keys
     if (!this.memberIsAdmin(this.userId)) {
diff --git a/packages/auth/src/team/reducer.ts b/packages/auth/src/team/reducer.ts
@@ -132,9 +132,9 @@ const getTransforms = (action: TeamAction): Transform[] => {
     }
 
     case 'REMOVE_DEVICE': {
-      const { deviceId } = action.payload
+      const { deviceId, lockboxes } = action.payload
       return [
-        removeDevice(deviceId), // Remove this device from the member's list of devices
+        removeDevice(deviceId, lockboxes), // Remove this device from the member's list of devices
       ]
     }
 
diff --git a/packages/auth/src/team/test/devices.test.ts b/packages/auth/src/team/test/devices.test.ts
@@ -130,6 +130,8 @@ describe('Team', () => {
 
       // Keys have never been rotated
       expect(alice.team.teamKeys().generation).toBe(0)
+      expect(bob.team.members(bob.userId)?.keys.generation).toBe(0)
+      expect(bob.user.keys.generation).toBe(0)
       const { secretKey } = alice.team.teamKeys()
 
       // Add bob's phone
@@ -139,6 +141,10 @@ describe('Team', () => {
       // Remove bob's phone
       bob.team.removeDevice(phone.deviceId)
 
+      // User keys have now been rotated once
+      expect(bob.team.members(bob.userId)?.keys.generation).toBe(1)
+      expect(bob.user.keys.generation).toBe(1)
+
       // Team keys have now been rotated once
       expect(bob.team.teamKeys().generation).toBe(1)
       expect(bob.team.teamKeys().secretKey).not.toBe(secretKey)
diff --git a/packages/auth/src/team/transforms/removeDevice.ts b/packages/auth/src/team/transforms/removeDevice.ts
@@ -1,18 +1,40 @@
+import { Keyset } from '@localfirst/crdx'
+import { Lockbox } from 'lockbox/index.js'
 import * as select from 'team/selectors/index.js'
 import { type Member, type Transform } from 'team/types.js'
+import { KeyType } from 'util/types.js'
 
 export const removeDevice =
-  (deviceId: string): Transform =>
+  (deviceId: string, lockboxes: Lockbox[] = []): Transform =>
   state => {
     const removedDevice = select.device(state, deviceId)
 
-    const { userId } = select.memberByDeviceId(state, deviceId)
+    const member = select.memberByDeviceId(state, deviceId)
+    const { userId } = member
+    let { keys } = member
+    const userLockbox = lockboxes.find(
+      ({ contents }) => contents.type == KeyType.USER && contents.name == userId
+    )
+
+    // When a device is removed, the user keys are rotated and the new key
+    // generation is in the lockboxes. The unencrypted lockbox contents
+    // contain the meta data along with the public keys which is all we need
+    // to update the user keys of the member to the latest generation.
+    if (userLockbox) {
+      const { type, name, generation, encryption, signature } =
+        userLockbox.contents as unknown as Keyset
+
+      if (keys.generation < generation && !!encryption && !!signature) {
+        keys = { type, name, generation, encryption, signature }
+      }
+    }
 
     const removeDeviceFromMember = (member: Member) =>
       member.userId === userId
         ? {
             ...member,
             devices: member.devices?.filter(d => d.deviceId !== removedDevice.deviceId),
+            keys,
           }
         : member
 

Original file line number	Diff line number	Diff line change
`@@ -132,9 +132,9 @@ const getTransforms = (action: TeamAction): Transform[] => {`
`132`	`132`	`}`
`133`	`133`
`134`	`134`	`case 'REMOVE_DEVICE': {`
`135`		`- const { deviceId } = action.payload`
	`135`	`+ const { deviceId, lockboxes } = action.payload`
`136`	`136`	`return [`
`137`		`- removeDevice(deviceId), // Remove this device from the member's list of devices`
	`137`	`+ removeDevice(deviceId, lockboxes), // Remove this device from the member's list of devices`
`138`	`138`	`]`
`139`	`139`	`}`
`140`	`140`