Skip to content

Commit

Permalink
Improve WiFi settings error handling (#5293)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
agners authored Sep 11, 2024
1 parent 549dddc commit 1ca2279
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 6 deletions.
10 changes: 6 additions & 4 deletions supervisor/dbus/network/setting/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
5 changes: 4 additions & 1 deletion supervisor/host/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,10 @@ async def apply_changes(
) 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:
Expand Down
26 changes: 25 additions & 1 deletion tests/api/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={
Expand All @@ -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(
Expand Down
13 changes: 13 additions & 0 deletions tests/dbus_service_mocks/network_manager.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -227,6 +228,18 @@ def AddAndActivateConnection(
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",
Expand Down

0 comments on commit 1ca2279

Please sign in to comment.