diff --git a/backend/app/auth.py b/backend/app/auth.py index c908f924..bf765b07 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -25,8 +25,6 @@ def operator_contact() -> str | None: return next(iter(sorted(settings.admin_email_set)), None) -MAX_SESSION_ACCOUNTS = 6 - # OAuth scope vocabulary — the atoms + the request bundles, kept together as core. has_*_scope check # the atoms against a user's stored grant; the *_SCOPES bundles are what the login/callback endpoints # request (imported by app/routes/auth.py). @@ -253,94 +251,37 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None: background.add_task(email_mod.send_account_deleted, email, lang, operator_contact()) -ACTIVE_ACCOUNT_HEADER = "x-siftlode-account" +def session_user_id(conn: HTTPConnection) -> int | None: + """The account this browser session signed in as, read from the signed cookie alone (no DB). + One account per session: signing in as another replaces it (`routes.auth._establish_session`).""" + return conn.session.get("user_id") -def resolved_user_id(conn: HTTPConnection, *, strict: bool = True) -> tuple[int | None, bool]: - """Which account this connection acts as, and whether that's the session's default account. - Takes an HTTPConnection rather than a Request: it reads only the session and the headers. - - The signed cookie holds the *wallet* (`account_ids`, every account signed into this browser) - plus a default `user_id`. A caller may name its account with the X-Siftlode-Account header (or - `?account=`), but ONLY an account in the wallet — so a tab can't impersonate an account that - never authenticated here. A connection that names no account acts as the cookie default. - - A connection naming an account NOT in the wallet acts as nobody (`None`) — typically a tab still - pinned to an account that was signed out in another tab. It must not fall back to the default: - that tab would go on acting as an account it never showed, its player checkpoints and pref saves - landing there and its Sign out signing THAT account out. Only the bootstrap /api/me probe passes - `strict=False` and gets the default, which is how such a tab learns who it now is (the frontend - re-pins it and reloads). An account whose session ended while it stayed in the wallet (deleted, - suspended, revoked) leaves the wallet when it is refused (`_session_user`), so a tab pinned to it - takes the same way back. - """ - default_id = conn.session.get("user_id") - wallet = conn.session.get("account_ids") or [] - # Header for normal XHR; ?account= for a context that can't set headers (a plain file - # download). Both are wallet-gated below. - hdr = conn.headers.get(ACTIVE_ACCOUNT_HEADER) or conn.query_params.get("account") - if hdr: - try: - hid = int(hdr) - except ValueError: - # Under `strict` this becomes a 401 and the tab authenticates as nobody, which reads - # in the browser exactly like a session that expired. The value itself stays out — - # it came from a request, and `wallet` says which accounts were actually on offer. - log.debug( - "the active-account header could not be read as an id", - extra=logs.event("auth.account_header.invalid", accounts=len(wallet)), - ) - hid = None - if hid is not None and hid in wallet: - return hid, hid == default_id - if strict: - return None, False - return default_id, True - - -def drop_from_wallet(session: dict, user_id: int) -> None: - """Take one account out of this browser's wallet (`account_ids`); the session default is untouched.""" - session["account_ids"] = [a for a in (session.get("account_ids") or []) if a != user_id] - - -def _end_account_session(request: Request, user_id: int, is_default: bool) -> None: - """An account whose session here is no longer valid. Only nuke the whole session when it is the - SESSION DEFAULT; a per-tab account just leaves the wallet (other tabs' default may still be valid). - Leaving is what lets a tab pinned to it recover: its next /api/me names an account no longer signed - in here and gets the default. Left in, it answered "signed out" — the landing page, with the default - still signed in, and no way off it but closing the tab.""" - if is_default: - request.session.clear() - else: - drop_from_wallet(request.session, user_id) +def stamp_session(request: Request, user: User) -> None: + """Record the account's current `session_epoch` in the signed cookie, so `current_user` can later + reject the cookie once the epoch moves (password reset/change, or "log out everywhere").""" + request.session["epoch"] = user.session_epoch or 0 def current_user(request: Request, db: Session = Depends(get_db)) -> User: - return _session_user(request, db, *resolved_user_id(request)) + return _session_user(request, db, session_user_id(request)) -def _session_user(request: Request, db: Session, user_id: int | None, is_default: bool) -> User: +def _session_user(request: Request, db: Session, user_id: int | None) -> User: if not user_id: raise refused(401, "not_authenticated") user = db.get(User, user_id) if user is None or not user.is_active or user.is_suspended: # Suspended/deactivated mid-session → end it so the block takes effect at once. - _end_account_session(request, user_id, is_default) + request.session.clear() raise refused(401, "not_authenticated") - # SA4: reject a cookie whose recorded epoch is behind the account's current one (revoked by a - # password reset/change or "log out everywhere"). A cookie with no recorded epoch (pre-SA4, or an - # account signed in before this shipped) is treated as epoch 0 — so the FIRST bump revokes it too. - epochs = request.session.get("epochs") or {} - if int(epochs.get(str(user_id), 0)) != (user.session_epoch or 0): - _end_account_session(request, user_id, is_default) + # A cookie whose epoch is behind the account's (revoked by a password reset/change or "log out + # everywhere") is refused — and so is one with no epoch at all, which only a build before the + # one-account session wrote: its holder signs in once more. + epoch = request.session.get("epoch") + if epoch is None or int(epoch) != (user.session_epoch or 0): + request.session.clear() raise refused(401, "not_authenticated") - # Always keep the session default in the switchable list — covers sessions created before - # multi-session existed (their account_ids was never seeded), so the switcher isn't empty. - default_id = request.session.get("user_id") - accounts = request.session.get("account_ids") or [] - if default_id and default_id not in accounts: - accounts.append(default_id) - request.session["account_ids"] = accounts[-MAX_SESSION_ACCOUNTS:] # Attribute this request's log lines to the account. Here, not in the request middleware: # this is the one place a session becomes an account, and the middleware runs outside both the # session middleware and this task's context. `request.state` is shared across middleware layers @@ -358,25 +299,17 @@ def _session_user(request: Request, db: Session, user_id: int | None, is_default def optional_current_user(request: Request, db: Session = Depends(get_db)) -> User | None: """Like `current_user` but returns None instead of raising 401 when there's no valid session. Used by the bootstrap /api/me probe so the logged-out landing doesn't log a failed request to - the browser console (a non-2xx fetch is a console error no matter how the app handles it). - The one lenient resolution (`strict=False`): a tab naming an account no longer signed in here gets - the default account, which is how it learns who it now is — see `resolved_user_id`. That includes an - account refused right here, which `_session_user` has just taken out of the wallet.""" - user_id, is_default = resolved_user_id(request, strict=False) + the browser console (a non-2xx fetch is a console error no matter how the app handles it).""" + user_id = session_user_id(request) + if user_id is None: + return None try: - return _session_user(request, db, user_id, is_default) - # silent-ok: this is the anonymous case as much as the refused one — every landing page hit - # comes through here. The second attempt below is the handling, and it reports. + return _session_user(request, db, user_id) except HTTPException: - if is_default: - return None - try: - return _session_user(request, db, *resolved_user_id(request, strict=False)) - except HTTPException: - # Both the pinned account AND the default one were refused, so the tab is signed out - # without being told why — and it got here from a session that named a real account. + # A session that named an account, refused: the browser is signed out without being told + # why, so say it here. log.info( - "a session was resolved to nobody: neither the named nor the default account held", + "a session was refused: its account no longer holds", extra=logs.event("auth.session.probe_rejected", user_id=user_id), ) return None diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index 336d665e..ff659b09 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -25,7 +25,6 @@ from app import sysconfig from app.auth import ( _YOUTUBE_SCOPES, BASE_SCOPES, - MAX_SESSION_ACCOUNTS, PASSWORD_MIN_LEN, READ_SCOPES, WRITE_SCOPES, @@ -37,7 +36,8 @@ from app.auth import ( is_allowed, log, operator_contact, - resolved_user_id, + session_user_id, + stamp_session, ) from app.config import settings from app.db import SessionLocal, get_db @@ -361,13 +361,13 @@ async def callback( token = await client.authorize_access_token(request) except OAuthError as exc: # The user cancelled or denied consent at Google — land back with a friendly notice instead - # of a raw 400 JSON error page. An already-signed-in user (adding another account via - # /auth/login, or linking/upgrading via /auth/link|upgrade) never sees the pre-auth login - # banner, so surface a neutral toast via ?oauth=cancelled; a fresh sign-in cancel lands on - # the login page with the oauth_cancelled banner instead. Keyed on the live session, not on - # link_uid, so the add-account path (no marker) is covered too. + # of a raw 400 JSON error page. An already-signed-in user (signing in again via /auth/login, + # or linking/upgrading via /auth/link|upgrade) never sees the pre-auth login banner, so + # surface a neutral toast via ?oauth=cancelled; a fresh sign-in cancel lands on the login page + # with the oauth_cancelled banner instead. Keyed on the live session, not on link_uid, so a + # sign-in started while signed in (no marker) is covered too. log.info("Google OAuth cancelled/failed: %s", exc.error) - if request.session.get("user_id") is not None: + if session_user_id(request) is not None: return RedirectResponse(url="/?oauth=cancelled") return RedirectResponse(url="/?login=oauth_cancelled") @@ -584,12 +584,9 @@ 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.""" - # 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: + # The marker must name the account still signed into THIS browser: a sign-in as someone else + # between the start and the callback replaced it, and the identity must not land on the newcomer. + if link_uid != session_user_id(request): return RedirectResponse(url="/?link=error") user = db.get(User, link_uid) if user is None: @@ -619,38 +616,17 @@ def _complete_link( @router.post("/logout") async def logout(request: Request): - """Sign the requesting tab's active account out of this browser (removing it from the wallet). - Per-tab aware: the account is taken from the X-Siftlode-Account header when present, so logging - out in one tab doesn't disturb another tab running a different account. If the account being - removed was the session default, promote the most recent remaining account (or clear the - session when none remain).""" - # A tab still pinned to an account already signed out in another tab resolves to nobody, so it - # signs nobody out — it used to fall back to the default and sign THAT account out in its place. - active, is_default = resolved_user_id(request) - remaining = [a for a in (request.session.get("account_ids") or []) if a != active] - request.session["account_ids"] = remaining - if is_default: - if remaining: - request.session["user_id"] = remaining[-1] - return JSONResponse({"ok": True, "switched": True}) - request.session.clear() - return JSONResponse({"ok": True, "switched": False}) - # A non-default (per-tab) account signed out: it's gone from the wallet and the session default - # is untouched. The tab drops its own override and falls back to the default on reload. - return JSONResponse({"ok": True, "switched": False}) + """Sign this browser out: the session holds one account, and it ends.""" + request.session.clear() + return JSONResponse({"ok": True}) def _establish_session(request: Request, user: User) -> None: """Sign `user` in for this browser session — the ONE sign-in path, shared by the Google callback - and password login, so both join the same account switcher and both stamp `last_login_at` (the - caller commits). Multi-session: every account that authenticated in this browser is remembered so - the user can switch between them; only accounts that actually proved themselves HERE land in that - wallet (it is the proof they may be switched to).""" - accounts = [a for a in (request.session.get("account_ids") or []) if a != user.id] - accounts.append(user.id) - request.session["account_ids"] = accounts[-MAX_SESSION_ACCOUNTS:] + and password login, so both stamp `last_login_at` (the caller commits). One account per session: + the sign-in replaces whoever was signed in here before.""" request.session["user_id"] = user.id - _remember_epoch(request, user) + stamp_session(request, user) user.last_login_at = datetime.now(timezone.utc) @@ -1135,15 +1111,6 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db)) return {"ok": True} -def _remember_epoch(request: Request, user: User) -> None: - """Record this account's current `session_epoch` in the signed cookie (SA4), so `current_user` - can later reject the cookie if the account's epoch was bumped (password reset/change, or - "log out everywhere"). Per-account so one account's revocation doesn't evict the others.""" - epochs = dict(request.session.get("epochs") or {}) - epochs[str(user.id)] = user.session_epoch - request.session["epochs"] = epochs - - def bump_session_epoch(db: Session, user: User) -> None: """Invalidate every EXISTING signed cookie for this account by advancing its epoch (SA4). Cookies minted before the bump carry a now-stale epoch and are rejected by `current_user`. The account's @@ -1220,7 +1187,7 @@ def logout_others( # session at a newer epoch than the DB (see set_password). bump_session_epoch(db, user) db.commit() - _remember_epoch(request, user) + stamp_session(request, user) log.info("Logout-others: uid=%s (epoch=%s)", user.id, user.session_epoch) return {"ok": True} @@ -1272,7 +1239,7 @@ def set_password( # can't leave the cookie at a newer epoch than the DB (which would wrongly 401 THIS session). bump_session_epoch(db, user) db.commit() - _remember_epoch(request, user) + stamp_session(request, user) log.info("Password set/changed: uid=%s", user.id) return {"ok": True} diff --git a/backend/app/routes/me.py b/backend/app/routes/me.py index e6b79e35..d3a3d5cd 100644 --- a/backend/app/routes/me.py +++ b/backend/app/routes/me.py @@ -9,11 +9,9 @@ from app.auth import ( count_admins, count_pending_activations, current_user, - drop_from_wallet, google_enabled, has_read_scope, has_write_scope, - is_allowed, optional_current_user, purge_user, ) @@ -26,79 +24,10 @@ from app.db import get_db from app.models import Invite, User from app.utils import iso from app.refusal import refused -from app.routes._common import not_found router = APIRouter(prefix="/api/me", tags=["me"]) -@router.get("/accounts") -def my_accounts( - request: Request, - user: User = Depends(current_user), - db: Session = Depends(get_db), -) -> list[dict]: - """The accounts that have authenticated in this browser session (switchable without a - new Google sign-in). The active one is flagged.""" - ids = request.session.get("account_ids") or [user.id] - rows = {u.id: u for u in db.execute(select(User).where(User.id.in_(ids))).scalars()} - out = [] - for uid in ids: # preserve recency order from the session - u = rows.get(uid) - if u is not None: - out.append( - { - "id": u.id, - "email": u.email, - "display_name": u.display_name, - "avatar_url": u.avatar_url, - "active": u.id == user.id, - } - ) - return out - - -class SwitchAccountIn(BaseModel): - user_id: int - - -@router.post("/switch") -def switch_account( - body: SwitchAccountIn, - request: Request, - user: User = Depends(current_user), - db: Session = Depends(get_db), -) -> dict: - """Switch the active account to another one already authenticated in this browser. No - Google round-trip — but only accounts present in the session list (proof they signed in - here) are allowed, and access is re-checked in case the invite was revoked since.""" - target = body.user_id - if target not in (request.session.get("account_ids") or []): - raise refused(403, "not_your_account") - # An account `_session_user` would refuse — gone, deactivated, suspended, or its session revoked — is - # refused here too, and dropped from the switch list the way `_session_user` drops it. Made the session - # default instead, the next request's current_user would refuse it as the DEFAULT and clear the whole - # wallet session, logging out every account here. (B-core-9) - u = db.get(User, target) - if u is None: - drop_from_wallet(request.session, target) - raise not_found("account") - if not is_allowed(db, u.email): - raise refused(403, "account_no_access") - if not u.is_active: - drop_from_wallet(request.session, target) - raise refused(403, "account_no_access") - if u.is_suspended: - drop_from_wallet(request.session, target) - raise refused(403, "account_suspended") - # Revoked: a password reset/change or "log out everywhere" bumped the account's epoch. - epochs = request.session.get("epochs") or {} - if int(epochs.get(str(target), 0)) != (u.session_epoch or 0): - drop_from_wallet(request.session, target) - raise refused(403, "account_signed_out") - request.session["user_id"] = target - return {"ok": True, "user_id": target} - - @router.get("") def get_me( user: User | None = Depends(optional_current_user), db: Session = Depends(get_db) @@ -187,17 +116,11 @@ def delete_my_account( surfaces).""" if user.role == "admin" and count_admins(db) <= 1: raise refused(400, "only_admin_delete") - user_id = user.id # Full erasure (cascades) + access-request cleanup + Google-grant revocation; shared with the - # admin delete path. Capture the id first — `user` is detached once purge_user deletes the row. + # admin delete path. purge_user(db, user, background) - # Drop this account from the browser session; switch to another signed-in account if any. - remaining = [a for a in (request.session.get("account_ids") or []) if a != user_id] - if remaining: - request.session["account_ids"] = remaining - request.session["user_id"] = remaining[-1] - else: - request.session.clear() + # The session's one account is gone: so is the session. + request.session.clear() return {"deleted": True} diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index d8cea012..77b7c231 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -19,7 +19,7 @@ from fastapi.responses import FileResponse from pydantic import BaseModel from sqlalchemy.orm import Session -from app.auth import current_user, resolved_user_id +from app.auth import current_user, session_user_id from app.db import SessionLocal, get_db from app.models import User from app.plex import imgcache, queries, service, subtitles @@ -42,7 +42,7 @@ def _require_session(request: Request) -> int: """Lightweight auth for the high-fan-out image proxies: validate the signed session cookie WITHOUT a DB user-load, so these endpoints never hold a pool connection for the auth. Poster images are low-sensitivity; a valid session is enough.""" - uid, _ = resolved_user_id(request) + uid = session_user_id(request) if not uid: raise refused(401, "not_authenticated") return uid diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index f1ec9bd2..611f22fc 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -1,21 +1,25 @@ """Auth / session security core (TEST-1), DB-lane. -The wallet impersonation gate, epoch-based session revocation, single-use tokens and the -enumeration-safe uniform 401 are the security mag, and had zero tests (only `_oauth_redirect_uri` was -covered). Starlette's SessionMiddleware round-trips the signed cookie under TestClient, so the whole -browser-less flow is exercisable. Uses the real auth stack via the `client` fixture (NO current_user -override) so login, the wallet gate and epoch revoke are the code under test. +One account per browser session, epoch-based session revocation, single-use tokens and the +enumeration-safe uniform 401 are the security mag. Starlette's SessionMiddleware round-trips the +signed cookie under TestClient, so the whole browser-less flow is exercisable. Uses the real auth +stack via the `client` fixture (NO current_user override) so login, the session and epoch revoke are +the code under test. Note: `/api/me` is the bootstrap probe — it returns 200 `{authenticated: False}` when logged out, not a 401. A revoked session is therefore checked on a PROTECTED endpoint (`/api/feed`), which does 401. """ +import base64 +import json from datetime import timedelta +from pathlib import Path import pytest from fastapi.testclient import TestClient +from itsdangerous import TimestampSigner -from app.auth import ACTIVE_ACCOUNT_HEADER +from app.config import settings from app.main import app _PW = "correct horse battery" @@ -46,74 +50,48 @@ def test_wrong_password_and_unknown_email_give_a_byte_identical_401(client, make assert wrong_pw.json() == unknown.json() -def test_a_non_wallet_account_header_falls_back_to_the_default(client, make_user): - # The X-Siftlode-Account header may only re-target an account already signed into THIS browser - # (in the session wallet). A header naming someone else must be ignored, not honored — otherwise - # it's an impersonation gate hole. - make_user(email="alice2@example.com", password=_PW) - mallory = make_user(email="mallory@example.com", password=_PW) - assert _login(client, "alice2@example.com", _PW).status_code == 200 - me = client.get("/api/me", headers={ACTIVE_ACCOUNT_HEADER: str(mallory.id)}) - assert me.status_code == 200 - assert me.json()["email"] == "alice2@example.com" # still Alice, not Mallory +def _cookie(payload: dict) -> str: + """A session cookie signed the way SessionMiddleware signs one — for a cookie an older build wrote.""" + data = base64.b64encode(json.dumps(payload).encode()) + return TimestampSigner(str(settings.secret_key)).sign(data).decode() -def test_a_tab_pinned_to_an_account_signed_out_elsewhere_acts_as_nobody(client, make_user): - # Two accounts in one browser: a tab pinned to Alice, Bob the default. Alice signs out in another - # tab. The pinned tab used to go on acting as the default, Bob — its player checkpoints and pref - # saves landed in his account, and its own Sign out signed HIM out. - alice = make_user(email="alice3@example.com", password=_PW) +def test_signing_in_as_another_account_replaces_the_first(client, make_user): + # One account per browser session: a second sign-in is a fresh session, not a second account + # beside the first — so signing out leaves nobody signed in here. + make_user(email="alice3@example.com", password=_PW) make_user(email="bob3@example.com", password=_PW) assert _login(client, "alice3@example.com", _PW).status_code == 200 - assert _login(client, "bob3@example.com", _PW).status_code == 200 # Bob is now the default - tab = {ACTIVE_ACCOUNT_HEADER: str(alice.id)} - assert client.get("/api/feed", headers=tab).status_code == 200 - assert client.post("/auth/logout", headers=tab).status_code == 200 # Alice signs out + assert _login(client, "bob3@example.com", _PW).status_code == 200 + assert client.get("/api/me").json()["email"] == "bob3@example.com" - assert client.get("/api/feed", headers=tab).status_code == 401 - assert client.post("/auth/logout", headers=tab).status_code == 200 - assert client.get("/api/feed").status_code == 200 # Bob is still signed in, the session intact - # The bootstrap probe still answers as the default: how the stale tab learns who it is now. - assert client.get("/api/me", headers=tab).json()["email"] == "bob3@example.com" + assert client.post("/auth/logout").status_code == 200 + assert client.get("/api/feed").status_code == 401 + assert client.get("/api/me").json() == {"authenticated": False} -@pytest.mark.parametrize("ended", ["revoked", "suspended"]) -def test_a_tab_pinned_to_an_account_whose_session_ended_comes_back_as_the_default( - client, make_user, db, ended -): - # A tab pinned to Alice, Bob the default. Alice's session then ends while she is still in this - # browser's wallet ("log out everywhere" on another device, or an admin suspends her). The tab's - # bootstrap probe used to answer "signed out": the landing page with Bob still signed in, and no way - # off it but closing the tab (signing in as anyone else there kept the tab pinned to Alice). +def test_no_header_or_parameter_picks_another_account(client, make_user): + # Alice signed in here before Bob: nothing a request carries makes it act as her any more. alice = make_user(email="alice4@example.com", password=_PW) make_user(email="bob4@example.com", password=_PW) assert _login(client, "alice4@example.com", _PW).status_code == 200 assert _login(client, "bob4@example.com", _PW).status_code == 200 - tab = {ACTIVE_ACCOUNT_HEADER: str(alice.id)} - assert client.get("/api/feed", headers=tab).status_code == 200 - if ended == "revoked": - alice.session_epoch = (alice.session_epoch or 0) + 1 - else: - alice.is_suspended = True - db.commit() + me = client.get(f"/api/me?account={alice.id}", headers={"X-Siftlode-Account": str(alice.id)}) + assert me.json()["email"] == "bob4@example.com" - assert client.get("/api/me", headers=tab).json()["email"] == "bob4@example.com" - assert client.get("/api/feed", headers=tab).status_code == 401 # still nobody, never Bob - assert client.get("/api/feed").status_code == 200 # Bob's session is intact + +@pytest.mark.parametrize("path", ["/api/me/accounts", "/api/me/switch"]) +def test_there_is_no_account_list_or_switch(client, make_user, path): + make_user(email="alice5@example.com", password=_PW) + assert _login(client, "alice5@example.com", _PW).status_code == 200 + r = client.get(path) if path.endswith("accounts") else client.post(path, json={"user_id": 1}) + assert r.status_code in (404, 405) @pytest.mark.parametrize("ended", ["revoked", "suspended", "deactivated"]) -def test_switching_to_an_account_whose_session_ended_refuses_it_and_drops_it_from_the_list( - client, make_user, db, monkeypatch, ended -): - # The account switcher lists this browser's wallet. A revoked account was dropped from it on a refused - # switch, but a suspended one stayed, refused on every click; a deactivated one was not refused at all — - # made the default, the next request found it inactive and signed every account here out. - monkeypatch.setattr("app.routes.me.is_allowed", lambda _db, _email: True) # both hold an invite - alice = make_user(email="alice5@example.com", password=_PW) - make_user(email="bob5@example.com", password=_PW) - assert _login(client, "alice5@example.com", _PW).status_code == 200 - assert _login(client, "bob5@example.com", _PW).status_code == 200 +def test_a_session_whose_account_ended_signs_the_browser_out(client, make_user, db, ended): + alice = make_user(email="alice6@example.com", password=_PW) + assert _login(client, "alice6@example.com", _PW).status_code == 200 if ended == "revoked": alice.session_epoch = (alice.session_epoch or 0) + 1 elif ended == "suspended": @@ -121,10 +99,70 @@ def test_switching_to_an_account_whose_session_ended_refuses_it_and_drops_it_fro else: alice.is_active = False db.commit() + assert client.get("/api/feed").status_code == 401 + assert "user_id" not in _session(client) # the refusal ended the session, not just this request + assert client.get("/api/me").json() == {"authenticated": False} - assert client.post("/api/me/switch", json={"user_id": alice.id}).status_code == 403 - assert [a["email"] for a in client.get("/api/me/accounts").json()] == ["bob5@example.com"] - assert client.get("/api/feed").status_code == 200 # Bob is still signed in + +def test_a_cookie_from_the_multi_account_build_signs_in_again(client, make_user): + # The old session held a wallet and one epoch per account. It is not read: whoever holds one + # signs in once more. + alice = make_user(email="alice7@example.com", password=_PW) + old = {"user_id": alice.id, "account_ids": [alice.id], "epochs": {str(alice.id): 0}} + client.cookies.set("session", _cookie(old)) + assert client.get("/api/feed").status_code == 401 + assert client.get("/api/me").json() == {"authenticated": False} + + +def test_deleting_the_account_signs_the_browser_out(client, make_user): + make_user(email="alice8@example.com", password=_PW) + assert _login(client, "alice8@example.com", _PW).status_code == 200 + assert client.delete("/api/me/account").status_code == 200 + # The cookie names nobody now: the probe answers signed out without looking the account up. + assert "user_id" not in _session(client) + + +def _session(client) -> dict: + """The decoded payload of the client's session cookie.""" + raw = client.cookies.get("session") + if not raw: + return {} + data = TimestampSigner(str(settings.secret_key)).unsign(raw.encode()) + return json.loads(base64.b64decode(data)) + + +@pytest.mark.parametrize("signed_in_as", ["starter", "someone_else"]) +def test_a_google_link_lands_only_on_the_account_still_signed_in(db, make_user, signed_in_as): + # The link marker names who started it; a sign-in as someone else before the callback must not + # get that Google identity attached to the account that started it from the other person's session. + from starlette.requests import Request + + from app.routes.auth import _complete_link + + starter, other = make_user(), make_user() + session = {"user_id": starter.id if signed_in_as == "starter" else other.id} + request = Request({"type": "http", "session": session, "headers": []}) + resp = _complete_link(request, db, starter.id, True, "sub-123", {"email": ""}, {"scope": ""}) + db.refresh(starter) + if signed_in_as == "starter": + assert resp.headers["location"] == "/?link=ok" + assert starter.google_sub == "sub-123" + else: + assert resp.headers["location"] == "/?link=error" + assert starter.google_sub is None + + +def test_no_app_code_reads_a_second_account(): + # The multi-account machinery is gone; a module reading its session keys or its header again + # would bring back a session that names more than one account. + app_dir = Path(__file__).parent.parent / "app" + hits = [ + f"{path.relative_to(app_dir)}: {word}" + for path in app_dir.rglob("*.py") + for word in ("account_ids", '"epochs"', "x-siftlode-account", 'query_params.get("account")') + if word in path.read_text(encoding="utf-8").lower() + ] + assert hits == [] def test_logout_others_revokes_the_other_session_but_keeps_the_caller(client, make_user): diff --git a/backend/tests/test_plex_image_sizes.py b/backend/tests/test_plex_image_sizes.py index de996f5a..0f617bb1 100644 --- a/backend/tests/test_plex_image_sizes.py +++ b/backend/tests/test_plex_image_sizes.py @@ -48,7 +48,7 @@ def _setup(db, make_user, monkeypatch, tmp_path) -> TestClient: ) db.commit() monkeypatch.setattr(app_state, "_configured_cache", True) - monkeypatch.setattr(playback, "resolved_user_id", lambda request: (user.id, True)) + monkeypatch.setattr(playback, "session_user_id", lambda request: user.id) monkeypatch.setattr(playback, "PlexClient", _FakePlex) monkeypatch.setattr(imgcache, "CACHE_DIR", tmp_path) _FakePlex.calls = [] diff --git a/backend/tests/test_plex_subtitle_auth.py b/backend/tests/test_plex_subtitle_auth.py index c9beb4f8..906ba125 100644 --- a/backend/tests/test_plex_subtitle_auth.py +++ b/backend/tests/test_plex_subtitle_auth.py @@ -32,7 +32,7 @@ def test_a_suspended_account_cannot_drive_the_subtitle_extractor( monkeypatch.setattr(app_state, "_configured_cache", True) # A validly SIGNED cookie for that account — which is all the old gate looked at. for mod in (auth, playback): - monkeypatch.setattr(mod, "resolved_user_id", lambda request: (user.id, True)) + monkeypatch.setattr(mod, "session_user_id", lambda request: user.id) monkeypatch.setattr(subtitles, "CACHE_DIR", tmp_path) pool = _NoPool() monkeypatch.setattr(subtitles, "_WIN_POOL", pool) diff --git a/frontend/src/i18n/locales/en/errors.json b/frontend/src/i18n/locales/en/errors.json index be730977..cb21200e 100644 --- a/frontend/src/i18n/locales/en/errors.json +++ b/frontend/src/i18n/locales/en/errors.json @@ -24,7 +24,6 @@ "screenshot": "This screenshot doesn't exist any more", "link": "This share link doesn't exist any more", "notification": "This notification doesn't exist any more", - "account": "That account no longer exists", "collection": "This collection doesn't exist any more", "config_key": "There's no such setting", "file": "The file is no longer available", @@ -135,9 +134,6 @@ "gone": "This playlist no longer exists on YouTube" }, "refused": { - "account_no_access": "That account no longer has access", - "account_signed_out": "That account was signed out elsewhere — sign in again", - "account_suspended": "That account is suspended", "address_has_account": "That address already has an account — delete the account instead", "admin_first": "Create the admin account first", "admin_only": "Only an admin can do that", @@ -174,7 +170,6 @@ "not_authenticated": "You're not signed in", "not_ready": "Not available yet", "not_redownloadable": "This download can't be downloaded again", - "not_your_account": "That's not an account you've signed into here", "only_admin_delete": "You're the only admin — make someone else admin before deleting your account", "own_delete": "Delete your own account in Settings → Account", "own_role": "You can't change your own role", diff --git a/frontend/src/i18n/locales/hu/errors.json b/frontend/src/i18n/locales/hu/errors.json index 3c70c9c6..5e024fe3 100644 --- a/frontend/src/i18n/locales/hu/errors.json +++ b/frontend/src/i18n/locales/hu/errors.json @@ -24,7 +24,6 @@ "screenshot": "Ez a képernyőkép már nem létezik", "link": "Ez a megosztási link már nem létezik", "notification": "Ez az értesítés már nem létezik", - "account": "Ez a fiók már nem létezik", "collection": "Ez a kollekció már nem létezik", "config_key": "Nincs ilyen beállítás", "file": "A fájl már nem érhető el", @@ -135,9 +134,6 @@ "gone": "Ez a lejátszási lista már nem létezik a YouTube-on" }, "refused": { - "account_no_access": "Ennek a fióknak már nincs hozzáférése", - "account_signed_out": "Ezt a fiókot máshol kijelentkeztették — lépj be újra", - "account_suspended": "Ez a fiók fel van függesztve", "address_has_account": "Ehhez a címhez már tartozik fiók — azt töröld helyette", "admin_first": "Előbb hozd létre az admin fiókot", "admin_only": "Ezt csak admin teheti meg", @@ -174,7 +170,6 @@ "not_authenticated": "Nem vagy bejelentkezve", "not_ready": "Még nem érhető el", "not_redownloadable": "Ez a letöltés nem tölthető le újra", - "not_your_account": "Ebbe a fiókba itt nem léptél be", "only_admin_delete": "Te vagy az egyetlen admin — a fiókod törlése előtt tegyél meg valaki mást adminnak", "own_delete": "A saját fiókodat a Beállítások → Fiók alatt törölheted", "own_role": "A saját szerepkörödet nem módosíthatod",