Push id.ai credentials at the end for 1.0 RP ID calculation (#3345)

lmuntaner · aterga · web-flow · commit 965ef0468320 · 2025-09-18T10:53:49.000Z
* Push id.ai credentials at the end for 1.0 RP ID calculation

* Fix lint

* Add test to iiConnection

* Refactor and improve comments

* Move helper inside function

* Fix test

* slightly generalize unit tests

* prettier

---------

Co-authored-by: Arshavir Ter-Gabrielyan &lt;arshavir.ter.gabrielyan@dfinity.org&gt;
diff --git 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
@@ -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<string | undefined>, 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
@@ -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,
