[INJIVER-1523] Fixed LSH comments - updated ReadMe, added design doc, fixed Code Rabbit comments from PR 230 for CWT#232
Conversation
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
|
Warning Rate limit exceeded
⌛ 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. WalkthroughAdds a README table entry for CWT_VC, a new design/spec doc for CWT-VC verification, and updates Kotlin build deps to replace a COSE library with UPoKe CBOR; a small error-handling tweak in JWKS resolver was also made. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Factory as CredentialVerifierFactory
participant Validator as CwtValidator
participant Verifier as CwtVerifier
participant Resolver as IssuerPublicKeyResolver
participant Issuer as Issuer (HTTP(S))
Client->>Factory: submit CWT_VC credential
Factory->>Validator: select validator for CWT_VC
Validator->>Verifier: validate COSE_Sign1 structure & CWT claims
Verifier->>Resolver: fetch issuer public key (/.well-known/jwks.json)
Resolver->>Issuer: GET public key
Issuer-->>Resolver: return key
Resolver-->>Verifier: deliver key
Verifier->>Verifier: verify COSE_Sign1 signature & numeric dates
Verifier-->>Validator: verification result
Validator-->>Factory: validation outcome
Factory-->>Client: final verification response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@doc/cwt-vc-verification-support.md`:
- Around line 28-39: The diagram title claims "validate and verify `cwt-vc`
credential" but the mermaid sequence shows only validation steps; either rename
the heading to "validate `cwt-vc` credential" or extend the sequence to include
explicit verification interactions (e.g., after validation add messages such as
CredentialsVerifier->>TrustResolver: Resolve trust,
CredentialsVerifier->>SignatureVerifier: Verify signature,
SignatureVerifier-->>CredentialsVerifier: Verification result, then
CredentialsVerifier-->>Wallet: Verification result) by updating the Mermaid
actors (Wallet, CredentialsVerifier, CredentialVerifierFactory,
CwtVerifiableCredential, CwtValidator) to include the verification flow so the
title matches the diagram content.
- Around line 85-87: Update the sequence/diagram text to avoid implying
duplicate structure validation: change the CwtVerifier steps so they indicate
the COSE_Sign1 structure has already been validated (e.g., "Extract COSE_Sign1
object" -> "Extract COSE_Sign1 object (already validated by CwtValidator)") and
remove or relabel the "Validate COSE_Sign1 structure" step; reference
CwtValidator.validate() as the place where structure and claims are checked and
keep CwtVerifier.verify() focused on signature verification only.
… com.upokecenter.cbor Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/jwks/JwksPublicKeyResolver.kt`:
- Around line 32-34: In JwksPublicKeyResolver's catch block that currently
rethrows exceptions (catching Exception and throwing
PublicKeyNotFoundException), preserve the original exception by creating the new
PublicKeyNotFoundException and calling initCause(e) on it before throwing;
specifically, replace the current conditional throw with constructing a new
PublicKeyNotFoundException("Failed to resolve JWKS public key: ${e.message}"),
invoke initCause(e) on that instance, and then throw it, while still rethrowing
the original e when it is already a PublicKeyNotFoundException.
...er/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/jwks/JwksPublicKeyResolver.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Summary by CodeRabbit
New Features
Documentation
Chores
Bug Fixes