Merge pull request #3896 from wolrajhti/patch-2

Fix bug in User.verify when confirm is disabled

Author: bajtos

common/models/user.js

@@ -736,22 +736,32 @@ module.exports = function(User) {
       (verifyOptions.protocol === 'https' && verifyOptions.port == '443')
     ) ? '' : ':' + verifyOptions.port;
 
-    var urlPath = joinUrlPath(
-      verifyOptions.restApiRoot,
-      userModel.http.path,
-      userModel.sharedClass.findMethodByName('confirm').http.path
-    );
+    if (!verifyOptions.verifyHref) {
+      const confirmMethod = userModel.sharedClass.findMethodByName('confirm');
+      if (!confirmMethod) {
+        throw new Error(
+          'Cannot build user verification URL, ' +
+            'the default confirm method is not public. ' +
+            'Please provide the URL in verifyOptions.verifyHref.');
+      }
 
-    verifyOptions.verifyHref = verifyOptions.verifyHref ||
-      verifyOptions.protocol +
-      '://' +
-      verifyOptions.host +
-      displayPort +
-      urlPath +
-      '?' + qs.stringify({
-        uid: '' + verifyOptions.user[pkName],
-        redirect: verifyOptions.redirect,
-      });
+      const urlPath = joinUrlPath(
+        verifyOptions.restApiRoot,
+        userModel.http.path,
+        confirmMethod.http.path
+      );
+
+      verifyOptions.verifyHref =
+        verifyOptions.protocol +
+        '://' +
+        verifyOptions.host +
+        displayPort +
+        urlPath +
+        '?' + qs.stringify({
+          uid: '' + verifyOptions.user[pkName],
+          redirect: verifyOptions.redirect,
+        });
+    }
 
     verifyOptions.to = verifyOptions.to || user.email;
     verifyOptions.subject = verifyOptions.subject || g.f('Thanks for Registering');
@@ -779,7 +789,10 @@ module.exports = function(User) {
 
     // TODO - support more verification types
     function sendEmail(user) {
-      verifyOptions.verifyHref += '&token=' + user.verificationToken;
+      verifyOptions.verifyHref +=
+        verifyOptions.verifyHref.indexOf('?') === -1 ? '?' : '&';
+      verifyOptions.verifyHref += 'token=' + user.verificationToken;
+
       verifyOptions.verificationToken = user.verificationToken;
       verifyOptions.text = verifyOptions.text || g.f('Please verify your email by opening ' +
         'this link in a web browser:\n\t%s', verifyOptions.verifyHref);

test/user.test.js

@@ -2155,6 +2155,27 @@ describe('User', function() {
           });
       });
 
+      it('handles the case when remote method "confirm" is disabled', () => {
+        let actualVerifyHref;
+        const VERIFY_HREF = 'http://example.com/a-verify-url';
+
+        Object.assign(verifyOptions, {
+          verifyHref: VERIFY_HREF,
+          templateFn: (options, cb) => {
+            actualVerifyHref = options.verifyHref;
+            cb(null, 'dummy body');
+          },
+        });
+
+        User.disableRemoteMethodByName('confirm');
+
+        return user.verify(verifyOptions)
+          .then(() => {
+            expect(actualVerifyHref.substring(0, VERIFY_HREF.length + 1))
+              .to.equal(`${VERIFY_HREF}?`);
+          });
+      });
+
       function givenUser() {
         return User.create({email: '[email protected]', password: 'pass'})
           .then(u => user = u);