oidc-client icon indicating copy to clipboard operation
oidc-client copied to clipboard

`renewTokens()` fails silently without throwing an error

Open abhilashlr7 opened this issue 1 year ago • 18 comments

Issue and Steps to Reproduce

When we perform an API call, we use the accessToken and send it to our API endpoint. If the response status is a 401, we refetch the latest accessToken by using the renewTokens() method. Since this method is a promise, we do wait for it and then reperform the API that initially returned a 401.

Now this method seems to eat the error when there is a peculiar scenario where we have the accessTokenPaylod like the following:

"accessTokenPayload": {
      "exp": 1701898951,
      "iat": 1701895411,
      ...
}

Apparently, the exp and iat are well before current UNIX timestamp. Therefore what happens is, the method fails (I suppose - not 100% sure). Because of it, we are unable to get the token for re-firing the API.

Versions

7.12.2

Screenshots

Expected

The method should return the valid accessToken instead of failing silently.

Actual

The method fails silently

Additional Details

  • Installed packages:

abhilashlr7 avatar Dec 08 '23 13:12 abhilashlr7

Hi @abhilashlr7 thank you for your issue. Do you have a sample of your jwt tokens? I think it wiln help me to understand the problem.

guillaume-chervet avatar Dec 08 '23 21:12 guillaume-chervet

Unfortunately I wont be able to share the tokens to the outside world @guillaume-chervet . Is there anything else I can do to help debug this issue?

abhilashlr7 avatar Dec 09 '23 04:12 abhilashlr7

Adding a few notes that I realised debugging this issue:

  1. When renewTokens() is invoked, the renewTokensAsync() method is fired which inturn fires renewTokensAndStartTimerAsync() method.
  2. Now inside this method, since we don't use sessionStorage or ServiceWorker, the line that sets tokens is navigator.locks.request(...).
  3. What I observed is that, the tab that this operation is performed has been idle for quite sometime, and perhaps the token's iat and exp are way behind time.
  4. On another note, I also added an onEvent props to the OidcProvider to observe the events and note down the logs to trace the issue. Doing so, there was this log in console for the event tryKeepExistingSessionAsync_end where it said:
    {
        "success": true,
        "message": "tokens inside storage are valid"
    }
    
  5. And another log for the event token_timer where it has -43116 which was basically way behind in time for the tokens. This means, the token has expired and a new token should've been refetched and stored in our Storage (Localstorage). But that did not happen.
  6. Coming back to point 2, since this tab has been alive for quite sometime the lock that it generated initially never gets resolved. Therefore any new locks that were requested gets queued but doesn't get executed. This basically means, the syncTokens inside the callback function of the locks request never got executed.
  7. I could also confirm that point 6 is true by printing doing the following in Chrome's console:
    const state = navigator.locks.query();
    
    console.log(state);
    
    This kept returning both held and pending where held was always having 1 item in the array while pending had my other 2 items in it. One was for the current navigator.locks.request() while the other was peculiar and is in point 8.
  8. In that same renewTokensAndStartTimerAsync() method if you look at, there is a condition to check for oidc.timeoutId and if so, invoke autoRenewTokens(). Internally this again triggers renewTokensAndStartTimerAsync() which again generates a promise on navigator.locks.request() and therefore gets pushed to the pending queue.

Now my thoughts are on, how can we release a lock on the stuck up locks request and if the timer is negative, how do we refetch the tokens and sync into storage.

Would adding { ifAvailable: true } make it release the promise? I'm not sure, I tried this in local and tried a yarn link but that asset change never reflects during my debug mode and hence I can't confirm if this is the right fix.

abhilashlr7 avatar Dec 09 '23 06:12 abhilashlr7

Thank you @abhilashlr7 for all your détail. do you know the lifetime span of your access token? Do you have a sample of your oidc configuration?

guillaume-chervet avatar Dec 09 '23 07:12 guillaume-chervet

@guillaume-chervet here's my config:

  scope: 'openid profile email offline_access',
  redirect_uri: `/authentication/callback`,
  silent_redirect_uri: `/authentication/silent-callback`,
  refresh_time_before_tokens_expiration_in_second: 60,
  token_request_timeout: 1830,
  storage: localStorage,
  token_renew_mode: TokenRenewMode.access_token_invalid,

abhilashlr avatar Dec 09 '23 07:12 abhilashlr

Hi @abhilashlr , thank you for your information. What happen if temporary you set up refresh_time_before_tokens_expiration_in_second: 180 ?

guillaume-chervet avatar Dec 09 '23 07:12 guillaume-chervet

I modified the script locally in debugger with { ifAvailable: true } and hence the page fetched the right tokens and stored them in the LocalStorage. Now I am unable to go back to that state where it was stuck forever 🤣

The change is here: https://github.com/AxaFrance/oidc-client/pull/1237

I can try with this temporary setup if in case this happens again though.

abhilashlr avatar Dec 09 '23 07:12 abhilashlr

Also @guillaume-chervet what does refresh_time_before_tokens_expiration_in_second do, just curious?

abhilashlr avatar Dec 09 '23 07:12 abhilashlr

Hi @abhilashlr very sorry for the delay, i have been stick this week.

The refresh_time_before_tokens_expiration_in_second will trigger the refresh of the tokens this time before expiration.

I think the if window available make the original problem back. I have to find a solution to remove local on sleeping Window or tab. I will read more the documentation about this.

guillaume-chervet avatar Dec 13 '23 20:12 guillaume-chervet

I this we need to test with an abort controller instead of ifavailable. With a timeout longer than refresh timeout and silentsigin timeout. But it is not simple because of retry integrated. It should be moved to an upper level.

guillaume-chervet avatar Dec 14 '23 08:12 guillaume-chervet

Hi @abhilashlr , I think that the latest error fix this issue.

guillaume-chervet avatar Feb 04 '24 12:02 guillaume-chervet

Hi @guillaume-chervet , Points 3, 4 and 5 in this still seem to be an issue: https://github.com/AxaFrance/oidc-client/issues/1236#issuecomment-1848265003

abhilashlr7 avatar Feb 27 '24 15:02 abhilashlr7

Hi @abhilashlr7 , i have just seen this today on my new app. It has happened after a computer sleep i have to understand what happened deepe.

guillaume-chervet avatar Feb 27 '24 20:02 guillaume-chervet

Hi @abhilashlr7 , i have just seen this today on my new app. It has happened after a computer sleep i have to understand what happened deepe.

Any updates on this @guillaume-chervet ?

abhilashlr7 avatar Apr 18 '24 12:04 abhilashlr7

Hi @abhilashlr7 do you still have error? RenewMethod still fail silently (i need to clean this) but now global app behavior should be very stable.

guillaume-chervet avatar Apr 19 '24 19:04 guillaume-chervet

@guillaume-chervet we are facing a new issue now, actually 2. The token gets reset if there are multiple tabs open for some of our users. Another issue is that the renewTokens() method returns an old token being sent.

I also wanted to know, if lets the internet is not connected for that browser tab, if the token API call fails, is there something like silently failing or not making that api request? For, everytime this token request fails, the authenticatingError component is being rendered.

abhilashlr7 avatar Apr 21 '24 06:04 abhilashlr7

I will look at the renew token return an ild tokens. Why do you use this method? In fact the library renew tokens automaticaly.

Yes i have to manage the not connected to internet case. I think i have an idea to test.

guillaume-chervet avatar Apr 22 '24 07:04 guillaume-chervet

I will look at the renew token return an ild tokens. Why do you use this method? In fact the library renew tokens automaticaly.

So, here's why we did it:

  1. GET /api-1, GET /api-2, GET /api-3 => These are all authn API calls and therefore need the accessToken.
  2. The access Token is obtained from the localstorage (could also use useOidcAccessToken() hook). => Here as well we get the old tokens
  3. Supposing let's say, this access token has expired, when the API call api-1 is fired, our backend services returns a 401.
  4. Now, we need to make sure we get the latest token and refire the same API. This is exactly where I have hooked up a logic to say, if status code is 401, then FETCH the latest token (renewTokens()) assuming it is from the POST api call to the tokens endpoint and then use that response for the headers and refire the failed API.

Now in the last point, if renewTokens() is going to give us old token then there are high chances our backend is going to reject this requests despite we sending the renewed tokens. That's exactly what I've mentioned in the first comment of this issue.

abhilashlr7 avatar Apr 22 '24 15:04 abhilashlr7