Skip to content

Conversation

@naftis
Copy link
Member

@naftis naftis commented Oct 22, 2025

Note

Currently, we do not run e2e tests as a check on opencrvs-countryconfig-repo PRs. Please ensure your PR doesn't break any e2e tests.

One method for doing this is to open a PR with these changes to opencrvs-farajaland as well, and see if the PR check passes there.

Description

Bumps mosip-api to 1.9.0-beta.4 and fetch NID & id type

Checklist

  • I have linked the correct Github issue under "Development"
  • I have tested the changes locally, and written appropriate tests
  • I have tested beyond the happy path (e.g. edge cases, failure paths)
  • I have updated the changelog with this change (if applicable)
  • I have updated the GitHub issue status accordingly

euanmillar and others added 30 commits February 6, 2025 08:59
Only use mock servers on QA environment
Merge develop and update email address aliases
* feat: add configuration to implement logic before forwarding to mosip for id creation

* chore: bump mosip package version

* fix: amends to verify approach

* fix: minimize changes

* fix: add logs

* fix: minor improvements
Added vadym mudryi to development and e2e servers
naftis and others added 28 commits October 6, 2025 15:28
* feat: mosip interop client for events v2

* compose the mosip payload

* don't await mosip

* initial dumb verification to assert the action input working

* initial verification api

* (wip) use the future state of the declaration

* verify mother in v2

* pull changes from Farajaland

* update toolkit

* update mosip toolkit

* fix tests and send notification
* add new form fields

* fix toolkit version

* add initial http fetch

* chore: extend configuration

* fix: more improvements

* fix: remove redundant code

* feat: loader

* feat: esignet configurations

* update toolkit & mosip api

* fix: use helper methods to integrate with MOSIP

* fix: do not verify if authenticated

* fix: more improvements

* fix: config forms to integrate mosip fields

* fix: improve util interface

* make v2 events default

* chore: configure multi-parent to register fields for both qr and esignet

* chore: update toolkit version

* fix: update configs to make multi-parent work

* fix: father form page configuration

* fix: esignet form population bugs

---------

Co-authored-by: tahmidrahman-dsi <[email protected]>
* add development time yarn db:clear:all script for easy analytics data clearing

* fix indent

* add translation

* fix street level address validation

* add ids to data config fields

* drop analytics database as part of data clearing

* cleanup

* fix: death informant phone number error message

* add 'ConditionalType.DISPLAY_ON_REVIEW'

* use new conditional

* use ConditionalType.DISPLAY_ON_REVIEW

* missing translation added

* fix

* fix: remove print in advance from death form of print cert (opencrvs#1071)

* fix: remove print in advance from death form of print cert

* fix: remove print in advancefor other collector type

* update metabase dump

* fix: dont show birth certificate after first print

* [OCRVS-10730] Remove 'Completeness Rates Dashboard'/ 'Vital Statistics Export' links in registration dashboard

* [OCRVS-10729] Metabase copy cleanup

* [OCRVS-10722] Remove Jurisdiction filter from Registrations dashboards

* chore: update toolkit version

* feat: use the new age field in birth form

* chore: upgrade toolkit version

* feat: use new AGE field

* [OCRVS-10729] Remove location filter value completely from registrations dashboards title

* Ensure docker daemon does not depend on cryptfs logic on workers. Data directories and cryptfs should only exist on manager node

* fix: Place of death in advanced search

* fix(analytics): clear existing event_actions before upsert and mark last action as accepted on immediate confirmation (opencrvs#1094)

* feat: add validations to age field

* chore: upgrade toolkit version

* Metabase: add country wide completeness rate

* Metabase: update dashboards

* Metabase: update readme

* update toolkit

* update toolkit

---------

Co-authored-by: Riku Rouvila <[email protected]>
Co-authored-by: cibelius <[email protected]>
Co-authored-by: tareq89 <[email protected]>
Co-authored-by: jamil314 <[email protected]>
Co-authored-by: Md. Ashikul Alam <[email protected]>
Co-authored-by: Tameem Bin Haider <[email protected]>
Co-authored-by: Tameem Bin Haider <[email protected]>
Co-authored-by: naftis <[email protected]>
fix: hidden fields should not receive a value
* fix: backend doesnt get the full http payload

* dont display http or query params on review
…mosip into fix/query-param-reader-removing-review-params
…mosip into fix/query-param-reader-removing-review-params
…review-params

[OCRVS-10759] Fix query param reader removing review params
@naftis naftis closed this Oct 22, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR integrates MOSIP (Modular Open Source Identity Platform) API version 1.9.0-beta.4 to enable national identity verification in OpenCRVS birth and death registration workflows. The changes introduce QR code scanning, eSignet OAuth authentication, and HTTP-based identity verification that auto-populate form fields with verified demographic data. New form helpers (connectToMOSIPIdReader, connectToMOSIPVerificationStatus, getMOSIPIntegrationFields) wrap existing person detail fields across all event types, while infrastructure updates add SQLite data storage, certificate volume mounts, and three new services (mosip-api, mosip-mock, esignet-mock) to the deployment stack. The EXTERNAL_VALIDATION_WORKQUEUE feature flag is enabled, routing declarations through MOSIP verification before registration. The implementation refactors ID-related field generation from scattered modules into common-custom-fields.ts, updates FHIR types from fhir.* to fhir3.* namespace, adds environment variables for OAuth endpoints across all deployment configurations, and extends backup scripts to handle the new SQLite database.

Changed Files
Filename Score Overview
src/form/v2/mosip.ts 4/5 New file implementing MOSIP integration helpers for QR scanning and eSignet authentication with conditional field orchestration
src/index.ts 2/5 Refactors routing to add event-specific action handlers but introduces route precedence conflict between generic and MOSIP-specific paths
src/api/registration/index.ts 2/5 Adds MOSIP registration handlers with unawaited async calls and @TODO placeholders in notification fields
src/api/custom-event/handler.ts 3/5 Introduces NID verification handlers for birth/death events with inconsistent parameter usage across verifyNid calls
src/utils/fhir.ts 3/5 New FHIR utility module with extensive non-null assertions that could fail if bundle structure doesn't match expectations
infrastructure/docker-compose.deploy.yml 4/5 Adds mosip-api service with comprehensive environment configuration and SQLite volume mount
infrastructure/docker-compose.qa-deploy.yml 3.5/5 Adds MOSIP mock services and eSignet OAuth provider with port mismatch concerns and missing resource limits
infrastructure/docker-compose.staging-deploy.yml 2/5 Restructures config with unauthenticated MONGO_URL and incorrect certificate file extension
src/form/v2/death/forms/pages/informant.ts 3/5 Integrates MOSIP ID reader with potential issues if getMOSIPIntegrationFields returns empty array
src/form/v2/death/forms/pages/spouse.ts 3/5 Wraps fields with MOSIP connectors but hidden fields still referenced in conditional logic may break validation
src/form/common/common-custom-fields.ts 3/5 New shared ID field generators with logical flaw in NATIONAL_ID pre-selection and redundant NONE conditional
src/form/common/id-reader-configurations.ts 3/5 Centralized QR validation and eSignet config with unused format parameter and brittle fixed schema assumptions
src/form/custom-fields.ts 3/5 Drastically simplified by removing ~180 lines of ID-related logic moved to common-custom-fields
src/api/tracking-id/handler.ts 0/5 References fhir3 namespace without import statement, will cause ReferenceError at runtime
src/form/death/index.ts 0/5 File marked with score 0 but no explanation provided in diff
src/form/v2/death/forms/pages/deceased.ts 0/5 File marked with score 0 but no explanation provided in diff
src/form/v2/birth/forms/pages/informant.ts 0/5 File marked with score 0 but no explanation provided in diff
src/form/v2/birth/forms/pages/mother.ts 0/5 File marked with score 0 but no explanation provided in diff
src/form/v2/birth/forms/pages/father.ts 0/5 File marked with score 0 but no explanation provided in diff
src/form/birth/index.ts 0/5 File marked with score 0 but no explanation provided in diff
CHANGELOG.md 2/5 Adds 1.9.0 section but contains duplicate version entries and missing MOSIP-related changelog updates
package.json 4/5 Adds @opencrvs/mosip 1.9.0-beta.4 dependency and updates toolkit and FHIR types with beta package in production dependencies
infrastructure/clear-all-data.sh 3/5 Adds SQLite cleanup without defensive error handling that other data store cleanups have
infrastructure/backups/backup.sh 4/5 Adds SQLite backup functionality following existing PostgreSQL pattern
infrastructure/server-setup/tasks/data-partition.yml 5/5 Creates SQLite data and backup directories following established infrastructure patterns
infrastructure/server-setup/inventory/qa.yml 4/5 Updates QA inventory with real team SSH keys and MOSIP development server details
infrastructure/server-setup/inventory/staging.yml 4/5 Migrates staging inventory from placeholders to concrete MOSIP collaboration environment
infrastructure/known-hosts 4/5 Adds SSH host keys for MOSIP QA infrastructure with potential duplication for qa-mosip-collab.opencrvs.dev
src/constants.ts 4/5 Adds five eSignet/MOSIP environment-sourced constants without validation or defaults
src/environment.ts 4/5 Adds MOSIP/eSignet environment variables with appropriate dev defaults and descriptions
infrastructure/docker-compose.development-deploy.yml 4/5 Adds MOSIP environment variables with inconsistent default operator usage
src/form/common/common-required-fields.ts 4/5 Adds initialValue parameters to enable dynamic field pre-population from MOSIP
src/form/common/default-validation-conditionals.ts 4/5 Extends conditionals to handle QR reader and verified/authenticated states for MOSIP integration
renovate.json 4/5 Simplifies config to only allow MOSIP package updates while blocking all other dependencies
src/utils/index.ts 4/5 Updates FHIR type references from fhir.* to fhir3.* namespace for explicit FHIR R3 typing
src/utils/mapping/section/birth/mapping-utils.ts 4/5 Adds childIdentifier mapping for NATIONAL_ID but excludes it from mutation parameters
src/form/birth/certificate-handlebars.ts 4/5 Adds childIdentifier variable for displaying child NID on certificates
src/api/application/application-config.ts 4/5 Enables EXTERNAL_VALIDATION_WORKQUEUE feature flag for MOSIP validation integration
src/api/tracking-id/service.ts 5/5 Updates all FHIR type references to fhir3 namespace for compatibility with mosip-api upgrade
src/api/event-registration/handler.ts 4/5 Updates FHIR Bundle type to fhir3.Bundle with missing import verification needed
src/api/event-registration/service.ts 5/5 Type-only change updating fhir.Bundle to fhir3.Bundle for explicit FHIR R3 typing
src/form/marriage/index.ts 5/5 Updates import path for ID field helpers to consolidated common module
src/api/notification/notification.informant.sms.test.ts 5/5 Adds MOSIP interop client mock to SMS notification test suite
src/api/notification/notification.informant.email.test.ts 5/5 Adds MOSIP interop client mock to email notification test suite
.eslintrc.js 5/5 Removes redundant no-undef rule for TypeScript projects
src/form/v2/birth/forms/pages/child.ts 5/5 Adds hidden child.nid field for storing MOSIP-provided National ID data

Confidence score: 2/5

  • This PR contains critical issues that will cause runtime failures if merged, particularly the missing fhir3 import in handler.ts, route precedence conflicts preventing MOSIP integration, and unawaited async operations in registration handlers
  • Score reflects multiple high-severity issues: missing TypeScript imports, unauthenticated MongoDB connection string, @TODO placeholders in production code, route conflicts, inconsistent API parameter usage, and extensive non-null assertions without defensive checks
  • Pay very close attention to src/api/tracking-id/handler.ts (missing import), src/index.ts (route conflict), src/api/registration/index.ts (unawaited calls and TODOs), infrastructure/docker-compose.staging-deploy.yml (unauthenticated MONGO_URL), and src/utils/fhir.ts (unsafe non-null assertions)

Sequence Diagram

sequenceDiagram
    participant User
    participant Client
    participant CountryConfig
    participant MOSIPInterop
    participant ESignet
    participant MOSIP

    User->>Client: Start Birth/Death Registration
    Client->>CountryConfig: GET /forms
    CountryConfig-->>Client: Form configuration with MOSIP fields
    
    alt QR Code Scan
        User->>Client: Scan QR Code
        Client->>Client: Extract NID & demographic data
        Client->>Client: Populate form fields
    else E-Signet Authentication
        User->>Client: Click "Authenticate with e-Signet"
        Client->>ESignet: Redirect to authorization
        ESignet->>User: Request biometric authentication
        User->>ESignet: Provide authentication
        ESignet-->>Client: Return authorization code
        Client->>CountryConfig: POST /trigger/events/{event}/actions/{action}
        CountryConfig->>MOSIPInterop: POST /esignet/get-oidp-user-info
        MOSIPInterop->>ESignet: Fetch user information
        ESignet-->>MOSIPInterop: User data (name, DOB, NID, gender)
        MOSIPInterop-->>CountryConfig: User information
        CountryConfig->>CountryConfig: Set verification status = "authenticated"
        CountryConfig-->>Client: Populated data + verification status
        Client->>Client: Disable authenticated fields
    end

    User->>Client: Complete form & submit
    Client->>CountryConfig: POST /trigger/events/{event}/actions/REGISTER
    
    alt Birth Registration with MOSIP
        CountryConfig->>CountryConfig: Check custom logic for MOSIP forwarding
        CountryConfig->>MOSIPInterop: POST /register (Birth)
        MOSIPInterop->>MOSIP: Create packet with NID & demographic data
        MOSIP-->>MOSIPInterop: Packet created
        MOSIPInterop-->>CountryConfig: HTTP 202 Accepted
        CountryConfig-->>Client: Registration deferred
    else Death Registration with MOSIP
        CountryConfig->>CountryConfig: Check custom logic for MOSIP forwarding
        CountryConfig->>MOSIPInterop: POST /register (Death)
        MOSIPInterop->>MOSIP: Deactivate identity + create packet
        MOSIP-->>MOSIPInterop: Identity deactivated
        MOSIPInterop-->>CountryConfig: HTTP 202 Accepted
        CountryConfig-->>Client: Registration deferred
    end

    MOSIP->>MOSIPInterop: Webhook notification (async)
    MOSIPInterop->>CountryConfig: Webhook callback
    CountryConfig->>CountryConfig: Update verification status
Loading

Additional Comments (4)

  1. src/index.ts, line 647-685 (link)

    logic: Route precedence issue: the generic /trigger/events/birth/actions/{action} route (line 649) will match /trigger/events/birth/actions/REGISTER before the specific route on line 669. Hapi uses first-match routing, so the specific MOSIP handler may never execute for birth REGISTER actions.

  2. CHANGELOG.md, line 34-56 (link)

    logic: Duplicate changelog entries for version1.8.0. The same content appears at lines 34-56 and was also added at lines 40-56 before the existing 1.6.4 section.

  3. CHANGELOG.md, line 57-68 (link)

    logic: Version 1.6.4 appears twice with different content - once at line 57 and again at line 272. This creates ambiguity about what changes belong to 1.6.4.

  4. CHANGELOG.md, line 64-65 (link)

    logic: Lines 64-65 duplicate lines 54-55 (build summary and ARM device support improvements). These improvements are already listed in the 1.8.0 section above.

46 files reviewed, 26 comments

Edit Code Review Agent Settings | Greptile

h: Hapi.ResponseToolkit
) {
const trackingId = generateTrackingId(request.payload as fhir.Bundle)
const trackingId = generateTrackingId(request.payload as fhir3.Bundle)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Type cast to fhir3.Bundle but fhir3 is not imported. This will cause a runtime reference error.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/tracking-id/handler.ts
Line: 8:8

Comment:
**logic:** Type cast to `fhir3.Bundle` but `fhir3` is not imported. This will cause a runtime reference error.

How can I resolve this? If you propose a fix, please make it concise.

function objectHasProperty(
property: string,
type: 'string' | 'number' | 'boolean' | 'array' | 'object',
format?: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: format parameter declared but never used in returned object - should be added conditionally to avoid undefined format values in schema

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/form/common/id-reader-configurations.ts
Line: 23:23

Comment:
**logic:** format parameter declared but never used in returned object - should be added conditionally to avoid undefined format values in schema

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +83 to +87
{
fieldName: 'childIdentifier',
operation: 'childIdentityToFieldTransformer',
parameters: [['NATIONAL_ID']]
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The new childIdentifier mapping is read-only in queries but not included in the mutation parameters (lines 105-112). This means NATIONAL_ID will be fetched and displayed but never saved back to FHIR when the form is submitted. Should NATIONAL_ID be persisted during mutations, or is it intentionally read-only because it comes from an external system like MOSIP?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/utils/mapping/section/birth/mapping-utils.ts
Line: 83:87

Comment:
**logic:** The new `childIdentifier` mapping is read-only in queries but not included in the mutation parameters (lines 105-112). This means NATIONAL_ID will be fetched and displayed but never saved back to FHIR when the form is submitted. Should NATIONAL_ID be persisted during mutations, or is it intentionally read-only because it comes from an external system like MOSIP?

How can I resolve this? If you propose a fix, please make it concise.

"enabled": false
},
{
"matchPackagePatterns": ["^@opencrvs/mosip$"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The package pattern ^@opencrvs/mosip$ uses both regex start anchor (^) and end anchor ($). Since matchPackagePatterns expects regex, this will match exactly @opencrvs/mosip, but using matchPackageNames with an exact string would be clearer.

Prompt To Fix With AI
This is a comment left during a code review.
Path: renovate.json
Line: 11:11

Comment:
**style:** The package pattern `^@opencrvs/mosip$` uses both regex start anchor (^) and end anchor ($). Since `matchPackagePatterns` expects regex, this will match exactly `@opencrvs/mosip`, but using `matchPackageNames` with an exact string would be clearer.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +161 to +162
docker run --rm -v /data/sqlite:/data/sqlite alpine \
sh -c "apk add --no-cache sqlite && sqlite3 /data/sqlite/mosip-api.db 'DELETE FROM transactions;'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The container installs sqlite every time this script runs, which is inefficient. Consider using a base image that already has sqlite installed, or cache the installation if this script is run frequently.

Prompt To Fix With AI
This is a comment left during a code review.
Path: infrastructure/clear-all-data.sh
Line: 161:162

Comment:
**style:** The container installs `sqlite` every time this script runs, which is inefficient. Consider using a base image that already has `sqlite` installed, or cache the installation if this script is run frequently.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +148 to +153
updatedFields['deceased.verified'] = await mosipInteropClient.verifyNid({
dob: declaration['deceased.dob'],
nid: declaration['deceased.nid'],
name: declaration['deceased.name'],
gender: declaration['deceased.gender']
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Missing transactionId parameter in deceased verification, while mother (line 81) and father (line 95) include it. Should all verifyNid calls include transactionId for consistency and traceability?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/custom-event/handler.ts
Line: 148:153

Comment:
**style:** Missing `transactionId` parameter in deceased verification, while mother (line 81) and father (line 95) include it. Should all verifyNid calls include transactionId for consistency and traceability?

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +164 to +168
updatedFields['informant.verified'] = await mosipInteropClient.verifyNid({
dob: declaration['informant.dob'],
nid: declaration['informant.nid'],
name: declaration['informant.name']
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Missing both gender and transactionId parameters

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/custom-event/handler.ts
Line: 164:168

Comment:
**style:** Missing both `gender` and `transactionId` parameters

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +176 to +180
updatedFields['spouse.verified'] = await mosipInteropClient.verifyNid({
dob: declaration['spouse.dob'],
nid: declaration['spouse.nid'],
name: declaration['spouse.name']
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Missing both gender and transactionId parameters

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/api/custom-event/handler.ts
Line: 176:180

Comment:
**style:** Missing both `gender` and `transactionId` parameters

How can I resolve this? If you propose a fix, please make it concise.

- APPLICATION_CONFIG_URL=http://config:2021
- CONFIRM_REGISTRATION_URL=http://workflow:5050/confirm/registration
- CHECK_INVALID_TOKEN=true
- MONGO_URL=mongodb://mongo1/user-mgnt?replicaSet=rs0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: MONGO_URL is missing authentication credentials (compare with line 44: user-mgnt:${USER_MGNT_MONGODB_PASSWORD}@mongo1). This will likely fail to connect to the secured MongoDB instance.

Prompt To Fix With AI
This is a comment left during a code review.
Path: infrastructure/docker-compose.staging-deploy.yml
Line: 163:163

Comment:
**logic:** MONGO_URL is missing authentication credentials (compare with line 44: `user-mgnt:${USER_MGNT_MONGODB_PASSWORD}@mongo1`). This will likely fail to connect to the secured MongoDB instance.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +186 to +187
- CREDENTIAL_PARTNER_CERTIFICATE_PATH=/certs/credential-partner.csr
- CREDENTIAL_PARTNER_PRIVATE_KEY_PATH=/certs/credential-partner.pem
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Using .csr extension for certificate path is incorrect—CSRs are certificate signing requests, not certificates. Should this be .crt or .pem? Was .csr intended here, or should this be the actual certificate file (typically .crt or .pem)?

Prompt To Fix With AI
This is a comment left during a code review.
Path: infrastructure/docker-compose.staging-deploy.yml
Line: 186:187

Comment:
**logic:** Using `.csr` extension for certificate path is incorrect—CSRs are certificate signing *requests*, not certificates. Should this be `.crt` or `.pem`? Was `.csr` intended here, or should this be the actual certificate file (typically `.crt` or `.pem`)?

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.