From e614ed9248359a3dd1e77a8e878a70aad3f9bfc1 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Fri, 8 Jun 2018 19:05:19 +0200 Subject: [PATCH 1/2] add emails endpoints --- .../Features/User/UserEmailsController.coffee | 43 +++++ .../coffee/Features/User/UserGetter.coffee | 11 ++ services/web/app/coffee/router.coffee | 13 ++ .../User/UserEmailsControllerTests.coffee | 157 ++++++++++++++++++ .../unit/coffee/User/UserGetterTests.coffee | 26 ++- .../unit/coffee/helpers/MockRequest.coffee | 1 + 6 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 services/web/app/coffee/Features/User/UserEmailsController.coffee create mode 100644 services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee diff --git a/services/web/app/coffee/Features/User/UserEmailsController.coffee b/services/web/app/coffee/Features/User/UserEmailsController.coffee new file mode 100644 index 0000000000..07433d2dde --- /dev/null +++ b/services/web/app/coffee/Features/User/UserEmailsController.coffee @@ -0,0 +1,43 @@ +AuthenticationController = require('../Authentication/AuthenticationController') +UserGetter = require("./UserGetter") +UserUpdater = require("./UserUpdater") +EmailHelper = require("../Helpers/EmailHelper") +logger = require("logger-sharelatex") + +module.exports = UserEmailsController = + + list: (req, res) -> + userId = AuthenticationController.getLoggedInUserId(req) + UserGetter.getUserFullEmails userId, (error, fullEmails) -> + return res.sendStatus 500 if error? + res.json fullEmails + + + add: (req, res) -> + userId = AuthenticationController.getLoggedInUserId(req) + email = EmailHelper.parseEmail(req.body.email) + return res.sendStatus 422 unless email? + + UserUpdater.addEmailAddress userId, email, (error)-> + return res.sendStatus 500 if error? + res.sendStatus 200 + + + remove: (req, res) -> + userId = AuthenticationController.getLoggedInUserId(req) + email = EmailHelper.parseEmail(req.body.email) + return res.sendStatus 422 unless email? + + UserUpdater.removeEmailAddress userId, email, (error)-> + return res.sendStatus 500 if error? + res.sendStatus 200 + + + setDefault: (req, res) -> + userId = AuthenticationController.getLoggedInUserId(req) + email = EmailHelper.parseEmail(req.body.email) + return res.sendStatus 422 unless email? + + UserUpdater.setDefaultEmailAddress userId, email, (error)-> + return res.sendStatus 500 if error? + res.sendStatus 200 diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index 4ba750be17..59e45e1795 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/services/web/app/coffee/Features/User/UserGetter.coffee @@ -25,6 +25,17 @@ module.exports = UserGetter = @getUser userId, { email: 1 }, (error, user) -> callback(error, user?.email) + getUserFullEmails: (userId, callback = (error, emails) ->) -> + @getUser userId, { email: 1, emails: 1 }, (error, user) -> + return callback error if error? + return callback new Error('User not Found') unless user + + fullEmails = user.emails.map (emailData) -> + emailData.default = emailData.email == user.email + emailData + + callback null, fullEmails + getUserByMainEmail: (email, projection, callback = (error, user) ->) -> email = email.trim() if arguments.length == 2 diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 22c4abe925..e2128effd7 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -17,6 +17,7 @@ NotificationsController = require("./Features/Notifications/NotificationsControl CollaboratorsRouter = require('./Features/Collaborators/CollaboratorsRouter') UserInfoController = require('./Features/User/UserInfoController') UserController = require("./Features/User/UserController") +UserEmailsController = require("./Features/User/UserEmailsController") UserPagesController = require('./Features/User/UserPagesController') DocumentController = require('./Features/Documents/DocumentController') CompileManager = require("./Features/Compile/CompileManager") @@ -107,6 +108,18 @@ module.exports = class Router timeInterval: 60 }), UserController.changePassword + webRouter.get '/user/emails', + AuthenticationController.requireLogin(), + UserEmailsController.list + webRouter.post '/user/emails', + AuthenticationController.requireLogin(), + UserEmailsController.add + webRouter.delete '/user/emails', + AuthenticationController.requireLogin(), + UserEmailsController.remove + webRouter.post '/user/emails/default', + AuthenticationController.requireLogin(), + UserEmailsController.setDefault webRouter.get '/user/sessions', AuthenticationController.requireLogin(), diff --git a/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee b/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee new file mode 100644 index 0000000000..5b4a32fb38 --- /dev/null +++ b/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee @@ -0,0 +1,157 @@ +sinon = require('sinon') +assertCalledWith = sinon.assert.calledWith +assertNotCalled = sinon.assert.notCalled +chai = require('chai') +should = chai.should() +assert = chai.assert +modulePath = "../../../../app/js/Features/User/UserEmailsController.js" +SandboxedModule = require('sandboxed-module') +MockRequest = require "../helpers/MockRequest" +MockResponse = require "../helpers/MockResponse" + +describe "UserEmailsController", -> + beforeEach -> + @req = new MockRequest() + @user = + _id: 'mock-user-id' + + @UserGetter = + getUserFullEmails: sinon.stub() + @AuthenticationController = + getLoggedInUserId: sinon.stub().returns(@user._id) + @UserUpdater = + addEmailAddress: sinon.stub() + removeEmailAddress: sinon.stub() + setDefaultEmailAddress: sinon.stub() + @EmailHelper = + parseEmail: sinon.stub() + @UserEmailsController = SandboxedModule.require modulePath, requires: + "../Authentication/AuthenticationController": @AuthenticationController + "./UserGetter": @UserGetter + "./UserUpdater": @UserUpdater + "../Helpers/EmailHelper": @EmailHelper + "logger-sharelatex": + log: -> console.log(arguments) + err: -> + + describe 'List', -> + beforeEach -> + + it 'lists emails', (done) -> + fullEmails = [{some: 'data'}] + @UserGetter.getUserFullEmails.callsArgWith 1, null, fullEmails + + @UserEmailsController.list @req, + json: (response) => + assert.deepEqual response, fullEmails + assertCalledWith @UserGetter.getUserFullEmails, @user._id + done() + + it 'handles error', (done) -> + @UserGetter.getUserFullEmails.callsArgWith 1, new Error('Oups') + + @UserEmailsController.list @req, + sendStatus: (code) => + code.should.equal 500 + done() + + describe 'Add', -> + beforeEach -> + @newEmail = 'new_email@baz.com' + @req.body.email = @newEmail + @EmailHelper.parseEmail.returns @newEmail + + it 'adds new email', (done) -> + @UserUpdater.addEmailAddress.callsArgWith 2, null + + @UserEmailsController.add @req, + sendStatus: (code) => + code.should.equal 200 + assertCalledWith @EmailHelper.parseEmail, @newEmail + assertCalledWith @UserUpdater.addEmailAddress, @user._id, @newEmail + done() + + it 'handles email parse error', (done) -> + @EmailHelper.parseEmail.returns null + + @UserEmailsController.add @req, + sendStatus: (code) => + code.should.equal 422 + assertNotCalled @UserUpdater.addEmailAddress + done() + + it 'handles error', (done) -> + @UserUpdater.addEmailAddress.callsArgWith 2, new Error('Oups') + + @UserEmailsController.add @req, + sendStatus: (code) => + code.should.equal 500 + done() + + describe 'remove', -> + beforeEach -> + @email = 'email_to_remove@bar.com' + @req.body.email = @email + @EmailHelper.parseEmail.returns @email + + it 'removes email', (done) -> + @UserUpdater.removeEmailAddress.callsArgWith 2, null + + @UserEmailsController.remove @req, + sendStatus: (code) => + code.should.equal 200 + assertCalledWith @EmailHelper.parseEmail, @email + assertCalledWith @UserUpdater.removeEmailAddress, @user._id, @email + done() + + it 'handles email parse error', (done) -> + @EmailHelper.parseEmail.returns null + + @UserEmailsController.remove @req, + sendStatus: (code) => + code.should.equal 422 + assertNotCalled @UserUpdater.removeEmailAddress + done() + + it 'handles error', (done) -> + @UserUpdater.removeEmailAddress.callsArgWith 2, new Error('Oups') + + @UserEmailsController.remove @req, + sendStatus: (code) => + code.should.equal 500 + done() + + + describe 'setDefault', -> + beforeEach -> + @email = "email_to_set_default@bar.com" + @req.body.email = @email + @EmailHelper.parseEmail.returns @email + + it 'sets default email', (done) -> + @UserUpdater.setDefaultEmailAddress.callsArgWith 2, null + + @UserEmailsController.setDefault @req, + sendStatus: (code) => + code.should.equal 200 + assertCalledWith @EmailHelper.parseEmail, @email + assertCalledWith @UserUpdater.setDefaultEmailAddress, @user._id, @email + done() + + it 'handles email parse error', (done) -> + @EmailHelper.parseEmail.returns null + + @UserEmailsController.setDefault @req, + sendStatus: (code) => + code.should.equal 422 + assertNotCalled @UserUpdater.setDefaultEmailAddress + done() + + it 'handles error', (done) -> + @UserUpdater.setDefaultEmailAddress.callsArgWith 2, new Error('Oups') + + @UserEmailsController.setDefault @req, + sendStatus: (code) => + code.should.equal 500 + done() + diff --git a/services/web/test/unit/coffee/User/UserGetterTests.coffee b/services/web/test/unit/coffee/User/UserGetterTests.coffee index fbea0e8b53..f2b7895b57 100644 --- a/services/web/test/unit/coffee/User/UserGetterTests.coffee +++ b/services/web/test/unit/coffee/User/UserGetterTests.coffee @@ -9,7 +9,13 @@ expect = require("chai").expect describe "UserGetter", -> beforeEach -> - @fakeUser = {_id:"12390i"} + @fakeUser = + _id: '12390i' + email: 'email2@foo.bar' + emails: [ + { email: 'email1@foo.bar' } + { email: 'email2@foo.bar' } + ] @findOne = sinon.stub().callsArgWith(2, null, @fakeUser) @Mongo = db: users: findOne: @findOne @@ -35,6 +41,24 @@ describe "UserGetter", -> error.should.exist done() + describe "getUserFullEmails", - + it "should get user", (done)-> + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @fakeUser) + projection = email: 1, emails: 1 + @UserGetter.getUserFullEmails @fakeUser._id, (error, fullEmails) => + @UserGetter.getUser.called.should.equal true + @UserGetter.getUser.calledWith(@fakeUser._id, projection).should.equal true + done() + + it "should fetch emails data", (done)-> + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @fakeUser) + @UserGetter.getUserFullEmails @fakeUser._id, (error, fullEmails) => + assert.deepEqual fullEmails, [ + { email: 'email1@foo.bar', default: false } + { email: 'email2@foo.bar', default: true } + ] + done() + describe "getUserbyMainEmail", -> it "query user by main email", (done)-> email = 'hello@world.com' diff --git a/services/web/test/unit/coffee/helpers/MockRequest.coffee b/services/web/test/unit/coffee/helpers/MockRequest.coffee index de3fba7a6b..d993763d27 100644 --- a/services/web/test/unit/coffee/helpers/MockRequest.coffee +++ b/services/web/test/unit/coffee/helpers/MockRequest.coffee @@ -5,6 +5,7 @@ class MockRequest params: {} query: {} + body: {} _parsedUrl:{} i18n: translate:-> From 97c145433e140a24d26ca28218480759cdcdf27f Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 11 Jun 2018 12:29:08 +0200 Subject: [PATCH 2/2] use EmailHelper.parseEmail on registration Also changed EmailHelper to use the regexp already used in UserRegistrationHandler rather than the `mimelib` package as it is deprecated. --- .../coffee/Features/Helpers/EmailHelper.coffee | 15 ++++++++------- .../Features/User/UserRegistrationHandler.coffee | 12 +++--------- services/web/package.json | 1 - .../User/UserRegistrationHandlerTests.coffee | 2 ++ 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/services/web/app/coffee/Features/Helpers/EmailHelper.coffee b/services/web/app/coffee/Features/Helpers/EmailHelper.coffee index 7bc93888fd..d45447788d 100644 --- a/services/web/app/coffee/Features/Helpers/EmailHelper.coffee +++ b/services/web/app/coffee/Features/Helpers/EmailHelper.coffee @@ -1,11 +1,12 @@ -mimelib = require("mimelib") - +EMAIL_REGEXP = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\ ".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA -Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ module.exports = EmailHelper = parseEmail: (email) -> - email = mimelib.parseAddresses(email or "")[0]?.address?.toLowerCase() - if !email? or email == "" - return null - else - return email + return null unless email? + email = email.trim().toLowerCase() + + matched = email.match EMAIL_REGEXP + return null unless matched? && matched[0]? + + matched[0] diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 4a42d7f5fa..85cdabdbdf 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -11,12 +11,9 @@ EmailHandler = require("../Email/EmailHandler") OneTimeTokenHandler = require "../Security/OneTimeTokenHandler" Analytics = require "../Analytics/AnalyticsManager" settings = require "settings-sharelatex" +EmailHelper = require("../Helpers/EmailHelper") module.exports = UserRegistrationHandler = - validateEmail : (email) -> - re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\ ".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA -Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ - return re.test(email) - hasZeroLengths : (props) -> hasZeroLength = false props.forEach (prop) -> @@ -25,13 +22,10 @@ module.exports = UserRegistrationHandler = return hasZeroLength _registrationRequestIsValid : (body, callback)-> - email = sanitize.escape(body.email).trim().toLowerCase() + email = EmailHelper.parseEmail(body.email) or '' password = body.password - username = email.match(/^[^@]*/) if @hasZeroLengths([password, email]) return false - else if !@validateEmail(email) - return false else return true @@ -47,7 +41,7 @@ module.exports = UserRegistrationHandler = requestIsValid = @_registrationRequestIsValid userDetails if !requestIsValid return callback(new Error("request is not valid")) - userDetails.email = userDetails.email?.trim()?.toLowerCase() + userDetails.email = EmailHelper.parseEmail(userDetails.email) UserGetter.getUserByAnyEmail userDetails.email, (err, user) => if err? return callback err diff --git a/services/web/package.json b/services/web/package.json index 418fa34a08..ed0eb06455 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -62,7 +62,6 @@ "marked": "^0.3.5", "method-override": "^2.3.3", "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1", - "mimelib": "0.2.14", "mocha": "^5.0.1", "mongojs": "2.4.0", "mongoose": "4.11.4", diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee index e738947171..3a8801bc08 100644 --- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee @@ -5,6 +5,7 @@ path = require('path') modulePath = path.join __dirname, '../../../../app/js/Features/User/UserRegistrationHandler' sinon = require("sinon") expect = require("chai").expect +EmailHelper = require '../../../../app/js/Features/Helpers/EmailHelper' describe "UserRegistrationHandler", -> @@ -37,6 +38,7 @@ describe "UserRegistrationHandler", -> "../Security/OneTimeTokenHandler": @OneTimeTokenHandler "../Analytics/AnalyticsManager": @AnalyticsManager = { recordEvent: sinon.stub() } "settings-sharelatex": @settings = {siteUrl: "http://sl.example.com"} + "../Helpers/EmailHelper": EmailHelper @passingRequest = {email:"something@email.com", password:"123"}