Remove email-based account merging from OAuth logins.

Users who log in via OAuth providers now do not have email addresses set in the top level 'emails' field. Instead, their emails are recorded inside the services field.

This means one user who logs in with facebook and google with the same email address will get two different accounts.

packages/accounts-base/accounts_server.js

@@ -122,6 +122,10 @@
         throw new Meteor.Error(403, "User validation failed");
     });
 
+    // XXX check for existing user with duplicate email or username.
+    // better here than the two places we call it (and immediately
+    // follow with an insert)
+
     return fullUser;
   };
 
@@ -137,85 +141,40 @@
 
   // Updates or creates a user after we authenticate with a 3rd party
   //
-  // NOTE: We trust any OAuth provider to properly validates email address
-  //
   // @param options {Object}
-  //   - email (optional)
   //   - services {Object} e.g. {facebook: {id: (facebook user id), ...}}
   // @param extra {Object, optional} Any additional fields to place on the user objet
   // @returns {String} userId
   Meteor.accounts.updateOrCreateUser = function(options, extra) {
     extra = extra || {};
 
-    var updateUserData = function() {
+    if (_.keys(options.services).length !== 1)
+      throw new Error("Must pass exactly one service to updateOrCreateUser");
+    var serviceName = _.keys(options.services)[0];
+
+    // Look for a user with the appropriate service user id.
+    var selector = {};
+    selector["services." + serviceName + ".id"] =
+      options.services[serviceName].id;
+    var user = Meteor.users.findOne(selector);
+
+    if (user) {
       // don't overwrite existing fields
+      // XXX subobjects (aka 'profile', 'services')?
       var newKeys = _.without(_.keys(extra), _.keys(user));
       var newAttrs = _.pick(extra, newKeys);
-      Meteor.users.update(user, {$set: newAttrs});
-    };
+      Meteor.users.update(user._id, {$set: newAttrs});
 
-    if (_.keys(options.services).length > 0) {
-      if (_.keys(options.services).length > 1) {
-        throw new Error("Can't pass more than one service to updateOrCreateUser");
-      }
-      var serviceName = _.keys(options.services)[0];
-    }
-
-    var email = options.email;
-    var userByEmail = email && Meteor.users.findOne({"emails.address": email});
-    var user;
-    if (userByEmail) {
-
-      // If we know about this email address that is our user.
-      // Update the information from this service.
-      user = userByEmail;
-      if (options.services && (!user.services || !user.services[serviceName])) {
-        var attrs = {};
-        attrs["services." + serviceName] = options.services[serviceName];
-
-        // XXX we will probably also need a hook for updating users,
-        // similar to Meteor.accounts.onCreateUser
-        Meteor.users.update(user, {$set: attrs});
-      }
-
-      updateUserData();
       return user._id;
-    } else if (options.services) {
-
-      // If not, look for a user with the appropriate service user id.
-      // Update the user's email.
-      var selector = {};
-      selector["services." + serviceName + ".id"] = options.services[serviceName].id;
-      var userByServiceUserId = Meteor.users.findOne(selector);
-      if (userByServiceUserId) {
-        user = userByServiceUserId;
-        if (email) {
-          // The user must have changed the email address associated
-          // with this service (since if we only reach this else
-          // clause if we didn't match the user by email to begin
-          // with). Store the new one in addition to the old one.
-
-          // XXX we will probably also need a hook for updating users,
-          // similar to Meteor.accounts.onCreateUser
-          Meteor.users.update(
-            {_id: user._id},
-            {$push: {emails: {address: email, validated: true}}});
-        }
-
-        updateUserData();
-        return user._id;
-      } else {
-
-        // Create a new user
-        var attrs = {};
-        attrs[serviceName] = options.services[serviceName];
-        var user = {
-          emails: (email ? [{address: email, validated: true}] : []),
-          services: attrs
-        };
-        user = Meteor.accounts.onCreateUserHook(options, extra, user);
-        return Meteor.users.insert(user);
-      }
+    } else {
+      // Create a new user
+      var attrs = {};
+      attrs[serviceName] = options.services[serviceName];
+      user = {
+        services: attrs
+      };
+      user = Meteor.accounts.onCreateUserHook(options, extra, user);
+      return Meteor.users.insert(user);
     }
   };
 

packages/accounts-base/accounts_tests.js

@@ -1,53 +1,41 @@
 Tinytest.add('accounts - updateOrCreateUser', function (test) {
-  var email = Meteor.uuid() + "@example.com";
-  var email2 = Meteor.uuid() + "@example.com";
   var facebookId = Meteor.uuid();
-  var googleId = Meteor.uuid();
   var weiboId1 = Meteor.uuid();
   var weiboId2 = Meteor.uuid();
 
-  // test that emails are matched correctly for users logging in
-  // through different services
-  Meteor.accounts.updateOrCreateUser({email: email, services: {facebook: {id: facebookId}}});
 
-  // twice just to make sure we don't accidentally duplicate email records
-  Meteor.accounts.updateOrCreateUser({email: email, services: {google: {id: googleId}}});
-  Meteor.accounts.updateOrCreateUser({email: email, services: {google: {id: googleId}}});
+  // create an account with facebook
+  var uid1 = Meteor.accounts.updateOrCreateUser(
+    {services: {facebook: {id: facebookId}}}, {foo: 1});
+  test.equal(Meteor.users.find({"services.facebook.id": facebookId}).count(), 1);
+  test.equal(Meteor.users.findOne({"services.facebook.id": facebookId}).foo, 1);
 
-  test.equal(
-    Meteor.users.findOne({"emails.address": email}).services.facebook.id, facebookId);
-  test.equal(
-    Meteor.users.findOne({"emails.address": email}).services.google.id, googleId);
-
-  // test that if the user changes their email on the login service
-  // we store the new one in addition to the old one
-  Meteor.accounts.updateOrCreateUser({email: email2, services: {facebook: {id: facebookId}}});
-  test.equal(
-    Meteor.users.findOne({"emails.address": email}).emails,
-    [{address: email, validated: true}, {address: email2, validated: true}]);
+  // create again with the same id, see that we get the same user
+  var uid2 = Meteor.accounts.updateOrCreateUser(
+    {services: {facebook: {id: facebookId}}}, {bar: 2});
+  test.equal(uid1, uid2);
+  test.equal(Meteor.users.find({"services.facebook.id": facebookId}).count(), 1);
+  test.equal(Meteor.users.findOne(uid1).foo, 1);
+  test.equal(Meteor.users.findOne(uid1).bar, 2);
 
   // cleanup
-  Meteor.users.remove({emails: {$in: [email, email2]}});
+  Meteor.users.remove(uid1);
 
-  // users with no email (such as on weibo) that have the same weibo
-  // id get the same user
-  Meteor.accounts.updateOrCreateUser({services: {weibo: {id: weiboId1}}}, {foo: 1});
-  Meteor.accounts.updateOrCreateUser({services: {weibo: {id: weiboId1}}}, {bar: 2});
-  test.equal(Meteor.users.find({"services.weibo.id": weiboId1}).count(), 1);
-  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).foo, 1);
-  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).bar, 2);
-  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).emails, []);
 
-  // cleanup
-  Meteor.users.remove({"services.weibo.id": weiboId1});
-
-  // users with no email (such as on weibo) that have different weibo
-  // ids get different users
-  Meteor.accounts.updateOrCreateUser({services: {weibo: {id: weiboId1}}}, {foo: 1});
-  Meteor.accounts.updateOrCreateUser({services: {weibo: {id: weiboId2}}}, {bar: 2});
+  // users that have different service ids get different users
+  uid1 = Meteor.accounts.updateOrCreateUser(
+    {services: {weibo: {id: weiboId1}}}, {foo: 1});
+  uid2 = Meteor.accounts.updateOrCreateUser(
+    {services: {weibo: {id: weiboId2}}}, {bar: 2});
   test.equal(Meteor.users.find({"services.weibo.id": {$in: [weiboId1, weiboId2]}}).count(), 2);
   test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).foo, 1);
-  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).emails, []);
+  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId1}).emails, undefined);
   test.equal(Meteor.users.findOne({"services.weibo.id": weiboId2}).bar, 2);
-  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId2}).emails, []);
+  test.equal(Meteor.users.findOne({"services.weibo.id": weiboId2}).emails, undefined);
+
+  // cleanup
+  Meteor.users.remove(uid1);
+  Meteor.users.remove(uid2);
+
 });
+

packages/accounts-facebook/facebook_server.js

@@ -11,8 +11,11 @@
 
     return {
       options: {
-        email: identity.email,
-        services: {facebook: {id: identity.id, accessToken: accessToken}}
+        services: {facebook: {
+          id: identity.id,
+          accessToken: accessToken,
+          email: identity.email
+        }}
       },
       extra: {profile: {name: identity.name}}
     };

packages/accounts-google/google_server.js

@@ -11,8 +11,11 @@
 
     return {
       options: {
-        email: identity.email,
-        services: {google: {id: identity.id, accessToken: accessToken}}
+        services: {google: {
+          id: identity.id,
+          accessToken: accessToken,
+          email: identity.email
+        }}
       },
       extra: {profile: {name: identity.name}}
     };

packages/accounts-oauth2-helper/oauth2_tests.js

@@ -1,6 +1,5 @@
 Tinytest.add("oauth2 - loginResultForState is stored", function (test) {
   var http = __meteor_bootstrap__.require('http');
-  var email = Meteor.uuid() + "@example.com";
   var foobookId = Meteor.uuid();
 
   // XXX XXX test isolation fail!  Avital: but actually -- why would
@@ -17,7 +16,6 @@ Tinytest.add("oauth2 - loginResultForState is stored", function (test) {
   Meteor.accounts.oauth.registerService("foobook", 2, function (query) {
     return {
       options: {
-        email: email,
         services: {foobook: {id: foobookId}}
       }
     };
@@ -30,10 +28,9 @@ Tinytest.add("oauth2 - loginResultForState is stored", function (test) {
   Meteor.accounts.oauth._middleware(req, new http.ServerResponse(req));
 
   // verify that a user is created
-  var user = Meteor.users.findOne({"emails.address": email});
+  var user = Meteor.users.findOne({"services.foobook.id": foobookId});
   test.notEqual(user, undefined);
   test.equal(user.services.foobook.id, foobookId);
-  test.equal(user.emails[0], {address: email, validated: true});
 
   // and that that user has a login token
   var token = Meteor.accounts._loginTokens.findOne({userId: user._id});
@@ -49,8 +46,8 @@ Tinytest.add("oauth2 - loginResultForState is stored", function (test) {
 
 Tinytest.add("oauth2 - error in user creation", function (test) {
   var http = __meteor_bootstrap__.require('http');
-  var email = Meteor.uuid() + "@example.com";
   var state = Meteor.uuid();
+  var failbookId = Meteor.uuid();
 
   Meteor.accounts.failbook = {};
   Meteor.accounts.failbook._requireConfigs = [];
@@ -60,8 +57,7 @@ Tinytest.add("oauth2 - error in user creation", function (test) {
   Meteor.accounts.oauth.registerService("failbook", 2, function (query) {
     return {
       options: {
-        email: email,
-        services: {foobook: {id: Meteor.uuid()}}
+        services: {failbook: {id: failbookId}}
       },
       extra: {
         invalid: true
@@ -83,7 +79,7 @@ Tinytest.add("oauth2 - error in user creation", function (test) {
   Meteor.accounts.oauth._middleware(req, new http.ServerResponse(req));
 
   // verify that a user is not created
-  var user = Meteor.users.findOne({"emails.address": email});
+  var user = Meteor.users.findOne({"services.failbook.id": failbookId});
   test.equal(user, undefined);
 
   // verify an error is stored in login state