feat(core,jose): add JWK support across core and jose packages#159
Merged
halvaradop merged 2 commits intomasterfrom Apr 28, 2026
Merged
feat(core,jose): add JWK support across core and jose packages#159halvaradop merged 2 commits intomasterfrom
halvaradop merged 2 commits intomasterfrom
Conversation
halvaradop
added
enhancement
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR extends JWT and JOSE operations to accept JWK-formatted cryptographic keys alongside existing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (3)
packages/jose/CHANGELOG.md (1)
13-14: Clarify the distinction from the CryptoKeyPair entry.The new JWK entry (line 13) and the existing CryptoKeyPair entry (line 15) list identical functions and use very similar wording, which may confuse readers about what differentiates them. The key distinction is that this PR extends the existing asymmetric cryptography support to also accept JWK-formatted keys as input, not that it adds asymmetric cryptography capabilities for the first time.
Consider rewording to make this clearer:
📝 Suggested rewording
-Added support to asymmetric cryptography using JWK key pairs across JOSE functions, including the dedicated `signJWS`, `verifyJWS`, `encryptJWE`, `decryptJWE`, `encryptCompactJWE`, `decryptCompactJWE`, `encodeJWT`, and `decodeJWT` functions, as well as the factory functions `createJWS`, `createJWE`, and `createJWT`. [`#159`](https://github.com/aura-stack-ts/auth/pull/159) +Extended asymmetric cryptography support to accept JWK (JSON Web Key) format keys in addition to `CryptoKeyPair` across JOSE functions, including the dedicated `signJWS`, `verifyJWS`, `encryptJWE`, `decryptJWE`, `encryptCompactJWE`, `decryptCompactJWE`, `encodeJWT`, and `decodeJWT` functions, as well as the factory functions `createJWS`, `createJWE`, and `createJWT`. [`#159`](https://github.com/aura-stack-ts/auth/pull/159)Alternatively, consider combining both entries into a single entry that mentions both
CryptoKeyPairand JWK format support were added together, if that better reflects the user-facing capability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/CHANGELOG.md` around lines 13 - 14, Update the CHANGELOG entry to clearly distinguish JWK input support from the existing CryptoKeyPair support: reword the JWK line so it states that the listed functions (signJWS, verifyJWS, encryptJWE, decryptJWE, encryptCompactJWE, decryptCompactJWE, encodeJWT, decodeJWT and the factories createJWS, createJWE, createJWT) now also accept JWK-formatted keys as input, rather than implying asymmetric cryptography was newly added; alternatively merge the two entries into one that explicitly notes both CryptoKeyPair and JWK format support were added for those functions.packages/core/src/shared/assert.ts (1)
130-132:isKeyPairduplicatesisCryptoKeyPairbehavior.Both guards currently use the same predicate (
publicKey+privateKeypresence), which makes intent and call sites harder to reason about. Consider consolidating into one guard (or making one a thin alias) to keep dispatch logic simpler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shared/assert.ts` around lines 130 - 132, The two type guards isKeyPair and isCryptoKeyPair duplicate the same predicate (checking for publicKey and privateKey), so consolidate them: keep a single implementation (choose one name, e.g., isCryptoKeyPair) that performs the object/null + "publicKey" in value + "privateKey" in value check, and make the other function a thin alias that returns that call (or remove it and update call sites to the retained name); update exports so only one implementation contains the logic and the other simply delegates to it to preserve compatibility.packages/jose/src/assert.ts (1)
28-30: Consider validating key-pair member shapes, not only field presence.This guard currently accepts any
{ publicKey, privateKey }object. A lightweight nested validation (CryptoKeyor JWK-shaped object) would reduce deferred runtime errors in downstream JOSE operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/src/assert.ts` around lines 28 - 30, The type guard isAsymmetricKeyPair currently only checks for the presence of publicKey/privateKey fields; update it to perform lightweight shape validation of those members (e.g., verify each is a CryptoKey-like object or a minimal JWK-like shape) to avoid accepting arbitrary objects. Implement or reuse a helper (e.g., isCryptoKey or isJwkShape) that checks member properties such as typeof === "object" && member !== null and either instanceOf CryptoKey (if available) or presence of minimal JWK fields like "kty" and "crv"/"alg" as appropriate, then call that helper for both publicKey and privateKey inside isAsymmetricKeyPair so the guard returns true only when both members match the expected key shape. Ensure helpers and type predicate signatures align with AsymmetricKeyPair so downstream code gets proper narrowed types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/`@types/session.ts:
- Line 6: Change the JWK import to a type-only import since JWK is used only in
type positions: replace the current import of JWK (symbol JWK) with an "import
type" form to match other type imports in this file (session-related types
referenced on lines where JWK appears such as interfaces using JWK). Ensure no
runtime import is introduced so the module only imports JWK for type-checking.
In `@packages/core/src/shared/assert.ts`:
- Line 11: The import for JWK should be a type-only import to avoid runtime
export inconsistencies: change the runtime import "import { JWK } from
'@aura-stack/jose/jose'" to a type import ("import type { JWK } ...") so JWK is
treated purely as a TypeScript type used in the type predicate (the JWK
reference around line 173) and no runtime module export is required; update the
import declaration where JWK is brought in and leave all usages (including the
type predicate) unchanged.
In `@packages/jose/src/assert.ts`:
- Around line 32-34: The isJWKKey type guard currently treats any object with a
kty property as a JWK; update the isJWKKey function to first ensure value is a
non-null object and then verify that value.kty exists and is a string (e.g.,
check typeof (value as any).kty === "string") so malformed non-string kty values
don't pass; adjust the return predicate in isJWKKey accordingly (reference
function name isJWKKey and the JsonWebKey type).
In `@packages/jose/src/deriveKey.ts`:
- Around line 67-68: The thrown KeyDerivationError message in deriveKey.ts is
stale: the branch checks "secretKey instanceof CryptoKey || isJWKKey(secretKey)"
but the message only mentions CryptoKey; update the KeyDerivationError thrown by
this conditional (referencing secretKey, isJWKKey, and KeyDerivationError in
deriveKey.ts) to mention both CryptoKey and JWK inputs (e.g., "Cannot derive key
from CryptoKey or JWK. Use Uint8Array or string secret instead.") so the error
accurately reflects the rejected types.
In `@packages/jose/src/secret.ts`:
- Line 54: The InvalidSecretError message thrown in createSecret is outdated: it
mentions AsymmetricKeyPair while createSecret now accepts JWKs detected via
isJWKKey(secret). Update the error text in the throw (InvalidSecretError) to
list the actual accepted types (e.g., "Secret must be a string, Uint8Array,
CryptoKey or JWK") or otherwise include JWK so the message matches the
createSecret and isJWKKey(secret) logic.
---
Nitpick comments:
In `@packages/core/src/shared/assert.ts`:
- Around line 130-132: The two type guards isKeyPair and isCryptoKeyPair
duplicate the same predicate (checking for publicKey and privateKey), so
consolidate them: keep a single implementation (choose one name, e.g.,
isCryptoKeyPair) that performs the object/null + "publicKey" in value +
"privateKey" in value check, and make the other function a thin alias that
returns that call (or remove it and update call sites to the retained name);
update exports so only one implementation contains the logic and the other
simply delegates to it to preserve compatibility.
In `@packages/jose/CHANGELOG.md`:
- Around line 13-14: Update the CHANGELOG entry to clearly distinguish JWK input
support from the existing CryptoKeyPair support: reword the JWK line so it
states that the listed functions (signJWS, verifyJWS, encryptJWE, decryptJWE,
encryptCompactJWE, decryptCompactJWE, encodeJWT, decodeJWT and the factories
createJWS, createJWE, createJWT) now also accept JWK-formatted keys as input,
rather than implying asymmetric cryptography was newly added; alternatively
merge the two entries into one that explicitly notes both CryptoKeyPair and JWK
format support were added for those functions.
In `@packages/jose/src/assert.ts`:
- Around line 28-30: The type guard isAsymmetricKeyPair currently only checks
for the presence of publicKey/privateKey fields; update it to perform
lightweight shape validation of those members (e.g., verify each is a
CryptoKey-like object or a minimal JWK-like shape) to avoid accepting arbitrary
objects. Implement or reuse a helper (e.g., isCryptoKey or isJwkShape) that
checks member properties such as typeof === "object" && member !== null and
either instanceOf CryptoKey (if available) or presence of minimal JWK fields
like "kty" and "crv"/"alg" as appropriate, then call that helper for both
publicKey and privateKey inside isAsymmetricKeyPair so the guard returns true
only when both members match the expected key shape. Ensure helpers and type
predicate signatures align with AsymmetricKeyPair so downstream code gets proper
narrowed types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2aff55ee-3b62-411e-8cc6-96c926629b42
📒 Files selected for processing (13)
packages/core/src/@types/session.tspackages/core/src/jose.tspackages/core/src/shared/assert.tspackages/core/src/shared/crypto.tspackages/core/test/jose.test.tspackages/jose/CHANGELOG.mdpackages/jose/src/assert.tspackages/jose/src/deriveKey.tspackages/jose/src/encrypt.tspackages/jose/src/index.tspackages/jose/src/secret.tspackages/jose/src/sign.tspackages/jose/test/index.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests