Merge pull request #1001 from ParsePlatform/flovilmart.queryStringForEmailResets

Properly querystring encode the parameters

Author: flovilmart

spec/ValidationAndPasswordsReset.spec.js

@@ -23,9 +23,9 @@ describe("Custom Pages Configuration", () => {
       },
       publicServerURL: "https://my.public.server.com/1"
     });
-    
+
     var config = new Config("test");
-    
+
     expect(config.invalidLinkURL).toEqual("myInvalidLink");
     expect(config.verifyEmailSuccessURL).toEqual("myVerifyEmailSuccess");
     expect(config.choosePasswordURL).toEqual("myChoosePassword");
@@ -78,7 +78,7 @@ describe("Email Verification", () => {
       }
     });
   });
-  
+
   it('does not send verification email when verification is enabled and email is not set', done => {
     var emailAdapter = {
       sendVerificationEmail: () => Promise.resolve(),
@@ -119,7 +119,7 @@ describe("Email Verification", () => {
       }
     });
   });
-  
+
   it('does send a validation email when updating the email', done => {
     var emailAdapter = {
       sendVerificationEmail: () => Promise.resolve(),
@@ -169,7 +169,7 @@ describe("Email Verification", () => {
       }
     });
   });
-  
+
   it('does send with a simple adapter', done => {
     var calls = 0;
     var emailAdapter = {
@@ -311,7 +311,7 @@ describe("Email Verification", () => {
           followRedirect: false,
         }, (error, response, body) => {
           expect(response.statusCode).toEqual(302);
-          expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/verify_email_success.html?username=zxcv');
+          expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/verify_email_success.html?username=user');
           user.fetch()
           .then(() => {
             expect(user.get('emailVerified')).toEqual(true);
@@ -342,7 +342,7 @@ describe("Email Verification", () => {
       publicServerURL: "http://localhost:8378/1"
     });
     user.setPassword("asdf");
-    user.setUsername("zxcv");
+    user.setUsername("user");
     user.set('email', '[email protected]');
     user.signUp();
   });
@@ -453,7 +453,7 @@ describe("Email Verification", () => {
 });
 
 describe("Password Reset", () => {
-  
+
   it('should send a password reset link', done => {
     var user = new Parse.User();
     var emailAdapter = {
@@ -468,7 +468,7 @@ describe("Password Reset", () => {
             return;
           }
           expect(response.statusCode).toEqual(302);
-          var re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=[a-zA-Z0-9]+\&id=test\&username=zxcv/;
+          var re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=[a-zA-Z0-9]+\&id=test\&username=zxcv%2Bzxcv/;
           expect(response.body.match(re)).not.toBe(null);
           done();
         });
@@ -491,7 +491,7 @@ describe("Password Reset", () => {
       publicServerURL: "http://localhost:8378/1"
     });
     user.setPassword("asdf");
-    user.setUsername("zxcv");
+    user.setUsername("zxcv+zxcv");
     user.set('email', '[email protected]');
     user.signUp().then(() => {
       Parse.User.requestPasswordReset('[email protected]', {
@@ -503,7 +503,7 @@ describe("Password Reset", () => {
       });
     });
   });
-  
+
   it('redirects you to invalid link if you try to request password for a nonexistant users email', done => {
     setServerConfiguration({
       serverURL: 'http://localhost:8378/1',
@@ -555,8 +555,8 @@ describe("Password Reset", () => {
             return;
           }
           var token = match[1];
-          
-          request.post({ 
+
+          request.post({
             url: "http://localhost:8378/1/apps/test/request_password_reset" ,
             body: `new_password=hello&token=${token}&username=zxcv`,
             headers: {
@@ -571,15 +571,15 @@ describe("Password Reset", () => {
             }
             expect(response.statusCode).toEqual(302);
             expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html');
-            
+
             Parse.User.logIn("zxcv", "hello").then(function(user){
               done();
             }, (err) => {
               console.error(err);
               fail("should login with new password");
               done();
             });
-            
+
           });
         });
       },
@@ -613,6 +613,5 @@ describe("Password Reset", () => {
       });
     });
   });
-  
-})
 
+})

src/PromiseRouter.js

@@ -21,18 +21,18 @@ export default class PromiseRouter {
     this.routes = routes;
     this.mountRoutes();
   }
-  
+
   // Leave the opportunity to
   // subclasses to mount their routes by overriding
   mountRoutes() {}
-  
+
   // Merge the routes into this one
   merge(router) {
     for (var route of router.routes) {
       this.routes.push(route);
     }
   };
-  
+
   route(method, path, ...handlers) {
     switch(method) {
     case 'POST':
@@ -45,7 +45,7 @@ export default class PromiseRouter {
     }
 
     let handler = handlers[0];
-  
+
     if (handlers.length > 1) {
       const length = handlers.length;
       handler = function(req) {
@@ -63,7 +63,7 @@ export default class PromiseRouter {
       handler: handler
     });
   };
-  
+
   // Returns an object with:
   //   handler: the handler that should deal with this request
   //   params: any :-params that got parsed from the path
@@ -99,7 +99,7 @@ export default class PromiseRouter {
       return {params: params, handler: route.handler};
     }
   };
-  
+
   // Mount the routes on this router onto an express app (or express router)
   mountOnto(expressApp) {
     for (var route of this.routes) {
@@ -121,7 +121,7 @@ export default class PromiseRouter {
       }
     }
   };
-  
+
   expressApp() {
     var expressApp = express();
     for (var route of this.routes) {
@@ -168,19 +168,21 @@ function makeExpressHandler(promiseHandler) {
         if (PromiseRouter.verbose) {
           console.log('response:', JSON.stringify(result, null, 2));
         }
-        
+
         var status = result.status || 200;
         res.status(status);
-        
+
         if (result.text) {
           return res.send(result.text);
         }
-        
-        if (result.location && !result.response) {
-          return res.redirect(result.location);
-        }
+
         if (result.location) {
           res.set('Location', result.location);
+          // Override the default expressjs response
+          // as it double encodes %encoded chars in URL
+          if (!result.response) {
+            return res.send('Found. Redirecting to '+result.location);
+          }
         }
         res.json(result.response);
       }, (e) => {

src/Routers/PublicAPIRouter.js

@@ -4,36 +4,38 @@ import Config from '../Config';
 import express from 'express';
 import path from 'path';
 import fs from 'fs';
+import qs from 'querystring';
 
 let public_html = path.resolve(__dirname, "../../public_html");
 let views = path.resolve(__dirname, '../../views');
 
 export class PublicAPIRouter extends PromiseRouter {
-  
+
   verifyEmail(req) {
     let { token, username }= req.query;
     let appId = req.params.appId;
     let config = new Config(appId);
-    
+
     if (!config.publicServerURL) {
       return this.missingPublicServerURL();
     }
-    
+
     if (!token || !username) {
       return this.invalidLink(req);
     }
 
     let userController = config.userController;
     return userController.verifyEmail(username, token).then( () => {
+      let params = qs.stringify({username});
       return Promise.resolve({
         status: 302,
-        location: `${config.verifyEmailSuccessURL}?username=${username}`
+        location: `${config.verifyEmailSuccessURL}?${params}`
       });
     }, ()=> {
       return this.invalidLink(req);
     })
   }
-  
+
   changePassword(req) {
     return new Promise((resolve, reject) => {
       let config = new Config(req.query.id);
@@ -55,61 +57,63 @@ export class PublicAPIRouter extends PromiseRouter {
       });
     });
   }
-  
+
   requestResetPassword(req) {
 
     let config = req.config;
-    
+
     if (!config.publicServerURL) {
       return this.missingPublicServerURL();
     }
-    
+
     let { username, token } = req.query;
-    
+
     if (!username || !token) {
       return this.invalidLink(req);
     }
-    
+
     return config.userController.checkResetTokenValidity(username, token).then( (user) => {
+      let params = qs.stringify({token, id: config.applicationId, username, app: config.appName, });
       return Promise.resolve({
         status: 302,
-        location: `${config.choosePasswordURL}?token=${token}&id=${config.applicationId}&username=${username}&app=${config.appName}`
+        location: `${config.choosePasswordURL}?${params}`
       })
     }, () => {
       return this.invalidLink(req);
     })
   }
-  
+
   resetPassword(req) {
 
     let config = req.config;
-    
+
     if (!config.publicServerURL) {
       return this.missingPublicServerURL();
     }
-    
+
     let {
       username,
       token,
       new_password
     } = req.body;
-    
+
     if (!username || !token || !new_password) {
       return this.invalidLink(req);
     }
-    
+
     return config.userController.updatePassword(username, token, new_password).then((result) => {
       return Promise.resolve({
         status: 302,
         location: config.passwordResetSuccessURL
       });
     }, (err) => {
+      let params = qs.stringify({username: username, token: token, id: config.applicationId, error:err, app:config.appName})
       return Promise.resolve({
         status: 302,
-        location: `${config.choosePasswordURL}?token=${token}&id=${config.applicationId}&username=${username}&error=${err}&app=${config.appName}`
+        location: `${config.choosePasswordURL}?${params}`
       });
     });
-    
+
   }
 
   invalidLink(req) {
@@ -118,36 +122,36 @@ export class PublicAPIRouter extends PromiseRouter {
         location: req.config.invalidLinkURL
     });
   }
-  
+
   missingPublicServerURL() {
     return Promise.resolve({
       text:  'Not found.',
       status: 404
     });
   }
-  
+
   setConfig(req) {
     req.config = new Config(req.params.appId);
     return Promise.resolve();
   }
-  
+
   mountRoutes() {
-    this.route('GET','/apps/:appId/verify_email',  
-      req => { this.setConfig(req) }, 
+    this.route('GET','/apps/:appId/verify_email',
+      req => { this.setConfig(req) },
       req => { return this.verifyEmail(req); });
-    
-    this.route('GET','/apps/choose_password', 
+
+    this.route('GET','/apps/choose_password',
       req => { return this.changePassword(req); });
-    
-    this.route('POST','/apps/:appId/request_password_reset', 
-      req => { this.setConfig(req) }, 
+
+    this.route('POST','/apps/:appId/request_password_reset',
+      req => { this.setConfig(req) },
       req => { return this.resetPassword(req); });
-    
-    this.route('GET','/apps/:appId/request_password_reset', 
-      req => { this.setConfig(req) }, 
+
+    this.route('GET','/apps/:appId/request_password_reset',
+      req => { this.setConfig(req) },
       req => { return this.requestResetPassword(req); });
   }
-  
+
   expressApp() {
     let router = express();
     router.use("/apps", express.static(public_html));