js-sdk: Expired credentials are used after re-authentication

I have noticed this behaviour on the first request that is made after each time credentials expire.

In the code I see that there is a condition that checks if the credentials are expired. And if they are, this.authenticate() (line 128) is called before the request is made.

https://github.com/moltin/js-sdk/blob/84d891fdb8f6d3596aefaaacd88620036e3405c2/src/factories/request.js#L122-L129

But the function const req = () => {... (line 85) uses stale credentials fetched from localStorage before re-authentication.

      const credentials = JSON.parse(storage.get('moltinCredentials'))
      const req = () => {
        const headers = {
          Authorization: `Bearer: ${credentials.access_token}`,

      /* ... */

      if (
        !credentials ||
        !credentials.access_token ||
        credentials.client_id !== config.client_id ||
        Math.floor(Date.now() / 1000) >= credentials.expires
      ) {
        return this.authenticate()
          .then(req)  // <--- This uses the same credentials
          .catch(error => reject(error))

This causes the request to fail with the message

{
    "errors": [
        {
            "status": 401,
            "title": "Unable to validate access token"
        }
    ]
}

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 1
  • Comments: 15 (7 by maintainers)

Commits related to this issue

Most upvoted comments

Thanks @aravindanve I’m looking into this now and will get back to you once I confirm and patch.

I would send a PR right now, but your contribution rules look a little intimidating. 😅

I’ll try and do it later if I can take some time out.

Thanks for submitting the issue @aravindanve - you are completely right.

We’re open for a PR but I’ll also make sure this is flagged internally so we can push a fix.