diff --git a/src/XeroClient.ts b/src/XeroClient.ts index 100e1817..c933c46a 100644 --- a/src/XeroClient.ts +++ b/src/XeroClient.ts @@ -1,5 +1,6 @@ import { Client, Issuer, TokenSet, TokenSetParameters, custom } from 'openid-client'; import * as xero from './gen/api'; +import { ApiError } from './model/ApiError'; const axios = require('axios'); import http = require('http'); @@ -220,7 +221,8 @@ export class XeroClient { } } catch (error) { - reject(error) + const errorResponse = new ApiError(error); + reject(JSON.stringify(errorResponse.generateError())); } }); } @@ -262,7 +264,8 @@ export class XeroClient { } } catch (error) { - reject(error) + const errorResponse = new ApiError(error); + reject(JSON.stringify(errorResponse.generateError())); } }); } diff --git a/src/model/ApiError.ts b/src/model/ApiError.ts index 8b547985..ec4bd13e 100644 --- a/src/model/ApiError.ts +++ b/src/model/ApiError.ts @@ -23,6 +23,24 @@ interface ErrorResponse { body: any } +const SENSITIVE_HEADER_NAME = /authorization|cookie|api[-_]?key|token|secret|password/i; + +function sanitizeHeaders(headers: any): any { + const safeHeaders: any = {}; + + if (!headers || typeof headers !== 'object') { + return safeHeaders; + } + + Object.keys(headers).forEach((headerName) => { + if (!SENSITIVE_HEADER_NAME.test(headerName)) { + safeHeaders[headerName] = headers[headerName]; + } + }); + + return safeHeaders; +} + export class ApiError { statusCode: number @@ -36,7 +54,7 @@ export class ApiError { this.statusCode = response.status || 0; this.body = response.data ?? axiosError.message; - this.headers = response.headers || {}; + this.headers = sanitizeHeaders(response.headers); this.request = { url: { protocol: request.protocol, @@ -44,7 +62,9 @@ export class ApiError { host: request.host, path: request.path, }, - headers: typeof request.getHeaders === 'function' ? request.getHeaders() : {}, + headers: sanitizeHeaders( + typeof request.getHeaders === 'function' ? request.getHeaders() : undefined + ), method: request.method } } diff --git a/src/test/apiError.spec.ts b/src/test/apiError.spec.ts index 2e1ff9f9..da820b92 100644 --- a/src/test/apiError.spec.ts +++ b/src/test/apiError.spec.ts @@ -1,7 +1,14 @@ import { ApiError } from '../model/ApiError'; +import { XeroClient } from '../XeroClient'; +import nock from 'nock'; describe('ApiError', () => { - it('handles axios errors without a response', () => { + it('removes sensitive headers from serialized generated errors', () => { + const authorization = 'Bearer access-token-that-must-not-leak'; + const cookie = 'session=session-cookie-that-must-not-leak'; + const proxyAuthorization = 'Basic proxy-credentials-that-must-not-leak'; + const apiKey = 'api-key-that-must-not-leak'; + const clientSecret = 'client-secret-that-must-not-leak'; const apiError = new ApiError({ message: 'Network Error', request: { @@ -12,13 +19,26 @@ describe('ApiError', () => { host: 'api.xero.com', path: '/api.xro/2.0/Invoices', getHeaders: () => ({ - authorization: 'Bearer token', + authorization, + Cookie: cookie, + 'Proxy-Authorization': proxyAuthorization, + 'X-API-Key': apiKey, + 'X-Client-Secret': clientSecret, + Accept: 'application/json', + 'X-Request-Id': 'request-id', }), method: 'GET', }, }); - expect(apiError.generateError()).toEqual({ + const serializedError = JSON.stringify(apiError.generateError()); + + expect(serializedError).not.toContain(authorization); + expect(serializedError).not.toContain(cookie); + expect(serializedError).not.toContain(proxyAuthorization); + expect(serializedError).not.toContain(apiKey); + expect(serializedError).not.toContain(clientSecret); + expect(JSON.parse(serializedError)).toEqual({ response: { statusCode: 0, body: 'Network Error', @@ -31,7 +51,8 @@ describe('ApiError', () => { path: '/api.xro/2.0/Invoices', }, headers: { - authorization: 'Bearer token', + Accept: 'application/json', + 'X-Request-Id': 'request-id', }, method: 'GET', }, @@ -49,6 +70,8 @@ describe('ApiError', () => { }, headers: { 'content-type': 'application/json', + 'set-cookie': 'session=response-cookie-that-must-not-leak', + 'X-Rate-Limit-Remaining': '59', }, }, request: { @@ -63,7 +86,10 @@ describe('ApiError', () => { }, }); - expect(apiError.generateError()).toEqual({ + const serializedError = JSON.stringify(apiError.generateError()); + + expect(serializedError).not.toContain('response-cookie-that-must-not-leak'); + expect(JSON.parse(serializedError)).toEqual({ response: { statusCode: 401, body: { @@ -71,6 +97,7 @@ describe('ApiError', () => { }, headers: { 'content-type': 'application/json', + 'X-Rate-Limit-Remaining': '59', }, request: { url: { @@ -89,3 +116,66 @@ describe('ApiError', () => { }); }); }); + +describe('ApiError integration with XeroClient', () => { + const clientId = 'client-id-that-must-not-leak'; + const clientSecret = 'client-secret-that-must-not-leak'; + const refreshToken = 'refresh-token-that-must-not-leak'; + const accessToken = 'access-token-that-must-not-leak'; + + const buildClient = () => new XeroClient({ + clientId, + clientSecret, + redirectUris: ['http://localhost:5000/callback'], + scopes: 'openid profile email offline_access'.split(' ') + }); + + const rejectionOf = (promise: Promise): Promise => promise.then( + () => { throw new Error('expected the request to reject'); }, + (error) => error + ); + + afterEach(() => { + nock.cleanAll(); + }); + + it('redacts the basic auth credentials and refresh token when the token request fails', async () => { + nock('https://identity.xero.com') + .post('/connect/token') + .reply(401, { error: 'invalid_grant' }); + + const rejection = await rejectionOf( + buildClient().refreshWithRefreshToken(clientId, clientSecret, refreshToken) + ); + + const serializedError = JSON.stringify(rejection); + const basicCredentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64'); + + expect(serializedError).not.toContain(basicCredentials); + expect(serializedError).not.toContain(clientSecret); + expect(serializedError).not.toContain(refreshToken); + expect(JSON.parse(rejection).response.statusCode).toEqual(401); + }); + + it('redacts the bearer token when the connections query fails', async () => { + nock('https://api.xero.com') + .get('/connections') + .reply(401, { Title: 'Unauthorized' }); + + const client = buildClient(); + client.setTokenSet({ + access_token: accessToken, + refresh_token: refreshToken, + token_type: 'Bearer', + expires_at: 1231231234 + }); + + const rejection = await rejectionOf(client.updateTenants()); + + const serializedError = JSON.stringify(rejection); + + expect(serializedError).not.toContain(accessToken); + expect(serializedError).not.toContain(refreshToken); + expect(JSON.parse(rejection).response.statusCode).toEqual(401); + }); +});