mirror of
https://github.com/overleaf/overleaf.git
synced 2024-11-29 19:23:49 -05:00
use parameter for bcrypt rounds, rehash passwords on login if necessary
This commit is contained in:
parent
5e30310aae
commit
98a0c54004
3 changed files with 47 additions and 3 deletions
|
@ -1,8 +1,11 @@
|
||||||
|
Settings = require "settings-sharelatex"
|
||||||
User = require("../../models/User").User
|
User = require("../../models/User").User
|
||||||
{db, ObjectId} = require("../../infrastructure/mongojs")
|
{db, ObjectId} = require("../../infrastructure/mongojs")
|
||||||
crypto = require 'crypto'
|
crypto = require 'crypto'
|
||||||
bcrypt = require 'bcrypt'
|
bcrypt = require 'bcrypt'
|
||||||
|
|
||||||
|
BCRYPT_ROUNDS = Settings?.security?.bcryptRounds or 12
|
||||||
|
|
||||||
module.exports = AuthenticationManager =
|
module.exports = AuthenticationManager =
|
||||||
authenticate: (query, password, callback = (error, user) ->) ->
|
authenticate: (query, password, callback = (error, user) ->) ->
|
||||||
# Using Mongoose for legacy reasons here. The returned User instance
|
# Using Mongoose for legacy reasons here. The returned User instance
|
||||||
|
@ -15,7 +18,9 @@ module.exports = AuthenticationManager =
|
||||||
bcrypt.compare password, user.hashedPassword, (error, match) ->
|
bcrypt.compare password, user.hashedPassword, (error, match) ->
|
||||||
return callback(error) if error?
|
return callback(error) if error?
|
||||||
if match
|
if match
|
||||||
callback null, user
|
AuthenticationManager.checkRounds user, user.hashedPassword, password, (err) ->
|
||||||
|
return callback(err) if err?
|
||||||
|
callback null, user
|
||||||
else
|
else
|
||||||
callback null, null
|
callback null, null
|
||||||
else
|
else
|
||||||
|
@ -24,7 +29,7 @@ module.exports = AuthenticationManager =
|
||||||
callback null, null
|
callback null, null
|
||||||
|
|
||||||
setUserPassword: (user_id, password, callback = (error) ->) ->
|
setUserPassword: (user_id, password, callback = (error) ->) ->
|
||||||
bcrypt.genSalt 7, (error, salt) ->
|
bcrypt.genSalt BCRYPT_ROUNDS, (error, salt) ->
|
||||||
return callback(error) if error?
|
return callback(error) if error?
|
||||||
bcrypt.hash password, salt, (error, hash) ->
|
bcrypt.hash password, salt, (error, hash) ->
|
||||||
return callback(error) if error?
|
return callback(error) if error?
|
||||||
|
@ -35,3 +40,10 @@ module.exports = AuthenticationManager =
|
||||||
$unset: password: true
|
$unset: password: true
|
||||||
}, callback)
|
}, callback)
|
||||||
|
|
||||||
|
checkRounds: (user, hashedPassword, password, callback = (error) ->) ->
|
||||||
|
# check current number of rounds and rehash if necessary
|
||||||
|
currentRounds = bcrypt.getRounds hashedPassword
|
||||||
|
if currentRounds < BCRYPT_ROUNDS
|
||||||
|
AuthenticationManager.setUserPassword user._id, password, callback
|
||||||
|
else
|
||||||
|
callback()
|
||||||
|
|
|
@ -137,6 +137,7 @@ module.exports =
|
||||||
# --------
|
# --------
|
||||||
security:
|
security:
|
||||||
sessionSecret: sessionSecret
|
sessionSecret: sessionSecret
|
||||||
|
bcryptRounds: 14
|
||||||
|
|
||||||
httpAuthUsers: httpAuthUsers
|
httpAuthUsers: httpAuthUsers
|
||||||
|
|
||||||
|
|
|
@ -16,6 +16,7 @@ describe "AuthenticationManager", ->
|
||||||
users: {}
|
users: {}
|
||||||
ObjectId: ObjectId
|
ObjectId: ObjectId
|
||||||
"bcrypt": @bcrypt = {}
|
"bcrypt": @bcrypt = {}
|
||||||
|
"settings-sharelatex": { security: { bcryptRounds: 12 } }
|
||||||
@callback = sinon.stub()
|
@callback = sinon.stub()
|
||||||
|
|
||||||
describe "authenticate", ->
|
describe "authenticate", ->
|
||||||
|
@ -31,6 +32,7 @@ describe "AuthenticationManager", ->
|
||||||
beforeEach (done) ->
|
beforeEach (done) ->
|
||||||
@user.hashedPassword = @hashedPassword = "asdfjadflasdf"
|
@user.hashedPassword = @hashedPassword = "asdfjadflasdf"
|
||||||
@bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
|
@bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
|
||||||
|
@bcrypt.getRounds = sinon.stub().returns 12
|
||||||
@AuthenticationManager.authenticate email: @email, @unencryptedPassword, (error, user) =>
|
@AuthenticationManager.authenticate email: @email, @unencryptedPassword, (error, user) =>
|
||||||
@callback(error, user)
|
@callback(error, user)
|
||||||
done()
|
done()
|
||||||
|
@ -54,6 +56,35 @@ describe "AuthenticationManager", ->
|
||||||
it "should not return the user", ->
|
it "should not return the user", ->
|
||||||
@callback.calledWith(null, null).should.equal true
|
@callback.calledWith(null, null).should.equal true
|
||||||
|
|
||||||
|
describe "when the hashed password matches but the number of rounds is too low", ->
|
||||||
|
beforeEach (done) ->
|
||||||
|
@user.hashedPassword = @hashedPassword = "asdfjadflasdf"
|
||||||
|
@bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
|
||||||
|
@bcrypt.getRounds = sinon.stub().returns 7
|
||||||
|
@AuthenticationManager.setUserPassword = sinon.stub().callsArgWith(2, null)
|
||||||
|
@AuthenticationManager.authenticate email: @email, @unencryptedPassword, (error, user) =>
|
||||||
|
@callback(error, user)
|
||||||
|
done()
|
||||||
|
|
||||||
|
it "should look up the correct user in the database", ->
|
||||||
|
@User.findOne.calledWith(email: @email).should.equal true
|
||||||
|
|
||||||
|
it "should check that the passwords match", ->
|
||||||
|
@bcrypt.compare
|
||||||
|
.calledWith(@unencryptedPassword, @hashedPassword)
|
||||||
|
.should.equal true
|
||||||
|
|
||||||
|
it "should check the number of rounds", ->
|
||||||
|
@bcrypt.getRounds.called.should.equal true
|
||||||
|
|
||||||
|
it "should set the users password (with a higher number of rounds)", ->
|
||||||
|
@AuthenticationManager.setUserPassword
|
||||||
|
.calledWith("user-id", @unencryptedPassword)
|
||||||
|
.should.equal true
|
||||||
|
|
||||||
|
it "should return the user", ->
|
||||||
|
@callback.calledWith(null, @user).should.equal true
|
||||||
|
|
||||||
describe "when the user does not exist in the database", ->
|
describe "when the user does not exist in the database", ->
|
||||||
beforeEach ->
|
beforeEach ->
|
||||||
@User.findOne = sinon.stub().callsArgWith(1, null, null)
|
@User.findOne = sinon.stub().callsArgWith(1, null, null)
|
||||||
|
@ -87,7 +118,7 @@ describe "AuthenticationManager", ->
|
||||||
|
|
||||||
it "should hash the password", ->
|
it "should hash the password", ->
|
||||||
@bcrypt.genSalt
|
@bcrypt.genSalt
|
||||||
.calledWith(7)
|
.calledWith(12)
|
||||||
.should.equal true
|
.should.equal true
|
||||||
@bcrypt.hash
|
@bcrypt.hash
|
||||||
.calledWith(@password, @salt)
|
.calledWith(@password, @salt)
|
||||||
|
|
Loading…
Reference in a new issue