feat(backend): Signal support for handshake nonce #5905
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: ef10516 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for signaling the handshake nonce flow by including a query parameter.
- In handshake.ts, the query parameter SupportsHandshakeNonce is appended to the URL.
- In handshake.test.ts, corresponding tests ensure the parameter is correctly set in both regular and development modes.
- In constants.ts, a new constant for SupportsHandshakeNonce is added to support the new query parameter.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/backend/src/tokens/handshake.ts | Appends a new query parameter for handshake nonce support. |
| packages/backend/src/tokens/tests/handshake.test.ts | Adds tests to validate the presence of the new query parameter. |
| packages/backend/src/constants.ts | Introduces the SupportsHandshakeNonce constant to hold the parameter key. |
|
LGTM. So to be doubly clear: this indicates "nonce support", but FAPI still has the option to return 'optimized' payload in initial response, right? |
|
@jfoshee Yeah, this is just to signal to the API that it COULD send a handshake nonce |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughThis change introduces a new query parameter, Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found based on the objectives from the linked issues. Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/backend/src/constants.ts (1)
37-39: Query-param keyformatis extremely generic – consider a Clerk-specific prefixUsing the literal string
"format"forHandshakeFormatbreaks the naming pattern of the surrounding query parameters (all are Clerk-prefixed or otherwise very explicit) and could easily collide with existing parameters added by reverse proxies or middleware.
A more self-describing key such ashandshake_formator even re-using the earlier suggestionhandshake_modewould avoid ambiguity while remaining backward-compatible (the API can accept both).- HandshakeFormat: 'format', + HandshakeFormat: 'handshake_format',If you agree, remember to update
buildRedirectToHandshakeand the tests accordingly.
🧹 Nitpick comments (1)
.changeset/six-ears-wash.md (1)
1-6: Consider mentioning the actual query-param nameFor clarity to consumers reading the changelog, you might add something like “…by appending
format=nonceto the handshake URL” so they immediately know what to look for.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/six-ears-wash.md(1 hunks)packages/backend/src/constants.ts(1 hunks)packages/backend/src/tokens/__tests__/handshake.test.ts(2 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/backend/src/tokens/handshake.ts (1)
148-153: Unconditional inclusion is fine, but confirm API fallbackAppending the new parameter on every redirect is straightforward and keeps the client logic simple.
Please double-check that the backend will gracefully ignoreformat=noncewhen it decides to return a classic token‐based handshake so that older environments remain functional.packages/backend/src/tokens/__tests__/handshake.test.ts (2)
154-160: Good assertion for the new paramThe extra expectation ensures we never regress on adding the nonce signal. 👍
170-173: Covers the dev-browser branch as wellNicely catches the development-mode path; no issues spotted.
jacekradko
changed the title
Description
Send query string param to signal support for handshake nonce flow from current version of
@clerk/backendRelated: SDKI-979
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests