Skip to content

Commit

Permalink
Unit test fixes for openid-client esm import (#3418)
Browse files Browse the repository at this point in the history
* Remove NODE_OPTIONS and mock utils module

* Fixes

* Update node version

* Fix mocking of userInfo call inside utils

* Remove console.log

* Revert node version change for CI

* Upgrade to node LTS v22

* Revert accidental local changes

* Cleanup
  • Loading branch information
niwsa authored Dec 11, 2024
1 parent f826e85 commit 3e8812f
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 105 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
BOXYHQ_LICENSE_KEY: 'dummy-license'
strategy:
matrix:
node-version: [20]
node-version: [22]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

services:
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ARG NODEJS_IMAGE=node:20.17-alpine3.19
ARG NODEJS_IMAGE=node:22.12-alpine3.21
FROM --platform=$BUILDPLATFORM $NODEJS_IMAGE AS base

# Install dependencies only when needed
Expand Down
2 changes: 1 addition & 1 deletion npm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"db:migration:run:mssql": "cross-env DB_TYPE=mssql DB_URL='sqlserver://localhost:1433;database=master;username=sa;password=123ABabc!' ts-node --transpile-only ../node_modules/typeorm/cli.js migration:run -d typeorm.ts",
"db:migration:run:sqlite": "cross-env DB_TYPE=sqlite DB_URL='file:///tmp/migration-sqlite.db' ts-node --transpile-only ../node_modules/typeorm/cli.js migration:run -d typeorm.ts",
"prepublishOnly": "npm run build",
"test": "cross-env BOXYHQ_NO_ANALYTICS=1 NODE_OPTIONS='--experimental-require-module' tap --timeout=0 --allow-incomplete-coverage --allow-empty-coverage --bail test/**/*.test.ts",
"test": "cross-env BOXYHQ_NO_ANALYTICS=1 tap --timeout=0 --allow-incomplete-coverage --allow-empty-coverage --bail test/**/*.test.ts",
"sort": "npx sort-package-json"
},
"tap": {
Expand Down
12 changes: 1 addition & 11 deletions npm/test/sso/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import boxyhqNobinding from './data/metadata/nobinding/boxyhq-nobinding';
import boxyhqNoentityID from './data/metadata/noentityID/boxyhq-noentityID';
import exampleOidc from './data/metadata/example.oidc';
import invalidssodescriptor from './data/metadata/invalidSSODescriptor/invalidssodescriptor';
import * as client from 'openid-client';

// BEGIN: Fixtures for authorize
export const authz_request_normal: Partial<OAuthReqBodyWithClientId> = {
Expand All @@ -19,8 +18,6 @@ export const authz_request_normal: Partial<OAuthReqBodyWithClientId> = {
client_id: `tenant=${boxyhq.tenant}&product=${boxyhq.product}`,
};

const code_verifier = client.randomPKCECodeVerifier();
export const calculateCodeChallenge = async () => await client.calculatePKCECodeChallenge(code_verifier);
export const authz_request_normal_with_code_challenge: Partial<OAuthReqBodyWithClientId> = {
redirect_uri: boxyhq.defaultRedirectUrl,
state: 'state-123',
Expand Down Expand Up @@ -230,19 +227,12 @@ export const token_req_encoded_client_id_idp_saml_login_wrong_secretverifier = {
client_secret: 'TOP-SECRET-WRONG',
};

export const token_req_cv_mismatch = {
export const token_req = {
grant_type: 'authorization_code',
code: CODE,
code_verifier: code_verifier + 'invalid_chars',
redirect_uri: boxyhq.defaultRedirectUrl,
};

export const token_req_with_cv = {
grant_type: 'authorization_code',
code: CODE,
code_verifier,
redirect_uri: boxyhq.defaultRedirectUrl,
};
// END: Fixtures for token

// BEGIN: Fixtures for *_api.test.ts
Expand Down
190 changes: 105 additions & 85 deletions npm/test/sso/oidc_idp_oauth.test.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,77 @@
import tap from 'tap';
import * as client from 'openid-client';
import { IConnectionAPIController, IOAuthController, OAuthReq } from '../../src/typings';
import * as utils from '../../src/controller/utils';
import { IConnectionAPIController, IOAuthController, OAuthReq, Profile } from '../../src/typings';
import { authz_request_oidc_provider, oidc_response, oidc_response_with_error } from './fixture';
import { JacksonError } from '../../src/controller/error';
import { addSSOConnections, jacksonOptions } from '../utils';
import path from 'path';
import type { Configuration } from 'openid-client';

let connectionAPIController: IConnectionAPIController;
let oauthController: IOAuthController;

const metadataPath = path.join(__dirname, '/data/metadata');

const code_verifier: string = client.randomPKCECodeVerifier();
let code_verifier: string;
let code_challenge: string;

const openIdClientMock = tap.createMock(client, {
...client,
randomPKCECodeVerifier: () => {
return code_verifier;
},
calculatePKCECodeChallenge: async () => {
code_challenge = await client.calculatePKCECodeChallenge(code_verifier);
return code_challenge;
},
});
let openIdClientMock: typeof import('openid-client');
let utilsMock: any;

tap.before(async () => {
const client = await import('openid-client');
code_verifier = client.randomPKCECodeVerifier();
code_challenge = await client.calculatePKCECodeChallenge(code_verifier);
openIdClientMock = {
...client,
randomPKCECodeVerifier: () => {
return code_verifier;
},
calculatePKCECodeChallenge: async () => {
return code_challenge;
},
};
utilsMock = tap.createMock(utils, {
...utils,
dynamicImport: async (packageName) => {
if (packageName === 'openid-client') {
return openIdClientMock;
}
// fallback to original impl for other packages
return utils.dynamicImport(packageName);
},
extractOIDCUserProfile: async (tokens: utils.AuthorizationCodeGrantResult, oidcConfig: Configuration) => {
const idTokenClaims = tokens.claims()!;
const client = openIdClientMock as typeof import('openid-client');
openIdClientMock.fetchUserInfo = async () => {
return {
sub: 'USER_IDENTIFIER',
email: '[email protected]',
given_name: 'jackson',
family_name: 'samuel',
picture: 'https://jackson.cloud.png',
email_verified: true,
};
};
const userinfo = await client.fetchUserInfo(oidcConfig, tokens.access_token, idTokenClaims.sub);

const profile: { claims: Partial<Profile & { raw: Record<string, unknown> }> } = { claims: {} };

profile.claims.id = idTokenClaims.sub;
profile.claims.email = typeof idTokenClaims.email === 'string' ? idTokenClaims.email : userinfo.email;
profile.claims.firstName =
typeof idTokenClaims.given_name === 'string' ? idTokenClaims.given_name : userinfo.given_name;
profile.claims.lastName =
typeof idTokenClaims.family_name === 'string' ? idTokenClaims.family_name : userinfo.family_name;
profile.claims.roles = idTokenClaims.roles ?? (userinfo.roles as any);
profile.claims.groups = idTokenClaims.groups ?? (userinfo.groups as any);
profile.claims.raw = { ...idTokenClaims, ...userinfo };

return profile;
},
});

const indexModule = tap.mockRequire('../../src/index', {
'openid-client': openIdClientMock,
'../../src/controller/utils': utilsMock,
});
const controller = await indexModule.default(jacksonOptions);

Expand All @@ -43,24 +87,20 @@ tap.teardown(async () => {
tap.test('[OIDCProvider]', async (t) => {
const context: Record<string, any> = {};

t.test(
'[authorize] Should return the IdP SSO URL',
{ todo: 'fix mocking of openid-client which is dynamically imported' },
async (t) => {
// will be matched in happy path test
context.codeVerifier = code_verifier;
t.test('[authorize] Should return the IdP SSO URL', async (t) => {
// will be matched in happy path test
context.codeVerifier = code_verifier;

const response = (await oauthController.authorize(<OAuthReq>authz_request_oidc_provider)) as {
redirect_url: string;
};
const params = new URLSearchParams(new URL(response.redirect_url!).search);
t.ok('redirect_url' in response, 'got the Idp authorize URL');
t.ok(params.has('state'), 'state present');
t.match(params.get('scope'), 'openid email profile', 'openid scopes present');
t.match(params.get('code_challenge'), code_challenge, 'codeChallenge present');
context.state = params.get('state');
}
);
const response = (await oauthController.authorize(<OAuthReq>authz_request_oidc_provider)) as {
redirect_url: string;
};
const params = new URLSearchParams(new URL(response.redirect_url!).search);
t.ok('redirect_url' in response, 'got the Idp authorize URL');
t.ok(params.has('state'), 'state present');
t.match(params.get('scope'), 'openid email profile', 'openid scopes present');
t.match(params.get('code_challenge'), code_challenge, 'codeChallenge present');
context.state = params.get('state');
});

t.test('[authorize] Should omit profile scope if openid.requestProfileScope is set to false', async (t) => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Expand Down Expand Up @@ -155,21 +195,17 @@ tap.test('[OIDCProvider]', async (t) => {
oauthController.opts.oidcPath = jacksonOptions.oidcPath;
});

t.test(
'[oidcAuthzResponse] Should throw an error if `state` is missing',
{ todo: 'fix mocking of openid-client which is dynamically imported' },
async (t) => {
try {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
//@ts-ignore
await oauthController.oidcAuthzResponse(oidc_response);
} catch (err) {
const { message, statusCode } = err as JacksonError;
t.equal(message, 'State from original request is missing.', 'got expected error message');
t.equal(statusCode, 403, 'got expected status code');
}
t.test('[oidcAuthzResponse] Should throw an error if `state` is missing', async (t) => {
try {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
//@ts-ignore
await oauthController.oidcAuthzResponse(oidc_response);
} catch (err) {
const { message, statusCode } = err as JacksonError;
t.equal(message, 'State from original request is missing.', 'got expected error message');
t.equal(statusCode, 403, 'got expected status code');
}
);
});

t.test('[oidcAuthzResponse] Should throw an error if `state` is invalid', async (t) => {
try {
Expand All @@ -181,50 +217,35 @@ tap.test('[OIDCProvider]', async (t) => {
}
});

t.test(
'[oidcAuthzResponse] Should forward any provider errors to redirect_uri',
{ todo: 'fix mocking of openid-client which is dynamically imported' },
async (t) => {
const { redirect_url } = await oauthController.oidcAuthzResponse({
...oidc_response_with_error,
state: context.state,
});
const response_params = new URLSearchParams(new URL(redirect_url!).search);
t.test('[oidcAuthzResponse] Should forward any provider errors to redirect_uri', async (t) => {
const { redirect_url } = await oauthController.oidcAuthzResponse({
...oidc_response_with_error,
state: context.state,
});
const response_params = new URLSearchParams(new URL(redirect_url!).search);

t.match(
response_params.get('error'),
oidc_response_with_error.error,
'mismatch in forwarded oidc provider error'
);
t.match(
response_params.get('error_description'),
oidc_response_with_error.error_description,
'mismatch in forwaded oidc error_description'
);
t.match(
response_params.get('state'),
authz_request_oidc_provider.state,
'state mismatch in error response'
);
}
);
t.match(
response_params.get('error'),
oidc_response_with_error.error,
'mismatch in forwarded oidc provider error'
);
t.match(
response_params.get('error_description'),
oidc_response_with_error.error_description,
'mismatch in forwaded oidc error_description'
);
t.match(
response_params.get('state'),
authz_request_oidc_provider.state,
'state mismatch in error response'
);
});

t.test(
'[oidcAuthzResponse] Should return the client redirect url with code and original state attached',
{ todo: 'fix mocking of openid-client which is dynamically imported' },
async (t) => {
// let capturedArgs: any;
openIdClientMock.fetchUserInfo = async () => {
return {
sub: 'USER_IDENTIFIER',
email: '[email protected]',
given_name: 'jackson',
family_name: 'samuel',
picture: 'https://jackson.cloud.png',
email_verified: true,
};
};
const mockAuthorizationCodeGrant = async () => {
openIdClientMock.authorizationCodeGrant = async () => {
return {
access_token: 'ACCESS_TOKEN',
id_token: 'ID_TOKEN',
Expand All @@ -241,7 +262,6 @@ tap.test('[OIDCProvider]', async (t) => {
}),
} as any;
};
openIdClientMock.authorizationCodeGrant = mockAuthorizationCodeGrant;

const { redirect_url } = await oauthController.oidcAuthzResponse({
...oidc_response,
Expand Down
19 changes: 13 additions & 6 deletions npm/test/sso/saml_idp_oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,13 @@ import {
redirect_uri_not_set,
response_type_not_code,
state_not_set,
token_req_cv_mismatch,
token_req_encoded_client_id,
token_req_dummy_client_id_idp_saml_login,
token_req_unencoded_client_id_gen,
token_req_with_cv,
token_req,
token_req_encoded_client_id_idp_saml_login,
token_req_dummy_client_id_idp_saml_login_wrong_secretverifier,
token_req_encoded_client_id_idp_saml_login_wrong_secretverifier,
calculateCodeChallenge,
} from './fixture';
import { addSSOConnections, jacksonOptions } from '../utils';
import boxyhq from './data/metadata/boxyhq';
Expand All @@ -59,8 +57,14 @@ const token = '24c1550190dd6a5a9bd6fe2a8ff69d593121c7b9';
const metadataPath = path.join(__dirname, '/data/metadata');

let connections: Array<any> = [];
let code_verifier: string;
let code_challenge: string;

tap.before(async () => {
const client = await import('openid-client');
code_verifier = client.randomPKCECodeVerifier();
code_challenge = await client.calculatePKCECodeChallenge(code_verifier);

keyPair = await jose.generateKeyPair('RS256', { modulusLength: 3072 });

const controller = await (await import('../../src/index')).default(jacksonOptions);
Expand Down Expand Up @@ -618,7 +622,7 @@ tap.test('token()', async (t) => {

t.test('PKCE check', async (t) => {
const authBody = authz_request_normal_with_code_challenge;
authBody.code_challenge = await calculateCodeChallenge();
authBody.code_challenge = code_challenge;

const { redirect_url } = (await oauthController.authorize(<OAuthReq>authBody)) as {
redirect_url: string;
Expand Down Expand Up @@ -647,15 +651,18 @@ tap.test('token()', async (t) => {
await oauthController.samlResponse(<SAMLResponsePayload>responseBody);

try {
await oauthController.token(<OAuthTokenReq>token_req_cv_mismatch);
await oauthController.token(<OAuthTokenReq>{
...token_req,
code_verifier: code_verifier + 'invalid_chars',
});
t.fail('Expecting JacksonError.');
} catch (err) {
const { message, statusCode } = err as JacksonError;
t.equal(message, 'Invalid code_verifier', 'got expected error message');
t.equal(statusCode, 401, 'got expected status code');
}

const tokenRes = await oauthController.token(<OAuthTokenReq>token_req_with_cv);
const tokenRes = await oauthController.token(<OAuthTokenReq>{ ...token_req, code_verifier });

t.ok('access_token' in tokenRes, 'includes access_token');
t.ok('token_type' in tokenRes, 'includes token_type');
Expand Down

0 comments on commit 3e8812f

Please sign in to comment.