From 8cb8395ceb376beaed9113361fbe0535fa257062 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Thu, 25 Jun 2020 15:31:30 -0400 Subject: [PATCH] feat: check password before saving a new account Also shows the error if there is one when adding. Should show a loading state too, but it doesn't work because the Nim function freezes the QML --- src/app/wallet/view.nim | 16 +++---- src/status/libstatus/accounts.nim | 21 ++++++++-- src/status/libstatus/libstatus.nim | 2 + src/status/wallet.nim | 42 ++++++++++++------- .../components/AddAccountWithPrivateKey.qml | 26 +++++++++--- .../Wallet/components/AddAccountWithSeed.qml | 26 +++++++++--- .../Wallet/components/AddWatchOnlyAccount.qml | 26 +++++++++--- .../components/GenerateAccountModal.qml | 26 +++++++++--- ui/onboarding/Login.qml | 2 +- 9 files changed, 140 insertions(+), 47 deletions(-) diff --git a/src/app/wallet/view.nim b/src/app/wallet/view.nim index da5a9e34d2..f4f7a388be 100644 --- a/src/app/wallet/view.nim +++ b/src/app/wallet/view.nim @@ -121,17 +121,17 @@ QtObject: self.setCurrentAccountByIndex(0) self.accountListChanged() - proc generateNewAccount*(self: WalletView, password: string, accountName: string, color: string) {.slot.} = - self.status.wallet.generateNewAccount(password, accountName, color) + proc generateNewAccount*(self: WalletView, password: string, accountName: string, color: string): string {.slot.} = + result = self.status.wallet.generateNewAccount(password, accountName, color) - proc addAccountsFromSeed*(self: WalletView, seed: string, password: string, accountName: string, color: string) {.slot.} = - self.status.wallet.addAccountsFromSeed(seed, password, accountName, color) + proc addAccountsFromSeed*(self: WalletView, seed: string, password: string, accountName: string, color: string): string {.slot.} = + result = self.status.wallet.addAccountsFromSeed(seed, password, accountName, color) - proc addAccountsFromPrivateKey*(self: WalletView, privateKey: string, password: string, accountName: string, color: string) {.slot.} = - self.status.wallet.addAccountsFromPrivateKey(privateKey, password, accountName, color) + proc addAccountsFromPrivateKey*(self: WalletView, privateKey: string, password: string, accountName: string, color: string): string {.slot.} = + result = self.status.wallet.addAccountsFromPrivateKey(privateKey, password, accountName, color) - proc addWatchOnlyAccount*(self: WalletView, address: string, accountName: string, color: string) {.slot.} = - self.status.wallet.addWatchOnlyAccount(address, accountName, color) + proc addWatchOnlyAccount*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} = + result = self.status.wallet.addWatchOnlyAccount(address, accountName, color) proc changeAccountSettings*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} = result = self.status.wallet.changeAccountSettings(address, accountName, color) diff --git a/src/status/libstatus/accounts.nim b/src/status/libstatus/accounts.nim index b44e26b748..eef7af73ea 100644 --- a/src/status/libstatus/accounts.nim +++ b/src/status/libstatus/accounts.nim @@ -7,7 +7,10 @@ import accounts/constants import ../../signals/types as signal_types import ../wallet/account -proc queryAccounts*(): string = +proc hashPassword(password: string): string = + result = "0x" & $keccak_256.digest(password) + +proc getDefaultAccount*(): string = var response = callPrivateRPC("eth_accounts") result = parseJson(response)["result"][0].getStr() @@ -49,7 +52,7 @@ proc saveAccountAndLogin*( password: string, configJSON: string, settingsJSON: string): types.Account = - let hashedPassword = "0x" & $keccak_256.digest(password) + let hashedPassword = hashPassword(password) let subaccountData = %* [ { "public-key": account.derived.defaultWallet.publicKey, @@ -81,7 +84,7 @@ proc saveAccountAndLogin*( raise newException(StatusGoException, "Error saving account and logging in: " & error) proc storeDerivedAccounts*(account: GeneratedAccount, password: string): MultiAccounts = - let hashedPassword = "0x" & $keccak_256.digest(password) + let hashedPassword = hashPassword(password) let multiAccount = %* { "accountID": account.id, "paths": [PATH_WALLET_ROOT, PATH_EIP_1581, PATH_WHISPER, PATH_DEFAULT_WALLET], @@ -150,7 +153,7 @@ proc setupAccount*(account: GeneratedAccount, password: string): types.Account = discard libstatus.addPeer(peer) proc login*(nodeAccount: NodeAccount, password: string): NodeAccount = - let hashedPassword = "0x" & $keccak_256.digest(password) + let hashedPassword = hashPassword(password) let account = nodeAccount.toAccount let loginResult = $libstatus.login($toJson(account), hashedPassword) let error = parseJson(loginResult)["error"].getStr @@ -162,6 +165,16 @@ proc login*(nodeAccount: NodeAccount, password: string): NodeAccount = raise newException(StatusGoException, "Error logging in: " & error) +proc verifyAccountPassword*(address: string, password: string): bool = + let hashedPassword = hashPassword(password) + let verifyResult = $libstatus.verifyAccountPassword(KEYSTOREDIR, address, hashedPassword) + let error = parseJson(verifyResult)["error"].getStr + + if error == "": + return true + + return false + proc multiAccountImportMnemonic*(mnemonic: string): GeneratedAccount = let mnemonicJson = %* { "mnemonicPhrase": mnemonic, diff --git a/src/status/libstatus/libstatus.nim b/src/status/libstatus/libstatus.nim index b79b1e1db8..2e851c9ac8 100644 --- a/src/status/libstatus/libstatus.nim +++ b/src/status/libstatus/libstatus.nim @@ -36,4 +36,6 @@ proc login*(acctData: cstring, password: cstring): cstring {.importc: "Login".} proc logout*(): cstring {.importc: "Logout".} +proc verifyAccountPassword*(keyStoreDir: cstring, address: cstring, password: cstring): cstring {.importc: "VerifyAccountPassword".} + proc validateMnemonic*(mnemonic: cstring): cstring {.importc: "ValidateMnemonic".} diff --git a/src/status/wallet.nim b/src/status/wallet.nim index 0e13599241..40252a891f 100644 --- a/src/status/wallet.nim +++ b/src/status/wallet.nim @@ -97,30 +97,44 @@ proc calculateTotalFiatBalance*(self: WalletModel) = for account in self.accounts: self.totalBalance += account.realFiatBalance -proc addNewGeneratedAccount(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true) = - generatedAccount.name = accountName - var derivedAccount: DerivedAccount = status_accounts.saveAccount(generatedAccount, password, color, accountType, isADerivedAccount) - var account = self.newAccount(accountName, derivedAccount.address, color, fmt"0.00 {self.defaultCurrency}", derivedAccount.publicKey) - self.accounts.add(account) - self.events.emit("newAccountAdded", AccountArgs(account: account)) +proc addNewGeneratedAccount(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true): string = + try: + generatedAccount.name = accountName + var derivedAccount: DerivedAccount = status_accounts.saveAccount(generatedAccount, password, color, accountType, isADerivedAccount) + var account = self.newAccount(accountName, derivedAccount.address, color, fmt"0.00 {self.defaultCurrency}", derivedAccount.publicKey) + self.accounts.add(account) + self.events.emit("newAccountAdded", AccountArgs(account: account)) + except Exception as e: + return fmt"Error adding new account: {e.msg}" -proc generateNewAccount*(self: WalletModel, password: string, accountName: string, color: string) = + return "" + +proc addNewGeneratedAccountWithPassword(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true): string = + let defaultAccount = status_accounts.getDefaultAccount() + let isPasswordOk = status_accounts.verifyAccountPassword(defaultAccount, password) + + if (not isPasswordOk): + return "Wrong password" + + result = self.addNewGeneratedAccount(generatedAccount, password, accountName, color, accountType, isADerivedAccount) + +proc generateNewAccount*(self: WalletModel, password: string, accountName: string, color: string): string = let accounts = status_accounts.generateAddresses(1) let generatedAccount = accounts[0] - self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.GENERATED) + return self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.GENERATED) -proc addAccountsFromSeed*(self: WalletModel, seed: string, password: string, accountName: string, color: string) = +proc addAccountsFromSeed*(self: WalletModel, seed: string, password: string, accountName: string, color: string): string = let mnemonic = replace(seed, ',', ' ') let generatedAccount = status_accounts.multiAccountImportMnemonic(mnemonic) - self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.SEED) + return self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.SEED) -proc addAccountsFromPrivateKey*(self: WalletModel, privateKey: string, password: string, accountName: string, color: string) = +proc addAccountsFromPrivateKey*(self: WalletModel, privateKey: string, password: string, accountName: string, color: string): string = let generatedAccount = status_accounts.MultiAccountImportPrivateKey(privateKey) - self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.KEY, false) + return self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.KEY, false) -proc addWatchOnlyAccount*(self: WalletModel, address: string, accountName: string, color: string) = +proc addWatchOnlyAccount*(self: WalletModel, address: string, accountName: string, color: string): string = let account = GeneratedAccount(address: address) - self.addNewGeneratedAccount(account, "", accountName, color, constants.WATCH, false) + return self.addNewGeneratedAccount(account, "", accountName, color, constants.WATCH, false) proc hasAsset*(self: WalletModel, account: string, symbol: string): bool = self.tokens.anyIt(it["symbol"].getStr == symbol) diff --git a/ui/app/AppLayouts/Wallet/components/AddAccountWithPrivateKey.qml b/ui/app/AppLayouts/Wallet/components/AddAccountWithPrivateKey.qml index 7545f0b579..e3e638c10b 100644 --- a/ui/app/AppLayouts/Wallet/components/AddAccountWithPrivateKey.qml +++ b/ui/app/AppLayouts/Wallet/components/AddAccountWithPrivateKey.qml @@ -1,4 +1,5 @@ import QtQuick 2.13 +import QtQuick.Dialogs 1.3 import "../../../../imports" import "../../../../shared" @@ -12,6 +13,7 @@ ModalPopup { property string passwordValidationError: "" property string privateKeyValidationError: "" property string accountNameValidationError: "" + property bool loading: false function validate() { if (passwordInput.text === "") { @@ -94,17 +96,31 @@ ModalPopup { anchors.top: parent.top anchors.right: parent.right anchors.rightMargin: Theme.padding - label: qsTr("Add account >") + label: loading ? qsTr("Loading...") : qsTr("Add account >") - disabled: passwordInput.text === "" || accountNameInput.text === "" || accountPKeyInput.text === "" + disabled: loading || passwordInput.text === "" || accountNameInput.text === "" || accountPKeyInput.text === "" + + MessageDialog { + id: accountError + title: "Adding the account failed" + icon: StandardIcon.Critical + standardButtons: StandardButton.Ok + } onClicked : { + // TODO the loaidng doesn't work because the function freezes th eview. Might need to use threads + loading = true if (!validate()) { - return + return loading = false + } + + const error = walletModel.addAccountsFromPrivateKey(accountPKeyInput.text, passwordInput.text, accountNameInput.text, selectedColor) + loading = false + if (error) { + accountError.text = error + return accountError.open() } - walletModel.addAccountsFromPrivateKey(accountPKeyInput.text, passwordInput.text, accountNameInput.text, selectedColor) - // TODO manage errors adding account popup.close(); } } diff --git a/ui/app/AppLayouts/Wallet/components/AddAccountWithSeed.qml b/ui/app/AppLayouts/Wallet/components/AddAccountWithSeed.qml index 2045925262..edd6dd4271 100644 --- a/ui/app/AppLayouts/Wallet/components/AddAccountWithSeed.qml +++ b/ui/app/AppLayouts/Wallet/components/AddAccountWithSeed.qml @@ -1,4 +1,5 @@ import QtQuick 2.13 +import QtQuick.Dialogs 1.3 import "../../../../imports" import "../../../../shared" @@ -11,6 +12,7 @@ ModalPopup { property string passwordValidationError: "" property string seedValidationError: "" property string accountNameValidationError: "" + property bool loading: false function validate() { if (passwordInput.text === "") { @@ -95,17 +97,31 @@ ModalPopup { anchors.top: parent.top anchors.right: parent.right anchors.rightMargin: Theme.padding - label: "Add account >" + label: loading ? qsTr("Loading...") : qsTr("Add account >") - disabled: passwordInput.text === "" || accountNameInput.text === "" || accountSeedInput.text === "" + disabled: loading || passwordInput.text === "" || accountNameInput.text === "" || accountSeedInput.text === "" + + MessageDialog { + id: accountError + title: "Adding the account failed" + icon: StandardIcon.Critical + standardButtons: StandardButton.Ok + } onClicked : { + // TODO the loaidng doesn't work because the function freezes th eview. Might need to use threads + loading = true if (!validate()) { - return + return loading = false + } + + const error = walletModel.addAccountsFromSeed(accountSeedInput.text, passwordInput.text, accountNameInput.text, selectedColor) + loading = false + if (error) { + accountError.text = error + return accountError.open() } - walletModel.addAccountsFromSeed(accountSeedInput.text, passwordInput.text, accountNameInput.text, selectedColor) - // TODO manage errors adding account popup.close(); } } diff --git a/ui/app/AppLayouts/Wallet/components/AddWatchOnlyAccount.qml b/ui/app/AppLayouts/Wallet/components/AddWatchOnlyAccount.qml index 5b78dceea2..885f8eea4b 100644 --- a/ui/app/AppLayouts/Wallet/components/AddWatchOnlyAccount.qml +++ b/ui/app/AppLayouts/Wallet/components/AddWatchOnlyAccount.qml @@ -1,4 +1,5 @@ import QtQuick 2.13 +import QtQuick.Dialogs 1.3 import "../../../../imports" import "../../../../shared" @@ -10,6 +11,7 @@ ModalPopup { property string selectedColor: Constants.accountColors[0] property string addressError: "" property string accountNameValidationError: "" + property bool loading: false function validate() { if (addressInput.text === "") { @@ -73,17 +75,31 @@ ModalPopup { anchors.top: parent.top anchors.right: parent.right anchors.rightMargin: Theme.padding - label: "Add account >" + label: loading ? qsTr("Loading...") : qsTr("Add account >") - disabled: addressInput.text === "" || accountNameInput.text === "" + disabled: loading || addressInput.text === "" || accountNameInput.text === "" + + MessageDialog { + id: accountError + title: "Adding the account failed" + icon: StandardIcon.Critical + standardButtons: StandardButton.Ok + } onClicked : { + // TODO the loaidng doesn't work because the function freezes th eview. Might need to use threads + loading = true if (!validate()) { - return + return loading = false + } + + const error = walletModel.addWatchOnlyAccount(addressInput.text, accountNameInput.text, selectedColor); + loading = false + if (error) { + accountError.text = error + return accountError.open() } - walletModel.addWatchOnlyAccount(addressInput.text, accountNameInput.text, selectedColor); - // TODO manage errors adding account popup.close(); } } diff --git a/ui/app/AppLayouts/Wallet/components/GenerateAccountModal.qml b/ui/app/AppLayouts/Wallet/components/GenerateAccountModal.qml index f551c4eb66..6abb9a5a52 100644 --- a/ui/app/AppLayouts/Wallet/components/GenerateAccountModal.qml +++ b/ui/app/AppLayouts/Wallet/components/GenerateAccountModal.qml @@ -1,4 +1,5 @@ import QtQuick 2.13 +import QtQuick.Dialogs 1.3 import "../../../../imports" import "../../../../shared" @@ -10,6 +11,7 @@ ModalPopup { property string selectedColor: Constants.accountColors[0] property string passwordValidationError: "" property string accountNameValidationError: "" + property bool loading: false function validate() { if (passwordInput.text === "") { @@ -73,17 +75,31 @@ ModalPopup { anchors.top: parent.top anchors.right: parent.right anchors.rightMargin: Theme.padding - label: "Add account >" + label: loading ? qsTr("Loading...") : qsTr("Add account >") - disabled: passwordInput.text === "" || accountNameInput.text === "" + disabled: loading || passwordInput.text === "" || accountNameInput.text === "" + + MessageDialog { + id: accountError + title: "Adding the account failed" + icon: StandardIcon.Critical + standardButtons: StandardButton.Ok + } onClicked : { + // TODO the loaidng doesn't work because the function freezes th eview. Might need to use threads + loading = true if (!validate()) { - return + return loading = false + } + + const error = walletModel.generateNewAccount(passwordInput.text, accountNameInput.text, selectedColor) + loading = false + if (error) { + accountError.text = error + return accountError.open() } - walletModel.generateNewAccount(passwordInput.text, accountNameInput.text, selectedColor); - // TODO manage errors adding account popup.close(); } } diff --git a/ui/onboarding/Login.qml b/ui/onboarding/Login.qml index e49c9bcbd7..06526ad9f9 100644 --- a/ui/onboarding/Login.qml +++ b/ui/onboarding/Login.qml @@ -149,7 +149,6 @@ Item { SVGImage { anchors.horizontalCenter: parent.horizontalCenter anchors.verticalCenter: parent.verticalCenter - // TODO replace by a real loading image source: "../app/img/arrowUp.svg" width: 13.5 height: 17.5 @@ -176,6 +175,7 @@ Item { anchors.left: txtPassword.right anchors.leftMargin: Theme.padding anchors.verticalCenter: txtPassword.verticalCenter + // TODO replace by a real loading image source: "../app/img/settings.svg" width: 30 height: 30