diff --git a/AGENTS.md b/AGENTS.md index cea4ed9..f967938 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,16 +61,27 @@ Two frameworks coexist: `find_package` requirement; the build fails without it. Test targets: `tests`, `test_assets`, `test_provisioning`, `test_network_unit`, -`test_config`, `test_db`, `test_proclamation_sync`, `test_api_client`, -`test_license_service`, `test_license_status`, `test_validation`, -`test_date_format`, `test_timezone`, `test_logger`, `test_audio`, -`test_audio_downloader`, `test_fibonacci_pattern`, `test_scroller_streams`, -`test_fibonacci_scrambler`, `test_color_palettes`. +`test_nm_device`, `test_config`, `test_db`, `test_proclamation_sync`, +`test_api_client`, `test_license_service`, `test_license_status`, +`test_validation`, `test_date_format`, `test_timezone`, `test_logger`, +`test_audio`, `test_audio_downloader`, `test_fibonacci_pattern`, +`test_scroller_streams`, `test_fibonacci_scrambler`, `test_color_palettes`. + +`test_nm_device` covers the pure `parseAccessPoint()` mapping; the sd-bus reads +in `WifiDevice::getAccessPoints()` are Linux-only and verified live. Do not +wrap the per-AP `ay` SSID property read in `sd_bus_message_enter_container()` +before `sd_bus_message_read_array()` — `read_array` enters/exits the array +itself, and the double-enter silently empties the scan list (see +`src/network/nm_device.cpp`). New test files must be registered as a new `add_executable` target in `CMakeLists.txt` (there is no glob). `test_assets` must pass before committing — it checks the font and image files exist. +`test_api_client` and `test_license_service` include "Live" tests that hit the +portal unconditionally — they are excluded from the Nix `checkPhase` offline +suite and need the `RWT_LIVE_*` secrets to pass. + --- ## Runtime data & keys diff --git a/CMakeLists.txt b/CMakeLists.txt index dd53c2b..43e057a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -128,6 +128,20 @@ add_executable(test_network_unit target_include_directories(test_network_unit PRIVATE src) target_link_libraries(test_network_unit PRIVATE GTest::gtest_main) +# Access point parsing unit tests (pure mapping; the sd-bus reads are Linux-only) +add_executable(test_nm_device + tests/network/test_nm_device.cpp + src/network/nm_device.cpp + src/network/nm_connection.cpp + src/logger/logger.cpp +) +target_include_directories(test_nm_device PRIVATE src) +target_link_libraries(test_nm_device PRIVATE GTest::gtest_main) +if(UNIX AND NOT APPLE) + target_link_libraries(test_nm_device PRIVATE ${SDBUS_LIBRARIES}) + target_include_directories(test_nm_device PRIVATE ${SDBUS_INCLUDE_DIRS}) +endif() + # Config unit tests add_executable(test_config tests/test_config.cpp diff --git a/nix/package.nix b/nix/package.nix index 0e021ac..e30be24 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -95,6 +95,7 @@ stdenv.mkDerivation (finalAttrs: { test_assets \ test_provisioning \ test_network_unit \ + test_nm_device \ test_config \ test_db \ test_proclamation_sync \ diff --git a/src/network/nm_device.cpp b/src/network/nm_device.cpp index ac9eaaa..47e0930 100644 --- a/src/network/nm_device.cpp +++ b/src/network/nm_device.cpp @@ -1,6 +1,7 @@ #include "nm_device.h" #include "nm_connection.h" #include "logger/logger.h" +#include #include #include @@ -13,8 +14,9 @@ namespace radiantwave::network { // ─── Access Point Helper ─────────────────────────────────────────────── -namespace { - +// Build an AccessPointInfo from raw D-Bus AccessPoint property values (see +// nm_device.h). The `ssid` argument carries the raw SSID bytes decoded from +// the AccessPoint.Ssid `ay` property as a std::string. AccessPointInfo parseAccessPoint( const std::string& object_path, const std::string& ssid, @@ -36,8 +38,6 @@ AccessPointInfo parseAccessPoint( return ap; } -} // anonymous namespace - // ─── WifiDevice Implementation ───────────────────────────────────────── #ifdef __linux__ @@ -229,13 +229,40 @@ std::vector WifiDevice::getAccessPoints() const { return result; } + // Read a single numeric AccessPoint property. Strength is a byte ("y"), + // the security flag properties are uint32 ("u"). Leaves `out` untouched on + // failure, so callers can rely on zero-initialized defaults. + auto readApProperty = [this](const char* ap_path, const char* prop, + const char* type, void* out) { + sd_bus_message* prop_msg = nullptr; + sd_bus_error prop_error = SD_BUS_ERROR_NULL; + int pr = sd_bus_get_property( + impl_->bus, + "org.freedesktop.NetworkManager", + ap_path, + "org.freedesktop.NetworkManager.AccessPoint", + prop, + &prop_error, + &prop_msg, + type + ); + if (pr >= 0 && prop_msg) { + if (strcmp(type, "u") == 0) { + sd_bus_message_read(prop_msg, "u", static_cast(out)); + } else { + sd_bus_message_read(prop_msg, "y", static_cast(out)); + } + sd_bus_message_unref(prop_msg); + } + sd_bus_error_free(&prop_error); + return pr >= 0; + }; + const char* ap_path = nullptr; while ((r = sd_bus_message_read(msg, "o", &ap_path)) > 0 && ap_path) { - // Get AP properties sd_bus_message* ap_msg = nullptr; sd_bus_error ap_error = SD_BUS_ERROR_NULL; - - // Get all properties at once + r = sd_bus_get_property( impl_->bus, "org.freedesktop.NetworkManager", @@ -246,51 +273,41 @@ std::vector WifiDevice::getAccessPoints() const { &ap_msg, "ay" ); - + if (r >= 0) { - // Read SSID byte array - sd_bus_message_enter_container(ap_msg, 'a', "y"); + // Read the SSID byte array. sd_bus_get_property() already leaves + // the message positioned at the array (inside the variant wrapper), + // and sd_bus_message_read_array() enters AND exits the array + // container itself. Do NOT call sd_bus_message_enter_container() + // here first, or the read fails and every network silently drops + // out of the scan list (it only ever shows up as an empty page with + // no error logs). const uint8_t* ssid_data = nullptr; size_t ssid_len = 0; r = sd_bus_message_read_array(ap_msg, 'y', (const void**)&ssid_data, &ssid_len); - + std::string ssid; if (r >= 0 && ssid_data && ssid_len > 0) { ssid = std::string(reinterpret_cast(ssid_data), ssid_len); } sd_bus_message_unref(ap_msg); - - // Get signal strength - sd_bus_message* sig_msg = nullptr; - sd_bus_error sig_error = SD_BUS_ERROR_NULL; + + // Remaining properties: signal strength plus the three flags that + // drive detectSecurityType() (otherwise every network is reported + // as Open and secured connections are built without a security + // section). uint8_t signal = 0; - - r = sd_bus_get_property( - impl_->bus, - "org.freedesktop.NetworkManager", - ap_path, - "org.freedesktop.NetworkManager.AccessPoint", - "Strength", - &sig_error, - &sig_msg, - "y" - ); - - if (r >= 0) { - sd_bus_message_read(sig_msg, "y", &signal); - sd_bus_message_unref(sig_msg); - } - sd_bus_error_free(&sig_error); - - // Get flags uint32_t flags = 0, wpa_flags = 0, rsn_flags = 0; - // ... (would need similar property reads for flags) - + readApProperty(ap_path, "Strength", "y", &signal); + readApProperty(ap_path, "Flags", "u", &flags); + readApProperty(ap_path, "WpaFlags", "u", &wpa_flags); + readApProperty(ap_path, "RsnFlags", "u", &rsn_flags); + if (!ssid.empty()) { result.push_back(parseAccessPoint(ap_path, ssid, signal, flags, wpa_flags, rsn_flags)); } } - + sd_bus_error_free(&ap_error); } diff --git a/src/network/nm_device.h b/src/network/nm_device.h index 7256140..c7cfed7 100644 --- a/src/network/nm_device.h +++ b/src/network/nm_device.h @@ -21,6 +21,17 @@ struct AccessPointInfo { SecurityType security = SecurityType::Open; }; +// Build an AccessPointInfo from the raw D-Bus AccessPoint property values. +// Pure mapping (no D-Bus I/O) so it is unit-testable; WifiDevice::getAccessPoints() +// feeds it with per-AP property reads. +AccessPointInfo parseAccessPoint( + const std::string& object_path, + const std::string& ssid, + uint8_t signal, + uint32_t flags, + uint32_t wpa_flags, + uint32_t rsn_flags); + // ─── WiFi Device Interface ───────────────────────────────────────────── // Abstract interface for WiFi device operations diff --git a/tests/network/test_nm_device.cpp b/tests/network/test_nm_device.cpp new file mode 100644 index 0000000..9f0404c --- /dev/null +++ b/tests/network/test_nm_device.cpp @@ -0,0 +1,68 @@ +#include "network/nm_device.h" +#include +#include + +using namespace radiantwave::network; + +// parseAccessPoint() maps the raw D-Bus AccessPoint property values into an +// AccessPointInfo. It is the pure half of WifiDevice::getAccessPoints(); the +// sd-bus reads that feed it are Linux-only and covered by live testing. + +TEST(ParseAccessPointTest, PassthroughOfProperties) { + AccessPointInfo ap = parseAccessPoint( + "/org/freedesktop/NetworkManager/AccessPoint/1", + "MyWiFi", 72, 0x01, 0x0000, 0x0000); + + EXPECT_EQ(ap.object_path, "/org/freedesktop/NetworkManager/AccessPoint/1"); + EXPECT_EQ(ap.ssid, "MyWiFi"); + EXPECT_EQ(ap.signal_strength, 72); + EXPECT_EQ(ap.flags, 0x01u); + EXPECT_EQ(ap.wpa_flags, 0x0000u); + EXPECT_EQ(ap.rsn_flags, 0x0000u); +} + +TEST(ParseAccessPointTest, SsidBytesRoundTrip) { + // The SSID arrives as raw bytes (an `ay` array); it must round-trip back + // into the ssid_bytes field so a connection can be built from it. + std::string ssid = "Café WiFi"; + AccessPointInfo ap = parseAccessPoint( + "/org/freedesktop/NetworkManager/AccessPoint/2", ssid, 50, 0, 0, 0); + + EXPECT_EQ(std::string(ap.ssid_bytes.begin(), ap.ssid_bytes.end()), ssid); +} + +TEST(ParseAccessPointTest, DetectsOpenNetwork) { + AccessPointInfo ap = parseAccessPoint("/x", "OpenNet", 30, 0, 0, 0); + EXPECT_EQ(ap.security, SecurityType::Open); +} + +TEST(ParseAccessPointTest, DetectsWepFromPrivacyFlag) { + AccessPointInfo ap = parseAccessPoint("/x", "WepNet", 30, 0x01, 0, 0); + EXPECT_EQ(ap.security, SecurityType::WEP); +} + +TEST(ParseAccessPointTest, DetectsWpaFromWpaFlags) { + AccessPointInfo ap = parseAccessPoint("/x", "WpaNet", 30, 0, 0x0001, 0); + EXPECT_EQ(ap.security, SecurityType::WPA); +} + +TEST(ParseAccessPointTest, DetectsWpa2FromRsnPsk) { + AccessPointInfo ap = parseAccessPoint("/x", "Wpa2Net", 30, 0, 0, 0x0100); + EXPECT_EQ(ap.security, SecurityType::WPA2); +} + +TEST(ParseAccessPointTest, DetectsWpa3FromRsnSae) { + AccessPointInfo ap = parseAccessPoint("/x", "Wpa3Net", 30, 0, 0, 0x0400); + EXPECT_EQ(ap.security, SecurityType::WPA3); +} + +TEST(ParseAccessPointTest, Wpa3TakesPrecedenceOverWpa2) { + // SAE (0x0400) must win over PSK (0x0100) when both are advertised. + AccessPointInfo ap = parseAccessPoint("/x", "MixedNet", 30, 0, 0, 0x0500); + EXPECT_EQ(ap.security, SecurityType::WPA3); +} + +TEST(ParseAccessPointTest, Wpa2TakesPrecedenceOverWpa) { + AccessPointInfo ap = parseAccessPoint("/x", "MixedNet", 30, 0, 0x0001, 0x0100); + EXPECT_EQ(ap.security, SecurityType::WPA2); +}