From eacf9a3aa33b5c4a519ca430f560a67db6a88eca Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Tue, 26 Apr 2016 16:50:42 -0700 Subject: [PATCH 1/5] Regression test #1649 --- spec/index.spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/index.spec.js b/spec/index.spec.js index d0d54401b1..a7cc1a7a4f 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -361,4 +361,26 @@ describe('server', () => { expect(() => setServerConfiguration({ revokeSessionOnPasswordReset: 'non-bool' })).toThrow(); done(); }); + + it('fails if you set verifyUserEmails to true without setting an app ID or publicServerURL (regression test for #1649)', done => { + expect(() => setServerConfiguration({ + serverURL: 'http://localhost:8378/1', + appId: 'test', + appName: 'unused', + javascriptKey: 'test', + dotNetKey: 'windows', + clientKey: 'client', + restAPIKey: 'rest', + masterKey: 'test', + collectionPrefix: 'test_', + fileKey: 'test', + verifyUserEmails: true, + emailAdapter: MockEmailAdapterWithOptions({ + fromAddress: 'parse@example.com', + apiKey: 'k', + domain: 'd', + }), + })).toThrow('A public server url is required when using email verification.'); + done(); + }); }); From 0737381534590d2088e42a270fea701fb2f93744 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 9 May 2016 17:36:11 -0700 Subject: [PATCH 2/5] Address comments --- spec/ValidationAndPasswordsReset.spec.js | 38 ++++++++++++++++++++++-- spec/index.spec.js | 22 -------------- src/Routers/UsersRouter.js | 38 ++++++++++++++++-------- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/spec/ValidationAndPasswordsReset.spec.js b/spec/ValidationAndPasswordsReset.spec.js index e953204931..a0981e1602 100644 --- a/spec/ValidationAndPasswordsReset.spec.js +++ b/spec/ValidationAndPasswordsReset.spec.js @@ -1,7 +1,9 @@ "use strict"; -var request = require('request'); -var Config = require("../src/Config"); +let MockEmailAdapterWithOptions = require('./MockEmailAdapterWithOptions'); +let request = require('request'); +let Config = require("../src/Config"); + describe("Custom Pages Configuration", () => { it("should set the custom pages", (done) => { setServerConfiguration({ @@ -282,6 +284,38 @@ describe("Email Verification", () => { }); }); + it('fails if you set include an emailAdapter, set verifyUserEmails to false, dont set a publicServerURL, and try to send a password reset email (regression test for #1649)', done => { + setServerConfiguration({ + serverURL: 'http://localhost:8378/1', + appId: 'test', + appName: 'unused', + javascriptKey: 'test', + dotNetKey: 'windows', + clientKey: 'client', + restAPIKey: 'rest', + masterKey: 'test', + collectionPrefix: 'test_', + fileKey: 'test', + verifyUserEmails: false, + emailAdapter: MockEmailAdapterWithOptions({ + fromAddress: 'parse@example.com', + apiKey: 'k', + domain: 'd', + }), + }) + + let user = new Parse.User(); + user.setPassword("asdf"); + user.setUsername("zxcv"); + user.set("email", "cool_guy@parse.com"); + user.signUp(null) + .then(user => Parse.User.requestPasswordReset("cool_guy@parse.com")) + .catch(error => { + expect(error.message).toEqual('An appName, publicServerURL, and emailAdapter are required for password reset functionality.') + done(); + }); + }); + it('does not send verification email if email verification is disabled', done => { var emailAdapter = { sendVerificationEmail: () => Promise.resolve(), diff --git a/spec/index.spec.js b/spec/index.spec.js index a7cc1a7a4f..d0d54401b1 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -361,26 +361,4 @@ describe('server', () => { expect(() => setServerConfiguration({ revokeSessionOnPasswordReset: 'non-bool' })).toThrow(); done(); }); - - it('fails if you set verifyUserEmails to true without setting an app ID or publicServerURL (regression test for #1649)', done => { - expect(() => setServerConfiguration({ - serverURL: 'http://localhost:8378/1', - appId: 'test', - appName: 'unused', - javascriptKey: 'test', - dotNetKey: 'windows', - clientKey: 'client', - restAPIKey: 'rest', - masterKey: 'test', - collectionPrefix: 'test_', - fileKey: 'test', - verifyUserEmails: true, - emailAdapter: MockEmailAdapterWithOptions({ - fromAddress: 'parse@example.com', - apiKey: 'k', - domain: 'd', - }), - })).toThrow('A public server url is required when using email verification.'); - done(); - }); }); diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index adba752f83..f92f42deef 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -1,7 +1,7 @@ // These methods handle the User-related routes. import deepcopy from 'deepcopy'; - +import Config from '../Config'; import ClassesRouter from './ClassesRouter'; import PromiseRouter from '../PromiseRouter'; import rest from '../rest'; @@ -155,19 +155,31 @@ export class UsersRouter extends ClassesRouter { } handleResetRequest(req) { - let { email } = req.body; - if (!email) { - throw new Parse.Error(Parse.Error.EMAIL_MISSING, "you must provide an email"); - } - let userController = req.config.userController; - + try { + Config.validateEmailConfiguration({ + verifyUserEmails: true, //A bit of a hack, as this isn't the intended purpose of this parameter + appName: req.config.appName, + publicServerURL: req.config.publicServerURL, + }); + } catch (e) { + if (typeof e === 'string') { + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'An appName, publicServerURL, and emailAdapter are required for password reset functionality.'); + } else { + throw e; + } + } + let { email } = req.body; + if (!email) { + throw new Parse.Error(Parse.Error.EMAIL_MISSING, "you must provide an email"); + } + let userController = req.config.userController; return userController.sendPasswordResetEmail(email).then((token) => { - return Promise.resolve({ - response: {} - }); - }, (err) => { - throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `no user found with email ${email}`); - }); + return Promise.resolve({ + response: {} + }); + }, (err) => { + throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `no user found with email ${email}`); + }); } From 8b910dba1d57b1ef2272ef1b4fa028a8cfd01398 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 9 May 2016 17:37:20 -0700 Subject: [PATCH 3/5] Comment --- src/Routers/UsersRouter.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index f92f42deef..319c6c67f9 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -163,6 +163,7 @@ export class UsersRouter extends ClassesRouter { }); } catch (e) { if (typeof e === 'string') { + // Maybe we need a Bad Configuration error, but the SDKs won't understand it. For now, Internal Server Error. throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'An appName, publicServerURL, and emailAdapter are required for password reset functionality.'); } else { throw e; From eea60d11a0edef8f6911a64ed14c2dd19e409c57 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Tue, 17 May 2016 18:17:06 -0700 Subject: [PATCH 4/5] Change emails to help debug flaky test failures --- spec/ValidationAndPasswordsReset.spec.js | 18 +++++++++--------- src/Routers/UsersRouter.js | 7 ++++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/spec/ValidationAndPasswordsReset.spec.js b/spec/ValidationAndPasswordsReset.spec.js index a0981e1602..dbcd838f6e 100644 --- a/spec/ValidationAndPasswordsReset.spec.js +++ b/spec/ValidationAndPasswordsReset.spec.js @@ -64,7 +64,7 @@ describe("Email Verification", () => { var user = new Parse.User(); user.setPassword("asdf"); user.setUsername("zxcv"); - user.setEmail('cool_guy@parse.com'); + user.setEmail('testIfEnabled@parse.com'); user.signUp(null, { success: function(user) { expect(emailAdapter.sendVerificationEmail).toHaveBeenCalled(); @@ -152,7 +152,7 @@ describe("Email Verification", () => { expect(emailAdapter.sendVerificationEmail).not.toHaveBeenCalled(); user.fetch() .then((user) => { - user.set("email", "cool_guy@parse.com"); + user.set("email", "testWhenUpdating@parse.com"); return user.save(); }).then((user) => { return user.fetch(); @@ -206,7 +206,7 @@ describe("Email Verification", () => { expect(emailAdapter.sendVerificationEmail).not.toHaveBeenCalled(); user.fetch() .then((user) => { - user.set("email", "cool_guy@parse.com"); + user.set("email", "testValidLinkWhenUpdating@parse.com"); return user.save(); }).then((user) => { return user.fetch(); @@ -230,7 +230,7 @@ describe("Email Verification", () => { var calls = 0; var emailAdapter = { sendMail: function(options){ - expect(options.to).toBe('cool_guy@parse.com'); + expect(options.to).toBe('testSendSimpleAdapter@parse.com'); if (calls == 0) { expect(options.subject).toEqual('Please verify your e-mail for My Cool App'); expect(options.text.match(/verify_email/)).not.toBe(null); @@ -260,7 +260,7 @@ describe("Email Verification", () => { var user = new Parse.User(); user.setPassword("asdf"); user.setUsername("zxcv"); - user.set("email", "cool_guy@parse.com"); + user.set("email", "testSendSimpleAdapter@parse.com"); user.signUp(null, { success: function(user) { expect(calls).toBe(1); @@ -268,7 +268,7 @@ describe("Email Verification", () => { .then((user) => { return user.save(); }).then((user) => { - return Parse.User.requestPasswordReset("cool_guy@parse.com").catch((err) => { + return Parse.User.requestPasswordReset("testSendSimpleAdapter@parse.com").catch((err) => { fail('Should not fail requesting a password'); done(); }) @@ -284,7 +284,7 @@ describe("Email Verification", () => { }); }); - it('fails if you set include an emailAdapter, set verifyUserEmails to false, dont set a publicServerURL, and try to send a password reset email (regression test for #1649)', done => { + it('fails if you include an emailAdapter, set verifyUserEmails to false, dont set a publicServerURL, and try to send a password reset email (regression test for #1649)', done => { setServerConfiguration({ serverURL: 'http://localhost:8378/1', appId: 'test', @@ -307,9 +307,9 @@ describe("Email Verification", () => { let user = new Parse.User(); user.setPassword("asdf"); user.setUsername("zxcv"); - user.set("email", "cool_guy@parse.com"); + user.set("email", "testInvalidConfig@parse.com"); user.signUp(null) - .then(user => Parse.User.requestPasswordReset("cool_guy@parse.com")) + .then(user => Parse.User.requestPasswordReset("testInvalidConfig@parse.com")) .catch(error => { expect(error.message).toEqual('An appName, publicServerURL, and emailAdapter are required for password reset functionality.') done(); diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index 319c6c67f9..50f4502cba 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -174,12 +174,13 @@ export class UsersRouter extends ClassesRouter { throw new Parse.Error(Parse.Error.EMAIL_MISSING, "you must provide an email"); } let userController = req.config.userController; - return userController.sendPasswordResetEmail(email).then((token) => { + return userController.sendPasswordResetEmail(email).then((token) => { return Promise.resolve({ response: {} }); - }, (err) => { - throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `no user found with email ${email}`); + }, err => { + console.log(err); + throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `No user found with email ${email}.`); }); } From e99b189db55307426fa3c0304b2aa429a894e0bc Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Tue, 17 May 2016 19:25:14 -0700 Subject: [PATCH 5/5] More logging info to debug flaky tests --- spec/ValidationAndPasswordsReset.spec.js | 6 +++++- src/Routers/UsersRouter.js | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/spec/ValidationAndPasswordsReset.spec.js b/spec/ValidationAndPasswordsReset.spec.js index dbcd838f6e..cac3c56cfd 100644 --- a/spec/ValidationAndPasswordsReset.spec.js +++ b/spec/ValidationAndPasswordsReset.spec.js @@ -310,7 +310,11 @@ describe("Email Verification", () => { user.set("email", "testInvalidConfig@parse.com"); user.signUp(null) .then(user => Parse.User.requestPasswordReset("testInvalidConfig@parse.com")) - .catch(error => { + .then(result => { + console.log(result); + fail('sending password reset email should not have succeeded'); + done(); + }, error => { expect(error.message).toEqual('An appName, publicServerURL, and emailAdapter are required for password reset functionality.') done(); }); diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index 50f4502cba..1d73359eb0 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -174,13 +174,16 @@ export class UsersRouter extends ClassesRouter { throw new Parse.Error(Parse.Error.EMAIL_MISSING, "you must provide an email"); } let userController = req.config.userController; - return userController.sendPasswordResetEmail(email).then((token) => { + return userController.sendPasswordResetEmail(email).then(token => { return Promise.resolve({ response: {} }); }, err => { - console.log(err); - throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `No user found with email ${email}.`); + if (err.code === Parse.Error.OBJECT_NOT_FOUND) { + throw new Parse.Error(Parse.Error.EMAIL_NOT_FOUND, `No user found with email ${email}.`); + } else { + throw err; + } }); }