Refresh token renewal

[atomicbrainman]

While the library handles Access Token refreshing with silent renew, it doesn't take into account the Refresh Token expiration time at refresh_expires_in. https://github.com/authts/oidc-client-ts/blob/8d8a700b23a2fffdada0b3ecaf271bc16a74759d/src/UserManager.ts#L226-L232

Assuming the user either sets automaticSilentRenew or uses events to do the same, this forces a constant Access Token refresh cycle when Refresh Token expiration time is less than accessTokenExpiringNotificationTimeInSeconds, since Access Token expiration time is limited by the Refresh Token expiration time.

[longsleep]

@atomicbrainman can you please clarify a bit, most of the time we won't know the expiration of a refresh token. refresh_expires_in is unsupported and afaict also non-standard. What providers/services do support it at the moment?

If there is a refresh token, it should be tried to exchange it for an access token.

The actual error and can be handled when using automatic silent renew using the corresponding error handler. When calling signinSilent directly it should be up to the caller to do error handling (promise reject). In any case, silent renew should abort on any errors which are related to the refresh token being expired.

[atomicbrainman]

@longsleep

refresh_expires_in is unsupported and afaict also non-standard. What providers/services do support it at the moment?

It seems at least Keycloak does support it, I don't know about the others.
And actually, we don't need it to detect a session that is bound to expire soon.

Also, there are no errors when refreshing the token.
Let's use an example to clarify the issue:

Say, we have an auth server with the following setup:

• Session time set to 12 hours (Refresh Token time)
• Access Token expiration time is set to 20 minutes

And the user:

• Has an existing session that expires in 22 minutes
• Has an expired Access Token, but a valid Refresh Token
• Client is configured with automaticSilentRenew: true and accessTokenExpiringNotificationTimeInSeconds: 60

The client does the following:

1. [Time passed: 0] Automatic silent renew successfully requests a new Access Token. It's expiration time is 20 minutes.
2. [Time passed: 19 minutes] Again, automatic silent renew successfully requests a new Access Token. But it's expiration time is 3 minutes, same as the time left before Refresh Token expires.
3. [Time passed: 21 minutes] Access Token is about to expire again. So automatic silent renew successfully requests a new Access Token. It's expiration time is 59 seconds.
4. [Time passed: 21 minutes 1 second] Automatic silent renew goes into a cycle of successful requests to refresh the Access Token. But the returned Access Token has an expiration time of less than 1 minute, so the next request is fired almost immediately after the previous one finishes.
5. [Time passed: 22 minutes] Automatic silent renew finally fails to renew the token and the client raises a logout event.

Step 4 is the issue here. On step 3 we can detect that the returned Access Token has an expiration time less than what is set in accessTokenExpiringNotificationTimeInSeconds and do something to prevent step 4, e.g. logout the user immediately.

A simple way to reproduce the issue would be to setup your client with a very large accessTokenExpiringNotificationTimeInSeconds, e.g. set it to 1000000

[Badisi]

I once reported the same issue with the previous library version (different scenario though) : oidc-client-js#948

Issue is still relevant with oidc-client-ts and @atomicbrainman's proposal to fix it seems the way to go.

[longsleep]

@atomicbrainman thanks for the details. That helps. So as far as i understand it its kind of unrelated to the refresh token - its more related to an access token when after it got renewed is only valid for a short period of time (we get the value via expires_in). If expires_in is small then basically instantly another automatic renew is triggered.

I guess some logic needs to be added which prevents too small expires_in values to trigger the automatic renew at all. We could add some minimal value in the configuration. Whenever expires_in comes back smaller than the configuration value, it can stop. Needs investigation if some event could be triggered if that happens.

[Badisi]

There's already a configuration value (ie. settings.accessTokenExpiringNotificationTimeInSeconds).

It is then passed as expiringNotificationTimeInSeconds here: UserManagerEvents.ts#L48.

Later, calculation is made here : AccessTokenEvents.ts#L35 :

  const duration = container.expires_in;
  logger.debug("access token present, remaining duration:", duration);

  if (duration > 0) {
    // only register expiring if we still have time
    let expiring = duration - this._expiringNotificationTimeInSeconds;
    if (expiring <= 0) {
      expiring = 1;
    }

    logger.debug("registering expiring timer, raising in", expiring, "seconds");
    this._expiringTimer.init(expiring);

In @atomicbrainman's example (step 3) : duration = 59s and _expiringNotificationTimeInSeconds defaults to 60s.

1. So that makes expiring = -1, then = 1 and then an expiring timer of 1s is made.
2. Timer times out, raise the expiring event which triggers another renew, which brings back here.
3. Another timer of 1s is made and so on.. Until the access_token really expire, gets rejected by the OP and breaks that loop.

---

The thing I don't understand here, is why the if (expiring <= 0) { expiring = 1; } was made... ? To me, if there is not enough time left to notify about an expiring then the timer should simply get canceled.

[jgladen]

I have no idea what I'm doing.. but I too am having this exact problem. in angular with keycloak. I randomly set settings.accessTokenExpiringNotificationTimeInSeconds to -1 and my problems seemed to go away. I'm in early development so for me it is a workaround until a more rigorous solution is developed.

[atomicbrainman]

A couple more cases for this issue that are related to refresh token.

Preconditions:

• User is loaded from storage with both Access Token and Refresh Token expired (it's easier to reproduce this with localStorage instead of sessionStorage)

Case 1:

1. automaticSilentRenew: true and monitorSession: true
2. The library tries to renew the Access Token using an expired Refresh Token and fails

Case 2:

1. automaticSilentRenew: true and monitorSession: false
2. Nothing happens

[lawrizs]

Hey,

I think there is a problem with token refresh at this line: https://github.com/authts/oidc-client-ts/blob/1d8254343748c367ef46e0992c510759ece990a2/src/utils/Timer.ts#L38

I think, Math.max instead of Math.min function should be used at this line:

  const timerDurationInSeconds = Math.min(durationInSeconds, 5);

[pamapa]

I think its correct the timer shall be called multiple times, its sliced.

The above code comment:

// we're using a fairly short timer and then checking the expiration in the
// callback to handle scenarios where the browser device sleeps, and then
// the timers end up getting delayed.

thus the callback must check for the expiration.

Math.min gives the smaller, which should be 5 min most of the times, except for the last call...

[lawrizs]

@pamapa Yes, 5 mins is a reasonable duration. But since this is 5 seconds, this simply DoS our authentication server. If set to 300 (5*60), Math.min should be just fine.

[pamapa]

Yes its 5 seconds sorry for confusion. The _callback itself should hold back and to call every 5secs. See _callback:

protected _callback = (): void => {
        const diff = this._expiration - Timer.getEpochTime();
        this._logger.debug("timer completes in", diff);

        if (this._expiration <= Timer.getEpochTime()) {
            this.cancel();
            super.raise();
        }
    };

Means it effectively calls the registered callbacks (via super.raise()) only when expired (this._expiration) and not every 5 seconds