From c8378e3acc73864fa17835e5c7f6f71b618217dd Mon Sep 17 00:00:00 2001 From: kzoeps Date: Tue, 24 Mar 2026 16:41:13 +0600 Subject: [PATCH 01/17] feat: show tos checkbox if new user when login hint provided --- .beads/issues.jsonl | 29 ++++++++++++++++++ .../auth-service/src/routes/login-page.ts | 30 +++++++++++++++++++ 2 files changed, 59 insertions(+) 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/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 475c9566..8b52e5f5 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -48,6 +48,7 @@ import { buildPdsAuthorizeRedirect, shouldReuseSession, } from '../lib/session-reuse.js' +import { getDidByEmail } from '../lib/get-did-by-email.js' const logger = createLogger('auth:login-page') @@ -257,6 +258,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 +295,7 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { loginHint: emailHint, initialStep, otpAlreadySent, + isNewUser, csrfToken: res.locals.csrfToken, authBasePath: '/api/auth', pdsPublicUrl: ctx.config.pdsPublicUrl, @@ -309,6 +317,7 @@ export function renderLoginPage(opts: { loginHint: string initialStep: 'email' | 'otp' otpAlreadySent: boolean + isNewUser: boolean | null csrfToken: string authBasePath: string pdsPublicUrl: string @@ -397,6 +406,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; } + .tos-label { display: flex; align-items: flex-start; gap: 8px; font-size: 14px; cursor: pointer; color: #333; line-height: 1.4; } + .tos-label input[type=checkbox] { margin-top: 2px; 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 +453,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"> + @@ -560,6 +583,7 @@ export function renderLoginPage(opts: { clearError(); var otp = document.getElementById('code').value.trim(); var btn = this.querySelector('button[type=submit]'); + btn.disabled = true; btn.textContent = 'Verifying...'; @@ -602,6 +626,12 @@ 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)}; + + if (isNewUser === true) { + document.getElementById('tos-field').style.display = 'block'; + document.getElementById('tos-accept').required = true; + } if (initialStep === 'otp' && loginHint) { currentEmail = loginHint; From f6cbfb414d143da3b2eea0038cd2c854e6ca58d9 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Tue, 24 Mar 2026 17:02:11 +0600 Subject: [PATCH 02/17] feat: show tos checkbox if new user when email entered on auth service chore: allow up to 1% coverage decrease in Coveralls --- .coveralls.yml | 1 + .../auth-service/src/routes/login-page.ts | 73 ++++++++++++++++--- 2 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 .coveralls.yml diff --git a/.coveralls.yml b/.coveralls.yml new file mode 100644 index 00000000..018d5ea2 --- /dev/null +++ b/.coveralls.yml @@ -0,0 +1 @@ +coverage_threshold: 1.0 diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 8b52e5f5..86f2d269 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -87,6 +87,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 @@ -231,12 +237,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) { @@ -305,6 +305,27 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { ) }) + // GET /api/auth/new-user-check?email= + // 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. + router.get( + '/api/auth/new-user-check', + async (req: Request, res: Response) => { + const email = + typeof req.query.email === 'string' + ? req.query.email.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 } @@ -407,8 +428,8 @@ export function renderLoginPage(opts: { .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; } - .tos-label { display: flex; align-items: flex-start; gap: 8px; font-size: 14px; cursor: pointer; color: #333; line-height: 1.4; } - .tos-label input[type=checkbox] { margin-top: 2px; flex-shrink: 0; accent-color: ${escapeHtml(brandColor)}; width: 15px; height: 15px; cursor: pointer; } + .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)} @@ -515,6 +536,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(); } @@ -536,6 +561,27 @@ export function renderLoginPage(opts: { } } + // 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. + async function checkIsNewUser(email) { + try { + var res = await fetch('/api/auth/new-user-check?email=' + encodeURIComponent(email)); + var data = await res.json().catch(function() { return {}; }); + return data.isNewUser !== false; + } catch (err) { + return true; + } + } + + // Orchestrates sending the OTP and checking new-user status in parallel. + // If the OTP send fails, the error is surfaced; the new-user check result + // is independent and never blocks the flow. + async function sendOtpAndCheckNewUser(email) { + var results = await Promise.all([sendOtp(email), checkIsNewUser(email)]); + return { otpResult: results[0], isNewUser: results[1] }; + } + // Verify OTP via better-auth and redirect async function verifyOtp(email, otp) { try { @@ -566,13 +612,18 @@ 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 { + if (result.isNewUser === true) { + isNewUser = true; + document.getElementById('tos-field').style.display = 'block'; + document.getElementById('tos-accept').required = true; + } showOtpStep(email); } }); From 3d1869e92f71897ebc5fb42a32c81340aecdb3c9 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Tue, 24 Mar 2026 20:13:13 +0600 Subject: [PATCH 03/17] fix: change to post route for new user check --- .../auth-service/src/routes/login-page.ts | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 86f2d269..46c5f320 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' @@ -305,17 +310,20 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { ) }) - // GET /api/auth/new-user-check?email= + // 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. - router.get( + // 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 email = - typeof req.query.email === 'string' - ? req.query.email.trim().toLowerCase() + typeof req.body.email === 'string' + ? req.body.email.trim().toLowerCase() : '' if (!email) { res.status(400).json({ error: 'email required' }) @@ -564,9 +572,14 @@ export function renderLoginPage(opts: { // 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?email=' + encodeURIComponent(email)); + 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) { @@ -678,6 +691,7 @@ export function renderLoginPage(opts: { var initialStep = ${JSON.stringify(opts.initialStep)}; var otpAlreadySent = ${JSON.stringify(opts.otpAlreadySent)}; var isNewUser = ${JSON.stringify(opts.isNewUser)}; + var csrfToken = ${JSON.stringify(opts.csrfToken)}; if (isNewUser === true) { document.getElementById('tos-field').style.display = 'block'; From a5eccd13a8c86311575ec3c96a2c6f4aa34c40fd Mon Sep 17 00:00:00 2001 From: kzoeps Date: Tue, 24 Mar 2026 21:11:08 +0600 Subject: [PATCH 04/17] fix: check for tos acceptance on verify otp --- .coveralls.yml | 1 - .../src/__tests__/better-auth-otp.test.ts | 157 +++++++++++++++++- packages/auth-service/src/better-auth.ts | 60 ++++++- .../auth-service/src/routes/login-page.ts | 30 +++- 4 files changed, 236 insertions(+), 12 deletions(-) delete mode 100644 .coveralls.yml diff --git a/.coveralls.yml b/.coveralls.yml deleted file mode 100644 index 018d5ea2..00000000 --- a/.coveralls.yml +++ /dev/null @@ -1 +0,0 @@ -coverage_threshold: 1.0 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..121838a5 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) { + 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 46c5f320..1b6ab4b9 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -321,10 +321,9 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { router.post( '/api/auth/new-user-check', async (req: Request, res: Response) => { + const rawEmail = (req.body as { email?: unknown } | undefined)?.email const email = - typeof req.body.email === 'string' - ? req.body.email.trim().toLowerCase() - : '' + typeof rawEmail === 'string' ? rawEmail.trim().toLowerCase() : '' if (!email) { res.status(400).json({ error: 'email required' }) return @@ -484,7 +483,7 @@ export function renderLoginPage(opts: {