Found while implementing the resolver. The spec claimed the difference between "no
client matched this IP" and "this IP is the proxy" made a TrustProxies
misconfiguration one glance to diagnose. That holds only for the case where the
forwarded header arrives and TrustProxies fails to apply it.
If Traefik is misconfigured such that no X-Forwarded-For is sent at all, the
resolver sees a plain unmatched IP and reports no_client — which reads to the
person in front of it as "your device is not on the network", and to an operator
as nothing at all. The alarm is silent in precisely one of the two ways the thing
it guards can break.
Recorded rather than patched, because the fix belongs to Plan 2's health line: it
should show the resolved client IP and say what it is, instead of reporting a
reason code. And the hardware validation step now has to confirm the true client
IP actually arrives — confirming the alarm is quiet is not evidence, since a quiet
alarm is what this blind spot produces.
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.
Two changes from the operator.
Session length and devices-per-role are set in the settings UI, on the existing
Portal settings page rather than a new one. Two presentation decisions that are
really correctness decisions:
· the role is picked from a DROPDOWN of seeded roles, not typed. match_value is
compared exactly, so "Student" or "studnet" matches nobody and the failure
surfaces as a pupil with no Wi-Fi and nothing logged anywhere. A dropdown
makes an unmatchable value impossible to enter.
· session length is entered in DAYS with presets. session_minutes stays the
stored unit, but a term is 172,800 minutes and nobody should type that — the
same reasoning that presents cron schedules as presets instead of raw strings.
And access is now an explicit allow-list: no role added means no Wi-Fi. That makes
`default` an anti-feature, because a default row matches everybody and turns the
allow-list into a suggestion, so it is removed from the available match types.
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 as soon as the catch-all cannot be created.
Safe to do now specifically because unifi_vlan_mappings has ZERO rows in every
environment — nothing is revoked from anybody. The same change made after a school
had leaned on a catch-all would cut off every unmapped person at once.
VLAN stays required per row. It was not in the request, so it is called out in the
spec rather than quietly dropped: the network still has to know where to put the
device.
Brainstormed spec for registered guest/BYOD Wi-Fi. The headline finding is that
this is not a new feature: the portal, per-cohort device limits, per-cohort VLANs,
session length, self-service eviction and admin eviction all already exist here,
and CaptureSnapshots is already collecting device fingerprints. So the spec is
about what is broken and what is missing, not what to build from scratch.
Two defects in quota accounting, in opposite directions, both of which get worse
the longer session_minutes is — and long sessions are the whole point of "do not
make users re-do the portal every connection":
· a slot is freed when isClientConnected() is false, so a phone that leaves the
building frees its slot while UniFi still holds it authorized — the user then
authorizes a second device and one of them is an orphan nothing tracks;
· expires_at is written and never read, and no job expires sessions, so a row
keeps consuming a slot long after UniFi stopped honouring it.
The fix is to count live AUTHORIZATIONS rather than present devices, and to let a
reconciler repair both directions.
Also specified: closing the MAC-ownership hole the original author documented and
declined to guess at, by deriving the MAC from the request's source IP and
verifying AP association instead of trusting ?mac=. That fails closed, which is
correct — and the spec is explicit that it can therefore fail closed for
EVERYONE if TrustProxies is wrong behind Traefik, so the resolver has to report
that case distinctly and it is the one thing to verify against real hardware
before release.
Device-type detection (keeping students off phones) is specified as report-only.
UniFi's fingerprint is derived from observed traffic and so is not available at
the moment of authorization, and unifi_client_snapshots has zero rows on dev, so
there is no evidence dev_cat is populated in this environment. Enforcing on
unvalidated data would lock out the wrong people. MAC OUI is rejected outright:
a randomized MAC has no real OUI, so it would fail on phones while working on the
laptops that should not be blocked.
No version bump: documentation only, no runtime change, and bumping would put the
repo ahead of prod for a file that cannot affect it.
