From 39396a520c6134f3486b89997ccf063afdc522aa Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 09:20:30 +0530 Subject: [PATCH 1/6] Add some user visible strings to localization table --- web/apps/accounts/src/pages/passkeys/index.tsx | 16 +++++++--------- web/apps/accounts/src/pages/passkeys/verify.tsx | 3 +-- web/packages/next/locales/en-US/translation.json | 3 +++ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/web/apps/accounts/src/pages/passkeys/index.tsx b/web/apps/accounts/src/pages/passkeys/index.tsx index fa6ecde21d..f9ba48e90f 100644 --- a/web/apps/accounts/src/pages/passkeys/index.tsx +++ b/web/apps/accounts/src/pages/passkeys/index.tsx @@ -91,12 +91,11 @@ const Page: React.FC = () => { // If the user cancels the operation, then an error with name // "NotAllowedError" is thrown. // - // Ignore this, but in other cases add an error indicator to the add - // passkey text field. The browser is expected to already have shown - // an error dialog to the user. + // Ignore these, but in other cases add an error indicator to the + // add passkey text field. The browser is expected to already have + // shown an error dialog to the user. if (!(e instanceof Error && e.name == "NotAllowedError")) { - // TODO-PK: localize - setFieldError("Could not add passkey"); + setFieldError(t("passkey_add_failed")); } return; } @@ -232,8 +231,7 @@ const ManagePasskeyDrawer: React.FC = ({ = ({ setShowRenameDialog(true); }} startIcon={} - label={"Rename Passkey"} + label={t("RENAME_PASSKEY")} /> = ({ setShowDeleteDialog(true); }} startIcon={} - label={"Delete Passkey"} + label={t("DELETE_PASSKEY")} color="critical" /> diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 94ff3e4481..97de676d1f 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -152,8 +152,7 @@ const UnknownRedirect: React.FC = () => { }; const WebAuthnNotSupported: React.FC = () => { - // TODO-PK(MR): Translate - return ; + return ; }; const UnrecoverableFailure: React.FC = () => { diff --git a/web/packages/next/locales/en-US/translation.json b/web/packages/next/locales/en-US/translation.json index 7111380d29..0cef58dfeb 100644 --- a/web/packages/next/locales/en-US/translation.json +++ b/web/packages/next/locales/en-US/translation.json @@ -610,6 +610,7 @@ "APPLY_CROP": "Apply Crop", "PHOTO_EDIT_REQUIRED_TO_SAVE": "At least one transformation or color adjustment must be performed before saving.", "PASSKEYS": "Passkeys", + "manage_passkey": "Manage passkey", "DELETE_PASSKEY": "Delete passkey", "DELETE_PASSKEY_CONFIRMATION": "Are you sure you want to delete this passkey? This action is irreversible.", "RENAME_PASSKEY": "Rename passkey", @@ -617,9 +618,11 @@ "ENTER_PASSKEY_NAME": "Enter passkey name", "PASSKEYS_DESCRIPTION": "Passkeys are a modern and secure second-factor for your Ente account. They use on-device biometric authentication for convenience and security.", "CREATED_AT": "Created at", + "passkey_add_failed": "Could not add passkey", "PASSKEY_LOGIN_FAILED": "Passkey login failed", "PASSKEY_LOGIN_URL_INVALID": "The login URL is invalid.", "PASSKEY_LOGIN_ERRORED": "An error occurred while logging in with passkey.", + "passkeys_not_supported": "Passkeys are not supported in this browser", "TRY_AGAIN": "Try again", "PASSKEY_FOLLOW_THE_STEPS_FROM_YOUR_BROWSER": "Follow the steps from your browser to continue logging in.", "LOGIN_WITH_PASSKEY": "Login with passkey", From d9e6379020e3fef9c00d9c3b38565e50f0217b8f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 09:34:29 +0530 Subject: [PATCH 2/6] Better JSON urlencode --- .../shared/crypto/internal/libsodium.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/web/packages/shared/crypto/internal/libsodium.ts b/web/packages/shared/crypto/internal/libsodium.ts index f23cabbc70..673d8facb3 100644 --- a/web/packages/shared/crypto/internal/libsodium.ts +++ b/web/packages/shared/crypto/internal/libsodium.ts @@ -414,6 +414,15 @@ export const toB64URLSafe = async (input: Uint8Array) => { * This differs from {@link toB64URLSafe} in that it does not append any * trailing padding character(s) "=" to make the resultant string's length be an * integer multiple of 4. + * + * - In some contexts, for example when serializing WebAuthn binary for + * transmission over the network, this is the required / recommended + * approach. + * + * - In other cases, for example when trying to pass an arbitrary JSON string + * via a URL parameter, this is also convenient so that we do not have to + * deal with any ambiguity surrounding the "=" which is also the query + * parameter key value separator. */ export const toB64URLSafeNoPadding = async (input: Uint8Array) => { await sodium.ready; @@ -431,6 +440,24 @@ export const fromB64URLSafeNoPadding = async (input: string) => { return sodium.from_base64(input, sodium.base64_variants.URLSAFE_NO_PADDING); }; +/** + * Variant of {@link toB64URLSafeNoPadding} that works with {@link strings}. See also + * its sibling method {@link fromB64URLSafeNoPaddingString}. + */ +export const toB64URLSafeNoPaddingString = async (input: string) => { + await sodium.ready; + return toB64URLSafeNoPadding(sodium.from_string(input)); +}; + +/** + * Variant of {@link fromB64URLSafeNoPadding} that works with {@link strings}. See also + * its sibling method {@link toB64URLSafeNoPaddingString}. + */ +export const fromB64URLSafeNoPaddingString = async (input: string) => { + await sodium.ready; + return sodium.to_string(await fromB64URLSafeNoPadding(input)); +}; + export async function fromUTF8(input: string) { await sodium.ready; return sodium.from_string(input); From f40da137cd39bbe2b3ed3be827dc02c22f535bc7 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 09:42:12 +0530 Subject: [PATCH 3/6] Use --- .../accounts/src/pages/passkeys/verify.tsx | 5 ++++- web/apps/accounts/src/services/passkey.ts | 10 +++++----- .../accounts/pages/passkeys/finish.tsx | 20 +++++++++++-------- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 97de676d1f..9041a5e400 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -116,7 +116,10 @@ const Page = () => { // Conceptually we can `setStatus("done")` at this point, but we'll // leave this page anyway, so no need to tickle React. - redirectAfterPasskeyAuthentication(redirectURL, authorizationResponse); + await redirectAfterPasskeyAuthentication( + redirectURL, + authorizationResponse, + ); }; useEffect(() => { diff --git a/web/apps/accounts/src/services/passkey.ts b/web/apps/accounts/src/services/passkey.ts index 2c7d4fe69a..99035e55b9 100644 --- a/web/apps/accounts/src/services/passkey.ts +++ b/web/apps/accounts/src/services/passkey.ts @@ -6,10 +6,10 @@ import { nullToUndefined } from "@/utils/transform"; import { fromB64URLSafeNoPadding, toB64URLSafeNoPadding, + toB64URLSafeNoPaddingString, } from "@ente/shared/crypto/internal/libsodium"; import { apiOrigin } from "@ente/shared/network/api"; import { getToken } from "@ente/shared/storage/localStorage/helpers"; -import _sodium from "libsodium-wrappers"; import { z } from "zod"; /** Return true if the user's browser supports WebAuthn (Passkeys). */ @@ -511,14 +511,14 @@ const authenticatorAssertionResponse = (credential: Credential) => { * @param twoFactorAuthorizationResponse The result of * {@link finishPasskeyAuthentication} returned by the backend. */ -export const redirectAfterPasskeyAuthentication = ( +export const redirectAfterPasskeyAuthentication = async ( redirectURL: URL, twoFactorAuthorizationResponse: TwoFactorAuthorizationResponse, ) => { - const encodedResponse = _sodium.to_base64( + const encodedResponse = await toB64URLSafeNoPaddingString( JSON.stringify(twoFactorAuthorizationResponse), ); - // TODO-PK: Shouldn't this be URL encoded? - window.location.href = `${redirectURL}?response=${encodedResponse}`; + redirectURL.searchParams.set("response", encodedResponse) + window.location.href = redirectURL.href; }; diff --git a/web/packages/accounts/pages/passkeys/finish.tsx b/web/packages/accounts/pages/passkeys/finish.tsx index 99e1e3f1da..cd866f6e11 100644 --- a/web/packages/accounts/pages/passkeys/finish.tsx +++ b/web/packages/accounts/pages/passkeys/finish.tsx @@ -1,5 +1,6 @@ import { VerticallyCentered } from "@ente/shared/components/Container"; import EnteSpinner from "@ente/shared/components/EnteSpinner"; +import { fromB64URLSafeNoPaddingString } from "@ente/shared/crypto/internal/libsodium"; import InMemoryStore, { MS_KEYS } from "@ente/shared/storage/InMemoryStore"; import { LS_KEYS, getData, setData } from "@ente/shared/storage/localStorage"; import { useRouter } from "next/router"; @@ -23,11 +24,11 @@ const Page: React.FC = () => { const response = searchParams.get("response"); if (!response) return; - saveCredentials(response); - - const redirectURL = InMemoryStore.get(MS_KEYS.REDIRECT_URL); - InMemoryStore.delete(MS_KEYS.REDIRECT_URL); - router.push(redirectURL ?? PAGES.CREDENTIALS); + saveCredentials(response).then(() => { + const redirectURL = InMemoryStore.get(MS_KEYS.REDIRECT_URL); + InMemoryStore.delete(MS_KEYS.REDIRECT_URL); + router.push(redirectURL ?? PAGES.CREDENTIALS); + }); }, []); return ( @@ -47,9 +48,12 @@ export default Page; * @param response The string that is passed as the response query parameter to * us (we're the final "finish" page in the passkey flow). */ -const saveCredentials = (response: string) => { - // Decode response string. - const decodedResponse = JSON.parse(atob(response)); +const saveCredentials = async (response: string) => { + // Decode response string (inverse of the steps we perform in + // `redirectAfterPasskeyAuthentication`). + const decodedResponse = JSON.parse( + await fromB64URLSafeNoPaddingString(response), + ); // Only one of `encryptedToken` or `token` will be present depending on the // account's lifetime: From 055cada5ed8cb4942073098b9fe58cee463553e5 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 10:00:35 +0530 Subject: [PATCH 4/6] Show error when pk fetch fails --- web/apps/accounts/src/pages/passkeys/index.tsx | 7 ++++++- web/packages/next/locales/en-US/translation.json | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/web/apps/accounts/src/pages/passkeys/index.tsx b/web/apps/accounts/src/pages/passkeys/index.tsx index f9ba48e90f..a476250417 100644 --- a/web/apps/accounts/src/pages/passkeys/index.tsx +++ b/web/apps/accounts/src/pages/passkeys/index.tsx @@ -31,7 +31,7 @@ import { } from "services/passkey"; const Page: React.FC = () => { - const { showNavBar } = useAppContext(); + const { showNavBar, setDialogBoxAttributesV2 } = useAppContext(); const [passkeys, setPasskeys] = useState([]); const [showPasskeyDrawer, setShowPasskeyDrawer] = useState(false); @@ -46,6 +46,11 @@ const Page: React.FC = () => { setPasskeys(await getPasskeys()); } catch (e) { log.error("Failed to fetch passkeys", e); + setDialogBoxAttributesV2({ + title: t("ERROR"), + content: t("passkey_fetch_failed"), + close: {}, + }); } }; diff --git a/web/packages/next/locales/en-US/translation.json b/web/packages/next/locales/en-US/translation.json index 0cef58dfeb..1d1fbd0315 100644 --- a/web/packages/next/locales/en-US/translation.json +++ b/web/packages/next/locales/en-US/translation.json @@ -610,6 +610,7 @@ "APPLY_CROP": "Apply Crop", "PHOTO_EDIT_REQUIRED_TO_SAVE": "At least one transformation or color adjustment must be performed before saving.", "PASSKEYS": "Passkeys", + "passkey_fetch_failed": "Could not get your passkeys.", "manage_passkey": "Manage passkey", "DELETE_PASSKEY": "Delete passkey", "DELETE_PASSKEY_CONFIRMATION": "Are you sure you want to delete this passkey? This action is irreversible.", From d814511dae7f256b3b6317a8cddb062191f79f6a Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 10:42:18 +0530 Subject: [PATCH 5/6] Named params Reduces accidental order of param errors --- .../accounts/src/pages/passkeys/verify.tsx | 4 +- web/apps/accounts/src/services/passkey.ts | 39 +++++++++++++------ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/web/apps/accounts/src/pages/passkeys/verify.tsx b/web/apps/accounts/src/pages/passkeys/verify.tsx index 9041a5e400..ebd391372b 100644 --- a/web/apps/accounts/src/pages/passkeys/verify.tsx +++ b/web/apps/accounts/src/pages/passkeys/verify.tsx @@ -102,11 +102,11 @@ const Page = () => { setStatus("loading"); - authorizationResponse = await finishPasskeyAuthentication( + authorizationResponse = await finishPasskeyAuthentication({ passkeySessionID, ceremonySessionID, credential, - ); + }); } catch (e) { log.error("Passkey authentication failed", e); setStatus("failed"); diff --git a/web/apps/accounts/src/services/passkey.ts b/web/apps/accounts/src/services/passkey.ts index 99035e55b9..39237b58a3 100644 --- a/web/apps/accounts/src/services/passkey.ts +++ b/web/apps/accounts/src/services/passkey.ts @@ -116,7 +116,11 @@ export const registerPasskey = async (name: string) => { // Finish by letting the backend know about these credentials so that it can // save the public key for future authentication. - await finishPasskeyRegistration(name, sessionID, credential); + await finishPasskeyRegistration({ + friendlyName: name, + sessionID, + credential, + }); }; interface BeginPasskeyRegistrationResponse { @@ -256,11 +260,17 @@ const binaryToServerB64 = async (b: ArrayBuffer) => { return b64String as unknown as BufferSource; }; -const finishPasskeyRegistration = async ( - sessionID: string, - friendlyName: string, - credential: Credential, -) => { +interface FinishPasskeyRegistrationOptions { + sessionID: string; + friendlyName: string; + credential: Credential; +} + +const finishPasskeyRegistration = async ({ + sessionID, + friendlyName, + credential, +}: FinishPasskeyRegistrationOptions) => { const attestationResponse = authenticatorAttestationResponse(credential); const attestationObject = await binaryToServerB64( @@ -423,6 +433,11 @@ export const signChallenge = async ( return await navigator.credentials.get({ publicKey }); }; +interface FinishPasskeyAuthenticationOptions { + passkeySessionID: string; + ceremonySessionID: string; + credential: Credential; +} /** * Finish the authentication by providing the signed assertion to the backend. * @@ -432,11 +447,11 @@ export const signChallenge = async ( * @returns The result of successful authentication, a * {@link TwoFactorAuthorizationResponse}. */ -export const finishPasskeyAuthentication = async ( - passkeySessionID: string, - ceremonySessionID: string, - credential: Credential, -) => { +export const finishPasskeyAuthentication = async ({ + passkeySessionID, + ceremonySessionID, + credential, +}: FinishPasskeyAuthenticationOptions) => { const response = authenticatorAssertionResponse(credential); const authenticatorData = await binaryToServerB64( @@ -519,6 +534,6 @@ export const redirectAfterPasskeyAuthentication = async ( JSON.stringify(twoFactorAuthorizationResponse), ); - redirectURL.searchParams.set("response", encodedResponse) + redirectURL.searchParams.set("response", encodedResponse); window.location.href = redirectURL.href; }; From 6a8cb8d1497bb9a36089284cd3b0283be7d572d9 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 10 Jun 2024 10:54:33 +0530 Subject: [PATCH 6/6] Retain original transports --- web/apps/accounts/src/services/passkey.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/apps/accounts/src/services/passkey.ts b/web/apps/accounts/src/services/passkey.ts index 39237b58a3..522df6996b 100644 --- a/web/apps/accounts/src/services/passkey.ts +++ b/web/apps/accounts/src/services/passkey.ts @@ -279,6 +279,7 @@ const finishPasskeyRegistration = async ({ const clientDataJSON = await binaryToServerB64( attestationResponse.clientDataJSON, ); + const transports = attestationResponse.getTransports(); const params = new URLSearchParams({ friendlyName, sessionID }); const url = `${apiOrigin()}/passkeys/registration/finish`; @@ -295,6 +296,7 @@ const finishPasskeyRegistration = async ({ response: { attestationObject, clientDataJSON, + transports, }, }), });