From 1aeb0010fc24e555a3167ee0904c7603b7ed13bb Mon Sep 17 00:00:00 2001 From: devswha Date: Thu, 5 Nov 2026 17:21:59 +0900 Subject: [PATCH] fix(account): end a row's other sessions when a link moves it to another key A session signed in with a row's old relay key kept drive access after the row was linked to the account key, until it reconnected. A link now ends the row's other sessions with auth_changed; the linking session goes on (with no member's rights) until it signs in again with the new key. Closes #2864 --- docs/account.md | 4 +++- server/account/account.contract.test.ts | 12 +++++++----- server/account/records.ts | 6 +++--- server/account/routes.ts | 2 +- server/devices.ts | 21 ++++++++++++++++++--- server/relay/session.ts | 5 ++++- 6 files changed, 36 insertions(+), 14 deletions(-) diff --git a/docs/account.md b/docs/account.md index 3695019118..014f618462 100644 --- a/docs/account.md +++ b/docs/account.md @@ -203,7 +203,9 @@ the relay too. Mutations need the app's mutation guard, like the other routes. `POST /api/account/link` `{ "record": }`: the request's own device must be the record's `device_id`, with `old_key` its current key, `server_id` this PC's. The row's key becomes `key` and it becomes a member, in one write. `old_key` equal to `key` makes a row whose key already is the account -key a member. Answers `{ "device": }`. +key a member. Answers `{ "device": }`. The row's other sessions end (`auth_changed`): +they signed in with a key it no longer has. The linking session goes on, with no member's rights, +until it signs in again with the new key. `POST /api/account/records` `{ "records": [, …] }`: at most 32. Applies each in order and answers every outcome, never failing the whole batch for one record: diff --git a/server/account/account.contract.test.ts b/server/account/account.contract.test.ts index 224aad5a0d..a6537df3fb 100644 --- a/server/account/account.contract.test.ts +++ b/server/account/account.contract.test.ts @@ -496,16 +496,16 @@ test("the device list shows account members' account, key and adder to a driving for (const row of await list(watcher)) for (const field of ["account", "account_key", "added_by"]) expect(row).not.toHaveProperty(field); }); -test("a session signed in with a row's key before it was linked acts as no member", async () => { +test("the session that linked a row, still on the row's former key, acts as no member", async () => { const f = fixture(); const legacy = await relayDevice(f); - const [first, second] = [await signedIn(f, legacy), await signedIn(f, legacy)]; + const first = await signedIn(f, legacy); const key = await accountKey(); expect((await post(first, "/api/account/link", { record: await linkRecord(legacy, key) })).status).toBe(200); const joining = await accountKey(); const fresh = await signedIn(f, { keys: key.keys, publicKey: key.spki, id: legacy.id }); await post(fresh, "/api/account/records", { records: [await add(key, joining.spki)] }); - expect((await post(second, "/api/account/introduce", { key: joining.spki, pcs: [introducedPc()] })).body.error?.code).toBe("account_required"); + expect((await post(first, "/api/account/introduce", { key: joining.spki, pcs: [introducedPc()] })).body.error?.code).toBe("account_required"); }); test("a device paired with a code, which sent its key but never signed with it, can neither read nor link as that key", async () => { @@ -543,13 +543,15 @@ test("a device paired with a code a relay device started on a PC in no account i expect(f.app.context.services.devices.hasKey(target.spki)).toBe(false); }); -test("a session from a row's former key cannot start a member's pairing code", async () => { +test("a link ends the row's other sessions from its former key, and keeps the one that linked", async () => { const f = fixture(); const legacy = await relayDevice(f); const [first, stale] = [await signedIn(f, legacy), await signedIn(f, legacy)]; const key = await accountKey(); expect((await post(first, "/api/account/link", { record: await linkRecord(legacy, key) })).status).toBe(200); - expect((await post(stale, "/api/devices/pair/start", {})).body.error?.code).toBe("account_required"); + expect(await bounded(stale.serverError)).toBe("auth_changed"); + // the linking session goes on (with the old key's rights until it signs in again) + expect((await first.open("/api/devices").response).status).toBe(200); }); test("a watching device added by a member reads its own introduction", async () => { diff --git a/server/account/records.ts b/server/account/records.ts index 3a07653894..a34b8c702a 100644 --- a/server/account/records.ts +++ b/server/account/records.ts @@ -59,8 +59,8 @@ export type LinkRefusal = RecordRefusal | "key_in_use"; * A `link` from the device `deviceId` over its own session to the PC `serverId`. It needs no * member: the session is its authority, which is how a PC gets its first one. */ -export async function applyLink(store: DeviceStore, value: unknown, session: { deviceId: string; serverId: string; sessionKey: string }): Promise<{ device: PairedDevice } | { refused: LinkRefusal }> { - const { deviceId, serverId, sessionKey } = session; +export async function applyLink(store: DeviceStore, value: unknown, session: { deviceId: string; serverId: string; sessionKey: string; sessionId?: string }): Promise<{ device: PairedDevice } | { refused: LinkRefusal }> { + const { deviceId, serverId, sessionKey, sessionId } = session; const record = await read(value); if ("refused" in record) return { refused: record.refused }; if (record.kind !== "link") return { refused: "record_invalid" }; @@ -72,6 +72,6 @@ export async function applyLink(store: DeviceStore, value: unknown, session: { d if (account !== null && account !== link.account) return { refused: "account_mismatch" }; if (store.isTombstoned(link.key)) return { refused: "tombstoned" }; if (store.keyHeldByOther(link.key, deviceId)) return { refused: "key_in_use" }; - const device = store.linkAccountKey(deviceId, link.old_key, link.key, link.account); + const device = store.linkAccountKey(deviceId, link.old_key, link.key, link.account, sessionId); return device ? { device } : { refused: "link_not_owner" }; } diff --git a/server/account/routes.ts b/server/account/routes.ts index e0521d745f..34e003a91a 100644 --- a/server/account/routes.ts +++ b/server/account/routes.ts @@ -51,7 +51,7 @@ export async function handleAccountRequest(request: Request, pathname: string, c setRequestRoute(pathname); // a link proves the key of the row whose session sends it, and only the relay carries a row's key if (peer.kind !== "relay") return fail("link_not_owner", "Link a device from its own connection through the relay", 403); - const linked = await applyLink(devices, body.record, { deviceId: peer.device.id, serverId: peer.serverId, sessionKey: peer.key }); + const linked = await applyLink(devices, body.record, { deviceId: peer.device.id, serverId: peer.serverId, sessionKey: peer.key, sessionId: peer.sessionId }); if ("refused" in linked) return fail(linked.refused, `The link was refused: ${linked.refused}`, LINK_STATUS[linked.refused]); return reply({ device: linked.device }); } diff --git a/server/devices.ts b/server/devices.ts index 38bd44af16..cffa3906fc 100644 --- a/server/devices.ts +++ b/server/devices.ts @@ -234,6 +234,8 @@ export class DeviceStore { private unconfirmedFirstPairs: UnconfirmedFirstPair[] = []; private readonly path: string; private readonly revocations = new Map void>>(); + /** Sessions of a row, told when a `link` moves it to another key (onLinked). */ + private readonly relinks = new Map void>>(); private readonly revokeAllListeners = new Set<() => void>(); private readonly pairings = new Set<(event: DevicePairedEvent, origin: readonly string[]) => void>(); private readonly removals = new Set<(event: DeviceRemovedEvent) => void>(); @@ -333,6 +335,17 @@ export class DeviceStore { this.unconfirmedFirstPairs = []; } + /** Hears of a `link` moving row `id` to another key, with the id of the session that sent it. */ + onLinked(id: string, listener: (sessionId: string) => void): () => void { + const listeners = this.relinks.get(id) ?? new Set<(sessionId: string) => void>(); + listeners.add(listener); + this.relinks.set(id, listeners); + return () => { + listeners.delete(listener); + if (listeners.size === 0) this.relinks.delete(id); + }; + } + /** Ends stale authority on revocation or role changes, never a label-only update. Called immediately for an already-revoked identity. */ onRevoke(id: string, close: () => void): () => void { if (!this.devices.some((device) => device.id === id)) { close(); return () => {}; } @@ -758,10 +771,11 @@ export class DeviceStore { } /** - * A `link` the device `deviceId` sent over its own session: its row takes `key` and joins `account` - * in one write. Its sessions stay: the device is the same, and a role change is what ends them. + * A `link` the device `deviceId` sent over its own session `sessionId`: its row takes `key` and joins + * `account` in one write. The row's other sessions signed in with a key it no longer has, so they end + * (`auth_changed`); the linking one goes on until it signs in again with the new key. */ - linkAccountKey(deviceId: string, oldKey: string, key: string, account: string): PairedDevice | null { + linkAccountKey(deviceId: string, oldKey: string, key: string, account: string, sessionId = ""): PairedDevice | null { this.assertAccountWritable(); const device = this.devices.find((d) => d.id === deviceId && d.relay_public_key === oldKey); if (!device) return null; @@ -769,6 +783,7 @@ export class DeviceStore { const next = this.devices.map((d) => d === device ? updated : d); this.save(next); this.devices = next; + for (const listener of [...this.relinks.get(deviceId) ?? []]) listener(sessionId); return this.list(null).find((d) => d.id === deviceId) ?? null; } diff --git a/server/relay/session.ts b/server/relay/session.ts index 00154791c7..c6e39e6373 100644 --- a/server/relay/session.ts +++ b/server/relay/session.ts @@ -377,7 +377,10 @@ export class RelaySession { const devices = this.options.services.devices; if (!this.mux.active) return; if (!device) { this.close("auth_failed"); return; } - this.unwatch = devices.onRevoke(device.id, () => this.close(devices.has(device.id) ? "auth_changed" : "auth_revoked")); + const unrevoke = devices.onRevoke(device.id, () => this.close(devices.has(device.id) ? "auth_changed" : "auth_revoked")); + // a link moves the row to another key: a session signed in with the one it had ends, all but the linking one + const unlinked = devices.onLinked(device.id, (sessionId) => { if (sessionId !== this.sessionId) this.close("auth_changed"); }); + this.unwatch = () => { unrevoke(); unlinked(); }; if (!devices.has(device.id) || !this.mux.active) { this.unwatch(); return; } if (devices.list(null).find((current) => current.id === device.id)?.role !== device.role) { this.close("auth_changed");