fix: let a user who lost the password delete the wallet, and warn before they can (closes #312)
Deleting a wallet was password-gated and importing its recovery phrase again was refused as a duplicate xpub by findWalletByXpub(), so a user who held the phrase but had forgotten the password could neither leave the wallet nor come back to it. The only escape was clearing extension storage through browser internals, which takes every other wallet with it, and nothing in the product ever warned that this was possible. DeleteWallet now offers "I have lost my password", a screen that destroys the wallet after the user types its name back. No password: requiring one to discard a secret protects nobody, because an attacker at the popup who wants the wallet gone can uninstall the extension, so the only person such a gate stops is the owner who forgot it. The typed name is a check that the user knows which wallet they are on, so it is matched with surrounding spaces and letter case ignored. This is the deletion route rather than the re-import route, and only one of the two. Re-import would have had to be built three times over (hd and xprv by xpub, key by address), would make the user retype the recovery phrase into a live popup in order to change a password, and reaches no end state that delete-then-import does not already reach through the existing import path and scanForAddresses(). Both routes share one finishDelete(), so the selection repair, the site permission cleanup and the AUTISTMASK_ACTIVE_CHANGED broadcast cannot diverge between them. The new screen is not in RESTORABLE_VIEWS, alongside delete-wallet-confirm: a popup reopened by accident must not land on a button that erases key material. It registers an onViewLeave() cleanup as well, not because a wallet name is a secret but because a typed confirmation left standing in a hidden view leaves a wallet one click from deletion. The two delete screens are siblings, so nothing is pushed on the way in and Back re-enters DeleteWallet through show(), which hands it back its wallet selection. AddWallet's password hint now states, per import mode, that the password cannot be recovered or reset and names what the only backup is. The hint line reserves its height so switching tabs cannot move the password fields under the pointer. The test drives the real view against a chrome.storage.local stub that structured-clones on both set and get, and asserts against what comes back out of storage rather than against the live state object, so it fails on the deletion of saveState() and not only on an in-memory splice.
This commit is contained in:
80
README.md
80
README.md
@@ -805,7 +805,9 @@ for the views listed in `RESTORABLE_VIEWS` (`src/popup/restorableViews.js`).
|
||||
Every other screen falls back to Home. The screens that display a secret —
|
||||
ExportPrivKey and ShowRecoveryPhrase — are deliberately absent from that list,
|
||||
so the popup can never reopen onto one of them with no password prompt in front
|
||||
of it.
|
||||
of it. So are the two that destroy one, DeleteWallet and
|
||||
DeleteWalletLostPassword: a popup reopened by accident must not land on a screen
|
||||
whose button erases key material.
|
||||
|
||||
A reopened popup renders the wallet list and the one screen it restores onto,
|
||||
and nothing else, so every screen on the stack behind that one is still the
|
||||
@@ -828,7 +830,10 @@ exit from that screen rather than only on its "Back" button, so nothing secret
|
||||
survives in a hidden view once the user has navigated away by any route. That
|
||||
covers the revealed private key and recovery phrase, the recovery phrase,
|
||||
private key or extended private key entered on AddWallet, and the password typed
|
||||
on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. DeleteWalletLostPassword
|
||||
registers one as well, for the neighbouring reason rather than that one: a
|
||||
wallet name is not a secret, but a typed confirmation left standing in a hidden
|
||||
view would leave a wallet one click from deletion.
|
||||
|
||||
#### Welcome (`welcome`)
|
||||
|
||||
@@ -889,7 +894,13 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- **From xprv**: instruction text and a masked extended private key
|
||||
input
|
||||
- Password + confirm password inputs, with a hint line whose wording depends
|
||||
on the selected tab
|
||||
on the selected tab. Every wording says that the password cannot be
|
||||
recovered or reset and names what the only backup of the wallet is — the
|
||||
recovery phrase, the private key or the extended private key, according to
|
||||
the tab. This is the only warning the user gets before the wallet exists;
|
||||
without it, the lost-password route on DeleteWallet is the first they
|
||||
would hear of it. The hint line reserves its height, so switching tabs
|
||||
cannot move the password fields under the pointer.
|
||||
- "Import" button
|
||||
- **Transitions**:
|
||||
- "Import" with a valid entry and a matching password of at least 12
|
||||
@@ -1244,6 +1255,7 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- Error line
|
||||
- Password input
|
||||
- "Confirm Delete" button
|
||||
- An underlined "I have lost my password" control
|
||||
- **Transitions**:
|
||||
- "Confirm Delete" (correct password, other wallets remain) → deletes the
|
||||
wallet and its site permissions, then → **Settings** with a "Wallet
|
||||
@@ -1253,10 +1265,54 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- Either way, the active address moves only if it belonged to the deleted
|
||||
wallet, and `AUTISTMASK_ACTIVE_CHANGED` is broadcast when it does
|
||||
(`src/shared/walletDelete.js`)
|
||||
- "Confirm Delete" (wrong password) → "Wrong password." on the error line,
|
||||
nothing deleted
|
||||
- "Confirm Delete" (wrong password) → "That password is incorrect. Please
|
||||
try again." on the error line, nothing deleted
|
||||
- "I have lost my password" → **DeleteWalletLostPassword**
|
||||
- "Back" → previous screen (Settings)
|
||||
|
||||
#### DeleteWalletLostPassword (`delete-wallet-lost-password`)
|
||||
|
||||
- **When**: User tapped "I have lost my password" on DeleteWallet.
|
||||
- **Why it exists**: without it, a user who has forgotten the password but still
|
||||
holds the recovery phrase has no route back into the product at all. Deletion
|
||||
was password-gated, and importing the phrase again is refused as a duplicate
|
||||
xpub by `findWalletByXpub()` while the wallet is still stored, so the only
|
||||
escape was clearing extension storage through browser internals — which takes
|
||||
every other wallet with it.
|
||||
- **Elements**:
|
||||
- "Back" button, "Delete Wallet Without a Password" heading
|
||||
- A statement that the password cannot be recovered or reset, so the wallet
|
||||
cannot be unlocked again, and that no password is needed to delete it
|
||||
- What deletion does and does not do: it erases the copy of the key stored
|
||||
on this device; nothing on chain changes and no money is moved
|
||||
- The route back — adding the wallet again with the recovery phrase and a
|
||||
new password — and, in bold, that without that phrase written down the
|
||||
deletion loses everything the wallet holds, forever
|
||||
- That the other wallets are not touched
|
||||
- The wallet's name, and a text input asking for it to be typed back
|
||||
- Error line
|
||||
- "Delete This Wallet Forever" button
|
||||
- **Transitions**:
|
||||
- "Delete This Wallet Forever" (name typed correctly) → the same two
|
||||
outcomes as "Confirm Delete" above, through the same `finishDelete()`, so
|
||||
the selection repair, permission cleanup and `AUTISTMASK_ACTIVE_CHANGED`
|
||||
broadcast are identical on both routes
|
||||
- "Delete This Wallet Forever" (name does not match) → "That is not the name
|
||||
of this wallet. Type <name> to confirm." on the error line, nothing
|
||||
deleted
|
||||
- "Back" → **DeleteWallet**, re-entered through its `show()` so the wallet
|
||||
selection comes back with it. The two delete screens are siblings rather
|
||||
than parent and child: nothing is pushed on the way here, so both have
|
||||
Settings as their Back target.
|
||||
- **Deliberately not password-gated.** A password in front of _discarding_ a
|
||||
secret protects nobody: an attacker at the popup who wants the wallet gone can
|
||||
uninstall the extension, so the only person such a gate stops is the owner who
|
||||
forgot it. The typed name is a check that the user knows which wallet they are
|
||||
on, not a secret, so it is matched with surrounding spaces and letter case
|
||||
ignored.
|
||||
- Not in `RESTORABLE_VIEWS`, alongside `delete-wallet-confirm`: a popup reopened
|
||||
by accident must not land on a screen whose button erases key material.
|
||||
|
||||
#### DeleteAddress (`delete-address-confirm`)
|
||||
|
||||
- **When**: User tapped the `[x]` next to an address on Home. Offered only on HD
|
||||
@@ -1273,13 +1329,13 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
refused: "+" derives the next unused index (`nextIndex` is a high-water
|
||||
mark), and re-importing the wallet's key material is rejected as a
|
||||
duplicate by `findWalletByXpub` while the wallet is still present. What
|
||||
works is deleting the whole wallet in Settings — password-gated, and it
|
||||
destroys the stored secret — then importing again, whereupon
|
||||
`scanForAddresses()` rediscovers the address **only if it has on-chain
|
||||
activity**. An address that was never used is not found by that scan. The
|
||||
text is written by `recoveryPathText()` rather than sitting in
|
||||
`index.html`, so it can name the wallet's own kind of key material: an
|
||||
xprv wallet has no recovery phrase to re-import.
|
||||
works is deleting the whole wallet in Settings — which destroys the stored
|
||||
secret — then importing again, whereupon `scanForAddresses()` rediscovers
|
||||
the address **only if it has on-chain activity**. An address that was
|
||||
never used is not found by that scan. The text is written by
|
||||
`recoveryPathText()` rather than sitting in `index.html`, so it can name
|
||||
the wallet's own kind of key material: an xprv wallet has no recovery
|
||||
phrase to re-import.
|
||||
- A warning when the address holds anything, ETH or any tracked ERC-20,
|
||||
followed by the holdings themselves via `balanceLinesForAddress()` and the
|
||||
USD total via `formatAddressTotal()` (see
|
||||
|
||||
Reference in New Issue
Block a user