Remove stale peer indices when getting peer by key after removing (#711)

When we delete a peer from an account, we save the account in the file store.
The file store maintains peerID -> accountID and peerKey -> accountID indices.
Those can't be updated when we delete a peer because the store saves the whole account
without a peer already and has no access to the removed peer.
In this PR, we dynamically check if there are stale indices when GetAccountByPeerPubKey
and GetAccountByPeerID.

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
 }
 

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)