From c3157fb38c8751d0b0ba0ac218b3423bd828bef3 Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Mon, 19 Jun 2023 17:08:51 +0300 Subject: [PATCH 1/6] Prevent proxy all HTML of bfx api error --- workers/loc.api/responder/index.js | 51 ++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/workers/loc.api/responder/index.js b/workers/loc.api/responder/index.js index fb1eac47..1230bdc7 100644 --- a/workers/loc.api/responder/index.js +++ b/workers/loc.api/responder/index.js @@ -15,8 +15,36 @@ const { AuthError } = require('../errors') +const _htmlRegExp = //i +const _htmlTitleRegExp = /(?.*)<\/title.*>/i + const JSON_RPC_VERSION = '2.0' +const _isHtml = (res) => (_htmlRegExp.test(res)) + +const _findHtmlTitle = (res) => ( + res?.match(_htmlTitleRegExp).groups?.body ?? 'HTML title not found' +) + +const _getBfxApiErrorMetadata = (err) => { + if (!err?.status) { + return null + } + + const isHtml = _isHtml(err.response) + const body = isHtml + ? _findHtmlTitle(err.response) + : err.response ?? 'Response is not abailable' + + return { + bfxApiStatus: err.status, + bfxApiStatusText: err.statustext ?? 'Status text is not abailable', + bfxApiRawBodyCode: err.code ?? 'Code is not abailable', + isBfxApiRawBodyResponseHtml: isHtml ? 'Yes' : 'No', + bfxApiRawBodyResponse: body + } +} + const _prepareErrorData = (err, name) => { const { message = 'ERR_ERROR_HAS_OCCURRED' } = err const _name = name @@ -29,7 +57,10 @@ const _prepareErrorData = (err, name) => { ? `\n - STATUS_MESSAGE: ${err.statusMessage}` : '' const _data = err.data - ? `\n - DATA: ${JSON.stringify(err.data)}` + ? `\n - DATA: ${JSON.stringify(err.data, null, 2) + .split('\n') + .map((v, i) => (i === 0 ? v : ` ${v}`)) + .join('\n')}` : '' const stackTrace = (err.stack || err) ? `\n - STACK_TRACE ${err.stack || err}` @@ -105,20 +136,20 @@ const _getErrorMetadata = (args, err) => { data = null } = errWithMetadata - const _message = err?.status - ? `${message} - - BFX_API_STATUS: ${err.status} - - BFX_API_STATUS_TEXT: ${err.statustext ?? 'Status text is not abailable'} - - BFX_API_RAW_BODY_CODE: ${err.code ?? 'Code is not abailable'} - - BFX_API_RAW_BODY_RESPONSE: ${err.response ?? 'Response is not abailable'}` - : message + const bfxApiErrorMessage = _getBfxApiErrorMetadata(err) + const extendedData = bfxApiErrorMessage + ? { + bfxApiErrorMessage, + ...data + } + : data const error = Object.assign( errWithMetadata, { statusCode: code, - statusMessage: _message, - data + statusMessage: message, + data: extendedData } ) From 2a2f14f48c1e652a53d2b65c90d12c4f522b8bd7 Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Tue, 20 Jun 2023 16:29:30 +0300 Subject: [PATCH 2/6] Add forbidden resource error tester --- workers/loc.api/helpers/api-errors-testers.js | 7 ++++++- workers/loc.api/helpers/index.js | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/workers/loc.api/helpers/api-errors-testers.js b/workers/loc.api/helpers/api-errors-testers.js index 71d1a840..52cff8b8 100644 --- a/workers/loc.api/helpers/api-errors-testers.js +++ b/workers/loc.api/helpers/api-errors-testers.js @@ -67,6 +67,10 @@ const isTempUnavailableError = (err) => { return /temporarily_unavailable/.test(_getErrorString(err)) } +const isForbiddenError = (err) => { + return /forbidden/i.test(_getErrorString(err)) +} + const isENetError = (err) => ( isENetUnreachError(err) || isEConnResetError(err) || @@ -96,5 +100,6 @@ module.exports = { isEHostUnreachError, isEProtoError, isTempUnavailableError, - isENetError + isENetError, + isForbiddenError } diff --git a/workers/loc.api/helpers/index.js b/workers/loc.api/helpers/index.js index ea3898d8..79699527 100644 --- a/workers/loc.api/helpers/index.js +++ b/workers/loc.api/helpers/index.js @@ -32,7 +32,8 @@ const { isEConnRefusedError, isENotFoundError, isESocketTimeoutError, - isENetError + isENetError, + isForbiddenError } = require('./api-errors-testers') const { accountCache, @@ -73,6 +74,7 @@ module.exports = { isENotFoundError, isESocketTimeoutError, isENetError, + isForbiddenError, accountCache, parseFields, parseLoginsExtraDataFields, From 23f8b2a3cfc7ae6acdf1c2268c403c138d1e1fe5 Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Tue, 20 Jun 2023 16:31:05 +0300 Subject: [PATCH 3/6] Handle forbidden resource error --- workers/loc.api/responder/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/workers/loc.api/responder/index.js b/workers/loc.api/responder/index.js index 1230bdc7..89a8aa37 100644 --- a/workers/loc.api/responder/index.js +++ b/workers/loc.api/responder/index.js @@ -7,7 +7,8 @@ const { isRateLimitError, isNonceSmallError, isUserIsNotMerchantError, - isSymbolInvalidError + isSymbolInvalidError, + isForbiddenError } = require('../helpers') const { @@ -124,6 +125,12 @@ const _getErrorWithMetadataForNonBaseError = (args, err) => { return err } + if (isForbiddenError(err)) { + err.statusCode = 403 + err.statusMessage = 'Forbidden' + + return err + } return err } From 898431cefb604abc0adb454f56dfb2a2a1dc69d6 Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Tue, 20 Jun 2023 16:32:24 +0300 Subject: [PATCH 4/6] Add mocked html body 403 response --- .../responder/__test__/mockedHtmlBody403.html | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 workers/loc.api/responder/__test__/mockedHtmlBody403.html diff --git a/workers/loc.api/responder/__test__/mockedHtmlBody403.html b/workers/loc.api/responder/__test__/mockedHtmlBody403.html new file mode 100644 index 00000000..1303c4c0 --- /dev/null +++ b/workers/loc.api/responder/__test__/mockedHtmlBody403.html @@ -0,0 +1,73 @@ + + + + + + + + + Access denied | api.staging.bitfinex.com used Cloudflare to restrict access + + + + + + + + + +
+ +
+
+

+ Error + 1006 +

+ Ray ID: + 1234567890 • + 2023-01-01 + 12:00:00 UTC +

Access denied

+
+
+
+

What happened?

+

The owner of this website (api.staging.bitfinex.com) has banned your IP address (12.123.123.12).

+
+
+ + +
+
+ + From 6d889f945ca7cb0e000246a93495774da90375cd Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Tue, 20 Jun 2023 16:34:43 +0300 Subject: [PATCH 5/6] Add test case for response service for handling html error --- .../responder/__test__/responder.spec.js | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 workers/loc.api/responder/__test__/responder.spec.js diff --git a/workers/loc.api/responder/__test__/responder.spec.js b/workers/loc.api/responder/__test__/responder.spec.js new file mode 100644 index 00000000..d19ac54c --- /dev/null +++ b/workers/loc.api/responder/__test__/responder.spec.js @@ -0,0 +1,87 @@ +'use strict' + +const fs = require('fs') +const path = require('path') +const { assert } = require('chai') + +require('reflect-metadata') +const responder = require('../index') +const AbstractWSEventEmitter = require('../../abstract.ws.event.emitter') + +const JSON_RPC_VERSION = '2.0' +const name = 'mockedResponderCall' +const args = { id: 5 } +const mockedHtmlBody403 = fs.readFileSync(path.join( + __dirname, 'mockedHtmlBody403.html' +)) + +const _makeApiError = (resp, rawBody) => { + const err = new Error(`HTTP code ${resp.status} ${resp.statusText || ''}`) + err.status = resp.status + err.statustext = resp.statusText + try { + const [, code, response] = JSON.parse(rawBody) + err.code = code + err.response = response + } catch (_err) { + err.response = rawBody + } + + return err +} + +describe('Responder service', () => { + let mockedResponder = null + + before(function () { + const mockedContainer = {} + const mockedLogger = { + debug (message) { + assert.isString(message) + }, + error (message) { + assert.isString(message) + } + } + const mockedWsEventEmitterFactory = () => new (class WSEventEmitter extends AbstractWSEventEmitter { + emitBfxUnamePwdAuthRequiredToOne (data, auth) { + assert.isObject(data) + assert.isObject(auth) + } + })() + + mockedResponder = responder( + mockedContainer, + mockedLogger, + mockedWsEventEmitterFactory + ) + }) + + it('handle HTML error', async function () { + await mockedResponder(async () => { + throw _makeApiError({ + status: 403, + statustext: 'Forbidden' + }, mockedHtmlBody403) + }, name, args, (err, res) => { + assert.isNull(err) + + assert.isObject(res) + assert.propertyVal(res, 'id', 5) + assert.propertyVal(res, 'jsonrpc', JSON_RPC_VERSION) + assert.isObject(res.error) + assert.propertyVal(res.error, 'code', 403) + assert.propertyVal(res.error, 'message', 'Forbidden') + assert.isObject(res.error.data) + assert.isObject(res.error.data.bfxApiErrorMessage) + + assert.containsAllKeys(res.error.data.bfxApiErrorMessage, [ + 'bfxApiStatus', + 'bfxApiStatusText', + 'bfxApiRawBodyCode', + 'isBfxApiRawBodyResponseHtml', + 'bfxApiRawBodyResponse' + ]) + }) + }) +}) From d760442dabba3fc5667c43800a8266a8ece8e673 Mon Sep 17 00:00:00 2001 From: Vladimir Voronkov Date: Wed, 21 Jun 2023 08:22:08 +0300 Subject: [PATCH 6/6] Add response error test cases for plain text --- .../responder/__test__/responder.spec.js | 91 ++++++++++++++----- 1 file changed, 69 insertions(+), 22 deletions(-) diff --git a/workers/loc.api/responder/__test__/responder.spec.js b/workers/loc.api/responder/__test__/responder.spec.js index d19ac54c..4bcf8f08 100644 --- a/workers/loc.api/responder/__test__/responder.spec.js +++ b/workers/loc.api/responder/__test__/responder.spec.js @@ -30,6 +30,27 @@ const _makeApiError = (resp, rawBody) => { return err } +const _errorResponseTestCases = (opts) => (err, res) => { + assert.isNull(err) + + assert.isObject(res) + assert.propertyVal(res, 'id', 5) + assert.propertyVal(res, 'jsonrpc', JSON_RPC_VERSION) + assert.isObject(res.error) + assert.propertyVal(res.error, 'code', opts.code) + assert.propertyVal(res.error, 'message', opts.message) + assert.isObject(res.error.data) + assert.isObject(res.error.data.bfxApiErrorMessage) + + assert.containsAllKeys(res.error.data.bfxApiErrorMessage, [ + 'bfxApiStatus', + 'bfxApiStatusText', + 'bfxApiRawBodyCode', + 'isBfxApiRawBodyResponseHtml', + 'bfxApiRawBodyResponse' + ]) +} + describe('Responder service', () => { let mockedResponder = null @@ -59,29 +80,55 @@ describe('Responder service', () => { it('handle HTML error', async function () { await mockedResponder(async () => { - throw _makeApiError({ - status: 403, - statustext: 'Forbidden' - }, mockedHtmlBody403) - }, name, args, (err, res) => { - assert.isNull(err) + throw _makeApiError( + { + status: 403, + statustext: 'Forbidden' + }, + mockedHtmlBody403 + ) + }, name, args, _errorResponseTestCases({ + code: 403, + message: 'Forbidden' + })) + }) - assert.isObject(res) - assert.propertyVal(res, 'id', 5) - assert.propertyVal(res, 'jsonrpc', JSON_RPC_VERSION) - assert.isObject(res.error) - assert.propertyVal(res.error, 'code', 403) - assert.propertyVal(res.error, 'message', 'Forbidden') - assert.isObject(res.error.data) - assert.isObject(res.error.data.bfxApiErrorMessage) + it('handle plain error status text', async function () { + await mockedResponder(async () => { + throw _makeApiError( + { + status: 500, + statusText: 'ERR_INVOICE_LIST: ERR_PAY_USER_NOT_MERCHANT' + }, + null + ) + }, name, args, _errorResponseTestCases({ + code: 409, + message: 'Pay invoice list error, the user is not a merchant' + })) + }) - assert.containsAllKeys(res.error.data.bfxApiErrorMessage, [ - 'bfxApiStatus', - 'bfxApiStatusText', - 'bfxApiRawBodyCode', - 'isBfxApiRawBodyResponseHtml', - 'bfxApiRawBodyResponse' - ]) - }) + it('handle error in body', async function () { + await mockedResponder(async () => { + throw _makeApiError( + {}, + JSON.stringify([500, 'ERR_INVOICE_LIST: ERR_PAY_USER_NOT_MERCHANT']) + ) + }, name, args, _errorResponseTestCases({ + code: 409, + message: 'Pay invoice list error, the user is not a merchant' + })) + }) + + it('handle plain error text in raw body', async function () { + await mockedResponder(async () => { + throw _makeApiError( + {}, + 'ERR_INVOICE_LIST: ERR_PAY_USER_NOT_MERCHANT' + ) + }, name, args, _errorResponseTestCases({ + code: 409, + message: 'Pay invoice list error, the user is not a merchant' + })) }) })