Plan 1 of 2: the BYOD Wi-Fi access model

Six TDD tasks covering the back-end half of the approved spec: the live-authorization
quota scope, dropping network presence as an eviction trigger, the role allow-list
with `default` removed, the IP-derived device resolver, randomized-address
detection, and the reconciler.

Split from the UI deliberately. Plan 1 leaves the portal correct and safe while
still being configured through the existing mappings CRUD, so it is worth shipping
on its own; Plan 2 covers the settings screen, the denial-page coaching copy and
the Wi-Fi Devices page.

Three things the self-review caught and the plan now handles:

  · the resolver had no retry, so a device that had only just associated — and so
    had no IP in UniFi's table yet — would have been told it was not connected at
    the exact moment it was connecting;
  · nothing captured the device fingerprint, which the report-only device-type
    decision depends on. The reconciler now does it, and only ever fills in: an
    unfingerprinted client must not blank a type already recorded, or the answer
    is lost every time the device happens to be quiet;
  · one task's test needed a method a later task introduced, and the first draft
    said to leave it red. Under subagent-driven execution a fresh agent per task
    would have seen red and tried to fix it, so the test now uses the parameter
    flow that exists at that point and the later task migrates it.

Also records what NOT to do: do not schedule the reconciler (it calls
unauthorizeGuest against a live controller that nothing here has been tested
against), do not register the unifi app, and do not touch disconnectSession.

docs/superpowers/specs/2026-08-21-byod-wifi-portal-hardening-design.md

@@ -0,0 +1,288 @@
+# 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 |
+| 7 | **Explicit allow-list only** — no `default` mapping | No Wi-Fi unless an admin has added that role and set an amount. A `default` row silently grants everyone, which is the opposite of the requirement |
+| 8 | Roles are chosen from a dropdown; session length entered in days | A free-text role slug that matches nothing fails silently — it presents as "this student has no Wi-Fi" with no error anywhere to explain it |
+
+---
+
+## 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=<ap_mac>&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.
+
+---
+
+## C2. Settings: who gets Wi-Fi, and how much
+
+Access is an **explicit allow-list**. A person gets Wi-Fi only if one of their
+roles has been added here with an amount. No role added means no Wi-Fi, and that
+is the intended answer rather than a gap to be filled by a catch-all.
+
+Consequences, stated so nobody re-adds the hole later:
+
+- **`default` is removed from the available match types.** A `default` row matches
+  everybody, so its existence makes the allow-list advisory. `resolveMapping()`
+  already returns null when nothing matches, and null already produces the
+  "No WiFi access configured for your account." denial — so deny-by-default is
+  the behaviour once the catch-all cannot be created.
+- Safe to remove now: **there are zero rows in `unifi_vlan_mappings` in any
+  environment**, so nothing is being revoked from anyone. Doing this later, after
+  a school had leaned on a catch-all, would cut off every unmapped person at once.
+- `email_domain`, `group` and `ou` stay available — they are narrow matches, not
+  catch-alls, and removing working options is not part of this.
+
+### The settings screen
+
+Extends the existing Portal settings page rather than adding another one. Per row:
+
+| Field | Control | Why |
+|---|---|---|
+| Role | **dropdown of real seeded roles** | Free text is the defect. `match_value` is a string compared exactly, so `Student` or `studnet` matches nobody and the failure surfaces as a pupil with no Wi-Fi and nothing logged. The dropdown makes an unmatchable value impossible to enter. |
+| Devices | number, 1..50 | The quota. Column is already bounded. |
+| Session length | **days, with presets** (a week / a month / a term / a year) | `session_minutes` stays the stored unit, but a term is 172,800 minutes and nobody should type that. Same reasoning as presenting cron schedules as presets rather than raw strings. |
+| VLAN | existing select | **Still required** — the network needs to know where to put the device, and this was not in the original request, so it is called out rather than quietly dropped. |
+| Order | existing `sort_order` | Precedence for multi-role people: staff above parent gives a staff-parent 4. |
+
+Roles already in the system and available to add: staff, student, parent,
+oncall-staff, coach, alumni, applicant, board-member, contact, homeschool,
+homeschool-parent, homeschool-student, open-house, tuition-payee, forms-payer.
+
+## 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.

src/Http/Controllers/GuestCardController.php

@@ -17,6 +17,26 @@ use Illuminate\Routing\Controller;
  */
 class GuestCardController extends Controller
 {
+    /**
+     * Is this card set up enough to ever appear?
+     *
+     * The three things payload() needs, asked as one question, so the permission
+     * screens can say "granted, but not configured yet" instead of leaving
+     * somebody staring at a home page wondering what they got wrong. The
+     * operator granted this key to two groups in production and saw nothing
+     * (2026-08-17); the card was simply never switched on.
+     *
+     * Note the third clause: last_password is written ONLY by
+     * unifi:rotate-passwords, so a school that sets a static guest password by
+     * hand and never rotates has no card and no obvious reason why.
+     */
+    public static function configured(): bool
+    {
+        return (bool) Setting::get('unifi.guest_card.enabled')
+            && trim((string) Setting::get('unifi.guest_card.ssid', '')) !== ''
+            && filled(Setting::get('unifi.password_rotation.last_password'));
+    }
+
     /** Payload shared by the card resolver and the poll endpoint. */
     public static function payload(): ?array
     {

src/UnifiServiceProvider.php

@@ -46,6 +46,10 @@ class UnifiServiceProvider extends ServiceProvider
                 fn (\App\Models\User $user) => \Dashboard\Unifi\Http\Controllers\GuestCardController::resolve($user),
                 'The guest network name, current password, and a join QR code — updates automatically when the password rotates.',
                 permission: 'unifi.guest_wifi',
+                // So the permission screens can say "granted, but not switched
+                // on yet" rather than leaving a grantee staring at a home page
+                // with nothing new on it.
+                configured: fn () => \Dashboard\Unifi\Http\Controllers\GuestCardController::configured(),
             );
         } catch (\Throwable) {
             // Older shell without the card registry — skip.