AzureAD / microsoft-authentication-library-for-js

Microsoft Authentication Library (MSAL) for JS
http://aka.ms/aadv2
MIT License
3.61k stars 2.64k forks source link

Login popup doesn't close and displays react app itself #5091

Closed Benni03 closed 1 year ago

Benni03 commented 2 years ago

Core Library

MSAL.js v2 (@azure/msal-browser)

Core Library Version

2.28.1

Wrapper Library

MSAL React (@azure/msal-react)

Wrapper Library Version

1.4.5

Description

Hi, I think I've just found a bug in MSAL.js. I installed the latest version in two different samples for React. When I try to log in, sometimes the popup shows the React Webapp itself and doesn't close.

Error Message

No response

Msal Logs

No response

MSAL Configuration

{
    auth: {
        clientId: "my_client_id",
        authority: "https://login.microsoftonline.com/...",
        redirectUri: "http://localhost:3000"
    },
    cache: {
        cacheLocation: "sessionStorage", 
        storeAuthStateInCookie: false
    },
    system: {   
        loggerOptions: {    
            loggerCallback: (level, message, containsPii) => {  
                if (containsPii) {      
                    return;     
                }       
                switch (level) {        
                    case LogLevel.Error:        
                        console.error(message);     
                        return;     
                    case LogLevel.Info:     
                        console.info(message);      
                        return;     
                    case LogLevel.Verbose:      
                        console.debug(message);     
                        return;     
                    case LogLevel.Warning:      
                        console.warn(message);      
                        return;     
                }   
            }   
        }   
    }
}

Relevant Code Snippets

There is no relevant code snippet as I have only changed the MSAL configuration.

Reproduction Steps

  1. Clone the sample
  2. Install the newest version of @azure/msal-browser and @azure/msal-react through npm.
  3. Adjust MSAL configuration
  4. Run the sample and try to log in several times.

Expected Behavior

Login popup should close, but it doesn't.

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Chrome, Firefox, Edge

Regression

@azure/msal-browser 2.15.0

Source

External (Customer)

ghost commented 2 years ago

This issue requires attention from the MSAL.js team and has not seen activity in 5 days. @bmahall please follow up.

vvolodin commented 2 years ago

I'm having the same issue with my Angular app. Popup shows up, I login, and then the app sign in page is displayed in the popup window which doesn't close. Console shows this error:

msal.js.browser@2.28.1 : Info - handleRedirectPromise was unable to extract state due to: BrowserAuthError: state_interaction_type_mismatch: Hash contains state but the interaction type does not match the caller.

I'm only using Popup flow, there's no mention of other interaction types anywhere in my config.

taoisu commented 2 years ago

I'm having the same issue, too. Any ideas how to resolve it? This is blocking my app from being widely adopted by my team.

vvolodin commented 2 years ago

Check this issue, it helped me to come up with a workaround: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/5013

I realized that I had some sort of race condition, here's how the popup interaction works: your main frame opens a popup and waits for a popup URL to contain a hash (#code....). When it catches the hash, popup is closed and token is being read from the hash. In my case the popup URL was being "eaten" by my app itself (I don't do anything crazy there, but something obviously was running faster than the hash-expecting main frame), making the main frame msal not able to read the hash.

taoisu commented 2 years ago

Thanks @vvolodin! This is useful information. I'll try redirecting to a blank page.

bmahall commented 2 years ago

@taoisu Thanks for opening this issue. Can you please share fiddler trace for me to be better able to debug this issue? You can share it on my email found in the Github profile. Meanwhile,

bmahall commented 2 years ago

@taoisu Also, did setting the redirect uri to blank page change the scenario? If you want to use redirect uri in your app you can set it on per request basis, instead of in the auth config.

ghost commented 2 years ago

@Benni03 This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 5 days. If your issue has been resolved please let us know by closing the issue. If your issue has not been resolved please leave a comment to keep this open. It will be closed automatically in 7 days if it remains stale.

Benni03 commented 2 years ago

Updating to react 18 solved this issue for me. I hope I can help others with this information. Thanks!

henrymcl commented 2 years ago

I have also encountered this bug, but only occasionally. Usually with my environment a refresh will sort out the issue and it isn't very common (1 out of 10 times?). It bugs me that my clients are facing this issue more often though.

Environment info:

Packages:

"@azure/msal-browser": "^2.28.1",
"@azure/msal-react": "^1.4.5"
"react": "^16.14.0"
"react-router-dom": "^5.3.0",

Config

PublicClientApplication({
  auth: {
    clientId: "...",
    authority: `https://login.microsoftonline.com/${"..."}`,
  },
})

Click handler

const loginClick = useCallback(() => {
    instance.loginPopup({
      scopes: ["email"],
    }).then(result => {
      return //some business logic follows
    })
}, [instance, history])
Benni03 commented 2 years ago

You are right. The problem has not completely disappeared in React 18, but it does not occur as often.

tnorling commented 2 years ago

@henry132109 @Benni03 This happens because something (most likely your router) is removing the response hash from the popup window before the calling page can extract it. Have you tried setting the redirectUri to a static blank page as suggested above? If that's not possible for your use case the alternative is to ensure that your router doesn't strip the hash or do any redirection on your redirectUri page, at least when opened in a popup window.

henrymcl commented 2 years ago

@henry132109 @Benni03 This happens because something (most likely your router) is removing the response hash from the popup window before the calling page can extract it. Have you tried setting the redirectUri to a static blank page as suggested above? If that's not possible for your use case the alternative is to ensure that your router doesn't strip the hash or do any redirection on your redirectUri page, at least when opened in a popup window.

My router doesn't strip hashes. Plus if that's the case, wouldn't it occur every time?

tnorling commented 2 years ago

No this is a race condition between the popup window and the caller. The problem only surfaces when the router (or other hash clearing/redirection logic) wins the race. Check if there's a hash with code= in the address bar of the popup window when this occurs. If there's not, something inside the popup is removing the hash.

ghost commented 2 years ago

@Benni03 This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 5 days. If your issue has been resolved please let us know by closing the issue. If your issue has not been resolved please leave a comment to keep this open. It will be closed automatically in 7 days if it remains stale.

aNyMoRe0505 commented 2 years ago

@tnorling thanks for your reply

I understand the race condition you have mentioned, and I log my path info in redirect page like this

import { useEffect } from 'react';
import { useRouter } from 'next/router';

const AzureLoginCallback = () => {
  const router = useRouter();
  useEffect(() => {
    console.log('router.asPath', router.asPath);
    console.log('window.location.hash', window.location.hash);
  }, [router.asPath]);

  return (
    ...
  );
};

截圖 2022-09-06 下午1 23 13

and I check the url in browser bar. Indeed, the url has been updated without the hash

but the console only log the url with hash, I have already redirect to a blank page, still don't know why ..

Is it possible related to https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/174 ?

and @Benni03 is using the official sample, is it possible a bug in msal ?

btw, in my scenario, only using mobile phone will occur this situation..

tnorling commented 2 years ago

@aNyMoRe0505 In your case there appears to be an interaction type mismatch (indicated on the final log of your screenshot) i.e. you've called both loginRedirect and loginPopup and the hash represents one of those operations whereas the temporary cache values in sessionStorage represent the other. In general we recommend picking one interaction type and sticking with it, if that's not possible for some reason or another you will need to ensure you don't call both at the same time (the library has protections to prevent this but certain race conditions can still cause this).

This is very likely different from the original issue being discussed here, if you need more help I'd suggest opening your own issue to keep the discussion focused on your scenario.

aNyMoRe0505 commented 2 years ago

@tnorling thanks for your reply I am sure that I don't call both loginRedirect and loginPopup at the same time I will open another issue for this, thank you ❤️

ludwich commented 2 years ago

I have the same issue in Vue 3. There is no logic to this either since it happens sometimes and sometimes it just works. Weird that it is showing the application itself.

aNyMoRe0505 commented 2 years ago

I just come up with a workaround

According to tnorling mentioned above, this is a race condition between the popup window and the caller.

if set the redirect uri to blank page not work for you, you can try my workaround

let's say your redirect uri page is A component, you can save the hash info and try to add back to url in A component

here is an example with Next.js

import { useEffect, useRef } from 'react';
import { useRouter } from 'next/router';

const LoginCallbackPage = () => {
  const router = useRouter();
  const urlHashRef = useRef('');

  useEffect(() => {
    if (window.location.hash) urlHashRef.current = window.location.hash;
  }, []);

  useEffect(() => {
    const handleNavigate = () => {
      if (urlHashRef.current) {
        router.replace(
          {
            pathname: router.pathname,
            hash: urlHashRef.current,
          },
          undefined,
          { shallow: true }
        );
      }
    };

    const intervalTimer = setInterval(handleNavigate, 1000); // need using setInterval to retry otherwise it will not work

    handleNavigate();

    return () => {
      clearInterval(intervalTimer);
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [router.pathname]); // router instance will be different after router.replace be called, so I ignore here

  return <div>Loading</div>;
};

this work for me, hope it helps someone

p-rk commented 2 years ago

+1 same issue, even with the blank route this is still not working.

sushrut111 commented 2 years ago

Workaround if you are using React and logins with only popups - In index.tsx / index.js => Replace

  ReactDOM.render(
    <React.StrictMode>
      <MsalProvider instance={msalInstance}>
        <App/>
      </MsalProvider>
    </React.StrictMode>,
    document.getElementById('root')
  );

with

if (window.location.hash !== ''){
  console.log("hash found" + window.location.hash);
}
else {
  ReactDOM.render(
    <React.StrictMode>
      <MsalProvider instance={msalInstance}>
        <App/>
      </MsalProvider>
    </React.StrictMode>,
    document.getElementById('root')
  );
}
jasonnutter commented 2 years ago

As noted above, this can happen when the page you use as the redirect URI removes/changes the hash when the application is loaded in the popup. Consider turning off any router logic when your application is loaded in the popup in this scenario, or using a completely blank html page as the redirect uri.

jasonnutter commented 2 years ago

Also, the redirect URI used for a given request must be on the same origin as your application.

mikerains commented 1 year ago

I experience this, and have not found a good solution. The Azure-Samples msal-react app does not set the RedirectUrl to a blank page either, the instructions say to set it to "https://localhost:3000"

If set to "https://localhost:3000/blank.html" and the user uses loginRedirect(...) then the user it routed to the blank.html page and is not in the SPA. I suppose it might be possible to put javascript in the blank,html page's onLoad event and forward the user back to the SPA page, but that seems hacky?

I am fairly confident that my React code is not removing hash, nor is the BrowserRouter doing so, because if I manually put a hash on the URL before browsing, that hash remains after BrowserRouter has routed me to the home page of my SPA app.

My app is structured like this:

index.tsx:

ReactDOM.render(
  <React.StrictMode>
    <AppContextProvider>
      <RoutedApp />
    </AppContextProvider>
  </React.StrictMode>,
  document.getElementById('root'),
);

AppContextProvider.tsx:

    <MsalProvider instance={msalInstance}>
      <AppContext.Provider value={appContext}>
        {children}
      </AppContext.Provider>
    </MsalProvider>

RoutedApp:

    <div className="App">
      <AppHeader />
      <BrowserRouter basename="/">
        {Routes}
      </BrowserRouter>
    </div>

Routes:

<Router>
  <Navbar>
  ...
  </Navbar>
  <Switch>
  ... bunch of <Route> elements
  </Switch>
</Router>

So is wrapping the entire SPA content and must be running before any of my code could even possibly being messing with the URL hash - including BrowserRouter.

The app login code is from the msal-react Azure-Samples

SignInButton.tsx:

    const handleLogin = (loginType) => {
        if (loginType === "popup") {
            instance.loginPopup(loginRequest).catch(e => {
                console.log(e);
            });
        } else if (loginType === "redirect") {
            instance.loginRedirect(loginRequest).catch(e => {
                console.log(e);
            });
        }
    }

Perhaps it needs to use async, ie:

  const handleLogin = async (loginType: string) => {
    if (loginType === 'popup') {
      await msalClient.loginPopup(appConfig.loginRequest).catch((e) => {
        const err = JSON.stringify(e);
        showToastMessage(`Login Popup failed with error ${err}.  You may need to log out and restart the app.`);
      });
    } else if (loginType === 'redirect') {
      await msalClient.loginRedirect(appConfig.loginRequest2).catch((e) => {
        const err = JSON.stringify(e);
        showToastMessage(`Login Redirect failed with error ${err}.  You may need to log out and restart the app.`);
      });
    }
  };

I am going to try this and see if it recurs.

jasonnutter commented 1 year ago

@hectormmg Will be this be addressed by #5276 ?

hectormmg commented 1 year ago

@hectormmg Will be this be addressed by #5276 ?

I believe so, increasing the polling frequency by lowering the poll interval time seems to greatly mitigate this race condition

babarmashkoor commented 1 year ago

Is that issue resolved? I am facing the same problem; the popup windows sometimes won't close and redirect to the app.

aNyMoRe0505 commented 1 year ago

I think this issue is not resolved yet, even with the blank route. As @hectormmg said, it just mitigate the race condition (?)

vladutclp commented 1 year ago

Workaround if you are using React and logins with only popups - In index.tsx / index.js => Replace

  ReactDOM.render(
    <React.StrictMode>
      <MsalProvider instance={msalInstance}>
        <App/>
      </MsalProvider>
    </React.StrictMode>,
    document.getElementById('root')
  );

with

if (window.location.hash !== ''){
  console.log("hash found" + window.location.hash);
}
else {
  ReactDOM.render(
    <React.StrictMode>
      <MsalProvider instance={msalInstance}>
        <App/>
      </MsalProvider>
    </React.StrictMode>,
    document.getElementById('root')
  );
}

@sushrut111 This seems to be working in my case but I don't understand why. Could you please give a thorough explanation for this solution? Thank you!

ViolentRage commented 1 year ago

Hi, same issue for me with Vue 2.0 and msal-browser@2.28.3

Actually, I can't replace my Redirect Uri with a blank page, because I have to add it in Azure AD and Azure won't let me. The redirect addresses accepted by Azure must necessarily be valid urls.

image

image

tushar3006 commented 1 year ago

@hectormmg I tried to Deep dive into the issue here:

The PopupWindow that opens has a hash and state in the URL that gets cleared on redirect, meanwhile if the browser polling for popup url state meets a race condition. It fails to close the popup and instead redirects the website to the application inside the popup creating a loop.

Also a key point to note, the setInterval is used for polling in @azure/msal-browser library.

But 1st iteration of setInterval gets triggered after the mentioned time so even if i set the popupInterval as 1ms then also it gets registered as an async event in event loop and can create the race condition as both handle redirect and setInterval are now in async state.

Do you think there's a workaround for this or any future updates?

tnorling commented 1 year ago

@tushar3006 You need to ensure that the hash/state do not get cleared from the popup window i.e. stop that 2nd redirection from happening. That is the root cause of the race. If you need further assistance please open a new issue, we do not regularly monitor closed issues.

MeredithMooreUX commented 1 year ago

We are having the double popup issue

image

BryceHayden commented 1 year ago

Leaving this here in cause it helps anyone else that stumbles on this issue. For us it is still very much an issue and we have tried all of the suggest approaches here and others (blank page, checking the window for a hash, adding a timeout to all redirects, etc). None of them worked.

We have been able to consistently reproduce the bug (as it only happens intermittently for users) by allowing the page to sit idle for a long period. Then when we come back and the session has expired, clicking on the app redirects us to our login page, where we get this double app issue in the popup. Should we clear the HttpOnly Cookies in the app via the developer tools then the problem goes away and we're able to login like normal.

Note above I was saying "we," as my team was able to reproduce it following the same pattern. The rest of this is my opinion so...I don't believe this has anything to do with the frontend code on our side but is a bug in MSAL. I say this as we are using a useEffect that checks if the user has been authorized (based on the useMSAL hook), and then double checks the MSAL instance for the account before attempting to redirect the user. If you close the popup and then click the login again MSAL notices that the user has a valid session token and redirects them correctly. The code for our login is below:

import { useNavigate } from "react-router-dom";
import { useIsAuthenticated, useMsal } from "@azure/msal-react";

export const Login = () => {
  const isAuth = useIsAuthenticated();
  const { instance } = useMsal();
  const navigate = useNavigate();

  useEffect(() => {
    (async () => {
      if (isAuth) {
        const account = await instance.getActiveAccount();

        if (account) {
          const timeout = setTimeout(() => {
            navigate("/overview/year")
          }, 500);

          return () => {
            clearTimeout(timeout);
          };
        } 
      }
    })();
  }, [isAuth, instance]);

  const handleLogin = async () => {
    try {
      const res = await instance.loginPopup(SCOPES_MICROSOFT);
      await instance.setActiveAccount(res.account);
    } catch (err) {
      console.warn("Unknown error logging the user into the app.");
    }
  };

But I'll be super happy if someone finds an error in our logic...this is just the latest attempt at fixing this issue. My team has tried so many variations, that we are giving up and switching to a different SSO provider sadly.

tnorling commented 1 year ago

@BryceHayden This works for you when you start with an unauthenticated state because when your redirectUri is loaded inside the popup it won't redirect to /overview/year (which clears the response hash). When you start with an authenticated state the page loaded inside the popup will redirect to /overview/year in 500ms (in theory this timeout would make this happen less often but isn't a full solution). If that happens before your top frame window reads the hash you will hit this bug. The permanent solution is to not do that redirect (or any redirect) ever inside the popup.

BryceHayden commented 1 year ago

@tnorling So I spent the rest of the day removing all redirects, having it point to a blank page again, and simply having a button change it's text once useIsAuthenticated changed from false to true and I still got the double app sign in error a couple of times. I really don't think the hash/redirect is the root cause. I think that might be part of the problem for some people, but I think there's something more going on here.

tnorling commented 1 year ago

Is your blank page in your route path? Try including it in the public folder instead so that your router does not get used.

One thing to check is if the url in the popup contains the hash. If it does not, something removed it. If it does, make sure your top frame (caller) isn't redirecting or refreshing before the popup closes.

sameerag commented 1 year ago

Is there an update on this @BryceHayden?

mikerains commented 1 year ago

I observed today something that might give a clue; I don't have time to investigate right now though.

This issue was recurring on my Edge browser which has the most recent updates. My Redirect is a registered FQDN, not running on local host. I had not logged-in for a week, so my oauth tokens were pretty old. I had just deployed my app to the same FQDN but a different IP Address.
package.json:

    "@azure/msal-browser": "^2",
    "@azure/msal-react": "^1",
    "react-router-dom": "^5.2.0",
    "react-scripts": "5.0.1",

package-lock.json:

node_modules/@azure/msal-browser": {
      "version": "2.28.3",

"node_modules/@azure/msal-react": {
      "version": "1.4.7",

"node_modules/@types/react-router-dom": {
      "version": "5.3.3",

"node_modules/react-scripts": {
      "version": "5.0.1",

In this deployment, the pop-up was stubbornly remaining open, showing the app with the user authenticated. In the past, this would happen only when I needed to authenticate, and then, once authenticated, refreshing the browser, the app would show me as authenticated. -- This is where it is stubborn and different than it has been in the past: today, after authenticating (and remaining in the pop-up window), and then refreshing the browser, it stubbornly still showed me as still not authenticated; and furthermore, on clicking SignIn, which is using loginPopup(), the popup dialog opens, sends the auth client request, and shows me logged in, still inside the popup.

The observation I am sharing from this is:

  1. opened my Edge Dev Tools, and enabled "Auto-open DevTools for popups"
  2. refreshed browser
  3. clicked SignIn (thus loginPopup())
  4. Pop-up opened, and so did DevTools
  5. I observed a request sent to auth client in the pop-up's dev tools
  6. the pop-up closed, and in the main browser window, the app successfully recognizes I am logged-in.

TL;DR - Causing the browser to open the Pop-up DevTools automatically seems to have influenced the behavior and made the pop-up work. I think I recall from years past that the chrome engine's thread behavior is different when the dev tool is open, regarding how it handles the "turning of the clock".

Again, that is anecdotal but might provide a clue as to the root cause.

mikerains commented 1 year ago

Also, I can recreate this in Edge by:

  1. using browser dev tools, in Application tab, Clear All Site Data, and
  2. Using browser refresh, "clear cache and hard reload"

Also also - I tried applying the work-around conditional to not render when hash exists, ie:

if (window.location.hash !== '') {
  // eslint-disable-next-line no-console
  console.log(`hash found: ${window.location.hash}`);
} else {
  root.render(

and to confirm this does work-around the issue.

However, also also also, I tried the redirectUri="blank.html" work-around, but my app uses , and with redirectUri='blank.html' work-around also applied, inside the popup, it tries to post to https://<<My-ADB2C-directory>>.onmicrosoft.com/B2C_1_signupsignin/client/perftrace?tx=State, Properties=eyJUS..... which causes a 404 and breaks the popup. With this the redirectUrl work-around removed, this 404 doesn't happen on this post.

Hope this helps.

wadeem commented 1 year ago

Same issue persists on mobile browser (chrome, firefox)

A workaround with tweaking the pollIntervalMilliseconds did not help.

theobaidur commented 1 year ago

Another workaround could be,

import { UrlString } from '@azure/msal-common';
const container = document.getElementById("root");
const root = createRoot(container!);

/**
 * We need to determine if we are in the auth popup
 * If so, we need to render a simple message
 * Otherwise, we render the app
 * [MSAL uses]{@link (https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/src/interaction_client/PopupClient.ts#L343)} "UrlString.hashContainsKnownProperties" method to validate the hash
 * @returns true if the current window is an auth popup
 */

const isAuthPopup = () => {
  const hash = window.location.hash;
  return UrlString.hashContainsKnownProperties(hash);
}

// check if we are in the auth popup
// if so, render a simple message
// otherwise, render the app
if (isAuthPopup()) {
  root.render(<div>
    <h1>Authenticating...</h1>
  </div>);
} else {
  root.render(<App />);
}
zmarkusdev commented 1 year ago

The problem in my case was, that the customer called the site with "www.site.com" and I configured the return url as "site.com", for future reference if anyone has the same problem..

nicolastrafenchuk commented 1 year ago

I had the same problem. The above methods didn't work in my case, so I started digging deeper. I realized that a couple of weeks ago I added the Helmet library for my Express backend. And the problem was there. By default, the Helmet enables Cross Origin Opener Policy with 'same-origin' policy: Cross-Origin-Opener-Policy: same-origin

You can read more about it on MDN portal. So, this header doesn’t allow sharing context between the pop-up and main window when origins are different - in that case, the pop-up can't redirect back to the main window, and code execution continues in the pop-up window.

I have changed Cross Origin Opener Policy to 'unsafe-none' policy and now all things are working as expected. Cross-Origin-Opener-Policy: unsafe-none

In my express app it looks like that: app.use( helmet({ ..., crossOriginOpenerPolicy: { policy: 'unsafe-none' }, // Needed for MSAL pop-up }), );

Hope that helps.

sivatumma commented 11 months ago

@nicolastrafenchuk - This is niche. Thank you

augustoicaro commented 3 weeks ago

I had a similar issue with render.com using the free tier web service with node. I tried everything, and what worked for me was upgrading msal-browser to the latest version and using the pollIntervalMilliseconds option set to 1000