fix(fastify): Add __internal_enableHandshake option to skip handshake flow - #8560
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 386bcff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
!snapshot |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR initializes a single Clerk client at middleware setup using resolved runtime keys, adds an optional enableHandshake flag to control handshake/redirect handling, strips handshake cookies/query params when disabled, and attaches the Clerk client to requests as request.clerk. Type declarations and the plugin decoration are updated accordingly, and tests were added to verify runtime key usage, enableHandshake behavior, handshake cookie stripping, and request.clerk exposure. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Title checkExplanation The pull request title 'chore(fastify): Explicit opt-out handshake option' describes only one aspect of the changeset. The PR objectives indicate two primary goals: (1) using runtime keys for auth client construction to fix authentication failures when secrets load at runtime, and (2) adding an explicit opt-out handshake option. The title captures the second objective but omits the first, which is the core bug fix documented in the changeset entry. The title is related to the changeset but does not convey the main point.
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
This comment has been minimized.
This comment has been minimized.
@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: |
71d3ff7 to
25efe99
Compare
|
Done — I pulled in the changes from #8563 in commits 19f3473, 0cc1e3c, and 60312d1, and added a follow-up fix for the React Router CI type error in 9243a0a. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
!snapshot |
|
Another bump here - this is debt that I feel that we need to clear out 😬 |
e5bb059 to
08cffa5
Compare
API Changes Report
Summary
@clerk/fastifyCurrent version: 3.1.74 Subpath
|
08cffa5 to
4777998
Compare
… flow When disabled, the plugin strips handshake cookies and query params before authenticating and skips handshake redirects, except dev-browser handshakes that development instances require. A skipped handshake redirect now falls back to a signed-out auth object instead of a null request.auth, and the handshake location/cache-control headers are dropped from the reply. Claude-Session: https://claude.ai/code/session_01KHSkuMqXon3pG5c7ifFTN2
4777998 to
815b7c0
Compare
|
!snapshot |
With the backend option in place the plugin no longer needs to suppress handshake redirects, so the dev-browser exception, header cleanup, and signed-out fallback go away and the redirect path returns to its pre-#8560 shape. Adds an end-to-end test through the real backend for the opt-out, including both development sandbox flows. Claude-Session: https://claude.ai/code/session_014TUNghY1SiahtRY5t1zZWC
Summary
__internal_enableHandshakeoption toclerkPlugin()(defaults totrue)false, strip handshake cookies (__clerk_handshake,__clerk_handshake_nonce) and query params before callingauthenticateRequest, and skip handshake redirectsdev-browser-missing,dev-browser-sync) even when disabled, since development instances cannot establish auth state without themtoAuth()isnull), and drop the already-appliedLocation/Cache-Controlheaders from the replyContext
Forced handshake nonce transport stores a short
__clerk_handshake_nonceinstead of the large__clerk_handshakepayload. Server SDKs then need to exchange that nonce through the Backend API client attached toauthenticateRequest.On an API-only backend that cannot deliver
Set-Cookieheaders back to the browser (e.g. Fastify on Lambda behind API Gateway, where multi-valueSet-Cookieheaders are dropped), the browser can never clear that nonce cookie. Every subsequent request replays the already-consumed nonce, and the exchange returns404 resource_not_found, loggingClerk: HandshakeService: error getting handshake payloadon each request.authenticateRequestalready tolerates the failed exchange and falls through to cookie verification, so disabling the handshake removes the per-request error noise, the wasted Backend API call, and handshake redirects that an API client can never complete.The option is
__internal_-prefixed rather than public because@clerk/expressdeprecated (#6499) and removed (#7926) its publicenableHandshakeoption, and we don't want to reintroduce public API surface for this. The affected customer has been running the equivalent behavior in production via snapshot since May 22 with the errors fully gone.Notes
mainand now contains only the handshake option.AuthErrorReasonis not exported from@clerk/backend/internal; exporting it is a one-line@clerk/backendchange if preferred.✎