From f3580a5a759d64565648d257d1e73b6d3491f5cb Mon Sep 17 00:00:00 2001 From: Joshua MacVey <joshua.macvey@optum.com> Date: Fri, 25 Jun 2021 12:49:50 -0500 Subject: [PATCH 1/4] added httpTimeout option to ConfigParams; added tests --- index.d.ts | 5 ++++ lib/client.js | 3 ++- lib/config.js | 1 + test/client.tests.js | 57 +++++++++++++++++++++++++++++++++++++++++++- test/config.tests.js | 45 ++++++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 2 deletions(-) diff --git a/index.d.ts b/index.d.ts index 74fcaf6f..3a28c274 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; + + /** + * Default http timeout for oidc client requests in milliseconds. Default is `5000`. + */ + 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..905b9969 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'); @@ -75,6 +75,12 @@ describe('client initialization', function () { }); describe('idTokenSigningAlg configuration is not overridden by discovery server', function () { + nock('https://op.example.com', { allowUnmocked: true }) + .persist() + .get('/.well-known/openid-configuration') + .delayBody(2000) // 2 second delay + .reply(200, { ...wellKnown }); + const config = getConfig({ secret: '__test_session_secret__', clientID: '__test_client_id__', @@ -98,4 +104,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({ From 5c2f75c83652a5a1e37e77801edb40f8d16fc36a Mon Sep 17 00:00:00 2001 From: Joshua MacVey <joshua.macvey@optum.com> Date: Fri, 25 Jun 2021 13:23:30 -0500 Subject: [PATCH 2/4] updated comment in index.d.ts --- index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.d.ts b/index.d.ts index 3a28c274..3ce52969 100644 --- a/index.d.ts +++ b/index.d.ts @@ -463,7 +463,7 @@ interface ConfigParams { tokenEndpointParams?: TokenParameters; /** - * Default http timeout for oidc client requests in milliseconds. Default is `5000`. + * Http timeout for oidc client requests in milliseconds. Default is 5000. */ httpTimeout?: number; } From 5cf153d4eb34ed3f15ac5a143050c48325a59f24 Mon Sep 17 00:00:00 2001 From: Joshua MacVey <joshua.macvey@optum.com> Date: Fri, 25 Jun 2021 13:42:25 -0500 Subject: [PATCH 3/4] removed accidental code checkin to different unit test --- test/client.tests.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/client.tests.js b/test/client.tests.js index 905b9969..702395a0 100644 --- a/test/client.tests.js +++ b/test/client.tests.js @@ -75,12 +75,6 @@ describe('client initialization', function () { }); describe('idTokenSigningAlg configuration is not overridden by discovery server', function () { - nock('https://op.example.com', { allowUnmocked: true }) - .persist() - .get('/.well-known/openid-configuration') - .delayBody(2000) // 2 second delay - .reply(200, { ...wellKnown }); - const config = getConfig({ secret: '__test_session_secret__', clientID: '__test_client_id__', From 09205d211b1a7c40a0dcbe9a6d47d63f732aa9c7 Mon Sep 17 00:00:00 2001 From: Joshua MacVey <joshua.macvey@optum.com> Date: Fri, 25 Jun 2021 13:43:53 -0500 Subject: [PATCH 4/4] added comment about minimum to httpTimeout in index.d.ts --- index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.d.ts b/index.d.ts index 3ce52969..617faea7 100644 --- a/index.d.ts +++ b/index.d.ts @@ -463,7 +463,7 @@ interface ConfigParams { tokenEndpointParams?: TokenParameters; /** - * Http timeout for oidc client requests in milliseconds. Default is 5000. + * Http timeout for oidc client requests in milliseconds. Default is 5000. Minimum is 500. */ httpTimeout?: number; }