Skip to content

Commit

Permalink
feat: improve diagnostics, allow custom HTTP options, account for IdP…
Browse files Browse the repository at this point in the history
… support for scopes (#189)

* chore: add token type to auth-succeeded log event

* fix: improve error message when `Issuer.discover()` fails COMPASS-7605

* chore: disable last remaining eslint warning in tests

* feat: do not request scopes if the IdP announces lack of support COMPASS-7437

* feat: track HTTP calls, allow custom HTTP options MONGOSH-1712
  • Loading branch information
addaleax authored Mar 11, 2024
1 parent 24470e0 commit 943dd33
Show file tree
Hide file tree
Showing 10 changed files with 359 additions and 27 deletions.
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"@mongodb-js/eslint-config-devtools": "^0.9.9",
"@mongodb-js/mocha-config-devtools": "^1.0.0",
"@mongodb-js/monorepo-tools": "^1.1.4",
"@mongodb-js/oidc-mock-provider": "^0.7.1",
"@mongodb-js/oidc-mock-provider": "^0.8.0",
"@mongodb-js/prettier-config-devtools": "^1.0.1",
"@mongodb-js/tsconfig-devtools": "^1.0.0",
"@types/chai": "^4.2.21",
Expand Down
25 changes: 25 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,24 @@ import type {
OIDCRequestFunction,
TypedEventEmitter,
} from './types';
import type { RequestOptions } from 'https';

/** @public */
export type HttpOptions = Partial<
Pick<
RequestOptions,
| 'agent'
| 'ca'
| 'cert'
| 'crl'
| 'headers'
| 'key'
| 'lookup'
| 'passphrase'
| 'pfx'
| 'timeout'
>
>;

/** @public */
export type AuthFlowType = 'auth-code' | 'device-auth';
Expand Down Expand Up @@ -167,6 +185,13 @@ export interface MongoDBOIDCPluginOptions {
* message being emitted but otherwise be ignored.
*/
throwOnIncompatibleSerializedState?: boolean;

/**
* Provide custom HTTP options for individual HTTP calls.
*/
customHttpOptions?:
| HttpOptions
| ((url: string, options: Readonly<HttpOptions>) => HttpOptions);
}

/** @public */
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type {
RedirectServerRequestHandler,
RedirectServerRequestInfo,
MongoDBOIDCPluginMongoClientOptions,
HttpOptions,
} from './api';

export type {
Expand Down
40 changes: 40 additions & 0 deletions src/log-hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ import type { MongoDBOIDCLogEventsMap, TypedEventEmitter } from './types';

/** @public */
export interface MongoLogWriter {
debug?(
c: string,
id: unknown,
ctx: string,
msg: string,
attr?: unknown,
level?: 1 | 2 | 3 | 4 | 5
): void;
info(c: string, id: unknown, ctx: string, msg: string, attr?: unknown): void;
warn(c: string, id: unknown, ctx: string, msg: string, attr?: unknown): void;
error(c: string, id: unknown, ctx: string, msg: string, attr?: unknown): void;
Expand Down Expand Up @@ -269,4 +277,36 @@ export function hookLoggerToMongoLogWriter(
'Missing ID token in IdP response'
);
});

emitter.on('mongodb-oidc-plugin:outbound-http-request', (ev) => {
log.debug?.(
'OIDC-PLUGIN',
mongoLogId(1_002_000_023),
`${contextPrefix}-oidc`,
'Outbound HTTP request',
{ url: redactUrl(ev.url) }
);
});

emitter.on('mongodb-oidc-plugin:inbound-http-request', (ev) => {
log.debug?.(
'OIDC-PLUGIN',
mongoLogId(1_002_000_024),
`${contextPrefix}-oidc`,
'Inbound HTTP request',
{ url: redactUrl(ev.url) }
);
});
}

function redactUrl(url: string): string {
let parsed: URL;
try {
parsed = new URL(url);
} catch {
return '<Invalid URL>';
}
for (const key of [...parsed.searchParams.keys()])
parsed.searchParams.set(key, '');
return parsed.toString();
}
168 changes: 168 additions & 0 deletions src/plugin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
OIDCCallbackContext,
IdPServerInfo,
OIDCRequestFunction,
OpenBrowserOptions,
} from './';
import { createMongoDBOIDCPlugin, hookLoggerToMongoLogWriter } from './';
import { once } from 'events';
Expand Down Expand Up @@ -32,6 +33,24 @@ import { publicPluginToInternalPluginMap_DoNotUseOutsideOfTests } from './api';
import type { Server as HTTPServer } from 'http';
import { createServer as createHTTPServer } from 'http';
import type { AddressInfo } from 'net';
import type {
OIDCMockProviderConfig,
TokenMetadata,
} from '@mongodb-js/oidc-mock-provider';
import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider';

// node-fetch@3 is ESM-only...
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
const fetch: typeof import('node-fetch').default = (...args) =>
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
eval("import('node-fetch')").then((fetch: typeof import('node-fetch')) =>
fetch.default(...args)
);

// A 'browser' implementation that just does HTTP requests and ignores the response.
async function fetchBrowser({ url }: OpenBrowserOptions): Promise<void> {
(await fetch(url)).body?.resume();
}

// Shorthand to avoid having to specify `principalName` and `abortSignal`
// if they aren't being used in the first place.
Expand Down Expand Up @@ -308,6 +327,7 @@ describe('OIDC plugin (local OIDC provider)', function () {
expect(serializedData.oidcPluginStateVersion).to.equal(0);
expect(serializedData.state).to.have.lengthOf(1);
expect(serializedData.state[0][0]).to.be.a('string');
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
expect(Object.keys(serializedData.state[0][1]).sort()).to.deep.equal([
'currentTokenSet',
'lastIdTokenClaims',
Expand Down Expand Up @@ -827,6 +847,20 @@ describe('OIDC plugin (local OIDC provider)', function () {
}
});

it('includes a helpful error message when attempting to reach out to invalid issuer', async function () {
try {
await requestToken(plugin, {
clientId: 'clientId',
issuer: 'https://doesnotexist.mongodb.com/',
});
expect.fail('missed exception');
} catch (err: any) {
expect(err.message).to.include(
'Unable to fetch issuer metadata for "https://doesnotexist.mongodb.com/":'
);
}
});

context('with an issuer that reports custom metadata', function () {
let server: HTTPServer;
let response: Record<string, unknown>;
Expand Down Expand Up @@ -1014,3 +1048,137 @@ describe('OIDC plugin (local OIDC provider)', function () {
});
});
});

// eslint-disable-next-line mocha/max-top-level-suites
describe('OIDC plugin (mock OIDC provider)', function () {
let provider: OIDCMockProvider;
let getTokenPayload: OIDCMockProviderConfig['getTokenPayload'];
let additionalIssuerMetadata: OIDCMockProviderConfig['additionalIssuerMetadata'];
let receivedHttpRequests: string[] = [];
const tokenPayload = {
expires_in: 3600,
payload: {
// Define the user information stored inside the access tokens
groups: ['testgroup'],
sub: 'testuser',
aud: 'resource-server-audience-value',
},
};

before(async function () {
if (+process.version.slice(1).split('.')[0] < 16) {
// JWK support for Node.js KeyObject.export() is only Node.js 16+
// but the OIDCMockProvider implementation needs it.
return this.skip();
}
provider = await OIDCMockProvider.create({
getTokenPayload(metadata: TokenMetadata) {
return getTokenPayload(metadata);
},
additionalIssuerMetadata() {
return additionalIssuerMetadata?.() ?? {};
},
overrideRequestHandler(url: string) {
receivedHttpRequests.push(url);
},
});
});

after(async function () {
await provider?.close?.();
});

beforeEach(function () {
receivedHttpRequests = [];
getTokenPayload = () => tokenPayload;
additionalIssuerMetadata = undefined;
});

context('with different supported built-in scopes', function () {
let getScopes: () => Promise<string[]>;

beforeEach(function () {
getScopes = async function () {
const plugin = createMongoDBOIDCPlugin({
openBrowserTimeout: 60_000,
openBrowser: fetchBrowser,
allowedFlows: ['auth-code'],
redirectURI: 'http://localhost:0/callback',
});
const result = await requestToken(plugin, {
issuer: provider.issuer,
clientId: 'mockclientid',
requestScopes: [],
});
const accessTokenContents = getJWTContents(result.accessToken);
return String(accessTokenContents.scope).split(' ').sort();
};
});

it('will get a list of built-in OpenID scopes by default', async function () {
additionalIssuerMetadata = undefined;
expect(await getScopes()).to.deep.equal(['offline_access', 'openid']);
});

it('will omit built-in scopes if the IdP does not announce support for them', async function () {
additionalIssuerMetadata = () => ({ scopes_supported: ['openid'] });
expect(await getScopes()).to.deep.equal(['openid']);
});
});

context('HTTP request tracking', function () {
it('will log all outgoing HTTP requests', async function () {
const pluginHttpRequests: string[] = [];
const localServerHttpRequests: string[] = [];
const browserHttpRequests: string[] = [];

const plugin = createMongoDBOIDCPlugin({
openBrowserTimeout: 60_000,
openBrowser: async ({ url }) => {
// eslint-disable-next-line no-constant-condition
while (true) {
browserHttpRequests.push(url);
const response = await fetch(url, { redirect: 'manual' });
response.body?.resume();
const redirectTarget =
response.status >= 300 &&
response.status < 400 &&
response.headers.get('location');
if (redirectTarget)
url = new URL(redirectTarget, response.url).href;
else break;
}
},
allowedFlows: ['auth-code'],
redirectURI: 'http://localhost:0/callback',
});
plugin.logger.on('mongodb-oidc-plugin:outbound-http-request', (ev) =>
pluginHttpRequests.push(ev.url)
);
plugin.logger.on('mongodb-oidc-plugin:inbound-http-request', (ev) =>
localServerHttpRequests.push(ev.url)
);
await requestToken(plugin, {
issuer: provider.issuer,
clientId: 'mockclientid',
requestScopes: [],
});

const removeSearchParams = (str: string) =>
Object.assign(new URL(str), { search: '' }).toString();
const allOutboundRequests = [
...pluginHttpRequests,
...browserHttpRequests,
]
.map(removeSearchParams)
.sort();
const allInboundRequests = [
...localServerHttpRequests,
...receivedHttpRequests,
]
.map(removeSearchParams)
.sort();
expect(allOutboundRequests).to.deep.equal(allInboundRequests);
});
});
});
Loading

0 comments on commit 943dd33

Please sign in to comment.