From 84a0cab16dd376156c95ca5f65b61e2c0c4139ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lloren=C3=A7=20Muntaner?= Date: Wed, 18 Dec 2024 13:43:32 +0100 Subject: [PATCH] Add nice UX when passkey is not found in that RP ID (#2754) * Add nice UX when passkey is not found in that RP ID * Fix test --- .../src/components/authenticateBox/index.ts | 37 +++++- .../src/components/infoToast/copy.json | 7 ++ .../src/components/infoToast/index.ts | 16 +++ .../src/flows/recovery/recoverWith/device.ts | 20 ++++ src/frontend/src/utils/iiConnection.test.ts | 106 +++++++++++++++++- src/frontend/src/utils/iiConnection.ts | 12 +- 6 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 src/frontend/src/components/infoToast/copy.json create mode 100644 src/frontend/src/components/infoToast/index.ts diff --git a/src/frontend/src/components/authenticateBox/index.ts b/src/frontend/src/components/authenticateBox/index.ts index 5a29edd310..09cd29d81e 100644 --- a/src/frontend/src/components/authenticateBox/index.ts +++ b/src/frontend/src/components/authenticateBox/index.ts @@ -33,6 +33,7 @@ import { InvalidCaller, LoginSuccess, NoRegistrationFlow, + PossiblyWrongRPID, RateLimitExceeded, RegisterNoSpace, UnexpectedCall, @@ -50,6 +51,8 @@ import { import { DerEncodedPublicKey } from "@dfinity/agent"; import { isNullish, nonNullish } from "@dfinity/utils"; import { TemplateResult, html, render } from "lit-html"; +import { infoToastTemplate } from "../infoToast"; +import infoToastCopy from "../infoToast/copy.json"; /** Template used for rendering specific authentication screens. See `authnScreens` below * for meaning of "firstTime", "useExisting" and "pick". */ @@ -189,7 +192,12 @@ export const authenticateBoxFlow = async ({ loginPasskey: ( userNumber: bigint ) => Promise< - LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError + | LoginSuccess + | AuthFail + | WebAuthnFailed + | PossiblyWrongRPID + | UnknownUser + | ApiError >; loginPinIdentityMaterial: ({ userNumber, @@ -218,6 +226,7 @@ export const authenticateBoxFlow = async ({ newAnchor: boolean; authnMethod: "pin" | "passkey" | "recovery"; }) + | PossiblyWrongRPID | FlowError | { tag: "canceled" } | { tag: "deviceAdded" } @@ -267,6 +276,7 @@ export const authenticateBoxFlow = async ({ newAnchor: boolean; authnMethod: "pin" | "passkey" | "recovery"; }) + | PossiblyWrongRPID | FlowError | { tag: "canceled" } | { tag: "deviceAdded" } @@ -345,7 +355,7 @@ export type FlowError = | RegisterNoSpace; export const handleLoginFlowResult = async ( - result: (LoginSuccess & E) | FlowError + result: (LoginSuccess & E) | PossiblyWrongRPID | FlowError ): Promise< ({ userNumber: bigint; connection: AuthenticatedConnection } & E) | undefined > => { @@ -354,6 +364,21 @@ export const handleLoginFlowResult = async ( return result; } + if (result.kind === "possiblyWrongRPID") { + const i18n = new I18n(); + const copy = i18n.i18n(infoToastCopy); + toast.info( + infoToastTemplate({ + title: copy.title_possibly_wrong_rp_id, + messages: [ + copy.message_possibly_wrong_rp_id_1, + copy.message_possibly_wrong_rp_id_2, + ], + }) + ); + return undefined; + } + result satisfies FlowError; toast.error(flowErrorToastTemplate(result)); @@ -657,7 +682,12 @@ const useIdentityFlow = async ({ loginPasskey: ( userNumber: bigint ) => Promise< - LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError + | LoginSuccess + | AuthFail + | WebAuthnFailed + | PossiblyWrongRPID + | UnknownUser + | ApiError >; allowPinLogin: boolean; verifyPinValidity: (opts: { @@ -680,6 +710,7 @@ const useIdentityFlow = async ({ }) | AuthFail | WebAuthnFailed + | PossiblyWrongRPID | UnknownUser | ApiError | BadPin diff --git a/src/frontend/src/components/infoToast/copy.json b/src/frontend/src/components/infoToast/copy.json new file mode 100644 index 0000000000..49509599a8 --- /dev/null +++ b/src/frontend/src/components/infoToast/copy.json @@ -0,0 +1,7 @@ +{ + "en": { + "title_possibly_wrong_rp_id": "Please try again", + "message_possibly_wrong_rp_id_1": "The wrong domain was set for the passkey and the browser couldn't find it.", + "message_possibly_wrong_rp_id_2": "Try again and another domain will be used." + } +} diff --git a/src/frontend/src/components/infoToast/index.ts b/src/frontend/src/components/infoToast/index.ts new file mode 100644 index 0000000000..62d36b1513 --- /dev/null +++ b/src/frontend/src/components/infoToast/index.ts @@ -0,0 +1,16 @@ +import { DynamicKey } from "$src/i18n"; +import { html } from "lit-html"; + +export const infoToastTemplate = ({ + title, + messages, +}: { + title: string | DynamicKey; + messages: string[] | DynamicKey[]; +}) => html` +

${title}

+ ${messages.map( + (message) => + html`

${message}

` + )} +`; diff --git a/src/frontend/src/flows/recovery/recoverWith/device.ts b/src/frontend/src/flows/recovery/recoverWith/device.ts index 5cea032d8b..62e152e8dc 100644 --- a/src/frontend/src/flows/recovery/recoverWith/device.ts +++ b/src/frontend/src/flows/recovery/recoverWith/device.ts @@ -1,11 +1,15 @@ import { CredentialId, DeviceData } from "$generated/internet_identity_types"; +import { infoToastTemplate } from "$src/components/infoToast"; +import infoToastCopy from "$src/components/infoToast/copy.json"; import { promptUserNumberTemplate } from "$src/components/promptUserNumber"; import { toast } from "$src/components/toast"; +import { I18n } from "$src/i18n"; import { convertToCredentialData } from "$src/utils/credential-devices"; import { AuthFail, Connection, LoginSuccess, + PossiblyWrongRPID, WebAuthnFailed, } from "$src/utils/iiConnection"; import { renderPage } from "$src/utils/lit-html"; @@ -50,6 +54,21 @@ export const recoverWithDevice = ({ } if (result.kind !== "loginSuccess") { + if (result.kind === "possiblyWrongRPID") { + const i18n = new I18n(); + const copy = i18n.i18n(infoToastCopy); + toast.info( + infoToastTemplate({ + title: copy.title_possibly_wrong_rp_id, + messages: [ + copy.message_possibly_wrong_rp_id_1, + copy.message_possibly_wrong_rp_id_2, + ], + }) + ); + return; + } + result satisfies AuthFail | WebAuthnFailed; toast.error("Could not authenticate using the device"); return; @@ -72,6 +91,7 @@ const attemptRecovery = async ({ }): Promise< | LoginSuccess | WebAuthnFailed + | PossiblyWrongRPID | AuthFail | { kind: "noRecovery" } | { kind: "tooManyRecovery" } diff --git a/src/frontend/src/utils/iiConnection.test.ts b/src/frontend/src/utils/iiConnection.test.ts index 4addc3c9f5..324bf0da0f 100644 --- a/src/frontend/src/utils/iiConnection.test.ts +++ b/src/frontend/src/utils/iiConnection.test.ts @@ -176,7 +176,7 @@ describe("Connection.login", () => { } }); - it("connection exludes rpId when user cancels", async () => { + it("connection excludes rpId when user cancels", async () => { // This one would fail because it's not the device the user is using at the moment. const currentOriginDevice: DeviceData = createMockDevice(currentOrigin); const currentOriginCredentialData = @@ -193,7 +193,7 @@ describe("Connection.login", () => { failSign = true; const firstLoginResult = await connection.login(BigInt(12345)); - expect(firstLoginResult.kind).toBe("webAuthnFailed"); + expect(firstLoginResult.kind).toBe("possiblyWrongRPID"); expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( expect.arrayContaining([ @@ -327,7 +327,107 @@ describe("Connection.login", () => { } }); - it("connection does not exlud rpId when user cancels", async () => { + it("connection does not exclude rpId when user cancels", async () => { + const currentOriginDevice: DeviceData = createMockDevice(currentOrigin); + const currentOriginCredentialData = + convertToCredentialData(currentOriginDevice); + const currentDevice: DeviceData = createMockDevice(); + const currentDeviceCredentialData = + convertToCredentialData(currentDevice); + const mockActor = { + identity_info: vi.fn().mockResolvedValue({ Ok: { metadata: [] } }), + lookup: vi.fn().mockResolvedValue([currentOriginDevice, currentDevice]), + } as unknown as ActorSubclass<_SERVICE>; + const connection = new Connection("aaaaa-aa", mockActor); + + failSign = true; + const firstLoginResult = await connection.login(BigInt(12345)); + + expect(firstLoginResult.kind).toBe("webAuthnFailed"); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + expect.arrayContaining([ + currentOriginCredentialData, + currentDeviceCredentialData, + ]), + undefined + ); + + failSign = false; + const secondLoginResult = await connection.login(BigInt(12345)); + + expect(secondLoginResult.kind).toBe("loginSuccess"); + if (secondLoginResult.kind === "loginSuccess") { + expect(secondLoginResult.connection).toBeInstanceOf( + AuthenticatedConnection + ); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(2); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenNthCalledWith( + 2, + expect.arrayContaining([ + currentDeviceCredentialData, + currentOriginCredentialData, + ]), + undefined + ); + } + }); + }); + + describe("domains compatibility flag enabled and browser doesn't support", () => { + beforeEach(() => { + DOMAIN_COMPATIBILITY.set(true); + vi.stubGlobal("navigator", { + // Does NOT Supports RoR + userAgent: + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:133.0) Gecko/20100101 Firefox/133.0", + }); + }); + + it("login returns authenticated connection without rpID if browser doesn't support it", async () => { + const connection = new Connection("aaaaa-aa", mockActor); + + const loginResult = await connection.login(BigInt(12345)); + + expect(loginResult.kind).toBe("loginSuccess"); + if (loginResult.kind === "loginSuccess") { + expect(loginResult.connection).toBeInstanceOf(AuthenticatedConnection); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + [convertToCredentialData(mockDevice)], + undefined + ); + } + }); + }); + + describe("domains compatibility flag disabled", () => { + beforeEach(() => { + DOMAIN_COMPATIBILITY.set(false); + vi.stubGlobal("navigator", { + // Supports RoR + userAgent: + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.0 Safari/605.1.15", + }); + }); + + it("login returns authenticated connection without rpID if flag is not enabled", async () => { + const connection = new Connection("aaaaa-aa", mockActor); + + const loginResult = await connection.login(BigInt(12345)); + + expect(loginResult.kind).toBe("loginSuccess"); + if (loginResult.kind === "loginSuccess") { + expect(loginResult.connection).toBeInstanceOf(AuthenticatedConnection); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + [convertToCredentialData(mockDevice)], + undefined + ); + } + }); + + it("connection does not exclude rpId when user cancels", async () => { const currentOriginDevice: DeviceData = createMockDevice(currentOrigin); const currentOriginCredentialData = convertToCredentialData(currentOriginDevice); diff --git a/src/frontend/src/utils/iiConnection.ts b/src/frontend/src/utils/iiConnection.ts index 9813d4e257..518d885814 100644 --- a/src/frontend/src/utils/iiConnection.ts +++ b/src/frontend/src/utils/iiConnection.ts @@ -124,6 +124,7 @@ export type RegisterNoSpace = { kind: "registerNoSpace" }; export type NoSeedPhrase = { kind: "noSeedPhrase" }; export type SeedPhraseFail = { kind: "seedPhraseFail" }; export type WebAuthnFailed = { kind: "webAuthnFailed" }; +export type PossiblyWrongRPID = { kind: "possiblyWrongRPID" }; export type InvalidAuthnMethod = { kind: "invalidAuthnMethod"; message: string; @@ -360,7 +361,12 @@ export class Connection { login = async ( userNumber: bigint ): Promise< - LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError + | LoginSuccess + | AuthFail + | WebAuthnFailed + | PossiblyWrongRPID + | UnknownUser + | ApiError > => { let devices: Omit[]; try { @@ -386,7 +392,7 @@ export class Connection { fromWebauthnCredentials = async ( userNumber: bigint, credentials: CredentialData[] - ): Promise => { + ): Promise => { const cancelledRpIds = this._cancelledRpIds.get(userNumber) ?? new Set(); const currentOrigin = window.location.origin; const dynamicRPIdEnabled = @@ -426,6 +432,8 @@ export class Connection { } else { this._cancelledRpIds.set(userNumber, new Set([rpId])); } + // We want to user to retry again and a new RP ID will be used. + return { kind: "possiblyWrongRPID" }; } return { kind: "webAuthnFailed" }; }