Support signing and verifying token using a JWK#692
Merged
Conversation
933099f to
2bbf6bc
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for signing and verifying JWT tokens using a JWK, refactoring the signing and verification workflow to accommodate both string keys and JWK objects. Key changes include:
- Updated implementation of sign! and verification methods to support JWKs.
- New tests covering token signing and signature verification using JWKs.
- Documentation updates in README and CHANGELOG to reflect the new JWK support.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/jwt/token_spec.rb | Added tests for signing with JWK and ensuring proper error handling. |
| spec/jwt/jwk/rsa_spec.rb | Extended RSA JWK tests to cover various scenarios (valid, missing/invalid alg). |
| spec/jwt/jwk/ec_spec.rb | Added EC JWK tests verifying behavior when alg parameter is present or absent. |
| spec/jwt/encoded_token_spec.rb | Introduced tests for signature verification including error cases and JWK-specific usage. |
| spec/integration/readme_examples_spec.rb | Provided integration examples of JWK usage in token signing/verification. |
| lib/jwt/token.rb | Modified sign! method signature and implementation for JWK support. |
| lib/jwt/jwk/key_base.rb | Added helper methods for signing and verifying using JWK instances. |
| lib/jwt/jwk/ec.rb | Adjusted EC key resolution when the alg parameter is missing. |
| lib/jwt/jwa/ecdsa.rb | Minor documentation update for curve resolution. |
| lib/jwt/jwa.rb | Introduced new SignerContext and VerifierContext classes and refactored algorithm resolution. |
| lib/jwt/encoded_token.rb | Updated signature verification methods to support both key and key_finder. |
| README.md | Updated documentation with a JWK signing/verification example. |
| CHANGELOG.md | Recorded the new JWK support feature. |
Comments suppressed due to low confidence (2)
lib/jwt/token.rb:94
- The new parameter ordering in the sign! method may impact existing usages. Please ensure that both the documentation and any dependent code are updated to reflect that algorithm is now optional and that a nil algorithm with a non-JWK key is handled appropriately.
def sign!(key:, algorithm: nil)
lib/jwt/encoded_token.rb:154
- [nitpick] The check for providing exactly one of key or key_finder is clear, but consider adding a brief inline comment or updating the method's documentation to clarify this requirement for future maintainers.
raise ArgumentError, 'Provide either key or key_finder, not both or neither' if key.nil? == key_finder.nil?
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
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.
Description
This implement the idea in #400, using the
alg/crvparameter to resolve the algorithm and use that for signing and verifying the tokens.Checklist
Before the PR can be merged be sure the following are checked: