Don't allow clients to set fields in the createUser method that they can't later

modify with the default Meteor.users update allow rule.

This does mean that it's difficult for trusted server code calling
Accounts.createUser, even with a custom onCreateUser hook, to set values on the
new user that can't also be set by arbitrary clients. For now, server code
needing to do this can just set it with a post-create update; later we might add
another parameter to onCreateUser.

packages/accounts-base/accounts_server.js

@@ -114,12 +114,18 @@
       onCreateUserHook = func;
   };
 
+  // XXX see comment on Accounts.createUser in passwords_server about adding a
+  // third "server options" argument.
   var defaultCreateUserHook = function (options, extra, user) {
-    if (!_.isEmpty(
-      _.intersection(
-        _.keys(extra),
-        ['services', 'username', 'email', 'emails'])))
+    // 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");
+    }
 
     if (Accounts._options.requireEmail &&
         (!user.emails || !user.emails.length))

packages/accounts-base/accounts_tests.js

@@ -6,17 +6,19 @@ Tinytest.add('accounts - updateOrCreateUserFromExternalService', function (test)
 
   // create an account with facebook
   var uid1 = Accounts.updateOrCreateUserFromExternalService(
-    'facebook', {id: facebookId}, {foo: 1}).id;
+    '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}).foo, 1);
+  test.equal(Meteor.users.findOne({"services.facebook.id": facebookId}).profile.foo, 1);
 
-  // create again with the same id, see that we get the same user
+  // 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).
   var uid2 = Accounts.updateOrCreateUserFromExternalService(
-    'facebook', {id: facebookId}, {foo: 1000, bar: 2}).id; // foo: 1000 shouldn't overwrite
+    'facebook', {id: facebookId}, {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).foo, 1);
-  test.equal(Meteor.users.findOne(uid1).bar, 2);
+  test.equal(Meteor.users.findOne(uid1).profile.foo, 1);
+  test.equal(Meteor.users.findOne(uid1).profile.bar, undefined);
 
   // cleanup
   Meteor.users.remove(uid1);
@@ -24,13 +26,13 @@ Tinytest.add('accounts - updateOrCreateUserFromExternalService', function (test)
 
   // users that have different service ids get different users
   uid1 = Accounts.updateOrCreateUserFromExternalService(
-    'weibo', {id: weiboId1}, {foo: 1}).id;
+    'weibo', {id: weiboId1}, {profile: {foo: 1}}).id;
   uid2 = Accounts.updateOrCreateUserFromExternalService(
-    'weibo', {id: weiboId2}, {bar: 2}).id;
+    'weibo', {id: weiboId2}, {profile: {bar: 2}}).id;
   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}).profile.foo, 1);
   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}).profile.bar, 2);
   test.equal(Meteor.users.findOne({"services.weibo.id": weiboId2}).emails, undefined);
 
   // cleanup

packages/accounts-oauth1-helper/oauth1_tests.js

@@ -93,7 +93,7 @@ Tinytest.add("oauth1 - error in user creation", function (test) {
         accessTokenSecret: twitterfailAccessTokenSecret
       },
       extra: {
-        invalid: true
+        profile: {invalid: true}
       }
     };
   });
@@ -101,7 +101,7 @@ Tinytest.add("oauth1 - error in user creation", function (test) {
   // a way to fail new users. duplicated from passwords_tests, but
   // shouldn't hurt.
   Accounts.validateNewUser(function (user) {
-    return !user.invalid;
+    return !(user.profile && user.profile.invalid);
   });
 
   // simulate logging in with failure

packages/accounts-oauth2-helper/oauth2_tests.js

@@ -56,7 +56,7 @@ Tinytest.add("oauth2 - error in user creation", function (test) {
         id: failbookId
       },
       extra: {
-        invalid: true
+        profile: {invalid: true}
       }
     };
   });
@@ -64,7 +64,7 @@ Tinytest.add("oauth2 - error in user creation", function (test) {
   // a way to fail new users. duplicated from passwords_tests, but
   // shouldn't hurt.
   Accounts.validateNewUser(function (user) {
-    return !user.invalid;
+    return !(user.profile && user.profile.invalid);
   });
 
   // simulate logging in with failure

packages/accounts-password/passwords_server.js

@@ -381,6 +381,11 @@
   // after creation.
   //
   // returns userId or throws an error if it can't create
+  //
+  // XXX add another argument ("server options") that gets sent to onCreateUser,
+  // 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) {
     options = _.clone(options);
     options.generateLoginToken = false;

packages/accounts-password/passwords_tests.js

@@ -200,18 +200,21 @@ if (Meteor.isClient) (function () {
     // test Accounts.validateNewUser
     function(test, expect) {
       Accounts.createUser({username: username3, password: password3},
-                        {invalid: true}, // should fail the new user validators
-                        expect(function (error) {
-                          test.equal(error.error, 403);
-                          test.equal(
-                            error.reason,
-                            "User validation failed");
-                        }));
+                          // should fail the new user validators
+                          {profile: {invalid: true}},
+                          expect(function (error) {
+                            test.equal(error.error, 403);
+                            test.equal(
+                              error.reason,
+                              "User validation failed");
+                          }));
     },
     logoutStep,
     function(test, expect) {
       Accounts.createUser({username: username3, password: password3},
-                        {invalidAndThrowException: true}, // should fail the new user validator with a special exception
+                           // should fail the new user validator with a special
+                           // exception
+                          {profile: {invalidAndThrowException: true}},
                         expect(function (error) {
                           test.equal(
                             error.reason,
@@ -271,8 +274,8 @@ if (Meteor.isServer) (function () {
     function (test) {
       var email = Meteor.uuid() + '@example.com';
       test.throws(function () {
-        Accounts.createUser({email: email},
-                            {invalid: true}); // should fail the new user validators
+        // should fail the new user validators
+        Accounts.createUser({email: email}, {profile: {invalid: true}});
         });
 
       // disable sending emails

packages/accounts-password/passwords_tests_setup.js

@@ -1,7 +1,7 @@
 Accounts.validateNewUser(function (user) {
-  if (user.invalidAndThrowException)
+  if (user.profile && user.profile.invalidAndThrowException)
     throw new Meteor.Error(403, "An exception thrown within Accounts.validateNewUser");
-  return !user.invalid;
+  return !(user.profile && user.profile.invalid);
 });
 
 Accounts.onCreateUser(function (options, extra, user) {