From 5d24a7d173529f373377ea3d7643c6dd3a93404b Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Tue, 3 Oct 2023 16:23:34 +0200 Subject: [PATCH 1/3] SNOW-334890: External browser SSO authentication with proxy --- lib/authentication/auth_web.js | 85 ++++--------------- lib/authentication/authentication.js | 26 ++---- lib/connection/connection.js | 4 +- lib/connection/connection_context.js | 4 +- lib/services/sso_url_provider.js | 85 +++++++++++++++++++ .../authentication/authentication_test.js | 59 ++++++------- 6 files changed, 138 insertions(+), 125 deletions(-) create mode 100644 lib/services/sso_url_provider.js diff --git a/lib/authentication/auth_web.js b/lib/authentication/auth_web.js index f86a3cff1..3b822d5a4 100644 --- a/lib/authentication/auth_web.js +++ b/lib/authentication/auth_web.js @@ -3,8 +3,6 @@ */ const util = require('../util'); -const rest = require('../global_config').rest; - const net = require('net'); const querystring = require('querystring'); const URLUtil = require('./../../lib/url_util'); @@ -13,15 +11,17 @@ const Util = require('./../../lib/util'); /** * Creates an external browser authenticator. * - * @param {String} host + * @param {Object} connectionConfig + * @param {Object} ssoUrlProvider * @param {module} webbrowser - * @param {module} httpclient - * @param {module} browserActionTimeout * * @returns {Object} * @constructor */ -function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { +function auth_web(connectionConfig, ssoUrlProvider, webbrowser) { + + const host = connectionConfig.host; + const browserActionTimeout = connectionConfig.getBrowserActionTimeout(); if (!Util.exists(host)) { throw new Error(`Invalid value for host: ${host}`); @@ -31,14 +31,10 @@ function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { } const open = typeof webbrowser !== "undefined" ? webbrowser : require('open'); - const axios = typeof httpclient !== "undefined" ? httpclient : require('axios'); - const browserTimeout = browserActionTimeout - const port = rest.HTTPS_PORT; - const protocol = rest.HTTPS_PROTOCOL; + let ssoURL; let proofKey; let token; - let data; const successResponse = 'HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\nYour identity was confirmed and propagated to Snowflake Node.js driver. You can close this window now and go back where you started from.'; @@ -79,11 +75,14 @@ function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { server.listen(0, 0); // Step 1: query Snowflake to obtain SSO url - const ssoURL = await getSSOURL(authenticator, + const ssoData = await ssoUrlProvider.getSSOURL(authenticator, serviceName, account, server.address().port, - username); + username, + host); + ssoURL = ssoData['ssoUrl']; + proofKey = ssoData['proofKey']; // Step 2: validate URL if (!URLUtil.isValidURL(ssoURL)) { @@ -94,8 +93,8 @@ function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { open(ssoURL); // Step 4: get SAML token - data = await withBrowserActionTimeout(browserActionTimeout, receiveData) - processGet(data); + const tokenData = await withBrowserActionTimeout(browserActionTimeout, receiveData) + processGet(tokenData); }; /** @@ -139,60 +138,6 @@ function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { return server; }; - /** - * Get SSO URL through POST request. - * - * @param {String} authenticator - * @param {String} serviceName - * @param {String} account - * @param {Number} callback_port - * @param {String} user - * - * @returns {String} the SSO URL. - */ - function getSSOURL(authenticator, serviceName, account, callback_port, user) - { - // Create URL to send POST request to - const url = protocol + '://' + host + "/session/authenticator-request"; - - let header; - if (serviceName) - { - header = { - 'HTTP_HEADER_SERVICE_NAME': serviceName - } - } - - // JSON body to send with POST request - const body = { - "data": { - "ACCOUNT_NAME": account, - "LOGIN_NAME": user, - "PORT": port, - "PROTOCOL": protocol, - "AUTHENTICATOR": authenticator, - "BROWSER_MODE_REDIRECT_PORT": callback_port.toString() - } - }; - - // Post request to get the SSO URL - return axios - .post(url, body, { - headers: header - }) - .then((response) => - { - var data = response['data']['data']; - proofKey = data['proofKey']; - - return data['ssoUrl']; - }) - .catch(requestErr => - { - throw requestErr; - }); - }; - /** * Parse the GET request and get token parameter value. * @@ -228,7 +173,7 @@ function auth_web(host, browserActionTimeout, webbrowser, httpclient, ) { const withBrowserActionTimeout = (millis, promise) => { const timeout = new Promise((resolve, reject) => setTimeout( - () => reject(`Browser action timed out after ${browserTimeout} ms.`), + () => reject(`Browser action timed out after ${browserActionTimeout} ms.`), millis)); return Promise.race([ promise, diff --git a/lib/authentication/authentication.js b/lib/authentication/authentication.js index 6972398d3..94ade0831 100644 --- a/lib/authentication/authentication.js +++ b/lib/authentication/authentication.js @@ -67,39 +67,29 @@ exports.formAuthJSON = function formAuthJSON( * * @returns {Object} the authenticator. */ -exports.getAuthenticator = function getAuthenticator(connectionConfig) +exports.getAuthenticator = function getAuthenticator(connectionConfig, ssoUrlProvider) { var auth = connectionConfig.getAuthenticator(); - if (auth == authenticationTypes.DEFAULT_AUTHENTICATOR) - { + if (auth == authenticationTypes.DEFAULT_AUTHENTICATOR) { return new auth_default(connectionConfig.password); + } else if (auth == authenticationTypes.EXTERNAL_BROWSER_AUTHENTICATOR) { + return new auth_web(connectionConfig, ssoUrlProvider); } - else if (auth == authenticationTypes.EXTERNAL_BROWSER_AUTHENTICATOR) - { - return new auth_web(connectionConfig.host, connectionConfig.getBrowserActionTimeout()); - } - if (auth == authenticationTypes.KEY_PAIR_AUTHENTICATOR) - { + if (auth == authenticationTypes.KEY_PAIR_AUTHENTICATOR) { return new auth_keypair(connectionConfig.getPrivateKey(), connectionConfig.getPrivateKeyPath(), connectionConfig.getPrivateKeyPass()); - } - else if (auth == authenticationTypes.OAUTH_AUTHENTICATOR) - { + } else if (auth == authenticationTypes.OAUTH_AUTHENTICATOR) { return new auth_oauth(connectionConfig.getToken()); - } - else if (auth.startsWith('HTTPS://')) - { + } else if (auth.startsWith('HTTPS://')) { return new auth_okta(connectionConfig.password, connectionConfig.region, connectionConfig.account, connectionConfig.getClientType(), connectionConfig.getClientVersion() ); - } - else - { + } else { // Authenticator specified does not exist return new auth_default(connectionConfig.password); } diff --git a/lib/connection/connection.js b/lib/connection/connection.js index 5822b4137..5bda06ed1 100644 --- a/lib/connection/connection.js +++ b/lib/connection/connection.js @@ -236,7 +236,7 @@ function Connection(context) } // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig); + var auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); try { @@ -297,7 +297,7 @@ function Connection(context) var self = this; // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig); + var auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); try { diff --git a/lib/connection/connection_context.js b/lib/connection/connection_context.js index 111118ea1..93c65970f 100644 --- a/lib/connection/connection_context.js +++ b/lib/connection/connection_context.js @@ -6,6 +6,7 @@ var Util = require('../util'); var Errors = require('../errors'); var SfService = require('../services/sf'); var LargeResultSetService = require('../services/large_result_set'); +var SsoUrlProvider = require('../services/sso_url_provider'); /** * Creates a new ConnectionContext. @@ -38,7 +39,8 @@ function ConnectionContext(connectionConfig, httpClient, config) var services = { sf: new SfService(connectionConfig, httpClient, sfServiceConfig), - largeResultSet: new LargeResultSetService(connectionConfig, httpClient) + largeResultSet: new LargeResultSetService(connectionConfig, httpClient), + ssoUrlProvider: new SsoUrlProvider(connectionConfig, httpClient) }; /** diff --git a/lib/services/sso_url_provider.js b/lib/services/sso_url_provider.js new file mode 100644 index 000000000..c249d7b2e --- /dev/null +++ b/lib/services/sso_url_provider.js @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2015-2023 Snowflake Computing Inc. All rights reserved. + */ + +const Util = require('../util'); +const Errors = require('../errors'); +const HttpClient = require("../http/node"); +const axios = require("axios"); +const {rest} = require("../global_config"); + +/** + * Creates a new instance of an LargeResultSetService. + * + * @param {Object} connectionConfig + * @param {Object} httpClient + * @constructor + */ +function SsoUrlProvider(connectionConfig, httpClient) { + // validate input + Errors.assertInternal(Util.isObject(connectionConfig)); + Errors.assertInternal(Util.isObject(httpClient)); + + const port = rest.HTTPS_PORT; + const protocol = rest.HTTPS_PROTOCOL; + + /** + * Get SSO URL through POST request. + * + * @param {String} authenticator + * @param {String} serviceName + * @param {String} account + * @param {Number} callback_port + * @param {String} user + * @param {String} host + * + * @returns {String} the SSO URL. + */ + this.getSSOURL = function (authenticator, serviceName, account, callback_port, user, host) { + // Create URL to send POST request to + const url = protocol + '://' + host + "/session/authenticator-request"; + + let header; + if (serviceName) { + header = { + 'HTTP_HEADER_SERVICE_NAME': serviceName + } + } + const body = { + "data": { + "ACCOUNT_NAME": account, + "LOGIN_NAME": user, + "PORT": port, + "PROTOCOL": protocol, + "AUTHENTICATOR": authenticator, + "BROWSER_MODE_REDIRECT_PORT": callback_port.toString() + } + }; + + const httpsClient = new HttpClient(connectionConfig) + const agent = httpsClient.getAgent(url, connectionConfig.getProxy()); + + const requestOptions = + { + method: 'post', + url: url, + headers: header, + data: body, + requestOCSP: false, + rejectUnauthorized: true, + httpsAgent: agent + }; + + // Post request to get the SSO URL + return axios.request(requestOptions) + .then((response) => { + const data = response['data']['data']; + return data; + }) + .catch(requestErr => { + throw requestErr; + }); + }; +} + +module.exports = SsoUrlProvider; diff --git a/test/unit/authentication/authentication_test.js b/test/unit/authentication/authentication_test.js index c2a1bd3dd..6d91c8218 100644 --- a/test/unit/authentication/authentication_test.js +++ b/test/unit/authentication/authentication_test.js @@ -64,7 +64,6 @@ describe('default authentication', function () describe('external browser authentication', function () { var webbrowser; - var httpclient; var browserRedirectPort; const mockProofKey = 'mockProofKey'; @@ -73,6 +72,10 @@ describe('external browser authentication', function () const credentials = connectionOptionsExternalBrowser; const BROWSER_ACTION_TIMEOUT = 10000; + const connectionConfig= { + getBrowserActionTimeout: () => BROWSER_ACTION_TIMEOUT, + host: 'fakehost' + } before(function () { @@ -86,41 +89,35 @@ describe('external browser authentication', function () return; } }); - mock('httpclient', { - post: async function (url, body, header) - { - var data = - { - data: { - data: - { - ssoUrl: mockSsoURL, - proofKey: mockProofKey - } + mock('ssoUrlProvider', { + getSSOURL: async function (authenticator, serviceName, account, callback_port, user, host) { + const data = + { + ssoUrl: mockSsoURL, + proofKey: mockProofKey } - } - browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; + browserRedirectPort = callback_port.toString(); return data; } }); webbrowser = require('webbrowser'); - httpclient = require('httpclient'); + ssoUrlProvider = require('ssoUrlProvider'); }); it('external browser - authenticate method is thenable', done => { - const auth = new auth_web('', BROWSER_ACTION_TIMEOUT, webbrowser.open, httpclient); + const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); - auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username) + auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host) .then(done) .catch(done); }); it('external browser - get success', async function () { - const auth = new auth_web('', BROWSER_ACTION_TIMEOUT, webbrowser.open, httpclient); - await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username); + const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); + await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host); var body = { data: {} }; auth.updateBody(body); @@ -141,28 +138,22 @@ describe('external browser authentication', function () return; } }); - mock('httpclient', { - post: async function (url, body, header) - { - var data = - { - data: { - data: - { - ssoUrl: mockSsoURL - } + mock('ssoUrlProvider', { + getSSOURL: async function (authenticator, serviceName, account, callback_port, user, host) { + const data = + { + ssoUrl: mockSsoURL } - } - browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; + browserRedirectPort = callback_port.toString(); return data; } }); webbrowser = require('webbrowser'); - httpclient = require('httpclient'); + ssoUrlProvider = require('ssoUrlProvider'); - const auth = new auth_web('', BROWSER_ACTION_TIMEOUT, webbrowser.open, httpclient); - await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username); + const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); + await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host); var body = { data: {} }; auth.updateBody(body); From e2984a5dcbf79259fd2538551964a61487c026d9 Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Wed, 4 Oct 2023 18:03:14 +0200 Subject: [PATCH 2/3] SNOW-930831: Add randomization to the table name in put/get test --- lib/authentication/auth_web.js | 8 +-- lib/authentication/authentication.js | 32 ++++++------ .../sso_url_provider.js | 38 ++++++-------- lib/connection/connection.js | 2 +- lib/connection/connection_context.js | 11 +++- lib/http/base.js | 2 + .../authentication/authentication_test.js | 51 ++++++++++++++----- 7 files changed, 85 insertions(+), 59 deletions(-) rename lib/{services => authentication}/sso_url_provider.js (63%) diff --git a/lib/authentication/auth_web.js b/lib/authentication/auth_web.js index 3b822d5a4..2f383affe 100644 --- a/lib/authentication/auth_web.js +++ b/lib/authentication/auth_web.js @@ -7,6 +7,7 @@ const net = require('net'); const querystring = require('querystring'); const URLUtil = require('./../../lib/url_util'); const Util = require('./../../lib/util'); +var SsoUrlProvider = require('../authentication/sso_url_provider'); /** * Creates an external browser authenticator. @@ -18,10 +19,11 @@ const Util = require('./../../lib/util'); * @returns {Object} * @constructor */ -function auth_web(connectionConfig, ssoUrlProvider, webbrowser) { +function auth_web(connectionConfig, httpClient, webbrowser) { const host = connectionConfig.host; const browserActionTimeout = connectionConfig.getBrowserActionTimeout(); + const ssoUrlProvider = new SsoUrlProvider(connectionConfig, httpClient); if (!Util.exists(host)) { throw new Error(`Invalid value for host: ${host}`); @@ -32,7 +34,6 @@ function auth_web(connectionConfig, ssoUrlProvider, webbrowser) { const open = typeof webbrowser !== "undefined" ? webbrowser : require('open'); - let ssoURL; let proofKey; let token; @@ -81,10 +82,11 @@ function auth_web(connectionConfig, ssoUrlProvider, webbrowser) { server.address().port, username, host); - ssoURL = ssoData['ssoUrl']; + proofKey = ssoData['proofKey']; // Step 2: validate URL + let ssoURL = ssoData['ssoUrl']; if (!URLUtil.isValidURL(ssoURL)) { throw new Error(util.format("Invalid SSO URL found - %s ", ssoURL)); } diff --git a/lib/authentication/authentication.js b/lib/authentication/authentication.js index 94ade0831..9ef098841 100644 --- a/lib/authentication/authentication.js +++ b/lib/authentication/authentication.js @@ -2,13 +2,13 @@ * Copyright (c) 2015-2021 Snowflake Computing Inc. All rights reserved. */ -var auth_default = require('./auth_default'); -var auth_web = require('./auth_web'); -var auth_keypair = require('./auth_keypair'); -var auth_oauth = require('./auth_oauth'); -var auth_okta = require('./auth_okta'); +const auth_default = require('./auth_default'); +const auth_web = require('./auth_web'); +const auth_keypair = require('./auth_keypair'); +const auth_oauth = require('./auth_oauth'); +const auth_okta = require('./auth_okta'); -var authenticationTypes = +const authenticationTypes = { DEFAULT_AUTHENTICATOR: 'SNOWFLAKE', // default authenticator name EXTERNAL_BROWSER_AUTHENTICATOR: 'EXTERNALBROWSER', @@ -37,9 +37,8 @@ exports.formAuthJSON = function formAuthJSON( clientType, clientVersion, clientEnv -) -{ - var body = +) { + const body = { data: { @@ -67,20 +66,19 @@ exports.formAuthJSON = function formAuthJSON( * * @returns {Object} the authenticator. */ -exports.getAuthenticator = function getAuthenticator(connectionConfig, ssoUrlProvider) -{ - var auth = connectionConfig.getAuthenticator(); +exports.getAuthenticator = function getAuthenticator(connectionConfig, httpClient) { + const auth = connectionConfig.getAuthenticator(); - if (auth == authenticationTypes.DEFAULT_AUTHENTICATOR) { + if (auth === authenticationTypes.DEFAULT_AUTHENTICATOR) { return new auth_default(connectionConfig.password); - } else if (auth == authenticationTypes.EXTERNAL_BROWSER_AUTHENTICATOR) { - return new auth_web(connectionConfig, ssoUrlProvider); + } else if (auth === authenticationTypes.EXTERNAL_BROWSER_AUTHENTICATOR) { + return new auth_web(connectionConfig, httpClient); } - if (auth == authenticationTypes.KEY_PAIR_AUTHENTICATOR) { + if (auth === authenticationTypes.KEY_PAIR_AUTHENTICATOR) { return new auth_keypair(connectionConfig.getPrivateKey(), connectionConfig.getPrivateKeyPath(), connectionConfig.getPrivateKeyPass()); - } else if (auth == authenticationTypes.OAUTH_AUTHENTICATOR) { + } else if (auth === authenticationTypes.OAUTH_AUTHENTICATOR) { return new auth_oauth(connectionConfig.getToken()); } else if (auth.startsWith('HTTPS://')) { return new auth_okta(connectionConfig.password, diff --git a/lib/services/sso_url_provider.js b/lib/authentication/sso_url_provider.js similarity index 63% rename from lib/services/sso_url_provider.js rename to lib/authentication/sso_url_provider.js index c249d7b2e..862f52a7b 100644 --- a/lib/services/sso_url_provider.js +++ b/lib/authentication/sso_url_provider.js @@ -4,19 +4,17 @@ const Util = require('../util'); const Errors = require('../errors'); -const HttpClient = require("../http/node"); -const axios = require("axios"); -const {rest} = require("../global_config"); +const { rest } = require('../global_config'); /** - * Creates a new instance of an LargeResultSetService. + * Creates a new instance of an SsoUrlProvider. * * @param {Object} connectionConfig * @param {Object} httpClient * @constructor */ function SsoUrlProvider(connectionConfig, httpClient) { - // validate input + Errors.assertInternal(Util.isObject(connectionConfig)); Errors.assertInternal(Util.isObject(httpClient)); @@ -29,49 +27,45 @@ function SsoUrlProvider(connectionConfig, httpClient) { * @param {String} authenticator * @param {String} serviceName * @param {String} account - * @param {Number} callback_port + * @param {Number} callbackPort * @param {String} user * @param {String} host * * @returns {String} the SSO URL. */ - this.getSSOURL = function (authenticator, serviceName, account, callback_port, user, host) { + this.getSSOURL = function (authenticator, serviceName, account, callbackPort, user, host) { // Create URL to send POST request to - const url = protocol + '://' + host + "/session/authenticator-request"; + const url = protocol + '://' + host + '/session/authenticator-request'; let header; if (serviceName) { header = { 'HTTP_HEADER_SERVICE_NAME': serviceName - } + }; } const body = { - "data": { - "ACCOUNT_NAME": account, - "LOGIN_NAME": user, - "PORT": port, - "PROTOCOL": protocol, - "AUTHENTICATOR": authenticator, - "BROWSER_MODE_REDIRECT_PORT": callback_port.toString() + 'data': { + 'ACCOUNT_NAME': account, + 'LOGIN_NAME': user, + 'PORT': port, + 'PROTOCOL': protocol, + 'AUTHENTICATOR': authenticator, + 'BROWSER_MODE_REDIRECT_PORT': callbackPort.toString() } }; - const httpsClient = new HttpClient(connectionConfig) - const agent = httpsClient.getAgent(url, connectionConfig.getProxy()); + const agent = httpClient.getAgent(url, connectionConfig.getProxy()); const requestOptions = { - method: 'post', - url: url, headers: header, - data: body, requestOCSP: false, rejectUnauthorized: true, httpsAgent: agent }; // Post request to get the SSO URL - return axios.request(requestOptions) + return httpClient.post(url, body, requestOptions) .then((response) => { const data = response['data']['data']; return data; diff --git a/lib/connection/connection.js b/lib/connection/connection.js index 5bda06ed1..3f0733c85 100644 --- a/lib/connection/connection.js +++ b/lib/connection/connection.js @@ -297,7 +297,7 @@ function Connection(context) var self = this; // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); + var auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); try { diff --git a/lib/connection/connection_context.js b/lib/connection/connection_context.js index 93c65970f..e6df0f0d5 100644 --- a/lib/connection/connection_context.js +++ b/lib/connection/connection_context.js @@ -6,7 +6,6 @@ var Util = require('../util'); var Errors = require('../errors'); var SfService = require('../services/sf'); var LargeResultSetService = require('../services/large_result_set'); -var SsoUrlProvider = require('../services/sso_url_provider'); /** * Creates a new ConnectionContext. @@ -40,7 +39,6 @@ function ConnectionContext(connectionConfig, httpClient, config) { sf: new SfService(connectionConfig, httpClient, sfServiceConfig), largeResultSet: new LargeResultSetService(connectionConfig, httpClient), - ssoUrlProvider: new SsoUrlProvider(connectionConfig, httpClient) }; /** @@ -79,6 +77,15 @@ function ConnectionContext(connectionConfig, httpClient, config) } }; }; + /** + * Returns instance of httpClient + * + * @returns {NodeHttpClient} + */ + this.getHttpClient = function () + { + return httpClient; + }; } module.exports = ConnectionContext; \ No newline at end of file diff --git a/lib/http/base.js b/lib/http/base.js index e4c0ebf5d..1793fa01b 100644 --- a/lib/http/base.js +++ b/lib/http/base.js @@ -192,6 +192,8 @@ HttpClient.prototype.getAgent = function (url, proxy, mock) { return null; }; +HttpClient.prototype.post = axios.post; + module.exports = HttpClient; /** diff --git a/test/unit/authentication/authentication_test.js b/test/unit/authentication/authentication_test.js index 6d91c8218..add03a563 100644 --- a/test/unit/authentication/authentication_test.js +++ b/test/unit/authentication/authentication_test.js @@ -13,6 +13,7 @@ var auth_keypair = require('./../../../lib/authentication/auth_keypair'); var auth_oauth = require('./../../../lib/authentication/auth_oauth'); var auth_okta = require('./../../../lib/authentication/auth_okta'); var authenticationTypes = require('./../../../lib/authentication/authentication').authenticationTypes; +const HttpAgent = require('https').Agent; var MockTestUtil = require('./../mock/mock_test_util'); @@ -74,6 +75,7 @@ describe('external browser authentication', function () const BROWSER_ACTION_TIMEOUT = 10000; const connectionConfig= { getBrowserActionTimeout: () => BROWSER_ACTION_TIMEOUT, + getProxy: () => {}, host: 'fakehost' } @@ -89,25 +91,35 @@ describe('external browser authentication', function () return; } }); - mock('ssoUrlProvider', { - getSSOURL: async function (authenticator, serviceName, account, callback_port, user, host) { + mock('httpclient', { + getAgent: function (url, body, header) + { + return new HttpAgent(); + }, + post: async function (url, body, header) + { const data = { - ssoUrl: mockSsoURL, - proofKey: mockProofKey + data: { + data: + { + ssoUrl: mockSsoURL, + proofKey: mockProofKey + } + } } - browserRedirectPort = callback_port.toString(); + browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; return data; } }); webbrowser = require('webbrowser'); - ssoUrlProvider = require('ssoUrlProvider'); + httpclient = require('httpclient'); }); it('external browser - authenticate method is thenable', done => { - const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); + const auth = new auth_web(connectionConfig, httpclient, webbrowser.open); auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host) .then(done) @@ -116,7 +128,7 @@ describe('external browser authentication', function () it('external browser - get success', async function () { - const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); + const auth = new auth_web(connectionConfig, httpclient, webbrowser.open); await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host); var body = { data: {} }; @@ -138,21 +150,32 @@ describe('external browser authentication', function () return; } }); - mock('ssoUrlProvider', { - getSSOURL: async function (authenticator, serviceName, account, callback_port, user, host) { + + mock('httpclient', { + getAgent: function (url, body, header) + { + return new HttpAgent(); + }, + post: async function (url, body, header) + { const data = { - ssoUrl: mockSsoURL + data: { + data: + { + ssoUrl: mockSsoURL + } + } } - browserRedirectPort = callback_port.toString(); + browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; return data; } }); webbrowser = require('webbrowser'); - ssoUrlProvider = require('ssoUrlProvider'); + httpclient = require('httpclient'); - const auth = new auth_web(connectionConfig, ssoUrlProvider, webbrowser.open); + const auth = new auth_web(connectionConfig, httpclient, webbrowser.open); await auth.authenticate(credentials.authenticator, '', credentials.account, credentials.username, credentials.host); var body = { data: {} }; From 53eacf656d6c167f25c272234d6054f75b7ea6fa Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Thu, 5 Oct 2023 14:54:08 +0200 Subject: [PATCH 3/3] SNOW-334890 - refactor of the base httpClient --- lib/authentication/auth_web.js | 4 +- lib/authentication/sso_url_provider.js | 15 +- lib/connection/connection.js | 4 +- lib/connection/connection_context.js | 2 +- lib/http/base.js | 179 ++++++++---------- lib/http/node.js | 14 +- .../ocsp_mock/https_ocsp_mock_agent.js | 1 - .../ocsp_mock/testConnectionOcspMock.js | 11 +- .../authentication/authentication_test.js | 19 +- 9 files changed, 105 insertions(+), 144 deletions(-) diff --git a/lib/authentication/auth_web.js b/lib/authentication/auth_web.js index 2f383affe..a95b3a703 100644 --- a/lib/authentication/auth_web.js +++ b/lib/authentication/auth_web.js @@ -7,7 +7,7 @@ const net = require('net'); const querystring = require('querystring'); const URLUtil = require('./../../lib/url_util'); const Util = require('./../../lib/util'); -var SsoUrlProvider = require('../authentication/sso_url_provider'); +const SsoUrlProvider = require('../authentication/sso_url_provider'); /** * Creates an external browser authenticator. @@ -23,7 +23,7 @@ function auth_web(connectionConfig, httpClient, webbrowser) { const host = connectionConfig.host; const browserActionTimeout = connectionConfig.getBrowserActionTimeout(); - const ssoUrlProvider = new SsoUrlProvider(connectionConfig, httpClient); + const ssoUrlProvider = new SsoUrlProvider(httpClient); if (!Util.exists(host)) { throw new Error(`Invalid value for host: ${host}`); diff --git a/lib/authentication/sso_url_provider.js b/lib/authentication/sso_url_provider.js index 862f52a7b..d84ad242e 100644 --- a/lib/authentication/sso_url_provider.js +++ b/lib/authentication/sso_url_provider.js @@ -9,13 +9,11 @@ const { rest } = require('../global_config'); /** * Creates a new instance of an SsoUrlProvider. * - * @param {Object} connectionConfig * @param {Object} httpClient * @constructor */ -function SsoUrlProvider(connectionConfig, httpClient) { +function SsoUrlProvider(httpClient) { - Errors.assertInternal(Util.isObject(connectionConfig)); Errors.assertInternal(Util.isObject(httpClient)); const port = rest.HTTPS_PORT; @@ -54,18 +52,17 @@ function SsoUrlProvider(connectionConfig, httpClient) { } }; - const agent = httpClient.getAgent(url, connectionConfig.getProxy()); - const requestOptions = { + method: 'post', + url: url, headers: header, - requestOCSP: false, - rejectUnauthorized: true, - httpsAgent: agent + data: body, + responseType: 'json' }; // Post request to get the SSO URL - return httpClient.post(url, body, requestOptions) + return httpClient.requestAsync(requestOptions) .then((response) => { const data = response['data']['data']; return data; diff --git a/lib/connection/connection.js b/lib/connection/connection.js index 3f0733c85..0ce426cce 100644 --- a/lib/connection/connection.js +++ b/lib/connection/connection.js @@ -236,7 +236,7 @@ function Connection(context) } // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); + const auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); try { @@ -297,7 +297,7 @@ function Connection(context) var self = this; // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); + const auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); try { diff --git a/lib/connection/connection_context.js b/lib/connection/connection_context.js index e6df0f0d5..35d44ab37 100644 --- a/lib/connection/connection_context.js +++ b/lib/connection/connection_context.js @@ -38,7 +38,7 @@ function ConnectionContext(connectionConfig, httpClient, config) var services = { sf: new SfService(connectionConfig, httpClient, sfServiceConfig), - largeResultSet: new LargeResultSetService(connectionConfig, httpClient), + largeResultSet: new LargeResultSetService(connectionConfig, httpClient) }; /** diff --git a/lib/http/base.js b/lib/http/base.js index 1793fa01b..2d92f9650 100644 --- a/lib/http/base.js +++ b/lib/http/base.js @@ -7,6 +7,7 @@ const Util = require('../util'); const Errors = require('../errors'); const Logger = require('../logger'); const axios = require('axios'); +const URL = require('node:url').URL; const DEFAULT_REQUEST_TIMEOUT = 360000; @@ -34,69 +35,9 @@ HttpClient.prototype.getConnectionConfig = function () * * @returns {Object} an object representing the request that was issued. */ -HttpClient.prototype.request = function (options) -{ - // validate input - Errors.assertInternal(Util.isObject(options)); - Errors.assertInternal(Util.isString(options.method)); - Errors.assertInternal(Util.isString(options.url)); - Errors.assertInternal(!Util.exists(options.headers) || - Util.isObject(options.headers)); - Errors.assertInternal(!Util.exists(options.json) || - Util.isObject(options.json)); - Errors.assertInternal(!Util.exists(options.callback) || - Util.isFunction(options.callback)); - - var headers; - var json; - var body; - var request; - - // normalize the headers - headers = normalizeHeaders(options.headers); - - // create a function to send the request - var sendRequest = async function sendRequest() - { - const url = options.url; - - const timeout = options.timeout || - this._connectionConfig.getTimeout() || - DEFAULT_REQUEST_TIMEOUT; - - Logger.getInstance().trace(`CALL ${options.method} with timeout ${timeout}: ${url}`); - // build the basic request options - - var requestOptions = - { - method: options.method, - url: url, - headers: headers, - gzip: options.gzip, - json: json, - body: body, - timeout: timeout, - requestOCSP: true, - rejectUnauthorized: true, - // we manually parse jsons or other structures from the server so they need to be text - responseType: 'text', - }; - - let mock; - if (this._connectionConfig.agentClass) - { - mock = { - agentClass: this._connectionConfig.agentClass - } - } - - // add the agent and proxy options - const agent = this.getAgent(url, this._connectionConfig.getProxy(), mock); - - requestOptions.data = requestOptions.body; - requestOptions.httpsAgent = agent; - requestOptions.retryDelay = this.constructExponentialBackoffStrategy(); - +HttpClient.prototype.request = function (options) { + const requestOptions = prepareRequestOptions.call(this, options); + let sendRequest = async function sendRequest() { request = axios.request(requestOptions).then(response => { if (Util.isFunction(options.callback)) { return options.callback(null, normalizeResponse(response), response.data); @@ -119,55 +60,34 @@ HttpClient.prototype.request = function (options) }; sendRequest = sendRequest.bind(this); - // if a request body is specified and compression is enabled, - // try to compress the request body before sending the request - json = body = options.json; - if (body) - { - var bufferUncompressed = Buffer.from(JSON.stringify(body), 'utf8'); - zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) - { - // if the compression was successful - if (!err) - { - json = undefined; // don't specify the 'json' option - - // use the compressed buffer as the body and - // set the appropriate content encoding - body = bufferCompressed; - headers['Content-Encoding'] = 'gzip'; - } - else - { - Logger.getInstance().warn('Could not compress request body.'); - } - - sendRequest(); - }); - } - else - { - if (body) - { - Logger.getInstance().trace('Request body compression disabled.'); - } - - process.nextTick(sendRequest); - } + Logger.getInstance().trace(`CALL ${requestOptions.method} with timeout ${requestOptions.timeout}: ${requestOptions.url}`); + process.nextTick(sendRequest); // return an externalized request object that only contains // methods we're comfortable exposing to the outside world return { - abort: function () - { - if (request) - { + abort: function () { + if (request) { request.abort(); } } }; }; +/** + * Issues an HTTP request. + * + * @param {Object} options + * + * @returns {Object} an object representing the request that was issued. + */ +HttpClient.prototype.requestAsync = async function (options) +{ + const requestOptions = prepareRequestOptions.call(this, options); + + return axios.request(requestOptions); +}; + /** * @abstract * Returns the module to use when making HTTP requests. Subclasses must override @@ -192,10 +112,61 @@ HttpClient.prototype.getAgent = function (url, proxy, mock) { return null; }; -HttpClient.prototype.post = axios.post; - module.exports = HttpClient; +function prepareRequestOptions(options) { + let headers = normalizeHeaders(options.headers) || {}; + + const timeout = options.timeout || + this._connectionConfig.getTimeout() || + DEFAULT_REQUEST_TIMEOUT; + + let data = options.data || options.json; + + if (data) { + var bufferUncompressed = Buffer.from(JSON.stringify(data), 'utf8'); + zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) { + // if the compression was successful + if (!err) { + data = bufferCompressed; + headers['Content-Encoding'] = 'gzip'; + } else { + Logger.getInstance().warn('Could not compress request data.'); + } + }); + } + let mock; + if (this._connectionConfig.agentClass) { + mock = { + agentClass: this._connectionConfig.agentClass + } + } + const backoffStrategy = this.constructExponentialBackoffStrategy(); + const requestOptions = { + method: options.method, + url: options.url, + headers: headers, + data: data, + timeout: timeout, + requestOCSP: true, + retryDelay: backoffStrategy, + rejectUnauthorized: true, + // we manually parse jsons or other structures from the server so they need to be text + responseType: options.responseType || 'text', + } + + const url = new URL(options.url); + const isHttps = url.protocol === 'https:'; + const agent = this.getAgent(url, this._connectionConfig.getProxy(), mock); + if (isHttps) { + requestOptions.httpsAgent = agent; + } else { + requestOptions.httpAgent = agent; + } + + return requestOptions; +} + /** * Normalizes a request headers object so that we get the same behavior * regardless of whether we're using request.js or browser-request.js. diff --git a/lib/http/node.js b/lib/http/node.js index 57f873d9f..d7cf65cc1 100644 --- a/lib/http/node.js +++ b/lib/http/node.js @@ -2,7 +2,6 @@ * Copyright (c) 2015-2019 Snowflake Computing Inc. All rights reserved. */ -const Url = require('url'); const Util = require('../util'); const Base = require('./base'); const HttpsAgent = require('../agent/https_ocsp_agent'); @@ -10,6 +9,7 @@ const HttpsProxyAgent = require('../agent/https_proxy_ocsp_agent'); const HttpAgent = require('http').Agent; const GlobalConfig = require('../../lib/global_config'); const Logger = require('../logger'); +const Url = require("url"); /** * Returns the delay time calculated by exponential backoff with @@ -94,16 +94,18 @@ function isBypassProxy(proxy, url, bypassProxy) { /** * @inheritDoc */ -NodeHttpClient.prototype.getAgent = function (url, proxy, mock) { +NodeHttpClient.prototype.getAgent = function (parsedUrl, proxy, mock) { const agentOptions = {keepAlive: GlobalConfig.getKeepAlive()}; if (mock) { - return mock.agentClass(agentOptions); + const mockAgent = mock.agentClass(agentOptions); + if (mockAgent.protocol === parsedUrl.protocol) { + return mockAgent; + } } - const parsedUrl = Url.parse(url); - const isHttps = parsedUrl.protocol === 'https:'; - const bypassProxy = isBypassProxy(proxy, url); + const bypassProxy = isBypassProxy(proxy, parsedUrl.href); let agent = {}; + const isHttps = parsedUrl.protocol === 'https:'; if (isHttps) { if (proxy && !bypassProxy) { diff --git a/test/integration/ocsp_mock/https_ocsp_mock_agent.js b/test/integration/ocsp_mock/https_ocsp_mock_agent.js index 2a3593feb..017c8a0aa 100644 --- a/test/integration/ocsp_mock/https_ocsp_mock_agent.js +++ b/test/integration/ocsp_mock/https_ocsp_mock_agent.js @@ -3,7 +3,6 @@ */ const HttpsAgent = require('https').Agent; -const Util = require('../../../lib/util'); const SocketUtil = require('../../../lib/agent/socket_util'); const Errors = require('../../../lib/errors'); const ErrorCodes = Errors.codes; diff --git a/test/integration/ocsp_mock/testConnectionOcspMock.js b/test/integration/ocsp_mock/testConnectionOcspMock.js index d9b78aef2..f48f1e6cf 100644 --- a/test/integration/ocsp_mock/testConnectionOcspMock.js +++ b/test/integration/ocsp_mock/testConnectionOcspMock.js @@ -9,6 +9,7 @@ const connOption = require('../connectionOptions'); const Errors = require('../../../lib/errors'); const ErrorCodes = Errors.codes; const HttpsMockAgent = require('./https_ocsp_mock_agent'); +const Logger = require("../../../lib/logger"); function cloneConnOption(connOption) { @@ -20,9 +21,12 @@ function cloneConnOption(connOption) return ret; } -describe('Connection test with OCSP Mock', function () +describe.only('Connection test with OCSP Mock', function () { const valid = cloneConnOption(connOption.valid); + + console.log(connOption.valid); + const isHttps = valid.accessUrl.startsWith("https"); function connect(errcode, connection, callback) @@ -32,9 +36,8 @@ describe('Connection test with OCSP Mock', function () if (isHttps) { assert.equal(err.cause.code, errcode); - } - else - { + } else { + Logger.getInstance().info('Test can be run only for https protocol'); assert.ok(!err, JSON.stringify(err)); } callback(); diff --git a/test/unit/authentication/authentication_test.js b/test/unit/authentication/authentication_test.js index add03a563..0f74c27d0 100644 --- a/test/unit/authentication/authentication_test.js +++ b/test/unit/authentication/authentication_test.js @@ -13,7 +13,6 @@ var auth_keypair = require('./../../../lib/authentication/auth_keypair'); var auth_oauth = require('./../../../lib/authentication/auth_oauth'); var auth_okta = require('./../../../lib/authentication/auth_okta'); var authenticationTypes = require('./../../../lib/authentication/authentication').authenticationTypes; -const HttpAgent = require('https').Agent; var MockTestUtil = require('./../mock/mock_test_util'); @@ -92,12 +91,7 @@ describe('external browser authentication', function () } }); mock('httpclient', { - getAgent: function (url, body, header) - { - return new HttpAgent(); - }, - post: async function (url, body, header) - { + requestAsync: async function (options) { const data = { data: { @@ -108,7 +102,7 @@ describe('external browser authentication', function () } } } - browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; + browserRedirectPort = options.data['data']['BROWSER_MODE_REDIRECT_PORT']; return data; } }); @@ -152,12 +146,7 @@ describe('external browser authentication', function () }); mock('httpclient', { - getAgent: function (url, body, header) - { - return new HttpAgent(); - }, - post: async function (url, body, header) - { + requestAsync: async function (options) { const data = { data: { @@ -167,7 +156,7 @@ describe('external browser authentication', function () } } } - browserRedirectPort = body['data']['BROWSER_MODE_REDIRECT_PORT']; + browserRedirectPort = options.data['data']['BROWSER_MODE_REDIRECT_PORT']; return data; } });