Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace fingerprint calculation with a secure option #1946

Merged
merged 1 commit into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading