From 6acbfab7819554dacb9504b8fa485c737f363a3a Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Mon, 15 Jan 2024 17:58:40 +0100 Subject: [PATCH 1/4] fix(NODE-5621): unskip and fix the unicode auth prose spec tests --- test/integration/auth/auth.prose.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/integration/auth/auth.prose.test.ts b/test/integration/auth/auth.prose.test.ts index e9d0fc651ff..8dc33a89754 100644 --- a/test/integration/auth/auth.prose.test.ts +++ b/test/integration/auth/auth.prose.test.ts @@ -357,6 +357,7 @@ describe('Authentication Spec Prose Tests', function () { const db = utilClient.db('admin'); await Promise.all(users.map(user => db.removeUser(user.username))); await utilClient?.close(); + await client?.close(); }); for (const { username, password } of [ @@ -365,7 +366,7 @@ describe('Authentication Spec Prose Tests', function () { { username: '\u2168', password: 'IV' }, { username: '\u2168', password: 'I\u00ADV' } ]) { - it.skip( + it( `logs in with username "${username}" and password "${password}"`, metadata, async function () { @@ -375,11 +376,11 @@ describe('Authentication Spec Prose Tests', function () { authMechanism: 'SCRAM-SHA-256' }; - client = this.configuration.newClient(options); + client = this.configuration.newClient(process.env.MONGODB_URI, options); const stats = await client.db('admin').stats(); expect(stats).to.exist; } - ).skipReason = 'todo(NODE-5621): fix the issue with unicode characters.'; + ); } }); }); From 06d52010a2127c4c41bb81eb7a9940e7ec19c73a Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 17 Jan 2024 15:56:13 +0100 Subject: [PATCH 2/4] test: update url helper --- test/integration/auth/auth.prose.test.ts | 72 ++++++++++++++++++------ test/tools/runner/config.ts | 10 +++- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/test/integration/auth/auth.prose.test.ts b/test/integration/auth/auth.prose.test.ts index 8dc33a89754..626cddc270b 100644 --- a/test/integration/auth/auth.prose.test.ts +++ b/test/integration/auth/auth.prose.test.ts @@ -305,7 +305,6 @@ describe('Authentication Spec Prose Tests', function () { ); }); - // todo(NODE-5621): fix the issue with unicode characters. describe('Step 4', function () { /** * Step 4 @@ -343,6 +342,12 @@ describe('Authentication Spec Prose Tests', function () { utilClient = this.configuration.newClient(); const db = utilClient.db('admin'); + try { + await Promise.all(users.map(user => db.removeUser(user.username))); + } catch (err) { + /** We ensure that users are deleted. No action needed. */ + } + const createUserCommands = users.map(user => ({ createUser: user.username, pwd: user.password, @@ -354,34 +359,65 @@ describe('Authentication Spec Prose Tests', function () { }); afterEach(async function () { - const db = utilClient.db('admin'); - await Promise.all(users.map(user => db.removeUser(user.username))); await utilClient?.close(); await client?.close(); }); - for (const { username, password } of [ - { username: 'IX', password: 'IX' }, - { username: 'IX', password: 'I\u00ADX' }, - { username: '\u2168', password: 'IV' }, - { username: '\u2168', password: 'I\u00ADV' } - ]) { - it( - `logs in with username "${username}" and password "${password}"`, - metadata, - async function () { + context('username and password in url', () => { + for (const { username, password } of [ + { username: 'IX', password: 'IX' }, + { username: 'IX', password: 'I\u00ADX' }, + { username: '\u2168', password: 'IV' }, + { username: '\u2168', password: 'I\u00ADV' } + ]) { + it('logs successfully', metadata, async function () { const options = { - auth: { username, password }, authSource: 'admin', authMechanism: 'SCRAM-SHA-256' }; + client = this.configuration.newClient( + this.configuration.url({ username, password }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + } + }); - client = this.configuration.newClient(process.env.MONGODB_URI, options); + context('username and password in server options', () => { + for (const { username, password } of [ + { username: 'IX', password: 'IX' }, + { username: 'IX', password: 'I\u00ADX' }, + { username: '\u2168', password: 'IV' }, + { username: '\u2168', password: 'I\u00ADV' } + ]) { + it('logs successfully', metadata, async function () { + const options = { + auth: { username, password }, + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient({}, options); const stats = await client.db('admin').stats(); expect(stats).to.exist; - } - ); - } + }); + } + }); + + context('username and password in db options', () => { + it('logs successfully', metadata, async function () { + const options = { + auth: { username: 'IX', password: 'IX' }, + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + + client = this.configuration.newClient(options); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + }); }); }); }); diff --git a/test/tools/runner/config.ts b/test/tools/runner/config.ts index b24a9e77d87..201bd25bfc1 100644 --- a/test/tools/runner/config.ts +++ b/test/tools/runner/config.ts @@ -277,8 +277,14 @@ export class TestConfiguration { url.pathname = `/${options.db}`; - const username = this.options.username || (this.options.auth && this.options.auth.username); - const password = this.options.password || (this.options.auth && this.options.auth.password); + const username = + options.username || + this.options.username || + (this.options.auth && this.options.auth.username); + const password = + options.password || + this.options.password || + (this.options.auth && this.options.auth.password); if (username) { url.username = username; From 6323e3bb004a67f55264e03d9fc6f4471407a4f4 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 17 Jan 2024 15:58:30 +0100 Subject: [PATCH 3/4] test: remove test for server options --- test/integration/auth/auth.prose.test.ts | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/integration/auth/auth.prose.test.ts b/test/integration/auth/auth.prose.test.ts index 626cddc270b..e8337a99fde 100644 --- a/test/integration/auth/auth.prose.test.ts +++ b/test/integration/auth/auth.prose.test.ts @@ -385,26 +385,6 @@ describe('Authentication Spec Prose Tests', function () { } }); - context('username and password in server options', () => { - for (const { username, password } of [ - { username: 'IX', password: 'IX' }, - { username: 'IX', password: 'I\u00ADX' }, - { username: '\u2168', password: 'IV' }, - { username: '\u2168', password: 'I\u00ADV' } - ]) { - it('logs successfully', metadata, async function () { - const options = { - auth: { username, password }, - authSource: 'admin', - authMechanism: 'SCRAM-SHA-256' - }; - client = this.configuration.newClient({}, options); - const stats = await client.db('admin').stats(); - expect(stats).to.exist; - }); - } - }); - context('username and password in db options', () => { it('logs successfully', metadata, async function () { const options = { From d395fab415356f910decca0ddac0a895537b1af3 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 18 Jan 2024 15:57:37 +0100 Subject: [PATCH 4/4] fix: do not prefer configuration options over server options in newClient --- test/integration/auth/auth.prose.test.ts | 184 ++++++++++++++++++++--- test/tools/runner/config.ts | 70 ++++----- 2 files changed, 197 insertions(+), 57 deletions(-) diff --git a/test/integration/auth/auth.prose.test.ts b/test/integration/auth/auth.prose.test.ts index e8337a99fde..77fa40562d2 100644 --- a/test/integration/auth/auth.prose.test.ts +++ b/test/integration/auth/auth.prose.test.ts @@ -339,7 +339,7 @@ describe('Authentication Spec Prose Tests', function () { ]; beforeEach(async function () { - utilClient = this.configuration.newClient(); + utilClient = this.configuration.newClient(this.configuration.url()); const db = utilClient.db('admin'); try { @@ -363,41 +363,185 @@ describe('Authentication Spec Prose Tests', function () { await client?.close(); }); - context('username and password in url', () => { - for (const { username, password } of [ - { username: 'IX', password: 'IX' }, - { username: 'IX', password: 'I\u00ADX' }, - { username: '\u2168', password: 'IV' }, - { username: '\u2168', password: 'I\u00ADV' } - ]) { - it('logs successfully', metadata, async function () { + context('auth credentials in options', () => { + it('logs in with non-normalized username and password', metadata, async function () { + const options = { + auth: { username: 'IX', password: 'IX' }, + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + + client = this.configuration.newClient({}, options); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + + it( + 'logs in with non-normalized username and normalized password', + metadata, + async function () { const options = { + auth: { username: 'IX', password: 'I\u00ADX' }, authSource: 'admin', authMechanism: 'SCRAM-SHA-256' }; - client = this.configuration.newClient( - this.configuration.url({ username, password }), - options - ); + + client = this.configuration.newClient({}, options); const stats = await client.db('admin').stats(); expect(stats).to.exist; - }); - } - }); + } + ); - context('username and password in db options', () => { - it('logs successfully', metadata, async function () { + it( + 'logs in with normalized username and non-normalized password', + metadata, + async function () { + const options = { + auth: { username: '\u2168', password: 'IV' }, + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + + client = this.configuration.newClient({}, options); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + } + ); + + it('logs in with normalized username and normalized password', metadata, async function () { const options = { - auth: { username: 'IX', password: 'IX' }, + auth: { username: '\u2168', password: 'I\u00ADV' }, authSource: 'admin', authMechanism: 'SCRAM-SHA-256' }; - client = this.configuration.newClient(options); + client = this.configuration.newClient({}, options); const stats = await client.db('admin').stats(); expect(stats).to.exist; }); }); + + context('auth credentials in url', () => { + context('encoded', () => { + it('logs in with not encoded username and password', metadata, async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: 'IX', password: 'IX' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + + it('logs in with not encoded username and encoded password', metadata, async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: 'IX', password: 'I%C2%ADX' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + + it('logs in with encoded username and not encoded password', metadata, async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: '%E2%85%A8', password: 'IV' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + + it('logs in with encoded username and encoded password', metadata, async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: '%E2%85%A8', password: 'I%C2%ADV' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + }); + + context('normalized', () => { + it('logs in with non-normalized username and password', metadata, async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: 'IX', password: 'IX' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + }); + + it( + 'logs in with non-normalized username and normalized password', + metadata, + async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: 'IX', password: 'I\u00ADX' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + } + ); + + it( + 'logs in with normalized username and non-normalized password', + metadata, + async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: '\u2168', password: 'I\u00ADV' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + } + ); + + it( + 'logs in with normalized username and normalized password', + metadata, + async function () { + const options = { + authSource: 'admin', + authMechanism: 'SCRAM-SHA-256' + }; + client = this.configuration.newClient( + this.configuration.url({ username: '\u2168', password: 'I\u00ADV' }), + options + ); + const stats = await client.db('admin').stats(); + expect(stats).to.exist; + } + ); + }); + }); }); }); }); diff --git a/test/tools/runner/config.ts b/test/tools/runner/config.ts index 201bd25bfc1..e4f3dd52a3f 100644 --- a/test/tools/runner/config.ts +++ b/test/tools/runner/config.ts @@ -158,54 +158,59 @@ export class TestConfiguration { return uri.indexOf('MONGODB-OIDC') > -1 && uri.indexOf('PROVIDER_NAME:azure') > -1; } - newClient(dbOptions?: string | Record, serverOptions?: Record) { + newClient(urlOrQueryOptions?: string | Record, serverOptions?: Record) { serverOptions = Object.assign({}, getEnvironmentalOptions(), serverOptions); - // support MongoClient constructor form (url, options) for `newClient` - if (typeof dbOptions === 'string') { - return new MongoClient(dbOptions, serverOptions); + // Support MongoClient constructor form (url, options) for `newClient`. + if (typeof urlOrQueryOptions === 'string') { + if (Reflect.has(serverOptions, 'host') || Reflect.has(serverOptions, 'port')) { + throw new Error(`Cannot use options to specify host/port, must be in ${urlOrQueryOptions}`); + } + + return new MongoClient(urlOrQueryOptions, serverOptions); } - dbOptions = dbOptions || {}; - // Fall back - let dbHost = (serverOptions && serverOptions.host) || this.options.host; - const dbPort = (serverOptions && serverOptions.port) || this.options.port; + const queryOptions = urlOrQueryOptions || {}; + + // Fall back. + let dbHost = serverOptions.host || this.options.host; if (dbHost.indexOf('.sock') !== -1) { dbHost = qs.escape(dbHost); } + const dbPort = serverOptions.port || this.options.port; - if (this.options.authMechanism) { - Object.assign(dbOptions, { + if (this.options.authMechanism && !serverOptions.authMechanism) { + Object.assign(queryOptions, { authMechanism: this.options.authMechanism }); } - if (this.options.authMechanismProperties) { - Object.assign(dbOptions, { + if (this.options.authMechanismProperties && !serverOptions.authMechanismProperties) { + Object.assign(queryOptions, { authMechanismProperties: convertToConnStringMap(this.options.authMechanismProperties) }); } - if (this.options.replicaSet) { - Object.assign(dbOptions, { replicaSet: this.options.replicaSet }); + if (this.options.replicaSet && !serverOptions.replicaSet) { + Object.assign(queryOptions, { replicaSet: this.options.replicaSet }); } if (this.options.proxyURIParams) { for (const [name, value] of Object.entries(this.options.proxyURIParams)) { if (value) { - dbOptions[name] = value; + queryOptions[name] = value; } } } - // Flatten any options nested under `writeConcern` before we make the connection string - if (dbOptions.writeConcern) { - Object.assign(dbOptions, dbOptions.writeConcern); - delete dbOptions.writeConcern; + // Flatten any options nested under `writeConcern` before we make the connection string. + if (queryOptions.writeConcern && !serverOptions.writeConcern) { + Object.assign(queryOptions, queryOptions.writeConcern); + delete queryOptions.writeConcern; } if (this.topologyType === TopologyType.LoadBalanced && !this.isServerless) { - dbOptions.loadBalanced = true; + queryOptions.loadBalanced = true; } const urlOptions: url.UrlObject = { @@ -213,33 +218,30 @@ export class TestConfiguration { slashes: true, hostname: dbHost, port: this.isServerless ? null : dbPort, - query: dbOptions, + query: queryOptions, pathname: '/' }; - if (this.options.auth) { + if (this.options.auth && !serverOptions.auth) { const { username, password } = this.options.auth; if (username) { urlOptions.auth = `${encodeURIComponent(username)}:${encodeURIComponent(password)}`; } } - if (dbOptions.auth) { - const { username, password } = dbOptions.auth; + if (queryOptions.auth) { + const { username, password } = queryOptions.auth; if (username) { urlOptions.auth = `${encodeURIComponent(username)}:${encodeURIComponent(password)}`; } } if (typeof urlOptions.query === 'object') { - // Auth goes at the top of the uri, not in the searchParams - delete urlOptions.query.auth; + // Auth goes at the top of the uri, not in the searchParams. + delete urlOptions.query?.auth; } const connectionString = url.format(urlOptions); - if (Reflect.has(serverOptions, 'host') || Reflect.has(serverOptions, 'port')) { - throw new Error(`Cannot use options to specify host/port, must be in ${connectionString}`); - } return new MongoClient(connectionString, serverOptions); } @@ -277,14 +279,8 @@ export class TestConfiguration { url.pathname = `/${options.db}`; - const username = - options.username || - this.options.username || - (this.options.auth && this.options.auth.username); - const password = - options.password || - this.options.password || - (this.options.auth && this.options.auth.password); + const username = options.username || this.options.auth?.username; + const password = options.password || this.options.auth?.password; if (username) { url.username = username;