Skip to content
This repository has been archived by the owner on Sep 6, 2023. It is now read-only.

Commit

Permalink
Fix password validation bug and add test for it
Browse files Browse the repository at this point in the history
This ensures that the password validation function `bcrypt.compare`
actually completes its process before the next step of the promise chain
executes, then the `===` ensures that the value of `valid` is actually
`true`, not just a truthy value. The test ensures that this function
actually returns an error for an invalid combination of username and
password.
  • Loading branch information
AdamVig committed Feb 17, 2017
1 parent 6da1344 commit 07376ff
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 3 deletions.
6 changes: 3 additions & 3 deletions controllers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ module.exports.authenticate = (req, res, next) => {
if (req.body.email && req.body.password) {
return db.get('organization', 'email', req.body.email)
.then(organization => {
return [
return Promise.all([
organization.organization_id,
bcrypt.compare(req.body.password, organization.password)
]
])
})
.then(([organizationID, valid]) => {
if (valid) {
if (valid === true) {
const token = makeToken({sub: organizationID})
res.send({
id: organizationID,
Expand Down
19 changes: 19 additions & 0 deletions test/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ test('Authenticates an organization', async t => {
t.false(next.called, 'no errors')
})

test('Returns error when email and password do not match', async t => {
const req = {
body: d.authOrganizationWrong
}
const res = {
send: sinon.spy()
}
const next = sinon.spy()

await knex(d.table).insert(d.authOrganizationWrongHash)
await auth.authenticate(req, res, next)

t.false(res.send.called, 'does not respond')
t.true(next.called, 'returns an error')
})

test.after.always('Clean up database', async t => {
// Delete created organizations
await knex(d.table)
Expand All @@ -74,4 +90,7 @@ test.after.always('Clean up database', async t => {
await knex(d.table)
.where('email', d.authOrganization.email)
.del()
await knex(d.table)
.where('email', d.authOrganizationWrong.email)
.del()
})
10 changes: 10 additions & 0 deletions test/fixtures/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,15 @@
"name": "Second Test Organization",
"email": "authtestorg@example.com",
"password": "$2a$10$iWpLbGBdE99lnTTSFOT.u.MgKvGSdzapRpTn4iIqeKrkoEq/308Lq"
},
"authOrganizationWrong": {
"name": "Third Test Organization",
"email": "authtestorg3@example.com",
"password": "testpassword"
},
"authOrganizationWrongHash": {
"name": "Third Test Organization",
"email": "authtestorg3@example.com",
"password": "$2a$10$iWpLbGBdE99lnTTSFOT.u.MgKvGSdzapRpTn4iIqeKrkoEq/308Lq"
}
}

0 comments on commit 07376ff

Please sign in to comment.