Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions modules/bitgo/test/unit/keychains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,63 @@ describe('Keychains', function v2keychains() {
}
result.should.hasOwnProperty('version');
});

it('should emit progressCallback outcomes for updated and skipped keychains', async function () {
const oldPassword = 'oldPassword';
const newPassword = 'newPassword';
const otherPassword = 'otherPassword';

const encryptedXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });
const encryptedXprv2 = await bitgo.encrypt({ input: 'xprv2', password: otherPassword });

nock(bgUrl)
.post('/api/v1/user/encrypted')
.reply(200, {
keychains: {
xpub1: encryptedXprv1,
xpub2: encryptedXprv2,
},
version: 1,
});

const events: Array<{ status: string; currentKeychainId?: string }> = [];
await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: (progress) => events.push(progress),
});

events.should.have.length(2);
events.should.containEql({ status: 'updated', currentKeychainId: 'xpub1' });
events.should.containEql({ status: 'skipped', currentKeychainId: 'xpub2' });
});

it('should not let a throwing progressCallback affect the rotation result', async function () {
const oldPassword = 'oldPassword';
const newPassword = 'newPassword';

const encryptedXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });

nock(bgUrl)
.post('/api/v1/user/encrypted')
.reply(200, {
keychains: {
xpub1: encryptedXprv1,
},
version: 1,
});

const result = await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: () => {
throw new Error('observer boom');
},
});

const decryptedPrv = await bitgo.decrypt({ input: result.keychains.xpub1, password: newPassword });
decryptedPrv.should.equal('xprv1');
});
});

after(function afterKeychains() {
Expand Down
133 changes: 133 additions & 0 deletions modules/bitgo/test/v2/unit/keychains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,139 @@ describe('V2 Keychains', function () {
const keys = await keychains.updatePassword({ oldPassword: oldPassword, newPassword: newPassword });
await validateKeys(keys, newPassword, 1);
});

it('should emit progressCallback outcomes in cursor order across every page', async function () {
const prevId = 'prevId';
const encXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });
const encXprv2 = await bitgo.encrypt({ input: 'xprv2', password: otherPassword });
const encXprv3 = await bitgo.encrypt({ input: 'xprv3', password: oldPassword });
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
nextBatchPrevId: prevId,
keys: [
{ pub: 'xpub1', encryptedPrv: encXprv1 },
{ pub: 'xpub2', encryptedPrv: encXprv2 },
],
});
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(function queryNextPageMatch(queryObject) {
return queryObject.prevId === prevId;
})
.reply(200, {
keys: [{ pub: 'xpub3', encryptedPrv: encXprv3 }],
});

const events: Array<{ status: string; currentKeychainId?: string }> = [];
await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: (progress) => events.push(progress),
});

events.should.deepEqual([
{ status: 'updated', currentKeychainId: 'xpub1', total: undefined },
{ status: 'skipped', currentKeychainId: 'xpub2', total: undefined },
{ status: 'updated', currentKeychainId: 'xpub3', total: undefined },
]);
});

it('threads totalCount from the first page into total on every emitted event', async function () {
const encXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });
const encXprv2 = await bitgo.encrypt({ input: 'xprv2', password: oldPassword });
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
keys: [
{ pub: 'xpub1', encryptedPrv: encXprv1 },
{ pub: 'xpub2', encryptedPrv: encXprv2 },
],
totalCount: 2,
});

const events: Array<{ status: string; currentKeychainId?: string; total?: number }> = [];
await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: (progress) => events.push(progress),
});

events.should.deepEqual([
{ status: 'updated', currentKeychainId: 'xpub1', total: 2 },
{ status: 'updated', currentKeychainId: 'xpub2', total: 2 },
]);
});

it('emits skipped for a listed record with no encryptedPrv so completed reaches totalCount (option A)', async function () {
const encXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
keys: [{ pub: 'xpub1', encryptedPrv: encXprv1 }, { pub: 'xpub2' }],
totalCount: 2,
});

const events: Array<{ status: string; currentKeychainId?: string; total?: number }> = [];
await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: (progress) => events.push(progress),
});

// Every listed record reports an outcome (updated | skipped): the placeholder
// record is counted by the list total, so it MUST report skipped — a silent
// drop would leave `completed` one short of `totalCount` in the UI.
events.should.deepEqual([
{ status: 'updated', currentKeychainId: 'xpub1', total: 2 },
{ status: 'skipped', currentKeychainId: 'xpub2', total: 2 },
]);
});

it('should emit skipped for a known decrypt-failure and never mislabel a fatal error as skipped', async function () {
const encXprv1 = await bitgo.encrypt({ input: 'xprv1', password: oldPassword });
const encXprv2 = await bitgo.encrypt({ input: 'xprv2', password: otherPassword });
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
keys: [
{ pub: 'xpub1', encryptedPrv: encXprv1 },
{ pub: 'xpub2', encryptedPrv: encXprv2 },
],
});

const events: Array<{ status: string; currentKeychainId?: string }> = [];
await keychains.updatePassword({
oldPassword,
newPassword,
progressCallback: (progress) => events.push(progress),
});

events.should.containEql({ status: 'updated', currentKeychainId: 'xpub1', total: undefined });
events.should.containEql({ status: 'skipped', currentKeychainId: 'xpub2', total: undefined });

const sandbox = sinon.createSandbox();
try {
nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
keys: [{ pub: 'xpub3', encryptedPrv: encXprv1 }],
});
sandbox.stub(keychains, 'updateSingleKeychainPassword').throws(new Error('some random error'));
const fatalEvents: Array<{ status: string; currentKeychainId?: string }> = [];
await keychains
.updatePassword({ oldPassword, newPassword, progressCallback: (progress) => fatalEvents.push(progress) })
.should.be.rejectedWith('some random error');
fatalEvents.should.have.length(0);
} finally {
sandbox.restore();
}
});
});

describe('Create TSS Keychains', function () {
Expand Down
119 changes: 105 additions & 14 deletions modules/sdk-api/src/bitgoAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
GetUserOptions,
ListWebhookNotificationsOptions,
LoginResponse,
PasswordRotationProgress,
PingOptions,
ProcessedAuthenticationOptions,
ReconstitutedSecret,
Expand Down Expand Up @@ -2010,7 +2011,12 @@ export class BitGoAPI implements BitGoBase {
* @param oldPassword {String} - the current password
* @param newPassword {String} - the new password
*/
async changePassword({ oldPassword, newPassword, encryptionVersion }: ChangePasswordOptions): Promise<any> {
async changePassword({
oldPassword,
newPassword,
encryptionVersion,
progressCallback,
Comment on lines +2015 to +2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
oldPassword,
newPassword,
encryptionVersion,
progressCallback,
oldPassword,
newPassword,
encryptionVersion,
keychainCallbacks: { onProgress, onError },

maybe something like this? The onError hook lets callers decide to handle cases. This can be called on the catch block

Comment on lines +2015 to +2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also params need to be documented in the jsDocs.

}: ChangePasswordOptions): Promise<any> {
if (!_.isString(oldPassword)) {
throw new Error('expected string oldPassword');
}
Expand All @@ -2029,6 +2035,58 @@ export class BitGoAPI implements BitGoBase {
throw new Error('the provided oldPassword is incorrect');
}

const emitProgress = (progress: PasswordRotationProgress) => {
if (!_.isFunction(progressCallback)) {
return;
}
try {
progressCallback(progress);
} catch (e) {
// ignore observer exceptions so a throwing callback never affects rotation results
}
};

// Single counter set shared by both the v1 and v2 lower-level keychain callbacks below.
const counters = { attempted: 0, completed: 0, succeeded: 0, skipped: 0 };
// The v1 set has no server-provided count; its size (the /user/encrypted map) is
// known only once v1 processing returns. Captured so v2 events can add it to the
// backend-served totalCount — without this, a mixed account finishes at
// `completed > total` (2 v1 + 32 v2 keychains read "34 of 32").
let v1KeychainCount = 0;
// A v1-only account emits no v2 event at all, so nothing would ever carry a total;
// tracked so the terminal denominator below can fire for those accounts.
let v2EventCount = 0;
emitProgress({ phase: 'keychains', status: 'started', ...counters });

const makeKeychainProgressCallback =
(keychainVersion: 'v1' | 'v2') =>
(progress: { status: 'updated' | 'skipped'; currentKeychainId?: string; total?: number }) => {
counters.attempted++;
counters.completed++;
if (keychainVersion === 'v2') {
v2EventCount++;
}
if (progress.status === 'updated') {
counters.succeeded++;
} else {
counters.skipped++;
}
emitProgress({
phase: 'keychains',
status: 'updated',
...counters,
// v1 events stay indeterminate: the v1 set has no server count, and surfacing
// its client-side size mid-flight would make the denominator jump the moment
// v2's totalCount arrives. v2 events add the v1 set size so the
// counter reaches the denominator exactly on mixed v1/v2 accounts. When the
// backend supplies no count, the total stays undefined (never fabricated).
total:
keychainVersion === 'v2' && progress.total !== undefined ? v1KeychainCount + progress.total : undefined,
currentKeychainId: progress.currentKeychainId,
keychainVersion,
});
};

// it doesn't matter which coin we choose because the v2 updatePassword functions updates all v2 keychains
// we just need to choose a coin that exists in the current environment
const coin = common.Environments[this.getEnv()].network === 'bitcoin' ? 'btc' : 'tbtc';
Expand All @@ -2038,14 +2096,36 @@ export class BitGoAPI implements BitGoBase {
const encryptionSession =
encryptionVersion === 2 ? await this.createEncryptionSession(newPassword, encryptionVersion) : undefined;
try {
const updateKeychainPasswordParams = {
const v1KeychainUpdatePWResult = await this.keychains().updatePassword({
oldPassword,
newPassword,
encryptionVersion,
encryptionSession,
};
const v1KeychainUpdatePWResult = await this.keychains().updatePassword(updateKeychainPasswordParams);
const v2Keychains = await this.coin(coin).keychains().updatePassword(updateKeychainPasswordParams);
progressCallback: makeKeychainProgressCallback('v1'),
});
// Every v1 entry emits exactly one progress event, so the returned map's size is
// the v1 contribution to the denominator; v2 events below add it to their total.
v1KeychainCount = Object.keys(v1KeychainUpdatePWResult.keychains).length;
const v2Keychains = await this.coin(coin)
.keychains()
.updatePassword({
oldPassword,
newPassword,
encryptionVersion,
encryptionSession,
progressCallback: makeKeychainProgressCallback('v2'),
});

// A v1-only account emits no v2 event, so its total would stay undefined for the
// whole rotation and the observer could never render "n of n". Emit the client-known
// v1 set size as the terminal denominator — guarded on zero v2 events so mixed
// accounts keep the single stable total from totalCount instead of one
// that jumps mid-rotation.
if (v2EventCount === 0 && v1KeychainCount > 0) {
emitProgress({ phase: 'keychains', status: 'updated', ...counters, total: v1KeychainCount });
}

emitProgress({ phase: 'finalizing', status: 'started' });

const [hmacOldPassword, hmacNewPassword] = await Promise.all([
this._hmacAuthStrategy.calculateHMAC(user.username, oldPassword),
Expand All @@ -2065,6 +2145,7 @@ export class BitGoAPI implements BitGoBase {
const payloadSizeKB = Math.ceil(payloadSizeBytes / 1024);

// Check if batching flow is enabled
let useBatchingFlow = false;
try {
const batchingFlowCheck = await this.get(this.url('/user/checkBatchingPasswordFlow', 2))
.query({ payloadSize: payloadSizeKB.toString() })
Expand All @@ -2077,20 +2158,30 @@ export class BitGoAPI implements BitGoBase {
batchingFlowCheck.maxBatchSizeKB,
3
);
// Call changepassword API without keychains for batching flow
return this.post(this.url('/user/changepassword'))
.send({
version: updatePasswordParams.version,
oldPassword: updatePasswordParams.oldPassword,
password: updatePasswordParams.password,
})
.result();
useBatchingFlow = true;
}
} catch (error) {
// batching flow check failed
}

return this.post(this.url('/user/changepassword')).send(updatePasswordParams).result();
if (useBatchingFlow) {
// Call changepassword API without keychains for batching flow. Awaited outside the
// check/upload try-catch above, so a rejection here still propagates to the caller
// with no legacy-POST fallback, exactly as before.
const result = await this.post(this.url('/user/changepassword'))
.send({
version: updatePasswordParams.version,
oldPassword: updatePasswordParams.oldPassword,
password: updatePasswordParams.password,
})
.result();
emitProgress({ phase: 'finalizing', status: 'completed' });
return result;
}

const result = await this.post(this.url('/user/changepassword')).send(updatePasswordParams).result();
emitProgress({ phase: 'finalizing', status: 'completed' });
return result;
} finally {
encryptionSession?.destroy();
}
Expand Down
Loading
Loading