Fix: keep the session when the auth server is briefly unreachable
Any transient failure used to end the session: the token route deleted the refresh cookies on every non-OK answer from the OAuth endpoint, refreshAccessToken logged out on any non-OK status or network error, and the startup restore evicted the account and deleted its session cookie. A server restart, a proxy hiccup, a Wi-Fi switch or a laptop waking before the network is back all kicked the user out despite "stay signed in". Failures are now classified. Only a definitive rejection (400/401/403 from the OAuth endpoint, 401 from the token route) tears the session down and deletes cookies, exactly as before. Network errors and 5xx keep the session: the token refresh re-arms itself and retries every ~30 seconds until the server is back, and the startup restore keeps the account, marked unreachable - the same treatment the rate-limit carve-out (#104) already applies. Token validity stays entirely server-enforced: the first definitive 401 after an outage still logs out as before.
This commit is contained in:
@@ -82,9 +82,16 @@ export async function PUT(request: NextRequest) {
|
||||
if (!tokenResponse.ok) {
|
||||
const errorText = await tokenResponse.text();
|
||||
logger.error('Token refresh failed', { status: tokenResponse.status, error: errorText });
|
||||
cookieStore.delete(cookieName);
|
||||
cookieStore.delete(refreshTokenServerCookieName(slot));
|
||||
return NextResponse.json({ error: 'Refresh failed' }, { status: 401 });
|
||||
// Drop the refresh token only when the server definitively rejected it
|
||||
// (invalid/expired/revoked grant). A 5xx or 429 is an outage - keeping
|
||||
// the cookie lets the session resume once the server is back.
|
||||
const status = tokenResponse.status;
|
||||
if (status === 400 || status === 401 || status === 403) {
|
||||
cookieStore.delete(cookieName);
|
||||
cookieStore.delete(refreshTokenServerCookieName(slot));
|
||||
return NextResponse.json({ error: 'Refresh failed' }, { status: 401 });
|
||||
}
|
||||
return NextResponse.json({ error: 'Token endpoint unavailable' }, { status: 503 });
|
||||
}
|
||||
|
||||
const tokens = await tokenResponse.json();
|
||||
|
||||
@@ -55,7 +55,7 @@ describe('auth-store logout redirects', () => {
|
||||
expect(fetchMock).toHaveBeenCalledWith('/api/auth/session?slot=0', { method: 'DELETE', keepalive: true });
|
||||
});
|
||||
|
||||
it('marks session expiry, preserves the current path, and redirects to login on refresh failure', async () => {
|
||||
it('marks session expiry, preserves the current path, and redirects to login when the refresh is rejected (401)', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const fetchMock = vi.fn(async (input: FetchInput, init?: FetchInit) => {
|
||||
@@ -63,7 +63,7 @@ describe('auth-store logout redirects', () => {
|
||||
const method = init?.method ?? 'GET';
|
||||
|
||||
if (url === '/api/auth/token?slot=0' && method === 'PUT') {
|
||||
return { ok: false, json: async () => ({}) };
|
||||
return { ok: false, status: 401, json: async () => ({}) };
|
||||
}
|
||||
|
||||
if (url === '/api/auth/token?slot=0' && method === 'DELETE') {
|
||||
@@ -94,4 +94,74 @@ describe('auth-store logout redirects', () => {
|
||||
expect(sessionStorage.getItem('redirect_after_login')).toBe('/en/calendar?view=day');
|
||||
expect(replaceSpy).toHaveBeenCalledWith('/en/login');
|
||||
});
|
||||
|
||||
it('keeps the session and schedules a retry when the token endpoint is unavailable (5xx)', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const fetchMock = vi.fn(async (input: FetchInput, init?: FetchInit) => {
|
||||
const url = String(input);
|
||||
const method = init?.method ?? 'GET';
|
||||
|
||||
if (url === '/api/auth/token?slot=0' && method === 'PUT') {
|
||||
return { ok: false, status: 503, json: async () => ({}) };
|
||||
}
|
||||
|
||||
throw new Error(`Unexpected fetch call: ${method} ${url}`);
|
||||
});
|
||||
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const replaceSpy = vi.spyOn(browserNavigation, 'replaceWindowLocation').mockImplementation(() => {});
|
||||
|
||||
useAuthStore.setState({
|
||||
isAuthenticated: true,
|
||||
authMode: 'oauth',
|
||||
activeAccountId: null,
|
||||
});
|
||||
|
||||
const token = await useAuthStore.getState().refreshAccessToken();
|
||||
|
||||
expect(token).toBeNull();
|
||||
expect(useAuthStore.getState().isAuthenticated).toBe(true);
|
||||
expect(sessionStorage.getItem('session_expired')).toBeNull();
|
||||
expect(replaceSpy).not.toHaveBeenCalled();
|
||||
|
||||
// A retry is armed: advancing past the ~30 s window fires a second PUT.
|
||||
await vi.advanceTimersByTimeAsync(31_000);
|
||||
const refreshPuts = fetchMock.mock.calls.filter(
|
||||
([input, init]) => String(input) === '/api/auth/token?slot=0' && init?.method === 'PUT',
|
||||
);
|
||||
expect(refreshPuts.length).toBe(2);
|
||||
expect(useAuthStore.getState().isAuthenticated).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps the session when the refresh request fails with a network error', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const fetchMock = vi.fn(async (input: FetchInput, init?: FetchInit) => {
|
||||
const url = String(input);
|
||||
const method = init?.method ?? 'GET';
|
||||
|
||||
if (url === '/api/auth/token?slot=0' && method === 'PUT') {
|
||||
throw new TypeError('Failed to fetch');
|
||||
}
|
||||
|
||||
throw new Error(`Unexpected fetch call: ${method} ${url}`);
|
||||
});
|
||||
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const replaceSpy = vi.spyOn(browserNavigation, 'replaceWindowLocation').mockImplementation(() => {});
|
||||
|
||||
useAuthStore.setState({
|
||||
isAuthenticated: true,
|
||||
authMode: 'oauth',
|
||||
activeAccountId: null,
|
||||
});
|
||||
|
||||
const token = await useAuthStore.getState().refreshAccessToken();
|
||||
|
||||
expect(token).toBeNull();
|
||||
expect(useAuthStore.getState().isAuthenticated).toBe(true);
|
||||
expect(sessionStorage.getItem('session_expired')).toBeNull();
|
||||
expect(replaceSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
+63
-8
@@ -75,6 +75,32 @@ function isRateLimitError(error: unknown): error is RateLimitError {
|
||||
return error instanceof RateLimitError;
|
||||
}
|
||||
|
||||
// An auth/session endpoint answered with a server-side error (5xx) - an
|
||||
// outage, not a rejection of our credentials.
|
||||
class TransientAuthError extends Error {
|
||||
constructor(message: string, readonly status: number) {
|
||||
super(`${message}: ${status}`);
|
||||
}
|
||||
}
|
||||
|
||||
// True when a restore/refresh attempt failed because the server could not be
|
||||
// reached (network error) or answered 5xx (restart, maintenance, proxy
|
||||
// hiccup). Such failures must keep the account and its cookies - "stay signed
|
||||
// in" has to survive downtime and offline spells. Only a definitive rejection
|
||||
// (401/400) may evict. Mirrors the rate-limit carve-out (#104).
|
||||
function isTransientAuthError(error: unknown): boolean {
|
||||
if (error instanceof TransientAuthError) return true;
|
||||
// fetch() rejects with TypeError when the network is unreachable.
|
||||
if (error instanceof TypeError) return true;
|
||||
// JMAPClient.connect()/refreshSession() embed the HTTP status in the
|
||||
// message - a 5xx there is the server being down, not an auth failure.
|
||||
if (error instanceof Error) {
|
||||
const m = error.message.match(/(?:Failed to get session|Session refresh failed): (\d{3})/);
|
||||
if (m) return m[1].startsWith('5');
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function getClientRateLimitState(client: IJMAPClient | null): Pick<AuthState, 'isRateLimited' | 'rateLimitUntil'> {
|
||||
if (!client) {
|
||||
return { isRateLimited: false, rateLimitUntil: null };
|
||||
@@ -294,6 +320,10 @@ const clients = new Map<string, JMAPClient>();
|
||||
const refreshTimers = new Map<string, ReturnType<typeof setTimeout>>();
|
||||
const refreshPromises = new Map<string, Promise<string | null>>();
|
||||
|
||||
// Pseudo-expiry passed to scheduleRefresh when a refresh failed transiently:
|
||||
// the "expiry - 60s" math below turns 90 into a retry in ~30 seconds.
|
||||
const TOKEN_REFRESH_RETRY_SECONDS = 90;
|
||||
|
||||
function scheduleRefresh(expiresIn: number, refreshFn: () => Promise<string | null>, accountId?: string): void {
|
||||
if (accountId) {
|
||||
const existing = refreshTimers.get(accountId);
|
||||
@@ -948,9 +978,18 @@ export const useAuthStore = create<AuthState>()(
|
||||
const res = await apiFetch(`/api/auth/token?slot=${slot}`, { method: 'PUT' });
|
||||
|
||||
if (!res.ok) {
|
||||
notifyParent('sso:session-expired');
|
||||
markSessionExpired();
|
||||
get().logout();
|
||||
// Only a definitive 401 ends the session. Anything else (5xx
|
||||
// while the server restarts, proxy errors) is an outage - keep
|
||||
// the session and retry shortly so "stay signed in" survives
|
||||
// maintenance windows and offline spells.
|
||||
if (res.status === 401) {
|
||||
notifyParent('sso:session-expired');
|
||||
markSessionExpired();
|
||||
get().logout();
|
||||
return null;
|
||||
}
|
||||
debug.error(`Token refresh unavailable (${res.status}), retrying shortly`);
|
||||
scheduleRefresh(TOKEN_REFRESH_RETRY_SECONDS, get().refreshAccessToken, accountId ?? undefined);
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -975,10 +1014,10 @@ export const useAuthStore = create<AuthState>()(
|
||||
scheduleRefresh(expires_in, get().refreshAccessToken, accountId ?? undefined);
|
||||
return access_token;
|
||||
} catch (error) {
|
||||
debug.error('Token refresh failed:', error);
|
||||
notifyParent('sso:session-expired');
|
||||
markSessionExpired();
|
||||
get().logout();
|
||||
// Network failure (offline, Wi-Fi switch, server unreachable) -
|
||||
// not a rejection. Keep the session and retry shortly.
|
||||
debug.error('Token refresh failed, retrying shortly:', error);
|
||||
scheduleRefresh(TOKEN_REFRESH_RETRY_SECONDS, get().refreshAccessToken, accountId ?? undefined);
|
||||
return null;
|
||||
} finally {
|
||||
refreshPromise = null;
|
||||
@@ -1374,6 +1413,8 @@ export const useAuthStore = create<AuthState>()(
|
||||
scheduleRefresh(expires_in, get().refreshAccessToken, account.id);
|
||||
await syncStalwartAuthContext(account.serverUrl, account.username, client.getAuthHeader(), account.cookieSlot);
|
||||
accountStore.updateAccount(account.id, { isConnected: true, hasError: false });
|
||||
} else if (res.status >= 500) {
|
||||
throw new TransientAuthError('Token refresh failed', res.status);
|
||||
} else {
|
||||
throw new Error(`Token refresh failed: ${res.status}`);
|
||||
}
|
||||
@@ -1387,6 +1428,8 @@ export const useAuthStore = create<AuthState>()(
|
||||
clients.set(account.id, client);
|
||||
await syncStalwartAuthContext(serverUrl, username, client.getAuthHeader(), account.cookieSlot);
|
||||
accountStore.updateAccount(account.id, { isConnected: true, hasError: false });
|
||||
} else if (res.status >= 500) {
|
||||
throw new TransientAuthError('Session restore failed', res.status);
|
||||
} else {
|
||||
throw new Error(`Session cookie missing: ${res.status}`);
|
||||
}
|
||||
@@ -1401,6 +1444,18 @@ export const useAuthStore = create<AuthState>()(
|
||||
});
|
||||
continue;
|
||||
}
|
||||
// Outage or offline - keep the account (and its cookies) so the
|
||||
// session resumes once the server is reachable again. Same
|
||||
// treatment as the rate-limit case above; only a definitive
|
||||
// rejection below evicts.
|
||||
if (isTransientAuthError(err)) {
|
||||
accountStore.updateAccount(account.id, {
|
||||
isConnected: false,
|
||||
hasError: true,
|
||||
errorMessage: 'Server unreachable',
|
||||
});
|
||||
continue;
|
||||
}
|
||||
// Remove unrestorable accounts so the user is prompted to log in
|
||||
// again rather than seeing a stale error entry forever.
|
||||
evictAccount(account.id);
|
||||
@@ -1627,7 +1682,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
}
|
||||
} catch (error) {
|
||||
debug.error('Basic session restore failed:', error);
|
||||
if (isRateLimitError(error)) {
|
||||
if (isRateLimitError(error) || isTransientAuthError(error)) {
|
||||
set({ isLoading: false, error: 'connection_failed', isRateLimited: false, rateLimitUntil: null });
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user