From 5cf1748446d5f7b914618d4e91ebaf671c8afc27 Mon Sep 17 00:00:00 2001 From: Sam <128482925+samuelcostae@users.noreply.github.com> Date: Wed, 29 Nov 2023 15:18:41 +0000 Subject: [PATCH] Adds openid parameters (#1637) Signed-off-by: Sam Co-authored-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> --- package.json | 2 +- server/auth/types/openid/helper.test.ts | 50 ++++++++++++++++++++++++- server/auth/types/openid/helper.ts | 14 +++++++ server/auth/types/openid/routes.ts | 4 +- server/index.ts | 1 + 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 6ae13f2c1..e3993e5a5 100644 --- a/package.json +++ b/package.json @@ -39,4 +39,4 @@ "resolutions": { "selenium-webdriver": "4.10.0" } -} \ No newline at end of file +} diff --git a/server/auth/types/openid/helper.test.ts b/server/auth/types/openid/helper.test.ts index 3a125a2bf..f5cce4af0 100644 --- a/server/auth/types/openid/helper.test.ts +++ b/server/auth/types/openid/helper.test.ts @@ -13,7 +13,13 @@ * permissions and limitations under the License. */ -import { composeLogoutUrl, getExpirationDate, getRootUrl, getNextUrl } from './helper'; +import { + composeLogoutUrl, + getExpirationDate, + getRootUrl, + getNextUrl, + includeAdditionalParameters, +} from './helper'; describe('test OIDC helper utility', () => { test('test compose logout url', () => { @@ -56,6 +62,48 @@ describe('test OIDC helper utility', () => { ); }); + test('test include additional parameters in query', () => { + const contextMock = { security_plugin: { logger: { warn: jest.fn() } } }; + + const query = { existingKey: 'value' }; + const constQueryModified = { existingKey: 'value', ping: 'pong', acr_values: '1' }; + const config = { + openid: { + enabled: true, + client_secret: '', + additional_parameters: { + ping: 'pong', + acr_values: '1', + existingKey: 'foobar', + }, + }, + }; + + includeAdditionalParameters(query, contextMock, config); + expect(query).toEqual(constQueryModified); + expect(contextMock.security_plugin.logger.warn).toHaveBeenCalledTimes(1); + expect(contextMock.security_plugin.logger.warn).toHaveBeenCalledWith( + "Additional parameter in OpenID config 'existingKey' was ignored as it would overwrite existing parameters" + ); + }); + + test('test include additional parameters in query when no additional parameters specified', () => { + const contextMock = { security_plugin: { logger: { warn: jest.fn() } } }; + + const query = { existingKey: 'value' }; + const constQueryModified = { existingKey: 'value' }; + const config = { + openid: { + enabled: true, + client_secret: '', + }, + }; + + includeAdditionalParameters(query, contextMock, config); + expect(query).toEqual(constQueryModified); + expect(contextMock.security_plugin.logger.warn).toHaveBeenCalledTimes(0); + }); + test('test root url when trusted header unset', () => { const config = { openid: { diff --git a/server/auth/types/openid/helper.ts b/server/auth/types/openid/helper.ts index 9839175ca..dcea8c7a2 100644 --- a/server/auth/types/openid/helper.ts +++ b/server/auth/types/openid/helper.ts @@ -144,3 +144,17 @@ export function getExpirationDate(tokenResponse: TokenResponse | undefined) { return Date.now() + tokenResponse.expiresIn! * 1000; } } + +export function includeAdditionalParameters(query: any, context, config) { + if (config.openid?.additional_parameters) { + for (const [key, value] of Object.entries(config.openid?.additional_parameters)) { + if (query[key] == null) { + query[key] = value; + } else { + context.security_plugin.logger.warn( + `Additional parameter in OpenID config '${key}' was ignored as it would overwrite existing parameters` + ); + } + } + } +} diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index b598dd1a8..46c0d6c88 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -34,6 +34,7 @@ import { composeLogoutUrl, getNextUrl, getExpirationDate, + includeAdditionalParameters, } from './helper'; import { validateNextUrl } from '../../../utils/next_url'; import { @@ -127,6 +128,7 @@ export class OpenIdAuthRoutes { state: nonce, scope: this.openIdAuthConfig.scope, }; + includeAdditionalParameters(query, context, this.config); const queryString = stringify(query); const location = `${this.openIdAuthConfig.authorizationEndpoint}?${queryString}`; const cookie: SecuritySessionCookie = { @@ -173,7 +175,7 @@ export class OpenIdAuthRoutes { client_id: clientId, client_secret: clientSecret, }; - + includeAdditionalParameters(query, context, this.config); try { const tokenResponse = await callTokenEndpoint( this.openIdAuthConfig.tokenEndpoint!, diff --git a/server/index.ts b/server/index.ts index 198b07fe2..c1af7fa16 100644 --- a/server/index.ts +++ b/server/index.ts @@ -181,6 +181,7 @@ export const configSchema = schema.object({ verify_hostnames: schema.boolean({ defaultValue: true }), refresh_tokens: schema.boolean({ defaultValue: true }), trust_dynamic_headers: schema.boolean({ defaultValue: false }), + additional_parameters: schema.maybe(schema.any({ defaultValue: null })), extra_storage: schema.object({ cookie_prefix: schema.string({ defaultValue: 'security_authentication_oidc',