Skip to content

Commit c39d25c

Browse files
authored
fix(auth): prevent re-entrant logout calls on session expiry (#7779)
# Motivation When the session expires, `logout()` calls `authStore.signOut()` which sets identity to `null`. This triggers reactive cascades where in-flight services call `getAuthenticatedIdentity()`, see the missing identity, and each independently call `logout()` again. Each call appends duplicate URL params via `.append()` before the browser reloads, which can lead to oversized headers (HTTP 431). Related to: https://forum.dfinity.org/t/proposal-140767-to-upgrade-the-nns-dapp-2026-03-06/65145/4?u=yhabib # Changes - Added a `logoutInProgress` re-entrancy guard to `logout()` so only the first call proceeds. - Switched `appendMsgToUrl` from `searchParams.append()` to `searchParams.set()` to prevent duplicate URL params as a secondary defense. # Tests - Added a test verifying concurrent logout calls only trigger one `signOut` and one `reload`. # Todos - [x] Accessibility (a11y) – any impact? - [x] Changelog – is it needed?
1 parent f6985a7 commit c39d25c

File tree

4 files changed

+34
-2
lines changed

4 files changed

+34
-2
lines changed

.config/spellcheck.dic

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,4 @@ nns-functions
7777
SEV
7878
SNP
7979
dropdowns
80+
params

CHANGELOG-Nns-Dapp-unreleased.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ proposal is successful, the changes it released will be moved from this file to
2424

2525
#### Fixed
2626

27+
- Fix re-entrant logout calls causing duplicate URL params and browser freeze on session expiry.
28+
2729
#### Security
2830

2931
#### Not Published

frontend/src/lib/services/auth.services.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { startBusy } from "$lib/stores/busy.store";
44
import { toastsError, toastsShow } from "$lib/stores/toasts.store";
55
import type { ToastMsg } from "$lib/types/toast";
66
import { replaceHistory } from "$lib/utils/route.utils";
7+
import { registerCleanupForTesting } from "$lib/utils/test-support.utils";
78
import type { ToastLevel } from "@dfinity/gix-components";
89
import type { Identity } from "@icp-sdk/core/agent";
910
import { AnonymousIdentity } from "@icp-sdk/core/agent";
@@ -12,6 +13,12 @@ import { get } from "svelte/store";
1213
const msgParam = "msg";
1314
const levelParam = "level";
1415

16+
let logoutInProgress = false;
17+
18+
registerCleanupForTesting(() => {
19+
logoutInProgress = false;
20+
});
21+
1522
export const login = async () => {
1623
const onError = (err: unknown) => {
1724
if (err === "UserInterrupt") {
@@ -33,6 +40,14 @@ export const logout = async ({
3340
}: {
3441
msg?: Pick<ToastMsg, "labelKey" | "level">;
3542
}) => {
43+
// Prevent re-entrant logout calls. When authStore.signOut() sets identity
44+
// to null, reactive cascades can cause multiple services to detect the
45+
// missing identity and each independently call logout() again, appending
46+
// duplicate URL params before the browser reloads.
47+
if (logoutInProgress) return;
48+
49+
logoutInProgress = true;
50+
3651
// To mask not operational UI (a side effect of sometimes slow JS loading after window.reload because of service worker and no cache).
3752
startBusy({ initiator: "logout" });
3853

@@ -95,8 +110,8 @@ const appendMsgToUrl = (msg: Pick<ToastMsg, "labelKey" | "level">) => {
95110

96111
const url: URL = new URL(window.location.href);
97112

98-
url.searchParams.append(msgParam, encodeURI(labelKey));
99-
url.searchParams.append(levelParam, level);
113+
url.searchParams.set(msgParam, encodeURI(labelKey));
114+
url.searchParams.set(levelParam, level);
100115

101116
replaceHistory(url);
102117
};

frontend/src/tests/lib/services/auth.services.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,20 @@ describe("auth-services", () => {
185185

186186
spy.mockClear();
187187
});
188+
189+
it("should not call logout twice when called concurrently", async () => {
190+
const reloadSpy = vi.spyOn(window.location, "reload");
191+
const signOutSpy = vi.spyOn(authStore, "signOut");
192+
193+
await Promise.all([
194+
logout({ msg: { labelKey: "warning.auth_sign_out", level: "warn" } }),
195+
logout({ msg: { labelKey: "warning.auth_sign_out", level: "warn" } }),
196+
logout({ msg: { labelKey: "error.missing_identity", level: "error" } }),
197+
]);
198+
199+
expect(signOutSpy).toHaveBeenCalledTimes(1);
200+
expect(reloadSpy).toHaveBeenCalledTimes(1);
201+
});
188202
});
189203

190204
describe("getCurrentIdentity", () => {

0 commit comments

Comments
 (0)