From f32f07cdcc12f279cf3b61794c3e36f1960373f8 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Thu, 18 Aug 2022 11:24:27 +0200
Subject: [PATCH] chore: Refactor user activation to command

---
 server/commands/userSuspender.ts        |  4 ++
 server/commands/userUnsuspender.test.ts | 46 +++++++++++++++++++++++
 server/commands/userUnsuspender.ts      | 49 +++++++++++++++++++++++++
 server/models/User.ts                   |  7 ----
 server/routes/api/users.ts              | 16 +++-----
 5 files changed, 105 insertions(+), 17 deletions(-)
 create mode 100644 server/commands/userUnsuspender.test.ts
 create mode 100644 server/commands/userUnsuspender.ts

diff --git a/server/commands/userSuspender.ts b/server/commands/userSuspender.ts
index fd10b4ef6..c2fe52039 100644
--- a/server/commands/userSuspender.ts
+++ b/server/commands/userSuspender.ts
@@ -10,6 +10,10 @@ type Props = {
   ip: string;
 };
 
+/**
+ * This command suspends an active user, this will cause them to lose access to
+ * the team.
+ */
 export default async function userSuspender({
   user,
   actorId,
diff --git a/server/commands/userUnsuspender.test.ts b/server/commands/userUnsuspender.test.ts
new file mode 100644
index 000000000..973ce4bf8
--- /dev/null
+++ b/server/commands/userUnsuspender.test.ts
@@ -0,0 +1,46 @@
+import { buildAdmin, buildUser } from "@server/test/factories";
+import { getTestDatabase } from "@server/test/support";
+import userUnsuspender from "./userUnsuspender";
+
+const db = getTestDatabase();
+
+afterAll(db.disconnect);
+
+beforeEach(db.flush);
+
+describe("userUnsuspender", () => {
+  const ip = "127.0.0.1";
+
+  it("should not allow unsuspending self", async () => {
+    const user = await buildUser();
+    let error;
+
+    try {
+      await userUnsuspender({
+        actorId: user.id,
+        user,
+        ip,
+      });
+    } catch (err) {
+      error = err;
+    }
+
+    expect(error.message).toEqual("Unable to unsuspend the current user");
+  });
+
+  it("should unsuspend the user", async () => {
+    const admin = await buildAdmin();
+    const user = await buildUser({
+      teamId: admin.teamId,
+      suspendedAt: new Date(),
+      suspendedById: admin.id,
+    });
+    await userUnsuspender({
+      actorId: admin.id,
+      user,
+      ip,
+    });
+    expect(user.suspendedAt).toEqual(null);
+    expect(user.suspendedById).toEqual(null);
+  });
+});
diff --git a/server/commands/userUnsuspender.ts b/server/commands/userUnsuspender.ts
new file mode 100644
index 000000000..c95aa9d3c
--- /dev/null
+++ b/server/commands/userUnsuspender.ts
@@ -0,0 +1,49 @@
+import { Transaction } from "sequelize";
+import { sequelize } from "@server/database/sequelize";
+import { User, Event } from "@server/models";
+import { ValidationError } from "../errors";
+
+type Props = {
+  user: User;
+  actorId: string;
+  ip: string;
+};
+
+/**
+ * This command unsuspends a previously suspended user, allowing access to the
+ * team again.
+ */
+export default async function userUnsuspender({
+  user,
+  actorId,
+  ip,
+}: Props): Promise<void> {
+  if (user.id === actorId) {
+    throw ValidationError("Unable to unsuspend the current user");
+  }
+
+  await sequelize.transaction(async (transaction: Transaction) => {
+    await user.update(
+      {
+        suspendedById: null,
+        suspendedAt: null,
+      },
+      { transaction }
+    );
+    await Event.create(
+      {
+        name: "users.activate",
+        actorId,
+        userId: user.id,
+        teamId: user.teamId,
+        data: {
+          name: user.name,
+        },
+        ip,
+      },
+      {
+        transaction,
+      }
+    );
+  });
+}
diff --git a/server/models/User.ts b/server/models/User.ts
index dd907c954..aae09cecb 100644
--- a/server/models/User.ts
+++ b/server/models/User.ts
@@ -438,13 +438,6 @@ class User extends ParanoidModel {
     });
   };
 
-  activate = () => {
-    return this.update({
-      suspendedById: null,
-      suspendedAt: null,
-    });
-  };
-
   // hooks
 
   @BeforeDestroy
diff --git a/server/routes/api/users.ts b/server/routes/api/users.ts
index 4dcae8d25..b408821ad 100644
--- a/server/routes/api/users.ts
+++ b/server/routes/api/users.ts
@@ -7,6 +7,7 @@ import userDemoter from "@server/commands/userDemoter";
 import userDestroyer from "@server/commands/userDestroyer";
 import userInviter from "@server/commands/userInviter";
 import userSuspender from "@server/commands/userSuspender";
+import userUnsuspender from "@server/commands/userUnsuspender";
 import { sequelize } from "@server/database/sequelize";
 import ConfirmUserDeleteEmail from "@server/emails/templates/ConfirmUserDeleteEmail";
 import InviteEmail from "@server/emails/templates/InviteEmail";
@@ -284,21 +285,16 @@ router.post("users.suspend", auth(), async (ctx) => {
 
 router.post("users.activate", auth(), async (ctx) => {
   const userId = ctx.body.id;
-  const teamId = ctx.state.user.teamId;
   const actor = ctx.state.user;
   assertPresent(userId, "id is required");
-  const user = await User.findByPk(userId);
+  const user = await User.findByPk(userId, {
+    rejectOnEmpty: true,
+  });
   authorize(actor, "activate", user);
 
-  await user.activate();
-  await Event.create({
-    name: "users.activate",
+  await userUnsuspender({
+    user,
     actorId: actor.id,
-    userId,
-    teamId,
-    data: {
-      name: user.name,
-    },
     ip: ctx.request.ip,
   });
   const includeDetails = can(actor, "readDetails", user);