From 29df6a8a7d3d6c2ed4cb5300317bbf5e7fe701e5 Mon Sep 17 00:00:00 2001 From: Rob Knight Date: Thu, 26 Sep 2024 18:27:19 +0100 Subject: [PATCH] Working error messages for failure cases --- .../LoginScreens/LoginInterstitialScreen.tsx | 2 - .../ZappScreens/AuthenticateIFrameScreen.tsx | 5 +- .../ZappScreens/ConnectPopupScreen.tsx | 190 ++++++++++++------ examples/test-zapp/src/utils.ts | 2 +- examples/test-zapp/src/vite-env.d.ts | 9 + test-packaging/vite/src/vite-env.d.ts | 1 - turbo.json | 2 +- 7 files changed, 147 insertions(+), 64 deletions(-) create mode 100644 examples/test-zapp/src/vite-env.d.ts delete mode 100644 test-packaging/vite/src/vite-env.d.ts diff --git a/apps/passport-client/components/screens/LoginScreens/LoginInterstitialScreen.tsx b/apps/passport-client/components/screens/LoginScreens/LoginInterstitialScreen.tsx index 6e7d50cbc7..e4dc3f6867 100644 --- a/apps/passport-client/components/screens/LoginScreens/LoginInterstitialScreen.tsx +++ b/apps/passport-client/components/screens/LoginScreens/LoginInterstitialScreen.tsx @@ -17,10 +17,8 @@ export function LoginInterstitialScreen(): JSX.Element { const loadedIssuedPCDs = useLoadedIssuedPCDs(); useLayoutEffect(() => { - console.log("loadedIssuedPCDs", loadedIssuedPCDs); if (loadedIssuedPCDs) { const pendingRequest = getPendingRequest(); - console.log("pendingRequest", pendingRequest); if (pendingRequest) { switch (pendingRequest.key) { case "proof": { diff --git a/apps/passport-client/components/screens/ZappScreens/AuthenticateIFrameScreen.tsx b/apps/passport-client/components/screens/ZappScreens/AuthenticateIFrameScreen.tsx index 887eb5e48f..82283450da 100644 --- a/apps/passport-client/components/screens/ZappScreens/AuthenticateIFrameScreen.tsx +++ b/apps/passport-client/components/screens/ZappScreens/AuthenticateIFrameScreen.tsx @@ -4,7 +4,7 @@ import { pendingRequestKeys } from "../../../src/sessionStorage"; import { Spacer } from "../../core"; import { RippleLoader } from "../../core/RippleLoader"; import { AppContainer } from "../../shared/AppContainer"; -import { AuthMessage } from "./ConnectPopupScreen"; +import { IFrameAuthenticationMessage } from "./ConnectPopupScreen"; export function AuthenticateIFrameScreen(): ReactNode { const encryptionKey = useSyncKey(); @@ -21,7 +21,6 @@ export function AuthenticateIFrameScreen(): ReactNode { }, []); useEffect(() => { - console.log("AuthenticateIFrameScreen", isLegitimateOpener); if (isLegitimateOpener && encryptionKey) { const chan = new MessageChannel(); chan.port1.onmessage = (): void => { @@ -36,7 +35,7 @@ export function AuthenticateIFrameScreen(): ReactNode { { type: "auth", encryptionKey: encryptionKey as string - } satisfies AuthMessage, + } satisfies IFrameAuthenticationMessage, origin, [chan.port2] ); diff --git a/apps/passport-client/components/screens/ZappScreens/ConnectPopupScreen.tsx b/apps/passport-client/components/screens/ZappScreens/ConnectPopupScreen.tsx index 3ee5305a51..02fb14868f 100644 --- a/apps/passport-client/components/screens/ZappScreens/ConnectPopupScreen.tsx +++ b/apps/passport-client/components/screens/ZappScreens/ConnectPopupScreen.tsx @@ -1,28 +1,32 @@ import { requestDownloadAndDecryptStorage } from "@pcd/passport-interface"; import { Button, Spacer } from "@pcd/passport-ui"; -import { ReactNode, useCallback, useState } from "react"; +import { ReactNode, useCallback, useEffect, useRef, useState } from "react"; +import styled from "styled-components"; import urljoin from "url-join"; import * as v from "valibot"; import { appConfig } from "../../../src/appConfig"; import { useDispatch } from "../../../src/appHooks"; +import { useSelector } from "../../../src/subscribe"; import { H1, TextCenter } from "../../core"; import { AppContainer } from "../../shared/AppContainer"; import { Spinner } from "../../shared/Spinner"; -const AuthMessage = v.object({ +const IFrameAuthenticationMessageSchema = v.object({ type: v.literal("auth"), encryptionKey: v.string() }); -export type AuthMessage = v.InferOutput; +export type IFrameAuthenticationMessage = v.InferOutput< + typeof IFrameAuthenticationMessageSchema +>; enum PopupAuthenticationStatus { - Start, - PopupOpen, - PopupBlocked, - PopupClosed, - Authenticating, - AuthenticationError + Start = "Start", + PopupOpen = "PopupOpen", + PopupBlocked = "PopupBlocked", + PopupClosed = "PopupClosed", + Authenticating = "Authenticating", + AuthenticationError = "AuthenticationError" } /** @@ -35,13 +39,12 @@ enum PopupAuthenticationStatus { * {@link useZappServer} which will tell the Zapp to close the modal window. */ export function ConnectPopupScreen(): ReactNode { - const [error, setError] = useState(null); const [status, setStatus] = useState( PopupAuthenticationStatus.Start ); const dispatch = useDispatch(); const tryToLogin = useCallback( - async (encryptionKey: string) => { + async (encryptionKey: string): Promise => { // Try to download and decrypt the storage const storageRequest = await requestDownloadAndDecryptStorage( appConfig.zupassServer, @@ -54,16 +57,95 @@ export function ConnectPopupScreen(): ReactNode { storage: storageRequest.value, encryptionKey }); + return true; } else { - // Something unexpected went wrong - setError( - "Unable to log in automatically, please enter your email to log in" - ); + return false; } }, [dispatch] ); + const messageListenerAbortRef = useRef(null); + const popupRef = useRef(null); + + useEffect(() => { + let closeCheckInterval: number; + let closeNotifyTimeout: number; + if (status === PopupAuthenticationStatus.PopupOpen) { + closeCheckInterval = window.setInterval(() => { + // The user may close the popup without authenticating, in which case we + // need to show a message to the user telling them not to do this. + // There is no reliable event for detecting this, so we have to check + // using a timer. + if (popupRef.current && popupRef.current.closed) { + clearInterval(closeCheckInterval); + // Race conditions are a risk here, so we wait 250ms to ensure that + // the popup has had a chance to send a message. + closeNotifyTimeout = window.setTimeout(() => { + if (status === PopupAuthenticationStatus.PopupOpen) { + setStatus(PopupAuthenticationStatus.PopupClosed); + messageListenerAbortRef.current?.abort(); + } + }, 250); + } + }, 250); + return () => { + if (closeCheckInterval) { + clearInterval(closeCheckInterval); + } + if (closeNotifyTimeout) { + clearTimeout(closeNotifyTimeout); + } + }; + } + }, [status]); + + useEffect(() => { + if ( + status === PopupAuthenticationStatus.PopupOpen || + status === PopupAuthenticationStatus.PopupBlocked + ) { + // If the user closes the popup, we need to abort the message listener + const messageListenerAbort = new AbortController(); + + // If we already have a message listener, abort it + if (messageListenerAbortRef.current) { + messageListenerAbortRef.current.abort(); + } + messageListenerAbortRef.current = messageListenerAbort; + + window.addEventListener( + "message", + async (event) => { + if (event.origin !== window.origin) { + return; + } + + // Check if the message is an authentication message + const parsed = v.safeParse( + IFrameAuthenticationMessageSchema, + event.data + ); + if ( + parsed.success && + status === PopupAuthenticationStatus.PopupOpen + ) { + setStatus(PopupAuthenticationStatus.Authenticating); + // Sending this message back to the iframe lets it know that + // we've received the authentication message and it's okay to + // close + event.ports[0].postMessage("ACK"); + const loginResult = await tryToLogin(parsed.output.encryptionKey); + if (!loginResult) { + setStatus(PopupAuthenticationStatus.AuthenticationError); + } + } + }, + { signal: messageListenerAbort.signal } + ); + } + }, [status, tryToLogin]); + const openPopup = useCallback(() => { const popup = window.open( urljoin(window.origin, "/#/authenticate-iframe"), @@ -71,64 +153,60 @@ export function ConnectPopupScreen(): ReactNode { "width=500,height=500,popup=true" ); if (!popup) { + // Although the popup was blocked, the user may cause it to open by + // allowing the browser to open it, so we should continue to set up + // the message listener. setStatus(PopupAuthenticationStatus.PopupBlocked); - return; + } else { + setStatus(PopupAuthenticationStatus.PopupOpen); + popupRef.current = popup; } - - // If the user closes the popup, we need to abort the message listener - const abortReceiveMessage = new AbortController(); - - const interval = window.setInterval(() => { - // The user may close the popup without authenticating, in which case we - // need to show a message to the user telling them not to do this. - // There is no reliable event for detecting this, so we have to check - // using a timer. - if (popup.closed) { - setStatus(PopupAuthenticationStatus.PopupClosed); - clearInterval(interval); - // Race conditions are a risk here, so we wait 250ms to ensure that - // the popup has had a chance to send a message. - window.setTimeout(() => { - abortReceiveMessage.abort(); - }, 250); - } - }, 250); - - window.addEventListener( - "message", - (event) => { - console.log("message", event); - if (event.origin !== window.origin || event.source !== popup) { - return; - } - - // Check if the message is an authentication message - const parsed = v.safeParse(AuthMessage, event.data); - if (parsed.success) { - event.ports[0].postMessage("ACK"); - tryToLogin(parsed.output.encryptionKey); - } - }, - { signal: abortReceiveMessage.signal } - ); - }, [tryToLogin]); + }, []); const inProgress = status === PopupAuthenticationStatus.PopupOpen || status === PopupAuthenticationStatus.Authenticating; + const zappName = useSelector((state) => state.connectedZapp?.name); + const zappOrigin = useSelector((state) => state.zappOrigin); + return (

ZUPASS

+ + {zappName} ({zappOrigin}) would like to connect + to your Zupass. + + - - {error && {error}} + + {status === PopupAuthenticationStatus.PopupBlocked && ( + + Your browser may be configured to block popup windows. Please check + your browser settings and click the button above to try again. + + )} + {status === PopupAuthenticationStatus.AuthenticationError && ( + + An unexpected error occurred. Please try again. + + )} + {status === PopupAuthenticationStatus.PopupClosed && ( + + The popup window was closed before authentication could complete. + Please try again by clicking the button above. + + )} ); } + +const ZappName = styled.span` + font-weight: bold; +`; diff --git a/examples/test-zapp/src/utils.ts b/examples/test-zapp/src/utils.ts index 63231a94c7..0cb65e35d0 100644 --- a/examples/test-zapp/src/utils.ts +++ b/examples/test-zapp/src/utils.ts @@ -5,7 +5,7 @@ export function cn(...classes: string[]): string { } export const DEFAULT_CONNECTION_INFO: ClientConnectionInfo = { - url: process.env.CLIENT_URL ?? "https://staging-rob.zupass.org", + url: import.meta.env.VITE_CLIENT_URL ?? "https://staging-rob.zupass.org", type: "iframe" }; diff --git a/examples/test-zapp/src/vite-env.d.ts b/examples/test-zapp/src/vite-env.d.ts new file mode 100644 index 0000000000..c1ea77d600 --- /dev/null +++ b/examples/test-zapp/src/vite-env.d.ts @@ -0,0 +1,9 @@ +/// + +interface ImportMetaEnv { + readonly VITE_CLIENT_URL: string; +} + +interface ImportMeta { + readonly env: ImportMetaEnv; +} diff --git a/test-packaging/vite/src/vite-env.d.ts b/test-packaging/vite/src/vite-env.d.ts deleted file mode 100644 index 11f02fe2a0..0000000000 --- a/test-packaging/vite/src/vite-env.d.ts +++ /dev/null @@ -1 +0,0 @@ -/// diff --git a/turbo.json b/turbo.json index 635ffaa2c7..0d8a7eedb4 100644 --- a/turbo.json +++ b/turbo.json @@ -115,7 +115,7 @@ "NODE_OPTIONS", "ZAPP_RESTRICT_ORIGINS", "ZAPP_ALLOWED_SIGNER_ORIGINS", - "CLIENT_URL", + "VITE_CLIENT_URL", "EMBEDDED_ZAPPS", "//// add env vars above this line ////" ],