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

getRequestInfo iframe state match not working #698

Closed kphung138 closed 6 years ago

kphung138 commented 6 years ago

Hi,

In 1.0.14, it obtained the _renewStates from the iframe parent. But in 1.1.16, this information seems to be lost, so the match checking doesnt work.

// external api requests may have many renewtoken requests for different resource if (!requestInfo.stateMatch && window.parent && window.parent.AuthenticationContext) { var statesInParentContext = window.parent.AuthenticationContext()._renewStates; for (var i = 0; i < statesInParentContext.length; i++) { if (statesInParentContext[i] === requestInfo.stateResponse) { requestInfo.requestType = this.REQUEST_TYPE.RENEW_TOKEN; requestInfo.stateMatch = true; break; } } }

In the new version I cant see any places where this variable is saved. So when it tries to compare the state in getRequestInfo it doesn't match. // external api requests may have many renewtoken requests for different resource if (!requestInfo.stateMatch && window.parent) { requestInfo.requestType = this._requestType; var statesInParentContext = this._renewStates; for (var i = 0; i < statesInParentContext.length; i++) { if (statesInParentContext[i] === requestInfo.stateResponse) { requestInfo.stateMatch = true; break; } } }

Thanks

rohitnarula7176 commented 6 years ago

@kphung138 We first save the state as follows in the _renewToken and _renewIdToken method which are called internally from acquireToken as follows:

this._renewStates.push(expectedState); // this refers to the instance of adal in the parent window.

getRequestinfo method is called internally in handleWindowCallback. If you look at this line of code in handleWindowCallback

if (this._openedWindows.length > 0 && this._openedWindows[this._openedWindows.length - 1].opener
                && this._openedWindows[this._openedWindows.length - 1].opener._adalInstance) {
                self = this._openedWindows[this._openedWindows.length - 1].opener._adalInstance;
                isPopup = true;
            }
            else if (window.parent && window.parent._adalInstance) {
                self = window.parent._adalInstance;
            }

            var requestInfo = self.getRequestInfo(hash); 

Since self refers to the parent instance, it should have the states defined which were saved earlier. Closing this issue as it is not a bug in the library. Please reopen if you still have issues.

kphung138 commented 6 years ago

Thanks for your pointer.

I'm currently using adal-angular4 which uses your library. I think his code needs to be refactored, with the latest 1.0.16 changes. Otherwise the _renewStates is empty when calling getRequestInfo.

Adal4Service.prototype.handleWindowCallback = function () { var hash = window.location.hash; if (this.adalContext.isCallback(hash)) { var requestInfo = this.adalContext.getRequestInfo(hash); this.adalContext.saveTokenFromHash(requestInfo); if (requestInfo.requestType === this.adalContext.REQUEST_TYPE.LOGIN) { this.updateDataFromCache(this.adalContext.config.loginResource); } .. ..

My work around for now is to maybe use your library directly.

Thanks for your help.