From b738e1bc5c5a9f1443d69f5ac541cd76cbfea855 Mon Sep 17 00:00:00 2001
From: Tankred Hase <mail@tankredhase.de>
Date: Thu, 24 Aug 2017 16:36:32 +0800
Subject: [PATCH] =?UTF-8?q?Allow=20update=20of=20an=20email=20address?=
 =?UTF-8?q?=E2=80=99=20key=20with=20remove/verify=20flow=20in=20between?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/service/public-key.js           | 21 ++++++++++------
 test/integration/public-key-test.js | 39 ++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/src/service/public-key.js b/src/service/public-key.js
index 9bb0e91..f4098c7 100644
--- a/src/service/public-key.js
+++ b/src/service/public-key.js
@@ -71,10 +71,10 @@ class PublicKey {
     await this._purgeOldUnverified();
     // parse key block
     const key = this._pgp.parseKey(publicKeyArmored);
-    // check for existing verfied key by id or email addresses
-    const verified = await this.getVerified(key);
+    // check for existing verified key with same id
+    const verified = await this.getVerified({keyId: key.keyId});
     if (verified) {
-      util.throw(304, 'Key for this user already exists');
+      util.throw(304, 'Key with this key id already exists');
     }
     // store key in database
     await this._persisKey(key);
@@ -144,11 +144,7 @@ class PublicKey {
     if (!key) {
       util.throw(404, 'User id not found');
     }
-    // check if user ids of this key have already been verified in another key
-    const verified = await this.getVerified(key);
-    if (verified && verified.keyId !== keyId) {
-      util.throw(304, 'Key for this user already exists');
-    }
+    await this._removeKeysWithSameEmail(key, nonce);
     // flag the user id as verified
     await this._mongo.update(query, {
       'userIds.$.verified': true,
@@ -156,6 +152,15 @@ class PublicKey {
     }, DB_TYPE);
   }
 
+  async _removeKeysWithSameEmail({keyId, userIds}, nonce) {
+    const {email} = userIds.find(uid => uid.nonce === nonce);
+    const keys = await this._mongo.list({
+      keyId: {$ne: keyId},
+      'userIds.email': email
+    }, DB_TYPE);
+    await Promise.all(keys.map(({_id}) => this._mongo.remove({_id}, DB_TYPE)));
+  }
+
   /**
    * Check if a verified key already exists either by fingerprint, 16 char key id,
    * or email address. There can only be one verified user ID for an email address
diff --git a/test/integration/public-key-test.js b/test/integration/public-key-test.js
index a1e0d60..cd2c32f 100644
--- a/test/integration/public-key-test.js
+++ b/test/integration/public-key-test.js
@@ -23,7 +23,6 @@ describe('Public Key Integration Tests', function() {
 
   const DB_TYPE = 'publickey';
   const primaryEmail = 'test1@example.com';
-  const primaryEmail2 = 'test2@example.com';
   const origin = {host: 'localhost', protocol: 'http'};
 
   before(async () => {
@@ -95,6 +94,13 @@ describe('Public Key Integration Tests', function() {
         expect(e.status).to.equal(304);
       }
     });
+
+    it('should work for a key with an existing/verified email address to allow key update without an extra delete step in between', async () => {
+      await publicKey.put({publicKeyArmored, origin});
+      await publicKey.verify(mailsSent[1].params);
+      await publicKey.put({publicKeyArmored: publicKeyArmored2, origin});
+      expect(mailsSent.length).to.equal(5);
+    });
   });
 
   describe('_purgeOldUnverified', () => {
@@ -166,23 +172,28 @@ describe('Public Key Integration Tests', function() {
       expect(gotten.userIds[1].nonce).to.exist;
     });
 
-    it('should not verify a second key for already verified user id of another key', async () => {
+    it('should verify a second key for an already verified user id and delete the old key', async () => {
       await publicKey.put({publicKeyArmored, origin});
-      expect(mailsSent.length).to.equal(4);
+      await publicKey.verify(mailsSent[1].params);
+      let firstKey = await publicKey.getVerified({keyId: mailsSent[1].params.keyId});
+      expect(firstKey).to.exist;
       await publicKey.put({publicKeyArmored: publicKeyArmored2, origin});
-      expect(mailsSent.length).to.equal(5);
       await publicKey.verify(mailsSent[4].params);
+      firstKey = await publicKey.getVerified({keyId: mailsSent[1].params.keyId});
+      expect(firstKey).to.not.exist;
+      const secondKey = await publicKey.getVerified({keyId: mailsSent[4].params.keyId});
+      expect(secondKey).to.exist;
+    });
 
-      try {
-        await publicKey.verify(mailsSent[0].params);
-        expect(true).to.be.false;
-      } catch (e) {
-        expect(e.status).to.equal(304);
-      }
-      const gotten = await mongo.get({keyId: mailsSent[0].params.keyId}, DB_TYPE);
-      expect(gotten.userIds[1].email).to.equal(primaryEmail2);
-      expect(gotten.userIds[1].verified).to.be.false;
-      expect(gotten.userIds[1].nonce).to.equal(mailsSent[1].params.nonce);
+    it('should delete other keys with the same user id when verifying', async () => {
+      await publicKey.put({publicKeyArmored, origin});
+      await publicKey.put({publicKeyArmored: publicKeyArmored2, origin});
+      expect(mailsSent[1].to).to.equal(mailsSent[4].to);
+      await publicKey.verify(mailsSent[1].params);
+      const firstKey = await publicKey.getVerified({keyId: mailsSent[1].params.keyId});
+      expect(firstKey).to.exist;
+      const secondKey = await mongo.get({keyId: mailsSent[4].params.keyId}, DB_TYPE);
+      expect(secondKey).to.not.exist;
     });
 
     it('should be able to verify multiple user ids', async () => {