Skip to content

Commit

Permalink
fix(articles): Orphaned User reference throws server error
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mleanos committed Dec 22, 2015
1 parent 7e8d2ed commit 2bdde4e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 3 deletions.
3 changes: 2 additions & 1 deletion modules/articles/client/views/list-articles.client.view.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
3 changes: 2 additions & 1 deletion modules/articles/client/views/view-article.client.view.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion modules/articles/server/policies/articles.server.policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
87 changes: 87 additions & 0 deletions modules/articles/tests/server/article.server.routes.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 2bdde4e

Please sign in to comment.