[INJIVER-1531] Added verifyV2() and verifyAndGetCredentialStatusV2() in PresentationVerifier #223
Conversation
…wing error handling Signed-off-by: jaswanthkumarpolisetty <[email protected]>
WalkthroughAdds V2 presentation verification APIs and V2 data models; refactors PresentationVerifier to route V1 through V2 helpers; implements VC verification and credential-status–aware V2 flows; expands error handling and test coverage for V2 while keeping existing V1 entry points. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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: 4
🤖 Fix all issues with AI agents
In
`@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt`:
- Around line 258-268: The method verifyAndGetCredentialStatusV2 currently
assumes Shared.KEY_VERIFIABLE_CREDENTIAL exists and will throw a JSONException
like verifyV2; update verifyAndGetCredentialStatusV2 to defensively check
JSONObject(presentation) for Shared.KEY_VERIFIABLE_CREDENTIAL before calling
getJSONArray and handle the missing-key case (e.g., set verifiableCredentials to
an empty JSONArray or return an appropriate
PresentationResultWithCredentialStatusV2 with empty vc results) so that
getVCVerificationResultsWithCredentialStatusV2 is only called when the key is
present.
- Around line 57-65: In verifyV2, add defensive handling for a missing
verifiableCredential key so getJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) does
not throw; after calling getPresentationVerificationStatusV2(presentation) check
JSONObject(presentation).has(Shared.KEY_VERIFIABLE_CREDENTIAL) (or use
optJSONArray) and if absent return a PresentationVerificationResultV2 using the
computed presentationVerificationStatus and an empty vcVerificationResults list
instead of calling getVCVerificationResultsV2; if present continue to call
getVCVerificationResultsV2(verifiableCredentials) as before.
- Around line 98-132: getPresentationVerificationStatusV2 currently throws
PresentationNotSupportedException and misses handlers for
SignatureNotSupportedException, IllegalStateException, and
InvalidKeySpecException from verifyPresentationProof; change the initial
try/catch so it returns a VerificationResult (e.g., false, "Unsupported VP token
type", "VP_UNSUPPORTED_FORMAT") instead of throwing
PresentationNotSupportedException, and add catch blocks for
SignatureNotSupportedException, IllegalStateException, and
InvalidKeySpecException that each return a descriptive VerificationResult with
an appropriate error message and unique code (follow the pattern used for
PublicKeyNotFoundException/UnsupportedDidUrl/SignatureVerificationException);
reference getPresentationVerificationStatusV2 and verifyPresentationProof when
locating where to add these returns.
In
`@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt`:
- Around line 211-216: The test currently expects
PresentationVerifier().verifyV2("invalid") to throw
PresentationNotSupportedException, which conflicts with the PR goal of
non-throwing V2 verification; update the test to call the non-throwing method
(getPresentationVerificationStatusV2 or ensure verifyV2 returns a
VerificationResult) and assert that the returned VerificationResult contains the
expected error code/status (e.g., PRESENTATION_NOT_SUPPORTED or equivalent)
instead of using assertThrows; reference PresentationVerifier.verifyV2 and/or
PresentationVerifier.getPresentationVerificationStatusV2 and the
VerificationResult error/status field in your assertion.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt
Show resolved
Hide resolved
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt
Outdated
Show resolved
Hide resolved
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt
Show resolved
Hide resolved
...fier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt
Show resolved
Hide resolved
Signed-off-by: jaswanthkumarpolisetty <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt`:
- Around line 198-204: The test is using the V1 throwing API; change it to call
the V2 non‑throwing API by invoking PresentationVerifier().verifyV2(vp), capture
the returned result, and assert that the result indicates an error of type
UnsupportedDidUrl (i.e., the result is a failure/error and the contained error
equals/instanceof UnsupportedDidUrl). Keep the same test setup (vp from
readClasspathFile) and timeout, but replace the assertThrows<UnsupportedDidUrl>
with an assertion on the verifyV2 result to verify the UnsupportedDidUrl error.
♻️ Duplicate comments (3)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt (2)
98-137: V2 verification still throws instead of returningVerificationResult.
getPresentationVerificationStatusV2throwsPresentationNotSupportedExceptionand rethrows key exceptions, soverifyV2remains throwing. To match the V2 contract, map all error paths toVerificationResult(e.g.,UNSUPPORTED_DID,PUBLIC_KEY_NOT_FOUND,SIGNATURE_NOT_SUPPORTED,INVALID_KEY_SPEC,ILLEGAL_STATE, andVP_UNSUPPORTED_FORMAT) instead of throwing.🔧 Sketch of the expected control flow
- try { - vcJsonLdObject = JsonLDObject.fromJson(presentation) - } catch (e: RuntimeException) { - throw PresentationNotSupportedException("Unsupported VP Token type") - } - return try { + return try { + val vcJsonLdObject = JsonLDObject.fromJson(presentation) val isVerified = verifyPresentationProof(vcJsonLdObject) ... - } catch (e: Exception) { ... throw e / UnknownException ... } + } catch (e: PublicKeyNotFoundException) { + VerificationResult(false, e.message ?: "Public key not found", "PUBLIC_KEY_NOT_FOUND") + } catch (e: UnsupportedDidUrl) { + VerificationResult(false, e.message ?: "Unsupported DID", "UNSUPPORTED_DID") + } catch (e: SignatureNotSupportedException) { + VerificationResult(false, e.message ?: "Signature not supported", "SIGNATURE_NOT_SUPPORTED") + } catch (e: InvalidKeySpecException) { + VerificationResult(false, e.message ?: "Invalid key spec", "INVALID_KEY_SPEC") + } catch (e: IllegalStateException) { + VerificationResult(false, e.message ?: "Illegal state", "ILLEGAL_STATE") + } catch (e: SignatureVerificationException) { + VerificationResult(false, e.message ?: "Signature verification error", "SIGNATURE_VERIFICATION_EXCEPTION") + } catch (e: RuntimeException) { + VerificationResult(false, "Unsupported VP token type", "VP_UNSUPPORTED_FORMAT") + }
57-65: Guard JSON access to keep V2 non‑throwing.
JSONObject(...).getJSONArray(...)throws if the key is absent or the input isn’t valid JSON, which breaks the non‑throwing V2 contract. PreferoptJSONArrayand safe parsing, returning an empty VC list when unavailable. Please confirmoptJSONArraybehavior in the org.json version used.🔧 Suggested pattern (apply to both V2 entry points)
- val verifiableCredentials = JSONObject(presentation).getJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) - val vcVerificationResults: List<VCResultV2> = getVCVerificationResultsV2(verifiableCredentials) + val jsonObject = runCatching { JSONObject(presentation) }.getOrNull() + val verifiableCredentials = jsonObject?.optJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) + val vcVerificationResults: List<VCResultV2> = + verifiableCredentials?.let { getVCVerificationResultsV2(it) } ?: emptyList()Also applies to: 264-274
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
207-212: Don’t assert exceptions for V2 non‑JSON‑LD input.This conflicts with the PR objective of non‑throwing V2 verification; assert the returned error result instead.
🔧 Proposed update
- fun `V2 should throw error when VP is not JSON-LD`() { - assertThrows<PresentationNotSupportedException> { - PresentationVerifier().verifyV2("invalid") - } - } + fun `V2 should return error when VP is not JSON-LD`() { + val result = PresentationVerifier().verifyV2("invalid") + assertFalse(result.proofVerificationResult.verificationStatus) + assertEquals("VP_UNSUPPORTED_FORMAT", result.proofVerificationResult.verificationErrorCode) + }
...fier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt
Show resolved
Hide resolved
Signed-off-by: jaswanthkumarpolisetty <[email protected]>
Signed-off-by: jaswanthkumarpolisetty <[email protected]>
…ests Signed-off-by: jaswanthkumarpolisetty <[email protected]>
Signed-off-by: jaswanthkumarpolisetty <[email protected]>
Signed-off-by: jaswanthkumarpolisetty <[email protected]>
…wing error handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.