From 1ca22799d1c38e9e3e0e0bb14ebc2fc4c809458f Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 11 Sep 2024 17:53:36 +0200 Subject: [PATCH] Improve WiFi settings error handling (#5293) * Improve WiFi settings error handling Currently, the frontend potentially provides no WiFi settings dictionary but still tries to update other (IP address) settings on the interface. This leads to a stack trace since network manager is not able to fetch the WiFi settings from the settings dictionary. Simply fill out what we can and let NetworkManager provide an error. Also allow to disable a network interface which has no configuration. This avoids an error when switching to auto and back to disabled then press save on a new wireless network interface. * Add debug message when already disabled * Add pytest for incomplete WiFi settings as psoted by frontend Simulate the frontend posting no WiFi settings. Make sure the Supervisor handles this gracefully. --- supervisor/dbus/network/setting/generate.py | 10 ++++---- supervisor/host/network.py | 5 +++- tests/api/test_network.py | 26 ++++++++++++++++++++- tests/dbus_service_mocks/network_manager.py | 13 +++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/supervisor/dbus/network/setting/generate.py b/supervisor/dbus/network/setting/generate.py index 0f59fc53f..554c5b230 100644 --- a/supervisor/dbus/network/setting/generate.py +++ b/supervisor/dbus/network/setting/generate.py @@ -199,15 +199,17 @@ def get_connection_from_interface( elif interface.type == InterfaceType.WIRELESS: wireless = { CONF_ATTR_802_WIRELESS_ASSIGNED_MAC: Variant("s", "preserve"), - CONF_ATTR_802_WIRELESS_SSID: Variant( - "ay", interface.wifi.ssid.encode("UTF-8") - ), CONF_ATTR_802_WIRELESS_MODE: Variant("s", "infrastructure"), CONF_ATTR_802_WIRELESS_POWERSAVE: Variant("i", 1), } + if interface.wifi and interface.wifi.ssid: + wireless[CONF_ATTR_802_WIRELESS_SSID] = Variant( + "ay", interface.wifi.ssid.encode("UTF-8") + ) + conn[CONF_ATTR_802_WIRELESS] = wireless - if interface.wifi.auth != "open": + if interface.wifi and interface.wifi.auth != "open": wireless["security"] = Variant("s", CONF_ATTR_802_WIRELESS_SECURITY) wireless_security = {} if interface.wifi.auth == "wep": diff --git a/supervisor/host/network.py b/supervisor/host/network.py index 57e288b12..ef3aa3dea 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -236,7 +236,10 @@ class NetworkManager(CoreSysAttributes): ) from err # Remove config from interface - elif inet and inet.settings and not interface.enabled: + elif inet and not interface.enabled: + if not inet.settings: + _LOGGER.debug("Interface %s is already disabled.", interface.name) + return try: await inet.settings.delete() except DBusError as err: diff --git a/tests/api/test_network.py b/tests/api/test_network.py index 5d5f2cfe3..309dccc71 100644 --- a/tests/api/test_network.py +++ b/tests/api/test_network.py @@ -234,7 +234,7 @@ async def test_api_network_interface_update_ethernet( async def test_api_network_interface_update_wifi(api_client: TestClient): - """Test network manager api.""" + """Test network interface WiFi API.""" resp = await api_client.post( f"/network/interface/{TEST_INTERFACE_WLAN_NAME}/update", json={ @@ -252,6 +252,30 @@ async def test_api_network_interface_update_wifi(api_client: TestClient): assert result["result"] == "ok" +async def test_api_network_interface_update_wifi_error(api_client: TestClient): + """Test network interface WiFi API error handling.""" + # Simulate frontend WiFi interface edit where the user did not select + # a WiFi SSID. + resp = await api_client.post( + f"/network/interface/{TEST_INTERFACE_WLAN_NAME}/update", + json={ + "enabled": True, + "ipv4": { + "method": "auto", + }, + "ipv6": { + "method": "auto", + }, + }, + ) + result = await resp.json() + assert result["result"] == "error" + assert ( + result["message"] + == "Can't create config and activate wlan0: A 'wireless' setting with a valid SSID is required if no AP path was given." + ) + + async def test_api_network_interface_update_remove(api_client: TestClient): """Test network manager api.""" resp = await api_client.post( diff --git a/tests/dbus_service_mocks/network_manager.py b/tests/dbus_service_mocks/network_manager.py index b5191e0d7..c0ebaeab8 100644 --- a/tests/dbus_service_mocks/network_manager.py +++ b/tests/dbus_service_mocks/network_manager.py @@ -1,5 +1,6 @@ """Mock of Network Manager service.""" +from dbus_fast import DBusError from dbus_fast.service import PropertyAccess, dbus_property, signal from .base import DBusServiceMock, dbus_method @@ -227,6 +228,18 @@ class NetworkManager(DBusServiceMock): self, connection: "a{sa{sv}}", device: "o", speciic_object: "o" ) -> "oo": """Do AddAndActivateConnection method.""" + if connection["connection"]["type"].value == "802-11-wireless": + if "802-11-wireless" not in connection: + raise DBusError( + "org.freedesktop.NetworkManager.Device.InvalidConnection", + "A 'wireless' setting is required if no AP path was given.", + ) + if "ssid" not in connection["802-11-wireless"]: + raise DBusError( + "org.freedesktop.NetworkManager.Device.InvalidConnection", + "A 'wireless' setting with a valid SSID is required if no AP path was given.", + ) + return [ "/org/freedesktop/NetworkManager/Settings/1", "/org/freedesktop/NetworkManager/ActiveConnection/1",