From 2f1b683ec92ecb9802bc6921c8a47fa7140a54df Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 3 Jan 2017 00:00:01 +0100
Subject: [PATCH] Avoid setting profile name to not found (#8357)

* avoid setting profile name to not found

* only set profile name when empty

* profile.name is required

* set profile name before validation

* fix and add tests
---
 test/api/v3/integration/user/PUT-user.test.js | 26 +++++++++++++++++++
 .../user/auth/POST-register_local.test.js     |  1 +
 .../user/auth/POST-user_auth_social.test.js   |  6 +++--
 website/server/models/user/hooks.js           | 14 +++++++---
 website/server/models/user/schema.js          |  6 ++++-
 5 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/test/api/v3/integration/user/PUT-user.test.js b/test/api/v3/integration/user/PUT-user.test.js
index 7ef650dce4..ecd932a7dc 100644
--- a/test/api/v3/integration/user/PUT-user.test.js
+++ b/test/api/v3/integration/user/PUT-user.test.js
@@ -26,6 +26,32 @@ describe('PUT /user', () => {
       expect(user.preferences.costume).to.eql(true);
       expect(user.stats.hp).to.eql(14);
     });
+
+    it('profile.name cannot be an empty string or null', async () => {
+      await expect(user.put('/user', {
+        'profile.name': ' ', // string should be trimmed
+      })).to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: 'User validation failed',
+      });
+
+      await expect(user.put('/user', {
+        'profile.name': '',
+      })).to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: 'User validation failed',
+      });
+
+      await expect(user.put('/user', {
+        'profile.name': null,
+      })).to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: 'User validation failed',
+      });
+    });
   });
 
   context('Top Level Protected Operations', () => {
diff --git a/test/api/v3/integration/user/auth/POST-register_local.test.js b/test/api/v3/integration/user/auth/POST-register_local.test.js
index e41cf27a69..3960a07477 100644
--- a/test/api/v3/integration/user/auth/POST-register_local.test.js
+++ b/test/api/v3/integration/user/auth/POST-register_local.test.js
@@ -32,6 +32,7 @@ describe('POST /user/auth/local/register', () => {
       expect(user._id).to.exist;
       expect(user.apiToken).to.exist;
       expect(user.auth.local.username).to.eql(username);
+      expect(user.profile.name).to.eql(username);
     });
 
     it('provides default tags and tasks', async () => {
diff --git a/test/api/v3/integration/user/auth/POST-user_auth_social.test.js b/test/api/v3/integration/user/auth/POST-user_auth_social.test.js
index a128034fb6..6ca68e1339 100644
--- a/test/api/v3/integration/user/auth/POST-user_auth_social.test.js
+++ b/test/api/v3/integration/user/auth/POST-user_auth_social.test.js
@@ -33,7 +33,7 @@ describe('POST /user/auth/social', () => {
 
   describe('facebook', () => {
     before(async () => {
-      let expectedResult = {id: facebookId};
+      let expectedResult = {id: facebookId, displayName: 'a facebook user'};
       sandbox.stub(passport._strategies.facebook, 'userProfile').yields(null, expectedResult);
       network = 'facebook';
     });
@@ -47,6 +47,7 @@ describe('POST /user/auth/social', () => {
       expect(response.apiToken).to.exist;
       expect(response.id).to.exist;
       expect(response.newUser).to.be.true;
+      await expect(getProperty('users', response.id, 'profile.name')).to.eventually.equal('a facebook user');
     });
 
     it('logs an existing user in', async () => {
@@ -88,7 +89,7 @@ describe('POST /user/auth/social', () => {
 
   describe('google', () => {
     before(async () => {
-      let expectedResult = {id: googleId};
+      let expectedResult = {id: googleId, displayName: 'a google user'};
       sandbox.stub(passport._strategies.google, 'userProfile').yields(null, expectedResult);
       network = 'google';
     });
@@ -102,6 +103,7 @@ describe('POST /user/auth/social', () => {
       expect(response.apiToken).to.exist;
       expect(response.id).to.exist;
       expect(response.newUser).to.be.true;
+      await expect(getProperty('users', response.id, 'profile.name')).to.eventually.equal('a google user');
     });
 
     it('logs an existing user in', async () => {
diff --git a/website/server/models/user/hooks.js b/website/server/models/user/hooks.js
index 9dbd664720..066618511d 100644
--- a/website/server/models/user/hooks.js
+++ b/website/server/models/user/hooks.js
@@ -131,6 +131,16 @@ function _setProfileName (user) {
   return localUsername || _getFacebookName(user.auth.facebook) || googleUsername || anonymous;
 }
 
+schema.pre('validate', function preValidateUser (next) {
+  // Populate new user with profile name, not running in pre('save') because the field
+  // is required and validation fails if it doesn't exists like for new users
+  if (this.isNew && !this.profile.name) {
+    this.profile.name = _setProfileName(this);
+  }
+
+  next();
+});
+
 schema.pre('save', true, function preSaveUser (next, done) {
   next();
 
@@ -138,10 +148,6 @@ schema.pre('save', true, function preSaveUser (next, done) {
     this.preferences.dayStart = 0;
   }
 
-  if (!this.profile.name) {
-    this.profile.name = _setProfileName(this);
-  }
-
   // Determines if Beast Master should be awarded
   let beastMasterProgress = shared.count.beastMasterProgress(this.items.pets);
 
diff --git a/website/server/models/user/schema.js b/website/server/models/user/schema.js
index a9a96eebc8..36c038a120 100644
--- a/website/server/models/user/schema.js
+++ b/website/server/models/user/schema.js
@@ -472,7 +472,11 @@ let schema = new Schema({
   profile: {
     blurb: String,
     imageUrl: String,
-    name: String,
+    name: {
+      type: String,
+      required: true,
+      trim: true,
+    },
   },
   stats: {
     hp: {type: Number, default: shared.maxHealth},