From 8a2cc858ae7474cba9b703c7a68e9b5a711d36c4 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 10:10:09 +0530 Subject: [PATCH 01/14] API method --- web/packages/accounts/services/passkey.ts | 45 +++++++++++++++++++++++ web/packages/next/http.ts | 10 +++++ 2 files changed, 55 insertions(+) diff --git a/web/packages/accounts/services/passkey.ts b/web/packages/accounts/services/passkey.ts index d2e854e64d..5438d06cab 100644 --- a/web/packages/accounts/services/passkey.ts +++ b/web/packages/accounts/services/passkey.ts @@ -1,6 +1,8 @@ +import { clientPackageHeaderIfPresent } from "@/next/http"; import log from "@/next/log"; import type { AppName } from "@/next/types/app"; import { clientPackageName } from "@/next/types/app"; +import { TwoFactorAuthorizationResponse } from "@/next/types/credentials"; import ComlinkCryptoWorker from "@ente/shared/crypto"; import { getRecoveryKey } from "@ente/shared/crypto/helpers"; import { @@ -192,3 +194,46 @@ const getAccountsToken = async () => { ); return resp.data["accountsToken"]; }; + +/** + * The passkey session whose status we are trying to check has already expired. + * The user should attempt to login again. + */ +export const passKeySessionExpiredErrorMessage = "Passkey session has expired"; + +/** + * Check if the user has already authenticated using their passkey for the given + * session. + * + * This is useful in case the automatic redirect back from accounts.ente.io to + * the desktop app does not work for some reason. In such cases, the user can + * press the "Check status" button: we'll make an API call to see if the + * authentication has already completed, and if so, get the same "response" + * object we'd have gotten as a query parameter in a redirect in + * {@link saveCredentialsAndNavigateTo} on the "/passkeys/finish" page. + * + * @param sessionID The passkey session whose session we wish to check the + * status of. + * + * @returns A {@link TwoFactorAuthorizationResponse} if the passkey + * authentication has completed, and `undefined` otherwise. + * + * @throws In addition to arbitrary errors, it throws errors with the message + * {@link passKeySessionExpiredErrorMessage}. + */ +export const checkPasskeyVerificationStatus = async ( + sessionID: string, +): Promise => { + const url = `${apiOrigin()}/users/two-factor/passkeys/get-token`; + const params = new URLSearchParams({ sessionID }); + const res = await fetch(`${url}?${params.toString()}`, { + headers: clientPackageHeaderIfPresent(), + }); + if (!res.ok) { + if (res.status == 404 || res.status == 410) + throw new Error(passKeySessionExpiredErrorMessage); + if (res.status == 400) return undefined; /* verification pending */ + throw new Error(`Failed to fetch ${url}: HTTP ${res.status}`); + } + return TwoFactorAuthorizationResponse.parse(await res.json()); +}; diff --git a/web/packages/next/http.ts b/web/packages/next/http.ts index bdcc9aef82..bf64a9b3a8 100644 --- a/web/packages/next/http.ts +++ b/web/packages/next/http.ts @@ -48,3 +48,13 @@ export const authenticatedRequestHeaders = (): Record => { if (_clientPackage) headers["X-Client-Package"] = _clientPackage; return headers; }; + +/** + * Return a headers object with "X-Client-Package" header if we have the client + * package value available to us from local storage. + */ +export const clientPackageHeaderIfPresent = (): Record => { + const headers: Record = {}; + if (_clientPackage) headers["X-Client-Package"] = _clientPackage; + return headers; +}; From fbf29585ebe5b1c60c5a7c3b225e71ffb27cd0bd Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 10:51:58 +0530 Subject: [PATCH 02/14] UI --- .../next/locales/en-US/translation.json | 2 + .../shared/components/LoginComponents.tsx | 44 +++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/web/packages/next/locales/en-US/translation.json b/web/packages/next/locales/en-US/translation.json index 3fc3fde7b9..4f9727a5ba 100644 --- a/web/packages/next/locales/en-US/translation.json +++ b/web/packages/next/locales/en-US/translation.json @@ -625,10 +625,12 @@ "passkey_login_generic_error": "An error occurred while logging in with passkey.", "passkeys_not_supported": "Passkeys are not supported in this browser", "try_again": "Try again", + "check_status": "Check status", "passkey_login_instructions": "Follow the steps from your browser to continue logging in.", "passkey_login": "Login with passkey", "passkey": "Passkey", "waiting_for_verification": "Waiting for verification...", + "verification_still_pending": "Verification is still pending", "passkey_verified": "Passkey verified", "redirecting_back_to_app": "Redirecting you back to the app...", "redirect_close_instructions": "You can close this window after the app opens.", diff --git a/web/packages/shared/components/LoginComponents.tsx b/web/packages/shared/components/LoginComponents.tsx index 76960a23e9..cb2b021dd4 100644 --- a/web/packages/shared/components/LoginComponents.tsx +++ b/web/packages/shared/components/LoginComponents.tsx @@ -1,8 +1,9 @@ import { isDevBuild } from "@/next/env"; import EnteButton from "@ente/shared/components/EnteButton"; import { apiOrigin } from "@ente/shared/network/api"; -import { Typography, styled } from "@mui/material"; +import { CircularProgress, Typography, styled } from "@mui/material"; import { t } from "i18next"; +import React, { useState } from "react"; import { VerticallyCentered } from "./Container"; import FormPaper from "./Form/FormPaper"; import FormPaperFooter from "./Form/FormPaper/Footer"; @@ -68,15 +69,37 @@ export const VerifyingPasskey: React.FC = ({ onRecover, onLogout, }) => { + type VerificationStatus = "waiting" | "checking" | "pending"; + const [verificationStatus, setVerificationStatus] = + useState("waiting"); + + const checkStatus = async () => { + setVerificationStatus("checking"); + setTimeout(() => setVerificationStatus("pending"), 2000); + // try { + // // const t = await checkPasskeyVerificationStatus("TODO"); + // } catch (e) {} + }; + return ( {email ?? ""} - - {t("waiting_for_verification")} - + + {verificationStatus == "checking" ? ( + + + + ) : ( + + {verificationStatus == "waiting" + ? t("waiting_for_verification") + : t("verification_still_pending")} + + )} + = ({ {t("try_again")} - {/* TODO-PK: Uncomment once the API is ready {}} + onClick={checkStatus} fullWidth color="accent" type="button" > - {"Check status"} - */} + {t("check_status")} + @@ -121,7 +143,13 @@ const VerifyingPasskeyMiddle = styled("div")` padding-block: 1rem; gap: 4rem; +`; + +const VerifyingPasskeyStatus = styled("div")` text-align: center; + /* Size of the CircularProgress (+ some margin) so that there is no layout + shift when it is shown */ + min-height: 1.75em; `; const ButtonStack = styled("div")` From 325c963b7abc0ff075212f26677f1186b4f000af Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 11:03:13 +0530 Subject: [PATCH 03/14] Mix --- .../shared/components/LoginComponents.tsx | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/web/packages/shared/components/LoginComponents.tsx b/web/packages/shared/components/LoginComponents.tsx index cb2b021dd4..a911d7f439 100644 --- a/web/packages/shared/components/LoginComponents.tsx +++ b/web/packages/shared/components/LoginComponents.tsx @@ -1,8 +1,11 @@ import { isDevBuild } from "@/next/env"; +import log from "@/next/log"; +import { checkPasskeyVerificationStatus } from "@ente/accounts/services/passkey"; import EnteButton from "@ente/shared/components/EnteButton"; import { apiOrigin } from "@ente/shared/network/api"; import { CircularProgress, Typography, styled } from "@mui/material"; import { t } from "i18next"; +import { useRouter } from "next/router"; import React, { useState } from "react"; import { VerticallyCentered } from "./Container"; import FormPaper from "./Form/FormPaper"; @@ -53,32 +56,50 @@ const ConnectionDetails_ = styled("div")` `; interface VerifyingPasskeyProps { - /** The email of the user whose passkey we're verifying */ + /** ID of the current passkey verification session. */ + passkeySessionID: string; + /** The email of the user whose passkey we're verifying. */ email: string | undefined; /** Called when the user wants to redirect again. */ onRetry: () => void; - /** Called when the user presses the "Recover account" button. */ - onRecover: () => void; - /** Called when the user presses the "Change email" button. */ + /** Called when the user presses the "Change email" button. */ onLogout: () => void; } export const VerifyingPasskey: React.FC = ({ + passkeySessionID, email, onRetry, - onRecover, onLogout, }) => { type VerificationStatus = "waiting" | "checking" | "pending"; const [verificationStatus, setVerificationStatus] = useState("waiting"); - const checkStatus = async () => { + const router = useRouter(); + + const handleRetry = () => { + setVerificationStatus("waiting"); + onRetry(); + }; + const handleCheckStatus = async () => { setVerificationStatus("checking"); - setTimeout(() => setVerificationStatus("pending"), 2000); - // try { - // // const t = await checkPasskeyVerificationStatus("TODO"); - // } catch (e) {} + try { + const response = + await checkPasskeyVerificationStatus(passkeySessionID); + if (!response) { + setVerificationStatus("pending"); + } else { + // TODO-PK: + } + } catch (e) { + log.error("Passkey verification status check failed", e); + // TODO-PK: + } + }; + + const handleRecover = () => { + router.push("/passkeys/recover"); }; return ( @@ -103,7 +124,7 @@ export const VerifyingPasskey: React.FC = ({ = ({ = ({ - + {t("RECOVER_ACCOUNT")} @@ -149,7 +170,7 @@ const VerifyingPasskeyStatus = styled("div")` text-align: center; /* Size of the CircularProgress (+ some margin) so that there is no layout shift when it is shown */ - min-height: 1.75em; + min-height: 2em; `; const ButtonStack = styled("div")` From dd0f7d31427c0d2604337b4a693ce4eede03cf07 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 11:17:51 +0530 Subject: [PATCH 04/14] Handle errors --- web/apps/auth/src/pages/auth.tsx | 15 +----- .../shared/components/ErrorComponents.tsx | 11 ++++ .../shared/components/LoginComponents.tsx | 54 ++++++++++++++++--- 3 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 web/packages/shared/components/ErrorComponents.tsx diff --git a/web/apps/auth/src/pages/auth.tsx b/web/apps/auth/src/pages/auth.tsx index 9c49ffbed3..7f40226e30 100644 --- a/web/apps/auth/src/pages/auth.tsx +++ b/web/apps/auth/src/pages/auth.tsx @@ -3,9 +3,9 @@ import { HorizontalFlex, VerticallyCentered, } from "@ente/shared/components/Container"; -import type { DialogBoxAttributesV2 } from "@ente/shared/components/DialogBoxV2/types"; import { EnteLogo } from "@ente/shared/components/EnteLogo"; import EnteSpinner from "@ente/shared/components/EnteSpinner"; +import { sessionExpiredDialogAttributes } from "@ente/shared/components/LoginComponents"; import NavbarBase from "@ente/shared/components/Navbar/base"; import OverflowMenu from "@ente/shared/components/OverflowMenu/menu"; import { OverflowMenuOption } from "@ente/shared/components/OverflowMenu/option"; @@ -140,19 +140,6 @@ const Page: React.FC = () => { export default Page; -const sessionExpiredDialogAttributes = ( - action: () => void, -): DialogBoxAttributesV2 => ({ - title: t("SESSION_EXPIRED"), - content: t("SESSION_EXPIRED_MESSAGE"), - nonClosable: true, - proceed: { - text: t("LOGIN"), - action, - variant: "accent", - }, -}); - const AuthNavbar: React.FC = () => { const { isMobile, logout } = ensure(useContext(AppContext)); diff --git a/web/packages/shared/components/ErrorComponents.tsx b/web/packages/shared/components/ErrorComponents.tsx new file mode 100644 index 0000000000..57587ce667 --- /dev/null +++ b/web/packages/shared/components/ErrorComponents.tsx @@ -0,0 +1,11 @@ +import { t } from "i18next"; +import type { DialogBoxAttributesV2 } from "./DialogBoxV2/types"; + +/** + * {@link DialogBoxAttributesV2} for showing a generic error. + */ +export const genericErrorAttributes = (): DialogBoxAttributesV2 => ({ + title: t("ERROR"), + close: { variant: "critical" }, + content: t("UNKNOWN_ERROR"), +}); diff --git a/web/packages/shared/components/LoginComponents.tsx b/web/packages/shared/components/LoginComponents.tsx index a911d7f439..bb51306d2b 100644 --- a/web/packages/shared/components/LoginComponents.tsx +++ b/web/packages/shared/components/LoginComponents.tsx @@ -1,13 +1,19 @@ import { isDevBuild } from "@/next/env"; import log from "@/next/log"; -import { checkPasskeyVerificationStatus } from "@ente/accounts/services/passkey"; +import { + checkPasskeyVerificationStatus, + passKeySessionExpiredErrorMessage, +} from "@ente/accounts/services/passkey"; import EnteButton from "@ente/shared/components/EnteButton"; import { apiOrigin } from "@ente/shared/network/api"; import { CircularProgress, Typography, styled } from "@mui/material"; import { t } from "i18next"; import { useRouter } from "next/router"; +import type { BaseAppContextT } from "packages/next/types/app"; import React, { useState } from "react"; import { VerticallyCentered } from "./Container"; +import type { DialogBoxAttributesV2 } from "./DialogBoxV2/types"; +import { genericErrorAttributes } from "./ErrorComponents"; import FormPaper from "./Form/FormPaper"; import FormPaperFooter from "./Form/FormPaper/Footer"; import LinkButton from "./LinkButton"; @@ -62,16 +68,23 @@ interface VerifyingPasskeyProps { email: string | undefined; /** Called when the user wants to redirect again. */ onRetry: () => void; - /** Called when the user presses the "Change email" button. */ - onLogout: () => void; + /** + * The appContext. + * + * Needs to be explicitly passed since this component is used in a package + * where the pages are not wrapped in the provider. + */ + appContext: BaseAppContextT; } export const VerifyingPasskey: React.FC = ({ passkeySessionID, email, onRetry, - onLogout, + appContext, }) => { + const { logout, setDialogBoxAttributesV2 } = appContext; + type VerificationStatus = "waiting" | "checking" | "pending"; const [verificationStatus, setVerificationStatus] = useState("waiting"); @@ -82,6 +95,7 @@ export const VerifyingPasskey: React.FC = ({ setVerificationStatus("waiting"); onRetry(); }; + const handleCheckStatus = async () => { setVerificationStatus("checking"); try { @@ -94,7 +108,12 @@ export const VerifyingPasskey: React.FC = ({ } } catch (e) { log.error("Passkey verification status check failed", e); - // TODO-PK: + setDialogBoxAttributesV2( + e instanceof Error && + e.message == passKeySessionExpiredErrorMessage + ? sessionExpiredDialogAttributes(logout) + : genericErrorAttributes(), + ); } }; @@ -147,7 +166,7 @@ export const VerifyingPasskey: React.FC = ({ {t("RECOVER_ACCOUNT")} - + {t("CHANGE_EMAIL")} @@ -178,3 +197,26 @@ const ButtonStack = styled("div")` flex-direction: column; gap: 1rem; `; + +/** + * {@link DialogBoxAttributesV2} for showing the error when the user's session + * has expired. + * + * It asks them to login again. There is one button, which allows them to + * logout. + * + * @param onLogin Called when the user presses the "Login" button on the error + * dialog. + */ +export const sessionExpiredDialogAttributes = ( + onLogin: () => void, +): DialogBoxAttributesV2 => ({ + title: t("SESSION_EXPIRED"), + content: t("SESSION_EXPIRED_MESSAGE"), + nonClosable: true, + proceed: { + text: t("LOGIN"), + action: onLogin, + variant: "accent", + }, +}); From cc3f398a7812094eedb4ace03f92d122cadb46e2 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 11:41:50 +0530 Subject: [PATCH 05/14] Happy path --- web/packages/accounts/services/passkey.ts | 35 +++++++++++++++++++ .../shared/components/LoginComponents.tsx | 9 +++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/web/packages/accounts/services/passkey.ts b/web/packages/accounts/services/passkey.ts index 5438d06cab..7f4c848fe0 100644 --- a/web/packages/accounts/services/passkey.ts +++ b/web/packages/accounts/services/passkey.ts @@ -3,6 +3,7 @@ import log from "@/next/log"; import type { AppName } from "@/next/types/app"; import { clientPackageName } from "@/next/types/app"; import { TwoFactorAuthorizationResponse } from "@/next/types/credentials"; +import { ensure } from "@/utils/ensure"; import ComlinkCryptoWorker from "@ente/shared/crypto"; import { getRecoveryKey } from "@ente/shared/crypto/helpers"; import { @@ -12,6 +13,8 @@ import { import { CustomError } from "@ente/shared/error"; import HTTPService from "@ente/shared/network/HTTPService"; import { accountsAppURL, apiOrigin } from "@ente/shared/network/api"; +import InMemoryStore, { MS_KEYS } from "@ente/shared/storage/InMemoryStore"; +import { LS_KEYS, getData, setData } from "@ente/shared/storage/localStorage"; import { getToken } from "@ente/shared/storage/localStorage/helpers"; /** @@ -237,3 +240,35 @@ export const checkPasskeyVerificationStatus = async ( } return TwoFactorAuthorizationResponse.parse(await res.json()); }; + +/** + * Extract credentials from a successful passkey verification response and save + * them to local storage for use by subsequent steps (or normal functioning) of + * the app. + * + * @param response The result of a successful + * {@link checkPasskeyVerificationStatus}. + * + * @returns the slug that we should navigate to now. + */ +export const saveCredentialsAndNavigateTo = ( + response: TwoFactorAuthorizationResponse, +) => { + // This method somewhat duplicates `saveCredentialsAndNavigateTo` in the + // /passkeys/finish page. + const { id, encryptedToken, keyAttributes } = response; + + setData(LS_KEYS.USER, { + ...getData(LS_KEYS.USER), + encryptedToken, + id, + }); + setData(LS_KEYS.KEY_ATTRIBUTES, ensure(keyAttributes)); + + // TODO(MR): Remove the cast. + const redirectURL = InMemoryStore.get(MS_KEYS.REDIRECT_URL) as + | string + | undefined; + InMemoryStore.delete(MS_KEYS.REDIRECT_URL); + return redirectURL ?? "/credentials"; +}; diff --git a/web/packages/shared/components/LoginComponents.tsx b/web/packages/shared/components/LoginComponents.tsx index bb51306d2b..eef0a1bca5 100644 --- a/web/packages/shared/components/LoginComponents.tsx +++ b/web/packages/shared/components/LoginComponents.tsx @@ -3,6 +3,7 @@ import log from "@/next/log"; import { checkPasskeyVerificationStatus, passKeySessionExpiredErrorMessage, + saveCredentialsAndNavigateTo, } from "@ente/accounts/services/passkey"; import EnteButton from "@ente/shared/components/EnteButton"; import { apiOrigin } from "@ente/shared/network/api"; @@ -101,11 +102,8 @@ export const VerifyingPasskey: React.FC = ({ try { const response = await checkPasskeyVerificationStatus(passkeySessionID); - if (!response) { - setVerificationStatus("pending"); - } else { - // TODO-PK: - } + if (!response) setVerificationStatus("pending"); + else router.push(saveCredentialsAndNavigateTo(response)); } catch (e) { log.error("Passkey verification status check failed", e); setDialogBoxAttributesV2( @@ -114,6 +112,7 @@ export const VerifyingPasskey: React.FC = ({ ? sessionExpiredDialogAttributes(logout) : genericErrorAttributes(), ); + setVerificationStatus("waiting"); } }; From 4123197c6d153783851f738ea94558cf4be4073d Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 11:46:55 +0530 Subject: [PATCH 06/14] Use --- web/packages/accounts/pages/credentials.tsx | 13 +++++----- web/packages/accounts/pages/verify.tsx | 12 ++++----- web/packages/accounts/services/passkey.ts | 27 +++++++++++++-------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/web/packages/accounts/pages/credentials.tsx b/web/packages/accounts/pages/credentials.tsx index 811c5b975a..b44bc4e103 100644 --- a/web/packages/accounts/pages/credentials.tsx +++ b/web/packages/accounts/pages/credentials.tsx @@ -67,10 +67,11 @@ const Page: React.FC = ({ appContext }) => { const [keyAttributes, setKeyAttributes] = useState(); const [user, setUser] = useState(); const [passkeyVerificationData, setPasskeyVerificationData] = useState< - [string, string] | undefined + { passkeySessionID: string; url: string } | undefined >(); const router = useRouter(); + useEffect(() => { const main = async () => { const user: User = getData(LS_KEYS.USER); @@ -180,8 +181,8 @@ const Page: React.FC = ({ appContext }) => { appName, passkeySessionID, ); - setPasskeyVerificationData([passkeySessionID, url]); - openPasskeyVerificationURL(passkeySessionID, url); + setPasskeyVerificationData({ passkeySessionID, url }); + openPasskeyVerificationURL({ passkeySessionID, url }); throw Error(CustomError.TWO_FACTOR_ENABLED); } else if (twoFactorSessionID) { const sessionKeyAttributes = @@ -295,11 +296,11 @@ const Page: React.FC = ({ appContext }) => { return ( - openPasskeyVerificationURL(...passkeyVerificationData) + openPasskeyVerificationURL(passkeyVerificationData) } - onRecover={() => router.push("/passkeys/recover")} - onLogout={logout} + appContext={appContext} /> ); } diff --git a/web/packages/accounts/pages/verify.tsx b/web/packages/accounts/pages/verify.tsx index 1848b38be8..c6e6954d12 100644 --- a/web/packages/accounts/pages/verify.tsx +++ b/web/packages/accounts/pages/verify.tsx @@ -42,7 +42,7 @@ const Page: React.FC = ({ appContext }) => { const [email, setEmail] = useState(""); const [resend, setResend] = useState(0); const [passkeyVerificationData, setPasskeyVerificationData] = useState< - [string, string] | undefined + { passkeySessionID: string; url: string } | undefined >(); const router = useRouter(); @@ -98,8 +98,8 @@ const Page: React.FC = ({ appContext }) => { appName, passkeySessionID, ); - setPasskeyVerificationData([passkeySessionID, url]); - openPasskeyVerificationURL(passkeySessionID, url); + setPasskeyVerificationData({ passkeySessionID, url }); + openPasskeyVerificationURL({ passkeySessionID, url }); } else if (twoFactorSessionID) { setData(LS_KEYS.USER, { email, @@ -193,11 +193,11 @@ const Page: React.FC = ({ appContext }) => { return ( - openPasskeyVerificationURL(...passkeyVerificationData) + openPasskeyVerificationURL(passkeyVerificationData) } - onRecover={() => router.push("/passkeys/recover")} - onLogout={logout} + appContext={appContext} /> ); } diff --git a/web/packages/accounts/services/passkey.ts b/web/packages/accounts/services/passkey.ts index 7f4c848fe0..70cd3ae0fa 100644 --- a/web/packages/accounts/services/passkey.ts +++ b/web/packages/accounts/services/passkey.ts @@ -51,16 +51,23 @@ export const passkeyVerificationRedirectURL = ( return `${accountsAppURL()}/passkeys/verify?${params.toString()}`; }; +interface OpenPasskeyVerificationURLOptions { + /** + * The passkeySessionID for which we are redirecting. + * + * This is compared to the saved session id in the browser's session storage + * to allow us to ignore redirects to the passkey flow finish page except + * the ones for this specific session we're awaiting. + */ + passkeySessionID: string; + /** The URL to redirect to or open in the system browser. */ + url: string; +} + /** * Open or redirect to a passkey verification URL previously constructed using * {@link passkeyVerificationRedirectURL}. * - * @param passkeySessionID The passkeySessionID for which we are redirecting. - * This is saved to session storage to allow us to ignore subsequent redirects - * to the passkey flow finish page except the ones for this specific session. - * - * @param url The URL to redirect to or open in the system browser. - * * [Note: Passkey verification in the desktop app] * * Our desktop app bundles the web app and serves it over a custom protocol. @@ -80,10 +87,10 @@ export const passkeyVerificationRedirectURL = ( * authentication happens at accounts.ente.io, and on success there is * redirected back to the desktop app. */ -export const openPasskeyVerificationURL = ( - passkeySessionID: string, - url: string, -) => { +export const openPasskeyVerificationURL = ({ + passkeySessionID, + url, +}: OpenPasskeyVerificationURLOptions) => { sessionStorage.setItem("inflightPasskeySessionID", passkeySessionID); if (globalThis.electron) window.open(url); From 93380d05b454a8ea535150134ce42e72e081453d Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 12:03:09 +0530 Subject: [PATCH 07/14] Add TODO --- web/packages/accounts/pages/passkeys/finish.tsx | 4 ++++ web/packages/next/types/credentials.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/web/packages/accounts/pages/passkeys/finish.tsx b/web/packages/accounts/pages/passkeys/finish.tsx index 2b1a8b8e61..be52cc02ae 100644 --- a/web/packages/accounts/pages/passkeys/finish.tsx +++ b/web/packages/accounts/pages/passkeys/finish.tsx @@ -90,6 +90,10 @@ const saveCredentialsAndNavigateTo = async ( // - The plaintext "token" will be passed during fresh signups, where we // don't yet have keys to encrypt it, the account itself is being created // as we go through this flow. + // TODO(MR): Conceptually this cannot happen. During a _real_ fresh signup + // we'll never enter the passkey verification flow. Remove this code after + // making sure that it doesn't get triggered in cases where an existing + // user goes through the new user flow. // // - The encrypted `encryptedToken` will be present otherwise (i.e. if the // user is signing into an existing account). diff --git a/web/packages/next/types/credentials.ts b/web/packages/next/types/credentials.ts index 950cfffcca..483377d301 100644 --- a/web/packages/next/types/credentials.ts +++ b/web/packages/next/types/credentials.ts @@ -9,7 +9,11 @@ export const KeyAttributes = z.object({}).passthrough(); */ export const TwoFactorAuthorizationResponse = z.object({ id: z.number(), + /** TODO: keyAttributes is guaranteed to be returned by museum, update the + * types to reflect that. */ keyAttributes: KeyAttributes.nullish().transform(nullToUndefined), + /** TODO: encryptedToken is guaranteed to be returned by museum, update the + * types to reflect that. */ encryptedToken: z.string().nullish().transform(nullToUndefined), }); From 228dd90bcebeff053ae46b829141c2da7786e3f7 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 12:11:43 +0530 Subject: [PATCH 08/14] Make the retry code (almost) exactly the same as it was before in an attempt at superstition (since rationality doesn't seem to work with Safari). --- web/apps/accounts/src/services/passkey.ts | 44 ++++++++++++----------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/web/apps/accounts/src/services/passkey.ts b/web/apps/accounts/src/services/passkey.ts index 132a4d5f8d..f0e8c1abe3 100644 --- a/web/apps/accounts/src/services/passkey.ts +++ b/web/apps/accounts/src/services/passkey.ts @@ -3,7 +3,6 @@ import log from "@/next/log"; import { clientPackageName } from "@/next/types/app"; import { TwoFactorAuthorizationResponse } from "@/next/types/credentials"; import { ensure } from "@/utils/ensure"; -import { wait } from "@/utils/promise"; import { nullToUndefined } from "@/utils/transform"; import { fromB64URLSafeNoPadding, @@ -425,28 +424,33 @@ export const beginPasskeyAuthentication = async ( export const signChallenge = async ( publicKey: PublicKeyCredentialRequestOptions, ) => { - const go = async () => await navigator.credentials.get({ publicKey }); + // Safari throws "NotAllowedError: The document is not focused" sometimes + // (no reason, just to show their incompetence). The retry doesn't seem to + // help mostly, but cargo cult anyway. - try { - return await go(); - } catch (e) { - // Safari throws "NotAllowedError: The document is not focused" for the - // first request sometimes (no reason, just to show their incompetence). - // "NotAllowedError" is also the error name that is thrown when the user - // explicitly cancels, so we can't even filter it out by name and also - // to do a message match. - if ( - e instanceof Error && - e.name == "NotAllowedError" && - e.message == "The document is not focused." - ) { - log.warn("Working around Safari bug by retrying after failure", e); - await wait(2000); - return await go(); - } else { - throw e; + let tries = 0; + const maxTries = 3; + + while (tries < maxTries) { + try { + return await navigator.credentials.get({ publicKey }); + } catch (e) { + if ( + e instanceof Error && + e.name == "NotAllowedError" && + e.message == "The document is not focused." + ) { + log.warn("Safari workaround for", e); + continue; + } else { + throw e; + } + } finally { + tries++; } } + + return undefined; }; interface FinishPasskeyAuthenticationOptions { From b2e56fc01e2f543246019bb6e9fd79dd6bd52108 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 12:23:09 +0530 Subject: [PATCH 09/14] Lint fix --- web/packages/shared/components/LoginComponents.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/shared/components/LoginComponents.tsx b/web/packages/shared/components/LoginComponents.tsx index eef0a1bca5..950c6dc4cf 100644 --- a/web/packages/shared/components/LoginComponents.tsx +++ b/web/packages/shared/components/LoginComponents.tsx @@ -1,5 +1,6 @@ import { isDevBuild } from "@/next/env"; import log from "@/next/log"; +import type { BaseAppContextT } from "@/next/types/app"; import { checkPasskeyVerificationStatus, passKeySessionExpiredErrorMessage, @@ -10,7 +11,6 @@ import { apiOrigin } from "@ente/shared/network/api"; import { CircularProgress, Typography, styled } from "@mui/material"; import { t } from "i18next"; import { useRouter } from "next/router"; -import type { BaseAppContextT } from "packages/next/types/app"; import React, { useState } from "react"; import { VerticallyCentered } from "./Container"; import type { DialogBoxAttributesV2 } from "./DialogBoxV2/types"; From ca080ad6b2d193a67f29dcd70cab0faa2d39a068 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 13:07:00 +0530 Subject: [PATCH 10/14] Split the flow --- .../accounts/src/pages/passkeys/verify.tsx | 132 ++++++++++++++++-- web/apps/accounts/src/services/passkey.ts | 31 +--- 2 files changed, 120 insertions(+), 43 deletions(-) diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 793576632e..03315450b8 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -6,9 +6,10 @@ import { VerticallyCentered } from "@ente/shared/components/Container"; import EnteButton from "@ente/shared/components/EnteButton"; import EnteSpinner from "@ente/shared/components/EnteSpinner"; import InfoIcon from "@mui/icons-material/Info"; +import KeyIcon from "@mui/icons-material/Key"; import { Paper, Typography, styled } from "@mui/material"; import { t } from "i18next"; -import React, { useEffect, useState } from "react"; +import React, { useCallback, useEffect, useState } from "react"; import { beginPasskeyAuthentication, finishPasskeyAuthentication, @@ -17,6 +18,7 @@ import { passkeyAuthenticationSuccessRedirectURL, redirectToPasskeyRecoverPage, signChallenge, + type BeginPasskeyAuthenticationResponse, } from "services/passkey"; const Page = () => { @@ -29,14 +31,37 @@ const Page = () => { | "loading" /* Can happen multiple times in the flow */ | "webAuthnNotSupported" /* Unrecoverable error */ | "unknownRedirect" /* Unrecoverable error */ - | "unrecoverableFailure" /* Unrocevorable error - generic */ + | "unrecoverableFailure" /* Unrecoverable error - generic */ | "failed" /* Recoverable error */ + | "needUserFocus" /* See docs for `Continuation` */ | "waitingForUser" /* ...to authenticate with their passkey */ | "redirectingWeb" /* Redirect back to the requesting app (HTTP) */ | "redirectingApp"; /* Other redirects (mobile / desktop redirect) */ const [status, setStatus] = useState("loading"); + /** + * Safari keeps on saying "NotAllowedError: The document is not focused" + * even though it just opened the page and brought it to the front. + * + * Because of their incompetence, we need to break our entire flow into two + * parts, and stash away a lot of state when we're in the "needUserFocus" + * state. + */ + interface Continuation { + redirectURL: URL; + clientPackage: string; + passkeySessionID: string; + beginResponse: BeginPasskeyAuthenticationResponse; + } + const [continuation, setContinuation] = useState< + Continuation | undefined + >(); + + // Safari throws sometimes + // (no reason, just to show their incompetence). The retry doesn't seem to + // help mostly, but cargo cult anyway. + // The URL we're redirecting to on success. // // This will only be set when status is "redirecting*". @@ -44,8 +69,8 @@ const Page = () => { URL | undefined >(); - /** (re)start the authentication flow */ - const authenticate = async () => { + /** Phase 1 of {@link authenticate}. */ + const authenticateBegin = useCallback(async () => { if (!isWebAuthnSupported()) { setStatus("webAuthnNotSupported"); return; @@ -86,21 +111,59 @@ const Page = () => { return; } - let authorizationResponse: TwoFactorAuthorizationResponse; + let beginResponse: BeginPasskeyAuthenticationResponse; try { - const { ceremonySessionID, options } = - await beginPasskeyAuthentication(passkeySessionID); + beginResponse = await beginPasskeyAuthentication(passkeySessionID); + } catch (e) { + log.error("Failed to begin passkey authentication", e); + setStatus("failed"); + return; + } - setStatus("waitingForUser"); + return { + redirectURL, + passkeySessionID, + clientPackage, + beginResponse, + }; + }, []); - const credential = await signChallenge(options.publicKey); + /** + * Phase 2 of {@link authenticate}, separated by a potential user + * interaction. + */ + const authenticateContinue = useCallback(async (cont: Continuation) => { + const { redirectURL, passkeySessionID, clientPackage, beginResponse } = + cont; + const { ceremonySessionID, options } = beginResponse; + + setStatus("waitingForUser"); + + let credential: Credential | undefined; + try { + credential = await signChallenge(options.publicKey); if (!credential) { setStatus("failed"); return; } + } catch (e) { + log.error("Failed to get credentials", e); + if ( + e instanceof Error && + e.name == "NotAllowedError" && + e.message == "The document is not focused." + ) { + setStatus("needUserFocus"); + } else { + setStatus("failed"); + } + return; + } - setStatus("loading"); + setStatus("loading"); + let authorizationResponse: TwoFactorAuthorizationResponse; + try { authorizationResponse = await finishPasskeyAuthentication({ passkeySessionID, ceremonySessionID, @@ -108,7 +171,7 @@ const Page = () => { credential, }); } catch (e) { - log.error("Passkey authentication failed", e); + log.error("Failed to finish passkey authentication", e); setStatus("failed"); return; } @@ -122,16 +185,27 @@ const Page = () => { authorizationResponse, ), ); - }; + }, []); + + /** (re)start the authentication flow */ + const authenticate = useCallback(async () => { + const cont = await authenticateBegin(); + if (cont) { + setContinuation(cont); + await authenticateContinue(cont); + } + }, [authenticateBegin, authenticateContinue]); useEffect(() => { void authenticate(); - }, []); + }, [authenticate]); useEffect(() => { if (successRedirectURL) redirectToURL(successRedirectURL); }, [successRedirectURL]); + const handleVerify = () => void authenticateContinue(ensure(continuation)); + const handleRetry = () => void authenticate(); const handleRecover = (() => { @@ -160,6 +234,7 @@ const Page = () => { failed: ( ), + needUserFocus: , waitingForUser: , redirectingWeb: , redirectingApp: , @@ -237,6 +312,37 @@ const ContentPaper = styled(Paper)` gap: 1rem; `; +interface VerifyProps { + /** Called when the user presses the "Verify" button. */ + onVerify: () => void; +} +/** + * Gain focus for the current page by requesting the user to explicitly click a + * button. For more details, see the documentation for `Continuation`. + */ +const Verify: React.FC = ({ onVerify }) => { + return ( + + + {t("passkey")} + + {t("passkey_login_generic_error")} + + + + {t("VERIFY")} + + + + ); +}; + interface RetriableFailedProps { /** Callback invoked when the user presses the try again button. */ onRetry: () => void; diff --git a/web/apps/accounts/src/services/passkey.ts b/web/apps/accounts/src/services/passkey.ts index f0e8c1abe3..0fda0c1e00 100644 --- a/web/apps/accounts/src/services/passkey.ts +++ b/web/apps/accounts/src/services/passkey.ts @@ -1,5 +1,4 @@ import { isDevBuild } from "@/next/env"; -import log from "@/next/log"; import { clientPackageName } from "@/next/types/app"; import { TwoFactorAuthorizationResponse } from "@/next/types/credentials"; import { ensure } from "@/utils/ensure"; @@ -423,35 +422,7 @@ export const beginPasskeyAuthentication = async ( */ export const signChallenge = async ( publicKey: PublicKeyCredentialRequestOptions, -) => { - // Safari throws "NotAllowedError: The document is not focused" sometimes - // (no reason, just to show their incompetence). The retry doesn't seem to - // help mostly, but cargo cult anyway. - - let tries = 0; - const maxTries = 3; - - while (tries < maxTries) { - try { - return await navigator.credentials.get({ publicKey }); - } catch (e) { - if ( - e instanceof Error && - e.name == "NotAllowedError" && - e.message == "The document is not focused." - ) { - log.warn("Safari workaround for", e); - continue; - } else { - throw e; - } - } finally { - tries++; - } - } - - return undefined; -}; +) => nullToUndefined(await navigator.credentials.get({ publicKey })); interface FinishPasskeyAuthenticationOptions { passkeySessionID: string; From 3689ecb6e7ba10c885fd3d53b4d0b5c535fc204d Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Jun 2024 13:22:16 +0530 Subject: [PATCH 11/14] Add a message --- web/apps/accounts/src/pages/passkeys/verify.tsx | 3 ++- web/packages/next/locales/en-US/translation.json | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 03315450b8..2c30c22641 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -316,6 +316,7 @@ interface VerifyProps { /** Called when the user presses the "Verify" button. */ onVerify: () => void; } + /** * Gain focus for the current page by requesting the user to explicitly click a * button. For more details, see the documentation for `Continuation`. @@ -326,7 +327,7 @@ const Verify: React.FC = ({ onVerify }) => { {t("passkey")} - {t("passkey_login_generic_error")} + {t("passkey_verify_description")} Date: Fri, 14 Jun 2024 13:42:30 +0530 Subject: [PATCH 12/14] Add a hint to retry on other devices --- .../accounts/src/pages/passkeys/verify.tsx | 27 ++++++++++++++++--- .../next/locales/en-US/translation.json | 1 + 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 2c30c22641..5c909e76f4 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -32,7 +32,8 @@ const Page = () => { | "webAuthnNotSupported" /* Unrecoverable error */ | "unknownRedirect" /* Unrecoverable error */ | "unrecoverableFailure" /* Unrecoverable error - generic */ - | "failed" /* Recoverable error */ + | "failedDuringSignChallenge" /* Recoverable error in signChallenge */ + | "failed" /* Recoverable error otherwise */ | "needUserFocus" /* See docs for `Continuation` */ | "waitingForUser" /* ...to authenticate with their passkey */ | "redirectingWeb" /* Redirect back to the requesting app (HTTP) */ @@ -143,7 +144,7 @@ const Page = () => { try { credential = await signChallenge(options.publicKey); if (!credential) { - setStatus("failed"); + setStatus("failedDuringSignChallenge"); return; } } catch (e) { @@ -155,7 +156,7 @@ const Page = () => { ) { setStatus("needUserFocus"); } else { - setStatus("failed"); + setStatus("failedDuringSignChallenge"); } return; } @@ -231,6 +232,13 @@ const Page = () => { unknownRedirect: , webAuthnNotSupported: , unrecoverableFailure: , + failedDuringSignChallenge: ( + + ), failed: ( ), @@ -345,6 +353,14 @@ const Verify: React.FC = ({ onVerify }) => { }; interface RetriableFailedProps { + /** + * Set this attribute to indicate that this failure occurred during the + * actual passkey verification (`navigator.credentials.get`). + * + * We customize the error message for such cases to give a hint to the user + * that they can try on their other devices too. + */ + duringSignChallenge?: boolean; /** Callback invoked when the user presses the try again button. */ onRetry: () => void; /** @@ -358,6 +374,7 @@ interface RetriableFailedProps { } const RetriableFailed: React.FC = ({ + duringSignChallenge, onRetry, onRecover, }) => { @@ -366,7 +383,9 @@ const RetriableFailed: React.FC = ({ {t("passkey_login_failed")} - {t("passkey_login_generic_error")} + {duringSignChallenge + ? t("passkey_login_credential_hint") + : t("passkey_login_generic_error")} Date: Fri, 14 Jun 2024 13:49:45 +0530 Subject: [PATCH 13/14] Change to photos favicon he accounts favicon does not show on a white background (second image is the hover state showing that the icon is actually there). For now, changing it to the photos favicon, until we have an app neutral favicon. --- web/apps/accounts/public/images/favicon.png | Bin 422 -> 1125 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/web/apps/accounts/public/images/favicon.png b/web/apps/accounts/public/images/favicon.png index 2d769dadf483130d5cee0f0429233cc75702978c..fcb8d1054bb23170d91f396130a78262fb261c84 100644 GIT binary patch delta 1097 zcmV-P1h)I81LX*iD}MsT0008T0g2Xs?*IS+Wobi0P(*2SaBN|DAW$;^0C=2rkiAR8 zP!z>at5VQ9hz=bbGKoXf(h7EQXe$&&FjNJrQ<{DWZG0ptQgIVkDfk~)!C7#yh*WTK za1cZX5#5|RDY$5O-j`I`BHqX4{WzR+xm>^-P#G)s0x0R0k$+6Yq_xz#jMo>Qu3 zvZ$gg3jY1O>+{j|-cN{GO}lmw*-u=T&MudBo&r;U{aw|c$(kDd-_${k85ZD?iIr`O z3Em|hF=#2`s^GlK$AZ%?pF5tFWSsbxSkek=hPX*Ar#tj~2bYRvt+j*L-k;T1m&F|F zQOsczCK!rPQGbC11>t}E(~dumEpk_2P%e)GEXqVl5fie+^U$GT5;AcJLF$mWbL)N& zt^3JsZSNJBZE(&n+S)<`IGv%B)!W*51Q;3sZt6@mR7dKe_U3ZH`w1|Z0*3D>vtj1K zA$NO!4Cs5|{9_F89RhZh^RFY$?HYxjfh+w7!65@S0Dk}irb$FWRCoc+m%ndPQ546& z??+QBO0fwH79!dh&BK8XHZtjiF)j|GCh9VlJv zHIW74LIP=JfJTu9NwlxedtQs-!MpE=seOKumq#v_d(J)Q-0!&vU_{?simKB3WA-r) z4FUXfgnxNyL&{XnJgW#>#y{QO$Z_oCr|VML;_Tuh0+|;Vl8L9e@m% zB*tP;_Cyie8%1~|sE@VQ+bFJ8u=uTn_g{-{mUHd^8O|I^;RJh|kqk}g@0Csi+46JD z)$(wH5Xqg_>?UOs6l&XY#3Dl=A#Odm~~ zez#EnfTzoE@ou$%#?K~3<=yZP`3!R+BO(1wwZVg11L0jE)19w$>bUrE2FonhhWV@@ zr;ns@I&|DHe(k_n%vN7>Q^8F^Vodo{<-M30-*l{j>xKI$wkn2kzwAS5{3LjA9iZ^{ zlYe7jH#6SLi&}nQEE$~Q&gZ5;@gE!*3$vU)jNOD4{dGWeYr&NRepXcdkoeH!7PC}# z`G2LDsy*BnI+9yDer~Z6Bd!Cbwd#Na0(-LMIcz7~6qq#?h2@`=(T>Ev7@WdY!g9NY zE0H9I#UbP3Tg~W8XLTU7bms3IzKnay4S!w878*;i!?GEX7Cbn{KX!{OO5{ZA*AI4V zqk4pTp{{6`GRt02Vns)pPjFf=)yRZzTsIyZ9=ul4??@px*D=TT7&_Sn4~kH(Zk+~4 z_W^>$gqo9?ME65!A*W}nuWaW!cA(c0s&Wc@j9tyTP^iDFZKe}TuC8US9oTL>mqKDu z5YU8&h%EjEc!h?SqNK`cFQPy;B~891(qCjzl_yy;=CK3ln-!2Jn`HYpW{vN|z%X)E P00000NkvXXu0mjf%2)^C delta 388 zcmV-~0ek-C2&My&D}M_U000XU0RWnu7ytkO0drDELIAGL9O(c600d`2O+f$vv5yP< zVFdsH0Xs=VK~#7F?UwIt!Y~lUUr@i74eAY=5j+ATumRnGY!Eh}D=-0!kWS!s0@&dC zQu{+&`A-}Z5K?~96X+6O&gb|!z|72@sgQ|?EcvsdUa6%EUw^1C>Qq%#hbiELHFcCU zhFasAFkvhC94UoTm}D>GF=Cf&vY8vFXeTfRv1=Y;_j_jje`*~mMk5yqzi@AnF^W1D zcflp{u^%wD35>0M_b&o@bRd2}njNe!49fWgKXo9!;Y>D2<9qWJG71Q79gXok`+^JH znNSBVfp{0WmVM#_83lX>;s+i!4W=ft4f{Q=a0?%*TF|_Gg8rUjAL~ i&8DMQoG>% Date: Fri, 14 Jun 2024 13:51:21 +0530 Subject: [PATCH 14/14] Enable passkeys for everyone --- web/apps/photos/src/components/Sidebar/index.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/web/apps/photos/src/components/Sidebar/index.tsx b/web/apps/photos/src/components/Sidebar/index.tsx index d2d7aa1e83..dc8c7b9c64 100644 --- a/web/apps/photos/src/components/Sidebar/index.tsx +++ b/web/apps/photos/src/components/Sidebar/index.tsx @@ -528,13 +528,11 @@ const UtilitySection: React.FC = ({ closeSidebar }) => { label={t("TWO_FACTOR")} /> - {isInternalUserViaEmailCheck() && ( - - )} +