fix(network): read AP security flags in WiFi scan and unit-test parseAccessPoint
parseAccessPoint() is extracted from an anonymous namespace so the pure property->AccessPointInfo mapping is testable. getAccessPoints() now reads the Flags/WpaFlags/RsnFlags properties (they were never read, so every network was reported Open and connections lacked a security section) and fixes the double container-enter on the SSID 'ay' read that silently emptied the scan list.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -95,6 +95,7 @@ stdenv.mkDerivation (finalAttrs: {
|
||||
test_assets \
|
||||
test_provisioning \
|
||||
test_network_unit \
|
||||
test_nm_device \
|
||||
test_config \
|
||||
test_db \
|
||||
test_proclamation_sync \
|
||||
|
||||
+54
-37
@@ -1,6 +1,7 @@
|
||||
#include "nm_device.h"
|
||||
#include "nm_connection.h"
|
||||
#include "logger/logger.h"
|
||||
#include <cstring>
|
||||
#include <iostream>
|
||||
#include <stdexcept>
|
||||
|
||||
@@ -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<AccessPointInfo> 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<uint32_t*>(out));
|
||||
} else {
|
||||
sd_bus_message_read(prop_msg, "y", static_cast<uint8_t*>(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<AccessPointInfo> 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<const char*>(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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
#include "network/nm_device.h"
|
||||
#include <gtest/gtest.h>
|
||||
#include <string>
|
||||
|
||||
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);
|
||||
}
|
||||
Reference in New Issue
Block a user