From 005549a9a791b0fb7c53a7f4097cad3d7fe134f0 Mon Sep 17 00:00:00 2001 From: Vlad Filippov Date: Tue, 21 Mar 2017 12:09:34 -0400 Subject: [PATCH] feat(server): try to verify emails on the server (#4794) r=vbudhram --- app/scripts/models/account.js | 34 +++-- app/scripts/models/user.js | 1 + app/scripts/views/complete_sign_up.js | 1 + app/tests/spec/models/account.js | 42 ++++++ app/tests/spec/views/complete_sign_up.js | 26 +++- server/lib/routes.js | 1 + server/lib/routes/get-frontend.js | 3 +- server/lib/routes/get-verify-email.js | 116 ++++++++++++++++ tests/.eslintrc | 1 + tests/intern_server.js | 1 + tests/server/routes/get-verify-email.js | 160 +++++++++++++++++++++++ 11 files changed, 368 insertions(+), 18 deletions(-) create mode 100644 server/lib/routes/get-verify-email.js create mode 100644 tests/server/routes/get-verify-email.js diff --git a/app/scripts/models/account.js b/app/scripts/models/account.js index 4854c20e0..a724bece3 100644 --- a/app/scripts/models/account.js +++ b/app/scripts/models/account.js @@ -482,23 +482,31 @@ define(function (require, exports, module) { * @param {String} code - the verification code * @param {Object} [options] * @param {Object} [options.service] - the service issuing signup request + * @param {String} [options.serverVerificationStatus] - the status of server verification * @returns {Promise} - resolves when complete */ verifySignUp (code, options = {}) { - return this._fxaClient.verifyCode( - this.get('uid'), - code, - options - ) - .then(() => { - this.set('verified', true); + return p() + .then(() => { + if (options.serverVerificationStatus !== 'verified') { + // if server verification was not present or not successful + // then attempt client verification + return this._fxaClient.verifyCode( + this.get('uid'), + code, + options + ); + } + }) + .then(() => { + this.set('verified', true); - if (this.get('needsOptedInToMarketingEmail')) { - this.unset('needsOptedInToMarketingEmail'); - var emailPrefs = this.getMarketingEmailPrefs(); - return emailPrefs.optIn(NEWSLETTER_ID); - } - }); + if (this.get('needsOptedInToMarketingEmail')) { + this.unset('needsOptedInToMarketingEmail'); + var emailPrefs = this.getMarketingEmailPrefs(); + return emailPrefs.optIn(NEWSLETTER_ID); + } + }); }, /** diff --git a/app/scripts/models/user.js b/app/scripts/models/user.js index 63d55c7e5..2f51297f8 100644 --- a/app/scripts/models/user.js +++ b/app/scripts/models/user.js @@ -482,6 +482,7 @@ define(function (require, exports, module) { * @param {String} code - verification code * @param {Object} [options] * @param {Object} [options.service] - the service issuing signup request + * @param {String} [options.serverVerificationStatus] - the status of server verification * @returns {Promise} - resolves with the account when complete */ completeAccountSignUp (account, code, options) { diff --git a/app/scripts/views/complete_sign_up.js b/app/scripts/views/complete_sign_up.js index 1e8167a8c..e58283924 100644 --- a/app/scripts/views/complete_sign_up.js +++ b/app/scripts/views/complete_sign_up.js @@ -74,6 +74,7 @@ define(function (require, exports, module) { const code = verificationInfo.get('code'); const options = { reminder: verificationInfo.get('reminder'), + serverVerificationStatus: this.getSearchParam('server_verification') || null, service: this.relier.get('service') }; diff --git a/app/tests/spec/models/account.js b/app/tests/spec/models/account.js index 09b003d88..20d7e1d94 100644 --- a/app/tests/spec/models/account.js +++ b/app/tests/spec/models/account.js @@ -691,6 +691,48 @@ define(function (require, exports, module) { }); describe('verifySignUp', function () { + describe('with custom server verification value', function () { + beforeEach(function () { + sinon.stub(fxaClient, 'verifyCode', function () { + return p(); + }); + }); + + it('does not call verifyCode with verified', function () { + account.set('uid', UID); + + return account.verifySignUp('CODE', { + serverVerificationStatus: 'verified' + }).then(() => { + assert.isFalse(fxaClient.verifyCode.called); + assert.isTrue(account.get('verified')); + }); + }); + + it('calls verifyCode with other status', function () { + account.set('uid', UID); + + return account.verifySignUp('CODE', { + serverVerificationStatus: 'test' + }).then(() => { + assert.isTrue(fxaClient.verifyCode.called); + assert.isTrue(account.get('verified')); + }); + }); + + it('calls verifyCode with undefined status', function () { + account.set('uid', UID); + + return account.verifySignUp('CODE', { + serverVerificationStatus: undefined + }).then(() => { + assert.isTrue(fxaClient.verifyCode.called); + assert.isTrue(account.get('verified')); + }); + }); + + }); + describe('without email opt-in', function () { beforeEach(function () { sinon.stub(fxaClient, 'verifyCode', function () { diff --git a/app/tests/spec/views/complete_sign_up.js b/app/tests/spec/views/complete_sign_up.js index 73668495c..9972db0ce 100644 --- a/app/tests/spec/views/complete_sign_up.js +++ b/app/tests/spec/views/complete_sign_up.js @@ -198,7 +198,7 @@ define(function (require, exports, module) { var args = account.verifySignUp.getCall(0).args; assert.isTrue(account.verifySignUp.called); assert.ok(args[0]); - assert.deepEqual(args[1], {reminder: null, service: validService}); + assert.deepEqual(args[1], {reminder: null, serverVerificationStatus: null, service: validService}); }); }); @@ -217,7 +217,7 @@ define(function (require, exports, module) { var args = account.verifySignUp.getCall(0).args; assert.isTrue(account.verifySignUp.called); assert.ok(args[0]); - assert.deepEqual(args[1], {reminder: validReminder, service: null}); + assert.deepEqual(args[1], {reminder: validReminder, serverVerificationStatus: null, service: null}); }); }); @@ -237,7 +237,27 @@ define(function (require, exports, module) { var args = account.verifySignUp.getCall(0).args; assert.isTrue(account.verifySignUp.called); assert.ok(args[0]); - assert.deepEqual(args[1], {reminder: validReminder, service: validService}); + assert.deepEqual(args[1], {reminder: validReminder, serverVerificationStatus: null, service: validService}); + }); + }); + + describe('if server_verification is in the url', function () { + beforeEach(function () { + windowMock.location.search = '?code=' + validCode + '&uid=' + validUid + + '&server_verification=verified'; + relier = new Relier({}, { + window: windowMock + }); + relier.fetch(); + initView(account); + return view.render(); + }); + + it('attempt to pass server_verification to verifySignUp', function () { + var args = account.verifySignUp.getCall(0).args; + assert.isTrue(account.verifySignUp.called); + assert.ok(args[0]); + assert.deepEqual(args[1], {reminder: null, serverVerificationStatus: 'verified', service: null}); }); }); diff --git a/server/lib/routes.js b/server/lib/routes.js index 989717fa0..08bb86c1c 100644 --- a/server/lib/routes.js +++ b/server/lib/routes.js @@ -29,6 +29,7 @@ module.exports = function (config, i18n) { redirectVersionedToUnversioned('complete_reset_password'), redirectVersionedToUnversioned('reset_password'), redirectVersionedToUnversioned('verify_email'), + require('./routes/get-verify-email')(), require('./routes/get-frontend')(), require('./routes/get-terms-privacy')(i18n), require('./routes/get-index')(config), diff --git a/server/lib/routes/get-frontend.js b/server/lib/routes/get-frontend.js index 2d2ab5f0d..2b490dfc9 100644 --- a/server/lib/routes/get-frontend.js +++ b/server/lib/routes/get-frontend.js @@ -51,8 +51,7 @@ module.exports = function () { 'signup_verified', 'sms', 'sms/sent', - 'sms/why', - 'verify_email' + 'sms/why' ].join('|'); // prepare for use in a RegExp return { diff --git a/server/lib/routes/get-verify-email.js b/server/lib/routes/get-verify-email.js new file mode 100644 index 000000000..4113144f6 --- /dev/null +++ b/server/lib/routes/get-verify-email.js @@ -0,0 +1,116 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const url = require('url'); +const got = require('got'); +const logger = require('mozlog')('server.get-verify-email'); +const config = require('../configuration'); +const ravenClient = require('../../lib/raven').ravenMiddleware; +const joi = require('joi'); +const validation = require('../validation'); + +const fxaAccountUrl = config.get('fxaccount_url'); +const STRING_TYPE = validation.TYPES.STRING; + +const VERIFICATION_ENDPOINT = `${fxaAccountUrl}/v1/recovery_email/verify_code`; +const VERIFICATION_TIMEOUT = 5000; +// Sentry combines both Info and Error into one if same name +const VERIFICATION_LEVEL_ERROR = 'VerificationValidationError'; +const VERIFICATION_LEVEL_INFO = 'VerificationValidationInfo'; + +const REQUIRED_SCHEMA = { + 'code': joi.string().hex().min(32).max(32).required(), + 'uid': joi.string().hex().min(32).max(32).required() +}; + +const REPORT_ONLY_SCHEMA = { + 'code': STRING_TYPE.alphanum().min(32).max(32).required(), + // resume token can be long, do not use the limited STRING_TYPE + 'resume': joi.string().alphanum().optional(), + 'service': STRING_TYPE.alphanum().max(100).optional(), + 'uid': STRING_TYPE.alphanum().min(32).max(32).required(), + 'utm_campaign': STRING_TYPE.alphanum().optional(), + 'utm_content': STRING_TYPE.alphanum().optional(), + 'utm_medium': STRING_TYPE.alphanum().optional(), + 'utm_source': STRING_TYPE.alphanum().optional() +}; + +module.exports = function () { + return { + method: 'get', + path: '/verify_email', + process: function (req, res, next) { + const rawQuery = url.parse(req.url).query; + + // reset the url for the front-end router + req.url = '/'; + + if (req.query.server_verification) { + return next(); + } + + const data = { + code: req.query.code, + uid: req.query.uid + }; + + joi.validate(data, REQUIRED_SCHEMA, (err) => { + if (err) { + ravenClient.captureMessage(VERIFICATION_LEVEL_ERROR, { + extra: { + details: err.details + } + }); + // if cannot validate required params then just forward to front-end + return next(); + } + + if (req.query.service) { + data.service = req.query.service; + } + + if (req.query.reminder) { + data.reminder = req.query.reminder; + } + + const options = { + body: data, + retries: 0, + timeout: { + connect: VERIFICATION_TIMEOUT, + socket: VERIFICATION_TIMEOUT + } + }; + + got.post(VERIFICATION_ENDPOINT, options) + .then(() => { + // In some cases the code can only be used once. + // Here we add an extra query param to signal the front-end that verification succeeded. + // See issue #4800 + return res.redirect(`/verify_email?${rawQuery}&server_verification=verified`); + }) + .catch((err) => { + ravenClient.captureError(err); + logger.error(err); + // failed to verify, continue to front-end + next(); + + }); + }); + + // Passive validation and error reporting, could be made required in the future. + joi.validate(req.query, REPORT_ONLY_SCHEMA, (err) => { + if (err) { + ravenClient.captureMessage(VERIFICATION_LEVEL_INFO, { + extra: { + details: err.details + }, + level: 'info' + }); + } + }); + + } + }; +}; diff --git a/tests/.eslintrc b/tests/.eslintrc index 31e1da46e..12b9eda2c 100644 --- a/tests/.eslintrc +++ b/tests/.eslintrc @@ -4,3 +4,4 @@ extends: ../.eslintrc rules: id-blacklist: 0 + strict: 0 diff --git a/tests/intern_server.js b/tests/intern_server.js index 84ee4d6ee..863b27683 100644 --- a/tests/intern_server.js +++ b/tests/intern_server.js @@ -30,6 +30,7 @@ define([ 'tests/server/statsd-collector', 'tests/server/raven', 'tests/server/routes/get-config', + 'tests/server/routes/get-verify-email', 'tests/server/routes/get-fxa-client-configuration', 'tests/server/routes/get-openid-configuration', 'tests/server/routes/get-index', diff --git a/tests/server/routes/get-verify-email.js b/tests/server/routes/get-verify-email.js new file mode 100644 index 000000000..a68eb8ace --- /dev/null +++ b/tests/server/routes/get-verify-email.js @@ -0,0 +1,160 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +'use strict'; + +define([ + 'intern', + 'intern!object', + 'intern/chai!assert', + 'intern/dojo/node!../../../server/lib/configuration', + 'intern/dojo/Promise', + 'intern/dojo/node!got', + 'intern/dojo/node!fs', + 'intern/dojo/node!path', + 'intern/dojo/node!proxyquire', + 'intern/dojo/node!sinon', +], function (intern, registerSuite, assert, config, dojoPromise, got, fs, path, proxyquire, sinon) { + + function mockModule(mocks) { + return proxyquire(path.join(process.cwd(), 'server', 'lib', 'routes', 'get-verify-email'), mocks)(); + } + + let logger; + let mocks; + let ravenMock; + let gotMock; + const res = { + json: () => {} + }; + + registerSuite({ + name: 'verify_email', + + beforeEach() { + gotMock = { + post: sinon.spy() + }; + logger = { + error: sinon.spy() + }; + ravenMock = { + ravenMiddleware: { + captureError: sinon.spy(), + captureMessage: sinon.spy() + } + }; + mocks = { + '../../lib/raven': ravenMock, + 'got': gotMock, + mozlog: () => { + return logger; + }, + }; + }, + + 'logs error without query params' () { + const dfd = new dojoPromise.Deferred(); + const req = { + query: { + code: '', + uid: '' + }, + url: '/verify_email' + }; + + mockModule(mocks).process(req, res, () => { + var c = ravenMock.ravenMiddleware.captureMessage; + var arg = c.args[0]; + assert.equal(c.calledOnce, true); + assert.equal(arg[0], 'VerificationValidationError'); + assert.equal(arg[1].extra.details[0].message, '"code" is not allowed to be empty'); + dfd.resolve(); + }); + + return dfd.dojoPromise; + }, + + 'no logs if successful' () { + const dfd = new dojoPromise.Deferred(); + mocks.got = { + post: (req, res, next) => { + return new Promise((resolve, reject) => { + resolve({ + 'statusCode': 200, + 'statusMessage': 'OK' + }); + }); + } + }; + + const req = { + query: { + code: '12345678912345678912345678912312', + uid: '12345678912345678912345678912312' + }, + url: '/verify_email' + }; + + mockModule(mocks).process(req, res, () => { + assert.equal(logger.error.callCount, 0); + assert.equal(ravenMock.ravenMiddleware.captureMessage.callCount, 0); + assert.equal(ravenMock.ravenMiddleware.captureError.callCount, 0); + + req.query.something = 'else'; + + mockModule(mocks).process(req, res, () => { + assert.equal(logger.error.callCount, 0); + assert.equal(ravenMock.ravenMiddleware.captureMessage.callCount, 0); + assert.equal(ravenMock.ravenMiddleware.captureError.callCount, 0); + dfd.resolve(); + }); + }); + + return dfd.dojoPromise; + }, + + 'logs errors when post fails' () { + const dfd = new dojoPromise.Deferred(); + mocks.got = { + post: (req, res, next) => { + return new Promise((resolve, reject) => { + reject({ + 'host': '127.0.0.1:9000', + 'hostname': '127.0.0.1', + 'message': 'Response code 400 (Bad Request)', + 'method': 'POST', + 'path': '/v1/recovery_email/verify_code', + 'statusCode': 400, + 'statusMessage': 'Bad Request' + }); + }); + } + }; + + const req = { + query: { + code: '12345678912345678912345678912312', + uid: '12345678912345678912345678912312' + }, + url: '/verify_email' + }; + + mockModule(mocks).process(req, res, () => { + assert.equal(logger.error.callCount, 1); + assert.equal(ravenMock.ravenMiddleware.captureMessage.callCount, 0); + assert.equal(ravenMock.ravenMiddleware.captureError.callCount, 1); + const result = ravenMock.ravenMiddleware.captureError.args[0][0]; + assert.equal(result.statusCode, 400); + assert.equal(result.statusMessage, 'Bad Request'); + + dfd.resolve(); + }); + + return dfd.dojoPromise; + } + + }); + +});