fix: Add 10 domain limit per team (#3733)

* fix: Validate team domains are FQDN's
Add 10 domain limit per team
fix: Deletion of domains not happening within request lifecycle

* tests

* docs

server/commands/teamUpdater.ts

@@ -97,9 +97,7 @@ const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
     const deletedDomains = existingAllowedDomains.filter(
       (x) => !allowedDomains.includes(x.name)
     );
-    for (const deletedDomain of deletedDomains) {
-      deletedDomain.destroy({ transaction });
-    }
+    await Promise.all(deletedDomains.map((x) => x.destroy({ transaction })));
 
     team.allowedDomains = newAllowedDomains;
   }

server/models/TeamDomain.test.ts

@@ -0,0 +1,72 @@
+import { buildAdmin, buildTeam } from "@server/test/factories";
+import { flushdb } from "@server/test/support";
+import TeamDomain from "./TeamDomain";
+
+beforeEach(() => flushdb());
+
+describe("team domain model", () => {
+  describe("create", () => {
+    it("should allow creation of domains", async () => {
+      const team = await buildTeam();
+      const user = await buildAdmin({ teamId: team.id });
+      const domain = await TeamDomain.create({
+        teamId: team.id,
+        name: "getoutline.com",
+        createdById: user.id,
+      });
+
+      expect(domain.name).toEqual("getoutline.com");
+    });
+
+    it("should not allow junk domains", async () => {
+      const team = await buildTeam();
+      const user = await buildAdmin({ teamId: team.id });
+
+      let error;
+      try {
+        await TeamDomain.create({
+          teamId: team.id,
+          name: "sdfsdf",
+          createdById: user.id,
+        });
+      } catch (err) {
+        error = err;
+      }
+      expect(error).toBeDefined();
+    });
+
+    it("should not allow creation of domains within restricted list", async () => {
+      const team = await buildTeam();
+      const user = await buildAdmin({ teamId: team.id });
+
+      let error;
+      try {
+        await TeamDomain.create({
+          teamId: team.id,
+          name: "gmail.com",
+          createdById: user.id,
+        });
+      } catch (err) {
+        error = err;
+      }
+      expect(error).toBeDefined();
+    });
+
+    it("should ignore casing and spaces when creating domains", async () => {
+      const team = await buildTeam();
+      const user = await buildAdmin({ teamId: team.id });
+
+      let error;
+      try {
+        await TeamDomain.create({
+          teamId: team.id,
+          name: "   GMail.com   ",
+          createdById: user.id,
+        });
+      } catch (err) {
+        error = err;
+      }
+      expect(error).toBeDefined();
+    });
+  });
+});

server/models/TeamDomain.ts

@@ -6,11 +6,16 @@ import {
   ForeignKey,
   NotEmpty,
   NotIn,
+  BeforeValidate,
+  BeforeCreate,
 } from "sequelize-typescript";
+import { MAX_TEAM_DOMAINS } from "@shared/constants";
+import { ValidationError } from "@server/errors";
 import Team from "./Team";
 import User from "./User";
 import IdModel from "./base/IdModel";
 import Fix from "./decorators/Fix";
+import IsFQDN from "./validators/IsFQDN";
 import Length from "./validators/Length";
 
 @Table({ tableName: "team_domains", modelName: "team_domain" })
@@ -22,6 +27,7 @@ class TeamDomain extends IdModel {
   })
   @NotEmpty
   @Length({ min: 0, max: 255, msg: "Must be less than 255 characters" })
+  @IsFQDN
   @Column
   name: string;
 
@@ -40,6 +46,25 @@ class TeamDomain extends IdModel {
   @ForeignKey(() => User)
   @Column
   createdById: string;
+
+  // hooks
+
+  @BeforeValidate
+  static async cleanupDomain(model: TeamDomain) {
+    model.name = model.name.toLowerCase().trim();
+  }
+
+  @BeforeCreate
+  static async checkLimit(model: TeamDomain) {
+    const count = await this.count({
+      where: { teamId: model.teamId },
+    });
+    if (count >= MAX_TEAM_DOMAINS) {
+      throw ValidationError(
+        `You have reached the limit of ${MAX_TEAM_DOMAINS} domains`
+      );
+    }
+  }
 }
 
 export default TeamDomain;

server/models/validators/IsFQDN.ts

@@ -0,0 +1,17 @@
+import { isFQDN } from "class-validator";
+import { addAttributeOptions } from "sequelize-typescript";
+
+/**
+ * A decorator that validates that a string is a fully qualified domain name.
+ */
+export default function IsFQDN(target: any, propertyName: string) {
+  return addAttributeOptions(target, propertyName, {
+    validate: {
+      validDomain(value: string) {
+        if (!isFQDN(value)) {
+          throw new Error("Must be a fully qualified domain name");
+        }
+      },
+    },
+  });
+}

shared/constants.ts

@@ -3,3 +3,5 @@ export const USER_PRESENCE_INTERVAL = 5000;
 export const MAX_AVATAR_DISPLAY = 6;
 
 export const MAX_TITLE_LENGTH = 100;
+
+export const MAX_TEAM_DOMAINS = 10;