[DRAFT PR] for Inji web 0.16.0 Release

[swatigoel]

Summary by CodeRabbit

• New Features

  • Added CSRF protection requiring X-XSRF-TOKEN header for state-changing API requests.
  • Enhanced credential rendering with SVG and PDF support for wallet presentations.
  • Improved OpenID4VP wallet presentation flows with better error handling.
  • Added wallet unlock dependencies to credential operations for enhanced security.

• Bug Fixes

  • Fixed URL validation in credential downloads to prevent path traversal attacks.
  • Enhanced error handling in credential matching and presentation submission flows.

• Documentation

  • Updated API documentation with CSRF token requirements for all state-changing operations.
  • Added deployment guidance for Datashare integration options.
  • Enhanced README with configuration and setup instructions.

• Chores

  • Upgraded Spring Boot and kernel dependencies to final releases.
  • Updated project version to 0.21.0.
  • Migrated package namespaces and test configurations.
  • Updated Helm charts and Docker configurations.

[coderabbitai]

Note

Reviews paused

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Major release (0.20.0 → 0.21.0) consolidating cryptographic utilities into a factory-based architecture, introducing cookie-based CSRF protection, refactoring wallet presentation APIs, adding SVG-based credential rendering, migrating test infrastructure from mosip to inji namespace, and updating deployment configurations with version bumps and image tag updates.

Changes

Cohort / File(s)	Summary
Cryptographic Utilities Consolidation src/main/java/io/mosip/mimoto/util/KeyGenerationUtil.java, src/main/java/io/mosip/mimoto/util/JwtGeneratorUtil.java, src/main/java/io/mosip/mimoto/util/Base64Util.java, src/main/java/io/mosip/mimoto/util/ProofSigningKeyFactory.java, src/main/java/io/mosip/mimoto/util/SigningKeyUtil.java, src/main/java/io/mosip/mimoto/util/factory/*.java	Removes legacy key/JWT utilities and introduces centralized SigningKeyUtil with algorithm-specific handlers (RS256, ES256, ES256K, Ed25519) and factory pattern for extensibility. Adds BouncyCastleProviderUtil for consistent provider management.
CSRF Protection Infrastructure src/main/java/io/mosip/mimoto/config/Config.java, src/main/resources/application-*.properties, docs/api-documentation-openapi.json, docs/postman-collections/MIMOTO.postman_collection.json, docker-compose/config/mimoto-default.properties	Implements cookie-based CSRF token lifecycle with CsrfTokenCookieFilter ensuring tokens are generated/propagated on GET requests and validated on state-changing operations. Updates OpenAPI specs and Postman collection with X-XSRF-TOKEN header documentation.
Wallet Presentations API Consolidation src/main/java/io/mosip/mimoto/controller/WalletPresentationsController.java, src/main/java/io/mosip/mimoto/service/WalletPresentationService.java, src/main/java/io/mosip/mimoto/service/impl/WalletPresentationServiceImpl.java, src/main/java/io/mosip/mimoto/service/PresentationService*.java, src/main/java/io/mosip/mimoto/dto/VP*.java	Consolidates multiple presentation services into unified WalletPresentationService; renames DTOs (VerifiablePresentationResponseDTO → VPResponseDTO, VerifiablePresentationAuthorizationRequest → VPAuthorizationRequestDTO); removes PresentationActionServiceImpl and PresentationSubmissionServiceImpl.
Credential Rendering & PDF Generation src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.java, src/main/java/io/mosip/mimoto/util/SvgFixerUtil.java, src/main/java/io/mosip/mimoto/constant/LdpVcV2Constants.java, src/main/java/io/mosip/mimoto/config/AppConfig.java	Adds InjiVcRenderer-based SVG rendering pathway for LDP VC v2 credentials with SvgFixerUtil for offset correction; extracts claim169-based QR codes; adds LdpVcV2Constants and renderMethod field support in VCCredentialProperties.
Test Infrastructure & Namespace Migration api-test/src/main/java/io/inji/testrig/apirig/mimoto/**, api-test/testNgXmlFiles/*.xml, api-test/pom.xml, api-test/Dockerfile, api-test/README.md	Migrates test runner and all test scripts from io.mosip to io.inji namespace; renames MosipTestRunner to InjiTestRunner; updates Docker ARG defaults and pom coordinates; adds additionalDependencies support in test infrastructure.
Deployment Configurations helm/mimoto/Chart.yaml, helm/mimoto/values.yaml, deploy/mimoto/install.sh, deploy/mimoto-apitestrig/install.sh, deploy/mimoto-apitestrig/values.yaml, docker-compose/docker-compose.yml, db_scripts/init_values.yaml	Updates Helm chart version (0.0.1-develop → 0.21.0-develop), image tags (develop → 0.21.x), install script versions, and postgres volume mount path (/var/lib/postgresql/data → /var/lib/postgresql).
API Test Fixtures & Test Case Dependencies api-test/src/main/resources/testCaseInterDependency.json, api-test/src/main/resources/mimoto/**/*.yml, api-test/src/main/resources/config/mimoto.properties	Adds extensive test case interdependencies (additionalDependencies fields) across LoginFlow, SunBirdC, and Presentations endpoints; restructures authorization and credential flows with wallet unlock prerequisites.
Credential Matching & Service Refactoring src/main/java/io/mosip/mimoto/service/CredentialMatchingService.java, src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java, src/main/java/io/mosip/mimoto/dto/MatchingCredentials*.java, src/main/java/io/mosip/mimoto/dto/CredentialDTO.java	Renames SelectableCredentialDTO → CredentialDTO and MatchingCredentialsWithWalletDataDTO → MatchingCredentialsDTO; refactors credential matching with JsonPath-based claim evaluation and fallback display property generation.
Data Security & Validation src/main/java/io/mosip/mimoto/service/impl/DataShareServiceImpl.java, docker-compose/config/mimoto-default.properties, src/main/resources/mimoto-trusted-verifiers.json	Adds URL path validation in DataShareServiceImpl with regex pattern enforcement; enables CSRF protection by default; updates trusted verifier configuration allowing unsigned requests.
Database & Version Updates db_upgrade_script/inji_mimoto/sql/0.19.2_to_0.20.0_rollback.sql, pom.xml, .talismanrc	Makes SQL rollback idempotent with IF EXISTS clauses; bumps project version 0.20.0 → 0.21.0; upgrades Spring Boot 3.2.3 → 3.3.10; migrates MOSIP kernel dependencies from beta.1 → 1.3.0; adds new rendering dependencies (pdfbox, batik-transcoder, fop, injivcrenderer).
Documentation & Configuration deploy/README.md, docker-compose/README.md, src/main/resources/application-default.properties, .github/workflows/push-trigger.yml	Adds Datashare deployment options (with/without); documents CSRF requirements; updates workflow sonar analysis reference; includes SQLAlchemy and configuration explanations.
Test Suite Additions & Refactoring src/test/java/io/mosip/mimoto/**, src/test/java/io/mosip/mimoto/util/factory/**	Adds comprehensive test coverage for SigningKeyUtil, algorithm handlers (RS256, ES256, ES256K, Ed25519), BouncyCastleProviderUtil, WalletPresentationServiceImpl, ConfigTest with CSRF scenarios; removes deprecated test suites (PresentationActionServiceTest, Base64UtilTest, JwtGeneratorUtilTest).

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Web as Web Server
    participant Session as Session Manager
    participant CSRF as CSRF Filter
    participant Handler as CSRF Token<br/>Cookie Filter
    participant Response as Response

    Client->>Web: GET /page
    Web->>CSRF: Check CSRF (GET)
    CSRF->>Handler: Pass through GET
    Handler->>Session: Load/Generate Token
    Session-->>Handler: XSRF-TOKEN
    Handler->>Response: Set XSRF-TOKEN Cookie
    Response-->>Client: Response + Cookie

    Client->>Web: POST /api/wallets<br/>(with X-XSRF-TOKEN header)
    Web->>CSRF: Validate CSRF Token
    CSRF->>Session: Verify Token
    Session-->>CSRF: Valid
    CSRF->>Web: Proceed
    Web-->>Client: 200 OK
    
    Client->>Web: POST /oauth/token<br/>(CSRF-ignored URL)
    Web->>CSRF: Check ignore list
    CSRF-->>Web: Skipped for this path
    Web-->>Client: 200 OK

Loading

sequenceDiagram
    actor User
    participant Controller as WalletPresentations<br/>Controller
    participant Service as WalletPresentation<br/>Service
    participant VP as OpenID4VP Lib
    participant Verifier as Verifier Backend
    participant DB as Database

    User->>Controller: POST /handleVPAuthorizationRequest
    Controller->>Service: handleVPAuthorizationRequest()
    Service->>VP: Initialize with URL
    VP->>Service: Return verifier metadata
    Service->>Service: Authenticate verifier
    Service->>DB: Store presentation record
    Service-->>Controller: VPResponseDTO
    Controller-->>User: 200 OK + VP Details

    User->>Controller: POST /presentations/{id}/action<br/>(submit)
    Controller->>Service: handlePresentationAction()
    Service->>Service: getMatchingCredentials()
    Service->>Service: signVPToken()
    Service->>VP: Share VP with verifier
    VP->>Verifier: POST VP token
    Verifier-->>VP: 200 Redirect URL
    Service->>DB: Update presentation status
    Service-->>Controller: SubmitPresentationResponseDTO
    Controller-->>User: 200 OK + Redirect

Loading

sequenceDiagram
    participant PDFService as CredentialPDF<br/>Generator Service
    participant Renderer as InjiVcRenderer
    participant SVGFixer as SvgFixerUtil
    participant PDFLib as PDF Library
    participant QRExtractor as Credential Props<br/>Parser

    PDFService->>PDFService: Check isSvgBasedRenderingSupported()
    
    alt SVG Rendering Path (LDP VC v2)
        PDFService->>QRExtractor: Extract claim169 QR
        QRExtractor-->>PDFService: QR data
        
        PDFService->>Renderer: generateCredentialDisplayContent()
        Renderer-->>PDFService: SVG markup
        
        PDFService->>SVGFixer: addMissingOffsetToStopElements()
        SVGFixer-->>PDFService: Fixed SVG
        
        PDFService->>PDFLib: Convert SVG → PDF
        PDFLib-->>PDFService: PDF ByteArray
    else Legacy Path (v1)
        PDFService->>PDFService: Extract masked claims
        PDFService->>PDFLib: Generate PDF
        PDFLib-->>PDFService: PDF ByteArray
    end
    
    PDFService-->>User: PDF Stream

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• [INJIWEB-1621] - Refractor: consolidate KeyGeneration, JWTGeneration, Proof Signing Key utilities using factory pattern #994: Consolidates key/JWT utilities into SigningKeyUtil with new SigningAlgorithmHandler implementations (RS256/ES256/ES256K/Ed25519), adds SigningAlgorithmHandlerFactory and BouncyCastleProviderUtil — directly overlaps cryptographic utilities refactoring.
• [INJIWEB-1779] - Update Service Class Implementations, Rename DTOs, Update Utils Classes, Update Test files #977: Makes overlapping code-level changes to SwaggerLiteralConstants, OpenID4VPConstants, and Wallet Presentation API DTOs/service/controller refactors — directly related to presentation API consolidation.
• [INJIWEB-1774] - Update XSRF token to be true by default #988: Both modify Config.java and properties for cookie-based CSRF protection, update OpenAPI/Postman/test infrastructure for XSRF token handling — directly aligned on CSRF feature implementation.

Suggested reviewers

• mohanachandran-s
• Prafulrakhade

---

🐰 A cryptographic waltz of algorithms and keys,
Presentations dance in a unified tease,
With CSRF guards and SVG renders so bright,
From mosip to inji—the namespace takes flight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.64% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (64 files): ⚔️ .github/workflows/push-trigger.yml (content) ⚔️ .talismanrc (content) ⚔️ api-test/Dockerfile (content) ⚔️ api-test/README.md (content) ⚔️ api-test/pom.xml (content) ⚔️ api-test/src/main/resources/config/mimoto.properties (content) ⚔️ api-test/src/main/resources/config/testCaseInterDependency.json (content) ⚔️ api-test/src/main/resources/mimoto/AuthorizationCode/AuthorizationCode.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/AuthorizationCode/AuthorizationCode.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/DownloadIssuerCredential/DownloadIssuerCredential.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/FetchAllCredentials/FetchAllCredentials.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/ViewCredential/ViewCredential.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadStayProtectedIssuerCredentialWithGoogleLogin/AuthenticateUserSunBirdC/AuthenticateUserSunBirdC.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadStayProtectedIssuerCredentialWithGoogleLogin/AuthorizationCodeSunBirdC/AuthorizationCodeSunBirdC.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/DownloadStayProtectedIssuerCredentialWithGoogleLogin/DownloadIssuerCredential/DownloadIssuerCredential.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/Presentations/AddTrustedVerifier/AddTrustedVerifier.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/Presentations/VerifiablePresentationAuthorization/VerifiablePresentationAuthorization.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/Users/GetUserProfileAfterLogout/GetUserProfileAfterLogout.yml (content) ⚔️ api-test/src/main/resources/mimoto/LoginFlow/Wallet/UnlockWalletAfterDelete/UnlockWalletAfterDelete.yml (content) ⚔️ api-test/src/main/resources/mimoto/SunBirdC/AuthenticateUserSunBirdC/AuthenticateUserSunBirdC.yml (content) ⚔️ api-test/testNgXmlFiles/mimotoPrerequisiteSuite.xml (content) ⚔️ api-test/testNgXmlFiles/mimotoSuite.xml (content) ⚔️ db_scripts/init_values.yaml (content) ⚔️ deploy/README.md (content) ⚔️ deploy/mimoto-apitestrig/install.sh (content) ⚔️ deploy/mimoto-apitestrig/values.yaml (content) ⚔️ deploy/mimoto/install.sh (content) ⚔️ docker-compose/README.md (content) ⚔️ docker-compose/config/mimoto-default.properties (content) ⚔️ docker-compose/docker-compose.yml (content) ⚔️ docs/api-documentation-openapi.json (content) ⚔️ docs/postman-collections/MIMOTO.postman_collection.json (content) ⚔️ helm/mimoto/Chart.yaml (content) ⚔️ helm/mimoto/values.yaml (content) ⚔️ partner-onboarder/install.sh (content) ⚔️ pom.xml (content) ⚔️ src/main/java/io/mosip/mimoto/config/AppConfig.java (content) ⚔️ src/main/java/io/mosip/mimoto/config/Config.java (content) ⚔️ src/main/java/io/mosip/mimoto/dto/mimoto/CredentialDefinitionResponseDto.java (content) ⚔️ src/main/java/io/mosip/mimoto/dto/mimoto/CredentialIssuerDisplayResponse.java (content) ⚔️ src/main/java/io/mosip/mimoto/dto/mimoto/VCCredentialProperties.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/CredentialMatchingServiceImpl.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/CredentialRequestServiceImpl.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/CredentialShareServiceImpl.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/KeyPairRetrievalServiceImpl.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/LdpVcCredentialFormatHandler.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/VcSdJwtCredentialFormatHandler.java (content) ⚔️ src/main/java/io/mosip/mimoto/service/impl/WalletPresentationServiceImpl.java (content) ⚔️ src/main/java/io/mosip/mimoto/util/WalletUtil.java (content) ⚔️ src/main/resources/application-default.properties (content) ⚔️ src/test/java/io/mosip/mimoto/security/oauth2/OAuth2AuthenticationSuccessHandlerTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/CredentialMatchingServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/CredentialPDFGeneratorServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/CredentialRequestServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/KeyPairServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/LdpVcCredentialFormatHandlerTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/PresentationServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/VcSdJwtCredentialFormatHandlerTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/WalletCredentialServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/service/WalletPresentationServiceTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/util/CredentialIssuerWellknownResponseValidatorTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/util/EncryptionDecryptionUtilTest.java (content) ⚔️ src/test/java/io/mosip/mimoto/util/WalletUtilTest.java (content) These conflicts must be resolved before merging into master.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title '[DRAFT PR] for Inji web 0.16.0 Release' clearly summarizes the primary change: preparing a release of Inji web version 0.16.0, which aligns with the comprehensive updates throughout the codebase including version bumps, package rebranding from mosip to inji, and feature additions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch release-0.21.x

---

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: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (10)

api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/FetchAllCredentials/FetchAllCredentials.yml (1)

38-54: ⚠️ Potential issue | 🟡 Minor

Inconsistent additionalDependencies across similar negative test cases.

TC_03 (Empty WalletId, line 44) now has additionalDependencies: TC_Mimoto_UnlockWallet_01, but TC_02 (Invalid WalletId, line 21) and TC_04 (Space WalletId, line 56) — which are structurally identical negative cases with bad wallet IDs and the same valid session cookie — do not. Either all three should have the dependency (if unlock is truly needed for session/state setup) or none of them should (since the walletId is invalid anyway). Please align them for consistency.

api-test/pom.xml (1)

11-11: ⚠️ Potential issue | 🟡 Minor

Project version is 0.20.0-SNAPSHOT while the release targets 0.21.x.

The artifact version on Line 11 and the fileName property on Line 64 still reference 0.20.0-SNAPSHOT. Should these be bumped to 0.21.0-SNAPSHOT to stay consistent with the rest of the release?

src/main/java/io/mosip/mimoto/dto/mimoto/VCCredentialProperties.java (1)

18-18: ⚠️ Potential issue | 🟡 Minor

renderMethod is not listed in @JsonPropertyOrder.

All other fields appear in the @JsonPropertyOrder annotation on line 18, but the new renderMethod field is omitted. This means its position in serialized JSON output will be non-deterministic relative to the ordered fields. If consistent output matters (e.g., for hashing or signature verification of the credential), add it to the ordering.

Proposed fix

-@JsonPropertyOrder({"@context", "credentialSubject", "validUntil", "id", "validFrom", "issuer", "proof", "type", "issuanceDate", "expirationDate"})
+@JsonPropertyOrder({"@context", "credentialSubject", "validUntil", "id", "validFrom", "issuer", "proof", "type", "issuanceDate", "expirationDate", "renderMethod"})

Also applies to: 52-54

api-test/src/main/java/io/mosip/testrig/apirig/mimoto/utils/MimotoUtil.java (1)

411-411: ⚠️ Potential issue | 🟠 Major

Sensitive token logged at INFO level.

Line 411 logs the full Google ID token. This is a bearer credential that could be misused if logs are exposed. Consider logging only a truncated portion or removing this log.

Proposed fix

-		logger.info("Obtained id_token: " + idToken); // Debug log
+		logger.info("Obtained id_token successfully (length: " + idToken.length() + ")");

api-test/src/main/resources/mimoto/LoginFlow/DownloadMosipIssuerCredential/ViewCredential/ViewCredential.yml (1)

301-304: ⚠️ Potential issue | 🟠 Major

Fix duplicate uniqueIdentifier: TC_Mimoto_ViewCredential_14 appears at both line 283 (SpaceIn_AcceptLanguage) and line 304 (Invalid_AcceptHeader).

This duplicate will cause test result shadowing and potential dependency resolution conflicts. Update line 304 to TC_Mimoto_ViewCredential_15 and shift subsequent IDs: line 325 to _16, line 346 to _17, and line 367 to _18.

Proposed fix

-      uniqueIdentifier: TC_Mimoto_ViewCredential_14
+      uniqueIdentifier: TC_Mimoto_ViewCredential_15

-      uniqueIdentifier: TC_Mimoto_ViewCredential_15
+      uniqueIdentifier: TC_Mimoto_ViewCredential_16

-      uniqueIdentifier: TC_Mimoto_ViewCredential_16
+      uniqueIdentifier: TC_Mimoto_ViewCredential_17

-      uniqueIdentifier: TC_Mimoto_ViewCredential_17
+      uniqueIdentifier: TC_Mimoto_ViewCredential_18

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

6-6: ⚠️ Potential issue | 🟡 Minor

NetworkResponse import appears unused after the migration to VerifierResponse.

Line 6 imports io.mosip.openID4VP.networkManager.NetworkResponse, but all usages in this file now reference VerifierResponse instead. This is likely a leftover from the rename.

🧹 Proposed fix

-import io.mosip.openID4VP.networkManager.NetworkResponse;

pom.xml (1)

470-473: ⚠️ Potential issue | 🟠 Major

Remove the explicit spring-core 5.1.12.RELEASE declaration.

This version uses the javax.* namespace (Java EE) and is incompatible with Spring Boot 3.3.10, which requires Spring Framework 6.1.x (Jakarta EE with jakarta.* namespace). The explicit pin will not be overridden by Spring Boot's BOM—explicit dependency declarations take precedence in Maven. Since spring-core is already provided transitively by spring-boot-starter-* dependencies, the explicit declaration is unnecessary and creates a runtime namespace mismatch that will cause failures.

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

153-195: ⚠️ Potential issue | 🟠 Major

Guard against null constraints/paths to prevent NPE during matching.

The current logic assumes inputDescriptor.getConstraints() and field.getPath() are non-null. If a verifier omits constraints or paths, matching will throw. Add null guards and treat missing constraints as “no additional constraints.”

Suggested fix

 private List<String> extractClaimsFromInputDescriptor(InputDescriptor inputDescriptor) {
-    return extractClaimsFromFields(inputDescriptor.getConstraints().getFields(), false);
+    if (inputDescriptor.getConstraints() == null) {
+        return Collections.emptyList();
+    }
+    return extractClaimsFromFields(inputDescriptor.getConstraints().getFields(), false);
 }
@@
 private List<String> extractClaimsFromFields(List<Fields> fields, boolean deduplicate) {
@@
-        .filter(field -> !field.getPath().isEmpty())
+        .filter(field -> field.getPath() != null && !field.getPath().isEmpty())
@@
 }
@@
 private boolean matchesInputDescriptor(VCCredentialResponse vc, InputDescriptor inputDescriptor) {
@@
-    if (inputDescriptor.getConstraints().getFields() != null) {
-        return matchesConstraints(vc, inputDescriptor.getConstraints());
-    }
-    return true;
+    Constraints constraints = inputDescriptor.getConstraints();
+    if (constraints == null || constraints.getFields() == null) {
+        return true;
+    }
+    return matchesConstraints(vc, constraints);
 }

docs/api-documentation-openapi.json (1)

2814-2839: ⚠️ Potential issue | 🟠 Major

CSRF header is documented as mandatory but marked optional.

The descriptions say the CSRF token must be provided for state-changing requests, yet the X-XSRF-TOKEN header is marked required: false. This mismatch can break client code generation and cause avoidable 403s. Either mark it required or soften the wording to indicate conditional use.

🔧 Suggested fix (apply to all X-XSRF-TOKEN parameters)

-        "required": false,
+        "required": true,

Also applies to: 3398-3432, 3507-3523, 3779-3803, 3891-3916, 4084-4103, 4235-4263, 4545-4569

src/test/java/io/mosip/mimoto/controller/WalletPresentationsControllerTest.java (1)

551-576: ⚠️ Potential issue | 🟠 Major

Test assertion expects 200 OK but service returns 400 BAD_REQUEST — incorrect expectation.

The test testHandlePresentationActionInvalidRequestFormat mocks the wrong service. The controller delegates to walletPresentationService.handlePresentationAction() (not presentationActionService), which returns ResponseEntity.status(HttpStatus.BAD_REQUEST) for invalid request format (line 129 of WalletPresentationServiceImpl). The test's mock on presentationActionService is never invoked. The assertion at line 575 should be .andExpect(status().isBadRequest()), not .andExpect(status().isOk()).

🤖 Fix all issues with AI agents

In @.github/workflows/push-trigger.yml:
- Line 77: The workflow reference uses a temporary/commit ref
"mosip/kattu/.github/workflows/maven-sonar-analysis.yml@dsd9685" which is
inconsistent with other reusable workflows; update this ref to the stable "
`@master-java21`" (or the intended stable tag/branch) so the uses entry matches
the other lines and avoids fragile short-SHA or feature-branch pins; confirm and
replace the string
"mosip/kattu/.github/workflows/maven-sonar-analysis.yml@dsd9685" with
"mosip/kattu/.github/workflows/maven-sonar-analysis.yml@master-java21" (or the
approved permanent ref) in the push-trigger workflow.

In @.talismanrc:
- Around line 161-164: Update the incorrect checksum entries in .talismanrc for
PresentationService.java and PresentationServiceImpl.java: replace the
duplicated checksum `9053ee654c...` with the correct checksums—set
PresentationService.java to
`3c303398b9977ab2f25b70431290a831154f52543120fd69f3661676284c2e3a` and
PresentationServiceImpl.java to
`524004aaa18f65e468c3f8a0899ece8d5f728bf3bf737eb272c9621c95dd9c3a`; ensure the
two entries are unique and, if you prefer, regenerate both checksums with your
project's checksum tool and paste the resulting values into the corresponding
filename entries.
- Around line 227-238: Remove the incorrect uppercase entry
"ED25519AlgorithmHandler.java" from the .talismanrc checksum list—only the
correctly cased "Ed25519AlgorithmHandler.java" exists in the repo; locate the
checksum entry referencing ED25519AlgorithmHandler.java (the duplicate/incorrect
filename) and delete that entry so the checksum list only contains the valid
"Ed25519AlgorithmHandler.java" record.

In `@deploy/README.md`:
- Around line 41-48: The note about active_profile_env in the
config-server-share ConfigMap is ambiguous for installs that skip Datashare;
update the README so it only applies when Datashare is deployed. Change the
paragraph referencing active_profile_env / config-server-share (injiweb
namespace) to either: 1) prefix it with “If deploying Datashare (WITH
Datashare):” and keep the instruction as-is, or 2) add a conditional sentence
stating “If you do not deploy Datashare, the config-server-share ConfigMap may
not exist and this step can be skipped.” Make the change near the “Datashare
Deployment” section so readers using only Inji Mobile Wallet won’t expect the
config-server-share ConfigMap to be present.

In `@docs/postman-collections/MIMOTO.postman_collection.json`:
- Around line 14-21: The test script currently grabs the cookie with
pm.cookies.get('XSRF-TOKEN') and unconditionally sets
pm.environment.set('XSRF_TOKEN', token), which allows a null token to silently
propagate; update the script to assert the cookie exists before setting the env
var (use pm.test/pm.expect or a pm.test that fails when token is falsy), and
only call pm.environment.set('XSRF_TOKEN', token) when token is present so
missing cookies produce a clear failing test instead of silent nulls.

In `@pom.xml`:
- Around line 437-441: The pom currently depends on a mutable SNAPSHOT for
groupId io.mosip and artifactId injivcrenderer-jar at version 0.2.0-SNAPSHOT;
update the dependency to a fixed, released version (e.g., 0.2.0 or the latest
stable non-SNAPSHOT release) by replacing the SNAPSHOT version in the <version>
element for the injivcrenderer-jar dependency so builds on the release-0.21.x
branch are reproducible and deterministic.

In `@src/main/java/io/mosip/mimoto/config/Config.java`:
- Around line 205-242: The CsrfToken passed to
csrfTokenRepository.saveToken(...) in CsrfTokenCookieFilter may be a lazy
SupplierCsrfToken; ensure you materialize it before saving by invoking its
getters (e.g., getToken(), getParameterName(), getHeaderName()) or by
constructing a concrete token (e.g., DefaultCsrfToken) using those values, then
call csrfTokenRepository.saveToken with that concrete token; update the logic
around CsrfTokenCookieFilter.doFilterInternal so csrfToken is replaced with the
materialized token prior to csrfTokenRepository.saveToken(...) to avoid
persisting an unmaterialized supplier.

In
`@src/main/java/io/mosip/mimoto/controller/WalletTrustedVerifiersController.java`:
- Line 73: The ApiResponse example in WalletTrustedVerifiersController (the
`@ApiResponse` annotation for 201) contains a malformed UUID; update the example
JSON value in that annotation to use a valid UUID string in the 8-4-4-4-12 hex
group format (for example replace "2d598dfc-7218-cb7a7afd5a07" with a correctly
formatted UUID such as "2d598dfc-7218-4b7a-afd5-a07e9d1c2f34") so the Swagger
docs show a valid UUID for TrustedVerifierResponseDTO.

In `@src/main/java/io/mosip/mimoto/service/CredentialPDFGeneratorService.java`:
- Around line 346-347: In CredentialPDFGeneratorService where
renderMethodProperties.get(LdpVcV2Constants.RENDER_SUITE) is cast to String into
the local variable renderSuite, avoid the unsafe cast; instead obtain a safe
string (e.g., use
Objects.toString(renderMethodProperties.get(LdpVcV2Constants.RENDER_SUITE),
null) or check instanceof String before casting) and use that value in the
subsequent boolean expression that checks
renderMethodProperties.containsKey(LdpVcV2Constants.TEMPLATE) &&
LdpVcV2Constants.SVG_MUSTACHE_RENDER_SUITE.equals(renderSuite); apply the same
defensive conversion/check for the second occurrence around lines referencing
renderSuite (the other place using LdpVcV2Constants.RENDER_SUITE and
SVG_MUSTACHE_RENDER_SUITE) so non-String values or nulls won’t throw
ClassCastException.
- Around line 340-356: The method containsSvgTemplate currently requires every
entry in a renderMethod List to be an SVG mustache template (it uses allMatch),
which is too strict; change the List branch in containsSvgTemplate to use
anyMatch so it returns true if at least one renderMethod entry is a Map
containing LdpVcV2Constants.TEMPLATE and with LdpVcV2Constants.RENDER_SUITE
equal to LdpVcV2Constants.SVG_MUSTACHE_RENDER_SUITE; keep the single Map branch
unchanged.
- Around line 209-221: extractClaim169Qr currently returns an arbitrary value
from the claim169 map by calling claim169Map.values().iterator().next(), which
is non-deterministic; update extractClaim169Qr to explicitly select the QR entry
you need (e.g., look up "identityQRCode" or "faceQRCode") from the map returned
by credentialFormatHandler.extractCredentialClaims(vcCredentialResponse) using
CLAIM_169_KEY, and return that value (toString()) if present, otherwise return
an empty string; alternatively, if the calling code expects all QR data, return
a deterministic representation (e.g., serialize the entire map) instead of a
single arbitrary value.
- Around line 358-393: generatePdfUsingSvgTemplate only builds qrCodeData for
QRCodeType.OnlineSharing, so EmbeddedVC QR codes are omitted; update the
qrCodeData construction to mirror the v1 logic: when issuerDTO.getQr_code_type()
is EmbeddedVC (or fallback claim169 present), set qrCodeData to the embedded VC
payload (e.g., the credential JSON or claim169 value) encoded similarly to
ovpQRDataPattern or the existing EmbeddedVC format used in the v1 path, using
presentationService.constructPresentationDefinition only for OnlineSharing and
otherwise using the credentialJsonString / claim169 fallback; touch
generatePdfUsingSvgTemplate, QRCodeType, ovpQRDataPattern,
presentationService.constructPresentationDefinition and the credentialJsonString
variables to add the EmbeddedVC branch so SVGs include the correct QR content.

In `@src/main/java/io/mosip/mimoto/service/impl/CredentialShareServiceImpl.java`:
- Around line 429-451: mapJsonNodeToJavaObject should stop using
FieldUtils.writeField and should populate JsonValue instances via their setters
(cast the created instance to JsonValue and call setLanguage/setValue) and
ensure you explicitly set javaObject[i] (to the new instance or to null) even
when getJSONObjectFromArray returns null so callers can detect missing entries;
also update appendLangToOutputJSONFields to null-guard each element before
calling jsonValue.getLanguage() (skip or handle nulls) to prevent NPEs.

In `@src/main/java/io/mosip/mimoto/service/impl/PresentationServiceImpl.java`:
- Line 70: In PresentationServiceImpl update the log.error call (the one
invoking log.error("Exception occured during processInputDesciptor: {}",
e.getMessage())) to correct the typos: change "occured" to "occurred" and
"processInputDesciptor" to "processInputDescriptor" so the message reads
something like "Exception occurred during processInputDescriptor: {}". Keep the
same log.error invocation and placeholders (use e.getMessage()) and ensure you
modify only the string literal.
- Around line 83-101: The code builds a format map with List.of(proofType) where
proofType can be null (computed via Optional.ofNullable on
VCCredentialProperties.getProof()), causing an NPE; update the block that
constructs the format map used in InputDescriptorDTO (where
CredentialFormat.LDP_VC is handled and VCCredentialProperties is parsed) to
avoid List.of(null) by either using Collections.singletonList(proofType) which
accepts a null element or, better, conditionally include "proofTypes" only when
proofType != null (i.e., build the inner map/format map by checking proofType
and adding "proofTypes" -> List.of(proofType) only if non-null) so
InputDescriptorDTO.format never receives a list containing null.
- Around line 103-132: The SD-JWT branch can NPE: guard against a null payload
returned from extractJwtPayloadFromSdJwt((String) vcRes.getCredential()) before
accessing jwtPayload.get("type") (e.g., return/skip this branch or treat as
empty when jwtPayload is null) and prevent passing null into List.of by checking
the parsed header from parseJwtHeader((String) vcRes.getCredential()) for a
missing "alg" (e.g., only add "sd-jwt_alg_values" when algo != null or use an
empty/default list), updating the code that builds FieldDTO, format Map, and
InputDescriptorDTO accordingly.

In
`@src/main/java/io/mosip/mimoto/service/impl/WalletPresentationServiceImpl.java`:
- Around line 345-358: In fetchSelectedCredentials, validate that every id in
selectedCredentialIds is present in sessionData.getMatchingCredentials() and
fail fast if any are missing: collect the matching DecryptedCredentialDTOs
(using DecryptedCredentialDTO.getId), compare the set of found ids to the
requested selectedCredentialIds, and if any requested ids are unresolved throw a
400-level exception (e.g. new ResponseStatusException(HttpStatus.BAD_REQUEST,
"Missing credential IDs: " + missingIds)) so the caller receives a Bad Request
instead of silently dropping unknown IDs.

In `@src/test/java/io/mosip/mimoto/service/DataShareServiceTest.java`:
- Around line 144-154: The implementation currently rejects any path containing
the substring ".." which causes false positives (e.g., "te..st"); update the
path traversal check in DataShareServiceImpl (the method that validates the
resource path used by downloadCredentialFromDataShare) to split the decoded path
on '/' and reject only when any individual segment equals ".." (and optionally
"."), rather than using decodedPath.contains(".."); locate the validation logic
in the method that decodes and validates the resource URL and replace the broad
contains check with per-segment equality checks to avoid rejecting legitimate
identifiers that include ".." as a substring.

In `@src/test/java/io/mosip/mimoto/service/WalletPresentationServiceTest.java`:
- Around line 698-710: The catch block in WalletPresentationServiceTest around
walletPresentationService.submitPresentation(...) is too permissive (it allows
many exceptions including NullPointerException); update the test to assert the
specific expected exception instead of the grab-bag: determine the contract for
submitPresentation when sessionDataWithDifferentCred is used, replace the broad
try/catch with assertThrows for the single expected exception type (or explicit
fail() for other exceptions), remove java.lang.NullPointerException from
accepted outcomes, and reference submitPresentation,
sessionDataWithDifferentCred, walletId, presentationId, request and base64Key
when making the assertion.
- Around line 770-791: The test testStorePresentationRecordNullSessionData
currently swallows all exceptions in an empty catch; replace that try/catch with
an assertion that submitPresentation(nullSessionData, walletId, presentationId,
submitRequest, base64Key) throws the expected exception (e.g., use
assertThrows(IllegalArgumentException.class, ...) or the appropriate exception
type) so the test actually verifies failure on null session; keep the
SigningKeyUtil and UrlParameterUtils mocks but remove the empty catch and add
the assertion around walletPresentationService.submitPresentation to validate
the error path.

🧹 Nitpick comments (34)

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

644-663: Extract duplicated exception-unwrapping logic into a helper.

The UndeclaredThrowableException unwrapping pattern is duplicated verbatim across shouldThrowDecryptionExceptionWhenDecryptionFails and shouldThrowIOExceptionWhenJsonParsingFails. Consider a small helper to reduce boilerplate and improve readability.

♻️ Suggested helper method

Add a private helper at the bottom of the test class:

private <T extends Exception> T unwrapException(Exception thrown, Class<T> expectedType) {
    if (expectedType.isInstance(thrown)) {
        return expectedType.cast(thrown);
    }
    if (thrown instanceof java.lang.reflect.UndeclaredThrowableException) {
        Throwable cause = thrown.getCause();
        assertTrue("Expected " + expectedType.getSimpleName() + " but got: " + cause.getClass().getName(),
                expectedType.isInstance(cause));
        return expectedType.cast(cause);
    }
    fail("Expected " + expectedType.getSimpleName() + " but got: " + thrown.getClass().getName());
    return null; // unreachable
}

Then simplify each test:

-        Exception thrownException = assertThrows(Exception.class, () ->
-                ReflectionTestUtils.invokeMethod(walletCredentialService, "decryptAndParseCredential", testCredential1, base64Key));
-
-        DecryptionException exception = null;
-        if (thrownException instanceof java.lang.reflect.UndeclaredThrowableException) {
-            Throwable cause = thrownException.getCause();
-            assertTrue("Expected DecryptionException but got: " + cause.getClass().getName(), 
-                    cause instanceof DecryptionException);
-            exception = (DecryptionException) cause;
-        } else if (thrownException instanceof DecryptionException) {
-            exception = (DecryptionException) thrownException;
-        } else {
-            fail("Expected DecryptionException but got: " + thrownException.getClass().getName());
-        }
-
-        assertNotNull("Exception should not be null", exception);
+        Exception thrownException = assertThrows(Exception.class, () ->
+                ReflectionTestUtils.invokeMethod(walletCredentialService, "decryptAndParseCredential", testCredential1, base64Key));
+        DecryptionException exception = unwrapException(thrownException, DecryptionException.class);

Also applies to: 675-693

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

126-126: Consider reducing visibility of extractClaimsFromSdJwt.

This method is declared public but appears to be an internal implementation detail of this handler. If it's only called from within this class and tests, package-private or private would better communicate intent and reduce the public API surface.

---

172-199: Fallback builder is clean; consider extracting the shared fallback-display pattern.

The fallback display construction (lines 192-194) duplicates the same pattern used in lines 112-116 of the normal path. Both create a CredentialIssuerDisplayResponse with camelToTitleCase(key) and hardcoded "en" locale. Consider extracting a small helper like buildDefaultDisplay(String key) to DRY this up.

♻️ Suggested helper extraction

+    private CredentialIssuerDisplayResponse buildDefaultDisplay(String key) {
+        CredentialIssuerDisplayResponse display = new CredentialIssuerDisplayResponse();
+        display.setName(camelToTitleCase(key));
+        display.setLocale("en");
+        return display;
+    }

Then replace lines 112-116 with:

-            if (display == null) {
-                display = new CredentialIssuerDisplayResponse();
-                display.setName(camelToTitleCase(key));
-                display.setLocale("en");
-            }
+            if (display == null) {
+                display = buildDefaultDisplay(key);
+            }

And lines 192-194 with:

-            CredentialIssuerDisplayResponse display = new CredentialIssuerDisplayResponse();
-            display.setName(camelToTitleCase(key));
-            display.setLocale("en");
+            CredentialIssuerDisplayResponse display = buildDefaultDisplay(key);

.talismanrc (1)

148-151: Pre-existing: Controller and its test share an identical checksum.

WalletPresentationsControllerTest.java (line 149) and WalletPresentationsController.java (line 151) have the same checksum. While these lines weren't changed in this PR, this is the same copy-paste pattern flagged above and worth fixing while you're updating the file.

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

111-113: orderedKeys can never be null here — the null check is dead code.

orderedKeys is initialized on line 77–79 via Optional...orElse(new LinkedHashSet<>()), so it's never null. The != null part of the condition is always true. This is minor but slightly misleading to future readers.

Suggested simplification

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

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

143-150: Test names are stale — they say "ShouldReturnEmptyMap" but now assert fallback results.

The method names loadDisplayPropertiesFromWellknownWithNullCredentialDefinitionShouldReturnEmptyMap (line 131) and loadDisplayPropertiesFromWellknownWithNullCredentialSubjectShouldReturnEmptyMap (line 154) no longer match their behavior. Consider renaming to reflect the new fallback expectation, e.g., ...ShouldReturnFallbackDisplayProperties.

Also applies to: 167-174

---

177-199: This test is a duplicate of loadDisplayPropertiesFromWellknownWithNullCredentialSubjectShouldReturnEmptyMap (lines 153–175).

Both tests set credentialSubject to null on a non-null credentialDefinition and assert the same fallback behavior. The test name references "NullDisplayConfigMap" but the setup is identical to the null-credential-subject test. Either remove this duplicate or differentiate the setup to test a genuinely different scenario.

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

50-50: Stale variable name after utility class rename.

The field keyGenerationUtilMockedStatic still references the old class name. Consider renaming to signingKeyUtilMockedStatic for consistency with the SigningKeyUtil class it now mocks.

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

14-20: Minor: BC_PROVIDER is always instantiated even if already registered.

The BouncyCastleProvider instance on Line 14 is always created, even when BC is already registered (Line 17 check). This is a negligible one-time cost, but if you want to be precise, you could defer creation inside the if block.

♻️ Optional: defer instantiation

-    private static final Provider BC_PROVIDER = new BouncyCastleProvider();
-
     static {
         if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
-            Security.addProvider(BC_PROVIDER);
+            Security.addProvider(new BouncyCastleProvider());
         }
     }

src/main/java/io/mosip/mimoto/util/factory/Ed25519AlgorithmHandler.java (2)

47-55: Avoid repeated getEncoded() calls — cache in local variables.

edECPublicKey.getEncoded() is called three times on Line 51 and edECPrivateKey.getEncoded() three times on Line 52. Each call allocates a new byte array (defensive copy). Store the result in a local variable.

♻️ Proposed fix

     public JWK createJWK(KeyPair keyPair) {
         EdECPublicKey edECPublicKey = (EdECPublicKey) keyPair.getPublic();
         EdECPrivateKey edECPrivateKey = (EdECPrivateKey) keyPair.getPrivate();
 
-        byte[] x = Arrays.copyOfRange(edECPublicKey.getEncoded(), edECPublicKey.getEncoded().length - ED25519_KEY_SIZE_BYTES, edECPublicKey.getEncoded().length);
-        byte[] d = Arrays.copyOfRange(edECPrivateKey.getEncoded(), edECPrivateKey.getEncoded().length - ED25519_KEY_SIZE_BYTES, edECPrivateKey.getEncoded().length);
+        byte[] publicEncoded = edECPublicKey.getEncoded();
+        byte[] privateEncoded = edECPrivateKey.getEncoded();
+        byte[] x = Arrays.copyOfRange(publicEncoded, publicEncoded.length - ED25519_KEY_SIZE_BYTES, publicEncoded.length);
+        byte[] d = Arrays.copyOfRange(privateEncoded, privateEncoded.length - ED25519_KEY_SIZE_BYTES, privateEncoded.length);
 
         return new OctetKeyPair.Builder(Curve.Ed25519, Base64URL.encode(x)).d(Base64URL.encode(d)).algorithm(JWSAlgorithm.Ed25519).keyUse(KeyUse.SIGNATURE).build();
     }

---

57-87: Replace the custom JWSSigner implementation with Nimbus's built-in Ed25519Signer.

The project already depends on Nimbus JOSE+JWT 9.41.2, which provides com.nimbusds.jose.crypto.Ed25519Signer(OctetKeyPair privateKey). This eliminates the need for the custom anonymous JWSSigner and avoids reimplementing logic already handled by the library.

Simply instantiate new Ed25519Signer(okp) instead of creating the custom implementation using BouncyCastle's low-level API.

src/test/java/io/mosip/mimoto/util/factory/ES256KAlgorithmHandlerTest.java (2)

24-30: Consider using BouncyCastleProviderUtil for provider registration.

The setUp method manually registers the BC provider, duplicating the logic already centralized in BouncyCastleProviderUtil. You could simply call BouncyCastleProviderUtil.getProvider() to trigger the static initializer and keep things DRY. The same applies to RS256AlgorithmHandlerTest.setUp().

---

69-76: Nit: reuse a single RS256AlgorithmHandler instance.

Lines 72–73 each create a new RS256AlgorithmHandler(). One instance suffices since handlers are stateless.

♻️ Minor cleanup

-        KeyPair rsaKeyPair = new RS256AlgorithmHandler().generateKeyPair();
-        JWK invalidJwk = new RS256AlgorithmHandler().createJWK(rsaKeyPair);
+        RS256AlgorithmHandler rsaHandler = new RS256AlgorithmHandler();
+        KeyPair rsaKeyPair = rsaHandler.generateKeyPair();
+        JWK invalidJwk = rsaHandler.createJWK(rsaKeyPair);

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

59-79: Reflection-based test mutates shared static state.

This works under sequential JUnit 4 execution, but if tests are ever parallelized, removing an entry from the static handlers map could cause flaky failures in other tests. The finally block restores state correctly, so this is acceptable for now.

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

177-183: Minor: Unnecessary manual time truncation.

The System.currentTimeMillis() / 1000 * 1000 round-trip to truncate milliseconds is functionally harmless but unnecessarily obscure. new Date() works fine for JWT claims since Nimbus internally handles second precision.

Simplification

     private static JWTClaimsSet createClaims(String clientId, String audience, String cNonce) {
-        long nowSeconds = System.currentTimeMillis() / 1000;
-        Date issuedAt = new Date(nowSeconds * 1000);
-        Date expiresAt = new Date((nowSeconds + JWT_EXPIRATION_SECONDS) * 1000);
+        Date issuedAt = new Date();
+        Date expiresAt = new Date(issuedAt.getTime() + JWT_EXPIRATION_SECONDS * 1000L);
 
         return new JWTClaimsSet.Builder().subject(clientId).audience(audience).issuer(clientId).issueTime(issuedAt).expirationTime(expiresAt).claim("nonce", cNonce).build();
     }

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

14-14: Prefer a standard @NonNull annotation over org.jetbrains.annotations.NotNull.

This introduces a compile-time dependency on the JetBrains annotations library. Since the project already uses Jakarta (jakarta.annotation.PostConstruct on Line 12), consider jakarta.annotation.Nonnull or Spring's org.springframework.lang.NonNull instead for consistency. Moreover, since getWildcardPart is a private static helper that always either returns a non-empty string or throws, the annotation provides limited value—it could be removed entirely.

Proposed fix

-import org.jetbrains.annotations.NotNull;

-    `@NotNull`
     private static String getWildcardPart(String decodedPath) {

Also applies to: 151-152

---

159-167: segments.length == 0 is unreachable in Java.

In Java, String.split("/") always returns an array with at least one element, even for an empty string ("".split("/") → [""]). The only realistic empty-segment scenario is handled by the wildcardPart.isEmpty() check on Line 169. This branch is dead code.

Not a bug (it's harmlessly defensive), but worth noting.

Simplification

     String[] segments = decodedPath.split("/");
-    String wildcardPart;
-    if( segments.length == 0 ){
-        throw new InvalidCredentialResourceException(
-                ErrorConstants.RESOURCE_INVALID.getErrorCode(),
-                "Invalid resource identifier in URL");
-    } else {
-        wildcardPart = segments[segments.length - 1];
-    }
+    String wildcardPart = segments[segments.length - 1];
 
     if (wildcardPart.isEmpty()) {

---

112-113: Nit: extra blank line.

There's a double blank line after validateResourceURL(credentialsResourceUri); (Lines 113-114). Consider removing one for consistency with the rest of the file.

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

144-217: Consider adding a positive test for a valid resource ID containing dots (e.g., file.json).

All six new tests are negative cases. Adding a positive test for a segment with allowed special characters (e.g., some-file.json or resource_v1.2) would confirm these don't get falsely rejected, strengthening confidence in the regex and traversal checks.

src/main/java/io/mosip/mimoto/config/AppConfig.java (1)

22-25: Consider externalizing the "inji-web" initializer value.

The renderer name is hardcoded. If this varies across environments or needs to change, extracting it to a @Value("${...}") property would be more flexible. Low priority if it's truly a constant.

api-test/src/main/java/io/mosip/testrig/apirig/mimoto/utils/MimotoUtil.java (1)

105-109: Redundant null check and verbose boolean comparison.

testCaseDTO is already dereferenced on line 92 (testCaseDTO.getTestCaseName()), so the null check on line 106 is unreachable as a guard — it would have already NPE'd. Also, == true is verbose for a boolean comparison.

Proposed simplification

 		// Handle extra workflow dependencies
-		if (testCaseDTO != null && testCaseDTO.getAdditionalDependencies() != null
-				&& AdminTestUtil.generateDependency == true) {
+		if (testCaseDTO.getAdditionalDependencies() != null
+				&& AdminTestUtil.generateDependency) {
 			addAdditionalDependencies(testCaseDTO);
 		}

src/main/java/io/mosip/mimoto/config/Config.java (1)

121-134: Side-effect initialization of csrfTokenRepository couples method ordering.

configureCsrf initializes the instance field csrfTokenRepository as a side-effect, which means filterChain must call configureCsrf(http) before new CsrfTokenCookieFilter(csrfTokenRepository) on line 99. This implicit ordering dependency is fragile. Consider initializing csrfTokenRepository as a field declaration or a @Bean method, then injecting/referencing it in both places.

♻️ Suggested refactor

-    private CookieCsrfTokenRepository csrfTokenRepository;
+    private final CookieCsrfTokenRepository csrfTokenRepository = createCsrfTokenRepository();
+
+    private static CookieCsrfTokenRepository createCsrfTokenRepository() {
+        CookieCsrfTokenRepository repository = CookieCsrfTokenRepository.withHttpOnlyFalse();
+        repository.setCookiePath(CSRF_COOKIE_PATH);
+        repository.setCookieName(CSRF_COOKIE_NAME);
+        repository.setHeaderName(CSRF_HEADER_NAME);
+        return repository;
+    }

     private void configureCsrf(HttpSecurity http) throws Exception {
-        csrfTokenRepository = CookieCsrfTokenRepository.withHttpOnlyFalse();
-        csrfTokenRepository.setCookiePath(CSRF_COOKIE_PATH);
-        csrfTokenRepository.setCookieName(CSRF_COOKIE_NAME);
-        csrfTokenRepository.setHeaderName(CSRF_HEADER_NAME);
-
         CsrfTokenRequestHandler requestHandler = new CsrfTokenRequestAttributeHandler();

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

6-35: Good basic coverage. Consider adding an empty-string input test.

The four cases cover the core scenarios well. An edge case for empty string input ("") would round out the null/empty handling.

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

101-112: Dead null check: responseUris can never be null on line 109.

Line 103 unconditionally assigns either List.of(responseUriValue) or List.of(), both non-null. The responseUris==null check on line 109 is dead code.

🧹 Simplify the guard

-        if (responseUris==null || responseUris.isEmpty()) {
+        if (responseUris.isEmpty()) {

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

213-255: Redirect-URI priority logic looks correct; consider reducing log verbosity.

The precedence chain (request redirectUri → response redirect_uri → fallback) is clear and well-structured. However, Line 237 logs the entire verifier response at INFO level. Consider using DEBUG to avoid exposing potentially sensitive verifier payloads in production logs.

Proposed change

-            log.info("Response from verifier after POST: {}", postResponse);
+            log.debug("Response from verifier after POST: {}", postResponse);

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

633-697: Tests validate credential serialization, not constructPresentationDefinition output.

Both constructPresentationDefinitionWithRenderMethod and WithoutRenderMethod assert on the serialized credential (via mapper.writeValueAsString(credential)) rather than the returned PresentationDefinitionDTO. Consider also asserting on result to verify the method's actual output.

src/test/java/io/mosip/mimoto/config/ConfigTest.java (1)

423-434: Weak assertion on whitespace-in-origins test.

Line 433 uses assertTrue(corsConfig.getAllowedOrigins().size() >= 2) which could pass even with unexpected parsing behavior. Consider asserting the exact count and checking the actual values to document whether whitespace is trimmed or preserved.

Suggested improvement

-        assertTrue(corsConfig.getAllowedOrigins().size() >= 2);
+        assertEquals(2, corsConfig.getAllowedOrigins().size());
+        // Document that whitespace is NOT trimmed by the current implementation
+        assertTrue(corsConfig.getAllowedOrigins().contains("localhost:8088 "));
+        assertTrue(corsConfig.getAllowedOrigins().contains(" localhost:3000"));

src/main/java/io/mosip/mimoto/constant/SwaggerLiteralConstants.java (1)

113-114: Nit: Constant name vs. display name mismatch could confuse API consumers.

WALLET_PRESENTATIONS_HANDLE_AUTHORIZATION_400_WALLET_LOCKED_NAME is set to "Wallet ID not found in session", but the constant name suggests a "wallet locked" scenario. The corresponding _VALUE uses errorCode: "wallet_locked". Consider aligning the display name (e.g., "Wallet is locked") to match the error code, or renaming the constant to reflect "not found in session".

src/main/java/io/mosip/mimoto/controller/WalletCredentialsController.java (1)

288-315: Consider adding a similar catch-all to deleteCredential for consistency.

getVerifiableCredential now has a catch-all Exception handler (Line 261), but deleteCredential does not. An unexpected exception here (e.g., a database error not wrapped in a known exception type) would propagate unhandled. The same applies to downloadCredential (Line 137) and fetchAllCredentialsForGivenWallet (Line 187).

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

461-469: Minor: redundant null check on line 465.

At line 464, identity is the result of casting a non-null LinkedHashMap (the instanceof check on line 463 already guarantees the object is a non-null LinkedHashMap). The identity != null check is always true.

Simplify

     if (object instanceof LinkedHashMap) {
         LinkedHashMap<String, Object> identity = (LinkedHashMap<String, Object>) jsonArray.get(index);
-        return identity != null ? new JSONObject(identity) : null;
+        return new JSONObject(identity);
     } else {

src/main/java/io/mosip/mimoto/constant/LdpVcV2Constants.java (1)

6-17: Add a private constructor to prevent instantiation.

This is a constants-only class. Adding a private constructor makes intent clear and prevents accidental instantiation.

Proposed fix

 public class LdpVcV2Constants {
+
+    private LdpVcV2Constants() {
+    }
 
     // Credential context and URL

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

422-426: Doc mismatch: @Transactional mentioned but not applied.

Either add the annotation or update the comment to match reality to avoid confusion for maintainers.

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

970-1054: Near-duplicate tests: testExtractVerifierAuthRequestExceptionDuringExtraction and testCreatePresentationDataExceptionDuringCreation have identical setup and assertions.

Both tests (lines 970–1011 and 1013–1054) configure the same mocks, use the same objectMapper.writeValueAsString chaining pattern (first returns JSON, then throws JsonProcessingException), and assert only assertNotNull(result). If they target different code paths, consider differentiating the assertions (e.g., verifying which code path handled the exception). If they are truly the same scenario, one should be removed.

---

211-254: Significant mock-setup duplication across submission-flow tests.

The happy-path mock configuration (create OpenID4VP, get trusted verifiers, get key pair, construct unsigned VP token, mock signing key util, mock URL parameter utils, mock JWS signer, mock objectMapper) is repeated verbatim in testHandlePresentationActionSubmissionRequestSuccess, testSubmitPresentationSuccess, testSubmitPresentationShareFailed, testSubmitPresentationExceptionDuringShare, testStorePresentationRecordExceptionDuringSave, and several testExtractVerifier* methods. Extracting a helper method (e.g., setupSubmissionHappyPathMocks()) would cut ~50% of the boilerplate and make each test's distinguishing setup easier to spot.

Also applies to: 431-471

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Clarify the config-server-share note for no-datashare installs.

The active_profile_env note reads like a requirement even when Datashare is skipped; in that path the config-server-share ConfigMap may not exist. Please scope the note to the “WITH Datashare” flow or add a conditional note to avoid confusion.

🤖 Prompt for AI Agents

In `@deploy/README.md` around lines 41 - 48, The note about active_profile_env in
the config-server-share ConfigMap is ambiguous for installs that skip Datashare;
update the README so it only applies when Datashare is deployed. Change the
paragraph referencing active_profile_env / config-server-share (injiweb
namespace) to either: 1) prefix it with “If deploying Datashare (WITH
Datashare):” and keep the instruction as-is, or 2) add a conditional sentence
stating “If you do not deploy Datashare, the config-server-share ConfigMap may
not exist and this step can be skipped.” Make the change near the “Datashare
Deployment” section so readers using only Inji Mobile Wallet won’t expect the
config-server-share ConfigMap to be present.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Guard against missing XSRF cookie to avoid silent failures.

If the cookie isn’t present (cookie jar disabled or domain mismatch), XSRF_TOKEN becomes null and later requests fail without a clear signal. Consider asserting the cookie is present before setting the env var.

Suggested tweak

- "var token = pm.cookies.get('XSRF-TOKEN');",
- "pm.environment.set('XSRF_TOKEN', token);"
+ "var token = pm.cookies.get('XSRF-TOKEN');",
+ "pm.test('XSRF token cookie present', function () {",
+ "  pm.expect(token, 'XSRF-TOKEN cookie missing').to.be.ok;",
+ "});",
+ "pm.environment.set('XSRF_TOKEN', token);"

🤖 Prompt for AI Agents

In `@docs/postman-collections/MIMOTO.postman_collection.json` around lines 14 -
21, The test script currently grabs the cookie with pm.cookies.get('XSRF-TOKEN')
and unconditionally sets pm.environment.set('XSRF_TOKEN', token), which allows a
null token to silently propagate; update the script to assert the cookie exists
before setting the env var (use pm.test/pm.expect or a pm.test that fails when
token is falsy), and only call pm.environment.set('XSRF_TOKEN', token) when
token is present so missing cookies produce a clear failing test instead of
silent nulls.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Two NPE risks in the SD-JWT path.

1. Line 104: extractJwtPayloadFromSdJwt returns null when the input is blank. The next line jwtPayload.get("type") will throw NPE. Add a null check.
2. Line 117/125: If the JWT header lacks an "alg" key, algo is null, and List.of(algo) on line 125 throws NPE.

Proposed fix sketch

             Map<String, Object> jwtPayload = extractJwtPayloadFromSdJwt((String) vcRes.getCredential());
+            if (jwtPayload == null) {
+                jwtPayload = Collections.emptyMap();
+            }
             List<?> typeList = (List<?>) jwtPayload.get("type");
             ...
             String algo = (String) jwtHeaders.get("alg");
+            if (algo == null) {
+                algo = "";
+            }

📝 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

	} else if (CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(vcFormat) || CredentialFormat.DC_SD_JWT.getFormat().equalsIgnoreCase(vcFormat)) {
	Map<String, Object> jwtPayload = extractJwtPayloadFromSdJwt((String) vcRes.getCredential());
	List<?> typeList = (List<?>) jwtPayload.get("type");
	String lastType = null;
	if (typeList != null && !typeList.isEmpty()) {
	Object lastItem = typeList.get(typeList.size() - 1);
	if (lastItem instanceof Map) {
	Object value = ((Map<?, ?>) lastItem).get("_value");
	lastType = value != null ? value.toString() : null;
	} else {
	lastType = lastItem.toString();
	}
	}
	Map<String, Object> jwtHeaders = parseJwtHeader((String) vcRes.getCredential());
	String algo = (String) jwtHeaders.get("alg");
	FieldDTO field = FieldDTO.builder()
	.path(new String[]{"$.type"})
	.filter(FilterDTO.builder().type("String").pattern(lastType).build())
	.build();
	Map<String, Map<String, List<String>>> format = Map.of(
	vcRes.getFormat(), Map.of(
	"sd-jwt_alg_values", List.of(algo)
	)
	);
	inputDescriptors.add(InputDescriptorDTO.builder()
	.id(UUID.randomUUID().toString())
	.constraints(ConstraintsDTO.builder().fields(new FieldDTO[]{field}).build())
	.format(format)
	.build());
	} else if (CredentialFormat.VC_SD_JWT.getFormat().equalsIgnoreCase(vcFormat) || CredentialFormat.DC_SD_JWT.getFormat().equalsIgnoreCase(vcFormat)) {
	Map<String, Object> jwtPayload = extractJwtPayloadFromSdJwt((String) vcRes.getCredential());
	if (jwtPayload == null) {
	jwtPayload = Collections.emptyMap();
	}
	List<?> typeList = (List<?>) jwtPayload.get("type");
	String lastType = null;
	if (typeList != null && !typeList.isEmpty()) {
	Object lastItem = typeList.get(typeList.size() - 1);
	if (lastItem instanceof Map) {
	Object value = ((Map<?, ?>) lastItem).get("_value");
	lastType = value != null ? value.toString() : null;
	} else {
	lastType = lastItem.toString();
	}
	}
	Map<String, Object> jwtHeaders = parseJwtHeader((String) vcRes.getCredential());
	String algo = (String) jwtHeaders.get("alg");
	if (algo == null) {
	algo = "";
	}
	FieldDTO field = FieldDTO.builder()
	.path(new String[]{"$.type"})
	.filter(FilterDTO.builder().type("String").pattern(lastType).build())
	.build();
	Map<String, Map<String, List<String>>> format = Map.of(
	vcRes.getFormat(), Map.of(
	"sd-jwt_alg_values", List.of(algo)
	)
	);
	inputDescriptors.add(InputDescriptorDTO.builder()
	.id(UUID.randomUUID().toString())
	.constraints(ConstraintsDTO.builder().fields(new FieldDTO[]{field}).build())
	.format(format)
	.build());

🤖 Prompt for AI Agents

In `@src/main/java/io/mosip/mimoto/service/impl/PresentationServiceImpl.java`
around lines 103 - 132, The SD-JWT branch can NPE: guard against a null payload
returned from extractJwtPayloadFromSdJwt((String) vcRes.getCredential()) before
accessing jwtPayload.get("type") (e.g., return/skip this branch or treat as
empty when jwtPayload is null) and prevent passing null into List.of by checking
the parsed header from parseJwtHeader((String) vcRes.getCredential()) for a
missing "alg" (e.g., only add "sd-jwt_alg_values" when algo != null or use an
empty/default list), updating the code that builds FieldDTO, format Map, and
InputDescriptorDTO accordingly.

[swatigoel]

@jackjain check this

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fail fast when selected credential IDs are missing from the session cache.

fetchSelectedCredentials silently drops unknown IDs, so a request can submit an empty/partial VP while still returning success. That’s a correctness issue; validate that all requested IDs are resolved and return 400 if not.

Suggested fix

-        return sessionData.getMatchingCredentials().stream().filter(credential -> selectedCredentialIds.contains(credential.getId())).collect(Collectors.toList());
+        List<DecryptedCredentialDTO> selected = sessionData.getMatchingCredentials().stream()
+                .filter(credential -> selectedCredentialIds.contains(credential.getId()))
+                .collect(Collectors.toList());
+
+        Set<String> foundIds = selected.stream()
+                .map(DecryptedCredentialDTO::getId)
+                .collect(Collectors.toSet());
+        List<String> missingIds = selectedCredentialIds.stream()
+                .filter(id -> !foundIds.contains(id))
+                .toList();
+        if (!missingIds.isEmpty()) {
+            throw new IllegalArgumentException("Selected credentials not found in session cache: " + missingIds);
+        }
+
+        return selected;

🤖 Prompt for AI Agents

In
`@src/main/java/io/mosip/mimoto/service/impl/WalletPresentationServiceImpl.java`
around lines 345 - 358, In fetchSelectedCredentials, validate that every id in
selectedCredentialIds is present in sessionData.getMatchingCredentials() and
fail fast if any are missing: collect the matching DecryptedCredentialDTOs
(using DecryptedCredentialDTO.getId), compare the set of found ids to the
requested selectedCredentialIds, and if any requested ids are unresolved throw a
400-level exception (e.g. new ResponseStatusException(HttpStatus.BAD_REQUEST,
"Missing credential IDs: " + missingIds)) so the caller receives a Bad Request
instead of silently dropping unknown IDs.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

The .. substring check in the implementation produces false positives for legitimate segments.

This test demonstrates that te..st is rejected because the path contains(".."). While this is a valid test for the current implementation, note that te..st is not a real path traversal—it just happens to contain .. as a substring. A more precise check in the implementation would test for segments that are exactly .. (e.g., splitting by / and checking each segment) rather than a broad contains call. Since resource IDs in this system are likely UUIDs or similar opaque identifiers, the over-restriction is probably fine in practice.

More precise traversal check (in DataShareServiceImpl)

Instead of the broad decodedPath.contains("..") on the full path, consider checking individual path segments:

-    if (decodedPath.contains("..") || decodedPath.contains("//")) {
+    String[] allSegments = decodedPath.split("/");
+    for (String seg : allSegments) {
+        if ("..".equals(seg) || ".".equals(seg)) {
+            throw new InvalidCredentialResourceException(
+                    ErrorConstants.RESOURCE_INVALID.getErrorCode(),
+                    "Invalid path structure in resource URL");
+        }
+    }
+    if (decodedPath.contains("//")) {

🤖 Prompt for AI Agents

In `@src/test/java/io/mosip/mimoto/service/DataShareServiceTest.java` around lines
144 - 154, The implementation currently rejects any path containing the
substring ".." which causes false positives (e.g., "te..st"); update the path
traversal check in DataShareServiceImpl (the method that validates the resource
path used by downloadCredentialFromDataShare) to split the decoded path on '/'
and reject only when any individual segment equals ".." (and optionally "."),
rather than using decodedPath.contains(".."); locate the validation logic in the
method that decodes and validates the resource URL and replace the broad
contains check with per-segment equality checks to avoid rejecting legitimate
identifiers that include ".." as a substring.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Overly permissive catch block masks real failures.

Accepting NullPointerException (and 7 other exception types) as valid outcomes means this test will pass even if the production code has an NPE bug. The test should assert the specific expected exception type, not a grab-bag of possibilities.

// Current: passes for almost any exception, including NPE
assertTrue("Unexpected exception type: ...",
    e instanceof IllegalStateException || e instanceof InvalidRequestException || 
    e instanceof ApiNotAccessibleException || e instanceof IOException || 
    e instanceof JOSEException || e instanceof DecryptionException ||
    e instanceof KeyGenerationException || e instanceof java.lang.NullPointerException);

If the expected behavior when a selected credential ID doesn't match any session credential is unclear, determine the contract first, then assert only that exception.

🤖 Prompt for AI Agents

In `@src/test/java/io/mosip/mimoto/service/WalletPresentationServiceTest.java`
around lines 698 - 710, The catch block in WalletPresentationServiceTest around
walletPresentationService.submitPresentation(...) is too permissive (it allows
many exceptions including NullPointerException); update the test to assert the
specific expected exception instead of the grab-bag: determine the contract for
submitPresentation when sessionDataWithDifferentCred is used, replace the broad
try/catch with assertThrows for the single expected exception type (or explicit
fail() for other exceptions), remove java.lang.NullPointerException from
accepted outcomes, and reference submitPresentation,
sessionDataWithDifferentCred, walletId, presentationId, request and base64Key
when making the assertion.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Test testStorePresentationRecordNullSessionData always passes — empty catch block swallows all exceptions.

The test catches any exception at line 788 and does nothing. It never asserts anything about the exception type, message, or that the correct code path was exercised. This effectively tests nothing.

🛡️ Suggested fix: assert the expected behavior

If submitPresentation should throw on a null session, use @Test(expected = ...) or assert in the catch:

-            try {
-                walletPresentationService.submitPresentation(
-                        nullSessionData, walletId, presentationId, submitRequest, base64Key);
-            } catch (Exception e) {
-            }
+            try {
+                walletPresentationService.submitPresentation(
+                        nullSessionData, walletId, presentationId, submitRequest, base64Key);
+                fail("Expected an exception for null session data");
+            } catch (NullPointerException | IllegalArgumentException e) {
+                // expected
+            }

🤖 Prompt for AI Agents

In `@src/test/java/io/mosip/mimoto/service/WalletPresentationServiceTest.java`
around lines 770 - 791, The test testStorePresentationRecordNullSessionData
currently swallows all exceptions in an empty catch; replace that try/catch with
an assertion that submitPresentation(nullSessionData, walletId, presentationId,
submitRequest, base64Key) throws the expected exception (e.g., use
assertThrows(IllegalArgumentException.class, ...) or the appropriate exception
type) so the test actually verifies failure on null session; keep the
SigningKeyUtil and UrlParameterUtils mocks but remove the empty catch and add
the assertion around walletPresentationService.submitPresentation to validate
the error path.

[swatigoel]

credentialSubject can be empty for ldp_vc?

[jackjain]

This DTO is used in processing the well-known response. There credential subject can be empty. It was fixed as part of bug - https://mosip.atlassian.net/browse/INJIWEB-1671

[swatigoel]

sd_jwt VP is not supported?

[jackjain]

yes, still pending to be implemented

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

api-test/src/main/java/io/inji/testrig/apirig/mimoto/utils/MimotoUtil.java (1)

411-411: ⚠️ Potential issue | 🟠 Major

Sensitive token logged in plaintext.

Line 411 logs the full id_token value. ID tokens contain user identity claims and should not be written to logs, as this can lead to token theft or PII exposure in log aggregation systems. Consider removing this log or redacting the token.

🔒 Proposed fix

-		logger.info("Obtained id_token: " + idToken); // Debug log
+		logger.info("Obtained id_token successfully"); // Debug log

api-test/src/main/java/io/inji/testrig/apirig/mimoto/testrunner/InjiTestRunner.java (1)

200-200: ⚠️ Potential issue | 🟡 Minor

Typo in system property name: testng.outpur.dir.

Should be testng.output.dir. This may be pre-existing, but worth fixing while you're refactoring this file.

🔤 Proposed fix

-				System.getProperties().setProperty("testng.outpur.dir", "testng-report");
+				System.getProperties().setProperty("testng.output.dir", "testng-report");

🤖 Fix all issues with AI agents

In `@api-test/pom.xml`:
- Around line 67-72: The dependency for
io.mosip.testrig.apitest.commons:apitest-commons is using a release version
1.4.0 but the POM only configures the Sonatype snapshots repository
(https://oss.sonatype.org/content/repositories/snapshots/) with no <releases>
element; either add a releases-capable repository entry in the POM's
<repositories> (or mirror/settings) that can serve non-snapshot artifacts,
ensuring it includes <releases><enabled>true</enabled></releases>, or change the
dependency version back to a snapshot (e.g., 1.4.0-SNAPSHOT) so Maven can
resolve it from the existing snapshots repository; update the <version> element
for the dependency or add the proper <repository> configuration accordingly.

In
`@api-test/src/main/java/io/inji/testrig/apirig/mimoto/utils/MimotoConfigManager.java`:
- Around line 40-42: The call in getMaxFailedAttemptsAllowedPerCycle currently
passes the full GitHub URL to MimotoUtil.getValueFromMimotoActuator which
prevents the String.contains() match in that method; update the section argument
in getMaxFailedAttemptsAllowedPerCycle to use the short path
"/mimoto-default.properties" (same style as other calls in MimotoUtil.java) so
the actuator property source name match succeeds and Integer.parseInt receives a
valid value.

🧹 Nitpick comments (2)

api-test/src/main/java/io/inji/testrig/apirig/mimoto/testrunner/InjiTestRunner.java (1)

266-290: generatePublicKeyForMimoto() is duplicated between InjiTestRunner and MimotoUtil.

This method has an identical implementation in MimotoUtil.java (Lines 300-324). Consider consolidating into a single location to avoid drift.

#!/bin/bash
# Verify the duplication
rg -n "generatePublicKeyForMimoto" --type=java

api-test/pom.xml (1)

64-64: Hardcoded version in fileName — consider using ${project.version} instead.

The version string 0.21.0-SNAPSHOT is duplicated here and in the <version> tag on line 11. If they ever diverge, the shaded JAR name will be wrong. This is a pre-existing pattern, but worth fixing now during the version bump.

♻️ Suggested fix

-		<fileName>apitest-mimoto-0.21.0-SNAPSHOT-jar-with-dependencies</fileName>
+		<fileName>apitest-mimoto-${project.version}-jar-with-dependencies</fileName>

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat api-test/pom.xml | head -100

Repository: inji/mimoto

Length of output: 3080

---

🏁 Script executed:

# Check for parent pom reference and search for any release repository definitions
rg -n "parent\|releases\|repository" api-test/pom.xml

Repository: inji/mimoto

Length of output: 37

---

🏁 Script executed:

wc -l api-test/pom.xml && echo "---" && cat api-test/pom.xml

Repository: inji/mimoto

Length of output: 8899

---

Add release repository configuration or keep snapshot dependency version.

The pom.xml configures only a snapshots repository (central.sonatype.com/repository/maven-snapshots/) with no <releases> configuration. Maven will fail to resolve the new release version 1.4.0 from this repository. Either:

1. Configure a release repository to resolve non-snapshot artifacts, or
2. Keep the dependency at a snapshot version (e.g., 1.4.0-SNAPSHOT if available)

🤖 Prompt for AI Agents

In `@api-test/pom.xml` around lines 67 - 72, The dependency for
io.mosip.testrig.apitest.commons:apitest-commons is using a release version
1.4.0 but the POM only configures the Sonatype snapshots repository
(https://oss.sonatype.org/content/repositories/snapshots/) with no <releases>
element; either add a releases-capable repository entry in the POM's
<repositories> (or mirror/settings) that can serve non-snapshot artifacts,
ensuring it includes <releases><enabled>true</enabled></releases>, or change the
dependency version back to a snapshot (e.g., 1.4.0-SNAPSHOT) so Maven can
resolve it from the existing snapshots repository; update the <version> element
for the dependency or add the proper <repository> configuration accordingly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for references to config URLs
echo "=== Searching for mosip-config references ==="
rg -n "mosip-config" --type=java

echo -e "\n=== Searching for inji-config references ==="
rg -n "inji-config" --type=java

echo -e "\n=== Searching for all mimoto-default.properties references ==="
rg -n "mimoto-default.properties" --type=java

echo -e "\n=== Searching for getValueFromMimotoActuator calls ==="
rg -n "getValueFromMimotoActuator" --type=java -A 2

Repository: inji/mimoto

Length of output: 6744

---

🏁 Script executed:

#!/bin/bash
# Look at the getValueFromMimotoActuator implementation
cat -n api-test/src/main/java/io/inji/testrig/apirig/mimoto/utils/MimotoUtil.java | sed -n '339,380p'

Repository: inji/mimoto

Length of output: 2104

---

🏁 Script executed:

#!/bin/bash
# Check if there are any other references or comments about actuator property source names
rg -n "propertySources\|property source" --type=java -B 2 -A 2

Repository: inji/mimoto

Length of output: 37

---

Change section parameter from full GitHub URL to short path to match other config calls.

Line 41 uses "https://github.com/inji/inji-config/mimoto-default.properties" as the section parameter, but the identical config calls in MimotoUtil.java (lines 59, 61) use "/mimoto-default.properties". The getValueFromMimotoActuator() method matches sections via String.contains() against Spring Boot actuator property source names (line 363), which won't contain the full GitHub URL. This will cause the match to fail, return null, and throw a NumberFormatException when parsing. Use "/mimoto-default.properties" instead.

🤖 Prompt for AI Agents

In
`@api-test/src/main/java/io/inji/testrig/apirig/mimoto/utils/MimotoConfigManager.java`
around lines 40 - 42, The call in getMaxFailedAttemptsAllowedPerCycle currently
passes the full GitHub URL to MimotoUtil.getValueFromMimotoActuator which
prevents the String.contains() match in that method; update the section argument
in getMaxFailedAttemptsAllowedPerCycle to use the short path
"/mimoto-default.properties" (same style as other calls in MimotoUtil.java) so
the actuator property source name match succeeds and Integer.parseInt receives a
valid value.

[swatigoel]

create a card to upgrade in next release as new version will be published to io.inji

[jackjain]

Created the card for it - https://mosip.atlassian.net/browse/INJIWEB-1849

[swatigoel]

create a card to upgrade in next release as new version will be published to io.inji

[jackjain]

Created the card for it - https://mosip.atlassian.net/browse/INJIWEB-1849

[jackjain]

@coderabbitai ignore

[coderabbitai]

✅ Actions performed

Reviews paused.