From 016bddc7e3adec51550d366b479088048a3be6f9 Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Fri, 22 Nov 2024 14:45:23 +0100 Subject: [PATCH 1/8] execute automated external browser tests --- .../parameters_aws_auth_tests.json.gpg | 3 + .gitignore | 2 +- Jenkinsfile | 53 +++++--- ci/container/test_authentication.sh | 8 ++ ci/test_authentication.sh | 15 +++ package.json | 1 + test/authentication/connectionParameters.js | 30 +++++ test/authentication/testExternalBrowser.js | 116 ++++++++++++++++++ test/integration/testManualConnection.js | 65 ---------- 9 files changed, 210 insertions(+), 83 deletions(-) create mode 100644 .github/workflows/parameters_aws_auth_tests.json.gpg create mode 100755 ci/container/test_authentication.sh create mode 100755 ci/test_authentication.sh create mode 100644 test/authentication/connectionParameters.js create mode 100644 test/authentication/testExternalBrowser.js diff --git a/.github/workflows/parameters_aws_auth_tests.json.gpg b/.github/workflows/parameters_aws_auth_tests.json.gpg new file mode 100644 index 000000000..6143d3b24 --- /dev/null +++ b/.github/workflows/parameters_aws_auth_tests.json.gpg @@ -0,0 +1,3 @@ +  Ptn4"Kx{3qr*V}h IV_F-я[qx/,0Njp:k~0;0fpttQF)qv5s٨>P~TyG;Jbo/S uNanlNtGz-T { + before(async () => { + await cleanBrowserProcesses(); + }); + + afterEach(async () => { + await cleanBrowserProcesses(); + }); + + it('Successful connection', async () => { + const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; + const connection = await snowflake.createConnection(connectionOption); + const connectAsyncPromise = connection.connectAsync(function (err, connection) { + connectionHandler(err, connection); + }); + const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); + await Promise.all([connectAsyncPromise, provideCredentialsPromise]); + }); + + it('Wrong credentials', async () => { + const login = 'itsnotanaccount.com'; + const password = 'fakepassword'; + const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; + const connection = await snowflake.createConnection(connectionOption); + + connection.connectAsync(function (err, connection) { + connectionHandler(err, connection); + }); + const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'fail', login, password]); + await Promise.all([provideCredentialsPromise]); + + }); + + it('External browser timeout', async () => { + const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 100, clientStoreTemporaryCredential: false }; + const connection = await snowflake.createConnection(connectionOption); + + const connectAsyncPromise = connection.connectAsync(function (err, connection) { + timeoutConnectionHandler(err, connection); + }); + + const connectToBrowserPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'timeout']); + await Promise.all([connectAsyncPromise, connectToBrowserPromise]); + }); + }); +}); + +async function timeoutConnectionHandler(err, timeout) { + try { + assert.ok(err, `Browser action timed out after ${timeout} ms.`); + } catch (err){ + await assert.fail(err); + } +} + +async function cleanBrowserProcesses() { + exec('pkill -f chromium'); + exec('pkill -f xdg-open'); +} + +function connectionHandler(err, connection) { + assert.ok(connection.isUp(), 'Connection is not active'); + testUtil.destroyConnection(connection, function () { + try { + assert.ok(!connection.isUp(), 'Connection is not closed'); + } catch (err) { + assert.fail(err); + } + }); +} + +function execWithTimeout(command, args, timeout = 5000) { + return new Promise((resolve, reject) => { + const child = spawn(command, args, { shell: true }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data; + }); + + child.stderr.on('data', (data) => { + stderr += data; + }); + + child.on('error', (err) => { + reject(err); + }); + + child.on('close', (code) => { + if (code !== 0) { + reject(new Error(`Provide browser credentials process exited with code: ${code}, error: ${stderr}`)); + } else { + resolve({ stdout, stderr }); + } + }); + + setTimeout(() => { + child.kill(); + reject(new Error('Provide browser credentials process timed out')); + }, timeout); + }); +} diff --git a/test/integration/testManualConnection.js b/test/integration/testManualConnection.js index ff375850f..a262ef738 100644 --- a/test/integration/testManualConnection.js +++ b/test/integration/testManualConnection.js @@ -13,71 +13,6 @@ const JsonCredentialManager = require('../../lib/authentication/secure_storage/j if (process.env.RUN_MANUAL_TESTS_ONLY === 'true') { describe('Run manual tests', function () { - describe('Connection test - external browser', function () { - it('Simple Connect', function (done) { - const connection = snowflake.createConnection( - connOption.externalBrowser - ); - - connection.connectAsync(function (err, connection) { - try { - assert.ok(connection.isUp(), 'not active'); - testUtil.destroyConnection(connection, function () { - try { - assert.ok(!connection.isUp(), 'not active'); - done(); - } catch (err) { - done(err); - } - }); - } catch (err) { - done(err); - } - }); - }); - - it('Connect - external browser timeout', function (done) { - const connection = snowflake.createConnection( - connOption.externalBrowserWithShortTimeout - ); - - connection.connectAsync(function (err) { - try { - const browserActionTimeout = - connOption.externalBrowserWithShortTimeout.browserActionTimeout; - assert.ok( - err, - `Browser action timed out after ${browserActionTimeout} ms.` - ); - done(); - } catch (err) { - done(err); - } - }); - }); - - it('Mismatched Username', function (done) { - const connection = snowflake.createConnection( - connOption.externalBrowserMismatchUser - ); - connection.connectAsync(function (err) { - try { - assert.ok( - err, - 'Logged in with different user than one on connection string' - ); - assert.equal( - 'The user you were trying to authenticate as differs from the user currently logged in at the IDP.', - err['message'] - ); - done(); - } catch (err) { - done(err); - } - }); - }); - }); - describe('Connection - ID Token authenticator', function () { const connectionOption = { ...connOption.externalBrowser, clientStoreTemporaryCredential: true }; const key = Util.buildCredentialCacheKey(connectionOption.host, connectionOption.username, 'ID_TOKEN'); From b93e5159201112b243db28a1f386fdbbab751bc0 Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Fri, 22 Nov 2024 16:28:50 +0100 Subject: [PATCH 2/8] fix tests regex --- ci/container/test_component.sh | 2 +- ci/test_windows.bat | 2 +- package.json | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ci/container/test_component.sh b/ci/container/test_component.sh index d52a56b79..0ffdf5022 100755 --- a/ci/container/test_component.sh +++ b/ci/container/test_component.sh @@ -110,7 +110,7 @@ if [[ -z "$GITHUB_ACTIONS" ]]; then fi echo "[INFO] Running Tests: Test result: $WORKSPACE/junit.xml" -if ! ${MOCHA_CMD[@]} "$SOURCE_ROOT/test/**/*.js"; then +if ! ${MOCHA_CMD[@]} 'test/{unit,integration}/**/*.js'; then echo "[ERROR] Test failed" [[ -f "$WORKSPACE/junit.xml" ]] && cat $WORKSPACE/junit.xml exit 1 diff --git a/ci/test_windows.bat b/ci/test_windows.bat index c6764a8ca..fd600ccd4 100644 --- a/ci/test_windows.bat +++ b/ci/test_windows.bat @@ -73,7 +73,7 @@ start /b python hang_webserver.py 12345 > hang_webserver.out 2>&1 popd echo [INFO] Testing -cmd /c node_modules\.bin\mocha --timeout %TIMEOUT% --recursive --full-trace --color --reporter spec test/**/*.js +cmd /c node_modules\.bin\mocha --timeout %TIMEOUT% --recursive --full-trace --color --reporter spec \"test/{unit,integration}/**/*.js\" if %ERRORLEVEL% NEQ 0 ( echo [ERROR] failed to run mocha exit /b 1 diff --git a/package.json b/package.json index 96f8984b1..20e7df34d 100644 --- a/package.json +++ b/package.json @@ -67,9 +67,9 @@ "test:system": "mocha -timeout 180000 --recursive --full-trace system_test/*.js", "test:unit": "mocha -timeout 180000 --recursive --full-trace test/unit/**/*.js test/unit/*.js", "test:unit:coverage": "nyc npm run test:unit", - "test:ci": "mocha -timeout 180000 --recursive --full-trace test/**/*.js", + "test:ci": "mocha -timeout 180000 --recursive --full-trace 'test/{unit,integration}/**/*.js'", "test:ci:coverage": "nyc npm run test:ci", - "test:ci:withSystemTests": "mocha -timeout 180000 --recursive --full-trace test/**/*.js system_test/*.js", + "test:ci:withSystemTests": "mocha -timeout 180000 --recursive --full-trace 'test/{unit,integration}/**/*.js' system_test/*.js", "test:ci:withSystemTests:coverage": "nyc npm run test:ci:withSystemTests", "test:manual": "mocha -timeout 180000 --full-trace --full-trace test/integration/testManualConnection.js" }, From d038a309d4c6e9056fd33bb57c84a62fefa54f74 Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Mon, 25 Nov 2024 13:19:45 +0100 Subject: [PATCH 3/8] cleanup --- Jenkinsfile | 1 - ci/test_authentication.sh | 1 - 2 files changed, 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 35f777db0..f293e9477 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -4,7 +4,6 @@ import groovy.json.JsonOutput timestamps { node('regular-memory-node') { stage('checkout') { - cleanWs() scmInfo = checkout scm println("${scmInfo}") env.GIT_BRANCH = scmInfo.GIT_BRANCH diff --git a/ci/test_authentication.sh b/ci/test_authentication.sh index 922bd9f45..19aa2c210 100755 --- a/ci/test_authentication.sh +++ b/ci/test_authentication.sh @@ -6,7 +6,6 @@ export WORKSPACE=${WORKSPACE:-/tmp} gpg --quiet --batch --yes --decrypt --passphrase="$PARAMETERS_SECRET" --output $THIS_DIR/../.github/workflows/parameters_aws_auth_tests.json "$THIS_DIR/../.github/workflows/parameters_aws_auth_tests.json.gpg" -docker rmi -f nexus.int.snowflakecomputing.com:8086/docker/snowdrivers-test-external-browser:1 docker run \ -v $(cd $THIS_DIR/.. && pwd):/mnt/host \ -v $WORKSPACE:/mnt/workspace \ From 0e979ef7d24d3bfa9d6fb8f464d39fa762dd612f Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Mon, 25 Nov 2024 17:44:13 +0100 Subject: [PATCH 4/8] fix assertions --- test/authentication/testExternalBrowser.js | 63 ++++++++++------------ 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/test/authentication/testExternalBrowser.js b/test/authentication/testExternalBrowser.js index f8546971a..2805147a8 100644 --- a/test/authentication/testExternalBrowser.js +++ b/test/authentication/testExternalBrowser.js @@ -8,7 +8,7 @@ describe('External browser authentication tests', function () { const provideBrowserCredentialsPath = '/externalbrowser/provideBrowserCredentials.js'; const login = connParameters.snowflakeTestBrowserUser; const password = connParameters.snowflakeAuthTestOktaPass; - + let connection; describe('External browser tests', async () => { before(async () => { @@ -17,68 +17,59 @@ describe('External browser authentication tests', function () { afterEach(async () => { await cleanBrowserProcesses(); + if (connection !== undefined && connection.isUp()) { + await testUtil.destroyConnectionAsync(connection); + } }); it('Successful connection', async () => { const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; - const connection = await snowflake.createConnection(connectionOption); - const connectAsyncPromise = connection.connectAsync(function (err, connection) { - connectionHandler(err, connection); - }); + connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.all([connectAsyncPromise, provideCredentialsPromise]); + await Promise.allSettled([connection.connectAsync(function () {}), provideCredentialsPromise]); + await verifyConnectionIsUp(connection); }); it('Wrong credentials', async () => { const login = 'itsnotanaccount.com'; const password = 'fakepassword'; - const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; - const connection = await snowflake.createConnection(connectionOption); + const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 5000, clientStoreTemporaryCredential: false }; + connection = await snowflake.createConnection(connectionOption); - connection.connectAsync(function (err, connection) { - connectionHandler(err, connection); - }); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'fail', login, password]); - await Promise.all([provideCredentialsPromise]); - + await Promise.allSettled([connection.connectAsync(function () {}), provideCredentialsPromise]); + await verifyConnectionIsNotUp(connection); }); it('External browser timeout', async () => { const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 100, clientStoreTemporaryCredential: false }; - const connection = await snowflake.createConnection(connectionOption); - - const connectAsyncPromise = connection.connectAsync(function (err, connection) { - timeoutConnectionHandler(err, connection); - }); + connection = await snowflake.createConnection(connectionOption); const connectToBrowserPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'timeout']); - await Promise.all([connectAsyncPromise, connectToBrowserPromise]); + await Promise.allSettled([connection.connectAsync(function () {}), connectToBrowserPromise]); + await verifyConnectionIsNotUp(connection); }); }); }); -async function timeoutConnectionHandler(err, timeout) { - try { - assert.ok(err, `Browser action timed out after ${timeout} ms.`); - } catch (err){ - await assert.fail(err); - } -} - async function cleanBrowserProcesses() { exec('pkill -f chromium'); exec('pkill -f xdg-open'); } -function connectionHandler(err, connection) { - assert.ok(connection.isUp(), 'Connection is not active'); - testUtil.destroyConnection(connection, function () { - try { - assert.ok(!connection.isUp(), 'Connection is not closed'); - } catch (err) { - assert.fail(err); - } - }); +async function verifyConnectionIsUp(connection) { + assert.ok(await connection.isValidAsync()); + await testUtil.executeCmdAsync(connection, 'Select 1'); +} + +async function verifyConnectionIsNotUp(connection) { + try { + assert(!(await connection.isValidAsync())); + await testUtil.executeCmdAsync(connection, 'Select 1'); + assert.fail('Expected error was not thrown'); + } catch (error) { + assert.strictEqual(error.message, 'Unable to perform operation because a connection was never established.'); + } } function execWithTimeout(command, args, timeout = 5000) { From 68b3030ddef50e609a7c424817a873772165b293 Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Tue, 26 Nov 2024 15:34:59 +0100 Subject: [PATCH 5/8] add tests for token authentication, more detailed assertions --- test/authentication/testExternalBrowser.js | 121 +++++++++++++++++---- test/integration/connectionOptions.js | 30 ----- test/integration/testManualConnection.js | 102 ----------------- 3 files changed, 102 insertions(+), 151 deletions(-) diff --git a/test/authentication/testExternalBrowser.js b/test/authentication/testExternalBrowser.js index 2805147a8..ec5ac7363 100644 --- a/test/authentication/testExternalBrowser.js +++ b/test/authentication/testExternalBrowser.js @@ -3,41 +3,54 @@ const assert = require('assert'); const testUtil = require('../integration/testUtil'); const connParameters = require('./connectionParameters'); const { exec, spawn } = require('child_process'); +const Util = require('../../lib/util'); +const JsonCredentialManager = require('../../lib/authentication/secure_storage/json_credential_manager'); describe('External browser authentication tests', function () { const provideBrowserCredentialsPath = '/externalbrowser/provideBrowserCredentials.js'; const login = connParameters.snowflakeTestBrowserUser; const password = connParameters.snowflakeAuthTestOktaPass; - let connection; + let connection, error; - describe('External browser tests', async () => { - before(async () => { - await cleanBrowserProcesses(); - }); + before(async () => { + await cleanBrowserProcesses(); + }); - afterEach(async () => { - await cleanBrowserProcesses(); - if (connection !== undefined && connection.isUp()) { - await testUtil.destroyConnectionAsync(connection); - } - }); + afterEach(async () => { + await cleanBrowserProcesses(); + await destroyConnection(connection); + error = undefined; + }); + describe('External browser tests', async () => { it('Successful connection', async () => { const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.allSettled([connection.connectAsync(function () {}), provideCredentialsPromise]); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + verifyNoErrorWasThrown(); await verifyConnectionIsUp(connection); }); + it('Mismatched Username', async () => { + const connectionOption = { ...connParameters.externalBrowser, username: 'differentUsername', clientStoreTemporaryCredential: false }; + connection = await snowflake.createConnection(connectionOption); + const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + assert.strictEqual(error?.message, 'The user you were trying to authenticate as differs from the user currently logged in at the IDP.'); + await verifyConnectionIsNotUp(connection, 'Unable to perform operation using terminated connection.'); + }); + it('Wrong credentials', async () => { const login = 'itsnotanaccount.com'; const password = 'fakepassword'; - const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 5000, clientStoreTemporaryCredential: false }; + const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 10000, clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'fail', login, password]); - await Promise.allSettled([connection.connectAsync(function () {}), provideCredentialsPromise]); + const results = await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + assert.strictEqual(results[1].status, 'fulfilled'); + assert.strictEqual(error?.message, 'Error while getting SAML token: Browser action timed out after 10000 ms.'); await verifyConnectionIsNotUp(connection); }); @@ -46,10 +59,72 @@ describe('External browser authentication tests', function () { connection = await snowflake.createConnection(connectionOption); const connectToBrowserPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'timeout']); - await Promise.allSettled([connection.connectAsync(function () {}), connectToBrowserPromise]); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), connectToBrowserPromise]); + assert.strictEqual(error?.message, 'Error while getting SAML token: Browser action timed out after 100 ms.'); await verifyConnectionIsNotUp(connection); }); }); + + describe('ID Token authentication tests', async () => { + const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: true }; + const key = Util.buildCredentialCacheKey(connectionOption.host, connectionOption.username, 'ID_TOKEN'); + const defaultCredentialManager = new JsonCredentialManager(); + let firstIdToken; + + before(async () => { + await defaultCredentialManager.remove(key); + await cleanBrowserProcesses(); + }); + + afterEach(async () => { + await cleanBrowserProcesses(); + await destroyConnection(connection); + }); + + it('obtains the id token from the server and saves it on the local storage', async function () { + connection = snowflake.createConnection(connectionOption); + const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + verifyNoErrorWasThrown(); + await verifyConnectionIsUp(connection); + }); + + it('the token is saved in the credential manager', async function () { + firstIdToken = await defaultCredentialManager.read(key); + assert.notStrictEqual(firstIdToken, null); + }); + + it('authenticates by token, browser credentials not needed', async function () { + connection = snowflake.createConnection(connectionOption); + await connection.connectAsync(connectAsyncCallback()); + verifyNoErrorWasThrown(); + await verifyConnectionIsUp(connection); + }); + + it('opens browser okta authentication again when token is incorrect', async function () { + await defaultCredentialManager.write(key, '1234'); + connection = snowflake.createConnection(connectionOption); + const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + verifyNoErrorWasThrown(); + await verifyConnectionIsUp(connection); + }); + + it('refreshes the token for credential cache key', async function () { + const newToken = await defaultCredentialManager.read(key); + assert.notStrictEqual(firstIdToken, newToken); + }); + }); + + function connectAsyncCallback() { + return function (err) { + error = err; + }; + } + + function verifyNoErrorWasThrown() { + assert.equal(error, null); + } }); async function cleanBrowserProcesses() { @@ -58,17 +133,25 @@ async function cleanBrowserProcesses() { } async function verifyConnectionIsUp(connection) { - assert.ok(await connection.isValidAsync()); + assert.ok(connection.isUp(), 'Connection is not up'); + assert.ok(await connection.isValidAsync(), 'Connection is not valid'); await testUtil.executeCmdAsync(connection, 'Select 1'); } -async function verifyConnectionIsNotUp(connection) { +async function verifyConnectionIsNotUp(connection, message = 'Unable to perform operation because a connection was never established.') { + assert.ok(!(connection.isUp()), 'Connection should not be up'); + assert.ok(!(await connection.isValidAsync()), 'Connection should not be valid'); try { - assert(!(await connection.isValidAsync())); await testUtil.executeCmdAsync(connection, 'Select 1'); assert.fail('Expected error was not thrown'); } catch (error) { - assert.strictEqual(error.message, 'Unable to perform operation because a connection was never established.'); + assert.strictEqual(error.message, message); + } +} + +async function destroyConnection(connection) { + if (connection !== undefined && connection.isUp() && connection.isValidAsync()) { + await testUtil.destroyConnectionAsync(connection); } } diff --git a/test/integration/connectionOptions.js b/test/integration/connectionOptions.js index 38c0ee623..28185ff2e 100644 --- a/test/integration/connectionOptions.js +++ b/test/integration/connectionOptions.js @@ -16,7 +16,6 @@ const snowflakeTestRole = process.env.SNOWFLAKE_TEST_ROLE; const snowflakeTestPassword = process.env.SNOWFLAKE_TEST_PASSWORD; const snowflakeTestAdminUser = process.env.SNOWFLAKE_TEST_ADMIN_USER; const snowflakeTestAdminPassword = process.env.SNOWFLAKE_TEST_ADMIN_PASSWORD; -const snowflakeTestBrowserUser = process.env.SNOWFLAKE_TEST_BROWSER_USER; const snowflakeTestPrivateKeyUser = process.env.SNOWFLAKE_JWT_TEST_USER; const snowflakeTestPrivateKey = process.env.SNOWFLAKE_TEST_PRIVATE_KEY; const snowflakeTestPrivateKeyPath = process.env.SNOWFLAKE_TEST_PRIVATE_KEY_PATH; @@ -89,32 +88,6 @@ const wrongPwd = account: snowflakeTestAccount }; -const externalBrowser = -{ - accessUrl: accessUrl, - username: snowflakeTestBrowserUser, - account: snowflakeTestAccount, - warehouse: snowflakeTestWarehouse, - database: snowflakeTestDatabase, - schema: snowflakeTestSchema, - role: snowflakeTestRole, - host: snowflakeTestHost, - authenticator: 'EXTERNALBROWSER' -}; - -const externalBrowserWithShortTimeout = { - ...externalBrowser, - browserActionTimeout: 100, -}; - -const externalBrowserMismatchUser = -{ - accessUrl: accessUrl, - username: 'node', - account: snowflakeTestAccount, - authenticator: 'EXTERNALBROWSER' -}; - const keypairPrivateKey = { accessUrl: accessUrl, @@ -233,9 +206,6 @@ exports.wrongUserName = wrongUserName; exports.wrongPwd = wrongPwd; exports.accessUrl = accessUrl; exports.account = snowflakeTestAccount; -exports.externalBrowser = externalBrowser; -exports.externalBrowserWithShortTimeout = externalBrowserWithShortTimeout; -exports.externalBrowserMismatchUser = externalBrowserMismatchUser; exports.keypairPrivateKey = keypairPrivateKey; exports.keypairPathEncrypted = keypairPathEncrypted; exports.keypairPathUnencrypted = keypairPathUnencrypted; diff --git a/test/integration/testManualConnection.js b/test/integration/testManualConnection.js index a262ef738..b3b93e4d2 100644 --- a/test/integration/testManualConnection.js +++ b/test/integration/testManualConnection.js @@ -13,78 +13,6 @@ const JsonCredentialManager = require('../../lib/authentication/secure_storage/j if (process.env.RUN_MANUAL_TESTS_ONLY === 'true') { describe('Run manual tests', function () { - describe('Connection - ID Token authenticator', function () { - const connectionOption = { ...connOption.externalBrowser, clientStoreTemporaryCredential: true }; - const key = Util.buildCredentialCacheKey(connectionOption.host, connectionOption.username, 'ID_TOKEN'); - const defaultCredentialManager = new JsonCredentialManager(); - let oldToken; - before( async () => { - await defaultCredentialManager.remove(key); - }); - - it('test - obtain the id token from the server and save it on the local storage', function (done) { - const connection = snowflake.createConnection(connectionOption); - connection.connectAsync(function (err) { - try { - assert.ok(!err); - done(); - } catch (err){ - done(err); - } - }); - }); - - it('test - the token is saved in the credential manager correctly', function (done) { - defaultCredentialManager.read(key).then((idToken) => { - try { - oldToken = idToken; - assert.notStrictEqual(idToken, null); - done(); - } catch (err){ - done(err); - } - }); - }); - - - // Web Browser should not be open. - it('test - id token authentication', function (done) { - const idTokenConnection = snowflake.createConnection(connectionOption); - try { - idTokenConnection.connectAsync(function (err) { - assert.ok(!err); - done(); - }); - } catch (err) { - done(err); - } - }); - - // Web Browser should be open. - it('test - id token reauthentication', function (done) { - defaultCredentialManager.write(key, '1234').then(() => { - const wrongTokenConnection = snowflake.createConnection(connectionOption); - wrongTokenConnection.connectAsync(function (err) { - assert.ok(!err); - done(); - }); - }); - }); - - //Compare two idToken. Those two should be different. - it('test - the token is refreshed', function (done) { - oldToken = undefined; - defaultCredentialManager.read(key).then((idToken) => { - try { - assert.notStrictEqual(idToken, oldToken); - done(); - } catch (err) { - done(err); - } - }); - }); - }); - describe('Connection test - oauth', function () { it('Simple Connect', function (done) { const connection = snowflake.createConnection(connOption.oauth); @@ -136,36 +64,6 @@ if (process.env.RUN_MANUAL_TESTS_ONLY === 'true') { }); }); - describe('Connection test - okta', function () { - it('Simple Connect', function (done) { - const connection = snowflake.createConnection(connOption.okta); - - async.series([ - function (callback) { - connection.connectAsync(function (err) { - done(err); - assert.ok(!err, JSON.stringify(err)); - callback(); - }); - }, - function (callback) { - assert.ok(connection.isUp(), 'not active'); - callback(); - }, - function (callback) { - connection.destroy(function (err) { - assert.ok(!err, JSON.stringify(err)); - callback(); - }); - }, - function (callback) { - assert.ok(!connection.isUp(), 'still active'); - callback(); - }, - ]); - }); - }); - describe('Connection - MFA authenticator with DUO', function () { const connectionOption = connOption.MFA; From c5b3f4ab10e9201d0fcedc21874a458588b3a389 Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Wed, 27 Nov 2024 09:40:48 +0100 Subject: [PATCH 6/8] code review changes --- test/authentication/testExternalBrowser.js | 7 ++--- test/integration/testManualConnection.js | 30 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/test/authentication/testExternalBrowser.js b/test/authentication/testExternalBrowser.js index ec5ac7363..253304c5f 100644 --- a/test/authentication/testExternalBrowser.js +++ b/test/authentication/testExternalBrowser.js @@ -48,8 +48,7 @@ describe('External browser authentication tests', function () { connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'fail', login, password]); - const results = await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); - assert.strictEqual(results[1].status, 'fulfilled'); + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); assert.strictEqual(error?.message, 'Error while getting SAML token: Browser action timed out after 10000 ms.'); await verifyConnectionIsNotUp(connection); }); @@ -133,14 +132,12 @@ async function cleanBrowserProcesses() { } async function verifyConnectionIsUp(connection) { - assert.ok(connection.isUp(), 'Connection is not up'); assert.ok(await connection.isValidAsync(), 'Connection is not valid'); await testUtil.executeCmdAsync(connection, 'Select 1'); } async function verifyConnectionIsNotUp(connection, message = 'Unable to perform operation because a connection was never established.') { assert.ok(!(connection.isUp()), 'Connection should not be up'); - assert.ok(!(await connection.isValidAsync()), 'Connection should not be valid'); try { await testUtil.executeCmdAsync(connection, 'Select 1'); assert.fail('Expected error was not thrown'); @@ -150,7 +147,7 @@ async function verifyConnectionIsNotUp(connection, message = 'Unable to perform } async function destroyConnection(connection) { - if (connection !== undefined && connection.isUp() && connection.isValidAsync()) { + if (connection !== undefined && connection.isUp()) { await testUtil.destroyConnectionAsync(connection); } } diff --git a/test/integration/testManualConnection.js b/test/integration/testManualConnection.js index b3b93e4d2..57f148412 100644 --- a/test/integration/testManualConnection.js +++ b/test/integration/testManualConnection.js @@ -64,6 +64,36 @@ if (process.env.RUN_MANUAL_TESTS_ONLY === 'true') { }); }); + describe('Connection test - okta', function () { + it('Simple Connect', function (done) { + const connection = snowflake.createConnection(connOption.okta); + + async.series([ + function (callback) { + connection.connectAsync(function (err) { + done(err); + assert.ok(!err, JSON.stringify(err)); + callback(); + }); + }, + function (callback) { + assert.ok(connection.isUp(), 'not active'); + callback(); + }, + function (callback) { + connection.destroy(function (err) { + assert.ok(!err, JSON.stringify(err)); + callback(); + }); + }, + function (callback) { + assert.ok(!connection.isUp(), 'still active'); + callback(); + }, + ]); + }); + }); + describe('Connection - MFA authenticator with DUO', function () { const connectionOption = connOption.MFA; From 2de8c6e9583d41c9f8afdcd722032c439196415d Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Wed, 27 Nov 2024 13:04:42 +0100 Subject: [PATCH 7/8] move cleaning browser processes to browser docker image --- ci/test_authentication.sh | 2 +- test/authentication/testExternalBrowser.js | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/ci/test_authentication.sh b/ci/test_authentication.sh index 19aa2c210..725520798 100755 --- a/ci/test_authentication.sh +++ b/ci/test_authentication.sh @@ -10,5 +10,5 @@ docker run \ -v $(cd $THIS_DIR/.. && pwd):/mnt/host \ -v $WORKSPACE:/mnt/workspace \ --rm \ - nexus.int.snowflakecomputing.com:8086/docker/snowdrivers-test-external-browser:1 \ + nexus.int.snowflakecomputing.com:8086/docker/snowdrivers-test-external-browser:2 \ "/mnt/host/ci/container/test_authentication.sh" diff --git a/test/authentication/testExternalBrowser.js b/test/authentication/testExternalBrowser.js index 253304c5f..a28dd11bc 100644 --- a/test/authentication/testExternalBrowser.js +++ b/test/authentication/testExternalBrowser.js @@ -2,11 +2,12 @@ const snowflake = require('../../lib/snowflake'); const assert = require('assert'); const testUtil = require('../integration/testUtil'); const connParameters = require('./connectionParameters'); -const { exec, spawn } = require('child_process'); +const { spawn } = require('child_process'); const Util = require('../../lib/util'); const JsonCredentialManager = require('../../lib/authentication/secure_storage/json_credential_manager'); describe('External browser authentication tests', function () { + const cleanBrowserProcessesPath = '/externalbrowser/cleanBrowserProcesses.js'; const provideBrowserCredentialsPath = '/externalbrowser/provideBrowserCredentials.js'; const login = connParameters.snowflakeTestBrowserUser; const password = connParameters.snowflakeAuthTestOktaPass; @@ -72,12 +73,6 @@ describe('External browser authentication tests', function () { before(async () => { await defaultCredentialManager.remove(key); - await cleanBrowserProcesses(); - }); - - afterEach(async () => { - await cleanBrowserProcesses(); - await destroyConnection(connection); }); it('obtains the id token from the server and saves it on the local storage', async function () { @@ -124,12 +119,11 @@ describe('External browser authentication tests', function () { function verifyNoErrorWasThrown() { assert.equal(error, null); } -}); -async function cleanBrowserProcesses() { - exec('pkill -f chromium'); - exec('pkill -f xdg-open'); -} + async function cleanBrowserProcesses() { + await execWithTimeout('node', [cleanBrowserProcessesPath], 15000); + } +}); async function verifyConnectionIsUp(connection) { assert.ok(await connection.isValidAsync(), 'Connection is not valid'); @@ -173,7 +167,7 @@ function execWithTimeout(command, args, timeout = 5000) { child.on('close', (code) => { if (code !== 0) { - reject(new Error(`Provide browser credentials process exited with code: ${code}, error: ${stderr}`)); + reject(new Error(`Process exited with code: ${code}, error: ${stderr}`)); } else { resolve({ stdout, stderr }); } @@ -181,7 +175,7 @@ function execWithTimeout(command, args, timeout = 5000) { setTimeout(() => { child.kill(); - reject(new Error('Provide browser credentials process timed out')); + reject(new Error('Process timed out')); }, timeout); }); } From d3c81a2e350763356e3695910c1b9f3a38e1c3be Mon Sep 17 00:00:00 2001 From: Adam Kolodziejczyk Date: Thu, 28 Nov 2024 13:18:38 +0100 Subject: [PATCH 8/8] add possibility to run auth tests manually --- test/authentication/testExternalBrowser.js | 42 ++++++++++++++++------ 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/test/authentication/testExternalBrowser.js b/test/authentication/testExternalBrowser.js index a28dd11bc..3ab3ef75b 100644 --- a/test/authentication/testExternalBrowser.js +++ b/test/authentication/testExternalBrowser.js @@ -11,7 +11,7 @@ describe('External browser authentication tests', function () { const provideBrowserCredentialsPath = '/externalbrowser/provideBrowserCredentials.js'; const login = connParameters.snowflakeTestBrowserUser; const password = connParameters.snowflakeAuthTestOktaPass; - let connection, error; + let connection, error, callbackCompleted; before(async () => { await cleanBrowserProcesses(); @@ -20,6 +20,7 @@ describe('External browser authentication tests', function () { afterEach(async () => { await cleanBrowserProcesses(); await destroyConnection(connection); + callbackCompleted = false; error = undefined; }); @@ -28,7 +29,7 @@ describe('External browser authentication tests', function () { const connectionOption = { ...connParameters.externalBrowser, clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + await connectAndProvideCredentials(connection, provideCredentialsPromise); verifyNoErrorWasThrown(); await verifyConnectionIsUp(connection); }); @@ -37,7 +38,7 @@ describe('External browser authentication tests', function () { const connectionOption = { ...connParameters.externalBrowser, username: 'differentUsername', clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + await connectAndProvideCredentials(connection, provideCredentialsPromise); assert.strictEqual(error?.message, 'The user you were trying to authenticate as differs from the user currently logged in at the IDP.'); await verifyConnectionIsNotUp(connection, 'Unable to perform operation using terminated connection.'); }); @@ -47,9 +48,8 @@ describe('External browser authentication tests', function () { const password = 'fakepassword'; const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 10000, clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); - const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'fail', login, password]); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + await connectAndProvideCredentials(connection, provideCredentialsPromise); assert.strictEqual(error?.message, 'Error while getting SAML token: Browser action timed out after 10000 ms.'); await verifyConnectionIsNotUp(connection); }); @@ -57,9 +57,8 @@ describe('External browser authentication tests', function () { it('External browser timeout', async () => { const connectionOption = { ...connParameters.externalBrowser, browserActionTimeout: 100, clientStoreTemporaryCredential: false }; connection = await snowflake.createConnection(connectionOption); - const connectToBrowserPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'timeout']); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), connectToBrowserPromise]); + await connectAndProvideCredentials(connection, connectToBrowserPromise); assert.strictEqual(error?.message, 'Error while getting SAML token: Browser action timed out after 100 ms.'); await verifyConnectionIsNotUp(connection); }); @@ -78,7 +77,7 @@ describe('External browser authentication tests', function () { it('obtains the id token from the server and saves it on the local storage', async function () { connection = snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + await connectAndProvideCredentials(connection, provideCredentialsPromise); verifyNoErrorWasThrown(); await verifyConnectionIsUp(connection); }); @@ -99,7 +98,7 @@ describe('External browser authentication tests', function () { await defaultCredentialManager.write(key, '1234'); connection = snowflake.createConnection(connectionOption); const provideCredentialsPromise = execWithTimeout('node', [provideBrowserCredentialsPath, 'success', login, password], 15000); - await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + await connectAndProvideCredentials(connection, provideCredentialsPromise); verifyNoErrorWasThrown(); await verifyConnectionIsUp(connection); }); @@ -113,6 +112,7 @@ describe('External browser authentication tests', function () { function connectAsyncCallback() { return function (err) { error = err; + callbackCompleted = true; }; } @@ -121,7 +121,29 @@ describe('External browser authentication tests', function () { } async function cleanBrowserProcesses() { - await execWithTimeout('node', [cleanBrowserProcessesPath], 15000); + if (process.env.RUN_AUTH_TESTS_MANUALLY !== 'true') { + await execWithTimeout('node', [cleanBrowserProcessesPath], 15000); + } + } + + async function connectAndProvideCredentials(connection, provideCredentialsPromise) { + if (process.env.RUN_AUTH_TESTS_MANUALLY === 'true') { + await connection.connectAsync(connectAsyncCallback()); + } else { + await Promise.allSettled([connection.connectAsync(connectAsyncCallback()), provideCredentialsPromise]); + } + await waitForCallbackCompletion(); + } + + async function waitForCallbackCompletion() { + const timeout = Date.now() + 5000; + while (Date.now() < timeout) { + await new Promise(resolve => setTimeout(resolve, 100)); + if (callbackCompleted) { + return; + } + } + throw new Error('Connection callback did not complete'); } });