Improve the email store plugin:

* Allow overwrite of data
* Clean up code
* Improve tests

plugins/emailstore.js

@@ -52,7 +52,7 @@ var emailPlugin = {};
 /**
  * Constant enum with the errors that the application may return
  */
-var errors = {
+emailPlugin.errors = {
   MISSING_PARAMETER: {
     code: 400,
     message: 'Missing required parameter'
@@ -65,6 +65,10 @@ var errors = {
     code: 404,
     message: 'Credentials were not found'
   },
+  INTERNAL_ERROR: {
+    code: 500,
+    message: 'Unable to save to database'
+  },
   EMAIL_TAKEN: {
     code: 409,
     message: 'That email is already registered'
@@ -77,8 +81,9 @@ var errors = {
 
 var NAMESPACE = 'credentials-store-';
 var VALIDATION_NAMESPACE = 'validation-code-';
+var MAP_EMAIL_TO_SECRET = 'map-email-';
 var EMAIL_NAMESPACE = 'validated-email-';
-var MAX_ALLOWED_STORAGE = 1024 /* no more than 1 kb */;
+var MAX_ALLOWED_STORAGE = 1024 * 100 /* no more than 100 kb */;
 
 /**
  * Initializes the plugin
@@ -111,7 +116,7 @@ emailPlugin.init = function(expressApp, config) {
  * @param {Express.Response} response - the express.js response. the methods status, json, and end
  *                                      will be called, terminating the request.
  */
-var returnError = function(error, response) {
+emailPlugin.returnError = function (error, response) {
   response.status(error.code).json({error: error.message}).end();
 };
 
@@ -121,7 +126,7 @@ var returnError = function(error, response) {
  * @param {string} email - the user's email
  * @param {string} secret - the verification secret
  */
-var sendVerificationEmail = function(email, secret) {
+emailPlugin.sendVerificationEmail = function (email, secret) {
 
   var emailBody = 'Activation code is ' + secret; // TODO: Use a template!
   var emailBodyHTML = '<h1>Activation code is ' + secret + '</h1>'; // TODO: Use a template!
@@ -144,6 +149,65 @@ var sendVerificationEmail = function(email, secret) {
   });
 };
 
+/**
+ * @param {string} email
+ * @param {Function(err, boolean)} callback
+ */
+emailPlugin.exists = function(email, callback) {
+  emailPlugin.db.get(EMAIL_NAMESPACE + email, function(err, value) {
+    if (err && err.notFound) {
+      return callback(null, false);
+    } else if (err) {
+      return callback(err);
+    }
+    return callback(null, true);
+  });
+};
+
+/**
+ * @param {string} email
+ * @param {string} passphrase
+ * @param {Function(err, boolean)} callback
+ */
+emailPlugin.checkPassphrase = function(email, passphrase, callback) {
+  emailPlugin.db.get(MAP_EMAIL_TO_SECRET + email, function(err, retrievedPassphrase) {
+    if (err) {
+      return callback(err);
+    }
+    return callback(err, passphrase === retrievedPassphrase);
+  });
+};
+
+/**
+ * @param {string} email
+ * @param {string} passphrase
+ * @param {Function(err)} callback
+ */
+emailPlugin.savePassphrase = function(email, passphrase, callback) {
+  emailPlugin.db.put(MAP_EMAIL_TO_SECRET + email, passphrase, callback);
+};
+
+/**
+ * @param {string} email
+ * @param {string} record
+ * @param {Function(err)} callback
+ */
+emailPlugin.saveEncryptedData = function(email, record, callback) {
+  emailPlugin.db.put(NAMESPACE + email, record, callback);
+};
+
+emailPlugin.createVerificationSecretAndSendEmail = function (email, callback) {
+  emailPlugin.createVerificationSecret(email, function(err, secret) {
+    if (err) {
+      return callback(err);
+    }
+    if (secret) {
+      emailPlugin.sendVerificationEmail(email, secret);
+    }
+    callback();
+  });
+};
+
 /**
  * Store a record in the database. The underlying database is merely a levelup instance (a key
  * value store) that uses the email concatenated with the secret as a key to store the record.
@@ -172,25 +236,40 @@ emailPlugin.post = function(request, response) {
     var secret = params.secret;
     var record = params.record;
     if (!email || !secret || !record) {
-      return returnError(errors.MISSING_PARAMETER, response);
+      return emailPlugin.returnError(emailPlugin.errors.MISSING_PARAMETER, response);
     }
 
+    emailPlugin.processPost(request, response, email, secret, record);
+  });
+};
+
+emailPlugin.processPost = function(request, response, email, secret, record) {
   async.series([
     /**
-       * Try to fetch this user's email. If it exists, fail.
+     * Try to fetch this user's email. If it exists, check the secret is the same.
      */
     function (callback) {
-        emailPlugin.db.get(VALIDATION_NAMESPACE + email, function(err, dbValue) {
+      emailPlugin.exists(email, function(err, exists) {
-          if (!dbValue) {
+        if (err) {
-            emailPlugin.db.get(EMAIL_NAMESPACE + email, function(err, dbValue) {
+          return callback({code: 500, message: err});
-              if (!dbValue) {
+        } else if (exists) {
-                callback();
+          emailPlugin.checkPassphrase(email, secret, function(err, match) {
+            if (err) {
+              return callback({code: 500, message: err});
+            }
+            if (match) {
+              return callback();
             } else {
-                callback(errors.EMAIL_TAKEN);
+              return callback(emailPlugin.errors.EMAIL_TAKEN);
             }
           });
         } else {
-            callback(errors.EMAIL_TAKEN);
+          emailPlugin.savePassphrase(email, secret, function(err) {
+            if (err) {
+              return callback({code: 500, message: err});
+            }
+            return callback();
+          });
         }
       });
     },
@@ -198,36 +277,33 @@ emailPlugin.post = function(request, response) {
      * Save the encrypted private key in the storage.
      */
     function (callback) {
-        emailPlugin.db.put(NAMESPACE + secret, record, function (err) {
+      emailPlugin.saveEncryptedData(email, record, function(err) {
         if (err) {
-            callback({code: 500, message: err});
+          return callback({code: 500, message: err});
-          } else {
-            callback();
         }
+        return callback();
       });
     },
     /**
      * Create and store the verification secret. If successful, send a verification email.
      */
     function (callback) {
-        emailPlugin.createVerificationSecret(email, function(err, secret) {
+      emailPlugin.createVerificationSecretAndSendEmail(email, function (err) {
         if (err) {
           callback({code: 500, message: err});
         } else {
-            sendVerificationEmail(email, secret);
           callback();
         }
       });
     }
   ], function (err) {
       if (err) {
-          returnError(err, response);
+        emailPlugin.returnError(err, response);
       } else {
         response.json({success: true}).end();
       }
     }
   );
-  });
 };
 
 /**
@@ -237,6 +313,8 @@ emailPlugin.post = function(request, response) {
  * @param {Function} callback - will be called with params (err, secret)
  */
 emailPlugin.createVerificationSecret = function (email, callback) {
+  emailPlugin.db.get(VALIDATION_NAMESPACE + email, function(err, value) {
+    if (err && err.notFound) {
       var secret = crypto.randomBytes(16).toString('hex');
       emailPlugin.db.put(VALIDATION_NAMESPACE + email, secret, function (err, value) {
         if (err) {
@@ -244,6 +322,32 @@ emailPlugin.createVerificationSecret = function(email, callback) {
         }
         callback(err, secret);
       });
+    } else {
+      callback(err, null);
+    }
+  });
+};
+
+
+/**
+ * @param {string} email
+ * @param {Function(err)} callback
+ */
+emailPlugin.retrieveByEmail = function(email, callback) {
+  emailPlugin.db.get(NAMESPACE + email, callback);
+};
+
+emailPlugin.retrieveDataByEmailAndPassphrase = function(email, passphrase, callback) {
+  emailPlugin.checkPassphrase(email, passphrase, function(err, matches) {
+    if (err) {
+      return callback(err);
+    }
+    if (matches) {
+      return emailPlugin.retrieveByEmail(email, callback);
+    } else {
+      return callback(emailPlugin.errors.INVALID_CODE);
+    }
+  });
 };
 
 /**
@@ -256,17 +360,18 @@ emailPlugin.createVerificationSecret = function(email, callback) {
  * @param {Express.Response} response
  */
 emailPlugin.get = function (request, response) {
+  var email = request.param('email');
   var secret = request.param('secret');
   if (!secret) {
-    return returnError(errors.MISSING_PARAMETER, response);
+    return emailPlugin.returnError(emailPlugin.errors.MISSING_PARAMETER, response);
   }
 
-  emailPlugin.db.get(NAMESPACE + secret, function (err, value) {
+  emailPlugin.retrieveDataByEmailAndPassphrase(email, secret, function (err, value) {
     if (err) {
       if (err.notFound) {
-        return returnError(errors.NOT_FOUND, response);
+        return emailPlugin.returnError(emailPlugin.errors.NOT_FOUND, response);
       }
-      return returnError({code: 500, message: err}, response);
+      return emailPlugin.returnError({code: 500, message: err}, response);
     }
     response.send(value).end();
   });
@@ -286,22 +391,22 @@ emailPlugin.validate = function(request, response) {
   var email = request.param('email');
   var secret = request.param('verification_code');
   if (!email || !secret) {
-    return returnError(errors.MISSING_PARAMETER, response);
+    return emailPlugin.returnError(emailPlugin.errors.MISSING_PARAMETER, response);
   }
 
   emailPlugin.db.get(VALIDATION_NAMESPACE + email, function (err, value) {
     logger.info('Recibido: ' + value);
     if (err) {
       if (err.notFound) {
-        return returnError(errors.NOT_FOUND, response);
+        return emailPlugin.returnError(emailPlugin.errors.NOT_FOUND, response);
       }
-      return returnError({code: 500, message: err}, response);
+      return emailPlugin.returnError({code: 500, message: err}, response);
     } else if (value !== secret) {
-      return returnError(errors.INVALID_CODE, response);
+      return emailPlugin.returnError(emailPlugin.errors.INVALID_CODE, response);
     } else {
       emailPlugin.db.put(EMAIL_NAMESPACE + email, true, function (err, value) {
         if (err) {
-          return returnError({code: 500, message: err}, response);
+          return emailPlugin.returnError({code: 500, message: err}, response);
         } else {
           response.json({success: true}).end();
         }

test/test.EmailStore.js

@@ -50,51 +50,57 @@ describe('emailstore test', function() {
 
   describe('on registration', function() {
 
+    var emailParam = 'email';
+    var secretParam = 'secret';
+    var recordParam = 'record';
     beforeEach(function() {
-      var data = 'email=1&secret=2&record=3';
+      var data = 'email=' + emailParam + '&secret=' + secretParam + '&record=' + recordParam;
       request.on.onFirstCall().callsArgWith(1, data);
       request.on.onFirstCall().returnsThis();
       request.on.onSecondCall().callsArg(1);
-      leveldb_stub.get.onFirstCall().callsArg(1);
-      leveldb_stub.get.onSecondCall().callsArg(1);
-      leveldb_stub.put.onFirstCall().callsArg(2);
-      leveldb_stub.put.onSecondCall().callsArg(2);
       response.json.returnsThis();
     });
 
-    it('should store the credentials correctly and generate a secret', function() {
+    it('should allow new registrations', function() {
+      plugin.exists = sinon.stub();
+      plugin.exists.onFirstCall().callsArgWith(1, null, false);
+      plugin.savePassphrase = sinon.stub();
+      plugin.savePassphrase.onFirstCall().callsArg(2);
+      plugin.saveEncryptedData = sinon.stub();
+      plugin.saveEncryptedData.onFirstCall().callsArg(2);
+      plugin.createVerificationSecretAndSendEmail = sinon.stub();
+      plugin.createVerificationSecretAndSendEmail.onFirstCall().callsArg(1);
+      response.send.onFirstCall().returnsThis();
 
       plugin.post(request, response);
 
-      assert(leveldb_stub.put.getCall(0).args[0] === 'credentials-store-2');
+      assert(plugin.exists.firstCall.args[0] === emailParam);
-      assert(leveldb_stub.put.getCall(0).args[1] === '3');
+      assert(plugin.savePassphrase.firstCall.args[0] === emailParam);
-      assert(leveldb_stub.put.getCall(1).args[0].indexOf('validation-code-1') === 0);
+      assert(plugin.savePassphrase.firstCall.args[1] === secretParam);
-      assert(leveldb_stub.put.getCall(1).args[1]);
+      assert(plugin.saveEncryptedData.firstCall.args[0] === emailParam);
-      assert(response.json.calledWith({success: true}));
+      assert(plugin.saveEncryptedData.firstCall.args[1] === recordParam);
+      assert(plugin.createVerificationSecretAndSendEmail.firstCall.args[0] === emailParam);
     });
 
-    it('should send an email on registration', function() {
+    it('should allow to overwrite data', function() {
+      plugin.exists = sinon.stub();
+      plugin.exists.onFirstCall().callsArgWith(1, null, true);
+      plugin.checkPassphrase = sinon.stub();
+      plugin.checkPassphrase.onFirstCall().callsArgWith(2, null, true);
+      plugin.saveEncryptedData = sinon.stub();
+      plugin.saveEncryptedData.onFirstCall().callsArg(2);
+      plugin.createVerificationSecretAndSendEmail = sinon.stub();
+      plugin.createVerificationSecretAndSendEmail.onFirstCall().callsArg(1);
+      response.send.onFirstCall().returnsThis();
 
       plugin.post(request, response);
 
-      assert(plugin.email.sendMail);
+      assert(plugin.exists.firstCall.args[0] === emailParam);
-      assert(plugin.email.sendMail.firstCall.args.length === 2);
+      assert(plugin.checkPassphrase.firstCall.args[0] === emailParam);
-      assert(plugin.email.sendMail.firstCall.args[0].to === '1');
+      assert(plugin.checkPassphrase.firstCall.args[1] === secretParam);
-    });
+      assert(plugin.saveEncryptedData.firstCall.args[0] === emailParam);
-
+      assert(plugin.saveEncryptedData.firstCall.args[1] === recordParam);
-    it('should allow the user to retrieve credentials', function() {
+      assert(plugin.createVerificationSecretAndSendEmail.firstCall.args[0] === emailParam);
-      request.param.onFirstCall().returns('secret');
-      leveldb_stub.get.reset();
-
-      var returnValue = '!@#$%';
-      leveldb_stub.get.onFirstCall().callsArgWith(1, null, returnValue);
-      response.send.returnsThis();
-  
-      plugin.get(request, response);
-  
-      assert(leveldb_stub.get.firstCall.args[0] === 'credentials-store-secret');
-      assert(response.send.calledWith(returnValue));
-      assert(response.end.calledOnce);
     });
   });
 
@@ -134,42 +140,24 @@ describe('emailstore test', function() {
     });
   });
 
-  describe('when validating registration data', function() {
+  describe('when retrieving data', function() {
 
-    beforeEach(function() {
+    it('should validate the secret and return the data', function() {
-      var data = 'email=1&secret=2&record=3';
+      request.param.onFirstCall().returns('email');
-      request.on.onFirstCall().callsArgWith(1, data);
+      request.param.onSecondCall().returns('secret');
-      request.on.onFirstCall().returnsThis();
+      plugin.retrieveDataByEmailAndPassphrase = sinon.stub();
-      request.on.onSecondCall().callsArg(1);
+      plugin.retrieveDataByEmailAndPassphrase.onFirstCall().callsArgWith(2, null, 'encrypted');
-      leveldb_stub.put = sinon.stub();
+      response.send.onFirstCall().returnsThis();
-      leveldb_stub.get = sinon.stub();
-      leveldb_stub.put.onFirstCall().callsArg(2);
-      leveldb_stub.put.onSecondCall().callsArg(2);
-      response.status.returnsThis();
-      response.json.returnsThis();
-    });
 
-    it('should\'nt allow the user to register with an already validated email', function() {
+      plugin.get(request, response);
-      leveldb_stub.get.onFirstCall().callsArgWith(1, null, {});
 
-      plugin.post(request, response);
+      assert(request.param.firstCall.args[0] === 'email');
-
+      assert(request.param.secondCall.args[0] === 'secret');
-      assert(response.status.firstCall.calledWith(409));
+      assert(plugin.retrieveDataByEmailAndPassphrase.firstCall.args[0] === 'email');
-      assert(response.json.firstCall.calledWith({error: 'That email is already registered'}));
+      assert(plugin.retrieveDataByEmailAndPassphrase.firstCall.args[1] === 'secret');
+      assert(response.send.firstCall.args[0] === 'encrypted');
       assert(response.end.calledOnce);
     });
-
-    it('should\'nt allow the user to register with a pending validation email', function() {
-      leveldb_stub.get.onFirstCall().callsArg(1);
-      leveldb_stub.get.onSecondCall().callsArgWith(1, null, {});
-
-      plugin.post(request, response);
-
-      assert(response.status.firstCall.args[0] === 409);
-      assert(response.json.firstCall.calledWith({error: 'That email is already registered'}));
-      assert(response.end.calledOnce);
-    });
-
   });
 });