From 3ac3d382e70889130bc456d7eee5b76d5aee31e1 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 2 Sep 2026 01:28:45 +0200 Subject: [PATCH 1/3] fix(auth): stop the address-bar mirror eating emailed auth tokens, and unlock revoked accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent bugs made an email+password registration unrecoverable in prod. The verification link did nothing. urlWithParams/clearParams rebuilt the URL as pathname+query and dropped the fragment — but the one-time auth tokens ride the fragment (#verify=/#reset=, SB3), and the providers that mirror nav params (useUrlMirror in FeedFilters/Playlists/Plex/Channels) sit ABOVE the lazily-loaded in main.tsx. Their mount effect replaceState'd the token away before the auth card's chunk had loaded, so the card read an empty hash: the link opened a bare sign-in page, no request, no banner. Both helpers now carry the fragment over. The account was then locked out for good. SEC-10 nulls password_hash when an unverified account is attested (here: admin approval landing before the user could verify — which the bug above made impossible). That is indistinguishable from a Google-only account, and reset eligibility keyed off password_hash alone, so no reset link was ever issued: no password to sign in with, and no way to get one. users.password_revoked_at records the revocation, reset eligibility honours it, and setting a password again (reset-confirm or set-password) clears it. An account that never had a password stays ineligible on purpose — its owner adds one from Settings. /auth/verify now also answers password_reset_required so the card can say why the registered password stopped working, instead of leaving the user at the uniform "Invalid email or password". Telling the holder of a valid verify token leaks nothing: they have already proved they own the mailbox. The 0078 migration backfills the stamp for rows carrying the exact lockout signature (local, non-demo, no password, unverified), so existing victims recover unaided. --- .../versions/0078_password_revoked_at.py | 50 +++++++ backend/app/models.py | 6 + backend/app/routes/auth.py | 37 ++++- backend/tests/test_auth_session.py | 131 ++++++++++++++++++ frontend/src/components/Welcome.tsx | 20 ++- frontend/src/i18n/locales/en/welcome.json | 2 + frontend/src/i18n/locales/hu/welcome.json | 2 + frontend/src/lib/api/auth.ts | 5 +- frontend/src/lib/urlState.dom.test.ts | 13 ++ frontend/src/lib/urlState.ts | 20 ++- frontend/src/lib/useUrlMirror.dom.test.ts | 10 ++ 11 files changed, 282 insertions(+), 14 deletions(-) create mode 100644 backend/alembic/versions/0078_password_revoked_at.py diff --git a/backend/alembic/versions/0078_password_revoked_at.py b/backend/alembic/versions/0078_password_revoked_at.py new file mode 100644 index 00000000..c6811125 --- /dev/null +++ b/backend/alembic/versions/0078_password_revoked_at.py @@ -0,0 +1,50 @@ +"""users.password_revoked_at — remember that SEC-10 dropped a preset password + +Revision ID: 0078_password_revoked_at +Revises: 0077_plex_playback_class +Create Date: 2026-09-02 + +SEC-10 (`auth.revoke_preset_password_if_unverified`) nulls `password_hash` when an account whose email +was never verified is attested (Google adoption, or admin approval). That left NO trace, and +password-reset eligibility keyed off `password_hash IS NOT NULL` — so such an account could neither +sign in (no password) nor request a reset link (not "eligible"): a permanent lockout with a silent, +uniform response at every step. + +This column is that trace. A row with `password_revoked_at` set is a local account that HAD a password +and had it revoked, so the reset flow treats it as eligible again. Setting a password (reset-confirm or +/auth/set-password) clears the stamp. A Google-only account that never had a password keeps it NULL and +stays reset-ineligible by design — its owner adds a password from Settings while signed in. + +Backfill: the pre-existing victims of the lockout can't be identified with certainty, but the signature +is exact and narrow — a local (google_sub IS NULL), non-demo account with no password whose email is +still unverified could ONLY have got there through the revoke, since registration always sets a hash. +Stamp those with the current time so they become recoverable without operator intervention. +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0078_password_revoked_at" +down_revision: Union[str, None] = "0077_plex_playback_class" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "users", + sa.Column("password_revoked_at", sa.DateTime(timezone=True), nullable=True), + ) + op.execute( + "UPDATE users SET password_revoked_at = now() " + "WHERE password_hash IS NULL " + "AND google_sub IS NULL " + "AND is_demo = false " + "AND email_verified = false" + ) + + +def downgrade() -> None: + op.drop_column("users", "password_revoked_at") diff --git a/backend/app/models.py b/backend/app/models.py index 315ca060..2544111b 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -47,6 +47,12 @@ class User(Base, TimestampMixin): email: Mapped[str] = mapped_column(String(320), unique=True, index=True) # argon2id hash for email+password login; NULL for Google-only / demo accounts. password_hash: Mapped[str | None] = mapped_column(String(255)) + # When SEC-10 dropped a preset password (see auth.revoke_preset_password_if_unverified). Without + # this trace a revoked account was indistinguishable from a Google-only one, so it failed the + # password-reset eligibility check and could never be recovered — it could neither sign in nor + # reset. Set = "had a password, lost it to the guard" -> reset-eligible. Cleared whenever a + # password is set again (reset-confirm / set-password). + password_revoked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) # Email ownership proven (Google sign-in is implicitly verified; password registration # verifies via a link). Login requires this AND is_active. email_verified: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false") diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index 58760f3b..8cc9c316 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -662,15 +662,22 @@ def _register_account(email: str, password: str) -> None: @router.post("/verify") def verify_email_confirm(body: TokenIn, db: Session = Depends(get_db)) -> dict: """Confirm an email-verification token the SPA read from the URL fragment (SB3). Uniform 400 - on any invalid/expired/used token.""" + on any invalid/expired/used token. + + `password_reset_required` tells the SPA to send the user straight to the reset flow: an admin who + approved this account BEFORE it was verified tripped SEC-10, so the password chosen at registration + is gone and sign-in would answer with the deliberately uniform "Invalid email or password" — true, + but unreadable to the person who just proved they own the mailbox. Saying so leaks nothing: holding + a valid single-use verify token for this account already proves that much.""" user = _consume_token(db, body.token, "verify") if user is None: raise HTTPException( status_code=400, detail="This verification link is invalid or has expired." ) user.email_verified = True + needs_password = user.password_hash is None and user.password_revoked_at is not None db.commit() - return {"ok": True} + return {"ok": True, "password_reset_required": needs_password} @router.post("/verify/resend") @@ -714,8 +721,10 @@ def verify_email(token: str, db: Session = Depends(get_db)): 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() - return RedirectResponse(url="/?verify=ok") + # 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") @@ -780,10 +789,18 @@ def _send_reset_if_eligible(email: str) -> None: """Background worker for password_reset_request: issue a reset token + email the link, but ONLY for a real (non-demo) password account. Runs after the response with its own DB session, so the request's timing never reveals whether the account exists. Token rides the URL FRAGMENT (#) so it - can't leak into proxy/access logs or a Referer (SB3).""" + can't leak into proxy/access logs or a Referer (SB3). + + "Password account" is `password_hash OR password_revoked_at` — an account SEC-10 stripped had a + password and must be able to get a new one, or it is locked out permanently (no password to sign in + with, and formerly no reset link either). A Google-only account that never had a password stays + ineligible on purpose: its owner adds one from Settings (/auth/set-password) while signed in, which + proves identity through the session rather than by mail.""" with SessionLocal() as db: user = db.execute(select(User).where(User.email == email)).scalar_one_or_none() - if user is not None and user.password_hash and not user.is_demo: + if user is None or user.is_demo: + return + if user.password_hash or user.password_revoked_at: raw = _issue_token(db, user, "reset", RESET_TTL) email_mod.send_password_reset(email, f"{_app_base()}/#reset={raw}") @@ -802,6 +819,7 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db)) if user is None: raise HTTPException(status_code=400, detail="This reset link is invalid or has expired.") user.password_hash = hash_password(new_password) + user.password_revoked_at = None # the account has a password again — SEC-10's trace is spent # A valid reset link proves control of the mailbox, so completing a reset also verifies the # email — this rescues an account that registered but never clicked the verification link (it # can then sign in). The separate admin-approval gate (is_active) is deliberately NOT touched @@ -844,9 +862,15 @@ def revoke_preset_password_if_unverified(user: User) -> None: ownership is finally attested (Google adoption, or admin approval), drop that password and bump the epoch to revoke any session it minted; the owner re-sets one via password-reset. No-op for an already-verified account, so a legit no-SMTP registration is unaffected. The single source for this - rule — both the Google-callback and admin-activation paths call it so they can't drift.""" + rule — both the Google-callback and admin-activation paths call it so they can't drift. + + `password_revoked_at` is what makes "the owner re-sets one via password-reset" actually true. The + revoke leaves `password_hash IS NULL`, which is also what a Google-only account looks like — and + _send_reset_if_eligible keyed off exactly that, so a revoked account silently got no reset link and + was locked out for good (it could not sign in either). The stamp separates the two cases.""" if not user.email_verified: user.password_hash = None + user.password_revoked_at = datetime.now(timezone.utc) bump_session_epoch(user) @@ -916,6 +940,7 @@ def set_password( if not verify_password(body.current_password or "", user.password_hash): raise HTTPException(status_code=403, detail="Current password is incorrect.") user.password_hash = hash_password(new_password) + user.password_revoked_at = None # same as the reset path: a password exists again # SA4: a password change logs out every OTHER session; re-stamp this cookie at the new epoch so # the person making the change stays signed in here. Commit BEFORE re-stamping so a failed commit # can't leave the cookie at a newer epoch than the DB (which would wrongly 401 THIS session). diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index b4e8adc0..fe63b04f 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -341,3 +341,134 @@ def test_register_route_is_refused_when_registration_is_closed(client, db, monke db.execute(_select(User).where(User.email == "closed@example.com")).scalar_one_or_none() is None ) + + +# --- SEC-10 lockout: a revoked preset password must stay RECOVERABLE --------------------------- +# +# Regression pack for the prod incident of 2026-09-02. An account registered with a password, then +# approved by an admin BEFORE its email was verified, ended up with password_hash = NULL. That is +# also what a Google-only account looks like, and reset eligibility keyed off `password_hash` alone — +# so the account could neither sign in (no password) nor obtain a reset link, and every response on +# the way was a deliberately uniform "ok"/401. `password_revoked_at` is the trace that tells the two +# apart; these pin each step of the way back in. + + +def _capture_reset_links(monkeypatch) -> list[str]: + """Patch the reset mailer and collect the recipients it is asked to write to.""" + from app.routes import auth as auth_mod + + sent: list[str] = [] + monkeypatch.setattr(auth_mod.email_mod, "send_password_reset", lambda to, link: sent.append(to)) + return sent + + +def test_the_approval_revoke_stamps_password_revoked_at(db, make_user): + from app.routes.admin import _activate_user_by_email + + victim = make_user( + email="stamped@example.com", + password="attacker-chosen", + email_verified=False, + is_active=False, + ) + assert victim.password_revoked_at is None + + _activate_user_by_email(db, "stamped@example.com") + + assert victim.password_hash is None + assert victim.password_revoked_at is not None # …so the reset flow can tell it HAD a password + + +def test_an_account_whose_password_was_revoked_can_still_get_a_reset_link( + db, make_user, monkeypatch +): + # THE lockout bug: without the stamp this sent nothing, and the account was unrecoverable. + from app.routes.admin import _activate_user_by_email + from app.routes.auth import _send_reset_if_eligible + + make_user( + email="lockedout@example.com", + password="chosen at registration", + email_verified=False, + is_active=False, + ) + _activate_user_by_email(db, "lockedout@example.com") + sent = _capture_reset_links(monkeypatch) + + _send_reset_if_eligible("lockedout@example.com") + + assert sent == ["lockedout@example.com"] + + +def test_a_google_only_account_is_still_not_reset_eligible(db, make_user, monkeypatch): + # The other side of the rule: an account that NEVER had a password gets no reset link — its owner + # adds one from Settings while signed in (/auth/set-password), which proves identity by session. + from app.routes.auth import _send_reset_if_eligible + + make_user(email="googler@example.com", password=None, google_sub="sub-123") + sent = _capture_reset_links(monkeypatch) + + _send_reset_if_eligible("googler@example.com") + + assert sent == [] + + +def test_completing_a_reset_clears_the_revocation_stamp(client, db, make_user): + from app.routes.admin import _activate_user_by_email + from app.routes.auth import _issue_token + + user = make_user( + email="recovered@example.com", + password="chosen at registration", + email_verified=False, + is_active=False, + ) + _activate_user_by_email(db, "recovered@example.com") + raw = _issue_token(db, user, "reset", timedelta(hours=1)) + db.commit() + + ok = client.post( + "/auth/password-reset/confirm", json={"token": raw, "password": "a brand new one"} + ) + + assert ok.status_code == 200 + db.expire_all() + assert user.password_hash is not None + assert user.password_revoked_at is None # spent — not a permanently reset-eligible account + assert _login(client, "recovered@example.com", "a brand new one").status_code == 200 + + +def test_verify_tells_a_revoked_account_that_it_needs_a_new_password(client, db, make_user): + # The banner's input. Saying so leaks nothing: the caller just proved mailbox control by holding a + # valid single-use verify token. Without it the user meets only the uniform "Invalid email or + # password" and has no way to learn why the password they chose stopped working. + from app.routes.admin import _activate_user_by_email + from app.routes.auth import _issue_token + + user = make_user( + email="needspw@example.com", + password="chosen at registration", + email_verified=False, + is_active=False, + ) + _activate_user_by_email(db, "needspw@example.com") + raw = _issue_token(db, user, "verify", timedelta(hours=1)) + db.commit() + + r = client.post("/auth/verify", json={"token": raw}) + + assert r.status_code == 200 + assert r.json() == {"ok": True, "password_reset_required": True} + + +def test_verify_reports_no_reset_needed_for_an_intact_account(client, db, make_user): + from app.routes.auth import _issue_token + + user = make_user(email="intact@example.com", email_verified=False, is_active=False) + raw = _issue_token(db, user, "verify", timedelta(hours=1)) + db.commit() + + r = client.post("/auth/verify", json={"token": raw}) + + assert r.status_code == 200 + assert r.json() == {"ok": True, "password_reset_required": False} diff --git a/frontend/src/components/Welcome.tsx b/frontend/src/components/Welcome.tsx index 3bded0d8..563181db 100644 --- a/frontend/src/components/Welcome.tsx +++ b/frontend/src/components/Welcome.tsx @@ -339,7 +339,10 @@ function AuthCard() { verifyFired.current = true; api .verifyEmail(verifyToken) - .then(() => setVerify("ok")) + // "ok_reset": verified, but SEC-10 had dropped the password this account registered with (an + // admin approved it before it was verified). 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. + .then((r) => setVerify(r.password_reset_required ? "ok_reset" : "ok")) .catch(() => setVerify("invalid")); }, [verifyToken]); @@ -392,6 +395,13 @@ function AuthCard() { setMode("resend"); }; + // Used by the "verified, but your password was revoked" banner — the only way back into that + // account is a reset link, so put the user one click from it. + const goForgot = () => { + reset(); + setMode("forgot"); + }; + async function onSubmit(e: React.FormEvent) { e.preventDefault(); reset(); @@ -465,6 +475,14 @@ function AuthCard() { {/* One-time status banners from a redirect (Google-denied, email verification). */} {verify === "ok" && {t("welcome.auth.verifyOk")}} + {verify === "ok_reset" && ( + + {t("welcome.auth.verifyOkReset")}{" "} + + + )} {verify === "invalid" && ( {t("welcome.auth.verifyInvalid")}{" "} diff --git a/frontend/src/i18n/locales/en/welcome.json b/frontend/src/i18n/locales/en/welcome.json index 2d7f071b..8c2b4f9f 100644 --- a/frontend/src/i18n/locales/en/welcome.json +++ b/frontend/src/i18n/locales/en/welcome.json @@ -86,6 +86,8 @@ "forgotDone": "If that email has an account, we've sent a reset link. Check your inbox.", "resetDone": "Your password has been set — you can sign in now.", "verifyOk": "Email confirmed. Once an admin approves your account, you can sign in.", + "verifyOkReset": "Email confirmed. Your account was approved before you confirmed it, so the password you registered with was cleared for security — choose a new one to sign in.", + "verifyOkResetCta": "Set a new password", "verifyInvalid": "That verification link is invalid or has expired.", "resendCta": "Send a new link", "resendDone": "If that email has an unverified account, we've sent a new verification link. Check your inbox.", diff --git a/frontend/src/i18n/locales/hu/welcome.json b/frontend/src/i18n/locales/hu/welcome.json index 73a7a293..c9ecbdaf 100644 --- a/frontend/src/i18n/locales/hu/welcome.json +++ b/frontend/src/i18n/locales/hu/welcome.json @@ -86,6 +86,8 @@ "forgotDone": "Ha ehhez az e-mailhez tartozik fiók, elküldtük a visszaállító linket. Nézd meg a postafiókod.", "resetDone": "A jelszavad be van állítva — most már beléphetsz.", "verifyOk": "E-mail megerősítve. Amint egy admin jóváhagyja a fiókodat, beléphetsz.", + "verifyOkReset": "E-mail megerősítve. A fiókodat még a megerősítés előtt jóváhagyták, ezért biztonsági okból töröltük a regisztrációkor megadott jelszavadat — válassz újat a belépéshez.", + "verifyOkResetCta": "Új jelszó beállítása", "verifyInvalid": "Ez a megerősítő link érvénytelen vagy lejárt.", "resendCta": "Új link küldése", "resendDone": "Ha ehhez az e-mailhez tartozik megerősítetlen fiók, elküldtünk egy új megerősítő linket. Nézd meg a postafiókod.", diff --git a/frontend/src/lib/api/auth.ts b/frontend/src/lib/api/auth.ts index 5567837e..66cd7416 100644 --- a/frontend/src/lib/api/auth.ts +++ b/frontend/src/lib/api/auth.ts @@ -37,7 +37,10 @@ export const authApi = { { quiet: true }, ), // Confirm an email-verification token the SPA read from the URL fragment (SB3). - verifyEmail: (token: string): Promise<{ ok: boolean }> => + // `password_reset_required` = SEC-10 dropped the password this account registered with (an admin + // approved it before it was verified), so signing in would fail with the uniform credentials error. + // The card sends the user to the reset flow instead of leaving them to guess. + verifyEmail: (token: string): Promise<{ ok: boolean; password_reset_required?: boolean }> => req("/auth/verify", { method: "POST", body: JSON.stringify({ token }) }, { quiet: true }), // Re-issue a verification link (the first one expired, or was lost). Uniform response — never // reveals whether the email has a pending account. diff --git a/frontend/src/lib/urlState.dom.test.ts b/frontend/src/lib/urlState.dom.test.ts index 051f1a9c..463cd7c1 100644 --- a/frontend/src/lib/urlState.dom.test.ts +++ b/frontend/src/lib/urlState.dom.test.ts @@ -64,6 +64,19 @@ describe("urlWithParams", () => { setLocation("?page=channels"); expect(urlWithParams({ page: null })).toBe("/app"); }); + + it("carries the fragment over — the one-time auth tokens ride it", () => { + // Regression (prod, 2026-09-02): the providers that mirror nav params mount ABOVE the lazy , + // so this URL was replaceState'd before the auth card's chunk loaded. Dropping the hash ate the + // #verify= token and the verification link opened a bare sign-in page. + window.history.replaceState(null, "", "/app?page=plex#verify=tok123"); + expect(urlWithParams({ v: "show:1" })).toBe("/app?page=plex&v=show%3A1#verify=tok123"); + }); + + it("carries the fragment over even when the query empties out", () => { + window.history.replaceState(null, "", "/app?page=channels#reset=tok456"); + expect(urlWithParams({ page: null })).toBe("/app#reset=tok456"); + }); }); describe("clearParams", () => { diff --git a/frontend/src/lib/urlState.ts b/frontend/src/lib/urlState.ts index c5a3801f..bfa4470c 100644 --- a/frontend/src/lib/urlState.ts +++ b/frontend/src/lib/urlState.ts @@ -146,10 +146,16 @@ export function readPage(): Page { return isPage(p) ? p : "feed"; } -/** Build a URL (path + query) from the CURRENT location's query with `updates` applied — a null value - * deletes that key, any other string sets it. Lets each provider own its own nav key(s) — page / chan / - * v / pl / the feed-filter keys — and update the address bar without clobbering the others (read-modify- - * write on the live query). The caller decides pushState vs replaceState with the returned string. */ +/** Build a URL (path + query + fragment) from the CURRENT location with `updates` applied to the query — + * a null value deletes that key, any other string sets it. Lets each provider own its own nav key(s) — + * page / chan / v / pl / the feed-filter keys — and update the address bar without clobbering the others + * (read-modify-write on the live query). The caller decides pushState vs replaceState with the string. + * + * ⚠️ The FRAGMENT must be carried over. The one-time auth tokens ride it (`#verify=…` / `#reset=…`, SB3), + * and the providers that mirror nav params (useUrlMirror in FeedFilters/Playlists/Plex/Channels) sit ABOVE + * the lazily-loaded in main.tsx — their mount effect replaceState's this URL before the auth card's + * chunk has even loaded. Dropping the hash here silently ate the token: the verification link opened a + * bare sign-in page and the account could never be verified. */ export function urlWithParams(updates: Record): string { const params = new URLSearchParams(window.location.search); for (const [k, v] of Object.entries(updates)) { @@ -157,7 +163,7 @@ export function urlWithParams(updates: Record): string { else params.set(k, v); } const qs = params.toString(); - return `${window.location.pathname}${qs ? `?${qs}` : ""}`; + return `${window.location.pathname}${qs ? `?${qs}` : ""}${window.location.hash}`; } /** Remove specific TRANSIENT params (one-time auth/admin links: ?link, ?oauth, ?admin, ?access, ?verify, @@ -178,6 +184,8 @@ export function clearParams(...keys: string[]): void { window.history.replaceState( window.history.state, "", - `${window.location.pathname}${qs ? `?${qs}` : ""}`, + // Same fragment rule as urlWithParams: this strips QUERY keys only, so a `#verify=…`/`#reset=…` + // token that has not been consumed yet must survive. Welcome's own cleanup is what clears it. + `${window.location.pathname}${qs ? `?${qs}` : ""}${window.location.hash}`, ); } diff --git a/frontend/src/lib/useUrlMirror.dom.test.ts b/frontend/src/lib/useUrlMirror.dom.test.ts index d1d85f55..3c9d2faa 100644 --- a/frontend/src/lib/useUrlMirror.dom.test.ts +++ b/frontend/src/lib/useUrlMirror.dom.test.ts @@ -48,4 +48,14 @@ describe("useUrlMirror", () => { renderHook(() => useUrlMirror({ pl: "5" })); expect(window.location.search).toBe("?page=playlists&pl=5"); }); + + it("leaves an unconsumed auth token in the fragment alone", () => { + // This hook's mount effect is what ran before (and therefore before the auth card) had even + // loaded, so whatever it writes to the address bar is what the card later reads. It must not eat + // the #verify=/#reset= token that the emailed link carries. + window.history.replaceState(null, "", "/?page=playlists#verify=tok123"); + renderHook(() => useUrlMirror({ pl: "5" })); + expect(window.location.hash).toBe("#verify=tok123"); + expect(window.location.search).toBe("?page=playlists&pl=5"); + }); }); From f8c63e80c2b7a67e231289e6a0e728d6309e32d9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 2 Sep 2026 02:21:46 +0200 Subject: [PATCH 2/3] fix(auth): widen the 0078 backfill so revoke victims who verified are rescued too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the backfill's fourth predicate `AND email_verified = false` strands exactly the subset that needs it most. The revoke does not invalidate the account's outstanding verification token, so clicking the emailed link is the natural next thing such a user does — and it sets email_verified=True while password_hash stays NULL. That row then has no route back at all: reset ineligible, resend refuses an already-verified address, set-password needs a session the account cannot obtain, and nothing re-stamps it because the revoke itself is guarded on being unverified. The remaining three predicates already pin the victim set exactly: registration and the setup wizard always set a hash, every Google row carries a google_sub (adoption assigns it before revoking), and nothing in the codebase ever clears google_sub — so a local, non-demo, passwordless row can only have come from the revoke. Widening is therefore safe, and it is the argument the docstring was already making while the SQL contradicted it. Also corrects that docstring: set-password is unreachable because the account cannot authenticate at all, not because the epoch bump killed a session — an unverified account is refused at login before a session is ever minted, so it never had one. --- .../versions/0078_password_revoked_at.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/backend/alembic/versions/0078_password_revoked_at.py b/backend/alembic/versions/0078_password_revoked_at.py index c6811125..e025c264 100644 --- a/backend/alembic/versions/0078_password_revoked_at.py +++ b/backend/alembic/versions/0078_password_revoked_at.py @@ -15,10 +15,19 @@ and had it revoked, so the reset flow treats it as eligible again. Setting a pas /auth/set-password) clears the stamp. A Google-only account that never had a password keeps it NULL and stays reset-ineligible by design — its owner adds a password from Settings while signed in. -Backfill: the pre-existing victims of the lockout can't be identified with certainty, but the signature -is exact and narrow — a local (google_sub IS NULL), non-demo account with no password whose email is -still unverified could ONLY have got there through the revoke, since registration always sets a hash. -Stamp those with the current time so they become recoverable without operator intervention. +Backfill: the signature is exact and narrow — a local (google_sub IS NULL), non-demo account with NO +password could ONLY have got there through the revoke. Registration always sets a hash, the setup +wizard sets one too, every Google row carries a google_sub (the adoption path assigns it BEFORE +revoking), and nothing in the codebase ever clears google_sub. Stamp those with the current time so +they become recoverable without operator intervention. + +Deliberately NOT also requiring `email_verified = false`, even though the revoke only fires on an +unverified account: verifying is exactly what such a victim does NEXT. The revoke does not invalidate +the outstanding verification token, so clicking the emailed link sets email_verified=True while +password_hash stays NULL — and that row has no route back at all (reset ineligible, resend refuses an +already-verified address, and set-password needs a signed-in session the account can no longer obtain: +no password to sign in with, and no Google identity). Those are the victims who most need the stamp, +so the predicate must not exclude them. """ from typing import Sequence, Union @@ -41,8 +50,7 @@ def upgrade() -> None: "UPDATE users SET password_revoked_at = now() " "WHERE password_hash IS NULL " "AND google_sub IS NULL " - "AND is_demo = false " - "AND email_verified = false" + "AND is_demo = false" ) From 2ecc70ca6dd8cec092f5417304ce93a0bd9c9ce7 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 2 Sep 2026 02:31:14 +0200 Subject: [PATCH 3/3] release: bump VERSION to 0.78.7 + release notes --- VERSION | 2 +- frontend/src/lib/releaseNotes.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index ba4cbeab..a2744c83 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.78.6 \ No newline at end of file +0.78.7 \ No newline at end of file diff --git a/frontend/src/lib/releaseNotes.ts b/frontend/src/lib/releaseNotes.ts index 035ed7ef..f9760bde 100644 --- a/frontend/src/lib/releaseNotes.ts +++ b/frontend/src/lib/releaseNotes.ts @@ -14,6 +14,21 @@ export interface ReleaseEntry { } export const RELEASE_NOTES: ReleaseEntry[] = [ + { + version: "0.78.7", + date: "2026-09-02", + summary: + "The verification link in your sign-up email did nothing — it opened a plain sign-in page. Accounts an admin approved before that link was clicked lost the password they registered with, and could not get a reset link either, which left them shut out with no way back.", + fixes: [ + "Confirming your email works again. The link carries its token in the part of the address after the #, and the app was wiping that away as it started up, before the sign-in card ever read it — so the click looked like it had done nothing at all. Password-reset links arrived through the same door and were lost the same way.", + "If an admin approved your account before you confirmed your email, the password you chose at sign-up is deleted for security — someone else could have registered your address and chosen it. That part is intended, but it used to be a dead end: sign-in refused you, and Forgot password sent nothing, because the account no longer looked like a password account. It does now, so a reset link reaches you and you can choose a new password.", + "Confirming your email in that situation now says so, and offers the reset in the same breath, instead of leaving you to work out why a password you just typed correctly is being refused.", + "Existing accounts already stuck this way are repaired on upgrade — including those that had confirmed their email after the fact, who had the least way out. An account that has only ever signed in with Google is deliberately unaffected: it still adds a password from Settings while signed in, not by email.", + ], + chores: [ + "urlWithParams/clearParams now carry window.location.hash. The nav-param mirrors (useUrlMirror in the FeedFilters/Playlists/Plex/Channels providers) mount ABOVE the lazily-loaded , so their mount effect replaceState'd the fragment away before AuthCard snapshotted it — the emailed token never reached POST /auth/verify. New users.password_revoked_at (migration 0078) records SEC-10's revoke so _send_reset_if_eligible can tell a stripped account from a Google-only one; cleared on reset-confirm and set-password. POST /auth/verify returns password_reset_required, driving an ok_reset banner. The backfill stamps every local, non-demo, passwordless row — deliberately not also requiring email_verified=false, since the revoke leaves the verify token usable and those victims have no other route back. Deep-reviewed across seven dimensions: nine findings, one confirmed (that backfill predicate) and fixed; the rest logged.", + ], + }, { version: "0.78.6", date: "2026-09-01",