jwed
dbebb6c964
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.
1416 lines
53 KiB
Markdown
1416 lines
53 KiB
Markdown
# 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 <repo> commit -- <paths>`; 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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use App\Models\User;
|
|
use Dashboard\Unifi\Models\PortalSession;
|
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* A SLOT IS CONSUMED BY A LIVE AUTHORIZATION, NOT BY A PRESENT DEVICE.
|
|
*
|
|
* The portal used to free a slot when isClientConnected() was false, which
|
|
* conflates "connected right now" with "authorized". A phone that leaves the
|
|
* building is still authorized by UniFi for the rest of its window, so freeing
|
|
* its slot let the owner authorize a second device — two devices on a one-device
|
|
* quota, one of them an orphan nothing tracked.
|
|
*
|
|
* expires_at is nullable and, before this change, was written and never read.
|
|
* A NULL is therefore a legacy row from before anything enforced it: treated as
|
|
* NOT live, which is safe because unifi_portal_sessions holds no rows anyone
|
|
* depends on.
|
|
*/
|
|
class PortalSessionLiveScopeTest extends TestCase
|
|
{
|
|
use RefreshDatabase;
|
|
|
|
private function session(array $overrides = []): PortalSession
|
|
{
|
|
return PortalSession::create(array_merge([
|
|
'user_id' => 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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use App\Models\User;
|
|
use Dashboard\Unifi\Models\PortalSession;
|
|
use Dashboard\Unifi\Models\VlanMapping;
|
|
use Dashboard\Unifi\Services\UnifiApiClient;
|
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
|
use Illuminate\Support\Facades\DB;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* THE DEVICE THAT WENT HOME KEEPS ITS SLOT.
|
|
*
|
|
* isClientConnected() must not be consulted when counting the quota. If it is,
|
|
* a one-device student whose phone is out of range is handed a second
|
|
* authorization, and UniFi ends up holding two for them.
|
|
*/
|
|
class PortalQuotaCountsAuthorizationsTest extends TestCase
|
|
{
|
|
use RefreshDatabase;
|
|
|
|
private function oneDeviceMappingFor(User $user): void
|
|
{
|
|
$user->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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use App\Models\Group;
|
|
use App\Models\User;
|
|
use Dashboard\Unifi\Models\VlanMapping;
|
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
|
use Illuminate\Support\Facades\DB;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* NOBODY GETS WI-FI UNTIL A ROLE IS ADDED WITH AN AMOUNT.
|
|
*
|
|
* `default` matched everybody, which made the allow-list advisory. It is removed
|
|
* rather than merely discouraged. Safe to remove now and not later:
|
|
* unifi_vlan_mappings has zero rows in every environment, so nothing is revoked
|
|
* — the same change after a school had leaned on a catch-all would cut off every
|
|
* unmapped person at once.
|
|
*/
|
|
class WifiRoleAllowListTest extends TestCase
|
|
{
|
|
use RefreshDatabase;
|
|
|
|
private function super(): User
|
|
{
|
|
$u = User::factory()->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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use Dashboard\Unifi\Services\DeviceResolver;
|
|
use Dashboard\Unifi\Services\UnifiApiClient;
|
|
use Illuminate\Http\Request;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* THE CALLER DOES NOT GET TO NAME A MAC.
|
|
*
|
|
* Everything identifying the device used to be request-supplied, so any signed-in
|
|
* account could call /portal/wifi/callback?mac=<somebody else's MAC> 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
|
|
<?php
|
|
|
|
namespace Dashboard\Unifi\Support;
|
|
|
|
/**
|
|
* Pure MAC helpers. No dependencies, so they are cheap to test and safe to call
|
|
* from anywhere.
|
|
*/
|
|
class MacAddress
|
|
{
|
|
/** Lower-case, colon-separated, or null if it is not a MAC at all. */
|
|
public static function normalise(?string $mac): ?string
|
|
{
|
|
$mac = strtolower(str_replace('-', ':', trim((string) $mac)));
|
|
|
|
return preg_match('/^([0-9a-f]{2}:){5}[0-9a-f]{2}$/', $mac) ? $mac : null;
|
|
}
|
|
|
|
/**
|
|
* Is this a private (randomized) address rather than a real manufacturer one?
|
|
*
|
|
* The locally-administered bit is the second-least-significant bit of the
|
|
* first octet. iOS "Private Wi-Fi Address" and Android per-network
|
|
* randomization both set it. This is why OUI is useless for identifying
|
|
* device types: a randomized MAC has no real manufacturer prefix.
|
|
*/
|
|
public static function isRandomized(string $mac): bool
|
|
{
|
|
return (bool) (hexdec(substr($mac, 0, 2)) & 0b10);
|
|
}
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Add the client lookup**
|
|
|
|
In `/project/central-unifi/src/Services/UnifiApiClient.php`, after `getActiveClients()`:
|
|
|
|
```php
|
|
/**
|
|
* The client currently holding this IP, or null.
|
|
*
|
|
* Reads the cached client list rather than querying per IP: the portal calls
|
|
* this once per connection attempt, and a class arriving at once would
|
|
* otherwise be one controller round trip each.
|
|
*/
|
|
public function findClientByIp(string $ip): ?array
|
|
{
|
|
foreach ($this->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
|
|
<?php
|
|
|
|
namespace Dashboard\Unifi\Services;
|
|
|
|
use Dashboard\Unifi\Support\MacAddress;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Support\Facades\Log;
|
|
|
|
/** The outcome of trying to identify the device making a portal request. */
|
|
class ResolvedDevice
|
|
{
|
|
public function __construct(
|
|
public bool $ok,
|
|
public ?string $mac = null,
|
|
public ?string $reason = null,
|
|
public bool $isRandomized = false,
|
|
public bool $macParameterDisagreed = false,
|
|
public array $client = [],
|
|
) {}
|
|
}
|
|
|
|
/**
|
|
* Identify the device that made a captive-portal request, or refuse.
|
|
*
|
|
* The `?mac=` parameter is NOT trusted. It is read only so a disagreement with
|
|
* the derived value can be logged — that disagreement is somebody trying to
|
|
* authorize a device they do not own, and before this nothing anywhere noticed.
|
|
*
|
|
* Every path refuses rather than guessing. The cost of that is real: if
|
|
* TrustProxies is misconfigured, `proxy_untrusted` is returned for everyone and
|
|
* nobody gets online, which is why it is a distinct reason the portal page can
|
|
* report instead of a generic miss.
|
|
*/
|
|
class DeviceResolver
|
|
{
|
|
public function __construct(private UnifiApiClient $unifi) {}
|
|
|
|
/** Attempts, and the pause between them, for a device that has just associated. */
|
|
private const ATTEMPTS = 3;
|
|
private const PAUSE_US = 750_000;
|
|
|
|
public function resolve(Request $request, string $apMac): ResolvedDevice
|
|
{
|
|
$ip = $request->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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use Dashboard\Unifi\Support\MacAddress;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* Detecting a private address is what lets the denial page explain why a phone
|
|
* looks like a new device, instead of asking somebody to disconnect the very
|
|
* handset they are holding. Plan 2 writes that copy; this records the fact.
|
|
*/
|
|
class RandomizedMacIsRecordedTest extends TestCase
|
|
{
|
|
/** 0x02 has the locally-administered bit set. */
|
|
public function test_a_private_address_is_detected(): void
|
|
{
|
|
$this->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
|
|
<?php
|
|
|
|
use Illuminate\Database\Migrations\Migration;
|
|
use Illuminate\Database\Schema\Blueprint;
|
|
use Illuminate\Support\Facades\Schema;
|
|
|
|
/**
|
|
* Whether the device presented a private (randomized) address.
|
|
*
|
|
* Nullable, because rows written before this existed genuinely do not know —
|
|
* defaulting them to false would assert something untrue about every historical
|
|
* session.
|
|
*/
|
|
return new class extends Migration
|
|
{
|
|
public function up(): void
|
|
{
|
|
Schema::table('unifi_portal_sessions', function (Blueprint $table) {
|
|
$table->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
|
|
<?php
|
|
|
|
namespace Tests\Feature\Unifi;
|
|
|
|
use App\Models\User;
|
|
use Dashboard\Unifi\Models\PortalSession;
|
|
use Dashboard\Unifi\Services\UnifiApiClient;
|
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
|
use Tests\TestCase;
|
|
|
|
/**
|
|
* The reconciler is what keeps our rows and UniFi's authorizations from drifting
|
|
* once session windows are measured in months.
|
|
*
|
|
* It repairs BOTH directions, and the second one matters as much as the first: a
|
|
* UniFi authorization with no live row of ours is exactly the orphan the old
|
|
* presence-based sweep created, and it is a device on the network nobody can see.
|
|
*/
|
|
class ReconcilePortalTest extends TestCase
|
|
{
|
|
use RefreshDatabase;
|
|
|
|
private function session(string $mac, array $overrides = []): PortalSession
|
|
{
|
|
return PortalSession::create(array_merge([
|
|
'user_id' => 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
|
|
<?php
|
|
|
|
namespace Dashboard\Unifi\Console;
|
|
|
|
use Dashboard\Unifi\Models\PortalSession;
|
|
use Dashboard\Unifi\Services\UnifiApiClient;
|
|
use Illuminate\Console\Command;
|
|
|
|
/**
|
|
* Keep our rows and UniFi's authorizations in step.
|
|
*
|
|
* Necessary because session windows are now measured in months: any drift that
|
|
* used to be washed away by short sessions and frequent portal visits now
|
|
* persists. Three jobs, and the middle one is the one that matters most.
|
|
*/
|
|
class ReconcilePortal extends Command
|
|
{
|
|
protected $signature = 'unifi:reconcile-portal';
|
|
protected $description = 'Expire finished Wi-Fi sessions and reconcile them against UniFi authorizations';
|
|
|
|
public function handle(UnifiApiClient $unifi): int
|
|
{
|
|
// 1. Expiry. expires_at used to be written and never read, so a finished
|
|
// row kept consuming a slot indefinitely.
|
|
$expired = PortalSession::where('is_active', true)
|
|
->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.
|