From eb6643366621009aedd633c178469c43da86e78a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 1 Aug 2024 11:04:25 +0200 Subject: [PATCH] fix: include username in server metadata key (#194) This is something we should have ideally always been doing (as hinted at by the doc comment for `getAuthState()`), but which the driver only added back support for in 6.7.0. This shouldn't have an immediate impact on our products, because we already have some mitigations in place for the fact that usernames were previously not being taken into account (e.g. here: https://github.com/mongodb-js/mongosh/blob/adc530e7ffcf092677d944fd69670c5cbd8e103d/packages/service-provider-server/src/cli-service-provider.ts#L172 ), but is semantically the right thing to do and should give us a bit more flexibility in the future. Also, add a test that confirms that changes to server metadata/username actually have the expected effect, and emit a message bus event that shares information about the server metadata, as that may be helpful debugging information. --- src/plugin.spec.ts | 45 ++++++++++++++++++++++++++++++++++++++++++++- src/plugin.ts | 12 +++++++++--- src/types.ts | 3 +++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/plugin.spec.ts b/src/plugin.spec.ts index 975824b..19b7df9 100644 --- a/src/plugin.spec.ts +++ b/src/plugin.spec.ts @@ -55,12 +55,14 @@ async function fetchBrowser({ url }: OpenBrowserOptions): Promise { function requestToken( plugin: MongoDBOIDCPlugin, oidcParams: IdPServerInfo, - abortSignal?: OIDCAbortSignal + abortSignal?: OIDCAbortSignal, + username?: string ): ReturnType { return plugin.mongoClientOptions.authMechanismProperties.OIDC_HUMAN_CALLBACK({ timeoutContext: abortSignal, version: 1, idpInfo: oidcParams, + username, }); } @@ -500,6 +502,47 @@ describe('OIDC plugin (local OIDC provider)', function () { expect(pluginOptions.allowedFlows).to.have.been.calledTwice; }); }); + + context('when the server metadata/user data changes', function () { + beforeEach(function () { + (pluginOptions.allowedFlows = sinon.stub().resolves(['auth-code'])), + (plugin = createMongoDBOIDCPlugin(pluginOptions)); + }); + + it('it will perform two different auth flows', async function () { + await requestToken( + plugin, + provider.getMongodbOIDCDBInfo(), + undefined, + 'usera' + ); + expect(pluginOptions.allowedFlows).to.have.callCount(1); + await requestToken( + plugin, + provider.getMongodbOIDCDBInfo(), + undefined, + 'userb' + ); + expect(pluginOptions.allowedFlows).to.have.callCount(2); + await requestToken( + plugin, + provider.getMongodbOIDCDBInfo(), + undefined, + 'userb' + ); + expect(pluginOptions.allowedFlows).to.have.callCount(2); + await requestToken( + plugin, + { + ...provider.getMongodbOIDCDBInfo(), + extraKey: 'asdf', + } as IdPServerInfo, + undefined, + 'userb' + ); + expect(pluginOptions.allowedFlows).to.have.callCount(3); + }); + }); }); context('when the user aborts an auth code flow', function () { diff --git a/src/plugin.ts b/src/plugin.ts index d8d5562..659d701 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -52,7 +52,7 @@ type LastIdTokenClaims = interface UserOIDCAuthState { // The information that the driver forwarded to us from the server // about the OIDC Identity Provider config. - serverOIDCMetadata: IdPServerInfo; + serverOIDCMetadata: IdPServerInfo & Pick; // A Promise that resolves when the current authentication attempt // is finished, if there is one at the moment. currentAuthAttempt: Promise | null; @@ -288,7 +288,9 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { // Return the current state for a given [server, username] configuration, // or create a new one if none exists. - private getAuthState(serverMetadata: IdPServerInfo): UserOIDCAuthState { + private getAuthState( + serverMetadata: IdPServerInfo & Pick + ): UserOIDCAuthState { if (!serverMetadata.issuer || typeof serverMetadata.issuer !== 'string') { throw new MongoDBOIDCError(`'issuer' is missing`); } @@ -909,6 +911,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { public async requestToken( params: OIDCCallbackParams ): Promise { + this.logger.emit('mongodb-oidc-plugin:received-server-params', { params }); if (params.version !== 1) { throw new MongoDBOIDCError( // eslint-disable-next-line @typescript-eslint/restrict-template-expressions @@ -926,7 +929,10 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { throw new MongoDBOIDCError('No IdP information provided'); } - const state = this.getAuthState(params.idpInfo); + const state = this.getAuthState({ + ...params.idpInfo, + username: params.username, + }); if (state.currentAuthAttempt) { return await state.currentAuthAttempt; diff --git a/src/types.ts b/src/types.ts index 72425c0..f130b72 100644 --- a/src/types.ts +++ b/src/types.ts @@ -75,6 +75,9 @@ export interface MongoDBOIDCLogEventsMap { 'mongodb-oidc-plugin:missing-id-token': () => void; 'mongodb-oidc-plugin:outbound-http-request': (event: { url: string }) => void; 'mongodb-oidc-plugin:inbound-http-request': (event: { url: string }) => void; + 'mongodb-oidc-plugin:received-server-params': (event: { + params: OIDCCallbackParams; + }) => void; } /** @public */