diff --git a/.planning/research/v1427-r1-settings-admin.md b/.planning/research/v1427-r1-settings-admin.md new file mode 100644 index 000000000..80902a93a --- /dev/null +++ b/.planning/research/v1427-r1-settings-admin.md @@ -0,0 +1,286 @@ +--- +file: .planning/research/v1427-r1-settings-admin.md +slot: R1.2 — Settings + Admin UX audit +target_tag: v1.4.27 +findings_covered: [9, 10, 11, 12, 13, 22, 23] +surfaces_surveyed: 26 +created: 2026-05-14 +--- + +# R1.2 — Settings and Admin UX audit (v1.4.27) + +## Headline + +Findings 9–13 fall out of three concrete causes inside the account form and the two shells: the profile form uses a free-running `space-y-4` rhythm instead of the same paired-grid the rest of the form uses, the language field never moved into the `sm:grid-cols-2` row that should host it, and the `TimezonePicker` introduces a tighter inner `gap-2` that does not match the outer `space-y-4` cadence. Findings 22 and 23 share one server-side surface: `src/lib/geo.ts`. Today it returns only `"City, CC"` and fires only on `auth.*` actions — the dash that lands in the table is the legitimate provider miss for private and recently-cached IPs, plus a `null` payload for any non-`auth.*` event that lands in the same viewer. ASN is never resolved. The recommended fix bundles the offline GeoLite2-City and GeoLite2-ASN MMDBs, swaps the runtime lookup to local DB reads, and adds a `carrier` field to the audit row so the Provider column can grow a `Telekom DE` chip next to `API-Token`. + +Surfaces surveyed: 11 settings sub-pages + 14 admin sub-pages + 1 admin overview = 26. + +## Surface roster (for traceability) + +Settings (`/settings/`): `account`, `integrations`, `notifications`, `dashboard`, `thresholds`, `sources`, `ai`, `api`, `export`, `advanced`, `about`. + +Admin (`/admin` + `/admin/`): overview, `system-status`, `general`, `services`, `integrations`, `ai-quality`, `coach-feedback`, `feedback`, `reminders`, `users`, `api-tokens`, `login-overview`, `app-logs`, `backups`, `danger-zone`. + +## Per-finding analysis + +### Finding 9 — Profile field arrangement is asymmetric + +**Current state.** `account-section.tsx` lines 354–446. The form uses `
` and then alternates between two-column `grid gap-4 sm:grid-cols-2` rows (Username + Email, Gender + Height) and single-cell rows (Date of birth on its own, Timezone via `TimezonePicker`, Language on its own). Field widths within the same grid cell vary because three different field primitives are interleaved: + +- `` (h-9, full width of cell) +- Native `` into the same `grid gap-4 sm:grid-cols-2` row as date of birth; drop the standalone language wrapper). + +**LOC estimate.** ≈ 25 lines moved + 5 lines deleted. + +**Risk.** Low. Touches only one form's JSX. The `` keeps its own layout so the detect-button gap (finding 10) is fixable independently. + +### Finding 10 — Zeitzone Berlin gap to its neighbour button is smaller than the top-row gaps + +**Current state.** `timezone-picker.tsx` lines 91–119. The picker renders: + +```tsx +
+ +
+ `. Consistent. EXCEPT the password-change dialog inside `account-section.tsx` which uses `space-y-1.5` — divergent. Standardise to `space-y-2`. + +**Files touched.** All settings + admin section components — but each is a one-line change. A grep-and-replace pass. + +**LOC estimate.** ≈ 30 lines across ~10 files. + +**Risk.** Low — purely cosmetic. Visual snapshot tests need to be re-baselined. + +### Finding 22 — Standort cell shows a dash + +**Current state.** `login-overview-section.tsx` line 444: `{entry.location ?? "—"}`. The dash is the explicit `??` fallback. `entry.location` is populated by `src/lib/auth/audit.ts` lines 21–39 which fires `lookupIpLocation()` only when `action.startsWith("auth.")` AND `opts.ipAddress` is truthy. + +Three classes of row will show "—": + +1. **Private IPs.** `lib/geo.ts` line 76: `if (!ip || PRIVATE_IP.test(ip)) return null`. Cloudflare-fronted deployments forward `127.0.0.1` for internal probes; local dev shows "—" for every login. +2. **Failed `ipwho.is` lookups.** Lines 80–94: 3-second timeout, then `null`. The provider rate-limits aggressively when the audit table fills with login retries during a credential-stuffing burst. +3. **Cached failures never retried.** The lookup is fire-and-forget and runs only at audit-creation time. If `ipwho.is` was down when the row was inserted, the `location` column stays `NULL` forever. + +The provider itself works — `lib/__tests__/geo.test.ts` exercises both response shapes and the umlaut path. The dash in production rows is real provider misses, not a code bug. + +**Problem.** No retry path, no offline fallback, and the public-IP egress contract (`IP_GEO_LOOKUP_DISABLED=1`) means privacy-conscious deployments accept "—" forever. + +**Proposed approach.** Two-tier strategy: + +1. **Primary (offline):** Bundle the MaxMind GeoLite2-City MMDB. The `mmdb-lib` npm package (zero-deps, MIT) reads it at request time. ~70 MB DB on disk but the lookup is microsecond-scale and gives city + country offline. License: GeoLite2 Creative Commons Attribution-ShareAlike 4.0 — requires attribution in a docs page. Refresh schedule: monthly DB update, baked into the Docker image at build time. +2. **Fallback (online):** Keep the current `ipwho.is` path for IPs the offline DB cannot resolve (the GeoLite2 free tier coverage is ~99 % for IPv4 but lags on freshly-allocated ranges). +3. **Backfill job:** A one-shot cron (`src/lib/jobs/`) that walks `auditLog` rows where `location IS NULL` AND `ipAddress IS NOT NULL` AND `createdAt > now() - 30 days` and re-resolves them through the offline DB. Caps at 5k rows per pass to stay polite. + +**Files touched.** +- `src/lib/geo.ts` (add offline-first resolver path). +- `package.json` (add `mmdb-lib`). +- `Dockerfile` (copy GeoLite2-City.mmdb into `/opt/geolite2/`). +- `src/lib/jobs/geo-backfill.ts` (new — backfill helper). +- `src/lib/jobs/__tests__/geo-backfill.test.ts` (new — TDD). +- `docs/audit/v1427-summary.md` (GeoLite2 attribution paragraph). + +**LOC estimate.** ≈ 200 LOC (resolver fork + job + tests). Lookup helper: ~40 LOC; backfill job: ~60 LOC; tests: ~100 LOC. + +**Risk.** Medium. Docker image grows by ~70 MB. Need to verify multi-arch GHCR build still fits the 2 GB image-size budget enforced in `.github/workflows/release.yml`. License attribution must land in `/about` and `docs/audit/`. Per-row offline cache means rows resolved while the DB was version-X stay at version-X — fine for "city, country" since neither moves often, but the backfill job should re-resolve when the DB roll is detected. + +### Finding 23 — Provider column lists API-Token / Passkey but not the carrier + +**Current state.** `_shared.tsx` lines 314–348: `providerForAction()` maps action names to `password | passkey | api_token | withings | unknown`. That's the auth-mechanism axis. The carrier axis is missing entirely. No ASN data lives in the schema. + +**Problem.** Marc wants the carrier (Telekom / Vodafone / 1&1) visible next to the auth mechanism. The two axes serve different forensic questions: "how did they sign in?" (provider) vs "where are they coming from?" (carrier). + +**Research — MaxMind GeoLite2-ASN.** + +- License: same CC BY-SA 4.0 as GeoLite2-City. Attribution mandatory. +- Size: ~7 MB compressed, ~10 MB uncompressed. +- Lookup performance: same MMDB layout as GeoLite2-City. Microsecond-scale local read. +- Coverage: full IPv4 + IPv6 ASN map. Known DACH carriers it resolves correctly: + - `AS3320` → Deutsche Telekom AG + - `AS3209` → Vodafone GmbH + - `AS8881` → 1&1 Versatel + - `AS6805` → Telefónica Germany (O2) + - `AS31334` → Vodafone Kabel Deutschland + - `AS3215` → Orange S.A. (for Marc's roaming events) +- Returns: `{ autonomous_system_number, autonomous_system_organization }`. We persist both — number for forensic precision, organization for human-readable display. + +**Proposed schema change.** + +Add two nullable columns to `AuditLog`: + +```prisma +asn Int? @map("asn") +carrier String? // e.g. "Deutsche Telekom AG" +``` + +Both populated at audit-creation time via the same fire-and-forget pattern that fills `location` today. Both nullable so older rows + private-IP rows + provider-miss rows stay valid. + +**Proposed UI change.** + +`login-overview-section.tsx`: split the Provider column into two visual chips. The auth-provider chip (`API-Token`, `Passkey`) stays where it is; the carrier chip renders beneath in a smaller font, gated on `entry.carrier !== null`. The action vocabulary in `_shared.tsx` does NOT change — Marc's "Provider" column header now means "credential + carrier" which is the natural reading. + +Alternative: add a dedicated "Netz" column to the right of "Standort". More tabular but uses a column slot in an already-busy 7-column table. The chip-stack approach below the auth provider is the recommended pick. + +**Files touched.** +- `prisma/schema.prisma` (additive migration: 2 nullable columns). +- `prisma/migrations/_audit_log_carrier/` (new — additive). +- `src/lib/geo.ts` (add `lookupIpAsn()` that returns `{ asn, carrier } | null`). +- `src/lib/auth/audit.ts` (resolve carrier in parallel with location, single `update()` carries both). +- `src/lib/__tests__/geo-asn.test.ts` (TDD). +- `src/components/admin/login-overview-section.tsx` (carrier chip below provider chip). +- `src/components/admin/_shared.tsx` (CSV: add `carrier` column to export). +- `src/components/admin/__tests__/login-overview-csv.test.ts` (extend). +- `src/lib/i18n/locales/{en,de}.json` (add `admin.carrier` + `admin.carrierUnknown`). + +**LOC estimate.** ≈ 250 LOC including tests. + +**Risk.** Medium. Prisma migration is additive + nullable so the `IF NOT EXISTS` guard pattern documented in `.planning/v1427-plan.md` applies. Docker image grows another ~10 MB for the ASN DB. Combined with the GeoLite2-City DB (finding 22), total Docker image growth ≈ 80 MB. Verify image-size budget before merge. + +## Cross-finding patterns + +1. **Two findings share `src/lib/geo.ts`.** 22 and 23 both want offline-first lookups against MaxMind GeoLite2 DBs. Ship them in one fix-surface bucket — the resolver refactor that introduces the offline path will hold both `lookupIpLocation()` and `lookupIpAsn()`. Otherwise two contributors will conflict on the same file. +2. **Three findings share `src/components/settings/account-section.tsx`.** 9, 10 (partial — shared form ergonomics), 11 all touch the profile form. Ship in one commit so the visual rhythm is reviewed once. +3. **Finding 12 ALSO touches the shells.** It shares `settings-shell.tsx` with no other R1.2 finding but it shares `admin-shell.tsx` with the R1.6 backlog sweep (which may pull forward another shell-related item). Coordinate with the R2 reconcile pass. +4. **Finding 13 is a polish-pass that touches every section component.** Because each edit is one line, this is genuinely a sweep — assign to a single contributor at the end of the implementation round so visual snapshots are re-baselined exactly once. + +## Recommended sequencing + +| Order | Bucket | Contains | Disjoint from | +|---|---|---|---| +| 1 | Profile form rhythm | 9, 10, 11 | All others | +| 2 | Shell layout-shift fix | 12 | Profile form | +| 3 | Geo offline-first + ASN | 22, 23 | UI changes | +| 4 | Symmetry sweep | 13 | All others — run last | + +Bucket 1 + 2 + 4 are pure front-end; bucket 3 is back-end + migration + a tiny UI delta. Run buckets 1–3 in parallel, then bucket 4 as a serial cleanup on top. + +## Deferrals + +None recommended. All seven findings fit inside v1.4.27. Bucket 3 is the heaviest (Docker image growth + Prisma migration) but the additive-nullable migration approach keeps it safe. + +## Implementation notes + +- **Mock the MMDB reader in tests.** The actual DB file is too large for the test fixtures dir; stub `mmdb-lib`'s `Reader` in `vitest.config` so `lookupIpAsn` tests run without the DB on disk. +- **GeoLite2 attribution.** Add a paragraph to the `/about` page and `docs/audit/v1427-summary.md` once it exists. The license is CC BY-SA 4.0 — silent inclusion would breach it. +- **Backfill job lives in `lib/jobs/`.** Reuse the existing pg-boss scheduler. Cap concurrency at 1 so the DB update does not collide with a live login burst. +- **Image-size budget.** Re-run the multi-arch publish CI (`.github/workflows/release.yml`) with the two MMDB files in place before merging. Reject the change if the image grows past 1.9 GB compressed. +- **Carrier display copy.** Marc-Voice English short labels: "Telekom", "Vodafone", "1&1", "O2" — fold the GeoLite2 `autonomous_system_organization` strings ("Deutsche Telekom AG") down to these via a small lookup table inside `_shared.tsx`. Unknown ASNs fall through to the raw organization string. +- **Hide carrier on private IPs.** The chip renders only when `entry.carrier !== null`. Same gate that already governs `entry.location`. + +## Verification checklist (post-implementation) + +- Profile form visually balanced on Pixel 5 (393 px) and a 1440 px desktop. +- Click `/settings/account` → `/settings/thresholds` → `/settings/sources`: viewport scroll delta < 50 px during the first 500 ms. +- `entry.carrier` populates on a fresh password login from a public German IP. +- Admin login-overview CSV export contains a `carrier` column with the correct DACH carriers for the test fixture set. +- `docs/audit/v1427-summary.md` includes the GeoLite2 attribution. +- Docker image growth < 90 MB combined.