From 0d0f0e8604aab3f795c8a8fa08b3cb368457f4dd Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 23 Sep 2016 15:44:47 +0100 Subject: [PATCH 1/3] wip --- .../Authentication/AuthenticationManager.coffee | 3 +++ .../app/views/subscriptions/edit-billing-details.jade | 2 +- services/web/app/views/user/settings.jade | 10 ++++++---- services/web/config/settings.defaults.coffee | 10 +++++----- services/web/public/coffee/directives/asyncForm.coffee | 5 +++++ 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index bfcd55855d..a64890088c 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -29,6 +29,9 @@ module.exports = AuthenticationManager = callback null, null setUserPassword: (user_id, password, callback = (error) ->) -> + if Settings.passwordStrengthOptions?.length?.max? and Settings.passwordStrengthOptions?.length?.max < password.length + return error("password is too long") + bcrypt.genSalt BCRYPT_ROUNDS, (error, salt) -> return callback(error) if error? bcrypt.hash password, salt, (error, hash) -> diff --git a/services/web/app/views/subscriptions/edit-billing-details.jade b/services/web/app/views/subscriptions/edit-billing-details.jade index caf204b79d..0e3ffc442f 100644 --- a/services/web/app/views/subscriptions/edit-billing-details.jade +++ b/services/web/app/views/subscriptions/edit-billing-details.jade @@ -3,7 +3,7 @@ extends ../layout block content - locals.supressDefaultJs = true script(data-main=jsPath+'main.js', src=jsPath+'libs/require.js', baseurl=jsPath) - script(src=buildJsPath('libs/recurly.min.js')) + script(src=buildJsPath('libs/recurly.min.js', {fingerprint:false})) .content.content-alt .container diff --git a/services/web/app/views/user/settings.jade b/services/web/app/views/user/settings.jade index a23c3660aa..d97404a68a 100644 --- a/services/web/app/views/user/settings.jade +++ b/services/web/app/views/user/settings.jade @@ -79,7 +79,7 @@ block content required, complex-password ) - span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.currentPassword.$dirty", ng-bind-html="complexPasswordErrorMessage") + span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.newPassword1.$dirty", ng-bind-html="complexPasswordErrorMessage") .form-group label(for='newPassword2') #{translate("confirm_new_password")} input.form-control( @@ -88,9 +88,11 @@ block content placeholder='*********', ng-model="newPassword2", equals="passwordField" - ) - span.small.text-primary(ng-show="changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") - | #{translate("doesnt_match")} + ) + span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") + | #{translate("doesnt_match")} + span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") + | #{translate("Invalid Password")} .actions button.btn.btn-primary( type='submit', diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index ea5b6ef8a1..cd8421336b 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -190,11 +190,11 @@ module.exports = settings = # ----------- # These restrict the passwords users can use when registering # opts are from http://antelle.github.io/passfield - # passwordStrengthOptions: - # pattern: "aA$3" - # length: - # min: 8 - # max: 50 + passwordStrengthOptions: + # pattern: "aA$3" + length: + min: 1 + max: 10 # Email support # ------------- diff --git a/services/web/public/coffee/directives/asyncForm.coffee b/services/web/public/coffee/directives/asyncForm.coffee index ec2e1dc0bb..b24f4952f3 100644 --- a/services/web/public/coffee/directives/asyncForm.coffee +++ b/services/web/public/coffee/directives/asyncForm.coffee @@ -112,6 +112,8 @@ define [ [asyncFormCtrl, ngModelCtrl] = ctrl ngModelCtrl.$parsers.unshift (modelValue) -> + + isValid = passField.validatePass() email = asyncFormCtrl.getEmail() || window.usersEmail if !isValid @@ -121,5 +123,8 @@ define [ if modelValue.indexOf(email) != -1 or modelValue.indexOf(startOfEmail) != -1 isValid = false scope.complexPasswordErrorMessage = "Password can not contain email address" + if opts.length.max? and modelValue.length == opts.length.max + isValid = false + scope.complexPasswordErrorMessage = "Maxium password length #{opts.length.max} reached" ngModelCtrl.$setValidity('complexPassword', isValid) return modelValue From 8a2b7d04612daa6b86433c0f3fec93b6c2516cc9 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 23 Sep 2016 16:51:46 +0100 Subject: [PATCH 2/3] 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 From ed906f085c8567c2dbaf61c57f6895e7a27ed979 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Mon, 3 Oct 2016 11:33:14 +0100 Subject: [PATCH 3/3] fix spelling mistake in error message and comment out settings.defaults --- services/web/config/settings.defaults.coffee | 10 +++++----- services/web/public/coffee/directives/asyncForm.coffee | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index cd8421336b..96625fa861 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -190,11 +190,11 @@ module.exports = settings = # ----------- # These restrict the passwords users can use when registering # opts are from http://antelle.github.io/passfield - passwordStrengthOptions: - # pattern: "aA$3" - length: - min: 1 - max: 10 + # passwordStrengthOptions: + # pattern: "aA$3" + # length: + # min: 1 + # max: 10 # Email support # ------------- diff --git a/services/web/public/coffee/directives/asyncForm.coffee b/services/web/public/coffee/directives/asyncForm.coffee index b24f4952f3..8bcf610640 100644 --- a/services/web/public/coffee/directives/asyncForm.coffee +++ b/services/web/public/coffee/directives/asyncForm.coffee @@ -125,6 +125,6 @@ define [ scope.complexPasswordErrorMessage = "Password can not contain email address" if opts.length.max? and modelValue.length == opts.length.max isValid = false - scope.complexPasswordErrorMessage = "Maxium password length #{opts.length.max} reached" + scope.complexPasswordErrorMessage = "Maximum password length #{opts.length.max} reached" ngModelCtrl.$setValidity('complexPassword', isValid) return modelValue