Skip to content

Commit 826061a

Browse files
authored
Merge pull request #8339 from BitGo/aloe/fixHmacStrategyInit
fix: sdk hmac strategy init & clear
2 parents fccfef1 + d7698ad commit 826061a

4 files changed

Lines changed: 246 additions & 0 deletions

File tree

modules/sdk-api/src/bitgoAPI.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,14 @@ export class BitGoAPI implements BitGoBase {
10461046
response.body.access_token = this._token;
10471047
}
10481048

1049+
// Sync the token into the strategy so that strategies which manage their
1050+
// own key material (e.g. WebCryptoHmacStrategy) can sign the subsequent
1051+
// ensureUserEcdhKeychainIsCreated requests. Must be awaited — the call
1052+
// may perform async key derivation (crypto.subtle.importKey).
1053+
if (this._token) {
1054+
await this._hmacAuthStrategy.setToken?.(this._token);
1055+
}
1056+
10491057
const userSettings = params.ensureEcdhKeychain ? await this.ensureUserEcdhKeychainIsCreated(password) : undefined;
10501058
if (userSettings?.ecdhKeychain) {
10511059
response.body.user.ecdhKeychain = userSettings.ecdhKeychain;
@@ -1089,6 +1097,7 @@ export class BitGoAPI implements BitGoBase {
10891097
if (body.access_token) {
10901098
this._token = body.access_token;
10911099
response.body.access_token = body.access_token;
1100+
await this._hmacAuthStrategy.setToken?.(body.access_token);
10921101
} else {
10931102
throw new Error('Failed to login. Please contact support@bitgo.com');
10941103
}
@@ -1194,6 +1203,11 @@ export class BitGoAPI implements BitGoBase {
11941203
this._ecdhXprv = undefined;
11951204
}
11961205

1206+
async clearAsync(): Promise<void> {
1207+
this.clear();
1208+
await this._hmacAuthStrategy.clearToken?.();
1209+
}
1210+
11971211
/**
11981212
* Use refresh token to get new access token.
11991213
* If the refresh token is null/defined, then we use the stored token from auth
@@ -1221,6 +1235,7 @@ export class BitGoAPI implements BitGoBase {
12211235
.result();
12221236
this._token = body.access_token;
12231237
this._refreshToken = body.refresh_token;
1238+
await this._hmacAuthStrategy?.setToken?.(body.access_token);
12241239
return body;
12251240
}
12261241

@@ -1945,6 +1960,9 @@ export class BitGoAPI implements BitGoBase {
19451960

19461961
this._token = body.access_token;
19471962
this._refreshToken = body.refresh_token;
1963+
1964+
await this._hmacAuthStrategy?.setToken?.(body.access_token);
1965+
19481966
this._user = await this.me();
19491967
return body;
19501968
}

modules/sdk-api/test/unit/bitgoAPI.ts

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { BitGoAPI } from '../../src/bitgoAPI';
33
import { ProxyAgent } from 'proxy-agent';
44
import * as sinon from 'sinon';
55
import nock from 'nock';
6+
import type { IHmacAuthStrategy } from '@bitgo/sdk-hmac';
67

78
describe('Constructor', function () {
89
describe('cookiesPropagationEnabled argument', function () {
@@ -481,6 +482,221 @@ describe('Constructor', function () {
481482
});
482483
});
483484

485+
describe('hmacAuthStrategy token lifecycle', function () {
486+
const ROOT = 'https://app.example.local';
487+
488+
// Builds a mock strategy whose setToken / clearToken are sinon stubs.
489+
function makeStrategy(overrides: Partial<IHmacAuthStrategy> = {}): {
490+
strategy: IHmacAuthStrategy;
491+
setTokenStub: sinon.SinonStub;
492+
clearTokenStub: sinon.SinonStub;
493+
} {
494+
const setTokenStub = sinon.stub().resolves();
495+
const clearTokenStub = sinon.stub().resolves();
496+
const strategy: IHmacAuthStrategy = {
497+
calculateRequestHeaders: sinon.stub().resolves({ hmac: 'hmac', timestamp: 1, tokenHash: 'hash' }),
498+
verifyResponse: sinon.stub().resolves({
499+
isValid: true,
500+
expectedHmac: 'hmac',
501+
signatureSubject: '',
502+
isInResponseValidityWindow: true,
503+
verificationTime: Date.now(),
504+
}),
505+
calculateHMAC: sinon.stub().resolves('hashed-pw'),
506+
setToken: setTokenStub,
507+
clearToken: clearTokenStub,
508+
...overrides,
509+
};
510+
return { strategy, setTokenStub, clearTokenStub };
511+
}
512+
513+
afterEach(function () {
514+
nock.cleanAll();
515+
sinon.restore();
516+
});
517+
518+
describe('authenticate()', function () {
519+
it('calls setToken with the access_token received from the server', async function () {
520+
const { strategy, setTokenStub } = makeStrategy();
521+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
522+
523+
nock(ROOT)
524+
.post('/api/auth/v1/session')
525+
.reply(200, {
526+
user: { username: 'test@example.com' },
527+
access_token: 'v2xmyaccesstoken',
528+
});
529+
530+
await bitgo.authenticate({ username: 'test@example.com', password: 'hunter2' });
531+
532+
setTokenStub.calledOnce.should.be.true();
533+
setTokenStub.firstCall.args[0].should.equal('v2xmyaccesstoken');
534+
});
535+
536+
it('awaits setToken before making ensureEcdhKeychain requests', async function () {
537+
// This is the core regression test: if setToken is not awaited, the
538+
// strategy's key material won't be ready before calculateRequestHeaders
539+
// is called for the GET /user/settings request, and it would throw.
540+
let keyReady = false;
541+
const { strategy } = makeStrategy({
542+
setToken: sinon.stub().callsFake(async () => {
543+
// Simulate non-trivial async key derivation (like crypto.subtle.importKey).
544+
await new Promise<void>((resolve) => setImmediate(resolve));
545+
keyReady = true;
546+
}),
547+
calculateRequestHeaders: sinon.stub().callsFake(async () => {
548+
if (!keyReady) {
549+
throw new Error('No token available. Call setToken() or restoreToken() first.');
550+
}
551+
return { hmac: 'hmac', timestamp: Date.now(), tokenHash: 'hash' };
552+
}),
553+
});
554+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
555+
556+
nock(ROOT)
557+
.post('/api/auth/v1/session')
558+
.reply(200, {
559+
user: { username: 'test@example.com' },
560+
access_token: 'v2xmytoken',
561+
});
562+
// The GET /user/settings request made by ensureUserEcdhKeychainIsCreated
563+
// must succeed — it would throw if setToken wasn't awaited first.
564+
nock(ROOT)
565+
.get('/api/v1/user/settings')
566+
.reply(200, {
567+
settings: { ecdhKeychain: 'xpub123' },
568+
});
569+
570+
await bitgo.authenticate({
571+
username: 'test@example.com',
572+
password: 'hunter2',
573+
ensureEcdhKeychain: true,
574+
});
575+
576+
keyReady.should.be.true();
577+
});
578+
});
579+
580+
describe('authenticateWithPasskey()', function () {
581+
const validPasskey = JSON.stringify({
582+
id: 'credential-id',
583+
rawId: 'raw-id',
584+
type: 'public-key',
585+
response: {
586+
authenticatorData: 'auth-data',
587+
clientDataJSON: 'client-data',
588+
signature: 'sig',
589+
userHandle: 'user-handle-123',
590+
},
591+
});
592+
593+
it('calls setToken with the access_token received from the server', async function () {
594+
const { strategy, setTokenStub } = makeStrategy();
595+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
596+
597+
nock(ROOT)
598+
.post('/api/auth/v1/session')
599+
.reply(200, {
600+
user: { username: 'test@example.com' },
601+
access_token: 'v2xpasskeytoken',
602+
});
603+
604+
await bitgo.authenticateWithPasskey(validPasskey);
605+
606+
setTokenStub.calledOnce.should.be.true();
607+
setTokenStub.firstCall.args[0].should.equal('v2xpasskeytoken');
608+
});
609+
});
610+
611+
describe('clearAsync()', function () {
612+
it('clears _token and calls clearToken on the strategy', async function () {
613+
const { strategy, clearTokenStub } = makeStrategy();
614+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
615+
616+
bitgo.authenticateWithAccessToken({ accessToken: 'v2xsometoken' });
617+
(bitgo as any)._token.should.equal('v2xsometoken');
618+
619+
await bitgo.clearAsync();
620+
621+
((bitgo as any)._token === undefined).should.be.true();
622+
clearTokenStub.calledOnce.should.be.true();
623+
});
624+
});
625+
626+
describe('refreshToken()', function () {
627+
it('calls setToken with the new access_token from the OAuth response', async function () {
628+
const { strategy, setTokenStub } = makeStrategy();
629+
const bitgo = new BitGoAPI({
630+
env: 'custom',
631+
customRootURI: ROOT,
632+
hmacAuthStrategy: strategy,
633+
clientId: 'client-id',
634+
clientSecret: 'client-secret',
635+
});
636+
(bitgo as any)._refreshToken = 'old-refresh-token';
637+
638+
nock(ROOT).post('/oauth/token').reply(200, {
639+
access_token: 'v2xnewtoken',
640+
refresh_token: 'new-refresh-token',
641+
});
642+
643+
await bitgo.refreshToken();
644+
645+
setTokenStub.calledOnce.should.be.true();
646+
setTokenStub.firstCall.args[0].should.equal('v2xnewtoken');
647+
});
648+
});
649+
650+
describe('authenticateWithAuthCode()', function () {
651+
it('calls setToken with the access_token from the OAuth response', async function () {
652+
const { strategy, setTokenStub } = makeStrategy();
653+
const bitgo = new BitGoAPI({
654+
env: 'custom',
655+
customRootURI: ROOT,
656+
hmacAuthStrategy: strategy,
657+
clientId: 'client-id',
658+
clientSecret: 'client-secret',
659+
});
660+
661+
nock(ROOT).post('/oauth/token').reply(200, {
662+
access_token: 'v2xauthcodetoken',
663+
refresh_token: 'refresh-token',
664+
});
665+
// authenticateWithAuthCode calls this.me() after setting the token
666+
nock(ROOT)
667+
.get('/api/v1/user/me')
668+
.reply(200, {
669+
user: { username: 'test@example.com' },
670+
});
671+
672+
await bitgo.authenticateWithAuthCode({ authCode: 'my-auth-code' });
673+
674+
setTokenStub.calledOnce.should.be.true();
675+
setTokenStub.firstCall.args[0].should.equal('v2xauthcodetoken');
676+
});
677+
});
678+
679+
describe('sync token-setting methods', function () {
680+
it('authenticateWithAccessToken does not call setToken (synchronous — caller must invoke setToken on the strategy manually)', function () {
681+
const { strategy, setTokenStub } = makeStrategy();
682+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
683+
684+
bitgo.authenticateWithAccessToken({ accessToken: 'v2xsynctoken' });
685+
686+
setTokenStub.called.should.be.false();
687+
});
688+
689+
it('fromJSON does not call setToken (synchronous — caller must invoke setToken on the strategy manually)', function () {
690+
const { strategy, setTokenStub } = makeStrategy();
691+
const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT, hmacAuthStrategy: strategy });
692+
693+
(bitgo as any).fromJSON({ user: { username: 'test@example.com' }, token: 'v2xjsontoken' });
694+
695+
setTokenStub.called.should.be.false();
696+
});
697+
});
698+
});
699+
484700
describe('constants parameter', function () {
485701
it('should allow passing constants via options and expose via fetchConstants', async function () {
486702
const bitgo = new BitGoAPI({

modules/sdk-hmac/src/types.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,17 @@ export interface IHmacAuthStrategy {
140140
* to the strategy even if no raw token string is available.
141141
*/
142142
isAuthenticated?(): boolean;
143+
144+
/**
145+
* Optional token initialization & destruction methods.
146+
*
147+
* Strategies that read the token directly from request params
148+
* (e.g. DefaultHmacAuthStrategy) may leave this unimplemented.
149+
*
150+
* Must be awaited — implementations may perform async key derivation.
151+
*/
152+
setToken?(token: string): Promise<void>;
153+
clearToken?(): Promise<void>;
143154
}
144155

145156
/**

modules/web-demo/src/components/WebCryptoAuth/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ const WebCryptoAuth = () => {
237237
username,
238238
password,
239239
otp,
240+
ensureEcdhKeychain: true,
240241
});
241242
log('Authentication successful.');
242243

0 commit comments

Comments
 (0)