From ee34ed5e1fb1da5de7970746cdde3a041e9dff8d Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 28 Nov 2024 11:14:12 +0530 Subject: [PATCH 1/3] 2FA choice place 1 --- .../components/SecondFactorChoice.tsx | 60 +++++++++++++++++ web/packages/accounts/pages/credentials.tsx | 67 ++++++++++++++++++- web/packages/accounts/services/user.ts | 6 ++ .../base/locales/en-US/translation.json | 1 + 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 web/packages/accounts/components/SecondFactorChoice.tsx diff --git a/web/packages/accounts/components/SecondFactorChoice.tsx b/web/packages/accounts/components/SecondFactorChoice.tsx new file mode 100644 index 0000000000..4ef28e44dc --- /dev/null +++ b/web/packages/accounts/components/SecondFactorChoice.tsx @@ -0,0 +1,60 @@ +import { FocusVisibleButton } from "@/base/components/mui/FocusVisibleButton"; +import type { ModalVisibilityProps } from "@/base/components/utils/modal"; +import { Dialog, DialogContent, DialogTitle, Stack } from "@mui/material"; +import { t } from "i18next"; +import React from "react"; + +export type SecondFactorType = "totp" | "passkey"; + +type SecondFactorChoiceProps = ModalVisibilityProps & { + /** + * Callback invoked with the selected choice. + * + * The dialog will automatically be closed before this callback is invoked. + */ + didSelect: (factor: SecondFactorType) => void; +}; + +/** + * A {@link Dialog} that allow the user to choose which second factor they'd + * like to verify during login. + */ +export const SecondFactorChoice: React.FC = ({ + open, + onClose, + didSelect, +}) => ( + { + if (reason != "backdropClick") onClose(); + }} + fullWidth + PaperProps={{ sx: { maxWidth: "360px", padding: "12px" } }} + > + {t("two_factor")} + + + { + onClose(); + didSelect("totp"); + }} + > + {t("totp_login")} + + + { + onClose(); + didSelect("passkey"); + }} + > + {t("passkey_login")} + + + + +); diff --git a/web/packages/accounts/pages/credentials.tsx b/web/packages/accounts/pages/credentials.tsx index 7a12367264..a6621ea68e 100644 --- a/web/packages/accounts/pages/credentials.tsx +++ b/web/packages/accounts/pages/credentials.tsx @@ -1,6 +1,7 @@ import { sessionExpiredDialogAttributes } from "@/accounts/components/utils/dialog"; import { FormPaper } from "@/base/components/FormPaper"; import { ActivityIndicator } from "@/base/components/mui/ActivityIndicator"; +import { useModalVisibility } from "@/base/components/utils/modal"; import { sharedCryptoWorker } from "@/base/crypto"; import type { B64EncryptionResult } from "@/base/crypto/libsodium"; import { clearLocalStorage } from "@/base/local-storage"; @@ -38,12 +39,16 @@ import type { KeyAttributes, User } from "@ente/shared/user/types"; import { Stack } from "@mui/material"; import { t } from "i18next"; import { useRouter } from "next/router"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { LoginFlowFormFooter, PasswordHeader, VerifyingPasskey, } from "../components/LoginComponents"; +import { + SecondFactorChoice, + type SecondFactorType, +} from "../components/SecondFactorChoice"; import { PAGES } from "../constants/pages"; import { openPasskeyVerificationURL, @@ -76,6 +81,14 @@ const Page: React.FC = ({ appContext }) => { const [sessionValidityCheck, setSessionValidityCheck] = useState< Promise | undefined >(); + const resolveSecondFactorChoice = useRef< + | ((value: SecondFactorType | PromiseLike) => void) + | undefined + >(); + const { + show: showSecondFactorChoice, + props: secondFactorChoiceVisibilityProps, + } = useModalVisibility(); const router = useRouter(); @@ -216,10 +229,47 @@ const Page: React.FC = ({ appContext }) => { encryptedToken, token, id, - twoFactorSessionID, - passkeySessionID, + twoFactorSessionID: _twoFactorSessionIDV1, + twoFactorSessionIDV2: _twoFactorSessionIDV2, + passkeySessionID: _passkeySessionID, } = await loginViaSRP(srpAttributes!, kek); setIsFirstLogin(true); + + // When the user has both TOTP and pk set as the second factor, + // we'll get two session IDs. For backward compat, the TOTP + // session ID will be in a V2 attribute during a transient + // migration period. + // + // Note the use of || instead of ?? since _twoFactorSessionIDV1 + // will be an empty string, not undefined, if it is unset. We + // might need to add a `xxx-eslint-disable + // @typescript-eslint/prefer-nullish-coalescing` here too later. + const _twoFactorSessionID = + _twoFactorSessionIDV1 || _twoFactorSessionIDV2; + let passkeySessionID: string | undefined; + let twoFactorSessionID: string | undefined; + if (_twoFactorSessionID && _passkeySessionID) { + // If both factors are set, ask the user which one they wish + // to use. + const choice = await new Promise( + (resolve) => { + resolveSecondFactorChoice.current = resolve; + showSecondFactorChoice(); + }, + ); + switch (choice) { + case "passkey": + passkeySessionID = _passkeySessionID; + break; + case "totp": + twoFactorSessionID = _twoFactorSessionID; + break; + } + } else { + passkeySessionID = _passkeySessionID; + twoFactorSessionID = _twoFactorSessionID; + } + if (passkeySessionID) { const sessionKeyAttributes = await cryptoWorker.generateKeyAndEncryptToB64(kek); @@ -322,6 +372,12 @@ const Page: React.FC = ({ appContext }) => { } }; + const handleSecondFactorChoice = (factor: SecondFactorType) => { + const resolve = resolveSecondFactorChoice.current!; + resolveSecondFactorChoice.current = undefined; + resolve(factor); + }; + if (!keyAttributes && !srpAttributes) { return ( @@ -387,6 +443,11 @@ const Page: React.FC = ({ appContext }) => { + + ); }; diff --git a/web/packages/accounts/services/user.ts b/web/packages/accounts/services/user.ts index a680e09950..3ed5e7c257 100644 --- a/web/packages/accounts/services/user.ts +++ b/web/packages/accounts/services/user.ts @@ -13,6 +13,12 @@ export interface UserVerificationResponse { token?: string; twoFactorSessionID: string; passkeySessionID: string; + /** + * If both passkeys and TOTP based two factors are enabled, then {@link + * twoFactorSessionIDV2} will be set to the TOTP session ID instead of + * {@link twoFactorSessionID}. + */ + twoFactorSessionIDV2?: string | undefined; srpM2?: string; } diff --git a/web/packages/base/locales/en-US/translation.json b/web/packages/base/locales/en-US/translation.json index 42cc995254..0608e4a78b 100644 --- a/web/packages/base/locales/en-US/translation.json +++ b/web/packages/base/locales/en-US/translation.json @@ -637,6 +637,7 @@ "check_status": "Check status", "passkey_login_instructions": "Follow the steps from your browser to continue logging in.", "passkey_login": "Login with passkey", + "totp_login": "Login with TOTP", "passkey": "Passkey", "passkey_verify_description": "Verify your passkey to login into your account.", "waiting_for_verification": "Waiting for verification...", From 6fed5c944b2c36267dafa66177f28ffbad315c32 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 28 Nov 2024 12:30:14 +0530 Subject: [PATCH 2/3] Extract --- .../components/SecondFactorChoice.tsx | 8 +- web/packages/accounts/pages/credentials.tsx | 76 ++++--------------- 2 files changed, 18 insertions(+), 66 deletions(-) diff --git a/web/packages/accounts/components/SecondFactorChoice.tsx b/web/packages/accounts/components/SecondFactorChoice.tsx index 4ef28e44dc..5f5bb35dea 100644 --- a/web/packages/accounts/components/SecondFactorChoice.tsx +++ b/web/packages/accounts/components/SecondFactorChoice.tsx @@ -12,7 +12,7 @@ type SecondFactorChoiceProps = ModalVisibilityProps & { * * The dialog will automatically be closed before this callback is invoked. */ - didSelect: (factor: SecondFactorType) => void; + onSelect: (factor: SecondFactorType) => void; }; /** @@ -22,7 +22,7 @@ type SecondFactorChoiceProps = ModalVisibilityProps & { export const SecondFactorChoice: React.FC = ({ open, onClose, - didSelect, + onSelect, }) => ( = ({ color="accent" onClick={() => { onClose(); - didSelect("totp"); + onSelect("totp"); }} > {t("totp_login")} @@ -49,7 +49,7 @@ export const SecondFactorChoice: React.FC = ({ color="accent" onClick={() => { onClose(); - didSelect("passkey"); + onSelect("passkey"); }} > {t("passkey_login")} diff --git a/web/packages/accounts/pages/credentials.tsx b/web/packages/accounts/pages/credentials.tsx index a6621ea68e..1cf601317e 100644 --- a/web/packages/accounts/pages/credentials.tsx +++ b/web/packages/accounts/pages/credentials.tsx @@ -1,7 +1,6 @@ import { sessionExpiredDialogAttributes } from "@/accounts/components/utils/dialog"; import { FormPaper } from "@/base/components/FormPaper"; import { ActivityIndicator } from "@/base/components/mui/ActivityIndicator"; -import { useModalVisibility } from "@/base/components/utils/modal"; import { sharedCryptoWorker } from "@/base/crypto"; import type { B64EncryptionResult } from "@/base/crypto/libsodium"; import { clearLocalStorage } from "@/base/local-storage"; @@ -39,16 +38,14 @@ import type { KeyAttributes, User } from "@ente/shared/user/types"; import { Stack } from "@mui/material"; import { t } from "i18next"; import { useRouter } from "next/router"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { LoginFlowFormFooter, PasswordHeader, VerifyingPasskey, } from "../components/LoginComponents"; -import { - SecondFactorChoice, - type SecondFactorType, -} from "../components/SecondFactorChoice"; +import { SecondFactorChoice } from "../components/SecondFactorChoice"; +import { useSecondFactorChoiceIfNeeded } from "../components/utils/second-factor-choice"; import { PAGES } from "../constants/pages"; import { openPasskeyVerificationURL, @@ -81,14 +78,10 @@ const Page: React.FC = ({ appContext }) => { const [sessionValidityCheck, setSessionValidityCheck] = useState< Promise | undefined >(); - const resolveSecondFactorChoice = useRef< - | ((value: SecondFactorType | PromiseLike) => void) - | undefined - >(); const { - show: showSecondFactorChoice, - props: secondFactorChoiceVisibilityProps, - } = useModalVisibility(); + secondFactorChoiceProps, + userVerificationResultAfterResolvingSecondFactorChoice, + } = useSecondFactorChoiceIfNeeded(); const router = useRouter(); @@ -224,51 +217,19 @@ const Page: React.FC = ({ appContext }) => { if (sessionValidityCheck) await sessionValidityCheck; const cryptoWorker = await sharedCryptoWorker(); + const { keyAttributes, encryptedToken, token, id, - twoFactorSessionID: _twoFactorSessionIDV1, - twoFactorSessionIDV2: _twoFactorSessionIDV2, - passkeySessionID: _passkeySessionID, - } = await loginViaSRP(srpAttributes!, kek); - setIsFirstLogin(true); - - // When the user has both TOTP and pk set as the second factor, - // we'll get two session IDs. For backward compat, the TOTP - // session ID will be in a V2 attribute during a transient - // migration period. - // - // Note the use of || instead of ?? since _twoFactorSessionIDV1 - // will be an empty string, not undefined, if it is unset. We - // might need to add a `xxx-eslint-disable - // @typescript-eslint/prefer-nullish-coalescing` here too later. - const _twoFactorSessionID = - _twoFactorSessionIDV1 || _twoFactorSessionIDV2; - let passkeySessionID: string | undefined; - let twoFactorSessionID: string | undefined; - if (_twoFactorSessionID && _passkeySessionID) { - // If both factors are set, ask the user which one they wish - // to use. - const choice = await new Promise( - (resolve) => { - resolveSecondFactorChoice.current = resolve; - showSecondFactorChoice(); - }, + twoFactorSessionID, + passkeySessionID, + } = + await userVerificationResultAfterResolvingSecondFactorChoice( + await loginViaSRP(srpAttributes!, kek), ); - switch (choice) { - case "passkey": - passkeySessionID = _passkeySessionID; - break; - case "totp": - twoFactorSessionID = _twoFactorSessionID; - break; - } - } else { - passkeySessionID = _passkeySessionID; - twoFactorSessionID = _twoFactorSessionID; - } + setIsFirstLogin(true); if (passkeySessionID) { const sessionKeyAttributes = @@ -372,12 +333,6 @@ const Page: React.FC = ({ appContext }) => { } }; - const handleSecondFactorChoice = (factor: SecondFactorType) => { - const resolve = resolveSecondFactorChoice.current!; - resolveSecondFactorChoice.current = undefined; - resolve(factor); - }; - if (!keyAttributes && !srpAttributes) { return ( @@ -444,10 +399,7 @@ const Page: React.FC = ({ appContext }) => { - + ); }; From 4bca5f8bf6367560c9c50a75962f298854914d76 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 28 Nov 2024 12:35:31 +0530 Subject: [PATCH 3/3] Use --- .../components/utils/second-factor-choice.ts | 92 +++++++++++++++++++ web/packages/accounts/pages/credentials.tsx | 1 - web/packages/accounts/pages/verify.tsx | 12 ++- 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 web/packages/accounts/components/utils/second-factor-choice.ts diff --git a/web/packages/accounts/components/utils/second-factor-choice.ts b/web/packages/accounts/components/utils/second-factor-choice.ts new file mode 100644 index 0000000000..754691244d --- /dev/null +++ b/web/packages/accounts/components/utils/second-factor-choice.ts @@ -0,0 +1,92 @@ +/** + * @file This code is conceputally related to `SecondFactorChoice.tsx`, but + * needs to be in a separate file to allow fast refresh. + */ + +import { useModalVisibility } from "@/base/components/utils/modal"; +import { useCallback, useMemo, useRef } from "react"; +import type { UserVerificationResponse } from "../../services/user"; +import type { SecondFactorType } from "../SecondFactorChoice"; + +/** + * A convenience hook for keeping track of the state and logic that is needed + * after password verification to determine which second factor (if any) we + * should be asking the user for. + * + * This is a rather ad-hoc abstraction meant to be used in a very specific way; + * the only intent is to reduce code duplication between the two pages that need + * this choice. + */ +export const useSecondFactorChoiceIfNeeded = () => { + const resolveSecondFactorChoice = useRef< + | ((value: SecondFactorType | PromiseLike) => void) + | undefined + >(); + const { + show: showSecondFactorChoice, + props: secondFactorChoiceVisibilityProps, + } = useModalVisibility(); + + const onSelect = useCallback((factor: SecondFactorType) => { + const resolve = resolveSecondFactorChoice.current!; + resolveSecondFactorChoice.current = undefined; + resolve(factor); + }, []); + + const secondFactorChoiceProps = useMemo( + () => ({ ...secondFactorChoiceVisibilityProps, onSelect }), + [secondFactorChoiceVisibilityProps, onSelect], + ); + + const userVerificationResultAfterResolvingSecondFactorChoice = useCallback( + async (response: UserVerificationResponse) => { + const { + twoFactorSessionID: _twoFactorSessionIDV1, + twoFactorSessionIDV2: _twoFactorSessionIDV2, + passkeySessionID: _passkeySessionID, + } = response; + + // When the user has both TOTP and pk set as the second factor, + // we'll get two session IDs. For backward compat, the TOTP session + // ID will be in a V2 attribute during a transient migration period. + // + // Note the use of || instead of ?? since _twoFactorSessionIDV1 will + // be an empty string, not undefined, if it is unset. We might need + // to add a `xxx-eslint-disable + // @typescript-eslint/prefer-nullish-coalescing` here too later. + const _twoFactorSessionID = + _twoFactorSessionIDV1 || _twoFactorSessionIDV2; + + let passkeySessionID: string | undefined; + let twoFactorSessionID: string | undefined; + // If both factors are set, ask the user which one they want to use. + if (_twoFactorSessionID && _passkeySessionID) { + const choice = await new Promise( + (resolve) => { + resolveSecondFactorChoice.current = resolve; + showSecondFactorChoice(); + }, + ); + switch (choice) { + case "passkey": + passkeySessionID = _passkeySessionID; + break; + case "totp": + twoFactorSessionID = _twoFactorSessionID; + break; + } + } else { + passkeySessionID = _passkeySessionID; + twoFactorSessionID = _twoFactorSessionID; + } + + return { ...response, passkeySessionID, twoFactorSessionID }; + }, + [showSecondFactorChoice], + ); + + return { + secondFactorChoiceProps, + userVerificationResultAfterResolvingSecondFactorChoice, + }; +}; diff --git a/web/packages/accounts/pages/credentials.tsx b/web/packages/accounts/pages/credentials.tsx index 1cf601317e..89aa8d1770 100644 --- a/web/packages/accounts/pages/credentials.tsx +++ b/web/packages/accounts/pages/credentials.tsx @@ -217,7 +217,6 @@ const Page: React.FC = ({ appContext }) => { if (sessionValidityCheck) await sessionValidityCheck; const cryptoWorker = await sharedCryptoWorker(); - const { keyAttributes, encryptedToken, diff --git a/web/packages/accounts/pages/verify.tsx b/web/packages/accounts/pages/verify.tsx index 666a410b20..dfbeb387d4 100644 --- a/web/packages/accounts/pages/verify.tsx +++ b/web/packages/accounts/pages/verify.tsx @@ -30,6 +30,8 @@ import { LoginFlowFormFooter, VerifyingPasskey, } from "../components/LoginComponents"; +import { SecondFactorChoice } from "../components/SecondFactorChoice"; +import { useSecondFactorChoiceIfNeeded } from "../components/utils/second-factor-choice"; import { PAGES } from "../constants/pages"; import { openPasskeyVerificationURL, @@ -51,6 +53,10 @@ const Page: React.FC = ({ appContext }) => { const [passkeyVerificationData, setPasskeyVerificationData] = useState< { passkeySessionID: string; url: string } | undefined >(); + const { + secondFactorChoiceProps, + userVerificationResultAfterResolvingSecondFactorChoice, + } = useSecondFactorChoiceIfNeeded(); const router = useRouter(); @@ -83,7 +89,9 @@ const Page: React.FC = ({ appContext }) => { id, twoFactorSessionID, passkeySessionID, - } = resp.data as UserVerificationResponse; + } = await userVerificationResultAfterResolvingSecondFactorChoice( + resp.data as UserVerificationResponse, + ); if (passkeySessionID) { const user = getData(LS_KEYS.USER); await setLSUser({ @@ -243,6 +251,8 @@ const Page: React.FC = ({ appContext }) => { + + ); };