diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..5dfda46 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,16 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + + - package-ecosystem: "npm" + directory: "/frontend" + schedule: + interval: "weekly" + + - package-ecosystem: "uv" + directory: "/backend" + schedule: + interval: "weekly" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47d3869..d4d23ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,9 @@ on: branches: [main, develop] pull_request: +permissions: + contents: read + jobs: # --------------------------------------------------------------------------- # Backend: ruff lint + pytest with coverage gate (rises as junior tests land) @@ -17,14 +20,14 @@ jobs: working-directory: backend steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: astral-sh/setup-uv@v5 + - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5 with: version: "latest" - name: Install dependencies - run: uv sync + run: uv sync --locked - name: Lint (ruff) run: uv run ruff check . @@ -47,9 +50,9 @@ jobs: working-directory: frontend steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: "npm" @@ -81,23 +84,23 @@ jobs: continue-on-error: true steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: astral-sh/setup-uv@v5 + - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5 with: version: "latest" - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" - name: Install backend dependencies working-directory: backend - run: uv sync + run: uv sync --locked - name: Install frontend dependencies working-directory: frontend - run: npm install + run: npm ci - name: Install Playwright browsers working-directory: frontend @@ -108,8 +111,8 @@ jobs: run: npm run e2e - name: Upload Playwright report - if: always() - uses: actions/upload-artifact@v4 + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: playwright-report path: frontend/playwright-report/ diff --git a/.gitignore b/.gitignore index dc93b00..4bfc82d 100644 --- a/.gitignore +++ b/.gitignore @@ -233,6 +233,9 @@ dist /package-lock.json yarn.lock +# Dead hand-committed minified bundle (see SECURITY_PERF_AUDIT_2026-09-18.md L5) +frontend/out.js + # Playwright (frontend/e2e) frontend/e2e/.auth/ frontend/playwright-report/ diff --git a/SECURITY_PERF_AUDIT_2026-09-18.md b/SECURITY_PERF_AUDIT_2026-09-18.md index 8bca535..d19c554 100644 --- a/SECURITY_PERF_AUDIT_2026-09-18.md +++ b/SECURITY_PERF_AUDIT_2026-09-18.md @@ -57,3 +57,44 @@ Fixed in commit "Harden the API and the data layer after a security and performa Final gates: backend 632 passed / ruff clean / alembic head OK; frontend 958 tests, typecheck+lint clean, cov 98.77/94.64/92.04/98.77. Deferred (follow-up list): H12 cold-load waterfall + no cross-route cache; M4 rate limiter proxy trust / shared store; M5 N+1 query collapsing (0.05 ms/query — not a bottleneck); M7 Calendar spinner on every mutation + sequential multi-day save; M9 (moot server-side); L2 drop personnummer_enc; L3 CI hardening (permissions block, SHA pins, npm ci, failure-only artefact upload, uv lock --check, dependabot); L5 out.js removal, CSP meta, dead /calendar?date= link. Phase-2 contract change (not started): drop workspace_id from request bodies and derive it from the caller. + +## Re-check (2026-09-20) +Verified every item of 2026-09-18 outcome against the tree. Confirmed intact: C1, C2, C3, H5–H11, M1, M2, M3, M6, M8, L1 (partly), L4. Five residual gaps found inside items marked fixed; deferred list untouched. + +Fixed this pass: +- R1 (residual H1) — IDOR on `/api/analytics/{ws}/overview` and `/projection`: `child_id` unchecked query param, analytics_service.get_overview() called get_days_left_at_age_4 without workspace_id (guard at analytics_service.py:240 never ran). Member of workspace A could read workspace B's child projection. Fixed: `scope.check_child(child_id)` in router (matching /age4 route pattern) and `workspace_id=` passed through service. +- R2 (residual H2) — session ids logged at INFO throughout fk_scraper.py (172,198). All session-id-carrying logger.info() calls downgraded to DEBUG; "scraper session done" kept at INFO without id; debug-artifact warning stays WARNING without id. One WARNING (tier sum mismatch, fk_scraper.py:723) deliberately keeps id. +- R3 (residual H3) — raw str(exc) from persist-callback failure and FK maintenance banner's scraped text reached poll client. Both replaced with fixed strings; full detail stays in ERROR/DEBUG logs. +- R4 (residual L1) — case-sensitive email: Foo@x.se and foo@x.se became two accounts. Lowercasing field_validator on RegisterRequest/LoginRequest; migration 009_lowercase_user_email (aborts with named-rows error if two rows would collide, not opaque UNIQUE constraint failure). +- R5 (residual H11) — usePlanProposal and usePlanSync had no stale-response guard. Both now use shared useLatestRequest helper (as 15 other hooks do). +- M9 — WorkspaceContext seeded workspaceId from localStorage synchronously; PlanSyncProvider's effect did not depend on auth → cold load POST /api/plan/sync before auth resolved, 401'd, raced AuthContext's own refresh. Effect now gated on isAuthenticated. +- M7 — every planned-day mutation blanked Calendar behind skeleton; multi-day save issued 2N requests (write+full refetch per date). usePlannedDays gained background/isRefreshing split (like useLeaveWishes) + skipRefresh option; Calendar's multi-day loop now N writes + one trailing background refresh + one bumpPlanVersion in finally (error path refreshes too). runSync's refresh also made background. +- H12 (partial) — take_pl_on_red_days added to GET /auth/context response; WorkspaceContext.refreshContext now builds workspace state from context alone, drops redundant GET /workspace/:id from every cold load. Workspace.name/createdAt optional (no read-side consumer in src/). +- L3 — CI hardened: top-level permissions: contents: read; all four actions pinned to commit SHAs; e2e npm install → npm ci; both uv sync → uv sync --locked; playwright-report upload if: always() → if: failure(). New .github/dependabot.yml (github-actions, npm, uv). +- L5 (partial) — frontend/out.js dead artifact removed + gitignored. CSP added to nginx.conf.template (from production build). Note: nginx does not merge add_header across levels; four security headers repeated in three location blocks (Cache-Control: /assets/, /fonts/, = /index.html) — verified curl against running container all four present on every route. Three pre-existing headers had silently been absent from those routes before. + +Decisions taken (not oversights): +- H4 refresh-token revocation (jti/iat) NOT implemented — stateless refresh stands. is_active checked on refresh → disabled account cut off; only per-token revocation missing. +- Phase-2 contract change (drop workspace_id from bodies) deferred — every route already reconciles body value against caller's scope, defence-in-depth not a live hole. +- H12 step 2 (cross-route cache) re-deferred: of 7 Dashboard fan-out endpoints only 2 safe to cache today; 5 have same-hook or cross-page writers (usePlannedDays, useLeaveWishes, useWorkspaceMembers/Settings, saveManualFetch) not wired to shared invalidation signal. URL-keyed cache would serve pre-mutation data to mutation's own refetch. If revisited, go endpoint-by-endpoint. +- Still deferred: M4 (rate limiter per-process/IP-keyed/proxy trust), M5 (N+1 fan-outs, 0.05 ms/query), L2 (drop personnummer_enc), L5 bundle splitting. +- New follow-up: /auth/context could also carry workspace name/created_at if future feature needs them on cold load. + +Gates after this pass: backend 901 passed / ruff clean / alembic head OK / coverage 92%. Frontend 1320 tests in 75 files / typecheck+lint clean / coverage 98.16 lines, 94.79 functions, 92.15 branches, 98.16 statements. + +### Browser verification (2026-09-20) and two defects it caught +Two rounds of real-browser verification against seeded dev instance caught two bugs the unit suites missed; both fixed + re-verified. +- M7 incomplete on first landing: `usePlannedDays`' `planVersion` effect called `fetch()` with no options, so mutators' `bumpPlanVersion()` re-ran it as second blocking fetch—2 GETs per save, page blanked on Advanced (direct planned-day) save path. Fixed: effect sole fetcher, mutators only bump, effect backgrounds everything but first load or childId switch. +- `isLoading` stranded forever on in-app navigation: `useLatestRequest` pattern latent flaw — `setBusy(true)` ran unconditional but `finally { if (isCurrent(requestId)) setBusy(false) }` cleared it only for still-current request, so superseded calls left flag set. React StrictMode double-invokes mount effect; on SPA navigation (childId already known) both invocations fetched—one foreground, one background—leaving `isLoading` true behind PageSkeleton. Fixed in all 16 hooks: unguarded busy-flag clear (`isCurrent` decides which response's DATA wins, never whether call releases flag it raised); plus in-flight dedup in `usePlannedDays`. Regression tests confirmed to fail against pre-fix code. + +Verified in-browser post-fix: 4 alternating in-app navigations all render without skeleton; 1 GET /api/planned-days per navigation; Advanced single-day save = 1 write + 1 GET, no blanking; 4-day drag-select = 4 writes back-to-back + exactly 1 trailing GET; cold load issues no GET /api/workspace/{id}; red-days flag round-trips through /auth/context and honored by Calendar's free-day filter; SV and EN both clean. + +New follow-ups found (not fixed): +- `useCalendarDays` mutates `isFirstPlanVersionRun` ref in effect body; StrictMode double-invoke can fire `background: true` fetch on page with nothing loaded. Unguarded flag clear stops hangs, can still cause transient extra request / brief flash (dev only). +- `PlanSyncProvider` handles stale `localStorage` workspaceId poorly: after workspace it names is gone, background sync 404s and logs error instead of clearing stale id. +- Cold load issues `GET /api/workspace/{ws}/members` twice. +- Cold load issues `POST /api/auth/refresh` twice (consistent with StrictMode double-invoke; dev-only, unconfirmed in prod). +- `POST /api/plan/sync` fires before `GET /api/auth/context` resolves (safe: auth resolved and server scope-checks; ordering surprise). +- Child-switch behavior could not be browser-tested: demo seed has one child only. + +Frontend gates after final fix: 75 files / 1322 tests / 98.17 lines, 94.79 functions, 92.14 branches, 98.17 statements. Branch coverage 0.14 above gate of 92—thin. diff --git a/backend/app/models/auth.py b/backend/app/models/auth.py index 4540b09..b168515 100644 --- a/backend/app/models/auth.py +++ b/backend/app/models/auth.py @@ -1,6 +1,6 @@ """Pydantic schemas for auth endpoints.""" -from pydantic import BaseModel, EmailStr, Field +from pydantic import BaseModel, EmailStr, Field, field_validator class RegisterRequest(BaseModel): @@ -13,11 +13,25 @@ class RegisterRequest(BaseModel): # if one exists) instead of creating a new one. invite_token: str | None = None + @field_validator("email") + @classmethod + def normalize_email(cls, v: str) -> str: + """Lowercase at the API boundary — storage/lookup is case-sensitive + (`WHERE email = ?`), so Foo@x.se and foo@x.se must resolve to one + account rather than silently becoming two.""" + return v.lower() + class LoginRequest(BaseModel): email: EmailStr password: str + @field_validator("email") + @classmethod + def normalize_email(cls, v: str) -> str: + """Lowercase at the API boundary — see `RegisterRequest.normalize_email`.""" + return v.lower() + class TokenResponse(BaseModel): access_token: str diff --git a/backend/app/routers/analytics.py b/backend/app/routers/analytics.py index 095d910..986c904 100644 --- a/backend/app/routers/analytics.py +++ b/backend/app/routers/analytics.py @@ -44,7 +44,9 @@ async def get_overview( pd_repo: PersonalDataRepositoryDep, workspace_repo: WorkspaceRepositoryDep, vacation_repo: VacationDaysRepositoryDep, + scope: WorkspaceScopeDep, ) -> WorkspaceOverviewResponse: + await scope.check_child(child_id) service = _make_service( leave_repo, planned_repo, child_repo, pd_repo, workspace_repo, vacation_repo ) @@ -65,7 +67,9 @@ async def get_projection( pd_repo: PersonalDataRepositoryDep, workspace_repo: WorkspaceRepositoryDep, vacation_repo: VacationDaysRepositoryDep, + scope: WorkspaceScopeDep, ) -> WorkspaceOverviewResponse: + await scope.check_child(child_id) service = _make_service( leave_repo, planned_repo, child_repo, pd_repo, workspace_repo, vacation_repo ) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 0cca4da..05caa9e 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -10,6 +10,7 @@ from app.dependencies import ( ChildRepositoryDep, CurrentUserDep, PersonalDataRepositoryDep, + WorkspaceRepositoryDep, ) from app.rate_limit import limiter from app.models.auth import LoginRequest, RegisterRequest, TokenResponse, UserMeResponse @@ -105,6 +106,11 @@ class _UserContext(BaseModel): workspace_id: str parent_name: str children: list[_ChildContext] + # Mirrors `WorkspaceResponse.take_pl_on_red_days` — included here so the + # frontend's cold-load `refreshContext()` can build its `workspace` + # state from this response alone, without a follow-up `GET + # /workspace/:id` (see AGENTS.md H12 step 1). + take_pl_on_red_days: bool @router.get("/context", response_model=_UserContext) @@ -112,12 +118,16 @@ async def get_context( current_user: CurrentUserDep, pd_repo: PersonalDataRepositoryDep, child_repo: ChildRepositoryDep, + ws_repo: WorkspaceRepositoryDep, ) -> _UserContext: """Return workspace + children for the current user. Used by the dev harness.""" pd = await pd_repo.get_by_id(current_user.personal_data_uuid) if pd is None: raise HTTPException(status.HTTP_422_UNPROCESSABLE_ENTITY, "No parent profile found") children = await child_repo.list_by_workspace(pd.workspace_id) + workspace = await ws_repo.get_by_id(pd.workspace_id) + if workspace is None: + raise HTTPException(status.HTTP_422_UNPROCESSABLE_ENTITY, "No workspace found") return _UserContext( personal_data_uuid=str(pd.uuid), workspace_id=str(pd.workspace_id), @@ -125,6 +135,7 @@ async def get_context( children=[ _ChildContext(id=str(c.id), name=c.name, birth_date=str(c.birth_date)) for c in children ], + take_pl_on_red_days=workspace.take_pl_on_red_days, ) diff --git a/backend/app/scrapers/fk_scraper.py b/backend/app/scrapers/fk_scraper.py index 7b399ce..7772d04 100644 --- a/backend/app/scrapers/fk_scraper.py +++ b/backend/app/scrapers/fk_scraper.py @@ -306,7 +306,7 @@ class FKScraperService(FetchDataService): # Schedule TTL watchdog asyncio.create_task(_ttl_watchdog(session_id)) - logger.info("scraper session started [id=%s]", session_id) + logger.debug("scraper session started [id=%s]", session_id) return session_id async def poll_session(self, session_id: str, owner_user_id: UUID) -> dict: @@ -345,7 +345,7 @@ class FKScraperService(FetchDataService): state.qr_data = None if state._task and not state._task.done(): state._task.cancel() - logger.info("scraper session cancelled [id=%s]", session_id) + logger.debug("scraper session cancelled [id=%s]", session_id) # --------------------------------------------------------------------------- @@ -390,7 +390,7 @@ async def _ttl_watchdog(session_id: str) -> None: 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) + logger.debug("scraper session timed out [id=%s]", session_id) elif state.status in ("done", "error", "cancelled"): _sessions.pop(session_id, None) @@ -451,13 +451,13 @@ async def _save_debug_artifacts(page, session_id: str, console_messages: list[st screenshot_path = prefix.with_suffix(".png") await page.screenshot(path=str(screenshot_path), full_page=True) - logger.info( + logger.debug( "scraper debug artifacts saved [id=%s] dir=%s", session_id, out.resolve(), ) except Exception as artifact_exc: - logger.warning("failed to save debug artifacts [id=%s]: %s", session_id, artifact_exc) + logger.warning("failed to save debug artifacts: %s", artifact_exc) async def _run_session(state: _SessionState) -> None: @@ -509,9 +509,9 @@ async def _do_login(page, state: _SessionState) -> None: # Check for maintenance message before proceeding maintenance_msg = await _check_maintenance(page) if maintenance_msg: - error_msg = f"FK is under maintenance: {maintenance_msg}" - _set_status(state, "error", error_msg) - logger.info("[%s] FK maintenance detected; aborting session", state.session_id) + _set_status(state, "error", "FK is currently under maintenance. Please try again later.") + logger.debug("[%s] FK maintenance banner text: %s", state.session_id, maintenance_msg) + logger.info("FK maintenance detected; aborting session") return # Accept cookie consent banner if present (only appears once per browser profile) @@ -533,7 +533,7 @@ async def _do_login(page, state: _SessionState) -> None: if is_disabled: error_msg = "FK login temporarily unavailable (BankID button disabled). Please try again later." _set_status(state, "error", error_msg) - logger.info("[%s] BankID button is disabled; aborting session", state.session_id) + logger.info("BankID button is disabled; aborting session") return except Exception: pass # proceed anyway; may not be disabled @@ -627,7 +627,9 @@ async def _do_extract(page, state: _SessionState) -> None: if family is not None: state.partner_deduction = _deduce_partner(data, family) except Exception as exc: - logger.info("[%s] family-panel deduction skipped: %s", state.session_id, type(exc).__name__) + logger.debug( + "[%s] family-panel deduction skipped: %s", state.session_id, type(exc).__name__ + ) state.family = None state.partner_deduction = None @@ -645,7 +647,7 @@ async def _do_extract(page, state: _SessionState) -> None: state.partner_deduction, transferable_days_left=transferable.partner_total ) except Exception as exc: - logger.info( + logger.debug( "[%s] transferable-days parse skipped: %s", state.session_id, type(exc).__name__ ) @@ -656,14 +658,20 @@ async def _do_extract(page, state: _SessionState) -> None: _set_status( state, "error", - f"Scrape succeeded but failed to persist data: {type(exc).__name__}: {exc}", + "Scrape succeeded but failed to persist the data. Please try again.", + ) + logger.error( + "persist error [id=%s]: %s: %s", + state.session_id, + type(exc).__name__, + exc, + exc_info=False, ) - logger.error("persist error [id=%s]: %s", state.session_id, exc, exc_info=False) return _set_status(state, "done") state.result = data - logger.info("scraper session done [id=%s]", state.session_id) + logger.info("scraper session done") async def _extract_parental_leave(page, state: _SessionState) -> ParentalLeaveFetch: @@ -746,19 +754,19 @@ async def _extract_family_summary(page, state: _SessionState) -> FamilySummary | Defensive by design: the real FK DOM for this panel is unconfirmed (see `_SEL.FAMILY*`'s comment), so any missing element or unexpected text is - treated as "no family data available" — logged at INFO and returned as + treated as "no family data available" — logged at DEBUG and returned as None — never raised. The caller's own `data` (this parent's snapshot) must succeed regardless of what happens here. """ try: family_panel = page.locator(_SEL.FAMILY) if await family_panel.count() == 0: - logger.info("[%s] family panel not present; skipping deduction", state.session_id) + logger.debug("[%s] family panel not present; skipping deduction", state.session_id) return None dagar_kvar_loc = page.locator(_SEL.FAMILY_DAGAR_KVAR) if await dagar_kvar_loc.count() == 0: - logger.info( + logger.debug( "[%s] family panel dagar-kvar element not found; skipping deduction", state.session_id, ) @@ -768,7 +776,7 @@ async def _extract_family_summary(page, state: _SessionState) -> FamilySummary | discrete_loc = page.locator(_SEL.FAMILY_DISCRETE) if await discrete_loc.count() == 0: - logger.info( + logger.debug( "[%s] family panel discrete element not found; skipping deduction", state.session_id, ) @@ -776,7 +784,7 @@ async def _extract_family_summary(page, state: _SessionState) -> FamilySummary | discrete_text = await discrete_loc.first.inner_text(timeout=2_000) total_match = re.search(r"av\s+(\d+)\s+dagar", discrete_text) if not total_match: - logger.info( + logger.debug( "[%s] could not parse 'av N dagar' from family panel text; skipping deduction", state.session_id, ) @@ -806,7 +814,7 @@ async def _extract_family_summary(page, state: _SessionState) -> FamilySummary | lagsta_niva_left=lagsta_niva_left, ) except Exception as exc: - logger.info( + logger.debug( "[%s] family panel parsing failed (%s); skipping deduction", state.session_id, type(exc).__name__, @@ -894,7 +902,7 @@ async def _extract_transferable_days( Fully defensive by design, mirroring `_extract_family_summary`: any missing element, unmatched child, or unexpected text is "no transferable - data available" — logged at INFO and returned as None — never raised. + data available" — logged at DEBUG and returned as None — never raised. The caller's own `data` (already validated) must be unaffected either way. """ try: @@ -924,7 +932,7 @@ async def _extract_transferable_days( sections = page.locator(_SEL.MIN_FP_BARN_SEKTION) section_count = await sections.count() if section_count == 0: - logger.info( + logger.debug( "[%s] no barn-sektion found on min-foraldrapenning page; skipping " "transferable-days parse", state.session_id, @@ -935,7 +943,7 @@ async def _extract_transferable_days( chosen = sections.first else: if not child_name or not child_name.strip(): - logger.info( + logger.debug( "[%s] multiple children on min-foraldrapenning page and no " "child_name to match; skipping transferable-days parse", state.session_id, @@ -953,7 +961,7 @@ async def _extract_transferable_days( chosen = section break if chosen is None: - logger.info( + logger.debug( "[%s] no barn-sektion matched child_name; skipping transferable-days parse", state.session_id, ) @@ -961,7 +969,7 @@ async def _extract_transferable_days( panel = chosen.locator(_SEL.PANEL_FORA_OVER_DAGAR) if await panel.count() == 0: - logger.info( + logger.debug( "[%s] chosen barn-sektion has no transfer panel; skipping transferable-days parse", state.session_id, ) @@ -1014,7 +1022,7 @@ async def _extract_transferable_days( partner_sjuk, partner_lagsta = sjuk, lagsta if mine_sjuk is None or mine_lagsta is None: - logger.info( + logger.debug( "[%s] 'Du kan föra över' group missing or incomplete; skipping " "transferable-days parse", state.session_id, @@ -1028,7 +1036,7 @@ async def _extract_transferable_days( partner_lagsta_niva=partner_lagsta, ) except Exception as exc: - logger.info( + logger.debug( "[%s] transferable-days panel parsing failed (%s); skipping", state.session_id, type(exc).__name__, diff --git a/backend/app/services/analytics_service.py b/backend/app/services/analytics_service.py index fa745b5..87d4be4 100644 --- a/backend/app/services/analytics_service.py +++ b/backend/app/services/analytics_service.py @@ -552,7 +552,7 @@ class AnalyticsService: projections = [] for pd in members: - proj = await self.get_days_left_at_age_4(pd.uuid, child_id) + proj = await self.get_days_left_at_age_4(pd.uuid, child_id, workspace_id=workspace_id) projections.append(proj) combined_used = sum(p.days_used + p.planned_days_before_b4 for p in projections) diff --git a/backend/migrations/versions/009_lowercase_user_email.py b/backend/migrations/versions/009_lowercase_user_email.py new file mode 100644 index 0000000..4a5a9bc --- /dev/null +++ b/backend/migrations/versions/009_lowercase_user_email.py @@ -0,0 +1,78 @@ +"""Lowercase existing "user".email values. + +Email lookup/storage is case-sensitive (`SQLiteUserRepository.get_by_email` +runs a bare `WHERE email = ?`), so `Foo@x.se` and `foo@x.se` used to register +as two distinct accounts. The API boundary now lowercases incoming email on +every register/login request (`app.models.auth.RegisterRequest`/ +`LoginRequest`, `normalize_email` field validators), so all NEW rows are +already lowercase; this migration normalizes rows written before that fix. + +If two existing rows would collapse to the same lowercased address (a +genuine pre-existing duplicate-account situation), blindly lowercasing both +would hit the `email` column's UNIQUE constraint and abort with an opaque +integrity error on whichever row happens to update second. Instead, +`upgrade()` scans for any such collision FIRST and raises a clear, +actionable `RuntimeError` naming the colliding rows before writing anything, +so an operator can merge or rename one of each colliding pair by hand and +re-run the migration, rather than facing a half-applied state. + +`downgrade()` cannot restore the original casing (it was never recorded) — +it is a deliberate, documented no-op. Lowercasing is a one-way normalization. + +Revision ID: 009 +Revises: 008 +Create Date: 2026-09-20 +""" + +from __future__ import annotations + +from typing import Any + +from alembic import op + +revision: str = "009" +down_revision: str | None = "008" +branch_labels: str | None = None +depends_on: str | None = None + + +def _collect_rows(conn: Any) -> list[Any]: + from sqlalchemy import text + + return conn.execute(text('SELECT id, email FROM "user"')).fetchall() + + +def upgrade() -> None: + from sqlalchemy import text + + conn = op.get_bind() + rows = _collect_rows(conn) + + by_lower: dict[str, list[tuple[str, str]]] = {} + for row in rows: + by_lower.setdefault(row.email.lower(), []).append((str(row.id), row.email)) + + collisions = {lower: entries for lower, entries in by_lower.items() if len(entries) > 1} + if collisions: + detail = "; ".join( + f"{lower!r} <- {[f'{id_} ({email})' for id_, email in entries]}" + for lower, entries in collisions.items() + ) + raise RuntimeError( + 'Cannot lowercase "user".email: the following addresses would collide ' + f"under case-insensitive comparison and must be resolved by hand first " + f"(merge or rename one account in each pair, then re-run this migration): " + f"{detail}" + ) + + for lower, entries in by_lower.items(): + (id_, email) = entries[0] + if email != lower: + conn.execute( + text('UPDATE "user" SET email = :email WHERE id = :id'), + {"email": lower, "id": id_}, + ) + + +def downgrade() -> None: + """No-op: the original casing was never recorded, so it cannot be restored.""" diff --git a/backend/tests/unit/test_auth_router.py b/backend/tests/unit/test_auth_router.py new file mode 100644 index 0000000..cd00970 --- /dev/null +++ b/backend/tests/unit/test_auth_router.py @@ -0,0 +1,132 @@ +"""R4: email case-normalisation at the auth API boundary. + +Storage/lookup (`SQLiteUserRepository.get_by_email`) is case-sensitive, so +without normalisation `Foo@x.se` and `foo@x.se` would register as two +distinct accounts. `RegisterRequest`/`LoginRequest` (`app/models/auth.py`) +now lowercase `email` via a field validator — these tests drive the real +app over HTTP (same technique as `tests/integration/test_user_journey.py`) +to prove that boundary actually normalises, rather than only unit-testing +the validator in isolation. +""" + +from __future__ import annotations + +import aiosqlite +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient + +from app.dependencies import get_connection +from app.main import app +from app.rate_limit import limiter + +_PASSWORD = "correct horse battery staple" + + +@pytest.fixture(autouse=True) +def _reset_rate_limiter() -> None: + """`/api/auth/register` and `/api/auth/login` are limited to 10/minute, + keyed on client IP (`app/rate_limit.py`). In the full suite, every test + file that POSTs to either endpoint shares that one in-memory bucket for + 127.0.0.1 (see `tests/integration/test_user_journey.py`'s comment on the + same issue) — by the time this file runs, other files may have already + spent it. Reset the limiter's storage before each test here rather than + disabling enforcement, so these tests still exercise the real + middleware stack; this is exactly `Limiter.reset()`'s documented purpose. + """ + limiter.reset() + + +@pytest_asyncio.fixture +async def client(connection: aiosqlite.Connection) -> AsyncClient: + """Real app over HTTP, with `get_connection` swapped for the in-memory + test connection — see `tests/integration/test_user_journey.py`'s fixture + of the same name.""" + 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) + + +async def test_register_lowercases_email(client: AsyncClient) -> None: + resp = await client.post( + "/api/auth/register", + json={"email": "Mixed.Case@Example.COM", "password": _PASSWORD, "display_name": "A"}, + ) + assert resp.status_code == 201, resp.text + + +async def test_login_with_different_case_resolves_to_same_account(client: AsyncClient) -> None: + register_resp = await client.post( + "/api/auth/register", + json={"email": "Case.Test@Example.com", "password": _PASSWORD, "display_name": "A"}, + ) + assert register_resp.status_code == 201, register_resp.text + + login_resp = await client.post( + "/api/auth/login", + json={"email": "CASE.TEST@EXAMPLE.COM", "password": _PASSWORD}, + ) + assert login_resp.status_code == 200, login_resp.text + assert "access_token" in login_resp.json() + + +async def test_registering_same_address_in_different_case_is_rejected_as_duplicate( + client: AsyncClient, +) -> None: + first = await client.post( + "/api/auth/register", + json={"email": "dupe@example.com", "password": _PASSWORD, "display_name": "First"}, + ) + assert first.status_code == 201, first.text + + second = await client.post( + "/api/auth/register", + json={"email": "DUPE@EXAMPLE.COM", "password": _PASSWORD, "display_name": "Second"}, + ) + assert second.status_code == 409, second.text + + +async def test_context_includes_take_pl_on_red_days_default_false(client: AsyncClient) -> None: + """H12 step 1: `/auth/context` must carry the same red-days flag + `GET /workspace/:id` does, so the frontend can drop that second cold-load + call. A freshly registered workspace defaults to `False`.""" + register_resp = await client.post( + "/api/auth/register", + json={"email": "red-days@example.com", "password": _PASSWORD, "display_name": "A"}, + ) + assert register_resp.status_code == 201, register_resp.text + headers = {"Authorization": f"Bearer {register_resp.json()['access_token']}"} + + ctx_resp = await client.get("/api/auth/context", headers=headers) + assert ctx_resp.status_code == 200, ctx_resp.text + assert ctx_resp.json()["take_pl_on_red_days"] is False + + +async def test_context_reflects_take_pl_on_red_days_after_workspace_update( + client: AsyncClient, +) -> None: + """Once the workspace's flag is flipped via `PUT /workspace/:id`, the + context endpoint must reflect that new value, not a stale default — + this is what `Settings.tsx`'s red-days toggle relies on after it saves.""" + register_resp = await client.post( + "/api/auth/register", + json={"email": "red-days-2@example.com", "password": _PASSWORD, "display_name": "A"}, + ) + assert register_resp.status_code == 201, register_resp.text + headers = {"Authorization": f"Bearer {register_resp.json()['access_token']}"} + + ctx_resp = await client.get("/api/auth/context", headers=headers) + assert ctx_resp.status_code == 200, ctx_resp.text + workspace_id = ctx_resp.json()["workspace_id"] + + update_resp = await client.put( + f"/api/workspace/{workspace_id}", + headers=headers, + json={"take_pl_on_red_days": True}, + ) + assert update_resp.status_code == 200, update_resp.text + + ctx_resp_2 = await client.get("/api/auth/context", headers=headers) + assert ctx_resp_2.status_code == 200, ctx_resp_2.text + assert ctx_resp_2.json()["take_pl_on_red_days"] is True diff --git a/backend/tests/unit/test_authz.py b/backend/tests/unit/test_authz.py index 358c85f..af35aa4 100644 --- a/backend/tests/unit/test_authz.py +++ b/backend/tests/unit/test_authz.py @@ -268,6 +268,29 @@ class TestForeignWorkspaceIdIs404: ) assert resp.status_code == 404 + async def test_analytics_overview_foreign_child_in_own_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + """R1: `workspace_id` is the caller's own (workspace A), but + `child_id` belongs to workspace B — the IDOR the router-level + `scope.check_child` guard closes (see `analytics.get_overview`). + """ + resp = await client.get( + f"/api/analytics/{two_workspaces.ws_a.id}/overview", + params={"child_id": str(two_workspaces.child_b.id)}, + ) + assert resp.status_code == 404 + + async def test_analytics_projection_foreign_child_in_own_workspace( + self, client: AsyncClient, two_workspaces: _TwoWorkspaces + ) -> None: + """Same IDOR as above, on the sibling `/projection` route.""" + resp = await client.get( + f"/api/analytics/{two_workspaces.ws_a.id}/projection", + 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: diff --git a/backend/tests/unit/test_fk_scraper.py b/backend/tests/unit/test_fk_scraper.py index 82d864a..0244982 100644 --- a/backend/tests/unit/test_fk_scraper.py +++ b/backend/tests/unit/test_fk_scraper.py @@ -31,6 +31,7 @@ from app.scrapers.fk_scraper import ( _apply_transferable, _deduce_partner, _do_extract, + _do_login, _extract_family_summary, _extract_transferable_days, _SEL, @@ -1218,6 +1219,45 @@ class TestDoExtractTransferableIntegration: assert state.result.transferable_days_left == 49 # unchanged from the original estimate +# --------------------------------------------------------------------------- +# Maintenance detection — client-facing message must not echo scraped text +# --------------------------------------------------------------------------- + + +class TestMaintenanceDetection: + """R3: `_do_login`'s maintenance branch must return a fixed message — + never the scraped FK banner text — to the poll client.""" + + async def test_maintenance_banner_text_not_leaked_to_client(self) -> None: + 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(), + ) + + page = MagicMock() + page.goto = AsyncMock() + page.url = "https://www.forsakringskassan.se/privatperson/logga-in" + + sensitive_banner = ( + "Planerat underhall pagar for personer med personnummer som slutar pa " + "1-5, tjansten ar otillganglig till kl 23." + ) + with patch( + "app.scrapers.fk_scraper._check_maintenance", new_callable=AsyncMock + ) as mock_check: + mock_check.return_value = sensitive_banner + await _do_login(page, state) + + assert state.status == "error" + assert state.message is not None + assert sensitive_banner not in state.message + assert "maintenance" in state.message.lower() + + # --------------------------------------------------------------------------- # Session state machine # --------------------------------------------------------------------------- @@ -1706,6 +1746,8 @@ class TestSessionStateMachine: assert state.status == "error" assert "persist" in (state.message or "").lower() + # R2/R3: the raw exception text must never reach the poll client. + assert "DB unavailable" not in (state.message or "") async def test_do_extract_waits_for_hydration_before_extracting(self) -> None: """_do_extract calls wait_for_function to ensure data hydrates before extraction.""" diff --git a/frontend/nginx.conf.template b/frontend/nginx.conf.template index f0cc69a..2a6e0b3 100644 --- a/frontend/nginx.conf.template +++ b/frontend/nginx.conf.template @@ -22,10 +22,23 @@ server { server_tokens off; + # nginx does NOT merge add_header across levels: if a location block adds + # even one add_header of its own, every add_header set here at server + # level is discarded for responses from that location — not appended to. + # /assets/, /fonts/ and = /index.html below each set their own + # Cache-Control, so the four security headers are repeated verbatim + # inside each of those blocks. Add a header here, and it must be added + # there too (or it silently never reaches those responses). add_header X-Content-Type-Options nosniff always; add_header X-Frame-Options DENY always; add_header Referrer-Policy no-referrer always; + # Derived from the actual production build (frontend/dist): no inline + #