Skip to content

Commit

Permalink
feat(NODE-5233)!: prevent session from one client from being used on …
Browse files Browse the repository at this point in the history
…another (#3790)

Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
  • Loading branch information
nbbeeken and baileympearson authored Aug 4, 2023
1 parent c08060d commit 9268b35
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 3 deletions.
9 changes: 8 additions & 1 deletion src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,14 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
return client.connect();
}

/** Starts a new session on the server */
/**
* Creates a new ClientSession. When using the returned session in an operation
* a corresponding ServerSession will be created.
*
* @remarks
* A ClientSession instance may only be passed to operations being performed on the same
* MongoClient it was started from.
*/
startSession(options?: ClientSessionOptions): ClientSession {
const session = new ClientSession(
this,
Expand Down
3 changes: 3 additions & 0 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
MongoError,
MongoErrorLabel,
MongoExpiredSessionError,
MongoInvalidArgumentError,
MongoNetworkError,
MongoNotConnectedError,
MongoRuntimeError,
Expand Down Expand Up @@ -118,6 +119,8 @@ async function executeOperationAsync<
throw new MongoExpiredSessionError('Use of expired sessions is not permitted');
} else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
throw new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later');
} else if (session.client !== client) {
throw new MongoInvalidArgumentError('ClientSession must be from the same MongoClient');
}

const readPreference = operation.readPreference ?? ReadPreference.primary;
Expand Down
55 changes: 54 additions & 1 deletion test/integration/sessions/sessions.prose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,64 @@ import {
type Collection,
type CommandStartedEvent,
MongoClient,
MongoDriverError
MongoDriverError,
MongoInvalidArgumentError
} from '../../mongodb';
import { sleep } from '../../tools/utils';

describe('Sessions Prose Tests', () => {
describe('5. Session argument is for the right client', () => {
let client1: MongoClient;
let client2: MongoClient;
beforeEach(async function () {
client1 = this.configuration.newClient();
client2 = this.configuration.newClient();
});

afterEach(async function () {
await client1?.close();
await client2?.close();
});

/**
* Steps:
* - Create client1 and client2
* - Get database from client1
* - Get collection from database
* - Start session from client2
* - Call collection.insertOne(session,...)
* - Assert that an error was reported because session was not started from client1
*
* This validation lives in our executeOperation layer so it applies universally.
* A find and an insert provide enough coverage, we determined we do not need to enumerate every possible operation.
*/
context(
'when session is started from a different client than operation is being run on',
() => {
it('insertOne operation throws a MongoInvalidArgumentError', async () => {
const db = client1.db();
const collection = db.collection('test');
const session = client2.startSession();
const error = await collection.insertOne({}, { session }).catch(error => error);
expect(error).to.be.instanceOf(MongoInvalidArgumentError);
expect(error).to.match(/ClientSession must be from the same MongoClient/i);
});

it('find operation throws a MongoInvalidArgumentError', async () => {
const db = client1.db();
const collection = db.collection('test');
const session = client2.startSession();
const error = await collection
.find({}, { session })
.toArray()
.catch(error => error);
expect(error).to.be.instanceOf(MongoInvalidArgumentError);
expect(error).to.match(/ClientSession must be from the same MongoClient/i);
});
}
);
});

describe('14. Implicit sessions only allocate their server session after a successful connection checkout', () => {
let client: MongoClient;
let testCollection: Collection<{ _id: number; a?: number }>;
Expand Down
28 changes: 28 additions & 0 deletions test/integration/sessions/sessions.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from 'chai';
import { MongoClient as LegacyMongoClient } from 'mongodb-legacy';

import type { CommandStartedEvent, CommandSucceededEvent, MongoClient } from '../../mongodb';
import { LEGACY_HELLO_COMMAND, MongoServerError } from '../../mongodb';
Expand Down Expand Up @@ -422,4 +423,31 @@ describe('Sessions Spec', function () {
expect(new Set(events.map(ev => ev.command.lsid.id.toString('hex'))).size).to.equal(2);
});
});

context('when using a LegacyMongoClient', () => {
let legacyClient;
beforeEach(async function () {
const options = this.configuration.serverApi
? { serverApi: this.configuration.serverApi }
: {};
legacyClient = new LegacyMongoClient(this.configuration.url(), options);
});

afterEach(async function () {
await legacyClient?.close();
});

it('insertOne accepts session started by legacy client', async () => {
const db = legacyClient.db();
const collection = db.collection('test');
const session = legacyClient.startSession();
const error = await collection.insertOne({}, { session }).catch(error => error);
expect(error).to.not.be.instanceOf(Error);
});

it('session returned by legacy startSession has reference to legacyClient', async () => {
const session = legacyClient.startSession();
expect(session).to.have.property('client', legacyClient);
});
});
});
3 changes: 2 additions & 1 deletion test/tools/runner/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
type AuthMechanism,
HostAddress,
MongoClient,
type ServerApi,
TopologyType,
type WriteConcernSettings
} from '../../mongodb';
Expand Down Expand Up @@ -71,7 +72,7 @@ export class TestConfiguration {
auth?: { username: string; password: string; authSource?: string };
proxyURIParams?: ProxyParams;
};
serverApi: string;
serverApi: ServerApi;

constructor(uri: string, context: Record<string, any>) {
const url = new ConnectionString(uri);
Expand Down

0 comments on commit 9268b35

Please sign in to comment.