From 2a8672135dc1ccb6d35ab7f6504f9d80b2bd3bff Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 22:34:39 +1300 Subject: [PATCH 1/7] Read and replace a network as raw JSON (#23) UnifiNetwork models about 12 of a live network's ~63 fields and rest/networkconf/{id} is a full replacement, so a typed update would reset the rest. Add ListNetworksRawAsync, GetNetworkRawAsync (exactly one row or throw) and ReplaceNetworkRawAsync (body sent verbatim) over JsonObject, and NetworkDhcpDns to read or change only dhcpd_dns_enabled and dhcpd_dns_1..4. There is deliberately no UpdateNetworkAsync(UnifiNetwork). UnifiLegacySession gains an internal handler seam so unit tests can assert the bytes on the wire. --- README.md | 10 + src/UnifiSharp/Legacy/NetworkDhcpDns.cs | 110 +++++++++ src/UnifiSharp/Legacy/UnifiLegacyClient.cs | 55 +++++ src/UnifiSharp/Legacy/UnifiLegacySession.cs | 11 +- src/UnifiSharp/UnifiSharp.csproj | 5 + .../UnifiLegacyClientTests.cs | 223 ++++++++++++++++++ .../UnifiSharp.Tests/UnifiLegacyLiveTests.cs | 15 ++ 7 files changed, 428 insertions(+), 1 deletion(-) create mode 100644 src/UnifiSharp/Legacy/NetworkDhcpDns.cs diff --git a/README.md b/README.md index bc2b39a..40a9504 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,16 @@ nullable: on v2 a partial is not something you can send, so the type should not Static-DNS **wildcards are supported** and match arbitrary labels (`*.lab.example.com` answers `anything.lab.example.com`). +**Networks are the exception on the legacy side.** `UnifiNetwork` models about a dozen of +a live network's ~63 fields, so there is no typed network update. Change a network as raw +JSON instead: read it, change named keys, put the whole object back. + +```csharp +var net = await legacy.GetNetworkRawAsync(id); // throws unless exactly one row +var dns = NetworkDhcpDns.Read(net); // null = auto +await legacy.ReplaceNetworkRawAsync(id, NetworkDhcpDns.With(net, ["10.20.0.53", "10.20.0.54"])); +``` + ## Firewall policies (`UnifiSharp.Firewall`) Zone-based firewall policies (UniFi Network 9+) go through the **official** integration API, diff --git a/src/UnifiSharp/Legacy/NetworkDhcpDns.cs b/src/UnifiSharp/Legacy/NetworkDhcpDns.cs new file mode 100644 index 0000000..8ca9aa3 --- /dev/null +++ b/src/UnifiSharp/Legacy/NetworkDhcpDns.cs @@ -0,0 +1,110 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace UnifiSharp.Legacy; + +/// +/// The DHCP DNS servers a rest/networkconf object hands out, kept as API knowledge +/// in one place: dhcpd_dns_enabled plus four fixed slots dhcpd_dns_1..4. +/// Works on the raw object from so a +/// change touches these keys and nothing else. +/// +public static class NetworkDhcpDns +{ + public const int MaxServers = 4; + + private const string EnabledKey = "dhcpd_dns_enabled"; + + private static string SlotKey(int slot) => $"dhcpd_dns_{slot}"; + + /// + /// The servers handed out, slots 1..4 in order with blanks dropped; or null when + /// dhcpd_dns_enabled is false or absent ("auto": the gateway hands out itself, and + /// any leftover slot values are not live). Throws when a + /// key is present but not the shape this reads — unknown is reported, never guessed. + /// + public static IReadOnlyList? Read(JsonObject network) + { + ArgumentNullException.ThrowIfNull(network); + + // Absent is treated as false: without the flag the controller does not apply the + // slots, so "auto" is the live behaviour. (Present-but-not-a-bool throws below.) + if (!network.TryGetPropertyValue(EnabledKey, out var enabled)) + { + return null; + } + + if (enabled?.GetValueKind() is not (JsonValueKind.True or JsonValueKind.False)) + { + throw new UnifiLegacyException($"{EnabledKey} is {Describe(enabled)}, expected a boolean"); + } + + if (!enabled.GetValue()) + { + return null; + } + + var servers = new List(MaxServers); + for (var slot = 1; slot <= MaxServers; slot++) + { + // An absent slot carries no server, the same as a blank one; With always writes all four. + if (!network.TryGetPropertyValue(SlotKey(slot), out var value)) + { + continue; + } + + if (value?.GetValueKind() is not JsonValueKind.String) + { + throw new UnifiLegacyException($"{SlotKey(slot)} is {Describe(value)}, expected a string"); + } + + var server = value.GetValue(); + if (!string.IsNullOrWhiteSpace(server)) + { + servers.Add(server); + } + } + + // Enabled with every slot blank comes back as an empty list, not null: it is a + // different live state from "auto", and the caller should see it as such. + return servers; + } + + /// + /// A deep clone of with dhcpd_dns_enabled true, slots + /// 1..n set to in order and the remaining slots blanked to + /// "" (blanked, not removed: the PUT is a full replacement and an absent key is not + /// a cleared one). No other key is touched and the input is never mutated. + /// + /// 0 or more than servers, or a blank entry. + public static JsonObject With(JsonObject network, IReadOnlyList servers) + { + ArgumentNullException.ThrowIfNull(network); + ArgumentNullException.ThrowIfNull(servers); + + // Zero would mean "clear DNS", which no declaration should do by accident; reverting to + // auto is a separate, explicit operation this helper does not offer. + if (servers.Count is 0 or > MaxServers) + { + throw new ArgumentException( + $"A network hands out 1 to {MaxServers} DHCP DNS servers; got {servers.Count}.", nameof(servers)); + } + + if (servers.Any(string.IsNullOrWhiteSpace)) + { + throw new ArgumentException("A DHCP DNS server cannot be blank.", nameof(servers)); + } + + var copy = network.DeepClone().AsObject(); + copy[EnabledKey] = true; + for (var slot = 1; slot <= MaxServers; slot++) + { + copy[SlotKey(slot)] = slot <= servers.Count ? servers[slot - 1] : ""; + } + + return copy; + } + + private static string Describe(JsonNode? node) => + node is null ? "null" : $"{node.GetValueKind()} {node.ToJsonString()}"; +} diff --git a/src/UnifiSharp/Legacy/UnifiLegacyClient.cs b/src/UnifiSharp/Legacy/UnifiLegacyClient.cs index 2cde8e5..cc5f3f6 100644 --- a/src/UnifiSharp/Legacy/UnifiLegacyClient.cs +++ b/src/UnifiSharp/Legacy/UnifiLegacyClient.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; namespace UnifiSharp.Legacy; @@ -73,6 +74,48 @@ public Task CreateNetworkAsync(UnifiNetwork spec, CancellationToke public Task DeleteNetworkAsync(string id, CancellationToken ct = default) => DeleteAsync("networkconf", id, ct); + // NOTE there is no UpdateNetworkAsync(UnifiNetwork), deliberately. UnifiNetwork models + // ~12 of a live network's ~63 fields and rest/networkconf/{id} is a full replacement, + // so a typed update would silently reset the rest. Updates go through the raw trio + // below: read the whole object, change named keys (e.g. NetworkDhcpDns.With), put it back. + + /// Every network as the controller sent it — no field dropped, no number re-formatted. + public Task> ListNetworksRawAsync(CancellationToken ct = default) + => ListRawAsync("networkconf", ct); + + /// + /// One network as the controller sent it. Throws unless the controller returns exactly + /// one row — zero or several is not "the" network, and a write built on it would be a guess. + /// + public async Task GetNetworkRawAsync(string id, CancellationToken ct = default) + { + ArgumentException.ThrowIfNullOrEmpty(id); + using var resp = await _session.SendAsync( + HttpMethod.Get, $"rest/networkconf/{id}", content: null, ct).ConfigureAwait(false); + var rows = RawRows(await ReadEnvelopeAsync(resp, "networkconf", ct).ConfigureAwait(false), "networkconf"); + return rows.Count == 1 + ? rows[0] + : throw new UnifiLegacyException($"networkconf/{id}: expected exactly one network, got {rows.Count}"); + } + + /// + /// Replace a network wholesale with , sent verbatim. Pass an object + /// read by and changed in place of named keys only; the + /// caller's object is not modified. Returns the controller's echo of the network, or a copy + /// of when the controller answers ok with no row — re-read to confirm. + /// + public async Task ReplaceNetworkRawAsync(string id, JsonObject body, CancellationToken ct = default) + { + ArgumentException.ThrowIfNullOrEmpty(id); + ArgumentNullException.ThrowIfNull(body); + // ToJsonString writes parsed numbers from their original text, so 1.50 stays 1.50. + using var resp = await _session.SendAsync( + HttpMethod.Put, $"rest/networkconf/{id}", UnifiLegacySession.JsonBody(body.ToJsonString()), ct).ConfigureAwait(false); + var rows = RawRows(await ReadEnvelopeAsync(resp, "networkconf", ct).ConfigureAwait(false), "networkconf"); + // Same echo-on-empty as UpdateAsync, but cloned so the caller's object stays theirs. + return rows.Count > 0 ? rows[0] : body.DeepClone().AsObject(); + } + // ── Known clients / DHCP reservations (rest/user) ───────────────────────── // NOTE there is no Delete here, deliberately. Removing a known client discards // its name and history along with the reservation; retiring a reservation is @@ -171,6 +214,18 @@ private async Task> ListAsync(string resource, CancellationT return (await ReadEnvelopeAsync(resp, resource, ct).ConfigureAwait(false)).Data; } + private async Task> ListRawAsync(string resource, CancellationToken ct) + { + using var resp = await _session.SendAsync(HttpMethod.Get, $"rest/{resource}", content: null, ct).ConfigureAwait(false); + return RawRows(await ReadEnvelopeAsync(resp, resource, ct).ConfigureAwait(false), resource); + } + + // A null row is a shape we don't recognise, not an empty object — refuse it rather + // than hand back something a caller could write over a real network. + private static IReadOnlyList RawRows(UnifiLegacyEnvelope env, string resource) => + env.Data.Select(r => r ?? throw new UnifiLegacyException($"{resource}: response row is null")) + .ToList(); + private async Task CreateAsync(string resource, T spec, CancellationToken ct) { var json = JsonSerializer.Serialize(spec, SerializerOptions); diff --git a/src/UnifiSharp/Legacy/UnifiLegacySession.cs b/src/UnifiSharp/Legacy/UnifiLegacySession.cs index bafe3f2..9ae918f 100644 --- a/src/UnifiSharp/Legacy/UnifiLegacySession.cs +++ b/src/UnifiSharp/Legacy/UnifiLegacySession.cs @@ -28,12 +28,21 @@ public sealed class UnifiLegacySession : IDisposable private bool _loggedIn; public UnifiLegacySession(UnifiLegacyOptions options) + : this(options, CreateHandler(options)) { } + + // Test seam: lets unit tests assert the exact bytes on the wire without a controller. + internal UnifiLegacySession(UnifiLegacyOptions options, HttpMessageHandler handler) { ArgumentNullException.ThrowIfNull(options); options.Validate(); _options = options; _loginUrl = new Uri(_options.ControllerRoot, "/api/auth/login"); + _http = new HttpClient(handler); + } + private static HttpClientHandler CreateHandler(UnifiLegacyOptions options) + { + ArgumentNullException.ThrowIfNull(options); var handler = new HttpClientHandler { CookieContainer = new CookieContainer(), UseCookies = true }; if (!options.VerifyTls) { @@ -41,7 +50,7 @@ public UnifiLegacySession(UnifiLegacyOptions options) HttpClientHandler.DangerousAcceptAnyServerCertificateValidator; } - _http = new HttpClient(handler); + return handler; } /// diff --git a/src/UnifiSharp/UnifiSharp.csproj b/src/UnifiSharp/UnifiSharp.csproj index a38358a..397b81b 100644 --- a/src/UnifiSharp/UnifiSharp.csproj +++ b/src/UnifiSharp/UnifiSharp.csproj @@ -23,4 +23,9 @@ + + + + + diff --git a/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs index 34eeb26..413e036 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs @@ -1,9 +1,13 @@ +using System.Net; using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; using UnifiSharp.Legacy; namespace UnifiSharp.Tests; +#pragma warning disable CS0618 // UnifiLegacyClient is intentionally obsolete (ADR-0003) + // Pure unit tests for the legacy write adapter — no controller required. public class UnifiLegacyClientTests { @@ -298,4 +302,223 @@ public void User_envelope_deserializes_reservation_fields() Assert.Equal("10.10.135.221", u.FixedIp); Assert.Equal("shell.devops.chrison.internal", u.LocalDnsRecord); } + + // ---- rest/networkconf as raw JSON — read-modify-write without the lossy model ---- + + // Synthetic, not captured: every JSON kind the controller can send, plus keys no model + // knows about. A number with a trailing zero and an exponent proves number TEXT survives, + // not just the value. + private const string RawNetwork = + """ + { "_id": "net1", "name": "Consumer", "purpose": "corporate", "vlan": 1010, + "vlan_enabled": true, "dhcpd_dns_enabled": true, + "dhcpd_dns_1": "10.20.0.1", "dhcpd_dns_2": "", "dhcpd_dns_3": "", "dhcpd_dns_4": "", + "dhcpd_leasetime": 86400, "ratio": 1.50, "big": 1e3, "nothing": null, + "nat_outbound_ip_addresses": [], "ipv6_ra_preferred_lifetime": 14400, + "dhcp_relay": { "servers": ["10.0.0.1", null], "enabled": false }, + "some_future_field": { "nested": [1, { "deep": true }] } } + """; + + private static string Envelope(string dataArray, string rc = "ok", string? msg = null) => + $$"""{ "meta": { "rc": "{{rc}}"{{(msg is null ? "" : $", \"msg\": \"{msg}\"")}} }, "data": {{dataArray}} }"""; + + [Fact] + public async Task GetNetworkRaw_gets_the_one_row_with_every_key_intact() + { + var handler = new FakeHandler(Envelope($"[{RawNetwork}]")); + using var client = handler.Client(); + + var net = await client.GetNetworkRawAsync("net1"); + + var req = Assert.Single(handler.Requests); + Assert.Equal(HttpMethod.Get, req.Method); + Assert.EndsWith("/api/s/default/rest/networkconf/net1", req.Uri.AbsolutePath); + Assert.True(JsonNode.DeepEquals(JsonNode.Parse(RawNetwork), net)); + Assert.Equal("1.50", net["ratio"]!.ToJsonString()); // number text, not a re-formatted double + Assert.True(net.ContainsKey("nothing")); // an explicit null is a key, not an absence + } + + [Theory] + [InlineData("[]")] + [InlineData("""[{ "_id": "a" }, { "_id": "b" }]""")] + [InlineData("[null]")] + public async Task GetNetworkRaw_throws_unless_exactly_one_row(string data) + { + // Zero rows or two is not "the" network — a read-modify-write on a guess is the bug. + using var client = new FakeHandler(Envelope(data)).Client(); + await Assert.ThrowsAsync(() => client.GetNetworkRawAsync("net1")); + } + + [Fact] + public async Task ListNetworksRaw_returns_every_row_raw() + { + var handler = new FakeHandler(Envelope($$"""[{{RawNetwork}}, { "_id": "net2", "x": 1 }]""")); + using var client = handler.Client(); + + var nets = await client.ListNetworksRawAsync(); + + Assert.EndsWith("/rest/networkconf", Assert.Single(handler.Requests).Uri.AbsolutePath); + Assert.Equal(2, nets.Count); + Assert.True(JsonNode.DeepEquals(JsonNode.Parse(RawNetwork), nets[0])); + } + + [Fact] + public async Task ReplaceNetworkRaw_puts_the_body_verbatim() + { + var handler = new FakeHandler(Envelope($"[{RawNetwork}]")); + using var client = handler.Client(); + var read = await client.GetNetworkRawAsync("net1"); + + await client.ReplaceNetworkRawAsync("net1", read); + + var put = handler.Requests[1]; + Assert.Equal(HttpMethod.Put, put.Method); + Assert.EndsWith("/rest/networkconf/net1", put.Uri.AbsolutePath); + var sent = JsonNode.Parse(put.Body!)!.AsObject(); + Assert.True(JsonNode.DeepEquals(JsonNode.Parse(RawNetwork), sent)); + // Byte-for-byte on numbers: a double round-trip would print 1.5 and 1000. + Assert.Contains("\"ratio\":1.50", put.Body); + Assert.Contains("\"big\":1e3", put.Body); + Assert.Contains("\"vlan\":1010", put.Body); + Assert.Contains("\"nothing\":null", put.Body); + } + + [Fact] + public async Task ReplaceNetworkRaw_does_not_mutate_the_callers_object() + { + using var client = new FakeHandler(Envelope("[]")).Client(); + var body = JsonNode.Parse(RawNetwork)!.AsObject(); + + await client.ReplaceNetworkRawAsync("net1", body); + + Assert.True(JsonNode.DeepEquals(JsonNode.Parse(RawNetwork), body)); + } + + [Theory] + [InlineData("GET")] + [InlineData("PUT")] + public async Task Raw_network_error_envelope_surfaces_the_api_msg(string verb) + { + // The legacy API answers validation failures with HTTP 400 and rc:error. + using var client = new FakeHandler( + Envelope("[]", rc: "error", msg: "api.err.InvalidPayload"), HttpStatusCode.BadRequest).Client(); + + var ex = await Assert.ThrowsAsync(() => verb == "GET" + ? client.GetNetworkRawAsync("net1") + : client.ReplaceNetworkRawAsync("net1", JsonNode.Parse(RawNetwork)!.AsObject())); + + Assert.Contains("api.err.InvalidPayload", ex.Message); + } + + // ---- NetworkDhcpDns — the dhcpd_dns_N naming, kept out of every caller ---- + + private static JsonObject Raw() => JsonNode.Parse(RawNetwork)!.AsObject(); + + [Fact] + public void DhcpDns_With_changes_only_the_dns_keys_and_leaves_the_input_alone() + { + var input = Raw(); + + var output = NetworkDhcpDns.With(input, ["10.20.0.53", "10.20.0.54"]); + + Assert.True(JsonNode.DeepEquals(JsonNode.Parse(RawNetwork), input)); // input untouched + Assert.NotSame(input, output); + + var changed = input.Select(kv => kv.Key) + .Union(output.Select(kv => kv.Key)) + .Where(k => !JsonNode.DeepEquals(input[k], output[k]) || input.ContainsKey(k) != output.ContainsKey(k)) + .ToHashSet(); + // dhcpd_dns_enabled was already true and slots 3-4 already blank, so only 1 and 2 move here; + // the invariant is that nothing OUTSIDE the DNS key set moves. + Assert.Subset( + new HashSet { "dhcpd_dns_enabled", "dhcpd_dns_1", "dhcpd_dns_2", "dhcpd_dns_3", "dhcpd_dns_4" }, + changed); + Assert.Equal("10.20.0.53", (string?)output["dhcpd_dns_1"]); + Assert.Equal("10.20.0.54", (string?)output["dhcpd_dns_2"]); + } + + [Fact] + public void DhcpDns_With_enables_dns_and_writes_all_four_slots_from_auto() + { + var input = JsonNode.Parse("""{ "_id": "n", "dhcpd_dns_enabled": false, "other": 7 }""")!.AsObject(); + + var output = NetworkDhcpDns.With(input, ["10.20.0.53"]); + + Assert.True((bool)output["dhcpd_dns_enabled"]!); + Assert.Equal("10.20.0.53", (string?)output["dhcpd_dns_1"]); + Assert.Equal("", (string?)output["dhcpd_dns_2"]); + Assert.Equal("", (string?)output["dhcpd_dns_3"]); + Assert.Equal("", (string?)output["dhcpd_dns_4"]); + Assert.Equal(7, (int)output["other"]!); + Assert.False((bool)input["dhcpd_dns_enabled"]!); + Assert.False(input.ContainsKey("dhcpd_dns_1")); + } + + [Fact] + public void DhcpDns_With_blanks_a_dropped_slot_rather_than_removing_it() + { + // Removing the key would leave the controller's old value in place on a full PUT. + var input = NetworkDhcpDns.With(Raw(), ["1.1.1.1", "8.8.8.8", "9.9.9.9"]); + + var output = NetworkDhcpDns.With(input, ["1.1.1.1", "8.8.8.8"]); + + Assert.True(output.ContainsKey("dhcpd_dns_3")); + Assert.Equal("", (string?)output["dhcpd_dns_3"]); + Assert.Equal("9.9.9.9", (string?)input["dhcpd_dns_3"]); + } + + [Theory] + [InlineData(0)] + [InlineData(5)] + public void DhcpDns_With_rejects_an_empty_or_oversized_list(int count) + { + // Empty would mean "clear the DNS" — never something a declaration should do silently. + var servers = Enumerable.Range(1, count).Select(i => $"10.0.0.{i}").ToList(); + Assert.Throws(() => NetworkDhcpDns.With(Raw(), servers)); + } + + [Theory] + [InlineData("""{ "dhcpd_dns_enabled": false, "dhcpd_dns_1": "1.1.1.1" }""")] + [InlineData("""{ "dhcpd_dns_1": "1.1.1.1" }""")] + public void DhcpDns_Read_is_null_when_dns_is_automatic(string json) + { + // Disabled means the gateway hands itself out; stale slot values are not the live list. + Assert.Null(NetworkDhcpDns.Read(JsonNode.Parse(json)!.AsObject())); + } + + [Fact] + public void DhcpDns_Read_returns_the_slots_in_order_skipping_blanks() + { + var net = JsonNode.Parse( + """{ "dhcpd_dns_enabled": true, "dhcpd_dns_1": "10.20.0.53", "dhcpd_dns_2": "", "dhcpd_dns_3": "10.20.0.54" }""")! + .AsObject(); + + Assert.Equal(["10.20.0.53", "10.20.0.54"], NetworkDhcpDns.Read(net)); + } + + [Theory] + [InlineData("""{ "dhcpd_dns_enabled": "yes" }""")] + [InlineData("""{ "dhcpd_dns_enabled": true, "dhcpd_dns_1": 10 }""")] + public void DhcpDns_Read_throws_on_a_shape_it_does_not_recognise(string json) + { + // A field present but shaped differently is unknown, not "auto" and not empty. + Assert.Throws(() => NetworkDhcpDns.Read(JsonNode.Parse(json)!.AsObject())); + } + + // Records each request (body read eagerly — the client disposes it) and answers with a + // canned envelope. API-key options, so the session never makes a login request. + private sealed class FakeHandler(string responseBody, HttpStatusCode status = HttpStatusCode.OK) : HttpMessageHandler + { + public List<(HttpMethod Method, Uri Uri, string? Body)> Requests { get; } = []; + + public UnifiLegacyClient Client() => + new(new UnifiLegacySession(KeyOptions(), this), ownsSession: true); + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken ct) + { + var body = request.Content is null ? null : await request.Content.ReadAsStringAsync(ct); + Requests.Add((request.Method, request.RequestUri!, body)); + return new HttpResponseMessage(status) { Content = new StringContent(responseBody) }; + } + } } diff --git a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs index ad048a8..87340dd 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs @@ -88,6 +88,21 @@ public async Task Static_dns_reads_from_the_v2_site_api() Assert.False(string.IsNullOrEmpty(r.RecordType)); }); } + + [SkippableFact] + public async Task Network_reads_raw_and_its_dhcp_dns_is_readable() + { + Skip.If(_fixture.Client is null, "No UniFi env — skipping live read test."); + + // Read only: proves the raw path and the dhcpd_dns_N naming against a real controller. + var networks = await _fixture.Client!.ListNetworksRawAsync(); + Assert.NotEmpty(networks); + var first = networks[0]; + var net = await _fixture.Client.GetNetworkRawAsync((string)first["_id"]!); + + Assert.Equal((string?)first["_id"], (string?)net["_id"]); + _ = NetworkDhcpDns.Read(net); // null (auto) or a list — either way, no throw + } } public class UnifiLegacyLiveTests : IClassFixture From 49b944357bb69a1fdf0a2753e94180abf1e183df Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 22:41:31 +1300 Subject: [PATCH 2/7] Guard raw network replace against a mismatched body, unsafe id and null data --- src/UnifiSharp/Legacy/NetworkDhcpDns.cs | 40 +++++++--- src/UnifiSharp/Legacy/UnifiLegacyClient.cs | 37 ++++++++- .../UnifiLegacyClientTests.cs | 76 +++++++++++++++++++ 3 files changed, 137 insertions(+), 16 deletions(-) diff --git a/src/UnifiSharp/Legacy/NetworkDhcpDns.cs b/src/UnifiSharp/Legacy/NetworkDhcpDns.cs index 8ca9aa3..8646d3f 100644 --- a/src/UnifiSharp/Legacy/NetworkDhcpDns.cs +++ b/src/UnifiSharp/Legacy/NetworkDhcpDns.cs @@ -34,12 +34,7 @@ public static class NetworkDhcpDns return null; } - if (enabled?.GetValueKind() is not (JsonValueKind.True or JsonValueKind.False)) - { - throw new UnifiLegacyException($"{EnabledKey} is {Describe(enabled)}, expected a boolean"); - } - - if (!enabled.GetValue()) + if (!ReadEnabled(enabled)) { return null; } @@ -53,12 +48,7 @@ public static class NetworkDhcpDns continue; } - if (value?.GetValueKind() is not JsonValueKind.String) - { - throw new UnifiLegacyException($"{SlotKey(slot)} is {Describe(value)}, expected a string"); - } - - var server = value.GetValue(); + var server = ReadSlot(slot, value); if (!string.IsNullOrWhiteSpace(server)) { servers.Add(server); @@ -77,6 +67,7 @@ public static class NetworkDhcpDns /// a cleared one). No other key is touched and the input is never mutated. /// /// 0 or more than servers, or a blank entry. + /// A DNS key is present but not the shape accepts. public static JsonObject With(JsonObject network, IReadOnlyList servers) { ArgumentNullException.ThrowIfNull(network); @@ -95,6 +86,21 @@ public static JsonObject With(JsonObject network, IReadOnlyList servers) throw new ArgumentException("A DHCP DNS server cannot be blank.", nameof(servers)); } + // Same checks as Read, slots included even when disabled: a key shaped differently means + // the API drifted, and overwriting it would hide that rather than report it. + if (network.TryGetPropertyValue(EnabledKey, out var enabled)) + { + ReadEnabled(enabled); + } + + for (var slot = 1; slot <= MaxServers; slot++) + { + if (network.TryGetPropertyValue(SlotKey(slot), out var value)) + { + ReadSlot(slot, value); + } + } + var copy = network.DeepClone().AsObject(); copy[EnabledKey] = true; for (var slot = 1; slot <= MaxServers; slot++) @@ -105,6 +111,16 @@ public static JsonObject With(JsonObject network, IReadOnlyList servers) return copy; } + private static bool ReadEnabled(JsonNode? enabled) => + enabled?.GetValueKind() is JsonValueKind.True or JsonValueKind.False + ? enabled.GetValue() + : throw new UnifiLegacyException($"{EnabledKey} is {Describe(enabled)}, expected a boolean"); + + private static string ReadSlot(int slot, JsonNode? value) => + value?.GetValueKind() is JsonValueKind.String + ? value.GetValue() + : throw new UnifiLegacyException($"{SlotKey(slot)} is {Describe(value)}, expected a string"); + private static string Describe(JsonNode? node) => node is null ? "null" : $"{node.GetValueKind()} {node.ToJsonString()}"; } diff --git a/src/UnifiSharp/Legacy/UnifiLegacyClient.cs b/src/UnifiSharp/Legacy/UnifiLegacyClient.cs index cc5f3f6..3f7f977 100644 --- a/src/UnifiSharp/Legacy/UnifiLegacyClient.cs +++ b/src/UnifiSharp/Legacy/UnifiLegacyClient.cs @@ -89,7 +89,7 @@ public Task> ListNetworksRawAsync(CancellationToken ct /// public async Task GetNetworkRawAsync(string id, CancellationToken ct = default) { - ArgumentException.ThrowIfNullOrEmpty(id); + ThrowUnlessPlainSegment(id); using var resp = await _session.SendAsync( HttpMethod.Get, $"rest/networkconf/{id}", content: null, ct).ConfigureAwait(false); var rows = RawRows(await ReadEnvelopeAsync(resp, "networkconf", ct).ConfigureAwait(false), "networkconf"); @@ -103,11 +103,27 @@ public async Task GetNetworkRawAsync(string id, CancellationToken ct /// read by and changed in place of named keys only; the /// caller's object is not modified. Returns the controller's echo of the network, or a copy /// of when the controller answers ok with no row — re-read to confirm. + /// "Verbatim" is per value, not per byte, for strings: numbers keep their original text + /// and keys their order, but a string is re-escaped on write, so "caf\u00e9" read in + /// may go out as "café" (same value, different bytes). /// + /// is not a plain path segment, or + /// 's _id is missing, not a string, or not . public async Task ReplaceNetworkRawAsync(string id, JsonObject body, CancellationToken ct = default) { - ArgumentException.ThrowIfNullOrEmpty(id); + ThrowUnlessPlainSegment(id); ArgumentNullException.ThrowIfNull(body); + // The PUT replaces the network at {id} with whatever body says, so a body read from + // another network would silently overwrite this one with it. Missing counts as mismatch. + var bodyId = body.TryGetPropertyValue("_id", out var idNode) && idNode?.GetValueKind() is JsonValueKind.String + ? idNode.GetValue() + : null; + if (bodyId != id) + { + throw new ArgumentException( + $"body._id is {(bodyId is null ? "missing or not a string" : $"'{bodyId}'")}, expected '{id}'.", nameof(body)); + } + // ToJsonString writes parsed numbers from their original text, so 1.50 stays 1.50. using var resp = await _session.SendAsync( HttpMethod.Put, $"rest/networkconf/{id}", UnifiLegacySession.JsonBody(body.ToJsonString()), ct).ConfigureAwait(false); @@ -221,11 +237,24 @@ private async Task> ListRawAsync(string resource, Canc } // A null row is a shape we don't recognise, not an empty object — refuse it rather - // than hand back something a caller could write over a real network. + // than hand back something a caller could write over a real network. Likewise "data":null: + // the deserializer leaves Data null despite its type, and that is not "no rows". private static IReadOnlyList RawRows(UnifiLegacyEnvelope env, string resource) => - env.Data.Select(r => r ?? throw new UnifiLegacyException($"{resource}: response row is null")) + (env.Data ?? throw new UnifiLegacyException($"{resource}: response data is null")) + .Select(r => r ?? throw new UnifiLegacyException($"{resource}: response row is null")) .ToList(); + // The id is interpolated into the path, so "a/../x" or "a?b" would address something else. + // Controller ids are hex ObjectIds; anything percent-encoding would change is refused, not escaped. + private static void ThrowUnlessPlainSegment(string id, [System.Runtime.CompilerServices.CallerArgumentExpression(nameof(id))] string? paramName = null) + { + ArgumentException.ThrowIfNullOrEmpty(id, paramName); + if (id is "." or ".." || Uri.EscapeDataString(id) != id) + { + throw new ArgumentException($"'{id}' is not a plain path segment.", paramName); + } + } + private async Task CreateAsync(string resource, T spec, CancellationToken ct) { var json = JsonSerializer.Serialize(spec, SerializerOptions); diff --git a/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs index 413e036..196067d 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyClientTests.cs @@ -410,6 +410,70 @@ public async Task Raw_network_error_envelope_surfaces_the_api_msg(string verb) Assert.Contains("api.err.InvalidPayload", ex.Message); } + [Theory] + [InlineData("""{ "_id": "net2", "name": "Other" }""")] + [InlineData("""{ "name": "no id" }""")] + [InlineData("""{ "_id": null }""")] + [InlineData("""{ "_id": 1 }""")] + public async Task ReplaceNetworkRaw_refuses_a_body_that_is_not_the_named_network(string json) + { + // A full replacement of net1 with net2's body would silently turn one network into another. + var handler = new FakeHandler(Envelope("[]")); + using var client = handler.Client(); + + await Assert.ThrowsAsync( + () => client.ReplaceNetworkRawAsync("net1", JsonNode.Parse(json)!.AsObject())); + Assert.Empty(handler.Requests); + } + + [Theory] + [InlineData("a/../x")] + [InlineData("..")] + [InlineData(".")] + [InlineData("a?b=c")] + [InlineData("a#b")] + [InlineData("a%2Fb")] + [InlineData("a b")] + public async Task Raw_network_rejects_an_id_that_is_not_a_plain_path_segment(string id) + { + var handler = new FakeHandler(Envelope($"[{RawNetwork}]")); + using var client = handler.Client(); + var body = JsonNode.Parse(RawNetwork)!.AsObject(); + body["_id"] = id; + + await Assert.ThrowsAsync(() => client.GetNetworkRawAsync(id)); + await Assert.ThrowsAsync(() => client.ReplaceNetworkRawAsync(id, body)); + Assert.Empty(handler.Requests); + } + + [Theory] + [InlineData("""{ "meta": { "rc": "ok" }, "data": null }""")] + public async Task Raw_network_reports_a_null_data_array_as_an_api_shape_error(string response) + { + using var client = new FakeHandler(response).Client(); + + await Assert.ThrowsAsync(() => client.GetNetworkRawAsync("net1")); + await Assert.ThrowsAsync(() => client.ListNetworksRawAsync()); + await Assert.ThrowsAsync( + () => client.ReplaceNetworkRawAsync("net1", JsonNode.Parse(RawNetwork)!.AsObject())); + } + + [Fact] + public async Task ReplaceNetworkRaw_sends_With_output_byte_for_byte() + { + // DeepEquals compares values, so it cannot see number text, key order or a dropped + // null; only the literal wire body can. Expected is RawNetwork minified by hand, with + // the two DNS slots changed in place. + var handler = new FakeHandler(Envelope("[]")); + using var client = handler.Client(); + + await client.ReplaceNetworkRawAsync("net1", NetworkDhcpDns.With(Raw(), ["10.20.0.53", "10.20.0.54"])); + + const string expected = + """{"_id":"net1","name":"Consumer","purpose":"corporate","vlan":1010,"vlan_enabled":true,"dhcpd_dns_enabled":true,"dhcpd_dns_1":"10.20.0.53","dhcpd_dns_2":"10.20.0.54","dhcpd_dns_3":"","dhcpd_dns_4":"","dhcpd_leasetime":86400,"ratio":1.50,"big":1e3,"nothing":null,"nat_outbound_ip_addresses":[],"ipv6_ra_preferred_lifetime":14400,"dhcp_relay":{"servers":["10.0.0.1",null],"enabled":false},"some_future_field":{"nested":[1,{"deep":true}]}}"""; + Assert.Equal(expected, Assert.Single(handler.Requests).Body); + } + // ---- NetworkDhcpDns — the dhcpd_dns_N naming, kept out of every caller ---- private static JsonObject Raw() => JsonNode.Parse(RawNetwork)!.AsObject(); @@ -505,6 +569,18 @@ public void DhcpDns_Read_throws_on_a_shape_it_does_not_recognise(string json) Assert.Throws(() => NetworkDhcpDns.Read(JsonNode.Parse(json)!.AsObject())); } + [Theory] + [InlineData("""{ "dhcpd_dns_enabled": "yes" }""")] + [InlineData("""{ "dhcpd_dns_enabled": null }""")] + [InlineData("""{ "dhcpd_dns_enabled": true, "dhcpd_dns_1": 10 }""")] + [InlineData("""{ "dhcpd_dns_enabled": false, "dhcpd_dns_2": { "ip": "1.1.1.1" } }""")] + public void DhcpDns_With_throws_on_a_shape_it_does_not_recognise(string json) + { + // Overwriting a malformed key would paper over the very drift Read refuses to guess at. + Assert.Throws( + () => NetworkDhcpDns.With(JsonNode.Parse(json)!.AsObject(), ["10.20.0.53"])); + } + // Records each request (body read eagerly — the client disposes it) and answers with a // canned envelope. API-key options, so the session never makes a login request. private sealed class FakeHandler(string responseBody, HttpStatusCode status = HttpStatusCode.OK) : HttpMessageHandler From eb23e710081a30af189e027e6eba0ac895f145c3 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 22:56:10 +1300 Subject: [PATCH 3/7] Add a local-only live test proving a raw DHCP DNS write is reversible Creates a scratch network, writes DHCP DNS through NetworkDhcpDns.With, asserts only the DNS keys moved, restores the saved object and asserts every key matches its JSON text, and checks no other network changed. Skips without UNIFI_LEGACY_BASE_URL + UNIFI_USERNAME + UNIFI_PASSWORD and refuses any host but localhost / 127.0.0.1 / ::1. --- README.md | 3 + .../UnifiSharp.Tests/UnifiLegacyLiveTests.cs | 148 ++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/README.md b/README.md index 40a9504..45b73e9 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,9 @@ With `UNIFI_LOCAL_HOST` alone the site URL is derived as > port-forward, a firewall group and a VLAN) are gated on *session* auth on purpose, > so a shell holding only an API key can never point them at a production gateway. > Read-only live checks (`UnifiLegacyReadOnlyLiveTests`) run in either mode. +> `UnifiLegacyNetworkRawLiveTests` (a raw DHCP-DNS write on a scratch network, then a +> byte-for-byte restore) is stricter still: it needs `UNIFI_LEGACY_BASE_URL` + +> `UNIFI_USERNAME` + `UNIFI_PASSWORD` and **refuses** any host but localhost / 127.0.0.1 / ::1. ### Two surfaces, two sets of rules diff --git a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs index 87340dd..8f0937b 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json.Nodes; using UnifiSharp.Legacy; using Xunit; @@ -183,3 +184,150 @@ public async Task Network_create_delete_roundtrip() await client.DeleteNetworkAsync(created.Id!); } } + +/// +/// Proves a field-scoped raw network write is reversible: setting DHCP DNS touches only the +/// DNS keys, and putting back the saved object restores the network exactly. +/// Stricter gate than : this PUTs over a whole network, +/// so it runs only against a controller on this machine. Missing env skips; an env pointing +/// anywhere else is refused (fails) before any login, never quietly skipped. +/// +public class UnifiLegacyNetworkRawLiveTests +{ + private static readonly string[] Servers = ["10.20.0.53", "10.20.0.54"]; + + private static readonly HashSet DnsKeys = + ["dhcpd_dns_enabled", "dhcpd_dns_1", "dhcpd_dns_2", "dhcpd_dns_3", "dhcpd_dns_4"]; + + // Keys the controller rewrites on its own between reads, excluded from every comparison + // below. Empty until a run shows one — add each by name with the reason it moves, so a + // real difference can never hide here unannounced. + private static readonly HashSet VolatileKeys = []; + + [SkippableFact] + public async Task Network_dhcp_dns_write_is_reversible_byte_for_byte() + { + var baseUrl = Environment.GetEnvironmentVariable("UNIFI_LEGACY_BASE_URL"); + Skip.If( + string.IsNullOrEmpty(baseUrl) + || string.IsNullOrEmpty(Environment.GetEnvironmentVariable("UNIFI_USERNAME")) + || string.IsNullOrEmpty(Environment.GetEnvironmentVariable("UNIFI_PASSWORD")), + "UNIFI_LEGACY_BASE_URL + UNIFI_USERNAME + UNIFI_PASSWORD not all set — skipping local write test."); + + // Check the URL the client will actually use, not just the variable: TryFromEnvironment + // is what resolves it, and it is the one that has to be local. + var options = UnifiLegacyOptions.TryFromEnvironment() + ?? throw new InvalidOperationException("UNIFI_* env is set but did not resolve to legacy options."); + RefuseUnlessLocal(new Uri(baseUrl!)); + RefuseUnlessLocal(options.BaseUrl); + + using var client = new UnifiLegacyClient(options); + + var before = await client.ListNetworksRawAsync(); + var (vlan, octet) = PickUnusedVlanAndSubnet(before); + + var created = await client.CreateNetworkAsync(new UnifiNetwork + { + Name = $"scratch-dhcpdns-{Guid.NewGuid().ToString("N")[..8]}", + Purpose = "corporate", + VlanEnabled = true, + Vlan = vlan.ToString(), + IpSubnet = $"10.232.{octet}.1/24", + DhcpdEnabled = true, + DhcpdStart = $"10.232.{octet}.6", + DhcpdStop = $"10.232.{octet}.254", + }); + Assert.False(string.IsNullOrEmpty(created.Id)); + var id = created.Id!; + + try + { + // Kept as text so nothing below can mutate the baseline. + var saved = (await client.GetNetworkRawAsync(id)).ToJsonString(); + var savedObj = Parse(saved); + + // UnifiNetwork cannot express dhcpd_dns_enabled, so "DHCP DNS off" is the controller's + // create default — asserted, not assumed. + Assert.True(NetworkDhcpDns.Read(savedObj) is null, + $"precondition: a new network should have DHCP DNS off (auto); dhcpd_dns_enabled is {savedObj["dhcpd_dns_enabled"]?.ToJsonString() ?? "absent"}"); + + await client.ReplaceNetworkRawAsync(id, NetworkDhcpDns.With(Parse(saved), Servers)); + var changed = await client.GetNetworkRawAsync(id); + + Assert.Equal(Servers, NetworkDhcpDns.Read(changed)); + AssertSameKeys(savedObj, changed, ignore: DnsKeys, what: "after the DNS write, a non-DNS key"); + Assert.Equal((string?)savedObj["_id"], (string?)changed["_id"]); + Assert.Equal((string?)savedObj["site_id"], (string?)changed["site_id"]); + + await client.ReplaceNetworkRawAsync(id, Parse(saved)); + var restored = await client.GetNetworkRawAsync(id); + + AssertSameKeys(savedObj, restored, ignore: [], what: "after restoring the saved object, key"); + } + finally + { + await client.DeleteNetworkAsync(id); + } + + // Every other network, whole: the scratch network is gone and nothing else moved. + var after = await client.ListNetworksRawAsync(); + var beforeById = ById(before); + var afterById = ById(after); + Assert.Equal(beforeById.Keys.Order(), afterById.Keys.Order()); + foreach (var (otherId, was) in beforeById) + { + AssertSameKeys(was, afterById[otherId], ignore: [], what: $"network {otherId} ('{was["name"]}') changed: key"); + } + } + + private static void RefuseUnlessLocal(Uri url) + { + // Uri.Host lower-cases names and brackets IPv6, so ::1 arrives as "[::1]". + if (url.Host is not ("localhost" or "127.0.0.1" or "[::1]")) + { + throw new InvalidOperationException( + $"Refusing to write networks on {url.Host}: this test only runs against a controller on localhost / 127.0.0.1 / ::1."); + } + } + + private static (int Vlan, int Octet) PickUnusedVlanAndSubnet(IReadOnlyList networks) + { + // node.ToString() gives the bare text for both shapes the vlan arrives in (number or string). + var vlans = networks.Select(n => n["vlan"]?.ToString()).ToHashSet(); + var subnets = networks.Select(n => n["ip_subnet"]?.ToString() ?? "").ToList(); + for (var attempt = 0; attempt < 20; attempt++) + { + var vlan = Random.Shared.Next(3900, 4000); // inside UniFi's valid 1–4009 + var octet = Random.Shared.Next(0, 256); + if (!vlans.Contains(vlan.ToString()) && !subnets.Any(s => s.StartsWith($"10.232.{octet}.", StringComparison.Ordinal))) + { + return (vlan, octet); + } + } + + throw new InvalidOperationException("No free VLAN 3900–3999 / 10.232.x.0/24 found for the scratch network."); + } + + // Compared as each value's JSON text, not semantically: the claim is byte-for-byte, so 1 + // vs 1.0 or a re-ordered nested object is a difference. The union of keys is walked, so a + // key the controller added or dropped is reported too. + private static void AssertSameKeys(JsonObject expected, JsonObject actual, IReadOnlyCollection ignore, string what) + { + foreach (var key in expected.Select(p => p.Key).Union(actual.Select(p => p.Key))) + { + if (ignore.Contains(key) || VolatileKeys.Contains(key)) + { + continue; + } + + var want = expected.TryGetPropertyValue(key, out var e) ? e?.ToJsonString() ?? "null" : ""; + var got = actual.TryGetPropertyValue(key, out var a) ? a?.ToJsonString() ?? "null" : ""; + Assert.True(want == got, $"{what} '{key}' differs: expected {want}, got {got}"); + } + } + + private static Dictionary ById(IReadOnlyList networks) => + networks.ToDictionary(n => (string?)n["_id"] ?? throw new InvalidOperationException($"network without a string _id: {n.ToJsonString()}")); + + private static JsonObject Parse(string json) => JsonNode.Parse(json)!.AsObject(); +} From 82f128e7ee62aeaa51efa33077aa025bdea17420 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 22:57:16 +1300 Subject: [PATCH 4/7] Make the live DNS write test start from an explicit baseline --- .../UnifiSharp.Tests/UnifiLegacyLiveTests.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs index 8f0937b..1893a1f 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs @@ -242,14 +242,24 @@ public async Task Network_dhcp_dns_write_is_reversible_byte_for_byte() try { + // Found by this test: the controller treats a PUT as a MERGE for omitted keys. A body + // without dhcpd_dns_enabled does not switch it off again, so "restore" cannot be done by + // replaying a body that lacks the key. Real LAN networks carry the key explicitly + // (dhcpd_dns_enabled=false), so the baseline is made explicit here, as on the live console. + var fresh = await client.GetNetworkRawAsync(id); + Assert.True(NetworkDhcpDns.Read(fresh) is null, + $"precondition: a new network should have DHCP DNS off (auto); dhcpd_dns_enabled is {fresh["dhcpd_dns_enabled"]?.ToJsonString() ?? "absent"}"); + var explicitOff = Parse(fresh.ToJsonString()); + explicitOff["dhcpd_dns_enabled"] = false; + for (var slot = 1; slot <= NetworkDhcpDns.MaxServers; slot++) + explicitOff[$"dhcpd_dns_{slot}"] = ""; // as on Consumer: all four keys present, blank when unused + await client.ReplaceNetworkRawAsync(id, explicitOff); + // Kept as text so nothing below can mutate the baseline. var saved = (await client.GetNetworkRawAsync(id)).ToJsonString(); var savedObj = Parse(saved); - - // UnifiNetwork cannot express dhcpd_dns_enabled, so "DHCP DNS off" is the controller's - // create default — asserted, not assumed. - Assert.True(NetworkDhcpDns.Read(savedObj) is null, - $"precondition: a new network should have DHCP DNS off (auto); dhcpd_dns_enabled is {savedObj["dhcpd_dns_enabled"]?.ToJsonString() ?? "absent"}"); + Assert.True(NetworkDhcpDns.Read(savedObj) is null && (bool?)savedObj["dhcpd_dns_enabled"] == false, + "precondition: the baseline carries dhcpd_dns_enabled=false explicitly"); await client.ReplaceNetworkRawAsync(id, NetworkDhcpDns.With(Parse(saved), Servers)); var changed = await client.GetNetworkRawAsync(id); From 16f3942d56ac9e37fa3c169da3e2d736d435da2e Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 23:11:33 +1300 Subject: [PATCH 5/7] Start the UniFi OS test controller from the tests with Testcontainers The legacy write tests now share one container per run, started from the compose.yml image and settings, set up through /api/setup and removed at the end. They read no UNIFI_* variable, so they cannot reach a real console. They skip when no container runtime is reachable, or with UNIFISHARP_SKIP_CONTAINER_TESTS=1. Refs #25 --- .../UnifiSharp.Tests/UnifiContainerFixture.cs | 306 ++++++++++++++++++ .../UnifiSharp.Tests/UnifiLegacyLiveTests.cs | 92 ++---- .../UnifiSharp.Tests/UnifiSharp.Tests.csproj | 1 + 3 files changed, 338 insertions(+), 61 deletions(-) create mode 100644 tests/UnifiSharp.Tests/UnifiContainerFixture.cs diff --git a/tests/UnifiSharp.Tests/UnifiContainerFixture.cs b/tests/UnifiSharp.Tests/UnifiContainerFixture.cs new file mode 100644 index 0000000..052cad3 --- /dev/null +++ b/tests/UnifiSharp.Tests/UnifiContainerFixture.cs @@ -0,0 +1,306 @@ +using System.Diagnostics; +using System.Net; +using System.Net.Http.Json; +using DotNet.Testcontainers.Builders; +using DotNet.Testcontainers.Configurations; +using DotNet.Testcontainers.Containers; +using UnifiSharp.Legacy; +using Xunit; + +namespace UnifiSharp.Tests; + +#pragma warning disable CS0618 // UnifiLegacyClient is intentionally obsolete (ADR-0003) + +/// +/// A throwaway UniFi OS Server, started by Testcontainers for the legacy WRITE tests and +/// removed when the run ends. Same image and settings as +/// .containers/unifi/compose.yml in the Homelab repo, plus the first-run setup that +/// bootstrap.sh performs. +/// The client only ever points at this fixture's own container: the URL is built +/// from the container's mapped port, never read from the environment. That is the whole +/// safety argument for the write tests — no UNIFI_* variable can redirect them at a +/// real console. +/// Skips (with the reason in ) when no container runtime is +/// reachable or UNIFISHARP_SKIP_CONTAINER_TESTS=1. Any other startup failure — +/// a timeout, a setup that never completes — is a failure, not a skip. +/// +public sealed class UnifiContainerFixture : IAsyncLifetime +{ + public const string Image = "hieutq/unifi-os-server:latest"; + public const string Username = "labadmin"; + public const string Password = "LabAdmin!2026"; // 8-64 chars (UniFi OS policy); throwaway, container-only + public const string Label = "dev.chrison.unifisharp.tests"; + + // First boot downloads Ubiquiti's installer into the fresh container, which takes + // minutes on a good line. Generous on purpose; UNIFISHARP_CONTAINER_STARTUP_MINUTES + // overrides it for slow links (or shortens it when the image is warm). + private static readonly TimeSpan DefaultStartupTimeout = TimeSpan.FromMinutes(20); + + private IContainer? _container; + + /// Why the write tests are skipped; null when the container is up. + public string? SkipReason { get; private set; } + + /// The logged-in client, shared by every test in the collection. + public UnifiLegacyClient Client => + _client ?? throw new InvalidOperationException($"UniFi container is not available: {SkipReason}"); + private UnifiLegacyClient? _client; + + /// How long the container took to become usable — reported, not asserted. + public TimeSpan StartupTime { get; private set; } + + public async Task InitializeAsync() + { + if (Environment.GetEnvironmentVariable("UNIFISHARP_SKIP_CONTAINER_TESTS") == "1") + { + SkipReason = "UNIFISHARP_SKIP_CONTAINER_TESTS=1 — skipping the UniFi container write tests."; + return; + } + + var timeout = StartupTimeout(); + using var cts = new CancellationTokenSource(timeout); + var stopwatch = Stopwatch.StartNew(); + + try + { + _container = new ContainerBuilder() + .WithImage(Image) + // Our own name and label, never the compose container's `unifi-os-server`: a + // manually bootstrapped one may be running and is not ours to reuse or stop. + .WithName($"unifisharp-tests-{Guid.NewGuid().ToString("N")[..8]}") + .WithLabel(Label, "legacy-write") + // Only the GUI/API port, on a free random host port. 8080/3478/10003 are for + // real device adoption and would collide with the compose container. + .WithPortBinding(443, assignRandomHostPort: true) + .WithBindMount("/sys/fs/cgroup", "/sys/fs/cgroup", AccessMode.ReadWrite) + .WithCreateParameterModifier(p => + { + // compose.yml's `cgroup: host`, cap set and tmpfs mounts: UniFi OS Server runs + // systemd plus ~10 internal services. No /unifi volume — the data lives in the + // container layer and goes with it. + var host = p.HostConfig ?? throw new InvalidOperationException("Testcontainers built no HostConfig to modify."); + host.CgroupnsMode = "host"; + host.CapDrop = ["ALL"]; + host.CapAdd = + [ + "SYS_ADMIN", "NET_ADMIN", "NET_RAW", "NET_BIND_SERVICE", "DAC_OVERRIDE", + "DAC_READ_SEARCH", "FOWNER", "CHOWN", "SETUID", "SETGID", "KILL", + "SYS_CHROOT", "SYS_PTRACE", "SYS_RESOURCE", "AUDIT_WRITE", "MKNOD", + ]; + host.Tmpfs = new Dictionary + { + ["/run"] = "exec", + ["/run/lock"] = "", + ["/tmp"] = "exec", + ["/var/lib/journal"] = "", + ["/var/opt/unifi/tmp"] = "size=64m", + }; + }) + .Build(); + await _container.StartAsync(cts.Token); + } + catch (DockerUnavailableException ex) + { + // Thrown before anything is created, from Build() or StartAsync(). + if (_container is not null) + { + await _container.DisposeAsync(); + _container = null; + } + + SkipReason = $"No container runtime reachable — skipping the UniFi container write tests. ({ex.Message.Split('\n')[0]})"; + return; + } + + try + { + var root = new UriBuilder("https", _container.Hostname, _container.GetMappedPublicPort(443)).Uri; + await Bootstrap.RunAsync(root, cts.Token); + + var session = new UnifiLegacySession(new UnifiLegacyOptions + { + BaseUrl = new Uri(root, "/proxy/network/api/s/default"), + Username = Username, + Password = Password, + VerifyTls = false, // the container serves a self-signed cert + }); + _client = new UnifiLegacyClient(session, ownsSession: true); + + // One login for the whole run, so the tests never add to the login rate limit. + await Bootstrap.WithLoginBackoffAsync(() => session.LoginAsync(cts.Token), cts.Token); + } + catch (Exception ex) + { + // Don't count on xunit disposing a fixture whose initialisation failed: remove the + // container here, then surface the failure. + var name = _container.Name; + await DisposeAsync(); + _client = null; + _container = null; + if (ex is OperationCanceledException && cts.IsCancellationRequested) + { + throw new TimeoutException( + $"UniFi OS container {name} was not ready within {timeout} " + + "(first boot downloads the installer; raise UNIFISHARP_CONTAINER_STARTUP_MINUTES on a slow link).", ex); + } + + throw; + } + + StartupTime = stopwatch.Elapsed; + } + + public async Task DisposeAsync() + { + _client?.Dispose(); + if (_container is not null) + { + await _container.DisposeAsync(); + } + } + + private static TimeSpan StartupTimeout() + { + var raw = Environment.GetEnvironmentVariable("UNIFISHARP_CONTAINER_STARTUP_MINUTES"); + if (string.IsNullOrEmpty(raw)) + { + return DefaultStartupTimeout; + } + + // A typo here should fail loudly, not quietly fall back to the default. + return int.TryParse(raw, out var minutes) && minutes > 0 + ? TimeSpan.FromMinutes(minutes) + : throw new InvalidOperationException($"UNIFISHARP_CONTAINER_STARTUP_MINUTES must be a positive whole number of minutes, got '{raw}'."); + } + + /// + /// bootstrap.sh's steps 1-3, against one controller root: wait for UniFi OS, run first-run + /// setup, then wait until a legacy-API read answers 200. + /// + private static class Bootstrap + { + private static readonly TimeSpan Poll = TimeSpan.FromSeconds(5); + + public static async Task RunAsync(Uri root, CancellationToken ct) + { + using var http = new HttpClient(new HttpClientHandler + { + // Throwaway container, self-signed cert. + ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator, + UseCookies = false, + }) + { Timeout = TimeSpan.FromSeconds(15) }; + + // 1) UniFi OS answers /api/system with a deviceState once it is up. + await UntilAsync(async () => + { + var body = await http.GetStringAsync(new Uri(root, "/api/system"), ct); + return body.Contains("\"deviceState\"", StringComparison.Ordinal); + }, ct); + + // 2) First-run setup. A fresh container is never set up, but the setup endpoint + // answers 400/500 to an empty body only once it accepts input; until then it + // is not ready. + await UntilAsync(async () => + { + using var probe = await http.PostAsJsonAsync(new Uri(root, "/api/setup"), new { }, ct); + return probe.StatusCode is HttpStatusCode.BadRequest or HttpStatusCode.InternalServerError; + }, ct); + try + { + using var _ = await http.PostAsJsonAsync(new Uri(root, "/api/setup"), + new { name = "UnifiSharp test controller", username = Username, password = Password }, ct); + } + catch (Exception ex) when (ex is HttpRequestException + || (ex is TaskCanceledException && !ct.IsCancellationRequested)) + { + // The connection often resets, or hangs until HttpClient.Timeout, while services + // restart after setup — expected. + // Whether setup took is decided by the login below, not by this response. + } + + // 3) The admin can log in and the legacy API answers. Its own token, not the + // shared session's: this is a readiness probe, retried until it holds. + await UntilAsync(async () => + { + using var login = await http.PostAsJsonAsync(new Uri(root, "/api/auth/login"), + new { username = Username, password = Password }, ct); + if (login.StatusCode == HttpStatusCode.TooManyRequests) + { + await Task.Delay(TimeSpan.FromSeconds(30), ct); + return false; + } + + if (!login.IsSuccessStatusCode) + { + return false; + } + + using var read = new HttpRequestMessage(HttpMethod.Get, new Uri(root, "/proxy/network/api/s/default/rest/portforward")); + foreach (var cookie in login.Headers.TryGetValues("Set-Cookie", out var c) ? c : []) + { + read.Headers.TryAddWithoutValidation("Cookie", cookie.Split(';')[0]); + } + + using var resp = await http.SendAsync(read, ct); + return resp.StatusCode == HttpStatusCode.OK; + }, ct); + } + + /// + /// Retry a session login while the controller answers 429. The library surfaces a + /// failed login as carrying the status in its + /// message ("login failed (429): …"); anything other than 429 is rethrown. + /// + public static async Task WithLoginBackoffAsync(Func login, CancellationToken ct) + { + for (var delay = TimeSpan.FromSeconds(15); ; delay *= 2) + { + try + { + await login(); + return; + } + catch (UnifiLegacyException ex) when (ex.Message.StartsWith("login failed (429)", StringComparison.Ordinal)) + { + await Task.Delay(delay, ct); + } + } + } + + private static async Task UntilAsync(Func> ready, CancellationToken ct) + { + while (true) + { + try + { + if (await ready()) + { + return; + } + } + catch (HttpRequestException) + { + // Not listening yet, or restarting a service — keep polling. + } + catch (TaskCanceledException) when (!ct.IsCancellationRequested) + { + // A single request timed out (HttpClient.Timeout), not the overall deadline. + } + + await Task.Delay(Poll, ct); + } + } + } +} + +/// +/// Every legacy WRITE test class joins this collection: one container for the run, and the +/// tests in it run one at a time, so a test comparing "every other network" never sees +/// another test's scratch objects. +/// +[CollectionDefinition(Name)] +public sealed class UnifiContainerCollection : ICollectionFixture +{ + public const string Name = "UniFi OS container"; +} diff --git a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs index 1893a1f..2046fa2 100644 --- a/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs +++ b/tests/UnifiSharp.Tests/UnifiLegacyLiveTests.cs @@ -4,30 +4,18 @@ namespace UnifiSharp.Tests; -// Live write round-trips against a UniFi controller — the .containers/unifi test -// container (run ./bootstrap.sh, then `set -a && . ./credentials.env`). Every test -// cleans up what it creates (add-only on the controller). +// Live tests against a UniFi controller, in two groups with different targets: // -// ⚠ These tests CREATE AND DELETE real objects — port-forwards, firewall groups and -// a VLAN. They are gated on SESSION auth (UNIFI_USERNAME + UNIFI_PASSWORD) on purpose, -// which is the only mode the test container offers. That gate is load-bearing: once -// UnifiLegacyOptions learned API-key auth, a plain `secrets.env` (UNIFI_API_KEY + -// UNIFI_LOCAL_HOST) started satisfying TryFromEnvironment — so keying these off -// TryFromEnvironment alone would silently point them at the live home gateway and -// create a VLAN on it. Read-only live checks use LegacyReadOnlyFixture instead. -// -// One shared session for the whole class (IClassFixture): the controller -// rate-limits logins (HTTP 429), so we log in once and reuse the cookie. +// • WRITE tests (UnifiLegacyLiveTests, UnifiLegacyNetworkRawLiveTests) CREATE AND DELETE +// real objects — port-forwards, firewall groups, VLANs, and a whole-network PUT. They +// run only against the throwaway container UnifiContainerFixture starts, and read no +// UNIFI_* variable at all: once UnifiLegacyOptions learned API-key auth, a plain +// `secrets.env` started satisfying TryFromEnvironment, so an env-gated write test could +// silently point at the live home gateway and create a VLAN on it. No env, no risk. +// • READ-ONLY tests (UnifiLegacyReadOnlyLiveTests) use LegacyReadOnlyFixture, which does +// take its controller from the environment — safe to point at a real gateway. #pragma warning disable CS0618 // UnifiLegacyClient is intentionally obsolete (ADR-0003) -public sealed class LegacyContainerFixture : IDisposable -{ - public UnifiLegacyClient? Client { get; } = - UnifiLegacyOptions.TryFromEnvironment() is { UsesApiKey: false } o ? new UnifiLegacyClient(o) : null; - - public void Dispose() => Client?.Dispose(); -} - /// /// Any configured controller, in either auth mode — for checks that only READ. /// Safe to point at a real gateway. @@ -106,18 +94,19 @@ public async Task Network_reads_raw_and_its_dhcp_dns_is_readable() } } -public class UnifiLegacyLiveTests : IClassFixture +[Collection(UnifiContainerCollection.Name)] +public class UnifiLegacyLiveTests { - private readonly UnifiLegacyClient? _client; - public UnifiLegacyLiveTests(LegacyContainerFixture fixture) => _client = fixture.Client; + private readonly UnifiContainerFixture _fixture; + public UnifiLegacyLiveTests(UnifiContainerFixture fixture) => _fixture = fixture; [SkippableFact] public async Task PortForward_create_list_delete_roundtrip() { - var client = _client; - Skip.If(client is null, "No UNIFI_LEGACY_* env — skipping live legacy write test."); + Skip.If(_fixture.SkipReason is not null, _fixture.SkipReason); + var client = _fixture.Client; - var created = await client!.CreatePortForwardAsync(new UnifiPortForward + var created = await client.CreatePortForwardAsync(new UnifiPortForward { Name = "unifisharp-test-pf", Enabled = true, @@ -150,10 +139,10 @@ public async Task PortForward_create_list_delete_roundtrip() [SkippableFact] public async Task FirewallGroup_create_delete_roundtrip() { - var client = _client; - Skip.If(client is null, "No UNIFI_LEGACY_* env — skipping live legacy write test."); + Skip.If(_fixture.SkipReason is not null, _fixture.SkipReason); + var client = _fixture.Client; - var created = await client!.CreateFirewallGroupAsync(new UnifiFirewallGroup + var created = await client.CreateFirewallGroupAsync(new UnifiFirewallGroup { Name = "unifisharp-test-grp", GroupType = "port-group", @@ -167,10 +156,10 @@ public async Task FirewallGroup_create_delete_roundtrip() [SkippableFact] public async Task Network_create_delete_roundtrip() { - var client = _client; - Skip.If(client is null, "No UNIFI_LEGACY_* env — skipping live legacy write test."); + Skip.If(_fixture.SkipReason is not null, _fixture.SkipReason); + var client = _fixture.Client; - var created = await client!.CreateNetworkAsync(new UnifiNetwork + var created = await client.CreateNetworkAsync(new UnifiNetwork { Name = "unifisharp-test-vlan", Purpose = "corporate", @@ -188,12 +177,16 @@ public async Task Network_create_delete_roundtrip() /// /// Proves a field-scoped raw network write is reversible: setting DHCP DNS touches only the /// DNS keys, and putting back the saved object restores the network exactly. -/// Stricter gate than : this PUTs over a whole network, -/// so it runs only against a controller on this machine. Missing env skips; an env pointing -/// anywhere else is refused (fails) before any login, never quietly skipped. +/// Runs against 's container, in the same collection as +/// the other write tests so none of them runs alongside it: it asserts every OTHER network is +/// untouched afterwards. /// +[Collection(UnifiContainerCollection.Name)] public class UnifiLegacyNetworkRawLiveTests { + private readonly UnifiContainerFixture _fixture; + public UnifiLegacyNetworkRawLiveTests(UnifiContainerFixture fixture) => _fixture = fixture; + private static readonly string[] Servers = ["10.20.0.53", "10.20.0.54"]; private static readonly HashSet DnsKeys = @@ -207,21 +200,8 @@ public class UnifiLegacyNetworkRawLiveTests [SkippableFact] public async Task Network_dhcp_dns_write_is_reversible_byte_for_byte() { - var baseUrl = Environment.GetEnvironmentVariable("UNIFI_LEGACY_BASE_URL"); - Skip.If( - string.IsNullOrEmpty(baseUrl) - || string.IsNullOrEmpty(Environment.GetEnvironmentVariable("UNIFI_USERNAME")) - || string.IsNullOrEmpty(Environment.GetEnvironmentVariable("UNIFI_PASSWORD")), - "UNIFI_LEGACY_BASE_URL + UNIFI_USERNAME + UNIFI_PASSWORD not all set — skipping local write test."); - - // Check the URL the client will actually use, not just the variable: TryFromEnvironment - // is what resolves it, and it is the one that has to be local. - var options = UnifiLegacyOptions.TryFromEnvironment() - ?? throw new InvalidOperationException("UNIFI_* env is set but did not resolve to legacy options."); - RefuseUnlessLocal(new Uri(baseUrl!)); - RefuseUnlessLocal(options.BaseUrl); - - using var client = new UnifiLegacyClient(options); + Skip.If(_fixture.SkipReason is not null, _fixture.SkipReason); + var client = _fixture.Client; var before = await client.ListNetworksRawAsync(); var (vlan, octet) = PickUnusedVlanAndSubnet(before); @@ -290,16 +270,6 @@ public async Task Network_dhcp_dns_write_is_reversible_byte_for_byte() } } - private static void RefuseUnlessLocal(Uri url) - { - // Uri.Host lower-cases names and brackets IPv6, so ::1 arrives as "[::1]". - if (url.Host is not ("localhost" or "127.0.0.1" or "[::1]")) - { - throw new InvalidOperationException( - $"Refusing to write networks on {url.Host}: this test only runs against a controller on localhost / 127.0.0.1 / ::1."); - } - } - private static (int Vlan, int Octet) PickUnusedVlanAndSubnet(IReadOnlyList networks) { // node.ToString() gives the bare text for both shapes the vlan arrives in (number or string). diff --git a/tests/UnifiSharp.Tests/UnifiSharp.Tests.csproj b/tests/UnifiSharp.Tests/UnifiSharp.Tests.csproj index a071f52..9403388 100644 --- a/tests/UnifiSharp.Tests/UnifiSharp.Tests.csproj +++ b/tests/UnifiSharp.Tests/UnifiSharp.Tests.csproj @@ -10,6 +10,7 @@ + From c83c48d304a42be8dc74fbcef94aea0ff036a380 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 23:11:33 +1300 Subject: [PATCH 6/7] Document the container-backed legacy write tests Refs #25 --- README.md | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 45b73e9..b724843 100644 --- a/README.md +++ b/README.md @@ -122,13 +122,38 @@ unifisharp clients # connected clients per site (name, type, ip, connectedAt With `UNIFI_LOCAL_HOST` alone the site URL is derived as `https:///proxy/network/api/s/default`. -> The **destructive** live tests (`UnifiLegacyLiveTests` — they create and delete a -> port-forward, a firewall group and a VLAN) are gated on *session* auth on purpose, -> so a shell holding only an API key can never point them at a production gateway. -> Read-only live checks (`UnifiLegacyReadOnlyLiveTests`) run in either mode. -> `UnifiLegacyNetworkRawLiveTests` (a raw DHCP-DNS write on a scratch network, then a -> byte-for-byte restore) is stricter still: it needs `UNIFI_LEGACY_BASE_URL` + -> `UNIFI_USERNAME` + `UNIFI_PASSWORD` and **refuses** any host but localhost / 127.0.0.1 / ::1. +> The **destructive** live tests never read `UNIFI_*`; they run only against a throwaway +> container the tests start themselves (below). Read-only live checks +> (`UnifiLegacyReadOnlyLiveTests`) take their controller from the environment, in either mode. + +### Write tests run against their own container + +`UnifiLegacyLiveTests` (create and delete a port-forward, a firewall group and a VLAN) and +`UnifiLegacyNetworkRawLiveTests` (a raw DHCP-DNS write on a scratch network, then a +byte-for-byte restore) share `UnifiContainerFixture`: Testcontainers starts a UniFi OS +Server (`hieutq/unifi-os-server`, the image and settings of the Homelab repo's +`.containers/unifi/compose.yml`), runs first-run setup, logs in once, and removes the +container when the run ends. The URL comes from the container's mapped port, so these tests +cannot reach a real console whatever the environment says. + +```sh +dotnet test # starts the container when a runtime is reachable +UNIFISHARP_SKIP_CONTAINER_TESTS=1 dotnet test # skip them on purpose +UNIFISHARP_CONTAINER_STARTUP_MINUTES=40 dotnet test # slow link (default 20) +``` + +- **Needs** a Docker-compatible runtime that allows `cgroup: host`, the `SYS_ADMIN` set of + capabilities and a read-write `/sys/fs/cgroup` bind (OrbStack and Docker Desktop do). + With no runtime reachable the tests **skip** and say why; any other startup failure fails. +- **First boot is slow:** every fresh container downloads Ubiquiti's installer (minutes). +- It gets its own name (`unifisharp-tests-…`), a random host port and only port 443, so it + runs beside a manually bootstrapped `unifi-os-server` container without touching it. +- Testcontainers finds the runtime through `DOCKER_HOST`, `~/.testcontainers.properties` or + the Docker context. A stale `tc.host` / `docker.host` in that properties file (say, from + an uninstalled Testcontainers Desktop) makes the tests skip; + `DOCKER_HOST=unix://$HOME/.orbstack/run/docker.sock` overrides it for OrbStack. +- **CI is untested.** A runner needs a privileged-enough Docker daemon and network access to + Ubiquiti's download servers; budget for the installer download on every run. ### Two surfaces, two sets of rules @@ -190,5 +215,5 @@ await fw.CreateAsync(spec, names); Read client + `discover` + `unifisharp` CLI building from the 10.4.57 spec. Live read tests skip without `UNIFI_*` — run them against the [`.containers/unifi`](https://github.com/Chrison-dev/Homelab/tree/main/.containers/unifi) -test controller (never the live network). **Next:** the legacy write adapter +test controller (never the live network). Legacy write tests start their own container. **Next:** the legacy write adapter (firewall rules / port profiles) once verified against the container. From e35a4e53f351b1711a37d8d7f837ac5d1657d41b Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Fri, 23 Oct 2026 23:19:36 +1300 Subject: [PATCH 7/7] Harden the UniFi container fixture: login backoff, pinned image, local daemon only The readiness probe now logs in once and reuses the session, re-logging in only on 401 and backing off on 429; the shared session's logins (initial and 401 re-login) go through the same backoff. The image is pinned to 5.0.8 by digest. A remote Docker endpoint or non-loopback host override fails the run instead of starting a SYS_ADMIN container elsewhere. A refused first-run setup is surfaced, and cancellation during start reports the last probe status. UNIFISHARP_SKIP_CONTAINER_TESTS accepts 1/true/yes and rejects typos. --- README.md | 11 +- .../UnifiSharp.Tests/UnifiContainerFixture.cs | 285 +++++++++++++----- .../UnifiContainerFixtureTests.cs | 99 ++++++ 3 files changed, 324 insertions(+), 71 deletions(-) create mode 100644 tests/UnifiSharp.Tests/UnifiContainerFixtureTests.cs diff --git a/README.md b/README.md index b724843..afd2116 100644 --- a/README.md +++ b/README.md @@ -131,20 +131,25 @@ With `UNIFI_LOCAL_HOST` alone the site URL is derived as `UnifiLegacyLiveTests` (create and delete a port-forward, a firewall group and a VLAN) and `UnifiLegacyNetworkRawLiveTests` (a raw DHCP-DNS write on a scratch network, then a byte-for-byte restore) share `UnifiContainerFixture`: Testcontainers starts a UniFi OS -Server (`hieutq/unifi-os-server`, the image and settings of the Homelab repo's -`.containers/unifi/compose.yml`), runs first-run setup, logs in once, and removes the +Server (`hieutq/unifi-os-server:5.0.8`, pinned by digest, with the settings of the Homelab +repo's `.containers/unifi/compose.yml`), runs first-run setup, logs in once, and removes the container when the run ends. The URL comes from the container's mapped port, so these tests cannot reach a real console whatever the environment says. ```sh dotnet test # starts the container when a runtime is reachable -UNIFISHARP_SKIP_CONTAINER_TESTS=1 dotnet test # skip them on purpose +UNIFISHARP_SKIP_CONTAINER_TESTS=1 dotnet test # skip them on purpose (1/true/yes) UNIFISHARP_CONTAINER_STARTUP_MINUTES=40 dotnet test # slow link (default 20) ``` - **Needs** a Docker-compatible runtime that allows `cgroup: host`, the `SYS_ADMIN` set of capabilities and a read-write `/sys/fs/cgroup` bind (OrbStack and Docker Desktop do). With no runtime reachable the tests **skip** and say why; any other startup failure fails. +- **Local daemon only:** a `DOCKER_HOST` that is not a local socket or loopback address, or a + `TESTCONTAINERS_HOST_OVERRIDE` / `tc.host` that is not loopback, **fails** the tests rather + than start a `SYS_ADMIN` container elsewhere and send writes to it. +- **The image is pinned** (Testcontainers only pulls a missing image, so a floating tag would + keep whatever was pulled first). Bump the tag and digest deliberately. - **First boot is slow:** every fresh container downloads Ubiquiti's installer (minutes). - It gets its own name (`unifisharp-tests-…`), a random host port and only port 443, so it runs beside a manually bootstrapped `unifi-os-server` container without touching it. diff --git a/tests/UnifiSharp.Tests/UnifiContainerFixture.cs b/tests/UnifiSharp.Tests/UnifiContainerFixture.cs index 052cad3..da7a745 100644 --- a/tests/UnifiSharp.Tests/UnifiContainerFixture.cs +++ b/tests/UnifiSharp.Tests/UnifiContainerFixture.cs @@ -19,18 +19,25 @@ namespace UnifiSharp.Tests; /// The client only ever points at this fixture's own container: the URL is built /// from the container's mapped port, never read from the environment. That is the whole /// safety argument for the write tests — no UNIFI_* variable can redirect them at a -/// real console. +/// real console. For the same reason the fixture refuses a remote Docker daemon or a +/// non-loopback host override: either would send the writes, and the privileged container, +/// somewhere other than this machine. /// Skips (with the reason in ) when no container runtime is -/// reachable or UNIFISHARP_SKIP_CONTAINER_TESTS=1. Any other startup failure — -/// a timeout, a setup that never completes — is a failure, not a skip. +/// reachable or UNIFISHARP_SKIP_CONTAINER_TESTS is 1/true/yes. Any other startup +/// failure — a timeout, a refused setup, a remote daemon — is a failure, not a skip. /// public sealed class UnifiContainerFixture : IAsyncLifetime { - public const string Image = "hieutq/unifi-os-server:latest"; + // Pinned by digest: Testcontainers only pulls an image that is missing, so a floating + // tag would silently keep whatever was pulled first. 5.0.8 is the build the Homelab + // compose container runs and the README's findings were verified against. + public const string Image = "hieutq/unifi-os-server:5.0.8@sha256:71393f7afc736e196eeb64a0a98b32a03e094a1f23008bfcfc48d28d2660ef24"; public const string Username = "labadmin"; public const string Password = "LabAdmin!2026"; // 8-64 chars (UniFi OS policy); throwaway, container-only public const string Label = "dev.chrison.unifisharp.tests"; + private const string SkipVariable = "UNIFISHARP_SKIP_CONTAINER_TESTS"; + // First boot downloads Ubiquiti's installer into the fresh container, which takes // minutes on a good line. Generous on purpose; UNIFISHARP_CONTAINER_STARTUP_MINUTES // overrides it for slow links (or shortens it when the image is warm). @@ -51,15 +58,16 @@ public sealed class UnifiContainerFixture : IAsyncLifetime public async Task InitializeAsync() { - if (Environment.GetEnvironmentVariable("UNIFISHARP_SKIP_CONTAINER_TESTS") == "1") + if (SkipRequested(Environment.GetEnvironmentVariable(SkipVariable))) { - SkipReason = "UNIFISHARP_SKIP_CONTAINER_TESTS=1 — skipping the UniFi container write tests."; + SkipReason = $"{SkipVariable} is set — skipping the UniFi container write tests."; return; } var timeout = StartupTimeout(); using var cts = new CancellationTokenSource(timeout); var stopwatch = Stopwatch.StartNew(); + var progress = new StartupProgress(); try { @@ -97,37 +105,49 @@ public async Task InitializeAsync() }; }) .Build(); - await _container.StartAsync(cts.Token); } catch (DockerUnavailableException ex) { - // Thrown before anything is created, from Build() or StartAsync(). - if (_container is not null) - { - await _container.DisposeAsync(); - _container = null; - } - + // Thrown from Build() when no endpoint is configured or reachable; nothing exists yet. SkipReason = $"No container runtime reachable — skipping the UniFi container write tests. ({ex.Message.Split('\n')[0]})"; return; } + // Before anything privileged starts: the daemon, and any host override, must be local. + RefuseIfNotLocal(TestcontainersSettings.OS.DockerEndpointAuthConfig?.Endpoint, TestcontainersSettings.DockerHostOverride); + try { + progress.Last = "starting the container"; + await _container.StartAsync(cts.Token); + + // Hostname is what the client will talk to, so it gets the same check: Testcontainers + // derives it from the endpoint and the override, and that derivation is not ours. + RefuseIfNotLocal(TestcontainersSettings.OS.DockerEndpointAuthConfig?.Endpoint, _container.Hostname); + var root = new UriBuilder("https", _container.Hostname, _container.GetMappedPublicPort(443)).Uri; - await Bootstrap.RunAsync(root, cts.Token); + await Bootstrap.RunAsync(root, progress, cts.Token); - var session = new UnifiLegacySession(new UnifiLegacyOptions + var options = new UnifiLegacyOptions { BaseUrl = new Uri(root, "/proxy/network/api/s/default"), Username = Username, Password = Password, VerifyTls = false, // the container serves a self-signed cert - }); + }; + // The session's handler backs off on a 429 from the login endpoint, which covers both + // the login below and the one the session repeats on a 401 mid-test. + var session = new UnifiLegacySession(options, new LoginBackoffHandler(new HttpClientHandler + { + CookieContainer = new CookieContainer(), + UseCookies = true, + ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator, + })); _client = new UnifiLegacyClient(session, ownsSession: true); // One login for the whole run, so the tests never add to the login rate limit. - await Bootstrap.WithLoginBackoffAsync(() => session.LoginAsync(cts.Token), cts.Token); + progress.Last = "logging the shared session in"; + await session.LoginAsync(cts.Token); } catch (Exception ex) { @@ -140,7 +160,7 @@ public async Task InitializeAsync() if (ex is OperationCanceledException && cts.IsCancellationRequested) { throw new TimeoutException( - $"UniFi OS container {name} was not ready within {timeout} " + + $"UniFi OS container {name} was not ready within {timeout}; last status: {progress.Last} " + "(first boot downloads the installer; raise UNIFISHARP_CONTAINER_STARTUP_MINUTES on a slow link).", ex); } @@ -159,6 +179,71 @@ public async Task DisposeAsync() } } + /// + /// Parses UNIFISHARP_SKIP_CONTAINER_TESTS: 1/true/yes skip, unset/0/false/no run, + /// anything else throws — a typo must not quietly decide whether the write tests run. + /// + internal static bool SkipRequested(string? value) => value?.Trim().ToLowerInvariant() switch + { + null or "" or "0" or "false" or "no" => false, + "1" or "true" or "yes" => true, + _ => throw new InvalidOperationException($"{SkipVariable} must be 1/true/yes or 0/false/no, got '{value}'."), + }; + + /// + /// Why the Docker endpoint or the host the tests will connect to is not this machine; + /// null when both are local. A socket or named pipe is a local daemon; a TCP/HTTP endpoint + /// and a host override must be loopback. An unknown endpoint is refused, not assumed local. + /// + internal static string? NonLocalDockerReason(Uri? endpoint, string? hostOverride) + { + if (endpoint is null) + { + return "the Docker endpoint could not be determined"; + } + + var localDaemon = endpoint.Scheme switch + { + "unix" or "npipe" => true, + "tcp" or "http" or "https" => IsLoopback(endpoint.Host), + _ => false, + }; + if (!localDaemon) + { + return $"the Docker endpoint {endpoint.OriginalString} is not a local socket or a loopback address"; + } + + if (!string.IsNullOrEmpty(hostOverride) && !IsLoopback(hostOverride)) + { + return $"the container host '{hostOverride}' (TESTCONTAINERS_HOST_OVERRIDE / tc.host, or derived from DOCKER_HOST) is not a loopback address"; + } + + return null; + } + + private static bool IsLoopback(string host) + { + if (string.Equals(host, "localhost", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + // Uri.Host keeps IPv6 brackets ("[::1]"); IPAddress.TryParse does not want them. + return IPAddress.TryParse(host.Trim('[', ']'), out var address) && IPAddress.IsLoopback(address); + } + + private static void RefuseIfNotLocal(Uri? endpoint, string? host) + { + if (NonLocalDockerReason(endpoint, host) is { } reason) + { + // A failure, not a skip: the run asked for these tests and something is pointing + // them away from this machine. The container runs with SYS_ADMIN and a rw cgroup mount. + throw new InvalidOperationException( + $"Refusing to run the UniFi container write tests: {reason}. They run only against a local Docker daemon; " + + $"set DOCKER_HOST to a local socket, or {SkipVariable}=1 to skip them."); + } + } + private static TimeSpan StartupTimeout() { var raw = Environment.GetEnvironmentVariable("UNIFISHARP_CONTAINER_STARTUP_MINUTES"); @@ -173,6 +258,58 @@ private static TimeSpan StartupTimeout() : throw new InvalidOperationException($"UNIFISHARP_CONTAINER_STARTUP_MINUTES must be a positive whole number of minutes, got '{raw}'."); } + /// What startup last saw, for the timeout message. + private sealed class StartupProgress + { + public string Last { get; set; } = "not started"; + } + + /// + /// The controller's login rate limit answers 429 for a while; one schedule for every + /// login the fixture makes, readiness probe and shared session alike. + /// + internal static class LoginBackoff + { + /// Attempts before a 429 is handed back to the caller. + public const int MaxAttempts = 6; + + /// The wait after the -th consecutive 429 (1-based). + public static TimeSpan Delay(int attempt) + { + ArgumentOutOfRangeException.ThrowIfLessThan(attempt, 1); + var seconds = 15 * Math.Pow(2, Math.Min(attempt - 1, 4)); + return TimeSpan.FromSeconds(Math.Min(seconds, 120)); + } + } + + /// + /// Retries POST /api/auth/login on 429 with ; every other + /// request passes straight through. After the 429 + /// goes back to the session, which reports it as a failed login. + /// + private sealed class LoginBackoffHandler(HttpMessageHandler inner) : DelegatingHandler(inner) + { + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken ct) + { + if (request.Method != HttpMethod.Post || request.RequestUri?.AbsolutePath != "/api/auth/login") + { + return await base.SendAsync(request, ct); + } + + for (var attempt = 1; ; attempt++) + { + var resp = await base.SendAsync(request, ct); + if (resp.StatusCode != HttpStatusCode.TooManyRequests || attempt == LoginBackoff.MaxAttempts) + { + return resp; + } + + resp.Dispose(); + await Task.Delay(LoginBackoff.Delay(attempt), ct); + } + } + } + /// /// bootstrap.sh's steps 1-3, against one controller root: wait for UniFi OS, run first-run /// setup, then wait until a legacy-API read answers 200. @@ -181,95 +318,105 @@ private static class Bootstrap { private static readonly TimeSpan Poll = TimeSpan.FromSeconds(5); - public static async Task RunAsync(Uri root, CancellationToken ct) + public static async Task RunAsync(Uri root, StartupProgress progress, CancellationToken ct) { using var http = new HttpClient(new HttpClientHandler { // Throwaway container, self-signed cert. ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator, - UseCookies = false, + // The readiness probe logs in once and keeps that session's cookie. + CookieContainer = new CookieContainer(), + UseCookies = true, }) { Timeout = TimeSpan.FromSeconds(15) }; // 1) UniFi OS answers /api/system with a deviceState once it is up. - await UntilAsync(async () => + await UntilAsync(progress, "waiting for /api/system", async () => { var body = await http.GetStringAsync(new Uri(root, "/api/system"), ct); + progress.Last = "/api/system answered without a deviceState"; return body.Contains("\"deviceState\"", StringComparison.Ordinal); }, ct); // 2) First-run setup. A fresh container is never set up, but the setup endpoint // answers 400/500 to an empty body only once it accepts input; until then it // is not ready. - await UntilAsync(async () => + await UntilAsync(progress, "waiting for /api/setup", async () => { using var probe = await http.PostAsJsonAsync(new Uri(root, "/api/setup"), new { }, ct); + progress.Last = $"/api/setup answered {(int)probe.StatusCode} to an empty body"; return probe.StatusCode is HttpStatusCode.BadRequest or HttpStatusCode.InternalServerError; }, ct); try { - using var _ = await http.PostAsJsonAsync(new Uri(root, "/api/setup"), + using var setup = await http.PostAsJsonAsync(new Uri(root, "/api/setup"), new { name = "UnifiSharp test controller", username = Username, password = Password }, ct); + if (!setup.IsSuccessStatusCode) + { + // An answer that says no is the reason, not something to wait out: without it + // the login below would only ever report "not ready". + var body = await setup.Content.ReadAsStringAsync(ct); + throw new InvalidOperationException( + $"UniFi OS first-run setup was refused ({(int)setup.StatusCode}): {(body.Length > 300 ? body[..300] : body)}"); + } + + progress.Last = $"setup accepted ({(int)setup.StatusCode})"; } - catch (Exception ex) when (ex is HttpRequestException - || (ex is TaskCanceledException && !ct.IsCancellationRequested)) + catch (HttpRequestException ex) { - // The connection often resets, or hangs until HttpClient.Timeout, while services - // restart after setup — expected. + // The connection often resets while services restart after setup — expected. // Whether setup took is decided by the login below, not by this response. + progress.Last = $"setup sent, response lost ({ex.Message})"; + } + catch (TaskCanceledException) when (!ct.IsCancellationRequested) + { + // Or it hangs until HttpClient.Timeout, for the same reason. + progress.Last = "setup sent, response timed out"; } - // 3) The admin can log in and the legacy API answers. Its own token, not the - // shared session's: this is a readiness probe, retried until it holds. - await UntilAsync(async () => + // 3) The admin can log in and the legacy API answers. Its own session, not the + // shared one's: logged in once and reused, re-logged-in only on a 401, and + // backed off on a 429 so polling never feeds the login rate limit. + var loggedIn = false; + var rateLimited = 0; + await UntilAsync(progress, "waiting for the admin login and legacy API", async () => { - using var login = await http.PostAsJsonAsync(new Uri(root, "/api/auth/login"), - new { username = Username, password = Password }, ct); - if (login.StatusCode == HttpStatusCode.TooManyRequests) + if (!loggedIn) { - await Task.Delay(TimeSpan.FromSeconds(30), ct); - return false; - } + using var login = await http.PostAsJsonAsync(new Uri(root, "/api/auth/login"), + new { username = Username, password = Password }, ct); + if (login.StatusCode == HttpStatusCode.TooManyRequests) + { + var delay = LoginBackoff.Delay(++rateLimited); + progress.Last = $"login answered 429 ({rateLimited} in a row); backing off {delay}"; + await Task.Delay(delay, ct); + return false; + } - if (!login.IsSuccessStatusCode) - { - return false; + rateLimited = 0; + if (!login.IsSuccessStatusCode) + { + progress.Last = $"login answered {(int)login.StatusCode}"; + return false; + } + + loggedIn = true; } - using var read = new HttpRequestMessage(HttpMethod.Get, new Uri(root, "/proxy/network/api/s/default/rest/portforward")); - foreach (var cookie in login.Headers.TryGetValues("Set-Cookie", out var c) ? c : []) + using var read = await http.GetAsync(new Uri(root, "/proxy/network/api/s/default/rest/portforward"), ct); + progress.Last = $"legacy read answered {(int)read.StatusCode}"; + if (read.StatusCode == HttpStatusCode.Unauthorized) { - read.Headers.TryAddWithoutValidation("Cookie", cookie.Split(';')[0]); + loggedIn = false; // session gone (services restarting); log in again next poll } - using var resp = await http.SendAsync(read, ct); - return resp.StatusCode == HttpStatusCode.OK; + return read.StatusCode == HttpStatusCode.OK; }, ct); } - /// - /// Retry a session login while the controller answers 429. The library surfaces a - /// failed login as carrying the status in its - /// message ("login failed (429): …"); anything other than 429 is rethrown. - /// - public static async Task WithLoginBackoffAsync(Func login, CancellationToken ct) - { - for (var delay = TimeSpan.FromSeconds(15); ; delay *= 2) - { - try - { - await login(); - return; - } - catch (UnifiLegacyException ex) when (ex.Message.StartsWith("login failed (429)", StringComparison.Ordinal)) - { - await Task.Delay(delay, ct); - } - } - } - - private static async Task UntilAsync(Func> ready, CancellationToken ct) + private static async Task UntilAsync(StartupProgress progress, string step, Func> ready, CancellationToken ct) { + progress.Last = step; while (true) { try @@ -279,13 +426,15 @@ private static async Task UntilAsync(Func> ready, CancellationToken c return; } } - catch (HttpRequestException) + catch (HttpRequestException ex) { // Not listening yet, or restarting a service — keep polling. + progress.Last = $"{step}: {ex.Message}"; } catch (TaskCanceledException) when (!ct.IsCancellationRequested) { // A single request timed out (HttpClient.Timeout), not the overall deadline. + progress.Last = $"{step}: request timed out"; } await Task.Delay(Poll, ct); diff --git a/tests/UnifiSharp.Tests/UnifiContainerFixtureTests.cs b/tests/UnifiSharp.Tests/UnifiContainerFixtureTests.cs new file mode 100644 index 0000000..1ff7ac4 --- /dev/null +++ b/tests/UnifiSharp.Tests/UnifiContainerFixtureTests.cs @@ -0,0 +1,99 @@ +namespace UnifiSharp.Tests; + +// The fixture's decisions that need no container: the opt-out switch, the refusal to run +// the privileged container anywhere but this machine, and the login backoff schedule. +public class UnifiContainerFixtureTests +{ + [Theory] + [InlineData("1")] + [InlineData("true")] + [InlineData("TRUE")] + [InlineData("True")] + [InlineData("yes")] + [InlineData("YES")] + [InlineData(" yes ")] + public void Skip_switch_accepts_the_usual_yes_spellings(string value) => + Assert.True(UnifiContainerFixture.SkipRequested(value)); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("0")] + [InlineData("false")] + [InlineData("No")] + public void Skip_switch_unset_or_no_runs_the_tests(string? value) => + Assert.False(UnifiContainerFixture.SkipRequested(value)); + + [Theory] + [InlineData("y")] + [InlineData("on")] + [InlineData("2")] + public void Skip_switch_with_an_unrecognised_value_fails_loudly(string value) + { + // A typo must not silently run (or silently skip) the write tests. + var ex = Assert.Throws(() => UnifiContainerFixture.SkipRequested(value)); + Assert.Contains("UNIFISHARP_SKIP_CONTAINER_TESTS", ex.Message); + } + + [Theory] + [InlineData("unix:///var/run/docker.sock")] + [InlineData("unix:///Users/me/.orbstack/run/docker.sock")] + [InlineData("npipe://./pipe/docker_engine")] + [InlineData("tcp://localhost:2375")] + [InlineData("tcp://127.0.0.1:2375")] + [InlineData("tcp://[::1]:2375")] + [InlineData("http://LOCALHOST:2375")] + public void Local_docker_endpoints_are_accepted(string endpoint) => + Assert.Null(UnifiContainerFixture.NonLocalDockerReason(new Uri(endpoint), hostOverride: null)); + + [Theory] + [InlineData("tcp://10.0.0.14:2375")] + [InlineData("tcp://docker.homelab.internal:2376")] + [InlineData("https://192.168.1.20:2376")] + [InlineData("ssh://root@hpe-01")] + public void Remote_or_unrecognised_docker_endpoints_are_refused(string endpoint) + { + var reason = UnifiContainerFixture.NonLocalDockerReason(new Uri(endpoint), hostOverride: null); + + Assert.NotNull(reason); + Assert.Contains(endpoint.TrimEnd('/'), reason, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void An_unknown_docker_endpoint_is_refused_not_assumed_local() => + Assert.NotNull(UnifiContainerFixture.NonLocalDockerReason(endpoint: null, hostOverride: null)); + + [Theory] + [InlineData("localhost")] + [InlineData("127.0.0.1")] + [InlineData("::1")] + [InlineData("[::1]")] + public void A_loopback_host_override_is_accepted(string host) => + Assert.Null(UnifiContainerFixture.NonLocalDockerReason(new Uri("unix:///var/run/docker.sock"), host)); + + [Theory] + [InlineData("10.0.0.14")] + [InlineData("hpe-01.homelab.chrison.internal")] + [InlineData("host.docker.internal")] + public void A_remote_host_override_is_refused_even_with_a_local_socket(string host) + { + // The override is where the tests send their writes, so it alone can redirect them. + var reason = UnifiContainerFixture.NonLocalDockerReason(new Uri("unix:///var/run/docker.sock"), host); + + Assert.NotNull(reason); + Assert.Contains(host, reason); + } + + [Fact] + public void Login_backoff_starts_at_15s_doubles_and_is_capped_at_two_minutes() + { + var delays = Enumerable.Range(1, 6).Select(UnifiContainerFixture.LoginBackoff.Delay).ToArray(); + + Assert.Equal( + [ + TimeSpan.FromSeconds(15), TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(60), + TimeSpan.FromMinutes(2), TimeSpan.FromMinutes(2), TimeSpan.FromMinutes(2), + ], + delays); + } +}