From 0f434fbfd6276811d18cf70db6fede962fdc7219 Mon Sep 17 00:00:00 2001 From: Joshua MacVey <15506741+jmacvey@users.noreply.github.com> Date: Tue, 29 Jun 2021 10:04:49 -0500 Subject: [PATCH] feat(patch): Added httpTimeout Option (#251) * added httpTimeout option to ConfigParams; added tests * updated comment in index.d.ts * removed accidental code checkin to different unit test * added comment about minimum to httpTimeout in index.d.ts Co-authored-by: Joshua MacVey --- index.d.ts | 5 +++++ lib/client.js | 3 ++- lib/config.js | 1 + test/client.tests.js | 51 +++++++++++++++++++++++++++++++++++++++++++- test/config.tests.js | 45 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 2 deletions(-) diff --git a/index.d.ts b/index.d.ts index 74fcaf6f..617faea7 100644 --- a/index.d.ts +++ b/index.d.ts @@ -461,6 +461,11 @@ interface ConfigParams { * Additional request body properties to be sent to the `token_endpoint` during authorization code exchange or token refresh. */ tokenEndpointParams?: TokenParameters; + + /** + * Http timeout for oidc client requests in milliseconds. Default is 5000. Minimum is 500. + */ + httpTimeout?: number; } interface SessionStorePayload { diff --git a/lib/client.js b/lib/client.js index 19980ee8..2d3d1d5f 100644 --- a/lib/client.js +++ b/lib/client.js @@ -32,9 +32,10 @@ async function get(config) { } : undefined), }; - options.timeout = 5000; + options.timeout = config.httpTimeout; return options; }; + const applyHttpOptionsCustom = (entity) => (entity[custom.http_options] = defaultHttpOptions); diff --git a/lib/config.js b/lib/config.js index 19b579dc..8568f425 100644 --- a/lib/config.js +++ b/lib/config.js @@ -184,6 +184,7 @@ const paramsSchema = Joi.object({ ? 'none' : 'client_secret_basic'; }), + httpTimeout: Joi.number().optional().min(500).default(5000), }); module.exports.get = function (config = {}) { diff --git a/test/client.tests.js b/test/client.tests.js index 71e9eb3f..702395a0 100644 --- a/test/client.tests.js +++ b/test/client.tests.js @@ -1,4 +1,4 @@ -const { assert } = require('chai').use(require('chai-as-promised')); +const { assert, expect } = require('chai').use(require('chai-as-promised')); const { get: getConfig } = require('../lib/config'); const { get: getClient } = require('../lib/client'); const wellKnown = require('./fixture/well-known.json'); @@ -98,4 +98,53 @@ describe('client initialization', function () { assert.equal(client.id_token_signed_response_alg, 'RS256'); }); }); + + describe('client respects httpTimeout configuration', function () { + const config = getConfig({ + secret: '__test_session_secret__', + clientID: '__test_client_id__', + clientSecret: '__test_client_secret__', + issuerBaseURL: 'https://op.example.com', + baseURL: 'https://example.org', + }); + + function mockRequest(delay = 0) { + nock('https://op.example.com').post('/slow').delay(delay).reply(200); + } + + async function invokeRequest(client) { + return await client.requestResource( + 'https://op.example.com/slow', + 'token', + { + method: 'POST', + headers: { + Authorization: 'Bearer foo', + }, + } + ); + } + + it('should not timeout for default', async function () { + mockRequest(0); + const client = await getClient({ ...config }); + const response = await invokeRequest(client); + assert.equal(response.statusCode, 200); + }); + + it('should not timeout for delay < httpTimeout', async function () { + mockRequest(1000); + const client = await getClient({ ...config, httpTimeout: 1500 }); + const response = await invokeRequest(client); + assert.equal(response.statusCode, 200); + }); + + it('should timeout for delay > httpTimeout', async function () { + mockRequest(1500); + const client = await getClient({ ...config, httpTimeout: 500 }); + await expect(invokeRequest(client)).to.be.rejectedWith( + `Timeout awaiting 'request' for 500ms` + ); + }); + }); }); diff --git a/test/config.tests.js b/test/config.tests.js index 0369ed69..4e0ecfdf 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -180,6 +180,7 @@ describe('get config', () => { }, }, }); + assert.deepInclude(config, { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], session: { @@ -379,6 +380,29 @@ describe('get config', () => { }, '"session.cookie.domain" must be a string'); }); + it('should fail when http timeout is invalid', function () { + assert.throws(() => { + getConfig({ + ...defaultConfig, + httpTimeout: 'abcd', + }); + }, '"httpTimeout" must be a number'); + + assert.throws(() => { + getConfig({ + ...defaultConfig, + httpTimeout: '-100', + }); + }, '"httpTimeout" must be greater than or equal to 500'); + + assert.throws(() => { + getConfig({ + ...defaultConfig, + httpTimeout: '499', + }); + }, '"httpTimeout" must be greater than or equal to 500'); + }); + it("shouldn't allow a secret of less than 8 chars", () => { assert.throws( () => getConfig({ ...defaultConfig, secret: 'short' }), @@ -630,6 +654,18 @@ describe('get config', () => { ); }); + it('should allow valid httpTimeout configuration', () => { + const config = (httpTimeout) => ({ + ...defaultConfig, + httpTimeout, + }); + + assert.doesNotThrow(() => config(5000)); + assert.doesNotThrow(() => config(10000)); + assert.doesNotThrow(() => config('5000')); + assert.doesNotThrow(() => config('10000')); + }); + it('should default clientAuthMethod to none for id_token response type', () => { { const config = getConfig(defaultConfig); @@ -648,6 +684,15 @@ describe('get config', () => { } }); + it('should default httpTimeout to 5000', () => { + { + const config = getConfig(defaultConfig); + assert.deepInclude(config, { + httpTimeout: 5000, + }); + } + }); + it('should default clientAuthMethod to client_secret_basic for other response types', () => { { const config = getConfig({