AzureAD / microsoft-authentication-library-for-js

Microsoft Authentication Library (MSAL) for JS
http://aka.ms/aadv2
MIT License
3.6k stars 2.64k forks source link

state_not_found in 3.0.0-alpha.2 #6042

Closed grosch-intl closed 1 year ago

grosch-intl commented 1 year ago

Core Library

MSAL.js v2 (@azure/msal-browser)

Core Library Version

3.0.0-alpha.2

Wrapper Library

MSAL Angular (@azure/msal-angular)

Wrapper Library Version

3.0.0-alpha.2

Public or Confidential Client?

Public

Description

When the app starts up now I'm getting an error about client auth state not being found. My startup (code below) is just calling initialize on the public client, and my AppComponent extends MsalRedirectComponent. The route I'm calling includes the MsalGuard in the canActivate.

Error Message

ERROR Error: Uncaught (in promise): ClientAuthError: state_not_found: State not found: Cached State ClientAuthError: state_not_found: State not found: Cached State at ClientAuthError.createStateNotFoundError (:4200/vendor.js:16081:12) at :4200/vendor.js:8865:79 at Generator.next () at asyncGeneratorStep (:4200/vendor.js:317888:24) at _next (:4200/vendor.js:317907:9) at :4200/vendor.js:317912:7 at new ZoneAwarePromise (:4200/polyfills.js:9120:21) at :4200/vendor.js:317904:12 at RedirectHandler.handleCodeResponseFromHash (:4200/vendor.js:8902:7) at :4200/vendor.js:7756:39 at resolvePromise (:4200/polyfills.js:8908:19) at :4200/polyfills.js:8817:9 at :4200/polyfills.js:8833:25 at rejected (:4200/vendor.js:54578:9) at _ZoneDelegate.invoke (:4200/polyfills.js:8107:158) at Object.onInvoke (:4200/vendor.js:132100:25) at _ZoneDelegate.invoke (:4200/polyfills.js:8107:46) at Zone.run (:4200/polyfills.js:7890:35) at :4200/polyfills.js:8968:28 at _ZoneDelegate.invokeTask (:4200/polyfills.js:8134:171)

Msal Logs

Not sure where to get the logs, but when it fails my browser redirects to a route that starts with #code= and includes query params of client_info, state and session_state. Not pasting them here as I don't know whether they contain info I wouldn't want made public.

MSAL Configuration

const publicClientApplication = new PublicClientApplication({
    auth: {
        clientId: environment.azure.clientId,
        authority: 'https://login.microsoftonline.com/...',
        redirectUri: window.location.origin
    },
    cache: {
        cacheLocation: BrowserCacheLocation.LocalStorage,
        storeAuthStateInCookie: false
    }
})

Relevant Code Snippets

export const msalProvider = MsalModule.forRoot(
    publicClientApplication,
    {
        interactionType: InteractionType.Redirect
    },
    {
        interactionType: InteractionType.Redirect,
        protectedResourceMap: new Map([
            [`${environment.graphUrl}/*`, [`${environment.azure.clientId}/.default`]],
            [`${environment.apiUrl}/*`, [`${environment.azure.clientId}/.default`]]
        ])
    }
)

export const provideAzure = () => makeEnvironmentProviders([
    {
        provide: APP_INITIALIZER,
        useFactory: () => async () => {
            await publicClientApplication.initialize()
        },
        multi: true
    },
    {provide: HTTP_INTERCEPTORS, useClass: MsalInterceptor, multi: true},
    MsalGuard
])

Reproduction Steps

I provideAzure() in the application bootstrap and when it starts up I'm now getting that cache error.

Expected Behavior

no error :)

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Chrome

Regression

3.0.0-alpha.1

Source

External (Customer)

tnorling commented 1 year ago

This indicates that sessionStorage is being cleared sometime between the redirect to the sign-in page and the invocation of handleRedirectObservable by the MsalRedirectComponent on return to your app. Can you check sessionStorage to confirm?

Not sure where to get the logs, but when it fails my browser redirects to a route that starts with #code= and includes query params of client_info, state and session_state. Not pasting them here as I don't know whether they contain info I wouldn't want made public.

You're right, please don't share the contents of the hash fragment :) But you can enable logging by following the instructions here

grosch-intl commented 1 year ago

@tnorling I was using LocalStorage, not SessionStorage. Switching to session didn't make a difference. If I look in SessionStorage I see three items starting with msal. One ends with request.params, one with request.origin, and one with request.correlationId.

Thanks for the log pointer. Which level should I provide? It's a ton right now because I have to enable the Preserve Log option in devtools since each redirect would clear the page and then error would get lost.

tnorling commented 1 year ago

Temporary storage values (which state is) are always stored in sessionStorage regardless of which option you choose to store your tokens. It's interesting that the other temporary values are there but state is missing...We'll need to understand if it fails to get set or was removed after the fact.

LogLevel Trace is best as it will provide everything and then we can filter out what we don't need. And yes, preserve log is crucial here.

grosch-intl commented 1 year ago

Here's the log file with trace level set.

localhost-1684516329991.log

tnorling commented 1 year ago

I'm seeing a lot more calls to handleRedirectPromise than I would expect to see. That's not normally an issue because we memoize the calls but in this case before the error is thrown I'm seeing 2 calls that were able to execute in parallel. That suggests to me that you have more than one instance of PublicClientApplication or it was reinstantiated at some point. That's going to be the root cause here.

grosch-intl commented 1 year ago

I'm also using the mgt components, so in my APP_INITIALIZE I also do this:

Providers.globalProvider = new Msal2Provider({publicClientApplication})

I just commented that line out after your comment and I don't get those redirects any longer, but of course that then means those components break. In my package.json I refernce them like so:

    "@microsoft/mgt": "3.0.0-preview.2",
    "@microsoft/mgt-mock-provider": "3.0.0-preview.2",
tnorling commented 1 year ago

Hmm I'm not super familiar with that package but it appears both you and the package are doing the right thing. You're passing in an already instantiated object, so I don't think that would be it unless mgt is somehow persisting this across pages.

grosch-intl commented 1 year ago

:(

Suggestions on my next step?

tnorling commented 1 year ago

Ok new theory - When the MsalRedirectComponent invokes handleRedirectPromise it does so by passing the hash into the function. The mgt package calls handleRedirectPromise without the hash which causes these two calls to be treated separately and allows them both to execute.

This is the first time I'm seeing this specific issue so I'm not sure yet what the right solution is for these two packages to coexist. But I'll throw out a couple ideas for workarounds and I'll let you try them out and figure out what makes the most sense to you to unblock you for the time being until we discuss internally.

  1. Delay building the Msal2Provider until after msal-angular has finished handling the redirect
  2. Don't use the MsalRedirectComponent at all on pages where you create the Msal2Provider

The purpose the MsalRedirectComponent is really just to ensure that applications are doing the initialization steps and handling the response from the redirect before attempting to use other features of the library, but since your other package is handling that for you, it may actually just be redundant in your case.

grosch-intl commented 1 year ago

Thank you! I left the APP_INITIALIZER alone and simply removed the MsalRedirectComponent from the AppComponent and it seems to be working properly now.

tnorling commented 1 year ago

Great to hear! I'll bring this up with the team and see if anyone can think of any reason not to make this our official recommendation when using graph toolkit going forward and will update our docs when we have consensus. Thanks and let us know if you run into any other issues!