diff --git a/.env.example b/.env.example index 938c95f..0c5ffa3 100644 --- a/.env.example +++ b/.env.example @@ -13,19 +13,7 @@ FEATURE_FLAGS=emailDeliveryApi=true,strictApiRateLimiting=true,telemetryIngestio NEXT_PUBLIC_FEATURE_FLAGS=integrityDeterrentMode=false # ----------------------------------------------------------------------------- -# Clerk authentication -# ----------------------------------------------------------------------------- - -NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=pk_test_replace_me -CLERK_SECRET_KEY=sk_test_replace_me -CLERK_WEBHOOK_SECRET=whsec_replace_me - -# Set this on the Convex deployment with `npx convex env set CLERK_ISSUER_URL ...`. -# Keep it here only as a setup checklist item. -CLERK_ISSUER_URL=https://your-clerk-issuer.clerk.accounts.dev - -# ----------------------------------------------------------------------------- -# Auth.js (replacing Clerk — see docs/superpowers/plans/2026-09-12-custom-auth.md) +# Auth.js — the only authentication provider # ----------------------------------------------------------------------------- # Signs the short-lived JWT that Convex verifies. Generate a keypair with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75a44e1..6dc55de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,10 +12,8 @@ jobs: runs-on: ubuntu-latest env: NEXT_PUBLIC_CONVEX_URL: https://example.convex.cloud - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: pk_test_placeholder NEXT_PUBLIC_STREAM_API_KEY: stream_test_placeholder STREAM_SECRET_KEY: stream_secret_placeholder - CLERK_WEBHOOK_SECRET: webhook_secret_placeholder SMTP_HOST: smtp.example.com SMTP_PORT: "587" SMTP_USER: smtp-user @@ -41,5 +39,6 @@ jobs: - name: Unit tests run: npm run test - # Production build is validated in deploy.yml with real secrets; - # running it here with placeholder keys fails Clerk's prerender validation. + # Production build is validated in deploy.yml with real secrets. It is not + # repeated here: prerendering reaches code that reads AUTH_* at request + # time, and placeholder values make it fail for reasons CI cannot act on. diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 5b47653..c0e3eba 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -88,7 +88,6 @@ jobs: ghcr.io/${{ github.repository }}:latest ghcr.io/${{ github.repository }}:${{ github.sha }} build-args: | - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=${{ secrets.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY }} NEXT_PUBLIC_CONVEX_URL=${{ secrets.NEXT_PUBLIC_CONVEX_URL }} NEXT_PUBLIC_STREAM_API_KEY=${{ secrets.NEXT_PUBLIC_STREAM_API_KEY }} NEXT_PUBLIC_APP_URL=${{ secrets.NEXT_PUBLIC_APP_URL }} diff --git a/.gitignore b/.gitignore index e01e7f1..0cb66ae 100644 --- a/.gitignore +++ b/.gitignore @@ -33,15 +33,11 @@ yarn-error.log* # vercel .vercel -.clerk # typescript *.tsbuildinfo next-env.d.ts -# clerk configuration (can include secrets) -/.clerk/ - # convex local state .convex/ diff --git a/CLAUDE.md b/CLAUDE.md index 9bf7c9e..7cda30c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,8 +92,8 @@ This is the most common source of silent bugs. | | | |---|---| | `users._id` | Convex document id. What an Auth.js session subject carries. | -| `users.clerkId` | Historical app id. Referenced by `interviewerIds`, `candidateId`, `auditLogs.actorClerkId`. For Auth.js-created users it equals `_id`. | -| `users.streamUserId` | The id **Stream** knows them by — always the original Clerk id. | +| `users.clerkId` | The **internal user id**, despite the name. Referenced as a plain `v.string()` by `interviewerIds`, `candidateId`, `auditLogs.actorClerkId`. For Auth.js-created users it equals `_id`; for the accounts that predate the migration it is still their original Clerk id. | +| `users.streamUserId` | The id **Stream** knows them by — whatever it was when their first call was created. | Anything talking to Stream must go through `resolveStreamUserId` (`src/lib/auth/streamIdentity.ts`). Passing the session id instead does not @@ -101,9 +101,14 @@ error: it mints a valid token for a user Stream has never seen, and their calls and recordings are simply absent. `convex/lib/subjectResolution.ts` is the one place that turns a token subject -into a user row, and it tries the document id, then `by_clerk_id`, then -`by_legacy_clerk_id`, because a token may come from either provider at any point -in the migration. +into a user row. It is a single `db.get` on the document id: Auth.js is the only +provider in `convex/auth.config.ts`, so that is the only thing a subject can be. +The `by_clerk_id` / `by_legacy_clerk_id` fallbacks went with Clerk — no token in +existence carries a Clerk id, so those reads could only ever miss. + +`clerkId` is deliberately **not** dropped. It stopped being an authentication +identifier when Clerk was removed, but it is still the id the rest of the +database references, so renaming it is a data migration of its own. ### `convex/lib/*` exists so Convex logic can be tested @@ -217,5 +222,4 @@ usually in a hurry. behind them. - `docs/superpowers/plans/2026-09-12-custom-auth.md` — the Clerk→Auth.js migration, including a list of where the plan turned out to be wrong. -- `README.md` — local setup and Docker runtime images. Its auth section still - describes Clerk and is out of date. +- `README.md` — local setup and Docker runtime images. diff --git a/Dockerfile b/Dockerfile index 8de1ad4..228047d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,13 +12,11 @@ COPY --from=deps /app/node_modules ./node_modules COPY . . # NEXT_PUBLIC_* vars are embedded into the client bundle at build time -ARG NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ARG NEXT_PUBLIC_CONVEX_URL ARG NEXT_PUBLIC_STREAM_API_KEY ARG NEXT_PUBLIC_APP_URL -ENV NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=$NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY \ - NEXT_PUBLIC_CONVEX_URL=$NEXT_PUBLIC_CONVEX_URL \ +ENV NEXT_PUBLIC_CONVEX_URL=$NEXT_PUBLIC_CONVEX_URL \ NEXT_PUBLIC_STREAM_API_KEY=$NEXT_PUBLIC_STREAM_API_KEY \ NEXT_PUBLIC_APP_URL=$NEXT_PUBLIC_APP_URL \ NEXT_TELEMETRY_DISABLED=1 diff --git a/README.md b/README.md index b2d0764..cc250ca 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Commit 💻 -Commit is a modern, real-time technical interviewing platform built with [Next.js](https://nextjs.org), [Convex](https://convex.dev/), [Clerk](https://clerk.com/), and [Stream](https://getstream.io/). +Commit is a modern, real-time technical interviewing platform built with [Next.js](https://nextjs.org), [Convex](https://convex.dev/), [Auth.js](https://authjs.dev/), and [Stream](https://getstream.io/). It offers real-time video, collaborative code execution, structured feedback scorecards, and scheduling tools to make technical interviewing seamless and professional. @@ -10,7 +10,7 @@ It offers real-time video, collaborative code execution, structured feedback sco - **Real-Time Video Intervews:** Powered by Stream with customizable rooms, host controls, and health metrics. - **Live Collaborative Code Editor:** Secure code execution environment for Python, JavaScript, and Java using Monaco Editor and Docker. -- **Identity & Roles:** Secure authentication via Clerk with a robust Hybrid RBAC (Role-Based Access Control) system. +- **Identity & Roles:** Self-hosted authentication via Auth.js — Google, GitHub, email magic links and passwords — with a Hybrid RBAC (Role-Based Access Control) system. - **Interactive Dashboards:** Comprehensive pipelines, schedules, and analytics powered by Convex's reactive datastore. - **Structured Feedback Scorecards:** Blind-grading, weighted scoring, and internal candidate packet drafting. - **Automated Notifications:** Email and in-app notifications with timezone-awareness and retry support. @@ -68,17 +68,32 @@ Ensure you have the following installed on your local machine: ### 2. Set Up Environment Variables -Create a `.env.local` file in the root of the project. Your environment variables should include keys for Clerk, Convex, Stream, and SMTP (optional for local dev). +Copy `.env.example` to `.env.local` — it is the authoritative list, with a note +on every variable and how to generate the ones that need generating. The +essentials: ```env # Convex NEXT_PUBLIC_CONVEX_URL=your_convex_url -# Clerk -NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY=your_clerk_pub_key -CLERK_SECRET_KEY=your_clerk_secret_key -# Required for user syncing across Convex -CLERK_WEBHOOK_SECRET=your_clerk_webhook_secret +# Auth.js — see .env.example for the full set and how to generate each one +AUTH_SECRET=openssl rand -base64 32 +AUTH_URL=http://localhost:3000 +AUTH_ADAPTER_SECRET=a long random string, also set on the Convex deployment +AUTH_JWT_KID=k1 +AUTH_JWT_PRIVATE_KEY="-----BEGIN PRIVATE KEY----- +... +-----END PRIVATE KEY----- +" +AUTH_JWT_PUBLIC_KEY="-----BEGIN PUBLIC KEY----- +... +-----END PUBLIC KEY----- +" +# OAuth providers are optional locally; email + password works without them. +AUTH_GOOGLE_ID=... +AUTH_GOOGLE_SECRET=... +AUTH_GITHUB_ID=... +AUTH_GITHUB_SECRET=... # Stream (Video & Chat) NEXT_PUBLIC_STREAM_API_KEY=your_stream_api_key @@ -225,16 +240,34 @@ access — but if you would rather not have a second container holding it, delet the service and add `0 4 * * * docker image prune -f` to the host crontab instead. Set `IMAGE_GC_INTERVAL_SECONDS` to change the cadence. -### Clerk + Convex Auth +### Auth.js + Convex -Convex validates Clerk JWTs against the issuer configured in `convex/auth.config.ts`. Set this on the Convex deployment itself, not only in `.env.local` or your Docker/Portainer environment: +The Next server mints a short-lived RS256 JWT for the signed-in user. Convex +verifies it against the public half, which the app serves at +`/.well-known/jwks.json`; `convex/auth.config.ts` registers exactly one +`customJwt` provider pointing there. + +Two variables must be set **on the Convex deployment itself**, not only in +`.env.local` or your Docker/Portainer environment: ```bash -npx convex env set CLERK_ISSUER_URL https://your-clerk-issuer +npx convex env set SITE_URL https://your-public-origin +npx convex env set AUTH_ADAPTER_SECRET the-same-value-the-app-has npx convex deploy ``` -Use the issuer from the Clerk JWT template used for Convex, and keep the JWT template audience/application ID as `convex`. If this value points at a development Clerk instance while the deployed frontend uses production Clerk keys, Convex will reject browser tokens with `No auth provider found matching the given token`. +`SITE_URL` is what Convex derives the issuer and JWKS URL from, so it has to be +the origin the browser actually reaches — if it points somewhere else, Convex +rejects every token with `No auth provider found matching the given token`. +`AUTH_ADAPTER_SECRET` guards `convex/authAdapter.ts`, which Auth.js calls to +create and read users; it must be identical on both sides or every sign-in +fails. + +`AUTH_URL` deserves its own warning. Auth.js only trusts the request host when +`AUTH_URL`, `AUTH_TRUST_HOST`, `VERCEL` or `CF_PAGES` is set — and it uses `??`, +so setting `AUTH_URL` to the **empty string** stops that chain at the first term +and yields `false`, at which point every auth route answers 500. An empty value +is strictly worse than an absent one. `/api/health` checks for exactly this. --- @@ -279,21 +312,26 @@ so they are *not* covered by the app container's limit and get their own caps in | monitoring profile | ~1.1 | ~2 GB | The practical ceilings are external before they are local: Stream -participant-minutes first, then Convex function calls and bandwidth, then Clerk -MAU. +participant-minutes first, then Convex function calls and bandwidth. Auth is +self-hosted, so it has no per-user ceiling of its own beyond the database. --- ## ✅ Before opening public signup -- [ ] Rotate `STREAM_SECRET_KEY` and `CLERK_WEBHOOK_SECRET`. An earlier version - of `convex/observability.ts` logged both to Convex function logs on every - developer-dashboard load, so treat the old values as compromised. Update - them in the Portainer stack env and in GitHub Actions secrets. -- [ ] Use a **production** Clerk instance, with the domain, redirect URLs and - webhook endpoint pointing at the public origin, and **email verification - required at signup** — the code runner refuses unverified accounts. -- [ ] Confirm `CLERK_ISSUER_URL` on the Convex deployment matches that instance. +- [ ] Rotate `STREAM_SECRET_KEY`. An earlier version of + `convex/observability.ts` logged it to Convex function logs on every + developer-dashboard load, so treat the old value as compromised. Update it + in the Portainer stack env and in GitHub Actions secrets. +- [ ] Rotate the OAuth client secrets, `AUTH_SECRET`, `AUTH_ADAPTER_SECRET` and + `INTERNAL_API_KEY`, and bump `AUTH_JWT_KID` when you replace the keypair — + the kid names the key in both halves, so changing the key without changing + the kid leaves cached JWKS entries rejecting valid tokens. +- [ ] Point the Google and GitHub OAuth apps' authorised redirect URIs at the + public origin: `https:///api/auth/callback/{google,github}`. +- [ ] Confirm `SITE_URL` and `AUTH_ADAPTER_SECRET` on the Convex deployment match + the app, and that `/api/health` reports `auth: true` — email verification + is required before the code runner will run anything. - [ ] Set `NEXT_PUBLIC_APP_URL` to the public origin so invitation links resolve. - [ ] Restore-test one backup zip from the `backup-data` volume. - [ ] Set `OWNER_EMAILS` on the Convex deployment to your own address, then sign diff --git a/convex/auth.config.ts b/convex/auth.config.ts index 8fcbc00..5ac7fc2 100644 --- a/convex/auth.config.ts +++ b/convex/auth.config.ts @@ -1,49 +1,40 @@ /** * Identity providers Convex will accept tokens from. * - * Both are listed on purpose during the Auth.js migration. Convex uses the - * first provider whose issuer and applicationID match the presented token, and - * Clerk's tokens carry Clerk's issuer while ours carry SITE_URL, so the two - * never collide. Keeping both means a half-migrated deployment authenticates - * users on either system instead of locking everyone out at the moment the - * config changes. The Clerk entry is removed in the final task of the migration - * (see docs/superpowers/plans/2026-09-12-custom-auth.md). + * One provider now. Clerk's entry was removed with the rest of Clerk, so a + * token from it is no longer accepted by this deployment — which is the point + * of removing it rather than leaving it configured and unused. + * + * `SITE_URL` is required, and Convex enforces that statically: any environment + * variable named in this file must be set on the deployment or `convex deploy` + * fails, whether or not the code path that reads it runs. Set it with + * `npx convex env set SITE_URL https://commit.kunjdeveloper.com`. */ -const clerkIssuerUrl = process.env.CLERK_ISSUER_URL; - // Trailing slashes are stripped because `issuer` must match the token's `iss` // claim exactly, and an operator pasting the URL with one would produce a -// mismatch whose only symptom is that every token is rejected. +// mismatch whose only symptom is that every token is rejected — which surfaces +// as "you must be signed in" shown to people who are. const siteUrl = process.env.SITE_URL?.trim().replace(/\/+$/, ""); -if (!clerkIssuerUrl && !siteUrl) { +if (!siteUrl) { throw new Error( - "No auth provider configured. Set SITE_URL (Auth.js) and/or CLERK_ISSUER_URL " + - "in the Convex environment, for example with " + + "SITE_URL is not set on this Convex deployment, so no auth provider is " + + "configured and every request would be unauthenticated. Set it with " + "`npx convex env set SITE_URL https://commit.kunjdeveloper.com`.", ); } -const providers = []; - -if (siteUrl) { - providers.push({ - // customJwt rather than OIDC: it needs only a JWKS endpoint, where OIDC - // would additionally require us to serve /.well-known/openid-configuration. - type: "customJwt", - applicationID: "convex", - issuer: siteUrl, - jwks: `${siteUrl}/.well-known/jwks.json`, - algorithm: "RS256", - }); -} - -if (clerkIssuerUrl) { - providers.push({ - domain: clerkIssuerUrl, - applicationID: "convex", - }); -} - -export default { providers }; +export default { + providers: [ + { + // customJwt rather than OIDC: it needs only a JWKS endpoint, where OIDC + // would additionally require us to serve /.well-known/openid-configuration. + type: "customJwt", + applicationID: "convex", + issuer: siteUrl, + jwks: `${siteUrl}/.well-known/jwks.json`, + algorithm: "RS256", + }, + ], +}; diff --git a/convex/authAdapter.ts b/convex/authAdapter.ts index d9ac77a..9c6922d 100644 --- a/convex/authAdapter.ts +++ b/convex/authAdapter.ts @@ -67,11 +67,14 @@ export const createUser = mutation({ // Public sign-up produces candidates. Elevating anyone beyond that stays // an explicit, audited action, exactly as it was under Clerk. role: "candidate", - // clerkId is still required by the schema and read in 137 places across - // convex/. Rather than churn all of them mid-migration, a user created by - // Auth.js becomes its own id here — which is what the field always meant - // semantically, and what identity.subject now carries. It is dropped in - // the final task of the migration. + // clerkId is no longer an authentication identifier — nothing verifies a + // token against it any more. It stays because it is the *internal user id* + // that interviews.candidateId, interviews.interviewerIds and + // auditLogs.actorClerkId hold, as plain v.string(), in hundreds of places. + // Renaming it is a data migration of its own, not part of removing Clerk. + // + // A user created by Auth.js becomes its own id here, which is what the + // field always meant semantically and what identity.subject now carries. // // A Convex id does not exist until the row does, so this is written twice: // a placeholder that cannot collide, then the real id. The placeholder is @@ -343,8 +346,8 @@ export const setCredential = mutation({ * * `source` is "server" rather than "auth": the schema's union does not include * an auth source, and widening it for this would be a migration for no gain. - * `provider: "authjs"` is what distinguishes these rows, mirroring the - * `provider: "clerk"` the webhook path already writes. + * `provider: "authjs"` is what distinguishes these rows from every other + * telemetry row sharing that source. */ export const recordAuthEvent = mutation({ args: { diff --git a/convex/http.ts b/convex/http.ts index fd8aeee..ece3c39 100644 --- a/convex/http.ts +++ b/convex/http.ts @@ -1,190 +1,10 @@ import { httpRouter } from "convex/server"; import { httpAction } from "./_generated/server"; import { internal } from "./_generated/api"; -import { WebhookEvent } from "@clerk/nextjs/server"; -import { Webhook } from "svix"; import { createServerError, logServerError } from "./lib/errorUtils"; const http = httpRouter(); -http.route({ - path: "/clerk-webhook", - method: "POST", - handler: httpAction(async (ctx, req) => { - const correlationId = req.headers.get("x-correlation-id") ?? crypto.randomUUID(); - const webhookSecret = process.env.CLERK_WEBHOOK_SECRET; - if (!webhookSecret) { - logServerError( - "clerk-webhook.config", - new Error("Webhook secret not configured"), - ); - await ctx.runMutation(internal.observability.recordOperationalEvent, { - source: "webhook", - scope: "clerk-webhook.config", - level: "critical", - message: "Clerk webhook secret not configured.", - correlationId, - provider: "clerk", - status: "misconfigured", - }); - return new Response("Webhook unavailable", { status: 503 }); - } - - const svix_id = req.headers.get("svix-id"); - const svix_timestamp = req.headers.get("svix-timestamp"); - const svix_signature = req.headers.get("svix-signature"); - - if (!svix_id || !svix_timestamp || !svix_signature) { - logServerError( - "clerk-webhook.headers", - new Error("Missing Svix headers"), - ); - await ctx.runMutation(internal.observability.recordOperationalEvent, { - source: "webhook", - scope: "clerk-webhook.headers", - level: "warn", - message: "Missing Svix headers.", - correlationId, - provider: "clerk", - status: "rejected", - }); - return new Response("Invalid webhook request", { status: 400 }); - } - - const body = await req.text(); - - const wh = new Webhook(webhookSecret); - - let event: WebhookEvent; - try { - event = wh.verify(body, { - svix_id: svix_id, - svix_timestamp: svix_timestamp, - svix_signature: svix_signature, - }) as WebhookEvent; - } catch (err) { - logServerError("clerk-webhook.verify", err); - await ctx.runMutation(internal.observability.recordOperationalEvent, { - source: "webhook", - scope: "clerk-webhook.verify", - level: "error", - message: "Webhook signature verification failed.", - correlationId, - provider: "clerk", - status: "rejected", - metadata: JSON.stringify({ svixId: svix_id }), - }); - return new Response("Invalid signature", { status: 400 }); - } - - const eventType = event.type; - const existingEvent = await ctx.runQuery( - internal.reliability.getWebhookEventByProviderEventId, - { - provider: "clerk", - eventId: svix_id, - }, - ); - - if (existingEvent?.status === "processed" || existingEvent?.status === "duplicate") { - await ctx.runMutation(internal.reliability.recordWebhookReceipt, { - provider: "clerk", - eventId: svix_id, - eventType, - payload: body, - correlationId, - }); - - return new Response("Webhook already processed", { status: 200 }); - } - - await ctx.runMutation(internal.reliability.recordWebhookReceipt, { - provider: "clerk", - eventId: svix_id, - eventType, - payload: body, - correlationId, - }); - - try { - if (eventType === "user.created" || eventType === "user.updated") { - const { id, email_addresses, first_name, last_name, image_url } = - event.data; - const email = email_addresses[0].email_address; - const name = `${first_name || ""} ${last_name || ""}`.trim(); - - await ctx.runMutation(internal.users.syncUserFromWebhook, { - clerkId: id, - email, - name, - image: image_url || undefined, - }); - - await ctx.runMutation(internal.auditLogs.recordSystemAuditLog, { - action: `clerk.${eventType}`, - actorClerkId: id, - actorEmail: email, - targetType: "user", - targetId: id, - metadata: JSON.stringify({ eventType }), - }); - } - - if (eventType === "session.created" || eventType === "session.ended") { - const sessionData = event.data as { user_id?: string; id?: string }; - - await ctx.runMutation(internal.auditLogs.recordSystemAuditLog, { - action: `clerk.${eventType}`, - actorClerkId: sessionData.user_id, - targetType: "session", - targetId: sessionData.id, - metadata: JSON.stringify({ - eventType, - userId: sessionData.user_id, - }), - }); - } - await ctx.runMutation(internal.reliability.markWebhookProcessed, { - provider: "clerk", - eventId: svix_id, - }); - await ctx.runMutation(internal.observability.recordOperationalEvent, { - source: "webhook", - scope: "clerk-webhook.processed", - level: "info", - message: "Webhook processed successfully.", - correlationId, - provider: "clerk", - status: eventType, - }); - return new Response("Webhook processed successfully", { status: 200 }); - } catch (error) { - logServerError("clerk-webhook.process", error, { - eventType, - svixId: svix_id, - }); - await ctx.runMutation(internal.reliability.markWebhookFailed, { - provider: "clerk", - eventId: svix_id, - errorMessage: - error instanceof Error ? error.message : "Unknown webhook processing error.", - payload: body, - }); - await ctx.runMutation(internal.observability.recordOperationalEvent, { - source: "webhook", - scope: "clerk-webhook.process", - level: "error", - message: "Webhook processing failed and was queued for recovery.", - correlationId, - provider: "clerk", - status: eventType, - metadata: JSON.stringify({ svixId: svix_id }), - }); - throw createServerError(error, "Unable to process the webhook payload."); - } - }), -}); - http.route({ path: "/internal/backup-record", method: "POST", diff --git a/convex/lib/subjectResolution.ts b/convex/lib/subjectResolution.ts index 2a19eae..c4b120e 100644 --- a/convex/lib/subjectResolution.ts +++ b/convex/lib/subjectResolution.ts @@ -1,6 +1,5 @@ /** - * Finding the signed-in user from `identity.subject`, while two auth providers - * are registered at once. + * Finding the signed-in user from `identity.subject`. * * Kept import-free and in convex/lib for the same reason as ./retention.ts and * ./owner.ts: a test can then import it without pulling in the Convex server @@ -8,63 +7,28 @@ * not throw — it silently reports that a signed-in user has no account, and * tells them to sign out and try again, which does not help. * - * During the Clerk migration `subject` means one of two different things, since - * convex/auth.config.ts registers both providers and either can have minted the - * token on any given request: + * `subject` is now always the Convex document id of the user. Auth.js is the + * only provider registered in convex/auth.config.ts, and it takes the subject + * from the id the adapter returned, so there is exactly one thing it can be. * - * - Auth.js — the Convex document id of the user. - * - Clerk — the Clerk user id, stored in `users.clerkId`. + * This used to try three lookups. While Clerk was also registered, a token + * could arrive carrying a Clerk user id instead, and resolving it meant falling + * back to `by_clerk_id` and then `by_legacy_clerk_id`. Both fallbacks went with + * Clerk: no token in existence carries a Clerk id any more, so those reads + * could only ever miss. * - * Three cases have to resolve, and only the first two are obvious: - * - * 1. A user Auth.js created. convex/authAdapter.ts sets their `clerkId` to - * their own document id, so either lookup finds them. - * 2. A legacy user presenting a Clerk token, before the backfill. Subject is - * the Clerk id, so `by_clerk_id` finds them and the id lookup cannot. - * 3. A legacy user presenting an Auth.js token — the case that appears the - * moment anyone migrates, and the reason this is not simply the old query. - * Subject is their document id while `clerkId` is still `user_2...`, so - * only the id lookup finds them. - * 4. A Clerk subject that is only present as `legacyClerkId`. - * - * Case 4 is defence rather than a live requirement, and the distinction is - * worth stating plainly because an earlier version of this comment got it - * wrong. The backfill (Task 14) copies `clerkId` into `legacyClerkId` and - * `streamUserId`; it does *not* rewrite `clerkId`, precisely because - * `interviewerIds`, `candidateId` and `auditLogs.actorClerkId` all reference - * that value across the database. So after the backfill, case 2 still resolves - * through `by_clerk_id` on its own. - * - * The third lookup earns its place at the other end of the migration: Task 16 - * makes `clerkId` optional and then drops it, at which point `legacyClerkId` is - * the only remaining record of a Clerk id. It costs one indexed read on a path - * that has already missed twice, and it means the order of those two steps - * cannot strand anyone. - * - * Both fallbacks are removed with the `clerkId` column in the final task of the - * migration. + * `users.clerkId` still exists and is still populated — it is the id that + * `interviewerIds`, `candidateId` and `auditLogs.actorClerkId` reference + * throughout the database. It is no longer an *authentication* identifier, and + * nothing here should read it again. */ -/** - * The slice of a Convex ctx this needs, narrow enough for a test to supply. - * - * Generic over the user row so the Convex call sites keep inferring whatever - * they inferred before — authz.ts passes `ctx: any` and reads `user.role` off - * the result, which a hardcoded `unknown` here would break at every call site. - */ +/** The slice of a Convex ctx this needs, narrow enough for a test to supply. */ export type SubjectResolutionCtx = { db: { /** Returns null for a string that is not an id for the table. */ normalizeId: (table: "users", id: string) => string | null; get: (id: string) => Promise; - query: (table: "users") => { - withIndex: ( - index: "by_clerk_id" | "by_legacy_clerk_id", - builder: (q: { - eq: (field: "clerkId" | "legacyClerkId", value: string) => unknown; - }) => unknown, - ) => { first: () => Promise }; - }; }; }; @@ -84,42 +48,17 @@ export const resolveUserBySubject = async ( ctx: any, subject: string, ): Promise => { - // An empty subject would otherwise reach the index and match any row whose - // clerkId was somehow empty. Nothing should produce one, which is exactly why - // it must not resolve to a user if something does. if (!subject) return null; /** - * `normalizeId` is what makes trying the id first safe: it returns null for a - * string that is not an id for this table, where `db.get` would throw. A - * Clerk subject simply falls through to the index below. + * `normalizeId` rather than passing the string straight to `db.get`, which + * throws on anything that is not a well-formed id for this table. A malformed + * subject should resolve to "no such user" rather than to a 500 — the caller + * turns the former into a sign-in prompt and the latter into an error page. */ const documentId = ctx.db.normalizeId("users", subject); - if (documentId) { - const user = await ctx.db.get(documentId); - // A well-formed id for a row that no longer exists still falls through: a - // deleted-and-recreated account should be found by its clerkId rather than - // reported as missing. - if (user) return user; - } - - const byClerkId = await ctx.db - .query("users") - .withIndex("by_clerk_id", (q: { eq: (field: "clerkId", value: string) => unknown }) => - q.eq("clerkId", subject), - ) - .first(); - - if (byClerkId) return byClerkId; + if (!documentId) return null; - // Case 4: a Clerk token after the backfill has moved the Clerk id here. - return await ctx.db - .query("users") - .withIndex( - "by_legacy_clerk_id", - (q: { eq: (field: "legacyClerkId", value: string) => unknown }) => - q.eq("legacyClerkId", subject), - ) - .first(); + return await ctx.db.get(documentId); }; diff --git a/convex/observability.ts b/convex/observability.ts index 5fbb883..f70eca0 100644 --- a/convex/observability.ts +++ b/convex/observability.ts @@ -109,13 +109,21 @@ export const captureHealthSnapshot = mutation({ // dashboard mount, and Convex retains function logs. Only presence is checked. const envChecks = [ { - provider: "clerk", - status: process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY - ? "healthy" - : "unhealthy", - message: process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY - ? "Clerk publishable key configured." - : "Missing Clerk publishable key.", + // The Convex half of authentication. SITE_URL is what auth.config.ts + // registers as the token issuer, and AUTH_ADAPTER_SECRET is what guards + // the adapter mutations Auth.js calls to create users and link + // accounts. Either one missing means nobody can sign in, and the + // symptom appears in the Next app rather than here. + provider: "auth", + status: + process.env.SITE_URL && process.env.AUTH_ADAPTER_SECRET + ? "healthy" + : "unhealthy", + message: !process.env.SITE_URL + ? "Missing SITE_URL, so no token issuer is configured." + : !process.env.AUTH_ADAPTER_SECRET + ? "Missing AUTH_ADAPTER_SECRET, so every adapter call is rejected." + : "Auth issuer and adapter secret configured.", }, { provider: "convex", @@ -137,13 +145,6 @@ export const captureHealthSnapshot = mutation({ ? "Stream video credentials configured." : "Missing Stream credentials.", }, - { - provider: "webhooks", - status: process.env.CLERK_WEBHOOK_SECRET ? "healthy" : "degraded", - message: process.env.CLERK_WEBHOOK_SECRET - ? "Webhook secret configured." - : "Webhook secret missing. Clerk sync will fail.", - }, { provider: "ownership", // Only the count, never the addresses — these land in a table that any diff --git a/convex/users.ts b/convex/users.ts index aaa277f..74f45e1 100644 --- a/convex/users.ts +++ b/convex/users.ts @@ -1,5 +1,5 @@ import { v } from "convex/values"; -import { internalMutation, mutation, query } from "./_generated/server"; +import { mutation, query } from "./_generated/server"; import { PERMISSION_VALUES, Permission, @@ -115,137 +115,6 @@ const sanitizeUserForViewer = < }; }; -type SyncUserArgs = { - clerkId: string; - email: string; - name: string; - image?: string; -}; - -/** - * Shared upsert used by both the Clerk webhook (trusted, no identity) and the - * signed-in client hook. Callers are responsible for authorizing `args.clerkId` - * before calling — this helper does no auth of its own. - * - * `role` is never taken from args: a new row is always `candidate` and an existing - * row keeps whatever role it already has, so sync can never escalate. - */ -const applySyncUser = async (ctx: any, args: SyncUserArgs) => { - const normalizedEmail = normalizeEmail(args.email); - - const existingUser = await ctx.db - .query("users") - .withIndex("by_clerk_id", (q: any) => q.eq("clerkId", args.clerkId)) - .first(); - - // Enforced on both create and update. Previously update skipped this, which let a - // row's email be repointed at an address someone else owned — and invitations are - // authorized by email match, so that was an invitation-hijack path. - if (normalizedEmail) { - const emailOwner = await ctx.db - .query("users") - .withIndex("by_email", (q: any) => q.eq("email", normalizedEmail)) - .first(); - - if (emailOwner && emailOwner.clerkId !== args.clerkId) { - throw createServerError( - new Error( - `Email ${normalizedEmail} already owned by ${emailOwner.clerkId}; ` + - `tried to sync ${args.clerkId}`, - ), - "An account with this email already exists. Please sign in with the provider you used the first time.", - ); - } - } - - if (normalizedEmail) { - await expireStalePendingInvitations(ctx, normalizedEmail); - } - - const nextRole = existingUser?.role ?? "candidate"; - - if (existingUser) { - await ctx.db.patch(existingUser._id, { - ...args, - email: normalizedEmail, - role: nextRole, - }); - - return existingUser._id; - } - - const userId = await ctx.db.insert("users", { - ...args, - email: normalizedEmail, - role: nextRole, - }); - - await logAuditEvent(ctx, { - action: "user.created", - actorClerkId: args.clerkId, - actorEmail: normalizedEmail, - targetType: "user", - targetId: userId, - metadata: { - role: nextRole, - }, - }); - - return userId; -}; - -/** - * Webhook entry point. Internal-only: Clerk httpActions carry no user identity, so - * this cannot go through the authenticated mutation below. - */ -export const syncUserFromWebhook = internalMutation({ - args: { - clerkId: v.string(), - email: v.string(), - name: v.string(), - image: v.optional(v.string()), - }, - handler: async (ctx, args) => applySyncUser(ctx, args), -}); - -/** - * Client entry point, called by useSyncUser on auth state change. The Clerk id comes - * from the verified token, never from the argument list. - */ -export const syncUser = mutation({ - args: { - email: v.string(), - name: v.string(), - image: v.optional(v.string()), - /** - * Accepted and discarded — a rollout compatibility shim, not an input. - * - * Convex arg validators are strict, and the image currently running on the - * VM still calls this with a `clerkId`. Without this field, deploying these - * functions would reject every sync from the live frontend, and because - * useUserRole waits on sync before it will query anything, the whole - * signed-in UI would sit blank until the new image was pulled. Declaring it - * optional decouples the Convex deploy from the image rollout. - * - * The value is never read: the trusted id comes from the verified token - * below, so a client still cannot sync a row it does not own. - * - * Safe to delete once the new image is live everywhere. - */ - clerkId: v.optional(v.string()), - }, - handler: async (ctx, args) => { - const identity = await requireIdentity(ctx); - - return applySyncUser(ctx, { - email: args.email, - name: args.name, - image: args.image, - clerkId: identity.subject, - }); - }, -}); - /** * Marks the first-run welcome as seen. Self-scoped: the row is resolved from the * caller's identity, so there is no user id to tamper with. diff --git a/docker-compose.yml b/docker-compose.yml index 712ca7b..b79f74b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,11 +14,6 @@ services: environment: NODE_ENV: production - # Clerk — authentication - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: ${NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY} - CLERK_SECRET_KEY: ${CLERK_SECRET_KEY} - CLERK_WEBHOOK_SECRET: ${CLERK_WEBHOOK_SECRET} - # Convex — backend / database NEXT_PUBLIC_CONVEX_URL: ${NEXT_PUBLIC_CONVEX_URL} CONVEX_DEPLOYMENT: ${CONVEX_DEPLOYMENT} diff --git a/package-lock.json b/package-lock.json index 17eae12..02a7e34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,7 +8,6 @@ "name": "commit", "version": "0.1.0", "dependencies": { - "@clerk/nextjs": "^6.0.0", "@hookform/resolvers": "^5.2.2", "@monaco-editor/react": "^4.7.0", "@node-rs/argon2": "^2.2.1", @@ -33,7 +32,6 @@ "react-player": "^3.4.0", "react-resizable-panels": "^4.10.0", "sonner": "^2.0.7", - "svix": "^1.90.0", "tailwind-merge": "^3.5.0", "tailwindcss-animate": "^1.0.7", "zod": "^4.3.6" @@ -566,102 +564,6 @@ "node": ">=6.9.0" } }, - "node_modules/@clerk/backend": { - "version": "2.33.3", - "resolved": "https://registry.npmjs.org/@clerk/backend/-/backend-2.33.3.tgz", - "integrity": "sha512-cgkFVEYFG2nZn4QDuYBhiAwPtMdo8Yj7DAtq/SBQ5C/ainh3uxNRDgUj4bFn52qJkWLiCkraYJIw1b8dEUbUBg==", - "license": "MIT", - "dependencies": { - "@clerk/shared": "^3.47.5", - "@clerk/types": "^4.101.23", - "standardwebhooks": "^1.0.0", - "tslib": "2.8.1" - }, - "engines": { - "node": ">=18.17.0" - } - }, - "node_modules/@clerk/clerk-react": { - "version": "5.61.6", - "resolved": "https://registry.npmjs.org/@clerk/clerk-react/-/clerk-react-5.61.6.tgz", - "integrity": "sha512-OiyBlrnkRr9IhZtPd7EwlzhYScBpvNKJ8lgg7Uw6JElzJYz854IeQaez5mAfpiib3LcW/Dn53E2PQhagcuLJ3Q==", - "license": "MIT", - "dependencies": { - "@clerk/shared": "^3.47.5", - "tslib": "2.8.1" - }, - "engines": { - "node": ">=18.17.0" - }, - "peerDependencies": { - "react": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0", - "react-dom": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0" - } - }, - "node_modules/@clerk/nextjs": { - "version": "6.39.3", - "resolved": "https://registry.npmjs.org/@clerk/nextjs/-/nextjs-6.39.3.tgz", - "integrity": "sha512-a64lJ1IlV1uA7eEe8DOx+v2bkNOhnTsNlB5THP/xkHvynHqZhc74Yt05sm1vTniWwhJpJspAZ95pCWUX/RVZ2Q==", - "license": "MIT", - "dependencies": { - "@clerk/backend": "^2.33.3", - "@clerk/clerk-react": "^5.61.6", - "@clerk/shared": "^3.47.5", - "@clerk/types": "^4.101.23", - "server-only": "0.0.1", - "tslib": "2.8.1" - }, - "engines": { - "node": ">=18.17.0" - }, - "peerDependencies": { - "next": "^13.5.7 || ^14.2.25 || ^15.2.3 || ^16", - "react": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0", - "react-dom": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0" - } - }, - "node_modules/@clerk/shared": { - "version": "3.47.5", - "resolved": "https://registry.npmjs.org/@clerk/shared/-/shared-3.47.5.tgz", - "integrity": "sha512-rDVe73/VN2NZXhtrLRHshkUpQDrevAqDRxeXUl2M0IBEBkcl+VMHlV7fep53cVWo0b3gIqLk82pmmi+WoyF/xg==", - "hasInstallScript": true, - "license": "MIT", - "dependencies": { - "csstype": "3.1.3", - "dequal": "2.0.3", - "glob-to-regexp": "0.4.1", - "js-cookie": "3.0.5", - "std-env": "^3.9.0", - "swr": "2.3.4" - }, - "engines": { - "node": ">=18.17.0" - }, - "peerDependencies": { - "react": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0", - "react-dom": "^18.0.0 || ~19.0.3 || ~19.1.4 || ~19.2.3 || ~19.3.0-0" - }, - "peerDependenciesMeta": { - "react": { - "optional": true - }, - "react-dom": { - "optional": true - } - } - }, - "node_modules/@clerk/types": { - "version": "4.101.23", - "resolved": "https://registry.npmjs.org/@clerk/types/-/types-4.101.23.tgz", - "integrity": "sha512-t5ypYYDkT5TPaNIDjLnYk9GpkJgwNTBiS7h6FuUTjoySQtf7amNDS1A1eOu7NOcVpqiSeKg+0wzGxxcre00kMA==", - "license": "MIT", - "dependencies": { - "@clerk/shared": "^3.47.5" - }, - "engines": { - "node": ">=18.17.0" - } - }, "node_modules/@date-fns/tz": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/@date-fns/tz/-/tz-1.4.1.tgz", @@ -4260,12 +4162,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/@stablelib/base64": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/@stablelib/base64/-/base64-1.0.1.tgz", - "integrity": "sha512-1bnPQqSxSuc3Ii6MhBysoWCg58j97aUjuCSZrGSmDxNqtytIi0k8utUenAwTZN4V5mXXYGsVUI9zeBqy+jBOSQ==", - "license": "MIT" - }, "node_modules/@standard-schema/utils": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/@standard-schema/utils/-/utils-0.3.0.tgz", @@ -5817,12 +5713,6 @@ "node": ">=4" } }, - "node_modules/csstype": { - "version": "3.1.3", - "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", - "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", - "license": "MIT" - }, "node_modules/custom-media-element": { "version": "1.4.6", "resolved": "https://registry.npmjs.org/custom-media-element/-/custom-media-element-1.4.6.tgz", @@ -5995,15 +5885,6 @@ "node": ">= 0.8" } }, - "node_modules/dequal": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/dequal/-/dequal-2.0.3.tgz", - "integrity": "sha512-0je+qPKHEMohvfRTCEo3CrPG6cAzAYgmzKyxRiYSSDkS6eGJdyVJm7WaYA5ECaAD9wLB2T4EEeymA5aFVcYXCA==", - "license": "MIT", - "engines": { - "node": ">=6" - } - }, "node_modules/detect-libc": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-2.1.2.tgz", @@ -6449,12 +6330,6 @@ "node": ">=8.6.0" } }, - "node_modules/fast-sha256": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/fast-sha256/-/fast-sha256-1.3.0.tgz", - "integrity": "sha512-n11RGP/lrWEFI/bWdygLxhI+pVeo1ZYIVwvvPkW7azl/rOy+F3HYRZ2K5zeE9mmkhQppyv9sQFx0JM9UabnpPQ==", - "license": "Unlicense" - }, "node_modules/fast-string-truncated-width": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/fast-string-truncated-width/-/fast-string-truncated-width-3.0.3.tgz", @@ -6824,12 +6699,6 @@ "node": ">= 6" } }, - "node_modules/glob-to-regexp": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/glob-to-regexp/-/glob-to-regexp-0.4.1.tgz", - "integrity": "sha512-lkX1HJXwyMcprw/5YUZc2s7DrpAiHB21/V+E1rHUrVNokkvB6bqMzT0VfV6/86ZNabt1k14YOIaT7nDvOX3Iiw==", - "license": "BSD-2-Clause" - }, "node_modules/gopd": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.2.0.tgz", @@ -7383,15 +7252,6 @@ "url": "https://github.com/sponsors/panva" } }, - "node_modules/js-cookie": { - "version": "3.0.5", - "resolved": "https://registry.npmjs.org/js-cookie/-/js-cookie-3.0.5.tgz", - "integrity": "sha512-cEiJEAEoIbWfCZYKWhVwFuvPX1gETRYPw6LlaTKoxD3s2AkXzkCjnp6h0V77ozyqj0jakteJ4YqDJT830+lVGw==", - "license": "MIT", - "engines": { - "node": ">=14" - } - }, "node_modules/js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -9519,12 +9379,6 @@ "url": "https://opencollective.com/express" } }, - "node_modules/server-only": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/server-only/-/server-only-0.0.1.tgz", - "integrity": "sha512-qepMx2JxAa5jjfzxG79yPPq+8BuFToHd1hm7kI+Z4zAq1ftQiP7HcxMhDDItrbtwVeLg/cY2JnKnrcFkmiswNA==", - "license": "MIT" - }, "node_modules/set-cookie-parser": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/set-cookie-parser/-/set-cookie-parser-3.1.0.tgz", @@ -9794,16 +9648,6 @@ "integrity": "sha512-QdKrJPkYCzaNwwz2vN2eDGyoW0KmQFmnwVprB41mpMzj4qujbqr6pegEchQeTn0b5PceKiLoVu0pp2QDpTcWnw==", "license": "MIT" }, - "node_modules/standardwebhooks": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/standardwebhooks/-/standardwebhooks-1.0.0.tgz", - "integrity": "sha512-BbHGOQK9olHPMvQNHWul6MYlrRTAOKn03rOe4A8O3CLWhNf4YHBqq2HJKKC+sfqpxiBY52pNeesD6jIiLDz8jg==", - "license": "MIT", - "dependencies": { - "@stablelib/base64": "^1.0.0", - "fast-sha256": "^1.3.0" - } - }, "node_modules/state-local": { "version": "1.0.7", "resolved": "https://registry.npmjs.org/state-local/-/state-local-1.0.7.tgz", @@ -9820,12 +9664,6 @@ "node": ">= 0.8" } }, - "node_modules/std-env": { - "version": "3.10.0", - "resolved": "https://registry.npmjs.org/std-env/-/std-env-3.10.0.tgz", - "integrity": "sha512-5GS12FdOZNliM5mAOxFRg7Ir0pWz8MdpYm6AY6VPkGpbA7ZzmbzNcBJQ0GPvvyWgcY7QAhCgf9Uy89I03faLkg==", - "license": "MIT" - }, "node_modules/stdin-discarder": { "version": "0.2.2", "resolved": "https://registry.npmjs.org/stdin-discarder/-/stdin-discarder-0.2.2.tgz", @@ -9950,28 +9788,6 @@ "integrity": "sha512-9pP/CVNp4NF2MNlRzLwQkjiTgKKe9WYXrLh9+8QokWmMxz+zt2mf1utkWLco26IuA3AfVcTb//qtlTIjY3VHxA==", "license": "MIT" }, - "node_modules/svix": { - "version": "1.92.2", - "resolved": "https://registry.npmjs.org/svix/-/svix-1.92.2.tgz", - "integrity": "sha512-ZmuA3UVvlnF9EgxlzmPtF7CKjQb64Z6OFlyfdDfU0sdcC7dJa+3aOYX5B9mA+RS6ch1AxBa4UP/l6KmqfGtWBQ==", - "license": "MIT", - "dependencies": { - "standardwebhooks": "1.0.0" - } - }, - "node_modules/swr": { - "version": "2.3.4", - "resolved": "https://registry.npmjs.org/swr/-/swr-2.3.4.tgz", - "integrity": "sha512-bYd2lrhc+VarcpkgWclcUi92wYCpOgMws9Sd1hG1ntAu0NEy+14CbotuFjshBU2kt9rYj9TSmDcybpxpeTU1fg==", - "license": "MIT", - "dependencies": { - "dequal": "^2.0.3", - "use-sync-external-store": "^1.4.0" - }, - "peerDependencies": { - "react": "^16.11.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" - } - }, "node_modules/tabbable": { "version": "6.4.0", "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.4.0.tgz", diff --git a/package.json b/package.json index aa11b98..4150651 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,6 @@ "ci:validate": "npm run typecheck && npm run test && npm run build" }, "dependencies": { - "@clerk/nextjs": "^6.0.0", "@hookform/resolvers": "^5.2.2", "@monaco-editor/react": "^4.7.0", "@node-rs/argon2": "^2.2.1", @@ -40,7 +39,6 @@ "react-player": "^3.4.0", "react-resizable-panels": "^4.10.0", "sonner": "^2.0.7", - "svix": "^1.90.0", "tailwind-merge": "^3.5.0", "tailwindcss-animate": "^1.0.7", "zod": "^4.3.6" diff --git a/src/app/api/health/route.ts b/src/app/api/health/route.ts index 426a4f1..e89d36c 100644 --- a/src/app/api/health/route.ts +++ b/src/app/api/health/route.ts @@ -39,10 +39,8 @@ export async function GET() { version, checkedAt: new Date().toISOString(), integrations: { - clerk: !!env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY, convex: !!env.NEXT_PUBLIC_CONVEX_URL, stream: !!env.NEXT_PUBLIC_STREAM_API_KEY && !!env.STREAM_SECRET_KEY, - webhooks: !!env.CLERK_WEBHOOK_SECRET, auth: auth.ready, }, }); diff --git a/src/components/providers/ConvexClerkProvider.tsx b/src/components/providers/ConvexClerkProvider.tsx deleted file mode 100644 index 3dbeb85..0000000 --- a/src/components/providers/ConvexClerkProvider.tsx +++ /dev/null @@ -1,28 +0,0 @@ -"use client"; -import { ClerkProvider, useAuth } from "@clerk/nextjs"; -import { ConvexReactClient } from "convex/react"; -import { ConvexProviderWithClerk } from "convex/react-clerk"; -import { UserSyncStatusProvider } from "@/components/providers/UserSyncStatusProvider"; -import { getValidatedClientEnv } from "@/lib/env"; - -const clientEnv = getValidatedClientEnv(); -const convex = new ConvexReactClient(clientEnv.NEXT_PUBLIC_CONVEX_URL); - -function ConvexClerkProvider({ children }: { children: React.ReactNode }) { - return ( - // No `appearance` here on purpose. It needs the resolved theme, and this - // provider sits above ThemeProvider, so `useTheme()` is unavailable at this - // point in the tree. Each Clerk surface applies it instead via - // useClerkAppearance, which runs where the theme is known. - - - {children} - - - ); -} - -export default ConvexClerkProvider; diff --git a/src/components/providers/UserSyncStatusProvider.tsx b/src/components/providers/UserSyncStatusProvider.tsx deleted file mode 100644 index 3ab923b..0000000 --- a/src/components/providers/UserSyncStatusProvider.tsx +++ /dev/null @@ -1,28 +0,0 @@ -"use client"; - -import { - createContext, - useContext, - type ReactNode, -} from "react"; -import { useSyncUser, type UserSyncState } from "@/hooks/useSyncUser"; - -const UserSyncStatusContext = createContext({ - status: "loading", -}); - -export function UserSyncStatusProvider({ - children, -}: { - children: ReactNode; -}) { - const syncState = useSyncUser(); - - return ( - - {children} - - ); -} - -export const useUserSyncStatus = () => useContext(UserSyncStatusContext); diff --git a/src/hooks/useClerkAppearance.ts b/src/hooks/useClerkAppearance.ts deleted file mode 100644 index db02890..0000000 --- a/src/hooks/useClerkAppearance.ts +++ /dev/null @@ -1,24 +0,0 @@ -"use client"; - -import { useMemo } from "react"; -import { useTheme } from "next-themes"; -import { buildClerkAppearance } from "@/lib/clerkAppearance"; - -/** - * Theme-aware Clerk appearance. - * - * A hook rather than a value on `` because that provider sits - * *above* `ThemeProvider` in the root layout, so `useTheme()` is not available - * where the provider is constructed. Every Clerk component that renders visible - * chrome — the sign-in and sign-up forms, the navbar's modals, `UserButton` — - * sits comfortably inside `ThemeProvider`, so reading the theme at the point of - * use avoids restructuring the provider tree. - */ -export const useClerkAppearance = () => { - const { resolvedTheme } = useTheme(); - - return useMemo( - () => buildClerkAppearance(resolvedTheme === "dark"), - [resolvedTheme], - ); -}; diff --git a/src/hooks/useSyncUser.ts b/src/hooks/useSyncUser.ts deleted file mode 100644 index ac9c063..0000000 --- a/src/hooks/useSyncUser.ts +++ /dev/null @@ -1,159 +0,0 @@ -"use client"; - -import { useClerk, useUser } from "@clerk/nextjs"; -import { useConvexAuth, useMutation } from "convex/react"; -import { api } from "../../convex/_generated/api"; -import { useCallback, useEffect, useRef, useState } from "react"; -import { toast } from "sonner"; -import { logError, sanitizeErrorMessage } from "@/lib/errors"; -import { retryAsync } from "@/lib/retry"; - -const DUPLICATE_ACCOUNT_PREFIX = "An account with this email already exists"; - -export type UserSyncStatus = - | "loading" - | "signedOut" - | "syncing" - | "ready" - | "error"; - -export type UserSyncState = { - status: UserSyncStatus; - clerkId?: string; - errorMessage?: string; -}; - -/** - * Hook to sync the currently signed-in Clerk user to Convex. - * Call this in a top-level component so it runs on every auth state change. - * - * IMPORTANT: this hook must not call any Convex `useQuery` — it renders inside - * the root layout (above app/error.tsx's reach), so a throwing query here would - * bubble straight to global-error.tsx and crash the whole signed-in shell. - * Reliability comes from the retry below plus the Clerk webhook in convex/http.ts. - */ -export function useSyncUser() { - const { user, isSignedIn, isLoaded } = useUser(); - const { isAuthenticated, isLoading } = useConvexAuth(); - const { signOut } = useClerk(); - const syncUser = useMutation(api.users.syncUser); - const [syncState, setSyncState] = useState({ - status: "loading", - }); - - const inFlightForClerkIdRef = useRef(null); - const syncedForClerkIdRef = useRef(null); - const fatalToastShownRef = useRef(false); - - const setNextSyncState = useCallback((next: UserSyncState) => { - setSyncState((current) => - current.status === next.status && - current.clerkId === next.clerkId && - current.errorMessage === next.errorMessage - ? current - : next, - ); - }, []); - - const runSync = useCallback( - async ( - clerkId: string, - payload: { - email: string; - name: string; - image?: string; - }, - ) => { - if (syncedForClerkIdRef.current === clerkId) { - setNextSyncState({ status: "ready", clerkId }); - return; - } - - if (inFlightForClerkIdRef.current === clerkId) return; - inFlightForClerkIdRef.current = clerkId; - setNextSyncState({ status: "syncing", clerkId }); - - try { - await retryAsync(() => syncUser(payload), { - retries: 2, - shouldRetry: (error) => { - const message = sanitizeErrorMessage(error, ""); - return !message.startsWith(DUPLICATE_ACCOUNT_PREFIX); - }, - }); - syncedForClerkIdRef.current = clerkId; - fatalToastShownRef.current = false; - setNextSyncState({ status: "ready", clerkId }); - } catch (error) { - logError("useSyncUser", error, { userId: clerkId }); - - const message = sanitizeErrorMessage(error, ""); - setNextSyncState({ - status: "error", - clerkId, - errorMessage: message || "Unable to sync the signed-in user.", - }); - - if (message.startsWith(DUPLICATE_ACCOUNT_PREFIX)) { - toast.error(message); - await signOut({ redirectUrl: "/" }); - return; - } - - if (!fatalToastShownRef.current) { - fatalToastShownRef.current = true; - toast.error( - "We couldn't finish setting up your account. Please refresh the page.", - ); - } - } finally { - if (inFlightForClerkIdRef.current === clerkId) { - inFlightForClerkIdRef.current = null; - } - } - }, - [setNextSyncState, syncUser, signOut], - ); - - useEffect(() => { - const clerkId = user?.id; - - if (!isLoaded || isLoading || (isSignedIn && !isAuthenticated)) { - setNextSyncState({ status: "loading", clerkId }); - return; - } - - if (!isSignedIn || !user) { - inFlightForClerkIdRef.current = null; - syncedForClerkIdRef.current = null; - fatalToastShownRef.current = false; - setNextSyncState({ status: "signedOut" }); - return; - } - - if ( - syncedForClerkIdRef.current && - syncedForClerkIdRef.current !== user.id - ) { - syncedForClerkIdRef.current = null; - } - - // clerkId is intentionally not sent: the mutation derives it from the verified - // Convex identity so a client cannot sync a row it does not own. - void runSync(user.id, { - email: user.primaryEmailAddress?.emailAddress ?? "", - name: user.fullName ?? user.firstName ?? "", - image: user.imageUrl ?? undefined, - }); - }, [ - isAuthenticated, - isLoaded, - isLoading, - isSignedIn, - user, - runSync, - setNextSyncState, - ]); - - return syncState; -} diff --git a/src/lib/auth/subjectResolution.test.ts b/src/lib/auth/subjectResolution.test.ts index f082490..be469d9 100644 --- a/src/lib/auth/subjectResolution.test.ts +++ b/src/lib/auth/subjectResolution.test.ts @@ -3,32 +3,23 @@ import { describe, it } from "node:test"; import { resolveUserBySubject } from "../../../convex/lib/subjectResolution.ts"; -type Row = { _id: string; clerkId: string; legacyClerkId?: string; email: string }; - -type Index = "by_clerk_id" | "by_legacy_clerk_id"; +type Row = { _id: string; clerkId: string; email: string }; /** - * A stand-in for the Convex ctx, with the three lookups this function uses. + * A stand-in for the Convex ctx. * * `normalizeId` mirrors the real one: it returns the string for something - * shaped like an id for this table and null for anything else, which is what - * makes trying `db.get` first safe. `get` records its calls so a test can - * assert it was never reached with a Clerk id — the real one throws on that. + * shaped like an id for this table and null for anything else. `get` records + * its calls so a test can assert it was never reached with a malformed id — the + * real one throws on that. */ const makeCtx = (options: { byId?: Record; - byClerkId?: Record; - byLegacyClerkId?: Record; isId?: (value: string) => boolean; }) => { const getCalls: string[] = []; const isId = options.isId ?? ((value: string) => value.startsWith("k5")); - const tables: Record | undefined> = { - by_clerk_id: options.byClerkId, - by_legacy_clerk_id: options.byLegacyClerkId, - }; - return { getCalls, ctx: { @@ -38,124 +29,56 @@ const makeCtx = (options: { getCalls.push(id); return options.byId?.[id] ?? null; }, - query: () => ({ - withIndex: ( - index: Index, - builder: (q: { - eq: (field: "clerkId" | "legacyClerkId", value: string) => unknown; - }) => unknown, - ) => { - let wanted = ""; - builder({ - eq: (_field, value) => { - wanted = value; - return null; - }, - }); - return { first: async () => tables[index]?.[wanted] ?? null }; - }, - }), }, }, }; }; /** - * The ways a signed-in user can arrive while both providers are registered. + * With Clerk removed, `identity.subject` is always the Convex document id: + * Auth.js is the only provider registered in convex/auth.config.ts, and it + * takes the subject from the id the adapter returned. * - * Case 3 is the one the original by_clerk_id query got wrong. The legacyClerkId - * case covers the far end of the migration, once Task 16 drops the clerkId - * column — not the state the backfill leaves, which keeps clerkId intact. - * Neither symptom is an error: both are a signed-in user being told their - * account is not ready yet. + * The `by_clerk_id` and `by_legacy_clerk_id` fallbacks this used to carry went + * with Clerk. No token in existence carries a Clerk id any more, so those reads + * could only ever miss. */ describe("resolveUserBySubject", () => { - it("finds a user Auth.js created, whose clerkId is their own id", async () => { - const row: Row = { _id: "k5abc", clerkId: "k5abc", email: "new@example.com" }; - const { ctx } = makeCtx({ byId: { k5abc: row }, byClerkId: { k5abc: row } }); + it("resolves a subject to its user row", async () => { + const row: Row = { _id: "k5abc", clerkId: "k5abc", email: "user@example.com" }; + const { ctx } = makeCtx({ byId: { k5abc: row } }); assert.deepEqual(await resolveUserBySubject(ctx, "k5abc"), row); }); - it("finds a legacy user presenting a Clerk token, before the backfill", async () => { - const row: Row = { _id: "k5xyz", clerkId: "user_2abc", email: "old@example.com" }; - const { ctx } = makeCtx({ byClerkId: { user_2abc: row } }); - - assert.deepEqual(await resolveUserBySubject(ctx, "user_2abc"), row); - }); - - it("finds a legacy user presenting an Auth.js token", async () => { - // Subject is the document id while clerkId is still the Clerk one, so only - // the id lookup finds them. This is the case that appears the moment - // anyone migrates. + it("resolves a migrated user, whose clerkId is not their id", async () => { + // clerkId still holds the original Clerk value for accounts that predate + // the migration, because interviewerIds, candidateId and auditLogs all + // reference it. It is simply not consulted here any more. const row: Row = { _id: "k5xyz", clerkId: "user_2abc", email: "old@example.com" }; const { ctx } = makeCtx({ byId: { k5xyz: row } }); assert.deepEqual(await resolveUserBySubject(ctx, "k5xyz"), row); }); - it("finds a user whose Clerk id survives only as legacyClerkId", async () => { - // Not the state the backfill leaves -- it copies clerkId to legacyClerkId - // without rewriting clerkId, because interviewerIds and auditLogs reference - // that value. This is the state after Task 16 drops the clerkId column, - // and covering it means the order of those two steps cannot strand anyone. - const row: Row = { - _id: "k5xyz", - clerkId: "k5xyz", - legacyClerkId: "user_2abc", - email: "migrated@example.com", - }; - const { ctx } = makeCtx({ byLegacyClerkId: { user_2abc: row } }); - - assert.deepEqual(await resolveUserBySubject(ctx, "user_2abc"), row); - }); - - it("prefers the current clerkId over a legacy one on collision", async () => { - // If one row still carries a value as clerkId and another has retired the - // same value to legacyClerkId, the live column wins. Reversing this would - // resolve a subject to an account that has already moved on from it. - const current: Row = { _id: "k5one", clerkId: "user_2abc", email: "current@example.com" }; - const retired: Row = { - _id: "k5two", - clerkId: "k5two", - legacyClerkId: "user_2abc", - email: "retired@example.com", - }; - const { ctx } = makeCtx({ - byClerkId: { user_2abc: current }, - byLegacyClerkId: { user_2abc: retired }, - }); + it("returns null for a well-formed id with no row", async () => { + const { ctx } = makeCtx({ byId: {} }); - assert.deepEqual(await resolveUserBySubject(ctx, "user_2abc"), current); + assert.equal(await resolveUserBySubject(ctx, "k5gone"), null); }); - it("never calls db.get with something that is not an id for the table", async () => { - // The real db.get throws on a malformed id, which would turn a Clerk - // sign-in into a 500 rather than a lookup. - const row: Row = { _id: "k5xyz", clerkId: "user_2abc", email: "old@example.com" }; - const { ctx, getCalls } = makeCtx({ byClerkId: { user_2abc: row } }); - - await resolveUserBySubject(ctx, "user_2abc"); + it("returns null rather than throwing on a malformed subject", async () => { + // The real db.get throws on anything that is not an id for this table. A + // malformed subject must become "no such user", which the caller turns into + // a sign-in prompt, rather than a 500 and an error page. + const { ctx, getCalls } = makeCtx({}); + assert.equal(await resolveUserBySubject(ctx, "user_2abc"), null); assert.deepEqual(getCalls, []); }); - it("falls through to the index when the id is well formed but the row is gone", async () => { - const row: Row = { _id: "k5old", clerkId: "k5gone", email: "recreated@example.com" }; - const { ctx } = makeCtx({ byId: {}, byClerkId: { k5gone: row } }); - - assert.deepEqual(await resolveUserBySubject(ctx, "k5gone"), row); - }); - - it("returns null when no lookup matches", async () => { - const { ctx } = makeCtx({}); - - assert.equal(await resolveUserBySubject(ctx, "k5nobody"), null); - }); - - it("refuses an empty subject rather than matching an empty clerkId", async () => { - const stray: Row = { _id: "k5stray", clerkId: "", email: "stray@example.com" }; - const { ctx, getCalls } = makeCtx({ byClerkId: { "": stray } }); + it("refuses an empty subject without touching the database", async () => { + const { ctx, getCalls } = makeCtx({}); assert.equal(await resolveUserBySubject(ctx, ""), null); assert.deepEqual(getCalls, []); diff --git a/src/lib/clerkAppearance.ts b/src/lib/clerkAppearance.ts deleted file mode 100644 index 8e3e265..0000000 --- a/src/lib/clerkAppearance.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { Appearance } from "@clerk/types"; - -/** - * Clerk's own UI, themed to match the app. - * - * Two lessons are baked into the shape of this file. - * - * **Let Clerk own its card.** The first attempt wrapped `` in an app - * card with its own heading, which produced a card inside a card and two - * headings saying the same thing — "Sign in" above "Sign in to Commit". Clerk's - * own chrome is well made; the job here is to recolour it, not to rebuild it. - * - * **Use `variables`, not `elements` with Tailwind classes.** Clerk's internal - * styles are more specific than a single Tailwind utility, so class overrides - * silently lost — the primary button stayed Clerk's default near-black rather - * than the brand orange. `variables` feed Clerk's own token system, so it - * derives its hover, focus and disabled shades correctly from them. - * - * Colours are concrete rather than `hsl(var(--primary))` for the same reason: - * Clerk computes derived shades from the value it is given and cannot do that - * arithmetic on a CSS variable reference. They mirror globals.css by hand, so a - * palette change there needs echoing here — the trade for Clerk deriving a - * correct hover state. - */ - -/** Orange-500, matching `--primary` in globals.css. */ -const BRAND = "#f97316"; - -const LIGHT = { - colorBackground: "#ffffff", - colorText: "#0f172a", - colorTextSecondary: "#64748b", - colorInputBackground: "#ffffff", - colorInputText: "#0f172a", - colorNeutral: "#0f172a", -}; - -const DARK = { - // Matches the lifted `--card` in dark mode, so Clerk's card reads as elevated - // against the page rather than floating as a white slab. - colorBackground: "#141417", - colorText: "#fafafa", - colorTextSecondary: "#a1a1aa", - colorInputBackground: "#1c1c20", - colorInputText: "#fafafa", - colorNeutral: "#fafafa", -}; - -export const buildClerkAppearance = (isDark: boolean): Appearance => { - const palette = isDark ? DARK : LIGHT; - - return { - layout: { - // The page already shows the wordmark directly above; Clerk repeating it - // was part of what made the screen feel like it said everything twice. - logoPlacement: "none", - socialButtonsVariant: "blockButton", - shimmer: false, - }, - variables: { - colorPrimary: BRAND, - colorDanger: "#e11d48", - colorSuccess: "#059669", - borderRadius: "0.75rem", - fontFamily: "var(--font-jakarta-sans), ui-sans-serif, system-ui, sans-serif", - ...palette, - }, - elements: { - // Clerk's card sits inside the page's own centred column, so it should not - // add a second drop shadow on top of the one the page already provides. - cardBox: "shadow-none", - card: "shadow-none", - // Clerk's free tier requires its attribution, so the footer stays. Only - // the extra padding around it is trimmed. - footer: "bg-transparent", - }, - }; -}; diff --git a/src/lib/env.ts b/src/lib/env.ts index 9fdad07..ba39ebb 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -2,13 +2,11 @@ import { z } from "zod"; const requiredServerEnvSchema = z.object({ NEXT_PUBLIC_CONVEX_URL: z.string().url(), - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: z.string().min(1), NEXT_PUBLIC_STREAM_API_KEY: z.string().min(1), STREAM_SECRET_KEY: z.string().min(1), }); const optionalServerEnvSchema = requiredServerEnvSchema.extend({ - CLERK_WEBHOOK_SECRET: z.string().min(1).optional(), // SMTP (optional in dev – emails are logged to console) SMTP_HOST: z.string().min(1).optional(), SMTP_PORT: z.string().optional(), @@ -26,7 +24,6 @@ const optionalServerEnvSchema = requiredServerEnvSchema.extend({ }); const productionServerEnvSchema = requiredServerEnvSchema.extend({ - CLERK_WEBHOOK_SECRET: z.string().min(1), // SMTP required in production SMTP_HOST: z.string().min(1), SMTP_PORT: z.string().min(1), @@ -41,7 +38,6 @@ const productionServerEnvSchema = requiredServerEnvSchema.extend({ const clientEnvSchema = z.object({ NEXT_PUBLIC_CONVEX_URL: z.string().url(), - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: z.string().min(1), NEXT_PUBLIC_STREAM_API_KEY: z.string().min(1), NEXT_PUBLIC_APP_URL: z.string().url().optional(), NEXT_PUBLIC_FEATURE_FLAGS: z.string().optional(), @@ -88,8 +84,6 @@ export const getValidatedClientEnv = () => { const parsed = clientEnvSchema.safeParse({ NEXT_PUBLIC_CONVEX_URL: process.env.NEXT_PUBLIC_CONVEX_URL, - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: - process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY, NEXT_PUBLIC_STREAM_API_KEY: process.env.NEXT_PUBLIC_STREAM_API_KEY, NEXT_PUBLIC_APP_URL: process.env.NEXT_PUBLIC_APP_URL, NEXT_PUBLIC_FEATURE_FLAGS: process.env.NEXT_PUBLIC_FEATURE_FLAGS,