FWD » Core Development » Runtime Infrastructure

================================================================================================

PROPOSAL - [MINOR] functional - p2j.sso_reauth.js handleSsoReauth

The expiry reload terminates the session it claims to re-attach to

================================================================================================

REVIEW FINDING (abridged)

   "The countdown-expiry window.location.reload() cannot 're-attach to a session the server

   re-authenticated' in the GUI web client [...] window.onpagehide [...] writes

   {exitTheApplication:true, date:now} to sessionStorage; on the reloaded page handleOpenEvent calls

   isRequiredToRedirect(), true for the whole watchdogTimeout window [...] and runs

   sendNotification(types.MSG_QUIT); doRedirectToLogoutPage();."

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

------------------------------------------------------------------------------------------------

1. ROOT CAUSE

------------------------------------------------------------------------------------------------

p2j.socket.js maintains an invariant that predates this change: in the GUI client, a page unload

that is not an explicit logout means "the user is closing or reloading the application", and the

next page to open within watchdogTimeout must quit the session rather than adopt it. That is

implemented by window.onpagehide (p2j.socket.js:6861) writing the exitTheApplication object to

sessionStorage, and isRequiredToRedirect() (:6419) reading it back:

   if (exit && (date < (exit.date + watchdogTimeout)))   // 120000 ms configured, 40 min in debug

   {

      return true;

   }

handleOpenEvent then sends MSG_QUIT and calls doRedirectToLogoutPage().

The new expiry branch calls window.location.reload() without opting out of that invariant, so it

inherits it: the reload is indistinguishable from a user-initiated one. The guard added at the top

of handleOpenEvent does not help, because exitTheApplication is a plain module variable that is a

fresh false on the reloaded document - the surviving state is the sessionStorage object, not the

variable.

The state this exists to rescue is genuinely reachable: SsoTokenManager.processCallbackReauthentication

clears reauthInProgress and cancels reauthTimeoutFuture *before* the callback page is emitted, and

VirtualDesktopWebHandler's notification is best-effort -

try{window.opener.postMessage(...)}catch(e){} followed by window.close() - so a COOP header or an

early popup close loses it while the session is alive. The grace period does not cover that case

either: the callback page's window.close() makes reauthPopup.closed true and

popupCloseCheckInterval nulls reauthMessageListener, so reauthInFlight() is false and the reload

fires immediately.

Severity is MINOR because the deleted showSessionExpiredPage was no better here - it wiped the DOM

but left the socket open, orphaning the re-authenticated session. The user lost it either way. What

is new is a comment and javadoc asserting a recovery the code cannot perform.

------------------------------------------------------------------------------------------------

2. PROPOSED FIX

------------------------------------------------------------------------------------------------

Two options. 2.1 makes the code match its comment and is the smaller change; 2.2 is the honest

long-term design. Doing neither and only correcting the javadoc (2.3) is an acceptable minimum.

2.1 Opt this one reload out of the quit-on-reload invariant

-------------------------------------------------------------

Add an explicit, narrowly scoped signal to p2j.socket, beside the other exported helpers:

   +  /**

   +   * Mark the imminent page unload as an internal reload rather than the user leaving, so the

   +   * reloaded page adopts the existing session instead of quitting it.

   +   * <p>

   +   * Used by the SSO re-auth overlay, whose expiry reload exists precisely to re-attach to a

   +   * session the server may have re-authenticated.  Also suppresses the leave-confirmation, which

   +   * would otherwise prompt the user about a reload they did not ask for.

   +   */

   +  me.beginInternalReload = function()

   +  {

   +     internalReload = true;

   +  };

with a module variable

   +  /** True while an internally requested reload is in flight; see me.beginInternalReload. */

   +  var internalReload = false;

and honoured in onpagehide (:6861):

         if (!exitTheApplication)

         {

   +        if (internalReload)

   +        {

   +           // an internal reload re-attaches to the same session; do not arm the quit-on-reload

   +           // path that a user-initiated reload or a tab close arms

   +           return;

   +        }

            // close or reload cases

            exitTheApplication = true;

            p2j.saveObject("exitTheApplication", { ... }, false);

         }

and in the expiry branch of p2j.sso_reauth.js, immediately before the reload:

   +        p2j.socket.beginInternalReload();

            countdownDiv.textContent = 'Confirming sign-in...';

            window.location.reload();

The same flag is what suppresses the leave-confirmation - see

proposal-issue-minor-functional-3, which owns that half and should land with this one.

With 2.1 the comment at :247-254 becomes true: a live session re-attaches on the reconnect, a killed

one is rejected at the websocket upgrade and lands on the login page.

2.2 Preferred alternative: ask the server instead of reloading

----------------------------------------------------------------

The socket is still open and the server already knows the answer - the entry's reauthInProgress is

false once processCallbackReauthentication has run. A small round trip (reuse the existing

notification mechanism, or a dedicated MSG_SSO_REAUTH response) would let the overlay simply

disappear on success and go to the login page on failure, with no page reload, no lost canvas state

and no dependence on sessionStorage semantics. That is a larger change and touches SsoTokenManager,

so it is recorded here as the direction rather than proposed as the patch.

2.3 Correct the documentation either way

------------------------------------------

If neither 2.1 nor 2.2 is taken, the comment at p2j.sso_reauth.js:247-254 and the two javadoc blocks

at :84-86 and :158-160 must stop claiming a re-attach. Replace with a statement of what actually

happens: the reload ends the session and lands on the login page, which is the safe outcome of the

two but is not recovery.

------------------------------------------------------------------------------------------------

3. RELATIONSHIP TO OTHER PROPOSALS

------------------------------------------------------------------------------------------------

   proposal-issue-minor-functional-3   the leave-confirmation prompt on the same reload; shares the

                                       internalReload flag introduced in 2.1.  Land together.

   proposal-issue-minor-functional-4   closing the orphaned popup before the same reload.

All three touch the same five lines of the expiry branch; apply them as one edit.

------------------------------------------------------------------------------------------------

4. RISK AND VERIFICATION

------------------------------------------------------------------------------------------------

Risk: moderate for 2.1, because it puts a hole in a long-standing invariant. The hole is narrow -

one caller, set only immediately before an unload the page itself requested, and never reset (the

document is going away) - but a future caller that sets it and then does not unload would leave a

GUI page that no longer arms quit-on-reload. Consider naming it to discourage that, and asserting in

onpagehide that it is only ever observed once.

Verification

   1. Reproduce the target case: trigger SSO re-auth, click Re-login, complete the IdP flow in the

      popup, and kill the postMessage (close the popup manually the instant it lands on /reauthcb,

      or serve the callback with a COOP header). Let the countdown expire. Expected with 2.1: the

      page reloads and comes back into the *same* live session; pre-fix it logs out.

   2. Let the countdown expire with no re-login attempt at all. Expected: the server force-logout

      has already fired, the websocket upgrade is rejected, and the page lands on the login page.

   3. Confirm the invariant is intact for ordinary use: F5 on a GUI page, and closing the tab, must

      both still quit the session (internalReload is false on those paths).

   4. Confirm the CHUI client is unaffected - onpagehide returns early on !p2j.isGui, and

      ChuiWebPageHandler emits isGui=false.