Skip to content

Commit

Permalink
Adds openid parameters (#1637)
Browse files Browse the repository at this point in the history

Signed-off-by: Sam <[email protected]>
Co-authored-by: Darshit Chanpura <[email protected]>
  • Loading branch information
samuelcostae and DarshitChanpura authored Nov 29, 2023
1 parent 8f1334e commit 5cf1748
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 3 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@
"resolutions": {
"selenium-webdriver": "4.10.0"
}
}
}
50 changes: 49 additions & 1 deletion server/auth/types/openid/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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: {
Expand Down
14 changes: 14 additions & 0 deletions server/auth/types/openid/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
);
}
}
}
}
4 changes: 3 additions & 1 deletion server/auth/types/openid/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
composeLogoutUrl,
getNextUrl,
getExpirationDate,
includeAdditionalParameters,
} from './helper';
import { validateNextUrl } from '../../../utils/next_url';
import {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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!,
Expand Down
1 change: 1 addition & 0 deletions server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 5cf1748

Please sign in to comment.