================================================================================ PROPOSAL major-functional-1 SSO re-auth expiry tick destroys the server-driven logout redirect ================================================================================ Branch : 11645a (task #11645), trunk base r16695 Files : src/com/goldencode/p2j/ui/client/driver/web/res/p2j.sso_reauth.js src/com/goldencode/p2j/ui/client/driver/web/res/p2j.socket.js Severity : MAJOR (functional) - confirmed in the challenge pass Category : regression introduced by this branch 1. PROBLEM -------------------------------------------------------------------------------- Nothing cancels the re-auth countdown when the session is ended by the server's own force-logout. The expiry tick therefore fires *into* the shutdown handshake and clobbers the logout redirect, leaving the user on a dead iframe instead of the login page. Before this branch the expiry path called cleanupReauthUi(true) and only replaced the document body, leaving the socket module intact to complete the MSG_QUIT redirect. The branch replaced that with beginInternalReload() + window.location.reload(), which unloads the document mid-handshake. 2. EVIDENCE / TRIGGER -------------------------------------------------------------------------------- The trigger is the ordinary "user ignores the overlay" case: a) SsoTokenManager.handleInvalidToken schedules the force-logout BEFORE it tells the client anything: SsoTokenManager.java:518 scheduler.schedule(() -> forceLogout(...), reauthTimeoutSec, SECONDS) SsoTokenManager.java:539 client.sendSsoReauth(reauthUrl, reauthTimeoutSec) The module sets reauthDeadline = Date.now() + timeoutSec * 1000 only on receipt, so the client deadline is always the server deadline plus delivery latency. The server therefore always wins, and the client tick always fires inside the shutdown window. reauthInFlight() is false when the user ignored the overlay, so the REAUTH_GRACE_MS branch does not apply. b) Nothing cancels countdownInterval on that path. cleanupReauthUi is the only thing that clears it, and it is reachable solely from this module's own success / failure listeners and btnLogout.onclick. p2j.socket.js touches the module only at :6037-6039 (handleReauth). There is no cleanup hook on MSG_SHUTTINGDOWN, MSG_QUIT or doRedirectToLogoutPage. c) The reload breaks the handshake two ways: - The browser's MSG_SHUTTINGDOWN ack is sent only from the .finally of requestDeleteAuthCookie (p2j.socket.js:3331-3338). The navigation discards it, so WebClientProtocol.quit()'s waitForResult burns the full clientResponseTimeout (default 5000 ms) and MSG_QUIT goes nowhere. - Even if MSG_QUIT arrived first, doRedirectToLogoutPage's window.top.location.replace(logoutPage) sits inside a fetch().then(); the reload aborts that pending top-level navigation. web_landing.html's 'CLOSE' handler only resets state and never navigates. d) The reloaded iframe cannot reach the login page either. web_landing.html:123 points it at the spawned client's own embedded web server, which shutdownServer() has just torn down, and beginInternalReload() deliberately suppresses exitTheApplication so isRequiredToRedirect() returns false and the reloaded page would not redirect either. Net effect: browser network-error page inside a dead iframe, ~5 s of needless server-side blocking, no logout redirect. 3. ROOT CAUSE -------------------------------------------------------------------------------- Two independent expiry mechanisms - the server's forceLogout and the client's countdown tick - race with no coordination, and the client one is now destructive (it navigates) where before it was inert (it swapped the body). The server is the authoritative one; the client tick must stand down when the server acts. 4. PROPOSED FIX -------------------------------------------------------------------------------- Two parts. Part A is the fix; Part B is defence in depth for orderings A cannot observe. PART A - give the shutdown path an explicit way to cancel the re-auth UI. A1. p2j.sso_reauth.js: export a cancellation entry point alongside the existing handleSsoReauth export. It must be synchronous and idempotent, because it is called from the shutdown path which cannot await: /** * Stand down the re-auth overlay because the session is ending by the * server's decision. Clears every timer, closes the popup and drops the * overlay without triggering the expiry reload. Idempotent. */ me.abortReauth = function() { reloadRequested = true; // makes the tick a no-op if it races if (countdownInterval) { clearInterval(countdownInterval); countdownInterval = null; } if (popupCloseCheckInterval) { clearInterval(popupCloseCheckInterval); popupCloseCheckInterval = null; } closeReauthPopup(); if (reauthMessageListener) { window.removeEventListener('message', reauthMessageListener); reauthMessageListener = null; } }; Setting reloadRequested is what closes the last window: the tick already checks it, so even a tick that fired between the server's decision and this call becomes a no-op. Note the overlay element itself is deliberately NOT removed - the user should keep seeing "session ended" text while the redirect completes. If the existing overlay text is unsuitable, swap it for a terminal message here rather than removing it. A2. p2j.socket.js: call it from both shutdown entry points, before either does anything that navigates or awaits: - in the MSG_SHUTTINGDOWN handler, ahead of requestDeleteAuthCookie() - in the MSG_QUIT handler, ahead of doRedirectToLogoutPage() Guard the call so it is safe when the SSO module is not loaded (non-SSO deployments do not include p2j.sso_reauth.js): if (p2j.sso_reauth && p2j.sso_reauth.abortReauth) { p2j.sso_reauth.abortReauth(); } PART B - make the tick refuse to act on a session that is already gone. In the expiry branch of the countdown tick, before beginInternalReload() and window.location.reload(), bail out when the session is finished: if (exitTheApplication || !p2j.socket.isConnected()) { // the server is already tearing the session down; its MSG_QUIT // redirect is authoritative, do not navigate over it return; } This needs a small read-only accessor on the socket module if none exists (isConnected() / getReadyState()); do not reach into its internals from the SSO module. 5. ALTERNATIVES CONSIDERED -------------------------------------------------------------------------------- (i) Add a grace margin to the client deadline so the server always fires first and the client tick never runs. Rejected as a primary fix: it makes the race quieter, not absent - the tick still runs and still navigates if the server's forceLogout is delayed or fails, which is exactly when the client-side expiry is supposed to help. Harmless as an addition to Part A if a margin is wanted for UX reasons. (ii) Have the server arm forceLogout at reauthTimeoutSec plus a grace, so the client's own expiry UX gets a chance to run first. This is a legitimate design change and may well be desirable, but it belongs to SsoTokenManager, changes observable session-lifetime behaviour, and does not remove the need for the cancel hook (the server still force-logs-out eventually). Out of scope here; raise separately if wanted. (iii) Revert the expiry path to the pre-branch showSessionExpiredPage behaviour. Rejected: the branch's stated goal (no prompt, no quit-on-reload, internal reload so a live session re-attaches) is sound. The defect is the missing coordination, not the internal reload itself. 6. RISK -------------------------------------------------------------------------------- Low. Part A adds one idempotent function and two guarded calls on paths that are already terminal. Part B only adds an early return. Watch for: abortReauth() must not throw when no overlay was ever shown (all four blocks are null-guarded), and must not be async - the MSG_SHUTTINGDOWN handler cannot await it without reintroducing a race. 7. TEST PLAN -------------------------------------------------------------------------------- 1. SSO deployment, short reauthTimeoutSec (e.g. 15 s). Trigger an invalid token, let the overlay appear, and IGNORE it. Expect: overlay stands down, the browser lands on the login page, and the server log shows quit() completing promptly rather than waiting out clientResponseTimeout. 2. Same, but click Re-login and complete the flow inside the window. Expect: no regression - session re-attaches, overlay cleared by the existing success path. 3. Same, but click Logout. Expect: existing logout behaviour unchanged. 4. Non-SSO deployment: confirm the guarded call in p2j.socket.js is a no-op and normal MSG_QUIT logout still redirects. 5. Reload the page manually while the overlay is up. Expect: no stray timers left behind (check for orphaned intervals in the console).