Skip to content

Commit

Permalink
fix: include username in server metadata key (#194)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
addaleax authored Aug 1, 2024
1 parent e8533b1 commit eb66433
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
45 changes: 44 additions & 1 deletion src/plugin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ async function fetchBrowser({ url }: OpenBrowserOptions): Promise<void> {
function requestToken(
plugin: MongoDBOIDCPlugin,
oidcParams: IdPServerInfo,
abortSignal?: OIDCAbortSignal
abortSignal?: OIDCAbortSignal,
username?: string
): ReturnType<OIDCCallbackFunction> {
return plugin.mongoClientOptions.authMechanismProperties.OIDC_HUMAN_CALLBACK({
timeoutContext: abortSignal,
version: 1,
idpInfo: oidcParams,
username,
});
}

Expand Down Expand Up @@ -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 () {
Expand Down
12 changes: 9 additions & 3 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<OIDCCallbackParams, 'username'>;
// A Promise that resolves when the current authentication attempt
// is finished, if there is one at the moment.
currentAuthAttempt: Promise<IdPServerResponse> | null;
Expand Down Expand Up @@ -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<OIDCCallbackParams, 'username'>
): UserOIDCAuthState {
if (!serverMetadata.issuer || typeof serverMetadata.issuer !== 'string') {
throw new MongoDBOIDCError(`'issuer' is missing`);
}
Expand Down Expand Up @@ -909,6 +911,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
public async requestToken(
params: OIDCCallbackParams
): Promise<IdPServerResponse> {
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
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit eb66433

Please sign in to comment.