From ed289c44ba7d9a62a3ec3438482da4ab1576e19f Mon Sep 17 00:00:00 2001 From: usingtechnology <39388115+usingtechnology@users.noreply.github.com> Date: Tue, 2 Jul 2024 13:27:26 -0700 Subject: [PATCH] Enhanced error handling and warning logging for Proxy/External API. (#1404) * Enhanced error handling and warning logging for Proxy/External API. Signed-off-by: Jason Sherman * Use sendStatus and return 400 for malformed calls to proxy. Signed-off-by: Jason Sherman --------- Signed-off-by: Jason Sherman --- app/src/forms/proxy/controller.js | 46 ++++++++++++++++--- app/src/forms/proxy/error.js | 8 ++++ app/src/forms/proxy/service.js | 20 ++++---- app/tests/unit/forms/proxy/controller.spec.js | 26 ++++++++++- docs/chefs-external-api-configuration.md | 19 +++++++- 5 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 app/src/forms/proxy/error.js diff --git a/app/src/forms/proxy/controller.js b/app/src/forms/proxy/controller.js index a2b8865ce..a0a0f2103 100644 --- a/app/src/forms/proxy/controller.js +++ b/app/src/forms/proxy/controller.js @@ -3,6 +3,9 @@ const jwtService = require('../../components/jwtService'); const axios = require('axios'); const { ExternalAPIStatuses } = require('../common/constants'); const Problem = require('api-problem'); +const ProxyServiceError = require('./error'); +const { NotFoundError } = require('objection'); +const log = require('../../components/log')(module.filename); module.exports = { generateProxyHeaders: async (req, res, next) => { @@ -19,22 +22,53 @@ module.exports = { const proxyHeaderInfo = await service.readProxyHeaders(req.headers); // find the specified external api configuration... const extAPI = await service.getExternalAPI(req.headers, proxyHeaderInfo); - if (extAPI.code != ExternalAPIStatuses.APPROVED) { - throw new Problem(407, 'External API has not been approved by CHEFS.'); - } // add path to endpoint url if included in headers... const extUrl = service.createExternalAPIUrl(req.headers, extAPI.endpointUrl); // build list of request headers based on configuration... const extHeaders = await service.createExternalAPIHeaders(extAPI, proxyHeaderInfo); + // check for approval before we call it.. + if (extAPI.code != ExternalAPIStatuses.APPROVED) { + throw new Problem(407, 'External API has not been approved by CHEFS.'); + } let axiosInstance = axios.create({ headers: extHeaders, }); // call the external api - const { data } = await axiosInstance.get(extUrl); + const { data, status } = await axiosInstance.get(extUrl).catch(function (err) { + let message = err.message; + if (err.response) { + // The request was made and the server responded with a status code + // that falls out of the range of 2xx + log.warn(`Error returned from the external API': ${message}`); + } else if (err.request) { + message = 'External API call made, no response received.'; + log.warn(message); + } else { + // Something happened in setting up the request that triggered an Error + log.warn(`Error setting up the external API request: ${message}`); + } + // send a bad gateway, the message should contain the real status + throw new Problem(502, message); + }); // if all good return data - res.status(200).json(data); + res.status(status).json(data); } catch (error) { - next(error); + if (error instanceof ProxyServiceError) { + // making an assumption that the form component making this call + // has not been setup correctly yet. + // formio components will call as soon as the URL is entered while designing. + // calls will fire before the designer has added the headers. + log.warn(error.message); + // send back status 400 Bad Request + res.sendStatus(400); + } else if (error instanceof NotFoundError) { + // may have created formio component before adding the External API config. + log.warn('External API configuration does not exist.'); + // send back status 400 Bad Request + res.sendStatus(400); + } else { + next(error); + } } }, }; diff --git a/app/src/forms/proxy/error.js b/app/src/forms/proxy/error.js new file mode 100644 index 000000000..f096a4f50 --- /dev/null +++ b/app/src/forms/proxy/error.js @@ -0,0 +1,8 @@ +class ProxyServiceError extends Error { + constructor(message) { + super(message); + this.name = 'ProxyServiceError'; + } +} + +module.exports = ProxyServiceError; diff --git a/app/src/forms/proxy/service.js b/app/src/forms/proxy/service.js index 845119648..f6cd334a1 100644 --- a/app/src/forms/proxy/service.js +++ b/app/src/forms/proxy/service.js @@ -1,6 +1,6 @@ const { encryptionService } = require('../../components/encryptionService'); const jwtService = require('../../components/jwtService'); - +const ProxyServiceError = require('./error'); const { ExternalAPI } = require('../../forms/common/models'); const headerValue = (headers, key) => { @@ -21,7 +21,7 @@ const trimTrailingSlashes = (str) => str.replace(/\/+$/g, ''); const service = { generateProxyHeaders: async (payload, currentUser, token) => { if (!payload || !currentUser || !currentUser.idp) { - throw new Error('Cannot generate proxy headers with missing or incomplete parameters'); + throw new ProxyServiceError('Cannot generate proxy headers with missing or incomplete parameters'); } const headerData = { @@ -51,16 +51,20 @@ const service = { const data = JSON.parse(decryptedHeaderData); return data; } catch (error) { - throw new Error(`Could not decrypt proxy headers: ${error.message}`); + throw new ProxyServiceError(`Could not decrypt proxy headers: ${error.message}`); } } else { - throw new Error('Proxy headers not found'); + throw new ProxyServiceError('X-CHEFS-PROXY-DATA headers not found or empty.'); } }, getExternalAPI: async (headers, proxyHeaderInfo) => { const externalApiName = headerValue(headers, 'X-CHEFS-EXTERNAL-API-NAME'); - const externalAPI = await ExternalAPI.query().modify('findByFormIdAndName', proxyHeaderInfo['formId'], externalApiName).first().throwIfNotFound(); - return externalAPI; + if (externalApiName) { + const externalAPI = await ExternalAPI.query().modify('findByFormIdAndName', proxyHeaderInfo['formId'], externalApiName).first().throwIfNotFound(); + return externalAPI; + } else { + throw new ProxyServiceError('X-CHEFS-EXTERNAL-API-NAME header not found or empty.'); + } }, createExternalAPIUrl: (headers, endpointUrl) => { //check incoming request headers for path to add to the endpoint url @@ -78,7 +82,7 @@ const service = { if (externalAPI.sendUserToken) { if (!proxyHeaderInfo || !proxyHeaderInfo.token) { // just assume that if there is no idpUserId than it isn't a userInfo object - throw new Error('Cannot create user token headers for External API without populated proxy header info token.'); + throw new ProxyServiceError('Cannot create user token headers for External API without populated proxy header info token.'); } const val = externalAPI.userTokenBearer ? `Bearer ${proxyHeaderInfo['token']}` : proxyHeaderInfo['token']; result[externalAPI.userTokenHeader] = val; @@ -86,7 +90,7 @@ const service = { if (externalAPI.sendUserInfo) { if (!proxyHeaderInfo || !proxyHeaderInfo.idp) { // just assume that if there is no idp than it isn't a userInfo object - throw new Error('Cannot create user headers for External API without populated proxy header info object.'); + throw new ProxyServiceError('Cannot create user headers for External API without populated proxy header info object.'); } // user information (no token) diff --git a/app/tests/unit/forms/proxy/controller.spec.js b/app/tests/unit/forms/proxy/controller.spec.js index 6ea934267..d52ad6c68 100644 --- a/app/tests/unit/forms/proxy/controller.spec.js +++ b/app/tests/unit/forms/proxy/controller.spec.js @@ -4,6 +4,7 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); const controller = require('../../../../src/forms/proxy/controller'); const service = require('../../../../src/forms/proxy/service'); const jwtService = require('../../../../src/components/jwtService'); +const { NotFoundError } = require('objection'); const bearerToken = Math.random().toString(36).substring(2); @@ -138,9 +139,30 @@ describe('callExternalApi', () => { expect(service.readProxyHeaders).toBeCalledTimes(1); expect(service.getExternalAPI).toBeCalledTimes(1); - expect(service.createExternalAPIUrl).not.toHaveBeenCalled(); - expect(service.createExternalAPIHeaders).not.toHaveBeenCalled(); + expect(service.createExternalAPIUrl).toBeCalledTimes(1); + expect(service.createExternalAPIHeaders).toBeCalledTimes(1); + // this is the point where we check the status code for external api + expect(axios.get).not.toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalled(); expect(next).toBeCalledTimes(1); }); + + it('should return 400 when headers missing', async () => { + const req = getMockReq({ headers: { 'X-CHEFS-PROXY-DATA': 'encrypted blob of proxy data' } }); + const { res, next } = getMockRes(); + + service.readProxyHeaders = jest.fn().mockReturnValue({}); + service.getExternalAPI = jest.fn().mockRejectedValueOnce(new NotFoundError()); + service.createExternalAPIUrl = jest.fn().mockReturnValue('http://external.api/private'); + service.createExternalAPIHeaders = jest.fn().mockReturnValue({ 'X-TEST-HEADERS': 'test-headers-err' }); + + await controller.callExternalApi(req, res, next); + + expect(service.readProxyHeaders).toBeCalledTimes(1); + expect(service.getExternalAPI).toBeCalledTimes(1); + expect(service.createExternalAPIUrl).not.toHaveBeenCalled(); + expect(service.createExternalAPIHeaders).not.toHaveBeenCalled(); + expect(res.sendStatus).toBeCalledWith(400); + expect(next).not.toHaveBeenCalled(); + }); }); diff --git a/docs/chefs-external-api-configuration.md b/docs/chefs-external-api-configuration.md index 12c421214..fbf46df5b 100644 --- a/docs/chefs-external-api-configuration.md +++ b/docs/chefs-external-api-configuration.md @@ -78,7 +78,7 @@ The user information initially comes from the user's token, as such the values f ## Configuring Form Component -For this example, we assume populating a drop down/select component... +For this example, we assume populating a drop-down/select component... **Data Source Type** = URL @@ -92,7 +92,7 @@ For this example, we assume populating a drop down/select component... | X-CHEFS-EXTERNAL-API-NAME | example-api - External API.name | | X-CHEFS-EXTERNAL-API-PATH | optional - add this value to External API.endpointUrl | -**Value Property**, **Item Template** and all other configuration is up to the Form Designer. +**Value Property**, **Item Template** and all other configurations are up to the Form Designer. `sessionStorage.getItem('X-CHEFS-PROXY-DATA')` is the User Info object in encrypted form that only CHEFS can decrypt. This is generated and stored on the form load. A call is made to the CHEFS backend using the current user's token (similar to fetching the form schema for rendering the form) and CHEFS encrypts the information. This prevents malicious form designers from having access to the user token but allows the form designer to provide context for their External API. @@ -101,3 +101,18 @@ The `sessionStorage` is cleared when the user navigates away from the form. The component will call the CHEFS proxy `{chefs host}/app/api/v1/proxy/external` with the headers, the proxy can decrypt the `X-CHEFS-PROXY-DATA` and formulate the call according to the External API configuration. It is expected that the External API endpoint is a `GET`. + +## HTTP Responses and errors + +Since formio components will make calls during the form design and configuration of the formio components (ie when the Datasource is URL and the URL has been populated), there will be many failed attempts calling the proxy. The most common failures will happen when the headers have not been added to the component configuration, or the `X-CHEFS-EXTERNAL-API-NAME` header has been set but the External API has not been configured. + +The following table will help you understand the HTTP statuses returned when calling `/api/v1/proxy/external`. + +| Http Status | Meaning | +| ----------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 400 | Generally, the formio component has not been completely configured (missing headers) or the External API has not been configured. | +| 407 | The External API is configured and exists but has not been approved. | +| 502 | Call has gone through the CHEFS proxy but failed on the external server (ie 404 not found on your server). Check the message for information on the underlying HTTP Error. | +| 500 | Unexpected CHEFS server error. | + +A successful call through the CHEFS proxy to your External API will return the status code from your External API.