Skip to content
This repository was archived by the owner on Aug 30, 2021. It is now read-only.

Commit 2bdde4e

Browse files
committed
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
1 parent 7e8d2ed commit 2bdde4e

File tree

4 files changed

+92
-3
lines changed

4 files changed

+92
-3
lines changed

modules/articles/client/views/list-articles.client.view.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ <h1>Articles</h1>
88
Posted on
99
<span ng-bind="article.created | date:'mediumDate'"></span>
1010
by
11-
<span ng-bind="article.user.displayName"></span>
11+
<span ng-if="article.user" ng-bind="article.user.displayName"></span>
12+
<span ng-if="!article.user">Deleted User</span>
1213
</small>
1314
<h4 class="list-group-item-heading" ng-bind="article.title"></h4>
1415
<p class="list-group-item-text" ng-bind="article.content"></p>

modules/articles/client/views/view-article.client.view.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ <h1 ng-bind="article.title"></h1>
1515
Posted on
1616
<span ng-bind="article.created | date:'mediumDate'"></span>
1717
by
18-
<span ng-bind="article.user.displayName"></span>
18+
<span ng-if="article.user" ng-bind="article.user.displayName"></span>
19+
<span ng-if="!article.user">Deleted User</span>
1920
</em>
2021
</small>
2122
<p class="lead" ng-bind="article.content"></p>

modules/articles/server/policies/articles.server.policy.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ exports.isAllowed = function (req, res, next) {
4949
var roles = (req.user) ? req.user.roles : ['guest'];
5050

5151
// If an article is being processed and the current user created it then allow any manipulation
52-
if (req.article && req.user && req.article.user.id === req.user.id) {
52+
if (req.article && req.user && req.article.user && req.article.user.id === req.user.id) {
5353
return next();
5454
}
5555

modules/articles/tests/server/article.server.routes.tests.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,93 @@ describe('Article CRUD tests', function () {
313313
});
314314
});
315315

316+
it('should be able to get a single article that has an orphaned user reference', function (done) {
317+
// Create orphan user creds
318+
var _creds = {
319+
username: 'orphan',
320+
password: '[email protected]$Aw3$0m3'
321+
};
322+
323+
// Create orphan user
324+
var _orphan = new User({
325+
firstName: 'Full',
326+
lastName: 'Name',
327+
displayName: 'Full Name',
328+
329+
username: _creds.username,
330+
password: _creds.password,
331+
provider: 'local'
332+
});
333+
334+
_orphan.save(function (err, orphan) {
335+
// Handle save error
336+
if (err) {
337+
return done(err);
338+
}
339+
340+
agent.post('/api/auth/signin')
341+
.send(_creds)
342+
.expect(200)
343+
.end(function (signinErr, signinRes) {
344+
// Handle signin error
345+
if (signinErr) {
346+
return done(signinErr);
347+
}
348+
349+
// Get the userId
350+
var orphanId = orphan._id;
351+
352+
// Save a new article
353+
agent.post('/api/articles')
354+
.send(article)
355+
.expect(200)
356+
.end(function (articleSaveErr, articleSaveRes) {
357+
// Handle article save error
358+
if (articleSaveErr) {
359+
return done(articleSaveErr);
360+
}
361+
362+
// Set assertions on new article
363+
(articleSaveRes.body.title).should.equal(article.title);
364+
should.exist(articleSaveRes.body.user);
365+
should.equal(articleSaveRes.body.user._id, orphanId);
366+
367+
// force the article to have an orphaned user reference
368+
orphan.remove(function () {
369+
// now signin with valid user
370+
agent.post('/api/auth/signin')
371+
.send(credentials)
372+
.expect(200)
373+
.end(function (err, res) {
374+
// Handle signin error
375+
if (err) {
376+
return done(err);
377+
}
378+
379+
// Get the article
380+
agent.get('/api/articles/' + articleSaveRes.body._id)
381+
.expect(200)
382+
.end(function (articleInfoErr, articleInfoRes) {
383+
// Handle article error
384+
if (articleInfoErr) {
385+
return done(articleInfoErr);
386+
}
387+
388+
// Set assertions
389+
(articleInfoRes.body._id).should.equal(articleSaveRes.body._id);
390+
(articleInfoRes.body.title).should.equal(article.title);
391+
should.equal(articleInfoRes.body.user, undefined);
392+
393+
// Call the assertion callback
394+
done();
395+
});
396+
});
397+
});
398+
});
399+
});
400+
});
401+
});
402+
316403
afterEach(function (done) {
317404
User.remove().exec(function () {
318405
Article.remove().exec(done);

0 commit comments

Comments
 (0)