refactor(core): the core's owner filters go through userscope, each held by a second account

Tokens, notifications, quota, debug sessions and the reset-link burn filtered
their owner inline. Ten of the fifteen filters had no test that noticed one
going missing; each now has a test where another account's rows must not
count, show or change.
This commit is contained in:
2026-10-19 23:36:01 +02:00
parent 531eb2eef1
commit ad6ccbac05
11 changed files with 164 additions and 15 deletions
+4 -1
View File
@@ -26,6 +26,7 @@ from fastapi import Depends, Request
from sqlalchemy import delete, or_, select, update from sqlalchemy import delete, or_, select, update
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import audit, debugsession, logs from app import audit, debugsession, logs
from app.audit import AuditAction from app.audit import AuditAction
from app.auth import current_user from app.auth import current_user
@@ -223,7 +224,9 @@ def revoke_all(db: Session, user_id: int) -> int:
change, "log out everywhere", a revoked untrusted password — ends its tokens too: one minted change, "log out everywhere", a revoked untrusted password — ends its tokens too: one minted
from a stolen session must not outlive the owner's answer to that theft. Returns how many.""" from a stolen session must not outlive the owner's answer to that theft. Returns how many."""
gone = ( gone = (
db.execute(delete(ApiToken).where(ApiToken.user_id == user_id).returning(ApiToken.id)) db.execute(
delete(ApiToken).where(owned_by(ApiToken.user_id, user_id)).returning(ApiToken.id)
)
.scalars() .scalars()
.all() .all()
) )
+2 -1
View File
@@ -4,6 +4,7 @@ from sqlalchemy import delete, func, select
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from starlette.requests import HTTPConnection from starlette.requests import HTTPConnection
from app.userscope import owned_by
from app import debugsession from app import debugsession
from app import email as email_mod from app import email as email_mod
from app import logs from app import logs
@@ -230,7 +231,7 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None:
# request's access line, a queued row, a later background task would otherwise put rows back # request's access line, a queued row, a later background task would otherwise put rows back
# under the id after this DELETE (logs.forget_user). # under the id after this DELETE (logs.forget_user).
logs.forget_user(user_id) logs.forget_user(user_id)
db.execute(delete(LogEvent).where(LogEvent.user_id == user_id)) db.execute(delete(LogEvent).where(owned_by(LogEvent.user_id, user_id)))
# Every canonically-equal invite goes, not just the literal spelling: is_allowed matches on the # Every canonically-equal invite goes, not just the literal spelling: is_allowed matches on the
# canonical form, so leaving a sibling row behind would keep granting access to a mailbox whose # canonical form, so leaving a sibling row behind would keep granting access to a mailbox whose
# account was just erased — and prod really does carry approved pairs for one mailbox. # account was just erased — and prod really does carry approved pairs for one mailbox.
+5 -2
View File
@@ -26,6 +26,7 @@ from uuid import uuid4
from sqlalchemy import and_, delete, func, not_, select, update from sqlalchemy import and_, delete, func, not_, select, update
from sqlalchemy.orm import Session, defer from sqlalchemy.orm import Session, defer
from app.userscope import owned_by
from app import diagnostics from app import diagnostics
from app.models import DebugScreenshot, DebugSession, LogEvent, User from app.models import DebugScreenshot, DebugSession, LogEvent, User
@@ -76,7 +77,9 @@ def open_session(db: Session, user: User) -> DebugSession | None:
def _unclosed(db: Session, user: User) -> DebugSession | None: def _unclosed(db: Session, user: User) -> DebugSession | None:
return db.scalar( return db.scalar(
select(DebugSession).where(DebugSession.user_id == user.id, DebugSession.ended_at.is_(None)) select(DebugSession).where(
owned_by(DebugSession.user_id, user), DebugSession.ended_at.is_(None)
)
) )
@@ -354,7 +357,7 @@ def stored_screenshot_bytes(db: Session, user_id: int) -> int:
db.scalar( db.scalar(
select(func.coalesce(func.sum(DebugScreenshot.size), 0)) select(func.coalesce(func.sum(DebugScreenshot.size), 0))
.join(DebugSession, DebugSession.id == DebugScreenshot.session_id) .join(DebugSession, DebugSession.id == DebugScreenshot.session_id)
.where(DebugSession.user_id == user_id) .where(owned_by(DebugSession.user_id, user_id))
) )
or 0 or 0
) )
+2 -1
View File
@@ -9,6 +9,7 @@ table can't grow without bound for a heavy user.
from sqlalchemy import delete, select from sqlalchemy import delete, select
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app.models import Notification from app.models import Notification
# Keep at most this many READ notifications per user; older read ones are pruned. Unread # Keep at most this many READ notifications per user; older read ones are pruned. Unread
@@ -52,7 +53,7 @@ def trim_read(db: Session, user_id: int, cap: int = READ_SOFT_CAP) -> int:
ids = ( ids = (
db.execute( db.execute(
select(Notification.id) select(Notification.id)
.where(Notification.user_id == user_id, Notification.read.is_(True)) .where(owned_by(Notification.user_id, user_id), Notification.read.is_(True))
.order_by(Notification.created_at.desc()) .order_by(Notification.created_at.desc())
.offset(cap) .offset(cap)
) )
+2 -1
View File
@@ -16,6 +16,7 @@ from sqlalchemy.dialects.postgresql import insert as pg_insert
from sqlalchemy.engine import CursorResult from sqlalchemy.engine import CursorResult
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import logs, sysconfig from app import logs, sysconfig
from app.db import SessionLocal from app.db import SessionLocal
from app.models import ApiQuotaUsage, QuotaEvent from app.models import ApiQuotaUsage, QuotaEvent
@@ -173,7 +174,7 @@ def actions_today(db: Session, user_id: int, action: str) -> int:
select(func.count()) select(func.count())
.select_from(QuotaEvent) .select_from(QuotaEvent)
.where( .where(
QuotaEvent.user_id == user_id, owned_by(QuotaEvent.user_id, user_id),
QuotaEvent.action == action, QuotaEvent.action == action,
QuotaEvent.created_at >= pacific_day_start_utc(), QuotaEvent.created_at >= pacific_day_start_utc(),
) )
+2 -1
View File
@@ -16,6 +16,7 @@ from sqlalchemy import select, update
from sqlalchemy.exc import IntegrityError from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import apitokens from app import apitokens
from app import email as email_mod from app import email as email_mod
from app import logs from app import logs
@@ -1124,7 +1125,7 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db))
# Burn any other outstanding reset tokens for this user. # Burn any other outstanding reset tokens for this user.
for row in db.execute( for row in db.execute(
select(AuthToken).where( select(AuthToken).where(
AuthToken.user_id == user.id, owned_by(AuthToken.user_id, user),
AuthToken.kind.in_(_PASSWORD_TOKEN_KINDS), AuthToken.kind.in_(_PASSWORD_TOKEN_KINDS),
AuthToken.used_at.is_(None), AuthToken.used_at.is_(None),
) )
+2 -1
View File
@@ -18,6 +18,7 @@ from sqlalchemy import select
from sqlalchemy.exc import IntegrityError from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import audit, debugsession, logs, logsink from app import audit, debugsession, logs, logsink
from app.audit import AuditAction from app.audit import AuditAction
from app.auth import current_user from app.auth import current_user
@@ -95,7 +96,7 @@ def stop_session(
def list_sessions(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict: def list_sessions(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict:
sessions = db.scalars( sessions = db.scalars(
select(DebugSession) select(DebugSession)
.where(DebugSession.user_id == user.id) .where(owned_by(DebugSession.user_id, user))
.order_by(DebugSession.started_at.desc()) .order_by(DebugSession.started_at.desc())
.limit(LIST_MAX) .limit(LIST_MAX)
).all() ).all()
+5 -4
View File
@@ -5,6 +5,7 @@ from fastapi import APIRouter, Depends, Query
from sqlalchemy import func, select, update from sqlalchemy import func, select, update
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app.auth import current_user from app.auth import current_user
from app.db import get_db from app.db import get_db
from app.models import Notification, User from app.models import Notification, User
@@ -36,7 +37,7 @@ def list_notifications(
user: User = Depends(current_user), user: User = Depends(current_user),
db: Session = Depends(get_db), db: Session = Depends(get_db),
) -> dict: ) -> dict:
where = [Notification.user_id == user.id] where = [owned_by(Notification.user_id, user)]
if not include_dismissed: if not include_dismissed:
where.append(Notification.dismissed.is_(False)) where.append(Notification.dismissed.is_(False))
total = db.scalar(select(func.count()).select_from(Notification).where(*where)) or 0 total = db.scalar(select(func.count()).select_from(Notification).where(*where)) or 0
@@ -61,7 +62,7 @@ def unread_count(user: User = Depends(current_user), db: Session = Depends(get_d
select(func.count()) select(func.count())
.select_from(Notification) .select_from(Notification)
.where( .where(
Notification.user_id == user.id, owned_by(Notification.user_id, user),
Notification.read.is_(False), Notification.read.is_(False),
Notification.dismissed.is_(False), Notification.dismissed.is_(False),
) )
@@ -86,7 +87,7 @@ def mark_read(
def mark_all_read(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict: def mark_all_read(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict:
db.execute( db.execute(
update(Notification) update(Notification)
.where(Notification.user_id == user.id, Notification.read.is_(False)) .where(owned_by(Notification.user_id, user), Notification.read.is_(False))
.values(read=True) .values(read=True)
) )
db.commit() db.commit()
@@ -113,7 +114,7 @@ def clear_all(user: User = Depends(current_user), db: Session = Depends(get_db))
"""Dismiss every (still-visible) notification for this user.""" """Dismiss every (still-visible) notification for this user."""
db.execute( db.execute(
update(Notification) update(Notification)
.where(Notification.user_id == user.id, Notification.dismissed.is_(False)) .where(owned_by(Notification.user_id, user), Notification.dismissed.is_(False))
.values(dismissed=True, read=True) .values(dismissed=True, read=True)
) )
db.commit() db.commit()
+2 -1
View File
@@ -10,6 +10,7 @@ from fastapi import APIRouter, Depends, Query
from sqlalchemy import func, select from sqlalchemy import func, select
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import quota from app import quota
from app.auth import admin_user, current_user from app.auth import admin_user, current_user
from app.db import get_db from app.db import get_db
@@ -27,7 +28,7 @@ def _sum(db: Session, *conds) -> int:
@router.get("/my-usage") @router.get("/my-usage")
def my_usage(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict: def my_usage(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict:
now = datetime.now(timezone.utc) now = datetime.now(timezone.utc)
mine = QuotaEvent.user_id == user.id mine = owned_by(QuotaEvent.user_id, user)
by_action = { by_action = {
action: int(units) action: int(units)
for action, units in db.execute( for action, units in db.execute(
+3 -2
View File
@@ -16,6 +16,7 @@ from pydantic import BaseModel, Field
from sqlalchemy import func, select from sqlalchemy import func, select
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from app.userscope import owned_by
from app import apitokens, audit from app import apitokens, audit
from app.apitokens import GRANT_COLUMNS, Right from app.apitokens import GRANT_COLUMNS, Right
from app.audit import AuditAction from app.audit import AuditAction
@@ -86,7 +87,7 @@ def _serialize(db: Session, token: ApiToken) -> dict:
@router.get("") @router.get("")
def list_tokens(user: User = Depends(current_user), db: Session = Depends(get_db)) -> list[dict]: def list_tokens(user: User = Depends(current_user), db: Session = Depends(get_db)) -> list[dict]:
tokens = ( tokens = (
db.execute(select(ApiToken).where(ApiToken.user_id == user.id).order_by(ApiToken.id)) db.execute(select(ApiToken).where(owned_by(ApiToken.user_id, user)).order_by(ApiToken.id))
.scalars() .scalars()
.all() .all()
) )
@@ -134,7 +135,7 @@ def create_token(
if (epoch or 0) != (user.session_epoch or 0): if (epoch or 0) != (user.session_epoch or 0):
raise refused(401, "not_authenticated") raise refused(401, "not_authenticated")
count = db.execute( count = db.execute(
select(func.count(ApiToken.id)).where(ApiToken.user_id == user.id) select(func.count(ApiToken.id)).where(owned_by(ApiToken.user_id, user))
).scalar_one() ).scalar_one()
if count >= MAX_TOKENS: if count >= MAX_TOKENS:
raise refused(409, "api_token_limit", limit=MAX_TOKENS) raise refused(409, "api_token_limit", limit=MAX_TOKENS)
+135
View File
@@ -0,0 +1,135 @@
"""The core's owner filters, each held by a second account whose rows must not count, show or change.
Every one of these reads or writes a user-owned table through `userscope.owned_by`; the fence holds
the form (no inline `Model.user_id == …`), these hold that the filter is there at all. Each test gives
a second account the same kind of rows, so a filter that went missing answers with theirs. DB lane.
"""
from datetime import datetime, timedelta, timezone
from sqlalchemy import func, select
from app import debugsession, notifications, quota
from app.models import DebugScreenshot, DebugSession, Notification, QuotaEvent
from app.routes import tokens as tokens_routes
from app.routes.auth import RESET_TTL, _issue_token
def _notify(db, user, n=1, *, read=False):
for i in range(n):
db.add(Notification(user_id=user.id, type="t", title=f"n{i}", read=read))
db.commit()
def _visible(db, user) -> int:
return db.scalar(
select(func.count())
.select_from(Notification)
.where(Notification.user_id == user.id, Notification.dismissed.is_(False))
)
def test_the_notification_list_shows_only_mine(db, make_user, as_user):
me, other = make_user(), make_user()
_notify(db, me)
_notify(db, other, 2)
body = as_user(me).get("/api/me/notifications").json()
assert (body["total"], len(body["items"])) == (1, 1)
def test_the_unread_count_counts_only_mine(db, make_user, as_user):
me, other = make_user(), make_user()
_notify(db, me)
_notify(db, other, 2)
assert as_user(me).get("/api/me/notifications/unread_count").json() == {"count": 1}
def test_reading_all_reads_only_mine(db, make_user, as_user):
me, other = make_user(), make_user()
_notify(db, me)
_notify(db, other, 2)
assert as_user(me).post("/api/me/notifications/read_all").status_code == 200
assert as_user(other).get("/api/me/notifications/unread_count").json() == {"count": 2}
def test_clearing_all_dismisses_only_mine(db, make_user, as_user):
me, other = make_user(), make_user()
_notify(db, me)
_notify(db, other, 2)
assert as_user(me).post("/api/me/notifications/clear").status_code == 200
db.expire_all()
assert (_visible(db, me), _visible(db, other)) == (0, 2)
def test_trimming_read_notifications_trims_only_mine(db, make_user):
me, other = make_user(), make_user()
_notify(db, me, 3, read=True)
_notify(db, other, 3, read=True)
assert notifications.trim_read(db, me.id, cap=1) == 2
db.commit()
assert _visible(db, other) == 3
def test_todays_actions_count_only_mine(db, make_user):
me, other = make_user(), make_user()
quota.log_action(me.id, quota.QuotaAction.VIDEOS_SEARCH)
quota.log_action(other.id, quota.QuotaAction.VIDEOS_SEARCH)
quota.log_action(other.id, quota.QuotaAction.VIDEOS_SEARCH)
assert quota.actions_today(db, me.id, quota.QuotaAction.VIDEOS_SEARCH) == 1
def test_my_usage_sums_only_mine(db, make_user, as_user):
me, other = make_user(), make_user()
db.add_all(
[
QuotaEvent(user_id=me.id, action="videos.list", units=5),
QuotaEvent(user_id=other.id, action="videos.list", units=7),
]
)
db.commit()
body = as_user(me).get("/api/quota/my-usage").json()
assert (body["today"], body["all_time"], body["by_action"]) == (5, 5, {"videos.list": 5})
def test_the_token_ceiling_counts_only_mine(db, make_user, as_user, monkeypatch):
monkeypatch.setattr(tokens_routes, "MAX_TOKENS", 1)
me, other = make_user(), make_user()
first = as_user(other).post("/api/tokens", json={"name": "theirs", "preset": "hub"})
assert first.status_code == 200, first.text
r = as_user(me).post("/api/tokens", json={"name": "mine", "preset": "hub"})
assert r.status_code == 200, r.text
def test_a_reset_burns_only_my_other_links(client, db, make_user):
me, other = make_user(), make_user()
mine = _issue_token(db, me, "reset", RESET_TTL)
theirs = _issue_token(db, other, "reset", RESET_TTL)
db.commit()
r = client.post(
"/auth/password-reset/confirm", json={"token": mine, "password": "a brand new passphrase"}
)
assert r.json() == {"ok": True}
assert client.post("/auth/password-reset/check", json={"token": theirs}).json() == {"ok": True}
def test_screenshot_storage_counts_only_mine(db, make_user):
me, other = make_user(), make_user()
now = datetime.now(timezone.utc)
for i, (user, size) in enumerate([(me, 10), (other, 1000)]):
db.add(
DebugSession(
id=f"s{i}",
user_id=user.id,
level="DEBUG",
started_at=now,
until=now + timedelta(hours=1),
)
)
db.flush()
db.add(
DebugScreenshot(
session_id=f"s{i}", created_at=now, mime="image/png", size=size, data=b"x" * size
)
)
db.commit()
assert debugsession.stored_screenshot_bytes(db, me.id) == 10