rstillwell/ubiquitous-invention
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions
ubiquitous-invention/plans/Plan-multitenant-saas-hardening/Epic-test-foundation/Task-bootstrap-vitest-for-apps-web.md
Randall Stillwell 3a657a4aed feat(identity): OAuth-aware sign-in writes accounts + identity rows (Task 1, part 2/2) 
Closes Task-multi-email-identity. Rebuilds the OAuth half of the jwt
callback around the new user_email_identities table AND the existing
(but until-now empty) accounts table, with per-provider email_verified
resolution and a cross-user conflict guard. Credentials sign-in path is
unchanged.

Per the OAuth research subagent: NextAuth has no adapter configured, so
the accounts table has been sitting empty since this app started. Rather
than leave it that way, the new resolveOAuthUser helper writes to it
on every OAuth sign-in. (provider, providerAccountId) is now the
canonical "this OAuth identity belongs to this user" record and gives
us a fast path that doesn't depend on email matching.

Sign-in resolution order for an OAuth account:

1. Lookup accounts by (provider, providerAccountId).
   Hit -> bump last_used_at on the matching identity row, return user_id.
2. Lookup user_email_identities by (email, verified_at IS NOT NULL).
   Hit AND the owner has zero existing OAuth accounts -> link this new
     OAuth account to that user (covers "Credentials user adds their
     first OAuth provider"). Insert a fresh accounts row.
   Hit AND the owner already has an OAuth account -> REFUSE.  Returning
     a token without an id field denies the session; the user lands on
     NextAuth's error page. (This is the "Bob's GitHub claims alice's
     verified email" rejection.)
3. Fall back to legacy users.email match.
   Hit -> link to that user (covers users created before migration 0005).
4. Otherwise mint a new users row + a source='primary' identity in the
   identities table, then write the accounts row.

The verified identity row is upserted only when the provider's
email_verified claim is true. The new resolveOAuthEmailVerified helper:

- Google + Authentik: read profile.email_verified directly (the Auth.js
  v5 jwt callback receives `profile` on the sign-in trigger). Authentik
  caveat documented inline: since the 2025.10 release the claim defaults
  to false unless an admin adds a custom property mapping.
- GitHub: GitHubProfile does not expose the claim. We GET /user/emails
  with the OAuth access_token and read `verified` on the entry matching
  the primary email. Failure to fetch (rate limit, network) is treated
  as unverified.

What's intentionally not in this commit:
- Vitest tests for the callback logic. apps/web has no vitest config
  yet (the test foundation only wired up the packages). Filed a
  follow-up: Task-bootstrap-vitest-for-apps-web.md (P2) under
  Epic-test-foundation. The auth-callback assertions will land against
  that harness when it's stood up.
- Race-condition transaction isolation. The current sequence (account
  lookup -> identity lookup -> account/identity upsert) has the same
  race window the old ensureUserIdByEmail had — two simultaneous OAuth
  sign-ins for a brand-new email could both pass the identity check
  before either INSERT fires. Mitigated in practice by the partial
  unique on email WHERE verified_at IS NOT NULL — postgres will reject
  the second insert — but the loser gets an opaque error. Filed as a
  follow-up if it becomes a real issue.

Task file (plans/.../Task-multi-email-identity.md) updated with the
detailed smoke-test playbook an operator needs to run before the OAuth
path goes to production (sign in fresh, sign in repeat, sign in
cross-provider, sign in cross-user-conflict). admin@tasks.dev signs in
via Credentials so the dev fixtures alone do not exercise this code.

Lint + type-check + test all green (14/14 tests, 0 lint errors, 14
unchanged warnings, 6/6 packages type-check).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-02 10:14:16 -05:00

3.8 KiB
Raw Blame History

kind slug title plan_slug epic_slug status priority tenant_id owner cursor_todo_id updated_at
task bootstrap-vitest-for-apps-web Bootstrap Vitest for the Next.js app (apps/web) with alias + env wiring multitenant-saas-hardening test-foundation draft P2 global unassigned null 2026-06-02

Task summary

Vitest is configured for packages/shared, packages/database, and packages/ai (see Task-bootstrap-vitest-and-ci). It is NOT configured for apps/web. This means callback logic, tRPC procedures, and React hooks in the web app have no unit-test home — only the manual smoke-test path covers them.

This task adds the harness so future work can put real assertions next to the code they verify.

Description

Vitest config

Add apps/web/vitest.config.ts. The Next.js app uses a @/* path alias and a mix of server-only modules (DB clients, NextAuth callbacks) and client modules (React components). The config has to:

  • Resolve @/* to apps/web/* (matches tsconfig.json).
  • Run server-side modules under environment: "node".
  • Run React-component modules under environment: "jsdom" with @testing-library/react available. (Optional in v1 — start with node tests only; React testing is its own incremental step.)
  • Mock next/headers, next/navigation, and next-auth for any test that imports a Next-specific module without booting the full framework.

Reference: Next.js + Vitest docs.

Add test + test:watch scripts to apps/web/package.json and verify pnpm test (turbo) picks them up

Initial tests (high-ROI)

The point of bootstrapping the harness is to immediately backfill the assertions that were deferred from Task-multi-email-identity. Specifically:

  1. resolveOAuthEmailVerified (currently a non-exported helper in apps/web/lib/auth.ts). Either extract to its own module or export it. Test:

    • Google + Authentik branches return profile.email_verified directly.
    • GitHub branch makes the right fetch call and reads verified from the matching entry. Mock globalThis.fetch.
    • Unknown provider returns false.

  2. resolveOAuthUser — harder, hits the DB. Either:

    • Set up a test-DB harness (transaction-per-test pattern with pg-test-transactions or hand-rolled BEGIN/ROLLBACK).
    • OR mock getSql and assert the SQL templates.

    I'd vote for the test-DB harness eventually — mocking SQL strings is brittle — but it's a meaningful chunk of infrastructure. Filed as a follow-up to this task if it bloats.

  3. userOwnsEmail + findUserIdByVerifiedEmail in apps/web/server/lib/identity.ts. Same DB-harness question.

Anti-goals (defer)

  • E2E tests via Playwright. That's a separate plan.
  • Full coverage of every tRPC procedure. Add tests where regressions would hurt; don't manufacture coverage.

Subtasks

  • Add apps/web/vitest.config.ts with alias + env wiring.
  • Add test / test:watch scripts to apps/web/package.json.
  • Decide on test-DB strategy (real Postgres via a _test suffix DB + per-test BEGIN/ROLLBACK is recommended). Document it in AGENTS.md.
  • Backfill the resolveOAuthEmailVerified test.
  • Backfill the userOwnsEmail test against the chosen DB harness.

Owner or assignee

Unassigned

Status

draft

Estimation

M

Acceptance criteria

  • pnpm --filter @tasks/web test runs and passes from a clean clone.
  • pnpm test (turbo, root) includes the apps/web suite.
  • At least one real test per planned target (resolveOAuthEmailVerified, userOwnsEmail).
  • CI gates the merged result.

Links

  • Epic: ./Epic-test-foundation.md
  • Plan: ../Plan-multitenant-saas-hardening.md
  • Unblocks proper test coverage for: Task-multi-email-identity.md, all subsequent Plan-multitenant-saas-hardening work that lives in apps/web.