Server: When re-activating an account, remove it from the deletion queue

packages/server/src/models/UserDeletionModel.ts

@@ -34,6 +34,11 @@ export default class UserDeletionModel extends BaseModel<UserDeletion> {
 		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<UserDeletion> {
 		options = {
 			...defaultAddOptions(),
@@ -120,4 +125,10 @@ export default class UserDeletionModel extends BaseModel<UserDeletion> {
 		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();
+	}
+
 }

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);
+	});
+
 });

packages/server/src/models/UserModel.ts

@@ -618,6 +618,20 @@ export default class UserModel extends BaseModel<User> {
 		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:
 	//

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 });