Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,16 @@ export function parseOptions(
}

mongoOptions.credentials.validate();

// Check if the only auth related option provided was authSource, if so we can remove credentials
if (
mongoOptions.credentials.password === '' &&
mongoOptions.credentials.username === '' &&
mongoOptions.credentials.mechanism === AuthMechanism.MONGODB_DEFAULT &&
Object.keys(mongoOptions.credentials.mechanismProperties).length === 0
) {
delete mongoOptions.credentials;
}
}

if (!mongoOptions.dbName) {
Expand Down
56 changes: 54 additions & 2 deletions test/unit/connection_string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ import { promisify } from 'util';
import { MongoCredentials } from '../../src/cmap/auth/mongo_credentials';
import { AUTH_MECHS_AUTH_SRC_EXTERNAL, AuthMechanism } from '../../src/cmap/auth/providers';
import { parseOptions, resolveSRVRecord } from '../../src/connection_string';
import { MongoDriverError, MongoInvalidArgumentError, MongoParseError } from '../../src/error';
import { MongoOptions } from '../../src/mongo_client';
import {
MongoAPIError,
MongoDriverError,
MongoInvalidArgumentError,
MongoParseError
} from '../../src/error';
import { MongoClient, MongoOptions } from '../../src/mongo_client';

describe('Connection String', function () {
it('should not support auth passed with user', function () {
Expand Down Expand Up @@ -89,6 +94,37 @@ describe('Connection String', function () {
expect(options.credentials.source).to.equal('$external');
});

it('should omit credentials option when the only authSource is provided', function () {
let options = parseOptions(`mongodb://a/?authSource=someDb`);
expect(options).to.not.have.property('credentials');
options = parseOptions(`mongodb+srv://a/?authSource=someDb`);
expect(options).to.not.have.property('credentials');
});

it('should omit credentials and not throw a MongoAPIError if the only auth related option is authSource', async () => {
// The error we're looking to **not** see is
// `new MongoInvalidArgumentError('No AuthProvider for ${credentials.mechanism} defined.')`
// in `prepareHandshakeDocument` and/or `performInitialHandshake`.
// Neither function is exported currently but if I did export them because of the inlined callbacks
// I think I would need to mock quite a bit of internals to get down to that layer.
// My thinking is I can lean on server selection failing for th unit tests to assert we at least don't get an error related to auth.
const client = new MongoClient('mongodb://localhost:123/?authSource=someDb', {
serverSelectionTimeoutMS: 500
});

let thrownError: Error;
try {
// relies on us not running a mongod on port 123, fairly likely assumption
await client.connect();
} catch (error) {
thrownError = error;
}

// We should fail to connect, not fail to find an auth provider thus we should not find a MongoAPIError
expect(thrownError).to.not.be.instanceOf(MongoAPIError);
expect(client.options).to.not.have.a.property('credentials');
});

it('should parse a numeric authSource with variable width', function () {
const options = parseOptions('mongodb://test@localhost/?authSource=0001');
expect(options.credentials.source).to.equal('0001');
Expand Down Expand Up @@ -334,5 +370,21 @@ describe('Connection String', function () {
expect(options).property('credentials').to.equal(credentials);
expect(options).to.have.nested.property('credentials.source', 'admin');
});

it('should retain specified authSource with no provided credentials', async function () {
makeStub('authSource=thisShouldBeAuthSource');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a similar test for a non-srv connection string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just added + added extra assertions on the DEFAULT mechanism to clearly test the specified error scenario in the ticket

const credentials = {};
const options = {
credentials,
srvHost: 'test.mock.test.build.10gen.cc',
srvServiceName: 'mongodb',
userSpecifiedAuthSource: false
} as MongoOptions;

await resolveSRVRecordAsync(options as any);
expect(options).to.have.nested.property('credentials.username', '');
expect(options).to.have.nested.property('credentials.mechanism', 'DEFAULT');
expect(options).to.have.nested.property('credentials.source', 'thisShouldBeAuthSource');
});
});
});