From 67a6b3fe4314061c1bd2a585bf81ecb65385c0c8 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom@getoutline.com>
Date: Wed, 3 Jan 2024 06:14:10 -0800
Subject: [PATCH] fix: Cleanup relationships when user is deleted (#6343)

* fix: Cleanup relationships when user is deleted

* Update tests

* Update User.test.ts
---
 server/models/User.test.ts                    | 19 +------
 server/models/User.ts                         | 20 -------
 .../processors/UserDeletedProcessor.test.ts   | 35 ++++++++++++
 .../queues/processors/UserDeletedProcessor.ts | 56 +++++++++++++++++++
 4 files changed, 94 insertions(+), 36 deletions(-)
 create mode 100644 server/queues/processors/UserDeletedProcessor.test.ts
 create mode 100644 server/queues/processors/UserDeletedProcessor.ts

diff --git a/server/models/User.test.ts b/server/models/User.test.ts
index 5e0c378ad..c8e2b533e 100644
--- a/server/models/User.test.ts
+++ b/server/models/User.test.ts
@@ -1,7 +1,6 @@
 import { faker } from "@faker-js/faker";
 import { CollectionPermission } from "@shared/types";
 import { buildUser, buildTeam, buildCollection } from "@server/test/factories";
-import UserAuthentication from "./UserAuthentication";
 import UserPermission from "./UserPermission";
 
 beforeAll(() => {
@@ -36,23 +35,11 @@ describe("user model", () => {
   });
 
   describe("destroy", () => {
-    it("should delete user authentications", async () => {
+    it("should clear PII", async () => {
       const user = await buildUser();
-      expect(
-        await UserAuthentication.count({
-          where: {
-            userId: user.id,
-          },
-        })
-      ).toBe(1);
       await user.destroy();
-      expect(
-        await UserAuthentication.count({
-          where: {
-            userId: user.id,
-          },
-        })
-      ).toBe(0);
+      expect(user.email).toBe(null);
+      expect(user.name).toBe("Unknown");
     });
   });
 
diff --git a/server/models/User.ts b/server/models/User.ts
index 8388f17d9..bfa8afdfc 100644
--- a/server/models/User.ts
+++ b/server/models/User.ts
@@ -43,11 +43,9 @@ import env from "@server/env";
 import DeleteAttachmentTask from "@server/queues/tasks/DeleteAttachmentTask";
 import parseAttachmentIds from "@server/utils/parseAttachmentIds";
 import { ValidationError } from "../errors";
-import ApiKey from "./ApiKey";
 import Attachment from "./Attachment";
 import AuthenticationProvider from "./AuthenticationProvider";
 import Collection from "./Collection";
-import Star from "./Star";
 import Team from "./Team";
 import UserAuthentication from "./UserAuthentication";
 import UserPermission from "./UserPermission";
@@ -586,24 +584,6 @@ class User extends ParanoidModel {
     model: User,
     options: { transaction: Transaction }
   ) => {
-    await ApiKey.destroy({
-      where: {
-        userId: model.id,
-      },
-      transaction: options.transaction,
-    });
-    await Star.destroy({
-      where: {
-        userId: model.id,
-      },
-      transaction: options.transaction,
-    });
-    await UserAuthentication.destroy({
-      where: {
-        userId: model.id,
-      },
-      transaction: options.transaction,
-    });
     model.email = null;
     model.name = "Unknown";
     model.avatarUrl = null;
diff --git a/server/queues/processors/UserDeletedProcessor.test.ts b/server/queues/processors/UserDeletedProcessor.test.ts
new file mode 100644
index 000000000..8c1267ac4
--- /dev/null
+++ b/server/queues/processors/UserDeletedProcessor.test.ts
@@ -0,0 +1,35 @@
+import UserAuthentication from "@server/models/UserAuthentication";
+import { buildUser } from "@server/test/factories";
+import UserDeletedProcessor from "./UserDeletedProcessor";
+
+const ip = "127.0.0.1";
+
+describe("UserDeletedProcessor", () => {
+  test("should remove relationships", async () => {
+    const user = await buildUser();
+    expect(
+      await UserAuthentication.count({
+        where: {
+          userId: user.id,
+        },
+      })
+    ).toBe(1);
+
+    const processor = new UserDeletedProcessor();
+    await processor.perform({
+      name: "users.delete",
+      userId: user.id,
+      actorId: user.id,
+      teamId: user.teamId,
+      ip,
+    });
+
+    expect(
+      await UserAuthentication.count({
+        where: {
+          userId: user.id,
+        },
+      })
+    ).toBe(0);
+  });
+});
diff --git a/server/queues/processors/UserDeletedProcessor.ts b/server/queues/processors/UserDeletedProcessor.ts
new file mode 100644
index 000000000..b767307ea
--- /dev/null
+++ b/server/queues/processors/UserDeletedProcessor.ts
@@ -0,0 +1,56 @@
+import {
+  ApiKey,
+  GroupUser,
+  Star,
+  Subscription,
+  UserAuthentication,
+  UserPermission,
+} from "@server/models";
+import { sequelize } from "@server/storage/database";
+import { Event as TEvent, UserEvent } from "@server/types";
+import BaseProcessor from "./BaseProcessor";
+
+export default class UsersDeletedProcessor extends BaseProcessor {
+  static applicableEvents: TEvent["name"][] = ["users.delete"];
+
+  async perform(event: UserEvent) {
+    await sequelize.transaction(async (transaction) => {
+      await GroupUser.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+      await UserAuthentication.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+      await UserPermission.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+      await Subscription.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+      await ApiKey.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+      await Star.destroy({
+        where: {
+          userId: event.userId,
+        },
+        transaction,
+      });
+    });
+  }
+}