From 5a5ebe9ff17ae826624fd94238f289f2a4b11825 Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:20:08 +0200 Subject: [PATCH] fix(ui): late Admin login reply could be misrouted after giving up UITask::onRoomLoginResult() dispatches a login reply to whichever screen is *currently* shown (curr == admin_screen ? AdminScreen : MessagesScreen), not to whoever actually sent the request. Neither giving up path (manual Cancel, or the timeout added in 23f43cac) told MyMesh to stop tracking the request, so a reply that still arrived after the user had navigated away landed on whatever screen they'd moved to instead -- most likely MessagesScreen, which then persisted its own unrelated _login_pw as the "confirmed" password for that pubkey, silently corrupting the saved password even on a genuine success. Adds MyMesh::cancelUiPendingLogin(pub_key), pubkey-guarded so it's a no-op if a newer request has since overwritten ui_pending_login, called from both of AdminScreen's give-up paths. A late reply now simply matches nothing and is dropped. Co-Authored-By: Claude Sonnet 5 --- examples/companion_radio/MyMesh.h | 13 +++++++++++++ examples/companion_radio/ui-new/AdminScreen.h | 11 ++++++++++- release-notes.md | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/examples/companion_radio/MyMesh.h b/examples/companion_radio/MyMesh.h index 8e87ff43..07b41795 100644 --- a/examples/companion_radio/MyMesh.h +++ b/examples/companion_radio/MyMesh.h @@ -229,6 +229,19 @@ public: return true; } + // Called by a screen that gave up waiting on its own sendRoomLogin() (Cancel + // or a timeout) so a reply that arrives after doesn't get misrouted: + // UITask::onRoomLoginResult() dispatches by whichever screen is *currently* + // showing, not by who actually sent the request, so a late reply landing + // after the requester navigated away is delivered to whatever screen the + // user is on instead -- which then persists its own unrelated login state + // (e.g. a different screen's _login_pw) as if it were that reply's answer. + // Pubkey-guarded so this is a no-op if a newer request (this screen retrying, + // or a different screen entirely) has since overwritten ui_pending_login. + void cancelUiPendingLogin(const uint8_t* pub_key) { + if (ui_pending_login && memcmp(&ui_pending_login, pub_key, 4) == 0) ui_pending_login = 0; + } + // On-device UI logout: drops the local keep-alive tracking (mirrors the app's // CMD_LOGOUT) and forgets the saved password, so the next room open prompts // for credentials again instead of silently re-using them. No packet is sent diff --git a/examples/companion_radio/ui-new/AdminScreen.h b/examples/companion_radio/ui-new/AdminScreen.h index 42164184..ac7dc54a 100644 --- a/examples/companion_radio/ui-new/AdminScreen.h +++ b/examples/companion_radio/ui-new/AdminScreen.h @@ -436,6 +436,11 @@ public: void poll() override { if (_phase == LOGIN && _login_waiting && (int32_t)(millis() - _login_deadline_ms) >= 0) { _login_waiting = false; + // Stop tracking this request on the MyMesh side too -- otherwise a reply + // that still arrives after we've given up gets misrouted to whatever + // screen the user is on by then (see cancelUiPendingLogin()'s comment), + // corrupting that screen's unrelated login state with our answer. + the_mesh.cancelUiPendingLogin(_target.id.pub_key); // No response at all is ambiguous (could be a wrong/stale password, could // just be out of range) -- but a saved password that's gone stale (the // remote's password changed) is exactly this: silence, not a nack. Treat @@ -515,7 +520,11 @@ public: bool handleInput(char c) override { if (_phase == LOGIN) { if (_login_waiting) { - if (c == KEY_CANCEL) { _login_waiting = false; returnToOrigin(); } + if (c == KEY_CANCEL) { + _login_waiting = false; + the_mesh.cancelUiPendingLogin(_target.id.pub_key); // see cancelUiPendingLogin()'s comment + returnToOrigin(); + } return true; } auto r = kb().handleInput(c); diff --git a/release-notes.md b/release-notes.md index c0ac88e3..dfa003a6 100644 --- a/release-notes.md +++ b/release-notes.md @@ -4,6 +4,7 @@ - **Tools › Admin login could get stuck on "Logging in..." forever** if the remote node never sent back a reply — most commonly after its admin/room password was changed, since the on-device UI auto-retries the old saved password with no timeout to fall back on. It now gives up after the same estimated-timeout window the rest of Admin's commands use, forgets the stale saved password, and returns you to the picker to try again with the new one. - **Changing a remote's admin password from Tools › Admin › System didn't update this device's own saved copy** — the very next login attempt to that node retried the password you'd just replaced, hitting the "stuck on Logging in…" case above. The confirmation the remote echoes back is now saved as the new on-device password to match. +- **A slow-to-arrive Admin login reply, after you'd already given up on it (Cancel or the new timeout above), could get delivered to whatever screen you'd since moved to and silently overwrite your saved password for that node with unrelated data.** Giving up on a login now also stops tracking it mesh-side, so a late reply is simply dropped instead of misrouted. ---