diff --git a/apps/desktop/__tests__/step-up.test.ts b/apps/desktop/__tests__/step-up.test.ts index 0e87c0c..d7b99a9 100644 --- a/apps/desktop/__tests__/step-up.test.ts +++ b/apps/desktop/__tests__/step-up.test.ts @@ -13,6 +13,10 @@ describe('stepUpKind', () => { expect(stepUpKind('STEP_UP_REQUIRED_FRESH_AUTH')).toBe('fresh_auth'); }); + it('treats the bare code as a password demand', () => { + expect(stepUpKind('GraphQL Error: STEP_UP_REQUIRED')).toBe('password'); + }); + it('does not fire on any other failure', () => { expect(stepUpKind('no GraphQL endpoint at http://x/graphql')).toBeNull(); expect(stepUpKind('invalid email or password')).toBeNull(); diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index a8379df..dfb8970 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -229,6 +229,14 @@ export const registerIpc = (service: VaultService): void => { await accounts().revokeApiKey(assertString(itemId), proof(stepUp)); service.scheduleSave(); }); + handle(CHANNELS.accountsLinkTotp, async (accountItemId: string, totpItemId: string) => { + await accounts().linkTotp(assertString(accountItemId), assertString(totpItemId)); + service.scheduleSave(); + }); + handle(CHANNELS.accountsUnlinkTotp, async (accountItemId: string) => { + await accounts().unlinkTotp(assertString(accountItemId)); + service.scheduleSave(); + }); // ─── audit ─── handle(CHANNELS.auditLog, (itemId?: string) => service.current().auditLog(itemId)); diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 778a71e..2d269e0 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -63,6 +63,9 @@ const api: DcryptApi & { invoke(CHANNELS.accountsCreateKey, accountItemId, request, stepUp), revealKey: (itemId) => invoke(CHANNELS.accountsRevealKey, itemId), revokeKey: (itemId, stepUp) => invoke(CHANNELS.accountsRevokeKey, itemId, stepUp), + linkTotp: (accountItemId, totpItemId) => + invoke(CHANNELS.accountsLinkTotp, accountItemId, totpItemId), + unlinkTotp: (accountItemId) => invoke(CHANNELS.accountsUnlinkTotp, accountItemId), }, audit: { log: (itemId) => invoke(CHANNELS.auditLog, itemId), diff --git a/apps/desktop/src/renderer/src/screens/AccountsScreen.tsx b/apps/desktop/src/renderer/src/screens/AccountsScreen.tsx index b9228fe..43f40ae 100644 --- a/apps/desktop/src/renderer/src/screens/AccountsScreen.tsx +++ b/apps/desktop/src/renderer/src/screens/AccountsScreen.tsx @@ -18,11 +18,25 @@ import { import { Input } from '@constructive-io/ui/input'; import { Label } from '@constructive-io/ui/label'; import { Separator } from '@constructive-io/ui/separator'; -import { Copy, KeyRound, LogIn, LogOut, Plus, Trash2, UserPlus } from 'lucide-react'; +import { + Copy, + KeyRound, + LogIn, + LogOut, + Plus, + Timer, + Trash2, + UserPlus, +} from 'lucide-react'; import { useCallback, useEffect, useState } from 'react'; import { toast } from 'sonner'; -import type { AccountRecord, ApiKeyRecord, StepUpProof } from '../../../shared/api'; +import type { + AccountRecord, + ApiKeyRecord, + StepUpProof, + TotpEntry, +} from '../../../shared/api'; import { StepUpKind, stepUpKind, @@ -60,14 +74,19 @@ export const AccountsScreen = () => { const [held, setHeld] = useState(null); const [proofValue, setProofValue] = useState(''); + const [codes, setCodes] = useState([]); + const [linkFor, setLinkFor] = useState(null); + const refresh = useCallback(async () => { try { - const [nextAccounts, nextKeys] = await Promise.all([ + const [nextAccounts, nextKeys, nextCodes] = await Promise.all([ dcrypt.accounts.list(), dcrypt.accounts.keys(), + dcrypt.totp.list(), ]); setAccounts(nextAccounts); setKeys(nextKeys); + setCodes(nextCodes); } catch { // vault locked mid-refresh } @@ -230,8 +249,33 @@ export const AccountsScreen = () => { > Forget + + {account.totpItemId && ( +

+ MFA challenges are answered with{' '} + {codes.find((entry) => entry.item.id === account.totpItemId)?.item + .title ?? 'a code in this vault'} + , so you are not asked for one. +

+ )} + {accountKeys.length > 0 && } {accountKeys.map((key) => (
@@ -371,6 +415,50 @@ export const AccountsScreen = () => { + !open && setLinkFor(null)}> + + + Link a one-time code + + When the server asks this account for MFA, dcrypt answers with the code + you pick here. Only the item is remembered — the seed stays where it is. + + + + {codes.length === 0 && ( +

+ This vault holds no one-time codes yet. +

+ )} + {codes.map((entry) => ( + + ))} + + + + + + + !open && setHeld(null)}> @@ -404,9 +492,7 @@ export const AccountsScreen = () => { diff --git a/apps/desktop/src/shared/api.ts b/apps/desktop/src/shared/api.ts index 22e02e6..f06c0b3 100644 --- a/apps/desktop/src/shared/api.ts +++ b/apps/desktop/src/shared/api.ts @@ -157,6 +157,9 @@ export interface DcryptApi { /** Reads the secret back out of the vault, on demand only. */ revealKey(itemId: string): Promise; revokeKey(itemId: string, stepUp?: StepUpProof): Promise; + /** Point an account at a vault code that then answers its MFA step-ups. */ + linkTotp(accountItemId: string, totpItemId: string): Promise; + unlinkTotp(accountItemId: string): Promise; }; audit: { log(itemId?: string): Promise; @@ -228,6 +231,8 @@ export const CHANNELS = { accountsCreateKey: 'accounts:create-key', accountsRevealKey: 'accounts:reveal-key', accountsRevokeKey: 'accounts:revoke-key', + accountsLinkTotp: 'accounts:link-totp', + accountsUnlinkTotp: 'accounts:unlink-totp', auditLog: 'audit:log', wbCreateWallet: 'workbench:create-wallet', wbDeriveAccounts: 'workbench:derive-accounts', diff --git a/apps/desktop/src/shared/step-up.ts b/apps/desktop/src/shared/step-up.ts index 4bba7cb..85229ce 100644 --- a/apps/desktop/src/shared/step-up.ts +++ b/apps/desktop/src/shared/step-up.ts @@ -5,12 +5,14 @@ export type StepUpKind = 'password' | 'mfa' | 'fresh_auth'; /** * An IPC rejection reaches the renderer as a string, so the only thing left to - * key off is the server's own `STEP_UP_REQUIRED_*` code, which survives being - * wrapped by Electron and by our own error types. + * key off is the server's own `STEP_UP_REQUIRED*` code, which survives being + * wrapped by Electron and by our own error types. The bare form, raised when a + * session has no recent password verification, asks for a password. */ export const stepUpKind = (message: string): StepUpKind | null => { - const found = /STEP_UP_REQUIRED_(PASSWORD|MFA|FRESH_AUTH)/.exec(message); - return found ? (found[1].toLowerCase() as StepUpKind) : null; + const found = /STEP_UP_REQUIRED(?:_(PASSWORD|MFA|FRESH_AUTH))?/.exec(message); + if (!found) return null; + return (found[1]?.toLowerCase() as StepUpKind) ?? 'password'; }; export const stepUpPrompt = ( diff --git a/packages/accounts/__tests__/accounts.test.ts b/packages/accounts/__tests__/accounts.test.ts index 26715cd..0675b05 100644 --- a/packages/accounts/__tests__/accounts.test.ts +++ b/packages/accounts/__tests__/accounts.test.ts @@ -11,6 +11,7 @@ import { CreateApiKeyOptions, hasExpired, StepUpKind, + stepUpKind, StepUpRequiredError, } from '../src'; @@ -61,7 +62,7 @@ class FakeServer { }, verifyTotp: async (code: string) => { this.calls.push({ operation: 'verifyTotp', token }); - if (code !== '123456') { + if (!/^\d{6}$/.test(code)) { throw new AuthError('verifyTotp', 'the code was not accepted'); } this.demandStepUp = null; @@ -120,6 +121,9 @@ beforeEach(async () => { for (const item of await vault.listItems({ kind: 'api_key' })) { await vault.deleteItemForever(item.id); } + for (const item of await vault.listItems({ kind: 'totp' })) { + await vault.deleteItemForever(item.id); + } server = new FakeServer(); accounts = new AccountManager(vault, { createClient: server.factory }); }); @@ -363,6 +367,108 @@ describe('step-up', () => { }); }); +describe('stepUpKind', () => { + it('reads the factor the server named', () => { + expect(stepUpKind('STEP_UP_REQUIRED_PASSWORD')).toBe('password'); + expect(stepUpKind('STEP_UP_REQUIRED_MFA')).toBe('mfa'); + expect(stepUpKind('STEP_UP_REQUIRED_FRESH_AUTH')).toBe('fresh_auth'); + }); + + it('treats the bare code as a password demand', () => { + expect(stepUpKind('GraphQL Error: STEP_UP_REQUIRED')).toBe('password'); + }); + + it('stays out of the way of every other failure', () => { + expect(stepUpKind('invalid email or password')).toBeNull(); + }); +}); + +describe('linked one-time codes', () => { + const signIn = () => + accounts.signIn({ + endpoint: ENDPOINT, + email: 'dev@example.com', + password: 'hunter22', + }); + + /** A vault code item, the same shape the app's importer writes. */ + const addCode = async (title: string): Promise => { + const item = await vault.createItem('totp', title); + await vault.setField(item.id, 'seed', 'totp_seed', 'JBSWY3DPEHPK3PXP'); + return item.id; + }; + + it('answers an MFA demand from the linked item, unprompted', async () => { + const account = await signIn(); + await accounts.linkTotp(account.itemId, await addCode('Constructive dev')); + expect(await accounts.stepUpCode(account.itemId)).toMatch(/^\d{6}$/); + + server.demandStepUp = 'mfa'; + const key = await accounts.createApiKey(account.itemId, { name: 'ci' }); + + expect(key.name).toBe('ci'); + expect(server.calls.map((call) => call.operation)).toEqual([ + 'signIn', + 'createApiKey', + 'verifyTotp', + 'createApiKey', + ]); + }); + + it('still asks for the password when that is what was demanded', async () => { + const account = await signIn(); + await accounts.linkTotp(account.itemId, await addCode('Constructive dev')); + server.demandStepUp = 'password'; + + await expect(accounts.createApiKey(account.itemId, { name: 'ci' })).rejects.toThrow( + StepUpRequiredError + ); + expect(server.calls.some((call) => call.operation === 'verifyTotp')).toBe(false); + }); + + it('stores the item id, not a copy of the seed', async () => { + const account = await signIn(); + const totpItemId = await addCode('Constructive dev'); + await accounts.linkTotp(account.itemId, totpItemId); + + const fields = await vault.listFields(account.itemId); + const link = fields.find((field) => field.name === 'totp_item_id'); + expect(link).toBeDefined(); + expect(await vault.revealField(account.itemId, 'totp_item_id')).toBe(totpItemId); + expect(fields.some((field) => field.purpose === 'totp_seed')).toBe(false); + expect((await accounts.getAccount(account.itemId)).totpItemId).toBe(totpItemId); + }); + + it('refuses an item that carries no seed', async () => { + const account = await signIn(); + const empty = await vault.createItem('totp', 'nothing in here'); + + await expect(accounts.linkTotp(account.itemId, empty.id)).rejects.toThrow( + 'carries no code seed' + ); + }); + + it('has no code to offer until something is linked', async () => { + const account = await signIn(); + expect(await accounts.stepUpCode(account.itemId)).toBeNull(); + + const totpItemId = await addCode('Constructive dev'); + await accounts.linkTotp(account.itemId, totpItemId); + await accounts.unlinkTotp(account.itemId); + expect(await accounts.stepUpCode(account.itemId)).toBeNull(); + }); + + it('forgets a link whose code has left the vault', async () => { + const account = await signIn(); + const totpItemId = await addCode('Constructive dev'); + await accounts.linkTotp(account.itemId, totpItemId); + await vault.deleteItemForever(totpItemId); + + expect(await accounts.stepUpCode(account.itemId)).toBeNull(); + expect((await accounts.getAccount(account.itemId)).totpItemId).toBeNull(); + }); +}); + describe('hasExpired', () => { const now = new Date('2026-01-01T00:00:00.000Z'); diff --git a/packages/accounts/src/client.ts b/packages/accounts/src/client.ts index 1607d60..94830d2 100644 --- a/packages/accounts/src/client.ts +++ b/packages/accounts/src/client.ts @@ -113,16 +113,20 @@ const explain = (message: string, endpoint: string): string => { return `no GraphQL endpoint at ${endpoint} — ${suffix}`; }; -const STEP_UP = /STEP_UP_REQUIRED_(PASSWORD|MFA|FRESH_AUTH)/; +const STEP_UP = /STEP_UP_REQUIRED(?:_(PASSWORD|MFA|FRESH_AUTH))?/; /** * Which factor a message is asking for, or null. Exported because a step-up * error arrives at a UI as plain text once it has crossed a process boundary, * and the server's own code is the only trustworthy thing to key off. + * + * The bare `STEP_UP_REQUIRED` raised by the generated guard means the session + * has no recent password verification, so it asks for a password. */ export const stepUpKind = (message: string): StepUpKind | null => { const found = STEP_UP.exec(message); - return found ? (found[1].toLowerCase() as StepUpKind) : null; + if (!found) return null; + return (found[1]?.toLowerCase() as StepUpKind) ?? 'password'; }; const rethrow = (operation: string, endpoint: string, error: unknown): never => { diff --git a/packages/accounts/src/manager.ts b/packages/accounts/src/manager.ts index 00d6b05..d981586 100644 --- a/packages/accounts/src/manager.ts +++ b/packages/accounts/src/manager.ts @@ -23,6 +23,7 @@ const ACCOUNT_FIELDS = { userId: 'user_id', accessToken: 'access_token', expiresAt: 'access_token_expires_at', + totpItem: 'totp_item_id', } as const; const KEY_FIELDS = { @@ -135,6 +136,48 @@ export class AccountManager { await this.vault.deleteItemForever(itemId); } + // ─── one-time codes ─────────────────────────────────────────────────────── + + /** + * Point an account at a code already in the vault, so an MFA step-up can be + * answered without reaching for a phone. Only the item id is stored; the seed + * stays where it was, concealed, and is never copied. + */ + async linkTotp(accountItemId: string, totpItemId: string): Promise { + await this.requireItem(accountItemId, 'account'); + await this.requireItem(totpItemId, 'totp'); + const fields = await this.vault.listFields(totpItemId); + if (!fields.some((field) => field.purpose === 'totp_seed')) { + throw new AuthError('linkTotp', `item ${totpItemId} carries no code seed`); + } + await this.vault.setField( + accountItemId, + ACCOUNT_FIELDS.totpItem, + 'text', + totpItemId, + false + ); + } + + async unlinkTotp(accountItemId: string): Promise { + await this.vault.deleteField(accountItemId, ACCOUNT_FIELDS.totpItem); + } + + /** + * A current code for the account's linked item, or null when nothing is + * linked — the caller then has to ask a human for one. + */ + async stepUpCode(accountItemId: string): Promise { + const fields = await this.readFields(accountItemId); + const totpItemId = fields[ACCOUNT_FIELDS.totpItem]; + if (!totpItemId) return null; + if (!(await this.vault.getItem(totpItemId))) { + await this.unlinkTotp(accountItemId); + return null; + } + return this.vault.totpCode(totpItemId); + } + // ─── api keys ───────────────────────────────────────────────────────────── /** @@ -150,7 +193,7 @@ export class AccountManager { const account = await this.readFields(accountItemId); const token = await this.requireToken(accountItemId); const endpoint = account[ACCOUNT_FIELDS.endpoint]; - const created = await this.withStepUp(endpoint, token, proof, (client) => + const created = await this.withStepUp(accountItemId, endpoint, token, proof, (client) => client.createApiKey(options) ); @@ -203,11 +246,8 @@ export class AccountManager { const fields = await this.readFields(itemId); const accountItemId = fields[KEY_FIELDS.accountId]; const token = await this.requireToken(accountItemId); - await this.withStepUp( - fields[KEY_FIELDS.endpoint], - token, - proof, - (client) => client.revokeApiKey(fields[KEY_FIELDS.keyId]) + await this.withStepUp(accountItemId, fields[KEY_FIELDS.endpoint], token, proof, (client) => + client.revokeApiKey(fields[KEY_FIELDS.keyId]) ); await this.vault.deleteItemForever(itemId); } @@ -219,9 +259,11 @@ export class AccountManager { * factor, prove it and run *the same* operation once more. The request is * held rather than rebuilt, so nothing about it can change between the two * attempts. Without a proof the `StepUpRequiredError` propagates, which is - * what lets a caller collect one and try again. + * what lets a caller collect one and try again — except for a code demand on + * an account with a linked code, which the vault answers by itself. */ private async withStepUp( + accountItemId: string, endpoint: string, token: string, proof: StepUpProof | undefined, @@ -233,14 +275,19 @@ export class AccountManager { } catch (error) { if (!(error instanceof StepUpRequiredError)) throw error; if (error.kind === 'mfa') { - if (!proof?.totpCode) throw error; - await client.verifyTotp(proof.totpCode); + const code = proof?.totpCode ?? (await this.stepUpCode(accountItemId)); + if (!code) throw error; + await client.verifyTotp(code); } else if (proof?.password) { await client.verifyPassword(proof.password); } else if (proof?.totpCode) { await client.verifyTotp(proof.totpCode); } else { - throw error; + // A password demand needs the password; only fresh auth takes a code. + const code = + error.kind === 'fresh_auth' ? await this.stepUpCode(accountItemId) : null; + if (!code) throw error; + await client.verifyTotp(code); } return run(client); } @@ -286,6 +333,7 @@ export class AccountManager { endpoint, email, userId: session.userId, + totpItemId: (await this.readFields(itemId))[ACCOUNT_FIELDS.totpItem] ?? null, accessTokenExpiresAt: session.accessTokenExpiresAt, signedIn: !hasExpired(session.accessTokenExpiresAt, this.now()), }; @@ -334,6 +382,7 @@ export class AccountManager { endpoint: fields[ACCOUNT_FIELDS.endpoint] ?? '', email: fields[ACCOUNT_FIELDS.email] ?? item.title, userId: fields[ACCOUNT_FIELDS.userId] ?? '', + totpItemId: fields[ACCOUNT_FIELDS.totpItem] ?? null, accessTokenExpiresAt: expiresAt, signedIn: Boolean(fields[ACCOUNT_FIELDS.accessToken]) && diff --git a/packages/accounts/src/types.ts b/packages/accounts/src/types.ts index a4cfca9..2b9ba30 100644 --- a/packages/accounts/src/types.ts +++ b/packages/accounts/src/types.ts @@ -6,6 +6,8 @@ export interface AccountRecord { endpoint: string; email: string; userId: string; + /** Vault item holding a one-time code that can answer an MFA step-up. */ + totpItemId: string | null; /** When the stored access token stops working, if the server said. */ accessTokenExpiresAt: string | null; /** False once signed out, or once the token has expired. */ diff --git a/packages/cli/src/commands/account.ts b/packages/cli/src/commands/account.ts index e83c1fb..6c4bb39 100644 --- a/packages/cli/src/commands/account.ts +++ b/packages/cli/src/commands/account.ts @@ -31,6 +31,8 @@ Subcommands: signin Sign in and store the session signout Sign out and drop the stored token forget Remove the account and its keys from the vault + link-code Answer this account's MFA step-ups with a vault code + unlink-code Stop answering its step-ups from the vault key list [email] List stored API keys key create Mint an API key for --account key reveal Print an API key secret @@ -50,6 +52,7 @@ Options: Examples: dcrypt account signin dev@example.com --endpoint http://auth.localhost:3000/graphql dcrypt account key create ci --account dev@example.com --expires-days 30 + dcrypt account link-code dev@example.com "Constructive dev" dcrypt account key reveal ci `; @@ -228,6 +231,50 @@ const forget = async (argv: ParsedArgs, prompter: Inquirerer): Promise => }); }; +/** + * Point an account at a one-time code already in the vault, so the server's + * MFA step-up is answered without a human reaching for a phone. + */ +const linkCode = async (argv: ParsedArgs, prompter: Inquirerer): Promise => { + const { first, newArgv } = takeFirst(argv); + const { first: codeRef, newArgv: rest } = takeFirst(newArgv); + if (!first) throw new CliError('an account is required'); + if (!codeRef) throw new CliError('a one-time code item is required'); + + await withVault(rest, prompter, async (accounts, vault) => { + const account = await findAccount(accounts, first); + const codes = await vault.listItems({ kind: 'totp' }); + const code = + codes.find((item) => item.id === codeRef) ?? + codes.find((item) => item.title === codeRef) ?? + codes.find((item) => item.title.toLowerCase() === codeRef.toLowerCase()); + if (!code) { + throw new CliError(`no one-time code "${codeRef}" in the vault`, EXIT.notFound); + } + + await accounts.linkTotp(account.itemId, code.id); + emit( + rest, + { itemId: account.itemId, totpItemId: code.id }, + () => `"${code.title}" will answer MFA for ${account.email}` + ); + }); +}; + +const unlinkCode = async (argv: ParsedArgs, prompter: Inquirerer): Promise => { + const { first, newArgv } = takeFirst(argv); + if (!first) throw new CliError('an account is required'); + await withVault(newArgv, prompter, async (accounts) => { + const account = await findAccount(accounts, first); + await accounts.unlinkTotp(account.itemId); + emit( + newArgv, + { itemId: account.itemId }, + () => `${account.email} will ask for a code again` + ); + }); +}; + const keyList = async (argv: ParsedArgs, prompter: Inquirerer): Promise => { const { first, newArgv } = takeFirst(argv); await withVault(newArgv, prompter, async (accounts) => { @@ -362,6 +409,8 @@ export const accountCommand = async ( signin, signout, forget, + 'link-code': linkCode, + 'unlink-code': unlinkCode, key: keyCommand, }, }); diff --git a/packages/vault/__tests__/vault.test.ts b/packages/vault/__tests__/vault.test.ts index 80182cb..002120d 100644 --- a/packages/vault/__tests__/vault.test.ts +++ b/packages/vault/__tests__/vault.test.ts @@ -140,8 +140,8 @@ describe('Vault', () => { await vault.tagItem(login.id, 'money'); await vault.setFavorite(login.id, true); const code = await vault.createItem('totp', 'Bank 2FA'); - await vault.setField(code.id, 'seed', 'totp_seed', 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ'); - const before = await vault.totpCode(code.id); + const seed = 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ'; + await vault.setField(code.id, 'seed', 'totp_seed', seed); // the rebuild asserts the fresh database really carries the deployed schema, // so this passing at all rules out a deploy that quietly did nothing @@ -157,7 +157,10 @@ describe('Vault', () => { expect((await vault.listTags(login.id)).map((tag) => tag.name)).toEqual(['money']); expect((await vault.getItem(login.id))!.favorite).toBe(true); expect((await vault.getItem(login.id))!.folderId).toBe(child.id); - expect(await vault.totpCode(code.id)).toBe(before); + // the seed, not the code it produces: codes read either side of the rebuild + // can straddle the 30s window and differ for reasons of clock, not of copy + expect(await vault.revealField(code.id, 'seed')).toBe(seed); + expect(await vault.totpCode(code.id)).toMatch(/^\d{6}$/); const folders = await vault.listFolders(); expect(folders.find((folder) => folder.id === child.id)!.parentId).toBe(parent.id);