Sync Improve WalletConfig and VPTokenSigningResult / UnsignedVPToken changes in library#2504
Sync Improve WalletConfig and VPTokenSigningResult / UnsignedVPToken changes in library#2504KiruthikaJeyashankar wants to merge 3 commits into
Conversation
Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com>
Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds ChangesOpenID4VP token id propagation and wallet config update
VC Card Disclosure Label Color
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/io/mosip/residentapp/utils/OpenId4VPUtils.java (1)
245-257: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate
idbefore constructingVPTokenSigningResult.
idis now required, but this path only asserts non-null; assertions are not reliable runtime validation on Android. Reject missing/nullidbefore decoding/constructing the result.Suggested fix
if (vpTokenSigningResultMap == null || !vpTokenSigningResultMap.hasKey("signedData") - || vpTokenSigningResultMap.isNull("signedData")) { + || vpTokenSigningResultMap.isNull("signedData") + || !vpTokenSigningResultMap.hasKey("id") + || vpTokenSigningResultMap.isNull("id")) { continue; } String signedData = vpTokenSigningResultMap.getString("signedData"); String id = vpTokenSigningResultMap.getString("id"); byte[] signedDataBytes = Base64.decode(signedData, Base64.URL_SAFE | Base64.NO_WRAP | Base64.NO_PADDING); - assert id != null; formattedVpTokenSigningResults.add( - new VPTokenSigningResult(id,signedDataBytes)); + new VPTokenSigningResult(id, signedDataBytes));🤖 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 `@android/app/src/main/java/io/mosip/residentapp/utils/OpenId4VPUtils.java` around lines 245 - 257, The OpenId4VPUtils path that builds VPTokenSigningResult only asserts id is non-null, so replace that with real runtime validation before decoding signedData and constructing the result. In the loop that reads vpTokenSigningResultMap, check the id field alongside signedData, skip or reject entries with missing/null id, and only call new VPTokenSigningResult(id, signedDataBytes) when id is present.
🤖 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 `@android/app/src/main/java/io/mosip/residentapp/utils/OpenId4VPUtils.java`:
- Around line 81-84: In OpenId4VPUtils, the optional config flag reads for
presentationDefinitionUriSupported and validateTrustedVerifier should not call
getBoolean directly because a present null value can fail instead of falling
back to true. Update these checks to use the existing null-safe boolean helper
already used for optional config flags, so the defaulting behavior stays
consistent when the key is missing or null.
---
Outside diff comments:
In `@android/app/src/main/java/io/mosip/residentapp/utils/OpenId4VPUtils.java`:
- Around line 245-257: The OpenId4VPUtils path that builds VPTokenSigningResult
only asserts id is non-null, so replace that with real runtime validation before
decoding signedData and constructing the result. In the loop that reads
vpTokenSigningResultMap, check the id field alongside signedData, skip or reject
entries with missing/null id, and only call new VPTokenSigningResult(id,
signedDataBytes) when id is present.
🪄 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: 2c6df879-aefb-41ef-a3c0-269390039014
📒 Files selected for processing (9)
android/app/src/main/java/io/mosip/residentapp/InjiOpenID4VPModule.javaandroid/app/src/main/java/io/mosip/residentapp/InjiVCIClientCallback.ktandroid/app/src/main/java/io/mosip/residentapp/utils/OpenId4VPUtils.javacomponents/VC/Views/VCCardViewContent.tsxios/Utils/OpenId4VPUtils.swiftmachines/openID4VP/openID4VPActions.tsshared/openID4VP/OpenID4VP.tsshared/openID4VP/OpenID4VPHelper.tsshared/openID4VP/openid4vp.types.ts
💤 Files with no reviewable changes (1)
- shared/openID4VP/OpenID4VP.ts
Signed-off-by: KiruthikaJeyashankar <kiruthikavjshankar@gmail.com>
Description
Wallet Config updates
Rename validatePreRegisteredVerifier → validateTrustedVerifier aligning with the trustedVerifiers convention
Remove supportedRequestUriMethods config
VPTokenSigningResult / UnsignedVPToken Updates
Include id in the VPTokenSigningResult / UnsignedVPToken data
Issue ticket number and link
Dependent PRs
This PR is dependent on
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
id, and the system now validates thatidis present when parsing results.Style