From c3984acff864ca88a079efb96324702cb5a22f96 Mon Sep 17 00:00:00 2001
From: Anselm McClain <atm@xh.io>
Date: Tue, 14 Jan 2025 10:30:37 -0800
Subject: [PATCH] Handle edge-case where OAuth redirect opens new tab instead
 (#3900)

* Handle edge-case where OAuth redirect opens new tab instead

- Fixes #3899

* Minor simplification
---
 security/BaseOAuthClient.ts         | 29 +++++++++++++++++++++++++++++
 security/authzero/AuthZeroClient.ts |  6 ++++--
 security/msal/MsalClient.ts         | 26 ++++++++++++--------------
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/security/BaseOAuthClient.ts b/security/BaseOAuthClient.ts
index 7c97893ed..1aeadf75c 100644
--- a/security/BaseOAuthClient.ts
+++ b/security/BaseOAuthClient.ts
@@ -4,8 +4,11 @@
  *
  * Copyright © 2025 Extremely Heavy Industries Inc.
  */
+import {br, fragment} from '@xh/hoist/cmp/layout';
 import {HoistBase, managed, XH} from '@xh/hoist/core';
+import {Icon} from '@xh/hoist/icon';
 import {action, makeObservable} from '@xh/hoist/mobx';
+import {never, wait} from '@xh/hoist/promise';
 import {Token, TokenMap} from '@xh/hoist/security/Token';
 import {Timer} from '@xh/hoist/utils/async';
 import {MINUTES, olderThan, ONE_MINUTE, SECONDS} from '@xh/hoist/utils/datetime';
@@ -285,6 +288,32 @@ export abstract class BaseOAuthClient<C extends BaseOAuthClientConfig<S>, S> ext
         window.history.replaceState(null, '', pathname + search);
     }
 
+    /** Call after requesting the provider library redirect the user away for auth. */
+    protected async maskAfterRedirect() {
+        // We expect the tab to unload momentarily and be redirected to the provider's login page.
+        // Wait here to ensure we mask the app while the browser processes the redirect...
+        await wait(10 * SECONDS);
+
+        // ...but also handle an observed edge case where the browser decided to open a new tab
+        // instead of redirecting the current one (https://github.com/xh/hoist-react/issues/3899).
+        // Show message below if/when user swaps back to the original tab, vs. endless spinner.
+        await XH.alert({
+            title: 'Auth / Redirect Error',
+            message: fragment(
+                'Authentication did not complete as expected / tab was not redirected.',
+                br(),
+                br(),
+                'Please click below or refresh this tab in your browser to try again.'
+            ),
+            confirmProps: {text: 'Reload', icon: Icon.refresh(), intent: 'primary', minimal: false}
+        });
+
+        XH.reloadApp();
+
+        // Ensure stale init does not progress - this tab should *really* be on its way out now!
+        await never();
+    }
+
     protected async fetchAllTokensAsync(useCache = true): Promise<TokenMap> {
         const ret: TokenMap = {},
             {accessSpecs} = this;
diff --git a/security/authzero/AuthZeroClient.ts b/security/authzero/AuthZeroClient.ts
index 34727360f..9a8f18dfb 100644
--- a/security/authzero/AuthZeroClient.ts
+++ b/security/authzero/AuthZeroClient.ts
@@ -7,7 +7,7 @@
 import type {Auth0ClientOptions} from '@auth0/auth0-spa-js';
 import {Auth0Client} from '@auth0/auth0-spa-js';
 import {XH} from '@xh/hoist/core';
-import {never, wait} from '@xh/hoist/promise';
+import {wait} from '@xh/hoist/promise';
 import {Token, TokenMap} from '@xh/hoist/security/Token';
 import {SECONDS} from '@xh/hoist/utils/datetime';
 import {mergeDeep, throwIf} from '@xh/hoist/utils/js';
@@ -99,11 +99,13 @@ export class AuthZeroClient extends BaseOAuthClient<AuthZeroClientConfig, AuthZe
 
     protected override async doLoginRedirectAsync(): Promise<void> {
         const appState = this.captureRedirectState();
+
         await this.client.loginWithRedirect({
             appState,
             authorizationParams: {scope: this.loginScope}
         });
-        await never();
+
+        await this.maskAfterRedirect();
     }
 
     protected override async doLoginPopupAsync(): Promise<void> {
diff --git a/security/msal/MsalClient.ts b/security/msal/MsalClient.ts
index 6f9548f6c..b6ea48c23 100644
--- a/security/msal/MsalClient.ts
+++ b/security/msal/MsalClient.ts
@@ -10,11 +10,9 @@ import {
     IPublicClientApplication,
     LogLevel,
     PopupRequest,
-    RedirectRequest,
     SilentRequest
 } from '@azure/msal-browser';
 import {XH} from '@xh/hoist/core';
-import {never} from '@xh/hoist/promise';
 import {Token, TokenMap} from '@xh/hoist/security/Token';
 import {logDebug, logError, logInfo, logWarn, mergeDeep, throwIf} from '@xh/hoist/utils/js';
 import {flatMap, union, uniq} from 'lodash';
@@ -201,18 +199,18 @@ export class MsalClient extends BaseOAuthClient<MsalClientConfig, MsalTokenSpec>
     }
 
     protected override async doLoginRedirectAsync(): Promise<void> {
-        const {client} = this,
-            state = this.captureRedirectState(),
-            opts: RedirectRequest = {
-                state,
-                loginHint: this.getSelectedUsername(),
-                domainHint: this.config.domainHint,
-                scopes: this.loginScopes,
-                extraScopesToConsent: this.loginExtraScopesToConsent,
-                redirectUri: this.redirectUrl
-            };
-        await client.acquireTokenRedirect(opts);
-        await never();
+        const state = this.captureRedirectState();
+
+        await this.client.acquireTokenRedirect({
+            state,
+            loginHint: this.getSelectedUsername(),
+            domainHint: this.config.domainHint,
+            scopes: this.loginScopes,
+            extraScopesToConsent: this.loginExtraScopesToConsent,
+            redirectUri: this.redirectUrl
+        });
+
+        await this.maskAfterRedirect();
     }
 
     protected override async fetchIdTokenAsync(useCache: boolean = true): Promise<Token> {