Add PIN change functionality #99
Conversation
Adds changePin(oldPin, newPin) method to EmvApplication for changing the cardholder PIN on cards that support it. Implementation details: - Uses CHANGE REFERENCE DATA command (INS 0x24) - Both old and new PINs encoded in ISO 9564 Format 2 (BCD) - Validates both PINs are 4-12 digits - Returns appropriate status words (9000 success, 63CX wrong PIN, 6983 blocked) Refactored buildPinBlock() as a shared helper for PIN encoding. Note: Most payment cards restrict PIN change to specific environments (ATM, bank). This feature is primarily useful for test cards. Closes #98
There was a problem hiding this comment.
Pull request overview
This PR adds PIN change functionality to the EMV application, implementing the CHANGE REFERENCE DATA command (INS 0x24) as specified in issue #98. The implementation includes proper validation, error handling, and comprehensive test coverage for the new feature.
Key Changes
- Refactored PIN encoding logic into a shared
buildPinBlock()helper function to reduce code duplication - Added
buildChangePinApdu()function to construct CHANGE REFERENCE DATA APDU commands - Implemented
changePin(oldPin, newPin)method with validation for both old and new PINs (4-12 digits)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/emv-application.ts | Refactored buildPinBlock() as a shared helper, added buildChangePinApdu() and changePin() methods with proper validation and documentation |
| src/emv-application.test.ts | Added comprehensive test suite for changePin() covering validation, APDU format, BCD encoding, and various status responses |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| () => emv.changePin('1234', 'abcd'), | ||
| /New PIN must be a string of 4-12 digits/ | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for new PIN longer than 12 digits. The verifyPin tests include a test for PINs longer than 12 digits, and similar coverage should be added here to ensure validation works correctly for new PINs exceeding the maximum length.
| assert.strictEqual(apdu[14], 0x56); | ||
| assert.strictEqual(apdu[15], 0x78); | ||
| assert.strictEqual(apdu[16], 0xff); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for different PIN lengths. The verifyPin tests include a test for 6-digit PINs to verify BCD encoding works correctly for different lengths. Similar test coverage should be added for changePin to ensure both old and new PINs of varying lengths (e.g., 6 digits, 12 digits) are encoded correctly.
| it('should return success for PIN change', async () => { | ||
| mockCard.transmit = async () => Buffer.from([0x90, 0x00]); | ||
| const response = await emv.changePin('1234', '5678'); | ||
| assert.strictEqual(response.isOk(), true); | ||
| }); |
There was a problem hiding this comment.
Incomplete test assertion for success case. The test only checks response.isOk() but doesn't verify the specific status words (sw1 and sw2) like the corresponding verifyPin test does. Consider adding assertions for response.sw1 and response.sw2 to be consistent with the verifyPin test pattern and ensure the correct status words are returned.
| () => emv.changePin('1234', 'abcd'), | ||
| /New PIN must be a string of 4-12 digits/ | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for old PIN longer than 12 digits. The verifyPin tests include a test for PINs longer than 12 digits, and similar coverage should be added here to ensure validation works correctly for old PINs exceeding the maximum length.
Summary
changePin(oldPin, newPin)method to EmvApplicationbuildPinBlock()as shared helper for PIN encodingImplementation
Note
Most payment cards restrict PIN change to specific environments (ATM, bank terminal). This feature is primarily useful for test cards.
Test plan
Closes #98