diff --git a/package-lock.json b/package-lock.json index e3596a4..079f48a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,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.6.2", + "@mongodb-js/oidc-mock-provider": "^0.7.1", "@mongodb-js/prettier-config-devtools": "^1.0.1", "@mongodb-js/tsconfig-devtools": "^1.0.0", "@types/chai": "^4.2.21", @@ -2854,9 +2854,9 @@ "dev": true }, "node_modules/@mongodb-js/oidc-mock-provider": { - "version": "0.6.8", - "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.6.8.tgz", - "integrity": "sha512-WYB1ngXSl7CsUcjTZp7g8RyYN8JbmwTnURCyoLiL1RhLVvPBT37g7w2hx+Y+p9xxKAQwVyhAx+poUOc5ik3P1g==", + "version": "0.7.1", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.7.1.tgz", + "integrity": "sha512-l2yYBoAqDV6Qoqenc7PUOIeEtgr3LDu2t1dvEJ2+mdncIlHpNT0oKSbwR7vjSuRSYk3AZgoj0VOPrw8dJ4XxDQ==", "dev": true, "dependencies": { "yargs": "17.7.2" @@ -18546,9 +18546,9 @@ } }, "@mongodb-js/oidc-mock-provider": { - "version": "0.6.8", - "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.6.8.tgz", - "integrity": "sha512-WYB1ngXSl7CsUcjTZp7g8RyYN8JbmwTnURCyoLiL1RhLVvPBT37g7w2hx+Y+p9xxKAQwVyhAx+poUOc5ik3P1g==", + "version": "0.7.1", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.7.1.tgz", + "integrity": "sha512-l2yYBoAqDV6Qoqenc7PUOIeEtgr3LDu2t1dvEJ2+mdncIlHpNT0oKSbwR7vjSuRSYk3AZgoj0VOPrw8dJ4XxDQ==", "dev": true, "requires": { "yargs": "17.7.2" diff --git a/package.json b/package.json index 7e7511b..a1d25aa 100644 --- a/package.json +++ b/package.json @@ -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.6.2", + "@mongodb-js/oidc-mock-provider": "^0.7.1", "@mongodb-js/prettier-config-devtools": "^1.0.1", "@mongodb-js/tsconfig-devtools": "^1.0.0", "@types/chai": "^4.2.21", diff --git a/src/integration.spec.ts b/src/integration.spec.ts index 66adf0f..1165fed 100644 --- a/src/integration.spec.ts +++ b/src/integration.spec.ts @@ -5,12 +5,17 @@ import { OIDCTestProvider, functioningAuthCodeBrowserFlow, } from '../test/oidc-test-provider'; +import type { + OIDCMockProviderConfig, + TokenMetadata, +} from '@mongodb-js/oidc-mock-provider'; import { MongoClient } from 'mongodb'; import type { OpenBrowserOptions } from './'; import { createMongoDBOIDCPlugin } from './'; import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; import { MongoCluster } from 'mongodb-runner'; import path from 'path'; +import sinon from 'sinon'; // node-fetch@3 is ESM-only... // eslint-disable-next-line @typescript-eslint/consistent-type-imports @@ -120,6 +125,16 @@ describe('integration test with mongod', function () { context('can authenticate with a mock IdP', function () { let provider: OIDCMockProvider; let connectionString: string; + let getTokenPayload: OIDCMockProviderConfig['getTokenPayload']; + 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) { @@ -128,16 +143,8 @@ describe('integration test with mongod', function () { return this.skip(); } provider = await OIDCMockProvider.create({ - getTokenPayload() { - return { - expires_in: 3600, - payload: { - // Define the user information stored inside the access tokens - groups: ['testgroup'], - sub: 'testuser', - aud: 'resource-server-audience-value', - }, - }; + getTokenPayload(metadata: TokenMetadata) { + return getTokenPayload(metadata); }, }); @@ -160,6 +167,10 @@ describe('integration test with mongod', function () { await Promise.all([cluster?.close?.(), provider?.close?.()]); }); + beforeEach(function () { + getTokenPayload = () => tokenPayload; + }); + it('can successfully authenticate with a fake Auth Code Flow', async function () { const plugin = createMongoDBOIDCPlugin({ openBrowserTimeout: 60_000, @@ -234,5 +245,41 @@ describe('integration test with mongod', function () { await client.close(); } }); + + it('can successfully authenticate with a fake Device Auth Flow without an id_token - with a warning', async function () { + getTokenPayload = () => ({ + ...tokenPayload, + // id_token will not be included + skipIdToken: true, + }); + + const { mongoClientOptions, logger } = createMongoDBOIDCPlugin({ + notifyDeviceFlow: () => {}, + allowedFlows: ['device-auth'], + }); + const logEmitSpy = sinon.spy(logger, 'emit'); + const client = await MongoClient.connect(connectionString, { + ...mongoClientOptions, + }); + // without the id token, a warning will be logged + sinon.assert.calledWith( + logEmitSpy, + 'mongodb-oidc-plugin:missing-id-token' + ); + try { + const status = await client + .db('admin') + .command({ connectionStatus: 1 }); + expect(status).to.deep.equal({ + ok: 1, + authInfo: { + authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], + authenticatedUserRoles: [{ role: 'dev/testgroup', db: 'admin' }], + }, + }); + } finally { + await client.close(); + } + }); }); }); diff --git a/src/log-hook.ts b/src/log-hook.ts index 8717fb9..7ba2e00 100644 --- a/src/log-hook.ts +++ b/src/log-hook.ts @@ -260,4 +260,13 @@ export function hookLoggerToMongoLogWriter( 'Destroyed OIDC plugin instance' ); }); + + emitter.on('mongodb-oidc-plugin:missing-id-token', () => { + log.warn( + 'OIDC-PLUGIN', + mongoLogId(1_002_000_022), + `${contextPrefix}-oidc`, + 'Missing ID token in IdP response' + ); + }); } diff --git a/src/plugin.ts b/src/plugin.ts index 98f782f..02921f7 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -36,6 +36,14 @@ import { kDefaultOpenBrowserTimeout } from './api'; import { spawn } from 'child_process'; /** @internal */ + +// The `sub` and `aud` claims in the ID token of the last-received +// TokenSet, if any. +// 'no-id-token' means that the previous token set contained no ID token +type LastIdTokenClaims = + | (Pick & { noIdToken?: never }) + | { noIdToken: true }; + interface UserOIDCAuthState { // The information that the driver forwarded to us from the server // about the OIDC Identity Provider config. @@ -52,9 +60,7 @@ interface UserOIDCAuthState { // A timer attached to this state that automatically calls // currentTokenSet.tryRefresh() before the token expires. timer?: ReturnType; - // The `sub` and `aud` claims in the ID token of the last-received - // TokenSet, if any. - lastIdTokenClaims?: Pick; + lastIdTokenClaims?: LastIdTokenClaims; // A cached Client instance that uses the issuer metadata as discovered // through serverOIDCMetadata. client?: Client; @@ -201,7 +207,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { } for (const [key, serializedState] of original.state) { - const state = { + const state: UserOIDCAuthState = { serverOIDCMetadata: { ...serializedState.serverOIDCMetadata }, currentAuthAttempt: null, currentTokenSet: null, @@ -433,28 +439,53 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { // for client A, the token expires before it is requested again by client A, // then the plugin is passed to client B which requests a token, and we // receive mismatching tokens for different users or different audiences. - const idTokenClaims = tokenSet.claims(); - if (state.lastIdTokenClaims) { - for (const claim of ['aud', 'sub'] as const) { - const normalize = (value: string | string[]): string => { - return JSON.stringify( - Array.isArray(value) ? [...value].sort() : value - ); - }; - const knownClaim = normalize(state.lastIdTokenClaims[claim]); - const newClaim = normalize(idTokenClaims[claim]); + if ( + !tokenSet.id_token && + state.lastIdTokenClaims && + !state.lastIdTokenClaims.noIdToken + ) { + throw new MongoDBOIDCError( + `ID token expected, but not found. Expected claims: ${JSON.stringify( + state.lastIdTokenClaims + )}` + ); + } - if (knownClaim !== newClaim) { - throw new MongoDBOIDCError( - `Unexpected '${claim}' field in id token: Expected ${knownClaim}, saw ${newClaim}` - ); + if ( + tokenSet.id_token && + state.lastIdTokenClaims && + state.lastIdTokenClaims.noIdToken + ) { + throw new MongoDBOIDCError(`Unexpected ID token received.`); + } + + if (tokenSet.id_token) { + const idTokenClaims = tokenSet.claims(); + if (state.lastIdTokenClaims && !state.lastIdTokenClaims.noIdToken) { + for (const claim of ['aud', 'sub'] as const) { + const normalize = (value: string | string[]): string => { + return JSON.stringify( + Array.isArray(value) ? [...value].sort() : value + ); + }; + const knownClaim = normalize(state.lastIdTokenClaims[claim]); + const newClaim = normalize(idTokenClaims[claim]); + + if (knownClaim !== newClaim) { + throw new MongoDBOIDCError( + `Unexpected '${claim}' field in id token: Expected ${knownClaim}, saw ${newClaim}` + ); + } } } + state.lastIdTokenClaims = { + aud: idTokenClaims.aud, + sub: idTokenClaims.sub, + }; + } else { + state.lastIdTokenClaims = { noIdToken: true }; + this.logger.emit('mongodb-oidc-plugin:missing-id-token'); } - state.lastIdTokenClaims = { - aud: idTokenClaims.aud, - sub: idTokenClaims.sub, - }; const timerDuration = automaticRefreshTimeoutMS(tokenSet); // Use `.call()` because in browsers, `setTimeout()` requires that it is called diff --git a/src/types.ts b/src/types.ts index 4102567..afc3c07 100644 --- a/src/types.ts +++ b/src/types.ts @@ -46,6 +46,7 @@ export interface MongoDBOIDCLogEventsMap { expiresAt: string | null; }) => void; 'mongodb-oidc-plugin:destroyed': () => void; + 'mongodb-oidc-plugin:missing-id-token': () => void; } /** @public */