fix: prevent crash on generate account wrong password
Fixes #2448. Currently, if a wrong password is entered when generating a wallet account, the app will crash due to attempting to decode a `GeneratedAccount ` from an rpc response containing only an error. With this PR, we are detecting if an error is returned in the response, and if so, raising a StatusGoException. This exception is caught in the call chain, and translated in to a `StatusGoError` which is serialised and sent to the QML view, where it is parsed and displayed as an invalid password error in the input box. refactor: remove string return values as error messages in wallet model In the wallet model, we were passing back empty strings for no error, or an error as a string. This is not only confusing, but does not benefit from leaning on the compiler and strong types. One has to read the entire code to understand if a string result is returned when there is no error instead of implicitly being able to understand there is no return type. To alleviate this, account creation fundtions that do not need to return a value have been changed to a void return type, and raise `StatusGoException` if there is an error encountered. This can be caught in the call chain and used as necessary (ie to pass to QML). refactor: move invalid password string detection to Utils Currently, we are reading returned view model values and checking to see if they include a known string from Status Go that means there was an invalid password used. This string was placed in the codebased in mulitple locations. This PR moves the string check to a Utils function and updates all the references to use the function in Utils.
This commit is contained in:
parent
4cfe7411ac
commit
ee1287b71d
|
@ -11,6 +11,7 @@ import ../../status/libstatus/eth/contracts
|
||||||
import ../../status/ens as status_ens
|
import ../../status/ens as status_ens
|
||||||
import views/[asset_list, account_list, account_item, token_list, transaction_list, collectibles_list]
|
import views/[asset_list, account_list, account_item, token_list, transaction_list, collectibles_list]
|
||||||
import ../../status/tasks/[qt, task_runner_impl]
|
import ../../status/tasks/[qt, task_runner_impl]
|
||||||
|
import ../../status/signals/types as signal_types
|
||||||
|
|
||||||
type
|
type
|
||||||
SendTransactionTaskArg = ref object of QObjectTaskArg
|
SendTransactionTaskArg = ref object of QObjectTaskArg
|
||||||
|
@ -390,16 +391,25 @@ QtObject:
|
||||||
result = fmt"{ethValue}"
|
result = fmt"{ethValue}"
|
||||||
|
|
||||||
proc generateNewAccount*(self: WalletView, password: string, accountName: string, color: string): string {.slot.} =
|
proc generateNewAccount*(self: WalletView, password: string, accountName: string, color: string): string {.slot.} =
|
||||||
result = self.status.wallet.generateNewAccount(password, accountName, color)
|
try:
|
||||||
|
self.status.wallet.generateNewAccount(password, accountName, color)
|
||||||
|
except StatusGoException as e:
|
||||||
|
result = StatusGoError(error: e.msg).toJson
|
||||||
|
|
||||||
proc addAccountsFromSeed*(self: WalletView, seed: string, password: string, accountName: string, color: string): string {.slot.} =
|
proc addAccountsFromSeed*(self: WalletView, seed: string, password: string, accountName: string, color: string): string {.slot.} =
|
||||||
result = self.status.wallet.addAccountsFromSeed(seed.strip(), password, accountName, color)
|
try:
|
||||||
|
self.status.wallet.addAccountsFromSeed(seed.strip(), password, accountName, color)
|
||||||
|
except StatusGoException as e:
|
||||||
|
result = StatusGoError(error: e.msg).toJson
|
||||||
|
|
||||||
proc addAccountsFromPrivateKey*(self: WalletView, privateKey: string, password: string, accountName: string, color: string): string {.slot.} =
|
proc addAccountsFromPrivateKey*(self: WalletView, privateKey: string, password: string, accountName: string, color: string): string {.slot.} =
|
||||||
result = self.status.wallet.addAccountsFromPrivateKey(privateKey, password, accountName, color)
|
try:
|
||||||
|
self.status.wallet.addAccountsFromPrivateKey(privateKey, password, accountName, color)
|
||||||
|
except StatusGoException as e:
|
||||||
|
result = StatusGoError(error: e.msg).toJson
|
||||||
|
|
||||||
proc addWatchOnlyAccount*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} =
|
proc addWatchOnlyAccount*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} =
|
||||||
result = self.status.wallet.addWatchOnlyAccount(address, accountName, color)
|
self.status.wallet.addWatchOnlyAccount(address, accountName, color)
|
||||||
|
|
||||||
proc changeAccountSettings*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} =
|
proc changeAccountSettings*(self: WalletView, address: string, accountName: string, color: string): string {.slot.} =
|
||||||
result = self.status.wallet.changeAccountSettings(address, accountName, color)
|
result = self.status.wallet.changeAccountSettings(address, accountName, color)
|
||||||
|
|
|
@ -218,7 +218,15 @@ proc loadAccount*(address: string, password: string): GeneratedAccount =
|
||||||
"password": hashedPassword
|
"password": hashedPassword
|
||||||
}
|
}
|
||||||
let loadResult = $status_go.multiAccountLoadAccount($inputJson)
|
let loadResult = $status_go.multiAccountLoadAccount($inputJson)
|
||||||
|
let parsedLoadResult = loadResult.parseJson
|
||||||
|
let error = parsedLoadResult{"error"}.getStr
|
||||||
|
|
||||||
|
if error == "":
|
||||||
|
debug "Account loaded succesfully"
|
||||||
result = Json.decode(loadResult, GeneratedAccount)
|
result = Json.decode(loadResult, GeneratedAccount)
|
||||||
|
return
|
||||||
|
|
||||||
|
raise newException(StatusGoException, "Error loading wallet account: " & error)
|
||||||
|
|
||||||
proc verifyAccountPassword*(address: string, password: string): bool =
|
proc verifyAccountPassword*(address: string, password: string): bool =
|
||||||
let hashedPassword = hashPassword(password)
|
let hashedPassword = hashPassword(password)
|
||||||
|
|
|
@ -238,7 +238,7 @@ proc calculateTotalFiatBalance*(self: WalletModel) =
|
||||||
if account.realFiatBalance.isSome:
|
if account.realFiatBalance.isSome:
|
||||||
self.totalBalance += account.realFiatBalance.get()
|
self.totalBalance += account.realFiatBalance.get()
|
||||||
|
|
||||||
proc addNewGeneratedAccount(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true, walletIndex: int = 0): string =
|
proc addNewGeneratedAccount(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true, walletIndex: int = 0) =
|
||||||
try:
|
try:
|
||||||
generatedAccount.name = accountName
|
generatedAccount.name = accountName
|
||||||
var derivedAccount: DerivedAccount = status_accounts.saveAccount(generatedAccount, password, color, accountType, isADerivedAccount, walletIndex)
|
var derivedAccount: DerivedAccount = status_accounts.saveAccount(generatedAccount, password, color, accountType, isADerivedAccount, walletIndex)
|
||||||
|
@ -246,49 +246,55 @@ proc addNewGeneratedAccount(self: WalletModel, generatedAccount: GeneratedAccoun
|
||||||
self.accounts.add(account)
|
self.accounts.add(account)
|
||||||
self.events.emit("newAccountAdded", AccountArgs(account: account))
|
self.events.emit("newAccountAdded", AccountArgs(account: account))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return fmt"Error adding new account: {e.msg}"
|
raise newException(StatusGoException, fmt"Error adding new account: {e.msg}")
|
||||||
|
|
||||||
return ""
|
proc generateNewAccount*(self: WalletModel, password: string, accountName: string, color: string) =
|
||||||
|
let
|
||||||
proc addNewGeneratedAccountWithPassword(self: WalletModel, generatedAccount: GeneratedAccount, password: string, accountName: string, color: string, accountType: string, isADerivedAccount = true, walletIndex: int = 0): string =
|
walletRootAddress = status_settings.getSetting[string](Setting.WalletRootAddress, "")
|
||||||
let defaultAccount = status_accounts.getDefaultAccount()
|
walletIndex = status_settings.getSetting[int](Setting.LatestDerivedPath) + 1
|
||||||
let isPasswordOk = status_accounts.verifyAccountPassword(defaultAccount, password)
|
loadedAccount = status_accounts.loadAccount(walletRootAddress, password)
|
||||||
|
derivedAccount = status_accounts.deriveWallet(loadedAccount.id, walletIndex)
|
||||||
if (not isPasswordOk):
|
generatedAccount = GeneratedAccount(
|
||||||
return "Wrong password"
|
|
||||||
result = self.addNewGeneratedAccount(generatedAccount, password, accountName, color, accountType, isADerivedAccount, walletIndex)
|
|
||||||
|
|
||||||
proc generateNewAccount*(self: WalletModel, password: string, accountName: string, color: string): string =
|
|
||||||
let walletRootAddress = status_settings.getSetting[string](Setting.WalletRootAddress, "")
|
|
||||||
let walletIndex = status_settings.getSetting[int](Setting.LatestDerivedPath) + 1
|
|
||||||
let loadedAccount = status_accounts.loadAccount(walletRootAddress, password)
|
|
||||||
let derivedAccount = status_accounts.deriveWallet(loadedAccount.id, walletIndex)
|
|
||||||
|
|
||||||
let generatedAccount = GeneratedAccount(
|
|
||||||
id: loadedAccount.id,
|
id: loadedAccount.id,
|
||||||
publicKey: derivedAccount.publicKey,
|
publicKey: derivedAccount.publicKey,
|
||||||
address: derivedAccount.address
|
address: derivedAccount.address
|
||||||
)
|
)
|
||||||
|
|
||||||
result = self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.GENERATED, true, walletIndex)
|
# if we've gotten here, the password is ok (loadAccount requires a valid password)
|
||||||
|
# so no need to check for a valid password
|
||||||
|
self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.GENERATED, true, walletIndex)
|
||||||
|
|
||||||
let statusGoResult = status_settings.saveSetting(Setting.LatestDerivedPath, $walletIndex)
|
let statusGoResult = status_settings.saveSetting(Setting.LatestDerivedPath, $walletIndex)
|
||||||
if statusGoResult.error != "":
|
if statusGoResult.error != "":
|
||||||
error "Error storing the latest wallet index", msg=statusGoResult.error
|
error "Error storing the latest wallet index", msg=statusGoResult.error
|
||||||
|
|
||||||
proc addAccountsFromSeed*(self: WalletModel, seed: string, password: string, accountName: string, color: string): string =
|
proc addAccountsFromSeed*(self: WalletModel, seed: string, password: string, accountName: string, color: string) =
|
||||||
let mnemonic = replace(seed, ',', ' ')
|
let mnemonic = replace(seed, ',', ' ')
|
||||||
var generatedAccount = status_accounts.multiAccountImportMnemonic(mnemonic)
|
var generatedAccount = status_accounts.multiAccountImportMnemonic(mnemonic)
|
||||||
generatedAccount.derived = status_accounts.deriveAccounts(generatedAccount.id)
|
generatedAccount.derived = status_accounts.deriveAccounts(generatedAccount.id)
|
||||||
return self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.SEED)
|
|
||||||
|
|
||||||
proc addAccountsFromPrivateKey*(self: WalletModel, privateKey: string, password: string, accountName: string, color: string): string =
|
let
|
||||||
let generatedAccount = status_accounts.MultiAccountImportPrivateKey(privateKey)
|
defaultAccount = status_accounts.getDefaultAccount()
|
||||||
return self.addNewGeneratedAccountWithPassword(generatedAccount, password, accountName, color, constants.KEY, false)
|
isPasswordOk = status_accounts.verifyAccountPassword(defaultAccount, password)
|
||||||
|
if not isPasswordOk:
|
||||||
|
raise newException(StatusGoException, "Error generating new account: invalid password")
|
||||||
|
|
||||||
proc addWatchOnlyAccount*(self: WalletModel, address: string, accountName: string, color: string): string =
|
self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.SEED)
|
||||||
|
|
||||||
|
proc addAccountsFromPrivateKey*(self: WalletModel, privateKey: string, password: string, accountName: string, color: string) =
|
||||||
|
let
|
||||||
|
generatedAccount = status_accounts.MultiAccountImportPrivateKey(privateKey)
|
||||||
|
defaultAccount = status_accounts.getDefaultAccount()
|
||||||
|
isPasswordOk = status_accounts.verifyAccountPassword(defaultAccount, password)
|
||||||
|
|
||||||
|
if not isPasswordOk:
|
||||||
|
raise newException(StatusGoException, "Error generating new account: invalid password")
|
||||||
|
|
||||||
|
self.addNewGeneratedAccount(generatedAccount, password, accountName, color, constants.KEY, false)
|
||||||
|
|
||||||
|
proc addWatchOnlyAccount*(self: WalletModel, address: string, accountName: string, color: string) =
|
||||||
let account = GeneratedAccount(address: address)
|
let account = GeneratedAccount(address: address)
|
||||||
return self.addNewGeneratedAccount(account, "", accountName, color, constants.WATCH, false)
|
self.addNewGeneratedAccount(account, "", accountName, color, constants.WATCH, false)
|
||||||
|
|
||||||
proc hasAsset*(self: WalletModel, symbol: string): bool =
|
proc hasAsset*(self: WalletModel, symbol: string): bool =
|
||||||
self.tokens.anyIt(it.symbol == symbol)
|
self.tokens.anyIt(it.symbol == symbol)
|
||||||
|
|
|
@ -215,7 +215,7 @@ property Component sendTransactionModalComponent: SignTransactionModal {}
|
||||||
toastMessage.link = `${_walletModel.etherscanLink}/${responseObj.result.result}`
|
toastMessage.link = `${_walletModel.etherscanLink}/${responseObj.result.result}`
|
||||||
toastMessage.open()
|
toastMessage.open()
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (e.message.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(e.message)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
sendDialog.transactionSigner.validationError = qsTrId("wrong-password")
|
sendDialog.transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
@ -256,7 +256,7 @@ property Component sendTransactionModalComponent: SignTransactionModal {}
|
||||||
throw new Error(responseObj.error)
|
throw new Error(responseObj.error)
|
||||||
}
|
}
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (e.message.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(e.message)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
signDialog.transactionSigner.validationError = qsTrId("wrong-password")
|
signDialog.transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
|
@ -283,7 +283,7 @@ ModalPopup {
|
||||||
stack.currentGroup.isPending = false
|
stack.currentGroup.isPending = false
|
||||||
|
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
if (response.result.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(response.result)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
transactionSigner.validationError = qsTrId("wrong-password")
|
transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
|
@ -32,7 +32,7 @@ ModalPopup {
|
||||||
let response = JSON.parse(responseStr)
|
let response = JSON.parse(responseStr)
|
||||||
|
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
if (response.result.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(response.result)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
transactionSigner.validationError = qsTrId("wrong-password")
|
transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
|
@ -32,7 +32,7 @@ ModalPopup {
|
||||||
let response = JSON.parse(responseStr)
|
let response = JSON.parse(responseStr)
|
||||||
|
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
if (response.error.message.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(response.error.message)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
transactionSigner.validationError = qsTrId("wrong-password")
|
transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
|
@ -231,7 +231,7 @@ ModalPopup {
|
||||||
stack.currentGroup.isPending = false
|
stack.currentGroup.isPending = false
|
||||||
|
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
if (response.result.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(response.result)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
transactionSigner.validationError = qsTrId("wrong-password")
|
transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
|
@ -126,13 +126,20 @@ ModalPopup {
|
||||||
return loading = false
|
return loading = false
|
||||||
}
|
}
|
||||||
|
|
||||||
const error = walletModel.addAccountsFromPrivateKey(accountPKeyInput.text, passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
const result = walletModel.addAccountsFromPrivateKey(accountPKeyInput.text, passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
||||||
|
|
||||||
loading = false
|
loading = false
|
||||||
if (error) {
|
if (result) {
|
||||||
errorSound.play()
|
let resultJson = JSON.parse(result);
|
||||||
accountError.text = error
|
errorSound.play();
|
||||||
return accountError.open()
|
if (Utils.isInvalidPasswordMessage(resultJson.error)) {
|
||||||
|
//% "Wrong password"
|
||||||
|
popup.passwordValidationError = qsTrId("wrong-password")
|
||||||
|
} else {
|
||||||
|
accountError.text = resultJson.error
|
||||||
|
accountError.open()
|
||||||
|
}
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
popup.close();
|
popup.close();
|
||||||
|
|
|
@ -128,12 +128,19 @@ ModalPopup {
|
||||||
return loading = false
|
return loading = false
|
||||||
}
|
}
|
||||||
|
|
||||||
const error = walletModel.addAccountsFromSeed(seedPhraseTextArea.textArea.text, passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
const result = walletModel.addAccountsFromSeed(seedPhraseTextArea.textArea.text, passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
||||||
loading = false
|
loading = false
|
||||||
if (error) {
|
if (result) {
|
||||||
errorSound.play()
|
let resultJson = JSON.parse(result);
|
||||||
accountError.text = error
|
errorSound.play();
|
||||||
return accountError.open()
|
if (Utils.isInvalidPasswordMessage(resultJson.error)) {
|
||||||
|
//% "Wrong password"
|
||||||
|
popup.passwordValidationError = qsTrId("wrong-password")
|
||||||
|
} else {
|
||||||
|
accountError.text = resultJson.error
|
||||||
|
accountError.open()
|
||||||
|
}
|
||||||
|
return
|
||||||
}
|
}
|
||||||
popup.reset()
|
popup.reset()
|
||||||
popup.close();
|
popup.close();
|
||||||
|
|
|
@ -100,12 +100,19 @@ ModalPopup {
|
||||||
return loading = false
|
return loading = false
|
||||||
}
|
}
|
||||||
|
|
||||||
const error = walletModel.generateNewAccount(passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
const result = walletModel.generateNewAccount(passwordInput.text, accountNameInput.text, accountColorInput.selectedColor)
|
||||||
loading = false
|
loading = false
|
||||||
if (error) {
|
if (result) {
|
||||||
errorSound.play()
|
let resultJson = JSON.parse(result);
|
||||||
accountError.text = error
|
errorSound.play();
|
||||||
return accountError.open()
|
if (Utils.isInvalidPasswordMessage(resultJson.error)) {
|
||||||
|
//% "Wrong password"
|
||||||
|
popup.passwordValidationError = qsTrId("wrong-password")
|
||||||
|
} else {
|
||||||
|
accountError.text = resultJson.error;
|
||||||
|
accountError.open();
|
||||||
|
}
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
popup.close();
|
popup.close();
|
||||||
|
|
|
@ -538,4 +538,11 @@ QtObject {
|
||||||
function isPunct(c) {
|
function isPunct(c) {
|
||||||
return /(!|\@|#|\$|%|\^|&|\*|\(|\)|_|\+|\||-|=|\\|{|}|[|]|"|;|'|<|>|\?|,|\.|\/)/.test(c)
|
return /(!|\@|#|\$|%|\^|&|\*|\(|\)|_|\+|\||-|=|\\|{|}|[|]|"|;|'|<|>|\?|,|\.|\/)/.test(c)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isInvalidPasswordMessage(msg) {
|
||||||
|
return (
|
||||||
|
msg.includes("could not decrypt key with given password") ||
|
||||||
|
msg.includes("invalid password")
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -38,7 +38,7 @@ ModalPopup {
|
||||||
let response = JSON.parse(responseStr)
|
let response = JSON.parse(responseStr)
|
||||||
|
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
if (response.result.includes("could not decrypt key with given password")){
|
if (Utils.isInvalidPasswordMessage(response.result)){
|
||||||
//% "Wrong password"
|
//% "Wrong password"
|
||||||
transactionSigner.validationError = qsTrId("wrong-password")
|
transactionSigner.validationError = qsTrId("wrong-password")
|
||||||
return
|
return
|
||||||
|
|
Loading…
Reference in New Issue