damienbod / angular-auth-oidc-client

npm package for OpenID Connect, OAuth Code Flow with PKCE, Refresh tokens, Implicit Flow
https://www.npmjs.com/package/angular-auth-oidc-client
MIT License
1.13k stars 429 forks source link

[Question]: authorizeWithPopUp closes too soon with Okta SSO + MFA #1851

Open emo7874 opened 10 months ago

emo7874 commented 10 months ago

What Version of the library are you using? 16.0.0

Question I'm not sure if this is considered a bug or not. Using authorizeWithPopUp to start authentication. Customer uses Okta SSO with an additional layer of security with MFA where it sends the code to their email. Once they press send code, the popup window goes away and they are not authorized.

Here's why this is happening.

We're using a custom redirect page in our app.

<html>
<head>
  <title>Login Landing Page</title>
  <script>
    function sendMessage() {
      // post url containing oidc response (redirected from OP)
      const urlWithOidcResp = window.location.href;
      window.opener.postMessage(urlWithOidcResp, window.opener.location.href);
    }
  </script>
</head>
<body onload="sendMessage()">
Transmitting authentication result ... (this popup will be closed automatically).
</body>
</html>

In the popup.service.ts there is an openPopUp method with the below code. It registers a listener for any messages. Okta was sending a message when the MFA process was initiated which triggered the cleanUp() method. This listener should only care about messages sent from my custom redirect page and should not care about direct Okta messages. Inside the listener, I had to ignore any messages not from my redirect domain. This fixed the issue.

Is this a bug or is this a configuration I don't know about?

const listener = (event: MessageEvent): void => {
      if (!event?.data || typeof event.data !== 'string') {
        this.cleanUp(listener, config);

        return;
      }

      this.loggerService.logDebug(
        config,
        'Received message from popup with url ' + event.data
      );

      this.resultInternal$.next({ userClosed: false, receivedUrl: event.data });

      this.cleanUp(listener, config);
    };

    this.windowInternal.addEventListener('message', listener, false);
Silvenga commented 6 months ago

Can confirm this also breaks with the react-devtools extension installed.

image

This does look like a bug.

lewdi4 commented 4 weeks ago

@damienbod This still appears to be an issue. Maybe we can have a configuration parameter to "only accept messages from redirect domain"?

icewindow commented 3 weeks ago

I can add the Angular dev tools to the list of things that send objects to the parent window.

I have created a very basic fix for the time being (#1988), until a more proper way to handle invalid data is introduced.

lewdi4 commented 3 weeks ago

I am not comfortable with posting pull requests, but I was hoping for something like this: const listener = (event: MessageEvent): void => { if (!event?.data || typeof event.data !== 'string') { this.cleanUp(listener, config); return; } this.loggerService.logDebug( config, 'Received message from popup with url ' + event.data + '. The configured redirect url is ' + config.redirectUrl ); if(!config.redirectUrl || (config.popoutMessageMatchRedirect && event.origin && config.redirectUrl.indexOf(event.origin) > -1)) { this.resultInternal$.next({ userClosed: false, receivedUrl: event.data }); this.cleanUp(listener, config); } else { this.loggerService.logError(config, 'Skipped invalid origin: ' + event.origin); } }; where the code checks the origin to match the redirectUrl, and enforces the result based on the new parameter : popoutMessageMatchRedirect