Skip to content

Commit

Permalink
Add nice UX when passkey is not found in that RP ID (#2754)
Browse files Browse the repository at this point in the history
* Add nice UX when passkey is not found in that RP ID

* Fix test
  • Loading branch information
lmuntaner authored Dec 18, 2024
1 parent e3e48d0 commit 84a0cab
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 8 deletions.
37 changes: 34 additions & 3 deletions src/frontend/src/components/authenticateBox/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
InvalidCaller,
LoginSuccess,
NoRegistrationFlow,
PossiblyWrongRPID,
RateLimitExceeded,
RegisterNoSpace,
UnexpectedCall,
Expand All @@ -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". */
Expand Down Expand Up @@ -189,7 +192,12 @@ export const authenticateBoxFlow = async <I>({
loginPasskey: (
userNumber: bigint
) => Promise<
LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError
| LoginSuccess
| AuthFail
| WebAuthnFailed
| PossiblyWrongRPID
| UnknownUser
| ApiError
>;
loginPinIdentityMaterial: ({
userNumber,
Expand Down Expand Up @@ -218,6 +226,7 @@ export const authenticateBoxFlow = async <I>({
newAnchor: boolean;
authnMethod: "pin" | "passkey" | "recovery";
})
| PossiblyWrongRPID
| FlowError
| { tag: "canceled" }
| { tag: "deviceAdded" }
Expand Down Expand Up @@ -267,6 +276,7 @@ export const authenticateBoxFlow = async <I>({
newAnchor: boolean;
authnMethod: "pin" | "passkey" | "recovery";
})
| PossiblyWrongRPID
| FlowError
| { tag: "canceled" }
| { tag: "deviceAdded" }
Expand Down Expand Up @@ -345,7 +355,7 @@ export type FlowError =
| RegisterNoSpace;

export const handleLoginFlowResult = async <E>(
result: (LoginSuccess & E) | FlowError
result: (LoginSuccess & E) | PossiblyWrongRPID | FlowError
): Promise<
({ userNumber: bigint; connection: AuthenticatedConnection } & E) | undefined
> => {
Expand All @@ -354,6 +364,21 @@ export const handleLoginFlowResult = async <E>(
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));
Expand Down Expand Up @@ -657,7 +682,12 @@ const useIdentityFlow = async <I>({
loginPasskey: (
userNumber: bigint
) => Promise<
LoginSuccess | AuthFail | WebAuthnFailed | UnknownUser | ApiError
| LoginSuccess
| AuthFail
| WebAuthnFailed
| PossiblyWrongRPID
| UnknownUser
| ApiError
>;
allowPinLogin: boolean;
verifyPinValidity: (opts: {
Expand All @@ -680,6 +710,7 @@ const useIdentityFlow = async <I>({
})
| AuthFail
| WebAuthnFailed
| PossiblyWrongRPID
| UnknownUser
| ApiError
| BadPin
Expand Down
7 changes: 7 additions & 0 deletions src/frontend/src/components/infoToast/copy.json
Original file line number Diff line number Diff line change
@@ -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."
}
}
16 changes: 16 additions & 0 deletions src/frontend/src/components/infoToast/index.ts
Original file line number Diff line number Diff line change
@@ -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`
<h3 class="t-title c-card__title">${title}</h3>
${messages.map(
(message) =>
html`<p data-role="info-message" class="t-paragraph">${message}</p>`
)}
`;
20 changes: 20 additions & 0 deletions src/frontend/src/flows/recovery/recoverWith/device.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
Expand All @@ -72,6 +91,7 @@ const attemptRecovery = async ({
}): Promise<
| LoginSuccess
| WebAuthnFailed
| PossiblyWrongRPID
| AuthFail
| { kind: "noRecovery" }
| { kind: "tooManyRecovery" }
Expand Down
106 changes: 103 additions & 3 deletions src/frontend/src/utils/iiConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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([
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions src/frontend/src/utils/iiConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<DeviceData, "alias">[];
try {
Expand All @@ -386,7 +392,7 @@ export class Connection {
fromWebauthnCredentials = async (
userNumber: bigint,
credentials: CredentialData[]
): Promise<LoginSuccess | WebAuthnFailed | AuthFail> => {
): Promise<LoginSuccess | WebAuthnFailed | PossiblyWrongRPID | AuthFail> => {
const cancelledRpIds = this._cancelledRpIds.get(userNumber) ?? new Set();
const currentOrigin = window.location.origin;
const dynamicRPIdEnabled =
Expand Down Expand Up @@ -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" };
}
Expand Down

0 comments on commit 84a0cab

Please sign in to comment.