diff --git a/packages/server/src/models/UserDeletionModel.ts b/packages/server/src/models/UserDeletionModel.ts index 63559bad0..748bbfb76 100644 --- a/packages/server/src/models/UserDeletionModel.ts +++ b/packages/server/src/models/UserDeletionModel.ts @@ -34,6 +34,11 @@ export default class UserDeletionModel extends BaseModel { return !!r; } + public async isDeletedOrBeingDeleted(userId: Uuid) { + const r: UserDeletion = await this.db(this.tableName).select(['id', 'start_time']).where('user_id', '=', userId).first(); + return !!r && !!r.start_time; + } + public async add(userId: Uuid, scheduledTime: number, options: AddOptions = null): Promise { options = { ...defaultAddOptions(), @@ -120,4 +125,10 @@ export default class UserDeletionModel extends BaseModel { return userIds; } + // Remove a user from the deletion queue, before it gets deleted. If it has + // already been deleted or if it's being deleted, no action is performed. + public async removeFromQueueByUserId(userId: Uuid) { + await this.db(this.tableName).where('user_id', '=', userId).andWhere('start_time', '=', 0).delete(); + } + } diff --git a/packages/server/src/models/UserModel.test.ts b/packages/server/src/models/UserModel.test.ts index 28a79f785..1af02ad67 100644 --- a/packages/server/src/models/UserModel.test.ts +++ b/packages/server/src/models/UserModel.test.ts @@ -5,6 +5,7 @@ import { betaUserDateRange, stripeConfig } from '../utils/stripe'; import { accountByType, AccountType } from './UserModel'; import { failedPaymentFinalAccount, failedPaymentWarningInterval } from './SubscriptionModel'; import { stripePortalUrl } from '../utils/urlUtils'; +import { Day } from '../utils/time'; describe('UserModel', function() { @@ -403,4 +404,54 @@ describe('UserModel', function() { expect(await models().userFlag().byUserId(user1.id, UserFlagType.AccountOverLimit)).toBeFalsy(); }); + test('should disable and enable users', async () => { + const { user: user1 } = await createUserAndSession(1); + const { user: user2 } = await createUserAndSession(2); + + jest.useFakeTimers('modern'); + + const t0 = new Date('2022-01-01').getTime(); + jest.setSystemTime(t0); + + await models().user().setEnabled(user1.id, false); + + expect((await models().user().load(user1.id)).enabled).toBe(0); + expect((await models().user().load(user2.id)).enabled).toBe(1); + + const t1 = new Date('2022-02-01').getTime(); + jest.setSystemTime(t1); + + // If we run the user deletion service at this point, it should add the + // disabled account + await models().userDeletion().autoAdd(10, 10 * Day, t1 + 3 * Day); + expect(await models().userDeletion().count()).toBe(1); + + // If we make the account enabled again, the user should be immediately + // removed from the queue + await models().user().setEnabled(user1.id, true); + expect(await models().userDeletion().count()).toBe(0); + + await models().user().setEnabled(user1.id, false); + + const t2 = new Date('2022-03-01').getTime(); + jest.setSystemTime(t2); + + // Should be added again + await models().userDeletion().autoAdd(10, 10 * Day, t2 + 3 * Day); + expect(await models().userDeletion().count()).toBe(1); + + const t3 = new Date('2022-04-01').getTime(); + jest.setSystemTime(t3); + + // Now if the service were to run, the user deletion would start and it + // should no longer be possible to remove it from the queue. And it + // shouldn't be possible to enable the user either. + const job = await models().userDeletion().next(); + expect(job.user_id).toBe(user1.id); + await models().userDeletion().start(job.id); + + await expectThrow(async () => models().user().setEnabled(user1.id, true)); + expect((await models().user().load(user1.id)).enabled).toBe(0); + }); + }); diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index 3062587bc..ea9492aad 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -618,6 +618,20 @@ export default class UserModel extends BaseModel { return syncInfo.ppk?.value || null; } + public async setEnabled(userId: string, enabled: boolean) { + const user = await this.load(userId); + if (!user) throw new ErrorNotFound(`Not found: ${userId}`); + + if (await this.models().userDeletion().isDeletedOrBeingDeleted(userId)) throw new ErrorBadRequest('User account is being deleted or already deleted and cannot be enabled again'); + + await this.models().userFlag().toggle(user.id, UserFlagType.ManuallyDisabled, !enabled); + + // If the user has been re-enabled, we want to remove it from the + // deletion queue (if it has been queued there) immediately, so that it + // doesn't incorrectly get deleted. + if (enabled) await this.models().userDeletion().removeFromQueueByUserId(userId); + } + // Note that when the "password" property is provided, it is going to be // hashed automatically. It means that it is not safe to do: // diff --git a/packages/server/src/routes/admin/users.ts b/packages/server/src/routes/admin/users.ts index 2860f38cb..900c21c58 100644 --- a/packages/server/src/routes/admin/users.ts +++ b/packages/server/src/routes/admin/users.ts @@ -339,9 +339,8 @@ router.post('admin/users', async (path: SubPath, ctx: AppContext) => { await stopImpersonating(ctx); return redirect(ctx, config().baseUrl); } else if (fields.disable_button || fields.restore_button) { - const user = await models.user().load(path.id); await models.user().checkIfAllowed(owner, AclAction.Delete, user); - await models.userFlag().toggle(user.id, UserFlagType.ManuallyDisabled, !fields.restore_button); + await models.user().setEnabled(path.id, !!fields.restore_button); } else if (fields.send_account_confirmation_email) { const user = await models.user().load(path.id); await models.user().save({ id: user.id, must_set_password: 1 });