From ecd6959c8577856f24a9ef04cd7e7fc5c9150d9c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 28 Nov 2022 14:37:49 +0000 Subject: [PATCH] Merge pull request #10637 from overleaf/bg-delete-user-from-dropbox delete user data from dropbox GitOrigin-RevId: d586c73b4500f4fe718927f537ae770356eaefc1 --- .../web/app/src/Features/User/UserDeleter.js | 3 +++ .../web/app/src/infrastructure/Modules.js | 5 ++++ services/web/test/acceptance/src/Init.js | 2 ++ .../src/mocks/MockThirdPartyDataStoreApi.js | 23 +++++++++++++++++++ .../test/unit/src/User/UserDeleterTests.js | 21 +++++++++++++++++ 5 files changed, 54 insertions(+) create mode 100644 services/web/test/acceptance/src/mocks/MockThirdPartyDataStoreApi.js diff --git a/services/web/app/src/Features/User/UserDeleter.js b/services/web/app/src/Features/User/UserDeleter.js index 825329750c..b2efb350a8 100644 --- a/services/web/app/src/Features/User/UserDeleter.js +++ b/services/web/app/src/Features/User/UserDeleter.js @@ -12,6 +12,7 @@ const SubscriptionLocator = require('../Subscription/SubscriptionLocator') const UserMembershipsHandler = require('../UserMembership/UserMembershipsHandler') const UserSessionsManager = require('./UserSessionsManager') const InstitutionsAPI = require('../Institutions/InstitutionsAPI') +const Modules = require('../../infrastructure/Modules') const Errors = require('../Errors/Errors') module.exports = { @@ -42,6 +43,7 @@ async function deleteUser(userId, options = {}) { await ensureCanDeleteUser(user) await _cleanupUser(user) + await Modules.promises.hooks.fire('deleteUser', userId) await _createDeletedUser(user, options) await ProjectDeleter.promises.deleteUsersProjects(user._id) await deleteMongoUser(user._id) @@ -63,6 +65,7 @@ async function deleteMongoUser(userId) { } async function expireDeletedUser(userId) { + await Modules.promises.hooks.fire('expireDeletedUser', userId) const deletedUser = await DeletedUser.findOne({ 'deleterData.deletedUserId': userId, }).exec() diff --git a/services/web/app/src/infrastructure/Modules.js b/services/web/app/src/infrastructure/Modules.js index 73f65131af..112d051ee8 100644 --- a/services/web/app/src/infrastructure/Modules.js +++ b/services/web/app/src/infrastructure/Modules.js @@ -141,6 +141,11 @@ function attachHook(name, method) { } function fireHook(name, ...rest) { + // ensure that modules are loaded if we need to fire a hook + // this can happen if a script calls a method that fires a hook + if (!_modulesLoaded) { + loadModules() + } const adjustedLength = Math.max(rest.length, 1) const args = rest.slice(0, adjustedLength - 1) const callback = rest[adjustedLength - 1] diff --git a/services/web/test/acceptance/src/Init.js b/services/web/test/acceptance/src/Init.js index aa97cad7af..6bd267306d 100644 --- a/services/web/test/acceptance/src/Init.js +++ b/services/web/test/acceptance/src/Init.js @@ -13,6 +13,7 @@ const MockSpellingApi = require('./mocks/MockSpellingApi') const MockV1Api = require('./mocks/MockV1Api') const MockV1HistoryApi = require('./mocks/MockV1HistoryApi') const MockHaveIBeenPwnedApi = require('./mocks/MockHaveIBeenPwnedApi') +const MockThirdPartyDataStoreApi = require('./mocks/MockThirdPartyDataStoreApi') const mockOpts = { debug: ['1', 'true', 'TRUE'].includes(process.env.DEBUG_MOCKS), @@ -32,4 +33,5 @@ if (Features.hasFeature('saas')) { MockProjectHistoryApi.initialize(23054, mockOpts) MockV1Api.initialize(25000, mockOpts) MockV1HistoryApi.initialize(23100, mockOpts) + MockThirdPartyDataStoreApi.initialize(23002, mockOpts) } diff --git a/services/web/test/acceptance/src/mocks/MockThirdPartyDataStoreApi.js b/services/web/test/acceptance/src/mocks/MockThirdPartyDataStoreApi.js new file mode 100644 index 0000000000..ade81abe31 --- /dev/null +++ b/services/web/test/acceptance/src/mocks/MockThirdPartyDataStoreApi.js @@ -0,0 +1,23 @@ +const AbstractMockApi = require('./AbstractMockApi') + +class MockThirdPartyDataStoreApi extends AbstractMockApi { + reset() {} + + deleteUser(req, res) { + res.sendStatus(200) + } + + applyRoutes() { + this.app.delete('/user/:user_id', (req, res) => this.deleteUser(req, res)) + } +} + +module.exports = MockThirdPartyDataStoreApi + +// type hint for the inherited `instance` method +/** + * @function instance + * @memberOf MockThirdPartyDataStoreApi + * @static + * @returns {MockThirdPartyDataStoreApi} + */ diff --git a/services/web/test/unit/src/User/UserDeleterTests.js b/services/web/test/unit/src/User/UserDeleterTests.js index 3f118d6b2c..7680650008 100644 --- a/services/web/test/unit/src/User/UserDeleterTests.js +++ b/services/web/test/unit/src/User/UserDeleterTests.js @@ -88,6 +88,10 @@ describe('UserDeleter', function () { deleteMany: sinon.stub().returns({ exec: sinon.stub().resolves() }), } + this.Modules = { + promises: { hooks: { fire: sinon.stub().resolves() } }, + } + this.UserDeleter = SandboxedModule.require(modulePath, { requires: { '../../models/User': { User }, @@ -103,6 +107,7 @@ describe('UserDeleter', function () { '../../models/UserAuditLogEntry': { UserAuditLogEntry: this.UserAuditLogEntry, }, + '../../infrastructure/Modules': this.Modules, }, }) }) @@ -194,6 +199,14 @@ describe('UserDeleter', function () { ).to.have.been.calledWith(this.userId) }) + it('should fire the deleteUser hook for modules', async function () { + await this.UserDeleter.promises.deleteUser(this.userId) + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'deleteUser', + this.userId + ) + }) + it('should stop the user sessions', async function () { await this.UserDeleter.promises.deleteUser(this.userId) expect( @@ -488,6 +501,14 @@ describe('UserDeleter', function () { this.mockedDeletedUser.verify() }) + it('should fire the expireDeletedUser hook for modules', async function () { + await this.UserDeleter.promises.expireDeletedUser('giraffe') + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'expireDeletedUser', + 'giraffe' + ) + }) + describe('when called as a callback', function () { it('should expire the user', function (done) { this.UserDeleter.expireDeletedUser('giraffe', err => {