Microsoft-authentication-library-for-js: handleRedirectCallback not useful in SPA, documentation misleading

[elhedran]

Please follow the issue template below. Failure to do so will result in a delay in answering your question.

Library

[email protected]

Framework

React v16, + parcel bundler

Description

The redirect flow handles the return from a redirect in a way that it does an additional navigation to hide the hash and return the user to the page they were on when they redirected. So far this is good.

However... using the handleRedirectCallback only gets called on the first redirect (with the hash) and on the second redirect (to remove the hash) the response is now removed. This means that on a page where such a second navigation happens, the page is reloaded such that the redirect response is no longer available.

What does work is using the silent token acquisition to get the current token as stored by msal from a redirect (e.g. get it if you have it, even if silent acquisition isn't supported).

This means the doc indicating handleRedirectCallback is either incomplete or incorrect, because the working sample code doesn't use it to get a token but instead also uses the silent token acquisition to get the most recent state of a token for that request.

In short, you can't always use handleRedirectCallback as the documentation indicates because it doesn't actually work when the msal library does the second redirect, but what does work is silent token acquisition, which isn't documented for the redirect pattern. I'm not entirely sure why the pattern can't be this instead in practice as its what happens in the working pattern.

token = userAgent.acquireCachedToken(authenticationParams);

This would also avoid the need to deal with the promise part of acquireTokenSilently.

Security

I don't believe so

Regression

Don't know, have only ever used this version.

Configuration

Please provide your MSAL configuration options.

const tenantId = 'e3b48527-4cbe-42a2-b4d2-11b3cc7a86fc';
const clientId = '8b1d6b0d-3777-4099-b6bd-6db1ff2ddb02';
const redirectUri = 'http://localhost:1234/';
const apiId = `062da26d-b94e-424d-b119-b6b12fc46618`
const apiUri = `api://${apiId}`;
const scopes = [`${apiUri}/all:full`];

const accessTokenRequest: MSAL.AuthenticationParameters = {
    scopes
};


const msalAppConfig: MSAL.Configuration = {
    auth: {
        clientId,
        authority: `https://login.microsoftonline.com/${tenantId}/`,
        redirectUri
    },
    cache: {
        cacheLocation: 'localStorage'
    }
}

Reproduction steps

export const AuthProvider = (props: React.PropsWithChildren<{}>) => {
    // access token is state as it comes back via a promise.
    const [accessToken, setAccessToken] = React.useState<string>();
    const userAgent = new MSAL.UserAgentApplication(msalAppConfig);

    userAgent.handleRedirectCallback((error, result) => {
        // msal requires a redirect callback, even though can't use it to
        // get the result as it will redirect again after it has the result
        // and not provide the result of the call back on the second
        // redirect.
    });

    function onLogin() {
        userAgent.loginRedirect(accessTokenRequest);
    }

    function onLogout() {
        console.log('clear access token');
        setAccessToken(undefined);
        userAgent.logout();
    }

    async function updateToken() {
        try {
            // silent will renew if possible, but also reply with
            // a token gotten from a redirect workflow.
            const result = await userAgent.acquireTokenSilent(accessTokenRequest);
            if (result && result.accessToken) {
                console.log(`silently acquired token ${result.accessToken}`);
                setAccessToken(result.accessToken);
            }
        } catch (error) {
            console.log(`access token not available`);
        }
    }

    React.useEffect(() => { updateToken(); return; }, []);

    const context: AppContext = {
        login: onLogin,
        logout: onLogout,
        accessToken
    };

    return (
        <AppContext.Provider value={context}>
            {props.children}
        </AppContext.Provider>
    );
}

Expected behavior

I expected there to be a clearer way to get the token returned via a redirectLogin that wasn't invalidated by msal cleaning up the redirect hash and didn't require redundant caching in the app as its already cached by msal as evident in acquire token silently.

Browsers

Not sure, but does happen in current Chrome. Version 79.0.3945.130 (Official Build) (64-bit)

[jasonnutter]

@elhedran I agree this is confusing, it's something we've talked about addressing but haven't gotten to it yet. For now as a workaround, can you try setting auth.navigateToLoginRequestUrl to false?

[elhedran]

The code I posted is a working work around. Sorry, I guess I didn't make
that clear.

On Thu, 20 Feb 2020, 2:33 am Jason Nutter, [e-mail] wrote:

@elhedran https://github.com/elhedran I agree this is confusing, it's
something we've talked about addressing but haven't gotten to it yet. For
now as a workaround, can you try setting auth.navigateToLoginRequestUrl
to false?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/1288?email_source=notifications&email_token=AABULHA4BQ36B5DPPXNCJH3RDVNPFA5CNFSM4KW2ZJFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMIPF5Q#issuecomment-588313334,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABULHDUWEMQMF4N2GP6VCTRDVNPFANCNFSM4KW2ZJFA
.

[jasonnutter]

@elhedran I see. Setting navigateToLoginRequestUrl will prevent the second redirect, is that not desired?

[elhedran]

If I prevented the second redirect I'd only have to handle getting the user
back to where they were before login anyway. Admittedly there are ways of
doing that without causing an actual redirect, but I like how little code
the whole pattern is with msal, even with redirect.

I guess I don't understand why callbacks for handling redirect instead of
queries against cached state, as msal internally does queries against
cached state anyway for the resurrect callback.

On Fri, 21 Feb 2020, 4:03 am Jason Nutter, [e-mail] wrote:

@elhedran https://github.com/elhedran I see. Setting
navigateToLoginRequestUrl will prevent the second redirect, is that not
desired?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/1288?email_source=notifications&email_token=AABULHFIJWHXYJFM4NEUQGLRD3AWDA5CNFSM4KW2ZJFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMPOTNA#issuecomment-589228468,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABULHBJ6ZV4ZE4XQWNQWC3RD3AWDANCNFSM4KW2ZJFA
.

[jasonnutter]

@elhedran The callback for redirect is implemented to provide your app with the response from the redirect action once it has been parsed and stored, so that you don't have to handle doing that.

Appreciate the feedback, I'll chat with the team and follow up with any adjustments we decide to make. Thanks!

[m-sterspace]

I also found this extremely confusing and in general found the documentation on how to implement a redirect flow (and why you're implementing what parts of it), very confusing.

It would help if there was a clear and concise explanation of what the flow is (i.e.):

• Wrap your application in an Auth component.
• When your application and hence the Auth component initializes, try and acquiretokensilent to see if you can silently retrieve a cached token. If you get one, congrats, your user is logged in, you can parse it for user info and attach it to server requests for authentication. However, if you did't get a valid token then your user is not presently logged in, and is going to need to be redirected away from your app at some point so that they can login. In this case you can either:
  
  
  
  • Immediately try and force them to log in by automatically calling loginRedirect (good for sites with no public pages), or
  
  
  • You can wait for them to click on a login button, at which point you should trigger loginRedirect (good for sites that are partially public or have a public welcome page).
  
  
• After logging in on the Microsoft site they will be sent back to whatever the callback page that you specified is, however, you don't need to worry about doing anything here, the msal package will automatically send them to the page they were on previously.

And as a developer you're done., you don't have to do anything else, because on the redirect, your application initializes again, which means your auth component calls acquiretokensilent, but this time it will get it since they just logged in.

... ....

At least I think that's how it works? I could be completely mistaken because I also don't understand why you need to set a redirect callback function, but the current documentation doesn't ever outline what the high level flow is from an app developer's perspective.

[jasonnutter]

@m-sterspace Our quickstarts are meant to help address this, and I agree they could be better with some of the specifics as you mention.

The handleRedirectCallback function needs to be called to parse the redirect response and register your callback, which we agree is confusing (it should probably just do the later, and have the response parsed automatically). We're looking at making this better and more intuitive.

[m-sterspace]

I read through that quick start, and it got me up and running with the loginpopup method in a couple of hours, but it doesn't provide any information on how to properly implement the redirect flow.

For instance this diagram is great: https://docs.microsoft.com/en-us/azure/active-directory/develop/media/quickstart-v2-javascript/javascriptspa-intro.svg ... but it doesn't really make sense with a redirect flow given that the token response is delivered to a different instance of your application then the one that requested it.

It also doesn't tell you the most critical thing about the redirect method which is where you actually need to put each of those function calls in relation to your app's lifecycle. For instance the way I had implemented it originally worked, but the last update caused all my sites to break and go into infinite redirect loops because of when my functions were being called. I rewrote it and got it working again, but I still don't know if I did it the way I'm supposed to because I haven't been able to find a complete redirect example.

[jasonnutter]

@m-sterspace Thanks again for the feedback. I agree the timing of handleRedirectCallback invocation isn't clear, we're looking at how to improve that (and making sure it is documented).

[m-sterspace]

@jasonnutter

I had also opened an issue on the Azure Docs repo, which is here, and recently received an update from @hamiltonha :
https://github.com/MicrosoftDocs/azure-docs/issues/41911

I figured I'd tie these together since they're both the same underlying issue which is that the official documentation could use a clearer life cycle explanation for the redirect method.

[jasonnutter]

@m-sterspace Gotya, thanks!

[tnorling]

@elhedran We have released a beta version: 1.3.0-beta.0 which contains a potential fix for this issue. With this release the hash is now processed in the constructor after the final redirect and calling handleRedirectCallback is now optional. If you do choose to register a callback it should now return the response as you expected. We hope that this is more intuitive.

Could you try it out and let us know if this solves the issue you were experiencing?

[DarkLite1]

I found this issue when struggling with the redirect part of my app. The pop up functionality works fine but the redirect does not, and it would be great to be able to support both.

I do agree that the the documentation on a complete lifecycle of the app is lacking a bit, especially with regards to the redirect flow which I clearly don't understand. I'm also a newbie, so this could be just me.

[DarkLite1]

@elhedran one thing of notice about your code is that if updateToken() fails because acquireTokenSilent isn't returning a token you don't call the redirect. So you still need to take care of that part in your app.

[tnorling]

[email protected] has been released and should fix this issue. Team has internal tracker as well as #41911 for documenting redirect flow.
Let us know if you're still having issues.

[DarkLite1]

Awesome! I just did the upgrade and we can totally skip the empty callbacks now or even calling the method at all when using acquireTokenRedirect:
```javascript
// no longer needed
// msal.handleRedirectCallback()
// msal.handleRedirectCallback(() => { })
````

Great work guys!

[m-sterspace]

@tnorling thanks, just upgraded and this is feeling more intuitive already!