diff --git a/management/server/file_store.go b/management/server/file_store.go index 73bece90a..8af8759d8 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -225,7 +225,6 @@ func (s *FileStore) SaveAccount(account *Account) error { accountCopy := account.Copy() - // todo will override, handle existing keys s.Accounts[accountCopy.Id] = accountCopy // todo check that account.Id and keyId are not exist already @@ -354,6 +353,14 @@ func (s *FileStore) GetAccountByPeerID(peerID string) (*Account, error) { return nil, err } + // this protection is needed because when we delete a peer, we don't really remove index peerID -> accountID. + // check Account.Peers for a match + if _, ok := account.Peers[peerID]; !ok { + delete(s.PeerID2AccountID, peerID) + log.Warnf("removed stale peerID %s to accountID %s index", peerID, accountID) + return nil, status.Errorf(status.NotFound, "provided peer doesn't exists %s", peerID) + } + return account.Copy(), nil } @@ -372,6 +379,21 @@ func (s *FileStore) GetAccountByPeerPubKey(peerKey string) (*Account, error) { return nil, err } + // this protection is needed because when we delete a peer, we don't really remove index peerKey -> accountID. + // check Account.Peers for a match + stale := true + for _, peer := range account.Peers { + if peer.Key == peerKey { + stale = false + break + } + } + if stale { + delete(s.PeerKeyID2AccountID, peerKey) + log.Warnf("removed stale peerKey %s to accountID %s index", peerKey, accountID) + return nil, status.Errorf(status.NotFound, "provided peer doesn't exists %s", peerKey) + } + return account.Copy(), nil } diff --git a/management/server/file_store_test.go b/management/server/file_store_test.go index 6a4823457..dadcb3eb2 100644 --- a/management/server/file_store_test.go +++ b/management/server/file_store_test.go @@ -14,6 +14,44 @@ type accounts struct { Accounts map[string]*Account } +func TestStalePeerIndices(t *testing.T) { + storeDir := t.TempDir() + + err := util.CopyFileContents("testdata/store.json", filepath.Join(storeDir, "store.json")) + if err != nil { + t.Fatal(err) + } + + store, err := NewFileStore(storeDir) + if err != nil { + return + } + + account, err := store.GetAccount("bf1c8084-ba50-4ce7-9439-34653001fc3b") + require.NoError(t, err) + + peerID := "some_peer" + peerKey := "some_peer_key" + account.Peers[peerID] = &Peer{ + ID: peerID, + Key: peerKey, + } + + err = store.SaveAccount(account) + require.NoError(t, err) + + account.DeletePeer(peerID) + + err = store.SaveAccount(account) + require.NoError(t, err) + + _, err = store.GetAccountByPeerID(peerID) + require.Error(t, err, "expecting to get an error when found stale index") + + _, err = store.GetAccountByPeerPubKey(peerKey) + require.Error(t, err, "expecting to get an error when found stale index") +} + func TestNewStore(t *testing.T) { store := newStore(t)