From 8a2b7d04612daa6b86433c0f3fec93b6c2516cc9 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Fri, 23 Sep 2016 16:51:46 +0100
Subject: [PATCH] server side protect passwords which are too long

---
 .../AuthenticationManager.coffee              |  2 +-
 .../AuthenticationManagerTests.coffee         | 70 +++++++++++++------
 2 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee
index a64890088c..b661455028 100644
--- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee
+++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee
@@ -30,7 +30,7 @@ module.exports = AuthenticationManager =
 
 	setUserPassword: (user_id, password, callback = (error) ->) ->
 		if Settings.passwordStrengthOptions?.length?.max? and Settings.passwordStrengthOptions?.length?.max < password.length
-			return error("password is too long")
+			return callback("password is too long")
 
 		bcrypt.genSalt BCRYPT_ROUNDS, (error, salt) ->
 			return callback(error) if error?
diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee
index 2805527259..7e82a7a3cb 100644
--- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee
+++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee
@@ -9,6 +9,7 @@ ObjectId = require("mongojs").ObjectId
 
 describe "AuthenticationManager", ->
 	beforeEach ->
+		@settings = { security: { bcryptRounds: 12 } }
 		@AuthenticationManager = SandboxedModule.require modulePath, requires:
 			"../../models/User": User: @User = {}
 			"../../infrastructure/mongojs":
@@ -16,7 +17,7 @@ describe "AuthenticationManager", ->
 					users: {}
 				ObjectId: ObjectId
 			"bcrypt": @bcrypt = {}
-			"settings-sharelatex": { security: { bcryptRounds: 12 } }
+			"settings-sharelatex": @settings
 		@callback = sinon.stub()
 
 	describe "authenticate", ->
@@ -102,27 +103,52 @@ describe "AuthenticationManager", ->
 			@bcrypt.genSalt = sinon.stub().callsArgWith(1, null, @salt)
 			@bcrypt.hash = sinon.stub().callsArgWith(2, null, @hashedPassword)
 			@db.users.update = sinon.stub().callsArg(2)
-			@AuthenticationManager.setUserPassword(@user_id, @password, @callback)
 
-		it "should update the user's password in the database", ->
-			@db.users.update
-				.calledWith({
-					_id: ObjectId(@user_id.toString())
-				}, {
-					$set: {
-						"hashedPassword": @hashedPassword
-					}
-					$unset: password: true
-				})
-				.should.equal true
+		describe "too long", ->
+			beforeEach ->
+				@settings.passwordStrengthOptions =
+					length: 
+						max:10
+				@password = "dsdsadsadsadsadsadkjsadjsadjsadljs"
+
+			it "should return and error", (done)->
+				@AuthenticationManager.setUserPassword @user_id, @password, (err)->
+					expect(err).to.exist
+					done()
+
+
+			it "should not start the bcrypt process", (done)->
+				@AuthenticationManager.setUserPassword @user_id, @password, (err)=>
+					@bcrypt.genSalt.called.should.equal false
+					@bcrypt.hash.called.should.equal false
+					done()
+
+		describe "successful set", ->
+			beforeEach -> 
+				@AuthenticationManager.setUserPassword(@user_id, @password, @callback)
+
+			it "should update the user's password in the database", ->
+				@db.users.update
+					.calledWith({
+						_id: ObjectId(@user_id.toString())
+					}, {
+						$set: {
+							"hashedPassword": @hashedPassword
+						}
+						$unset: password: true
+					})
+					.should.equal true
+
+			it "should hash the password", ->
+				@bcrypt.genSalt
+					.calledWith(12)
+					.should.equal true
+				@bcrypt.hash
+					.calledWith(@password, @salt)
+					.should.equal true
+
+			it "should call the callback", ->
+				@callback.called.should.equal true
+
 
-		it "should hash the password", ->
-			@bcrypt.genSalt
-				.calledWith(12)
-				.should.equal true
-			@bcrypt.hash
-				.calledWith(@password, @salt)
-				.should.equal true
 
-		it "should call the callback", ->
-			@callback.called.should.equal true