docusign-esign-node-client icon indicating copy to clipboard operation
docusign-esign-node-client copied to clipboard

Generate Access Token SDK Method is broken since most recent RC Version

Open Ivanrenes opened this issue 1 year ago • 14 comments

When trying to generate token with generateAccessToken using the client it just don't work, throws an empty error (see screenshot). With previous version it works as usual, anything changed for that?

image

Update: Just found the issue in your code, please merge this PR ASAP

Ivanrenes avatar May 23 '24 20:05 Ivanrenes

@Ivanrenes

Thank you for raising this issue and for submitting the PR with a solution. We've resolved this in release 7.0.1.

We genuinely appreciate your active participation and timely feedback. Please update to the latest version and let us know if you encounter any further problems.

sonawane-sanket avatar May 24 '24 08:05 sonawane-sanket

@sonawane-sanket Please check https://github.com/docusign/docusign-esign-node-client/pull/352#pullrequestreview-2077517229 comment, generateAccessToken keeps broken

Ivanrenes avatar May 24 '24 17:05 Ivanrenes

@Ivanrenes I reviewed your comment. We'll validate it internally and get back to you at the earliest. Thank you for bringing this to our attention.

sonawane-sanket avatar May 24 '24 17:05 sonawane-sanket

@Ivanrenes We need some more information on how you're using our SDK. Are you loading it in a browser? It would be very helpful if you could share a sample code snippet so we can better understand the context and reproduce the issue on our end.

sonawane-sanket avatar May 31 '24 10:05 sonawane-sanket

@sonawane-sanket Yep, broken now for us too on a node server integration. Following your docs you recommend doing:

const auth = await authenticate()
const dsApiClient = new docusign.ApiClient()
dsApiClient.setBasePath(auth.basePath)

is where it throws or RC2 with TypeError: Cannot read properties of undefined (reading 'basePath')

was auth.basePath removed?

nikodunk avatar May 31 '24 22:05 nikodunk

@sonawane-sanket Yep, broken now for us too on a node server integration. Following your docs you recommend doing:

const auth = await authenticate()
const dsApiClient = new docusign.ApiClient()
dsApiClient.setBasePath(auth.basePath)

is where it throws or RC2 with TypeError: Cannot read properties of undefined (reading 'basePath')

was auth.basePath removed?

Hey @nikodunk We recently released a new version, 7.0.1.

Could you please confirm if you are still experiencing the issue? And If the issue is not related to generating a token, can you please create a new ticket so we can close this one?

We would be happy to assist you further.

sonawane-sanket avatar Jun 04 '24 11:06 sonawane-sanket

@sonawane-sanket will create a POC today.

Ivanrenes avatar Jun 04 '24 15:06 Ivanrenes

is where it throws or RC2 with TypeError: Cannot read properties of undefined (reading 'basePath') was auth.basePath removed?

Hey @nikodunk We recently released a new version, 7.0.1.

Could you please confirm if you are still experiencing the issue? And If the issue is not related to generating a token, can you please create a new ticket so we can close this one?

Validated that this now works for us again in 7.0.2. Both the security alert, and the RC regression are now fixed for us. Thank you!

nikodunk avatar Jun 10 '24 23:06 nikodunk

@nikodunk also this main issue topic here ?

Ivanrenes avatar Jun 10 '24 23:06 Ivanrenes

I think so. The reproduction steps I described in my original comment above no longer bugs out on 7.0.2 (the path issues). I'm not sure I'd close this yet until we get a second positive though. Can you give 7.0.2 a spin and see if you can reproduce?

nikodunk avatar Jun 11 '24 03:06 nikodunk

@nikodunk I found the issue niko,

If you use addDefaultHeader

const apiClient = new ApiClient();
apiClient.addDefaultHeader('Authorization', `Bearer ${tokensParsed.accessToken}`);

You are writing defaultHeaders in a global scope inside the library

  /**
   * Adds request headers to the API client. Useful for Authentication.
   */
  exports.prototype.addDefaultHeader = function addDefaultHeader(
    header,
    value
  ) {
    defaultHeaders[header] = value;
  };

So every time need to create a new client instace You must call addDefaultHeader

const apiClient = new ApiClient();
apiClient.setOAuthBasePath(this.docuSignKeys.oAuthApi);
apiClient.addDefaultHeader(
  'Authorization',
  `Basic ${Buffer.from(clientComb).toString('base64')}`
);

Because the way that you override headers in generateAccessToken image

SOLUTION: So you MUST internally RESET global default headers every time a new api client instance is created to avoid this issue.

Ivanrenes avatar Jun 11 '24 03:06 Ivanrenes

Thank you for the workaround!

nikodunk avatar Jun 11 '24 04:06 nikodunk

@sonawane-sanket This looks like a bug and a security issue to me. The docs show to set default headers for a given client but they are being applied at a global level and not set on the client instance. Here's an example of the issue:

import { ApiClient } from "docusign-esign";

const getClient = (token: string) => {
  const client = new ApiClient({
    basePath: "base_path",
    oAuthBasePath: "oauth-path",
  });
  client.addDefaultHeader("Authorization", `Bearer ${token}`);

  return client;
};

// request comes in for first user
const client1 = getClient("request1");
// request comes in for second user
const client2 = getClient("request2");

try {
  // first user makes a call
  await client1.generateAccessToken("clientId", "clientSecret", "code");
} catch (e: any) {
  // call was made with authorization token for second user
  console.log("ERROR", e.request.headers.authorization);
}

zz-kivo avatar Jul 03 '24 20:07 zz-kivo

@zz-kivo I had the same issue, I created a PR fixing that but they just ignore it

Ivanrenes avatar Jul 03 '24 23:07 Ivanrenes

Hi @Ivanrenes , @zz-kivo , @nikodunk ,

A new RC version (8.0.0-rc2) has been released with requested fixes. Please test out the same when possible. There will be a patch version 7.0.3 will be released as well with similar fix in a day or two. Thank You.

garg-mudit avatar Jul 22 '24 18:07 garg-mudit

Hi @Ivanrenes, @zz-kivo , @nikodunk ,

The 7.0.3 patch version has been released. Please test it and let us know if there are any remaining issues that need resolution.

Thank You.

garg-mudit avatar Jul 24 '24 09:07 garg-mudit

It looks like the issue is resolved in 7.0.3 for me. Thanks!

zz-kivo avatar Jul 24 '24 15:07 zz-kivo