fix(dashboard): properly handle JWKS response structure for JWT verification #1105
Conversation
…ication
The JWKS endpoint returns {"keys":[{...}]} but the code was trying to use
the entire response as a single JWK. This caused JWT verification to fail.
Changes:
- Parse JWKS response as {keys: JsonWebKey[]}
- Use the first key from the keys array (standard practice)
- Add validation to ensure keys array is not empty
- Simplify code by removing unnecessary kid matching logic
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughReworks JWT verification in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant createHandler as create-handler
participant Env as environment
participant JWKS as JWKS endpoint
participant verifyJwt as verifyJwt()
Caller->>createHandler: request with Authorization: Bearer <token>
createHandler->>Env: read CLERK_PUB_JWT_KEY / CLERK_PUB_JWT_URL
alt CLERK_PUB_JWT_KEY present
createHandler->>verifyJwt: verifyToken(token, { jwtKey })
verifyJwt-->>createHandler: success / failure
createHandler-->>Caller: success / error
else static key absent
createHandler->>createHandler: decode token payload → iss
alt CLERK_PUB_JWT_URL provided
createHandler->>JWKS: GET CLERK_PUB_JWT_URL (5s timeout)
else issuer used
createHandler->>JWKS: GET https://<iss>/.well-known/jwks.json (5s timeout)
end
JWKS-->>createHandler: { keys: [jwk1, ...] } or error
alt keys non-empty
createHandler->>verifyJwt: verifyJwt(token, jwk1)
verifyJwt-->>createHandler: success / failure
createHandler-->>Caller: success / error
else no keys
createHandler-->>Caller: error ("No JWKS keys found")
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
dashboard/backend/create-handler.ts (3)
82-82: Avoid pretending Response.json is generic.TS lib DOM doesn’t define json(); prefer casting the parsed value.
- const jwksResponse = await rJwtKey.Ok().json<{ keys: JsonWebKey[] }>(); + const jwksResponse = (await rJwtKey.Ok().json()) as { keys: JsonWebKey[] };
69-71: Fix string interpolation in error.Backticks are needed; otherwise the URL won’t be inserted.
- throw new Error("CLERK_PUB_JWT_URL(${CLERK_PUB_JWT_URL}) must use HTTPS"); + throw new Error(`CLERK_PUB_JWT_URL(${CLERK_PUB_JWT_URL}) must use HTTPS`);
100-103: Clarify the final error message to reflect actual failure mode.Current text implies config not set even when fetch/verify failed.
- throw new Error( - "You must set CLERK_PUB_JWT_URL(${CLERK_PUB_JWT_URL}) verify with CLERK_PUB_JWT_KEY(${CLERK_PUB_JWT_KEY}) failed", - ); + throw new Error(`JWT verification failed. JWKS fetch/verify did not succeed for URL=${CLERK_PUB_JWT_URL ?? "<unset>"}; ${CLERK_PUB_JWT_KEY ? "static key path also failed" : "no static key provided"}.`);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard/backend/create-handler.ts(1 hunks)
🔇 Additional comments (1)
dashboard/backend/create-handler.ts (1)
76-76: AbortSignal.timeout support confirmed in CI (Node); no fallback needed.
dashboard/backend/create-handler.ts
Outdated
| // Parse JWKS response | ||
| const jwksResponse = await rJwtKey.Ok().json<{ keys: JsonWebKey[] }>(); | ||
|
|
||
| if (!jwksResponse.keys || jwksResponse.keys.length === 0) { | ||
| throw new Error(`No keys found in JWKS from ${CLERK_PUB_JWT_URL}`); | ||
| } | ||
|
|
||
| // Use the first key (standard practice for JWKS endpoints) | ||
| const jwsPubKey = jwksResponse.keys[0]; | ||
|
|
There was a problem hiding this comment.
Don’t assume the first JWKS key; match kid or try all keys.
Picking the first key will fail during key rotation or multi-key JWKS. Decode the JWT header, prefer a kid match, and fall back to trying all keys before erroring.
Apply this diff:
- // Use the first key (standard practice for JWKS endpoints)
- const jwsPubKey = jwksResponse.keys[0];
-
- return (await verifyJwt(token, { key: jwsPubKey })) as unknown as ClerkTemplate;
+ const keys = jwksResponse.keys;
+ // Prefer kid match; fall back to trying all keys
+ const { kid } = JSON.parse(atob(token.split(".")[0])) as { kid?: string };
+ const candidates = kid ? keys.filter(k => k.kid === kid) : keys;
+ if (kid && candidates.length === 0) {
+ throw new Error(`No JWK with kid=${kid} found in JWKS from ${CLERK_PUB_JWT_URL}`);
+ }
+ let lastErr: unknown;
+ for (const jwk of (candidates.length ? candidates : keys)) {
+ try {
+ return (await verifyJwt(token, { key: jwk })) as unknown as ClerkTemplate;
+ } catch (e) {
+ lastErr = e;
+ }
+ }
+ throw new Error(`verifyJwt failed for all JWKS keys from ${CLERK_PUB_JWT_URL}: ${String(lastErr)}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Parse JWKS response | |
| const jwksResponse = await rJwtKey.Ok().json<{ keys: JsonWebKey[] }>(); | |
| if (!jwksResponse.keys || jwksResponse.keys.length === 0) { | |
| throw new Error(`No keys found in JWKS from ${CLERK_PUB_JWT_URL}`); | |
| } | |
| // Use the first key (standard practice for JWKS endpoints) | |
| const jwsPubKey = jwksResponse.keys[0]; | |
| // Parse JWKS response | |
| const jwksResponse = await rJwtKey.Ok().json<{ keys: JsonWebKey[] }>(); | |
| if (!jwksResponse.keys || jwksResponse.keys.length === 0) { | |
| throw new Error(`No keys found in JWKS from ${CLERK_PUB_JWT_URL}`); | |
| } | |
| const keys = jwksResponse.keys; | |
| // Prefer kid match; fall back to trying all keys | |
| const { kid } = JSON.parse(atob(token.split(".")[0])) as { kid?: string }; | |
| const candidates = kid ? keys.filter(k => k.kid === kid) : keys; | |
| if (kid && candidates.length === 0) { | |
| throw new Error(`No JWK with kid=${kid} found in JWKS from ${CLERK_PUB_JWT_URL}`); | |
| } | |
| let lastErr: unknown; | |
| for (const jwk of (candidates.length ? candidates : keys)) { | |
| try { | |
| return (await verifyJwt(token, { key: jwk })) as unknown as ClerkTemplate; | |
| } catch (e) { | |
| lastErr = e; | |
| } | |
| } | |
| throw new Error(`verifyJwt failed for all JWKS keys from ${CLERK_PUB_JWT_URL}: ${String(lastErr)}`); |
🤖 Prompt for AI Agents
In dashboard/backend/create-handler.ts around lines 81 to 90, the code currently
unconditionally picks the first JWKS key which breaks when keys rotate or
multiple keys exist; instead, decode the incoming JWT header to extract the kid,
then search jwksResponse.keys for a key whose kid matches and use that; if no
matching kid is found, attempt verification with each key in jwksResponse.keys
(or convert each JWK to a usable public key and try verification) before
throwing an error; ensure you throw a clear error only after all keys have been
tried and none validated the token.
Instead of requiring hardcoded JWKS URLs, automatically discover the JWKS
endpoint from the JWT's issuer field. This makes the system more robust
across different Clerk instances (dev, prod, etc).
Changes:
- Extract issuer from JWT payload automatically
- Construct JWKS URL as {issuer}/.well-known/jwks.json
- Maintain backward compatibility with CLERK_PUB_JWT_URL env var
- Add comprehensive error handling and security validation
- Support both explicit URLs and auto-discovery
Benefits:
- Works seamlessly with different Clerk environments
- No need to manage multiple JWKS URLs
- Follows JWT/JWKS standards for auto-discovery
- More maintainable and flexible
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
dashboard/backend/create-handler.ts (1)
116-125: Don’t assume the first JWKS key; match kid or try all keys.Key rotation or multi-key JWKS will break with “first key” logic. Decode kid and prefer a match; otherwise try all keys.
Apply this diff:
- // Use the first key (standard practice for JWKS endpoints) - const jwsPubKey = jwksResponse.keys[0]; - - return (await verifyJwt(token, { key: jwsPubKey })) as unknown as ClerkTemplate; + const keys = jwksResponse.keys; + const { kid } = decodeProtectedHeader(token); + const candidates = kid ? keys.filter(k => k.kid === kid) : keys; + if (kid && candidates.length === 0) { + throw new Error(`No JWK with kid=${kid} found in JWKS from ${jwksUrl}`); + } + let lastErr: unknown; + for (const jwk of (candidates.length ? candidates : keys)) { + try { + return (await verifyJwt(token, { key: jwk })) as unknown as ClerkTemplate; + } catch (e) { + lastErr = e; + } + } + throw new Error(`verifyJwt failed for all JWKS keys from ${jwksUrl}: ${String(lastErr)}`);Add import:
import { decodeProtectedHeader } from "jose";
🧹 Nitpick comments (4)
dashboard/backend/create-handler.ts (4)
90-92: Use structured logger instead of console.log.Prefer your logger to keep logs consistent.
Apply this diff:
- console.log('🔍 Auto-discovered JWKS URL:', jwksUrl); + this.sthis.logger.Info().Str("jwksUrl", jwksUrl).Msg("Auto-discovered JWKS URL");
99-105: AbortSignal.timeout may not be available in all runtimes—add a fallback.Cloudflare Workers support can vary; a simple AbortController fallback is safer.
Apply this diff:
- const rJwtKey = await exception2Result( - async () => - await fetch(jwksUrl, { - method: "GET", - signal: AbortSignal.timeout(5000), // 5 second timeout - }), - ); + const rJwtKey = await exception2Result(async () => { + const ac = new AbortController(); + const to = setTimeout(() => ac.abort(), 5000); + try { + return await fetch(jwksUrl, { method: "GET", signal: ac.signal }); + } finally { + clearTimeout(to); + } + });
110-114: Avoid generic type arg on Response.json.Response.json isn’t generic in DOM types; cast the result instead to keep TS happy.
Apply this diff:
- const jwksResponse = await rJwtKey.Ok().json<{ keys: JsonWebKey[] }>(); + const jwksResponse = (await rJwtKey.Ok().json()) as { keys: JsonWebKey[] };
99-125: Optional: use a Remote JWKS with caching and kid handling.jose.createRemoteJWKSet + jwtVerify handles kid selection, caching, and timeouts for you; reduces custom code and fetches.
Example (outside this hunk):
import { createRemoteJWKSet, jwtVerify } from "jose"; const jwks = createRemoteJWKSet(new URL(jwksUrl), { timeoutDuration: 5000 }); await jwtVerify(token, jwks, { issuer: expectedIssuer /*, audience */ });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard/backend/create-handler.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
dashboard/backend/create-handler.ts (2)
use-fireproof/react/use-attach.ts (1)
token(87-114)dashboard/backend/cf-serve.ts (1)
fetch(12-37)
🪛 GitHub Actions: @fireproof/dashboard-deploy
dashboard/backend/create-handler.ts
[warning] 1-1: Prettier detected formatting issues in backend/create-handler.ts. Run 'prettier --write' to fix.
🪛 GitHub Actions: @fireproof/dashboard
dashboard/backend/create-handler.ts
[warning] 1-1: Code style issues found in backend/create-handler.ts. Run Prettier with --write to fix.
🔇 Additional comments (2)
dashboard/backend/create-handler.ts (2)
56-64: Static-key fast path looks good.Good early exit when CLERK_PUB_JWT_KEY is set.
56-126: Run Prettier to satisfy CI.Pipeline flagged formatting in this file. Please run Prettier with --write.
You can run:
- pnpm prettier --write dashboard/backend/create-handler.ts
dashboard/backend/create-handler.ts
Outdated
| const [, payloadB64] = token.split('.'); | ||
| if (!payloadB64) { | ||
| throw new Error("Invalid JWT format - missing payload"); | ||
| } | ||
| const rJwtKey = await exception2Result( | ||
| async () => | ||
| await fetch(CLERK_PUB_JWT_URL, { | ||
| method: "GET", | ||
| signal: AbortSignal.timeout(5000), // 5 second timeout | ||
| }), | ||
| ); | ||
| if (rJwtKey.isOk() && rJwtKey.Ok().ok) { | ||
| const rCt = await exception2Result(async () => { | ||
| const jwsPubKey = await rJwtKey.Ok().json<JsonWebKey>(); | ||
| return (await verifyJwt(token, { key: jwsPubKey })) as unknown as ClerkTemplate; | ||
| }); | ||
| if (rCt.isOk()) { | ||
| return rCt.Ok(); | ||
| } else { | ||
| throw new Error(`verifyJwt failed ${rCt.Err()} from ${CLERK_PUB_JWT_URL}`); | ||
| } | ||
|
|
||
| const payload = JSON.parse(atob(payloadB64)); | ||
| const issuer = payload.iss; | ||
|
|
There was a problem hiding this comment.
Fix JWT payload decoding: base64url ≠ base64 (atob will break).
JWT parts are base64url; atob expects base64 and will misdecode or throw. Use a base64url-aware decoder (e.g., jose.decodeJwt).
Apply this diff:
- const [, payloadB64] = token.split('.');
- if (!payloadB64) {
- throw new Error("Invalid JWT format - missing payload");
- }
-
- const payload = JSON.parse(atob(payloadB64));
- const issuer = payload.iss;
+ const { iss: issuer } = decodeJwt(token) as { iss?: string };
+ if (!issuer) {
+ throw new Error("JWT missing issuer (iss) field - cannot auto-discover JWKS URL");
+ }Add import:
import { decodeJwt } from "jose";🤖 Prompt for AI Agents
In dashboard/backend/create-handler.ts around lines 74 to 81, the code decodes
the JWT payload using atob on a base64url segment which is incorrect and can
misdecode or throw; replace the manual atob/JSON.parse approach with a
base64url-aware decoder such as jose.decodeJwt: import decodeJwt from "jose" (or
the named import) at top, call decodeJwt(token) to obtain the payload object,
and then read issuer from payload. Ensure you remove the atob/JSON.parse logic
and keep proper null/exception handling if decodeJwt throws.
dashboard/backend/create-handler.ts
Outdated
| if (!issuer) { | ||
| throw new Error("JWT missing issuer (iss) field - cannot auto-discover JWKS URL"); | ||
| } | ||
|
|
||
| if (!issuer.startsWith("https://")) { | ||
| throw new Error(`JWT issuer must use HTTPS: ${issuer}`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not accept arbitrary issuers; enforce an allowlist.
Auto-discovery based on untrusted iss widens trust to any issuer whose JWKS verifies. Require/validate an expected issuer from env.
Apply this diff after computing issuer:
+ const expectedIssuer = this.sthis.env.get("CLERK_ISSUER");
+ if (expectedIssuer && issuer !== expectedIssuer && !issuer.startsWith(expectedIssuer + "/")) {
+ throw new Error(`Unexpected issuer: ${issuer}`);
+ }Optionally also pass issuer/audience into verification if supported.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!issuer) { | |
| throw new Error("JWT missing issuer (iss) field - cannot auto-discover JWKS URL"); | |
| } | |
| if (!issuer.startsWith("https://")) { | |
| throw new Error(`JWT issuer must use HTTPS: ${issuer}`); | |
| } | |
| if (!issuer) { | |
| throw new Error("JWT missing issuer (iss) field - cannot auto-discover JWKS URL"); | |
| } | |
| if (!issuer.startsWith("https://")) { | |
| throw new Error(`JWT issuer must use HTTPS: ${issuer}`); | |
| } | |
| // Enforce allowlist: only accept the configured issuer (or its sub-paths) | |
| const expectedIssuer = this.sthis.env.get("CLERK_ISSUER"); | |
| if (expectedIssuer && issuer !== expectedIssuer && !issuer.startsWith(expectedIssuer + "/")) { | |
| throw new Error(`Unexpected issuer: ${issuer}`); | |
| } |
🤖 Prompt for AI Agents
In dashboard/backend/create-handler.ts around lines 82 to 88, currently any
issuer from the JWT is accepted for JWKS auto-discovery; update the code to read
an allowlist (or single expected issuer) from environment (e.g.
EXPECTED_JWT_ISSUERS or EXPECTED_JWT_ISSUER), parse it into a set, and after
computing issuer verify the issuer is present in that allowlist—if not, throw a
clear error rejecting the issuer; retain the HTTPS check and then, if the JWT
verification library supports it, pass the validated issuer (and configured
audience from env) into the verification call so only tokens for the expected
issuer/audience are accepted.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
dashboard/backend/create-handler.ts (5)
1-14: Import jose helpers for safe JWT decoding and header parsing.Needed for base64url-safe decode and kid handling.
+import { decodeJwt, decodeProtectedHeader } from "jose";
46-54: Plumb CLERK_ISSUER from env to allowlist issuers.Prepare for issuer validation below.
- const rEnvVal = this.sthis.env.gets({ - CLERK_PUB_JWT_KEY: param.OPTIONAL, - CLERK_PUB_JWT_URL: param.OPTIONAL, - }); + const rEnvVal = this.sthis.env.gets({ + CLERK_PUB_JWT_KEY: param.OPTIONAL, + CLERK_PUB_JWT_URL: param.OPTIONAL, + CLERK_ISSUER: param.OPTIONAL, + }); @@ - const { CLERK_PUB_JWT_KEY, CLERK_PUB_JWT_URL } = rEnvVal.Ok(); + const { CLERK_PUB_JWT_KEY, CLERK_PUB_JWT_URL, CLERK_ISSUER } = rEnvVal.Ok();
66-74: Fix JWT payload decoding: atob is wrong for base64url. Use jose.decodeJwt.Current code can throw or mis-decode; also improve error if iss missing.
- const [, payloadB64] = token.split("."); - if (!payloadB64) { - throw new Error("Invalid JWT format - missing payload"); - } - - const payload = JSON.parse(atob(payloadB64)); - const issuer = payload.iss; + const { iss: issuer } = decodeJwt(token) as { iss?: string }; + if (!issuer) { + throw new Error("JWT missing issuer (iss) field - cannot auto-discover JWKS URL"); + }
75-85: Enforce an issuer allowlist to avoid trusting arbitrary issuers.Without this, any token whose iss hosts a JWKS could be accepted.
let jwksUrl: string; - if (issuer && issuer.startsWith("https://")) { + if (issuer && issuer.startsWith("https://")) { + if (CLERK_ISSUER && issuer !== CLERK_ISSUER && !issuer.startsWith(`${CLERK_ISSUER}/`)) { + throw new Error(`Unexpected issuer: ${issuer}`); + } // Use issuer from JWT (preferred) jwksUrl = `${issuer}/.well-known/jwks.json`;
108-112: Don’t assume the first JWKS key; match kid or try all keys.Prevents failures during rotation/multi-key JWKS.
- // Use the first key (standard practice for JWKS endpoints) - const jwsPubKey = jwksResponse.keys[0]; - - return (await verifyJwt(token, { key: jwsPubKey })) as unknown as ClerkTemplate; + const keys = jwksResponse.keys; + const { kid } = decodeProtectedHeader(token) as { kid?: string }; + const candidates = kid ? keys.filter(k => k.kid === kid) : keys; + if (kid && candidates.length === 0) { + throw new Error(`No JWK with kid=${kid} found in JWKS from ${jwksUrl}`); + } + let lastErr: unknown; + for (const jwk of (candidates.length ? candidates : keys)) { + try { + return (await verifyJwt(token, { key: jwk })) as unknown as ClerkTemplate; + } catch (e) { + lastErr = e; + } + } + throw new Error(`verifyJwt failed for all JWKS keys from ${jwksUrl}: ${String(lastErr)}`);
🧹 Nitpick comments (4)
dashboard/backend/create-handler.ts (4)
91-97: Timeout portability: provide fallback if AbortSignal.timeout is unavailable.Avoid runtime errors in environments missing AbortSignal.timeout.
- await fetch(jwksUrl, { - method: "GET", - signal: AbortSignal.timeout(5000), // 5 second timeout - }), + await (async () => { + const signal = + "timeout" in AbortSignal + ? // @ts-expect-error: runtime check + AbortSignal.timeout(5000) + : (() => { + const c = new AbortController(); + setTimeout(() => c.abort(), 5000); + return c.signal; + })(); + return fetch(jwksUrl, { method: "GET", signal }); + })(),
120-121: Include HTTP status in JWKS fetch error for faster triage.- throw new Error(`Failed to fetch JWKS from ${jwksUrl}`); + if (rJwtKey.isOk()) { + const res = rJwtKey.Ok(); + throw new Error(`Failed to fetch JWKS from ${jwksUrl} (status ${res.status})`); + } + throw new Error(`Failed to fetch JWKS from ${jwksUrl}: ${String(rJwtKey.Err())}`);
127-134: Set provider to "clerk" instead of "TBD".- provider: "TBD", + provider: "clerk",
91-112: Cache JWKS per issuer with a short TTL.Reduces latency and rate-limit risk; refresh on kid-miss or signature failure.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard/backend/create-handler.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
dashboard/backend/create-handler.ts (1)
use-fireproof/react/use-attach.ts (1)
token(87-114)
🔇 Additional comments (1)
dashboard/backend/create-handler.ts (1)
56-65: Static key fast-path LGTM.Short-circuits correctly when CLERK_PUB_JWT_KEY is present.
Summary
{"keys":[{...}]}but code was trying to use entire response as single JWKChanges
{keys: JsonWebKey[]}Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements