src-advisory/SRCmail
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: refactor logout to use synchronous flow with full page redirect

Browse Source 
- Rewrite logout() from async to synchronous to prevent React re-renders with stale state
- Replace router.push('/login') with redirectToLogin() (window.location.replace) in all page auth guards for reliable navigation in Edge/Safari
- Add performFullLogout() helper that clears auth state, feature stores, and localStorage
- Fix persist middleware partialize to return {} when not authenticated, preventing state resurrection
- Use keepalive fetch for background cookie/token cleanup so redirect fires immediately
- Remove unused useRouter imports from page.tsx and contacts/page.tsx
- Simplify all page logout handlers to directly call logout()

Fixes #63
This commit is contained in:
Linus Rath
2026-03-21 20:45:17 +01:00
parent c73940e22a
commit 4c2d185be4
7 changed files with 104 additions and 233 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/[locale]/calendar/page.tsx
+3 -3
View File
@@ -11,7 +11,7 @@ import {
} from "date-fns";
import { useCalendarStore } from "@/stores/calendar-store";
import { isCalendarViewMode } from "@/stores/calendar-store";
import { useAuthStore } from "@/stores/auth-store";
import { useAuthStore, redirectToLogin } from "@/stores/auth-store";
import { useEmailStore } from "@/stores/email-store";
import { useSettingsStore } from "@/stores/settings-store";
import { useIdentityStore } from "@/stores/identity-store";
@@ -105,7 +105,7 @@ export default function CalendarPage() {
useEffect(() => {
if (initialCheckDone && !isAuthenticated && !authLoading) {
try { sessionStorage.setItem('redirect_after_login', window.location.pathname); } catch { /* ignore */ }
router.push("/login");
redirectToLogin();
} else if (client && !supportsCalendar) {
router.push("/");
}
@@ -721,7 +721,7 @@ export default function CalendarPage() {
collapsed
quota={quota}
isPushConnected={isPushConnected}
onLogout={() => { logout(); if (!useAuthStore.getState().isAuthenticated) router.push('/login'); }}
onLogout={logout}
onManageApps={handleManageApps}
onInlineApp={handleInlineApp}
onCloseInlineApp={closeInlineApp}
app/[locale]/contacts/page.tsx
+4 -6
View File
@@ -1,7 +1,6 @@
"use client";
import { useState, useEffect, useCallback, useRef, useMemo } from "react";
import { useRouter } from "@/i18n/navigation";
import { useTranslations } from "next-intl";
import { ArrowLeft, Users } from "lucide-react";
import { Button } from "@/components/ui/button";
@@ -16,7 +15,7 @@ import { ContactsSidebar, type ContactCategory } from "@/components/contacts/con
import { ContactImportDialog } from "@/components/contacts/contact-import-dialog";
import { exportContacts } from "@/components/contacts/contact-export";
import { useContactStore, getContactDisplayName } from "@/stores/contact-store";
import { useAuthStore } from "@/stores/auth-store";
import { useAuthStore, redirectToLogin } from "@/stores/auth-store";
import { useEmailStore } from "@/stores/email-store";
import { toast } from "@/stores/toast-store";
import { cn } from "@/lib/utils";
@@ -39,7 +38,6 @@ type View =
| "bulk-add-to-group";
export default function ContactsPage() {
const router = useRouter();
const t = useTranslations("contacts");
const { client, isAuthenticated, logout, checkAuth, isLoading: authLoading } = useAuthStore();
const { showAppsModal, inlineApp, loadedApps, handleManageApps, handleInlineApp, closeInlineApp, closeAppsModal } = useSidebarApps();
@@ -109,9 +107,9 @@ export default function ContactsPage() {
useEffect(() => {
if (initialCheckDone && !isAuthenticated && !authLoading) {
try { sessionStorage.setItem('redirect_after_login', window.location.pathname); } catch { /* ignore */ }
router.push("/login");
redirectToLogin();
}
}, [initialCheckDone, isAuthenticated, authLoading, router]);
}, [initialCheckDone, isAuthenticated, authLoading]);
useEffect(() => {
if (client && supportsSync && !hasFetched.current) {
@@ -594,7 +592,7 @@ export default function ContactsPage() {
collapsed
quota={quota}
isPushConnected={isPushConnected}
onLogout={() => { logout(); if (!useAuthStore.getState().isAuthenticated) router.push('/login'); }}
onLogout={logout}
onManageApps={handleManageApps}
onInlineApp={handleInlineApp}
onCloseInlineApp={closeInlineApp}
app/[locale]/files/page.tsx
+4 -4
View File
@@ -7,7 +7,7 @@ import { ArrowLeft } from "lucide-react";
import { Button } from "@/components/ui/button";
import { ConfirmDialog } from "@/components/ui/confirm-dialog";
import { useConfirmDialog } from "@/hooks/use-confirm-dialog";
import { useAuthStore } from "@/stores/auth-store";
import { useAuthStore, redirectToLogin } from "@/stores/auth-store";
import { useEmailStore } from "@/stores/email-store";
import { useFileStore } from "@/stores/file-store";
import { toast } from "@/stores/toast-store";
@@ -112,9 +112,9 @@ export default function FilesPage() {
useEffect(() => {
if (initialCheckDone && !isAuthenticated && !authLoading) {
try { sessionStorage.setItem('redirect_after_login', window.location.pathname); } catch { /* ignore */ }
router.push("/login");
redirectToLogin();
}
}, [initialCheckDone, isAuthenticated, authLoading, router]);
}, [initialCheckDone, isAuthenticated, authLoading]);
// Initialize JMAP files client
useEffect(() => {
@@ -357,7 +357,7 @@ export default function FilesPage() {
collapsed
quota={quota}
isPushConnected={isPushConnected}
onLogout={() => { logout(); if (!useAuthStore.getState().isAuthenticated) router.push('/login'); }}
onLogout={logout}
onManageApps={handleManageApps}
onInlineApp={handleInlineApp}
onCloseInlineApp={closeInlineApp}
app/[locale]/page.tsx
+4 -11
View File
@@ -1,7 +1,6 @@
"use client";
import { useEffect, useState, useRef, useMemo, useCallback } from "react";
import { useRouter } from "@/i18n/navigation";
import { useTranslations } from "next-intl";
import { Sidebar } from "@/components/layout/sidebar";
import { EmailList } from "@/components/email/email-list";
@@ -13,7 +12,7 @@ import { MobileHeader, MobileViewerHeader } from "@/components/layout/mobile-hea
import { ThreadGroup, Email } from "@/lib/jmap/types";
import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal";
import { useEmailStore } from "@/stores/email-store";
import { useAuthStore } from "@/stores/auth-store";
import { useAuthStore, redirectToLogin } from "@/stores/auth-store";
import { useSettingsStore } from "@/stores/settings-store";
import { useIdentityStore } from "@/stores/identity-store";
import { useUIStore } from "@/stores/ui-store";
@@ -48,7 +47,6 @@ import { Button } from "@/components/ui/button";
import { useConfig } from "@/hooks/use-config";
export default function Home() {
const router = useRouter();
const t = useTranslations();
const tCommon = useTranslations('common');
const { appName } = useConfig();
@@ -285,9 +283,9 @@ export default function Home() {
useEffect(() => {
if (initialCheckDone && !isAuthenticated && !authLoading) {
try { sessionStorage.setItem('redirect_after_login', window.location.pathname); } catch { /* ignore */ }
router.push('/login');
redirectToLogin();
}
}, [initialCheckDone, isAuthenticated, authLoading, router]);
}, [initialCheckDone, isAuthenticated, authLoading]);
// Load mailboxes and emails when authenticated (only if not already loaded)
useEffect(() => {
@@ -768,12 +766,7 @@ export default function Home() {
}
};
const handleLogout = () => {
logout();
if (!useAuthStore.getState().isAuthenticated) {
router.push('/login');
}
};
const handleLogout = logout;
const handleSearch = async (query: string) => {
if (!client) return;
app/[locale]/settings/page.tsx
+5 -5
View File
@@ -44,7 +44,7 @@ import { FilesSettingsComponent } from '@/components/settings/files-settings';
import { ContactsSettings } from '@/components/settings/contacts-settings';
import { SmimeSettings } from '@/components/settings/smime-settings';
import { SidebarAppsSettings } from '@/components/settings/sidebar-apps-settings';
import { useAuthStore } from '@/stores/auth-store';
import { useAuthStore, redirectToLogin } from '@/stores/auth-store';
import { useEmailStore } from '@/stores/email-store';
import { useIsDesktop } from '@/hooks/use-media-query';
import { NavigationRail } from '@/components/layout/navigation-rail';
@@ -122,9 +122,9 @@ export default function SettingsPage() {
useEffect(() => {
if (initialCheckDone && !isAuthenticated && !authLoading) {
try { sessionStorage.setItem('redirect_after_login', window.location.pathname); } catch { /* ignore */ }
router.push('/login');
redirectToLogin();
}
}, [initialCheckDone, isAuthenticated, authLoading, router]);
}, [initialCheckDone, isAuthenticated, authLoading]);
if (!isAuthenticated) {
return null;
@@ -286,7 +286,7 @@ export default function SettingsPage() {
{/* Logout */}
<div className="border-t border-border px-5 py-3">
<button
onClick={() => { logout(); if (!useAuthStore.getState().isAuthenticated) router.push('/login'); }}
onClick={logout}
className="w-full flex items-center gap-3 py-2.5 text-sm text-destructive hover:bg-muted rounded-md px-2 transition-colors duration-150"
>
<LogOut className="w-4 h-4" />
@@ -317,7 +317,7 @@ export default function SettingsPage() {
collapsed
quota={quota}
isPushConnected={isPushConnected}
onLogout={() => { logout(); if (!useAuthStore.getState().isAuthenticated) router.push('/login'); }}
onLogout={logout}
onManageApps={handleManageApps}
onInlineApp={handleInlineApp}
onCloseInlineApp={closeInlineApp}
components/layout/account-switcher.tsx
-4
View File
@@ -101,15 +101,11 @@ export function AccountSwitcher({ variant = "rail", className }: AccountSwitcher
const handleLogout = () => {
setOpen(false);
logout();
if (useAccountStore.getState().accounts.length === 0) {
router.push("/login" as never);
}
};
const handleLogoutAll = () => {
setOpen(false);
logoutAll();
router.push("/login" as never);
};
const handleSetDefault = (accountId: string) => {
stores/auth-store.ts
+84 -200
View File
@@ -37,7 +37,7 @@ interface AuthState {
loginWithOAuth: (serverUrl: string, code: string, codeVerifier: string, redirectUri: string) => Promise<boolean>;
loginDemo: () => Promise<boolean>;
refreshAccessToken: () => Promise<string | null>;
logout: () => Promise<void>;
logout: () => void;
logoutAll: () => void;
switchAccount: (accountId: string) => Promise<void>;
checkAuth: () => Promise<void>;
@@ -125,7 +125,7 @@ function saveRedirectAfterLogin(): void {
}
}
function redirectToLogin(): void {
export function redirectToLogin(): void {
if (typeof window === 'undefined') return;
const loginPath = getLocaleLoginPath();
@@ -228,6 +228,39 @@ function clearAllRefreshTimers(): void {
refreshPromises.clear();
}
/**
* Synchronously clears all auth and feature store state.
* Called during full logout (no remaining accounts).
*/
function performFullLogout(set: (state: Partial<AuthState>) => void): void {
useSettingsStore.getState().disableSync();
set({
isAuthenticated: false,
isLoading: false,
serverUrl: null,
username: null,
client: null,
identities: [],
primaryIdentity: null,
authMode: 'basic',
rememberMe: false,
accessToken: null,
tokenExpiresAt: null,
connectionLost: false,
error: null,
activeAccountId: null,
isDemoMode: false,
});
clearAllStores();
// Remove persisted state AFTER the final set() so the persist middleware
// doesn't re-write stale values.
try { localStorage.removeItem('auth-storage'); } catch { /* noop */ }
try { localStorage.removeItem('account-storage'); } catch { /* noop */ }
}
export const useAuthStore = create<AuthState>()(
persist(
(set, get) => ({
@@ -576,7 +609,7 @@ export const useAuthStore = create<AuthState>()(
return promise;
},
logout: async () => {
logout: () => {
const state = get();
const wasDemoMode = state.isDemoMode;
const wasOAuth = state.authMode === 'oauth';
@@ -585,39 +618,14 @@ export const useAuthStore = create<AuthState>()(
const account = accountId ? accountStore.getAccountById(accountId) : null;
const slot = account?.cookieSlot ?? 0;
// Demo mode: simple cleanup, no network calls
if (wasDemoMode) {
set({ client: null });
state.client?.disconnect();
set({
isAuthenticated: false,
serverUrl: null,
username: null,
client: null,
identities: [],
primaryIdentity: null,
authMode: 'basic',
rememberMe: false,
accessToken: null,
tokenExpiresAt: null,
connectionLost: false,
error: null,
activeAccountId: null,
isDemoMode: false,
});
localStorage.removeItem('auth-storage');
clearAllStores();
redirectToLogin();
return;
}
// Stop refresh timers immediately
clearRefreshTimer(accountId ?? undefined);
// Null out the client BEFORE disconnecting so the page doesn't fire
// data-loading effects with the stale disconnected client while
// stores are being cleared.
// Disconnect and null out the client BEFORE clearing stores so the
// page doesn't fire data-loading effects with the stale client.
const oldClient = state.client;
set({ client: null });
state.client?.disconnect();
oldClient?.disconnect();
// Remove client from multi-account map
if (accountId) {
@@ -630,61 +638,17 @@ export const useAuthStore = create<AuthState>()(
// Check if there are remaining accounts to switch to
const remainingAccounts = accountStore.accounts;
const shouldRedirectToLogin = remainingAccounts.length === 0;
if (remainingAccounts.length > 0) {
// Switch to the next account
if (remainingAccounts.length > 0 && !wasDemoMode) {
// Switch to the next account — this is the one path that stays in-app
const nextAccount = remainingAccounts[0];
// Clean current stores, then switch
clearAllStores();
// Restore next account
let nextClient = clients.get(nextAccount.id);
// If the client isn't in memory, try to restore it from the session
if (!nextClient) {
try {
if (nextAccount.authMode === 'oauth') {
const res = await fetch(`/api/auth/token?slot=${nextAccount.cookieSlot}`, { method: 'PUT' });
if (res.ok) {
const { access_token, expires_in } = await res.json();
const refreshFn = get().refreshAccessToken;
nextClient = JMAPClient.withBearer(nextAccount.serverUrl, access_token, nextAccount.username, () => refreshFn());
nextClient.onConnectionChange((connected) => {
if (get().activeAccountId === nextAccount.id) {
set({ connectionLost: !connected });
}
accountStore.updateAccount(nextAccount.id, { isConnected: connected });
});
await nextClient.connect();
clients.set(nextAccount.id, nextClient);
scheduleRefresh(expires_in, get().refreshAccessToken, nextAccount.id);
}
} else if (nextAccount.authMode === 'basic' && nextAccount.rememberMe) {
const res = await fetch(`/api/auth/session?slot=${nextAccount.cookieSlot}`);
if (res.ok) {
const { serverUrl: sUrl, username: uName, password: pwd } = await res.json();
nextClient = new JMAPClient(sUrl, uName, pwd);
nextClient.onConnectionChange((connected) => {
if (get().activeAccountId === nextAccount.id) {
set({ connectionLost: !connected });
}
accountStore.updateAccount(nextAccount.id, { isConnected: connected });
});
await nextClient.connect();
clients.set(nextAccount.id, nextClient);
}
}
} catch (err) {
debug.error(`Failed to restore next account ${nextAccount.id} during logout:`, err);
nextClient = undefined;
}
}
const nextClient = clients.get(nextAccount.id);
if (nextClient) {
const restored = restoreAccount(nextAccount.id);
accountStore.setActiveAccount(nextAccount.id);
// Build identity state up front so the name updates atomically
const restoredIdentities = restored ? useIdentityStore.getState().identities : [];
const restoredPrimary = restoredIdentities[0] ?? null;
@@ -711,132 +675,47 @@ export const useAuthStore = create<AuthState>()(
}).catch((err) => debug.error('Failed to load identities after switch:', err));
}
} else {
// Could not restore the next account — remove it and do a full logout
// Client not in memory — clear everything and redirect.
// Trying to async-restore during logout caused the original bug.
debug.error(`Cannot restore next account ${nextAccount.id}, performing full logout`);
evictAccount(nextAccount.id);
accountStore.removeAccount(nextAccount.id);
set({
isAuthenticated: false,
serverUrl: null,
username: null,
client: null,
identities: [],
primaryIdentity: null,
authMode: 'basic',
rememberMe: false,
accessToken: null,
tokenExpiresAt: null,
connectionLost: false,
error: null,
activeAccountId: null,
});
localStorage.removeItem('auth-storage');
clearAllStores();
redirectToLogin();
performFullLogout(set);
}
} else {
// No accounts remaining — full logout
set({
isAuthenticated: false,
serverUrl: null,
username: null,
client: null,
identities: [],
primaryIdentity: null,
authMode: 'basic',
rememberMe: false,
accessToken: null,
tokenExpiresAt: null,
connectionLost: false,
error: null,
activeAccountId: null,
});
localStorage.removeItem('auth-storage');
clearAllStores();
// Background cookie cleanup for the removed account
fetch(`/api/auth/session?slot=${slot}`, { method: 'DELETE', keepalive: true }).catch(() => {});
if (wasOAuth) {
fetch(`/api/auth/token?slot=${slot}`, { method: 'DELETE', keepalive: true }).catch(() => {});
}
return;
}
// Clean up cookies for the removed account
fetch(`/api/auth/session?slot=${slot}`, { method: 'DELETE', keepalive: shouldRedirectToLogin }).catch((err) => {
debug.error('Failed to clear session cookie:', err);
});
// No accounts remaining (or demo mode) — full logout + redirect
performFullLogout(set);
if (wasOAuth && shouldRedirectToLogin) {
let redirectCommitted = false;
const commitLoginRedirect = () => {
if (redirectCommitted) return;
redirectCommitted = true;
redirectToLogin();
};
window.setTimeout(commitLoginRedirect, 0);
fetch(`/api/auth/token?slot=${slot}`, { method: 'DELETE', keepalive: true })
.then((res) => {
if (!res.ok) throw new Error(`Revocation failed: ${res.status}`);
return res.json();
})
.then((data) => {
if (redirectCommitted) return;
if (data.end_session_url) {
redirectCommitted = true;
const locale = window.location.pathname.split('/')[1] || 'en';
const redirectUri = `${window.location.origin}/${locale}/login`;
const url = new URL(data.end_session_url);
url.searchParams.set('post_logout_redirect_uri', redirectUri);
replaceWindowLocation(url.toString());
return;
}
commitLoginRedirect();
})
.catch((err) => {
debug.error('OAuth logout cleanup failed:', err);
commitLoginRedirect();
});
} else if (wasOAuth) {
fetch(`/api/auth/token?slot=${slot}`, { method: 'DELETE', keepalive: false })
.catch((err) => {
debug.error('OAuth logout cleanup failed:', err);
});
} else if (shouldRedirectToLogin) {
redirectToLogin();
// Background cookie/token cleanup — keepalive ensures completion during navigation
if (!wasDemoMode) {
fetch(`/api/auth/session?slot=${slot}`, { method: 'DELETE', keepalive: true }).catch(() => {});
if (wasOAuth) {
fetch(`/api/auth/token?slot=${slot}`, { method: 'DELETE', keepalive: true }).catch(() => {});
}
}
// Redirect to login — this is synchronous and happens AFTER all state is cleared
redirectToLogin();
},
logoutAll: () => {
// Disconnect all clients
for (const client of clients.values()) {
client.disconnect();
for (const c of clients.values()) {
c.disconnect();
}
clients.clear();
clearAllRefreshTimers();
evictAll();
useSettingsStore.getState().disableSync();
useAccountStore.getState().accounts.forEach(() => {});
set({
isAuthenticated: false,
serverUrl: null,
username: null,
client: null,
identities: [],
primaryIdentity: null,
authMode: 'basic',
rememberMe: false,
accessToken: null,
tokenExpiresAt: null,
connectionLost: false,
error: null,
activeAccountId: null,
});
localStorage.removeItem('auth-storage');
clearAllStores();
performFullLogout(set);
// Clear all accounts from registry
const accountStore = useAccountStore.getState();
@@ -845,9 +724,10 @@ export const useAuthStore = create<AuthState>()(
accountStore.removeAccount(account.id);
}
// Delete all cookies
// Background cookie/token cleanup
fetch('/api/auth/session?all=true', { method: 'DELETE', keepalive: true }).catch(() => {});
fetch('/api/auth/token?all=true', { method: 'DELETE', keepalive: true }).catch(() => {});
redirectToLogin();
},
@@ -1302,16 +1182,20 @@ export const useAuthStore = create<AuthState>()(
}),
{
name: 'auth-storage',
partialize: (state) => ({
serverUrl: state.serverUrl,
username: state.username,
authMode: state.authMode,
isAuthenticated: (state.authMode === 'oauth' || state.rememberMe)
? state.isAuthenticated
: undefined,
rememberMe: state.rememberMe,
activeAccountId: state.activeAccountId,
}),
partialize: (state) => {
// Don't persist unauthenticated state — prevents resurrecting stale sessions
if (!state.isAuthenticated) return {};
return {
serverUrl: state.serverUrl,
username: state.username,
authMode: state.authMode,
isAuthenticated: (state.authMode === 'oauth' || state.rememberMe)
? state.isAuthenticated
: undefined,
rememberMe: state.rememberMe,
activeAccountId: state.activeAccountId,
};
},
}
)
);