Auth hardening: JWT middleware, sign-in bypass fix, dev-mode gate, marketing build#21
Merged
Merged
Conversation
Dev auto-login bypass in apps/api/lib/user.ts previously triggered on NODE_ENV=development alone, which is load-bearing: any environment that accidentally sets NODE_ENV=development would serve every request as dev@baseline.local. Now requires all three: - NODE_ENV === 'development' - BASELIE_DEV_AUTO_LOGIN === 'true' (explicit opt-in) - DATABASE_URL containing 'localhost' (defense in depth against pointing a dev-mode container at a shared/prod database) Also logs a warning every time the bypass fires so it can't be forgotten. Separately, fix the marketing Docker build so NEXT_PUBLIC_GITHUB_USERNAME actually reaches Next.js at build time. Next.js inlines NEXT_PUBLIC_* vars into the client bundle during `next build`, but docker-compose was only passing the value as a runtime env, so the fallback literal "YOUR_GITHUB_USERNAME" was being baked into the compiled bundle. - apps/marketing/Dockerfile: accept ARG in builder stage, export as ENV - docker-compose.yml: pass NEXT_PUBLIC_GITHUB_USERNAME via build.args Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The sign-in handler POSTed credentials to /api/auth/callback/credentials but ignored the response, then checked /api/auth/session to decide success. Because next-auth's session endpoint honors any valid session cookie sent along, a user with a previous valid session could submit any password (including nonsense) and be treated as signed-in: the session check picked up the stale cookie, the sentinel baseline-session cookie got set, and middleware let them through. Three changes: 1. Explicitly sign out and clear baseline-session BEFORE attempting the new sign-in, so the subsequent session check only reflects this attempt. 2. Parse the credentials callback response; treat an error url or missing url as a failed sign-in and bail. 3. Secondary verification: after the callback succeeds, confirm session.user.email matches the submitted email (case-insensitive) before setting the session cookie. Any one of these defenses would block the bypass; combining them is defense in depth against future regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Andrew5194
changed the title
The previous design used a client-settable `baseline-session` cookie (value "true") as the sole signal for middleware to decide auth. Two problems: 1. The cookie was decoupled from the real NextAuth session — it could live on after the real session was invalidated, or be missing while the real session was valid. 2. It was trivially forgeable: `document.cookie = "baseline-session=true"` in DevTools was enough to bypass middleware and land on the dashboard. This replaces the sentinel with a real JWT check using getToken() from next-auth/jwt, which cryptographically verifies the session cookie against AUTH_SECRET. No client-controllable bypass. Changes: - apps/web/middleware.ts: getToken() with explicit cookieName + salt, derived from AUTH_URL/NEXT_PUBLIC_API_URL scheme (http vs https) - apps/web/src/app/sign-in/page.tsx: stop writing baseline-session - apps/web/src/app/sign-up/page.tsx: stop writing baseline-session - apps/web/src/app/components/sidebar.tsx: sign out now POSTs to /api/auth/signout (the prior client-side cookie clearing was a no-op for HttpOnly NextAuth cookies) - docker-compose.yml: AUTH_SECRET added to web service env so the web middleware can verify JWTs signed by the API Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Andrew5194
changed the title
The web app uses src/app directory structure, which means Next.js looks for middleware at src/middleware.ts, not at the project root. The file sat at apps/web/middleware.ts and was never being loaded — the built middleware-manifest.json contained an empty middleware map. This meant the previous sentinel-cookie check was dead code, and the new JWT check added in this PR was equally inert. Moving to apps/web/src/middleware.ts so the manifest actually registers it and requests to protected routes are evaluated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The defense-in-depth signout before signing in was rotating NextAuth's CSRF token, which then caused the subsequent credentials callback to fail with CredentialsSignin (stale CSRF token vs. new session state). Users could not sign back in after creating an account. The remaining defenses — parsing the callback response and verifying session.user.email matches the submitted email — already prevent the stale-session bypass, so the signout was redundant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NextAuth v5 beta's /api/auth/callback/credentials returns a 302 redirect on BOTH success and failure, so checking the response body or status can't distinguish the two. My previous code expected a JSON body with `url` on success, so valid sign-ins were being treated as failures. Switching to a post-callback session check: authorize() only sets a session cookie when credentials are valid, so if `session.user.email === submitted email` after the callback, the attempt succeeded. Invalid credentials leave the prior session state unchanged (or empty), and the email comparison catches the stale-session bypass the original PR was designed to close. Removed `redirect: 'manual'` since it made response parsing impossible anyway; `redirect: 'follow'` lets the browser handle NextAuth's 302. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related auth/build fixes bundled together — all surfaced during dev setup.
1. Replace sentinel-cookie middleware with real JWT verification (new)
Previously
apps/web/middleware.tsonly checked for a custombaseline-session=truecookie — which was trivially forgeable in DevTools and decoupled from the real NextAuth session. Middleware now verifies the actual NextAuth JWT viagetToken()againstAUTH_SECRET.baseline-sessionwrites from sign-in and sign-up pages/api/auth/signout(the prior client-sidedocument.cookieclearing was a no-op for HttpOnly cookies)2. Fix sign-in bypass when a prior session cookie exists
The sign-in handler POSTed credentials but ignored the response, then checked
/api/auth/sessionto decide success. Because the session endpoint honored any valid session cookie sent along, a user with a previous valid session could submit any password and be treated as signed-in.Three layers of defense added in apps/web/src/app/sign-in/page.tsx:
session.user.emailmatches the submitted email before redirecting3. Harden dev auth bypass
apps/api/lib/user.ts previously auto-logged any unauthenticated request as
dev@baseline.localwheneverNODE_ENV === 'development'. Now requires all three:NODE_ENV === 'development'BASELINE_DEV_AUTO_LOGIN === 'true'(explicit opt-in)DATABASE_URLcontainslocalhost(defense in depth against pointing a dev-mode container at a shared/prod DB)Logs a warning every time the bypass fires.
4. Fix marketing NEXT_PUBLIC_ Docker build
NEXT_PUBLIC_GITHUB_USERNAMEwas being baked into the client bundle as"YOUR_GITHUB_USERNAME". Root cause: Next.js inlinesNEXT_PUBLIC_*at build time, but docker-compose only passed it as a runtime env.ARGin builder stagebuild.argsTest plan
Middleware JWT verification
/redirects to/sign-indocument.cookie = "baseline-session=true"→ middleware should still redirect (previously this bypassed)/redirect to/sign-inSign-in bypass
Dev auth bypass
BASELINE_DEV_AUTO_LOGIN=true, unauthenticated API request returns 401Marketing build
docker compose build --no-cache marketingbakes real username into bundlePost-merge setup notes
Add
BASELINE_DEV_AUTO_LOGIN=trueto your local.envif you want to keep the dev auto-login behavior. Rundocker compose build --no-cache && docker compose up -dto pick up all four fixes.Generated with Claude Code