From 4323443dbd0586e0ce05756a69748dfc0fc955b9 Mon Sep 17 00:00:00 2001
From: Tyler Renelle <tylerrenelle@gmail.com>
Date: Tue, 10 Dec 2013 08:55:04 -0700
Subject: [PATCH] [#1977] stricter PUT /user set-path validation, require
 they're setting on a leaf-path (except for tasks.* - we still need to handle
 that properly)

---
 migrations/20131204_classes.js |  2 ++
 package.json                   |  4 +--
 src/controllers/user.js        | 51 +++++++++++++++++-----------------
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/migrations/20131204_classes.js b/migrations/20131204_classes.js
index 8dddd0a826..4df17ad61a 100644
--- a/migrations/20131204_classes.js
+++ b/migrations/20131204_classes.js
@@ -9,6 +9,8 @@ db.users.find().forEach(function(user){
 
   user.stats.class = 'warrior';
 
+  // set default stats (inc mp)
+
   // grant backer/contrib gear, 300, rather than using js logic
 
   // customizations redo: https://trello.com/c/YKXmHNjY/306-customization-redo
diff --git a/package.json b/package.json
index 58a0529c23..31ddab7388 100644
--- a/package.json
+++ b/package.json
@@ -22,7 +22,6 @@
     "lodash": "~2.2.1",
     "async": "~0.2.9",
     "optimist": "~0.5.2",
-    "mongoose": "~3.6.20",
     "stylus": "~0.37.0",
     "grunt": "~0.4.1",
     "grunt-contrib-uglify": "~0.2.4",
@@ -44,7 +43,8 @@
     "paypal-ipn": "~1.0.1",
     "express-csv": "~0.6.0",
     "pretty-data": "git://github.com/vkiryukhin/pretty-data#master",
-    "js2xmlparser": "~0.1.2"
+    "js2xmlparser": "~0.1.2",
+    "mongoose": "~3.8.1"
   },
   "private": true,
   "subdomain": "habitrpg",
diff --git a/src/controllers/user.js b/src/controllers/user.js
index fcae5a6596..73f3d959fb 100644
--- a/src/controllers/user.js
+++ b/src/controllers/user.js
@@ -14,6 +14,7 @@ var sanitize = validator.sanitize;
 var User = require('./../models/user').model;
 var Group = require('./../models/group').model;
 var Challenge = require('./../models/challenge').model;
+var acceptablePUTPaths;
 var api = module.exports;
 
 // FIXME put this in a proper location
@@ -34,7 +35,6 @@ api.marketBuy = function(req, res, next){
   })
 }
 
-
 /*
   ------------------------------------------------------------------------
   Tasks
@@ -240,38 +240,39 @@ api.getUser = function(req, res, next) {
   return res.json(200, user);
 };
 
+
+/**
+ * This tells us for which paths users can call `PUT /user` (or batch-update equiv, which use `User.set()` on our client).
+ * The trick here is to only accept leaf paths, not root/intermediate paths (see http://goo.gl/OEzkAs)
+ * FIXME - one-by-one we want to widdle down this list, instead replacing each needed set path with API operations
+ *
+ * Note: custom is for 3rd party apps
+ */
+acceptablePUTPaths = _.reduce(require('./../models/user').schema.paths, function(m,v,leaf){
+  if (_.find('tasks achievements filters flags invitations items lastCron party preferences profile stats tags custom'.split(' '), function(root){
+    return leaf.indexOf(root) == 0;
+  });
+  if (found) m[leaf]=true;
+  return m;
+}, {})
+acceptablePUTPaths['tasks']=true; // and for now, let them fully set tasks.* (for advanced editing, TODO lock down)
+
 /**
  * Update user
- * FIXME add documentation here
+ * Send up PUT /user as `req.body={path1:val, path2:val, etc}`. Example:
+ * PUT /user {'stats.hp':50, 'tasks.TASK_ID.repeat.m':false}
+ * See acceptablePUTPaths for which user paths are supported
 */
 api.updateUser = function(req, res, next) {
-  var acceptableAttrs, errors, user;
-  user = res.locals.user;
-  errors = [];
-  if (_.isEmpty(req.body)) {
-    return res.json(200, user);
-  }
-  /*
-  # FIXME we need to do some crazy sanitiazation if they're using the old `PUT /user {data}` method.
-  # The new `PUT /user {'stats.hp':50}
+  var user = res.locals.user;
+  var errors = [];
+  if (_.isEmpty(req.body)) return res.json(200, user);
 
-  # FIXME - one-by-one we want to widdle down this list, instead replacing each needed set path with API operations
-  # There's a trick here. In order to prevent prevent clobering top-level paths, we add `.` to make sure they're
-  # sending bodies as {"set.this.path":value} instead of {set:{this:{path:value}}}. Permit lastCron since it's top-level
-  # Note: custom is for 3rd party apps
-  */
-
-  acceptableAttrs = 'tasks. achievements. filters. flags. invitations. items. lastCron party. preferences. profile. stats. tags custom.'.split(' ');
   _.each(req.body, function(v, k) {
-    var found = _.find(acceptableAttrs, function(attr) { return k.indexOf(attr) == 0; })
-    if (found) {
-//      if (_.isObject(v)) {
-//        errors.push("Value for " + k + " was an object. Be careful here, you could clobber stuff.");
-//      }
+    if (acceptablePUTPaths[k])
       helpers.dotSet(k, v, user);
-    } else {
+    else
       errors.push("path `" + k + "` was not saved, as it's a protected path. Make sure to send `PUT /api/v1/user` request bodies as `{'set.this.path':value}` instead of `{set:{this:{path:value}}}`");
-    }
     return true;
   });
   user.save(function(err) {