From 2bdde4e9e2329a2526cf9519e2e682b014e9ae17 Mon Sep 17 00:00:00 2001
From: mleanos <michael.leanos@gmail.com>
Date: Wed, 16 Dec 2015 15:20:22 -0800
Subject: [PATCH] fix(articles): Orphaned User reference throws server error

Adds an additional check for the existence of a populated user
reference, when determining if the current user has immediate access to
the requested article.

Without this fix, the server will throw an error if the requested
article doesn't have a populated user field.

Modified the article & articles list view's to check if the article has
a populated user. If not, then it will display "Deleted User" in place
of the missing user reference.

Added a server-side test that ensures we can get a single article if
the article.user field is referencing a deleted user.

Fixes #1082
---
 .../views/list-articles.client.view.html      |  3 +-
 .../views/view-article.client.view.html       |  3 +-
 .../server/policies/articles.server.policy.js |  2 +-
 .../server/article.server.routes.tests.js     | 87 +++++++++++++++++++
 4 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/modules/articles/client/views/list-articles.client.view.html b/modules/articles/client/views/list-articles.client.view.html
index 488a4d59e6..cd6ab94520 100644
--- a/modules/articles/client/views/list-articles.client.view.html
+++ b/modules/articles/client/views/list-articles.client.view.html
@@ -8,7 +8,8 @@ <h1>Articles</h1>
         Posted on
         <span ng-bind="article.created | date:'mediumDate'"></span>
         by
-        <span ng-bind="article.user.displayName"></span>
+        <span ng-if="article.user" ng-bind="article.user.displayName"></span>
+        <span ng-if="!article.user">Deleted User</span>
       </small>
       <h4 class="list-group-item-heading" ng-bind="article.title"></h4>
       <p class="list-group-item-text" ng-bind="article.content"></p>
diff --git a/modules/articles/client/views/view-article.client.view.html b/modules/articles/client/views/view-article.client.view.html
index 2ecbb2f484..f70d6307f3 100644
--- a/modules/articles/client/views/view-article.client.view.html
+++ b/modules/articles/client/views/view-article.client.view.html
@@ -15,7 +15,8 @@ <h1 ng-bind="article.title"></h1>
       Posted on
       <span ng-bind="article.created | date:'mediumDate'"></span>
       by
-      <span ng-bind="article.user.displayName"></span>
+      <span ng-if="article.user" ng-bind="article.user.displayName"></span>
+      <span ng-if="!article.user">Deleted User</span>
     </em>
   </small>
   <p class="lead" ng-bind="article.content"></p>
diff --git a/modules/articles/server/policies/articles.server.policy.js b/modules/articles/server/policies/articles.server.policy.js
index 536f8bccb1..279277eeba 100644
--- a/modules/articles/server/policies/articles.server.policy.js
+++ b/modules/articles/server/policies/articles.server.policy.js
@@ -49,7 +49,7 @@ exports.isAllowed = function (req, res, next) {
   var roles = (req.user) ? req.user.roles : ['guest'];
 
   // If an article is being processed and the current user created it then allow any manipulation
-  if (req.article && req.user && req.article.user.id === req.user.id) {
+  if (req.article && req.user && req.article.user && req.article.user.id === req.user.id) {
     return next();
   }
 
diff --git a/modules/articles/tests/server/article.server.routes.tests.js b/modules/articles/tests/server/article.server.routes.tests.js
index 572d626f74..05208b2f06 100644
--- a/modules/articles/tests/server/article.server.routes.tests.js
+++ b/modules/articles/tests/server/article.server.routes.tests.js
@@ -313,6 +313,93 @@ describe('Article CRUD tests', function () {
     });
   });
 
+  it('should be able to get a single article that has an orphaned user reference', function (done) {
+    // Create orphan user creds
+    var _creds = {
+      username: 'orphan',
+      password: 'M3@n.jsI$Aw3$0m3'
+    };
+
+    // Create orphan user
+    var _orphan = new User({
+      firstName: 'Full',
+      lastName: 'Name',
+      displayName: 'Full Name',
+      email: 'orphan@test.com',
+      username: _creds.username,
+      password: _creds.password,
+      provider: 'local'
+    });
+
+    _orphan.save(function (err, orphan) {
+      // Handle save error
+      if (err) {
+        return done(err);
+      }
+
+      agent.post('/api/auth/signin')
+        .send(_creds)
+        .expect(200)
+        .end(function (signinErr, signinRes) {
+          // Handle signin error
+          if (signinErr) {
+            return done(signinErr);
+          }
+
+          // Get the userId
+          var orphanId = orphan._id;
+
+          // Save a new article
+          agent.post('/api/articles')
+            .send(article)
+            .expect(200)
+            .end(function (articleSaveErr, articleSaveRes) {
+              // Handle article save error
+              if (articleSaveErr) {
+                return done(articleSaveErr);
+              }
+
+              // Set assertions on new article
+              (articleSaveRes.body.title).should.equal(article.title);
+              should.exist(articleSaveRes.body.user);
+              should.equal(articleSaveRes.body.user._id, orphanId);
+
+              // force the article to have an orphaned user reference
+              orphan.remove(function () {
+                // now signin with valid user
+                agent.post('/api/auth/signin')
+                  .send(credentials)
+                  .expect(200)
+                  .end(function (err, res) {
+                    // Handle signin error
+                    if (err) {
+                      return done(err);
+                    }
+
+                    // Get the article
+                    agent.get('/api/articles/' + articleSaveRes.body._id)
+                      .expect(200)
+                      .end(function (articleInfoErr, articleInfoRes) {
+                        // Handle article error
+                        if (articleInfoErr) {
+                          return done(articleInfoErr);
+                        }
+
+                        // Set assertions
+                        (articleInfoRes.body._id).should.equal(articleSaveRes.body._id);
+                        (articleInfoRes.body.title).should.equal(article.title);
+                        should.equal(articleInfoRes.body.user, undefined);
+
+                        // Call the assertion callback
+                        done();
+                      });
+                  });
+              });
+            });
+        });
+    });
+  });
+
   afterEach(function (done) {
     User.remove().exec(function () {
       Article.remove().exec(done);