Restore sign-in redirect for signed-out dashboard requests - #47
Conversation
next-auth only performs its automatic redirect to the sign-in page when auth() is used without a custom middleware function. The Forms MVP wrapped the middleware to rewrite the forms host, which disabled that redirect, so signed-out visitors reached dashboard pages whose data actions failed and rendered the not-found page. Return the redirect from the authorized callback instead, which next-auth honours ahead of the middleware function, and cover the decision with a unit test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hhKHKm5i7XvzjVpTbjQ5T
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a reusable authorization helper, delegates the NextAuth authorization callback to it, and adds tests for redirects, authenticated requests, the sign-in path, and public form surfaces. ChangesAuthorization flow
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant NextAuthAuthorizedCallback
participant authorizeRequest
participant NextResponse
NextAuthAuthorizedCallback->>authorizeRequest: pass authenticated and nextUrl
authorizeRequest->>NextResponse: return true or a 307 login redirect
Merge Risk: ⚪ Minimal · up to Signed-out dashboard requests redirect to login while public form surfaces and authenticated requests remain accessible. No actionable current-head risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
__tests__/authorize.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. lib/auth/authorize.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). lib/auth/index.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Signed-out visitors to app.router.so were seeing the 404 page instead of the login page after #45.
next-auth only performs its automatic redirect to the sign-in page when
auth()is used without a custom middleware function. #45 wrapped the middleware to rewrite theforms.router.sohost, which silently disabled that redirect, so signed-out requests reached dashboard pages whose data actions failed and rendered not-found. Signed-in sessions were unaffected.The
authorizedcallback now returns the redirect itself, which next-auth honours ahead of the middleware function. The decision lives inlib/auth/authorize.tswith a unit test covering the redirect, the sign-in page, signed-in requests, and public form surfaces.Verification
pnpm typecheck,pnpm lint,pnpm test:unit(160 passed, 4 new)/and/endpointsreturn 307 to/login?callbackUrl=...when signed out;/login,/f/*, and/embed/v1.jsstill return 200.🤖 Generated with Claude Code
https://claude.ai/code/session_011hhKHKm5i7XvzjVpTbjQ5T
Summary by CodeRabbit
Access Control
Public Access