From 203465913c1b11f6515ee0dd8eaeedd92658fb1b Mon Sep 17 00:00:00 2001 From: Jon Vanvik Date: Sun, 31 May 2026 16:36:49 +0200 Subject: [PATCH] Apply code-review fixes: identity/pagination, load races, TLS pin, leaks Addresses a multi-agent code review (26 confirmed findings + a URLSession leak) across correctness, concurrency, security, and cleanup: - MongoDocument.id: deterministic fallback (was UUID() on every access) to stop SwiftUI List/ForEach identity churn; getAll pagination drives its cursor off the real _id via documentId, so a missing _id stops paging instead of looping on a fabricated cursor. - View models: per-load generation guard (+ captured-tab guard on ONU detail) so a cancelled or overlapping load can't commit stale results or land data on the wrong tab. - JSONValue.intValue: guard Int(d) against NaN/inf/out-of-range (crash). - ServerTrustEvaluator: hash the real SubjectPublicKeyInfo (prepend the algorithm-specific ASN.1 SPKI header) so a standard openssl-captured public-key pin actually matches; unknown key types fail closed. - AppEnvironment.configure: clear stale cookies + Keychain creds on host change. - FirmwareFile.isCompatible: empty metadata no longer passes as compatible-for-all. - APIClient: invalidate URLSession on deinit (per-configure/probe leak). - Dashboard best-effort sections keep last-good on transient failure; unified optical thresholds; LoginView prefill-once; populatedBuckets unique ForEach id; Format rounding rollover; dead-code/docstring/dedup cleanups. Verification pending on the Mac (no Swift toolchain on the Linux dev box). Co-Authored-By: Claude Opus 4.8 (1M context) --- APIClient.swift | 14 +++++- APIEnvelope.swift | 9 +++- APIQuery.swift | 5 +++ AppEnvironment.swift | 13 +++++- AuthModels.swift | 2 - DashboardDrilldownViews.swift | 2 +- DashboardViewModel.swift | 54 +++++++++++++---------- DeviceModels.swift | 83 +++++++++++++++++++++++------------ DeviceRepositories.swift | 12 +++-- Firmware.swift | 17 ++++--- Format.swift | 26 +++++++---- LoginView.swift | 6 +++ MCMSEndpoint.swift | 4 ++ OLTDetailView.swift | 2 +- OLTDetailViewModel.swift | 12 ++++- OLTListViewModel.swift | 9 +++- ONUDetailView.swift | 8 ++-- ONUDetailViewModel.swift | 48 +++++++++++++++----- ONUListViewModel.swift | 9 +++- ONURepository.swift | 4 +- ServerTrustEvaluator.swift | 67 +++++++++++++++++++++++++--- 21 files changed, 303 insertions(+), 103 deletions(-) diff --git a/APIClient.swift b/APIClient.swift index 8568edd..ad98959 100644 --- a/APIClient.swift +++ b/APIClient.swift @@ -47,6 +47,12 @@ final class APIClient { self.session = URLSession(configuration: cfg, delegate: trustDelegate, delegateQueue: nil) } + /// A `URLSession` created with a delegate retains that delegate (and itself) + /// until invalidated — Apple: an un-invalidated session "leaks memory until + /// it exits." This client is rebuilt on every `configure`/`probe`, so tear the + /// session down when the client goes away. + deinit { session.invalidateAndCancel() } + // MARK: - Public API /// GET returning a decoded `T` from the envelope's `data`. @@ -128,6 +134,11 @@ final class APIClient { /// is returned. `pageLimit` defaults to 100: full Mongo docs are ~30 KB, so /// larger pages (the server default is 1000) can exceed the front-end proxy /// timeout on big fleets. See MCMS field guide §3.3–3.4. + /// + /// `idOf` MUST return the document's real `_id` (the cursor MCMS excludes + /// at/before), or nil. Pass `{ $0.documentId }`, never the synthesized + /// `Identifiable.id` — a fabricated id is a meaningless cursor that makes the + /// server re-serve or skip pages. A nil id safely stops pagination. func getAll( _ endpoint: MCMSEndpoint, baseQuery: APIQuery = .empty, @@ -143,7 +154,8 @@ final class APIClient { q.next = cursor let page: [T] = try await get(endpoint, query: q, as: [T].self, version: version) all.append(contentsOf: page) - cursor = page.count == pageLimit ? page.last.flatMap(idOf) : nil + // Stop on a short/empty page, or when the last doc has no usable `_id`. + cursor = (page.count == pageLimit) ? page.last.flatMap(idOf) : nil } while cursor != nil return all } diff --git a/APIEnvelope.swift b/APIEnvelope.swift index 10fa611..65ec4bf 100644 --- a/APIEnvelope.swift +++ b/APIEnvelope.swift @@ -78,7 +78,14 @@ indirect enum JSONValue: Codable, Equatable { } var stringValue: String? { if case .string(let s) = self { return s }; return nil } var intValue: Int? { - switch self { case .int(let i): return i; case .double(let d): return Int(d); default: return nil } + switch self { + case .int(let i): return i + // `Int(_:)` traps on NaN/±inf/out-of-range; JSONDecoder can yield those + // from extreme literals (e.g. `1e400` → .infinity). Guard so a malformed + // server number becomes nil instead of a crash. + case .double(let d): return (d.isFinite && d >= Double(Int.min) && d < Double(Int.max)) ? Int(d) : nil + default: return nil + } } var doubleValue: Double? { switch self { case .double(let d): return d; case .int(let i): return Double(i); default: return nil } diff --git a/APIQuery.swift b/APIQuery.swift index 95da4bf..a7867af 100644 --- a/APIQuery.swift +++ b/APIQuery.swift @@ -32,6 +32,11 @@ struct APIQuery { return f }() + /// NOTE: `query`/`projection`/`sort` are flattened with literal `=`/`,` + /// separators that MCMS uses to split pairs. Keys/values are NOT escaped, so a + /// key or value that itself contains `=` or `,` will be mis-parsed into the + /// wrong filter. Current callers only use simple equality on safe field names; + /// keep it that way (or escape per the MCMS parser's rules) — see field guide §3.8. func queryItems() -> [URLQueryItem] { var items: [URLQueryItem] = [] diff --git a/AppEnvironment.swift b/AppEnvironment.swift index 35b56c6..29a35e2 100644 --- a/AppEnvironment.swift +++ b/AppEnvironment.swift @@ -23,6 +23,13 @@ final class AppEnvironment { // MARK: Configuration func configure(_ config: APIConfiguration) { + // Pointing at a different server: drop the old host's session cookies and + // saved credentials. Keychain accounts aren't host-scoped, so leaving them + // would offer server A's login (and replay A's stale `sessionid`) on B. + if configuration?.host != config.host { + connection?.session.clear() + forgetCredentials() + } configuration = config connection = makeConnection(config) isAuthenticated = false @@ -86,8 +93,10 @@ final class AppEnvironment { isAuthenticated = false } - /// Reachability / trust probe used by the Settings screen. Builds a - /// throwaway connection so testing does NOT disturb the active session. + /// Reachability / trust probe used by the Settings screen. Builds a throwaway + /// connection and issues only an unauthenticated `version()` GET. (It shares + /// `HTTPCookieStorage.shared` with the live session, but performs no login, so + /// it does not alter authenticated session state.) /// A 401/403/404 still means the host answered over a trusted TLS channel — /// that's a successful reachability result; only transport/trust errors fail. func probe(_ config: APIConfiguration) async throws -> String { diff --git a/AuthModels.swift b/AuthModels.swift index cb92110..755da2a 100644 --- a/AuthModels.swift +++ b/AuthModels.swift @@ -16,8 +16,6 @@ struct AuthResponse: Decodable, EmptyRepresentable { private init(raw: JSONValue) { self.raw = raw } static var emptyValue: AuthResponse { AuthResponse(raw: .null) } - - var email: String? { raw["email"]?.stringValue ?? raw["User"]?["email"]?.stringValue } } /// `GET /v{n}/ponmgr/version/` — attribute set describing the install. diff --git a/DashboardDrilldownViews.swift b/DashboardDrilldownViews.swift index 5caeaf7..bc9adb5 100644 --- a/DashboardDrilldownViews.swift +++ b/DashboardDrilldownViews.swift @@ -23,7 +23,7 @@ struct AbnormalRxListView: View { VStack(alignment: .trailing, spacing: 2) { Text("\(rx.formatted(.number.precision(.fractionLength(1)))) dBm") .font(.callout.monospacedDigit()).foregroundStyle(.orange) - Text(rx < -28 ? "weak" : "strong") + Text(rx < OpticalThreshold.rxGreenFloor ? "weak" : "strong") .font(.caption2).foregroundStyle(.secondary) } } diff --git a/DashboardViewModel.swift b/DashboardViewModel.swift index afade79..b1d7951 100644 --- a/DashboardViewModel.swift +++ b/DashboardViewModel.swift @@ -24,55 +24,65 @@ final class DashboardViewModel { var lasersOffOLTs: [OLTStateDoc] = [] private let connection: MCMSConnection + /// Bumped per load; only the latest load commits, so the 30s auto-refresh loop + /// and a manual pull-to-refresh can't interleave partial writes. + private var loadGeneration = 0 init(connection: MCMSConnection) { self.connection = connection } /// Light by default: the ONU count, state breakdown, OLT traffic and /// laser-off count all come from the (small) OLT-STATE docs. The heavy /// per-ONU optical scan runs only when `extraStats` is on. func load(extraStats: Bool = false) async { + loadGeneration += 1 + let gen = loadGeneration phase = .loading do { // OLT-STATE drives the OLT count, ONU registration (dev guide // Procedure 2), total traffic, and laser state — all from a handful - // of small docs. + // of small docs. Fetch everything into locals first, then commit once, + // so an overlapping load can't publish a half-updated dashboard. let olts = try await connection.olt.allStates() - oltCount = olts.count - let registration = OLTRepository.registrationMap(from: olts) + + // Best-effort sections: nil means "fetch failed this cycle" → keep the + // last good value rather than flicker to empty/zero on a transient blip. + let controllers = try? await connection.controller.allConfigs().count + let alarms = try? await connection.onu.alarmHistories() + + // Opt-in heavy scan: pull every ONU-STATE for the Rx optical check. + var optAvailable = false + var abnormal: [ONUStateDoc] = [] + if extraStats { + let onus = try await connection.onu.allStates() + optAvailable = onus.contains { $0.rxOpticalDBm != nil } + abnormal = onus.filter { $0.rxOpticalDBm.map(OpticalThreshold.rxAbnormal) ?? false } + } + + guard gen == loadGeneration, !Task.isCancelled else { return } + + oltCount = olts.count onuTotal = registration.count onuCounts = Dictionary(grouping: registration.values, by: { $0 }) .map { (state: $0.key, count: $0.value.count) } .sorted { $0.count > $1.count } - totalDownstreamBps = olts.compactMap(\.txRateBps).reduce(0, +) totalUpstreamBps = olts.compactMap(\.rxRateBps).reduce(0, +) lasersOffOLTs = olts.filter(\.isLaserOff) lasersOffCount = lasersOffOLTs.count - controllerCount = (try? await connection.controller.allConfigs().count) ?? 0 - - if let alarms = try? await connection.onu.alarmHistories() { + if let controllers { controllerCount = controllers } // else keep last good + if let alarms { severityCounts = Dictionary(grouping: alarms, by: { $0.severity }) .map { (severity: $0.key, count: $0.value.count) } .sorted { $0.severity.rank < $1.severity.rank } - } + } // else keep last good - // Opt-in heavy scan: pull every ONU-STATE for the Rx optical check. - if extraStats { - let onus = try await connection.onu.allStates() - opticalAvailable = onus.contains { $0.rxOpticalDBm != nil } - abnormalRxONUs = onus.filter { onu in - guard let rx = onu.rxOpticalDBm else { return false } - return rx < -28 || rx > -10 - } - abnormalRxCount = abnormalRxONUs.count - } else { - opticalAvailable = false - abnormalRxONUs = [] - abnormalRxCount = 0 - } + opticalAvailable = optAvailable + abnormalRxONUs = abnormal + abnormalRxCount = abnormal.count phase = .loaded } catch { + guard gen == loadGeneration, !Task.isCancelled else { return } phase = .failed((error as? APIError)?.localizedDescription ?? error.localizedDescription) } } diff --git a/DeviceModels.swift b/DeviceModels.swift index 9727daf..94da313 100644 --- a/DeviceModels.swift +++ b/DeviceModels.swift @@ -80,6 +80,12 @@ enum AlarmSeverity: String, Decodable, CaseIterable, Comparable { } static func < (lhs: AlarmSeverity, rhs: AlarmSeverity) -> Bool { lhs.rank < rhs.rank } + + /// Read a severity from a Mongo doc that may key it as `Severity` or `severity`. + static func from(_ raw: JSONValue) -> AlarmSeverity { + guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue else { return .unknown } + return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown + } } // MARK: - Flexible documents @@ -96,15 +102,41 @@ protocol MongoDocument: Decodable, Identifiable { } extension MongoDocument { + /// The document's real Mongo `_id`, or nil when absent. Use THIS for the + /// pagination cursor (`getAll`) — never the `id` fallback below. var documentId: String? { raw["_id"]?.stringValue } } +// NOTE on `id`: each document's `id` falls back to `raw.compactDescription` when +// `_id` is missing — a deterministic value derived from the content, NOT a fresh +// `UUID()`. `id` is a *computed* property, so a random UUID would change on every +// access and hand SwiftUI a new identity each render, tearing down and rebuilding +// List/ForEach rows (lost scroll position, broken animations). The content-derived +// key is stable for the same instance, so identity holds across re-renders. + +/// nil when the string is nil or empty, else the string. Shared by the document +/// accessors that should treat "" as "absent". +private func nonEmpty(_ s: String?) -> String? { (s?.isEmpty == false) ? s : nil } + +/// Single source of truth for ONU optical thresholds (dBm, user guide §7). Shared +/// by the dashboard abnormal-Rx filter and the detail-view tinting so the two +/// screens can't give the operator contradicting verdicts for the same reading. +enum OpticalThreshold { + static let rxGreenFloor = -28.0 // at/above (and ≤ rxHigh) is in-spec / green + static let rxRedFloor = -30.0 // below this is critical / red + static let rxHigh = -10.0 // above this is abnormal (too hot) + static let txGreenFloor = 3.0 + + /// Whether an RX reading (dBm) is abnormal: too weak or too hot. + static func rxAbnormal(_ dBm: Double) -> Bool { dBm < rxGreenFloor || dBm > rxHigh } +} + /// ONU state document (`ONU-STATE`). struct ONUStateDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } var serialNumber: String? { raw["_id"]?.stringValue } /// Parent OLT MAC, from `OLT.MAC Address` (dev guide Procedure 2 §2). @@ -143,7 +175,7 @@ struct ONUStateDoc: MongoDocument { struct ONUConfigDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } /// Operator-set label (`ONU.Name`, §5.1). var name: String? { raw["ONU"]?["Name"]?.stringValue } var netconfName: String? { raw["Netconf"]?["Name"]?.stringValue } @@ -153,7 +185,7 @@ struct ONUConfigDoc: MongoDocument { struct OLTConfigDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // MAC address + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // MAC address /// Operator label from `OLT.Name` (confirmed against a live OLT-CFG, e.g. /// "OLT69-RIN"). Note `NETCONF.Name` defaults to the MAC, so it's *not* the /// friendly name. Returns nil when unset. @@ -168,11 +200,12 @@ struct OLTConfigDoc: MongoDocument { struct OLTStateDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // OLT MAC address + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // OLT MAC address var lastUpdate: String? { raw["Time"]?.stringValue } - /// Operator name from OLT-CFG (`Netconf.Name`); resolved + stamped by the - /// list VM via `OLTRepository.nameMap()`. `nil` until resolved / if unset. + /// Operator name from OLT-CFG (`OLT.Name` — NOT `Netconf.Name`, which defaults + /// to the MAC); resolved + stamped by the list VM via `OLTRepository.nameMap()`. + /// `nil` until resolved / if unset. var resolvedName: String? /// Best label to show: the operator name if set, else the MAC id. var displayName: String { @@ -194,14 +227,18 @@ struct OLTStateDoc: MongoDocument { } } - /// Non-empty registration buckets, ordered by lifecycle severity. - var populatedBuckets: [(state: ONULifecycleState, ids: [String])] { + /// Non-empty registration buckets, ordered by lifecycle severity. Carries the + /// raw bucket `name` so callers key `ForEach` on it — keying on `state` would + /// collide when two unrecognized bucket names both map to `.unknown`. + var populatedBuckets: [(name: String, state: ONULifecycleState, ids: [String])] { onuStatesByBucket .filter { !$0.value.isEmpty } - .map { (state: ONULifecycleState(rawValue: $0.key) ?? .unknown, ids: $0.value.sorted()) } + .map { (name: $0.key, state: ONULifecycleState(rawValue: $0.key) ?? .unknown, ids: $0.value.sorted()) } .sorted { let order = ONULifecycleState.allCases - return (order.firstIndex(of: $0.state) ?? .max) < (order.firstIndex(of: $1.state) ?? .max) + let a = order.firstIndex(of: $0.state) ?? .max + let b = order.firstIndex(of: $1.state) ?? .max + return (a, $0.name) < (b, $1.name) // name tiebreak → stable order for unknowns } } } @@ -210,7 +247,7 @@ struct OLTStateDoc: MongoDocument { struct ControllerConfigDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // MAC address + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // MAC address var version: String? { raw["CNTL"]?["Version"]?.stringValue } } @@ -218,14 +255,10 @@ struct ControllerConfigDoc: MongoDocument { struct AlarmHistoryDoc: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // TODO: confirm key paths. - var severity: AlarmSeverity { - guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue - else { return .unknown } - return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown - } + var severity: AlarmSeverity { AlarmSeverity.from(raw) } var message: String? { raw["Message"]?.stringValue ?? raw["message"]?.stringValue } var raisedAt: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue } } @@ -234,7 +267,7 @@ struct AlarmHistoryDoc: MongoDocument { struct StatSample: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } var timestamp: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue } /// Pull a numeric counter by key for charting. @@ -251,12 +284,8 @@ struct StatSample: MongoDocument { struct LogEntry: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } - var severity: AlarmSeverity { - guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue - else { return .unknown } - return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown - } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } + var severity: AlarmSeverity { AlarmSeverity.from(raw) } var message: String? { raw["Message"]?.stringValue ?? raw["message"]?.stringValue } var time: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue } } @@ -266,7 +295,7 @@ struct CPEState: MongoDocument { let raw: JSONValue init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } init(raw: JSONValue) { self.raw = raw } - var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } var mac: String? { raw["_id"]?.stringValue } // IPv4 (DHCP) @@ -290,8 +319,6 @@ struct CPEState: MongoDocument { } var hasLease: Bool { ipv4 != nil || ipv6 != nil } - - private func nonEmpty(_ value: String?) -> String? { (value?.isEmpty == false) ? value : nil } } // MARK: - ONU-STATE detail accessors (status / firmware / UNI / traffic) @@ -391,8 +418,6 @@ extension OLTStateDoc { var switchName: String? { nonEmpty(raw["Switch"]?["System Name"]?.stringValue) } var switchPort: String? { nonEmpty(raw["Switch"]?["Port ID"]?.stringValue) } var switchAddress: String? { nonEmpty(raw["Switch"]?["IPv4 Address"]?.stringValue) } - - private func nonEmpty(_ s: String?) -> String? { (s?.isEmpty == false) ? s : nil } } // MARK: - OLT action payloads diff --git a/DeviceRepositories.swift b/DeviceRepositories.swift index 7453700..3e0f159 100644 --- a/DeviceRepositories.swift +++ b/DeviceRepositories.swift @@ -11,7 +11,7 @@ final class OLTRepository { } func allConfigs() async throws -> [OLTConfigDoc] { - try await client.getAll(.oltConfigs, idOf: { $0.id }) + try await client.getAll(.oltConfigs, idOf: { $0.documentId }) } /// Map of OLT id → operator name (`OLT.Name`), via projection. Skips names @@ -19,7 +19,7 @@ final class OLTRepository { func nameMap() async throws -> [String: String] { var query = APIQuery() query.projection = ["OLT.Name": 1] - let configs: [OLTConfigDoc] = try await client.getAll(.oltConfigs, baseQuery: query, idOf: { $0.id }) + let configs: [OLTConfigDoc] = try await client.getAll(.oltConfigs, baseQuery: query, idOf: { $0.documentId }) return configs.reduce(into: [:]) { map, config in if let name = config.name, !name.isEmpty, name != config.id { map[config.id] = name } } @@ -36,7 +36,7 @@ final class OLTRepository { } func allStates() async throws -> [OLTStateDoc] { - try await client.getAll(.oltStates, idOf: { $0.id }) + try await client.getAll(.oltStates, idOf: { $0.documentId }) } func state(id: String) async throws -> OLTStateDoc { @@ -88,6 +88,10 @@ final class OLTRepository { let _: JSONValue = try await client.action("PUT", .oltReset(id: id), as: JSONValue.self) } + // NOTE: the four methods below (disable-onu / broadcast-enable + their pending + // reads) are not yet wired to any screen. Their request shapes are documented + // but UNEXERCISED — verify against a live box before building UI on them. + /// `PUT /olts//disable-onu//` with body `{ "disable": Bool }` (unwrapped). func setONUDisabled(olt: String, onu: String, disabled: Bool) async throws { let _: JSONValue = try await client.send( @@ -127,7 +131,7 @@ final class ControllerRepository { } func allConfigs() async throws -> [ControllerConfigDoc] { - try await client.getAll(.controllerConfigs, idOf: { $0.id }) + try await client.getAll(.controllerConfigs, idOf: { $0.documentId }) } func config(id: String) async throws -> ControllerConfigDoc { diff --git a/Firmware.swift b/Firmware.swift index 2818f82..e88e5ea 100644 --- a/Firmware.swift +++ b/Firmware.swift @@ -6,7 +6,7 @@ struct FirmwareFile: MongoDocument { init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) } init(raw: JSONValue) { self.raw = raw } - var id: String { raw["_id"]?.stringValue ?? filename ?? UUID().uuidString } + var id: String { raw["_id"]?.stringValue ?? filename ?? raw.compactDescription } var filename: String? { raw["filename"]?.stringValue ?? raw["_id"]?.stringValue } var version: String? { raw["metadata"]?["Version"]?.stringValue } var manufacturer: String? { raw["metadata"]?["Compatible Manufacturer"]?.stringValue } @@ -14,14 +14,19 @@ struct FirmwareFile: MongoDocument { var sizeBytes: Double? { raw["length"]?.doubleValue } var uploadDate: String? { raw["uploadDate"]?.stringValue } - /// True when this image targets the given ONU: manufacturer matches the ONU - /// vendor AND the ONU's equipment id is one of the compatible models (exact, - /// case-insensitive — substring matching would wrongly pass e.g. an - /// "XGS2110" image for an "FTXGS2110B" ONU). + /// True only when this image POSITIVELY targets the given ONU: manufacturer + /// matches the ONU vendor (when both are known) AND the ONU's equipment id is + /// one of the compatible models (exact, case-insensitive — substring matching + /// would wrongly pass e.g. an "XGS2110" image for an "FTXGS2110B" ONU). + /// + /// An image with NO declared compatibility metadata (empty manufacturer/models), + /// or when the ONU's equipment id is unknown, is treated as NOT compatible — for + /// a destructive flash, an un-typed image must be chosen deliberately via the + /// sheet's "Show all" escape hatch rather than appearing as compatible-for-all. func isCompatible(vendor: String?, equipmentId: String?) -> Bool { if let vendor, let manufacturer, !manufacturer.isEmpty, manufacturer.caseInsensitiveCompare(vendor) != .orderedSame { return false } - guard let equipmentId, !models.isEmpty else { return true } + guard let equipmentId, !models.isEmpty else { return false } return models.contains { $0.caseInsensitiveCompare(equipmentId) == .orderedSame } } } diff --git a/Format.swift b/Format.swift index 448a93d..b4ea56f 100644 --- a/Format.swift +++ b/Format.swift @@ -5,21 +5,29 @@ enum Format { /// Bit-rate, e.g. 224229 → "224 kbps", 7216819 → "7.2 Mbps". static func bps(_ value: Double?) -> String { guard let value, value >= 0 else { return "—" } - let units = ["bps", "kbps", "Mbps", "Gbps", "Tbps"] - var x = value, i = 0 - while x >= 1000 && i < units.count - 1 { x /= 1000; i += 1 } - let digits = (i == 0 || x >= 100) ? "%.0f" : "%.1f" - return "\(String(format: digits, x)) \(units[i])" + return scaled(value, divisor: 1000, units: ["bps", "kbps", "Mbps", "Gbps", "Tbps"]) } /// Byte count, e.g. 7401438 → "7.1 MB". static func bytes(_ value: Double?) -> String { guard let value, value >= 0 else { return "—" } - let units = ["B", "KB", "MB", "GB", "TB", "PB"] + return scaled(value, divisor: 1024, units: ["B", "KB", "MB", "GB", "TB", "PB"]) + } + + private static func scaled(_ value: Double, divisor: Double, units: [String]) -> String { var x = value, i = 0 - while x >= 1024 && i < units.count - 1 { x /= 1024; i += 1 } - let digits = (i == 0 || x >= 100) ? "%.0f" : "%.1f" - return "\(String(format: digits, x)) \(units[i])" + while x >= divisor && i < units.count - 1 { x /= divisor; i += 1 } + // Round at display precision FIRST, then carry to the next unit if rounding + // reached the ceiling (999.95 kbps → "1000" should read "1.0 Mbps"). + var digits = (i == 0 || x >= 100) ? 0 : 1 + var rounded = (x * pow(10, Double(digits))).rounded() / pow(10, Double(digits)) + if rounded >= divisor && i < units.count - 1 { + i += 1 + rounded /= divisor + digits = rounded >= 100 ? 0 : 1 + rounded = (rounded * pow(10, Double(digits))).rounded() / pow(10, Double(digits)) + } + return "\(String(format: "%.\(digits)f", rounded)) \(units[i])" } /// Integer percent, e.g. 13 → "13%". diff --git a/LoginView.swift b/LoginView.swift index afd83e1..07ac644 100644 --- a/LoginView.swift +++ b/LoginView.swift @@ -7,6 +7,7 @@ struct LoginView: View { @State private var password = "" @State private var remember = true @State private var phase: LoadPhase = .idle + @State private var didPrefill = false var body: some View { NavigationStack { @@ -56,6 +57,11 @@ struct LoginView: View { } } .onAppear { + // Prefill from the Keychain ONCE — onAppear fires again when the + // user returns from Settings, and re-running this would wipe any + // credentials they'd already typed. + guard !didPrefill else { return } + didPrefill = true email = env.savedEmail ?? "" password = env.savedPassword ?? "" } diff --git a/MCMSEndpoint.swift b/MCMSEndpoint.swift index 0a330ec..7a3112b 100644 --- a/MCMSEndpoint.swift +++ b/MCMSEndpoint.swift @@ -5,6 +5,10 @@ import Foundation /// /// Reconcile exact paths with `openapi.json`; these mirror the MCMS 6.0 /// REST API Developer Guide. +// Some cases below are not yet wired to any repository/UI — they are documented +// path scaffolding for upcoming features, NOT validated against a live box: +// `databaseStatus`, `controllerStates`, `onuAlarmConfigs`/`onuAlarmConfig`, +// `onuAutomationConfigs`/`onuAutomationConfig`. Confirm them before building on them. enum MCMSEndpoint { // Auth / session / system case authenticate diff --git a/OLTDetailView.swift b/OLTDetailView.swift index 0b6b9e0..25e0dec 100644 --- a/OLTDetailView.swift +++ b/OLTDetailView.swift @@ -105,7 +105,7 @@ struct OLTDetailView: View { if olt.populatedBuckets.isEmpty { Section { Text("No ONUs registered on this OLT.").foregroundStyle(.secondary) } } else { - ForEach(olt.populatedBuckets, id: \.state) { bucket in + ForEach(olt.populatedBuckets, id: \.name) { bucket in Section { ForEach(bucket.ids, id: \.self) { onuId in NavigationLink { diff --git a/OLTDetailViewModel.swift b/OLTDetailViewModel.swift index 26a087a..c6ac5fb 100644 --- a/OLTDetailViewModel.swift +++ b/OLTDetailViewModel.swift @@ -13,17 +13,27 @@ final class OLTDetailViewModel { var isPerformingAction = false var actionMessage: String? + /// Bumped on every load; a load only commits if it's still the latest, so a + /// stale/overlapping request (pull-to-refresh racing the .task) can't clobber + /// newer results. + private var loadGeneration = 0 + init(connection: MCMSConnection, oltId: String) { self.connection = connection self.oltId = oltId } func load() async { + loadGeneration += 1 + let gen = loadGeneration phase = .loading do { - olt = try await connection.olt.state(id: oltId) + let fetched = try await connection.olt.state(id: oltId) + guard gen == loadGeneration, !Task.isCancelled else { return } + olt = fetched phase = .loaded } catch { + guard gen == loadGeneration, !Task.isCancelled else { return } phase = .failed((error as? APIError)?.localizedDescription ?? error.localizedDescription) } } diff --git a/OLTListViewModel.swift b/OLTListViewModel.swift index 4bc96bb..faadd4e 100644 --- a/OLTListViewModel.swift +++ b/OLTListViewModel.swift @@ -9,6 +9,8 @@ final class OLTListViewModel { var search = "" private let connection: MCMSConnection + /// Bumped per load so an overlapping/stale fetch can't commit over a newer one. + private var loadGeneration = 0 init(connection: MCMSConnection) { self.connection = connection } var filtered: [OLTStateDoc] { @@ -21,15 +23,20 @@ final class OLTListViewModel { } func load() async { + loadGeneration += 1 + let gen = loadGeneration phase = .loading do { let fetched = try await connection.olt.allStates() let names = (try? await connection.olt.nameMap()) ?? [:] - olts = fetched + let merged = fetched .map { olt in var olt = olt; olt.resolvedName = names[olt.id]; return olt } .sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } + guard gen == loadGeneration, !Task.isCancelled else { return } + olts = merged phase = .loaded } catch { + guard gen == loadGeneration, !Task.isCancelled else { return } phase = .failed((error as? APIError)?.localizedDescription ?? error.localizedDescription) } } diff --git a/ONUDetailView.swift b/ONUDetailView.swift index 66e2cd5..cb9e13f 100644 --- a/ONUDetailView.swift +++ b/ONUDetailView.swift @@ -129,12 +129,14 @@ struct ONUDetailView: View { @ViewBuilder private func opticalSection(_ s: ONUStateDoc) -> some View { Section { - opticalRow("RX optical", s.rxOpticalDBm, green: -28, warn: -30) - opticalRow("TX optical", s.txOpticalDBm, green: 3, warn: 3) + opticalRow("RX optical", s.rxOpticalDBm, + green: OpticalThreshold.rxGreenFloor, warn: OpticalThreshold.rxRedFloor) + opticalRow("TX optical", s.txOpticalDBm, + green: OpticalThreshold.txGreenFloor, warn: OpticalThreshold.txGreenFloor) } header: { Text("Live optical") } footer: { - Text("Green RX ≥ −30 dBm, TX ≥ 3 dBm (user guide §7).") + Text("Green RX ≥ −28 dBm, TX ≥ 3 dBm (user guide §7).") } Section("FEC error counters") { fecRow("ONU pre-FEC", s.onuPreFEC, isPost: false) diff --git a/ONUDetailViewModel.swift b/ONUDetailViewModel.swift index 90df5ab..9043f7b 100644 --- a/ONUDetailViewModel.swift +++ b/ONUDetailViewModel.swift @@ -15,6 +15,9 @@ enum ONUDetailTab: String, CaseIterable, Identifiable { final class ONUDetailViewModel { let onuId: String private let connection: MCMSConnection + /// Bumped per (re)load so a late/cancelled request — or one for a tab the user + /// has since left — can't commit its results over the current tab's data. + private var loadGeneration = 0 var tab: ONUDetailTab = .stats var phase: LoadPhase = .idle @@ -51,27 +54,52 @@ final class ONUDetailViewModel { } private func reloadCurrentTab() async { + loadGeneration += 1 + let gen = loadGeneration + let requested = tab + // Commit only if this is still the latest load AND the user is still on the + // tab we fetched for — otherwise drop the result. + func current() -> Bool { gen == loadGeneration && requested == tab && !Task.isCancelled } do { - switch tab { + switch requested { case .stats: // Live ONU-STATE carries the current optical/FEC levels (§3.12); // the /onus/stats/ time-series is a best-effort supplement. - liveState = try await connection.onu.state(id: onuId) - stats = (try? await connection.onu.stats(id: onuId, lastHours: 24)) ?? [] - case .config: config = try await connection.onu.config(id: onuId) - case .cpe: cpes = try await connection.onu.cpes(id: onuId) - case .alarms: alarms = try await connection.onu.alarmHistories( - query: queryForONU()).sorted { $0.severity.rank < $1.severity.rank } - case .logs: logs = try await connection.onu.logs(id: onuId, lastHours: 24) + let state = try await connection.onu.state(id: onuId) + let samples = (try? await connection.onu.stats(id: onuId, lastHours: 24)) ?? [] + guard current() else { return } + liveState = state + stats = samples + case .config: + let doc = try await connection.onu.config(id: onuId) + guard current() else { return } + config = doc + case .cpe: + let list = try await connection.onu.cpes(id: onuId) + guard current() else { return } + cpes = list + case .alarms: + let list = try await connection.onu.alarmHistories(query: queryForONU()) + .sorted { $0.severity.rank < $1.severity.rank } + guard current() else { return } + alarms = list + case .logs: + let list = try await connection.onu.logs(id: onuId, lastHours: 24) + guard current() else { return } + logs = list } phase = .loaded } catch { + guard current() else { return } phase = .failed((error as? APIError)?.localizedDescription ?? error.localizedDescription) } } - /// Filter alarm-history list to this ONU. Reconcile the key with the real - /// document shape; `_id` is a safe default for the per-ONU collection. + /// Filter the fleet-wide alarm-history list to this ONU. ⚠️ The `_id` key is + /// UNVERIFIED — if it's wrong the server ignores the filter and returns EVERY + /// ONU's alarms into this detail screen (no error). Confirm the real key (or + /// switch to the per-id `.onuAlarmHistory(id:)` endpoint) against openapi.json / + /// a live response before trusting this tab. private func queryForONU() -> APIQuery { var q = APIQuery() q.query = ["_id": onuId] diff --git a/ONUListViewModel.swift b/ONUListViewModel.swift index a67ed71..f69371c 100644 --- a/ONUListViewModel.swift +++ b/ONUListViewModel.swift @@ -12,6 +12,8 @@ final class ONUListViewModel { let stateFilter: ONULifecycleState? private let connection: MCMSConnection + /// Bumped per load so an overlapping/stale fetch can't commit over a newer one. + private var loadGeneration = 0 init(connection: MCMSConnection, stateFilter: ONULifecycleState? = nil) { self.connection = connection self.stateFilter = stateFilter @@ -32,13 +34,15 @@ final class ONUListViewModel { } func load() async { + loadGeneration += 1 + let gen = loadGeneration phase = .loading do { let fetched = try await connection.onu.allStates() // Registration lives on OLT-STATE, operator names on ONU-CFG. Both best-effort. let registration = (try? await connection.olt.registrationMap()) ?? [:] let names = (try? await connection.onu.nameMap()) ?? [:] - onus = fetched + let merged = fetched .map { onu in var onu = onu onu.resolvedLifecycle = registration[onu.id] @@ -46,8 +50,11 @@ final class ONUListViewModel { return onu } .sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } + guard gen == loadGeneration, !Task.isCancelled else { return } + onus = merged phase = .loaded } catch { + guard gen == loadGeneration, !Task.isCancelled else { return } phase = .failed((error as? APIError)?.localizedDescription ?? error.localizedDescription) } } diff --git a/ONURepository.swift b/ONURepository.swift index 09b3eaf..0f3b5e3 100644 --- a/ONURepository.swift +++ b/ONURepository.swift @@ -14,7 +14,7 @@ final class ONURepository { /// Fetch every ONU state, paging through with the `next` cursor. func allStates() async throws -> [ONUStateDoc] { - try await client.getAll(.onuStates, idOf: { $0.id }) + try await client.getAll(.onuStates, idOf: { $0.documentId }) } func configs(query: APIQuery = .empty) async throws -> [ONUConfigDoc] { @@ -26,7 +26,7 @@ final class ONURepository { func nameMap() async throws -> [String: String] { var query = APIQuery() query.projection = ["ONU.Name": 1] - let configs: [ONUConfigDoc] = try await client.getAll(.onuConfigs, baseQuery: query, idOf: { $0.id }) + let configs: [ONUConfigDoc] = try await client.getAll(.onuConfigs, baseQuery: query, idOf: { $0.documentId }) return configs.reduce(into: [:]) { map, config in if let name = config.name, !name.isEmpty { map[config.id] = name } } diff --git a/ServerTrustEvaluator.swift b/ServerTrustEvaluator.swift index abb4ddb..1f19c6b 100644 --- a/ServerTrustEvaluator.swift +++ b/ServerTrustEvaluator.swift @@ -84,16 +84,69 @@ final class ServerTrustEvaluator: NSObject, URLSessionDelegate { } } - /// Base64 of SHA-256 over the DER-encoded SubjectPublicKeyInfo's key bits. - /// (Capture once with `openssl` and store as a pin; this returns the raw - /// public-key hash for comparison.) + /// Base64 of SHA-256 over the DER-encoded **SubjectPublicKeyInfo**, i.e. the + /// standard public-key pin. `SecKeyCopyExternalRepresentation` returns only the + /// raw key (PKCS#1 `RSAPublicKey` for RSA, the `04||X||Y` point for EC) — NOT + /// the SPKI — so we prepend the algorithm-specific ASN.1 SPKI header before + /// hashing. The result matches the canonical capture: + /// + /// openssl s_client -connect host:443 /dev/null \ + /// | openssl x509 -pubkey -noout \ + /// | openssl pkey -pubin -outform der \ + /// | openssl dgst -sha256 -binary | base64 + /// + /// Returns nil for key types/sizes without a known header (caller fails closed). static func publicKeySHA256Base64(for certificate: SecCertificate) -> String? { guard let key = SecCertificateCopyKey(certificate), - let data = SecKeyCopyExternalRepresentation(key, nil) as Data? else { - return nil - } - return CryptoSHA256.base64(of: data) + let raw = SecKeyCopyExternalRepresentation(key, nil) as Data?, + let attrs = SecKeyCopyAttributes(key) as? [String: Any], + let keyType = attrs[kSecAttrKeyType as String] as? String, + let keySize = attrs[kSecAttrKeySizeInBits as String] as? Int, + let header = spkiHeader(keyType: keyType, keySizeInBits: keySize) + else { return nil } + var spki = Data(header) + spki.append(raw) + return CryptoSHA256.base64(of: spki) } + + /// ASN.1 SubjectPublicKeyInfo prefix for the raw key returned by + /// `SecKeyCopyExternalRepresentation`, keyed by (algorithm, key size). These + /// are the fixed, well-known DER headers; the raw key bits follow. + private static func spkiHeader(keyType: String, keySizeInBits: Int) -> [UInt8]? { + if keyType == (kSecAttrKeyTypeRSA as String) { + switch keySizeInBits { + case 2048: return rsa2048SPKIHeader + case 3072: return rsa3072SPKIHeader + case 4096: return rsa4096SPKIHeader + default: return nil + } + } + if keyType == (kSecAttrKeyTypeECSECPrimeRandom as String) { + switch keySizeInBits { + case 256: return ecP256SPKIHeader + case 384: return ecP384SPKIHeader + default: return nil + } + } + return nil + } + + private static let rsa2048SPKIHeader: [UInt8] = [ + 0x30, 0x82, 0x01, 0x22, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, 0x0f, 0x00] + private static let rsa3072SPKIHeader: [UInt8] = [ + 0x30, 0x82, 0x01, 0xa2, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, 0x8f, 0x00] + private static let rsa4096SPKIHeader: [UInt8] = [ + 0x30, 0x82, 0x02, 0x22, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x02, 0x0f, 0x00] + private static let ecP256SPKIHeader: [UInt8] = [ + 0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03, + 0x42, 0x00] + private static let ecP384SPKIHeader: [UInt8] = [ + 0x30, 0x76, 0x30, 0x10, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x22, 0x03, 0x62, 0x00] } /// Minimal SHA-256 wrapper using CryptoKit.