From e50a7eabc9ad96a8d2d80d784d3245e8f8ddb653 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 13 Feb 2024 14:52:54 +0100 Subject: [PATCH 01/15] feat(NODE-5616): cache the AWS credentials provider in the MONGODB-AWS auth logic --- src/cmap/connect.ts | 42 ++++++++--------------- src/cmap/connection_pool.ts | 8 +++-- src/mongo_client.ts | 23 +++++++++++++ test/integration/auth/mongodb_aws.test.ts | 20 +++++++++++ 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index c484a80f49..d279c75ef5 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -18,14 +18,7 @@ import { } from '../error'; import { HostAddress, ns, promiseWithResolvers } from '../utils'; import { AuthContext, type AuthProvider } from './auth/auth_provider'; -import { GSSAPI } from './auth/gssapi'; -import { MongoCR } from './auth/mongocr'; -import { MongoDBAWS } from './auth/mongodb_aws'; -import { MongoDBOIDC } from './auth/mongodb_oidc'; -import { Plain } from './auth/plain'; import { AuthMechanism } from './auth/providers'; -import { ScramSHA1, ScramSHA256 } from './auth/scram'; -import { X509 } from './auth/x509'; import { type CommandOptions, Connection, @@ -40,27 +33,18 @@ import { MIN_SUPPORTED_WIRE_VERSION } from './wire_protocol/constants'; -/** @internal */ -export const AUTH_PROVIDERS = new Map([ - [AuthMechanism.MONGODB_AWS, new MongoDBAWS()], - [AuthMechanism.MONGODB_CR, new MongoCR()], - [AuthMechanism.MONGODB_GSSAPI, new GSSAPI()], - [AuthMechanism.MONGODB_OIDC, new MongoDBOIDC()], - [AuthMechanism.MONGODB_PLAIN, new Plain()], - [AuthMechanism.MONGODB_SCRAM_SHA1, new ScramSHA1()], - [AuthMechanism.MONGODB_SCRAM_SHA256, new ScramSHA256()], - [AuthMechanism.MONGODB_X509, new X509()] -]); - /** @public */ export type Stream = Socket | TLSSocket; -export async function connect(options: ConnectionOptions): Promise { +export async function connect( + options: ConnectionOptions, + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined +): Promise { let connection: Connection | null = null; try { const socket = await makeSocket(options); connection = makeConnection(options, socket); - await performInitialHandshake(connection, options); + await performInitialHandshake(connection, options, getAuthProvider); return connection; } catch (error) { connection?.destroy({ force: false }); @@ -104,14 +88,15 @@ function checkSupportedServer(hello: Document, options: ConnectionOptions) { export async function performInitialHandshake( conn: Connection, - options: ConnectionOptions + options: ConnectionOptions, + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined ): Promise { const credentials = options.credentials; if (credentials) { if ( !(credentials.mechanism === AuthMechanism.MONGODB_DEFAULT) && - !AUTH_PROVIDERS.get(credentials.mechanism) + !getAuthProvider(credentials.mechanism) ) { throw new MongoInvalidArgumentError(`AuthMechanism '${credentials.mechanism}' not supported`); } @@ -120,7 +105,7 @@ export async function performInitialHandshake( const authContext = new AuthContext(conn, credentials, options); conn.authContext = authContext; - const handshakeDoc = await prepareHandshakeDocument(authContext); + const handshakeDoc = await prepareHandshakeDocument(authContext, getAuthProvider); // @ts-expect-error: TODO(NODE-5141): The options need to be filtered properly, Connection options differ from Command options const handshakeOptions: CommandOptions = { ...options }; @@ -166,7 +151,7 @@ export async function performInitialHandshake( authContext.response = response; const resolvedCredentials = credentials.resolveAuthMechanism(response); - const provider = AUTH_PROVIDERS.get(resolvedCredentials.mechanism); + const provider = getAuthProvider(resolvedCredentials.mechanism); if (!provider) { throw new MongoInvalidArgumentError( `No AuthProvider for ${resolvedCredentials.mechanism} defined.` @@ -210,7 +195,8 @@ export interface HandshakeDocument extends Document { * This function is only exposed for testing purposes. */ export async function prepareHandshakeDocument( - authContext: AuthContext + authContext: AuthContext, + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined ): Promise { const options = authContext.options; const compressors = options.compressors ? options.compressors : []; @@ -232,7 +218,7 @@ export async function prepareHandshakeDocument( if (credentials.mechanism === AuthMechanism.MONGODB_DEFAULT && credentials.username) { handshakeDoc.saslSupportedMechs = `${credentials.source}.${credentials.username}`; - const provider = AUTH_PROVIDERS.get(AuthMechanism.MONGODB_SCRAM_SHA256); + const provider = getAuthProvider(AuthMechanism.MONGODB_SCRAM_SHA256); if (!provider) { // This auth mechanism is always present. throw new MongoInvalidArgumentError( @@ -241,7 +227,7 @@ export async function prepareHandshakeDocument( } return provider.prepare(handshakeDoc, authContext); } - const provider = AUTH_PROVIDERS.get(credentials.mechanism); + const provider = getAuthProvider(credentials.mechanism); if (!provider) { throw new MongoInvalidArgumentError(`No AuthProvider for ${credentials.mechanism} defined.`); } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 4fe5249738..64803dcf76 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -28,7 +28,7 @@ import { import { CancellationToken, TypedEventEmitter } from '../mongo_types'; import type { Server } from '../sdam/server'; import { type Callback, eachAsync, List, makeCounter, TimeoutController } from '../utils'; -import { AUTH_PROVIDERS, connect } from './connect'; +import { connect } from './connect'; import { Connection, type ConnectionEvents, type ConnectionOptions } from './connection'; import { ConnectionCheckedInEvent, @@ -622,7 +622,9 @@ export class ConnectionPool extends TypedEventEmitter { ); } const resolvedCredentials = credentials.resolveAuthMechanism(connection.hello); - const provider = AUTH_PROVIDERS.get(resolvedCredentials.mechanism); + const provider = this[kServer].topology.client?.s.getAuthProvider( + resolvedCredentials.mechanism + ); if (!provider) { return callback( new MongoMissingCredentialsError( @@ -710,7 +712,7 @@ export class ConnectionPool extends TypedEventEmitter { new ConnectionCreatedEvent(this, { id: connectOptions.id }) ); - connect(connectOptions).then( + connect(connectOptions, this[kServer].topology.client.s?.getAuthProvider).then( connection => { // The pool might have closed since we started trying to create a connection if (this[kPoolState] !== PoolState.ready) { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 476a912aec..a7b4d39e6d 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -6,12 +6,20 @@ import { promisify } from 'util'; import { type BSONSerializeOptions, type Document, resolveBSONOptions } from './bson'; import { ChangeStream, type ChangeStreamDocument, type ChangeStreamOptions } from './change_stream'; import type { AutoEncrypter, AutoEncryptionOptions } from './client-side-encryption/auto_encrypter'; +import { type AuthProvider } from './cmap/auth/auth_provider'; +import { GSSAPI } from './cmap/auth/gssapi'; import { type AuthMechanismProperties, DEFAULT_ALLOWED_HOSTS, type MongoCredentials } from './cmap/auth/mongo_credentials'; +import { MongoCR } from './cmap/auth/mongocr'; +import { MongoDBAWS } from './cmap/auth/mongodb_aws'; +import { MongoDBOIDC } from './cmap/auth/mongodb_oidc'; +import { Plain } from './cmap/auth/plain'; import { AuthMechanism } from './cmap/auth/providers'; +import { ScramSHA1, ScramSHA256 } from './cmap/auth/scram'; +import { X509 } from './cmap/auth/x509'; import type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS } from './cmap/connect'; import type { Connection } from './cmap/connection'; import type { ClientMetadata } from './cmap/handshake/client_metadata'; @@ -297,6 +305,7 @@ export interface MongoClientPrivate { bsonOptions: BSONSerializeOptions; namespace: MongoDBNamespace; hasBeenClosed: boolean; + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; /** * We keep a reference to the sessions that are acquired from the pool. * - used to track and close all sessions in client.close() (which is non-standard behavior) @@ -371,6 +380,17 @@ export class MongoClient extends TypedEventEmitter { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; + const authProviders = new Map([ + [AuthMechanism.MONGODB_AWS, new MongoDBAWS()], + [AuthMechanism.MONGODB_CR, new MongoCR()], + [AuthMechanism.MONGODB_GSSAPI, new GSSAPI()], + [AuthMechanism.MONGODB_OIDC, new MongoDBOIDC()], + [AuthMechanism.MONGODB_PLAIN, new Plain()], + [AuthMechanism.MONGODB_SCRAM_SHA1, new ScramSHA1()], + [AuthMechanism.MONGODB_SCRAM_SHA256, new ScramSHA256()], + [AuthMechanism.MONGODB_X509, new X509()] + ]); + // The internal state this.s = { url, @@ -380,6 +400,9 @@ export class MongoClient extends TypedEventEmitter { sessionPool: new ServerSessionPool(this), activeSessions: new Set(), + getAuthProvider(name: AuthMechanism | string) { + return authProviders.get(name); + }, get options() { return client[kOptions]; }, diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index e075b67775..76910dd90b 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -183,6 +183,7 @@ describe('MONGODB-AWS', function () { calledWith: [] } ]; + let n = 0; for (const test of tests) { context(test.ctx, () => { @@ -221,6 +222,7 @@ describe('MONGODB-AWS', function () { calledArguments = []; MongoDBAWS.credentialProvider = { fromNodeProviderChain(...args) { + n += 1; calledArguments = args; return credentialProvider.fromNodeProviderChain(...args); } @@ -253,6 +255,24 @@ describe('MONGODB-AWS', function () { expect(calledArguments).to.deep.equal(test.calledWith); }); + + it('stores a provider instance per client after the first AWS auth attemp', async function () { + await client?.close(); + await client?.connect(); + + const result = await client + .db('aws') + .collection('aws_test') + .estimatedDocumentCount() + .catch(error => error); + + expect(result).to.not.be.instanceOf(MongoServerError); + expect(n).to.deep.equal(1); + + expect(client).to.have.nested.property('s.getAuthProvider'); + const provider = client.s.getAuthProvider('MONGODB-AWS'); + expect(provider).to.be.instanceOf(MongoDBAWS); + }); }); } }); From 0f0143a4f88266bf862b8ab45910ec1244b166dd Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 13 Feb 2024 15:40:43 +0100 Subject: [PATCH 02/15] test: test aws instance per client --- test/integration/auth/mongodb_aws.test.ts | 30 ++++++++--------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index 76910dd90b..6d3a3bfff2 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -67,6 +67,16 @@ describe('MONGODB-AWS', function () { .that.equals(''); }); + it('should store a provider instance per client', function () { + client = this.configuration.newClient(this.configuration.url(), { + authMechanismProperties: { AWS_SESSION_TOKEN: '' } + }); + + expect(client).to.have.nested.property('s.getAuthProvider'); + const provider = client.s.getAuthProvider('MONGODB-AWS'); + expect(provider).to.be.instanceOf(MongoDBAWS); + }); + describe('EC2 with missing credentials', () => { let client; @@ -183,7 +193,6 @@ describe('MONGODB-AWS', function () { calledWith: [] } ]; - let n = 0; for (const test of tests) { context(test.ctx, () => { @@ -222,7 +231,6 @@ describe('MONGODB-AWS', function () { calledArguments = []; MongoDBAWS.credentialProvider = { fromNodeProviderChain(...args) { - n += 1; calledArguments = args; return credentialProvider.fromNodeProviderChain(...args); } @@ -255,24 +263,6 @@ describe('MONGODB-AWS', function () { expect(calledArguments).to.deep.equal(test.calledWith); }); - - it('stores a provider instance per client after the first AWS auth attemp', async function () { - await client?.close(); - await client?.connect(); - - const result = await client - .db('aws') - .collection('aws_test') - .estimatedDocumentCount() - .catch(error => error); - - expect(result).to.not.be.instanceOf(MongoServerError); - expect(n).to.deep.equal(1); - - expect(client).to.have.nested.property('s.getAuthProvider'); - const provider = client.s.getAuthProvider('MONGODB-AWS'); - expect(provider).to.be.instanceOf(MongoDBAWS); - }); }); } }); From 108967f1acfea68bfcbf89522bc7ca13fd779224 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 14 Feb 2024 14:30:05 +0100 Subject: [PATCH 03/15] test: try with cashed provider --- src/cmap/auth/mongodb_aws.ts | 106 +++++++++++----------- test/integration/auth/mongodb_aws.test.ts | 3 + 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/src/cmap/auth/mongodb_aws.ts b/src/cmap/auth/mongodb_aws.ts index e2b4604084..71486c2b8f 100644 --- a/src/cmap/auth/mongodb_aws.ts +++ b/src/cmap/auth/mongodb_aws.ts @@ -4,7 +4,7 @@ import { promisify } from 'util'; import type { Binary, BSONSerializeOptions } from '../../bson'; import * as BSON from '../../bson'; -import { aws4, getAwsCredentialProvider } from '../../deps'; +import { aws4, type AWSCredentials, getAwsCredentialProvider } from '../../deps'; import { MongoAWSError, MongoCompatibilityError, @@ -58,11 +58,41 @@ interface AWSSaslContinuePayload { export class MongoDBAWS extends AuthProvider { static credentialProvider: ReturnType | null = null; + static provider?: () => Promise; randomBytesAsync: (size: number) => Promise; constructor() { super(); this.randomBytesAsync = promisify(crypto.randomBytes); + MongoDBAWS.credentialProvider ??= getAwsCredentialProvider(); + + let { AWS_STS_REGIONAL_ENDPOINTS = '', AWS_REGION = '' } = process.env; + AWS_STS_REGIONAL_ENDPOINTS = AWS_STS_REGIONAL_ENDPOINTS.toLowerCase(); + AWS_REGION = AWS_REGION.toLowerCase(); + + /** The option setting should work only for users who have explicit settings in their environment, the driver should not encode "defaults" */ + const awsRegionSettingsExist = + AWS_REGION.length !== 0 && AWS_STS_REGIONAL_ENDPOINTS.length !== 0; + + /** + * If AWS_STS_REGIONAL_ENDPOINTS is set to regional, users are opting into the new behavior of respecting the region settings + * + * If AWS_STS_REGIONAL_ENDPOINTS is set to legacy, then "old" regions need to keep using the global setting. + * Technically the SDK gets this wrong, it reaches out to 'sts.us-east-1.amazonaws.com' when it should be 'sts.amazonaws.com'. + * That is not our bug to fix here. We leave that up to the SDK. + */ + const useRegionalSts = + AWS_STS_REGIONAL_ENDPOINTS === 'regional' || + (AWS_STS_REGIONAL_ENDPOINTS === 'legacy' && !LEGACY_REGIONS.has(AWS_REGION)); + + if ('fromNodeProviderChain' in MongoDBAWS.credentialProvider) { + MongoDBAWS.provider = + awsRegionSettingsExist && useRegionalSts + ? MongoDBAWS.credentialProvider.fromNodeProviderChain({ + clientConfig: { region: AWS_REGION } + }) + : MongoDBAWS.credentialProvider.fromNodeProviderChain(); + } } override async auth(authContext: AuthContext): Promise { @@ -198,11 +228,31 @@ async function makeTempCredentials(credentials: MongoCredentials): Promise { @@ -232,6 +233,7 @@ describe('MONGODB-AWS', function () { MongoDBAWS.credentialProvider = { fromNodeProviderChain(...args) { calledArguments = args; + n += 1; return credentialProvider.fromNodeProviderChain(...args); } }; @@ -260,6 +262,7 @@ describe('MONGODB-AWS', function () { expect(result).to.not.be.instanceOf(MongoServerError); expect(result).to.be.a('number'); + expect(n).to.be.eql(1); expect(calledArguments).to.deep.equal(test.calledWith); }); From 197e10944b22f0653d7602711cf26bfdf42ab42c Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 14 Feb 2024 15:06:02 +0100 Subject: [PATCH 04/15] test: try to reset counter per test --- src/cmap/auth/mongodb_aws.ts | 6 +++++- test/integration/auth/mongodb_aws.test.ts | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cmap/auth/mongodb_aws.ts b/src/cmap/auth/mongodb_aws.ts index 71486c2b8f..3d85fceeba 100644 --- a/src/cmap/auth/mongodb_aws.ts +++ b/src/cmap/auth/mongodb_aws.ts @@ -230,7 +230,11 @@ async function makeTempCredentials(credentials: MongoCredentials): Promise { @@ -201,6 +200,7 @@ describe('MONGODB-AWS', function () { let storedEnv; let calledArguments; let shouldSkip = false; + let n; const envCheck = () => { const { AWS_WEB_IDENTITY_TOKEN_FILE = '' } = process.env; @@ -216,6 +216,7 @@ describe('MONGODB-AWS', function () { } client = this.configuration.newClient(process.env.MONGODB_URI); + n = 0; storedEnv = process.env; if (test.env.AWS_STS_REGIONAL_ENDPOINTS === undefined) { From c036433d60d2d8b7cd824217efee6f8eb4b9aa17 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 14 Feb 2024 15:52:44 +0100 Subject: [PATCH 05/15] test: check existing assertions --- src/cmap/auth/mongodb_aws.ts | 8 ++++++++ test/integration/auth/mongodb_aws.test.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/cmap/auth/mongodb_aws.ts b/src/cmap/auth/mongodb_aws.ts index 3d85fceeba..01dc022e6a 100644 --- a/src/cmap/auth/mongodb_aws.ts +++ b/src/cmap/auth/mongodb_aws.ts @@ -85,6 +85,14 @@ export class MongoDBAWS extends AuthProvider { AWS_STS_REGIONAL_ENDPOINTS === 'regional' || (AWS_STS_REGIONAL_ENDPOINTS === 'legacy' && !LEGACY_REGIONS.has(AWS_REGION)); + console.log('awsRegionSettingsExist----------------------'); + console.log(awsRegionSettingsExist); + console.log('----------------------'); + + console.log('useRegionalSts----------------------'); + console.log(useRegionalSts); + console.log('----------------------'); + if ('fromNodeProviderChain' in MongoDBAWS.credentialProvider) { MongoDBAWS.provider = awsRegionSettingsExist && useRegionalSts diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index 3304d416e0..e761223824 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -263,7 +263,7 @@ describe('MONGODB-AWS', function () { expect(result).to.not.be.instanceOf(MongoServerError); expect(result).to.be.a('number'); - expect(n).to.be.eql(1); + // expect(n).to.be.eql(1); expect(calledArguments).to.deep.equal(test.calledWith); }); From a9df63ceebe08eaf2be53ef6f666eae8d981bd2b Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 14 Feb 2024 16:04:36 +0100 Subject: [PATCH 06/15] test: try without static --- src/cmap/auth/mongodb_aws.ts | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/cmap/auth/mongodb_aws.ts b/src/cmap/auth/mongodb_aws.ts index 01dc022e6a..fc00ae3782 100644 --- a/src/cmap/auth/mongodb_aws.ts +++ b/src/cmap/auth/mongodb_aws.ts @@ -58,7 +58,7 @@ interface AWSSaslContinuePayload { export class MongoDBAWS extends AuthProvider { static credentialProvider: ReturnType | null = null; - static provider?: () => Promise; + provider?: () => Promise; randomBytesAsync: (size: number) => Promise; constructor() { @@ -85,16 +85,8 @@ export class MongoDBAWS extends AuthProvider { AWS_STS_REGIONAL_ENDPOINTS === 'regional' || (AWS_STS_REGIONAL_ENDPOINTS === 'legacy' && !LEGACY_REGIONS.has(AWS_REGION)); - console.log('awsRegionSettingsExist----------------------'); - console.log(awsRegionSettingsExist); - console.log('----------------------'); - - console.log('useRegionalSts----------------------'); - console.log(useRegionalSts); - console.log('----------------------'); - if ('fromNodeProviderChain' in MongoDBAWS.credentialProvider) { - MongoDBAWS.provider = + this.provider = awsRegionSettingsExist && useRegionalSts ? MongoDBAWS.credentialProvider.fromNodeProviderChain({ clientConfig: { region: AWS_REGION } @@ -121,7 +113,7 @@ export class MongoDBAWS extends AuthProvider { } if (!authContext.credentials.username) { - authContext.credentials = await makeTempCredentials(authContext.credentials); + authContext.credentials = await makeTempCredentials(authContext.credentials, this.provider); } const { credentials } = authContext; @@ -219,7 +211,10 @@ interface AWSTempCredentials { Expiration?: Date; } -async function makeTempCredentials(credentials: MongoCredentials): Promise { +async function makeTempCredentials( + credentials: MongoCredentials, + provider?: () => Promise +): Promise { function makeMongoCredentialsFromAWSTemp(creds: AWSTempCredentials) { if (!creds.AccessKeyId || !creds.SecretAccessKey || !creds.Token) { throw new MongoMissingCredentialsError('Could not obtain temporary MONGODB-AWS credentials'); @@ -239,7 +234,7 @@ async function makeTempCredentials(credentials: MongoCredentials): Promise Date: Thu, 15 Feb 2024 12:15:23 +0100 Subject: [PATCH 07/15] test: try to move client creation --- test/integration/auth/mongodb_aws.test.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index e761223824..6856990a8e 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -154,7 +154,6 @@ describe('MONGODB-AWS', function () { }, calledWith: [] }, - { ctx: 'when AWS_STS_REGIONAL_ENDPOINTS is set to regional and region is legacy', title: 'uses the region from the environment', @@ -173,7 +172,6 @@ describe('MONGODB-AWS', function () { }, calledWith: [{ clientConfig: { region: 'sa-east-1' } }] }, - { ctx: 'when AWS_STS_REGIONAL_ENDPOINTS is set to legacy and region is legacy', title: 'uses the region from the environment', @@ -215,9 +213,6 @@ describe('MONGODB-AWS', function () { return this.skip(); } - client = this.configuration.newClient(process.env.MONGODB_URI); - n = 0; - storedEnv = process.env; if (test.env.AWS_STS_REGIONAL_ENDPOINTS === undefined) { delete process.env.AWS_STS_REGIONAL_ENDPOINTS; @@ -230,7 +225,8 @@ describe('MONGODB-AWS', function () { process.env.AWS_REGION = test.env.AWS_REGION; } - calledArguments = []; + n = 0; + MongoDBAWS.credentialProvider = { fromNodeProviderChain(...args) { calledArguments = args; @@ -238,6 +234,8 @@ describe('MONGODB-AWS', function () { return credentialProvider.fromNodeProviderChain(...args); } }; + + client = this.configuration.newClient(process.env.MONGODB_URI); }); afterEach(() => { @@ -263,7 +261,7 @@ describe('MONGODB-AWS', function () { expect(result).to.not.be.instanceOf(MongoServerError); expect(result).to.be.a('number'); - // expect(n).to.be.eql(1); + expect(n).to.be.eql(1); expect(calledArguments).to.deep.equal(test.calledWith); }); From 3307cb132776ade5da5457ab4368ef1f4ace571b Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 15 Feb 2024 13:21:07 +0100 Subject: [PATCH 08/15] test: move n check to a separate test --- test/integration/auth/mongodb_aws.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index 6856990a8e..2536f544e1 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -67,7 +67,7 @@ describe('MONGODB-AWS', function () { .that.equals(''); }); - it('should store a provider instance per client', function () { + it('should store a MongoDBAWS provider instance per client', function () { client = this.configuration.newClient(this.configuration.url(), { authMechanismProperties: { AWS_SESSION_TOKEN: '' } }); @@ -265,6 +265,18 @@ describe('MONGODB-AWS', function () { expect(calledArguments).to.deep.equal(test.calledWith); }); + + it('creates one aws provider pre MongoDBAWS instance', async function () { + await client.close(); + await client.connect(); + await client + .db('aws') + .collection('aws_test') + .estimatedDocumentCount() + .catch(error => error); + + expect(n).to.be.eql(1); + }); }); } }); From f67e257b3d08a24103cfad2720c5b84074e2e17f Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 15 Feb 2024 19:30:23 +0100 Subject: [PATCH 09/15] refactor: store getAuthProvider in connection options --- src/cmap/auth/auth_provider.ts | 4 ++++ src/cmap/connect.ts | 20 ++++++++++---------- src/cmap/connection.ts | 5 ++++- src/cmap/connection_pool.ts | 5 +++-- src/index.ts | 3 ++- src/mongo_client.ts | 3 +++ test/unit/cmap/connect.test.ts | 4 +++- 7 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 37a47889b9..e40c791ea5 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -34,6 +34,10 @@ export class AuthContext { } } +/** + * Provider used during authentication. + * @internal + */ export abstract class AuthProvider { /** * Prepare the handshake document before the initial handshake. diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index d279c75ef5..5f383a92c4 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -36,15 +36,12 @@ import { /** @public */ export type Stream = Socket | TLSSocket; -export async function connect( - options: ConnectionOptions, - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined -): Promise { +export async function connect(options: ConnectionOptions): Promise { let connection: Connection | null = null; try { const socket = await makeSocket(options); connection = makeConnection(options, socket); - await performInitialHandshake(connection, options, getAuthProvider); + await performInitialHandshake(connection, options); return connection; } catch (error) { connection?.destroy({ force: false }); @@ -88,15 +85,14 @@ function checkSupportedServer(hello: Document, options: ConnectionOptions) { export async function performInitialHandshake( conn: Connection, - options: ConnectionOptions, - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined + options: ConnectionOptions ): Promise { const credentials = options.credentials; if (credentials) { if ( !(credentials.mechanism === AuthMechanism.MONGODB_DEFAULT) && - !getAuthProvider(credentials.mechanism) + !options.getAuthProvider(credentials.mechanism) ) { throw new MongoInvalidArgumentError(`AuthMechanism '${credentials.mechanism}' not supported`); } @@ -105,7 +101,7 @@ export async function performInitialHandshake( const authContext = new AuthContext(conn, credentials, options); conn.authContext = authContext; - const handshakeDoc = await prepareHandshakeDocument(authContext, getAuthProvider); + const handshakeDoc = await prepareHandshakeDocument(authContext, options.getAuthProvider); // @ts-expect-error: TODO(NODE-5141): The options need to be filtered properly, Connection options differ from Command options const handshakeOptions: CommandOptions = { ...options }; @@ -151,7 +147,7 @@ export async function performInitialHandshake( authContext.response = response; const resolvedCredentials = credentials.resolveAuthMechanism(response); - const provider = getAuthProvider(resolvedCredentials.mechanism); + const provider = options.getAuthProvider(resolvedCredentials.mechanism); if (!provider) { throw new MongoInvalidArgumentError( `No AuthProvider for ${resolvedCredentials.mechanism} defined.` @@ -176,6 +172,10 @@ export async function performInitialHandshake( conn.established = true; } +/** + * HandshakeDocument used during authentication. + * @internal + */ export interface HandshakeDocument extends Document { /** * @deprecated Use hello instead diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 3406fed594..4da1b54a8a 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -40,8 +40,9 @@ import { uuidV4 } from '../utils'; import type { WriteConcern } from '../write_concern'; -import type { AuthContext } from './auth/auth_provider'; +import type { AuthContext, AuthProvider } from './auth/auth_provider'; import type { MongoCredentials } from './auth/mongo_credentials'; +import { type AuthMechanism } from './auth/providers'; import { CommandFailedEvent, CommandStartedEvent, @@ -117,6 +118,8 @@ export interface ConnectionOptions metadata: ClientMetadata; /** @internal */ mongoLogger?: MongoLogger | undefined; + /** @internal */ + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; } /** @internal */ diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 64803dcf76..2a88541ebd 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -702,7 +702,8 @@ export class ConnectionPool extends TypedEventEmitter { id: this[kConnectionCounter].next().value, generation: this[kGeneration], cancellationToken: this[kCancellationToken], - mongoLogger: this.mongoLogger + mongoLogger: this.mongoLogger, + getAuthProvider: this[kServer].topology.client.s?.getAuthProvider }; this[kPending]++; @@ -712,7 +713,7 @@ export class ConnectionPool extends TypedEventEmitter { new ConnectionCreatedEvent(this, { id: connectOptions.id }) ); - connect(connectOptions, this[kServer].topology.client.s?.getAuthProvider).then( + connect(connectOptions).then( connection => { // The pool might have closed since we started trying to create a connection if (this[kPoolState] !== PoolState.ready) { diff --git a/src/index.ts b/src/index.ts index 6366e74665..30764627c1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -243,7 +243,7 @@ export type { CSFLEKMSTlsOptions, StateMachineExecutable } from './client-side-encryption/state_machine'; -export type { AuthContext } from './cmap/auth/auth_provider'; +export type { AuthContext, AuthProvider } from './cmap/auth/auth_provider'; export type { AuthMechanismProperties, MongoCredentials, @@ -268,6 +268,7 @@ export type { OpResponseOptions, WriteProtocolMessageType } from './cmap/commands'; +export type { HandshakeDocument } from './cmap/connect'; export type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS, Stream } from './cmap/connect'; export type { CommandOptions, diff --git a/src/mongo_client.ts b/src/mongo_client.ts index a7b4d39e6d..e896a8bc93 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -856,6 +856,9 @@ export interface MongoOptions /** @internal */ connectionType?: typeof Connection; + /** @internal */ + getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; + /** @internal */ encrypter: Encrypter; /** @internal */ diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 7f69f54d17..26408723c0 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -1,5 +1,6 @@ import { expect } from 'chai'; +import { Plain } from '../../../src/cmap/auth/plain'; import { CancellationToken, type ClientMetadata, @@ -22,7 +23,8 @@ const CONNECT_DEFAULTS = { generation: 1, monitorCommands: false, metadata: {} as ClientMetadata, - loadBalanced: false + loadBalanced: false, + getAuthProvider: () => new Plain() }; describe('Connect Tests', function () { From 480a49d8c4c480f2fd319fb98ac92eac4a5abf05 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 16 Feb 2024 12:53:39 +0100 Subject: [PATCH 10/15] fix: import from mongodb --- test/unit/cmap/connect.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 26408723c0..62f2d3c0b4 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; -import { Plain } from '../../../src/cmap/auth/plain'; import { CancellationToken, type ClientMetadata, @@ -12,6 +11,7 @@ import { LEGACY_HELLO_COMMAND, MongoCredentials, MongoNetworkError, + Plain, prepareHandshakeDocument } from '../../mongodb'; import { genClusterTime } from '../../tools/common'; From fbb553757612bb275630326b3f1ad9e6ebc1cc24 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 16 Feb 2024 14:16:41 +0100 Subject: [PATCH 11/15] refactor: try with factory --- src/mongo_client.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index e896a8bc93..e3ba3171dd 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -328,8 +328,21 @@ export type MongoClientEvents = Pick AuthProvider>([ + [AuthMechanism.MONGODB_AWS, () => new MongoDBAWS()], + [AuthMechanism.MONGODB_CR, () => new MongoCR()], + [AuthMechanism.MONGODB_GSSAPI, () => new GSSAPI()], + [AuthMechanism.MONGODB_OIDC, () => new MongoDBOIDC()], + [AuthMechanism.MONGODB_PLAIN, () => new Plain()], + [AuthMechanism.MONGODB_SCRAM_SHA1, () => new ScramSHA1()], + [AuthMechanism.MONGODB_SCRAM_SHA256, () => new ScramSHA256()], + [AuthMechanism.MONGODB_X509, () => new X509()] +]); + /** * The **MongoClient** class is a class that allows for making Connections to MongoDB. * @public @@ -380,16 +393,7 @@ export class MongoClient extends TypedEventEmitter { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; - const authProviders = new Map([ - [AuthMechanism.MONGODB_AWS, new MongoDBAWS()], - [AuthMechanism.MONGODB_CR, new MongoCR()], - [AuthMechanism.MONGODB_GSSAPI, new GSSAPI()], - [AuthMechanism.MONGODB_OIDC, new MongoDBOIDC()], - [AuthMechanism.MONGODB_PLAIN, new Plain()], - [AuthMechanism.MONGODB_SCRAM_SHA1, new ScramSHA1()], - [AuthMechanism.MONGODB_SCRAM_SHA256, new ScramSHA256()], - [AuthMechanism.MONGODB_X509, new X509()] - ]); + const clientAuthProviders: { [authMechanism: AuthMechanism | string]: AuthProvider } = {}; // The internal state this.s = { @@ -401,7 +405,7 @@ export class MongoClient extends TypedEventEmitter { activeSessions: new Set(), getAuthProvider(name: AuthMechanism | string) { - return authProviders.get(name); + return clientAuthProviders[name] || AUTH_PROVIDERS.get(name)?.(); }, get options() { return client[kOptions]; From 40ea9a55b66c90d828696a5ff240e0ee33fd1c1d Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 16 Feb 2024 14:45:04 +0100 Subject: [PATCH 12/15] fix: save provider per client --- src/mongo_client.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index e3ba3171dd..cbc1c20509 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -405,7 +405,13 @@ export class MongoClient extends TypedEventEmitter { activeSessions: new Set(), getAuthProvider(name: AuthMechanism | string) { - return clientAuthProviders[name] || AUTH_PROVIDERS.get(name)?.(); + if (!clientAuthProviders[name]) { + const provider = AUTH_PROVIDERS.get(name)?.(); + if (provider) { + clientAuthProviders[name] = provider; + } + } + return clientAuthProviders[name]; }, get options() { return client[kOptions]; From 74e9c801d289c8b4df12fbc95a39345a85f3b435 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 16 Feb 2024 15:40:15 +0100 Subject: [PATCH 13/15] refactor: clean up --- src/cmap/auth/mongodb_aws.ts | 8 ++------ test/integration/auth/mongodb_aws.test.ts | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/cmap/auth/mongodb_aws.ts b/src/cmap/auth/mongodb_aws.ts index fc00ae3782..b6676656cc 100644 --- a/src/cmap/auth/mongodb_aws.ts +++ b/src/cmap/auth/mongodb_aws.ts @@ -57,7 +57,7 @@ interface AWSSaslContinuePayload { } export class MongoDBAWS extends AuthProvider { - static credentialProvider: ReturnType | null = null; + static credentialProvider: ReturnType; provider?: () => Promise; randomBytesAsync: (size: number) => Promise; @@ -233,11 +233,7 @@ async function makeTempCredentials( // Check if the AWS credential provider from the SDK is present. If not, // use the old method. - if ( - provider && - MongoDBAWS.credentialProvider && - !('kModuleError' in MongoDBAWS.credentialProvider) - ) { + if (provider && !('kModuleError' in MongoDBAWS.credentialProvider)) { /* * Creates a credential provider that will attempt to find credentials from the * following sources (listed in order of precedence): diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index 2536f544e1..5218818a7c 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -266,7 +266,7 @@ describe('MONGODB-AWS', function () { expect(calledArguments).to.deep.equal(test.calledWith); }); - it('creates one aws provider pre MongoDBAWS instance', async function () { + it('creates one AWS provider per client', async function () { await client.close(); await client.connect(); await client From 3e5999c7352545b08979a0c7f3b5c68cdcd95425 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Mon, 19 Feb 2024 19:35:24 +0100 Subject: [PATCH 14/15] refactor: move client auth providers to a separate module --- src/cmap/connect.ts | 15 ++++--- src/cmap/connection.ts | 8 ++-- src/cmap/connection_pool.ts | 4 +- src/index.ts | 1 + src/mongo_client.ts | 40 ++--------------- src/mongo_client_auth_providers.ts | 44 +++++++++++++++++++ test/integration/auth/mongodb_aws.test.ts | 27 +++++++----- .../connection.test.ts | 4 +- test/unit/cmap/connect.test.ts | 8 ++-- test/unit/cmap/connection.test.ts | 24 +++++++--- test/unit/cmap/connection_pool.test.js | 4 ++ test/unit/index.test.ts | 1 + test/unit/mongo_client.test.js | 21 ++++++++- 13 files changed, 127 insertions(+), 74 deletions(-) create mode 100644 src/mongo_client_auth_providers.ts diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 5f383a92c4..9b3e25d827 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -16,8 +16,9 @@ import { MongoRuntimeError, needsRetryableWriteLabel } from '../error'; +import { type MongoClientAuthProviders } from '../mongo_client_auth_providers'; import { HostAddress, ns, promiseWithResolvers } from '../utils'; -import { AuthContext, type AuthProvider } from './auth/auth_provider'; +import { AuthContext } from './auth/auth_provider'; import { AuthMechanism } from './auth/providers'; import { type CommandOptions, @@ -92,7 +93,7 @@ export async function performInitialHandshake( if (credentials) { if ( !(credentials.mechanism === AuthMechanism.MONGODB_DEFAULT) && - !options.getAuthProvider(credentials.mechanism) + !options.authProviders.getOrCreateProvider(credentials.mechanism) ) { throw new MongoInvalidArgumentError(`AuthMechanism '${credentials.mechanism}' not supported`); } @@ -101,7 +102,7 @@ export async function performInitialHandshake( const authContext = new AuthContext(conn, credentials, options); conn.authContext = authContext; - const handshakeDoc = await prepareHandshakeDocument(authContext, options.getAuthProvider); + const handshakeDoc = await prepareHandshakeDocument(authContext, options.authProviders); // @ts-expect-error: TODO(NODE-5141): The options need to be filtered properly, Connection options differ from Command options const handshakeOptions: CommandOptions = { ...options }; @@ -147,7 +148,7 @@ export async function performInitialHandshake( authContext.response = response; const resolvedCredentials = credentials.resolveAuthMechanism(response); - const provider = options.getAuthProvider(resolvedCredentials.mechanism); + const provider = options.authProviders.getOrCreateProvider(resolvedCredentials.mechanism); if (!provider) { throw new MongoInvalidArgumentError( `No AuthProvider for ${resolvedCredentials.mechanism} defined.` @@ -196,7 +197,7 @@ export interface HandshakeDocument extends Document { */ export async function prepareHandshakeDocument( authContext: AuthContext, - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined + authProviders: MongoClientAuthProviders ): Promise { const options = authContext.options; const compressors = options.compressors ? options.compressors : []; @@ -218,7 +219,7 @@ export async function prepareHandshakeDocument( if (credentials.mechanism === AuthMechanism.MONGODB_DEFAULT && credentials.username) { handshakeDoc.saslSupportedMechs = `${credentials.source}.${credentials.username}`; - const provider = getAuthProvider(AuthMechanism.MONGODB_SCRAM_SHA256); + const provider = authProviders.getOrCreateProvider(AuthMechanism.MONGODB_SCRAM_SHA256); if (!provider) { // This auth mechanism is always present. throw new MongoInvalidArgumentError( @@ -227,7 +228,7 @@ export async function prepareHandshakeDocument( } return provider.prepare(handshakeDoc, authContext); } - const provider = getAuthProvider(credentials.mechanism); + const provider = authProviders.getOrCreateProvider(credentials.mechanism); if (!provider) { throw new MongoInvalidArgumentError(`No AuthProvider for ${credentials.mechanism} defined.`); } diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 4da1b54a8a..2f7a80ad9c 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -24,6 +24,7 @@ import { MongoWriteConcernError } from '../error'; import type { ServerApi, SupportedNodeConnectionOptions } from '../mongo_client'; +import { type MongoClientAuthProviders } from '../mongo_client_auth_providers'; import { MongoLoggableComponent, type MongoLogger, SeverityLevel } from '../mongo_logger'; import { type CancellationToken, TypedEventEmitter } from '../mongo_types'; import type { ReadPreferenceLike } from '../read_preference'; @@ -40,9 +41,8 @@ import { uuidV4 } from '../utils'; import type { WriteConcern } from '../write_concern'; -import type { AuthContext, AuthProvider } from './auth/auth_provider'; +import type { AuthContext } from './auth/auth_provider'; import type { MongoCredentials } from './auth/mongo_credentials'; -import { type AuthMechanism } from './auth/providers'; import { CommandFailedEvent, CommandStartedEvent, @@ -110,6 +110,8 @@ export interface ConnectionOptions /** @internal */ connectionType?: any; credentials?: MongoCredentials; + /** @internal */ + authProviders: MongoClientAuthProviders; connectTimeoutMS?: number; tls: boolean; noDelay?: boolean; @@ -118,8 +120,6 @@ export interface ConnectionOptions metadata: ClientMetadata; /** @internal */ mongoLogger?: MongoLogger | undefined; - /** @internal */ - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; } /** @internal */ diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 2a88541ebd..b5e0818061 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -622,7 +622,7 @@ export class ConnectionPool extends TypedEventEmitter { ); } const resolvedCredentials = credentials.resolveAuthMechanism(connection.hello); - const provider = this[kServer].topology.client?.s.getAuthProvider( + const provider = this[kServer].topology.client.s.authProviders.getOrCreateProvider( resolvedCredentials.mechanism ); if (!provider) { @@ -703,7 +703,7 @@ export class ConnectionPool extends TypedEventEmitter { generation: this[kGeneration], cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, - getAuthProvider: this[kServer].topology.client.s?.getAuthProvider + authProviders: this[kServer].topology.client.s.authProviders }; this[kPending]++; diff --git a/src/index.ts b/src/index.ts index 30764627c1..aae568dd79 100644 --- a/src/index.ts +++ b/src/index.ts @@ -366,6 +366,7 @@ export type { SupportedTLSSocketOptions, WithSessionCallback } from './mongo_client'; +export { MongoClientAuthProviders } from './mongo_client_auth_providers'; export type { Log, LogComponentSeveritiesClientOptions, diff --git a/src/mongo_client.ts b/src/mongo_client.ts index cbc1c20509..be039944a4 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -6,20 +6,12 @@ import { promisify } from 'util'; import { type BSONSerializeOptions, type Document, resolveBSONOptions } from './bson'; import { ChangeStream, type ChangeStreamDocument, type ChangeStreamOptions } from './change_stream'; import type { AutoEncrypter, AutoEncryptionOptions } from './client-side-encryption/auto_encrypter'; -import { type AuthProvider } from './cmap/auth/auth_provider'; -import { GSSAPI } from './cmap/auth/gssapi'; import { type AuthMechanismProperties, DEFAULT_ALLOWED_HOSTS, type MongoCredentials } from './cmap/auth/mongo_credentials'; -import { MongoCR } from './cmap/auth/mongocr'; -import { MongoDBAWS } from './cmap/auth/mongodb_aws'; -import { MongoDBOIDC } from './cmap/auth/mongodb_oidc'; -import { Plain } from './cmap/auth/plain'; import { AuthMechanism } from './cmap/auth/providers'; -import { ScramSHA1, ScramSHA256 } from './cmap/auth/scram'; -import { X509 } from './cmap/auth/x509'; import type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS } from './cmap/connect'; import type { Connection } from './cmap/connection'; import type { ClientMetadata } from './cmap/handshake/client_metadata'; @@ -29,6 +21,7 @@ import { MONGO_CLIENT_EVENTS } from './constants'; import { Db, type DbOptions } from './db'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; +import { MongoClientAuthProviders } from './mongo_client_auth_providers'; import { type LogComponentSeveritiesClientOptions, type MongoDBLogWritable, @@ -305,7 +298,7 @@ export interface MongoClientPrivate { bsonOptions: BSONSerializeOptions; namespace: MongoDBNamespace; hasBeenClosed: boolean; - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; + authProviders: MongoClientAuthProviders; /** * We keep a reference to the sessions that are acquired from the pool. * - used to track and close all sessions in client.close() (which is non-standard behavior) @@ -331,18 +324,6 @@ export type MongoClientEvents = Pick AuthProvider>([ - [AuthMechanism.MONGODB_AWS, () => new MongoDBAWS()], - [AuthMechanism.MONGODB_CR, () => new MongoCR()], - [AuthMechanism.MONGODB_GSSAPI, () => new GSSAPI()], - [AuthMechanism.MONGODB_OIDC, () => new MongoDBOIDC()], - [AuthMechanism.MONGODB_PLAIN, () => new Plain()], - [AuthMechanism.MONGODB_SCRAM_SHA1, () => new ScramSHA1()], - [AuthMechanism.MONGODB_SCRAM_SHA256, () => new ScramSHA256()], - [AuthMechanism.MONGODB_X509, () => new X509()] -]); - /** * The **MongoClient** class is a class that allows for making Connections to MongoDB. * @public @@ -393,8 +374,6 @@ export class MongoClient extends TypedEventEmitter { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; - const clientAuthProviders: { [authMechanism: AuthMechanism | string]: AuthProvider } = {}; - // The internal state this.s = { url, @@ -403,16 +382,8 @@ export class MongoClient extends TypedEventEmitter { hasBeenClosed: false, sessionPool: new ServerSessionPool(this), activeSessions: new Set(), + authProviders: new MongoClientAuthProviders(), - getAuthProvider(name: AuthMechanism | string) { - if (!clientAuthProviders[name]) { - const provider = AUTH_PROVIDERS.get(name)?.(); - if (provider) { - clientAuthProviders[name] = provider; - } - } - return clientAuthProviders[name]; - }, get options() { return client[kOptions]; }, @@ -862,13 +833,10 @@ export interface MongoOptions proxyUsername?: string; proxyPassword?: string; serverMonitoringMode: ServerMonitoringMode; - /** @internal */ connectionType?: typeof Connection; - /** @internal */ - getAuthProvider: (name: AuthMechanism | string) => AuthProvider | undefined; - + authProviders: MongoClientAuthProviders; /** @internal */ encrypter: Encrypter; /** @internal */ diff --git a/src/mongo_client_auth_providers.ts b/src/mongo_client_auth_providers.ts new file mode 100644 index 0000000000..12f3507a9a --- /dev/null +++ b/src/mongo_client_auth_providers.ts @@ -0,0 +1,44 @@ +import { type AuthProvider } from './cmap/auth/auth_provider'; +import { GSSAPI } from './cmap/auth/gssapi'; +import { MongoCR } from './cmap/auth/mongocr'; +import { MongoDBAWS } from './cmap/auth/mongodb_aws'; +import { MongoDBOIDC } from './cmap/auth/mongodb_oidc'; +import { Plain } from './cmap/auth/plain'; +import { AuthMechanism } from './cmap/auth/providers'; +import { ScramSHA1, ScramSHA256 } from './cmap/auth/scram'; +import { X509 } from './cmap/auth/x509'; +import { MongoInvalidArgumentError } from './error'; + +/** @internal */ +const AUTH_PROVIDERS = new Map AuthProvider>([ + [AuthMechanism.MONGODB_AWS, () => new MongoDBAWS()], + [AuthMechanism.MONGODB_CR, () => new MongoCR()], + [AuthMechanism.MONGODB_GSSAPI, () => new GSSAPI()], + [AuthMechanism.MONGODB_OIDC, () => new MongoDBOIDC()], + [AuthMechanism.MONGODB_PLAIN, () => new Plain()], + [AuthMechanism.MONGODB_SCRAM_SHA1, () => new ScramSHA1()], + [AuthMechanism.MONGODB_SCRAM_SHA256, () => new ScramSHA256()], + [AuthMechanism.MONGODB_X509, () => new X509()] +]); + +/** + * @internal + */ +export class MongoClientAuthProviders { + private existingProviders: Map = new Map(); + + getOrCreateProvider(name: AuthMechanism | string): AuthProvider { + const authProvider = this.existingProviders.get(name); + if (authProvider) { + return authProvider; + } + + const provider = AUTH_PROVIDERS.get(name)?.(); + if (!provider) { + throw new MongoInvalidArgumentError(`authMechanism ${name} not supported`); + } + + this.existingProviders.set(name, provider); + return provider; + } +} diff --git a/test/integration/auth/mongodb_aws.test.ts b/test/integration/auth/mongodb_aws.test.ts index 5218818a7c..635880c04a 100644 --- a/test/integration/auth/mongodb_aws.test.ts +++ b/test/integration/auth/mongodb_aws.test.ts @@ -67,13 +67,17 @@ describe('MONGODB-AWS', function () { .that.equals(''); }); - it('should store a MongoDBAWS provider instance per client', function () { - client = this.configuration.newClient(this.configuration.url(), { - authMechanismProperties: { AWS_SESSION_TOKEN: '' } - }); + it('should store a MongoDBAWS provider instance per client', async function () { + client = this.configuration.newClient(process.env.MONGODB_URI); + + await client + .db('aws') + .collection('aws_test') + .estimatedDocumentCount() + .catch(error => error); - expect(client).to.have.nested.property('s.getAuthProvider'); - const provider = client.s.getAuthProvider('MONGODB-AWS'); + expect(client).to.have.nested.property('s.authProviders'); + const provider = client.s.authProviders.getOrCreateProvider('MONGODB-AWS'); expect(provider).to.be.instanceOf(MongoDBAWS); }); @@ -198,7 +202,7 @@ describe('MONGODB-AWS', function () { let storedEnv; let calledArguments; let shouldSkip = false; - let n; + let numberOfFromNodeProviderChainCalls; const envCheck = () => { const { AWS_WEB_IDENTITY_TOKEN_FILE = '' } = process.env; @@ -225,12 +229,12 @@ describe('MONGODB-AWS', function () { process.env.AWS_REGION = test.env.AWS_REGION; } - n = 0; + numberOfFromNodeProviderChainCalls = 0; MongoDBAWS.credentialProvider = { fromNodeProviderChain(...args) { calledArguments = args; - n += 1; + numberOfFromNodeProviderChainCalls += 1; return credentialProvider.fromNodeProviderChain(...args); } }; @@ -261,12 +265,11 @@ describe('MONGODB-AWS', function () { expect(result).to.not.be.instanceOf(MongoServerError); expect(result).to.be.a('number'); - expect(n).to.be.eql(1); expect(calledArguments).to.deep.equal(test.calledWith); }); - it('creates one AWS provider per client', async function () { + it('fromNodeProviderChain called once', async function () { await client.close(); await client.connect(); await client @@ -275,7 +278,7 @@ describe('MONGODB-AWS', function () { .estimatedDocumentCount() .catch(error => error); - expect(n).to.be.eql(1); + expect(numberOfFromNodeProviderChainCalls).to.be.eql(1); }); }); } diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index e702795b40..a1e8f1f957 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -8,6 +8,7 @@ import { LEGACY_HELLO_COMMAND, makeClientMetadata, MongoClient, + MongoClientAuthProviders, MongoServerError, ns, ServerHeartbeatStartedEvent, @@ -23,7 +24,8 @@ const commonConnectOptions = { tls: false, loadBalanced: false, // Will be overridden by configuration options - hostAddress: HostAddress.fromString('127.0.0.1:1') + hostAddress: HostAddress.fromString('127.0.0.1:1'), + authProviders: new MongoClientAuthProviders() }; describe('Connection', function () { diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 62f2d3c0b4..7697c124fb 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -9,9 +9,9 @@ import { HostAddress, isHello, LEGACY_HELLO_COMMAND, + MongoClientAuthProviders, MongoCredentials, MongoNetworkError, - Plain, prepareHandshakeDocument } from '../../mongodb'; import { genClusterTime } from '../../tools/common'; @@ -23,8 +23,7 @@ const CONNECT_DEFAULTS = { generation: 1, monitorCommands: false, metadata: {} as ClientMetadata, - loadBalanced: false, - getAuthProvider: () => new Plain() + loadBalanced: false }; describe('Connect Tests', function () { @@ -46,7 +45,8 @@ describe('Connect Tests', function () { source: 'admin', mechanism: 'PLAIN', mechanismProperties: {} - }) + }), + authProviders: new MongoClientAuthProviders() }; }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index ec902fffd3..9127d68c99 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1,7 +1,14 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { connect, Connection, isHello, MongoNetworkTimeoutError, ns } from '../../mongodb'; +import { + connect, + Connection, + isHello, + MongoClientAuthProviders, + MongoNetworkTimeoutError, + ns +} from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { getSymbolFrom } from '../../tools/utils'; @@ -32,7 +39,8 @@ describe('new Connection()', function () { const options = { ...connectionOptionsDefaults, connectionType: Connection, - hostAddress: server.hostAddress() + hostAddress: server.hostAddress(), + authProviders: new MongoClientAuthProviders() }; const conn = await connect(options); @@ -54,7 +62,8 @@ describe('new Connection()', function () { const options = { ...connectionOptionsDefaults, connectionType: Connection, - hostAddress: server.hostAddress() + hostAddress: server.hostAddress(), + authProviders: new MongoClientAuthProviders() }; const conn = await connect(options); @@ -76,7 +85,8 @@ describe('new Connection()', function () { const options = { hostAddress: server.hostAddress(), - ...connectionOptionsDefaults + ...connectionOptionsDefaults, + authProviders: new MongoClientAuthProviders() }; const conn = await connect(options); @@ -101,7 +111,8 @@ describe('new Connection()', function () { const options = { ...connectionOptionsDefaults, - hostAddress: server.hostAddress() + hostAddress: server.hostAddress(), + authProviders: new MongoClientAuthProviders() }; const connection = await connect(options); @@ -119,7 +130,8 @@ describe('new Connection()', function () { const options = { ...connectionOptionsDefaults, hostAddress: server.hostAddress(), - socketTimeoutMS: 50 + socketTimeoutMS: 50, + authProviders: new MongoClientAuthProviders() }; const error = await connect(options).catch(error => error); diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index cdbf00bf67..43177b7296 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -11,6 +11,7 @@ const { ns, isHello } = require('../../mongodb'); const { LEGACY_HELLO_COMMAND } = require('../../mongodb'); const { createTimerSandbox } = require('../timer_sandbox'); const { topologyWithPlaceholderClient } = require('../../tools/utils'); +const { MongoClientAuthProviders } = require('../../mongodb'); describe('Connection Pool', function () { let mockMongod; @@ -20,6 +21,9 @@ describe('Connection Pool', function () { mongoLogger: { debug: () => null, willLog: () => null + }, + s: { + authProviders: new MongoClientAuthProviders() } } } diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 6e36a54fa7..508f3d85c2 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -71,6 +71,7 @@ const EXPECTED_EXPORTS = [ 'MongoBulkWriteError', 'MongoChangeStreamError', 'MongoClient', + 'MongoClientAuthProviders', 'MongoCompatibilityError', 'MongoCryptAzureKMSRequestError', 'MongoCryptCreateDataKeyError', diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index c9c3c9923f..04ef86a503 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -11,14 +11,20 @@ const { ReadConcern } = require('../mongodb'); const { WriteConcern } = require('../mongodb'); const { ReadPreference } = require('../mongodb'); const { MongoCredentials } = require('../mongodb'); -const { MongoClient, MongoParseError, ServerApiVersion, MongoAPIError } = require('../mongodb'); +const { + MongoClient, + MongoParseError, + ServerApiVersion, + MongoAPIError, + MongoInvalidArgumentError +} = require('../mongodb'); const { MongoLogger } = require('../mongodb'); // eslint-disable-next-line no-restricted-modules const { SeverityLevel, MongoLoggableComponent } = require('../../src/mongo_logger'); const sinon = require('sinon'); const { Writable } = require('stream'); -describe('MongoOptions', function () { +describe('MongoClient', function () { it('MongoClient should always freeze public options', function () { const client = new MongoClient('mongodb://localhost:27017'); expect(client.options).to.be.frozen; @@ -1182,4 +1188,15 @@ describe('MongoOptions', function () { }); }); }); + + context('getAuthProvider', function () { + it('throws MongoInvalidArgumentError if provided authMechanism is not supported', function () { + const client = new MongoClient('mongodb://localhost:27017'); + try { + client.s.authProviders.getOrCreateProvider('NOT_SUPPORTED'); + } catch (error) { + expect(error).to.be.an.instanceof(MongoInvalidArgumentError); + } + }); + }); }); From 2cd3d60229a5271cacdc31632ad349be2563a698 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Mon, 19 Feb 2024 19:47:36 +0100 Subject: [PATCH 15/15] docs: add comments --- src/mongo_client_auth_providers.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/mongo_client_auth_providers.ts b/src/mongo_client_auth_providers.ts index 12f3507a9a..557783c4e1 100644 --- a/src/mongo_client_auth_providers.ts +++ b/src/mongo_client_auth_providers.ts @@ -22,11 +22,21 @@ const AUTH_PROVIDERS = new Map AuthProvider>([ ]); /** + * Create a set of providers per client + * to avoid sharing the provider's cache between different clients. * @internal */ export class MongoClientAuthProviders { private existingProviders: Map = new Map(); + /** + * Get or create an authentication provider based on the provided mechanism. + * We don't want to create all providers at once, as some providers may not be used. + * @param name - The name of the provider to get or create. + * @returns The provider. + * @throws MongoInvalidArgumentError if the mechanism is not supported. + * @internal + */ getOrCreateProvider(name: AuthMechanism | string): AuthProvider { const authProvider = this.existingProviders.get(name); if (authProvider) {