From f569b19c49e4902c93112049f3e08afee00692da Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 15 Sep 2026 01:35:25 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20review=20round=202=20=E2=80=94=20a=20sig?= =?UTF-8?q?ned-out=20tab=20acts=20as=20nobody,=20honest=20share-page=20fai?= =?UTF-8?q?lures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review round 2 over main..improvement/watch-background-audio. Account resolution (server) - A request naming an account that is no longer signed in to this browser (X-Siftlode-Account or ?account=) now acts as nobody instead of the session's default account. A tab still pinned to an account signed out in another tab went on acting as the default: its player checkpoints and preference saves landed in that account, the messages socket and Plex playback ran as it, and its Sign out signed THAT account out. A hidden tab never polls /api/me, so a tab playing in the background kept doing it. The /api/me bootstrap probe alone keeps the fallback, which is how such a tab learns who it now is: the app's 401 handler reloads, and BootGate re-pins the tab. BootGate - On an identity change the tab reloads without moving its pin first. The tree being torn down still runs its unmount saves before the page goes, and with the pin already moved they wrote into the new account's local keys (a player volume still pending). The reloaded page corrects the pin. Share page (/watch) - Only a 404 says the link is gone. A 5xx, a dropped connection, and the unlock's own rate limit all rendered "no longer available ... expired or revoked" to a viewer with a valid link, with no way to try again. A load failure now offers Retry; too many password tries says so and keeps the form; an unlock that gets no answer says so instead of doing nothing. Strings in en/hu/de. Downloads - A cookie-authed asset is recognised under the current signature version and the one before it, so a job queued before a deploy that bumps the version still downloads with its cookie instead of failing on the login wall with an error that says no cookie was used. The app chunk is preloaded through lazyWithPreload, so a failed fetch no longer leaves an unhandled rejection beside the ErrorBoundary. The panel layout seed is one helper (seedLayout). Tests: the stale-tab refusal, its harmless Sign out and the probe fallback; the pre-bump authed asset; BootGate keeping the pin on reload and staying mounted through a same-account change; the reveal clock's later-commit hand-off; the prefs, panel layout and demo-scope seeds from the me cache; the share page's failure states. Each mutation-checked RED for its own reason. --- backend/app/auth.py | 31 +++++--- backend/app/downloads/credentials.py | 11 ++- backend/app/downloads/formats.py | 20 ++++- backend/app/routes/auth.py | 2 + backend/app/routes/messages.py | 7 +- backend/tests/test_auth_session.py | 19 +++++ backend/tests/test_download_credentials.py | 15 ++++ frontend/src/App.tsx | 11 ++- frontend/src/components/BootGate.dom.test.tsx | 32 ++++++++ frontend/src/components/BootGate.tsx | 7 +- .../FeedFiltersProvider.seed.dom.test.tsx | 58 ++++++++++++++ .../src/components/PrefsProvider.dom.test.tsx | 54 +++++++++++++ .../src/components/WatchPage.dom.test.tsx | 76 +++++++++++++++++++ frontend/src/components/WatchPage.tsx | 65 ++++++++++++++-- .../src/components/ui/QueryState.dom.test.tsx | 10 +++ frontend/src/lib/panelLayout.dom.test.ts | 32 ++++++++ frontend/src/lib/panelLayout.ts | 14 +++- frontend/src/main.tsx | 9 ++- 18 files changed, 433 insertions(+), 40 deletions(-) create mode 100644 frontend/src/components/FeedFiltersProvider.seed.dom.test.tsx create mode 100644 frontend/src/components/PrefsProvider.dom.test.tsx create mode 100644 frontend/src/lib/panelLayout.dom.test.ts diff --git a/backend/app/auth.py b/backend/app/auth.py index 95d027ec..1797b8af 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -269,21 +269,27 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None: ACTIVE_ACCOUNT_HEADER = "x-siftlode-account" -def resolved_user_id(conn: HTTPConnection) -> tuple[int | None, bool]: +def resolved_user_id(conn: HTTPConnection, *, strict: bool = True) -> tuple[int | None, bool]: """Which account this connection acts as, and whether that's the session's default account. Takes any HTTPConnection — an HTTP Request OR a WebSocket — so the WS push channel shares this exact per-tab resolution instead of re-implementing it. The signed cookie holds the *wallet* (`account_ids`, every account signed into this browser) - plus a default `user_id`. A caller may override the default with the X-Siftlode-Account header, - but ONLY for an account already in the wallet — so a tab can't impersonate an account that never - authenticated here. Everything else (WebSocket, plain navigations) keeps using the cookie default. + plus a default `user_id`. A caller may name its account with the X-Siftlode-Account header (or + `?account=`), but ONLY an account in the wallet — so a tab can't impersonate an account that + never authenticated here. A connection that names no account acts as the cookie default. + + A connection naming an account NOT in the wallet acts as nobody (`None`) — typically a tab still + pinned to an account that was signed out in another tab. It must not fall back to the default: + that tab would go on acting as an account it never showed, its player checkpoints and pref saves + landing there and its Sign out signing THAT account out. Only the bootstrap /api/me probe passes + `strict=False` and gets the default, which is how such a tab learns who it now is (the frontend + re-pins it and reloads). """ default_id = conn.session.get("user_id") wallet = conn.session.get("account_ids") or [] # Header for normal XHR; ?account= for contexts that can't set headers (WebSocket, and a - # plain file download). Both are wallet-gated below, so neither can impersonate an - # account that never signed into this browser. + # plain file download). Both are wallet-gated below. hdr = conn.headers.get(ACTIVE_ACCOUNT_HEADER) or conn.query_params.get("account") if hdr: try: @@ -292,11 +298,16 @@ def resolved_user_id(conn: HTTPConnection) -> tuple[int | None, bool]: hid = None if hid is not None and hid in wallet: return hid, hid == default_id + if strict: + return None, False return default_id, True def current_user(request: Request, db: Session = Depends(get_db)) -> User: - user_id, is_default = resolved_user_id(request) + return _session_user(request, db, *resolved_user_id(request)) + + +def _session_user(request: Request, db: Session, user_id: int | None, is_default: bool) -> User: if not user_id: raise HTTPException(status_code=401, detail="Not authenticated") user = db.get(User, user_id) @@ -328,9 +339,11 @@ def current_user(request: Request, db: Session = Depends(get_db)) -> User: def optional_current_user(request: Request, db: Session = Depends(get_db)) -> User | None: """Like `current_user` but returns None instead of raising 401 when there's no valid session. Used by the bootstrap /api/me probe so the logged-out landing doesn't log a failed request to - the browser console (a non-2xx fetch is a console error no matter how the app handles it).""" + the browser console (a non-2xx fetch is a console error no matter how the app handles it). + The one lenient resolution (`strict=False`): a tab naming an account no longer signed in here gets + the default account, which is how it learns who it now is — see `resolved_user_id`.""" try: - return current_user(request, db) + return _session_user(request, db, *resolved_user_id(request, strict=False)) except HTTPException: return None diff --git a/backend/app/downloads/credentials.py b/backend/app/downloads/credentials.py index cb6fa3cc..1ff82711 100644 --- a/backend/app/downloads/credentials.py +++ b/backend/app/downloads/credentials.py @@ -118,14 +118,17 @@ def is_authed_asset(asset: MediaAsset, spec: dict, user_id: int, url: str | None """Whether an asset is `user_id`'s private, cookie-salted identity for `url`'s host — the one kind of asset a cookie is ever attached to: the one enqueue creates (`authed_format_sig`), or a Re-download's rebuild of it (`rebuild_salt`). The worker's cookie gate asks it before a download, - and the failure diagnosis asks it after one (a failure on this asset happened WITH the cookie).""" + and the failure diagnosis asks it after one (a failure on this asset happened WITH the cookie). + Either signature version counts (`formats.format_sigs`): a job queued before a deploy that bumped + the version still downloads into the asset it was given.""" host_key = host_key_for_url(url) if host_key is None: return False - if asset.format_sig == authed_format_sig(spec, user_id, host_key): + salt = cookie_salt(user_id, host_key) + if asset.format_sig in formats.format_sigs(spec, salt=salt): return True - return asset.id is not None and asset.format_sig == formats.format_sig( - spec, salt=rebuild_salt(asset.id, cookie_salt(user_id, host_key)) + return asset.id is not None and asset.format_sig in formats.format_sigs( + spec, salt=rebuild_salt(asset.id, salt) ) diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index 115b11f8..cc05becf 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -100,20 +100,34 @@ def normalize(spec: dict) -> dict: return s -def format_sig(spec: dict, salt: str | None = None) -> str: +def _sig_version(s: dict) -> int: + return _AUDIO_SIG_VERSION if s["mode"] == "a" else _SIG_VERSION + + +def format_sig(spec: dict, salt: str | None = None, *, version: int | None = None) -> str: """Stable short hash of the output-affecting spec — the MediaAsset cache key component. `salt` forks a distinct, non-dedup identity for the same spec (used by force-redownload so it - always re-fetches into its own asset instead of sharing a cached file with other jobs).""" + always re-fetches into its own asset instead of sharing a cached file with other jobs). + `version` hashes under a signature version other than the current one (see `format_sigs`).""" s = normalize(spec) data = {k: s[k] for k in _SIG_KEYS} - data["_v"] = _AUDIO_SIG_VERSION if s["mode"] == "a" else _SIG_VERSION + data["_v"] = _sig_version(s) if version is None else version if salt is not None: data["_salt"] = salt payload = json.dumps(data, sort_keys=True, separators=(",", ":")) return hashlib.sha256(payload.encode()).hexdigest()[:24] +def format_sigs(spec: dict, salt: str | None = None) -> tuple[str, str]: + """`format_sig` under the current signature version and under the one before it. An asset keeps + the signature it was created with, so a job queued before a deploy that bumped the version and + run after it downloads into an asset carrying the previous one — which must still be recognised + as that user's private asset (`credentials.is_authed_asset`), or it downloads without its cookie.""" + previous = _sig_version(normalize(spec)) - 1 + return format_sig(spec, salt), format_sig(spec, salt, version=previous) + + def build_ydl_opts( spec: dict, outtmpl: str, diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index b96d4304..53da4fd7 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -631,6 +631,8 @@ async def logout(request: Request): out in one tab doesn't disturb another tab running a different account. If the account being removed was the session default, promote the most recent remaining account (or clear the session when none remain).""" + # A tab still pinned to an account already signed out in another tab resolves to nobody, so it + # signs nobody out — it used to fall back to the default and sign THAT account out in its place. active, is_default = resolved_user_id(request) remaining = [a for a in (request.session.get("account_ids") or []) if a != active] request.session["account_ids"] = remaining diff --git a/backend/app/routes/messages.py b/backend/app/routes/messages.py index eb7463f0..c2c030a9 100644 --- a/backend/app/routes/messages.py +++ b/backend/app/routes/messages.py @@ -423,9 +423,10 @@ async def messages_ws(ws: WebSocket) -> None: """Live push channel: authenticated by the session cookie, registers the connection so sent messages reach this user's open tabs instantly. We don't consume client→server frames (sending goes through POST /api/messages); the receive loop only detects disconnect.""" - # Which signed-in account this socket acts as — the SHARED per-tab resolution (wallet-gated - # ?account= override, session default otherwise). A browser WebSocket can't send the - # X-Siftlode-Account header, so resolved_user_id falls through to the ?account= query param. + # Which signed-in account this socket acts as — the SHARED per-tab resolution (a wallet-gated + # ?account=, the session default when none is named, nobody for an account no longer signed in + # here). A browser WebSocket can't send the X-Siftlode-Account header, so resolved_user_id falls + # through to the ?account= query param. uid, _ = resolved_user_id(ws) if not uid: await ws.close(code=1008) diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index e5f9382d..c9b549e8 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -57,6 +57,25 @@ def test_a_non_wallet_account_header_falls_back_to_the_default(client, make_user assert me.json()["email"] == "alice2@example.com" # still Alice, not Mallory +def test_a_tab_pinned_to_an_account_signed_out_elsewhere_acts_as_nobody(client, make_user): + # Two accounts in one browser: a tab pinned to Alice, Bob the default. Alice signs out in another + # tab. The pinned tab used to go on acting as the default, Bob — its player checkpoints and pref + # saves landed in his account, and its own Sign out signed HIM out. + alice = make_user(email="alice3@example.com", password=_PW) + make_user(email="bob3@example.com", password=_PW) + assert _login(client, "alice3@example.com", _PW).status_code == 200 + assert _login(client, "bob3@example.com", _PW).status_code == 200 # Bob is now the default + tab = {ACTIVE_ACCOUNT_HEADER: str(alice.id)} + assert client.get("/api/feed", headers=tab).status_code == 200 + assert client.post("/auth/logout", headers=tab).status_code == 200 # Alice signs out + + assert client.get("/api/feed", headers=tab).status_code == 401 + assert client.post("/auth/logout", headers=tab).status_code == 200 + assert client.get("/api/feed").status_code == 200 # Bob is still signed in, the session intact + # The bootstrap probe still answers as the default: how the stale tab learns who it is now. + assert client.get("/api/me", headers=tab).json()["email"] == "bob3@example.com" + + def test_logout_others_revokes_the_other_session_but_keeps_the_caller(client, make_user): # Two independent cookie jars (browsers) signed into the same account. `logout-others` from one # bumps the account epoch so the OTHER's cookie is rejected on its next request, while the caller diff --git a/backend/tests/test_download_credentials.py b/backend/tests/test_download_credentials.py index 94412ba4..1dcdace6 100644 --- a/backend/tests/test_download_credentials.py +++ b/backend/tests/test_download_credentials.py @@ -72,6 +72,21 @@ def test_authed_sig_differs_per_user_and_from_the_shared_sig(): assert a != shared and b != shared and a != b +def test_an_authed_asset_queued_before_a_signature_version_bump_still_gets_its_cookie(monkeypatch): + # A job keeps the asset it was queued with. A deploy that bumps the signature version between the + # queueing and the download used to make the user's private asset stop counting as theirs, so the + # download went without its cookie and failed on the login wall. + spec = formats.normalize({"mode": "a"}) + queued = MediaAsset(format_sig=credentials.authed_format_sig(spec, 1, "facebook")) + rebuild = credentials.rebuild_salt(7, credentials.cookie_salt(1, "facebook")) + rebuilt = MediaAsset(id=7, format_sig=formats.format_sig(spec, salt=rebuild)) + monkeypatch.setattr(formats, "_AUDIO_SIG_VERSION", formats._AUDIO_SIG_VERSION + 1) # the deploy + + for asset in (queued, rebuilt): + assert credentials.is_authed_asset(asset, spec, 1, FB_URL) + assert not credentials.is_authed_asset(asset, spec, 2, FB_URL) # still private to its user + + def test_write_cookiefile_is_private_and_survives_the_retry_wipe(tmp_path): staging = tmp_path / "staging" staging.mkdir() diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 4570486f..c9ecd790 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -8,10 +8,9 @@ import { setLanguage, type LangCode } from "./i18n"; import { clearParams } from "./lib/urlState"; import type { Page } from "./lib/urlState"; import { - loadLayout, - normalizeLayout, PREF_KEY, saveLayoutLocal, + seedLayout, type PanelId, type PanelLayout, } from "./lib/panelLayout"; @@ -88,11 +87,11 @@ export default function App() { // has never stored. const [panelLayouts, setPanelLayouts] = useState>(() => { const prefs = queryClient.getQueryData(qk.me())?.preferences; - const seed = (p: PanelId) => { - const raw = prefs?.[PREF_KEY[p]]; - return raw ? normalizeLayout(p, raw) : loadLayout(p); + return { + feed: seedLayout("feed", prefs), + plex: seedLayout("plex", prefs), + playlists: seedLayout("playlists", prefs), }; - return { feed: seed("feed"), plex: seed("plex"), playlists: seed("playlists") }; }); const [aboutOpen, setAboutOpen] = useState(false); const [notesOpen, setNotesOpen] = useState(false); diff --git a/frontend/src/components/BootGate.dom.test.tsx b/frontend/src/components/BootGate.dom.test.tsx index 22e76133..958218db 100644 --- a/frontend/src/components/BootGate.dom.test.tsx +++ b/frontend/src/components/BootGate.dom.test.tsx @@ -173,4 +173,36 @@ describe("BootGate", () => { await waitFor(() => expect(reload).toHaveBeenCalled()); }); + + it("an identity change reloads without moving the pin first", async () => { + // The tree being torn down still runs its unmount saves before the page goes; with the pin already + // moved they would land under the NEW account's local keys (a player volume still pending, say). + const me = vi.spyOn(api, "me").mockResolvedValue(account()); + const qc = mount(); + await screen.findByTestId("app"); + const reload = vi.fn(); + vi.stubGlobal("location", { ...window.location, reload }); + + me.mockResolvedValue(account({ id: 6 })); // 5 was signed out elsewhere: the server answers as 6 + await act(() => qc.refetchQueries({ queryKey: qk.me() })); + + await waitFor(() => expect(reload).toHaveBeenCalled()); + expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBe("5"); + }); + + it("a change to the account's data, not its identity, leaves the app as it is", async () => { + vi.spyOn(api, "me").mockResolvedValue(account()); + const qc = mount(); + await screen.findByTestId("app"); + const reload = vi.fn(); + vi.stubGlobal("location", { ...window.location, reload }); + + // What every pref save does to the cache (usePref's mergePrefsCache): a new `me` for the same account. + act(() => qc.setQueryData(qk.me(), account({ preferences: { language: "hu" } }))); + await act(() => new Promise((r) => setTimeout(r, 20))); + + expect(reload).not.toHaveBeenCalled(); + expect(mounts).toBe(1); + expect(i18n.language).toBe("en"); // the language is adopted once, never over the user's own choice + }); }); diff --git a/frontend/src/components/BootGate.tsx b/frontend/src/components/BootGate.tsx index 0946c585..45a54680 100644 --- a/frontend/src/components/BootGate.tsx +++ b/frontend/src/components/BootGate.tsx @@ -85,11 +85,14 @@ export default function BootGate({ children }: { children: ReactNode }) { const [readyFor, setReadyFor] = useState(undefined); useLayoutEffect(() => { if (identity === undefined) return; - if (me && getActiveAccount() !== me.id) setActiveAccount(me.id); if (readyFor !== undefined) { - location.reload(); // the tree was built for another identity — see the header + // The tree was built for another identity — see the header. The pin stays as it is: the tree + // being torn down still runs its unmount saves before the page goes, and with the pin moved they + // would land under the new account's local keys. The reloaded page's first probe corrects it. + location.reload(); return; } + if (me && getActiveAccount() !== me.id) setActiveAccount(me.id); const lang = me?.preferences?.language; if (typeof lang === "string" && isSupported(lang) && lang !== i18n.language) setLanguage(lang); setReadyFor(identity); diff --git a/frontend/src/components/FeedFiltersProvider.seed.dom.test.tsx b/frontend/src/components/FeedFiltersProvider.seed.dom.test.tsx new file mode 100644 index 00000000..85367d23 --- /dev/null +++ b/frontend/src/components/FeedFiltersProvider.seed.dom.test.tsx @@ -0,0 +1,58 @@ +// @vitest-environment jsdom +// +// The shared demo account has no subscriptions, so its feed opens on the whole library. The filters are +// seeded from the `me` cache at mount — BootGate mounts the tree only once it is there. The adopt effect +// this replaced applied the demo default after mounting; with it gone, a seed that did not read `me` +// would open the demo account on its empty subscriptions scope. +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { cleanup, render } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { NavigationProvider } from "./NavigationProvider"; +import { FeedFiltersProvider, useFeedFilters } from "./FeedFiltersProvider"; +import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount"; +import { qk } from "../lib/queryKeys"; +import type { Me } from "../lib/api"; + +const scopes: string[] = []; +function Probe() { + scopes.push(useFeedFilters().scope); + return null; +} + +function mount(me: Partial) { + const qc = new QueryClient(); + qc.setQueryData(qk.me(), { id: 5, preferences: {}, ...me }); + render( + + + + + + + , + ); +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, "5"); + scopes.length = 0; + window.history.replaceState(null, "", "/"); +}); +afterEach(() => { + cleanup(); + window.history.replaceState(null, "", "/"); +}); + +describe("FeedFiltersProvider seed", () => { + it("opens the shared demo account on the whole library from its first render", () => { + mount({ is_demo: true }); + expect(scopes.length).toBeGreaterThan(0); + expect(scopes).toEqual(scopes.map(() => "all")); + }); + + it("opens any other account on its own scope", () => { + mount({ is_demo: false }); + expect(scopes[0]).toBe("my"); + }); +}); diff --git a/frontend/src/components/PrefsProvider.dom.test.tsx b/frontend/src/components/PrefsProvider.dom.test.tsx new file mode 100644 index 00000000..76dd9f83 --- /dev/null +++ b/frontend/src/components/PrefsProvider.dom.test.tsx @@ -0,0 +1,54 @@ +// @vitest-environment jsdom +// +// The Settings prefs are seeded from the account's server prefs in the `me` cache — BootGate mounts the +// tree only once it is there. The adopt effect this replaced corrected them AFTER mounting; with it +// gone, a seed that read only this browser's copy would never show a performance mode or hints setting +// changed on another device. And the seed is not an edit: it must not be saved straight back. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { PrefsProvider, usePrefs } from "./PrefsProvider"; +import { api } from "../lib/api"; +import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount"; +import { accountKey, LS } from "../lib/storage"; +import { qk } from "../lib/queryKeys"; + +const seen: { perf: boolean; hints: boolean }[] = []; +function Probe() { + const { perf, hints } = usePrefs(); + seen.push({ perf, hints }); + return null; +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, "5"); + seen.length = 0; + vi.useFakeTimers(); +}); +afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe("PrefsProvider seed", () => { + it("renders the account's server prefs from the first render, and does not save them back", () => { + localStorage.setItem(accountKey(LS.perfMode, 5)!, "0"); // this browser last had it off + localStorage.setItem(accountKey(LS.hints, 5)!, "1"); + const save = vi.spyOn(api, "savePrefs").mockResolvedValue(undefined as never); + const qc = new QueryClient(); + qc.setQueryData(qk.me(), { id: 5, preferences: { performanceMode: true, hints: false } }); + render( + + + + + , + ); + + expect(seen[0]).toEqual({ perf: true, hints: false }); + act(() => vi.advanceTimersByTime(1000)); // past the auto-save debounce + expect(save).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/WatchPage.dom.test.tsx b/frontend/src/components/WatchPage.dom.test.tsx index 82165d34..f3847381 100644 --- a/frontend/src/components/WatchPage.dom.test.tsx +++ b/frontend/src/components/WatchPage.dom.test.tsx @@ -121,3 +121,79 @@ describe("WatchPage media element (S3 · 5a)", () => { expect(container.querySelector('a[href="/f.zip"]')).toBeNull(); }); }); + +// Only a 404 means a link is gone. Every other failure used to render the permanent "no longer +// available" state too: a server restart, a dropped connection, and the unlock's own rate limit told a +// viewer with a valid link that it had expired or been revoked, with no way to try again. +describe("WatchPage honest failures", () => { + const res = (status: number, body: unknown = {}) => ({ + ok: status < 400, + status, + json: async () => body, + }); + const GONE = "This link is no longer available."; + + it("a server error is a failure with a retry, not a dead link", async () => { + window.history.replaceState({}, "", "/watch/tok-5xx"); + vi.stubGlobal( + "fetch", + vi + .fn() + .mockResolvedValueOnce(res(503)) + .mockResolvedValue( + res(200, { needs_password: false, file_url: "/f.mp4", title: "A clip" }), + ), + ); + const page = render(); + const retry = await page.findByRole("button", { name: "Try again" }); + expect(page.queryByText(GONE)).toBeNull(); + + fireEvent.click(retry); + await waitFor(() => expect(page.container.querySelector("video")).not.toBeNull()); + }); + + it("a dropped connection is a failure with a retry too", async () => { + window.history.replaceState({}, "", "/watch/tok-offline"); + vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new TypeError("Failed to fetch"))); + const page = render(); + await page.findByRole("button", { name: "Try again" }); + expect(page.queryByText(GONE)).toBeNull(); + }); + + it("a 404 says the link is gone", async () => { + window.history.replaceState({}, "", "/watch/tok-404"); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(res(404))); + const page = render(); + await page.findByText(GONE); + expect(page.queryByRole("button", { name: "Try again" })).toBeNull(); + }); + + async function tryPassword(second: () => Promise) { + vi.stubGlobal( + "fetch", + vi + .fn() + .mockResolvedValueOnce(res(200, { needs_password: true })) + .mockImplementationOnce(second), + ); + const page = render(); + fireEvent.change(await page.findByPlaceholderText("Enter password"), { + target: { value: "guess" }, + }); + fireEvent.click(page.getByRole("button", { name: "Watch" })); + return page; + } + + it("too many password tries says so, and keeps the form", async () => { + window.history.replaceState({}, "", "/watch/tok-429"); + const page = await tryPassword(async () => res(429)); + await page.findByText("Too many attempts. Try again in a few minutes."); + expect(page.getByPlaceholderText("Enter password")).toBeTruthy(); + }); + + it("an unlock that gets no answer says so instead of doing nothing", async () => { + window.history.replaceState({}, "", "/watch/tok-unlock-offline"); + const page = await tryPassword(() => Promise.reject(new TypeError("Failed to fetch"))); + await page.findByText("Couldn't check the password. Try again."); + }); +}); diff --git a/frontend/src/components/WatchPage.tsx b/frontend/src/components/WatchPage.tsx index 8e15ca63..fd1fefa6 100644 --- a/frontend/src/components/WatchPage.tsx +++ b/frontend/src/components/WatchPage.tsx @@ -83,6 +83,11 @@ const STR = { links: "Links", brand: "Shared via Siftlode", repeat: "Repeat", + failed: "Couldn't load this link.", + failedHint: "Check your connection and try again.", + retry: "Try again", + tooMany: "Too many attempts. Try again in a few minutes.", + unlockFailed: "Couldn't check the password. Try again.", }, hu: { loading: "Betöltés…", @@ -97,6 +102,11 @@ const STR = { links: "Linkek", brand: "Megosztva a Siftlode-dal", repeat: "Ismétlés", + failed: "Nem sikerült betölteni a linket.", + failedHint: "Ellenőrizd a kapcsolatot, és próbáld újra.", + retry: "Újrapróbálás", + tooMany: "Túl sok próbálkozás. Próbáld újra pár perc múlva.", + unlockFailed: "Nem sikerült ellenőrizni a jelszót. Próbáld újra.", }, de: { loading: "Wird geladen…", @@ -111,6 +121,11 @@ const STR = { links: "Links", brand: "Geteilt über Siftlode", repeat: "Wiederholen", + failed: "Der Link konnte nicht geladen werden.", + failedHint: "Prüfe deine Verbindung und versuche es erneut.", + retry: "Erneut versuchen", + tooMany: "Zu viele Versuche. Versuche es in ein paar Minuten erneut.", + unlockFailed: "Das Passwort konnte nicht geprüft werden. Versuche es erneut.", }, } as const; @@ -122,7 +137,9 @@ const T = STR[lang]; export default function WatchPage() { const token = window.location.pathname.split("/watch/")[1]?.split(/[/?#]/)[0] ?? ""; - const [status, setStatus] = useState<"loading" | "ready" | "password" | "gone">("loading"); + const [status, setStatus] = useState<"loading" | "ready" | "password" | "gone" | "error">( + "loading", + ); const [meta, setMeta] = useState(null); const [password, setPassword] = useState(""); const [error, setError] = useState(null); @@ -144,22 +161,36 @@ export default function WatchPage() { writeJSON(LS.watchRepeat, next); }; + // Bumped by Retry, which asks again. + const [attempt, setAttempt] = useState(0); useEffect(() => { if (!token) { setStatus("gone"); return; } fetch(`/api/public/watch/${encodeURIComponent(token)}`) - .then((r) => (r.ok ? r.json() : Promise.reject(r.status))) - .then((m: Meta) => { - if (m.needs_password) setStatus("password"); + .then((r) => { + // Only a 404 says the link is gone (unknown, expired, revoked, or its file removed). Anything + // else — a 5xx while the server restarts, a dropped connection — is a failure worth retrying, + // not a dead link. + if (r.status === 404) return null; + if (!r.ok) throw new Error(`HTTP ${r.status}`); + return r.json() as Promise; + }) + .then((m) => { + if (!m) setStatus("gone"); + else if (m.needs_password) setStatus("password"); else { setMeta(m); setStatus("ready"); } }) - .catch(() => setStatus("gone")); - }, [token]); + .catch(() => setStatus("error")); + }, [token, attempt]); + const retry = () => { + setStatus("loading"); + setAttempt((n) => n + 1); + }; const unlock = async () => { setUnlocking(true); @@ -174,12 +205,20 @@ export default function WatchPage() { setError(T.wrong); return; } - if (!r.ok) { + // Unlocking is rate-limited per link: too many tries is not a dead link either. + if (r.status === 429) { + setError(T.tooMany); + return; + } + if (r.status === 404) { setStatus("gone"); return; } + if (!r.ok) throw new Error(`HTTP ${r.status}`); setMeta(await r.json()); setStatus("ready"); + } catch { + setError(T.unlockFailed); } finally { setUnlocking(false); } @@ -206,6 +245,18 @@ export default function WatchPage() { )} + {status === "error" && ( +
+
+
{T.failed}
+
{T.failedHint}
+ +
+
+ )} + {status === "password" && (
diff --git a/frontend/src/components/ui/QueryState.dom.test.tsx b/frontend/src/components/ui/QueryState.dom.test.tsx index cdfc420e..089e147b 100644 --- a/frontend/src/components/ui/QueryState.dom.test.tsx +++ b/frontend/src/components/ui/QueryState.dom.test.tsx @@ -56,6 +56,16 @@ describe("LoadingState reveal", () => { expect(revealDelay(container)).toBe(300); }); + it("one that mounts just after another left, in a later commit, keeps that one's clock", () => { + now = 45_000; + const first = render(); + now = 45_050; + first.unmount(); // a Suspense fallback leaves a commit before the page's own loading state mounts + now = 45_080; + const { container } = render(); + expect(revealDelay(container)).toBe(220); + }); + it("`since` counts the wait from an earlier start (the boot screen: navigation start)", () => { now = 50_250; const { container } = render(); diff --git a/frontend/src/lib/panelLayout.dom.test.ts b/frontend/src/lib/panelLayout.dom.test.ts new file mode 100644 index 00000000..1cb32892 --- /dev/null +++ b/frontend/src/lib/panelLayout.dom.test.ts @@ -0,0 +1,32 @@ +// @vitest-environment jsdom +// +// A panel's layout is seeded from the account's server prefs when the app mounts. The adopt effect this +// replaced corrected it after mounting; with it gone, a seed that read only this browser's copy would +// never show a layout changed on another device. +import { beforeEach, describe, expect, it } from "vitest"; +import { seedLayout } from "./panelLayout"; +import { ACTIVE_ACCOUNT_KEY } from "./activeAccount"; +import { accountKey, LS } from "./storage"; + +const local = { order: [], collapsed: { genre: true }, hidden: {} }; + +beforeEach(() => { + localStorage.clear(); + sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, "5"); + localStorage.setItem(accountKey(LS.plexLayout, 5)!, JSON.stringify(local)); +}); + +describe("seedLayout", () => { + it("takes the account's server layout over this browser's copy", () => { + const l = seedLayout("plex", { + plexLayout: { order: [], collapsed: {}, hidden: { year: true } }, + }); + expect(l.hidden).toEqual({ year: true }); + expect(l.collapsed).toEqual({}); + }); + + it("uses this browser's copy for a layout the server has never stored", () => { + expect(seedLayout("plex", {}).collapsed).toEqual({ genre: true }); + expect(seedLayout("plex", undefined).collapsed).toEqual({ genre: true }); + }); +}); diff --git a/frontend/src/lib/panelLayout.ts b/frontend/src/lib/panelLayout.ts index f1f66c73..76161489 100644 --- a/frontend/src/lib/panelLayout.ts +++ b/frontend/src/lib/panelLayout.ts @@ -50,7 +50,7 @@ export function defaultLayout(panel: PanelId): PanelLayout { return { order: [...PANEL_ORDER[panel]], collapsed: {}, hidden: {} }; } -export function normalizeLayout(panel: PanelId, raw: unknown): PanelLayout { +function normalizeLayout(panel: PanelId, raw: unknown): PanelLayout { const all = PANEL_ORDER[panel]; const r = (raw ?? {}) as Partial; const order = Array.isArray(r.order) ? r.order.filter((x) => all.includes(x)) : []; @@ -62,10 +62,20 @@ export function normalizeLayout(panel: PanelId, raw: unknown): PanelLayout { }; } -export function loadLayout(panel: PanelId): PanelLayout { +function loadLayout(panel: PanelId): PanelLayout { return normalizeLayout(panel, readAccount(LS_KEY[panel], {})); } +// A panel's layout when the app mounts: the account's server copy (the `me` cache's preferences) when +// it has one — so a layout changed on another device is right on the first paint — else this browser's. +export function seedLayout( + panel: PanelId, + prefs: Record | undefined, +): PanelLayout { + const raw = prefs?.[PREF_KEY[panel]]; + return raw ? normalizeLayout(panel, raw) : loadLayout(panel); +} + export function saveLayoutLocal(panel: PanelId, l: PanelLayout): void { writeAccount(LS_KEY[panel], l); } diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 7a5cecae..da369038 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -19,14 +19,14 @@ import { WizardProvider } from "./components/WizardProvider"; import "./i18n"; import "./index.css"; import "flag-icons/css/flag-icons.min.css"; +import { lazyWithPreload } from "./lib/lazyWithPreload"; // Split by top-level route so each entry point is its own chunk: a public /watch share link or a // legal page never downloads the authenticated app bundle, and the app never carries them either. // The legal pages live outside the App tree (no /api/me) so Google's consent screen and reviewers // can fetch them directly; the watch page is a login-free public player. Reached by full-page // navigation (plain ), so a simple pathname switch is enough — no router needed. -const loadApp = () => import("./App"); -const App = lazy(loadApp); +const App = lazyWithPreload(() => import("./App")); const PrivacyPolicy = lazy(() => import("./components/legal/PrivacyPolicy")); const Terms = lazy(() => import("./components/legal/Terms")); const WatchPage = lazy(() => import("./components/WatchPage")); @@ -43,8 +43,9 @@ de.scheme ||= "midnight"; const path = window.location.pathname; const isPublicPage = path === "/privacy" || path === "/terms" || path.startsWith("/watch/"); // Fetch the app chunk NOW, alongside BootGate's identity probe, instead of only once the gate opens: -// the two waits then overlap rather than queue, and the gate usually opens straight onto the app. -if (!isPublicPage) void loadApp(); +// the two waits then overlap rather than queue, and the gate usually opens straight onto the app. A +// failed fetch is left to the render's own load, which retries it and reports to the ErrorBoundary. +if (!isPublicPage) App.preload().catch(() => {}); const root = path === "/privacy" ? (