Merge pull request #46 from mailvelope/dev/key-update

Allow update of an email address’ key with remove/verify flow in between

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
+    // check for existing verified key with same id
-    const verified = await this.getVerified(key);
+    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);
@@ -91,11 +91,10 @@ class PublicKey {
     const xDaysAgo = new Date();
     xDaysAgo.setDate(xDaysAgo.getDate() - config.publicKey.purgeTimeInDays);
     // remove unverified keys older than x days (or no 'uploaded' attribute)
-    const query = {
+    return this._mongo.remove({
       'userIds.verified': {$ne: true},
       uploaded: {$lt: xDaysAgo}
-    };
+    }, DB_TYPE);
-    return this._mongo.remove(query, DB_TYPE);
   }
 
   /**
@@ -144,11 +143,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
+    await this._removeKeysWithSameEmail(key, nonce);
-    const verified = await this.getVerified(key);
-    if (verified && verified.keyId !== keyId) {
-      util.throw(304, 'Key for this user already exists');
-    }
     // flag the user id as verified
     await this._mongo.update(query, {
       'userIds.$.verified': true,
@@ -156,6 +151,13 @@ class PublicKey {
     }, DB_TYPE);
   }
 
+  async _removeKeysWithSameEmail({keyId, userIds}, nonce) {
+    return this._mongo.remove({
+      keyId: {$ne: keyId},
+      'userIds.email': userIds.find(u => u.nonce === nonce).email
+    }, 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

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 {
+    it('should delete other keys with the same user id when verifying', async () => {
-        await publicKey.verify(mailsSent[0].params);
+      await publicKey.put({publicKeyArmored, origin});
-        expect(true).to.be.false;
+      await publicKey.put({publicKeyArmored: publicKeyArmored2, origin});
-      } catch (e) {
+      expect(mailsSent[1].to).to.equal(mailsSent[4].to);
-        expect(e.status).to.equal(304);
+      await publicKey.verify(mailsSent[1].params);
-      }
+      const firstKey = await publicKey.getVerified({keyId: mailsSent[1].params.keyId});
-      const gotten = await mongo.get({keyId: mailsSent[0].params.keyId}, DB_TYPE);
+      expect(firstKey).to.exist;
-      expect(gotten.userIds[1].email).to.equal(primaryEmail2);
+      const secondKey = await mongo.get({keyId: mailsSent[4].params.keyId}, DB_TYPE);
-      expect(gotten.userIds[1].verified).to.be.false;
+      expect(secondKey).to.not.exist;
-      expect(gotten.userIds[1].nonce).to.equal(mailsSent[1].params.nonce);
     });
 
     it('should be able to verify multiple user ids', async () => {