Release 0.8.x Testing coderabbit on develop/ #458
Conversation
…ng ES256 algorithm in eSignet (#7) * [INJICERT-42] upgraded sunbird images Signed-off-by: Challarao <challarao@beehyv.com> * [INJICERT-42] added detailed steps for generating DID in README.md Signed-off-by: Challarao <challarao@beehyv.com> * [INJICERT-42] added steps for hosting multiple DIDs in a single git repository Signed-off-by: Challarao <challarao@beehyv.com>
Signed-off-by: Harsh Vardhan <harsh59v@gmail.com>
[INJICERT-14] update esignet config to release branch
Signed-off-by: Harsh Vardhan <harsh59v@gmail.com>
[INJICERT-14] update esignet config to include KBA
WalkthroughDocumentation updates to the installation flow in README. Configuration changes for Esignet properties. Sunbird stack updates: environment variables, QR type, signing algorithm, and service images bumped to v2.0.0-rc3. Added WEB_DID_BASE_URL across docs/env. Expanded supported JWT proof algorithms to include ES256 and updated issuer/schema IDs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose-sunbird/.env (1)
2-9: Consolidate duplicate env var definitions in docker-compose-sunbird/.env
Multiple keys are declared more than once, so only the last value takes effect. Rename or remove duplicates and update references in docker-compose:
- VAULT_ADDR (lines 2 & 7): rename second occurrence (e.g. VAULT_ADDR_INTERNAL) and adjust service env.
- JWKS_URI: unify into a single definition or namespace per service (e.g. JWKS_URI_PUBLIC, JWKS_URI_INTERNAL).
- ENABLE_AUTH: remove the redundant entry or scope it (e.g. ENABLE_AUTH_IDENTITY).
- IDENTITY_BASE_URL: consolidate or rename duplicates to avoid conflicts.
Optional cleanup: remove unused aliases (VAULT_API_ADDR, VAULT_ADDRESS).
🧹 Nitpick comments (12)
docker-compose-sunbird/.env (2)
16-16: Avoid committing a personal domain as defaultWEB_DID_BASE_URL points to a user-specific GitHub Pages site. Consider defaulting to a neutral placeholder (e.g., https://example.org/DID-Resolve) and rely on README to instruct overrides.
Example:-WEB_DID_BASE_URL=https://challabeehyv.github.io/DID-Resolve +WEB_DID_BASE_URL=https://example.org/DID-Resolve
13-16: Key order warnings are non-functional but easy to tidydotenv-linter flagged ordering around SIGNING_ALGORITHM/JWKS_URI/ENABLE_AUTH. Not required, but reordering improves readability.
docker-compose-esignet/config/esignet-default.properties (1)
137-147: Don’t hardcode issuer DID and schema IDs; parameterize for deploymentsThe issuerId and cred-schema-id are repo-specific and will vary per environment. Recommend using env-backed placeholders to simplify setup and reduce accidental misuse.
Example:-mosip.esignet.vciplugin.sunbird-rc.credential-type.HealthInsuranceCredential.static-value-map.issuerId=did:web:challabeehyv.github.io:DID-Resolve:3313e611-d08a-49c8-b478-7f55eafe62f2 +mosip.esignet.vciplugin.sunbird-rc.credential-type.HealthInsuranceCredential.static-value-map.issuerId=${ESIGNET_ISSUER_DID:CHANGE_ME} -mosip.esignet.vciplugin.sunbird-rc.credential-type.HealthInsuranceCredential.cred-schema-id=did:schema:dc000a57-e889-4347-b6c1-710d1ec8b31a +mosip.esignet.vciplugin.sunbird-rc.credential-type.HealthInsuranceCredential.cred-schema-id=${ESIGNET_SCHEMA_ID:CHANGE_ME}Repeat for LifeInsuranceCredential and InsuranceCredential. Document these in README. Based on learnings.
Also applies to: 152-156
docker-compose-sunbird/docker-compose.yml (2)
31-31: Image bump to v2.0.0-rc3—pin or verify compatibilityRC tags can shift. For stability, pin digests or confirm no breaking env var/name changes (e.g., SIGNING_ALGORITHM, WEB_DID_BASE_URL handling).
Optionally pin:-image: ghcr.io/sunbird-rc/sunbird-rc-identity-service:v2.0.0-rc3 +image: ghcr.io/sunbird-rc/sunbird-rc-identity-service@sha256:... # digest of rc3Also applies to: 60-60, 82-82
99-99: Confirm QR_TYPE value and document itEnsure the credentials service recognizes QR_TYPE=W3C_VC (exact literal). If configurable, consider exposing via .env and mention in README.
- - QR_TYPE=W3C_VC + - QR_TYPE=${QR_TYPE-W3C_VC}README.md (7)
50-51: Replace “here” with descriptive link textImproves accessibility and passes MD059.
-7. Post Sunbird installation, proceed to create an issuer and credential schema. Refer to the Postman collections available [here](https://github.com/Sunbird-RC/demo-mosip-rc/blob/main/Demo%20Mosip%20RC.postman_collection.json). +7. Post Sunbird installation, proceed to create an issuer and credential schema. Refer to the Sunbird RC demo Postman collection ([Demo Mosip RC.postman_collection.json](https://github.com/Sunbird-RC/demo-mosip-rc/blob/main/Demo%20Mosip%20RC.postman_collection.json)).
53-61: List indentation is inconsistent (MD005)Normalize nested list indentation.
- * Now create a credential schema and create an issuance registry - * take note of `$.schema[0].author` and `$.schema[0].id` from the create credential schema request + * Now create a credential schema and create an issuance registry + * Take note of `$.schema[0].author` and `$.schema[0].id` from the create credential schema request
26-27: Clarify WEB_DID_BASE_URL formatAdd explicit example and note about trailing path segment.
-2. Change the value of `WEB_DID_BASE_URL` in [.env](docker-compose-sunbird/.env) file to your public domain where did.json will be hosted(You can use your github profile to host DIDs). +2. Set `WEB_DID_BASE_URL` in [.env](docker-compose-sunbird/.env) to your public domain where did.json will be hosted (e.g., `https://yourname.github.io/DID-Resolve`). You can use GitHub Pages to host DIDs.
56-60: Optional: mention .nojekyll for GitHub Pages DID hostingGitHub Pages may ignore folders starting with underscores; adding a
.nojekyllavoids surprises.- * Publish the did.json as a webpage. + * Publish the did.json as a webpage (tip: add an empty `.nojekyll` file at the repo root to ensure all paths are served).
69-71: Avoid bare URLs; format consistentlyWrap in code or links to satisfy MD034.
-13. Change `aud` variable in environment to 'http://localhost:8088/v1/esignet/oauth/v2/token' and set `audUrl` to http://localhost:8088 +13. Change `aud` variable in environment to `'http://localhost:8088/v1/esignet/oauth/v2/token'` and set `audUrl` to `'http://localhost:8088'`.
73-79: Security note: Avoid encoding real PII on third‑party sitesWhen base64‑encoding KBA details, prefer local tooling (e.g.,
echo -n '{"fullName":"..."}' | base64 -w0) instead of online services.
Example:echo -n '{"fullName":"Abhishek Gangwar","dob":"1967-10-24"}' | base64 -w0
34-45: Consistent product namingUse a single casing for “Esignet” vs “eSignet” across the doc.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(2 hunks)docker-compose-esignet/config/esignet-default.properties(3 hunks)docker-compose-sunbird/.env(1 hunks)docker-compose-sunbird/docker-compose.yml(5 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
docker-compose-sunbird/.env
[warning] 13-13: [UnorderedKey] The SIGNING_ALGORITHM key should go before the VAULT_ADDR key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The JWKS_URI key should go before the SIGNING_ALGORITHM key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The ENABLE_AUTH key should go before the JWKS_URI key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
README.md
50-50: Link text should be descriptive
(MD059, descriptive-link-text)
56-56: Inconsistent indentation for list items at the same level
Expected: 6; Actual: 8
(MD005, list-indent)
58-58: Inconsistent indentation for list items at the same level
Expected: 6; Actual: 8
(MD005, list-indent)
59-59: Inconsistent indentation for list items at the same level
Expected: 6; Actual: 8
(MD005, list-indent)
60-60: Inconsistent indentation for list items at the same level
Expected: 6; Actual: 8
(MD005, list-indent)
68-68: Link text should be descriptive
(MD059, descriptive-link-text)
69-69: Bare URL used
(MD034, no-bare-urls)
70-70: Bare URL used
(MD034, no-bare-urls)
70-70: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
docker-compose-sunbird/.env (1)
13-13: Signing suite change looks consistent with LDP VC configEd25519Signature2020 aligns with LDP VC suites referenced elsewhere. Ensure the identity image tag in compose supports this suite; older tags expected plain Ed25519.
You can verify at runtime by checking the identity service logs for “Ed25519Signature2020” initialization and a 200 on its /.well-known/did methods if any.docker-compose-esignet/config/esignet-default.properties (2)
426-426: ES256 support added—ensure an EC P-256 key is exposed via JWKSWith ES256 enabled, your JWKS must include an EC P-256 key and clients must be configured accordingly. Also review discovery sections listing only RS256 for some values to avoid client confusion.
Checklist:
- JWKS at ${server.servlet.path}/oauth/.well-known/jwks.json contains { "kty":"EC","crv":"P-256","alg":"ES256" }.
- If using private_key_jwt with ES256, consider adding ES256 to token_endpoint_auth_signing_alg_values_supported in discovery (optional if you keep RS256).
51-51: AMR-ACR mapping URL verifiedHTTP 200 response confirms the file exists at the “collab1” path; no update needed.
docker-compose-sunbird/docker-compose.yml (1)
50-50: Good: wiring WEB_DID_BASE_URL into identityMatches .env and README guidance.
Summary by CodeRabbit
New Features
Documentation
Chores