From c134b514c1cd6219d3098ff1a983d1a4ce7c9c32 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 3 Sep 2026 23:45:31 +0200 Subject: [PATCH] fix(auth): confirm an emailed verification link wherever it is opened (E9.5 S2) The token was only ever consumed inside Welcome.tsx, which App renders ONLY when there is no session. Opening the emailed link in a browser that already held a Siftlode session posted nothing, the fragment was then stripped, and the user got no signal at all -- the link simply did nothing. Incognito worked, which is why it survived. Confirmation now happens in App, above that branch: signed out the outcome still drives the auth card's banners, signed in it drives a top-of-app bar. The entry URL is snapshotted once at import time (lib/entryUrl) instead of per component, so no future URL writer can strip a token before whoever needs it has read it -- that ordering race is what broke this in the first place. POST /auth/verify now names the address it confirmed, which matters when the browser holds a session for a different account; the same reasoning as password_reset_required already covers the disclosure. Also here, from the 2026-09-02 review tail: the dead GET /auth/verify goes (both senders emit the fragment form, and the last query-token link expired 24h after the 0.78.7 deploy), a spent #verify=/#reset= fragment is cleared while signed in, and the auth-card banners get role=status. The verifyOkReset copy no longer blames admin approval, which E9.5 S1 made false. --- backend/app/routes/auth.py | 21 ++--- backend/tests/test_auth_session.py | 18 +++- frontend/src/App.tsx | 10 ++- frontend/src/components/Banner.tsx | 10 ++- frontend/src/components/VerifyBanner.tsx | 41 +++++++++ frontend/src/components/Welcome.tsx | 76 ++++++---------- .../welcomeMode.render.dom.test.tsx | 47 +++++++++- frontend/src/i18n/locales/en/header.json | 6 ++ frontend/src/i18n/locales/en/welcome.json | 2 +- frontend/src/i18n/locales/hu/header.json | 6 ++ frontend/src/i18n/locales/hu/welcome.json | 2 +- frontend/src/lib/api/auth.ts | 6 +- frontend/src/lib/entryUrl.ts | 26 ++++++ frontend/src/lib/useVerifyEntry.ts | 52 +++++++++++ frontend/src/lib/verifyEntry.dom.test.tsx | 87 +++++++++++++++++++ 15 files changed, 337 insertions(+), 73 deletions(-) create mode 100644 frontend/src/components/VerifyBanner.tsx create mode 100644 frontend/src/lib/entryUrl.ts create mode 100644 frontend/src/lib/useVerifyEntry.ts create mode 100644 frontend/src/lib/verifyEntry.dom.test.tsx diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index c6ff8a91..f60b469c 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -678,8 +678,13 @@ def verify_email_confirm(body: TokenIn, db: Session = Depends(get_db)) -> dict: ) user.email_verified = True needs_password = user.password_hash is None and user.password_revoked_at is not None + email = user.email db.commit() - return {"ok": True, "password_reset_required": needs_password} + # Naming the address lets the SPA say WHICH mailbox it just confirmed — the same reasoning as + # password_reset_required above: holding a valid single-use verify token for this account already + # proves that much, so echoing it back leaks nothing. It matters in a browser that holds a session + # for a different account, where an unqualified "email confirmed" would be actively misleading. + return {"ok": True, "password_reset_required": needs_password, "email": email} @router.post("/verify/resend") @@ -715,20 +720,6 @@ def _resend_verify_if_eligible(email: str) -> None: email_mod.send_verify_email(email, f"{_app_base()}/#verify={raw}") -@router.get("/verify") -def verify_email(token: str, db: Session = Depends(get_db)): - """Legacy query-token verification link (pre-SB3 emails still in flight). New emails use the - fragment + POST flow above; this bounces to a friendly status for older links until they expire.""" - user = _consume_token(db, token, "verify") - if user is None: - return RedirectResponse(url="/?verify=invalid") - user.email_verified = True - needs_password = user.password_hash is None and user.password_revoked_at is not None - db.commit() - # Same two outcomes as the POST flow above, so an old link in flight lands on the same guidance. - return RedirectResponse(url="/?verify=" + ("ok_reset" if needs_password else "ok")) - - @router.post("/password-login") def password_login( body: EmailPasswordIn, diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index 35ef8143..b3a3fcd3 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -157,7 +157,11 @@ def test_verifying_afterwards_lets_the_original_password_in(client, db, make_use verified = client.post("/auth/verify", json={"token": raw}) assert verified.status_code == 200 - assert verified.json() == {"ok": True, "password_reset_required": False} + assert verified.json() == { + "ok": True, + "password_reset_required": False, + "email": "ordering@example.com", + } assert _login(client, "ordering@example.com", "chosen at registration").status_code == 200 @@ -515,7 +519,11 @@ def test_verify_tells_a_revoked_account_that_it_needs_a_new_password(client, db, r = client.post("/auth/verify", json={"token": raw}) assert r.status_code == 200 - assert r.json() == {"ok": True, "password_reset_required": True} + assert r.json() == { + "ok": True, + "password_reset_required": True, + "email": "needspw@example.com", + } def test_verify_reports_no_reset_needed_for_an_intact_account(client, db, make_user): @@ -528,4 +536,8 @@ def test_verify_reports_no_reset_needed_for_an_intact_account(client, db, make_u r = client.post("/auth/verify", json={"token": raw}) assert r.status_code == 200 - assert r.json() == {"ok": True, "password_reset_required": False} + assert r.json() == { + "ok": True, + "password_reset_required": False, + "email": "intact@example.com", + } diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 9ec6a00d..5d4975d2 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -17,6 +17,7 @@ import { import { notify, removeByMetaKind } from "./lib/notifications"; import { LS, setAccountRaw } from "./lib/storage"; import { usePref } from "./lib/usePref"; +import { useVerifyEntry } from "./lib/useVerifyEntry"; import { useNavigation, useNavigationActions } from "./components/NavigationProvider"; import { useFeedFilters, useFeedFiltersActions } from "./components/FeedFiltersProvider"; import { useChannelLayout } from "./components/ChannelsProvider"; @@ -37,6 +38,7 @@ import { StateMessage } from "./components/QueryState"; import ErrorDialog from "./components/ErrorDialog"; import VersionBanner from "./components/VersionBanner"; import DemoBanner from "./components/DemoBanner"; +import VerifyBanner from "./components/VerifyBanner"; import { focusAccessRequestsTab } from "./lib/adminUsersTab"; // The built app version, NOT imported from releaseNotes — importing anything from that module // pulls its whole ~900-line changelog into the eager App chunk and defeats the lazy ReleaseNotes @@ -54,6 +56,11 @@ export default function App() { const { t, i18n } = useTranslation(); const { page, channelView, ytSearch } = useNavigation(); const { setPage, enterYtSearch } = useNavigationActions(); + // An emailed verification link is confirmed HERE, above the signed-in/signed-out branch below, so + // it works in a browser that already holds a session. It used to be handled inside Welcome, which + // that branch never renders when signed in — the token was silently dropped. Signed out, the + // outcome goes to the auth card's banners; signed in, to the top-of-app bar. + const verifyEntry = useVerifyEntry(); // The global feed filters live in FeedFiltersProvider (per-account persistence + the account-load // effect + share-link capture all moved there). App reads them to hand to and to the // handlers that mutate the feed view (onShowInFeed, tag-click, the feed scrollKey). @@ -288,7 +295,7 @@ export default function App() { ); - if (!meQuery.data) return ; + if (!meQuery.data) return ; // Shown briefly while a lazy page/module chunk loads (first visit to that page only). const pageFallback = ( @@ -386,6 +393,7 @@ export default function App() { header={
} banners={ <> + {meQuery.data!.is_demo && } openReleaseNotes(FRONTEND_VERSION)} /> diff --git a/frontend/src/components/Banner.tsx b/frontend/src/components/Banner.tsx index ecdc536d..e175ab9b 100644 --- a/frontend/src/components/Banner.tsx +++ b/frontend/src/components/Banner.tsx @@ -26,6 +26,7 @@ export default function Banner({ action, onDismiss, dismissTitle, + role, }: { tone: BannerTone; icon: LucideIcon; @@ -33,10 +34,17 @@ export default function Banner({ action?: { label: string; onClick: () => void }; onDismiss?: () => void; dismissTitle?: string; + /** `"status"` for a bar that APPEARS in response to something the user just did, so a screen + * reader announces it. Left unset for the permanent bars (demo, version): those are part of the + * page when it loads, and announcing them on every visit is noise, not information. */ + role?: "status"; }) { const c = TONES[tone]; return ( -
+
{children} {action && ( diff --git a/frontend/src/components/VerifyBanner.tsx b/frontend/src/components/VerifyBanner.tsx new file mode 100644 index 00000000..0d99bab2 --- /dev/null +++ b/frontend/src/components/VerifyBanner.tsx @@ -0,0 +1,41 @@ +import { useTranslation } from "react-i18next"; +import { CheckCircle2, AlertTriangle } from "lucide-react"; +import Banner from "./Banner"; +import type { VerifyEntry } from "../lib/useVerifyEntry"; + +// The top-of-app outcome of an emailed verification link opened in a browser that ALREADY holds a +// session. That case used to be silent: the token was only ever consumed inside Welcome.tsx, which a +// signed-in visit never reaches, so the link opened a tab, the fragment was stripped, and nothing +// happened. Signed OUT, the same outcome is rendered by the auth card's own banners instead — this +// bar is only for the signed-in half. +// +// No "send a new link" action here, unlike the auth card: this viewer is signed in, so the sign-in +// screen (where resending lives) is not where they are, and the message is a report, not a task. A +// wrong-account case is why the address is named. +export default function VerifyBanner({ entry }: { entry: VerifyEntry }) { + const { t } = useTranslation(); + if (!entry.outcome) return null; + + const ok = entry.outcome !== "invalid"; + const text = + entry.outcome === "invalid" + ? t("header.verify.invalid") + : entry.email + ? t("header.verify.confirmedNamed", { email: entry.email }) + : t("header.verify.confirmed"); + + return ( + + {text} + {entry.outcome === "ok_reset" && ( + {t("header.verify.reset")} + )} + + ); +} diff --git a/frontend/src/components/Welcome.tsx b/frontend/src/components/Welcome.tsx index a016d015..bc20ef52 100644 --- a/frontend/src/components/Welcome.tsx +++ b/frontend/src/components/Welcome.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import { useHistorySubview, type SubviewUrl } from "../lib/history"; import { useTranslation } from "react-i18next"; import { @@ -24,6 +24,8 @@ import { HttpError } from "../lib/api"; import { setLanguage, type LangCode } from "../i18n"; import LanguageSwitcher from "./LanguageSwitcher"; import { inputCls } from "./ui/form"; +import { entryHash, entrySearch } from "../lib/entryUrl"; +import type { VerifyEntry } from "../lib/useVerifyEntry"; const EMAIL_RE = /^[^@\s]+@[^@\s]+\.[^@\s]+$/; const PASSWORD_MIN = 10; @@ -63,7 +65,7 @@ const SUBMIT_KEY: Record = { // password, Google, explicit demo) over a short marketing pitch with feature cards. Shown // whenever the user isn't signed in (see App.tsx). The auth forms talk to /auth/* (see the // backend); errors are surfaced inline (the api calls use the quiet flag). -export default function Welcome() { +export default function Welcome({ verify }: { verify: VerifyEntry }) { const { t, i18n } = useTranslation(); // Which screenshot (if any) is open in the full-size lightbox. const [zoomed, setZoomed] = useState<{ src: string; label: string } | null>(null); @@ -87,7 +89,7 @@ export default function Welcome() {

{t("welcome.hero.subtitle")}

{t("welcome.hero.trust")}

- + {/* App preview */} @@ -301,51 +303,24 @@ function Lightbox({ src, label, onClose }: { src: string; label: string; onClose ); } -function AuthCard() { +function AuthCard({ verify }: { verify: VerifyEntry }) { const { t } = useTranslation(); - // Snapshot the pre-login params ONCE (query + fragment) so we can clean the address bar below - // without dismissing the banners/reset-mode they drive. Secret tokens (reset, verify) ride the - // URL FRAGMENT so they never reach the server / proxy logs / Referer (SB3); plain status flags - // stay in the query string. - const [params] = useState(() => new URLSearchParams(window.location.search)); - const [hashParams] = useState(() => new URLSearchParams(window.location.hash.replace(/^#/, ""))); + // The entry URL is snapshotted once at import time (lib/entryUrl) rather than here, so nothing can + // rewrite the address bar out from under this card before it mounts — and so the App-level hook and + // this card read the SAME values regardless of which mounts first. Clearing the address bar is that + // hook's job now, since it is the one thing that runs on every visit, signed in or not. + // // State, not a const: once the token has been spent the reset view must stop being the card's // root, or a Back from the "password changed" screen lands on a form whose token is already used. - const [resetToken, setResetToken] = useState(() => hashParams.get("reset")); - const verifyToken = hashParams.get("verify"); // raw token → POST to confirm (new flow) - const bounced = params.get("access"); // Google denied → "requested" | "denied" - const deleted = params.get("deleted"); // self-service account deletion just completed + const [resetToken, setResetToken] = useState(() => entryHash.get("reset")); + const bounced = entrySearch.get("access"); // Google denied → "requested" | "denied" + const deleted = entrySearch.get("deleted"); // self-service account deletion just completed // Google login outcome → "suspended" | "oauth_cancelled". State (not a const) so it clears on the // first deliberate action, like the verify banner — otherwise it lingers stalely on later screens. - const [login, setLogin] = useState(() => params.get("login")); - // Verification outcome: from POSTing the fragment token (new flow), or the legacy - // ?verify=ok|invalid redirect (pre-SB3 emails still in flight). - const [verify, setVerify] = useState(() => params.get("verify")); - - // Strip BOTH the query string and the fragment so the address bar stays clean and the token - // doesn't linger in history. - useEffect(() => { - if (window.location.search || window.location.hash) { - window.history.replaceState(null, "", window.location.pathname); - } - }, []); - - // Confirm a fragment verification token by POSTing it (new SB3 flow). Fire ONCE: the token is - // single-use, so a double POST (StrictMode remount / any re-render) would 400 on the second call - // and wrongly flip the banner to "invalid". - const verifyFired = useRef(false); - useEffect(() => { - if (!verifyToken || verifyFired.current) return; - verifyFired.current = true; - api - .verifyEmail(verifyToken) - // "ok_reset": verified, but SEC-10 had dropped the password this account registered with. Sign-in - // would answer with the uniform credentials error, so point the user at the reset flow instead. - // Same value the legacy ?verify= redirect uses. Rare since E9.5 S1 (admin approval no longer - // revokes) — it still covers accounts stripped before that and the Google-adoption path. - .then((r) => setVerify(r.password_reset_required ? "ok_reset" : "ok")) - .catch(() => setVerify("invalid")); - }, [verifyToken]); + const [login, setLogin] = useState(() => entrySearch.get("login")); + // The verification outcome is confirmed in App (above the signed-in branch) and handed down, so + // one emailed link behaves the same whether or not this browser already holds a session. + const verifyOutcome = verify.outcome; const { view: mode, open: setMode } = useHistorySubview( resetToken ? "reset" : "signin", @@ -386,7 +361,7 @@ function AuthCard() { setDone(""); // The verify/login banners are one-time entry notices — dismiss once the user takes any // deliberate action (mode switch, CTA, submit) so they don't linger stalely on later screens. - setVerify(null); + verify.dismiss(); setLogin(null); }; @@ -475,8 +450,8 @@ function AuthCard() {

{t(titleKey)}

{/* One-time status banners from a redirect (Google-denied, email verification). */} - {verify === "ok" && {t("welcome.auth.verifyOk")}} - {verify === "ok_reset" && ( + {verifyOutcome === "ok" && {t("welcome.auth.verifyOk")}} + {verifyOutcome === "ok_reset" && ( {t("welcome.auth.verifyOkReset")}{" "} )} - {verify === "invalid" && ( + {verifyOutcome === "invalid" && ( {t("welcome.auth.verifyInvalid")}{" "}