Răsfoiți Sursa

[web] Remove `overleaf-integration` overrides for `/user/emails/delete` & `/user/emails/resend_confirmation` (#19438)

* Use hooks to call `clearSamlSession`

* Promisify `UserEmailsController.remove`

* Use hook for `userDeleteEmailMiddleware`

* Remove `/user/emails/delete` override

* Remove "removeRoute of `/user/emails/resend-secondary-confirmation`"

That route isn't defined elsewhere

* Promisify `UserEmailsController.resendConfirmation`

* Promisify `UserEmailsController.sendReconfirmation`

* Use hook for `resendConfirmationEmailMiddleware`

* Remove `/user/emails/resend_confirmation` override

* Promisify `tryDeleteUser`

* Proxy `clearSamlSession` through `SAMLHelper`

* Revert "Use hook for `resendConfirmationEmailMiddleware`"

This reverts commit f028d9c8

* Inject `SAMLMiddleware.resendConfirmationEmailMiddleware` in `/user/emails/resend_confirmation`

* Update `middleware` syntax and grammar

* Update tests

* Use Module middleware instead of hook for `userDeleteEmailMiddleware`

* Remove "promises" export of tryDeleteUser

GitOrigin-RevId: 211e194fc1ef82dc452ee4e837dcddd9b23690a0
Antoine Clausse 2 ani în urmă
părinte
comite
afd965c04b

+ 41 - 57
services/web/app/src/Features/User/UserEmailsController.js

@@ -107,56 +107,42 @@ async function add(req, res, next) {
   res.sendStatus(204)
   res.sendStatus(204)
 }
 }
 
 
-function resendConfirmation(req, res, next) {
+async function resendConfirmation(req, res) {
   const userId = SessionManager.getLoggedInUserId(req.session)
   const userId = SessionManager.getLoggedInUserId(req.session)
   const email = EmailHelper.parseEmail(req.body.email)
   const email = EmailHelper.parseEmail(req.body.email)
   if (!email) {
   if (!email) {
     return res.sendStatus(422)
     return res.sendStatus(422)
   }
   }
-  UserGetter.getUserByAnyEmail(email, { _id: 1 }, function (error, user) {
-    if (error) {
-      return next(error)
-    }
-    if (!user || user._id.toString() !== userId) {
-      return res.sendStatus(422)
-    }
-    UserEmailsConfirmationHandler.sendConfirmationEmail(
-      userId,
-      email,
-      function (error) {
-        if (error) {
-          return next(error)
-        }
-        res.sendStatus(200)
-      }
-    )
-  })
+  const user = await UserGetter.promises.getUserByAnyEmail(email, { _id: 1 })
+
+  if (!user || user._id.toString() !== userId) {
+    return res.sendStatus(422)
+  }
+
+  await UserEmailsConfirmationHandler.promises.sendConfirmationEmail(
+    userId,
+    email
+  )
+  res.sendStatus(200)
 }
 }
 
 
-function sendReconfirmation(req, res, next) {
+async function sendReconfirmation(req, res) {
   const userId = SessionManager.getLoggedInUserId(req.session)
   const userId = SessionManager.getLoggedInUserId(req.session)
   const email = EmailHelper.parseEmail(req.body.email)
   const email = EmailHelper.parseEmail(req.body.email)
   if (!email) {
   if (!email) {
     return res.sendStatus(400)
     return res.sendStatus(400)
   }
   }
-  UserGetter.getUserByAnyEmail(email, { _id: 1 }, function (error, user) {
-    if (error) {
-      return next(error)
-    }
-    if (!user || user._id.toString() !== userId) {
-      return res.sendStatus(422)
-    }
-    UserEmailsConfirmationHandler.sendReconfirmationEmail(
-      userId,
-      email,
-      function (error) {
-        if (error) {
-          return next(error)
-        }
-        res.sendStatus(204)
-      }
-    )
-  })
+  const user = await UserGetter.promises.getUserByAnyEmail(email, { _id: 1 })
+
+  if (!user || user._id.toString() !== userId) {
+    return res.sendStatus(422)
+  }
+  await UserEmailsConfirmationHandler.promises.sendReconfirmationEmail(
+    userId,
+    email
+  )
+
+  res.sendStatus(204)
 }
 }
 
 
 /**
 /**
@@ -521,6 +507,20 @@ async function showConfirm(req, res, next) {
   })
   })
 }
 }
 
 
+async function remove(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const email = EmailHelper.parseEmail(req.body.email)
+  if (!email) {
+    return res.sendStatus(422)
+  }
+  const auditLog = {
+    initiatorId: userId,
+    ipAddress: req.ip,
+  }
+  await UserUpdater.promises.removeEmailAddress(userId, email, auditLog)
+  res.sendStatus(200)
+}
+
 const UserEmailsController = {
 const UserEmailsController = {
   list(req, res, next) {
   list(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
     const userId = SessionManager.getLoggedInUserId(req.session)
@@ -541,23 +541,7 @@ const UserEmailsController = {
     resendSecondaryEmailConfirmationCode
     resendSecondaryEmailConfirmationCode
   ),
   ),
 
 
-  remove(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const email = EmailHelper.parseEmail(req.body.email)
-    if (!email) {
-      return res.sendStatus(422)
-    }
-    const auditLog = {
-      initiatorId: userId,
-      ipAddress: req.ip,
-    }
-    UserUpdater.removeEmailAddress(userId, email, auditLog, function (error) {
-      if (error) {
-        return next(error)
-      }
-      res.sendStatus(200)
-    })
-  },
+  remove: expressify(remove),
 
 
   setDefault(req, res, next) {
   setDefault(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
     const userId = SessionManager.getLoggedInUserId(req.session)
@@ -618,9 +602,9 @@ const UserEmailsController = {
     )
     )
   },
   },
 
 
-  resendConfirmation,
+  resendConfirmation: expressify(resendConfirmation),
 
 
-  sendReconfirmation,
+  sendReconfirmation: expressify(sendReconfirmation),
 
 
   addSecondaryEmailPage: expressify(addSecondaryEmailPage),
   addSecondaryEmailPage: expressify(addSecondaryEmailPage),
 
 

+ 2 - 0
services/web/app/src/router.js

@@ -326,6 +326,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     '/user/emails/resend_confirmation',
     '/user/emails/resend_confirmation',
     AuthenticationController.requireLogin(),
     AuthenticationController.requireLogin(),
     RateLimiterMiddleware.rateLimit(rateLimiters.resendConfirmation),
     RateLimiterMiddleware.rateLimit(rateLimiters.resendConfirmation),
+    Modules.middleware('resendConfirmationEmail'),
     UserEmailsController.resendConfirmation
     UserEmailsController.resendConfirmation
   )
   )
 
 
@@ -356,6 +357,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       '/user/emails/delete',
       '/user/emails/delete',
       AuthenticationController.requireLogin(),
       AuthenticationController.requireLogin(),
       RateLimiterMiddleware.rateLimit(rateLimiters.deleteEmail),
       RateLimiterMiddleware.rateLimit(rateLimiters.deleteEmail),
+      Modules.middleware('userDeleteEmail'),
       UserEmailsController.remove
       UserEmailsController.remove
     )
     )
     webRouter.post(
     webRouter.post(

+ 30 - 30
services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -23,10 +23,10 @@ describe('UserEmailsController', function () {
     this.UserGetter = {
     this.UserGetter = {
       getUser: sinon.stub().yields(),
       getUser: sinon.stub().yields(),
       getUserFullEmails: sinon.stub(),
       getUserFullEmails: sinon.stub(),
-      getUserByAnyEmail: sinon.stub(),
       promises: {
       promises: {
         ensureUniqueEmailAddress: sinon.stub().resolves(),
         ensureUniqueEmailAddress: sinon.stub().resolves(),
         getUser: sinon.stub().resolves(this.user),
         getUser: sinon.stub().resolves(this.user),
+        getUserByAnyEmail: sinon.stub(),
       },
       },
     }
     }
     this.SessionManager = {
     this.SessionManager = {
@@ -42,12 +42,12 @@ describe('UserEmailsController', function () {
     }
     }
     this.UserUpdater = {
     this.UserUpdater = {
       addEmailAddress: sinon.stub(),
       addEmailAddress: sinon.stub(),
-      removeEmailAddress: sinon.stub(),
       setDefaultEmailAddress: sinon.stub(),
       setDefaultEmailAddress: sinon.stub(),
       updateV1AndSetDefaultEmailAddress: sinon.stub(),
       updateV1AndSetDefaultEmailAddress: sinon.stub(),
       promises: {
       promises: {
         addEmailAddress: sinon.stub().resolves(),
         addEmailAddress: sinon.stub().resolves(),
         confirmEmail: sinon.stub().resolves(),
         confirmEmail: sinon.stub().resolves(),
+        removeEmailAddress: sinon.stub(),
       },
       },
     }
     }
     this.EmailHelper = { parseEmail: sinon.stub() }
     this.EmailHelper = { parseEmail: sinon.stub() }
@@ -91,9 +91,9 @@ describe('UserEmailsController', function () {
         '../Helpers/EmailHelper': this.EmailHelper,
         '../Helpers/EmailHelper': this.EmailHelper,
         './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler =
         './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler =
           {
           {
-            sendReconfirmationEmail: sinon.stub(),
             promises: {
             promises: {
               sendConfirmationEmail: sinon.stub().resolves(),
               sendConfirmationEmail: sinon.stub().resolves(),
+              sendReconfirmationEmail: sinon.stub(),
             },
             },
           }),
           }),
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
@@ -486,14 +486,14 @@ describe('UserEmailsController', function () {
         initiatorId: this.user._id,
         initiatorId: this.user._id,
         ipAddress: this.req.ip,
         ipAddress: this.req.ip,
       }
       }
-      this.UserUpdater.removeEmailAddress.callsArgWith(3, null)
+      this.UserUpdater.promises.removeEmailAddress.resolves()
 
 
       this.UserEmailsController.remove(this.req, {
       this.UserEmailsController.remove(this.req, {
         sendStatus: code => {
         sendStatus: code => {
           code.should.equal(200)
           code.should.equal(200)
           assertCalledWith(this.EmailHelper.parseEmail, this.email)
           assertCalledWith(this.EmailHelper.parseEmail, this.email)
           assertCalledWith(
           assertCalledWith(
-            this.UserUpdater.removeEmailAddress,
+            this.UserUpdater.promises.removeEmailAddress,
             this.user._id,
             this.user._id,
             this.email,
             this.email,
             auditLog
             auditLog
@@ -509,7 +509,7 @@ describe('UserEmailsController', function () {
       this.UserEmailsController.remove(this.req, {
       this.UserEmailsController.remove(this.req, {
         sendStatus: code => {
         sendStatus: code => {
           code.should.equal(422)
           code.should.equal(422)
-          assertNotCalled(this.UserUpdater.removeEmailAddress)
+          assertNotCalled(this.UserUpdater.promises.removeEmailAddress)
           done()
           done()
         },
         },
       })
       })
@@ -703,7 +703,7 @@ describe('UserEmailsController', function () {
   describe('resendConfirmation', function () {
   describe('resendConfirmation', function () {
     beforeEach(function () {
     beforeEach(function () {
       this.EmailHelper.parseEmail.returnsArg(0)
       this.EmailHelper.parseEmail.returnsArg(0)
-      this.UserGetter.getUserByAnyEmail.yields(undefined, {
+      this.UserGetter.promises.getUserByAnyEmail.resolves({
         _id: this.user._id,
         _id: this.user._id,
       })
       })
       this.req = {
       this.req = {
@@ -718,20 +718,20 @@ describe('UserEmailsController', function () {
         .yields()
         .yields()
     })
     })
 
 
-    it('should send the email', function (done) {
+    it('should send the email', async function () {
       this.req = {
       this.req = {
         body: {
         body: {
           email: 'test@example.com',
           email: 'test@example.com',
         },
         },
       }
       }
-      this.UserEmailsController.sendReconfirmation(
+      await this.UserEmailsController.sendReconfirmation(
         this.req,
         this.req,
         this.res,
         this.res,
         this.next
         this.next
       )
       )
-      expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.have
-        .been.calledOnce
-      done()
+      expect(
+        this.UserEmailsConfirmationHandler.promises.sendReconfirmationEmail
+      ).to.have.been.calledOnce
     })
     })
 
 
     it('should return 422 if email not valid', function (done) {
     it('should return 422 if email not valid', function (done) {
@@ -751,17 +751,17 @@ describe('UserEmailsController', function () {
 
 
     describe('email on another user account', function () {
     describe('email on another user account', function () {
       beforeEach(function () {
       beforeEach(function () {
-        this.UserGetter.getUserByAnyEmail.yields(undefined, {
+        this.UserGetter.promises.getUserByAnyEmail.resolves({
           _id: 'another-user-id',
           _id: 'another-user-id',
         })
         })
       })
       })
-      it('should return 422', function (done) {
+      it('should return 422', async function () {
         this.req = {
         this.req = {
           body: {
           body: {
             email: 'test@example.com',
             email: 'test@example.com',
           },
           },
         }
         }
-        this.UserEmailsController.resendConfirmation(
+        await this.UserEmailsController.resendConfirmation(
           this.req,
           this.req,
           this.res,
           this.res,
           this.next
           this.next
@@ -769,7 +769,6 @@ describe('UserEmailsController', function () {
         expect(this.UserEmailsConfirmationHandler.sendConfirmationEmail).to.not
         expect(this.UserEmailsConfirmationHandler.sendConfirmationEmail).to.not
           .have.been.called
           .have.been.called
         expect(this.res.sendStatus.lastCall.args[0]).to.equal(422)
         expect(this.res.sendStatus.lastCall.args[0]).to.equal(422)
-        done()
       })
       })
     })
     })
   })
   })
@@ -777,25 +776,25 @@ describe('UserEmailsController', function () {
   describe('sendReconfirmation', function () {
   describe('sendReconfirmation', function () {
     beforeEach(function () {
     beforeEach(function () {
       this.res.sendStatus = sinon.stub()
       this.res.sendStatus = sinon.stub()
-      this.UserGetter.getUserByAnyEmail.yields(undefined, {
+      this.UserGetter.promises.getUserByAnyEmail.resolves({
         _id: this.user._id,
         _id: this.user._id,
       })
       })
       this.EmailHelper.parseEmail.returnsArg(0)
       this.EmailHelper.parseEmail.returnsArg(0)
     })
     })
-    it('should send the email', function (done) {
+    it('should send the email', async function () {
       this.req = {
       this.req = {
         body: {
         body: {
           email: 'test@example.com',
           email: 'test@example.com',
         },
         },
       }
       }
-      this.UserEmailsController.sendReconfirmation(
+      await this.UserEmailsController.sendReconfirmation(
         this.req,
         this.req,
         this.res,
         this.res,
         this.next
         this.next
       )
       )
-      expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.have
-        .been.calledOnce
-      done()
+      expect(
+        this.UserEmailsConfirmationHandler.promises.sendReconfirmationEmail
+      ).to.have.been.calledOnce
     })
     })
     it('should return 400 if email not valid', function (done) {
     it('should return 400 if email not valid', function (done) {
       this.req = {
       this.req = {
@@ -806,32 +805,33 @@ describe('UserEmailsController', function () {
         this.res,
         this.res,
         this.next
         this.next
       )
       )
-      expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.not
-        .have.been.called
+      expect(
+        this.UserEmailsConfirmationHandler.promises.sendReconfirmationEmail
+      ).to.not.have.been.called
       expect(this.res.sendStatus.lastCall.args[0]).to.equal(400)
       expect(this.res.sendStatus.lastCall.args[0]).to.equal(400)
       done()
       done()
     })
     })
     describe('email on another user account', function () {
     describe('email on another user account', function () {
       beforeEach(function () {
       beforeEach(function () {
-        this.UserGetter.getUserByAnyEmail.yields(undefined, {
+        this.UserGetter.promises.getUserByAnyEmail.resolves({
           _id: 'another-user-id',
           _id: 'another-user-id',
         })
         })
       })
       })
-      it('should return 422', function (done) {
+      it('should return 422', async function () {
         this.req = {
         this.req = {
           body: {
           body: {
             email: 'test@example.com',
             email: 'test@example.com',
           },
           },
         }
         }
-        this.UserEmailsController.sendReconfirmation(
+        await this.UserEmailsController.sendReconfirmation(
           this.req,
           this.req,
           this.res,
           this.res,
           this.next
           this.next
         )
         )
-        expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to
-          .not.have.been.called
+        expect(
+          this.UserEmailsConfirmationHandler.promises.sendReconfirmationEmail
+        ).to.not.have.been.called
         expect(this.res.sendStatus.lastCall.args[0]).to.equal(422)
         expect(this.res.sendStatus.lastCall.args[0]).to.equal(422)
-        done()
       })
       })
     })
     })
   })
   })