Access token is refreshed to late.

[JessArschoot]

Before reporting an issue

• I have searched existing issues
• I have reproduced the issue with the latest release

Area

adapter/javascript

Describe the bug

In our React application we use access tokens to fetch data from our API. After the token expires, it is automatically refreshed. However it's possible that a fetch is triggered by a user between the expiry of the access token, and the refresh. As a result, our API returns 401 because the token is expired.

We've been debugging the JS adapter to determine the cause and saw that there's a property called "minValidity". Here we assume that the purpose of this property is to "never" have an expired token. For example, refresh the token 5 seconds (default behaviour) before it actually expires.

    kc.isTokenExpired = function(minValidity) {
        if (!kc.tokenParsed || (!kc.refreshToken && kc.flow != 'implicit' )) {
            throw 'Not authenticated';
        }

        if (kc.timeSkew == null) {
            logInfo('[KEYCLOAK] Unable to determine if token is expired as timeskew is not set');
            return true;
        }

        var expiresIn = kc.tokenParsed['exp'] - Math.ceil(new Date().getTime() / 1000) + kc.timeSkew;
        if (minValidity) {
            if (isNaN(minValidity)) {
                throw 'Invalid minValidity';
            }
            expiresIn -= minValidity;
        }
        return expiresIn < 0;
    }

However, we've seen that the token is refreshed using a async handler, "tokenTimeoutHandle", but this doesn't take the "minValidity" into account.

    function setToken(token, refreshToken, idToken, timeLocal) {
        if (kc.tokenTimeoutHandle) {
            clearTimeout(kc.tokenTimeoutHandle);
            kc.tokenTimeoutHandle = null;
        }

        if (refreshToken) {
            kc.refreshToken = refreshToken;
            kc.refreshTokenParsed = decodeToken(refreshToken);
        } else {
            delete kc.refreshToken;
            delete kc.refreshTokenParsed;
        }

        if (idToken) {
            kc.idToken = idToken;
            kc.idTokenParsed = decodeToken(idToken);
        } else {
            delete kc.idToken;
            delete kc.idTokenParsed;
        }

        if (token) {
            kc.token = token;
            kc.tokenParsed = decodeToken(token);
            kc.sessionId = kc.tokenParsed.session_state;
            kc.authenticated = true;
            kc.subject = kc.tokenParsed.sub;
            kc.realmAccess = kc.tokenParsed.realm_access;
            kc.resourceAccess = kc.tokenParsed.resource_access;

            if (timeLocal) {
                kc.timeSkew = Math.floor(timeLocal / 1000) - kc.tokenParsed.iat;
            }

            if (kc.timeSkew != null) {
                logInfo('[KEYCLOAK] Estimated time difference between browser and server is ' + kc.timeSkew + ' seconds');

                if (kc.onTokenExpired) {
                    var expiresIn = (kc.tokenParsed['exp'] - (new Date().getTime() / 1000) + kc.timeSkew) * 1000;
                    logInfo('[KEYCLOAK] Token expires in ' + Math.round(expiresIn / 1000) + ' s');
                    if (expiresIn <= 0) {
                        kc.onTokenExpired();
                    } else {
                        kc.tokenTimeoutHandle = setTimeout(kc.onTokenExpired, expiresIn);
                    }
                }
            }
        } else {
            delete kc.token;
            delete kc.tokenParsed;
            delete kc.subject;
            delete kc.realmAccess;
            delete kc.resourceAccess;

            kc.authenticated = false;
        }
    }

Version

16.1.1

Expected behavior

The new access token is refreshed before it actually expires.

Actual behavior

The new access token is refreshed when it's expired.

How to Reproduce?

We've noticed that this issue is mostly present when there's a "bad internet connection". This creates a bigger "window" for data to be fetched while the token is not refreshed yet.

Anything else?

We wanted to create a PR for this, but couldn't push to github. Hence, this issue.

[amanfrinati]

Follow because I had the same problem.

@JessArschoot I wrote a function that set a timeout (after onAuthSuccess and onAuthRefreshSuccess events) to refresh the token before the expiring: in this way, I trigger the refresh manualy. Did you write a fix for the keycloak library instead?

  ...

  // Called when the token is refreshed.
  keycloak.onAuthRefreshSuccess = updateToken;

  // Called when a user is successfully authenticated.
  keycloak.onAuthSuccess = updateToken;

  const updateToken = () => {
    if (config.keycloak.tokenMinValidity >= config.keycloak.tokenExpirationAdvance) {
      config.keycloak.tokenExpirationAdvance = 0;
    }

    const keycloakTokenExp = (keycloak && keycloak.tokenParsed && keycloak.tokenParsed.exp) || 0;
    const expirationIn =
      keycloakTokenExp -
      Math.ceil(new Date().getTime() / 1000) -
      Number(config.keycloak.tokenExpirationAdvance);
    console.log("[AuthenticationProvider] token expire in " + expirationIn);
    
    // To anticipate the token update of tokenExpirationAdvance seconds,
    // you need to set tokenExpirationAdvance < tokenMinValidity
    setTimeout(() => {
      keycloak.updateToken(Number(config.keycloak.tokenMinValidity));
    }, expirationIn * 1000);
   
  };

[mathias-pahlen]

@amanfrinati, we wanted to indeed fix the library itself since we we're debugging in it to figure out what was going on. However we didn't manage to push a branch to the github repo and create a PR.

The issue we think was in the following:

var expiresIn = (kc.tokenParsed['exp'] - (new Date().getTime() / 1000) + kc.timeSkew) * 1000;
kc.tokenTimeoutHandle = setTimeout(kc.onTokenExpired, expiresIn);

The idea of the fix we had was indeed similar to your implementation.

Do you know if we can submit a PR for the Keycloak library? (We're not experienced in commiting in open-source projects)

[amanfrinati]

@mathias-pahlen yes, the problem is the value of expiresIn, that trigger the "timeout event function" exactly when the token expires.

I suppose you cannot commit and push to the project: you must fork the project, then create a branch and commit/push on it, then open a new PR on Keycloak project from your fork to the original repo.

IMO, to force the refresh of the token before the expiring, the library needs:

• a new config value (in this way, the user can set it as prefer)
• a validation of the value of minValidity config

[edewit]

I don't think we should be refreshing the token automatically to begin with. This would mean that the user will never needs login you might as well remove keycloak entirely

[mathias-pahlen]

@edewit, some React component does the refreshing part for us. It calls the onTokenExpired and fetches a new token.

Before we introduced our "fix", the user would briefly have an expired token. Even though they were still working on the application. As a result, the user could trigger a call with the expired token resulting in an "unauthorized".

The desired result is to be able to fetch a new token briefly before the current one expires.

We've only discovered that in all related code that we've found concerning token expiry this "minValidity" was taken into account. Except when setting this async function kc.tokenTimeoutHandle which triggers the onTokenExpired.

After overriding this behaviour, by taking the "minValidity" into account, we had our desired result. Only we believe that this is a shortcoming of the JS adapter, and not something that we (and apparently others like @amanfrinati) need to implement.

[edewit]

I'm not saying there isn't a bug here, I'm only saying that you shouldn't be updating the token without user interaction. When you call the backend you can update the token if that is not possible the user needs to login

[hugo-instasense]

I don't think we should be refreshing the token automatically to begin with. This would mean that the user will never needs login you might as well remove keycloak entirely

I thought you might appreciate using time-based refresh done with the precision of a Swiss watch? But "joggel" aside.

We are having the same issue. So just for context for others that find this, there are two refresh strategies, namely time-based (fixed timer that refreshes the token say every 4 minutes) or interaction-based (every time a user say hits an endpoint refreshes the token).

Now you (sarcastically?) said it is not necessary for Keycloak if one uses time-based refreshing if I understand you correctly. However, I disagree since dashboards might be a use case and it is hard to predict the average user session.

On that note, testing this has been frustrating since all the settings on keycloak's dashboard for max session length with user interaction-based strategy fails to refuse token refreshing when it expires (i.e. waited 3 minutes before hitting another attempted refresh) when setting advanced settings to 1-minute refresh, it seems to default to 5 minutes.

The first call will be 401, the second call will be 200 which is a massive problem.

If I can ask @edewit for the sake of all the different FE frameworks out there, can you point me to any typescript/javascript implementation that has the user-interaction-based strategy you are talking about? Something similar to this https://github.com/dasniko/keycloak-reactjs-demo/blob/main/src/services/UserService.js

On the interaction based way we had the same issue where a token would be expired when a user made a call which was frustrating.

[edewit]

sorry @hugo-instasense seems I own you an apology, I didn't know there where two strategies.

we use this to update the token each time the user requests something from the backend, like this:
https://github.com/keycloak/keycloak/blob/main/js/apps/account-ui/src/api/request.ts

[vladimir-kouznetsov]

I see the issues is still there for the latest Keycloak 23.0.6.

Could the priority please increased from "medium" to "high" ?

[keycloak-github-bot]

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

[jonkoops]

The best way to guarantee that you have a token at the moment of making a request is by calling .updateToken(minValidity) before you make a request. This method will check if the token is expired, taking minValidity into account (defaults to 5 seconds), and only if the token is expired will it refresh it (and the other tokens) for you.

For example:

try {
  await keycloak.updateToken();
} catch {
  keycloak.login();
}

await fetch(`/api/users`, {
  headers: {
    authorization: `Bearer ${keycloak.token}`,
  },
});

This approach is 'on demand' and is the recommended approach that we outline in the guide for Keycloak JS. This allows the refresh token to expire naturally after a period of inactivity, after which it is no longer possible to use the refresh token to obtain an access token. This is preferred for security reasons so that a user does not remain signed in on public applications.

However, if you want to keep the access token valid you could opt to call updateToken() inside of an interval to automatically refresh it whenever there is a need preemptively. Do still check if the token is actually valid before making the request though, and handle any errors appropriately.

For example:

// Check if the token needs a refresh every 5 seconds with a window of 15 seconds to expiry.
setInterval(() => keycloak.updateToken(15), 5000);

Perhaps we should be building in these strategies a feature so users do not have to write them themselves, e.g:

const keycloak = new Keycloak({
  // The type of strategy to use to refresh the access token.
  refreshStrategy: 'none' | 'on-demand' | 'on-expire',
  // The 'window' in which a token can be considered in danger of expiring.
  expiryWindow: 5000,
});

// A new method that returns a Promise that takes the aforementioned strategies into account.
const token = await keycloak.getToken();

The values for refreshStrategy here would mean:

• none — Never refresh the access token automatically.
• on-demand — Refresh the access token automatically when getToken() is called, and the token is at risk of expiring.
• on-expire — Refresh the access token automatically when the token is at risk of expiring.

One problem I have with this approach is that it makes Keycloak JS core more messy and tightly coupled, so perhaps we would make these strategies classes that could be passed in instead, that would also allow for custom strategies to be implemented by users themselves, for example:

import { Keycloak, OnDemandStrategy } from 'keycloak-js'

const keycloak = new Keycloak({
  refreshStrategy: new OnDemandStrategy({  expiryWindow: 5000 }),
});

Either way, this would be considered a feature request, and not a bug. I'll re-label it as such. Really looking forward to what you might think of this API from a user perceptive, suggestions more than welcome!