feat(sdk-api): implement V4 token issuance flow#8138
feat(sdk-api): implement V4 token issuance flow#8138
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the V4 authentication token issuance flow with a key architectural change: separating token identification from HMAC signing. Unlike V2/V3 where a single token serves both purposes, V4 introduces a plaintext tokenId for identification and an encrypted signingKey for HMAC authentication. The implementation carefully isolates V4-specific logic behind version checks to maintain backward compatibility, while reusing existing ECDH decryption infrastructure.
Changes:
- Implements V4 token issuance with dual token system (tokenId + signingKey) in authentication and access token flows
- Extends HMAC module to support AuthVersion 4 using V3-compatible subject format with method prefix
- Adds comprehensive test coverage with 24 V4-specific tests and regression tests for V2/V3 backward compatibility
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/sdk-hmac/src/types.ts | Adds AuthVersion 4 to supported authentication versions |
| modules/sdk-hmac/src/hmac.ts | Updates HMAC subject calculation to treat V4 identically to V3 (method prefix pattern) |
| modules/sdk-api/src/types.ts | Adds V4-specific type definitions for token issuance parameters/results and optional tokenId fields |
| modules/sdk-api/src/bitgoAPI.ts | Implements V4 token issuance flows in authenticate() and addAccessToken(), adds processV4TokenIssuance() helper, updates session management for tokenId |
| modules/sdk-api/test/unit/bitgoAPI.ts | Adds comprehensive test suite covering V4 token issuance, serialization, HMAC calculation, edge cases, and V2/V3 regression tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Response to Copilot Review CommentsComment #1: Invalid Parameter Validation TestFixed - Changed the test to use an empty object with Comment #2: Use ES6 Property ShorthandFixed - Applied shorthand syntax: Comment #3: Missing V3-Compatibility Test for HMACNot Applicable - V4 HMAC pre-image computation is intentionally not implemented in this PR. This PR scope is limited to token issuance only. V4 HMAC calculation will be implemented in a separate PR. Comment #4: Incorrect verifyResponse StubFixed - Corrected the stub to use the module pattern: |
|
@copilot re-review this PR |
|
@arvind-bg I've opened a new pull request, #8140, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d25852f to
b050c5a
Compare
Additional Copilot Feedback AddressedFixed Issues
Not Fixed - Assertion Library SyntaxCopilot suggested changing Out of ScopeHMAC calculation for V4 - V4 token validation, including HMAC calculation with the V4-specific pre-image format, will be implemented in a separate PR. This PR is limited to token issuance only. |
b050c5a to
3f5a09c
Compare
Summary
Implements V4 authentication token issuance flow with separate token identifier and signing key.
Implementation Details
Token Issuance
tokenId: Plaintext identifier from response.idsigningKey: ECDH-encrypted signing key from encryptedTokenaddAccessToken()handles V4 token creation whenauthVersion: 4is sethandleTokenIssuance()method for ECDH decryption (same as V2/V3)tokenIdfrom response.id fieldState Management
BitGoAPIstores_tokenIdalongside existing_tokenfor V4 sessionstoJSON()andfromJSON()include tokenId for session persistenceclear()clears both token and tokenIdType Support
AuthVersiontype including value 4 in sdk-hmacBitGoJson,TokenIssuanceResponse, andAddAccessTokenResponseBackwards Compatibility
V2 and V3 authentication flows remain unchanged.
Ticket: CAAS-783