Merge pull request #765 from sharelatex/ja-update-email-in-v1

Update email in v1 when setting as default in v2
This commit is contained in:
James Allen
2018-07-26 10:58:57 +01:00
committed by GitHub
13 changed files with 391 additions and 96 deletions
@@ -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
@@ -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")
@@ -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)
@@ -4,6 +4,7 @@ logger = require('logger-sharelatex')
db = mongojs.db
ObjectId = mongojs.ObjectId
{ getAffiliations } = require("./UserAffiliationsManager")
Errors = require("../Errors/Errors")
module.exports = UserGetter =
getUser: (query, projection, callback = (error, user) ->) ->
@@ -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) ->
@@ -9,6 +9,8 @@ UserGetter = require("./UserGetter")
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?
@@ -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")}
| &nbsp;#{translate("save_or_cancel-or" )}&nbsp;
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")}
| &nbsp;
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
| &nbsp;#{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
| &nbsp;#{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
| &nbsp;#{translate("loading")}...
td.text-center(colspan="3", ng-if="ui.isResendingConfirmation")
i.fa.fa-fw.fa-spin.fa-refresh
| &nbsp;#{translate("sending")}...
td.text-center(colspan="3", ng-if="!ui.isLoadingEmails && !ui.isResendingConfirmation")
i.fa.fa-fw.fa-spin.fa-refresh
| &nbsp;#{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
| &nbsp;#{translate("error_performing_request")}
div
button.btn.btn-xs(
ng-click="acknowledgeError();"
) #{translate("reload_emails_and_affiliations")}
span(ng-if="!ui.errorMessage") &nbsp;#{translate("error_performing_request")}
span(ng-if="ui.errorMessage") &nbsp;{{ui.errorMessage}}
hr
@@ -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()
]
@@ -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
@@ -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) ->
@@ -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()
@@ -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()
@@ -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) ->
@@ -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", ->
@@ -30,6 +31,7 @@ describe "UserGetter", ->
'settings-sharelatex': settings
'./UserAffiliationsManager':
getAffiliations: @getAffiliations
"../Errors/Errors": Errors
describe "getUser", ->
it "should get user", (done)->
@@ -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()
@@ -31,6 +31,8 @@ describe "UserUpdater", ->
'../Subscription/FeaturesUpdater': refreshFeatures: @refreshFeatures
"../../infrastructure/mongojs":@mongojs
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
"settings-sharelatex": @settings = {}
"request": @request = {}
@stubbedUser =
_id: "3131231"