From b1a9e5f0da0e524a16d6d65734760e064e95c342 Mon Sep 17 00:00:00 2001 From: Florent Date: Tue, 3 Sep 2024 23:10:18 +0200 Subject: [PATCH] OIDC: allow configuring the request timeout (#1177) Add IdP timeout, controlled by env var GRIST_OIDC_SP_HTTP_TIMEOUT --------- Co-authored-by: atropos --- app/server/lib/OIDCConfig.ts | 12 +++++++-- test/server/lib/OIDCConfig.ts | 51 ++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 8a353545..2343f468 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -47,7 +47,9 @@ * A JSON object with extra client metadata to pass to openid-client. Optional. * Be aware that setting this object may override any other values passed to the openid client. * More info: https://github.com/panva/node-openid-client/tree/main/docs#new-clientmetadata-jwks-options - * + * env GRIST_OIDC_SP_HTTP_TIMEOUT + * The timeout in milliseconds for HTTP requests to the IdP. The default value is set to 3500 by the + * openid-client library. See: https://github.com/panva/node-openid-client/blob/main/docs/README.md#customizing-http-requests * * This version of OIDCConfig has been tested with Keycloak OIDC IdP following the instructions * at: @@ -66,7 +68,7 @@ import * as express from 'express'; import { GristLoginSystem, GristServer } from './GristServer'; import { - Client, ClientMetadata, Issuer, errors as OIDCError, TokenSet, UserinfoResponse + Client, ClientMetadata, custom, Issuer, errors as OIDCError, TokenSet, UserinfoResponse } from 'openid-client'; import { Sessions } from './Sessions'; import log from 'app/server/lib/log'; @@ -137,6 +139,9 @@ export class OIDCConfig { envVar: 'GRIST_OIDC_IDP_CLIENT_SECRET', censor: true, }); + const httpTimeout = section.flag('httpTimeout').readInt({ + envVar: 'GRIST_OIDC_SP_HTTP_TIMEOUT', + }); this._namePropertyKey = section.flag('namePropertyKey').readString({ envVar: 'GRIST_OIDC_SP_PROFILE_NAME_ATTR', }); @@ -173,6 +178,9 @@ export class OIDCConfig { this._protectionManager = new ProtectionsManager(enabledProtections); this._redirectUrl = new URL(CALLBACK_URL, spHost).href; + custom.setHttpOptionsDefaults({ + ...(httpTimeout !== undefined ? {timeout: httpTimeout} : {}), + }); await this._initClient({ issuerUrl, clientId, clientSecret, extraMetadata }); if (this._client.issuer.metadata.end_session_endpoint === undefined && diff --git a/test/server/lib/OIDCConfig.ts b/test/server/lib/OIDCConfig.ts index a7bb2d65..17014896 100644 --- a/test/server/lib/OIDCConfig.ts +++ b/test/server/lib/OIDCConfig.ts @@ -5,7 +5,7 @@ import {Sessions} from "app/server/lib/Sessions"; import log from "app/server/lib/log"; import {assert} from "chai"; import Sinon from "sinon"; -import {Client, generators, errors as OIDCError} from "openid-client"; +import {Client, custom, generators, errors as OIDCError} from "openid-client"; import express from "express"; import _ from "lodash"; import {RequestWithLogin} from "app/server/lib/Authorizer"; @@ -192,6 +192,55 @@ describe('OIDCConfig', () => { }); }); }); + + describe('GRIST_OIDC_SP_HTTP_TIMEOUT', function () { + [ + { + itMsg: 'when omitted should not override openid-client default value', + expectedUserDefinedHttpOptions: {} + }, + { + itMsg: 'should reject when the provided value is not a number', + env: { + GRIST_OIDC_SP_HTTP_TIMEOUT: '__NOT_A_NUMBER__', + }, + expectedErrorMsg: /__NOT_A_NUMBER__ does not look like a number/, + }, + { + itMsg: 'should override openid-client timeout accordingly to the provided value', + env: { + GRIST_OIDC_SP_HTTP_TIMEOUT: '10000', + }, + shouldSetTimeout: true, + expectedUserDefinedHttpOptions: { + timeout: 10000 + } + }, + { + itMsg: 'should allow disabling the timeout by having its value set to 0', + env: { + GRIST_OIDC_SP_HTTP_TIMEOUT: '0', + }, + expectedUserDefinedHttpOptions: { + timeout: 0 + } + } + ].forEach(ctx => { + it(ctx.itMsg, async () => { + const setHttpOptionsDefaultsStub = sandbox.stub(custom, 'setHttpOptionsDefaults'); + setEnvVars(); + Object.assign(process.env, ctx.env); + const promise = OIDCConfigStubbed.buildWithStub(); + if (ctx.expectedErrorMsg) { + await assert.isRejected(promise, ctx.expectedErrorMsg); + } else { + await assert.isFulfilled(promise, 'initOIDC should have been fulfilled'); + assert.isTrue(setHttpOptionsDefaultsStub.calledOnce, 'Should have called custom.setHttpOptionsDefaults'); + assert.deepEqual(setHttpOptionsDefaultsStub.firstCall.args[0], ctx.expectedUserDefinedHttpOptions); + } + }); + }); + }); }); describe('GRIST_OIDC_IDP_ENABLED_PROTECTIONS', () => {