injiweb-1671 made credSub opt for ldp_vc

[cyber-titan]

Summary by CodeRabbit

• Bug Fixes / Improvements

  • Relaxed validation on some credential display fields to allow empty or missing values.
  • Improved and centralized fallback logic for building display properties: preserves ordering, skips nulls, excludes identifier fields, and provides sensible default labels/locales.

• New Features

  • VP submission flow: supports an explicit redirect URI, prefers verifier-provided redirect, and sends the presentation as form-encoded with encoded token.

• Tests

  • Expanded unit tests for fallback ordering, null-value skipping, id exclusion, empty-claims fallback and looser error-message assertions.

[coderabbitai]

Walkthrough

Removed strict validation annotations from two DTO fields and added centralized fallback builders in two credential format handlers to construct locale-aware display properties when issuer well-known data is missing or incomplete; tests were expanded to cover fallback behaviors, ordering, and exclusion rules. Also adjusted PresentationService handling for redirectUri and changed several build/dependency entries in pom.xml.

Changes

Cohort / File(s)	Summary
Validation Constraint Removals src/main/java/io/mosip/mimoto/dto/mimoto/CredentialDefinitionResponseDto.java, src/main/java/io/mosip/mimoto/dto/mimoto/CredentialIssuerDisplayResponse.java	Removed @NotEmpty from credentialSubject map and removed @NotBlank from name and locale fields, relaxing non-empty/non-blank validation while keeping other annotations.
LDP VC Display Fallback src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java	Capture issuer order early, replace some early returns/warnings with info logs, centralize fallback construction in new private buildFallbackDisplayProperties(Map<String,Object>, List<String>), preserve ordering, skip "id" and nulls, and use title-cased labels/locales when well-known display is missing.
SD-JWT Display Fallback src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java	Build orderedKeys early (merge with credentialProperties), switch internal claims map to LinkedHashMap to preserve order, add id to removed metadata set, add buildFallbackDisplayProperties(Map<String,Object>, Set<String>) and delegate to it when claims/display config absent; some log level adjustments.
Presentation POST / Redirect handling src/main/java/io/mosip/mimoto/service/impl/PresentationServiceImpl.java	postVpToResponseUri signature changed to accept redirectUri; payload switched to form-encoded with Base64-URL-encoded vp_token; response redirect selection now prefers verifier redirect_uri, then request redirectUri, then fallback.
Miscellaneous service change src/main/java/io/mosip/mimoto/service/impl/DataShareServiceImpl.java	Removed an informational log line after mapping the credential; no behavioral change otherwise.
DTO JSON property rename src/main/java/io/mosip/mimoto/dto/openid/presentation/PresentationDefinitionDTO.java	Changed JSON mapping for inputDescriptors field from inputDescriptors → input_descriptors.
Pom / Dependencies pom.xml	Bumped spring.boot.version property (3.2.3 → 3.3.10); removed several dependencies (ld-signatures-java, jsonld-common-java, h2, logback, servlet-api, jaxb-runtime, junit-vintage) and added exclusions to kernel modules to prevent junit-vintage/junit leakage.
Unit tests added/updated src/test/java/io/mosip/mimoto/service/*, src/test/java/io/mosip/mimoto/util/*	Added tests for fallback behavior, ordering, null-value skipping, and id exclusion in LDP and SD-JWT handlers; loosened an exact error-message assertion to substring checks; added helper to build PresentationRequestDTO with empty redirect URI; adjusted presentation tests to new redirect behavior.

Sequence Diagram

sequenceDiagram
    participant Caller as loadDisplayPropertiesFromWellknown()
    participant Issuer as Issuer well-known
    participant Builder as buildFallbackDisplayProperties()
    participant Mapper as Display mapper

    Caller->>Issuer: fetch credentialDefinition / claims / order
    alt Display config present
        Caller->>Mapper: convert & map display properties (locale-aware)
        Mapper-->>Caller: mapped display properties
    else Missing or incomplete display config
        Caller->>Builder: build fallback (credentialProperties, orderedKeys)
        Builder->>Builder: determine ordering (issuer order preferred, else property keys)
        Builder->>Builder: for each key: skip "id"/nulls, use nested display if present, else title-case fallback with locale "en"
        Builder-->>Caller: fallback display mappings
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Areas to focus:
  • LdpVcCredentialFormatHandler.java — fallback algorithm, ordering, null/"id" exclusion, locale/label generation.
  • VcSdJwtCredentialFormatHandler.java — orderedKeys merging, LinkedHashMap usage, parity with LDP handler.
  • PresentationServiceImpl.java — signature change, form-encoding, Base64-URL encoding, redirect resolution.
  • Tests — new/updated tests verifying fallback semantics and presentation redirect behavior.
  • pom.xml — dependency removals and exclusions that could affect build/test classpath.

Possibly related PRs

• [INJIWEB-1746] - Fix direct_post response mode: use form-encoded data and update redirect URI handling #962 — touches PresentationService direct-post handling and PresentationDefinitionDTO mapping; highly related to redirect/payload changes.
• Injiweb 1757 #967 — modifies pom.xml dependency removals/exclusions similar to this PR.
• [INJIWEB-1758] - Upgrade spring boot version for security vulnerabililties #961 — updates spring.boot.version property (related version bump).

Suggested reviewers

• jackjain
• mohanachandran-s

Poem

🐰 I nibbled at annotations, light and spry,
When well-known whispers nothing, I still try,
I order keys, skip the id with grace,
Add locales, labels — tidy each place,
Hopping home while tests clap a happy eye.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title refers only to making credentialSubject optional for ldp_vc, but the PR includes substantial changes across 11+ files including credential format handlers, DTOs, validation logic, dependency updates, and API contract changes.	Update the title to accurately reflect the broader scope, such as 'Relax credential validation and add fallback display properties support' or provide a more comprehensive summary of the primary changes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a8a8ae and eda7239.

📒 Files selected for processing (3)

• src/main/java/io/mosip/mimoto/dto/mimoto/CredentialDefinitionResponseDto.java (0 hunks)
• src/main/java/io/mosip/mimoto/dto/mimoto/CredentialIssuerDisplayResponse.java (0 hunks)
• src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (2 hunks)

💤 Files with no reviewable changes (2)

• src/main/java/io/mosip/mimoto/dto/mimoto/CredentialDefinitionResponseDto.java
• src/main/java/io/mosip/mimoto/dto/mimoto/CredentialIssuerDisplayResponse.java

🧰 Additional context used

🧬 Code graph analysis (1)

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (1)

src/main/java/io/mosip/mimoto/util/LocaleUtils.java (1)

• LocaleUtils (13-78)

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (1)

124-181: Consider extracting to a shared utility to eliminate duplication.

This method duplicates the implementation in VcSdJwtCredentialFormatHandler.java (lines 173-228), with a parameter type inconsistency: this uses List<String> orderedKeys while the SD-JWT handler uses Set<String>. Extracting to a shared utility would eliminate duplication and ensure consistent behavior across format handlers.

🧹 Nitpick comments (1)

src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (1)

173-228: Consider extracting to a shared utility to eliminate duplication.

The buildFallbackDisplayProperties method is nearly identical to the one in LdpVcCredentialFormatHandler.java (lines 124-181), with only a minor parameter type difference (Set<String> vs List<String> for orderedKeys). This duplication makes maintenance harder and increases the risk of inconsistent behavior.

Consider:

1. Extracting this method to a shared utility class (e.g., DisplayPropertiesUtils)
2. Standardizing on a single parameter type (probably List<String> since order matters)
3. Having both format handlers delegate to the shared implementation

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eda7239 and af375dd.

📒 Files selected for processing (5)

• src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (2 hunks)
• src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (3 hunks)
• src/test/java/io/mosip/mimoto/service/LdpVcCredentialFormatHandlerTest.java (4 hunks)
• src/test/java/io/mosip/mimoto/service/VcSdJwtCredentialFormatHandlerTest.java (1 hunks)
• src/test/java/io/mosip/mimoto/util/CredentialIssuerWellknownResponseValidatorTest.java (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (4)

src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (1)

src/main/java/io/mosip/mimoto/util/LocaleUtils.java (1)

• LocaleUtils (13-78)

src/test/java/io/mosip/mimoto/service/LdpVcCredentialFormatHandlerTest.java (1)

src/main/java/io/mosip/mimoto/util/LocaleUtils.java (1)

• LocaleUtils (13-78)

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (1)

src/main/java/io/mosip/mimoto/util/LocaleUtils.java (1)

• LocaleUtils (13-78)

src/test/java/io/mosip/mimoto/service/VcSdJwtCredentialFormatHandlerTest.java (1)

src/main/java/io/mosip/mimoto/util/LocaleUtils.java (1)

• LocaleUtils (13-78)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build-maven-apitest-mimoto / maven-build
• GitHub Check: build-mimoto / maven-build

🔇 Additional comments (8)

src/test/java/io/mosip/mimoto/util/CredentialIssuerWellknownResponseValidatorTest.java (1)

285-289: LGTM: More flexible assertion approach.

The change from exact message matching to substring assertions makes the test more resilient to message formatting variations while still verifying the essential validation behavior.

src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (2)

68-71: LGTM: Appropriate guard clause for missing claims.

The null/empty check with info-level logging and graceful fallback is a good defensive programming practice.

---

86-88: LGTM: Consistent fallback handling.

The log level change to info and the fallback delegation maintain consistency with the other fallback paths in this method.

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (2)

80-84: LGTM: Critical issue from previous review has been addressed.

The null checks are now performed correctly before accessing the credential subject, resolving the previous critical issue. The fallback delegation is clean and appropriate.

---

89-92: LGTM: Consistent fallback handling.

The null check with info-level logging and graceful fallback maintains consistency with the other fallback paths.

src/test/java/io/mosip/mimoto/service/VcSdJwtCredentialFormatHandlerTest.java (1)

457-688: LGTM: Comprehensive test coverage for fallback behavior.

The new tests thoroughly validate the fallback display property logic, including ordering, nested display structures, locale matching, edge cases with null/empty values, and id field exclusion. The test organization and naming are clear and descriptive.

src/test/java/io/mosip/mimoto/service/LdpVcCredentialFormatHandlerTest.java (2)

143-150: LGTM: Test expectations updated to match new fallback behavior.

The updated assertions correctly reflect the new fallback behavior where missing configuration results in generated display properties rather than empty results. The expectations are consistent and appropriate.

Also applies to: 167-174, 191-198

---

504-708: LGTM: Comprehensive test coverage for fallback behavior.

The new tests provide thorough validation of the fallback display property logic for the LDP VC format, covering ordering, nested display structures, locale matching, and edge cases. The parallel structure with VcSdJwtCredentialFormatHandlerTest ensures consistent validation across format handlers.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af375dd and 7fa7b00.

📒 Files selected for processing (4)

• src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (2 hunks)
• src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (4 hunks)
• src/test/java/io/mosip/mimoto/service/LdpVcCredentialFormatHandlerTest.java (4 hunks)
• src/test/java/io/mosip/mimoto/service/VcSdJwtCredentialFormatHandlerTest.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build-maven-apitest-mimoto / maven-build
• GitHub Check: build-mimoto / maven-build

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (1)

111-112: Remove redundant null check for orderedKeys.

The null check is unnecessary since orderedKeys is initialized on line 77 via Optional.ofNullable(...).orElse(new LinkedHashSet<>()), guaranteeing it's never null (at most empty).

Apply this diff:

-        List<String> fieldKeys = (orderedKeys != null && !orderedKeys.isEmpty())
+        List<String> fieldKeys = (!orderedKeys.isEmpty())
                 ? new ArrayList<>(orderedKeys)
                 : new ArrayList<>(localizedDisplayMap.keySet());

pom.xml (1)

56-56: Consider updating to the latest 3.3.x patch
Spring Boot 3.3.13 (released 19 Jun 2025) is available and contains additional fixes over 3.3.10. Bumping straight to the newest patch avoids missing security and bug fixes on this line.(versionlog.com)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be43bcb and 4419083.

📒 Files selected for processing (7)

• pom.xml (4 hunks)
• src/main/java/io/mosip/mimoto/dto/openid/presentation/PresentationDefinitionDTO.java (1 hunks)
• src/main/java/io/mosip/mimoto/service/impl/DataShareServiceImpl.java (0 hunks)
• src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (3 hunks)
• src/main/java/io/mosip/mimoto/service/impl/PresentationServiceImpl.java (4 hunks)
• src/test/java/io/mosip/mimoto/service/PresentationServiceTest.java (2 hunks)
• src/test/java/io/mosip/mimoto/util/TestUtilities.java (1 hunks)

💤 Files with no reviewable changes (1)

• src/main/java/io/mosip/mimoto/service/impl/DataShareServiceImpl.java

🧰 Additional context used

🧬 Code graph analysis (1)

src/test/java/io/mosip/mimoto/service/PresentationServiceTest.java (1)

src/test/java/io/mosip/mimoto/util/TestUtilities.java (1)

• TestUtilities (44-564)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build-maven-apitest-mimoto / maven-build
• GitHub Check: build-mimoto / maven-build

🔇 Additional comments (6)

src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (3)

77-84: LGTM! Proper ordering initialization.

The logic correctly initializes orderedKeys from the issuer-provided order (if available) and supplements it with any remaining keys from the actual credential properties, preserving insertion order via LinkedHashSet.

---

87-91: LGTM! Null checks and fallback properly implemented.

The conditional correctly guards against null credentialDefinition or credentialSubject, logging appropriately before delegating to the centralized fallback method. This addresses the critical issue mentioned in previous reviews.

---

126-155: LGTM! Well-structured fallback method.

The buildFallbackDisplayProperties helper correctly centralizes fallback logic, respects issuer-provided ordering when available, excludes metadata fields like "id", and generates locale-aware display entries using camelToTitleCase. The defensive null check for the orderedKeys parameter is appropriate since it's a method input.

src/main/java/io/mosip/mimoto/dto/openid/presentation/PresentationDefinitionDTO.java (1)

18-19: Annotation change looks good
The switch to @JsonProperty("input_descriptors") aligns the DTO with the expected wire format. Nicely done.

src/test/java/io/mosip/mimoto/util/TestUtilities.java (1)

296-303: Helper addition looks good
The dedicated builder for an empty redirectUri keeps the tests tidy.

src/test/java/io/mosip/mimoto/service/PresentationServiceTest.java (1)

483-486: Add coverage for the posted vp_token content
Once the service posts the raw VP token again, please extend this test (or a sibling) to assert that restApiClient.postApi receives the unwrapped JSON/JWT rather than a Base64 surrogate—so the regression won’t slip back in.(ewc-consortium.github.io)

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Do not Base64-encode vp_token for direct_post
OID4VP’s direct_post mode requires the wallet to form-post the actual VP token (JSON/JWT) as a regular form field. Base64-wrapping it here means verifiers receive a different payload than the spec calls for, so existing integrations will fail (or have to guess whether to decode). Please send the raw token and let the HTTP client handle percent-encoding.(ewc-consortium.github.io)

-        postRequest.add("vp_token", Base64.getUrlEncoder().encodeToString(vpToken.getBytes(StandardCharsets.UTF_8)));
+        postRequest.add("vp_token", vpToken);

📝 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

	postRequest.add("vp_token", Base64.getUrlEncoder().encodeToString(vpToken.getBytes(StandardCharsets.UTF_8)));
	postRequest.add("presentation_submission", presentationSubmission);
	postRequest.add("vp_token", vpToken);
	postRequest.add("presentation_submission", presentationSubmission);

🤖 Prompt for AI Agents

In src/main/java/io/mosip/mimoto/service/impl/PresentationServiceImpl.java
around lines 217 to 218, the code Base64-encodes the vp_token before adding it
to the form which breaks OID4VP direct_post expectations; remove the Base64
encoding and add the raw vpToken string as the vp_token form field (let the HTTP
client handle any percent-encoding), so the wallet posts the actual VP token
(JSON/JWT) per the spec.