From fc6965be053a3e8b227c5c2b334f85b2cae19315 Mon Sep 17 00:00:00 2001 From: bitgobot Date: Thu, 29 Oct 2026 17:33:30 +0000 Subject: [PATCH] feat(sdk-core): reshare wallets to recovering spenders Add admin and recovery filters and report per-recipient outcomes. This lets custody lightning wallet members regain spending access after their userAuth key is reset without one blocked share stopping the remaining recipients. BREAKING CHANGE: recipient share failures are returned rather than thrown; callers must inspect the failed results. Ticket: WCN-3894 Session-Id: 3e80d32f-b610-4dde-bcdf-73d89ebe4147 Task-Id: ce5e1cc0-4061-4fb5-a492-2234993dadd8 --- modules/bitgo/test/v2/unit/wallets.ts | 95 ++++++++++++++++++- modules/sdk-core/CHANGELOG.md | 4 + modules/sdk-core/src/bitgo/wallet/iWallet.ts | 2 +- modules/sdk-core/src/bitgo/wallet/iWallets.ts | 16 ++++ modules/sdk-core/src/bitgo/wallet/wallets.ts | 58 ++++++----- 5 files changed, 146 insertions(+), 29 deletions(-) diff --git a/modules/bitgo/test/v2/unit/wallets.ts b/modules/bitgo/test/v2/unit/wallets.ts index da8330b8084..3c9e087c581 100644 --- a/modules/bitgo/test/v2/unit/wallets.ts +++ b/modules/bitgo/test/v2/unit/wallets.ts @@ -3777,7 +3777,9 @@ describe('V2 Wallets:', function () { }); // Mock reshareWalletWithSpenders (called for keychainOverrideRequired cases) - const reshareStub = sinon.stub(Wallets.prototype, 'reshareWalletWithSpenders').resolves(); + const reshareStub = sinon + .stub(Wallets.prototype, 'reshareWalletWithSpenders') + .resolves({ shared: [], failed: [] }); const result = await wallets.bulkUpdateWalletShare({ shares: [{ walletShareId: 'share1', status: 'accept' }], @@ -3846,7 +3848,9 @@ describe('V2 Wallets:', function () { }); // Mock reshareWalletWithSpenders - const reshareStub = sinon.stub(Wallets.prototype, 'reshareWalletWithSpenders').resolves(); + const reshareStub = sinon + .stub(Wallets.prototype, 'reshareWalletWithSpenders') + .resolves({ shared: [], failed: [] }); const result = await ofcWallets.bulkUpdateWalletShare({ shares: [{ walletShareId: 'share1', status: 'accept' }], @@ -4384,7 +4388,7 @@ describe('V2 Wallets:', function () { rejectedWalletShares: [], walletShareUpdateErrors: [], }); - sinon.stub(Wallets.prototype, 'reshareWalletWithSpenders').resolves(); + sinon.stub(Wallets.prototype, 'reshareWalletWithSpenders').resolves({ shared: [], failed: [] }); await ofcWallets.bulkUpdateWalletShare({ shares: [{ walletShareId: shareId, status: 'accept' }], @@ -4436,6 +4440,88 @@ describe('V2 Wallets:', function () { return { walletObj, users }; } + function stubRecoveryRecipients() { + const users = [ + { user: 'spender-recovery', permissions: ['view', 'spend'], needsRecovery: true }, + { user: 'spender-ready', permissions: ['view', 'spend'], needsRecovery: false }, + { user: 'spender-unspecified', permissions: ['view', 'spend'] }, + { user: 'admin-recovery', permissions: ['admin', 'view', 'spend'], needsRecovery: true }, + { user: 'admin-viewer', permissions: ['admin', 'view'], needsRecovery: true }, + { user: 'missing-enterprise-user', permissions: ['spend'], needsRecovery: true }, + ]; + sinon.stub(Wallets.prototype, 'get').resolves( + new Wallet(bitgo, bitgo.coin('tsol'), { + id: 'recovery-wallet', + coin: 'tsol', + enterprise: 'recovery-enterprise', + users, + keys: ['userKeyId', 'backupKeyId', 'bitgoKeyId'], + }) + ); + nock(bgUrl) + .get('/api/v1/enterprise/recovery-enterprise/user') + .reply(200, { + adminUsers: users.slice(3, 5).map((u) => ({ id: u.user, email: { email: `${u.user}@example.com` } })), + nonAdminUsers: users.slice(0, 3).map((u) => ({ id: u.user, email: { email: `${u.user}@example.com` } })), + }); + sinon.stub(Wallet.prototype, 'getDecryptedKeychainForSharing').resolves({ prv: 'p', pub: 'q' }); + return sinon.stub(Wallet.prototype, 'shareWallet').resolves({ shared: true }); + } + + it('defaults to non-admin spenders regardless of recovery status', async function () { + stubRecoveryRecipients(); + const result = await wallets.reshareWalletWithSpenders('recovery-wallet', 'pw'); + assert.deepStrictEqual(result, { + shared: ['spender-recovery', 'spender-ready', 'spender-unspecified'], + failed: [], + }); + }); + + it('includes admin spenders when requested', async function () { + stubRecoveryRecipients(); + const result = await wallets.reshareWalletWithSpenders('recovery-wallet', 'pw', undefined, { + includeAdmins: true, + }); + assert.deepStrictEqual(result.shared, [ + 'spender-recovery', + 'spender-ready', + 'spender-unspecified', + 'admin-recovery', + ]); + assert.deepStrictEqual(result.failed, []); + }); + + it('filters for recovery without including admins by default', async function () { + stubRecoveryRecipients(); + const result = await wallets.reshareWalletWithSpenders('recovery-wallet', 'pw', undefined, { + onlyNeedsRecovery: true, + }); + assert.deepStrictEqual(result, { shared: ['spender-recovery'], failed: [] }); + }); + + it('filters for users needing recovery, including admins only when requested', async function () { + stubRecoveryRecipients(); + const result = await wallets.reshareWalletWithSpenders('recovery-wallet', 'pw', undefined, { + includeAdmins: true, + onlyNeedsRecovery: true, + }); + assert.deepStrictEqual(result, { shared: ['spender-recovery', 'admin-recovery'], failed: [] }); + }); + + it('collects recipient errors and continues sharing', async function () { + const share = stubRecoveryRecipients(); + share.onFirstCall().rejects(new Error('active share already exists')); + share.onSecondCall().callsFake(() => Promise.reject('pending share already exists')); + const result = await wallets.reshareWalletWithSpenders('recovery-wallet', 'pw'); + assert.deepStrictEqual(result, { + shared: ['spender-unspecified'], + failed: [ + { userId: 'spender-recovery', error: 'active share already exists' }, + { userId: 'spender-ready', error: 'pending share already exists' }, + ], + }); + }); + it('decrypts the wallet keychain once and threads it into every shareWallet call', async function () { const walletId = 'reshare-wallet-1'; const enterprise = 'ent-1'; @@ -4511,7 +4597,8 @@ describe('V2 Wallets:', function () { const decryptStub = sinon.stub(Wallet.prototype, 'getDecryptedKeychainForSharing'); const shareWalletStub = sinon.stub(Wallet.prototype, 'shareWallet'); - await wallets.reshareWalletWithSpenders(walletId, 'pw'); + const result = await wallets.reshareWalletWithSpenders(walletId, 'pw'); + assert.deepStrictEqual(result, { shared: [], failed: [] }); // No decrypt burned, no shares attempted decryptStub.called.should.equal(false); diff --git a/modules/sdk-core/CHANGELOG.md b/modules/sdk-core/CHANGELOG.md index e1867587dbb..edf96b58f0d 100644 --- a/modules/sdk-core/CHANGELOG.md +++ b/modules/sdk-core/CHANGELOG.md @@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +### Unreleased + +* Add recipient filters and per-user results to `reshareWalletWithSpenders`. Setup and key decryption errors still reject the call. + # [40.3.0](https://github.com/BitGo/BitGoJS/compare/%40bitgo%2Fsdk-core%4040.2.0...%40bitgo%2Fsdk-core%4040.3.0) (2026-10-27) ### Bug Fixes diff --git a/modules/sdk-core/src/bitgo/wallet/iWallet.ts b/modules/sdk-core/src/bitgo/wallet/iWallet.ts index 107962047e9..a99edd6e028 100644 --- a/modules/sdk-core/src/bitgo/wallet/iWallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/iWallet.ts @@ -1142,7 +1142,7 @@ export interface WalletData { tokens?: Record[]; nfts?: { [contractAddressOrToken: string]: NftBalance }; unsupportedNfts?: { [contractAddress: string]: NftBalance }; - users?: WalletUser[]; + users?: (WalletUser & { needsRecovery?: boolean })[]; evmKeyRingReferenceWalletId?: string; isParent?: boolean; enabledChildChains?: string[]; diff --git a/modules/sdk-core/src/bitgo/wallet/iWallets.ts b/modules/sdk-core/src/bitgo/wallet/iWallets.ts index d33e4efae43..748121e13f5 100644 --- a/modules/sdk-core/src/bitgo/wallet/iWallets.ts +++ b/modules/sdk-core/src/bitgo/wallet/iWallets.ts @@ -484,7 +484,23 @@ export interface BulkAcceptShareResponse { acceptedWalletShares: AcceptShareResponse[]; } +export interface ReshareWalletOptions { + includeAdmins?: boolean; + onlyNeedsRecovery?: boolean; +} + +export interface ReshareWalletResult { + shared: string[]; + failed: { userId: string; error: string }[]; +} + export interface IWallets { + reshareWalletWithSpenders( + walletId: string, + userPassword: string, + encryptionVersion?: EncryptionVersion, + opts?: ReshareWalletOptions + ): Promise; get(params?: GetWalletOptions): Promise; list(params?: ListWalletOptions): Promise<{ wallets: IWallet[] }>; add(params?: AddWalletOptions): Promise; diff --git a/modules/sdk-core/src/bitgo/wallet/wallets.ts b/modules/sdk-core/src/bitgo/wallet/wallets.ts index 772482dbf3f..8bce188a233 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallets.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallets.ts @@ -39,6 +39,8 @@ import { IWallets, LightningWalletWithKeychains, ListWalletOptions, + ReshareWalletOptions, + ReshareWalletResult, UpdateShareOptions, WalletShares, WalletWithKeychains, @@ -1110,12 +1112,16 @@ export class Wallets implements IWallets { * Re-share wallet with existing spenders of the wallet * @param walletId * @param userPassword + * @param encryptionVersion + * @param opts - recipient selection options + * @returns successful user IDs and individual share failures */ async reshareWalletWithSpenders( walletId: string, userPassword: string, - encryptionVersion?: EncryptionVersion - ): Promise { + encryptionVersion?: EncryptionVersion, + opts?: ReshareWalletOptions + ): Promise { const wallet = await this.get({ id: walletId }); if (!wallet?._wallet?.enterprise) { throw new Error('Enterprise not found for the wallet'); @@ -1129,16 +1135,17 @@ export class Wallets implements IWallets { [...enterpriseUsersResponse?.adminUsers, ...enterpriseUsersResponse?.nonAdminUsers].map((obj) => [obj.id, obj]) ); - if (!wallet._wallet.users) { - return; - } - - const spenders = wallet._wallet.users.filter( - (user) => user.permissions.includes('spend') && !user.permissions.includes('admin') && usersMap.has(user.user) + const result: ReshareWalletResult = { shared: [], failed: [] }; + const spenders = (wallet._wallet.users ?? []).filter( + (user) => + user.permissions.includes('spend') && + (opts?.includeAdmins || !user.permissions.includes('admin')) && + (!opts?.onlyNeedsRecovery || user.needsRecovery === true) && + usersMap.has(user.user) ); if (spenders.length === 0) { - return; + return result; } // Decrypt the wallet keychain once and thread the plaintext into each shareWallet call. @@ -1152,27 +1159,30 @@ export class Wallets implements IWallets { if (!(e instanceof MissingEncryptedKeychainError)) { throw e; } - // Cold wallet: no encrypted keychain to decrypt. Fall through and let shareWallet handle - // it per-recipient (which will also throw MissingEncryptedKeychainError but at a point - // where the caller expects it). } // Sequential loop to bound concurrent Argon2id WASM instances on the per-recipient ECDH // encrypt side (each recipient's secret is unique so Promise.all could OOM at scale). for (const user of spenders) { - const userObject = usersMap.get(user.user); - assert(userObject, `User ${user.user} not found in enterprise users map`); - const shareOptions: ShareWalletOptions = { - permissions: user.permissions.join(','), - walletPassphrase: userPassword, - email: userObject.email.email, - reshare: true, - skipKeychain: false, - encryptionVersion, - decryptedKeychain, - }; - await wallet.shareWallet(shareOptions); + try { + const userObject = usersMap.get(user.user); + assert(userObject, `User ${user.user} not found in enterprise users map`); + const shareOptions: ShareWalletOptions = { + permissions: user.permissions.join(','), + walletPassphrase: userPassword, + email: userObject.email.email, + reshare: true, + skipKeychain: false, + encryptionVersion, + decryptedKeychain, + }; + await wallet.shareWallet(shareOptions); + result.shared.push(user.user); + } catch (e) { + result.failed.push({ userId: user.user, error: e instanceof Error ? e.message : String(e) }); + } } + return result; } /**