From 3b8fbe8249e2b0421b64a598ad281ce4c1a93a42 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 17 Nov 2016 14:34:02 +0000 Subject: [PATCH 1/2] If using external auth, show non-editable email field. Also defend server-side against setting email when using external auth. --- .../coffee/Features/User/UserController.coffee | 6 +++++- services/web/app/views/user/settings.jade | 6 ++++++ .../coffee/User/UserControllerTests.coffee | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 47c093d590..c1b75c1c57 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -44,6 +44,7 @@ module.exports = UserController = updateUserSettings : (req, res)-> user_id = AuthenticationController.getLoggedInUserId(req) + usingExternalAuth = !!(settings.ldap? or settings.saml?) logger.log user_id: user_id, "updating account settings" User.findById user_id, (err, user)-> if err? or !user? @@ -74,12 +75,15 @@ module.exports = UserController = user.ace.syntaxValidation = req.body.syntaxValidation user.save (err)-> newEmail = req.body.email?.trim().toLowerCase() - if !newEmail? or newEmail == user.email + if !newEmail? or newEmail == user.email or usingExternalAuth + # end here, don't update email AuthenticationController.setInSessionUser(req, {first_name: user.first_name, last_name: user.last_name}) return res.sendStatus 200 else if newEmail.indexOf("@") == -1 + # email invalid return res.sendStatus(400) else + # update the user email UserUpdater.changeEmailAddress user_id, newEmail, (err)-> if err? logger.err err:err, user_id:user_id, newEmail:newEmail, "problem updaing users email address" diff --git a/services/web/app/views/user/settings.jade b/services/web/app/views/user/settings.jade index eb5bf671fa..14d6902899 100644 --- a/services/web/app/views/user/settings.jade +++ b/services/web/app/views/user/settings.jade @@ -33,6 +33,12 @@ block content ) span.small.text-primary(ng-show="settingsForm.email.$invalid && settingsForm.email.$dirty") | #{translate("must_be_email_address")} + else + // show the email, non-editable + .form-group + label.control-label #{translate("email")} + div.form-control(readonly="true") #{user.email} + .form-group label(for='firstName').control-label #{translate("first_name")} input.form-control( diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index cc1d2190ef..96ba614d22 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -259,6 +259,24 @@ describe "UserController", -> done() @UserController.updateUserSettings @req, @res + describe 'when using an external auth source', -> + + beforeEach -> + @UserUpdater.changeEmailAddress.callsArgWith(2) + @newEmail = 'someone23@example.com' + @settings.ldap = {active: true} + + afterEach -> + delete @settings.ldap + + it 'should not set a new email', (done) -> + @req.body.email = @newEmail + @res.sendStatus = (code)=> + code.should.equal 200 + @UserUpdater.changeEmailAddress.calledWith(@user_id, @newEmail).should.equal false + done() + @UserController.updateUserSettings @req, @res + describe "logout", -> it "should destroy the session", (done)-> From fa146a1558e88bd894425ca84fc4c4f1a02d0aa1 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 17 Nov 2016 14:48:15 +0000 Subject: [PATCH 2/2] Remove redundant `!!` --- services/web/app/coffee/Features/User/UserController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index c1b75c1c57..27e37b91be 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -44,7 +44,7 @@ module.exports = UserController = updateUserSettings : (req, res)-> user_id = AuthenticationController.getLoggedInUserId(req) - usingExternalAuth = !!(settings.ldap? or settings.saml?) + usingExternalAuth = settings.ldap? or settings.saml? logger.log user_id: user_id, "updating account settings" User.findById user_id, (err, user)-> if err? or !user?