[INJIWEB-1856] - Add Logic for sd-jwt ovp credential matching#1038
Draft
kongadurgesh wants to merge 6 commits intoinji:developfrom
Draft
[INJIWEB-1856] - Add Logic for sd-jwt ovp credential matching#1038kongadurgesh wants to merge 6 commits intoinji:developfrom
kongadurgesh wants to merge 6 commits intoinji:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds SD‑JWT extraction and exposure end‑to‑end: extends CredentialDTO with publicClaims/sdClaims, expands CredentialFormatHandler API, implements extractAllCredentialProperties in LDP and SD‑JWT handlers, and refactors CredentialMatchingServiceImpl to use a CredentialFormatHandlerFactory for format‑specific extraction. Changes
Sequence DiagramsequenceDiagram
participant CMS as CredentialMatchingServiceImpl
participant Factory as CredentialFormatHandlerFactory
participant Handler as CredentialFormatHandler
participant VC as VCCredentialResponse
participant JWT as JwtUtils
CMS->>CMS: getCredentialData(vcResponse)
CMS->>Factory: getCredentialFormatHandler(format)
Factory-->>CMS: Handler (LDP or SD‑JWT)
CMS->>Handler: extractAllCredentialProperties(VC)
alt VC_SD_JWT
Handler->>JWT: parse SD‑JWT (header/payload/disclosures)
JWT-->>Handler: payload + disclosures
Handler->>Handler: extractPublicClaims & extractSdClaims -> Map{"publicClaims","sdClaims"}
else LDP_VC
Handler->>VC: getCredential()
Handler-->>Handler: convert credential -> Map(properties)
end
CMS->>CMS: build CredentialDTO (include publicClaims, sdClaims)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 4
🧹 Nitpick comments (1)
src/main/java/io/mosip/mimoto/dto/CredentialDTO.java (1)
34-40: Default-initialize new claim lists to avoid nullable API fields.
publicClaimsandsdClaimscan be serialized asnullwhen not explicitly set. Initializing defaults keeps response shape stable.♻️ Proposed fix
import lombok.Builder; @@ import java.util.List; +import java.util.ArrayList; @@ - private List<String> publicClaims; + `@Builder.Default` + private List<String> publicClaims = new ArrayList<>(); @@ - private List<String> sdClaims; + `@Builder.Default` + private List<String> sdClaims = new ArrayList<>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/dto/CredentialDTO.java` around lines 34 - 40, The CredentialDTO fields publicClaims and sdClaims can be serialized as null; update CredentialDTO to default-initialize these fields to empty lists (e.g., set publicClaims = new ArrayList<>() and sdClaims = new ArrayList<>() in the field declarations or in the class constructor) so API responses always include empty arrays instead of null; ensure any existing constructors preserve or assign these defaults and that getters return non-null lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 348-356: publicClaims and sdClaims are left empty; populate them
by reading the credential payload (from decryptedCredentialDTO.getCredential())
and using the _sd disclosure list to determine which keys are
selective-disclosure. Specifically, extract the payload/map (e.g., via
decryptedCredentialDTO.getCredential().getPayload() or getClaims()), read the
_sd array/set of disclosed claim keys, build sdClaims as a map of key->value for
every key listed in _sd, and build publicClaims as a map of the remaining keys
(excluding the _sd entry itself); add null checks for payload and _sd, and wire
these populated maps into the CredentialDTO.builder call where publicClaims and
sdClaims are set.
- Around line 211-223: The ldp_vc branch in CredentialMatchingServiceImpl can
NPE when descriptorFormat.get(LDP_VC_FORMAT) returns null or its PROOF_TYPE_KEY
list is null; update the block handling CredentialFormat.LDP_VC to null-guard
the ldpVcFormat Map and the requiredProofTypes List (from
descriptorFormat.get(LDP_VC_FORMAT) and ldpVcFormat.get(PROOF_TYPE_KEY)) and
return false when either is null/empty before calling objectMapper.convertValue
or checking vcProofType; ensure you still extract ldpCredential via
objectMapper.convertValue and compare ldpCredential.getProof().getType() only
after these guards so contains(...) is safe.
- Around line 255-261: The code in
CredentialMatchingServiceImpl.extractAllCredentialProperties currently throws
InvalidRequestException for unsupported formats (using UNSUPPORTED_FORMAT),
which can abort matching; instead, catch the unsupported-format branch: do not
throw InvalidRequestException, log a warning mentioning the credential format
and vc.getFormat(), and return an empty result (e.g., Collections.emptyList() or
equivalent return type) so the credential is treated as non-match; keep the
existing handling for supported formats that uses
credentialFormatHandlerFactory.getHandler(vc.getFormat()) and
credentialFormatHandler.extractAllCredentialProperties(vc).
In
`@src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java`:
- Around line 181-183: The code currently hardcodes sdJwtString (assignment to
the long test token) inside VcSdJwtCredentialFormatHandler, overwriting the real
input; remove that hardcoded assignment and instead initialize sdJwtString from
the actual input (e.g., use the vcCredentialResponse field/parameter or its
SD-JWT property) so the parsing path uses the provided credential data; ensure
no test tokens remain and add a simple null/empty check before parsing.
---
Nitpick comments:
In `@src/main/java/io/mosip/mimoto/dto/CredentialDTO.java`:
- Around line 34-40: The CredentialDTO fields publicClaims and sdClaims can be
serialized as null; update CredentialDTO to default-initialize these fields to
empty lists (e.g., set publicClaims = new ArrayList<>() and sdClaims = new
ArrayList<>() in the field declarations or in the class constructor) so API
responses always include empty arrays instead of null; ensure any existing
constructors preserve or assign these defaults and that getters return non-null
lists.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.talismanrcsrc/main/java/io/mosip/mimoto/dto/CredentialDTO.javasrc/main/java/io/mosip/mimoto/service/CredentialFormatHandler.javasrc/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.javasrc/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.javasrc/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java
Comment on lines
+211
to
+223
| if(CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(vcFormat) && descriptorFormat.containsKey(LDP_VC_FORMAT)) { | ||
| Map<String, List<String>> ldpVcFormat = descriptorFormat.get(LDP_VC_FORMAT); | ||
|
|
||
| VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class); | ||
| String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null; | ||
| List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); | ||
| if (!ldpVcFormat.containsKey(PROOF_TYPE_KEY)) { | ||
| return true; | ||
| } | ||
|
|
||
| return vcProofType != null && requiredProofTypes.contains(vcProofType); | ||
| VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class); | ||
| String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null; | ||
| List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); | ||
|
|
||
| return vcProofType != null && requiredProofTypes.contains(vcProofType); | ||
| } |
There was a problem hiding this comment.
Add null guards for ldp_vc format map and proof_type list.
If descriptorFormat contains ldp_vc with a null payload (or null proof_type list), Lines 214/222 can throw NPE and fail matching.
🛡️ Proposed fix
if(CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(vcFormat) && descriptorFormat.containsKey(LDP_VC_FORMAT)) {
Map<String, List<String>> ldpVcFormat = descriptorFormat.get(LDP_VC_FORMAT);
+ if (ldpVcFormat == null) {
+ return false;
+ }
if (!ldpVcFormat.containsKey(PROOF_TYPE_KEY)) {
return true;
}
VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class);
String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null;
List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY);
+ if (requiredProofTypes == null || requiredProofTypes.isEmpty()) {
+ return true;
+ }
return vcProofType != null && requiredProofTypes.contains(vcProofType);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`
around lines 211 - 223, The ldp_vc branch in CredentialMatchingServiceImpl can
NPE when descriptorFormat.get(LDP_VC_FORMAT) returns null or its PROOF_TYPE_KEY
list is null; update the block handling CredentialFormat.LDP_VC to null-guard
the ldpVcFormat Map and the requiredProofTypes List (from
descriptorFormat.get(LDP_VC_FORMAT) and ldpVcFormat.get(PROOF_TYPE_KEY)) and
return false when either is null/empty before calling objectMapper.convertValue
or checking vcProofType; ensure you still extract ldpCredential via
objectMapper.convertValue and compare ldpCredential.getProof().getType() only
after these guards so contains(...) is safe.
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (3)
382-390:⚠️ Potential issue | 🟡 Minor
publicClaimsandsdClaimsare still placeholders.Both lists are always empty, so the new response fields are not usable yet.
If you want, I can draft a follow-up patch to populate these from extracted payload claims and
_sddisclosures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 382 - 390, CredentialDTO.builder() currently returns empty publicClaims and sdClaims; populate these from the decrypted credential payload by extracting claim entries from decryptedCredentialDTO.getCredential().getPayload() (or equivalent payload accessor) and splitting them based on the selective disclosure list _sd in the payload/disclosures: build publicClaims as the map/list of claims not listed in _sd and sdClaims as the claims listed in _sd (preserving original claim keys/values and any metadata), then pass those populated collections into CredentialDTO.builder().publicClaims(...) and .sdClaims(...), ensuring you handle missing/null _sd (treat as all public) and maintain the same DTO types expected by CredentialDTO.
289-295:⚠️ Potential issue | 🟠 MajorDo not throw for unsupported formats during filtering.
Throwing here can fail the entire matching flow for a single unsupported credential; this path should treat it as non-match.
🧯 Proposed fix
private Object getCredentialData(VCCredentialResponse vc) { String format = vc.getFormat(); - if (CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(format) || CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)) { - CredentialFormatHandler credentialFormatHandler = credentialFormatHandlerFactory.getHandler(vc.getFormat()); - return credentialFormatHandler.extractAllCredentialProperties(vc); - } - else{ - throw new InvalidRequestException(UNSUPPORTED_FORMAT.getErrorCode(), "Unsupported credential format: " + format); - } + try { + CredentialFormatHandler credentialFormatHandler = credentialFormatHandlerFactory.getHandler(format); + if (credentialFormatHandler == null) { + log.debug("Skipping unsupported credential format during matching: {}", format); + return Collections.emptyMap(); + } + return credentialFormatHandler.extractAllCredentialProperties(vc); + } catch (InvalidRequestException ex) { + log.debug("Skipping unsupported credential format during matching: {}", format); + return Collections.emptyMap(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 289 - 295, The current block in CredentialMatchingServiceImpl throws InvalidRequestException for unsupported credential formats, which aborts matching for a single unsupported credential; instead, change the else branch so unsupported formats are treated as non-match by returning an empty result (e.g., Collections.emptyList() or the same empty-type that extractAllCredentialProperties(vc) returns) rather than throwing; locate the conditional using CredentialFormat.LDP_VC, CredentialFormat.VC_SD_JWT, credentialFormatHandlerFactory.getHandler(vc.getFormat()), and vc, and update the else path to return an empty collection/value so the matching flow continues.
214-224:⚠️ Potential issue | 🟠 MajorAdd null guards in the
ldp_vcformat branch.
ldpVcFormatandproof_typecan be null and currently may trigger NPE during matching.🛡️ Proposed fix
if(CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(vcFormat) && descriptorFormat.containsKey(LDP_VC_FORMAT)) { Map<String, List<String>> ldpVcFormat = descriptorFormat.get(LDP_VC_FORMAT); + if (ldpVcFormat == null) { + return false; + } if (!ldpVcFormat.containsKey(PROOF_TYPE_KEY)) { return true; } + List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); + if (requiredProofTypes == null || requiredProofTypes.isEmpty()) { + return true; + } + VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class); String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null; - List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); return vcProofType != null && requiredProofTypes.contains(vcProofType); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 214 - 224, Guard against null ldpVcFormat and null proof list before accessing keys and calling contains: in CredentialMatchingServiceImpl (the ldp_vc branch) first check if descriptorFormat.get(LDP_VC_FORMAT) is null and return true if so; only then call containsKey(PROOF_TYPE_KEY) and return true if the key is absent; after fetching requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY) treat a null requiredProofTypes as no constraint (return true) before constructing VCCredentialProperties and comparing vcProofType via getProof().getType(); keep the final comparison but ensure vcProofType and requiredProofTypes are non-null to avoid NPEs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 229-241: In matchesSdJwtAlgorithm: avoid unsafe casting and
case-sensitive key misses by first verifying vc.getCredential() is a String
(return false if not) before calling extractSdJwtAlgorithm, and locate the
format entry in requestFormat using a case-insensitive key match (iterate
requestFormat.keySet() and find one that equalsIgnoreCase(vc.getFormat()) then
use that key to fetch formatConfig) instead of direct requestFormat.get(format);
also treat algorithmValues as List<String> (null-check) and then call
contains(algorithm) against it; reference: matchesSdJwtAlgorithm,
SD_JWT_ALG_VALUES_KEY, extractSdJwtAlgorithm, vc.getCredential(),
vc.getFormat().
---
Duplicate comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 382-390: CredentialDTO.builder() currently returns empty
publicClaims and sdClaims; populate these from the decrypted credential payload
by extracting claim entries from
decryptedCredentialDTO.getCredential().getPayload() (or equivalent payload
accessor) and splitting them based on the selective disclosure list _sd in the
payload/disclosures: build publicClaims as the map/list of claims not listed in
_sd and sdClaims as the claims listed in _sd (preserving original claim
keys/values and any metadata), then pass those populated collections into
CredentialDTO.builder().publicClaims(...) and .sdClaims(...), ensuring you
handle missing/null _sd (treat as all public) and maintain the same DTO types
expected by CredentialDTO.
- Around line 289-295: The current block in CredentialMatchingServiceImpl throws
InvalidRequestException for unsupported credential formats, which aborts
matching for a single unsupported credential; instead, change the else branch so
unsupported formats are treated as non-match by returning an empty result (e.g.,
Collections.emptyList() or the same empty-type that
extractAllCredentialProperties(vc) returns) rather than throwing; locate the
conditional using CredentialFormat.LDP_VC, CredentialFormat.VC_SD_JWT,
credentialFormatHandlerFactory.getHandler(vc.getFormat()), and vc, and update
the else path to return an empty collection/value so the matching flow
continues.
- Around line 214-224: Guard against null ldpVcFormat and null proof list before
accessing keys and calling contains: in CredentialMatchingServiceImpl (the
ldp_vc branch) first check if descriptorFormat.get(LDP_VC_FORMAT) is null and
return true if so; only then call containsKey(PROOF_TYPE_KEY) and return true if
the key is absent; after fetching requiredProofTypes =
ldpVcFormat.get(PROOF_TYPE_KEY) treat a null requiredProofTypes as no constraint
(return true) before constructing VCCredentialProperties and comparing
vcProofType via getProof().getType(); keep the final comparison but ensure
vcProofType and requiredProofTypes are non-null to avoid NPEs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.talismanrcsrc/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java
Comment on lines
+229
to
+241
| private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) { | ||
| String format = vc.getFormat(); | ||
| String sdJwtString = (String) vc.getCredential(); | ||
| String algorithm = extractSdJwtAlgorithm(sdJwtString); | ||
| if (algorithm == null){ | ||
| return false; | ||
| } | ||
|
|
||
| return vcProofType != null && requiredProofTypes.contains(vcProofType); | ||
| Map<String, List<String>> formatConfig = requestFormat.get(format); | ||
| if (formatConfig != null) { | ||
| List<?> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY); | ||
| if (algorithmValues != null) { | ||
| return algorithmValues.contains(algorithm); |
There was a problem hiding this comment.
Harden SD-JWT algorithm matching against case/key mismatch and bad payload type.
requestFormat.get(vc.getFormat()) is case-sensitive after an equalsIgnoreCase gate, and direct (String) casting of credential can throw.
♻️ Proposed fix
private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) {
- String format = vc.getFormat();
- String sdJwtString = (String) vc.getCredential();
+ if (!(vc.getCredential() instanceof String)) {
+ return false;
+ }
+ String sdJwtString = (String) vc.getCredential();
String algorithm = extractSdJwtAlgorithm(sdJwtString);
if (algorithm == null){
return false;
}
- Map<String, List<String>> formatConfig = requestFormat.get(format);
+ Map<String, List<String>> formatConfig = requestFormat.get(CredentialFormat.VC_SD_JWT.getFormat());
if (formatConfig != null) {
List<?> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY);
if (algorithmValues != null) {
return algorithmValues.contains(algorithm);
}📝 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.
Suggested change
| private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) { | |
| String format = vc.getFormat(); | |
| String sdJwtString = (String) vc.getCredential(); | |
| String algorithm = extractSdJwtAlgorithm(sdJwtString); | |
| if (algorithm == null){ | |
| return false; | |
| } | |
| return vcProofType != null && requiredProofTypes.contains(vcProofType); | |
| Map<String, List<String>> formatConfig = requestFormat.get(format); | |
| if (formatConfig != null) { | |
| List<?> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY); | |
| if (algorithmValues != null) { | |
| return algorithmValues.contains(algorithm); | |
| private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) { | |
| if (!(vc.getCredential() instanceof String)) { | |
| return false; | |
| } | |
| String sdJwtString = (String) vc.getCredential(); | |
| String algorithm = extractSdJwtAlgorithm(sdJwtString); | |
| if (algorithm == null){ | |
| return false; | |
| } | |
| Map<String, List<String>> formatConfig = requestFormat.get(CredentialFormat.VC_SD_JWT.getFormat()); | |
| if (formatConfig != null) { | |
| List<?> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY); | |
| if (algorithmValues != null) { | |
| return algorithmValues.contains(algorithm); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`
around lines 229 - 241, In matchesSdJwtAlgorithm: avoid unsafe casting and
case-sensitive key misses by first verifying vc.getCredential() is a String
(return false if not) before calling extractSdJwtAlgorithm, and locate the
format entry in requestFormat using a case-insensitive key match (iterate
requestFormat.keySet() and find one that equalsIgnoreCase(vc.getFormat()) then
use that key to fetch formatConfig) instead of direct requestFormat.get(format);
also treat algorithmValues as List<String> (null-check) and then call
contains(algorithm) against it; reference: matchesSdJwtAlgorithm,
SD_JWT_ALG_VALUES_KEY, extractSdJwtAlgorithm, vc.getCredential(),
vc.getFormat().
kongadurgesh
force-pushed
the
injiweb-1856
branch
from
0f7cc18 to
75c2d12
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (2)
229-246:⚠️ Potential issue | 🟠 MajorHarden SD-JWT algorithm matching against type mismatch and case sensitivity.
The method has two issues:
- Line 231 casts
vc.getCredential()directly toStringwithout type checking, which can throwClassCastException- Line 237 uses
requestFormat.get(format)whereformatcomes fromvc.getFormat(), but the key inrequestFormatmight have different casing🛡️ Proposed fix
private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) { - String format = vc.getFormat(); - String sdJwtString = (String) vc.getCredential(); + if (!(vc.getCredential() instanceof String)) { + return false; + } + String sdJwtString = (String) vc.getCredential(); String algorithm = extractSdJwtAlgorithm(sdJwtString); if (algorithm == null){ return false; } - Map<String, List<String>> formatConfig = requestFormat.get(format); + Map<String, List<String>> formatConfig = requestFormat.get(CredentialFormat.VC_SD_JWT.getFormat()); if (formatConfig != null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 229 - 246, The matchesSdJwtAlgorithm method should avoid ClassCastException and be case-insensitive: first verify vc.getCredential() is a String (or safely convert/handle non-string values) before calling extractSdJwtAlgorithm and return false if it's not a valid string; normalize the format key (e.g., toLowerCase/trim) before looking it up in requestFormat and perform a case-insensitive lookup (check both the normalized key and original or iterate keys comparing lowercase) so requestFormat.get(format) won’t miss matches; normalize the extracted algorithm and the entries in the retrieved algorithmValues (ensure they are List<String> or filter/convert non-string entries) to the same case before using contains; reference functions/fields: matchesSdJwtAlgorithm, VCCredentialResponse.getCredential(), VCCredentialResponse.getFormat(), extractSdJwtAlgorithm, and SD_JWT_ALG_VALUES_KEY.
213-225:⚠️ Potential issue | 🟠 MajorAdd null guards for
ldp_vcformat map andproof_typelist.The LDP_VC handling path can throw NPE if
descriptorFormat.get(LDP_VC_FORMAT)returns null or if theproof_typelist is null.🛡️ Proposed fix
if(CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(vcFormat) && descriptorFormat.containsKey(LDP_VC_FORMAT)) { Map<String, List<String>> ldpVcFormat = descriptorFormat.get(LDP_VC_FORMAT); + if (ldpVcFormat == null) { + return false; + } if (!ldpVcFormat.containsKey(PROOF_TYPE_KEY)) { return true; } VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class); String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null; List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); + if (requiredProofTypes == null || requiredProofTypes.isEmpty()) { + return true; + } return vcProofType != null && requiredProofTypes.contains(vcProofType); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 213 - 225, Add null guards around the LDP_VC format map and its proof_type list to avoid NPEs: check that descriptorFormat.get(LDP_VC_FORMAT) is not null before assigning to ldpVcFormat and if it is null return the same "no constraint" result as when proof_type is absent; likewise check that ldpVcFormat.get(PROOF_TYPE_KEY) (assigned to requiredProofTypes) is not null before using contains/iteration and treat a null requiredProofTypes as "no constraint" (return true) rather than dereferencing it. Update the block in CredentialMatchingServiceImpl that references descriptorFormat, ldpVcFormat, PROOF_TYPE_KEY, requiredProofTypes and vcProofType to perform these null checks and return consistently when constraints are missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 286-302: The unchecked cast in getCredentialData when handling
CredentialFormat.VC_SD_JWT is unsafe; update the VC_SD_JWT branch to validate
the type returned by credentialFormatHandler.extractAllCredentialProperties(vc)
before casting: call extractAllCredentialProperties, check that the result is an
instance of Map (and that each value is a Map<String,Object> or otherwise
convert/validate entries), throw an InvalidRequestException with a clear message
if the type is unexpected, and only then flatten into flattenedPropertiesMap;
reference getCredentialData,
credentialFormatHandler.extractAllCredentialProperties, and
CredentialFormat.VC_SD_JWT to locate the change.
---
Duplicate comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 229-246: The matchesSdJwtAlgorithm method should avoid
ClassCastException and be case-insensitive: first verify vc.getCredential() is a
String (or safely convert/handle non-string values) before calling
extractSdJwtAlgorithm and return false if it's not a valid string; normalize the
format key (e.g., toLowerCase/trim) before looking it up in requestFormat and
perform a case-insensitive lookup (check both the normalized key and original or
iterate keys comparing lowercase) so requestFormat.get(format) won’t miss
matches; normalize the extracted algorithm and the entries in the retrieved
algorithmValues (ensure they are List<String> or filter/convert non-string
entries) to the same case before using contains; reference functions/fields:
matchesSdJwtAlgorithm, VCCredentialResponse.getCredential(),
VCCredentialResponse.getFormat(), extractSdJwtAlgorithm, and
SD_JWT_ALG_VALUES_KEY.
- Around line 213-225: Add null guards around the LDP_VC format map and its
proof_type list to avoid NPEs: check that descriptorFormat.get(LDP_VC_FORMAT) is
not null before assigning to ldpVcFormat and if it is null return the same "no
constraint" result as when proof_type is absent; likewise check that
ldpVcFormat.get(PROOF_TYPE_KEY) (assigned to requiredProofTypes) is not null
before using contains/iteration and treat a null requiredProofTypes as "no
constraint" (return true) rather than dereferencing it. Update the block in
CredentialMatchingServiceImpl that references descriptorFormat, ldpVcFormat,
PROOF_TYPE_KEY, requiredProofTypes and vcProofType to perform these null checks
and return consistently when constraints are missing.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.talismanrcsrc/main/java/io/mosip/mimoto/dto/CredentialDTO.javasrc/main/java/io/mosip/mimoto/service/CredentialFormatHandler.javasrc/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.javasrc/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.javasrc/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/io/mosip/mimoto/dto/CredentialDTO.java
- src/main/java/io/mosip/mimoto/service/CredentialFormatHandler.java
- src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java
Comment on lines
286
to
302
| private Object getCredentialData(VCCredentialResponse vc) { | ||
| String format = vc.getFormat(); | ||
| CredentialFormatHandler credentialFormatHandler = credentialFormatHandlerFactory.getHandler(vc.getFormat()); | ||
|
|
||
| if (CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(format)) { | ||
| Object credentialData = vc.getCredential(); | ||
|
|
||
| if (credentialData instanceof Map) { | ||
| return credentialData; | ||
| } else { | ||
| return objectMapper.convertValue(credentialData, VCCredentialProperties.class); | ||
| } | ||
| } else if (CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format) || CredentialFormat.DC_SD_JWT.getFormat().equalsIgnoreCase(format)) { | ||
| throw new InvalidRequestException(UNSUPPORTED_FORMAT.getErrorCode(), | ||
| "SD-JWT credential format (" + format + ") is not supported for credential matching"); | ||
| return credentialFormatHandler.extractAllCredentialProperties(vc); | ||
| } | ||
| else if(CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)){ | ||
| Map<String, Map<String, Object>> allCredentialProperties= (Map<String, Map<String, Object>>) credentialFormatHandler.extractAllCredentialProperties(vc); | ||
| Map<String, Object> flattenedPropertiesMap = new HashMap<>(); | ||
| allCredentialProperties.values().forEach(flattenedPropertiesMap::putAll); | ||
| return flattenedPropertiesMap; | ||
| } | ||
| else{ | ||
| throw new InvalidRequestException(UNSUPPORTED_FORMAT.getErrorCode(), "Unsupported credential format: " + format); | ||
| } | ||
|
|
||
| return vc.getCredential(); | ||
| } |
There was a problem hiding this comment.
Add type safety for the credential properties extraction.
Line 294 performs an unchecked cast to Map<String, Map<String, Object>> which could fail at runtime if the handler returns a different type.
🛡️ Proposed fix with type validation
else if(CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)){
- Map<String, Map<String, Object>> allCredentialProperties= (Map<String, Map<String, Object>>) credentialFormatHandler.extractAllCredentialProperties(vc);
+ Object extracted = credentialFormatHandler.extractAllCredentialProperties(vc);
+ if (!(extracted instanceof Map)) {
+ return Collections.emptyMap();
+ }
+ `@SuppressWarnings`("unchecked")
+ Map<String, Map<String, Object>> allCredentialProperties = (Map<String, Map<String, Object>>) extracted;
Map<String, Object> flattenedPropertiesMap = new HashMap<>();
allCredentialProperties.values().forEach(flattenedPropertiesMap::putAll);
return flattenedPropertiesMap;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`
around lines 286 - 302, The unchecked cast in getCredentialData when handling
CredentialFormat.VC_SD_JWT is unsafe; update the VC_SD_JWT branch to validate
the type returned by credentialFormatHandler.extractAllCredentialProperties(vc)
before casting: call extractAllCredentialProperties, check that the result is an
instance of Map (and that each value is a Map<String,Object> or otherwise
convert/validate entries), throw an InvalidRequestException with a clear message
if the type is unexpected, and only then flatten into flattenedPropertiesMap;
reference getCredentialData,
credentialFormatHandler.extractAllCredentialProperties, and
CredentialFormat.VC_SD_JWT to locate the change.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (3)
286-301:⚠️ Potential issue | 🟠 MajorAvoid aborting matching on unsupported/invalid extracted format data.
This method runs inside constraint evaluation; throwing here can fail the whole match flow for one credential. Also, Line 294’s unchecked cast is a runtime risk.
🛠️ Proposed fix
private Object getCredentialData(VCCredentialResponse vc) { String format = vc.getFormat(); - CredentialFormatHandler credentialFormatHandler = credentialFormatHandlerFactory.getHandler(vc.getFormat()); + CredentialFormatHandler credentialFormatHandler; + try { + credentialFormatHandler = credentialFormatHandlerFactory.getHandler(format); + } catch (InvalidRequestException ex) { + log.debug("Skipping unsupported credential format during matching: {}", format); + return Collections.emptyMap(); + } if (CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(format)) { return credentialFormatHandler.extractAllCredentialProperties(vc); } else if(CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)){ - Map<String, Map<String, Object>> allCredentialProperties= (Map<String, Map<String, Object>>) credentialFormatHandler.extractAllCredentialProperties(vc); + Object extracted = credentialFormatHandler.extractAllCredentialProperties(vc); + if (!(extracted instanceof Map)) { + return Collections.emptyMap(); + } + Map<?, ?> allCredentialProperties = (Map<?, ?>) extracted; Map<String, Object> flattenedPropertiesMap = new HashMap<>(); - allCredentialProperties.values().forEach(flattenedPropertiesMap::putAll); + allCredentialProperties.values().stream() + .filter(Map.class::isInstance) + .map(Map.class::cast) + .forEach(map -> map.forEach((k, v) -> { + if (k instanceof String) { + flattenedPropertiesMap.put((String) k, v); + } + })); return flattenedPropertiesMap; } else{ - throw new InvalidRequestException(UNSUPPORTED_FORMAT.getErrorCode(), "Unsupported credential format: " + format); + return Collections.emptyMap(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 286 - 301, In getCredentialData(VCCredentialResponse vc) avoid throwing an InvalidRequestException (which aborts the whole matching flow) and remove the unchecked cast: check vc.getFormat() and the result of credentialFormatHandler.extractAllCredentialProperties(vc) safely (use instanceof to confirm it's a Map<String, Map<String,Object>> before casting), if the shape is unexpected log a warning and return an empty Map (or null) so constraint evaluation can continue; for VC_SD_JWT, merge values only after safe type checks and handle non-map results gracefully instead of throwing in getCredentialData.
213-224:⚠️ Potential issue | 🟠 MajorGuard
ldp_vcconfig nulls before dereference.Line 214 can return null, and Line 224 can NPE when
proof_typeexists but maps to null.🛠️ Proposed fix
if(CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(vcFormat) && descriptorFormat.containsKey(LDP_VC_FORMAT)) { Map<String, List<String>> ldpVcFormat = descriptorFormat.get(LDP_VC_FORMAT); + if (ldpVcFormat == null) { + return false; + } if (!ldpVcFormat.containsKey(PROOF_TYPE_KEY)) { return true; } + List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); + if (requiredProofTypes == null || requiredProofTypes.isEmpty()) { + return true; + } + VCCredentialProperties ldpCredential = objectMapper.convertValue(vc.getCredential(), VCCredentialProperties.class); String vcProofType = ldpCredential.getProof() != null ? ldpCredential.getProof().getType() : null; - List<String> requiredProofTypes = ldpVcFormat.get(PROOF_TYPE_KEY); return vcProofType != null && requiredProofTypes.contains(vcProofType); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 213 - 224, The code dereferences descriptorFormat.get(LDP_VC_FORMAT), ldpVcFormat.get(PROOF_TYPE_KEY) and vc.getCredential() without null checks which can cause NPEs; update the CredentialFormat.LDP_VC branch in CredentialMatchingServiceImpl to first null-check descriptorFormat and descriptorFormat.get(LDP_VC_FORMAT) and return false if missing, ensure ldpVcFormat.containsKey(PROOF_TYPE_KEY) also checks ldpVcFormat.get(PROOF_TYPE_KEY) != null before using it, and guard vc.getCredential() and the result of objectMapper.convertValue(...) (VCCredentialProperties) for null before calling getProof()/getType(), returning false when required values are absent so no null dereference occurs.
229-241:⚠️ Potential issue | 🟠 MajorMake SD-JWT algorithm matching type-safe and key-stable.
Line 231 can throw
ClassCastException, and Line 237 may miss config due to case-sensitive map lookup.🛠️ Proposed fix
private boolean matchesSdJwtAlgorithm(VCCredentialResponse vc, Map<String, Map<String, List<String>>> requestFormat) { - String format = vc.getFormat(); - String sdJwtString = (String) vc.getCredential(); + if (!(vc.getCredential() instanceof String)) { + return false; + } + String sdJwtString = (String) vc.getCredential(); String algorithm = extractSdJwtAlgorithm(sdJwtString); if (algorithm == null){ return false; } - Map<String, List<String>> formatConfig = requestFormat.get(format); + Map<String, List<String>> formatConfig = requestFormat.get(CredentialFormat.VC_SD_JWT.getFormat()); if (formatConfig != null) { - List<?> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY); + List<String> algorithmValues = formatConfig.get(SD_JWT_ALG_VALUES_KEY); if (algorithmValues != null) { return algorithmValues.contains(algorithm); } return true; // If no specific algorithms are required, any algorithm is acceptable }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 229 - 241, The matchesSdJwtAlgorithm method can throw ClassCastException and miss the config due to case-sensitive map keys; update matchesSdJwtAlgorithm to (1) safely get the credential by checking vc.getCredential() instanceof String and return false if not a String before calling extractSdJwtAlgorithm, (2) retrieve the SD_JWT_ALG_VALUES_KEY from formatConfig in a case-insensitive way (e.g., search formatConfig.entrySet() for a key.equalsIgnoreCase(SD_JWT_ALG_VALUES_KEY) and use its value), and (3) treat the retrieved algorithmValues as a typed List<?> and only consider it a match if it is a Collection (or List) whose elements are Strings (or convert elements to String) before calling contains(algorithm); reference methods/vars: matchesSdJwtAlgorithm, VCCredentialResponse.getCredential, extractSdJwtAlgorithm, requestFormat, SD_JWT_ALG_VALUES_KEY.
🧹 Nitpick comments (1)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (1)
401-402: Hoist SD-JWT metadata keys to a constant set.This list is rebuilt per credential and removes keys one-by-one. A static
Set+removeAllis cleaner and cheaper.♻️ Proposed refactor
+private static final Set<String> SD_JWT_METADATA_KEYS = Set.of( + "vct", "cnf", "iss", "sub", "aud", "exp", "nbf", "iat", "jti", "_sd", "_sd_alg", "id" +); ... -List<String> metadataKeys = Arrays.asList("vct", "cnf", "iss", "sub", "aud", "exp", "nbf", "iat", "jti", "_sd", "_sd_alg", "id"); -metadataKeys.forEach(publicClaims::remove); +publicClaims.removeAll(SD_JWT_METADATA_KEYS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 401 - 402, Hoist the SD-JWT metadata keys into a single static final Set (e.g., SD_JWT_METADATA_KEYS) in CredentialMatchingServiceImpl and initialize it once (unmodifiable/immutable), then replace the per-credential List creation and metadataKeys.forEach(publicClaims::remove) with publicClaims.removeAll(SD_JWT_METADATA_KEYS) to remove all keys in one call; ensure the constant contains "vct","cnf","iss","sub","aud","exp","nbf","iat","jti","_sd","_sd_alg","id".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 286-301: In getCredentialData(VCCredentialResponse vc) avoid
throwing an InvalidRequestException (which aborts the whole matching flow) and
remove the unchecked cast: check vc.getFormat() and the result of
credentialFormatHandler.extractAllCredentialProperties(vc) safely (use
instanceof to confirm it's a Map<String, Map<String,Object>> before casting), if
the shape is unexpected log a warning and return an empty Map (or null) so
constraint evaluation can continue; for VC_SD_JWT, merge values only after safe
type checks and handle non-map results gracefully instead of throwing in
getCredentialData.
- Around line 213-224: The code dereferences
descriptorFormat.get(LDP_VC_FORMAT), ldpVcFormat.get(PROOF_TYPE_KEY) and
vc.getCredential() without null checks which can cause NPEs; update the
CredentialFormat.LDP_VC branch in CredentialMatchingServiceImpl to first
null-check descriptorFormat and descriptorFormat.get(LDP_VC_FORMAT) and return
false if missing, ensure ldpVcFormat.containsKey(PROOF_TYPE_KEY) also checks
ldpVcFormat.get(PROOF_TYPE_KEY) != null before using it, and guard
vc.getCredential() and the result of objectMapper.convertValue(...)
(VCCredentialProperties) for null before calling getProof()/getType(), returning
false when required values are absent so no null dereference occurs.
- Around line 229-241: The matchesSdJwtAlgorithm method can throw
ClassCastException and miss the config due to case-sensitive map keys; update
matchesSdJwtAlgorithm to (1) safely get the credential by checking
vc.getCredential() instanceof String and return false if not a String before
calling extractSdJwtAlgorithm, (2) retrieve the SD_JWT_ALG_VALUES_KEY from
formatConfig in a case-insensitive way (e.g., search formatConfig.entrySet() for
a key.equalsIgnoreCase(SD_JWT_ALG_VALUES_KEY) and use its value), and (3) treat
the retrieved algorithmValues as a typed List<?> and only consider it a match if
it is a Collection (or List) whose elements are Strings (or convert elements to
String) before calling contains(algorithm); reference methods/vars:
matchesSdJwtAlgorithm, VCCredentialResponse.getCredential,
extractSdJwtAlgorithm, requestFormat, SD_JWT_ALG_VALUES_KEY.
---
Nitpick comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 401-402: Hoist the SD-JWT metadata keys into a single static final
Set (e.g., SD_JWT_METADATA_KEYS) in CredentialMatchingServiceImpl and initialize
it once (unmodifiable/immutable), then replace the per-credential List creation
and metadataKeys.forEach(publicClaims::remove) with
publicClaims.removeAll(SD_JWT_METADATA_KEYS) to remove all keys in one call;
ensure the constant contains
"vct","cnf","iss","sub","aud","exp","nbf","iat","jti","_sd","_sd_alg","id".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.talismanrcsrc/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .talismanrc
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (2)
297-314:⚠️ Potential issue | 🟠 MajorFail-soft unsupported formats and avoid unchecked nested-map casts.
Line [297] can throw during handler resolution for unsupported formats, and Line [307]/Line [406] use unchecked casts that may raise
ClassCastException. In matching flows, this should degrade to non-match, not abort the full evaluation.♻️ Proposed fix
private Object getCredentialData(VCCredentialResponse vc) { String format = vc.getFormat(); - CredentialFormatHandler credentialFormatHandler = credentialFormatHandlerFactory.getHandler(vc.getFormat()); + CredentialFormatHandler credentialFormatHandler; + try { + credentialFormatHandler = credentialFormatHandlerFactory.getHandler(format); + } catch (InvalidRequestException ex) { + log.debug("Skipping unsupported credential format during matching: {}", format); + return Collections.emptyMap(); + } if (CredentialFormat.LDP_VC.getFormat().equalsIgnoreCase(format)) { return credentialFormatHandler.extractAllCredentialProperties(vc); } - else if(CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)){ - Object extracted = credentialFormatHandler.extractAllCredentialProperties(vc); - if (extracted == null) { + else if (CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(format)) { + Map<String, Map<String, Object>> extracted = credentialFormatHandler.extractAllCredentialProperties(vc); + if (extracted == null || extracted.isEmpty()) { return Collections.emptyMap(); } - Map<String, Map<String, Object>> allCredentialProperties = (Map<String, Map<String, Object>>) extracted; - Map<String, Object> flattenedPropertiesMap = new HashMap<>(); - allCredentialProperties.values().forEach(flattenedPropertiesMap::putAll); + Map<String, Object> flattenedPropertiesMap = new HashMap<>(); + extracted.values().stream() + .filter(Objects::nonNull) + .forEach(flattenedPropertiesMap::putAll); return flattenedPropertiesMap; } else{ - throw new InvalidRequestException(UNSUPPORTED_FORMAT.getErrorCode(), "Unsupported credential format: " + format); + return Collections.emptyMap(); } }#!/bin/bash set -euo pipefail # Verify all contract signatures and cast-heavy call sites. rg -n "extractAllCredentialProperties\\(" src/main/java -C2 rg -n "\\(Map<String, Map<String, Object>>\\)" src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java -C2 rg -n "getHandler\\(" src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java -C2Also applies to: 404-407
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 297 - 314, The code currently throws or performs unchecked casts when the handler is missing or extractAllCredentialProperties returns unexpected types; update the logic in CredentialMatchingServiceImpl to fail-soft: when credentialFormatHandlerFactory.getHandler(vc.getFormat()) returns null or the handler cannot handle the format, return Collections.emptyMap() instead of throwing InvalidRequestException; for the VC_SD_JWT branch, avoid the unchecked cast of extracted to Map<String, Map<String, Object>> by validating with instanceof (or checking Map structure) and safely iterating keys/values to build flattenedPropertiesMap, returning empty map if the shape is invalid or extracted is null; ensure you reference credentialFormatHandlerFactory, CredentialFormat.LDP_VC, CredentialFormat.VC_SD_JWT, credentialFormatHandler.extractAllCredentialProperties(vc), and InvalidRequestException while making these defensive checks.
216-218:⚠️ Potential issue | 🟠 MajorFail closed when
ldp_vcformat block is null.Line [217] returns
truewhendescriptorFormatcontainsldp_vcbut its value is null. That treats malformed format constraints as a match.🛡️ Proposed fix
- if(ldpVcFormat == null) { - return true; - } + if (ldpVcFormat == null) { + return false; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 216 - 218, In CredentialMatchingServiceImpl (the method that inspects descriptorFormat and the ldp_vc block, identified by the ldpVcFormat variable), change the null handling so that if ldpVcFormat == null the method returns false (fail closed) instead of true; update the conditional that currently reads "if (ldpVcFormat == null) { return true; }" to return false and add/adjust unit tests asserting malformed/null ldp_vc blocks do not match.
🧹 Nitpick comments (1)
src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (1)
435-454: If these are JSONPath values, add$.prefix at generation time.
extractJsonPaths/collectPathscurrently producea.b[0], not JSONPath ($.a.b[0]). That can confuse downstream consumers expecting actual JsonPath syntax.♻️ Proposed fix
private List<String> extractJsonPaths(Map<String, Object> sdClaimsMap) { List<String> paths = new ArrayList<>(); for (Map.Entry<String, Object> entry : sdClaimsMap.entrySet()) { - collectPaths(entry.getKey(), entry.getValue(), paths); + collectPaths(JSON_PATH_PREFIX + entry.getKey(), entry.getValue(), paths); } return paths; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java` around lines 435 - 454, The extracted paths from extractJsonPaths/collectPaths are missing the JSONPath root prefix; update collectPaths (and the initial call in extractJsonPaths) so generated paths are prefixed with "$." (e.g., produce "$.a.b[0]" instead of "a.b[0]"). Ensure when collectPaths is first invoked for top-level entries you prepend "$." (or have collectPaths add it when currentPath is a top-level key), and preserve this prefix for nested Map and List concatenations so all returned strings from extractJsonPaths are valid JSONPath expressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java`:
- Around line 159-171: extractAllPropertiesFromSdJwt currently puts raw JWT
payload into "publicClaims" causing metadata keys to leak; update the logic so
publicClaims contains the same filtered shape as extractClaimsFromSdJwt (i.e.,
remove metadata keys like iss, iat, _sd, etc.). Either modify
extractPublicClaims to call or reuse extractClaimsFromSdJwt's filtering routine,
or pass the parsed payload through extractClaimsFromSdJwt before putting it into
the claims map; ensure the symbols extractAllPropertiesFromSdJwt,
extractPublicClaims, and extractClaimsFromSdJwt are used so metadata is
consistently removed.
---
Duplicate comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 297-314: The code currently throws or performs unchecked casts
when the handler is missing or extractAllCredentialProperties returns unexpected
types; update the logic in CredentialMatchingServiceImpl to fail-soft: when
credentialFormatHandlerFactory.getHandler(vc.getFormat()) returns null or the
handler cannot handle the format, return Collections.emptyMap() instead of
throwing InvalidRequestException; for the VC_SD_JWT branch, avoid the unchecked
cast of extracted to Map<String, Map<String, Object>> by validating with
instanceof (or checking Map structure) and safely iterating keys/values to build
flattenedPropertiesMap, returning empty map if the shape is invalid or extracted
is null; ensure you reference credentialFormatHandlerFactory,
CredentialFormat.LDP_VC, CredentialFormat.VC_SD_JWT,
credentialFormatHandler.extractAllCredentialProperties(vc), and
InvalidRequestException while making these defensive checks.
- Around line 216-218: In CredentialMatchingServiceImpl (the method that
inspects descriptorFormat and the ldp_vc block, identified by the ldpVcFormat
variable), change the null handling so that if ldpVcFormat == null the method
returns false (fail closed) instead of true; update the conditional that
currently reads "if (ldpVcFormat == null) { return true; }" to return false and
add/adjust unit tests asserting malformed/null ldp_vc blocks do not match.
---
Nitpick comments:
In
`@src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java`:
- Around line 435-454: The extracted paths from extractJsonPaths/collectPaths
are missing the JSONPath root prefix; update collectPaths (and the initial call
in extractJsonPaths) so generated paths are prefixed with "$." (e.g., produce
"$.a.b[0]" instead of "a.b[0]"). Ensure when collectPaths is first invoked for
top-level entries you prepend "$." (or have collectPaths add it when currentPath
is a top-level key), and preserve this prefix for nested Map and List
concatenations so all returned strings from extractJsonPaths are valid JSONPath
expressions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.talismanrcsrc/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.javasrc/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java
src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java
Show resolved
Hide resolved
Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
…i response Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
Signed-off-by: Durgesh Konga <kongadurgesh20@gmail.com>
kongadurgesh
force-pushed
the
injiweb-1856
branch
from
d154186 to
c6f0d69
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores