diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 835e8efb7e..8b4dc17346 100644 --- a/packages/accounts-base/accounts_server.js +++ b/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); } }; diff --git a/packages/accounts-base/accounts_tests.js b/packages/accounts-base/accounts_tests.js index 6cd8612672..13f939fe06 100644 --- a/packages/accounts-base/accounts_tests.js +++ b/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); + }); + diff --git a/packages/accounts-facebook/facebook_server.js b/packages/accounts-facebook/facebook_server.js index b5abdae4fd..b24d745e51 100644 --- a/packages/accounts-facebook/facebook_server.js +++ b/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}} }; diff --git a/packages/accounts-google/google_server.js b/packages/accounts-google/google_server.js index 973acc4f47..295010ab38 100644 --- a/packages/accounts-google/google_server.js +++ b/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}} }; diff --git a/packages/accounts-oauth2-helper/oauth2_tests.js b/packages/accounts-oauth2-helper/oauth2_tests.js index 81215a5fa7..8584926d69 100644 --- a/packages/accounts-oauth2-helper/oauth2_tests.js +++ b/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