[INJICERT-#902]: Integrate inji-verify as library in certify #928
Merged
swatigoel merged 11 commits intoJul 6, 2026
Conversation
Signed-off-by: Sakshiijk <sakshi.jhanwar@infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
…elease-1.0.x Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
WalkthroughCertify service now embeds inji-verify for VP request and presentation processing, updates Spring wiring and configuration, and adds supporting database schema plus startup scripts for the new verification tables. ChangesEmbedded inji-verify integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Wallet
participant IarPresentationService
participant VerifiablePresentationSubmissionService
participant VerifiablePresentationRequestService
participant IarSessionRepository
Wallet->>IarPresentationService: submit VP token
IarPresentationService->>IarPresentationService: parse vp_token and presentation_submission
IarPresentationService->>VerifiablePresentationSubmissionService: submit(requestId, transactionId, vpData)
VerifiablePresentationSubmissionService-->>IarPresentationService: submission response
IarPresentationService->>VerifiablePresentationRequestService: getLatestRequestIds(transactionId)
VerifiablePresentationRequestService-->>IarPresentationService: request ids
IarPresentationService->>VerifiablePresentationSubmissionService: getVPResultV2(...)
VerifiablePresentationSubmissionService-->>IarPresentationService: VPVerificationResultDto
IarPresentationService->>IarSessionRepository: store authorization code
IarPresentationService-->>Wallet: authorization response
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
certify-service/src/main/java/io/mosip/certify/services/IarVpRequestService.java (1)
194-205: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
validateConfiguration()throws uncheckedIllegalStateException, breaking the method'sCertifyExceptioncontract.
createVpRequest(...)declaresthrows CertifyException, butvalidateConfiguration()(called at line 78, before the try/catch blocks) throwsIllegalStateExceptionon missing config. This propagates as an unhandled runtime exception rather than a structuredCertifyException, likely surfacing as a generic 500 to callers instead of the API's expected error contract.Per prior guidance on this codebase, configuration validation is best performed once at application startup rather than per-request; if runtime validation must remain here, at minimum wrap it in
CertifyExceptionfor a consistent error contract.🛠️ Suggested fix
private void validateConfiguration() { if (!StringUtils.hasText(vpRequestConfigUrl)) { - throw new IllegalStateException("mosip.certify.vp-request.config-file-url must be configured"); + throw new CertifyException("unknown_error", "mosip.certify.vp-request.config-file-url must be configured"); } if (!StringUtils.hasText(verifierClientId)) { - throw new IllegalStateException("mosip.certify.verify.service.verifier-client-id must be configured"); + throw new CertifyException("unknown_error", "mosip.certify.verify.service.verifier-client-id must be configured"); } if (!StringUtils.hasText(certifyIaeEndpoint)) { - throw new IllegalStateException("mosip.certify.oauth.interactive-authorization-endpoint must be configured"); + throw new CertifyException("unknown_error", "mosip.certify.oauth.interactive-authorization-endpoint must be configured"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@certify-service/src/main/java/io/mosip/certify/services/IarVpRequestService.java` around lines 194 - 205, validateConfiguration() in IarVpRequestService currently throws IllegalStateException, which bypasses createVpRequest(...)’s CertifyException contract and can surface as an unhandled runtime error. Move this configuration check to startup if possible; otherwise, update validateConfiguration() (or its call site in createVpRequest()) to catch missing-config failures and rethrow a CertifyException with the same messages so the service returns a consistent error type.Source: Learnings
🧹 Nitpick comments (3)
certify-service/pom.xml (1)
29-41: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueBroadly-enabled snapshot repository widens supply-chain surface.
The new
ossrh-injirepository enables snapshots without restricting resolution to a specific groupId, so any transitive/direct dependency declared as a-SNAPSHOTcould resolve through this repo, not justio.inji.verify. Combined with the pre-existingjitpack.iorepo, this increases the trusted-artifact surface for the build.Consider scoping the repository to the
io.inji*groupId (e.g., via a repository manager mirror/content filter) once your build tooling supports it, to reduce accidental snapshot resolution from unrelated coordinates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@certify-service/pom.xml` around lines 29 - 41, The new ossrh-inji entry in the repository section is too broad because snapshots can resolve for any -SNAPSHOT dependency, not just the intended io.inji.verify artifacts. Update the Maven repository configuration in the repository block to scope snapshot access to the io.inji* groupId using the build tool’s supported content filter or a repository manager mirror. Keep the change localized to the repository definitions so only the ossrh-inji repository behavior is tightened.db_upgrade_script/inji_certify/sql/0.14.0_to_1.0.0_upgrade.sql (1)
28-97: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffNo enforced FK constraints between the new verify tables despite documented relationships.
vp_submission.request_idis commented as a foreign key toauthorization_request_details.request_id(line 87), and bothauthorization_request_details.transaction_idandvc_submission.transaction_idare described as linking to the issuance session — but none of these are backed by actualFOREIGN KEYconstraints. This allows orphaned rows if either side of the relationship is deleted independently.Given this schema is likely mirrored from the embedded inji-verify library's own JPA entities, confirm whether the library manages these relationships itself (e.g., via application-level cleanup) before adding constraints that could conflict with its expected DDL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@db_upgrade_script/inji_certify/sql/0.14.0_to_1.0.0_upgrade.sql` around lines 28 - 97, The new verify tables document relationships that are not enforced by the schema, so add the missing foreign key constraints in the upgrade script for certify.vp_submission, certify.authorization_request_details, and certify.vc_submission where appropriate. Use the existing table definitions and column names in the script to wire vp_submission.request_id to authorization_request_details.request_id, and verify whether transaction_id should also reference the issuance-session table before adding constraints that could conflict with the embedded inji-verify library’s expected DDL. Keep the current indexes and comments, but make the actual relational links explicit in the CREATE TABLE or follow-up ALTER statements.docker-compose/docker-compose-injistack/config/certify-default.properties (1)
245-245: 🚀 Performance & Scalability | 🔵 TrivialLong-polling in the embedded library has a documented multi-pod scaling limitation.
Per inji-verify's own release notes, Long-polling listeners are implemented within the service layer, preventing the backend from scaling effectively in a multi-pod (distributed) environment. Since this PR embeds the library directly into certify-service (rather than running it as an independently-scaled service), this limitation now directly constrains certify-service's own horizontal scalability for VP flows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/docker-compose-injistack/config/certify-default.properties` at line 245, The inji.vp-request.long-polling-timeout setting introduces a backend long-polling dependency that limits horizontal scaling in multi-pod deployments. Remove this property from certify-service’s embedded configuration or replace it with a non-long-polling VP flow setting, and update the related certify-default.properties entry so the service no longer relies on the long-polling listener behavior from the embedded library.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@certify-service/pom.xml`:
- Around line 252-263: The dependency on verify-service is pinned to a SNAPSHOT,
which makes builds non-reproducible over time. Update the verify-service entry
in the pom dependency block to a stable release version instead of
0.18.2-SNAPSHOT, while keeping the existing pixelpass-jar exclusion unchanged;
if a release is not available yet, add an explicit fallback strategy for
snapshot unavailability before merging.
In `@docker-compose/docker-compose-injistack/config/certify-default.properties`:
- Around line 244-250: The shared docker-compose injistack config is using a
checked-in test keystore and password for VP signing/verification, which should
not be hardcoded in a network-facing deployment. Update the
inji.verify.keystore.file.path and inji.verify.keystore.file.pass entries in
certify-default.properties to read from environment-injected values or Docker
secrets, using the existing inji-verify property keys so the compose stack can
supply a per-environment keystore instead of classpath:sample-keystore/test.p12
and mosip. If this stack is intentionally ephemeral, add clear documentation
near these properties to state that it is non-production only.
---
Outside diff comments:
In
`@certify-service/src/main/java/io/mosip/certify/services/IarVpRequestService.java`:
- Around line 194-205: validateConfiguration() in IarVpRequestService currently
throws IllegalStateException, which bypasses createVpRequest(...)’s
CertifyException contract and can surface as an unhandled runtime error. Move
this configuration check to startup if possible; otherwise, update
validateConfiguration() (or its call site in createVpRequest()) to catch
missing-config failures and rethrow a CertifyException with the same messages so
the service returns a consistent error type.
---
Nitpick comments:
In `@certify-service/pom.xml`:
- Around line 29-41: The new ossrh-inji entry in the repository section is too
broad because snapshots can resolve for any -SNAPSHOT dependency, not just the
intended io.inji.verify artifacts. Update the Maven repository configuration in
the repository block to scope snapshot access to the io.inji* groupId using the
build tool’s supported content filter or a repository manager mirror. Keep the
change localized to the repository definitions so only the ossrh-inji repository
behavior is tightened.
In `@db_upgrade_script/inji_certify/sql/0.14.0_to_1.0.0_upgrade.sql`:
- Around line 28-97: The new verify tables document relationships that are not
enforced by the schema, so add the missing foreign key constraints in the
upgrade script for certify.vp_submission, certify.authorization_request_details,
and certify.vc_submission where appropriate. Use the existing table definitions
and column names in the script to wire vp_submission.request_id to
authorization_request_details.request_id, and verify whether transaction_id
should also reference the issuance-session table before adding constraints that
could conflict with the embedded inji-verify library’s expected DDL. Keep the
current indexes and comments, but make the actual relational links explicit in
the CREATE TABLE or follow-up ALTER statements.
In `@docker-compose/docker-compose-injistack/config/certify-default.properties`:
- Line 245: The inji.vp-request.long-polling-timeout setting introduces a
backend long-polling dependency that limits horizontal scaling in multi-pod
deployments. Remove this property from certify-service’s embedded configuration
or replace it with a non-long-polling VP flow setting, and update the related
certify-default.properties entry so the service no longer relies on the
long-polling listener behavior from the embedded library.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: faaecf4d-d353-4b63-9e5d-504680ab6807
📒 Files selected for processing (20)
certify-service/pom.xmlcertify-service/src/main/java/io/mosip/certify/CertifyServiceApplication.javacertify-service/src/main/java/io/mosip/certify/config/AppConfig.javacertify-service/src/main/java/io/mosip/certify/config/CertifyContextPathOverride.javacertify-service/src/main/java/io/mosip/certify/services/CertifyIssuanceServiceImpl.javacertify-service/src/main/java/io/mosip/certify/services/IarPresentationService.javacertify-service/src/main/java/io/mosip/certify/services/IarVpRequestService.javacertify-service/src/main/resources/application-local.propertiescertify-service/src/main/resources/sample-keystore/test.p12certify-service/src/test/resources/application-test.propertiesdb_scripts/inji_certify/ddl.sqldb_scripts/inji_certify/ddl/verify-authorization_request_details.sqldb_scripts/inji_certify/ddl/verify-presentation_definition.sqldb_scripts/inji_certify/ddl/verify-vc_submission.sqldb_scripts/inji_certify/ddl/verify-vp_submission.sqldb_upgrade_script/inji_certify/sql/0.14.0_to_1.0.0_rollback.sqldb_upgrade_script/inji_certify/sql/0.14.0_to_1.0.0_upgrade.sqldocker-compose/docker-compose-injistack/certify_init.sqldocker-compose/docker-compose-injistack/config/certify-default.propertiesdocker-compose/docker-compose-injistack/docker-compose.yaml
💤 Files with no reviewable changes (1)
- docker-compose/docker-compose-injistack/docker-compose.yaml
Signed-off-by: Sakshiijk <sakshi.jhanwar@ad.infosys.com>
Sakshiijk
changed the title
swatigoel
approved these changes
Jul 6, 2026
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#902
Summary by CodeRabbit
/).