Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3795): unexpected No auth provider for DEFAULT defined error #3092

Merged
merged 10 commits into from Jan 21, 2022
10 changes: 10 additions & 0 deletions src/connection_string.ts
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
dariakp marked this conversation as resolved.
Show resolved Hide resolved
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
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 retain user-specified authSource with no credentials', function () {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
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', {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
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');
});
});
});