chore:(@desktop/general): storing to keychain logic simplified, improved

src/app/boot/app_controller.nim

@@ -1,4 +1,4 @@
-import NimQml, sequtils, sugar, chronicles, os
+import NimQml, sequtils, sugar, chronicles, os, uuids
 
 import ../../app_service/service/general/service as general_service
 import ../../app_service/service/keychain/service as keychain_service
@@ -37,6 +37,7 @@ import ../modules/startup/module as startup_module
 import ../modules/main/module as main_module
 import ../core/notifications/notifications_manager
 
+import ../../constants as main_constants
 import ../global/global_singleton
 
 import ../core/[main]
@@ -51,6 +52,7 @@ type
     changedKeycardUids: seq[tuple[oldKcUid: string, newKcUid: string]] # used in case user unlocked keycard during onboarding using seed phrase
     statusFoundation: StatusFoundation
     notificationsManager*: NotificationsManager
+    keychainConnectionIds: seq[UUID]
 
     # Global
     appSettingsVariant: QVariant
@@ -310,6 +312,47 @@ proc delete*(self: AppController) =
   self.gifService.delete
   self.keycardService.delete
 
+proc disconnectKeychain(self: AppController) =
+  for id in self.keychainConnectionIds:
+    self.statusFoundation.events.disconnect(id)
+  self.keychainConnectionIds = @[]
+
+proc connectKeychain(self: AppController) =
+  var handlerId = self.statusFoundation.events.onWithUUID(SIGNAL_KEYCHAIN_SERVICE_SUCCESS) do(e:Args):
+    let args = KeyChainServiceArg(e)
+    self.disconnectKeychain()
+    ## we need to set local `storeToKeychain` prop to `store` value since in this context means pass/pin is stored well
+    singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_STORE)
+  self.keychainConnectionIds.add(handlerId)
+
+  handlerId = self.statusFoundation.events.onWithUUID(SIGNAL_KEYCHAIN_SERVICE_ERROR) do(e:Args):
+    let args = KeyChainServiceArg(e)
+    self.disconnectKeychain()
+    ## no need for any other activity in this context, local `storeToKeychain` prop remains as it was
+    ## maybe in some point in future we add a popup letting user know about this
+    info "unable to store the data to keychain", errCode=args.errCode, errType=args.errType, errDesc=args.errDescription
+  self.keychainConnectionIds.add(handlerId)
+
+proc checkForStoringPasswordToKeychain(self: AppController) =
+  ## This proc is used to store pass/pin depends on user's selection during onboarding flow.
+  let account = self.accountsService.getLoggedInAccount()
+  let value = singletonInstance.localAccountSettings.getStoreToKeychainValue()
+  if not main_constants.IS_MACOS or # This is MacOS only feature
+    value == LS_VALUE_STORE or # means pass is already stored, no need to store it again
+    value == LS_VALUE_NEVER or # means pass doesn't need to be stored at all
+    account.name.len == 0:
+      return
+  # We are here if stored "storeToKeychain" property for the logged in user is either empty or set to "NotNow".
+
+  #TODO: we should store PubKey of this account instead of display name (display name is not unique)
+  #      and we may run into a problem if 2 accounts with the same display name are generated.
+  self.connectKeychain()
+  let pass = self.startupModule.getPassword()
+  if pass.len > 0:
+    self.keychainService.storeData(account.name, pass)
+  else:
+    self.keychainService.storeData(account.name, self.startupModule.getPin())
+
 proc startupDidLoad*(self: AppController) =
   singletonInstance.engine.setRootContextProperty("localAppSettings", self.localAppSettingsVariant)
   singletonInstance.engine.setRootContextProperty("localAccountSettings", self.localAccountSettingsVariant)
@@ -323,7 +366,7 @@ proc startupDidLoad*(self: AppController) =
 
 proc mainDidLoad*(self: AppController) =
   self.startupModule.moveToAppState()
-  self.startupModule.checkForStoringPasswordToKeychain()
+  self.checkForStoringPasswordToKeychain()
 
 proc start*(self: AppController) =
   self.keycardService.init()

src/app/modules/main/controller.nim

@@ -8,7 +8,6 @@ import ../../core/notifications/notifications_manager
 import ../../../app_service/common/types
 import ../../../app_service/service/settings/service as settings_service
 import ../../../app_service/service/node_configuration/service as node_configuration_service
-import ../../../app_service/service/keychain/service as keychain_service
 import ../../../app_service/service/accounts/service as accounts_service
 import ../../../app_service/service/chat/service as chat_service
 import ../../../app_service/service/community/service as community_service
@@ -34,7 +33,6 @@ type
     events: EventEmitter
     settingsService: settings_service.Service
     nodeConfigurationService: node_configuration_service.Service
-    keychainService: keychain_service.Service
     accountsService: accounts_service.Service
     chatService: chat_service.Service
     communityService: community_service.Service
@@ -54,7 +52,6 @@ proc newController*(delegate: io_interface.AccessInterface,
   events: EventEmitter,
   settingsService: settings_service.Service,
   nodeConfigurationService: node_configuration_service.Service,
-  keychainService: keychain_service.Service,
   accountsService: accounts_service.Service,
   chatService: chat_service.Service,
   communityService: community_service.Service,
@@ -71,7 +68,6 @@ proc newController*(delegate: io_interface.AccessInterface,
   result.events = events
   result.settingsService = settingsService
   result.nodeConfigurationService = nodeConfigurationService
-  result.keychainService = keychainService
   result.accountsService = accountsService
   result.chatService = chatService
   result.communityService = communityService
@@ -104,15 +100,6 @@ proc init*(self: Controller) =
   self.events.on(SIGNAL_MAILSERVER_NOT_WORKING) do(e: Args):
     self.delegate.emitMailserverNotWorking()
 
-  self.events.on(SIGNAL_KEYCHAIN_SERVICE_SUCCESS) do(e:Args):
-    let args = KeyChainServiceArg(e)
-    self.delegate.emitStoringPasswordSuccess()
-
-  self.events.on(SIGNAL_KEYCHAIN_SERVICE_ERROR) do(e:Args):
-    let args = KeyChainServiceArg(e)
-    if self.accountsService.getLoggedInAccount().isValid():
-      self.delegate.emitStoringPasswordError(args.errDescription)
-
   self.events.on(SIGNAL_COMMUNITY_JOINED) do(e:Args):
     let args = CommunityArgs(e)
     self.delegate.communityJoined(
@@ -312,15 +299,6 @@ proc isConnected*(self: Controller): bool =
 proc getChannelGroups*(self: Controller): seq[ChannelGroupDto] =
   return self.chatService.getChannelGroups()
 
-proc storePassword*(self: Controller, password: string) =
-  let account = self.accountsService.getLoggedInAccount()
-
-  let value = singletonInstance.localAccountSettings.getStoreToKeychainValue()
-  if (value != LS_VALUE_STORE or account.name.len == 0):
-    return
-
-  self.keychainService.storeData(account.name, password)
-
 proc getActiveSectionId*(self: Controller): string =
   result = self.activeSectionId
 

src/app/modules/main/io_interface.nim

@@ -81,12 +81,6 @@ method getActiveSectionId*(self: AccessInterface): string {.base.} =
 method communitiesModuleDidLoad*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method emitStoringPasswordError*(self: AccessInterface, errorDescription: string) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
-method emitStoringPasswordSuccess*(self: AccessInterface) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
 method emitMailserverWorking*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
@@ -165,9 +159,6 @@ method onNetworkDisconnected*(self: AccessInterface) {.base.} =
 method viewDidLoad*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method storePassword*(self: AccessInterface, password: string) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
 method setActiveSection*(self: AccessInterface, item: SectionItem) {.base.} =
   raise newException(ValueError, "No implementation available")
 

src/app/modules/main/module.nim

@@ -145,7 +145,6 @@ proc newModule*[T](
     events,
     settingsService,
     nodeConfigurationService,
-    keychainService,
     accountsService,
     chatService,
     communityService,
@@ -566,15 +565,6 @@ method networksModuleDidLoad*[T](self: Module[T]) =
 method viewDidLoad*[T](self: Module[T]) =
   self.checkIfModuleDidLoad()
 
-method storePassword*[T](self: Module[T], password: string) =
-  self.controller.storePassword(password)
-
-method emitStoringPasswordError*[T](self: Module[T], errorDescription: string) =
-  self.view.emitStoringPasswordError(errorDescription)
-
-method emitStoringPasswordSuccess*[T](self: Module[T]) =
-  self.view.emitStoringPasswordSuccess()
-
 method emitMailserverWorking*[T](self: Module[T]) =
   self.view.emitMailserverWorking()
 
@@ -709,24 +699,6 @@ method communityJoined*[T](
   var firstCommunityJoined = false
   if (self.channelGroupModules.len == 1): # First one is personal chat section
     firstCommunityJoined = true
-
-  if(community.permissions.access == COMMUNITY_PERMISSION_ACCESS_ON_REQUEST and 
-    community.requestedToJoinAt > 0 and 
-    community.joined):
-    singletonInstance.globalEvents.myRequestToJoinCommunityAcccepted("Community Request Accepted",
-      fmt "Your request to join community {community.name} is accepted", community.id)
-    self.displayEphemeralNotification(fmt "{community.name} membership approved ", "", conf.COMMUNITIESPORTAL_SECTION_ICON, false, EphemeralNotificationType.Success.int, "")
-
-  # if we are joining spectated community
-  if self.channelGroupModules.hasKey(community.id):
-    let communityModule = self.channelGroupModules[community.id]
-    # Must never happen
-    if not communityModule.isLoaded():
-      error "Community module was not loaded in spectated mode", communityId=community.id
-
-    communityModule.joinSpectatedCommunity()
-    return
-    
   self.channelGroupModules[community.id] = chat_section_module.newModule(
       self,
       events,

src/app/modules/main/view.nim

@@ -110,19 +110,6 @@ QtObject:
 
   proc openStoreToKeychainPopup*(self: View) {.signal.}
 
-  proc storePassword*(self: View, password: string) {.slot.} =
-    self.delegate.storePassword(password)
-
-  proc storingPasswordError*(self:View, errorDescription: string) {.signal.}
-
-  proc emitStoringPasswordError*(self: View, errorDescription: string) =
-    self.storingPasswordError(errorDescription)
-
-  proc storingPasswordSuccess*(self:View) {.signal.}
-
-  proc emitStoringPasswordSuccess*(self: View) =
-    self.storingPasswordSuccess()
-
   proc mailserverWorking*(self:View) {.signal.}
 
   proc mailserverNotWorking*(self:View) {.signal.}

src/app/modules/startup/controller.nim

@@ -80,21 +80,24 @@ proc newController*(delegate: io_interface.AccessInterface,
 proc cleanTmpData*(self: Controller)
 proc storeMetadataForNewKeycardUser(self: Controller)
 proc storeIdentityImage*(self: Controller): seq[Image]
+proc getSelectedLoginAccount*(self: Controller): AccountDto
 
-proc disconnectKeychain*(self: Controller) =
+proc disconnectKeychain(self: Controller) =
   for id in self.keychainConnectionIds:
     self.events.disconnect(id)
   self.keychainConnectionIds = @[]
 
-proc connectKeychain*(self: Controller) =
+proc connectKeychain(self: Controller) =
   var handlerId = self.events.onWithUUID(SIGNAL_KEYCHAIN_SERVICE_SUCCESS) do(e:Args):
     let args = KeyChainServiceArg(e)
+    self.disconnectKeychain()
     self.delegate.emitObtainingPasswordSuccess(args.data)
   self.keychainConnectionIds.add(handlerId)
 
   handlerId = self.events.onWithUUID(SIGNAL_KEYCHAIN_SERVICE_ERROR) do(e:Args):
     let args = KeyChainServiceArg(e)
     self.tmpKeychainErrorOccurred = true
+    self.disconnectKeychain()
     self.delegate.emitObtainingPasswordError(args.errDescription, args.errType)
   self.keychainConnectionIds.add(handlerId)
 
@@ -122,8 +125,6 @@ proc delete*(self: Controller) =
   self.disconnect()
 
 proc init*(self: Controller) =
-  self.connectKeychain()
-
   var handlerId = self.events.onWithUUID(SignalType.NodeLogin.event) do(e:Args):
     let signal = NodeSignal(e)
     self.delegate.onNodeLogin(signal.event.error)
@@ -306,15 +307,16 @@ proc cleanTmpData*(self: Controller) =
   self.setKeycardEvent(KeycardEvent())
   self.setRecoverUsingSeedPhraseWhileLogin(false)
 
-proc storePasswordToKeychain(self: Controller) =
+proc tryToObtainDataFromKeychain*(self: Controller) =
-  let account = self.accountsService.getLoggedInAccount()
+  ## This proc is used to fetch pass/pin from the keychain while user is trying to login.
   let value = singletonInstance.localAccountSettings.getStoreToKeychainValue()
-  if (value != LS_VALUE_STORE or account.name.len == 0):
+  if not main_constants.IS_MACOS or # This is MacOS only feature
-    return
+    value != LS_VALUE_STORE:
-  #TODO: we should store PubKey of this account instead of display name (display name is not unique)
+      return
-  #      and we may run into a problem if 2 accounts with the same display name are generated.
+  self.connectKeychain() # handling the results is done in slots connected in `connectKeychain` proc 
-  let data = if self.tmpPassword.len > 0: self.tmpPassword else: self.tmpPin
+  self.tmpKeychainErrorOccurred = false
-  self.keychainService.storeData(account.name, data)
+  let selectedAccount = self.getSelectedLoginAccount()
+  self.keychainService.tryToObtainData(selectedAccount.name)
 
 proc storeIdentityImage*(self: Controller): seq[Image] =
   if self.tmpProfileImageDetails.url.len == 0:
@@ -343,8 +345,7 @@ proc importMnemonic*(self: Controller): bool =
 
 proc setupKeychain(self: Controller, store: bool) =
   if store:
-    singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_STORE)
+    singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_NOT_NOW)
-    self.storePasswordToKeychain()
   else:
     singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_NEVER)
 
@@ -423,17 +424,6 @@ proc isSelectedAccountAKeycardAccount*(self: Controller): bool =
   let selectedAccount = self.getSelectedLoginAccount()
   return selectedAccount.keycardPairing.len > 0
 
-proc tryToObtainDataFromKeychain*(self: Controller) =
-  # Dealing with Keychain is the MacOS only feature
-  if not main_constants.IS_MACOS:
-    return
-  let value = singletonInstance.localAccountSettings.getStoreToKeychainValue()
-  if (value != LS_VALUE_STORE):
-    return
-  self.tmpKeychainErrorOccurred = false
-  let selectedAccount = self.getSelectedLoginAccount()
-  self.keychainService.tryToObtainData(selectedAccount.name)
-
 proc login*(self: Controller) =
   self.delegate.moveToLoadingAppState()
   let selectedAccount = self.getSelectedLoginAccount()
@@ -464,11 +454,7 @@ proc loginAccountKeycardUsingSeedPhrase*(self: Controller, storeToKeychain: bool
   if acc.derivedAccounts.whisper.privateKey.startsWith("0x"):
     kcData.whisperKey.privateKey = acc.derivedAccounts.whisper.privateKey[2..^1]
 
-  if storeToKeychain:
+  self.setupKeychain(storeToKeychain)
-    ## storing not now, user will be asked to store the pin once he is logged in
-    singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_NOT_NOW)
-  else:
-    singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_NEVER)
 
   self.delegate.moveToLoadingAppState()
   let error = self.accountsService.loginAccountKeycard(selAcc, kcData)
@@ -555,19 +541,3 @@ proc storeMetadataForNewKeycardUser(self: Controller) =
   ## Stores metadata, default Status account only, to the keycard for a newly created keycard user.
   let paths = @[account_constants.PATH_DEFAULT_WALLET]
   self.runStoreMetadataFlow(self.getDisplayName(), self.getPin(), paths)
-
-proc checkForStoringPasswordToKeychain*(self: Controller) =
-  # This proc is called once user is logged in irrespective he is logged in
-  # through the onboarding or login view. 
-  # This is MacOS only feature
-  if not main_constants.IS_MACOS:
-    return
-
-  let value = singletonInstance.localAccountSettings.getStoreToKeychainValue()
-  if (value == LS_VALUE_STORE or value == LS_VALUE_NEVER):
-    return
-
-  # We are here if stored "storeToKeychain" property for the logged in user
-  # is either empty or set to "NotNow".
-  singletonInstance.localAccountSettings.setStoreToKeychainValue(LS_VALUE_STORE)
-  self.storePasswordToKeychain()

src/app/modules/startup/io_interface.nim

@@ -156,9 +156,6 @@ method addToKeycardUidPairsToCheckForAChangeAfterLogin*(self: AccessInterface, o
 method removeAllKeycardUidPairsForCheckingForAChangeAfterLogin*(self: AccessInterface) {.base.} =
   raise newException(ValueError, "No implementation available")
 
-method checkForStoringPasswordToKeychain*(self: AccessInterface) {.base.} =
-  raise newException(ValueError, "No implementation available")
-
 method onFetchingFromWakuMessageReceived*(self: AccessInterface, section: string, totalMessages: int, loadedMessages: int) {.base.} =
   raise newException(ValueError, "No implementation available")
 

src/app/modules/startup/module.nim

@@ -272,7 +272,6 @@ method importAccountSuccess*[T](self: Module[T]) =
   self.view.importAccountSuccess()
 
 method setSelectedLoginAccount*[T](self: Module[T], item: login_acc_item.Item) =
-  self.controller.disconnectKeychain()
   self.controller.cancelCurrentFlow()
   self.controller.setSelectedLoginAccount(item.getKeyUid(), item.getKeycardCreatedAccount())
   if item.getKeycardCreatedAccount():
@@ -286,7 +285,6 @@ method setSelectedLoginAccount*[T](self: Module[T], item: login_acc_item.Item) =
     else:
       self.view.setCurrentStartupState(newLoginKeycardEnterPasswordState(FlowType.AppLogin, nil))
   self.view.setSelectedLoginAccount(item)
-  self.controller.connectKeychain()
 
 method emitAccountLoginError*[T](self: Module[T], error: string) =
   self.controller.setPassword("")
@@ -492,6 +490,3 @@ method addToKeycardUidPairsToCheckForAChangeAfterLogin*[T](self: Module[T], oldK
 
 method removeAllKeycardUidPairsForCheckingForAChangeAfterLogin*[T](self: Module[T]) =
   self.delegate.removeAllKeycardUidPairsForCheckingForAChangeAfterLogin()
-
-method checkForStoringPasswordToKeychain*[T](self: Module[T]) =
-  self.controller.checkForStoringPasswordToKeychain()

src/app_service/service/keychain/service.nim

@@ -48,6 +48,9 @@ QtObject:
   proc tryToObtainData*(self: Service, key: string) =
     self.keychainManager.readDataAsync(key)
 
+  proc tryToDeleteData*(self: Service, key: string) =
+    self.keychainManager.deleteDataAsync(key)
+
   proc onKeychainManagerError*(self: Service, errorType: string, errorCode: int,
     errorDescription: string) {.slot.} =
     ## This slot is called in case an error occured while we're dealing with