Skip to content

Commit

Permalink
Cleanup integration and unit test suites
Browse files Browse the repository at this point in the history
  • Loading branch information
tanvipise committed Sep 19, 2024
1 parent b4c002f commit b300bfb
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 16 deletions.
6 changes: 3 additions & 3 deletions packages/v-connection-string/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ function parse(str) {
}
}

// if the tls mode specified in the connection string is an invalid option, use the default - disable.
if (config.tls_mode && !['require', 'verify-ca', 'verify-full'].includes(config.tls_mode)) {
config.tls_mode = 'disable'
// if the tls mode specified in the connection string is an invalid option, use the default - prefer.
if (config.tls_mode && !['disable', 'require', 'verify-ca', 'verify-full'].includes(config.tls_mode)) {
config.tls_mode = 'prefer'
}

var auth = (result.auth || ':').split(':')
Expand Down
4 changes: 2 additions & 2 deletions packages/v-connection-string/test/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,9 @@ describe('parse', function () {
})

it('configuration parameter tls_mode=no-verify', function () {
var connectionString = 'vertica:///?tls_mode=no-verify' // not a supported tls_mode, should instead default to disable
var connectionString = 'vertica:///?tls_mode=no-verify' // not a supported tls_mode, should instead default to prefer
var subject = parse(connectionString)
subject.tls_mode.should.eql('disable')
subject.tls_mode.should.eql('prefer')
})

it('configuration parameter tls_mode=verify-ca', function () {
Expand Down
22 changes: 20 additions & 2 deletions packages/vertica-nodejs/lib/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class Connection extends EventEmitter {

if (this.tls_config === undefined) {
this.tls_mode = config.tls_mode || 'prefer'
//this.tls_client_key = config.tls_client_key
//this.tls_client_cert = config.tls_client_cert
this.tls_client_key = config.tls_client_key
this.tls_client_cert = config.tls_client_cert
this.tls_trusted_certs = config.tls_trusted_certs
this.tls_host = config.tls_host
}
Expand Down Expand Up @@ -146,6 +146,24 @@ class Connection extends EventEmitter {
return self.emit('error', err)
}
}
else if (self.tls_mode === 'prefer') { // basic TLS connection, does not verify CA certificate
tls_options.rejectUnauthorized = false
tls_options.checkServerIdentity = (host , cert) => undefined
if (self.tls_trusted_certs) {
tls_options.ca = fs.readFileSync(self.tls_trusted_certs).toString()
}
/*if (self.tls_client_cert) {// the client won't know whether or not this is required, depends on server mode
tls_options.cert = fs.readFileSync(self.tls_client_cert).toString()
}
if (self.tls_client_key) {
tls_options.key = fs.readFileSync(self.tls_client_key).toString()
}*/
try {
self.stream = tls.connect(tls_options);
} catch (err) {
return self.emit('error', err)
}
}
else if (self.tls_mode === 'verify-ca') { //verify that the server certificate is signed by a trusted CA
try {
tls_options.rejectUnauthorized = true
Expand Down
12 changes: 7 additions & 5 deletions packages/vertica-nodejs/test/integration/connection/tls-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,22 @@ suite.test('vertica tls - prefer mode', function () {
assert.equal(client.tls_mode, vertica.defaults.tls_mode)
client.connect(err => {
if (err) {
console.log(err)
assert(false)
// shouldn't fail to connect
assert(err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
return
}
//Verify if client is using a TLS connection
// If connection succeeds, check for TLS connection
client.query("SELECT mode FROM tls_configurations where name = 'server' LIMIT 1", (err, res) => {
if (err) {
console.log(err)
assert(false)
}
// Assert only if server supports TLS
if (['ENABLE', 'TRY_VERIFY', 'VERIFY_CA', 'VERIFY_FULL'].includes(res.rows[0].mode)) {
assert.equal(client.connection.stream.constructor.name.toString(), "TLSSocket")
assert(client.connection.stream.constructor.name.toString(), "TLSSocket")
}
else {
assert.notEqual(client.connection.stream.constructor.name.toString(), "TLSSocket")
assert.notEqual(client.connection.stream.constructor.name.toString(), "TLSSocket")
}
client.end()
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
//Should we remove this file as its a redundant test for TLS mode?
//All TLS test suites for various TLS modes can be found at vertica-nodejs/test/integration/connection/tls-tests.js


'use strict'

const vertica = require('../../../lib')
const helper = require('../test-helper')

const suite = new helper.Suite()

suite.test('bad tls credentials do not cause crash', (done) => {
/*suite.test('bad tls credentials do not cause crash', (done) => {
const config = {
//tls_client_cert: 'invalid_value',
//tls_client_key: 'invalid_value',
Expand All @@ -20,4 +24,4 @@ suite.test('bad tls credentials do not cause crash', (done) => {
client.end()
done()
})
})
})*/
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ suite.test('ConnectionParameters initializing from config and config.connectionS
})
var subject4 = new ConnectionParameters({
connectionString: 'vertica://test@host/db?tls_mode=require', // connection string has preference
tls_mode: 'disable',
tls_mode: 'require',
})

assert.equal(subject1.tls_mode, 'prefer')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ suite.test('connection string parsing - tls_mode', function () {
assert.equal(subject.tls_mode, 'verify-ca')
})

suite.test('tls mode is disable by default', function () {
suite.test('tls mode is prefer by default', function () {
clearEnv()
var subject = new ConnectionParameters()
assert.equal(subject.tls_mode, 'prefer')
Expand Down

0 comments on commit b300bfb

Please sign in to comment.