From 946eafc96d86902adc8f4ed2c0bc0285cd7b3df8 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 10 Dec 2014 12:22:12 -0300 Subject: [PATCH 1/3] added expiration date to email validation --- plugins/emailstore.js | 79 +++++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/plugins/emailstore.js b/plugins/emailstore.js index 59f6f01c..46f93f5f 100644 --- a/plugins/emailstore.js +++ b/plugins/emailstore.js @@ -13,6 +13,7 @@ var levelup = require('levelup'); var nodemailer = require('nodemailer'); var querystring = require('querystring'); + var moment = require('moment'); var logger = require('../lib/logger').logger; var globalConfig = require('../config/config'); @@ -50,7 +51,11 @@ OVER_QUOTA: { code: 406, message: 'User quota exceeded', - } + }, + REGISTRATION_EXPIRED: { + code: 400, + message: 'Registration expired', + }, }; var EMAIL_TO_PASSPHRASE = 'email-to-passphrase-'; @@ -69,6 +74,8 @@ var POST_LIMIT = 1024 * 300 /* Max POST 300 kb */ ; + var DAYS_TO_EXPIRATION = 7; // An email can be awaiting validation for this long before expiring + var valueKey = function(email, key) { return STORED_VALUE + bitcore.util.twoSha256(email + SEPARATOR + key).toString('hex'); }; @@ -361,9 +368,20 @@ */ emailPlugin.createVerificationSecret = function(email, callback) { emailPlugin.db.get(pendingKey(email), function(err, value) { - if (err && err.notFound) { + var available = false; + + var notFound = err && err.notFound; + var expired = !err && _.isObject(value) && moment().isAfter(value.expires); + + var available = notFound || expired; + + if (available) { var secret = emailPlugin.crypto.randomBytes(16).toString('hex'); - emailPlugin.db.put(pendingKey(email), secret, function(err) { + var value = { + secret: secret, + expires: moment().add(DAYS_TO_EXPIRATION, 'days'), + }; + emailPlugin.db.put(pendingKey(email), value, function(err) { if (err) { logger.error('error saving pending data:', email, secret); return callback(emailPlugin.errors.INTERNAL_ERROR); @@ -741,29 +759,40 @@ code: 500, message: err }, response); - } else if (value !== secret) { - return emailPlugin.returnError(emailPlugin.errors.INVALID_CODE, response); - } else { - emailPlugin.db.put(validatedKey(email), true, function(err, value) { - if (err) { - return emailPlugin.returnError({ - code: 500, - message: err - }, response); - } else { - emailPlugin.db.del(pendingKey(email), function(err, value) { - if (err) { - return emailPlugin.returnError({ - code: 500, - message: err - }, response); - } else { - response.redirect(emailPlugin.redirectUrl); - } - }); - } - }); } + + var secret; + if (_.isObject(value)) { + if (moment().isAfter(value.expires)) { + return emailPlugin.returnError(emailPlugin.errors.REGISTRATION_EXPIRED, response); + } else { + value = value.secret; + } + } + + if (value !== secret) { + return emailPlugin.returnError(emailPlugin.errors.INVALID_CODE, response); + } + + emailPlugin.db.put(validatedKey(email), true, function(err, value) { + if (err) { + return emailPlugin.returnError({ + code: 500, + message: err + }, response); + } else { + emailPlugin.db.del(pendingKey(email), function(err, value) { + if (err) { + return emailPlugin.returnError({ + code: 500, + message: err + }, response); + } else { + response.redirect(emailPlugin.redirectUrl); + } + }); + } + }); }); }; From 83e6d9bad0b70921d52459eb7a487eccbf6de546 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 10 Dec 2014 13:26:15 -0300 Subject: [PATCH 2/3] added tests --- plugins/emailstore.js | 1 - test/test.EmailStore.js | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/plugins/emailstore.js b/plugins/emailstore.js index 46f93f5f..55417ecc 100644 --- a/plugins/emailstore.js +++ b/plugins/emailstore.js @@ -761,7 +761,6 @@ }, response); } - var secret; if (_.isObject(value)) { if (moment().isAfter(value.expires)) { return emailPlugin.returnError(emailPlugin.errors.REGISTRATION_EXPIRED, response); diff --git a/test/test.EmailStore.js b/test/test.EmailStore.js index ac898457..02c12ac7 100644 --- a/test/test.EmailStore.js +++ b/test/test.EmailStore.js @@ -8,6 +8,7 @@ var bitcore = require('bitcore'); var logger = require('../lib/logger').logger; var should = chai.should; var expect = chai.expect; +var moment = require('moment'); logger.transports.console.level = 'non'; @@ -225,9 +226,12 @@ describe('emailstore test', function() { it('saves data under the expected key', function(done) { setupLevelDb(); - + var clock = sinon.useFakeTimers(); plugin.createVerificationSecretAndSendEmail(fakeEmail, function(err) { - leveldb_stub.put.firstCall.args[1].should.equal(fakeRandom); + var arg = leveldb_stub.put.firstCall.args[1]; + arg.secret.should.equal(fakeRandom); + arg.expires.isSame(moment().add(7, 'days')).should.be.true; + clock.restore(); done(); }); }); @@ -363,7 +367,7 @@ describe('emailstore test', function() { response.json.returnsThis(); }); - it('should validate correctly an email if the secret matches', function() { + it('should validate correctly an email if the secret matches (without expiration date)', function() { leveldb_stub.get.onFirstCall().callsArgWith(1, null, secret); leveldb_stub.del = sinon.stub().yields(null); response.redirect = sinon.stub(); @@ -373,6 +377,19 @@ describe('emailstore test', function() { assert(response.redirect.firstCall.calledWith(plugin.redirectUrl)); }); + it('should validate correctly an email if the secret matches (using expiration date)', function() { + leveldb_stub.get.onFirstCall().callsArgWith(1, null, { + secret: secret, + expires: moment().add(7, 'days') + }); + leveldb_stub.del = sinon.stub().yields(null); + response.redirect = sinon.stub(); + + plugin.validate(request, response); + + assert(response.redirect.firstCall.calledWith(plugin.redirectUrl)); + }); + it('should fail to validate an email if the secret doesn\'t match', function() { var invalid = '3'; leveldb_stub.get.onFirstCall().callsArgWith(1, null, invalid); @@ -387,6 +404,23 @@ describe('emailstore test', function() { })); assert(response.end.calledOnce); }); + + it('should fail to validate an email if the secret has expired', function() { + leveldb_stub.get.onFirstCall().callsArgWith(1, null, { + secret: secret, + expires: moment().subtract(2, 'days') + }); + response.status.returnsThis(); + response.json.returnsThis(); + + plugin.validate(request, response); + + assert(response.status.firstCall.calledWith(plugin.errors.REGISTRATION_EXPIRED.code)); + assert(response.json.firstCall.calledWith({ + error: 'Registration expired' + })); + assert(response.end.calledOnce); + }); }); describe('removing items', function() { From ff0a8612fa395eea4629d315d586847b35f9fcbb Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 10 Dec 2014 13:32:44 -0300 Subject: [PATCH 3/3] storing unix timestamps --- plugins/emailstore.js | 6 +++--- test/test.EmailStore.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/emailstore.js b/plugins/emailstore.js index 55417ecc..54b1bc3f 100644 --- a/plugins/emailstore.js +++ b/plugins/emailstore.js @@ -371,7 +371,7 @@ var available = false; var notFound = err && err.notFound; - var expired = !err && _.isObject(value) && moment().isAfter(value.expires); + var expired = !err && _.isObject(value) && moment().unix() > value.expires; var available = notFound || expired; @@ -379,7 +379,7 @@ var secret = emailPlugin.crypto.randomBytes(16).toString('hex'); var value = { secret: secret, - expires: moment().add(DAYS_TO_EXPIRATION, 'days'), + expires: moment().add(DAYS_TO_EXPIRATION, 'days').unix(), }; emailPlugin.db.put(pendingKey(email), value, function(err) { if (err) { @@ -762,7 +762,7 @@ } if (_.isObject(value)) { - if (moment().isAfter(value.expires)) { + if (moment().unix() > value.expires) { return emailPlugin.returnError(emailPlugin.errors.REGISTRATION_EXPIRED, response); } else { value = value.secret; diff --git a/test/test.EmailStore.js b/test/test.EmailStore.js index 02c12ac7..d46993c2 100644 --- a/test/test.EmailStore.js +++ b/test/test.EmailStore.js @@ -230,7 +230,7 @@ describe('emailstore test', function() { plugin.createVerificationSecretAndSendEmail(fakeEmail, function(err) { var arg = leveldb_stub.put.firstCall.args[1]; arg.secret.should.equal(fakeRandom); - arg.expires.isSame(moment().add(7, 'days')).should.be.true; + arg.expires.should.equal(moment().add(7, 'days').unix()); clock.restore(); done(); }); @@ -380,7 +380,7 @@ describe('emailstore test', function() { it('should validate correctly an email if the secret matches (using expiration date)', function() { leveldb_stub.get.onFirstCall().callsArgWith(1, null, { secret: secret, - expires: moment().add(7, 'days') + expires: moment().add(7, 'days').unix(), }); leveldb_stub.del = sinon.stub().yields(null); response.redirect = sinon.stub(); @@ -408,7 +408,7 @@ describe('emailstore test', function() { it('should fail to validate an email if the secret has expired', function() { leveldb_stub.get.onFirstCall().callsArgWith(1, null, { secret: secret, - expires: moment().subtract(2, 'days') + expires: moment().subtract(2, 'days').unix(), }); response.status.returnsThis(); response.json.returnsThis();