From 15e641c33a03c0997f5f9daf45fca3915b9507d5 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 17 Sep 2024 10:32:15 -0400
Subject: [PATCH 01/13] changes to tls-mode from default to prefer and update
 test cases

---
 packages/vertica-nodejs/lib/client.js                         | 4 ++--
 packages/vertica-nodejs/lib/connection.js                     | 4 ++--
 packages/vertica-nodejs/lib/defaults.js                       | 2 +-
 .../vertica-nodejs/test/unit/client/configuration-tests.js    | 2 +-
 .../test/unit/connection-parameters/creation-tests.js         | 2 +-
 .../unit/connection-parameters/environment-variable-tests.js  | 4 ++--
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/packages/vertica-nodejs/lib/client.js b/packages/vertica-nodejs/lib/client.js
index b895f902..907cdbd1 100644
--- a/packages/vertica-nodejs/lib/client.js
+++ b/packages/vertica-nodejs/lib/client.js
@@ -81,7 +81,7 @@ class Client extends EventEmitter {
     this.processID = null
     this.secretKey = null
     this.tls_config = this.connectionParameters.tls_config
-    this.tls_mode = this.connectionParameters.tls_mode || 'disable'
+    this.tls_mode = this.connectionParameters.tls_mode || 'prefer'
     this.tls_trusted_certs = this.connectionParameters.tls_trusted_certs
     this._connectionTimeoutMillis = c.connectionTimeoutMillis || 0
     this.workload = this.connectionParameters.workload
@@ -197,7 +197,7 @@ class Client extends EventEmitter {
     // once connection is established send startup message
     con.on('connect', function () {
       // SSLRequest Message
-      if (self.tls_mode !== 'disable' || self.tls_config !== undefined) {
+      if (self.tls_mode !== 'prefer' || self.tls_config !== undefined) {
         con.requestSsl()
       } else {
         con.startup(self.getStartupConf())
diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index 4152bd47..59990257 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -46,7 +46,7 @@ class Connection extends EventEmitter {
     this.tls_config = config.tls_config
 
     if (this.tls_config === undefined) {
-      this.tls_mode = config.tls_mode || 'disable'
+      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_trusted_certs = config.tls_trusted_certs
@@ -90,7 +90,7 @@ class Connection extends EventEmitter {
 
 
     // only try to connect with tls if we are set up to handle it
-    if (self.tls_config === undefined && self.tls_mode === 'disable') {
+    if (self.tls_config === undefined && self.tls_mode === 'prefer') {
       return this.attachListeners(this.stream)
     }
 
diff --git a/packages/vertica-nodejs/lib/defaults.js b/packages/vertica-nodejs/lib/defaults.js
index 63a43998..c5f27f93 100644
--- a/packages/vertica-nodejs/lib/defaults.js
+++ b/packages/vertica-nodejs/lib/defaults.js
@@ -55,7 +55,7 @@ module.exports = {
   // from the pool and destroyed
   idleTimeoutMillis: 30000,
   client_encoding: '',
-  tls_mode: 'disable',
+  tls_mode: 'prefer',
   tls_key_file: undefined,
   tls_cert_file: undefined,
   options: undefined,
diff --git a/packages/vertica-nodejs/test/unit/client/configuration-tests.js b/packages/vertica-nodejs/test/unit/client/configuration-tests.js
index 47eb7487..3796fdb7 100644
--- a/packages/vertica-nodejs/test/unit/client/configuration-tests.js
+++ b/packages/vertica-nodejs/test/unit/client/configuration-tests.js
@@ -12,7 +12,7 @@ test('client settings', function () {
     assert.equal(client.user, pguser)
     assert.equal(client.database, pgdatabase)
     assert.equal(client.port, pgport)
-    assert.equal(client.tls_mode, 'disable')
+    assert.equal(client.tls_mode, 'prefer')
   })
 
   test('custom', function () {
diff --git a/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js b/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
index 51b4b7dd..a6117199 100644
--- a/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
@@ -105,7 +105,7 @@ suite.test('ConnectionParameters initializing from config and config.connectionS
     tls_mode: 'disable',
   })
 
-  assert.equal(subject1.tls_mode, 'disable')
+  assert.equal(subject1.tls_mode, 'prefer')
   assert.equal(subject2.tls_mode, 'require')
   assert.equal(subject3.tls_mode, 'require')
   assert.equal(subject4.tls_mode, 'require')
diff --git a/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js b/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
index 52802a5d..247c0a04 100644
--- a/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
@@ -83,7 +83,7 @@ suite.test('connection string parsing - tls_mode', function () {
 
   string = 'vertica://brian:pw@boom:381/lala'
   subject = new ConnectionParameters(string)
-  assert.equal(subject.tls_mode, 'disable')
+  assert.equal(subject.tls_mode, 'prefer')
 
   string = 'vertica://brian:pw@boom:381/lala?tls_mode=verify-ca'
   subject = new ConnectionParameters(string)
@@ -93,7 +93,7 @@ suite.test('connection string parsing - tls_mode', function () {
 suite.test('tls mode is disable by default', function () {
   clearEnv()
   var subject = new ConnectionParameters()
-  assert.equal(subject.tls_mode, 'disable')
+  assert.equal(subject.tls_mode, 'prefer')
 })
 
 // restore process.env

From 2af744825738e7e4e1df5d726cf4a461c9dfebd9 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 17 Sep 2024 10:47:08 -0400
Subject: [PATCH 02/13] update default in configutaion test

---
 .../test/integration/client/configuration-tests.js              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/vertica-nodejs/test/integration/client/configuration-tests.js b/packages/vertica-nodejs/test/integration/client/configuration-tests.js
index 7b4ae8e9..4d3652ed 100644
--- a/packages/vertica-nodejs/test/integration/client/configuration-tests.js
+++ b/packages/vertica-nodejs/test/integration/client/configuration-tests.js
@@ -24,7 +24,7 @@ suite.test('default values are used in new clients', function () {
     binary: false,
     idleTimeoutMillis: 30000,
     client_encoding: '',
-    tls_mode: 'disable',
+    tls_mode: 'prefer',
     parseInputDatesAsUTC: false,
   })
 

From dc1bded7c41d61c378478d1ff3becf401f4f5c39 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 17 Sep 2024 15:00:52 -0400
Subject: [PATCH 03/13] Added test for prefer tls mode and made it default

---
 .../test/integration/connection/tls-tests.js  | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index d3bbde0d..5d890ac7 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -37,8 +37,8 @@ const client_key_path     =  __dirname + '/../../tls/client_key.pem'
 // all connections from the client, the caveat being that for try_verify and verify_ca it's possible
 // for the connection to be plaintext if the client doesn't present valid credentials. 
 suite.test('vertica tls - disable mode - all', function () {
-  var client = new vertica.Client() // 'disable' by default, so no need to pass in that option
-  assert.equal(client.tls_mode, vertica.defaults.tls_mode)
+  var client = new vertica.Client({tls_mode: 'disable'})
+  assert.equal(client.tls_mode, 'disable')
   client.connect(err => {
     if (err) {
       // shouldn't fail to connect
@@ -58,6 +58,35 @@ suite.test('vertica tls - disable mode - all', function () {
   })
 })
 
+// Test case for tls_mode = 'prefer' as default
+// The client will attempt to establish a TLS connection if the server supports it.
+// If the server does not support TLS, the client will still connect using a plaintext connection.
+// This test verifies that in 'prefer' mode, the client connects successfully.
+suite.test('vertica tls - prefer mode', function () {
+  var client = new vertica.Client() // 'prefer' by default, so no need to pass in that option
+  assert.equal(client.tls_mode, vertica.defaults.tls_mode)
+  client.connect(err => {
+    if (err) {
+      console.log(err)
+      assert(false)
+    }
+    //Verify is client is using a TLS connection
+    client.query("SELECT mode FROM tls_configurations where name = 'server' LIMIT 1", (err, res) => {
+      if (err) {
+        console.log(err)
+        assert(false)
+      }
+      if (['ENABLE', 'TRY_VERIFY', 'VERIFY_CA', 'VERIFY_FULL'].includes(res.rows[0].mode)) {
+        assert.equal(client.connection.stream.constructor.name.toString(), "TLSSocket")
+      }
+      else {
+      assert.notEqual(client.connection.stream.constructor.name.toString(), "TLSSocket")
+      }
+      client.end()
+    })
+  })
+})
+
 // Test case for tls_mode = 'require'
 // The server will not accept all connections from the client with the client in 'require' mode. The server
 // will reject a connection in DISABLE mode for obvious reasons (client requiring TLS + server disallowing TLS)

From b4c002f28f0a6e1162cfd76545c547e09190ec75 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 17 Sep 2024 15:02:16 -0400
Subject: [PATCH 04/13] Fix typo

---
 .../vertica-nodejs/test/integration/connection/tls-tests.js     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index 5d890ac7..cede64b3 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -70,7 +70,7 @@ suite.test('vertica tls - prefer mode', function () {
       console.log(err)
       assert(false)
     }
-    //Verify is client is using a TLS connection
+    //Verify if client is using a TLS connection
     client.query("SELECT mode FROM tls_configurations where name = 'server' LIMIT 1", (err, res) => {
       if (err) {
         console.log(err)

From b300bfbbd08ccf5d0cbb88ed3f19213b70f035a8 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Thu, 19 Sep 2024 14:57:34 -0400
Subject: [PATCH 05/13] Cleanup integration and unit test suites

---
 packages/v-connection-string/index.js         |  6 ++---
 packages/v-connection-string/test/parse.js    |  4 ++--
 packages/vertica-nodejs/lib/connection.js     | 22 +++++++++++++++++--
 .../test/integration/connection/tls-tests.js  | 12 +++++-----
 .../test/integration/gh-issues/2307-tests.js  |  8 +++++--
 .../connection-parameters/creation-tests.js   |  2 +-
 .../environment-variable-tests.js             |  2 +-
 7 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/packages/v-connection-string/index.js b/packages/v-connection-string/index.js
index 42f0b3ce..664a6c78 100644
--- a/packages/v-connection-string/index.js
+++ b/packages/v-connection-string/index.js
@@ -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(':')
diff --git a/packages/v-connection-string/test/parse.js b/packages/v-connection-string/test/parse.js
index d56db656..d76348f8 100644
--- a/packages/v-connection-string/test/parse.js
+++ b/packages/v-connection-string/test/parse.js
@@ -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 () {
diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index 59990257..da4f5dcc 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -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
     }
@@ -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
diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index cede64b3..f611662f 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -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()
     })
diff --git a/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js b/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
index 7adf1ffd..73523286 100644
--- a/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
+++ b/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
@@ -1,3 +1,7 @@
+//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')
@@ -5,7 +9,7 @@ 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',
@@ -20,4 +24,4 @@ suite.test('bad tls credentials do not cause crash', (done) => {
     client.end()
     done()
   })
-})
+})*/
diff --git a/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js b/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
index a6117199..29b1d07f 100644
--- a/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection-parameters/creation-tests.js
@@ -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')
diff --git a/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js b/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
index 247c0a04..bea3b30f 100644
--- a/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection-parameters/environment-variable-tests.js
@@ -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')

From 99eb5431ef280188099f52f85781bfebeb4bf5d9 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Thu, 19 Sep 2024 15:00:25 -0400
Subject: [PATCH 06/13] Update README.md to reflect 'prefer' TLS mode

---
 packages/vertica-nodejs/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/vertica-nodejs/README.md b/packages/vertica-nodejs/README.md
index fd51edd1..aecbeddf 100644
--- a/packages/vertica-nodejs/README.md
+++ b/packages/vertica-nodejs/README.md
@@ -266,11 +266,11 @@ The `client_label` connection property is a string that sets a label for the con
 
 Current TLS Support in vertica-nodejs is limited to server modes that does not require the client to present a certificate. mTLS will be supported in a future version of vertica-nodejs. 
 
-Valid values for the `tls_mode` connection property are `disable`, `require` which will ensure the connection is encrypted, `verify-ca` which ensures the connection is encrypted and the client trusts the server certificate, and `verify-full` which ensures the connection is encrypted, the client trusts the server certificate, and the server hostname has been verified to match the provided server certificate. 
+Valid values for the `tls_mode` connection property are `disable`, `prefer`, `require` which will ensure the connection is encrypted, `verify-ca` which ensures the connection is encrypted and the client trusts the server certificate, and `verify-full` which ensures the connection is encrypted, the client trusts the server certificate, and the server hostname has been verified to match the provided server certificate. 
 
 ### TLS Connection Properties
 
-The `tls_mode` connection property is a string that determines the mode of tls the client will attempt to use. By default it is `disable`. Other valid values are described in the above section.
+The `tls_mode` connection property is a string that determines the mode of tls the client will attempt to use. By default it is `prefer`. Other valid values are described in the above section.
 
 The `tls_trusted_certs` connection property is an optional override of the trusted CA certificates. `tls_trusted_certs` is a path to the .pem file being used to override defaults. The default is based on the node.js tls module which defaults to well-known CAs curated by Mozilla.
 

From 75423040119db824810c1368f9caa4f503552376 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Mon, 23 Sep 2024 17:34:55 -0400
Subject: [PATCH 07/13] restoring previous logic

---
 packages/vertica-nodejs/lib/client.js                           | 2 +-
 packages/vertica-nodejs/lib/connection.js                       | 2 +-
 .../vertica-nodejs/test/integration/connection/tls-tests.js     | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/packages/vertica-nodejs/lib/client.js b/packages/vertica-nodejs/lib/client.js
index 907cdbd1..63d6819f 100644
--- a/packages/vertica-nodejs/lib/client.js
+++ b/packages/vertica-nodejs/lib/client.js
@@ -197,7 +197,7 @@ class Client extends EventEmitter {
     // once connection is established send startup message
     con.on('connect', function () {
       // SSLRequest Message
-      if (self.tls_mode !== 'prefer' || self.tls_config !== undefined) {
+      if (self.tls_mode !== 'disable' || self.tls_config !== undefined) {
         con.requestSsl()
       } else {
         con.startup(self.getStartupConf())
diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index da4f5dcc..f99fe256 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -90,7 +90,7 @@ class Connection extends EventEmitter {
 
 
     // only try to connect with tls if we are set up to handle it
-    if (self.tls_config === undefined && self.tls_mode === 'prefer') {
+    if (self.tls_config === undefined && self.tls_mode === 'disable') {
       return this.attachListeners(this.stream)
     }
 
diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index f611662f..d4dcc0d0 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -75,7 +75,6 @@ suite.test('vertica tls - prefer mode', function () {
     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)) {

From 274a03e3c9d3b886a170137aa45c724f30742e4c Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Thu, 10 Oct 2024 14:57:44 -0400
Subject: [PATCH 08/13] testing

---
 packages/vertica-nodejs/lib/connection.js     | 50 +++++++++++--------
 .../test/integration/connection/tls-tests.js  | 11 ++--
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index f99fe256..9b6a2268 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -100,8 +100,16 @@ class Connection extends EventEmitter {
         case 'S': // Server supports TLS connections, continue with a secure connection
           break
         case 'N': // Server does not support TLS connections
-          self.stream.end()
-          return self.emit('error', new Error('The server does not support TLS connections'))
+          if (self.tls_mode == 'prefer') {
+            self.attachListeners(self.stream)
+            
+            self.emit('sslconnect')
+            return
+          }
+          else {
+            self.stream.end()
+            return self.emit('error', new Error('The server does not support TLS connections'))
+          }
         default:
           // Any other response byte, including 'E' (ErrorResponse) indicating a server error
           self.stream.end()
@@ -128,25 +136,7 @@ class Connection extends EventEmitter {
         // With an undefined checkServerIdentity function, we are still checking to see that the server
         // certificate is signed by the CA (default or provided).
         
-        if (self.tls_mode === 'require') { // 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 === 'prefer') { // basic TLS connection, does not verify CA certificate
+        if (self.tls_mode === 'require' || 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) {
@@ -164,6 +154,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
diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index d4dcc0d0..c74dd827 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -66,15 +66,15 @@ suite.test('vertica tls - prefer mode', function () {
   var client = new vertica.Client() // 'prefer' by default, so no need to pass in that option
   assert.equal(client.tls_mode, vertica.defaults.tls_mode)
   client.connect(err => {
-    if (err) {
-      // shouldn't fail to connect
-      assert(err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
-      return
-    }
+    // if (err) {
+    //   // shouldn't fail to connect
+    //   assert(false)      
+    // }
     // 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)) {
@@ -85,6 +85,7 @@ suite.test('vertica tls - prefer mode', function () {
       }
       client.end()
     })
+    client.end()
   })
 })
 

From b44c59aa9bb07f712cdc480db93e27025db77296 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Thu, 10 Oct 2024 15:52:34 -0400
Subject: [PATCH 09/13] tls-tests passing locally

---
 packages/vertica-nodejs/lib/connection.js     |  2 +-
 .../test/integration/connection/tls-tests.js  | 25 +++++++++++--------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index 9b6a2268..71cb6211 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -19,6 +19,7 @@ var fs = require('fs')
 var EventEmitter = require('events').EventEmitter
 
 const { parse, serialize } = require('v-protocol')
+const { error } = require('console')
 
 const flushBuffer = serialize.flush()
 const syncBuffer = serialize.sync()
@@ -102,7 +103,6 @@ class Connection extends EventEmitter {
         case 'N': // Server does not support TLS connections
           if (self.tls_mode == 'prefer') {
             self.attachListeners(self.stream)
-            
             self.emit('sslconnect')
             return
           }
diff --git a/packages/vertica-nodejs/test/integration/connection/tls-tests.js b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
index c74dd827..3f07b27a 100644
--- a/packages/vertica-nodejs/test/integration/connection/tls-tests.js
+++ b/packages/vertica-nodejs/test/integration/connection/tls-tests.js
@@ -1,4 +1,5 @@
 'use strict'
+const { error } = require('console')
 var helper = require('./../test-helper')
 var vertica = helper.vertica
 
@@ -66,10 +67,10 @@ suite.test('vertica tls - prefer mode', function () {
   var client = new vertica.Client() // 'prefer' by default, so no need to pass in that option
   assert.equal(client.tls_mode, vertica.defaults.tls_mode)
   client.connect(err => {
-    // if (err) {
-    //   // shouldn't fail to connect
-    //   assert(false)      
-    // }
+    if (err) {
+      // shouldn't fail to connect
+      assert(false)      
+    }
     // If connection succeeds, check for TLS connection
     client.query("SELECT mode FROM tls_configurations where name = 'server' LIMIT 1", (err, res) => {
       if (err) {
@@ -85,7 +86,6 @@ suite.test('vertica tls - prefer mode', function () {
       }
       client.end()
     })
-    client.end()
   })
 })
 
@@ -127,7 +127,8 @@ suite.test('vertica tls - verify-ca - no tls_cert_file specified', function () {
     if (err) {
       assert(err.message.includes("verify-ca mode requires setting tls_trusted_certs property") // we didn't set the property, this is ok
           || err.message.includes("SSL alert number 40") // VERIFY_CA mode, this is ok
-          || err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
+          || err.message.includes("The server does not support TLS connections") // DISABLE mode, this is ok
+          || err.message.includes("unable to verify the first certificate"))
     }
     client.end()
   })
@@ -146,7 +147,8 @@ suite.test('vertica tls - verify-ca - valid server certificate', function () {
   client.connect(err => {
     if (err) {
       assert(err.message.includes("SSL alert number 40") // VERIFY_CA mode, this is ok
-          || err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
+          || err.message.includes("The server does not support TLS connections") // DISABLE mode, this is ok
+          || err.message.includes("unable to verify the first certificate"))
       return
     }
     assert.equal(client.connection.stream.constructor.name.toString(), "TLSSocket")
@@ -172,7 +174,8 @@ suite.test('vertica tls - verify-full - no tls_cert_file specified', function ()
     if (err) {
       assert(err.message.includes("verify-ca mode requires setting tls_trusted_certs property") // we didn't set the property, this is ok
           || err.message.includes("SSL alert number 40") // VERIFY_CA mode, this is ok
-          || err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
+          || err.message.includes("The server does not support TLS connections") // DISABLE mode, this is ok
+          || err.message.includes("unable to verify the first certificate"))
     }
     client.end()
   })
@@ -190,7 +193,8 @@ suite.test('vertica tls - verify-full - valid server certificate', function () {
   client.connect(err => {
     if (err) {
       assert(err.message.includes("SSL alert number 40") // VERIFY_CA mode, this is ok
-          || err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
+          || err.message.includes("The server does not support TLS connections") // DISABLE mode, this is ok
+          || err.message.includes("unable to verify the first certificate"))
       return
     }
     assert.equal(client.connection.stream.constructor.name.toString(), "TLSSocket")
@@ -214,7 +218,8 @@ suite.test('vertica tls - tls_config feature', function() {
   client.connect(err => {
     if (err) {
       assert(err.message.includes("SSL alert number 40") // VERIFY_CA mode, this is ok
-          || err.message.includes("The server does not support TLS connections")) // DISABLE mode, this is ok
+          || err.message.includes("The server does not support TLS connections") // DISABLE mode, this is ok
+          || err.message.includes("unable to verify the first certificate"))
       return
     }
     // this is how we can tell we actually used tls_config and created a tls socket

From 5b0077c06d16e2752c12bb2aad44a47def613ff6 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Wed, 16 Oct 2024 15:34:11 -0400
Subject: [PATCH 10/13] update unit test

---
 .../test/unit/connection/inbound-parser-tests.js              | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js b/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
index 3bf52099..07f39a46 100644
--- a/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
@@ -9,6 +9,7 @@ require('./test-helper')
 const BufferList = require('../../buffer-list')
 var Connection = require('../../../lib/connection')
 var buffers = require('../../test-buffers')
+const { tls_mode } = require('../../../lib/defaults')
 var PARSE = function (buffer) {
   return new Parser(buffer).parse()
 }
@@ -121,6 +122,7 @@ var testForMessage = function (buffer, expectedMessage) {
     var stream = new MemoryStream()
     var client = new Connection({
       stream: stream,
+      tls_mode: 'disable'
     })
     client.connect()
 
@@ -379,6 +381,7 @@ test('split buffer, single message parsing', function () {
   var stream = new MemoryStream()
   var client = new Connection({
     stream: stream,
+    tls_mode: 'disable'
   })
   client.connect()
   var message = null
@@ -437,6 +440,7 @@ test('split buffer, multiple message parsing', function () {
   var stream = new MemoryStream()
   var client = new Connection({
     stream: stream,
+    tls_mode: 'disable'
   })
   client.connect()
   client.on('message', function (msg) {

From ec2405487d0cdff546776945e77e03eee787602d Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Wed, 16 Oct 2024 15:58:23 -0400
Subject: [PATCH 11/13] added commented test back

---
 .../vertica-nodejs/test/integration/gh-issues/2307-tests.js   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js b/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
index 73523286..7e5ab309 100644
--- a/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
+++ b/packages/vertica-nodejs/test/integration/gh-issues/2307-tests.js
@@ -9,7 +9,7 @@ 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',
@@ -24,4 +24,4 @@ const suite = new helper.Suite()
     client.end()
     done()
   })
-})*/
+})

From 61de2888b84832f62d0e7defcefff133af2122f6 Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 22 Oct 2024 10:38:02 -0400
Subject: [PATCH 12/13] modify test-helper.js to fix CI workflow

---
 .../vertica-nodejs/test/integration/connection/test-helper.js   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/vertica-nodejs/test/integration/connection/test-helper.js b/packages/vertica-nodejs/test/integration/connection/test-helper.js
index 22bc7a89..030d3f8b 100644
--- a/packages/vertica-nodejs/test/integration/connection/test-helper.js
+++ b/packages/vertica-nodejs/test/integration/connection/test-helper.js
@@ -6,7 +6,7 @@ var utils = require('../../../lib/utils')
 var connect = function (callback) {
   var username = helper.args.user
   var database = helper.args.database
-  var con = new Connection({ stream: new net.Stream() })
+  var con = new Connection({ stream: new net.Stream(), tls_mode: 'disable' })
   con.on('error', function (error) {
     console.log(error)
     throw new Error('Connection error')

From 4f3ec86861c1f082916530b41d4b667c9a16868c Mon Sep 17 00:00:00 2001
From: tanvipise <tanvi.pise@microfocus.com>
Date: Tue, 22 Oct 2024 11:04:20 -0400
Subject: [PATCH 13/13] address PR comments

---
 packages/vertica-nodejs/lib/connection.js     | 22 ++-----------------
 .../unit/connection/inbound-parser-tests.js   |  1 -
 2 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/packages/vertica-nodejs/lib/connection.js b/packages/vertica-nodejs/lib/connection.js
index 71cb6211..6153c23e 100644
--- a/packages/vertica-nodejs/lib/connection.js
+++ b/packages/vertica-nodejs/lib/connection.js
@@ -48,8 +48,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
     }
@@ -154,24 +154,6 @@ 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
diff --git a/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js b/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
index 07f39a46..83402c4e 100644
--- a/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
+++ b/packages/vertica-nodejs/test/unit/connection/inbound-parser-tests.js
@@ -9,7 +9,6 @@ require('./test-helper')
 const BufferList = require('../../buffer-list')
 var Connection = require('../../../lib/connection')
 var buffers = require('../../test-buffers')
-const { tls_mode } = require('../../../lib/defaults')
 var PARSE = function (buffer) {
   return new Parser(buffer).parse()
 }