From c5f642cb86aeef78c7ba50ea8066359a3887916b Mon Sep 17 00:00:00 2001
From: Hugh Willson <hugh@octonary.com>
Date: Thu, 28 Sep 2017 08:55:16 -0400
Subject: [PATCH] Make sure Accounts login only sets 1 onReconnect callback

The changes added in
https://github.com/meteor/meteor/commit/d854a4b9ba97a230d9d57b4a23f358edd2a36702
fixed a long standing issue where the Accounts system was
overwriting other DDP `connection.onReconnect` callbacks,
that were potentially set by developers (and vice-versa -
developers could overwrite the `onReconnect` callback registered
by the Accounts system, which impacted logging back in after
reconnecting). Unfortunately these changes are also registering a
new duplicate `onReconnect` callback to be called, after every
login. These duplicate callbacks pile up and are all called when
reconnecting, which eventually breaks user logins.

The changes in this commit make sure that any previously set
Accounts login `onReconnect` callback is first removed, before
adding a new callback. This ensures the Accounts system is only
ever setting one `onReconnect` callback after logging in.

Fixes #9140.
---
 packages/accounts-base/accounts_client.js     |  7 +++
 .../accounts-base/accounts_reconnect_tests.js | 53 ++++++++++++++-----
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/packages/accounts-base/accounts_client.js b/packages/accounts-base/accounts_client.js
index 27163cc8d15..29a743dd527 100644
--- a/packages/accounts-base/accounts_client.js
+++ b/packages/accounts-base/accounts_client.js
@@ -281,6 +281,13 @@ Ap.callLoginMethod = function (options) {
       // already logged in they will still get logged in on reconnect.
       // See issue #4970.
     } else {
+      // First clear out any previously set Acccounts login onReconnect
+      // callback (to make sure we don't keep piling up duplicate callbacks,
+      // which would then all be triggered when reconnecting).
+      if (self._reconnectStopper) {
+        self._reconnectStopper.stop();
+      }
+
       self._reconnectStopper = DDP.onReconnect(function (conn) {
         if (conn != self.connection) {
           return;
diff --git a/packages/accounts-base/accounts_reconnect_tests.js b/packages/accounts-base/accounts_reconnect_tests.js
index 30bf469e61e..21d2ab92246 100644
--- a/packages/accounts-base/accounts_reconnect_tests.js
+++ b/packages/accounts-base/accounts_reconnect_tests.js
@@ -7,36 +7,32 @@ if (Meteor.isServer) {
 }
 
 if (Meteor.isClient) {
+  const loginAsUser1 = (onUser1LoggedIn) => {
+    Accounts.createUser({
+      username: `testuser1-${Random.id()}`,
+      password: `password1-${Random.id()}`
+    }, onUser1LoggedIn);
+  };
+
   Tinytest.addAsync('accounts - reconnect auto-login', function(test, done) {
     var onReconnectCalls = 0;
     var reconnectHandler = function () {
       onReconnectCalls++;
     };
     Meteor.connection.onReconnect = reconnectHandler;
-    
-    var username1 = 'testuser1-' + Random.id();
+
     var username2 = 'testuser2-' + Random.id();
-    var password1 = 'password1-' + Random.id();
     var password2 = 'password2-' + Random.id();
     var timeoutHandle;
     var onLoginStopper;
 
-    loginAsUser1();
-
-    function loginAsUser1() {
-      Accounts.createUser({
-        username: username1,
-        password: password1
-      }, onUser1LoggedIn);
-    }
-
-    function onUser1LoggedIn(err) {
+    loginAsUser1((err) => {
       test.isUndefined(err, 'Unexpected error logging in as user1');
       Accounts.createUser({
         username: username2,
         password: password2
       }, onUser2LoggedIn);
-    }
+    });
 
     function onUser2LoggedIn(err) {
       test.isUndefined(err, 'Unexpected error logging in as user2');
@@ -81,4 +77,33 @@ if (Meteor.isClient) {
       done();
     }
   });
+
+  // Make sure that when a logged in user is disconnected then reconnected,
+  // they still only have one Accounts login onReconnect callback set.
+  // Addresses: https://github.com/meteor/meteor/issues/9140
+  Tinytest.addAsync(
+    'accounts - verify single onReconnect callback',
+    function (test, done) {
+      loginAsUser1((err) => {
+        test.isUndefined(err, 'Unexpected error logging in as user1');
+        test.equal(
+          Object.keys(DDP._reconnectHook.callbacks).length,
+          1,
+          'Only one onReconnect callback should be registered'
+        );
+        Meteor.disconnect();
+        test.isFalse(Meteor.status().connected);
+        Meteor.reconnect();
+        setTimeout(() => {
+          test.isTrue(Meteor.status().connected);
+          test.equal(
+            Object.keys(DDP._reconnectHook.callbacks).length,
+            1,
+            'Only one onReconnect callback should be registered'
+          );
+          done();
+        }, 1000);
+      });
+    }
+  );
 }