Password
+
Reveal
diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js
index 97f7f9a..b6e0c3e 100644
--- a/src/popup/views/addWallet.js
+++ b/src/popup/views/addWallet.js
@@ -2,6 +2,8 @@ const {
$,
showView,
showFlash,
+ showError,
+ hideError,
goBack,
clearViewStack,
onViewLeave,
@@ -98,6 +100,7 @@ function clear() {
$("add-wallet-password").value = "";
$("add-wallet-password-confirm").value = "";
$("add-wallet-phrase-warning").style.visibility = "hidden";
+ hideError("add-wallet-password-error");
}
// Each wallet has its own password (its own encryptedSecret), so adding a
@@ -125,15 +128,18 @@ function validatePassword() {
const pw = $("add-wallet-password").value;
const pw2 = $("add-wallet-password-confirm").value;
if (!pw) {
- showFlash("Please choose a password.");
+ showError("add-wallet-password-error", "Please choose a password.");
return null;
}
if (pw.length < 12) {
- showFlash("Password must be at least 12 characters.");
+ showError(
+ "add-wallet-password-error",
+ "Password must be at least 12 characters.",
+ );
return null;
}
if (pw !== pw2) {
- showFlash("Passwords do not match.");
+ showError("add-wallet-password-error", "Passwords do not match.");
return null;
}
return pw;
@@ -342,8 +348,10 @@ function init(ctx) {
$("add-wallet-phrase-warning").style.visibility = "visible";
});
- // Import / confirm
+ // Import / confirm. Each press starts with no password error on screen:
+ // validatePassword() puts it back if the password is still wrong.
$("btn-add-wallet-confirm").addEventListener("click", async () => {
+ hideError("add-wallet-password-error");
if (currentMode === "mnemonic") {
await importMnemonic(ctx);
} else if (currentMode === "privkey") {
diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js
index e7bbb89..5808fcb 100644
--- a/src/popup/views/deleteWallet.js
+++ b/src/popup/views/deleteWallet.js
@@ -2,6 +2,8 @@ const {
$,
showView,
showFlash,
+ showError,
+ hideError,
goBack,
clearViewStack,
onViewLeave,
@@ -55,8 +57,7 @@ function confirmKey(name) {
function clear() {
deleteWalletIndex = null;
$("delete-wallet-password").value = "";
- $("delete-wallet-flash").textContent = "";
- $("delete-wallet-flash").style.visibility = "hidden";
+ hideError("delete-wallet-password-error");
}
// The lost-password screen holds no secret — a wallet name is not one —
@@ -232,19 +233,22 @@ function init(_ctx) {
$("btn-delete-wallet-confirm").addEventListener("click", async () => {
const pw = $("delete-wallet-password").value;
if (!pw) {
- $("delete-wallet-flash").textContent =
- "Please enter your password.";
- $("delete-wallet-flash").style.visibility = "visible";
+ showError(
+ "delete-wallet-password-error",
+ "Please enter your password.",
+ );
return;
}
if (deleteWalletIndex === null) {
- $("delete-wallet-flash").textContent =
- "No wallet selected for deletion.";
- $("delete-wallet-flash").style.visibility = "visible";
+ showError(
+ "delete-wallet-password-error",
+ "No wallet selected for deletion.",
+ );
return;
}
+ hideError("delete-wallet-password-error");
const btn = $("btn-delete-wallet-confirm");
btn.disabled = true;
btn.classList.add("text-muted");
@@ -256,9 +260,10 @@ function init(_ctx) {
try {
await decryptWithPassword(wallet.encryptedSecret, pw);
} catch {
- $("delete-wallet-flash").textContent =
- "That password is incorrect. Please try again.";
- $("delete-wallet-flash").style.visibility = "visible";
+ showError(
+ "delete-wallet-password-error",
+ "That password is incorrect. Please try again.",
+ );
btn.disabled = false;
btn.classList.remove("text-muted");
return;
diff --git a/src/popup/views/exportPrivkey.js b/src/popup/views/exportPrivkey.js
index b027c13..3860387 100644
--- a/src/popup/views/exportPrivkey.js
+++ b/src/popup/views/exportPrivkey.js
@@ -20,6 +20,8 @@ const {
$,
showView,
showFlash,
+ showError,
+ hideError,
flashCopyFeedback,
goBack,
onViewLeave,
@@ -56,11 +58,6 @@ function isCurrentReveal(generation) {
);
}
-function fail(message) {
- $("export-privkey-flash").textContent = message;
- $("export-privkey-flash").style.visibility = "visible";
-}
-
// Wipe every trace of the key and drop the address selection. Safe to call
// when nothing was ever revealed, and safe to call twice.
function clear() {
@@ -71,8 +68,7 @@ function clear() {
$("export-privkey-password").value = "";
$("export-privkey-result").classList.add("hidden");
$("export-privkey-password-section").classList.remove("hidden");
- $("export-privkey-flash").textContent = "";
- $("export-privkey-flash").style.visibility = "hidden";
+ hideError("export-privkey-password-error");
}
function show(walletIdx, addrIdx) {
@@ -112,15 +108,19 @@ function show(walletIdx, addrIdx) {
async function reveal() {
const password = $("export-privkey-password").value;
if (!password) {
- fail("Please enter your password.");
+ showError(
+ "export-privkey-password-error",
+ "Please enter your password.",
+ );
return;
}
if (walletIndex === null) {
- fail("No address is selected.");
+ showError("export-privkey-password-error", "No address is selected.");
return;
}
const wallet = state.wallets[walletIndex];
+ hideError("export-privkey-password-error");
const btn = $("btn-export-privkey-confirm");
btn.disabled = true;
btn.classList.add("text-muted");
@@ -140,11 +140,12 @@ async function reveal() {
$("export-privkey-password-section").classList.add("hidden");
$("export-privkey-value").textContent = signer.privateKey;
$("export-privkey-result").classList.remove("hidden");
- $("export-privkey-flash").textContent = "";
- $("export-privkey-flash").style.visibility = "hidden";
} catch {
if (!isCurrentReveal(generation)) return;
- fail("That password is incorrect. Please try again.");
+ showError(
+ "export-privkey-password-error",
+ "That password is incorrect. Please try again.",
+ );
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");
diff --git a/src/popup/views/showPhrase.js b/src/popup/views/showPhrase.js
index 839cf67..482dbe9 100644
--- a/src/popup/views/showPhrase.js
+++ b/src/popup/views/showPhrase.js
@@ -21,6 +21,8 @@ const {
$,
showView,
showFlash,
+ showError,
+ hideError,
flashCopyFeedback,
goBack,
onViewLeave,
@@ -52,11 +54,6 @@ function isCurrentReveal(generation) {
);
}
-function fail(message) {
- $("show-phrase-flash").textContent = message;
- $("show-phrase-flash").style.visibility = "visible";
-}
-
// Wipe every trace of the phrase and drop the wallet selection. Safe to
// call when nothing was ever revealed, and safe to call twice.
function clear() {
@@ -66,8 +63,7 @@ function clear() {
$("show-phrase-password").value = "";
$("show-phrase-result").classList.add("hidden");
$("show-phrase-password-section").classList.remove("hidden");
- $("show-phrase-flash").textContent = "";
- $("show-phrase-flash").style.visibility = "hidden";
+ hideError("show-phrase-password-error");
}
function show(walletIdx) {
@@ -90,19 +86,23 @@ function show(walletIdx) {
async function reveal() {
const password = $("show-phrase-password").value;
if (!password) {
- fail("Please enter your password.");
+ showError("show-phrase-password-error", "Please enter your password.");
return;
}
if (walletIndex === null) {
- fail("No wallet is selected.");
+ showError("show-phrase-password-error", "No wallet is selected.");
return;
}
const wallet = state.wallets[walletIndex];
if (!walletHasRecoveryPhrase(wallet)) {
- fail("This wallet does not have a recovery phrase.");
+ showError(
+ "show-phrase-password-error",
+ "This wallet does not have a recovery phrase.",
+ );
return;
}
+ hideError("show-phrase-password-error");
const btn = $("btn-show-phrase-reveal");
btn.disabled = true;
btn.classList.add("text-muted");
@@ -120,13 +120,14 @@ async function reveal() {
$("show-phrase-password-section").classList.add("hidden");
$("show-phrase-value").textContent = phrase;
$("show-phrase-result").classList.remove("hidden");
- $("show-phrase-flash").textContent = "";
- $("show-phrase-flash").style.visibility = "hidden";
} catch {
if (!isCurrentReveal(generation)) return;
// Deliberately not the caught error: the message is fixed so that
// nothing derived from the ciphertext or the attempt can surface.
- fail("That password is incorrect. Please try again.");
+ showError(
+ "show-phrase-password-error",
+ "That password is incorrect. Please try again.",
+ );
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");
diff --git a/tests/addressScanCancelled.test.js b/tests/addressScanCancelled.test.js
index 984a51d..febfd63 100644
--- a/tests/addressScanCancelled.test.js
+++ b/tests/addressScanCancelled.test.js
@@ -39,6 +39,8 @@ jest.doMock("../src/popup/views/helpers", () => ({
$: element,
showView: () => {},
showFlash: () => {},
+ showError: () => {},
+ hideError: () => {},
goBack: () => {},
clearViewStack: () => {},
onViewLeave: () => {},
diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js
index 4cc8877..0ba4df4 100644
--- a/tests/deleteWalletLostPassword.test.js
+++ b/tests/deleteWalletLostPassword.test.js
@@ -253,7 +253,7 @@ describe("reaching the screen", () => {
const { decryptWithPassword } = require("../src/shared/vault");
decryptWithPassword.mockRejectedValue(new Error("nope"));
await click("btn-delete-wallet-confirm");
- expect(node("delete-wallet-flash").textContent).toBe(
+ expect(node("delete-wallet-password-error").textContent).toBe(
"That password is incorrect. Please try again.",
);
});
diff --git a/tests/e2e/run.js b/tests/e2e/run.js
index 8723f03..15e310b 100644
--- a/tests/e2e/run.js
+++ b/tests/e2e/run.js
@@ -809,7 +809,7 @@ async function secretScreenState(page, view) {
return page.evaluate(
(v) => ({
value: document.getElementById(v + "-value").textContent,
- error: document.getElementById(v + "-flash").textContent,
+ error: document.getElementById(v + "-password-error").textContent,
html: document.getElementById("view-" + v).innerHTML,
resultHidden: document
.getElementById(v + "-result")
@@ -906,7 +906,8 @@ test("a wrong password reveals nothing (#161)", async (env) => {
await env.page.click("#btn-show-phrase-reveal");
await env.page.waitForFunction(
() =>
- document.getElementById("show-phrase-flash").textContent.length > 0,
+ document.getElementById("show-phrase-password-error").textContent
+ .length > 0,
null,
{ timeout: 60000 },
);
@@ -1993,13 +1994,14 @@ test("an over-long flash message keeps to one line (#252)", async (env) => {
// Every screen that asks for a password reserves room for one line of error.
// The two on the dApp approval screens also have a border and padding, which
-// that reserved height has to cover too.
+// that reserved height has to cover too. The add wallet screen's line sits
+// beside its button rather than above it, and has its own test below.
const PASSWORD_ERROR_CONTAINERS = [
"approve-tx-error",
"approve-sign-error",
- "export-privkey-flash",
- "show-phrase-flash",
- "delete-wallet-flash",
+ "export-privkey-password-error",
+ "show-phrase-password-error",
+ "delete-wallet-password-error",
"confirm-tx-password-error",
];
@@ -2064,6 +2066,107 @@ test("a password error moves nothing on any screen (#297)", async (env) => {
}
});
+// ------------------------------------------ add wallet Import button (#493)
+
+// At 360x600 the add wallet screen's Import button already starts near the
+// bottom of the popup, so its password error line sits beside the button
+// rather than above it. Measures the button and the line empty and again
+// filled with the longest error addWallet.js puts there. Runs in the page.
+function measureImportButton() {
+ const button = document.getElementById("btn-add-wallet-confirm");
+ const line = document.getElementById("add-wallet-password-error");
+ const measure = () => {
+ const b = button.getBoundingClientRect();
+ const l = line.getBoundingClientRect();
+ return {
+ buttonTop: b.top + window.scrollY,
+ buttonBottom: b.bottom + window.scrollY,
+ lineTop: l.top + window.scrollY,
+ lineBottom: l.bottom + window.scrollY,
+ };
+ };
+ const empty = measure();
+ line.textContent = "Password must be at least 12 characters.";
+ line.style.visibility = "visible";
+ const filled = measure();
+ line.textContent = "";
+ line.style.visibility = "hidden";
+ return { empty, filled };
+}
+
+test("the add wallet password error leaves Import where it was (#493)", async (env) => {
+ const page = await openPopup(env.ctx, env.popupUrl);
+ try {
+ await page.setViewportSize(POPUP_VIEWPORT);
+ // Brought up by toggling classes, as in the test above. The note
+ // addWallet.js shows once a wallet exists is the only part of the
+ // screen that differs between the first wallet and a later one.
+ await page.evaluate(() => {
+ const screen = document.getElementById("view-add-wallet");
+ for (const view of document.querySelectorAll(".view")) {
+ view.classList.toggle("hidden", view !== screen);
+ }
+ });
+ for (const walletExists of [false, true]) {
+ await page.evaluate(
+ (shown) =>
+ document
+ .getElementById("add-wallet-separate-password-note")
+ .classList.toggle("hidden", !shown),
+ walletExists,
+ );
+ for (const tab of ["tab-mnemonic", "tab-privkey", "tab-xprv"]) {
+ await page.click("#" + tab);
+ const { empty, filled } =
+ await page.evaluate(measureImportButton);
+ const where =
+ "#" +
+ tab +
+ (walletExists
+ ? " with a wallet already added"
+ : " for the first wallet");
+ // Printed pass or fail, as the dust threshold test does.
+ console.log(
+ "# add wallet Import top, " +
+ where +
+ ": " +
+ empty.buttonTop +
+ "px",
+ );
+ for (const m of [empty, filled]) {
+ assert(
+ m.lineTop >= m.buttonTop &&
+ m.lineBottom <= m.buttonBottom,
+ "the error line on " +
+ where +
+ " does not fit beside Import, so it adds height: " +
+ JSON.stringify(m),
+ );
+ }
+ assert(
+ filled.buttonTop === empty.buttonTop,
+ "Import moved " +
+ (filled.buttonTop - empty.buttonTop) +
+ "px when the error appeared on " +
+ where,
+ );
+ if (!walletExists) {
+ assert(
+ empty.buttonTop < POPUP_VIEWPORT.height,
+ "Import starts at " +
+ empty.buttonTop +
+ "px on " +
+ where +
+ ", below the fold",
+ );
+ }
+ }
+ }
+ } finally {
+ await page.close();
+ }
+});
+
// --------------------------------------------- confirmation screen (#238)
//
// The screen that decides what gets signed. The arithmetic underneath it
diff --git a/tests/exportPrivkey.test.js b/tests/exportPrivkey.test.js
index 8629431..75f8081 100644
--- a/tests/exportPrivkey.test.js
+++ b/tests/exportPrivkey.test.js
@@ -207,7 +207,7 @@ describe("a decrypt still running when the screen is left", () => {
});
// Same hole on the failure path: a wrong-password error written after
- // the wipe would restore the flash line on a screen the user has left.
+ // the wipe would restore the error line on a screen the user has left.
test("never writes the failure message either", async () => {
const { helpers, vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
@@ -217,8 +217,10 @@ describe("a decrypt still running when the screen is left", () => {
reveal.reject(new Error("decryption failed"));
await reveal.pending;
- expect(node("export-privkey-flash").textContent).toBe("");
- expect(node("export-privkey-flash").style.visibility).toBe("hidden");
+ expect(node("export-privkey-password-error").textContent).toBe("");
+ expect(node("export-privkey-password-error").style.visibility).toBe(
+ "hidden",
+ );
});
});
@@ -260,7 +262,7 @@ describe("a reveal that is not interrupted", () => {
await reveal.pending;
expect(node("export-privkey-value").textContent).toBe("");
- expect(node("export-privkey-flash").textContent).toBe(
+ expect(node("export-privkey-password-error").textContent).toBe(
"That password is incorrect. Please try again.",
);
});
diff --git a/tests/passwordErrorLines.test.js b/tests/passwordErrorLines.test.js
new file mode 100644
index 0000000..bbc09f5
--- /dev/null
+++ b/tests/passwordErrorLines.test.js
@@ -0,0 +1,159 @@
+// Every screen that asks for a password shows a password error the same way:
+// through showError() and hideError() in src/popup/views/helpers.js, in a
+// fixed-height error line below the password field, and never in the flash
+// line at the top of the popup
+// (https://git.eeqj.de/sneak/AutistMask/issues/493). These boot the real popup
+// over src/popup/index.html, so each error line has to exist in the markup,
+// and check that the error appears in it and clears again.
+
+jest.mock("../src/shared/vault", () => ({
+ decryptWithPassword: jest.fn(),
+ encryptWithPassword: jest.fn(),
+}));
+
+const {
+ bootPopup,
+ cleanupPopup,
+ unversionedValidProfile,
+} = require("./support/popupBoot");
+
+const PASSWORD = "correct horse battery staple";
+const WRONG_PASSWORD = "That password is incorrect. Please try again.";
+
+afterEach(() => {
+ cleanupPopup();
+});
+
+// The error line as the user sees it.
+function errorLine(page, id) {
+ return {
+ inMarkup: page.document.authoredIds.has(id),
+ text: page.text(id),
+ visibility: page.node(id).style.visibility,
+ };
+}
+
+function shown(text) {
+ return { inMarkup: true, text, visibility: "visible" };
+}
+
+const cleared = { inMarkup: true, text: "", visibility: "hidden" };
+
+describe("the add wallet screen", () => {
+ const ERROR = "add-wallet-password-error";
+
+ // First run: Welcome, "Add wallet", then the die for a valid phrase.
+ async function openAddWallet() {
+ const page = await bootPopup(undefined);
+ await page.click("btn-welcome-add");
+ await page.click("btn-generate-phrase");
+ return page;
+ }
+
+ function setPasswords(page, password, confirm) {
+ page.node("add-wallet-password").value = password;
+ page.node("add-wallet-password-confirm").value = confirm;
+ }
+
+ test("shows a password problem below the password fields, and clears it on the next press", async () => {
+ const page = await openAddWallet();
+ setPasswords(page, "short", "short");
+ await page.click("btn-add-wallet-confirm");
+ expect(errorLine(page, ERROR)).toEqual(
+ shown("Password must be at least 12 characters."),
+ );
+ expect(page.text("flash-msg")).toBe("");
+
+ // The password is fixed and the phrase emptied: the password error
+ // goes, and the phrase problem is still reported in the flash line.
+ setPasswords(page, PASSWORD, PASSWORD);
+ page.node("wallet-mnemonic").value = "";
+ await page.click("btn-add-wallet-confirm");
+ expect(errorLine(page, ERROR)).toEqual(cleared);
+ expect(page.text("flash-msg")).toBe(
+ "Enter a recovery phrase, or press the die.",
+ );
+
+ // Leaving clears the flash line and stops its timer, which would
+ // otherwise fire after this page is gone.
+ await page.click("btn-add-wallet-back");
+ });
+
+ test("clears the error when the screen is shown again", async () => {
+ const page = await openAddWallet();
+ setPasswords(page, PASSWORD, PASSWORD + " typo");
+ await page.click("btn-add-wallet-confirm");
+ expect(errorLine(page, ERROR)).toEqual(
+ shown("Passwords do not match."),
+ );
+
+ await page.click("btn-add-wallet-back");
+ await page.click("btn-welcome-add");
+ expect(errorLine(page, ERROR)).toEqual(cleared);
+ });
+});
+
+describe.each([
+ {
+ screen: "the private key export screen",
+ open: () => require("../src/popup/views/exportPrivkey").show(0, 0),
+ field: "export-privkey-password",
+ button: "btn-export-privkey-confirm",
+ error: "export-privkey-password-error",
+ },
+ {
+ screen: "the recovery phrase screen",
+ open: () => require("../src/popup/views/showPhrase").show(0),
+ field: "show-phrase-password",
+ button: "btn-show-phrase-reveal",
+ error: "show-phrase-password-error",
+ },
+ {
+ screen: "the delete wallet screen",
+ open: () => require("../src/popup/views/deleteWallet").show(0),
+ field: "delete-wallet-password",
+ button: "btn-delete-wallet-confirm",
+ error: "delete-wallet-password-error",
+ },
+])("$screen", ({ open, field, button, error }) => {
+ // Opens the screen and enters a password the vault rejects.
+ async function failedAttempt() {
+ const page = await bootPopup(unversionedValidProfile());
+ open();
+ const { decryptWithPassword } = require("../src/shared/vault");
+ decryptWithPassword.mockRejectedValue(new Error("wrong password"));
+ page.node(field).value = "not the password";
+ await page.click(button);
+ return { page, decryptWithPassword };
+ }
+
+ test("shows a wrong password below the password field", async () => {
+ const { page } = await failedAttempt();
+ expect(errorLine(page, error)).toEqual(shown(WRONG_PASSWORD));
+ });
+
+ test("clears the error while the next password is checked", async () => {
+ const { page, decryptWithPassword } = await failedAttempt();
+ let rejectDecrypt;
+ decryptWithPassword.mockReturnValue(
+ new Promise((resolve, reject) => {
+ rejectDecrypt = reject;
+ }),
+ );
+
+ page.node(field).value = "another guess";
+ const pressed = page.click(button);
+ await page.settle();
+ expect(errorLine(page, error)).toEqual(cleared);
+
+ rejectDecrypt(new Error("wrong password"));
+ await pressed;
+ expect(errorLine(page, error)).toEqual(shown(WRONG_PASSWORD));
+ });
+
+ test("clears the error when the screen is shown again", async () => {
+ const { page } = await failedAttempt();
+ open();
+ expect(errorLine(page, error)).toEqual(cleared);
+ });
+});