From 4291d08e2bb0cfa4308aa642247b72c5f165563b Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 27 Sep 2026 06:17:31 +0800 Subject: [PATCH] =?UTF-8?q?fix(tier):=20/me=20=E4=B8=B4=E6=97=B6=E5=A4=B1?= =?UTF-8?q?=E8=B4=A5=E6=97=B6=E7=AE=A1=E7=90=86=E9=A1=B5=E6=98=BE=E7=A4=BA?= =?UTF-8?q?=E6=9D=83=E9=99=90=E7=A1=AE=E8=AE=A4=E5=A4=B1=E8=B4=A5=E5=92=8C?= =?UTF-8?q?=E9=87=8D=E8=AF=95=EF=BC=8C=E4=B8=8D=E5=86=8D=E8=AF=AF=E6=8A=A5?= =?UTF-8?q?=E6=97=A0=E6=9D=83=E8=AE=BF=E9=97=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- panel/src/components/RequireAdmin.test.tsx | 89 ++++++++++++++++++++++ panel/src/components/RequireAdmin.tsx | 10 ++- panel/src/components/RequireOwner.tsx | 11 ++- panel/src/components/States.tsx | 40 +++++++++- panel/src/i18n/resources/en-US/common.json | 2 + panel/src/i18n/resources/zh-CN/common.json | 2 + panel/src/lib/auth.test.ts | 13 ++++ panel/src/lib/auth.ts | 6 ++ panel/src/lib/tier.test.tsx | 69 +++++++++++++++++ panel/src/lib/tier.tsx | 3 + 10 files changed, 237 insertions(+), 8 deletions(-) create mode 100644 panel/src/components/RequireAdmin.test.tsx create mode 100644 panel/src/lib/tier.test.tsx diff --git a/panel/src/components/RequireAdmin.test.tsx b/panel/src/components/RequireAdmin.test.tsx new file mode 100644 index 0000000..b2b72e4 --- /dev/null +++ b/panel/src/components/RequireAdmin.test.tsx @@ -0,0 +1,89 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import i18next from "i18next"; +import { humanizeError } from "@/lib/api"; +import { RequireAdmin } from "./RequireAdmin"; +import { RequireOwner } from "./RequireOwner"; + +const tier = vi.hoisted(() => ({ + loading: false, + isAdmin: false, + isOwner: false, + identityError: null as unknown, + revalidate: vi.fn(), +})); +vi.mock("@/lib/tier", () => ({ useTier: () => tier })); + +const t = (key: string) => i18next.t(key); +const err503 = { status: 503, code: "unavailable", message: "database is restarting" }; + +function renderAt(path: string) { + return render( + + + }> + admin page

} /> + + }> + users page

} /> + + + , + ); +} + +const retryButton = () => screen.getByRole("button", { name: t("common:try_again") }); + +beforeEach(() => { + Object.assign(tier, { loading: false, isAdmin: false, isOwner: false, identityError: null }); + tier.revalidate.mockReset(); + tier.revalidate.mockResolvedValue(undefined); +}); + +describe.each([ + ["RequireAdmin", "/admin", "admin page", "isAdmin"], + ["RequireOwner", "/admin/users", "users page", "isOwner"], +] as const)("%s", (_name, path, page, flag) => { + it("says the access check failed, with why, when /me failed with anything but a 401", () => { + tier.identityError = err503; + renderAt(path); + + expect(screen.getByText(t("common:access_unknown_title"))).toBeTruthy(); + expect(screen.getByRole("alert").textContent).toBe(humanizeError(err503)); + expect(retryButton()).toBeTruthy(); + expect(screen.queryByText(t("common:not_authorized_title"))).toBeNull(); + expect(screen.queryByText(page)).toBeNull(); + }); + + it("re-reads /me in place from the retry, and holds the button until it answers", async () => { + tier.identityError = err503; + let answer!: () => void; + tier.revalidate.mockReturnValue(new Promise((resolve) => (answer = resolve))); + renderAt(path); + + fireEvent.click(retryButton()); + expect(tier.revalidate).toHaveBeenCalledTimes(1); + expect((retryButton() as HTMLButtonElement).disabled).toBe(true); + fireEvent.click(retryButton()); + expect(tier.revalidate).toHaveBeenCalledTimes(1); + + await act(async () => answer()); + expect((retryButton() as HTMLButtonElement).disabled).toBe(false); + }); + + it("still says not authorized to a known account without the role", () => { + renderAt(path); + + expect(screen.getByText(t("common:not_authorized_title"))).toBeTruthy(); + expect(screen.queryByText(t("common:access_unknown_title"))).toBeNull(); + }); + + it("renders the page for an account with the role", () => { + tier[flag] = true; + renderAt(path); + + expect(screen.getByText(page)).toBeTruthy(); + }); +}); diff --git a/panel/src/components/RequireAdmin.tsx b/panel/src/components/RequireAdmin.tsx index 9e4d4c3..4f476c4 100644 --- a/panel/src/components/RequireAdmin.tsx +++ b/panel/src/components/RequireAdmin.tsx @@ -1,18 +1,20 @@ import { Outlet } from "react-router-dom"; import { useTranslation } from "react-i18next"; import { useTier } from "@/lib/tier"; -import { Loading, NotAuthorized } from "@/components/States"; +import { AccessUnknown, Loading, NotAuthorized } from "@/components/States"; // RequireAdmin is the single route wrapper for BOTH /admin/* and /ops/* (the two // admin-tier concerns). It is belt-and-suspenders: it spares non-admins a wall of // 403s, but the server enforces the boundary independently on every data call. // // While /me is still loading we show a spinner rather than NotAuthorized, so a -// genuine admin is never briefly told "not authorized" on a slow boot. Once -// settled, isAdmin is the fail-closed verdict (false on any /me failure). +// genuine admin is never briefly told "not authorized" on a slow boot. A /me that +// failed with anything but a 401 leaves the answer unknown, so the page says the +// check failed and offers a retry. Otherwise isAdmin is the fail-closed verdict. export function RequireAdmin() { - const { isAdmin, loading } = useTier(); + const { isAdmin, loading, identityError } = useTier(); const { t } = useTranslation("common"); if (loading) return ; + if (identityError) return ; return isAdmin ? : ; } diff --git a/panel/src/components/RequireOwner.tsx b/panel/src/components/RequireOwner.tsx index 8b79f23..d2ae54e 100644 --- a/panel/src/components/RequireOwner.tsx +++ b/panel/src/components/RequireOwner.tsx @@ -1,14 +1,19 @@ import { Outlet } from "react-router-dom"; import { useTier } from "@/lib/tier"; -import { NotAuthorized } from "@/components/States"; -import { Loading } from "@/components/States"; +import { AccessUnknown, Loading, NotAuthorized } from "@/components/States"; +// RequireOwner gates user management, one level above admin. Like RequireAdmin +// it waits for /me, and a /me that failed with anything but a 401 gets a retry: +// the owner is not told "not authorized" on a flaky read. export function RequireOwner() { - const { loading, isOwner } = useTier(); + const { loading, isOwner, identityError } = useTier(); if (loading) { return ; } + if (identityError) { + return ; + } if (!isOwner) { return ; } diff --git a/panel/src/components/States.tsx b/panel/src/components/States.tsx index c462111..f232b90 100644 --- a/panel/src/components/States.tsx +++ b/panel/src/components/States.tsx @@ -1,9 +1,12 @@ -import { Loader2, AlertTriangle, Inbox, ShieldX, Construction, Moon, SearchX } from "lucide-react"; +import { useState } from "react"; +import { Loader2, AlertTriangle, Inbox, ShieldX, ShieldQuestion, Construction, Moon, SearchX, RefreshCw } from "lucide-react"; import { Link } from "react-router-dom"; import { useTranslation } from "react-i18next"; import { MAX_AUTO_RESTARTS, shownPhase, type StartFailure } from "@/components/PhaseBadge"; import { PowerButton } from "@/components/PowerButton"; +import { Button } from "@/components/ui/button"; import { humanizeError } from "@/lib/api"; +import { useTier } from "@/lib/tier"; import type { Phase, RetireState } from "@/lib/types"; import { cn } from "@/lib/utils"; @@ -94,6 +97,41 @@ export function NotAuthorized() { ); } +// AccessUnknown stands in for an admin or owner page while /me has failed with +// anything but a 401: whether this account may see the page is unknown, so it +// says the check failed and why, and retries in place (the app stays mounted). +export function AccessUnknown({ error }: { error: unknown }) { + const { t } = useTranslation("common"); + const { revalidate } = useTier(); + const [retrying, setRetrying] = useState(false); + + async function retry() { + setRetrying(true); + try { + await revalidate(); + } finally { + setRetrying(false); + } + } + + return ( +
+ +
+

{t("access_unknown_title")}

+

{t("access_unknown_body")}

+

+ {humanizeError(error)} +

+
+ +
+ ); +} + export interface NotFoundProps { title?: string; body?: string; diff --git a/panel/src/i18n/resources/en-US/common.json b/panel/src/i18n/resources/en-US/common.json index aab9826..c9b2264 100644 --- a/panel/src/i18n/resources/en-US/common.json +++ b/panel/src/i18n/resources/en-US/common.json @@ -12,6 +12,8 @@ "not_wired_yet": "Not wired yet", "pending_backend_prefix": "pending backend: ", "checking_access": "Checking access…", + "access_unknown_title": "Couldn't check your access", + "access_unknown_body": "Your account details didn't load, so this page stays hidden until they do.", "loading_config": "Loading config…", "brand_name": "Felis", "brand_tagline": "K8s-native Minecraft orchestration", diff --git a/panel/src/i18n/resources/zh-CN/common.json b/panel/src/i18n/resources/zh-CN/common.json index c00c5f9..43eb832 100644 --- a/panel/src/i18n/resources/zh-CN/common.json +++ b/panel/src/i18n/resources/zh-CN/common.json @@ -12,6 +12,8 @@ "not_wired_yet": "功能尚未接入", "pending_backend_prefix": "待接入接口：", "checking_access": "正在验证权限…", + "access_unknown_title": "暂时无法确认你的权限", + "access_unknown_body": "账户信息没有读到，读到之前这一页先不显示。", "loading_config": "正在加载配置…", "brand_name": "Felis", "brand_tagline": "Kubernetes 原生的 Minecraft 管理平台", diff --git a/panel/src/lib/auth.test.ts b/panel/src/lib/auth.test.ts index c7d45dd..53c3b37 100644 --- a/panel/src/lib/auth.test.ts +++ b/panel/src/lib/auth.test.ts @@ -68,6 +68,19 @@ describe("deriveAuth", () => { expect(s.isAdmin).toBe(false); expect(s.unauthenticated).toBe(false); }); + + it("keeps a settled non-401 failure as identityError, so a gate can offer a retry", () => { + expect(deriveAuth(null, err500, false).identityError).toBe(err500); + const offline = new TypeError("Failed to fetch"); + expect(deriveAuth(null, offline, false).identityError).toBe(offline); + }); + + it("has no identityError for a 401, while loading, or once an identity is known", () => { + expect(deriveAuth(null, err401, false).identityError).toBeNull(); + expect(deriveAuth(null, err500, true).identityError).toBeNull(); + expect(deriveAuth(admin, err500, false).identityError).toBeNull(); + expect(deriveAuth(null, null, false).identityError).toBeNull(); + }); }); describe("loginReturnPath", () => { diff --git a/panel/src/lib/auth.ts b/panel/src/lib/auth.ts index 883b725..3f6e0cc 100644 --- a/panel/src/lib/auth.ts +++ b/panel/src/lib/auth.ts @@ -20,6 +20,11 @@ export interface AuthState { /** True ONLY when /me returned 401 — no/expired session, route to /login. A * transient or 5xx failure leaves this false so the app keeps rendering. */ unauthenticated: boolean; + /** The settled /me failure that left the identity unknown (anything but a + * 401), else null. The app keeps running, but an admin cannot be told from a + * user until a retry succeeds, so a gate says so and offers the retry instead + * of "not authorized". */ + identityError: unknown; } /** isUnauthorized reports whether a caught error is the request() 401 envelope — @@ -48,6 +53,7 @@ export function deriveAuth( loading, isAdmin: identity?.is_admin === true, unauthenticated: !loading && identity === null && isUnauthorized(error), + identityError: !loading && identity === null && error != null && !isUnauthorized(error) ? error : null, }; } diff --git a/panel/src/lib/tier.test.tsx b/panel/src/lib/tier.test.tsx new file mode 100644 index 0000000..9e1262f --- /dev/null +++ b/panel/src/lib/tier.test.tsx @@ -0,0 +1,69 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { act, render, screen } from "@testing-library/react"; +import { TierProvider, useTier } from "./tier"; +import type { Identity } from "./types"; + +const calls = vi.hoisted(() => ({ me: vi.fn() })); +vi.mock("./api", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, api: { ...actual.api, ...calls } }; +}); + +const admin: Identity = { user_id: "u1", email: "a@b.c", role: "admin", is_admin: true, is_owner: false }; +const err503 = { status: 503, code: "unavailable", message: "restarting" }; + +let tier: ReturnType; +function Probe() { + tier = useTier(); + return

{tier.isAdmin ? "admin" : "not admin"}

; +} + +async function boot() { + render( + + + , + ); + await act(async () => {}); +} + +beforeEach(() => { + calls.me.mockReset(); +}); + +describe("TierProvider after a /me that failed with anything but a 401", () => { + it("exposes the failure, and a revalidate that succeeds clears it without a reload", async () => { + calls.me.mockRejectedValueOnce(err503); + await boot(); + expect(tier.identityError).toBe(err503); + expect(tier.unauthenticated).toBe(false); + + let answer!: (id: Identity) => void; + calls.me.mockReturnValueOnce(new Promise((resolve) => (answer = resolve))); + let retry!: Promise; + act(() => { + retry = tier.revalidate(); + }); + // Mid-check the app stays mounted: loading never comes back. + expect(tier.loading).toBe(false); + + await act(async () => { + answer(admin); + await retry; + }); + expect(tier.identityError).toBeNull(); + expect(screen.getByText("admin")).toBeTruthy(); + }); + + it("keeps the failure when the retry fails the same way", async () => { + calls.me.mockRejectedValueOnce(err503); + await boot(); + + calls.me.mockRejectedValueOnce(new TypeError("Failed to fetch")); + await act(() => tier.revalidate()); + + expect(tier.identityError).toBe(err503); + expect(tier.unauthenticated).toBe(false); + }); +}); diff --git a/panel/src/lib/tier.tsx b/panel/src/lib/tier.tsx index cd85748..6755a04 100644 --- a/panel/src/lib/tier.tsx +++ b/panel/src/lib/tier.tsx @@ -25,6 +25,8 @@ import { deriveAuth, isUnauthorized, type AuthState } from "./auth"; // principal whose /me momentarily fails still gets the full User-Side app; they // simply don't see admin surfaces. (The backend 403s admin data calls // independently, so this is safe.) Only a genuine 401 sets `unauthenticated`. +// The failure itself is `identityError`: the admin and owner gates show it +// with a retry, since "not authorized" would be a guess. // // 3. Login-aware: `unauthenticated` (a true 401) routes to /login; `refresh()` // re-reads /me after a login / logout so the gate re-evaluates without a reload. @@ -61,6 +63,7 @@ const TierContext = createContext({ isAdmin: false, isOwner: false, unauthenticated: false, + identityError: null, refresh: async () => {}, revalidate: async () => {}, sessionEnded: false,