From 57dea7b08df96b51cd8c8534694f03108608410a Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Fri, 25 Aug 2023 15:20:53 +0200 Subject: [PATCH] fix: saving/updating a keycard updates the accounts' operability This commit skips deleting a keystore file for account which are not marked as fully operable and also skips deleting master key keystore file if a keypair is non operable. It also takes into consideration the operable property of an account when adding/updating/handling a keypair/keycard. u3 --- multiaccounts/accounts/database.go | 30 ++++++++++++++++-- protocol/messenger_handler.go | 51 +++++++++++++++++++----------- protocol/messenger_wallet.go | 6 ++-- services/accounts/accounts.go | 22 +++++++------ 4 files changed, 77 insertions(+), 32 deletions(-) diff --git a/multiaccounts/accounts/database.go b/multiaccounts/accounts/database.go index 5308fd312..3465926ea 100644 --- a/multiaccounts/accounts/database.go +++ b/multiaccounts/accounts/database.go @@ -254,6 +254,27 @@ func (a *Keypair) GetChatPublicKey() types.HexBytes { return nil } +func (a *Keypair) MigratedToKeycard() bool { + return len(a.Keycards) > 0 +} + +// Returns operability of a keypair: +// - if any of keypair's account is not operable, then a keyapir is considered as non operable +// - if any of keypair's account is partially operable, then a keyapir is considered as partially operable +// - if all accounts are fully operable, then a keyapir is considered as fully operable +func (a *Keypair) Operability() AccountOperable { + for _, acc := range a.Accounts { + if acc.Operable == AccountNonOperable { + return AccountNonOperable + } + if acc.Operable == AccountPartiallyOperable { + return AccountPartiallyOperable + } + } + + return AccountFullyOperable +} + // Database sql wrapper for operations with browser objects. type Database struct { *settings.Database @@ -1287,7 +1308,8 @@ func (db *Database) GetNodeConfig() (*params.NodeConfig, error) { // local pairing and then imports seed/private key for the non profile keypair on one of those two devices // to make that keypair fully operable. In that case we need to inform other device about the change, that // other device may offer other options for importing that keypair on it. -func (db *Database) MarkKeypairFullyOperable(keyUID string, clock uint64) (err error) { +// If the clock is set to -1, do not update it. +func (db *Database) MarkKeypairFullyOperable(keyUID string, clock uint64, updateKeypairClock bool) (err error) { tx, err := db.db.Begin() if err != nil { return err @@ -1317,7 +1339,11 @@ func (db *Database) MarkKeypairFullyOperable(keyUID string, clock uint64) (err e return err } - return db.updateKeypairClock(tx, keyUID, clock) + if updateKeypairClock { + return db.updateKeypairClock(tx, keyUID, clock) + } + + return nil } func (db *Database) MarkAccountFullyOperable(address types.Address) (err error) { diff --git a/protocol/messenger_handler.go b/protocol/messenger_handler.go index f4d9b5177..1f604c74d 100644 --- a/protocol/messenger_handler.go +++ b/protocol/messenger_handler.go @@ -3105,6 +3105,11 @@ func (m *Messenger) resolveAccountOperability(syncAcc *protobuf.SyncAccount, syn if accountReceivedFromLocalPairing { return accounts.AccountOperable(syncAcc.Operable), nil } + + if syncKpMigratedToKeycard { + return accounts.AccountFullyOperable, nil + } + accountsOperability := accounts.AccountNonOperable dbAccount, err := m.settings.GetAccountByAddress(types.BytesToAddress(syncAcc.Address)) if err != nil && err != accounts.ErrDbAccountNotFound { @@ -3129,7 +3134,7 @@ func (m *Messenger) resolveAccountOperability(syncAcc *protobuf.SyncAccount, syn } } - if syncKpMigratedToKeycard || syncAcc.Chat || syncAcc.Wallet { + if syncAcc.Chat || syncAcc.Wallet { accountsOperability = accounts.AccountFullyOperable } else { partiallyOrFullyOperable, err := m.settings.IsAnyAccountPartiallyOrFullyOperableForKeyUID(syncAcc.KeyUid) @@ -3259,8 +3264,8 @@ func (m *Messenger) handleSyncKeypair(message *protobuf.SyncKeypair, fromLocalPa } } + syncKpMigratedToKeycard := len(message.Keycards) > 0 for _, sAcc := range message.Accounts { - syncKpMigratedToKeycard := len(message.Keycards) > 0 if message.SyncedFrom == accounts.SyncedFromBackup && kp.Type == accounts.KeypairTypeProfile { // if a profile keypair is coming from backup, we're handling within this block the case when a recovering // was inititiated via keycard, while backed up profile keypair data refers to a regular profile @@ -3279,27 +3284,37 @@ func (m *Messenger) handleSyncKeypair(message *protobuf.SyncKeypair, fromLocalPa kp.Accounts = append(kp.Accounts, acc) } - if kp.Removed { - // delete all keystore files - err = m.deleteKeystoreFilesForKeypair(dbKeypair) - if err != nil { - return nil, err - } - } else if !fromLocalPairing && dbKeypair != nil { - for _, dbAcc := range dbKeypair.Accounts { - removeAcc := false - for _, acc := range kp.Accounts { - if dbAcc.Address == acc.Address && acc.Removed && !dbAcc.Removed { - removeAcc = true - break - } + if !fromLocalPairing { + if kp.Removed || + dbKeypair != nil && !dbKeypair.MigratedToKeycard() && syncKpMigratedToKeycard { + // delete all keystore files + err = m.deleteKeystoreFilesForKeypair(dbKeypair) + if err != nil { + return nil, err } - if removeAcc { - err = m.deleteKeystoreFileForAddress(dbAcc.Address) + + if syncKpMigratedToKeycard { + err = m.settings.MarkKeypairFullyOperable(dbKeypair.KeyUID, 0, false) if err != nil { return nil, err } } + } else if dbKeypair != nil { + for _, dbAcc := range dbKeypair.Accounts { + removeAcc := false + for _, acc := range kp.Accounts { + if dbAcc.Address == acc.Address && acc.Removed && !dbAcc.Removed { + removeAcc = true + break + } + } + if removeAcc { + err = m.deleteKeystoreFileForAddress(dbAcc.Address) + if err != nil { + return nil, err + } + } + } } } diff --git a/protocol/messenger_wallet.go b/protocol/messenger_wallet.go index 104b8cc02..70749c403 100644 --- a/protocol/messenger_wallet.go +++ b/protocol/messenger_wallet.go @@ -177,7 +177,7 @@ func (m *Messenger) SaveOrUpdateAccount(acc *accounts.Account) error { func (m *Messenger) MarkKeypairFullyOperable(keyUID string) error { clock, _ := m.getLastClockWithRelatedChat() - err := m.settings.MarkKeypairFullyOperable(keyUID, clock) + err := m.settings.MarkKeypairFullyOperable(keyUID, clock, true) if err != nil { return err } @@ -201,7 +201,7 @@ func (m *Messenger) deleteKeystoreFileForAddress(address types.Address) error { return err } - if len(kp.Keycards) == 0 { + if !kp.MigratedToKeycard() { err = m.accountsManager.DeleteAccount(address) var e *account.ErrCannotLocateKeyFile if err != nil && !errors.As(err, &e) { @@ -225,7 +225,7 @@ func (m *Messenger) deleteKeystoreFileForAddress(address types.Address) error { } func (m *Messenger) deleteKeystoreFilesForKeypair(keypair *accounts.Keypair) (err error) { - if keypair == nil || len(keypair.Keycards) > 0 { + if m.accountsManager == nil || keypair == nil || keypair.MigratedToKeycard() { return } diff --git a/services/accounts/accounts.go b/services/accounts/accounts.go index b5d26be1f..def610bff 100644 --- a/services/accounts/accounts.go +++ b/services/accounts/accounts.go @@ -346,7 +346,7 @@ func (api *API) MakePartiallyOperableAccoutsFullyOperable(ctx context.Context, p return } - if len(profileKeypair.Keycards) == 0 && !api.VerifyPassword(password) { + if !profileKeypair.MigratedToKeycard() && !api.VerifyPassword(password) { err = errors.New("wrong password provided") return } @@ -467,11 +467,6 @@ func (api *API) SaveOrUpdateKeycard(ctx context.Context, keycard *accounts.Keyca return err } - relatedKeycardsByKeyUID, err := api.db.GetKeycardsWithSameKeyUID(keycard.KeyUID) - if err != nil { - return err - } - err = (*api.messenger).SaveOrUpdateKeycard(ctx, keycard) if err != nil { return err @@ -480,9 +475,13 @@ func (api *API) SaveOrUpdateKeycard(ctx context.Context, keycard *accounts.Keyca if !accountsComingFromKeycard { // Once we migrate a keypair, corresponding keystore files need to be deleted // if the keypair being migrated is not already migrated (in case user is creating a copy of an existing Keycard) - if len(relatedKeycardsByKeyUID) == 0 { - for _, addr := range keycard.AccountsAddresses { - err = api.manager.DeleteAccount(addr) + // and if keypair operability is different from non operable (otherwise there are not keystore files to be deleted). + if !kpDb.MigratedToKeycard() && kpDb.Operability() != accounts.AccountNonOperable { + for _, acc := range kpDb.Accounts { + if acc.Operable != accounts.AccountFullyOperable { + continue + } + err = api.manager.DeleteAccount(acc.Address) if err != nil { return err } @@ -493,6 +492,11 @@ func (api *API) SaveOrUpdateKeycard(ctx context.Context, keycard *accounts.Keyca return err } } + + err = (*api.messenger).MarkKeypairFullyOperable(keycard.KeyUID) + if err != nil { + return err + } } return nil