diff --git a/api/backend_test.go b/api/backend_test.go index 508eec2c9..8648c394d 100644 --- a/api/backend_test.go +++ b/api/backend_test.go @@ -1199,9 +1199,19 @@ func TestLoginAndMigrationsStillWorkWithExistingUsers(t *testing.T) { } func TestChangeDatabasePassword(t *testing.T) { + oldPassword := "password" + newPassword := "newPassword" + backend := NewGethStatusBackend() backend.UpdateRootDataDir(t.TempDir()) + // Setup keystore to test decryption of it + keyStoreDir := t.TempDir() + require.NoError(t, backend.accountManager.InitKeystore(keyStoreDir)) + + _, accountInfo, _, err := backend.accountManager.CreateAccount(oldPassword) + require.NoError(t, err) + account := multiaccounts.Account{ Name: "TestAccount", Timestamp: 1, @@ -1209,11 +1219,8 @@ func TestChangeDatabasePassword(t *testing.T) { KDFIterations: 1, } - oldPassword := "password" - newPassword := "newPassword" - // Initialize accounts DB - err := backend.OpenAccounts() + err = backend.OpenAccounts() require.NoError(t, err) err = backend.SaveAccount(account) require.NoError(t, err) @@ -1234,4 +1241,11 @@ func TestChangeDatabasePassword(t *testing.T) { walletDb, err := sqlite.OpenDB(backend.getWalletDBPath(account.KeyUID), newPassword, account.KDFIterations) require.NoError(t, err) walletDb.Close() + + // Test that keystore can be decrypted with the new password + acc, key, err := backend.accountManager.AddressToDecryptedAccount(accountInfo.WalletAddress, newPassword) + require.NoError(t, err) + require.NotNil(t, acc) + require.NotNil(t, key) + require.Equal(t, acc.Address, key.Address) } diff --git a/api/geth_backend.go b/api/geth_backend.go index cab0aaf8f..7800b4b9b 100644 --- a/api/geth_backend.go +++ b/api/geth_backend.go @@ -858,9 +858,15 @@ func (b *GethStatusBackend) ChangeDatabasePassword(keyUID string, password strin } isCurrentAccount := b.getAppDBPath(keyUID) == internalDbPath + restartNode := func() { if isCurrentAccount { if err != nil { + // TODO https://github.com/status-im/status-go/issues/3906 + // Fix restarting node, as it always fails but the error is ignored + // because UI calls Logout and Quit afterwards. It should not be UI-dependent + // and should be handled gracefully here if it makes sense to run dummy node after + // logout _ = b.startNodeWithAccount(*account, password, nil) } else { _ = b.startNodeWithAccount(*account, newPassword, nil) @@ -876,14 +882,23 @@ func (b *GethStatusBackend) ChangeDatabasePassword(keyUID string, password strin } noLogout := func() {} - err = b.changeWalletDBPassword(account, logout, password, newPassword) + // First change app DB password, because it also reencrypts the keystore, + // otherwise if we call changeWalletDbPassword first and logout, we will fail + // to reencrypt the keystore + err = b.changeAppDBPassword(account, logout, password, newPassword) if err != nil { return err } - // Already logged out - err = b.changeAppDBPassword(account, noLogout, password, newPassword) + // Already logged out but pass a param to decouple the logic for testing + err = b.changeWalletDBPassword(account, noLogout, password, newPassword) if err != nil { + // Revert the password to original + err2 := b.changeAppDBPassword(account, noLogout, newPassword, password) + if err2 != nil { + log.Error("failed to revert app db password", "err", err2) + } + return err }