IdentityModel / oidc-token-manager

Apache License 2.0
51 stars 36 forks source link

Add token expiring setting #10

Closed appetere closed 8 years ago

appetere commented 9 years ago

When using the TokenManager with silent token renewal, and multiple tabs open in the browser, each instance of the TokenManager will try to renew the token at roughly the same time (as outlined in https://github.com/IdentityServer/IdentityServer3.Samples/issues/111).

If each TokenManager had a randomised time for firing the TokenExpiring callbacks (for example within the range 45 - 75 seconds instead of hard-coded at 60 seconds) then the TokenManager with the shortest time would effectively act as the "master" for all TokenManagers.

This could be achieved by exposing the time at which early expiry should happen as a setting:

function TokenManager(settings) {
    this._settings = settings || {};    
    this._settings.token_expiring_threshold = this._settings.token_expiring_threshold || 60;
function configureTokenExpiring(mgr) {
 [...]
    function configure() {
        cancel();

        if (!mgr.expired) {
            var duration = mgr.expires_in;
            var expiringThreshold = mgr._settings.token_expiring_threshold;
            if (duration > expiringThreshold) {
                setup(duration - expiringThreshold);
            }
            else {
                callback();
            }
        }
    }

Then each instance of the TokenManager could be setup with randomised times, such as:

var config = {
   [...]
    token_expiring_threshold: 45 + Math.floor(Math.random() * 30),
};

var mgr = new OidcTokenManager(config);
brockallen commented 9 years ago

Yep, not a bad idea. The token manager needs to do a better job of handling multiple tabs.

a23o commented 9 years ago

I have run into this too. Is there any development on this, or should I just fork it and do what @appetere suggested?

brockallen commented 9 years ago

For the time being you should. I'm swamped right now.

a23o commented 9 years ago

OK. Thx.

brockallen commented 8 years ago

Just following up on this -- how are you using this feature now? Just staggering the different tabs so they don't step on each other?

appetere commented 8 years ago

@brockallen Rather timely - we will be deploying this to production in a few weeks. I'd been ignoring the issue, but would use the randomised expiry times if added to the Token Manager code.

I had also considered turning off the silent-renewals completely, and just doing a silent token-request whenever a token was needed, but a valid one wasn't currently held by the Token Manager. This wouldn't be so good for performance though.

brockallen commented 8 years ago

I had also considered turning off the silent-renewals completely, and just doing a silent token-request whenever a token was needed, but a valid one wasn't currently held by the Token Manager. This wouldn't be so good for performance though.

Why would this be bad for perf?

appetere commented 8 years ago

Meant the responsiveness from the user's perspective - in that it would take longer to first get a token, then make the request using the token. But that would only affect one request every 20 minutes (or whatever the token expiry is) so not a big problem.

brockallen commented 8 years ago

Yep, ok. One more thought -- in the next (breaking change) version I'll be changing the default to use sessionStorage and not localStorage. Perhaps this would work better for you, and you could leave the silent renew enabled. Just configure the store with window.localStorage on the settings object passed to the OidcTokenManager ctor.

appetere commented 8 years ago

I've used SessionStorage, as you suggested.

To get tokens only when they are needed I created an ensureTokenAsync method (below). Once a token-request is underway it returns the same promise to all callers, so they all wait for the single token-request to be completed, before continuing.

    var isRenewingToken = false;
    var renewTokenSilentPromise;

    OidcTokenManager.prototype.ensureTokenAsync = function() {
        var mgr = this;

        if (!isRenewingToken) {

            renewTokenSilentPromise = new Promise(function (resolve, reject) {

                // When enabled, Token Manager's silent-renew runs at 60s from expiry. If token still not renewed 
                // at 30s then something is probably wrong, but we re-try the renewal incase was transient problem.
                var expiresInSeconds = mgr._settings.silent_renew ? 30 : 60;

                if (mgr.expires_in > expiresInSeconds) {
                    resolve(mgr);
                    isRenewingToken = false;
                } else {
                    isRenewingToken = true;

                    mgr.renewTokenSilentAsync()
                        .then(
                            function () {
                                resolve(mgr);
                                isRenewingToken = false;
                            },
                            function (e) {
                                reject(e);
                                isRenewingToken = false;
                            });
                }
            });
        }

        return renewTokenSilentPromise;
    };
brockallen commented 8 years ago

Closing, as this has been addressed in the updated oidc-client (https://github.com/IdentityModel/oidc-client-js) replacement.