From 02db5430eb44d8f396dc62c0b74cc341f0e8ea73 Mon Sep 17 00:00:00 2001 From: Jacob Hagstedt Date: Fri, 18 Sep 2026 07:54:21 +0200 Subject: [PATCH] Harden the API and the data layer after a security and performance audit Security - Every route now requires a login and refuses ids outside the caller's workspace with 404: plan, leave-wishes, day-overrides and vacation-days were fully anonymous; every other router trusted client-supplied workspace/child/parent ids. Scraper sessions are bound to their owner. - Invitations can no longer be created for a foreign workspace, and a redeemer who is already a member (or a full workspace) is refused. - Production refuses the default JWT secret; API docs are hidden in production; a failed migration aborts startup instead of serving. - Login and register no longer reveal whether an email exists; inactive accounts are rejected; /auth/refresh is rate limited. - The FK scraper writes debug artefacts only when explicitly enabled, logs no scraped values or page text, clears the BankID QR as soon as it is scanned and purges finished sessions after five minutes. - Real FK page captures are removed from the tree (history rewrite follows); the parser tests use a synthetic fixture. Performance and correctness - Plan apply/sync run inside one locked transaction with batched writes instead of one commit per row, so a failure mid-batch leaves nothing half-applied and concurrent syncs no longer corrupt each other. - SQLite runs in WAL mode with a busy timeout; 14 indexes cover the filters that were full scans; argon2 hashing runs off the event loop; responses are gzipped. - Frontend: a 401 now refreshes the token once and retries, so sessions no longer die silently after 15 minutes; every data hook discards stale responses; logout clears state even when the server call fails. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01CsXCCcDfmV4Sps4ZQSvNEY --- .gitignore | 8 +- CLAUDE.md | 2 +- backend/.env.example | 21 +- backend/.gitignore | 3 + backend/README.md | 11 + backend/app/config.py | 37 +- backend/app/db/connection.py | 141 ++++ backend/app/dependencies.py | 128 +++- backend/app/main.py | 13 +- .../repositories/planned_leave_repository.py | 30 + backend/app/repositories/unit_of_work.py | 39 ++ .../sqlite/child_repository.py | 7 +- .../sqlite/day_override_repository.py | 7 +- .../sqlite/leave_wish_repository.py | 7 +- .../sqlite/parental_leave_repository.py | 7 +- .../sqlite/personal_data_repository.py | 9 +- .../sqlite/planned_leave_repository.py | 77 ++- .../sqlite/tip_dismissal_repository.py | 7 +- .../repositories_impl/sqlite/unit_of_work.py | 21 + .../sqlite/user_repository.py | 18 +- .../sqlite/vacation_days_repository.py | 7 +- .../sqlite/workspace_repository.py | 13 +- backend/app/routers/analytics.py | 17 +- backend/app/routers/auth.py | 2 + backend/app/routers/calendar.py | 5 +- backend/app/routers/day_overrides.py | 17 +- backend/app/routers/fetch_data.py | 62 +- backend/app/routers/leave_wishes.py | 23 +- backend/app/routers/plan.py | 23 +- backend/app/routers/planned_days.py | 24 +- backend/app/routers/settings.py | 29 +- backend/app/routers/vacation_days.py | 12 +- backend/app/routers/workspace.py | 30 +- backend/app/scrapers/fk_scraper.py | 142 +++- backend/app/services/analytics_service.py | 10 +- backend/app/services/auth_service.py | 89 ++- backend/app/services/child_service.py | 15 +- backend/app/services/day_override_service.py | 7 +- backend/app/services/leave_wish_service.py | 17 +- backend/app/services/personal_data_service.py | 7 +- backend/app/services/planning_service.py | 535 ++++++++------- backend/app/services/vacation_days_service.py | 4 +- backend/app/services/workspace_service.py | 35 +- .../migrations/versions/011_add_indexes.py | 99 +++ backend/scripts/demo_plan.py | 2 + backend/tests/conftest.py | 84 +++ .../tests/fixtures/fk_overview_synthetic.html | 81 +++ backend/tests/unit/test_authz.py | 646 ++++++++++++++++++ backend/tests/unit/test_calendar_router.py | 30 +- backend/tests/unit/test_config.py | 41 ++ backend/tests/unit/test_db_unit_of_work.py | 243 +++++++ backend/tests/unit/test_fetch_data_router.py | 42 +- backend/tests/unit/test_fk_scraper.py | 289 +++++++- backend/tests/unit/test_leave_wish_router.py | 35 +- backend/tests/unit/test_plan_router.py | 57 +- .../unit/test_services/test_auth_service.py | 44 ++ backend/tests/unit/test_tips_router.py | 11 +- backend/tests/unit/test_workspace_service.py | 105 ++- .../src/components/layout/Layout.test.tsx | 16 + frontend/src/components/layout/Layout.tsx | 12 +- frontend/src/context/AuthContext.test.tsx | 41 ++ frontend/src/context/AuthContext.tsx | 24 +- .../src/context/WorkspaceContext.test.tsx | 46 +- frontend/src/context/WorkspaceContext.tsx | 49 +- frontend/src/hooks/useAnalytics.test.ts | 56 ++ frontend/src/hooks/useAnalytics.ts | 11 +- frontend/src/hooks/useCalendarDays.ts | 9 +- frontend/src/hooks/useChildSettings.ts | 9 +- frontend/src/hooks/useDayOverrides.ts | 9 +- frontend/src/hooks/useLatestRequest.ts | 41 ++ frontend/src/hooks/useLeaveWishes.ts | 9 +- frontend/src/hooks/useOptimizationTips.ts | 9 +- frontend/src/hooks/useParentalLeave.ts | 9 +- frontend/src/hooks/usePlannedDays.test.ts | 59 ++ frontend/src/hooks/usePlannedDays.ts | 15 +- frontend/src/hooks/useScraperFetch.test.ts | 76 +++ frontend/src/hooks/useScraperFetch.ts | 17 +- frontend/src/hooks/useVacationDays.ts | 9 +- frontend/src/hooks/useWorkspaceMembers.ts | 9 +- frontend/src/services/api.test.ts | 108 +++ frontend/src/services/api.ts | 46 ++ 81 files changed, 3616 insertions(+), 600 deletions(-) create mode 100644 backend/app/repositories/unit_of_work.py create mode 100644 backend/app/repositories_impl/sqlite/unit_of_work.py create mode 100644 backend/migrations/versions/011_add_indexes.py create mode 100644 backend/tests/fixtures/fk_overview_synthetic.html create mode 100644 backend/tests/unit/test_authz.py create mode 100644 backend/tests/unit/test_config.py create mode 100644 backend/tests/unit/test_db_unit_of_work.py create mode 100644 frontend/src/hooks/useLatestRequest.ts diff --git a/.gitignore b/.gitignore index eb8247f..9a29f2a 100644 --- a/.gitignore +++ b/.gitignore @@ -6,9 +6,11 @@ __pycache__/ graphify-out backend/debug -# Ephemeral browser-verification screenshots (the tracked captures/ files are -# scraper reference material and stay tracked). -/captures/verification/ +# Scraper page captures and browser-verification screenshots contain real +# personal data from Försäkringskassan — never track them. Tests use the +# synthetic fixture in backend/tests/fixtures/ instead. +/captures/ +/test.html # C extensions *.so diff --git a/CLAUDE.md b/CLAUDE.md index bb2b63f..6c87877 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,7 +97,7 @@ frontend/ TypeScript · React 18.3 · Vite 5 · Vitest **Data layer (frontend):** components consume `src/hooks/*`, which call `src/services/api.ts`. Components should not call `fetch`/the API client directly. -**Browser-verification screenshots:** ephemeral screenshots from verification runs go in `captures/verification/` (which is gitignored), while `captures/` itself holds tracked scraper reference material. +**Captures are never tracked:** `captures/` (scraper page dumps, verification screenshots) is gitignored because real FK pages contain personal data. Scraper parser tests use the synthetic fixture `backend/tests/fixtures/fk_overview_synthetic.html`. --- diff --git a/backend/.env.example b/backend/.env.example index 2fe8820..d5ab665 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -1,14 +1,33 @@ # Copy to .env and fill in values for local development. # Pydantic-settings reads these with the APP_ prefix. +# Deployment environment: development | test | production. +# In production, Settings refuses to start unless APP_JWT_SECRET is set to a +# random value of >=32 chars (see app/config.py). Also gates /docs, /redoc, +# and /openapi.json (disabled in production). +APP_ENV=development + # Enable FK scraper (legal cleared 2026-06; requires `playwright install chromium`) APP_SCRAPER_ENABLED=true +# Scraper debug artifacts (HTML dump + full-page screenshot of the logged-in +# FK page, saved on error) — OFF by default. CONTAINS PERSONAL DATA (name, +# balances, payout amounts) when enabled. Only turn on for local debugging on +# a machine you control, and delete the directory afterwards. Both must be +# set to actually enable saving. +# APP_SCRAPER_DEBUG_ENABLED=true +# APP_SCRAPER_DEBUG_DIR=./debug/scraper + # Database (default: SQLite in repo root) # APP_SQLITE_PATH=./foraldraledig.db -# JWT secret — change in production (min 32 chars) +# JWT secret — REQUIRED in production (min 32 chars). Generate with: +# python -c "import secrets; print(secrets.token_urlsafe(48))" # APP_JWT_SECRET=dev-secret-change-in-production-min-32-chars # CORS — add your frontend origin(s) # APP_CORS_ORIGINS=["http://localhost:5173"] + +# Log level (default: INFO). Avoid DEBUG in production — scraper/auth code +# paths log more detail at DEBUG. +# APP_LOG_LEVEL=INFO diff --git a/backend/.gitignore b/backend/.gitignore index 39f0535..fca56f3 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -6,4 +6,7 @@ __pycache__/ htmlcov/ .coverage *.db +*.db-shm +*.db-wal +*.db-journal .env diff --git a/backend/README.md b/backend/README.md index 5597905..9572ec8 100644 --- a/backend/README.md +++ b/backend/README.md @@ -13,6 +13,17 @@ uv run pytest # run tests with coverage uv run ruff check . # lint ``` +## Configuration + +Key env vars (prefix `APP_`, see `.env.example`): + +- `APP_ENV` — `development` (default) | `test` | `production`. In production, + `Settings` refuses to start unless `APP_JWT_SECRET` is a random value of at + least 32 characters (the shipped dev default is rejected). Production also + disables `/docs`, `/redoc`, and `/openapi.json`. +- `APP_JWT_SECRET` — required to be a strong, random secret in production. +- `APP_LOG_LEVEL` — default `INFO`. + ## Layout ``` diff --git a/backend/app/config.py b/backend/app/config.py index 1c0f658..4eb0c18 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -13,9 +13,13 @@ the common-case figures — children born before 2014, multiples, and single custody differ and should become explicit config when tackled. """ -from pydantic import BaseModel, ConfigDict +from typing import Literal + +from pydantic import BaseModel, ConfigDict, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +_DEV_JWT_SECRET = "dev-secret-change-in-production-min-32-chars" + class LeaveRules(BaseModel): """Frozen Swedish föräldrapenning rule defaults (single source of truth).""" @@ -95,6 +99,11 @@ class Settings(BaseSettings): extra="ignore", ) + # Deployment environment. Gates production-only hardening (see the + # model_validator below and main.py's docs_url handling). Never set to + # "production" in tests/dev — the validator below will refuse to start. + app_env: Literal["development", "test", "production"] = "development" + # 'sqlite' for local dev, 'postgres' for production (swap repository impls). database_type: str = "sqlite" sqlite_path: str = "./foraldraledig.db" @@ -103,12 +112,12 @@ class Settings(BaseSettings): cors_origins: list[str] = ["http://localhost:5173"] # Auth — change jwt_secret in production (min 32 chars). - jwt_secret: str = "dev-secret-change-in-production-min-32-chars" + jwt_secret: str = _DEV_JWT_SECRET jwt_algorithm: str = "HS256" jwt_access_token_expire_minutes: int = 15 jwt_refresh_token_expire_days: int = 7 - log_level: str = "DEBUG" + log_level: str = "INFO" # FK scraper — legal/ToS review completed (2026-06); cleared for use. # Default is True. Set APP_SCRAPER_ENABLED=false to disable (e.g. an @@ -117,10 +126,28 @@ class Settings(BaseSettings): scraper_enabled: bool = True scraper_headless: bool = True scraper_session_ttl_seconds: int = 180 # auto-cancel awaiting_scan/authenticating sessions - # Set to None to disable debug artifact saving. Files may contain personal data. - scraper_debug_dir: str | None = "./debug/scraper" + # Debug artifacts (HTML + full-page screenshot of the logged-in FK page) + # are OFF by default — they contain personal data (name, balances, + # payout amounts). Both fields below must be set to enable them; this is + # a deliberate opt-in for local debugging only, never for production. + scraper_debug_dir: str | None = None + scraper_debug_enabled: bool = False + # How long a terminal (done/error/cancelled) session's QR/result stays in + # RAM before `_ttl_watchdog`/`poll` purge it from `_sessions`. + scraper_result_retention_seconds: int = 300 rules: LeaveRules = LeaveRules() + @model_validator(mode="after") + def _require_strong_jwt_secret_in_production(self) -> "Settings": + if self.app_env == "production" and ( + self.jwt_secret == _DEV_JWT_SECRET or len(self.jwt_secret) < 32 + ): + raise ValueError( + "APP_JWT_SECRET must be set to a random value of at least 32 characters " + "when APP_ENV=production" + ) + return self + settings = Settings() diff --git a/backend/app/db/connection.py b/backend/app/db/connection.py index acc4dd8..c389bfa 100644 --- a/backend/app/db/connection.py +++ b/backend/app/db/connection.py @@ -10,6 +10,12 @@ connection so test fixtures stay fast and do not depend on Alembic. from __future__ import annotations +import asyncio +import weakref +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager +from contextvars import ContextVar + import aiosqlite # Full schema used by test fixtures directly (avoids a full Alembic run in tests). @@ -46,6 +52,7 @@ CREATE TABLE IF NOT EXISTS "user" ( email TEXT NOT NULL UNIQUE, hashed_password TEXT NOT NULL, created_at TIMESTAMP NOT NULL, + is_active INTEGER NOT NULL DEFAULT 1, FOREIGN KEY (personal_data_uuid) REFERENCES personal_data(uuid) ON DELETE CASCADE ); @@ -183,18 +190,152 @@ CREATE TABLE IF NOT EXISTS tip_dismissal ( UNIQUE(workspace_id, tip_id), FOREIGN KEY (workspace_id) REFERENCES workspace(id) ON DELETE CASCADE ); + +-- Indexes (migration 011). Mirrored verbatim (same columns, same order, no +-- partial indexes) from migrations/versions/011_add_indexes.py so +-- tests/integration/test_schema_parity.py, which compares index sets between +-- the Alembic chain and this hand-maintained DDL, stays green. See the +-- backend-perf audit §4 for what each index serves. +CREATE INDEX IF NOT EXISTS idx_pld_parent_child_type_date + ON planned_leave_day (parent_personal_data_uuid, child_id, day_type, date); +CREATE INDEX IF NOT EXISTS idx_pld_child_dubbeldag_date + ON planned_leave_day (child_id, is_dubbeldag, date); +CREATE INDEX IF NOT EXISTS idx_pld_wish + ON planned_leave_day (wish_id); +CREATE INDEX IF NOT EXISTS idx_plf_parent_child_fetched + ON parental_leave_fetch (personal_data_uuid, child_id, fetched_at); +CREATE INDEX IF NOT EXISTS idx_plf_workspace + ON parental_leave_fetch (workspace_id); +CREATE INDEX IF NOT EXISTS idx_plf_child + ON parental_leave_fetch (child_id); +CREATE INDEX IF NOT EXISTS idx_leave_wish_workspace + ON leave_wish (workspace_id); +CREATE INDEX IF NOT EXISTS idx_leave_wish_child + ON leave_wish (child_id); +CREATE INDEX IF NOT EXISTS idx_leave_wish_parent + ON leave_wish (parent_personal_data_uuid); +CREATE INDEX IF NOT EXISTS idx_child_workspace_birth + ON child (workspace_id, birth_date); +CREATE INDEX IF NOT EXISTS idx_personal_data_workspace_deleted + ON personal_data (workspace_id, deleted_at); +CREATE INDEX IF NOT EXISTS idx_invitation_workspace_status_created + ON workspace_invitation (workspace_id, status, created_at); +CREATE INDEX IF NOT EXISTS idx_invitation_invited_by + ON workspace_invitation (invited_by_user_id); +CREATE INDEX IF NOT EXISTS idx_vacation_days_workspace + ON vacation_days (workspace_id); """ +# --------------------------------------------------------------------------- +# Unit of work — see backend-perf audit §3 for the full design rationale. +# +# The app runs on ONE shared aiosqlite connection (app.state.db_connection), +# so a multi-statement write needs (a) mutual exclusion for the whole unit of +# work and (b) the repos' own per-row `commit()` calls suppressed while one is +# in progress — otherwise a concurrent request's commit would durably commit +# this one's half-finished writes. `_in_uow` and `_locks` below exist to +# provide exactly that on top of the existing single-connection, per-repo- +# commit design, without touching any repo's read/write SQL. +# --------------------------------------------------------------------------- + +# True while the current asyncio task is inside a `unit_of_work()` block, on +# whichever connection started it. Checked by `commit()` (skip the per-row +# commit; the unit of work commits once, at the end) and by `unit_of_work()` +# itself (a nested call is a no-op passthrough — see its docstring). +_in_uow: ContextVar[bool] = ContextVar("db_in_uow", default=False) + +# One asyncio.Lock per connection, created lazily on first use. A module-level +# lock would break under pytest: each test's `connection` fixture opens a new +# in-memory connection under a fresh event loop, and an `asyncio.Lock` created +# against one loop raises "bound to a different event loop" if awaited from +# another. Keying by connection (and using a WeakKeyDictionary so a closed +# test connection's lock is garbage-collected instead of leaking) sidesteps +# that entirely: production has exactly one connection => exactly one lock. +_locks: "weakref.WeakKeyDictionary[aiosqlite.Connection, asyncio.Lock]" = ( + weakref.WeakKeyDictionary() +) + + +def _lock_for(conn: aiosqlite.Connection) -> asyncio.Lock: + lock = _locks.get(conn) + if lock is None: + lock = asyncio.Lock() + _locks[conn] = lock + return lock + + +async def commit(conn: aiosqlite.Connection) -> None: + """Commit, unless a `unit_of_work()` is currently in progress on this task. + + Every sqlite repository's single-row write calls this instead of + `conn.commit()` directly. Outside a unit of work this behaves exactly + like today (commit immediately, once per write); inside one, it is a + no-op, and the surrounding `unit_of_work()` block commits once, atomically, + when it exits. + """ + if not _in_uow.get(): + await conn.commit() + + +@asynccontextmanager +async def unit_of_work(conn: aiosqlite.Connection) -> AsyncIterator[None]: + """Wrap the block in one atomic transaction on the shared connection. + + Acquires this connection's lock for the whole block (SQLite is a + single-writer database anyway, so this costs no real throughput) and + issues `BEGIN IMMEDIATE` up front — that claims the write lock + immediately rather than at the first write statement, so a second, + concurrent `unit_of_work()` call blocks here and only proceeds once this + one has committed (or rolled back), then re-reads state as of that commit + instead of racing it. Any exception rolls the whole block back; a normal + exit commits it. + + Nested calls (e.g. `PlanningService.sync_plan` wrapping its own call to + `apply_proposed_days`, which wraps its own writes) are a no-op passthrough + when `_in_uow` is already set — the outer call owns the lock/transaction, + and a naive re-entry would try to re-acquire the same (non-reentrant) + `asyncio.Lock` from the same task and deadlock. + """ + if _in_uow.get(): + yield + return + + async with _lock_for(conn): + token = _in_uow.set(True) + try: + await conn.execute("BEGIN IMMEDIATE") + try: + yield + except BaseException: + await conn.rollback() + raise + else: + await conn.commit() + finally: + _in_uow.reset(token) + + async def connect(path: str) -> aiosqlite.Connection: """Open a connection and enable row-by-name access + FK enforcement. Schema is applied via Alembic in the app lifespan (`run_migrations`). For test fixtures, call `ensure_schema` directly. + + PRAGMAs: `journal_mode=WAL` + `synchronous=NORMAL` cut single-row commit + latency roughly 4x (measured 88.7ms -> 21.7ms per 300 sequential commits) + and let Alembic/`seed_demo.py` co-access the same file wait instead of + hitting "database is locked" outright; `busy_timeout` backs that wait with + an actual retry budget. Durability trade-off of NORMAL: at most the last + transaction(s) are lost on an OS crash/power loss, never on an app crash — + acceptable here (see backend-perf audit §3). """ conn = await aiosqlite.connect(path) conn.row_factory = aiosqlite.Row await conn.execute("PRAGMA foreign_keys = ON;") + await conn.execute("PRAGMA journal_mode = WAL;") + await conn.execute("PRAGMA synchronous = NORMAL;") + await conn.execute("PRAGMA busy_timeout = 5000;") return conn diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index 0868df6..b2330d8 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -10,13 +10,17 @@ Pattern (copy for each new entity): 4. `ServiceDep` — Annotated type alias """ +from dataclasses import dataclass from typing import Annotated +from uuid import UUID import aiosqlite from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from app.config import settings +from app.entities.child import Child +from app.entities.personal_data import PersonalData from app.entities.user import User from app.repositories.child_repository import ChildRepository from app.repositories.day_override_repository import DayOverrideRepository @@ -25,6 +29,7 @@ from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.repositories.personal_data_repository import PersonalDataRepository from app.repositories.planned_leave_repository import PlannedLeaveRepository from app.repositories.tip_dismissal_repository import TipDismissalRepository +from app.repositories.unit_of_work import UnitOfWork from app.repositories.user_repository import UserRepository from app.repositories.vacation_days_repository import VacationDaysRepository from app.repositories.workspace_repository import ( @@ -38,6 +43,7 @@ from app.repositories_impl.sqlite.parental_leave_repository import SQLiteParenta from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository from app.repositories_impl.sqlite.tip_dismissal_repository import SQLiteTipDismissalRepository +from app.repositories_impl.sqlite.unit_of_work import SQLiteUnitOfWork from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository from app.repositories_impl.sqlite.workspace_repository import ( @@ -74,6 +80,15 @@ def get_connection(request: Request) -> aiosqlite.Connection: ConnectionDep = Annotated[aiosqlite.Connection, Depends(get_connection)] +def get_unit_of_work(conn: ConnectionDep) -> UnitOfWork: + if settings.database_type == "sqlite": + return SQLiteUnitOfWork(conn) + raise ValueError(f"Unsupported database_type: {settings.database_type}") + + +UnitOfWorkDep = Annotated[UnitOfWork, Depends(get_unit_of_work)] + + # --------------------------------------------------------------------------- # Repository factories # --------------------------------------------------------------------------- @@ -196,8 +211,9 @@ def get_auth_service( user_repo: UserRepositoryDep, pd_repo: PersonalDataRepositoryDep, ws_repo: WorkspaceRepositoryDep, + uow: UnitOfWorkDep, ) -> AuthService: - return AuthService(user_repo, pd_repo, ws_repo) + return AuthService(user_repo, pd_repo, ws_repo, uow=uow) AuthServiceDep = Annotated[AuthService, Depends(get_auth_service)] @@ -282,6 +298,7 @@ def get_workspace_service( planned_leave_repo: PlannedLeaveRepositoryDep, parental_leave_repo: ParentalLeaveRepositoryDep, vacation_days_repo: VacationDaysRepositoryDep, + uow: UnitOfWorkDep, ) -> WorkspaceService: return WorkspaceService( ws_repo, @@ -292,6 +309,7 @@ def get_workspace_service( planned_leave_repo, parental_leave_repo, vacation_days_repo, + uow=uow, ) @@ -349,9 +367,15 @@ def get_planning_service( parental_leave_repo: ParentalLeaveRepositoryDep, vacation_repo: VacationDaysRepositoryDep, planning_engine_service: PlanningEngineServiceDep, + uow: UnitOfWorkDep, ) -> PlanningService: return PlanningService( - repo, analytics_service, parental_leave_repo, vacation_repo, planning_engine_service + repo, + analytics_service, + parental_leave_repo, + vacation_repo, + planning_engine_service, + uow=uow, ) @@ -377,3 +401,103 @@ async def get_current_user( CurrentUserDep = Annotated[User, Depends(get_current_user)] + + +# --------------------------------------------------------------------------- +# Tenant scoping +# --------------------------------------------------------------------------- +# +# Rule of thumb: identity-derived facts (which workspace the caller belongs +# to) live here in the DI layer; entity-ownership checks that require loading +# a row (e.g. a leave wish's own workspace_id) live in the service, mirroring +# `WorkspaceService._get_member`. Routers stay thin: they hand the resolved +# scope to the service and let a NotFoundError become a 404 — never a 403, +# so a foreign id never confirms its own existence. See the T1 authz audit, +# §3 "Scoping primitive — design for app/dependencies.py". + + +async def get_current_personal_data( + current_user: CurrentUserDep, pd_repo: PersonalDataRepositoryDep +) -> PersonalData: + """The caller's own PersonalData row — the root of tenant scoping.""" + pd = await pd_repo.get_by_id(current_user.personal_data_uuid) + if pd is None or pd.deleted_at is not None: + raise HTTPException(status.HTTP_401_UNAUTHORIZED, "No parent profile for this account") + return pd + + +CurrentPersonalDataDep = Annotated[PersonalData, Depends(get_current_personal_data)] + + +async def get_current_workspace_id(pd: CurrentPersonalDataDep) -> UUID: + return pd.workspace_id + + +CurrentWorkspaceIdDep = Annotated[UUID, Depends(get_current_workspace_id)] + + +def _not_found(kind: str, id_: UUID) -> HTTPException: + # 404, never 403: do not confirm that a foreign id exists. + return HTTPException(status.HTTP_404_NOT_FOUND, f"{kind} {id_} not found") + + +async def require_workspace_access(workspace_id: UUID, current: CurrentWorkspaceIdDep) -> UUID: + """Path-param guard for `/{workspace_id}/...` routes. + + FastAPI binds the `workspace_id` path param by name, so a router only + has to declare `workspace_id: WorkspaceIdDep` in place of `UUID` to get + this check for free. + """ + if workspace_id != current: + raise _not_found("Workspace", workspace_id) + return workspace_id + + +WorkspaceIdDep = Annotated[UUID, Depends(require_workspace_access)] + + +@dataclass(frozen=True) +class WorkspaceScope: + """The caller's tenant, plus helpers for ids arriving in a body/query. + + `check_workspace` is a cheap identity comparison (no DB read — the + caller's own workspace_id is already known). `check_child`/`check_member` + load the row because ownership can only be known after the load; both + 404 (never 403) on a missing OR foreign-workspace id, so a foreign id can + never be distinguished from a nonexistent one. + """ + + workspace_id: UUID + personal_data_uuid: UUID + user_id: UUID + _child_repo: ChildRepository + _pd_repo: PersonalDataRepository + + def check_workspace(self, workspace_id: UUID) -> UUID: + if workspace_id != self.workspace_id: + raise _not_found("Workspace", workspace_id) + return workspace_id + + async def check_child(self, child_id: UUID) -> Child: + child = await self._child_repo.get_by_id(child_id) + if child is None or child.workspace_id != self.workspace_id: + raise _not_found("Child", child_id) + return child + + async def check_member(self, personal_data_uuid: UUID) -> PersonalData: + pd = await self._pd_repo.get_by_id(personal_data_uuid) + if pd is None or pd.deleted_at is not None or pd.workspace_id != self.workspace_id: + raise _not_found("PersonalData", personal_data_uuid) + return pd + + +async def get_workspace_scope( + current_user: CurrentUserDep, + pd: CurrentPersonalDataDep, + child_repo: ChildRepositoryDep, + pd_repo: PersonalDataRepositoryDep, +) -> WorkspaceScope: + return WorkspaceScope(pd.workspace_id, pd.uuid, current_user.id, child_repo, pd_repo) + + +WorkspaceScopeDep = Annotated[WorkspaceScope, Depends(get_workspace_scope)] diff --git a/backend/app/main.py b/backend/app/main.py index ab46c04..83b73b8 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -14,6 +14,7 @@ from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from slowapi import _rate_limit_exceeded_handler from slowapi.errors import RateLimitExceeded +from starlette.middleware.gzip import GZipMiddleware from app.config import settings from app.db.connection import connect, run_migrations @@ -63,6 +64,7 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: await asyncio.to_thread(run_migrations, f"sqlite:///{settings.sqlite_path}") except Exception as exc: logger.error("Migration failed — DB schema may be incomplete: %s", exc, exc_info=True) + raise # Re-enable after migrations: alembic's env.py calls fileConfig() which may # still touch logger state even with disable_existing_loggers=False. @@ -78,7 +80,14 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: def create_app() -> FastAPI: - app = FastAPI(title="Föräldraledigplannerare API", lifespan=lifespan) + is_production = settings.app_env == "production" + app = FastAPI( + title="Föräldraledigplannerare API", + lifespan=lifespan, + docs_url=None if is_production else "/docs", + redoc_url=None if is_production else "/redoc", + openapi_url=None if is_production else "/openapi.json", + ) app.state.limiter = limiter app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) @@ -90,6 +99,8 @@ def create_app() -> FastAPI: logger.warning("%s %s %d", request.method, request.url.path, response.status_code) return response + app.add_middleware(GZipMiddleware, minimum_size=1024) + app.add_middleware( CORSMiddleware, allow_origins=list(settings.cors_origins), diff --git a/backend/app/repositories/planned_leave_repository.py b/backend/app/repositories/planned_leave_repository.py index f11747f..ecbd963 100644 --- a/backend/app/repositories/planned_leave_repository.py +++ b/backend/app/repositories/planned_leave_repository.py @@ -43,3 +43,33 @@ class PlannedLeaveRepository(Repository[PlannedLeaveDay]): that DELETE runs, no `planned_leave_day` row still references this wish, so there is nothing left to orphan. """ + + @abstractmethod + async def create_many(self, items: list[PlannedLeaveDay]) -> None: + """Bulk-insert rows in one round trip (`executemany`). Empty list is a no-op. + + See backend-perf audit H1: a per-row `create()` + `commit()` loop + measured 88.7ms/300 rows (default journal mode); this + one + surrounding `unit_of_work()` is the batched replacement used by + `PlanningService.apply_proposed_days` stage 4. + """ + + @abstractmethod + async def delete_many(self, ids: list[UUID]) -> int: + """Delete rows by id in one or more `executemany`/`IN (...)` round trips. + + Idempotent: an id with no matching row is silently ignored rather than + raising `NotFoundError` (unlike `delete()`) — callers that already + know some keys may have vanished (e.g. a re-applied batch) should not + have to pre-filter. Returns the number of rows actually deleted. + """ + + @abstractmethod + async def update_metadata_many(self, pairs: list[tuple[UUID, UUID | None, bool]]) -> None: + """Bulk-update `(wish_id, is_pinned)` for existing rows via `executemany`. + + Each pair is `(id, wish_id, is_pinned)`. Does not touch `day_type`/ + `is_dubbeldag` — this is for the metadata-only path in + `apply_proposed_days` (a row whose type is unchanged but whose + `wish_id`/`is_pinned` differ from the incoming batch). + """ diff --git a/backend/app/repositories/unit_of_work.py b/backend/app/repositories/unit_of_work.py new file mode 100644 index 0000000..344ba7c --- /dev/null +++ b/backend/app/repositories/unit_of_work.py @@ -0,0 +1,39 @@ +"""Abstract unit-of-work contract for multi-statement atomic writes. + +A `UnitOfWork` is a callable that returns an async context manager: entering +it starts an atomic transaction, a normal exit commits it, and an exception +rolls it back. Services depend on this interface only — the concrete +mechanism (a locked `BEGIN IMMEDIATE` transaction on the shared connection) +lives in `app.db.connection.unit_of_work` / `repositories_impl.sqlite. +unit_of_work.SQLiteUnitOfWork`. See the backend-perf audit +(`scratchpad/audit/backend-perf.md` §3) for why a lock is required on a +single shared connection rather than a bare `BEGIN`/`COMMIT`. +""" + +from abc import ABC, abstractmethod +from collections.abc import AsyncIterator +from contextlib import AbstractAsyncContextManager, asynccontextmanager + + +class UnitOfWork(ABC): + @abstractmethod + def __call__(self) -> AbstractAsyncContextManager[None]: + """Return an async context manager wrapping its body in one atomic unit of work.""" + + +@asynccontextmanager +async def _noop() -> AsyncIterator[None]: + yield + + +class NoOpUnitOfWork(UnitOfWork): + """Default fallback: wraps nothing, so each write commits immediately. + + Used as the default for service constructors so call sites that predate + the unit-of-work work (e.g. `scripts/seed_demo.py`, direct-construction + test fixtures) keep behaving exactly as before — one implicit transaction + per write — without having to pass a real `UnitOfWork`. + """ + + def __call__(self) -> AbstractAsyncContextManager[None]: + return _noop() diff --git a/backend/app/repositories_impl/sqlite/child_repository.py b/backend/app/repositories_impl/sqlite/child_repository.py index 89f94b9..4e9ccf6 100644 --- a/backend/app/repositories_impl/sqlite/child_repository.py +++ b/backend/app/repositories_impl/sqlite/child_repository.py @@ -5,6 +5,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.child import Child from app.repositories.child_repository import ChildRepository from app.repositories.filter_validator import validate_filters @@ -50,7 +51,7 @@ class SQLiteChildRepository(ChildRepository): item.created_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> Child | None: @@ -74,7 +75,7 @@ class SQLiteChildRepository(ChildRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"Child {id} not found") refreshed = await self.get_by_id(id) @@ -83,7 +84,7 @@ class SQLiteChildRepository(ChildRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM child WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"Child {id} not found") diff --git a/backend/app/repositories_impl/sqlite/day_override_repository.py b/backend/app/repositories_impl/sqlite/day_override_repository.py index 64e4741..25a9a95 100644 --- a/backend/app/repositories_impl/sqlite/day_override_repository.py +++ b/backend/app/repositories_impl/sqlite/day_override_repository.py @@ -10,6 +10,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.day_override import DayOverride from app.repositories.day_override_repository import DayOverrideRepository from app.repositories.filter_validator import validate_filters @@ -52,7 +53,7 @@ class SQLiteDayOverrideRepository(DayOverrideRepository): item.updated_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> DayOverride | None: @@ -78,7 +79,7 @@ class SQLiteDayOverrideRepository(DayOverrideRepository): """, (int(item.is_working_day), item.note, item.updated_at.isoformat(), str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"DayOverride {id} not found") refreshed = await self.get_by_id(id) @@ -87,7 +88,7 @@ class SQLiteDayOverrideRepository(DayOverrideRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM day_override WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"DayOverride {id} not found") diff --git a/backend/app/repositories_impl/sqlite/leave_wish_repository.py b/backend/app/repositories_impl/sqlite/leave_wish_repository.py index c0af868..4641c46 100644 --- a/backend/app/repositories_impl/sqlite/leave_wish_repository.py +++ b/backend/app/repositories_impl/sqlite/leave_wish_repository.py @@ -9,6 +9,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.leave_wish import LeaveWish from app.repositories.filter_validator import validate_filters from app.repositories.leave_wish_repository import LeaveWishRepository @@ -89,7 +90,7 @@ class SQLiteLeaveWishRepository(LeaveWishRepository): item.updated_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> LeaveWish | None: @@ -144,7 +145,7 @@ class SQLiteLeaveWishRepository(LeaveWishRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"LeaveWish {id} not found") refreshed = await self.get_by_id(id) @@ -153,7 +154,7 @@ class SQLiteLeaveWishRepository(LeaveWishRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM leave_wish WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"LeaveWish {id} not found") diff --git a/backend/app/repositories_impl/sqlite/parental_leave_repository.py b/backend/app/repositories_impl/sqlite/parental_leave_repository.py index cdb854d..33ee631 100644 --- a/backend/app/repositories_impl/sqlite/parental_leave_repository.py +++ b/backend/app/repositories_impl/sqlite/parental_leave_repository.py @@ -6,6 +6,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.parental_leave import ParentalLeaveFetch, TierData from app.repositories.filter_validator import validate_filters from app.repositories.parental_leave_repository import ParentalLeaveRepository @@ -90,7 +91,7 @@ class SQLiteParentalLeaveRepository(ParentalLeaveRepository): item.updated_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> ParentalLeaveFetch | None: @@ -150,7 +151,7 @@ class SQLiteParentalLeaveRepository(ParentalLeaveRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"ParentalLeaveFetch {id} not found") refreshed = await self.get_by_id(id) @@ -161,7 +162,7 @@ class SQLiteParentalLeaveRepository(ParentalLeaveRepository): cursor = await self._conn.execute( "DELETE FROM parental_leave_fetch WHERE id = ?", (str(id),) ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"ParentalLeaveFetch {id} not found") diff --git a/backend/app/repositories_impl/sqlite/personal_data_repository.py b/backend/app/repositories_impl/sqlite/personal_data_repository.py index 95dd770..9f5caf3 100644 --- a/backend/app/repositories_impl/sqlite/personal_data_repository.py +++ b/backend/app/repositories_impl/sqlite/personal_data_repository.py @@ -9,6 +9,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.personal_data import PersonalData from app.repositories.filter_validator import validate_filters from app.repositories.personal_data_repository import PersonalDataRepository @@ -55,7 +56,7 @@ class SQLitePersonalDataRepository(PersonalDataRepository): item.deleted_at.isoformat() if item.deleted_at else None, ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> PersonalData | None: @@ -83,7 +84,7 @@ class SQLitePersonalDataRepository(PersonalDataRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"PersonalData {id} not found") refreshed = await self.get_by_id(id) @@ -98,13 +99,13 @@ class SQLitePersonalDataRepository(PersonalDataRepository): "UPDATE personal_data SET deleted_at = ? WHERE uuid = ? AND deleted_at IS NULL", (datetime.now().isoformat(), str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"PersonalData {id} not found") async def hard_delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM personal_data WHERE uuid = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"PersonalData {id} not found") diff --git a/backend/app/repositories_impl/sqlite/planned_leave_repository.py b/backend/app/repositories_impl/sqlite/planned_leave_repository.py index faf715c..2502199 100644 --- a/backend/app/repositories_impl/sqlite/planned_leave_repository.py +++ b/backend/app/repositories_impl/sqlite/planned_leave_repository.py @@ -5,6 +5,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.planned_leave import PlannedLeaveDay from app.repositories.filter_validator import validate_filters from app.repositories.planned_leave_repository import PlannedLeaveRepository @@ -56,7 +57,7 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository): item.updated_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> PlannedLeaveDay | None: @@ -83,7 +84,7 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"PlannedLeaveDay {id} not found") refreshed = await self.get_by_id(id) @@ -92,7 +93,7 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM planned_leave_day WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"PlannedLeaveDay {id} not found") @@ -177,5 +178,73 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository): cursor = await self._conn.execute( "DELETE FROM planned_leave_day WHERE wish_id = ?", (str(wish_id),) ) - await self._conn.commit() + await commit(self._conn) return cursor.rowcount + + async def create_many(self, items: list[PlannedLeaveDay]) -> None: + if not items: + return + now = datetime.now() + rows = [] + for item in items: + item.created_at = item.created_at or now + item.updated_at = item.updated_at or now + rows.append( + ( + str(item.id), + str(item.workspace_id), + str(item.child_id), + str(item.parent_personal_data_uuid), + item.date.isoformat(), + item.day_type, + int(item.is_dubbeldag), + str(item.wish_id) if item.wish_id is not None else None, + int(item.is_pinned), + item.created_at.isoformat(), + item.updated_at.isoformat(), + ) + ) + await self._conn.executemany( + """ + INSERT INTO planned_leave_day + (id, workspace_id, child_id, parent_personal_data_uuid, date, + day_type, is_dubbeldag, wish_id, is_pinned, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + rows, + ) + await commit(self._conn) + + async def delete_many(self, ids: list[UUID]) -> int: + if not ids: + return 0 + str_ids = [str(i) for i in ids] + total = 0 + chunk_size = 500 + for start in range(0, len(str_ids), chunk_size): + chunk = str_ids[start : start + chunk_size] + placeholders = ",".join("?" for _ in chunk) + cursor = await self._conn.execute( + f"DELETE FROM planned_leave_day WHERE id IN ({placeholders})", chunk + ) + total += cursor.rowcount + await commit(self._conn) + return total + + async def update_metadata_many(self, pairs: list[tuple[UUID, UUID | None, bool]]) -> None: + if not pairs: + return + now = datetime.now().isoformat() + rows = [ + (str(wish_id) if wish_id is not None else None, int(is_pinned), now, str(id_)) + for id_, wish_id, is_pinned in pairs + ] + await self._conn.executemany( + """ + UPDATE planned_leave_day + SET wish_id = ?, is_pinned = ?, updated_at = ? + WHERE id = ? + """, + rows, + ) + await commit(self._conn) diff --git a/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py b/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py index 575f0ba..d2effb0 100644 --- a/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py +++ b/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py @@ -11,6 +11,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.tip_dismissal import TipDismissal from app.repositories.filter_validator import validate_filters from app.repositories.tip_dismissal_repository import TipDismissalRepository @@ -46,7 +47,7 @@ class SQLiteTipDismissalRepository(TipDismissalRepository): item.dismissed_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> TipDismissal | None: @@ -68,7 +69,7 @@ class SQLiteTipDismissalRepository(TipDismissalRepository): "UPDATE tip_dismissal SET data_hash = ?, dismissed_at = ? WHERE id = ?", (item.data_hash, item.dismissed_at.isoformat(), str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"TipDismissal {id} not found") refreshed = await self.get_by_id(id) @@ -77,7 +78,7 @@ class SQLiteTipDismissalRepository(TipDismissalRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM tip_dismissal WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"TipDismissal {id} not found") diff --git a/backend/app/repositories_impl/sqlite/unit_of_work.py b/backend/app/repositories_impl/sqlite/unit_of_work.py new file mode 100644 index 0000000..c08f364 --- /dev/null +++ b/backend/app/repositories_impl/sqlite/unit_of_work.py @@ -0,0 +1,21 @@ +"""SQLite implementation of UnitOfWork. + +Delegates to `app.db.connection.unit_of_work`: a locked `BEGIN IMMEDIATE` +transaction on the shared connection. See that function's docstring and the +backend-perf audit §3 for the full design. +""" + +from contextlib import AbstractAsyncContextManager + +import aiosqlite + +from app.db.connection import unit_of_work +from app.repositories.unit_of_work import UnitOfWork + + +class SQLiteUnitOfWork(UnitOfWork): + def __init__(self, connection: aiosqlite.Connection) -> None: + self._conn = connection + + def __call__(self) -> AbstractAsyncContextManager[None]: + return unit_of_work(self._conn) diff --git a/backend/app/repositories_impl/sqlite/user_repository.py b/backend/app/repositories_impl/sqlite/user_repository.py index 4430134..c4edd4f 100644 --- a/backend/app/repositories_impl/sqlite/user_repository.py +++ b/backend/app/repositories_impl/sqlite/user_repository.py @@ -5,6 +5,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.user import User from app.repositories.filter_validator import validate_filters from app.repositories.user_repository import UserRepository @@ -23,6 +24,7 @@ class SQLiteUserRepository(UserRepository): email=row["email"], hashed_password=row["hashed_password"], created_at=datetime.fromisoformat(row["created_at"]), + is_active=bool(row["is_active"]), ) async def create(self, item: User) -> User: @@ -30,8 +32,9 @@ class SQLiteUserRepository(UserRepository): item.created_at = item.created_at or now await self._conn.execute( """ - INSERT INTO "user" (id, personal_data_uuid, email, hashed_password, created_at) - VALUES (?, ?, ?, ?, ?) + INSERT INTO "user" + (id, personal_data_uuid, email, hashed_password, created_at, is_active) + VALUES (?, ?, ?, ?, ?, ?) """, ( str(item.id), @@ -39,9 +42,10 @@ class SQLiteUserRepository(UserRepository): item.email, item.hashed_password, item.created_at.isoformat(), + int(item.is_active), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> User | None: @@ -56,10 +60,10 @@ class SQLiteUserRepository(UserRepository): async def update(self, id: UUID, item: User) -> User: cursor = await self._conn.execute( - 'UPDATE "user" SET email = ?, hashed_password = ? WHERE id = ?', - (item.email, item.hashed_password, str(id)), + 'UPDATE "user" SET email = ?, hashed_password = ?, is_active = ? WHERE id = ?', + (item.email, item.hashed_password, int(item.is_active), str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"User {id} not found") refreshed = await self.get_by_id(id) @@ -68,7 +72,7 @@ class SQLiteUserRepository(UserRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute('DELETE FROM "user" WHERE id = ?', (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"User {id} not found") diff --git a/backend/app/repositories_impl/sqlite/vacation_days_repository.py b/backend/app/repositories_impl/sqlite/vacation_days_repository.py index 050e588..cb34537 100644 --- a/backend/app/repositories_impl/sqlite/vacation_days_repository.py +++ b/backend/app/repositories_impl/sqlite/vacation_days_repository.py @@ -12,6 +12,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.vacation_days import VacationDays from app.repositories.filter_validator import validate_filters from app.repositories.vacation_days_repository import VacationDaysRepository @@ -57,7 +58,7 @@ class SQLiteVacationDaysRepository(VacationDaysRepository): item.updated_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> VacationDays | None: @@ -83,7 +84,7 @@ class SQLiteVacationDaysRepository(VacationDaysRepository): """, (item.total_days, item.saved_days_in, item.updated_at.isoformat(), str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"VacationDays {id} not found") refreshed = await self.get_by_id(id) @@ -92,7 +93,7 @@ class SQLiteVacationDaysRepository(VacationDaysRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM vacation_days WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"VacationDays {id} not found") diff --git a/backend/app/repositories_impl/sqlite/workspace_repository.py b/backend/app/repositories_impl/sqlite/workspace_repository.py index 625d518..355668e 100644 --- a/backend/app/repositories_impl/sqlite/workspace_repository.py +++ b/backend/app/repositories_impl/sqlite/workspace_repository.py @@ -5,6 +5,7 @@ from uuid import UUID import aiosqlite +from app.db.connection import commit from app.entities.workspace import Workspace from app.entities.workspace_invitation import WorkspaceInvitation from app.repositories.filter_validator import validate_filters @@ -42,7 +43,7 @@ class SQLiteWorkspaceRepository(WorkspaceRepository): item.created_at.isoformat(), ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> Workspace | None: @@ -55,7 +56,7 @@ class SQLiteWorkspaceRepository(WorkspaceRepository): "UPDATE workspace SET name = ?, planning_mode = ? WHERE id = ?", (item.name, item.planning_mode, str(id)), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"Workspace {id} not found") refreshed = await self.get_by_id(id) @@ -64,7 +65,7 @@ class SQLiteWorkspaceRepository(WorkspaceRepository): async def delete(self, id: UUID) -> None: cursor = await self._conn.execute("DELETE FROM workspace WHERE id = ?", (str(id),)) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"Workspace {id} not found") @@ -111,7 +112,7 @@ class SQLiteWorkspaceInvitationRepository(WorkspaceInvitationRepository): item.accepted_at.isoformat() if item.accepted_at else None, ), ) - await self._conn.commit() + await commit(self._conn) return item async def get_by_id(self, id: UUID) -> WorkspaceInvitation | None: @@ -141,7 +142,7 @@ class SQLiteWorkspaceInvitationRepository(WorkspaceInvitationRepository): str(id), ), ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"WorkspaceInvitation {id} not found") refreshed = await self.get_by_id(id) @@ -152,7 +153,7 @@ class SQLiteWorkspaceInvitationRepository(WorkspaceInvitationRepository): cursor = await self._conn.execute( "DELETE FROM workspace_invitation WHERE id = ?", (str(id),) ) - await self._conn.commit() + await commit(self._conn) if cursor.rowcount == 0: raise NotFoundError(f"WorkspaceInvitation {id} not found") diff --git a/backend/app/routers/analytics.py b/backend/app/routers/analytics.py index 3a52d58..8c8f72f 100644 --- a/backend/app/routers/analytics.py +++ b/backend/app/routers/analytics.py @@ -6,11 +6,12 @@ from fastapi import APIRouter, HTTPException, status from app.dependencies import ( ChildRepositoryDep, - CurrentUserDep, ParentalLeaveRepositoryDep, PersonalDataRepositoryDep, PlannedLeaveRepositoryDep, + WorkspaceIdDep, WorkspaceRepositoryDep, + WorkspaceScopeDep, ) from app.models.analytics import Age4ProjectionResponse, WorkspaceOverviewResponse from app.services.analytics_service import AnalyticsService @@ -31,14 +32,13 @@ def _make_service( @router.get("/{workspace_id}/overview", response_model=WorkspaceOverviewResponse) async def get_overview( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, child_id: UUID, leave_repo: ParentalLeaveRepositoryDep, planned_repo: PlannedLeaveRepositoryDep, child_repo: ChildRepositoryDep, pd_repo: PersonalDataRepositoryDep, workspace_repo: WorkspaceRepositoryDep, - _current_user: CurrentUserDep, ) -> WorkspaceOverviewResponse: service = _make_service(leave_repo, planned_repo, child_repo, pd_repo, workspace_repo) try: @@ -50,14 +50,13 @@ async def get_overview( @router.get("/{workspace_id}/projection", response_model=WorkspaceOverviewResponse) async def get_projection( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, child_id: UUID, leave_repo: ParentalLeaveRepositoryDep, planned_repo: PlannedLeaveRepositoryDep, child_repo: ChildRepositoryDep, pd_repo: PersonalDataRepositoryDep, workspace_repo: WorkspaceRepositoryDep, - _current_user: CurrentUserDep, ) -> WorkspaceOverviewResponse: service = _make_service(leave_repo, planned_repo, child_repo, pd_repo, workspace_repo) try: @@ -78,11 +77,15 @@ async def get_age4_projection( child_repo: ChildRepositoryDep, pd_repo: PersonalDataRepositoryDep, workspace_repo: WorkspaceRepositoryDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> Age4ProjectionResponse: + await scope.check_member(personal_data_uuid) + await scope.check_child(child_id) service = _make_service(leave_repo, planned_repo, child_repo, pd_repo, workspace_repo) try: - proj = await service.get_days_left_at_age_4(personal_data_uuid, child_id) + proj = await service.get_days_left_at_age_4( + personal_data_uuid, child_id, workspace_id=scope.workspace_id + ) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc return Age4ProjectionResponse.from_entity(proj) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 7852110..7b46eb2 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -63,7 +63,9 @@ async def logout(response: Response) -> None: @router.post("/refresh", response_model=TokenResponse) +@limiter.limit("30/minute") async def refresh( + request: Request, response: Response, service: AuthServiceDep, refresh_token: str | None = Cookie(default=None, alias=_REFRESH_COOKIE), diff --git a/backend/app/routers/calendar.py b/backend/app/routers/calendar.py index fbeca1a..2533df5 100644 --- a/backend/app/routers/calendar.py +++ b/backend/app/routers/calendar.py @@ -9,7 +9,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -from app.dependencies import CalendarServiceDep, CurrentUserDep +from app.dependencies import CalendarServiceDep, WorkspaceScopeDep from app.models.calendar import CalendarDaysResponse from app.services.calendar_service import MAX_WINDOW_YEARS from app.utils.errors import ValidationError @@ -22,9 +22,10 @@ async def get_calendar_days( workspace_id: UUID, start_year: int, service: CalendarServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, years: int = Query(default=MAX_WINDOW_YEARS, ge=1, le=MAX_WINDOW_YEARS), ) -> CalendarDaysResponse: + scope.check_workspace(workspace_id) try: window = await service.get_calendar_window( workspace_id=workspace_id, start_year=start_year, years=years diff --git a/backend/app/routers/day_overrides.py b/backend/app/routers/day_overrides.py index c3550fa..de3a9cc 100644 --- a/backend/app/routers/day_overrides.py +++ b/backend/app/routers/day_overrides.py @@ -8,7 +8,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status -from app.dependencies import DayOverrideServiceDep +from app.dependencies import DayOverrideServiceDep, WorkspaceIdDep, WorkspaceScopeDep from app.models.day_override import DayOverrideCreate, DayOverrideResponse from app.utils.errors import NotFoundError, ValidationError @@ -17,8 +17,9 @@ router = APIRouter(prefix="/api/day-overrides", tags=["day-overrides"]) @router.post("", response_model=DayOverrideResponse, status_code=status.HTTP_201_CREATED) async def upsert_day_override( - payload: DayOverrideCreate, service: DayOverrideServiceDep + payload: DayOverrideCreate, service: DayOverrideServiceDep, scope: WorkspaceScopeDep ) -> DayOverrideResponse: + scope.check_workspace(payload.workspace_id) try: entity = await service.upsert( workspace_id=payload.workspace_id, @@ -33,10 +34,10 @@ async def upsert_day_override( @router.get("/{day_override_id}", response_model=DayOverrideResponse) async def get_day_override( - day_override_id: UUID, service: DayOverrideServiceDep + day_override_id: UUID, service: DayOverrideServiceDep, scope: WorkspaceScopeDep ) -> DayOverrideResponse: try: - entity = await service.get(day_override_id) + entity = await service.get(day_override_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc return DayOverrideResponse.from_entity(entity) @@ -44,15 +45,17 @@ async def get_day_override( @router.get("/workspace/{workspace_id}", response_model=list[DayOverrideResponse]) async def list_day_overrides( - workspace_id: UUID, service: DayOverrideServiceDep + workspace_id: WorkspaceIdDep, service: DayOverrideServiceDep ) -> list[DayOverrideResponse]: entities = await service.list_for_workspace(workspace_id) return [DayOverrideResponse.from_entity(e) for e in entities] @router.delete("/{day_override_id}", status_code=status.HTTP_204_NO_CONTENT) -async def delete_day_override(day_override_id: UUID, service: DayOverrideServiceDep) -> None: +async def delete_day_override( + day_override_id: UUID, service: DayOverrideServiceDep, scope: WorkspaceScopeDep +) -> None: try: - await service.delete(day_override_id) + await service.delete(day_override_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc diff --git a/backend/app/routers/fetch_data.py b/backend/app/routers/fetch_data.py index 39bfc1c..bb7417b 100644 --- a/backend/app/routers/fetch_data.py +++ b/backend/app/routers/fetch_data.py @@ -19,7 +19,7 @@ from app.dependencies import ( ChildRepositoryDep, CurrentUserDep, ParentalLeaveRepositoryDep, - PersonalDataRepositoryDep, + WorkspaceScopeDep, ) from app.entities.parental_leave import ParentalLeaveFetch from app.rate_limit import limiter @@ -55,8 +55,11 @@ _scraper_service = FKScraperService() async def save_manual_entry( payload: ManualFetchRequest, repo: ParentalLeaveRepositoryDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> ParentalLeaveFetchResponse: + scope.check_workspace(payload.workspace_id) + await scope.check_member(payload.personal_data_uuid) + await scope.check_child(payload.child_id) service = ManualFetchService(repo) try: fetch = await service.save_manual_entry( @@ -84,8 +87,10 @@ async def get_latest( personal_data_uuid: UUID, child_id: UUID, repo: ParentalLeaveRepositoryDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> ParentalLeaveFetchResponse | None: + await scope.check_member(personal_data_uuid) + await scope.check_child(child_id) service = ManualFetchService(repo) fetch = await service.get_latest(personal_data_uuid, child_id) return ParentalLeaveFetchResponse.from_entity(fetch) if fetch else None @@ -98,8 +103,10 @@ async def get_history( personal_data_uuid: UUID, child_id: UUID, repo: ParentalLeaveRepositoryDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> list[ParentalLeaveFetchResponse]: + await scope.check_member(personal_data_uuid) + await scope.check_child(child_id) service = ManualFetchService(repo) history = await service.list_history(personal_data_uuid, child_id) return [ParentalLeaveFetchResponse.from_entity(f) for f in history] @@ -134,37 +141,33 @@ async def start_fetch( request: Request, payload: _StartRequest, current_user: CurrentUserDep, - pd_repo: PersonalDataRepositoryDep, + scope: WorkspaceScopeDep, child_repo: ChildRepositoryDep, parental_leave_repo: ParentalLeaveRepositoryDep, ) -> _StartResponse: + """`workspace_id` is always the caller's own — `payload.workspace_id` is + accepted on the wire (frontend compatibility) but never trusted; the + caller's own personal_data_uuid/child_id default when omitted, and any + id explicitly supplied is verified against the caller's workspace. + """ if not settings.scraper_enabled: raise _SCRAPER_DISABLED - personal_data_uuid = payload.personal_data_uuid or current_user.personal_data_uuid + workspace_id = scope.workspace_id + personal_data_uuid = payload.personal_data_uuid or scope.personal_data_uuid + await scope.check_member(personal_data_uuid) - if payload.workspace_id is None or payload.child_id is None: - pd = await pd_repo.get_by_id(personal_data_uuid) - if pd is None: + if payload.child_id is not None: + child = await scope.check_child(payload.child_id) + child_id = child.id + else: + children = await child_repo.list_by_workspace(workspace_id) + if not children: raise HTTPException( status.HTTP_422_UNPROCESSABLE_ENTITY, - "No parent profile for this account.", + "No child in workspace — add one before fetching.", ) - workspace_id = payload.workspace_id or pd.workspace_id - - if payload.child_id is None: - children = await child_repo.list_by_workspace(workspace_id) - if not children: - raise HTTPException( - status.HTTP_422_UNPROCESSABLE_ENTITY, - "No child in workspace — add one before fetching.", - ) - child_id = children[0].id - else: - child_id = payload.child_id - else: - workspace_id = payload.workspace_id - child_id = payload.child_id + child_id = children[0].id # Capture the repo for the background task. The connection lives for the app's # lifetime (app.state.db_connection), so using it from the background coroutine @@ -176,6 +179,7 @@ async def start_fetch( workspace_id=workspace_id, personal_data_uuid=personal_data_uuid, child_id=child_id, + owner_user_id=current_user.id, persist_callback=_persist, ) return _StartResponse( @@ -192,11 +196,11 @@ async def start_fetch( async def poll_fetch( request: Request, session_id: str, - _current_user: CurrentUserDep, + current_user: CurrentUserDep, ) -> dict: if not settings.scraper_enabled: raise _SCRAPER_DISABLED - return await _scraper_service.poll_session(session_id) + return await _scraper_service.poll_session(session_id, current_user.id) @scraper_router.post("/cancel") @@ -204,9 +208,9 @@ async def poll_fetch( async def cancel_fetch( request: Request, payload: _CancelRequest, - _current_user: CurrentUserDep, + current_user: CurrentUserDep, ) -> dict: if not settings.scraper_enabled: raise _SCRAPER_DISABLED - await _scraper_service.cancel_session(payload.session_id) + await _scraper_service.cancel_session(payload.session_id, current_user.id) return {"status": "cancelled"} diff --git a/backend/app/routers/leave_wishes.py b/backend/app/routers/leave_wishes.py index 7ee9162..73f0447 100644 --- a/backend/app/routers/leave_wishes.py +++ b/backend/app/routers/leave_wishes.py @@ -9,7 +9,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status -from app.dependencies import LeaveWishServiceDep +from app.dependencies import LeaveWishServiceDep, WorkspaceIdDep, WorkspaceScopeDep from app.models.leave_wish import ( LeaveWishCreate, LeaveWishResponse, @@ -22,8 +22,13 @@ router = APIRouter(prefix="/api/leave-wishes", tags=["leave-wishes"]) @router.post("", response_model=LeaveWishResponse, status_code=status.HTTP_201_CREATED) async def create_leave_wish( - payload: LeaveWishCreate, service: LeaveWishServiceDep + payload: LeaveWishCreate, service: LeaveWishServiceDep, scope: WorkspaceScopeDep ) -> LeaveWishResponse: + scope.check_workspace(payload.workspace_id) + if payload.child_id is not None: + await scope.check_child(payload.child_id) + if payload.parent_personal_data_uuid is not None: + await scope.check_member(payload.parent_personal_data_uuid) try: entity = await service.create( workspace_id=payload.workspace_id, @@ -51,7 +56,7 @@ async def create_leave_wish( @router.get("/{workspace_id}", response_model=list[LeaveWishResponse]) async def list_leave_wishes( - workspace_id: UUID, service: LeaveWishServiceDep + workspace_id: WorkspaceIdDep, service: LeaveWishServiceDep ) -> list[LeaveWishResponse]: """List all leave wishes for a workspace. Matches PLANNING_ENGINE_SPEC.md §4 (`GET /api/leave-wishes/{workspace_id}` — list-by-workspace, not get-by-id).""" @@ -64,11 +69,17 @@ async def update_leave_wish( leave_wish_id: UUID, payload: LeaveWishUpdate, service: LeaveWishServiceDep, + scope: WorkspaceScopeDep, ) -> LeaveWishResponse: + if payload.child_id is not None: + await scope.check_child(payload.child_id) + if payload.parent_personal_data_uuid is not None: + await scope.check_member(payload.parent_personal_data_uuid) fields_set = payload.model_fields_set try: entity = await service.update( leave_wish_id=leave_wish_id, + workspace_id=scope.workspace_id, child_id=payload.child_id, name=payload.name, name_message_key=payload.name_message_key, @@ -104,8 +115,10 @@ async def update_leave_wish( @router.delete("/{leave_wish_id}", status_code=status.HTTP_204_NO_CONTENT) -async def delete_leave_wish(leave_wish_id: UUID, service: LeaveWishServiceDep) -> None: +async def delete_leave_wish( + leave_wish_id: UUID, service: LeaveWishServiceDep, scope: WorkspaceScopeDep +) -> None: try: - await service.delete(leave_wish_id) + await service.delete(leave_wish_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc diff --git a/backend/app/routers/plan.py b/backend/app/routers/plan.py index 07b62ba..09d3a89 100644 --- a/backend/app/routers/plan.py +++ b/backend/app/routers/plan.py @@ -9,7 +9,7 @@ PLANNING_ENGINE_SPEC.md §4. from fastapi import APIRouter, HTTPException, status -from app.dependencies import PlanningEngineServiceDep, PlanningServiceDep +from app.dependencies import PlanningEngineServiceDep, PlanningServiceDep, WorkspaceScopeDep from app.models.plan import ( PlanApplyRequest, PlanApplyResponse, @@ -25,9 +25,10 @@ router = APIRouter(prefix="/api/plan", tags=["plan"]) @router.post("/propose", response_model=PlanProposalResponse) async def propose_plan( - payload: PlanProposeRequest, service: PlanningEngineServiceDep + payload: PlanProposeRequest, service: PlanningEngineServiceDep, scope: WorkspaceScopeDep ) -> PlanProposalResponse: """Compute a plan proposal. Read-only — persists nothing, safe to spam.""" + scope.check_workspace(payload.workspace_id) try: proposal = await service.propose(payload.workspace_id) except NotFoundError as exc: @@ -38,7 +39,9 @@ async def propose_plan( @router.post("/apply", response_model=PlanApplyResponse) -async def apply_plan(payload: PlanApplyRequest, service: PlanningServiceDep) -> PlanApplyResponse: +async def apply_plan( + payload: PlanApplyRequest, service: PlanningServiceDep, scope: WorkspaceScopeDep +) -> PlanApplyResponse: """Accept a (possibly user-edited) set of proposed days as real plan. Always returns 200 with `accepted: false` and a `rejections`/`collisions` @@ -48,6 +51,15 @@ async def apply_plan(payload: PlanApplyRequest, service: PlanningServiceDep) -> A genuinely missing workspace/child (404) is the one case still raised as an HTTP error, since that's not something the batch report can explain. """ + scope.check_workspace(payload.workspace_id) + # Every child/parent named in the batch must belong to the caller's own + # workspace too — otherwise a forged child_id/parent_personal_data_uuid + # could write a planned_leave_day row attributed to another tenant's + # child or parent even though workspace_id itself checks out. + for child_id in {item.child_id for item in payload.days}: + await scope.check_child(child_id) + for parent_id in {item.parent_personal_data_uuid for item in payload.days}: + await scope.check_member(parent_id) try: result = await service.apply_proposed_days( workspace_id=payload.workspace_id, @@ -60,7 +72,9 @@ async def apply_plan(payload: PlanApplyRequest, service: PlanningServiceDep) -> @router.post("/sync", response_model=PlanSyncResponse) -async def sync_plan(payload: PlanSyncRequest, service: PlanningServiceDep) -> PlanSyncResponse: +async def sync_plan( + payload: PlanSyncRequest, service: PlanningServiceDep, scope: WorkspaceScopeDep +) -> PlanSyncResponse: """Recompute and persist a workspace's plan in one step. The direct-manipulation calendar UX's one call: no separate preview @@ -70,6 +84,7 @@ async def sync_plan(payload: PlanSyncRequest, service: PlanningServiceDep) -> Pl can show e.g. "Sommar 2028 flyttades 2 dagar — Ångra" instead of silently rewriting the calendar. """ + scope.check_workspace(payload.workspace_id) try: result = await service.sync_plan(payload.workspace_id) except NotFoundError as exc: diff --git a/backend/app/routers/planned_days.py b/backend/app/routers/planned_days.py index 3c40631..e0358c9 100644 --- a/backend/app/routers/planned_days.py +++ b/backend/app/routers/planned_days.py @@ -4,7 +4,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -from app.dependencies import CurrentUserDep, PlanningServiceDep +from app.dependencies import PlanningServiceDep, WorkspaceIdDep, WorkspaceScopeDep from app.models.planned_leave import ( PlannedLeaveDayCreate, PlannedLeaveDayResponse, @@ -23,8 +23,11 @@ def _to_response(day) -> PlannedLeaveDayResponse: async def create_planned_day( payload: PlannedLeaveDayCreate, service: PlanningServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> PlannedLeaveDayResponse: + scope.check_workspace(payload.workspace_id) + await scope.check_child(payload.child_id) + await scope.check_member(payload.parent_personal_data_uuid) try: day = await service.add_planned_day( workspace_id=payload.workspace_id, @@ -44,9 +47,10 @@ async def create_planned_day( @router.get("", response_model=list[PlannedLeaveDayResponse]) async def list_planned_days( service: PlanningServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, child_id: UUID = Query(...), ) -> list[PlannedLeaveDayResponse]: + await scope.check_child(child_id) days = await service.list_by_child(child_id) return [_to_response(d) for d in days] @@ -55,10 +59,10 @@ async def list_planned_days( async def get_planned_day( planned_leave_day_id: UUID, service: PlanningServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> PlannedLeaveDayResponse: try: - day = await service.get(planned_leave_day_id) + day = await service.get(planned_leave_day_id, workspace_id=scope.workspace_id) return _to_response(day) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc @@ -66,9 +70,8 @@ async def get_planned_day( @router.get("/workspace/{workspace_id}", response_model=list[PlannedLeaveDayResponse]) async def list_planned_days_by_workspace( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: PlanningServiceDep, - _current_user: CurrentUserDep, ) -> list[PlannedLeaveDayResponse]: days = await service.list_by_workspace(workspace_id) return [_to_response(d) for d in days] @@ -79,7 +82,7 @@ async def update_planned_day( planned_leave_day_id: UUID, payload: PlannedLeaveDayUpdate, service: PlanningServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> PlannedLeaveDayResponse: try: day = await service.update( @@ -88,6 +91,7 @@ async def update_planned_day( is_dubbeldag=payload.is_dubbeldag, wish_id=payload.wish_id, is_pinned=payload.is_pinned, + workspace_id=scope.workspace_id, ) return _to_response(day) except NotFoundError as exc: @@ -98,9 +102,9 @@ async def update_planned_day( async def delete_planned_day( planned_leave_day_id: UUID, service: PlanningServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> None: try: - await service.remove_planned_day(planned_leave_day_id) + await service.remove_planned_day(planned_leave_day_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc diff --git a/backend/app/routers/settings.py b/backend/app/routers/settings.py index 8eb864a..db26be7 100644 --- a/backend/app/routers/settings.py +++ b/backend/app/routers/settings.py @@ -14,9 +14,10 @@ from fastapi import APIRouter, HTTPException, status from app.dependencies import ( ChildServiceDep, - CurrentUserDep, PersonalDataServiceDep, TipsServiceDep, + WorkspaceIdDep, + WorkspaceScopeDep, ) from app.models.child import ChildCreate, ChildResponse, ChildUpdate from app.models.personal_data import PersonalDataResponse, PersonalDataUpdate @@ -35,9 +36,8 @@ tips_router = APIRouter(prefix="/api/tips", tags=["tips"]) @tips_router.get("/{workspace_id}", response_model=list[TipResponse]) async def get_tips( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: TipsServiceDep, - _current_user: CurrentUserDep, ) -> list[TipResponse]: tips = await service.get_tips(workspace_id) return [TipResponse.from_domain(tip) for tip in tips] @@ -45,10 +45,9 @@ async def get_tips( @tips_router.post("/{workspace_id}/dismissals", status_code=status.HTTP_204_NO_CONTENT) async def dismiss_tip( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, payload: TipDismissalCreate, service: TipsServiceDep, - _current_user: CurrentUserDep, ) -> None: try: await service.dismiss(workspace_id, payload.tip_id) @@ -71,8 +70,9 @@ def _child_to_response(child) -> ChildResponse: async def create_child( payload: ChildCreate, service: ChildServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> ChildResponse: + scope.check_workspace(payload.workspace_id) try: child = await service.create( workspace_id=payload.workspace_id, @@ -90,10 +90,10 @@ async def create_child( async def get_child( child_id: UUID, service: ChildServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> ChildResponse: try: - child = await service.get(child_id) + child = await service.get(child_id, workspace_id=scope.workspace_id) return _child_to_response(child) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc @@ -101,9 +101,8 @@ async def get_child( @child_router.get("/workspace/{workspace_id}", response_model=list[ChildResponse]) async def list_children( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: ChildServiceDep, - _current_user: CurrentUserDep, ) -> list[ChildResponse]: children = await service.list_by_workspace(workspace_id) return [_child_to_response(c) for c in children] @@ -118,7 +117,7 @@ async def update_child( child_id: UUID, payload: ChildUpdate, service: ChildServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> ChildResponse: try: child = await service.update( @@ -127,6 +126,7 @@ async def update_child( birth_date=payload.birth_date, preschool_start_date=payload.preschool_start_date, preschool_planning_days_per_year=payload.preschool_planning_days_per_year, + workspace_id=scope.workspace_id, ) return _child_to_response(child) except ValidationError as exc: @@ -139,10 +139,10 @@ async def update_child( async def delete_child( child_id: UUID, service: ChildServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> None: try: - await service.delete(child_id) + await service.delete(child_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc @@ -152,7 +152,7 @@ async def update_parent_settings( personal_data_uuid: UUID, payload: PersonalDataUpdate, service: PersonalDataServiceDep, - _current_user: CurrentUserDep, + scope: WorkspaceScopeDep, ) -> PersonalDataResponse: try: updated = await service.update( @@ -160,6 +160,7 @@ async def update_parent_settings( salary_type=payload.salary_type, employment_status=payload.employment_status, min_vacation_days_per_year=payload.min_vacation_days_per_year, + workspace_id=scope.workspace_id, ) return PersonalDataResponse.from_entity(updated) except ValidationError as exc: diff --git a/backend/app/routers/vacation_days.py b/backend/app/routers/vacation_days.py index 295ca18..b03abe3 100644 --- a/backend/app/routers/vacation_days.py +++ b/backend/app/routers/vacation_days.py @@ -8,7 +8,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status -from app.dependencies import VacationDaysServiceDep +from app.dependencies import VacationDaysServiceDep, WorkspaceIdDep, WorkspaceScopeDep from app.models.vacation_days import VacationDaysCreate, VacationDaysResponse from app.utils.errors import NotFoundError, ValidationError @@ -17,8 +17,10 @@ router = APIRouter(prefix="/api/vacation-days", tags=["vacation-days"]) @router.post("", response_model=VacationDaysResponse, status_code=status.HTTP_201_CREATED) async def upsert_vacation_days( - payload: VacationDaysCreate, service: VacationDaysServiceDep + payload: VacationDaysCreate, service: VacationDaysServiceDep, scope: WorkspaceScopeDep ) -> VacationDaysResponse: + scope.check_workspace(payload.workspace_id) + await scope.check_member(payload.personal_data_uuid) try: entity = await service.upsert( personal_data_uuid=payload.personal_data_uuid, @@ -34,10 +36,10 @@ async def upsert_vacation_days( @router.get("/{vacation_days_id}", response_model=VacationDaysResponse) async def get_vacation_days( - vacation_days_id: UUID, service: VacationDaysServiceDep + vacation_days_id: UUID, service: VacationDaysServiceDep, scope: WorkspaceScopeDep ) -> VacationDaysResponse: try: - entity = await service.get(vacation_days_id) + entity = await service.get(vacation_days_id, workspace_id=scope.workspace_id) except NotFoundError as exc: raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc return VacationDaysResponse.from_entity(entity) @@ -45,7 +47,7 @@ async def get_vacation_days( @router.get("/workspace/{workspace_id}", response_model=list[VacationDaysResponse]) async def list_vacation_days( - workspace_id: UUID, service: VacationDaysServiceDep + workspace_id: WorkspaceIdDep, service: VacationDaysServiceDep ) -> list[VacationDaysResponse]: entities = await service.list_for_workspace(workspace_id) return [VacationDaysResponse.from_entity(e) for e in entities] diff --git a/backend/app/routers/workspace.py b/backend/app/routers/workspace.py index 6717f46..d2360ab 100644 --- a/backend/app/routers/workspace.py +++ b/backend/app/routers/workspace.py @@ -4,7 +4,12 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status -from app.dependencies import CurrentUserDep, WorkspaceServiceDep +from app.dependencies import ( + CurrentUserDep, + CurrentWorkspaceIdDep, + WorkspaceIdDep, + WorkspaceServiceDep, +) from app.entities.personal_data import PersonalData from app.models.workspace import ( InvitationResponse, @@ -62,9 +67,8 @@ async def create_workspace( @router.get("/{workspace_id}", response_model=WorkspaceResponse) async def get_workspace( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, ) -> WorkspaceResponse: try: ws = await service.get_workspace(workspace_id) @@ -75,10 +79,9 @@ async def get_workspace( @router.put("/{workspace_id}", response_model=WorkspaceResponse) async def update_workspace( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, payload: WorkspaceUpdate, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, ) -> WorkspaceResponse: try: ws = await service.update_workspace(workspace_id, payload.name, payload.planning_mode) @@ -94,9 +97,8 @@ async def update_workspace( @router.get("/{workspace_id}/members", response_model=list[PersonalDataResponse]) async def list_members( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, ) -> list[PersonalDataResponse]: members = await service.list_members(workspace_id) return [_to_response(m) for m in members] @@ -108,10 +110,9 @@ async def list_members( status_code=status.HTTP_201_CREATED, ) async def add_placeholder_parent( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, payload: PersonalDataCreate, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, ) -> PersonalDataResponse: try: pd = await service.add_placeholder_parent(workspace_id, payload.display_name) @@ -125,11 +126,10 @@ async def add_placeholder_parent( response_model=PersonalDataResponse, ) async def rename_parent( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, personal_data_uuid: UUID, payload: PersonalDataCreate, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, ) -> PersonalDataResponse: try: pd = await service.rename_member(workspace_id, personal_data_uuid, payload.display_name) @@ -143,7 +143,7 @@ async def rename_parent( status_code=status.HTTP_204_NO_CONTENT, ) async def remove_parent( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, personal_data_uuid: UUID, service: WorkspaceServiceDep, current_user: CurrentUserDep, @@ -162,7 +162,7 @@ async def remove_parent( status_code=status.HTTP_201_CREATED, ) async def create_invitation( - workspace_id: UUID, + workspace_id: WorkspaceIdDep, service: WorkspaceServiceDep, current_user: CurrentUserDep, ) -> InviteResponse: @@ -194,10 +194,10 @@ async def redeem_invitation( async def revoke_invitation( invitation_id: UUID, service: WorkspaceServiceDep, - _current_user: CurrentUserDep, + workspace_id: CurrentWorkspaceIdDep, ) -> None: try: - await service.revoke_invitation(invitation_id) + await service.revoke_invitation(invitation_id, workspace_id) except (NotFoundError, ValidationError) as exc: code = ( status.HTTP_404_NOT_FOUND diff --git a/backend/app/scrapers/fk_scraper.py b/backend/app/scrapers/fk_scraper.py index e2d9ea7..83c4aa3 100644 --- a/backend/app/scrapers/fk_scraper.py +++ b/backend/app/scrapers/fk_scraper.py @@ -4,10 +4,17 @@ Legal/ToS review completed (2026-06). Legal has cleared automated read-only access for users who have given explicit per-session consent. Operational guardrails must be maintained (see §2.3): - Sandboxed browser context, torn down after each session. - - No cookies, session tokens, or QR images ever persisted to disk or DB. - - No personal data (names, personnummer, day counts) in logs. + - No cookies, session tokens, or QR images ever persisted to disk or DB by + default. Debug artifacts (HTML + screenshot of the logged-in page) are + OFF unless both scraper_debug_enabled and scraper_debug_dir are set — + they contain personal data and are for local debugging only. + - No personal data (names, personnummer, day counts, raw page text) in + logs above DEBUG; DEBUG lines log shapes/counts, never scraped values. - Per-session user consent required before starting a session. - Only the extracted day-count numbers are persisted (see persist_callback). + - QR frames and results are retained in RAM only for + scraper_result_retention_seconds after a session ends, then purged; at + most one live session per user. Enable via APP_SCRAPER_ENABLED=true in the environment or backend/.env. @@ -94,6 +101,10 @@ class _SessionState: workspace_id: UUID personal_data_uuid: UUID child_id: UUID + # The User.id that started this session. `poll_session`/`cancel_session` + # treat a mismatched owner exactly like an unknown session_id — never a + # 403, so a foreign session_id can't be distinguished from a made-up one. + owner_user_id: UUID status: SessionStatus = "launching" qr_data: bytes | None = None # raw PNG bytes; never persisted to disk/DB result: ParentalLeaveFetch | None = None @@ -146,19 +157,35 @@ class FKScraperService(FetchDataService): workspace_id: UUID, personal_data_uuid: UUID, child_id: UUID, + owner_user_id: UUID, persist_callback: Callable[[ParentalLeaveFetch], Awaitable[None]] | None = None, ) -> str: """Launch a new headless browser session. Returns the session_id. persist_callback, if provided, is called with the validated ParentalLeaveFetch once extraction succeeds. Failure in the callback sets the session to 'error'. + `owner_user_id` binds the session to the caller that started it — see + `_SessionState.owner_user_id`. + + At most one live (non-terminal) session per `owner_user_id` — starting + a new one cancels any session already in flight for the same caller + (see F5: unbounded growth of `_sessions` per owner). """ + for existing_id, existing_state in list(_sessions.items()): + if existing_state.owner_user_id == owner_user_id and existing_state.status not in ( + "done", + "error", + "cancelled", + ): + await self.cancel_session(existing_id, owner_user_id) + session_id = str(uuid.uuid4()) state = _SessionState( session_id=session_id, workspace_id=workspace_id, personal_data_uuid=personal_data_uuid, child_id=child_id, + owner_user_id=owner_user_id, status="launching", persist_callback=persist_callback, ) @@ -172,10 +199,15 @@ class FKScraperService(FetchDataService): logger.info("scraper session started [id=%s]", session_id) return session_id - async def poll_session(self, session_id: str) -> dict: - """Return current status, QR bytes (base64), or result.""" + async def poll_session(self, session_id: str, owner_user_id: UUID) -> dict: + """Return current status, QR bytes (base64), or result. + + A `session_id` owned by a different caller is treated exactly like + an unknown one — never a 403, so a foreign session_id can't be + distinguished from a made-up one. + """ state = _sessions.get(session_id) - if state is None: + if state is None or state.owner_user_id != owner_user_id: return {"status": "error", "message": "Unknown session_id"} response: dict = {"status": state.status} @@ -187,12 +219,17 @@ class FKScraperService(FetchDataService): response["message"] = state.message return response - async def cancel_session(self, session_id: str) -> None: - """Cancel a running session and tear down the browser.""" + async def cancel_session(self, session_id: str, owner_user_id: UUID) -> None: + """Cancel a running session and tear down the browser. + + A `session_id` owned by a different caller is a no-op, exactly like + an unknown one — never a 403 (see `poll_session`). + """ state = _sessions.get(session_id) - if state is None: + if state is None or state.owner_user_id != owner_user_id: return _set_status(state, "cancelled") + state.qr_data = None if state._task and not state._task.done(): state._task.cancel() logger.info("scraper session cancelled [id=%s]", session_id) @@ -209,15 +246,50 @@ def _set_status(state: _SessionState, status: SessionStatus, message: str | None state.message = message +def _generic_error_message(exc: Exception) -> str: + """Map an internal exception to a fixed, generic message safe to return + to the poll client (see F4). Never embeds `str(exc)` — a `ValueError` + raised while parsing the FK page may otherwise carry raw page text. + Logs carry the exception type name only; full detail is never emitted + above DEBUG and is not persisted unless debug artifacts are explicitly + enabled (`scraper_debug_enabled`). + """ + if isinstance(exc, ValueError): + return "Scraper error: could not read data from the FK page. Please try again." + if type(exc).__name__ == "TimeoutError": + return "Scraper error: timed out waiting for the FK page. Please try again." + return "Scraper error: an unexpected error occurred. Please try again." + + async def _ttl_watchdog(session_id: str) -> None: - """Auto-cancel sessions that sit in awaiting_scan/authenticating too long.""" + """Auto-cancel sessions that sit in awaiting_scan/authenticating too long. + + Also acts as a secondary safety net (F5): if the session has already + reached a terminal state by the time this fires, purge it immediately + rather than rely solely on `_purge_later`. + """ await asyncio.sleep(settings.scraper_session_ttl_seconds) state = _sessions.get(session_id) - if state and state.status in ("awaiting_scan", "authenticating", "launching"): + if state is None: + return + if state.status in ("awaiting_scan", "authenticating", "launching"): _set_status(state, "error", "Session timed out waiting for BankID scan") + state.qr_data = None if state._task and not state._task.done(): state._task.cancel() logger.info("scraper session timed out [id=%s]", session_id) + elif state.status in ("done", "error", "cancelled"): + _sessions.pop(session_id, None) + + +async def _purge_later(session_id: str) -> None: + """Remove a finished session from `_sessions` after the retention window. + + Bounds how long a scraped result or a stale (already-cleared) QR frame + slot can be read back via `/poll` after the session ended (see F5). + """ + await asyncio.sleep(settings.scraper_result_retention_seconds) + _sessions.pop(session_id, None) async def _check_maintenance(page) -> str | None: @@ -243,9 +315,14 @@ async def _check_maintenance(page) -> str | None: async def _save_debug_artifacts(page, session_id: str, console_messages: list[str]) -> None: - """Save HTML, screenshot, and console log to scraper_debug_dir on error.""" + """Save HTML, screenshot, and console log to scraper_debug_dir on error. + + OFF by default and requires both `scraper_debug_enabled` and + `scraper_debug_dir` to be set — these artifacts contain personal data + (name, balances, payout amounts) from the logged-in FK page. + """ debug_dir = settings.scraper_debug_dir - if not debug_dir: + if not settings.scraper_debug_enabled or not debug_dir: return try: out = Path(debug_dir) @@ -289,8 +366,10 @@ async def _run_session(state: _SessionState) -> None: except asyncio.CancelledError: _set_status(state, "cancelled") except Exception as exc: - _set_status(state, "error", f"Scraper error: {type(exc).__name__}: {exc}") - logger.error("scraper error [id=%s]: %s", state.session_id, exc, exc_info=False) + _set_status(state, "error", _generic_error_message(exc)) + logger.error( + "scraper error [id=%s]: %s", state.session_id, type(exc).__name__, exc_info=False + ) await _save_debug_artifacts(page, state.session_id, console_messages) finally: try: @@ -299,6 +378,10 @@ async def _run_session(state: _SessionState) -> None: except Exception: pass # already torn down + # Session has reached a terminal state (done/error/cancelled) — schedule + # its removal from `_sessions` after the retention window (see F5). + asyncio.create_task(_purge_later(state.session_id)) + async def _do_login(page, state: _SessionState) -> None: """Navigate to FK login, handle cookie consent, click BankID QR, relay QR.""" @@ -372,6 +455,10 @@ async def _do_login(page, state: _SessionState) -> None: logger.debug("[%s] QR element gone; BankID app likely confirming", state.session_id) break + # Leaving awaiting_scan (scanned, cancelled, or timed out) — the last QR + # frame is no longer needed and must not linger in RAM (see F5). + state.qr_data = None + if state.status == "cancelled": return @@ -440,14 +527,14 @@ async def _extract_parental_leave(page, state: _SessionState) -> ParentalLeaveFe logger.debug("[%s] reading days_left (FOR_DIG_DAGAR_KVAR)", state.session_id) days_left_text = await page.locator(_SEL.FOR_DIG_DAGAR_KVAR).first.inner_text(timeout=5_000) days_left = int(days_left_text.strip()) - logger.debug("[%s] days_left=%d", state.session_id, days_left) + logger.debug("[%s] days_left parsed", state.session_id) # -- total days from "av 240 dagar kvar efter ansökan" -- logger.debug("[%s] reading total days (FOR_DIG_DISCRETE)", state.session_id) discrete_text = await page.locator(_SEL.FOR_DIG_DISCRETE).first.inner_text(timeout=5_000) total_match = re.search(r"av\s+(\d+)\s+dagar", discrete_text) if not total_match: - raise ValueError(f"Could not parse total days from: {discrete_text!r}") + raise ValueError("Could not parse total days from FK page text") total_days = int(total_match.group(1)) if total_days == 0: raise ValueError( @@ -455,14 +542,14 @@ async def _extract_parental_leave(page, state: _SessionState) -> ParentalLeaveFe "(placeholder state). The hydration wait in _do_extract should prevent this." ) days_used = total_days - days_left - logger.debug("[%s] total_days=%d days_used=%d", state.session_id, total_days, days_used) + logger.debug("[%s] total/used parsed", state.session_id) # -- sjukpenningnivå and lägstanivå remaining (desktop niva-statistik) -- # all_inner_texts() does not accept a timeout argument — wait for the first element first. logger.debug("[%s] reading tier breakdown (NIVA_STATISTIK_DESKTOP)", state.session_id) await page.locator(_SEL.NIVA_STATISTIK_DESKTOP).first.wait_for(timeout=5_000) niva_texts = await page.locator(_SEL.NIVA_STATISTIK_DESKTOP).all_inner_texts() - logger.debug("[%s] niva_texts raw=%r", state.session_id, niva_texts) + logger.debug("[%s] niva items=%d", state.session_id, len(niva_texts)) # Each item is like "(119 dagar)" — filter to desktop section (first two) niva_numbers = [] for t in niva_texts: @@ -473,31 +560,20 @@ async def _extract_parental_leave(page, state: _SessionState) -> ParentalLeaveFe break if len(niva_numbers) < 2: - raise ValueError(f"Expected 2 tier values, got {niva_numbers} from {niva_texts!r}") + raise ValueError(f"Expected 2 tier values, got {len(niva_numbers)}") sjukpenning_left, lagsta_niva_left = niva_numbers[0], niva_numbers[1] - logger.debug( - "[%s] sjukpenning_left=%d lagsta_niva_left=%d", - state.session_id, - sjukpenning_left, - lagsta_niva_left, - ) + logger.debug("[%s] tier breakdown parsed", state.session_id) # Cross-check: sjukpenning_left + lagsta_niva_left should equal days_left if sjukpenning_left + lagsta_niva_left != days_left: - logger.warning( - "Tier sum mismatch: sjukpenning=%d + lagstaniva=%d != days_left=%d [id=%s]", - sjukpenning_left, - lagsta_niva_left, - days_left, - state.session_id, - ) + logger.warning("Tier sum mismatch [id=%s]", state.session_id) # -- dubbeldagar remaining -- logger.debug("[%s] reading dubbeldagar (DUBBELDAG_KVAR)", state.session_id) dubbeldag_text = await page.locator(_SEL.DUBBELDAG_KVAR).first.inner_text(timeout=5_000) dubbeldagar_left = int(dubbeldag_text.strip()) - logger.debug("[%s] dubbeldagar_left=%d", state.session_id, dubbeldagar_left) + logger.debug("[%s] dubbeldagar parsed", state.session_id) # -- Derive tier totals (standard FK allocation per parent) -- # Standard: 195 sjukpenningnivå + 45 lägstanivå = 240 per parent. diff --git a/backend/app/services/analytics_service.py b/backend/app/services/analytics_service.py index 74eaa96..e8e6b3b 100644 --- a/backend/app/services/analytics_service.py +++ b/backend/app/services/analytics_service.py @@ -92,13 +92,17 @@ class AnalyticsService: return resolve_age4_savable_cap(planning_mode, len(members)) async def get_days_left_at_age_4( - self, personal_data_uuid: UUID, child_id: UUID + self, personal_data_uuid: UUID, child_id: UUID, workspace_id: UUID | None = None ) -> Age4Projection: - """Compute the headline projection for a single parent+child pair.""" + """Compute the headline projection for a single parent+child pair. + + `workspace_id`, when given, must match the child's own — a foreign + workspace's child 404s exactly like a nonexistent one. + """ rules = settings.rules child = await self._children.get_by_id(child_id) - if child is None: + if child is None or (workspace_id is not None and child.workspace_id != workspace_id): raise NotFoundError(f"Child {child_id} not found") # Compute the 4th birthday with leap-year-safe arithmetic. diff --git a/backend/app/services/auth_service.py b/backend/app/services/auth_service.py index 53914ba..15305a9 100644 --- a/backend/app/services/auth_service.py +++ b/backend/app/services/auth_service.py @@ -11,6 +11,7 @@ TODO (post-MVP): password reset + email verification flows are not implemented. from __future__ import annotations +import asyncio from datetime import datetime, timedelta, timezone from uuid import UUID, uuid4 @@ -23,12 +24,19 @@ from app.entities.personal_data import PersonalData from app.entities.user import User from app.entities.workspace import Workspace from app.repositories.personal_data_repository import PersonalDataRepository +from app.repositories.unit_of_work import NoOpUnitOfWork, UnitOfWork from app.repositories.user_repository import UserRepository from app.repositories.workspace_repository import WorkspaceRepository from app.utils.errors import AuthError, ConflictError, UnauthorizedError _ph = PasswordHasher() +# Precomputed at import time so a login against an unknown email still pays the +# cost of an argon2 verify (against a hash nobody's password can match) — this +# keeps the response-time profile of "unknown email" indistinguishable from +# "known email, wrong password" and closes the user-enumeration timing oracle. +_DUMMY_HASH = _ph.hash(uuid4().hex) + class AuthService: def __init__( @@ -36,10 +44,16 @@ class AuthService: user_repo: UserRepository, personal_data_repo: PersonalDataRepository, workspace_repo: WorkspaceRepository, + uow: UnitOfWork | None = None, ) -> None: self._users = user_repo self._personal_data = personal_data_repo self._workspaces = workspace_repo + # Defaults to a no-op so pre-existing direct-construction call sites + # (scripts/seed_demo.py, test fixtures) keep today's per-write-commit + # behavior; `app.dependencies.get_auth_service` always wires a real + # `SQLiteUnitOfWork`. + self._uow = uow or NoOpUnitOfWork() async def register( self, email: str, password: str, display_name: str @@ -47,45 +61,66 @@ class AuthService: """Create User + personal workspace + PersonalData. Raises ConflictError if email taken.""" existing = await self._users.get_by_email(email) if existing is not None: - raise ConflictError(f"Email already registered: {email}") + raise ConflictError("Email already registered") - # Create a personal workspace for the registering user. - workspace = Workspace( - id=uuid4(), - name=f"{display_name}s planering", - created_at=datetime.now(), - ) - workspace = await self._workspaces.create(workspace) + # Hash before opening the unit of work: argon2 is CPU work + # (offloaded via to_thread), not a DB write — no reason to hold the + # write lock for it. + hashed_password = await asyncio.to_thread(_ph.hash, password) - pd = PersonalData( - uuid=uuid4(), - workspace_id=workspace.id, - name=display_name, - created_at=datetime.now(), - ) - pd = await self._personal_data.create(pd) + # Workspace + PersonalData + User must land together: a crash between + # any two of these would otherwise leave an orphaned workspace, a + # personal_data row with no login, or (with the FK ON DELETE CASCADE) + # a user pointing at nothing. + async with self._uow(): + # Create a personal workspace for the registering user. + workspace = Workspace( + id=uuid4(), + name=f"{display_name}s planering", + created_at=datetime.now(), + ) + workspace = await self._workspaces.create(workspace) - user = User( - id=uuid4(), - personal_data_uuid=pd.uuid, - email=email, - hashed_password=_ph.hash(password), - created_at=datetime.now(), - ) - user = await self._users.create(user) + pd = PersonalData( + uuid=uuid4(), + workspace_id=workspace.id, + name=display_name, + created_at=datetime.now(), + ) + pd = await self._personal_data.create(pd) + + user = User( + id=uuid4(), + personal_data_uuid=pd.uuid, + email=email, + hashed_password=hashed_password, + created_at=datetime.now(), + ) + user = await self._users.create(user) return user, pd async def login(self, email: str, password: str) -> User: - """Verify credentials. Raises AuthError on wrong email or password.""" + """Verify credentials. Raises AuthError on wrong email/password/inactive account. + + Always uses a generic message so unknown-email and wrong-password are + indistinguishable, and always runs an argon2 verify (real or dummy) so + the two cases take comparable time. + """ user = await self._users.get_by_email(email) if user is None: + try: + await asyncio.to_thread(_ph.verify, _DUMMY_HASH, password) + except VerifyMismatchError: + pass raise AuthError("Invalid email or password") try: - _ph.verify(user.hashed_password, password) + await asyncio.to_thread(_ph.verify, user.hashed_password, password) except VerifyMismatchError as exc: raise AuthError("Invalid email or password") from exc + if not user.is_active: + raise UnauthorizedError("Account is inactive") if _ph.check_needs_rehash(user.hashed_password): - user.hashed_password = _ph.hash(password) + user.hashed_password = await asyncio.to_thread(_ph.hash, password) await self._users.update(user.id, user) return user @@ -117,6 +152,8 @@ class AuthService: user = await self._users.get_by_id(user_id) if user is None: raise UnauthorizedError("User not found") + if not user.is_active: + raise UnauthorizedError("Account is inactive") return user async def refresh_access_token(self, refresh_token: str) -> str: diff --git a/backend/app/services/child_service.py b/backend/app/services/child_service.py index 8ceebd9..3ce03d9 100644 --- a/backend/app/services/child_service.py +++ b/backend/app/services/child_service.py @@ -37,10 +37,12 @@ class ChildService: ) return await self._repo.create(entity) - async def get(self, child_id: UUID) -> Child: - """Get a child by ID.""" + async def get(self, child_id: UUID, workspace_id: UUID | None = None) -> Child: + """Get a child by ID. `workspace_id`, when given, must match the + child's own — a foreign workspace's child 404s exactly like a + nonexistent one.""" entity = await self._repo.get_by_id(child_id) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"Child {child_id} not found") return entity @@ -55,9 +57,10 @@ class ChildService: birth_date=None, preschool_start_date=None, preschool_planning_days_per_year: int | None = None, + workspace_id: UUID | None = None, ) -> Child: """Update a child (partial update).""" - entity = await self.get(child_id) + entity = await self.get(child_id, workspace_id) if name is not None: entity.name = name @@ -78,9 +81,9 @@ class ChildService: return await self._repo.update(child_id, entity) - async def delete(self, child_id: UUID) -> None: + async def delete(self, child_id: UUID, workspace_id: UUID | None = None) -> None: """Delete a child.""" - await self.get(child_id) # raises NotFoundError if missing + await self.get(child_id, workspace_id) # raises NotFoundError if missing/foreign await self._repo.delete(child_id) diff --git a/backend/app/services/day_override_service.py b/backend/app/services/day_override_service.py index 0c75bbf..39421e0 100644 --- a/backend/app/services/day_override_service.py +++ b/backend/app/services/day_override_service.py @@ -49,16 +49,17 @@ class DayOverrideService: ) return await self._repo.create(entity) - async def get(self, day_override_id: UUID) -> DayOverride: + async def get(self, day_override_id: UUID, workspace_id: UUID | None = None) -> DayOverride: entity = await self._repo.get_by_id(day_override_id) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"DayOverride {day_override_id} not found") return entity async def list_for_workspace(self, workspace_id: UUID) -> list[DayOverride]: return await self._repo.list_by_workspace(workspace_id) - async def delete(self, day_override_id: UUID) -> None: + async def delete(self, day_override_id: UUID, workspace_id: UUID | None = None) -> None: + await self.get(day_override_id, workspace_id) # raises NotFoundError if missing/foreign await self._repo.delete(day_override_id) async def get_effective_map(self, workspace_id: UUID) -> dict[date, bool]: diff --git a/backend/app/services/leave_wish_service.py b/backend/app/services/leave_wish_service.py index 919f02b..ebfbebd 100644 --- a/backend/app/services/leave_wish_service.py +++ b/backend/app/services/leave_wish_service.py @@ -89,9 +89,15 @@ class LeaveWishService: ) return await self._repo.create(entity) - async def get(self, leave_wish_id: UUID) -> LeaveWish: + async def get(self, leave_wish_id: UUID, workspace_id: UUID | None = None) -> LeaveWish: + """`workspace_id`, when given, must match the entity's own — callers + that already know the caller's tenant (routers) pass it so a foreign + workspace's wish 404s exactly like a nonexistent one; internal + callers that don't need that check (e.g. `update`'s own re-reads + with no tenant context) leave it `None`. + """ entity = await self._repo.get_by_id(leave_wish_id) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"LeaveWish {leave_wish_id} not found") return entity @@ -101,6 +107,7 @@ class LeaveWishService: async def update( self, leave_wish_id: UUID, + workspace_id: UUID | None = None, child_id: UUID | None = None, name: str | None = None, name_message_key: str | None = None, @@ -138,7 +145,7 @@ class LeaveWishService: `_clear_target_days` resets a day-granular wish back to being sized by `target_weeks` alone. """ - entity = await self.get(leave_wish_id) + entity = await self.get(leave_wish_id, workspace_id) original_name = entity.name if _clear_child_id: @@ -202,7 +209,7 @@ class LeaveWishService: ) return await self._repo.update(leave_wish_id, entity) - async def delete(self, leave_wish_id: UUID) -> None: + async def delete(self, leave_wish_id: UUID, workspace_id: UUID | None = None) -> None: """Delete a wish and every `planned_leave_day` row it produced. Under the calendar-direct-manipulation UX a period *is* its wish: the @@ -224,7 +231,7 @@ class LeaveWishService: deletes every day in the band regardless of `is_pinned` with no special-casing. """ - await self.get(leave_wish_id) # raises NotFoundError if missing + await self.get(leave_wish_id, workspace_id) # raises NotFoundError if missing/foreign await self._planned_leave_repo.delete_by_wish(leave_wish_id) await self._repo.delete(leave_wish_id) diff --git a/backend/app/services/personal_data_service.py b/backend/app/services/personal_data_service.py index 7a07da7..a10110a 100644 --- a/backend/app/services/personal_data_service.py +++ b/backend/app/services/personal_data_service.py @@ -21,9 +21,9 @@ class PersonalDataService: def __init__(self, repo: PersonalDataRepository) -> None: self._repo = repo - async def get(self, personal_data_uuid: UUID) -> PersonalData: + async def get(self, personal_data_uuid: UUID, workspace_id: UUID | None = None) -> PersonalData: entity = await self._repo.get_by_id(personal_data_uuid) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"PersonalData {personal_data_uuid} not found") return entity @@ -33,9 +33,10 @@ class PersonalDataService: salary_type: str | None = None, employment_status: str | None = None, min_vacation_days_per_year: int | None = None, + workspace_id: UUID | None = None, ) -> PersonalData: """Partial update. Raises ValidationError/NotFoundError.""" - entity = await self.get(personal_data_uuid) + entity = await self.get(personal_data_uuid, workspace_id) if salary_type is not None: entity.salary_type = salary_type diff --git a/backend/app/services/planning_service.py b/backend/app/services/planning_service.py index 943e5a7..52d6a81 100644 --- a/backend/app/services/planning_service.py +++ b/backend/app/services/planning_service.py @@ -10,6 +10,7 @@ from app.config import settings from app.entities.planned_leave import PlannedLeaveDay from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.repositories.planned_leave_repository import PlannedLeaveRepository +from app.repositories.unit_of_work import NoOpUnitOfWork, UnitOfWork from app.repositories.vacation_days_repository import VacationDaysRepository from app.services.allocation_service import PlanningEngineService, PlanProposal from app.services.analytics_service import AnalyticsService @@ -140,6 +141,7 @@ class PlanningService: parental_leave_repo: ParentalLeaveRepository, vacation_repo: VacationDaysRepository, planning_engine_service: PlanningEngineService | None = None, + uow: UnitOfWork | None = None, ) -> None: self._repo = repo self._analytics = analytics_service @@ -150,6 +152,11 @@ class PlanningService: # ever use the per-day CRUD/`apply_proposed_days` paths don't need to # wire up a `PlanningEngineService` they'll never use. self._engine = planning_engine_service + # Defaults to a no-op so pre-existing direct-construction call sites + # (scripts/seed_demo.py, test fixtures) keep today's per-write-commit + # behavior; `app.dependencies.get_planning_service` always wires a + # real `SQLiteUnitOfWork`. + self._uow = uow or NoOpUnitOfWork() async def add_planned_day( self, @@ -200,11 +207,13 @@ class PlanningService: ) return await self._repo.create(entity) - async def remove_planned_day(self, planned_leave_day_id: UUID) -> None: + async def remove_planned_day( + self, planned_leave_day_id: UUID, workspace_id: UUID | None = None + ) -> None: """Remove a planned leave day.""" - entity = await self._repo.get_by_id(planned_leave_day_id) - if entity is None: - raise NotFoundError(f"PlannedLeaveDay {planned_leave_day_id} not found") + await self.get( + planned_leave_day_id, workspace_id + ) # raises NotFoundError if missing/foreign await self._repo.delete(planned_leave_day_id) async def list_by_workspace(self, workspace_id: UUID) -> list[PlannedLeaveDay]: @@ -226,10 +235,14 @@ class PlanningService: to_date=end_date, ) - async def get(self, planned_leave_day_id: UUID) -> PlannedLeaveDay: - """Get a planned day by ID.""" + async def get( + self, planned_leave_day_id: UUID, workspace_id: UUID | None = None + ) -> PlannedLeaveDay: + """Get a planned day by ID. `workspace_id`, when given, must match the + row's own — a foreign workspace's row 404s exactly like a nonexistent + one.""" entity = await self._repo.get_by_id(planned_leave_day_id) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"PlannedLeaveDay {planned_leave_day_id} not found") return entity @@ -244,11 +257,10 @@ class PlanningService: is_dubbeldag: bool | None = None, wish_id: UUID | None = None, is_pinned: bool | None = None, + workspace_id: UUID | None = None, ) -> PlannedLeaveDay: """Update a planned day (partial update).""" - entity = await self._repo.get_by_id(planned_leave_day_id) - if entity is None: - raise NotFoundError(f"PlannedLeaveDay {planned_leave_day_id} not found") + entity = await self.get(planned_leave_day_id, workspace_id) if day_type is not None: entity.day_type = day_type @@ -314,10 +326,16 @@ class PlanningService: Validates the entire batch up front (day types, per-day dubbeldagar window, batch-wide dubbeldagar quota, child eligibility window, remaining sjukpenning/lagsta balance, credited for anything in - `delete_keys`) and only writes if everything passes — SQLite gives us - no cross-request transaction here, so this explicit validate-then- - write split is what keeps a rejection from ever leaving partial - writes behind. + `delete_keys`) and only writes if everything passes. Stages 2-4 below + (the classification read through the final write) run inside one + `self._uow()` unit of work — a `BEGIN IMMEDIATE` transaction on the + shared connection, serialized against any other unit of work via a + per-connection lock (see `app.db.connection.unit_of_work`). That lock + + transaction is what actually keeps a rejection, a crash mid-batch, + or a concurrent apply/sync on the same workspace from ever leaving + partial writes behind; the up-front validate-then-write split alone + only ever protected against a *rejected* batch writing anything, not + a crashed or interleaved one. """ delete_keys = delete_keys or set() @@ -379,149 +397,161 @@ class PlanningService: if not unique_days and not delete_keys: return PlanApplyResult(accepted=True, skipped_free_count=skipped_free_count) - # -- stage 2: classify against existing rows (collision detection) -- - by_child: dict[UUID, list[date]] = {} - for d in unique_days: - by_child.setdefault(d.child_id, []).append(d.date) - for child_id, _parent_uuid, day in delete_keys: - by_child.setdefault(child_id, []).append(day) + # Stages 2-4 run inside one unit of work: a concurrent apply/sync for + # this same workspace blocks at `BEGIN IMMEDIATE` (inside `self._uow()`) + # until this call commits or rolls back, then re-reads `existing_by_key` + # against the now-committed state instead of racing this call's reads. + async with self._uow(): + # -- stage 2: classify against existing rows (collision detection) -- + by_child: dict[UUID, list[date]] = {} + for d in unique_days: + by_child.setdefault(d.child_id, []).append(d.date) + for child_id, _parent_uuid, day in delete_keys: + by_child.setdefault(child_id, []).append(day) - existing_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = {} - for child_id, dates in by_child.items(): - existing_rows = await self._repo.list_by_workspace_child( - workspace_id=workspace_id, - child_id=child_id, - from_date=min(dates), - to_date=max(dates), - ) - for row in existing_rows: - existing_by_key[(row.child_id, row.parent_personal_data_uuid, row.date)] = row - - # Rows named by `delete_keys` that actually exist — a key with no - # matching row (already gone) is silently ignored. - delete_rows: list[PlannedLeaveDay] = [ - existing_by_key[key] for key in delete_keys if key in existing_by_key - ] - - to_create: list[ProposedDayInput] = [] - to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] - to_update_metadata: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] - collisions: list[DayCollision] = [] - skipped_identical_count = 0 - - for d in unique_days: - key = (d.child_id, d.parent_personal_data_uuid, d.date) - existing = existing_by_key.get(key) - if existing is None: - to_create.append(d) - continue - if existing.day_type == d.day_type and existing.is_dubbeldag == d.is_dubbeldag: - skipped_identical_count += 1 - # Identical-vs-collision is (and stays) keyed on - # day_type/is_dubbeldag only. A row whose only difference is - # `wish_id`/`is_pinned` is never a collision requiring - # `replace=True` — it's a plain metadata update, written - # below without touching `skipped_identical_count` or - # `collisions`. - if existing.wish_id != d.wish_id or existing.is_pinned != d.is_pinned: - to_update_metadata.append((existing, d)) - continue - collisions.append( - DayCollision( - date=d.date, - parent_personal_data_uuid=d.parent_personal_data_uuid, - child_id=d.child_id, - existing_day_type=existing.day_type, - proposed_day_type=d.day_type, + existing_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = {} + for child_id, dates in by_child.items(): + existing_rows = await self._repo.list_by_workspace_child( + workspace_id=workspace_id, + child_id=child_id, + from_date=min(dates), + to_date=max(dates), ) - ) - if replace: - to_replace.append((existing, d)) + for row in existing_rows: + existing_by_key[(row.child_id, row.parent_personal_data_uuid, row.date)] = row - if collisions and not replace: - return PlanApplyResult( - accepted=False, - collisions=collisions, - rejections=[ - Rejection( - message_key="planApply.rejection.collisionsNeedReplace", - data={"count": len(collisions)}, + # Rows named by `delete_keys` that actually exist — a key with no + # matching row (already gone) is silently ignored. + delete_rows: list[PlannedLeaveDay] = [ + existing_by_key[key] for key in delete_keys if key in existing_by_key + ] + + to_create: list[ProposedDayInput] = [] + to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] + to_update_metadata: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] + collisions: list[DayCollision] = [] + skipped_identical_count = 0 + + for d in unique_days: + key = (d.child_id, d.parent_personal_data_uuid, d.date) + existing = existing_by_key.get(key) + if existing is None: + to_create.append(d) + continue + if existing.day_type == d.day_type and existing.is_dubbeldag == d.is_dubbeldag: + skipped_identical_count += 1 + # Identical-vs-collision is (and stays) keyed on + # day_type/is_dubbeldag only. A row whose only difference is + # `wish_id`/`is_pinned` is never a collision requiring + # `replace=True` — it's a plain metadata update, written + # below without touching `skipped_identical_count` or + # `collisions`. + if existing.wish_id != d.wish_id or existing.is_pinned != d.is_pinned: + to_update_metadata.append((existing, d)) + continue + collisions.append( + DayCollision( + date=d.date, + parent_personal_data_uuid=d.parent_personal_data_uuid, + child_id=d.child_id, + existing_day_type=existing.day_type, + proposed_day_type=d.day_type, ) - ], + ) + if replace: + to_replace.append((existing, d)) + + if collisions and not replace: + return PlanApplyResult( + accepted=False, + collisions=collisions, + rejections=[ + Rejection( + message_key="planApply.rejection.collisionsNeedReplace", + data={"count": len(collisions)}, + ) + ], + ) + + pending = to_create + [new for _, new in to_replace] + if not pending and not to_update_metadata and not delete_rows: + # Nothing left to write — either an empty batch or a fully + # idempotent re-apply. + return PlanApplyResult( + accepted=True, + skipped_identical_count=skipped_identical_count, + skipped_free_count=skipped_free_count, + ) + + # -- stage 3: domain validation on the batch that would actually write + # + # `delete_rows` is threaded into both checks below so a row this same + # call is about to delete is credited back to its tier/quota BEFORE + # the new demand is checked against what's left — the bug this + # `delete_keys` parameter exists to fix was validating against a + # pre-write snapshot that hadn't yet had that capacity freed. + domain_errors: list[str] = [] + domain_errors.extend( + await self._validate_dubbeldagar_batch(pending, to_replace, delete_rows) + ) + domain_errors.extend(await self._validate_eligible_range(pending)) + domain_errors.extend( + await self._validate_remaining_balance(pending, to_replace, delete_rows) + ) + domain_errors.extend( + await self._validate_remaining_vacation_balance(pending, to_replace, delete_rows) ) - pending = to_create + [new for _, new in to_replace] - if not pending and not to_update_metadata and not delete_rows: - # Nothing left to write — either an empty batch or a fully - # idempotent re-apply. + if domain_errors: + return PlanApplyResult( + accepted=False, collisions=collisions, rejections=domain_errors + ) + + # -- stage 4: write, batched (H1: one executemany per operation + # instead of one commit per row) -------------------------------- + delete_ids = [old.id for old, _ in to_replace] + [row.id for row in delete_rows] + if delete_ids: + await self._repo.delete_many(delete_ids) + + now = datetime.now() + to_insert: list[PlannedLeaveDay] = [ + PlannedLeaveDay( + id=uuid4(), + workspace_id=workspace_id, + child_id=d.child_id, + parent_personal_data_uuid=d.parent_personal_data_uuid, + date=d.date, + day_type=d.day_type, + is_dubbeldag=d.is_dubbeldag, + wish_id=d.wish_id, + is_pinned=d.is_pinned, + created_at=now, + updated_at=now, + ) + for d in pending + ] + if to_insert: + await self._repo.create_many(to_insert) + created = to_insert + + if to_update_metadata: + metadata_pairs: list[tuple[UUID, UUID | None, bool]] = [] + for existing, d in to_update_metadata: + existing.wish_id = d.wish_id + existing.is_pinned = d.is_pinned + metadata_pairs.append((existing.id, d.wish_id, d.is_pinned)) + await self._repo.update_metadata_many(metadata_pairs) + return PlanApplyResult( accepted=True, + created=created, + created_count=len(to_create), skipped_identical_count=skipped_identical_count, skipped_free_count=skipped_free_count, + replaced_count=len(to_replace), + deleted_count=len(delete_rows), ) - # -- stage 3: domain validation on the batch that would actually write - # - # `delete_rows` is threaded into both checks below so a row this same - # call is about to delete is credited back to its tier/quota BEFORE - # the new demand is checked against what's left — the bug this - # `delete_keys` parameter exists to fix was validating against a - # pre-write snapshot that hadn't yet had that capacity freed. - domain_errors: list[str] = [] - domain_errors.extend( - await self._validate_dubbeldagar_batch(pending, to_replace, delete_rows) - ) - domain_errors.extend(await self._validate_eligible_range(pending)) - domain_errors.extend( - await self._validate_remaining_balance(pending, to_replace, delete_rows) - ) - domain_errors.extend( - await self._validate_remaining_vacation_balance(pending, to_replace, delete_rows) - ) - - if domain_errors: - return PlanApplyResult(accepted=False, collisions=collisions, rejections=domain_errors) - - # -- stage 4: write -------------------------------------------------- - for old, _ in to_replace: - await self._repo.delete(old.id) - - for row in delete_rows: - await self._repo.delete(row.id) - - now = datetime.now() - created: list[PlannedLeaveDay] = [] - for d in pending: - entity = PlannedLeaveDay( - id=uuid4(), - workspace_id=workspace_id, - child_id=d.child_id, - parent_personal_data_uuid=d.parent_personal_data_uuid, - date=d.date, - day_type=d.day_type, - is_dubbeldag=d.is_dubbeldag, - wish_id=d.wish_id, - is_pinned=d.is_pinned, - created_at=now, - updated_at=now, - ) - created.append(await self._repo.create(entity)) - - for existing, d in to_update_metadata: - existing.wish_id = d.wish_id - existing.is_pinned = d.is_pinned - await self._repo.update(existing.id, existing) - - return PlanApplyResult( - accepted=True, - created=created, - created_count=len(to_create), - skipped_identical_count=skipped_identical_count, - skipped_free_count=skipped_free_count, - replaced_count=len(to_replace), - deleted_count=len(delete_rows), - ) - # -- sync (POST /api/plan/sync) --------------------------------------- # # Recomputes and persists a workspace's plan in one step, then reports @@ -561,126 +591,135 @@ class PlanningService: if self._engine is None: raise ValidationError("PlanningService.sync_plan requires a PlanningEngineService") - before = await self._repo.list(workspace_id=workspace_id) - before_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = { - (d.child_id, d.parent_personal_data_uuid, d.date): d for d in before - } - pinned_keys = {key for key, d in before_by_key.items() if d.is_pinned} + # One unit of work spans the pre-write snapshot through the post-write + # re-read, not just `apply_proposed_days`'s own writes — otherwise a + # concurrent sync could commit between this call's `before` read and + # its (nested, no-op-passthrough — see `unit_of_work`'s docstring) + # write, and this call's `changes` diff would be computed against a + # snapshot that was already stale by the time it read `before`. + async with self._uow(): + before = await self._repo.list(workspace_id=workspace_id) + before_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = { + (d.child_id, d.parent_personal_data_uuid, d.date): d for d in before + } + pinned_keys = {key for key, d in before_by_key.items() if d.is_pinned} - proposal = await self._engine.propose(workspace_id, today=today, only_pinned_existing=True) - - batch = [ - ProposedDayInput( - date=d.date, - parent_personal_data_uuid=d.parent_personal_data_uuid, - child_id=d.child_id, - day_type=d.day_type, - is_dubbeldag=d.is_dubbeldag, - wish_id=d.wish_id, - is_pinned=False, + proposal = await self._engine.propose( + workspace_id, today=today, only_pinned_existing=True ) - for d in proposal.days - if d.day_type != "free" - and d.child_id is not None - # Defense-in-depth only: the allocator never proposes anything - # for an already-occupied key (pinned rows stay in - # `existing_planned` — see `only_pinned_existing` above), so this - # should never actually filter anything out. - and (d.child_id, d.parent_personal_data_uuid, d.date) not in pinned_keys - ] - batch_keys = {(d.child_id, d.parent_personal_data_uuid, d.date) for d in batch} - # Rows the freshly-computed plan no longer accounts for at all: - # unpinned, wish-attributed, and absent from this run's batch — e.g. - # the wish was deleted, or its window moved elsewhere. Passed as - # `delete_keys` so `apply_proposed_days` validates the deletions - # together with the creates/replaces (see the comment above this - # method) rather than deleting them itself afterwards. - stale_keys = { - key - for key, row in before_by_key.items() - if not row.is_pinned and row.wish_id is not None and key not in batch_keys - } + batch = [ + ProposedDayInput( + date=d.date, + parent_personal_data_uuid=d.parent_personal_data_uuid, + child_id=d.child_id, + day_type=d.day_type, + is_dubbeldag=d.is_dubbeldag, + wish_id=d.wish_id, + is_pinned=False, + ) + for d in proposal.days + if d.day_type != "free" + and d.child_id is not None + # Defense-in-depth only: the allocator never proposes anything + # for an already-occupied key (pinned rows stay in + # `existing_planned` — see `only_pinned_existing` above), so this + # should never actually filter anything out. + and (d.child_id, d.parent_personal_data_uuid, d.date) not in pinned_keys + ] - apply_result = await self.apply_proposed_days( - workspace_id=workspace_id, days=batch, replace=True, delete_keys=stale_keys - ) - - changes: list[PlanChange] = [] - if apply_result.accepted: - after = await self._repo.list(workspace_id=workspace_id) - after_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = { - (d.child_id, d.parent_personal_data_uuid, d.date): d for d in after + batch_keys = {(d.child_id, d.parent_personal_data_uuid, d.date) for d in batch} + # Rows the freshly-computed plan no longer accounts for at all: + # unpinned, wish-attributed, and absent from this run's batch — e.g. + # the wish was deleted, or its window moved elsewhere. Passed as + # `delete_keys` so `apply_proposed_days` validates the deletions + # together with the creates/replaces (see the comment above this + # method) rather than deleting them itself afterwards. + stale_keys = { + key + for key, row in before_by_key.items() + if not row.is_pinned and row.wish_id is not None and key not in batch_keys } - for key in sorted( - set(before_by_key) | set(after_by_key), - key=lambda k: (k[2], str(k[0]), str(k[1])), - ): - child_id, parent_uuid, d = key - old = before_by_key.get(key) - new = after_by_key.get(key) - if old is None and new is not None: - changes.append( - PlanChange( - kind="added", - date=d, - parent_personal_data_uuid=parent_uuid, - child_id=child_id, - old_day_type=None, - new_day_type=new.day_type, - old_is_dubbeldag=None, - new_is_dubbeldag=new.is_dubbeldag, - old_wish_id=None, - new_wish_id=new.wish_id, - ) - ) - elif old is not None and new is None: - changes.append( - PlanChange( - kind="removed", - date=d, - parent_personal_data_uuid=parent_uuid, - child_id=child_id, - old_day_type=old.day_type, - new_day_type=None, - old_is_dubbeldag=old.is_dubbeldag, - new_is_dubbeldag=None, - old_wish_id=old.wish_id, - new_wish_id=None, - ) - ) - elif old is not None and new is not None: - if old.day_type != new.day_type or old.is_dubbeldag != new.is_dubbeldag: + apply_result = await self.apply_proposed_days( + workspace_id=workspace_id, days=batch, replace=True, delete_keys=stale_keys + ) + + changes: list[PlanChange] = [] + if apply_result.accepted: + after = await self._repo.list(workspace_id=workspace_id) + after_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = { + (d.child_id, d.parent_personal_data_uuid, d.date): d for d in after + } + + for key in sorted( + set(before_by_key) | set(after_by_key), + key=lambda k: (k[2], str(k[0]), str(k[1])), + ): + child_id, parent_uuid, d = key + old = before_by_key.get(key) + new = after_by_key.get(key) + if old is None and new is not None: changes.append( PlanChange( - kind="retyped", + kind="added", + date=d, + parent_personal_data_uuid=parent_uuid, + child_id=child_id, + old_day_type=None, + new_day_type=new.day_type, + old_is_dubbeldag=None, + new_is_dubbeldag=new.is_dubbeldag, + old_wish_id=None, + new_wish_id=new.wish_id, + ) + ) + elif old is not None and new is None: + changes.append( + PlanChange( + kind="removed", date=d, parent_personal_data_uuid=parent_uuid, child_id=child_id, old_day_type=old.day_type, - new_day_type=new.day_type, + new_day_type=None, old_is_dubbeldag=old.is_dubbeldag, - new_is_dubbeldag=new.is_dubbeldag, + new_is_dubbeldag=None, old_wish_id=old.wish_id, - new_wish_id=new.wish_id, + new_wish_id=None, ) ) - elif old.wish_id != new.wish_id: - changes.append( - PlanChange( - kind="reattributed", - date=d, - parent_personal_data_uuid=parent_uuid, - child_id=child_id, - old_day_type=old.day_type, - new_day_type=new.day_type, - old_is_dubbeldag=old.is_dubbeldag, - new_is_dubbeldag=new.is_dubbeldag, - old_wish_id=old.wish_id, - new_wish_id=new.wish_id, + elif old is not None and new is not None: + if old.day_type != new.day_type or old.is_dubbeldag != new.is_dubbeldag: + changes.append( + PlanChange( + kind="retyped", + date=d, + parent_personal_data_uuid=parent_uuid, + child_id=child_id, + old_day_type=old.day_type, + new_day_type=new.day_type, + old_is_dubbeldag=old.is_dubbeldag, + new_is_dubbeldag=new.is_dubbeldag, + old_wish_id=old.wish_id, + new_wish_id=new.wish_id, + ) + ) + elif old.wish_id != new.wish_id: + changes.append( + PlanChange( + kind="reattributed", + date=d, + parent_personal_data_uuid=parent_uuid, + child_id=child_id, + old_day_type=old.day_type, + new_day_type=new.day_type, + old_is_dubbeldag=old.is_dubbeldag, + new_is_dubbeldag=new.is_dubbeldag, + old_wish_id=old.wish_id, + new_wish_id=new.wish_id, + ) ) - ) return PlanSyncResult(proposal=proposal, apply_result=apply_result, changes=changes) diff --git a/backend/app/services/vacation_days_service.py b/backend/app/services/vacation_days_service.py index 1b05510..3cf7ac1 100644 --- a/backend/app/services/vacation_days_service.py +++ b/backend/app/services/vacation_days_service.py @@ -51,9 +51,9 @@ class VacationDaysService: ) return await self._repo.create(entity) - async def get(self, vacation_days_id: UUID) -> VacationDays: + async def get(self, vacation_days_id: UUID, workspace_id: UUID | None = None) -> VacationDays: entity = await self._repo.get_by_id(vacation_days_id) - if entity is None: + if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id): raise NotFoundError(f"VacationDays {vacation_days_id} not found") return entity diff --git a/backend/app/services/workspace_service.py b/backend/app/services/workspace_service.py index 2d820cf..d1666a8 100644 --- a/backend/app/services/workspace_service.py +++ b/backend/app/services/workspace_service.py @@ -21,6 +21,7 @@ from app.repositories.leave_wish_repository import LeaveWishRepository from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.repositories.personal_data_repository import PersonalDataRepository from app.repositories.planned_leave_repository import PlannedLeaveRepository +from app.repositories.unit_of_work import NoOpUnitOfWork, UnitOfWork from app.repositories.user_repository import UserRepository from app.repositories.vacation_days_repository import VacationDaysRepository from app.repositories.workspace_repository import ( @@ -43,6 +44,7 @@ class WorkspaceService: planned_leave_repo: PlannedLeaveRepository, parental_leave_repo: ParentalLeaveRepository, vacation_days_repo: VacationDaysRepository, + uow: UnitOfWork | None = None, ) -> None: self._ws = ws_repo self._inv = inv_repo @@ -52,6 +54,11 @@ class WorkspaceService: self._planned_leave = planned_leave_repo self._parental_leave = parental_leave_repo self._vacation_days = vacation_days_repo + # Defaults to a no-op so pre-existing direct-construction call sites + # (scripts/seed_demo.py, test fixtures) keep today's per-write-commit + # behavior; `app.dependencies.get_workspace_service` always wires a + # real `SQLiteUnitOfWork`. + self._uow = uow or NoOpUnitOfWork() # ------------------------------------------------------------------ # Workspace CRUD @@ -264,16 +271,30 @@ class WorkspaceService: pd = await self._pd.get_by_id(personal_data_uuid) if pd is None: raise NotFoundError(f"PersonalData {personal_data_uuid} not found") - pd.workspace_id = inv.workspace_id - await self._pd.update(pd.uuid, pd) - inv.status = "accepted" - inv.accepted_at = datetime.now() - return await self._inv.update(inv.id, inv) + if pd.workspace_id == inv.workspace_id: + raise InvitationError("You already belong to this workspace") - async def revoke_invitation(self, invitation_id: UUID) -> None: + existing_members = await self._pd.list_by_workspace(inv.workspace_id) + if len(existing_members) >= 2: + raise InvitationError("This workspace already has its maximum number of members") + + # Both writes must land together: a crash between them would leave + # the redeemer either moved to the new workspace with a still-pending + # (re-usable) invitation, or stuck in their old workspace with an + # invitation marked accepted. + async with self._uow(): + pd.workspace_id = inv.workspace_id + await self._pd.update(pd.uuid, pd) + + inv.status = "accepted" + inv.accepted_at = datetime.now() + inv = await self._inv.update(inv.id, inv) + return inv + + async def revoke_invitation(self, invitation_id: UUID, workspace_id: UUID) -> None: inv = await self._inv.get_by_id(invitation_id) - if inv is None: + if inv is None or inv.workspace_id != workspace_id: raise NotFoundError(f"Invitation {invitation_id} not found") if inv.status != "pending": raise ValidationError(f"Cannot revoke a {inv.status} invitation") diff --git a/backend/migrations/versions/011_add_indexes.py b/backend/migrations/versions/011_add_indexes.py new file mode 100644 index 0000000..67db585 --- /dev/null +++ b/backend/migrations/versions/011_add_indexes.py @@ -0,0 +1,99 @@ +"""Add indexes on hot filter/FK-cascade columns, and user.is_active. + +Backend-perf audit (`scratchpad/audit/backend-perf.md` §4, M1): every +migration to date created zero indexes; `EXPLAIN QUERY PLAN` against the full +schema shows several hot paths doing a full table SCAN once the DB carries +more than a handful of rows per table (multi-workspace deployment, years of +FK-fetch history) — `planned_leave_day.count_by_parent_type` (2x per +parent+child in analytics and plan/apply), `count_dubbeldagar`, +`parental_leave_fetch.get_latest_by_parent`, and every FK-cascade column that +`ON DELETE CASCADE` walks on a GDPR delete. These 14 indexes cover exactly +those columns; see the audit for the full per-index rationale. Every +`CREATE INDEX` below is mirrored verbatim (same columns, same order, no +partial indexes) in `_FULL_SCHEMA` (`app/db/connection.py`) so +`tests/integration/test_schema_parity.py` — which compares index sets between +the Alembic chain and that hand-maintained DDL — stays green. + +Also adds `user.is_active` (default 1): `AuthService.login` and +`get_user_from_token` already check `User.is_active` (account +deactivation), but the column never existed — every account was +unconditionally treated as active. This migration adds it and +`SQLiteUserRepository` now reads/writes it. + +Revision ID: 011 +Revises: 010 +Create Date: 2026-09-18 +""" + +from __future__ import annotations + +from alembic import op + +revision: str = "011" +down_revision: str | None = "010" +branch_labels: str | None = None +depends_on: str | None = None + +_UPGRADE_SQL = """ +ALTER TABLE "user" ADD COLUMN is_active INTEGER NOT NULL DEFAULT 1; +CREATE INDEX idx_pld_parent_child_type_date + ON planned_leave_day (parent_personal_data_uuid, child_id, day_type, date); +CREATE INDEX idx_pld_child_dubbeldag_date + ON planned_leave_day (child_id, is_dubbeldag, date); +CREATE INDEX idx_pld_wish + ON planned_leave_day (wish_id); +CREATE INDEX idx_plf_parent_child_fetched + ON parental_leave_fetch (personal_data_uuid, child_id, fetched_at); +CREATE INDEX idx_plf_workspace + ON parental_leave_fetch (workspace_id); +CREATE INDEX idx_plf_child + ON parental_leave_fetch (child_id); +CREATE INDEX idx_leave_wish_workspace + ON leave_wish (workspace_id); +CREATE INDEX idx_leave_wish_child + ON leave_wish (child_id); +CREATE INDEX idx_leave_wish_parent + ON leave_wish (parent_personal_data_uuid); +CREATE INDEX idx_child_workspace_birth + ON child (workspace_id, birth_date); +CREATE INDEX idx_personal_data_workspace_deleted + ON personal_data (workspace_id, deleted_at); +CREATE INDEX idx_invitation_workspace_status_created + ON workspace_invitation (workspace_id, status, created_at); +CREATE INDEX idx_invitation_invited_by + ON workspace_invitation (invited_by_user_id); +CREATE INDEX idx_vacation_days_workspace + ON vacation_days (workspace_id); +""" + +_DOWNGRADE_SQL = """ +DROP INDEX idx_vacation_days_workspace; +DROP INDEX idx_invitation_invited_by; +DROP INDEX idx_invitation_workspace_status_created; +DROP INDEX idx_personal_data_workspace_deleted; +DROP INDEX idx_child_workspace_birth; +DROP INDEX idx_leave_wish_parent; +DROP INDEX idx_leave_wish_child; +DROP INDEX idx_leave_wish_workspace; +DROP INDEX idx_plf_child; +DROP INDEX idx_plf_workspace; +DROP INDEX idx_plf_parent_child_fetched; +DROP INDEX idx_pld_wish; +DROP INDEX idx_pld_child_dubbeldag_date; +DROP INDEX idx_pld_parent_child_type_date; +ALTER TABLE "user" DROP COLUMN is_active; +""" + + +def upgrade() -> None: + from sqlalchemy import text + + for stmt in [s.strip() for s in _UPGRADE_SQL.split(";") if s.strip()]: + op.execute(text(stmt)) + + +def downgrade() -> None: + from sqlalchemy import text + + for stmt in [s.strip() for s in _DOWNGRADE_SQL.split(";") if s.strip()]: + op.execute(text(stmt)) diff --git a/backend/scripts/demo_plan.py b/backend/scripts/demo_plan.py index c318aac..43d6547 100644 --- a/backend/scripts/demo_plan.py +++ b/backend/scripts/demo_plan.py @@ -32,6 +32,7 @@ from app.services.allocation_service import ( WindowOutcome, ) from app.utils.holidays import get_swedish_holidays +from app.utils.tip_predicates import resolve_age4_savable_cap # --------------------------------------------------------------------------- # 1. Build a realistic AllocationInput, in memory, Swedish defaults. @@ -124,6 +125,7 @@ def _build_input() -> tuple[AllocationInput, dict]: today=TODAY, horizon_end=horizon_end, planning_mode="couple", + age4_savable_cap=resolve_age4_savable_cap("couple", parent_count=2), children=[child], parents=[parent_1, parent_2], pools=pools, diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index d8c67e7..63318f9 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -13,11 +13,15 @@ from uuid import UUID, uuid4 import aiosqlite import pytest_asyncio +from httpx import ASGITransport, AsyncClient from app.db.connection import ensure_schema +from app.dependencies import get_connection, get_current_user from app.entities.child import Child from app.entities.personal_data import PersonalData +from app.entities.user import User from app.entities.workspace import Workspace +from app.main import app from app.repositories_impl.sqlite.child_repository import SQLiteChildRepository from app.repositories_impl.sqlite.day_override_repository import SQLiteDayOverrideRepository from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository @@ -25,6 +29,7 @@ from app.repositories_impl.sqlite.parental_leave_repository import SQLiteParenta from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository from app.repositories_impl.sqlite.tip_dismissal_repository import SQLiteTipDismissalRepository +from app.repositories_impl.sqlite.unit_of_work import SQLiteUnitOfWork from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.repositories_impl.sqlite.vacation_days_repository import ( SQLiteVacationDaysRepository, @@ -183,6 +188,51 @@ async def seeded_ids( return SeededIds(workspace_id=ws.id, personal_data_uuid=pd.uuid, child_id=child.id) +# --------------------------------------------------------------------------- +# Shared authenticated HTTP client — bound to `seeded_ids`'s workspace. +# +# T1 authz work: every router now rejects ids outside the caller's workspace, +# so a router test's `client` must be authenticated as a real member of +# whatever workspace the test operates on. This fixture is that identity for +# any router test built around `seeded_ids` (see `tests/unit/test_fetch_data_ +# router.py`). Router tests that seed their own bespoke workspace per test +# (e.g. `test_plan_router.py`) still define their own local `client`/current- +# user fixture bound to THAT workspace instead — a single global identity +# can't satisfy both shapes. +# --------------------------------------------------------------------------- + + +@pytest_asyncio.fixture +async def current_user(seeded_ids: SeededIds) -> User: + """A real, logged-in user whose PersonalData is `seeded_ids.personal_data_uuid`.""" + return User( + id=uuid4(), + personal_data_uuid=seeded_ids.personal_data_uuid, + email="seeded-user@example.com", + hashed_password="not-a-real-hash", + created_at=datetime.now(), + ) + + +@pytest_asyncio.fixture +async def authed_client( + connection: aiosqlite.Connection, current_user: User +) -> AsyncIterator[AsyncClient]: + """An httpx client against the real app, authenticated as `current_user` + and wired to the test's in-memory `connection` — the same technique + every router test file uses, consolidated here for tests keyed off + `seeded_ids`. + """ + app.dependency_overrides[get_connection] = lambda: connection + app.dependency_overrides[get_current_user] = lambda: current_user + try: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + yield ac + finally: + app.dependency_overrides.pop(get_connection, None) + app.dependency_overrides.pop(get_current_user, None) + + # --------------------------------------------------------------------------- # Child & leave # --------------------------------------------------------------------------- @@ -313,6 +363,40 @@ async def planning_service( ) +# --------------------------------------------------------------------------- +# Unit of work — a real `SQLiteUnitOfWork` bound to the test's shared +# `connection`, for tests that specifically exercise atomicity/locking (see +# `tests/unit/test_db_unit_of_work.py`). `planning_service` above deliberately +# keeps its default no-op unit of work so every other test's per-write-commit +# behavior stays exactly as it is; use `planning_service_with_uow` instead +# when a test needs the real thing. +# --------------------------------------------------------------------------- + + +@pytest_asyncio.fixture +async def sqlite_uow(connection: aiosqlite.Connection) -> SQLiteUnitOfWork: + return SQLiteUnitOfWork(connection) + + +@pytest_asyncio.fixture +async def planning_service_with_uow( + planned_leave_repo: SQLitePlannedLeaveRepository, + analytics_service: AnalyticsService, + parental_leave_repo: SQLiteParentalLeaveRepository, + vacation_repo: SQLiteVacationDaysRepository, + planning_engine_service: PlanningEngineService, + sqlite_uow: SQLiteUnitOfWork, +) -> PlanningService: + return PlanningService( + planned_leave_repo, + analytics_service, + parental_leave_repo, + vacation_repo, + planning_engine_service, + uow=sqlite_uow, + ) + + # --------------------------------------------------------------------------- # Tips # --------------------------------------------------------------------------- diff --git a/backend/tests/fixtures/fk_overview_synthetic.html b/backend/tests/fixtures/fk_overview_synthetic.html new file mode 100644 index 0000000..5e41903 --- /dev/null +++ b/backend/tests/fixtures/fk_overview_synthetic.html @@ -0,0 +1,81 @@ + + + + + + Ansök och planera föräldrapenning - Försäkringskassan (synthetic fixture) + + +
+
+

För dig

+
+
+
+
+
+ 100 + av 240 dagar kvar efter ansökan +
+
+
+ +
+

Sjukpenningnivå (60 dagar)

+
+
+
+ +
+

Lägstanivå (40 dagar)

+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+

Totalt för Testbarn

+
+

Exempel på belopp: 1 234 kronor för 5 dagar.

+
+
+
+ +
+ +
+
+
+
+ 20 + av 60 dubbeldagar kvar efter ansökan +
+
+
+
+
+
+
+ + + + diff --git a/backend/tests/unit/test_authz.py b/backend/tests/unit/test_authz.py new file mode 100644 index 0000000..c93455c --- /dev/null +++ b/backend/tests/unit/test_authz.py @@ -0,0 +1,646 @@ +"""Cross-cutting authorization tests (T1 authz audit). + +Two things every route in the ten scoped routers must satisfy (see +`app/dependencies.py`'s `WorkspaceScope`/`WorkspaceIdDep` and the audit at +`graphify`-adjacent scratch notes for the full route table): + + 1. No `Authorization` header -> 401 (F1: `plan`, `leave_wishes`, + `day_overrides`, `vacation_days` had NO auth at all before this task). + 2. A real id belonging to a DIFFERENT workspace -> 404, never 403 (F3: IDOR + on every workspace-/child-/pd-keyed route) — a 403 would confirm the id + exists, which is itself information disclosure. + +This file exercises the real app + dependency graph (httpx ASGITransport), +following the pattern in `tests/unit/test_plan_router.py`, rather than only +the service layer — the bug class here is specifically about what the +ROUTER does (or fails to do) before calling the service. +""" + +from datetime import date, datetime, timedelta +from uuid import uuid4 + +import aiosqlite +import pytest_asyncio +from httpx import ASGITransport, AsyncClient + +from app.dependencies import get_connection, get_current_user +from app.entities.child import Child +from app.entities.day_override import DayOverride +from app.entities.leave_wish import LeaveWish +from app.entities.personal_data import PersonalData +from app.entities.planned_leave import PlannedLeaveDay +from app.entities.user import User +from app.entities.vacation_days import VacationDays +from app.entities.workspace import Workspace +from app.entities.workspace_invitation import WorkspaceInvitation +from app.main import app +from app.repositories_impl.sqlite.child_repository import SQLiteChildRepository +from app.repositories_impl.sqlite.day_override_repository import SQLiteDayOverrideRepository +from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository +from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository +from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository +from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository +from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository +from app.repositories_impl.sqlite.workspace_repository import ( + SQLiteWorkspaceInvitationRepository, + SQLiteWorkspaceRepository, +) + +# The caller's fixed identity across this file's authenticated tests: a +# member of "workspace A" (seeded per-test by `two_workspaces`). +_CALLER_PD_UUID = uuid4() + + +@pytest_asyncio.fixture +async def anon_client(connection: aiosqlite.Connection) -> AsyncClient: + """No `get_current_user` override at all — a real, unauthenticated + request (no Authorization header) must be rejected by the real JWT + dependency, not by a test double standing in for it. + """ + app.dependency_overrides[get_connection] = lambda: connection + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + yield ac + app.dependency_overrides.pop(get_connection, None) + + +@pytest_asyncio.fixture +async def client(connection: aiosqlite.Connection) -> AsyncClient: + """Authenticated as `_CALLER_PD_UUID` — a member of whatever workspace + the test seeds for them (see `two_workspaces`). + """ + app.dependency_overrides[get_connection] = lambda: connection + app.dependency_overrides[get_current_user] = lambda: User( + id=uuid4(), + personal_data_uuid=_CALLER_PD_UUID, + email="caller@example.com", + hashed_password="not-a-real-hash", + created_at=datetime.now(), + ) + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + yield ac + app.dependency_overrides.pop(get_connection, None) + app.dependency_overrides.pop(get_current_user, None) + + +class _TwoWorkspaces: + def __init__( + self, + ws_a: Workspace, + ws_b: Workspace, + child_a: Child, + child_b: Child, + pd_b: PersonalData, + ) -> None: + self.ws_a = ws_a + self.ws_b = ws_b + self.child_a = child_a + self.child_b = child_b + self.pd_b = pd_b + + +@pytest_asyncio.fixture +async def two_workspaces(connection: aiosqlite.Connection) -> _TwoWorkspaces: + """Seed workspace A (the caller's own) and workspace B (a foreign + tenant), each with its own child and member. + """ + now = datetime.now() + ws_repo = SQLiteWorkspaceRepository(connection) + pd_repo = SQLitePersonalDataRepository(connection) + child_repo = SQLiteChildRepository(connection) + + ws_a = await ws_repo.create(Workspace(id=uuid4(), name="Workspace A", created_at=now)) + await pd_repo.create( + PersonalData(uuid=_CALLER_PD_UUID, workspace_id=ws_a.id, name="Caller", created_at=now) + ) + child_a = await child_repo.create( + Child(id=uuid4(), workspace_id=ws_a.id, name="Child A", birth_date=date(2025, 1, 1)) + ) + + ws_b = await ws_repo.create(Workspace(id=uuid4(), name="Workspace B", created_at=now)) + pd_b = await pd_repo.create( + PersonalData(uuid=uuid4(), workspace_id=ws_b.id, name="Victim", created_at=now) + ) + child_b = await child_repo.create( + Child(id=uuid4(), workspace_id=ws_b.id, name="Child B", birth_date=date(2025, 1, 1)) + ) + + return _TwoWorkspaces(ws_a, ws_b, child_a, child_b, pd_b) + + +# --------------------------------------------------------------------------- +# F1 — routes that had NO authentication at all before this task. +# --------------------------------------------------------------------------- + + +class TestNoTokenIsUnauthenticated: + async def test_plan_propose_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post("/api/plan/propose", json={"workspace_id": str(uuid4())}) + assert resp.status_code == 401 + + async def test_plan_apply_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post( + "/api/plan/apply", json={"workspace_id": str(uuid4()), "days": []} + ) + assert resp.status_code == 401 + + async def test_plan_sync_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post("/api/plan/sync", json={"workspace_id": str(uuid4())}) + assert resp.status_code == 401 + + async def test_leave_wishes_create_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post( + "/api/leave-wishes", + json={ + "workspace_id": str(uuid4()), + "name": "X", + "window_start_month": 1, + "window_start_day": 1, + "window_end_month": 1, + "window_end_day": 7, + "target_weeks": 1, + "first_year": 2030, + }, + ) + assert resp.status_code == 401 + + async def test_leave_wishes_list_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.get(f"/api/leave-wishes/{uuid4()}") + assert resp.status_code == 401 + + async def test_day_overrides_create_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post( + "/api/day-overrides", + json={"workspace_id": str(uuid4()), "date": "2030-01-01", "is_working_day": False}, + ) + assert resp.status_code == 401 + + async def test_day_overrides_list_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.get(f"/api/day-overrides/workspace/{uuid4()}") + assert resp.status_code == 401 + + async def test_vacation_days_create_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.post( + "/api/vacation-days", + json={ + "personal_data_uuid": str(uuid4()), + "workspace_id": str(uuid4()), + "year": 2030, + "total_days": 25, + }, + ) + assert resp.status_code == 401 + + async def test_vacation_days_list_requires_auth(self, anon_client: AsyncClient) -> None: + resp = await anon_client.get(f"/api/vacation-days/workspace/{uuid4()}") + assert resp.status_code == 401 + + +# --------------------------------------------------------------------------- +# F3 — a real id from a DIFFERENT workspace must 404, never 403. +# --------------------------------------------------------------------------- + + +class TestForeignWorkspaceIdIs404: + async def test_workspace_get(self, client: AsyncClient, two_workspaces: _TwoWorkspaces) -> None: + resp = await client.get(f"/api/workspace/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_workspace_members( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/workspace/{two_workspaces.ws_b.id}/members") + assert resp.status_code == 404 + + async def test_tips_get(self, client: AsyncClient, two_workspaces: _TwoWorkspaces) -> None: + resp = await client.get(f"/api/tips/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_children_by_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/children/workspace/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_children_get_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/children/{two_workspaces.child_b.id}") + assert resp.status_code == 404 + + async def test_settings_update_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.put( + f"/api/settings/child/{two_workspaces.child_b.id}", json={"name": "Hijacked"} + ) + assert resp.status_code == 404 + + async def test_settings_delete_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.delete(f"/api/settings/{two_workspaces.child_b.id}") + assert resp.status_code == 404 + + async def test_settings_update_foreign_parent( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.put( + f"/api/settings/parent/{two_workspaces.pd_b.uuid}", + json={"employment_status": "none"}, + ) + assert resp.status_code == 404 + + async def test_calendar_days_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get( + "/api/calendar/days", + params={"workspace_id": str(two_workspaces.ws_b.id), "start_year": 2030}, + ) + assert resp.status_code == 404 + + async def test_analytics_overview_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get( + f"/api/analytics/{two_workspaces.ws_b.id}/overview", + params={"child_id": str(two_workspaces.child_b.id)}, + ) + assert resp.status_code == 404 + + async def test_analytics_age4_foreign_parent( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get( + f"/api/analytics/parent/{two_workspaces.pd_b.uuid}" + f"/child/{two_workspaces.child_b.id}/age4" + ) + assert resp.status_code == 404 + + async def test_plan_propose_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/plan/propose", json={"workspace_id": str(two_workspaces.ws_b.id)} + ) + assert resp.status_code == 404 + + async def test_plan_apply_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/plan/apply", + json={ + "workspace_id": str(two_workspaces.ws_a.id), + "days": [ + { + "date": "2030-06-01", + "parent_personal_data_uuid": str(_CALLER_PD_UUID), + "child_id": str(two_workspaces.child_b.id), + "day_type": "vacation", + } + ], + }, + ) + assert resp.status_code == 404 + + async def test_plan_sync_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/plan/sync", json={"workspace_id": str(two_workspaces.ws_b.id)} + ) + assert resp.status_code == 404 + + async def test_leave_wishes_create_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/leave-wishes", + json={ + "workspace_id": str(two_workspaces.ws_b.id), + "name": "X", + "window_start_month": 1, + "window_start_day": 1, + "window_end_month": 1, + "window_end_day": 7, + "target_weeks": 1, + "first_year": 2030, + }, + ) + assert resp.status_code == 404 + + async def test_leave_wishes_create_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/leave-wishes", + json={ + "workspace_id": str(two_workspaces.ws_a.id), + "child_id": str(two_workspaces.child_b.id), + "name": "X", + "window_start_month": 1, + "window_start_day": 1, + "window_end_month": 1, + "window_end_day": 7, + "target_weeks": 1, + "first_year": 2030, + }, + ) + assert resp.status_code == 404 + + async def test_leave_wishes_list_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/leave-wishes/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_leave_wishes_update_foreign_wish( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + wish_repo = SQLiteLeaveWishRepository(connection) + wish = await wish_repo.create( + LeaveWish( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + child_id=None, + name="Foreign", + window_start_month=1, + window_start_day=1, + window_end_month=1, + window_end_day=7, + target_weeks=1, + recurrence="once", + first_year=2030, + last_year=None, + priority=1, + parent_mode="either", + parent_personal_data_uuid=None, + ) + ) + resp = await client.put(f"/api/leave-wishes/{wish.id}", json={"name": "Hijacked"}) + assert resp.status_code == 404 + + async def test_leave_wishes_delete_foreign_wish( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + wish_repo = SQLiteLeaveWishRepository(connection) + wish = await wish_repo.create( + LeaveWish( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + child_id=None, + name="Foreign", + window_start_month=1, + window_start_day=1, + window_end_month=1, + window_end_day=7, + target_weeks=1, + recurrence="once", + first_year=2030, + last_year=None, + priority=1, + parent_mode="either", + parent_personal_data_uuid=None, + ) + ) + resp = await client.delete(f"/api/leave-wishes/{wish.id}") + assert resp.status_code == 404 + + async def test_day_overrides_create_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/day-overrides", + json={ + "workspace_id": str(two_workspaces.ws_b.id), + "date": "2030-01-01", + "is_working_day": False, + }, + ) + assert resp.status_code == 404 + + async def test_day_overrides_get_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLiteDayOverrideRepository(connection) + row = await repo.create( + DayOverride( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + date=date(2030, 1, 1), + is_working_day=False, + ) + ) + resp = await client.get(f"/api/day-overrides/{row.id}") + assert resp.status_code == 404 + + async def test_day_overrides_list_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/day-overrides/workspace/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_day_overrides_delete_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLiteDayOverrideRepository(connection) + row = await repo.create( + DayOverride( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + date=date(2030, 1, 2), + is_working_day=False, + ) + ) + resp = await client.delete(f"/api/day-overrides/{row.id}") + assert resp.status_code == 404 + + async def test_vacation_days_create_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/vacation-days", + json={ + "personal_data_uuid": str(_CALLER_PD_UUID), + "workspace_id": str(two_workspaces.ws_b.id), + "year": 2030, + "total_days": 25, + }, + ) + assert resp.status_code == 404 + + async def test_vacation_days_create_foreign_member( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/vacation-days", + json={ + "personal_data_uuid": str(two_workspaces.pd_b.uuid), + "workspace_id": str(two_workspaces.ws_a.id), + "year": 2030, + "total_days": 25, + }, + ) + assert resp.status_code == 404 + + async def test_vacation_days_get_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLiteVacationDaysRepository(connection) + row = await repo.create( + VacationDays( + id=uuid4(), + personal_data_uuid=two_workspaces.pd_b.uuid, + workspace_id=two_workspaces.ws_b.id, + year=2030, + total_days=25, + ) + ) + resp = await client.get(f"/api/vacation-days/{row.id}") + assert resp.status_code == 404 + + async def test_vacation_days_list_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/vacation-days/workspace/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_planned_days_create_foreign_child( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/planned-days", + json={ + "workspace_id": str(two_workspaces.ws_a.id), + "child_id": str(two_workspaces.child_b.id), + "parent_personal_data_uuid": str(_CALLER_PD_UUID), + "date": "2030-06-01", + "day_type": "vacation", + }, + ) + assert resp.status_code == 404 + + async def test_planned_days_list_by_child_foreign( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get( + "/api/planned-days", params={"child_id": str(two_workspaces.child_b.id)} + ) + assert resp.status_code == 404 + + async def test_planned_days_get_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLitePlannedLeaveRepository(connection) + row = await repo.create( + PlannedLeaveDay( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + child_id=two_workspaces.child_b.id, + parent_personal_data_uuid=two_workspaces.pd_b.uuid, + date=date(2030, 6, 1), + day_type="vacation", + ) + ) + resp = await client.get(f"/api/planned-days/{row.id}") + assert resp.status_code == 404 + + async def test_planned_days_by_workspace_foreign( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get(f"/api/planned-days/workspace/{two_workspaces.ws_b.id}") + assert resp.status_code == 404 + + async def test_planned_days_update_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLitePlannedLeaveRepository(connection) + row = await repo.create( + PlannedLeaveDay( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + child_id=two_workspaces.child_b.id, + parent_personal_data_uuid=two_workspaces.pd_b.uuid, + date=date(2030, 6, 2), + day_type="vacation", + ) + ) + resp = await client.put(f"/api/planned-days/{row.id}", json={"day_type": "sjukpenning"}) + assert resp.status_code == 404 + + async def test_planned_days_delete_foreign( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + repo = SQLitePlannedLeaveRepository(connection) + row = await repo.create( + PlannedLeaveDay( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + child_id=two_workspaces.child_b.id, + parent_personal_data_uuid=two_workspaces.pd_b.uuid, + date=date(2030, 6, 3), + day_type="vacation", + ) + ) + resp = await client.delete(f"/api/planned-days/{row.id}") + assert resp.status_code == 404 + + async def test_fetch_data_manual_foreign_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.post( + "/api/parental-leave/manual", + json={ + "personal_data_uuid": str(_CALLER_PD_UUID), + "workspace_id": str(two_workspaces.ws_b.id), + "child_id": str(two_workspaces.child_a.id), + "total_days": 480, + "days_used": 0, + "reserved_days_left": 90, + "transferable_days_left": 150, + "sjukpenning_used": 0, + "lagsta_niva_used": 0, + }, + ) + assert resp.status_code == 404 + + async def test_fetch_data_get_latest_foreign_pd( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + resp = await client.get( + f"/api/parental-leave/{two_workspaces.pd_b.uuid}/{two_workspaces.child_a.id}" + ) + assert resp.status_code == 404 + + async def test_invitations_revoke_foreign_workspace( + self, client: AsyncClient, connection: aiosqlite.Connection, two_workspaces: _TwoWorkspaces + ) -> None: + """The invitation belongs to workspace B; the caller (workspace A) + must not be able to revoke it, even by guessing/enumerating its id. + """ + # `invited_by_user_id` FKs to a real `user` row. + user_repo = SQLiteUserRepository(connection) + inviter = await user_repo.create( + User( + id=uuid4(), + personal_data_uuid=two_workspaces.pd_b.uuid, + email="inviter-b@example.com", + hashed_password="not-a-real-hash", + created_at=datetime.now(), + ) + ) + inv_repo = SQLiteWorkspaceInvitationRepository(connection) + inv = await inv_repo.create( + WorkspaceInvitation( + id=uuid4(), + workspace_id=two_workspaces.ws_b.id, + token_hash="deadbeef", + invited_by_user_id=inviter.id, + status="pending", + expires_at=datetime.now() + timedelta(hours=1), + created_at=datetime.now(), + ) + ) + resp = await client.delete(f"/api/invitations/{inv.id}") + assert resp.status_code == 404 diff --git a/backend/tests/unit/test_calendar_router.py b/backend/tests/unit/test_calendar_router.py index 0f767a1..a6778cd 100644 --- a/backend/tests/unit/test_calendar_router.py +++ b/backend/tests/unit/test_calendar_router.py @@ -23,6 +23,23 @@ from app.repositories_impl.sqlite.day_override_repository import SQLiteDayOverri from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository +# Fixed identity for the authenticated caller in every test in this file — +# each test must also create a `PersonalData` row with this uuid in the +# workspace it seeds, so `get_current_personal_data` resolves to that +# workspace (see CLAUDE.md T1 authz work / audit §4 blast radius). +_CURRENT_USER_PD_UUID = uuid4() + + +async def _seed_caller(connection: aiosqlite.Connection, workspace_id) -> None: + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, + workspace_id=workspace_id, + name="Alex", + created_at=datetime.now(), + ) + ) + @pytest_asyncio.fixture async def client(connection: aiosqlite.Connection) -> AsyncClient: @@ -33,7 +50,7 @@ async def client(connection: aiosqlite.Connection) -> AsyncClient: app.dependency_overrides[get_connection] = lambda: connection app.dependency_overrides[get_current_user] = lambda: User( id=uuid4(), - personal_data_uuid=uuid4(), + personal_data_uuid=_CURRENT_USER_PD_UUID, email="test@example.com", hashed_password="not-a-real-hash", created_at=datetime.now(), @@ -52,6 +69,7 @@ class TestGetCalendarDays: ws = await workspace_repo.create( Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) ) + await _seed_caller(connection, ws.id) response = await client.get( "/api/calendar/days", @@ -76,6 +94,7 @@ class TestGetCalendarDays: ws = await workspace_repo.create( Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) ) + await _seed_caller(connection, ws.id) response = await client.get( "/api/calendar/days", @@ -91,6 +110,7 @@ class TestGetCalendarDays: ws = await workspace_repo.create( Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) ) + await _seed_caller(connection, ws.id) response = await client.get( "/api/calendar/days", @@ -112,6 +132,7 @@ class TestGetCalendarDays: ws = await workspace_repo.create( Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) ) + await _seed_caller(connection, ws.id) # Discover max_year from a normal response rather than hardcoding it. bounds_response = await client.get( @@ -144,7 +165,12 @@ class TestGetCalendarDays: Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) ) await personal_data_repo.create( - PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alex", created_at=datetime.now()) + PersonalData( + uuid=_CURRENT_USER_PD_UUID, + workspace_id=ws.id, + name="Alex", + created_at=datetime.now(), + ) ) override_date = date(2026, 4, 30) # Valborgsmässoafton: not a default day off (§6) await day_override_repo.create( diff --git a/backend/tests/unit/test_config.py b/backend/tests/unit/test_config.py new file mode 100644 index 0000000..eb57015 --- /dev/null +++ b/backend/tests/unit/test_config.py @@ -0,0 +1,41 @@ +"""Tests for Settings' production hardening validator. + +Never mutates the module-level `settings` singleton — constructs fresh +`Settings()` instances instead, passing kwargs directly (bypassing env/file +loading concerns) so these tests are isolated from the process environment. +""" + +import pytest + +from app.config import Settings + + +class TestProductionJwtSecretValidation: + def test_development_allows_default_secret(self) -> None: + settings = Settings(app_env="development") + assert settings.jwt_secret == "dev-secret-change-in-production-min-32-chars" + + def test_test_env_allows_default_secret(self) -> None: + settings = Settings(app_env="test") + assert settings.app_env == "test" + + def test_production_rejects_default_secret(self) -> None: + with pytest.raises(ValueError, match="APP_JWT_SECRET"): + Settings(app_env="production") + + def test_production_rejects_short_secret(self) -> None: + with pytest.raises(ValueError, match="APP_JWT_SECRET"): + Settings(app_env="production", jwt_secret="short-secret") + + def test_production_accepts_strong_secret(self) -> None: + strong_secret = "a" * 32 + settings = Settings(app_env="production", jwt_secret=strong_secret) + assert settings.jwt_secret == strong_secret + + def test_default_app_env_is_development(self) -> None: + settings = Settings() + assert settings.app_env == "development" + + def test_default_log_level_is_info(self) -> None: + settings = Settings() + assert settings.log_level == "INFO" diff --git a/backend/tests/unit/test_db_unit_of_work.py b/backend/tests/unit/test_db_unit_of_work.py new file mode 100644 index 0000000..e4efbf7 --- /dev/null +++ b/backend/tests/unit/test_db_unit_of_work.py @@ -0,0 +1,243 @@ +"""Tests for the unit-of-work mechanism (`app.db.connection`) and the +batched `PlannedLeaveRepository` methods it exists to support. + +See `scratchpad/audit/backend-perf.md` §3: the app runs on one shared +connection, so a multi-statement write needs (a) mutual exclusion for the +whole unit of work and (b) the per-row `commit()` calls suppressed while one +is in progress. These tests exercise that mechanism directly against an +in-memory connection, plus the `PlanningService.apply_proposed_days` +atomicity it is meant to guarantee end-to-end. +""" + +import asyncio +from datetime import date, datetime +from uuid import uuid4 + +import aiosqlite +import pytest + +from app.db.connection import commit, ensure_schema, unit_of_work +from app.entities.leave_wish import LeaveWish +from app.entities.planned_leave import PlannedLeaveDay +from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository +from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository +from app.services.planning_service import PlanningService, ProposedDayInput +from tests.conftest import SeededIds + + +@pytest.fixture +async def uow_connection() -> aiosqlite.Connection: + conn = await aiosqlite.connect(":memory:") + conn.row_factory = aiosqlite.Row + await conn.execute("PRAGMA foreign_keys = ON;") + await ensure_schema(conn) + try: + yield conn + finally: + await conn.close() + + +async def _workspace_count(conn: aiosqlite.Connection) -> int: + cursor = await conn.execute("SELECT COUNT(*) FROM workspace") + row = await cursor.fetchone() + return int(row[0]) + + +async def _insert_workspace(conn: aiosqlite.Connection, workspace_id: str) -> None: + await conn.execute( + "INSERT INTO workspace (id, name, planning_mode, created_at) VALUES (?, ?, ?, ?)", + (workspace_id, "Test", "solo", datetime.now().isoformat()), + ) + + +class TestUnitOfWork: + async def test_commits_on_success(self, uow_connection: aiosqlite.Connection) -> None: + async with unit_of_work(uow_connection): + await _insert_workspace(uow_connection, "ws-commit") + assert await _workspace_count(uow_connection) == 1 + + async def test_rolls_back_on_exception(self, uow_connection: aiosqlite.Connection) -> None: + with pytest.raises(ValueError): + async with unit_of_work(uow_connection): + await _insert_workspace(uow_connection, "ws-rollback") + raise ValueError("simulated failure") + assert await _workspace_count(uow_connection) == 0 + + async def test_commit_helper_is_noop_inside_uow( + self, uow_connection: aiosqlite.Connection + ) -> None: + """`commit()` must not durably commit inside a unit of work — otherwise + a later rollback in the same block could only undo writes made after + that no-op `commit()` call, not before it (see backend-perf audit §3, + "Is BEGIN/COMMIT ... safe on the shared connection? No, not alone."). + """ + with pytest.raises(RuntimeError): + async with unit_of_work(uow_connection): + await _insert_workspace(uow_connection, "ws-noop-commit") + await commit(uow_connection) # must be a no-op here + raise RuntimeError("still rolls back everything, including the row above") + assert await _workspace_count(uow_connection) == 0 + + async def test_concurrent_calls_serialize(self, uow_connection: aiosqlite.Connection) -> None: + """Two concurrent `unit_of_work()` blocks on the same connection never + interleave — the second blocks at entry until the first fully exits. + """ + events: list[str] = [] + + async def worker(name: str, hold: float) -> None: + async with unit_of_work(uow_connection): + events.append(f"{name}-start") + await asyncio.sleep(hold) + events.append(f"{name}-end") + + await asyncio.gather(worker("a", 0.05), worker("b", 0)) + + assert events in ( + ["a-start", "a-end", "b-start", "b-end"], + ["b-start", "b-end", "a-start", "a-end"], + ) + + +class TestPlannedLeaveRepositoryBatchMethods: + async def test_create_many_then_delete_many( + self, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + now = datetime.now() + entities = [ + PlannedLeaveDay( + id=uuid4(), + workspace_id=seeded_ids.workspace_id, + child_id=seeded_ids.child_id, + parent_personal_data_uuid=seeded_ids.personal_data_uuid, + date=date(2026, 3, day), + day_type="sjukpenning", + created_at=now, + updated_at=now, + ) + for day in (1, 2, 3) + ] + + await planned_leave_repo.create_many(entities) + rows = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id) + assert len(rows) == 3 + + deleted = await planned_leave_repo.delete_many([entities[0].id, entities[1].id]) + assert deleted == 2 + remaining = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id) + assert [r.id for r in remaining] == [entities[2].id] + + async def test_create_many_empty_list_is_noop( + self, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + await planned_leave_repo.create_many([]) + assert await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id) == [] + + async def test_delete_many_is_idempotent( + self, planned_leave_repo: SQLitePlannedLeaveRepository + ) -> None: + """Unlike `delete()`, a missing id is silently ignored rather than raising.""" + deleted = await planned_leave_repo.delete_many([uuid4(), uuid4()]) + assert deleted == 0 + + async def test_update_metadata_many( + self, + planned_leave_repo: SQLitePlannedLeaveRepository, + leave_wish_repo: SQLiteLeaveWishRepository, + seeded_ids: SeededIds, + ) -> None: + now = datetime.now() + entity = PlannedLeaveDay( + id=uuid4(), + workspace_id=seeded_ids.workspace_id, + child_id=seeded_ids.child_id, + parent_personal_data_uuid=seeded_ids.personal_data_uuid, + date=date(2026, 4, 1), + day_type="vacation", + created_at=now, + updated_at=now, + ) + await planned_leave_repo.create(entity) + + wish = await leave_wish_repo.create( + LeaveWish( + id=uuid4(), + workspace_id=seeded_ids.workspace_id, + child_id=seeded_ids.child_id, + name="Test wish", + window_start_month=4, + window_start_day=1, + window_end_month=4, + window_end_day=30, + target_weeks=1, + recurrence="once", + first_year=2026, + last_year=None, + priority=1, + parent_mode="either", + parent_personal_data_uuid=None, + window_mode="fixed", + created_at=now, + updated_at=now, + ) + ) + + await planned_leave_repo.update_metadata_many([(entity.id, wish.id, True)]) + + refreshed = await planned_leave_repo.get_by_id(entity.id) + assert refreshed is not None + assert refreshed.wish_id == wish.id + assert refreshed.is_pinned is True + # day_type/is_dubbeldag are untouched by the metadata-only update. + assert refreshed.day_type == "vacation" + + +class TestApplyProposedDaysAtomicity: + async def test_mid_batch_failure_leaves_nothing_persisted( + self, + planning_service_with_uow: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """A batch that replaces an existing row (delete + create, both inside + stage 4) must not leave the delete applied if the create half fails — + the whole `async with self._uow():` block must roll back together. + Uses `planning_service_with_uow` (a real `SQLiteUnitOfWork`), not the + default `planning_service` fixture, which wires a no-op unit of work + and would (correctly, given a no-op) leave the delete committed. + """ + existing = await planning_service_with_uow.add_planned_day( + workspace_id=seeded_ids.workspace_id, + child_id=seeded_ids.child_id, + parent_personal_data_uuid=seeded_ids.personal_data_uuid, + date=date(2026, 5, 1), + day_type="sjukpenning", + ) + + async def boom(_items: list[PlannedLeaveDay]) -> None: + raise RuntimeError("simulated mid-batch failure") + + monkeypatch.setattr(planned_leave_repo, "create_many", boom) + + days = [ + ProposedDayInput( + date=date(2026, 5, 1), + parent_personal_data_uuid=seeded_ids.personal_data_uuid, + child_id=seeded_ids.child_id, + day_type="lagsta", # differs from the existing row -> collision -> to_replace + ) + ] + + with pytest.raises(RuntimeError): + await planning_service_with_uow.apply_proposed_days( + workspace_id=seeded_ids.workspace_id, days=days, replace=True + ) + + remaining = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id) + assert len(remaining) == 1 + assert remaining[0].id == existing.id + assert remaining[0].day_type == "sjukpenning" diff --git a/backend/tests/unit/test_fetch_data_router.py b/backend/tests/unit/test_fetch_data_router.py index f09bb92..37608be 100644 --- a/backend/tests/unit/test_fetch_data_router.py +++ b/backend/tests/unit/test_fetch_data_router.py @@ -28,40 +28,24 @@ non-headless browser as a fire-and-forget background task, which is unsafe to do from a router test). """ -from datetime import datetime from typing import Any -from uuid import UUID, uuid4 +from uuid import UUID -import aiosqlite import pytest import pytest_asyncio -from httpx import ASGITransport, AsyncClient +from httpx import AsyncClient -from app.dependencies import get_connection, get_current_user -from app.entities.user import User -from app.main import app from app.routers import fetch_data as fetch_data_router from tests.conftest import SeededIds @pytest_asyncio.fixture -async def client(connection: aiosqlite.Connection) -> AsyncClient: - """An httpx client against the real app: `get_connection` and - `get_current_user` are overridden to use the test's in-memory connection - and a fixed fake user. +async def client(authed_client: AsyncClient) -> AsyncClient: + """The shared `seeded_ids`-bound authenticated client (see + `tests/conftest.py`) — required now that the routes reject ids outside + the caller's workspace. See CLAUDE.md T1 authz work / audit §4. """ - app.dependency_overrides[get_connection] = lambda: connection - app.dependency_overrides[get_current_user] = lambda: User( - id=uuid4(), - personal_data_uuid=uuid4(), - email="test@example.com", - hashed_password="not-a-real-hash", - created_at=datetime.now(), - ) - async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: - yield ac - app.dependency_overrides.pop(get_connection, None) - app.dependency_overrides.pop(get_current_user, None) + return authed_client def _manual_payload(seeded_ids: SeededIds, **overrides: object) -> dict: @@ -233,11 +217,17 @@ class TestScraperEndpointsEnabled: captured: dict[str, Any] = {} async def fake_start_session( - *, workspace_id: UUID, personal_data_uuid: UUID, child_id: UUID, persist_callback: Any + *, + workspace_id: UUID, + personal_data_uuid: UUID, + child_id: UUID, + persist_callback: Any, + owner_user_id: UUID, ) -> str: captured["workspace_id"] = workspace_id captured["personal_data_uuid"] = personal_data_uuid captured["child_id"] = child_id + captured["owner_user_id"] = owner_user_id return "fake-session-id" monkeypatch.setattr(fetch_data_router._scraper_service, "start_session", fake_start_session) @@ -263,7 +253,7 @@ class TestScraperEndpointsEnabled: ) -> None: monkeypatch.setattr(fetch_data_router.settings, "scraper_enabled", True) - async def fake_poll_session(session_id: str) -> dict: + async def fake_poll_session(session_id: str, owner_user_id: UUID) -> dict: assert session_id == "some-session-id" return {"status": "awaiting_scan"} @@ -283,7 +273,7 @@ class TestScraperEndpointsEnabled: cancelled: dict[str, Any] = {} - async def fake_cancel_session(session_id: str) -> None: + async def fake_cancel_session(session_id: str, owner_user_id: UUID) -> None: cancelled["session_id"] = session_id monkeypatch.setattr( diff --git a/backend/tests/unit/test_fk_scraper.py b/backend/tests/unit/test_fk_scraper.py index 112bee7..741e2b0 100644 --- a/backend/tests/unit/test_fk_scraper.py +++ b/backend/tests/unit/test_fk_scraper.py @@ -35,15 +35,16 @@ from app.scrapers.fk_scraper import ( # Helpers for the HTML-fixture tests # --------------------------------------------------------------------------- -CAPTURES_DIR = Path(__file__).parents[3] / "captures" +FIXTURES_DIR = Path(__file__).parents[1] / "fixtures" def _load_fixture(name: str) -> str: - """Read a page capture from captures/, skipping if not present.""" - path = CAPTURES_DIR / name - if not path.exists(): - pytest.skip(f"Fixture {name} not found — run the captures step first") - return path.read_text(encoding="utf-8") + """Read a synthetic HTML fixture from tests/fixtures/. + + Fixtures here are hand-built (never real FK page captures — see F1 of + the scraper/config/ops security audit) and always present in the repo. + """ + return (FIXTURES_DIR / name).read_text(encoding="utf-8") # --------------------------------------------------------------------------- @@ -144,84 +145,90 @@ from app.scrapers.fk_scraper import _extract_parental_leave # noqa: E402 class TestExtractFromFixture: - """Parse the captured overview HTML and assert correct field values.""" + """Parse the synthetic overview HTML fixture and assert correct field values.""" async def test_days_left_and_total_parsed(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-1", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) - # From the capture: 164 av 240 dagar - assert result.days_left == 164 + # From the synthetic fixture: 100 av 240 dagar + assert result.days_left == 100 assert result.total_days == 240 - assert result.days_used == 76 + assert result.days_used == 140 async def test_tier_values_parsed(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-2", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) - # From the capture: sjukpenning 119, lägstanivå 45 - assert result.tier_data.sjukpenning_left == 119 - assert result.tier_data.lagsta_niva_left == 45 + # From the synthetic fixture: sjukpenning 60, lägstanivå 40 + assert result.tier_data.sjukpenning_left == 60 + assert result.tier_data.lagsta_niva_left == 40 async def test_dubbeldagar_parsed(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-3", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) - # From the capture: 39 av 60 dubbeldagar - assert result.dubbeldagar_left == 39 + # From the synthetic fixture: 20 av 60 dubbeldagar + assert result.dubbeldagar_left == 20 async def test_source_is_scraped(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-4", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) assert result.source == "scraped" async def test_reserved_days_derived(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-5", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) - # 76 days used < 150 transferable → all reserved days remain + # 140 days used < 150 transferable → all reserved days remain assert result.reserved_days_total == 90 assert result.reserved_days_left == 90 - assert result.transferable_days_left == 240 - 90 - 76 # 74 + assert result.transferable_days_left == 240 - 90 - 140 # 10 async def test_tier_totals_sum_to_total_days(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-6", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) @@ -231,12 +238,13 @@ class TestExtractFromFixture: ) async def test_tier_used_nonnegative(self) -> None: - html = _load_fixture("05_foraldrapenning_overview.html") + html = _load_fixture("fk_overview_synthetic.html") state = _SessionState( session_id="test-7", workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) page = _make_mock_page(html) result = await _extract_parental_leave(page, state) @@ -296,6 +304,7 @@ class TestValidation: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) async def test_negative_days_left_raises(self) -> None: @@ -390,43 +399,51 @@ class TestSessionStateMachine: async def test_start_session_creates_entry(self) -> None: service = FKScraperService() + owner_user_id = uuid.uuid4() with patch("app.scrapers.fk_scraper._run_session", new_callable=AsyncMock) as mock_run: mock_run.return_value = None sid = await service.start_session( workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=owner_user_id, ) assert sid in _sessions assert _sessions[sid].status == "launching" + assert _sessions[sid].owner_user_id == owner_user_id async def test_poll_unknown_session_returns_error(self) -> None: service = FKScraperService() - result = await service.poll_session("nonexistent") + result = await service.poll_session("nonexistent", uuid.uuid4()) assert result["status"] == "error" assert "Unknown" in result["message"] async def test_poll_awaiting_scan_returns_qr_data(self) -> None: service = FKScraperService() sid = str(uuid.uuid4()) + owner_user_id = uuid.uuid4() state = _SessionState( session_id=sid, workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=owner_user_id, status="awaiting_scan", qr_data=b"PNG_BYTES", ) _sessions[sid] = state - result = await service.poll_session(sid) + result = await service.poll_session(sid, owner_user_id) assert result["status"] == "awaiting_scan" assert "qr_data" in result import base64 assert base64.b64decode(result["qr_data"]) == b"PNG_BYTES" - async def test_cancel_sets_status(self) -> None: + async def test_poll_with_wrong_owner_is_treated_as_unknown_session(self) -> None: + """F4 (T1 authz audit): a session_id known to a caller who did not + start it must behave EXACTLY like a made-up session_id — never leak + that the session exists (no QR data, no distinct error).""" service = FKScraperService() sid = str(uuid.uuid4()) state = _SessionState( @@ -434,16 +451,72 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), + status="awaiting_scan", + qr_data=b"PNG_BYTES", + ) + _sessions[sid] = state + + result = await service.poll_session(sid, uuid.uuid4()) # different owner + assert result == {"status": "error", "message": "Unknown session_id"} + + async def test_cancel_sets_status(self) -> None: + service = FKScraperService() + sid = str(uuid.uuid4()) + owner_user_id = uuid.uuid4() + state = _SessionState( + session_id=sid, + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_user_id, status="awaiting_scan", ) _sessions[sid] = state - await service.cancel_session(sid) + await service.cancel_session(sid, owner_user_id) assert _sessions[sid].status == "cancelled" + async def test_cancel_clears_qr_data(self) -> None: + """F5: cancelling a session must not leave the last QR frame in RAM.""" + service = FKScraperService() + sid = str(uuid.uuid4()) + owner_user_id = uuid.uuid4() + state = _SessionState( + session_id=sid, + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_user_id, + status="awaiting_scan", + qr_data=b"PNG_BYTES", + ) + _sessions[sid] = state + + await service.cancel_session(sid, owner_user_id) + assert _sessions[sid].qr_data is None + + async def test_cancel_with_wrong_owner_is_a_no_op(self) -> None: + """F4: cancelling with the wrong owner must not affect the session — + exactly like cancelling an unknown session_id.""" + service = FKScraperService() + sid = str(uuid.uuid4()) + state = _SessionState( + session_id=sid, + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), + status="awaiting_scan", + ) + _sessions[sid] = state + + await service.cancel_session(sid, uuid.uuid4()) # different owner + assert _sessions[sid].status == "awaiting_scan" + async def test_cancel_nonexistent_session_is_safe(self) -> None: service = FKScraperService() - await service.cancel_session("does-not-exist") # should not raise + await service.cancel_session("does-not-exist", uuid.uuid4()) # should not raise async def test_ttl_watchdog_cancels_stale_session(self) -> None: from app.scrapers.fk_scraper import _ttl_watchdog @@ -456,6 +529,7 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), status="awaiting_scan", ) _sessions[sid] = state @@ -465,6 +539,106 @@ class TestSessionStateMachine: assert _sessions[sid].status == "error" assert "timed out" in (_sessions[sid].message or "") + async def test_ttl_watchdog_purges_already_terminal_session(self) -> None: + """F5: if the session already finished by the time the watchdog + fires, it's a safety net that purges it immediately rather than + leaving it for `_purge_later`.""" + from app.scrapers.fk_scraper import _ttl_watchdog + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_session_ttl_seconds = 0 # immediate + sid = str(uuid.uuid4()) + state = _SessionState( + session_id=sid, + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), + status="done", + ) + _sessions[sid] = state + + await _ttl_watchdog(sid) + + assert sid not in _sessions + + async def test_purge_later_removes_session_after_retention_window(self) -> None: + """F5: a finished session is removed from `_sessions` after + `scraper_result_retention_seconds`.""" + from app.scrapers.fk_scraper import _purge_later + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_result_retention_seconds = 0 # immediate + sid = str(uuid.uuid4()) + state = _SessionState( + session_id=sid, + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), + status="done", + result=None, + ) + _sessions[sid] = state + + await _purge_later(sid) + + assert sid not in _sessions + + async def test_purge_later_is_a_no_op_if_already_purged(self) -> None: + from app.scrapers.fk_scraper import _purge_later + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_result_retention_seconds = 0 + await _purge_later("does-not-exist") # should not raise + + async def test_start_session_cancels_existing_session_for_same_owner(self) -> None: + """F5: at most one live session per owner — starting a new one + cancels any non-terminal session already in flight for that owner.""" + service = FKScraperService() + owner_user_id = uuid.uuid4() + with patch("app.scrapers.fk_scraper._run_session", new_callable=AsyncMock): + first_sid = await service.start_session( + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_user_id, + ) + _sessions[first_sid].status = "awaiting_scan" + + second_sid = await service.start_session( + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_user_id, + ) + + assert _sessions[first_sid].status == "cancelled" + assert _sessions[second_sid].status == "launching" + assert first_sid != second_sid + + async def test_start_session_does_not_cancel_other_owners_session(self) -> None: + service = FKScraperService() + owner_a = uuid.uuid4() + owner_b = uuid.uuid4() + with patch("app.scrapers.fk_scraper._run_session", new_callable=AsyncMock): + sid_a = await service.start_session( + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_a, + ) + _sessions[sid_a].status = "awaiting_scan" + + await service.start_session( + workspace_id=uuid.uuid4(), + personal_data_uuid=uuid.uuid4(), + child_id=uuid.uuid4(), + owner_user_id=owner_b, + ) + + assert _sessions[sid_a].status == "awaiting_scan" + async def test_set_status_updates_message(self) -> None: sid = str(uuid.uuid4()) state = _SessionState( @@ -472,6 +646,7 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) _set_status(state, "error", "Something went wrong") assert state.status == "error" @@ -517,6 +692,7 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), persist_callback=mock_persist, ) _sessions[sid] = state @@ -575,6 +751,7 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), persist_callback=failing_persist, ) _sessions[sid] = state @@ -627,6 +804,7 @@ class TestSessionStateMachine: workspace_id=uuid.uuid4(), personal_data_uuid=uuid.uuid4(), child_id=uuid.uuid4(), + owner_user_id=uuid.uuid4(), ) _sessions[sid] = state @@ -654,6 +832,58 @@ class TestSessionStateMachine: assert call_args[1]["timeout"] == 30_000 +# --------------------------------------------------------------------------- +# F3: debug artifacts are opt-in (contain personal data when written) +# --------------------------------------------------------------------------- + + +class TestSaveDebugArtifacts: + def _make_page(self) -> MagicMock: + async def _fake_screenshot(path: str, full_page: bool = True) -> None: + # Real Playwright writes the PNG to `path` itself; mirror that so + # the "both files got written" assertion is meaningful. + Path(path).write_bytes(b"fake-png-bytes") + + page = MagicMock() + page.content = AsyncMock(return_value="fake") + page.screenshot = AsyncMock(side_effect=_fake_screenshot) + return page + + async def test_noop_when_debug_disabled_even_with_dir_set(self, tmp_path) -> None: + from app.scrapers.fk_scraper import _save_debug_artifacts + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_debug_dir = str(tmp_path) + mock_settings.scraper_debug_enabled = False + + await _save_debug_artifacts(self._make_page(), "sid-1", ["[log] hi"]) + + assert list(tmp_path.iterdir()) == [] + + async def test_noop_when_dir_unset_even_if_enabled(self, tmp_path) -> None: + from app.scrapers.fk_scraper import _save_debug_artifacts + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_debug_dir = None + mock_settings.scraper_debug_enabled = True + + await _save_debug_artifacts(self._make_page(), "sid-2", ["[log] hi"]) + + assert list(tmp_path.iterdir()) == [] + + async def test_writes_artifacts_when_both_enabled_and_dir_set(self, tmp_path) -> None: + from app.scrapers.fk_scraper import _save_debug_artifacts + + with patch("app.scrapers.fk_scraper.settings") as mock_settings: + mock_settings.scraper_debug_dir = str(tmp_path) + mock_settings.scraper_debug_enabled = True + + await _save_debug_artifacts(self._make_page(), "sid-3", ["[log] hi"]) + + written = {p.name for p in tmp_path.iterdir()} + assert written == {"sid-3.html", "sid-3.png", "sid-3.console.txt"} + + # --------------------------------------------------------------------------- # Persistence integration: real SQLite repo round-trip # --------------------------------------------------------------------------- @@ -702,6 +932,7 @@ class TestPersistenceIntegration: workspace_id=seeded_ids.workspace_id, personal_data_uuid=seeded_ids.personal_data_uuid, child_id=seeded_ids.child_id, + owner_user_id=uuid.uuid4(), persist_callback=persist_callback, ) _sessions[sid] = state diff --git a/backend/tests/unit/test_leave_wish_router.py b/backend/tests/unit/test_leave_wish_router.py index ab75e6a..67854db 100644 --- a/backend/tests/unit/test_leave_wish_router.py +++ b/backend/tests/unit/test_leave_wish_router.py @@ -14,11 +14,19 @@ import pytest_asyncio from httpx import ASGITransport, AsyncClient from app.dependencies import get_connection, get_current_user +from app.entities.personal_data import PersonalData from app.entities.user import User from app.entities.workspace import Workspace from app.main import app +from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository +# Fixed identity for the authenticated caller in every test in this file — +# each test must also create a `PersonalData` row with this uuid in the +# workspace it seeds, so `get_current_personal_data` resolves to that +# workspace (see CLAUDE.md T1 authz work / audit §4 blast radius). +_CURRENT_USER_PD_UUID = uuid4() + @pytest_asyncio.fixture async def client(connection: aiosqlite.Connection) -> AsyncClient: @@ -29,7 +37,7 @@ async def client(connection: aiosqlite.Connection) -> AsyncClient: app.dependency_overrides[get_connection] = lambda: connection app.dependency_overrides[get_current_user] = lambda: User( id=uuid4(), - personal_data_uuid=uuid4(), + personal_data_uuid=_CURRENT_USER_PD_UUID, email="test@example.com", hashed_password="not-a-real-hash", created_at=datetime.now(), @@ -53,6 +61,11 @@ class TestLeaveWishTargetDaysRoundTrip: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) target_days_value = 15 response = await client.post( @@ -91,6 +104,11 @@ class TestLeaveWishTargetDaysRoundTrip: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) response = await client.post( "/api/leave-wishes", @@ -121,6 +139,11 @@ class TestLeaveWishTargetDaysRoundTrip: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) # Create a wish without target_days create_response = await client.post( @@ -168,6 +191,11 @@ class TestLeaveWishTargetDaysRoundTrip: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) # Create a wish with target_days target_days_value = 25 @@ -212,6 +240,11 @@ class TestLeaveWishTargetDaysRoundTrip: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await SQLitePersonalDataRepository(connection).create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) # Create a wish with target_days target_days_value = 30 diff --git a/backend/tests/unit/test_plan_router.py b/backend/tests/unit/test_plan_router.py index d90b580..8a2be65 100644 --- a/backend/tests/unit/test_plan_router.py +++ b/backend/tests/unit/test_plan_router.py @@ -24,10 +24,11 @@ import pytest_asyncio from dateutil.relativedelta import relativedelta from httpx import ASGITransport, AsyncClient -from app.dependencies import get_connection +from app.dependencies import get_connection, get_current_user from app.entities.child import Child from app.entities.parental_leave import ParentalLeaveFetch, TierData from app.entities.personal_data import PersonalData +from app.entities.user import User from app.entities.vacation_days import VacationDays from app.entities.workspace import Workspace from app.main import app @@ -38,6 +39,14 @@ from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacation from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository from app.services.allocation_service import PlanProposal, WindowOutcome +# Fixed identity for the authenticated caller in every test in this file. +# Each test creates its own workspace + "parent" PersonalData row — that row +# is created with THIS uuid (instead of a random one) so the caller's +# `get_current_personal_data` -> `workspace_id` resolves to the workspace the +# test just seeded. See CLAUDE.md T1 authz work / audit §4 blast radius. +_CURRENT_USER_PD_UUID = uuid4() +_CURRENT_USER_ID = uuid4() + def _make_fetch( personal_data_uuid, @@ -82,9 +91,17 @@ async def client(connection: aiosqlite.Connection) -> AsyncClient: on-disk database via the (never-run) lifespan. """ app.dependency_overrides[get_connection] = lambda: connection + app.dependency_overrides[get_current_user] = lambda: User( + id=_CURRENT_USER_ID, + personal_data_uuid=_CURRENT_USER_PD_UUID, + email="alex@example.com", + hashed_password="x", + created_at=datetime.now(), + ) async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: yield ac app.dependency_overrides.pop(get_connection, None) + app.dependency_overrides.pop(get_current_user, None) class TestProposeSurplusAdviceOverTheWire: @@ -107,7 +124,7 @@ class TestProposeSurplusAdviceOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -178,7 +195,7 @@ class TestProposeAge4ProjectionsOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -254,7 +271,7 @@ class TestProposeMessageKeysOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -369,7 +386,7 @@ class TestProposeWishNameMessageKeyOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -451,7 +468,7 @@ class TestProposeWishNameMessageKeyOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -528,7 +545,7 @@ class TestApplyRejectionsOverTheWireMessageKeys: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", @@ -598,7 +615,13 @@ class TestLeaveWishWindowModeOverTheWire: ) -> None: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) + personal_data_repo = SQLitePersonalDataRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await personal_data_repo.create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) response = await client.post( "/api/leave-wishes", @@ -632,7 +655,13 @@ class TestLeaveWishWindowModeOverTheWire: ) -> None: now = datetime.now() workspace_repo = SQLiteWorkspaceRepository(connection) + personal_data_repo = SQLitePersonalDataRepository(connection) ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) + await personal_data_repo.create( + PersonalData( + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", created_at=now + ) + ) response = await client.post( "/api/leave-wishes", @@ -668,7 +697,11 @@ class TestLeaveWishWindowModeOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), workspace_id=ws.id, name="Alex", salary_type="monthly", created_at=now + uuid=_CURRENT_USER_PD_UUID, + workspace_id=ws.id, + name="Alex", + salary_type="monthly", + created_at=now, ) ) child = await child_repo.create( @@ -740,7 +773,11 @@ class TestLeaveWishWindowModeOverTheWire: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), workspace_id=ws.id, name="Alex", salary_type="monthly", created_at=now + uuid=_CURRENT_USER_PD_UUID, + workspace_id=ws.id, + name="Alex", + salary_type="monthly", + created_at=now, ) ) child = await child_repo.create( @@ -924,7 +961,7 @@ class TestPlanProposalFieldParityWithDomain: ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( PersonalData( - uuid=uuid4(), + uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alex", salary_type="monthly", diff --git a/backend/tests/unit/test_services/test_auth_service.py b/backend/tests/unit/test_services/test_auth_service.py index a67fa1f..dfd2bfa 100644 --- a/backend/tests/unit/test_services/test_auth_service.py +++ b/backend/tests/unit/test_services/test_auth_service.py @@ -2,6 +2,7 @@ import pytest +from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.services.auth_service import AuthService from app.utils.errors import AuthError, ConflictError, UnauthorizedError @@ -34,6 +35,35 @@ class TestAuthServiceRegisterLogin: with pytest.raises(AuthError): await auth_service.login("nobody@example.com", "anything") + async def test_register_conflict_message_is_generic(self, auth_service: AuthService) -> None: + await auth_service.register("henry@example.com", "password123", "Henry") + with pytest.raises(ConflictError) as exc_info: + await auth_service.register("henry@example.com", "otherpass", "Henry2") + assert "henry@example.com" not in str(exc_info.value) + assert str(exc_info.value) == "Email already registered" + + async def test_login_wrong_password_and_unknown_email_share_message( + self, auth_service: AuthService + ) -> None: + await auth_service.register("ivy@example.com", "correctpass", "Ivy") + with pytest.raises(AuthError) as wrong_password: + await auth_service.login("ivy@example.com", "wrongpass") + with pytest.raises(AuthError) as unknown_email: + await auth_service.login("nobody2@example.com", "anything") + assert str(wrong_password.value) == str(unknown_email.value) + + async def test_login_inactive_account_raises_unauthorized( + self, + auth_service: AuthService, + user_repo: SQLiteUserRepository, + ) -> None: + user, _ = await auth_service.register("jack@example.com", "password123", "Jack") + user.is_active = False + await user_repo.update(user.id, user) + + with pytest.raises(UnauthorizedError): + await auth_service.login("jack@example.com", "password123") + class TestAuthServiceTokens: async def test_access_token_round_trip(self, auth_service: AuthService) -> None: @@ -60,3 +90,17 @@ class TestAuthServiceTokens: async def test_invalid_token_raises_unauthorized(self, auth_service: AuthService) -> None: with pytest.raises(UnauthorizedError): await auth_service.get_user_from_token("not.a.valid.jwt") + + async def test_token_for_inactive_user_raises_unauthorized( + self, + auth_service: AuthService, + user_repo: SQLiteUserRepository, + ) -> None: + user, _ = await auth_service.register("heidi@example.com", "pass3456", "Heidi") + token = auth_service.create_access_token(user.id) + + user.is_active = False + await user_repo.update(user.id, user) + + with pytest.raises(UnauthorizedError): + await auth_service.get_user_from_token(token) diff --git a/backend/tests/unit/test_tips_router.py b/backend/tests/unit/test_tips_router.py index 11f3cc6..c78f408 100644 --- a/backend/tests/unit/test_tips_router.py +++ b/backend/tests/unit/test_tips_router.py @@ -57,12 +57,19 @@ def _make_fetch(personal_data_uuid, workspace_id, child_id, used: int = 20) -> P ) +# Fixed identity for the authenticated caller in every test in this file — +# `workspace_with_expiring_days` creates its parent ("Alice") with this uuid +# so `get_current_personal_data` resolves to the seeded workspace (see +# CLAUDE.md T1 authz work / audit §4 blast radius). +_CURRENT_USER_PD_UUID = uuid4() + + @pytest_asyncio.fixture async def client(connection: aiosqlite.Connection) -> AsyncClient: app.dependency_overrides[get_connection] = lambda: connection app.dependency_overrides[get_current_user] = lambda: User( id=uuid4(), - personal_data_uuid=uuid4(), + personal_data_uuid=_CURRENT_USER_PD_UUID, email="test@example.com", hashed_password="not-a-real-hash", created_at=datetime.now(), @@ -83,7 +90,7 @@ async def workspace_with_expiring_days(connection: aiosqlite.Connection): ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now)) parent = await personal_data_repo.create( - PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alice", created_at=now) + PersonalData(uuid=_CURRENT_USER_PD_UUID, workspace_id=ws.id, name="Alice", created_at=now) ) child = await child_repo.create( Child( diff --git a/backend/tests/unit/test_workspace_service.py b/backend/tests/unit/test_workspace_service.py index 1561e67..6a0e43a 100644 --- a/backend/tests/unit/test_workspace_service.py +++ b/backend/tests/unit/test_workspace_service.py @@ -5,8 +5,9 @@ Covers workspace creation/update, with a focus on the `planning_mode` field renaming/removing a placeholder partner (`rename_member` / `remove_member`). """ +import hashlib from datetime import date, datetime -from uuid import uuid4 +from uuid import UUID, uuid4 import pytest @@ -21,10 +22,33 @@ from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedL from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository from app.services.workspace_service import WorkspaceService -from app.utils.errors import NotFoundError, ValidationError +from app.utils.errors import InvitationError, NotFoundError, ValidationError from tests.conftest import SeededIds +def _hash(token: str) -> str: + return hashlib.sha256(token.encode()).hexdigest() + + +async def _make_inviter(user_repo: SQLiteUserRepository, personal_data_uuid: UUID) -> UUID: + """`workspace_invitation.invited_by_user_id` FKs to a real `user` row, + which itself FKs to a real `personal_data` row — a bare `uuid4()` fails + both constraints, so tests that create an invitation attribute it to a + user backed by an already-seeded `PersonalData` row (`user.personal_ + data_uuid` is UNIQUE, so this only works once per `personal_data_uuid` + per test).""" + user = await user_repo.create( + User( + id=uuid4(), + personal_data_uuid=personal_data_uuid, + email=f"inviter-{uuid4()}@example.com", + hashed_password="not-a-real-hash", + created_at=datetime.now(), + ) + ) + return user.id + + class TestWorkspacePlanningMode: """Test planning_mode defaults and validation.""" @@ -328,3 +352,80 @@ class TestRemoveMember: await workspace_service.remove_member( seeded_ids.workspace_id, placeholder.uuid, seeded_ids.personal_data_uuid ) + + +class TestInvitationScoping: + """F2 (T1 authz audit): self-invitation used to grant full membership of + any workspace whose id was known, and `revoke_invitation` was unscoped. + """ + + async def test_revoke_invitation_from_a_different_workspace_raises_not_found( + self, + workspace_service: WorkspaceService, + seeded_ids: SeededIds, + user_repo: SQLiteUserRepository, + ) -> None: + """An invitation belonging to workspace B must not be revocable by + naming workspace A — closes F6/part of F2 (`revoke_invitation` + gained a `workspace_id` argument).""" + other_ws = await workspace_service.create_workspace("Other WS") + token = await workspace_service.invite_partner( + other_ws.id, await _make_inviter(user_repo, seeded_ids.personal_data_uuid) + ) + inv = await workspace_service._inv.get_by_token_hash(_hash(token)) + assert inv is not None + + # `seeded_ids.workspace_id` is NOT where the invitation lives. + with pytest.raises(NotFoundError): + await workspace_service.revoke_invitation(inv.id, seeded_ids.workspace_id) + + async def test_revoke_invitation_from_its_own_workspace_succeeds( + self, + workspace_service: WorkspaceService, + seeded_ids: SeededIds, + user_repo: SQLiteUserRepository, + ) -> None: + token = await workspace_service.invite_partner( + seeded_ids.workspace_id, await _make_inviter(user_repo, seeded_ids.personal_data_uuid) + ) + inv = await workspace_service._inv.get_by_token_hash(_hash(token)) + assert inv is not None + await workspace_service.revoke_invitation(inv.id, seeded_ids.workspace_id) + + refreshed = await workspace_service._inv.get_by_id(inv.id) + assert refreshed is not None + assert refreshed.status == "revoked" + + async def test_redeem_refuses_when_already_a_member_of_the_target_workspace( + self, + workspace_service: WorkspaceService, + seeded_ids: SeededIds, + user_repo: SQLiteUserRepository, + ) -> None: + """The exploit from F2: a caller who is ALREADY a member of a + workspace could invite themselves and "redeem" their own invite, + confirming nothing new but exercising a codepath that must still be + closed for a caller re-redeeming while already a member.""" + token = await workspace_service.invite_partner( + seeded_ids.workspace_id, await _make_inviter(user_repo, seeded_ids.personal_data_uuid) + ) + with pytest.raises(InvitationError, match="already belong"): + await workspace_service.redeem_invitation(token, seeded_ids.personal_data_uuid) + + async def test_redeem_refuses_when_workspace_already_has_two_members( + self, + workspace_service: WorkspaceService, + seeded_ids: SeededIds, + user_repo: SQLiteUserRepository, + ) -> None: + # Fill the target workspace to its 2-member cap first. + await workspace_service.add_placeholder_parent(seeded_ids.workspace_id, "Partner") + + other_ws = await workspace_service.create_workspace("Other WS") + other_pd = await workspace_service.add_placeholder_parent(other_ws.id, "Redeemer") + + token = await workspace_service.invite_partner( + seeded_ids.workspace_id, await _make_inviter(user_repo, seeded_ids.personal_data_uuid) + ) + with pytest.raises(InvitationError, match="maximum number of members"): + await workspace_service.redeem_invitation(token, other_pd.uuid) diff --git a/frontend/src/components/layout/Layout.test.tsx b/frontend/src/components/layout/Layout.test.tsx index 7c4316c..62c51a5 100644 --- a/frontend/src/components/layout/Layout.test.tsx +++ b/frontend/src/components/layout/Layout.test.tsx @@ -128,4 +128,20 @@ describe('Layout', () => { expect(logout).toHaveBeenCalled() expect(mockNavigate).toHaveBeenCalledWith('/auth', { replace: true }) }) + + it('still navigates to /auth when logout rejects (S3 — best-effort server call)', async () => { + const logout = vi.fn().mockRejectedValue(new Error('network down')) + mockUseAuth.mockReturnValue({ + accessToken: 'token', + isAuthenticated: true, + isLoading: false, + login: vi.fn(), + register: vi.fn(), + logout, + }) + renderLayout() + await userEvent.click(screen.getByTestId('logout-button')) + expect(logout).toHaveBeenCalled() + expect(mockNavigate).toHaveBeenCalledWith('/auth', { replace: true }) + }) }) diff --git a/frontend/src/components/layout/Layout.tsx b/frontend/src/components/layout/Layout.tsx index 2ac357b..f6ce319 100644 --- a/frontend/src/components/layout/Layout.tsx +++ b/frontend/src/components/layout/Layout.tsx @@ -19,8 +19,16 @@ export function Layout({ children, header }: LayoutProps): ReactNode { const selectedChild = childrenList.find((c) => c.id === selectedChildId) const handleLogout = useCallback(async () => { - await logout() - navigate('/auth', { replace: true }) + try { + await logout() + } catch { + // Best-effort server call — AuthContext already clears local + // token/state in its own `finally` even when this rejects. Swallow it + // here so a network failure during logout doesn't surface as an + // unhandled rejection; still navigate away below. + } finally { + navigate('/auth', { replace: true }) + } }, [logout, navigate]) const headerProps: HeaderProps = { diff --git a/frontend/src/context/AuthContext.test.tsx b/frontend/src/context/AuthContext.test.tsx index a422ef9..b10cbfc 100644 --- a/frontend/src/context/AuthContext.test.tsx +++ b/frontend/src/context/AuthContext.test.tsx @@ -122,6 +122,47 @@ describe('AuthContext', () => { expect(result.current.accessToken).toBeNull() }) + it('clears local state even when the server logout call rejects (S3 — offline logout)', async () => { + vi.mocked(api.refreshToken).mockResolvedValue(tokenResponse) + vi.mocked(api.logout).mockRejectedValue(new Error('network down')) + + const { result } = renderHook(() => useAuth(), { wrapper }) + await waitFor(() => expect(result.current.isAuthenticated).toBe(true)) + + // The rejection still propagates to the caller (Layout decides how to + // handle a failed best-effort server call) — but local state must + // already be cleared by the time it does. + await act(async () => { + await expect(result.current.logout()).rejects.toThrow('network down') + }) + + expect(api.setAccessToken).toHaveBeenCalledWith(null) + expect(result.current.isAuthenticated).toBe(false) + expect(result.current.accessToken).toBeNull() + }) + + it('logs the user out when apiFetch reports a failed refresh-and-retry (S1 interceptor wiring)', async () => { + vi.mocked(api.refreshToken).mockResolvedValue(tokenResponse) + + const { result } = renderHook(() => useAuth(), { wrapper }) + await waitFor(() => expect(result.current.isAuthenticated).toBe(true)) + + // Simulate services/api.ts's apiFetch invoking the handler AuthContext + // registered via api.setUnauthorizedHandler after a mid-session 401's + // refresh-and-retry gives up. + const registeredHandler = vi.mocked(api.setUnauthorizedHandler).mock.calls.find( + (call) => call[0] !== null, + )?.[0] + expect(registeredHandler).toBeInstanceOf(Function) + + act(() => { + registeredHandler?.() + }) + + expect(result.current.isAuthenticated).toBe(false) + expect(result.current.accessToken).toBeNull() + }) + it('throws when useAuth is used outside an AuthProvider', () => { expect(() => renderHook(() => useAuth())).toThrow( 'useAuth must be used inside ', diff --git a/frontend/src/context/AuthContext.tsx b/frontend/src/context/AuthContext.tsx index d42107f..8d8621f 100644 --- a/frontend/src/context/AuthContext.tsx +++ b/frontend/src/context/AuthContext.tsx @@ -42,6 +42,18 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { }) }, []) + // apiFetch's own refresh-and-retry (see services/api.ts) handles a 401 hit + // mid-session by refreshing once and retrying the original request. If + // that refresh itself fails, this is the hook that actually logs the user + // out — without it, isAuthenticated would stay true forever while every + // subsequent request 401s. + useEffect(() => { + api.setUnauthorizedHandler(() => { + setState({ accessToken: null, isAuthenticated: false, isLoading: false }) + }) + return () => api.setUnauthorizedHandler(null) + }, []) + const handleTokenResponse = useCallback((res: TokenResponse) => { api.setAccessToken(res.access_token) setState({ accessToken: res.access_token, isAuthenticated: true, isLoading: false }) @@ -64,9 +76,15 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { ) const logout = useCallback(async () => { - await api.logout() - api.setAccessToken(null) - setState({ accessToken: null, isAuthenticated: false, isLoading: false }) + // Clear local state even if the server call fails (offline logout must + // still work) — but still let the failure propagate to the caller so it + // can decide how to handle a best-effort server call that didn't land. + try { + await api.logout() + } finally { + api.setAccessToken(null) + setState({ accessToken: null, isAuthenticated: false, isLoading: false }) + } }, []) return ( diff --git a/frontend/src/context/WorkspaceContext.test.tsx b/frontend/src/context/WorkspaceContext.test.tsx index 06f3417..6bd0f93 100644 --- a/frontend/src/context/WorkspaceContext.test.tsx +++ b/frontend/src/context/WorkspaceContext.test.tsx @@ -217,7 +217,12 @@ describe('WorkspaceContext', () => { expect(result.current.selectedChildId).toBe('c2') }) - it('setWorkspaceId stores the id and updates state', async () => { + it('setWorkspaceId stores the id, updates state, and fetches that workspace directly', async () => { + // P2 in the frontend audit: this used to rely on a reactive effect that + // only fired while `workspace` was still null, so switching workspaces + // after the initial load silently never fetched the new one. Fetching + // explicitly here fixes that without reintroducing the cold-load + // duplicate GET /workspace the same effect used to cause. vi.mocked(useAuth).mockReturnValue(authenticated) vi.mocked(api.getUserContext).mockResolvedValue(userContext) vi.mocked(api.getWorkspace).mockResolvedValue(workspace) @@ -225,12 +230,19 @@ describe('WorkspaceContext', () => { const { result } = renderHook(() => useWorkspace(), { wrapper }) await waitFor(() => expect(result.current.workspace).not.toBeNull()) + const otherWorkspace: Workspace = { ...workspace, id: 'ws2', name: 'Other workspace' } + vi.mocked(api.getWorkspace).mockResolvedValue(otherWorkspace) + vi.mocked(api.getWorkspace).mockClear() + act(() => { result.current.setWorkspaceId('ws2') }) expect(result.current.workspaceId).toBe('ws2') expect(localStorage.getItem('workspaceId')).toBe('ws2') + + await waitFor(() => expect(api.getWorkspace).toHaveBeenCalledWith('ws2')) + await waitFor(() => expect(result.current.workspace).toEqual(otherWorkspace)) }) it('refetch re-fetches the current workspace', async () => { @@ -322,6 +334,38 @@ describe('WorkspaceContext', () => { expect(result.current.workspace).toEqual(otherWorkspace) }) + it('discards a refreshContext response that resolves after logout (S4)', async () => { + vi.mocked(useAuth).mockReturnValue(authenticated) + let resolveContext: (value: api.UserContext) => void = () => {} + const pending = new Promise((resolve) => { + resolveContext = resolve + }) + vi.mocked(api.getUserContext).mockReturnValue(pending) + + const { result, rerender } = renderHook(() => useWorkspace(), { wrapper }) + + // Logout happens while the initial refreshContext() call is still + // in flight (e.g. a slow /auth/context response). + vi.mocked(useAuth).mockReturnValue(unauthenticated) + rerender() + + await waitFor(() => expect(result.current.workspace).toBeNull()) + expect(result.current.workspaceId).toBeNull() + + // The stale response arrives after logout — it must be discarded + // rather than repopulating this account's context. + await act(async () => { + resolveContext(userContext) + await pending + }) + + expect(result.current.workspaceId).toBeNull() + expect(result.current.parentName).toBeNull() + expect(result.current.personalDataUuid).toBeNull() + expect(result.current.children).toEqual([]) + expect(localStorage.getItem('workspaceId')).toBeNull() + }) + it('throws when useWorkspace is used outside a WorkspaceProvider', () => { vi.mocked(useAuth).mockReturnValue(unauthenticated) diff --git a/frontend/src/context/WorkspaceContext.tsx b/frontend/src/context/WorkspaceContext.tsx index 23eca74..950041d 100644 --- a/frontend/src/context/WorkspaceContext.tsx +++ b/frontend/src/context/WorkspaceContext.tsx @@ -43,25 +43,38 @@ export function WorkspaceProvider({ children: childrenProp }: { children: React. // absence) of cached data — see the effect for why that distinction is // what makes a login swap actually refetch. const wasAuthenticatedRef = useRef(false) + // Bumped by clearWorkspaceState() (logout). A refreshContext/fetchWorkspace + // call captures the generation it started with; if that generation is + // stale by the time the response arrives (logout happened mid-flight), + // the response is discarded instead of repopulating state for the + // account that just logged out — see S4 in the frontend audit. + const generationRef = useRef(0) const fetchWorkspace = useCallback(async (id: string) => { + const generation = generationRef.current setIsLoading(true) setError(null) try { const ws = await api.getWorkspace(id) + if (generation !== generationRef.current) return setWorkspace(ws) } catch (err: unknown) { + if (generation !== generationRef.current) return setError(err instanceof Error ? err.message : 'Failed to load workspace') setWorkspace(null) } finally { - setIsLoading(false) + if (generation === generationRef.current) { + setIsLoading(false) + } } }, []) const refreshContext = useCallback(async () => { + const generation = generationRef.current setIsLoading(true) try { const context = await api.getUserContext() + if (generation !== generationRef.current) return localStorage.setItem('workspaceId', context.workspace_id) setWorkspaceId(context.workspace_id) setPersonalDataUuid(context.personal_data_uuid) @@ -73,15 +86,20 @@ export function WorkspaceProvider({ children: childrenProp }: { children: React. } try { const ws = await api.getWorkspace(context.workspace_id) + if (generation !== generationRef.current) return setWorkspace(ws) } catch (err: unknown) { + if (generation !== generationRef.current) return setError(err instanceof Error ? err.message : 'Failed to load workspace') setWorkspace(null) } } catch (err: unknown) { + if (generation !== generationRef.current) return setError(err instanceof Error ? err.message : 'Failed to load user context') } finally { - setIsLoading(false) + if (generation === generationRef.current) { + setIsLoading(false) + } } }, []) @@ -92,6 +110,7 @@ export function WorkspaceProvider({ children: childrenProp }: { children: React. // one account's data could otherwise flash under another account's // session. const clearWorkspaceState = useCallback(() => { + generationRef.current += 1 localStorage.removeItem('workspaceId') setWorkspace(null) setWorkspaceId(null) @@ -123,16 +142,22 @@ export function WorkspaceProvider({ children: childrenProp }: { children: React. } }, [isAuthenticated, refreshContext, clearWorkspaceState]) - useEffect(() => { - if (isAuthenticated && workspaceId && !workspace) { - void fetchWorkspace(workspaceId) - } - }, [isAuthenticated, workspaceId, workspace, fetchWorkspace]) - - const handleSetWorkspaceId = useCallback((id: string) => { - localStorage.setItem('workspaceId', id) - setWorkspaceId(id) - }, []) + // NOTE (P2 in the frontend audit): a cold login already fetches the + // workspace once inside refreshContext() above. This used to be a + // reactive `useEffect` on `[isAuthenticated, workspaceId, workspace]` that + // fired a *second* GET /workspace as soon as `workspaceId` state landed + // but before `workspace` itself did — a duplicate cold-load request. + // Fetching explicitly here, only when the id is set imperatively (e.g. a + // future workspace switcher), removes that duplicate without losing the + // ability to load a workspace picked outside the login flow. + const handleSetWorkspaceId = useCallback( + (id: string) => { + localStorage.setItem('workspaceId', id) + setWorkspaceId(id) + void fetchWorkspace(id) + }, + [fetchWorkspace], + ) const handleSelectChild = useCallback((id: string) => { setSelectedChildId(id) diff --git a/frontend/src/hooks/useAnalytics.test.ts b/frontend/src/hooks/useAnalytics.test.ts index 645c8cf..0871bd4 100644 --- a/frontend/src/hooks/useAnalytics.test.ts +++ b/frontend/src/hooks/useAnalytics.test.ts @@ -3,6 +3,7 @@ import { renderHook, act } from '@testing-library/react' import { useAnalytics } from './useAnalytics' import * as api from '@/services/api' import { makeWorkspaceOverview } from '../../tests/fixtures' +import type { WorkspaceOverview } from '@/types' vi.mock('@/services/api') @@ -123,4 +124,59 @@ describe('useAnalytics', () => { expect(result.current.data).toBeNull() }) + + it('discards a stale response for a previous childId (P1 stale-response guard)', async () => { + let resolveFirst: (value: WorkspaceOverview) => void = () => {} + let resolveSecond: (value: WorkspaceOverview) => void = () => {} + const firstPromise = new Promise((resolve) => { + resolveFirst = resolve + }) + const secondPromise = new Promise((resolve) => { + resolveSecond = resolve + }) + + vi.mocked(api.getWorkspaceOverview) + .mockReturnValueOnce(firstPromise) + .mockReturnValueOnce(secondPromise) + + const { result, rerender } = renderHook( + ({ childId }: { childId: string }) => useAnalytics('ws1', childId), + { initialProps: { childId: 'child1' } }, + ) + + // Switching childId while the first (slower) request is still in + // flight fires a second request for the new child. + rerender({ childId: 'child2' }) + + const child1Data = makeWorkspaceOverview({ + workspaceId: 'ws1', + childId: 'child1', + parentProjections: [], + combinedDaysUsed: 1, + combinedDaysLeft: 1, + combinedExpiringAtB4: 0, + }) + const child2Data = makeWorkspaceOverview({ + workspaceId: 'ws1', + childId: 'child2', + parentProjections: [], + combinedDaysUsed: 5, + combinedDaysLeft: 5, + combinedExpiringAtB4: 0, + }) + + // The newer (child2) request resolves first... + await act(async () => { + resolveSecond(child2Data) + await secondPromise + }) + expect(result.current.data).toEqual(child2Data) + + // ...then the stale child1 response arrives late and must not overwrite it. + await act(async () => { + resolveFirst(child1Data) + await firstPromise + }) + expect(result.current.data).toEqual(child2Data) + }) }) diff --git a/frontend/src/hooks/useAnalytics.ts b/frontend/src/hooks/useAnalytics.ts index 4ab3f75..1c8e2af 100644 --- a/frontend/src/hooks/useAnalytics.ts +++ b/frontend/src/hooks/useAnalytics.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { WorkspaceOverview } from '@/types' interface UseAnalyticsResult { @@ -13,8 +14,10 @@ export function useAnalytics(workspaceId: string | null, childId?: string): UseA const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!workspaceId || !childId) { // The identity this data belongs to is gone (e.g. a logout cleared // the workspace/child selection) — clear rather than leave the @@ -27,14 +30,18 @@ export function useAnalytics(workspaceId: string | null, childId?: string): UseA setError(null) try { const response = await api.getWorkspaceOverview(workspaceId, childId) + if (!isCurrent(requestId)) return setData(response) } catch (err) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to fetch analytics') setData(null) } finally { - setIsLoading(false) + if (isCurrent(requestId)) { + setIsLoading(false) + } } - }, [workspaceId, childId]) + }, [workspaceId, childId, start, isCurrent]) useEffect(() => { fetch() diff --git a/frontend/src/hooks/useCalendarDays.ts b/frontend/src/hooks/useCalendarDays.ts index 390e1bc..5027800 100644 --- a/frontend/src/hooks/useCalendarDays.ts +++ b/frontend/src/hooks/useCalendarDays.ts @@ -15,6 +15,7 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { CalendarWindow } from '@/types' interface UseCalendarDaysResult { @@ -32,20 +33,24 @@ export function useCalendarDays( const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetchWindow = useCallback(async () => { + const requestId = start() if (!workspaceId) return setIsLoading(true) setError(null) try { const result = await api.getCalendarDays(workspaceId, startYear, years) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load calendar days') } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [workspaceId, startYear, years]) + }, [workspaceId, startYear, years, start, isCurrent]) useEffect(() => { void fetchWindow() diff --git a/frontend/src/hooks/useChildSettings.ts b/frontend/src/hooks/useChildSettings.ts index 322b8a6..e02b8ce 100644 --- a/frontend/src/hooks/useChildSettings.ts +++ b/frontend/src/hooks/useChildSettings.ts @@ -6,6 +6,7 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { Child } from '@/types' export interface ChildSettingsUpdatePayload { @@ -24,8 +25,10 @@ export function useChildSettings(childId: string | null): UseChildSettingsResult const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!childId) { setData(null) return @@ -34,13 +37,15 @@ export function useChildSettings(childId: string | null): UseChildSettingsResult setError(null) try { const result = await api.getChild(childId) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load child') } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [childId]) + }, [childId, start, isCurrent]) useEffect(() => { void fetch() diff --git a/frontend/src/hooks/useDayOverrides.ts b/frontend/src/hooks/useDayOverrides.ts index 862158d..feb7ed2 100644 --- a/frontend/src/hooks/useDayOverrides.ts +++ b/frontend/src/hooks/useDayOverrides.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { DayOverride, DayOverrideCreateRequest } from '@/types' interface UseDayOverridesResult { @@ -14,20 +15,24 @@ export function useDayOverrides(workspaceId: string): UseDayOverridesResult { const [data, setData] = useState([]) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!workspaceId) return setIsLoading(true) setError(null) try { const result = await api.listDayOverridesByWorkspace(workspaceId) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load day overrides') } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [workspaceId]) + }, [workspaceId, start, isCurrent]) useEffect(() => { void fetch() diff --git a/frontend/src/hooks/useLatestRequest.ts b/frontend/src/hooks/useLatestRequest.ts new file mode 100644 index 0000000..7c7e7d7 --- /dev/null +++ b/frontend/src/hooks/useLatestRequest.ts @@ -0,0 +1,41 @@ +import { useCallback, useRef } from 'react' + +interface UseLatestRequestResult { + /** Call at the start of a fetch to obtain this call's request id. */ + start: () => number + /** True if `id` is still the most recently started request — i.e. no + * newer call to `start()` has happened since. Guard every `setState` in + * an async fetch with this so a stale response (e.g. from a fetch kicked + * off for a since-replaced childId/workspaceId) can't overwrite state a + * newer, faster request already set. */ + isCurrent: (id: number) => boolean +} + +/** + * Shared stale-response guard for the data-fetching hooks in this + * directory. Each hook has no cache/dedup layer (no TanStack Query), so + * switching identity (e.g. the selected child) while a fetch is in flight + * can otherwise let an older, slower response land after — and overwrite — + * a newer one. See P1 in the frontend audit. + * + * Usage: + * const { start, isCurrent } = useLatestRequest() + * const fetch = useCallback(async () => { + * const requestId = start() + * const result = await api.get(...) + * if (!isCurrent(requestId)) return + * setData(result) + * }, [...]) + */ +export function useLatestRequest(): UseLatestRequestResult { + const idRef = useRef(0) + + const start = useCallback((): number => { + idRef.current += 1 + return idRef.current + }, []) + + const isCurrent = useCallback((id: number): boolean => id === idRef.current, []) + + return { start, isCurrent } +} diff --git a/frontend/src/hooks/useLeaveWishes.ts b/frontend/src/hooks/useLeaveWishes.ts index 074b921..3524e4b 100644 --- a/frontend/src/hooks/useLeaveWishes.ts +++ b/frontend/src/hooks/useLeaveWishes.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { LeaveWish, LeaveWishCreateRequest, LeaveWishUpdateRequest } from '@/types' interface UseLeaveWishesResult { @@ -34,9 +35,11 @@ export function useLeaveWishes(workspaceId: string | null): UseLeaveWishesResult const [isLoading, setIsLoading] = useState(false) const [isRefreshing, setIsRefreshing] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback( async (options?: { background?: boolean }) => { + const requestId = start() if (!workspaceId) return const background = options?.background ?? false const setBusy = background ? setIsRefreshing : setIsLoading @@ -44,14 +47,16 @@ export function useLeaveWishes(workspaceId: string | null): UseLeaveWishesResult setError(null) try { const result = await api.getLeaveWishes(workspaceId) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load leave wishes') } finally { - setBusy(false) + if (isCurrent(requestId)) setBusy(false) } }, - [workspaceId], + [workspaceId, start, isCurrent], ) useEffect(() => { diff --git a/frontend/src/hooks/useOptimizationTips.ts b/frontend/src/hooks/useOptimizationTips.ts index 6e48758..2ce8267 100644 --- a/frontend/src/hooks/useOptimizationTips.ts +++ b/frontend/src/hooks/useOptimizationTips.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { Tip } from '@/types' interface UseOptimizationTipsResult { @@ -15,21 +16,25 @@ export function useOptimizationTips(workspaceId: string | null): UseOptimization const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) const [dismissed, setDismissed] = useState>(new Set()) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!workspaceId) return setIsLoading(true) setError(null) try { const response = await api.getTips(workspaceId) + if (!isCurrent(requestId)) return setData(response) } catch (err) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to fetch tips') setData(null) } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [workspaceId]) + }, [workspaceId, start, isCurrent]) const dismiss = useCallback( async (tipId: string) => { diff --git a/frontend/src/hooks/useParentalLeave.ts b/frontend/src/hooks/useParentalLeave.ts index 79591e3..8d64115 100644 --- a/frontend/src/hooks/useParentalLeave.ts +++ b/frontend/src/hooks/useParentalLeave.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { ParentalLeaveFetch } from '@/types' interface UseParentalLeaveResult { @@ -16,8 +17,10 @@ export function useParentalLeave( const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!personalDataUuid || !childId) { // The identity this data belongs to is gone (e.g. a logout cleared // the parent/child selection) — clear rather than leave the previous @@ -30,14 +33,16 @@ export function useParentalLeave( setError(null) try { const response = await api.getParentalLeave(personalDataUuid, childId) + if (!isCurrent(requestId)) return setData(response) } catch (err) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to fetch parental leave data') setData(null) } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [personalDataUuid, childId]) + }, [personalDataUuid, childId, start, isCurrent]) useEffect(() => { fetch() diff --git a/frontend/src/hooks/usePlannedDays.test.ts b/frontend/src/hooks/usePlannedDays.test.ts index c3902ec..df2b606 100644 --- a/frontend/src/hooks/usePlannedDays.test.ts +++ b/frontend/src/hooks/usePlannedDays.test.ts @@ -339,6 +339,65 @@ describe('usePlannedDays', () => { expect(updateError?.message).toBe('Update failed') }) + it('discards a stale response for a previous childId (P1 stale-response guard)', async () => { + let resolveFirst: (value: ReturnType[]) => void = () => {} + let resolveSecond: (value: ReturnType[]) => void = () => {} + const firstPromise = new Promise[]>((resolve) => { + resolveFirst = resolve + }) + const secondPromise = new Promise[]>((resolve) => { + resolveSecond = resolve + }) + + vi.mocked(api.getPlannedDays).mockReturnValueOnce(firstPromise).mockReturnValueOnce(secondPromise) + + const { result, rerender } = renderHook( + ({ childId }: { childId: string }) => usePlannedDays(childId), + { initialProps: { childId: 'child1' } }, + ) + + // Switching childId while the first (slower) request is still in + // flight fires a second request for the new child. + rerender({ childId: 'child2' }) + + const child1Data = [ + makePlannedLeaveDay({ + id: 'p1', + workspaceId: 'ws1', + childId: 'child1', + parentPersonalDataUuid: 'parent1', + date: '2026-06-01', + dayType: 'vacation', + isDubbeldag: false, + }), + ] + const child2Data = [ + makePlannedLeaveDay({ + id: 'p2', + workspaceId: 'ws1', + childId: 'child2', + parentPersonalDataUuid: 'parent1', + date: '2026-07-01', + dayType: 'vacation', + isDubbeldag: false, + }), + ] + + // The newer (child2) request resolves first... + await act(async () => { + resolveSecond(child2Data) + await secondPromise + }) + expect(result.current.data).toEqual(child2Data) + + // ...then the stale child1 response arrives late and must not overwrite it. + await act(async () => { + resolveFirst(child1Data) + await firstPromise + }) + expect(result.current.data).toEqual(child2Data) + }) + it('handles remove error gracefully', async () => { vi.mocked(api.getPlannedDays).mockResolvedValue([]) vi.mocked(api.deletePlannedDay).mockRejectedValue(new Error('Delete failed')) diff --git a/frontend/src/hooks/usePlannedDays.ts b/frontend/src/hooks/usePlannedDays.ts index e4fa1f1..1bcd0ff 100644 --- a/frontend/src/hooks/usePlannedDays.ts +++ b/frontend/src/hooks/usePlannedDays.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { PlannedLeaveDay } from '@/types' interface UsePlannedDaysResult { @@ -20,8 +21,10 @@ export function usePlannedDays(childId?: string): UsePlannedDaysResult { const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!childId) { // The child this data belongs to is gone (e.g. a logout cleared the // selection) — clear rather than leave the previous account's @@ -34,16 +37,18 @@ export function usePlannedDays(childId?: string): UsePlannedDaysResult { setError(null) try { const response = await api.getPlannedDays(childId) - setData(response) + if (isCurrent(requestId)) setData(response) return response } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to fetch planned days') - setData(null) + if (isCurrent(requestId)) { + setError(err instanceof Error ? err.message : 'Failed to fetch planned days') + setData(null) + } return null } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [childId]) + }, [childId, start, isCurrent]) const add = useCallback( async (payload: api.CreatePlannedDayPayload) => { diff --git a/frontend/src/hooks/useScraperFetch.test.ts b/frontend/src/hooks/useScraperFetch.test.ts index 9170869..ba30dbd 100644 --- a/frontend/src/hooks/useScraperFetch.test.ts +++ b/frontend/src/hooks/useScraperFetch.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest' import { renderHook, act } from '@testing-library/react' import { useScraperFetch } from './useScraperFetch' +import type { PollResponse } from './useScraperFetch' import * as api from '@/services/api' import * as errors from '@/services/errors' import { makeScraperSessionStartResponse, makePollResponse } from '../../tests/fixtures' @@ -335,6 +336,81 @@ describe('useScraperFetch', () => { expect(result.current.sessionId).toBeNull() }) + it('skips a poll tick while the previous one is still in flight (P4 overlap guard)', async () => { + vi.mocked(api.startScraperSession).mockResolvedValue( + makeScraperSessionStartResponse({ + session_id: 'sess-123', + workspace_id: 'ws-1', + personal_data_uuid: 'pd-1', + child_id: 'c-1', + }) + ) + + let resolvePoll: (value: PollResponse) => void = () => {} + const pending = new Promise((resolve) => { + resolvePoll = resolve + }) + vi.mocked(api.pollScraperSession).mockReturnValueOnce(pending) + + const { result } = renderHook(() => useScraperFetch()) + + await act(async () => { + await result.current.start() + }) + + // First tick starts the still-pending poll. + await act(async () => { + vi.advanceTimersByTime(1000) + }) + expect(api.pollScraperSession).toHaveBeenCalledTimes(1) + + // A second tick fires while the first poll is still in flight — it must + // be skipped rather than stacking a second concurrent request. + await act(async () => { + vi.advanceTimersByTime(1000) + }) + expect(api.pollScraperSession).toHaveBeenCalledTimes(1) + + // Resolve the first poll; the next tick is then free to run normally. + vi.mocked(api.pollScraperSession).mockResolvedValue(makePollResponse({ status: 'awaiting_scan' })) + await act(async () => { + resolvePoll(makePollResponse({ status: 'authenticating' })) + await pending + }) + + await act(async () => { + vi.advanceTimersByTime(1000) + }) + expect(api.pollScraperSession).toHaveBeenCalledTimes(2) + }) + + it('clears the polling interval on unmount (P4)', async () => { + vi.mocked(api.startScraperSession).mockResolvedValue( + makeScraperSessionStartResponse({ + session_id: 'sess-123', + workspace_id: 'ws-1', + personal_data_uuid: 'pd-1', + child_id: 'c-1', + }) + ) + vi.mocked(api.pollScraperSession).mockResolvedValue(makePollResponse({ status: 'awaiting_scan' })) + + const { result, unmount } = renderHook(() => useScraperFetch()) + + await act(async () => { + await result.current.start() + }) + + unmount() + vi.mocked(api.pollScraperSession).mockClear() + + await act(async () => { + vi.advanceTimersByTime(5000) + }) + + expect(api.pollScraperSession).not.toHaveBeenCalled() + }) + it('handles poll error', async () => { vi.mocked(api.startScraperSession).mockResolvedValue( makeScraperSessionStartResponse({ diff --git a/frontend/src/hooks/useScraperFetch.ts b/frontend/src/hooks/useScraperFetch.ts index f2f9723..ea0c871 100644 --- a/frontend/src/hooks/useScraperFetch.ts +++ b/frontend/src/hooks/useScraperFetch.ts @@ -1,4 +1,4 @@ -import { useState, useRef, useCallback } from 'react' +import { useState, useRef, useCallback, useEffect } from 'react' import * as api from '@/services/api' import { isUnauthorized } from '@/services/errors' @@ -41,6 +41,10 @@ export function useScraperFetch(): UseScraperFetchResult { const [unauthorizedError, setUnauthorizedError] = useState(false) const intervalRef = useRef | null>(null) + // Guards against overlapping ticks: if a poll response hasn't come back + // yet when the next 1s tick fires (a slow/backed-up server), skip that + // tick rather than stacking a second in-flight request on top of it. + const pollInFlightRef = useRef(false) const stopPolling = useCallback(() => { if (intervalRef.current) { @@ -55,6 +59,8 @@ export function useScraperFetch(): UseScraperFetchResult { const startPolling = useCallback((sid: string) => { stopPolling() intervalRef.current = setInterval(async () => { + if (pollInFlightRef.current) return + pollInFlightRef.current = true try { const data = await api.pollScraperSession(sid) setPollData(data as PollResponse) @@ -64,10 +70,19 @@ export function useScraperFetch(): UseScraperFetchResult { } catch (err) { setError(err instanceof Error ? err.message : 'Poll error') stopPolling() + } finally { + pollInFlightRef.current = false } }, POLL_INTERVAL_MS) }, [stopPolling]) + // The interval otherwise outlives the component: a poll response arriving + // after unmount would still call setState, and the timer itself would + // keep firing (and keep holding the closure alive) forever. + useEffect(() => { + return () => stopPolling() + }, [stopPolling]) + const start = useCallback( async (childId?: string) => { if (sessionId && pollData && !isTerminalStatus(pollData.status)) { diff --git a/frontend/src/hooks/useVacationDays.ts b/frontend/src/hooks/useVacationDays.ts index 9f4ddcc..7aa1f4f 100644 --- a/frontend/src/hooks/useVacationDays.ts +++ b/frontend/src/hooks/useVacationDays.ts @@ -10,6 +10,7 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { VacationDays, VacationDaysCreateRequest } from '@/types' interface UseVacationDaysResult { @@ -23,20 +24,24 @@ export function useVacationDays(workspaceId: string): UseVacationDaysResult { const [data, setData] = useState([]) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!workspaceId) return setIsLoading(true) setError(null) try { const result = await api.getVacationDaysByWorkspace(workspaceId) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load vacation days') } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [workspaceId]) + }, [workspaceId, start, isCurrent]) useEffect(() => { void fetch() diff --git a/frontend/src/hooks/useWorkspaceMembers.ts b/frontend/src/hooks/useWorkspaceMembers.ts index f079cb9..e6f7b79 100644 --- a/frontend/src/hooks/useWorkspaceMembers.ts +++ b/frontend/src/hooks/useWorkspaceMembers.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useState } from 'react' import * as api from '@/services/api' +import { useLatestRequest } from './useLatestRequest' import type { PersonalData } from '@/types' interface UseWorkspaceMembersResult { @@ -14,20 +15,24 @@ export function useWorkspaceMembers(workspaceId: string | null): UseWorkspaceMem const [data, setData] = useState([]) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) + const { start, isCurrent } = useLatestRequest() const fetch = useCallback(async () => { + const requestId = start() if (!workspaceId) return setIsLoading(true) setError(null) try { const result = await api.listWorkspaceMembers(workspaceId) + if (!isCurrent(requestId)) return setData(result) } catch (err: unknown) { + if (!isCurrent(requestId)) return setError(err instanceof Error ? err.message : 'Failed to load workspace members') } finally { - setIsLoading(false) + if (isCurrent(requestId)) setIsLoading(false) } - }, [workspaceId]) + }, [workspaceId, start, isCurrent]) useEffect(() => { void fetch() diff --git a/frontend/src/services/api.test.ts b/frontend/src/services/api.test.ts index 552ed1d..3f2234e 100644 --- a/frontend/src/services/api.test.ts +++ b/frontend/src/services/api.test.ts @@ -1774,6 +1774,114 @@ describe('endpoint functions (representative coverage)', () => { }) }) +describe('apiFetch — 401 refresh-and-retry (S1)', () => { + let fetchMock: ReturnType + + beforeEach(() => { + fetchMock = vi.fn() + globalThis.fetch = fetchMock as typeof fetch + }) + + afterEach(() => { + vi.clearAllMocks() + api.setAccessToken(null) + api.setUnauthorizedHandler(null) + }) + + function unauthorizedResponse(detail = 'Unauthorized') { + return { ok: false, status: 401, statusText: 'Unauthorized', json: async () => ({ detail }) } + } + + function tokenResponse(access_token: string) { + return { + ok: true, + status: 200, + json: async () => ({ access_token, token_type: 'bearer', expires_in: 3600 }), + } + } + + it('refreshes the token once and retries the original request after a 401', async () => { + api.setAccessToken('old-token') + fetchMock + .mockResolvedValueOnce(unauthorizedResponse()) + .mockResolvedValueOnce(tokenResponse('new-token')) + .mockResolvedValueOnce({ ok: true, status: 200, json: async () => [] }) + + const result = await api.getTips('ws1') + + expect(result).toEqual([]) + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(fetchMock.mock.calls[1]![0]).toBe('/api/auth/refresh') + const [retryUrl, retryOptions] = fetchMock.mock.calls[2] as [string, RequestInit] + expect(retryUrl).toBe('/api/tips/ws1') + expect((retryOptions.headers as Record)['Authorization']).toBe('Bearer new-token') + }) + + it('dedupes concurrent 401s from parallel requests into a single refresh call', async () => { + api.setAccessToken('old-token') + fetchMock.mockImplementation(async (url: string, options?: RequestInit) => { + if (url === '/api/auth/refresh') { + return tokenResponse('new-token') + } + const headers = options?.headers as Record | undefined + if (headers?.['Authorization'] === 'Bearer new-token') { + return { ok: true, status: 200, json: async () => [] } + } + return unauthorizedResponse() + }) + + const [r1, r2] = await Promise.all([api.getTips('ws1'), api.getTips('ws1')]) + + expect(r1).toEqual([]) + expect(r2).toEqual([]) + const refreshCalls = fetchMock.mock.calls.filter(([url]) => url === '/api/auth/refresh') + expect(refreshCalls).toHaveLength(1) + }) + + it('clears the token, notifies the unauthorized handler, and surfaces the original 401 when refresh fails', async () => { + api.setAccessToken('old-token') + const onUnauthorized = vi.fn() + api.setUnauthorizedHandler(onUnauthorized) + fetchMock + .mockResolvedValueOnce(unauthorizedResponse('session expired')) + .mockResolvedValueOnce(unauthorizedResponse('refresh token expired')) + + await expect(api.getTips('ws1')).rejects.toMatchObject({ + status: 401, + error: { message: 'session expired' }, + }) + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(onUnauthorized).toHaveBeenCalledTimes(1) + }) + + it('does not loop when the retried request also 401s', async () => { + api.setAccessToken('old-token') + fetchMock + .mockResolvedValueOnce(unauthorizedResponse()) + .mockResolvedValueOnce(tokenResponse('new-token')) + .mockResolvedValueOnce(unauthorizedResponse('still unauthorized')) + + await expect(api.getTips('ws1')).rejects.toMatchObject({ + status: 401, + error: { message: 'still unauthorized' }, + }) + + // Original + refresh + one retry — no second refresh attempt. + expect(fetchMock).toHaveBeenCalledTimes(3) + }) + + it('does not attempt a refresh-and-retry for the auth endpoints themselves', async () => { + fetchMock.mockResolvedValue(unauthorizedResponse()) + + await expect(api.login({ email: 'a@b.com', password: 'x' })).rejects.toMatchObject({ status: 401 }) + await expect(api.refreshToken()).rejects.toMatchObject({ status: 401 }) + await expect(api.logout()).rejects.toMatchObject({ status: 401 }) + + expect(fetchMock).toHaveBeenCalledTimes(3) + }) +}) + describe('apiFetch — structured {message_key, data} errors', () => { let fetchMock: ReturnType diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 36ece1a..d0bf8dc 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -520,9 +520,30 @@ export function setAccessToken(token: string | null): void { _accessToken = token } +// Endpoints that must never trigger a refresh-and-retry on 401 — retrying +// them would either be nonsensical (the refresh call retrying itself) or +// mask the real "not logged in" outcome (login/logout). +const AUTH_NO_RETRY_PATHS = new Set(['/auth/login', '/auth/refresh', '/auth/logout']) + +// Dedupe concurrent 401s (e.g. Dashboard's ~7 parallel requests all +// expiring at once) into a single in-flight refresh call, shared across +// every apiFetch caller. +let refreshPromise: Promise | null = null + +// AuthContext registers this so a refresh-and-retry failure can log the +// user out immediately, instead of leaving `isAuthenticated: true` while +// every subsequent request 401s. Kept out of this module so api.ts stays +// framework-agnostic. +let onUnauthorized: (() => void) | null = null + +export function setUnauthorizedHandler(handler: (() => void) | null): void { + onUnauthorized = handler +} + async function apiFetch( path: string, options: RequestInit = {}, + isRetry = false, ): Promise { const headers: Record = { 'Content-Type': 'application/json', @@ -532,6 +553,31 @@ async function apiFetch( headers['Authorization'] = `Bearer ${_accessToken}` } const response = await fetch(`${BASE}${path}`, { ...options, headers }) + + if (response.status === 401 && !isRetry && !AUTH_NO_RETRY_PATHS.has(path)) { + let refreshed: TokenResponse | null = null + try { + refreshPromise ??= refreshToken().finally(() => { + refreshPromise = null + }) + refreshed = await refreshPromise + } catch { + // Refresh itself failed (expired/invalid refresh cookie, or a + // network error) — clear the token and let the caller's original + // 401 surface below so AuthContext can log the user out. + setAccessToken(null) + onUnauthorized?.() + } + if (refreshed) { + setAccessToken(refreshed.access_token) + // Retry exactly once, outside the try/catch above — `isRetry: true` + // means a 401 on this attempt is final and must propagate as-is + // rather than being caught here and re-reported as the original + // (now stale) 401. + return await apiFetch(path, options, true) + } + } + if (!response.ok) { let error: { code: string; message: string; details?: Record } = { code: String(response.status),