mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
feat(auth): warn on a Secure-cookie / plain-HTTP transport mismatch at login
The most common self-host login loop is a Secure session cookie issued over plain HTTP: the browser drops it and bounces the user back to the sign-in page with no error. Detect it at cookie-mint time (detectInsecureCookieTransport) and emit a clear warn-log plus a session_cookie_secure_transport_mismatch wide-event, pointing at SESSION_COOKIE_SECURE=false / X-Forwarded-Proto. Non-fatal; login still completes.
This commit is contained in:
1 parent
ced4e45272
commit
50031911c2
3 files changed
+131
-3
No files matched your search
@@ -1,6 +1,13 @@
|
||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||
|
||||
import { shouldEmitSecureCookie } from "../secure-cookie";
|
||||
import {
|
||||
shouldEmitSecureCookie,
|
||||
detectInsecureCookieTransport,
|
||||
} from "../secure-cookie";
|
||||
|
||||
function req(headers: Record<string, string>, url = "http://10.0.0.5:3000/x") {
|
||||
return { headers: new Headers(headers), url };
|
||||
}
|
||||
|
||||
describe("shouldEmitSecureCookie", () => {
|
||||
afterEach(() => {
|
||||
@@ -47,3 +54,56 @@ describe("shouldEmitSecureCookie", () => {
|
||||
expect(shouldEmitSecureCookie()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectInsecureCookieTransport", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
it("returns null when the Secure flag is off (SESSION_COOKIE_SECURE=false)", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "false");
|
||||
expect(detectInsecureCookieTransport(req({}))).toBeNull();
|
||||
});
|
||||
|
||||
it("warns when a Secure cookie will be set on a plain-HTTP request (no proxy)", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "");
|
||||
const msg = detectInsecureCookieTransport(req({}));
|
||||
expect(msg).toContain("SESSION_COOKIE_SECURE=false");
|
||||
});
|
||||
|
||||
it("returns null behind a TLS proxy that forwards X-Forwarded-Proto: https", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "");
|
||||
expect(
|
||||
detectInsecureCookieTransport(req({ "x-forwarded-proto": "https" })),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("reads the left-most entry of a comma-separated proto chain", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "");
|
||||
expect(
|
||||
detectInsecureCookieTransport(
|
||||
req({ "x-forwarded-proto": "https, http" }),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("warns when the proxy forwards X-Forwarded-Proto: http explicitly", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "");
|
||||
expect(
|
||||
detectInsecureCookieTransport(req({ "x-forwarded-proto": "http" })),
|
||||
).not.toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when the request URL is already https", () => {
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("SESSION_COOKIE_SECURE", "");
|
||||
expect(
|
||||
detectInsecureCookieTransport(req({}, "https://healthlog.example.com/x")),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,8 @@ import {
|
||||
} from "@/lib/auth/native-client";
|
||||
import { issueAccessAndRefresh } from "@/lib/auth/refresh-token";
|
||||
import { recordSignInDevice } from "@/lib/auth/login-alert";
|
||||
import { detectInsecureCookieTransport } from "@/lib/auth/secure-cookie";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import type { User } from "@/generated/prisma/client";
|
||||
|
||||
export interface FinishLoginParams {
|
||||
@@ -116,8 +118,20 @@ export async function finishLogin(
|
||||
});
|
||||
}
|
||||
|
||||
// Browser path — session cookie. `createSession` anchors the onboarding
|
||||
// cookie itself; thread the user's onboarding state through.
|
||||
// Browser path — session cookie. Before minting it, surface the single
|
||||
// most common self-host snag: a Secure cookie about to be set on a plain-HTTP
|
||||
// request drops silently and loops the login. Warn-log + wide-event so an
|
||||
// operator sees the cause; the login still completes.
|
||||
const transportWarning = detectInsecureCookieTransport(request);
|
||||
if (transportWarning) {
|
||||
console.warn(
|
||||
`[auth] session-cookie transport mismatch: ${transportWarning}`,
|
||||
);
|
||||
annotate({ meta: { session_cookie_secure_transport_mismatch: true } });
|
||||
}
|
||||
|
||||
// `createSession` anchors the onboarding cookie itself; thread the user's
|
||||
// onboarding state through.
|
||||
await createSession(
|
||||
user.id,
|
||||
user.onboardingCompletedAt == null,
|
||||
|
||||
@@ -28,3 +28,57 @@ export function shouldEmitSecureCookie(): boolean {
|
||||
if (override === "true") return true;
|
||||
return process.env.NODE_ENV === "production";
|
||||
}
|
||||
|
||||
/**
|
||||
* One-line diagnostic for the single most common self-host login-loop cause:
|
||||
* the app is about to set a `Secure` session cookie
|
||||
* (`shouldEmitSecureCookie() === true`) while the request itself arrived over
|
||||
* plain HTTP. The browser silently drops a `Secure` cookie on an `http://`
|
||||
* origin, so the next request is unauthenticated and the user bounces back to
|
||||
* the login page with no error at all — it reads like a wrong password forever.
|
||||
*
|
||||
* Returns a human-readable message when the mismatch is detected, else `null`.
|
||||
* The caller logs it at login time (`console.warn` survives the prod
|
||||
* `console.*` strip) so an operator sees WHY login loops without having to find
|
||||
* the note buried in three docs. Non-fatal: the login still completes; the
|
||||
* cookie just won't stick until `SESSION_COOKIE_SECURE=false` is set (or TLS is
|
||||
* fronted).
|
||||
*
|
||||
* A request counts as HTTPS when `x-forwarded-proto` names https (the shape
|
||||
* every documented TLS reverse proxy sets) OR the request URL is already
|
||||
* https. A proxy that terminates TLS but omits `x-forwarded-proto` can produce
|
||||
* a benign false positive — the message names that case so it stays actionable
|
||||
* either way.
|
||||
*/
|
||||
export function detectInsecureCookieTransport(req: {
|
||||
headers: Headers;
|
||||
url: string;
|
||||
}): string | null {
|
||||
if (!shouldEmitSecureCookie()) return null;
|
||||
|
||||
// `x-forwarded-proto` can be a comma-separated proto chain; the left-most
|
||||
// entry is the client-facing scheme the browser actually spoke.
|
||||
const clientProto = req.headers
|
||||
.get("x-forwarded-proto")
|
||||
?.split(",")[0]
|
||||
?.trim()
|
||||
.toLowerCase();
|
||||
if (clientProto === "https") return null;
|
||||
|
||||
let urlIsHttps = false;
|
||||
try {
|
||||
urlIsHttps = new URL(req.url).protocol === "https:";
|
||||
} catch {
|
||||
urlIsHttps = false;
|
||||
}
|
||||
if (urlIsHttps) return null;
|
||||
|
||||
return (
|
||||
"This request arrived over plain HTTP but the session cookie is being " +
|
||||
"issued with the `Secure` flag — the browser will drop it and login will " +
|
||||
"loop back to the sign-in page with no error message. If you serve HTTP " +
|
||||
"directly (LAN / NAS / Tailscale, no TLS reverse proxy), set " +
|
||||
"SESSION_COOKIE_SECURE=false. If a reverse proxy terminates TLS in front " +
|
||||
"of this container, make sure it forwards `X-Forwarded-Proto: https`."
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user