Prevent recovery with identity that has ever been inserted (#639)

src/app.rs

@@ -477,8 +477,7 @@ impl App {
             return Err(ServerError::UnreducedCommitment);
         }
 
-        let identity_exists = self.database.identity_exists(commitment).await?;
-        if identity_exists {
+        if self.database.identity_exists(commitment).await? {
             return Err(ServerError::DuplicateCommitment);
         }
 
@@ -561,7 +560,14 @@ impl App {
         existing_commitment: &Hash,
         new_commitment: &Hash,
     ) -> Result<(), ServerError> {
-        // Ensure that insertion provers exist
+        if *new_commitment == self.identity_manager.initial_leaf_value() {
+            warn!(
+                ?new_commitment,
+                "Attempt to insert initial leaf in recovery."
+            );
+            return Err(ServerError::InvalidCommitment);
+        }
+
         if !self.identity_manager.has_insertion_provers().await {
             warn!(
                 ?new_commitment,
@@ -578,6 +584,10 @@ impl App {
             return Err(ServerError::UnreducedCommitment);
         }
 
+        if self.database.identity_exists(*new_commitment).await? {
+            return Err(ServerError::DuplicateCommitment);
+        }
+
         // Delete the existing id and insert the commitments into the recovery table
         self.delete_identity(existing_commitment).await?;
 

tests/recover_identities.rs

@@ -127,6 +127,22 @@ async fn recover_identities() -> anyhow::Result<()> {
         test_inclusion_proof(&uri, &client, i, &ref_tree, &identities_ref[i], false).await;
     }
 
+    // Test that we cannot recover with an identity that has previously been
+    // inserted
+    test_recover_identity(
+        &uri,
+        &client,
+        &mut ref_tree,
+        &identities_ref,
+        // Last inserted identity
+        insertion_batch_size - 1,
+        // Second to last inserted identity as recovery
+        identities_ref[insertion_batch_size - 2],
+        next_leaf_index,
+        true,
+    )
+    .await;
+
     // Insert enough recoveries to trigger a batch
     for i in 0..deletion_batch_size {
         // Delete the identity at i and replace it with an identity at the back of the