diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..9b84633 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "editor.fontSize": 22, + "workbench.colorTheme": "Visual Studio Light", + "window.autoDetectColorScheme": true +} \ No newline at end of file diff --git a/README.md b/README.md index 0ddac45..a31c1ce 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A self-hosted, full-stack network management platform for MikroTik devices. Monitor, configure, and manage your entire MikroTik infrastructure — routers, switches, and wireless access points — from a single web interface. -![Version](https://img.shields.io/badge/version-0.23.0_Beta-blue) +![Version](https://img.shields.io/badge/version-0.23.1_Beta-blue) ![License](https://img.shields.io/badge/license-AGPLv3-blue) ![Docker](https://img.shields.io/badge/docker-compose-2496ED?logo=docker&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5.3-3178C6?logo=typescript&logoColor=white) diff --git a/backend/package.json b/backend/package.json index 84b08b0..86b18e7 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.23.0-beta", + "version": "0.23.1-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/db/migrate.ts b/backend/src/db/migrate.ts index 0607fa3..f211fb1 100644 --- a/backend/src/db/migrate.ts +++ b/backend/src/db/migrate.ts @@ -571,6 +571,10 @@ ALTER TABLE capsman_radios ADD COLUMN IF NOT EXISTS tx_power INTEGER; -- collected so no one is left looking at two kilobytes of text. UPDATE capsman_radios SET current_channel = NULL WHERE length(current_channel) > 64; +-- A CAP holds no local configuration, so its interface rows carry no SSID. The +-- controller does know it, on its own mirror of the CAP interface (#94). +ALTER TABLE capsman_radios ADD COLUMN IF NOT EXISTS ssid VARCHAR(128); + -- ntfy.sh notification channel (github issue #93). The channel type is a CHECK -- constraint rather than a lookup table, so widening it means replacing it. ALTER TABLE alert_channels DROP CONSTRAINT IF EXISTS alert_channels_type_check; diff --git a/backend/src/routes/wireless.ts b/backend/src/routes/wireless.ts index 912f47c..e26c5d8 100644 --- a/backend/src/routes/wireless.ts +++ b/backend/src/routes/wireless.ts @@ -156,7 +156,7 @@ router.get('/capsman', async (_req: Request, res: Response) => { query(` SELECT r.controller_device_id, r.radio_mac, r.interface_name, r.local, r.hw_type, r.current_channel, r.remote_cap_name, r.matched_device_id, - r.state, r.registered_peers, r.authorized_peers, r.tx_power, + r.state, r.registered_peers, r.authorized_peers, r.tx_power, r.ssid, d.name AS matched_device_name, d.status AS matched_device_status FROM capsman_radios r LEFT JOIN devices d ON d.id = r.matched_device_id diff --git a/backend/src/services/mikrotik/DeviceCollector.ts b/backend/src/services/mikrotik/DeviceCollector.ts index a09044e..b760461 100644 --- a/backend/src/services/mikrotik/DeviceCollector.ts +++ b/backend/src/services/mikrotik/DeviceCollector.ts @@ -9,8 +9,8 @@ import { buildServerArpMap } from '../../utils/serverArp'; import { aggregateBridgeVlans, portVlanMembership, expandVlanIds } from '../../utils/vlan'; import { classifyWifiRole, isCapsmanManaged, parseCapsmanStatus, parseCapStatus, - normalizeRadios, lookupDeviceForMac, macIndexKeys, parseRadioMonitor, resolveDatapath, - clientsPerRadio, + normalizeRadios, macIndexKeys, parseRadioMonitor, resolveDatapath, + clientsPerRadio, buildMacIndex, matchRadiosToDevices, type WifiRole, } from './capsman'; import { BackupService } from '../BackupService'; @@ -1472,12 +1472,7 @@ export class DeviceCollector { UNION SELECT device_id, mac_address FROM wireless_interfaces WHERE mac_address IS NOT NULL` ).catch(() => []); - const macIndex = new Map(); - for (const r of macRows) { - for (const key of macIndexKeys(r.mac_address)) { - if (!macIndex.has(key)) macIndex.set(key, r.device_id); - } - } + const macIndex = buildMacIndex(macRows); if (radioRows !== null) { const radios = normalizeRadios(radioRows); @@ -1497,30 +1492,39 @@ export class DeviceCollector { live.set(iface, parseRadioMonitor(mon[0])); })); + // The controller mirrors each CAP interface locally, which is also where the + // provisioned SSID is visible — the CAP's own row has none. + const ssidByRadioMac = new Map(); + for (const iface of ifaceRows) { + const mac = (iface['radio-mac'] || '').toUpperCase(); + const ssid = iface['configuration.ssid'] || iface['ssid']; + if (mac && ssid && !ssidByRadioMac.has(mac)) ssidByRadioMac.set(mac, ssid); + } + + const attributed = new Map( + matchRadiosToDevices(radios, macIndex, this.device.id) + .map((m) => [m.radio.radioMac ?? '', m.deviceId]) + ); + const seen: string[] = []; for (const radio of radios) { if (!radio.radioMac) continue; seen.push(radio.radioMac); const capInfo = parseCapStatus(radio.raw); const st = radio.interfaceName ? live.get(radio.interfaceName) : undefined; - // A remote radio belongs to a CAP, never to the controller — so a - // near-miss prefix match against the controller's own MAC block must not - // pull it back onto the controller. - const matched = radio.local - ? this.device.id - : lookupDeviceForMac(radio.radioMac, macIndex, this.device.id); + const matched = attributed.get(radio.radioMac) ?? null; await query( `INSERT INTO capsman_radios (controller_device_id, radio_mac, interface_name, local, hw_type, current_channel, remote_cap_name, matched_device_id, config_json, - state, registered_peers, authorized_peers, tx_power, updated_at) - VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,NOW()) + state, registered_peers, authorized_peers, tx_power, ssid, updated_at) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,NOW()) ON CONFLICT (controller_device_id, radio_mac) DO UPDATE SET interface_name=$3, local=$4, hw_type=$5, current_channel=$6, remote_cap_name=$7, matched_device_id=$8, config_json=$9, state=$10, registered_peers=$11, authorized_peers=$12, tx_power=$13, - updated_at=NOW()`, + ssid=$14, updated_at=NOW()`, [ this.device.id, radio.radioMac, radio.interfaceName, radio.local, radio.hwType, st?.channel ?? null, @@ -1529,6 +1533,7 @@ export class DeviceCollector { st?.state ?? null, clientCounts.get(radio.radioMac) ?? st?.registeredPeers ?? null, st?.authorizedPeers ?? null, st?.txPower ?? null, + ssidByRadioMac.get(radio.radioMac) ?? null, ] ); } diff --git a/backend/src/services/mikrotik/__tests__/capsman.test.ts b/backend/src/services/mikrotik/__tests__/capsman.test.ts index 4d5ecdd..4de662c 100644 --- a/backend/src/services/mikrotik/__tests__/capsman.test.ts +++ b/backend/src/services/mikrotik/__tests__/capsman.test.ts @@ -1,6 +1,6 @@ import { classifyWifiRole, parseCapsmanStatus, parseCapStatus, isCapsmanManaged, - normalizeRadios, matchRadiosToDevices, lookupDeviceForMac, macIndexKeys, + normalizeRadios, matchRadiosToDevices, lookupDeviceForMac, macIndexKeys, buildMacIndex, } from '../capsman'; describe('classifyWifiRole', () => { @@ -116,7 +116,7 @@ describe('normalizeRadios', () => { }); describe('matchRadiosToDevices', () => { - const index = new Map([['AA:BB:CC:00:00:01', 7]]); + const index = buildMacIndex([{ device_id: 7, mac_address: 'AA:BB:CC:00:00:01' }]); it('attributes a local radio to the controller itself', () => { const radios = normalizeRadios([{ 'radio-mac': 'FF:FF:FF:00:00:09', local: 'true' }]); @@ -136,7 +136,7 @@ describe('matchRadiosToDevices', () => { describe('lookupDeviceForMac', () => { it('matches exactly when the MAC is known', () => { - const idx = new Map([['AA:BB:CC:00:00:01', 5]]); + const idx = buildMacIndex([{ device_id: 5, mac_address: 'AA:BB:CC:00:00:01' }]); expect(lookupDeviceForMac('aa:bb:cc:00:00:01', idx)).toBe(5); }); @@ -145,17 +145,17 @@ describe('lookupDeviceForMac', () => { * octet, so an exact-only lookup would leave real CAPs unattributed. */ it('falls back to the five-octet prefix when the last octet differs', () => { - const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 5] as [string, number])); + const idx = buildMacIndex([{ device_id: 5, mac_address: 'AA:BB:CC:00:00:01' }]); expect(lookupDeviceForMac('AA:BB:CC:00:00:07', idx)).toBe(5); }); it('does not match a different device sharing only an OUI', () => { - const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 5] as [string, number])); + const idx = buildMacIndex([{ device_id: 5, mac_address: 'AA:BB:CC:00:00:01' }]); expect(lookupDeviceForMac('AA:BB:CC:99:99:99', idx)).toBeNull(); }); it('returns null for a missing MAC', () => { - expect(lookupDeviceForMac(null, new Map())).toBeNull(); + expect(lookupDeviceForMac(null, new Map())).toBeNull(); }); }); @@ -268,29 +268,83 @@ describe('clientsPerRadio', () => { }); }); -describe('lookupDeviceForMac — prefix must not steal a CAP for the controller (#94)', () => { +describe('lookupDeviceForMac — a remote radio never belongs to the controller (#94)', () => { it('refuses a prefix match against the excluded device', () => { - // Controller (id 3) owns AA:BB:CC:00:00:01; a CAP radio one octet away must not - // be attributed to it. - const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 3] as [string, number])); + const idx = buildMacIndex([{ device_id: 3, mac_address: 'AA:BB:CC:00:00:01' }]); expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx, 3)).toBeNull(); }); - it('still honours an exact match even against the excluded device', () => { - const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 3] as [string, number])); - expect(lookupDeviceForMac('AA:BB:CC:00:00:01', idx, 3)).toBe(3); + /** + * The case that survived the first fix. A controller mirrors each CAP interface + * locally, so the CAP's radio MAC is an *exact* match against the controller too. + * Excluding only prefix matches left half a fleet's radios on the controller. + */ + it('refuses an exact match against the excluded device', () => { + const idx = buildMacIndex([{ device_id: 12, mac_address: '04:F4:1C:A2:C4:66' }]); + expect(lookupDeviceForMac('04:F4:1C:A2:C4:66', idx, 12)).toBeNull(); + }); + + it('picks the real access point when both it and the controller carry the MAC', () => { + // Exactly the shape reported: the controller mirrors the CAP's interface, so + // both devices index the same radio MAC. + const idx = buildMacIndex([ + { device_id: 12, mac_address: '04:F4:1C:A2:C4:66' }, // controller's mirror + { device_id: 22, mac_address: '04:F4:1C:A2:C4:66' }, // the CAP itself + ]); + expect(lookupDeviceForMac('04:F4:1C:A2:C4:66', idx, 12)).toBe(22); + }); + + it('is order-independent — the CAP wins whichever row was indexed first', () => { + const idx = buildMacIndex([ + { device_id: 22, mac_address: '04:F4:1C:A2:C4:66' }, + { device_id: 12, mac_address: '04:F4:1C:A2:C4:66' }, + ]); + expect(lookupDeviceForMac('04:F4:1C:A2:C4:66', idx, 12)).toBe(22); }); it('still prefix-matches a different device', () => { - const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 7] as [string, number])); + const idx = buildMacIndex([{ device_id: 7, mac_address: 'AA:BB:CC:00:00:01' }]); expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx, 3)).toBe(7); }); it('prefers an exact match over a prefix match', () => { - const idx = new Map([ - ['AA:BB:CC:00:00', 7], // prefix → device 7 - ['AA:BB:CC:00:00:09', 9], // exact → device 9 + const idx = buildMacIndex([ + { device_id: 7, mac_address: 'AA:BB:CC:00:00:01' }, + { device_id: 9, mac_address: 'AA:BB:CC:00:00:09' }, ]); expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx)).toBe(9); }); }); + +/** + * The reporter's fleet: a router acting purely as controller, mirroring six CAP + * interfaces. Before the fix these resolved to the controller or the CAP depending + * on database row order, so the same AP appeared in both places between polls. + */ +describe('matchRadiosToDevices — controller mirrors (#94 field report)', () => { + const CONTROLLER = 12; + const index = buildMacIndex([ + { device_id: CONTROLLER, mac_address: '04:F4:1C:A2:C4:65' }, // mirrors + { device_id: CONTROLLER, mac_address: '04:F4:1C:A2:C4:66' }, + { device_id: 22, mac_address: '04:F4:1C:A2:C4:65' }, // bathroom AP + { device_id: 22, mac_address: '04:F4:1C:A2:C4:66' }, + ]); + + it('puts both radios of one AP on that AP, not the controller', () => { + const radios = normalizeRadios([ + { 'radio-mac': '04:F4:1C:A2:C4:65', interface: 'wifi-legacy-bathroom-ap-001-main-5g', local: 'false' }, + { 'radio-mac': '04:F4:1C:A2:C4:66', interface: 'wifi-legacy-bathroom-ap-001-main-2g', local: 'false' }, + ]); + expect(matchRadiosToDevices(radios, index, CONTROLLER).map((m) => m.deviceId)).toEqual([22, 22]); + }); + + it('still keeps a genuinely local radio on the controller', () => { + const radios = normalizeRadios([{ 'radio-mac': 'FF:FF:FF:00:00:01', local: 'true' }]); + expect(matchRadiosToDevices(radios, index, CONTROLLER)[0].deviceId).toBe(CONTROLLER); + }); + + it('leaves a CAP that is not in the fleet unattributed rather than on the controller', () => { + const radios = normalizeRadios([{ 'radio-mac': '04:F4:1C:99:99:99', local: 'false' }]); + expect(matchRadiosToDevices(radios, index, CONTROLLER)[0].deviceId).toBeNull(); + }); +}); diff --git a/backend/src/services/mikrotik/capsman.ts b/backend/src/services/mikrotik/capsman.ts index 0aa1fd5..246b68d 100644 --- a/backend/src/services/mikrotik/capsman.ts +++ b/backend/src/services/mikrotik/capsman.ts @@ -187,15 +187,46 @@ export function normalizeRadios(rows: Record[]): RadioRow[] { * on hardware identity alone: a MAC is globally unique, so unlike an address it * cannot mean two different devices on two different segments. */ +/** + * Index every known interface MAC to the device(s) carrying it. + * + * Multi-valued deliberately. A CAPsMAN controller mirrors each CAP's interfaces + * locally, so a CAP's radio MAC legitimately appears under both the CAP and the + * controller. A single-valued first-wins index picked whichever row the database + * returned first, which attributed half a fleet's radios to the controller and + * shuffled between polls (#94). + */ +export function buildMacIndex( + rows: { device_id: number; mac_address: string | null }[] +): Map { + const index = new Map(); + for (const row of rows) { + if (!row.mac_address) continue; + for (const key of macIndexKeys(row.mac_address)) { + const list = index.get(key) ?? []; + if (!list.includes(row.device_id)) list.push(row.device_id); + index.set(key, list); + } + } + return index; +} + +/** + * Attribute each radio a controller reports to a managed device, by MAC. + * + * A radio the controller flags `local: false` physically lives on a CAP, so the + * controller is never the answer — even when its mirror of that interface makes the + * MAC look like its own. Matching is on hardware identity: a MAC is globally unique, + * so unlike an address it cannot mean two devices on two segments. + */ export function matchRadiosToDevices( radios: RadioRow[], - macToDevice: Map, + macToDevice: Map, controllerDeviceId: number ): { radio: RadioRow; deviceId: number | null }[] { return radios.map((radio) => { if (radio.local) return { radio, deviceId: controllerDeviceId }; - const hit = radio.radioMac ? macToDevice.get(radio.radioMac.toUpperCase()) : undefined; - return { radio, deviceId: hit ?? null }; + return { radio, deviceId: lookupDeviceForMac(radio.radioMac, macToDevice, controllerDeviceId) }; }); } @@ -215,28 +246,22 @@ export function macIndexKeys(mac: string): string[] { /** * Look up a device for a radio MAC, falling back to the five-octet prefix. * - * `excludeOnPrefix` refuses a *prefix* match against a given device — used to stop a - * remote CAP being attributed to the controller when the two happen to share the - * first five octets. An exact match is still honoured, since that is hardware - * identity rather than a near-miss. Reported on #94, where a CAP's radio was grouped - * under the controller instead of its access point. + * `excludeDeviceId` is rejected at both exact and prefix strength. That is not a + * tie-break heuristic: the caller passes the controller for a radio the controller + * itself reported as remote, so the controller is known to be the wrong answer, and + * an exact match against it only means the controller mirrors that CAP's interface. */ export function lookupDeviceForMac( mac: string | null, - index: Map, - excludeOnPrefix?: number + index: Map, + excludeDeviceId?: number ): number | null { if (!mac) return null; - const keys = macIndexKeys(mac); - - const exact = index.get(keys[0]); - if (exact !== undefined) return exact; - - for (const key of keys.slice(1)) { - const hit = index.get(key); - if (hit === undefined) continue; - if (excludeOnPrefix !== undefined && hit === excludeOnPrefix) continue; - return hit; + for (const key of macIndexKeys(mac)) { + const candidates = index.get(key); + if (!candidates) continue; + const hit = candidates.find((id) => id !== excludeDeviceId); + if (hit !== undefined) return hit; } return null; } diff --git a/frontend/package.json b/frontend/package.json index 9a276e6..9cdb4bd 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.23.0-beta", + "version": "0.23.1-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/wireless/CapsmanPanel.tsx b/frontend/src/components/wireless/CapsmanPanel.tsx index 1a66000..ea17b81 100644 --- a/frontend/src/components/wireless/CapsmanPanel.tsx +++ b/frontend/src/components/wireless/CapsmanPanel.tsx @@ -2,7 +2,7 @@ import { useQuery } from '@tanstack/react-query'; import { Link } from 'react-router-dom'; import { Server, AlertTriangle, ChevronDown, ChevronRight } from 'lucide-react'; import { useState } from 'react'; -import { wirelessApi, type CapsmanController, type CapsmanRadio } from '../../services/api'; +import { wirelessApi, type CapsmanController } from '../../services/api'; import clsx from 'clsx'; /** One row of the unified radio/interface table. */ @@ -13,55 +13,33 @@ interface Row { local: boolean; interfaceName: string; ssid: string | null; - band: string | null; - frequency: number | null; channel: string | null; clients: number | null; - running: boolean | null; - disabled: boolean | null; + running: boolean; } /** - * Flatten a controller into one row per interface. + * Flatten a controller into one row per radio. * - * Radios and provisioned interfaces were two separate tables describing the same - * things from different angles, which left the reader joining them by eye. Each - * interface is emitted against the access point it lives on; a radio with no - * provisioned interface still appears, so a CAP that is up but carrying nothing is - * visible rather than absent. + * Driven by the controller's radio list rather than by the CAPs' own interface rows. + * A CAP holds no configuration — its rows report a generic name and null SSID, band + * and frequency — whereas the controller knows the provisioned SSID, the operating + * channel and the client count for every radio it manages. */ function toRows(c: CapsmanController): Row[] { const rows: Row[] = []; - const radioOf = new Map(); - for (const ap of c.access_points) for (const r of ap.radios) radioOf.set(r.radio_mac, r); - for (const ap of c.access_points) { - const ifaces = c.ssids.filter((s) => s.device_id === ap.device_id); - if (ifaces.length > 0) { - for (const s of ifaces) { - const radio = s.radio_mac ? radioOf.get(s.radio_mac) : undefined; - rows.push({ - key: `${ap.device_id ?? ap.device_name}:${s.name}`, - deviceId: ap.device_id, deviceName: ap.device_name, - local: ap.radios[0]?.local ?? false, - interfaceName: s.name, ssid: s.ssid, - band: s.band, frequency: s.frequency, - channel: radio?.current_channel ?? null, - clients: radio?.registered_peers ?? null, - running: s.running, disabled: s.disabled, - }); - } - continue; - } - // No provisioned interface reported — still show the radio itself. for (const r of ap.radios) { rows.push({ key: `${ap.device_id ?? ap.device_name}:${r.radio_mac}`, - deviceId: ap.device_id, deviceName: ap.device_name, local: r.local, + deviceId: ap.device_id, + deviceName: ap.device_name, + local: r.local, interfaceName: r.interface_name || r.radio_mac, - ssid: null, band: null, frequency: null, - channel: r.current_channel, clients: r.registered_peers, - running: r.state === 'running', disabled: null, + ssid: r.ssid, + channel: r.current_channel, + clients: r.registered_peers, + running: r.state === 'running', }); } } @@ -99,7 +77,7 @@ function Controller({ c }: { c: CapsmanController }) {
{c.access_points.length} access point{c.access_points.length !== 1 ? 's' : ''} ·{' '} - {rows.length} interface{rows.length !== 1 ? 's' : ''} ·{' '} + {rows.length} radio{rows.length !== 1 ? 's' : ''} ·{' '} {c.client_count} client{c.client_count !== 1 ? 's' : ''}
@@ -122,7 +100,7 @@ function Controller({ c }: { c: CapsmanController }) { - {['Access point', 'Interface', 'SSID', 'Band', 'Frequency', 'Channel', 'Clients', 'Status'].map((h) => ( + {['Access point', 'Interface', 'SSID', 'Channel', 'Clients', 'Status'].map((h) => ( ))} @@ -146,8 +124,6 @@ function Controller({ c }: { c: CapsmanController }) { - - diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 04f428b..a0b8fb3 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -903,6 +903,8 @@ export interface CapsmanRadio { /** Operating channel from /interface/wifi/monitor, not the supported-channel list. */ current_channel: string | null; state: string | null; + /** SSID the controller provisioned onto this radio; the CAP itself does not know it. */ + ssid: string | null; registered_peers: number | null; authorized_peers: number | null; tx_power: number | null; diff --git a/frontend/src/version.ts b/frontend/src/version.ts index 4bc2b3a..febff45 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.23.0 Beta'; +export const APP_VERSION = 'v0.23.1 Beta';
{h}
{r.interfaceName} {r.ssid || —}{r.band || —}{r.frequency ?? —} {r.channel || —} {r.clients ?? —} @@ -155,11 +131,10 @@ function Controller({ c }: { c: CapsmanController }) { - {r.disabled ? 'disabled' : r.running ? 'running' : 'enabled'} + {r.running ? 'running' : 'idle'}