Skip to content

Commit

Permalink
User model's defaultScope should exclude password. (#157)
Browse files Browse the repository at this point in the history
By default, the API will now return User records *without* the encrypted password field.
Explicitly use the db model scope “withSensitiveInfo” to include the encrypted password field in the response (e.g. for passport configuration).
Add tests to confirm both the default and the scoped behavior.
Unskip all the “getWriters” tests.
  • Loading branch information
iamigo committed Dec 28, 2016
1 parent 1a09765 commit 5e85640
Show file tree
Hide file tree
Showing 12 changed files with 174 additions and 19 deletions.
3 changes: 0 additions & 3 deletions api/v1/helpers/nouns/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ module.exports = {
POST: `Create a new ${m}`,
PUT: `Overwrite all attributes of this ${m}`,
},
scopeMap: {
withoutSensitiveInfo: 'withoutSensitiveInfo',
},
baseUrl: '/v1/users',
model: User,
modelName: 'User',
Expand Down
2 changes: 1 addition & 1 deletion config/passportconfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
'use strict'; // eslint-disable-line strict

const LocalStrategy = require('passport-local').Strategy;
const User = require('../db/index').User;
const User = require('../db/index').User.scope('withSensitiveInfo');
const Token = require('../db/index').Token;
const Profile = require('../db/index').Profile;
const bcrypt = require('bcrypt-nodejs');
Expand Down
13 changes: 5 additions & 8 deletions db/model/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,20 @@ module.exports = function user(seq, dataTypes) {
foreignKey: 'userId',
});
User.addScope('defaultScope', {

attributes: {
exclude: ['password'],
},
include: [
{
association: assoc.profile,
attributes: ['name'],
},
],
order: ['User.name'],
}, {
override: true,
});
User.addScope('withoutSensitiveInfo', {
attributes: {
exclude: ['password'],
},
User.addScope('withSensitiveInfo', {
include: [
{
association: assoc.profile,
Expand All @@ -115,9 +115,6 @@ module.exports = function user(seq, dataTypes) {
});
},
},
defaultScope: {
order: ['User.name'],
},
hooks: {

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/api/v1/aspects/getWriters.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('api: aspects: get writer(s)', () => {
});
});

it.skip('find Writers and make sure the passwords are not returned', (done) => {
it('find Writers and make sure the passwords are not returned', (done) => {
api.get(getWritersPath.replace('{key}', aspect.name))
.set('Authorization', token)
.expect(constants.httpStatus.OK)
Expand Down
2 changes: 1 addition & 1 deletion tests/api/v1/lenses/getWriters.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('api: lenses: get writers}', () => {
});
});

it.skip('find Writers and make sure the passwords are not returned', (done) => {
it('find Writers and make sure the passwords are not returned', (done) => {
api.get(getWritersPath.replace('{key}', lens.name))
.set('Authorization', token)
.expect(constants.httpStatus.OK)
Expand Down
2 changes: 1 addition & 1 deletion tests/api/v1/perspectives/getWriters.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe('api: perspective: get writers', () => {
});
});

it.skip('find Writers and make sure the passwords are not returned', (done) => {
it('find Writers and make sure the passwords are not returned', (done) => {
api.get(getWritersPath.replace('{key}', perspective.name))
.set('Authorization', token)
.expect(constants.httpStatus.OK)
Expand Down
2 changes: 1 addition & 1 deletion tests/api/v1/subjects/getWriters.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('api: subjects: get writers}', () => {
});
});

it.skip('find Writers and make sure the passwords are not returned', (done) => {
it('find Writers and make sure the passwords are not returned', (done) => {
api.get(getWritersPath.replace('{key}', subject.name))
.set('Authorization', token)
.expect(constants.httpStatus.OK)
Expand Down
2 changes: 1 addition & 1 deletion tests/api/v1/userTokens/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,4 @@ describe(`api: GET ${path}/U/tokens/T`, () => {
.expect(constants.httpStatus.NOT_FOUND)
.end(() => done());
});
});
});
82 changes: 82 additions & 0 deletions tests/api/v1/users/get.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* Copyright (c) 2016, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or
* https://opensource.org/licenses/BSD-3-Clause
*/

/**
* tests/api/v1/users/get.js
*/
'use strict';

const supertest = require('supertest');
const api = supertest(require('../../../../index').app);
const constants = require('../../../../api/v1/constants');
const tu = require('../../../testUtils');
const u = require('./utils');
const path = '/v1/users';
const expect = require('chai').expect;
const Profile = tu.db.Profile;
const User = tu.db.User;
const Token = tu.db.Token;

describe(`api: GET ${path}`, () => {
const uname = `${tu.namePrefix}test@refocus.com`;
const tname = `${tu.namePrefix}Voldemort`;

before((done) => {
Profile.create({
name: `${tu.namePrefix}testProfile`,
})
.then((profile) =>
User.create({
profileId: profile.id,
name: uname,
email: uname,
password: 'user123password',
})
)
.then(() => done())
.catch(done);
});

after(u.forceDelete);

it('user found', (done) => {
api.get(`${path}/${uname}`)
.expect(constants.httpStatus.OK)
.end((err, res) => {
if (err) {
done(err);
} else {
expect(res.body).to.have.property('name', uname);
expect(res.body).to.not.have.property('password');
expect(res.body.isDeleted).to.not.equal(0);
done();
}
});
});

it('users array returned', (done) => {
api.get(`${path}`)
.expect(constants.httpStatus.OK)
.end((err, res) => {
if (err) {
done(err);
} else {
expect(res.body).to.be.instanceof(Array);
expect(res.body[0]).to.not.have.property('password');
done();
}
});
});

it('user not found', (done) => {
api.get(`${path}/who@what.com`)
.set('Authorization', '???')
.expect(constants.httpStatus.NOT_FOUND)
.end(() => done());
});
});
25 changes: 25 additions & 0 deletions tests/api/v1/users/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright (c) 2016, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or
* https://opensource.org/licenses/BSD-3-Clause
*/

/**
* tests/api/v1/users/utils.js
*/
'use strict';

const tu = require('../../../testUtils');

const testStartTime = new Date();

module.exports = {
forceDelete(done) {
tu.forceDelete(tu.db.User, testStartTime)
.then(() => tu.forceDelete(tu.db.Profile, testStartTime))
.then(() => done())
.catch(done);
},
};
5 changes: 3 additions & 2 deletions tests/db/model/user/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ describe('db: User: create', () => {
expect(user).to.have.property('email').to.equal('user@example.com');
expect(user.password).to.not.equal('user123password');
bcrypt.compare('user123password', user.password, (err, res) => {
if(err){
if (err) {
throw err;
}
expect(res).to.be.true; // eslint-disable-line no-unused-expressions

expect(res).to.be.true; // eslint-disable-line no-unused-expressions
});
done();
});
Expand Down
53 changes: 53 additions & 0 deletions tests/db/model/user/find.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) 2016, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or
* https://opensource.org/licenses/BSD-3-Clause
*/

/**
* tests/db/model/user/find.js
*/
'use strict'; // eslint-disable-line strict
const tu = require('../../../testUtils');
const u = require('./utils');
const expect = require('chai').expect;
const User = tu.db.User;
const Profile = tu.db.Profile;

describe('db: user: find: ', () => {
beforeEach((done) => {
Profile.create({ name: `${tu.namePrefix}1` })
.then((createdProfile) => {
return User.create({
profileId: createdProfile.id,
name: `${tu.namePrefix}1`,
email: 'user@example.com',
password: 'user123password',
});
})
.then(() => done())
.catch(done);
});

afterEach(u.forceDelete);

it('default scope no password', (done) => {
User.find({ name: `${tu.namePrefix}1` })
.then((found) => {
expect(found.dataValues).to.not.have.property('password');
done();
})
.catch(done);
});

it('withSensitiveInfo scope', (done) => {
User.scope('withSensitiveInfo').find({ name: `${tu.namePrefix}1` })
.then((found) => {
expect(found.dataValues).to.have.property('password');
done();
})
.catch(done);
});
});

0 comments on commit 5e85640

Please sign in to comment.