Merge pull request #19861 from overleaf/ab-overleaf-integration-delete-flow

[web] Remove /user/delete override from overleaf-integration

GitOrigin-RevId: 4f679f6835522c2325fe7b0368f58e7a952ee73d

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

@@ -225,17 +225,30 @@ async function tryDeleteUser(req, res, next) {
   const { password } = req.body
   req.logger.addFields({ userId })
 
+  logger.debug({ userId }, 'trying to delete user account')
   if (password == null || password === '') {
     logger.err({ userId }, 'no password supplied for attempt to delete account')
     return res.sendStatus(403)
   }
 
-  const { user } = await AuthenticationManager.promises.authenticate(
+  let user
+  try {
+    user = (
+      await AuthenticationManager.promises.authenticate(
         { _id: userId },
         password,
         null,
         { enforceHIBPCheck: false }
       )
+    ).user
+  } catch (err) {
+    throw OError.tag(
+      err,
+      'error authenticating during attempt to delete account',
+      { userId }
+    )
+  }
+
   if (!user) {
     logger.err({ userId }, 'auth failed during attempt to delete account')
     return res.sendStatus(403)
@@ -265,10 +278,12 @@ async function tryDeleteUser(req, res, next) {
         errorData.info.public
       )
     } else {
-      throw err
+      throw OError.tag(err, errorData.message, errorData.info)
     }
   }
 
+  await Modules.promises.hooks.fire('tryDeleteV1Account', user)
+
   const sessionId = req.sessionID
 
   if (typeof req.logout === 'function') {

services/web/modules/launchpad/test/unit/src/LaunchpadControllerTests.js

@@ -696,7 +696,6 @@ describe('LaunchpadController', function () {
     describe('when overleaf', function () {
       beforeEach(function () {
         this.Settings.overleaf = { one: 1 }
-        this.Settings.createV1AccountOnLogin = true
         this._atLeastOneAdminExists.resolves(false)
         this.email = 'someone@example.com'
         this.password = 'a_really_bad_password'

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

@@ -133,6 +133,14 @@ describe('UserController', function () {
       promises: { expireAllTokensForUser: sinon.stub().resolves() },
     }
 
+    this.Modules = {
+      promises: {
+        hooks: {
+          fire: sinon.stub().resolves(),
+        },
+      },
+    }
+
     this.UserController = SandboxedModule.require(modulePath, {
       requires: {
         '../Helpers/UrlHelper': this.UrlHelper,
@@ -156,6 +164,7 @@ describe('UserController', function () {
         '../Security/OneTimeTokenHandler': this.OneTimeTokenHandler,
         '../../infrastructure/RequestContentTypeDetection':
           this.RequestContentTypeDetection,
+        '../../infrastructure/Modules': this.Modules,
       },
     })
 
@@ -215,6 +224,17 @@ describe('UserController', function () {
       this.UserController.tryDeleteUser(this.req, this.res, this.next)
     })
 
+    it('should call hook to try to delete v1 account', function (done) {
+      this.res.sendStatus = code => {
+        expect(this.Modules.promises.hooks.fire).to.have.been.calledWith(
+          'tryDeleteV1Account',
+          this.user
+        )
+        done()
+      }
+      this.UserController.tryDeleteUser(this.req, this.res, this.next)
+    })
+
     describe('when no password is supplied', function () {
       beforeEach(function () {
         this.req.body.password = ''
@@ -873,6 +893,7 @@ describe('UserController', function () {
           }
           this.EmailHandler.promises.sendEmail.rejects(anError)
         })
+
         it('should not return error but should log it', function (done) {
           this.res.json.callsFake(result => {
             expect(result.message.type).to.equal('success')
@@ -901,16 +922,19 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should not run affiliation check', function () {
         expect(this.UserGetter.promises.getUser).to.not.have.been.called
         expect(this.UserUpdater.promises.confirmEmail).to.not.have.been.called
         expect(this.UserUpdater.promises.addAffiliationForNewUser).to.not.have
           .been.called
       })
+
       it('should not return an error', function () {
         expect(this.next).to.be.calledWith()
       })
     })
+
     describe('without ensureAffiliation query parameter', function () {
       beforeEach(async function () {
         this.Features.hasFeature.withArgs('affiliations').returns(true)
@@ -920,16 +944,19 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should not run middleware', function () {
         expect(this.UserGetter.promises.getUser).to.not.have.been.called
         expect(this.UserUpdater.promises.confirmEmail).to.not.have.been.called
         expect(this.UserUpdater.promises.addAffiliationForNewUser).to.not.have
           .been.called
       })
+
       it('should not return an error', function () {
         expect(this.next).to.be.calledWith()
       })
     })
+
     describe('no flagged email', function () {
       beforeEach(async function () {
         const email = 'unit-test@overleaf.com'
@@ -947,19 +974,23 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should get the user', function () {
         expect(this.UserGetter.promises.getUser).to.have.been.calledWith(
           this.user._id
         )
       })
+
       it('should not try to add affiliation or update user', function () {
         expect(this.UserUpdater.promises.addAffiliationForNewUser).to.not.have
           .been.called
       })
+
       it('should not return an error', function () {
         expect(this.next).to.be.calledWith()
       })
     })
+
     describe('flagged non-SSO email', function () {
       let emailFlagged
       beforeEach(async function () {
@@ -980,11 +1011,13 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should check the user has permission', function () {
         expect(this.req.assertPermission).to.have.been.calledWith(
           'add-affiliation'
         )
       })
+
       it('should unflag the emails but not confirm', function () {
         expect(
           this.UserUpdater.promises.addAffiliationForNewUser
@@ -993,10 +1026,12 @@ describe('UserController', function () {
           this.UserUpdater.promises.confirmEmail
         ).to.not.have.been.calledWith(this.user._id, emailFlagged)
       })
+
       it('should not return an error', function () {
         expect(this.next).to.be.calledWith()
       })
     })
+
     describe('flagged SSO email', function () {
       let emailFlagged
       beforeEach(async function () {
@@ -1018,11 +1053,13 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should check the user has permission', function () {
         expect(this.req.assertPermission).to.have.been.calledWith(
           'add-affiliation'
         )
       })
+
       it('should add affiliation to v1, unflag and confirm on v2', function () {
         expect(this.UserUpdater.promises.addAffiliationForNewUser).to.have.not
           .been.called
@@ -1031,10 +1068,12 @@ describe('UserController', function () {
           emailFlagged
         )
       })
+
       it('should not return an error', function () {
         expect(this.next).to.be.calledWith()
       })
     })
+
     describe('when v1 returns an error', function () {
       let emailFlagged
       beforeEach(async function () {
@@ -1056,11 +1095,13 @@ describe('UserController', function () {
           this.next
         )
       })
+
       it('should check the user has permission', function () {
         expect(this.req.assertPermission).to.have.been.calledWith(
           'add-affiliation'
         )
       })
+
       it('should return the error', function () {
         expect(this.next).to.be.calledWith(sinon.match.instanceOf(Error))
       })