[INJIVER-1369] - Add verification and validation for cwt formet VC #215
Conversation
WalkthroughIntroduces CWT (CBOR Web Token) credential support and refactors the credential verification system to replace legacy revocation checking with a comprehensive credential status checking mechanism that retrieves and validates status lists, returning detailed per-purpose status results. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CV as CredentialsVerifier
participant VC as VerifiableCredential
participant SC as StatusChecker<br/>(LdpStatusChecker)
participant HTTP as HTTP Get
C->>CV: verifyAndGetCredentialStatus(vc, format, purposes)
CV->>VC: verify(credential)
VC->>VC: validate() & verify()
alt Verification Success
VC-->>CV: true
CV->>SC: getStatuses(credential, purposes)
SC->>SC: parse credentialStatus
SC->>HTTP: fetch statusListCredential
HTTP-->>SC: statusListVC
SC->>SC: validate statusListVC signature
SC->>SC: decode & decompress encodedList
SC->>SC: extract bit at statusListIndex
SC-->>CV: Map<purpose, CredentialStatusResult>
CV-->>C: CredentialVerificationSummary<br/>(success, statusMap)
else Verification Failed
VC-->>CV: false
CV-->>C: CredentialVerificationSummary<br/>(failure, empty)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.kt (1)
55-56: Remove trailing blank line.The constant definition is correct and follows the existing pattern. However, Line 56 contains an unnecessary blank line.
🔎 Proposed fix
- const val ERROR_MESSAGE_EMPTY_VC_CWT="${VALIDATION_ERROR}Input VC CWT string is null or empty." - + const val ERROR_MESSAGE_EMPTY_VC_CWT="${VALIDATION_ERROR}Input VC CWT string is null or empty."vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt (1)
14-16: Remove unnecessary semicolons for idiomatic Kotlin.Kotlin does not require semicolons at the end of statements.
🔎 Proposed fix
override fun verify(credential: String): Boolean { - return CwtVerifer().verify(credential); + return CwtVerifier().verify(credential) } override fun checkStatus(credential: String, statusPurposes: List<String>?): List<CredentialStatusResult>? { - return null; + return null }Also applies to: 18-20
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
162-176: Consider throwing a domain-specific exception for consistency.The
requireon line 171 throwsIllegalArgumentException, which will be caught byCwtValidator's genericExceptionhandler and mapped toERROR_CODE_GENERIC. Consider throwingValidationExceptiondirectly for more precise error handling and consistent error codes.🔎 Proposed alternative
fun validateNumericDate( claims: CBORObject, key: CBORObject, name: String ): Long? { if (!claims.ContainsKey(key)) return null val value = claims[key] - require(value.type == CBORType.Integer) { - "$name must be a CBOR integer (NumericDate)" + if (value.type != CBORType.Integer) { + throw IllegalArgumentException("$name must be a CBOR integer (NumericDate)") } return value.AsInt64() }Alternatively, make this function throw
ValidationExceptiondirectly if validation-specific error handling is needed.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
119-119: Extract CBOR claim keys as companion object constants.
ALG,EXP,NBF, andIATare created as local variables on each validation call. Extract these as class-level constants to avoid repeated object creation.🔎 Proposed fix
class CwtValidator { companion object { private val ALG = CBORObject.FromObject(1) private val EXP = CBORObject.FromObject(4) private val NBF = CBORObject.FromObject(5) private val IAT = CBORObject.FromObject(6) } // ... rest of the class }Also applies to: 159-161
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
26-28: Remove unused code.
cborMapper(lines 26-28) is never usedheader1andheaderValue(lines 127-128) are extracted but never used🔎 Proposed fix
- private val cborMapper = ObjectMapper(CBORFactory()) - .registerKotlinModule() -val payloadBytes = coseObj[2].GetByteString() - val header1 = coseObj[0].GetByteString() - val headerValue = CBORObject.DecodeFromBytes(header1) val claims = CBORObject.DecodeFromBytes(payloadBytes)Also applies to: 127-128
16-24: Consider dependency injection forRestTemplate.Creating
RestTemplateas an instance variable works but makes testing difficult. Consider injecting it as a constructor parameter or using a factory for better testability.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.ktvc-verifier/kotlin/gradle/libs.versions.tomlvc-verifier/kotlin/vcverifier/build.gradle.ktsvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialFormat.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/CredentialVerifierFactory.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.kt
👮 Files not reviewed due to content moderation or server errors (1)
- vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-03T12:07:06.442Z
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
Applied to files:
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt
🧬 Code graph analysis (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
hexToBytes(153-160)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
decodeCose(193-196)
🪛 detekt (1.23.8)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt
[warning] 3-3: The class or object CwtValidatorTest is empty.
(detekt.empty-blocks.EmptyClassBlock)
🔇 Additional comments (5)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/CredentialVerifierFactory.kt (1)
4-4: LGTM!The CWT credential format integration follows the existing factory pattern correctly and is consistent with other credential format handlers.
Also applies to: 16-16
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialFormat.kt (1)
4-4: LGTM!The CWT_VC enum entry addition is correct and follows the existing naming convention.
vc-verifier/kotlin/gradle/libs.versions.toml (1)
23-24: Run tests for Jackson module compatibility, particularly Kotlin data class deserialization and CBOR format parsing.The Jackson upgrade from 2.14.0 to 2.17.2 introduces several behavior changes across the 2.15–2.17 releases. While your codebase uses only straightforward Jackson features (basic
ObjectMapper.readValue()with Map/TypeReference, Kotlin module, and CBOR format), verify these areas:
- Kotlin module: Test data class deserialization and constructor handling (Kotlin module refinements in 2.17 may affect behavior)
- CBOR parsing: Validate CBOR deserialization in
CwtVerifer.kt(CBOR format enhancements in 2.15+ may alter parsing behavior)- ObjectMapper usage: Confirm
readValue()calls inUtils.kt,DidWebPublicKeyResolver.kt,LdpRevokeChecker.kt, andSdJwtValidator.ktproduce correct resultsThe upgrade is safe for this codebase since it does not use polymorphic types (@JsonTypeInfo), enum annotations, JsonNode tree mutations, or large value streaming where breaking changes are more likely.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
153-160: LGTM!The
hexToBytesfunction correctly strips whitespace, validates even length, and converts hex pairs to bytes.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
169-174: The current implementation is correct per RFC 8392 (CWT) and RFC 7519 (JWT) specifications. The "exp" claim processing rule states: the current date/time MUST be before the value of "exp"; the token MUST NOT be accepted on or after the exp time. The conditionexp <= nowcorrectly implements this requirement—rejecting tokens when the current time equals or exceeds the expiration time. The suggested alternativeexp < nowwould violate the specification by incorrectly accepting tokens at the exact moment of expiration.
| cbor = "0.9" | ||
| identity = "20231002" | ||
| authleteSdJwt = "1.5" | ||
| augustcellars = "1.1.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the gradle libs.versions.toml file to see the full context
find . -name "libs.versions.toml" -path "*/vc-verifier/kotlin/*" -exec cat -n {} +Repository: mosip/vc-verifier
Length of output: 178
🌐 Web query:
augustcellars cose-java 1.1.0 Maven Central latest version
💡 Result:
The latest version of com.augustcellars.cose:cose-java on Maven Central is 1.1.0 (released Jan 27, 2020). [1][2]
Sources:
[1] Maven Central (repo1.maven.org) listing for com/augustcellars/cose/cose-java/1.1.0.
[2] MVNRepository entry for cose-java showing newest version 1.1.0.
🌐 Web query:
cose-java security vulnerabilities CVE
💡 Result:
Short answer — cose-java itself has no widely‑published CVE entries; reported CVEs are for its third‑party dependencies (notably Bouncy Castle). Examples reported for cose-java releases include CVE-2024-29857, CVE-2024-30171 and CVE-2025-8916 (these are Bouncy Castle issues flagged on Maven/metadata for com.augustcellars.cose:cose-java). [1][2][3][4]
Sources:
- cose-java package page (lists dependency CVEs). [1]
- NVD entry for CVE-2024-30171 (Bouncy Castle timing/leakage issue). [2]
- CVE-2024-29857 (Bouncy Castle EC curve CPU exhaustion). [3]
- CVE-2025-8916 (Bouncy Castle excessive-allocation / DoS). [4]
If you want, I can:
- list every CVE shown for a specific cose-java version, or
- check your project’s dependency tree and tell you which versions of bcprov/bcpkix are pulled in and whether they are vulnerable. Which would you prefer?
🌐 Web query:
augustcellars CBOR library versions security
💡 Result:
Short answer: AugustCellars itself does not publish a standalone "AugustCellars.CBOR" with known CVEs, but AugustCellars packages (notably Com.AugustCellars.COSE) use other CBOR libraries and have historically bumped the CBOR dependency for security reasons. Notable findings:
- Com.AugustCellars.COSE release notes state they updated to a newer CBOR (CBOR version 4.0) "due to a security parsing problem" (release 1.6.0). (nuget.org)
- Many .NET CBOR issues are in other libraries (e.g., PeterO.Cbor has documented denial‑of‑service vulnerabilities / GHSA advisories and CVEs). If you use AugustCellars packages that depend on PeterO.Cbor, check the PeterO.Cbor advisory/CVE for affected versions and upgrade accordingly. (cvedetails.com)
Recommendation: identify which AugustCellars NuGet packages you use (e.g., Com.AugustCellars.COSE, Com.AugustCellars.CoAP) and inspect their current package metadata to see which CBOR implementation and version they depend on; then verify that CBOR dependency against its advisories/CVEs and upgrade to the fixed versions. Example NuGet pages show the CBOR-related dependency history for Com.AugustCellars.COSE. (nuget.org)
If you want, tell me which AugustCellars package and version you use and I’ll list the exact CBOR dependency and any known vulnerabilities and fixed versions.
Citations:
- 1: https://www.nuget.org/packages/Com.AugustCellars.COSE/?utm_source=openai
- 2: https://www.cvedetails.com/cve/CVE-2024-21909/?utm_source=openai
- 3: https://www.nuget.org/packages/Com.AugustCellars.COSE/1.0.3?utm_source=openai
Verify Bouncy Castle transitive dependencies for known vulnerabilities.
Version 1.1.0 is the latest available on Maven Central. However, cose-java has transitive dependencies on Bouncy Castle (versions pulled by 1.1.0) which contain known security vulnerabilities: CVE-2024-29857 (EC curve CPU exhaustion), CVE-2024-30171 (timing/leakage), and CVE-2025-8916 (DoS). Audit the Bouncy Castle versions in your dependency tree and consider upgrading cose-java or managing Bouncy Castle transitive versions to mitigate these vulnerabilities.
| implementation(libs.jackson.dataformat.cbor) | ||
| implementation(libs.cose.java) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd vc-verifier/kotlin/vcverifier && cat build.gradle.kts | head -100Repository: mosip/vc-verifier
Length of output: 2626
🏁 Script executed:
find . -name "libs.versions.toml" -o -name "libs.versions.json" -o -name "gradle.properties" | head -20Repository: mosip/vc-verifier
Length of output: 146
🏁 Script executed:
cat ./vc-verifier/kotlin/gradle/libs.versions.tomlRepository: mosip/vc-verifier
Length of output: 4441
🏁 Script executed:
#!/bin/bash
echo "=== Checking jackson-dataformat-cbor latest version ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:com.fasterxml.jackson.dataformat+AND+a:jackson-dataformat-cbor&rows=1" | jq -r '.response.docs[0] | "Latest Version: \(.latestVersion)"'
echo ""
echo "=== Checking cose-java latest version ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:com.augustcellars.cose+AND+a:cose-java&rows=1" | jq -r '.response.docs[0] | "Latest Version: \(.latestVersion)"'
echo ""
echo "=== Checking jackson-core for version mismatch ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:com.fasterxml.jackson.core+AND+a:jackson-core&rows=1" | jq -r '.response.docs[0] | "Latest Version: \(.latestVersion)"'Repository: mosip/vc-verifier
Length of output: 147
🌐 Web query:
jackson-dataformat-cbor latest version 2024 2025
💡 Result:
Latest released version (as of Dec 24, 2025): jackson-dataformat-cbor 2.20.1. [1][2]
References:
- Maven Central listing (artifact directories). [1]
- mvnrepository artifact page showing 2.20.1. [2]
🌐 Web query:
cose-java latest version security vulnerabilities
💡 Result:
Summary — cose-java (com.augustcellars.cose)
- Latest released version: 1.1.0 (published Jan 27, 2020). [1][2]
- There are no public CVEs reported against the cose-java codebase itself; however repository metadata and artifact pages show reported vulnerabilities in its dependencies (examples surfaced by Maven/SCA databases): CVE-2025-8916, CVE-2024-30171, CVE-2024-29857 (these are dependency/packaging-level findings surfaced on artifact pages). [2][3]
- Common dependency issues referenced include older CBOR (upokecenter/com.upokecenter:cbor), BouncyCastle, and older cryptography helper libraries used by the project — these are the likely sources of the dependency CVE listings. [2][4]
Recommendations
- If you use com.augustcellars.cose, audit your build-time/runtime dependencies (CBOR, BouncyCastle, EdDSA libs) and upgrade those transitive libs to patched versions. [2][4]
- Consider using a maintained fork or alternate Java COSE implementations (e.g., se.digg.cose / cose‑lib) if you need more recent dependency updates and active maintenance. [5][6]
- Run your own SCA (OSS vulnerability scanner) against the exact artifact/version you use to get a complete, current list of vulnerabilities affecting your environment.
Sources
- Maven Central / Javadoc showing cose-java 1.1.0 as current. [https://javadoc.io/doc/com.augustcellars.cose/cose-java] [2]
- Maven Repository artifact page for com.augustcellars.cose:cose-java (1.1.0) — lists dependency CVEs and dependency versions. [https://mvnrepository.com/artifact/com.augustcellars.cose/cose-java/1.1.0] [3]
- mvnrepository pages for older cose-java versions (dependency vulnerability details). [https://mvnrepository.com/artifact/com.augustcellars.cose/cose-java/1.0.0] [4]
- se.digg.cose / cose-lib (maintained fork/alternative) information. [https://mvnrepository.com/artifact/se.digg.cose/cose-lib] [5]
If you want, I can run a focused SCA for the specific version you use (give me the exact groupId:artifactId:version) and return the full list of matching CVEs and recommended fixed versions.
🌐 Web query:
jackson 2.17.2 2.14.0 version compatibility security CVE
💡 Result:
Short answer: 2.17.2 is newer and fixes multiple security issues present in older 2.14.x releases — use 2.17.2 (or later) for better CVE coverage.
Details (high‑level):
- CVE-2023-35116 (DoS via crafted cyclic object) affects jackson-databind < 2.16.0, so 2.14.0 is affected and 2.17.2 is not. [1][2]
- Jackson project patch/release notes show 2.17.2 is a security/bugfix patch on top of the 2.17 line (includes fixes not present in 2.14). Upgrading to the latest 2.17.x (or newer stable) is recommended. [3]
- Many older deserialization/XXE/remote‑code‑execution CVEs historically impacted older jackson‑databind versions (examples documented by vendor/security advisories); staying on a maintained recent patch (2.17.2+ or later) reduces exposure. Verify your usage of features like DefaultTyping or polymorphic deserialization, which increase risk. [2][4]
Recommendations:
- Upgrade 2.14.0 → 2.17.2 (or preferably the latest 2.17+ / 2.20+ available) and test. [3][4]
- Audit uses of polymorphic typing/DefaultTyping, restrict class whitelist, and follow Jackson security guidance. (If you want, I can list exact CVEs that apply to 2.14.0 in your environment.)
Sources:
[1] MITRE/CVE-2023-35116 summary via CVE database.
[2] IBM/other security advisories referencing jackson-databind CVEs.
[3] Jackson Release 2.17.2 notes (FasterXML).
[4] CVEDetails / Snyk package vulnerability listings for Jackson versions.
Fix critical version mismatch and address unmaintained dependencies.
The versions require updates:
jackson-dataformat-cbor(2.17.2) is outdated; latest is 2.20.1. Upgrade recommended.cose-java(1.1.0) is no longer actively maintained (released Jan 2020) and has reported dependency vulnerabilities (CVE-2025-8916, CVE-2024-30171, CVE-2024-29857). Consider migrating to a maintained COSE library such asse.digg.cose:cose-lib.- CRITICAL: The build.gradle.kts forces
jackson-core:2.14.0, which is vulnerable to CVE-2023-35116 and incompatible with the other Jackson modules at 2.17.2. Remove the forced resolution and align all Jackson modules to 2.17.2 or upgrade to 2.20.1.
🤖 Prompt for AI Agents
In vc-verifier/kotlin/vcverifier/build.gradle.kts around lines 68-69, the
project includes outdated/unmaintained dependencies and forces a vulnerable
jackson-core version: update jackson-dataformat-cbor to the current release
(e.g., 2.20.1) and ensure all Jackson modules use the same 2.20.1 version
(remove any forced resolution to jackson-core:2.14.0), replace the unmaintained
cose-java dependency with a maintained COSE library such as
se.digg.cose:cose-lib (update coordinates and adapt code/usages accordingly),
and run a full build & tests to confirm no API mismatches and that the
CVE-vulnerable versions are removed.
...rc/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt
Outdated
Show resolved
Hide resolved
...ifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt
Show resolved
Hide resolved
...ifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt
Outdated
Show resolved
Hide resolved
...erifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt
Show resolved
Hide resolved
...erifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt
Show resolved
Hide resolved
...verifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt
Outdated
Show resolved
Hide resolved
...r/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt
Outdated
Show resolved
Hide resolved
...ier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 15
Fix all issues with AI Agents 🤖
In @README.md:
- Line 61: The markdown list item `statusChecker` is indented with 4 spaces
instead of 2; update the README.md list so the `statusChecker` entry uses
2-space indentation to match the other list items, preserving the backticks and
list marker.
In @vc-verifier/kotlin/README.md:
- Around line 394-411: Update the example JSON to reflect an invalid VC: set
verificationResult.verificationStatus to false, populate
verificationResult.verificationErrorCode with a suitable code like
"ERR_VC_EXPIRED", and adjust verificationResult.verificationMessage to clearly
state the VC is expired (and revoked if you want to mention both); keep
credentialStatus.revocation.isValid as false to indicate revocation.
- Around line 42-43: Update the README table entry that references the SD-JWT VC
draft for `vc+sd-jwt`, `dc+sd-jwt` so the link points to the latest spec (change
the URL from draft-ietf-oauth-sd-jwt-vc/10 to draft-ietf-oauth-sd-jwt-vc-13);
locate the table row containing the symbols `vc+sd-jwt`, `dc+sd-jwt` and replace
the old draft link with the new draft-ietf-oauth-sd-jwt-vc-13 URL (published
November 2025).
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt:
- Around line 189-192: Remove the duplicate hexToBytes implementation from
CwtVerifer.kt and use the shared utility from Util.kt instead: delete the local
hexToBytes function in CwtVerifer.kt, add an import for the Util.hexToBytes (or
adjust package-import to access Util), and ensure callers such as
decodeCose(credential: String) call the imported Util.hexToBytes; rebuild to
confirm no unresolved references remain.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt:
- Around line 17-27: Remove the unused CBOR ObjectMapper field and the duplicate
hexToBytes function: delete the private val cborMapper =
ObjectMapper(CBORFactory()).registerKotlinModule() and remove the
hexToBytes(hex: String) method from CwtVerifer, and replace any calls (if
present) with the shared utility implementation in Util.kt (reuse the existing
hexToBytes there); also clean up any now-unused imports resulting from these
deletions.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt:
- Around line 47-48: The JSON extraction of verifiableCredentials in
PresentationVerifier.verify() (the
JSONObject(presentation).getJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) call)
can throw JSONException and is not currently handled; wrap this access in a
try/catch for JSONException (or validate presence with has/optJSONArray) and on
failure throw the existing PresentationNotSupportedException (or return an empty
VC result list) so a malformed or missing verifiableCredential array doesn’t
propagate an unhandled exception after getPresentationVerificationStatus()
succeeds; apply the same pattern to the similar extraction site around
getVCVerificationResults() at the other occurrence (line ~179).
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt:
- Around line 267-295: The test `should return error when statusSize is zero or
negative` leaks the second MockWebServer because the second prepareVC call
destructures its server into `_`; capture it (e.g., val (replacedVCZero,
serverZero) = prepareVC(...)) and ensure you call serverZero.shutdown() (or wrap
both servers in a try/finally or use a resource block to shut down both servers)
so both MockWebServer instances started by prepareVC are properly closed.
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt:
- Around line 32-41: This test belongs to the wrong test class and unnecessarily
loads an SD-JWT file; move the test to a new MsoMdocVerifiableCredentialTest and
delete the SD-JWT file loading from the test in SdJwtVerifierTest: create a test
method in MsoMdocVerifiableCredentialTest that calls
MsoMdocVerifiableCredential().checkStatus(...) and asserts it throws
UnsupportedOperationException with the message "Credential status checking not
supported for this credential format" (no sdJwt.txt or SD-JWT content should be
used), and remove the entire misplaced test block from SdJwtVerifierTest.
In @vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.json:
- Around line 1-42: The file mockDid.json is missing a trailing newline; open
the test fixture "mockDid.json" (the DID document containing verificationMethod
entries like "RsaVerificationKey2018", "EcdsaSecp256k1VerificationKey2019", and
"Ed25519VerificationKey2020") and add a single newline character at the end of
the file so it ends with a newline per POSIX conventions.
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.json:
- Around line 1-25: The test fixture file mosipRevokedStatusList.json is missing
a trailing newline; open mosipRevokedStatusList.json and add a single newline
character at the end of the file so it ends with '\n' (ensure no other changes
to the JSON content).
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json:
- Line 5: The test fixture contains a real email under the "email" key in
mosipUnrevokedVC.json; replace that PII with a clearly fake address (e.g.,
"test@example.com") to remove personal data; apply the same change to the other
fixture mentioned (VPWithRevokedVC.json) to ensure no real emails remain in test
resources.
- Line 51: The test fixture contains real PII in the "phone" property of
mosipUnrevokedVC.json; replace the real number "+919427357934" with an obviously
fake number (e.g., "+1-555-0100" or another reserved test number) so the "phone"
field in the fixture no longer contains real personal data.
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json:
- Line 8: Replace the real email string "test@gmail.com" in the test resource
with a reserved test domain such as "test@example.com" (i.e., update the value
"test@gmail.com" in didEdPublicKey.json to "test@example.com") to avoid
referencing a real domain in test fixtures.
In @vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json:
- Line 60: The test fixture VPWithRevokedVC.json contains a realistic phone
number in the "phone" field; replace the current value "+919427357934" with an
obviously fake number (e.g., "+1-555-0100" or similar clearly test-only value)
so no real PII remains in the fixture.
- Line 14: Replace the real-looking email value in the VPWithRevokedVC.json test
fixture by using clearly fake/test data (e.g., change the "email" field value
from "siwer.km@gmail.com" to "test@example.com" or similar); update the "email"
property inside the JSON object in VPWithRevokedVC.json so no real PII remains,
and run tests to ensure the fixture is still valid.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
84-84: Update outdated API reference.This line still references the old
isRevoked(credential)method which was replaced bycheckStatus()in this PR. The documentation should be updated to reflect the new status checking mechanism.🔎 Proposed fix
-This class provides the primary interface for interacting with the library and encapsulates all the core functionalities -related to validating and verifying Verifiable Credentials (VCs). It also performs a revocation check by calling the method isRevoked(credential). +This class provides the primary interface for interacting with the library and encapsulates all the core functionalities +related to validating and verifying Verifiable Credentials (VCs). It also performs credential status checks using the checkStatus() method.
🧹 Nitpick comments (15)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt (1)
9-11: Clarify the semantics of the Map key in the return type.The default implementation appropriately allows formats that don't support status checking to gracefully fail. However, the return type
Map<String, CredentialStatusResult>lacks documentation about what the String key represents (e.g., status purpose name, credential ID, or status type).Consider adding KDoc to clarify the key semantics, for example:
/** * Checks the status of a credential. * @param credential The credential to check * @param statusPurposes Optional list of status purposes to filter (e.g., "revocation", "suspension") * @return Map where keys are status purpose names and values are the corresponding status results */ fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { throw UnsupportedOperationException("Credential status checking not supported for this credential format") }vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json (1)
112-112: Clarify the mock JWS payload.The JWS contains "hello" as the payload, which is unconventional. If this is intentional mock data for testing, consider adding a comment in the test code that uses this fixture to clarify that this is a deliberately simplified test signature.
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (1)
13-17: Consider type-safe JSON parsing in test utilities.The cast
as Map<String, Any>?afterreadValue(responseJson, Map::class.java)suppresses type checking and could mask issues if test data doesn't match expected structure. While acceptable for test utilities, consider using a more specific type parameter or typed readValue overload for better compile-time safety.🔎 Safer alternative
fun mockHttpResponse(url: String, responseJson: String) { every { NetworkManagerClient.Companion.sendHTTPRequest(url, any()) } answers { - mapper.readValue(responseJson, Map::class.java) as Map<String, Any>? + mapper.readValue<Map<String, Any>>(responseJson) } }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (1)
122-126: Remove unused variables.
header1andheaderValueare extracted but never used. This appears to be dead code.🔎 Proposed fix
val payloadBytes = coseObj[2].GetByteString() - val header1 = coseObj[0].GetByteString() - val headerValue = CBORObject.DecodeFromBytes(header1) val claims = CBORObject.DecodeFromBytes(payloadBytes)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
177-183: Consider adding error logging for debugging.The
httpGetfunction silently swallows all exceptions, which may make debugging network issues difficult. Consider logging the error at DEBUG or TRACE level.🔎 Proposed fix
+ private val logger = java.util.logging.Logger.getLogger(Util::class.java.name) + fun httpGet(url: String): String? { return try { restTemplate.getForObject(url, String::class.java) } catch (e: Exception) { + logger.fine("HTTP GET failed for $url: ${e.message}") null } }vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
100-127: Good test coverage for credential status verification.The new tests properly mock HTTP responses and verify the credential status flow.
Minor suggestion: On line 153, use
assertTruefor consistency with line 126 which usesassertFalse:🔎 Proposed fix
- assertEquals(credentialStatusEntry.value.isValid, true) + assertTrue(credentialStatusEntry.value.isValid)Also applies to: 129-154
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (2)
31-32: Consider lazy initialization for test fixtures.
mockDidJsonis initialized at class property level, which runs before@BeforeAll. While this works, consider usinglateinit varwith initialization insetup()for consistency and clearer test lifecycle.🔎 Proposed fix
- val didDocumentUrl = "https://mosip.github.io/inji-config/qa-inji1/mock/did.json" - val mockDidJson = readClasspathFile("ldp_vc/mockDid.json") + val didDocumentUrl = "https://mosip.github.io/inji-config/qa-inji1/mock/did.json" + lateinit var mockDidJson: String @BeforeAll fun setup() { mockkObject(NetworkManagerClient.Companion) Util.documentLoader = LocalDocumentLoader + mockDidJson = readClasspathFile("ldp_vc/mockDid.json") }
179-179: Remove redundant mock setup.
mockkObject(NetworkManagerClient.Companion)is already called in@BeforeAll(line 36). This duplicate call is unnecessary.🔎 Proposed fix
- mockkObject(NetworkManagerClient.Companion) - mockHttpResponse(realUrl, mockStatusListJson)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (2)
163-168: Edge case: Credential expires exactly at current time.The condition
exp <= nowtreats a credential as expired whenexpequalsnow. Typically, expiration checks useexp < now(strictly less than), meaning the credential is valid up to and including the expiration time. The current logic causes credentials to be considered expired one second earlier than expected.🔎 Proposed fix
- if (exp != null && exp <= now) { + if (exp != null && exp < now) {
204-207: Hex validation doesn't account for whitespace.The
isValidHexmethod rejects any whitespace, buthexToBytesinUtil.kt(line 152) strips whitespace viacleanHex = hex.replace("\\s".toRegex(), ""). This inconsistency means a hex string with spaces would fail validation here but would be successfully decoded byhexToBytes.🔎 Proposed fix
private fun isValidHex(credential: String): Boolean { - val hexRegex = "^[0-9a-fA-F]+$".toRegex() - return credential.matches(hexRegex) + val cleanHex = credential.replace("\\s".toRegex(), "") + val hexRegex = "^[0-9a-fA-F]+$".toRegex() + return cleanHex.isNotEmpty() && cleanHex.matches(hexRegex) }vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt (1)
13-15: Remove emptysetup()method.The
@BeforeEachmethod is empty and can be removed to reduce noise. As flagged by static analysis (detekt), empty blocks should be eliminated.🔎 Proposed fix
- @BeforeEach - fun setup() { - } - private val validator = CwtValidator()vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (3)
45-45: Remove empty default constructor.As flagged by static analysis, an empty default constructor can be removed since the class has no constructor parameters.
🔎 Proposed fix
-class LdpStatusChecker() { +class LdpStatusChecker {
145-150: Original exception cause is lost when re-throwing.The catch block wraps the exception message but doesn't chain the original exception, losing the stack trace. This makes debugging network or parsing issues harder.
🔎 Proposed fix
} catch (e: Exception) { throw StatusCheckException( "Retrieval of the status list failed: ${e.message}", - STATUS_RETRIEVAL_ERROR + STATUS_RETRIEVAL_ERROR, + e ) }This requires
StatusCheckExceptionto accept acauseparameter. If it doesn't currently support this, consider adding it:class StatusCheckException( val errorMessage: String, val errorCode: StatusCheckErrorCode, cause: Throwable? = null ) : Exception(errorMessage, cause)
265-265: Potential NPE when parsingstatusListIndex.If
entry[STATUS_LIST_INDEX]is null (despite earlier validation), calling.toString().toInt()will throw. WhilevalidateCredentialStatusEntrychecks for this, the validation usestoIntOrNull()which handles null/invalid values, but here the value is re-parsed without the same safety.🔎 Proposed fix
- val statusListIndex = entry[STATUS_LIST_INDEX].toString().toInt() + val statusListIndex = entry[STATUS_LIST_INDEX]?.toString()?.toIntOrNull() + ?: throw StatusCheckException( + "Invalid '$STATUS_LIST_INDEX'", + INVALID_INDEX + )vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (1)
48-79: Consider extracting server shutdown to@AfterEach.Multiple tests create
MockWebServerinstances and must remember to shut them down. Consider using a test-level field and shutting down inteardown()to prevent resource leaks if assertions fail beforeshutdown()is called.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
README.mdvc-verifier/kotlin/README.mdvc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.ktvc-verifier/kotlin/vcverifier/publish-artifact.gradlevc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.ktvc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.ktvc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedStatusList.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json
💤 Files with no reviewable changes (8)
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.kt
✅ Files skipped from review due to trivial changes (9)
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.json
- vc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.json
- vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedStatusList.json
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.kt
👮 Files not reviewed due to content moderation or server errors (1)
- vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.kt
🧰 Additional context used
🧬 Code graph analysis (4)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
readClasspathFile(8-9)mockHttpResponse(13-17)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (1)
isValid(57-60)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
isValidUri(80-91)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
hexToBytes(152-159)httpGet(177-183)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
decodeCose(189-192)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
decodeCose(29-36)hexToBytes(20-27)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
validateNumericDate(161-175)hexToBytes(152-159)
🪛 detekt (1.23.8)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt
[warning] 45-45: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
[warning] 145-145: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt
[warning] 14-15: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🪛 Gitleaks (8.30.0)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json
[high] 114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json
[high] 114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.18.1)
vc-verifier/kotlin/README.md
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
91-91: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
102-102: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
107-107: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
108-108: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
110-110: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
112-112: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
113-113: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
115-115: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
116-116: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
117-117: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
220-220: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
221-221: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
268-268: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
269-269: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
276-276: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
280-280: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
292-292: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
293-293: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
298-298: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
299-299: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
300-300: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
305-305: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
317-317: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
318-318: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
322-322: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
324-324: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
325-325: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
326-326: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
327-327: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
328-328: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
332-332: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
496-496: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
503-503: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
593-593: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
604-604: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
605-605: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
607-607: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
608-608: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
609-609: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
610-610: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
611-611: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
627-627: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
741-741: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
784-784: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
793-793: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
833-833: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README.md
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (36)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json (1)
1-42: LGTM! Well-structured DID Document for testing.This test resource provides good coverage with three different verification key types (RSA, EC secp256k1, Ed25519), each using appropriate key representation formats. The structure follows DID Core specification conventions.
Minor: The file is missing a trailing newline, which is a common convention for text files and may be flagged by some linters.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.kt (1)
45-46: LGTM! Clean addition following the established pattern.The new context mapping follows the same pattern as all existing entries and correctly routes the mock-identity-context URL to the local test resource.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.json (1)
1-7: LGTM! Valid Ed25519 test fixture.The JSON structure is well-formed and follows the W3C security vocabulary specification for Ed25519VerificationKey2018 keys. The PEM-encoded public key has the correct format and ASN.1 structure for Ed25519 keys, and the mock URLs are appropriate for a test resource.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json (1)
1-26: LGTM! DID document structure is valid.The JSON structure correctly follows the W3C DID v1 specification for a did:web document with Ed25519VerificationKey2020. The verification method, authentication, and assertionMethod references are properly configured for test scenarios.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt (1)
3-7: LGTM: Explicit imports improve clarity.Replacing wildcard imports with explicit imports is a good practice that enhances code readability.
vc-verifier/kotlin/vcverifier/publish-artifact.gradle (1)
99-99: LGTM! Version bump aligns with new features.The version increment from 1.6.0-SNAPSHOT to 1.7.0-SNAPSHOT is appropriate for the new CWT verification and credential status checking features being introduced in this PR.
Also applies to: 113-113
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.kt (5)
7-7: LGTM! Import supports the new test.The import is required for the new credential status check test added at lines 105-112.
18-19: Good refactoring! Credential constant improves maintainability.Extracting the credential string into a reusable constant follows the DRY principle and makes the test code cleaner.
21-25: LGTM! Proper test initialization.Mocking
BuildConfig.versionin the setup ensures consistent test behavior across different SDK environments.
35-35: LGTM! Using the extracted constant.This change leverages the
validMdocconstant defined above, improving code maintainability.
105-112: LGTM! Validates unsupported operation behavior.The test correctly verifies that
checkStatusthrowsUnsupportedOperationExceptionwith the expected message for the MSO-MDOC credential format, which doesn't support credential status checking.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt (1)
3-3: LGTM!The import for
CredentialStatusResultis correctly added to support the new status checking functionality.README.md (2)
41-41: LGTM!The updated terminology "Status Check" correctly reflects the broader capability beyond just revocation checking.
46-52: LGTM!The table updates correctly reflect:
- Addition of
EcdsaSecp256k1Signature2019support forldp_vcformat- Clarification that SD-JWT formats use algorithm headers rather than named signature suites
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json (1)
1-116: LGTM - Gitleaks warning is a false positive.The Gitleaks warning on line 114 is a false positive. The flagged value is a DID identifier (
did:key:z6Mkp...), which is a public decentralized identifier, not an API key or secret. Similarly, the JWS signature on line 112 is public cryptographic proof data.This test fixture correctly represents a Verifiable Presentation with an embedded credential that has status checking capabilities, supporting the new status verification flows.
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json (1)
114-114: Static analysis false positive - safe to ignore.The static analysis tool flagged this line as containing a generic API key, but this is a W3C DID (Decentralized Identifier) which is a public identifier, not a secret. This can be safely ignored.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (1)
17-20: LGTM - Well-structured status checking data model.The new data classes properly separate verification results from status checking results, use immutable fields (val), and handle error cases appropriately with nullable StatusCheckException. The design aligns well with the status-check subsystem introduced in this PR.
Also applies to: 27-31, 57-65
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt (1)
19-21: LGTM - Clean refactoring to status checking.The replacement of
isRevoked()withcheckStatus()properly delegates to the newLdpStatusCheckerand returns richer status information viaMap<String, CredentialStatusResult>, supporting multiple status purposes. This aligns well with the new status-check subsystem introduced in this PR.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.kt (1)
1-16: LGTM!The exception class and error code enum are well-structured. The error codes cover the expected failure scenarios for credential status checking.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
29-36: LGTM on core verification logic.The critical bugs from previous reviews have been addressed:
decodeCosenow correctly returns the decodedCBORObjectfetchPublicKeynow properly converts JWK toPublicKeyverifynow returns theverifySignatureresultThe verification flow is well-structured with appropriate null-safety handling.
Also applies to: 118-135
15-15: Fix typo in class name:CwtVerifer→CwtVerifier.The class name is missing an 'i'. Also update the filename to match.
Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
36-37: LGTM on RestTemplate singleton.
RestTemplateis thread-safe for concurrent use. Using a single instance is appropriate for this utility object.
152-175: LGTM!Both utility functions are well-implemented:
hexToBytesproperly validates input and handles whitespacevalidateNumericDatecorrectly validates CBOR integer types for numeric datesvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
30-39: LGTM!Test setup and teardown are properly implemented. The mock configuration using
mockkObject(NetworkManagerClient)is correct for mocking companion object methods.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.kt (1)
1-13: LGTM!Constants are well-organized and properly named for the BitstringStatusList credential format. Using
const valensures compile-time constants.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (2)
18-18: Good refactor: Factory instance reuse.Creating the
CredentialVerifierFactoryas a class field instead of instantiating it in each method is a good optimization that avoids redundant object creation.
76-104: LGTM on new public API methods.The new
getCredentialStatusandverifyAndGetCredentialStatusmethods are well-designed:
- Graceful handling of
UnsupportedOperationExceptionfor formats without status support- Status is only fetched when verification succeeds, avoiding unnecessary work
- Sensible default parameter for
statusPurposeListvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (1)
157-228: Good test coverage for credential status verification.The three new tests comprehensively cover:
- Empty status when verification fails
- Valid (unrevoked) credential status
- Invalid (revoked) credential status
Each test properly mocks external dependencies and validates the expected behavior.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
24-59: Well-structured validation flow with clear error handling.The validation pipeline follows a logical sequence: empty check → hex validation → COSE structure → protected header → CWT claims → date validation. The try-catch structure correctly maps
ValidationExceptiontoValidationStatuswhile catching unexpected exceptions with a generic error.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt (1)
34-151: Good test coverage addressing prior review feedback.The test class now provides comprehensive coverage for CwtValidator scenarios including empty input, invalid hex, COSE structure validation, ALG type checking, payload validation, missing headers, temporal claims (exp, nbf, iat), and a positive path test. This addresses the previous review comment about the empty test class.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt (2)
162-171: New status-aware VC verification follows established patterns.The
getVCVerificationResultsWithCredentialStatusmethod correctly delegates tocredentialsVerifier.verifyAndGetCredentialStatus()and maps results to the newVCResultWithCredentialStatustype. The implementation is consistent with the existinggetVCVerificationResultsmethod.
173-183: Clean API extension for credential status retrieval.The new
verifyAndGetCredentialStatusmethod provides a natural extension to the existingverifyAPI, with sensible defaults (statusPurposeList = emptyList()). The implementation reuses the extracted helper methods appropriately.vc-verifier/kotlin/README.md (1)
1-848: Comprehensive documentation update.The README now provides thorough coverage of supported formats, validation/verification flows, credential status checking, API references, and error codes. The addition of example JSON responses and feature matrices improves developer experience significantly.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (2)
362-388: Well-implemented bit extraction logic.The
readBitsandreadBitmethods correctly handle multi-bit status values with proper bounds checking. The bit ordering (MSB first within the status value) aligns with the StatusList2021 specification.
169-170: ReadvalidFromandvalidUntilfrom credential root, notcredentialSubject.Per W3C VC Data Model and StatusList2021 specifications,
validFromandvalidUntilare properties at the credential root level indicating the validity period of the status list itself. The code currently reads these fromcredentialSubject(lines 169–170), but should access them fromstatusListVC.jsonObjectinstead.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (1)
81-453: Comprehensive test coverage for status checking scenarios.The test suite covers a wide range of scenarios including:
- Valid/revoked credential detection
- Missing/invalid credentialStatus handling
- Range errors for statusListIndex
- Encoding/decoding failures (Base64, GZIP)
- Purpose mismatches
- Temporal validation (validFrom/validUntil)
- statusSize and statusMessage validation
This provides good confidence in the LdpStatusChecker implementation.
| - `types` | ||
| - `validator` | ||
| - `verifier` | ||
| - `statusChecker` |
There was a problem hiding this comment.
Fix markdown list indentation.
The indentation should be 2 spaces to match the other list items, not 4 spaces.
🔎 Proposed fix
- - `statusChecker`
+ - `statusChecker`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `statusChecker` | |
| - `statusChecker` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In @README.md around line 61, The markdown list item `statusChecker` is indented
with 4 spaces instead of 2; update the README.md list so the `statusChecker`
entry uses 2-space indentation to match the other list items, preserving the
backticks and list marker.
| | `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/10/) | | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest version IETF SD-JWT VC draft specification 2025
💡 Result:
The latest IETF SD-JWT VC draft (2025) is draft-ietf-oauth-sd-jwt-vc-13, published November 2025 (expires 10 May 2026). [1]
Sources: IETF Datatracker entry for draft-ietf-oauth-sd-jwt-vc-13. [1]
🏁 Script executed:
# Search for SD-JWT version references in the kotlin codebase
rg "sd-jwt|SD-JWT" --type kotlin -i -A 3 -B 3Repository: inji/vc-verifier
Length of output: 12923
🏁 Script executed:
# Look for any version specifications in build files or configuration
fd "build\|pom\|gradle\|package" --type f | xargs rg "sd-jwt" -i 2>/dev/null || trueRepository: inji/vc-verifier
Length of output: 8438
🏁 Script executed:
# Check for any explicit version references in the kotlin directory
fd . vc-verifier/kotlin --type f | xargs rg "draft-ietf-oauth-sd-jwt|10\|11\|12" 2>/dev/null || trueRepository: inji/vc-verifier
Length of output: 266
Update documentation to reference the latest IETF SD-JWT VC specification draft.
The README currently references draft-ietf-oauth-sd-jwt-vc/10, but the latest version is draft-ietf-oauth-sd-jwt-vc-13 (published November 2025). Update the link to point to the current specification.
🤖 Prompt for AI Agents
In @vc-verifier/kotlin/README.md around lines 42-43, Update the README table
entry that references the SD-JWT VC draft for `vc+sd-jwt`, `dc+sd-jwt` so the
link points to the latest spec (change the URL from
draft-ietf-oauth-sd-jwt-vc/10 to draft-ietf-oauth-sd-jwt-vc-13); locate the
table row containing the symbols `vc+sd-jwt`, `dc+sd-jwt` and replace the old
draft link with the new draft-ietf-oauth-sd-jwt-vc-13 URL (published November
2025).
...ifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt
Show resolved
Hide resolved
| private val cborMapper = ObjectMapper(CBORFactory()) | ||
| .registerKotlinModule() | ||
|
|
||
| fun hexToBytes(hex: String): ByteArray { | ||
| val cleanHex = hex.replace("\\s".toRegex(), "") | ||
| require(cleanHex.length % 2 == 0) { "Invalid hex length" } | ||
|
|
||
| return ByteArray(cleanHex.length / 2) { i -> | ||
| cleanHex.substring(i * 2, i * 2 + 2).toInt(16).toByte() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused field and duplicate function.
cborMapper(lines 17-18) is never used in this class.hexToBytesduplicates the existing utility inUtil.kt(lines 151-158).
🔎 Proposed fix
Remove the unused mapper and import the utility function:
+import io.mosip.vercred.vcverifier.utils.Util.hexToBytes
class CwtVerifer {
-
- private val cborMapper = ObjectMapper(CBORFactory())
- .registerKotlinModule()
-
- fun hexToBytes(hex: String): ByteArray {
- val cleanHex = hex.replace("\\s".toRegex(), "")
- require(cleanHex.length % 2 == 0) { "Invalid hex length" }
-
- return ByteArray(cleanHex.length / 2) { i ->
- cleanHex.substring(i * 2, i * 2 + 2).toInt(16).toByte()
- }
- }Also remove unused imports:
-import com.fasterxml.jackson.databind.ObjectMapper
-import com.fasterxml.jackson.dataformat.cbor.CBORFactory
-import com.fasterxml.jackson.module.kotlin.registerKotlinModuleCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt
around lines 17-27, Remove the unused CBOR ObjectMapper field and the duplicate
hexToBytes function: delete the private val cborMapper =
ObjectMapper(CBORFactory()).registerKotlinModule() and remove the
hexToBytes(hex: String) method from CwtVerifer, and replace any calls (if
present) with the shared utility implementation in Util.kt (reuse the existing
hexToBytes there); also clean up any now-unused imports resulting from these
deletions.
| val verifiableCredentials = JSONObject(presentation).getJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) | ||
| val vcVerificationResults: List<VCResult> = getVCVerificationResults(verifiableCredentials) |
There was a problem hiding this comment.
JSON parsing not wrapped in error handling.
JSONObject(presentation).getJSONArray(...) can throw JSONException if the presentation is malformed or missing the verifiableCredential key. In verify(), this happens after getPresentationVerificationStatus() which already handles JSON parsing failures via PresentationNotSupportedException. However, if the proof verification succeeds but the verifiableCredential array is missing, an unhandled exception will propagate.
Consider wrapping the JSON access or validating the structure earlier in the flow.
Also applies to: 179-179
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt
around lines 47-48, The JSON extraction of verifiableCredentials in
PresentationVerifier.verify() (the
JSONObject(presentation).getJSONArray(Shared.KEY_VERIFIABLE_CREDENTIAL) call)
can throw JSONException and is not currently handled; wrap this access in a
try/catch for JSONException (or validate presence with has/optJSONArray) and on
failure throw the existing PresentationNotSupportedException (or return an empty
VC result list) so a malformed or missing verifiableCredential array doesn’t
propagate an unhandled exception after getPresentationVerificationStatus()
succeeds; apply the same pattern to the similar extraction site around
getVCVerificationResults() at the other occurrence (line ~179).
| { | ||
| "credentialSubject": { | ||
| "statusPurpose": "revocation", | ||
| "id": "https://injicertify-mock.qa-inji1.mosip.net/v1/certify/credentials/status-list/56622ad1-c304-4d7a-baf0-08836d63c2bf", | ||
| "type": "BitstringStatusList", | ||
| "encodedList": "uH4sIAAAAAAAA_-3OMQ0AIRQFQS6hoDwJSEU6GgjJfwUzBnZbAwAAAACgwEgPAG_6K2OrMgYAwJ0vPQBAnZ4eAAA4NNMDAAAASRupW2djAEAAAA" | ||
| }, | ||
| "id": "https://injicertify-mock.qa-inji1.mosip.net/v1/certify/credentials/status-list/56622ad1-c304-4d7a-baf0-08836d63c2bf", | ||
| "validFrom": "2025-11-11T07:29:00.054Z", | ||
| "proof": { | ||
| "proofValue": "z2P8k8zWJ9st2hHCir7gjWs1JxznsdsBvKXNuaeN7kmrTqHaA6srziB5kZahz2iC2TsKsJ6FUniwPtAKKA2mvNmr", | ||
| "created": "2025-11-11T07:29:00Z", | ||
| "proofPurpose": "assertionMethod", | ||
| "type": "Ed25519Signature2020", | ||
| "verificationMethod": "did:web:mosip.github.io:inji-config:qa-inji1:mock#kMVyZTvx8G0h1YZnUW4OJtr2LRVZUuGl8pQuJl3ymXI" | ||
| }, | ||
| "type": [ | ||
| "VerifiableCredential", | ||
| "BitstringStatusListCredential" | ||
| ], | ||
| "@context": [ | ||
| "https://www.w3.org/ns/credentials/v2" | ||
| ], | ||
| "issuer": "did:web:mosip.github.io:inji-config:qa-inji1:mock" | ||
| } No newline at end of file |
There was a problem hiding this comment.
Add newline at end of file.
This test fixture is missing a newline at the end of the file. While this doesn't affect JSON parsing, it's best practice to end files with a newline character per POSIX standards.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.json
around lines 1-25, The test fixture file mosipRevokedStatusList.json is missing
a trailing newline; open mosipRevokedStatusList.json and add a single newline
character at the end of the file so it ends with '\n' (ensure no other changes
to the JSON content).
| "credentialSubject": { | ||
| "dateOfBirth": "1999/10/21", | ||
| "UIN": 123456, | ||
| "email": "siwer.km@gmail.com", |
There was a problem hiding this comment.
Remove PII from test fixtures.
Same issue as VPWithRevokedVC.json: use clearly fake email addresses (e.g., "test@example.com") to avoid compliance risks.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json
around line 5, The test fixture contains a real email under the "email" key in
mosipUnrevokedVC.json; replace that PII with a clearly fake address (e.g.,
"test@example.com") to remove personal data; apply the same change to the other
fixture mentioned (VPWithRevokedVC.json) to ensure no real emails remain in test
resources.
| } | ||
| ], | ||
| "id": "did:jwk:eyJrdHkiOiJPS1AiLCJ1c2UiOiJzaWciLCJjcnYiOiJFZDI1NTE5IiwieCI6IlVFSWlCZXk3Y0xUakNxZVZpejZBWFFEV2lvR2pNNXo0M21qV3NCTjRwTTgifQ==", | ||
| "phone": "+919427357934", |
There was a problem hiding this comment.
Remove PII from test fixtures.
Same issue as VPWithRevokedVC.json: use obviously fake phone numbers (e.g., "+1-555-0100") to avoid compliance concerns.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json
around line 51, The test fixture contains real PII in the "phone" property of
mosipUnrevokedVC.json; replace the real number "+919427357934" with an obviously
fake number (e.g., "+1-555-0100" or another reserved test number) so the "phone"
field in the fixture no longer contains real personal data.
| "id": "did:web:vharsh.github.io:DID:local", | ||
| "alsoKnownAs": [ | ||
| "testUUID", | ||
| "test@gmail.com" |
There was a problem hiding this comment.
Consider using a reserved test domain.
The email "test@gmail.com" references a real domain. For test resources, it's better practice to use reserved test domains like "test@example.com" per RFC 2606 to avoid any potential confusion or privacy concerns.
🔎 Suggested fix
- "test@gmail.com"
+ "test@example.com"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "test@gmail.com" | |
| "test@example.com" |
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json
around line 8, Replace the real email string "test@gmail.com" in the test
resource with a reserved test domain such as "test@example.com" (i.e., update
the value "test@gmail.com" in didEdPublicKey.json to "test@example.com") to
avoid referencing a real domain in test fixtures.
| "credentialSubject": { | ||
| "dateOfBirth": "1999/10/21", | ||
| "UIN": 123456, | ||
| "email": "siwer.km@gmail.com", |
There was a problem hiding this comment.
Remove PII from test fixtures.
The email address appears to be real or realistic personal data. Test fixtures should use clearly fake data (e.g., "test@example.com") to avoid compliance risks.
Based on the AI summary, this PR introduces status-check and revocation-related components that handle sensitive credential data. Ensuring test data does not contain real PII is essential for GDPR/CCPA compliance.
🤖 Prompt for AI Agents
In @vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json
around line 14, Replace the real-looking email value in the VPWithRevokedVC.json
test fixture by using clearly fake/test data (e.g., change the "email" field
value from "siwer.km@gmail.com" to "test@example.com" or similar); update the
"email" property inside the JSON object in VPWithRevokedVC.json so no real PII
remains, and run tests to ensure the fixture is still valid.
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
* [INJIMOB-3591] refactor revokechecker to statuschecker Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3591] add validations for status check Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3591] expose method for getting verification result and status as verfication summary Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3581] replace expired mso_mdoc vc Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3581] rename verifyAndGetStatus to verifyAndGetCredentialStatus and refactor Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> --------- Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
…nji#205) * [INJIVER-1400] - support revocation in presentationVerifier Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - add test for revocation Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - Add PresentationResultWithCredentialStatus and VCResultWithCredentialStatus data classes Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - add mock-identity and w3v1 context mappings Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - add VPWithRevokedVC json Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - add VPWithUnrevokedVC json Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - add context json Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - remove w3v1 context Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - rename presentationVerificationStatus to getPresentationVerificationStatus Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - rename tests and add VP verification check Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - Remove handling of VC_REVOKED error code Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - Remove unused import Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - Remove test for VC_Revoked error code Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - Remove constants ERROR_CODE_VC_REVOKED Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - extract verifiableCredentials Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> * [INJIVER-1400] - update version to 1.7.0-snapshot Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> --------- Signed-off-by: sarupmoni <93664377+sarupmoni@users.noreply.github.com> Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
…tus check (inji#211) * [INJIMOB-3647] refactor: enhance response structure of credential status check (inji#207) * [INJIMOB-3647] refactor: remove getting ldp class from factory in LdpStatusChecker Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: remove unused variable Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: throw error for unsupported status check instead of returning null Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: modify CredentialStatusResult structure CredentialStatusResult has 2 params 1. purpose 2. result -> This holds a boolean of iSuccess and error of type StatusCheckException for any failure Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: extract magic numbers to const in status checker Other changes: - add info logs to print status message for avoiding loss of data Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: change Result's isSuccess to isValid Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: update ldp status check to return empty list when not applicable Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: modify error code for - invalid credential status Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: update isValid function Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] enhance readme Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * Revert "[INJIMOB-3647] enhance readme" This reverts commit 1f3aa8c. Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3647] enhance readme Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3647] refactor: modify CredentialVerificationSummary data type CredentialVerificationSummary -> has verification result and credential status - credentialStatus will now be holding a map of purpose to its relative result instead of a list of (purpose + result) Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] test: mock did web url responses Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: update PresentationVerifier to use updated Credential summary result class Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] enhance readme Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3647] refactor: remove unused exception Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] refactor: CredentialVerificationSummary - credentialStatus credentialStatus is a map of purpose to its CredentialStatusResult. CredentialStatusResult holds isValid and error Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] test: reuse extracted test utils for reading resource files Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] enhance readme Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3647] test: mock all network calls in verifier test Mock context and https / did web public key loading Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> * [INJIMOB-3647] docs: fix typo in readme Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> --------- Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> Co-authored-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> * [INJIMOB-3647] enhance readme47] enhance readme Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> --------- Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com> Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> Co-authored-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com> Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
jaswanthkumartw
force-pushed
the
injiver-1369
branch
from
392a035 to
0fab654
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI Agents
In @vc-verifier/kotlin/gradle/libs.versions.toml:
- Around line 23-24: You upgraded jacksonAnnotations and jacksonModuleKotlin to
2.17.2; before merging, run the full test suite and explicitly verify
breaking-change areas: confirm StreamReadConstraints limits won't reject large
strings/deep nesting or set custom StreamReadConstraints in Jackson
configuration; audit any uses of JsonNode.with() for keys starting with "/" and
update callers expecting literal "/" behavior; re-test places using
@JsonIgnore/@JsonProperty to ensure precedence changes don't break mappings;
exercise all enum (serialize/deserialize) paths and @JsonTypeInfo polymorphic
deserialization to catch behavioral changes; verify Kotlin compatibility (module
requires Kotlin >=1.5), and check the dependency tree for transitive changes
(SnakeYAML/Guava/Woodstox) that could cause conflicts. Ensure CI includes JSON
parsing tests for large/nested payloads, enum cases, and polymorphic types
before approving the version bump of jacksonAnnotations and jacksonModuleKotlin.
In @vc-verifier/kotlin/README.md:
- Around line 394-411: The example JSON in README uses
verificationResult.verificationStatus: true while describing an expired &
revoked VC; update the example so verificationResult.verificationStatus is
false, set verificationResult.verificationErrorCode to a suitable code (e.g.,
"EXPIRED") and keep verificationResult.verificationMessage as "VC is expired",
and ensure credentialStatus.revocation.isValid remains false to reflect
revocation.
- Line 42: Update the SD-JWT VC spec link and label in the README table entry
for `vc+sd-jwt`, `dc+sd-jwt` to point to the latest draft (change the URL from
draft-ietf-oauth-sd-jwt-vc/10 to draft-ietf-oauth-sd-jwt-vc-13) and, if present,
update any displayed version number text to "-13" or remove the version suffix
so it reflects the current draft; modify the table row containing the string
`vc+sd-jwt`, `dc+sd-jwt` and the linked URL to use the new draft URL.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt:
- Around line 18-20: The CwtVerifiableCredential.checkStatus override currently
returns an empty map which contradicts the interface's default behavior; change
the implementation in CwtVerifiableCredential.checkStatus to throw
UnsupportedOperationException (or, if you prefer the empty-map semantics, add
clear javadoc to the method stating that an empty map denotes "not supported"
for CWT), but prefer throwing UnsupportedOperationException to align with the
interface contract so callers receive a clear error when status checking isn't
supported by CWT.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt:
- Around line 19-21: Add a Breaking Changes entry in README.md documenting that
the LdpVerifiableCredential API changed: the old isRevoked(credential: String):
Boolean method was removed and replaced by checkStatus(credential: String,
statusPurposes: List<String>?): Map<String, CredentialStatusResult>; explain how
callers should migrate by replacing boolean checks with calls to
LdpVerifiableCredential.checkStatus, passing any relevant statusPurposes (or
null) and inspecting the returned Map<String, CredentialStatusResult> for the
credential's status (e.g., map lookup for the expected purpose and interpreting
CredentialStatusResult fields), and include a short example snippet and notes
about nullability of statusPurposes and how multiple purposes are represented in
the map.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt:
- Around line 20-27: The file defines a duplicate hexToBytes function; remove
this local hexToBytes and update decodeCose to call the shared utility
implementation (the hexToBytes in Util.kt) instead; also remove any now-unused
imports brought in solely for the duplicate function so the file compiles
without warnings.
- Around line 122-125: In CwtVerifer.kt remove the dead code that extracts and
decodes the COSE header: delete the unused variables header1 and headerValue and
the call that decodes header1 (the lines declaring header1 and headerValue and
CBORObject.DecodeFromBytes(header1)); keep only payloadBytes and claims
decoding, and remove any now-unused imports if needed.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt:
- Around line 162-170: The method getVCVerificationResultsWithCredentialStatus
currently hardcodes CredentialFormat.LDP_VC which breaks verification for
non-LDP credentials; update it to detect the credential format per item (e.g.,
implement a detectCredentialFormat(credentialJsonString): CredentialFormat) and
pass that detected format into credentialsVerifier.verifyAndGetCredentialStatus
instead of CredentialFormat.LDP_VC, leaving the rest of the mapping to construct
VCResultWithCredentialStatus unchanged.
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt:
- Around line 177-183: The httpGet function currently swallows all exceptions;
add logging so failures are visible: import org.slf4j.LoggerFactory, declare a
logger for Util (e.g., private val logger =
LoggerFactory.getLogger(Util::class.java) or similar), and inside the catch in
httpGet(url: String) log an error including the url and the exception (e.g.,
logger.error("HTTP GET failed for url: $url", e)) before returning null; keep
the method signature and restTemplate.getForObject usage unchanged.
- Line 36: The RestTemplate instance declared as val restTemplate =
RestTemplate() in Util.kt has no timeouts configured and can hang; replace it
with a RestTemplate configured with sensible connection and read/socket timeouts
(e.g., via RestTemplateBuilder or by creating an
HttpComponentsClientHttpRequestFactory/OkHttp3ClientHttpRequestFactory and
setting connect and read timeouts) and use that configured instance everywhere
the restTemplate variable is referenced; ensure the chosen timeout values are
reasonable for your environment and include them when constructing the
RestTemplate.
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt:
- Around line 267-295: Test leaks the second MockWebServer because
prepareVC(...) for the zero case discards the server; capture that server (e.g.,
val (replacedVCZero, serverZero) = prepareVC(statusListJson =
statusListJsonZero)) and call serverZero.shutdown() at the end (or shut both
down in a finally/tearDown block). Update the underscore in the second prepareVC
call to a named variable and ensure shutdown is invoked to close the resource.
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt:
- Around line 32-41: The test asserting UnsupportedOperationException for
MsoMdocVerifiableCredential.checkStatus is misplaced in SdJwtVerifierTest and
unnecessarily reads an SD-JWT file; move the entire test to a new test class
MsoMdocVerifiableCredentialTest (create file/class named
MsoMdocVerifiableCredentialTest) and remove the ResourceUtils file-loading lines
(the lines that set file and vc); call MsoMdocVerifiableCredential().checkStatus
with a simple placeholder (e.g., empty string) or null for the VC parameter and
keep the assertThrows and assertEquals checking the message unchanged.
In @vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json:
- Line 60: The test fixture contains real-looking PII in the "phone" JSON field;
replace the value in the VPWithRevokedVC fixture's "phone" key with an obviously
fake number (e.g., "+1-555-0100") to remove personal data, ensuring any other
test assertions still accept the updated fake value.
🧹 Nitpick comments (8)
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.json (1)
7-7: Consider adding a trailing newline.The file ends without a trailing newline. While not critical, many coding standards and version control systems prefer files to end with a newline character.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt (1)
15-19: Remove unnecessary semicolons.Kotlin doesn't require semicolons at the end of statements. Remove them for idiomatic Kotlin code.
🔎 Proposed fix
override fun verify(credential: String): Boolean { - return CwtVerifer().verify(credential); + return CwtVerifer().verify(credential) } override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { - return emptyMap(); + return emptyMap() }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (1)
118-135: Remove unnecessary semicolons.Kotlin doesn't require semicolons. Remove them for idiomatic code.
🔎 Proposed fix
fun verify(credential: String): Boolean { - val coseObj = decodeCose(credential) ?: return false; - if (!isValidCoseStructure(coseObj)) return false; + val coseObj = decodeCose(credential) ?: return false + if (!isValidCoseStructure(coseObj)) return false val payloadBytes = coseObj[2].GetByteString() - val header1 = coseObj[0].GetByteString() - val headerValue = CBORObject.DecodeFromBytes(header1) val claims = CBORObject.DecodeFromBytes(payloadBytes) - if (!isValidCwtStructure(claims)) return false; + if (!isValidCwtStructure(claims)) return false - val issuer = extractIssuer(claims) ?: return false; + val issuer = extractIssuer(claims) ?: return false - val issuerMetadata = resolveIssuerMetadata(issuer) ?: return false; + val issuerMetadata = resolveIssuerMetadata(issuer) ?: return false - val publicKey = fetchPublicKey(coseObj, issuerMetadata) ?: return false; + val publicKey = fetchPublicKey(coseObj, issuerMetadata) ?: return false return verifySignature(coseObj, publicKey) }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
204-207: Consider validating hex string length is even.
isValidHexonly checks for valid hex characters but doesn't verify the string has even length. WhilehexToBytesinUtil.ktdoes have arequire(cleanHex.length % 2 == 0)check, validating upfront would provide a clearer error message.🔎 Proposed fix
private fun isValidHex(credential: String): Boolean { val hexRegex = "^[0-9a-fA-F]+$".toRegex() - return credential.matches(hexRegex) + return credential.length % 2 == 0 && credential.matches(hexRegex) }vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt (1)
13-15: Remove empty@BeforeEachmethod.The empty
setup()method adds no value. Based on static analysis hints, this should be removed.🔎 Proposed fix
- @BeforeEach - fun setup() { - } - private val validator = CwtValidator()vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (1)
265-265: Consider using safe parsing for consistency.While
validateCredentialStatusEntryensuresSTATUS_LIST_INDEXis valid, usingtoInt()directly could still throw if the map entry is modified concurrently. UsingtoIntOrNull()with a fallback would be more defensive.🔎 Proposed fix
- val statusListIndex = entry[STATUS_LIST_INDEX].toString().toInt() + val statusListIndex = entry[STATUS_LIST_INDEX]?.toString()?.toIntOrNull() + ?: throw StatusCheckException( + "Invalid '$STATUS_LIST_INDEX'", + INVALID_INDEX + )vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (2)
40-43: Complete the test cleanup by unmocking NetworkManagerClient.The
teardownAll()method resets thedocumentLoaderbut doesn't unmockNetworkManagerClient.Companionthat was mocked insetup()(line 36). This could cause test pollution if other tests interact withNetworkManagerClient.🔎 Proposed fix
@AfterAll fun teardownAll() { Util.documentLoader = null + unmockkAll() }
179-179: Remove redundant mockkObject call.The
mockkObject(NetworkManagerClient.Companion)call is redundant since it's already executed in thesetup()method (line 36) which runs once before all tests due to the@BeforeAllannotation.🔎 Proposed fix
- mockkObject(NetworkManagerClient.Companion) - mockHttpResponse(realUrl, mockStatusListJson)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
README.mdvc-verifier/kotlin/README.mdvc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.ktvc-verifier/kotlin/gradle/libs.versions.tomlvc-verifier/kotlin/vcverifier/build.gradle.ktsvc-verifier/kotlin/vcverifier/publish-artifact.gradlevc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialFormat.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/CredentialVerifierFactory.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.ktvc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.ktvc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedStatusList.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.jsonvc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.jsonvc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json
💤 Files with no reviewable changes (8)
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.kt
✅ Files skipped from review due to trivial changes (1)
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.json
🚧 Files skipped from review as they are similar to previous changes (16)
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json
- vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.kt
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.json
- vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialFormat.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/CredentialVerifierFactory.kt
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.json
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.kt
- vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.json
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedStatusList.json
👮 Files not reviewed due to content moderation or server errors (1)
- vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.kt
🧰 Additional context used
🧬 Code graph analysis (5)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
mockHttpResponse(13-17)readClasspathFile(8-9)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
decodeCose(29-36)hexToBytes(20-27)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
validateNumericDate(161-175)hexToBytes(152-159)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
hexToBytes(152-159)httpGet(177-183)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (1)
decodeCose(189-192)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
readClasspathFile(8-9)mockHttpResponse(13-17)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (1)
isValid(57-60)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
isValidUri(80-91)
🪛 detekt (1.23.8)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
[warning] 180-180: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt
[warning] 14-15: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🪛 Gitleaks (8.30.0)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json
[high] 114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json
[high] 114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.18.1)
vc-verifier/kotlin/README.md
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
91-91: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
102-102: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
107-107: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
108-108: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
110-110: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
112-112: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
113-113: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
115-115: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
116-116: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
117-117: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
220-220: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
221-221: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
268-268: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
269-269: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
276-276: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
280-280: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
292-292: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
293-293: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
298-298: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
299-299: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
300-300: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
305-305: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
317-317: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
318-318: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
322-322: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
324-324: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
325-325: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
326-326: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
327-327: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
328-328: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
332-332: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
496-496: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
503-503: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
593-593: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
604-604: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
605-605: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
607-607: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
608-608: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
609-609: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
610-610: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
611-611: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
627-627: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
741-741: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
784-784: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
793-793: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
833-833: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README.md
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (47)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt (1)
3-7: LGTM! Good practice to use explicit imports.Replacing wildcard imports with explicit imports improves code clarity and makes dependencies more transparent.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.json (1)
4-5: No changes needed. The external URLs in the test fixture are properly handled by test mocks. ThemockHttpResponse()function intercepts HTTP calls to these URLs and returns the fixture data locally, preventing any actual network requests or external dependencies. Using real MOSIP system URLs in fixture data (while mocking the network calls) is an appropriate testing pattern.Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/publish-artifact.gradle (2)
99-99: LGTM!Version bump to 1.7.0-SNAPSHOT is appropriate for the new feature additions in this PR.
113-113: LGTM!Version bump to 1.7.0-SNAPSHOT is consistent with the aar publication and appropriate for the new features.
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json (1)
114-114: Static analysis false positive - DID is not a secret.The static analysis tool flagged this line as containing an API key, but this is a DID (Decentralized Identifier) following the
did:keymethod specification. DIDs are public identifiers designed to be shared and are not secrets. This is a false positive.vc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.json (1)
1-103: LGTM!Well-structured JSON-LD context for test resources. The multilingual field mappings and namespace declarations follow JSON-LD best practices and support the expanded test coverage for credential verification.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.json (1)
1-94: LGTM!Well-formed test fixture for a revoked Verifiable Credential. The
credentialStatusstructure properly follows the Bitstring Status List specification and supports the new status-checking functionality introduced in this PR.vc-verifier/kotlin/gradle/libs.versions.toml (1)
66-67: New dependencies support CWT verification.The addition of
jackson-dataformat-cborandcose-javaappropriately supports the new CWT (CBOR Web Token) verification functionality introduced in this PR.README.md (2)
41-41: Well-documented new feature.The addition of the "Status Check" feature description clearly communicates the new credential status verification capability.
46-51: Table accurately reflects signature support.The updates to the VC formats table correctly document the addition of
EcdsaSecp256k1Signature2019support and clarify that SD-JWT formats use X.509 certificates rather than specific signature suites.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
152-159: Hex conversion implementation is correct.The
hexToBytesfunction properly validates input (even length after removing whitespace) and correctly converts hex pairs to bytes.
161-175: CBOR numeric date validation is well-implemented.The
validateNumericDatefunction correctly enforces CBOR type requirements for numeric dates, providing clear error messages and proper null handling for optional claims.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt (2)
29-30: Good test isolation with HTTP mocking.The addition of
mockkObject(NetworkManagerClient)andloadMockPublicKeys()properly isolates tests from external network dependencies, ensuring deterministic and fast test execution.
98-104: Mock setup is well-organized.The
loadMockPublicKeys()helper cleanly organizes HTTP response mocking for multiple public key endpoints, improving test maintainability.vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json (2)
112-114: Static analysis false positive - DID is not an API key.The Gitleaks warning about a "Generic API Key" on line 114 is a false positive. The
holderfield contains a valid DID (did:key:...), which is the expected format for identifying the presentation holder.The JWS signature at line 112 contains
.hello.as a placeholder payload, which is appropriate for test fixtures where the actual signature content is mocked.
14-14: Remove PII from test fixtures.The email address appears to be real or realistic personal data. Use clearly fake data (e.g.,
"test@example.com") to avoid GDPR/CCPA compliance risks.Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt (1)
9-11: Good use of default implementation for backward compatibility.Providing a default implementation that throws
UnsupportedOperationExceptionis appropriate for extending the interface without breaking existing implementations. Formats that don't support status checking will get a clear error message.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt (2)
173-183: New public APIverifyAndGetCredentialStatuslooks good.The method provides a clean way to get combined VP verification and per-credential status results. The default empty list for
statusPurposeListis a sensible default.
47-48: JSON parsing not wrapped in error handling.
JSONObject(presentation).getJSONArray(...)can throwJSONExceptionif theverifiableCredentialkey is missing or malformed. If VP proof verification succeeds but the array is missing, an unhandled exception propagates.Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.kt (1)
1-16: Well-structured exception with typed error codes.The
StatusCheckExceptionwithStatusCheckErrorCodeenum provides clear categorization of status-check failures. Error codes cover the expected failure modes comprehensively.Note: File is missing a trailing newline (minor style issue).
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifer.kt (4)
38-46: LGTM: COSE structure validation is thorough.The validation correctly checks for the COSE Sign1 structure (4-element array with expected types: protected header, unprotected header, payload, signature as byte strings, and unprotected header as map).
103-116: LGTM: Signature verification properly wraps exceptions.The
verifySignaturemethod correctly catches exceptions and returnsfalseon failure, preventing exception propagation during verification.
15-15: Typo in class name:CwtVerifer→CwtVerifier.The class name is missing an 'i'.
Likely an incorrect or invalid review comment.
17-18: Remove unusedcborMapperfield.The
ObjectMapperwithCBORFactoryis initialized but never used in this class.Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt (1)
5-5: Typo:CwtVerifershould beCwtVerifier.The imported class name is misspelled (missing 'i').
Likely an incorrect or invalid review comment.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.kt (1)
1-13: LGTM!The constants are well-defined and align with the W3C StatusList2021 specification field names.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidator.kt (4)
24-59: LGTM!The
validatemethod follows a clear validation pipeline with appropriate error handling for both specific validation failures and unexpected exceptions.
62-109: LGTM!The COSE_Sign1 structure validation correctly checks the RFC 8152 requirements: array with 4 elements where protected header and payload are byte strings, unprotected header is a map, and signature is a byte string.
112-136: LGTM!Protected header validation correctly ensures ALG (COSE label 1) is present and numeric.
150-185: LGTM!The numeric date validation correctly implements CWT temporal claim checks using the standard CBOR integer keys (4=exp, 5=nbf, 6=iat) with appropriate boundary conditions.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (2)
17-31: LGTM!The new data classes
PresentationResultWithCredentialStatusandVCResultWithCredentialStatusprovide clean wrappers for combining verification status with credential status results.
57-65: LGTM!
CredentialStatusResultandCredentialVerificationSummaryproperly model the status check outcomes with nullable error for failure cases.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/CwtValidatorTest.kt (2)
19-32: LGTM!The
createCoseHexhelper is well-designed with sensible defaults, making it easy to test specific failure cases by overriding individual components.
116-151: LGTM!The success path test thoroughly validates a complete valid COSE/CWT structure with proper temporal claims, confirming the validator returns empty error code and message on success.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (3)
18-18: LGTM!Using a shared
credentialVerifierFactoryinstance is a good improvement over creating new instances inline.
41-74: LGTM!The
verifymethod correctly separates validation and signature verification with appropriate fallback error codes.
76-104: LGTM!The status checking methods are well-designed:
getCredentialStatusgracefully handles unsupported formats by returning an empty mapverifyAndGetCredentialStatuscorrectly skips status checks for failed verifications, avoiding unnecessary workvc-verifier/kotlin/README.md (1)
183-246: LGTM!The credential status check documentation clearly explains the StatusList2021 flow, bit extraction logic, and result structure with helpful tables.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (4)
62-124: LGTM!The
getStatusesmethod correctly handles both single and list forms ofcredentialStatus, with case-insensitive purpose filtering and per-entry error isolation.
226-283: LGTM!The status computation correctly implements the StatusList2021 bit extraction: decoding the encoded list, computing bit positions based on index and status size, and interpreting 0 as valid.
362-388: LGTM!The bit extraction correctly uses big-endian bit ordering where bit 0 is the MSB within each byte, matching the StatusList2021 specification.
169-201: MovevalidFromandvalidUntilvalidation to read from the top-level VC, notcredentialSubject.Per the W3C StatusList2021 specification and the actual test data structure,
validFromandvalidUntilare properties of the status list VC itself, not the credential subject. The code currently reads these fromcredentialSubject(lines 169–170), which will fail to locate them and silently skip validation. Change to read fromstatusListVC.jsonObjectinstead.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (2)
157-168: LGTM!The test correctly verifies that when credential verification fails, the status list is empty. The assertions properly check both the verification failure and the empty status list.
202-228: LGTM!The test correctly verifies that a revoked VC returns an invalid status result. The mocking setup and assertions are appropriate.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (2)
27-79: LGTM!The test setup, teardown, and helper methods are well-structured. The
prepareVCandprepareVCFromRawmethods correctly return theMockWebServerinstance to the caller, making it clear that the caller is responsible for shutting down the server.
81-452: Excellent test coverage!The test suite comprehensively covers various edge cases and error scenarios for status list checking, including:
- Valid and invalid status checks
- Missing or malformed fields
- Encoding/decoding errors
- Purpose mismatches
- Temporal validity checks
- Status size and message validation
All tests (except the one flagged separately) properly manage MockWebServer lifecycle by shutting down servers after use.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.json (1)
1-28: The nested @context in verificationMethod is an intentional test pattern.This file follows a consistent structure used across multiple DID test resources (
didEcKey.json,didEdPublicKey.json, anddidMosipKey.jsonall contain nested@contextin verificationMethod objects). While nested@contextis non-standard per W3C DID Core spec (which places it only at root level), it appears to be deliberately used in these test cases to verify how the verifier handles security context information attached directly to verification methods. If the verifier correctly handles this pattern in tests, no changes are needed.
| jacksonAnnotations = "2.17.2" | ||
| jacksonModuleKotlin = "2.17.2" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jackson 2.14.0 CVE vulnerabilities security issues
💡 Result:
Short answer: Jackson (jackson-databind) 2.14.0 mainly contains fixes for many prior "unsafe deserialization" CVEs, but the jackson-databind family has a long history of deserialization and resource‑exhaustion vulnerabilities. Distributors/security trackers mark several high‑severity deserialization CVEs as fixed by the 2.14.x fixes; additional denial‑of‑service / resource exhaustion issues have also been addressed in later 2.11–2.14 series releases. Upgrade to a patched release (or the latest 2.15+/maintenance release) and avoid enabling DefaultTyping / untrusted polymorphic deserialization. Sources: Snyk vulnerability list, Debian security tracker, CVE summary. [1][2][3]
References
- Snyk jackson-databind vulnerability summary. [1]
- Debian security tracker entries showing fixes in 2.14.0 packaging. [2]
- CVE/CVEdetails summary for 2.14.x. [3]
If you want, I can run a live check of the very latest jackson-databind release and list specific CVE numbers and which are fixed in 2.14.0 vs later releases.
🌐 Web query:
Jackson breaking changes 2.14.0 to 2.17.2 upgrade guide
💡 Result:
Below is a concise upgrade checklist noting the important breaking/behavior changes and recommended actions when moving from Jackson 2.14.0 → 2.17.2.
High-impact changes to review and adapt for
- Java / platform: Jackson 2.14 raised baseline to Java 8 for most components. Confirm your runtime/CI use Java 8+ (and Android SDK minimum changes noted). [1]
- JsonNode API/behavior:
- JsonNode.with(...) and withArray(...) treat arguments starting with "/" as JsonPointer expressions since 2.14; use withObjectProperty/withArrayProperty (added later) if you relied on literal names containing slashes. [1][3]
- Annotation/visibility/ignore semantics:
- Handling of conflicting @JsonIgnore vs @JsonProperty changed (2.14): @JsonIgnore now generally wins in ambiguous cases — audit any code relying on previous precedence. [1]
- Stream / parsing constraints and defaults:
- Starting in 2.15+ Jackson introduced StreamReadConstraints (limits on textual length, nesting, numeric sizes) and defaults that may cause deserialization failures for very large payloads; either raise/override constraints or explicitly configure JsonFactory/StreamReadConstraints if you process large inputs. [2][4]
- Number parsing/formatting:
- New faster float/double parsing/writing options (Schubfach) and changes to numeric handling (multi-release JAR optimizations) exist since 2.14–2.15; these are opt-in features but numeric behavior and rounding/parsing of corner cases (e.g., "3.") have been tightened/fixed across 2.15–2.17 — run numeric tests. [1][2][4]
- Enums / mix-ins / naming:
- 2.15–2.16 added better enum annotation support, new EnumNaming/EnumFeature options, and mix-in fixes for enums; if you used enum mix-ins or custom naming, re-test and consider the new annotation options. [3][2]
- Polymorphic typing / type id handling:
- 2.16 introduced per-type strict type-id options (OptBoolean in @JsonTypeInfo, JsonTypeInfo.Value) and new Id.SIMPLE_NAME — review polymorphic type handling and any custom type-id code. [5]
- Kotlin/other language modules:
- 2.15+ changed Kotlin support (no longer Kotlin 1.4 supported) — check module compatibility for kotlin/scala modules you use. [2]
- Transitive dependency changes:
- SnakeYAML bumped for YAML module, and other module dependency versions changed (check your BOM/custom dependency constraints). [2]
- Bugfixes you may benefit from:
- 2.17.x patch releases (2.17.2) contain many fixes for Reactor interaction, BigDecimal parsing edge-cases and other regressions — if you hit those issues upgrade to latest 2.17.x. [4]
Recommended upgrade steps
- Update BOM / dependencies to Jackson 2.17.2 (or latest 2.17.x) in your build (Maven/Gradle). [4]
- Run full test suite (unit + integration) — focus on:
- JSON parsing of large strings, deep nesting and large numbers (StreamReadConstraints).
- Enum serialization/deserialization, enum mix-ins, and any custom EnumNaming usage.
- Polymorphic type handling and @JsonTypeInfo-annotated types.
- Any code relying on JsonNode.with(...) semantics or on @JsonIgnore/@JsonProperty precedence.
- If you encounter "limit" errors (too long string, depth, numeric size), configure StreamReadConstraints or JsonFactory.overrideDefaultStreamReadConstraints(...) to the values your application needs. [2][4]
- If you use Kotlin/Scala modules, ensure module versions match supported language versions (2.15+ Kotlin change). [2]
- Check transitive deps (SnakeYAML, Guava baseline, Woodstox etc.) and adjust BOM pins or exclusions if your ecosystem requires older versions. [2]
- Smoke-test performance-sensitive code paths that parse/serialize floating point numbers if you opt-in to new fast parsers/writers. [1][2]
- Review changelogs of intermediate releases (2.15, 2.16, 2.17.2) for any additional fixes affecting your code and for patch-level bugfixes to apply. [2][3][4]
Primary sources / release notes (read before upgrading)
- Jackson 2.14 release notes (behavior/API changes). [1]
- Jackson 2.15 release notes (StreamReadConstraints, multi-release jar, module changes, enum features). [2]
- Jackson 2.16 overview / 2.16.x notes (enum improvements, JsonNode additions, JsonTypeInfo per-type config). [3][5]
- Jackson 2.17.2 release notes (patch fixes, Reactor/BigDecimal/edge-case fixes). [4]
If you want, I can:
- produce a diff-style checklist for your codebase (search for usages of JsonNode.with, @JsonIgnore/@JsonProperty, @JsonTypeInfo, enum mix-ins), or
- generate concrete JsonFactory/StreamReadConstraints sample code to override defaults. Which would you prefer?
References
[1] Jackson Release 2.14 — FasterXML (release notes).
[2] Jackson Release 2.15 — FasterXML (release notes).
[3] Jackson 2.16 overview (cowtowncoder blog / rc notes).
[4] Jackson Release 2.17.2 — FasterXML (release notes).
[5] Jackson 2.16.1 / 2.16.x summary (package/announcements).
Test Jackson upgrade to 2.17.2 for breaking changes before merging.
The upgrade spans multiple minor versions with several breaking changes requiring verification:
- StreamReadConstraints (2.15+): May reject deserialization of large strings, deep nesting, or large numbers; verify your payload sizes don't exceed new defaults or configure
StreamReadConstraintsif needed. - JsonNode.with() (2.14+): Arguments starting with "/" are now treated as JsonPointer expressions; audit code relying on literal "/" in names.
- @JsonIgnore/@JsonProperty (2.14+): Precedence rules changed; @JsonIgnore now generally wins in conflicts.
- Enum handling (2.15+): New enum annotation support and naming options; re-test any enum serialization/deserialization.
- Polymorphic types (2.16+): Per-type strict type-id options added; review @JsonTypeInfo-annotated types.
- Kotlin module compatibility (2.15+): Module no longer supports Kotlin 1.4; confirm your Kotlin version matches.
- Transitive dependencies: SnakeYAML, Guava, Woodstox versions changed; verify no conflicts in your dependency tree.
Additionally, 2.17.2 includes security patches and BigDecimal/Reactor fixes beyond what 2.14.0 had. Run full test suite, especially for JSON parsing of large/nested data, enum handling, and polymorphic deserialization.
🤖 Prompt for AI Agents
In @vc-verifier/kotlin/gradle/libs.versions.toml around lines 23-24, You
upgraded jacksonAnnotations and jacksonModuleKotlin to 2.17.2; before merging,
run the full test suite and explicitly verify breaking-change areas: confirm
StreamReadConstraints limits won't reject large strings/deep nesting or set
custom StreamReadConstraints in Jackson configuration; audit any uses of
JsonNode.with() for keys starting with "/" and update callers expecting literal
"/" behavior; re-test places using @JsonIgnore/@JsonProperty to ensure
precedence changes don't break mappings; exercise all enum
(serialize/deserialize) paths and @JsonTypeInfo polymorphic deserialization to
catch behavioral changes; verify Kotlin compatibility (module requires Kotlin
>=1.5), and check the dependency tree for transitive changes
(SnakeYAML/Guava/Woodstox) that could cause conflicts. Ensure CI includes JSON
parsing tests for large/nested payloads, enum cases, and polymorphic types
before approving the version bump of jacksonAnnotations and jacksonModuleKotlin.
| |--------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| | `ldp_vc` | W3C Linked Data Proof Verifiable Credentials [(v2.0)](https://www.w3.org/TR/vc-data-model-2.0/) ([v1.1](https://www.w3.org/TR/vc-data-model-1.1/)) | | ||
| | `mso_mdoc` | ISO/IEC 18013-5 compliant mobile documents | | ||
| | `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/10/) | |
There was a problem hiding this comment.
Update SD-JWT VC specification reference to latest draft.
The README references draft-ietf-oauth-sd-jwt-vc/10, but the latest version is draft-ietf-oauth-sd-jwt-vc-13 (published November 2025).
🔎 Proposed fix
-| `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/10/) |
+| `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/13/) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/10/) | | |
| | `vc+sd-jwt`, `dc+sd-jwt` | [IETF SD-JWT based Verifiable Credentials](https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/13/) | |
🤖 Prompt for AI Agents
In @vc-verifier/kotlin/README.md around line 42, Update the SD-JWT VC spec link
and label in the README table entry for `vc+sd-jwt`, `dc+sd-jwt` to point to the
latest draft (change the URL from draft-ietf-oauth-sd-jwt-vc/10 to
draft-ietf-oauth-sd-jwt-vc-13) and, if present, update any displayed version
number text to "-13" or remove the version suffix so it reflects the current
draft; modify the table row containing the string `vc+sd-jwt`, `dc+sd-jwt` and
the linked URL to use the new draft URL.
| ```json | ||
| { | ||
| "verificationResult": { | ||
| "verificationStatus": true, | ||
| "verificationMessage": "VC is expired", | ||
| "verificationErrorCode": "" | ||
| }, | ||
| "credentialStatus": { | ||
| "revocation": { | ||
| "isValid": false, | ||
| "error": null | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| The VC is expired and revoked. |
There was a problem hiding this comment.
Inconsistent example: expired VC shows verificationStatus: true.
The example text says "The VC is expired and revoked" but shows verificationStatus: true. An expired VC should have verificationStatus: false with an appropriate error code.
🔎 Proposed fix
{
"verificationResult": {
- "verificationStatus": true,
+ "verificationStatus": false,
"verificationMessage": "VC is expired",
- "verificationErrorCode": ""
+ "verificationErrorCode": "ERR_VC_EXPIRED"
},🤖 Prompt for AI Agents
In @vc-verifier/kotlin/README.md around lines 394-411, The example JSON in
README uses verificationResult.verificationStatus: true while describing an
expired & revoked VC; update the example so
verificationResult.verificationStatus is false, set
verificationResult.verificationErrorCode to a suitable code (e.g., "EXPIRED")
and keep verificationResult.verificationMessage as "VC is expired", and ensure
credentialStatus.revocation.isValid remains false to reflect revocation.
| override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { | ||
| return emptyMap(); | ||
| } |
There was a problem hiding this comment.
Returning empty map vs throwing UnsupportedOperationException.
The interface's default implementation throws UnsupportedOperationException, but this override returns an empty map. This creates ambiguity:
- Empty map: "no status entries found" (successful check, nothing to report)
- Exception: "format doesn't support status checking"
Consider either throwing the exception to be consistent with the interface contract, or documenting that an empty map means "not supported" for CWT format.
🔎 Suggested fix to align with interface semantics
override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> {
- return emptyMap();
+ throw UnsupportedOperationException("Credential status checking not supported for CWT format")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { | |
| return emptyMap(); | |
| } | |
| override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { | |
| throw UnsupportedOperationException("Credential status checking not supported for CWT format") | |
| } |
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt
around lines 18-20, The CwtVerifiableCredential.checkStatus override currently
returns an empty map which contradicts the interface's default behavior; change
the implementation in CwtVerifiableCredential.checkStatus to throw
UnsupportedOperationException (or, if you prefer the empty-map semantics, add
clear javadoc to the method stating that an empty map denotes "not supported"
for CWT), but prefer throwing UnsupportedOperationException to align with the
interface contract so callers receive a clear error when status checking isn't
supported by CWT.
| override fun checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> { | ||
| return LdpStatusChecker().getStatuses(credential, statusPurposes) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -iname "README*" -o -iname "CHANGELOG*" -o -iname "BREAKING*" | head -20Repository: inji/vc-verifier
Length of output: 102
🏁 Script executed:
rg -i "breaking.*change|migration.*guide" --type md | head -30Repository: inji/vc-verifier
Length of output: 341
🏁 Script executed:
rg -i "isRevoked|checkStatus" README.md CHANGELOG.md 2>/dev/null | head -20Repository: inji/vc-verifier
Length of output: 216
🏁 Script executed:
cat README.md | grep -A 20 "## 🚨 Breaking Changes"Repository: inji/vc-verifier
Length of output: 677
🏁 Script executed:
rg -A 10 -B 2 "isRevoked|checkStatus" README.mdRepository: inji/vc-verifier
Length of output: 927
Document the breaking API change from isRevoked to checkStatus.
The replacement of isRevoked(credential: String): Boolean with checkStatus(credential: String, statusPurposes: List<String>?): Map<String, CredentialStatusResult> is a breaking API change that is not currently documented. The README still references the old isRevoked(credential) method and does not list this signature change in the Breaking Changes section.
Update the Breaking Changes section in README.md to include this API change and provide migration guidance for users upgrading from versions that used isRevoked.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt
around lines 19-21, Add a Breaking Changes entry in README.md documenting that
the LdpVerifiableCredential API changed: the old isRevoked(credential: String):
Boolean method was removed and replaced by checkStatus(credential: String,
statusPurposes: List<String>?): Map<String, CredentialStatusResult>; explain how
callers should migrate by replacing boolean checks with calls to
LdpVerifiableCredential.checkStatus, passing any relevant statusPurposes (or
null) and inspecting the returned Map<String, CredentialStatusResult> for the
credential's status (e.g., map lookup for the expected purpose and interpreting
CredentialStatusResult fields), and include a short example snippet and notes
about nullability of statusPurposes and how multiple purposes are represented in
the map.
|
|
||
| object Util { | ||
|
|
||
| val restTemplate = RestTemplate() |
There was a problem hiding this comment.
Configure RestTemplate with timeouts to prevent hanging.
The RestTemplate instance lacks timeout configuration. When making HTTP calls to external services (e.g., for key resolution or status checks), missing timeouts can cause indefinite hangs if remote services are unresponsive, impacting application reliability.
🔎 Proposed fix with connection and read timeouts
+import org.springframework.boot.web.client.RestTemplateBuilder
+import java.time.Duration
+
object Util {
- val restTemplate = RestTemplate()
+ val restTemplate = RestTemplateBuilder()
+ .setConnectTimeout(Duration.ofSeconds(10))
+ .setReadTimeout(Duration.ofSeconds(10))
+ .build()Alternatively, if RestTemplateBuilder is not available:
+import org.springframework.http.client.SimpleClientHttpRequestFactory
+
object Util {
- val restTemplate = RestTemplate()
+ val restTemplate = RestTemplate().apply {
+ requestFactory = SimpleClientHttpRequestFactory().apply {
+ setConnectTimeout(10_000) // 10 seconds
+ setReadTimeout(10_000) // 10 seconds
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val restTemplate = RestTemplate() | |
| import org.springframework.boot.web.client.RestTemplateBuilder | |
| import java.time.Duration | |
| object Util { | |
| val restTemplate = RestTemplateBuilder() | |
| .setConnectTimeout(Duration.ofSeconds(10)) | |
| .setReadTimeout(Duration.ofSeconds(10)) | |
| .build() |
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
around line 36, The RestTemplate instance declared as val restTemplate =
RestTemplate() in Util.kt has no timeouts configured and can hang; replace it
with a RestTemplate configured with sensible connection and read/socket timeouts
(e.g., via RestTemplateBuilder or by creating an
HttpComponentsClientHttpRequestFactory/OkHttp3ClientHttpRequestFactory and
setting connect and read timeouts) and use that configured instance everywhere
the restTemplate variable is referenced; ensure the chosen timeout values are
reasonable for your environment and include them when constructing the
RestTemplate.
| fun httpGet(url: String): String? { | ||
| return try { | ||
| restTemplate.getForObject(url, String::class.java) | ||
| } catch (e: Exception) { | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
Add logging for HTTP failures to aid debugging.
The httpGet method silently swallows all exceptions and returns null, making it difficult to diagnose failures in production. Consider logging the exception at minimum, or returning a Result type that preserves error information.
🔎 Proposed fix with logging
Add logging import if not already present:
import org.slf4j.LoggerFactoryThen update the method:
+ private val logger = LoggerFactory.getLogger(Util::class.java)
+
fun httpGet(url: String): String? {
return try {
restTemplate.getForObject(url, String::class.java)
} catch (e: Exception) {
+ logger.warn("HTTP GET failed for URL: $url", e)
null
}
}🧰 Tools
🪛 detekt (1.23.8)
[warning] 180-180: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
around lines 177-183, The httpGet function currently swallows all exceptions;
add logging so failures are visible: import org.slf4j.LoggerFactory, declare a
logger for Util (e.g., private val logger =
LoggerFactory.getLogger(Util::class.java) or similar), and inside the catch in
httpGet(url: String) log an error including the url and the exception (e.g.,
logger.error("HTTP GET failed for url: $url", e)) before returning null; keep
the method signature and restTemplate.getForObject usage unchanged.
| @Test | ||
| fun `should return error when statusSize is zero or negative`() { | ||
| val statusListJsonNegative = readFile("classpath:ldp_vc/status-list-vc.json") | ||
| .replace("\"encodedList\"", "\"statusSize\": -2, \"encodedList\"") | ||
| val statusListJsonZero = readFile("classpath:ldp_vc/status-list-vc.json") | ||
| .replace("\"encodedList\"", "\"statusSize\": 0, \"encodedList\"") | ||
| val (replacedVC, server) = prepareVC(statusListJson = statusListJsonNegative) | ||
| val (replacedVCZero, _) = prepareVC( | ||
| statusListJson = statusListJsonZero | ||
| ) | ||
|
|
||
| val resultNeg = checker.getStatuses(replacedVC).entries.first() | ||
|
|
||
| assertFalse(resultNeg.value.isValid) | ||
| assertEquals( | ||
| StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, | ||
| resultNeg.value.error?.errorCode | ||
| ) | ||
|
|
||
| val resultZero = checker.getStatuses(replacedVCZero).entries.first() | ||
|
|
||
| assertFalse(resultZero.value.isValid) | ||
| assertEquals( | ||
| StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, | ||
| resultZero.value.error?.errorCode | ||
| ) | ||
|
|
||
| server.shutdown() | ||
| } |
There was a problem hiding this comment.
Resource leak: second MockWebServer not shut down.
The test creates two mock web servers (lines 273 and 274-276) but only shuts down the first one at line 294. The server created for replacedVCZero is never shut down.
🔎 Proposed fix
val (replacedVC, server) = prepareVC(statusListJson = statusListJsonNegative)
- val (replacedVCZero, _) = prepareVC(
+ val (replacedVCZero, serverZero) = prepareVC(
statusListJson = statusListJsonZero
)
val resultNeg = checker.getStatuses(replacedVC).entries.first()
assertFalse(resultNeg.value.isValid)
assertEquals(
StatusCheckErrorCode.STATUS_VERIFICATION_ERROR,
resultNeg.value.error?.errorCode
)
val resultZero = checker.getStatuses(replacedVCZero).entries.first()
assertFalse(resultZero.value.isValid)
assertEquals(
StatusCheckErrorCode.STATUS_VERIFICATION_ERROR,
resultZero.value.error?.errorCode
)
server.shutdown()
+ serverZero.shutdown()
}🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt
around lines 267-295, Test leaks the second MockWebServer because prepareVC(...)
for the zero case discards the server; capture that server (e.g., val
(replacedVCZero, serverZero) = prepareVC(statusListJson = statusListJsonZero))
and call serverZero.shutdown() at the end (or shut both down in a
finally/tearDown block). Update the underscore in the second prepareVC call to a
named variable and ensure shutdown is invoked to close the resource.
| @Test | ||
| fun `should throw exception for mso_mdoc status check as its not supported`() { | ||
| val file = ResourceUtils.getFile(ResourceUtils.CLASSPATH_URL_PREFIX + "sd-jwt_vc/sdJwt.txt") | ||
| val vc = String(Files.readAllBytes(file.toPath())) | ||
| val unsupportedStatusCheckException = assertThrows(UnsupportedOperationException::class.java) { | ||
| MsoMdocVerifiableCredential().checkStatus(vc, null) | ||
| } | ||
|
|
||
| assertEquals("Credential status checking not supported for this credential format",unsupportedStatusCheckException.message) | ||
| } |
There was a problem hiding this comment.
Test remains misplaced and loads irrelevant SD-JWT file.
This issue was previously flagged but not addressed. The test verifies MsoMdocVerifiableCredential behavior but is in SdJwtVerifierTest. Move it to a dedicated MsoMdocVerifiableCredentialTest file and remove the unnecessary SD-JWT file loading (lines 34-35), as the file content is irrelevant for testing unsupported status check behavior.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt
around lines 32-41, The test asserting UnsupportedOperationException for
MsoMdocVerifiableCredential.checkStatus is misplaced in SdJwtVerifierTest and
unnecessarily reads an SD-JWT file; move the entire test to a new test class
MsoMdocVerifiableCredentialTest (create file/class named
MsoMdocVerifiableCredentialTest) and remove the ResourceUtils file-loading lines
(the lines that set file and vc); call MsoMdocVerifiableCredential().checkStatus
with a simple placeholder (e.g., empty string) or null for the VC parameter and
keep the assertThrows and assertEquals checking the message unchanged.
| } | ||
| ], | ||
| "id": "did:jwk:eyJrdHkiOiJPS1AiLCJ1c2UiOiJzaWciLCJjcnYiOiJFZDI1NTE5IiwieCI6IlVFSWlCZXk3Y0xUakNxZVZpejZBWFFEV2lvR2pNNXo0M21qV3NCTjRwTTgifQ==", | ||
| "phone": "+919427357934", |
There was a problem hiding this comment.
Remove PII from test fixtures.
The phone number appears to be realistic personal data. Use obviously fake data (e.g., "+1-555-0100") to avoid compliance concerns.
🤖 Prompt for AI Agents
In @vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json
around line 60, The test fixture contains real-looking PII in the "phone" JSON
field; replace the value in the VPWithRevokedVC fixture's "phone" key with an
obviously fake number (e.g., "+1-555-0100") to remove personal data, ensuring
any other test assertions still accept the updated fake value.
Signed-off-by: jaswanthkumarpolisetty <jaswanthkumar.p@thoughtworks.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
61-69: Revocation is properly handled through the new credential status checking mechanism, but theREVOKEDenum value should be removed.Revocation is not lost—it's now handled via the
credentialStatusmap inCredentialVerificationSummaryandVCResultWithCredentialStatus, populated byLdpStatusChecker.getStatuses(). Tests confirm this works correctly (e.g., revocation checks returnCredentialStatusResultwithisValid = falsefor revoked credentials).However, the
REVOKEDenum value inVerificationStatusis now orphaned—never returned bygetVerificationStatus(). Remove it to avoid confusion and maintain a single source of truth for revocation status through thecredentialStatusmap.
🤖 Fix all issues with AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt:
- Around line 72-98: The methods resolveIssuerMetadata and fetchPublicKey are
dead code and should be removed: delete the private functions
resolveIssuerMetadata(issuer: String): String? and fetchPublicKey(coseObj:
CBORObject, issuerMetadataJson: String): PublicKey? from CwtVerifier.kt and
remove any imports that become unused as a result (e.g., org.json.JSONObject,
JWKSet, RSAKey, ECKey, OctetKeyPair, CBORObject-related imports and any JSON/JWK
libraries only referenced by those methods) to keep the class tidy; leave
verify() and the existing PublicKeyResolverFactory().get(issuer) usage
unchanged.
- Around line 148-152: The call to PublicKeyResolverFactory().get(issuer) can
throw PublicKeyTypeNotSupportedException even though it is non-nullable; wrap
the resolver call in a try-catch that catches PublicKeyTypeNotSupportedException
(alongside any existing exception handling pattern used in this class), log or
handle the exception consistently, and return false on failure; specifically
modify the code around extractIssuerUri(claims) and the call to
PublicKeyResolverFactory().get(issuer) in CwtVerifier (referencing
extractIssuerUri, PublicKeyResolverFactory.get, and verifySignature) so
unsupported public key types do not propagate uncaught.
🧹 Nitpick comments (3)
vc-verifier/kotlin/vcverifier/build.gradle.kts (1)
11-13: Consolidate duplicateconfigurations.allblocks.There are two separate
configurations.allblocks in this file. Consider merging them for better maintainability.🔎 Proposed consolidation
configurations.all { resolutionStrategy.force( "com.fasterxml.jackson.core:jackson-core:2.14.0") + exclude(group = "org.bouncycastle", module = "bcprov-jdk15on") + exclude(group = "org.bouncycastle", module = "bcpkix-jdk15on") + exclude(group = "org.bouncycastle", module = "bcutil-jdk15on") + exclude(group = "org.bouncycastle", module = "bcprov-jdk15to18") } -... - -configurations.all { - exclude(group = "org.bouncycastle", module = "bcprov-jdk15on") - exclude(group = "org.bouncycastle", module = "bcpkix-jdk15on") - exclude(group = "org.bouncycastle", module = "bcutil-jdk15on") - exclude(group = "org.bouncycastle", module = "bcprov-jdk15to18") -}Also applies to: 76-81
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.kt (1)
10-16: Consider reusing validator/verifier instances.Creating new
CwtValidator()andCwtVerifier()instances on each method call adds unnecessary object allocation. Consider using singleton instances or dependency injection if these classes are stateless.🔎 Proposed fix
-class CwtVerifiableCredential: VerifiableCredential { +class CwtVerifiableCredential: VerifiableCredential { + private val validator = CwtValidator() + private val verifier = CwtVerifier() + override fun validate(credential: String): ValidationStatus { - return CwtValidator().validate(credential) + return validator.validate(credential) } override fun verify(credential: String): Boolean { - return CwtVerifier().verify(credential); + return verifier.verify(credential) }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt (1)
3-3: Remove unused import.
android.net.Uriis imported but never used. The code usesjava.net.URIinstead.🔎 Proposed fix
-import android.net.Uri
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
vc-verifier/kotlin/example/build.gradle.ktsvc-verifier/kotlin/gradle/libs.versions.tomlvc-verifier/kotlin/vcverifier/build.gradle.ktsvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/CwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifierTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- vc-verifier/kotlin/gradle/libs.versions.toml
🧰 Additional context used
🧬 Code graph analysis (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
httpGet(177-183)
🔇 Additional comments (5)
vc-verifier/kotlin/example/build.gradle.kts (1)
49-51: LGTM!The additional exclusion for the nested OSGI manifest path is appropriate since the wildcard
META-INF/*only matches direct children. This prevents packaging conflicts from the new COSE library dependency.vc-verifier/kotlin/vcverifier/build.gradle.kts (1)
68-68: Good choice usingse.digg.cose:cose-lib.This addresses the previous concern about using the unmaintained
com.augustcellars.cose:cose-java. These.digg.cose:cose-libis an actively maintained alternative.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (3)
5-6: LGTM! CBOR imports support CWT credential handling.The addition of CBOR imports is appropriate for implementing CWT (CBOR Web Token) verification capabilities.
24-24: LGTM! RestTemplate import needed for HTTP utilities.The RestTemplate import supports the new httpGet utility function for external HTTP calls.
152-159: LGTM! Hex to bytes conversion is correctly implemented.The function properly validates input (even length requirement) and handles the conversion correctly. The use of
require()for validation appropriately throwsIllegalArgumentExceptionon malformed input.
| private fun resolveIssuerMetadata(issuer: String): String? { | ||
| val metadataUrl = "$issuer/.well-known/openid-credential-issuer" | ||
| return httpGet(metadataUrl) | ||
| } | ||
|
|
||
| private fun fetchPublicKey( | ||
| coseObj: CBORObject, | ||
| issuerMetadataJson: String | ||
| ): PublicKey? { | ||
|
|
||
| val metadata = org.json.JSONObject(issuerMetadataJson) | ||
| val jwksUri = metadata.optString("jwks_uri", null) ?: return null | ||
|
|
||
| val jwksJson = httpGet(jwksUri) ?: return null | ||
| val jwkSet = JWKSet.parse(jwksJson) | ||
|
|
||
| val kid = extractKid(coseObj) ?: return null | ||
|
|
||
| val jwk = jwkSet.keys.firstOrNull { it.keyID == kid } ?: return null | ||
|
|
||
| return when (jwk) { | ||
| is RSAKey -> jwk.toRSAPublicKey() | ||
| is ECKey -> jwk.toECPublicKey() | ||
| is OctetKeyPair -> jwk.toPublicKey() | ||
| else -> null | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if resolveIssuerMetadata or fetchPublicKey are used anywhere
echo "=== Searching for usages of resolveIssuerMetadata ==="
rg -n "resolveIssuerMetadata" --type=kotlin
echo ""
echo "=== Searching for usages of fetchPublicKey ==="
rg -n "fetchPublicKey" --type=kotlin
echo ""
echo "=== Searching for calls in verify() method ==="
rg -n "verify" vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt --type=kotlin -A 10 -B 2Repository: inji/vc-verifier
Length of output: 1599
Remove unused methods: resolveIssuerMetadata and fetchPublicKey are dead code.
These methods are never called anywhere in the codebase. The verify() method obtains the public key directly via PublicKeyResolverFactory().get(issuer) (line 150), bypassing both methods entirely. Remove them to reduce code clutter.
🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt
around lines 72 - 98, The methods resolveIssuerMetadata and fetchPublicKey are
dead code and should be removed: delete the private functions
resolveIssuerMetadata(issuer: String): String? and fetchPublicKey(coseObj:
CBORObject, issuerMetadataJson: String): PublicKey? from CwtVerifier.kt and
remove any imports that become unused as a result (e.g., org.json.JSONObject,
JWKSet, RSAKey, ECKey, OctetKeyPair, CBORObject-related imports and any JSON/JWK
libraries only referenced by those methods) to keep the class tidy; leave
verify() and the existing PublicKeyResolverFactory().get(issuer) usage
unchanged.
| val issuer = extractIssuerUri(claims) ?: return false; | ||
|
|
||
| val publicKey = PublicKeyResolverFactory().get(issuer) | ||
|
|
||
| return verifySignature(coseBytes, publicKey) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Finding PublicKeyResolverFactory implementation ==="
find . -type f -name "*.kt" | xargs grep -l "class PublicKeyResolverFactory"
echo ""
echo "=== Searching for PublicKeyResolverFactory in all Kotlin files ==="
fd --type f -e kt | xargs grep -l "PublicKeyResolverFactory" | head -10Repository: inji/vc-verifier
Length of output: 969
🏁 Script executed:
#!/bin/bash
echo "=== Reading PublicKeyResolverFactory.kt ==="
cat -n "./vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/PublicKeyResolverFactory.kt"
echo ""
echo "=== Reading CwtVerifier.kt lines 140-160 ==="
sed -n '140,160p' "./vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt"Repository: inji/vc-verifier
Length of output: 1793
🏁 Script executed:
#!/bin/bash
echo "=== Checking usage in LdpVerifier.kt ==="
grep -n -B2 -A2 "PublicKeyResolverFactory" "./vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifier.kt"
echo ""
echo "=== Checking usage in PresentationVerifier.kt ==="
grep -n -B2 -A2 "PublicKeyResolverFactory" "./vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt"
echo ""
echo "=== Checking test file for expected behavior ==="
cat -n "./vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/PublicKeyResolverFactoryTest.kt"Repository: inji/vc-verifier
Length of output: 6190
Add exception handling for unsupported public key types.
PublicKeyResolverFactory().get(issuer) does not return null (return type is non-nullable PublicKey), but it throws PublicKeyTypeNotSupportedException when the issuer URI type is not supported. Add try-catch handling similar to other potential failure points in this method.
Proposed fix
val issuer = extractIssuerUri(claims) ?: return false;
- val publicKey = PublicKeyResolverFactory().get(issuer)
+ val publicKey = try {
+ PublicKeyResolverFactory().get(issuer)
+ } catch (e: PublicKeyTypeNotSupportedException) {
+ return false
+ }
return verifySignature(coseBytes, publicKey)🤖 Prompt for AI Agents
In
@vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/CwtVerifier.kt
around lines 148 - 152, The call to PublicKeyResolverFactory().get(issuer) can
throw PublicKeyTypeNotSupportedException even though it is non-nullable; wrap
the resolver call in a try-catch that catches PublicKeyTypeNotSupportedException
(alongside any existing exception handling pattern used in this class), log or
handle the exception consistently, and return false on failure; specifically
modify the code around extractIssuerUri(claims) and the call to
PublicKeyResolverFactory().get(issuer) in CwtVerifier (referencing
extractIssuerUri, PublicKeyResolverFactory.get, and verifySignature) so
unsupported public key types do not propagate uncaught.
| fun validateNumericDate( | ||
| claims: CBORObject, | ||
| key: CBORObject, | ||
| name: String | ||
| ): Long? { | ||
| if (!claims.ContainsKey(key)) return null | ||
|
|
||
| val value = claims[key] | ||
|
|
||
| require(value.type == CBORType.Integer) { | ||
| "$name must be a CBOR integer (NumericDate)" | ||
| } | ||
|
|
||
| return value.ToObject(Long::class.java) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
RFC 8392 CWT NumericDate section 3.1 specification
💡 Result:
Section 3.1 (Terminology) of RFC 8392 defines NumericDate as:
- It has the same meaning and processing rules as the JWT "NumericDate" from RFC 7519 Section 2.
- In CWT it is represented as a CBOR numeric date (RFC 7049 §2.4.1) instead of a JSON number.
- The CBOR encoding MUST omit the leading tag 1 (epoch-based date/time).
- The registered claims exp, nbf, and iat use this NumericDate type. (rfc-editor.org)
Note: an erratum clarifies that RFC 8392 should require a finite CBOR numeric date (i.e., exclude IEEE infinities), aligning the intent with JSON's numeric limitations. (rfc-editor.org)
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc8392?utm_source=openai
- 2: https://www.rfc-editor.org/errata/eid8617?utm_source=openai
🌐 Web query:
com.upokecenter.cbor library ContainsKey ToObject methods documentation
💡 Result:
ContainsKey (overloads) — determines whether a specified key exists in a CBOR map:
- ContainsKey(CBORObject key): true if the CBOR key is present (null checks CBORObject.Null). [1]
- ContainsKey(Object objKey): converts the object to a CBOR key and returns true if present. [1]
- ContainsKey(String key): converts the string to a CBOR key (null → CBORObject.Null) and returns true if present. [1]
ToObject — converts a CBORObject to a value of a specified type:
- Java signature example: T ToObject(Type t, CBORTypeMapper mapper> — converts this CBOR object to an instance of the given type; mapper controls POD deserialization and custom converters. Use the overloads that take CBORTypeMapper and PODOptions for more control. Note: reflection-based conversions may not be compatible with AOT/self-contained environments. [1][2]
References:
[1] com.upokecenter.cbor.CBORObject API (Java). [2] PeterO.Cbor.CBORObject documentation (.NET).
🏁 Script executed:
#!/bin/bash
# Search for imports and usage of com.upokecenter.cbor in the file
grep -n "import.*cbor\|import.*CBOR" vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
# Look for other usages of validateNumericDate in the codebase
rg "validateNumericDate" -t kt -t java -A 3 -B 3Repository: inji/vc-verifier
Length of output: 183
🌐 Web query:
RFC 7049 CBOR numeric types section 2.4 floating point double integer
💡 Result:
RFC 7049: CBOR supports IEEE 754 half-, single-, and double-precision floating-point encodings (major type 7, additional info 25, 26, 27) and treats different encodings of the same numeric value as equivalent. Floating-point formats and “no-content” simple values are specified in Section 2.3; optional tagging (Section 2.4) covers things like bignums, decimal fractions and bigfloats (tags 2–5). The specification explicitly says an encoder may choose the shortest exact floating-point form and that integer and floating representations of the same numeric value are equivalent (Section 3.6). (rfc-editor.org)
Citations:
🏁 Script executed:
#!/bin/bash
# Search for CBORType enum definition or usage
rg "CBORType\." --max-count 20 | head -40
# Search for validateNumericDate usage in tests or code
rg "validateNumericDate" --max-count 30
# Look for any floating point CBOR handling in the file
rg "Float|Double|Decimal" vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktRepository: inji/vc-verifier
Length of output: 3921
Accept all CBOR numeric types for NumericDate validation, not just integers.
RFC 7049 Section 3.6 specifies that integer and floating-point representations of the same numeric value are equivalent. The current check require(value.type == CBORType.Integer) rejects valid floating-point NumericDate values (e.g., 1609459200.0). Update the validation to accept CBORType.Integer, CBORType.Float, CBORType.Double, etc.
Additionally, RFC 8392 errata EID8617 requires that NumericDate values be finite; add a check to reject IEEE infinities and NaN after conversion to ensure full spec compliance.
| class CwtVerifierTest { | ||
|
|
||
| // @Test | ||
| // fun `should verify valid CWT`() { | ||
| // | ||
| // val kpg = KeyPairGenerator.getInstance("EC") | ||
| // kpg.initialize(256) | ||
| // val keyPair = kpg.generateKeyPair() | ||
| // | ||
| // val publicKey = keyPair.public as ECPublicKey | ||
| // val privateKey = keyPair.private as ECPrivateKey | ||
| // | ||
| // val kid = "test-key-1" | ||
| // | ||
| // | ||
| // val jwk = ECKey.Builder(Curve.P_256, publicKey) | ||
| // .privateKey(privateKey) | ||
| // .keyID(kid) | ||
| // .build() | ||
| // | ||
| // val jwksJson = JWKSet(jwk.toPublicJWK()).toJSONObject().toString() | ||
| // | ||
| // val issuer = "https://issuer.example.com" | ||
| // | ||
| // val issuerMetadataJson = """ | ||
| // { | ||
| // "jwks_uri": "$issuer/jwks" | ||
| // } | ||
| // """.trimIndent() | ||
| // | ||
| // | ||
| // val claims = CBORObject.NewMap().apply { | ||
| // Add(1, issuer) | ||
| // Add(4, Date().time / 1000 + 600) | ||
| // } | ||
| // | ||
| // | ||
| // val sign1 = Sign1COSEObject() | ||
| // | ||
| // sign1.addAttribute( | ||
| // HeaderKeys.Algorithm, | ||
| // CBORObject.FromObject(-7), // ES256 | ||
| // Attribute.PROTECTED | ||
| // ) | ||
| // | ||
| // sign1.addAttribute( | ||
| // HeaderKeys.KID, | ||
| // CBORObject.FromObject(kid.toByteArray()), | ||
| // Attribute.PROTECTED | ||
| // ) | ||
| // | ||
| // sign1.SetContent(claims.EncodeToBytes()) | ||
| // | ||
| // val coseKey = COSEKey(publicKey, privateKey) | ||
| // sign1.sign(coseKey) | ||
| // | ||
| // val coseHex = sign1.EncodeToBytes().joinToString("") { | ||
| // "%02x".format(it) | ||
| // } | ||
| // | ||
| // mockkObject(Util) | ||
| // | ||
| // every { Util.httpGet(any()) } answers { | ||
| // when (firstArg<String>()) { | ||
| // "$issuer/.well-known/openid-credential-issuer" -> issuerMetadataJson | ||
| // "$issuer/jwks" -> jwksJson | ||
| // else -> null | ||
| // } | ||
| // } | ||
| // | ||
| //// val coseHex = "d83dd28443a10126a059015fa701782b6469643a7765623a706979757368373033342e6769746875622e696f3a4449443a7069797573682d6d6163041a6a4a2f79051a695ce179061a695ce1796d44617465204f662042697274686a32352d30352d313939306946756c6c204e616d65684a6f686e20446f656446616365a36b4461746120666f726d617465496d6167656f446174612073756220666f726d6174644a455047644461746178be646174613a696d6167652f706e673b6261736536342c6956424f5277304b47676f414141414e5355684555674141414277414141415143415941414141467a782f764141414151306c455156513479324e674741576a594c41445269435943574c382f2f382f48556c774a724969617371424c5077506c5752456b76795070704671636b7a30446c4957494a364652587757486a32306b4273466f3241514151445370535370396b6851704141414141424a52553545726b4a6767673d3d5840baafa376fee591618a03c10dc33b97e5d87053b52882640e3a317fbfa57b571c8c4c3d1bf2a67f68f7d62b4c547fc49a8a4aad89921de06bf85db518ada6b42f" | ||
| // assertTrue(CwtVerifier().verify(coseHex)) | ||
| // } | ||
|
|
||
| } |
There was a problem hiding this comment.
Enable the commented test or remove dead code.
The entire test class is effectively empty with all test logic commented out. This provides no coverage for CwtVerifier. Either uncomment and fix the test so it runs, or remove the commented code to avoid confusion. Leaving large blocks of commented code is a maintenance burden.
If the test is not ready, consider tracking this with a TODO or creating an issue.
Do you want me to help complete this test or open an issue to track the work?
4 participants
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.