diff --git a/package.json b/package.json index 9aaf0b27cf..7c6c6b6cba 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "generate:types": "didc bind ./src/internet_identity/internet_identity.did -t ts > src/frontend/generated/internet_identity_types.d.ts", "generate:js": "didc bind ./src/internet_identity/internet_identity.did -t js > src/frontend/generated/internet_identity_idl.js", "build": "NODE_ENV=production webpack", - "test": "jest --roots ./src/frontend --verbose --testPathIgnorePatterns=\"src/frontend/src/test-e2e\"", + "test": "jest --maxWorkers 1 --roots ./src/frontend --verbose --testPathIgnorePatterns=\"src/frontend/src/test-e2e\"", "test:e2e": "./scripts/with-selenium-standalone jest --roots ./src/frontend --verbose --testPathPattern=\"src/frontend/src/test-e2e\" --detectOpenHandles", "test:e2e-desktop": "SCREEN=desktop npm run test:e2e", "test:e2e-mobile": "SCREEN=mobile npm run test:e2e", diff --git a/screenshots/05-single-device-warning-mobile.png b/screenshots/05-single-device-warning-mobile.png index ed3df90ecb..491b65be83 100644 Binary files a/screenshots/05-single-device-warning-mobile.png and b/screenshots/05-single-device-warning-mobile.png differ diff --git a/screenshots/09-new-device-instructions-desktop.png b/screenshots/09-new-device-instructions-desktop.png index 0816b9f20d..71f2da322e 100644 Binary files a/screenshots/09-new-device-instructions-desktop.png and b/screenshots/09-new-device-instructions-desktop.png differ diff --git a/screenshots/11-new-device-alias-desktop.png b/screenshots/11-new-device-alias-desktop.png index 391f8da6d4..6f61cbc11f 100644 Binary files a/screenshots/11-new-device-alias-desktop.png and b/screenshots/11-new-device-alias-desktop.png differ diff --git a/screenshots/11-new-device-alias-mobile.png b/screenshots/11-new-device-alias-mobile.png index 7c4f382a49..4083fe8909 100644 Binary files a/screenshots/11-new-device-alias-mobile.png and b/screenshots/11-new-device-alias-mobile.png differ diff --git a/src/frontend/src/crypto/mnemonic.test.ts b/src/frontend/src/crypto/mnemonic.test.ts new file mode 100644 index 0000000000..c76d827503 --- /dev/null +++ b/src/frontend/src/crypto/mnemonic.test.ts @@ -0,0 +1,107 @@ +import { + Warning, + parseRecoveryPhrase, + getWarnings, + dropLeadingUserNumber, +} from "./mnemonic"; + +const expectLeadingUserNumberToBe = ( + received: string, + expected: number | null +) => { + const parsed = parseRecoveryPhrase(received); + const lun = parsed.leadingUserNumber + ? Number(parsed.leadingUserNumber) + : null; + expect(lun).toBe(expected); +}; + +test("Leading user number is parsed correctly", () => { + expectLeadingUserNumberToBe("10043", 10043); + expectLeadingUserNumberToBe(" 10043 ", 10043); + expectLeadingUserNumberToBe(" 10043 foo bar", 10043); + expectLeadingUserNumberToBe(" 10043 foo bar", 10043); + expectLeadingUserNumberToBe("10043n foo bar", null); + expectLeadingUserNumberToBe("foo 123 bar", null); + expectLeadingUserNumberToBe("", null); +}); + +test("Leading user number is dropped correctly", () => { + expect(dropLeadingUserNumber("10043")).toEqual(""); + expect(dropLeadingUserNumber("10043 ")).toEqual(""); + expect(dropLeadingUserNumber("10043 foo bar")).toEqual("foo bar"); + expect(dropLeadingUserNumber("10043 foo bar")).toEqual("foo bar"); + expect(dropLeadingUserNumber("foo bar")).toEqual("foo bar"); +}); + +const expectWordsToEqual = (received: string, expected: string[]) => { + expect(parseRecoveryPhrase(received).words).toEqual(expected); +}; + +test("Word list is parsed correctly", () => { + expectWordsToEqual("about abandon", ["about", "abandon"]); + expectWordsToEqual(" about abandon ", ["about", "abandon"]); + expectWordsToEqual("", []); + expectWordsToEqual("about about about abandon 123", [ + "about", + "about", + "about", + "abandon", + "123", + ]); +}); + +const expectWarningsInclude = ( + userNumber: number, + input: string, + warning: Warning +) => { + expect(getWarnings(BigInt(userNumber), input)).toEqual( + expect.arrayContaining([warning]) + ); +}; + +const expectNoWarnings = (userNumber: number, input: string) => { + expect(getWarnings(BigInt(userNumber), input)).toEqual([]); +}; + +test("Warnings are correctly generated", () => { + expectWarningsInclude(10001, "squirrel &", { + type: "bad_chars", + chars: ["&"], + }); + + expectWarningsInclude(10001, "about squirrel bar abandon", { + type: "repeated_whitespace", + between: ["squirrel", "bar"], + }); + + expectWarningsInclude(10001, "10042 about about", { + type: "bad_anchor", + anchor: BigInt(10042), + }); + + expectWarningsInclude(10001, Array(23).fill("about").join(" "), { + type: "bad_word_count", + count: 23, + }); + + expectWarningsInclude(10001, `10042 ${Array(23).fill("about").join(" ")}`, { + type: "bad_word_count", + count: 23, + }); + + expectWarningsInclude(10001, "about abut squirrel squirel abandon squirel", { + type: "bad_words", + words: ["abut", "squirel"], + }); + + expectWarningsInclude(10001, Array(24).fill("about").join(" "), { + type: "invalid", + }); + + expectNoWarnings( + 10001, + "10001 trust dad oak bright arrow pipe omit provide material donkey hole worry parade test paper fix clutch state range census dust fan hurry almost" + ); +}); diff --git a/src/frontend/src/crypto/mnemonic.ts b/src/frontend/src/crypto/mnemonic.ts index d86f7affc6..0342b32fbc 100644 --- a/src/frontend/src/crypto/mnemonic.ts +++ b/src/frontend/src/crypto/mnemonic.ts @@ -1,18 +1,183 @@ import { entropyToMnemonic, wordlists, validateMnemonic } from "bip39"; import { toHexString } from "@dfinity/identity/lib/cjs/buffer"; +import { isUserNumber } from "../utils/userNumber"; /** - * @returns A random BIP39 mnemonic with 256 bits of entropy. + * @returns A random BIP39 mnemonic with 256 bits of entropy (generates a list of 24 words) */ export function generate(): string { - const entropy = new Uint32Array(8); + const entropy = new Uint32Array(8); // NOTE: please change RECOVERYPHRASE_WORDCOUNT if this changes crypto.getRandomValues(entropy); - return entropyToMnemonic(toHexString(entropy.buffer), wordlists.english); + return entropyToMnemonic( + toHexString(entropy.buffer), + RECOVERYPHRASE_WORDLIST + ); } +/** How many words are expected in the recovery phrase */ +export const RECOVERYPHRASE_WORDCOUNT = 24; + +/** The list of words used to make the recovery phrase */ +// NOTE: "english" is the only one actually bundled in (see webpack config) +export const RECOVERYPHRASE_WORDLIST: string[] = wordlists.english; + /** * @returns true if the mnemonic is valid, false otherwise. */ export function validate(mnemonic: string): boolean { return validateMnemonic(mnemonic); } + +/** A recovery phrase that was parsed. + * An object of this type should not be used to reconstruct a recovery phrase, since + * some information (though most likely incorrectly inserted) like extra whitespaces + * is lost. + */ +type ParsedRecoveryPhrase = { + leadingUserNumber: string | null; + words: string[]; +}; + +/** Parse recovery phrase into a structured object */ +export const parseRecoveryPhrase = (s: string): ParsedRecoveryPhrase => { + /* Split nicely into words + * ' 2002 foo bar ' -> split(" ") -> [ '', '2002', 'foo', '', 'bar', '' ] + * [ '', '2002', 'foo', '', 'bar', '' ] -> filter(...) -> [ '2002', 'foo', 'bar' ] + */ + const words = s.split(" ").filter((w) => w !== ""); + + // check if there's a leading user (anchor) number + const leadingUserNumber: string | null = + words.length >= 1 && isUserNumber(words[0]) ? words[0] : null; + + // If the first element is indeed a number (BigInt), drop it from the actual mnemonic + if (leadingUserNumber !== null) { + words.shift(); + } + + return { leadingUserNumber, words }; +}; + +/** Drop the anchor number from a string + * Example: " 10005 foo bar" -> "foo bar" + */ +export const dropLeadingUserNumber = (input: string): string => { + const parsed = parseRecoveryPhrase(input); + return parsed.leadingUserNumber !== null + ? input.replace(parsed.leadingUserNumber, "").trim() + : input; +}; + +/** The warning types that may be generated. For more information on each, see 'recoveryPhraseWarnings'. + * + * We generate the messages independently mostly so that we can programatically test the warnings generated + * (and avoid brittle regex-based testing).*/ +export type Warning = + | { type: "bad_chars"; chars: string[] } + | { type: "repeated_whitespace"; between: [string, string] | null } + | { type: "bad_anchor"; anchor: bigint } + | { type: "bad_word_count"; count: number } + | { type: "bad_words"; words: string[] } + | { type: "invalid" }; + +/** Generates warnings for a given recovery phrase and user number */ +export const recoveryPhraseWarnings: { + (userNumber: bigint, inputRaw: string): Warning | null; +}[] = [ + // The input contains some characters that are not expected in a recovery phrase + (userNumber: bigint, inputRaw: string): Warning | null => { + // Check for anything that's not a letter, a number or a whitespace (' ', not \n) + const badChars = inputRaw.match(/[^a-z0-9\s]|[\n\t\r]/g); + + if (badChars) { + return { type: "bad_chars", chars: [...new Set(badChars)] }; + } + + return null; + }, + + // Check for double (or triple) whitespaces between words + (userNumber: bigint, inputRaw: string): Warning | null => { + const repeatedSpace = inputRaw.match(/(\S+)\s\s+(\S+)/); + if (repeatedSpace) { + let between: [string, string] | null = null; + if (repeatedSpace.length === 3) { + between = [repeatedSpace[1], repeatedSpace[2]]; + } + + return { type: "repeated_whitespace", between }; + } + return null; + }, + + // Check for an incorrect anchor number (e.g. "10005 foo bar" when recovery anchor 10004) + (userNumber: bigint, inputRaw: string): Warning | null => { + const input = parseRecoveryPhrase(inputRaw); + const leadingUserNumber = + (input.leadingUserNumber !== null && BigInt(input.leadingUserNumber)) || + null; + + if (leadingUserNumber !== null && leadingUserNumber !== userNumber) { + return { type: "bad_anchor", anchor: leadingUserNumber }; + } + + return null; + }, + + // We expect a specific word count + (userNumber: bigint, inputRaw: string): Warning | null => { + const input = parseRecoveryPhrase(inputRaw); + + if ( + input.words.length >= 1 && + input.words.length !== RECOVERYPHRASE_WORDCOUNT + ) { + return { type: "bad_word_count", count: input.words.length }; + } + + return null; + }, + + // As for characters, check for any words that we do not expect + (userNumber: bigint, inputRaw: string): Warning | null => { + const input = parseRecoveryPhrase(inputRaw); + + const badWords = []; + for (const word of input.words) { + if (!RECOVERYPHRASE_WORDLIST.includes(word)) { + badWords.push(word); + } + } + + if (badWords.length >= 1) { + return { type: "bad_words", words: [...new Set(badWords)] }; + } + + return null; + }, + + // We expect a specific word count + (userNumber: bigint, inputRaw: string): Warning | null => { + const input = parseRecoveryPhrase(inputRaw); + + if ( + input.words.length === RECOVERYPHRASE_WORDCOUNT && + !validate(dropLeadingUserNumber(inputRaw)) + ) { + return { type: "invalid" }; + } + + return null; + }, +]; + +export const getWarnings = (userNumber: bigint, input: string): Warning[] => { + const warnings = []; + for (const mkWarning of recoveryPhraseWarnings) { + const warning = mkWarning(userNumber, input); + if (warning) { + warnings.push(warning); + } + } + return warnings; +}; diff --git a/src/frontend/src/flows/recovery/inputSeedPhrase.ts b/src/frontend/src/flows/recovery/inputSeedPhrase.ts index 11fa135ad4..116d01c84b 100644 --- a/src/frontend/src/flows/recovery/inputSeedPhrase.ts +++ b/src/frontend/src/flows/recovery/inputSeedPhrase.ts @@ -1,35 +1,93 @@ -import { html, render } from "lit-html"; -import { displayError } from "../../components/displayError"; -import { validate } from "../../crypto/mnemonic"; -import { parseUserNumber } from "../../utils/userNumber"; +import { html, nothing, render, TemplateResult } from "lit-html"; +import { + Warning, + RECOVERYPHRASE_WORDCOUNT, + dropLeadingUserNumber, + getWarnings, +} from "../../crypto/mnemonic"; +import { warningIcon } from "../../components/icons"; import { questions } from "../faq/questions"; const pageContent = () => html`

Your seed phrase

Please provide your seed phrase

-

+
@@ -48,18 +106,46 @@ const init = (userNumber: bigint): Promise => const inputSeedPhraseInput = document.getElementById( "inputSeedPhrase" ) as HTMLInputElement; - const warningBadAnchor = document.getElementById( - "warningBadAnchor" - ) as HTMLParagraphElement; + + // Look up the warningsDiv element early; this must not be done inside in the warnings + // generation callback since -- due to debouncing -- it may be called after some period of + // time and the user may have left the page; we may effectively pick up the wrong element. + const warningsDiv = document.getElementById("warnings") as HTMLDivElement; + const warningsBox = document.querySelector( + "details.warnings-box" + ) as HTMLDetailsElement; + + // Debounce the warning generation as not to spam the user + let handle: number; + const debouncedWarnings = () => { + clearTimeout(handle); + handle = window.setTimeout(() => { + warningsDiv.innerHTML = ""; // Clear previously generated warnings + + const warnings = getWarningMessages( + userNumber, + inputSeedPhraseInput.value.trim() + ); + + if (warnings.length >= 1) { + warningsBox.classList.add("visible"); + warningsBox.classList.remove("hidden"); + + // Actually generate the warnings + for (const warning of warnings) { + const div = document.createElement("div"); + render(mkWarningDiv(warning), div); + warningsDiv.appendChild(div); + } + } else { + warningsBox.classList.add("hidden"); + warningsBox.classList.remove("visible"); + } + }, 500); + }; + inputSeedPhraseInput.oninput = () => { - const lun = leadingUserNumber(inputSeedPhraseInput.value.trim()); - if (lun !== null && lun !== userNumber) { - warningBadAnchor.innerHTML = `warning
- it looks like you are using a seed phrase for anchor ${lun} but the anchor you are trying to recover is ${userNumber} - `; - } else { - warningBadAnchor.innerHTML = ""; - } + debouncedWarnings(); }; const inputSeedPhraseContinue = document.getElementById( "inputSeedPhraseContinue" @@ -74,40 +160,78 @@ const init = (userNumber: bigint): Promise => const inputValue = dropLeadingUserNumber( inputSeedPhraseInput.value.trim() ); - if (validate(inputValue)) { - resolve(inputValue); - } else { - await displayError({ - title: "Invalid Seedphrase", - message: html`This is not a seed phrase generated by Internet Identity, please - make sure to copy the full seedphrase and try again. For more - information, please see - ${questions.invalidSeedphrase.question}`, - primaryButton: "Try again", - }); - resolve(inputSeedPhrase(userNumber)); - } + resolve(inputValue); }; }); -const dropLeadingUserNumber = (s: string): string => { - const i = s.indexOf(" "); - if (i !== -1 && parseUserNumber(s.slice(0, i)) !== null) { - return s.slice(i + 1); - } else { - return s; - } +export const getWarningMessages = ( + userNumber: bigint, + input: string +): (TemplateResult | string)[] => { + return getWarnings(userNumber, input).map((warning) => + warningMessage(userNumber, warning) + ); }; -const leadingUserNumber = (s: string): bigint | null => { - const i = s.indexOf(" "); - if (i !== -1) { - return parseUserNumber(s.slice(0, i)); - } else { - return null; +/** The warning messages, for each warning type. */ +export const warningMessage = ( + userNumber: bigint, + warning: Warning +): TemplateResult | string => { + switch (warning.type) { + case "bad_chars": { + return html`Unexpected character${warning.chars.length > 1 ? "s" : ""}: + ${warning.chars.map((c, i) => [ + html`${c === "\n" ? "newline" : c === "\t" ? "tab" : c}`, + i < warning.chars.length - 1 ? ", " : nothing, + ])}`; + } + + case "repeated_whitespace": { + return html`Multiple + whitespaces${warning.between + ? html` between ${warning.between[0]} and + ${warning.between[1]}` + : ""}`; + } + + case "bad_anchor": { + return html`Recovering anchor ${userNumber}, but recovery + phrase suggests anchor ${warning.anchor}`; + } + + case "bad_word_count": { + return `Recovery phrase should contain ${RECOVERYPHRASE_WORDCOUNT} words, but input contains ${ + warning.count + } word${warning.count > 1 ? "s" : ""}.`; + } + + case "bad_words": { + return html`Unexpected word${warning.words.length > 1 ? "s" : ""}: + ${warning.words.map((word, i) => [ + html`${word}`, + i < warning.words.length - 1 ? ", " : nothing, + ])}`; + } + + case "invalid": { + return html` + This does not look like a seed phrase generated by Internet Identity, + please make sure to copy the full seedphrase and try again. For more + information, please see + ${questions.invalidSeedphrase.question} + `; + } } }; + +const mkWarningDiv = (warningMessage: string | TemplateResult) => html`
+ ${warningIcon} +
+

${warningMessage}

+
+
`; diff --git a/src/frontend/src/utils/userNumber.ts b/src/frontend/src/utils/userNumber.ts index f3ad61c9e2..98e07a45d7 100644 --- a/src/frontend/src/utils/userNumber.ts +++ b/src/frontend/src/utils/userNumber.ts @@ -11,14 +11,10 @@ export const setUserNumber = (userNumber: bigint | undefined): void => { } }; -// BigInt parses various things we do not want to allow, like: -// - BigInt(whitespace) == 0 -// - Hex/Octal formatted numbers -// - Scientific notation -// So we check that the user has entered a sequence of digits only, +// We check that the user has entered a sequence of digits only, // before attempting to parse export const parseUserNumber = (s: string): bigint | null => { - if (/^\d+$/.test(s)) { + if (isUserNumber(s)) { try { return BigInt(s); } catch (err) { @@ -28,3 +24,13 @@ export const parseUserNumber = (s: string): bigint | null => { return null; } }; + +// This makes sure `s` is a viable user number; naively using BigInt +// on any string would also accept the following values, which are +// _not_ valid user numbers: +// - BigInt(whitespace) == 0 +// - Hex/Octal formatted numbers +// - Scientific notation +export const isUserNumber = (s: string): boolean => { + return /^\d+$/.test(s); +};