Skip to content

Commit

Permalink
Changing User.read API to default to active users.
Browse files Browse the repository at this point in the history 
refs #3542
- Properly handle forgotten screen (ember)
- Change Users API to only return active users on read
- Adjust tests
  • Loading branch information
@halfdan
halfdan committed Aug 5, 2014
1 parent 954fde1 commit a2d5105
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 67 deletions.
2 changes: 1 addition & 1 deletion core/client/controllers/forgotten.js
View file
Expand Up @@ -26,7 +26,7 @@ var ForgottenController = Ember.Controller.extend(ValidationEngine, {
}
}).then(function (resp) {
self.toggleProperty('submitting');
self.notifications.showSuccess('Please check your email for instructions.');
self.notifications.showSuccess('Please check your email for instructions.', {delayed: true});
self.transitionToRoute('signin');
}).catch(function (resp) {
self.toggleProperty('submitting');
Expand Down
30 changes: 15 additions & 15 deletions core/server/api/authentication.js
View file
Expand Up @@ -4,6 +4,7 @@ var _ = require('lodash'),
mail = require('./mail'),
globalUtils = require('../utils'),
utils = require('./utils'),
users = require('./users'),
when = require('when'),
errors = require('../errors'),
config = require('../config'),
Expand Down Expand Up @@ -41,7 +42,12 @@ authentication = {
return when.reject(new errors.BadRequestError('No email provided.'));
}

return settings.read({context: {internal: true}, key: 'dbHash'}).then(function (response) {
return users.read({ context: {internal: true}, email: email, status: 'active' }).then(function (foundUser) {
if (!foundUser) {
when.reject(new errors.NotFound('Invalid email address'));
}
return settings.read({context: {internal: true}, key: 'dbHash'});
}).then(function (response) {
var dbHash = response.settings[0].value;
return dataProvider.User.generateResetToken(email, expires, dbHash);
}).then(function (resetToken) {
Expand Down Expand Up @@ -147,20 +153,14 @@ authentication = {

isSetup: function () {
return dataProvider.User.query(function (qb) {
qb.where('status', '=', 'active')
.orWhere('status', '=', 'warn-1')
.orWhere('status', '=', 'warn-2')
.orWhere('status', '=', 'warn-3')
.orWhere('status', '=', 'warn-4')
.orWhere('status', '=', 'locked');
qb.whereIn('status', ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked']);
}).fetch().then(function (users) {

if (users) {
return when.resolve({ setup: [{status: true}]});
} else {
return when.resolve({ setup: [{status: false}]});
}
});
if (users) {
return when.resolve({ setup: [{status: true}]});
} else {
return when.resolve({ setup: [{status: false}]});
}
});
},

setup: function (object) {
Expand All @@ -184,7 +184,7 @@ authentication = {
status: 'active'
};

return dataProvider.User.findOne({role: 'Owner'});
return dataProvider.User.findOne({role: 'Owner', status: 'all'});
}).then(function (ownerUser) {
if (ownerUser) {
return dataProvider.User.setup(setupUser, _.extend(internal, {id: ownerUser.id}));
Expand Down
6 changes: 3 additions & 3 deletions core/server/api/users.js
View file
Expand Up @@ -96,7 +96,7 @@ users = {
* @returns {Promise(User)} User
*/
read: function read(options) {
var attrs = ['id', 'slug', 'email'],
var attrs = ['id', 'slug', 'email', 'status'],
data = _.pick(options, attrs);

options = _.omit(options, attrs);
Expand Down Expand Up @@ -154,7 +154,7 @@ users = {
roleId = parseInt(role.id || role, 10);

return dataProvider.User.findOne(
{id: options.context.user, include: 'roles'}
{id: options.context.user, status: 'all'}, {include: 'roles'}
).then(function (contextUser) {
var contextRoleId = contextUser.related('roles').toJSON()[0].id;

Expand Down Expand Up @@ -240,7 +240,7 @@ users = {

// If sending the invitation failed, set status to invited-pending
return dataProvider.User.edit({status: 'invited-pending'}, {id: user.id}).then(function (user) {
return dataProvider.User.findOne({ id: user.id }, options).then(function (user) {
return dataProvider.User.findOne({ id: user.id, status: 'all' }, options).then(function (user) {
return { users: [user] };
});
});
Expand Down
42 changes: 34 additions & 8 deletions core/server/models/user.js
View file
Expand Up @@ -10,6 +10,8 @@ var _ = require('lodash'),
Role = require('./role').Role,

tokenSecurity = {},
activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked'],
invitedStates = ['invited', 'invited-pending'],
User,
Users;

Expand Down Expand Up @@ -109,7 +111,7 @@ User = ghostBookshelf.Model.extend({
// whitelists for the `options` hash argument on methods, by method name.
// these are the only options that can be passed to Bookshelf / Knex.
validOptions = {
findOne: ['withRelated'],
findOne: ['withRelated', 'status'],
findAll: ['withRelated'],
setup: ['id'],
edit: ['withRelated', 'id'],
Expand Down Expand Up @@ -196,9 +198,9 @@ User = ghostBookshelf.Model.extend({
}

if (options.status === 'active') {
userCollection.query().whereIn('status', ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked']);
userCollection.query().whereIn('status', activeStates);
} else if (options.status === 'invited') {
userCollection.query().whereIn('status', ['invited', 'invited-pending']);
userCollection.query().whereIn('status', invitedStates);
} else if (options.status !== 'all') {
options.where.status = options.status;
}
Expand Down Expand Up @@ -320,6 +322,16 @@ User = ghostBookshelf.Model.extend({
* **See:** [ghostBookshelf.Model.findOne](base.js.html#Find%20One)
*/
findOne: function (data, options) {
var query,
status;

data = _.extend({
status: 'active'
}, data || {});

status = data.status;
delete data.status;

options = options || {};
options.withRelated = _.union([ 'roles' ], options.include);

Expand All @@ -333,11 +345,24 @@ User = ghostBookshelf.Model.extend({
delete data.role;
}

if (data.status === 'all') {
delete data.status;
// We pass include to forge so that toJSON has access
query = this.forge(data, {include: options.include});

data = this.filterData(data);


if (status === 'active') {
query.query('whereIn', 'status', activeStates);
} else if (status === 'invited') {
query.query('whereIn', 'status', invitedStates);
} else if (status !== 'all') {
query.query('where', { 'status': options.status});
}

return ghostBookshelf.Model.findOne.call(this, data, options);
options = this.filterOptions(options, 'findOne');
delete options.include;

return query.fetch(options);
},

/**
Expand Down Expand Up @@ -379,7 +404,8 @@ User = ghostBookshelf.Model.extend({
return user.roles().updatePivot({role_id: roleId});
}
}).then(function () {
return self.findOne(user, options);
options.status = 'all';
return self.findOne({id: user.id}, options);
});
}
return user;
Expand Down Expand Up @@ -446,7 +472,7 @@ User = ghostBookshelf.Model.extend({
return userData.roles().attach(roles, options);
}).then(function () {
// find and return the added user
return self.findOne({id: userData.id}, options);
return self.findOne({id: userData.id, status: 'all'}, options);
});
},

Expand Down
2 changes: 1 addition & 1 deletion core/server/permissions/effective.js
View file
Expand Up @@ -4,7 +4,7 @@ var _ = require('lodash'),

var effective = {
user: function (id) {
return Models.User.findOne({id: id}, { include: ['permissions', 'roles', 'roles.permissions'] })
return Models.User.findOne({id: id, status: 'all'}, { include: ['permissions', 'roles', 'roles.permissions'] })
.then(function (foundUser) {
var seenPerms = {},
rolePerms = _.map(foundUser.related('roles').models, function (role) {
Expand Down
2 changes: 1 addition & 1 deletion core/test/integration/api/api_authentication_spec.js
View file
Expand Up @@ -21,7 +21,7 @@ describe('Authentication API', function () {
describe('Not completed', function () {

// TODO: stub settings
beforeEach(testUtils.setup('owner:pre', 'settings', 'perms:setting', 'perms:init'));
beforeEach(testUtils.setup('roles', 'owner:pre', 'settings', 'perms:setting', 'perms:init'));

it('should report that setup has not been completed', function (done) {
AuthAPI.isSetup().then(function (result) {
Expand Down
55 changes: 29 additions & 26 deletions core/test/integration/api/api_users_spec.js
View file
Expand Up @@ -38,7 +38,7 @@ describe('Users API', function () {
});

describe('Browse', function () {
function checkBrowseResponse(response, count) {
function checkBrowseResponse(response, count) {
should.exist(response);
testUtils.API.checkResponse(response, 'users');
should.exist(response.users);
Expand All @@ -47,60 +47,63 @@ describe('Users API', function () {
testUtils.API.checkResponse(response.users[1], 'user', ['roles']);
testUtils.API.checkResponse(response.users[2], 'user', ['roles']);
testUtils.API.checkResponse(response.users[3], 'user', ['roles']);
}
}

it('Owner can browse', function (done) {
it('Owner can browse', function (done) {
UserAPI.browse(context.owner).then(function (response) {
checkBrowseResponse(response, 5);
checkBrowseResponse(response, 7);
done();
}).catch(done);
});
});

it('Admin can browse', function (done) {
it('Admin can browse', function (done) {
UserAPI.browse(context.admin).then(function (response) {
checkBrowseResponse(response, 5);
checkBrowseResponse(response, 7);
done();
}).catch(done);
});
});

it('Editor can browse', function (done) {
it('Editor can browse', function (done) {
UserAPI.browse(context.editor).then(function (response) {
checkBrowseResponse(response, 5);
checkBrowseResponse(response, 7);
done();
}).catch(done);
});
});

it('Author can browse active', function (done) {
it('Author can browse active', function (done) {
UserAPI.browse(context.author).then(function (response) {
checkBrowseResponse(response, 5);
checkBrowseResponse(response, 7);
done();
}).catch(done);
});
});

it('No-auth CANNOT browse', function (done) {
it('No-auth CANNOT browse', function (done) {
UserAPI.browse().then(function () {
done(new Error('Browse users is not denied without authentication.'));
}, function () {
done();
}).catch(done);
});
});


it('Can browse invited/invited-pending (admin)', function (done) {
UserAPI.browse(_.extend(testUtils.context.admin, { status: 'invited' })).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'users');
should.exist(response.users);
response.users.should.have.length(1);
testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
response.users[0].status.should.equal('invited-pending');
testUtils.fixtures.createInvitedUsers().then(function () {
UserAPI.browse(_.extend(testUtils.context.admin, { status: 'invited' })).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'users');
should.exist(response.users);
response.users.should.have.length(3);
testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
response.users[0].status.should.equal('invited-pending');

done();
}).catch(done);
done();
}).catch(done);
});
});

it('Author can browse', function (done) {
UserAPI.browse(context.author).then(function (response) {
checkBrowseResponse(response, 5);
checkBrowseResponse(response, 7);
done();
}).catch(done);
});
Expand Down
4 changes: 2 additions & 2 deletions core/test/integration/model/model_users_spec.js
View file
Expand Up @@ -179,7 +179,7 @@ describe('User Model', function run() {
results.meta.pagination.page.should.equal(1);
results.meta.pagination.limit.should.equal(15);
results.meta.pagination.pages.should.equal(1);
results.users.length.should.equal(3);
results.users.length.should.equal(4);

done();
}).catch(done);
Expand All @@ -193,7 +193,7 @@ describe('User Model', function run() {
results.meta.pagination.limit.should.equal(15);
results.meta.pagination.pages.should.equal(1);
results.meta.pagination.total.should.equal(2);
results.users.length.should.equal(1);
results.users.length.should.equal(2);

return UserModel.findPage({role: 'Owner'});
}).then(function (results) {
Expand Down
15 changes: 5 additions & 10 deletions core/test/utils/fixtures/data-generator.js
View file
Expand Up @@ -75,36 +75,31 @@ DataGenerator.Content = {
name: 'Joe Bloggs',
slug: 'joe-blogs',
email: 'jbloggs@example.com',
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6',
status: 'active'
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6'
},
{
name: 'Smith Wellingsworth',
slug: 'smith-wellingsworth',
email: 'swellingsworth@example.com',
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6',
status: 'invited-pending'
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6'
},
{
name: 'Jimothy Bogendath',
slug: 'jimothy-bogendath',
email: 'jbOgendAth@example.com',
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6',
status: 'warn-1'
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6'
},
{
name: 'Slimer McEctoplasm',
slug: 'slimer-mcectoplasm',
email: 'smcectoplasm@example.com',
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6',
status: 'warn-2'
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6'
},
{
name: 'Ivan Email',
slug: 'ivan-email',
email: 'info@ghost.org',
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6',
status: 'inactive'
password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6'
}
],

Expand Down
22 changes: 22 additions & 0 deletions core/test/utils/index.js
View file
Expand Up @@ -225,6 +225,28 @@ fixtures = {
});
},

createInvitedUsers: function createInvitedUser() {
var knex = config.database.knex,
// grab 3 more users
extraUsers = DataGenerator.Content.users.slice(2, 5);

extraUsers = _.map(extraUsers, function (user) {
return DataGenerator.forKnex.createUser(_.extend({}, user, {
email: 'inv' + user.email,
slug: 'inv' + user.slug,
status: 'invited-pending'
}));
});

return knex('users').insert(extraUsers).then(function () {
return knex('roles_users').insert([
{ user_id: 8, role_id: 1},
{ user_id: 9, role_id: 2},
{ user_id: 10, role_id: 3}
]);
});
},

insertOne: function insertOne(obj, fn) {
var knex = config.database.knex;
return knex(obj)
Expand Down

0 comments on commit a2d5105

Please sign in to comment.