Trouble getting account info from B2C token in componentDidMount

leskodan commented 3 years ago

Library

[ ] msal@1.x.x or @azure/msal@1.x.x
[ x] @azure/msal-browser@2.x.x
[ ] @azure/msal-node@1.x.x
[x ] @azure/msal-react@1.x.x
[ ] @azure/msal-angular@0.x.x
[ ] @azure/msal-angular@1.x.x
[ ] @azure/msal-angular@2.x.x
[ ] @azure/msal-angularjs@1.x.x

Framework

[ ] Angular
[x ] React
[ ] Other

Description

I have an API call that I would like to make as soon as the logged in user has been authenticated to populate a list of items specific to the user. Part of the query string in the api call is information that I am trying to grab from the claims in the token returned by B2C but I am having trouble handling the promises.

When I am logged in, I am able to console log the claim that I am looking for. However, when I try to go through the complete auth flow, it tells me that the claim I am looking for is undefined which leads me to believe that the code in the promise is being called before the login is complete and the token is cached.

Error Message

MSAL Configuration

export const msalConfig = {
    auth: {
        clientId: 'my id',
        authority: 'https://mytenant.com/mytenant.onmicrosoft.com/B2C_1_signupsignin1',
        knownAuthorities: ['mytenant.b2clogin.com'],
        redirectUri: 'http://localhost:3001',
        postLogoutRedirectUri: 'http://localhost:3001'
    }
};

Reproduction steps

class App extends Component{
    static contextType = MsalContext;

componentDidMount(){
        this.context.instance.handleRedirectPromise()
        .then((tokenResponse) => {
            if (!tokenResponse) {
                const accounts = this.context.getAllAccounts();
                if (accounts.length === 0) {
                    // No user signed in
                    this.context.loginRedirect(loginRequest);
                }
            } else {
                // Do something with the tokenResponse
        }})
        .catch(err => {
            // Handle error
            console.error(err);
        })
        .then((res) => {
            console.log(this.context.accounts[0].localAccountId)
        });
    }
}

Expected behavior

Gain access to the localAccountId in the first index of the accounts array.

Identity Provider

[ ] Azure AD
[ x] Azure B2C Basic Policy
[ ] Azure B2C Custom Policy
[ ] ADFS
[ ] Other

Browsers/Environment

[ x] Chrome
[ ] Firefox
[ ] Edge
[ ] Safari
[ ] IE
[ ] Other (Please add browser name here)

Regression

[ ] Did this behavior work before? Version:

Security

[ ] Is this issue security related?

Source

[ ] Internal (Microsoft)
[ ] Customer request

leskodan commented 3 years ago

In playing around with this a little bit more, I am seeing now that refreshing the browser is another way to trigger the "cannot read claim of undefined" error. This is not how I would expect this to work as it was my understanding that the token is cached (or could be retrieved) when needed. Eager to see what I'm doing wrong here.

tnorling commented 3 years ago

@leskodan A couple points to note:

You should not call handleRedirectPromise yourself as this is done under the hood in the msal-react library. Instead if you need direct access to the response from a redirect please use the event API. This is called out in the FAQ
On the initial render after page load, context properties are not yet populated. You should use the inProgress context value to determine when it is safe to access the accounts. You'll find a class component example in our sample here

leskodan commented 3 years ago

Hi @tnorling, thanks for the pointers. On the first note regarding the handleRedirectPromise, I was attempting to follow directions laid out about 2/3s of the way down the page in the "errors" wiki linked here.

I'll look into the "inProgress" context value. As a side note, since posting this issue, I've learned that Azure is having some issues at the moment with Active Directory at the moment: https://status.azure.com/en-us/status. They claim it is not effecting B2C, but at the moment, I can't even log into my apps that are authenticated against B2C.

tnorling commented 3 years ago

@leskodan

On the first note regarding the handleRedirectPromise, I was attempting to follow directions laid out about 2/3s of the way down the page in the "errors" wiki linked here.

Right beneath that is a link to different directions if you're using msal-react. I see the confusion and I'll see what we can do to call this out more clearly.

leskodan commented 3 years ago

@tnorling, thanks for the pointer, I'll be sure to read the docs more carefully next time.

That said, I am still having trouble getting this to work. My goal is to execute a login on app load by putting this method in the componentDidMount() of my App.js.

componentDidMount(){
        if (this.context.accounts[0] && this.context.inProgress === InteractionStatus.None) {
            this.context.instance.acquireTokenSilent({
                ...loginRequest,
                account: this.context.accounts[0]
            });
        }
    }

This does not trigger a login. I have successfully gotten this to work using a login button, I would like this to work on load in my class based App components so that I can set state with info called from my api immediately after authentication. Appreciate the help!

tnorling commented 3 years ago

@leskodan This will not trigger a login. This will get you a cached token after you have already been logged in. If you need to login on page load you should check that there are no accounts instead of checking the first account and call loginRedirect or loginPopup instead of acquireTokenSilent

leskodan commented 3 years ago

@tnorling, that makes sense, and I have tried the following code to account for this:

componentDidMount(){
        if (this.context.accounts[0] && this.context.inProgress === InteractionStatus.None) {
            this.context.instance.acquireTokenSilent({
                ...loginRequest,
                account: this.context.accounts[0]
            }).then((res) => {console.log(res.uniqueId)});

        } else {
            if(!this.context.accounts[0] && this.context.inProgress === InteractionStatus.None){
                this.context.instance.loginRedirect(loginRequest)
                .then((res) => {console.log(res.uniqueId)});
            }
        }
    }

My current issue is that my console.logs are not being fired which is where I would need to be grabbing the homeAccountId to call my api. Appreciate your continued help!

tnorling commented 3 years ago

@leskodan The promise returned from loginRedirect is not expected to resolve as it will trigger a full frame redirect away from your page. If you just need the homeAccountId you can use this.context.accounts[0].homeAccountId inside the first if block. If you need to get the full response from loginRedirect you should register an event callback that will be run when you return from the auth server.

leskodan commented 3 years ago

@tnorling gotcha. This still isn't working the way that I would expect unfortunately.

componentDidMount(){
        if (this.context.accounts[0] && this.context.inProgress === InteractionStatus.None) {
            this.context.instance.acquireTokenSilent({
                ...loginRequest,
                account: this.context.accounts[0]
            }).then((res) => {console.log(res.uniqueId)});
           console.log(this.context.accounts[0].homeAccountId) //Inserted to test access to homeAccountId
        } else {
            if(!this.context.accounts[0] && this.context.inProgress === InteractionStatus.None){
                this.context.instance.loginRedirect(loginRequest)
            }
        }
    }

When I navigate to the app, login is still not triggered on load. (I can get it to trigger if I change the if statements on the dev server, but not through a normal flow. Since I am using the AuthenticatedTemplate on my content (render function below), I just get a white screen. Similarly, the console log in the first if statement of my componentDidMount() is not working either.

render(){
        return(
            <ThemeContextWrapper>   
                <AuthenticatedTemplate>
                    <MyContent/>
                </AuthenticatedTemplate> 
            </ThemeContextWrapper>
        );
    }

I am wondering if I am simply trying to achieve functionality that isn't intended.

tnorling commented 3 years ago

@leskodan Sounds like this component is mounting too early and InteractionStatus is not yet None. Does it work if you put this logic in componentDidUpdate?

An easier way to do this is by using MsalAuthenticationTemplate instead of AuthenticatedTemplate which will invoke login on page load if not already signed in. This is documented here and is demonstrated in our samples.

leskodan commented 3 years ago

@tnorling , it does work if I put this code in componentDidUpdate(). HOWEVER, this causes infinite looping of the console log. That said, I think that confirms your theory of the component mounting too early (I'm assuming this isn't really meant to be used on the App.js component with conditional rendering).

Based on what you describe, it does sound like the MsalAuthenticationTemplate is the way to go over the AuthenticatedTemplate. I'll have to see if this works timing wise in terms of getting my id on componentDidMount(). Thanks!

leskodan commented 3 years ago

To expand on that having refactored a bit:

By wrapping my render() in MsalAuthenticationTemplate and updating the `componentDidMount()' to:

componentDidUpdate(){
        console.log(this.context.accounts[0])
    }

This fires the console.log 3 times. Once undefined, and two more with the accounts[0] object (apparently coinciding with updates happening in the background). I'd like to find a way for this to only happen once so that my api call doesn't trigger multiple times.

tnorling commented 3 years ago

@leskodan If you put your API call in a component underneath MsalAuthenticationTemplate in the component tree it will only ever render when a user is signed in. This way you can put your logic in componentDidMount and ensure it's run only once and has the account it needs.

leskodan commented 3 years ago

@tnorling , that makes sense. I'm currently using the api call to set global state, so I would need to lift it up to my App.js component. I came up with this as well, not sure if it's best practice though haha:

componentDidUpdate(){
        if(this.context.accounts[0]){
            if(this.context.accounts[0].localAccountId !== this.state.userId){
                this.setState({userId: this.context.accounts[0].localAccountId}, () => {
                    console.log(this.state.userId)});
            } 
        }
    }

This way, it will only update state if the account has switched. That said, it definitely adds additional logic from what you suggested.

Thanks for bearing with me through this!

tnorling commented 3 years ago

@leskodan Great, glad to hear you got it working! Can we consider this resolved?

leskodan commented 3 years ago

Yep, appreciate the help!

AzureAD / microsoft-authentication-library-for-js