From c752668c2b708ea46511f26c28f7eaa95d88ac11 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 27 Feb 2023 19:34:34 +0100 Subject: [PATCH 01/17] feat(NODE-5036): reauthenticate OIDC and retry --- src/cmap/auth/auth_provider.ts | 8 +- src/cmap/auth/mongodb_oidc.ts | 4 +- .../auth/mongodb_oidc/callback_workflow.ts | 10 +- src/cmap/auth/mongodb_oidc/workflow.ts | 6 +- src/cmap/auth/scram.ts | 4 +- src/cmap/connect.ts | 4 +- src/cmap/connection.ts | 4 +- src/cmap/connection_pool.ts | 72 ++++++- src/error.ts | 3 +- src/index.ts | 1 + test/integration/auth/auth.spec.test.ts | 8 + test/manual/mongodb_oidc.prose.test.ts | 148 ++++++++++++++ .../unified/reauthenticate_with_retry.json | 191 ++++++++++++++++++ .../unified/reauthenticate_with_retry.yml | 104 ++++++++++ .../unified/reauthenticate_without_retry.json | 191 ++++++++++++++++++ .../unified/reauthenticate_without_retry.yml | 104 ++++++++++ test/unit/index.test.ts | 2 + 17 files changed, 849 insertions(+), 15 deletions(-) create mode 100644 test/integration/auth/auth.spec.test.ts create mode 100644 test/spec/auth/unified/reauthenticate_with_retry.json create mode 100644 test/spec/auth/unified/reauthenticate_with_retry.yml create mode 100644 test/spec/auth/unified/reauthenticate_without_retry.json create mode 100644 test/spec/auth/unified/reauthenticate_without_retry.yml diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 2a38abe9b4..26f90dd779 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -5,14 +5,20 @@ import type { HandshakeDocument } from '../connect'; import type { Connection, ConnectionOptions } from '../connection'; import type { MongoCredentials } from './mongo_credentials'; +/** @internal */ export type AuthContextOptions = ConnectionOptions & ClientMetadataOptions; -/** Context used during authentication */ +/** + * Context used during authentication + * @internal + */ export class AuthContext { /** The connection to authenticate */ connection: Connection; /** The credentials to use for authentication */ credentials?: MongoCredentials; + /** If the context if for reauthentication. */ + reauthenticating = false; /** The options passed to the `connect` method */ options: AuthContextOptions; diff --git a/src/cmap/auth/mongodb_oidc.ts b/src/cmap/auth/mongodb_oidc.ts index be06df6d9e..8f5f4af5b7 100644 --- a/src/cmap/auth/mongodb_oidc.ts +++ b/src/cmap/auth/mongodb_oidc.ts @@ -65,7 +65,7 @@ export class MongoDBOIDC extends AuthProvider { * Authenticate using OIDC */ override auth(authContext: AuthContext, callback: Callback): void { - const { connection, credentials, response } = authContext; + const { connection, credentials, response, reauthenticating } = authContext; if (response?.speculativeAuthenticate) { return callback(); @@ -86,7 +86,7 @@ export class MongoDBOIDC extends AuthProvider { ) ); } - workflow.execute(connection, credentials).then( + workflow.execute(connection, credentials, reauthenticating).then( result => { return callback(undefined, result); }, diff --git a/src/cmap/auth/mongodb_oidc/callback_workflow.ts b/src/cmap/auth/mongodb_oidc/callback_workflow.ts index 59da9eb250..ffebe7e49b 100644 --- a/src/cmap/auth/mongodb_oidc/callback_workflow.ts +++ b/src/cmap/auth/mongodb_oidc/callback_workflow.ts @@ -58,7 +58,11 @@ export class CallbackWorkflow implements Workflow { * - put the new entry in the cache. * - execute step two. */ - async execute(connection: Connection, credentials: MongoCredentials): Promise { + async execute( + connection: Connection, + credentials: MongoCredentials, + reauthenticate = false + ): Promise { const request = credentials.mechanismProperties.REQUEST_TOKEN_CALLBACK; const refresh = credentials.mechanismProperties.REFRESH_TOKEN_CALLBACK; @@ -69,8 +73,8 @@ export class CallbackWorkflow implements Workflow { refresh || null ); if (entry) { - // Check if the entry is not expired. - if (entry.isValid()) { + // Check if the entry is not expired and if we are reauthenticating. + if (!reauthenticate && entry.isValid()) { // Skip step one and execute the step two saslContinue. try { const result = await finishAuth(entry.tokenResult, undefined, connection, credentials); diff --git a/src/cmap/auth/mongodb_oidc/workflow.ts b/src/cmap/auth/mongodb_oidc/workflow.ts index 8b88ee00a8..68d0b97688 100644 --- a/src/cmap/auth/mongodb_oidc/workflow.ts +++ b/src/cmap/auth/mongodb_oidc/workflow.ts @@ -8,7 +8,11 @@ export interface Workflow { * All device workflows must implement this method in order to get the access * token and then call authenticate with it. */ - execute(connection: Connection, credentials: MongoCredentials): Promise; + execute( + connection: Connection, + credentials: MongoCredentials, + reauthenticate?: boolean + ): Promise; /** * Get the document to add for speculative authentication. diff --git a/src/cmap/auth/scram.ts b/src/cmap/auth/scram.ts index 7a339151fa..dbe4ce25a3 100644 --- a/src/cmap/auth/scram.ts +++ b/src/cmap/auth/scram.ts @@ -53,8 +53,8 @@ class ScramSHA extends AuthProvider { } override auth(authContext: AuthContext, callback: Callback) { - const response = authContext.response; - if (response && response.speculativeAuthenticate) { + const { reauthenticating, response } = authContext; + if (response?.speculativeAuthenticate && !reauthenticating) { continueScramConversation( this.cryptoMethod, response.speculativeAuthenticate, diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 278864cda2..e7e7a87c89 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -36,7 +36,8 @@ import { MIN_SUPPORTED_WIRE_VERSION } from './wire_protocol/constants'; -const AUTH_PROVIDERS = new Map([ +/** @internal */ +export const AUTH_PROVIDERS = new Map([ [AuthMechanism.MONGODB_AWS, new MongoDBAWS()], [AuthMechanism.MONGODB_CR, new MongoCR()], [AuthMechanism.MONGODB_GSSAPI, new GSSAPI()], @@ -117,6 +118,7 @@ function performInitialHandshake( } const authContext = new AuthContext(conn, credentials, options); + conn.authContext = authContext; prepareHandshakeDocument(authContext, (err, handshakeDoc) => { if (err || !handshakeDoc) { return callback(err); diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 8557f9275d..464f86d9b3 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -37,6 +37,7 @@ import { uuidV4 } from '../utils'; import type { WriteConcern } from '../write_concern'; +import type { AuthContext } from './auth/auth_provider'; import type { MongoCredentials } from './auth/mongo_credentials'; import { CommandFailedEvent, @@ -126,7 +127,6 @@ export interface ConnectionOptions noDelay?: boolean; socketTimeoutMS?: number; cancellationToken?: CancellationToken; - metadata: ClientMetadata; } @@ -164,6 +164,8 @@ export class Connection extends TypedEventEmitter { cmd: Document, options: CommandOptions | undefined ) => Promise; + /** @internal */ + authContext?: AuthContext; /**@internal */ [kDelayedTimeoutId]: NodeJS.Timeout | null; diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 3991f37868..12c7d8bc07 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -16,8 +16,10 @@ import { CONNECTION_READY } from '../constants'; import { + MONGODB_ERROR_CODES, MongoError, MongoInvalidArgumentError, + MongoMissingCredentialsError, MongoNetworkError, MongoRuntimeError, MongoServerError @@ -25,7 +27,7 @@ import { import { CancellationToken, TypedEventEmitter } from '../mongo_types'; import type { Server } from '../sdam/server'; import { Callback, eachAsync, List, makeCounter } from '../utils'; -import { connect } from './connect'; +import { AUTH_PROVIDERS, connect } from './connect'; import { Connection, ConnectionEvents, ConnectionOptions } from './connection'; import { ConnectionCheckedInEvent, @@ -544,7 +546,17 @@ export class ConnectionPool extends TypedEventEmitter { fn(undefined, conn, (fnErr, result) => { if (typeof callback === 'function') { if (fnErr) { - callback(fnErr); + if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + this.reauthenticate(conn, fn, (error, res) => { + if (error) { + callback(error); + } else { + callback(undefined, res); + } + }); + } else { + callback(fnErr); + } } else { callback(undefined, result); } @@ -559,7 +571,17 @@ export class ConnectionPool extends TypedEventEmitter { fn(err as MongoError, conn, (fnErr, result) => { if (typeof callback === 'function') { if (fnErr) { - callback(fnErr); + if (conn && (fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + this.reauthenticate(conn, fn, (error, res) => { + if (error) { + callback(error); + } else { + callback(undefined, res); + } + }); + } else { + callback(fnErr); + } } else { callback(undefined, result); } @@ -572,6 +594,50 @@ export class ConnectionPool extends TypedEventEmitter { }); } + /** + * Reauthenticate on the same connection and then retry the operation. + */ + private reauthenticate( + connection: Connection, + fn: WithConnectionCallback, + callback: Callback + ): void { + const authContext = connection.authContext; + if (!authContext) { + return callback(new MongoRuntimeError('No auth context found on connection.')); + } + authContext.reauthenticating = true; + const credentials = authContext.credentials; + if (!credentials) { + return callback( + new MongoMissingCredentialsError( + 'Connection is missing credentials when asked to reauthenticate' + ) + ); + } + const resolvedCredentials = credentials.resolveAuthMechanism(connection.hello || undefined); + const provider = AUTH_PROVIDERS.get(resolvedCredentials.mechanism); + if (!provider) { + return callback( + new MongoMissingCredentialsError( + `Reauthenticate failed due to no auth provider for ${credentials.mechanism}` + ) + ); + } + provider.auth(authContext, error => { + authContext.reauthenticating = false; + if (error) { + return callback(error); + } + return fn(undefined, connection, (fnErr, fnResult) => { + if (fnErr) { + return callback(fnErr); + } + callback(undefined, fnResult); + }); + }); + } + /** Clear the min pool size timer */ private clearMinPoolSizeTimer(): void { const minPoolSizeTimer = this[kMinPoolSizeTimer]; diff --git a/src/error.ts b/src/error.ts index 0e9590423f..f40dfd5229 100644 --- a/src/error.ts +++ b/src/error.ts @@ -58,7 +58,8 @@ export const MONGODB_ERROR_CODES = Object.freeze({ IllegalOperation: 20, MaxTimeMSExpired: 50, UnknownReplWriteConcern: 79, - UnsatisfiableWriteConcern: 100 + UnsatisfiableWriteConcern: 100, + Reauthenticate: 391 } as const); // From spec@https://github.com/mongodb/specifications/blob/f93d78191f3db2898a59013a7ed5650352ef6da8/source/change-streams/change-streams.rst#resumable-error diff --git a/src/index.ts b/src/index.ts index 22ab7f4b99..1bd5e812c5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -197,6 +197,7 @@ export type { ResumeToken, UpdateDescription } from './change_stream'; +export { AuthContext, AuthContextOptions } from './cmap/auth/auth_provider'; export type { AuthMechanismProperties, MongoCredentials, diff --git a/test/integration/auth/auth.spec.test.ts b/test/integration/auth/auth.spec.test.ts new file mode 100644 index 0000000000..cfce338e8d --- /dev/null +++ b/test/integration/auth/auth.spec.test.ts @@ -0,0 +1,8 @@ +import * as path from 'path'; + +import { loadSpecTests } from '../../spec'; +import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner'; + +describe('Auth (unified)', function () { + runUnifiedSuite(loadSpecTests(path.join('auth', 'unified'))); +}); diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index f89eec1001..b5665d7b51 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -452,5 +452,153 @@ describe('MONGODB-OIDC', function () { }); }); }); + + // The driver MUST test reauthentication with MONGODB-OIDC for a read operation. + describe('6. Reauthentication', function () { + let refreshInvokations = 0; + let findStarted = 0; + let findSucceeded = 0; + let findFailed = 0; + let saslStarted = 0; + let saslSucceeded = 0; + let client; + let collection; + const cache = OIDC_WORKFLOWS.get('callback').cache; + + // - Create request and refresh callbacks that return valid credentials that + // will not expire soon. + const requestCallback = async () => { + const token = await readFile(`${process.env.OIDC_TOKEN_DIR}/test_user1`, { + encoding: 'utf8' + }); + return { accessToken: token, expiresInSeconds: 300 }; + }; + + const refreshCallback = async () => { + const token = await readFile(`${process.env.OIDC_TOKEN_DIR}/test_user1`, { + encoding: 'utf8' + }); + refreshInvokations++; + return { accessToken: token, expiresInSeconds: 300 }; + }; + + const commandStarted = event => { + if (event.commandName === 'find') { + findStarted++; + } + if (event.commandName === 'saslStart') { + saslStarted++; + } + }; + + const commandSucceeded = event => { + if (event.commandName === 'find') { + findSucceeded++; + } + if (event.commandName === 'saslStart') { + saslSucceeded++; + } + }; + + const commandFailed = event => { + if (event.commandName === 'find') { + findFailed++; + } + }; + + before(function () { + // - Clear the cache + cache.clear(); + // - Create a client with the callbacks and an event listener capable of + // listening for SASL commands + // + // - TODO(NODE-3494): Driver does not observe sensitive commands. + client = new MongoClient('mongodb://test_user1@localhost/?authMechanism=MONGODB-OIDC', { + authMechanismProperties: { + REQUEST_TOKEN_CALLBACK: requestCallback, + REFRESH_TOKEN_CALLBACK: refreshCallback + }, + monitorCommands: true + }); + client.on('commandStarted', commandStarted); + client.on('commandSucceeded', commandSucceeded); + client.on('commandFailed', commandFailed); + collection = client.db('test').collection('test'); + }); + + after(async function () { + await client?.close(); + }); + + context('on the first find invokation', function () { + before(function () { + findStarted = 0; + findSucceeded = 0; + findFailed = 0; + refreshInvokations = 0; + saslStarted = 0; + saslSucceeded = 0; + }); + + // - Perform a find operation. + // - Assert that the refresh callback has not been called. + it('does not call the refresh callback', async function () { + await collection.findOne(); + expect(refreshInvokations).to.equal(0); + }); + }); + + context('when a command errors and needs reauthentication', function () { + // Force a reauthenication using a failCommand of the form: + before(async function () { + findStarted = 0; + findSucceeded = 0; + findFailed = 0; + refreshInvokations = 0; + saslStarted = 0; + saslSucceeded = 0; + await client.db('admin').command({ + configureFailPoint: 'failCommand', + mode: { times: 1 }, + data: { + failCommands: ['find'], + errorCode: 391 + } + }); + // Perform another find operation. + await collection.findOne(); + }); + + // - Assert that the refresh callback has been called, if possible. + it('calls the refresh callback', function () { + expect(refreshInvokations).to.equal(1); + }); + + // - Assert that a find operation was started twice and a saslStart operation + // was started once during the command execution. + it('starts the find operation twice', function () { + expect(findStarted).to.equal(2); + }); + + it('starts saslStart once', function () { + expect(saslStarted).to.equal(1); + }); + + // - Assert that a find operation succeeeded once and the saslStart operation + // succeeded during the command execution. + it('succeeds on the find once', function () { + expect(findSucceeded).to.equal(1); + }); + + it('succeeds on saslStart once', function () { + expect(saslSucceeded).to.equal(1); + }); + + // Assert that a find operation failed once during the command execution. + it('fails on the find once', function () { + expect(findFailed).to.equal(1); + }); + }); + }); }); }); diff --git a/test/spec/auth/unified/reauthenticate_with_retry.json b/test/spec/auth/unified/reauthenticate_with_retry.json new file mode 100644 index 0000000000..ef110562ed --- /dev/null +++ b/test/spec/auth/unified/reauthenticate_with_retry.json @@ -0,0 +1,191 @@ +{ + "description": "reauthenticate_with_retry", + "schemaVersion": "1.12", + "runOnRequirements": [ + { + "minServerVersion": "6.3", + "auth": true + } + ], + "createEntities": [ + { + "client": { + "id": "client0", + "uriOptions": { + "retryReads": true, + "retryWrites": true + }, + "observeEvents": [ + "commandStartedEvent", + "commandSucceededEvent", + "commandFailedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "db" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "collName" + } + } + ], + "initialData": [ + { + "collectionName": "collName", + "databaseName": "db", + "documents": [] + } + ], + "tests": [ + { + "description": "Read command should reauthenticate when receive ReauthenticationRequired error code and retryReads=true", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "find" + ], + "errorCode": 391 + } + } + } + }, + { + "name": "find", + "arguments": { + "filter": {} + }, + "object": "collection0", + "expectResult": [] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } + } + }, + { + "commandFailedEvent": { + "commandName": "find" + } + }, + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } + } + }, + { + "commandSucceededEvent": { + "commandName": "find" + } + } + ] + } + ] + }, + { + "description": "Write command should reauthenticate when receive ReauthenticationRequired error code and retryWrites=true", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 391 + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] + } + } + }, + { + "commandFailedEvent": { + "commandName": "insert" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] + } + } + }, + { + "commandSucceededEvent": { + "commandName": "insert" + } + } + ] + } + ] + } + ] +} diff --git a/test/spec/auth/unified/reauthenticate_with_retry.yml b/test/spec/auth/unified/reauthenticate_with_retry.yml new file mode 100644 index 0000000000..bf7cb56f3c --- /dev/null +++ b/test/spec/auth/unified/reauthenticate_with_retry.yml @@ -0,0 +1,104 @@ +--- +description: reauthenticate_with_retry +schemaVersion: '1.12' +runOnRequirements: +- minServerVersion: '6.3' + auth: true +createEntities: +- client: + id: client0 + uriOptions: + retryReads: true + retryWrites: true + observeEvents: + - commandStartedEvent + - commandSucceededEvent + - commandFailedEvent +- database: + id: database0 + client: client0 + databaseName: db +- collection: + id: collection0 + database: database0 + collectionName: collName +initialData: +- collectionName: collName + databaseName: db + documents: [] +tests: +- description: Read command should reauthenticate when receive ReauthenticationRequired + error code and retryReads=true + operations: + - name: failPoint + object: testRunner + arguments: + client: client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 1 + data: + failCommands: + - find + errorCode: 391 + - name: find + arguments: + filter: {} + object: collection0 + expectResult: [] + expectEvents: + - client: client0 + events: + - commandStartedEvent: + command: + find: collName + filter: {} + - commandFailedEvent: + commandName: find + - commandStartedEvent: + command: + find: collName + filter: {} + - commandSucceededEvent: + commandName: find +- description: Write command should reauthenticate when receive ReauthenticationRequired + error code and retryWrites=true + operations: + - name: failPoint + object: testRunner + arguments: + client: client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 1 + data: + failCommands: + - insert + errorCode: 391 + - name: insertOne + object: collection0 + arguments: + document: + _id: 1 + x: 1 + expectEvents: + - client: client0 + events: + - commandStartedEvent: + command: + insert: collName + documents: + - _id: 1 + x: 1 + - commandFailedEvent: + commandName: insert + - commandStartedEvent: + command: + insert: collName + documents: + - _id: 1 + x: 1 + - commandSucceededEvent: + commandName: insert diff --git a/test/spec/auth/unified/reauthenticate_without_retry.json b/test/spec/auth/unified/reauthenticate_without_retry.json new file mode 100644 index 0000000000..6fded47634 --- /dev/null +++ b/test/spec/auth/unified/reauthenticate_without_retry.json @@ -0,0 +1,191 @@ +{ + "description": "reauthenticate_without_retry", + "schemaVersion": "1.12", + "runOnRequirements": [ + { + "minServerVersion": "6.3", + "auth": true + } + ], + "createEntities": [ + { + "client": { + "id": "client0", + "uriOptions": { + "retryReads": false, + "retryWrites": false + }, + "observeEvents": [ + "commandStartedEvent", + "commandSucceededEvent", + "commandFailedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "db" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "collName" + } + } + ], + "initialData": [ + { + "collectionName": "collName", + "databaseName": "db", + "documents": [] + } + ], + "tests": [ + { + "description": "Read command should reauthenticate when receive ReauthenticationRequired error code and retryReads=false", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "find" + ], + "errorCode": 391 + } + } + } + }, + { + "name": "find", + "arguments": { + "filter": {} + }, + "object": "collection0", + "expectResult": [] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } + } + }, + { + "commandFailedEvent": { + "commandName": "find" + } + }, + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } + } + }, + { + "commandSucceededEvent": { + "commandName": "find" + } + } + ] + } + ] + }, + { + "description": "Write command should reauthenticate when receive ReauthenticationRequired error code and retryWrites=false", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 391 + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] + } + } + }, + { + "commandFailedEvent": { + "commandName": "insert" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] + } + } + }, + { + "commandSucceededEvent": { + "commandName": "insert" + } + } + ] + } + ] + } + ] +} diff --git a/test/spec/auth/unified/reauthenticate_without_retry.yml b/test/spec/auth/unified/reauthenticate_without_retry.yml new file mode 100644 index 0000000000..394c4be91e --- /dev/null +++ b/test/spec/auth/unified/reauthenticate_without_retry.yml @@ -0,0 +1,104 @@ +--- +description: reauthenticate_without_retry +schemaVersion: '1.13' +runOnRequirements: +- minServerVersion: '6.3' + auth: true +createEntities: +- client: + id: client0 + uriOptions: + retryReads: false + retryWrites: false + observeEvents: + - commandStartedEvent + - commandSucceededEvent + - commandFailedEvent +- database: + id: database0 + client: client0 + databaseName: db +- collection: + id: collection0 + database: database0 + collectionName: collName +initialData: +- collectionName: collName + databaseName: db + documents: [] +tests: +- description: Read command should reauthenticate when receive ReauthenticationRequired + error code and retryReads=false + operations: + - name: failPoint + object: testRunner + arguments: + client: client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 1 + data: + failCommands: + - find + errorCode: 391 + - name: find + arguments: + filter: {} + object: collection0 + expectResult: [] + expectEvents: + - client: client0 + events: + - commandStartedEvent: + command: + find: collName + filter: {} + - commandFailedEvent: + commandName: find + - commandStartedEvent: + command: + find: collName + filter: {} + - commandSucceededEvent: + commandName: find +- description: Write command should reauthenticate when receive ReauthenticationRequired + error code and retryWrites=false + operations: + - name: failPoint + object: testRunner + arguments: + client: client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 1 + data: + failCommands: + - insert + errorCode: 391 + - name: insertOne + object: collection0 + arguments: + document: + _id: 1 + x: 1 + expectEvents: + - client: client0 + events: + - commandStartedEvent: + command: + insert: collName + documents: + - _id: 1 + x: 1 + - commandFailedEvent: + commandName: insert + - commandStartedEvent: + command: + insert: collName + documents: + - _id: 1 + x: 1 + - commandSucceededEvent: + commandName: insert diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 833082b983..77d40a4d52 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -16,6 +16,8 @@ const EXPECTED_EXPORTS = [ 'AbstractCursor', 'Admin', 'AggregationCursor', + 'AuthContext', + 'AuthContextOptions', 'AuthMechanism', 'AutoEncryptionLoggerLevel', 'BatchType', From bb9d6cfbe8a1f5c237973a453aff3304419c6e0d Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 15:28:49 +0100 Subject: [PATCH 02/17] fix: make callback required --- src/cmap/connection_pool.ts | 56 +++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 12c7d8bc07..afb5675ce9 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -539,52 +539,46 @@ export class ConnectionPool extends TypedEventEmitter { withConnection( conn: Connection | undefined, fn: WithConnectionCallback, - callback?: Callback + callback: Callback ): void { if (conn) { // use the provided connection, and do _not_ check it in after execution fn(undefined, conn, (fnErr, result) => { - if (typeof callback === 'function') { - if (fnErr) { - if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { - this.reauthenticate(conn, fn, (error, res) => { - if (error) { - callback(error); - } else { - callback(undefined, res); - } - }); - } else { - callback(fnErr); - } + if (fnErr) { + if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + this.reauthenticate(conn, fn, (error, res) => { + if (error) { + callback(error); + } else { + callback(undefined, res); + } + }); } else { - callback(undefined, result); + callback(fnErr); } + } else { + callback(undefined, result); } }); - - return; } this.checkOut((err, conn) => { // don't callback with `err` here, we might want to act upon it inside `fn` fn(err as MongoError, conn, (fnErr, result) => { - if (typeof callback === 'function') { - if (fnErr) { - if (conn && (fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { - this.reauthenticate(conn, fn, (error, res) => { - if (error) { - callback(error); - } else { - callback(undefined, res); - } - }); - } else { - callback(fnErr); - } + if (fnErr) { + if (conn && (fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + this.reauthenticate(conn, fn, (error, res) => { + if (error) { + callback(error); + } else { + callback(undefined, res); + } + }); } else { - callback(undefined, result); + callback(fnErr); } + } else { + callback(undefined, result); } if (conn) { From 489ab79a357e45ea3d3b85a1ffbe5af1d8791e5f Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 15:30:06 +0100 Subject: [PATCH 03/17] fix: remove todo --- test/manual/mongodb_oidc.prose.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index b5665d7b51..792fed37d7 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -511,8 +511,6 @@ describe('MONGODB-OIDC', function () { cache.clear(); // - Create a client with the callbacks and an event listener capable of // listening for SASL commands - // - // - TODO(NODE-3494): Driver does not observe sensitive commands. client = new MongoClient('mongodb://test_user1@localhost/?authMechanism=MONGODB-OIDC', { authMechanismProperties: { REQUEST_TOKEN_CALLBACK: requestCallback, From f65d11c5456fb7d7b71f984e2bf97c79e60f64ed Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 15:52:45 +0100 Subject: [PATCH 04/17] refactor: pull out withReauthentication --- src/cmap/connection_pool.ts | 45 ++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index afb5675ce9..f7ac8a70e7 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -16,6 +16,7 @@ import { CONNECTION_READY } from '../constants'; import { + AnyError, MONGODB_ERROR_CODES, MongoError, MongoInvalidArgumentError, @@ -545,17 +546,7 @@ export class ConnectionPool extends TypedEventEmitter { // use the provided connection, and do _not_ check it in after execution fn(undefined, conn, (fnErr, result) => { if (fnErr) { - if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { - this.reauthenticate(conn, fn, (error, res) => { - if (error) { - callback(error); - } else { - callback(undefined, res); - } - }); - } else { - callback(fnErr); - } + this.withRequthentication(fnErr, conn, fn, callback); } else { callback(undefined, result); } @@ -566,17 +557,10 @@ export class ConnectionPool extends TypedEventEmitter { // don't callback with `err` here, we might want to act upon it inside `fn` fn(err as MongoError, conn, (fnErr, result) => { if (fnErr) { - if (conn && (fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { - this.reauthenticate(conn, fn, (error, res) => { - if (error) { - callback(error); - } else { - callback(undefined, res); - } - }); - } else { - callback(fnErr); + if (conn) { + return this.withRequthentication(fnErr, conn, fn, callback); } + return callback(fnErr); } else { callback(undefined, result); } @@ -588,6 +572,25 @@ export class ConnectionPool extends TypedEventEmitter { }); } + private withRequthentication( + fnErr: AnyError, + conn: Connection, + fn: WithConnectionCallback, + callback: Callback + ) { + if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + this.reauthenticate(conn, fn, (error, res) => { + if (error) { + callback(error); + } else { + callback(undefined, res); + } + }); + } else { + callback(fnErr); + } + } + /** * Reauthenticate on the same connection and then retry the operation. */ From d19cbda36a996e36b5fddea2d8e659790387cd11 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 17:12:17 +0100 Subject: [PATCH 05/17] refactor: use reauth --- src/cmap/auth/auth_provider.ts | 14 ++++++++++++++ src/cmap/connection_pool.ts | 4 +--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 26f90dd779..4464630026 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -63,4 +63,18 @@ export class AuthProvider { // TODO(NODE-3483): Replace this with MongoMethodOverrideError callback(new MongoRuntimeError('`auth` method must be overridden by subclass')); } + + /** + * Reauthenticate. + * @param context - The shared auth context. + * @param callback - The callback. + */ + reauth(context: AuthContext, callback: Callback): void { + context.reauthenticating = true; + const cb: Callback = (error, result) => { + context.reauthenticating = false; + callback(error, result); + }; + this.auth(context, cb); + } } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index f7ac8a70e7..a999917e6d 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -603,7 +603,6 @@ export class ConnectionPool extends TypedEventEmitter { if (!authContext) { return callback(new MongoRuntimeError('No auth context found on connection.')); } - authContext.reauthenticating = true; const credentials = authContext.credentials; if (!credentials) { return callback( @@ -621,8 +620,7 @@ export class ConnectionPool extends TypedEventEmitter { ) ); } - provider.auth(authContext, error => { - authContext.reauthenticating = false; + provider.reauth(authContext, error => { if (error) { return callback(error); } From ae0734675e36519d9b33a7a9f4e7698d77f174d6 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 17:20:01 +0100 Subject: [PATCH 06/17] chore: mark oidc experimental --- src/cmap/auth/mongo_credentials.ts | 3 +++ src/cmap/auth/mongodb_oidc.ts | 9 +++++---- src/cmap/auth/providers.ts | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cmap/auth/mongo_credentials.ts b/src/cmap/auth/mongo_credentials.ts index adfd531411..b24c395fcc 100644 --- a/src/cmap/auth/mongo_credentials.ts +++ b/src/cmap/auth/mongo_credentials.ts @@ -37,8 +37,11 @@ export interface AuthMechanismProperties extends Document { SERVICE_REALM?: string; CANONICALIZE_HOST_NAME?: GSSAPICanonicalizationValue; AWS_SESSION_TOKEN?: string; + /** @experimental */ REQUEST_TOKEN_CALLBACK?: OIDCRequestFunction; + /** @experimental */ REFRESH_TOKEN_CALLBACK?: OIDCRefreshFunction; + /** @experimental */ PROVIDER_NAME?: 'aws'; } diff --git a/src/cmap/auth/mongodb_oidc.ts b/src/cmap/auth/mongodb_oidc.ts index 8f5f4af5b7..92c179cbaa 100644 --- a/src/cmap/auth/mongodb_oidc.ts +++ b/src/cmap/auth/mongodb_oidc.ts @@ -11,7 +11,7 @@ import { AwsServiceWorkflow } from './mongodb_oidc/aws_service_workflow'; import { CallbackWorkflow } from './mongodb_oidc/callback_workflow'; import type { Workflow } from './mongodb_oidc/workflow'; -/** @public */ +/** @experimental */ export interface OIDCMechanismServerStep1 { authorizationEndpoint?: string; tokenEndpoint?: string; @@ -21,21 +21,21 @@ export interface OIDCMechanismServerStep1 { requestScopes?: string[]; } -/** @public */ +/** @experimental */ export interface OIDCRequestTokenResult { accessToken: string; expiresInSeconds?: number; refreshToken?: string; } -/** @public */ +/** @experimental */ export type OIDCRequestFunction = ( principalName: string, serverResult: OIDCMechanismServerStep1, timeout: AbortSignal | number ) => Promise; -/** @public */ +/** @experimental */ export type OIDCRefreshFunction = ( principalName: string, serverResult: OIDCMechanismServerStep1, @@ -52,6 +52,7 @@ OIDC_WORKFLOWS.set('aws', new AwsServiceWorkflow()); /** * OIDC auth provider. + * @experimental */ export class MongoDBOIDC extends AuthProvider { /** diff --git a/src/cmap/auth/providers.ts b/src/cmap/auth/providers.ts index 74e3638ecc..d01c06324b 100644 --- a/src/cmap/auth/providers.ts +++ b/src/cmap/auth/providers.ts @@ -8,6 +8,7 @@ export const AuthMechanism = Object.freeze({ MONGODB_SCRAM_SHA1: 'SCRAM-SHA-1', MONGODB_SCRAM_SHA256: 'SCRAM-SHA-256', MONGODB_X509: 'MONGODB-X509', + /** @experimental */ MONGODB_OIDC: 'MONGODB-OIDC' } as const); From 36c5df06e0eae84e708d6b20240f048533556dbb Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 18:18:53 +0100 Subject: [PATCH 07/17] test: connect to replica set --- .evergreen/setup-oidc-roles.sh | 2 +- test/manual/mongodb_oidc.prose.test.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.evergreen/setup-oidc-roles.sh b/.evergreen/setup-oidc-roles.sh index f3f2e0a024..6be43905cf 100644 --- a/.evergreen/setup-oidc-roles.sh +++ b/.evergreen/setup-oidc-roles.sh @@ -5,4 +5,4 @@ set -o xtrace # Write all commands first to stderr cd ${DRIVERS_TOOLS}/.evergreen/auth_oidc . ./activate-authoidcvenv.sh -${DRIVERS_TOOLS}/mongodb/bin/mongosh setup_oidc.js +${DRIVERS_TOOLS}/mongodb/bin/mongosh "mongodb://localhost:27017,localhost:27018/?replicaSet=oidc-repl0&readPreference=primary" setup_oidc.js diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index 792fed37d7..14607df558 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -525,6 +525,9 @@ describe('MONGODB-OIDC', function () { }); after(async function () { + client.removeAllListeners('commandStarted'); + client.removeAllListeners('commandSucceeded'); + client.removeAllListeners('commandFailed'); await client?.close(); }); From e4677d5cdbb26f445f182d29790a9536d669e516 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 16 Mar 2023 18:47:17 +0100 Subject: [PATCH 08/17] fix: typo --- package.json | 2 +- src/cmap/connection_pool.ts | 17 ++++++++--------- test/manual/mongodb_oidc.prose.test.ts | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 9e5d40f3b2..651f11b1ed 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,7 @@ "check:atlas": "mocha --config test/manual/mocharc.json test/manual/atlas_connectivity.test.js", "check:adl": "mocha --config test/mocha_mongodb.json test/manual/atlas-data-lake-testing", "check:aws": "mocha --config test/mocha_mongodb.json test/integration/auth/mongodb_aws.test.ts", - "check:oidc": "mocha --config test/manual/mocharc.json test/manual/mongodb_oidc.prose.test.ts", + "check:oidc": "TRACE_SOCKETS=true mocha --config test/manual/mocharc.json test/manual/mongodb_oidc.prose.test.ts", "check:ocsp": "mocha --config test/manual/mocharc.json test/manual/ocsp_support.test.js", "check:kerberos": "mocha --config test/manual/mocharc.json test/manual/kerberos.test.js", "check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.js", diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index a999917e6d..5be3e332e4 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -546,10 +546,9 @@ export class ConnectionPool extends TypedEventEmitter { // use the provided connection, and do _not_ check it in after execution fn(undefined, conn, (fnErr, result) => { if (fnErr) { - this.withRequthentication(fnErr, conn, fn, callback); - } else { - callback(undefined, result); + return this.withReauthentication(fnErr, conn, fn, callback); } + return callback(undefined, result); }); } @@ -558,9 +557,10 @@ export class ConnectionPool extends TypedEventEmitter { fn(err as MongoError, conn, (fnErr, result) => { if (fnErr) { if (conn) { - return this.withRequthentication(fnErr, conn, fn, callback); + this.withReauthentication(fnErr, conn, fn, callback); + } else { + callback(fnErr); } - return callback(fnErr); } else { callback(undefined, result); } @@ -572,7 +572,7 @@ export class ConnectionPool extends TypedEventEmitter { }); } - private withRequthentication( + private withReauthentication( fnErr: AnyError, conn: Connection, fn: WithConnectionCallback, @@ -581,10 +581,9 @@ export class ConnectionPool extends TypedEventEmitter { if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { this.reauthenticate(conn, fn, (error, res) => { if (error) { - callback(error); - } else { - callback(undefined, res); + return callback(error); } + callback(undefined, res); }); } else { callback(fnErr); diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index 14607df558..58dd0ca6a3 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -525,10 +525,16 @@ describe('MONGODB-OIDC', function () { }); after(async function () { + console.log('removing listeners'); client.removeAllListeners('commandStarted'); client.removeAllListeners('commandSucceeded'); client.removeAllListeners('commandFailed'); + console.log('1 clearing cache'); + cache.clear(); + console.log('1 closing client'); await client?.close(); + console.log('1 closed'); + console.log(process._getActiveHandles()); }); context('on the first find invokation', function () { @@ -570,6 +576,18 @@ describe('MONGODB-OIDC', function () { await collection.findOne(); }); + after(async function () { + await client.db('admin').command({ + configureFailPoint: 'failCommand', + mode: 'off' + }); + console.log('clearing cache'); + cache.clear(); + console.log('closing client'); + await client?.close(); + console.log('closed'); + }); + // - Assert that the refresh callback has been called, if possible. it('calls the refresh callback', function () { expect(refreshInvokations).to.equal(1); From f31906755994f4d21908ffa9e72ce69b23d6f117 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 14:39:53 +0100 Subject: [PATCH 09/17] chore: fix tags --- src/cmap/auth/mongo_credentials.ts | 15 ++++++++++++--- src/cmap/auth/mongodb_oidc.ts | 20 ++++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/cmap/auth/mongo_credentials.ts b/src/cmap/auth/mongo_credentials.ts index b24c395fcc..139a8ec277 100644 --- a/src/cmap/auth/mongo_credentials.ts +++ b/src/cmap/auth/mongo_credentials.ts @@ -37,11 +37,20 @@ export interface AuthMechanismProperties extends Document { SERVICE_REALM?: string; CANONICALIZE_HOST_NAME?: GSSAPICanonicalizationValue; AWS_SESSION_TOKEN?: string; - /** @experimental */ + /** + * @alpha + * @experimental + */ REQUEST_TOKEN_CALLBACK?: OIDCRequestFunction; - /** @experimental */ + /** + * @alpha + * @experimental + */ REFRESH_TOKEN_CALLBACK?: OIDCRefreshFunction; - /** @experimental */ + /** + * @alpha + * @experimental + */ PROVIDER_NAME?: 'aws'; } diff --git a/src/cmap/auth/mongodb_oidc.ts b/src/cmap/auth/mongodb_oidc.ts index 92c179cbaa..e033999545 100644 --- a/src/cmap/auth/mongodb_oidc.ts +++ b/src/cmap/auth/mongodb_oidc.ts @@ -11,7 +11,10 @@ import { AwsServiceWorkflow } from './mongodb_oidc/aws_service_workflow'; import { CallbackWorkflow } from './mongodb_oidc/callback_workflow'; import type { Workflow } from './mongodb_oidc/workflow'; -/** @experimental */ +/** + * @alpha + * @experimental + */ export interface OIDCMechanismServerStep1 { authorizationEndpoint?: string; tokenEndpoint?: string; @@ -21,21 +24,30 @@ export interface OIDCMechanismServerStep1 { requestScopes?: string[]; } -/** @experimental */ +/** + * @alpha + * @experimental + */ export interface OIDCRequestTokenResult { accessToken: string; expiresInSeconds?: number; refreshToken?: string; } -/** @experimental */ +/** + * @alpha + * @experimental + */ export type OIDCRequestFunction = ( principalName: string, serverResult: OIDCMechanismServerStep1, timeout: AbortSignal | number ) => Promise; -/** @experimental */ +/** + * @alpha + * @experimental + */ export type OIDCRefreshFunction = ( principalName: string, serverResult: OIDCMechanismServerStep1, From 9f83ca4aa51f521cb4e98f3db83deef4209d2237 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 16:42:04 +0100 Subject: [PATCH 10/17] test: withConnection always has callback --- src/cmap/connection_pool.ts | 3 ++- test/unit/cmap/connection_pool.test.js | 31 -------------------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 5be3e332e4..a1ffecb0ec 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -548,8 +548,9 @@ export class ConnectionPool extends TypedEventEmitter { if (fnErr) { return this.withReauthentication(fnErr, conn, fn, callback); } - return callback(undefined, result); + callback(undefined, result); }); + return; } this.checkOut((err, conn) => { diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 62a9e04b5a..20b8ae0074 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -308,36 +308,5 @@ describe('Connection Pool', function () { callback ); }); - - it('should still manage a connection if no callback is provided', function (done) { - server.setMessageHandler(request => { - const doc = request.document; - if (isHello(doc)) { - request.reply(mock.HELLO); - } - }); - - const pool = new ConnectionPool(server, { - maxPoolSize: 1, - hostAddress: server.hostAddress() - }); - pool.ready(); - - const events = []; - pool.on('connectionCheckedOut', event => events.push(event)); - pool.on('connectionCheckedIn', event => { - events.push(event); - - expect(events).to.have.length(2); - expect(events[0]).to.be.instanceOf(cmapEvents.ConnectionCheckedOutEvent); - expect(events[1]).to.be.instanceOf(cmapEvents.ConnectionCheckedInEvent); - pool.close(done); - }); - - pool.withConnection(undefined, (err, conn, cb) => { - expect(err).to.not.exist; - cb(); - }); - }); }); }); From d791506a588c197a8168b0b998f66f3f55cd0cd5 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 17:13:40 +0100 Subject: [PATCH 11/17] fix: linter --- test/unit/cmap/connection_pool.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 20b8ae0074..28d88379b8 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -3,7 +3,6 @@ const { ConnectionPool } = require('../../mongodb'); const { WaitQueueTimeoutError } = require('../../mongodb'); const mock = require('../../tools/mongodb-mock/index'); -const cmapEvents = require('../../mongodb'); const sinon = require('sinon'); const { expect } = require('chai'); const { setImmediate } = require('timers'); From d5b949ee5db89b353b14fbc75e625a2ce3c56c3a Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 17:17:56 +0100 Subject: [PATCH 12/17] chore: update package json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 651f11b1ed..9e5d40f3b2 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,7 @@ "check:atlas": "mocha --config test/manual/mocharc.json test/manual/atlas_connectivity.test.js", "check:adl": "mocha --config test/mocha_mongodb.json test/manual/atlas-data-lake-testing", "check:aws": "mocha --config test/mocha_mongodb.json test/integration/auth/mongodb_aws.test.ts", - "check:oidc": "TRACE_SOCKETS=true mocha --config test/manual/mocharc.json test/manual/mongodb_oidc.prose.test.ts", + "check:oidc": "mocha --config test/manual/mocharc.json test/manual/mongodb_oidc.prose.test.ts", "check:ocsp": "mocha --config test/manual/mocharc.json test/manual/ocsp_support.test.js", "check:kerberos": "mocha --config test/manual/mocharc.json test/manual/kerberos.test.js", "check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.js", From c2bbde376ff47edee7ba1158c39407bf2ce3f72b Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 17:19:17 +0100 Subject: [PATCH 13/17] chore: remove console logs --- test/manual/mongodb_oidc.prose.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index 58dd0ca6a3..45642bd3ce 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -525,16 +525,11 @@ describe('MONGODB-OIDC', function () { }); after(async function () { - console.log('removing listeners'); client.removeAllListeners('commandStarted'); client.removeAllListeners('commandSucceeded'); client.removeAllListeners('commandFailed'); - console.log('1 clearing cache'); cache.clear(); - console.log('1 closing client'); await client?.close(); - console.log('1 closed'); - console.log(process._getActiveHandles()); }); context('on the first find invokation', function () { @@ -581,11 +576,8 @@ describe('MONGODB-OIDC', function () { configureFailPoint: 'failCommand', mode: 'off' }); - console.log('clearing cache'); cache.clear(); - console.log('closing client'); await client?.close(); - console.log('closed'); }); // - Assert that the refresh callback has been called, if possible. From d16ec0a0977e96a4025e1a8b43f1080f87755fe9 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 17:21:58 +0100 Subject: [PATCH 14/17] chore: update check and add type --- src/cmap/connection_pool.ts | 2 +- src/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index a1ffecb0ec..5365d19d07 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -579,7 +579,7 @@ export class ConnectionPool extends TypedEventEmitter { fn: WithConnectionCallback, callback: Callback ) { - if ((fnErr as MongoError).code === MONGODB_ERROR_CODES.Reauthenticate) { + if (fnErr instanceof MongoError && fnErr.code === MONGODB_ERROR_CODES.Reauthenticate) { this.reauthenticate(conn, fn, (error, res) => { if (error) { return callback(error); diff --git a/src/index.ts b/src/index.ts index 1bd5e812c5..2ea5b26124 100644 --- a/src/index.ts +++ b/src/index.ts @@ -197,7 +197,7 @@ export type { ResumeToken, UpdateDescription } from './change_stream'; -export { AuthContext, AuthContextOptions } from './cmap/auth/auth_provider'; +export type { AuthContext, AuthContextOptions } from './cmap/auth/auth_provider'; export type { AuthMechanismProperties, MongoCredentials, From c12a061831c3b557b0f8287d9a857665897ab066 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 17:39:18 +0100 Subject: [PATCH 15/17] chore: make experimental public --- src/cmap/auth/mongo_credentials.ts | 15 +++------------ src/cmap/auth/mongodb_oidc.ts | 8 ++++---- tsdoc.json | 13 +++++++++++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/cmap/auth/mongo_credentials.ts b/src/cmap/auth/mongo_credentials.ts index 139a8ec277..b24c395fcc 100644 --- a/src/cmap/auth/mongo_credentials.ts +++ b/src/cmap/auth/mongo_credentials.ts @@ -37,20 +37,11 @@ export interface AuthMechanismProperties extends Document { SERVICE_REALM?: string; CANONICALIZE_HOST_NAME?: GSSAPICanonicalizationValue; AWS_SESSION_TOKEN?: string; - /** - * @alpha - * @experimental - */ + /** @experimental */ REQUEST_TOKEN_CALLBACK?: OIDCRequestFunction; - /** - * @alpha - * @experimental - */ + /** @experimental */ REFRESH_TOKEN_CALLBACK?: OIDCRefreshFunction; - /** - * @alpha - * @experimental - */ + /** @experimental */ PROVIDER_NAME?: 'aws'; } diff --git a/src/cmap/auth/mongodb_oidc.ts b/src/cmap/auth/mongodb_oidc.ts index e033999545..d5983ce6c6 100644 --- a/src/cmap/auth/mongodb_oidc.ts +++ b/src/cmap/auth/mongodb_oidc.ts @@ -12,7 +12,7 @@ import { CallbackWorkflow } from './mongodb_oidc/callback_workflow'; import type { Workflow } from './mongodb_oidc/workflow'; /** - * @alpha + * @public * @experimental */ export interface OIDCMechanismServerStep1 { @@ -25,7 +25,7 @@ export interface OIDCMechanismServerStep1 { } /** - * @alpha + * @public * @experimental */ export interface OIDCRequestTokenResult { @@ -35,7 +35,7 @@ export interface OIDCRequestTokenResult { } /** - * @alpha + * @public * @experimental */ export type OIDCRequestFunction = ( @@ -45,7 +45,7 @@ export type OIDCRequestFunction = ( ) => Promise; /** - * @alpha + * @public * @experimental */ export type OIDCRefreshFunction = ( diff --git a/tsdoc.json b/tsdoc.json index 0d65244184..e572eb7d25 100644 --- a/tsdoc.json +++ b/tsdoc.json @@ -1,5 +1,8 @@ { "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json", + "extends": [ + "@microsoft/api-extractor/extends/tsdoc-base.json" + ], "tagDefinitions": [ { "syntaxKind": "block", @@ -17,5 +20,11 @@ "syntaxKind": "block", "tagName": "@category" } - ] -} + ], + "supportForTags": { + "@event": true, + "@since": true, + "@sinceServerVersion": true, + "@category": true + } +} \ No newline at end of file From b8e135534c695888730c2684e63af9f37c02e350 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 18:06:34 +0100 Subject: [PATCH 16/17] test: fix index test --- test/unit/index.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 77d40a4d52..833082b983 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -16,8 +16,6 @@ const EXPECTED_EXPORTS = [ 'AbstractCursor', 'Admin', 'AggregationCursor', - 'AuthContext', - 'AuthContextOptions', 'AuthMechanism', 'AutoEncryptionLoggerLevel', 'BatchType', From 455fa1321650dbf95b665435eb91d5b41506fc85 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 17 Mar 2023 19:41:13 +0100 Subject: [PATCH 17/17] fix: suggestions --- src/cmap/auth/auth_provider.ts | 6 +++++- test/manual/mongodb_oidc.prose.test.ts | 12 ++++++------ test/unit/cmap/auth/auth_provider.test.ts | 20 ++++++++++++++++++++ tsdoc.json | 13 ++----------- 4 files changed, 33 insertions(+), 18 deletions(-) create mode 100644 test/unit/cmap/auth/auth_provider.test.ts diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 4464630026..82942c0a9a 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -17,7 +17,7 @@ export class AuthContext { connection: Connection; /** The credentials to use for authentication */ credentials?: MongoCredentials; - /** If the context if for reauthentication. */ + /** If the context is for reauthentication. */ reauthenticating = false; /** The options passed to the `connect` method */ options: AuthContextOptions; @@ -70,6 +70,10 @@ export class AuthProvider { * @param callback - The callback. */ reauth(context: AuthContext, callback: Callback): void { + // If we are already reauthenticating this is a no-op. + if (context.reauthenticating) { + return callback(new MongoRuntimeError('Reauthentication already in progress.')); + } context.reauthenticating = true; const cb: Callback = (error, result) => { context.reauthenticating = false; diff --git a/test/manual/mongodb_oidc.prose.test.ts b/test/manual/mongodb_oidc.prose.test.ts index 45642bd3ce..ce5728e597 100644 --- a/test/manual/mongodb_oidc.prose.test.ts +++ b/test/manual/mongodb_oidc.prose.test.ts @@ -455,7 +455,7 @@ describe('MONGODB-OIDC', function () { // The driver MUST test reauthentication with MONGODB-OIDC for a read operation. describe('6. Reauthentication', function () { - let refreshInvokations = 0; + let refreshInvocations = 0; let findStarted = 0; let findSucceeded = 0; let findFailed = 0; @@ -478,7 +478,7 @@ describe('MONGODB-OIDC', function () { const token = await readFile(`${process.env.OIDC_TOKEN_DIR}/test_user1`, { encoding: 'utf8' }); - refreshInvokations++; + refreshInvocations++; return { accessToken: token, expiresInSeconds: 300 }; }; @@ -537,7 +537,7 @@ describe('MONGODB-OIDC', function () { findStarted = 0; findSucceeded = 0; findFailed = 0; - refreshInvokations = 0; + refreshInvocations = 0; saslStarted = 0; saslSucceeded = 0; }); @@ -546,7 +546,7 @@ describe('MONGODB-OIDC', function () { // - Assert that the refresh callback has not been called. it('does not call the refresh callback', async function () { await collection.findOne(); - expect(refreshInvokations).to.equal(0); + expect(refreshInvocations).to.equal(0); }); }); @@ -556,7 +556,7 @@ describe('MONGODB-OIDC', function () { findStarted = 0; findSucceeded = 0; findFailed = 0; - refreshInvokations = 0; + refreshInvocations = 0; saslStarted = 0; saslSucceeded = 0; await client.db('admin').command({ @@ -582,7 +582,7 @@ describe('MONGODB-OIDC', function () { // - Assert that the refresh callback has been called, if possible. it('calls the refresh callback', function () { - expect(refreshInvokations).to.equal(1); + expect(refreshInvocations).to.equal(1); }); // - Assert that a find operation was started twice and a saslStart operation diff --git a/test/unit/cmap/auth/auth_provider.test.ts b/test/unit/cmap/auth/auth_provider.test.ts new file mode 100644 index 0000000000..96d49c650c --- /dev/null +++ b/test/unit/cmap/auth/auth_provider.test.ts @@ -0,0 +1,20 @@ +import { expect } from 'chai'; + +import { AuthProvider, MongoRuntimeError } from '../../../mongodb'; + +describe('AuthProvider', function () { + describe('#reauth', function () { + context('when the provider is already reauthenticating', function () { + const provider = new AuthProvider(); + const context = { reauthenticating: true }; + + it('returns an error', function () { + provider.reauth(context, error => { + expect(error).to.exist; + expect(error).to.be.instanceOf(MongoRuntimeError); + expect(error?.message).to.equal('Reauthentication already in progress.'); + }); + }); + }); + }); +}); diff --git a/tsdoc.json b/tsdoc.json index e572eb7d25..0d65244184 100644 --- a/tsdoc.json +++ b/tsdoc.json @@ -1,8 +1,5 @@ { "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json", - "extends": [ - "@microsoft/api-extractor/extends/tsdoc-base.json" - ], "tagDefinitions": [ { "syntaxKind": "block", @@ -20,11 +17,5 @@ "syntaxKind": "block", "tagName": "@category" } - ], - "supportForTags": { - "@event": true, - "@since": true, - "@sinceServerVersion": true, - "@category": true - } -} \ No newline at end of file + ] +}