diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index c4bbfa3b..d69ae9c9 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -19,6 +19,7 @@ from app.auth import ( ) from app.db import get_db from app.demo import reset_demo_state +from app.routes.auth import bump_session_epoch from app.models import ( DemoWhitelist, Invite, @@ -62,6 +63,13 @@ def _activate_user_by_email(db: Session, email: str) -> None: it can sign in. No-op if no such user exists yet (a Google user is created at first login).""" user = db.execute(select(User).where(User.email == email.lower())).scalar_one_or_none() if user is not None and not user.is_active: + # SEC-10: same guard as the Google-adoption path — approving an account whose email was never + # verified must not activate a password set on it by whoever registered the address (a + # pre-emption). Drop the untrusted password + revoke any session it minted; the owner re-sets + # one via password-reset. (No-op when email_verified is True, e.g. legit no-SMTP registrations.) + if not user.email_verified: + user.password_hash = None + bump_session_epoch(user) user.is_active = True db.commit() diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index b71f8457..a2ba0a26 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -286,6 +286,14 @@ async def callback( return RedirectResponse(url="/?access=denied") user = existing user.google_sub = sub + # SEC-10: a password set on this account BEFORE its email was ever verified is untrusted — + # whoever registered the address (to pre-empt the real owner, who is only now arriving via + # Google) could have chosen it. Drop it now that Google attests ownership, and bump the epoch + # to revoke any session that pre-set password may have minted. The owner can set a fresh + # password via password-reset later if they also want email+password sign-in. + if not user.email_verified: + user.password_hash = None + bump_session_epoch(user) # Google verified the email and is_allowed granted access, so finish activating a # previously pending password registration (otherwise current_user would bounce it). user.email_verified = True @@ -392,8 +400,12 @@ def _complete_link( 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"): + # SEC-11: the marker must name an account signed into THIS browser — the session default OR any + # account in the wallet — not only the default. A tab acting as a secondary account links Google to + # ITSELF (the marker was set from current_user, which resolves the ?account= it navigated with), so + # accepting wallet membership stops the identity/token landing on the default account A instead of B. + accounts = request.session.get("account_ids") or [] + if link_uid != request.session.get("user_id") and link_uid not in accounts: return RedirectResponse(url="/?link=error") user = db.get(User, link_uid) if user is None or user.is_demo: diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index 0b05464f..c44dfe50 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -90,3 +90,32 @@ def test_reset_confirm_is_single_use_and_kills_existing_sessions(client, make_us # The token is single-use: a replay is a clean 400, not a second reset. replay = client.post("/auth/password-reset/confirm", json={"token": raw, "password": "another pw!!"}) assert replay.status_code == 400 + + +def test_activating_an_unverified_account_drops_its_preset_password(db, make_user): + # SEC-10 (account pre-emption): an attacker registers victim@x with a password before the approved + # owner arrives. The address was never verified, so admin approval must NOT hand that pre-set + # password a working login — it is dropped and any session it minted is revoked. + from app.routes.admin import _activate_user_by_email + + victim = make_user( + email="victim@example.com", password="attacker-chosen", email_verified=False, is_active=False + ) + epoch0 = victim.session_epoch or 0 + _activate_user_by_email(db, "victim@example.com") + assert victim.is_active is True + assert victim.password_hash is None + assert (victim.session_epoch or 0) == epoch0 + 1 + + +def test_activating_a_verified_account_keeps_its_password(db, make_user): + # A legit account whose email IS verified (e.g. a no-SMTP registration) keeps its password on + # approval — the guard must not break the normal flow. + from app.routes.admin import _activate_user_by_email + + user = make_user( + email="legit@example.com", password="my own password", email_verified=True, is_active=False + ) + _activate_user_by_email(db, "legit@example.com") + assert user.is_active is True + assert user.password_hash is not None diff --git a/frontend/src/components/SettingsPanel.tsx b/frontend/src/components/SettingsPanel.tsx index f8eaa900..4b6f233b 100644 --- a/frontend/src/components/SettingsPanel.tsx +++ b/frontend/src/components/SettingsPanel.tsx @@ -1,4 +1,5 @@ import { useState } from "react"; +import { withActiveAccount } from "../lib/activeAccount"; import { qk } from "../lib/queryKeys"; import { useTranslation } from "react-i18next"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; @@ -418,7 +419,7 @@ function SignInMethods({ me }: { me: Me }) { ) : (