diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 9a4b112466..10d77fa56f 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -115,21 +115,13 @@ }; // XXX see comment on Accounts.createUser in passwords_server about adding a - // third "server options" argument. - var defaultCreateUserHook = function (options, extra, user) { - // This hook gets 'extra' directly from the createUser method, so make sure - // we don't allow users to set any fields at creation time that they won't - // later be able to set according to the default Meteor.users.allow. Set - // your own onCreateUser if you want users to be able to specify other - // fields at creation time. - if (_.any(extra, function(value, key) {return key != 'profile';})) { - console.log(JSON.stringify(extra)); - throw new Meteor.Error(400, "Disallowed fields in extra"); - } - - return _.extend(user, extra); + // second "server options" argument. + var defaultCreateUserHook = function (options, user) { + if (options.profile) + user.profile = options.profile; + return user; }; - Accounts.insertUserDoc = function (options, extra, user) { + Accounts.insertUserDoc = function (options, user) { // add created at timestamp (and protect passed in user object from // modification) user = _.extend({createdAt: +(new Date)}, user); @@ -137,15 +129,15 @@ var fullUser; if (onCreateUserHook) { - fullUser = onCreateUserHook(options, extra, user); + fullUser = onCreateUserHook(options, user); // This is *not* part of the API. We need this because we can't isolate // the global server environment between tests, meaning we can't test // both having a create user hook set and not having one set. if (fullUser === 'TEST DEFAULT HOOK') - fullUser = defaultCreateUserHook(options, extra, user); + fullUser = defaultCreateUserHook(options, user); } else { - fullUser = defaultCreateUserHook(options, extra, user); + fullUser = defaultCreateUserHook(options, user); } _.each(validateNewUserHooks, function (hook) { @@ -199,13 +191,13 @@ // @param serviceData {Object} Data to store in the user's record // under services[serviceName]. Must include an "id" field // which is a unique identifier for the user in the service. - // @param extra {Object, optional} Any additional fields to place on the user - // object + // @param options {Object, optional} Other options to pass to insertUserDoc + // (eg, profile) // @returns {Object} Object with token and id keys, like the result // of the "login" method. Accounts.updateOrCreateUserFromExternalService = function( - serviceName, serviceData, extra) { - extra = extra || {}; + serviceName, serviceData, options) { + options = _.clone(options || {}); if (serviceName === "password" || serviceName === "resume") throw new Error( @@ -221,26 +213,28 @@ var user = Meteor.users.findOne(selector); if (user) { - // don't overwrite existing fields - // XXX subobjects (aka 'profile', 'services')? - var newKeys = _.difference(_.keys(extra), _.keys(user)); - var newAttrs = _.pick(extra, newKeys); + // We *don't* process options (eg, profile) for update, but we do replace + // the serviceData (eg, so that we keep an unexpired access token and + // don't cache old email addresses in serviceData.email). + // XXX provide an onUpdateUser hook which would let apps update + // the profile too var stampedToken = Accounts._generateStampedLoginToken(); - var result = {token: stampedToken.token}; + var setAttrs = {}; + setAttrs["services." + serviceName] = serviceData; + // XXX Maybe we should re-use the selector above and notice if the update + // touches nothing? Meteor.users.update( user._id, - {$set: newAttrs, $push: {'services.resume.loginTokens': stampedToken}}); - result.id = user._id; - return result; + {$set: setAttrs, + $push: {'services.resume.loginTokens': stampedToken}}); + return {token: stampedToken.token, id: user._id}; } else { - // Create a new user. - var servicesClause = {}; - servicesClause[serviceName] = serviceData; - var insertOptions = {services: servicesClause, generateLoginToken: true}; - // Build a user doc; clone to make sure sure mutating - // insertOptions.services doesn't affect user.services or vice versa. - user = {services: JSON.parse(JSON.stringify(servicesClause))}; - return Accounts.insertUserDoc(insertOptions, extra, user); + // Create a new user with the service data. Pass other options through to + // insertUserDoc. + user = {services: {}}; + user.services[serviceName] = serviceData; + options.generateLoginToken = true; + return Accounts.insertUserDoc(options, user); } }; diff --git a/packages/accounts-base/accounts_tests.js b/packages/accounts-base/accounts_tests.js index d350d811d6..b771e17de1 100644 --- a/packages/accounts-base/accounts_tests.js +++ b/packages/accounts-base/accounts_tests.js @@ -6,20 +6,24 @@ Tinytest.add('accounts - updateOrCreateUserFromExternalService', function (test) // create an account with facebook var uid1 = Accounts.updateOrCreateUserFromExternalService( - 'facebook', {id: facebookId}, {profile: {foo: 1}}).id; - test.equal(Meteor.users.find({"services.facebook.id": facebookId}).count(), 1); - test.equal(Meteor.users.findOne({"services.facebook.id": facebookId}).profile.foo, 1); + 'facebook', {id: facebookId, monkey: 42}, {profile: {foo: 1}}).id; + var users = Meteor.users.find({"services.facebook.id": facebookId}).fetch(); + test.length(users, 1); + test.equal(users[0].profile.foo, 1); + test.equal(users[0].services.facebook.monkey, 42); - // create again with the same id, see that we get the same user. profile - // doesn't get overwritten in this implementation (though we should do - // something better with merging later). + // create again with the same id, see that we get the same user. + // it should update services.facebook but not profile. var uid2 = Accounts.updateOrCreateUserFromExternalService( - 'facebook', {id: facebookId}, {profile: {foo: 1000, bar: 2}}).id; + 'facebook', {id: facebookId, llama: 50}, + {profile: {foo: 1000, bar: 2}}).id; test.equal(uid1, uid2); - test.equal(Meteor.users.find({"services.facebook.id": facebookId}).count(), 1); - test.equal(Meteor.users.findOne(uid1).profile.foo, 1); - test.equal(Meteor.users.findOne(uid1).profile.bar, undefined); - + users = Meteor.users.find({"services.facebook.id": facebookId}).fetch(); + test.length(users, 1); + test.equal(users[0].profile.foo, 1); + test.equal(users[0].profile.bar, undefined); + test.equal(users[0].services.facebook.llama, 50); + test.equal(users[0].services.facebook.monkey, undefined); // cleanup Meteor.users.remove(uid1); @@ -48,7 +52,6 @@ Tinytest.add('accounts - insertUserDoc username', function (test) { // user does not already exist. create a user object with fields set. var result = Accounts.insertUserDoc( - userIn, {profile: {name: 'Foo Bar'}}, userIn ); @@ -61,7 +64,6 @@ Tinytest.add('accounts - insertUserDoc username', function (test) { // run the hook again. now the user exists, so it throws an error. test.throws(function () { Accounts.insertUserDoc( - userIn, {profile: {name: 'Foo Bar'}}, userIn ); @@ -83,7 +85,6 @@ Tinytest.add('accounts - insertUserDoc email', function (test) { // user does not already exist. create a user object with fields set. var result = Accounts.insertUserDoc( - userIn, {profile: {name: 'Foo Bar'}}, userIn ); @@ -97,7 +98,6 @@ Tinytest.add('accounts - insertUserDoc email', function (test) { // run the hook again. now the user exists, so it throws an error. test.throws(function () { Accounts.insertUserDoc( - userIn, {profile: {name: 'Foo Bar'}}, userIn ); @@ -106,20 +106,20 @@ Tinytest.add('accounts - insertUserDoc email', function (test) { // now with only one of them. test.throws(function () { Accounts.insertUserDoc( - {}, {}, {emails: [{address: email1}]} + {}, {emails: [{address: email1}]} ); }); test.throws(function () { Accounts.insertUserDoc( - {}, {}, {emails: [{address: email2}]} + {}, {emails: [{address: email2}]} ); }); // a third email works. var result3 = Accounts.insertUserDoc( - {}, {}, {emails: [{address: email3}]} + {}, {emails: [{address: email3}]} ); var user3 = Meteor.users.findOne(result3.id); test.equal(typeof user3.createdAt, 'number'); diff --git a/packages/accounts-facebook/facebook_server.js b/packages/accounts-facebook/facebook_server.js index 2184bb1f36..2a003f9003 100644 --- a/packages/accounts-facebook/facebook_server.js +++ b/packages/accounts-facebook/facebook_server.js @@ -11,7 +11,7 @@ accessToken: accessToken, email: identity.email }, - extra: {profile: {name: identity.name}} + options: {profile: {name: identity.name}} }; }); diff --git a/packages/accounts-github/github_server.js b/packages/accounts-github/github_server.js index ae695e3230..f9dc9147fb 100644 --- a/packages/accounts-github/github_server.js +++ b/packages/accounts-github/github_server.js @@ -11,7 +11,7 @@ email: identity.email, username: identity.login }, - extra: {profile: {name: identity.name}} + options: {profile: {name: identity.name}} }; }); diff --git a/packages/accounts-google/google_server.js b/packages/accounts-google/google_server.js index d0e8726c71..167e5f1b75 100644 --- a/packages/accounts-google/google_server.js +++ b/packages/accounts-google/google_server.js @@ -11,7 +11,7 @@ accessToken: accessToken, email: identity.email }, - extra: {profile: {name: identity.name}} + options: {profile: {name: identity.name}} }; }); diff --git a/packages/accounts-oauth-helper/oauth_server.js b/packages/accounts-oauth-helper/oauth_server.js index 5e49755a54..c94e540a42 100644 --- a/packages/accounts-oauth-helper/oauth_server.js +++ b/packages/accounts-oauth-helper/oauth_server.js @@ -14,7 +14,7 @@ // - (For OAuth1 only) oauthBinding {OAuth1Binding} bound to the appropriate provider // - (For OAuth2 only) query {Object} parameters passed in query string // - return value is: - // - {serviceData, (optional extra)} where serviceData should end + // - {serviceData:, (optional options:)} where serviceData should end // up in the user's services[name] field // - `null` if the user declined to give permissions Accounts.oauth.registerService = function (name, version, handleOauthRequest) { diff --git a/packages/accounts-oauth1-helper/oauth1_server.js b/packages/accounts-oauth1-helper/oauth1_server.js index 9733417d73..cd2e934871 100644 --- a/packages/accounts-oauth1-helper/oauth1_server.js +++ b/packages/accounts-oauth1-helper/oauth1_server.js @@ -55,7 +55,7 @@ // Get or create user doc and login token for reconnect. Accounts.oauth._loginResultForState[query.state] = Accounts.updateOrCreateUserFromExternalService( - service.serviceName, oauthResult.serviceData, oauthResult.extra); + service.serviceName, oauthResult.serviceData, oauthResult.options); } } diff --git a/packages/accounts-oauth1-helper/oauth1_tests.js b/packages/accounts-oauth1-helper/oauth1_tests.js index b6610e892a..78c0318088 100644 --- a/packages/accounts-oauth1-helper/oauth1_tests.js +++ b/packages/accounts-oauth1-helper/oauth1_tests.js @@ -92,7 +92,7 @@ Tinytest.add("oauth1 - error in user creation", function (test) { accessToken: twitterfailAccessToken, accessTokenSecret: twitterfailAccessTokenSecret }, - extra: { + options: { profile: {invalid: true} } }; diff --git a/packages/accounts-oauth2-helper/oauth2_server.js b/packages/accounts-oauth2-helper/oauth2_server.js index f0104418ed..696a59afe5 100644 --- a/packages/accounts-oauth2-helper/oauth2_server.js +++ b/packages/accounts-oauth2-helper/oauth2_server.js @@ -14,7 +14,7 @@ // Get or create user doc and login token for reconnect. Accounts.oauth._loginResultForState[query.state] = Accounts.updateOrCreateUserFromExternalService( - service.serviceName, oauthResult.serviceData, oauthResult.extra); + service.serviceName, oauthResult.serviceData, oauthResult.options); } // Either close the window, redirect, or render nothing diff --git a/packages/accounts-oauth2-helper/oauth2_tests.js b/packages/accounts-oauth2-helper/oauth2_tests.js index b10b183c9f..2331f691fb 100644 --- a/packages/accounts-oauth2-helper/oauth2_tests.js +++ b/packages/accounts-oauth2-helper/oauth2_tests.js @@ -55,7 +55,7 @@ Tinytest.add("oauth2 - error in user creation", function (test) { serviceData: { id: failbookId }, - extra: { + options: { profile: {invalid: true} } }; diff --git a/packages/accounts-password/email_tests.js b/packages/accounts-password/email_tests.js index 843e98c261..fc7da6b80e 100644 --- a/packages/accounts-password/email_tests.js +++ b/packages/accounts-password/email_tests.js @@ -17,9 +17,9 @@ function (test, expect) { email1 = Meteor.uuid() + "-intercept@example.com"; Accounts.createUser({email: email1, password: 'foobar'}, - expect(function (error) { - test.equal(error, undefined); - })); + expect(function (error) { + test.equal(error, undefined); + })); }, function (test, expect) { Accounts.forgotPassword({email: email1}, expect(function (error) { diff --git a/packages/accounts-password/passwords_client.js b/packages/accounts-password/passwords_client.js index 00e21e0498..2b877c36ea 100644 --- a/packages/accounts-password/passwords_client.js +++ b/packages/accounts-password/passwords_client.js @@ -1,12 +1,7 @@ (function () { - Accounts.createUser = function (options, extra, callback) { + Accounts.createUser = function (options, callback) { options = _.clone(options); // we'll be modifying options - if (typeof extra === "function") { - callback = extra; - extra = {}; - } - if (!options.password) throw new Error("Must set options.password"); var verifier = Meteor._srp.generateVerifier(options.password); @@ -14,7 +9,7 @@ delete options.password; options.srp = verifier; - Meteor.apply('createUser', [options, extra], {wait: true}, + Meteor.apply('createUser', [options], {wait: true}, function (error, result) { if (error || !result) { error = error || new Error("No result"); diff --git a/packages/accounts-password/passwords_server.js b/packages/accounts-password/passwords_server.js index 5ce4b5bf06..7fb474a773 100644 --- a/packages/accounts-password/passwords_server.js +++ b/packages/accounts-password/passwords_server.js @@ -355,8 +355,7 @@ // // returns an object with id: userId, and (if options.generateLoginToken is // set) token: loginToken. - var createUser = function (options, extra) { - extra = extra || {}; + var createUser = function (options) { var username = options.username; var email = options.email; if (!username && !email) @@ -379,19 +378,19 @@ if (email) user.emails = [{address: email, verified: false}]; - return Accounts.insertUserDoc(options, extra, user); + return Accounts.insertUserDoc(options, user); }; // method for create user. Requests come from the client. Meteor.methods({ - createUser: function (options, extra) { + createUser: function (options) { options = _.clone(options); options.generateLoginToken = true; if (Accounts._options.forbidClientAccountCreation) throw new Meteor.Error(403, "Signups forbidden"); // Create user. result contains id and token. - var result = createUser(options, extra); + var result = createUser(options); // safety belt. createUser is supposed to throw on error. send 500 error // instead of sending a verification email with empty userid. if (!result.id) @@ -420,20 +419,16 @@ // which is always empty when called from the createUser method? eg, "admin: // true", which we want to prevent the client from setting, but which a custom // method calling Accounts.createUser could set? - Accounts.createUser = function (options, extra, callback) { + Accounts.createUser = function (options, callback) { options = _.clone(options); options.generateLoginToken = false; - if (typeof extra === "function") { - callback = extra; - extra = {}; - } // XXX allow an optional callback? if (callback) { throw new Error("Meteor.createUser with callback not supported on the server yet."); } - var userId = createUser(options, extra).id; + var userId = createUser(options).id; return userId; }; diff --git a/packages/accounts-password/passwords_tests.js b/packages/accounts-password/passwords_tests.js index 100ac7024d..99177c2474 100644 --- a/packages/accounts-password/passwords_tests.js +++ b/packages/accounts-password/passwords_tests.js @@ -199,9 +199,9 @@ if (Meteor.isClient) (function () { logoutStep, // test Accounts.validateNewUser function(test, expect) { - Accounts.createUser({username: username3, password: password3}, - // should fail the new user validators - {profile: {invalid: true}}, + Accounts.createUser({username: username3, password: password3, + // should fail the new user validators + profile: {invalid: true}}, expect(function (error) { test.equal(error.error, 403); test.equal( @@ -211,10 +211,10 @@ if (Meteor.isClient) (function () { }, logoutStep, function(test, expect) { - Accounts.createUser({username: username3, password: password3}, + Accounts.createUser({username: username3, password: password3, // should fail the new user validator with a special // exception - {profile: {invalidAndThrowException: true}}, + profile: {invalidAndThrowException: true}}, expect(function (error) { test.equal( error.reason, @@ -224,8 +224,8 @@ if (Meteor.isClient) (function () { // test Accounts.onCreateUser function(test, expect) { Accounts.createUser( - {username: username3, password: password3}, - {testOnCreateUserHook: true}, + {username: username3, password: password3, + testOnCreateUserHook: true}, loggedInAs(username3, test, expect)); }, function(test, expect) { @@ -282,14 +282,14 @@ if (Meteor.isServer) (function () { var email = Meteor.uuid() + '@example.com'; test.throws(function () { // should fail the new user validators - Accounts.createUser({email: email}, {profile: {invalid: true}}); + Accounts.createUser({email: email, profile: {invalid: true}}); }); // disable sending emails var oldEmailSend = Email.send; Email.send = function() {}; - var userId = Accounts.createUser({email: email}, - {testOnCreateUserHook: true}); + var userId = Accounts.createUser({email: email, + testOnCreateUserHook: true}); Email.send = oldEmailSend; test.isTrue(userId); @@ -303,7 +303,7 @@ if (Meteor.isServer) (function () { function (test) { var username = Meteor.uuid(); - var userId = Accounts.createUser({username: username}, {}); + var userId = Accounts.createUser({username: username}); var user = Meteor.users.findOne(userId); // no services yet. diff --git a/packages/accounts-password/passwords_tests_setup.js b/packages/accounts-password/passwords_tests_setup.js index 10d22aaaa5..e1142de742 100644 --- a/packages/accounts-password/passwords_tests_setup.js +++ b/packages/accounts-password/passwords_tests_setup.js @@ -4,9 +4,9 @@ Accounts.validateNewUser(function (user) { return !(user.profile && user.profile.invalid); }); -Accounts.onCreateUser(function (options, extra, user) { - if (extra.testOnCreateUserHook) { - user.profile = (user.profile || {}); +Accounts.onCreateUser(function (options, user) { + if (options.testOnCreateUserHook) { + user.profile = user.profile || {}; user.profile.touchedByOnCreateUser = true; return user; } else { diff --git a/packages/accounts-twitter/twitter_server.js b/packages/accounts-twitter/twitter_server.js index 5414dae9d9..4008224b3d 100644 --- a/packages/accounts-twitter/twitter_server.js +++ b/packages/accounts-twitter/twitter_server.js @@ -10,7 +10,7 @@ accessToken: oauthBinding.accessToken, accessTokenSecret: oauthBinding.accessTokenSecret }, - extra: { + options: { profile: { name: identity.name } diff --git a/packages/accounts-weibo/weibo_server.js b/packages/accounts-weibo/weibo_server.js index fe5e4bb4e6..a061d5054d 100644 --- a/packages/accounts-weibo/weibo_server.js +++ b/packages/accounts-weibo/weibo_server.js @@ -11,7 +11,7 @@ accessToken: accessToken.access_token, screenName: identity.screen_name }, - extra: {profile: {name: identity.screen_name}} + options: {profile: {name: identity.screen_name}} }; });