add kid to custom JWT header#279
Conversation
|
@fabiorizzomatos is attempting to deploy a commit to the Convex Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes add optional JWKS (JSON Web Key Set) support to JWT token generation, extracting a key ID (kid) from JWKS and conditionally including it in the JWT protected header. When JWKS is unavailable or invalid, the system safely falls back to the existing RS256 signing behavior without kid. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
🤖 Fix all issues with AI agents
In `@src/server/implementation/tokens.ts`:
- Around line 17-23: The current code silently parses JWKS and picks
jwks.keys[0] into the variable kid, which is unsafe when JWKS is present but
contains multiple keys or is invalid; change tokens.ts to fail fast when JWKS is
set: if requireEnv("JWKS") returns a value, validate that it parses to an object
with a non-empty keys array and deterministically select the signing key by
reading an explicit requireEnv("JWT_KID") (or error if JWT_KID is missing) and
ensure that a matching jwks.keys[].kid exists; if parsing fails or the requested
kid is absent, throw/log an explicit error instead of swallowing it so
misconfiguration is surfaced.
| let kid: string | undefined; | ||
| try { | ||
| const jwks = JSON.parse(requireEnv("JWKS")); | ||
| kid = jwks?.keys?.[0]?.kid; | ||
| } catch { | ||
| // ignore if JWKS is missing or invalid | ||
| } |
There was a problem hiding this comment.
Fail fast when JWKS is set but doesn’t deterministically map to the signing key.
Selecting jwks.keys[0] is unstable when JWKS has multiple keys (rotation) and may not match the private key used for signing, yielding tokens that consistently fail verification. Also, swallowing invalid JWKS silently hides misconfiguration. Prefer: only ignore when JWKS is absent; if present, validate it and require an explicit selection (e.g., JWT_KID) or error out when ambiguous.
🔧 Proposed fix (deterministic selection + validation)
- let kid: string | undefined;
- try {
- const jwks = JSON.parse(requireEnv("JWKS"));
- kid = jwks?.keys?.[0]?.kid;
- } catch {
- // ignore if JWKS is missing or invalid
- }
+ let kid: string | undefined;
+ const jwksEnv = process.env.JWKS;
+ if (jwksEnv) {
+ let jwks: any;
+ try {
+ jwks = JSON.parse(jwksEnv);
+ } catch {
+ throw new Error("JWKS env is not valid JSON");
+ }
+ if (!Array.isArray(jwks?.keys) || jwks.keys.length === 0) {
+ throw new Error("JWKS must contain at least one key with a kid");
+ }
+ if (jwks.keys.length > 1 && !process.env.JWT_KID) {
+ throw new Error("JWKS has multiple keys; set JWT_KID to select the signing key");
+ }
+ kid = process.env.JWT_KID ?? jwks.keys[0]?.kid;
+ if (!kid) {
+ throw new Error("Selected JWKS key is missing kid");
+ }
+ }📝 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.
| let kid: string | undefined; | |
| try { | |
| const jwks = JSON.parse(requireEnv("JWKS")); | |
| kid = jwks?.keys?.[0]?.kid; | |
| } catch { | |
| // ignore if JWKS is missing or invalid | |
| } | |
| let kid: string | undefined; | |
| const jwksEnv = process.env.JWKS; | |
| if (jwksEnv) { | |
| let jwks: any; | |
| try { | |
| jwks = JSON.parse(jwksEnv); | |
| } catch { | |
| throw new Error("JWKS env is not valid JSON"); | |
| } | |
| if (!Array.isArray(jwks?.keys) || jwks.keys.length === 0) { | |
| throw new Error("JWKS must contain at least one key with a kid"); | |
| } | |
| if (jwks.keys.length > 1 && !process.env.JWT_KID) { | |
| throw new Error("JWKS has multiple keys; set JWT_KID to select the signing key"); | |
| } | |
| kid = process.env.JWT_KID ?? jwks.keys[0]?.kid; | |
| if (!kid) { | |
| throw new Error("Selected JWKS key is missing kid"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/server/implementation/tokens.ts` around lines 17 - 23, The current code
silently parses JWKS and picks jwks.keys[0] into the variable kid, which is
unsafe when JWKS is present but contains multiple keys or is invalid; change
tokens.ts to fail fast when JWKS is set: if requireEnv("JWKS") returns a value,
validate that it parses to an object with a non-empty keys array and
deterministically select the signing key by reading an explicit
requireEnv("JWT_KID") (or error if JWT_KID is missing) and ensure that a
matching jwks.keys[].kid exists; if parsing fails or the requested kid is
absent, throw/log an explicit error instead of swallowing it so misconfiguration
is surfaced.
1 participant
Context
Convex requires JWTs for customJwt providers to include a
kidin the header so it can select the correct key from JWKS.Problem
@convex-dev/authgenerates access tokens withoutkid. Convex then fails auth with:Fix
Read JWKS from env and include the first key's
kidin the JWT protected header.Changes
src/server/implementation/tokens.tsSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.