From dbebb6c96412cd1a71b38fca5b0c26992509ef16 Mon Sep 17 00:00:00 2001 From: jwed Date: Fri, 21 Aug 2026 12:56:23 -0400 Subject: [PATCH] Plan 1 of 2: the BYOD Wi-Fi access model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../2026-08-21-byod-wifi-access-model.md | 1415 +++++++++++++++++ 1 file changed, 1415 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-21-byod-wifi-access-model.md diff --git a/docs/superpowers/plans/2026-08-21-byod-wifi-access-model.md b/docs/superpowers/plans/2026-08-21-byod-wifi-access-model.md new file mode 100644 index 0000000..5f65714 --- /dev/null +++ b/docs/superpowers/plans/2026-08-21-byod-wifi-access-model.md @@ -0,0 +1,1415 @@ +# BYOD Wi-Fi Access Model Implementation Plan (Plan 1 of 2) + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make Wi-Fi access correct and safe — count live authorizations instead of present devices, prove the device belongs to the person signing in, and gate access on an explicit per-role allow-list. + +**Architecture:** `PortalController::captiveCallback` stops trusting the `?mac=` parameter and stops using network presence as a quota signal. A new `DeviceResolver` service identifies the device from the request's source IP via UniFi's client table and refuses if it cannot prove association to the AP that issued the redirect. A `live` scope on `PortalSession` becomes the single definition of a consumed slot, and a scheduled reconciler repairs drift between our rows and UniFi's authorizations. + +**Tech Stack:** Laravel 11 snap-in (`Dashboard\Unifi`), MariaDB, Inertia/Vue for the pages it renders, PHPUnit. Tests live in `dashboard-shell/tests` even for snap-in code. + +**Out of scope (Plan 2):** the settings screen (role dropdown, day presets, removing the `default` option from the UI), the denial page's randomized-MAC coaching copy, the Wi-Fi Devices admin page, browser E2E. + +--- + +## Before you start + +**Run tests like this.** Claim a database slot once, reuse it, release at the end: + +```bash +cd /project/dashboard-shell +DB=$(bin/agentdb claim --agent wifi-plan1) # prints e.g. dashboard_testing_2 +bin/agentdb test "$DB" tests/Feature/Unifi/ +# ...at the very end: +bin/agentdb release "$DB" +``` + +Never run `php artisan test` directly and never pass `-e DB_DATABASE=` — both silently use the shared database. + +**Two repos, one feature.** Snap-in code is in `/project/central-unifi`; every test file is in `/project/dashboard-shell/tests`. Commit each repo separately with `git -C commit -- `; never `git add -A` (other agents share this tree). + +**Bump `central-unifi/composer.json` patch version** on the last commit only, not every task. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `central-unifi/src/Models/PortalSession.php` (modify) | Owns the definition of a consumed slot (`scopeLive`) | +| `central-unifi/src/Services/DeviceResolver.php` (create) | Turns a request + AP mac into a trusted client MAC, or a refusal reason. No knowledge of quotas | +| `central-unifi/src/Support/MacAddress.php` (create) | Pure MAC helpers — normalise, detect randomized. No dependencies | +| `central-unifi/src/Services/UnifiApiClient.php` (modify) | Gains `findClientByIp()` and `getAuthorizedGuestMacs()` | +| `central-unifi/src/Http/Controllers/PortalController.php` (modify) | Orchestrates: resolve → resolve mapping → count live → authorize or deny | +| `central-unifi/src/Console/ReconcilePortal.php` (create) | Scheduled repair of drift in both directions | +| `central-unifi/database/migrations/2026_08_21_000001_add_is_randomized_to_portal_sessions.php` (create) | One nullable boolean column | + +`DeviceResolver` is deliberately separate from `PortalController`: it is the security boundary, it has four distinct refusal modes, and it needs to be testable without touching quotas or Inertia. + +--- + +## Task 1: A live authorization is the unit of quota + +**Files:** +- Modify: `/project/central-unifi/src/Models/PortalSession.php` +- Test: `/project/dashboard-shell/tests/Feature/Unifi/PortalSessionLiveScopeTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/PortalSessionLiveScopeTest.php`: + +```php + User::factory()->create(['is_active' => true])->id, + 'mac_address' => '02:11:22:33:44:55', + 'is_active' => true, + 'authorized_at' => now()->subHour(), + 'expires_at' => now()->addDay(), + ], $overrides)); + } + + public function test_an_unexpired_active_session_is_live(): void + { + $this->session(); + $this->assertSame(1, PortalSession::live()->count()); + } + + public function test_an_expired_session_is_not_live(): void + { + $this->session(['expires_at' => now()->subMinute()]); + $this->assertSame(0, PortalSession::live()->count(), + 'expires_at was written and never read — an expired row kept consuming a slot'); + } + + public function test_an_inactive_session_is_not_live(): void + { + $this->session(['is_active' => false]); + $this->assertSame(0, PortalSession::live()->count()); + } + + public function test_a_session_with_no_expiry_is_not_live(): void + { + $this->session(['expires_at' => null]); + $this->assertSame(0, PortalSession::live()->count(), + 'a NULL expiry is a legacy row from before this was enforced, not an eternal grant'); + } +} +``` + +- [ ] **Step 2: Run it and confirm it fails** + +```bash +cd /project/dashboard-shell +bin/agentdb test "$DB" tests/Feature/Unifi/PortalSessionLiveScopeTest.php +``` + +Expected: FAIL — `Call to undefined method ...PortalSession::live()`. + +- [ ] **Step 3: Add the scope** + +In `/project/central-unifi/src/Models/PortalSession.php`, add inside the class: + +```php + /** + * The rows that consume a device slot. + * + * Presence on the network is deliberately NOT part of this. UniFi holds an + * authorization for its full window whether or not the device is in the + * building, so a phone that went home still owns its slot — freeing it here + * is what let one person hold two authorizations against a one-device quota. + * + * A NULL expires_at is not live: it predates this being enforced. + */ + public function scopeLive($query) + { + return $query->where('is_active', true) + ->whereNotNull('expires_at') + ->where('expires_at', '>', now()); + } +``` + +- [ ] **Step 4: Run it and confirm it passes** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/PortalSessionLiveScopeTest.php +``` + +Expected: PASS, 4 tests. + +- [ ] **Step 5: Commit** + +```bash +git -C /project/central-unifi commit -m "A live authorization is what consumes a Wi-Fi device slot" -- src/Models/PortalSession.php +git -C /project/dashboard-shell add tests/Feature/Unifi/PortalSessionLiveScopeTest.php +git -C /project/dashboard-shell commit -m "A live authorization is what consumes a Wi-Fi device slot" -- tests/Feature/Unifi/PortalSessionLiveScopeTest.php +``` + +--- + +## Task 2: Presence stops freeing slots + +**Files:** +- Modify: `/project/central-unifi/src/Http/Controllers/PortalController.php` (the `foreach ($activeSessions ...)` block and the count below it) +- Modify: `/project/dashboard-shell/tests/Feature/Unifi/PortalDeniedTest.php` (its fixtures predate `expires_at` mattering) +- Test: `/project/dashboard-shell/tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php`: + +```php +addRole('student'); + VlanMapping::create([ + 'group_name' => 'Students', + 'match_type' => 'role', + 'match_value' => 'student', + 'vlan_id' => 20, + 'max_devices' => 1, + 'session_minutes' => 60, + 'sort_order' => 1, + ]); + } + + public function test_an_absent_device_still_holds_its_slot(): void + { + $user = User::factory()->create(['is_active' => true]); + $this->oneDeviceMappingFor($user); + + PortalSession::create([ + 'user_id' => $user->id, + 'mac_address' => '02:aa:bb:cc:dd:ee', + 'is_active' => true, + 'authorized_at' => now()->subHour(), + 'expires_at' => now()->addHour(), + ]); + + // The device is NOT on the network, and that must change nothing. + $this->mock(UnifiApiClient::class, function ($mock) { + $mock->shouldReceive('isClientConnected')->andReturn(false); + $mock->shouldNotReceive('authorizeGuest'); + }); + + // The MAC still arrives as a parameter at this point in the plan; Task 4 + // replaces that and updates this test with it. + $response = $this->actingAs($user->fresh()) + ->get(route('unifi.portal.callback', [ + 'mac' => '02:99:88:77:66:55', 'ap' => 'ac:8b:a9:00:00:01', 'ssid' => 'BYOD', + ])); + + $response->assertOk(); + $response->assertInertia(fn ($page) => $page->component('Unifi/PortalDenied')); + $this->assertSame(1, PortalSession::live()->where('user_id', $user->id)->count(), + 'the absent device must still own its slot'); + } + + public function test_an_expired_session_does_not_block_a_new_device(): void + { + $user = User::factory()->create(['is_active' => true]); + $this->oneDeviceMappingFor($user); + + PortalSession::create([ + 'user_id' => $user->id, + 'mac_address' => '02:aa:bb:cc:dd:ee', + 'is_active' => true, + 'authorized_at' => now()->subDays(2), + 'expires_at' => now()->subDay(), + ]); + + $this->mock(UnifiApiClient::class, function ($mock) { + $mock->shouldReceive('isClientConnected')->andReturn(false); + $mock->shouldReceive('authorizeGuest')->once()->andReturn([]); + }); + + $this->actingAs($user->fresh()) + ->get(route('unifi.portal.callback', [ + 'mac' => '02:99:88:77:66:55', 'ap' => 'ac:8b:a9:00:00:01', 'ssid' => 'BYOD', + ])) + ->assertRedirect(); + + $this->assertDatabaseHas('unifi_portal_sessions', [ + 'user_id' => $user->id, 'mac_address' => '02:99:88:77:66:55', 'is_active' => 1, + ]); + } +} +``` + +- [ ] **Step 2: Run it and confirm it fails** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php +``` + +Expected: FAIL — `role` is not a match type yet, so `resolveMapping()` returns null and both cases render the "no access configured" denial instead of the quota one. + +Do Task 3 first if you want this green immediately; the two are independent and either order works. Nothing here is left knowingly red. + +- [ ] **Step 3: Replace the presence sweep with a live count** + +In `/project/central-unifi/src/Http/Controllers/PortalController.php`, delete the whole `$activeSessions = ...` block, the `foreach ($activeSessions as $session)` loop, and the `$activeCount = ...` line, and put this in their place: + +```php + /* + * Presence is NOT the question — a live authorization is. + * + * This used to walk the user's sessions and mark any device UniFi did not + * currently see as stale, freeing its slot. But UniFi holds an + * authorization for its whole window regardless of whether the device is + * in the building, so a phone that went home had its slot freed while + * still being authorized: the owner then authorized a second device and + * one of the two was an orphan nothing tracked. The longer + * session_minutes is — and it is now meant to be a term — the more of + * those accumulate. + * + * Slots are now freed by eviction, by expiry, or by ReconcilePortal. + */ + $live = PortalSession::live()->where('user_id', $user->id)->get(); + + // The same device coming back is a renewal, not a new device: it keeps + // its slot and its row, and simply gets a fresh window. + $existing = $live->firstWhere('mac_address', $clientMac); + + if (! $existing && $live->count() >= $mapping->max_devices) { + return Inertia::render('Unifi/PortalDenied', [ + 'reason' => "You already have {$live->count()} device(s) connected (limit: {$mapping->max_devices}).", + 'sessions' => $live->map(fn ($s) => [ + 'id' => $s->id, + 'mac' => $s->mac_address, + 'hostname' => $s->device_hostname, + 'os' => $s->device_os, + 'since' => $s->authorized_at->toISOString(), + ]), + 'can_disconnect' => true, + ]); + } +``` + +Then, further down, replace the `PortalSession::create([...])` call with an upsert so a renewal reuses its row: + +```php + $unifi->authorizeGuest($clientMac, $minutes); + + PortalSession::updateOrCreate( + ['user_id' => $user->id, 'mac_address' => $clientMac], + [ + 'device_hostname' => $request->input('hostname'), + 'device_os' => $request->input('os'), + 'device_type' => $request->input('dev_cat'), + 'ssid' => $ssid, + 'vlan_id' => $mapping->vlan_id, + 'ap_mac' => $apMac, + 'is_active' => true, + 'authorized_at' => now(), + 'expires_at' => now()->addMinutes($minutes), + ] + ); +``` + +- [ ] **Step 4: Fix the existing test's fixtures** + +`tests/Feature/Unifi/PortalDeniedTest.php` creates sessions without `expires_at`, which are now not live. In `test_a_user_over_the_device_limit_sees_their_devices` and `test_an_ordinary_user_can_disconnect_their_own_device`, add `'expires_at' => now()->addDay(),` to every `PortalSession::create([...])`. Its `shouldReceive('isClientConnected')` line can stay — it is now simply never called. + +- [ ] **Step 5: Run the whole Unifi suite** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ +``` + +Expected: `PortalDeniedTest` and `PortalSessionLiveScopeTest` PASS. `PortalQuotaCountsAuthorizationsTest` still FAILS until Task 4. Nothing else regresses. + +- [ ] **Step 6: Commit** + +```bash +git -C /project/central-unifi commit -m "Network presence no longer frees a device slot" -- src/Http/Controllers/PortalController.php +git -C /project/dashboard-shell add tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php +git -C /project/dashboard-shell commit -m "Network presence no longer frees a device slot" -- tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php tests/Feature/Unifi/PortalDeniedTest.php +``` + +--- + +## Task 3: Access is an explicit per-role allow-list + +**Files:** +- Modify: `/project/central-unifi/src/Http/Controllers/PortalController.php` (`resolveMapping`, and the two `match_type` validation rules) +- Test: `/project/dashboard-shell/tests/Feature/Unifi/WifiRoleAllowListTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/WifiRoleAllowListTest.php`: + +```php +create(['is_active' => true]); + $u->groups()->attach(Group::where('is_super', true)->first()); + + return $u->fresh(); + } + + private function mapping(array $overrides = []): array + { + return array_merge([ + 'group_name' => 'Students', + 'match_type' => 'role', + 'match_value' => 'student', + 'vlan_id' => 20, + 'max_devices' => 1, + 'session_minutes' => 60, + ], $overrides); + } + + public function test_a_role_mapping_can_be_created(): void + { + $this->actingAs($this->super()) + ->post(route('unifi.portal.mappings.store'), $this->mapping()) + ->assertRedirect(); + + $this->assertDatabaseHas('unifi_vlan_mappings', ['match_type' => 'role', 'match_value' => 'student']); + } + + public function test_a_default_catch_all_mapping_is_refused(): void + { + $this->actingAs($this->super()) + ->post(route('unifi.portal.mappings.store'), $this->mapping(['match_type' => 'default', 'match_value' => '*'])) + ->assertSessionHasErrors('match_type'); + + $this->assertDatabaseCount('unifi_vlan_mappings', 0); + } + + public function test_a_user_holding_the_mapped_role_resolves_to_it(): void + { + $user = User::factory()->create(['is_active' => true]); + $user->addRole('student'); + $this->assertTrue($user->fresh()->hasRole('student'), + 'addRole() silently drops an unseeded slug, which would make this pass for the wrong reason'); + + VlanMapping::create($this->mapping() + ['sort_order' => 1]); + + $this->assertSame(1, $this->resolvedMaxDevices($user->fresh())); + } + + public function test_a_user_holding_no_mapped_role_gets_nothing(): void + { + $user = User::factory()->create(['is_active' => true]); + VlanMapping::create($this->mapping() + ['sort_order' => 1]); + + $this->assertNull($this->resolvedMaxDevices($user->fresh()), + 'no mapping must mean no Wi-Fi, not a fallback'); + } + + /** sort_order decides for somebody who holds two mapped roles. */ + public function test_precedence_follows_sort_order(): void + { + $user = User::factory()->create(['is_active' => true]); + $user->addRole('staff'); + $user->addRole('parent'); + + VlanMapping::create($this->mapping(['group_name' => 'Staff', 'match_value' => 'staff', 'max_devices' => 4]) + ['sort_order' => 1]); + VlanMapping::create($this->mapping(['group_name' => 'Parents', 'match_value' => 'parent', 'max_devices' => 2]) + ['sort_order' => 2]); + + $this->assertSame(4, $this->resolvedMaxDevices($user->fresh()), + 'a staff member who is also a parent should get the staff allowance'); + } + + /** Exercises resolveMapping through the only public door it has. */ + private function resolvedMaxDevices(User $user): ?int + { + $method = new \ReflectionMethod(\Dashboard\Unifi\Http\Controllers\PortalController::class, 'resolveMapping'); + $method->setAccessible(true); + $mapping = $method->invoke(app(\Dashboard\Unifi\Http\Controllers\PortalController::class), $user); + + return $mapping?->max_devices; + } +} +``` + +- [ ] **Step 2: Run it and confirm it fails** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/WifiRoleAllowListTest.php +``` + +Expected: FAIL — the `role` match returns false (no such branch) and `default` is still accepted. + +- [ ] **Step 3: Add `role`, remove `default`** + +In `resolveMapping()`, replace the `match` block with: + +```php + $matched = match ($mapping->match_type) { + 'email_domain' => str_ends_with($user->email ?? '', '@' . $mapping->match_value), + 'group' => $user->groups?->contains('name', $mapping->match_value) ?? false, + 'ou' => str_contains($user->ou ?? '', $mapping->match_value), + /* + * Roles are the platform's own authority and need no external + * sync to be correct, which is why quotas key off them. + * + * `default` is deliberately absent. It matched everybody, so its + * existence made this an advisory list rather than an allow-list. + * Returning null here is already the deny path: captiveCallback + * renders "No WiFi access configured for your account." + */ + 'role' => $user->hasRole($mapping->match_value), + default => false, + }; +``` + +In **both** `storeMapping()` and `updateMapping()`, change the match_type rule: + +```php + 'match_type' => 'required|in:ou,group,email_domain,role', +``` + +- [ ] **Step 4: Run it and confirm it passes** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/WifiRoleAllowListTest.php +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git -C /project/central-unifi commit -m "Wi-Fi access becomes an explicit per-role allow-list" -- src/Http/Controllers/PortalController.php +git -C /project/dashboard-shell add tests/Feature/Unifi/WifiRoleAllowListTest.php +git -C /project/dashboard-shell commit -m "Wi-Fi access becomes an explicit per-role allow-list" -- tests/Feature/Unifi/WifiRoleAllowListTest.php +``` + +--- + +## Task 4: Prove the device belongs to the person + +**Files:** +- Create: `/project/central-unifi/src/Support/MacAddress.php` +- Create: `/project/central-unifi/src/Services/DeviceResolver.php` +- Modify: `/project/central-unifi/src/Services/UnifiApiClient.php` (add `findClientByIp`) +- Modify: `/project/central-unifi/src/Http/Controllers/PortalController.php` (use the resolver instead of `?mac=`) +- Test: `/project/dashboard-shell/tests/Feature/Unifi/DeviceResolverTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/DeviceResolverTest.php`: + +```php + and admit a + * device they did not own, up to max_devices. The MAC is now derived from the + * request's source IP via UniFi's own client table, and the client must be + * associated to the AP that issued the redirect. + * + * Every failure mode refuses. That is deliberate, and it is also the risk: if + * TrustProxies is wrong behind Traefik, every request resolves to the proxy and + * NOBODY gets on the Wi-Fi. So "this IP is the proxy" is a distinct, reportable + * outcome rather than just another miss. + */ +class DeviceResolverTest extends TestCase +{ + private function resolver(array $client = null): DeviceResolver + { + $mock = \Mockery::mock(UnifiApiClient::class); + $mock->shouldReceive('findClientByIp')->andReturn($client); + $this->app->instance(UnifiApiClient::class, $mock); + + return app(DeviceResolver::class); + } + + private function request(array $server = [], array $headers = []): Request + { + $r = Request::create('/portal/wifi/callback', 'GET', [], [], [], array_merge(['REMOTE_ADDR' => '10.20.0.9'], $server)); + foreach ($headers as $k => $v) { + $r->headers->set($k, $v); + } + + return $r; + } + + public function test_it_derives_the_mac_from_the_source_ip(): void + { + $result = $this->resolver([ + 'mac' => '02:aa:bb:cc:dd:ee', 'ap_mac' => 'ac:8b:a9:00:00:01', 'ip' => '10.20.0.9', + ])->resolve($this->request(), 'ac:8b:a9:00:00:01'); + + $this->assertTrue($result->ok); + $this->assertSame('02:aa:bb:cc:dd:ee', $result->mac); + } + + public function test_it_refuses_when_no_client_matches_the_ip(): void + { + $result = $this->resolver(null)->resolve($this->request(), 'ac:8b:a9:00:00:01'); + + $this->assertFalse($result->ok); + $this->assertSame('no_client', $result->reason); + } + + public function test_it_refuses_when_the_client_is_on_a_different_ap(): void + { + $result = $this->resolver([ + 'mac' => '02:aa:bb:cc:dd:ee', 'ap_mac' => 'ac:8b:a9:00:00:99', 'ip' => '10.20.0.9', + ])->resolve($this->request(), 'ac:8b:a9:00:00:01'); + + $this->assertFalse($result->ok); + $this->assertSame('ap_mismatch', $result->reason); + } + + /** + * The outage case, reported distinctly. A forwarded header present while the + * request IP still equals REMOTE_ADDR means TrustProxies is not applying. + */ + public function test_it_reports_an_untrusted_proxy_separately(): void + { + $result = $this->resolver(null)->resolve( + $this->request([], ['X-Forwarded-For' => '10.20.0.9']), + 'ac:8b:a9:00:00:01' + ); + + $this->assertFalse($result->ok); + $this->assertSame('proxy_untrusted', $result->reason); + } + + /** + * A device that has just associated has no IP yet, so it is briefly absent + * from UniFi's table. Refusing on the first miss would tell a pupil their + * device is not connected at the exact moment it is connecting. + */ + public function test_it_retries_before_giving_up_on_a_missing_client(): void + { + $mock = \Mockery::mock(UnifiApiClient::class); + $mock->shouldReceive('findClientByIp')->times(3)->andReturn(null); + $this->app->instance(UnifiApiClient::class, $mock); + + $result = app(DeviceResolver::class)->resolve($this->request(), 'ac:8b:a9:00:00:01'); + + $this->assertFalse($result->ok); + $this->assertSame('no_client', $result->reason); + } + + /** It must stop retrying the moment it succeeds. */ + public function test_it_does_not_retry_once_it_has_the_client(): void + { + $mock = \Mockery::mock(UnifiApiClient::class); + $mock->shouldReceive('findClientByIp')->once()->andReturn([ + 'mac' => '02:aa:bb:cc:dd:ee', 'ap_mac' => 'ac:8b:a9:00:00:01', 'ip' => '10.20.0.9', + ]); + $this->app->instance(UnifiApiClient::class, $mock); + + $this->assertTrue(app(DeviceResolver::class)->resolve($this->request(), 'ac:8b:a9:00:00:01')->ok); + } + + public function test_it_ignores_a_supplied_mac_entirely(): void + { + $request = $this->request(); + $request->query->set('mac', '02:99:99:99:99:99'); // somebody else's device + + $result = $this->resolver([ + 'mac' => '02:aa:bb:cc:dd:ee', 'ap_mac' => 'ac:8b:a9:00:00:01', 'ip' => '10.20.0.9', + ])->resolve($request, 'ac:8b:a9:00:00:01'); + + $this->assertSame('02:aa:bb:cc:dd:ee', $result->mac, + 'the derived MAC must win; a supplied one is never trusted'); + $this->assertTrue($result->macParameterDisagreed, + 'a disagreement is somebody authorizing a device they do not own, and must be visible'); + } +} +``` + +- [ ] **Step 2: Run it and confirm it fails** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/DeviceResolverTest.php +``` + +Expected: FAIL — `Class "Dashboard\Unifi\Services\DeviceResolver" not found`. + +- [ ] **Step 3: Add the MAC helper** + +Create `/project/central-unifi/src/Support/MacAddress.php`: + +```php +getActiveClients() as $client) { + if (($client['ip'] ?? null) === $ip) { + return $client; + } + } + + return null; + } +``` + +- [ ] **Step 5: Add the resolver** + +Create `/project/central-unifi/src/Services/DeviceResolver.php`: + +```php +ip(); + + /* + * A device that has only just associated may have no IP yet, so UniFi's + * client table does not know it. Retry briefly rather than telling a + * pupil their device is not connected when it is about to be. + * + * 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. + */ + $client = null; + for ($attempt = 1; $attempt <= self::ATTEMPTS; $attempt++) { + $client = $this->unifi->findClientByIp($ip); + if ($client) { + break; + } + if ($attempt < self::ATTEMPTS) { + usleep(self::PAUSE_US); + } + } + + if (! $client) { + // A forwarded header present while the request IP is still the raw + // socket address means TrustProxies is not applying, so every lookup + // here is doomed for the same reason. Say which failure this is. + $forwarded = $request->headers->get('X-Forwarded-For'); + $raw = $request->server('REMOTE_ADDR'); + $reason = ($forwarded && $ip === $raw) ? 'proxy_untrusted' : 'no_client'; + + return new ResolvedDevice(ok: false, reason: $reason); + } + + $mac = MacAddress::normalise($client['mac'] ?? null); + if (! $mac) { + return new ResolvedDevice(ok: false, reason: 'no_client'); + } + + $clientAp = MacAddress::normalise($client['ap_mac'] ?? null); + if (! $clientAp || $clientAp !== MacAddress::normalise($apMac)) { + return new ResolvedDevice(ok: false, reason: 'ap_mismatch'); + } + + $supplied = MacAddress::normalise($request->input('mac')); + $disagreed = $supplied !== null && $supplied !== $mac; + + if ($disagreed) { + Log::warning('unifi.portal: supplied MAC disagreed with the derived one', [ + 'derived' => $mac, 'supplied' => $supplied, 'ip' => $ip, 'ap' => $apMac, + ]); + } + + return new ResolvedDevice( + ok: true, + mac: $mac, + isRandomized: MacAddress::isRandomized($mac), + macParameterDisagreed: $disagreed, + client: $client, + ); + } +} +``` + +- [ ] **Step 6: Run it and confirm it passes** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/DeviceResolverTest.php +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 7: Use it in the controller** + +In `captiveCallback()`, delete the `$clientMac = strtolower($request->input('mac', ...))` line, the `if (! $clientMac)` guard, and the whole `preg_match` block with its security comment. Replace with: + +```php + $apMac = (string) $request->input('ap', ''); + $ssid = (string) $request->input('ssid', ''); + + $device = app(\Dashboard\Unifi\Services\DeviceResolver::class)->resolve($request, $apMac); + + if (! $device->ok) { + return Inertia::render('Unifi/PortalDenied', [ + 'reason' => match ($device->reason) { + 'ap_mismatch' => 'That device is not connected to this access point. Reconnect and try again.', + 'proxy_untrusted' => 'Wi-Fi sign-in is misconfigured and cannot identify your device. Please tell IT.', + default => 'Still connecting — wait a moment and try again.', + }, + 'resolveFailure' => $device->reason, + ]); + } + + $clientMac = $device->mac; + $isRandomized = $device->isRandomized; +``` + +- [ ] **Step 7b: Move the Task 2 test onto the derived MAC** + +`PortalQuotaCountsAuthorizationsTest` still passes `?mac=`, which is now ignored. In both tests, drop the `'mac' => '02:99:88:77:66:55',` query parameter, add `->withServerVariables(['REMOTE_ADDR' => '10.20.0.9'])` before `->get(...)`, and add this to each mock closure: + +```php + $mock->shouldReceive('findClientByIp')->andReturn([ + 'mac' => '02:99:88:77:66:55', 'ap_mac' => 'ac:8b:a9:00:00:01', 'ip' => '10.20.0.9', + ]); +``` + +The assertions do not change — the same device is identified a trustworthy way. + +- [ ] **Step 8: Run the whole Unifi suite** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ +``` + +Expected: everything PASSES. + +- [ ] **Step 9: Commit** + +```bash +git -C /project/central-unifi commit -m "The portal derives the device MAC instead of trusting the caller" -- src/Support/MacAddress.php src/Services/DeviceResolver.php src/Services/UnifiApiClient.php src/Http/Controllers/PortalController.php +git -C /project/dashboard-shell add tests/Feature/Unifi/DeviceResolverTest.php +git -C /project/dashboard-shell commit -m "The portal derives the device MAC instead of trusting the caller" -- tests/Feature/Unifi/DeviceResolverTest.php tests/Feature/Unifi/PortalQuotaCountsAuthorizationsTest.php +``` + +--- + +## Task 5: Record whether the address was randomized + +**Files:** +- Create: `/project/central-unifi/database/migrations/2026_08_21_000001_add_is_randomized_to_portal_sessions.php` +- Modify: `/project/central-unifi/src/Models/PortalSession.php` (`$fillable`, `$casts`) +- Modify: `/project/central-unifi/src/Http/Controllers/PortalController.php` (write it on the session) +- Test: `/project/dashboard-shell/tests/Feature/Unifi/RandomizedMacIsRecordedTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/RandomizedMacIsRecordedTest.php`: + +```php +assertTrue(MacAddress::isRandomized('02:aa:bb:cc:dd:ee')); + $this->assertTrue(MacAddress::isRandomized('a6:11:22:33:44:55')); + } + + /** A real Ubiquiti prefix does not. */ + public function test_a_manufacturer_address_is_not(): void + { + $this->assertFalse(MacAddress::isRandomized('ac:8b:a9:00:00:01')); + $this->assertFalse(MacAddress::isRandomized('f0:9f:c2:11:22:33')); + } + + public function test_it_normalises_before_judging(): void + { + $this->assertSame('02:aa:bb:cc:dd:ee', MacAddress::normalise('02-AA-BB-CC-DD-EE')); + $this->assertNull(MacAddress::normalise('not a mac')); + } +} +``` + +- [ ] **Step 2: Run it and confirm it passes already** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/RandomizedMacIsRecordedTest.php +``` + +Expected: PASS — `MacAddress` landed in Task 4. This test exists to pin the bit arithmetic, which is the part nobody will remember. + +- [ ] **Step 3: Add the column** + +Create `/project/central-unifi/database/migrations/2026_08_21_000001_add_is_randomized_to_portal_sessions.php`: + +```php +boolean('is_randomized')->nullable()->after('mac_address'); + }); + } + + public function down(): void + { + Schema::table('unifi_portal_sessions', function (Blueprint $table) { + $table->dropColumn('is_randomized'); + }); + } +}; +``` + +- [ ] **Step 4: Store it** + +In `PortalSession.php` add `'is_randomized'` to `$fillable` and `'is_randomized' => 'boolean'` to `$casts`. + +In `PortalController::captiveCallback`, add to the `updateOrCreate` payload from Task 2: + +```php + 'is_randomized' => $isRandomized, +``` + +- [ ] **Step 5: Run the suite** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ +``` + +Expected: all PASS. + +- [ ] **Step 6: Commit** + +```bash +git -C /project/central-unifi commit -m "Record whether a portal session used a private Wi-Fi address" -- database/migrations/2026_08_21_000001_add_is_randomized_to_portal_sessions.php src/Models/PortalSession.php src/Http/Controllers/PortalController.php +git -C /project/dashboard-shell add tests/Feature/Unifi/RandomizedMacIsRecordedTest.php +git -C /project/dashboard-shell commit -m "Record whether a portal session used a private Wi-Fi address" -- tests/Feature/Unifi/RandomizedMacIsRecordedTest.php +``` + +--- + +## Task 6: The reconciler + +**Files:** +- Create: `/project/central-unifi/src/Console/ReconcilePortal.php` +- Modify: `/project/central-unifi/src/Services/UnifiApiClient.php` (add `getAuthorizedGuestMacs`) +- Modify: `/project/central-unifi/src/UnifiServiceProvider.php` (register the command) +- Test: `/project/dashboard-shell/tests/Feature/Unifi/ReconcilePortalTest.php` + +- [ ] **Step 1: Write the failing test** + +Create `/project/dashboard-shell/tests/Feature/Unifi/ReconcilePortalTest.php`: + +```php + User::factory()->create(['is_active' => true])->id, + 'mac_address' => $mac, + 'is_active' => true, + 'authorized_at' => now()->subHour(), + 'expires_at' => now()->addDay(), + ], $overrides)); + } + + /** + * One fake for the whole class. The command calls four methods on the client + * and every test needs all four stubbed — doing that per test means adding a + * line to four closures every time the command learns a new call, and + * whichever one gets missed fails for a reason unrelated to what it tests. + * + * Returns the mock so a test can tighten an expectation on it. + */ + private function fakeUnifi(array $authorized = [], array $clients = []) + { + $mock = \Mockery::mock(UnifiApiClient::class); + $mock->shouldReceive('getAuthorizedGuestMacs')->andReturn($authorized); + $mock->shouldReceive('getActiveClients')->andReturn($clients); + $mock->shouldReceive('unauthorizeGuest')->andReturn([]); + $mock->shouldReceive('authorizeGuest')->andReturn([]); + $this->app->instance(UnifiApiClient::class, $mock); + + return $mock; + } + + public function test_it_deactivates_rows_past_their_expiry(): void + { + $stale = $this->session('02:00:00:00:00:01', ['expires_at' => now()->subMinute()]); + $this->fakeUnifi(); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + + $this->assertFalse((bool) $stale->fresh()->is_active); + } + + public function test_it_revokes_a_unifi_authorization_with_no_live_row(): void + { + $mock = $this->fakeUnifi(authorized: ['02:00:00:00:00:99']); + $mock->shouldReceive('unauthorizeGuest')->once()->with('02:00:00:00:00:99')->andReturn([]); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + } + + /** + * A controller upgrade or restore can lose an authorization we still consider + * live. Re-asserting it means the user is not bounced to the portal for + * something that was not their doing. + */ + public function test_it_reauthorizes_a_live_row_unifi_has_forgotten(): void + { + $this->session('02:00:00:00:00:07'); + $mock = $this->fakeUnifi(authorized: []); + $mock->shouldReceive('authorizeGuest')->once()->andReturn([]); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + } + + public function test_it_leaves_a_matching_live_row_alone(): void + { + $this->session('02:00:00:00:00:07'); + + $mock = \Mockery::mock(UnifiApiClient::class); + $mock->shouldReceive('getAuthorizedGuestMacs')->andReturn(['02:00:00:00:00:07']); + $mock->shouldReceive('getActiveClients')->andReturn([]); + $mock->shouldNotReceive('unauthorizeGuest'); + $mock->shouldNotReceive('authorizeGuest'); + $this->app->instance(UnifiApiClient::class, $mock); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + } + + /** + * Device type is REPORTED, not enforced. UniFi infers it from observed + * traffic, so it is not available at the moment of authorization — the + * reconciler is the first point at which there is anything to record. + */ + public function test_it_records_the_device_type_once_unifi_has_fingerprinted_it(): void + { + $session = $this->session('02:00:00:00:00:07'); + $this->fakeUnifi( + authorized: ['02:00:00:00:00:07'], + clients: [[ + 'mac' => '02:00:00:00:00:07', 'dev_cat' => 'Smartphone', + 'os_name' => 'iOS', 'hostname' => 'Alices-iPhone', + ]], + ); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + + $session->refresh(); + $this->assertSame('Smartphone', $session->device_type); + $this->assertSame('iOS', $session->device_os); + $this->assertSame('Alices-iPhone', $session->device_hostname); + } + + /** An unfingerprinted client must not blank what we already knew. */ + public function test_it_does_not_overwrite_a_known_type_with_nothing(): void + { + $session = $this->session('02:00:00:00:00:07', ['device_type' => 'Laptop']); + $this->fakeUnifi( + authorized: ['02:00:00:00:00:07'], + clients: [['mac' => '02:00:00:00:00:07']], // no dev_cat yet + ); + + $this->artisan('unifi:reconcile-portal')->assertExitCode(0); + + $this->assertSame('Laptop', $session->fresh()->device_type); + } +} +``` + +- [ ] **Step 2: Run it and confirm it fails** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ReconcilePortalTest.php +``` + +Expected: FAIL — `The command "unifi:reconcile-portal" does not exist.` + +- [ ] **Step 3: Add the authorized-guest lookup** + +In `UnifiApiClient.php`, after `unauthorizeGuest()`: + +```php + /** + * The MACs UniFi currently holds an active guest authorization for. + * + * Read from /stat/guest, which is the authorization list — deliberately not + * /stat/sta, which is who is associated right now. Those are the two facts + * this whole feature had been conflating. + */ + public function getAuthorizedGuestMacs(): array + { + $rows = $this->get('/stat/guest'); + + return collect($rows) + ->filter(fn ($g) => (int) ($g['end'] ?? 0) > now()->timestamp) + ->pluck('mac') + ->map(fn ($m) => strtolower((string) $m)) + ->values() + ->all(); + } +``` + +- [ ] **Step 4: Add the command** + +Create `/project/central-unifi/src/Console/ReconcilePortal.php`: + +```php +whereNotNull('expires_at') + ->where('expires_at', '<=', now()) + ->update(['is_active' => false]); + + if ($expired) { + $this->info("Expired {$expired} finished session(s)."); + } + + try { + $authorized = $unifi->getAuthorizedGuestMacs(); + } catch (\Throwable $e) { + // A controller we cannot reach is not evidence that anything should + // change. Expiry above is local and already done. + $this->error('Could not read UniFi authorizations: ' . $e->getMessage()); + + return self::SUCCESS; + } + + $live = PortalSession::live()->get()->keyBy(fn ($s) => strtolower($s->mac_address)); + + // 2. Orphans: authorized on the network with no live row of ours. These + // are what the old presence-based sweep produced — a device nobody + // can see in the UI and nobody's quota is paying for. + foreach (array_diff($authorized, $live->keys()->all()) as $orphan) { + $unifi->unauthorizeGuest($orphan); + $this->warn("Revoked orphan authorization {$orphan}."); + } + + // 3. The other direction: we consider it live but UniFi has forgotten it + // (upgrade, restore). Re-assert rather than making the person sign in + // again for something that was not their doing. + foreach ($live->keys()->diff($authorized) as $mac) { + $session = $live->get($mac); + $minutes = max(1, (int) now()->diffInMinutes($session->expires_at, false)); + $unifi->authorizeGuest($mac, $minutes); + $this->warn("Re-asserted authorization for {$mac} ({$minutes}m remaining)."); + } + + $this->captureFingerprints($unifi, $live); + + return self::SUCCESS; + } + + /** + * Record what UniFi thinks each device IS. Reported only — nothing acts on + * it, per the decision that enforcing on unvalidated fingerprint data would + * lock out the wrong people. + * + * This is the earliest point the data exists: UniFi infers dev_cat from + * observed traffic, so it is absent at the moment of authorization. + * + * @param \Illuminate\Support\Collection $live keyed by lower-case MAC + */ + private function captureFingerprints(UnifiApiClient $unifi, $live): void + { + try { + $clients = collect($unifi->getActiveClients()) + ->keyBy(fn ($c) => strtolower((string) ($c['mac'] ?? ''))); + } catch (\Throwable $e) { + $this->error('Could not read clients for fingerprints: ' . $e->getMessage()); + + return; + } + + foreach ($live as $mac => $session) { + $client = $clients->get($mac); + if (! $client) { + continue; + } + + // Only ever fill in. An unfingerprinted client must not blank a type + // we already recorded — that would lose the answer every time the + // device happened to be quiet. + $updates = array_filter([ + 'device_type' => $client['dev_cat'] ?? null, + 'device_os' => $client['os_name'] ?? null, + 'device_hostname' => $client['hostname'] ?? null, + ], fn ($v) => $v !== null && $v !== ''); + + if ($updates) { + $session->update($updates); + } + } + } +} +``` + +- [ ] **Step 5: Register it** + +In `/project/central-unifi/src/UnifiServiceProvider.php`, add `ReconcilePortal::class` to the existing `$this->commands([...])` array (alongside `CaptureSnapshots::class`). Import it at the top. + +- [ ] **Step 6: Run it and confirm it passes** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ReconcilePortalTest.php +``` + +Expected: PASS, 4 tests. + +- [ ] **Step 7: Run the whole Unifi suite and the full suite once** + +```bash +bin/agentdb test "$DB" tests/Feature/Unifi/ +bin/agentdb test "$DB" +``` + +Expected: Unifi all PASS. Full suite: only `Tests\Feature\Logs\ScriptRunsTest` fails — that is a known pre-existing failure, unrelated. Anything else is yours. + +- [ ] **Step 8: Bump the version and commit** + +```bash +# patch bump central-unifi/composer.json (1.13.4 -> 1.13.5) by hand, then: +git -C /project/central-unifi commit -m "Reconcile Wi-Fi sessions against UniFi authorizations" -- src/Console/ReconcilePortal.php src/Services/UnifiApiClient.php src/UnifiServiceProvider.php composer.json +git -C /project/dashboard-shell add tests/Feature/Unifi/ReconcilePortalTest.php +git -C /project/dashboard-shell commit -m "Reconcile Wi-Fi sessions against UniFi authorizations" -- tests/Feature/Unifi/ReconcilePortalTest.php +``` + +- [ ] **Step 9: Release the database slot** + +```bash +bin/agentdb release "$DB" +``` + +--- + +## Do not do these + +- **Do not schedule the reconciler yet.** It calls `unauthorizeGuest` against the live controller, and nothing here has been checked against real hardware. Scheduling belongs with the supervised verification session in the spec's section F, not in this plan. +- **Do not register the `unifi` app.** Every route here still 404s because there is no `dashboard_apps` row, and whether that should change is an open question for the operator, not a step in this plan. +- **Do not touch `disconnectSession`.** It already does `kickClient` + `unauthorizeGuest` and authorises owner-or-`unifi.auth` correctly. +- **Do not remove `email_domain`, `group` or `ou`.** Only `default` is a catch-all; the others are narrow matches and are still useful.