diff --git a/modules/bitgo/test/unit/keychains.ts b/modules/bitgo/test/unit/keychains.ts index a817dbfd1bb..b53c747a5b5 100644 --- a/modules/bitgo/test/unit/keychains.ts +++ b/modules/bitgo/test/unit/keychains.ts @@ -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() { diff --git a/modules/bitgo/test/v2/unit/keychains.ts b/modules/bitgo/test/v2/unit/keychains.ts index 1847d0a43e0..382a322a246 100644 --- a/modules/bitgo/test/v2/unit/keychains.ts +++ b/modules/bitgo/test/v2/unit/keychains.ts @@ -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 () { diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 797889c164a..d44967d9870 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -68,6 +68,7 @@ import { GetUserOptions, ListWebhookNotificationsOptions, LoginResponse, + PasswordRotationProgress, PingOptions, ProcessedAuthenticationOptions, ReconstitutedSecret, @@ -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 { + async changePassword({ + oldPassword, + newPassword, + encryptionVersion, + progressCallback, + }: ChangePasswordOptions): Promise { if (!_.isString(oldPassword)) { throw new Error('expected string oldPassword'); } @@ -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'; @@ -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), @@ -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() }) @@ -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(); } diff --git a/modules/sdk-api/src/types.ts b/modules/sdk-api/src/types.ts index 17c20c1b69a..29d7a1af772 100644 --- a/modules/sdk-api/src/types.ts +++ b/modules/sdk-api/src/types.ts @@ -265,6 +265,25 @@ export interface GetEcdhSecretOptions { eckey: ECPairInterface; } +export type PasswordRotationProgress = + | { + phase: 'keychains'; + status: 'started' | 'updated'; + completed: number; + total?: number; + attempted: number; + succeeded: number; + skipped: number; + currentKeychainId?: string; + keychainVersion?: 'v1' | 'v2'; + } + | { + phase: 'finalizing'; + status: 'started' | 'completed'; + }; + +export type PasswordRotationProgressCallback = (progress: PasswordRotationProgress) => void; + export interface ChangePasswordOptions { oldPassword: string; newPassword: string; @@ -274,6 +293,8 @@ export interface ChangePasswordOptions { * Argon2id upgrade for v1 (SJCL) keychains once the caller is ready. */ encryptionVersion?: EncryptionVersion; + /** Optional observer invoked as the password rotation progresses through keychains and finalization. */ + progressCallback?: PasswordRotationProgressCallback; } /** diff --git a/modules/sdk-api/src/v1/keychains.ts b/modules/sdk-api/src/v1/keychains.ts index bc557563a13..bc8d98d38cf 100644 --- a/modules/sdk-api/src/v1/keychains.ts +++ b/modules/sdk-api/src/v1/keychains.ts @@ -188,6 +188,16 @@ Keychains.prototype.updatePassword = function (params, callback) { const newKeychains = {}; // @ts-expect-error - no implicit this const self = this; + const notifyProgress = (status: 'updated' | 'skipped', currentKeychainId: string) => { + if (!_.isFunction(params.progressCallback)) { + return; + } + try { + params.progressCallback({ status, currentKeychainId }); + } catch (e) { + // ignore observer exceptions so a throwing callback never affects rotation results + } + }; for (const [xpub, oldEncryptedXprv] of Object.entries((encrypted as any).keychains)) { try { const decryptedPrv = await self.bitgo.decrypt({ @@ -201,9 +211,11 @@ Keychains.prototype.updatePassword = function (params, callback) { encryptionVersion, }); newKeychains[xpub] = newEncryptedPrv; + notifyProgress('updated', xpub); } catch (e) { // decrypting the keychain with the old password didn't work so we just keep it the way it is newKeychains[xpub] = oldEncryptedXprv as string; + notifyProgress('skipped', xpub); } } return { keychains: newKeychains, version: (encrypted as any).version }; diff --git a/modules/sdk-api/test/unit/bitgoAPI.ts b/modules/sdk-api/test/unit/bitgoAPI.ts index 9c6666b8688..f887e2ed24b 100644 --- a/modules/sdk-api/test/unit/bitgoAPI.ts +++ b/modules/sdk-api/test/unit/bitgoAPI.ts @@ -6,6 +6,7 @@ import * as sinon from 'sinon'; import nock from 'nock'; import type { IHmacAuthStrategy } from '@bitgo/sdk-hmac'; import type { IEncryptionSession } from '@bitgo/sdk-core'; +import type { PasswordRotationProgress } from '../../src/types'; describe('Constructor', function () { describe('cookiesPropagationEnabled argument', function () { @@ -1189,6 +1190,266 @@ describe('Constructor', function () { }); sinon.assert.calledOnce(destroy); }); + + it('emits keychains/started, monotonic keychains/updated events, then finalizing/started and finalizing/completed', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(200, { isBatchingFlowEnabled: false }); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + v1UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub1' }); + params.progressCallback?.({ status: 'skipped', currentKeychainId: 'xpub2' }); + // one map entry per emitted event, since the map's size now feeds the v2 total + return { keychains: { k1: 'v1enc', k2: 'v1enc2' }, version: 25 }; + }); + v2UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub3' }); + return { v2k1: 'v2enc' }; + }); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + events.should.deepEqual([ + { phase: 'keychains', status: 'started', completed: 0, attempted: 0, succeeded: 0, skipped: 0 }, + { + phase: 'keychains', + status: 'updated', + completed: 1, + attempted: 1, + succeeded: 1, + skipped: 0, + total: undefined, + currentKeychainId: 'xpub1', + keychainVersion: 'v1', + }, + { + phase: 'keychains', + status: 'updated', + completed: 2, + attempted: 2, + succeeded: 1, + skipped: 1, + total: undefined, + currentKeychainId: 'xpub2', + keychainVersion: 'v1', + }, + { + phase: 'keychains', + status: 'updated', + completed: 3, + attempted: 3, + succeeded: 2, + skipped: 1, + total: undefined, + currentKeychainId: 'xpub3', + keychainVersion: 'v2', + }, + { phase: 'finalizing', status: 'started' }, + { phase: 'finalizing', status: 'completed' }, + ]); + }); + + it('forwards a truthful total from the v2 lower-level callback into every keychains/updated event', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(200, { isBatchingFlowEnabled: false }); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + v2UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub1', total: 2 }); + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub2', total: 2 }); + return { v2k1: 'v2enc', v2k2: 'v2enc2' }; + }); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + const v2Events = events.filter( + (e): e is Extract => + e.phase === 'keychains' && e.status === 'updated' && e.keychainVersion === 'v2' + ); + // the v1 stub keeps its default 1-entry map, so v2 events forward 1 (v1) + 2 (v2) = 3 + v2Events.every((e) => e.total === 3).should.be.true(); + }); + + it('covers both keychain sets in the total so mixed v1/v2 accounts complete exactly (2 v1 + 32 v2 = 34 of 34)', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(200, { isBatchingFlowEnabled: false }); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + v1UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub1' }); + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub2' }); + return { keychains: { k1: 'v1enc', k2: 'v1enc2' }, version: 25 }; + }); + v2UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + for (let i = 1; i <= 32; i++) { + params.progressCallback?.({ status: 'updated', currentKeychainId: `v2key-${i}`, total: 32 }); + } + return { v2k1: 'v2enc' }; + }); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + const keychainEvents = events.filter( + (e): e is Extract => + e.phase === 'keychains' && e.status === 'updated' + ); + const v1Events = keychainEvents.filter((e) => e.keychainVersion === 'v1'); + const v2Events = keychainEvents.filter((e) => e.keychainVersion === 'v2'); + + // v1 events stay indeterminate: the v1 set has no server-provided count + v1Events.should.have.length(2); + v1Events.every((e) => e.total === undefined).should.be.true(); + + // v2 events carry the combined denominator: 2 (v1 set) + 32 (totalCount) + v2Events.should.have.length(32); + v2Events.every((e) => e.total === 34).should.be.true(); + + // the counter completes exactly instead of overshooting the total + const last = v2Events[v2Events.length - 1]; + last.completed.should.equal(34); + last.total!.should.equal(34); + }); + + it('emits a terminal total for v1-only accounts so the counter can render n of n', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(200, { isBatchingFlowEnabled: false }); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + v1UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub1' }); + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub2' }); + return { keychains: { k1: 'v1enc', k2: 'v1enc2' }, version: 25 }; + }); + // a v1-only account: the v2 walk finds nothing and emits nothing + v2UpdatePasswordStub.callsFake(async () => ({})); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + const keychainEvents = events.filter( + (e): e is Extract => + e.phase === 'keychains' && e.status === 'updated' + ); + const v1Events = keychainEvents.filter((e) => e.keychainVersion === 'v1'); + v1Events.should.have.length(2); + v1Events.every((e) => e.total === undefined).should.be.true(); + + // no v2 event ever arrived, so the terminal keychains/updated event carries the + // client-known v1 set size as the denominator: 2 of 2, immediately before finalizing + const last = keychainEvents[keychainEvents.length - 1]; + last.completed.should.equal(2); + last.total!.should.equal(2); + events[events.length - 2].should.deepEqual({ phase: 'finalizing', status: 'started' }); + }); + + it('does not let a throwing progressCallback abort the rotation', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(200, { isBatchingFlowEnabled: false }); + const changePassScope = nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: () => { + throw new Error('observer boom'); + }, + }); + + changePassScope.isDone().should.be.true(); + }); + + it('emits no finalizing/completed and no legacy fallback when the batching-path final POST fails', async function () { + nock(ROOT) + .get('/api/v2/user/checkBatchingPasswordFlow') + .query(true) + .reply(200, { isBatchingFlowEnabled: true, maxBatchSizeKB: 900 }); + nock(ROOT).put('/api/v2/user/keychains').reply(200, {}); + const changePassScope = nock(ROOT).post('/api/v1/user/changepassword').reply(500, { error: 'boom' }); + + const events: PasswordRotationProgress[] = []; + await bitgo + .changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }) + .should.be.rejected(); + + changePassScope.isDone().should.be.true(); + events.some((e) => e.phase === 'finalizing' && e.status === 'completed').should.be.false(); + }); + + it('completes exactly once via legacy fallback when the batching check fails', async function () { + nock(ROOT).get('/api/v2/user/checkBatchingPasswordFlow').query(true).reply(503, { error: 'service unavailable' }); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !!body.keychains && !!body.v2_keychains) + .reply(200, {}); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + events.filter((e) => e.phase === 'finalizing' && e.status === 'completed').should.have.length(1); + }); + + it('does not add keychain events or double-count when a transport batch is retried', async function () { + nock(ROOT) + .get('/api/v2/user/checkBatchingPasswordFlow') + .query(true) + .reply(200, { isBatchingFlowEnabled: true, maxBatchSizeKB: 900 }); + nock(ROOT).put('/api/v2/user/keychains').reply(500, { error: 'transient' }); + nock(ROOT).put('/api/v2/user/keychains').reply(200, {}); + nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !body.keychains && !body.v2_keychains) + .reply(200, {}); + + v1UpdatePasswordStub.callsFake(async (params: { progressCallback?: (p: unknown) => void }) => { + params.progressCallback?.({ status: 'updated', currentKeychainId: 'xpub1' }); + return { keychains: { k1: 'v1enc' }, version: 25 }; + }); + + const events: PasswordRotationProgress[] = []; + await bitgo.changePassword({ + oldPassword: 'oldpw', + newPassword: 'newpw', + progressCallback: (progress) => events.push(progress), + }); + + const keychainEvents = events.filter((e) => e.phase === 'keychains' && e.status === 'updated'); + // one real keychain event plus the terminal denominator event for this v1-only + // account — the retried transport must contribute neither a duplicate event + // nor a double-count to either + keychainEvents.should.have.length(2); + keychainEvents[0].should.have.properties({ succeeded: 1, skipped: 0, attempted: 1, completed: 1 }); + keychainEvents[1].should.have.properties({ succeeded: 1, skipped: 0, attempted: 1, completed: 1, total: 1 }); + }); }); describe('createUserEcdhKeychain - encryptionVersion threading', function () { diff --git a/modules/sdk-core/src/bitgo/keychain/iKeychains.ts b/modules/sdk-core/src/bitgo/keychain/iKeychains.ts index 0868ef73e31..696449191bc 100644 --- a/modules/sdk-core/src/bitgo/keychain/iKeychains.ts +++ b/modules/sdk-core/src/bitgo/keychain/iKeychains.ts @@ -81,6 +81,8 @@ export interface ChangedKeychains { export interface ListKeychainsResult { keys: Keychain[]; nextBatchPrevId?: string; + /** Total of the listed keychains — the count of the list's own query (WCN-2084 total-count support). */ + totalCount?: number; } export interface GetKeychainOptions { @@ -102,6 +104,21 @@ export interface ListKeychainOptions { safeId?: string; } +export type KeychainPasswordUpdateStatus = 'updated' | 'skipped'; + +export interface KeychainPasswordUpdateProgress { + status: KeychainPasswordUpdateStatus; + currentKeychainId?: string; + /** + * Truthful total of the listed keychains (`totalCount` from the backend, WCN-2084). + * Every listed record reports an outcome (updated | skipped), so `completed` reaches + * this total exactly on any account composition. + */ + total?: number; +} + +export type KeychainPasswordUpdateProgressCallback = (progress: KeychainPasswordUpdateProgress) => void; + export interface UpdatePasswordOptions { oldPassword: string; newPassword: string; @@ -131,6 +148,8 @@ export interface UpdatePasswordOptions { * per-keychain source versions. */ safeId?: string; + /** Optional observer invoked once per nonfatal keychain outcome during password rotation. */ + progressCallback?: KeychainPasswordUpdateProgressCallback; } export interface UpdateSingleKeychainPasswordOptions { diff --git a/modules/sdk-core/src/bitgo/keychain/keychains.ts b/modules/sdk-core/src/bitgo/keychain/keychains.ts index eb4207dc70a..56f9b7140a0 100644 --- a/modules/sdk-core/src/bitgo/keychain/keychains.ts +++ b/modules/sdk-core/src/bitgo/keychain/keychains.ts @@ -155,13 +155,35 @@ export class Keychains implements IKeychains { return this.updateSafePassword(params as UpdatePasswordOptions & { safeId: string }); } const changedKeys: ChangedKeychains = {}; + let total: number | undefined; + const notifyProgress = (status: 'updated' | 'skipped', currentKeychainId?: string) => { + if (!_.isFunction(params.progressCallback)) { + return; + } + try { + params.progressCallback({ status, currentKeychainId, total }); + } catch (e) { + // ignore observer exceptions so a throwing callback never affects rotation results + } + }; + const observationId = (key: Keychain): string | undefined => + key.type === 'tss' || Keychains.isMultiUserKey(key) ? key.id : key.pub; let prevId; let keysLeft = true; while (keysLeft) { const result: ListKeychainsResult = await this.list({ limit: 500, prevId }); + if (total === undefined && result.totalCount !== undefined) { + total = result.totalCount; + } for (const key of result.keys) { const oldEncryptedPrv = key.encryptedPrv; if (_.isUndefined(oldEncryptedPrv)) { + // Option A counting unit (WCN-2084): every listed record reports an outcome, + // so `completed` reaches the server's `totalCount` on any account composition. + // A record with no serialized material is reported as skipped rather than + // silently dropped — it is counted by the list total, and the walk still + // leaves it untouched (no re-encryption, no entry in the changedKeys map). + notifyProgress('skipped', observationId(key)); continue; } try { @@ -180,7 +202,12 @@ export class Keychains implements IKeychains { : updatedKeychain.pub; if (changedKeyIdentifier) { changedKeys[changedKeyIdentifier] = updatedKeychain.encryptedPrv; + notifyProgress('updated', changedKeyIdentifier); + } else { + notifyProgress('skipped', observationId(key)); } + } else { + notifyProgress('skipped', observationId(key)); } } catch (e) { // A decrypt failure is usually not a wrong password — the keychain may be @@ -192,6 +219,7 @@ export class Keychains implements IKeychains { ) { throw e; } + notifyProgress('skipped', observationId(key)); } } if (result.nextBatchPrevId) {