mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-13 14:32:03 +00:00
fix(auth): end the session only on HealthLog's own 401 or 403
A refetch of the account that answered 401 or 403 now signs the person out and clears the offline copies, which is right for HealthLog's own answer. But a self-hosted instance behind a forward-auth proxy or a firewall can get a 401 or 403 that HealthLog never wrote, and that signed a person out whose session was fine. HealthLog's error envelope always names a code, so only a 401 or 403 that carries one is read as the session's end; one without is treated like a network failure and keeps the last known state. The tests now give the account route the answer the server really sends.
This commit is contained in:
1 parent
37734c9fd8
commit
510ce3128d
3 files changed
+72
-25
No files matched your search
@@ -72,6 +72,13 @@ function shellTree(queryClient: QueryClient) {
|
||||
);
|
||||
}
|
||||
|
||||
/** HealthLog's answer once there is no session; it always names a code. */
|
||||
const SIGNED_OUT = {
|
||||
status: 401,
|
||||
error: "Unauthorized",
|
||||
meta: { errorCode: "auth.missing" },
|
||||
};
|
||||
|
||||
async function mountShell(user: Parameters<typeof authUser>[0] | null) {
|
||||
return renderWithApp(
|
||||
<AuthShell>
|
||||
@@ -91,10 +98,7 @@ afterEach(() => {
|
||||
|
||||
describe("AuthShell on a sign-in page", () => {
|
||||
it("clears the offline copies once the server says there is no session", async () => {
|
||||
const fetchStub = stubFetch().on("GET", ME, {
|
||||
status: 401,
|
||||
error: "Unauthorized",
|
||||
});
|
||||
const fetchStub = stubFetch().on("GET", ME, SIGNED_OUT);
|
||||
openAt("/auth/login");
|
||||
const { queryClient } = await mountShell(null);
|
||||
|
||||
@@ -108,7 +112,7 @@ describe("AuthShell on a sign-in page", () => {
|
||||
});
|
||||
|
||||
it("does the same on the profile take-over page", async () => {
|
||||
stubFetch().on("GET", ME, { status: 401, error: "Unauthorized" });
|
||||
stubFetch().on("GET", ME, SIGNED_OUT);
|
||||
openAt("/auth/claim");
|
||||
await mountShell(null);
|
||||
await expect.poll(() => clearOffline.mock.calls.length).toBe(1);
|
||||
@@ -139,8 +143,22 @@ describe("AuthShell on a sign-in page", () => {
|
||||
expect(clearOffline).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("leaves them alone for a 401 from something in front of the server", async () => {
|
||||
// A forward-auth proxy or a firewall answers with its own page, which
|
||||
// names no HealthLog code and says nothing about this session.
|
||||
const fetchStub = stubFetch().on("GET", ME, {
|
||||
status: 401,
|
||||
error: "Unauthorized",
|
||||
});
|
||||
openAt("/auth/login");
|
||||
await mountShell(null);
|
||||
await expect.poll(() => fetchStub.calls("GET", ME).length).toBe(1);
|
||||
await settle();
|
||||
expect(clearOffline).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not clear on a public page outside the sign-in flow", async () => {
|
||||
stubFetch().on("GET", ME, { status: 401, error: "Unauthorized" });
|
||||
stubFetch().on("GET", ME, SIGNED_OUT);
|
||||
openAt("/about");
|
||||
await mountShell(null);
|
||||
await settle();
|
||||
@@ -153,9 +171,7 @@ describe("AuthShell on a protected page", () => {
|
||||
async function signedInAt(path: string) {
|
||||
let session = true;
|
||||
stubFetch().on("GET", ME, () =>
|
||||
session
|
||||
? { data: authUser({ id: "signed-in" }) }
|
||||
: { status: 401, error: "Unauthorized" },
|
||||
session ? { data: authUser({ id: "signed-in" }) } : SIGNED_OUT,
|
||||
);
|
||||
openAt(path);
|
||||
const view = await mountShell(null);
|
||||
|
||||
@@ -55,6 +55,11 @@ import {
|
||||
useAuth,
|
||||
} from "../use-auth";
|
||||
|
||||
/** What `/api/auth/me` answers once the session is gone. */
|
||||
const SESSION_GONE = new ApiError("Unauthorized", 401, {
|
||||
errorCode: "auth.missing",
|
||||
});
|
||||
|
||||
// Node test env has no localStorage — back it with a Map.
|
||||
const store = new Map<string, string>();
|
||||
beforeEach(() => {
|
||||
@@ -97,9 +102,20 @@ describe("isAuthVerdictUnknown — failure classification", () => {
|
||||
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);
|
||||
it("treats HealthLog's 401 and 403 as the real session-end verdict", () => {
|
||||
expect(isAuthVerdictUnknown(SESSION_GONE)).toBe(false);
|
||||
expect(
|
||||
isAuthVerdictUnknown(
|
||||
new ApiError("forbidden", 403, { errorCode: "http.forbidden" }),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("treats a 401 or 403 without HealthLog's envelope as no verdict", () => {
|
||||
// A forward-auth proxy or a firewall in front of the instance answers
|
||||
// with its own page; the session behind it may be fine.
|
||||
expect(isAuthVerdictUnknown(new ApiError("Unauthorized", 401))).toBe(true);
|
||||
expect(isAuthVerdictUnknown(new ApiError("Forbidden", 403))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -128,7 +144,7 @@ describe("useAuth — last-known state on a non-verdict failure", () => {
|
||||
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);
|
||||
queryState.error = SESSION_GONE;
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(false);
|
||||
@@ -141,7 +157,7 @@ describe("useAuth — a verdict on a refetch", () => {
|
||||
store.set("healthlog-was-authenticated", "1");
|
||||
queryState.data = { id: "u1", username: "someone" };
|
||||
queryState.isError = true;
|
||||
queryState.error = new ApiError("unauthorized", 401);
|
||||
queryState.error = SESSION_GONE;
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(false);
|
||||
@@ -149,6 +165,16 @@ describe("useAuth — a verdict on a refetch", () => {
|
||||
expect(auth.isAuthUnknown).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the cached account through a 401 from something in front of the server", () => {
|
||||
queryState.data = { id: "u1", username: "someone" };
|
||||
queryState.isError = true;
|
||||
queryState.error = new ApiError("Unauthorized", 401);
|
||||
|
||||
const auth = useAuth();
|
||||
expect(auth.isAuthenticated).toBe(true);
|
||||
expect(auth.user).toEqual({ id: "u1", username: "someone" });
|
||||
});
|
||||
|
||||
it("keeps the cached account through a network failure on a refetch", () => {
|
||||
queryState.data = { id: "u1", username: "someone" };
|
||||
queryState.isError = true;
|
||||
|
||||
+16
-11
@@ -338,18 +338,22 @@ function clearWasAuthenticated(): void {
|
||||
* 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.
|
||||
* browser reporting itself offline, a server-side 5xx, or a 401/403 that
|
||||
* HealthLog did not write. None of these mean "logged out"; only HealthLog's
|
||||
* own 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;
|
||||
// other non-auth status: the server never ruled on the session.
|
||||
if (error.status !== 401 && error.status !== 403) return true;
|
||||
// HealthLog's error envelope always names a code. A 401/403 without one
|
||||
// comes from something in front of the instance (a forward-auth proxy,
|
||||
// a firewall), which says nothing about this session.
|
||||
return typeof error.meta?.errorCode !== "string";
|
||||
}
|
||||
// Everything below the HTTP layer is transport-level by construction:
|
||||
// fetch `TypeError` (offline / DNS / connection refused), `AbortError` /
|
||||
@@ -528,7 +532,7 @@ export function useAuth() {
|
||||
});
|
||||
|
||||
// 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
|
||||
// it. Only HealthLog's own 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
|
||||
@@ -536,8 +540,9 @@ export function useAuth() {
|
||||
// /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);
|
||||
// A 401/403 on a refetch ends the session although the cache still holds
|
||||
// the last good payload: React Query keeps `data` when a refetch fails.
|
||||
// HealthLog's 401/403 on a refetch ends the session although the cache
|
||||
// still holds the last good payload: React Query keeps `data` when a
|
||||
// refetch fails.
|
||||
const sessionEnded = query.isError && !verdictUnknown;
|
||||
const user = sessionEnded ? null : (query.data ?? null);
|
||||
const isAuthenticated = !!user || (verdictUnknown && readWasAuthenticated());
|
||||
|
||||
Reference in new issue
Block a user