Fix creation of verification links

bajtos · bajtos · commit ea05a2c4dcb2 · 2017-02-06T12:39:21.000+01:00
Fix User.prototype.verify to call `querystring.stringify` instead
of concatenating query-string components directly.

In particular, this fixes the bug where `options.redirect` containing
a hash fragment like `#/home?arg1=value1&amp;arg2=value2` produced incorrect
URL, because the `redirect` value was not correctly encoded.
diff --git a/common/models/user.js b/common/models/user.js
@@ -13,6 +13,7 @@ var isEmail = require('isemail');
 var loopback = require('../../lib/loopback');
 var utils = require('../../lib/utils');
 var path = require('path');
+var qs = require('querystring');
 var SALT_WORK_FACTOR = 10;
 var crypto = require('crypto');
 var MAX_PASSWORD_LENGTH = 72;
@@ -436,10 +437,10 @@ module.exports = function(User) {
       options.host +
       displayPort +
       urlPath +
-      '?uid=' +
-      options.user[pkName] +
-      '&redirect=' +
-      options.redirect;
+      '?' + qs.stringify({
+        uid: options.user[pkName],
+        redirect: options.redirect,
+      });
 
     options.templateFn = options.templateFn || createVerificationEmailBody;
 
diff --git a/test/user.test.js b/test/user.test.js
@@ -10,6 +10,7 @@ var request = require('supertest');
 var loopback = require('../');
 var User, AccessToken;
 var async = require('async');
+var url = require('url');
 
 describe('User', function() {
   this.timeout(10000);
@@ -1690,6 +1691,29 @@ describe('User', function() {
             expect(result.email).to.not.have.property('template');
           });
       });
+
+      it('allows hash fragment in redirectUrl', function() {
+        return User.create({email: 'test@example.com', password: 'pass'})
+          .then(user => {
+            let actualVerifyHref;
+            return user.verify({
+              type: 'email',
+              to: user.email,
+              from: 'noreply@myapp.org',
+              redirect: '#/some-path?a=1&b=2',
+              templateFn: (options, cb) => {
+                actualVerifyHref = options.verifyHref;
+                cb(null, 'dummy body');
+              },
+            })
+            .then(() => actualVerifyHref);
+          })
+          .then(verifyHref => {
+            var parsed = url.parse(verifyHref, true);
+            expect(parsed.query.redirect, 'redirect query')
+              .to.equal('#/some-path?a=1&b=2');
+          });
+      });
     });
 
     describe('User.confirm(options, fn)', function() {
