fix(nextjs): return 401 for RSC navigations with recoverable sessions - #9285
fix(nextjs): return 401 for RSC navigations with recoverable sessions#9285nikosdouvlis wants to merge 1 commit into
Conversation
When clerkMiddleware answers an App Router RSC navigation fetch with a Clerk redirect (protect() or redirectToSignIn/redirectToSignUp), the router soft-navigates the user to the sign-in page even when the signed-out state is recoverable, e.g. an expired but refreshable session token. RSC fetches are not handshake-eligible and the refresh cookie is only planted by a handshake, so users signed in via clerk-js could not recover and got bounced to sign-in on the first stale navigation. Answer these navigations with a 401 instead. The Next router treats a non-2xx RSC response as an MPA fallback and performs a full document navigation, which is handshake-eligible: the handshake restores the session and plants the refresh cookie so later stale RSC navigations refresh server-side. Genuinely signed-out users keep the redirect so their soft-nav to the sign-in page stays instant.
🦋 Changeset detectedLatest commit: 44c3285 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughNext.js middleware now detects recoverable signed-out sessions during App Router RSC navigations and returns a non-cached Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/nextjs/src/server/protect.ts (1)
235-242: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit return type now that this is exported.
Coding guidelines require explicit return types for exported functions/public APIs.
♻️ Proposed change
-export const isServerActionRequest = (req: Request) => { +export const isServerActionRequest = (req: Request): boolean => {As per coding guidelines: "Always define explicit return types for functions, especially public APIs".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextjs/src/server/protect.ts` around lines 235 - 242, Add an explicit boolean return type to the exported isServerActionRequest function, preserving its existing request-header evaluation and behavior.Source: Coding guidelines
packages/nextjs/src/server/clerkMiddleware.ts (1)
614-622: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare the reason list as readonly.
♻️ Proposed change
-const RECOVERABLE_SIGNED_OUT_REASONS: string[] = [ +const RECOVERABLE_SIGNED_OUT_REASONS: readonly string[] = [As per coding guidelines: "Declare readonly arrays and objects for immutable data structures".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextjs/src/server/clerkMiddleware.ts` around lines 614 - 622, Declare RECOVERABLE_SIGNED_OUT_REASONS as a readonly array while preserving its existing AuthErrorReason entries and ordering.Source: Coding guidelines
packages/nextjs/src/server/__tests__/clerkMiddleware.test.ts (1)
1115-1124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for server-action requests.
isAppRouterNavigationFetchnewly excludes server actions viaisServerActionRequest, but no test exercises that branch (e.g.rscRequest({ 'next-action': '1' })on a POST/GET should still redirect).As per coding guidelines: "Verify proper error handling and edge cases".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextjs/src/server/__tests__/clerkMiddleware.test.ts` around lines 1115 - 1124, Add a test alongside the existing Pages Router internal-navigation case that invokes clerkMiddleware with a server-action request marked by the next-action header, covering the relevant POST/GET request method, and assert that auth.protect still produces a 307 redirect with a sign-in location. Reuse the existing mockSignedOut, expiredReason, rscRequest, and middleware setup.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/nextjs/src/server/__tests__/clerkMiddleware.test.ts`:
- Around line 1115-1124: Add a test alongside the existing Pages Router
internal-navigation case that invokes clerkMiddleware with a server-action
request marked by the next-action header, covering the relevant POST/GET request
method, and assert that auth.protect still produces a 307 redirect with a
sign-in location. Reuse the existing mockSignedOut, expiredReason, rscRequest,
and middleware setup.
In `@packages/nextjs/src/server/clerkMiddleware.ts`:
- Around line 614-622: Declare RECOVERABLE_SIGNED_OUT_REASONS as a readonly
array while preserving its existing AuthErrorReason entries and ordering.
In `@packages/nextjs/src/server/protect.ts`:
- Around line 235-242: Add an explicit boolean return type to the exported
isServerActionRequest function, preserving its existing request-header
evaluation and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3af58a14-66ba-48b2-aa63-3922861d1ddb
📒 Files selected for processing (7)
.changeset/rsc-navigation-401-for-recoverable-sessions.mdpackages/backend/src/__tests__/exports.test.tspackages/backend/src/internal.tspackages/nextjs/src/server/__tests__/clerkMiddleware.test.tspackages/nextjs/src/server/clerkMiddleware.tspackages/nextjs/src/server/nextErrors.tspackages/nextjs/src/server/protect.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
API Changes Report
Summary
🔴 Breaking changes index (93)Every breaking change, up front. Full diffs are in the package sections below.
@clerk/backendVersion: 3.14.0 → 3.13.1 Subpath
|
1 participant
Why
On Next.js App Router, client-side navigations can bounce a signed-in user to the sign-in page (or hang) once their session token goes stale. RSC navigation fetches are not handshake-eligible, and the refresh cookie is only planted by a handshake, so a user who signed in through clerk-js and never handshaked cannot recover: clerkMiddleware answers the RSC fetch with a 307 to sign-in and the router soft-navigates them there while they are still authenticated. Reported by an enterprise customer after upgrading to a recent @clerk/nextjs.
What changed
When the request is an App Router navigation fetch (GET, next-url header, Accept /, not a server action or pages-router data request) and the signed-out reason is recoverable (expired but refreshable token, UAT mismatches, dev browser states, clock skew), clerkMiddleware answers 401 with Cache-Control: no-store instead of the redirect. The Next router treats a non-2xx RSC response as an MPA fallback and re-runs the navigation as a full document request, which is handshake-eligible: the handshake restores the session and plants the refresh cookie, so later stale RSC navigations refresh server-side without the fallback. Genuinely signed-out users keep the 307, so navigating to a protected page after sign-out still soft-navs instantly. Also exports AuthErrorReason from @clerk/backend/internal.