diff --git a/HANDOFF.md b/HANDOFF.md index b65313d..cdc7c6b 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -1517,6 +1517,96 @@ Nutzerwünsche in einer Runde): setzt die Adressfelder jetzt explizit statt sich auf Modell-Defaults zu verlassen). Live bestätigt ("gut soweit"). +**M26: "Bekannte Router" hinterlegt jetzt die Seriennummer** +(2026-09-16). Nutzer-Szenario, live so passiert: mit einem zweiten +Router desselben Modells verbunden (Werks-Adresse+Zugangsdaten +identisch zum bisherigen Testrouter, `192.168.88.1`/`admin`) — die App +hat daraufhin den *bestehenden* "Bekannte Router"-Eintrag des alten +Testrouters einfach für den neuen umbenannt (`SavedRoutersStore. +recordSuccessfulConnection` matchte bisher nur auf Host+Benutzername, +kein Feld unterschied zwei physisch verschiedene Geräte an derselben +Werks-Adresse). Klick auf den alten Eintrag verband danach zwangsläufig +mit dem tatsächlich erreichbaren Gerät (dem neuen Router), nicht mit +dem gemeinten alten. Fix (Nutzervorschlag: "ist es nicht besser die +Seriennummer des Gerätes mit zu hinterlegen?"): `SavedRouter` bekam ein +neues optionales `serialNumber`-Feld (`/system routerboard`s +"serial-number", bereits seit M14 als `RouterBoardInfo.serialNumber` +ausgelesen — "unbekannt" wird dabei wie `nil` behandelt, sonst würden +zwei Geräte ohne auslesbare Seriennummer fälschlich als "gleich" +gelten). `recordSuccessfulConnection` matcht jetzt gestuft: exakte +Seriennummer gewinnt zuerst; ein einzelner bestehender Eintrag ohne +gespeicherte Seriennummer (alte, vor diesem Feld gespeicherte Einträge, +oder Geräte ohne physisches Routerboard) wird beim nächsten Connect +einmalig nachträglich damit befüllt statt dupliziert; alles andere legt +einen neuen, getrennten Eintrag an. In `SavedRouterRow` als kleine +"SN: ..."-Zeile sichtbar gemacht, damit zwei gleich benannte/adressierte +Einträge auch optisch unterscheidbar sind. Drei neue Tests +(`testDifferentSerialAtSameHostUsernameCreatesSeparateEntry`, +`testLegacyEntryWithoutSerialGetsUpgradedInPlaceNotDuplicated`, +`testSameSerialReconnectUpdatesRecencyNotDuplicated`). Migrationshinweis +für den konkreten Fall: der bereits verschmolzene Bestandseintrag zeigt +aktuell die Seriennummer des neuen Routers — der nächste Connect zum +alten Testrouter legt automatisch einen neuen, eigenen Eintrag an (da +dessen Seriennummer nicht mehr passt), bei Bedarf danach umbenennen. +Live bestätigt ("passt, funktioniert") — allerdings dabei zwei weitere, +eng verwandte Bugs gefunden, siehe M27 direkt danach. + +**M27: Passwort-Anzeige-Button, Schlüsselbund-Trennung, `terse`-Fallback +verallgemeinert** (2026-09-16, beim Live-Test von M26 gefunden): + +1. **Passwort-Anzeige-Button** (Nutzerwunsch: "setze hinter das Passwort + eine Auge-Symbol zum aufdecken des Passwortes, so kann ich prüfen, ob + die Daten übernommen wurden") — Augen-Icon neben dem Passwortfeld im + Verbinden-Tab, schaltet zwischen `SecureField` und `TextField` um. + War das Diagnose-Werkzeug, das die beiden folgenden Bugs erst + aufdeckte. +2. **Bug 35: Schlüsselbund teilte sich einen Eintrag zwischen zwei + Routern** — beim Umschalten zwischen zwei "Bekannte Router"-Einträgen + (seit M26 korrekt als getrennte Einträge geführt) blieb das + angezeigte Passwort trotzdem identisch. Aufwendig eingegrenzt: ein + temporärer Debug-Print bestätigte zunächst, dass `ConnectViewModel`s + `@Published`-Werte bei jedem Klick korrekt gesetzt wurden (auch die + View selbst rendert nachweislich jedes Mal frisch, per zweitem + Debug-Print in `mainContent` bestätigt) — zwei SwiftUI-`.id()`- + Reparaturversuche (erst auf Section-, dann auf Feld-Ebene) liefen + deshalb ins Leere, weil das eigentliche Problem gar keins der + Anzeige war. Tatsächliche Ursache: `KeychainService` speichert + Passwörter nur nach `"username@host"` — zwei physisch verschiedene + Router mit identischem Host+Benutzername (Werks-Adresse) teilen sich + dadurch denselben Schlüsselbund-Eintrag, unabhängig davon, dass + `SavedRoutersStore` sie seit M26 bereits korrekt als getrennte + Einträge führt. Fix: `KeychainService.save`/`loadPassword`/ + `deletePassword` bekommen einen optionalen `serialNumber`-Parameter, + der den Account-Key auf `"username@host#seriennummer"` erweitert; + `loadPassword` fällt auf den alten, unqualifizierten Key zurück, + wenn kein qualifizierter Eintrag existiert (Rückwärtskompatibilität + für vor diesem Fix gespeicherte Passwörter). `ConnectViewModel. + connect()`s Passwort-Speicherung musste dafür hinter den + erfolgreichen Verbindungsaufbau verschoben werden (die Seriennummer + ist vorher noch nicht bekannt). Die beiden zuvor eingebauten + `.id()`-Umwege wieder entfernt, da wirkungslos und irreführend + (falsche Fehlerursache im Kommentar). +3. **Bug 36, direkt danach beim selben Live-Test gefunden: neuer Router + zeigte gar keine Routerboard-Infos, keine Seriennummer** — derselbe + Root Cause wie der vorherige "Jetzt prüfen"-Fehler (`/system package + update`, siehe oben): `/system routerboard print ... terse` schlägt + auf diesem Router/dieser RouterOS-Version mit `"expected end of + command (line 1 column N)"` fehl statt mit dem bisher einzig + abgefangenen `"bad parameter terse"`. `fetchMenuItems`s + Singleton-Fallback (Bug 8/20) griff deshalb nicht, + `routerBoardInfo` blieb `nil` (best-effort, Fehler wurde still + verschluckt) — betraf gleichzeitig die Seriennummer (M26) und die + komplette Routerboard-Anzeige im Verbinden-Tab. Fix: + `SSHTransport.fetchMenuItems`s Erkennung in eine neue + `rejectsTerse(_:)`-Hilfsfunktion ausgelagert, die jetzt beide + bekannten Formulierungen abfängt. Sicher eingegrenzt, da die + geprüfte Fehlermeldung ausschließlich vom exakten Aufruf `" + print without-paging terse"` stammt — jeder Parserfehler dort betrifft + per Konstruktion das anhängende `"terse"`. Behebt nebenbei auch den + "Jetzt prüfen"-Fehler von vorhin (`/system package update`), da + beide über denselben Code-Pfad laufen. Live bestätigt + ("passt, funktioniert"). + ## Nächste Schritte 1. ~~M16: restliche `RouterOSSchemaCatalog.swift`-Sektionen übersetzen~~, @@ -1713,6 +1803,10 @@ Nutzerwünsche in einer Runde): dieses Symptom nochmal auftritt: zuerst per `defaults read` prüfen, ob der Trust wirklich gespeichert wurde, nicht nur der UI-Bestätigung vertrauen. +14. ~~M26 (Seriennummer bei "Bekannte Router") noch live testen~~ — + erledigt, siehe M27 oben (dabei zwei weitere Bugs gefunden und + gefixt: geteilter Schlüsselbund-Eintrag, `/system routerboard`s + `terse`-Ablehnung mit anderem Fehlertext als bisher erkannt). Gitea-Remote `origin` ist eingerichtet und wird laufend gepusht (siehe oben) — dieser Hinweis war veraltet, korrigiert am 2026-09-15. diff --git a/Manual.md b/Manual.md index 2d1b28e..3b2eb63 100644 --- a/Manual.md +++ b/Manual.md @@ -49,7 +49,9 @@ bleiben unübersetzt. ## 1. Verbinden -- Router-Adresse, Benutzername und Passwort eingeben, verbinden. +- Router-Adresse, Benutzername und Passwort eingeben, verbinden. Ein + Augen-Symbol neben dem Passwortfeld deckt die Eingabe zur Kontrolle + auf. - Beim ersten Verbinden wird ein unbekanntes Zertifikat (REST-API, meist der Fall) oder ein unbekannter SSH-Host-Schlüssel (SSH- Fallback) angezeigt und muss einmalig bestätigt werden @@ -65,14 +67,17 @@ bleiben unübersetzt. - Schnell-Backup-Button direkt im Verbinden-Tab. - "Trennen"-Button beendet die Verbindung sauber. - **Bekannte Router**: nach jeder erfolgreichen Verbindung merkt sich die - App Host und Benutzername (Passwort bleibt wie gewohnt im macOS- - Schlüsselbund). Anzeigename wird beim ersten Mal automatisch auf die - Hardware-Bezeichnung gesetzt (z.B. "hEX"), lässt sich aber jederzeit - über "Bearbeiten" ändern — dort auch ein freies Standort-Feld (z.B. - "Keller, Serverschrank" oder "1. OG") zur besseren Unterscheidung - mehrerer Router. Klick auf einen Eintrag füllt Host/Benutzername/ - Passwort ins Formular, ohne sofort zu verbinden. Ab ca. 4 Einträgen - scrollt die Liste in sich selbst. + App Host, Benutzername und (falls vom Router gemeldet) Seriennummer. + Zwei unterschiedliche Router mit identischem Host+Benutzername (z.B. + beide auf MikroTiks Werks-Adresse 192.168.88.1/admin) bleiben dadurch + getrennte Einträge, erkennbar an der "SN: ..."-Zeile; auch das + gemerkte Passwort wird pro Gerät getrennt gespeichert. Anzeigename + wird beim ersten Mal automatisch auf die Hardware-Bezeichnung gesetzt + (z.B. "hEX"), lässt sich aber jederzeit über "Bearbeiten" ändern — + dort auch ein freies Standort-Feld (z.B. "Keller, Serverschrank" oder + "1. OG") zur besseren Unterscheidung mehrerer Router. Klick auf einen + Eintrag füllt Host/Benutzername/Passwort ins Formular, ohne sofort zu + verbinden. Ab ca. 4 Einträgen scrollt die Liste in sich selbst. - **Live-Traffic-Anzeige**: der Punkt vor jedem Interface in der Geräte-Übersicht ist grau (kein Link), grün (Link, aber kein Datenverkehr) oder pulsierend grün (überträgt gerade tatsächlich @@ -232,7 +237,8 @@ router (error messages, CLI command lines, live log) stay untranslated. ## 1. Connect -- Enter the router's address, username and password, then connect. +- Enter the router's address, username and password, then connect. An + eye icon next to the password field reveals it for a quick check. - On the first connection, an unknown certificate (REST API, the usual case) or an unknown SSH host key (SSH fallback) is shown and must be confirmed once (trust-on-first-use) — protects against a swapped/ @@ -247,13 +253,17 @@ router (error messages, CLI command lines, live log) stay untranslated. - Quick-backup button right in the Connect tab. - "Disconnect" button cleanly ends the session. - **Known Routers**: after every successful connection, the app remembers - the host and username (the password stays in macOS Keychain as usual). - The display name defaults to the hardware designation (e.g. "hEX") the - first time, but can be changed anytime via "Edit" — which also has a - free-text location field (e.g. "Basement, server rack" or "1st Floor") - to tell multiple routers apart. Clicking an entry fills in host/ - username/password without connecting yet. The list scrolls in place - past about 4 entries. + the host, username, and (when the router reports one) serial number. + Two different routers sharing the same host+username (e.g. both left + at MikroTik's factory default 192.168.88.1/admin) stay separate + entries because of this, shown by an "SN: ..." line; the remembered + password is also kept separate per device. The display name defaults + to the hardware designation (e.g. "hEX") the first time, but can be + changed anytime via "Edit" — which also has a free-text location + field (e.g. "Basement, server rack" or "1st Floor") to tell multiple + routers apart. Clicking an entry fills in host/username/password + without connecting yet. The list scrolls in place past about 4 + entries. - **Live traffic indicator**: the dot in front of each interface in the device overview is grey (no link), green (link but no traffic), or pulsing green (actually carrying data right now). diff --git a/README.md b/README.md index 708a22a..a34e3b3 100644 --- a/README.md +++ b/README.md @@ -158,6 +158,8 @@ nur die zugehörigen Passwörter liegen weiterhin im macOS-Schlüsselbund. | M23 | Experte-Tab: Sektionsüberschriften prominenter+eingefärbt, einklappbar (Standard: zugeklappt) | ✅ live verifiziert | | M24 | LAN-Scanner: "Aktionen"-Button statt Rechtsklick, Traffic-Monitor+Sparkline pro Port, ARP-Bug gefixt | ✅ live verifiziert | | M25 | Einrichten-Wizard-Politur: WAN-Zurück-Button, prominente Aktionsbuttons, leere Platzhalter-Felder (LAN/VLAN) | ✅ live verifiziert | +| M26 | Bekannte Router: Seriennummer hinterlegt, trennt zwei Geräte mit identischem Host+Benutzername | ✅ live verifiziert | +| M27 | Passwort-Anzeige-Button, Schlüsselbund nach Seriennummer getrennt, `terse`-Fallback verallgemeinert | ✅ live verifiziert | | — | LAN-Port-Konflikt-Prüfung + "Fertig"-Button (Einrichten) | 🔶 gebaut, Live-Test offen | Ausführlicher Stand inkl. aller gefundenen Bugs, offener Punkte und diff --git a/RouterOSAssistant/Core/Localization/L10n.swift b/RouterOSAssistant/Core/Localization/L10n.swift index 112a07f..d3e7282 100644 --- a/RouterOSAssistant/Core/Localization/L10n.swift +++ b/RouterOSAssistant/Core/Localization/L10n.swift @@ -541,6 +541,8 @@ enum L10n { "Das Passwort für diesen Benutzer. Bei unverändertem Werkszustand oft leer.": "The password for this user. Often empty in an unchanged factory state.", "Passwort merken": "Remember Password", + "Passwort verbergen": "Hide Password", + "Passwort anzeigen": "Show Password", "Speichert das Passwort verschlüsselt in der macOS-Schlüsselbundverwaltung, damit du es nicht jedes Mal neu eingeben musst.": "Stores the password encrypted in macOS Keychain so you don't have to type it in every time.", "Verbinde…": "Connecting…", diff --git a/RouterOSAssistant/Core/Models/SavedRouter.swift b/RouterOSAssistant/Core/Models/SavedRouter.swift index ed8ee9c..3fc5943 100644 --- a/RouterOSAssistant/Core/Models/SavedRouter.swift +++ b/RouterOSAssistant/Core/Models/SavedRouter.swift @@ -19,23 +19,39 @@ struct SavedRouter: Identifiable, Codable, Equatable { /// user-supplied, no default; empty means "not set", never shown then. var location: String var lastConnectedAt: Date + /// RouterOS' own hardware serial number (`/system routerboard`'s "serial-number" field) — + /// the only thing that actually identifies *this physical device*, unlike host+username, + /// which two different routers can share (e.g. both left at MikroTik's factory default + /// 192.168.88.1/admin). Nil for a device with no physical routerboard at all (e.g. a CHR + /// virtual router) or for an entry saved before this field existed — never a placeholder + /// string, so it's never mistaken for a real match. `SavedRoutersStore. + /// recordSuccessfulConnection` uses this to tell two physically different routers apart even + /// when they briefly share the same host/username, instead of silently merging them into one + /// entry (confirmed live, 2026-09-16: connecting to a second, same-model router at the same + /// factory-default address/username just renamed the *existing* entry from the first router + /// in place, with no way to tell the two apart afterwards). + var serialNumber: String? - init(id: UUID = UUID(), host: String, username: String, name: String, location: String = "", lastConnectedAt: Date = Date()) { + init( + id: UUID = UUID(), host: String, username: String, name: String, location: String = "", + lastConnectedAt: Date = Date(), serialNumber: String? = nil + ) { self.id = id self.host = host self.username = username self.name = name self.location = location self.lastConnectedAt = lastConnectedAt + self.serialNumber = serialNumber } private enum CodingKeys: String, CodingKey { - case id, host, username, name, location, lastConnectedAt + case id, host, username, name, location, lastConnectedAt, serialNumber } - /// Custom decoding so an already-saved list from before `location` existed keeps loading - /// (missing key -> "") instead of the whole list silently vanishing — `SavedRoutersStore. - /// load()` treats any decode failure as "no saved routers at all". + /// Custom decoding so an already-saved list from before `location`/`serialNumber` existed + /// keeps loading (missing key -> "" / nil) instead of the whole list silently vanishing — + /// `SavedRoutersStore.load()` treats any decode failure as "no saved routers at all". init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) id = try container.decode(UUID.self, forKey: .id) @@ -44,5 +60,6 @@ struct SavedRouter: Identifiable, Codable, Equatable { name = try container.decode(String.self, forKey: .name) location = try container.decodeIfPresent(String.self, forKey: .location) ?? "" lastConnectedAt = try container.decode(Date.self, forKey: .lastConnectedAt) + serialNumber = try container.decodeIfPresent(String.self, forKey: .serialNumber) } } diff --git a/RouterOSAssistant/Core/Networking/SSHTransport.swift b/RouterOSAssistant/Core/Networking/SSHTransport.swift index 176bf8f..5a21ec8 100644 --- a/RouterOSAssistant/Core/Networking/SSHTransport.swift +++ b/RouterOSAssistant/Core/Networking/SSHTransport.swift @@ -121,7 +121,7 @@ final class SSHTransport: RouterOSTransport { let output: String do { output = try await run("\(menuPath) print without-paging terse") - } catch RouterOSError.invalidResponse(let detail) where detail.contains("bad parameter terse") { + } catch RouterOSError.invalidResponse(let detail) where Self.rejectsTerse(detail) { // Singleton menu (e.g. "/ip dns", "/system identity") — no list, no "terse" support. let plain = try await run("\(menuPath) print without-paging") return [RouterOSCliParser.parseSingletonItem(plain)] @@ -132,7 +132,7 @@ final class SSHTransport: RouterOSTransport { // lesson from Bug 10 applying to a *read* command here, not just add/set/remove. Without // this, such a menu's data silently comes back empty rather than throwing or falling // back — no crash, no error, just nothing, which is worse than either. - if output.contains("bad parameter terse") { + if Self.rejectsTerse(output) { let plain = try await run("\(menuPath) print without-paging") return [RouterOSCliParser.parseSingletonItem(plain)] } @@ -149,6 +149,20 @@ final class SSHTransport: RouterOSTransport { return items } + /// RouterOS rejects `terse` on a singleton menu in more than one wording, confirmed live + /// across two different devices/RouterOS versions (2026-09-16): "bad parameter terse" + /// (original hEX test device) and a harder parser error, "expected end of command (line 1 + /// column N)" (a second, different router) — both were seen on `/system routerboard`, the + /// second one also on `/system package update`, silently leaving Routerboard info (including + /// the serial number "Bekannte Router" needs to tell two same-address devices apart) and the + /// software-update check both empty with no visible error. Scoped safely to this one call + /// site: `detail`/`output` here only ever come from running `" print + /// without-paging terse"`, so any parser error on that exact line is — by construction — + /// about the trailing "terse" token, not some unrelated syntax problem elsewhere. + private static func rejectsTerse(_ text: String) -> Bool { + text.contains("bad parameter terse") || text.contains("expected end of command") + } + /// `:foreach i in=[ find whereField=whereValue] do={:put [ get $i /// returnField]}` — built from two independently confirmed-live primitives only: a bare /// `find` with one condition (verified repeatedly this session, e.g. diff --git a/RouterOSAssistant/Core/Services/KeychainService.swift b/RouterOSAssistant/Core/Services/KeychainService.swift index 0a23baf..ec792e2 100644 --- a/RouterOSAssistant/Core/Services/KeychainService.swift +++ b/RouterOSAssistant/Core/Services/KeychainService.swift @@ -2,15 +2,29 @@ import Foundation import Security /// Stores router login passwords in the macOS Keychain, never in plaintext. +/// +/// Account keys are `"username@host"`, optionally suffixed `"#"` when the caller +/// knows which physical device it is. Without the serial, two different routers left at +/// MikroTik's factory default (192.168.88.1/admin) share one Keychain entry — confirmed live, +/// 2026-09-16: switching between two "Bekannte Router" entries for such devices (already +/// correctly kept separate by `SavedRoutersStore` since M26) kept loading the *same* remembered +/// password for both, since this store never looked past host+username. `loadPassword` still +/// falls back to the plain, unqualified key when no serial-qualified entry exists yet, so +/// passwords saved before this change (or for a device with no routerboard/serial at all) keep +/// working. struct KeychainService { private let service = "com.focus72.RouterOSAssistant" - func save(password: String, forHost host: String, username: String) { - let account = "\(username)@\(host)" + private func account(host: String, username: String, serialNumber: String?) -> String { + guard let serialNumber else { return "\(username)@\(host)" } + return "\(username)@\(host)#\(serialNumber)" + } + + func save(password: String, forHost host: String, username: String, serialNumber: String? = nil) { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, - kSecAttrAccount as String: account + kSecAttrAccount as String: account(host: host, username: username, serialNumber: serialNumber) ] SecItemDelete(query as CFDictionary) @@ -19,8 +33,15 @@ struct KeychainService { SecItemAdd(attributes as CFDictionary, nil) } - func loadPassword(forHost host: String, username: String) -> String? { - let account = "\(username)@\(host)" + func loadPassword(forHost host: String, username: String, serialNumber: String? = nil) -> String? { + if let serialNumber, + let password = loadPassword(account: account(host: host, username: username, serialNumber: serialNumber)) { + return password + } + return loadPassword(account: account(host: host, username: username, serialNumber: nil)) + } + + private func loadPassword(account: String) -> String? { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, @@ -34,12 +55,11 @@ struct KeychainService { return String(data: data, encoding: .utf8) } - func deletePassword(forHost host: String, username: String) { - let account = "\(username)@\(host)" + func deletePassword(forHost host: String, username: String, serialNumber: String? = nil) { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, - kSecAttrAccount as String: account + kSecAttrAccount as String: account(host: host, username: username, serialNumber: serialNumber) ] SecItemDelete(query as CFDictionary) } diff --git a/RouterOSAssistant/Core/Services/SavedRoutersStore.swift b/RouterOSAssistant/Core/Services/SavedRoutersStore.swift index bcc6f23..6a8abd3 100644 --- a/RouterOSAssistant/Core/Services/SavedRoutersStore.swift +++ b/RouterOSAssistant/Core/Services/SavedRoutersStore.swift @@ -39,17 +39,35 @@ struct SavedRoutersStore { } /// Called after a successful connection: adds a new entry (name defaulting to `defaultName`) - /// if this host/username pair isn't known yet, or just bumps `lastConnectedAt` on the existing - /// one. An existing entry's `name` is never touched here — that's what preserves a user's own - /// rename across later reconnects. + /// if this host/username/serial combination isn't known yet, or just bumps `lastConnectedAt` + /// on the existing one. An existing entry's `name` is never touched here — that's what + /// preserves a user's own rename across later reconnects. + /// + /// `serialNumber` disambiguates two physically different routers that happen to share the + /// same host/username (e.g. both left at MikroTik's factory default 192.168.88.1/admin) — + /// without it, connecting to a second such device would silently rename the first one's + /// existing entry in place instead of creating a new one, live-confirmed 2026-09-16. Matching + /// order: (1) an entry with the exact same serial wins outright; (2) failing that, exactly one + /// same-host/username entry with no recorded serial yet is treated as the same device seen for + /// the first time since this field existed, and gets its serial filled in now; (3) otherwise + /// this is a genuinely new device (or an ambiguous multi-match this app won't guess at) and + /// gets its own new entry. @discardableResult - func recordSuccessfulConnection(host: String, username: String, defaultName: String) -> [SavedRouter] { + func recordSuccessfulConnection(host: String, username: String, defaultName: String, serialNumber: String? = nil) -> [SavedRouter] { var routers = loadRaw() - if let index = routers.firstIndex(where: { $0.host == host && $0.username == username }) { + let candidateIndices = routers.indices.filter { routers[$0].host == host && routers[$0].username == username } + + if let serialNumber, let index = candidateIndices.first(where: { routers[$0].serialNumber == serialNumber }) { routers[index].lastConnectedAt = Date() - } else { - routers.append(SavedRouter(host: host, username: username, name: defaultName)) + return save(routers) } + let unrecordedSerialIndices = candidateIndices.filter { routers[$0].serialNumber == nil } + if unrecordedSerialIndices.count == 1, let index = unrecordedSerialIndices.first { + routers[index].lastConnectedAt = Date() + routers[index].serialNumber = serialNumber + return save(routers) + } + routers.append(SavedRouter(host: host, username: username, name: defaultName, serialNumber: serialNumber)) return save(routers) } diff --git a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift index 5c4f11b..3dff1a3 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift @@ -7,6 +7,10 @@ struct ConnectView: View { @State private var showUpdateInstallConfirmation = false @State private var showFirmwareUpgradeConfirmation = false @State private var showRebootConfirmation = false + /// Per explicit request: a way to check what's actually in the password field, e.g. after + /// selecting a "Bekannte Router" entry ("setze hinter das Passwort ein Auge-Symbol ... so + /// kann ich prüfen, ob die Daten übernommen wurden"). + @State private var isPasswordVisible = false @AppStorage("appLanguage") private var appLanguage: String = "de" /// How many "Bekannte Router" rows are visible before the list scrolls in place — per @@ -71,8 +75,23 @@ struct ConnectView: View { .help(L10n.t("Die Adresse deines Routers im Netzwerk. Werkseinstellung bei Mikrotik ist meist 192.168.88.1.", appLanguage)) TextField(L10n.t("Benutzername", appLanguage), text: $viewModel.username) .help(L10n.t("Der Admin-Benutzername deines Routers. Werkseinstellung ist meist \"admin\".", appLanguage)) - SecureField(L10n.t("Passwort", appLanguage), text: $viewModel.password) + HStack { + Group { + if isPasswordVisible { + TextField(L10n.t("Passwort", appLanguage), text: $viewModel.password) + } else { + SecureField(L10n.t("Passwort", appLanguage), text: $viewModel.password) + } + } .help(L10n.t("Das Passwort für diesen Benutzer. Bei unverändertem Werkszustand oft leer.", appLanguage)) + Button { + isPasswordVisible.toggle() + } label: { + Image(systemName: isPasswordVisible ? "eye.slash" : "eye") + } + .buttonStyle(.plain) + .help(isPasswordVisible ? L10n.t("Passwort verbergen", appLanguage) : L10n.t("Passwort anzeigen", appLanguage)) + } Toggle(L10n.t("Passwort merken", appLanguage), isOn: $viewModel.rememberPassword) .help(L10n.t("Speichert das Passwort verschlüsselt in der macOS-Schlüsselbundverwaltung, damit du es nicht jedes Mal neu eingeben musst.", appLanguage)) } @@ -560,6 +579,16 @@ private struct SavedRouterRow: View { Text("\(router.username)@\(router.host)") .font(.caption) .foregroundStyle(.secondary) + // Nutzerwunsch (2026-09-16): Seriennummer mit hinterlegen, damit zwei + // physisch unterschiedliche Router mit identischem Host+Benutzername (z.B. + // beide auf MikroTiks Werks-Adresse 192.168.88.1/admin) trotzdem als + // getrennte Einträge unterscheidbar bleiben — siehe + // SavedRoutersStore.recordSuccessfulConnection. + if let serialNumber = router.serialNumber { + Text("SN: \(serialNumber)") + .font(.caption2) + .foregroundStyle(.tertiary) + } } .contentShape(Rectangle()) .onTapGesture { diff --git a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift index 107f0a9..b31c56e 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift @@ -49,7 +49,7 @@ final class ConnectViewModel: ObservableObject { func selectSavedRouter(_ router: SavedRouter) { host = router.host username = router.username - password = keychain.loadPassword(forHost: router.host, username: router.username) ?? "" + password = keychain.loadPassword(forHost: router.host, username: router.username, serialNumber: router.serialNumber) ?? "" } func renameSavedRouter(_ id: SavedRouter.ID, to newName: String) { @@ -66,15 +66,27 @@ final class ConnectViewModel: ObservableObject { func connect() { let credentials = RouterOSCredentials(host: host, username: username, password: password) - if rememberPassword { - keychain.save(password: password, forHost: host, username: username) - } Task { await connectionService.connect(with: credentials) if case .connected = connectionService.state { let defaultName = connectionService.deviceInfo?.boardName ?? host + // "unbekannt" is RouterOSModels' own fallback when RouterOS reports the + // routerboard menu but leaves this one field out — treated as "no real serial" + // here too, or two such devices would falsely look like the same one. + let rawSerial = connectionService.routerBoardInfo?.serialNumber + let serialNumber = (rawSerial?.isEmpty == false && rawSerial != "unbekannt") ? rawSerial : nil + // Saved only now, not before attempting the connection — the serial (needed to + // keep two same-host/username devices' passwords apart, see `KeychainService`'s + // doc comment) isn't known until the connection actually succeeds. Confirmed + // live (2026-09-16): saving unqualified beforehand made both of two same-model + // routers, left at MikroTik's factory default, silently share one Keychain + // entry — reconnecting to either one always loaded whichever password was saved + // most recently, regardless of which device the user actually selected. + if rememberPassword { + keychain.save(password: password, forHost: host, username: username, serialNumber: serialNumber) + } savedRouters = savedRoutersStore.recordSuccessfulConnection( - host: host, username: username, defaultName: defaultName + host: host, username: username, defaultName: defaultName, serialNumber: serialNumber ) } } diff --git a/RouterOSAssistantTests/SavedRoutersStoreTests.swift b/RouterOSAssistantTests/SavedRoutersStoreTests.swift index 4f98b18..fce441f 100644 --- a/RouterOSAssistantTests/SavedRoutersStoreTests.swift +++ b/RouterOSAssistantTests/SavedRoutersStoreTests.swift @@ -85,6 +85,37 @@ final class SavedRoutersStoreTests: XCTestCase { XCTAssertTrue(store.load().isEmpty) } + /// Live-confirmed (2026-09-16): a second, same-model router left at MikroTik's factory + /// default (192.168.88.1/admin) previously just renamed the *existing* entry from the first + /// router in place, with no way to tell the two physical devices apart afterwards. + func testDifferentSerialAtSameHostUsernameCreatesSeparateEntry() { + let store = makeStore() + store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: "AAA111") + let routers = store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: "BBB222") + + XCTAssertEqual(routers.count, 2) + } + + /// A router saved before `serialNumber` existed (or from a device with no routerboard) has + /// no recorded serial yet — the next connection that DOES report one should fill it in on + /// the same entry rather than creating a duplicate. + func testLegacyEntryWithoutSerialGetsUpgradedInPlaceNotDuplicated() { + let store = makeStore() + store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: nil) + let routers = store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: "AAA111") + + XCTAssertEqual(routers.count, 1) + XCTAssertEqual(routers[0].serialNumber, "AAA111") + } + + func testSameSerialReconnectUpdatesRecencyNotDuplicated() { + let store = makeStore() + store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: "AAA111") + let routers = store.recordSuccessfulConnection(host: "192.168.88.1", username: "admin", defaultName: "hEX", serialNumber: "AAA111") + + XCTAssertEqual(routers.count, 1) + } + func testLoadOrdersMostRecentlyConnectedFirst() { let store = makeStore() store.recordSuccessfulConnection(host: "10.0.0.1", username: "admin", defaultName: "Router A")