mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 22:26:13 +00:00
fix(insights): return values sub-page back-link to the originating metric
The show-all-readings link now carries the originating metric page as a sanitised from param, so a weight to show-all drill-in returns to weight rather than the Insights overview. The param is constrained to an internal /insights/<slug> path so a crafted value cannot redirect off-site.
This commit is contained in:
1 parent
732e85e4df
commit
dbdc99a4a1
4 files changed
+93
-4
No files matched your search
@@ -2,10 +2,14 @@
|
||||
|
||||
import { use } from "react";
|
||||
import Link from "next/link";
|
||||
import { notFound } from "next/navigation";
|
||||
import { notFound, useSearchParams } from "next/navigation";
|
||||
import { ArrowLeft } from "lucide-react";
|
||||
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import {
|
||||
INSIGHTS_OVERVIEW,
|
||||
resolveValuesBackHref,
|
||||
} from "@/lib/insights/values-back-link";
|
||||
import { measurementTypeEnum } from "@/lib/validations/measurement";
|
||||
import { MEASUREMENT_TYPE_LABEL_KEYS } from "@/components/measurements/measurement-list-meta";
|
||||
import { Button } from "@/components/ui/button";
|
||||
@@ -33,6 +37,7 @@ export default function InsightsMetricValuesPage({
|
||||
}) {
|
||||
const { t } = useTranslations();
|
||||
const { type } = use(params);
|
||||
const searchParams = useSearchParams();
|
||||
|
||||
// Reject any segment that is not a real MeasurementType so a typo'd
|
||||
// deep-link 404s instead of mounting a blank list.
|
||||
@@ -43,6 +48,16 @@ export default function InsightsMetricValuesPage({
|
||||
const labelKey = MEASUREMENT_TYPE_LABEL_KEYS[type];
|
||||
const metricLabel = labelKey ? t(labelKey) : type;
|
||||
|
||||
// v1.10.2 — return to the originating metric page when the link carried a
|
||||
// `from` param (set by `<SubPageShell>` on the "show all readings" link),
|
||||
// so a `weight → show all values` drill-in lands back on `weight` rather
|
||||
// than the Insights overview. `resolveValuesBackHref` sanitises the value to
|
||||
// an internal `/insights/<slug>` path so a crafted `?from=` can never become
|
||||
// an off-site or protocol-relative target; anything else falls back to the
|
||||
// overview.
|
||||
const backHref = resolveValuesBackHref(searchParams.get("from"));
|
||||
const backToOrigin = backHref !== INSIGHTS_OVERVIEW;
|
||||
|
||||
return (
|
||||
<SubPageShell
|
||||
title={t("insights.subPage.valuesPageTitle", { metric: metricLabel })}
|
||||
@@ -55,9 +70,11 @@ export default function InsightsMetricValuesPage({
|
||||
data-slot="metric-values-back"
|
||||
className="-ml-2 w-fit"
|
||||
>
|
||||
<Link href="/insights">
|
||||
<Link href={backHref}>
|
||||
<ArrowLeft className="mr-1 size-4" aria-hidden="true" />
|
||||
{t("insights.subPage.valuesBack")}
|
||||
{backToOrigin
|
||||
? t("insights.subPage.valuesBackToMetric", { metric: metricLabel })
|
||||
: t("insights.subPage.valuesBack")}
|
||||
</Link>
|
||||
</Button>
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useEffect, useRef, type ReactNode } from "react";
|
||||
import Link from "next/link";
|
||||
import { usePathname } from "next/navigation";
|
||||
import { ListOrdered } from "lucide-react";
|
||||
|
||||
import { Badge } from "@/components/ui/badge";
|
||||
@@ -129,6 +130,11 @@ export function SubPageShell({
|
||||
children,
|
||||
}: SubPageShellProps) {
|
||||
const { t } = useTranslations();
|
||||
// v1.10.2 — the "show all readings" link carries the originating metric
|
||||
// page as a `from` param so the values sub-page can offer a back-link to
|
||||
// where the user drilled in from (e.g. `weight → show all values → back to
|
||||
// weight`) rather than always returning to the Insights overview.
|
||||
const pathname = usePathname();
|
||||
// a11y: focus the heading on mount so a tab-strip navigation actually
|
||||
// moves screen-reader focus into the sub-page body. v1.4.27 MB7 /
|
||||
// CF-35 made the focus call opt-in via `focusOnMount` — the
|
||||
@@ -254,7 +260,11 @@ export function SubPageShell({
|
||||
data-slot="metric-show-all-values"
|
||||
className="h-10 w-full sm:w-auto"
|
||||
>
|
||||
<Link href={`/insights/values/${showAllValuesType}`}>
|
||||
<Link
|
||||
href={`/insights/values/${showAllValuesType}${
|
||||
pathname ? `?from=${encodeURIComponent(pathname)}` : ""
|
||||
}`}
|
||||
>
|
||||
<ListOrdered className="mr-1.5 size-4" aria-hidden="true" />
|
||||
{t("insights.subPage.showAllValues")}
|
||||
</Link>
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
import { resolveValuesBackHref } from "../values-back-link";
|
||||
|
||||
/**
|
||||
* v1.10.2 — the `/insights/values/[type]` back-link returns to the originating
|
||||
* metric page via a sanitised `from` param. The guard keeps the target an
|
||||
* internal `/insights/<slug>` path so a crafted value can never redirect
|
||||
* off-site.
|
||||
*/
|
||||
describe("resolveValuesBackHref", () => {
|
||||
it("returns the originating metric page when from is an internal insights path", () => {
|
||||
expect(resolveValuesBackHref("/insights/weight")).toBe("/insights/weight");
|
||||
expect(resolveValuesBackHref("/insights/blood-pressure")).toBe(
|
||||
"/insights/blood-pressure",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the overview when from is missing", () => {
|
||||
expect(resolveValuesBackHref(null)).toBe("/insights");
|
||||
expect(resolveValuesBackHref(undefined)).toBe("/insights");
|
||||
expect(resolveValuesBackHref("")).toBe("/insights");
|
||||
});
|
||||
|
||||
it("rejects the overview itself and any non-metric path", () => {
|
||||
expect(resolveValuesBackHref("/insights")).toBe("/insights");
|
||||
expect(resolveValuesBackHref("/settings/account")).toBe("/insights");
|
||||
});
|
||||
|
||||
it("rejects off-site and protocol-relative targets", () => {
|
||||
expect(resolveValuesBackHref("https://evil.example/insights/weight")).toBe(
|
||||
"/insights",
|
||||
);
|
||||
expect(resolveValuesBackHref("//evil.example")).toBe("/insights");
|
||||
expect(resolveValuesBackHref("/insights//evil.example")).toBe("/insights");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,25 @@
|
||||
/**
|
||||
* v1.10.2 — resolve the back-link target for `/insights/values/[type]`.
|
||||
*
|
||||
* The "show all readings" link (built by `<SubPageShell>`) carries the
|
||||
* originating metric page as a `from` query param so the values sub-page can
|
||||
* return the user to where they drilled in from (e.g. `weight → show all
|
||||
* values → back to weight`) rather than always to the Insights overview.
|
||||
*
|
||||
* The param is sanitised to an internal `/insights/<slug>` path — a single
|
||||
* leading slash, no protocol / host, no protocol-relative `//` — so a crafted
|
||||
* `?from=` value can never become an off-site or protocol-relative navigation
|
||||
* target. Anything that fails the check falls back to the Insights overview.
|
||||
*/
|
||||
export const INSIGHTS_OVERVIEW = "/insights";
|
||||
|
||||
export function resolveValuesBackHref(from: string | null | undefined): string {
|
||||
if (
|
||||
from &&
|
||||
from.startsWith("/insights/") &&
|
||||
!from.startsWith("/insights//")
|
||||
) {
|
||||
return from;
|
||||
}
|
||||
return INSIGHTS_OVERVIEW;
|
||||
}
|
||||
Reference in new issue
Block a user