Adjust beforeExternalLogin hook & add tests

Adjust the beforeExternalLogin hook to be like onExternalLogin hook and add test.

packages/accounts-base/accounts_server.js

@@ -58,7 +58,6 @@ export class AccountsServer extends AccountsCommon {
     setExpireTokensInterval(this);
 
     this._validateLoginHook = new Hook({ bindEnvironment: false });
-    this._beforeExternalLoginHook = new Hook({ bindEnvironment: false });
     this._validateNewUserHooks = [
       defaultValidateNewUserHook.bind(this)
     ];
@@ -72,9 +71,9 @@ export class AccountsServer extends AccountsCommon {
       resetPassword: token => Meteor.absoluteUrl(`#/reset-password/${token}`),
       verifyEmail: token => Meteor.absoluteUrl(`#/verify-email/${token}`),
       enrollAccount: token => Meteor.absoluteUrl(`#/enroll-account/${token}`),
-    }
+    };
 
-    this.addDefaultRateLimit()
+    this.addDefaultRateLimit();
   }
 
   ///
@@ -123,8 +122,12 @@ export class AccountsServer extends AccountsCommon {
    * @locus Server
    * @param {Function} func Called whenever login/user creation from external service is attempted. Login or user creation based on this login can be aborted by by passing a falsy value or throwing an exception.
    */
-  beforeExternalLoginHook(func) {
-    this._beforeExternalLoginHook.register(func);
+  beforeExternalLogin(func) {
+    if (this._beforeExternalLoginHook) {
+      throw new Error("Can only call beforeExternalLogin once");
+    }
+
+    this._beforeExternalLoginHook = func;
   }
 
   ///
@@ -1222,11 +1225,9 @@ export class AccountsServer extends AccountsCommon {
     let user = this.users.findOne(selector, {fields: this._options.defaultFieldSelector});
 
     // Before continuing, run user hook to see if we should continue
-    this._beforeExternalLoginHook.forEach(hook => {
-      if (!hook(serviceName, serviceData, user)) {
-        throw new Meteor.Error(403, "Login forbidden");
-      }
-    });
+    if (this._beforeExternalLoginHook && !this._beforeExternalLoginHook(serviceName, serviceData, user)) {
+      throw new Meteor.Error(403, "Login forbidden");
+    }
 
     // When creating a new user we pass through all options. When updating an
     // existing user, by default we only process/pass through the serviceData

packages/accounts-base/accounts_tests.js

@@ -623,3 +623,44 @@ Tinytest.add(
     Accounts._options = accountsOptions;
   }
 );
+
+Tinytest.add(
+    'accounts - verify beforeExternalLogin hook can stop user login',
+    test => {
+        // Verify user data is saved properly when not using the
+        // beforeExternalLogin hook.
+        let facebookId = Random.id();
+        const uid1 = Accounts.updateOrCreateUserFromExternalService(
+            'facebook',
+            { id: facebookId },
+            { profile: { foo: 1 } },
+        ).userId;
+        const ignoreFieldName = "bigArray";
+        const c = Meteor.users.update(uid1, {$set: {[ignoreFieldName]: [1]}});
+        let users =
+            Meteor.users.find({ 'services.facebook.id': facebookId }).fetch();
+        test.length(users, 1);
+        test.equal(users[0].profile.foo, 1);
+        test.isNotUndefined(users[0][ignoreFieldName], 'ignoreField - before limit fields');
+
+        // Verify that when beforeExternalLogin returns false
+        // that an error throws and user is not saved
+        Accounts.beforeExternalLogin((serviceName, serviceData, user) => {
+            // Check that we get the correct data
+            test.equal(serviceName, 'facebook');
+            test.equal(serviceData, { id: facebookId });
+            test.equal(user._id, uid1);
+            return false
+        });
+
+        test.throws(() => Accounts.updateOrCreateUserFromExternalService(
+            'facebook',
+            { id: facebookId },
+            { profile: { foo: 1 } },
+        ));
+
+        // Cleanup
+        Meteor.users.remove(uid1);
+        Accounts._beforeExternalLoginHook = null;
+    }
+);