Primary Email Check (#6471)

* added primary-email-check page, route and controllers
* add `#add-email` internal link in settings to display new email form
* added primary-email-check redirection with split test
* update `lastPrimaryEmailCheck` when the default email address is set
* added `lastPrimaryCheck` to admin panel
* translations for primary-email-check
* acceptance tests for primary-email-check
* [web] multi-submit for primary email check
* Using `confirmedAt` to prevent from displaying primary-email-check page

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>
Co-Authored-By: Miguel Serrano <mserranom@gmail.com>
GitOrigin-RevId: d8e3a280439da08038a4487d8bfd7b3b0596e3b5

services/web/app/src/Features/Project/ProjectController.js

@@ -41,6 +41,7 @@ const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')
 const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
 const SpellingHandler = require('../Spelling/SpellingHandler')
+const UserPrimaryEmailCheckHandler = require('../User/UserPrimaryEmailCheckHandler')
 
 const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => {
   if (!affiliation.institution) return false
@@ -404,7 +405,7 @@ const ProjectController = {
         user(cb) {
           User.findById(
             userId,
-            'emails featureSwitches overleaf awareOfV2 features lastLoginIp',
+            'email emails featureSwitches overleaf awareOfV2 features lastLoginIp lastPrimaryEmailCheck signUpDate',
             cb
           )
         },
@@ -440,13 +441,40 @@ const ProjectController = {
             )
           })
         },
+
+        primaryEmailCheckActive(cb) {
+          SplitTestHandler.getAssignment(
+            req,
+            'primary-email-check',
+            (err, assignment) => {
+              if (err) {
+                logger.warn(
+                  { err },
+                  'failed to get "primary-email-check" split test assignment'
+                )
+                cb(null, false)
+              } else {
+                cb(null, assignment.variant === 'active')
+              }
+            }
+          )
+        },
       },
       (err, results) => {
         if (err != null) {
           OError.tag(err, 'error getting data for project list page')
           return next(err)
         }
-        const { notifications, user, userEmailsData } = results
+        const { notifications, user, userEmailsData, primaryEmailCheckActive } =
+          results
+
+        if (
+          user &&
+          primaryEmailCheckActive &&
+          UserPrimaryEmailCheckHandler.requiresPrimaryEmailCheck(user)
+        ) {
+          return res.redirect('/user/emails/primary-email-check')
+        }
 
         const userEmails = userEmailsData.list || []
 

services/web/app/src/Features/User/UserEmailsController.js

@@ -10,6 +10,9 @@ const { endorseAffiliation } = require('../Institutions/InstitutionsAPI')
 const Errors = require('../Errors/Errors')
 const HttpErrorHandler = require('../Errors/HttpErrorHandler')
 const { expressify } = require('../../util/promises')
+const AsyncFormHelper = require('../Helpers/AsyncFormHelper')
+const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const UserPrimaryEmailCheckHandler = require('../User/UserPrimaryEmailCheckHandler')
 
 async function _sendSecurityAlertEmail(user, email) {
   const emailOptions = {
@@ -113,6 +116,35 @@ function sendReconfirmation(req, res, next) {
   })
 }
 
+async function primaryEmailCheckPage(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const user = await UserGetter.promises.getUser(userId, {
+    lastPrimaryEmailCheck: 1,
+    signUpDate: 1,
+    email: 1,
+    emails: 1,
+  })
+
+  if (!UserPrimaryEmailCheckHandler.requiresPrimaryEmailCheck(user)) {
+    return res.redirect('/project')
+  }
+
+  AnalyticsManager.recordEventForUser(
+    userId,
+    'primary-email-check-page-displayed'
+  )
+  res.render('user/primaryEmailCheck')
+}
+
+async function primaryEmailCheck(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  await UserUpdater.promises.updateUser(userId, {
+    $set: { lastPrimaryEmailCheck: new Date() },
+  })
+  AnalyticsManager.recordEventForUser(userId, 'primary-email-check-done')
+  AsyncFormHelper.redirect(req, res, '/project')
+}
+
 const UserEmailsController = {
   list(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
@@ -204,6 +236,10 @@ const UserEmailsController = {
 
   sendReconfirmation,
 
+  primaryEmailCheckPage: expressify(primaryEmailCheckPage),
+
+  primaryEmailCheck: expressify(primaryEmailCheck),
+
   showConfirm(req, res, next) {
     res.render('user/confirm_email', {
       token: req.query.token,

services/web/app/src/Features/User/UserPrimaryEmailCheckHandler.js

@@ -0,0 +1,29 @@
+const Settings = require('@overleaf/settings')
+
+function requiresPrimaryEmailCheck({
+  email,
+  emails,
+  lastPrimaryEmailCheck,
+  signUpDate,
+}) {
+  const hasExpired = date =>
+    Date.now() - date.getTime() > Settings.primary_email_check_expiration
+
+  const primaryEmailConfirmedAt = emails.find(
+    emailEntry => emailEntry.email === email
+  ).confirmedAt
+
+  if (primaryEmailConfirmedAt && !hasExpired(primaryEmailConfirmedAt)) {
+    return false
+  }
+
+  if (lastPrimaryEmailCheck) {
+    return hasExpired(lastPrimaryEmailCheck)
+  } else {
+    return hasExpired(signUpDate)
+  }
+}
+
+module.exports = {
+  requiresPrimaryEmailCheck,
+}

services/web/app/src/Features/User/UserUpdater.js

@@ -18,6 +18,7 @@ const Errors = require('../Errors/Errors')
 const NewsletterManager = require('../Newsletter/NewsletterManager')
 const RecurlyWrapper = require('../Subscription/RecurlyWrapper')
 const UserAuditLogHandler = require('./UserAuditLogHandler')
+const AnalyticsManager = require('../Analytics/AnalyticsManager')
 
 async function _sendSecurityAlertPrimaryEmailChanged(userId, oldEmail, email) {
   // send email to both old and new primary email
@@ -47,6 +48,8 @@ async function addEmailAddress(userId, newEmail, affiliationOptions, auditLog) {
 
   await UserGetter.promises.ensureUniqueEmailAddress(newEmail)
 
+  AnalyticsManager.recordEventForUser(userId, 'secondary-email-added')
+
   await UserAuditLogHandler.promises.addEntry(
     userId,
     'add-email',
@@ -157,7 +160,7 @@ async function setDefaultEmailAddress(
   )
 
   const query = { _id: userId, 'emails.email': email }
-  const update = { $set: { email } }
+  const update = { $set: { email, lastPrimaryEmailCheck: new Date() } }
   const res = await UserUpdater.promises.updateUser(query, update)
 
   // this should not happen
@@ -165,6 +168,8 @@ async function setDefaultEmailAddress(
     throw new Error('email update error')
   }
 
+  AnalyticsManager.recordEventForUser(userId, 'primary-email-address-updated')
+
   if (sendSecurityAlert) {
     // no need to wait, errors are logged and not passed back
     _sendSecurityAlertPrimaryEmailChanged(userId, oldEmail, email)

services/web/app/src/models/User.js

@@ -61,6 +61,7 @@ const UserSchema = new Schema({
   lastFailedLogin: { type: Date },
   lastLoggedIn: { type: Date },
   lastLoginIp: { type: String, default: '' },
+  lastPrimaryEmailCheck: { type: Date },
   loginCount: { type: Number, default: 0 },
   holdingAccount: { type: Boolean, default: false },
   ace: {

services/web/app/src/router.js

@@ -203,6 +203,18 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     UserEmailsController.resendConfirmation
   )
 
+  webRouter.get(
+    '/user/emails/primary-email-check',
+    AuthenticationController.requireLogin(),
+    UserEmailsController.primaryEmailCheckPage
+  )
+
+  webRouter.post(
+    '/user/emails/primary-email-check',
+    AuthenticationController.requireLogin(),
+    UserEmailsController.primaryEmailCheck
+  )
+
   if (Features.hasFeature('affiliations')) {
     webRouter.post(
       '/user/emails',

services/web/app/views/user/primaryEmailCheck.pug

@@ -0,0 +1,41 @@
+extends ../layout-marketing
+
+block content
+	main.content.content-alt#main-content
+		.login-register-container.text-center.primary-email-check-container
+			.card.login-register-card.primary-email-check-card
+				.login-register-header
+					h1.login-register-header-heading #{translate("keep_your_account_safe")}
+				.login-register-form(data-ol-multi-submit)
+					p
+						| !{translate("primary_email_check_question", { email: getUserEmail() }, ["strong"])}
+					form(
+						data-ol-async-form
+						action="/user/emails/primary-email-check"
+						method="POST"
+					)
+						input(name='_csrf', type='hidden', value=csrfToken)
+						+formMessages()
+
+						.primary-email-check-button-container
+							button.btn-primary.btn.btn-block.btn-primary-email-check-button(
+								type='submit'
+								data-ol-disabled-inflight
+							)
+								span(data-ol-inflight="idle") #{translate("yes_that_is_correct")}
+								span(hidden data-ol-inflight="pending") #{translate("confirming")}…
+
+					.primary-email-check-button-container
+						a.btn-default.btn.btn-block.btn-primary-email-check-button(
+							href="/user/settings#add-email"
+							data-ol-slow-link
+							event-tracking="primary-email-check-change-email"
+							event-tracking-mb="true"
+							event-tracking-trigger="click"
+						)
+							span(data-ol-inflight="idle") #{translate("no_update_email")}
+							span(hidden data-ol-inflight="pending") #{translate("redirecting")}…	
+					p
+						|  #{translate("keep_your_email_updated")}
+					p
+						|  !{translate("learn_more_about_emails", {}, [{name: 'a', attrs: {href: '/learn/how-to/Keeping_your_account_secure', 'event-tracking': 'primary-email-check-learn-more', 'event-tracking-mb': 'true', 'event-tracking-trigger': 'click' }}])}

services/web/app/views/user/settings/user-affiliations.pug

@@ -40,6 +40,7 @@ form.row(
 	.col-md-12
 		h3 #{translate("emails_and_affiliations_title")}
 		p.small #{translate("emails_and_affiliations_explanation")}
+		p.small !{translate("change_primary_email_address_instructions", {}, [{name: "strong"}, { name: "a", attrs: { href: "/learn/how-to/Keeping_your_account_secure"}}])}
 		table.table.affiliations-table
 			thead
 				tr
@@ -190,11 +191,12 @@ form.row(
 								ng-model-options="{ allowInvalid: true }"
 								get-suggestion="getEmailSuggestion(userInput)"
 								on-blur="handleEmailInputBlur()"
-								input-id="affilitations-email"
-								input-name="affilitationsEmail"
+								input-id="affiliations-email"
+								input-name="affiliationsEmail"
 								input-placeholder="e.g. johndoe@mit.edu"
 								input-type="email"
 								input-required="true"
+								focus-on-render="true"
 							)
 					td(
 						colspan="2"

services/web/config/settings.defaults.js

@@ -533,6 +533,8 @@ module.exports = {
   // Maximum size of text documents in the real-time editing system.
   max_doc_length: 2 * 1024 * 1024, // 2mb
 
+  primary_email_check_expiration: 1000 * 60 * 60 * 24 * 90, // 90 days
+
   // Maximum JSON size in HTTP requests
   // We should be able to process twice the max doc length, to allow for
   //   - the doc content

services/web/frontend/js/main/affiliations/components/inputSuggestions.js

@@ -91,6 +91,7 @@ export default App.component('inputSuggestions', {
     ' ng-attr-placeholder="{{ ::$ctrl.inputPlaceholder }}"',
     ' ng-attr-type="{{ ::$ctrl.inputType }}"',
     ' ng-attr-name="{{ ::$ctrl.inputName }}"',
+    ' autofocus="::$ctrl.focusOnRender"',
     ' ng-required="::$ctrl.inputRequired">',
     '</div>',
   ].join(''),

services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js

@@ -309,6 +309,7 @@ export default App.controller(
       return _resetAddingEmail()
     }
     _reset()
+    $scope.ui.showAddEmailUI = window.location.hash === '#add-email'
 
     function _monitorRequest(promise) {
       $scope.ui.hasError = false

services/web/frontend/stylesheets/_style_includes.less

@@ -99,6 +99,7 @@
 @import 'app/editor/history-v2.less';
 @import 'app/metrics.less';
 @import 'app/open-in-overleaf.less';
+@import 'app/primary-email-check';
 
 // module styles
 // TODO: find a way for modules to add styles dynamically

services/web/frontend/stylesheets/app/primary-email-check.less

@@ -0,0 +1,15 @@
+.primary-email-check-button-container {
+  padding: 0 @padding-md;
+}
+
+.btn-primary-email-check-button {
+  margin-bottom: @margin-md;
+}
+
+.primary-email-check-container {
+  max-width: 420px !important;
+}
+
+.primary-email-check-card {
+  padding: 0 !important;
+}

services/web/locales/en.json

@@ -1563,5 +1563,14 @@
   "search_previous": "previous",
   "search_regexp": "regexp",
   "search_replace": "replace",
-  "search_replace_all": "replace all"
+  "search_replace_all": "replace all",
+  "keep_your_account_safe": "Keep your account safe",
+  "primary_email_check_question": "Is <0>__email__</0> still your email address?",
+  "yes_that_is_correct": "Yes, that’s correct",
+  "no_update_email": "No, update email",
+  "redirecting": "Redirecting",
+  "keep_your_email_updated": "Keep your email updated so that you don’t lose access to your account and data.",
+  "learn_more_about_emails": "<0>Learn more</0> about managing your __appName__ emails.",
+  "thank_you_email_checked": "Thank you, we’re now taking you back to the projects page",
+  "change_primary_email_address_instructions": "To change your primary email, please add your new primary email address first (by clicking <0>Add another email</0>) and confirm it. Then click the <0>Make Primary</0> button. <1>Learn more</1> about managing your __appName__ emails."
 }

services/web/test/acceptance/src/PrimaryEmailCheckTests.js

@@ -0,0 +1,162 @@
+const UserHelper = require('./helpers/UserHelper')
+const Settings = require('@overleaf/settings')
+const { expect } = require('chai')
+
+// While the split test is in progress this must be appended to URLs during tests
+const SPLIT_TEST_QUERY = '?primary-email-check=active'
+
+describe('PrimaryEmailCheck', function () {
+  let userHelper
+
+  beforeEach(async function () {
+    userHelper = await UserHelper.createUser()
+    userHelper = await UserHelper.loginUser(
+      userHelper.getDefaultEmailPassword()
+    )
+  })
+
+  describe('redirections', function () {
+    describe('when the user has signed up recently', function () {
+      it("shouldn't be redirected from project list to the primary email check page", async function () {
+        const response = await userHelper.request.get(
+          '/project' + SPLIT_TEST_QUERY
+        )
+        expect(response.statusCode).to.equal(200)
+      })
+
+      it('should be redirected from the primary email check page to the project list', async function () {
+        const response = await userHelper.request.get(
+          '/user/emails/primary-email-check' + SPLIT_TEST_QUERY,
+          { simple: false }
+        )
+        expect(response.statusCode).to.equal(302)
+        expect(response.headers.location).to.equal('/project')
+      })
+    })
+
+    describe('when the user has checked their email recently', function () {
+      beforeEach(async function () {
+        const time = Date.now() - Settings.primary_email_check_expiration * 0.5
+        await UserHelper.updateUser(userHelper.user._id, {
+          $set: { lastPrimaryEmailCheck: new Date(time) },
+        })
+      })
+
+      it("shouldn't be redirected from project list to the primary email check page", async function () {
+        const response = await userHelper.request.get(
+          '/project' + SPLIT_TEST_QUERY
+        )
+        expect(response.statusCode).to.equal(200)
+      })
+
+      it('should be redirected from the primary email check page to the project list', async function () {
+        const response = await userHelper.request.get(
+          '/user/emails/primary-email-check' + SPLIT_TEST_QUERY,
+          { simple: false }
+        )
+        expect(response.statusCode).to.equal(302)
+        expect(response.headers.location).to.equal('/project')
+      })
+    })
+
+    describe('when the user has confirmed their primary email recently', function () {
+      beforeEach(async function () {
+        // the user should check again their email according to `lastPrimaryEmailCheck` timestamp, but the behaviour is
+        // overridden by email confirmation
+        const time = Date.now() - Settings.primary_email_check_expiration * 2
+        await UserHelper.updateUser(userHelper.user._id, {
+          $set: { lastPrimaryEmailCheck: new Date(time) },
+        })
+
+        await userHelper.confirmEmail(
+          userHelper.user._id,
+          userHelper.user.email
+        )
+      })
+
+      it("shouldn't be redirected from project list to the primary email check page", async function () {
+        const response = await userHelper.request.get(
+          '/project' + SPLIT_TEST_QUERY
+        )
+        expect(response.statusCode).to.equal(200)
+      })
+
+      it('should be redirected from the primary email check page to the project list', async function () {
+        const response = await userHelper.request.get(
+          '/user/emails/primary-email-check' + SPLIT_TEST_QUERY,
+          { simple: false }
+        )
+        expect(response.statusCode).to.equal(302)
+        expect(response.headers.location).to.equal('/project')
+      })
+    })
+
+    describe('when the user has signed for longer than the email check expiration period', function () {
+      beforeEach(async function () {
+        const time = Date.now() - Settings.primary_email_check_expiration * 2
+        await UserHelper.updateUser(userHelper.user._id, {
+          $set: { lastPrimaryEmailCheck: new Date(time) },
+        })
+      })
+
+      it('should be redirected from project list to the primary email check page', async function () {
+        const response = await userHelper.request.get(
+          '/project' + SPLIT_TEST_QUERY,
+          { simple: false }
+        )
+        expect(response.statusCode).to.equal(302)
+        expect(response.headers.location).to.equal(
+          '/user/emails/primary-email-check'
+        )
+      })
+
+      it('can visit the primary email check page', async function () {
+        const response = await userHelper.request.get(
+          '/user/emails/primary-email-check'
+        )
+        expect(response.statusCode).to.equal(200)
+      })
+    })
+  })
+
+  describe('when user checks their primary email address', function () {
+    let checkResponse
+
+    beforeEach(async function () {
+      // make sure the user requires checking their primary email address
+      const time = Date.now() - Settings.primary_email_check_expiration * 2
+      await UserHelper.updateUser(userHelper.user._id, {
+        $set: { lastPrimaryEmailCheck: new Date(time) },
+      })
+
+      checkResponse = await userHelper.request.post(
+        '/user/emails/primary-email-check' + SPLIT_TEST_QUERY,
+        {
+          form: {},
+          simple: false,
+        }
+      )
+    })
+
+    it('should be redirected to the project list page', function () {
+      expect(checkResponse.statusCode).to.equal(302)
+      expect(checkResponse.headers.location).to.equal('/project')
+    })
+
+    it("shouldn't be redirected from project list to the primary email check page any longer", async function () {
+      const response = await userHelper.request.get(
+        '/project' + SPLIT_TEST_QUERY
+      )
+      expect(response.statusCode).to.equal(200)
+    })
+
+    it('visiting the primary email check page should redirect to the project list page', async function () {
+      const response = await userHelper.request.get(
+        '/user/emails/primary-email-check',
+        { simple: false }
+      )
+      expect(response.statusCode).to.equal(302)
+      expect(response.headers.location).to.equal('/project')
+    })
+  })
+})

services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -49,6 +49,9 @@ describe('UserEmailsController', function () {
       endorseAffiliation: this.endorseAffiliation,
     }
     this.HttpErrorHandler = { conflict: sinon.stub() }
+    this.AnalyticsManager = {
+      recordEventForUser: sinon.stub(),
+    }
     this.UserEmailsController = SandboxedModule.require(modulePath, {
       requires: {
         '../Authentication/SessionManager': this.SessionManager,
@@ -71,6 +74,7 @@ describe('UserEmailsController', function () {
           }),
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
+        '../Analytics/AnalyticsManager': this.AnalyticsManager,
       },
     })
   })

services/web/test/unit/src/User/UserUpdaterTests.js

@@ -40,6 +40,9 @@ describe('UserUpdater', function () {
         updateAccountEmailAddress: sinon.stub(),
       },
     }
+    this.AnalyticsManager = {
+      recordEventForUser: sinon.stub(),
+    }
     this.UserUpdater = SandboxedModule.require(modulePath, {
       requires: {
         '../Helpers/Mongo': { normalizeQuery },
@@ -79,6 +82,7 @@ describe('UserUpdater', function () {
             addEntry: sinon.stub().resolves(),
           },
         }),
+        '../Analytics/AnalyticsManager': this.AnalyticsManager,
       },
     })
 
@@ -549,7 +553,12 @@ describe('UserUpdater', function () {
           this.UserUpdater.promises.updateUser
             .calledWith(
               { _id: this.stubbedUser._id, 'emails.email': this.newEmail },
-              { $set: { email: this.newEmail } }
+              {
+                $set: {
+                  email: this.newEmail,
+                  lastPrimaryEmailCheck: sinon.match.date,
+                },
+              }
             )
             .should.equal(true)
           done()