`unifi_vlan_mappings.session_minutes` is `unsignedInteger` (ceiling 4294967295)
but the mapping validator only said `integer|min:1`. Anything larger reached the
INSERT and raised SQLSTATE 22003, so the request died as an unhandled 500 where
a validation error belongs — the same shape just fixed in central-oncall#7.
Adds `max:525600` (a year of minutes) to both storeMapping and updateMapping.
Found while fixing #5, which reported a *silent* rejection rather than a crash.
The VLAN ID validation the issue was actually about was already correct on the
server (`min:1|max:4094` on all three forms); what was missing was any way to
see it, and that half of the fix is in the shell's Unifi/Portal.vue. Sibling
numeric fields were checked at the same time: `vlan_id` (unsignedSmallInteger,
bounded 1–4094) and `max_devices` (unsignedTinyInteger, bounded 1–50) were
already safe against their columns; `session_minutes` was the only one that
could overflow.
Version 1.13.3 -> 1.13.4.
Test: dashboard-shell tests/Feature/Unifi/VlanGroupValidationTest.php
Co-Authored-By: Claude <noreply@anthropic.com>
Found while triaging every authenticated route that carries no authorisation
middleware. Of 91 such routes this was the ONLY one that proved nothing: 35 scope to
$request->user(), 51 check ownership in the controller, 4 carry a credential in the URL.
TWO DEFECTS.
1. Arbitrary device admission. Everything identifying the device is request-supplied —
$clientMac = $request->input('mac', $request->input('id', '')) — and this endpoint
WRITES network state: authorizeGuest() puts a MAC on the mapped VLAN and a
PortalSession row is created. So any signed-in account, a student or a parent
included, could call
GET /portal/wifi/callback?mac=<a MAC they do not own>&url=…
and admit that device, up to the mapping's max_devices. No signature, no nonce, no
check that the MAC was even present on the network.
Fixed HALFWAY, on purpose. A well-formed MAC is now required, which stops the
parameter smuggling junk into authorizeGuest and into the session row — but it
cannot prove the caller owns the device, and the comment says so rather than
implying the hole is closed.
The likely proper fix is already in this codebase: UnifiApiClient::isClientConnected()
exists and is used a few lines below for stale-session cleanup, so requiring the MAC
to be currently associated would bind it. I did NOT add it, because if a pre-auth
guest reports as not-connected that check breaks WiFi onboarding for the entire
school, and there is no hardware here to test against. It needs one deliberate test
on real APs. The alternative is having UniFi sign the redirect.
2. Open redirect. `return redirect($request->input('url', '/'))`, unvalidated, on an
authenticated route — a phishing primitive serving from this school's own domain.
A captive portal exists to forward people to an ARBITRARY external site they were
already trying to reach, so an allowlist is not available. safeRedirectTarget()
instead accepts only absolute http/https URLs and site-relative paths, rejecting
javascript:, data: and protocol-relative "//evil.example" values that redirect()
would otherwise follow off-site. A rejected value falls back to '/' rather than
erroring: the guest is already online by then, and a bad `url` should not look like
a failed login.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDA3fQbsa3gaHCFoV1Q8Gg
- Add password rotation: RotatePasswords console command + migration + service updates
- Add PPSK management: UnifiPpsk model, migration, SyncPpskSchedules console
- Add VLAN groups and AP groups: VlanGroupController, ApGroupController, model, migration
- Add RebootAllAps console command
- Add in_alert column to device states
- Wire new features through service provider, routes, and existing controllers/services
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
