mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-05-03 22:03:00 +00:00
fix(wifi): reliable open AP with captive portal — tested on Trixie Pi (#320)
* fix(wifi): create truly open AP via nmcli connection add; add captive portal to nmcli path nmcli device wifi hotspot always attaches a WPA2 PSK on Bookworm/Trixie and silently ignores post-creation security modifications, causing users to be prompted for an unknown password. Switch to nmcli connection add with 802-11-wireless.mode ap and no security section — NM cannot auto-add a password to a profile that has no 802-11-wireless-security block. Also: - Remove dead DEFAULT_AP_PASSWORD / ap_password config field (stored but never passed to hostapd or nmcli, causing user confusion) - Add iptables port 80→5000 redirect to the nmcli AP path so captive portal auto-popup works on phones without hostapd (previously only worked on the hostapd path) - Clean up iptables rules on disable for the nmcli path - Improve LED message on AP enable: show SSID, "No password", and IP:port on both paths so users know exactly how to connect - Fix systemd template: replace hardcoded /home/ledpi/LEDMatrix/ with __PROJECT_ROOT_DIR__ placeholder (install script already writes correct path) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): address Codacy review findings in AP mode implementation - Validate ap_ssid/ap_channel from config before passing to subprocess (printable ASCII ≤32 chars; channel 1-14) to prevent command injection - Fix INPUT iptables rule: PREROUTING redirects port 80→5000 so the INPUT chain sees dport=5000, not 80. Old INPUT rule on port 80 was a no-op. - Refactor iptables setup/teardown into _setup_iptables_redirect() and _teardown_iptables_redirect() helpers, eliminating duplicate logic in the hostapd and nmcli paths - Save/restore ip_forward state (via /tmp/ledmatrix_ip_forward_saved) instead of forcing it to 0 on cleanup, which could break VPNs or bridges already relying on forwarding - nmcli path skips ip_forward management entirely: NM's ipv4.method=shared already manages it for the duration of the connection - Fix _get_ap_status_nmcli() verification: new 'connection add type wifi' profiles have type '802-11-wireless', not 'hotspot', so verification was always returning False. Now also matches by our known connection name. - Remove SSID-based connection deletion: deleting any profile whose SSID matched the AP SSID could destroy a user's saved home WiFi profile. Now only deletes by our application-managed profile names. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugins): fix async race in refreshPlugins; use cache TTL to gate re-swap metadata fetch refreshPlugins() called searchPluginStore(true) and showNotification() immediately after refreshInstalledPlugins() without awaiting the returned Promise, so window.installedPlugins could still be stale when the store rendered its Installed/Reinstall badges. Chain .then() so both run only after the fetch completes. In initializePlugins(), the re-swap path always passed fetchCommitInfo=false to searchPluginStore, skipping GitHub metadata even when the 5-minute cache TTL had expired. Add storeCacheExpired() helper and compute isReswapWarm = _reswap && !storeCacheExpired() so fresh metadata is fetched whenever the cache is cold, regardless of whether the render is a first load or a tab re-swap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address three wifi_manager and one plugins_manager review findings wifi_manager.py: - _create_hostapd_config: use _validate_ap_config() for ssid/channel instead of raw self.config values; strip newlines from SSID to prevent config-file injection via the generated hostapd.conf - _setup_iptables_redirect: check return codes of sysctl ip_forward enable and both iptables -A calls; on any failure log the error output, call _teardown_iptables_redirect() to restore state, and return False instead of silently succeeding - _enable_ap_mode_nmcli_hotspot: on AP verification failure roll back fully — tear down iptables redirect, delete the LEDMatrix-Setup-AP connection profile, clear the LED message — before returning False plugins_manager.js: - initializePlugins: chain searchPluginStore(!isReswapWarm) inside loadInstalledPlugins().then() so window.installedPlugins is populated before the store renders Installed/Reinstall badges (same pattern applied to refreshPlugins() in the previous commit) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): use _find_command_path for iptables/sysctl; harden ip_forward save/restore Add _find_command_path() helper that extends _check_command()'s sbin-aware lookup to return the absolute binary path rather than a boolean. Use it in _setup_iptables_redirect and _teardown_iptables_redirect so iptables and sysctl are resolved via /sbin or /usr/sbin even when those directories are absent from PATH in systemd service environments. Also harden the ip_forward save/restore logic: - Read ip_forward from /proc/sys/net/ipv4/ip_forward (no subprocess, no PATH dependency) instead of spawning sysctl -n - Skip the sysctl -w ip_forward=1 write when the value is already "1" to avoid mutating state owned by another service (VPN, NM shared mode, bridge) - Track save success via presence of the save file: if the /proc read or file write fails, leave the file absent so teardown knows not to restore - In _teardown_iptables_redirect, only restore ip_forward when the save file exists; if absent, leave the current value untouched rather than forcing "0" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): check _setup_iptables_redirect return; fix hostapd LED SSID; teardown on exception - Both AP startup paths (hostapd and nmcli) now check the bool returned by _setup_iptables_redirect() and treat False as a hard failure: the hostapd path stops hostapd/dnsmasq and returns an error tuple; the nmcli path brings down and deletes the LEDMatrix-Setup-AP profile and clears the LED message - _enable_ap_mode_hostapd's LED message now calls _validate_ap_config() to get the same sanitized SSID that _create_hostapd_config() uses, so the displayed name always matches the AP actually broadcast by hostapd - _setup_iptables_redirect's outer except block now calls _teardown_iptables_redirect() before returning False so partial iptables/ ip_forward state is always cleaned up on unexpected exceptions; cleanup exceptions are caught and logged separately Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(wifi): add unit tests for AP mode — open network, iptables, LED, cleanup ordering Six pytest unit tests covering the five review scenarios. All subprocess and filesystem side-effects are mocked so the tests run without root, hardware, or a Pi OS environment. 1. test_nmcli_ap_profile_has_no_security_params — asserts the nmcli connection add command has no key-mgmt / psk / WPA arguments and sets mode=ap. 2. test_iptables_nat_rules_added_on_ap_start — verifies _setup_iptables_redirect emits a PREROUTING REDIRECT 80→5000 rule and an INPUT ACCEPT rule for port 5000 (not 80, which never hits INPUT after PREROUTING rewrites it). 3. test_iptables_rules_and_ip_forward_reverted_on_teardown — verifies the -D PREROUTING/-D INPUT calls and that sysctl restores the saved ip_forward value and removes the save file. 4. test_ip_forward_not_restored_when_save_file_absent — verifies teardown skips sysctl when the save file was never written, preventing blind ip_forward=0 on systems using ip_forward for VPNs or NM shared mode. 5. test_led_message_shows_ssid_no_password_and_url — asserts the LED message includes the SSID, 'No password', and the 192.168.4.1:5000 setup URL. 6. test_existing_ap_profiles_deleted_before_new_profile_created — asserts all known profile names are targeted for deletion before 'nmcli connection add'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(wifi): adopt adsb-feeder-image hotspot patterns — DNS spoofing, connectivity check, idle timeout, wrong-password UX, watchdog escalation Inspired by the production-proven approach in dirkhh/adsb-feeder-image. 1. DNS spoofing for automatic captive-portal popup (Change 1 — Critical) Write /etc/NetworkManager/dnsmasq-shared.d/ledmatrix-captive.conf with address=/#/192.168.4.1 before nmcli connection up so NM's built-in dnsmasq (ipv4.method=shared) resolves every hostname to the AP IP. This triggers the OS captive-portal popup automatically on iOS / Android / Windows / macOS — no manual navigation to 192.168.4.1:5000/setup required. New helpers: _write_nm_dnsmasq_captive_conf / _remove_nm_dnsmasq_captive_conf. New constants: NM_DNSMASQ_SHARED_DIR / NM_DNSMASQ_SHARED_CONF. 2. Real internet connectivity check (Change 2 — High) Add _check_internet_connectivity() (ping 8.8.8.8 + HTTP fallback). check_and_manage_ap_mode() now considers a device "disconnected" when nmcli shows connected but no real internet reachability, matching adsb-feeder's multi-method gateway/DNS/HTTP test approach. 3. AP idle timeout (Change 3 — Medium) Track _ap_enabled_at timestamp in enable_ap_mode(). Add _has_ap_clients() using 'iw dev <iface> station dump'. check_and_manage_ap_mode() auto-disables AP after ap_idle_timeout_minutes (default 15) with no associated clients. 4. Wrong-password error feedback (Change 4 — Medium) _connect_nmcli() detects "Secrets were required" / "authentication rejected" in nmcli stderr and prefixes the message with "wrong_password: ". The /api/v3/wifi/connect route propagates error_type="wrong_password" in the JSON response. captive_setup.html shows "Incorrect password — try again" (keeping the form active) instead of the generic failure message. 5. Escalating watchdog NM restart (Change 5 — Low) wifi_monitor_daemon.py tracks _consecutive_internet_failures. After _nm_restart_threshold (5) consecutive checks where nmcli shows connected but internet is unreachable, restart NetworkManager as a recovery step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): restore safe AP-enable trigger; decouple internet check from AP logic The previous commit introduced _check_internet_connectivity() into check_and_manage_ap_mode(), which shared the same _disconnected_checks counter that triggers AP enable. This created a false-positive risk: 90 seconds of packet loss on working WiFi would enable AP mode and kick off the connection. Fix: restore nmcli association state as the sole AP-enable trigger (original, safe behaviour). The internet connectivity check is now used only in the daemon watchdog for the NM-restart escalation — matching how adsb-feeder-image actually structures the two concerns (initial setup detection vs. ongoing monitoring). Also clarify daemon comment: the connectivity check runs once per cycle in the watchdog block, not inside check_and_manage_ap_mode, so there is no double-call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): remove PMF setting from open AP profile — breaks nmcli connection add on Trixie NM 1.52+ 802-11-wireless-security.pmf is only valid within a security section that also includes key-mgmt. Adding it to an open-network profile causes NM 1.52+ to reject the connection add with 'key-mgmt: property is missing'. PMF has no meaning for open APs (it only applies to WPA2/WPA3), so the setting is simply removed rather than worked around. Found by testing on devpi (Trixie, NM 1.52.1). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): add nftables fallback for port redirect; graceful degradation when neither available Tested on devpi (Trixie, NM 1.52.1): iptables is not installed; nftables is. The original code called _setup_iptables_redirect() and treated 'iptables not found' as a hard failure, rolling back the entire AP setup. Changes: - _setup_iptables_redirect() now tries iptables first, then nftables as a fallback. When neither is available it logs a warning and returns True so the AP still comes up (DNS spoofing still triggers the captive portal popup; users land on port 5000 directly instead of being auto-redirected from 80). - Split into _setup_iptables_redirect_iptables() and _setup_iptables_redirect_nftables() for clarity. - Added _redirect_backend instance var ("iptables" | "nftables" | None) so _teardown_iptables_redirect() uses the same tool that setup used. - nftables teardown: deletes the 'ledmatrix' table (clean, no leftover rules). - iptables teardown: unchanged logic (ip_forward save/restore). - Also removed the PMF workaround for Trixie: 802-11-wireless-security.pmf requires key-mgmt to also be set, breaking open-network creation on NM 1.52+. Open APs have no management frame protection by definition. - Update teardown test to set _redirect_backend = "iptables" before calling it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): public check_internet_connectivity(); absolute systemctl path; stricter mode assertion wifi_manager.py: - Add public check_internet_connectivity() wrapping the private method so the daemon does not reach into the private API wifi_monitor_daemon.py: - Call wifi_manager.check_internet_connectivity() instead of the private _check_internet_connectivity() - Use /usr/bin/systemctl (absolute path) instead of bare "systemctl" - Wrap NM restart in try/except with check=True; only reset _consecutive_internet_failures on success — on CalledProcessError or other exception, log the error and leave the counter unchanged so the next cycle retries test/test_wifi_manager_ap.py: - Replace loose `assert "ap" in add_calls[0]` (list-membership check that could be satisfied by any element equal to "ap") with an explicit key/value check: locate "802-11-wireless.mode" in the command list and assert the next element is exactly "ap" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
334
test/test_wifi_manager_ap.py
Normal file
334
test/test_wifi_manager_ap.py
Normal file
@@ -0,0 +1,334 @@
|
||||
"""
|
||||
Unit tests for WiFi AP mode — src.wifi_manager.
|
||||
|
||||
Each test exercises logic that can be verified through the subprocess calls the
|
||||
manager emits, without requiring root access, hardware, or a running Pi.
|
||||
|
||||
Scenarios covered:
|
||||
1. nmcli AP profile is created with no security parameters (open/passwordless).
|
||||
2. iptables PREROUTING and INPUT rules are added when the nmcli AP starts.
|
||||
3. iptables rules and ip_forward are reverted when the AP is torn down.
|
||||
4. LED matrix message includes the SSID, 'No password', and the setup URL.
|
||||
5. Known AP profile names are deleted before the new profile is created.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from src.wifi_manager import WiFiManager
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _ok(stdout: str = "", stderr: str = "") -> MagicMock:
|
||||
r = MagicMock()
|
||||
r.returncode = 0
|
||||
r.stdout = stdout
|
||||
r.stderr = stderr
|
||||
return r
|
||||
|
||||
|
||||
def _fail(stdout: str = "", stderr: str = "error") -> MagicMock:
|
||||
r = MagicMock()
|
||||
r.returncode = 1
|
||||
r.stdout = stdout
|
||||
r.stderr = stderr
|
||||
return r
|
||||
|
||||
|
||||
def _find_path_side_effect(name: str) -> str:
|
||||
"""Deterministic fake for _find_command_path."""
|
||||
return {"iptables": "/usr/sbin/iptables", "sysctl": "/usr/sbin/sysctl"}.get(
|
||||
name, f"/usr/bin/{name}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.fixture()
|
||||
def wifi_config(tmp_path: Path) -> Path:
|
||||
"""Minimal wifi_config.json in a temporary directory."""
|
||||
cfg_dir = tmp_path / "config"
|
||||
cfg_dir.mkdir()
|
||||
cfg = {
|
||||
"ap_ssid": "LEDMatrix-Setup",
|
||||
"ap_channel": 7,
|
||||
"auto_enable_ap_mode": True,
|
||||
"saved_networks": [],
|
||||
}
|
||||
p = cfg_dir / "wifi_config.json"
|
||||
p.write_text(json.dumps(cfg))
|
||||
return p
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def manager(wifi_config: Path, tmp_path: Path) -> WiFiManager:
|
||||
"""
|
||||
WiFiManager with all system calls stubbed out during construction and the
|
||||
ip_forward save file redirected to a per-test temporary path.
|
||||
"""
|
||||
with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")), \
|
||||
patch.object(WiFiManager, "_detect_trixie", return_value=False):
|
||||
mgr = WiFiManager(config_path=wifi_config)
|
||||
|
||||
# Force clean, deterministic state regardless of what __init__ inferred
|
||||
mgr._wifi_interface = "wlan0"
|
||||
mgr.has_nmcli = True
|
||||
mgr.has_hostapd = False
|
||||
mgr.has_dnsmasq = False
|
||||
mgr.has_iwlist = False
|
||||
mgr._is_trixie = False
|
||||
# Redirect the ip_forward save file to tmp so tests never share state
|
||||
mgr._IP_FORWARD_SAVE_PATH = tmp_path / "ip_fwd_saved"
|
||||
return mgr
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 1. AP profile is open (no password)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_nmcli_ap_profile_has_no_security_params(manager: WiFiManager) -> None:
|
||||
"""
|
||||
The 'nmcli connection add' command must not include key-mgmt, psk, or any
|
||||
WPA-related parameter. On Bookworm/Trixie, NM creates a WPA2-protected
|
||||
hotspot even when those values are set to 'none'/empty via a later
|
||||
'connection modify', so the profile must be created without a security
|
||||
section from the start.
|
||||
"""
|
||||
captured: list[list[str]] = []
|
||||
|
||||
def _run(cmd, **kw):
|
||||
captured.append(list(cmd))
|
||||
return _ok()
|
||||
|
||||
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
|
||||
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
|
||||
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
|
||||
patch.object(manager, "_get_ap_status_nmcli",
|
||||
return_value={"active": True, "ip": "192.168.4.1"}), \
|
||||
patch.object(manager, "_show_led_message"):
|
||||
|
||||
success, _ = manager._enable_ap_mode_nmcli_hotspot()
|
||||
|
||||
assert success, "AP enable should report success"
|
||||
|
||||
add_calls = [c for c in captured if "nmcli" in c and "connection" in c and "add" in c]
|
||||
assert add_calls, "Expected at least one 'nmcli connection add' invocation"
|
||||
|
||||
add_str = " ".join(add_calls[0])
|
||||
assert "key-mgmt" not in add_str, "AP profile must not set key-mgmt"
|
||||
assert "psk" not in add_str, "AP profile must not include a PSK/password"
|
||||
assert "wpa" not in add_str.lower(), "AP profile must not reference WPA"
|
||||
assert "802-11-wireless.mode" in add_str, "AP profile must declare wireless mode"
|
||||
# Verify the value for 802-11-wireless.mode is exactly "ap" — check the element
|
||||
# that immediately follows the key in the command list, not a loose substring match.
|
||||
cmd = add_calls[0]
|
||||
try:
|
||||
mode_idx = cmd.index("802-11-wireless.mode")
|
||||
assert cmd[mode_idx + 1] == "ap", \
|
||||
f"802-11-wireless.mode value must be exactly 'ap', got {cmd[mode_idx + 1]!r}"
|
||||
except ValueError:
|
||||
pytest.fail("802-11-wireless.mode not found as a list element in nmcli command")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. iptables NAT rules are added when the AP starts
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_iptables_nat_rules_added_on_ap_start(manager: WiFiManager) -> None:
|
||||
"""
|
||||
_setup_iptables_redirect must add:
|
||||
- a PREROUTING REDIRECT rule that maps incoming TCP port 80 to port 5000, and
|
||||
- an INPUT ACCEPT rule for port 5000 (the post-redirect destination port,
|
||||
NOT port 80 which never hits the INPUT chain after PREROUTING rewrites it).
|
||||
"""
|
||||
captured: list[list[str]] = []
|
||||
|
||||
def _run(cmd, **kw):
|
||||
captured.append(list(cmd))
|
||||
# iptables -C (check) → rc=1 so the -A (add) branch executes
|
||||
if "iptables" in " ".join(str(x) for x in cmd) and "-C" in cmd:
|
||||
return _fail()
|
||||
return _ok()
|
||||
|
||||
# Patch Path.read_text so /proc/sys/net/ipv4/ip_forward is readable on any OS
|
||||
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
|
||||
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect), \
|
||||
patch("pathlib.Path.read_text", return_value="0\n"):
|
||||
|
||||
result = manager._setup_iptables_redirect()
|
||||
|
||||
assert result, "_setup_iptables_redirect must return True on success"
|
||||
|
||||
prerouting_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "PREROUTING" in c]
|
||||
assert prerouting_adds, "Expected 'iptables -A PREROUTING' invocation"
|
||||
pr_str = " ".join(prerouting_adds[0])
|
||||
assert "--dport" in pr_str and "80" in pr_str, "PREROUTING rule must match dport 80"
|
||||
assert "5000" in pr_str, "PREROUTING rule must redirect to port 5000"
|
||||
assert "REDIRECT" in pr_str, "PREROUTING rule must use REDIRECT target"
|
||||
|
||||
input_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "INPUT" in c]
|
||||
assert input_adds, "Expected 'iptables -A INPUT' invocation"
|
||||
in_str = " ".join(input_adds[0])
|
||||
assert "5000" in in_str, "INPUT rule must accept port 5000 (post-PREROUTING destination)"
|
||||
assert "ACCEPT" in in_str, "INPUT rule must use ACCEPT target"
|
||||
|
||||
# Port 80 must NOT be used in the INPUT rule (it is already redirected by PREROUTING)
|
||||
input_80 = [c for c in captured if "iptables" in " ".join(c) and "INPUT" in c and "--dport" in c and "80" in c]
|
||||
assert not input_80, "INPUT rule must target port 5000, not port 80"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3a. iptables rules and ip_forward reverted on teardown
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_iptables_rules_and_ip_forward_reverted_on_teardown(manager: WiFiManager) -> None:
|
||||
"""
|
||||
_teardown_iptables_redirect must:
|
||||
- remove the PREROUTING and INPUT iptables rules, and
|
||||
- restore ip_forward to the exact value recorded in the save file.
|
||||
"""
|
||||
original_fwd = "0"
|
||||
manager._IP_FORWARD_SAVE_PATH.write_text(original_fwd)
|
||||
# Teardown dispatches on the backend recorded during setup
|
||||
manager._redirect_backend = "iptables"
|
||||
|
||||
captured: list[list[str]] = []
|
||||
|
||||
with patch("src.wifi_manager.subprocess.run",
|
||||
side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
|
||||
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
|
||||
|
||||
manager._teardown_iptables_redirect()
|
||||
|
||||
assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "PREROUTING" in c], \
|
||||
"Expected 'iptables -D PREROUTING' invocation"
|
||||
|
||||
assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "INPUT" in c], \
|
||||
"Expected 'iptables -D INPUT' invocation"
|
||||
|
||||
restore_calls = [
|
||||
c for c in captured
|
||||
if "sysctl" in " ".join(c) and f"ip_forward={original_fwd}" in " ".join(c)
|
||||
]
|
||||
assert restore_calls, f"Expected sysctl to restore ip_forward to {original_fwd!r}"
|
||||
|
||||
assert not manager._IP_FORWARD_SAVE_PATH.exists(), \
|
||||
"Save file must be removed after successful teardown"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3b. ip_forward untouched when no save file exists
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_ip_forward_not_restored_when_save_file_absent(manager: WiFiManager) -> None:
|
||||
"""
|
||||
When the save file is missing (setup never wrote it, e.g. because /proc was
|
||||
unreadable or the write failed), teardown must NOT call sysctl so it does not
|
||||
accidentally clobber ip_forward state owned by a VPN or NetworkManager.
|
||||
"""
|
||||
assert not manager._IP_FORWARD_SAVE_PATH.exists()
|
||||
|
||||
captured: list[list[str]] = []
|
||||
|
||||
with patch("src.wifi_manager.subprocess.run",
|
||||
side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
|
||||
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
|
||||
|
||||
manager._teardown_iptables_redirect()
|
||||
|
||||
sysctl_calls = [
|
||||
c for c in captured
|
||||
if "sysctl" in " ".join(c) and "ip_forward" in " ".join(c)
|
||||
]
|
||||
assert not sysctl_calls, \
|
||||
"sysctl must not be called when no ip_forward save file exists"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. LED message content
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_led_message_shows_ssid_no_password_and_url(manager: WiFiManager) -> None:
|
||||
"""
|
||||
When the nmcli AP activates, the LED message must include:
|
||||
- the AP SSID ('LEDMatrix-Setup')
|
||||
- the string 'No password'
|
||||
- the AP IP address (192.168.4.1) and Flask port (5000)
|
||||
"""
|
||||
led_messages: list[str] = []
|
||||
|
||||
with patch("src.wifi_manager.subprocess.run", return_value=_ok()), \
|
||||
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
|
||||
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
|
||||
patch.object(manager, "_get_ap_status_nmcli",
|
||||
return_value={"active": True, "ip": "192.168.4.1"}), \
|
||||
patch.object(manager, "_show_led_message",
|
||||
side_effect=lambda msg, **kw: led_messages.append(msg)):
|
||||
|
||||
success, _ = manager._enable_ap_mode_nmcli_hotspot()
|
||||
|
||||
assert success, "AP enable should report success"
|
||||
assert led_messages, "Expected at least one _show_led_message call"
|
||||
|
||||
combined = "\n".join(led_messages)
|
||||
assert "No password" in combined, "LED message must say 'No password'"
|
||||
assert "LEDMatrix-Setup" in combined, "LED message must include the AP SSID"
|
||||
assert "192.168.4.1" in combined, "LED message must include the AP IP address"
|
||||
assert "5000" in combined, "LED message must include the Flask port"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 5. Stale AP profiles deleted before the new one is created
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_existing_ap_profiles_deleted_before_new_profile_created(manager: WiFiManager) -> None:
|
||||
"""
|
||||
Before 'nmcli connection add', the manager must issue
|
||||
'nmcli connection down/delete' for every known AP profile name so stale
|
||||
profiles (from a previous crash or partial setup) cannot block the new one.
|
||||
"""
|
||||
captured: list[list[str]] = []
|
||||
|
||||
def _run(cmd, **kw):
|
||||
captured.append(list(cmd))
|
||||
return _ok()
|
||||
|
||||
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
|
||||
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
|
||||
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
|
||||
patch.object(manager, "_get_ap_status_nmcli",
|
||||
return_value={"active": True, "ip": "192.168.4.1"}), \
|
||||
patch.object(manager, "_show_led_message"):
|
||||
|
||||
success, _ = manager._enable_ap_mode_nmcli_hotspot()
|
||||
|
||||
assert success
|
||||
|
||||
cmd_strs = [" ".join(c) for c in captured]
|
||||
|
||||
for profile in ("LEDMatrix-Setup-AP", "Hotspot", "TickerSetup-AP"):
|
||||
assert any("connection delete" in s and profile in s for s in cmd_strs), \
|
||||
f"Expected 'nmcli connection delete {profile}' before creating the new profile"
|
||||
|
||||
add_indices = [i for i, s in enumerate(cmd_strs) if "connection add" in s]
|
||||
del_indices = [i for i, s in enumerate(cmd_strs) if "connection delete" in s]
|
||||
|
||||
assert add_indices, "Expected 'nmcli connection add' call"
|
||||
assert del_indices, "Expected 'nmcli connection delete' calls"
|
||||
assert max(del_indices) < min(add_indices), \
|
||||
"All connection deletions must complete before the new profile is created"
|
||||
Reference in New Issue
Block a user