Skip to content

feat(sdk-api): implement V4 token issuance flow#8138

Draft
arvind-bg wants to merge 1 commit intomasterfrom
CAAS-783-auth-v4-token-issuance-flow
Draft

feat(sdk-api): implement V4 token issuance flow#8138
arvind-bg wants to merge 1 commit intomasterfrom
CAAS-783-auth-v4-token-issuance-flow

Conversation

@arvind-bg
Copy link
Contributor

@arvind-bg arvind-bg commented Feb 13, 2026

Summary

Implements V4 authentication token issuance flow with separate token identifier and signing key.

Implementation Details

Token Issuance

  • V4 tokens consist of two components:
    • tokenId: Plaintext identifier from response.id
    • signingKey: ECDH-encrypted signing key from encryptedToken
  • addAccessToken() handles V4 token creation when authVersion: 4 is set
  • V4 reuses existing handleTokenIssuance() method for ECDH decryption (same as V2/V3)
  • Only V4-specific logic: Extract and store tokenId from response.id field

State Management

  • BitGoAPI stores _tokenId alongside existing _token for V4 sessions
  • toJSON() and fromJSON() include tokenId for session persistence
  • clear() clears both token and tokenId

Type Support

  • Added AuthVersion type including value 4 in sdk-hmac
  • Added tokenId fields to BitGoJson, TokenIssuanceResponse, and AddAccessTokenResponse

Backwards Compatibility

V2 and V3 authentication flows remain unchanged.

Ticket: CAAS-783

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@arvind-bg
Copy link
Contributor Author

Response to Copilot Review Comments

Comment #1: Invalid Parameter Validation Test

Fixed - Changed the test to use an empty object with @ts-expect-error directive instead of empty strings. This properly validates that missing required fields throw an error.

Comment #2: Use ES6 Property Shorthand

Fixed - Applied shorthand syntax: signingKey, instead of signingKey: signingKey in modules/sdk-api/src/bitgoAPI.ts:1157.

Comment #3: Missing V3-Compatibility Test for HMAC

Not 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 Stub

Fixed - Corrected the stub to use the module pattern: sinon.stub(require('../../src/api'), 'verifyResponse').callsFake(verifyResponseStub) instead of stubbing an instance method.

@arvind-bg
Copy link
Contributor Author

@copilot re-review this PR

Copy link

Copilot AI commented Feb 13, 2026

@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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@arvind-bg arvind-bg force-pushed the CAAS-783-auth-v4-token-issuance-flow branch from d25852f to b050c5a Compare February 13, 2026 15:52
@arvind-bg
Copy link
Contributor Author

Additional Copilot Feedback Addressed

Fixed Issues

  1. Removed ecdhXprvOverride parameter - The handleTokenIssuance() method no longer needs this parameter. The processV4TokenIssuance() helper now relies on the instance's _ecdhXprv property directly, simplifying the implementation.

  2. Removed redundant ECDH check - The check for this._ecdhXprv at line 1333 was unreachable because the code already returns early via forceV1Auth when _ecdhXprv is not set (lines 1309-1318). This redundant check has been removed.

  3. Updated tests accordingly - All processV4TokenIssuance tests now set bitgo._ecdhXprv on the instance rather than passing it as a parameter.

Not Fixed - Assertion Library Syntax

Copilot suggested changing .should.be.a.String() to .should.be.a('string'). However, this suggestion is incorrect for this codebase's assertion library. The original syntax .should.be.a.String() is the correct format and tests pass with it. The suggested change causes test failures.

Out of Scope

HMAC 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.

@arvind-bg arvind-bg force-pushed the CAAS-783-auth-v4-token-issuance-flow branch from b050c5a to 3f5a09c Compare February 13, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants