try moving all the logic to the model

common/locales/en/api-v3.json

@@ -1,6 +1,8 @@
 {
   "missingAuthHeaders": "Missing authentication headers.",
   "missingUsernameEmail": "Missing username or email.",
+  "missingEmail": "Missing email.",
+  "missingUsername": "Missing username.",
   "missingPassword": "Missing password.",
   "invalidEmail": "Invalid email address.",
   "emailTaken": "Email already taken.",

website/src/controllers/api-v3/user.js

@@ -20,45 +20,11 @@ let api = {};
  * @apiParam {String} passwordConfirmation Password confirmation
  *
  * @apiSuccess {Object} user The user object
- *
- * @apiUse NotAuthorized
  */
 api.registerLocal = {
   method: 'POST',
   url: '/user/register/local',
   handler (req, res, next) {
-    req.checkBody({
-      username: {
-        notEmpty: true,
-        errorMessage: res.t('missingEmail'),
-      },
-      email: {
-        notEmpty: true,
-        isEmail: true,
-        errorMessage: res.t('invalidEmail'),
-      },
-      password: {
-        notEmpty: true,
-        errorMessage: res.t('missingPassword'),
-      },
-      passwordConfirmation: {
-        notEmpty: true,
-        equals: {
-          options: [req.body.password],
-        },
-        errorMessage: res.t('passwordConfirmationMatch'),
-      },
-    });
-
-    let validationErrors = req.validationErrors();
-
-    if (validationErrors) return next(validationErrors);
-
-    req.sanitizeBody('username').trim();
-    req.sanitizeBody('email').trim();
-    req.sanitizeBody('password').trim();
-    req.sanitizeBody('passwordConfirmation').trim();
-
     let email = req.body.email.toLowerCase();
     let username = req.body.username;
     // Get the lowercase version of username to check that we do not have duplicates
@@ -71,22 +37,22 @@ api.registerLocal = {
       {'auth.local.lowerCaseUsername': lowerCaseUsername},
     ]}, {'auth.local': 1})
     .exec()
-    .then((results) => {
-      if (results[0]) {
-        if (email === results[0].auth.local.email) return next(new NotAuthorized(res.t('emailTaken')));
+    .then((user) => {
+      if (user) {
+        if (email === user.auth.local.email) return next(new NotAuthorized(res.t('emailTaken')));
         // Check that the lowercase username isn't already used
-        if (lowerCaseUsername === results[0].auth.local.lowerCaseUsername) return next(new NotAuthorized(res.t('usernameTaken')));
+        if (lowerCaseUsername === user.auth.local.lowerCaseUsername) return next(new NotAuthorized(res.t('usernameTaken')));
       }
 
-      let salt = passwordUtils.makeSalt();
       let newUser = new User({
         auth: {
           local: {
             username,
             lowerCaseUsername, // Store the lowercase version of the username
             email, // Store email as lowercase
-            salt,
-            hashed_password: passwordUtils.encrypt(req.body.password, salt), // eslint-disable-line camelcase
+            salt: passwordUtils.makeSalt(),
+            password: req.body.password,
+            passwordConfirmation: req.body.passwordConfirmation,
           },
         },
         preferences: {
@@ -131,7 +97,6 @@ api.registerLocal = {
  * @apiSuccess {String} _id The user's unique identifier
  * @apiSuccess {String} apiToken The user's api token that must be used to authenticate requests.
  *
- * @apiUse NotAuthorized
  */
 api.loginLocal = {
   method: 'POST',

website/src/models/user.js

@@ -1,7 +1,9 @@
 // User schema and model
 import mongoose from 'mongoose';
 import shared from '../../../common';
+import passwordUtils from '../libs/api-v3/password';
 import _ from 'lodash';
+import validator from 'validator';
 import moment from 'moment';
 import TaskSchemas from './task';
 // import {model as Challenge} from './challenge';
@@ -26,12 +28,29 @@ export let schema = new Schema({
     blocked: Boolean,
     facebook: Schema.Types.Mixed, // TODO validate
     local: {
-      email: String,
-      hashed_password: String, // eslint-disable-line camelcase
-      salt: String,
-      username: String,
+      email: {
+        type: String,
+        trim: true,
+        lowercase: true,
+        validate: [validator.isEmail, shared.i18n.t('invalidEmail')], // TODO translate error messages here, use preferences.language?
+      },
+      username: {
+        type: String,
+        trim: true,
+      },
       // Store a lowercase version of username to check for duplicates
       lowerCaseUsername: String,
+      hashed_password: String, // eslint-disable-line camelcase
+      salt: String,
+      // password and passwordConfirmation are not stored in the database, used only for validation
+      password: {
+        type: String,
+        trim: true,
+      },
+      passwordConfirmation: {
+        type: String,
+        trim: true,
+      },
     },
     timestamps: {
       created: {type: Date, default: Date.now},
@@ -208,6 +227,7 @@ export let schema = new Schema({
     party: Schema.Types.Mixed, // TODO dictionary
   },
 
+  // TODO we're storing too many fields here, find a way to reduce them
   items: {
     gear: {
       owned: _.transform(shared.content.gear.flat, (m, v) => {
@@ -328,6 +348,7 @@ export let schema = new Schema({
     orderAscending: {type: String, default: 'ascending'},
     quest: {
       key: String,
+      // TODO why are we storing quest progress here too and not only on party object?
       progress: {
         up: {type: Number, default: 0},
         down: {type: Number, default: 0},
@@ -589,6 +610,37 @@ function _setProfileName (user) {
 }
 
 schema.pre('save', function postSaveUser (next) {
+  // Validate the auth path (doesn't work with schema.path('auth').validate)
+  if (!this.auth.facebook.id) {
+    if (!this.auth.local.email) {
+      this.invalidate('auth.local.email', shared.i18n.t('missingEmail'));
+      return next();
+    }
+
+    if (!this.auth.local.email) {
+      this.invalidate('auth.local.username', shared.i18n.t('missingUsername'));
+      return next();
+    }
+  }
+
+  // Validate password and password confirmation and create hashed version
+  if (this.isModified('auth.local.password') || this.isNew() && !this.auth.facebook.id) {
+    if (!this.auth.local.password) {
+      this.invalidate('auth.local.password', shared.i18n.t('missingPassword'));
+      return next();
+    }
+
+    if (this.auth.local.password !== this.auth.local.passwordConfirmation) {
+      this.invalidate('auth.local.passwordConfirmation', shared.i18n.t('passwordConfirmationMatch'));
+      return next();
+    }
+
+    this.hashed_password = passwordUtils.encrypt(this.auth.local.password, this.auth.local.salt); // eslint-disable-line camelcase
+  }
+
+  // Do not store password and passwordConfirmation
+  this.auth.local.password = this.local.auth.passwordConfirmation = undefined;
+
   // Populate new users with default content
   if (this.isNew) {
     _populateDefaultsForNewUser(this);