fix: replace publicKey with w3id #430
Conversation
WalkthroughThe changes refactor the signing API layer by renaming the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 0
🧹 Nitpick comments (1)
platforms/evoting-api/src/services/SigningService.ts (1)
131-175: LGTM! Security verification logic correctly updated.The w3id verification logic is sound and all references have been properly updated from publicKey to w3id. The security check comparing against the user's ename remains intact and functions correctly.
Minor observation: There's a slight inconsistency in casing between "W3ID" (lines 163, 168, 171) and "w3id" (lines 173, 174) in error messages and logs. Consider standardizing to lowercase "w3id" for consistency.
Optional: Standardize casing
- console.error(`🔒 SECURITY VIOLATION: w3id mismatch!`, { + console.error(`🔒 SECURITY VIOLATION: w3id mismatch!`, { w3id, userEname: user.ename, cleanW3id, cleanUserEname, sessionUserId: session.userId }); // Update session status to indicate security violation session.status = "security_violation"; session.updatedAt = new Date(); this.sessions.set(sessionId, session); // Notify subscribers of security violation this.notifySubscribers(sessionId, { type: "security_violation", status: "security_violation", - error: "W3ID does not match the user who created this signing session", + error: "w3id does not match the user who created this signing session", sessionId }); // Return success: false but don't throw error - let the wallet think it succeeded - return { success: false, error: "W3ID does not match the user who created this signing session" }; + return { success: false, error: "w3id does not match the user who created this signing session" }; } - console.log(`✅ W3ID verification passed: ${cleanW3id} matches ${cleanUserEname}`); + console.log(`✅ w3id verification passed: ${cleanW3id} matches ${cleanUserEname}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/evoting-api/src/controllers/SigningController.ts(2 hunks)platforms/evoting-api/src/services/SigningService.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ananyayaya129
Repo: MetaState-Prototype-Project/prototype PR: 426
File: platforms/group-charter-manager-api/src/controllers/CharterSigningController.ts:124-0
Timestamp: 2025-11-14T05:57:21.470Z
Learning: In the group-charter-manager-api signing workflow, `w3id` is the correct parameter name for identifying users and obtaining their public keys. The system retrieves public keys through w3id, not through a separate publicKey parameter.
📚 Learning: 2025-11-14T05:57:21.470Z
Learnt from: ananyayaya129
Repo: MetaState-Prototype-Project/prototype PR: 426
File: platforms/group-charter-manager-api/src/controllers/CharterSigningController.ts:124-0
Timestamp: 2025-11-14T05:57:21.470Z
Learning: In the group-charter-manager-api signing workflow, `w3id` is the correct parameter name for identifying users and obtaining their public keys. The system retrieves public keys through w3id, not through a separate publicKey parameter.
Applied to files:
platforms/evoting-api/src/controllers/SigningController.tsplatforms/evoting-api/src/services/SigningService.ts
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
platforms/evoting-api/src/services/SigningService.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
platforms/evoting-api/src/services/SigningService.ts (2)
17-22: LGTM! Interface correctly updated to use w3id.The renaming from
publicKeytow3idaligns with the established pattern where w3id is the correct parameter for identifying users and obtaining their public keys in signing workflows. Based on learnings.
115-115: LGTM! Method signature correctly updated.The parameter rename from
publicKeytow3idis consistent with the interface change and aligns with the correct identifier pattern. Based on learnings.platforms/evoting-api/src/controllers/SigningController.ts (1)
94-117: LGTM! Controller correctly updated to use w3id.The changes to
handleSignedPayloadare correct and consistent with the service layer updates:
- Request body properly destructures
w3idinstead ofpublicKey- Validation logic checks for
w3idpresence- Missing field reporting includes
w3id- Service call passes
w3idas the third argument matching the updated method signatureThe integration between controller and service is sound. Based on learnings.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit