mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
fix(pwa): stop treating network failure on the auth probe as session end
A failed /api/auth/me now only counts as a session END on an explicit 401/403. Network-level failures (offline relaunch fetch TypeError, the service worker's synthetic 503 offline envelope, abort/timeout) and 5xx carry no session verdict: the hook holds the last-known auth state via a non-sensitive localStorage marker instead of flipping unauthenticated. Pre-fix, an airplane-mode relaunch of the installed PWA redirected to /auth/login and ran clearCachesForSessionEnd(), wiping the SW data/page caches and the IndexedDB query snapshot — the entire offline read path was unreachable in exactly the scenario it exists for. - use-auth: isAuthVerdictUnknown() classification, was-authenticated marker (set on every successful /me, cleared on session end), isAuthUnknown exposed for the shell - auth-shell: the wipe runs only on a real verdict; a non-verdict failure without a prior-session marker redirects without wiping - explicit logout and true 401/403 expiry wipe exactly as before H1
This commit is contained in:
1 parent
c54386e95b
commit
85058716f6
3 files changed
+258
-12
No files matched your search
@@ -48,7 +48,7 @@ export function AuthShell({
|
||||
*/
|
||||
demoMode?: boolean;
|
||||
}) {
|
||||
const { user, isAuthenticated, isLoading } = useAuth();
|
||||
const { user, isAuthenticated, isAuthUnknown, isLoading } = useAuth();
|
||||
const { t } = useTranslations();
|
||||
const pathname = usePathname();
|
||||
const router = useRouter();
|
||||
@@ -129,17 +129,35 @@ export function AuthShell({
|
||||
|
||||
useEffect(() => {
|
||||
if (!isLoading && !isAuthenticated && !isPublicPage) {
|
||||
// Session END (expired / invalidated cookie → `/api/auth/me` 401). Wipe
|
||||
// the in-memory query cache (the cross-user guard — the QueryClient
|
||||
// outlives the navigation on this SPA and the health-data families are
|
||||
// not user-scoped) plus the persisted IndexedDB + SW caches, so nothing
|
||||
// leaks to the next account on a shared browser. Logout does the same
|
||||
// from `useLogout`; this covers the expiry path logout never runs
|
||||
// through. Best-effort; the redirect proceeds regardless.
|
||||
clearCachesForSessionEnd(queryClient);
|
||||
// Session END (expired / invalidated cookie → `/api/auth/me` 401/403).
|
||||
// Wipe the in-memory query cache (the cross-user guard — the
|
||||
// QueryClient outlives the navigation on this SPA and the health-data
|
||||
// families are not user-scoped) plus the persisted IndexedDB + SW
|
||||
// caches, so nothing leaks to the next account on a shared browser.
|
||||
// Logout does the same from `useLogout`; this covers the expiry path
|
||||
// logout never runs through. Best-effort; the redirect proceeds
|
||||
// regardless.
|
||||
//
|
||||
// `isAuthUnknown` = the auth probe failed WITHOUT a session verdict
|
||||
// (offline relaunch, network blip, 5xx) and no prior-session marker
|
||||
// exists. Nothing to protect and nothing to render — send the visitor
|
||||
// to the login gate, but never run the wipe off a non-verdict: the
|
||||
// wipe on a network failure is exactly what used to destroy the
|
||||
// offline caches. (With the marker present, `isAuthenticated` holds
|
||||
// the last-known state and this branch never runs.)
|
||||
if (!isAuthUnknown) {
|
||||
clearCachesForSessionEnd(queryClient);
|
||||
}
|
||||
router.replace("/auth/login");
|
||||
}
|
||||
}, [isLoading, isAuthenticated, isPublicPage, router, queryClient]);
|
||||
}, [
|
||||
isLoading,
|
||||
isAuthenticated,
|
||||
isAuthUnknown,
|
||||
isPublicPage,
|
||||
router,
|
||||
queryClient,
|
||||
]);
|
||||
|
||||
// Redirect non-admins away from /admin
|
||||
useEffect(() => {
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
/**
|
||||
* Offline auth gate — a failed `/api/auth/me` is only a session END when the
|
||||
* server said so (401/403).
|
||||
*
|
||||
* Pre-fix, ANY auth-probe error read as "logged out": an airplane-mode
|
||||
* relaunch redirected to /auth/login and `clearCachesForSessionEnd()` wiped
|
||||
* the SW data/page caches and the IndexedDB query snapshot — destroying
|
||||
* exactly the installed-PWA-opened-offline scenario the offline plumbing
|
||||
* exists for. These tests pin the classification (`isAuthVerdictUnknown`),
|
||||
* the last-known-state fallback (`readWasAuthenticated` marker), and the
|
||||
* marker's session-end lifecycle.
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { QueryClient } from "@tanstack/react-query";
|
||||
|
||||
const clearOfflineCachesForSessionEnd = vi.fn().mockResolvedValue(undefined);
|
||||
vi.mock("@/lib/pwa/query-persister", () => ({
|
||||
clearOfflineCachesForSessionEnd: () => clearOfflineCachesForSessionEnd(),
|
||||
}));
|
||||
|
||||
vi.mock("next/navigation", () => ({
|
||||
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
|
||||
}));
|
||||
vi.mock("@/lib/i18n/context", () => ({
|
||||
useTranslations: () => ({ t: (key: string) => key }),
|
||||
}));
|
||||
|
||||
// Drivable query state so the hook's classification can be exercised for
|
||||
// success / network-error / 401 without a fetch layer.
|
||||
interface MockQueryState {
|
||||
data: unknown;
|
||||
isError: boolean;
|
||||
error: unknown;
|
||||
isLoading: boolean;
|
||||
}
|
||||
let queryState: MockQueryState;
|
||||
vi.mock("@tanstack/react-query", async () => {
|
||||
const actual = await vi.importActual<
|
||||
typeof import("@tanstack/react-query")
|
||||
>("@tanstack/react-query");
|
||||
return {
|
||||
...actual,
|
||||
useQuery: () => ({ ...queryState, refetch: vi.fn() }),
|
||||
useQueryClient: () => ({}),
|
||||
useMutation: () => ({ mutate: () => undefined, isPending: false }),
|
||||
};
|
||||
});
|
||||
|
||||
import { ApiError } from "@/lib/api/api-fetch";
|
||||
import {
|
||||
clearCachesForSessionEnd,
|
||||
isAuthVerdictUnknown,
|
||||
readWasAuthenticated,
|
||||
useAuth,
|
||||
} from "../use-auth";
|
||||
|
||||
// Node test env has no localStorage — back it with a Map.
|
||||
const store = new Map<string, string>();
|
||||
beforeEach(() => {
|
||||
store.clear();
|
||||
queryState = { data: undefined, isError: false, error: null, isLoading: false };
|
||||
vi.stubGlobal("localStorage", {
|
||||
getItem: (k: string) => store.get(k) ?? null,
|
||||
setItem: (k: string, v: string) => void store.set(k, v),
|
||||
removeItem: (k: string) => void store.delete(k),
|
||||
});
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
clearOfflineCachesForSessionEnd.mockClear();
|
||||
});
|
||||
|
||||
describe("isAuthVerdictUnknown — failure classification", () => {
|
||||
it("treats a fetch TypeError (offline / DNS) as no verdict", () => {
|
||||
expect(isAuthVerdictUnknown(new TypeError("Failed to fetch"))).toBe(true);
|
||||
});
|
||||
|
||||
it("treats the SW's synthetic 503 offline envelope as no verdict", () => {
|
||||
expect(
|
||||
isAuthVerdictUnknown(new ApiError("offline", 503, { offline: true })),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats an abort/timeout as no verdict", () => {
|
||||
expect(
|
||||
isAuthVerdictUnknown(new DOMException("timeout", "TimeoutError")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats a 5xx as no verdict — a server hiccup is not a logout", () => {
|
||||
expect(isAuthVerdictUnknown(new ApiError("boom", 500))).toBe(true);
|
||||
});
|
||||
|
||||
it("treats 401 and 403 as the real session-end verdict", () => {
|
||||
expect(isAuthVerdictUnknown(new ApiError("unauthorized", 401))).toBe(false);
|
||||
expect(isAuthVerdictUnknown(new ApiError("forbidden", 403))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useAuth — last-known state on a non-verdict failure", () => {
|
||||
it("stays authenticated on a network failure when a prior session marker exists", () => {
|
||||
store.set("healthlog-was-authenticated", "1");
|
||||
queryState.isError = true;
|
||||
queryState.error = new TypeError("Failed to fetch");
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(true);
|
||||
expect(auth.isAuthUnknown).toBe(true);
|
||||
expect(auth.user).toBeNull();
|
||||
});
|
||||
|
||||
it("reads unauthenticated-but-unknown on a network failure without a marker", () => {
|
||||
queryState.isError = true;
|
||||
queryState.error = new TypeError("Failed to fetch");
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(false);
|
||||
// The shell must NOT run the session-end wipe off this state.
|
||||
expect(auth.isAuthUnknown).toBe(true);
|
||||
});
|
||||
|
||||
it("reads a true 401 as session end — wipe and redirect may proceed", () => {
|
||||
store.set("healthlog-was-authenticated", "1");
|
||||
queryState.isError = true;
|
||||
queryState.error = new ApiError("unauthorized", 401);
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(false);
|
||||
expect(auth.isAuthUnknown).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("session-end marker lifecycle", () => {
|
||||
it("clearCachesForSessionEnd drops the marker so the next cold offline launch lands on the login gate", () => {
|
||||
store.set("healthlog-was-authenticated", "1");
|
||||
expect(readWasAuthenticated()).toBe(true);
|
||||
|
||||
clearCachesForSessionEnd(new QueryClient());
|
||||
|
||||
expect(readWasAuthenticated()).toBe(false);
|
||||
expect(clearOfflineCachesForSessionEnd).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
+86
-2
@@ -9,7 +9,7 @@ import {
|
||||
import { useRouter } from "next/navigation";
|
||||
import { toast } from "sonner";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { apiGet, apiFetchRaw } from "@/lib/api/api-fetch";
|
||||
import { apiGet, apiFetchRaw, ApiError } from "@/lib/api/api-fetch";
|
||||
import { retryOnceOnTransientError } from "@/lib/queries/retry-transient";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import type {
|
||||
@@ -149,6 +149,64 @@ export interface AuthUser {
|
||||
moduleAvailability?: Partial<Record<ModuleKey, boolean>>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Non-sensitive "a session existed here" marker (localStorage boolean, no
|
||||
* identity data). A cold offline launch cannot reach `/api/auth/me`, so the
|
||||
* auth query errors without ever answering the session question — the marker
|
||||
* is the last-known state that lets the shell render the cached app chrome
|
||||
* instead of bouncing to the login gate and wiping the offline caches. Set on
|
||||
* every successful `/api/auth/me`, cleared on every session END (logout and
|
||||
* true 401/403).
|
||||
*/
|
||||
const WAS_AUTHENTICATED_KEY = "healthlog-was-authenticated";
|
||||
|
||||
export function readWasAuthenticated(): boolean {
|
||||
try {
|
||||
return localStorage.getItem(WAS_AUTHENTICATED_KEY) === "1";
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function storeWasAuthenticated(): void {
|
||||
try {
|
||||
localStorage.setItem(WAS_AUTHENTICATED_KEY, "1");
|
||||
} catch {
|
||||
/* storage unavailable — the marker is best-effort */
|
||||
}
|
||||
}
|
||||
|
||||
function clearWasAuthenticated(): void {
|
||||
try {
|
||||
localStorage.removeItem(WAS_AUTHENTICATED_KEY);
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True when an `/api/auth/me` failure carries NO verdict on the session —
|
||||
* a network-level failure (fetch `TypeError` offline, abort/timeout), the
|
||||
* service worker's synthetic 503 offline envelope (`meta.offline`), the
|
||||
* browser reporting itself offline, or a server-side 5xx. None of these mean
|
||||
* "logged out"; only an explicit 401/403 does. Treating them as session end
|
||||
* used to redirect an offline relaunch to /auth/login and wipe the offline
|
||||
* caches — destroying exactly the installed-PWA-opened-offline scenario the
|
||||
* SW + query persister exist for.
|
||||
*/
|
||||
export function isAuthVerdictUnknown(error: unknown): boolean {
|
||||
if (error instanceof ApiError) {
|
||||
// 5xx (incl. the SW's synthetic 503 with `meta.offline: true`) and any
|
||||
// other non-auth status: the server never ruled on the session. Only an
|
||||
// explicit 401/403 is a verdict.
|
||||
return error.status !== 401 && error.status !== 403;
|
||||
}
|
||||
// Everything below the HTTP layer is transport-level by construction:
|
||||
// fetch `TypeError` (offline / DNS / connection refused), `AbortError` /
|
||||
// `TimeoutError` DOMException from the 15 s default signal.
|
||||
return true;
|
||||
}
|
||||
|
||||
async function fetchMe(): Promise<AuthUser> {
|
||||
// v1.16.4 — routed through the typed wrapper; a non-OK /me (401)
|
||||
// throws `ApiError`, which `useAuth` treats as "not authenticated"
|
||||
@@ -183,6 +241,8 @@ async function fetchMe(): Promise<AuthUser> {
|
||||
? data.dateFormat
|
||||
: "AUTO";
|
||||
storeDateFormat(dateFormat);
|
||||
// A confirmed session: refresh the non-sensitive offline-relaunch marker.
|
||||
storeWasAuthenticated();
|
||||
return {
|
||||
...(data as AuthUser),
|
||||
disableCoach: data.disableCoach ?? false,
|
||||
@@ -227,10 +287,30 @@ export function useAuth() {
|
||||
staleTime: 5 * 60 * 1000,
|
||||
});
|
||||
|
||||
// v1.27.x offline fix — classify a failed `/api/auth/me` before acting on
|
||||
// it. Only an explicit 401/403 means "session ended"; a network-level
|
||||
// failure (offline relaunch, SW 503 offline envelope, timeout) or a 5xx
|
||||
// carries no verdict, so the hook holds the LAST-KNOWN auth state (the
|
||||
// localStorage marker) instead of flipping to unauthenticated. Pre-fix, an
|
||||
// airplane-mode relaunch read as a logout: the shell redirected to
|
||||
// /auth/login and wiped every offline cache (data 7→0 entries, pages 1→0
|
||||
// in the audit measurement), making the whole offline story unreachable.
|
||||
const verdictUnknown = query.isError && isAuthVerdictUnknown(query.error);
|
||||
const isAuthenticated =
|
||||
!!query.data || (verdictUnknown && readWasAuthenticated());
|
||||
|
||||
return {
|
||||
user: query.data ?? null,
|
||||
isLoading: query.isLoading,
|
||||
isAuthenticated: !!query.data,
|
||||
isAuthenticated,
|
||||
/**
|
||||
* True while the session question is genuinely unanswered — the auth
|
||||
* probe failed at the transport level (or 5xx) and no cached `/me`
|
||||
* payload exists. The shell uses it to suppress the session-end cache
|
||||
* wipe: a wipe without a 401/403 verdict would destroy the offline
|
||||
* caches on every network blip.
|
||||
*/
|
||||
isAuthUnknown: !query.data && verdictUnknown,
|
||||
error: query.error,
|
||||
refetch: query.refetch,
|
||||
};
|
||||
@@ -257,6 +337,10 @@ export function useAuth() {
|
||||
*/
|
||||
export function clearCachesForSessionEnd(queryClient: QueryClient): void {
|
||||
queryClient.clear();
|
||||
// The offline-relaunch marker is session state too: after a real session
|
||||
// end the next cold offline launch must land on the login gate, not the
|
||||
// cached shell.
|
||||
clearWasAuthenticated();
|
||||
void clearOfflineCachesForSessionEnd();
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user