fix(cacti-plugin-consortium-static): add jti replay protection to verifyOrganization#4373
Conversation
RafaelAPB
left a comment
There was a problem hiding this comment.
Thank you for your contribution
There was a problem hiding this comment.
Pull request overview
This PR strengthens JWT verification in @hyperledger/cacti-plugin-consortium-static by adding optional JWT ID (jti) replay protection to verifyOrganization(), wiring it into StaticConsortiumRepository, and adding unit tests for the new behavior.
Changes:
- Extended
verifyOrganization()with an optionalseenJtiscache to reject replayed tokens based onjti. - Updated
StaticConsortiumRepository.verifyJWT()to maintain and pass a replay-cache across verifications. - Added unit tests covering valid, replayed, wrong issuer, expired, and “no cache provided” cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/cacti-plugin-consortium-static/src/main/typescript/utils.ts | Adds optional jti replay tracking in verifyOrganization() and removes the previous manual expiry/error-string behavior. |
| packages/cacti-plugin-consortium-static/src/main/typescript/repository/static-consortium-repository.ts | Introduces repository-owned replay cache and passes it into verifyOrganization() for every verification. |
| packages/cacti-plugin-consortium-static/src/test/typescript/unit/verify-organization.test.ts | Adds unit coverage for first-use acceptance, replay rejection, wrong issuer, expired token, and backwards-compatible mode without replay tracking. |
a92e08a to
5e9fdf3
Compare
|
Hi @VRamakrishna @sandeepnRES @outSH @AndreAugusto11 — just a gentle ping. The Copilot review comments have been addressed (force-pushed Jun 10) and RafaelAPB has approved. Would appreciate a second review when you get a chance. Happy to make any changes if needed! |
AndreAugusto11
left a comment
There was a problem hiding this comment.
LGMT! @bhavyamsharmaa thank you very much for the contribution!
45a8c38 to
55880a1
Compare
|
@RafaelAPB — could you approve the workflow runs so CI can proceed? |
There was a problem hiding this comment.
Please add these files to gitignore; in fact, the cache should not be in version control
- Add optional seenJtis parameter to verifyOrganization to track used JWT IDs - StaticConsortiumRepository owns the Map and passes it on each verifyJWT call - Reject tokens missing exp claim; jwtVerify does not require exp to be present - Namespace jti cache key as iss:jti to prevent cross-org collisions - Replace unbounded Set with Map<string, number> storing jti -> exp; prune expired entries on each verify call - Remove buggy manual expiry check (payload.exp seconds vs Date.now() ms mismatch); jose's jwtVerify already throws JWTExpired for expired tokens - Remove console.error in catch block; return false for all failures - Add unit tests: first-use acceptance, replay rejection, wrong issuer, expired token, no-exp token, cross-org same-jti, no-seenJtis mode Closes hyperledger-cacti#4372 Assisted-by: anthropic:claude-sonnet-4-6 Signed-off-by: Bhavyam Sharma <positionbhavyamsharma@gmail.com>
55880a1 to
8a4181c
Compare
|
@RafaelAPB, Done — removed all five @iroha2 cache zips and reverted the .gitignore change. The cache is no longer in version control. |
Summary
seenJtis: Set<string>parameter toverifyOrganization— if provided, a token whosejtihas already been seen is rejected, preventing replay attacksStaticConsortiumRepositoryowns theSetas private state and passes it on everyverifyJWTcallpayload.expin seconds vsDate.now()in milliseconds);jose'sjwtVerifyalready throwsJWTExpiredfor expired tokens, making the manual check both dead code and incorrectconsole.errorin the catch block; all verification failures now returnfalseuniformlyseenJtismodeCloses #4372
PR Checklist
upstream/mainand squashed into a single commitSigned-off-bypresent (git commit -s)Assisted-by: anthropic:claude-sonnet-4-6included per AI Guidelines §2.2