diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index f6d7d5a1..763db185 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -1451,16 +1451,32 @@ function HotspotSettingsForm() { if (!form.device) { return; } + // Ignore out-of-order responses when the user switches devices quickly. + let cancelled = false; const fetchProtocols = async () => { try { const response = await getHotspotProtocols(form.device); - setProtocols(response.protocols || []); + if (cancelled) { + return; + } + const supported = response.protocols || []; + setProtocols(supported); + // Reset a selection the probed device does not support (e.g. WPA3 kept + // after switching to a WPA2-only adapter). + if (supported.length) { + setForm(f => supported.includes(f.protocol) ? f : {...f, protocol: supported[0]}); + } } catch (e) { - console.error('Failed to fetch hotspot protocols', e); - setProtocols([]); + if (!cancelled) { + console.error('Failed to fetch hotspot protocols', e); + setProtocols([]); + } } }; fetchProtocols(); + return () => { + cancelled = true; + }; }, [form.device]); const handleSave = async () => { diff --git a/app/src/components/admin/ControllerPage.test.js b/app/src/components/admin/ControllerPage.test.js index f41bebb7..ae374107 100644 --- a/app/src/components/admin/ControllerPage.test.js +++ b/app/src/components/admin/ControllerPage.test.js @@ -208,6 +208,19 @@ describe('ControllerPage', () => { }); }); + test('a saved protocol the device does not support is reset to a supported one', async () => { + const controllerApi = require('../../api/controller'); + controllerApi.getHotspotSettings.mockResolvedValue( + {device: 'wlan0', ssid: 'WROLPi', password: 'wrolpi hotspot', protocol: 'wpa3'}); + controllerApi.getHotspotProtocols.mockResolvedValue({device: 'wlan0', protocols: ['wpa2']}); + renderControllerPage(); + // The stale WPA3 selection is replaced by the only protocol the device supports. + await waitFor(() => { + expect(screen.getAllByText('WPA2 (most compatible)').length).toBeGreaterThan(0); + }); + expect(screen.queryByText('WPA3')).not.toBeInTheDocument(); + }); + test('hotspot protocol dropdown only offers WPA2 on a Raspberry Pi', async () => { const {getHotspotProtocols} = require('../../api/controller'); getHotspotProtocols.mockResolvedValue({device: 'wlan0', protocols: ['wpa2']}); diff --git a/controller/lib/admin.py b/controller/lib/admin.py index 681a14ad..e59e4bc7 100644 --- a/controller/lib/admin.py +++ b/controller/lib/admin.py @@ -476,18 +476,26 @@ def enable_hotspot() -> dict: if protocol == "wpa3": # The driver likely rejected SAE; revert so the hotspot still works. logger.warning("Failed to enable WPA3 hotspot, reverting to WPA2: %s", error) - subprocess.run( + revert_modify = subprocess.run( ["nmcli", "connection", "modify", "Hotspot", "802-11-wireless-security.key-mgmt", "wpa-psk", "802-11-wireless-security.pmf", "0"], capture_output=True, text=True, timeout=10, ) - subprocess.run( - ["nmcli", "connection", "up", "Hotspot"], - capture_output=True, text=True, timeout=30, - ) + revert_up = None + if revert_modify.returncode == 0: + revert_up = subprocess.run( + ["nmcli", "connection", "up", "Hotspot"], + capture_output=True, text=True, timeout=30, + ) + if revert_modify.returncode == 0 and revert_up.returncode == 0: + return {"success": False, + "error": f"WPA3 is not supported by {device}; hotspot reverted to WPA2: {error}"} + revert_error = (revert_modify.stderr if revert_modify.returncode != 0 else revert_up.stderr).strip() + logger.error("Reverting hotspot to WPA2 also failed: %s", revert_error) return {"success": False, - "error": f"WPA3 is not supported by {device}; hotspot reverted to WPA2: {error}"} + "error": f"WPA3 failed on {device} and falling back to WPA2 also failed;" + f" the hotspot may be down: {error}"} # WPA2 is what nmcli created the hotspot with anyway; not fatal. logger.warning("Failed to explicitly set WPA2 on hotspot: %s", error) diff --git a/controller/templates/index.html b/controller/templates/index.html index 0741cd53..325d4483 100644 --- a/controller/templates/index.html +++ b/controller/templates/index.html @@ -1464,14 +1464,19 @@ savedProtocol = savedProtocol || select.value; try { const response = await apiCall(`api/hotspot/protocols?device=${encodeURIComponent(device)}`); + // Ignore out-of-order responses when the user switches devices quickly. + if (document.getElementById('hotspot-device').value !== device) return; select.innerHTML = ''; - // Keep the saved protocol selectable even if the device does not report it. - const protocols = [...new Set([...(response.protocols || []), savedProtocol])].filter(Boolean); + // Keep the saved protocol only when probing found nothing (e.g. the saved + // device is unplugged); a successful probe is authoritative. + const supported = (response.protocols || []).filter(Boolean); + const protocols = supported.length ? supported : [savedProtocol].filter(Boolean); + const selected = protocols.includes(savedProtocol) ? savedProtocol : protocols[0]; for (const protocol of protocols) { const option = document.createElement('option'); option.value = protocol; option.textContent = hotspotProtocolLabels[protocol] || protocol; - option.selected = protocol === savedProtocol; + option.selected = protocol === selected; select.appendChild(option); } } catch (e) { diff --git a/controller/test/test_admin.py b/controller/test/test_admin.py index ef019334..33530533 100644 --- a/controller/test/test_admin.py +++ b/controller/test/test_admin.py @@ -730,6 +730,19 @@ class TestEnableHotspot: modify_calls = [c[0][0] for c in calls if 'modify' in c[0][0]] assert len(modify_calls) == 2 and "wpa-psk" in modify_calls[1] + def test_wpa3_failed_revert_is_reported(self): + """If reverting to WPA2 also fails, the error must not claim the hotspot was reverted.""" + ok = mock.Mock(returncode=0, stdout="", stderr="") + device_ready = mock.Mock(returncode=0, stdout="wlan0:disconnected\n", stderr="") + sae_failed = mock.Mock(returncode=1, stdout="", stderr="Error: SAE not supported") + revert_failed = mock.Mock(returncode=1, stdout="", stderr="Error: connection not found") + # radio, poll, hotspot, modify(sae) fails, revert modify fails. + result, _ = self._run_enable('wpa3', [ok, device_ready, ok, sae_failed, revert_failed]) + + assert result["success"] is False + assert "reverted" not in result["error"].lower() + assert "wpa3" in result["error"].lower() + def test_wpa2_modify_failure_still_succeeds(self): """A failure setting WPA2 explicitly is not fatal; nmcli already created a WPA2 hotspot.""" ok = mock.Mock(returncode=0, stdout="", stderr="")