Skip to content

Commit 872c705

Browse files
tnorlingjo-arroyo
tnorling
and
jo-arroyo
authored
Fix react initialization race (#7720)
Fixes a race condition that can occur between cache initialization and react state initialization Fixes #7654, #7561 --------- Co-authored-by: Jo Arroyo <[email protected]>
1 parent e83ce9d commit 872c705

File tree

15 files changed

+231
-32
lines changed

15 files changed

+231
-32
lines changed

‎change/@azure-msal-browser-6911f6de-1b4b-48fc-9ac9-f298b28461bc.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix throw when attempting to getAccount before initialization #7720",
4+
"packageName": "@azure/msal-browser",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-msal-react-652ee5f7-5382-4665-b7b0-63e35a83d49c.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix race between cache initialization and state initialization",
4+
"packageName": "@azure/msal-react",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎lib/msal-browser/src/cache/LocalStorage.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ export class LocalStorage implements IWindowStorage<string> {
7777
}
7878

7979
async initialize(correlationId: string): Promise<void> {
80-
this.initialized = true;
81-
8280
const cookies = new CookieStorage();
8381
const cookieString = cookies.getItem(ENCRYPTION_KEY);
8482
let parsedCookie = { key: "", id: "" };
@@ -158,6 +156,8 @@ export class LocalStorage implements IWindowStorage<string> {
158156

159157
// Register listener for cache updates in other tabs
160158
this.broadcast.addEventListener("message", this.updateCache.bind(this));
159+
160+
this.initialized = true;
161161
}
162162

163163
getItem(key: string): string | null {

‎lib/msal-browser/test/app/PublicClientApplication.spec.ts

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6607,6 +6607,33 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
66076607
expect(accounts).toEqual([]);
66086608
});
66096609

6610+
it("getAllAccounts throws if called before initialize", (done) => {
6611+
pca = new PublicClientApplication({
6612+
auth: {
6613+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6614+
},
6615+
cache: {
6616+
cacheLocation: "localStorage",
6617+
},
6618+
});
6619+
6620+
window.localStorage.setItem(
6621+
"msal.account.keys",
6622+
JSON.stringify([testAccount1.generateAccountKey()])
6623+
);
6624+
6625+
try {
6626+
pca.getAllAccounts();
6627+
} catch (e) {
6628+
expect(e).toEqual(
6629+
createBrowserAuthError(
6630+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6631+
)
6632+
);
6633+
done();
6634+
}
6635+
});
6636+
66106637
it("getAccountByUsername returns account specified", () => {
66116638
const account = pca.getAccountByUsername(
66126639
ID_TOKEN_CLAIMS.preferred_username
@@ -6642,6 +6669,33 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
66426669
expect(account).toBe(null);
66436670
});
66446671

6672+
it("getAccountByUsername throws if called before initialize", (done) => {
6673+
pca = new PublicClientApplication({
6674+
auth: {
6675+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6676+
},
6677+
cache: {
6678+
cacheLocation: "localStorage",
6679+
},
6680+
});
6681+
6682+
window.localStorage.setItem(
6683+
"msal.account.keys",
6684+
JSON.stringify([testAccount1.generateAccountKey()])
6685+
);
6686+
6687+
try {
6688+
pca.getAccountByUsername(testAccount1.username);
6689+
} catch (e) {
6690+
expect(e).toEqual(
6691+
createBrowserAuthError(
6692+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6693+
)
6694+
);
6695+
done();
6696+
}
6697+
});
6698+
66456699
it("getAccountByHomeId returns account specified", () => {
66466700
const account = pca.getAccountByHomeId(
66476701
testAccountInfo1.homeAccountId
@@ -6661,6 +6715,33 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
66616715
expect(account).toBe(null);
66626716
});
66636717

6718+
it("getAccountByUsername throws if called before initialize", (done) => {
6719+
pca = new PublicClientApplication({
6720+
auth: {
6721+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6722+
},
6723+
cache: {
6724+
cacheLocation: "localStorage",
6725+
},
6726+
});
6727+
6728+
window.localStorage.setItem(
6729+
"msal.account.keys",
6730+
JSON.stringify([testAccount1.generateAccountKey()])
6731+
);
6732+
6733+
try {
6734+
pca.getAccountByHomeId(testAccount1.homeAccountId);
6735+
} catch (e) {
6736+
expect(e).toEqual(
6737+
createBrowserAuthError(
6738+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6739+
)
6740+
);
6741+
done();
6742+
}
6743+
});
6744+
66646745
it("getAccountByLocalId returns account specified", () => {
66656746
const account = pca.getAccountByLocalId(ID_TOKEN_CLAIMS.oid);
66666747
expect(account?.idTokenClaims).not.toBeUndefined();
@@ -6678,12 +6759,66 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
66786759
expect(account).toBe(null);
66796760
});
66806761

6762+
it("getAccountByLocalId throws if called before initialize", (done) => {
6763+
pca = new PublicClientApplication({
6764+
auth: {
6765+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6766+
},
6767+
cache: {
6768+
cacheLocation: "localStorage",
6769+
},
6770+
});
6771+
6772+
window.localStorage.setItem(
6773+
"msal.account.keys",
6774+
JSON.stringify([testAccount1.generateAccountKey()])
6775+
);
6776+
6777+
try {
6778+
pca.getAccountByLocalId(testAccount1.localAccountId);
6779+
} catch (e) {
6780+
expect(e).toEqual(
6781+
createBrowserAuthError(
6782+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6783+
)
6784+
);
6785+
done();
6786+
}
6787+
});
6788+
66816789
describe("getAccount", () => {
66826790
it("getAccount returns null if empty filter is passed in", () => {
66836791
const account = pca.getAccount({});
66846792
expect(account).toBe(null);
66856793
});
66866794

6795+
it("getAccount throws if called before initialize", (done) => {
6796+
pca = new PublicClientApplication({
6797+
auth: {
6798+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6799+
},
6800+
cache: {
6801+
cacheLocation: "localStorage",
6802+
},
6803+
});
6804+
6805+
window.localStorage.setItem(
6806+
"msal.account.keys",
6807+
JSON.stringify([testAccount1.generateAccountKey()])
6808+
);
6809+
6810+
try {
6811+
pca.getAccount({ username: testAccount1.username });
6812+
} catch (e) {
6813+
expect(e).toEqual(
6814+
createBrowserAuthError(
6815+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6816+
)
6817+
);
6818+
done();
6819+
}
6820+
});
6821+
66876822
describe("loginHint filter", () => {
66886823
it("getAccount returns account specified using login_hint", () => {
66896824
const account = pca.getAccount({
@@ -6814,6 +6949,40 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
68146949
expect(pca.getActiveAccount()).toBe(null);
68156950
});
68166951

6952+
it("getActiveAccount throws if called before initialize", (done) => {
6953+
pca = new PublicClientApplication({
6954+
auth: {
6955+
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
6956+
},
6957+
cache: {
6958+
cacheLocation: "localStorage",
6959+
},
6960+
});
6961+
6962+
window.localStorage.setItem(
6963+
`msal.${TEST_CONFIG.MSAL_CLIENT_ID}.active-account-filters`,
6964+
JSON.stringify({
6965+
homeAccountId: testAccount1.homeAccountId,
6966+
localAccountId: testAccount1.localAccountId,
6967+
})
6968+
);
6969+
window.localStorage.setItem(
6970+
"msal.account.keys",
6971+
JSON.stringify([testAccount1.generateAccountKey()])
6972+
);
6973+
6974+
try {
6975+
pca.getActiveAccount();
6976+
} catch (e) {
6977+
expect(e).toEqual(
6978+
createBrowserAuthError(
6979+
BrowserAuthErrorCodes.uninitializedPublicClientApplication
6980+
)
6981+
);
6982+
done();
6983+
}
6984+
});
6985+
68176986
it("setActiveAccount() sets the active account local id value correctly", () => {
68186987
expect(pca.getActiveAccount()).toBe(null);
68196988
pca.setActiveAccount(testAccountInfo1);

‎lib/msal-react/src/MsalProvider.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ const reducer = (
8585
throw new Error(`Unknown action type: ${type}`);
8686
}
8787

88+
if (newInProgress === InteractionStatus.Startup) {
89+
// Can't start checking accounts until initialization is complete
90+
return previousState;
91+
}
92+
8893
const currentAccounts = payload.instance.getAllAccounts();
8994
if (
9095
newInProgress !== previousState.inProgress &&
@@ -135,7 +140,7 @@ export function MsalProvider({
135140
// Lazy initialization of the initial state
136141
return {
137142
inProgress: InteractionStatus.Startup,
138-
accounts: instance.getAllAccounts(),
143+
accounts: [],
139144
};
140145
});
141146

‎lib/msal-react/src/hooks/useAccount.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
AccountInfo,
99
IPublicClientApplication,
1010
AccountEntity,
11+
InteractionStatus,
1112
} from "@azure/msal-browser";
1213
import { useMsal } from "./useMsal.js";
1314
import { AccountIdentifiers } from "../types/AccountIdentifiers.js";
@@ -42,26 +43,32 @@ export function useAccount(
4243
): AccountInfo | null {
4344
const { instance, inProgress, logger } = useMsal();
4445

45-
const [account, setAccount] = useState<AccountInfo | null>(() =>
46-
getAccount(instance, accountIdentifiers)
47-
);
46+
const [account, setAccount] = useState<AccountInfo | null>(() => {
47+
if (inProgress === InteractionStatus.Startup) {
48+
return null;
49+
} else {
50+
return getAccount(instance, accountIdentifiers);
51+
}
52+
});
4853

4954
useEffect(() => {
50-
setAccount((currentAccount: AccountInfo | null) => {
51-
const nextAccount = getAccount(instance, accountIdentifiers);
52-
if (
53-
!AccountEntity.accountInfoIsEqual(
54-
currentAccount,
55-
nextAccount,
56-
true
57-
)
58-
) {
59-
logger.info("useAccount - Updating account");
60-
return nextAccount;
61-
}
55+
if (inProgress !== InteractionStatus.Startup) {
56+
setAccount((currentAccount: AccountInfo | null) => {
57+
const nextAccount = getAccount(instance, accountIdentifiers);
58+
if (
59+
!AccountEntity.accountInfoIsEqual(
60+
currentAccount,
61+
nextAccount,
62+
true
63+
)
64+
) {
65+
logger.info("useAccount - Updating account");
66+
return nextAccount;
67+
}
6268

63-
return currentAccount;
64-
});
69+
return currentAccount;
70+
});
71+
}
6572
}, [inProgress, accountIdentifiers, instance, logger]);
6673

6774
return account;

‎lib/msal-react/src/hooks/useMsalAuthentication.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,8 @@ export function useMsalAuthentication(
253253
shouldAcquireToken.current &&
254254
inProgress === InteractionStatus.None
255255
) {
256-
shouldAcquireToken.current = false;
257256
if (!isAuthenticated) {
257+
shouldAcquireToken.current = false;
258258
logger.info(
259259
"useMsalAuthentication - No user is authenticated, attempting to login"
260260
);
@@ -263,6 +263,7 @@ export function useMsalAuthentication(
263263
return;
264264
});
265265
} else if (account) {
266+
shouldAcquireToken.current = false;
266267
logger.info(
267268
"useMsalAuthentication - User is authenticated, attempting to acquire token"
268269
);

‎samples/msal-react-samples/b2c-sample/src/ui-components/WelcomeName.jsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { useEffect, useState } from "react";
22
import { useMsal } from "@azure/msal-react";
3+
import { InteractionStatus } from "@azure/msal-browser";
34
import Typography from "@mui/material/Typography";
45

56
const WelcomeName = () => {
6-
const { instance } = useMsal();
7+
const { instance, inProgress } = useMsal();
78
const [name, setName] = useState(null);
89

9-
const activeAccount = instance.getActiveAccount();
10+
const activeAccount = inProgress === InteractionStatus.None ? instance.getActiveAccount() : null;
1011
useEffect(() => {
1112
if (activeAccount && activeAccount.name) {
1213
setName(activeAccount.name.split(' ')[0]);

‎samples/msal-react-samples/nextjs-sample/src/ui-components/WelcomeName.jsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { useEffect, useState } from "react";
22
import { useMsal } from "@azure/msal-react";
3+
import { InteractionStatus } from "@azure/msal-browser";
34
import Typography from "@mui/material/Typography";
45

56
const WelcomeName = () => {
6-
const { instance } = useMsal();
7+
const { instance, inProgress } = useMsal();
78
const [name, setName] = useState(null);
89

9-
const activeAccount = instance.getActiveAccount();
10+
const activeAccount = inProgress === InteractionStatus.None ? instance.getActiveAccount() : null;
1011
useEffect(() => {
1112
if (activeAccount && activeAccount.name) {
1213
setName(activeAccount.name.split(' ')[0]);

‎samples/msal-react-samples/nextjs-sample/test/profile.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ async function verifyTokenStore(
2424
"b5c2e510-4a17-4feb-b219-e55aa5b74144"
2525
);
2626
expect(telemetryCacheEntry).not.toBeNull;
27-
expect(telemetryCacheEntry["cacheHits"]).toBe(1);
27+
expect(telemetryCacheEntry["cacheHits"]).toBeGreaterThanOrEqual(1);
2828
}
2929

3030
describe("/profile", () => {

‎samples/msal-react-samples/react-router-sample/src/ui-components/WelcomeName.jsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { useEffect, useState } from "react";
22
import { useMsal } from "@azure/msal-react";
3+
import { InteractionStatus } from "@azure/msal-browser";
34
import Typography from "@mui/material/Typography";
45

56
const WelcomeName = () => {
6-
const { instance } = useMsal();
7+
const { instance, inProgress } = useMsal();
78
const [name, setName] = useState(null);
89

9-
const activeAccount = instance.getActiveAccount();
10+
const activeAccount = inProgress === InteractionStatus.None ? instance.getActiveAccount() : null;
1011
useEffect(() => {
1112
if (activeAccount) {
1213
setName(activeAccount.name.split(' ')[0]);

0 commit comments

Comments
 (0)