release: 0.78.7 (bug/verify-link-and-approval-lockout)
gate / frontend (push) Failing after 1m44s
gate / backend (push) Successful in 2m42s

This commit is contained in:
2026-09-02 02:31:19 +02:00
13 changed files with 306 additions and 15 deletions
+1 -1
View File
@@ -1 +1 @@
0.78.6
0.78.7
@@ -0,0 +1,58 @@
"""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 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
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"
)
def downgrade() -> None:
op.drop_column("users", "password_revoked_at")
+6
View File
@@ -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")
+31 -6
View File
@@ -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).
+131
View File
@@ -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}
+19 -1
View File
@@ -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" && <Banner tone="ok">{t("welcome.auth.verifyOk")}</Banner>}
{verify === "ok_reset" && (
<Banner tone="warn">
{t("welcome.auth.verifyOkReset")}{" "}
<button onClick={goForgot} className="font-semibold text-accent hover:underline">
{t("welcome.auth.verifyOkResetCta")}
</button>
</Banner>
)}
{verify === "invalid" && (
<Banner tone="warn">
{t("welcome.auth.verifyInvalid")}{" "}
@@ -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.",
@@ -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.",
+4 -1
View File
@@ -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.
+15
View File
@@ -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 <App>, 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",
+13
View File
@@ -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 <App>,
// 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", () => {
+14 -6
View File
@@ -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 <App> 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, string | null>): 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, string | null>): 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}`,
);
}
+10
View File
@@ -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 <App> (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");
});
});