Skip to content

Commit 15cd69d

Browse files
owencrastonmetamaskbot
owencraston
and
metamaskbot
authored
fix: password reset (#25847)
## **Description** 1. What is the reason for the change? - This fix addresses a user facing bug in production - When a user locks the wallet and goes through the forget password flow, the wallet is not accessible with the new password. Instead the user must use the old password to unlock the wallet. - This is a bug in the keyring controller because the encryptionKey gets generated with the old password and is not reset when the user locks the wallet and generates a new password. 2. What is the improvement/solution? - This fix upgrades the keyring controller to version [17.1.1](https://github.com/MetaMask/core/blob/main/packages/keyring-controller/CHANGELOG.md#1711) which fixes this bug by clearing the encryption key/salt when the user locks the wallet as well as when they submit their password. This ensures that the encryption key is always in sync with the latest password. - The keyring controller pr for this fix can be found [here](MetaMask/core#4514) - This change involved a major version bump for the keyring controller from 16 to 17. [Here](https://github.com/MetaMask/core/blob/main/packages/keyring-controller/CHANGELOG.md#1711) is the changelog for this bump. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25847?quickstart=1) ## **Related issues** Fixes: #25696 ## **Manual testing steps** - Open the extension - Proceed to Forget password flow - Paste your secret recovery phrase - Set a new different password - Proceed to Restore your Wallet - After entering the account, lock it and try to login with the new password - You should be able to log in with your new password ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** https://github.com/MetaMask/metamask-extension/assets/82837486/0de00905-ee01-4241-a90e-da60c5eb0976 ### **After** https://github.com/user-attachments/assets/f1f091c4-8974-415d-af28-d7b0b46184cb ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
1 parent 3c100bc commit 15cd69d

File tree

7 files changed

+186
-29
lines changed

7 files changed

+186
-29
lines changed

‎lavamoat/browserify/beta/policy.json‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,14 +1643,22 @@
16431643
"@metamask/keyring-controller": {
16441644
"packages": {
16451645
"@ethereumjs/tx>@ethereumjs/util": true,
1646-
"@metamask/base-controller": true,
16471646
"@metamask/browser-passworder": true,
16481647
"@metamask/eth-sig-util": true,
1648+
"@metamask/keyring-controller>@metamask/base-controller": true,
16491649
"@metamask/keyring-controller>@metamask/eth-hd-keyring": true,
16501650
"@metamask/keyring-controller>@metamask/eth-simple-keyring": true,
1651+
"@metamask/keyring-controller>@metamask/utils": true,
16511652
"@metamask/keyring-controller>ethereumjs-wallet": true,
1652-
"@metamask/name-controller>async-mutex": true,
1653-
"@metamask/utils": true
1653+
"@metamask/name-controller>async-mutex": true
1654+
}
1655+
},
1656+
"@metamask/keyring-controller>@metamask/base-controller": {
1657+
"globals": {
1658+
"setTimeout": true
1659+
},
1660+
"packages": {
1661+
"immer": true
16541662
}
16551663
},
16561664
"@metamask/keyring-controller>@metamask/eth-hd-keyring": {
@@ -1676,6 +1684,21 @@
16761684
"mocha>serialize-javascript>randombytes": true
16771685
}
16781686
},
1687+
"@metamask/keyring-controller>@metamask/utils": {
1688+
"globals": {
1689+
"TextDecoder": true,
1690+
"TextEncoder": true
1691+
},
1692+
"packages": {
1693+
"@metamask/rpc-errors>@metamask/utils>@metamask/superstruct": true,
1694+
"@metamask/utils>@scure/base": true,
1695+
"@metamask/utils>pony-cause": true,
1696+
"@noble/hashes": true,
1697+
"browserify>buffer": true,
1698+
"nock>debug": true,
1699+
"semver": true
1700+
}
1701+
},
16791702
"@metamask/keyring-controller>ethereumjs-wallet": {
16801703
"packages": {
16811704
"@metamask/keyring-controller>ethereumjs-wallet>ethereum-cryptography": true,

‎lavamoat/browserify/flask/policy.json‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,14 +1643,22 @@
16431643
"@metamask/keyring-controller": {
16441644
"packages": {
16451645
"@ethereumjs/tx>@ethereumjs/util": true,
1646-
"@metamask/base-controller": true,
16471646
"@metamask/browser-passworder": true,
16481647
"@metamask/eth-sig-util": true,
1648+
"@metamask/keyring-controller>@metamask/base-controller": true,
16491649
"@metamask/keyring-controller>@metamask/eth-hd-keyring": true,
16501650
"@metamask/keyring-controller>@metamask/eth-simple-keyring": true,
1651+
"@metamask/keyring-controller>@metamask/utils": true,
16511652
"@metamask/keyring-controller>ethereumjs-wallet": true,
1652-
"@metamask/name-controller>async-mutex": true,
1653-
"@metamask/utils": true
1653+
"@metamask/name-controller>async-mutex": true
1654+
}
1655+
},
1656+
"@metamask/keyring-controller>@metamask/base-controller": {
1657+
"globals": {
1658+
"setTimeout": true
1659+
},
1660+
"packages": {
1661+
"immer": true
16541662
}
16551663
},
16561664
"@metamask/keyring-controller>@metamask/eth-hd-keyring": {
@@ -1676,6 +1684,21 @@
16761684
"mocha>serialize-javascript>randombytes": true
16771685
}
16781686
},
1687+
"@metamask/keyring-controller>@metamask/utils": {
1688+
"globals": {
1689+
"TextDecoder": true,
1690+
"TextEncoder": true
1691+
},
1692+
"packages": {
1693+
"@metamask/rpc-errors>@metamask/utils>@metamask/superstruct": true,
1694+
"@metamask/utils>@scure/base": true,
1695+
"@metamask/utils>pony-cause": true,
1696+
"@noble/hashes": true,
1697+
"browserify>buffer": true,
1698+
"nock>debug": true,
1699+
"semver": true
1700+
}
1701+
},
16791702
"@metamask/keyring-controller>ethereumjs-wallet": {
16801703
"packages": {
16811704
"@metamask/keyring-controller>ethereumjs-wallet>ethereum-cryptography": true,

‎lavamoat/browserify/main/policy.json‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,14 +1643,22 @@
16431643
"@metamask/keyring-controller": {
16441644
"packages": {
16451645
"@ethereumjs/tx>@ethereumjs/util": true,
1646-
"@metamask/base-controller": true,
16471646
"@metamask/browser-passworder": true,
16481647
"@metamask/eth-sig-util": true,
1648+
"@metamask/keyring-controller>@metamask/base-controller": true,
16491649
"@metamask/keyring-controller>@metamask/eth-hd-keyring": true,
16501650
"@metamask/keyring-controller>@metamask/eth-simple-keyring": true,
1651+
"@metamask/keyring-controller>@metamask/utils": true,
16511652
"@metamask/keyring-controller>ethereumjs-wallet": true,
1652-
"@metamask/name-controller>async-mutex": true,
1653-
"@metamask/utils": true
1653+
"@metamask/name-controller>async-mutex": true
1654+
}
1655+
},
1656+
"@metamask/keyring-controller>@metamask/base-controller": {
1657+
"globals": {
1658+
"setTimeout": true
1659+
},
1660+
"packages": {
1661+
"immer": true
16541662
}
16551663
},
16561664
"@metamask/keyring-controller>@metamask/eth-hd-keyring": {
@@ -1676,6 +1684,21 @@
16761684
"mocha>serialize-javascript>randombytes": true
16771685
}
16781686
},
1687+
"@metamask/keyring-controller>@metamask/utils": {
1688+
"globals": {
1689+
"TextDecoder": true,
1690+
"TextEncoder": true
1691+
},
1692+
"packages": {
1693+
"@metamask/rpc-errors>@metamask/utils>@metamask/superstruct": true,
1694+
"@metamask/utils>@scure/base": true,
1695+
"@metamask/utils>pony-cause": true,
1696+
"@noble/hashes": true,
1697+
"browserify>buffer": true,
1698+
"nock>debug": true,
1699+
"semver": true
1700+
}
1701+
},
16791702
"@metamask/keyring-controller>ethereumjs-wallet": {
16801703
"packages": {
16811704
"@metamask/keyring-controller>ethereumjs-wallet>ethereum-cryptography": true,

‎lavamoat/browserify/mmi/policy.json‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,14 +1735,22 @@
17351735
"@metamask/keyring-controller": {
17361736
"packages": {
17371737
"@ethereumjs/tx>@ethereumjs/util": true,
1738-
"@metamask/base-controller": true,
17391738
"@metamask/browser-passworder": true,
17401739
"@metamask/eth-sig-util": true,
1740+
"@metamask/keyring-controller>@metamask/base-controller": true,
17411741
"@metamask/keyring-controller>@metamask/eth-hd-keyring": true,
17421742
"@metamask/keyring-controller>@metamask/eth-simple-keyring": true,
1743+
"@metamask/keyring-controller>@metamask/utils": true,
17431744
"@metamask/keyring-controller>ethereumjs-wallet": true,
1744-
"@metamask/name-controller>async-mutex": true,
1745-
"@metamask/utils": true
1745+
"@metamask/name-controller>async-mutex": true
1746+
}
1747+
},
1748+
"@metamask/keyring-controller>@metamask/base-controller": {
1749+
"globals": {
1750+
"setTimeout": true
1751+
},
1752+
"packages": {
1753+
"immer": true
17461754
}
17471755
},
17481756
"@metamask/keyring-controller>@metamask/eth-hd-keyring": {
@@ -1768,6 +1776,21 @@
17681776
"mocha>serialize-javascript>randombytes": true
17691777
}
17701778
},
1779+
"@metamask/keyring-controller>@metamask/utils": {
1780+
"globals": {
1781+
"TextDecoder": true,
1782+
"TextEncoder": true
1783+
},
1784+
"packages": {
1785+
"@metamask/rpc-errors>@metamask/utils>@metamask/superstruct": true,
1786+
"@metamask/utils>@scure/base": true,
1787+
"@metamask/utils>pony-cause": true,
1788+
"@noble/hashes": true,
1789+
"browserify>buffer": true,
1790+
"nock>debug": true,
1791+
"semver": true
1792+
}
1793+
},
17711794
"@metamask/keyring-controller>ethereumjs-wallet": {
17721795
"packages": {
17731796
"@metamask/keyring-controller>ethereumjs-wallet>ethereum-cryptography": true,

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@
314314
"@metamask/gas-fee-controller": "^18.0.0",
315315
"@metamask/jazzicon": "^2.0.0",
316316
"@metamask/keyring-api": "^8.0.0",
317-
"@metamask/keyring-controller": "^16.1.0",
317+
"@metamask/keyring-controller": "^17.1.1",
318318
"@metamask/logging-controller": "^3.0.1",
319319
"@metamask/logo": "^3.1.2",
320320
"@metamask/message-manager": "^7.3.0",

‎test/e2e/accounts/forgot-password.spec.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { Suite } from 'mocha';
2+
import { unlockWallet, withFixtures, TEST_SEED_PHRASE_TWO } from '../helpers';
3+
import FixtureBuilder from '../fixture-builder';
4+
import { Driver } from '../webdriver/driver';
5+
6+
const newPassword = 'this is the best password ever';
7+
8+
describe('Forgot password', function (this: Suite) {
9+
it('resets password and then unlock wallet with new password', async function () {
10+
await withFixtures(
11+
{
12+
fixtures: new FixtureBuilder().build(),
13+
title: this.test?.fullTitle(),
14+
},
15+
async ({ driver }: { driver: Driver }) => {
16+
await unlockWallet(driver);
17+
18+
// Lock Wallet
19+
await driver.waitForSelector(
20+
'[data-testid="account-options-menu-button"]',
21+
);
22+
await driver.clickElement(
23+
'[data-testid="account-options-menu-button"]',
24+
);
25+
await driver.clickElement({
26+
text: 'Lock MetaMask',
27+
tag: 'div',
28+
});
29+
30+
// Go to reset password page
31+
await driver.waitForSelector('.unlock-page__link');
32+
await driver.clickElement({
33+
text: 'Forgot password?',
34+
tag: 'a',
35+
});
36+
37+
// Reset password with a new password
38+
await driver.pasteIntoField(
39+
'[data-testid="import-srp__srp-word-0"]',
40+
TEST_SEED_PHRASE_TWO,
41+
);
42+
43+
await driver.fill('#password', newPassword);
44+
await driver.fill('#confirm-password', newPassword);
45+
await driver.press('#confirm-password', driver.Key.ENTER);
46+
47+
// Lock wallet again
48+
await driver.waitForSelector(
49+
'[data-testid="account-options-menu-button"]',
50+
);
51+
await driver.clickElement(
52+
'[data-testid="account-options-menu-button"]',
53+
);
54+
await driver.clickElement({
55+
text: 'Lock MetaMask',
56+
tag: 'div',
57+
});
58+
59+
// log in with new password
60+
await driver.fill('#password', newPassword);
61+
await driver.press('#password', driver.Key.ENTER);
62+
},
63+
);
64+
});
65+
});

‎yarn.lock‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5648,7 +5648,7 @@ __metadata:
56485648
languageName: node
56495649
linkType: hard
56505650

5651-
"@metamask/keyring-controller@npm:^16.0.0, @metamask/keyring-controller@npm:^16.1.0":
5651+
"@metamask/keyring-controller@npm:^16.0.0":
56525652
version: 16.1.0
56535653
resolution: "@metamask/keyring-controller@npm:16.1.0"
56545654
dependencies:
@@ -5669,24 +5669,24 @@ __metadata:
56695669
languageName: node
56705670
linkType: hard
56715671

5672-
"@metamask/keyring-controller@npm:^17.1.0":
5673-
version: 17.1.0
5674-
resolution: "@metamask/keyring-controller@npm:17.1.0"
5672+
"@metamask/keyring-controller@npm:^17.1.0, @metamask/keyring-controller@npm:^17.1.1":
5673+
version: 17.1.1
5674+
resolution: "@metamask/keyring-controller@npm:17.1.1"
56755675
dependencies:
56765676
"@ethereumjs/util": "npm:^8.1.0"
56775677
"@keystonehq/metamask-airgapped-keyring": "npm:^0.14.1"
5678-
"@metamask/base-controller": "npm:^6.0.0"
5678+
"@metamask/base-controller": "npm:^6.0.1"
56795679
"@metamask/browser-passworder": "npm:^4.3.0"
56805680
"@metamask/eth-hd-keyring": "npm:^7.0.1"
56815681
"@metamask/eth-sig-util": "npm:^7.0.1"
56825682
"@metamask/eth-simple-keyring": "npm:^6.0.1"
56835683
"@metamask/keyring-api": "npm:^8.0.0"
5684-
"@metamask/message-manager": "npm:^10.0.0"
5685-
"@metamask/utils": "npm:^8.3.0"
5684+
"@metamask/message-manager": "npm:^10.0.1"
5685+
"@metamask/utils": "npm:^9.0.0"
56865686
async-mutex: "npm:^0.5.0"
56875687
ethereumjs-wallet: "npm:^1.0.1"
56885688
immer: "npm:^9.0.6"
5689-
checksum: 10/59eee1451c6bb8ea9dfee3249a49b2092464d1759ce7a0bb3c052a18c979a49d8e410bbece78d9659ecb80efb26f7cf7e3163314efc476bd39c1912bbb63b7b8
5689+
checksum: 10/441bd01b42658819281ef45ebfb214d3282db2cf523284314ca8e6786e7c06fa564a6799e506edc67153d2b51126a8aca3b67762bbb5221c382f70c81a81db29
56905690
languageName: node
56915691
linkType: hard
56925692

@@ -5711,18 +5711,18 @@ __metadata:
57115711
languageName: node
57125712
linkType: hard
57135713

5714-
"@metamask/message-manager@npm:^10.0.0":
5715-
version: 10.0.0
5716-
resolution: "@metamask/message-manager@npm:10.0.0"
5714+
"@metamask/message-manager@npm:^10.0.1":
5715+
version: 10.0.1
5716+
resolution: "@metamask/message-manager@npm:10.0.1"
57175717
dependencies:
5718-
"@metamask/base-controller": "npm:^6.0.0"
5719-
"@metamask/controller-utils": "npm:^11.0.0"
5718+
"@metamask/base-controller": "npm:^6.0.1"
5719+
"@metamask/controller-utils": "npm:^11.0.1"
57205720
"@metamask/eth-sig-util": "npm:^7.0.1"
5721-
"@metamask/utils": "npm:^8.3.0"
5721+
"@metamask/utils": "npm:^9.0.0"
57225722
"@types/uuid": "npm:^8.3.0"
57235723
jsonschema: "npm:^1.2.4"
57245724
uuid: "npm:^8.3.2"
5725-
checksum: 10/ad0d3e92693c668d6959a0ad6a7262923b892206adb22137ff002e50b1156f687afb007c6f983767e8efbb04f34e4563d6290c7240f03100731b62a8c0a55795
5725+
checksum: 10/732909c19027ceb84eec66c57d1dd37193b17394768c902562d2022cde29cfc913e959228caaf751996318edabd090029bb5d7f46dc4be16a0cb655b91172931
57265726
languageName: node
57275727
linkType: hard
57285728

@@ -25296,7 +25296,7 @@ __metadata:
2529625296
"@metamask/gas-fee-controller": "npm:^18.0.0"
2529725297
"@metamask/jazzicon": "npm:^2.0.0"
2529825298
"@metamask/keyring-api": "npm:^8.0.0"
25299-
"@metamask/keyring-controller": "npm:^16.1.0"
25299+
"@metamask/keyring-controller": "npm:^17.1.1"
2530025300
"@metamask/logging-controller": "npm:^3.0.1"
2530125301
"@metamask/logo": "npm:^3.1.2"
2530225302
"@metamask/message-manager": "npm:^7.3.0"

0 commit comments

Comments
 (0)