Skip to content

Commit

Permalink
fix(@desktop/keycard): keycard may be factory reseted during unlock f…
Browse files Browse the repository at this point in the history
…low in some scenarios (onboarding part)

- Unexpected wiping out the data during the unlock flow handled (onboarding part)
- Back button actions fixed part 2/2 (onboarding part)

Fixes: #9183
  • Loading branch information
saledjenic committed Jan 26, 2023
1 parent bf34239 commit b44fc7a
Show file tree
Hide file tree
Showing 27 changed files with 323 additions and 152 deletions.
68 changes: 52 additions & 16 deletions src/app/boot/app_controller.nim
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ logScope:

type
AppController* = ref object of RootObj
storeKeyPair: bool
syncWalletForReplacedKeycard: bool
storeDefaultKeyPair: bool
syncKeycardBasedOnAppWalletState: bool
changedKeycardUids: seq[tuple[oldKcUid: string, newKcUid: string]] # used in case user unlocked keycard during onboarding using seed phrase
statusFoundation: StatusFoundation
notificationsManager*: NotificationsManager

Expand Down Expand Up @@ -108,8 +109,10 @@ proc startupDidLoad*(self: AppController)
proc userLoggedIn*(self: AppController): string
proc logout*(self: AppController)
proc finishAppLoading*(self: AppController)
proc storeKeyPairForNewKeycardUser*(self: AppController)
proc syncWalletAccountsOnLoginForReplacedKeycard*(self: AppController)
proc storeDefaultKeyPairForNewKeycardUser*(self: AppController)
proc syncKeycardBasedOnAppWalletStateAfterLogin*(self: AppController)
proc addToKeycardUidPairsToCheckForAChangeAfterLogin*(self: AppController, oldKeycardUid: string, newKeycardUid: string)
proc removeAllKeycardUidPairsForCheckingForAChangeAfterLogin*(self: AppController)

# Main Module Delegate Interface
proc mainDidLoad*(self: AppController)
Expand All @@ -131,8 +134,8 @@ proc connect(self: AppController) =

proc newAppController*(statusFoundation: StatusFoundation): AppController =
result = AppController()
result.storeKeyPair = false
result.syncWalletForReplacedKeycard = false
result.storeDefaultKeyPair = false
result.syncKeycardBasedOnAppWalletState = false
result.statusFoundation = statusFoundation

# Preparing settings service to be exposed later as global QObject
Expand Down Expand Up @@ -440,8 +443,29 @@ proc buildAndRegisterUserProfile(self: AppController) =

singletonInstance.engine.setRootContextProperty("userProfile", self.userProfileVariant)

############################################################################## store def kc sync with app kc uid
## Onboarding flows sync keycard state after login keypair | (inc. kp store) | update
## `I’m new to Status` -> `Generate new keys` -> na | na | na
## `I’m new to Status` -> `Generate keys for a new Keycard` -> yes | no | no
## `I’m new to Status` -> `Import a seed phrase` -> `Import a seed phrase` -> na | na | na
## `I’m new to Status` -> `Import a seed phrase` -> `Import a seed phrase into a new Keycard` -> yes | no | no
##
## `I already use Status` -> `Scan sync code` -> flow not developed yet
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (fetched) -> no | yes | no
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (unlock via puk, fetched) -> no | yes | no
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (unlock via seed phrase, fetched) -> no | yes | yes (kc details should be fetched and set to db while recovering, that's the reason why)
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (not fetched) -> no | yes | no
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (unlock via puk, not fetched) -> no | yes | no
## `I already use Status` -> `I don’t have other device` -> `Login with Keycard` (unlock via seed phrase, not fetched) -> no | yes | no
## `I already use Status` -> `I don’t have other device` -> `Enter a seed phrase` -> na | na | na
##
## `Login` -> na | na | na
## `Login` -> if card was unlocked via puk -> na | na | na
## `Login` -> if card was unlocked via seed phrase -> no | no | yes
## `Login` -> `Create replacement Keycard with seed phrase` -> no | yes | no (we don't know which kc is replaced if user has more kc for the same kp)
##############################################################################
if singletonInstance.userProfile.getIsKeycardUser():
if self.storeKeyPair:
if self.storeDefaultKeyPair:
let allAccounts = self.walletAccountService.fetchAccounts()
let defaultWalletAccounts = allAccounts.filter(a =>
a.walletType == WalletTypeDefaultStatusAccount and
Expand All @@ -460,18 +484,18 @@ proc buildAndRegisterUserProfile(self: AppController) =
keyUid: loggedInAccount.keyUid)
let keystoreDir = self.accountsService.getKeyStoreDir()
discard self.walletAccountService.addMigratedKeyPair(keyPair, keystoreDir)
if self.syncWalletForReplacedKeycard:
if self.syncKeycardBasedOnAppWalletState:
let allAccounts = self.walletAccountService.fetchAccounts()
let accountsForLoggedInUser = allAccounts.filter(a => a.keyUid == loggedInAccount.keyUid)
var kpDto = KeyPairDto(keycardUid: "",
var keyPair = KeyPairDto(keycardUid: "",
keycardName: displayName,
keycardLocked: false,
accountsAddresses: @[],
keyUid: loggedInAccount.keyUid)
var activeValidPathsToStoreToAKeycard: seq[string]
for acc in accountsForLoggedInUser:
activeValidPathsToStoreToAKeycard.add(acc.path)
kpDto.accountsAddresses.add(acc.address)
keyPair.accountsAddresses.add(acc.address)
self.keycardService.startStoreMetadataFlow(displayName, self.startupModule.getPin(), activeValidPathsToStoreToAKeycard)
## sleep for 3 seconds, since that is more than enough to store metadata to a keycard, if the reader is still plugged in
## and the card is still inserted, otherwise we just skip that.
Expand All @@ -480,14 +504,26 @@ proc buildAndRegisterUserProfile(self: AppController) =
## instance uid for GetMetadata flow we will be able to use SIGNAL_SHARED_KEYCARD_MODULE_TRY_KEYCARD_SYNC signal for syncing
## otherwise we need to handle that way separatelly in `handleKeycardSyncing` of shared module
sleep(3000)
self.keycardService.cancelCurrentFlow()
let (_, kcEvent) = self.keycardService.getLastReceivedKeycardData()
if kcEvent.instanceUID.len > 0:
kpDto.keycardUid = kcEvent.instanceUID
keyPair.keycardUid = kcEvent.instanceUID
let keystoreDir = self.accountsService.getKeyStoreDir()
discard self.walletAccountService.addMigratedKeyPair(kpDto, keystoreDir)
discard self.walletAccountService.addMigratedKeyPair(keyPair, keystoreDir)

proc storeKeyPairForNewKeycardUser*(self: AppController) =
self.storeKeyPair = true
if self.changedKeycardUids.len > 0:
let oldUid = self.changedKeycardUids[0].oldKcUid
let newUid = self.changedKeycardUids[^1].newKcUid
discard self.walletAccountService.updateKeycardUid(oldUid, newUid)

proc syncWalletAccountsOnLoginForReplacedKeycard*(self: AppController) =
self.syncWalletForReplacedKeycard = true
proc storeDefaultKeyPairForNewKeycardUser*(self: AppController) =
self.storeDefaultKeyPair = true

proc syncKeycardBasedOnAppWalletStateAfterLogin*(self: AppController) =
self.syncKeycardBasedOnAppWalletState = true

proc addToKeycardUidPairsToCheckForAChangeAfterLogin*(self: AppController, oldKeycardUid: string, newKeycardUid: string) =
self.changedKeycardUids.add((oldKcUid: oldKeycardUid, newKcUid: newKeycardUid))

proc removeAllKeycardUidPairsForCheckingForAChangeAfterLogin*(self: AppController) =
self.changedKeycardUids = @[]
45 changes: 38 additions & 7 deletions src/app/modules/startup/controller.nim
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type
tmpSeedPhraseLength: int
tmpKeyUid: string
tmpKeycardEvent: KeycardEvent
tmpCardMetadata: CardMetadata
tmpKeychainErrorOccurred: bool
tmpRecoverUsingSeedPhraseWhileLogin: bool

Expand Down Expand Up @@ -162,6 +163,7 @@ proc shouldStartWithOnboardingScreen*(self: Controller): bool =
return self.accountsService.openedAccounts().len == 0

proc storeProfileDataAndProceedWithAppLoading*(self: Controller) =
self.delegate.removeAllKeycardUidPairsForCheckingForAChangeAfterLogin() # reason for this is in the table in AppController.nim file
self.profileService.setDisplayName(self.tmpDisplayName)
let images = self.storeIdentityImage()
self.accountsService.updateLoggedInAccount(self.tmpDisplayName, images)
Expand Down Expand Up @@ -266,6 +268,18 @@ proc setRemainingAttempts*(self: Controller, value: int) =
proc setKeycardEvent*(self: Controller, value: KeycardEvent) =
self.tmpKeycardEvent = value

proc setMetadataFromKeycard*(self: Controller, cardMetadata: CardMetadata) =
self.tmpCardMetadata = cardMetadata

proc getMetadataFromKeycard*(self: Controller): CardMetadata =
return self.tmpCardMetadata

proc addToKeycardUidPairsToCheckForAChangeAfterLogin*(self: Controller, oldKeycardUid: string, newKeycardUid: string) =
self.delegate.addToKeycardUidPairsToCheckForAChangeAfterLogin(oldKeycardUid, newKeycardUid)

proc syncKeycardBasedOnAppWalletStateAfterLogin(self: Controller) =
self.delegate.syncKeycardBasedOnAppWalletStateAfterLogin()

proc keychainErrorOccurred*(self: Controller): bool =
return self.tmpKeychainErrorOccurred

Expand Down Expand Up @@ -354,23 +368,33 @@ proc storeImportedAccountAndLogin*(self: Controller, storeToKeychain: bool) =
let accountId = self.getImportedAccount().id
self.setupAccount(accountId, storeToKeychain)

proc storeKeycardAccountAndLogin*(self: Controller, storeToKeychain: bool) =
proc storeKeycardAccountAndLogin*(self: Controller, storeToKeychain: bool, newKeycard: bool) =
if self.importMnemonic():
self.delegate.moveToLoadingAppState()
self.delegate.storeKeyPairForNewKeycardUser()
self.storeMetadataForNewKeycardUser()
if newKeycard:
self.delegate.storeDefaultKeyPairForNewKeycardUser()
self.storeMetadataForNewKeycardUser()
else:
self.syncKeycardBasedOnAppWalletStateAfterLogin()
self.accountsService.setupAccountKeycard(KeycardEvent(), self.tmpDisplayName, useImportedAcc = true)
self.setupKeychain(storeToKeychain)
else:
error "an error ocurred while importing mnemonic"

proc setupKeycardAccount*(self: Controller, storeToKeychain: bool) =
proc setupKeycardAccount*(self: Controller, storeToKeychain: bool, newKeycard: bool) =
if self.tmpSeedPhrase.len > 0:
# if `tmpSeedPhrase` is not empty means user has recovered keycard via seed phrase
self.storeKeycardAccountAndLogin(storeToKeychain)
self.storeKeycardAccountAndLogin(storeToKeychain, newKeycard)
else:
if self.tmpKeycardEvent.keyUid.len == 0 or
self.accountsService.openedAccountsContainsKeyUid(self.tmpKeycardEvent.keyUid):
self.delegate.importAccountError(ACCOUNT_ALREADY_EXISTS_ERROR)
return
self.delegate.moveToLoadingAppState()
self.delegate.storeKeyPairForNewKeycardUser()
if newKeycard:
self.delegate.storeDefaultKeyPairForNewKeycardUser()
else:
self.syncKeycardBasedOnAppWalletStateAfterLogin()
self.accountsService.setupAccountKeycard(self.tmpKeycardEvent, self.tmpDisplayName, useImportedAcc = false)
self.setupKeychain(storeToKeychain)

Expand Down Expand Up @@ -419,7 +443,7 @@ proc login*(self: Controller) =

proc loginAccountKeycard*(self: Controller, storeToKeychain = false, syncWalletAfterLogin = false) =
if syncWalletAfterLogin:
self.delegate.syncWalletAccountsOnLoginForReplacedKeycard()
self.syncKeycardBasedOnAppWalletStateAfterLogin()
if storeToKeychain:
## storing not now, user will be asked to store the pin once he is logged in
singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_NOT_NOW)
Expand All @@ -434,6 +458,9 @@ proc getKeyUidForSeedPhrase*(self: Controller, seedPhrase: string): string =
let acc = self.accountsService.createAccountFromMnemonic(seedPhrase)
return acc.keyUid

proc getCurrentKeycardServiceFlow*(self: Controller): keycard_service.KCSFlowType =
return self.keycardService.getCurrentFlow()

proc getLastReceivedKeycardData*(self: Controller): tuple[flowType: string, flowEvent: KeycardEvent] =
return self.keycardService.getLastReceivedKeycardData()

Expand Down Expand Up @@ -463,6 +490,10 @@ proc runStoreMetadataFlow*(self: Controller, cardName: string, pin: string, wall
self.cancelCurrentFlow()
self.keycardService.startStoreMetadataFlow(cardName, pin, walletPaths)

proc runGetMetadataFlow*(self: Controller, resolveAddress = false, exportMasterAddr = false, pin = "") =
self.cancelCurrentFlow()
self.keycardService.startGetMetadataFlow(resolveAddress, exportMasterAddr, pin)

proc resumeCurrentFlow*(self: Controller) =
self.keycardService.resumeCurrentFlow()

Expand Down
16 changes: 6 additions & 10 deletions src/app/modules/startup/internal/biometrics_state.nim
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ proc newBiometricsState*(flowType: FlowType, backState: State): BiometricsState
proc delete*(self: BiometricsState) =
self.State.delete

method executeBackCommand*(self: BiometricsState, controller: Controller) =
if self.flowType == FlowType.FirstRunOldUserKeycardImport:
controller.runRecoverAccountFlow()

method executePrimaryCommand*(self: BiometricsState, controller: Controller) =
let storeToKeychain = true # true, cause we have support for keychain for mac os
if self.flowType == FlowType.FirstRunNewUserNewKeys:
Expand All @@ -25,11 +21,11 @@ method executePrimaryCommand*(self: BiometricsState, controller: Controller) =
## but since current implementation is like that and this is not a bug fixing issue, left as it is.
controller.storeImportedAccountAndLogin(storeToKeychain)
elif self.flowType == FlowType.FirstRunNewUserNewKeycardKeys:
controller.storeKeycardAccountAndLogin(storeToKeychain)
controller.storeKeycardAccountAndLogin(storeToKeychain, newKeycard = true)
elif self.flowType == FlowType.FirstRunNewUserImportSeedPhraseIntoKeycard:
controller.storeKeycardAccountAndLogin(storeToKeychain)
controller.storeKeycardAccountAndLogin(storeToKeychain, newKeycard = true)
elif self.flowType == FlowType.FirstRunOldUserKeycardImport:
controller.setupKeycardAccount(storeToKeychain)
controller.setupKeycardAccount(storeToKeychain, newKeycard = false)
elif self.flowType == FlowType.LostKeycardReplacement:
self.storeToKeychain = storeToKeychain
controller.startLoginFlowAutomatically(controller.getPin())
Expand All @@ -45,11 +41,11 @@ method executeSecondaryCommand*(self: BiometricsState, controller: Controller) =
## but since current implementation is like that and this is not a bug fixing issue, left as it is.
controller.storeImportedAccountAndLogin(storeToKeychain)
elif self.flowType == FlowType.FirstRunNewUserNewKeycardKeys:
controller.storeKeycardAccountAndLogin(storeToKeychain)
controller.storeKeycardAccountAndLogin(storeToKeychain, newKeycard = true)
elif self.flowType == FlowType.FirstRunNewUserImportSeedPhraseIntoKeycard:
controller.storeKeycardAccountAndLogin(storeToKeychain)
controller.storeKeycardAccountAndLogin(storeToKeychain, newKeycard = true)
elif self.flowType == FlowType.FirstRunOldUserKeycardImport:
controller.setupKeycardAccount(storeToKeychain)
controller.setupKeycardAccount(storeToKeychain, newKeycard = false)
elif self.flowType == FlowType.LostKeycardReplacement:
self.storeToKeychain = storeToKeychain
controller.startLoginFlowAutomatically(controller.getPin())
Expand Down
5 changes: 3 additions & 2 deletions src/app/modules/startup/internal/keycard_enter_pin_state.nim
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ method resolveKeycardNextState*(self: KeycardEnterPinState, keycardFlowType: str
if keycardFlowType == ResponseTypeValueKeycardFlowResult:
controller.setKeycardEvent(keycardEvent)
if not main_constants.IS_MACOS:
controller.setupKeycardAccount(false)
controller.setupKeycardAccount(storeToKeychain = false, newKeycard = false)
return nil
return createState(StateType.Biometrics, self.flowType, self)
let backState = findBackStateWithTargetedStateType(self, StateType.RecoverOldUser)
return createState(StateType.Biometrics, self.flowType, backState)
9 changes: 7 additions & 2 deletions src/app/modules/startup/internal/keycard_enter_puk_state.nim
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ method resolveKeycardNextState*(self: KeycardEnterPukState, keycardFlowType: str
controller.setKeycardEvent(keycardEvent)
controller.setPukValid(true)
if not main_constants.IS_MACOS:
controller.setupKeycardAccount(false)
controller.setupKeycardAccount(storeToKeychain = false, newKeycard = false)
return nil
let backState = findBackStateWithTargetedStateType(self, StateType.RecoverOldUser)
return createState(StateType.Biometrics, self.flowType, backState)
Expand All @@ -52,4 +52,9 @@ method resolveKeycardNextState*(self: KeycardEnterPukState, keycardFlowType: str
controller.setRemainingAttempts(keycardEvent.pukRetries)
if keycardEvent.pukRetries > 0:
return createState(StateType.KeycardWrongPuk, self.flowType, self.getBackState)
return createState(StateType.KeycardMaxPukRetriesReached, self.flowType, self.getBackState)
return createState(StateType.KeycardMaxPukRetriesReached, self.flowType, self.getBackState)
if keycardFlowType == ResponseTypeValueKeycardFlowResult:
controller.setKeycardEvent(keycardEvent)
controller.setPukValid(true)
controller.loginAccountKeycard()
return nil
Loading

0 comments on commit b44fc7a

Please sign in to comment.