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.
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
- fix: Fixed an issue where credentials are not available to request after initial auth Pass the credentials object to the request function Fixes #150 — committed to moltin/js-sdk by ynnoj 6 years ago
- fix: Request credentials params (#152) * fix: Fixed an issue where credentials are not available to request after initial auth Pass the credentials object to the request function Fixes #150 ... — committed to moltin/js-sdk by ynnoj 6 years ago
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.