diff --git a/docs/superpowers/specs/2026-08-21-byod-wifi-portal-hardening-design.md b/docs/superpowers/specs/2026-08-21-byod-wifi-portal-hardening-design.md new file mode 100644 index 0000000..c3207fe --- /dev/null +++ b/docs/superpowers/specs/2026-08-21-byod-wifi-portal-hardening-design.md @@ -0,0 +1,251 @@ +# Registered guest / BYOD Wi-Fi — hardening the captive portal + +**Date:** 2026-08-21 +**Repos:** `central-unifi` (most of it), `dashboard-shell` (tests, one Vue page) +**Status:** design approved, no implementation plan yet + +--- + +## Why this document is short on new features + +Nearly everything asked for already exists in `central-unifi` v1.13.4 and has for +some time. The request was "design registered guest Wi-Fi"; the finding is that +the feature is largely built, partly broken in ways that only show up under real +use, and **currently unreachable**. So this is a hardening spec, not a greenfield +one. + +### What already works + +| Requirement | Where it lives | +|---|---| +| Captive portal + Google auth | `PortalController::captiveCallback`, `/portal/wifi/callback`; the shell's OAuth has already authenticated the user by then | +| Per-cohort device limits | `unifi_vlan_mappings.max_devices` | +| Per-cohort VLAN | `unifi_vlan_mappings.vlan_id` | +| Session length | `unifi_vlan_mappings.session_minutes` (validated 1..525600) | +| Self-service eviction | `Unifi/PortalDenied` page lists the user's devices with `can_disconnect`, plus `unifi.portal.self-disconnect` | +| Admin eviction | `unifi.portal.sessions.disconnect` behind `app.access:unifi` + `permission:unifi.auth` | +| Device fingerprint capture | `CaptureSnapshots` already writes `dev_cat` and `os_name` per MAC into `unifi_client_snapshots` | + +Eviction authorisation is already correct and is **not** being changed: +`disconnectSession()` does `kickClient` + `unauthorizeGuest` and authorises +owner-or-`unifi.auth` inside the action, so a student can free their own slot and +nobody else's. + +### The answer to "must users re-do the portal every time?" + +No, and the mechanism is already wired. `authorizeGuest($mac, $minutes)` sends +UniFi `cmd: authorize-guest` with a **minutes** value taken from +`session_minutes`. UniFi holds that MAC as authorized for the whole window and +does not redirect it again. Set the window to a term and a student signs in once +a term. + +The portal is not the obstacle. **MAC randomization is** — see decision 2. + +--- + +## Decisions + +| # | Decision | Rationale | +|---|---|---| +| 1 | Harden and finish the existing portal rather than redesign | 80% exists and the schema shape is right; the defects are in accounting and trust, not structure | +| 2 | Detect a randomized MAC and coach the user; keep manual eviction | Manual choice is the stated requirement and already built; randomization is detectable, so the confusing case can be explained rather than hidden | +| 3 | Derive the MAC from the request's source IP and verify AP association; ignore `?mac=` | Closes the ownership hole by construction — the caller never gets to name a MAC | +| 4 | Add `role` as a `match_type` | Quotas were asked for per role; roles are the platform's own authority and need no external sync | +| 5 | One Wi-Fi Devices page grouped by person; no profile card | Matches the requirement exactly; keeps `/admin/users` as the one directory | +| 6 | Device type is **reported, not enforced**, for now | Whether `dev_cat` is populated in this environment is unverified; enforcing on unvalidated data can lock out the wrong people | + +--- + +## A. What consumes a device slot + +### The defect + +A slot is currently freed when `isClientConnected($mac)` is false. That conflates +**connected right now** with **authorized**, and they are different facts. Two +leaks follow, in opposite directions, and both get worse the longer +`session_minutes` is — which is exactly the direction this design pushes it. + +**Leak 1 — offline treated as unauthorized.** A phone leaves the building. Its +session row is marked inactive, freeing a slot, while UniFi still holds the MAC +authorized for the rest of its window. The user authorizes a second device. Two +devices are now on the network against a one-device quota, and one of them is an +orphan nothing tracks. + +**Leak 2 — `expires_at` is written and never read.** No scheduled job expires +sessions. A row stays `is_active` long after UniFi stopped honouring it, so the +user is refused by slots that do not exist. + +They compound: the same student can be simultaneously over-provisioned on the +network and locked out of the portal. + +### The model + +- A slot is consumed by a **live authorization**: `is_active AND expires_at > now()`. + Presence on the network is irrelevant to the count. +- A slot is freed by exactly three things: eviction (user or admin), expiry, or the + reconciler finding UniFi no longer holds it. +- `isClientConnected()` is **no longer an eviction trigger**. It becomes a display + value only — "connected" / "away" on the admin page. +- Quota resolution gains `role`. Precedence stays `sort_order`, so a mapping for + `staff` above one for `parent` gives a staff-parent 4 devices. + +--- + +## B. Identifying the device + +### Flow + +1. UniFi redirects the device to `/portal/wifi/callback?ap=&ssid=…&url=…`. +2. Google auth via the shell — unchanged. +3. Resolve the device **server-side**: match the request's client IP against + UniFi's live client table (`getActiveClients()`, on `ip`) to obtain the real + MAC, its `ap_mac`, and its authorization state. +4. Require that the resolved client's `ap_mac` equals the `ap` in the redirect — + the device must be on the AP that sent it here. +5. `?mac=` is **ignored**: not validated, not used. It is read for one purpose — + if present and disagreeing with the derived MAC, log it. That disagreement is + somebody attempting to authorize a device they do not own, and today nothing + would notice. +6. Check the locally-administered bit on the **derived** MAC. Record it on the + session; it is what triggers the coaching copy on the denial page. + +This replaces the hole the original author documented and declined to guess at: +every identifying value was request-supplied, so any signed-in account could +authorize somebody else's MAC up to `max_devices`. + +### Failure modes, deliberately closed + +- **IP matches no client** → refuse. +- **IP resolves to the proxy** → refuse, and report it distinctly (see below). +- **AP mismatch** → refuse. +- **Client not in the table yet** (associated, pre-DHCP) → retry twice at ~750ms, + then show "still connecting, try again" rather than a hard error. Bounded on + purpose: this runs inside a web request, and a longer wait would hold a worker + per connecting device at exactly the moment a class of them arrives at once. + +### The operational risk this creates + +Failing closed means it can fail closed **for everyone**. If `TrustProxies` is +not correct for Traefik, every request resolves to the proxy, no client ever +matches, and nobody can get on the Wi-Fi — a worse outage than the vulnerability +being fixed. + +Mitigation: the resolver distinguishes "no client matched this IP" from "this IP +is the proxy", and the second is surfaced as a health line on the Wi-Fi Devices +page. The failure becomes one glance to diagnose instead of a mystery. This is +also the single most important thing to verify against the live controller. + +### MAC randomization + +Normally stable per SSID, so it does not break the design. When it does rotate, +the device looks new: it consumes a slot and the user is asked to evict what +appears to be the same phone. With `max_devices: 1` this is the likeliest support +call. + +Handling: when the derived MAC is randomized **and** the user is at limit, the +denial page leads with an explanation — the device is using a private Wi-Fi +address, which is why it looks new — followed by the iOS and Android steps to +turn it off for this network, then the normal eviction list. Detection is the +locally-administered bit; no guessing. + +--- + +## C. Persistence, and the reconciler + +`session_minutes` is the lever for "no portal on every connection". Default to a +term (~120 days) rather than a year, so lost or handed-down devices age out. + +A scheduled reconciler (following `CaptureSnapshots` and the `unifi_cron_runs` +pattern) does three jobs: + +1. **Expire** rows past `expires_at` — fixes leak 2. +2. **Reconcile both directions** — revoke UniFi authorizations with no live row + (the orphans leak 1 produced), and expire rows UniFi no longer holds. +3. **Capture device type** onto the session from the client record — the + report-only fingerprint data for decision 6. + +Job 2 also heals controller-side loss: if UniFi forgets an authorization while our +row is still live and in-window (upgrade, restore), the reconciler re-authorizes +it. The user is not bounced to the portal for something that was not their doing. + +Windows are **fixed, not sliding** — a device's authorization ends when it ends, +so "one term" means one term. + +--- + +## D. The Wi-Fi Devices page + +New page under Unifi, gated on the existing `unifi.auth`, grouped by person. + +- **Person row:** name, role, quota usage ("staff · 2 of 4"). +- **Device rows:** hostname, device type (reported), MAC in `font-mono` as a + technical id, a "private address" marker when randomized, AP, SSID, authorized + at, expires at, and connected/away — display only. +- **Per-device disconnect** as a `.btn-icon-danger` row action behind the shared + confirm modal. Never a native `confirm()`. +- Search, plus a filter for over-quota people. +- The portal health line from section B. + +Follows the house table and empty-state conventions in the shell's `CLAUDE.md`. + +--- + +## E. Testing + +### Covered here, with `Http::fake()` for the controller + +- Both quota leaks, one regression test each: an offline device keeps its slot; an + expired row stops consuming one. +- `role` matching, and `sort_order` precedence including the staff-parent case. +- The reconciler in both directions, including the re-authorize-on-controller-loss + path. +- Randomized-MAC bit detection, both values. +- Resolver fail-closed paths: no client match, IP resolves to the proxy, AP + mismatch, client not yet in the table. +- That `?mac=` is ignored, and that a disagreement is logged. +- Authorization-not-connectivity: a slot survives the device going away. + +Plus a browser pass over the denial page (including the randomized-MAC copy), +self-service eviction, and the admin page — UI behaviour changes, so the +before-push browser rule applies. + +### NOT provable without the live controller + +- The actual parameters UniFi's redirect sends. +- Whether `dev_cat` is populated for these clients — `unifi_client_snapshots` has + zero rows on dev, so there is no evidence either way. +- Whether `TrustProxies` yields the true client IP through Traefik. + +These need a supervised session against real hardware, in the same spirit as the +Bosch cutover. The third is the one that gates release: it is the difference +between a closed hole and nobody getting online. + +--- + +## F. Rollout + +1. **Register the app.** Every route here 404s today, because `unifi` has no + `dashboard_apps` row and `app.access:unifi` aborts 404 when the lookup misses. + Nothing in this spec is reachable until that is resolved. Note `central-unifi` + declares `stability: stable`, so unlike On-Call this is a gap rather than a + decision — **and it is still an open question, not settled by this document.** +2. Verify the three hardware-dependent items above against the live controller. +3. Configure mappings: `role` → student (1 device) and staff (4 devices), with + `sort_order` set so multi-role people land deliberately. +4. Pilot with staff, then students. + +--- + +## Explicitly out of scope + +- **Enforcing** a no-phones rule. Decision 6 is report-only until the fingerprint + data is shown to be trustworthy. When it is, enforcement belongs in the + reconciler (authorize, then sweep), because UniFi's fingerprint is derived from + observed traffic and is not available at the moment of authorization. +- MAC OUI as a device-type signal. A randomized MAC has no real OUI, so this + would fail on phones — its intended target — while working on the school + laptops that should not be blocked. +- Blocking on the portal's User-Agent. Defeated by one "Request Desktop Site" tap. +- Tethering. A blocked phone can share a laptop's connection and vice versa; no + design here addresses that, and none is proposed. +- Any change to `disconnectSession`, which is already correct.