diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 25b3f3e06d..f33f3e523e 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -68,6 +68,20 @@ V1ConnectionError = (message) -> return error V1ConnectionError.prototype.__proto___ = Error.prototype +UnconfirmedEmailError = (message) -> + error = new Error(message) + error.name = "UnconfirmedEmailError" + error.__proto__ = UnconfirmedEmailError.prototype + return error +UnconfirmedEmailError.prototype.__proto___ = Error.prototype + +EmailExistsError = (message) -> + error = new Error(message) + error.name = "EmailExistsError" + error.__proto__ = EmailExistsError.prototype + return error +EmailExistsError.prototype.__proto___ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError ServiceNotConfiguredError: ServiceNotConfiguredError @@ -79,3 +93,5 @@ module.exports = Errors = V1HistoryNotSyncedError: V1HistoryNotSyncedError ProjectHistoryDisabledError: ProjectHistoryDisabledError V1ConnectionError: V1ConnectionError + UnconfirmedEmailError: UnconfirmedEmailError + EmailExistsError: EmailExistsError diff --git a/services/web/app/coffee/Features/User/UserAffiliationsManager.coffee b/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee similarity index 82% rename from services/web/app/coffee/Features/User/UserAffiliationsManager.coffee rename to services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee index 77dad7a8ef..8ce39d68e3 100644 --- a/services/web/app/coffee/Features/User/UserAffiliationsManager.coffee +++ b/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee @@ -3,12 +3,20 @@ metrics = require("metrics-sharelatex") settings = require "settings-sharelatex" request = require "request" -module.exports = UserAffiliationsManager = - getAffiliations: (userId, callback = (error, body) ->) -> +module.exports = InstitutionsAPI = + getInstitutionAffiliations: (institutionId, callback = (error, body) ->) -> + makeAffiliationRequest { + method: 'GET' + path: "/api/v2/institutions/#{institutionId.toString()}/affiliations" + defaultErrorMessage: "Couldn't get institution affiliations" + }, callback + + + getUserAffiliations: (userId, callback = (error, body) ->) -> makeAffiliationRequest { method: 'GET' path: "/api/v2/users/#{userId.toString()}/affiliations" - defaultErrorMessage: "Couldn't get affiliations" + defaultErrorMessage: "Couldn't get user affiliations" }, callback @@ -77,10 +85,11 @@ makeAffiliationRequest = (requestOptions, callback = (error) ->) -> callback(null, body) [ - 'getAffiliations', + 'getInstitutionAffiliations' + 'getUserAffiliations', 'addAffiliation', 'removeAffiliation', ].map (method) -> metrics.timeAsyncMethod( - UserAffiliationsManager, method, 'mongo.UserAffiliationsManager', logger + InstitutionsAPI, method, 'mongo.InstitutionsAPI', logger ) diff --git a/services/web/app/coffee/Features/Institutions/InstitutionsManager.coffee b/services/web/app/coffee/Features/Institutions/InstitutionsManager.coffee new file mode 100644 index 0000000000..15233c8849 --- /dev/null +++ b/services/web/app/coffee/Features/Institutions/InstitutionsManager.coffee @@ -0,0 +1,17 @@ +logger = require 'logger-sharelatex' +async = require 'async' +db = require("../../infrastructure/mongojs").db +ObjectId = require("../../infrastructure/mongojs").ObjectId +{ getInstitutionAffiliations } = require('./InstitutionsAPI') +FeaturesUpdater = require('../Subscription/FeaturesUpdater') + +ASYNC_LIMIT = 10 +module.exports = InstitutionsManager = + upgradeInstitutionUsers: (institutionId, callback = (error) ->) -> + getInstitutionAffiliations institutionId, (error, affiliations) -> + return callback(error) if error + async.eachLimit affiliations, ASYNC_LIMIT, refreshFeatures, callback + +refreshFeatures = (affiliation, callback) -> + userId = ObjectId(affiliation.user_id) + FeaturesUpdater.refreshFeatures(userId, true, callback) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 9b102d13dd..823919f999 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -13,6 +13,7 @@ UserSessionsManager = require("./UserSessionsManager") UserUpdater = require("./UserUpdater") SudoModeHandler = require('../SudoMode/SudoModeHandler') settings = require "settings-sharelatex" +Errors = require "../Errors/Errors" module.exports = UserController = @@ -100,7 +101,7 @@ module.exports = UserController = UserUpdater.changeEmailAddress user_id, newEmail, (err)-> if err? logger.err err:err, user_id:user_id, newEmail:newEmail, "problem updaing users email address" - if err.message == "alread_exists" + if err instanceof Errors.EmailExistsError message = req.i18n.translate("email_already_registered") else message = req.i18n.translate("problem_changing_email_address") diff --git a/services/web/app/coffee/Features/User/UserCreator.coffee b/services/web/app/coffee/Features/User/UserCreator.coffee index b0676e09e0..2dfa7f5ac0 100644 --- a/services/web/app/coffee/Features/User/UserCreator.coffee +++ b/services/web/app/coffee/Features/User/UserCreator.coffee @@ -1,7 +1,7 @@ User = require("../../models/User").User logger = require("logger-sharelatex") metrics = require('metrics-sharelatex') -{ addAffiliation } = require("./UserAffiliationsManager") +{ addAffiliation } = require("../Institutions/InstitutionsAPI") module.exports = UserCreator = diff --git a/services/web/app/coffee/Features/User/UserDeleter.coffee b/services/web/app/coffee/Features/User/UserDeleter.coffee index c09bebbd8c..149959c00f 100644 --- a/services/web/app/coffee/Features/User/UserDeleter.coffee +++ b/services/web/app/coffee/Features/User/UserDeleter.coffee @@ -4,7 +4,7 @@ ProjectDeleter = require("../Project/ProjectDeleter") logger = require("logger-sharelatex") SubscriptionHandler = require("../Subscription/SubscriptionHandler") async = require("async") -{ deleteAffiliations } = require("./UserAffiliationsManager") +{ deleteAffiliations } = require("../Institutions/InstitutionsAPI") module.exports = diff --git a/services/web/app/coffee/Features/User/UserEmailsController.coffee b/services/web/app/coffee/Features/User/UserEmailsController.coffee index 1afb60b99b..e37a0452e7 100644 --- a/services/web/app/coffee/Features/User/UserEmailsController.coffee +++ b/services/web/app/coffee/Features/User/UserEmailsController.coffee @@ -3,7 +3,7 @@ UserGetter = require("./UserGetter") UserUpdater = require("./UserUpdater") EmailHelper = require("../Helpers/EmailHelper") UserEmailsConfirmationHandler = require "./UserEmailsConfirmationHandler" -{ endorseAffiliation } = require("./UserAffiliationsManager") +{ endorseAffiliation } = require("../Institutions/InstitutionsAPI") logger = require("logger-sharelatex") Errors = require "../Errors/Errors" @@ -26,7 +26,8 @@ module.exports = UserEmailsController = role: req.body.role department: req.body.department UserUpdater.addEmailAddress userId, email, affiliationOptions, (error)-> - return next(error) if error? + if error? + return UserEmailsController._handleEmailError error, req, res, next UserEmailsConfirmationHandler.sendConfirmationEmail userId, email, (err) -> return next(error) if error? res.sendStatus 204 @@ -47,9 +48,11 @@ module.exports = UserEmailsController = email = EmailHelper.parseEmail(req.body.email) return res.sendStatus 422 unless email? - UserUpdater.setDefaultEmailAddress userId, email, (error)-> - return next(error) if error? - res.sendStatus 200 + UserUpdater.updateV1AndSetDefaultEmailAddress userId, email, (error)-> + if error? + return UserEmailsController._handleEmailError error, req, res, next + else + return res.sendStatus 200 endorse: (req, res, next) -> @@ -95,3 +98,15 @@ module.exports = UserEmailsController = next(error) else res.sendStatus 200 + + _handleEmailError: (error, req, res, next) -> + if error instanceof Errors.UnconfirmedEmailError + return res.status(409).json { + message: 'email must be confirmed' + } + else if error instanceof Errors.EmailExistsError + return res.status(409).json { + message: req.i18n.translate("email_already_registered") + } + else + return next(error) \ No newline at end of file diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index bdcdd24482..f3aa4ad4d6 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/services/web/app/coffee/Features/User/UserGetter.coffee @@ -3,7 +3,8 @@ metrics = require('metrics-sharelatex') logger = require('logger-sharelatex') db = mongojs.db ObjectId = mongojs.ObjectId -{ getAffiliations } = require("./UserAffiliationsManager") +{ getUserAffiliations } = require("../Institutions/InstitutionsAPI") +Errors = require("../Errors/Errors") module.exports = UserGetter = getUser: (query, projection, callback = (error, user) ->) -> @@ -31,7 +32,7 @@ module.exports = UserGetter = return callback error if error? return callback new Error('User not Found') unless user - getAffiliations userId, (error, affiliationsData) -> + getUserAffiliations userId, (error, affiliationsData) -> return callback error if error? callback null, decorateFullEmails(user.email, user.emails, affiliationsData) @@ -77,7 +78,7 @@ module.exports = UserGetter = # check for duplicate email address. This is also enforced at the DB level ensureUniqueEmailAddress: (newEmail, callback) -> @getUserByAnyEmail newEmail, (error, user) -> - return callback(message: 'alread_exists') if user? + return callback(new Errors.EmailExistsError('alread_exists')) if user? callback(error) decorateFullEmails = (defaultEmail, emailsData, affiliationsData) -> diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee index 5e9e3d906d..8d5e3658f9 100644 --- a/services/web/app/coffee/Features/User/UserUpdater.coffee +++ b/services/web/app/coffee/Features/User/UserUpdater.coffee @@ -5,10 +5,12 @@ db = mongojs.db async = require("async") ObjectId = mongojs.ObjectId UserGetter = require("./UserGetter") -{ addAffiliation, removeAffiliation } = require("./UserAffiliationsManager") +{ addAffiliation, removeAffiliation } = require("../Institutions/InstitutionsAPI") FeaturesUpdater = require("../Subscription/FeaturesUpdater") EmailHelper = require "../Helpers/EmailHelper" Errors = require "../Errors/Errors" +Settings = require "settings-sharelatex" +request = require 'request' module.exports = UserUpdater = updateUser: (query, update, callback = (error) ->) -> @@ -107,6 +109,49 @@ module.exports = UserUpdater = return callback(new Error('Default email does not belong to user')) callback() + updateV1AndSetDefaultEmailAddress: (userId, email, callback) -> + @updateEmailAddressInV1 userId, email, (error) => + return callback(error) if error? + @setDefaultEmailAddress userId, email, callback + + updateEmailAddressInV1: (userId, newEmail, callback) -> + if !Settings.apis?.v1?.url? + return callback() + UserGetter.getUser userId, { 'overleaf.id': 1, emails: 1 }, (error, user) -> + return callback(error) if error? + return callback(new Errors.NotFoundError('no user found')) if !user? + if !user.overleaf?.id? + return callback() + newEmailIsConfirmed = false + for email in user.emails + if email.email == newEmail and email.confirmedAt? + newEmailIsConfirmed = true + break + if !newEmailIsConfirmed + return callback(new Errors.UnconfirmedEmailError("can't update v1 with unconfirmed email")) + request { + baseUrl: Settings.apis.v1.url + url: "/api/v1/sharelatex/users/#{user.overleaf.id}/email" + method: 'PUT' + auth: + user: Settings.apis.v1.user + pass: Settings.apis.v1.pass + sendImmediately: true + json: + user: + email: newEmail + timeout: 5 * 1000 + }, (error, response, body) -> + if error? + error = new Errors.V1ConnectionError('No V1 connection') if error.code == 'ECONNREFUSED' + return callback(error) + if response.statusCode == 409 # Conflict + return callback(new Errors.EmailExistsError('email exists in v1')) + else if 200 <= response.statusCode < 300 + return callback() + else + return callback new Error("non-success code from v1: #{response.statusCode}") + confirmEmail: (userId, email, callback) -> email = EmailHelper.parseEmail(email) return callback(new Error('invalid email')) if !email? diff --git a/services/web/app/views/user/settings/user-affiliations.pug b/services/web/app/views/user/settings/user-affiliations.pug index 7333a67a08..32b7d8fdf3 100644 --- a/services/web/app/views/user/settings/user-affiliations.pug +++ b/services/web/app/views/user/settings/user-affiliations.pug @@ -11,9 +11,7 @@ form.row( th.affiliations-table-email #{translate("email")} th.affiliations-table-institution #{translate("institution_and_role")} th.affiliations-table-inline-actions - tbody( - ng-if="!ui.hasError" - ) + tbody tr( ng-repeat="userEmail in userEmails" ) @@ -59,7 +57,7 @@ form.row( .affiliation-change-actions.small a( href - ng-click="saveAffiliationChange();" + ng-click="saveAffiliationChange(userEmail);" ) #{translate("save_or_cancel-save")} |  #{translate("save_or_cancel-or" )}  a( @@ -71,30 +69,26 @@ form.row( // so create a wrapper for the tooltip div.affiliations-table-inline-action-disabled-wrapper( tooltip=translate("please_confirm_your_email_before_making_it_default") - ng-if="!userEmail.default && !userEmail.confirmedAt" + tooltip-enable="!ui.isMakingRequest" + ng-if="!userEmail.default && (!userEmail.confirmedAt || ui.isMakingRequest)" ) button.btn.btn-sm.btn-success.affiliations-table-inline-action( disabled ) #{translate("make_default")} button.btn.btn-sm.btn-success.affiliations-table-inline-action( - ng-if="!userEmail.default && userEmail.confirmedAt" + ng-if="!userEmail.default && (userEmail.confirmedAt && !ui.isMakingRequest)" ng-click="setDefaultUserEmail(userEmail)" ) #{translate("make_default")} |   button.btn.btn-sm.btn-danger.affiliations-table-inline-action( ng-if="!userEmail.default" ng-click="removeUserEmail(userEmail)" + ng-disabled="ui.isMakingRequest" tooltip=translate("remove") ) i.fa.fa-fw.fa-trash tr.affiliations-table-highlighted-row( - ng-if="ui.isLoadingEmails" - ) - td.text-center(colspan="3") - i.fa.fa-fw.fa-spin.fa-refresh - |  #{translate("loading")}... - tr.affiliations-table-highlighted-row( - ng-if="!ui.showAddEmailUI && !ui.isLoadingEmails" + ng-if="!ui.showAddEmailUI && !ui.isMakingRequest" ) td(colspan="3") a( @@ -103,7 +97,7 @@ form.row( ) #{translate("add_another_email")} tr.affiliations-table-highlighted-row( - ng-if="ui.showAddEmailUI" + ng-if="ui.showAddEmailUI && !ui.isLoadingEmails" ) td .affiliations-form-group @@ -149,31 +143,30 @@ form.row( ) td button.btn.btn-sm.btn-primary( - ng-disabled="affiliationsForm.$invalid || ui.isAddingNewEmail" + ng-disabled="affiliationsForm.$invalid || ui.isMakingRequest" ng-click="addNewEmail()" ) - span( - ng-if="!ui.isAddingNewEmail" - ) #{translate("add_new_email")} - span( - ng-if="ui.isAddingNewEmail" - ) - i.fa.fa-fw.fa-spin.fa-refresh - |  #{translate("adding")}... - tbody( - ng-if="ui.hasError" - ) + | #{translate("add_new_email")} + tr.affiliations-table-highlighted-row( + ng-if="ui.isMakingRequest" + ) + td.text-center(colspan="3", ng-if="ui.isLoadingEmails") + i.fa.fa-fw.fa-spin.fa-refresh + |  #{translate("loading")}... + td.text-center(colspan="3", ng-if="ui.isResendingConfirmation") + i.fa.fa-fw.fa-spin.fa-refresh + |  #{translate("sending")}... + td.text-center(colspan="3", ng-if="!ui.isLoadingEmails && !ui.isResendingConfirmation") + i.fa.fa-fw.fa-spin.fa-refresh + |  #{translate("saving")}... tr.affiliations-table-error-row( - ng-if="true" + ng-if="ui.hasError" ) td.text-center(colspan="3") div i.fa.fa-fw.fa-exclamation-triangle - |  #{translate("error_performing_request")} - div - button.btn.btn-xs( - ng-click="acknowledgeError();" - ) #{translate("reload_emails_and_affiliations")} + span(ng-if="!ui.errorMessage")  #{translate("error_performing_request")} + span(ng-if="ui.errorMessage")  {{ui.errorMessage}} hr diff --git a/services/web/public/coffee/main/affiliations/controllers/UserAffiliationsController.coffee b/services/web/public/coffee/main/affiliations/controllers/UserAffiliationsController.coffee index 66162eda86..14ea1a7d29 100644 --- a/services/web/public/coffee/main/affiliations/controllers/UserAffiliationsController.coffee +++ b/services/web/public/coffee/main/affiliations/controllers/UserAffiliationsController.coffee @@ -55,25 +55,23 @@ define [ $scope.affiliationToChange.role = userEmail.affiliation.role $scope.affiliationToChange.department = userEmail.affiliation.department - $scope.saveAffiliationChange = () -> - $scope.ui.isLoadingEmails = true - UserAffiliationsDataService - .addRoleAndDepartment( - $scope.affiliationToChange.email, - $scope.affiliationToChange.role, - $scope.affiliationToChange.department - ) + $scope.saveAffiliationChange = (userEmail) -> + userEmail.affiliation.role = $scope.affiliationToChange.role + userEmail.affiliation.department = $scope.affiliationToChange.department + _resetAffiliationToChange() + _monitorRequest( + UserAffiliationsDataService + .addRoleAndDepartment( + userEmail.email, + userEmail.affiliation.role, + userEmail.affiliation.department + ) + ) .then () -> - _reset() - _getUserEmails() - .catch () -> - $scope.ui.hasError = true + setTimeout () -> _getUserEmails() $scope.cancelAffiliationChange = (email) -> - $scope.affiliationToChange.email = "" - $scope.affiliationToChange.university = null - $scope.affiliationToChange.role = null - $scope.affiliationToChange.department = null + _resetAffiliationToChange() $scope.isChangingAffiliation = (email) -> $scope.affiliationToChange.email == email @@ -82,7 +80,6 @@ define [ $scope.ui.showAddEmailUI = true $scope.addNewEmail = () -> - $scope.ui.isAddingNewEmail = true if !$scope.newAffiliation.university? addEmailPromise = UserAffiliationsDataService .addUserEmail $scope.newAffiliation.email @@ -104,75 +101,104 @@ define [ $scope.newAffiliation.role, $scope.newAffiliation.department ) - addEmailPromise - .then () -> - _reset() - _getUserEmails() - .catch () -> - $scope.ui.hasError = true + + $scope.ui.isAddingNewEmail = true + $scope.ui.showAddEmailUI = false + _monitorRequest(addEmailPromise) + .then () -> + _resetNewAffiliation() + _resetAddingEmail() + setTimeout () -> _getUserEmails() + .finally () -> + $scope.ui.isAddingNewEmail = false $scope.setDefaultUserEmail = (userEmail) -> - $scope.ui.isLoadingEmails = true - UserAffiliationsDataService - .setDefaultUserEmail userEmail.email - .then () -> _getUserEmails() - .catch () -> $scope.ui.hasError = true + _monitorRequest( + UserAffiliationsDataService + .setDefaultUserEmail userEmail.email + ) + .then () -> + for email in $scope.userEmails or [] + email.default = false + userEmail.default = true $scope.removeUserEmail = (userEmail) -> - $scope.ui.isLoadingEmails = true - userEmailIdx = _.indexOf $scope.userEmails, userEmail - if userEmailIdx > -1 - $scope.userEmails.splice userEmailIdx, 1 - UserAffiliationsDataService - .removeUserEmail userEmail.email - .then () -> _getUserEmails() - .catch () -> $scope.ui.hasError = true + $scope.userEmails = $scope.userEmails.filter (ue) -> ue != userEmail + _monitorRequest( + UserAffiliationsDataService + .removeUserEmail userEmail.email + ) $scope.resendConfirmationEmail = (userEmail) -> - $scope.ui.isLoadingEmails = true - UserAffiliationsDataService - .resendConfirmationEmail userEmail.email - .then () -> _getUserEmails() - .catch () -> $scope.ui.hasError = true + $scope.ui.isResendingConfirmation = true + _monitorRequest( + UserAffiliationsDataService + .resendConfirmationEmail userEmail.email + ) + .finally () -> + $scope.ui.isResendingConfirmation = false $scope.acknowledgeError = () -> _reset() _getUserEmails() - _reset = () -> + _resetAffiliationToChange = () -> + $scope.affiliationToChange = + email: "" + university: null + role: null + department: null + + _resetNewAffiliation = () -> $scope.newAffiliation = email: "" country: null university: null role: null department: null + + _resetAddingEmail = () -> + $scope.ui.showAddEmailUI = false + $scope.ui.isValidEmail = false + $scope.ui.isBlacklistedEmail = false + $scope.ui.showManualUniversitySelectionUI = false + + _reset = () -> $scope.ui = hasError: false + errorMessage: "" showChangeAffiliationUI: false - showManualUniversitySelectionUI: false + isMakingRequest: false isLoadingEmails: false isAddingNewEmail: false - showAddEmailUI: false - isValidEmail: false - isBlacklistedEmail: false - $scope.affiliationToChange = - email: "" - university: null - role: null - department: null + isResendingConfirmation: false + _resetAffiliationToChange() + _resetNewAffiliation() + _resetAddingEmail() _reset() + _monitorRequest = (promise) -> + $scope.ui.hasError = false + $scope.ui.isMakingRequest = true + promise + .catch (response) -> + $scope.ui.hasError = true + $scope.ui.errorMessage = response?.data?.message + .finally () -> + $scope.ui.isMakingRequest = false + return promise + # Populates the emails table _getUserEmails = () -> $scope.ui.isLoadingEmails = true - UserAffiliationsDataService - .getUserEmails() + _monitorRequest( + UserAffiliationsDataService + .getUserEmails() + ) .then (emails) -> $scope.userEmails = emails + .finally () -> $scope.ui.isLoadingEmails = false - .catch () -> - $scope.ui.hasError = true - _getUserEmails() ] \ No newline at end of file diff --git a/services/web/scripts/upgrade_institution_users.js b/services/web/scripts/upgrade_institution_users.js new file mode 100644 index 0000000000..8cf098ca2b --- /dev/null +++ b/services/web/scripts/upgrade_institution_users.js @@ -0,0 +1,16 @@ +const InstitutionsManager = require( + '../app/js/Features/Institutions/InstitutionsManager' +) + +const institutionId = parseInt(process.argv[2]) +if (isNaN(institutionId)) throw new Error('No institution id') +console.log('Upgrading users of institution', institutionId) + +InstitutionsManager.upgradeInstitutionUsers(institutionId, function (error) { + if (error) { + console.log(error) + } else { + console.log('DONE 👌') + } + process.exit() +}) diff --git a/services/web/test/acceptance/coffee/UserEmailsTests.coffee b/services/web/test/acceptance/coffee/UserEmailsTests.coffee index 71837f4633..3e8ffbd777 100644 --- a/services/web/test/acceptance/coffee/UserEmailsTests.coffee +++ b/services/web/test/acceptance/coffee/UserEmailsTests.coffee @@ -302,3 +302,181 @@ describe "UserEmails", -> expect(tokens.length).to.equal 0 cb() ], done + + describe 'setting a default email', -> + it 'should update confirmed emails for users not in v1', (done) -> + token = null + async.series [ + (cb) => + @user.request { + method: 'POST', + url: '/user/emails', + json: + email: 'new-confirmed-default@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 204 + cb() + (cb) => + # Mark the email as confirmed + db.users.update { + 'emails.email': 'new-confirmed-default@example.com' + }, { + $set: { + 'emails.$.confirmedAt': new Date() + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails/default', + json: + email: 'new-confirmed-default@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 200 + cb() + (cb) => + @user.request { url: '/user/emails', json: true }, (error, response, body) -> + expect(response.statusCode).to.equal 200 + expect(body[0].confirmedAt).to.not.exist + expect(body[0].default).to.equal false + expect(body[1].confirmedAt).to.exist + expect(body[1].default).to.equal true + cb() + ], done + + it 'should not allow changing unconfirmed emails in v1', (done) -> + token = null + async.series [ + (cb) => + db.users.update { + _id: ObjectId(@user._id) + }, { + $set: { + 'overleaf.id': 42 + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails', + json: + email: 'new-unconfirmed-default@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 204 + cb() + (cb) => + @user.request { + method: 'POST', + url: '/user/emails/default', + json: + email: 'new-unconfirmed-default@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 409 + cb() + (cb) => + @user.request { url: '/user/emails', json: true }, (error, response, body) -> + expect(body[0].default).to.equal true + expect(body[1].default).to.equal false + cb() + ], done + + it 'should update the email in v1 if confirmed', (done) -> + token = null + async.series [ + (cb) => + db.users.update { + _id: ObjectId(@user._id) + }, { + $set: { + 'overleaf.id': 42 + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails', + json: + email: 'new-confirmed-default-in-v1@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 204 + cb() + (cb) => + # Mark the email as confirmed + db.users.update { + 'emails.email': 'new-confirmed-default-in-v1@example.com' + }, { + $set: { + 'emails.$.confirmedAt': new Date() + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails/default', + json: + email: 'new-confirmed-default-in-v1@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 200 + cb() + ], (error) => + return done(error) if error? + expect( + MockV1Api.updateEmail.calledWith(42, 'new-confirmed-default-in-v1@example.com') + ).to.equal true + done() + + it 'should return an error if the email exists in v1', (done) -> + MockV1Api.existingEmails.push 'exists-in-v1@example.com' + async.series [ + (cb) => + db.users.update { + _id: ObjectId(@user._id) + }, { + $set: { + 'overleaf.id': 42 + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails', + json: + email: 'exists-in-v1@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 204 + cb() + (cb) => + # Mark the email as confirmed + db.users.update { + 'emails.email': 'exists-in-v1@example.com' + }, { + $set: { + 'emails.$.confirmedAt': new Date() + } + }, cb + (cb) => + @user.request { + method: 'POST', + url: '/user/emails/default', + json: + email: 'exists-in-v1@example.com' + }, (error, response, body) => + return done(error) if error? + expect(response.statusCode).to.equal 409 + expect(body).to.deep.equal { + message: "This email is already registered" + } + cb() + (cb) => + @user.request { url: '/user/emails', json: true }, (error, response, body) -> + expect(body[0].default).to.equal true + expect(body[1].default).to.equal false + cb() + ], done \ No newline at end of file diff --git a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee index 7599230b09..eeafa6a44b 100644 --- a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee @@ -28,6 +28,10 @@ module.exports = MockV1Api = affiliations: [] + updateEmail: sinon.stub() + + existingEmails: [] + setAffiliations: (affiliations) -> @affiliations = affiliations run: () -> @@ -64,6 +68,14 @@ module.exports = MockV1Api = app.get '/university/domains', (req, res, next) -> res.json [] + app.put '/api/v1/sharelatex/users/:id/email', (req, res, next) => + { email } = req.body?.user + if email in @existingEmails + return res.sendStatus 409 + else + @updateEmail parseInt(req.params.id), email + return res.sendStatus 200 + app.listen 5000, (error) -> throw error if error? .on "error", (error) -> diff --git a/services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee b/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee similarity index 73% rename from services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee rename to services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee index ff3d96a283..7500e46e40 100644 --- a/services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee +++ b/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee @@ -4,16 +4,16 @@ SandboxedModule = require('sandboxed-module') assert = require('assert') path = require('path') sinon = require('sinon') -modulePath = path.join __dirname, "../../../../app/js/Features/User/UserAffiliationsManager" +modulePath = path.join __dirname, "../../../../app/js/Features/Institutions/InstitutionsAPI" expect = require("chai").expect -describe "UserAffiliationsManager", -> +describe "InstitutionsAPI", -> beforeEach -> @logger = err: sinon.stub(), log: -> settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } @request = sinon.stub() - @UserAffiliationsManager = SandboxedModule.require modulePath, requires: + @InstitutionsAPI = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger "metrics-sharelatex": timeAsyncMethod: sinon.stub() 'settings-sharelatex': settings @@ -25,11 +25,27 @@ describe "UserAffiliationsManager", -> email:"hello@world.com" @newEmail = "bob@bob.com" - describe 'getAffiliations', -> + describe 'getInstitutionAffiliations', -> + it 'get affiliations', (done)-> + @institutionId = 123 + responseBody = ['123abc', '456def'] + @request.yields(null, { statusCode: 200 }, responseBody) + @InstitutionsAPI.getInstitutionAffiliations @institutionId, (err, body) => + should.not.exist(err) + @request.calledOnce.should.equal true + requestOptions = @request.lastCall.args[0] + expectedUrl = "v1.url/api/v2/institutions/#{@institutionId}/affiliations" + requestOptions.url.should.equal expectedUrl + requestOptions.method.should.equal 'GET' + should.not.exist(requestOptions.body) + body.should.equal responseBody + done() + + describe 'getUserAffiliations', -> it 'get affiliations', (done)-> responseBody = [{ foo: 'bar' }] @request.callsArgWith(1, null, { statusCode: 201 }, responseBody) - @UserAffiliationsManager.getAffiliations @stubbedUser._id, (err, body) => + @InstitutionsAPI.getUserAffiliations @stubbedUser._id, (err, body) => should.not.exist(err) @request.calledOnce.should.equal true requestOptions = @request.lastCall.args[0] @@ -43,7 +59,7 @@ describe "UserAffiliationsManager", -> it 'handle error', (done)-> body = errors: 'affiliation error message' @request.callsArgWith(1, null, { statusCode: 503 }, body) - @UserAffiliationsManager.getAffiliations @stubbedUser._id, (err) => + @InstitutionsAPI.getUserAffiliations @stubbedUser._id, (err) => should.exist(err) err.message.should.have.string 503 err.message.should.have.string body.errors @@ -58,7 +74,7 @@ describe "UserAffiliationsManager", -> university: { id: 1 } role: 'Prof' department: 'Math' - @UserAffiliationsManager.addAffiliation @stubbedUser._id, @newEmail, affiliationOptions, (err)=> + @InstitutionsAPI.addAffiliation @stubbedUser._id, @newEmail, affiliationOptions, (err)=> should.not.exist(err) @request.calledOnce.should.equal true requestOptions = @request.lastCall.args[0] @@ -77,7 +93,7 @@ describe "UserAffiliationsManager", -> it 'handle error', (done)-> body = errors: 'affiliation error message' @request.callsArgWith(1, null, { statusCode: 422 }, body) - @UserAffiliationsManager.addAffiliation @stubbedUser._id, @newEmail, {}, (err)=> + @InstitutionsAPI.addAffiliation @stubbedUser._id, @newEmail, {}, (err)=> should.exist(err) err.message.should.have.string 422 err.message.should.have.string body.errors @@ -88,7 +104,7 @@ describe "UserAffiliationsManager", -> @request.callsArgWith(1, null, { statusCode: 404 }) it 'remove affiliation', (done)-> - @UserAffiliationsManager.removeAffiliation @stubbedUser._id, @newEmail, (err)=> + @InstitutionsAPI.removeAffiliation @stubbedUser._id, @newEmail, (err)=> should.not.exist(err) @request.calledOnce.should.equal true requestOptions = @request.lastCall.args[0] @@ -100,7 +116,7 @@ describe "UserAffiliationsManager", -> it 'handle error', (done)-> @request.callsArgWith(1, null, { statusCode: 500 }) - @UserAffiliationsManager.removeAffiliation @stubbedUser._id, @newEmail, (err)=> + @InstitutionsAPI.removeAffiliation @stubbedUser._id, @newEmail, (err)=> should.exist(err) err.message.should.exist done() @@ -108,7 +124,7 @@ describe "UserAffiliationsManager", -> describe 'deleteAffiliations', -> it 'delete affiliations', (done)-> @request.callsArgWith(1, null, { statusCode: 200 }) - @UserAffiliationsManager.deleteAffiliations @stubbedUser._id, (err) => + @InstitutionsAPI.deleteAffiliations @stubbedUser._id, (err) => should.not.exist(err) @request.calledOnce.should.equal true requestOptions = @request.lastCall.args[0] @@ -120,7 +136,7 @@ describe "UserAffiliationsManager", -> it 'handle error', (done)-> body = errors: 'affiliation error message' @request.callsArgWith(1, null, { statusCode: 518 }, body) - @UserAffiliationsManager.deleteAffiliations @stubbedUser._id, (err) => + @InstitutionsAPI.deleteAffiliations @stubbedUser._id, (err) => should.exist(err) err.message.should.have.string 518 err.message.should.have.string body.errors @@ -131,7 +147,7 @@ describe "UserAffiliationsManager", -> @request.callsArgWith(1, null, { statusCode: 204 }) it 'endorse affiliation', (done)-> - @UserAffiliationsManager.endorseAffiliation @stubbedUser._id, @newEmail, 'Student','Physics', (err)=> + @InstitutionsAPI.endorseAffiliation @stubbedUser._id, @newEmail, 'Student','Physics', (err)=> should.not.exist(err) @request.calledOnce.should.equal true requestOptions = @request.lastCall.args[0] diff --git a/services/web/test/unit/coffee/Institutions/InstitutionsManagerTests.coffee b/services/web/test/unit/coffee/Institutions/InstitutionsManagerTests.coffee new file mode 100644 index 0000000000..acf0478ae2 --- /dev/null +++ b/services/web/test/unit/coffee/Institutions/InstitutionsManagerTests.coffee @@ -0,0 +1,30 @@ +should = require('chai').should() +SandboxedModule = require('sandboxed-module') +path = require('path') +sinon = require('sinon') +modulePath = path.join __dirname, "../../../../app/js/Features/Institutions/InstitutionsManager" + +describe "InstitutionsManager", -> + beforeEach -> + @institutionId = 123 + @logger = log: -> + @getInstitutionAffiliations = sinon.stub() + @refreshFeatures = sinon.stub().yields() + @InstitutionsManager = SandboxedModule.require modulePath, requires: + 'logger-sharelatex': @logger + './InstitutionsAPI': + getInstitutionAffiliations: @getInstitutionAffiliations + '../Subscription/FeaturesUpdater': + refreshFeatures: @refreshFeatures + + describe 'upgradeInstitutionUsers', -> + it 'refresh all users Features', (done) -> + affiliations = [ + { user_id: '123abc123abc123abc123abc' } + { user_id: '456def456def456def456def' } + ] + @getInstitutionAffiliations.yields(null, affiliations) + @InstitutionsManager.upgradeInstitutionUsers @institutionId, (error) => + should.not.exist(error) + sinon.assert.calledTwice(@refreshFeatures) + done() diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index 26f345db39..d589852a37 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserControllerTests.coffee @@ -9,6 +9,7 @@ MockResponse = require "../helpers/MockResponse" MockRequest = require "../helpers/MockRequest" ObjectId = require("mongojs").ObjectId assert = require("assert") +Errors = require "../../../../app/js/Features/Errors/Errors" describe "UserController", -> beforeEach -> @@ -81,6 +82,7 @@ describe "UserController", -> log:-> err:-> "metrics-sharelatex": inc:-> + "../Errors/Errors": Errors @res = send: sinon.stub() diff --git a/services/web/test/unit/coffee/User/UserCreatorTests.coffee b/services/web/test/unit/coffee/User/UserCreatorTests.coffee index 1945453b9e..f9d88e4cf8 100644 --- a/services/web/test/unit/coffee/User/UserCreatorTests.coffee +++ b/services/web/test/unit/coffee/User/UserCreatorTests.coffee @@ -22,7 +22,7 @@ describe "UserCreator", -> "../../models/User": User:@UserModel "logger-sharelatex":{log:->} 'metrics-sharelatex': {timeAsyncMethod: ()->} - "./UserAffiliationsManager": addAffiliation: @addAffiliation + "../Institutions/InstitutionsAPI": addAffiliation: @addAffiliation @email = "bob.oswald@gmail.com" diff --git a/services/web/test/unit/coffee/User/UserDeleterTests.coffee b/services/web/test/unit/coffee/User/UserDeleterTests.coffee index d2be2d211b..fc5f2ef370 100644 --- a/services/web/test/unit/coffee/User/UserDeleterTests.coffee +++ b/services/web/test/unit/coffee/User/UserDeleterTests.coffee @@ -31,7 +31,7 @@ describe "UserDeleter", -> "../Newsletter/NewsletterManager": @NewsletterManager "../Subscription/SubscriptionHandler": @SubscriptionHandler "../Project/ProjectDeleter": @ProjectDeleter - "./UserAffiliationsManager": + "../Institutions/InstitutionsAPI": deleteAffiliations: @deleteAffiliations "logger-sharelatex": @logger = { log: sinon.stub() } diff --git a/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee b/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee index 384623a2b5..800a44336f 100644 --- a/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee @@ -24,6 +24,7 @@ describe "UserEmailsController", -> addEmailAddress: sinon.stub() removeEmailAddress: sinon.stub() setDefaultEmailAddress: sinon.stub() + updateV1AndSetDefaultEmailAddress: sinon.stub() @EmailHelper = parseEmail: sinon.stub() @endorseAffiliation = sinon.stub().yields() @@ -33,7 +34,7 @@ describe "UserEmailsController", -> "./UserUpdater": @UserUpdater "../Helpers/EmailHelper": @EmailHelper "./UserEmailsConfirmationHandler": @UserEmailsConfirmationHandler = {} - "./UserAffiliationsManager": endorseAffiliation: @endorseAffiliation + "../Institutions/InstitutionsAPI": endorseAffiliation: @endorseAffiliation "../Errors/Errors": Errors "logger-sharelatex": log: -> console.log(arguments) @@ -126,13 +127,13 @@ describe "UserEmailsController", -> @EmailHelper.parseEmail.returns @email it 'sets default email', (done) -> - @UserUpdater.setDefaultEmailAddress.callsArgWith 2, null + @UserUpdater.updateV1AndSetDefaultEmailAddress.callsArgWith 2, null @UserEmailsController.setDefault @req, sendStatus: (code) => code.should.equal 200 assertCalledWith @EmailHelper.parseEmail, @email - assertCalledWith @UserUpdater.setDefaultEmailAddress, @user._id, @email + assertCalledWith @UserUpdater.updateV1AndSetDefaultEmailAddress, @user._id, @email done() it 'handles email parse error', (done) -> diff --git a/services/web/test/unit/coffee/User/UserGetterTests.coffee b/services/web/test/unit/coffee/User/UserGetterTests.coffee index 50ab2261f1..79d9032283 100644 --- a/services/web/test/unit/coffee/User/UserGetterTests.coffee +++ b/services/web/test/unit/coffee/User/UserGetterTests.coffee @@ -5,6 +5,7 @@ path = require('path') sinon = require('sinon') modulePath = path.join __dirname, "../../../../app/js/Features/User/UserGetter" expect = require("chai").expect +Errors = require "../../../../app/js/Features/Errors/Errors" describe "UserGetter", -> @@ -21,15 +22,16 @@ describe "UserGetter", -> db: users: findOne: @findOne ObjectId: (id) -> return id settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } - @getAffiliations = sinon.stub().callsArgWith(1, null, []) + @getUserAffiliations = sinon.stub().callsArgWith(1, null, []) @UserGetter = SandboxedModule.require modulePath, requires: "logger-sharelatex": log:-> "../../infrastructure/mongojs": @Mongo "metrics-sharelatex": timeAsyncMethod: sinon.stub() 'settings-sharelatex': settings - './UserAffiliationsManager': - getAffiliations: @getAffiliations + '../Institutions/InstitutionsAPI': + getUserAffiliations: @getUserAffiliations + "../Errors/Errors": Errors describe "getUser", -> it "should get user", (done)-> @@ -75,7 +77,7 @@ describe "UserGetter", -> institution: { name: 'University Name', isUniversity: true } } ] - @getAffiliations.callsArgWith(1, null, affiliationsData) + @getUserAffiliations.callsArgWith(1, null, affiliationsData) @UserGetter.getUserFullEmails @fakeUser._id, (error, fullEmails) => assert.deepEqual fullEmails, [ { @@ -150,6 +152,7 @@ describe "UserGetter", -> @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @fakeUser) @UserGetter.ensureUniqueEmailAddress @newEmail, (err)=> should.exist(err) + expect(err).to.be.an.instanceof(Errors.EmailExistsError) err.message.should.equal 'alread_exists' done() diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee index cd8bb48cf4..f2ac951727 100644 --- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee +++ b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee @@ -25,12 +25,14 @@ describe "UserUpdater", -> @UserUpdater = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger "./UserGetter": @UserGetter - './UserAffiliationsManager': + '../Institutions/InstitutionsAPI': addAffiliation: @addAffiliation removeAffiliation: @removeAffiliation '../Subscription/FeaturesUpdater': refreshFeatures: @refreshFeatures "../../infrastructure/mongojs":@mongojs "metrics-sharelatex": timeAsyncMethod: sinon.stub() + "settings-sharelatex": @settings = {} + "request": @request = {} @stubbedUser = _id: "3131231"