diff --git a/src/__tests__/api/advancedWalletManager/kmsClient.test.ts b/src/__tests__/api/advancedWalletManager/kmsClient.test.ts new file mode 100644 index 00000000..a112b4d1 --- /dev/null +++ b/src/__tests__/api/advancedWalletManager/kmsClient.test.ts @@ -0,0 +1,112 @@ +import { AppMode, AdvancedWalletManagerConfig, TlsMode } from '../../../initConfig'; +import { app as expressApp } from '../../../advancedWalletManagerApp'; + +import express from 'express'; +import nock from 'nock'; +import 'should'; +import * as request from 'supertest'; + +describe('postMpcV2Key', () => { + let cfg: AdvancedWalletManagerConfig; + let app: express.Application; + let agent: request.SuperAgentTest; + + // test config + const kmsUrl = 'http://kms.invalid'; + const coin = 'tsol'; + const accessToken = 'test-token'; + + before(() => { + // nock config + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + + // app config + cfg = { + appMode: AppMode.ADVANCED_WALLET_MANAGER, + port: 0, // Let OS assign a free port + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + kmsUrl: kmsUrl, + tlsMode: TlsMode.DISABLED, + allowSelfSigned: true, + }; + + // app setup + app = expressApp(cfg); + agent = request.agent(app); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + it('should bubble up 400 KMS errors', async () => { + nock(kmsUrl).post(/.*/).reply(400, { message: 'This is an error message' }).persist(); + + const response = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(400); + response.body.should.have.property('error', 'BadRequestError'); + response.body.should.have.property('details', 'This is an error message'); + }); + + it('should bubble up 404 KMS errors', async () => { + nock(kmsUrl).post(/.*/).reply(404, { message: 'This is an error message' }).persist(); + + const response = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(404); + response.body.should.have.property('error', 'NotFoundError'); + response.body.should.have.property('details', 'This is an error message'); + }); + + it('should bubble up 409 KMS errors', async () => { + nock(kmsUrl).post(/.*/).reply(409, { message: 'This is an error message' }).persist(); + + const response = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(409); + response.body.should.have.property('error', 'ConflictError'); + response.body.should.have.property('details', 'This is an error message'); + }); + + it('should bubble up 500 KMS errors', async () => { + nock(kmsUrl).post(/.*/).reply(500, { message: 'This is an error message' }).persist(); + + const response = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(500); + response.body.should.have.property('error', 'Internal Server Error'); + response.body.should.have.property('details', 'This is an error message'); + }); + + it('should handle unexpected KMS errors', async () => { + nock(kmsUrl).post(/.*/).reply(502, { message: 'Unexpected error' }).persist(); + + const response = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(500); + response.body.should.have.property('error', 'Internal Server Error'); + response.body.should.have.property( + 'details', + 'KMS returned unexpected response. 502: Unexpected error', + ); + }); +}); diff --git a/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts b/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts index 118ce5fa..2abebdf2 100644 --- a/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts +++ b/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts @@ -155,7 +155,9 @@ describe('recoveryMpcV2', async () => { signatureResponse.status.should.equal(400); signatureResponse.body.should.have.property('error'); - signatureResponse.body.error.should.startWith( + signatureResponse.body.error.should.equal('BadRequestError'); + signatureResponse.body.should.have.property('details'); + signatureResponse.body.details.should.startWith( 'Failed to construct eth transaction from message hex', ); }); diff --git a/src/__tests__/api/master/recoveryWalletMpcV2.test.ts b/src/__tests__/api/master/recoveryWalletMpcV2.test.ts index d468670f..e4ffcfdf 100644 --- a/src/__tests__/api/master/recoveryWalletMpcV2.test.ts +++ b/src/__tests__/api/master/recoveryWalletMpcV2.test.ts @@ -202,7 +202,9 @@ describe('MBE mpcv2 recovery', () => { response.status.should.equal(422); response.body.should.have.property('error'); - response.body.error.should.equal( + response.body.error.should.equal('ValidationError'); + response.body.should.have.property('details'); + response.body.details.should.equal( 'ECDSA ETH-like recovery specific parameters are required for MPC recovery', ); }); diff --git a/src/__tests__/api/master/sendMany.test.ts b/src/__tests__/api/master/sendMany.test.ts index e6df4599..9d99e75b 100644 --- a/src/__tests__/api/master/sendMany.test.ts +++ b/src/__tests__/api/master/sendMany.test.ts @@ -5,7 +5,7 @@ import * as request from 'supertest'; import nock from 'nock'; import { app as expressApp } from '../../../masterBitGoExpressApp'; import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; -import { ApiResponseError, Environments, Wallet } from '@bitgo-beta/sdk-core'; +import { Environments, Wallet } from '@bitgo-beta/sdk-core'; import { Tbtc } from '@bitgo-beta/sdk-coin-btc'; import assert from 'assert'; @@ -747,15 +747,11 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => { const verifyStub = sinon.stub(Tbtc.prototype, 'verifyTransaction').resolves(true); - // Mock advanced wallet manager sign request to return an error - const signNock = nock(advancedWalletManagerUrl) - .post(`/api/${coin}/multisig/sign`) - .replyWithError( - new ApiResponseError('Custom API error', 500, { - error: 'Custom API error', - requestId: 'test-request-id', - }), - ); + // Mock enclaved express sign request to return an error + const signNock = nock(advancedWalletManagerUrl).post(`/api/${coin}/multisig/sign`).reply(500, { + error: 'Internal Server Error', + details: 'Custom API error details', + }); const response = await agent .post(`/api/${coin}/wallet/${walletId}/sendMany`) @@ -775,11 +771,8 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => { response.status.should.equal(500); response.body.should.have.property('error'); response.body.should.have.property('details'); - response.body.error.should.equal('BitGoApiResponseError'); - response.body.details.should.deepEqual({ - error: 'Custom API error', - requestId: 'test-request-id', - }); + response.body.error.should.equal('Internal Server Error'); + response.body.details.should.deepEqual('Custom API error details'); walletGetNock.done(); keychainGetNock.done(); diff --git a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts index fd840c30..84e2f6e4 100644 --- a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts +++ b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts @@ -17,12 +17,8 @@ import { DklsDkg, DklsTypes } from '@bitgo-beta/sdk-lib-mpc'; import express from 'express'; import * as t from 'io-ts'; -import { - BadRequestResponse, - InternalServerErrorResponse, - NotImplementedResponse, - UnprocessableEntityResponse, -} from '../../shared/errors'; +import coinFactory from '../../shared/coinFactory'; +import { ErrorResponses, NotImplementedError } from '../../shared/errors'; import { postIndependentKey } from '../../api/advancedWalletManager/handlers/postIndependentKey'; import { recoveryMultisigTransaction } from '../../api/advancedWalletManager/handlers/recoveryMultisigTransaction'; @@ -39,8 +35,6 @@ import { ecdsaMPCv2Finalize } from '../../api/advancedWalletManager/handlers/ecd import { ecdsaMPCv2Recovery } from '../../api/advancedWalletManager/handlers/ecdsaMPCv2Recovery'; import { signEddsaRecoveryTransaction } from '../../api/advancedWalletManager/handlers/signEddsaRecoveryTransaction'; import { isEddsaCoin } from '../../shared/coinUtils'; -import { MethodNotImplementedError } from '@bitgo-beta/sdk-core'; -import coinFactory from '../../shared/coinFactory'; // Request type for /key/independent endpoint const IndependentKeyRequest = { @@ -52,7 +46,7 @@ const IndependentKeyRequest = { const IndependentKeyResponse: HttpResponse = { // TODO: Define proper response type 200: t.any, - ...InternalServerErrorResponse, + ...ErrorResponses, }; // Request type for /multisig/sign endpoint @@ -66,7 +60,7 @@ const SignMultisigRequest = { const SignMultisigResponse: HttpResponse = { // TODO: Define proper response type for signed multisig transaction 200: t.any, - ...InternalServerErrorResponse, + ...ErrorResponses, }; // Request type for /multisig/recovery endpoint @@ -83,7 +77,7 @@ const RecoveryMultisigResponse: HttpResponse = { 200: t.type({ txHex: t.string, }), // the full signed tx - ...InternalServerErrorResponse, + ...ErrorResponses, }; const RecoveryMpcRequest = { @@ -105,7 +99,7 @@ const RecoveryMpcResponse: HttpResponse = { 200: t.type({ txHex: t.string, }), // the full signed tx - ...InternalServerErrorResponse, + ...ErrorResponses, }; // Request type for /mpc/sign endpoint @@ -163,7 +157,7 @@ const SignMpcResponse: HttpResponse = { signatureShareRound3: t.any, }), ]), - ...InternalServerErrorResponse, + ...ErrorResponses, }; const KeyShare = { @@ -373,10 +367,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: t.type(MpcInitializeResponse), - 500: t.type({ - error: t.string, - details: t.string, - }), + ...ErrorResponses, }, description: 'Initialize MPC for EdDSA key generation', }), @@ -414,10 +405,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: MpcFinalizeResponseType, - 500: t.type({ - error: t.string, - details: t.string, - }), + ...ErrorResponses, }, description: 'Finalize key generation and confirm commonKeychain', }), @@ -432,10 +420,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: MpcV2InitializeResponseType, - 500: t.type({ - error: t.string, - details: t.string, - }), + ...ErrorResponses, }, description: 'Initialize MPC for EdDSA key generation', }), @@ -450,9 +435,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: MpcV2RoundResponseType, - ...InternalServerErrorResponse, - ...BadRequestResponse, - ...UnprocessableEntityResponse, + ...ErrorResponses, }, description: 'Perform a round in the MPC protocol', }), @@ -467,10 +450,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: MpcV2FinalizeResponseType, - 500: t.type({ - error: t.string, - details: t.string, - }), + ...ErrorResponses, }, description: 'Finalize the MPC protocol', }), @@ -485,9 +465,7 @@ export const AdvancedWalletManagerApiSpec = apiSpec({ }), response: { 200: MpcV2RecoveryResponseType, - ...BadRequestResponse, - ...InternalServerErrorResponse, - ...NotImplementedResponse, + ...ErrorResponses, }, description: 'Recover a MPC transaction', }), @@ -567,7 +545,9 @@ export function createKeyGenRouter( }); return Response.ok(result); } else { - throw new MethodNotImplementedError(); + throw new NotImplementedError( + `MPC recovery is not implemented for ${coin.getFamily()} coins`, + ); } }), ]); diff --git a/src/api/master/clients/advancedWalletManagerClient.ts b/src/api/master/clients/advancedWalletManagerClient.ts index 88654af8..b3b2c6b6 100644 --- a/src/api/master/clients/advancedWalletManagerClient.ts +++ b/src/api/master/clients/advancedWalletManagerClient.ts @@ -1,6 +1,5 @@ import assert from 'assert'; import https from 'https'; -import debug from 'debug'; import superagent from 'superagent'; import { @@ -19,7 +18,12 @@ import { TxRequest, } from '@bitgo-beta/sdk-core'; import { RecoveryTransaction } from '@bitgo-beta/sdk-coin-trx'; -import { ApiClient, buildApiClient, superagentRequestFactory } from '@api-ts/superagent-wrapper'; +import { + ApiClient, + buildApiClient, + DecodeError, + superagentRequestFactory, +} from '@api-ts/superagent-wrapper'; import { OfflineVaultTxInfo, RecoveryInfo, UnsignedSweepTxMPCv2 } from '@bitgo-beta/sdk-coin-eth'; import { MasterExpressConfig, TlsMode } from '../../../shared/types'; @@ -37,8 +41,7 @@ import { } from '../../../advancedWalletManager/routers/advancedWalletManagerApiSpec'; import { FormattedOfflineVaultTxInfo } from '@bitgo-beta/abstract-utxo'; import { RecoveryTxRequest } from '@bitgo-beta/sdk-core'; - -const debugLogger = debug('bitgo:express:awmClient'); +import logger from '../../../logger'; export type InitMpcKeyGenerationParams = { source: 'user' | 'backup'; @@ -188,7 +191,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Recovering MPC for coin: %s', this.coin); + logger.info('Recovering MPC for coin: %s', this.coin); // Extract the required information from the sweep tx using our utility function const tx = params.unsignedSweepPrebuildTx; @@ -216,7 +219,7 @@ export class AdvancedWalletManagerClient { return response.body; } catch (error) { const err = error as Error; - debugLogger('Failed to recover MPC: %s', err.message); + logger.error('Failed to recover MPC: %s', err.message); throw err; } } @@ -255,7 +258,7 @@ export class AdvancedWalletManagerClient { // Build the type-safe API client this.apiClient = buildApiClient(requestFactory, AdvancedWalletManagerApiSpec); - debugLogger('awmClient initialized with URL: %s', this.baseUrl); + logger.info('EnclavedExpressClient initialized with URL: %s', this.baseUrl); } private createHttpsAgent(): https.Agent { @@ -282,7 +285,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Creating independent keychain for coin: %s', this.coin); + logger.info('Creating independent keychain for coin: %s', this.coin); let request = this.apiClient['v1.key.independent'].post({ coin: this.coin, source: params.source, @@ -296,9 +299,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to create independent keychain: %s', err.message); - throw err; + logger.error( + 'Failed to create independent keychain: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -326,9 +331,8 @@ export class AdvancedWalletManagerClient { return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign multisig: %s', err.message); - throw err; + logger.error('Failed to sign multisig: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -338,7 +342,7 @@ export class AdvancedWalletManagerClient { */ async ping(): Promise { try { - debugLogger('Pinging advanced wallet manager service at: %s', this.baseUrl); + logger.info('Pinging enclaved express service at: %s', this.baseUrl); let request = this.apiClient['v1.health.ping'].post({}); if (this.tlsMode === TlsMode.MTLS) { @@ -347,12 +351,14 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); - debugLogger('Advanced Wallet Manager service ping successful'); + logger.info('Enclaved express service ping successful'); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Advanced Wallet Manager service ping failed: %s', err.message); - throw err; + logger.error( + 'Failed to ping enclaved express service: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -361,7 +367,7 @@ export class AdvancedWalletManagerClient { */ async getVersion(): Promise { try { - debugLogger('Getting version information from advanced wallet manager service'); + logger.info('Getting version information from enclaved express service'); let request = this.apiClient['v1.health.version'].get({}); if (this.tlsMode === TlsMode.MTLS) { @@ -370,12 +376,14 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); - debugLogger('Successfully retrieved version information'); + logger.info('Successfully retrieved version information'); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to get version information: %s', err.message); - throw err; + logger.error( + 'Failed to get version information: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -393,14 +401,13 @@ export class AdvancedWalletManagerClient { if (this.tlsMode === TlsMode.MTLS) { request = request.agent(this.createHttpsAgent()); } - debugLogger('Recovering multisig for coin: %s', this.coin); + logger.info('Recovering multisig for coin: %s', this.coin); const res = await request.decodeExpecting(200); return res.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to recover multisig: %s', err.message); - throw err; + logger.error('Failed to recover multisig: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -415,7 +422,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Initializing MPC key generation for coin: %s', this.coin); + logger.info('Initializing MPC key generation for coin: %s', this.coin); let request = this.apiClient['v1.mpc.key.initialize'].post({ coin: this.coin, source: params.source, @@ -430,9 +437,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to initialize MPC key generation: %s', err.message); - throw err; + logger.error( + 'Failed to initialize MPC key generation: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -452,7 +461,7 @@ export class AdvancedWalletManagerClient { ); try { - debugLogger('Finalizing MPC key generation for coin: %s', this.coin); + logger.info('Finalizing MPC key generation for coin: %s', this.coin); let request = this.apiClient['v1.mpc.key.finalize'].post({ coin: this.coin, source: params.source, @@ -476,9 +485,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to finalize MPC key generation: %s', err.message); - throw err; + logger.error( + 'Failed to finalize MPC key generation: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -500,9 +511,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpc commitment: %s', err.message); - throw err; + logger.error( + 'Failed to sign mpc commitment: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -524,9 +537,8 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpc r-share: %s', err.message); - throw err; + logger.error('Failed to sign mpc r-share: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -548,9 +560,8 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpc g-share: %s', err.message); - throw err; + logger.error('Failed to sign mpc g-share: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -565,7 +576,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Initializing MPCv2 key generation for coin: %s', this.coin); + logger.info('Initializing MPCv2 key generation for coin: %s', this.coin); let request = this.apiClient['v1.mpcv2.initialize'].post({ coin: this.coin, source: params.source, @@ -578,9 +589,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to initialize MPCv2 key generation: %s', err.message); - throw err; + logger.error( + 'Failed to initialize MPCv2 key generation: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -602,7 +615,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Executing MPCv2 round %d for coin: %s', params.round, this.coin); + logger.info('Executing MPCv2 round %s for coin: %s', params.round, this.coin); let request = this.apiClient['v1.mpcv2.round'].post({ coin: this.coin, ...params, @@ -615,9 +628,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to execute MPCv2 round: %s', err.message); - throw err; + logger.error( + 'Failed to execute MPCv2 round: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -636,7 +651,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Finalizing MPCv2 key generation for coin: %s', this.coin); + logger.info('Finalizing MPCv2 key generation for coin: %s', this.coin); let request = this.apiClient['v1.mpcv2.finalize'].post({ coin: this.coin, ...params, @@ -649,9 +664,11 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to finalize MPCv2 key generation: %s', err.message); - throw err; + logger.error( + 'Failed to finalize MPCv2 key generation: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } @@ -682,9 +699,8 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpcv2 round 1: %s', err.message); - throw err; + logger.error('Failed to sign MPCv2 round 1: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -715,9 +731,8 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpcv2 round 2: %s', err.message); - throw err; + logger.error('Failed to sign MPCv2 round 2: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -748,9 +763,8 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; } catch (error) { - const err = error as Error; - debugLogger('Failed to sign mpcv2 round 3: %s', err.message); - throw err; + logger.error('Failed to sign MPCv2 round 3: %s', (error as DecodeError).decodedResponse.body); + throw error; } } @@ -763,7 +777,7 @@ export class AdvancedWalletManagerClient { } try { - debugLogger('Signing MPCv2 recovery transaction for coin: %s', this.coin); + logger.info('Recovering MPCv2 wallet for coin: %s', this.coin); let request = this.apiClient['v1.mpcv2.recovery'].post({ coin: this.coin, ...params, @@ -775,10 +789,12 @@ export class AdvancedWalletManagerClient { const response = await request.decodeExpecting(200); return response.body; - } catch (error) { - const err = error as Error; - debugLogger('Failed to sign MPCv2 recovery transction: %s', err.message); - throw err; + } catch (error: any) { + logger.error( + 'Failed to recover MPCv2 wallet: %s', + (error as DecodeError).decodedResponse.body, + ); + throw error; } } } @@ -794,7 +810,7 @@ export function createawmClient( return new AdvancedWalletManagerClient(cfg, coin); } catch (error) { const err = error as Error; - debugLogger('Failed to create advanced wallet manager client: %s', err.message); + logger.error('Failed to create enclaved express client: %s', err.message); return undefined; } } diff --git a/src/api/master/routers/masterBitGoExpressApiSpec.ts b/src/api/master/routers/masterBitGoExpressApiSpec.ts index 807a89d1..0c979482 100644 --- a/src/api/master/routers/masterBitGoExpressApiSpec.ts +++ b/src/api/master/routers/masterBitGoExpressApiSpec.ts @@ -27,12 +27,7 @@ import { handleAccelerate } from '../handlers/handleAccelerate'; import { handleConsolidateUnspents } from '../handlers/handleConsolidateUnspents'; import { handleSignAndSendTxRequest } from '../handlers/handleSignAndSendTxRequest'; import { handleRecoveryConsolidationsOnPrem } from '../handlers/recoveryConsolidationsWallet'; -import { - BadRequestResponse, - InternalServerErrorResponse, - UnprocessableEntityResponse, - NotFoundResponse, -} from '../../../shared/errors'; +import { ErrorResponses } from '../../../shared/errors'; export type ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3; @@ -232,8 +227,7 @@ export function parseBody(req: express.Request, res: express.Response, next: exp const GenerateWalletResponse: HttpResponse = { // TODO: Get type from public types repo 200: t.any, - ...InternalServerErrorResponse, - ...BadRequestResponse, + ...ErrorResponses, }; // Request type for /generate endpoint @@ -294,9 +288,7 @@ export const SendManyRequest = { export const SendManyResponse: HttpResponse = { // TODO: Get type from public types repo / Wallet Platform 200: t.any, - ...BadRequestResponse, - ...NotFoundResponse, - ...InternalServerErrorResponse, + ...ErrorResponses, }; // Request type for /consolidate endpoint @@ -311,8 +303,7 @@ export const ConsolidateRequest = { // Response type for /consolidate endpoint const ConsolidateResponse: HttpResponse = { 200: t.any, - ...BadRequestResponse, // All failed - ...InternalServerErrorResponse, + ...ErrorResponses, }; /** @@ -398,7 +389,7 @@ const AccelerateResponse: HttpResponse = { */ tx: t.string, }), - ...InternalServerErrorResponse, + ...ErrorResponses, }; /** @@ -420,8 +411,7 @@ const RecoveryWalletResponse: HttpResponse = { */ txHex: t.string, }), - ...UnprocessableEntityResponse, - ...InternalServerErrorResponse, + ...ErrorResponses, }; /** @@ -613,7 +603,7 @@ const RecoveryConsolidationsWalletResponse: HttpResponse = { * The exact structure depends on the coin and recovery type. */ 200: t.any, // Complex response structure varies by coin and recovery type - ...InternalServerErrorResponse, + ...ErrorResponses, }; export const ConsolidateUnspentsRequest = { @@ -640,8 +630,7 @@ const ConsolidateUnspentsResponse: HttpResponse = { tx: t.string, txid: t.string, }), - ...BadRequestResponse, - ...InternalServerErrorResponse, + ...ErrorResponses, }; const SignMpcRequest = { @@ -651,7 +640,7 @@ const SignMpcRequest = { const SignMpcResponse: HttpResponse = { 200: t.any, - ...InternalServerErrorResponse, + ...ErrorResponses, }; /** diff --git a/src/kms/kmsClient.ts b/src/kms/kmsClient.ts index 8ddd21c4..96db8841 100644 --- a/src/kms/kmsClient.ts +++ b/src/kms/kmsClient.ts @@ -13,6 +13,7 @@ import { GenerateDataKeyResponse, } from './types/generateDataKey'; import https from 'https'; +import { BadRequestError, ConflictError, NotFoundError } from '../shared/errors'; import { URL } from 'url'; import logger from '../logger'; @@ -46,11 +47,32 @@ export class KmsClient { } this.url = kmsUrlObj.toString().replace(/\/$/, ''); - logger.debug('kmsClient initialized with URL: %s', this.url); + logger.info('kmsClient initialized with URL: %s', this.url); + } + + // Handles http erros from KMS + private errorHandler(error: superagent.ResponseError, errorLog: string) { + logger.error(errorLog, error); + switch (error.status) { + case 400: + throw new BadRequestError(error.response?.body.message); + case 404: + throw new NotFoundError(error.response?.body.message); + case 409: + throw new ConflictError(error.response?.body.message); + case 500: + throw new Error(error.response?.body.message); + default: + throw new Error( + `KMS returned unexpected response.${error.status ? ` ${error.status}` : ''}${ + error.response?.body.message ? `: ${error.response?.body.message}` : '' + }`, + ); + } } async postKey(params: PostKeyParams): Promise { - logger.debug('Posting key to KMS: %O', params); + logger.info('Posting key to KMS with pub: %s and source: %s', params.pub, params.source); // Call KMS to post the key let kmsResponse: any; @@ -59,8 +81,7 @@ export class KmsClient { if (this.agent) req = req.agent(this.agent); kmsResponse = await req; } catch (error: any) { - logger.error('Error posting key to KMS', error); - throw Error(`Failed to post key to KMS: ${error.message}`); + this.errorHandler(error, 'Error posting key to KMS'); } // validate the response @@ -80,7 +101,7 @@ export class KmsClient { } async getKey(params: GetKeyParams): Promise { - logger.debug('Getting key from KMS: %O', params); + logger.info('Getting key from KMS with pub: %s and source: %s', params.pub, params.source); // Call KMS to get the key let kmsResponse: any; @@ -92,8 +113,8 @@ export class KmsClient { if (this.agent) req = req.agent(this.agent); kmsResponse = await req; } catch (error: any) { - logger.error('Error getting key from KMS', error); - throw new Error(`Failed to get key from KMS: ${error.message}`); + console.log('Error getting key from KMS:', error); + this.errorHandler(error, 'Error getting key from KMS'); } // validate the response @@ -112,7 +133,7 @@ export class KmsClient { } async generateDataKey(params: GenerateDataKeyParams): Promise { - logger.debug('Generating data key from KMS: %O', params); + logger.info('Generating data key from KMS with type: %s', params.keyType); // Call KMS to generate the data key let kmsResponse: any; @@ -121,10 +142,7 @@ export class KmsClient { if (this.agent) req = req.agent(this.agent); kmsResponse = await req; } catch (error: any) { - logger.error('Error generating data key from KMS when generating data key', error); - throw new Error( - `Failed to generate data key from KMS when generating data key: ${error.message}`, - ); + this.errorHandler(error, 'Error generating data key from KMS'); } // validate the response @@ -146,7 +164,7 @@ export class KmsClient { } async decryptDataKey(params: DecryptDataKeyParams): Promise { - logger.debug('Decrypting data key from KMS: %O', params); + logger.info('Decrypting data key from KMS'); // Call KMS to decrypt the data key let kmsResponse: any; @@ -155,8 +173,7 @@ export class KmsClient { if (this.agent) req = req.agent(this.agent); kmsResponse = await req; } catch (error: any) { - logger.error('Error decrypting data key from KMS', error); - throw new Error(`Failed to decrypt data key from KMS: ${error.message}`); + this.errorHandler(error, 'Error decrypting data key from KMS'); } // validate the response diff --git a/src/shared/errors.ts b/src/shared/errors.ts index 24e586ee..8f93d9e5 100644 --- a/src/shared/errors.ts +++ b/src/shared/errors.ts @@ -98,7 +98,17 @@ const ErrorResponse = t.type({ details: t.string, }); export const BadRequestResponse = { 400: ErrorResponse }; +export const NotFoundResponse = { 404: ErrorResponse }; +export const ConflictErrorResponse = { 409: ErrorResponse }; export const UnprocessableEntityResponse = { 422: ErrorResponse }; export const InternalServerErrorResponse = { 500: ErrorResponse }; -export const NotFoundResponse = { 404: ErrorResponse }; export const NotImplementedResponse = { 501: ErrorResponse }; + +export const ErrorResponses = { + ...BadRequestResponse, + ...NotFoundResponse, + ...ConflictErrorResponse, + ...UnprocessableEntityResponse, + ...InternalServerErrorResponse, + ...NotImplementedResponse, +}; diff --git a/src/shared/responseHandler.ts b/src/shared/responseHandler.ts index 61fef202..f32b67dd 100644 --- a/src/shared/responseHandler.ts +++ b/src/shared/responseHandler.ts @@ -12,6 +12,7 @@ import { ConflictError, } from './errors'; import logger from '../logger'; +import { DecodeError } from '@api-ts/superagent-wrapper'; // Extend Express Response to include sendEncoded interface EncodedResponse extends ExpressResponse { @@ -43,7 +44,6 @@ export function responseHandler(fn: ServiceFunction(fn: ServiceFunction(fn: ServiceFunction