From 35deec5d8d50384b474d5703b400cd48d9b7bd54 Mon Sep 17 00:00:00 2001 From: mthines Date: Sat, 17 Oct 2026 13:47:22 +0200 Subject: [PATCH 1/2] fix(web): stop calling revalidatePath during render on first sign-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildOnboardingSteps({ autoGenerateToken: true }) (rendered inside /lore's Suspense boundary) called the 'use server' generateToken action, whose success path calls revalidatePath — Next 15 throws during render for that, the bare catch{} swallowed it, and the user was left with an orphan api_tokens row and no token on screen. Split the mint core (insert + scoping + audit, no revalidate) into a new server-only lib/token-mint.ts. generateToken (lib/tokens.ts) now wraps it and revalidates only on success, from the action path. The render path goes through a new lib/first-token.ts (resolveFirstToken), which calls mintToken directly and never revalidates — the render already returns the freshly minted record as initialTokens, so nothing cached is stale. Adds lorekit.api_token.origin (onboarding|dashboard) to the generate span so prod can prove the fix. Pins both paths with node-env specs. --- docs/key-files.md | 4 +- docs/otel.md | 11 +- packages/web/src/lib/first-token.spec.ts | 94 +++++++ packages/web/src/lib/first-token.ts | 63 +++++ packages/web/src/lib/onboarding-steps.tsx | 32 +-- packages/web/src/lib/token-mint.spec.ts | 143 +++++++++++ packages/web/src/lib/token-mint.ts | 297 ++++++++++++++++++++++ packages/web/src/lib/tokens.spec.ts | 73 ++++++ packages/web/src/lib/tokens.ts | 271 ++------------------ 9 files changed, 721 insertions(+), 267 deletions(-) create mode 100644 packages/web/src/lib/first-token.spec.ts create mode 100644 packages/web/src/lib/first-token.ts create mode 100644 packages/web/src/lib/token-mint.spec.ts create mode 100644 packages/web/src/lib/token-mint.ts create mode 100644 packages/web/src/lib/tokens.spec.ts diff --git a/docs/key-files.md b/docs/key-files.md index 66f4330c8..a649a1c3d 100644 --- a/docs/key-files.md +++ b/docs/key-files.md @@ -10,7 +10,9 @@ else. Grouped loosely by subsystem in migration/PR order. | `packages/mcp-core/src/scope/scope.ts` | Canonical scope validation + wildcard expansion | | `packages/mcp-core/src/telemetry/telemetry.ts` | Shared tracer/meter getters, `lorekit.tool.duration` histogram | | `packages/web/src/lib/scope.ts` | Lightweight copy of `scopeType` for Next.js bundle (no OTel deps), plus `scopeRepoUrl` and `scopeRepoRef` — the ONE derivation of what a scope already says about a repository (`{repo, branch}`, null for `global`/`project`/malformed). `scopeRepoUrl` is built on `scopeRepoRef`, and `lib/origin.ts` consumes it so the stored provenance rows can never duplicate the scope-derived Repo row | -| `packages/web/src/lib/tokens.ts` | Server actions: `generateToken`, `listTokens`, `revokeToken` | +| `packages/web/src/lib/tokens.ts` | Server actions: `generateToken`, `listTokens`, `revokeToken`, `setTokenScoping`. `generateToken` is a thin wrapper over `token-mint.ts`'s `mintToken` that revalidates `/lore` and the `/settings` layout on success only — the action path, triggered by a form submission | +| `packages/web/src/lib/token-mint.ts` | `server-only` (not a Server Action) mint core: `mintToken`, `applyScoping`, `TOKEN_COLUMNS`, the `ApiToken`/`TokenPermission` types. Does NOT call `revalidatePath` — that is what lets `/lore`'s Server Component render mint a token directly (via `first-token.ts`) without hitting Next 15's "revalidatePath during render" error, which previously minted an orphan `api_tokens` row on every first sign-in | +| `packages/web/src/lib/first-token.ts` | `resolveFirstToken` — the first-sign-in auto-mint decision for `/lore`'s render, with the mint injected so it is node-env testable. Replaces a bare `catch {}` that used to swallow the `revalidatePath`-during-render error above along with every other failure | | `packages/web/src/lib/api/` | The dashboard's client for LoreKit's OWN REST API. `rest.ts` (runtime-agnostic `restFetch` — takes an access token rather than reaching for a session, so one module serves browser hooks and server actions; `RestApiError` carries the `{error, code}` envelope and a non-JSON body still yields a usable message), `memories.ts` (typed wrappers whose request/response types come from `@lorekit/schemas`, so the dashboard cannot invent a parameter the API does not accept), `session-browser.ts` / `session-server.ts` (the user's own Supabase session token, split by runtime because the server half imports `next/headers`). See `packages/web/CLAUDE.md` → "Data fetching goes through LoreKit's REST API" | | `packages/web/src/lib/lesson-entry.ts` | Pure `lessonFromMemoryEntry` — the ONE translation from a REST `MemoryEntry` to the dashboard's `LessonEntry`. Replaced three hand-written `row.key as string` mappings that had already drifted (one populated `id`, another did not) | | `packages/schemas/src/shared/tags.ts` | Pure `normalizeTagList` / `parseTagsParam` / `pgArrayLiteral` — label normalisation and Postgres array-literal quoting, shared by the `GET /memories` handler's `tags_mode=all` branch and the dashboard's label picker (which re-exports them from `lib/tag-filter.ts`). The `filter.ts` pattern: pure, part of the wire contract, needed by more than one runtime | diff --git a/docs/otel.md b/docs/otel.md index 45ae34b35..ce7ea7afc 100644 --- a/docs/otel.md +++ b/docs/otel.md @@ -804,11 +804,18 @@ and the outbound Supabase fetch calls; the custom spans add business context | `lorekit.org.invite.accept` | `lorekit.invite.id` | | `lorekit.org.invite.decline` | `lorekit.invite.id` | -### API token management (`lib/tokens.ts`) +### API token management (`lib/token-mint.ts`, `lib/tokens.ts`) + +The `lorekit.api_token.generate` span is emitted from `lib/token-mint.ts`'s `mintToken` — +the `server-only` mint core shared by both callers: `lib/tokens.ts`'s `generateToken` +action (dashboard "Generate token" button, revalidates on success) and `lib/first-token.ts`'s +`resolveFirstToken` (the `/lore` first-sign-in auto-mint, which runs during a Server +Component render and must never revalidate). `lorekit.api_token.origin` tells the two +apart in telemetry rather than guessing from auth tier. | Span name | Key attributes | |-----------|---------------| -| `lorekit.api_token.generate` | `lorekit.api_token.permissions`, `lorekit.api_token.id` (on success), `lorekit.api_token.limit_reached` (when cap is hit) | +| `lorekit.api_token.generate` | `lorekit.api_token.permissions`, `lorekit.api_token.origin` (`onboarding`\|`dashboard`), `lorekit.api_token.id` (on success), `lorekit.api_token.limit_reached` (when cap is hit) | | `lorekit.api_token.revoke` | `lorekit.api_token.id`, `lorekit.api_token.prefix` | On **any RPC/DB failure** the span also carries: diff --git a/packages/web/src/lib/first-token.spec.ts b/packages/web/src/lib/first-token.spec.ts new file mode 100644 index 000000000..89fceb39b --- /dev/null +++ b/packages/web/src/lib/first-token.spec.ts @@ -0,0 +1,94 @@ +/** + * `resolveFirstToken` owns the first-sign-in auto-mint decision for `/lore`'s + * render. It replaces a bare `catch {}` that used to hide the production bug + * (`token-mint.ts`'s header): these specs pin the decision table on its own, + * with `mint` injected so no Supabase/Next mocking is needed. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { resolveFirstToken } from './first-token'; +import type { ApiToken } from './token-mint'; + +const EXISTING: ApiToken = { + id: 'tok-existing', + name: 'Old token', + token_prefix: 'lk_rw_old...', + permissions: ['read', 'write'], + scopes: [], + org_access: 'all', + org_ids: [], + last_used_at: null, + created_at: '2025-01-01T00:00:00.000Z', +}; + +const MINTED: ApiToken = { + id: 'tok-new', + name: 'My first token', + token_prefix: 'lk_rw_new...', + permissions: ['read', 'write'], + scopes: [], + org_access: 'all', + org_ids: [], + last_used_at: null, + created_at: '2026-01-01T00:00:00.000Z', +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('resolveFirstToken', () => { + it('mints exactly once and returns the record + token when there are no existing tokens', async () => { + const mint = vi.fn().mockResolvedValue({ token: 'lk_rw_abc', record: MINTED }); + + const result = await resolveFirstToken({ existingTokens: [], autoGenerate: true, mint }); + + expect(mint).toHaveBeenCalledTimes(1); + expect(result).toEqual({ tokens: [MINTED], autoGeneratedToken: 'lk_rw_abc' }); + }); + + it('does not mint when existing tokens are non-empty', async () => { + const mint = vi.fn(); + + const result = await resolveFirstToken({ existingTokens: [EXISTING], autoGenerate: true, mint }); + + expect(mint).not.toHaveBeenCalled(); + expect(result).toEqual({ tokens: [EXISTING], autoGeneratedToken: null }); + }); + + it('does not mint when autoGenerate is false', async () => { + const mint = vi.fn(); + + const result = await resolveFirstToken({ existingTokens: [], autoGenerate: false, mint }); + + expect(mint).not.toHaveBeenCalled(); + expect(result).toEqual({ tokens: [], autoGeneratedToken: null }); + }); + + it('degrades to existing tokens when the mint returns an error', async () => { + const mint = vi.fn().mockResolvedValue({ error: 'cap reached' }); + + const result = await resolveFirstToken({ existingTokens: [], autoGenerate: true, mint }); + + expect(result).toEqual({ tokens: [], autoGeneratedToken: null }); + }); + + it('degrades to existing tokens when the mint throws a non-Next error', async () => { + const mint = vi.fn().mockRejectedValue(new Error('unexpected')); + + const result = await resolveFirstToken({ existingTokens: [EXISTING], autoGenerate: true, mint }); + + expect(result).toEqual({ tokens: [EXISTING], autoGeneratedToken: null }); + }); + + it('re-throws a Next control-flow error (redirect) rather than degrading', async () => { + const redirectError = new Error('NEXT_REDIRECT'); + redirectError.name = 'NEXT_REDIRECT'; + // Mirrors Next's internal digest convention that `unstable_rethrow` keys off. + (redirectError as unknown as { digest: string }).digest = 'NEXT_REDIRECT;replace;/login;307;'; + const mint = vi.fn().mockRejectedValue(redirectError); + + await expect(resolveFirstToken({ existingTokens: [], autoGenerate: true, mint })).rejects.toThrow( + 'NEXT_REDIRECT', + ); + }); +}); diff --git a/packages/web/src/lib/first-token.ts b/packages/web/src/lib/first-token.ts new file mode 100644 index 000000000..5c8de55b8 --- /dev/null +++ b/packages/web/src/lib/first-token.ts @@ -0,0 +1,63 @@ +/** + * The first-sign-in auto-mint decision for `/lore`'s onboarding render. + * + * Pulled out of `onboarding-steps.tsx` so it is a plain function a node-env + * `.spec.ts` can exercise with an injected `mint` — the render itself must + * never call a `'use server'` action whose success path revalidates (that is + * the bug `lib/token-mint.ts` exists to fix), so this module only ever calls + * the injected `mint`, never `generateToken`. + * + * Previously this decision lived inline behind a bare `catch {}`, which is + * exactly what hid the production bug: the mint's `revalidatePath` call threw + * during render, and the bare catch swallowed it along with every other kind + * of failure, leaving an orphan `api_tokens` row and no token on screen. + * `unstable_rethrow` restores the one distinction that catch erased — Next's + * own control-flow errors (redirect, not-found, the dynamic-rendering bailout) + * are not failures and must propagate, while a genuine mint failure degrades + * to "no auto token" so the page still renders. + */ +import { unstable_rethrow } from 'next/navigation'; +import type { ApiToken } from '@/lib/token-mint'; + +export interface ResolveFirstTokenArgs { + /** Tokens the user already has, as read by `listTokens()`. */ + existingTokens: ApiToken[]; + /** Whether the caller wants a first-sign-in auto-mint at all. */ + autoGenerate: boolean; + /** Injected effect — the render path passes `mintToken(..., 'onboarding')`. */ + mint: () => Promise<{ token: string; record: ApiToken } | { error: string }>; +} + +export interface ResolveFirstTokenResult { + /** `existingTokens` unchanged, or `[record]` when a mint just succeeded. */ + tokens: ApiToken[]; + /** The freshly minted token string, shown once, or `null`. */ + autoGeneratedToken: string | null; +} + +export async function resolveFirstToken({ + existingTokens, + autoGenerate, + mint, +}: ResolveFirstTokenArgs): Promise { + if (!autoGenerate || existingTokens.length > 0) { + return { tokens: existingTokens, autoGeneratedToken: null }; + } + + try { + const result = await mint(); + if ('token' in result) { + return { tokens: [result.record], autoGeneratedToken: result.token }; + } + // `{ error }` — already logged by `mintToken` itself. Degrade quietly. + return { tokens: existingTokens, autoGeneratedToken: null }; + } catch (error) { + // Next's own control-flow errors (redirect, not-found, the dynamic- + // rendering bailout) are not mint failures and must propagate. + unstable_rethrow(error); + // Any other thrown error is a genuine, unexpected mint failure. `withSpan` + // inside `mintToken` already logged it — degrade to no auto token so the + // page still renders rather than taking down the whole route. + return { tokens: existingTokens, autoGeneratedToken: null }; + } +} diff --git a/packages/web/src/lib/onboarding-steps.tsx b/packages/web/src/lib/onboarding-steps.tsx index 9629f46cd..fcb13068e 100644 --- a/packages/web/src/lib/onboarding-steps.tsx +++ b/packages/web/src/lib/onboarding-steps.tsx @@ -1,7 +1,9 @@ import { Bot } from 'lucide-react'; import type { OnboardingStep } from './onboarding'; import { OnboardingStepContent } from '@/components/dashboard/OnboardingStepContent'; -import { listTokens, generateToken } from '@/lib/tokens'; +import { listTokens } from '@/lib/tokens'; +import { mintToken } from '@/lib/token-mint'; +import { resolveFirstToken } from '@/lib/first-token'; import { resolveMcpUrls } from '@/lib/mcp-url'; import { getKeyScopeCatalog } from '@/lib/key-scope-catalog'; import { SETTINGS_LANDING_HREF } from '@/lib/settings-routes'; @@ -22,8 +24,14 @@ import { SETTINGS_LANDING_HREF } from '@/lib/settings-routes'; * * First login has no tokens: with `autoGenerateToken`, we mint a read+write * token so the user can copy a ready-to-use config with zero extra clicks. - * Wrapped in try/catch: if generation fails the page still renders, just - * without the auto-filled token. + * The mint runs through `mintToken` (the `server-only` core in + * `lib/token-mint.ts`), NOT `lib/tokens.ts`'s `'use server'` generate action — + * a Server Component render must never call that action, because its success + * path calls `revalidatePath`, which Next 15 forbids during render. This + * render needs no revalidation anyway: it returns the freshly minted record + * as `initialTokens` itself. `resolveFirstToken` (`lib/first-token.ts`) owns + * the decision and degrades to "no auto token" on failure so the page still + * renders. */ export async function buildOnboardingSteps( { autoGenerateToken = false }: { autoGenerateToken?: boolean } = {}, @@ -39,19 +47,11 @@ export async function buildOnboardingSteps( // same shape `settings/api-keys/page.tsx` already uses for the same pair. const [existingTokens, catalog] = await Promise.all([listTokens(), getKeyScopeCatalog()]); - let tokens = existingTokens; - let autoGeneratedToken: string | null = null; - if (autoGenerateToken && existingTokens.length === 0) { - try { - const result = await generateToken('My first token', ['read', 'write']); - if ('token' in result) { - autoGeneratedToken = result.token; - tokens = [result.record]; - } - } catch { - // Non-fatal — page renders without the auto-generated token. - } - } + const { tokens, autoGeneratedToken } = await resolveFirstToken({ + existingTokens, + autoGenerate: autoGenerateToken, + mint: () => mintToken('My first token', ['read', 'write'], undefined, 'onboarding'), + }); const { mcpUrl } = resolveMcpUrls(); diff --git a/packages/web/src/lib/token-mint.spec.ts b/packages/web/src/lib/token-mint.spec.ts new file mode 100644 index 000000000..f5ed30090 --- /dev/null +++ b/packages/web/src/lib/token-mint.spec.ts @@ -0,0 +1,143 @@ +/** + * Regression pin for the production bug this module exists to fix: + * `mintToken` must NEVER call `revalidatePath` — that call only belongs in + * `lib/tokens.ts`'s `generateToken` (pinned separately in `tokens.spec.ts`), + * because a Server Component render (the first-sign-in auto-mint on `/lore`) + * calls `mintToken` directly and Next 15 throws if `revalidatePath` runs + * during render. + * + * `server-only` is not resolvable outside Next's bundler, so it is mocked as + * a no-op virtual module — verified at plan time that vitest accepts this. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('server-only', () => ({})); + +const revalidatePath = vi.fn(); +vi.mock('next/cache', () => ({ revalidatePath: (...args: unknown[]) => revalidatePath(...args) })); + +const getVerifiedUser = vi.fn(); +vi.mock('@/lib/auth/verified-user', () => ({ getVerifiedUser: () => getVerifiedUser() })); + +const recordAuditEvent = vi.fn(); +vi.mock('@/lib/audit-log', () => ({ recordAuditEvent: (...args: unknown[]) => recordAuditEvent(...args) })); + +// ── Chainable Supabase fake ──────────────────────────────────────────────── +// `mintToken` only ever touches `api_tokens` through two shapes: +// .from('api_tokens').select('id', {count:'exact', head:true}).eq(...) +// .from('api_tokens').insert(...).select(...).single() +// plus, on the scoped/cleanup path, .rpc(...) and .delete().eq().eq(). Each +// is stubbed to resolve a scripted result so a test only has to say what it +// wants back, not reconstruct the real client. +interface SupabaseScript { + count?: { count: number | null; error: { message: string } | null }; + insert?: { data: Record | null; error: { message: string } | null }; + rpc?: { data: unknown; error: { message: string } | null }; + delete?: { error: { message: string } | null }; +} + +const insertCalls: unknown[] = []; + +function makeSupabase(script: SupabaseScript) { + return { + from: (_table: string) => ({ + select: (_cols: string) => ({ + eq: () => Promise.resolve(script.count ?? { count: 0, error: null }), + }), + insert: (row: unknown) => { + insertCalls.push(row); + return { + select: (_cols: string) => ({ + single: () => Promise.resolve(script.insert ?? { data: null, error: null }), + }), + }; + }, + delete: () => ({ + eq: () => ({ + eq: () => Promise.resolve(script.delete ?? { error: null }), + }), + }), + }), + rpc: () => Promise.resolve(script.rpc ?? { data: null, error: null }), + }; +} + +const createServerClient = vi.fn(); +vi.mock('@/lib/supabase/server', () => ({ createServerClient: () => createServerClient() })); + +const { mintToken } = await import('./token-mint'); + +const USER = { id: 'user-1' }; +const RECORD = { + id: 'tok-1', + name: 'My first token', + token_prefix: 'lk_rw_aBcD1...', + permissions: ['read', 'write'], + scopes: [], + org_access: 'all', + org_ids: [], + last_used_at: null, + created_at: '2026-01-01T00:00:00.000Z', +}; + +beforeEach(() => { + vi.clearAllMocks(); + insertCalls.length = 0; + getVerifiedUser.mockResolvedValue(USER); + recordAuditEvent.mockResolvedValue(undefined); +}); + +describe('mintToken', () => { + it('never calls revalidatePath on a successful mint, and inserts exactly once', async () => { + createServerClient.mockResolvedValue( + makeSupabase({ + count: { count: 0, error: null }, + insert: { data: RECORD, error: null }, + }), + ); + + const result = await mintToken('My first token', ['read', 'write']); + + expect('token' in result).toBe(true); + expect(revalidatePath).not.toHaveBeenCalled(); + expect(insertCalls).toHaveLength(1); + }); + + it('never calls revalidatePath when the insert fails', async () => { + createServerClient.mockResolvedValue( + makeSupabase({ + count: { count: 0, error: null }, + insert: { data: null, error: { message: 'insert boom' } }, + }), + ); + + const result = await mintToken('My first token', ['read', 'write']); + + expect(result).toEqual({ error: 'insert boom' }); + expect(revalidatePath).not.toHaveBeenCalled(); + }); + + it('never calls revalidatePath when the token cap is reached', async () => { + createServerClient.mockResolvedValue(makeSupabase({ count: { count: 20, error: null } })); + + const result = await mintToken('My first token', ['read', 'write']); + + expect('error' in result).toBe(true); + expect(revalidatePath).not.toHaveBeenCalled(); + expect(insertCalls).toHaveLength(0); + }); + + it('stamps the origin attribute without affecting the result shape', async () => { + createServerClient.mockResolvedValue( + makeSupabase({ + count: { count: 0, error: null }, + insert: { data: RECORD, error: null }, + }), + ); + + const result = await mintToken('My first token', ['read', 'write'], undefined, 'onboarding'); + + expect('token' in result).toBe(true); + expect(revalidatePath).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/lib/token-mint.ts b/packages/web/src/lib/token-mint.ts new file mode 100644 index 000000000..ba90d12fc --- /dev/null +++ b/packages/web/src/lib/token-mint.ts @@ -0,0 +1,297 @@ +import 'server-only'; + +/** + * The revalidate-free core of API-token minting. + * + * Extracted out of `lib/tokens.ts` (a Server Actions module) so a Server + * Component's render can mint a token without ever calling `revalidatePath` + * during render — Next 15 throws "used revalidatePath during render which is + * unsupported" for that, which is exactly the production bug this module + * exists to fix: `buildOnboardingSteps({ autoGenerateToken: true })` used to + * call the generate action in `lib/tokens.ts`, which inserted the + * `api_tokens` row and then called `revalidatePath('/lore')` — the throw was + * swallowed by a bare `catch {}` in `onboarding-steps.tsx`, leaving an orphan + * row and no token on screen. + * + * The `server-only` import (no Server Actions directive) is deliberate: it + * guards against a client bundle pulling this in, WITHOUT making `mintToken` + * a client-callable action endpoint. Same pattern as `lib/key-scope-catalog.ts`. + * + * `lib/tokens.ts`'s `generateToken` wraps `mintToken` and adds the + * `revalidatePath` calls — that action path is the one place in the app a + * mint is followed by a revalidate, because it runs from a form submission, + * not a render. + */ + +import { createServerClient } from '@/lib/supabase/server'; +import { getVerifiedUser } from '@/lib/auth/verified-user'; +import { permissionSuffix } from '@/lib/token-permission'; +import { isScoped, type OrgAccess, type TokenScoping } from '@/lib/token-scoping'; +import { recordAuditEvent } from '@/lib/audit-log'; +import { withSpan, logger, SpanStatusCode } from '@/lib/telemetry'; +import { ATTR_ERROR_TYPE } from '@opentelemetry/semantic-conventions'; + +export type TokenPermission = 'read' | 'write'; + +export interface ApiToken { + id: string; + name: string; + token_prefix: string; + permissions: TokenPermission[]; + /** Scope allowlist (migration 00068). EMPTY = unrestricted. */ + scopes: string[]; + org_access: OrgAccess; + /** Meaningful only under `org_access: 'selected'`. */ + org_ids: string[]; + last_used_at: string | null; + created_at: string; +} + +/** Which call site minted a token — bounded enum, safe as a span attribute. */ +export type TokenMintOrigin = 'onboarding' | 'dashboard'; + +/** + * The columns every read of `api_tokens` returns. Named once so a new column + * cannot land in one query and be missed by the other — which is how + * `listTokens` and `mintToken` would otherwise hand the UI two different + * shapes of the same row. + * + * `token_hash` is deliberately absent and must stay absent: it is the stored + * credential, and nothing in the dashboard has a use for it. + */ +export const TOKEN_COLUMNS = + 'id, name, token_prefix, permissions, scopes, org_access, org_ids, last_used_at, created_at'; + +/** Random alphanumeric string of given length. */ +function randomAlphanumeric(length: number): string { + const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + const bytes = new Uint8Array(length); + crypto.getRandomValues(bytes); + return Array.from(bytes, (b) => chars[b % chars.length]).join(''); +} + +/** SHA-256 hex of a string — matches the Deno implementation in the Edge Function. */ +async function sha256hex(text: string): Promise { + const buf = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(text)); + return Array.from(new Uint8Array(buf)).map((b) => b.toString(16).padStart(2, '0')).join(''); +} + +const MAX_TOKENS_PER_USER = 20; + +/** + * Mint a new API token. Returns the full token string ONCE — it is not + * stored in plain text and cannot be retrieved again. + * + * Does NOT revalidate any path — the caller decides whether that is + * appropriate (an action following a form submission does; a render does + * not, because the same render already returns the fresh record). + */ +export async function mintToken( + name: string, + permissions: TokenPermission[], + scoping: TokenScoping = { scopes: [], org_access: 'all', org_ids: [] }, + origin: TokenMintOrigin = 'dashboard', +): Promise<{ token: string; record: ApiToken } | { error: string }> { + return withSpan( + 'lorekit.api_token.generate', + { + // permissions is a bounded set — safe as a span attribute. + 'lorekit.api_token.permissions': permissions.join(','), + // Bounded too: the tenancy is an enum and the other two are COUNTS. The + // scope patterns themselves are repo and project names — unbounded + // cardinality and arguably sensitive — so they never become an attribute. + 'lorekit.api_token.org_access': scoping.org_access, + 'lorekit.api_token.scope_count': scoping.scopes.length, + 'lorekit.api_token.org_count': scoping.org_ids.length, + // Distinguishes the render-path auto-mint from the dashboard's explicit + // "Generate token" button in telemetry — lets prod prove this fix (first + // sign-in mints succeed, zero `.failed`) without guessing from auth tier. + 'lorekit.api_token.origin': origin, + }, + async (span) => { + const supabase = await createServerClient(); + const user = await getVerifiedUser(); + if (!user) return { error: 'Not authenticated' }; + if (!name.trim()) return { error: 'Token name is required' }; + + // Enforce per-user token cap + const { count, error: countError } = await supabase + .from('api_tokens') + .select('id', { count: 'exact', head: true }) + .eq('user_id', user.id); + if (countError) { + span.setAttribute(ATTR_ERROR_TYPE, 'SupabaseQueryError'); + span.setStatus({ code: SpanStatusCode.ERROR, message: `SupabaseQueryError: ${countError.message}` }); + logger.error('lorekit.api_token.generate.failed', { + 'exception.type': 'SupabaseQueryError', + 'exception.message': countError.message, + }); + return { error: countError.message }; + } + if ((count ?? 0) >= MAX_TOKENS_PER_USER) { + span.setAttribute('lorekit.api_token.limit_reached', true); + return { error: `Maximum ${MAX_TOKENS_PER_USER} tokens per user. Revoke an existing token first.` }; + } + + // Build token: lk_rw_<32>, lk_ro_<32>, or lk_wo_<32> + const permSuffix = permissionSuffix(permissions); + const random = randomAlphanumeric(32); + const fullToken = `lk_${permSuffix}_${random}`; + const prefix = fullToken.slice(0, 12) + '...'; // "lk_rw_aBcD1..." + const hash = await sha256hex(fullToken); + + const { data, error } = await supabase + .from('api_tokens') + .insert({ + user_id: user.id, + name: name.trim(), + token_prefix: prefix, + token_hash: hash, + permissions, + }) + .select(TOKEN_COLUMNS) + .single(); + + if (error) { + span.setAttribute(ATTR_ERROR_TYPE, 'SupabaseInsertError'); + span.setStatus({ code: SpanStatusCode.ERROR, message: `SupabaseInsertError: ${error.message}` }); + logger.error('lorekit.api_token.generate.failed', { + 'exception.type': 'SupabaseInsertError', + 'exception.message': error.message, + 'lorekit.api_token.permissions': permissions.join(','), + }); + return { error: error.message }; + } + + const record = data as ApiToken; + span.setAttribute('lorekit.api_token.id', record.id); + span.setAttribute('lorekit.api_token.prefix', record.token_prefix); + + // Scoping is applied through the RPC rather than written into the INSERT + // above, because the RPC is where the membership check lives: the insert + // policy only asserts `user_id = auth.uid()`, so a hand-written + // `org_ids` could name an org the user is not in. One gate, reused. + // + // A failure here DELETES the key just created. Returning it unscoped + // would hand back a working credential that is broader than the one the + // user asked for, and the shown-once banner would tell them it is + // narrowed — the single worst outcome available on this path. Better no + // key and a legible error. + if (isScoped(scoping)) { + const applied = await applyScoping(supabase, record.id, scoping); + if ('error' in applied) { + // The cleanup's own error is NOT discarded. If the DELETE fails the + // very outcome the comment above calls the worst available has + // happened anyway — a live UNSCOPED key exists while the caller is + // told the operation failed — so the caller is told which of the two + // situations they are in, and the prefix is named so they can revoke + // it. Swallowing it would leave the credential invisible. + const { error: cleanupError } = await supabase + .from('api_tokens') + .delete() + .eq('id', record.id) + .eq('user_id', user.id); + span.setAttribute(ATTR_ERROR_TYPE, 'ScopingRejected'); + span.setAttribute('lorekit.api_token.cleanup_failed', cleanupError != null); + span.setStatus({ code: SpanStatusCode.ERROR, message: `ScopingRejected: ${applied.error}` }); + logger.error('lorekit.api_token.generate.failed', { + 'exception.type': 'ScopingRejected', + 'exception.message': applied.error, + ...(cleanupError + ? { + 'lorekit.api_token.cleanup_error': cleanupError.message, + 'lorekit.api_token.prefix': record.token_prefix, + } + : {}), + }); + if (cleanupError) { + return { + error: + `${applied.error} An UNSCOPED token (${record.token_prefix}) could not be removed ` + + `afterwards (${cleanupError.message}) — revoke it below.`, + }; + } + return { error: applied.error }; + } + record.scopes = applied.scoping.scopes; + record.org_access = applied.scoping.org_access; + record.org_ids = applied.scoping.org_ids; + } + + // Audit metadata is limited to the name + prefix — NEVER the raw token or + // its hash, so the trail can never leak a usable credential. + await recordAuditEvent({ + action: 'api_key.create', + resourceType: 'api_token', + resourceId: record.id, + target: record.name, + metadata: { + name: record.name, + token_prefix: record.token_prefix, + // The scoping the key was BORN with, so the trail answers "what could + // this key reach when it was issued?" without joining to a later + // scope_change event. + scopes: record.scopes, + org_access: record.org_access, + org_ids: record.org_ids, + }, + }); + + return { token: fullToken, record }; + }, + ); +} + +/** + * Call `lorekit_api_token_set_scoping` and normalise its two failure shapes. + * + * The RPC raises with a `LKnnn:` prefix that is meaningful to the API and noise + * to a person reading a form, so the known codes are translated and anything + * else falls through verbatim rather than being flattened into a generic + * "something went wrong" — an unrecognised database error is exactly when the + * raw text is worth having. + * + * Exported so `lib/tokens.ts`'s `setTokenScoping` action can reuse the same + * translation rather than duplicating it. + */ +export async function applyScoping( + supabase: Awaited>, + tokenId: string, + scoping: TokenScoping, +): Promise<{ scoping: TokenScoping } | { error: string }> { + const { data, error } = await supabase.rpc('lorekit_api_token_set_scoping', { + p_token_id: tokenId, + p_scopes: scoping.scopes, + p_org_access: scoping.org_access, + p_org_ids: scoping.org_ids, + }); + + if (error) return { error: translateScopingError(error.message) }; + + // `RETURNS TABLE` resolves to an array. An empty one means the UPDATE matched + // nothing, which the RPC should have raised on — treat it as a failure rather + // than reporting a save that did not happen. + const row = (data as { scopes: string[]; org_access: OrgAccess; org_ids: string[] }[] | null)?.[0]; + if (!row) return { error: 'Could not update this key. It may have been revoked.' }; + + return { + scoping: { + scopes: row.scopes ?? [], + org_access: row.org_access ?? 'all', + org_ids: row.org_ids ?? [], + }, + }; +} + +/** Map the RPC's `LKnnn:` codes onto something worth showing under a form field. */ +function translateScopingError(message: string): string { + if (message.includes('LK003')) return 'Could not update this key. It may have been revoked.'; + if (message.includes('LK002')) return 'You are not a member of one of the selected organisations.'; + if (message.includes('LK004')) { + // The RPC's own text after the code is already written for a person — it + // names the offending rule ("at most 50 scope patterns per key") — so pass + // it through rather than replacing it with something vaguer. + return message.slice(message.indexOf('LK004:') + 'LK004:'.length).trim(); + } + return message; +} diff --git a/packages/web/src/lib/tokens.spec.ts b/packages/web/src/lib/tokens.spec.ts new file mode 100644 index 000000000..51147f272 --- /dev/null +++ b/packages/web/src/lib/tokens.spec.ts @@ -0,0 +1,73 @@ +/** + * `generateToken` (the `'use server'` action, triggered by the dashboard's + * "Generate token" button) is the one place in the app a mint is followed by + * a revalidate — it runs from a form submission, never a render. This pins + * that it revalidates BOTH `/lore` and the `/settings` layout on success, and + * revalidates NOTHING when the mint fails — the render-path mint + * (`token-mint.spec.ts`) must never revalidate at all. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const revalidatePath = vi.fn(); +vi.mock('next/cache', () => ({ revalidatePath: (...args: unknown[]) => revalidatePath(...args) })); + +const mintToken = vi.fn(); +vi.mock('@/lib/token-mint', () => ({ + mintToken: (...args: unknown[]) => mintToken(...args), + applyScoping: vi.fn(), + TOKEN_COLUMNS: 'id, name, token_prefix, permissions, scopes, org_access, org_ids, last_used_at, created_at', +})); + +const getVerifiedUser = vi.fn(); +vi.mock('@/lib/auth/verified-user', () => ({ getVerifiedUser: () => getVerifiedUser() })); + +const createServerClient = vi.fn(); +vi.mock('@/lib/supabase/server', () => ({ createServerClient: () => createServerClient() })); + +vi.mock('@/lib/audit-log', () => ({ recordAuditEvent: vi.fn() })); + +const { generateToken } = await import('./tokens'); + +const RECORD = { + id: 'tok-1', + name: 'My token', + token_prefix: 'lk_rw_aBcD1...', + permissions: ['read', 'write'], + scopes: [], + org_access: 'all', + org_ids: [], + last_used_at: null, + created_at: '2026-01-01T00:00:00.000Z', +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('generateToken', () => { + it('revalidates /lore and the /settings layout on success', async () => { + mintToken.mockResolvedValue({ token: 'lk_rw_xyz', record: RECORD }); + + const result = await generateToken('My token', ['read', 'write']); + + expect(result).toEqual({ token: 'lk_rw_xyz', record: RECORD }); + expect(mintToken).toHaveBeenCalledWith( + 'My token', + ['read', 'write'], + { scopes: [], org_access: 'all', org_ids: [] }, + 'dashboard', + ); + expect(revalidatePath).toHaveBeenCalledTimes(2); + expect(revalidatePath).toHaveBeenNthCalledWith(1, '/lore'); + expect(revalidatePath).toHaveBeenNthCalledWith(2, '/settings', 'layout'); + }); + + it('revalidates nothing when the mint returns an error', async () => { + mintToken.mockResolvedValue({ error: 'boom' }); + + const result = await generateToken('My token', ['read', 'write']); + + expect(result).toEqual({ error: 'boom' }); + expect(revalidatePath).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/lib/tokens.ts b/packages/web/src/lib/tokens.ts index c6353dda5..427d2b4cc 100644 --- a/packages/web/src/lib/tokens.ts +++ b/packages/web/src/lib/tokens.ts @@ -3,217 +3,46 @@ /** * Server actions for API token management. * All actions validate the user session before operating. + * + * Minting itself lives in `lib/token-mint.ts` (a `server-only`, non-action + * module) so a Server Component's render can mint a token without this + * file's `revalidatePath` calls running during render — see that module's + * header for the production bug this split fixes. `generateToken` below is + * the action path: mint, then revalidate on success only. */ import { createServerClient } from '@/lib/supabase/server'; import { getVerifiedUser } from '@/lib/auth/verified-user'; import { revalidatePath } from 'next/cache'; -import { permissionSuffix } from '@/lib/token-permission'; -import { isScoped, type OrgAccess, type TokenScoping } from '@/lib/token-scoping'; import { recordAuditEvent } from '@/lib/audit-log'; import { withSpan, logger, SpanStatusCode } from '@/lib/telemetry'; import { ATTR_ERROR_TYPE } from '@opentelemetry/semantic-conventions'; +import { mintToken, applyScoping, TOKEN_COLUMNS, type ApiToken, type TokenPermission } from '@/lib/token-mint'; +import type { TokenScoping } from '@/lib/token-scoping'; -export type TokenPermission = 'read' | 'write'; - -export interface ApiToken { - id: string; - name: string; - token_prefix: string; - permissions: TokenPermission[]; - /** Scope allowlist (migration 00068). EMPTY = unrestricted. */ - scopes: string[]; - org_access: OrgAccess; - /** Meaningful only under `org_access: 'selected'`. */ - org_ids: string[]; - last_used_at: string | null; - created_at: string; -} - -/** - * The columns every read of `api_tokens` returns. Named once so a new column - * cannot land in one query and be missed by the other — which is how - * `listTokens` and `generateToken` would otherwise hand the UI two different - * shapes of the same row. - * - * `token_hash` is deliberately absent and must stay absent: it is the stored - * credential, and nothing in the dashboard has a use for it. - */ -const TOKEN_COLUMNS = 'id, name, token_prefix, permissions, scopes, org_access, org_ids, last_used_at, created_at'; - -/** Random alphanumeric string of given length. */ -function randomAlphanumeric(length: number): string { - const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; - const bytes = new Uint8Array(length); - crypto.getRandomValues(bytes); - return Array.from(bytes, (b) => chars[b % chars.length]).join(''); -} - -/** SHA-256 hex of a string — matches the Deno implementation in the Edge Function. */ -async function sha256hex(text: string): Promise { - const buf = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(text)); - return Array.from(new Uint8Array(buf)).map((b) => b.toString(16).padStart(2, '0')).join(''); -} +export type { ApiToken, TokenPermission }; /** * Generate a new API token. Returns the full token string ONCE — it is not * stored in plain text and cannot be retrieved again. + * + * Thin wrapper over `mintToken` (origin `'dashboard'`): revalidates `/lore` + * and the `/settings` layout on success, and only on success — this action + * runs from a form submission (`TokenManager`), never from a render. */ -const MAX_TOKENS_PER_USER = 20; - export async function generateToken( name: string, permissions: TokenPermission[], scoping: TokenScoping = { scopes: [], org_access: 'all', org_ids: [] }, ): Promise<{ token: string; record: ApiToken } | { error: string }> { - return withSpan( - 'lorekit.api_token.generate', - { - // permissions is a bounded set — safe as a span attribute. - 'lorekit.api_token.permissions': permissions.join(','), - // Bounded too: the tenancy is an enum and the other two are COUNTS. The - // scope patterns themselves are repo and project names — unbounded - // cardinality and arguably sensitive — so they never become an attribute. - 'lorekit.api_token.org_access': scoping.org_access, - 'lorekit.api_token.scope_count': scoping.scopes.length, - 'lorekit.api_token.org_count': scoping.org_ids.length, - }, - async (span) => { - const supabase = await createServerClient(); - const user = await getVerifiedUser(); - if (!user) return { error: 'Not authenticated' }; - if (!name.trim()) return { error: 'Token name is required' }; - - // Enforce per-user token cap - const { count, error: countError } = await supabase - .from('api_tokens') - .select('id', { count: 'exact', head: true }) - .eq('user_id', user.id); - if (countError) { - span.setAttribute(ATTR_ERROR_TYPE, 'SupabaseQueryError'); - span.setStatus({ code: SpanStatusCode.ERROR, message: `SupabaseQueryError: ${countError.message}` }); - logger.error('lorekit.api_token.generate.failed', { - 'exception.type': 'SupabaseQueryError', - 'exception.message': countError.message, - }); - return { error: countError.message }; - } - if ((count ?? 0) >= MAX_TOKENS_PER_USER) { - span.setAttribute('lorekit.api_token.limit_reached', true); - return { error: `Maximum ${MAX_TOKENS_PER_USER} tokens per user. Revoke an existing token first.` }; - } - - // Build token: lk_rw_<32>, lk_ro_<32>, or lk_wo_<32> - const permSuffix = permissionSuffix(permissions); - const random = randomAlphanumeric(32); - const fullToken = `lk_${permSuffix}_${random}`; - const prefix = fullToken.slice(0, 12) + '...'; // "lk_rw_aBcD1..." - const hash = await sha256hex(fullToken); - - const { data, error } = await supabase - .from('api_tokens') - .insert({ - user_id: user.id, - name: name.trim(), - token_prefix: prefix, - token_hash: hash, - permissions, - }) - .select(TOKEN_COLUMNS) - .single(); - - if (error) { - span.setAttribute(ATTR_ERROR_TYPE, 'SupabaseInsertError'); - span.setStatus({ code: SpanStatusCode.ERROR, message: `SupabaseInsertError: ${error.message}` }); - logger.error('lorekit.api_token.generate.failed', { - 'exception.type': 'SupabaseInsertError', - 'exception.message': error.message, - 'lorekit.api_token.permissions': permissions.join(','), - }); - return { error: error.message }; - } - - const record = data as ApiToken; - span.setAttribute('lorekit.api_token.id', record.id); - span.setAttribute('lorekit.api_token.prefix', record.token_prefix); - - // Scoping is applied through the RPC rather than written into the INSERT - // above, because the RPC is where the membership check lives: the insert - // policy only asserts `user_id = auth.uid()`, so a hand-written - // `org_ids` could name an org the user is not in. One gate, reused. - // - // A failure here DELETES the key just created. Returning it unscoped - // would hand back a working credential that is broader than the one the - // user asked for, and the shown-once banner would tell them it is - // narrowed — the single worst outcome available on this path. Better no - // key and a legible error. - if (isScoped(scoping)) { - const applied = await applyScoping(supabase, record.id, scoping); - if ('error' in applied) { - // The cleanup's own error is NOT discarded. If the DELETE fails the - // very outcome the comment above calls the worst available has - // happened anyway — a live UNSCOPED key exists while the caller is - // told the operation failed — so the caller is told which of the two - // situations they are in, and the prefix is named so they can revoke - // it. Swallowing it would leave the credential invisible. - const { error: cleanupError } = await supabase - .from('api_tokens') - .delete() - .eq('id', record.id) - .eq('user_id', user.id); - span.setAttribute(ATTR_ERROR_TYPE, 'ScopingRejected'); - span.setAttribute('lorekit.api_token.cleanup_failed', cleanupError != null); - span.setStatus({ code: SpanStatusCode.ERROR, message: `ScopingRejected: ${applied.error}` }); - logger.error('lorekit.api_token.generate.failed', { - 'exception.type': 'ScopingRejected', - 'exception.message': applied.error, - ...(cleanupError - ? { - 'lorekit.api_token.cleanup_error': cleanupError.message, - 'lorekit.api_token.prefix': record.token_prefix, - } - : {}), - }); - if (cleanupError) { - return { - error: - `${applied.error} An UNSCOPED token (${record.token_prefix}) could not be removed ` + - `afterwards (${cleanupError.message}) — revoke it below.`, - }; - } - return { error: applied.error }; - } - record.scopes = applied.scoping.scopes; - record.org_access = applied.scoping.org_access; - record.org_ids = applied.scoping.org_ids; - } - - // Audit metadata is limited to the name + prefix — NEVER the raw token or - // its hash, so the trail can never leak a usable credential. - await recordAuditEvent({ - action: 'api_key.create', - resourceType: 'api_token', - resourceId: record.id, - target: record.name, - metadata: { - name: record.name, - token_prefix: record.token_prefix, - // The scoping the key was BORN with, so the trail answers "what could - // this key reach when it was issued?" without joining to a later - // scope_change event. - scopes: record.scopes, - org_access: record.org_access, - org_ids: record.org_ids, - }, - }); - - revalidatePath('/lore'); - // 'layout' so nested /settings/* pages (where tokens actually render) revalidate, - // not just the /settings redirect page. - revalidatePath('/settings', 'layout'); - return { token: fullToken, record }; - }, - ); + const result = await mintToken(name, permissions, scoping, 'dashboard'); + if ('token' in result) { + revalidatePath('/lore'); + // 'layout' so nested /settings/* pages (where tokens actually render) revalidate, + // not just the /settings redirect page. + revalidatePath('/settings', 'layout'); + } + return result; } /** List all tokens for the current user. Returns [] on auth failure or DB error. */ @@ -321,7 +150,8 @@ export async function revokeToken(tokenId: string): Promise<{ error?: string }> * Everything about WHO may do this lives in `lorekit_api_token_set_scoping` * (migration 00068): ownership, the org-membership check, and the cross-field * rule between `org_access` and `org_ids`. This action does not re-derive any - * of it — it calls the one gate and translates the refusal. + * of it — it calls the one gate (via `token-mint.ts`'s `applyScoping`) and + * translates the refusal. * * Clearing is the same call with an unrestricted argument; there is no separate * "unscope" action, for the same reason the audit vocabulary has one term. @@ -398,58 +228,3 @@ export async function setTokenScoping( }, ); } - -/** - * Call `lorekit_api_token_set_scoping` and normalise its two failure shapes. - * - * The RPC raises with a `LKnnn:` prefix that is meaningful to the API and noise - * to a person reading a form, so the known codes are translated and anything - * else falls through verbatim rather than being flattened into a generic - * "something went wrong" — an unrecognised database error is exactly when the - * raw text is worth having. - * - * Not exported: a `'use server'` module may export async functions only, and - * this one is an implementation detail of the two actions above rather than - * something a component should be able to reach past them to call. - */ -async function applyScoping( - supabase: Awaited>, - tokenId: string, - scoping: TokenScoping, -): Promise<{ scoping: TokenScoping } | { error: string }> { - const { data, error } = await supabase.rpc('lorekit_api_token_set_scoping', { - p_token_id: tokenId, - p_scopes: scoping.scopes, - p_org_access: scoping.org_access, - p_org_ids: scoping.org_ids, - }); - - if (error) return { error: translateScopingError(error.message) }; - - // `RETURNS TABLE` resolves to an array. An empty one means the UPDATE matched - // nothing, which the RPC should have raised on — treat it as a failure rather - // than reporting a save that did not happen. - const row = (data as { scopes: string[]; org_access: OrgAccess; org_ids: string[] }[] | null)?.[0]; - if (!row) return { error: 'Could not update this key. It may have been revoked.' }; - - return { - scoping: { - scopes: row.scopes ?? [], - org_access: row.org_access ?? 'all', - org_ids: row.org_ids ?? [], - }, - }; -} - -/** Map the RPC's `LKnnn:` codes onto something worth showing under a form field. */ -function translateScopingError(message: string): string { - if (message.includes('LK003')) return 'Could not update this key. It may have been revoked.'; - if (message.includes('LK002')) return 'You are not a member of one of the selected organisations.'; - if (message.includes('LK004')) { - // The RPC's own text after the code is already written for a person — it - // names the offending rule ("at most 50 scope patterns per key") — so pass - // it through rather than replacing it with something vaguer. - return message.slice(message.indexOf('LK004:') + 'LK004:'.length).trim(); - } - return message; -} From 7834620daecaaebec02b6bc1518d42243110f7d9 Mon Sep 17 00:00:00 2001 From: mthines Date: Sat, 17 Oct 2026 14:02:16 +0200 Subject: [PATCH 2/2] test(web): pin the onboarding render to mintToken and assert the origin span attribute The revalidatePath assertions in token-mint.spec could not fail (token-mint never imports next/cache). onboarding-steps.spec now fails if the render goes back to generateToken, and the origin test reads the span attribute. Also correct first-token's comment: expected refusals are not logged. --- packages/web/src/lib/first-token.ts | 4 +- packages/web/src/lib/onboarding-steps.spec.ts | 62 +++++++++++++++++++ packages/web/src/lib/token-mint.spec.ts | 20 +++++- 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 packages/web/src/lib/onboarding-steps.spec.ts diff --git a/packages/web/src/lib/first-token.ts b/packages/web/src/lib/first-token.ts index 5c8de55b8..de876bce9 100644 --- a/packages/web/src/lib/first-token.ts +++ b/packages/web/src/lib/first-token.ts @@ -49,7 +49,9 @@ export async function resolveFirstToken({ if ('token' in result) { return { tokens: [result.record], autoGeneratedToken: result.token }; } - // `{ error }` — already logged by `mintToken` itself. Degrade quietly. + // `{ error }` — a DB failure is already logged by `mintToken`; the + // expected refusals (not authenticated, token cap reached) are not + // logged anywhere. Either way, degrade to no auto token. return { tokens: existingTokens, autoGeneratedToken: null }; } catch (error) { // Next's own control-flow errors (redirect, not-found, the dynamic- diff --git a/packages/web/src/lib/onboarding-steps.spec.ts b/packages/web/src/lib/onboarding-steps.spec.ts new file mode 100644 index 000000000..271995a4b --- /dev/null +++ b/packages/web/src/lib/onboarding-steps.spec.ts @@ -0,0 +1,62 @@ +/** + * Pins the render-path binding the first-sign-in fix depends on: the `/lore` + * onboarding render mints through `mintToken(..., 'onboarding')`, never the + * `'use server'` `generateToken` action whose success path calls + * `revalidatePath` (which Next 15 forbids during render). Pointing the render + * back at `generateToken` — the original bug — fails this spec. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as React from 'react'; + +// The node-env vitest transform compiles JSX to `React.createElement`, so the +// module under test needs React in scope at call time. +vi.stubGlobal('React', React); + +vi.mock('server-only', () => ({})); + +const listTokens = vi.fn(); +const generateToken = vi.fn(); +vi.mock('@/lib/tokens', () => ({ + listTokens: () => listTokens(), + generateToken: (...args: unknown[]) => generateToken(...args), +})); + +const mintToken = vi.fn(); +vi.mock('@/lib/token-mint', () => ({ mintToken: (...args: unknown[]) => mintToken(...args) })); + +vi.mock('@/lib/key-scope-catalog', () => ({ + getKeyScopeCatalog: () => Promise.resolve({ scopes: [], orgs: [] }), +})); +vi.mock('@/components/dashboard/OnboardingStepContent', () => ({ + OnboardingStepContent: () => null, +})); + +const { buildOnboardingSteps } = await import('./onboarding-steps'); + +const RECORD = { id: 'tok-1', name: 'My first token' }; + +beforeEach(() => { + vi.clearAllMocks(); + mintToken.mockResolvedValue({ token: 'lk_rw_x', record: RECORD }); +}); + +describe('buildOnboardingSteps', () => { + it('auto-mints a first token through mintToken with the onboarding origin, never generateToken', async () => { + listTokens.mockResolvedValue([]); + + await buildOnboardingSteps({ autoGenerateToken: true }); + + expect(mintToken).toHaveBeenCalledTimes(1); + expect(mintToken).toHaveBeenCalledWith('My first token', ['read', 'write'], undefined, 'onboarding'); + expect(generateToken).not.toHaveBeenCalled(); + }); + + it('mints nothing when the user already has a token', async () => { + listTokens.mockResolvedValue([RECORD]); + + await buildOnboardingSteps({ autoGenerateToken: true }); + + expect(mintToken).not.toHaveBeenCalled(); + expect(generateToken).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/lib/token-mint.spec.ts b/packages/web/src/lib/token-mint.spec.ts index f5ed30090..8b4dee115 100644 --- a/packages/web/src/lib/token-mint.spec.ts +++ b/packages/web/src/lib/token-mint.spec.ts @@ -62,6 +62,20 @@ function makeSupabase(script: SupabaseScript) { }; } +// Capture every span's start attributes while still running the real +// `withSpan`, so a test can assert what `mintToken` stamps on its span. +const spanAttrs = vi.hoisted(() => [] as Record[]); +vi.mock('@/lib/telemetry', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + withSpan: ((name: string, attrs: Record, fn: never) => { + spanAttrs.push(attrs); + return (actual.withSpan as (...a: unknown[]) => unknown)(name, attrs, fn); + }) as typeof actual.withSpan, + }; +}); + const createServerClient = vi.fn(); vi.mock('@/lib/supabase/server', () => ({ createServerClient: () => createServerClient() })); @@ -83,6 +97,7 @@ const RECORD = { beforeEach(() => { vi.clearAllMocks(); insertCalls.length = 0; + spanAttrs.length = 0; getVerifiedUser.mockResolvedValue(USER); recordAuditEvent.mockResolvedValue(undefined); }); @@ -127,7 +142,7 @@ describe('mintToken', () => { expect(insertCalls).toHaveLength(0); }); - it('stamps the origin attribute without affecting the result shape', async () => { + it('stamps lorekit.api_token.origin on its span, defaulting to dashboard', async () => { createServerClient.mockResolvedValue( makeSupabase({ count: { count: 0, error: null }, @@ -136,8 +151,9 @@ describe('mintToken', () => { ); const result = await mintToken('My first token', ['read', 'write'], undefined, 'onboarding'); + await mintToken('Another', ['read', 'write']); expect('token' in result).toBe(true); - expect(revalidatePath).not.toHaveBeenCalled(); + expect(spanAttrs.map((a) => a['lorekit.api_token.origin'])).toEqual(['onboarding', 'dashboard']); }); });