release/5.27.2 (#1827) (#1828)

* Feature/memory security improvement 5.27.2 (#1827) * add temporary store extra data * add tests for switching accounts * reset session length * upgrade jest and add unit tests * rm unused selectors * fix sendpayment test * rm npm package and add unit tests * add better error handling and Sentry capture * rm console.log * make sure to login before adding new stellar address * add a test for imported S key payment * Fix/import acct when timed out (#1832) * make sure to login to all accounts before importing by private key if session has timed out * update comment * login before showing mnemonic phrase (#1834) * login before showing mnemonic phrase * add more Sentry error capture
stellar · Feb 21, 2025 · c934bef · c934bef
1 parent 9134812
commit c934bef
Show file tree

Hide file tree

Showing 16 changed files with 2,637 additions and 2,297 deletions.
diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts
@@ -87,16 +87,22 @@ export const createAccount = async (
   let publicKey = "";
   let allAccounts = [] as Array<Account>;
   let hasPrivateKey = false;
+  let error = "";
 
   try {
-    ({ allAccounts, publicKey, hasPrivateKey } = await sendMessageToBackground({
-      password,
-      type: SERVICE_TYPES.CREATE_ACCOUNT,
-    }));
+    ({ allAccounts, publicKey, hasPrivateKey, error } =
+      await sendMessageToBackground({
+        password,
+        type: SERVICE_TYPES.CREATE_ACCOUNT,
+      }));
   } catch (e) {
     console.error(e);
   }
 
+  if (error) {
+    throw new Error(error);
+  }
+
   return { allAccounts, publicKey, hasPrivateKey };
 };
 

diff --git a/config/jest/setupTests.tsx b/config/jest/setupTests.tsx
@@ -3,10 +3,12 @@
 // expect(element).toHaveTextContent(/react/i)
 // learn more: https://github.com/testing-library/jest-dom
 import { JSDOM } from "jsdom";
+import crypto from "crypto";
 import React from "react";
 import fetch from "isomorphic-unfetch";
 import "jest-localstorage-mock";
 import "jsdom-global";
+import { TextEncoder, TextDecoder } from "util";
 
 // make a JSDOM thing so we can fuck with mount
 const jsdom = new JSDOM("<!doctype html><html><body></body></html>");
@@ -19,8 +21,16 @@ global.DEV_EXTENSION = true;
 global.PRODUCTION = false;
 global.EXPERIMENTAL = false;
 global.TextEncoder = TextEncoder;
+// @ts-expect-error
 global.TextDecoder = TextDecoder;
 
+Object.defineProperty(global.self, "crypto", {
+  value: {
+    getRandomValues: crypto.getRandomValues,
+    subtle: crypto.webcrypto.subtle,
+  },
+});
+
 process.env.INDEXER_URL = "http://localhost:3002/api/v1";
 
 jest.mock("helpers/metrics", () => ({

diff --git a/extension/e2e-tests/helpers/sendPayment.ts b/extension/e2e-tests/helpers/sendPayment.ts
@@ -0,0 +1,66 @@
+import { expect, expectPageToHaveScreenshot } from "../test-fixtures";
+
+export const sendXlmPayment = async ({ page }) => {
+  await page.getByTitle("Send Payment").click({ force: true });
+
+  await expect(page.getByText("Send To")).toBeVisible();
+  await expectPageToHaveScreenshot({
+    page,
+    screenshot: "send-payment-to.png",
+  });
+  await page
+    .getByTestId("send-to-input")
+    .fill("GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF");
+  await page.getByText("Continue").click({ force: true });
+
+  await expect(page.getByText("Send XLM")).toBeVisible();
+  await expectPageToHaveScreenshot({
+    page,
+    screenshot: "send-payment-amount.png",
+  });
+  await page.getByTestId("send-amount-amount-input").fill("1");
+  await page.getByText("Continue").click({ force: true });
+
+  await expect(page.getByText("Send Settings")).toBeVisible();
+  await expect(page.getByTestId("SendSettingsTransactionFee")).toHaveText(
+    /[0-9]/,
+  );
+  // 100 XLM is the default, so likely a sign the fee was not set properly from Horizon
+  await expect(
+    page.getByTestId("SendSettingsTransactionFee"),
+  ).not.toContainText("100 XLM");
+  await expectPageToHaveScreenshot(
+    {
+      page,
+      screenshot: "send-payment-settings.png",
+    },
+    {
+      mask: [page.locator("[data-testid='SendSettingsTransactionFee']")],
+    },
+  );
+  await page.getByText("Review Send").click({ force: true });
+
+  await expect(page.getByText("Confirm Send")).toBeVisible();
+  await expect(page.getByText("XDR")).toBeVisible();
+  await expectPageToHaveScreenshot({
+    page,
+    screenshot: "send-payment-confirm.png",
+  });
+  await page.getByTestId("transaction-details-btn-send").click({ force: true });
+
+  await expect(page.getByText("Successfully sent")).toBeVisible({
+    timeout: 60000,
+  });
+  await expectPageToHaveScreenshot({
+    page,
+    screenshot: "send-payment-sent.png",
+  });
+
+  await page.getByText("Details").click({ force: true });
+  await expectPageToHaveScreenshot({
+    page,
+    screenshot: "send-payment-details.png",
+  });
+  await expect(page.getByText("Sent XLM")).toBeVisible();
+  await expect(page.getByTestId("asset-amount")).toContainText("1");
+};
diff --git a/extension/e2e-tests/sendPayment.test.ts b/extension/e2e-tests/sendPayment.test.ts
@@ -6,7 +6,7 @@ import {
   PASSWORD,
 } from "./helpers/login";
 import { TEST_TOKEN_ADDRESS } from "./helpers/test-token";
-import { toBeVisible } from "@testing-library/jest-dom/matchers";
+import { sendXlmPayment } from "./helpers/sendPayment";
 
 test("Swap doesn't throw error when account is unfunded", async ({
   page,
@@ -39,71 +39,64 @@ test("Send doesn't throw error when account is unfunded", async ({
   );
 });
 
-test("Send XLM payment to G address", async ({ page, extensionId }) => {
+test("Send XLM payments from multiple accounts to G Address", async ({
+  page,
+  extensionId,
+}) => {
   test.slow();
   await loginAndFund({ page, extensionId });
-  await page.getByTitle("Send Payment").click({ force: true });
-
-  await expect(page.getByText("Send To")).toBeVisible();
-  await expectPageToHaveScreenshot({
-    page,
-    screenshot: "send-payment-to.png",
+  await sendXlmPayment({ page });
+
+  await page.getByTestId("BackButton").click();
+  await page.getByTestId("BottomNav-link-account").click();
+  await page.getByTestId("AccountHeader__icon-btn").click();
+  await page.getByText("Create a new Stellar address").click();
+
+  // test incorrect password
+  await page.locator("#password-input").fill("wrong password");
+  await page.getByText("Create New Address").click();
+  await expect(page.getByText("Incorrect password")).toBeVisible();
+  await page.locator("#password-input").fill(PASSWORD);
+  await page.getByText("Create New Address").click();
+
+  await expect(page.getByTestId("not-funded")).toBeVisible({
+    timeout: 10000,
   });
-  await page
-    .getByTestId("send-to-input")
-    .fill("GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF");
-  await page.getByText("Continue").click({ force: true });
+  await page.getByRole("button", { name: "Fund with Friendbot" }).click();
 
-  await expect(page.getByText("Send XLM")).toBeVisible();
-  await expectPageToHaveScreenshot({
-    page,
-    screenshot: "send-payment-amount.png",
+  await expect(page.getByTestId("account-assets")).toBeVisible({
+    timeout: 30000,
   });
-  await page.getByTestId("send-amount-amount-input").fill("1");
-  await page.getByText("Continue").click({ force: true });
+  await sendXlmPayment({ page });
 
-  await expect(page.getByText("Send Settings")).toBeVisible();
-  await expect(page.getByTestId("SendSettingsTransactionFee")).toHaveText(
-    /[0-9]/,
-  );
-  // 100 XLM is the default, so likely a sign the fee was not set properly from Horizon
-  await expect(
-    page.getByTestId("SendSettingsTransactionFee"),
-  ).not.toContainText("100 XLM");
-  await expectPageToHaveScreenshot(
-    {
-      page,
-      screenshot: "send-payment-settings.png",
-    },
-    {
-      mask: [page.locator("[data-testid='SendSettingsTransactionFee']")],
-    },
-  );
-  await page.getByText("Review Send").click({ force: true });
+  await page.getByTestId("BackButton").click();
+  await page.getByTestId("BottomNav-link-account").click();
+  await page.getByTestId("AccountHeader__icon-btn").click();
 
-  await expect(page.getByText("Confirm Send")).toBeVisible();
-  await expect(page.getByText("XDR")).toBeVisible();
-  await expectPageToHaveScreenshot({
-    page,
-    screenshot: "send-payment-confirm.png",
-  });
-  await page.getByTestId("transaction-details-btn-send").click({ force: true });
+  await page.getByText("Account 1").click();
+  await sendXlmPayment({ page });
 
-  await expect(page.getByText("Successfully sent")).toBeVisible({
-    timeout: 60000,
-  });
-  await expectPageToHaveScreenshot({
-    page,
-    screenshot: "send-payment-sent.png",
-  });
+  await page.getByTestId("BackButton").click();
+  await page.getByTestId("BottomNav-link-account").click();
+  await page.getByTestId("AccountHeader__icon-btn").click();
+  await page.getByText("Import a Stellar secret key").click();
 
-  await page.getByText("Details").click({ force: true });
-  await expectPageToHaveScreenshot({
-    page,
-    screenshot: "send-payment-details.png",
-  });
-  await expect(page.getByText("Sent XLM")).toBeVisible();
-  await expect(page.getByTestId("asset-amount")).toContainText("1");
+  // test private key account from different mnemonic phrase
+  await page
+    .locator("#privateKey-input")
+    .fill("SDCUXKGHQ4HX5NRX5JN7GMJZUXQBWZXLKF34DLVYZ4KLXXIZTG7Q26JJ");
+  // test incorrect password
+  await page.locator("#password-input").fill("wrongpassword");
+  await page.locator("#authorization-input").click({ force: true });
+
+  await page.getByTestId("import-account-button").click();
+  await expect(
+    page.getByText("Please enter a valid secret key/password combination"),
+  ).toHaveCount(2);
+  await page.locator("#password-input").fill(PASSWORD);
+  await page.getByTestId("import-account-button").click();
+
+  await sendXlmPayment({ page });
 });
 
 test("Send XLM payment to C address", async ({ page, extensionId }) => {

diff --git a/extension/src/background/ducks/session.ts b/extension/src/background/ducks/session.ts
@@ -14,12 +14,11 @@ export const logIn = createAsyncThunk<
   UiData,
   UiData,
   { rejectValue: ErrorMessage }
->("logIn", async ({ publicKey, mnemonicPhrase, allAccounts }, thunkApi) => {
+>("logIn", async ({ publicKey, allAccounts }, thunkApi) => {
   try {
     await internalSubscribeAccount(publicKey);
     return {
       publicKey,
-      mnemonicPhrase,
       allAccounts,
     };
   } catch (e) {
@@ -45,24 +44,32 @@ export const setActivePublicKey = createAsyncThunk<
   }
 });
 
-const initialState = {
+export type InitialState = UiData & AppData;
+
+export interface SessionState {
+  session: InitialState;
+}
+
+const initialState: InitialState = {
   publicKey: "",
-  privateKey: "",
-  mnemonicPhrase: "",
+  hashKey: {
+    iv: "",
+    key: "",
+  },
   allAccounts: [] as Account[],
   migratedMnemonicPhrase: "",
 };
 
 interface UiData {
   publicKey: string;
-  mnemonicPhrase?: string;
   allAccounts?: Account[];
   migratedMnemonicPhrase?: string;
 }
 
 interface AppData {
   privateKey?: string;
-  password: string;
+  hashKey?: { key: string; iv: string };
+  password?: string;
 }
 
 export const sessionSlice = createSlice({
@@ -71,13 +78,17 @@ export const sessionSlice = createSlice({
   reducers: {
     reset: () => initialState,
     logOut: () => initialState,
-    setActivePrivateKey: (state, action: { payload: AppData }) => {
-      const { privateKey = "", password = "" } = action.payload;
+    setActiveHashKey: (state, action: { payload: AppData }) => {
+      const {
+        hashKey = {
+          iv: "",
+          key: "",
+        },
+      } = action.payload;
 
       return {
         ...state,
-        privateKey,
-        password,
+        hashKey,
       };
     },
     setMigratedMnemonicPhrase: (
@@ -93,7 +104,10 @@ export const sessionSlice = createSlice({
     },
     timeoutAccountAccess: (state) => ({
       ...state,
-      privateKey: "",
+      hashKey: {
+        iv: "",
+        key: "",
+      },
       password: "",
     }),
     updateAllAccountsAccountName: (
@@ -102,6 +116,10 @@ export const sessionSlice = createSlice({
     ) => {
       const { updatedAccountName = "" } = action.payload;
 
+      if (!state.allAccounts) {
+        return state;
+      }
+
       const newAllAccounts = state.allAccounts.map((account) => {
         if (state.publicKey === account.publicKey) {
           // this is the current active public key, let's edit it
@@ -123,7 +141,6 @@ export const sessionSlice = createSlice({
   extraReducers: (builder) => {
     builder.addCase(logIn.fulfilled, (state, action) => {
       state.publicKey = action.payload.publicKey;
-      state.mnemonicPhrase = action.payload.mnemonicPhrase || "";
       state.allAccounts = action.payload.allAccounts || [];
     });
     builder.addCase(setActivePublicKey.fulfilled, (state, action) => {
@@ -140,7 +157,7 @@ export const {
   actions: {
     reset,
     logOut,
-    setActivePrivateKey,
+    setActiveHashKey,
     timeoutAccountAccess,
     updateAllAccountsAccountName,
     setMigratedMnemonicPhrase,
@@ -151,10 +168,6 @@ export const publicKeySelector = createSelector(
   sessionSelector,
   (session) => session.publicKey,
 );
-export const mnemonicPhraseSelector = createSelector(
-  sessionSelector,
-  (session) => session.mnemonicPhrase,
-);
 export const migratedMnemonicPhraseSelector = createSelector(
   sessionSelector,
   (session) => session.migratedMnemonicPhrase,
@@ -167,14 +180,10 @@ export const hasPrivateKeySelector = createSelector(
   sessionSelector,
   async (session) => {
     const isHardwareWalletActive = await getIsHardwareWalletActive();
-    return isHardwareWalletActive || !!session?.privateKey?.length;
+    return isHardwareWalletActive || !!session?.hashKey?.key;
   },
 );
-export const privateKeySelector = createSelector(
-  sessionSelector,
-  (session) => session.privateKey || "",
-);
-export const passwordSelector = createSelector(
+export const hashKeySelector = createSelector(
   sessionSelector,
-  (session) => session.password,
+  (session) => session.hashKey,
 );