AzureAD / azure-activedirectory-library-for-js

The code for ADAL.js and ADAL Angular has been moved to the MSAL.js repo. Please open any issues or PRs at the link below.
https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/maintenance/adal-angular
Apache License 2.0
627 stars 372 forks source link

getCachedToken has side effects that can cause intermittent 401's #742

Closed Wozbo closed 4 years ago

Wozbo commented 6 years ago

In adal.js, the getCachedToken function (provided) has a side effect that affects people using this.

AuthenticationContext.prototype.getCachedToken = function (resource) {
        if (!this._hasResource(resource)) {
            return null;
        }

        var token = this._getItem(this.CONSTANTS.STORAGE.ACCESS_TOKEN_KEY + resource);
        var expiry = this._getItem(this.CONSTANTS.STORAGE.EXPIRATION_KEY + resource);

        // If expiration is within offset, it will force renew
        var offset = this.config.expireOffsetSeconds || 300;

        if (expiry && (expiry > this._now() + offset)) {
            return token;
        } else {
            this._saveItem(this.CONSTANTS.STORAGE.ACCESS_TOKEN_KEY + resource, '');
            this._saveItem(this.CONSTANTS.STORAGE.EXPIRATION_KEY + resource, 0);
            return null;
        }
    };

If the token's expiry is within the zone of the expire offset, the token gets nulled out in the session and nothing is returned. The issue here is that depending on this for JWT bearer requests will fail intermittently while the application is refreshing the token.

Why are we unsetting and returning null if the token is within the expiry zone? Should we not instead return the token and have a separate check that determines if the token should be renewed?

jillcary commented 6 years ago

I have also been seeing similar behavior.

sliekens commented 6 years ago

Should we not instead return the token and have a separate check that determines if the token should be renewed?

I agree but I'm not sure what that separate check would look like and when would it be called?

Wozbo commented 6 years ago

This is still an issue and is now affecting tokens issued in India; there seems to be a time skew on the return from AD that is causing tokens to expire earlier than expected. This seems to be about 30 - 45 seconds or so.

Wozbo commented 6 years ago

This seems to be related/ exacerbated by: https://github.com/Azure/azure-documentdb-dotnet/issues/512

sameerag commented 4 years ago

All current authentication work from microsoft is delivered through msal js library here. adal js is still supported only for security fixes. We would recommend to move to msal js for any advanced feature asks.