Skip to content

Fix useIsAuthenticated hook briefly returning false even though the user is authenticated #7057

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

Merged
merged 9 commits into from
Apr 26, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix useIsAuthenticated returning incorrect value during useEffect update #7057",
"packageName": "@azure/msal-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
12 changes: 4 additions & 8 deletions lib/msal-react/src/hooks/useIsAuthenticated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { useState, useEffect } from "react";
import { useMemo } from "react";
import { useMsal } from "./useMsal";
import { AccountIdentifiers } from "../types/AccountIdentifiers";
import { AccountInfo, InteractionStatus } from "@azure/msal-browser";
Expand Down Expand Up @@ -32,16 +32,12 @@ function isAuthenticated(
export function useIsAuthenticated(matchAccount?: AccountIdentifiers): boolean {
const { accounts: allAccounts, inProgress } = useMsal();

const [hasAuthenticated, setHasAuthenticated] = useState<boolean>(() => {
const isUserAuthenticated = useMemo(() => {
if (inProgress === InteractionStatus.Startup) {
return false;
}
return isAuthenticated(allAccounts, matchAccount);
});
}, [allAccounts, inProgress, matchAccount]);

useEffect(() => {
setHasAuthenticated(isAuthenticated(allAccounts, matchAccount));
}, [allAccounts, matchAccount]);

return hasAuthenticated;
return isUserAuthenticated;
}
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ describe("MsalAuthenticationTemplate tests", () => {
expect(acquireTokenPopupSpy).toHaveBeenCalledTimes(1)
);
expect(
screen.queryByText("This text will always display.")
await screen.findByText("This text will always display.")
).toBeInTheDocument();
expect(
screen.queryByText("A user is authenticated!")
Expand Down Expand Up @@ -697,16 +697,18 @@ describe("MsalAuthenticationTemplate tests", () => {
return Promise.resolve();
});

render(
<MsalProvider instance={pca}>
<p>This text will always display.</p>
<MsalAuthenticationTemplate
interactionType={InteractionType.Redirect}
>
<span> A user is authenticated!</span>
</MsalAuthenticationTemplate>
</MsalProvider>
);
await act(async () => {
render(
<MsalProvider instance={pca}>
<p>This text will always display.</p>
<MsalAuthenticationTemplate
interactionType={InteractionType.Redirect}
>
<span> A user is authenticated!</span>
</MsalAuthenticationTemplate>
</MsalProvider>
);
});

await waitFor(() =>
expect(handleRedirectSpy).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -765,7 +767,7 @@ describe("MsalAuthenticationTemplate tests", () => {
);
await waitFor(() => expect(ssoSilentSpy).toHaveBeenCalledTimes(1));
expect(
screen.queryByText("This text will always display.")
await screen.findByText("This text will always display.")
).toBeInTheDocument();
expect(
screen.queryByText("A user is authenticated!")
Expand Down Expand Up @@ -863,7 +865,7 @@ describe("MsalAuthenticationTemplate tests", () => {
await waitFor(() =>
expect(acquireTokenSilentSpy).toHaveBeenCalledTimes(1)
);
act(() => {
await act(async () => {
const eventMessage: EventMessage = {
eventType: EventType.ACQUIRE_TOKEN_SUCCESS,
interactionType: InteractionType.Redirect,
Expand Down Expand Up @@ -923,7 +925,9 @@ describe("MsalAuthenticationTemplate tests", () => {
await waitFor(() =>
expect(acquireTokenSilentSpy).toHaveBeenCalledTimes(1)
);
expect(screen.queryByText("Error Occurred")).toBeInTheDocument();
expect(
await screen.findByText("Error Occurred")
).toBeInTheDocument();
expect(
screen.queryByText("This text will always display.")
).toBeInTheDocument();
Expand Down
82 changes: 82 additions & 0 deletions lib/msal-react/test/hooks/useIsAuthenticated.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import React from "react";
import { render, screen, waitFor, act } from "@testing-library/react";
import "@testing-library/jest-dom";
import { Configuration, PublicClientApplication } from "@azure/msal-browser";
import { TEST_CONFIG, testAccount } from "../TestConstants";
import { MsalProvider, useIsAuthenticated, withMsal } from "../../src/index";

describe("useIsAuthenticated tests", () => {
let pca: PublicClientApplication;
const msalConfig: Configuration = {
auth: {
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
},
system: {
allowNativeBroker: false,
},
};

let handleRedirectSpy: jest.SpyInstance;

beforeEach(() => {
pca = new PublicClientApplication(msalConfig);
handleRedirectSpy = jest.spyOn(pca, "handleRedirectPromise");
jest.spyOn(pca, "getAllAccounts").mockImplementation(() => []);
});

afterEach(() => {
jest.clearAllMocks();
});

test("useAuthenticated always returns true if user has an account", async () => {
const invalidAuthStateCallback = jest.fn();

const testComponent = ({ ...props }) => {
const isAuth = useIsAuthenticated();
const accounts = props.msalContext.accounts;

if (accounts.length > 0 && !isAuth) {
invalidAuthStateCallback();
}

return (
<>
<p>This component has been wrapped by msal</p>
{accounts.length === 0 && <p>No accounts</p>}
{accounts.length > 0 && <p>Has accounts</p>}
{isAuth && <p>Is authed</p>}
{!isAuth && <p>Not authed</p>}
</>
);
};

const WrappedComponent = withMsal(testComponent);
const { rerender } = render(
<MsalProvider instance={pca}>
<WrappedComponent />
</MsalProvider>
);

await waitFor(() => expect(handleRedirectSpy).toHaveBeenCalledTimes(1));

expect(await screen.findByText("No accounts")).toBeInTheDocument();
expect(await screen.findByText("Not authed")).toBeInTheDocument();

const pcaWithAccounts = new PublicClientApplication(msalConfig);
jest.spyOn(pcaWithAccounts, "getAllAccounts").mockImplementation(() => [
testAccount,
]);

await act(async () =>
rerender(
<MsalProvider instance={pcaWithAccounts}>
<WrappedComponent />
</MsalProvider>
)
);

expect(await screen.findByText("Has accounts")).toBeInTheDocument();
expect(await screen.findByText("Is authed")).toBeInTheDocument();
expect(invalidAuthStateCallback).toHaveBeenCalledTimes(0);
});
});
Loading