mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 22:26:13 +00:00
fix(auth): bring a deep link back to its page after signing in
A home-screen shortcut, a notification tap or a bookmark opened without a session landed on the sign-in page with a bare URL, and after signing in the person was on the dashboard instead of the page they asked for. The sign-in page already honoured a sanitised `next`; neither redirect set one. The proxy now passes the request's own path and query as `next` (the bare start page goes without one). The shell does the same when the account answer ends a session on a protected page, except after a deliberate sign-out: the marker a confirmed session leaves is gone by then, so the next person on the device starts from a bare sign-in page rather than the page the previous one left.
This commit is contained in:
1 parent
75b879279f
commit
6c5a6eacbb
7 files changed
+152
-37
No files matched your search
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.",
|
||||
"count": 2892,
|
||||
"total": 9220,
|
||||
"total": 9217,
|
||||
"entries": {
|
||||
"__tests__/account-payload-consumer-guard.test.ts": 2,
|
||||
"__tests__/acting-account-boundary-guard.test.ts": 7,
|
||||
@@ -2892,7 +2892,7 @@
|
||||
"lib/withings/sync.ts": 6,
|
||||
"lib/withings/webhook-handler.ts": 3,
|
||||
"lib/workouts/list-read.ts": 1,
|
||||
"proxy.ts": 27,
|
||||
"proxy.ts": 26,
|
||||
"types/analytics.ts": 8,
|
||||
"types/backups.ts": 1
|
||||
}
|
||||
|
||||
@@ -87,7 +87,7 @@ describe("proxy early exits carry the baseline security headers", () => {
|
||||
|
||||
it("unauthenticated page redirect to /auth/login", () => {
|
||||
const res = proxy(makeRequest("/dashboard"));
|
||||
expect(res.headers.get("location")).toMatch(/\/auth\/login$/);
|
||||
expect(res.headers.get("location")).toMatch(/\/auth\/login(\?|$)/);
|
||||
expectBaselineHeaders(res);
|
||||
});
|
||||
|
||||
|
||||
@@ -82,7 +82,7 @@ describe("proxy.ts onboarding redirect (v1.4.22 C4)", () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers.get("location")).toBeNull();
|
||||
const anonymous = proxy(makeRequest("/insights"));
|
||||
expect(anonymous.headers.get("location")).toMatch(/\/auth\/login$/);
|
||||
expect(anonymous.headers.get("location")).toMatch(/\/auth\/login(\?|$)/);
|
||||
});
|
||||
|
||||
it("passes through when hl_onboarding cookie is absent (completed users)", () => {
|
||||
@@ -123,7 +123,7 @@ describe("proxy.ts onboarding redirect (v1.4.22 C4)", () => {
|
||||
// were still in place it would short-circuit through public-path
|
||||
// handling and 200 instead.
|
||||
expect([307, 308]).toContain(res.status);
|
||||
expect(res.headers.get("location")).toMatch(/\/auth\/login$/);
|
||||
expect(res.headers.get("location")).toMatch(/\/auth\/login(\?|$)/);
|
||||
});
|
||||
|
||||
it("still treats subroutes /onboarding/<x> as the onboarding surface", () => {
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
/**
|
||||
* The edge sends a visitor without a session to the sign-in page with the
|
||||
* page they asked for in `next`, so a home-screen shortcut or a notification
|
||||
* tap still arrives there after signing in. The sign-in page re-checks the
|
||||
* value (`sanitizeSameOriginPath`); the proxy only ever puts in the
|
||||
* request's own path and query.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
vi.mock("@/lib/process-type", () => ({
|
||||
shouldRunWeb: () => true,
|
||||
}));
|
||||
|
||||
import { proxy } from "../proxy";
|
||||
|
||||
function signInRedirect(pathAndQuery: string): URL {
|
||||
const res = proxy(new NextRequest(`http://localhost${pathAndQuery}`));
|
||||
expect([307, 308]).toContain(res.status);
|
||||
return new URL(res.headers.get("location")!, "http://localhost");
|
||||
}
|
||||
|
||||
describe("proxy.ts sign-in redirect", () => {
|
||||
it.each([
|
||||
"/measurements?add=WEIGHT&source=shortcut",
|
||||
"/mood?add=1",
|
||||
"/medications",
|
||||
"/?source=pwa",
|
||||
])("carries %s to the sign-in page", (target) => {
|
||||
const location = signInRedirect(target);
|
||||
expect(location.pathname).toBe("/auth/login");
|
||||
expect(location.searchParams.get("next")).toBe(target);
|
||||
});
|
||||
|
||||
it("sends the bare start page to a bare sign-in page", () => {
|
||||
const location = signInRedirect("/");
|
||||
expect(location.pathname).toBe("/auth/login");
|
||||
expect(location.search).toBe("");
|
||||
});
|
||||
});
|
||||
@@ -5,8 +5,15 @@
|
||||
* so a session that ended without a sign-out (an expired cookie the edge sent
|
||||
* straight to the sign-in page) leaves nothing on the device for the next
|
||||
* account. It never clears on an answer that is not a verdict.
|
||||
*
|
||||
* On a protected page it sends the visitor to the sign-in page, carrying the
|
||||
* page along when the session ended on its own, and not after a deliberate
|
||||
* sign-out: the next person on the device starts from a bare sign-in page.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
|
||||
import { authUser, renderWithApp } from "../../../../tests/browser/render";
|
||||
import { stubFetch } from "../../../../tests/browser/fetch-stub";
|
||||
@@ -42,6 +49,8 @@ vi.mock("@/components/command-palette/command-palette.lazy", () => ({
|
||||
}));
|
||||
vi.mock("../global-shortcuts", () => ({ GlobalShortcuts: () => null }));
|
||||
|
||||
import { clearCachesForSessionEnd } from "@/hooks/use-auth";
|
||||
|
||||
import { AuthShell } from "../auth-shell";
|
||||
|
||||
const ME = "/api/auth/me";
|
||||
@@ -50,6 +59,19 @@ function openAt(path: string): void {
|
||||
window.history.replaceState(null, "", path);
|
||||
}
|
||||
|
||||
/** The tree `renderWithApp` mounts, for rendering it again. */
|
||||
function shellTree(queryClient: QueryClient) {
|
||||
return (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<I18nProvider initialLocale="en">
|
||||
<AuthShell>
|
||||
<div data-slot="page" />
|
||||
</AuthShell>
|
||||
</I18nProvider>
|
||||
</QueryClientProvider>
|
||||
);
|
||||
}
|
||||
|
||||
async function mountShell(user: Parameters<typeof authUser>[0] | null) {
|
||||
return renderWithApp(
|
||||
<AuthShell>
|
||||
@@ -125,3 +147,55 @@ describe("AuthShell on a sign-in page", () => {
|
||||
expect(clearOffline).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("AuthShell on a protected page", () => {
|
||||
/** Signed in on `path`, with the marker a confirmed session leaves. */
|
||||
async function signedInAt(path: string) {
|
||||
let session = true;
|
||||
stubFetch().on("GET", ME, () =>
|
||||
session
|
||||
? { data: authUser({ id: "signed-in" }) }
|
||||
: { status: 401, error: "Unauthorized" },
|
||||
);
|
||||
openAt(path);
|
||||
const view = await mountShell(null);
|
||||
await expect
|
||||
.poll(() => document.querySelector('[data-slot="page"]'))
|
||||
.toBeTruthy();
|
||||
return {
|
||||
...view,
|
||||
endSession: () => {
|
||||
session = false;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("returns to the page after a session that ended on its own", async () => {
|
||||
// Signed in once on this browser; the session has since expired while
|
||||
// the cookie stayed, so the next visit gets past the edge and the
|
||||
// account answer is the first to say so.
|
||||
const first = await signedInAt("/");
|
||||
first.endSession();
|
||||
await first.screen.unmount();
|
||||
openAt("/measurements?add=WEIGHT");
|
||||
await mountShell(null);
|
||||
|
||||
await expect.poll(() => window.location.pathname).toBe("/auth/login");
|
||||
expect(new URLSearchParams(window.location.search).get("next")).toBe(
|
||||
"/measurements?add=WEIGHT",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not carry the page past a deliberate sign-out", async () => {
|
||||
const { screen, queryClient, endSession } =
|
||||
await signedInAt("/medications");
|
||||
// What the sign-out does once the server confirmed it; the navigation
|
||||
// it starts renders the shell again, which asks for the account anew.
|
||||
endSession();
|
||||
clearCachesForSessionEnd(queryClient);
|
||||
await screen.rerender(shellTree(queryClient));
|
||||
|
||||
await expect.poll(() => window.location.pathname).toBe("/auth/login");
|
||||
expect(window.location.search).toBe("");
|
||||
});
|
||||
});
|
||||
@@ -10,9 +10,14 @@ import { MaintainershipBanner } from "@/components/i18n/maintainership-banner";
|
||||
import { LayoutCoachFab } from "@/components/insights/layout-coach-fab";
|
||||
import { LayoutCoachMount } from "@/components/insights/layout-coach-mount";
|
||||
import { TourLauncher } from "@/components/onboarding/tour-launcher";
|
||||
import { clearCachesForSessionEnd, useAuth } from "@/hooks/use-auth";
|
||||
import {
|
||||
clearCachesForSessionEnd,
|
||||
readWasAuthenticated,
|
||||
useAuth,
|
||||
} from "@/hooks/use-auth";
|
||||
import { useRecordCapabilities } from "@/hooks/use-record-capabilities";
|
||||
import { useRecordSessionTransition } from "@/hooks/use-record-session-transition";
|
||||
import { signInHref } from "@/lib/api/read-failure";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context";
|
||||
import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister";
|
||||
@@ -243,27 +248,26 @@ export function AuthShell({
|
||||
window.scrollTo(0, 0);
|
||||
}, [pathname]);
|
||||
|
||||
// Session END on a protected page (`/api/auth/me` answered 401/403): wipe
|
||||
// every client cache, since the QueryClient outlives navigation and health
|
||||
// data is not user-scoped, then go to the sign-in page. Logout does the same
|
||||
// from `useLogout`; this covers the expiry path it never runs through. No
|
||||
// verdict (`isAuthUnknown`: offline, 5xx, no marker) never wipes: a wipe on
|
||||
// a network failure is what used to destroy the offline caches.
|
||||
//
|
||||
// The target is fixed once per signed-out stretch, before the wipe clears
|
||||
// the marker it reads (the refetch that follows runs this again). A session
|
||||
// that ended on its own returns here after the next sign-in; a deliberate
|
||||
// sign-out already cleared the marker, so the next person gets a bare page.
|
||||
const signInTarget = useRef<string | null>(null);
|
||||
useEffect(() => {
|
||||
if (isAuthenticated || isPublicPage) signInTarget.current = null;
|
||||
if (!isLoading && !isAuthenticated && !isPublicPage) {
|
||||
// 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 it
|
||||
// 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");
|
||||
signInTarget.current ??= readWasAuthenticated()
|
||||
? signInHref()
|
||||
: "/auth/login";
|
||||
if (!isAuthUnknown) clearCachesForSessionEnd(queryClient);
|
||||
router.replace(signInTarget.current);
|
||||
}
|
||||
}, [
|
||||
isLoading,
|
||||
|
||||
+9
-12
@@ -565,20 +565,17 @@ export function proxy(request: NextRequest) {
|
||||
const isSessionGatedPage = !isApiRoute && !isStaticFile && !isPublic;
|
||||
if (isSessionGatedPage) {
|
||||
if (!readSessionCookie(request.cookies)) {
|
||||
return applyBaselineSecurityHeaders(
|
||||
NextResponse.redirect(new URL("/auth/login", request.url)),
|
||||
);
|
||||
// Back here after signing in; the sign-in page re-checks `next`.
|
||||
const signIn = new URL("/auth/login", request.url);
|
||||
const here = `${pathname}${request.nextUrl.search}`;
|
||||
if (here !== "/") signIn.searchParams.set("next", here);
|
||||
return applyBaselineSecurityHeaders(NextResponse.redirect(signIn));
|
||||
}
|
||||
|
||||
// v1.4.22 — server-side onboarding redirect. Previously the
|
||||
// `<AuthShell>` ran this in a post-hydration `useEffect`, so a user
|
||||
// with `onboardingCompletedAt === null` briefly saw the dashboard
|
||||
// flash before the redirect fired. The auth flows (login, passkey,
|
||||
// register, /api/auth/me) keep an `hl_onboarding` cookie in sync
|
||||
// with the DB state; the proxy reads it without a DB roundtrip so
|
||||
// the redirect lands on the first server response. The /onboarding
|
||||
// page itself is in PUBLIC_PATHS above, so this branch only runs
|
||||
// for the surfaces that need to redirect away.
|
||||
// The onboarding redirect, here so it lands on the first response with no
|
||||
// dashboard flash. The auth flows (login, passkey, register,
|
||||
// /api/auth/me) keep the `hl_onboarding` cookie in step with the DB, so no
|
||||
// DB round trip; `/onboarding` itself is public above.
|
||||
const onboardingPending =
|
||||
request.cookies.get("hl_onboarding")?.value === "pending";
|
||||
// v1.4.22 — exact match the redirect
|
||||
|
||||
Reference in new issue
Block a user