Re-authenticate on reconnect to recover from server-side session expiry#161
Open
l-qing wants to merge 2 commits into
Open
Re-authenticate on reconnect to recover from server-side session expiry#161l-qing wants to merge 2 commits into
l-qing wants to merge 2 commits into
Conversation
- Re-authenticate when a saved cookie has expired after a server restart - Persist refreshed cookie and CSRF token before reconnecting - Notify the user once when stored credentials are unavailable
- Prevent sleeping auto-reconnect loops from logging back in after a user disconnects - Return the disconnected state before refreshing the session or retrying the websocket - Keep intentional reconnects working because manual_reconnect() clears the flag first
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After the server restarts (or its session store is otherwise cleared), the desktop client can never reconnect on its own: it loops forever logging
scheme http is invalid, and even an explicit Disconnect → Connect from the tray does not help. The only way to recover today is to fully quit and relaunch the client.This PR makes the client detect an expired session and re-authenticate during (re)connect, so it recovers automatically. It also stops an unintended reconnect that happens when the user disconnects while an auto-reconnect is pending.
Symptom
Desktop client log, repeating roughly every 10s and never recovering:
Steps to reproduce
SessionRegistryImpl), so a restart invalidates every existingJSESSIONID.scheme http is invalidand never reconnects.Any server-side session invalidation reproduces this; a server restart is simply the easiest trigger.
Root cause
Two layers:
1. The client reuses a dead session on every reconnect path.
STOMPManager.connect()builds the WebSocket with the storedconfig.data["cookie"]and never re-authenticates. This holds for both auto-reconnect (_on_close→connect()) and manual reconnect (manual_reconnect()→connect()). Only launching the app re-authenticates, because onlyauthenticate_and_connect()performs a freshlogin(). That is exactly why "Disconnect + Connect" does not help but "restart the app" does.2. A dead session turns the handshake into an unrecoverable error.
Once the session is gone, Spring Security answers the WebSocket handshake with a
302redirect to the login page.websocket-clientfollows that redirect and callsparse_url()on the target, which is not aws/wssURL, so it raisesscheme http is invalid. (The scheme ishttp/httpsdepending on the server's TLS/reverse-proxy setup; any non-ws scheme fails the same way.) The attempt then times out and the loop repeats forever with the same dead cookie.Fix
1. Detect an expired session — new
RequestManager.is_session_valid().Probe an authenticated endpoint (
/csrf-token) withallow_redirects=False:200= alive,3xx= expired. Disabling redirect-following is the crucial detail — otherwiserequestssilently follows the302to the login page (which returns200) and a dead session looks alive. Network/other errors returnTrue, so transient connectivity issues fall through to the normal WebSocket retry path instead of forcing an unnecessary re-login.2. Re-authenticate before reconnecting — new
STOMPManager._try_relogin(), called fromconnect().When the session is expired and credentials are stored (Save Password), silently
login()and refresh the cookie/CSRF token in place, so the following handshake uses a live session and sync resumes with no user action. Without stored credentials, notify the user once ("session expired, please re-login") instead of looping silently. Because this runs on everyconnect()outside the initial login phase, both auto-reconnect and manual reconnect now recover — no app restart needed.3. Respect an explicit disconnect —
if self.disconnected: returnat the top ofconnect().connect()previously only checkedself.disconnectedafter opening the socket. An auto-reconnect thread that was already sleeping in_on_closewhen the user pressed Disconnect would wake and callconnect()anyway. With fix #2 the re-login makes that racing reconnect succeed, so it would connect and then immediately tear itself down (via the existing post-connectdisconnectedcheck), producing a visible connect/disconnect churn on an intentional disconnect. The early guard makes a user's Disconnect stick;manual_reconnect()clears the flag before calling, so intentional reconnects still work.Why this works
302 → scheme http is invaliddead end.websocket-clientinternals and of the redirect's scheme, so it is robust across reverse-proxy / TLS setups.Behavior notes
Testing
python -m py_compileon both changed files.is_session_valid()returnsFalsefor the stale cookie,login()returns a fresh cookie, andis_session_valid()then returnsTrue.Session had expired; silently re-logged in→Websocket connected), and that an explicit Disconnect no longer triggers a reconnect.