Skip to content

Commit

Permalink
Removed bypassing option filtering in User model
Browse files Browse the repository at this point in the history
no issue

- the logic here bypasses filtering options!
- that is wrong, because if we filter out certain options e.g. include
- the tests from the previous commit fail because of this
- if we don't fix this logic, the tests won't pass, because as said, you can bypass certain logic e.g. remove roles from include
- this has worked before, because we passed the wrong options via the API layer
- was introduced here 014e2c8, because of #6122
- add proper tests to proof that these queries work!!
  • Loading branch information
kirrg001 authored and ErisDS committed Sep 28, 2017
1 parent 1e2befa commit e347163
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 7 deletions.
8 changes: 1 addition & 7 deletions core/server/models/user.js
Expand Up @@ -336,7 +336,6 @@ User = ghostBookshelf.Model.extend({
findOne: function findOne(dataToClone, options) {
var query,
status,
optInc,
data = _.cloneDeep(dataToClone),
lookupRole = data.role;

Expand All @@ -348,14 +347,9 @@ User = ghostBookshelf.Model.extend({
status = data.status;
delete data.status;

options = _.cloneDeep(options || {});
optInc = options.include;
options.withRelated = _.union(options.withRelated, options.include);

data = this.filterData(data);
options = this.filterOptions(options, 'findOne');
delete options.include;
options.include = optInc;
options.withRelated = _.union(options.withRelated, options.include);

// Support finding by role
if (lookupRole) {
Expand Down
46 changes: 46 additions & 0 deletions core/test/functional/routes/api/public_api_spec.js
Expand Up @@ -373,6 +373,52 @@ describe('Public API', function () {
});
});

it('browse user by slug: count.posts', function (done) {
request.get(testUtils.API.getApiQuery('users/slug/ghost/?client_id=ghost-admin&client_secret=not_available&include=count.posts'))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}

should.not.exist(res.headers['x-cache-invalidate']);
var jsonResponse = res.body;

should.exist(jsonResponse.users);
jsonResponse.users.should.have.length(1);

// We don't expose the email address.
testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['count'], ['email']);
done();
});
});

it('browse user by id: count.posts', function (done) {
request.get(testUtils.API.getApiQuery('users/1/?client_id=ghost-admin&client_secret=not_available&include=count.posts'))
.set('Origin', testUtils.API.getURL())
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}

should.not.exist(res.headers['x-cache-invalidate']);
var jsonResponse = res.body;

should.exist(jsonResponse.users);
jsonResponse.users.should.have.length(1);

// We don't expose the email address.
testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['count'], ['email']);
done();
});
});

it('[unsupported] browse user by email', function (done) {
request
.get(testUtils.API.getApiQuery('users/email/ghost-author@ghost.org/?client_id=ghost-admin&client_secret=not_available'))
Expand Down
46 changes: 46 additions & 0 deletions core/test/functional/routes/api/users_spec.js
Expand Up @@ -312,6 +312,52 @@ describe('User API', function () {
});
});

it('can retrieve a user by slug with count.posts', function (done) {
request.get(testUtils.API.getApiQuery('users/slug/joe-bloggs/?include=count.posts'))
.set('Authorization', 'Bearer ' + ownerAccessToken)
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}

should.not.exist(res.headers['x-cache-invalidate']);
var jsonResponse = res.body;
should.exist(jsonResponse.users);
should.not.exist(jsonResponse.meta);

jsonResponse.users.should.have.length(1);
testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['count']);

done();
});
});

it('can retrieve a user by id with count.posts', function (done) {
request.get(testUtils.API.getApiQuery('users/1/?include=count.posts'))
.set('Authorization', 'Bearer ' + ownerAccessToken)
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}

should.not.exist(res.headers['x-cache-invalidate']);
var jsonResponse = res.body;
should.exist(jsonResponse.users);
should.not.exist(jsonResponse.meta);

jsonResponse.users.should.have.length(1);
testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['count']);

done();
});
});

it('can\'t retrieve non existent user by id', function (done) {
request.get(testUtils.API.getApiQuery('users/' + ObjectId.generate() + '/'))
.set('Authorization', 'Bearer ' + ownerAccessToken)
Expand Down

0 comments on commit e347163

Please sign in to comment.