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

Support callback via postMessage #752

Closed sliekens closed 4 years ago

sliekens commented 6 years ago

Using hidden iframes to acquire tokens silently is brilliant except for one thing: the token callback is invoked from the iframe instead of the top window.

// this code runs in the main window
authContext.acquireToken(myResource, (errorDesc, token, error) => {
    // this code seems to run in the context of the hidden iframe
    console.log('hello from hidden iframe?!');
    if (error) {
        // example: error === 'login_required'
        throw new Error(error);
    }
});

This is bad for me because any errors that are thrown inside the callback function cannot be logged in window.onerror.

Instead you get a generic "Script error." because of browser security and stuff.

// in top window
window.addEventListener('error', function(err) {
    console.log(err.message);
    // expected: login_required
    //   actual: Script error.
})
sliekens commented 6 years ago

A good solution is to use the postMessage API to pass the window.location.hash from the iframe to the top window before doing any processing.

AuthenticationContext.prototype.handleWindowCallback = function (hash) {
    if (hash == null) {
        hash = window.location.hash;
    }

    if (/* we are inside the hidden iframe */) {
        window.parent.postMessage(hash, window.location.origin);
    } else if (/* we are inside the popup */) {
        window.opener.postMessage(hash, window.location.origin);
    } else if (window.self === window.top) {
         // handle callback as usual
    }
}

Then in the top window, listen for messages that contain hashes and then send them back to the handleWindowCallback function.

// in top window
window.addEventListener('message', e => {
    if (e.origin === window.location.origin) {
        if (typeof e.data === 'string' && isCallback(e.data)) {
            handleWindowCallback(e.data);
        }
    }
});

This is a safe way to send the hash to the top window before invoking any token callbacks.

For more information about the postMessage API, see MDN: https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage

rohitnarula7176 commented 6 years ago

@StevenLiekens We have code in handleWindowCallcack which gets a reference to the main window from the iframe and then calls the callback on the main window. Please see below: https://github.com/AzureAD/azure-activedirectory-library-for-js/blob/49ecdcc655312a111d470858cb424c3c609de4d7/lib/adal.js#L1321 You should never call acquireToken from inside the iframe and you can avoid this by just checking (window.parent!=window).

sliekens commented 6 years ago

You should never call acquireToken from inside the iframe

That's not quite what I'm saying.

We have code in handleWindowCallcack which gets a reference to the main window from the iframe and then calls the callback on the main window.

That's what this issue is about: there is a better way to do it with window.postMessage.

Talk is cheap so here's the code https://github.com/StevenLiekens/azure-activedirectory-library-for-js/commit/8437431213eaa620390ca44451a52405054d72a6

--

So what's wrong with the current code?

You're inside a hidden iframe and executing code that "lives" in the main window. In other words: you're breaking out of the iframe.

This kind of works, but it's not a good idea. In particular it breaks global error handling (via window.onerror). Errors that are thrown in the callback code cannot be inspected in the window.onerror function of the main window.

sliekens commented 6 years ago

bump

sliekens commented 6 years ago

@rohitnarula7176 I rewrote my last comment because it was incorrect. Please take another look.

Kurren123 commented 5 years ago

I would love to see this feature

sameerag commented 4 years ago

This is a good ask - Majority of these issues in error handling should be fixed in the latest msal@1.2.0;

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.