From 82efcaa3718b1603d1ceb5345b4da2436f9435a8 Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:48:29 -0700 Subject: [PATCH] fix(wireless,topology): CAPsMAN write guards, MAC neighbour resolution, unified panel (v0.23.0-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #95. Addresses the second round of #94 feedback. Includes a regression introduced in v0.22.0. Wireless write paths were not CAPsMAN-aware (regression) v0.22.0 widened getAP() and the bulk-SSID selector to include cap/controller roles so a controller — normally tagged a router — would stop being invisible to the wireless section. Those same helpers gate the write endpoints, so the change also allowed writes to devices whose radios a controller owns. Reads needed widening; writes needed a guard, and did not get one. Five paths now refuse or skip provisioned radios: bulk SSID deployment, single interface create, interface edit, interface delete, and guest network setup. Delete was the worst of them — RouterOS will not remove a provisioned interface, but the cached row was deleted anyway, so it vanished from the UI until the next poll brought it back. The check is shared: partitionRadios() splits physical radios into locally controllable and controller-owned, assertNotCapsmanManaged() refuses an edit to a provisioned interface and names the controller. Neighbours identified only by MAC (#95) Topology resolved a neighbour to a managed device by IP address. A trunk port has no IP, so LLDP across one reports a MAC and an identity but no address — and those links stayed unresolved, drawing an external node beside the very device it already had on the map. Two directly connected devices appeared four times. Resolution by MAC now runs first, before address matching, because a MAC is hardware identity and unique fleet-wide where an address is not. This is the same join the CAPsMAN radio matching uses. CAPsMAN panel, from the reporter's testing - Radios and provisioned interfaces were two tables describing the same things from different angles; they are now one, per interface, with the access point, SSID, band, frequency, channel, clients and status. - Provisioning rules sit inside the configuration they apply. Rules referencing a configuration we did not collect are listed separately rather than dropped, since that usually means a stale reference. - The Wireless header reported zero clients while the Clients page reported 32: it summed the CAP-local count, which is empty under central forwarding. It now takes the greater of that and the controller's per-radio counts, so a controller that is also a CAP is not double-counted. - A remote radio could be attributed to the controller by the five-octet prefix fallback when the two shared a MAC block. Prefix matches now refuse the controller for a non-local radio; exact matches are still honoured. Radios tab followed device_type device_type is user-supplied tagging, and a router with radios had no way to display them. The tab now follows the detected wifi_role, which is a fact about the hardware. 48 tests on the CAPsMAN module, 22 on topology, including a fixture built from the device output in #95. Verified against the fleet: topology output unchanged, so MAC resolution merged nothing it should not have; guards correctly permissive on a standalone AP. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- backend/package.json | 2 +- backend/src/routes/devices.ts | 6 +- backend/src/routes/topology.ts | 11 +- backend/src/routes/wireless.ts | 51 +++- .../src/services/mikrotik/DeviceCollector.ts | 84 +++++-- .../mikrotik/__tests__/capsman.test.ts | 27 +++ backend/src/services/mikrotik/capsman.ts | 24 +- .../topology/__tests__/buildTopology.test.ts | 68 ++++++ .../src/services/topology/buildTopology.ts | 30 ++- frontend/package.json | 2 +- .../src/components/wireless/CapsmanPanel.tsx | 226 ++++++++++-------- frontend/src/pages/DeviceDetailPage.tsx | 6 +- frontend/src/services/api.ts | 4 + frontend/src/types/index.ts | 2 + frontend/src/version.ts | 2 +- 16 files changed, 415 insertions(+), 132 deletions(-) diff --git a/README.md b/README.md index 4737062..0ddac45 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.22.1_Beta-blue) +![Version](https://img.shields.io/badge/version-0.23.0_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 f9d741d..84b08b0 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.22.1-beta", + "version": "0.23.0-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/routes/devices.ts b/backend/src/routes/devices.ts index 9dbb777..c858044 100644 --- a/backend/src/routes/devices.ts +++ b/backend/src/routes/devices.ts @@ -302,7 +302,7 @@ router.get('/:id', async (req: Request, res: Response) => { notes, location_address, location_lat::float8 AS location_lat, location_lng::float8 AS location_lng, - rack_name, rack_slot, created_at, updated_at + rack_name, rack_slot, wifi_role, created_at, updated_at FROM devices WHERE id = $1`, [req.params.id] ); @@ -339,11 +339,11 @@ router.patch('/:id/location', requireWrite, async (req: Request, res: Response) const updated = await queryOne( `SELECT id, name, ip_address, api_port, api_username, ssh_port, ssh_username, model, - serial_number, firmware_version, ros_version, device_type, status, last_seen, + serial_number, firmware_version, ros_version, device_type, status, last_seen, wifi_role, notes, location_address, location_lat::float8 AS location_lat, location_lng::float8 AS location_lng, - rack_name, rack_slot, created_at, updated_at + rack_name, rack_slot, wifi_role, created_at, updated_at FROM devices WHERE id = $1`, [req.params.id] ); diff --git a/backend/src/routes/topology.ts b/backend/src/routes/topology.ts index 4287692..871027f 100644 --- a/backend/src/routes/topology.ts +++ b/backend/src/routes/topology.ts @@ -20,7 +20,7 @@ export function setPollerService(p: PollerService): void { // GET /api/topology router.get('/', async (_req: Request, res: Response) => { - const [devices, allLinks, manualLinks] = await Promise.all([ + const [devices, allLinks, manualLinks, deviceMacs] = await Promise.all([ query( `SELECT id, name, ip_address, model, device_type, status, ros_version, ip_addresses_jsonb FROM devices ORDER BY name ASC` @@ -40,9 +40,16 @@ router.get('/', async (_req: Request, res: Response) => { JOIN devices fd ON fd.id = ml.from_device_id JOIN devices td ON td.id = ml.to_device_id` ), + // Every interface MAC in the fleet, so a neighbour seen only by MAC — an LLDP + // sighting across a trunk port carries no address — still resolves to its device. + query<{ device_id: number; mac_address: string }>( + `SELECT device_id, mac_address FROM interfaces WHERE mac_address IS NOT NULL + UNION + SELECT device_id, mac_address FROM wireless_interfaces WHERE mac_address IS NOT NULL` + ), ]); - const graph = buildTopology(devices, allLinks, manualLinks); + const graph = buildTopology(devices, allLinks, manualLinks, deviceMacs); res.json({ devices, diff --git a/backend/src/routes/wireless.ts b/backend/src/routes/wireless.ts index 5d55eb4..912f47c 100644 --- a/backend/src/routes/wireless.ts +++ b/backend/src/routes/wireless.ts @@ -42,6 +42,18 @@ router.post('/ssid/bulk', requireWrite, async (req: Request, res: Response) => { try { await collector.connect(); const pkg = await collector.detectWifiPackage(); + + // Widening the device selector so CAPsMAN controllers stop being invisible + // also let CAPs through here. An SSID created on a provisioned radio is + // discarded by the controller, so refuse rather than report success. + const { local: localRadios, capsmanOwned } = await collector.partitionRadios(); + if (localRadios.length === 0 && capsmanOwned.length > 0) { + throw new Error( + `${ap.name}: every radio is provisioned by CAPsMAN (${capsmanOwned.join(', ')}). ` + + `Add this SSID to the controller's configuration instead.` + ); + } + const autoName = await collector.getNextInterfaceName(); // Normalize mode to the correct value for this AP's package type @@ -98,7 +110,15 @@ router.get('/', async (_req: Request, res: Response) => { SELECT d.id, d.name, d.ip_address, d.model, d.device_type, d.status, d.last_seen, d.ros_version, d.firmware_version, d.serial_number, d.rack_name, d.rack_slot, COUNT(DISTINCT wi.id) AS radio_count, - COALESCE(SUM(wi.registered_clients), 0) AS client_count, + -- A CAP's own registration table is empty when the controller processes + -- traffic centrally, so its per-interface counts read zero. GREATEST + -- prefers whichever source actually has the clients without adding the + -- two together on a controller that is also a CAP. + GREATEST( + COALESCE(SUM(wi.registered_clients), 0), + COALESCE((SELECT SUM(cr.registered_peers) FROM capsman_radios cr + WHERE cr.matched_device_id = d.id), 0) + ) AS client_count, COUNT(DISTINCT wi.id) FILTER (WHERE wi.ssid IS NOT NULL AND wi.disabled = false) AS ssid_count FROM devices d LEFT JOIN wireless_interfaces wi ON wi.device_id = d.id @@ -158,8 +178,8 @@ router.get('/capsman', async (_req: Request, res: Response) => { `, [ids]), ]); - // SSIDs actually broadcasting, so the view shows what is live rather than only - // what was configured. + // Interfaces provisioned onto the CAPs, joined to their radio so the UI can show + // one row per interface with the access point it lives on. const interfaces = await query(` SELECT wi.device_id, wi.name, wi.ssid, wi.band, wi.frequency, wi.disabled, wi.running, wi.managed_by_capsman, wi.radio_mac, wi.registered_clients, d.name AS device_name @@ -208,8 +228,17 @@ router.get('/capsman', async (_req: Request, res: Response) => { ...g, client_count: peers(g.radios as { registered_peers?: number | null }[]), })), - configurations: byController(configs as { controller_device_id: number }[], c.id), - provisioning: byController(provisioning as { controller_device_id: number }[], c.id), + configurations: byController(configs as { controller_device_id: number; name: string }[], c.id) + .map((cfg) => ({ + ...cfg, + // The rule and the configuration it applies are one idea; showing them in + // separate boxes made the reader join them by eye (#94). + rules: byController(provisioning as { controller_device_id: number; master_configuration: string | null }[], c.id) + .filter((p) => p.master_configuration === cfg.name), + })), + provisioning: byController(provisioning as { controller_device_id: number; master_configuration: string | null }[], c.id) + .filter((p) => !configs.some((cfg) => (cfg as { controller_device_id: number; name: string }).controller_device_id === c.id + && (cfg as { name: string }).name === p.master_configuration)), ssids, }; }), @@ -278,6 +307,16 @@ router.post('/:id/interfaces', requireWrite, async (req: Request, res: Response) try { await collector.connect(); + // A virtual AP created on a CAPsMAN-provisioned radio is discarded by the + // controller at the next provision, so refuse instead of reporting success. + const { local: localRadios, capsmanOwned } = await collector.partitionRadios(); + if (localRadios.length === 0 && capsmanOwned.length > 0) { + return res.status(409).json({ + error: `Every radio on ${ap.name} is provisioned by CAPsMAN (${capsmanOwned.join(', ')}). ` + + `Add this SSID to the controller's configuration so it is provisioned to the access points.`, + }); + } + // Auto-generate the next available interface name (e.g. wifi3, wifi4 …) const autoName = await collector.getNextInterfaceName(); @@ -358,6 +397,7 @@ router.put('/:id/interfaces/:name', requireWrite, async (req: Request, res: Resp const collector = new DeviceCollector(ap); try { await collector.connect(); + await collector.assertNotCapsmanManaged(ifaceName); await collector.setWirelessInterface(ifaceName, params); // Bridge port membership — only touched when 'bridge' key is present in the request if (body.bridge !== undefined) { @@ -386,6 +426,7 @@ router.delete('/:id/interfaces/:name', requireWrite, async (req: Request, res: R const collector = new DeviceCollector(ap); try { await collector.connect(); + await collector.assertNotCapsmanManaged(req.params.name); await collector.removeWirelessInterface(req.params.name); await query( `DELETE FROM wireless_interfaces WHERE device_id=$1 AND name=$2`, diff --git a/backend/src/services/mikrotik/DeviceCollector.ts b/backend/src/services/mikrotik/DeviceCollector.ts index 788ea9e..a09044e 100644 --- a/backend/src/services/mikrotik/DeviceCollector.ts +++ b/backend/src/services/mikrotik/DeviceCollector.ts @@ -1344,6 +1344,63 @@ export class DeviceCollector { } // ─── CAPsMAN ────────────────────────────────────────────────────────────── + /** + * Split this device's physical radios into ones we may configure locally and ones + * a CAPsMAN controller owns. + * + * Writing to a provisioned radio does not reliably take effect: the change is + * either rejected or silently replaced the next time the controller provisions the + * CAP, while the API reports success. Callers use this to skip those radios and + * say so rather than claim a change that will not survive. + */ + async partitionRadios(): Promise<{ local: Record[]; capsmanOwned: string[] }> { + const pkg = await this.detectWifiPackage(); + if (pkg === 'none') return { local: [], capsmanOwned: [] }; + + const raw = pkg === 'wifi' + ? await this.client.execute('/interface/wifi/print', { detail: '' }).catch(() => [] as Record[]) + : await this.client.execute('/interface/wireless/print', { detail: '' }).catch(() => [] as Record[]); + + const physicals = raw.filter((r) => !r['master-interface'] && r['name']); + if (pkg !== 'wifi') return { local: physicals, capsmanOwned: [] }; + + const role = await this.detectWifiRole(); + const owned = physicals.filter((r) => isCapsmanManaged(r, role)); + return { + local: physicals.filter((r) => !isCapsmanManaged(r, role)), + capsmanOwned: owned.map((r) => r['name']).filter(Boolean), + }; + } + + /** + * Refuse a local edit to an interface a controller owns. + * + * RouterOS will not let a provisioned interface be changed or removed locally, and + * where it appears to accept one the controller undoes it. Failing here with an + * explanation beats reporting success for a change that never happened — which is + * what the delete path did, removing the cached row while the interface stayed. + */ + async assertNotCapsmanManaged(interfaceName: string): Promise { + const pkg = await this.detectWifiPackage(); + if (pkg !== 'wifi') return; + const role = await this.detectWifiRole(); + if (role !== 'cap' && role !== 'controller_cap') return; + + const rows = await this.client.execute('/interface/wifi/print', { detail: '' }) + .catch(() => [] as Record[]); + const row = rows.find((r) => r['name'] === interfaceName); + if (!row || !isCapsmanManaged(row, role)) return; + + const status = parseCapsmanStatus(row); + throw new Error( + `${interfaceName} is provisioned by CAPsMAN` + + (status?.controllerMac ? ` from controller ${status.controllerMac}` : '') + + `, so it cannot be changed on this device. Edit it in the controller's configuration ` + + `instead — a local change would be discarded the next time the CAP is provisioned.` + ); + } + + private wifiRoleCache: WifiRole | null = null; @@ -1446,9 +1503,12 @@ export class DeviceCollector { 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); + : lookupDeviceForMac(radio.radioMac, macIndex, this.device.id); await query( `INSERT INTO capsman_radios @@ -2429,21 +2489,15 @@ export class DeviceCollector { const pkg = await this.detectWifiPackage(); if (pkg === 'none') throw new Error('No wireless package detected on this device — cannot create a guest SSID'); const raw = pkg === 'wifi' - ? await this.client.execute('/interface/wifi/print').catch(() => [] as Record[]) - : await this.client.execute('/interface/wireless/print').catch(() => [] as Record[]); - let physicals = raw.filter(r => !r['master-interface'] && r['name']); - if (physicals.length === 0) throw new Error('No physical radios found to attach the guest SSID to'); + ? await this.client.execute('/interface/wifi/print', { detail: '' }).catch(() => [] as Record[]) + : await this.client.execute('/interface/wireless/print', { detail: '' }).catch(() => [] as Record[]); - // A CAPsMAN-provisioned radio is owned by the controller. Adding a local - // virtual AP to it does not reliably take effect and can be wiped or - // contradicted the next time the controller provisions the CAP — so skip - // those radios and say so, rather than reporting success for something that - // silently will not work. Raised by the reporter of issue #94. - const role = await this.detectWifiRole(); - const managedRadios = physicals.filter(r => isCapsmanManaged(r, role)); - if (managedRadios.length > 0) { - physicals = physicals.filter(r => !isCapsmanManaged(r, role)); - const names = managedRadios.map(r => r['name']).join(', '); + const { local: physicals, capsmanOwned } = await this.partitionRadios(); + if (physicals.length === 0 && capsmanOwned.length === 0) { + throw new Error('No physical radios found to attach the guest SSID to'); + } + if (capsmanOwned.length > 0) { + const names = capsmanOwned.join(', '); if (physicals.length === 0) { throw new Error( `Every radio on this device (${names}) is provisioned by CAPsMAN, so a guest SSID ` diff --git a/backend/src/services/mikrotik/__tests__/capsman.test.ts b/backend/src/services/mikrotik/__tests__/capsman.test.ts index ece3f81..4d5ecdd 100644 --- a/backend/src/services/mikrotik/__tests__/capsman.test.ts +++ b/backend/src/services/mikrotik/__tests__/capsman.test.ts @@ -267,3 +267,30 @@ describe('clientsPerRadio', () => { expect(clientsPerRadio(interfaces, []).size).toBe(0); }); }); + +describe('lookupDeviceForMac — prefix must not steal a CAP for 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])); + 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); + }); + + 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])); + 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 + ]); + expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx)).toBe(9); + }); +}); diff --git a/backend/src/services/mikrotik/capsman.ts b/backend/src/services/mikrotik/capsman.ts index a8fe370..0aa1fd5 100644 --- a/backend/src/services/mikrotik/capsman.ts +++ b/backend/src/services/mikrotik/capsman.ts @@ -212,15 +212,31 @@ export function macIndexKeys(mac: string): string[] { return [norm, parts.slice(0, 5).join(':')]; } -/** Look up a device for a radio MAC, falling back to the five-octet prefix. */ +/** + * 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. + */ export function lookupDeviceForMac( mac: string | null, - index: Map + index: Map, + excludeOnPrefix?: number ): number | null { if (!mac) return null; - for (const key of macIndexKeys(mac)) { + 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) return hit; + if (hit === undefined) continue; + if (excludeOnPrefix !== undefined && hit === excludeOnPrefix) continue; + return hit; } return null; } diff --git a/backend/src/services/topology/__tests__/buildTopology.test.ts b/backend/src/services/topology/__tests__/buildTopology.test.ts index b73cc50..af72a79 100644 --- a/backend/src/services/topology/__tests__/buildTopology.test.ts +++ b/backend/src/services/topology/__tests__/buildTopology.test.ts @@ -209,3 +209,71 @@ describe('buildTopology — behaviour preserved for unambiguous fleets', () => { expect(graph.links[0].to_device_id).toBeNull(); }); }); + +/** + * Issue #95. Two managed devices connected by a trunk port. LLDP across a trunk + * carries no IP, so address matching has nothing to work with — but it does carry + * the peer's port MAC. Without MAC resolution both devices were drawn twice: once + * as themselves and once as an external node. + */ +describe('buildTopology — neighbours identified only by MAC (#95)', () => { + const devices = [ + dev(12, 'legacy-core-rt-001', '10.99.0.1'), + dev(13, 'legacy-core-sw-001', '10.99.0.2'), + ]; + // Trunk-port MACs, which differ from the management-VLAN MACs the devices answer on. + const macs = [ + { device_id: 12, mac_address: '04:F4:1C:9F:3D:A9' }, + { device_id: 13, mac_address: '04:F4:1C:36:B5:E9' }, + ]; + const trunkLinks = () => [ + link({ + from_device_id: 12, from_interface: 'trunk1', link_type: 'lldp', + neighbor_mac: '04:F4:1C:36:B5:E9', neighbor_identity: 'legacy-core-sw-001', + neighbor_address: null, from_device_name: 'legacy-core-rt-001', + }), + link({ + from_device_id: 13, from_interface: 'sfp-sfpplus2', link_type: 'lldp', + neighbor_mac: '04:F4:1C:9F:3D:A9', neighbor_identity: 'legacy-core-rt-001', + neighbor_address: null, from_device_name: 'legacy-core-sw-001', + }), + ]; + + it('resolves both directions to the managed devices', () => { + const graph = buildTopology(devices, trunkLinks(), [], macs); + for (const l of graph.links) expect(l.to_device_id).not.toBeNull(); + }); + + it('draws no external node duplicating a managed device', () => { + const graph = buildTopology(devices, trunkLinks(), [], macs); + expect(graph.externalNodes.filter((n) => n.caps !== 'segment')).toHaveLength(0); + }); + + it('collapses the bidirectional pair into one link', () => { + const graph = buildTopology(devices, trunkLinks(), [], macs); + expect(graph.links).toHaveLength(1); + expect(graph.links[0].from_device_id).toBe(12); + expect(graph.links[0].to_device_id).toBe(13); + }); + + it('still produces external nodes for genuinely unmanaged neighbours', () => { + const graph = buildTopology(devices, [ + link({ from_device_id: 12, from_interface: 'ether5', link_type: 'lldp', + neighbor_mac: 'FF:FF:FF:00:00:01', neighbor_identity: 'some-switch' }), + ], [], macs); + expect(graph.externalNodes.filter((n) => n.caps !== 'segment')).toHaveLength(1); + }); + + it('never resolves a device to itself through its own MAC', () => { + const graph = buildTopology(devices, [ + link({ from_device_id: 12, from_interface: 'ether1', link_type: 'lldp', + neighbor_mac: '04:F4:1C:9F:3D:A9' }), + ], [], macs); + expect(graph.links[0].to_device_id).toBeNull(); + }); + + it('works without a MAC index, as before', () => { + const graph = buildTopology(devices, trunkLinks(), []); + expect(graph.links.every((l) => l.to_device_id === null)).toBe(true); + }); +}); diff --git a/backend/src/services/topology/buildTopology.ts b/backend/src/services/topology/buildTopology.ts index 4ac825d..0b42148 100644 --- a/backend/src/services/topology/buildTopology.ts +++ b/backend/src/services/topology/buildTopology.ts @@ -188,11 +188,39 @@ function makeNeighborKey() { export function buildTopology( devices: TopoDevice[], allLinks: LinkRow[], - manualLinks: ManualLinkRow[] + manualLinks: ManualLinkRow[], + deviceMacs: { device_id: number; mac_address: string }[] = [] ): TopologyGraph { const amb = findAmbiguousIdentifiers(devices, allLinks); const neighborKey = makeNeighborKey(); + // ── Step 0: Resolve neighbours by MAC ────────────────────────────────────── + // + // Runs before everything else because a MAC identifies hardware and is unique + // fleet-wide, which no other neighbour field is. It also covers the case address + // matching cannot: a trunk port carries no IP, so LLDP across one reports a MAC + // and an identity but no address. Those links stayed unresolved and rendered as + // external nodes duplicating devices already on the map + // (github.com/2GT-Media-Group-LLC/mikrotik-manager/issues/95). + const macToDevice = new Map(); + const deviceNames = new Map(devices.map((d) => [d.id, d.name])); + for (const row of deviceMacs) { + if (!row.mac_address) continue; + const key = row.mac_address.toUpperCase(); + if (!macToDevice.has(key)) { + macToDevice.set(key, { id: row.device_id, name: deviceNames.get(row.device_id) ?? '' }); + } + } + + for (const link of allLinks) { + if (link.to_device_id || !link.neighbor_mac) continue; + const hit = macToDevice.get(link.neighbor_mac.toUpperCase()); + if (hit && hit.id !== link.from_device_id) { + link.to_device_id = hit.id; + link.to_device_name = hit.name || link.to_device_name; + } + } + // ── Step 1: Per-(device, neighbor) best-protocol dedup ───────────────────── const bestByPair = new Map(); for (const link of allLinks) { diff --git a/frontend/package.json b/frontend/package.json index 969894f..9a276e6 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.22.1-beta", + "version": "0.23.0-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/wireless/CapsmanPanel.tsx b/frontend/src/components/wireless/CapsmanPanel.tsx index 23c5bbf..1a66000 100644 --- a/frontend/src/components/wireless/CapsmanPanel.tsx +++ b/frontend/src/components/wireless/CapsmanPanel.tsx @@ -1,22 +1,77 @@ import { useQuery } from '@tanstack/react-query'; import { Link } from 'react-router-dom'; -import { Radio, RefreshCw, Server, Wifi, AlertTriangle, ChevronDown, ChevronRight } from 'lucide-react'; +import { Server, AlertTriangle, ChevronDown, ChevronRight } from 'lucide-react'; import { useState } from 'react'; -import { wirelessApi, type CapsmanController } from '../../services/api'; +import { wirelessApi, type CapsmanController, type CapsmanRadio } from '../../services/api'; import clsx from 'clsx'; +/** One row of the unified radio/interface table. */ +interface Row { + key: string; + deviceId: number | null; + deviceName: string; + 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; +} + /** - * CAPsMAN controllers and the access points they provision. + * Flatten a controller into one row per interface. * - * Read-only by design. A CAPsMAN configuration is applied to every AP bound to it, - * so an edit here would change the whole wireless fleet at once — that needs the - * same prediction-and-revert treatment device changes get, and is held back until - * it does (issue #94). + * 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. */ +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, + 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, + }); + } + } + return rows; +} + function Controller({ c }: { c: CapsmanController }) { const [open, setOpen] = useState(true); const unmanaged = c.access_points.filter((ap) => ap.device_id == null); - const radioCount = c.access_points.reduce((n, ap) => n + ap.radios.length, 0); + const rows = toRows(c); return (
@@ -44,9 +99,8 @@ function Controller({ c }: { c: CapsmanController }) {
{c.access_points.length} access point{c.access_points.length !== 1 ? 's' : ''} ·{' '} - {radioCount} radio{radioCount !== 1 ? 's' : ''} ·{' '} - {c.client_count} client{c.client_count !== 1 ? 's' : ''} ·{' '} - {c.configurations.length} configuration{c.configurations.length !== 1 ? 's' : ''} + {rows.length} interface{rows.length !== 1 ? 's' : ''} ·{' '} + {c.client_count} client{c.client_count !== 1 ? 's' : ''}
@@ -64,91 +118,56 @@ function Controller({ c }: { c: CapsmanController }) { )} - {/* Radios grouped by the AP they physically live on */} -
-
- Access points -
-
- {c.access_points.map((ap, i) => ( -
-
- - {ap.device_id != null ? ( - - {ap.device_name} - - ) : ( - {ap.device_name} - )} - {ap.radios[0]?.local && ( - on controller - )} - - {ap.client_count} client{ap.client_count !== 1 ? 's' : ''} - -
-
- {ap.radios.map((r) => ( -
- - {r.interface_name || r.radio_mac} - {r.current_channel && {r.current_channel}} - {r.registered_peers != null && ( - {r.registered_peers} client{r.registered_peers !== 1 ? 's' : ''} +
+ + + + {['Access point', 'Interface', 'SSID', 'Band', 'Frequency', 'Channel', 'Clients', 'Status'].map((h) => ( + + ))} + + + + {rows.map((r) => ( + + + + + + + + + + + ))} + +
{h}
+
+ {r.deviceId != null ? ( + + {r.deviceName} + + ) : ( + {r.deviceName} + )} + {r.local && ( + local )} - {r.tx_power != null && {r.tx_power} dBm} - {r.hw_type && {r.hw_type}}
- ))} - - - ))} - + 		{r.interfaceName}{r.ssid || —}{r.band || —}{r.frequency ?? —}{r.channel || —} + {r.clients ?? —} + + + {r.disabled ? 'disabled' : r.running ? 'running' : 'enabled'} + +
- {/* Provisioned SSIDs actually broadcasting */} - {c.ssids.length > 0 && ( -
-
- Provisioned interfaces -
-
- - - - {['Access point', 'Interface', 'SSID', 'Band', ''].map((h, i) => ( - - ))} - - - - {c.ssids.map((s, i) => ( - - - - - - - - ))} - -
{h}
{s.device_name}{s.name}{s.ssid || '—'} - {s.band || '—'}{s.frequency ? ` · ${s.frequency}` : ''} - - - {s.disabled ? 'disabled' : 'active'} - -
-
-
- )} - - {/* Configurations and the rules binding them to radios */} {c.configurations.length > 0 && (
@@ -156,22 +175,37 @@ function Controller({ c }: { c: CapsmanController }) {
{c.configurations.map((cfg) => ( -
+
{cfg.name}
{[cfg.ssid && `SSID ${cfg.ssid}`, cfg.band, cfg.mode, cfg.security] .filter(Boolean).join(' · ') || 'no details reported'}
+ {/* The rule and the configuration it applies are one idea. */} + {cfg.rules.length > 0 && ( +
    + {cfg.rules.map((p) => ( +
  • + {p.action || 'rule'} + {p.radio_mac && {p.radio_mac}} + {p.disabled && · disabled} + {p.comment && — {p.comment}} +
    • + ))} +
+ )}
))}
)} + {/* Rules that reference no configuration we collected — worth surfacing + rather than dropping, since that usually means a stale reference. */} {c.provisioning.length > 0 && (
- Provisioning rules + Unmatched provisioning rules
    {c.provisioning.map((p) => ( @@ -179,7 +213,6 @@ function Controller({ c }: { c: CapsmanController }) { {p.action || 'rule'} {p.master_configuration && <> → {p.master_configuration}} {p.radio_mac && ({p.radio_mac})} - {p.disabled && · disabled} {p.comment && — {p.comment}} ))} @@ -200,8 +233,7 @@ export default function CapsmanPanel() { }); const controllers = data?.controllers ?? []; - // Nothing to show on a fleet with no controller — this panel should be invisible - // rather than an empty box everyone has to scroll past. + // Invisible on a fleet with no controller rather than an empty box to scroll past. if (isLoading || controllers.length === 0) return null; return ( diff --git a/frontend/src/pages/DeviceDetailPage.tsx b/frontend/src/pages/DeviceDetailPage.tsx index d2c0b96..3145aed 100644 --- a/frontend/src/pages/DeviceDetailPage.tsx +++ b/frontend/src/pages/DeviceDetailPage.tsx @@ -104,7 +104,11 @@ export default function DeviceDetailPage() { ); } - const isWirelessAP = device.device_type === 'wireless_ap'; + // Show the Radios tab when the device actually has radios, not when someone + // tagged it "wireless_ap". A router can carry radios, and a CAPsMAN controller is + // normally tagged a router — both had no way to reach this tab (#94). + const isWirelessAP = device.device_type === 'wireless_ap' + || (device.wifi_role != null && device.wifi_role !== 'none'); const tabs: { key: TabKey; label: string }[] = [ { key: 'overview', label: 'Overview' }, diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index d983b99..04f428b 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -927,6 +927,8 @@ export interface CapsmanConfiguration { band: string | null; security: string | null; authentication_types: string | null; + /** Provisioning rules that apply this configuration. */ + rules: CapsmanProvisioning[]; } export interface CapsmanProvisioning { @@ -943,6 +945,8 @@ export interface CapsmanSsid { device_id: number; device_name: string; name: string; + /** Radio this interface sits on, used to join live channel and client counts. */ + radio_mac: string | null; ssid: string | null; band: string | null; frequency: number | null; diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index 4084a66..075ef14 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -69,6 +69,8 @@ export interface Device { location_lng?: number; rack_name?: string; rack_slot?: string; + /** Detected wireless role: none | standalone | cap | controller | controller_cap. */ + wifi_role?: string | null; created_at: string; updated_at?: string; tags?: { id: number; name: string; color: string }[]; diff --git a/frontend/src/version.ts b/frontend/src/version.ts index 32bc3ba..4bc2b3a 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.22.1 Beta'; +export const APP_VERSION = 'v0.23.0 Beta';