fix(api): retire the placebo scopes and make token issue prove itself

apps/community/src/components/admin/api-token-forms.tsx

@@ -65,6 +65,10 @@ export function IssueTokenForm({ scopes }: { scopes: readonly string[] }) {
           placeholder="Leave empty for no expiry"
           className="w-56 rounded-sm border border-input bg-card px-2 py-1"
         />
+        <span className="text-xs text-muted-foreground">
+          A whole number of days. Leave it empty for a token that never expires — anything
+          else is refused rather than read as never.
+        </span>
       </label>
 
       <SubmitButton>Issue token</SubmitButton>

apps/community/src/server/api-token-actions.test.ts

@@ -0,0 +1,160 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const adminCalls: Array<{ action: string; detail: unknown }> = []
+const requireAdminMock = vi.fn(async () => ({ userId: 1 }))
+const requireFreshAdminMock = vi.fn(async () => ({ userId: 1 }))
+const revalidated: string[] = []
+
+vi.mock('next/cache', () => ({
+  revalidatePath: (path: string) => {
+    revalidated.push(path)
+  },
+}))
+
+vi.mock('./admin', () => ({
+  requireAdmin: () => requireAdminMock(),
+  requireFreshAdmin: () => requireFreshAdminMock(),
+  recordAdminAction: async (input: { action: string; detail?: unknown }) => {
+    adminCalls.push({ action: input.action, detail: input.detail })
+  },
+}))
+
+const issued: Array<{ name: string; scopes: readonly string[]; expiresAt: Date | null }> = []
+const revocations: number[] = []
+
+vi.mock('./api-tokens-admin', () => ({
+  issueApiToken: async (input: {
+    name: string
+    scopes: readonly string[]
+    expiresAt: Date | null
+  }) => {
+    issued.push({ name: input.name, scopes: input.scopes, expiresAt: input.expiresAt })
+    return 'forum_pat_abcd1234_secretsecretsecretsecret'
+  },
+  apiTokenStore: () => ({
+    async revoke(id: number) {
+      revocations.push(id)
+      return true
+    },
+  }),
+}))
+
+const { issueApiTokenAction, revokeApiTokenAction } = await import('./api-token-actions')
+
+function form(fields: Record<string, string | string[]>): FormData {
+  const data = new FormData()
+  for (const [key, value] of Object.entries(fields)) {
+    for (const one of Array.isArray(value) ? value : [value]) data.append(key, one)
+  }
+  return data
+}
+
+const NAMED = { name: 'ci', scopes: 'forums:read' }
+
+beforeEach(() => {
+  adminCalls.length = 0
+  revalidated.length = 0
+  issued.length = 0
+  revocations.length = 0
+  requireAdminMock.mockClear()
+  requireAdminMock.mockResolvedValue({ userId: 1 })
+  requireFreshAdminMock.mockClear()
+  requireFreshAdminMock.mockResolvedValue({ userId: 1 })
+})
+
+describe('the admin gate', () => {
+  it('asks for a fresh password before minting a credential', async () => {
+    await issueApiTokenAction({}, form(NAMED))
+
+    expect(requireFreshAdminMock).toHaveBeenCalledTimes(1)
+    expect(requireAdminMock).not.toHaveBeenCalled()
+  })
+
+  it('mints nothing when the proof is stale', async () => {
+    requireFreshAdminMock.mockRejectedValue(
+      Object.assign(new Error('confirm'), {
+        code: 'FORBIDDEN',
+        publicMessage: 'Confirm your password again before doing this.',
+      }),
+    )
+
+    const state = await issueApiTokenAction({}, form(NAMED))
+
+    expect(state.error).toBeDefined()
+    expect(state.values?.token).toBeUndefined()
+    expect(issued).toEqual([])
+    expect(adminCalls).toEqual([])
+  })
+
+  it('lets a revocation through on the panel session alone', async () => {
+    const state = await revokeApiTokenAction({}, form({ tokenId: '4' }))
+
+    expect(state.notice).toBe('revoked')
+    expect(requireAdminMock).toHaveBeenCalledTimes(1)
+    expect(requireFreshAdminMock).not.toHaveBeenCalled()
+    expect(revocations).toEqual([4])
+  })
+})
+
+describe('the expiry field', () => {
+  it('reads a whole number of days as a deadline', async () => {
+    const before = Date.now()
+    await issueApiTokenAction({}, form({ ...NAMED, expiresInDays: '30' }))
+    const after = Date.now()
+
+    const expiresAt = issued[0]!.expiresAt!
+    expect(expiresAt.getTime()).toBeGreaterThanOrEqual(before + 30 * 86_400_000)
+    expect(expiresAt.getTime()).toBeLessThanOrEqual(after + 30 * 86_400_000)
+  })
+
+  it('reads an empty field as the no-expiry the label promises', async () => {
+    const state = await issueApiTokenAction({}, form({ ...NAMED, expiresInDays: '   ' }))
+
+    expect(state.notice).toBe('issued')
+    expect(issued).toEqual([
+      { name: 'ci', scopes: ['forums:read'], expiresAt: null },
+    ])
+  })
+
+  it('reads a field that was never submitted as no expiry too', async () => {
+    const state = await issueApiTokenAction({}, form(NAMED))
+
+    expect(state.notice).toBe('issued')
+    expect(issued[0]!.expiresAt).toBeNull()
+  })
+
+  it.each(['30.5', 'abc', '1e2', '-7', '0', '9007199254740993', '30 days'])(
+    'refuses %o rather than reading it as never expires',
+    async (value) => {
+      const state = await issueApiTokenAction({}, form({ ...NAMED, expiresInDays: value }))
+
+      expect(state.error).toMatch(/whole number of days/)
+      expect(state.values?.token).toBeUndefined()
+      expect(issued).toEqual([])
+      expect(adminCalls).toEqual([])
+      expect(revalidated).toEqual([])
+    },
+  )
+})
+
+describe('issuing', () => {
+  it('shows the credential once and logs the issue', async () => {
+    const state = await issueApiTokenAction({}, form({ name: 'ci', scopes: 'forums:read' }))
+
+    expect(state.notice).toBe('issued')
+    expect(state.values?.token).toMatch(/^forum_pat_/)
+    expect(adminCalls).toEqual([
+      { action: 'system.api_token_issued', detail: { name: 'ci' } },
+    ])
+    expect(revalidated).toEqual(['/admin/api-tokens'])
+  })
+
+  it('passes every ticked scope through to the issuer', async () => {
+    await issueApiTokenAction(
+      {},
+      form({ name: 'ci', scopes: ['forums:read', 'posts:write'] }),
+    )
+
+    expect(issued[0]!.scopes).toEqual(['forums:read', 'posts:write'])
+  })
+})

apps/community/src/server/api-token-actions.ts

@@ -1,9 +1,9 @@
 'use server'
 
-import { isAppError, logger } from '@meith/core'
+import { ValidationError, isAppError, logger } from '@meith/core'
 import { revalidatePath } from 'next/cache'
 
-import { requireAdmin } from './admin'
+import { requireAdmin, requireFreshAdmin } from './admin'
 import { apiTokenStore, issueApiToken } from './api-tokens-admin'
 import { recordAdminAction } from './admin'
 import type { FormState } from './auth-form-state'
@@ -13,6 +13,20 @@ function field(form: FormData, name: string): string {
   return typeof value === 'string' ? value.trim() : ''
 }
 
+function expiryFrom(raw: string, now: number): Date | null {
+  if (raw === '') return null
+
+  const days = /^\d+$/.test(raw) ? Number(raw) : Number.NaN
+  if (!Number.isSafeInteger(days) || days <= 0) {
+    throw new ValidationError(
+      'Expires in (days) must be a whole number of days above zero. ' +
+        'Leave it empty for a token that never expires.',
+    )
+  }
+
+  return new Date(now + days * 86_400_000)
+}
+
 function refreshTokenList(): void {
   revalidatePath('/admin/api-tokens')
 }
@@ -28,13 +42,9 @@ export async function issueApiTokenAction(
   form: FormData,
 ): Promise<FormState> {
   try {
-    const admin = await requireAdmin()
+    const admin = await requireFreshAdmin()
 
-    const days = Number(field(form, 'expiresInDays'))
-    const expiresAt =
-      Number.isSafeInteger(days) && days > 0
-        ? new Date(Date.now() + days * 86_400_000)
-        : null
+    const expiresAt = expiryFrom(field(form, 'expiresInDays'), Date.now())
 
     const token = await issueApiToken({
       userId: admin.userId,

docs/rest-api.md

@@ -8,7 +8,7 @@
   `pnpm api:docs:check` and fail when this file and the code disagree.
 -->
 
-7 endpoints, 8 scopes. Base path: `/api/v1`.
+7 endpoints, 6 scopes. Base path: `/api/v1`.
 
 ## Authentication
 
@@ -32,16 +32,35 @@ telling a caller "expired" confirms the token was real.
 
 - `forums:read`
 - `threads:read`
-- `threads:write`
 - `posts:read`
 - `posts:write`
 - `members:read`
 - `search:read`
-- `admin:read`
 
-There is deliberately no `admin:write`. A token is a long-lived string in
-somebody’s CI configuration; reconfiguring a board should need a person at a
-keyboard with the admin panel’s re-authentication in front of them.
+Every scope on that list is required by at least one endpoint below, and a test
+holds it that way: a scope no route consumes is a checkbox that grants nothing,
+which reads as a permission and is not one.
+
+There is deliberately no administrative scope at all. A token is a long-lived
+string in somebody’s CI configuration; reconfiguring a board should need a person
+at a keyboard with the admin panel’s re-authentication in front of them.
+
+A token stored before a scope was retired keeps working. The scope is dropped as
+the token is read, so it simply no longer carries it — the endpoints it still has
+a scope for answer as before, and the rest answer `missing_scope`.
+
+## Issuing a token
+
+Tokens are issued from **API tokens** in the control panel. Issuing one is treated
+as a destructive operation: it asks for the administrator’s password again, on the
+same clock as banning a member or moving a forum, because a bearer string that
+leaves the building is at least as consequential. Revoking one does
+not ask — a revocation is the thing you want to be quick during an incident, and
+it is undone by issuing a new token rather than by recovering the old one.
+
+**Expires in (days)** takes a whole number of days, or nothing at all for a token
+that never expires. Anything else — a fraction, a word, a number in exponent
+notation — is refused and mints nothing, rather than being read as "never".
 
 ## Rate limits
 

packages/api/src/api.test.ts

@@ -162,6 +162,8 @@ describe('scopes', () => {
   it('recognises exactly the declared set', () => {
     expect(isScope('posts:write')).toBe(true)
     expect(isScope('posts:delete')).toBe(false)
+    expect(isScope('threads:write')).toBe(false)
+    expect(isScope('admin:read')).toBe(false)
   })
 
   it('answers only for the scopes a token carries', () => {
@@ -170,8 +172,13 @@ describe('scopes', () => {
     expect(hasScope(token, 'posts:write')).toBe(false)
   })
 
-  it('offers no administrative write scope', () => {
-    expect(SCOPES.filter((scope) => scope.startsWith('admin:'))).toEqual(['admin:read'])
+  it('offers no scope that no route consumes', () => {
+    const consumed = new Set<string>(ROUTES.map((route) => route.scope))
+    expect(SCOPES.filter((scope) => !consumed.has(scope))).toEqual([])
+  })
+
+  it('offers no administrative scope', () => {
+    expect(SCOPES.filter((scope) => scope.startsWith('admin:'))).toEqual([])
   })
 })
 

packages/api/src/tokens.ts

@@ -8,12 +8,10 @@ const SECRET_BYTES = 32
 export const SCOPES = [
   'forums:read',
   'threads:read',
-  'threads:write',
   'posts:read',
   'posts:write',
   'members:read',
   'search:read',
-  'admin:read',
 ] as const
 
 export type Scope = (typeof SCOPES)[number]

packages/db/src/client.pg.test.ts

@@ -103,7 +103,7 @@ describeIfPg('against real Postgres', () => {
     expect(Object.keys(rows[0]!)).toEqual(['now', 'due_before', 'locked_until'])
   })
 
-  describe('timestamps a repository returns from a raw execute', () => {
+  describe('what the API-token repository returns from a raw execute', () => {
     let repo: PostgresApiTokenRepository
 
     beforeAll(async () => {
@@ -138,5 +138,22 @@ describeIfPg('against real Postgres', () => {
 
       expect(token!.revokedAt).toBeNull()
     })
+
+    it('drops a scope the board no longer declares instead of refusing the token', async () => {
+      await harness.db.execute(sql`
+        insert into api_tokens (user_id, name, lookup, secret_hash, scopes, created_at)
+        values (1, 'issued before a scope was retired', 'deadbeef', 'x',
+                '["forums:read", "threads:write", "admin:read"]'::jsonb,
+                now() - interval '1 hour')
+      `)
+
+      const token = await repo.findByLookup('deadbeef')
+
+      expect(token).not.toBeNull()
+      expect(token!.scopes).toEqual(['forums:read'])
+
+      const listed = (await repo.listAll()).find((row) => row.lookup === 'deadbeef')
+      expect(listed!.scopes).toEqual(['forums:read'])
+    })
   })
 })

scripts/api-docs.mjs

@@ -104,9 +104,30 @@ function render({ routes, scopes }) {
     '',
     ...scopes.map((scope) => `- \`${scope}\``),
     '',
-    'There is deliberately no `admin:write`. A token is a long-lived string in',
-    'somebody’s CI configuration; reconfiguring a board should need a person at a',
-    'keyboard with the admin panel’s re-authentication in front of them.',
+    'Every scope on that list is required by at least one endpoint below, and a test',
+    'holds it that way: a scope no route consumes is a checkbox that grants nothing,',
+    'which reads as a permission and is not one.',
+    '',
+    'There is deliberately no administrative scope at all. A token is a long-lived',
+    'string in somebody’s CI configuration; reconfiguring a board should need a person',
+    'at a keyboard with the admin panel’s re-authentication in front of them.',
+    '',
+    'A token stored before a scope was retired keeps working. The scope is dropped as',
+    'the token is read, so it simply no longer carries it — the endpoints it still has',
+    'a scope for answer as before, and the rest answer `missing_scope`.',
+    '',
+    '## Issuing a token',
+    '',
+    'Tokens are issued from **API tokens** in the control panel. Issuing one is treated',
+    'as a destructive operation: it asks for the administrator’s password again, on the',
+    'same clock as banning a member or moving a forum, because a bearer string that',
+    'leaves the building is at least as consequential. Revoking one does',
+    'not ask — a revocation is the thing you want to be quick during an incident, and',
+    'it is undone by issuing a new token rather than by recovering the old one.',
+    '',
+    '**Expires in (days)** takes a whole number of days, or nothing at all for a token',
+    'that never expires. Anything else — a fraction, a word, a number in exponent',
+    'notation — is refused and mints nothing, rather than being read as "never".',
     '',
     '## Rate limits',
     '',