Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use the same Instance of PublicClientApplication as Prop for MsalProvider and in Loader of a React Router Route #7068

Open
2 tasks
Robstei opened this issue Apr 30, 2024 · 6 comments
Labels
bug-unconfirmed A reported bug that needs to be investigated and confirmed msal-browser Related to msal-browser package msal-react Related to @azure/msal-react Needs: Attention 👋 Awaiting response from the MSAL.js team public-client Issues regarding PublicClientApplications question Customer is asking for a clarification, use case or information.

Comments

@Robstei
Copy link

Robstei commented Apr 30, 2024

Core Library

MSAL.js (@azure/msal-browser)

Core Library Version

3.10.0

Wrapper Library

MSAL React (@azure/msal-react)

Wrapper Library Version

2.0.12

Public or Confidential Client?

Public

Description

The State of the MsalProvider gets updated when invoking acquireTokenSilent inside the loader function of a react router route.

This leeds to the error below because updating state of another component during rendering is not supported by react.

The reason for that seems to be the adtion of a callback to the PublicClientApplication which leeds me to think that it is not possible to use the same instance as a prop for the MsalProvider and in a routes loader function.

The Goal is to redirect the user under certain conditions. A graph request is necessary to determin if a user should be redirected.

Possible Solution:
It works without error when using two instances of the PublicClientApplication and provide one as a prop to the MsalProvider and use the other instance inside the loader function.

Is this considered a bad pattern? Are there recommendations to use acquireTokenSilent during rendering of other components.

I will gladly provide an example if necessary.

Error Message

Warning: Cannot update a component (MsalProvider) while rendering a different component (ReactRouterWrapper). To locate the bad setState() call inside ReactRouterWrapper, follow the stack trace as described in https://reactjs.org/link/setstate-in-render
at ReactRouterWrapper (https://localhost:4200/src/app/context/ReactRouterWrapper.tsx?t=1714488769684:34:23)
at div
at TextDirectionProvider (https://localhost:4200/node_modules/.vite/deps/chunk-OVEVGIFW.js?v=cab54ad8:2580:3)
at https://localhost:4200/node_modules/.vite/deps/chunk-ELTDTHBU.js?v=cab54ad8:662:17
at FluentProviderWrapper (https://localhost:4200/src/app/context/FluentProviderWrapper.tsx:22:17)
at QueryClientProvider (https://localhost:4200/node_modules/.vite/deps/@tanstack_react-query.js?v=5638002c:2642:3)
at ErrorContextProvider (https://localhost:4200/src/app/context/ErrorContext.tsx:20:40)
at AuthenticatedTemplate (https://localhost:4200/node_modules/.vite/deps/@azure_msal-react.js?v=7e9f978c:221:34)
at MsalAuthenticationTemplate (https://localhost:4200/node_modules/.vite/deps/@azure_msal-react.js?v=7e9f978c:465:39)
at MsalProvider (https://localhost:4200/node_modules/.vite/deps/@azure_msal-react.js?v=7e9f978c:131:25)
at AuthContextProvider (https://localhost:4200/src/app/context/AuthContext.tsx?t=1714488769684:35:39)
at ErrorBoundary (https://localhost:4200/node_modules/.vite/deps/react-error-boundary.js?v=1ce3fe28:18:5)
at App

MSAL Logs

No response

Network Trace (Preferrably Fiddler)

  • Sent
  • Pending

MSAL Configuration

{
  auth: {
    clientId: import.meta.env.VITE_CLIENTID,
    authority: 'https://login.microsoftonline.com/' + import.meta.env.VITE_TENANT
    redirectUri: import.meta.env.VITE_URL,
  },
  cache: {
    cacheLocation: 'sessionStorage',
    storeAuthStateInCookie: false
  },
  system: {
    pollIntervalMilliseconds: 30,
  },
};

Relevant Code Snippets

export const ReactRouterWrapper = () => {
  const loader = async () => {
    await pca.acquireTokenSilent();

    return null;
  };

  const router = createBrowserRouter(
    createRoutesFromElements(
      <Route path="/" loader={loader} shouldRevalidate={() => true} />
    )
  );

  return <RouterProvider router={router} />;
};

Reproduction Steps

  1. Use React Router inside of the MsalProvider and render the Router only after the user is authenticated.
  2. Use a loader function of the root rout (or any other route) and use the instance of the PublicClientApplication that was provided to the MsalProvider.

Expected Behavior

Allow the usage of the nethods of a PublicClientApplication inside a loader of a react router route.

Identity Provider

Entra ID (formerly Azure AD) / MSA

Browsers Affected (Select all that apply)

Chrome

Regression

No response

Source

Internal (Microsoft)

@Robstei Robstei added bug-unconfirmed A reported bug that needs to be investigated and confirmed question Customer is asking for a clarification, use case or information. labels Apr 30, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Attention 👋 Awaiting response from the MSAL.js team label Apr 30, 2024
@github-actions github-actions bot added msal-browser Related to msal-browser package msal-react Related to @azure/msal-react public-client Issues regarding PublicClientApplications labels Apr 30, 2024
@tnorling
Copy link
Collaborator

It works without error when using two instances of the PublicClientApplication and provide one as a prop to the MsalProvider and use the other instance inside the loader function.
Is this considered a bad pattern? Are there recommendations to use acquireTokenSilent during rendering of other components.

Yes please use only one instance of PublicClientApplication. acquireTokenSilent should only be invoked after inProgress == None. Alternatively you might consider wrapping your RouterProvider in one of the components provided by msal-react such as the AuthenticatedTemplate or MsalAuthenticationTemplate

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Author Feedback Awaiting response from issue author and removed Needs: Attention 👋 Awaiting response from the MSAL.js team labels Apr 30, 2024
@Robstei
Copy link
Author

Robstei commented May 2, 2024

Thanks for the fast answer.

The recommended way to redirect is a loader .

We are rendering everything inside AuthenticatedTemplate already.

You don't have access to hooks in a loader and my understanding is that the loader is executed during rendering.

Is there a way to get the token from the instance of PublicClientApplication that was provided to the MsalProvider that does not update the state of the MsalProvider.

Otherwise my conclusion would be that the redirect pattern from the react router docs can't be used if one also needs to call acquireTokenSilent to determine if a redirect is necessary.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Awaiting response from the MSAL.js team and removed Needs: Author Feedback Awaiting response from issue author labels May 2, 2024
@tnorling
Copy link
Collaborator

tnorling commented May 2, 2024

acquireTokenSilent does not update state, however, there are several state changes at the beginning of the render cycle of MsalProvider, which I assume are the ones causing the problems here. You might consider delaying the render of the component that defines the loader until the MsalProvider is done with its initial renders.

We are rendering everything inside AuthenticatedTemplate already.

If you're rendering everything inside AuthenticatedTemplate I'm not sure I understand why you additionally want to build conditional routing logic. AuthenticatedTemplate and your conditional routing serve similar purposes and I'd suggest doing one or the other, AuthenticatedTemplate being the preferred method. Can you help me understand why the AuthenticatedTemplate isn't enough on its own?

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Author Feedback Awaiting response from issue author and removed Needs: Attention 👋 Awaiting response from the MSAL.js team labels May 2, 2024
@Robstei
Copy link
Author

Robstei commented May 5, 2024

I went through the code with the chrome debugger and here is what i think is happening (I might be wrong):

  1. The MsalContext registers a callback in the instance of the PublicClientApplication
  2. We use the PublicClientApplication instance to call acquireTokenSilent from the controller
  3. The controller calls acquireTokenSIlentAsync
  4. An Event is emmitted and the result gets returned
  5. The callback registered by the MsalProvider is called
  6. The reducer function in the MsalProvider is called and it returns the same value as before..
  7. This gives an error since react does not allow to que state changes from other components during rendering and at this point does not check if the state actually changed. I made an example reproduction to validate this. Notice that there is an error in the console.

If you're rendering everything inside AuthenticatedTemplate I'm not sure I understand why you additionally want to build conditional routing logic. AuthenticatedTemplate and your conditional routing serve similar purposes and I'd suggest doing one or the other, AuthenticatedTemplate being the preferred method. Can you help me understand why the AuthenticatedTemplate isn't enough on its own?

Sure. We develope a teams tab application that is supposed to work as a standalone aswell. We use msal for the standalone version. If a user logs in they might have to go through an onboarding process / accept new information. We determine if that is needed by comparing the up to date graph api data with what we have in our database.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Awaiting response from the MSAL.js team and removed Needs: Author Feedback Awaiting response from issue author labels May 5, 2024
@EldinHb
Copy link

EldinHb commented Jul 12, 2024

I ran into the same problem. Did you by any chance find a workaround or is it on Microsoft's radar to fix this issue?

@Robstei
Copy link
Author

Robstei commented Jul 13, 2024

I am not at work right now and can't check, but if I remember correctly, I used the same instance of PublicClientApplication in the loader and called (and awaited) publicClientApplication.initialize() in the loader again, and the error disappeared.

I did not understand why this worked. The Router is rendered inside the MsalProvider, which is why the instance of PublicClientApplication should already be initialized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-unconfirmed A reported bug that needs to be investigated and confirmed msal-browser Related to msal-browser package msal-react Related to @azure/msal-react Needs: Attention 👋 Awaiting response from the MSAL.js team public-client Issues regarding PublicClientApplications question Customer is asking for a clarification, use case or information.
Projects
None yet
Development

No branches or pull requests

3 participants