diff --git a/src/frontend/src/lib/utils/findWebAuthnFlows.test.ts b/src/frontend/src/lib/utils/findWebAuthnFlows.test.ts index cd71c20bfb..01500cc38c 100644 --- a/src/frontend/src/lib/utils/findWebAuthnFlows.test.ts +++ b/src/frontend/src/lib/utils/findWebAuthnFlows.test.ts @@ -2,13 +2,22 @@ import { LEGACY_II_URL } from "$lib/config"; import { CredentialData } from "./credential-devices"; import { findWebAuthnFlows } from "./findWebAuthnFlows"; +vi.mock("$lib/globals", () => ({ + canisterConfig: { + new_flow_origins: [["https://id.ai"]], + }, +})); + describe("findWebAuthnFlows", () => { + const newOrigin = "https://id.ai"; + const newOriginRpId = new URL(newOrigin).hostname; const currentOrigin = "https://identity.internetcomputer.org"; const nonCurrentOrigin1 = "https://identity.ic0.app"; const nonCurrentOrigin1RpId = new URL(nonCurrentOrigin1).hostname; const nonCurrentOrigin2 = "https://identity.icp0.io"; const nonCurrentOrigin2RpId = new URL(nonCurrentOrigin2).hostname; const relatedOrigins = [ + newOrigin, "https://identity.ic0.app", "https://identity.internetcomputer.org", "https://identity.icp0.io", @@ -149,4 +158,77 @@ describe("findWebAuthnFlows", () => { { useIframe: true, rpId: nonCurrentOrigin1RpId }, ]); }); + + it("pushes RP IDs from new_flow_origins to the end while preserving relative order for some permutations", () => { + const test_cases = [ + { + label: "newOrigin first, 1st currentOrigin before nonCurrentOrigin1", + devices: [ + createMockCredential(newOrigin), + createMockCredential(currentOrigin), + createMockCredential(nonCurrentOrigin1), + createMockCredential(currentOrigin), + ], + expectedOrder: [ + { useIframe: false, rpId: undefined }, + { useIframe: true, rpId: nonCurrentOrigin1RpId }, + { useIframe: true, rpId: newOriginRpId }, + ], + }, + { + label: "newOrigin first, 1st currentOrigin after nonCurrentOrigin1", + devices: [ + createMockCredential(newOrigin), + createMockCredential(nonCurrentOrigin1), + createMockCredential(currentOrigin), + createMockCredential(currentOrigin), + ], + expectedOrder: [ + { useIframe: true, rpId: nonCurrentOrigin1RpId }, + { useIframe: false, rpId: undefined }, + { useIframe: true, rpId: newOriginRpId }, + ], + }, + { + label: "newOrigin last, 1st currentOrigin after nonCurrentOrigin1", + devices: [ + createMockCredential(nonCurrentOrigin1), + createMockCredential(currentOrigin), + createMockCredential(currentOrigin), + createMockCredential(newOrigin), + ], + expectedOrder: [ + { useIframe: true, rpId: nonCurrentOrigin1RpId }, + { useIframe: false, rpId: undefined }, + { useIframe: true, rpId: newOriginRpId }, + ], + }, + { + label: "triplicate currentOrigin", + devices: [ + createMockCredential(newOrigin), + createMockCredential(currentOrigin), + createMockCredential(nonCurrentOrigin1), + createMockCredential(currentOrigin), + createMockCredential(currentOrigin), + ], + expectedOrder: [ + { useIframe: false, rpId: undefined }, + { useIframe: true, rpId: nonCurrentOrigin1RpId }, + { useIframe: true, rpId: newOriginRpId }, + ], + }, + ]; + + for (const { label, devices, expectedOrder } of test_cases) { + const result = findWebAuthnFlows({ + supportsRor: true, // irrelevant for this test + devices, + currentOrigin, + relatedOrigins: [currentOrigin, nonCurrentOrigin1, newOrigin], + }); + + expect(result, label).toEqual(expectedOrder); + } + }); }); diff --git a/src/frontend/src/lib/utils/findWebAuthnFlows.ts b/src/frontend/src/lib/utils/findWebAuthnFlows.ts index 0178964108..943ce02f49 100644 --- a/src/frontend/src/lib/utils/findWebAuthnFlows.ts +++ b/src/frontend/src/lib/utils/findWebAuthnFlows.ts @@ -1,6 +1,8 @@ import { II_LEGACY_ORIGIN } from "$lib/legacy/constants"; import { isNullish, nonNullish } from "@dfinity/utils"; import { CredentialData } from "./credential-devices"; +import { canisterConfig } from "$lib/globals"; +import { isSameOrigin } from "./urlUtils"; export type WebAuthnFlow = { useIframe: boolean; @@ -25,7 +27,13 @@ type Parameters = { * * Logic: * - To calculate the RP IDs, we look for all RP IDs within the devices - * - At the moment, we only use non-iframe if the RP ID matches the current origin. to avoid bad UX, if the RP ID doesn't match the current origin, the iframe will be used. + * - We sort the devices to move the devices registered on the new flow origins to the end + * - The rest of the order we keep as is because it's the order by last used (recently used first) returned by the backend + * We do this because during the upgrade flow a new passkey is created and it will be used to authenticate in 1.0. This was initially considered a feature, not a bug. + * But users don't know where passkeys are stored. + * Therefore, the passkey that they use to authenticate in 1.0 is not in the same place where they upgraded. + * Which triggers a new UX for the user that confuses them because they were used to a different UX. + * - We only use non-iframe if the RP ID matches the current origin. to avoid bad UX, if the RP ID doesn't match the current origin, the iframe will be used. * * @param {Parameters} params - The parameters to find the webauthn steps. * @returns {WebAuthnFlow[]} The ordered steps to try to perform the webauthn authentication. @@ -35,32 +43,58 @@ export const findWebAuthnFlows = ({ currentOrigin, relatedOrigins, }: Parameters): WebAuthnFlow[] => { + // We need the helpers inside so that when `canisterConfig` is accessed, it already exists. + // The devices are expected to be ordered by recently used already + // Move devices registered on the new flow origins to the end using toSorted (preserving relative order within groups) + const newFlowOrigins = canisterConfig.new_flow_origins[0] ?? []; + const isInNewFlow = (credentialData: CredentialData): boolean => { + const origin = credentialData.origin ?? II_LEGACY_ORIGIN; + return newFlowOrigins.some((o) => isSameOrigin(o, origin)); + }; + const sortNewFlowOriginsToEnd = ( + a: CredentialData, + b: CredentialData, + ): number => { + const aIn = isInNewFlow(a); + const bIn = isInNewFlow(b); + // Keep the order if both are in the new flow or both are not + if (aIn === bIn) return 0; + // Move the one that is in the new flow to the end + return aIn ? 1 : -1; + }; + const currentRpId = new URL(currentOrigin).hostname; const relatedRpIds = relatedOrigins.map( (relatedOrigin) => new URL(relatedOrigin).hostname, ); - // The devices are expected to be ordered by recently used already - const orderedDeviceRpIds = [ - ...new Set( - devices - // Device origin to RP ID (hostname) - .map((device) => - device.origin === currentOrigin || - (currentOrigin === II_LEGACY_ORIGIN && isNullish(device.origin)) - ? undefined - : new URL(device.origin ?? II_LEGACY_ORIGIN).hostname, - ) - // Filter out RP IDs that are not within `relatedRpIds` - .filter((rpId) => isNullish(rpId) || relatedRpIds.includes(rpId)), - ), - ]; - - // Create steps from `deviceRpIds`, currently that's one step per RP ID - const steps: WebAuthnFlow[] = orderedDeviceRpIds.map((rpId) => ({ - rpId, - useIframe: nonNullish(rpId) && rpId !== currentRpId, - })); + // Sort devices in place + devices.sort(sortNewFlowOriginsToEnd); + // Create steps from `devices`, currently that's one step per RP ID + const steps: WebAuthnFlow[] = devices + // Device origin to RP ID (hostname) + .map((device: CredentialData) => + device.origin === currentOrigin || + (currentOrigin === II_LEGACY_ORIGIN && isNullish(device.origin)) + ? undefined + : new URL(device.origin ?? II_LEGACY_ORIGIN).hostname, + ) + // Filter out RP IDs that are not within `relatedRpIds` + .filter( + (rpId: string | undefined) => + isNullish(rpId) || relatedRpIds.includes(rpId), + ) + // Remove duplicates + .reduce((rpIds: Array, rpId: string | undefined) => { + if (rpIds.includes(rpId)) { + return rpIds; + } + return [...rpIds, rpId]; + }, []) + .map((rpId) => ({ + rpId, + useIframe: nonNullish(rpId) && rpId !== currentRpId, + })); // If there are no steps, add a default step. if (steps.length === 0) { diff --git a/src/frontend/src/lib/utils/iiConnection.test.ts b/src/frontend/src/lib/utils/iiConnection.test.ts index 9885f38cca..920d97d0ab 100644 --- a/src/frontend/src/lib/utils/iiConnection.test.ts +++ b/src/frontend/src/lib/utils/iiConnection.test.ts @@ -85,6 +85,12 @@ const DEFAULT_INIT: InternetIdentityInit = { feature_flag_enable_generic_open_id_fe: [], }; +vi.mock("$lib/globals", () => ({ + canisterConfig: { + new_flow_origins: [["https://id.ai"]], + }, +})); + const mockActor = { identity_info: vi.fn().mockImplementation(async () => { // The `await` is necessary to make sure that the `getterResponse` is set before the test continues. @@ -232,6 +238,39 @@ describe("Connection.login", () => { } }); + it("login returns authenticated connection with expected rpID if new flow origins are enabled", async () => { + const newOriginDevice = createMockDevice("https://id.ai"); + const mockActor = { + identity_info: vi.fn().mockImplementation(async () => { + // The `await` is necessary to make sure that the `getterResponse` is set before the test continues. + infoResponse = await mockRawMetadata; + return { Ok: { metadata: mockRawMetadata } }; + }), + identity_metadata_replace: vi.fn().mockResolvedValue({ Ok: null }), + // The order here matters, the first device is the one that would be used normally. + // But we changed to push the new_flow_origins to the end. + lookup: vi.fn().mockResolvedValue([newOriginDevice, mockDevice]), + } as unknown as ActorSubclass<_SERVICE>; + const connection = new Connection("aaaaa-aa", DEFAULT_INIT, mockActor); + + const loginResult = await connection.login(BigInt(12345)); + + expect(loginResult.kind).toBe("loginSuccess"); + if (loginResult.kind === "loginSuccess") { + expect(loginResult.connection).toBeInstanceOf(AuthenticatedConnection); + expect(loginResult.showAddCurrentDevice).toBe(false); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledTimes(1); + expect(MultiWebAuthnIdentity.fromCredentials).toHaveBeenCalledWith( + [ + convertToValidCredentialData(mockDevice), + convertToValidCredentialData(newOriginDevice), + ], + "identity.ic0.app", + true, + ); + } + }); + it("login returns undefined RP ID if no related origins are in the config", async () => { const config: InternetIdentityInit = { ...DEFAULT_INIT,