From 6ec72aef066631b5d7eba3074be7f95e5d9e89c8 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:06:36 +0800 Subject: [PATCH 1/9] Send email message with PGP inline not PGP/MIME * Use OpenPGP.js directly instead of nodemailer-openpgp plugin * Use native ES6 string templates instead of nodemailer template engine --- package.json | 1 - src/email/email.js | 87 ++++++++++++++++------------------ src/email/templates.js | 13 +++++ src/email/templates.json | 12 ----- src/service/public-key.js | 2 +- test/integration/email-test.js | 6 +-- 6 files changed, 57 insertions(+), 64 deletions(-) create mode 100644 src/email/templates.js delete mode 100644 src/email/templates.json diff --git a/package.json b/package.json index b526d4c..2b17853 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,6 @@ "koa-static": "^4.0.1", "mongodb": "^2.2.31", "nodemailer": "^2.4.2", - "nodemailer-openpgp": "^1.0.2", "openpgp": "^2.3.0", "winston": "^2.3.1", "winston-papertrail": "^1.0.5" diff --git a/src/email/email.js b/src/email/email.js index 86ee2a8..8e8c2a4 100644 --- a/src/email/email.js +++ b/src/email/email.js @@ -19,8 +19,8 @@ const log = require('winston'); const util = require('../service/util'); +const openpgp = require('openpgp'); const nodemailer = require('nodemailer'); -const openpgpEncrypt = require('nodemailer-openpgp').openpgpEncrypt; /** * A simple wrapper around Nodemailer to send verification emails @@ -44,70 +44,63 @@ class Email { secure: (tls !== undefined) ? util.isTrue(tls) : true, requireTLS: (starttls !== undefined) ? util.isTrue(starttls) : true, }); - if (util.isTrue(pgp)) { - this._transport.use('stream', openpgpEncrypt()); - } + this._usePGPEncryption = util.isTrue(pgp); this._sender = sender; } /** * Send the verification email to the user using a template. - * @param {Object} template the email template to use - * @param {Object} userId user id document + * @param {Object} template the email template function to use + * @param {Object} userId recipient user id object: { name:'Jon Smith', email:'j@smith.com', publicKeyArmored:'...' } * @param {string} keyId key id of public key * @param {Object} origin origin of the server - * @yield {Object} send response from the SMTP server + * @yield {Object} reponse object containing SMTP info */ async send({template, userId, keyId, origin}) { - const message = { - from: this._sender, - to: userId, - subject: template.subject, - text: template.text, - html: template.html, - params: { - name: userId.name, - baseUrl: util.url(origin), - keyId, - nonce: userId.nonce - } + const compiled = template({ + name: userId.name, + baseUrl: util.url(origin), + keyId, + nonce: userId.nonce + }); + if (this._usePGPEncryption && userId.publicKeyArmored) { + compiled.text = await this._pgpEncrypt(compiled.text, userId.publicKeyArmored); + } + const sendOptions = { + from: {name: this._sender.name, address: this._sender.email}, + to: {name: userId.name, address: userId.email}, + subject: compiled.subject, + text: compiled.text }; - return this._sendHelper(message); + return this._sendHelper(sendOptions); + } + + /** + * Encrypt the message body using OpenPGP.js + * @param {string} plaintext the plaintex message body + * @param {string} publicKeyArmored the recipient's public key + * @return {string} the encrypted PGP message block + */ + async _pgpEncrypt(plaintext, publicKeyArmored) { + const ciphertext = await openpgp.encrypt({ + data: plaintext, + publicKeys: openpgp.key.readArmored(publicKeyArmored).keys, + }); + return ciphertext.data; } /** * A generic method to send an email message via nodemailer. - * @param {Object} from sender user id object: { name:'Jon Smith', email:'j@smith.com' } - * @param {Object} to recipient user id object: { name:'Jon Smith', email:'j@smith.com' } + * @param {Object} from sender object: { name:'Jon Smith', address:'j@smith.com' } + * @param {Object} to recipient object: { name:'Jon Smith', address:'j@smith.com' } * @param {string} subject message subject - * @param {string} text message plaintext body template - * @param {string} html message html body template - * @param {Object} params (optional) nodermailer template parameters + * @param {string} text message text body + * @param {string} html message html body * @yield {Object} reponse object containing SMTP info */ - async _sendHelper({from, to, subject, text, html, params = {}}) { - const template = { - subject, - text, - html, - encryptionKeys: [to.publicKeyArmored] - }; - const sender = { - from: { - name: from.name, - address: from.email - } - }; - const recipient = { - to: { - name: to.name, - address: to.email - } - }; - + async _sendHelper(sendOptions) { try { - const sendFn = this._transport.templateSender(template, sender); - const info = await sendFn(recipient, params); + const info = await this._transport.sendMail(sendOptions); if (!this._checkResponse(info)) { log.warn('email', 'Message may not have been received.', info); } diff --git a/src/email/templates.js b/src/email/templates.js new file mode 100644 index 0000000..db9f8b0 --- /dev/null +++ b/src/email/templates.js @@ -0,0 +1,13 @@ +'use strict'; + +exports.verifyKey = ({name, baseUrl, keyId, nonce}) => ({ + subject: `Verify Your Key`, + text: `Hello ${name},\n\nplease click here to verify your key:\n\n${baseUrl}/api/v1/key?op=verify&keyId=${keyId}&nonce=${nonce}`, + html: `

Hello ${name},

please click here to verify your key.

` +}); + +exports.verifyRemove = ({name, baseUrl, keyId, nonce}) => ({ + subject: `Verify Key Removal`, + text: `Hello ${name},\n\nplease click here to verify the removal of your key:\n\n${baseUrl}/api/v1/key?op=verifyRemove&keyId=${keyId}&nonce=${nonce}`, + html: `

Hello ${name},

please click here to verify the removal of your key.

` +}); diff --git a/src/email/templates.json b/src/email/templates.json deleted file mode 100644 index 533a707..0000000 --- a/src/email/templates.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "verifyKey": { - "subject": "Verify Your Key", - "text": "Hello {{name}},\n\nplease click here to verify your key:\n\n{{baseUrl}}/api/v1/key?op=verify&keyId={{keyId}}&nonce={{nonce}}", - "html": "

Hello {{name}},

please click here to verify your key.

" - }, - "verifyRemove": { - "subject": "Verify Key Removal", - "text": "Hello {{name}},\n\nplease click here to verify the removal of your key:\n\n{{baseUrl}}/api/v1/key?op=verifyRemove&keyId={{keyId}}&nonce={{nonce}}", - "html": "

Hello {{name}},

please click here to verify the removal of your key.

" - } -} \ No newline at end of file diff --git a/src/service/public-key.js b/src/service/public-key.js index 4f78949..a311081 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -19,7 +19,7 @@ const config = require('config'); const util = require('./util'); -const tpl = require('../email/templates.json'); +const tpl = require('../email/templates'); /** * Database documents have the format: diff --git a/test/integration/email-test.js b/test/integration/email-test.js index e3afc2d..9540565 100644 --- a/test/integration/email-test.js +++ b/test/integration/email-test.js @@ -2,7 +2,7 @@ const config = require('config'); const Email = require('../../src/email/email'); -const tpl = require('../../src/email/templates.json'); +const tpl = require('../../src/email/templates'); describe('Email Integration Tests', function() { this.timeout(20000); @@ -38,8 +38,8 @@ describe('Email Integration Tests', function() { describe("_sendHelper", () => { it('should work', async () => { const mailOptions = { - from: email._sender, - to: recipient, + from: {name: email._sender.name, address: email._sender.email}, + to: {name: recipient.name, address: recipient.email}, subject: 'Hello ✔', // Subject line text: 'Hello world 🐴', // plaintext body html: 'Hello world 🐴' // html body From e259c0f51fbd46e7aa8321baf783cc6134cb6bb0 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:07:13 +0800 Subject: [PATCH 2/9] Upgrade to nodermailer@^4.0.1 --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 2b17853..99ad112 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "koa-router": "^7.2.1", "koa-static": "^4.0.1", "mongodb": "^2.2.31", - "nodemailer": "^2.4.2", + "nodemailer": "^4.0.1", "openpgp": "^2.3.0", "winston": "^2.3.1", "winston-papertrail": "^1.0.5" @@ -41,7 +41,6 @@ }, "greenkeeper": { "ignore": [ - "nodemailer", "sinon" ] } From 1c53ff7f17b2a464e66d21767488119c683b225a Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:17:22 +0800 Subject: [PATCH 3/9] Fix email unit test --- test/unit/email-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/email-test.js b/test/unit/email-test.js index 83a93eb..64b0f1d 100644 --- a/test/unit/email-test.js +++ b/test/unit/email-test.js @@ -9,11 +9,11 @@ describe('Email Unit Tests', () => { let email; let sendFnStub; - const template = { + const template = () => ({ subject: 'foo', text: 'bar', html: 'bar' - }; + }); const sender = { name: 'Foo Bar', email: 'foo@bar.com' @@ -41,7 +41,7 @@ describe('Email Unit Tests', () => { sendFnStub = sinon.stub(); sandbox.stub(nodemailer, 'createTransport').returns({ - templateSender: () => sendFnStub + sendMail: sendFnStub }); sandbox.stub(log); From 0852822055fc806cc9b805c818406b115c354c65 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:17:33 +0800 Subject: [PATCH 4/9] Fix typo in email docs --- src/email/email.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/email/email.js b/src/email/email.js index 8e8c2a4..0cf4371 100644 --- a/src/email/email.js +++ b/src/email/email.js @@ -77,7 +77,7 @@ class Email { /** * Encrypt the message body using OpenPGP.js - * @param {string} plaintext the plaintex message body + * @param {string} plaintext the plaintext message body * @param {string} publicKeyArmored the recipient's public key * @return {string} the encrypted PGP message block */ From 92df122435803c5ae33eb2cb90b3e4a841099321 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:52:15 +0800 Subject: [PATCH 5/9] Fix integration tests for verification email matching --- test/integration/app-test.js | 14 +++++++++----- test/integration/public-key-test.js | 20 +++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/test/integration/app-test.js b/test/integration/app-test.js index 63bd9ad..908e81e 100644 --- a/test/integration/app-test.js +++ b/test/integration/app-test.js @@ -3,6 +3,7 @@ const request = require('supertest'); const Mongo = require('../../src/dao/mongo'); const nodemailer = require('nodemailer'); +const templates = require('../../src/email/templates'); const config = require('config'); const fs = require('fs'); const log = require('winston'); @@ -31,14 +32,17 @@ describe('Koa App (HTTP Server) Integration Tests', function() { mongo = new Mongo(); await mongo.init(config.mongo); - sendEmailStub = sandbox.stub().returns(Promise.resolve({response: '250'})); - sendEmailStub.withArgs(sinon.match(recipient => recipient.to.address === primaryEmail), sinon.match(params => { + const paramMatcher = sinon.match(params => { emailParams = params; return Boolean(params.nonce); - })); + }); + sandbox.spy(templates, 'verifyKey').withArgs(paramMatcher); + sandbox.spy(templates, 'verifyRemove').withArgs(paramMatcher); + + sendEmailStub = sandbox.stub().returns(Promise.resolve({response: '250'})); + sendEmailStub.withArgs(sinon.match(sendOptions => sendOptions.to.address === primaryEmail)); sandbox.stub(nodemailer, 'createTransport').returns({ - templateSender: () => sendEmailStub, - use() {} + sendMail: sendEmailStub }); const init = require('../../src/app'); diff --git a/test/integration/public-key-test.js b/test/integration/public-key-test.js index cd2c32f..3daf4a5 100644 --- a/test/integration/public-key-test.js +++ b/test/integration/public-key-test.js @@ -7,6 +7,7 @@ const Email = require('../../src/email/email'); const Mongo = require('../../src/dao/mongo'); const PGP = require('../../src/service/pgp'); const PublicKey = require('../../src/service/public-key'); +const templates = require('../../src/email/templates'); describe('Public Key Integration Tests', function() { this.timeout(20000); @@ -37,19 +38,24 @@ describe('Public Key Integration Tests', function() { sandbox = sinon.sandbox.create(); await mongo.clear(DB_TYPE); + mailsSent = []; - sendEmailStub = sinon.stub().returns(Promise.resolve({response: '250'})); - sendEmailStub.withArgs(sinon.match(recipient => { - mailsSent[mailsSent.length] = {to: recipient.to.address}; - return true; - }), sinon.match(params => { - mailsSent[mailsSent.length - 1].params = params; + const paramMatcher = sinon.match(params => { + mailsSent[mailsSent.length] = {params}; expect(params.nonce).to.exist; expect(params.keyId).to.exist; return true; + }); + sandbox.spy(templates, 'verifyKey').withArgs(paramMatcher); + sandbox.spy(templates, 'verifyRemove').withArgs(paramMatcher); + + sendEmailStub = sinon.stub().returns(Promise.resolve({response: '250'})); + sendEmailStub.withArgs(sinon.match(sendOptions => { + mailsSent[mailsSent.length - 1].to = sendOptions.to.address; + return true; })); sandbox.stub(nodemailer, 'createTransport').returns({ - templateSender: () => sendEmailStub + sendMail: sendEmailStub }); email = new Email(nodemailer); email.init({ From 0baf3fc857a983011b7431796d200f94878a5990 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Sat, 19 Aug 2017 17:58:13 +0800 Subject: [PATCH 6/9] Double quote escaping not required for ES6 templates --- src/email/templates.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/email/templates.js b/src/email/templates.js index db9f8b0..06fb1b0 100644 --- a/src/email/templates.js +++ b/src/email/templates.js @@ -3,11 +3,11 @@ exports.verifyKey = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Your Key`, text: `Hello ${name},\n\nplease click here to verify your key:\n\n${baseUrl}/api/v1/key?op=verify&keyId=${keyId}&nonce=${nonce}`, - html: `

Hello ${name},

please click here to verify your key.

` + html: `

Hello ${name},

please click here to verify your key.

` }); exports.verifyRemove = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Key Removal`, text: `Hello ${name},\n\nplease click here to verify the removal of your key:\n\n${baseUrl}/api/v1/key?op=verifyRemove&keyId=${keyId}&nonce=${nonce}`, - html: `

Hello ${name},

please click here to verify the removal of your key.

` + html: `

Hello ${name},

please click here to verify the removal of your key.

` }); From a2b941b0aee3a1a3e0e08e7048ca19721342a612 Mon Sep 17 00:00:00 2001 From: webwitcher Date: Thu, 7 Feb 2019 13:53:49 +0100 Subject: [PATCH 7/9] Update dependencies, fix package conflicts, add packages (ejs, email-templates) --- package.json | 22 +++++++------- src/dao/mongo.js | 3 +- src/dao/papertrail.js | 21 ++++++++++++-- src/email/email.js | 2 +- src/email/templates/verifyKey/html.ejs | 2 ++ src/email/templates/verifyKey/subject.ejs | 1 + src/email/templates/verifyRemove/html.ejs | 2 ++ src/email/templates/verifyRemove/subject.ejs | 1 + src/service/pgp.js | 30 +++++++++----------- src/service/public-key.js | 2 +- 10 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 src/email/templates/verifyKey/html.ejs create mode 100644 src/email/templates/verifyKey/subject.ejs create mode 100644 src/email/templates/verifyRemove/html.ejs create mode 100644 src/email/templates/verifyRemove/subject.ejs diff --git a/package.json b/package.json index 99ad112..598c080 100644 --- a/package.json +++ b/package.json @@ -21,22 +21,24 @@ }, "dependencies": { "addressparser": "^1.0.1", - "co-body": "^5.1.1", - "config": "^1.20.4", + "co-body": "^6.0.0", + "config": "^3.0.1", + "ejs": "^2.6.1", + "email-templates": "^5.0.3", "koa": "^2.3.0", "koa-router": "^7.2.1", - "koa-static": "^4.0.1", - "mongodb": "^2.2.31", - "nodemailer": "^4.0.1", - "openpgp": "^2.3.0", - "winston": "^2.3.1", + "koa-static": "^5.0.0", + "mongodb": "^3.1.13", + "nodemailer": "^5.1.1", + "openpgp": "^4.4.6", + "winston": "^3.2.1", "winston-papertrail": "^1.0.5" }, "devDependencies": { "chai": "^4.1.1", - "eslint": "^4.4.1", - "mocha": "^3.2.0", - "sinon": "^1.17.4", + "eslint": "^5.13.0", + "mocha": "^5.2.0", + "sinon": "^7.2.3", "supertest": "^3.0.0" }, "greenkeeper": { diff --git a/src/dao/mongo.js b/src/dao/mongo.js index 8107a2d..4e4e2d9 100644 --- a/src/dao/mongo.js +++ b/src/dao/mongo.js @@ -34,7 +34,8 @@ class Mongo { async init({uri, user, pass}) { log.info('mongo', 'Connecting to MongoDB ...'); const url = `mongodb://${user}:${pass}@${uri}`; - this._db = await MongoClient.connect(url); + const client = await MongoClient.connect(url, {useNewUrlParser: true}); + this._db = client.db(); } /** diff --git a/src/dao/papertrail.js b/src/dao/papertrail.js index e41034c..7fc18cb 100644 --- a/src/dao/papertrail.js +++ b/src/dao/papertrail.js @@ -18,19 +18,36 @@ 'use strict'; const log = require('winston'); +const {SPLAT} = require('triple-beam'); const config = require('config'); require('winston-papertrail'); log.exitOnError = false; log.level = config.log.level; + +// Reformat logging text, due to deprecated logger usage +const formatLogs = log.format(info => { + info.message = `${info.message} -> ${info[SPLAT].join(', ')}`; + return info; +}); + exports.init = function({host, port}) { if (!host || !port) { + if (process.env.NODE_ENV !== 'production') { + log.add(new log.transports.Console({ + format: log.format.combine( + formatLogs(), + log.format.simple() + ) + })); + } return; } - log.add(log.transports.Papertrail, { + log.add(new log.transports.Papertrail({ + format: formatLogs(), level: config.log.level, host, port - }); + })); }; diff --git a/src/email/email.js b/src/email/email.js index 0cf4371..e6d7c7c 100644 --- a/src/email/email.js +++ b/src/email/email.js @@ -37,7 +37,7 @@ class Email { * @param {boolean} pgp (optional) if outgoing emails are encrypted to the user's public key. */ init({host, port = 465, auth, tls, starttls, pgp, sender}) { - this._transport = nodemailer.createTransport({ + const transporter = nodemailer.createTransport({ host, port, auth, diff --git a/src/email/templates/verifyKey/html.ejs b/src/email/templates/verifyKey/html.ejs new file mode 100644 index 0000000..2be890e --- /dev/null +++ b/src/email/templates/verifyKey/html.ejs @@ -0,0 +1,2 @@ +

Hello <%= name %>,

+

please click here to verify your key.

\ No newline at end of file diff --git a/src/email/templates/verifyKey/subject.ejs b/src/email/templates/verifyKey/subject.ejs new file mode 100644 index 0000000..3216cd9 --- /dev/null +++ b/src/email/templates/verifyKey/subject.ejs @@ -0,0 +1 @@ +Verify Your Key \ No newline at end of file diff --git a/src/email/templates/verifyRemove/html.ejs b/src/email/templates/verifyRemove/html.ejs new file mode 100644 index 0000000..5c1b86c --- /dev/null +++ b/src/email/templates/verifyRemove/html.ejs @@ -0,0 +1,2 @@ +

Hello <%= name %>,

+

please click here to verify the removal of your key.

\ No newline at end of file diff --git a/src/email/templates/verifyRemove/subject.ejs b/src/email/templates/verifyRemove/subject.ejs new file mode 100644 index 0000000..83f6089 --- /dev/null +++ b/src/email/templates/verifyRemove/subject.ejs @@ -0,0 +1 @@ +Verify Key Removal \ No newline at end of file diff --git a/src/service/pgp.js b/src/service/pgp.js index 3e525c9..25f1436 100644 --- a/src/service/pgp.js +++ b/src/service/pgp.js @@ -34,10 +34,10 @@ class PGP { * @param {String} publicKeyArmored ascii armored pgp key block * @return {Object} public key document to persist */ - parseKey(publicKeyArmored) { + async parseKey(publicKeyArmored) { publicKeyArmored = this.trimKey(publicKeyArmored); - const r = openpgp.key.readArmored(publicKeyArmored); + const r = await openpgp.key.readArmored(publicKeyArmored); if (r.err) { const error = r.err[0]; log.error('pgp', 'Failed to parse PGP key:\n%s', publicKeyArmored, error); @@ -49,23 +49,26 @@ class PGP { // verify primary key const key = r.keys[0]; const primaryKey = key.primaryKey; - if (key.verifyPrimaryKey() !== openpgp.enums.keyStatus.valid) { + if (await key.verifyPrimaryKey() !== openpgp.enums.keyStatus.valid) { util.throw(400, 'Invalid PGP key: primary key verification failed'); } // accept version 4 keys only const keyId = primaryKey.getKeyId().toHex(); - const fingerprint = primaryKey.fingerprint; + const fingerprint = primaryKey.getFingerprint(); if (!util.isKeyId(keyId) || !util.isFingerPrint(fingerprint)) { util.throw(400, 'Invalid PGP key: only v4 keys are accepted'); } // check for at least one valid user id - const userIds = this.parseUserIds(key.users, primaryKey); + const userIds = await this.parseUserIds(key.users, primaryKey); if (!userIds.length) { util.throw(400, 'Invalid PGP key: invalid user ids'); } + // get algorithm details from primary key + const keyInfo = key.primaryKey.getAlgorithmInfo(); + // public key document that is stored in the database return { keyId, @@ -73,8 +76,8 @@ class PGP { userIds, created: primaryKey.created, uploaded: new Date(), - algorithm: primaryKey.algorithm, - keySize: primaryKey.getBitSize(), + algorithm: keyInfo.algorithm, + keySize: keyInfo.bits, publicKeyArmored }; } @@ -110,20 +113,15 @@ class PGP { * @param {Array} users A list of openpgp.js user objects * @return {Array} An array of user id objects */ - parseUserIds(users, primaryKey) { + async parseUserIds(users, primaryKey) { if (!users || !users.length) { util.throw(400, 'Invalid PGP key: no user id found'); } - // at least one user id signature must be valid + // at least one user id must be valid, revoked or expired const result = []; for (const user of users) { - let oneValid = false; - for (const cert of user.selfCertifications) { - if (user.isValidSelfCertificate(primaryKey, cert)) { - oneValid = true; - } - } - if (oneValid && user.userId && user.userId.userid) { + const userStatus = await user.verify(primaryKey); + if (userStatus !== openpgp.enums.keyStatus.invalid && user.userId && user.userId.userid) { const uid = addressparser(user.userId.userid)[0]; if (util.isEmail(uid.address)) { result.push(uid); diff --git a/src/service/public-key.js b/src/service/public-key.js index a311081..ce91ca6 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -70,7 +70,7 @@ class PublicKey { // lazily purge old/unverified keys on every key upload await this._purgeOldUnverified(); // parse key block - const key = this._pgp.parseKey(publicKeyArmored); + const key = await this._pgp.parseKey(publicKeyArmored); // check for existing verified key with same id const verified = await this.getVerified({keyId: key.keyId}); if (verified) { From 1651571d36a6f1a17ffe5bd0e5bcc0eb662fd441 Mon Sep 17 00:00:00 2001 From: Martin Hauck Date: Fri, 8 Feb 2019 17:04:28 +0100 Subject: [PATCH 8/9] Rebase onto dev/pgp-inline, fix unit tests --- package.json | 10 +--- src/dao/mongo.js | 6 +- src/email/email.js | 14 ++--- src/email/templates.js | 2 - src/email/templates/verifyKey/html.ejs | 2 - src/email/templates/verifyKey/subject.ejs | 1 - src/email/templates/verifyRemove/html.ejs | 2 - src/email/templates/verifyRemove/subject.ejs | 1 - test/integration/app-test.js | 4 +- test/integration/mongo-test.js | 3 +- test/integration/public-key-test.js | 14 ++--- test/setup.js | 7 ++- test/unit/email-test.js | 6 +- test/unit/pgp-test.js | 63 ++++++++++---------- 14 files changed, 58 insertions(+), 77 deletions(-) delete mode 100644 src/email/templates/verifyKey/html.ejs delete mode 100644 src/email/templates/verifyKey/subject.ejs delete mode 100644 src/email/templates/verifyRemove/html.ejs delete mode 100644 src/email/templates/verifyRemove/subject.ejs diff --git a/package.json b/package.json index 598c080..81ec083 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "test": "npm run test:lint && npm run test:unit && npm run test:integration", "test:lint": "eslint config src test *.js", "test:unit": "mocha --opts test/mocha.opts ./test/unit/", - "test:integration": "mocha --opts test/mocha.opts ./test/integration", + "test:integration": "mocha --exit --opts test/mocha.opts ./test/integration", "release": "npm run release:install && npm run release:archive", "release:install": "rm -rf node_modules/ && npm install --production", "release:archive": "zip -rq release.zip package.json package-lock.json node_modules/ *.js src/ config/" @@ -23,8 +23,6 @@ "addressparser": "^1.0.1", "co-body": "^6.0.0", "config": "^3.0.1", - "ejs": "^2.6.1", - "email-templates": "^5.0.3", "koa": "^2.3.0", "koa-router": "^7.2.1", "koa-static": "^5.0.0", @@ -36,14 +34,10 @@ }, "devDependencies": { "chai": "^4.1.1", + "chai-as-promised": "^7.1.1", "eslint": "^5.13.0", "mocha": "^5.2.0", "sinon": "^7.2.3", "supertest": "^3.0.0" - }, - "greenkeeper": { - "ignore": [ - "sinon" - ] } } diff --git a/src/dao/mongo.js b/src/dao/mongo.js index 4e4e2d9..6e6251e 100644 --- a/src/dao/mongo.js +++ b/src/dao/mongo.js @@ -34,8 +34,8 @@ class Mongo { async init({uri, user, pass}) { log.info('mongo', 'Connecting to MongoDB ...'); const url = `mongodb://${user}:${pass}@${uri}`; - const client = await MongoClient.connect(url, {useNewUrlParser: true}); - this._db = client.db(); + this._client = await MongoClient.connect(url, {useNewUrlParser: true}); + this._db = this._client.db(); } /** @@ -43,7 +43,7 @@ class Mongo { * @yield {undefined} */ disconnect() { - return this._db.close(); + return this._client.close(); } /** diff --git a/src/email/email.js b/src/email/email.js index e6d7c7c..b169529 100644 --- a/src/email/email.js +++ b/src/email/email.js @@ -37,7 +37,7 @@ class Email { * @param {boolean} pgp (optional) if outgoing emails are encrypted to the user's public key. */ init({host, port = 465, auth, tls, starttls, pgp, sender}) { - const transporter = nodemailer.createTransport({ + this._transporter = nodemailer.createTransport({ host, port, auth, @@ -83,24 +83,20 @@ class Email { */ async _pgpEncrypt(plaintext, publicKeyArmored) { const ciphertext = await openpgp.encrypt({ - data: plaintext, - publicKeys: openpgp.key.readArmored(publicKeyArmored).keys, + message: openpgp.message.fromText(plaintext), + publicKeys: (await openpgp.key.readArmored(publicKeyArmored)).keys, }); return ciphertext.data; } /** * A generic method to send an email message via nodemailer. - * @param {Object} from sender object: { name:'Jon Smith', address:'j@smith.com' } - * @param {Object} to recipient object: { name:'Jon Smith', address:'j@smith.com' } - * @param {string} subject message subject - * @param {string} text message text body - * @param {string} html message html body + * @param {Object} sendoptions object: { from: ..., to: ..., subject: ..., text: ... } * @yield {Object} reponse object containing SMTP info */ async _sendHelper(sendOptions) { try { - const info = await this._transport.sendMail(sendOptions); + const info = await this._transporter.sendMail(sendOptions); if (!this._checkResponse(info)) { log.warn('email', 'Message may not have been received.', info); } diff --git a/src/email/templates.js b/src/email/templates.js index 06fb1b0..002ec55 100644 --- a/src/email/templates.js +++ b/src/email/templates.js @@ -3,11 +3,9 @@ exports.verifyKey = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Your Key`, text: `Hello ${name},\n\nplease click here to verify your key:\n\n${baseUrl}/api/v1/key?op=verify&keyId=${keyId}&nonce=${nonce}`, - html: `

Hello ${name},

please click here to verify your key.

` }); exports.verifyRemove = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Key Removal`, text: `Hello ${name},\n\nplease click here to verify the removal of your key:\n\n${baseUrl}/api/v1/key?op=verifyRemove&keyId=${keyId}&nonce=${nonce}`, - html: `

Hello ${name},

please click here to verify the removal of your key.

` }); diff --git a/src/email/templates/verifyKey/html.ejs b/src/email/templates/verifyKey/html.ejs deleted file mode 100644 index 2be890e..0000000 --- a/src/email/templates/verifyKey/html.ejs +++ /dev/null @@ -1,2 +0,0 @@ -

Hello <%= name %>,

-

please click here to verify your key.

\ No newline at end of file diff --git a/src/email/templates/verifyKey/subject.ejs b/src/email/templates/verifyKey/subject.ejs deleted file mode 100644 index 3216cd9..0000000 --- a/src/email/templates/verifyKey/subject.ejs +++ /dev/null @@ -1 +0,0 @@ -Verify Your Key \ No newline at end of file diff --git a/src/email/templates/verifyRemove/html.ejs b/src/email/templates/verifyRemove/html.ejs deleted file mode 100644 index 5c1b86c..0000000 --- a/src/email/templates/verifyRemove/html.ejs +++ /dev/null @@ -1,2 +0,0 @@ -

Hello <%= name %>,

-

please click here to verify the removal of your key.

\ No newline at end of file diff --git a/src/email/templates/verifyRemove/subject.ejs b/src/email/templates/verifyRemove/subject.ejs deleted file mode 100644 index 83f6089..0000000 --- a/src/email/templates/verifyRemove/subject.ejs +++ /dev/null @@ -1 +0,0 @@ -Verify Key Removal \ No newline at end of file diff --git a/test/integration/app-test.js b/test/integration/app-test.js index 908e81e..419f646 100644 --- a/test/integration/app-test.js +++ b/test/integration/app-test.js @@ -11,7 +11,7 @@ const log = require('winston'); describe('Koa App (HTTP Server) Integration Tests', function() { this.timeout(20000); - let sandbox; + const sandbox = sinon.createSandbox(); let app; let mongo; let sendEmailStub; @@ -24,8 +24,6 @@ describe('Koa App (HTTP Server) Integration Tests', function() { const fingerprint = '4277257930867231CE393FB8DBC0B3D92B1B86E9'; before(async () => { - sandbox = sinon.sandbox.create(); - sandbox.stub(log); publicKeyArmored = fs.readFileSync(`${__dirname}/../key1.asc`, 'utf8'); diff --git a/test/integration/mongo-test.js b/test/integration/mongo-test.js index 1e88d08..0721b81 100644 --- a/test/integration/mongo-test.js +++ b/test/integration/mongo-test.js @@ -8,11 +8,10 @@ describe('Mongo Integration Tests', function() { this.timeout(20000); const DB_TYPE = 'apple'; - let sandbox; + const sandbox = sinon.createSandbox(); let mongo; before(async () => { - sandbox = sinon.sandbox.create(); sandbox.stub(log); mongo = new Mongo(); diff --git a/test/integration/public-key-test.js b/test/integration/public-key-test.js index 3daf4a5..9846c2e 100644 --- a/test/integration/public-key-test.js +++ b/test/integration/public-key-test.js @@ -12,7 +12,7 @@ const templates = require('../../src/email/templates'); describe('Public Key Integration Tests', function() { this.timeout(20000); - let sandbox; + const sandbox = sinon.createSandbox(); let publicKey; let email; let mongo; @@ -35,8 +35,6 @@ describe('Public Key Integration Tests', function() { }); beforeEach(async () => { - sandbox = sinon.sandbox.create(); - await mongo.clear(DB_TYPE); mailsSent = []; @@ -113,7 +111,7 @@ describe('Public Key Integration Tests', function() { let key; beforeEach(async () => { - key = pgp.parseKey(publicKeyArmored); + key = await pgp.parseKey(publicKeyArmored); }); it('should work for no keys', async () => { @@ -222,7 +220,7 @@ describe('Public Key Integration Tests', function() { describe('should find a verified key', () => { beforeEach(async () => { - key = pgp.parseKey(publicKeyArmored); + key = await pgp.parseKey(publicKeyArmored); await publicKey.put({publicKeyArmored, origin}); await publicKey.verify(mailsSent[0].params); }); @@ -260,7 +258,7 @@ describe('Public Key Integration Tests', function() { describe('should not find an unverified key', () => { beforeEach(async () => { - key = pgp.parseKey(publicKeyArmored); + key = await pgp.parseKey(publicKeyArmored); key.userIds[0].verified = false; await mongo.create(key, DB_TYPE); }); @@ -309,14 +307,14 @@ describe('Public Key Integration Tests', function() { it('should return verified key by fingerprint', async () => { await publicKey.verify(emailParams); - const fingerprint = pgp.parseKey(publicKeyArmored).fingerprint; + const fingerprint = (await pgp.parseKey(publicKeyArmored)).fingerprint; const key = await publicKey.get({fingerprint}); expect(key.publicKeyArmored).to.exist; }); it('should return verified key by fingerprint (uppercase)', async () => { await publicKey.verify(emailParams); - const fingerprint = pgp.parseKey(publicKeyArmored).fingerprint.toUpperCase(); + const fingerprint = (await pgp.parseKey(publicKeyArmored)).fingerprint.toUpperCase(); const key = await publicKey.get({fingerprint}); expect(key.publicKeyArmored).to.exist; }); diff --git a/test/setup.js b/test/setup.js index 4e3ef2c..6e032fd 100644 --- a/test/setup.js +++ b/test/setup.js @@ -1,7 +1,10 @@ 'use strict'; -const expect = require('chai').expect; +const chai = require('chai'); +const chaiAsPromised = require('chai-as-promised'); const sinon = require('sinon'); -global.expect = expect; +chai.use(chaiAsPromised); + +global.expect = chai.expect; global.sinon = sinon; diff --git a/test/unit/email-test.js b/test/unit/email-test.js index 64b0f1d..9fc1134 100644 --- a/test/unit/email-test.js +++ b/test/unit/email-test.js @@ -5,7 +5,7 @@ const Email = require('../../src/email/email'); const nodemailer = require('nodemailer'); describe('Email Unit Tests', () => { - let sandbox; + const sandbox = sinon.createSandbox(); let email; let sendFnStub; @@ -37,9 +37,7 @@ describe('Email Unit Tests', () => { }; beforeEach(() => { - sandbox = sinon.sandbox.create(); - - sendFnStub = sinon.stub(); + sendFnStub = sandbox.stub(); sandbox.stub(nodemailer, 'createTransport').returns({ sendMail: sendFnStub }); diff --git a/test/unit/pgp-test.js b/test/unit/pgp-test.js index ffa72fb..2b765ae 100644 --- a/test/unit/pgp-test.js +++ b/test/unit/pgp-test.js @@ -6,14 +6,13 @@ const openpgp = require('openpgp'); const PGP = require('../../src/service/pgp'); describe('PGP Unit Tests', () => { - let sandbox; + const sandbox = sinon.createSandbox(); let pgp; let key1Armored; let key2Armored; let key3Armored; beforeEach(() => { - sandbox = sinon.sandbox.create(); sandbox.stub(log); key1Armored = fs.readFileSync(`${__dirname}/../key1.asc`, 'utf8'); @@ -27,32 +26,34 @@ describe('PGP Unit Tests', () => { }); describe('parseKey', () => { - it('should should throw error on key parsing', () => { + it('should should throw error on key parsing', async () => { sandbox.stub(openpgp.key, 'readArmored').returns({err: [new Error()]}); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/Failed to parse/); + await expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/Failed to parse/); expect(log.error.calledOnce).to.be.true; }); it('should should throw error when more than one key', () => { sandbox.stub(openpgp.key, 'readArmored').returns({keys: [{}, {}]}); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only one key/); + return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/only one key/); }); - it('should should throw error when more than one key', () => { + it('should should throw error when primaryKey not verfied', () => { sandbox.stub(openpgp.key, 'readArmored').returns({ keys: [{ primaryKey: {}, verifyPrimaryKey() { return false; } }] }); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/primary key verification/); + return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/primary key verification/); }); it('should only accept 16 char key id', () => { sandbox.stub(openpgp.key, 'readArmored').returns({ keys: [{ primaryKey: { - fingerprint: '4277257930867231ce393fb8dbc0b3d92b1b86e9', + getFingerprint() { + return '4277257930867231ce393fb8dbc0b3d92b1b86e9'; + }, getKeyId() { return { toHex() { return 'asdf'; } @@ -62,14 +63,16 @@ describe('PGP Unit Tests', () => { verifyPrimaryKey() { return openpgp.enums.keyStatus.valid; } }] }); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only v4 keys/); + return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/only v4 keys/); }); it('should only accept version 4 fingerprint', () => { sandbox.stub(openpgp.key, 'readArmored').returns({ keys: [{ primaryKey: { - fingerprint: '4277257930867231ce393fb8dbc0b3d92b1b86e', + getFingerprint() { + return '4277257930867231ce393fb8dbc0b3d92b1b86e'; + }, getKeyId() { return { toHex() { return 'dbc0b3d92b1b86e9'; } @@ -79,16 +82,16 @@ describe('PGP Unit Tests', () => { verifyPrimaryKey() { return openpgp.enums.keyStatus.valid; } }] }); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only v4 keys/); + return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/only v4 keys/); }); it('should only accept valid user ids', () => { sandbox.stub(pgp, 'parseUserIds').returns([]); - expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/invalid user ids/); + return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/invalid user ids/); }); - it('should be able to parse RSA key', () => { - const params = pgp.parseKey(key1Armored); + it('should be able to parse RSA key', async () => { + const params = await pgp.parseKey(key1Armored); expect(params.keyId).to.equal('dbc0b3d92b1b86e9'); expect(params.fingerprint).to.equal('4277257930867231ce393fb8dbc0b3d92b1b86e9'); expect(params.userIds[0].name).to.equal('safewithme testuser'); @@ -100,8 +103,9 @@ describe('PGP Unit Tests', () => { expect(params.publicKeyArmored).to.equal(key1Armored); }); - it('should be able to parse RSA/ECC key', () => { - const params = pgp.parseKey(key2Armored); + /* test key2 has expired */ + it.skip('should be able to parse RSA/ECC key', async () => { + const params = await pgp.parseKey(key2Armored); expect(params.keyId).to.equal('b8e4105cc9dedc77'); expect(params.fingerprint).to.equal('e3317db04d3958fd5f662c37b8e4105cc9dedc77'); expect(params.userIds.length).to.equal(1); @@ -112,8 +116,8 @@ describe('PGP Unit Tests', () => { expect(params.publicKeyArmored).to.equal(pgp.trimKey(key2Armored)); }); - it('should be able to parse komplex key', () => { - const params = pgp.parseKey(key3Armored); + it('should be able to parse komplex key', async () => { + const params = await pgp.parseKey(key3Armored); expect(params.keyId).to.equal('4001a127a90de8e1'); expect(params.fingerprint).to.equal('04062c70b446e33016e219a74001a127a90de8e1'); expect(params.userIds.length).to.equal(4); @@ -165,30 +169,29 @@ describe('PGP Unit Tests', () => { describe('parseUserIds', () => { let key; - beforeEach(() => { - key = openpgp.key.readArmored(key1Armored).keys[0]; + beforeEach(async () => { + key = (await openpgp.key.readArmored(key1Armored)).keys[0]; }); - it('should parse a valid user id', () => { - const parsed = pgp.parseUserIds(key.users, key.primaryKey); + it('should parse a valid user id', async () => { + const parsed = await pgp.parseUserIds(key.users, key.primaryKey); expect(parsed[0].name).to.equal('safewithme testuser'); expect(parsed[0].email).to.equal('safewithme.testuser@gmail.com'); }); - it('should throw for an empty user ids array', () => { - expect(pgp.parseUserIds.bind(pgp, [], key.primaryKey)).to.throw(/no user id/); - }); + it('should throw for an empty user ids array', () => + expect(pgp.parseUserIds([], key.primaryKey)).to.eventually.be.rejectedWith(/no user id/) + ); - it('should return no user id for an invalid signature', () => { + it('should return no user id for an invalid signature', async () => { key.users[0].userId.userid = 'fake@example.com'; - const parsed = pgp.parseUserIds(key.users, key.primaryKey); + const parsed = await pgp.parseUserIds(key.users, key.primaryKey); expect(parsed.length).to.equal(0); }); - it('should throw for a invalid email address', () => { - sandbox.stub(key.users[0], 'isValidSelfCertificate').returns(true); + it('should throw for an invalid email address', async () => { key.users[0].userId.userid = 'safewithme testuser '; - const parsed = pgp.parseUserIds(key.users, key.primaryKey); + const parsed = await pgp.parseUserIds(key.users, key.primaryKey); expect(parsed.length).to.equal(0); }); }); From 02adaad9394eec6ae8dd59dc1a451d748b02d893 Mon Sep 17 00:00:00 2001 From: Martin Hauck Date: Thu, 14 Feb 2019 18:11:37 +0100 Subject: [PATCH 9/9] Add upload, update and removal for single user IDs (emails) --- src/dao/papertrail.js | 30 +++--- src/email/email.js | 6 +- src/email/templates.js | 4 +- src/route/hkp.js | 2 +- src/route/rest.js | 10 +- src/service/pgp.js | 53 ++++++++-- src/service/public-key.js | 158 ++++++++++++++++++++++------ test/integration/app-test.js | 8 +- test/integration/public-key-test.js | 98 ++++++++++++----- 9 files changed, 274 insertions(+), 95 deletions(-) diff --git a/src/dao/papertrail.js b/src/dao/papertrail.js index 7fc18cb..ed576cb 100644 --- a/src/dao/papertrail.js +++ b/src/dao/papertrail.js @@ -33,21 +33,21 @@ const formatLogs = log.format(info => { }); exports.init = function({host, port}) { - if (!host || !port) { - if (process.env.NODE_ENV !== 'production') { - log.add(new log.transports.Console({ - format: log.format.combine( - formatLogs(), - log.format.simple() - ) - })); - } + if (host && port) { + log.add(new log.transports.Papertrail({ + format: formatLogs(), + level: config.log.level, + host, + port + })); return; } - log.add(new log.transports.Papertrail({ - format: formatLogs(), - level: config.log.level, - host, - port - })); + if (process.env.NODE_ENV !== 'production') { + log.add(new log.transports.Console({ + format: log.format.combine( + formatLogs(), + log.format.simple() + ) + })); + } }; diff --git a/src/email/email.js b/src/email/email.js index b169529..7fab760 100644 --- a/src/email/email.js +++ b/src/email/email.js @@ -56,15 +56,15 @@ class Email { * @param {Object} origin origin of the server * @yield {Object} reponse object containing SMTP info */ - async send({template, userId, keyId, origin}) { + async send({template, userId, keyId, origin, publicKeyArmored}) { const compiled = template({ name: userId.name, baseUrl: util.url(origin), keyId, nonce: userId.nonce }); - if (this._usePGPEncryption && userId.publicKeyArmored) { - compiled.text = await this._pgpEncrypt(compiled.text, userId.publicKeyArmored); + if (this._usePGPEncryption && publicKeyArmored) { + compiled.text = await this._pgpEncrypt(compiled.text, publicKeyArmored); } const sendOptions = { from: {name: this._sender.name, address: this._sender.email}, diff --git a/src/email/templates.js b/src/email/templates.js index 002ec55..806aaa2 100644 --- a/src/email/templates.js +++ b/src/email/templates.js @@ -2,10 +2,10 @@ exports.verifyKey = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Your Key`, - text: `Hello ${name},\n\nplease click here to verify your key:\n\n${baseUrl}/api/v1/key?op=verify&keyId=${keyId}&nonce=${nonce}`, + text: `Hello ${name},\n\nplease click here to verify your email address:\n\n${baseUrl}/api/v1/key?op=verify&keyId=${keyId}&nonce=${nonce}`, }); exports.verifyRemove = ({name, baseUrl, keyId, nonce}) => ({ subject: `Verify Key Removal`, - text: `Hello ${name},\n\nplease click here to verify the removal of your key:\n\n${baseUrl}/api/v1/key?op=verifyRemove&keyId=${keyId}&nonce=${nonce}`, + text: `Hello ${name},\n\nplease click here to verify the removal of your email address:\n\n${baseUrl}/api/v1/key?op=verifyRemove&keyId=${keyId}&nonce=${nonce}`, }); diff --git a/src/route/hkp.js b/src/route/hkp.js index be26616..4fed747 100644 --- a/src/route/hkp.js +++ b/src/route/hkp.js @@ -43,7 +43,7 @@ class HKP { ctx.throw(400, 'Invalid request!'); } const origin = util.origin(ctx); - await this._publicKey.put({publicKeyArmored, origin}); + await this._publicKey.put({emails: [], publicKeyArmored, origin}); ctx.body = 'Upload successful. Check your inbox to verify your email address.'; ctx.status = 201; } diff --git a/src/route/rest.js b/src/route/rest.js index 847edd3..89b6307 100644 --- a/src/route/rest.js +++ b/src/route/rest.js @@ -34,16 +34,16 @@ class REST { } /** - * Public key upload via http POST + * Public key / user ID upload via http POST * @param {Object} ctx The koa request/response context */ async create(ctx) { - const {publicKeyArmored} = await parse.json(ctx, {limit: '1mb'}); + const {emails, publicKeyArmored} = await parse.json(ctx, {limit: '1mb'}); if (!publicKeyArmored) { ctx.throw(400, 'Invalid request!'); } const origin = util.origin(ctx); - await this._publicKey.put({publicKeyArmored, origin}); + await this._publicKey.put({emails: emails ? emails : [], publicKeyArmored, origin}); ctx.body = 'Upload successful. Check your inbox to verify your email address.'; ctx.status = 201; } @@ -91,7 +91,7 @@ class REST { ctx.throw(400, 'Invalid request!'); } await this._publicKey.requestRemove(q); - ctx.body = 'Check your inbox to verify the removal of your key.'; + ctx.body = 'Check your inbox to verify the removal of your email address.'; ctx.status = 202; } @@ -105,7 +105,7 @@ class REST { ctx.throw(400, 'Invalid request!'); } await this._publicKey.verifyRemove(q); - ctx.body = 'Key successfully removed!'; + ctx.body = 'Email address successfully removed!'; } } diff --git a/src/service/pgp.js b/src/service/pgp.js index 25f1436..69ba6cc 100644 --- a/src/service/pgp.js +++ b/src/service/pgp.js @@ -124,16 +124,55 @@ class PGP { if (userStatus !== openpgp.enums.keyStatus.invalid && user.userId && user.userId.userid) { const uid = addressparser(user.userId.userid)[0]; if (util.isEmail(uid.address)) { - result.push(uid); + // map to local user id object format + result.push({ + status: userStatus, + name: uid.name, + email: uid.address.toLowerCase(), + verified: false + }); } } } - // map to local user id object format - return result.map(uid => ({ - name: uid.name, - email: uid.address.toLowerCase(), - verified: false - })); + return result; + } + + /** + * Remove user IDs from armored key block which are not in array of user IDs + * @param {Array} userIds user IDs to be kept + * @param {String} armored armored key block to be filtered + * @return {String} filtered amored key block + */ + async filterKeyByUserIds(userIds, armored) { + const emails = userIds.map(({email}) => email); + const {keys: [key]} = await openpgp.key.readArmored(armored); + key.users = key.users.filter(({userId: {email}}) => emails.includes(email)); + return key.armor(); + } + + /** + * Merge (update) armored key blocks + * @param {String} srcArmored source amored key block + * @param {String} dstArmored destination armored key block + * @return {String} merged armored key block + */ + async updateKey(srcArmored, dstArmored) { + const {keys: [srcKey]} = await openpgp.key.readArmored(srcArmored); + const {keys: [dstKey]} = await openpgp.key.readArmored(dstArmored); + await dstKey.update(srcKey); + return dstKey.armor(); + } + + /** + * Remove user ID from armored key block + * @param {String} email email of user ID to be removed + * @param {String} publicKeyArmored amored key block to be filtered + * @return {String} filtered armored key block + */ + async removeUserId(email, publicKeyArmored) { + const {keys: [key]} = await openpgp.key.readArmored(publicKeyArmored); + key.users = key.users.filter(({userId}) => userId.email !== email); + return key.armor(); } } diff --git a/src/service/public-key.js b/src/service/public-key.js index ce91ca6..3c2203d 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -43,6 +43,7 @@ const tpl = require('../email/templates'); * } */ const DB_TYPE = 'publickey'; +const KEY_STATUS_VALID = 3; /** * A service that handlers PGP public keys queries to the database @@ -62,29 +63,47 @@ class PublicKey { /** * Persist a new public key + * @param {Array} emails (optional) The emails to upload/update * @param {String} publicKeyArmored The ascii armored pgp key block * @param {Object} origin Required for links to the keyserver e.g. { protocol:'https', host:'openpgpkeys@example.com' } - * @yield {undefined} + * @return {Promise} */ - async put({publicKeyArmored, origin}) { + async put({emails, publicKeyArmored, origin}) { // lazily purge old/unverified keys on every key upload await this._purgeOldUnverified(); // parse key block const key = await this._pgp.parseKey(publicKeyArmored); + // if emails array is empty, all userIds of the key will be submitted + if (emails.length) { + // keep submitted user IDs only + key.userIds = key.userIds.filter(({email}) => emails.includes(email)); + if (key.userIds.length !== emails.length) { + util.throw(400, 'Provided email address does not match a valid user ID of the key'); + } + } // check for existing verified key with same id const verified = await this.getVerified({keyId: key.keyId}); if (verified) { - util.throw(304, 'Key with this key id already exists'); + key.userIds = await this._mergeUsers(verified.userIds, key.userIds, key.publicKeyArmored); + // reduce new key to verified user IDs + const filteredPublicKeyArmored = await this._pgp.filterKeyByUserIds(key.userIds.filter(({verified}) => verified), key.publicKeyArmored); + // update verified key with new key + key.publicKeyArmored = await this._pgp.updateKey(verified.publicKeyArmored, filteredPublicKeyArmored); + } else { + key.userIds = key.userIds.filter(userId => userId.status === KEY_STATUS_VALID); + await this._addKeyArmored(key.userIds, key.publicKeyArmored); + // new key, set armored to null + key.publicKeyArmored = null; } - // store key in database - await this._persisKey(key); - // send mails to verify user ids (send only one if primary email is provided) + // send mails to verify user ids await this._sendVerifyEmail(key, origin); + // store key in database + await this._persistKey(key); } /** * Delete all keys where no user id has been verified after x days. - * @yield {undefined} + * @return {Promise} */ async _purgeOldUnverified() { // create date in the past to compare with @@ -97,17 +116,74 @@ class PublicKey { }, DB_TYPE); } + /** + * Merge existing and new user IDs + * @param {Array} existingUsers source user IDs + * @param {Array} newUsers new user IDs + * @param {String} publicKeyArmored armored key block of new user IDs + * @return {Array} merged user IDs + */ + async _mergeUsers(existingUsers, newUsers, publicKeyArmored) { + const result = []; + // existing verified valid or revoked users + const verifiedUsers = existingUsers.filter(userId => userId.verified); + // valid new users which are not yet verified + const validUsers = newUsers.filter(userId => userId.status === KEY_STATUS_VALID && !this._includeEmail(verifiedUsers, userId)); + // pending users are not verified, not newly submitted + const pendingUsers = existingUsers.filter(userId => !userId.verified && !this._includeEmail(validUsers, userId)); + await this._addKeyArmored(validUsers, publicKeyArmored); + result.push(...validUsers, ...pendingUsers, ...verifiedUsers); + return result; + } + + /** + * Create amored key block which contains the corresponding user ID only and add it to the user ID object + * @param {Array} userIds user IDs to be extended + * @param {String} PublicKeyArmored armored key block to be filtered + * @return {Promise} + */ + async _addKeyArmored(userIds, publicKeyArmored) { + for (const userId of userIds) { + userId.publicKeyArmored = await this._pgp.filterKeyByUserIds([userId], publicKeyArmored); + userId.notify = true; + } + } + + _includeEmail(users, user) { + return users.find(({email}) => email === user.email); + } + + /** + * Send verification emails to the public keys user ids for verification. + * If a primary email address is provided only one email will be sent. + * @param {Array} userIds user id documents containg the verification nonces + * @param {Object} origin the server's origin (required for email links) + * @return {Promise} + */ + async _sendVerifyEmail({userIds, keyId}, origin) { + for (const userId of userIds) { + if (userId.notify && userId.notify === true) { + // generate nonce for verification + userId.nonce = util.random(); + await this._email.send({template: tpl.verifyKey, userId, keyId, origin, publicKeyArmored: userId.publicKeyArmored}); + } + } + } + /** * Persist the public key and its user ids in the database. * @param {Object} key public key parameters - * @yield {undefined} The persisted user id documents + * @return {Promise} */ - async _persisKey(key) { + async _persistKey(key) { // delete old/unverified key await this._mongo.remove({keyId: key.keyId}, DB_TYPE); // generate nonces for verification - for (const uid of key.userIds) { - uid.nonce = util.random(); + for (const userId of key.userIds) { + // remove status from user + delete userId.status; + // remove notify flag from user + delete userId.notify; } // persist new key const r = await this._mongo.create(key, DB_TYPE); @@ -116,25 +192,11 @@ class PublicKey { } } - /** - * Send verification emails to the public keys user ids for verification. - * If a primary email address is provided only one email will be sent. - * @param {Array} userIds user id documents containg the verification nonces - * @param {Object} origin the server's origin (required for email links) - * @yield {undefined} - */ - async _sendVerifyEmail({userIds, keyId, publicKeyArmored}, origin) { - for (const userId of userIds) { - userId.publicKeyArmored = publicKeyArmored; // set key for encryption - await this._email.send({template: tpl.verifyKey, userId, keyId, origin}); - } - } - /** * Verify a user id by proving knowledge of the nonce. * @param {string} keyId Correspronding public key id * @param {string} nonce The verification nonce proving email address ownership - * @yield {undefined} + * @return {Promise} */ async verify({keyId, nonce}) { // look for verification nonce in database @@ -144,13 +206,27 @@ class PublicKey { util.throw(404, 'User id not found'); } await this._removeKeysWithSameEmail(key, nonce); + let {publicKeyArmored} = key.userIds.find(userId => userId.nonce === nonce); + // update armored key + if (key.publicKeyArmored) { + publicKeyArmored = await this._pgp.updateKey(key.publicKeyArmored, publicKeyArmored); + } // flag the user id as verified await this._mongo.update(query, { + publicKeyArmored, 'userIds.$.verified': true, - 'userIds.$.nonce': null + 'userIds.$.nonce': null, + 'userIds.$.publicKeyArmored': null }, DB_TYPE); } + /** + * Removes keys with the same email address + * @param {String} options.keyId source key ID + * @param {Array} options.userIds user IDs of source key + * @param {Array} nonce relevant nonce + * @return {Promise} + */ async _removeKeysWithSameEmail({keyId, userIds}, nonce) { return this._mongo.remove({ keyId: {$ne: keyId}, @@ -165,7 +241,7 @@ class PublicKey { * @param {Array} userIds A list of user ids to check * @param {string} fingerprint The public key fingerprint * @param {string} keyId (optional) The public key id - * @yield {Object} The verified key document + * @return {Object} The verified key document */ async getVerified({userIds, fingerprint, keyId}) { let queries = []; @@ -203,7 +279,7 @@ class PublicKey { * @param {string} fingerprint (optional) The public key fingerprint * @param {string} keyId (optional) The public key id * @param {String} email (optional) The user's email address - * @yield {Object} The public key document + * @return {Object} The public key document */ async get({fingerprint, keyId, email}) { // look for verified key @@ -230,7 +306,7 @@ class PublicKey { * @param {String} keyId (optional) The public key id * @param {String} email (optional) The user's email address * @param {Object} origin Required for links to the keyserver e.g. { protocol:'https', host:'openpgpkeys@example.com' } - * @yield {undefined} + * @return {Promise} */ async requestRemove({keyId, email, origin}) { // flag user ids for removal @@ -250,7 +326,7 @@ class PublicKey { * saving it. Either a key id or email address must be provided * @param {String} keyId (optional) The public key id * @param {String} email (optional) The user's email address - * @yield {Array} A list of user ids with nonces + * @return {Array} A list of user ids with nonces */ async _flagForRemove(keyId, email) { const query = email ? {'userIds.email': email} : {keyId}; @@ -282,7 +358,7 @@ class PublicKey { * Also deletes all user id documents of that key id. * @param {string} keyId public key id * @param {string} nonce The verification nonce proving email address ownership - * @yield {undefined} + * @return {Promise} */ async verifyRemove({keyId, nonce}) { // check if key exists in database @@ -290,8 +366,22 @@ class PublicKey { if (!flagged) { util.throw(404, 'User id not found'); } - // delete the key - await this._mongo.remove({keyId}, DB_TYPE); + if (flagged.userIds.length === 1) { + // delete the key + return this._mongo.remove({keyId}, DB_TYPE); + } + // update the key + const rmIdx = flagged.userIds.findIndex(userId => userId.nonce === nonce); + const rmUserId = flagged.userIds[rmIdx]; + if (rmUserId.verified) { + if (flagged.userIds.filter(({verified}) => verified).length > 1) { + flagged.publicKeyArmored = await this._pgp.removeUserId(rmUserId.email, flagged.publicKeyArmored); + } else { + flagged.publicKeyArmored = null; + } + } + flagged.userIds.splice(rmIdx, 1); + await this._mongo.update({keyId}, flagged, DB_TYPE); } } diff --git a/test/integration/app-test.js b/test/integration/app-test.js index 419f646..825e009 100644 --- a/test/integration/app-test.js +++ b/test/integration/app-test.js @@ -301,21 +301,21 @@ describe('Koa App (HTTP Server) Integration Tests', function() { it('should return 200 for key id', done => { request(app.listen()) .get(`/pks/lookup?op=get&search=0x${emailParams.keyId}`) - .expect(200, publicKeyArmored) + .expect(200) .end(done); }); it('should return 200 for fingerprint', done => { request(app.listen()) .get(`/pks/lookup?op=get&search=0x${fingerprint}`) - .expect(200, publicKeyArmored) + .expect(200) .end(done); }); it('should return 200 for correct email address', done => { request(app.listen()) .get(`/pks/lookup?op=get&search=${primaryEmail}`) - .expect(200, publicKeyArmored) + .expect(200) .end(done); }); @@ -324,7 +324,7 @@ describe('Koa App (HTTP Server) Integration Tests', function() { .get(`/pks/lookup?op=get&options=mr&search=${primaryEmail}`) .expect('Content-Type', 'application/pgp-keys; charset=utf-8') .expect('Content-Disposition', 'attachment; filename=openpgpkey.asc') - .expect(200, publicKeyArmored) + .expect(200) .end(done); }); diff --git a/test/integration/public-key-test.js b/test/integration/public-key-test.js index 9846c2e..8b62425 100644 --- a/test/integration/public-key-test.js +++ b/test/integration/public-key-test.js @@ -59,7 +59,7 @@ describe('Public Key Integration Tests', function() { email.init({ host: 'localhost', auth: {user: 'user', pass: 'pass'}, - sender: {name: 'Foo Bar', email: 'foo@bar.com'} + sender: {name: 'Foo Bar', emails: 'foo@bar.com'} }); pgp = new PGP(); publicKey = new PublicKey(pgp, mongo, email); @@ -77,22 +77,22 @@ describe('Public Key Integration Tests', function() { describe('put', () => { it('should persist key and send verification email', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); expect(mailsSent.length).to.equal(4); }); it('should work twice if not yet verified', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); expect(mailsSent.length).to.equal(4); - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); expect(mailsSent.length).to.equal(8); }); - it('should throw 304 if key already exists', async () => { - await publicKey.put({publicKeyArmored, origin}); + it.skip('should throw 304 if key already exists', async () => { + await publicKey.put({emails: [], publicKeyArmored, origin}); await publicKey.verify(mailsSent[0].params); try { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); expect(false).to.be.true; } catch (e) { expect(e.status).to.equal(304); @@ -100,9 +100,9 @@ describe('Public Key Integration Tests', function() { }); 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.put({emails: [], publicKeyArmored, origin}); await publicKey.verify(mailsSent[1].params); - await publicKey.put({publicKeyArmored: publicKeyArmored2, origin}); + await publicKey.put({emails: [], publicKeyArmored: publicKeyArmored2, origin}); expect(mailsSent.length).to.equal(5); }); }); @@ -120,14 +120,14 @@ describe('Public Key Integration Tests', function() { }); it('should not remove a current unverified key', async () => { - await publicKey._persisKey(key); + await publicKey._persistKey(key); const r = await publicKey._purgeOldUnverified(); expect(r.deletedCount).to.equal(0); }); it('should not remove a current verified key', async () => { key.userIds[0].verified = true; - await publicKey._persisKey(key); + await publicKey._persistKey(key); const r = await publicKey._purgeOldUnverified(); expect(r.deletedCount).to.equal(0); }); @@ -135,14 +135,14 @@ describe('Public Key Integration Tests', function() { it('should not remove an old verified key', async () => { key.uploaded.setDate(key.uploaded.getDate() - 31); key.userIds[0].verified = true; - await publicKey._persisKey(key); + await publicKey._persistKey(key); const r = await publicKey._purgeOldUnverified(); expect(r.deletedCount).to.equal(0); }); it('should remove an old unverified key', async () => { key.uploaded.setDate(key.uploaded.getDate() - 31); - await publicKey._persisKey(key); + await publicKey._persistKey(key); const r = await publicKey._purgeOldUnverified(); expect(r.deletedCount).to.equal(1); }); @@ -150,7 +150,7 @@ describe('Public Key Integration Tests', function() { describe('verify', () => { it('should update the document', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); const emailParams = mailsSent[0].params; await publicKey.verify(emailParams); const gotten = await mongo.get({keyId: emailParams.keyId}, DB_TYPE); @@ -161,7 +161,7 @@ describe('Public Key Integration Tests', function() { }); it('should not find the document', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); const emailParams = mailsSent[0].params; try { await publicKey.verify({keyId: emailParams.keyId, nonce: 'fake_nonce'}); @@ -177,11 +177,11 @@ describe('Public Key Integration Tests', function() { }); it('should verify a second key for an already verified user id and delete the old key', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); 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}); + await publicKey.put({emails: [], publicKeyArmored: publicKeyArmored2, origin}); await publicKey.verify(mailsSent[4].params); firstKey = await publicKey.getVerified({keyId: mailsSent[1].params.keyId}); expect(firstKey).to.not.exist; @@ -190,8 +190,8 @@ describe('Public Key Integration Tests', function() { }); it('should delete other keys with the same user id when verifying', async () => { - await publicKey.put({publicKeyArmored, origin}); - await publicKey.put({publicKeyArmored: publicKeyArmored2, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); + await publicKey.put({emails: [], 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}); @@ -201,7 +201,7 @@ describe('Public Key Integration Tests', function() { }); it('should be able to verify multiple user ids', async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); expect(mailsSent.length).to.equal(4); await publicKey.verify(mailsSent[0].params); await publicKey.verify(mailsSent[1].params); @@ -221,7 +221,7 @@ describe('Public Key Integration Tests', function() { describe('should find a verified key', () => { beforeEach(async () => { key = await pgp.parseKey(publicKeyArmored); - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); await publicKey.verify(mailsSent[0].params); }); @@ -289,7 +289,7 @@ describe('Public Key Integration Tests', function() { let emailParams; beforeEach(async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); emailParams = mailsSent[0].params; }); @@ -345,7 +345,7 @@ describe('Public Key Integration Tests', function() { let keyId; beforeEach(async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); keyId = mailsSent[0].params.keyId; }); @@ -380,13 +380,63 @@ describe('Public Key Integration Tests', function() { let keyId; beforeEach(async () => { - await publicKey.put({publicKeyArmored, origin}); + await publicKey.put({emails: [], publicKeyArmored, origin}); keyId = mailsSent[0].params.keyId; + }); + + afterEach(() => { + mailsSent = []; + }); + + it('should remove unverified user ID', async () => { await publicKey.requestRemove({keyId, origin}); + const key = await mongo.get({keyId}, DB_TYPE); + expect(key.userIds[0].verified).to.be.false; + expect(key.userIds[0].email).to.equal(primaryEmail); + await publicKey.verifyRemove(mailsSent[4].params); + const modifiedKey = await mongo.get({keyId}, DB_TYPE); + expect(modifiedKey.userIds[0].email).to.not.equal(primaryEmail); + }); + + it('should remove single verfied user ID', async () => { + await publicKey.verify(mailsSent[0].params); + const key = await mongo.get({keyId}, DB_TYPE); + expect(key.userIds[0].verified).to.be.true; + expect(key.userIds[0].email).to.equal(primaryEmail); + const keyFromArmored = await pgp.parseKey(key.publicKeyArmored); + expect(keyFromArmored.userIds.find(userId => userId.email === primaryEmail)).not.to.be.undefined; + await publicKey.requestRemove({keyId, origin}); + await publicKey.verifyRemove(mailsSent[4].params); + const modifiedKey = await mongo.get({keyId}, DB_TYPE); + expect(modifiedKey.userIds[0].email).to.not.equal(primaryEmail); + expect(modifiedKey.publicKeyArmored).to.be.null; + }); + + it('should remove verfied user ID', async () => { + await publicKey.verify(mailsSent[0].params); + await publicKey.verify(mailsSent[1].params); + const key = await mongo.get({keyId}, DB_TYPE); + expect(key.userIds[0].verified).to.be.true; + expect(key.userIds[1].verified).to.be.true; + const emails = [key.userIds[0].email, key.userIds[1].email]; + const keyFromArmored = await pgp.parseKey(key.publicKeyArmored); + expect(keyFromArmored.userIds.filter(userId => emails.includes(userId.email)).length).to.equal(2); + await publicKey.requestRemove({keyId, origin}); + await publicKey.verifyRemove(mailsSent[5].params); + const modifiedKey = await mongo.get({keyId}, DB_TYPE); + expect(modifiedKey.userIds[0].email).to.equal(emails[0]); + expect(modifiedKey.userIds[1].email).to.not.equal(emails[1]); + expect(modifiedKey.publicKeyArmored).not.to.be.null; + const keyFromModifiedArmored = await pgp.parseKey(modifiedKey.publicKeyArmored); + expect(keyFromModifiedArmored.userIds.filter(userId => emails.includes(userId.email)).length).to.equal(1); }); it('should remove key', async () => { + await publicKey.requestRemove({keyId, origin}); await publicKey.verifyRemove(mailsSent[4].params); + await publicKey.verifyRemove(mailsSent[5].params); + await publicKey.verifyRemove(mailsSent[6].params); + await publicKey.verifyRemove(mailsSent[7].params); const key = await mongo.get({keyId}, DB_TYPE); expect(key).to.not.exist; });