Skip to content

Commit

Permalink
server listen is async, so it isn't bound yet. fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Kelly Huntlin committed Feb 11, 2025
1 parent 30c7f76 commit 315e534
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
23 changes: 20 additions & 3 deletions lib/authentication/auth_web.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,24 @@ function AuthWeb(connectionConfig, httpClient, webbrowser) {
body['data']['AUTHENTICATOR'] = 'EXTERNALBROWSER';
};

/**
* net's server.listen is async, so it isn't ready until after the 'listening' event is emitted
* @param {*} server
* @param {*} port
* @param {*} host
* @returns
*/
function listenAsync(server, port, host) {
return new Promise((resolve, reject) => {
// When custom parameters are not provided, the port will be set to 0.
// That means it will use a random port and fallback to localhost
server.listen(port, host);

server.on('listening', () => resolve(server.address()));
server.on('error', reject);
});
}

/**
* Obtain SAML token through SSO URL.
*
Expand All @@ -77,9 +95,8 @@ function AuthWeb(connectionConfig, httpClient, webbrowser) {
});

try {
// When custom parameters are not provided, localServerPort and localServerHost will both be 0
// That means it will use a random port and fallback to localhost
server.listen(URLUtil.parseAddress(samlRedirectUri));
const address = URLUtil.parseAddress(samlRedirectUri);
await listenAsync(server, address.port, address.host);
} catch (err) {
throw new Error(util.format('Error while creating server, samlRedirectUri likely incorrect: %s', err));

Check warning on line 101 in lib/authentication/auth_web.js

View check run for this annotation

Codecov / codecov/patch

lib/authentication/auth_web.js#L101

Added line #L101 was not covered by tests
}
Expand Down
4 changes: 3 additions & 1 deletion test/unit/authentication/authentication_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('external browser authentication', function () {
getAuthenticator: () => credentials.authenticator,
getServiceName: () => '',
getDisableConsoleLogin: () => true,
getSamlRedirectUri: () => 'localhost:0',
getSamlRedirectUri: () => credentials.samlRedirectUri,
host: 'fakehost'
};

Expand Down Expand Up @@ -211,6 +211,7 @@ describe('external browser authentication', function () {
getAuthenticator: () => credentials.authenticator,
getServiceName: () => '',
getDisableConsoleLogin: () => true,
getSamlRedirectUri: () => credentials.samlRedirectUri,
host: 'fakehost'
};

Expand Down Expand Up @@ -688,6 +689,7 @@ describe('okta authentication', function () {
getClientStoreTemporaryCredential: () => true,
getPasscode: () => '',
getPasscodeInPassword: () => false,
getSamlRedirectUri: () => '127.0.0.1:8080',
idToken: idToken || null,
host: 'host',
};
Expand Down
6 changes: 4 additions & 2 deletions test/unit/mock/mock_test_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ const connectionOptionsExternalBrowser =
accessUrl: 'http://fakeaccount.snowflakecomputing.com',
username: 'fakeusername',
account: 'fakeaccount',
authenticator: 'EXTERNALBROWSER'
authenticator: 'EXTERNALBROWSER',
samlRedirectUri: 'localhost:3000'
};

const connectionOptionsidToken =
Expand Down Expand Up @@ -165,7 +166,8 @@ const connectionOptionsOkta =
getTimeout: () => 90,
getRetryTimeout: () => 300,
getRetrySfMaxLoginRetries: () => 7,
getDisableSamlURLCheck: () => false
getDisableSamlURLCheck: () => false,
getSamlRedirectUri: () => 'localhost:3000'
};

exports.connectionOptions =
Expand Down

0 comments on commit 315e534

Please sign in to comment.