Skip to content

Commit

Permalink
Replace fingerprint calculation with a secure option
Browse files Browse the repository at this point in the history
  • Loading branch information
serjonya-trili committed Oct 2, 2024
1 parent 52c7687 commit 4da3d07
Show file tree
Hide file tree
Showing 15 changed files with 265 additions and 85 deletions.
5 changes: 2 additions & 3 deletions apps/desktop-e2e/src/helpers/AccountGroup.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { InMemorySigner } from "@taquito/signer";
import { type RawPkh, derivePublicKeyPair, getFingerPrint, makeDerivationPath } from "@umami/tezos";
import { type RawPkh, derivePublicKeyPair, makeDerivationPath } from "@umami/tezos";
import lodash from "lodash";

export type AccountGroup = {
Expand Down Expand Up @@ -46,12 +46,11 @@ export class AccountGroupBuilder {
this.derivationPathTemplate = derivationPathTemplate;
}

async setSeedPhrase(seedPhrase: string[]): Promise<void> {
setSeedPhrase(seedPhrase: string[]): void {
if (this.accountGroup.type !== "mnemonic") {
throw new Error(`Seed phrase is not used for ${this.accountGroup.type} accounts}`);
}
this.seedPhrase = seedPhrase;
this.accountGroup.label = `Seedphrase ${await getFingerPrint(seedPhrase.join(" "))}`;
}

getSeedPhrase = () => this.seedPhrase;
Expand Down
14 changes: 7 additions & 7 deletions apps/desktop-e2e/src/helpers/backedupAccountGroups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ import { type AccountGroup, AccountGroupBuilder } from "./AccountGroup";

export const v1BackedupAccountGroups = async () => {
const expectedGroups: AccountGroup[] = [];
// Seedphrase 5fd091e1

let accountGroupBuilder = new AccountGroupBuilder("mnemonic", 1);
accountGroupBuilder.setAllAccountNames("Account");
await accountGroupBuilder.setSeedPhrase(mnemonic1.split(" "));
accountGroupBuilder.setSeedPhrase(mnemonic1.split(" "));
accountGroupBuilder.setDerivationPathTemplate(DEFAULT_DERIVATION_PATH_TEMPLATE.value);
expectedGroups.push(await accountGroupBuilder.build());
// Seedphrase 1b406abf

const mnemonic =
"top skirt fan helmet ankle pave original ivory push song bridge broom hawk food parade nation involve sunny rely security ladder beach gown imitate";
accountGroupBuilder = new AccountGroupBuilder("mnemonic", 1);
accountGroupBuilder.setAllAccountNames("Account");
await accountGroupBuilder.setSeedPhrase(mnemonic.split(" "));
accountGroupBuilder.setSeedPhrase(mnemonic.split(" "));
accountGroupBuilder.setDerivationPathTemplate(DEFAULT_DERIVATION_PATH_TEMPLATE.value);
expectedGroups.push(await accountGroupBuilder.build());
// TODO: add related multisig accounts.
Expand All @@ -32,23 +32,23 @@ export const v2BackedupAccountGroups = async () => {
accountGroupBuilder.setAccountName("Restored account 2", 2);
accountGroupBuilder.setAccountName("htrthrh", 3);
accountGroupBuilder.setAccountName("12asd", 4);
await accountGroupBuilder.setSeedPhrase(mnemonic1.split(" "));
accountGroupBuilder.setSeedPhrase(mnemonic1.split(" "));
accountGroupBuilder.setDerivationPathTemplate(DEFAULT_DERIVATION_PATH_TEMPLATE.value);
expectedGroups.push(await accountGroupBuilder.build());
// Seedphrase fa3f3982
const mnemonic2 =
"cluster umbrella blade second miss margin jazz joke blur column bulk monkey wrestle spell day produce noble front alley kangaroo auction sight truck other";
accountGroupBuilder = new AccountGroupBuilder("mnemonic", 1);
accountGroupBuilder.setAccountName("test");
await accountGroupBuilder.setSeedPhrase(mnemonic2.split(" "));
accountGroupBuilder.setSeedPhrase(mnemonic2.split(" "));
accountGroupBuilder.setDerivationPathTemplate(DEFAULT_DERIVATION_PATH_TEMPLATE.value);
expectedGroups.push(await accountGroupBuilder.build());
// Seedphrase 2263e19b
const mnemonic3 =
"few gauge word visa april now grace allow ozone box loop pudding clap barely loud casino ugly demise bottom urge toast fan wedding exclude";
accountGroupBuilder = new AccountGroupBuilder("mnemonic", 1);
accountGroupBuilder.setAccountName("test2");
await accountGroupBuilder.setSeedPhrase(mnemonic3.split(" "));
accountGroupBuilder.setSeedPhrase(mnemonic3.split(" "));
accountGroupBuilder.setDerivationPathTemplate(DEFAULT_DERIVATION_PATH_TEMPLATE.value);
expectedGroups.push(await accountGroupBuilder.build());
// TODO: add ledger account
Expand Down
60 changes: 38 additions & 22 deletions apps/desktop-e2e/src/pages/AccountsPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,54 @@ export class AccountsPage {
};
}

async getGroup(groupTitle: string): Promise<AccountGroup> {
const group = this.page.getByTestId(`account-group-${groupTitle}`);

const label = await group.getByTestId("account-group-title").innerText();

const accountTiles = await group.getByTestId("account-identifier").all();
const accounts: Account[] = [];
for (const accountTile of accountTiles) {
const name = await accountTile.getByRole("heading", { level: 2 }).innerText();
const pkh = await accountTile.getByTestId("short-address").innerText();

// pkh here will be a shortened version of the real pkh
accounts.push({ name, pkh });
async getAllGroups(): Promise<AccountGroup[]> {
const groupElements = await this.page.getByTestId(/account-group-.*/).all();
const groups = [];
for (const groupElement of groupElements) {
groups.push(await buildGroup(groupElement));
}
return groups;
}

const groupType = /Secret Key/.test(label) ? "secret_key" : "mnemonic";

return {
type: groupType,
label,
accounts,
};
async getGroup(groupTitle: string): Promise<AccountGroup> {
const group = this.page.getByTestId(`account-group-${groupTitle}`);
return buildGroup(group);
}

async checkAccountGroup(expectedGroup: AccountGroup): Promise<void> {
const group = await this.getGroup(expectedGroup.label);
const allGroups = await this.getAllGroups();

const group = allGroups.find(group =>
group.accounts.some(acc => acc.pkh === formatPkh(expectedGroup.accounts[0].pkh))
)!;

expect(group.label).toEqual(expectedGroup.label);
expect(group).not.toBeUndefined();
expect(group.accounts.length).toEqual(expectedGroup.accounts.length);
for (let i = 0; i < group.accounts.length; i++) {
expect(group.accounts[i].name).toEqual(expectedGroup.accounts[i].name);
expect(group.accounts[i].pkh).toEqual(formatPkh(expectedGroup.accounts[i].pkh));
}
}
}

const buildGroup = async (groupElement: Locator): Promise<AccountGroup> => {
const label = await groupElement.getByTestId("group-title").innerText();

const accountTiles = await groupElement.getByTestId("account-identifier").all();
const accounts: Account[] = [];
for (const accountTile of accountTiles) {
const name = await accountTile.getByRole("heading", { level: 2 }).innerText();
const pkh = await accountTile.getByTestId("short-address").innerText();

// pkh here will be a shortened version of the real pkh
accounts.push({ name, pkh });
}

const groupType = /Secret Key/.test(label) ? "secret_key" : "mnemonic";

return {
type: groupType,
label,
accounts,
};
};
4 changes: 2 additions & 2 deletions apps/desktop-e2e/src/steps/onboarding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Then("I record generated seedphrase", async function (this: CustomWorld) {
for (let i = 0; i < 24; i++) {
words.push(await this.page.getByTestId(`mnemonic-word-${i}`).innerText());
}
await accountGroupBuilder.setSeedPhrase(words);
accountGroupBuilder.setSeedPhrase(words);
});

When("I enter recorded seedphrase", async function (this: CustomWorld) {
Expand All @@ -46,7 +46,7 @@ When("I enter recorded seedphrase", async function (this: CustomWorld) {
});

When("I enter existing seedphrase", async function (this: CustomWorld) {
await accountGroupBuilder.setSeedPhrase(existingSeedphrase.split(" "));
accountGroupBuilder.setSeedPhrase(existingSeedphrase.split(" "));
for (let i = 0; i < 24; i++) {
await this.page.getByRole("textbox").nth(i).fill(accountGroupBuilder.getSeedPhrase()[i]);
}
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/views/home/AccountGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const AccountGroup = ({
return (
<Box data-testid={`account-group-${groupLabel}`}>
<Center justifyContent="space-between" marginTop="24px" marginBottom="16px">
<Heading data-testid="account-group-title" size="md">
<Heading data-testid="group-title" size="md">
{groupLabel}
</Heading>

Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/views/home/AccountsList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe("<AccountsList />", () => {
restore();
render(<AccountsList />, { store });
expect(screen.getAllByTestId("account-tile-container")).toHaveLength(7);
expect(screen.getAllByTestId(/account-group-title/)).toHaveLength(4);
expect(screen.getAllByTestId(/group-title/)).toHaveLength(4);

const socialAccounts = screen.getByTestId(/account-group-social/i);
expect(within(socialAccounts).getAllByTestId("account-tile-container")).toHaveLength(2);
Expand Down
6 changes: 3 additions & 3 deletions packages/state/src/hooks/mnemonic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { mnemonic1 } from "@umami/test-utils";
import {
MAINNET,
defaultDerivationPathTemplate,
generateHash,
getDefaultDerivationPath,
getFingerPrint,
isAccountRevealed,
} from "@umami/tezos";

Expand All @@ -21,7 +21,7 @@ import { addTestAccount, fakeIsAccountRevealed, renderHook } from "../testUtils"
jest.mock("@umami/tezos", () => ({
...jest.requireActual("@umami/tezos"),
isAccountRevealed: jest.fn(),
getFingerPrint: jest.fn(),
generateHash: jest.fn(),
}));

let store: UmamiStore;
Expand Down Expand Up @@ -75,7 +75,7 @@ const testPublicKeys = {
};

beforeEach(() => {
jest.mocked(getFingerPrint).mockResolvedValue("mockFingerPrint");
jest.mocked(generateHash).mockResolvedValue("mockFingerPrint");
});

describe.each(["ed25519", "secp256k1", "p256"] as const)("with %s curve", curve => {
Expand Down
4 changes: 2 additions & 2 deletions packages/state/src/hooks/mnemonic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
type Network,
type PublicKeyPair,
derivePublicKeyPair,
getFingerPrint,
generateHash,
isAccountRevealed,
makeDerivationPath,
} from "@umami/tezos";
Expand Down Expand Up @@ -89,7 +89,7 @@ export const useRestoreRevealedMnemonicAccounts = () => {
curve,
network
);
const seedFingerPrint = await getFingerPrint(mnemonic);
const seedFingerPrint = await generateHash();
const accountLabels = getNextAvailableAccountLabels(label, pubKeyPairs.length);

return pubKeyPairs.map(({ pk, pkh }, accountIndex) => ({
Expand Down
10 changes: 5 additions & 5 deletions packages/state/src/hooks/setAccountData.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { mnemonic1 } from "@umami/test-utils";
import {
AVAILABLE_DERIVATION_PATH_TEMPLATES,
derivePublicKeyPair,
getFingerPrint,
generateHash,
isAccountRevealed,
makeDerivationPath,
} from "@umami/tezos";
Expand Down Expand Up @@ -47,7 +47,7 @@ jest.mock("@umami/crypto", () => ({
}));
jest.mock("@umami/tezos", () => ({
...jest.requireActual("@umami/tezos"),
getFingerPrint: jest.fn(),
generateHash: jest.fn(),
isAccountRevealed: jest.fn(),
}));

Expand Down Expand Up @@ -94,7 +94,7 @@ describe.each(["ed25519", "secp256k1", "p256"] as const)(

describe("useRestoreFromMnemonic", () => {
beforeEach(() => {
jest.mocked(getFingerPrint).mockResolvedValue(MOCK_FINGERPRINT);
jest.mocked(generateHash).mockResolvedValue(MOCK_FINGERPRINT);
jest.mocked(encrypt).mockReturnValue(Promise.resolve(MOCK_ENCRYPTED));
});

Expand All @@ -118,7 +118,7 @@ describe.each(["ed25519", "secp256k1", "p256"] as const)(
expect(store.getState().accounts.seedPhrases).toEqual({
[MOCK_FINGERPRINT]: MOCK_ENCRYPTED,
});
expect(getFingerPrint).toHaveBeenCalledWith(mnemonic1);
expect(generateHash).toHaveBeenCalledTimes(1);
// Encrypts given mnemonic with the given password.
expect(encrypt).toHaveBeenCalledWith(mnemonic1, PASSWORD);
});
Expand Down Expand Up @@ -159,7 +159,7 @@ describe.each(["ed25519", "secp256k1", "p256"] as const)(
expect(store.getState().accounts.seedPhrases).toEqual({
[MOCK_FINGERPRINT]: MOCK_ENCRYPTED,
});
expect(getFingerPrint).toHaveBeenCalledWith(mnemonic1);
expect(generateHash).toHaveBeenCalledTimes(1);
// Encrypts given mnemonic with the given password.
expect(encrypt).toHaveBeenCalledWith(mnemonic1, PASSWORD);
});
Expand Down
7 changes: 3 additions & 4 deletions packages/state/src/hooks/setAccountData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "@umami/core";
import { decrypt, encrypt } from "@umami/crypto";
import { type IDP } from "@umami/social-auth";
import { derivePublicKeyPair, getFingerPrint, makeDerivationPath } from "@umami/tezos";
import { derivePublicKeyPair, makeDerivationPath } from "@umami/tezos";
import { useCallback } from "react";
import { useDispatch } from "react-redux";

Expand Down Expand Up @@ -62,7 +62,6 @@ export const useRestoreFromMnemonic = () => {
curve: Curves;
isVerified?: boolean;
}) => {
const seedFingerprint = await getFingerPrint(mnemonic);
const accounts = await restoreRevealedMnemonicAccounts(
mnemonic,
network,
Expand All @@ -75,7 +74,7 @@ export const useRestoreFromMnemonic = () => {

dispatch(
accountsActions.addMnemonicAccounts({
seedFingerprint,
seedFingerprint: accounts[0].seedFingerPrint,
accounts,
encryptedMnemonic,
})
Expand All @@ -91,7 +90,7 @@ export const useRestoreFromMnemonic = () => {
*
* New account is added to the {@link accountsSlice}.
*
* @param fingerPrint - Hash of the mnemonic. Generated with {@link getFingerPrint}. We use it to group together accounts derived from the same mnemonic
* @param fingerPrint - Hash of the mnemonic. Generated with {@link generateHash}. We use it to group together accounts derived from the same mnemonic
* @param password - User's password, used for decrypting the mnemonic.
* @param label - Account name prefix, used to create a unique account name.
*/
Expand Down
18 changes: 9 additions & 9 deletions packages/state/src/thunks/changeMnemonicPassword.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { type MnemonicAccount, mockImplicitAccount } from "@umami/core";
import { type EncryptedData, decrypt, encrypt } from "@umami/crypto";
import { mnemonic1, mnemonic2 } from "@umami/test-utils";
import { getFingerPrint } from "@umami/tezos";
import { generateHash } from "@umami/tezos";

import { changeMnemonicPassword } from "./changeMnemonicPassword";
import { accountsActions } from "../slices/accounts/accounts";
Expand All @@ -16,8 +16,15 @@ beforeEach(() => {
store = makeStore();
});

let fingerPrint1: string;
let fingerPrint2: string;

beforeAll(async () => {
fingerPrint1 = await generateHash();
fingerPrint2 = await generateHash();
});

beforeEach(async () => {
const fingerPrint1 = await getFingerPrint(mnemonic1);
store.dispatch(
accountsActions.addMnemonicAccounts({
seedFingerprint: fingerPrint1,
Expand All @@ -29,7 +36,6 @@ beforeEach(async () => {
})
);

const fingerPrint2 = await getFingerPrint(mnemonic2);
store.dispatch(
accountsActions.addMnemonicAccounts({
seedFingerprint: fingerPrint2,
Expand All @@ -43,9 +49,6 @@ beforeEach(async () => {

describe("changeMnemonicPassword", () => {
it("should update password", async () => {
const fingerPrint1 = await getFingerPrint(mnemonic1);
const fingerPrint2 = await getFingerPrint(mnemonic2);

const action = await store.dispatch<any>(
changeMnemonicPassword({ currentPassword, newPassword })
);
Expand All @@ -64,9 +67,6 @@ describe("changeMnemonicPassword", () => {
});

it("should throw with old password", async () => {
const fingerPrint1 = await getFingerPrint(mnemonic1);
const fingerPrint2 = await getFingerPrint(mnemonic2);

const action: {
type: string;
payload: { newEncryptedMnemonics: Record<string, EncryptedData | undefined> };
Expand Down
1 change: 1 addition & 0 deletions packages/tezos/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"eslint": "^8.57.0",
"jest": "^29.7.0",
"madge": "^8.0.0",
"mockdate": "^3.0.5",
"prettier": "^3.3.2",
"rimraf": "^6.0.1",
"tsup": "^8.3.0",
Expand Down
Loading

0 comments on commit 4da3d07

Please sign in to comment.