fix(SignInPage): Ensure click handler and login popup occur in same tick

The current SignIn implementation is broken on Safari iOS by default, due to the combination of useEffect and the extra asynchronous login check that occurs before the authentication routine that causes the popup (instantPopup: true). This change refactors the internals of SignIn page to avoid useEffect, which was arguably confusing for implementors, and also avoid overloading the use of the poorly scoped autoSignIn variable.

Signed-off-by: Chris Carpita <ccarpita@butterflynetinc.com>

.changeset/hot-shirts-shake.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': minor
+---
+
+Fixed a popup-blocking bug affecting iOS Safari in SignInPage.tsx by ensuring that the popup occurs in the same tick as the tap/click

packages/core-components/src/layout/SignInPage/SignInPage.tsx

@@ -15,6 +15,7 @@
  */
 
 import {
+  BackstageIdentity,
   configApiRef,
   SignInPageProps,
   useApi,
@@ -91,62 +92,63 @@ export const SingleSignInPage = ({
   const authApi = useApi(provider.apiRef);
   const configApi = useApi(configApiRef);
 
-  const [autoShowPopup, setAutoShowPopup] = useState<boolean>(auto ?? false);
-  // Defaults to true so that an initial check for existing user session is made
-  const [retry, setRetry] = useState<{} | boolean | undefined>(undefined);
   const [error, setError] = useState<Error>();
+  const [loginCount, setLoginCount] = useState(0);
 
   // The SignIn component takes some time to decide whether the user is logged-in or not.
   // showLoginPage is used to prevent a glitch-like experience where the sign-in page is
   // displayed for a split second when the user is already logged-in.
   const [showLoginPage, setShowLoginPage] = useState<boolean>(false);
 
-  useEffect(() => {
-    const login = async () => {
-      try {
-        let identity;
+  type LoginOpts = { checkExisting?: boolean; showPopup?: boolean };
+  const login = async ({ checkExisting, showPopup }: LoginOpts) => {
+    setLoginCount(prev => prev + 1);
+    try {
+      let identity: BackstageIdentity | undefined;
+      if (checkExisting) {
         // Do an initial check if any logged-in session exists
         identity = await authApi.getBackstageIdentity({
           optional: true,
         });
-
-        // If no session exists, show the sign-in page
-        if (!identity && autoShowPopup) {
-          // Unless auto is set to true, this step should not happen.
-          // When user intentionally clicks the Sign In button, autoShowPopup is set to true
-          setShowLoginPage(true);
-          identity = await authApi.getBackstageIdentity({
-            instantPopup: true,
-          });
-        }
-
-        if (!identity) {
-          setShowLoginPage(true);
-          return;
-        }
-
-        const profile = await authApi.getProfile();
-        onResult({
-          userId: identity!.id,
-          profile: profile!,
-          getIdToken: () => {
-            return authApi
-              .getBackstageIdentity()
-              .then(i => i!.token ?? i!.idToken);
-          },
-          signOut: async () => {
-            await authApi.signOut();
-          },
-        });
-      } catch (err) {
-        // User closed the sign-in modal
-        setError(err);
-        setShowLoginPage(true);
       }
-    };
 
-    login();
-  }, [onResult, authApi, retry, autoShowPopup]);
+      // If no session exists, show the sign-in page
+      if (!identity && (showPopup || auto)) {
+        // Unless auto is set to true, this step should not happen.
+        // When user intentionally clicks the Sign In button, autoShowPopup is set to true
+        setShowLoginPage(true);
+        identity = await authApi.getBackstageIdentity({
+          instantPopup: true,
+        });
+      }
+
+      if (!identity) {
+        setShowLoginPage(true);
+        return;
+      }
+
+      const profile = await authApi.getProfile();
+      onResult({
+        userId: identity!.id,
+        profile: profile!,
+        getIdToken: () => {
+          return authApi
+            .getBackstageIdentity()
+            .then(i => i!.token ?? i!.idToken);
+        },
+        signOut: async () => {
+          await authApi.signOut();
+        },
+      });
+    } catch (err: any) {
+      // User closed the sign-in modal
+      setError(err);
+      setShowLoginPage(true);
+    }
+  };
+  if (loginCount === 0) {
+    login({ checkExisting: true });
+  }
 
   return showLoginPage ? (
     <Page themeId="home">
@@ -168,8 +170,7 @@ export const SingleSignInPage = ({
                   color="primary"
                   variant="outlined"
                   onClick={() => {
-                    setRetry({});
-                    setAutoShowPopup(true);
+                    login({ showPopup: true });
                   }}
                 >
                   Sign In