From a851fb213b299c661947e62f89d8c1573b118a36 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 23 Jul 2026 00:15:26 +0200 Subject: [PATCH] =?UTF-8?q?feat(auth):=20R4=20S1=20=E2=80=94=20fix=20auth?= =?UTF-8?q?=20dead=20ends?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - password reset also sets email_verified (the mail proves the mailbox), rescuing an account that never clicked the verify link; is_active (admin approval) is deliberately left untouched - add POST /auth/verify/resend (rate-limited, off-response-path, anti-enum) + a "resend" mode/CTA on the verifyInvalid banner, so an expired/lost verification link is no longer a dead end (re-register was a silent no-op) - OAuth cancel/deny now redirects to /?login=oauth_cancelled with a friendly banner instead of a raw 400 JSON page; pop the oauth_link markers BEFORE the token exchange so an aborted link flow can't poison the next clean sign-in --- backend/app/auth.py | 72 +++++++++++++++++++---- frontend/src/components/Welcome.tsx | 38 ++++++++++-- frontend/src/i18n/locales/en/welcome.json | 5 ++ frontend/src/i18n/locales/hu/welcome.json | 5 ++ frontend/src/lib/api.ts | 8 +++ 5 files changed, 112 insertions(+), 16 deletions(-) diff --git a/backend/app/auth.py b/backend/app/auth.py index cb5b83e..a1de24c 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -31,6 +31,7 @@ _DECOY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16)) _register_limiter = RateLimiter(max_events=5, window_seconds=300) _login_limiter = RateLimiter(max_events=10, window_seconds=300) _reset_limiter = RateLimiter(max_events=5, window_seconds=300) +_verify_resend_limiter = RateLimiter(max_events=5, window_seconds=300) _request_access_limiter = RateLimiter(max_events=5, window_seconds=300) # At most one "your account is suspended" email per address per hour, so a suspended user who # keeps retrying (or a script with their valid credentials) can't be used to mailbomb them. @@ -274,10 +275,21 @@ async def callback( client = google_oauth(db) if client is None: return RedirectResponse(url="/") + # Linking mode: an already-authenticated account is attaching this Google identity (a + # password account enabling YouTube or connecting SSO). The markers are set by /auth/link and + # /auth/upgrade; when present we attach to the current user instead of identifying by sub. + # Pop them BEFORE the token exchange: if the user aborts Google's consent screen the exchange + # throws below, and a marker left behind would silently turn the NEXT clean sign-in into a link + # attempt. Popping up front means an aborted link flow leaves no residue in the session. + link_uid = request.session.pop("oauth_link_uid", None) + link_explicit = request.session.pop("oauth_link_explicit", False) try: token = await client.authorize_access_token(request) except OAuthError as exc: - raise HTTPException(status_code=400, detail=f"OAuth error: {exc.error}") + # The user cancelled or denied consent at Google — land back on the login page with a + # friendly banner instead of a raw 400 JSON error page (applies to sign-in and link flows). + log.info("Google OAuth cancelled/failed: %s", exc.error) + return RedirectResponse(url="/?login=oauth_cancelled") userinfo = token.get("userinfo") if not userinfo or not userinfo.get("sub"): @@ -286,12 +298,8 @@ async def callback( sub = userinfo["sub"] email = (userinfo.get("email") or "").lower() - # Linking mode: an already-authenticated account is attaching this Google identity (a - # password account enabling YouTube or connecting SSO). The marker is set by /auth/link and - # /auth/upgrade; when present we attach to the current user instead of identifying by sub. - link_uid = request.session.pop("oauth_link_uid", None) if link_uid is not None: - return _complete_link(request, db, link_uid, sub, userinfo, token) + return _complete_link(request, db, link_uid, link_explicit, sub, userinfo, token) if not is_allowed(db, email): log.warning("Login denied (not approved): %s", email or "") @@ -456,13 +464,19 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None: def _complete_link( - request: Request, db: Session, link_uid: int, sub: str, userinfo: dict, token: dict + request: Request, + db: Session, + link_uid: int, + explicit: bool, + sub: str, + userinfo: dict, + token: dict, ): """Attach the just-authorized Google identity to the already-signed-in account that started the link/upgrade. No is_allowed gate — they're already an active user. Refuses to hijack a - Google identity owned by another account, or to silently swap a different linked identity.""" - # Consume the companion marker up front so it never lingers across the early returns below. - explicit = request.session.pop("oauth_link_explicit", False) + Google identity owned by another account, or to silently swap a different linked identity. + `explicit` (the popped oauth_link_explicit marker) distinguishes a "Connect Google" click from + a YouTube upgrade — the caller pops it up front so an aborted flow leaves nothing behind.""" # The marker must match the live session user (guards a stale/forged marker). if link_uid != request.session.get("user_id"): return RedirectResponse(url="/?link=error") @@ -693,6 +707,39 @@ def verify_email_confirm(payload: dict, db: Session = Depends(get_db)) -> dict: return {"ok": True} +@router.post("/verify/resend") +def verify_email_resend( + payload: dict, + request: Request, + background: BackgroundTasks, +) -> dict: + """Re-issue an email-verification link. Needed because the original link expires (VERIFY_TTL) + and re-registering an already-registered email is a silent no-op, so a user who lost the first + mail had no way back. Uniform response + timing regardless of whether the email has a pending + account (the lookup runs off the response path), so it can't probe for registered emails.""" + if not _verify_resend_limiter.allow(_client_ip(request)): + return {"status": "ok"} # silently throttle; uniform response + email = (payload.get("email") or "").strip().lower() + if valid_email(email): + background.add_task(_resend_verify_if_eligible, email) + return {"status": "ok"} + + +def _resend_verify_if_eligible(email: str) -> None: + """Background worker for verify/resend: issue a fresh verify token + email the link, but ONLY for + a real account that still needs verifying. Runs after the response with its own DB session, so the + request's timing never reveals whether the account exists. No-op when SMTP is disabled (accounts + are auto-verified on a no-SMTP instance, so there is nothing to resend). Token rides the URL + FRAGMENT (#) so it can't leak into proxy/access logs or a Referer (SB3).""" + if not email_mod.email_enabled(): + return + with SessionLocal() as db: + user = db.execute(select(User).where(User.email == email)).scalar_one_or_none() + if user is not None and not user.email_verified and not user.is_demo: + raw = _issue_token(db, user, "verify", VERIFY_TTL) + 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 @@ -787,6 +834,11 @@ def password_reset_confirm(payload: dict, db: Session = Depends(get_db)) -> dict 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) + # 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 + # here: a reset proves email ownership, not that an admin approved the account. + user.email_verified = True bump_session_epoch(user) # SA4: a reset (often a compromise response) kills all existing sessions. # Burn any other outstanding reset tokens for this user. for row in db.execute( diff --git a/frontend/src/components/Welcome.tsx b/frontend/src/components/Welcome.tsx index f899cd4..da6942b 100644 --- a/frontend/src/components/Welcome.tsx +++ b/frontend/src/components/Welcome.tsx @@ -24,7 +24,7 @@ import LanguageSwitcher from "./LanguageSwitcher"; const EMAIL_RE = /^[^@\s]+@[^@\s]+\.[^@\s]+$/; const PASSWORD_MIN = 10; -type Mode = "signin" | "register" | "forgot" | "reset"; +type Mode = "signin" | "register" | "forgot" | "reset" | "resend"; // Public landing + authentication. One scrollable page: a hero with the auth card (email+ // password, Google, explicit demo) over a short marketing pitch with feature cards. Shown @@ -327,7 +327,10 @@ function AuthCard() { e.preventDefault(); reset(); const mail = email.trim().toLowerCase(); - if ((mode === "signin" || mode === "register" || mode === "forgot") && !EMAIL_RE.test(mail)) { + if ( + (mode === "signin" || mode === "register" || mode === "forgot" || mode === "resend") && + !EMAIL_RE.test(mail) + ) { setError(t("welcome.auth.invalidEmail")); return; } @@ -348,6 +351,9 @@ function AuthCard() { } else if (mode === "forgot") { await api.requestPasswordReset(mail); setDone(t("welcome.auth.forgotDone")); + } else if (mode === "resend") { + await api.resendVerification(mail); + setDone(t("welcome.auth.resendDone")); } else if (mode === "reset") { await api.confirmPasswordReset(resetToken ?? "", password); setDone(t("welcome.auth.resetDone")); @@ -390,7 +396,9 @@ function AuthCard() { ? "welcome.auth.forgotTitle" : mode === "reset" ? "welcome.auth.resetTitle" - : "welcome.auth.signinTitle"; + : mode === "resend" + ? "welcome.auth.resendTitle" + : "welcome.auth.signinTitle"; return (
@@ -398,10 +406,26 @@ function AuthCard() { {/* One-time status banners from a redirect (Google-denied, email verification). */} {verify === "ok" && {t("welcome.auth.verifyOk")}} - {verify === "invalid" && {t("welcome.auth.verifyInvalid")}} + {verify === "invalid" && ( + + {t("welcome.auth.verifyInvalid")}{" "} + + + )} {bounced === "requested" && {t("welcome.auth.accessRequested")}} {bounced === "denied" && {t("welcome.auth.accessDenied")}} {login === "suspended" && {t("welcome.auth.suspended")}} + {login === "oauth_cancelled" && ( + {t("welcome.auth.oauthCancelled")} + )} {deleted && {t("welcome.auth.deleted")}} {done ? ( @@ -431,7 +455,7 @@ function AuthCard() { className={inputCls} /> )} - {mode !== "forgot" && ( + {mode !== "forgot" && mode !== "resend" && ( diff --git a/frontend/src/i18n/locales/en/welcome.json b/frontend/src/i18n/locales/en/welcome.json index 40daa11..c5d32e3 100644 --- a/frontend/src/i18n/locales/en/welcome.json +++ b/frontend/src/i18n/locales/en/welcome.json @@ -57,6 +57,8 @@ "createButton": "Create account", "forgotButton": "Send reset link", "resetButton": "Set password", + "resendButton": "Resend verification", + "resendTitle": "Resend verification email", "createLink": "Create an account", "forgotLink": "Forgot password?", "backToSignin": "Back to sign in", @@ -72,6 +74,9 @@ "resetDone": "Your password has been set — you can sign in now.", "verifyOk": "Email confirmed. Once an admin approves your account, you can sign in.", "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.", + "oauthCancelled": "Google sign-in was cancelled. You can try again or use email and password.", "accessRequested": "Thanks — we recorded your request. An admin will review it.", "accessDenied": "That account isn't approved for this instance yet — register below or ask the admin for access.", "suspended": "This account has been suspended and can't sign in. If you think this is a mistake, contact the operator (check your email for details).", diff --git a/frontend/src/i18n/locales/hu/welcome.json b/frontend/src/i18n/locales/hu/welcome.json index 03a4689..d3ba005 100644 --- a/frontend/src/i18n/locales/hu/welcome.json +++ b/frontend/src/i18n/locales/hu/welcome.json @@ -57,6 +57,8 @@ "createButton": "Fiók létrehozása", "forgotButton": "Visszaállító link küldése", "resetButton": "Jelszó beállítása", + "resendButton": "Megerősítő link újraküldése", + "resendTitle": "Megerősítő e-mail újraküldése", "createLink": "Hozz létre egy fiókot", "forgotLink": "Elfelejtetted a jelszót?", "backToSignin": "Vissza a belépéshez", @@ -72,6 +74,9 @@ "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.", "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.", + "oauthCancelled": "A Google-belépés megszakadt. Próbáld újra, vagy lépj be e-maillel és jelszóval.", "accessRequested": "Köszönjük — rögzítettük a kérésed. Egy admin felülvizsgálja.", "accessDenied": "Ez a fiók még nincs jóváhagyva ehhez a példányhoz — regisztrálj lent, vagy kérj hozzáférést az admintól.", "suspended": "Ez a fiók fel van függesztve, nem tud belépni. Ha tévedésnek gondolod, keresd az üzemeltetőt (a részletek az e-mailedben).", diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 94b47c9..5294090 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -1557,6 +1557,14 @@ export const api = { // Confirm an email-verification token the SPA read from the URL fragment (SB3). verifyEmail: (token: string): Promise<{ ok: 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. + resendVerification: (email: string): Promise<{ status: string }> => + req( + "/auth/verify/resend", + { method: "POST", body: JSON.stringify({ email }) }, + { quiet: true }, + ), // SA4: sign every OTHER session for this account out (keeps the current one). logoutOthers: (): Promise<{ ok: boolean }> => req("/auth/logout-others", { method: "POST" }), // Set or change the signed-in account's password (errors shown inline via quiet).