From b738e1bc5c5a9f1443d69f5ac541cd76cbfea855 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Thu, 24 Aug 2017 16:36:32 +0800 Subject: [PATCH 1/3] =?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 () => { From 77fc0fd19577209b657a314783b2677b4f393d44 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Fri, 25 Aug 2017 16:11:35 +0800 Subject: [PATCH 2/3] Cleanup purge old keys --- src/service/public-key.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/service/public-key.js b/src/service/public-key.js index f4098c7..5c61eaf 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -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} - }; - return this._mongo.remove(query, DB_TYPE); + }, DB_TYPE); } /** From b93db84c6a1c2fe39ac0ca74f0a6ff91d708e5e8 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Fri, 25 Aug 2017 16:20:33 +0800 Subject: [PATCH 3/3] Optimize key removal during verification --- src/service/public-key.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/service/public-key.js b/src/service/public-key.js index 5c61eaf..4f78949 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -152,12 +152,10 @@ class PublicKey { } async _removeKeysWithSameEmail({keyId, userIds}, nonce) { - const {email} = userIds.find(uid => uid.nonce === nonce); - const keys = await this._mongo.list({ + return this._mongo.remove({ keyId: {$ne: keyId}, - 'userIds.email': email + 'userIds.email': userIds.find(u => u.nonce === nonce).email }, DB_TYPE); - await Promise.all(keys.map(({_id}) => this._mongo.remove({_id}, DB_TYPE))); } /**