fix(auth): never sign as a conflicting source when the pin is missing

extension/src/background/helpers/__tests__/signing-identity.test.ts

@@ -14,6 +14,7 @@ import { sessionSlice } from "background/ducks/session";
 import {
   KEY_ID,
   KEY_ID_LIST,
+  LINK_STATUS_ID,
   PINNED_SOURCE_ID,
 } from "constants/localStorageTypes";
 
@@ -262,6 +263,67 @@ describe("getSigningIdentity", () => {
     expect((await getSigningIdentity(stores))?.sourceId).toBe(phraseAId);
   });
 
+  describe("when the pinned source is not held", () => {
+    // A phrase and an imported key, neither of them the pinned source.
+    const unlockWithoutPin = async (status: Record<string, string>) => {
+      const stores = await unlock({
+        keystores: { "key-imported": importedSecret },
+        mnemonics: { [phraseAId]: phraseA.mnemonic },
+        order: [],
+      });
+      await setPinnedSourceId({
+        localStore: stores.localStore,
+        sourceId: "f".repeat(64),
+      });
+      await stores.localStore.setItem(LINK_STATUS_ID, status);
+      return stores;
+    };
+
+    it("signs with a linked source over a phrase another user owns", async () => {
+      const stores = await unlockWithoutPin({
+        [phraseAId]: "conflict",
+        [importedId]: "linked",
+      });
+
+      expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId);
+      expect(await getPinnedSourceId(stores)).toBe("f".repeat(64));
+    });
+
+    it("signs with an unconfirmed source over one another user owns", async () => {
+      const stores = await unlockWithoutPin({ [phraseAId]: "conflict" });
+
+      expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId);
+    });
+
+    it("prefers a linked source over an unconfirmed phrase", async () => {
+      const stores = await unlockWithoutPin({ [importedId]: "linked" });
+
+      expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId);
+    });
+
+    it("returns null when every held source belongs to another user", async () => {
+      const stores = await unlockWithoutPin({
+        [phraseAId]: "conflict",
+        [importedId]: "conflict",
+      });
+
+      expect(await getSigningIdentity(stores)).toBeNull();
+    });
+  });
+
+  it("signs with the held pinned source whatever the link status says", async () => {
+    const stores = await unlock({
+      keystores: { "key-imported": importedSecret },
+      mnemonics: { [phraseAId]: phraseA.mnemonic },
+      order: [phrase, key],
+    });
+    await stores.localStore.setItem(LINK_STATUS_ID, {
+      [importedId]: "linked",
+    });
+
+    expect((await getSigningIdentity(stores))?.sourceId).toBe(phraseAId);
+  });
+
   it("signs as the earliest registered source, without pinning it, when the wallet has no pin", async () => {
     // Held sources list phrases first, so the key is not the first held one.
     const stores = await unlock({

extension/src/background/helpers/signing-identity.ts

@@ -8,6 +8,7 @@ import {
   getFirstSourceId,
   getPinnedSourceId,
 } from "background/helpers/secret-sources";
+import { getLinkStatus } from "background/helpers/link-status";
 import { getWalletSources } from "background/helpers/wallet-sources";
 
 /*
@@ -42,10 +43,14 @@ export interface SigningIdentity {
  * - The pinned source is held: sign with it. With no pin, as after a failed
  *   pin write, the earliest registered source stands in for it (the source
  *   the next registration or unlock will pin).
- * - That source is not held, as after a restore that recovered only some
- *   sources: sign with the first held source (phrases first) and leave the
- *   pin alone. Nothing re-pins from the server yet, so the pin stays put,
- *   and a source that failed to derive this unlock cannot move it either.
+ * - That source is not held, as when it failed to derive this unlock: sign
+ *   with a held source the server listed in this wallet's cluster
+ *   ("linked"), else one it has not answered for yet, phrases first in each
+ *   case. Never sign with a source the server reported as another user's
+ *   ("conflict"): that would speak as that user. With only those held,
+ *   return null. The pin stays put, so a source that failed to derive this
+ *   unlock cannot move it; moving it waits for account removal, the first
+ *   way a pinned source can leave the wallet for good.
  * - Nothing pinned and nothing registered, as when a request overlaps a
  *   wallet wipe: sign with the first held phrase, or return null when no
  *   phrase is held. Nothing is persisted, so a wrong guess lasts one request.
@@ -85,8 +90,13 @@ export const getSigningIdentity = async ({
     }
 
     captureException("getSigningIdentity: the pinned source is not held");
-    const [{ id: firstHeldId }] = sources;
-    return { sourceId: firstHeldId, keypair: signers.get(firstHeldId)! };
+    const status = await getLinkStatus({ localStore });
+    const fallback =
+      sources.find(({ id }) => status[id] === "linked") ??
+      sources.find(({ id }) => status[id] === undefined);
+    return fallback
+      ? { sourceId: fallback.id, keypair: signers.get(fallback.id)! }
+      : null;
   } catch (e) {
     // A locked or empty wallet is handled above, so anything reaching here
     // is unexpected (a WebCrypto/PBKDF2 failure, a storage read error).