diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 80eccd89..afddce9f 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -381,3 +381,32 @@ {"id":"ePDS-yzs","title":"Redesign OTP step buttons: primary full-width, secondary as text links","description":"Replace the OTP step btn-row (3 buttons) with otp-actions layout: full-width primary button + two text link-btn elements below. Also make the email step button full-width. Remove unused .btn-row, .btn-spacer, .btn-secondary CSS. Add .otp-actions, .btn-block, .otp-links, .otp-links-dot, .link-btn CSS. Files: packages/auth-service/src/routes/login-page.ts only.","status":"closed","priority":2,"issue_type":"task","created_at":"2026-03-03T18:13:41.493927086+06:00","updated_at":"2026-03-03T18:13:50.32561971+06:00","closed_at":"2026-03-03T18:13:50.32561971+06:00","close_reason":"c6a4587 redesign OTP step buttons: full-width primary + text link secondaries","created_by":"karma.gainforest.id"} {"id":"ePDS-zdr","title":"Fix: missing test coverage for ePDS-wh1.3/wh1.4/wh1.6 new behaviors (from ePDS-wh1)","description":"Review of ePDS-wh1 tasks found three new behaviors with no test coverage:\n\n1. ePDS-wh1.4 (verifyCallback hex validation): The existing test 'rejects wrong-length sig' passes 'tooshort' but there is no test for a 64-character string containing non-hex characters (e.g. 64 x 'Z'). This is the exact attack vector the fix was designed to prevent — a malformed sig that passes the old length check but causes Buffer.from truncation. A test like: expect(verifyCallback(params, ts, 'Z'.repeat(64), secret)).toBe(false) should be added to packages/shared/src/__tests__/crypto.test.ts.\n\n2. ePDS-wh1.3 (check-handle 503): No test covers the new catch branch returning 503 { error: 'handle_check_failed' }. The choose-handle.test.ts has no test for when the PDS check-handle endpoint returns 503 and how the auth-service handles it.\n\n3. ePDS-wh1.6 (ping non-OK response): No test covers the new pingRes.ok check in the choose-handle GET handler. The choose-handle.test.ts has no test for when ping-request returns a non-2xx status (e.g. 404 request_expired) and the handler should return 400 with 'Session expired'.\n\nEvidence: grep -n 'non-hex|uppercase|ABCDEF|handle_check_failed|503|ping|pingRes|request_expired|session expired' packages/shared/src/__tests__/crypto.test.ts packages/auth-service/src/__tests__/choose-handle.test.ts returns no matches.","status":"open","priority":3,"issue_type":"bug","created_at":"2026-03-12T14:20:37.207718023+06:00","updated_at":"2026-03-12T14:20:37.207718023+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.4","type":"discovered-from","created_at":"2026-03-12T14:20:41.319967668+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.3","type":"discovered-from","created_at":"2026-03-12T14:20:41.381893767+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.6","type":"discovered-from","created_at":"2026-03-12T14:20:41.441546677+06:00","created_by":"karma.gainforest.id"}]} {"id":"ePDS-zgl","title":"Fix: better-auth.ts re-parses OTP_LENGTH independently instead of using ctx.config.otpLength (from ePDS-ipm)","description":"Review of ePDS-ipm found: createBetterAuth() and runBetterAuthMigrations() both re-read process.env.OTP_LENGTH directly instead of accepting the already-validated otpLength from AuthServiceConfig. This creates a seam where the two can diverge.\n\nEvidence:\n- index.ts main() validates config.otpLength (range 4-12) and stores it in AuthServiceConfig\n- better-auth.ts createBetterAuth() re-reads process.env.OTP_LENGTH independently (line 149)\n- better-auth.ts runBetterAuthMigrations() re-reads process.env.OTP_LENGTH independently (line 84)\n- createBetterAuth() is called from createAuthService() which receives a fully-built AuthServiceConfig, but otpLength is never passed through\n\nDivergence scenarios:\n1. If OTP_LENGTH='' (empty string): index.ts uses || so defaults to '8'; better-auth.ts uses ?? so passes '' to parseInt → NaN → better-auth gets NaN otpLength (validation in main() doesn't protect this path since createAuthService can be called without main())\n2. If createAuthService is called programmatically with a config.otpLength that differs from process.env.OTP_LENGTH, the UI and better-auth will disagree on code length\n\nFix: Add otpLength parameter to createBetterAuth() and runBetterAuthMigrations(), pass ctx.config.otpLength from index.ts. This makes the config the single source of truth.","status":"closed","priority":2,"issue_type":"bug","created_at":"2026-03-11T23:16:49.135849984+06:00","updated_at":"2026-03-11T23:36:39.900982214+06:00","closed_at":"2026-03-11T23:36:39.900982214+06:00","close_reason":"1b73865 fix: pass otpLength param to createBetterAuth and runBetterAuthMigrations","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-zgl","depends_on_id":"ePDS-ipm","type":"discovered-from","created_at":"2026-03-11T23:16:49.248905084+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zgl","depends_on_id":"ePDS-ipm","type":"parent-child","created_at":"2026-03-11T23:33:29.694197249+06:00","created_by":"karma.gainforest.id"}]} +{"id":"atproto-6v2","title":"Set up GitHub Actions CI (prettier, eslint, tests)","description":"Add a GitHub Actions workflow that runs on PRs and pushes to main: 1) prettier --check, 2) eslint, 3) pnpm typecheck, 4) pnpm test. Should use pnpm caching for speed. Depends on Prettier and ESLint being set up first.","status":"closed","priority":1,"issue_type":"task","created_at":"2026-02-21T17:15:59Z","updated_at":"2026-02-21T17:37:42Z","closed_at":"2026-02-21T17:37:42Z","close_reason":"CI workflow created: format:check, lint, typecheck, test. All 4 steps verified passing locally.","created_by":"Adam Spiers","dependencies":[{"issue_id":"atproto-6v2","depends_on_id":"atproto-nfm","type":"blocks","created_at":"2026-02-21T17:16:08Z","created_by":"Adam Spiers"},{"issue_id":"atproto-6v2","depends_on_id":"atproto-35v","type":"blocks","created_at":"2026-02-21T17:16:08Z","created_by":"Adam Spiers"}]} +{"id":"atproto-961","title":"Final review, manual test, and push all MAGIC_ → EPDS_ renames","description":"After all rename beads are committed locally:\n\n1. Review the full diff from before the rename series: git diff --no-ext-diff origin/main..HEAD\n2. Run pnpm typecheck \u0026\u0026 pnpm test one final time\n3. Manual smoke test: pnpm dev and verify the login flow still works (if environment available)\n4. Push to origin/main\n5. Export beads and commit","status":"closed","priority":2,"issue_type":"task","created_at":"2026-02-24T10:18:07Z","updated_at":"2026-03-04T23:02:34.573390497+06:00","closed_at":"2026-02-24T12:55:19Z","created_by":"Adam Spiers","dependencies":[{"issue_id":"atproto-961","depends_on_id":"atproto-d9j","type":"blocks","created_at":"2026-02-24T10:18:18Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-lf7","type":"blocks","created_at":"2026-02-24T10:18:18Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-3xz","type":"blocks","created_at":"2026-02-24T10:18:16Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-pp6","type":"blocks","created_at":"2026-02-24T10:18:16Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-10p","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-3x0","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"},{"issue_id":"atproto-961","depends_on_id":"atproto-di4","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"}]} +{"id":"atproto-9fa.5","title":"Add CSS injection middleware to pds-core","description":"Middleware wraps res.setHeader and res.end on GET /oauth/authorize for trusted clients. Computes SHA256 hash, appends to CSP style-src, injects style before head close.","status":"closed","priority":2,"issue_type":"task","created_at":"2026-03-03T21:51:17Z","updated_at":"2026-03-03T21:51:24Z","closed_at":"2026-03-03T21:51:24Z","close_reason":"Closed","created_by":"Adam Spiers","dependencies":[{"issue_id":"atproto-9fa.5","depends_on_id":"atproto-9fa.1","type":"blocks","created_at":"2026-03-03T23:41:23Z","created_by":"Adam Spiers"},{"issue_id":"atproto-9fa.5","depends_on_id":"atproto-9fa","type":"parent-child","created_at":"2026-03-03T21:51:16Z","created_by":"Adam Spiers"}]} +{"id":"atproto-ab0","title":"Wire email sending into better-auth sendVerificationOTP callback","description":"## Context\n\nPart of Phase 2 of the better-auth migration plan. See `better-auth-migration-plan.md`.\n\n## Problem\n\nbetter-auth's emailOTP plugin needs a `sendVerificationOTP` callback to actually deliver OTP codes. The existing `EmailSender` class in `packages/auth-service/src/email/sender.ts` supports SMTP, SendGrid, AWS SES, and Postmark, plus custom client-branded templates. This infrastructure should be reused.\n\n## Solution\n\nWire the existing `EmailSender.sendOtpCode()` into the `sendVerificationOTP` callback in `packages/auth-service/src/better-auth.ts`.\n\n### Client branding challenge\n\nThe current template system needs a `client_id` to resolve branding (logo, colors, email template). During a better-auth emailOTP flow, the `sendVerificationOTP` callback does not have access to the OAuth client context.\n\nSolution: read the `magic_auth_flow` cookie from the request context to recover the `flow_id`, look up the `auth_flow` row to get the `client_id`, and pass it to `emailSender.sendOtpCode()`.\n\nIf the request does not have a `magic_auth_flow` cookie (e.g., account settings login), use the default PDS email template.\n\n## Files to modify\n\n- `packages/auth-service/src/better-auth.ts` — wire sendVerificationOTP callback\n\n## Acceptance criteria\n\n- [ ] OTP emails delivered via existing EmailSender infrastructure\n- [ ] Client branding applied when client_id available from auth_flow\n- [ ] Default template used when no client_id context\n- [ ] All four email providers (SMTP, SendGrid, SES, Postmark) still work\n- [ ] Dev mode (no provider configured) still logs to console","status":"closed","priority":2,"issue_type":"task","created_at":"2026-02-20T02:17:11Z","updated_at":"2026-02-20T12:56:45Z","closed_at":"2026-02-20T12:56:45Z","close_reason":"Wired existing EmailSender into sendVerificationOTP callback with client branding support. createBetterAuth() now accepts MagicPdsDb; callback reads magic_auth_flow cookie via ctx.getCookie(), looks up auth_flow to get client_id, passes to emailSender.sendOtpCode(). Falls back to default template when no flow context. 6 tests added. 81 tests pass.","created_by":"Adam Spiers","dependencies":[{"issue_id":"atproto-ab0","depends_on_id":"atproto-cr3","type":"blocks","created_at":"2026-02-20T02:18:44Z","created_by":"Adam Spiers"},{"issue_id":"atproto-ab0","depends_on_id":"atproto-bvg","type":"blocks","created_at":"2026-02-20T02:18:43Z","created_by":"Adam Spiers"}]} +{"id":"atproto-mej","title":"Epic: Resolve CodeRabbit review for PR #8","description":"Resolve open CodeRabbit inline review comments on PR #8. 1 actionable comment (security fix for domain boundary matching in /tls-check). 1 comment skipped (Caddyfile ask URL — env var default is intentional).","status":"open","priority":1,"issue_type":"epic","created_at":"2026-03-02T18:00:25.008723779+06:00","updated_at":"2026-03-02T18:00:25.008723779+06:00","created_by":"karma.gainforest.id"} +{"id":"atproto-mej.1","title":"Tighten domain boundary matching in /tls-check to prevent sibling domain spoofing","description":"## Files\n- packages/pds-core/src/index.ts (modify)\n\n## What to do\nIn the `checkHandleRoute()` function (~line 478), the current domain matching uses `domain.endsWith(avail)` which allows sibling domains (e.g. `evil-example.com` matches `example.com`). Fix this by:\n\n1. Normalize the incoming `domain` query param before any checks:\n ```ts\n const normalizedDomain = domain.trim().toLowerCase().replace(/\\.$/, '')\n ```\n Place this right after the `typeof domain !== 'string'` guard.\n\n2. Replace the `find` + `endsWith` check with `some` + proper boundary matching:\n ```ts\n const isHostedHandle = pds.ctx.cfg.identity.serviceHandleDomains.some(\n (avail: string) =\u003e {\n const normalizedAvail = avail.toLowerCase()\n return (\n normalizedDomain === normalizedAvail ||\n normalizedDomain.endsWith(`.${normalizedAvail}`)\n )\n },\n )\n ```\n\n3. Use `normalizedDomain` (instead of raw `domain`) in the `getAccount()` call below:\n ```ts\n const account = await pds.ctx.accountManager.getAccount(normalizedDomain)\n ```\n\n4. Also use `normalizedDomain` in the hostname/auth checks above:\n ```ts\n if (normalizedDomain === pds.ctx.cfg.service.hostname || normalizedDomain === authHostname) {\n ```\n\n## Don't\n- Change any other function or route\n- Add new dependencies\n- Modify the Caddyfile\n- Change the response shapes or status codes","status":"open","priority":1,"issue_type":"task","created_at":"2026-03-02T18:00:43.860641244+06:00","updated_at":"2026-03-02T18:00:43.860641244+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"atproto-mej.1","depends_on_id":"atproto-mej","type":"parent-child","created_at":"2026-03-02T18:00:43.862742976+06:00","created_by":"karma.gainforest.id"}]} +{"id":"atproto-sbw","title":"Build unified login page with multiple auth methods","description":"## Context\n\nPhase 2 of the better-auth migration plan. See `better-auth-migration-plan.md` Phase 2.4.\n\n## Problem\n\nThe current auth-service login flow is hardcoded for email OTP only (authorize.ts → send-code.ts → verify-code.ts). It needs to become a landing page offering multiple login methods.\n\n## Solution\n\nReplace the existing `GET /oauth/authorize` on the auth-service with a unified login page that:\n\n1. Receives `?request_uri=...\u0026client_id=...\u0026prompt=...\u0026login_hint=...` from the pds-core AS metadata redirect\n2. Creates an `auth_flow` row (flow_id, request_uri, client_id)\n3. Sets `magic_auth_flow` cookie (10 min, httpOnly)\n4. Resolves client metadata for branding via existing `lib/client-metadata.ts` (logo, colors, name)\n5. Renders a login page with:\n - Email OTP form (calls better-auth `/api/auth/*` endpoints on auth-service)\n - Social login buttons (only for providers enabled via env vars — check exported `socialProviders` object from `better-auth.ts`)\n - \"Recover with backup email\" link (to existing recovery flow)\n\n### Social provider buttons\n\nThe login page template conditionally renders buttons based on `socialProviders`:\n```typescript\nif (\"google\" in socialProviders) { /* render Google button */ }\nif (\"github\" in socialProviders) { /* render GitHub button */ }\n```\n\nSocial login redirects to better-auth `/api/auth/sign-in/{provider}`, which handles the full OAuth exchange and redirects back to `/auth/complete` (the bridge route).\n\n### Delete on completion\n\nAfter this is working, delete the old files:\n- `routes/authorize.ts`, `routes/send-code.ts`, `routes/verify-code.ts`\n- `magic-link/token.ts`, `magic-link/rate-limit.ts`\n- `middleware/rate-limit.ts`, `middleware/csrf.ts`\n- Drop `magic_link_token` table from auth-service SQLite\n\nAlso remove `tokenService` and `rateLimiter` from `context.ts`.\n\n## Files to create\n\n- `packages/auth-service/src/routes/login-page.ts` — unified login page\n\n## Files to modify\n\n- `packages/auth-service/src/index.ts` — mount new /oauth/authorize, remove old routes\n- `packages/auth-service/src/context.ts` — remove tokenService, rateLimiter\n\n## Files to delete\n\n- `packages/auth-service/src/routes/authorize.ts`\n- `packages/auth-service/src/routes/send-code.ts`\n- `packages/auth-service/src/routes/verify-code.ts`\n- `packages/auth-service/src/magic-link/token.ts`\n- `packages/auth-service/src/magic-link/rate-limit.ts`\n- `packages/auth-service/src/middleware/rate-limit.ts`\n- `packages/auth-service/src/middleware/csrf.ts`\n\n## Acceptance criteria\n\n- [ ] Login page renders with email OTP form\n- [ ] Social login buttons appear only for configured providers\n- [ ] Client branding (logo, name, colors) displayed from OAuth metadata\n- [ ] Email OTP flow works end-to-end through better-auth\n- [ ] Social login (if configured) works end-to-end through better-auth → /auth/complete bridge\n- [ ] Old OTP route files deleted\n- [ ] Old middleware files deleted\n- [ ] context.ts cleaned up","status":"closed","priority":1,"issue_type":"feature","created_at":"2026-02-20T02:16:34Z","updated_at":"2026-02-20T13:04:32Z","closed_at":"2026-02-20T13:04:32Z","close_reason":"Created routes/login-page.ts: unified GET /oauth/authorize that creates auth_flow row, sets magic_auth_flow cookie, renders login page with better-auth email OTP form (JavaScript fetch to /api/auth/* endpoints) and optional social provider buttons (Google/GitHub via env vars). Client branding applied. Deleted old authorize.ts, send-code.ts, verify-code.ts. Removed tokenService/rateLimiter from context.ts, deleted magic-link/ directory. 10 login-page tests added, 83 tests total pass.","created_by":"Adam Spiers","dependencies":[{"issue_id":"atproto-sbw","depends_on_id":"atproto-ab0","type":"blocks","created_at":"2026-02-20T02:18:46Z","created_by":"Adam Spiers"},{"issue_id":"atproto-sbw","depends_on_id":"atproto-cr3","type":"blocks","created_at":"2026-02-20T02:18:45Z","created_by":"Adam Spiers"}]} +{"id":"atproto-ye4","title":"Remove malicious scanner IP blocks from Caddyfile","description":"## Files\n- Caddyfile (modify)\n\n## What to do\nRemove the malicious scanner IP block section from the Caddyfile. Specifically, remove these lines:\n\n```\n # Block potentially malicious scanner IP ranges\n # 185.177.72.0/24 - AS211590 Bucklog SARL (FR), known credential/secret harvesting botnet\n # 195.221.56.0/24 - known scanner infrastructure\n @malicious_scanners remote_ip 185.177.72.0/24 195.221.56.0/24\n abort @malicious_scanners\n```\n\nThis is a general-purpose open-source project and hardcoded IP blocks don't belong in the default config.\n\n## Don't\n- Change anything else in the Caddyfile\n- Touch any other files\n- Add any replacement security middleware","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-02T17:50:10.163088326+06:00","updated_at":"2026-03-02T17:51:56.861628458+06:00","closed_at":"2026-03-02T17:51:56.861628458+06:00","close_reason":"7a501cb Remove malicious scanner IP blocks from Caddyfile","created_by":"karma.gainforest.id"} +{"id":"ePDS-1k2.6","title":"Modify epds-callback in pds-core to use handle from signed callback params","description":"## Files\n- packages/pds-core/src/index.ts (modify)\n\n## What to do\nModify the `/oauth/epds-callback` handler so that when a `handle` parameter is present in the verified callback params, it uses that handle for account creation instead of calling `generateRandomHandle()`.\n\n### Step 1: Read handle from verified params\nAfter `verifyCallback()` succeeds (around line 80-90), extract the handle:\n```ts\nconst handle = req.query.handle as string | undefined\n```\nThe handle is already covered by the HMAC signature (task ePDS-1k2.1 adds it to the payload), so if verification passed, the handle is tamper-proof.\n\n### Step 2: Modify the account creation block\nCurrently (around line 150-183), the code does:\n```ts\nfor (let attempt = 0; attempt \u003c 3; attempt++) {\n try {\n const handle = generateRandomHandle(handleDomain)\n account = await provider.accountManager.createAccount(...)\n break\n } catch (createErr) {\n if (attempt === 2) throw createErr\n }\n}\n```\n\nChange to:\n```ts\nif (handle) {\n // User chose a handle — use it directly, no retry loop\n try {\n account = await provider.accountManager.createAccount(\n deviceId, deviceMetadata,\n { locale: 'en', handle, email,\n password: randomBytes(32).toString('hex'),\n inviteCode: process.env.EPDS_INVITE_CODE }\n )\n } catch (createErr: unknown) {\n // Handle collision — redirect back to choose-handle page\n logger.warn({ err: createErr, handle }, 'chosen handle collision at createAccount')\n const authServiceUrl = process.env.AUTH_SERVICE_URL || 'http://auth:3001'\n return res.redirect(`${authServiceUrl}/auth/choose-handle?error=handle_taken`)\n }\n} else {\n // No handle provided — fall back to random handle generation (backward compat)\n for (let attempt = 0; attempt \u003c 3; attempt++) {\n try {\n const randomHandle = generateRandomHandle(handleDomain)\n account = await provider.accountManager.createAccount(\n deviceId, deviceMetadata,\n { locale: 'en', handle: randomHandle, email,\n password: randomBytes(32).toString('hex'),\n inviteCode: process.env.EPDS_INVITE_CODE }\n )\n break\n } catch (createErr: unknown) {\n if (attempt === 2) throw createErr\n }\n }\n}\n```\n\n### Step 3: Validate handle format (defense in depth)\nBefore using the handle, add a basic format check:\n```ts\nif (handle) {\n // Basic format validation — defense in depth (auth-service already validated)\n const localPart = handle.split('.')[0]\n if (!localPart || !/^[a-z0-9][a-z0-9-]{1,18}[a-z0-9]$/.test(localPart)) {\n logger.error({ handle }, 'invalid handle format in epds-callback')\n return res.status(400).send('Invalid handle format')\n }\n}\n```\n\n### Important: AUTH_SERVICE_URL for redirect-back\nThe redirect on handle collision needs to go back to the auth-service. Use `process.env.AUTH_SERVICE_URL` or derive it from the existing config. Check how other redirects to auth-service are constructed in this file (e.g., the redirect to `/oauth/authorize` or `/auth/complete`). Use the same pattern.\n\n## Don't\n- Don't remove the random handle fallback — it must remain for backward compatibility\n- Don't modify the existing-user flow (when account already exists)\n- Don't modify the HMAC verification logic (that's in crypto.ts, handled by task ePDS-1k2.1)\n- Don't add new environment variables — reuse existing ones for the auth-service URL","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-11T22:06:37.974693091+06:00","updated_at":"2026-03-11T22:12:55.608773256+06:00","closed_at":"2026-03-11T22:12:55.608773256+06:00","close_reason":"8e17409 Use chosen handle in epds-callback when present","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-1k2.6","depends_on_id":"ePDS-1k2.1","type":"blocks","created_at":"2026-03-11T22:07:03.939359154+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-1k2.6","depends_on_id":"ePDS-1k2","type":"parent-child","created_at":"2026-03-11T22:06:37.976082926+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-1k2.7","title":"Add tests for handle selection flow","description":"## Files\n- packages/shared/src/__tests__/crypto.test.ts (modify)\n- packages/auth-service/src/__tests__/choose-handle.test.ts (create)\n\n## What to do\n\n### 1. Extend crypto.test.ts with HMAC handle tests\n\nAdd a new describe block for handle-aware signing:\n\n```ts\ndescribe('signCallback / verifyCallback with handle', () =\u003e {\n it('signs and verifies callback with handle param', () =\u003e {\n const secret = 'test-secret'\n const params = {\n request_uri: 'urn:ietf:params:oauth:request_uri:test',\n email: 'alice@example.com',\n approved: '1',\n new_account: '1',\n handle: 'alice.pds.example.com',\n }\n const url = signCallback(secret, params)\n const result = verifyCallback(secret, url)\n expect(result.valid).toBe(true)\n expect(result.handle).toBe('alice.pds.example.com')\n })\n\n it('signs and verifies callback WITHOUT handle (backward compat)', () =\u003e {\n const secret = 'test-secret'\n const params = {\n request_uri: 'urn:ietf:params:oauth:request_uri:test',\n email: 'alice@example.com',\n approved: '1',\n new_account: '1',\n }\n const url = signCallback(secret, params)\n const result = verifyCallback(secret, url)\n expect(result.valid).toBe(true)\n expect(result.handle).toBeUndefined()\n })\n\n it('produces different signatures with vs without handle', () =\u003e {\n const secret = 'test-secret'\n const baseParams = {\n request_uri: 'urn:ietf:params:oauth:request_uri:test',\n email: 'alice@example.com',\n approved: '1',\n new_account: '1',\n }\n const url1 = signCallback(secret, baseParams)\n const url2 = signCallback(secret, { ...baseParams, handle: 'alice.pds.example.com' })\n // Extract sig params — they should differ\n const sig1 = new URL(url1).searchParams.get('sig')\n const sig2 = new URL(url2).searchParams.get('sig')\n expect(sig1).not.toBe(sig2)\n })\n\n it('rejects tampered handle', () =\u003e {\n const secret = 'test-secret'\n const params = {\n request_uri: 'urn:ietf:params:oauth:request_uri:test',\n email: 'alice@example.com',\n approved: '1',\n new_account: '1',\n handle: 'alice.pds.example.com',\n }\n const url = signCallback(secret, params)\n // Tamper with handle\n const tampered = url.replace('alice.pds.example.com', 'evil.pds.example.com')\n const result = verifyCallback(secret, tampered)\n expect(result.valid).toBe(false)\n })\n})\n```\n\n### 2. Create choose-handle.test.ts\n\nTest the handle validation logic and reserved blocklist. Since the route handlers depend on the full auth-service context (DB, better-auth, PDS internal API), focus on unit-testable pieces:\n\n- Extract the handle validation regex and reserved blocklist into exported constants from choose-handle.ts (e.g., `HANDLE_REGEX`, `RESERVED_HANDLES`)\n- Test the regex against edge cases:\n - Valid: 'alice', 'my-handle', 'a1b', 'abc', 'a'.repeat(20) (20 chars)\n - Invalid: 'a' (too short), 'ab' (too short), '-abc' (starts with hyphen), 'abc-' (ends with hyphen), 'ABC' (uppercase), 'a b' (space), '' (empty), 'a'.repeat(21) (too long)\n- Test reserved blocklist: 'admin', 'root', 'support' should be in the list\n\nAdapt the test patterns based on what the choose-handle.ts file actually exports. If the validation logic is inline in the handlers, test it indirectly or suggest extracting it.\n\n## Don't\n- Don't mock the PDS internal API for integration tests (that's beyond scope — focus on unit tests)\n- Don't test the HTML rendering (visual testing is out of scope)\n- Don't add new test dependencies","status":"closed","priority":2,"issue_type":"task","created_at":"2026-03-11T22:06:58.95899271+06:00","updated_at":"2026-03-11T22:15:19.243416937+06:00","closed_at":"2026-03-11T22:15:19.243416937+06:00","close_reason":"5e29d88 Add tests for handle selection flow","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-1k2.7","depends_on_id":"ePDS-1k2.1","type":"blocks","created_at":"2026-03-11T22:07:03.999819933+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-1k2.7","depends_on_id":"ePDS-1k2.4","type":"blocks","created_at":"2026-03-11T22:07:04.0573703+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-1k2.7","depends_on_id":"ePDS-1k2","type":"parent-child","created_at":"2026-03-11T22:06:58.960641795+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-5k2.4","title":"Update login-page.ts OTP input and JS for charset support","description":"## Files\n- packages/auth-service/src/routes/login-page.ts (modify)\n\n## What to do\n\n### 1. Add otpCharset to renderLoginPage opts type (~line 207)\nAfter the existing otpLength field:\n```ts\notpLength: number\notpCharset: 'numeric' | 'alphanumeric'\n```\n\n### 2. Pass otpCharset from the route handler (~line 188)\n```ts\notpLength: ctx.config.otpLength,\notpCharset: ctx.config.otpCharset,\n```\n\n### 3. Update OTP input HTML attributes (line 324-327)\nCurrent code:\n```html\n\u003cinput type=\"text\" id=\"code\" name=\"code\" required\n maxlength=\"${opts.otpLength}\" pattern=\"[0-9]{${opts.otpLength}}\" inputmode=\"numeric\"\n autocomplete=\"one-time-code\" placeholder=\"${'0'.repeat(opts.otpLength)}\" class=\"otp-input\"\n style=\"letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px\"\u003e\n```\n\nReplace with charset-aware version:\n- pattern: `[0-9]{${N}}` → `[A-Za-z0-9]{${N}}` when alphanumeric (accept lowercase input, OTP is uppercase but be lenient on input)\n- inputmode: `numeric` → `text` when alphanumeric\n- Add autocapitalize: `characters` when alphanumeric, `off` when numeric\n- placeholder: `'0'.repeat(N)` → `'X'.repeat(N)` when alphanumeric\n- Keep autocomplete=\"one-time-code\" unchanged\n- Keep the letter-spacing style unchanged\n\n### 4. Inject otpCharset into inline script (~line 363, after var otpLength)\n```js\nvar otpLength = ${opts.otpLength};\nvar otpCharset = ${JSON.stringify(opts.otpCharset)};\n```\n\n### 5. Update subtitle copy in showOtpStep function (~line 370)\nCurrent:\n```js\notpSubtitle.textContent = 'We sent ' + article + ' ' + otpLength + '-digit code to ' + masked;\n```\nChange to:\n```js\notpSubtitle.textContent = 'We sent ' + article + ' ' + otpLength + (otpCharset === 'alphanumeric' ? '-character' : '-digit') + ' code to ' + masked;\n```\n\n### 6. Update subtitle copy in auto-send then callback (~line 502)\nSame change — find the second occurrence of '-digit code to ' in the script and apply the same ternary.\n\n## Don't\n- Don't change autocomplete=\"one-time-code\" — keep it for SMS autofill\n- Don't change the article computation — it's based on the number, not the word, and works correctly for both\n- Don't modify any other route files\n- Don't modify better-auth.ts or context.ts","status":"closed","priority":2,"issue_type":"task","created_at":"2026-03-12T15:40:30.514287425+06:00","updated_at":"2026-03-12T15:46:23.254619166+06:00","closed_at":"2026-03-12T15:46:23.254619166+06:00","close_reason":"997fe95 Update login-page.ts OTP input and JS for charset support","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-5k2.4","depends_on_id":"ePDS-5k2.1","type":"blocks","created_at":"2026-03-12T15:40:30.519152906+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-5k2.4","depends_on_id":"ePDS-5k2","type":"parent-child","created_at":"2026-03-12T15:40:30.516409041+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-6v2","title":"Set up GitHub Actions CI (prettier, eslint, tests)","description":"Add a GitHub Actions workflow that runs on PRs and pushes to main: 1) prettier --check, 2) eslint, 3) pnpm typecheck, 4) pnpm test. Should use pnpm caching for speed. Depends on Prettier and ESLint being set up first.","status":"closed","priority":1,"issue_type":"task","created_at":"2026-02-21T17:15:59Z","updated_at":"2026-02-21T17:37:42Z","closed_at":"2026-02-21T17:37:42Z","close_reason":"CI workflow created: format:check, lint, typecheck, test. All 4 steps verified passing locally.","created_by":"Adam Spiers","dependencies":[{"issue_id":"ePDS-6v2","depends_on_id":"ePDS-nfm","type":"blocks","created_at":"2026-02-21T17:16:08Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-6v2","depends_on_id":"ePDS-35v","type":"blocks","created_at":"2026-02-21T17:16:08Z","created_by":"Adam Spiers"}]} +{"id":"ePDS-7j6l.2","title":"Create tsconfig.e2e.json for features/ TypeScript","description":"## Files\n- tsconfig.e2e.json (create)\n\n## What to do\nCreate tsconfig.e2e.json at the repo root. This is needed because the root tsconfig.json is a composite project-references config that only covers packages/. The features/ directory TypeScript files need their own config.\n\n```json\n{\n \"compilerOptions\": {\n \"target\": \"ES2022\",\n \"module\": \"NodeNext\",\n \"moduleResolution\": \"NodeNext\",\n \"strict\": true,\n \"esModuleInterop\": true,\n \"skipLibCheck\": true,\n \"outDir\": \".e2e-dist\",\n \"rootDir\": \".\",\n \"types\": [\"node\"]\n },\n \"include\": [\n \"features/**/*.ts\"\n ]\n}\n```\n\nThen update the test:e2e script in package.json to tell tsx to use this config:\n```\n\"test:e2e\": \"TSX_TSCONFIG_PATH=tsconfig.e2e.json node --import tsx/esm ./node_modules/.bin/cucumber-js\"\n\"test:e2e:ci\": \"TSX_TSCONFIG_PATH=tsconfig.e2e.json E2E_HEADLESS=true node --import tsx/esm ./node_modules/.bin/cucumber-js\"\n```\n\nKey details:\n- Use `NodeNext` for both module and moduleResolution (matches repo convention, forward-compatible alias for Node16)\n- The `outDir` is `.e2e-dist` — tsx doesn't actually emit files there, but TypeScript needs it set when rootDir is `.\"\n- Do NOT add this tsconfig to the root tsconfig.json references array — it's standalone, not part of the composite build\n- Import paths in features/*.ts files should use .js extensions per repo convention\n\n## Don't\n- Don't modify tsconfig.json (the root composite config)\n- Don't add features/ to any existing tsconfig\n- Don't set moduleResolution to 'bundler' — must be NodeNext for consistency","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:11:50.330611687+06:00","updated_at":"2026-03-23T15:25:43.467636555+06:00","closed_at":"2026-03-23T15:25:43.467636555+06:00","close_reason":"f1a4662 superseded: recreating with e2e/ directory layout","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-7j6l.2","depends_on_id":"ePDS-7j6l.1","type":"blocks","created_at":"2026-03-23T15:11:50.336495315+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.2","depends_on_id":"ePDS-7j6l","type":"parent-child","created_at":"2026-03-23T15:11:50.333058499+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-7j6l.3","title":"Create features/support/env.ts — test environment config","description":"## Files\n- features/support/env.ts (create)\n\n## What to do\nCreate the environment configuration module that all step definitions and hooks import. It reads from environment variables with Railway defaults.\n\n```ts\nexport const testEnv = {\n pdsUrl: process.env.E2E_PDS_URL ?? 'https://karmas-e2e-pds.up.railway.app',\n authUrl: process.env.E2E_AUTH_URL ?? 'https://certified-e2e-auth.up.railway.app',\n demoUrl: process.env.E2E_DEMO_URL ?? 'https://certified-appdemo-production.up.railway.app',\n mailhogUrl: process.env.E2E_MAILHOG_URL ?? '',\n headless: process.env.E2E_HEADLESS === 'true',\n}\n```\n\nKey details:\n- `headless` defaults to `false` (headed) — only true when E2E_HEADLESS is explicitly 'true'. This is intentional: the developer wants to see the browser.\n- `mailhogUrl` defaults to empty string. When empty, MailHog-dependent steps will call `this.pending()` and skip. When set (e.g. 'http://localhost:8025'), those steps execute.\n- All URLs should NOT have trailing slashes.\n- This file has no side effects — it's a pure config export.\n- Use .js extension in any imports from this file (e.g. `import { testEnv } from './env.js'`) per repo convention.\n\n## Don't\n- Don't add any Railway secrets or API keys\n- Don't add Docker-local defaults (pds.test, auth.pds.test) — Railway-first\n- Don't import anything from the monorepo packages (features/ is standalone)","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:12:08.774671069+06:00","updated_at":"2026-03-23T15:25:43.547075911+06:00","closed_at":"2026-03-23T15:25:43.547075911+06:00","close_reason":"f1a4662 superseded: recreating with e2e/ directory layout","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-7j6l.3","depends_on_id":"ePDS-7j6l.2","type":"blocks","created_at":"2026-03-23T15:12:08.779831352+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.3","depends_on_id":"ePDS-7j6l","type":"parent-child","created_at":"2026-03-23T15:12:08.776445192+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-7j6l.5","title":"Create features/support/hooks.ts — browser lifecycle and screenshot on failure","description":"## Files\n- features/support/hooks.ts (create)\n\n## What to do\nCreate the Cucumber hooks file that manages the Playwright browser lifecycle and captures screenshots on failure.\n\n```ts\nimport { BeforeAll, AfterAll, Before, After, setDefaultTimeout } from '@cucumber/cucumber'\nimport { chromium, type Browser } from '@playwright/test'\nimport { mkdir } from 'node:fs/promises'\nimport type { EpdsWorld } from './world.js'\nimport { testEnv } from './env.js'\n\n// Railway services have cold-start latency. Default cucumber step timeout\n// is 5 seconds which is too short for OAuth redirect chains.\nsetDefaultTimeout(60_000)\n\nlet sharedBrowser: Browser\n\nBeforeAll(async function () {\n await mkdir('reports/screenshots', { recursive: true })\n sharedBrowser = await chromium.launch({ headless: testEnv.headless })\n})\n\nBefore(async function (this: EpdsWorld) {\n this.context = await sharedBrowser.newContext()\n this.page = await this.context.newPage()\n this.page.setDefaultNavigationTimeout(30_000)\n this.page.setDefaultTimeout(15_000)\n})\n\nAfter(async function (this: EpdsWorld, scenario) {\n if (scenario.result?.status === 'FAILED') {\n const safeName = scenario.pickle.name\n .replace(/[^a-z0-9]+/gi, '-')\n .replace(/^-|-$/g, '')\n .toLowerCase()\n .slice(0, 100)\n await this.page?.screenshot({\n path: `reports/screenshots/${safeName}.png`,\n fullPage: true,\n })\n }\n await this.context?.close() // closes page too\n})\n\nAfterAll(async function () {\n await sharedBrowser?.close()\n})\n```\n\nKey details:\n- `sharedBrowser` is a MODULE-LEVEL variable, NOT a World property. BeforeAll's `this` is not a World instance — you cannot set `this.browser` in BeforeAll and read it in Before.\n- `setDefaultTimeout(60_000)` — 60 seconds per step. Railway cold starts + OAuth redirect chains can take 10-20 seconds. The default 5s will cause false failures.\n- Each scenario gets its own BrowserContext (isolated cookies, storage, auth state). The page is created from the context. Both are closed in After.\n- Screenshots are saved on FAILURE only, with sanitized filenames (no special chars, max 100 chars).\n- `mkdir('reports/screenshots', { recursive: true })` in BeforeAll ensures the directory exists before any After hook tries to save a screenshot.\n- The After hook checks `scenario.result?.status === 'FAILED'` — the status property comes from cucumber-js's ITestCaseHookParameter.\n\n## Don't\n- Don't store browser on the World class\n- Don't use `scenario.result.status === 'PENDING'` for screenshots — only capture on failure\n- Don't set timeout lower than 30 seconds — Railway latency will cause flakes\n- Don't close the browser in After (only close context) — browser is shared across scenarios\n- Don't use `this.page.close()` — closing the context closes the page automatically","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:12:54.836304099+06:00","updated_at":"2026-03-23T15:25:43.697602544+06:00","closed_at":"2026-03-23T15:25:43.697602544+06:00","close_reason":"f1a4662 superseded: recreating with e2e/ directory layout","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-7j6l.5","depends_on_id":"ePDS-7j6l.4","type":"blocks","created_at":"2026-03-23T15:12:54.840787034+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.5","depends_on_id":"ePDS-7j6l","type":"parent-child","created_at":"2026-03-23T15:12:54.83776654+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-7j6l.9","title":"Add .gitignore entries and verify full e2e run","description":"## Files\n- .gitignore (modify)\n\n## What to do\nAdd gitignore entries for e2e artifacts, then run the full test suite to verify everything works end-to-end.\n\n### 1. Add to .gitignore:\n```\n# E2E test artifacts\nreports/\n.e2e-dist/\n```\n\n### 2. Verify the full run:\n```bash\n# Install deps (should already be done)\npnpm install\n\n# Install browser binary\nnpx playwright install chromium\n\n# Run the e2e suite\npnpm test:e2e\n```\n\n### Expected behavior:\nA Chromium browser window opens (headed mode). For each scenario in passwordless-authentication.feature:\n\n**Scenario 1 (New user authenticates):**\n- Browser navigates to https://certified-appdemo-production.up.railway.app\n- The demo login page loads with an email input\n- The email input is visible (step passes)\n- Email 'alice@example.com' is entered and form submitted\n- Browser follows OAuth redirect chain to auth service\n- OTP form appears on auth service (step passes)\n- Next step 'an OTP email arrives in the mail trap' → PENDING (no MailHog)\n- Remaining steps skipped\n\n**Scenario 2 (Returning user):**\n- Background step 'alice already has a PDS account' → PENDING (needs MailHog for account creation)\n- Remaining steps skipped\n\n**Scenarios 3-5 (OTP config, wrong code, lockout):**\n- Various steps go PENDING (need MailHog or server config changes)\n\n**Scenario 6 (Refresh page):**\n- Browser navigates to demo app\n- Page refreshes\n- Login page renders normally (step passes)\n- 'OTP flow still works to completion' → PENDING\n\n### Expected output:\n- 0 failures (red)\n- Multiple pending steps (yellow)\n- Some passing steps (green)\n- HTML report generated at reports/e2e.html\n- No 'Undefined step' errors\n\n### 3. Also verify:\n- `pnpm typecheck` still passes\n- `pnpm test` (unit tests) still passes\n- `pnpm lint` still passes (or at least no new errors from e2e files)\n\n## Don't\n- Don't commit reports/ or screenshots\n- Don't modify any .feature files\n- Don't change any existing test configuration","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:15:47.3383014+06:00","updated_at":"2026-03-23T15:25:43.972443273+06:00","closed_at":"2026-03-23T15:25:43.972443273+06:00","close_reason":"f1a4662 superseded: recreating with e2e/ directory layout","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-7j6l.9","depends_on_id":"ePDS-7j6l.8","type":"blocks","created_at":"2026-03-23T15:15:47.348336258+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.9","depends_on_id":"ePDS-7j6l","type":"parent-child","created_at":"2026-03-23T15:15:47.339850086+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.9","depends_on_id":"ePDS-7j6l.6","type":"blocks","created_at":"2026-03-23T15:15:47.343003815+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-7j6l.9","depends_on_id":"ePDS-7j6l.7","type":"blocks","created_at":"2026-03-23T15:15:47.345786127+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-961","title":"Final review, manual test, and push all MAGIC_ → EPDS_ renames","description":"After all rename beads are committed locally:\n\n1. Review the full diff from before the rename series: git diff --no-ext-diff origin/main..HEAD\n2. Run pnpm typecheck \u0026\u0026 pnpm test one final time\n3. Manual smoke test: pnpm dev and verify the login flow still works (if environment available)\n4. Push to origin/main\n5. Export beads and commit","status":"closed","priority":2,"issue_type":"task","created_at":"2026-02-24T10:18:07Z","updated_at":"2026-03-11T23:16:00.762047448+06:00","closed_at":"2026-02-24T12:55:19Z","created_by":"Adam Spiers","dependencies":[{"issue_id":"ePDS-961","depends_on_id":"ePDS-di4","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-d9j","type":"blocks","created_at":"2026-02-24T10:18:18Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-lf7","type":"blocks","created_at":"2026-02-24T10:18:18Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-pp6","type":"blocks","created_at":"2026-02-24T10:18:16Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-3xz","type":"blocks","created_at":"2026-02-24T10:18:16Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-10p","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-961","depends_on_id":"ePDS-3x0","type":"blocks","created_at":"2026-02-24T10:18:17Z","created_by":"Adam Spiers"}]} +{"id":"ePDS-9kf","title":"Update recovery flow to use better-auth OTP and /_internal/account-by-email","description":"The current recovery flow (routes/recovery.ts) allows login via backup email. Update it to:\n1. Use better-auth's OTP infrastructure (auth.api.sendVerificationOTP()) for verification instead of custom token service\n2. Call /_internal/account-by-email for backup email→DID lookup instead of reading from account_email table\n3. Integrate with the auth_flow table so request_uri is preserved through recovery\n\nThis is orthogonal to the main login flow but must use the same bridge pattern (better-auth session → /auth/complete → HMAC-signed magic-callback).\n\nRef: better-auth-migration-plan.md 'Open question: Recovery via backup email'","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-02-20T02:19:00Z","updated_at":"2026-02-20T13:04:33Z","closed_at":"2026-02-20T13:04:33Z","close_reason":"Rewrote recovery.ts to use better-auth OTP: auth.api.sendVerificationOTP() for sending, auth.api.signInEmailOTP() for verification. After successful OTP, redirects to /auth/complete (bridge pattern) with magic_auth_flow cookie set to thread request_uri. Backup email lookup via ctx.db.getDidByBackupEmail() retained (auth-service-owned data). 9 recovery tests added, all 83 tests pass.","created_by":"Adam Spiers","dependencies":[{"issue_id":"ePDS-9kf","depends_on_id":"ePDS-cr3","type":"blocks","created_at":"2026-02-20T02:19:11Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-9kf","depends_on_id":"ePDS-hg3","type":"blocks","created_at":"2026-02-20T02:19:12Z","created_by":"Adam Spiers"},{"issue_id":"ePDS-9kf","depends_on_id":"ePDS-bvg","type":"blocks","created_at":"2026-02-20T02:19:11Z","created_by":"Adam Spiers"}]} +{"id":"ePDS-nr36.3","title":"Create e2e/support/env.ts — test environment config","description":"## Files\n- e2e/support/env.ts (create)\n\n## What to do\nCreate the environment configuration module that all step definitions and hooks import. It reads from environment variables with Railway defaults.\n\n```ts\nexport const testEnv = {\n pdsUrl: process.env.E2E_PDS_URL ?? 'https://karmas-e2e-pds.up.railway.app',\n authUrl: process.env.E2E_AUTH_URL ?? 'https://certified-e2e-auth.up.railway.app',\n demoUrl: process.env.E2E_DEMO_URL ?? 'https://certified-appdemo-production.up.railway.app',\n mailhogUrl: process.env.E2E_MAILHOG_URL ?? '',\n headless: process.env.E2E_HEADLESS === 'true',\n}\n```\n\nKey details:\n- `headless` defaults to `false` (headed) — only true when E2E_HEADLESS is explicitly 'true'. This is intentional: the developer wants to see the browser.\n- `mailhogUrl` defaults to empty string. When empty, MailHog-dependent steps will call `this.pending()` and skip. When set (e.g. 'http://localhost:8025'), those steps execute.\n- All URLs should NOT have trailing slashes.\n- This file has no side effects — it's a pure config export.\n- Use .js extension in any imports from this file (e.g. `import { testEnv } from './env.js'`) per repo convention.\n\n## Don't\n- Don't add any Railway secrets or API keys\n- Don't add Docker-local defaults (pds.test, auth.pds.test) — Railway-first\n- Don't import anything from the monorepo packages (e2e/ is standalone)","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:27:03.280793589+06:00","updated_at":"2026-03-23T15:35:45.218200465+06:00","closed_at":"2026-03-23T15:35:45.218200465+06:00","close_reason":"806f838 Create e2e/support/env.ts — test environment config","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-nr36.3","depends_on_id":"ePDS-nr36.2","type":"blocks","created_at":"2026-03-23T15:27:03.286973533+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.3","depends_on_id":"ePDS-nr36","type":"parent-child","created_at":"2026-03-23T15:27:03.283103428+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-nr36.6","title":"Create e2e/step-definitions/common.steps.ts — shared Background steps","description":"## Files\n- e2e/step-definitions/common.steps.ts (create)\n\n## What to do\nImplement the Background steps that appear in passwordless-authentication.feature (and most other feature files). These run before every scenario.\n\nSteps to implement:\n\n### 1. `Given the ePDS test environment is running`\nMake an HTTP GET to `{testEnv.pdsUrl}/xrpc/_health` and assert the response status is 200. This confirms the Railway PDS is reachable before the scenario runs. Use the global `fetch` (Node 18+ built-in).\n\nIf the health check fails, throw an error with a clear message like 'PDS health check failed: {status} at {url}'.\n\n### 2. `Given a demo OAuth client is registered`\nNo-op. The Railway demo app is always registered as an OAuth client via its /client-metadata.json endpoint.\n\n### 3. `Given {string} already has a PDS account`\nFor now, call `this.skipIfNoMailhog()` — creating accounts requires the full OTP flow which needs MailHog.\n\n```ts\nimport { Given } from '@cucumber/cucumber'\nimport type { EpdsWorld } from '../support/world.js'\nimport { testEnv } from '../support/env.js'\n\nGiven('the ePDS test environment is running', async function (this: EpdsWorld) {\n const res = await fetch(\\`\\${testEnv.pdsUrl}/xrpc/_health\\`)\n if (!res.ok) {\n throw new Error(\\`PDS health check failed: \\${res.status} at \\${testEnv.pdsUrl}/xrpc/_health\\`)\n }\n})\n\nGiven('a demo OAuth client is registered', async function (this: EpdsWorld) {\n // No-op: Railway demo app is always registered via /client-metadata.json\n})\n\nGiven('{string} already has a PDS account', async function (this: EpdsWorld, _email: string) {\n this.skipIfNoMailhog()\n})\n```\n\nKey details:\n- Every step function must use `function(this: EpdsWorld)` syntax, NOT arrow functions.\n- Import paths use .js extension: `'../support/world.js'`, `'../support/env.js'`\n- The health check uses global `fetch` — no need to import it (Node 18+)\n\n## Don't\n- Don't use arrow functions for step definitions\n- Don't import from monorepo packages (@certified-app/*)\n- Don't make the health check retry on failure — fail fast","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:28:08.586583064+06:00","updated_at":"2026-03-23T15:42:26.941533578+06:00","closed_at":"2026-03-23T15:42:26.941533578+06:00","close_reason":"4b595e6 feat(e2e): add common Background step definitions","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-nr36.6","depends_on_id":"ePDS-nr36.5","type":"blocks","created_at":"2026-03-23T15:28:08.591030465+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.6","depends_on_id":"ePDS-nr36","type":"parent-child","created_at":"2026-03-23T15:28:08.587920646+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-nr36.8","title":"Create e2e/step-definitions/email.steps.ts — MailHog stub steps","description":"## Files\n- e2e/step-definitions/email.steps.ts (create)\n\n## What to do\nCreate stub step definitions for all email/MailHog-dependent steps from passwordless-authentication.feature. Every step calls `this.skipIfNoMailhog()` immediately, which marks the step as pending when E2E_MAILHOG_URL is not set.\n\n```ts\nimport { Then } from '@cucumber/cucumber'\nimport type { EpdsWorld } from '../support/world.js'\n\nThen('an OTP email arrives in the mail trap for {string}', async function (this: EpdsWorld, _email: string) {\n this.skipIfNoMailhog()\n // TODO: GET {mailhogUrl}/api/v2/search?kind=to\u0026query={email}\n // Parse OTP from email body, store on World for later use\n})\n\nThen('an OTP email arrives in the mail trap', async function (this: EpdsWorld) {\n this.skipIfNoMailhog()\n})\n\nThen('the email subject contains {string} \\\\(new user\\\\)', async function (this: EpdsWorld, _expected: string) {\n this.skipIfNoMailhog()\n})\n\nThen('the email subject contains {string} \\\\(returning user\\\\)', async function (this: EpdsWorld, _expected: string) {\n this.skipIfNoMailhog()\n})\n\nThen('the OTP code in the mail trap is {int} characters of uppercase letters and digits', async function (this: EpdsWorld, _length: number) {\n this.skipIfNoMailhog()\n})\n```\n\nKey details:\n- `this.skipIfNoMailhog()` calls `this.pending()` which throws 'pending'. Cucumber marks the step pending and skips remaining steps.\n- Step text patterns must EXACTLY match the Gherkin text including parenthetical notes like '(new user)' and '(returning user)'. Parentheses are special in cucumber expressions — escape them with `\\\\\\\\` in the string.\n- Import paths use .js extension: `'../support/world.js'`\n\n## Don't\n- Don't implement any actual MailHog API calls yet\n- Don't use arrow functions\n- Don't skip the TODO comments — they document the future implementation","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:29:13.846017205+06:00","updated_at":"2026-03-23T15:43:31.955004366+06:00","closed_at":"2026-03-23T15:43:31.955004366+06:00","close_reason":"10b8301 Add email.steps.ts stub step definitions for MailHog-dependent steps","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-nr36.8","depends_on_id":"ePDS-nr36.5","type":"blocks","created_at":"2026-03-23T15:29:13.8518047+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.8","depends_on_id":"ePDS-nr36","type":"parent-child","created_at":"2026-03-23T15:29:13.847825837+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-nr36.9","title":"Add .gitignore entries and verify full e2e run","description":"## Files\n- .gitignore (modify)\n\n## What to do\nAdd gitignore entries for e2e artifacts, then run the full test suite to verify everything works end-to-end.\n\n### 1. Add to .gitignore:\n```\n# E2E test artifacts\nreports/\n.e2e-dist/\n```\n\n### 2. Verify the full run:\n```bash\npnpm install\nnpx playwright install chromium\npnpm test:e2e\n```\n\n### Expected behavior:\nA Chromium browser window opens (headed mode). For each scenario in passwordless-authentication.feature:\n\n**Scenario 1 (New user authenticates):**\n- Browser navigates to https://certified-appdemo-production.up.railway.app\n- The demo login page loads with an email input\n- Email 'alice@example.com' is entered and form submitted\n- Browser follows OAuth redirect chain to auth service\n- OTP form appears on auth service (step passes)\n- Next step 'an OTP email arrives in the mail trap' → PENDING (no MailHog)\n- Remaining steps skipped\n\n**Scenario 2 (Returning user):**\n- Background step 'alice already has a PDS account' → PENDING (needs MailHog)\n- Remaining steps skipped\n\n**Scenarios 3-5 (OTP config, wrong code, lockout):**\n- Various steps go PENDING\n\n**Scenario 6 (Refresh page):**\n- Browser navigates to demo app, page refreshes, login page renders normally\n- 'OTP flow still works to completion' → PENDING\n\n### Expected output:\n- 0 failures (red)\n- Multiple pending steps (yellow)\n- Some passing steps (green)\n- HTML report generated at reports/e2e.html\n- No 'Undefined step' errors\n\n### 3. Also verify:\n- `pnpm typecheck` still passes\n- `pnpm test` (unit tests) still passes\n\n## Don't\n- Don't commit reports/ or screenshots\n- Don't modify any .feature files\n- Don't change any existing test configuration","status":"closed","priority":1,"issue_type":"task","created_at":"2026-03-23T15:29:35.941774603+06:00","updated_at":"2026-03-23T15:49:49.407405532+06:00","closed_at":"2026-03-23T15:49:49.407405532+06:00","close_reason":"59987d7 Add .gitignore entries and fix e2e run","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-nr36.9","depends_on_id":"ePDS-nr36.7","type":"blocks","created_at":"2026-03-23T15:29:35.949660788+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.9","depends_on_id":"ePDS-nr36.8","type":"blocks","created_at":"2026-03-23T15:29:35.953063879+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.9","depends_on_id":"ePDS-nr36","type":"parent-child","created_at":"2026-03-23T15:29:35.943672423+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-nr36.9","depends_on_id":"ePDS-nr36.6","type":"blocks","created_at":"2026-03-23T15:29:35.946962174+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-se3.11","title":"stale epds-callback references in comments across auth-service and shared packages","description":"Several comments still reference /oauth/epds-callback as the destination of the bridge flow, even though complete.ts now redirects to /oauth/epds-setup: (1) packages/auth-service/src/routes/recovery.ts line 12: 'better-auth session → /auth/complete → HMAC-signed epds-callback' (2) packages/auth-service/src/context.ts line 10: 'Shared HMAC-SHA256 secret for signing epds-callback redirect URLs' (3) packages/shared/src/crypto.ts lines 58 and 83: JSDoc for signCallback/verifyCallback still says 'epds-callback redirect' (4) packages/auth-service/src/__tests__/social-providers.test.ts lines 11 and 115: comments say 'epds-callback'. These are documentation-only issues with no runtime impact, but they will mislead future developers.","status":"open","priority":4,"issue_type":"bug","created_at":"2026-03-11T15:16:36.338701209+06:00","updated_at":"2026-03-11T15:16:36.338701209+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-se3.11","depends_on_id":"ePDS-se3.2","type":"discovered-from","created_at":"2026-03-11T15:16:43.339055561+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-se3.11","depends_on_id":"ePDS-se3","type":"parent-child","created_at":"2026-03-11T15:16:36.340123697+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-se3.6","title":"pds-core/index.ts: duplicate redirect-URL-building logic between epds-setup and epds-callback","description":"The auto-approve redirect branch in /oauth/epds-setup (lines 243-269) duplicates ~30 lines of redirect URL construction that already exist verbatim in /oauth/epds-callback (lines 484-506). Both blocks build a redirect URL with iss/code/state params, handle fragment vs query response_mode, set Cache-Control: no-store, and call res.redirect(303). Any future fix (e.g. adding nonce, changing response_mode handling) must be applied in two places. Extract into a shared helper function, e.g. 'function buildAuthCodeRedirect(redirectUri, code, state, responseMode, pdsUrl): string' in a local lib file or inline helper within index.ts.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-03-11T15:15:39.906912586+06:00","updated_at":"2026-03-11T15:15:39.906912586+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-se3.6","depends_on_id":"ePDS-se3.1","type":"discovered-from","created_at":"2026-03-11T15:16:42.741352608+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-se3.6","depends_on_id":"ePDS-se3","type":"parent-child","created_at":"2026-03-11T15:15:39.908315347+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-se3.7","title":"pds-core/index.ts: @ts-ignore should be @ts-expect-error for sendAuthorizePageFactory import","description":"Line 31 of pds-core/src/index.ts uses '// @ts-ignore' to suppress the missing type declarations for the internal @atproto/oauth-provider import. TypeScript best practice (and the stricter lint rule @typescript-eslint/ban-ts-comment) prefers '@ts-expect-error' because it fails the build if the error is ever resolved (i.e. if @atproto/oauth-provider starts exporting types for this path), preventing stale suppressions. The existing eslint-disable comment on line 30 suppresses ban-ts-comment, which means the weaker @ts-ignore is silently accepted. Change to @ts-expect-error and update the eslint-disable comment accordingly.","status":"open","priority":3,"issue_type":"bug","created_at":"2026-03-11T15:15:50.709423304+06:00","updated_at":"2026-03-11T15:15:50.709423304+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-se3.7","depends_on_id":"ePDS-se3.1","type":"discovered-from","created_at":"2026-03-11T15:16:42.880886876+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-se3.7","depends_on_id":"ePDS-se3","type":"parent-child","created_at":"2026-03-11T15:15:50.71155358+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-wae.2","title":"Fix: client background_color override broken by body background in light mode (from ePDS-wae.1)","description":"Review of ePDS-wae.1 found: The client branding `background_color` override is applied as `background` on the `\u003chtml\u003e` element via `rootStyle`, but the `body` has an explicit `background: white` rule inside `@media (prefers-color-scheme: light)` that takes precedence (body \u003e html for background painting). As a result, any client-supplied `background_color` is silently ignored in light mode. Evidence: login-page.ts line 335-337 sets `body { background: white; }` in a light-mode media query, overriding the html-level inline style. Fix: apply the `background_color` override directly to `body` via a `\u003cstyle\u003e` block or move the override to a CSS custom property consumed by body.","status":"closed","priority":2,"issue_type":"bug","created_at":"2026-03-03T16:04:00.059722+06:00","updated_at":"2026-03-03T16:12:35.253652998+06:00","closed_at":"2026-03-03T16:12:35.253652998+06:00","close_reason":"2bc9fac fix background_color override: move from html inline style to body-targeted style block after light-mode media query","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-wae.2","depends_on_id":"ePDS-wae.1","type":"discovered-from","created_at":"2026-03-03T16:04:43.960030499+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-wae.2","depends_on_id":"ePDS-wae","type":"parent-child","created_at":"2026-03-03T16:04:00.060999358+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-wae.5","title":"Fix: success admonition uses wrong icon (alert triangle instead of checkmark) (from ePDS-wae.1)","description":"Review of ePDS-wae.1 found: The admonition element (#error-msg) always renders an alert triangle SVG icon regardless of whether it is showing an error or a success message. When `showSuccess('Code resent!')` is called, the element switches to the `.success` CSS class (green border/background) but the icon remains the warning triangle, which is semantically incorrect and visually jarring. Evidence: login-page.ts lines 699-706 — the alert triangle SVG is hardcoded inside the admonition div. Fix: either use two separate icon spans (one for error, one for success) toggled by JS, or replace the icon SVG via JS in `showError()`/`showSuccess()`.","status":"closed","priority":3,"issue_type":"bug","created_at":"2026-03-03T16:04:19.77674497+06:00","updated_at":"2026-03-03T16:11:00.896703344+06:00","closed_at":"2026-03-03T16:11:00.896703344+06:00","close_reason":"2d58468 fix: success admonition shows checkmark icon instead of alert triangle","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-wae.5","depends_on_id":"ePDS-wae.1","type":"discovered-from","created_at":"2026-03-03T16:04:44.125002962+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-wae.5","depends_on_id":"ePDS-wae","type":"parent-child","created_at":"2026-03-03T16:04:19.777896746+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-zdr","title":"Fix: missing test coverage for ePDS-wh1.3/wh1.4/wh1.6 new behaviors (from ePDS-wh1)","description":"Review of ePDS-wh1 tasks found three new behaviors with no test coverage:\n\n1. ePDS-wh1.4 (verifyCallback hex validation): The existing test 'rejects wrong-length sig' passes 'tooshort' but there is no test for a 64-character string containing non-hex characters (e.g. 64 x 'Z'). This is the exact attack vector the fix was designed to prevent — a malformed sig that passes the old length check but causes Buffer.from truncation. A test like: expect(verifyCallback(params, ts, 'Z'.repeat(64), secret)).toBe(false) should be added to packages/shared/src/__tests__/crypto.test.ts.\n\n2. ePDS-wh1.3 (check-handle 503): No test covers the new catch branch returning 503 { error: 'handle_check_failed' }. The choose-handle.test.ts has no test for when the PDS check-handle endpoint returns 503 and how the auth-service handles it.\n\n3. ePDS-wh1.6 (ping non-OK response): No test covers the new pingRes.ok check in the choose-handle GET handler. The choose-handle.test.ts has no test for when ping-request returns a non-2xx status (e.g. 404 request_expired) and the handler should return 400 with 'Session expired'.\n\nEvidence: grep -n 'non-hex|uppercase|ABCDEF|handle_check_failed|503|ping|pingRes|request_expired|session expired' packages/shared/src/__tests__/crypto.test.ts packages/auth-service/src/__tests__/choose-handle.test.ts returns no matches.","status":"open","priority":3,"issue_type":"bug","created_at":"2026-03-12T14:20:37.207718023+06:00","updated_at":"2026-03-12T14:20:37.207718023+06:00","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.3","type":"discovered-from","created_at":"2026-03-12T14:20:41.381893767+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.6","type":"discovered-from","created_at":"2026-03-12T14:20:41.441546677+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zdr","depends_on_id":"ePDS-wh1.4","type":"discovered-from","created_at":"2026-03-12T14:20:41.319967668+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-zgl","title":"Fix: better-auth.ts re-parses OTP_LENGTH independently instead of using ctx.config.otpLength (from ePDS-ipm)","description":"Review of ePDS-ipm found: createBetterAuth() and runBetterAuthMigrations() both re-read process.env.OTP_LENGTH directly instead of accepting the already-validated otpLength from AuthServiceConfig. This creates a seam where the two can diverge.\n\nEvidence:\n- index.ts main() validates config.otpLength (range 4-12) and stores it in AuthServiceConfig\n- better-auth.ts createBetterAuth() re-reads process.env.OTP_LENGTH independently (line 149)\n- better-auth.ts runBetterAuthMigrations() re-reads process.env.OTP_LENGTH independently (line 84)\n- createBetterAuth() is called from createAuthService() which receives a fully-built AuthServiceConfig, but otpLength is never passed through\n\nDivergence scenarios:\n1. If OTP_LENGTH='' (empty string): index.ts uses || so defaults to '8'; better-auth.ts uses ?? so passes '' to parseInt → NaN → better-auth gets NaN otpLength (validation in main() doesn't protect this path since createAuthService can be called without main())\n2. If createAuthService is called programmatically with a config.otpLength that differs from process.env.OTP_LENGTH, the UI and better-auth will disagree on code length\n\nFix: Add otpLength parameter to createBetterAuth() and runBetterAuthMigrations(), pass ctx.config.otpLength from index.ts. This makes the config the single source of truth.","status":"closed","priority":2,"issue_type":"bug","created_at":"2026-03-11T23:16:49.135849984+06:00","updated_at":"2026-03-11T23:36:39.900982214+06:00","closed_at":"2026-03-11T23:36:39.900982214+06:00","close_reason":"1b73865 fix: pass otpLength param to createBetterAuth and runBetterAuthMigrations","created_by":"karma.gainforest.id","dependencies":[{"issue_id":"ePDS-zgl","depends_on_id":"ePDS-ipm","type":"parent-child","created_at":"2026-03-11T23:33:29.694197249+06:00","created_by":"karma.gainforest.id"},{"issue_id":"ePDS-zgl","depends_on_id":"ePDS-ipm","type":"discovered-from","created_at":"2026-03-11T23:16:49.248905084+06:00","created_by":"karma.gainforest.id"}]} diff --git a/e2e/step-definitions/auth.steps.ts b/e2e/step-definitions/auth.steps.ts index 84c1b4b4..08947147 100644 --- a/e2e/step-definitions/auth.steps.ts +++ b/e2e/step-definitions/auth.steps.ts @@ -318,6 +318,14 @@ When('enters an incorrect OTP code', async function (this: EpdsWorld) { const wrongOtp = await buildIncorrectOtpCode(this) await page.fill('#code', wrongOtp) + // New users see a ToS checkbox that becomes `required` once the async + // new-user check resolves. `requests an OTP for a unique test email` + // always creates a new user, so wait for #tos-field to appear and check + // it — otherwise the HTML5 `required` guard or the JS submit guard will + // short-circuit before the server OTP validation we actually want to + // exercise here. Synchronous isVisible() races the background check. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await page.check('#tos-accept') await page.click('#form-verify-otp .btn-primary') }) @@ -327,6 +335,14 @@ When( const page = getPage(this) const wrongOtp = await buildIncorrectOtpCode(this) + // New users see a ToS checkbox that becomes `required` once the async + // new-user check resolves. `requests an OTP for a unique test email` + // always creates a new user, so wait for #tos-field and accept the ToS + // once up front so the POSTs go through on every iteration. Synchronous + // isVisible() races the background check. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await page.check('#tos-accept') + for (let i = 0; i < times; i++) { await page.fill('#code', wrongOtp) await Promise.all([ diff --git a/e2e/step-definitions/client-branding.steps.ts b/e2e/step-definitions/client-branding.steps.ts index f07499f2..445a94fe 100644 --- a/e2e/step-definitions/client-branding.steps.ts +++ b/e2e/step-definitions/client-branding.steps.ts @@ -186,6 +186,10 @@ When( const message = await waitForEmail(`to:${email}`) const otp = await extractOtp(message.ID) await page.fill('#code', otp) + // New users must accept ToS before the client-side submit guard lets the + // sign-in proceed. Wait for #tos-field to be revealed by checkIsNewUser. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await page.check('#tos-accept') await page.click('#form-verify-otp .btn-primary') // Wait for the choose-handle page — don't submit the handle form diff --git a/e2e/step-definitions/tos.steps.ts b/e2e/step-definitions/tos.steps.ts new file mode 100644 index 00000000..634f5418 --- /dev/null +++ b/e2e/step-definitions/tos.steps.ts @@ -0,0 +1,207 @@ +/** + * Step definitions for the Terms-of-Service acceptance enforcement + * scenarios in features/security.feature and features/passwordless-authentication.feature. + * + * Server-side enforcement lives in packages/auth-service/src/better-auth.ts + * (enforceTosAcceptance, wired as a hooks.before on better-auth). The unit + * tests in better-auth-otp.test.ts cover that helper in isolation; these + * E2E steps exercise it end-to-end against the deployed auth service: + * + * - "new user without tosAccepted" → direct HTTP against + * /api/auth/email-otp/send-verification-otp + /api/auth/sign-in/email-otp, + * asserting a 400 with the expected error message. + * + * - "returning user without tosAccepted" → creates a real account via + * the browser-driven sign-up flow, then drives a fresh sign-in via + * direct HTTP with no tosAccepted flag, asserting success. + * + * - "OTP form does not show a ToS checkbox" → DOM assertion that + * #tos-field stays hidden on the login page for the returning-user + * case (the email step has already left the world on the OTP form). + */ + +import { Then, When } from '@cucumber/cucumber' +import { expect } from '@playwright/test' +import { testEnv } from '../support/env.js' +import type { EpdsWorld } from '../support/world.js' +import { getPage } from '../support/utils.js' +import { clearMailpit, extractOtp, waitForEmail } from '../support/mailpit.js' + +const AUTH_BASE = () => `${testEnv.authUrl}/api/auth` + +async function sendVerificationOtp(email: string): Promise { + const res = await fetch(`${AUTH_BASE()}/email-otp/send-verification-otp`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, type: 'sign-in' }), + }) + if (!res.ok) { + const text = await res.text() + throw new Error( + `send-verification-otp failed: ${res.status} ${text.slice(0, 200)}`, + ) + } +} + +async function signInWithOtp( + world: EpdsWorld, + email: string, + otp: string, + includeTosAccepted: boolean, +): Promise { + const body: Record = { email, otp } + if (includeTosAccepted) body.tosAccepted = true + + const res = await fetch(`${AUTH_BASE()}/sign-in/email-otp`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + world.lastHttpStatus = res.status + const text = await res.text() + try { + world.lastHttpJson = JSON.parse(text) as Record + } catch { + world.lastHttpJson = { body: text } + } +} + +When( + 'a new user submits a valid OTP code without the tosAccepted flag', + async function (this: EpdsWorld) { + if (!testEnv.mailpitPass) return 'pending' + const email = `tos-new-${Date.now()}@example.com` + this.testEmail = email + + await clearMailpit(email) + await sendVerificationOtp(email) + const message = await waitForEmail(`to:${email}`) + const otp = await extractOtp(message.ID) + + await signInWithOtp(this, email, otp, false) + }, +) + +When( + 'the returning user submits a valid OTP code without the tosAccepted flag', + async function (this: EpdsWorld) { + if (!testEnv.mailpitPass) return 'pending' + if (!this.testEmail) { + throw new Error( + 'No testEmail — "a returning user has a PDS account" Given must run first', + ) + } + await clearMailpit(this.testEmail) + await sendVerificationOtp(this.testEmail) + const message = await waitForEmail(`to:${this.testEmail}`) + const otp = await extractOtp(message.ID) + + await signInWithOtp(this, this.testEmail, otp, false) + }, +) + +Then('the auth service returns a 400 Bad Request', function (this: EpdsWorld) { + expect(this.lastHttpStatus).toBe(400) +}) + +Then( + 'the error message is {string}', + function (this: EpdsWorld, expected: string) { + const body = this.lastHttpJson ?? {} + const message = (body as { message?: string }).message + expect(message).toBe(expected) + }, +) + +Then('the sign-in succeeds', function (this: EpdsWorld) { + expect(this.lastHttpStatus).toBe(200) +}) + +Then( + 'the OTP form does not show a Terms of Service checkbox', + async function (this: EpdsWorld) { + const page = getPage(this) + // The server renders #tos-field into the DOM unconditionally but + // sets inline display:none for non-new-users; the client-side + // isNewUser check flips it to block only when isNewUser === true. + // Assert it's hidden (either absent or display:none). + const tosField = page.locator('#tos-field') + await expect(tosField).toBeHidden() + }, +) + +Then( + 'the OTP form shows a Terms of Service checkbox', + async function (this: EpdsWorld) { + const page = getPage(this) + // Client-side JS flips #tos-field from display:none to display:block + // once checkIsNewUser confirms isNewUser === true. Wait up to 10s for + // the Path B new-user-check POST to resolve. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await expect(page.locator('#tos-accept')).toBeVisible() + }, +) + +When( + 'the user submits the OTP code without accepting the Terms of Service', + async function (this: EpdsWorld) { + if (!testEnv.mailpitPass) return 'pending' + const page = getPage(this) + + // The preceding "the user requests an OTP for {string}" step doesn't + // populate world.otpCode, so fetch it here from the input that was + // used on the login page — mailpit is keyed by recipient. + const email = await page.inputValue('#email') + if (!email) throw new Error('Email input is empty — cannot locate OTP') + const message = await waitForEmail(`to:${email}`) + const otp = await extractOtp(message.ID) + this.otpCode = otp + + // Deliberately leave #tos-accept unchecked. This scenario verifies the + // *client-side* guard: by the time we click Verify, #tos-field is + // visible (the prior step waited for it) so isNewUser === true, and the + // submit handler short-circuits with showError(...) before any fetch is + // issued. The "sign-in is not completed" assertion below confirms the UI + // stayed put. The server-side enforceTosAcceptance hook is covered + // separately by the direct-HTTP scenario in features/security.feature. + // + // Strip the HTML5 `required` attribute so the browser's native form + // validation doesn't block submission first — we want the JS guard to + // be the thing that fires. + await page.evaluate(() => { + document.getElementById('tos-accept')?.removeAttribute('required') + }) + await page.fill('#code', otp) + await page.click('#form-verify-otp .btn-primary') + }, +) + +When('the user accepts the Terms of Service', async function (this: EpdsWorld) { + const page = getPage(this) + await page.check('#tos-accept') +}) + +When( + 'the user enters the OTP code from the email', + async function (this: EpdsWorld) { + if (!testEnv.mailpitPass) return 'pending' + const page = getPage(this) + const email = await page.inputValue('#email') + if (!email) throw new Error('Email input is empty — cannot locate OTP') + const message = await waitForEmail(`to:${email}`) + const otp = await extractOtp(message.ID) + this.otpCode = otp + await page.fill('#code', otp) + await page.click('#form-verify-otp .btn-primary') + }, +) + +Then('the sign-in is not completed', async function (this: EpdsWorld) { + const page = getPage(this) + // Stay on the login page — never reach /welcome or /auth/choose-handle. + // Give the browser a moment to process any navigation that might occur. + await page.waitForTimeout(1500) + const url = page.url() + expect(url).not.toMatch(/\/welcome/) + expect(url).not.toMatch(/\/auth\/choose-handle/) +}) diff --git a/e2e/support/flows.ts b/e2e/support/flows.ts index e2256fc2..0df1052d 100644 --- a/e2e/support/flows.ts +++ b/e2e/support/flows.ts @@ -117,6 +117,11 @@ export async function startSignUpAwaitingConsent( const message = await waitForEmail(`to:${email}`) const otp = await extractOtp(message.ID) await page.fill('#code', otp) + // New users must accept the Terms of Service before the client-side submit + // guard (and the server-side enforceTosAcceptance hook) will let the sign-in + // through. The #tos-field is revealed once checkIsNewUser resolves to true. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await page.check('#tos-accept') await page.click('#form-verify-otp .btn-primary') await pickHandle(world) @@ -154,6 +159,11 @@ export async function createAccountViaOAuth( const message = await waitForEmail(`to:${email}`) const otp = await extractOtp(message.ID) await page.fill('#code', otp) + // New users must accept the Terms of Service before the client-side submit + // guard (and the server-side enforceTosAcceptance hook) will let the sign-in + // through. The #tos-field is revealed once checkIsNewUser resolves to true. + await expect(page.locator('#tos-field')).toBeVisible({ timeout: 10_000 }) + await page.check('#tos-accept') await page.click('#form-verify-otp .btn-primary') // Pick a handle on the /auth/choose-handle page. The handle-picking logic diff --git a/features/passwordless-authentication.feature b/features/passwordless-authentication.feature index d95e585d..a1a8574f 100644 --- a/features/passwordless-authentication.feature +++ b/features/passwordless-authentication.feature @@ -21,11 +21,21 @@ Feature: Passwordless authentication via email OTP # ("{{code}} — your {{app_name}} code"), so subject contains app name. And the email subject contains "ePDS Demo" And the login page shows an OTP verification form - When the user enters the OTP code + And the OTP form shows a Terms of Service checkbox + When the user accepts the Terms of Service + And the user enters the OTP code from the email And the user picks a handle Then the browser is redirected back to the demo client And the demo client has a valid OAuth access token + Scenario: New user cannot complete OTP sign-in without accepting Terms of Service + When the user requests an OTP for "newuser@example.com" + Then the login page shows an OTP verification form + And the OTP form shows a Terms of Service checkbox + When the user submits the OTP code without accepting the Terms of Service + Then the verification form shows an error message + And the sign-in is not completed + @email Scenario: Returning user authenticates with email OTP Given a returning user has a PDS account @@ -43,6 +53,7 @@ Feature: Passwordless authentication via email OTP And the user enters the test email on the login page Then an OTP email arrives in the mail trap And the email subject contains "ePDS Demo" + And the OTP form does not show a Terms of Service checkbox When the user enters the OTP code Then the browser is redirected back to the demo client with a valid session diff --git a/features/security.feature b/features/security.feature index f8d0b005..3cec6240 100644 --- a/features/security.feature +++ b/features/security.feature @@ -97,6 +97,20 @@ Feature: Security measures When a GET request is sent to the PDS /oauth/authorize with sec-fetch-site "same-site" Then the response is not a 400 error about forbidden sec-fetch-site header + # --- Terms of Service enforcement --- + + @email + Scenario: Server rejects OTP sign-in for new user without ToS acceptance + When a new user submits a valid OTP code without the tosAccepted flag + Then the auth service returns a 400 Bad Request + And the error message is "You must accept the Terms of Service to create an account." + + @email + Scenario: Returning user can sign in without sending tosAccepted + Given a returning user has a PDS account + When the returning user submits a valid OTP code without the tosAccepted flag + Then the sign-in succeeds + # --- Email privacy --- @pending diff --git a/packages/auth-service/src/__tests__/better-auth-otp.test.ts b/packages/auth-service/src/__tests__/better-auth-otp.test.ts index a1ad0b24..9ebe49d5 100644 --- a/packages/auth-service/src/__tests__/better-auth-otp.test.ts +++ b/packages/auth-service/src/__tests__/better-auth-otp.test.ts @@ -6,10 +6,14 @@ * 2. Looks up the auth_flow row to find the client_id for branding * 3. Falls back to default PDS template if no client context * - * Since we can't easily instantiate a full better-auth instance in tests, - * we test the DB lookup logic and EmailSender integration directly. + * Also tests enforceTosAcceptance (the exported hooks.before logic): + * 1. Returning users (has PDS DID) — skip ToS check entirely + * 2. New user + tosAccepted: true — allow through + * 3. New user + tosAccepted: false/absent — reject with BAD_REQUEST + * 4. getDidByEmail throws (null DID) — fail closed; tosAccepted === true required to proceed */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { enforceTosAcceptance } from '../better-auth.js' import * as fs from 'node:fs' import * as path from 'node:path' import * as os from 'node:os' @@ -186,3 +190,152 @@ describe('sendVerificationOTP client branding via auth_flow', () => { fetchSpy.mockRestore() }) }) + +// --------------------------------------------------------------------------- +// enforceTosAcceptance — the exported hooks.before logic +// +// Tests call the real function from better-auth.ts directly. +// globalThis.fetch is mocked to control what getDidByEmail sees, following +// the same pattern used in the sendVerificationOTP tests above. +// --------------------------------------------------------------------------- + +describe('enforceTosAcceptance (hooks.before for sign-in/email-otp)', () => { + let fetchSpy: ReturnType + + beforeEach(() => { + fetchSpy = vi.spyOn(globalThis, 'fetch') + }) + + afterEach(() => { + fetchSpy.mockRestore() + }) + + const PDS_URL = 'https://core:3000' + const SECRET = 'test-secret' + + it('returns without error when path is not /sign-in/email-otp (path guard)', async () => { + // fetch should never be called — path guard short-circuits + await expect( + enforceTosAcceptance( + { + path: '/email-otp/send-verification-otp', + body: { email: 'user@example.com', tosAccepted: false }, + }, + PDS_URL, + SECRET, + ), + ).resolves.toBeUndefined() + expect(fetchSpy).not.toHaveBeenCalled() + }) + + it('returns without error when email is absent (let schema validation handle it)', async () => { + await expect( + enforceTosAcceptance( + { path: '/sign-in/email-otp', body: {} }, + PDS_URL, + SECRET, + ), + ).resolves.toBeUndefined() + expect(fetchSpy).not.toHaveBeenCalled() + }) + + it('returns without error for returning user regardless of tosAccepted', async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ did: 'did:plc:returning123' }), { + status: 200, + }), + ) + await expect( + enforceTosAcceptance( + { + path: '/sign-in/email-otp', + body: { email: 'returning@example.com', tosAccepted: false }, + }, + PDS_URL, + SECRET, + ), + ).resolves.toBeUndefined() + }) + + it('returns without error for new user when tosAccepted is true', async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ did: null }), { status: 200 }), + ) + await expect( + enforceTosAcceptance( + { + path: '/sign-in/email-otp', + body: { email: 'newuser@example.com', tosAccepted: true }, + }, + PDS_URL, + SECRET, + ), + ).resolves.toBeUndefined() + }) + + it('throws BAD_REQUEST for new user when tosAccepted is false', async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ did: null }), { status: 200 }), + ) + await expect( + enforceTosAcceptance( + { + path: '/sign-in/email-otp', + body: { email: 'newuser@example.com', tosAccepted: false }, + }, + PDS_URL, + SECRET, + ), + ).rejects.toThrow( + 'You must accept the Terms of Service to create an account.', + ) + }) + + it('throws BAD_REQUEST for new user when tosAccepted is absent', async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ did: null }), { status: 200 }), + ) + await expect( + enforceTosAcceptance( + { path: '/sign-in/email-otp', body: { email: 'newuser@example.com' } }, + PDS_URL, + SECRET, + ), + ).rejects.toThrow( + 'You must accept the Terms of Service to create an account.', + ) + }) + + it('enforces ToS when PDS is unreachable (getDidByEmail returns null on error)', async () => { + // getDidByEmail catches all fetch errors internally and returns null. + // A null DID → treated as new user → ToS is still required. + // This is the safe default: we cannot confirm the account exists. + fetchSpy.mockRejectedValueOnce(new Error('Network error: PDS unreachable')) + await expect( + enforceTosAcceptance( + { + path: '/sign-in/email-otp', + body: { email: 'newuser@example.com', tosAccepted: false }, + }, + PDS_URL, + SECRET, + ), + ).rejects.toThrow( + 'You must accept the Terms of Service to create an account.', + ) + }) + + it('allows sign-in when PDS is unreachable but tosAccepted is true', async () => { + fetchSpy.mockRejectedValueOnce(new Error('Network error: PDS unreachable')) + await expect( + enforceTosAcceptance( + { + path: '/sign-in/email-otp', + body: { email: 'newuser@example.com', tosAccepted: true }, + }, + PDS_URL, + SECRET, + ), + ).resolves.toBeUndefined() + }) +}) diff --git a/packages/auth-service/src/better-auth.ts b/packages/auth-service/src/better-auth.ts index 19e1de18..79c10cb7 100644 --- a/packages/auth-service/src/better-auth.ts +++ b/packages/auth-service/src/better-auth.ts @@ -11,7 +11,8 @@ */ import type { EpdsDb } from '@certified-app/shared' import { createLogger } from '@certified-app/shared' -import { betterAuth } from 'better-auth' +import { betterAuth, APIError } from 'better-auth' +import { createAuthMiddleware } from 'better-auth/api' import { generateRandomString } from 'better-auth/crypto' import { getMigrations } from 'better-auth/db' import { emailOTP } from 'better-auth/plugins' @@ -22,6 +23,52 @@ import { ensurePdsUrl } from './lib/pds-url.js' export type BetterAuthInstance = ReturnType +/** + * Enforce ToS acceptance for new users on the OTP sign-in endpoint. + * + * Exported separately from createBetterAuth so it can be unit-tested directly + * without instantiating a full better-auth instance. + * + * Called from hooks.before — fires on every better-auth request, so the path + * guard is load-bearing (better-auth's matcher is hardcoded to `() => true`). + * + * Execution order: runBeforeHooks → endpoint (Zod validation + handler). + * ctx.body therefore still contains raw pre-validation JSON, including any + * tosAccepted field, before Zod strips unknown keys. + * + * PDS-unreachable behaviour: getDidByEmail never throws — it catches all + * errors internally and returns null. A null DID is treated as a new user, + * so ToS acceptance is still required. This is the safe default: we cannot + * confirm the account exists, so we enforce consent. + */ +export async function enforceTosAcceptance( + ctx: { path: string; body: unknown }, + pdsUrl: string, + internalSecret: string, +): Promise { + // Path guard — this function is called on every better-auth request + if (ctx.path !== '/sign-in/email-otp') return + + const rawEmail = (ctx.body as { email?: unknown } | undefined)?.email + if (typeof rawEmail !== 'string') return // let better-auth's own schema validation reject it + const email = rawEmail.trim().toLowerCase() + if (!email) return // let better-auth's own schema validation reject it + + const did = await getDidByEmail(email, pdsUrl, internalSecret) + const isNewUser = !did + if (!isNewUser) return // returning users already accepted ToS at sign-up + + // New user (or PDS unreachable — null DID): require explicit ToS acceptance. + // tosAccepted is sent as a JSON boolean by the client's verifyOtp(). + const tosAccepted = (ctx.body as { tosAccepted?: boolean } | undefined) + ?.tosAccepted + if (tosAccepted !== true) { + throw new APIError('BAD_REQUEST', { + message: 'You must accept the Terms of Service to create an account.', + }) + } +} + const logger = createLogger('auth:better-auth') const AUTH_FLOW_COOKIE = 'epds_auth_flow' @@ -165,6 +212,17 @@ export function createBetterAuth( socialProviders, + hooks: { + before: createAuthMiddleware(async (ctx) => { + const pdsUrl = ensurePdsUrl( + process.env.PDS_INTERNAL_URL, + `https://${process.env.PDS_HOSTNAME ?? 'localhost'}`, + ) + const internalSecret = process.env.EPDS_INTERNAL_SECRET ?? '' + await enforceTosAcceptance(ctx, pdsUrl, internalSecret) + }), + }, + plugins: [ emailOTP({ otpLength, diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 475c9566..76d819e5 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -19,6 +19,11 @@ * - Social: user clicks button → /api/auth/sign-in/{provider} → OAuth exchange * - On success, better-auth redirects to /auth/complete (the bridge route) * - Bridge reads epds_auth_flow cookie → auth_flow → HMAC-signed redirect + * + * Path B (no login_hint) new-user check: + * POST /api/auth/new-user-check — email in JSON body, x-csrf-token header required. + * Called in parallel with sendOtp to determine whether to show the ToS checkbox. + * Email is kept out of the URL to avoid PII appearing in access logs. */ import { Router, type Request, type Response } from 'express' import { randomBytes } from 'node:crypto' @@ -48,6 +53,7 @@ import { buildPdsAuthorizeRedirect, shouldReuseSession, } from '../lib/session-reuse.js' +import { getDidByEmail } from '../lib/get-did-by-email.js' const logger = createLogger('auth:login-page') @@ -86,6 +92,12 @@ export function resolveHandleMode( export function createLoginPageRouter(ctx: AuthServiceContext): Router { const router = Router() + const pdsInternalUrl = ensurePdsUrl( + process.env.PDS_INTERNAL_URL, + ctx.config.pdsPublicUrl, + ) + const internalSecret = process.env.EPDS_INTERNAL_SECRET ?? '' + router.get('/oauth/authorize', async (req: Request, res: Response) => { const requestUri = req.query.request_uri as string | undefined const clientId = req.query.client_id as string | undefined @@ -230,12 +242,6 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { // b) On the query string as a handle/DID (unlikely but possible) // c) Only in the stored PAR request (third-party apps like sdsls.dev put // the handle in the PAR body but don't duplicate it on the redirect URL) - const pdsInternalUrl = ensurePdsUrl( - process.env.PDS_INTERNAL_URL, - ctx.config.pdsPublicUrl, - ) - const internalSecret = process.env.EPDS_INTERNAL_SECRET ?? '' - // If no login_hint on the query string, try to retrieve it from the PAR request let effectiveLoginHint = loginHint ?? null if (!effectiveLoginHint && requestUri) { @@ -257,6 +263,12 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { const hasLoginHint = !!resolvedEmail const initialStep = hasLoginHint ? 'otp' : 'email' + // Determine if this is a new user when we already know the email (login_hint path). + // null means unknown — no login_hint was provided, Path B will handle it later. + const isNewUser: boolean | null = resolvedEmail + ? !(await getDidByEmail(resolvedEmail, pdsInternalUrl, internalSecret)) + : null + // Pillar 3 — Idempotency (Option A): when this is a duplicate GET for an // existing flow (e.g. browser extension, StayFocusd), tell the client-side // script that OTP was already sent so it skips the auto-send. @@ -288,6 +300,7 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { loginHint: emailHint, initialStep, otpAlreadySent, + isNewUser, csrfToken: res.locals.csrfToken, authBasePath: '/api/auth', pdsPublicUrl: ctx.config.pdsPublicUrl, @@ -297,6 +310,29 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { ) }) + // POST /api/auth/new-user-check + // Used by the login page (Path B) when no login_hint is present — after the + // user types their email and the OTP is sent, this is called in parallel to + // determine whether to show the ToS checkbox. Defaults to isNewUser: true on + // the client side if this route fails, so the checkbox is never silently skipped. + // Email is in the JSON request body (not the URL) to avoid PII in access logs. + // CSRF validation (x-csrf-token header) is enforced by the global csrfProtection + // middleware to prevent cross-origin probing. + router.post( + '/api/auth/new-user-check', + async (req: Request, res: Response) => { + const rawEmail = (req.body as { email?: unknown } | undefined)?.email + const email = + typeof rawEmail === 'string' ? rawEmail.trim().toLowerCase() : '' + if (!email) { + res.status(400).json({ error: 'email required' }) + return + } + const did = await getDidByEmail(email, pdsInternalUrl, internalSecret) + res.json({ isNewUser: !did }) + }, + ) + return router } @@ -309,6 +345,7 @@ export function renderLoginPage(opts: { loginHint: string initialStep: 'email' | 'otp' otpAlreadySent: boolean + isNewUser?: boolean | null csrfToken: string authBasePath: string pdsPublicUrl: string @@ -397,6 +434,10 @@ export function renderLoginPage(opts: { .step-email.hidden { display: none; } .recovery-link { display: block; margin-top: 16px; color: #888; font-size: 13px; text-decoration: none; } .recovery-link:hover { color: #555; } + .tos-field { text-align: left; margin-bottom: 16px; } + .field .tos-label { display: flex; align-items: center; gap: 8px; font-size: 14px; font-weight: normal; cursor: pointer; color: #333; line-height: 1.4; margin-bottom: 0; } + .tos-label input[type=checkbox] { flex-shrink: 0; accent-color: ${escapeHtml(brandColor)}; width: 15px; height: 15px; cursor: pointer; } + .tos-label a { color: ${escapeHtml(brandColor)}; text-decoration: underline; } ${renderOptionalStyleTag(opts.customCss)} @@ -440,6 +481,16 @@ export function renderLoginPage(opts: { oninput="this.value=this.value.replace(/[\\s-]/g,'')" style="letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px"> + @@ -492,6 +543,10 @@ export function renderLoginPage(opts: { stepOtp.classList.remove('active'); stepEmail.classList.remove('hidden'); recoveryLink.style.display = 'none'; + isNewUser = null; + document.getElementById('tos-field').style.display = 'none'; + document.getElementById('tos-accept').required = false; + document.getElementById('tos-accept').checked = false; clearError(); } @@ -513,13 +568,43 @@ export function renderLoginPage(opts: { } } - // Verify OTP via better-auth and redirect - async function verifyOtp(email, otp) { + // Check whether the email belongs to a new user. + // Defaults to true on any failure — safer to show the ToS checkbox + // than to silently skip it for a genuinely new user. + // Email is sent in the POST body (not the URL) to avoid PII in logs. + async function checkIsNewUser(email) { + try { + var res = await fetch('/api/auth/new-user-check', { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'x-csrf-token': csrfToken }, + body: JSON.stringify({ email: email }), + }); + var data = await res.json().catch(function() { return {}; }); + return data.isNewUser !== false; + } catch (err) { + return true; + } + } + + // Sends the OTP and kicks off the new-user check concurrently. + // The OTP result is returned as soon as sendOtp() resolves; isNewUser + // is a Promise that resolves independently so a slow DID lookup never + // delays the OTP screen or surfaces OTP send errors. + async function sendOtpAndCheckNewUser(email) { + var isNewUserPromise = checkIsNewUser(email); + var otpResult = await sendOtp(email); + return { otpResult: otpResult, isNewUser: isNewUserPromise }; + } + + // Verify OTP via better-auth and redirect. + // tosAccepted is a boolean from the #tos-accept checkbox — sent for new + // users so the server-side hook can enforce ToS acceptance before sign-in. + async function verifyOtp(email, otp, tosAccepted) { try { var res = await fetch(authBasePath + '/sign-in/email-otp', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email: email, otp: otp }), + body: JSON.stringify({ email: email, otp: otp, tosAccepted: tosAccepted }), }); if (!res.ok) { var data = await res.json().catch(function() { return {}; }); @@ -543,14 +628,22 @@ export function renderLoginPage(opts: { btn.disabled = true; btn.textContent = 'Sending...'; - var result = await sendOtp(email); + var result = await sendOtpAndCheckNewUser(email); btn.disabled = false; btn.textContent = 'Continue with email'; - if (result.error) { - showError(result.error); + if (result.otpResult.error) { + showError(result.otpResult.error); } else { showOtpStep(email); + // Resolve new-user check asynchronously — OTP screen is already visible. + result.isNewUser.then(function(newUser) { + if (newUser === true) { + isNewUser = true; + document.getElementById('tos-field').style.display = 'block'; + document.getElementById('tos-accept').required = true; + } + }); } }); @@ -559,11 +652,25 @@ export function renderLoginPage(opts: { e.preventDefault(); clearError(); var otp = document.getElementById('code').value.trim(); + // Read checkbox as a boolean — sent to the server for new-user ToS enforcement. + // For returning users tosChecked will be false (checkbox hidden) which is fine: + // the server hook skips the check entirely when the user already has a PDS account. + var tosChecked = document.getElementById('tos-accept').checked; var btn = this.querySelector('button[type=submit]'); + + // Client-side guard: belt-and-suspenders on top of the HTML required + // attribute and the authoritative server-side hook. + // isNewUser === true (strict) so null (Path B before check resolves) is + // intentionally not blocked here — the server hook is the real gate. + if (isNewUser === true && !tosChecked) { + showError('You must accept the Terms of Service to create an account.'); + return; + } + btn.disabled = true; btn.textContent = 'Verifying...'; - var result = await verifyOtp(currentEmail, otp); + var result = await verifyOtp(currentEmail, otp, tosChecked); btn.disabled = false; btn.textContent = 'Verify'; @@ -602,6 +709,13 @@ export function renderLoginPage(opts: { var loginHint = ${JSON.stringify(opts.loginHint)}; var initialStep = ${JSON.stringify(opts.initialStep)}; var otpAlreadySent = ${JSON.stringify(opts.otpAlreadySent)}; + var isNewUser = ${JSON.stringify(opts.isNewUser ?? null)}; + var csrfToken = ${JSON.stringify(opts.csrfToken)}; + + if (isNewUser === true) { + document.getElementById('tos-field').style.display = 'block'; + document.getElementById('tos-accept').required = true; + } if (initialStep === 'otp' && loginHint) { currentEmail = loginHint; diff --git a/packages/auth-service/src/routes/recovery.ts b/packages/auth-service/src/routes/recovery.ts index 83227037..7a7e07ff 100644 --- a/packages/auth-service/src/routes/recovery.ts +++ b/packages/auth-service/src/routes/recovery.ts @@ -197,10 +197,38 @@ export function createRecoveryRouter( return } + // Gate the ToS bypass on the submitted email actually being a verified + // backup email for some PDS account. Without this check, a fresh + // attacker could request an OTP via /sign-in/email-otp for any email, + // then POST it here and bypass the enforceTosAcceptance hook — creating + // an account without ever ticking the ToS box. + const recoveryDid = ctx.db.getDidByBackupEmail(email) + if (!recoveryDid) { + const { customCss, backUri } = await getFlowCss(req) + res.send( + renderRecoveryOtpForm({ + email, + csrfToken: res.locals.csrfToken, + requestUri, + error: 'Invalid or expired code. Please try again.', + otpLength, + otpCharset, + customCss, + backUri, + }), + ) + return + } + try { - // Verify OTP via better-auth — this creates/updates a session + // Verify OTP via better-auth — this creates/updates a session. + // + // `tosAccepted: true` bypasses the enforceTosAcceptance hook. Recovery + // is only reachable for existing users who have already accepted the + // ToS at sign-up. The hook otherwise treats the backup email as a + // "new user" (no PDS account is bound to it) and would reject with 400. const response = await auth.api.signInEmailOTP({ - body: { email, otp: code.toUpperCase() }, + body: { email, otp: code.toUpperCase(), tosAccepted: true }, asResponse: true, })