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

feat(NODE-3924)!: read tls files async #3776

Merged
merged 29 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9881228
chore(NODE-3924): update package-lock.json
W-A-James Jul 19, 2023
b7763ae
feat(NODE-3924)!: read tls files async
W-A-James Jul 19, 2023
0fbb2f9
test(NODE-3924): add unit test
W-A-James Jul 19, 2023
28a2ba5
test(NODE-3924): move tests to manual tests
W-A-James Jul 19, 2023
8bc7296
style(NODE-3924): eslint
W-A-James Jul 19, 2023
ddb1e7f
test(NODE-3924): update assertions
W-A-James Jul 19, 2023
32de88b
style(NODE-3924): remove unneeded import
W-A-James Jul 19, 2023
d69dd69
fix(NODE-3924): remove unused option
W-A-James Jul 20, 2023
b418f99
test(NODE-3924): update test name
W-A-James Jul 20, 2023
e571b80
test(NODE-3924): fix test setup and timeouts
W-A-James Jul 21, 2023
84a3a89
test(NODE-3924): update existing unit test
W-A-James Jul 21, 2023
8acfbb6
test(NODE-3924): update table
W-A-James Jul 21, 2023
52bf930
docs(NODE-3924): update tls options API docs
W-A-James Jul 21, 2023
694b4a7
style(NODE-3924): eslint
W-A-James Jul 21, 2023
0dfac88
test(NODE-3924): fix spec test
W-A-James Jul 21, 2023
135aadf
fix(NODE-3924): change option names to shadow spec tls option names
W-A-James Jul 21, 2023
428aa6f
Merge branch 'main' into NODE-3924
W-A-James Jul 21, 2023
1053452
ci(NODE-3924): revert unneeded ci change
W-A-James Jul 24, 2023
1e6d240
fix(NODE-3924): throw error on parse
W-A-James Jul 24, 2023
20d09ea
test(NODE-3924): update tests
W-A-James Jul 24, 2023
702f6f8
docs(NODE-3924): revert table change and modify heading
W-A-James Jul 24, 2023
7438df7
style(NODE-3924): eslint
W-A-James Jul 24, 2023
b1d4e51
fix(NODE-3924): address review comments
W-A-James Jul 26, 2023
253aa6d
fix(NODE-3924): revert unintended change
W-A-James Jul 26, 2023
c454e37
Update test/unit/mongo_client.test.js
W-A-James Jul 26, 2023
5352581
fix(NODE-3924): Change to throw at connect time
W-A-James Jul 28, 2023
a67d3a1
fix(NODE-3924): throw on non-string
W-A-James Jul 31, 2023
af14ffe
test(NODE-3924): add test to throw on non-string values
W-A-James Jul 31, 2023
1550a94
revert validation
W-A-James Jul 31, 2023
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@
"check:oidc-azure": "mocha --config test/mocha_mongodb.json test/integration/auth/mongodb_oidc_azure.prose.test.ts",
"check:ocsp": "mocha --config test/manual/mocharc.json test/manual/ocsp_support.test.js",
"check:kerberos": "nyc mocha --config test/manual/mocharc.json test/manual/kerberos.test.ts",
"check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.js",
"check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.ts",
"check:ldap": "nyc mocha --config test/manual/mocharc.json test/manual/ldap.test.js",
"check:socks5": "mocha --config test/manual/mocharc.json test/manual/socks5.test.ts",
"check:csfle": "mocha --config test/mocha_mongodb.json test/integration/client-side-encryption",
Expand Down
11 changes: 2 additions & 9 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as dns from 'dns';
import * as fs from 'fs';
import ConnectionString from 'mongodb-connection-string-url';
import { URLSearchParams } from 'url';

Expand Down Expand Up @@ -1097,16 +1096,10 @@ export const OPTIONS = {
}
},
tlsCAFile: {
target: 'ca',
transform({ values: [value] }) {
return fs.readFileSync(String(value), { encoding: 'ascii' });
}
type: 'string'
},
tlsCertificateKeyFile: {
target: 'key',
transform({ values: [value] }) {
return fs.readFileSync(String(value), { encoding: 'ascii' });
}
type: 'string'
},
tlsCertificateKeyFilePassword: {
target: 'passphrase',
Expand Down
22 changes: 21 additions & 1 deletion src/mongo_client.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { promises as fs } from 'fs';
import type { TcpNetConnectOpts } from 'net';
import type { ConnectionOptions as TLSConnectionOptions, TLSSocketOptions } from 'tls';
import { promisify } from 'util';
Expand Down Expand Up @@ -433,6 +434,14 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {

const options = this[kOptions];

if (options.tls) {
if (typeof options.tlsCAFile === 'string') {
options.ca ??= await fs.readFile(options.tlsCAFile, { encoding: 'utf8' });
}
if (typeof options.tlsCertificateKeyFile === 'string') {
options.key ??= await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' });
}
}
if (typeof options.srvHost === 'string') {
const hosts = await resolveSRVRecord(options);

Expand Down Expand Up @@ -767,7 +776,7 @@ export interface MongoOptions
*
* ### Additional options:
*
* | nodejs native option | driver spec compliant option name | driver option type |
* | nodejs native option | driver spec equivalent option name | driver option type |
* |:----------------------|:----------------------------------------------|:-------------------|
* | `ca` | `tlsCAFile` | `string` |
* | `crl` | N/A | `string` |
Expand All @@ -783,9 +792,20 @@ export interface MongoOptions
* If `tlsInsecure` is set to `false`, then it will set the node native options `checkServerIdentity`
* to a no-op and `rejectUnauthorized` to the inverse value of `tlsAllowInvalidCertificates`. If
* `tlsAllowInvalidCertificates` is not set, then `rejectUnauthorized` will be set to `true`.
*
* ### Note on `tlsCAFile` and `tlsCertificateKeyFile`
*
* The files specified by the paths passed in to the `tlsCAFile` and `tlsCertificateKeyFile` fields
* are read lazily on the first call to `MongoClient.connect`. Once these files have been read and
* the `ca` and `key` fields are populated, they will not be read again on subsequent calls to
* `MongoClient.connect`. As a result, until the first call to `MongoClient.connect`, the `ca`
* and `key` fields will be undefined.
*/
tls: boolean;

tlsCAFile?: string;
tlsCertificateKeyFile?: string;

/** @internal */
[featureFlag: symbol]: any;

Expand Down
3 changes: 2 additions & 1 deletion test/manual/mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
"require": "ts-node/register",
"reporter": "test/tools/reporter/mongodb_reporter.js",
"failZero": true,
"color": true
"color": true,
"timeout": 10000
}
44 changes: 0 additions & 44 deletions test/manual/tls_support.test.js

This file was deleted.

120 changes: 120 additions & 0 deletions test/manual/tls_support.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { expect } from 'chai';
import { promises as fs } from 'fs';

import { LEGACY_HELLO_COMMAND, MongoClient, type MongoClientOptions } from '../mongodb';

const REQUIRED_ENV = ['MONGODB_URI', 'SSL_KEY_FILE', 'SSL_CA_FILE'];

describe('TLS Support', function () {
for (const key of REQUIRED_ENV) {
if (process.env[key] == null) {
throw new Error(`skipping SSL tests, ${key} environment variable is not defined`);
}
}

const CONNECTION_STRING = process.env.MONGODB_URI as string;
const TLS_CERT_KEY_FILE = process.env.SSL_KEY_FILE as string;
const TLS_CA_FILE = process.env.SSL_CA_FILE as string;
const tlsSettings = {
tls: true,
tlsCertificateKeyFile: TLS_CERT_KEY_FILE,
tlsCAFile: TLS_CA_FILE
};

it(
'should connect with tls via client options',
makeConnectionTest(CONNECTION_STRING, tlsSettings)
);

it(
'should connect with tls via url options',
makeConnectionTest(
`${CONNECTION_STRING}?${Object.keys(tlsSettings)
.map(key => `${key}=${tlsSettings[key]}`)
.join('&')}`
)
);

context('when tls filepaths are provided', () => {
let client: MongoClient;
afterEach(async () => {
if (client) await client.close();
});

context('when tls filepaths have length > 0', () => {
beforeEach(async () => {
client = new MongoClient(CONNECTION_STRING, tlsSettings);
});

it('should read in files async at connect time', async () => {
expect(client.options).property('tlsCAFile', TLS_CA_FILE);
expect(client.options).property('tlsCertificateKeyFile', TLS_CERT_KEY_FILE);
expect(client.options).not.have.property('ca');
expect(client.options).not.have.property('key');

await client.connect();

expect(client.options).property('ca').to.exist;
expect(client.options).property('key').to.exist;
});

context('when client has been opened and closed more than once', function () {
it('should only read files once', async () => {
await client.connect();
await client.close();

const caFileAccessTime = (await fs.stat(TLS_CA_FILE)).atime;
const certKeyFileAccessTime = (await fs.stat(TLS_CERT_KEY_FILE)).atime;

await client.connect();

expect((await fs.stat(TLS_CA_FILE)).atime).to.deep.equal(caFileAccessTime);
expect((await fs.stat(TLS_CERT_KEY_FILE)).atime).to.deep.equal(certKeyFileAccessTime);
});
});
});

context('when tlsCAFile has length === 0', () => {
beforeEach(() => {
client = new MongoClient(CONNECTION_STRING, {
tls: true,
tlsCAFile: '',
tlsCertificateKeyFile: TLS_CERT_KEY_FILE
});
});

it('should throw an error at connect time', async () => {
const err = await client.connect().catch(e => e);

expect(err).to.be.instanceof(Error);
});
});

context('when tlsCertificateKeyFile has length === 0', () => {
beforeEach(() => {
client = new MongoClient(CONNECTION_STRING, {
tls: true,
tlsCAFile: TLS_CA_FILE,
tlsCertificateKeyFile: ''
});
});

it('should throw an error at connect time', async () => {
const err = await client.connect().catch(e => e);

expect(err).to.be.instanceof(Error);
});
});
});
});

function makeConnectionTest(connectionString: string, clientOptions?: MongoClientOptions) {
return async function () {
const client = new MongoClient(connectionString, clientOptions);

await client.connect();
await client.db('admin').command({ [LEGACY_HELLO_COMMAND]: 1 });
await client.db('test').collection('test').findOne({});
return await client.close();
};
}
4 changes: 2 additions & 2 deletions test/tools/uri_spec_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,13 @@ export function executeUriValidationTest(
.equal(optionValue);
break;
case 'tlsCertificateKeyFile':
expectedProp = 'key';
expectedProp = 'tlsCertificateKeyFile';
expect(options, `${errorMessage} ${optionKey} -> ${expectedProp}`)
.to.have.property(expectedProp)
.equal(optionValue);
break;
case 'tlsCAFile':
expectedProp = 'ca';
expectedProp = 'tlsCAFile';
expect(options, `${errorMessage} ${optionKey} -> ${expectedProp}`)
.to.have.property(expectedProp)
.equal(optionValue);
Expand Down
15 changes: 7 additions & 8 deletions test/unit/mongo_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('MongoOptions', function () {
*
* ### Additional options:
*
* | nodejs native option | driver spec compliant option name | driver option type |
* | nodejs native option | driver spec equivalent option name | driver option type |
* |:----------------------|:----------------------------------------------|:-------------------|
* | `ca` | `tlsCAFile` | `string` |
* | `crl` | N/A | `string` |
Expand All @@ -55,12 +55,11 @@ describe('MongoOptions', function () {
* | see note below | `tlsInsecure` | `boolean` |
*
*/
expect(options).to.not.have.property('tlsCertificateKeyFile');
expect(options).to.not.have.property('tlsCAFile');
expect(options).to.not.have.property('tlsCertificateKeyFilePassword');
expect(options).has.property('ca', '');
expect(options).has.property('key');
expect(options.key).has.length(0);
expect(options).to.not.have.property('key');
expect(options).to.not.have.property('ca');
expect(options).to.have.property('tlsCertificateKeyFile', filename);
expect(options).to.have.property('tlsCAFile', filename);
expect(options).has.property('passphrase', 'tlsCertificateKeyFilePassword');
expect(options).has.property('tls', true);
});
Expand Down Expand Up @@ -394,10 +393,10 @@ describe('MongoOptions', function () {
const optsFromObject = parseOptions('mongodb://localhost/', {
tlsCertificateKeyFile: 'testCertKey.pem'
});
expect(optsFromObject).to.have.property('key', 'cert key');
expect(optsFromObject).to.have.property('tlsCertificateKeyFile', 'testCertKey.pem');

const optsFromUri = parseOptions('mongodb://localhost?tlsCertificateKeyFile=testCertKey.pem');
expect(optsFromUri).to.have.property('key', 'cert key');
expect(optsFromUri).to.have.property('tlsCertificateKeyFile', 'testCertKey.pem');
});
});

Expand Down