From e6ceb314cbcc764036c19ce21103e135e7bbf481 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Fri, 10 Jan 2025 09:57:32 +0100 Subject: [PATCH] Merge pull request #22321 from overleaf/msm-force-flag-delete-user [web] Fix user deletion in CE/SP when email does not exist GitOrigin-RevId: 051f822318d63c8a9f50d5e9aeca095f3b37efb4 --- .../web/app/src/Features/User/UserDeleter.js | 17 +++++-- .../server-ce-scripts/scripts/delete-user.mjs | 1 + .../test/unit/src/User/UserDeleterTests.js | 49 ++++++++++++++----- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/services/web/app/src/Features/User/UserDeleter.js b/services/web/app/src/Features/User/UserDeleter.js index 22d90f7b31..2ead415a54 100644 --- a/services/web/app/src/Features/User/UserDeleter.js +++ b/services/web/app/src/Features/User/UserDeleter.js @@ -56,7 +56,7 @@ async function deleteUser(userId, options) { ) await _createDeletedUser(user, options) await ProjectDeleter.promises.deleteUsersProjects(user._id) - await _sendDeleteEmail(user) + await _sendDeleteEmail(user, options.force) await deleteMongoUser(user._id) } catch (error) { logger.warn({ error, userId }, 'something went wrong deleting the user') @@ -119,13 +119,24 @@ async function ensureCanDeleteUser(user) { } } -async function _sendDeleteEmail(user) { +async function _sendDeleteEmail(user, force) { const emailOptions = { to: user.email, action: 'account deleted', actionDescribed: 'your Overleaf account was deleted', } - await EmailHandler.promises.sendEmail('securityAlert', emailOptions) + try { + await EmailHandler.promises.sendEmail('securityAlert', emailOptions) + } catch (error) { + if (force) { + logger.error( + { error }, + 'error sending account deletion email notification' + ) + } else { + throw error + } + } } async function _createDeletedUser(user, options) { diff --git a/services/web/modules/server-ce-scripts/scripts/delete-user.mjs b/services/web/modules/server-ce-scripts/scripts/delete-user.mjs index 6b1f4c3fe5..9b7b4592a3 100644 --- a/services/web/modules/server-ce-scripts/scripts/delete-user.mjs +++ b/services/web/modules/server-ce-scripts/scripts/delete-user.mjs @@ -24,6 +24,7 @@ async function main() { } const options = { ipAddress: '0.0.0.0', + force: true, } UserDeleter.deleteUser(user._id, options, function (err) { if (err) { diff --git a/services/web/test/unit/src/User/UserDeleterTests.js b/services/web/test/unit/src/User/UserDeleterTests.js index 6489fea035..fd800de7aa 100644 --- a/services/web/test/unit/src/User/UserDeleterTests.js +++ b/services/web/test/unit/src/User/UserDeleterTests.js @@ -299,18 +299,45 @@ describe('UserDeleter', function () { this.DeletedUserMock.verify() }) - it('should email the user', async function () { - await this.UserDeleter.promises.deleteUser(this.userId, { - ipAddress: this.ipAddress, + describe('email notifications', function () { + it('should email the user', async function () { + await this.UserDeleter.promises.deleteUser(this.userId, { + ipAddress: this.ipAddress, + }) + const emailOptions = { + to: 'bob@bob.com', + action: 'account deleted', + actionDescribed: 'your Overleaf account was deleted', + } + expect( + this.EmailHandler.promises.sendEmail + ).to.have.been.calledWith('securityAlert', emailOptions) + }) + + it('should fail when the email service fails', async function () { + this.EmailHandler.promises.sendEmail = sinon + .stub() + .rejects(new Error('email failed')) + await expect( + this.UserDeleter.promises.deleteUser(this.userId, { + ipAddress: this.ipAddress, + }) + ).to.be.rejectedWith('email failed') + }) + + describe('with "force: true" option', function () { + it('should succeed when the email service fails', async function () { + this.EmailHandler.promises.sendEmail = sinon + .stub() + .rejects(new Error('email failed')) + await expect( + this.UserDeleter.promises.deleteUser(this.userId, { + ipAddress: this.ipAddress, + force: true, + }) + ).to.be.fulfilled + }) }) - const emailOptions = { - to: 'bob@bob.com', - action: 'account deleted', - actionDescribed: 'your Overleaf account was deleted', - } - expect( - this.EmailHandler.promises.sendEmail - ).to.have.been.calledWith('securityAlert', emailOptions) }) it('should add an audit log entry', async function () {