Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1018408: Unhandled promise rejections on keypair auth #757

Closed
beckjake opened this issue Jan 24, 2024 · 3 comments
Closed

SNOW-1018408: Unhandled promise rejections on keypair auth #757

beckjake opened this issue Jan 24, 2024 · 3 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team

Comments

@beckjake
Copy link
Contributor

beckjake commented Jan 24, 2024

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

  1. What version of NodeJS driver are you using?
    1.9.3

  2. What operating system and processor architecture are you using?

M1 mac (arm64) running Sonoma 14.2.1

  1. What version of NodeJS are you using?
    (node --version and npm --version)
    $ node --version
    v18.11.0
    $ npm --version
    8.19.2

  2. What are the component versions in the environment (npm list)?
    $ npm list
    [email protected] /Users/jake/tmp/snowflake-sdk-repeat-creates
    └── [email protected]

5.Server version:* E.g. 1.90.1
8.3.1

I think the server shouldn't be involved here (it is, but that's because of the bug)

  1. What did you do?

Attempted to connect to snowflake with an unsupported elliptic curve key. Please note that the key material in this paste is NOT sensitive in any way, though I'm sure it will set something off:

const snowflake = require('snowflake-sdk');
const { readFileSync } = require('fs');

// json file with { username, account } parameters
const accountInfo = JSON.parse(readFileSync('./config.json'));

// generated with `openssl ecparam -name prime192v1 -genkey | openssl pkcs8 -topk8 -inform PEM -nocrypt` on my mac
const invalidKey = `-----BEGIN PRIVATE KEY-----
MG8CAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQEEVTBTAgEBBBiewv/L/sz5HfviLo2W
MRuxbx0XVuihzV2hNAMyAAQt34zOKsR/cBWjc4bneTV/4bk5DoRvfh5geE2BKyzD
JlHoE78XxBmrXSf3v7OLvRg=
-----END PRIVATE KEY-----`

const args = {
    authenticator: "SNOWFLAKE_JWT",
    privateKey: invalidKey,
    ...accountInfo
};
const conn = snowflake.createConnection(args);


process.on("unhandledRejection", (reason, source) => {
  console.error("unhandled rejection reason:", reason);
});

conn.connect(err => {
  console.log("connect callback should have an auth error, got:", err);
});
  1. What did you expect to see?

I expect to see exactly one message, something like this (+ stack trace):

connect callback should have an auth error, got: Error: "alg" parameter for "ec" key type must be one of: ES256, ES384, ES512.

Instead, I got 2 messages:

unhandled rejection reason: Error: "alg" parameter for "ec" key type must be one of: ES256, ES384, ES512.
    at module.exports (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/jsonwebtoken/lib/validateAsymmetricKey.js:29:11)
    at module.exports [as sign] (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/jsonwebtoken/sign.js:179:7)
    at AuthKeypair.authenticate (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/authentication/auth_keypair.js:148:20)
    at Connection.connect (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/connection/connection.js:221:10)
    at Object.<anonymous> (/Users/jake/tmp/snowflake-sdk-repeat-creates/recreate.js:31:6)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
connect callback should have an auth error, got: Error [OperationFailedError]: Cannot read field "message" because "response" is null
    at createError (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/errors.js:526:17)
    at exports.createOperationFailedError (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/errors.js:329:10)
    at Object.callback (/Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/services/sf.js:626:28)
    at /Users/jake/tmp/snowflake-sdk-repeat-creates/node_modules/snowflake-sdk/lib/http/base.js:42:24
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: '370001',
  data: {
    cause: null,
    stackTrace: [
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object], [Object], [Object],
      [Object], [Object], [Object]
    ],
    message: 'Cannot read field "message" because "response" is null',
    suppressed: [],
    localizedMessage: 'Cannot read field "message" because "response" is null'
  }
}

I definitely did not expect that!

  1. Can you set logging to DEBUG and collect the logs?
    I can if you actually need it, but I don't think it'll be too helpful. The issue is here where the code calls authenticate(), which is always async and can reject, and then doesn't wait for the promise to resolve or reject, but instead immediately tries to connect. The control flow here is a little tough to fix, but here's what I did locally to get my desired behavior. Obviously I haven't tested any other failure modes:
    // Get authenticator to use
    const auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient());
    auth.authenticate(connectionConfig.getAuthenticator(),
      connectionConfig.getServiceName(),
      connectionConfig.account,
      connectionConfig.username).then(
        () => {
          try {
            // JSON for connection
            var body = Authenticator.formAuthJSON(connectionConfig.getAuthenticator(),
              connectionConfig.account,
              connectionConfig.username,
              connectionConfig.getClientType(),
              connectionConfig.getClientVersion(),
              connectionConfig.getClientEnvironment());
      
            // Update JSON body with the authentication values
            auth.updateBody(body);
          } catch {
            callback(err);
          }
          // Can also throw? If so it might also need to go into the try-block
          services.sf.connect({
            callback: connectCallback(self, callback),
            json: body
          });
          return this;

        },
        (err) => callback(err),
      );

    return this;

The idea is this needs to get rearranged to make sure that all the connection stuff doesn't happen if auth fails and instead calls the callback with the error. It seems like all the other authentication methods should be susceptible to some form of this since they're also all async.

@beckjake beckjake added the bug Something isn't working label Jan 24, 2024
@github-actions github-actions bot changed the title Unhandled promise rejections on keypair auth SNOW-1018408: Unhandled promise rejections on keypair auth Jan 24, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

thank you for reporting this issue and for providing all the details and the fix proposal !

We do document the supported key generation process but indeed the driver could be a bit more resilient about the error handling, when an unsupported key is attempted to be used. We'll take a look.

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Jan 26, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage_done Initial triage done, will be further handled by the driver team enhancement The issue is a request for improvement or a new feature and removed status-in_progress Issue is worked on by the driver team bug Something isn't working labels Feb 11, 2024
@sfc-gh-pbulawa sfc-gh-pbulawa added the status-in_progress Issue is worked on by the driver team label Apr 2, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

sfc-gh-dszmolka commented Apr 2, 2024

PR in progress on #815

edit: actually it's already merged; will be released with the next upcoming release hopefully in the coming days

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-in_progress Issue is worked on by the driver team status-pr_pending_merge A PR is made and is under review labels Apr 2, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

fix released with 2024 March release cycle in v1.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

4 participants