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

Using an iFrame for getting the token - Is adal-angular doing something I'm not aware of? Is this a bad idea? #746

Closed butchmarshall closed 6 years ago

butchmarshall commented 6 years ago

This code seems to work just fine for acquiring a token (no popups, no redirects except for in the iFrame).

Is there any reasons you can think of why this is a bad idea?

const getToken = (adal_config) => {
    return new Promise((resolve,reject) => {
        const src = adal_config.instance+""+adal_config.tenant+"/oauth2/authorize?"+$.param({
            "response_type": "id_token",
            "client_id": adal_config.clientId,
            "redirect_uri": window.location.href,
            "nonce": Math.random(),
        }),
        $iframe = $("<iframe src='about:blank' style='visibility: hidden; position: absolute; left: -10000px; top: -10000px;' />");
        $iframe.load(function() {
            const hash = $iframe.get(0).contentWindow.location.hash,
            queryStr = $.deparam(hash.replace(/^#/,''));

            resolve(queryStr.id_token);

            $iframe.remove();
        });
        $iframe.attr("src", src);
        $(document.body).append($iframe);
    });
};
sliekens commented 6 years ago

Well for one you're not doing any error handling. Can you explain how this question relates to adal-angular?

butchmarshall commented 6 years ago

Sorry - I tried to keep my example small.

I'm wondering why ADAL doesn't have a similar option to prevent page redirects entirely. Is there something wrong with doing it this way? Is this worth building as an option available in ADAL?

The actual code is...


import AuthenticationContext from 'adal-angular';

const getTokenByRedirect = (adal_config) => {
    return new Promise((resolve,reject) => {
        console.log("ADAL CONFIG", adal_config);
        delete adal_config.redirectUri;

        const authContext = new AuthenticationContext(adal_config),
        isCallback = authContext.isCallback(window.location.hash);

        if (isCallback && !authContext.getLoginError()) {
            authContext.handleWindowCallback(window.location.hash);
        }

        const  user = authContext.getCachedUser(),
        tokenCache = authContext.getCachedToken(adal_config.clientId);

        if (!user || !tokenCache) {
            authContext.login();
        }
        else {
            resolve(tokenCache);
        }
    });
}

const getTokenByIFrame = (adal_config) => {
    return new Promise((resolve,reject) => {
        const src = adal_config.instance+""+adal_config.tenant+"/oauth2/authorize?"+$.param({
            "response_type": "id_token",
            "client_id": adal_config.clientId,
            "redirect_uri": window.location.href,
            "nonce": Math.random(),
        }),
        $iframe = $("<iframe src='about:blank' style='visibility: hidden; position: absolute; left: -10000px; top: -10000px;' />");

        // When iFrame loads, get the token
        $iframe.load(function() {
            try {
                const contentWindow = $iframe.get(0).contentWindow,
                hash = contentWindow.location.hash,
                queryStr = PSLIB.Request.deparam(hash.replace(/^#/,''));

                resolve(queryStr.id_token);

                $iframe.remove();
            }
            catch(e) {
                getTokenByRedirect(adal_config).then(
                    (authToken) => {
                        resolve(authToken);
                    }
                );
            }
        });

        $iframe.attr("src", src);

        $(document.body).append($iframe);
    });
};
sliekens commented 6 years ago

Gotcha. I think this is the important bit:

My spfx webpart lives within SharePoint so the users already guaranteed to be signed in.

This is probably one of the rare occasions where you can guarantee that the user is already authenticated. Other apps don't have that luxury and have to be able to show a sign-in page.

So is it a bad idea to hide the id_token flow? Personally I don't think so. But is it worth building it into this library? Maybe if there was more demand.