diff --git a/scripts/dev-api.ts b/scripts/dev-api.ts index 6b54a3d..dc637bf 100644 --- a/scripts/dev-api.ts +++ b/scripts/dev-api.ts @@ -35,12 +35,15 @@ import { createServer } from 'node:http' import { createInboxApi } from '../src/api/index.js' +import { createPasswordAuthProvider } from '../src/auth/password-provider.js' +import type { AuthProvider } from '../src/auth/provider.js' import { createPgliteDb } from '../src/db/client.js' import { migrate } from '../src/db/migrate.js' import { createDevEmailSender } from '../src/dev/dev-sender.js' import { createHttpBridge } from '../src/dev/http-adapter.js' import { seedDevData } from '../src/dev/seed.js' import type { Keyring } from '../src/mail/reply-token.js' +import { createAgentStore } from '../src/store/agents.js' import { createConversationStore } from '../src/store/conversations.js' const PORT = Number(process.env.HT_DEV_PORT ?? 8787) @@ -62,6 +65,13 @@ async function main(): Promise { const store = createConversationStore(db) const sender = createDevEmailSender() + // Agents & Authentication (HT-54) — core, required by createInboxApi. + // No HELPTHREAD_UI_BASE_URL in this harness (there is no web dev server + // wired up here), so uiBaseUrl stays absent: sendInvite still creates the + // Agent but inviteSent is always false, matching a fresh, UI-less deploy. + const agentStore = createAgentStore(db) + const authProviders: AuthProvider[] = [createPasswordAuthProvider({ agentStore })] + let seededCount: number | undefined if (DB_PATH === undefined) { const seeded = await seedDevData({ @@ -82,6 +92,7 @@ async function main(): Promise { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: { store: agentStore, providers: authProviders }, }) const baseUrl = `http://127.0.0.1:${PORT}` diff --git a/specs/auth/agents-and-auth.md b/specs/auth/agents-and-auth.md index 5383512..1cd739b 100644 --- a/specs/auth/agents-and-auth.md +++ b/specs/auth/agents-and-auth.md @@ -276,7 +276,16 @@ Auth / bootstrap: to me", gates admin controls) and treats `401` as "log in again." Agents (management): -- **`GET /api/v1/agents`** (admin) → `Agent[]`. +Response envelopes (as built): a single Agent rides as `{ agent }` (`/setup`, +`/auth/verify`, `/auth/invite/accept`, `GET`/`PATCH /agents/{id}`), the roster as +`{ agents }`, and provider discovery as `{ providers, needsSetup }` — object envelopes +throughout, extensible without breaking clients, matching the wrapped shapes below. + +- **`GET /api/v1/agents`** (any active Agent) → `{ agents: Agent[] }`. *(Amended at build time, HT-54: + was admin-only in the draft, but the assignee UI — any Agent may assign any Agent, §5 — + needs the roster to render names and offer choices; an admin-only list would make a + non-admin's assignee menu impossible. The roster carries no secrets (no identities, no + hashes). Every mutation below remains admin-gated.)* - **`POST /api/v1/agents`** (admin) `{ name, email, role, sendInvite, password? }` → creates an Agent (§8 provisioning): with `sendInvite`, `status='invited'` and no password; with `password` (the admin-set fallback), a `password` identity and `status='active'` outright. @@ -495,6 +504,8 @@ is retired (§8). ## Changelog +- **draft.4 (2026-07-18, HT-54 build):** `GET /agents` opened to any active Agent (was + admin-only) — the assignee UI needs the roster; mutations stay admin-gated (§6). - **draft.3 (2026-07-18):** status is a closed lifecycle (CodeRabbit round 2): PATCH may only toggle `active`↔`disabled`; `invited` exits solely via invite acceptance (or delete/re-create); password writes on an `invited` Agent are refused (§6) — closing the diff --git a/src/api/acting-agent.ts b/src/api/acting-agent.ts new file mode 100644 index 0000000..9b899f9 --- /dev/null +++ b/src/api/acting-agent.ts @@ -0,0 +1,48 @@ +/** + * Resolve the acting Agent from `X-Helpthread-Agent-Id` (HT-54; + * specs/auth/agents-and-auth.md §8) — the one place every handler that needs + * the acting Agent goes through, so the "load the row, re-check status" + * policy lives in exactly one function rather than being re-implemented per + * handler. + * + * The web derives this header ONLY from the verified session `sub`, never + * from client input (spec §5's guardrail) — the engine trusts it because the + * caller already holds the service Bearer token (`src/api/auth.ts`); this + * function's job is the engine-side half of that trust model: even a + * genuinely web-asserted header must be re-checked against the CURRENT row, + * since a signed session cookie can outlive an Agent being disabled or + * deleted (spec §8's "bounding a disabled Agent whose cookie is still + * valid" point — Edge middleware verifies the cookie but never touches the + * Agent store, so this engine-side check is the only place that can). + * + * `null` covers every failure uniformly (missing header, malformed/non-uuid + * value, no such Agent, or a non-`active` status) — callers map `null` to a + * generic `401`, never a more specific message that would leak which case + * applied. + */ + +import type { AgentRecord, AgentStore } from '../store/agents.js' +import { isUuid } from './uuid.js' + +/** The header the web asserts the session's verified `sub` under (spec §8). `Request.headers.get` is case-insensitive, so the exact casing here is cosmetic. */ +export const ACTING_AGENT_HEADER = 'X-Helpthread-Agent-Id' + +/** + * Resolve `request`'s acting Agent, or `null` if the header is absent, + * malformed, or names an Agent that is missing or not `status: 'active'` + * (an `invited` Agent is treated the same as `disabled` for acting + * purposes — spec: "only `active` can act"). Never throws. + */ +export async function resolveActingAgent( + request: Request, + store: AgentStore, +): Promise { + const header = request.headers.get(ACTING_AGENT_HEADER) + if (header === null) return null + const id = header.trim() + if (id.length === 0 || !isUuid(id)) return null + + const agent = await store.getAgent(id) + if (agent === null || agent.status !== 'active') return null + return agent +} diff --git a/src/api/agents.test.ts b/src/api/agents.test.ts new file mode 100644 index 0000000..6dc0766 --- /dev/null +++ b/src/api/agents.test.ts @@ -0,0 +1,1044 @@ +/** + * End-to-end tests for the Agents & Authentication API (HT-54; + * specs/auth/agents-and-auth.md §6) — driven through the real + * `createInboxApi` pipeline (`src/api/index.ts`), a real PGlite-backed + * `AgentStore`, and the real `password` `AuthProvider`, matching this + * codebase's convention of testing the API handlers via the full HTTP + * pipeline (`src/api/index.test.ts`) rather than calling handler functions + * directly. + */ + +import { afterEach, describe, expect, it } from 'vitest' +import { mintInviteToken } from '../auth/invite-token.js' +import { hashPassword } from '../auth/password-hash.js' +import { createPasswordAuthProvider } from '../auth/password-provider.js' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import type { Keyring } from '../mail/reply-token.js' +import type { EmailSender, OutboundEmail } from '../providers/index.js' +import { type AgentRecord, type AgentStore, createAgentStore } from '../store/agents.js' +import { createConversationStore } from '../store/conversations.js' +import { createInboxApi } from './index.js' + +const TOKEN = 'test-token-for-the-agents-and-auth-suite' +const MAIL_DOMAIN = 'mail.example.test' +const SUPPORT_ADDRESS = 'support@example.test' +const UI_BASE_URL = 'https://desk.example.test' +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const AGENT_HEADER = 'X-Helpthread-Agent-Id' + +/** A fake `EmailSender` that records every send and never fails. */ +function createFakeSender(): { sender: EmailSender; sent: OutboundEmail[] } { + const sent: OutboundEmail[] = [] + return { + sender: { + maxSendMs: 30_000, + async send(email) { + sent.push(email) + return {} + }, + }, + sent, + } +} + +/** An `EmailSender` that always rejects — for exercising `502 send_failed`. */ +function createThrowingSender(): EmailSender { + return { + maxSendMs: 30_000, + async send() { + throw new Error('provider rejected the message') + }, + } +} + +describe('Agents & Authentication API', () => { + let db: Db | undefined + + afterEach(async () => { + await db?.close() + db = undefined + }) + + async function freshApi(overrides: { uiBaseUrl?: string; sender?: EmailSender } = {}): Promise<{ + db: Db + agentStore: AgentStore + api: (request: Request) => Promise + sent: OutboundEmail[] + }> { + db = await createPgliteDb() + await migrate(db) + const agentStore = createAgentStore(db) + const { sender: defaultSender, sent } = createFakeSender() + const api = createInboxApi({ + store: createConversationStore(db), + apiToken: TOKEN, + sender: overrides.sender ?? defaultSender, + keyring: KEYRING, + mailDomain: MAIL_DOMAIN, + supportAddress: SUPPORT_ADDRESS, + agents: { + store: agentStore, + providers: [createPasswordAuthProvider({ agentStore })], + ...(overrides.uiBaseUrl !== undefined ? { uiBaseUrl: overrides.uiBaseUrl } : {}), + }, + }) + return { db, agentStore, api, sent } + } + + /** Build a `Request`, always Bearer-authenticated, optionally with an acting-Agent header and/or a JSON body. */ + function req( + method: string, + path: string, + opts: { agentId?: string; body?: unknown } = {}, + ): Request { + const headers: Record = { Authorization: `Bearer ${TOKEN}` } + if (opts.agentId !== undefined) headers[AGENT_HEADER] = opts.agentId + const init: RequestInit = { method, headers } + if (opts.body !== undefined) { + headers['Content-Type'] = 'application/json' + init.body = JSON.stringify(opts.body) + } + return new Request(`https://x.example.test${path}`, init) + } + + async function createActiveAgent( + agentStore: AgentStore, + overrides: { email?: string; role?: 'admin' | 'agent'; name?: string } = {}, + ): Promise { + const result = await agentStore.createAgent({ + name: overrides.name ?? 'Test Agent', + email: overrides.email ?? 'agent@example.test', + role: overrides.role ?? 'agent', + status: 'active', + passwordHash: 'scrypt$N=16384,r=8,p=1$c2FsdA$aGFzaA', + }) + if (!result.ok) throw new Error('expected ok') + return result.agent + } + + // --- GET /api/v1/auth/providers --------------------------------------------- + + describe('GET /auth/providers', () => { + it('reports the core password provider and needsSetup:true on a fresh deployment', async () => { + const { api } = await freshApi() + const res = await api(req('GET', '/api/v1/auth/providers')) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + providers: [{ key: 'password', label: expect.any(String), kind: 'credentials' }], + needsSetup: true, + }) + }) + + it('needsSetup:false once at least one Agent exists', async () => { + const { api, agentStore } = await freshApi() + await createActiveAgent(agentStore) + const res = await api(req('GET', '/api/v1/auth/providers')) + expect((await res.json()).needsSetup).toBe(false) + }) + + it('still requires the service Bearer token', async () => { + const { api } = await freshApi() + const res = await api(new Request('https://x.example.test/api/v1/auth/providers')) + expect(res.status).toBe(401) + }) + }) + + // --- POST /api/v1/setup ------------------------------------------------------ + + describe('POST /setup', () => { + it('creates the first admin, active, with a usable password', async () => { + const { api } = await freshApi() + const res = await api( + req('POST', '/api/v1/setup', { + body: { name: 'Ada Admin', email: 'ada@example.test', password: 'correct-horse-battery' }, + }), + ) + expect(res.status).toBe(201) + const body = (await res.json()) as { agent: { role: string; status: string; email: string } } + expect(body.agent.role).toBe('admin') + expect(body.agent.status).toBe('active') + expect(body.agent.email).toBe('ada@example.test') + + const verify = await api( + req('POST', '/api/v1/auth/verify', { + body: { + providerKey: 'password', + email: 'ada@example.test', + password: 'correct-horse-battery', + }, + }), + ) + expect(verify.status).toBe(200) + }) + + it('409s once an Agent already exists', async () => { + const { api, agentStore } = await freshApi() + await createActiveAgent(agentStore) + const res = await api( + req('POST', '/api/v1/setup', { + body: { name: 'Late Admin', email: 'late@example.test', password: 'another-password' }, + }), + ) + expect(res.status).toBe(409) + expect(await res.json()).toEqual({ error: { code: 'conflict', message: expect.any(String) } }) + }) + + it('400s on missing/invalid fields', async () => { + const { api } = await freshApi() + for (const body of [ + {}, + { name: '', email: 'a@example.test', password: 'password123' }, + { name: 'A', email: 'not-an-email', password: 'password123' }, + { name: 'A', email: 'a@example.test', password: 'short' }, + ]) { + const res = await api(req('POST', '/api/v1/setup', { body })) + expect(res.status).toBe(400) + } + }) + }) + + // --- POST /api/v1/auth/verify ------------------------------------------------ + + describe('POST /auth/verify', () => { + it('verifies the correct email + password, returning the Agent', async () => { + const { api, agentStore } = await freshApi() + const result = await agentStore.createAgent({ + name: 'Real Agent', + email: 'real@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + if (!result.ok) throw new Error('expected ok') + + const res = await api( + req('POST', '/api/v1/auth/verify', { + body: { + providerKey: 'password', + email: 'real@example.test', + password: 'correct-password', + }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { agent: { id: string } } + expect(body.agent.id).toBe(result.agent.id) + }) + + it('is uniformly 401 for unknown email, wrong password, invited, and disabled — same status/code/body shape', async () => { + const { api, agentStore } = await freshApi() + + const active = await agentStore.createAgent({ + name: 'Active', + email: 'active@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + if (!active.ok) throw new Error('expected ok') + const disabledAgent = await agentStore.createAgent({ + name: 'Disabled', + email: 'disabled@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + if (!disabledAgent.ok) throw new Error('expected ok') + await agentStore.updateAgent(disabledAgent.agent.id, { status: 'disabled' }) + await agentStore.createAgent({ + name: 'Invited', + email: 'invited@example.test', + role: 'agent', + status: 'invited', + }) + + const attempts = [ + { providerKey: 'password', email: 'nobody@example.test', password: 'anything' }, + { providerKey: 'password', email: 'active@example.test', password: 'wrong-password' }, + { providerKey: 'password', email: 'invited@example.test', password: 'anything' }, + { providerKey: 'password', email: 'disabled@example.test', password: 'correct-password' }, + { + providerKey: 'unknown-provider', + email: 'active@example.test', + password: 'correct-password', + }, + ] + const bodies: unknown[] = [] + for (const body of attempts) { + const res = await api(req('POST', '/api/v1/auth/verify', { body })) + expect(res.status).toBe(401) + bodies.push(await res.json()) + } + // Every failure shares the exact same envelope shape — no distinguishing detail. + for (const body of bodies) { + expect(body).toEqual(bodies[0]) + } + }) + + it('malformed bodies are also 401 (no distinguishing validation_failed oracle on this endpoint)', async () => { + const { api } = await freshApi() + for (const body of [{}, { providerKey: 123 }, { providerKey: 'password' }]) { + const res = await api(req('POST', '/api/v1/auth/verify', { body })) + expect(res.status).toBe(401) + } + }) + }) + + // --- GET /api/v1/auth/me ----------------------------------------------------- + + describe('GET /auth/me', () => { + it('returns the acting Agent', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore, { email: 'me@example.test' }) + const res = await api(req('GET', '/api/v1/auth/me', { agentId: agent.id })) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + id: agent.id, + email: agent.email, + name: agent.name, + role: agent.role, + timezone: agent.timezone, + }) + }) + + it('401s without the header, and for a disabled/invited/missing Agent', async () => { + const { api, agentStore } = await freshApi() + const disabled = await createActiveAgent(agentStore, { email: 'disabled@example.test' }) + await agentStore.updateAgent(disabled.id, { status: 'disabled' }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + expect((await api(req('GET', '/api/v1/auth/me'))).status).toBe(401) + expect((await api(req('GET', '/api/v1/auth/me', { agentId: disabled.id }))).status).toBe(401) + expect( + (await api(req('GET', '/api/v1/auth/me', { agentId: invitedResult.agent.id }))).status, + ).toBe(401) + expect( + ( + await api( + req('GET', '/api/v1/auth/me', { agentId: '00000000-0000-0000-0000-000000000000' }), + ) + ).status, + ).toBe(401) + expect((await api(req('GET', '/api/v1/auth/me', { agentId: 'not-a-uuid' }))).status).toBe(401) + }) + }) + + // --- GET /api/v1/agents (any active Agent — coordinator amendment) --------- + + describe('GET /agents', () => { + it('a non-admin ACTIVE Agent gets 200 — the roster is not admin-gated', async () => { + const { api, agentStore } = await freshApi() + const nonAdmin = await createActiveAgent(agentStore, { + email: 'nonadmin@example.test', + role: 'agent', + }) + await createActiveAgent(agentStore, { email: 'someone@example.test' }) + + const res = await api(req('GET', '/api/v1/agents', { agentId: nonAdmin.id })) + expect(res.status).toBe(200) + const body = (await res.json()) as { agents: Array<{ email: string }> } + expect(body.agents.map((a) => a.email).sort()).toEqual( + ['nonadmin@example.test', 'someone@example.test'].sort(), + ) + }) + + it('401s without the header, and for a disabled acting Agent', async () => { + const { api, agentStore } = await freshApi() + const disabled = await createActiveAgent(agentStore, { email: 'disabled2@example.test' }) + await agentStore.updateAgent(disabled.id, { status: 'disabled' }) + + expect((await api(req('GET', '/api/v1/agents'))).status).toBe(401) + expect((await api(req('GET', '/api/v1/agents', { agentId: disabled.id }))).status).toBe(401) + }) + }) + + // --- POST /api/v1/agents ----------------------------------------------------- + + describe('POST /agents', () => { + it('admin creates an invited Agent (sendInvite, no uiBaseUrl configured) — inviteSent:false, Agent still created', async () => { + const { api, agentStore, sent } = await freshApi() // no uiBaseUrl + const admin = await createActiveAgent(agentStore, { + email: 'admin@example.test', + role: 'admin', + }) + + const res = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { name: 'New Agent', email: 'new@example.test', role: 'agent', sendInvite: true }, + }), + ) + expect(res.status).toBe(201) + const body = (await res.json()) as { agent: { status: string }; inviteSent: boolean } + expect(body.agent.status).toBe('invited') + expect(body.inviteSent).toBe(false) + expect(sent).toHaveLength(0) + }) + + it('admin creates an invited Agent WITH uiBaseUrl configured — inviteSent:true, email sent', async () => { + const { api, agentStore, sent } = await freshApi({ uiBaseUrl: UI_BASE_URL }) + const admin = await createActiveAgent(agentStore, { + email: 'admin2@example.test', + role: 'admin', + }) + + const res = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { name: 'New Agent', email: 'new2@example.test', role: 'agent', sendInvite: true }, + }), + ) + expect(res.status).toBe(201) + const body = (await res.json()) as { inviteSent: boolean } + expect(body.inviteSent).toBe(true) + expect(sent).toHaveLength(1) + expect(sent[0].to).toEqual(['new2@example.test']) + }) + + it('admin creates an active Agent directly with an admin-set password', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin3@example.test', + role: 'admin', + }) + + const res = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { + name: 'Direct Agent', + email: 'direct@example.test', + role: 'agent', + sendInvite: false, + password: 'admin-set-password', + }, + }), + ) + expect(res.status).toBe(201) + const body = (await res.json()) as { agent: { status: string } } + expect(body.agent.status).toBe('active') + }) + + it('400s when both or neither of sendInvite/password are given', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin4@example.test', + role: 'admin', + }) + + const both = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { + name: 'X', + email: 'x@example.test', + role: 'agent', + sendInvite: true, + password: 'some-password', + }, + }), + ) + expect(both.status).toBe(400) + + const neither = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { name: 'X', email: 'x2@example.test', role: 'agent' }, + }), + ) + expect(neither.status).toBe(400) + }) + + it('409s on a duplicate email', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin5@example.test', + role: 'admin', + }) + await createActiveAgent(agentStore, { email: 'dup@example.test' }) + + const res = await api( + req('POST', '/api/v1/agents', { + agentId: admin.id, + body: { name: 'Dup', email: 'dup@example.test', role: 'agent', sendInvite: true }, + }), + ) + expect(res.status).toBe(409) + }) + + it('403s for a non-admin acting Agent', async () => { + const { api, agentStore } = await freshApi() + const nonAdmin = await createActiveAgent(agentStore, { email: 'nonadmin2@example.test' }) + + const res = await api( + req('POST', '/api/v1/agents', { + agentId: nonAdmin.id, + body: { name: 'X', email: 'x3@example.test', role: 'agent', sendInvite: true }, + }), + ) + expect(res.status).toBe(403) + expect(await res.json()).toEqual({ + error: { code: 'forbidden', message: expect.any(String) }, + }) + }) + + it('401s without the acting-Agent header', async () => { + const { api } = await freshApi() + const res = await api( + req('POST', '/api/v1/agents', { + body: { name: 'X', email: 'x4@example.test', role: 'agent', sendInvite: true }, + }), + ) + expect(res.status).toBe(401) + }) + }) + + // --- GET /api/v1/agents/{id} ------------------------------------------------- + + describe('GET /agents/{id}', () => { + it('admin can view anyone', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin6@example.test', + role: 'admin', + }) + const other = await createActiveAgent(agentStore, { email: 'other@example.test' }) + + const res = await api(req('GET', `/api/v1/agents/${other.id}`, { agentId: admin.id })) + expect(res.status).toBe(200) + }) + + it('self can view own profile', async () => { + const { api, agentStore } = await freshApi() + const self = await createActiveAgent(agentStore, { email: 'self@example.test' }) + const res = await api(req('GET', `/api/v1/agents/${self.id}`, { agentId: self.id })) + expect(res.status).toBe(200) + }) + + it("403s for a non-admin viewing SOMEONE ELSE's profile", async () => { + const { api, agentStore } = await freshApi() + const nonAdmin = await createActiveAgent(agentStore, { email: 'nonadmin3@example.test' }) + const other = await createActiveAgent(agentStore, { email: 'other2@example.test' }) + + const res = await api(req('GET', `/api/v1/agents/${other.id}`, { agentId: nonAdmin.id })) + expect(res.status).toBe(403) + }) + + it('404s for a missing id', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin7@example.test', + role: 'admin', + }) + const res = await api( + req('GET', '/api/v1/agents/00000000-0000-0000-0000-000000000000', { agentId: admin.id }), + ) + expect(res.status).toBe(404) + }) + }) + + // --- PATCH /api/v1/agents/{id} ----------------------------------------------- + + describe('PATCH /agents/{id}', () => { + it('self may PATCH own name/timezone', async () => { + const { api, agentStore } = await freshApi() + const self = await createActiveAgent(agentStore, { email: 'self2@example.test' }) + const res = await api( + req('PATCH', `/api/v1/agents/${self.id}`, { + agentId: self.id, + body: { name: 'Renamed', timezone: 'America/New_York' }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { agent: { name: string; timezone: string } } + expect(body.agent.name).toBe('Renamed') + expect(body.agent.timezone).toBe('America/New_York') + }) + + it('self attempting to PATCH role or status is 403', async () => { + const { api, agentStore } = await freshApi() + const self = await createActiveAgent(agentStore, { email: 'self3@example.test' }) + const roleRes = await api( + req('PATCH', `/api/v1/agents/${self.id}`, { agentId: self.id, body: { role: 'admin' } }), + ) + expect(roleRes.status).toBe(403) + const statusRes = await api( + req('PATCH', `/api/v1/agents/${self.id}`, { + agentId: self.id, + body: { status: 'disabled' }, + }), + ) + expect(statusRes.status).toBe(403) + }) + + it('admin may PATCH name/timezone/role/status on anyone', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin8@example.test', + role: 'admin', + }) + const other = await createActiveAgent(agentStore, { email: 'other3@example.test' }) + + const res = await api( + req('PATCH', `/api/v1/agents/${other.id}`, { + agentId: admin.id, + body: { role: 'admin', status: 'disabled', name: 'Changed' }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { + agent: { role: string; status: string; name: string } + } + expect(body.agent).toMatchObject({ role: 'admin', status: 'disabled', name: 'Changed' }) + }) + + it('email is never settable — 400 regardless of actor', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin9@example.test', + role: 'admin', + }) + const res = await api( + req('PATCH', `/api/v1/agents/${admin.id}`, { + agentId: admin.id, + body: { email: 'new-email@example.test' }, + }), + ) + expect(res.status).toBe(400) + }) + + it('PATCHing status on an INVITED Agent is 409, either direction', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin10@example.test', + role: 'admin', + }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited2@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('PATCH', `/api/v1/agents/${invitedResult.agent.id}`, { + agentId: admin.id, + body: { status: 'active' }, + }), + ) + expect(res.status).toBe(409) + }) + + it('setting status to a value other than active/disabled is 400 (invited is never a settable target)', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin11@example.test', + role: 'admin', + }) + const other = await createActiveAgent(agentStore, { email: 'other4@example.test' }) + const res = await api( + req('PATCH', `/api/v1/agents/${other.id}`, { + agentId: admin.id, + body: { status: 'invited' }, + }), + ) + expect(res.status).toBe(400) + }) + + it('demoting the last active admin is 409 conflict', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'soloadmin@example.test', + role: 'admin', + }) + const res = await api( + req('PATCH', `/api/v1/agents/${admin.id}`, { + agentId: admin.id, + body: { role: 'agent' }, + }), + ) + expect(res.status).toBe(409) + }) + + it('404s for a missing id', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin12@example.test', + role: 'admin', + }) + const res = await api( + req('PATCH', '/api/v1/agents/00000000-0000-0000-0000-000000000000', { + agentId: admin.id, + body: { name: 'X' }, + }), + ) + expect(res.status).toBe(404) + }) + }) + + // --- DELETE /api/v1/agents/{id} ---------------------------------------------- + + describe('DELETE /agents/{id}', () => { + it('admin hard-deletes an Agent — 204', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin13@example.test', + role: 'admin', + }) + const other = await createActiveAgent(agentStore, { email: 'other5@example.test' }) + + const res = await api(req('DELETE', `/api/v1/agents/${other.id}`, { agentId: admin.id })) + expect(res.status).toBe(204) + expect(await agentStore.getAgent(other.id)).toBeNull() + }) + + it('deleting the last active admin is 409 conflict', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'soloadmin2@example.test', + role: 'admin', + }) + const res = await api(req('DELETE', `/api/v1/agents/${admin.id}`, { agentId: admin.id })) + expect(res.status).toBe(409) + }) + + it('403s for a non-admin', async () => { + const { api, agentStore } = await freshApi() + const nonAdmin = await createActiveAgent(agentStore, { email: 'nonadmin4@example.test' }) + const other = await createActiveAgent(agentStore, { email: 'other6@example.test' }) + const res = await api(req('DELETE', `/api/v1/agents/${other.id}`, { agentId: nonAdmin.id })) + expect(res.status).toBe(403) + }) + + it('404s for a missing id', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin14@example.test', + role: 'admin', + }) + const res = await api( + req('DELETE', '/api/v1/agents/00000000-0000-0000-0000-000000000000', { + agentId: admin.id, + }), + ) + expect(res.status).toBe(404) + }) + }) + + // --- POST /api/v1/agents/{id}/password --------------------------------------- + + describe('POST /agents/{id}/password', () => { + it('self may set their own password', async () => { + const { api, agentStore } = await freshApi() + const self = await createActiveAgent(agentStore, { email: 'self4@example.test' }) + const res = await api( + req('POST', `/api/v1/agents/${self.id}/password`, { + agentId: self.id, + body: { password: 'brand-new-password' }, + }), + ) + expect(res.status).toBe(204) + }) + + it("admin may reset someone else's password", async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin15@example.test', + role: 'admin', + }) + const other = await createActiveAgent(agentStore, { email: 'other7@example.test' }) + const res = await api( + req('POST', `/api/v1/agents/${other.id}/password`, { + agentId: admin.id, + body: { password: 'admin-reset-password' }, + }), + ) + expect(res.status).toBe(204) + }) + + it("403s for a non-admin trying to set SOMEONE ELSE's password", async () => { + const { api, agentStore } = await freshApi() + const nonAdmin = await createActiveAgent(agentStore, { email: 'nonadmin5@example.test' }) + const other = await createActiveAgent(agentStore, { email: 'other8@example.test' }) + const res = await api( + req('POST', `/api/v1/agents/${other.id}/password`, { + agentId: nonAdmin.id, + body: { password: 'sneaky-password' }, + }), + ) + expect(res.status).toBe(403) + }) + + it('409s when the target is invited', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin16@example.test', + role: 'admin', + }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited3@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('POST', `/api/v1/agents/${invitedResult.agent.id}/password`, { + agentId: admin.id, + body: { password: 'wont-work' }, + }), + ) + expect(res.status).toBe(409) + }) + + it('is allowed for a disabled target (admin reset)', async () => { + const { api, agentStore } = await freshApi() + const admin = await createActiveAgent(agentStore, { + email: 'admin17@example.test', + role: 'admin', + }) + const disabled = await createActiveAgent(agentStore, { email: 'disabled3@example.test' }) + await agentStore.updateAgent(disabled.id, { status: 'disabled' }) + + const res = await api( + req('POST', `/api/v1/agents/${disabled.id}/password`, { + agentId: admin.id, + body: { password: 'reset-while-disabled' }, + }), + ) + expect(res.status).toBe(204) + }) + + it('400s on a too-short password', async () => { + const { api, agentStore } = await freshApi() + const self = await createActiveAgent(agentStore, { email: 'self5@example.test' }) + const res = await api( + req('POST', `/api/v1/agents/${self.id}/password`, { + agentId: self.id, + body: { password: 'short' }, + }), + ) + expect(res.status).toBe(400) + }) + }) + + // --- POST /api/v1/agents/{id}/invite ----------------------------------------- + + describe('POST /agents/{id}/invite', () => { + it('admin resends an invite when uiBaseUrl is configured — 204, email sent', async () => { + const { api, agentStore, sent } = await freshApi({ uiBaseUrl: UI_BASE_URL }) + const admin = await createActiveAgent(agentStore, { + email: 'admin18@example.test', + role: 'admin', + }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited4@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('POST', `/api/v1/agents/${invitedResult.agent.id}/invite`, { agentId: admin.id }), + ) + expect(res.status).toBe(204) + expect(sent).toHaveLength(1) + expect(sent[0].to).toEqual(['invited4@example.test']) + }) + + it('409s when the target is active/disabled (not invited)', async () => { + const { api, agentStore } = await freshApi({ uiBaseUrl: UI_BASE_URL }) + const admin = await createActiveAgent(agentStore, { + email: 'admin19@example.test', + role: 'admin', + }) + const active = await createActiveAgent(agentStore, { email: 'active2@example.test' }) + + const res = await api( + req('POST', `/api/v1/agents/${active.id}/invite`, { agentId: admin.id }), + ) + expect(res.status).toBe(409) + }) + + it('409s when no uiBaseUrl is configured', async () => { + const { api, agentStore } = await freshApi() // no uiBaseUrl + const admin = await createActiveAgent(agentStore, { + email: 'admin20@example.test', + role: 'admin', + }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited5@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('POST', `/api/v1/agents/${invitedResult.agent.id}/invite`, { agentId: admin.id }), + ) + expect(res.status).toBe(409) + }) + + it('502s when the sender rejects the message', async () => { + const { api, agentStore } = await freshApi({ + uiBaseUrl: UI_BASE_URL, + sender: createThrowingSender(), + }) + const admin = await createActiveAgent(agentStore, { + email: 'admin21@example.test', + role: 'admin', + }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited6@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('POST', `/api/v1/agents/${invitedResult.agent.id}/invite`, { agentId: admin.id }), + ) + expect(res.status).toBe(502) + expect(await res.json()).toEqual({ + error: { code: 'send_failed', message: expect.any(String) }, + }) + }) + + it('403s for a non-admin', async () => { + const { api, agentStore } = await freshApi({ uiBaseUrl: UI_BASE_URL }) + const nonAdmin = await createActiveAgent(agentStore, { email: 'nonadmin6@example.test' }) + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited7@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + + const res = await api( + req('POST', `/api/v1/agents/${invitedResult.agent.id}/invite`, { agentId: nonAdmin.id }), + ) + expect(res.status).toBe(403) + }) + }) + + // --- POST /api/v1/auth/invite/accept ----------------------------------------- + + describe('POST /auth/invite/accept', () => { + it('activates the invited Agent and sets the password', async () => { + const { api, agentStore } = await freshApi() + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited8@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + const token = mintInviteToken(invitedResult.agent.id, KEYRING) + + const res = await api( + req('POST', '/api/v1/auth/invite/accept', { + body: { token, password: 'accepted-password' }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { agent: { status: string } } + expect(body.agent.status).toBe('active') + + // The web would now sign in with this password. + const verify = await api( + req('POST', '/api/v1/auth/verify', { + body: { + providerKey: 'password', + email: 'invited8@example.test', + password: 'accepted-password', + }, + }), + ) + expect(verify.status).toBe(200) + }) + + it('is one-time: accepting the SAME token twice 401s the second time', async () => { + const { api, agentStore } = await freshApi() + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited9@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + const token = mintInviteToken(invitedResult.agent.id, KEYRING) + + const first = await api( + req('POST', '/api/v1/auth/invite/accept', { body: { token, password: 'first-password' } }), + ) + expect(first.status).toBe(200) + + const second = await api( + req('POST', '/api/v1/auth/invite/accept', { body: { token, password: 'second-password' } }), + ) + expect(second.status).toBe(401) + }) + + it('401s for a bogus/tampered token', async () => { + const { api } = await freshApi() + const res = await api( + req('POST', '/api/v1/auth/invite/accept', { + body: { token: 'hti.k1.garbage.sig', password: 'whatever-password' }, + }), + ) + expect(res.status).toBe(401) + }) + + it('400s on a too-short password (checked independently of the token)', async () => { + const { api, agentStore } = await freshApi() + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited10@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + const token = mintInviteToken(invitedResult.agent.id, KEYRING) + + const res = await api( + req('POST', '/api/v1/auth/invite/accept', { body: { token, password: 'short' } }), + ) + expect(res.status).toBe(400) + }) + + it('does not require the acting-Agent header (pre-session)', async () => { + const { api, agentStore } = await freshApi() + const invitedResult = await agentStore.createAgent({ + name: 'Invited', + email: 'invited11@example.test', + role: 'agent', + status: 'invited', + }) + if (!invitedResult.ok) throw new Error('expected ok') + const token = mintInviteToken(invitedResult.agent.id, KEYRING) + + // No X-Helpthread-Agent-Id header at all — must still succeed. + const res = await api( + new Request('https://x.example.test/api/v1/auth/invite/accept', { + method: 'POST', + headers: { Authorization: `Bearer ${TOKEN}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ token, password: 'no-header-needed' }), + }), + ) + expect(res.status).toBe(200) + }) + }) +}) diff --git a/src/api/agents.ts b/src/api/agents.ts new file mode 100644 index 0000000..f76ede8 --- /dev/null +++ b/src/api/agents.ts @@ -0,0 +1,610 @@ +/** + * The Agents & Authentication API handlers (HT-54; specs/auth/agents-and-auth.md + * §6) — auth bootstrap (`/auth/providers`, `/setup`, `/auth/verify`, + * `/auth/me`, `/auth/invite/accept`) and Agent management (`/agents`, + * `/agents/{id}`, `/agents/{id}/password`, `/agents/{id}/invite`). + * + * Same shape as `src/api/conversations.ts`: each handler is a pure function + * of an already-authenticated (service Bearer), already-routed `Request` + * plus its dependencies — `src/api/index.ts` authenticates and routes; + * nothing here re-checks either. Every response goes through + * `src/api/responses.ts`'s helpers. + * + * ## The acting-Agent header is a SEPARATE check from the service Bearer + * + * Per-endpoint, `src/api/index.ts` resolves the acting Agent + * (`resolveActingAgent`, `src/api/acting-agent.ts`) and passes the result + * (an {@link AgentRecord} or `null`) into the handlers below that need it. + * `null` means "no acting Agent" — missing header, malformed value, or an + * Agent that is missing/not `active` — and every handler that requires one + * maps `null` to a generic `401 unauthorized`, exactly as spec §8 requires + * (never a more specific message that would distinguish "no header" from + * "disabled Agent" from "unknown id"). + * + * ## Role gates + * + * - `GET /agents` (the roster) — any ACTIVE acting Agent, not admin-only: + * the inbox's assignee picker (any Agent may assign any Agent, spec §5's + * role model) needs the roster to render names, so admin-gating the list + * would make a non-admin's own assignee menu impossible. (Coordinator + * amendment, 2026-07-18 — the canonical spec text is being updated in the + * same PR; this comment states the AS-BUILT behavior.) + * - `GET /agents/{id}` — admin, or self. + * - Every mutation (`POST /agents`, `PATCH`, `DELETE`, `/password` on + * someone else, `/invite`) — admin-only, except a self `PATCH` (own + * name/timezone) and a self `/password` (own password), both spec-pinned + * exceptions. + * + * ## Error codes + * + * Two NEW slugs beyond the existing `unauthorized`/`not_found`/ + * `validation_failed`/`send_failed`/`server_error` set: `forbidden` (403, an + * authenticated-but-not-permitted acting Agent) and `conflict` (409 — email + * taken, last-admin violation, an invited Agent's status/password touched + * outside its lifecycle, invites unavailable). Never `secret_hash`, + * a password, or a token anywhere in a response body. + */ + +import { buildInviteEmail } from '../auth/invite-email.js' +import { mintInviteToken, verifyInviteToken } from '../auth/invite-token.js' +import { hashPassword, MAX_PASSWORD_LENGTH } from '../auth/password-hash.js' +import type { AuthAttempt, AuthProvider } from '../auth/provider.js' +import type { Keyring } from '../mail/reply-token.js' +import type { OutboundEmail } from '../providers/email-sender.js' +import type { EmailSender } from '../providers/index.js' +import type { AgentRecord, AgentRole, AgentStore } from '../store/agents.js' +import { apiError, json, noContent } from './responses.js' +import { isUuid } from './uuid.js' + +/** + * The new `agents` field on `InboxApiDeps` (`src/api/index.ts`) — agents/auth + * is CORE, not an optional/absent-by-default feature like `openTracking` or + * `gmailPush`, so this is a REQUIRED part of `InboxApiDeps` (brief's + * explicit pin). Deliberately narrow: `keyring`/`sender`/`mailDomain`/ + * `supportAddress` are already required top-level `InboxApiDeps` fields + * (the invite path reuses them rather than duplicating config surface), so + * this object carries only what's genuinely NEW. + */ +export interface AgentsApiDeps { + store: AgentStore + providers: AuthProvider[] + /** The web UI's base URL (`HELPTHREAD_UI_BASE_URL`) — ABSENT when unset (spec §8's "a fresh deploy can't email before it can"): `sendInvite` still creates the Agent (`inviteSent: false`), and `/agents/{id}/invite` refuses with `409 conflict`. */ + uiBaseUrl?: string +} + +/** Dependencies every handler in this module may need. Built once per request by `src/api/index.ts`, merging `InboxApiDeps.agents` with the top-level `keyring`/`sender`/`mailDomain`/`supportAddress` fields every request already carries. */ +export interface AgentsHandlerDeps extends AgentsApiDeps { + keyring: Keyring + sender: EmailSender + mailDomain: string + supportAddress: string +} + +// --- validation (spec's pinned rules) --------------------------------------- + +const MAX_EMAIL_LENGTH = 254 +const MIN_NAME_LENGTH = 1 +const MAX_NAME_LENGTH = 200 +const MIN_PASSWORD_LENGTH = 8 +const MAX_TIMEZONE_LENGTH = 64 + +/** Trim + lowercase; require exactly one `@` with a nonempty local part and domain, ≤254 chars — no heroic regex (brief's pinned rule). `null` on any violation. */ +function normalizeEmail(raw: unknown): string | null { + if (typeof raw !== 'string') return null + const trimmed = raw.trim().toLowerCase() + if (trimmed.length === 0 || trimmed.length > MAX_EMAIL_LENGTH) return null + const at = trimmed.indexOf('@') + if (at <= 0 || at === trimmed.length - 1) return null + if (trimmed.indexOf('@', at + 1) !== -1) return null + return trimmed +} + +/** Trim; 1-200 chars. `null` on any violation. */ +function validateName(raw: unknown): string | null { + if (typeof raw !== 'string') return null + const trimmed = raw.trim() + return trimmed.length >= MIN_NAME_LENGTH && trimmed.length <= MAX_NAME_LENGTH ? trimmed : null +} + +/** NOT trimmed (a user secret) — 8-256 chars. `null` on any violation. */ +function validatePassword(raw: unknown): string | null { + if (typeof raw !== 'string') return null + return raw.length >= MIN_PASSWORD_LENGTH && raw.length <= MAX_PASSWORD_LENGTH ? raw : null +} + +function validateRole(raw: unknown): AgentRole | null { + return raw === 'admin' || raw === 'agent' ? raw : null +} + +/** ≤64 chars; validated by asking `Intl.DateTimeFormat` to accept it as a `timeZone` — the brief's pinned check, not a hand-rolled IANA-name allowlist. `null` on any violation. */ +function validateTimezone(raw: unknown): string | null { + if (typeof raw !== 'string' || raw.length === 0 || raw.length > MAX_TIMEZONE_LENGTH) return null + try { + new Intl.DateTimeFormat(undefined, { timeZone: raw }) + return raw + } catch { + return null + } +} + +/** Read and JSON-parse `request`'s body without ever throwing — mirrors `src/api/conversations.ts`'s helper of the same name (kept local per this codebase's per-file convention). */ +async function parseJsonBody( + request: Request, +): Promise<{ ok: true; value: unknown } | { ok: false }> { + try { + return { ok: true, value: await request.json() } + } catch { + return { ok: false } + } +} + +/** `typeof value === 'object' && value !== null`, narrowed to a plain record — the shared "is this a JSON object" gate every body-shape parser below starts with. */ +function asRecord(value: unknown): Record | null { + return typeof value === 'object' && value !== null ? (value as Record) : null +} + +// --- wire shape -------------------------------------------------------------- + +interface AgentJson { + id: string + email: string + name: string + role: AgentRole + status: 'invited' | 'active' | 'disabled' + timezone: string + createdAt: string + updatedAt: string +} + +function toAgentJson(agent: AgentRecord): AgentJson { + return { + id: agent.id, + email: agent.email, + name: agent.name, + role: agent.role, + status: agent.status, + timezone: agent.timezone, + createdAt: agent.createdAt.toISOString(), + updatedAt: agent.updatedAt.toISOString(), + } +} + +const UNAUTHORIZED = () => apiError(401, 'unauthorized', 'Missing or invalid Agent identity.') +/** The login endpoint's own uniform failure — every `/auth/verify` miss is this exact response (spec §9's no-oracle rule), phrased for a sign-in, not for the acting-Agent header. */ +const INVALID_CREDENTIALS = () => apiError(401, 'unauthorized', 'Invalid email or password.') +const NOT_FOUND = () => apiError(404, 'not_found', 'No Agent with that id.') + +// --- GET /api/v1/auth/providers --------------------------------------------- + +/** `GET /api/v1/auth/providers` (spec §6) — no acting-Agent header. */ +export async function handleAuthProviders( + deps: Pick, +): Promise { + const count = await deps.store.countAgents() + return json(200, { + providers: deps.providers.map((provider) => provider.descriptor()), + needsSetup: count === 0, + }) +} + +// --- POST /api/v1/setup ----------------------------------------------------- + +/** `POST /api/v1/setup` (spec §6) — creates the first admin. No acting-Agent header (pre-session). */ +export async function handleSetup( + request: Request, + deps: Pick, +): Promise { + const parsed = await parseJsonBody(request) + if (!parsed.ok) return apiError(400, 'validation_failed', 'Request body must be valid JSON.') + const body = asRecord(parsed.value) + if (body === null) + return apiError(400, 'validation_failed', 'Request body must be a JSON object.') + + const name = validateName(body.name) + const email = normalizeEmail(body.email) + const password = validatePassword(body.password) + if (name === null || email === null || password === null) { + return apiError( + 400, + 'validation_failed', + 'name, email, and password are required and must be valid (password 8-256 characters).', + ) + } + + const agent = await deps.store.createFirstAdmin({ + name, + email, + passwordHash: hashPassword(password), + }) + if (agent === null) { + return apiError(409, 'conflict', 'Setup has already been completed on this deployment.') + } + return json(201, { agent: toAgentJson(agent) }) +} + +// --- POST /api/v1/auth/verify ----------------------------------------------- + +/** + * `POST /api/v1/auth/verify` (spec §6, §9) — dispatch to the named + * provider. EVERY failure mode is the SAME generic `401` (unknown email, + * wrong password, an unknown `providerKey`, a malformed body, an + * `invited`/`disabled` Agent) — spec §9: "no oracle." No acting-Agent + * header (pre-session). + */ +export async function handleAuthVerify( + request: Request, + deps: Pick, +): Promise { + const parsed = await parseJsonBody(request) + if (!parsed.ok) return INVALID_CREDENTIALS() + const body = asRecord(parsed.value) + if (body === null) return INVALID_CREDENTIALS() + + const providerKey = body.providerKey + if (typeof providerKey !== 'string') return INVALID_CREDENTIALS() + + const provider = deps.providers.find((candidate) => candidate.key === providerKey) + if (provider === undefined) return INVALID_CREDENTIALS() + + const attempt: AuthAttempt = { ...body, providerKey } + const verified = await provider.authenticate(attempt) + if (verified === null) return INVALID_CREDENTIALS() + + const agent = await deps.store.getAgent(verified.agentId) + if (agent === null) return INVALID_CREDENTIALS() + + return json(200, { agent: toAgentJson(agent) }) +} + +// --- GET /api/v1/auth/me ---------------------------------------------------- + +/** `GET /api/v1/auth/me` (spec §6) — acting-Agent header REQUIRED. */ +export function handleAuthMe(actingAgent: AgentRecord | null): Response { + if (actingAgent === null) return UNAUTHORIZED() + return json(200, { + id: actingAgent.id, + email: actingAgent.email, + name: actingAgent.name, + role: actingAgent.role, + timezone: actingAgent.timezone, + }) +} + +// --- GET /api/v1/agents ------------------------------------------------------ + +/** `GET /api/v1/agents` (spec §6, as amended) — any ACTIVE acting Agent; the roster every assignee picker needs, not admin-gated. */ +export async function handleListAgents( + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + const agents = await deps.store.listAgents() + return json(200, { agents: agents.map(toAgentJson) }) +} + +// --- POST /api/v1/agents ----------------------------------------------------- + +/** `POST /api/v1/agents` (spec §6, §8) — admin only. Exactly one of `sendInvite: true` or `password` (else `400`). Duplicate email → `409 conflict`. */ +export async function handleCreateAgent( + actingAgent: AgentRecord | null, + request: Request, + deps: AgentsHandlerDeps, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + if (actingAgent.role !== 'admin') return apiError(403, 'forbidden', 'Admin role required.') + + const parsed = await parseJsonBody(request) + if (!parsed.ok) return apiError(400, 'validation_failed', 'Request body must be valid JSON.') + const body = asRecord(parsed.value) + if (body === null) + return apiError(400, 'validation_failed', 'Request body must be a JSON object.') + + const name = validateName(body.name) + const email = normalizeEmail(body.email) + const role = validateRole(body.role) + if (name === null || email === null || role === null) { + return apiError( + 400, + 'validation_failed', + 'name, email, and role are required and must be valid.', + ) + } + + const sendInvite = body.sendInvite === true + const hasPassword = typeof body.password === 'string' + if (sendInvite === hasPassword) { + return apiError( + 400, + 'validation_failed', + 'Exactly one of sendInvite (true) or password must be provided.', + ) + } + + let passwordHash: string | undefined + if (hasPassword) { + const password = validatePassword(body.password) + if (password === null) { + return apiError(400, 'validation_failed', 'password must be 8-256 characters.') + } + passwordHash = hashPassword(password) + } + + const result = await deps.store.createAgent({ + name, + email, + role, + status: sendInvite ? 'invited' : 'active', + ...(passwordHash !== undefined ? { passwordHash } : {}), + }) + if (!result.ok) { + return apiError(409, 'conflict', 'An Agent with that email already exists.') + } + + const inviteSent = sendInvite ? await sendInviteEmail(result.agent, deps) : false + + return json(201, { agent: toAgentJson(result.agent), inviteSent }) +} + +/** Mint an invite token and send the invite email — shared by `handleCreateAgent` and `handleResendInvite`'s success path. Returns whether the send actually happened; NEVER throws (a send failure here is `inviteSent: false`, not a request failure — the Agent is already created). */ +async function sendInviteEmail(agent: AgentRecord, deps: AgentsHandlerDeps): Promise { + if (deps.uiBaseUrl === undefined) return false + const token = mintInviteToken(agent.id, deps.keyring) + const email: OutboundEmail = buildInviteEmail({ + to: agent.email, + token, + uiBaseUrl: deps.uiBaseUrl, + supportAddress: deps.supportAddress, + mailDomain: deps.mailDomain, + }) + try { + await deps.sender.send(email) + return true + } catch (err) { + console.error('[agents] invite send failed', err) + return false + } +} + +// --- GET /api/v1/agents/{id} ------------------------------------------------- + +/** `GET /api/v1/agents/{id}` (spec §6) — admin, or self. */ +export async function handleGetAgent( + id: string, + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + if (actingAgent.role !== 'admin' && actingAgent.id !== id) { + return apiError(403, 'forbidden', 'You may only view your own profile.') + } + if (!isUuid(id)) return NOT_FOUND() + + const agent = await deps.store.getAgent(id) + if (agent === null) return NOT_FOUND() + return json(200, { agent: toAgentJson(agent) }) +} + +// --- PATCH /api/v1/agents/{id} ----------------------------------------------- + +/** Fields an admin may PATCH on ANY Agent. */ +const ADMIN_PATCH_FIELDS = ['name', 'timezone', 'role', 'status'] +/** Fields a non-admin may PATCH on THEMSELF. */ +const SELF_PATCH_FIELDS = ['name', 'timezone'] + +/** + * `PATCH /api/v1/agents/{id}` (spec §6) — self (non-admin) may set only + * `name`/`timezone` on themself; admin may set `name`/`timezone`/`role`/ + * `status` on anyone. `email` is never settable. `status` is a closed + * lifecycle: only `active`↔`disabled` (never `invited` as source or + * target) — naming `status` on a currently-`invited` Agent is `409 + * conflict`. + */ +export async function handlePatchAgent( + id: string, + actingAgent: AgentRecord | null, + request: Request, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const isAdmin = actingAgent.role === 'admin' + const isSelf = actingAgent.id === id + if (!isAdmin && !isSelf) { + return apiError(403, 'forbidden', 'You may only edit your own profile.') + } + if (!isUuid(id)) return NOT_FOUND() + + const parsed = await parseJsonBody(request) + if (!parsed.ok) return apiError(400, 'validation_failed', 'Request body must be valid JSON.') + const body = asRecord(parsed.value) + if (body === null) + return apiError(400, 'validation_failed', 'Request body must be a JSON object.') + + if ('email' in body) { + return apiError(400, 'validation_failed', 'email cannot be changed (immutable in v1).') + } + + const allowedFields = isAdmin ? ADMIN_PATCH_FIELDS : SELF_PATCH_FIELDS + for (const key of Object.keys(body)) { + if (!allowedFields.includes(key)) { + return isAdmin + ? apiError(400, 'validation_failed', `Unknown field '${key}'.`) + : apiError(403, 'forbidden', 'You may only edit your name and timezone.') + } + } + + const patch: { + name?: string + timezone?: string + role?: AgentRole + status?: 'active' | 'disabled' + } = {} + if ('name' in body) { + const name = validateName(body.name) + if (name === null) return apiError(400, 'validation_failed', 'name must be 1-200 characters.') + patch.name = name + } + if ('timezone' in body) { + const timezone = validateTimezone(body.timezone) + if (timezone === null) { + return apiError(400, 'validation_failed', 'timezone must be a valid IANA time zone.') + } + patch.timezone = timezone + } + if ('role' in body) { + const role = validateRole(body.role) + if (role === null) return apiError(400, 'validation_failed', "role must be 'admin' or 'agent'.") + patch.role = role + } + if ('status' in body) { + if (body.status !== 'active' && body.status !== 'disabled') { + return apiError(400, 'validation_failed', "status must be 'active' or 'disabled'.") + } + patch.status = body.status + } + + if (patch.status !== undefined) { + const current = await deps.store.getAgent(id) + if (current === null) return NOT_FOUND() + if (current.status === 'invited') { + return apiError( + 409, + 'conflict', + 'An invited Agent cannot have its status changed via PATCH — it activates only by accepting its invite.', + ) + } + } + + const result = await deps.store.updateAgent(id, patch) + if (!result.ok) { + if (result.reason === 'not_found') return NOT_FOUND() + return apiError(409, 'conflict', 'This would leave the deployment with no active admin.') + } + return json(200, { agent: toAgentJson(result.agent) }) +} + +// --- DELETE /api/v1/agents/{id} ---------------------------------------------- + +/** `DELETE /api/v1/agents/{id}` (spec §6) — admin only, hard delete. Last active admin → `409 conflict`. */ +export async function handleDeleteAgent( + id: string, + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + if (actingAgent.role !== 'admin') return apiError(403, 'forbidden', 'Admin role required.') + if (!isUuid(id)) return NOT_FOUND() + + const result = await deps.store.deleteAgent(id) + if (!result.ok) { + if (result.reason === 'not_found') return NOT_FOUND() + return apiError(409, 'conflict', 'This would leave the deployment with no active admin.') + } + return noContent() +} + +// --- POST /api/v1/agents/{id}/password --------------------------------------- + +/** `POST /api/v1/agents/{id}/password` (spec §6) — self, or admin reset. Refused (`409`) for an `invited` target. */ +export async function handleSetAgentPassword( + id: string, + actingAgent: AgentRecord | null, + request: Request, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + if (!isUuid(id)) return NOT_FOUND() + + const isAdmin = actingAgent.role === 'admin' + const isSelf = actingAgent.id === id + if (!isAdmin && !isSelf) { + return apiError(403, 'forbidden', 'You may only change your own password.') + } + + const target = await deps.store.getAgent(id) + if (target === null) return NOT_FOUND() + if (target.status === 'invited') { + return apiError( + 409, + 'conflict', + 'Cannot set a password for an invited Agent — accept the invite instead.', + ) + } + + const parsed = await parseJsonBody(request) + if (!parsed.ok) return apiError(400, 'validation_failed', 'Request body must be valid JSON.') + const body = asRecord(parsed.value) + const password = body === null ? null : validatePassword(body.password) + if (password === null) { + return apiError(400, 'validation_failed', 'password must be 8-256 characters.') + } + + await deps.store.setPassword(id, hashPassword(password)) + return noContent() +} + +// --- POST /api/v1/agents/{id}/invite ----------------------------------------- + +/** `POST /api/v1/agents/{id}/invite` (spec §6, §8) — admin only, re-mint + re-send for an `invited` Agent. */ +export async function handleResendInvite( + id: string, + actingAgent: AgentRecord | null, + deps: AgentsHandlerDeps, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + if (actingAgent.role !== 'admin') return apiError(403, 'forbidden', 'Admin role required.') + if (!isUuid(id)) return NOT_FOUND() + + const target = await deps.store.getAgent(id) + if (target === null) return NOT_FOUND() + if (target.status !== 'invited') { + return apiError(409, 'conflict', 'This Agent is not awaiting an invite.') + } + if (deps.uiBaseUrl === undefined) { + return apiError(409, 'conflict', 'Invites are not available on this deployment.') + } + + const token = mintInviteToken(target.id, deps.keyring) + const email = buildInviteEmail({ + to: target.email, + token, + uiBaseUrl: deps.uiBaseUrl, + supportAddress: deps.supportAddress, + mailDomain: deps.mailDomain, + }) + try { + await deps.sender.send(email) + } catch (err) { + console.error('[agents] invite resend failed', err) + return apiError(502, 'send_failed', 'The invite email could not be sent.') + } + return noContent() +} + +// --- POST /api/v1/auth/invite/accept ----------------------------------------- + +/** `POST /api/v1/auth/invite/accept` (spec §6, §9) — validate the token, set the password, flip `invited` → `active`, atomically. Expired/replayed/invalid are ALL the same generic `401`. No acting-Agent header (pre-session). */ +export async function handleInviteAccept( + request: Request, + deps: Pick, +): Promise { + const parsed = await parseJsonBody(request) + if (!parsed.ok) return apiError(401, 'unauthorized', 'Invalid or expired invite.') + const body = asRecord(parsed.value) + if (body === null) return apiError(401, 'unauthorized', 'Invalid or expired invite.') + + if (typeof body.token !== 'string') + return apiError(401, 'unauthorized', 'Invalid or expired invite.') + const password = validatePassword(body.password) + if (password === null) { + return apiError(400, 'validation_failed', 'password must be 8-256 characters.') + } + + const verified = verifyInviteToken(body.token, deps.keyring) + if (verified === null) return apiError(401, 'unauthorized', 'Invalid or expired invite.') + + const agent = await deps.store.acceptInvite(verified.agentId, hashPassword(password)) + if (agent === null) return apiError(401, 'unauthorized', 'Invalid or expired invite.') + + return json(200, { agent: toAgentJson(agent) }) +} diff --git a/src/api/conversations.ts b/src/api/conversations.ts index b619e4a..20291de 100644 --- a/src/api/conversations.ts +++ b/src/api/conversations.ts @@ -18,6 +18,7 @@ import type { Keyring } from '../mail/reply-token.js' import { type SelfEchoGuardDeps, sendReply } from '../mail/send.js' import type { BlobStore, EmailSender } from '../providers/index.js' +import type { AgentRecord, AgentStore } from '../store/agents.js' import type { StoredThreadAttachment, ThreadAttachmentStore } from '../store/attachments.js' import { type ConversationFolder, @@ -89,7 +90,7 @@ interface ConversationSummaryJson { threadCount: number preview: string tags: string[] - assignee: 'me' | null + assigneeAgentId: string | null createdAt: string updatedAt: string } @@ -245,7 +246,7 @@ export async function handleGetConversation( threadCount: conversation.threads.length, preview: previewFromThreads(conversation.threads), tags: conversation.tags, - assignee: conversation.assignee, + assigneeAgentId: conversation.assigneeAgentId, createdAt: conversation.createdAt.toISOString(), updatedAt: conversation.updatedAt.toISOString(), threads: conversation.threads.map((thread) => @@ -652,17 +653,30 @@ export async function handlePutTags( } /** - * Handle `PUT /api/v1/conversations/{id}/assignee` — claim or release (spec - * §4f, v1.1). Body: `{ assignee: 'me' | null }` — the property must be - * present and exactly one of those two values (`'me'` = the deployment's - * one operator; `null` = Anyone); anything else is `400 validation_failed`. + * Handle `PUT /api/v1/conversations/{id}/assignee` — assign or release + * (spec §4f, v1.1; graduated to a real Agent identity by HT-54, + * specs/auth/agents-and-auth.md §3.3/§10 — **breaking**: the body was + * `{ assignee: 'me' | null }`, now `{ assigneeAgentId: uuid | null }`; the + * old shape is simply a `400` now, since `assignee` is not a recognized + * property). This is now the one existing inbox endpoint that requires the + * acting-Agent header (spec §8) — any ACTIVE Agent may assign any Agent + * (spec §5's role model; no admin gate here). + * + * A non-null `assigneeAgentId` that isn't uuid-shaped, or doesn't name an + * existing Agent, is `400 validation_failed` (a generic message — no + * existence oracle beyond what any Agent can already see via `GET + * /api/v1/agents`, per the brief's acceptance of that as fine here). * `200` with the updated summary; missing or deleted conversation → `404`. */ export async function handlePutAssignee( id: string, + actingAgent: AgentRecord | null, request: Request, - deps: { store: ConversationStore }, + deps: { store: ConversationStore; agentStore: AgentStore }, ): Promise { + if (actingAgent === null) { + return apiError(401, 'unauthorized', 'Missing or invalid Agent identity.') + } if (!isUuid(id)) { return apiError(404, 'not_found', 'No conversation with that id.') } @@ -672,12 +686,26 @@ export async function handlePutAssignee( return apiError(400, 'validation_failed', 'Request body must be valid JSON.') } - const assignee = parseAssigneeBody(parsedBody.value) - if (assignee === undefined) { - return apiError(400, 'validation_failed', "assignee must be 'me' or null.") + const assigneeAgentId = parseAssigneeBody(parsedBody.value) + if (assigneeAgentId === undefined) { + return apiError(400, 'validation_failed', 'assigneeAgentId must be a uuid string or null.') + } + if (assigneeAgentId !== null) { + if (!isUuid(assigneeAgentId)) { + return apiError(400, 'validation_failed', 'assigneeAgentId must be a valid uuid.') + } + const assigneeExists = await deps.agentStore.getAgent(assigneeAgentId) + if (assigneeExists === null) { + return apiError(400, 'validation_failed', 'assigneeAgentId does not name an existing Agent.') + } } - const updated = await deps.store.setConversationAssignee(id, assignee) + const updated = await deps.store.setConversationAssignee(id, assigneeAgentId) + if (updated === 'invalid_agent') { + // The Agent existed at the check above but was deleted before the write + // landed — same caller-facing outcome as never having existed. + return apiError(400, 'validation_failed', 'assigneeAgentId does not name an existing Agent.') + } if (updated === null) { return apiError(404, 'not_found', 'No conversation with that id.') } @@ -785,16 +813,20 @@ function parseTagsBody(raw: unknown): string[] | null { } /** - * Validate a PUT-assignee body against spec §4f: the `assignee` property - * must be PRESENT and exactly `'me'` or `null`. Returns the value, or - * `undefined` on any violation (which is unambiguous precisely because - * `undefined` — a missing property — is itself a violation) — never throws. + * Validate a PUT-assignee body against spec §4f/§10 (HT-54's breaking body + * shape): the `assigneeAgentId` property must be PRESENT and either a + * string (uuid-shape checked by the caller, which also confirms the Agent + * exists) or `null`. Returns the value, or `undefined` on any violation + * (unambiguous precisely because `undefined` — a missing property, or the + * OLD `{ assignee: 'me' }` shape, which has no `assigneeAgentId` key at + * all — is itself a violation) — never throws. */ -function parseAssigneeBody(raw: unknown): 'me' | null | undefined { +function parseAssigneeBody(raw: unknown): string | null | undefined { if (typeof raw !== 'object' || raw === null) return undefined - if (!('assignee' in raw)) return undefined - const { assignee } = raw as Record - return assignee === 'me' || assignee === null ? assignee : undefined + if (!('assigneeAgentId' in raw)) return undefined + const { assigneeAgentId } = raw as Record + if (assigneeAgentId === null) return null + return typeof assigneeAgentId === 'string' ? assigneeAgentId : undefined } /** @@ -847,7 +879,7 @@ function toConversationSummaryJson(row: { threadCount: number preview: string tags: string[] - assignee: 'me' | null + assigneeAgentId: string | null createdAt: Date updatedAt: Date }): ConversationSummaryJson { @@ -860,7 +892,7 @@ function toConversationSummaryJson(row: { threadCount: row.threadCount, preview: row.preview, tags: row.tags, - assignee: row.assignee, + assigneeAgentId: row.assigneeAgentId, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), } diff --git a/src/api/index.test.ts b/src/api/index.test.ts index c27ae19..222f121 100644 --- a/src/api/index.test.ts +++ b/src/api/index.test.ts @@ -1,5 +1,6 @@ import { randomBytes } from 'node:crypto' import { afterEach, describe, expect, it, vi } from 'vitest' +import { createPasswordAuthProvider } from '../auth/password-provider.js' import { createPgliteDb, type Db } from '../db/client.js' import { migrate } from '../db/migrate.js' import { createGmailConnectService } from '../mail/gmail-connect.js' @@ -14,6 +15,7 @@ import type { OutboundEmail, QueueProvider, } from '../providers/index.js' +import { type AgentRecord, type AgentStore, createAgentStore } from '../store/agents.js' import { createThreadAttachmentStore, insertThreadAttachmentsInTx } from '../store/attachments.js' import { type ConversationStore, @@ -24,6 +26,7 @@ import { createGmailWatchStateStore } from '../store/gmail-watch-state.js' import { createMailboxTokenStore } from '../store/mailbox-tokens.js' import { createMailboxStore } from '../store/mailboxes.js' import { ENCRYPTION_KEY_BYTES } from '../store/token-crypto.js' +import type { AgentsApiDeps } from './agents.js' import type { GmailReconcileJob } from './gmail-webhook.js' import { createInboxApi, type InboxApiDeps } from './index.js' @@ -35,6 +38,20 @@ const SUPPORT_ADDRESS = 'support@example.test' const MAIL_DOMAIN = 'mail.example.test' const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +/** + * Build the REQUIRED `agents` deps (HT-54) for a `createInboxApi` call + * wired to `db` — a real PGlite-backed `AgentStore` plus the core + * `password` provider, matching how `src/composition/root.ts` wires them. + * None of these tests exercise `/agents/*`/`/auth/*` routes directly (that + * surface has its own describe block below), so this is just enough for + * `createInboxApi` to construct and for existing conversation routes to + * behave unchanged. + */ +function testAgentsDeps(db: Db): AgentsApiDeps { + const store = createAgentStore(db) + return { store, providers: [createPasswordAuthProvider({ agentStore: store })] } +} + /** A fake `EmailSender` that records every `OutboundEmail` it's asked to send, never fails. */ function createFakeSender(): { sender: EmailSender; sent: OutboundEmail[] } { const sent: OutboundEmail[] = [] @@ -214,6 +231,7 @@ describe('createInboxApi', () => { ): Promise<{ db: Db store: ConversationStore + agentStore: AgentStore api: (request: Request) => Promise /** Emails recorded by the default fake sender (empty if `overrides.sender` was supplied instead). */ sent: OutboundEmail[] @@ -221,6 +239,7 @@ describe('createInboxApi', () => { db = await createPgliteDb() await migrate(db) const store = createConversationStore(db) + const agentsDeps = testAgentsDeps(db) const { sender: defaultSender, sent } = createFakeSender() const api = createInboxApi({ store, @@ -229,6 +248,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: agentsDeps, ...(overrides.openTracking !== undefined ? { openTracking: overrides.openTracking } : {}), ...(overrides.gmailPush !== undefined ? { gmailPush: overrides.gmailPush } : {}), ...(overrides.gmailConnect !== undefined ? { gmailConnect: overrides.gmailConnect } : {}), @@ -241,7 +261,7 @@ describe('createInboxApi', () => { } : {}), }) - return { db, store, api, sent } + return { db, store, agentStore: agentsDeps.store, api, sent } } // --- auth ------------------------------------------------------------------ @@ -677,6 +697,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), }) const res = await api( @@ -783,6 +804,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), }) const res = await api( @@ -987,6 +1009,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), }) const res = await api( @@ -1526,6 +1549,31 @@ describe('createInboxApi', () => { return withJsonBody('PUT', path, JSON.stringify(body), tokenArg) } + /** Like {@link put}, additionally setting `X-Helpthread-Agent-Id` — the assignee route requires it (HT-54, spec §8). */ + function putWithAgent(path: string, body: unknown, agentId: string): Request { + const request = put(path, body) + const headers = new Headers(request.headers) + headers.set('X-Helpthread-Agent-Id', agentId) + return new Request(request.url, { + method: request.method, + headers, + body: JSON.stringify(body), + }) + } + + /** Create a real, active Agent directly via the store — the assignee tests need one both as the acting Agent and as a valid assignment target. */ + async function activeAgent(agentStore: AgentStore, email: string): Promise { + const result = await agentStore.createAgent({ + name: 'Test Agent', + email, + role: 'agent', + status: 'active', + passwordHash: 'scrypt$unused', + }) + if (!result.ok) throw new Error('expected ok') + return result.agent + } + it('PUT tags replaces the set, normalizing: trim, lowercase, dedupe preserving first occurrence', async () => { const { store, api } = await freshApi() const { conversationId } = await store.createConversation(newConversation()) @@ -1560,63 +1608,125 @@ describe('createInboxApi', () => { } }) - it('PUT assignee claims with me, releases with null; 400s otherwise (including a missing property)', async () => { - const { store, api } = await freshApi() + it('PUT assignee (HT-54 body shape) assigns to a real Agent id, releases with null; 400s otherwise', async () => { + const { store, agentStore, api } = await freshApi() const { conversationId } = await store.createConversation(newConversation()) + const acting = await activeAgent(agentStore, 'acting@example.test') + const assignee = await activeAgent(agentStore, 'assignee@example.test') const claimed = await api( - put(`/api/v1/conversations/${conversationId}/assignee`, { assignee: 'me' }), + putWithAgent( + `/api/v1/conversations/${conversationId}/assignee`, + { assigneeAgentId: assignee.id }, + acting.id, + ), ) expect(claimed.status).toBe(200) - expect(((await claimed.json()) as { assignee: string | null }).assignee).toBe('me') + expect(((await claimed.json()) as { assigneeAgentId: string | null }).assigneeAgentId).toBe( + assignee.id, + ) const released = await api( - put(`/api/v1/conversations/${conversationId}/assignee`, { assignee: null }), + putWithAgent( + `/api/v1/conversations/${conversationId}/assignee`, + { assigneeAgentId: null }, + acting.id, + ), ) expect(released.status).toBe(200) - expect(((await released.json()) as { assignee: string | null }).assignee).toBeNull() + expect( + ((await released.json()) as { assigneeAgentId: string | null }).assigneeAgentId, + ).toBeNull() - for (const bad of [{ assignee: 'someone' }, { assignee: 42 }, {}]) { - const res = await api(put(`/api/v1/conversations/${conversationId}/assignee`, bad)) + // Malformed shapes, and the OLD `{ assignee: 'me' }` body — all 400. + for (const bad of [ + { assigneeAgentId: 42 }, + {}, + { assignee: 'me' }, // the pre-HT-54 shape — no `assigneeAgentId` key at all + ]) { + const res = await api( + putWithAgent(`/api/v1/conversations/${conversationId}/assignee`, bad, acting.id), + ) expect(res.status).toBe(400) } + + // A syntactically-uuid-shaped id that names no real Agent is also 400 + // (validation_failed, no existence oracle beyond what any Agent can + // already see via GET /agents). + const nonexistent = await api( + putWithAgent( + `/api/v1/conversations/${conversationId}/assignee`, + { assigneeAgentId: RANDOM_UUID }, + acting.id, + ), + ) + expect(nonexistent.status).toBe(400) + }) + + it('PUT assignee requires the acting-Agent header — 401 without it, 401 for a disabled acting Agent', async () => { + const { store, agentStore, api } = await freshApi() + const { conversationId } = await store.createConversation(newConversation()) + const disabled = await activeAgent(agentStore, 'disabled@example.test') + await agentStore.updateAgent(disabled.id, { status: 'disabled' }) + + const noHeader = await api( + put(`/api/v1/conversations/${conversationId}/assignee`, { assigneeAgentId: null }), + ) + expect(noHeader.status).toBe(401) + + const disabledActing = await api( + putWithAgent( + `/api/v1/conversations/${conversationId}/assignee`, + { assigneeAgentId: null }, + disabled.id, + ), + ) + expect(disabledActing.status).toBe(401) }) it('both PUT routes 404 for missing, deleted, and non-UUID ids', async () => { - const { db, store, api } = await freshApi() + const { db, store, agentStore, api } = await freshApi() const { conversationId } = await store.createConversation(newConversation()) await setStatus(db, conversationId, 'deleted') + const acting = await activeAgent(agentStore, 'acting2@example.test') for (const suffix of ['tags', 'assignee'] as const) { - const body = suffix === 'tags' ? { tags: ['x'] } : { assignee: 'me' } - expect( - (await api(put(`/api/v1/conversations/${RANDOM_UUID}/${suffix}`, body))).status, - ).toBe(404) - expect( - (await api(put(`/api/v1/conversations/${conversationId}/${suffix}`, body))).status, - ).toBe(404) - expect((await api(put(`/api/v1/conversations/not-a-uuid/${suffix}`, body))).status).toBe( + const body = suffix === 'tags' ? { tags: ['x'] } : { assigneeAgentId: null } + const request = (path: string) => + suffix === 'tags' ? put(path, body) : putWithAgent(path, body, acting.id) + expect((await api(request(`/api/v1/conversations/${RANDOM_UUID}/${suffix}`))).status).toBe( 404, ) + expect( + (await api(request(`/api/v1/conversations/${conversationId}/${suffix}`))).status, + ).toBe(404) + expect((await api(request(`/api/v1/conversations/not-a-uuid/${suffix}`))).status).toBe(404) } }) - it('list summaries and the detail response carry tags and assignee', async () => { - const { store, api } = await freshApi() + it('list summaries and the detail response carry tags and assigneeAgentId', async () => { + const { store, agentStore, api } = await freshApi() const { conversationId } = await store.createConversation(newConversation()) + const assignee = await activeAgent(agentStore, 'assignee2@example.test') await store.setConversationTags(conversationId, ['bug']) - await store.setConversationAssignee(conversationId, 'me') + await store.setConversationAssignee(conversationId, assignee.id) const list = await api(get('/api/v1/conversations')) const listBody = (await list.json()) as { - conversations: Array<{ tags: string[]; assignee: string | null }> + conversations: Array<{ tags: string[]; assigneeAgentId: string | null }> } - expect(listBody.conversations[0]).toMatchObject({ tags: ['bug'], assignee: 'me' }) + expect(listBody.conversations[0]).toMatchObject({ + tags: ['bug'], + assigneeAgentId: assignee.id, + }) const detail = await api(get(`/api/v1/conversations/${conversationId}`)) - const detailBody = (await detail.json()) as { tags: string[]; assignee: string | null } + const detailBody = (await detail.json()) as { + tags: string[] + assigneeAgentId: string | null + } expect(detailBody.tags).toEqual(['bug']) - expect(detailBody.assignee).toBe('me') + expect(detailBody.assigneeAgentId).toBe(assignee.id) }) it('GET on the tags route is 405 with Allow: PUT; 401 without a token', async () => { @@ -1808,6 +1918,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), gmailPush: { verifySignature: async () => true, subscription: SUBSCRIPTION, @@ -1838,6 +1949,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), gmailPush: { verifySignature: async () => true, subscription: SUBSCRIPTION, @@ -1972,6 +2084,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), ...(gmailConnect !== undefined ? { gmailConnect } : {}), }) } @@ -2204,6 +2317,7 @@ describe('createInboxApi', () => { keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: testAgentsDeps(db), ...(gmailDisconnect !== undefined ? { gmailDisconnect } : {}), }) } @@ -2291,11 +2405,15 @@ describe('createInboxApi', () => { describe('createInboxApi — hardening (Codex review)', () => { const dummyStore = {} as unknown as ConversationStore const dummySender = createThrowingSender() + // None of these tests ever exercise an /agents/*|/auth/* route, so a + // never-invoked dummy AgentStore is fine — this block is purely about + // construction-time validation and the conversations-route error paths. const dummyDeps = { sender: dummySender, keyring: KEYRING, mailDomain: MAIL_DOMAIN, supportAddress: SUPPORT_ADDRESS, + agents: { store: {} as unknown as AgentStore, providers: [] } satisfies AgentsApiDeps, } it('throws at construction on an empty apiToken (fail closed — an empty token would authenticate every request)', () => { diff --git a/src/api/index.ts b/src/api/index.ts index aa73c23..8450cc7 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -35,6 +35,23 @@ import type { SelfEchoGuardDeps } from '../mail/send.js' import type { BlobStore, EmailSender } from '../providers/index.js' import type { ThreadAttachmentStore } from '../store/attachments.js' import type { ConversationStore } from '../store/conversations.js' +import { resolveActingAgent } from './acting-agent.js' +import { + type AgentsApiDeps, + type AgentsHandlerDeps, + handleAuthMe, + handleAuthProviders, + handleAuthVerify, + handleCreateAgent, + handleDeleteAgent, + handleGetAgent, + handleInviteAccept, + handleListAgents, + handlePatchAgent, + handleResendInvite, + handleSetAgentPassword, + handleSetup, +} from './agents.js' import { authenticateRequest } from './auth.js' import { handleDeleteConversation, @@ -91,6 +108,15 @@ export interface InboxApiDeps { mailDomain: string /** The deployment's configured support address — the `from` on every Agent reply (spec §4a). */ supportAddress: string + /** + * Agents & Authentication (HT-54; specs/auth/agents-and-auth.md) — REQUIRED, + * unlike every `?`-suffixed field below: this is core product surface, not + * an absent-by-default feature. See `src/api/agents.ts`'s `AgentsApiDeps` + * doc for why this is deliberately narrow (the invite path reuses this + * interface's own `keyring`/`sender`/`mailDomain`/`supportAddress` rather + * than duplicating them here). + */ + agents: AgentsApiDeps /** * Open tracking (spec §4g, v1.1 — HT-32): ABSENT BY DEFAULT — a deliberate * privacy stance, not an unset knob. When present, outbound replies get a @@ -168,6 +194,26 @@ export interface InboxApiDeps { selfEchoGuard?: SelfEchoGuardDeps } +/** + * Merge `deps.agents` with the top-level `keyring`/`sender`/`mailDomain`/ + * `supportAddress` fields every `InboxApiDeps` already carries, into the + * combined shape `src/api/agents.ts`'s handlers accept — see + * `AgentsHandlerDeps`'s doc for why `deps.agents` itself doesn't duplicate + * those fields. Called once per request, only by the two dispatch cases + * (`auth-invite-accept`'s success path needs `store`+`keyring`; + * `agents-create`/`agent-invite` need the full set for the invite-email + * path) that need more than `deps.agents` alone provides. + */ +function agentsHandlerDeps(deps: InboxApiDeps): AgentsHandlerDeps { + return { + ...deps.agents, + keyring: deps.keyring, + sender: deps.sender, + mailDomain: deps.mailDomain, + supportAddress: deps.supportAddress, + } +} + /** * Build the API's request handler. Returns a plain `(request: Request) => * Promise` — the entire deploy-time surface a Vercel/Next.js @@ -319,7 +365,16 @@ export function createInboxApi(deps: InboxApiDeps): (request: Request) => Promis return await handlePutTags(route.id, request, { store: deps.store }) case 'conversation-assignee': - return await handlePutAssignee(route.id, request, { store: deps.store }) + // The one existing inbox endpoint that now requires the + // acting-Agent header (spec §8) — any ACTIVE Agent may assign any + // Agent (spec §5), so resolveActingAgent's null → 401 is the whole + // authz check here; no role gate. + return await handlePutAssignee( + route.id, + await resolveActingAgent(request, deps.agents.store), + request, + { store: deps.store, agentStore: deps.agents.store }, + ) case 'conversation-reply': return await handleReply(route.id, request, { @@ -341,6 +396,78 @@ export function createInboxApi(deps: InboxApiDeps): (request: Request) => Promis return deps.gmailDisconnect !== undefined ? await handleGmailDisconnect(request, deps.gmailDisconnect) : apiError(404, 'not_found', 'No such route.') + + // --- Agents & Authentication (HT-54) -------------------------------- + // + // agentsDeps merges InboxApiDeps.agents (store/providers/uiBaseUrl) + // with the top-level keyring/sender/mailDomain/supportAddress every + // request already carries — see AgentsHandlerDeps's doc for why + // those aren't duplicated onto `deps.agents` itself. + + case 'auth-providers': + return await handleAuthProviders(deps.agents) + + case 'setup': + return await handleSetup(request, deps.agents) + + case 'auth-verify': + return await handleAuthVerify(request, deps.agents) + + case 'auth-me': + return handleAuthMe(await resolveActingAgent(request, deps.agents.store)) + + case 'auth-invite-accept': + return await handleInviteAccept(request, agentsHandlerDeps(deps)) + + case 'agents-list': + return await handleListAgents( + await resolveActingAgent(request, deps.agents.store), + deps.agents, + ) + + case 'agents-create': + return await handleCreateAgent( + await resolveActingAgent(request, deps.agents.store), + request, + agentsHandlerDeps(deps), + ) + + case 'agent-item': + return await handleGetAgent( + route.id, + await resolveActingAgent(request, deps.agents.store), + deps.agents, + ) + + case 'agent-patch': + return await handlePatchAgent( + route.id, + await resolveActingAgent(request, deps.agents.store), + request, + deps.agents, + ) + + case 'agent-delete': + return await handleDeleteAgent( + route.id, + await resolveActingAgent(request, deps.agents.store), + deps.agents, + ) + + case 'agent-password': + return await handleSetAgentPassword( + route.id, + await resolveActingAgent(request, deps.agents.store), + request, + deps.agents, + ) + + case 'agent-invite': + return await handleResendInvite( + route.id, + await resolveActingAgent(request, deps.agents.store), + agentsHandlerDeps(deps), + ) } } catch (err) { console.error('[inbox-api] unhandled error handling request', err) diff --git a/src/api/router.test.ts b/src/api/router.test.ts index 83033cd..ceefe78 100644 --- a/src/api/router.test.ts +++ b/src/api/router.test.ts @@ -101,6 +101,85 @@ describe('matchRoute', () => { expect(matchRoute('POST', '/api/v1/inbound/gmail/connect')).toEqual({ kind: 'gmail-connect' }) expect(matchRoute('POST', '/api/v1/inbound/gmail')).toEqual({ kind: 'not-found' }) }) + + // --- Agents & Authentication (HT-54) ---------------------------------------- + + it('matches GET /api/v1/auth/providers', () => { + expect(matchRoute('GET', '/api/v1/auth/providers')).toEqual({ kind: 'auth-providers' }) + expect(matchRoute('POST', '/api/v1/auth/providers')).toEqual({ + kind: 'method-not-allowed', + allow: ['GET'], + }) + }) + + it('matches POST /api/v1/setup', () => { + expect(matchRoute('POST', '/api/v1/setup')).toEqual({ kind: 'setup' }) + }) + + it('matches POST /api/v1/auth/verify', () => { + expect(matchRoute('POST', '/api/v1/auth/verify')).toEqual({ kind: 'auth-verify' }) + }) + + it('matches GET /api/v1/auth/me', () => { + expect(matchRoute('GET', '/api/v1/auth/me')).toEqual({ kind: 'auth-me' }) + }) + + it('matches POST /api/v1/auth/invite/accept — a distinct prefix (/auth/) from /agents/{id}/invite, never confused', () => { + expect(matchRoute('POST', '/api/v1/auth/invite/accept')).toEqual({ kind: 'auth-invite-accept' }) + }) + + it('matches GET/POST /api/v1/agents as list/create', () => { + expect(matchRoute('GET', '/api/v1/agents')).toEqual({ kind: 'agents-list' }) + expect(matchRoute('POST', '/api/v1/agents')).toEqual({ kind: 'agents-create' }) + expect(matchRoute('DELETE', '/api/v1/agents')).toEqual({ + kind: 'method-not-allowed', + allow: ['GET', 'POST'], + }) + }) + + it('matches GET/PATCH/DELETE /api/v1/agents/{id}, extracting the id', () => { + expect(matchRoute('GET', '/api/v1/agents/abc-123')).toEqual({ + kind: 'agent-item', + id: 'abc-123', + }) + expect(matchRoute('PATCH', '/api/v1/agents/abc-123')).toEqual({ + kind: 'agent-patch', + id: 'abc-123', + }) + expect(matchRoute('DELETE', '/api/v1/agents/abc-123')).toEqual({ + kind: 'agent-delete', + id: 'abc-123', + }) + expect(matchRoute('PUT', '/api/v1/agents/abc-123')).toEqual({ + kind: 'method-not-allowed', + allow: ['GET', 'PATCH', 'DELETE'], + }) + }) + + it('matches POST /api/v1/agents/{id}/password, distinct from the item route', () => { + expect(matchRoute('POST', '/api/v1/agents/abc-123/password')).toEqual({ + kind: 'agent-password', + id: 'abc-123', + }) + expect(matchRoute('GET', '/api/v1/agents/abc-123/password')).toEqual({ + kind: 'method-not-allowed', + allow: ['POST'], + }) + }) + + it('matches POST /api/v1/agents/{id}/invite, distinct from the item route and /password', () => { + expect(matchRoute('POST', '/api/v1/agents/abc-123/invite')).toEqual({ + kind: 'agent-invite', + id: 'abc-123', + }) + }) + + it("agent item route never matches a /password or /invite suffix (anchored, mirrors CONVERSATION_ITEM's own anchoring)", () => { + expect(matchRoute('GET', '/api/v1/agents/abc-123/password')).not.toEqual({ + kind: 'agent-item', + id: 'abc-123/password', + }) + }) }) describe('matchGmailPushWebhook', () => { diff --git a/src/api/router.ts b/src/api/router.ts index 0c5251c..1e18b9e 100644 --- a/src/api/router.ts +++ b/src/api/router.ts @@ -89,6 +89,67 @@ const GMAIL_DISCONNECT: RouteDef = { methods: ['POST'], } +// --- Agents & Authentication (HT-54; specs/auth/agents-and-auth.md §6) ----- +// +// All still Bearer-gated ordinary routes (spec §6: "All under the existing +// service-bearer channel") — the acting-Agent header is a SEPARATE, +// per-endpoint check the handlers perform themselves +// (`src/api/acting-agent.ts`), not something this matcher is aware of. + +/** `/api/v1/auth/providers` — GET only, no acting-Agent header (spec §6, §8). */ +const AUTH_PROVIDERS: RouteDef = { + pattern: /^\/api\/v1\/auth\/providers$/, + methods: ['GET'], +} + +/** `/api/v1/setup` — the zero-Agents-gated first-admin bootstrap (spec §6), POST only, no acting-Agent header (spec §8's pre-session carve-out). */ +const SETUP: RouteDef = { + pattern: /^\/api\/v1\/setup$/, + methods: ['POST'], +} + +/** `/api/v1/auth/verify` — dispatch to a registered `AuthProvider` (spec §6), POST only, no acting-Agent header (pre-session). */ +const AUTH_VERIFY: RouteDef = { + pattern: /^\/api\/v1\/auth\/verify$/, + methods: ['POST'], +} + +/** `/api/v1/auth/me` — the acting Agent (spec §6), GET only, acting-Agent header REQUIRED. */ +const AUTH_ME: RouteDef = { + pattern: /^\/api\/v1\/auth\/me$/, + methods: ['GET'], +} + +/** `/api/v1/auth/invite/accept` — validate an invite token and activate (spec §6), POST only, no acting-Agent header (pre-session — no session exists yet). Anchored so it never collides with `AGENT_INVITE`'s `/agents/{id}/invite`. */ +const AUTH_INVITE_ACCEPT: RouteDef = { + pattern: /^\/api\/v1\/auth\/invite\/accept$/, + methods: ['POST'], +} + +/** `/api/v1/agents` — list (any active Agent, per the coordinator's roster-visibility amendment) and create (admin), acting-Agent header REQUIRED on both. */ +const AGENTS_LIST: RouteDef = { + pattern: /^\/api\/v1\/agents$/, + methods: ['GET', 'POST'], +} + +/** `/api/v1/agents/{id}` — get (admin or self), patch (admin for anyone, self for own name/timezone), hard delete (admin) — spec §6. Anchored `[^/]+$` so it never matches a `.../password` or `.../invite` suffix, mirroring `CONVERSATION_ITEM`'s own anchoring. */ +const AGENT_ITEM: RouteDef = { + pattern: /^\/api\/v1\/agents\/(?[^/]+)$/, + methods: ['GET', 'PATCH', 'DELETE'], +} + +/** `/api/v1/agents/{id}/password` — set/replace a password (self, or admin reset) — spec §6, POST only. */ +const AGENT_PASSWORD: RouteDef = { + pattern: /^\/api\/v1\/agents\/(?[^/]+)\/password$/, + methods: ['POST'], +} + +/** `/api/v1/agents/{id}/invite` — (re)send an invite (admin) — spec §6, POST only. */ +const AGENT_INVITE: RouteDef = { + pattern: /^\/api\/v1\/agents\/(?[^/]+)\/invite$/, + methods: ['POST'], +} + /** Every route this API recognizes, checked in order. */ const ROUTES: readonly RouteDef[] = [ CONVERSATIONS_LIST, @@ -99,6 +160,15 @@ const ROUTES: readonly RouteDef[] = [ CONVERSATION_ASSIGNEE, GMAIL_CONNECT, GMAIL_DISCONNECT, + AUTH_PROVIDERS, + SETUP, + AUTH_VERIFY, + AUTH_ME, + AUTH_INVITE_ACCEPT, + AGENTS_LIST, + AGENT_PASSWORD, + AGENT_INVITE, + AGENT_ITEM, ] /** The outcome of matching a `(method, pathname)` pair against {@link ROUTES}. */ @@ -113,6 +183,18 @@ export type RouteMatch = | { kind: 'conversation-assignee'; id: string } | { kind: 'gmail-connect' } | { kind: 'gmail-disconnect' } + | { kind: 'auth-providers' } + | { kind: 'setup' } + | { kind: 'auth-verify' } + | { kind: 'auth-me' } + | { kind: 'auth-invite-accept' } + | { kind: 'agents-list' } + | { kind: 'agents-create' } + | { kind: 'agent-item'; id: string } + | { kind: 'agent-patch'; id: string } + | { kind: 'agent-delete'; id: string } + | { kind: 'agent-password'; id: string } + | { kind: 'agent-invite'; id: string } | { kind: 'method-not-allowed'; allow: string[] } | { kind: 'not-found' } @@ -217,9 +299,27 @@ export function matchRoute(method: string, pathname: string): RouteMatch { if (route === GMAIL_DISCONNECT) { return { kind: 'gmail-disconnect' } } + if (route === AUTH_PROVIDERS) { + return { kind: 'auth-providers' } + } + if (route === SETUP) { + return { kind: 'setup' } + } + if (route === AUTH_VERIFY) { + return { kind: 'auth-verify' } + } + if (route === AUTH_ME) { + return { kind: 'auth-me' } + } + if (route === AUTH_INVITE_ACCEPT) { + return { kind: 'auth-invite-accept' } + } + if (route === AGENTS_LIST) { + return method === 'GET' ? { kind: 'agents-list' } : { kind: 'agents-create' } + } - // Both CONVERSATION_ITEM and CONVERSATION_REPLIES guarantee a present, - // non-empty `id` group (per their `[^/]+` pattern) whenever they matched. + // Every remaining route guarantees a present, non-empty `id` group (per + // its `[^/]+` pattern) whenever it matched. const id = match.groups?.id as string if (route === CONVERSATION_REPLIES) { @@ -234,6 +334,17 @@ export function matchRoute(method: string, pathname: string): RouteMatch { if (route === CONVERSATION_ASSIGNEE) { return { kind: 'conversation-assignee', id } } + if (route === AGENT_PASSWORD) { + return { kind: 'agent-password', id } + } + if (route === AGENT_INVITE) { + return { kind: 'agent-invite', id } + } + if (route === AGENT_ITEM) { + if (method === 'GET') return { kind: 'agent-item', id } + if (method === 'DELETE') return { kind: 'agent-delete', id } + return { kind: 'agent-patch', id } + } // route === CONVERSATION_ITEM: GET reads, PATCH updates status, DELETE // soft-deletes (spec §4d, v1.1). if (method === 'GET') return { kind: 'conversation-item', id } diff --git a/src/auth/invite-email.test.ts b/src/auth/invite-email.test.ts new file mode 100644 index 0000000..b35c419 --- /dev/null +++ b/src/auth/invite-email.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from 'vitest' +import { buildInviteEmail } from './invite-email.js' + +describe('buildInviteEmail', () => { + it('builds an email to the invitee, from the support address, with the accept link', () => { + const email = buildInviteEmail({ + to: 'invitee@example.test', + token: 'hti.k1.payload.sig', + uiBaseUrl: 'https://desk.example.test', + supportAddress: 'support@example.test', + mailDomain: 'mail.example.test', + }) + expect(email.to).toEqual(['invitee@example.test']) + expect(email.from).toBe('support@example.test') + expect(email.subject).toContain("You're invited") + expect(email.text).toContain('https://desk.example.test/invite/hti.k1.payload.sig') + }) + + it('mints a bare, non-reply-token messageId, scoped to mailDomain', () => { + const email = buildInviteEmail({ + to: 'invitee@example.test', + token: 'hti.k1.payload.sig', + uiBaseUrl: 'https://desk.example.test', + supportAddress: 'support@example.test', + mailDomain: 'mail.example.test', + }) + expect(email.messageId).toMatch(/^$/) + // Never shaped like a reply token (ht.-prefixed local part). + expect(email.messageId.includes('ht.')).toBe(false) + }) + + it('two builds for the same invite mint DIFFERENT messageIds (fresh uuid per call)', () => { + const input = { + to: 'invitee@example.test', + token: 'hti.k1.payload.sig', + uiBaseUrl: 'https://desk.example.test', + supportAddress: 'support@example.test', + mailDomain: 'mail.example.test', + } + const a = buildInviteEmail(input) + const b = buildInviteEmail(input) + expect(a.messageId).not.toBe(b.messageId) + }) + + it('never sets inReplyTo/references — this is not a reply', () => { + const email = buildInviteEmail({ + to: 'invitee@example.test', + token: 'hti.k1.payload.sig', + uiBaseUrl: 'https://desk.example.test', + supportAddress: 'support@example.test', + mailDomain: 'mail.example.test', + }) + expect(email.inReplyTo).toBeUndefined() + expect(email.references).toBeUndefined() + }) +}) diff --git a/src/auth/invite-email.ts b/src/auth/invite-email.ts new file mode 100644 index 0000000..fb95f6f --- /dev/null +++ b/src/auth/invite-email.ts @@ -0,0 +1,62 @@ +/** + * Build the invite email `OutboundEmail` (HT-54; specs/auth/agents-and-auth.md + * §8) — the ONE place this feature constructs a message for the core + * `EmailSender` transport (`src/providers/email-sender.ts`). + * + * ## This is NOT `sendReply`/`src/mail/send.ts` + * + * An invite has no conversation, no thread, no reply-token, no delivery + * lease — routing it through `sendReply` would mint bogus `threads`/ + * `send_envelope` rows for something that isn't a reply to anything (spec + * §8's explicit call-out). This module builds a fresh {@link OutboundEmail} + * and the caller (`src/api/agents.ts`) hands it directly to the configured + * `EmailSender`. + * + * ## `messageId` is a bare id, not a reply token + * + * `` — NOT `ht.`-shaped (`mintReplyMessageId`), + * carries no signature, and nothing ever routes on it (there is no inbound + * reply to an invite email that needs threading back to anything). A bare + * RFC 5322 Message-ID is exactly what a message with no threading identity + * needs, and reusing the reply-token format here would falsely imply this + * id has threading authority it does not. + */ + +import { randomUUID } from 'node:crypto' +import type { OutboundEmail } from '../providers/email-sender.js' + +/** Input to {@link buildInviteEmail}. */ +export interface InviteEmailInput { + /** The invited Agent's email address. */ + to: string + /** The signed invite token (`mintInviteToken`, `src/auth/invite-token.ts`) — embedded verbatim in the accept link. */ + token: string + /** The web UI's base URL (`HELPTHREAD_UI_BASE_URL`) — the link is `${uiBaseUrl}/invite/${token}`. */ + uiBaseUrl: string + /** The deployment's configured support address — the `from` on this email, matching every other Agent-facing message this engine sends. */ + supportAddress: string + /** Domain minted into the bare `Message-ID` — matches every other outbound message's `@domain` part. */ + mailDomain: string +} + +/** + * Build the invite email. Text-only (per the brief: "do NOT fabricate + * tracking or styling") — a minimal, professional message using Agent/Team + * vocabulary (CLAUDE.md), never "user". + */ +export function buildInviteEmail(input: InviteEmailInput): OutboundEmail { + const link = `${input.uiBaseUrl}/invite/${input.token}` + return { + messageId: ``, + from: input.supportAddress, + to: [input.to], + subject: `You're invited to join ${input.mailDomain} on Helpthread`, + text: [ + "You've been invited to join the support team on Helpthread.", + '', + `Accept your invite: ${link}`, + '', + "If you weren't expecting this invite, you can safely ignore this email.", + ].join('\n'), + } +} diff --git a/src/auth/invite-token.test.ts b/src/auth/invite-token.test.ts new file mode 100644 index 0000000..b693593 --- /dev/null +++ b/src/auth/invite-token.test.ts @@ -0,0 +1,130 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mintConnectState } from '../mail/gmail-connect.js' +import { type Keyring, mintReplyMessageId } from '../mail/reply-token.js' +import { DEFAULT_INVITE_TOKEN_TTL_MS, mintInviteToken, verifyInviteToken } from './invite-token.js' + +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const AGENT_ID = '00000000-0000-4000-8000-000000000000' + +describe('mintInviteToken / verifyInviteToken', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('a freshly minted token verifies and recovers the agentId', () => { + const token = mintInviteToken(AGENT_ID, KEYRING) + expect(verifyInviteToken(token, KEYRING)).toEqual({ agentId: AGENT_ID }) + }) + + it('is shaped hti.{keyId}.{payload}.{sig} — four dot-separated segments, hti prefix', () => { + const token = mintInviteToken(AGENT_ID, KEYRING) + const segments = token.split('.') + expect(segments).toHaveLength(4) + expect(segments[0]).toBe('hti') + expect(segments[1]).toBe('k1') + }) + + it('mintInviteToken throws for a non-uuid agentId', () => { + expect(() => mintInviteToken('not-a-uuid', KEYRING)).toThrow() + }) + + it('mintInviteToken throws for a malformed keyring', () => { + expect(() => + mintInviteToken(AGENT_ID, { current: { keyId: 'k1', secret: 'too-short' } }), + ).toThrow() + }) + + it('a forged signature does not verify', () => { + const segments = mintInviteToken(AGENT_ID, KEYRING).split('.') + segments[3] = 'A'.repeat(segments[3].length) + expect(verifyInviteToken(segments.join('.'), KEYRING)).toBeNull() + }) + + it('a tampered payload does not verify (the payload is part of the signed canonical string)', () => { + const otherAgentId = '11111111-1111-4111-8111-111111111111' + const forged = mintInviteToken(otherAgentId, KEYRING) + const segments = mintInviteToken(AGENT_ID, KEYRING).split('.') + const forgedSegments = forged.split('.') + // Splice a different agent's payload onto this token's signature. + segments[2] = forgedSegments[2] + expect(verifyInviteToken(segments.join('.'), KEYRING)).toBeNull() + }) + + it('expires past the TTL (default 72 hours)', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z')) + const token = mintInviteToken(AGENT_ID, KEYRING) + + vi.setSystemTime(new Date(Date.now() + DEFAULT_INVITE_TOKEN_TTL_MS - 1000)) + expect(verifyInviteToken(token, KEYRING)).toEqual({ agentId: AGENT_ID }) + + vi.setSystemTime(new Date(Date.now() + 2000)) + expect(verifyInviteToken(token, KEYRING)).toBeNull() + }) + + it('respects a custom ttlMs', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z')) + const token = mintInviteToken(AGENT_ID, KEYRING) + + vi.setSystemTime(new Date('2026-01-01T00:00:30.000Z')) + expect(verifyInviteToken(token, KEYRING, 60_000)).toEqual({ agentId: AGENT_ID }) + expect(verifyInviteToken(token, KEYRING, 10_000)).toBeNull() + }) + + it('verifyInviteToken is TOTAL over garbage input — never throws, always null', () => { + const garbage = [ + '', + 'not-a-token', + 'hti.only.two', + 'hti..payload.sig', + 'gmc.k1.123.nonce.sig', // a connect-state token, wrong shape entirely + 'ht.k1.c1.t1.sig@mail.example.test', // a reply token local part, wrong shape + ] + for (const value of garbage) { + expect(() => verifyInviteToken(value, KEYRING)).not.toThrow() + expect(verifyInviteToken(value, KEYRING)).toBeNull() + } + }) + + it('rejects an unknown keyId', () => { + const token = mintInviteToken(AGENT_ID, KEYRING) + const otherKeyring: Keyring = { current: { keyId: 'other', secret: 'b'.repeat(32) } } + expect(verifyInviteToken(token, otherKeyring)).toBeNull() + }) + + it('an hti. signature never verifies as a gmc. connect-state token, and vice versa (domain separation)', () => { + // Same keyring, same underlying secret material — the prefix embedded in + // the signed bytes is what keeps the two token types from ever + // cross-verifying, not merely a difference in which keys are configured. + const inviteToken = mintInviteToken(AGENT_ID, KEYRING) + const connectState = mintConnectState(KEYRING) + + // A gmc. token is the wrong shape for verifyInviteToken (5 segments, + // wrong prefix) — rejected structurally, before any signature check. + expect(verifyInviteToken(connectState, KEYRING)).toBeNull() + + // Splice the gmc. token's payload+sig onto an hti. prefix: still fails, + // because the SIGNED bytes for a gmc. token never included the literal + // "hti." this verifier requires as part of the canonical string. + const gmcSegments = connectState.split('.') + const frankensteined = `hti.${gmcSegments[1]}.${gmcSegments[2]}.${gmcSegments[4]}` + expect(verifyInviteToken(frankensteined, KEYRING)).toBeNull() + + // And a genuine hti. token is the wrong shape (4 segments) for a reply + // token / connect-state verifier expecting 5 — asserted here structurally + // via segment count, since this module doesn't import those verifiers. + expect(inviteToken.split('.')).toHaveLength(4) + expect(connectState.split('.')).toHaveLength(5) + }) + + it('an hti. signature never verifies as an ht. reply token (domain separation)', () => { + const replyMessageId = mintReplyMessageId( + { conversationId: 'c1', threadId: 't1', mailDomain: 'mail.example.test' }, + KEYRING, + ) + // Not even the right shape (angle-bracketed, @domain suffix) — rejected + // outright by verifyInviteToken's structural parse. + expect(verifyInviteToken(replyMessageId, KEYRING)).toBeNull() + }) +}) diff --git a/src/auth/invite-token.ts b/src/auth/invite-token.ts new file mode 100644 index 0000000..ea55f00 --- /dev/null +++ b/src/auth/invite-token.ts @@ -0,0 +1,191 @@ +/** + * Signed invite tokens (HT-54; specs/auth/agents-and-auth.md §8, §9) — the + * credential a `POST /api/v1/agents` (`sendInvite: true`) or `POST + * /api/v1/agents/{id}/invite` call mints and emails as `/invite/{token}`. + * + * Mirrors `src/mail/gmail-connect.ts`'s `gmc.` connect-state token: the same + * stateless, server-session-free HMAC pattern off the same {@link Keyring} + * (full HMAC-SHA256, base64url, current+retired key rotation, constant-time + * verification) — a natural fit for a serverless deployment with no session + * store, reused here for a different domain. `hti.` is the domain + * separator — distinct from reply tokens' `ht.`, view tokens' `v`, and + * connect state's `gmc.` — so a signature minted for one purpose can never + * verify as another (spec §8's explicit requirement). + * + * ## Token format + * + * ``` + * hti.{keyId}.{payload-b64url}.{sig-b64url} + * ``` + * + * Unlike the `gmc.`/`ht.` tokens (which sign a handful of dot-separated + * scalar fields), this token's payload is a small JSON object + * (`{ agentId, issuedAtMs, nonce }`) base64url-encoded as ONE segment — + * simpler than adding a fourth scalar field to a dot-separated scheme, and + * there is no risk of a `.`-containing value (an `agentId` is a uuid, never + * containing `.`) colliding with the delimiter here since the payload is a + * single opaque segment either way. + * + * `sig = base64url( HMAC-SHA256( secret, "hti.{keyId}.{payload-b64url}" ) )` + * — the literal `hti.` prefix is part of the SIGNED bytes (not just the + * wire format), which is what makes an `hti.` signature structurally unable + * to verify against a `gmc.`/`ht.` token's secret-and-canonical-string pair + * even where key material happened to be shared (it is not, in practice — + * each token type is minted off the same `Keyring` object, but the + * domain-separated canonical string is the actual guarantee, not an + * assumption about key reuse). + * + * ## One-time-ness is NOT a token property + * + * Unlike a nonce-tracked single-use token, nothing here records "this token + * was already used." Replay-safety comes from the atomic `invited` → + * `active` status transition the accept endpoint performs + * (`AgentStore.acceptInvite`, spec §6/§9): a second accept of the same + * token finds the Agent no longer `invited` and affects zero rows, + * regardless of whether the token itself still "verifies" cryptographically. + * This module's job ends at "is this a genuine, unexpired invite for this + * `agentId`" — the store is what makes it single-use in effect. + * + * ## Security properties (mirrors reply-token.ts / gmail-connect.ts) + * + * - {@link mintInviteToken} is STRICT — throws on a malformed keyring or a + * non-uuid-shaped `agentId` (a deploy-time/programmer bug, fail loud). + * - {@link verifyInviteToken} is TOTAL over `token` (the untrusted input a + * customer-facing accept endpoint receives verbatim) — every rejection + * path returns `null`, never throws. + * - TTL default 72 hours (spec says "short-lived"; pinned here per the + * HT-54 implementation brief — long enough that an invite sent on a + * Friday is still good Monday, short enough that a stale, unaccepted + * invite eventually stops being a live credential). + */ + +import { createHmac, randomBytes, timingSafeEqual } from 'node:crypto' +import { assertValidKeyring, type Keyring, type SigningKey } from '../mail/reply-token.js' + +/** Fixed literal prefix marking a token as one of this module's invite tokens — the domain separator (module doc). */ +const TOKEN_PREFIX = 'hti' + +/** Number of dot-separated segments in a well-formed token: `hti`, keyId, payload, sig. */ +const SEGMENT_COUNT = 4 + +/** Random nonce size (bytes) minted into every invite payload — belt-and-suspenders alongside `issuedAtMs`; not relied on for one-time-ness (module doc). */ +const NONCE_BYTES = 16 + +/** Default invite token TTL: 72 hours (module doc — pinned by the implementation brief; spec only says "short-lived"). */ +export const DEFAULT_INVITE_TOKEN_TTL_MS = 72 * 60 * 60 * 1000 + +/** The uuid-shape check for `agentId` at mint time — mirrors `src/api/uuid.ts`, duplicated locally so this module has no dependency on `src/api/**` (an auth seam should not depend on the HTTP layer). */ +const UUID_PATTERN = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/ + +/** The payload signed and carried inside an invite token. */ +interface InviteTokenPayload { + agentId: string + issuedAtMs: number + nonce: string +} + +/** The exact bytes signed: `hti.{keyId}.{payload-b64url}` — see the module doc on why the prefix is part of the signed string. */ +function canonicalString(keyId: string, payloadB64: string): string { + return `${TOKEN_PREFIX}.${keyId}.${payloadB64}` +} + +function sign(secret: string, canonical: string): string { + return createHmac('sha256', secret).update(canonical).digest('base64url') +} + +/** + * Mint an invite token for `agentId`, signing with `keyring.current`. + * STRICT: throws if `keyring` is malformed ({@link assertValidKeyring}) or + * `agentId` is not a well-formed uuid — emitting a token for a bogus id + * would be a programmer error, not something to silently tolerate. + */ +export function mintInviteToken(agentId: string, keyring: Keyring): string { + assertValidKeyring(keyring) + if (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId)) { + throw new Error(`mintInviteToken: agentId must be a uuid (got ${JSON.stringify(agentId)})`) + } + + const { keyId, secret } = keyring.current + const payload: InviteTokenPayload = { + agentId, + issuedAtMs: Date.now(), + nonce: randomBytes(NONCE_BYTES).toString('base64url'), + } + const payloadB64 = Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url') + const sig = sign(secret, canonicalString(keyId, payloadB64)) + return `${TOKEN_PREFIX}.${keyId}.${payloadB64}.${sig}` +} + +/** + * Verify a candidate invite token: well-formed, correctly signed by a known + * (current or retired) key, and minted no more than `ttlMs` ago + * (default {@link DEFAULT_INVITE_TOKEN_TTL_MS}). TOTAL over `token` — never + * throws; every rejection is `null`. `keyring` is trusted deploy-time + * configuration and still fails loudly if malformed ({@link + * assertValidKeyring}) — mirrors `verifyReplyMessageId`/`verifyConnectState`, + * whose doc comments explain why that does not weaken totality over the + * untrusted argument. + * + * Signature verification happens BEFORE the payload is ever JSON-parsed: + * tampering with `payloadB64` changes the signed bytes themselves, so a + * forged/tampered payload always fails the signature check first — this + * function never hands an attacker a "here's whether your JSON parsed" + * oracle independent of the signature. + */ +export function verifyInviteToken( + token: string, + keyring: Keyring, + ttlMs: number = DEFAULT_INVITE_TOKEN_TTL_MS, +): { agentId: string } | null { + assertValidKeyring(keyring) + + if (typeof token !== 'string') return null + const segments = token.split('.') + if (segments.length !== SEGMENT_COUNT) return null + const [prefix, keyId, payloadB64, sig] = segments + if (prefix !== TOKEN_PREFIX) return null + if (keyId.length === 0 || payloadB64.length === 0 || sig.length === 0) return null + + const canonical = canonicalString(keyId, payloadB64) + let signatureOk = false + for (const key of candidateKeys(keyring, keyId)) { + if (signatureMatches(key.secret, canonical, sig)) { + signatureOk = true + break + } + } + if (!signatureOk) return null + + let parsed: unknown + try { + parsed = JSON.parse(Buffer.from(payloadB64, 'base64url').toString('utf8')) + } catch { + return null + } + if (typeof parsed !== 'object' || parsed === null) return null + const { agentId, issuedAtMs, nonce } = parsed as Record + + if (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId)) return null + if (typeof nonce !== 'string' || nonce.length === 0) return null + + const now = Date.now() + if (typeof issuedAtMs !== 'number' || !Number.isFinite(issuedAtMs) || issuedAtMs < 0) return null + if (issuedAtMs > now) return null + if (now - issuedAtMs > ttlMs) return null + + return { agentId } +} + +/** Keys in the ring (current first, then retired) whose keyId matches the token's. Same helper reply-token.ts/gmail-connect.ts each keep a local copy of. */ +function candidateKeys(keyring: Keyring, keyId: string): SigningKey[] { + const all = keyring.retired ? [keyring.current, ...keyring.retired] : [keyring.current] + return all.filter((key) => key.keyId === keyId) +} + +/** Constant-time signature check — same length-guarded pattern as reply-token.ts/gmail-connect.ts. */ +function signatureMatches(secret: string, canonical: string, providedSig: string): boolean { + const expected = Buffer.from(sign(secret, canonical)) + const provided = Buffer.from(providedSig) + if (expected.length !== provided.length) return false + return timingSafeEqual(expected, provided) +} diff --git a/src/auth/password-hash.test.ts b/src/auth/password-hash.test.ts new file mode 100644 index 0000000..b5600aa --- /dev/null +++ b/src/auth/password-hash.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest' +import { DUMMY_HASH, hashPassword, verifyPassword } from './password-hash.js' + +describe('hashPassword / verifyPassword', () => { + it('round-trips: the correct password verifies', () => { + const encoded = hashPassword('correct horse battery staple') + expect(verifyPassword('correct horse battery staple', encoded)).toBe(true) + }) + + it('rejects the wrong password', () => { + const encoded = hashPassword('correct horse battery staple') + expect(verifyPassword('wrong password', encoded)).toBe(false) + }) + + it('is encoded as scrypt$N=16384,r=8,p=1$$ — four $-separated segments', () => { + const encoded = hashPassword('hunter2') + const segments = encoded.split('$') + expect(segments).toHaveLength(4) + expect(segments[0]).toBe('scrypt') + expect(segments[1]).toBe('N=16384,r=8,p=1') + expect(segments[2].length).toBeGreaterThan(0) + expect(segments[3].length).toBeGreaterThan(0) + }) + + it('two hashes of the SAME password never match byte-for-byte (random salt per call)', () => { + const a = hashPassword('same password') + const b = hashPassword('same password') + expect(a).not.toBe(b) + expect(verifyPassword('same password', a)).toBe(true) + expect(verifyPassword('same password', b)).toBe(true) + }) + + it('verifyPassword is TOTAL over a malformed encoded value — never throws, always false', () => { + const malformed = [ + '', + 'not-our-format', + 'scrypt$onlytwo$segments', + 'scrypt$N=16384,r=8,p=1$$', // empty salt and hash + 'scrypt$N=16384,r=8,p=1$onlysalt$', + 'bcrypt$N=16384,r=8,p=1$c2FsdA$aGFzaA', // wrong prefix + 'scrypt$bogus-params$c2FsdA$aGFzaA', + 'scrypt$N=0,r=8,p=1$c2FsdA$aGFzaA', // N must be positive + 'scrypt$N=16384,r=8,p=1$not!!valid!!base64url$aGFzaA', + // Decode-time cost ceilings: a syntactically valid tuple must not be + // able to buy unbounded scrypt work (attacker/corruption-controlled + // stored value) — capped params or oversized digests fail fast. + 'scrypt$N=2097152,r=8,p=1$c2FsdA$aGFzaA', // N over the 2^20 ceiling + 'scrypt$N=16384,r=64,p=1$c2FsdA$aGFzaA', // r over ceiling + 'scrypt$N=16384,r=8,p=32$c2FsdA$aGFzaA', // p over ceiling + `scrypt$N=16384,r=8,p=1$c2FsdA$${Buffer.alloc(256).toString('base64url')}`, // hash over 128 bytes + `scrypt$N=16384,r=8,p=1$${Buffer.alloc(96).toString('base64url')}$aGFzaA`, // salt over 64 bytes + ] + for (const value of malformed) { + expect(() => verifyPassword('anything', value)).not.toThrow() + expect(verifyPassword('anything', value)).toBe(false) + } + }) + + it('DUMMY_HASH is a real, verifiable hash — used for timing-comparable rejection of unknown emails', () => { + expect(DUMMY_HASH.split('$')).toHaveLength(4) + expect(DUMMY_HASH.startsWith('scrypt$')).toBe(true) + // Nobody knows the random string it was hashed from — verifying any + // guess against it must fail, exercising the SAME scrypt cost as a real + // verification (that's the whole point: comparable timing). + expect(verifyPassword('whatever an attacker might guess', DUMMY_HASH)).toBe(false) + }) +}) diff --git a/src/auth/password-hash.ts b/src/auth/password-hash.ts new file mode 100644 index 0000000..29da570 --- /dev/null +++ b/src/auth/password-hash.ts @@ -0,0 +1,183 @@ +/** + * Password hashing for `agent_auth_identities.secret_hash` (HT-54; + * specs/auth/agents-and-auth.md §9). + * + * scrypt (`node:crypto`, no new dependency), per-identity random salt, fixed + * explicit cost parameters. CodeQL's `js/insufficient-password-hash` rejects + * bare SHA-256 AND keyed HMAC (learned on HT-51, where a keyed-HMAC compare + * against a single shared operator password still tripped it) — scrypt is + * what actually satisfies the check. Unlike HT-51 (which held a slow KDF + * over a `crypto.timingSafeEqual`-length-blind comparison against a + * plaintext env value, so the KDF's slowness was almost cosmetic), there is + * now a REAL hash at rest, so the memory-hard cost genuinely matters. + * + * ## Encoded format + * + * One self-describing string, so a future cost-parameter bump never breaks + * verifying an already-stored hash: + * + * ``` + * scrypt$N=16384,r=8,p=1$$ + * ``` + * + * `decode` is TOTAL — the same totality bar `verifyReplyMessageId` + * (`src/mail/reply-token.ts`) holds an untrusted-input parser to: a + * malformed or corrupted `secret_hash` value must make {@link verifyPassword} + * return `false`, never throw. A stored hash is not attacker-controlled in + * the way a Message-ID is, but treating it as untrusted costs nothing and + * means a DB-level corruption degrades to "this password doesn't match" + * rather than a 500. + */ + +import { randomBytes, scryptSync, timingSafeEqual } from 'node:crypto' + +/** + * The longest password any entry point will feed to the KDF, shared by the + * API's validation (`src/api/agents.ts`) and the login path + * (`src/auth/password-provider.ts`) so no caller can push unbounded input + * into scrypt. Defined here — next to the KDF it bounds — because the two + * enforcement sites must never drift apart. + */ +export const MAX_PASSWORD_LENGTH = 256 + +/** scrypt cost parameter — CPU/memory cost, a power of two. Fixed and explicit (spec §9), not derived from any env. */ +const SCRYPT_N = 16384 +/** scrypt block size. */ +const SCRYPT_R = 8 +/** scrypt parallelization factor. */ +const SCRYPT_P = 1 +/** Derived key length, in bytes. */ +const KEY_LENGTH = 32 +/** Random salt length, in bytes — per-identity, generated fresh on every {@link hashPassword} call. */ +const SALT_LENGTH = 16 + +/** Fixed literal prefix marking an encoded hash as this module's scrypt format. */ +const ENCODED_PREFIX = 'scrypt' + +/** The exact shape {@link encode} produces and {@link decode} parses: `scrypt$N=..,r=..,p=..$$` — four `$`-separated segments. */ +const PARAMS_PATTERN = /^N=(\d+),r=(\d+),p=(\d+)$/ + +/** Decode-time ceilings on the embedded cost parameters and digest sizes — see the bounds note inside {@link decode}. */ +const MAX_DECODED_N = 1 << 20 +const MAX_DECODED_R = 32 +const MAX_DECODED_P = 16 +const MAX_DECODED_SALT_BYTES = 64 +const MAX_DECODED_HASH_BYTES = 128 + +interface DecodedHash { + n: number + r: number + p: number + salt: Buffer + hash: Buffer +} + +/** Build the one-string encoding for a salt+hash pair, embedding the cost parameters used to produce it. */ +function encode(salt: Buffer, hash: Buffer): string { + return `${ENCODED_PREFIX}$N=${SCRYPT_N},r=${SCRYPT_R},p=${SCRYPT_P}$${salt.toString('base64url')}$${hash.toString('base64url')}` +} + +/** + * Parse an encoded hash string back into its parts. TOTAL: any deviation + * from the exact expected shape (wrong segment count, wrong prefix, + * malformed params, empty salt/hash, non-base64url bytes) returns `null` + * rather than throwing — see the module doc. + */ +function decode(encoded: string): DecodedHash | null { + if (typeof encoded !== 'string') return null + const segments = encoded.split('$') + if (segments.length !== 4) return null + const [prefix, params, saltB64, hashB64] = segments + if (prefix !== ENCODED_PREFIX) return null + if (saltB64.length === 0 || hashB64.length === 0) return null + + const match = PARAMS_PATTERN.exec(params) + if (match === null) return null + const n = Number(match[1]) + const r = Number(match[2]) + const p = Number(match[3]) + // Upper bounds as well as lower: the embedded parameters DRIVE the scrypt + // work `verifyPassword` performs, so a syntactically valid tuple with a + // huge N/r/p (a corrupted or hostile stored value) must not be allowed to + // buy unbounded CPU/memory before failing. The caps leave generous + // headroom over the current constants for future cost bumps while keeping + // the worst case bounded. + if ( + !Number.isFinite(n) || + !Number.isFinite(p) || + !Number.isFinite(r) || + n <= 0 || + r <= 0 || + p <= 0 || + n > MAX_DECODED_N || + r > MAX_DECODED_R || + p > MAX_DECODED_P + ) { + return null + } + + // Buffer.from(..., 'base64url') never throws on arbitrary input (it + // silently drops characters outside the alphabet) — no try/catch needed, + // but an empty result after decoding a non-empty string still means + // "not a real hash", guarded below. + const salt = Buffer.from(saltB64, 'base64url') + const hash = Buffer.from(hashB64, 'base64url') + if (salt.length === 0 || hash.length === 0) return null + if (salt.length > MAX_DECODED_SALT_BYTES || hash.length > MAX_DECODED_HASH_BYTES) return null + + return { n, r, p, salt, hash } +} + +/** + * Hash `password` with a freshly-generated random salt, at the fixed cost + * parameters above. Returns the one-string encoding to store verbatim in + * `agent_auth_identities.secret_hash`. + */ +export function hashPassword(password: string): string { + const salt = randomBytes(SALT_LENGTH) + const hash = scryptSync(password, salt, KEY_LENGTH, { N: SCRYPT_N, r: SCRYPT_R, p: SCRYPT_P }) + return encode(salt, hash) +} + +/** + * Verify `password` against a previously-{@link hashPassword}-produced + * `encoded` string, in constant time. TOTAL over `encoded` (the module + * doc's totality note): a malformed or corrupted value returns `false`, + * never throws. Re-derives the hash using the cost parameters and salt + * EMBEDDED in `encoded` — not this module's current constants — so a future + * bump to {@link SCRYPT_N}/etc. never invalidates hashes stored under the + * old parameters. + */ +export function verifyPassword(password: string, encoded: string): boolean { + const decoded = decode(encoded) + if (decoded === null) return false + + let candidate: Buffer + try { + // A decoded params triple that scrypt itself rejects (e.g. a corrupted + // N that isn't a power of two, or a memory requirement past Node's + // default maxmem) is still "not our format" in effect — caught and + // treated as a verification failure, not a crash, preserving totality + // over a value this module does not fully control the shape of once + // it's round-tripped through storage. + candidate = scryptSync(password, decoded.salt, decoded.hash.length, { + N: decoded.n, + r: decoded.r, + p: decoded.p, + }) + } catch { + return false + } + + if (candidate.length !== decoded.hash.length) return false + return timingSafeEqual(candidate, decoded.hash) +} + +/** + * A real hash of a random, never-reused string, computed ONCE at module + * load. `PasswordAuthProvider` (`src/auth/password-provider.ts`) runs + * {@link verifyPassword} against this for an unknown email so the scrypt + * work — and therefore the wall-clock timing — is the same whether the + * email exists or not (spec §9's no-account-enumeration requirement). + */ +export const DUMMY_HASH: string = hashPassword(randomBytes(32).toString('base64url')) diff --git a/src/auth/password-provider.test.ts b/src/auth/password-provider.test.ts new file mode 100644 index 0000000..6fcbd1a --- /dev/null +++ b/src/auth/password-provider.test.ts @@ -0,0 +1,162 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import { type AgentStore, createAgentStore } from '../store/agents.js' +import { hashPassword, MAX_PASSWORD_LENGTH } from './password-hash.js' +import { createPasswordAuthProvider } from './password-provider.js' +import type { AuthProvider } from './provider.js' + +describe('PasswordAuthProvider', () => { + let db: Db | undefined + let agentStore: AgentStore | undefined + let provider: AuthProvider | undefined + + afterEach(async () => { + await db?.close() + db = undefined + agentStore = undefined + provider = undefined + }) + + async function freshProvider(): Promise<{ agentStore: AgentStore; provider: AuthProvider }> { + db = await createPgliteDb() + await migrate(db) + agentStore = createAgentStore(db) + provider = createPasswordAuthProvider({ agentStore }) + return { agentStore, provider } + } + + it('descriptor() reports key: password, kind: credentials', async () => { + const { provider } = await freshProvider() + expect(provider.descriptor()).toEqual({ + key: 'password', + label: expect.any(String), + kind: 'credentials', + }) + }) + + it('resolves the correct email + password to the Agent identity', async () => { + const { agentStore, provider } = await freshProvider() + const created = await agentStore.createAgent({ + name: 'Real Agent', + email: 'real@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + if (!created.ok) throw new Error('expected ok') + + const result = await provider.authenticate({ + providerKey: 'password', + email: 'real@example.test', + password: 'correct-password', + }) + expect(result).toEqual({ agentId: created.agent.id }) + }) + + it('email is matched case-insensitively', async () => { + const { agentStore, provider } = await freshProvider() + await agentStore.createAgent({ + name: 'Real Agent', + email: 'real@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + + const result = await provider.authenticate({ + providerKey: 'password', + email: 'REAL@EXAMPLE.TEST', + password: 'correct-password', + }) + expect(result).not.toBeNull() + }) + + it('rejects a wrong password', async () => { + const { agentStore, provider } = await freshProvider() + await agentStore.createAgent({ + name: 'Real Agent', + email: 'real@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + + const result = await provider.authenticate({ + providerKey: 'password', + email: 'real@example.test', + password: 'wrong-password', + }) + expect(result).toBeNull() + }) + + it('rejects an unknown email', async () => { + const { provider } = await freshProvider() + const result = await provider.authenticate({ + providerKey: 'password', + email: 'nobody@example.test', + password: 'anything', + }) + expect(result).toBeNull() + }) + + it("rejects an 'invited' Agent even with no password set yet (no identity to verify against)", async () => { + const { agentStore, provider } = await freshProvider() + await agentStore.createAgent({ + name: 'Invited Agent', + email: 'invited@example.test', + role: 'agent', + status: 'invited', + }) + + const result = await provider.authenticate({ + providerKey: 'password', + email: 'invited@example.test', + password: 'anything', + }) + expect(result).toBeNull() + }) + + it("rejects a 'disabled' Agent even with the CORRECT password", async () => { + const { agentStore, provider } = await freshProvider() + const created = await agentStore.createAgent({ + name: 'Disabled Agent', + email: 'disabled@example.test', + role: 'agent', + status: 'active', + passwordHash: hashPassword('correct-password'), + }) + if (!created.ok) throw new Error('expected ok') + await agentStore.updateAgent(created.agent.id, { status: 'disabled' }) + + const result = await provider.authenticate({ + providerKey: 'password', + email: 'disabled@example.test', + password: 'correct-password', + }) + expect(result).toBeNull() + }) + + it('is TOTAL over a malformed attempt — non-string email/password never throws, resolves null', async () => { + const { provider } = await freshProvider() + const attempts = [ + { providerKey: 'password', email: 123, password: 'x' }, + { providerKey: 'password', email: 'x@example.test', password: 123 }, + { providerKey: 'password', email: undefined, password: undefined }, + { providerKey: 'password' }, + ] + for (const attempt of attempts) { + await expect(provider.authenticate(attempt)).resolves.toBeNull() + } + }) + + it('rejects an over-length password before any KDF work (the pre-session scrypt-cost cap)', async () => { + const { provider } = await freshProvider() + const attempt = { + providerKey: 'password', + email: 'agent@example.test', + password: 'x'.repeat(MAX_PASSWORD_LENGTH + 1), + } + await expect(provider.authenticate(attempt)).resolves.toBeNull() + }) +}) diff --git a/src/auth/password-provider.ts b/src/auth/password-provider.ts new file mode 100644 index 0000000..6fe873a --- /dev/null +++ b/src/auth/password-provider.ts @@ -0,0 +1,94 @@ +/** + * `PasswordAuthProvider` — the core's one `AuthProvider` (HT-54; + * specs/auth/agents-and-auth.md §4). The free-core login: email + password, + * verified against `agent_auth_identities`. + * + * ## No account enumeration (spec §9) + * + * `authenticate` returns the SAME `null` outcome for an unknown email, a + * wrong password, an `invited` Agent (no usable credential yet), and a + * `disabled` Agent (even with the correct password) — a caller cannot + * distinguish any of these from the response alone. Timing is kept + * comparable for the "unknown email" case specifically: when no `password` + * identity exists for the given email, this still runs a real scrypt + * verification — against {@link DUMMY_HASH}, a fixed hash computed once at + * module load — so the wall-clock cost of "no such identity" matches "wrong + * password against a real identity." (A `disabled`/`invited` Agent's + * rejection happens AFTER the real scrypt verification against their own + * stored hash, so that branch is inherently no faster than a genuine + * password check either.) + */ + +import type { AgentStore } from '../store/agents.js' +import { DUMMY_HASH, MAX_PASSWORD_LENGTH, verifyPassword } from './password-hash.js' +import type { + AuthAttempt, + AuthProvider, + AuthProviderDescriptor, + VerifiedIdentity, +} from './provider.js' + +/** Dependencies {@link createPasswordAuthProvider} needs. */ +export interface PasswordAuthProviderDeps { + agentStore: AgentStore +} + +/** The wire key/label this provider serializes as (`GET /api/v1/auth/providers`, spec §6). */ +const DESCRIPTOR: AuthProviderDescriptor = { + key: 'password', + label: 'Email and password', + kind: 'credentials', +} + +/** + * Build the core `password` `AuthProvider`. `authenticate` is TOTAL over + * `attempt` — a malformed attempt (non-string `email`/`password`, or either + * field missing) returns `null` without ever touching the store or the KDF; + * this is a distinct, cheap rejection from the "real but wrong" cases above, + * and is fine to be fast (a malformed request carries no timing signal + * about any real Agent's existence). + */ +export function createPasswordAuthProvider(deps: PasswordAuthProviderDeps): AuthProvider { + return { + key: 'password', + + descriptor(): AuthProviderDescriptor { + return DESCRIPTOR + }, + + async authenticate(attempt: AuthAttempt): Promise { + const { email, password } = attempt + if (typeof email !== 'string' || typeof password !== 'string') { + return null + } + // Cap BEFORE any KDF work: this is the one pre-session, rate-limit-free + // entry point (HT-53), so an unbounded candidate would buy an attacker + // arbitrarily large scrypt input for free. A fast reject here carries no + // per-Agent timing signal — it is conditioned only on the caller's own + // input, same as the malformed-attempt rejection above. No real password + // can exceed this: every set-password path enforces the same cap. + if (password.length > MAX_PASSWORD_LENGTH) { + return null + } + + const identity = await deps.agentStore.getPasswordIdentityByEmail(email) + if (identity === null) { + // Burn the same scrypt cost as a real verification, discard the + // result — see the module doc's timing-comparability note. + verifyPassword(password, DUMMY_HASH) + return null + } + + if (!verifyPassword(password, identity.secretHash)) { + return null + } + + const agent = await deps.agentStore.getAgent(identity.agentId) + if (agent === null || agent.status !== 'active') { + return null + } + + return { agentId: identity.agentId } + }, + } +} diff --git a/src/auth/provider.ts b/src/auth/provider.ts new file mode 100644 index 0000000..54e627a --- /dev/null +++ b/src/auth/provider.ts @@ -0,0 +1,72 @@ +/** + * The auth-provider seam (HT-54; specs/auth/agents-and-auth.md §4) — the + * interface core and marketplace login methods share. + * + * The core ships this interface plus exactly one implementation + * (`PasswordAuthProvider`, `src/auth/password-provider.ts`). A marketplace + * module (Google SSO, passkey, ...) is a package that provides another + * `AuthProvider` and is wired into the registry at the composition root + * (`src/composition/root.ts`) — `AuthProvider[]`, an ordered list, no + * discovery mechanism. Spec §4's "honest scope note": this build is the + * interface + the registry + the one core provider, not a dynamic + * module-loading system — adding a second provider later is still a `root.ts` + * code edit, not a drop-in. Kept deliberately minimal for that reason: + * `AuthProviderDescriptor` carries only what a password form needs to + * render, not speculative OAuth fields (redirect URLs, client ids, ...) no + * shipped provider uses yet. + */ + +/** + * What the login UI needs to render one login method, serialized verbatim + * by `GET /api/v1/auth/providers` (spec §6). `kind: 'credentials'` is the + * only kind the core seam defines today (a password form) — deliberately + * not widened to anticipate an OAuth `kind` before a module that needs one + * actually ships (module doc's "do not speculate" note). + */ +export interface AuthProviderDescriptor { + key: string + label: string + kind: 'credentials' +} + +/** + * What a provider resolves a verified attempt to: which Agent this is. + * Never a session — minting one is the core's job (spec §8), not a + * provider's; a provider only ever answers "who is this," never "let them + * in." + */ +export interface VerifiedIdentity { + agentId: string +} + +/** + * One login attempt, as posted to `POST /api/v1/auth/verify` (spec §6). + * `providerKey` selects which registered `AuthProvider` handles it; every + * other field is provider-specific (`password`'s reads `email`/`password`) + * and is intentionally untyped here (`Record`) — this + * interface has no business knowing another provider's shape. + */ +export type AuthAttempt = { providerKey: string } & Record + +/** + * One login method: the core's `password`, or a marketplace module's own + * (`google`, `passkey`, ...). See the module doc for the registry model. + */ +export interface AuthProvider { + readonly key: string + + /** What the login UI needs to render this method (a password field; a "Sign in with X" button + start URL) — see {@link AuthProviderDescriptor}. */ + descriptor(): AuthProviderDescriptor + + /** + * Verify `attempt` and resolve it to an existing Agent identity, or + * `null` on any failure — wrong credentials, an unknown/inactive Agent, + * a malformed attempt. `password`'s implementation reads + * `agent_auth_identities`; an OAuth module would run its own flow then + * map the verified external subject to an Agent via a core-owned + * identity service (spec §4 — not built in this increment; see that + * section's note on why the link/provision API is only sketched, not + * shipped, until the first module needs it). + */ + authenticate(attempt: AuthAttempt): Promise +} diff --git a/src/composition/config.test.ts b/src/composition/config.test.ts index 2b0dab8..4995da7 100644 --- a/src/composition/config.test.ts +++ b/src/composition/config.test.ts @@ -127,6 +127,47 @@ describe('loadConfig — missing / malformed values', () => { }) }) +describe('loadConfig — HELPTHREAD_UI_BASE_URL (HT-54, optional)', () => { + it('is absent from AppConfig when unset — no error, invite deps simply absent', () => { + const config = loadConfig(validEnv()) + expect(config.uiBaseUrl).toBeUndefined() + }) + + it('is absent when set to whitespace only — treated the same as unset', () => { + const config = loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: ' ' }) + expect(config.uiBaseUrl).toBeUndefined() + }) + + it('is read and normalized to a bare origin when set', () => { + const config = loadConfig({ + ...validEnv(), + HELPTHREAD_UI_BASE_URL: 'https://app.resonantiq.app/', + }) + expect(config.uiBaseUrl).toBe('https://app.resonantiq.app') + }) + + it('rejects a malformed value at boot rather than silently ignoring it', () => { + expect(() => loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: 'not a url' })).toThrow( + /HELPTHREAD_UI_BASE_URL/, + ) + expect(() => + loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: 'https://app.example.com/some/path' }), + ).toThrow(/HELPTHREAD_UI_BASE_URL/) + }) + + it('refuses plain http except for loopback hosts — invite links carry a signed credential', () => { + expect(() => + loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: 'http://app.example.com' }), + ).toThrow(/https/) + expect( + loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: 'http://localhost:3000' }).uiBaseUrl, + ).toBe('http://localhost:3000') + expect( + loadConfig({ ...validEnv(), HELPTHREAD_UI_BASE_URL: 'http://127.0.0.1:3000' }).uiBaseUrl, + ).toBe('http://127.0.0.1:3000') + }) +}) + describe('loadConfig — never leaks a secret value', () => { it('reports a too-short token by LENGTH, never echoing the secret value', () => { const secretValue = 'sekret' diff --git a/src/composition/config.ts b/src/composition/config.ts index 5acc675..f6a3852 100644 --- a/src/composition/config.ts +++ b/src/composition/config.ts @@ -86,6 +86,16 @@ export interface AppConfig { mailDomain: string /** The connected support mailbox's address — the `from` on every Agent reply, and the mailbox outbound sends resolve their token from. */ supportAddress: string + /** + * The web UI's base origin (HT-54; specs/auth/agents-and-auth.md §8) — + * invite links are `${uiBaseUrl}/invite/${token}`. OPTIONAL, unlike every + * other field above: when `HELPTHREAD_UI_BASE_URL` is unset, invite email + * deps are simply absent — the Agents API still works, `sendInvite` + * creates `invited` Agents with `inviteSent: false`, and `POST + * /agents/{id}/invite` refuses with `409 conflict` (the admin-set-password + * fallback remains the only path that works before a UI origin is known). + */ + uiBaseUrl?: string } /** Accumulates human-readable, secret-free validation problems for a single combined throw. */ @@ -158,6 +168,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): AppConfig { const tokenEncryptionKey = resolveEncryptionKey(env, errors) const publicBaseUrl = resolvePublicBaseUrl(env, errors) + const uiBaseUrl = resolveUiBaseUrl(env, errors) errors.throwIfAny() @@ -181,6 +192,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): AppConfig { publicBaseUrl: publicBaseUrl as string, mailDomain: mailDomain as string, supportAddress: supportAddress as string, + ...(uiBaseUrl !== undefined ? { uiBaseUrl } : {}), } } @@ -251,3 +263,67 @@ function resolvePublicBaseUrl(env: NodeJS.ProcessEnv, errors: ConfigErrors): str } return parsed.origin } + +/** + * `HELPTHREAD_UI_BASE_URL` is OPTIONAL (HT-54; unlike every `require*` field + * above) — absent means "no invite email deps configured" (`AppConfig.uiBaseUrl`'s + * doc). `undefined` here means "not set, and that's fine, no error." When + * SET, it must still be a well-formed http(s) origin (same shape as + * `PUBLIC_BASE_URL`, but a distinct origin — the UI is a separate Vercel + * project from the engine, `HELPTHREAD_UI_SESSION_SECRET`'s deployment) — + * a malformed value IS a boot-time error, since a garbage invite link is + * worse than no invite feature at all. + */ +/** Hosts whose traffic never leaves the machine — the one place plain http is acceptable for invite links. */ +function isLoopbackHost(hostname: string): boolean { + return ( + hostname === 'localhost' || + hostname === '127.0.0.1' || + hostname === '[::1]' || + hostname === '::1' + ) +} + +function resolveUiBaseUrl(env: NodeJS.ProcessEnv, errors: ConfigErrors): string | undefined { + const raw = env.HELPTHREAD_UI_BASE_URL + if (raw === undefined || raw.trim().length === 0) return undefined + + let parsed: URL + try { + parsed = new URL(raw) + } catch { + errors.add( + `HELPTHREAD_UI_BASE_URL must be an absolute URL (e.g. https://desk.example.com), got ${JSON.stringify(raw)}`, + ) + return undefined + } + if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { + errors.add( + `HELPTHREAD_UI_BASE_URL must be an http(s) URL, got protocol ${JSON.stringify(parsed.protocol)}`, + ) + return undefined + } + // Invite links carry a credential (the signed invite token), so plaintext + // transport is refused outright — except explicit loopback hosts, where + // local development genuinely runs over http and the traffic never leaves + // the machine. + if (parsed.protocol === 'http:' && !isLoopbackHost(parsed.hostname)) { + errors.add( + `HELPTHREAD_UI_BASE_URL must use https (invite links carry a signed credential); http is allowed only for loopback hosts, got ${JSON.stringify(raw)}`, + ) + return undefined + } + if ( + (parsed.pathname !== '/' && parsed.pathname !== '') || + parsed.search !== '' || + parsed.hash !== '' || + parsed.username !== '' || + parsed.password !== '' + ) { + errors.add( + 'HELPTHREAD_UI_BASE_URL must be a bare origin with no path, query, fragment, or credentials (e.g. https://desk.example.com)', + ) + return undefined + } + return parsed.origin +} diff --git a/src/composition/root.ts b/src/composition/root.ts index f1d65cf..e18433d 100644 --- a/src/composition/root.ts +++ b/src/composition/root.ts @@ -47,6 +47,8 @@ import { type GmailReconcileJob, } from '../api/gmail-webhook.js' import { createInboxApi } from '../api/index.js' +import { createPasswordAuthProvider } from '../auth/password-provider.js' +import type { AuthProvider } from '../auth/provider.js' import type { Db } from '../db/client.js' import { createPostgresDb } from '../db/postgres.js' import { createGmailConnectService } from '../mail/gmail-connect.js' @@ -70,6 +72,7 @@ import { createPostgresQueue } from '../providers/adapters/postgres-queue/index. import { createSupabaseStorageBlobStore } from '../providers/adapters/supabase-storage/index.js' import type { BlobStore } from '../providers/blob.js' import type { QueueMessage, QueueMessageHandler } from '../providers/queue.js' +import { createAgentStore } from '../store/agents.js' import { createConversationStore, createGmailWatchStateStore, @@ -142,6 +145,13 @@ export async function buildApp( const watchStateStore = createGmailWatchStateStore(db) const inboundDeliveryStore = createInboundDeliveryStore(db) const attachmentStore = createThreadAttachmentStore(db) + const agentStore = createAgentStore(db) + + // --- Agents & Authentication (HT-54): the core provider registry is just + // `[password]` — an ordered list, no discovery mechanism (spec §4's + // honest-scope note). A marketplace module adds a provider HERE, in a + // future ticket, not via any plugin loader this build ships. --- + const authProviders: AuthProvider[] = [createPasswordAuthProvider({ agentStore })] // --- The HMAC keyring backing reply/state/view tokens (single current key). --- const keyring: Keyring = { current: { keyId: SIGNING_KEY_ID, secret: config.signingSecret } } @@ -232,6 +242,15 @@ export async function buildApp( gmailConnect, gmailDisconnect, attachments: { store: attachmentStore, blobStore }, + // Agents & Authentication (HT-54) — CORE, required (unlike the + // absent-by-default fields above). uiBaseUrl is spread in only when + // configured (config.ts's own optional-field convention) so the invite + // path stays genuinely absent, not present-with-undefined. + agents: { + store: agentStore, + providers: authProviders, + ...(config.uiBaseUrl !== undefined ? { uiBaseUrl: config.uiBaseUrl } : {}), + }, // HT-49 review fix: Gmail delivers a sent reply's own copy back into the // SAME mailbox it was sent from, where reconcile would otherwise re-ingest // it as a phantom inbound message (src/mail/send.ts's "The reply token's diff --git a/src/db/migrate.test.ts b/src/db/migrate.test.ts index 351026a..51e32ac 100644 --- a/src/db/migrate.test.ts +++ b/src/db/migrate.test.ts @@ -57,6 +57,7 @@ describe('migrate', () => { { id: 15, name: 'thread_attachments' }, { id: 16, name: 'gmail_reconcile_lease' }, { id: 17, name: 'mailboxes_disconnected_status' }, + { id: 18, name: 'agents_and_auth' }, ]) }) @@ -84,6 +85,7 @@ describe('migrate', () => { { id: 15 }, { id: 16 }, { id: 17 }, + { id: 18 }, ]) }) @@ -450,7 +452,12 @@ describe('migrate', () => { ['customer@example.test'], ) - await expect(migrate(database)).resolves.toBeUndefined() + // Bounded to throughId: 17 — this test is about migration 006's OWN + // upgrade behavior, which migration 018 later supersedes (it drops + // `assignee` entirely; see that migration's own test). Running the + // unbounded migrate() here would apply 018 too and this SELECT would + // fail with "column assignee does not exist". + await expect(migrate(database, { throughId: 17 })).resolves.toBeUndefined() const [row] = await database.query<{ tags: unknown; assignee: string | null }>( 'SELECT tags, assignee FROM conversations WHERE id = $1', @@ -848,4 +855,183 @@ describe('migrate', () => { expect(afterDelete.id).toBe(delivery.id) expect(afterDelete.thread_id).toBeNull() }) + + it('migration 018 creates agents with default role/status/timezone, enforces the email UNIQUE (case-insensitive) and the role/status CHECKs', async () => { + db = await createPgliteDb() + await migrate(db) + + const [agent] = await db.query<{ + id: string + role: string + status: string + timezone: string + }>( + `INSERT INTO agents (email, name) VALUES ($1, $2) + RETURNING id, role, status, timezone`, + ['agent@example.test', 'Agent One'], + ) + expect(agent.id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/) + expect(agent.role).toBe('agent') + expect(agent.status).toBe('invited') + expect(agent.timezone).toBe('UTC') + + // Case-insensitive email uniqueness. + await expect( + db.query('INSERT INTO agents (email, name) VALUES ($1, $2)', [ + 'Agent@Example.test', + 'Duplicate', + ]), + ).rejects.toThrow() + + // Out-of-domain role/status are rejected. + await expect( + db.query('INSERT INTO agents (email, name, role) VALUES ($1, $2, $3)', [ + 'other@example.test', + 'Other', + 'superadmin', + ]), + ).rejects.toThrow() + await expect( + db.query('INSERT INTO agents (email, name, status) VALUES ($1, $2, $3)', [ + 'other@example.test', + 'Other', + 'bogus', + ]), + ).rejects.toThrow() + }) + + it('migration 018 enforces one password identity per Agent via the partial unique index, but allows multiple providers', async () => { + db = await createPgliteDb() + await migrate(db) + + const [agent] = await db.query<{ id: string }>( + 'INSERT INTO agents (email, name) VALUES ($1, $2) RETURNING id', + ['agent@example.test', 'Agent One'], + ) + + await db.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + VALUES ($1, 'password', $2, $3)`, + [agent.id, 'agent@example.test', 'scrypt$hash'], + ) + + // A second 'password' identity for the SAME agent collides with the + // partial unique index, even though (provider, subject) itself differs. + await expect( + db.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + VALUES ($1, 'password', $2, $3)`, + [agent.id, 'agent-alias@example.test', 'scrypt$hash2'], + ), + ).rejects.toThrow() + + // A different provider for the SAME agent is fine — one Agent, many login methods. + await expect( + db.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject) VALUES ($1, 'google', $2)`, + [agent.id, 'google-sub-123'], + ), + ).resolves.toBeDefined() + + // (provider, subject) UNIQUE holds independent of the partial index. + await expect( + db.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject) VALUES ($1, 'google', $2)`, + [agent.id, 'google-sub-123'], + ), + ).rejects.toThrow() + + // Deleting the agent cascades its identities. + await db.query('DELETE FROM agents WHERE id = $1', [agent.id]) + const remaining = await db.query('SELECT id FROM agent_auth_identities WHERE agent_id = $1', [ + agent.id, + ]) + expect(remaining).toEqual([]) + }) + + it('migration 018 models agent_mailbox_access as a pure join with a composite PK, cascading on either side', async () => { + db = await createPgliteDb() + await migrate(db) + + const [agent] = await db.query<{ id: string }>( + 'INSERT INTO agents (email, name) VALUES ($1, $2) RETURNING id', + ['agent@example.test', 'Agent One'], + ) + const [mailbox] = await db.query<{ id: string }>( + 'INSERT INTO mailboxes (address, provider) VALUES ($1, $2) RETURNING id', + ['support@example.test', 'gmail'], + ) + + await db.query('INSERT INTO agent_mailbox_access (agent_id, mailbox_id) VALUES ($1, $2)', [ + agent.id, + mailbox.id, + ]) + + // Duplicate pair collides on the composite PK. + await expect( + db.query('INSERT INTO agent_mailbox_access (agent_id, mailbox_id) VALUES ($1, $2)', [ + agent.id, + mailbox.id, + ]), + ).rejects.toThrow() + + await db.query('DELETE FROM agents WHERE id = $1', [agent.id]) + const remaining = await db.query( + 'SELECT agent_id FROM agent_mailbox_access WHERE mailbox_id = $1', + [mailbox.id], + ) + expect(remaining).toEqual([]) + }) + + it("migration 018 replaces conversations.assignee with assignee_agent_id: existing 'me' rows become NULL, the new column FKs to agents with ON DELETE SET NULL", async () => { + const database = await createPgliteDb() + db = database + + // Apply only through migration 17, then write a conversation the way a + // pre-018 deployment would have — the single-operator 'me' flag. + await migrate(database, { throughId: 17 }) + const [existing] = await database.query<{ id: string }>( + `INSERT INTO conversations (customer_email, assignee) VALUES ($1, 'me') RETURNING id`, + ['customer@example.test'], + ) + + await expect(migrate(database)).resolves.toBeUndefined() + + // The old column is gone; the new one defaults every pre-existing row to + // NULL (no migrated value — spec §3.3: Agents don't exist until this + // migration runs, so there is nothing 'me' could map to). + const [row] = await database.query<{ assignee_agent_id: string | null }>( + 'SELECT assignee_agent_id FROM conversations WHERE id = $1', + [existing.id], + ) + expect(row.assignee_agent_id).toBeNull() + await expect( + database.query('SELECT assignee FROM conversations WHERE id = $1', [existing.id]), + ).rejects.toThrow() + + // Assigning to a real Agent works; deleting that Agent un-assigns (SET + // NULL) rather than deleting the conversation. + const [agent] = await database.query<{ id: string }>( + 'INSERT INTO agents (email, name) VALUES ($1, $2) RETURNING id', + ['agent@example.test', 'Agent One'], + ) + await database.query('UPDATE conversations SET assignee_agent_id = $1 WHERE id = $2', [ + agent.id, + existing.id, + ]) + await database.query('DELETE FROM agents WHERE id = $1', [agent.id]) + const [afterDelete] = await database.query<{ assignee_agent_id: string | null }>( + 'SELECT assignee_agent_id FROM conversations WHERE id = $1', + [existing.id], + ) + expect(afterDelete.assignee_agent_id).toBeNull() + + // A nonexistent agent id violates the FK. + await expect( + database.query('UPDATE conversations SET assignee_agent_id = $1 WHERE id = $2', [ + '00000000-0000-0000-0000-000000000000', + existing.id, + ]), + ).rejects.toThrow() + }) }) diff --git a/src/db/migrate.ts b/src/db/migrate.ts index b903ab6..ca408b6 100644 --- a/src/db/migrate.ts +++ b/src/db/migrate.ts @@ -816,6 +816,104 @@ ALTER TABLE mailboxes DROP CONSTRAINT mailboxes_status_check; ALTER TABLE mailboxes ADD CONSTRAINT mailboxes_status_check CHECK (status IN ('active','paused','needs_reconnect','disconnected')); ` +/** + * Migration 018 — `agents`, `agent_auth_identities`, `agent_mailbox_access`, + * and the `conversations.assignee` → `assignee_agent_id` swap (HT-54; + * specs/auth/agents-and-auth.md §3). + * + * ## `agents` — the identity (spec §3.1) + * + * One row per human support-staff member (never an Assistant — CLAUDE.md's + * vocabulary rule). `role`/`status` are both CHECK-constrained closed sets, + * matching this file's standing convention for closed-set lifecycle columns + * (`conversations.status`, `mailboxes.status`, ...). `email` is written + * already-lowercased by every application writer (`src/store/agents.ts` + * normalizes before every INSERT); `agents_email_key` on `lower(email)` is + * schema-level defense-in-depth, not the only place normalization happens. + * `status = 'invited'` is produced ONLY by the invite-provisioning path + * (spec §8) — every other creation path (`/setup`, the admin-set-password + * fallback) inserts `'active'` directly, so a credential-less `active` row + * is unrepresentable by construction, not just by application discipline. + * + * ## `agent_auth_identities` — how an Agent proves who they are (spec §3.2) + * + * The marketplace seam: one row per (Agent, auth method). `secret_hash` is + * NULL for every non-`'password'` provider (an OAuth module never writes a + * hash) — nullable rather than a second table, since every row already + * carries `provider` to discriminate. `UNIQUE (provider, subject)` is the + * ordinary "no two Agents can claim the same external identity" invariant; + * `agent_auth_identities_one_password_per_agent` is a SEPARATE, additional + * invariant that constraint alone cannot express — see the inline SQL + * comment (kept in the SQL, not just here, because the "why not just the + * UNIQUE above" reasoning is exactly the kind of non-obvious constraint + * rationale that belongs beside the DDL it explains, matching how migration + * 002's NULL-semantics comment lives next to its CHECK). + * + * ## `agent_mailbox_access` — schema now, behavior deferred (spec §3.4) + * + * Modeled so a future per-Agent mailbox-scoping increment is a store/API + * change, not a migration against live rows (TJ, 2026-07-18, spec §12.4). + * Nothing in this build reads or writes this table — an EMPTY table means + * "every Agent may access every mailbox" by definition, not by a runtime + * check anywhere. `PRIMARY KEY (agent_id, mailbox_id)` needs no separate + * surrogate `id`: this is a pure many-to-many join with no attributes of + * its own beyond `created_at`. + * + * ## `conversations.assignee` → `assignee_agent_id` — breaking (spec §3.3) + * + * `assignee` (migration 006) was deliberately NOT identity — a `text CHECK + * (assignee IS NULL OR assignee = 'me')` flag for the single-operator era. + * Multi-Agent replaces it with a real FK. **No UPDATE/backfill step**: every + * existing `'me'` row has no Agent to map to (Agents are created only + * starting with THIS migration, at first-run) — spec §3.3 is explicit that + * those rows become unassigned (`NULL`) by construction, simply by the new + * column defaulting `NULL` and the old column being dropped, not by any + * migrated value. `ON DELETE SET NULL` (not `CASCADE`): deleting an Agent + * un-assigns their conversations, it does not delete them — the same + * "the record outlives the pointer" policy migration 012's + * `inbound_deliveries.thread_id` already uses. Dropping `assignee` also + * drops `conversations_assignee_check` (migration 006's CHECK, which + * references only that column) automatically — Postgres removes a + * single-column constraint along with the column it's built on, no + * explicit `DROP CONSTRAINT`/`CASCADE` needed. + */ +const MIGRATION_018_AGENTS_AND_AUTH = ` +CREATE TABLE agents ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + email text NOT NULL, + name text NOT NULL, + role text NOT NULL DEFAULT 'agent' CHECK (role IN ('admin', 'agent')), + status text NOT NULL DEFAULT 'invited' CHECK (status IN ('invited', 'active', 'disabled')), + timezone text NOT NULL DEFAULT 'UTC', + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX agents_email_key ON agents (lower(email)); +CREATE TABLE agent_auth_identities ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + agent_id uuid NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + provider text NOT NULL, + subject text NOT NULL, + secret_hash text, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + UNIQUE (provider, subject), + CONSTRAINT agent_auth_identities_password_secret_check + CHECK (provider <> 'password' OR secret_hash IS NOT NULL) +); +CREATE INDEX agent_auth_identities_agent ON agent_auth_identities (agent_id); +CREATE UNIQUE INDEX agent_auth_identities_one_password_per_agent ON agent_auth_identities (agent_id) WHERE provider = 'password'; +CREATE TABLE agent_mailbox_access ( + agent_id uuid NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + mailbox_id uuid NOT NULL REFERENCES mailboxes(id) ON DELETE CASCADE, + created_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (agent_id, mailbox_id) +); +ALTER TABLE conversations ADD COLUMN assignee_agent_id uuid REFERENCES agents(id) ON DELETE SET NULL; +CREATE INDEX conversations_assignee_agent ON conversations (assignee_agent_id); +ALTER TABLE conversations DROP COLUMN assignee; +` + /** * Every migration, in the order they must apply. `id` is the sole ordering * key (ascending) — array position is not relied upon, so re-sorting this @@ -903,6 +1001,11 @@ const MIGRATIONS: Migration[] = [ name: 'mailboxes_disconnected_status', sql: MIGRATION_017_MAILBOXES_DISCONNECTED_STATUS, }, + { + id: 18, + name: 'agents_and_auth', + sql: MIGRATION_018_AGENTS_AND_AUTH, + }, ] /** diff --git a/src/db/postgres.test.ts b/src/db/postgres.test.ts index 4465b6e..e0ff345 100644 --- a/src/db/postgres.test.ts +++ b/src/db/postgres.test.ts @@ -296,6 +296,9 @@ describe('createPostgresDb with a schema option', () => { ) expect(placed.rows.map((r) => r.table_name)).toEqual([ '_migrations', + 'agent_auth_identities', + 'agent_mailbox_access', + 'agents', 'conversations', 'gmail_watch_state', 'inbound_deliveries', diff --git a/src/store/agents.test.ts b/src/store/agents.test.ts new file mode 100644 index 0000000..5ee2187 --- /dev/null +++ b/src/store/agents.test.ts @@ -0,0 +1,603 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import { type AgentStore, createAgentStore } from './agents.js' + +describe('AgentStore', () => { + let db: Db | undefined + let store: AgentStore | undefined + + afterEach(async () => { + await db?.close() + db = undefined + store = undefined + }) + + async function freshStore(): Promise<{ db: Db; store: AgentStore }> { + db = await createPgliteDb() + await migrate(db) + store = createAgentStore(db) + return { db, store } + } + + // --- createFirstAdmin ------------------------------------------------------- + + describe('createFirstAdmin', () => { + it('creates the first admin, active, with a password identity', async () => { + const { store } = await freshStore() + const agent = await store.createFirstAdmin({ + name: 'Ada Admin', + email: 'Ada@Example.test', + passwordHash: 'scrypt$hash1', + }) + expect(agent).not.toBeNull() + expect(agent?.role).toBe('admin') + expect(agent?.status).toBe('active') + // Email normalized to lowercase on insert. + expect(agent?.email).toBe('ada@example.test') + + const identity = await store.getPasswordIdentity(agent?.id as string) + expect(identity).toEqual({ agentId: agent?.id, secretHash: 'scrypt$hash1' }) + }) + + it('returns null when Agents already exist — the /setup 409 case', async () => { + const { store } = await freshStore() + const first = await store.createFirstAdmin({ + name: 'Ada Admin', + email: 'ada@example.test', + passwordHash: 'scrypt$hash1', + }) + expect(first).not.toBeNull() + + const second = await store.createFirstAdmin({ + name: 'Bea Admin', + email: 'bea@example.test', + passwordHash: 'scrypt$hash2', + }) + expect(second).toBeNull() + + // Only the first admin exists — the guard predicate (WHERE NOT + // EXISTS-equivalent) actually prevented a second insert, not just + // reported a decoy failure. + const all = await store.listAgents() + expect(all).toHaveLength(1) + expect(all[0].email).toBe('ada@example.test') + }) + + it("the guard runs under an advisory lock distinct from migrate.ts's own lock key (issues real lock SQL; true concurrency isn't reproducible against single-connection PGlite)", async () => { + // PGlite is a single, in-process connection — two genuinely concurrent + // transactions can't race each other here the way two Postgres backends + // could (see src/db/migrate.ts's own module doc making the identical + // point about its cross-process lock). What IS verified here: the lock + // statement is actually issued (a raw SELECT pg_advisory_xact_lock call + // succeeds against a real Postgres, proving the SQL is valid and takes + // the bigint overload), and the zero-Agents predicate genuinely holds + // (asserted by the two tests above). Real concurrent-createFirstAdmin + // coverage waits for a Supabase-backed Db, exactly as migrate.test.ts + // documents for its own advisory lock. + const { db } = await freshStore() + await expect( + db.query('SELECT pg_advisory_xact_lock($1::bigint)', [7_331_009_881]), + ).resolves.toBeDefined() + }) + + it('acquires the advisory lock BEFORE the zero-Agents check, inside one transaction (instrumented Db)', async () => { + // True two-backend concurrency isn't reproducible on single-connection + // PGlite (test above), but the ordering that MAKES the guard sound is + // unit-testable: wrap the Db, record every statement the transaction + // issues, and assert the lock precedes any read of `agents`. A refactor + // that drops the lock or moves the check ahead of it fails here. + const { db } = await freshStore() + const statements: string[] = [] + const instrumented: typeof db = { + query: (sql, params) => { + statements.push(sql) + return db.query(sql, params) + }, + transaction: (fn) => + db.transaction((tx) => + fn({ + query: (sql, params) => { + statements.push(sql) + return tx.query(sql, params) + }, + }), + ), + close: () => db.close(), + } + const instrumentedStore = createAgentStore(instrumented) + const created = await instrumentedStore.createFirstAdmin({ + name: 'Ada Admin', + email: 'ada@example.test', + passwordHash: 'scrypt$hash1', + }) + expect(created).not.toBeNull() + + const lockIndex = statements.findIndex((sql) => sql.includes('pg_advisory_xact_lock')) + const agentsReadIndex = statements.findIndex( + (sql) => sql !== statements[lockIndex] && /FROM agents|INSERT INTO agents/i.test(sql), + ) + expect(lockIndex).toBeGreaterThanOrEqual(0) + expect(agentsReadIndex).toBeGreaterThanOrEqual(0) + expect(lockIndex).toBeLessThan(agentsReadIndex) + }) + }) + + // --- createAgent ------------------------------------------------------------- + + describe('createAgent', () => { + it('creates an invited Agent with no password identity', async () => { + const { store } = await freshStore() + const result = await store.createAgent({ + name: 'Invitee', + email: 'invitee@example.test', + role: 'agent', + status: 'invited', + }) + expect(result.ok).toBe(true) + if (!result.ok) throw new Error('expected ok') + expect(result.agent.status).toBe('invited') + const identity = await store.getPasswordIdentity(result.agent.id) + expect(identity).toBeNull() + }) + + it('creates an active Agent with a password identity when passwordHash is given', async () => { + const { store } = await freshStore() + const result = await store.createAgent({ + name: 'Active One', + email: 'active@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + expect(result.ok).toBe(true) + if (!result.ok) throw new Error('expected ok') + const identity = await store.getPasswordIdentity(result.agent.id) + expect(identity?.secretHash).toBe('scrypt$hash') + }) + + it('returns email_taken on a duplicate email (case-insensitive), without throwing', async () => { + const { store } = await freshStore() + await store.createAgent({ + name: 'First', + email: 'dup@example.test', + role: 'agent', + status: 'invited', + }) + const result = await store.createAgent({ + name: 'Second', + email: 'Dup@Example.test', + role: 'agent', + status: 'invited', + }) + expect(result).toEqual({ ok: false, reason: 'email_taken' }) + }) + }) + + // --- getAgent / getAgentByEmail / listAgents -------------------------------- + + describe('getAgent / getAgentByEmail / listAgents', () => { + it('getAgent returns null for a missing id', async () => { + const { store } = await freshStore() + expect(await store.getAgent('00000000-0000-0000-0000-000000000000')).toBeNull() + }) + + it('getAgentByEmail is case-insensitive', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Casey', + email: 'casey@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + const found = await store.getAgentByEmail('CASEY@EXAMPLE.TEST') + expect(found?.id).toBe(created.agent.id) + }) + + it('listAgents orders by name', async () => { + const { store } = await freshStore() + await store.createAgent({ + name: 'Zoe', + email: 'zoe@example.test', + role: 'agent', + status: 'invited', + }) + await store.createAgent({ + name: 'Amir', + email: 'amir@example.test', + role: 'agent', + status: 'invited', + }) + const all = await store.listAgents() + expect(all.map((a) => a.name)).toEqual(['Amir', 'Zoe']) + }) + }) + + // --- updateAgent ------------------------------------------------------------- + + describe('updateAgent', () => { + it('updates name/timezone with no role/status change — no admin-count guard involved', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Original', + email: 'a@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + + const result = await store.updateAgent(created.agent.id, { + name: 'Renamed', + timezone: 'America/New_York', + }) + expect(result.ok).toBe(true) + if (!result.ok) throw new Error('expected ok') + expect(result.agent.name).toBe('Renamed') + expect(result.agent.timezone).toBe('America/New_York') + expect(result.agent.status).toBe('invited') // untouched + }) + + it('returns not_found for a missing id', async () => { + const { store } = await freshStore() + const result = await store.updateAgent('00000000-0000-0000-0000-000000000000', { + name: 'x', + }) + expect(result).toEqual({ ok: false, reason: 'not_found' }) + }) + + it('a no-op patch (no fields at all) returns the current record unchanged', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Same', + email: 'same@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + const result = await store.updateAgent(created.agent.id, {}) + expect(result).toEqual({ ok: true, agent: created.agent }) + }) + + it('demoting the ONLY active admin (role admin -> agent) is refused: last_admin', async () => { + const { store } = await freshStore() + const admin = await store.createFirstAdmin({ + name: 'Solo Admin', + email: 'solo@example.test', + passwordHash: 'scrypt$hash', + }) + if (admin === null) throw new Error('expected an admin') + + const result = await store.updateAgent(admin.id, { role: 'agent' }) + expect(result).toEqual({ ok: false, reason: 'last_admin' }) + + const stillAdmin = await store.getAgent(admin.id) + expect(stillAdmin?.role).toBe('admin') + }) + + it('disabling the ONLY active admin (status active -> disabled) is refused: last_admin', async () => { + const { store } = await freshStore() + const admin = await store.createFirstAdmin({ + name: 'Solo Admin', + email: 'solo@example.test', + passwordHash: 'scrypt$hash', + }) + if (admin === null) throw new Error('expected an admin') + + const result = await store.updateAgent(admin.id, { status: 'disabled' }) + expect(result).toEqual({ ok: false, reason: 'last_admin' }) + }) + + it('demoting ONE of TWO active admins succeeds', async () => { + const { store } = await freshStore() + const admin1 = await store.createFirstAdmin({ + name: 'Admin One', + email: 'admin1@example.test', + passwordHash: 'scrypt$hash', + }) + if (admin1 === null) throw new Error('expected an admin') + const created2 = await store.createAgent({ + name: 'Admin Two', + email: 'admin2@example.test', + role: 'admin', + status: 'active', + passwordHash: 'scrypt$hash2', + }) + if (!created2.ok) throw new Error('expected ok') + + const result = await store.updateAgent(admin1.id, { role: 'agent' }) + expect(result.ok).toBe(true) + if (!result.ok) throw new Error('expected ok') + expect(result.agent.role).toBe('agent') + }) + + it('a DISABLED admin does not count toward the active-admin invariant — demoting the sole remaining ACTIVE admin is still refused even if a disabled admin also exists', async () => { + const { store } = await freshStore() + const activeAdmin = await store.createFirstAdmin({ + name: 'Active Admin', + email: 'active-admin@example.test', + passwordHash: 'scrypt$hash', + }) + if (activeAdmin === null) throw new Error('expected an admin') + const disabledAdminCreated = await store.createAgent({ + name: 'Disabled Admin', + email: 'disabled-admin@example.test', + role: 'admin', + status: 'active', + passwordHash: 'scrypt$hash2', + }) + if (!disabledAdminCreated.ok) throw new Error('expected ok') + await store.updateAgent(disabledAdminCreated.agent.id, { status: 'disabled' }) + + const result = await store.updateAgent(activeAdmin.id, { role: 'agent' }) + expect(result).toEqual({ ok: false, reason: 'last_admin' }) + }) + + it('promoting an agent to admin never triggers the guard', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Promotable', + email: 'promotable@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + const result = await store.updateAgent(created.agent.id, { role: 'admin' }) + expect(result.ok).toBe(true) + }) + }) + + // --- deleteAgent ------------------------------------------------------------- + + describe('deleteAgent', () => { + it('returns not_found for a missing id', async () => { + const { store } = await freshStore() + const result = await store.deleteAgent('00000000-0000-0000-0000-000000000000') + expect(result).toEqual({ ok: false, reason: 'not_found' }) + }) + + it('deleting the ONLY active admin is refused: last_admin', async () => { + const { store } = await freshStore() + const admin = await store.createFirstAdmin({ + name: 'Solo Admin', + email: 'solo@example.test', + passwordHash: 'scrypt$hash', + }) + if (admin === null) throw new Error('expected an admin') + const result = await store.deleteAgent(admin.id) + expect(result).toEqual({ ok: false, reason: 'last_admin' }) + }) + + it('deleting one of two active admins succeeds', async () => { + const { store } = await freshStore() + const admin1 = await store.createFirstAdmin({ + name: 'Admin One', + email: 'admin1@example.test', + passwordHash: 'scrypt$hash', + }) + if (admin1 === null) throw new Error('expected an admin') + const created2 = await store.createAgent({ + name: 'Admin Two', + email: 'admin2@example.test', + role: 'admin', + status: 'active', + passwordHash: 'scrypt$hash2', + }) + if (!created2.ok) throw new Error('expected ok') + + const result = await store.deleteAgent(admin1.id) + expect(result).toEqual({ ok: true }) + expect(await store.getAgent(admin1.id)).toBeNull() + }) + + it('deleting a non-admin never triggers the guard', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Plain Agent', + email: 'plain@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + const result = await store.deleteAgent(created.agent.id) + expect(result).toEqual({ ok: true }) + }) + + it('cascades password identities', async () => { + const { db, store } = await freshStore() + const created = await store.createAgent({ + name: 'Has Identity', + email: 'has-identity@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + + await store.deleteAgent(created.agent.id) + + const remaining = await db.query('SELECT id FROM agent_auth_identities WHERE agent_id = $1', [ + created.agent.id, + ]) + expect(remaining).toEqual([]) + }) + + it('un-assigns (does not delete) conversations the deleted Agent was assigned to', async () => { + const { db, store } = await freshStore() + const created = await store.createAgent({ + name: 'Assignee', + email: 'assignee@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + + const [conversation] = await db.query<{ id: string }>( + 'INSERT INTO conversations (customer_email, assignee_agent_id) VALUES ($1, $2) RETURNING id', + ['customer@example.test', created.agent.id], + ) + + await store.deleteAgent(created.agent.id) + + const [row] = await db.query<{ assignee_agent_id: string | null }>( + 'SELECT assignee_agent_id FROM conversations WHERE id = $1', + [conversation.id], + ) + expect(row.assignee_agent_id).toBeNull() + }) + }) + + // --- setPassword / getPasswordIdentity(ByEmail) ----------------------------- + + describe('setPassword / getPasswordIdentity / getPasswordIdentityByEmail', () => { + it('sets a password identity for an Agent with none yet', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'No Password Yet', + email: 'nopass@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + + await store.setPassword(created.agent.id, 'scrypt$new-hash') + const identity = await store.getPasswordIdentity(created.agent.id) + expect(identity).toEqual({ agentId: created.agent.id, secretHash: 'scrypt$new-hash' }) + }) + + it('replaces an existing password identity (honors the one-password-per-agent partial unique index)', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Has Password', + email: 'haspass@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$original', + }) + if (!created.ok) throw new Error('expected ok') + + await store.setPassword(created.agent.id, 'scrypt$replacement') + const identity = await store.getPasswordIdentity(created.agent.id) + expect(identity?.secretHash).toBe('scrypt$replacement') + }) + + it('throws for a nonexistent agentId', async () => { + const { store } = await freshStore() + await expect( + store.setPassword('00000000-0000-0000-0000-000000000000', 'scrypt$hash'), + ).rejects.toThrow() + }) + + it('getPasswordIdentityByEmail joins through agents, case-insensitively', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Email Lookup', + email: 'lookup@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + + const identity = await store.getPasswordIdentityByEmail('LOOKUP@example.test') + expect(identity).toEqual({ agentId: created.agent.id, secretHash: 'scrypt$hash' }) + }) + + it('getPasswordIdentityByEmail returns null for an unknown email', async () => { + const { store } = await freshStore() + expect(await store.getPasswordIdentityByEmail('nobody@example.test')).toBeNull() + }) + }) + + // --- acceptInvite -------------------------------------------------------------- + + describe('acceptInvite', () => { + it('flips invited -> active and sets the password, atomically', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Invitee', + email: 'invitee@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + + const agent = await store.acceptInvite(created.agent.id, 'scrypt$new-password') + expect(agent?.status).toBe('active') + const identity = await store.getPasswordIdentity(created.agent.id) + expect(identity?.secretHash).toBe('scrypt$new-password') + }) + + it('is one-time: a second accept affects zero rows and returns null', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Invitee', + email: 'invitee2@example.test', + role: 'agent', + status: 'invited', + }) + if (!created.ok) throw new Error('expected ok') + + const first = await store.acceptInvite(created.agent.id, 'scrypt$first') + expect(first).not.toBeNull() + + const second = await store.acceptInvite(created.agent.id, 'scrypt$second') + expect(second).toBeNull() + + // The FIRST password is what survived — the replay never overwrote it. + const identity = await store.getPasswordIdentity(created.agent.id) + expect(identity?.secretHash).toBe('scrypt$first') + }) + + it('returns null for a nonexistent Agent', async () => { + const { store } = await freshStore() + const result = await store.acceptInvite('00000000-0000-0000-0000-000000000000', 'scrypt$hash') + expect(result).toBeNull() + }) + + it('returns null for an already-active Agent (not invited)', async () => { + const { store } = await freshStore() + const created = await store.createAgent({ + name: 'Already Active', + email: 'already-active@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$original', + }) + if (!created.ok) throw new Error('expected ok') + + const result = await store.acceptInvite(created.agent.id, 'scrypt$hijack') + expect(result).toBeNull() + const identity = await store.getPasswordIdentity(created.agent.id) + expect(identity?.secretHash).toBe('scrypt$original') + }) + }) + + // --- countAgents --------------------------------------------------------------- + + describe('countAgents', () => { + it('is 0 on a fresh database', async () => { + const { store } = await freshStore() + expect(await store.countAgents()).toBe(0) + }) + + it('counts every Agent regardless of status', async () => { + const { store } = await freshStore() + await store.createFirstAdmin({ + name: 'Admin', + email: 'admin@example.test', + passwordHash: 'scrypt$hash', + }) + await store.createAgent({ + name: 'Invited', + email: 'invited@example.test', + role: 'agent', + status: 'invited', + }) + expect(await store.countAgents()).toBe(2) + }) + }) +}) diff --git a/src/store/agents.ts b/src/store/agents.ts new file mode 100644 index 0000000..de38062 --- /dev/null +++ b/src/store/agents.ts @@ -0,0 +1,507 @@ +/** + * `AgentStore` — persistence for `agents` and `agent_auth_identities` + * (migration 018, `src/db/migrate.ts`; HT-54, specs/auth/agents-and-auth.md). + * + * Follows this codebase's standing store convention: an interface + a + * `create*Store(db)` factory, raw parameterized SQL over the `Db`/`Queryable` + * seam (`src/db/client.ts`), expected failures signaled as discriminated + * result unions rather than thrown exceptions (mirrors + * `ConversationStore.appendThread`'s `AppendResult`) — a duplicate email or a + * would-be last-admin removal is an ordinary, anticipated outcome of calling + * this API, not a bug. + * + * ## The last-admin invariant (spec §5) and the advisory lock + * + * Under Postgres's default READ COMMITTED isolation, two concurrent + * mutations that each reduce the active-admin set (demote, disable, or + * delete an admin) can both observe "there are 2 active admins" in their own + * snapshot and both proceed, silently dropping the count to zero — a + * guard predicate ALONE does not close this race (spec §5's own worked + * example). Every mutation here that CAN reduce the active-admin set + * therefore runs inside a transaction that takes a + * {@link AGENTS_ADMIN_ADVISORY_LOCK_KEY} `pg_advisory_xact_lock` FIRST, then + * re-checks the count under that lock — the same tool `src/db/migrate.ts` + * already uses for its own cross-instance race, applied here to a + * cross-request one. {@link createFirstAdmin} shares this SAME lock key + * (spec §6 says so explicitly: "the same `pg_advisory_xact_lock` the + * last-admin guard uses") — a distinct key from `migrate.ts`'s own + * {@link MIGRATION_ADVISORY_LOCK_KEY}-equivalent, since these are unrelated + * critical sections that must never block each other. + * + * `updateAgent`/`deleteAgent` only pay the lock's cost when the mutation + * actually TOUCHES `role`/`status` (i.e., could conceivably matter) — a + * plain name/timezone edit never takes it. Once a role/status mutation does + * take the lock, the guard is evaluated unconditionally inside it (rather + * than trying to skip the count query for "obviously safe" cases like + * promoting to admin) — the extra count query is cheap, and a single + * "always guard once locked" code path is far easier to prove correct than + * a "guard only on the branches that reduce" one; see each method's comment + * for the exact condition. + */ + +import type { Db, SqlValue } from '../db/client.js' + +/** An Agent's role (spec §5): `admin` manages Agents/settings and can do everything an `agent` can; `agent` works the inbox and their own profile. */ +export type AgentRole = 'admin' | 'agent' + +/** + * An Agent's lifecycle status (spec §3.1). `invited` is produced ONLY by the + * invite-provisioning path and exits only via invite acceptance or + * delete/re-create (spec §6's closed status lifecycle) — never settable by + * `PATCH`. `active` can sign in; `disabled` is a reversible soft-off. + */ +export type AgentStatus = 'invited' | 'active' | 'disabled' + +/** An Agent, as read back from storage. Never carries a secret — `secret_hash` lives only in `agent_auth_identities`, a separate table this type has no field for. */ +export interface AgentRecord { + id: string + email: string + name: string + role: AgentRole + status: AgentStatus + timezone: string + createdAt: Date + updatedAt: Date +} + +/** A stored `password` identity, as {@link AgentStore.getPasswordIdentity}/{@link AgentStore.getPasswordIdentityByEmail} return it — exactly the two fields `PasswordAuthProvider` needs. */ +export interface PasswordIdentity { + agentId: string + secretHash: string +} + +/** Fields {@link AgentStore.updateAgent} may change. `email` is deliberately absent — immutable in v1 (spec §3.2, §6); re-create the Agent to change it. */ +export interface AgentUpdate { + name?: string + timezone?: string + role?: AgentRole + status?: 'active' | 'disabled' +} + +/** The outcome of {@link AgentStore.createAgent}. */ +export type CreateAgentResult = + | { ok: true; agent: AgentRecord } + | { ok: false; reason: 'email_taken' } + +/** The outcome of {@link AgentStore.updateAgent}. */ +export type UpdateAgentResult = + | { ok: true; agent: AgentRecord } + | { ok: false; reason: 'not_found' | 'last_admin' } + +/** The outcome of {@link AgentStore.deleteAgent}. */ +export type DeleteAgentResult = { ok: true } | { ok: false; reason: 'not_found' | 'last_admin' } + +/** Persistence operations for `agents` and `agent_auth_identities`. See the module doc for the last-admin locking discipline. */ +export interface AgentStore { + /** + * Create the FIRST admin (spec §6's zero-Agents-gated `/setup`): an + * `agents` row (`role='admin'`, `status='active'`) plus its `password` + * identity, in ONE transaction guarded by + * {@link AGENTS_ADMIN_ADVISORY_LOCK_KEY} — the same + * `pg_advisory_xact_lock`-then-check pattern `migrate()` uses, closing the + * "two concurrent zero-Agents checks both see an empty table" race (module + * doc). Returns the created {@link AgentRecord}, or `null` if another call + * won the race (or Agents already existed) — the caller maps `null` to + * `409`. + */ + createFirstAdmin(input: { + name: string + email: string + passwordHash: string + }): Promise + + /** + * Create an Agent (admin-authored, spec §6/§8): with `passwordHash` + * present, also inserts its `password` identity in the SAME transaction + * (the admin-set-password provisioning path, `status` must be `'active'`); + * with `passwordHash` omitted, no identity is created (the invite path, + * `status` must be `'invited'` — no usable credential yet, spec §3.1). The + * caller (`src/api/agents.ts`) is what enforces "exactly one of + * `sendInvite`/`password`" and picks the matching `status`; this method + * does not re-derive that choice. + * + * A duplicate email (the `agents_email_key` unique index, case-insensitive) + * is an `INSERT ... ON CONFLICT (lower(email)) DO NOTHING` — `{ ok: false, + * reason: 'email_taken' }`, never a thrown constraint-violation error, so + * the API layer can map it to `409` without parsing a raw pg error. + */ + createAgent(input: { + name: string + email: string + role: AgentRole + status: 'invited' | 'active' + passwordHash?: string + }): Promise + + /** Look up an Agent by id. `null` if no row has that id. */ + getAgent(id: string): Promise + + /** Look up an Agent by email, case-insensitively (`lower(email) = lower($1)`). `null` if no row matches. */ + getAgentByEmail(email: string): Promise + + /** List every Agent, ordered by `name` — the roster `GET /api/v1/agents` (spec §6) serves to any active Agent (any Agent may assign any Agent, spec §5's role model). */ + listAgents(): Promise + + /** + * Apply `patch` to Agent `id`. When `patch.role`/`patch.status` is + * present, runs inside the advisory-locked last-admin guard (module doc); + * a bare `{name?, timezone?}` patch (both `role` and `status` omitted) + * is a plain, unlocked `UPDATE` — it can never reduce the active-admin + * set. Returns the updated record, `{ ok: false, reason: 'not_found' }` + * if `id` doesn't exist, or `{ ok: false, reason: 'last_admin' }` if the + * change would leave the deployment with zero active admins. The + * **status lifecycle** (PATCH may only target `active`/`disabled`, + * `AgentUpdate['status']` is typed to exclude `'invited'` entirely) and + * the "an `invited` Agent's status is immovable via PATCH" rule are both + * enforced by the API layer (`src/api/agents.ts`), which has the + * pre-mutation Agent it needs to check "was this Agent `invited`" — + * this store method only knows the last-admin invariant. + */ + updateAgent(id: string, patch: AgentUpdate): Promise + + /** + * Hard delete: cascades `agent_auth_identities` (FK `ON DELETE CASCADE`) + * and un-assigns any conversation this Agent held (FK `ON DELETE SET + * NULL` on `conversations.assignee_agent_id`) — both handled by the + * schema, not application code. ALWAYS runs inside the advisory-locked + * last-admin guard (module doc) — unlike {@link updateAgent}, there is no + * cheap "obviously safe" fast path worth special-casing here, since a + * delete's effect on the admin set depends on the row's CURRENT + * role/status, which this method must read under the lock anyway. + */ + deleteAgent(id: string): Promise + + /** + * Set (insert or replace) Agent `agentId`'s single `password` identity — + * `subject` is always written as the Agent's CURRENT email (read from the + * same `agents` row inside this one statement, never a caller-supplied + * value that could drift from it). Honors the partial unique index + * (`agent_auth_identities_one_password_per_agent`) via `ON CONFLICT + * (agent_id) WHERE provider = 'password' DO UPDATE`. Throws if no Agent + * exists with `agentId` — every caller (`src/api/agents.ts`) already + * loaded the Agent to check its status before calling this, so a genuinely + * missing row here is structurally unreachable in practice, matching + * `MailboxStore.markDisconnected`'s same throw-on-zero-rows convention. + */ + setPassword(agentId: string, passwordHash: string): Promise + + /** The Agent's `password` identity, if any — `{ agentId, secretHash }` or `null`. */ + getPasswordIdentity(agentId: string): Promise + + /** The `password` identity for the Agent whose email matches (case-insensitively), joined through `agents` — what `PasswordAuthProvider` looks up on every login attempt. `null` if no Agent has that email, or the Agent has no `password` identity. */ + getPasswordIdentityByEmail(email: string): Promise + + /** + * Atomically accept an invite: `UPDATE agents SET status = 'active' ... + * WHERE id = $1 AND status = 'invited'` and set the Agent's `password` + * identity, in ONE transaction (spec §6/§9 — this is what makes accepting + * the SAME invite twice, or a replay after the Agent is already `active`, + * a no-op rather than a second password write). Returns the updated + * {@link AgentRecord} on success, or `null` if the `UPDATE` matched zero + * rows (no such Agent, or the Agent was not `invited`) — the caller + * (`src/api/agents.ts`) maps `null` to the same generic `401` an invalid + * token gets, so expired/replayed/invalid are indistinguishable (spec + * §6). + */ + acceptInvite(agentId: string, passwordHash: string): Promise + + /** Count every Agent regardless of status — `needsSetup` (`GET /api/v1/auth/providers`, spec §6) is `count === 0`. */ + countAgents(): Promise +} + +/** + * The advisory lock key {@link createFirstAdmin}'s zero-Agents guard and the + * last-admin guard (`updateAgent`/`deleteAgent`) both serialize on — a + * DISTINCT constant from `src/db/migrate.ts`'s own + * `MIGRATION_ADVISORY_LOCK_KEY` (module doc): these are unrelated critical + * sections (schema migration vs. admin-roster mutation) that must never + * contend with each other. Chosen larger than `int4`'s max (2^31-1) so it + * binds unambiguously to `pg_advisory_xact_lock`'s `bigint` overload, same + * reasoning as `migrate.ts`'s constant. + */ +export const AGENTS_ADMIN_ADVISORY_LOCK_KEY = 7_331_009_881 + +/** Raw `agents` row shape, before mapping to {@link AgentRecord}. */ +interface AgentRow { + id: string + email: string + name: string + role: string + status: string + timezone: string + created_at: Date | string + updated_at: Date | string +} + +const AGENT_COLUMNS = 'id, email, name, role, status, timezone, created_at, updated_at' + +function toDate(value: Date | string): Date { + return value instanceof Date ? value : new Date(value) +} + +function toAgentRecord(row: AgentRow): AgentRecord { + return { + id: row.id, + email: row.email, + name: row.name, + role: row.role as AgentRole, + status: row.status as AgentStatus, + timezone: row.timezone, + createdAt: toDate(row.created_at), + updatedAt: toDate(row.updated_at), + } +} + +/** Create an {@link AgentStore} backed by `db`. */ +export function createAgentStore(db: Db): AgentStore { + return { + async createFirstAdmin(input) { + const email = input.email.trim().toLowerCase() + return db.transaction(async (tx) => { + await tx.query('SELECT pg_advisory_xact_lock($1::bigint)', [AGENTS_ADMIN_ADVISORY_LOCK_KEY]) + const existing = await tx.query('SELECT 1 FROM agents LIMIT 1') + if (existing.length > 0) return null + + const [agentRow] = await tx.query( + `INSERT INTO agents (email, name, role, status) + VALUES ($1, $2, 'admin', 'active') + RETURNING ${AGENT_COLUMNS}`, + [email, input.name], + ) + await tx.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + VALUES ($1, 'password', $2, $3)`, + [agentRow.id, email, input.passwordHash], + ) + return toAgentRecord(agentRow) + }) + }, + + async createAgent(input) { + const email = input.email.trim().toLowerCase() + return db.transaction(async (tx) => { + const rows = await tx.query( + `INSERT INTO agents (email, name, role, status) + VALUES ($1, $2, $3, $4) + ON CONFLICT (lower(email)) DO NOTHING + RETURNING ${AGENT_COLUMNS}`, + [email, input.name, input.role, input.status], + ) + if (rows.length === 0) { + return { ok: false, reason: 'email_taken' } + } + const agentRow = rows[0] + if (input.passwordHash !== undefined) { + await tx.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + VALUES ($1, 'password', $2, $3)`, + [agentRow.id, email, input.passwordHash], + ) + } + return { ok: true, agent: toAgentRecord(agentRow) } + }) + }, + + async getAgent(id) { + const rows = await db.query(`SELECT ${AGENT_COLUMNS} FROM agents WHERE id = $1`, [ + id, + ]) + const row = rows[0] + return row === undefined ? null : toAgentRecord(row) + }, + + async getAgentByEmail(email) { + const rows = await db.query( + `SELECT ${AGENT_COLUMNS} FROM agents WHERE lower(email) = lower($1)`, + [email], + ) + const row = rows[0] + return row === undefined ? null : toAgentRecord(row) + }, + + async listAgents() { + const rows = await db.query(`SELECT ${AGENT_COLUMNS} FROM agents ORDER BY name`) + return rows.map(toAgentRecord) + }, + + async updateAgent(id, patch) { + // A plain name/timezone-only patch can never reduce the active-admin + // set — no lock needed, no last-admin re-check, just an ordinary + // UPDATE (module doc). + if (patch.role === undefined && patch.status === undefined) { + const sets: string[] = [] + const params: SqlValue[] = [] + if (patch.name !== undefined) { + params.push(patch.name) + sets.push(`name = $${params.length}`) + } + if (patch.timezone !== undefined) { + params.push(patch.timezone) + sets.push(`timezone = $${params.length}`) + } + if (sets.length === 0) { + // Nothing to change — a no-op patch. Fetch-and-return rather than + // issue a malformed `UPDATE ... SET , updated_at = now()`; matches + // ConversationStore.appendThread's "a replay that changes nothing + // touches the row not at all" convention. + const rows = await db.query( + `SELECT ${AGENT_COLUMNS} FROM agents WHERE id = $1`, + [id], + ) + const row = rows[0] + return row === undefined + ? { ok: false, reason: 'not_found' } + : { ok: true, agent: toAgentRecord(row) } + } + params.push(id) + const rows = await db.query( + `UPDATE agents SET ${sets.join(', ')}, updated_at = now() + WHERE id = $${params.length} + RETURNING ${AGENT_COLUMNS}`, + params, + ) + const row = rows[0] + return row === undefined + ? { ok: false, reason: 'not_found' } + : { ok: true, agent: toAgentRecord(row) } + } + + // role and/or status is being touched — always take the advisory lock + // and re-check the guard, even on a branch that plainly can't reduce + // the admin set (e.g. promoting to admin): one code path, provably + // correct, cheap enough (module doc). + return db.transaction(async (tx) => { + await tx.query('SELECT pg_advisory_xact_lock($1::bigint)', [AGENTS_ADMIN_ADVISORY_LOCK_KEY]) + const current = await tx.query( + `SELECT ${AGENT_COLUMNS} FROM agents WHERE id = $1 FOR UPDATE`, + [id], + ) + const currentRow = current[0] + if (currentRow === undefined) { + return { ok: false, reason: 'not_found' } + } + + const newRole = patch.role ?? currentRow.role + const newStatus = patch.status ?? currentRow.status + const wasActiveAdmin = currentRow.role === 'admin' && currentRow.status === 'active' + const willBeActiveAdmin = newRole === 'admin' && newStatus === 'active' + if (wasActiveAdmin && !willBeActiveAdmin) { + const [{ count }] = await tx.query<{ count: number }>( + "SELECT count(*)::int AS count FROM agents WHERE role = 'admin' AND status = 'active'", + ) + if (count <= 1) { + return { ok: false, reason: 'last_admin' } + } + } + + const sets: string[] = ['role = $1', 'status = $2'] + const params: SqlValue[] = [newRole, newStatus] + if (patch.name !== undefined) { + params.push(patch.name) + sets.push(`name = $${params.length}`) + } + if (patch.timezone !== undefined) { + params.push(patch.timezone) + sets.push(`timezone = $${params.length}`) + } + params.push(id) + const [updatedRow] = await tx.query( + `UPDATE agents SET ${sets.join(', ')}, updated_at = now() + WHERE id = $${params.length} + RETURNING ${AGENT_COLUMNS}`, + params, + ) + return { ok: true, agent: toAgentRecord(updatedRow) } + }) + }, + + async deleteAgent(id) { + return db.transaction(async (tx) => { + await tx.query('SELECT pg_advisory_xact_lock($1::bigint)', [AGENTS_ADMIN_ADVISORY_LOCK_KEY]) + const rows = await tx.query<{ role: string; status: string }>( + 'SELECT role, status FROM agents WHERE id = $1 FOR UPDATE', + [id], + ) + const row = rows[0] + if (row === undefined) { + return { ok: false, reason: 'not_found' } + } + if (row.role === 'admin' && row.status === 'active') { + const [{ count }] = await tx.query<{ count: number }>( + "SELECT count(*)::int AS count FROM agents WHERE role = 'admin' AND status = 'active'", + ) + if (count <= 1) { + return { ok: false, reason: 'last_admin' } + } + } + await tx.query('DELETE FROM agents WHERE id = $1', [id]) + return { ok: true } + }) + }, + + async setPassword(agentId, passwordHash) { + const rows = await db.query<{ agent_id: string }>( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + SELECT id, 'password', email, $2 FROM agents WHERE id = $1 + ON CONFLICT (agent_id) WHERE provider = 'password' + DO UPDATE SET secret_hash = EXCLUDED.secret_hash, subject = EXCLUDED.subject, updated_at = now() + RETURNING agent_id`, + [agentId, passwordHash], + ) + if (rows.length === 0) { + throw new Error(`setPassword: no agent with id ${agentId}`) + } + }, + + async getPasswordIdentity(agentId) { + const rows = await db.query<{ agent_id: string; secret_hash: string }>( + `SELECT agent_id, secret_hash FROM agent_auth_identities + WHERE agent_id = $1 AND provider = 'password'`, + [agentId], + ) + const row = rows[0] + return row === undefined ? null : { agentId: row.agent_id, secretHash: row.secret_hash } + }, + + async getPasswordIdentityByEmail(email) { + const rows = await db.query<{ agent_id: string; secret_hash: string }>( + `SELECT ai.agent_id, ai.secret_hash + FROM agent_auth_identities ai + JOIN agents a ON a.id = ai.agent_id + WHERE lower(a.email) = lower($1) AND ai.provider = 'password'`, + [email], + ) + const row = rows[0] + return row === undefined ? null : { agentId: row.agent_id, secretHash: row.secret_hash } + }, + + async acceptInvite(agentId, passwordHash) { + return db.transaction(async (tx) => { + const rows = await tx.query( + `UPDATE agents SET status = 'active', updated_at = now() + WHERE id = $1 AND status = 'invited' + RETURNING ${AGENT_COLUMNS}`, + [agentId], + ) + const row = rows[0] + if (row === undefined) return null + + await tx.query( + `INSERT INTO agent_auth_identities (agent_id, provider, subject, secret_hash) + VALUES ($1, 'password', $2, $3) + ON CONFLICT (agent_id) WHERE provider = 'password' + DO UPDATE SET secret_hash = EXCLUDED.secret_hash, subject = EXCLUDED.subject, updated_at = now()`, + [agentId, row.email, passwordHash], + ) + return toAgentRecord(row) + }) + }, + + async countAgents() { + const [{ count }] = await db.query<{ count: number }>( + 'SELECT count(*)::int AS count FROM agents', + ) + return count + }, + } +} diff --git a/src/store/conversations.test.ts b/src/store/conversations.test.ts index 9742063..60e7afd 100644 --- a/src/store/conversations.test.ts +++ b/src/store/conversations.test.ts @@ -995,18 +995,27 @@ describe('createConversationStore', () => { }) }) - describe('tags & assignee (HT-29/HT-31, spec §4e/§4f v1.1)', () => { - it('defaults: a new conversation has [] tags and null assignee, on summaries and detail alike', async () => { + describe('tags & assignee (HT-29/HT-31, spec §4e/§4f v1.1; HT-54 graduates assignee to a real Agent id, spec §3.3)', () => { + /** Insert a minimal `agents` row directly (raw SQL, not via `AgentStore`, matching this file's fixture-setup convention) — just enough to satisfy `assignee_agent_id`'s FK. */ + async function insertAgent(db: Db, email: string): Promise { + const [row] = await db.query<{ id: string }>( + 'INSERT INTO agents (email, name) VALUES ($1, $2) RETURNING id', + [email, 'Test Agent'], + ) + return row.id + } + + it('defaults: a new conversation has [] tags and null assigneeAgentId, on summaries and detail alike', async () => { const { store } = await freshStore() const { conversationId } = await store.createConversation(newConversation()) const [summary] = await store.listConversations({ limit: 50 }) expect(summary.tags).toEqual([]) - expect(summary.assignee).toBeNull() + expect(summary.assigneeAgentId).toBeNull() const detail = await store.getConversation(conversationId) expect(detail?.tags).toEqual([]) - expect(detail?.assignee).toBeNull() + expect(detail?.assigneeAgentId).toBeNull() }) it('setConversationTags replace-set round-trip: set, re-set, clear — persisted verbatim, no updated_at bump', async () => { @@ -1026,32 +1035,48 @@ describe('createConversationStore', () => { expect(cleared?.tags).toEqual([]) }) - it('setConversationAssignee claims and releases, no updated_at bump', async () => { + it('setConversationAssignee assigns and releases, no updated_at bump', async () => { const { db, store } = await freshStore() const { conversationId } = await store.createConversation(newConversation()) await setUpdatedAt(db, conversationId, new Date('2020-01-01T00:00:00.000Z')) + const agentId = await insertAgent(db, 'agent@example.test') - const claimed = await store.setConversationAssignee(conversationId, 'me') - expect(claimed?.assignee).toBe('me') - expect(claimed?.updatedAt.getTime()).toBe(new Date('2020-01-01T00:00:00.000Z').getTime()) + const claimed = await store.setConversationAssignee(conversationId, agentId) + if (claimed === null || claimed === 'invalid_agent') throw new Error('expected a summary') + expect(claimed.assigneeAgentId).toBe(agentId) + expect(claimed.updatedAt.getTime()).toBe(new Date('2020-01-01T00:00:00.000Z').getTime()) const released = await store.setConversationAssignee(conversationId, null) - expect(released?.assignee).toBeNull() + if (released === null || released === 'invalid_agent') throw new Error('expected a summary') + expect(released.assigneeAgentId).toBeNull() + }) + + it("setConversationAssignee returns 'invalid_agent' when the id no longer names an Agent (the FK race, translated)", async () => { + const { store } = await freshStore() + const { conversationId } = await store.createConversation(newConversation()) + // Calling the store DIRECTLY with an id no Agent has — the same state + // the API's check-then-act race lands in when the Agent is deleted + // between the handler's existence check and this UPDATE. + const outcome = await store.setConversationAssignee(conversationId, RANDOM_UUID) + expect(outcome).toBe('invalid_agent') + const raw = await store.getConversation(conversationId) + expect(raw?.assigneeAgentId).toBeNull() }) it('both return null for a missing or deleted conversation — nothing is written', async () => { const { db, store } = await freshStore() const { conversationId } = await store.createConversation(newConversation()) await setStatus(db, conversationId, 'deleted') + const agentId = await insertAgent(db, 'agent2@example.test') expect(await store.setConversationTags(RANDOM_UUID, ['x'])).toBeNull() expect(await store.setConversationTags(conversationId, ['x'])).toBeNull() - expect(await store.setConversationAssignee(RANDOM_UUID, 'me')).toBeNull() - expect(await store.setConversationAssignee(conversationId, 'me')).toBeNull() + expect(await store.setConversationAssignee(RANDOM_UUID, agentId)).toBeNull() + expect(await store.setConversationAssignee(conversationId, agentId)).toBeNull() const raw = await store.getConversation(conversationId) expect(raw?.tags).toEqual([]) - expect(raw?.assignee).toBeNull() + expect(raw?.assigneeAgentId).toBeNull() }) }) diff --git a/src/store/conversations.ts b/src/store/conversations.ts index 4e048c6..4c4c70d 100644 --- a/src/store/conversations.ts +++ b/src/store/conversations.ts @@ -249,11 +249,12 @@ export interface StoredConversation { /** Short lowercase labels, replace-set via {@link ConversationStore.setConversationTags} (v1.1, HT-29). `[]` default. */ tags: string[] /** - * Single-Agent claim flag (v1.1, HT-31): `'me'` = the deployment's one - * operator, `null` = Anyone. Deliberately NOT identity — the multi-Agent - * increment replaces `'me'` with real Agent ids (spec §4f). + * The assigned Agent's id, or `null` for Anyone (v1.1, HT-31; HT-54: + * graduated from the single-operator `'me'` flag to a real Agent identity + * — specs/auth/agents-and-auth.md §3.3, a coordinated breaking change with + * `PUT /api/v1/conversations/{id}/assignee`'s new body shape). */ - assignee: 'me' | null + assigneeAgentId: string | null createdAt: Date updatedAt: Date } @@ -485,15 +486,24 @@ export interface ConversationStore { setConversationTags(conversationId: string, tags: string[]): Promise /** - * Claim (`'me'`) or release (`null`) a conversation — the write path - * behind `PUT /api/v1/conversations/{id}/assignee` (spec §4f, v1.1). Does - * NOT bump `updated_at` (spec §4f). Returns the updated summary, or - * `null` for a missing/deleted conversation. + * Assign a conversation to `assigneeAgentId`, or release it (`null`) — + * the write path behind `PUT /api/v1/conversations/{id}/assignee` + * (spec §4f, v1.1; graduated to a real Agent id by HT-54, + * specs/auth/agents-and-auth.md §3.3/§10 — the new body shape is + * `{ assigneeAgentId: uuid | null }`, breaking). The caller + * (`src/api/conversations.ts`) is what validates `assigneeAgentId` + * names an existing Agent before calling this — but that check-then-act + * pair is not atomic (the Agent can be hard-deleted between the two), so + * the `assignee_agent_id` FK (migration 018) is the real guard and its + * violation is translated here to `'invalid_agent'` rather than escaping + * as an uncontrolled error. Does NOT bump `updated_at` (spec §4f). + * Returns the updated summary, `null` for a missing/deleted conversation, + * or `'invalid_agent'` when the id no longer names an Agent. */ setConversationAssignee( conversationId: string, - assignee: 'me' | null, - ): Promise + assigneeAgentId: string | null, + ): Promise /** * Record that the customer viewed an outbound thread (open tracking, spec @@ -545,11 +555,25 @@ const PREVIEW_MAX_LENGTH = 120 * caller data. */ function summaryReturningSql(idParam: string): string { - return `RETURNING id, number, subject, customer_email, status, tags, assignee, created_at, updated_at, + return `RETURNING id, number, subject, customer_email, status, tags, assignee_agent_id, created_at, updated_at, (SELECT count(*)::int FROM threads WHERE conversation_id = ${idParam}) AS thread_count, (SELECT t.body_text FROM threads t WHERE t.conversation_id = ${idParam} AND t.body_text IS NOT NULL ORDER BY t.created_at DESC, t.id DESC LIMIT 1) AS latest_body_text` } +/** + * Is `err` the `conversations.assignee_agent_id` FK rejecting a + * just-deleted Agent? Matched by SQLSTATE 23503 (foreign_key_violation) + * when the driver surfaces it (`pg` and PGlite both set `code`), with the + * constraint/message text as a fallback so a driver that doesn't is still + * recognized. Total: any non-object input is simply "no". + */ +function isAssigneeFkViolation(err: unknown): boolean { + if (typeof err !== 'object' || err === null) return false + const { code, message } = err as { code?: unknown; message?: unknown } + if (code === '23503') return true + return typeof message === 'string' && message.includes('assignee_agent_id') +} + /** * Derive a `ConversationSummary.preview` from a thread body (spec §2, v1.1): * whitespace collapsed to single spaces, trimmed, first @@ -584,8 +608,8 @@ export interface ConversationSummary { preview: string /** Short lowercase labels — see {@link StoredConversation.tags}. */ tags: string[] - /** Single-Agent claim flag — see {@link StoredConversation.assignee}. */ - assignee: 'me' | null + /** The assigned Agent's id, or `null` for Anyone — see {@link StoredConversation.assigneeAgentId}. */ + assigneeAgentId: string | null createdAt: Date updatedAt: Date } @@ -629,7 +653,7 @@ interface ConversationRow { status: string /** jsonb — arrives already-decoded (same driver behavior as `send_envelope`); this codebase only ever writes string arrays. */ tags: unknown - assignee: string | null + assignee_agent_id: string | null created_at: Date | string updated_at: Date | string } @@ -772,8 +796,8 @@ export function createConversationStore(db: Db): ConversationStore { // work is done proportional to a deleted conversation's size. const conversationRows = await db.query( includeDeleted - ? 'SELECT id, number, subject, customer_email, status, tags, assignee, created_at, updated_at FROM conversations WHERE id = $1' - : "SELECT id, number, subject, customer_email, status, tags, assignee, created_at, updated_at FROM conversations WHERE id = $1 AND status <> 'deleted'", + ? 'SELECT id, number, subject, customer_email, status, tags, assignee_agent_id, created_at, updated_at FROM conversations WHERE id = $1' + : "SELECT id, number, subject, customer_email, status, tags, assignee_agent_id, created_at, updated_at FROM conversations WHERE id = $1 AND status <> 'deleted'", [conversationId], ) const conversationRow = conversationRows[0] @@ -913,7 +937,7 @@ export function createConversationStore(db: Db): ConversationStore { const limitParam = params.length const rows = await db.query( - `SELECT c.id, c.number, c.subject, c.customer_email, c.status, c.tags, c.assignee, c.created_at, c.updated_at, ${THREAD_COUNT_SUBQUERY}, ${LATEST_BODY_TEXT_SUBQUERY} + `SELECT c.id, c.number, c.subject, c.customer_email, c.status, c.tags, c.assignee_agent_id, c.created_at, c.updated_at, ${THREAD_COUNT_SUBQUERY}, ${LATEST_BODY_TEXT_SUBQUERY} FROM conversations c WHERE ${conditions.join(' AND ')} ORDER BY c.updated_at DESC, c.id DESC @@ -952,15 +976,24 @@ export function createConversationStore(db: Db): ConversationStore { return row === undefined ? null : toConversationSummary(row) }, - async setConversationAssignee(conversationId, assignee) { + async setConversationAssignee(conversationId, assigneeAgentId) { // No updated_at bump: claiming is metadata, not activity (spec §4f). - const rows = await db.query( - `UPDATE conversations - SET assignee = $1 - WHERE id = $2 AND status <> 'deleted' - ${summaryReturningSql('$2')}`, - [assignee, conversationId], - ) + let rows: ConversationSummaryRow[] + try { + rows = await db.query( + `UPDATE conversations + SET assignee_agent_id = $1 + WHERE id = $2 AND status <> 'deleted' + ${summaryReturningSql('$2')}`, + [assigneeAgentId, conversationId], + ) + } catch (err) { + // The Agent was deleted between the caller's existence check and this + // UPDATE — the FK is the authoritative guard for that race (interface + // doc above), and its violation is a caller-facing outcome, not a 500. + if (isAssigneeFkViolation(err)) return 'invalid_agent' + throw err + } const row = rows[0] return row === undefined ? null : toConversationSummary(row) }, @@ -1120,10 +1153,9 @@ function toStoredConversation(row: ConversationRow): StoredConversation { status: row.status as StoredConversation['status'], // Cast, not parsed — same reasoning as send_envelope in toStoredThread: // this codebase is the only writer (always a JSON string array), and the - // jsonb arrives already decoded. The assignee CHECK (migration 006) - // makes 'me'/NULL the only representable values. + // jsonb arrives already decoded. tags: row.tags as string[], - assignee: row.assignee as StoredConversation['assignee'], + assigneeAgentId: row.assignee_agent_id, createdAt: toDate(row.created_at), updatedAt: toDate(row.updated_at), } @@ -1147,7 +1179,7 @@ function toConversationSummary(row: ConversationSummaryRow): ConversationSummary threadCount: row.thread_count, preview: derivePreview(row.latest_body_text), tags: row.tags as string[], - assignee: row.assignee as ConversationSummary['assignee'], + assigneeAgentId: row.assignee_agent_id, createdAt: toDate(row.created_at), updatedAt: toDate(row.updated_at), }