From 6c2fec34501ef15fd6724f764c63b1a9abe09e66 Mon Sep 17 00:00:00 2001
From: Daniel Huigens <d.huigens@protonmail.com>
Date: Thu, 26 Apr 2018 12:30:45 +0200
Subject: [PATCH] Parse user IDs

Also, support comments when creating user IDs
---
 Gruntfile.js             |  4 +-
 package.json             |  1 +
 src/key.js               |  2 +-
 src/openpgp.js           | 34 +--------------
 src/packet/userid.js     | 27 +++++++++++-
 src/util.js              | 25 +++++++++++
 test/general/ecc_nist.js | 10 ++++-
 test/general/key.js      | 10 ++++-
 test/general/openpgp.js  | 90 +++++++++++++++++++++++-----------------
 9 files changed, 125 insertions(+), 78 deletions(-)

diff --git a/Gruntfile.js b/Gruntfile.js
index af6a0ba0..93ea0cee 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -29,8 +29,8 @@ module.exports = function(grunt) {
           transform: [
             ["babelify", {
               global: true,
-              // Only babelify asmcrypto in node_modules
-              only: /^(?:.*\/node_modules\/asmcrypto\.js\/|(?!.*\/node_modules\/)).*$/,
+              // Only babelify asmcrypto and address-rfc2822 in node_modules
+              only: /^(?:.*\/node_modules\/asmcrypto\.js\/|.*\/node_modules\/address-rfc2822\/|(?!.*\/node_modules\/)).*$/,
               plugins: ["transform-async-to-generator",
                         "syntax-async-functions",
                         "transform-regenerator",
diff --git a/package.json b/package.json
index 025ab6c6..1a674fa4 100644
--- a/package.json
+++ b/package.json
@@ -72,6 +72,7 @@
     "whatwg-fetch": "^2.0.3"
   },
   "dependencies": {
+    "address-rfc2822": "^2.0.3",
     "asmcrypto.js": "^0.22.0",
     "asn1.js": "^5.0.0",
     "bn.js": "^4.11.8",
diff --git a/src/key.js b/src/key.js
index 0d58a2a3..cf48a3d0 100644
--- a/src/key.js
+++ b/src/key.js
@@ -1244,7 +1244,7 @@ async function wrapKeyObject(secretKeyPacket, secretSubkeyPackets, options) {
 
   await Promise.all(options.userIds.map(async function(userId, index) {
     const userIdPacket = new packet.Userid();
-    userIdPacket.read(util.str_to_Uint8Array(userId));
+    userIdPacket.format(userId);
 
     const dataToSign = {};
     dataToSign.userid = userIdPacket;
diff --git a/src/openpgp.js b/src/openpgp.js
index 9f0a690e..163cf94f 100644
--- a/src/openpgp.js
+++ b/src/openpgp.js
@@ -115,7 +115,7 @@ export function destroyWorker() {
  */
 
 export function generateKey({ userIds=[], passphrase="", numBits=2048, keyExpirationTime=0, curve="", date=new Date(), subkeys=[{}] }) {
-  userIds = formatUserIds(userIds);
+  userIds = toArray(userIds);
   const options = { userIds, passphrase, numBits, keyExpirationTime, curve, date, subkeys };
   if (util.getWebCryptoAll() && numBits < 2048) {
     throw new Error('numBits should be 2048 or 4096, found: ' + numBits);
@@ -146,7 +146,7 @@ export function generateKey({ userIds=[], passphrase="", numBits=2048, keyExpira
  * @static
  */
 export function reformatKey({privateKey, userIds=[], passphrase="", keyExpirationTime=0, date}) {
-  userIds = formatUserIds(userIds);
+  userIds = toArray(userIds);
   const options = { privateKey, userIds, passphrase, keyExpirationTime, date};
   if (asyncProxy) {
     return asyncProxy.delegate('reformatKey', options);
@@ -484,36 +484,6 @@ function checkCleartextOrMessage(message) {
   }
 }
 
-/**
- * Format user ids for internal use.
- */
-function formatUserIds(userIds) {
-  if (!userIds) {
-    return userIds;
-  }
-  userIds = toArray(userIds); // normalize to array
-  userIds = userIds.map(id => {
-    if (util.isString(id) && !util.isUserId(id)) {
-      throw new Error('Invalid user id format');
-    }
-    if (util.isUserId(id)) {
-      return id; // user id is already in correct format... no conversion necessary
-    }
-    // name and email address can be empty but must be of the correct type
-    id.name = id.name || '';
-    id.email = id.email || '';
-    if (!util.isString(id.name) || (id.email && !util.isEmailAddress(id.email))) {
-      throw new Error('Invalid user id format');
-    }
-    id.name = id.name.trim();
-    if (id.name.length > 0) {
-      id.name += ' ';
-    }
-    return id.name + '<' + id.email + '>';
-  });
-  return userIds;
-}
-
 /**
  * Normalize parameter to an array if it is not undefined.
  * @param  {Object} param              the parameter to be normalized
diff --git a/src/packet/userid.js b/src/packet/userid.js
index 99a5fbdb..90ce88e6 100644
--- a/src/packet/userid.js
+++ b/src/packet/userid.js
@@ -41,6 +41,10 @@ function Userid() {
    * @type {String}
    */
   this.userid = '';
+
+  this.name = '';
+  this.email = '';
+  this.comment = '';
 }
 
 /**
@@ -48,7 +52,17 @@ function Userid() {
  * @param {Uint8Array} input payload of a tag 13 packet
  */
 Userid.prototype.read = function (bytes) {
-  this.userid = util.decode_utf8(util.Uint8Array_to_str(bytes));
+  this.parse(util.decode_utf8(util.Uint8Array_to_str(bytes)));
+};
+
+/**
+ * Parse userid string, e.g. 'John Doe <john@example.com>'
+ */
+Userid.prototype.parse = function (userid) {
+  try {
+    Object.assign(this, util.parseUserId(userid));
+  } catch(e) {}
+  this.userid = userid;
 };
 
 /**
@@ -59,4 +73,15 @@ Userid.prototype.write = function () {
   return util.str_to_Uint8Array(util.encode_utf8(this.userid));
 };
 
+/**
+ * Set userid string from object, e.g. { name:'Phil Zimmermann', email:'phil@openpgp.org' }
+ */
+Userid.prototype.format = function (userid) {
+  if (util.isString(userid)) {
+    userid = util.parseUserId(userid);
+  }
+  Object.assign(this, userid);
+  this.userid = util.formatUserId(userid);
+};
+
 export default Userid;
diff --git a/src/util.js b/src/util.js
index 375dc887..4cca968a 100644
--- a/src/util.js
+++ b/src/util.js
@@ -17,11 +17,13 @@
 
 /**
  * This object contains utility functions
+ * @requires address-rfc2822
  * @requires config
  * @requires encoding/base64
  * @module util
  */
 
+import rfc2822 from 'address-rfc2822';
 import config from './config';
 import util from './util'; // re-import module to access util functions
 import b64 from './encoding/base64';
@@ -569,6 +571,29 @@ export default {
     return re.test(data);
   },
 
+  /**
+   * Format user id for internal use.
+   */
+  formatUserId: function(id) {
+    // name and email address can be empty but must be of the correct type
+    if ((id.name && !util.isString(id.name)) || (id.email && !util.isEmailAddress(id.email))) {
+      throw new Error('Invalid user id format');
+    }
+    return new rfc2822.Address(id.name, id.email, id.comment).format();
+  },
+
+  /**
+   * Parse user id.
+   */
+  parseUserId: function(userid) {
+    try {
+      const [{ phrase: name, address: email, comment }] = rfc2822.parse(userid);
+      return { name, email, comment: comment.replace(/^\(|\)$/g, '') };
+    } catch(e) {
+      throw new Error('Invalid user id format');
+    }
+  },
+
   isUserId: function(data) {
     if (!util.isString(data)) {
       return false;
diff --git a/test/general/ecc_nist.js b/test/general/ecc_nist.js
index 264e07cd..f6766272 100644
--- a/test/general/ecc_nist.js
+++ b/test/general/ecc_nist.js
@@ -156,8 +156,14 @@ describe('Elliptic Curve Cryptography', function () {
     return data[name].priv_key;
   }
   it('Load public key', function (done) {
-    load_pub_key('romeo');
-    load_pub_key('juliet');
+    const romeoPublic = load_pub_key('romeo');
+    expect(romeoPublic.users[0].userId.name).to.equal('Romeo Montague');
+    expect(romeoPublic.users[0].userId.email).to.equal('romeo@example.net');
+    expect(romeoPublic.users[0].userId.comment).to.equal('secp256k1');
+    const julietPublic = load_pub_key('juliet');
+    expect(julietPublic.users[0].userId.name).to.equal('Juliet Capulet');
+    expect(julietPublic.users[0].userId.email).to.equal('juliet@example.net');
+    expect(julietPublic.users[0].userId.comment).to.equal('secp256k1');
     done();
   });
   it('Load private key', async function () {
diff --git a/test/general/key.js b/test/general/key.js
index 3003b8f1..c051e8fd 100644
--- a/test/general/key.js
+++ b/test/general/key.js
@@ -1297,6 +1297,9 @@ p92yZgB3r2+f6/GIe2+7
     const primUser = await key.getPrimaryUser();
     expect(primUser).to.exist;
     expect(primUser.user.userId.userid).to.equal('Signature Test <signature@test.com>');
+    expect(primUser.user.userId.name).to.equal('Signature Test');
+    expect(primUser.user.userId.email).to.equal('signature@test.com');
+    expect(primUser.user.userId.comment).to.equal('');
     expect(primUser.selfCertification).to.be.an.instanceof(openpgp.packet.Signature);
   });
 
@@ -1315,13 +1318,16 @@ p92yZgB3r2+f6/GIe2+7
   });
 
   it('Generate key - single userid', function() {
-    const userId = 'test <a@b.com>';
+    const userId = { name: 'test', email: 'a@b.com', comment: 'test comment' };
     const opt = {numBits: 512, userIds: userId, passphrase: '123'};
     if (openpgp.util.getWebCryptoAll()) { opt.numBits = 2048; } // webkit webcrypto accepts minimum 2048 bit keys
     return openpgp.generateKey(opt).then(function(key) {
       key = key.key;
       expect(key.users.length).to.equal(1);
-      expect(key.users[0].userId.userid).to.equal(userId);
+      expect(key.users[0].userId.userid).to.equal('test <a@b.com> (test comment)');
+      expect(key.users[0].userId.name).to.equal(userId.name);
+      expect(key.users[0].userId.email).to.equal(userId.email);
+      expect(key.users[0].userId.comment).to.equal(userId.comment);
     });
   });
 
diff --git a/test/general/openpgp.js b/test/general/openpgp.js
index 3f6e8ed3..7c878a37 100644
--- a/test/general/openpgp.js
+++ b/test/general/openpgp.js
@@ -371,73 +371,57 @@ describe('OpenPGP.js public api tests', function() {
     });
   });
 
-  describe('generateKey - unit tests', function() {
-    let keyGenStub;
-    let keyObjStub;
-    let getWebCryptoAllStub;
+  describe('generateKey - validate user ids', function() {
+    let rsaGenStub;
+    let rsaGenValue = openpgp.crypto.publicKey.rsa.generate(2048, "10001");
 
     beforeEach(function() {
-      keyObjStub = {
-        armor: function() {
-          return 'priv_key';
-        },
-        toPublic: function() {
-          return {
-            armor: function() {
-              return 'pub_key';
-            }
-          };
-        }
-      };
-      keyGenStub = stub(openpgp.key, 'generate');
-      keyGenStub.returns(resolves(keyObjStub));
-      getWebCryptoAllStub = stub(openpgp.util, 'getWebCryptoAll');
+      rsaGenStub = stub(openpgp.crypto.publicKey.rsa, 'generate');
+      rsaGenStub.returns(rsaGenValue);
     });
 
     afterEach(function() {
-      keyGenStub.restore();
-      openpgp.destroyWorker();
-      getWebCryptoAllStub.restore();
+      rsaGenStub.restore();
     });
 
-    it('should fail for invalid user name', function() {
+    it('should fail for invalid user name', async function() {
       const opt = {
         userIds: [{ name: {}, email: 'text@example.com' }]
       };
-      const test = openpgp.generateKey.bind(null, opt);
-      expect(test).to.throw(/Invalid user id format/);
+      const test = openpgp.generateKey(opt);
+      await expect(test).to.eventually.be.rejectedWith(/Invalid user id format/);
     });
 
-    it('should fail for invalid user email address', function() {
+    it('should fail for invalid user email address', async function() {
       const opt = {
         userIds: [{ name: 'Test User', email: 'textexample.com' }]
       };
-      const test = openpgp.generateKey.bind(null, opt);
-      expect(test).to.throw(/Invalid user id format/);
+      const test = openpgp.generateKey(opt);
+      await expect(test).to.eventually.be.rejectedWith(/Invalid user id format/);
     });
 
-    it('should fail for invalid user email address', function() {
+    it('should fail for invalid user email address', async function() {
       const opt = {
         userIds: [{ name: 'Test User', email: 'text@examplecom' }]
       };
-      const test = openpgp.generateKey.bind(null, opt);
-      expect(test).to.throw(/Invalid user id format/);
+      const test = openpgp.generateKey(opt);
+      await expect(test).to.eventually.be.rejectedWith(/Invalid user id format/);
     });
 
-    it('should fail for invalid string user id', function() {
+    it('should fail for invalid string user id', async function() {
       const opt = {
         userIds: ['Test User text@example.com>']
       };
-      const test = openpgp.generateKey.bind(null, opt);
-      expect(test).to.throw(/Invalid user id format/);
+      const test = openpgp.generateKey(opt);
+      await expect(test).to.eventually.be.rejectedWith(/Invalid user id format/);
     });
 
-    it('should fail for invalid single string user id', function() {
+    it('should fail for invalid single string user id', async function() {
       const opt = {
         userIds: 'Test User text@example.com>'
       };
-      const test = openpgp.generateKey.bind(null, opt);
-      expect(test).to.throw(/Invalid user id format/);
+      const test = openpgp.generateKey(opt);
+      await expect(test).to.eventually.be.rejectedWith(/Invalid user id format/);
     });
 
     it('should work for valid single string user id', function() {
@@ -481,6 +465,36 @@ describe('OpenPGP.js public api tests', function() {
       };
       return openpgp.generateKey(opt);
     });
+  });
+
+  describe('generateKey - unit tests', function() {
+    let keyGenStub;
+    let keyObjStub;
+    let getWebCryptoAllStub;
+
+    beforeEach(function() {
+      keyObjStub = {
+        armor: function() {
+          return 'priv_key';
+        },
+        toPublic: function() {
+          return {
+            armor: function() {
+              return 'pub_key';
+            }
+          };
+        }
+      };
+      keyGenStub = stub(openpgp.key, 'generate');
+      keyGenStub.returns(resolves(keyObjStub));
+      getWebCryptoAllStub = stub(openpgp.util, 'getWebCryptoAll');
+    });
+
+    afterEach(function() {
+      keyGenStub.restore();
+      openpgp.destroyWorker();
+      getWebCryptoAllStub.restore();
+    });
 
     it('should have default params set', function() {
       const now = openpgp.util.normalizeDate(new Date());
@@ -492,7 +506,7 @@ describe('OpenPGP.js public api tests', function() {
       };
       return openpgp.generateKey(opt).then(function(newKey) {
         expect(keyGenStub.withArgs({
-          userIds: ['Test User <text@example.com>'],
+          userIds: [{ name: 'Test User', email: 'text@example.com' }],
           passphrase: 'secret',
           numBits: 2048,
           keyExpirationTime: 0,