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 374 forks source link

Pop-up redirected to /null instead of closing itself #850

Closed nvigne closed 4 years ago

nvigne commented 5 years ago

I'm submitting a...

Browser:

Library Name

Library version

Library version: 1.0.17

Current behavior

The current behavior was reproducted in Chrome, but could probably happens in other browser as well.

Our login flow is happening in a pop-up for multiple reason. When you try to open the window for the first time, Chrome (and other browser) are blocking the pop-up (we open the pop-up by calling the login method in a programmatic way without user interaction).

The user is asked to either allow all the popups for the specific domain or he can click on the link to open the specific pop-up.

popupblocked

If the user click on the link, the pop-up will open and he will be able to complete the login flow in the login.onmicrosoft.com domain. Then he is redirected to the redirect url page where we simply call the handleWindowCallback.

Calling this method should not be necessary for pop-up, however, for Adal.js library, since the browser blocked the pop-up, we lost the reference on the pop-up from the main window, so the main window will not handle the callback. At that point we enter the code in handleWindowCallback, in line 1365, the library does the following logic:

if (window.parent === window && !isPopup) {
                if (self.config.navigateToLoginRequestUrl) {
                    window.location.href = self._getItem(self.CONSTANTS.STORAGE.LOGIN_REQUEST);
                } else window.location.hash = '';
            }

We enter the firs if condition, because the pop-up does'nt know this is a popUp, then goes to the second if, and since we didn't provide any value for navigateToLoginRequestUrl, it is set to true by default. At that point we set the window.location.href to the value stored in the local storage. Infortuantelly, this value is set to nothing, which cause the window.location.href to be set to null.

Expected behavior

I would expected the pop-up to close nicely.

I think there is globally two issues here:

Minimal reproduction of the problem with instructions

Steps are detailed in the expected behaviour. Basically, if the pop-up is not directly open by Adal.js, in that case it is the browser which opened the pop-up, then we loose reference on the parent window, which cause the redirection to /null.

nvigne commented 5 years ago

The popUp was redirecting to /null because the redirect Uri was using the session storage when the caller was using the local Storage.

It does not solve the issue where the popUp does not close by itself when the login process finish, due to the fact that the main window lose reference on the popUp

NaveenK-J commented 5 years ago

Any updates on this? We are running into similar issues

jmckennon commented 4 years ago

This bug is fixed in msal js. All current authentication work from Microsoft is delivered through the msal js library here. adal js is still supported only for security fixes. We recommend moving to msal js for any advanced feature requests and bugfixes.