feat: Migrate allowedDomains to a Team Level Settings (#3489)

Fixes #3412

Previously the only way to restrict the domains for a Team were with the ALLOWED_DOMAINS environment variable for self hosted instances.
This PR migrates this to be a database backed setting on the Team object. This is done through the creation of a TeamDomain model that is associated with the Team and contains the domain name

This settings is updated on the Security Tab. Here domains can be added or removed from the Team.

On the server side, we take the code paths that previously were using ALLOWED_DOMAINS and switched them to use the Team allowed domains instead

server/commands/accountProvisioner.test.ts

@@ -1,8 +1,9 @@
 import WelcomeEmail from "@server/emails/templates/WelcomeEmail";
+import { TeamDomain } from "@server/models";
 import Collection from "@server/models/Collection";
 import UserAuthentication from "@server/models/UserAuthentication";
 import { buildUser, buildTeam } from "@server/test/factories";
-import { flushdb } from "@server/test/support";
+import { flushdb, seed } from "@server/test/support";
 import accountProvisioner from "./accountProvisioner";
 
 beforeEach(() => {
@@ -148,6 +149,100 @@ describe("accountProvisioner", () => {
     expect(error).toBeTruthy();
   });
 
+  it("should throw an error when the domain is not allowed", async () => {
+    const { admin, team: existingTeam } = await seed();
+    const providers = await existingTeam.$get("authenticationProviders");
+    const authenticationProvider = providers[0];
+
+    await TeamDomain.create({
+      teamId: existingTeam.id,
+      name: "other.com",
+      createdById: admin.id,
+    });
+
+    let error;
+
+    try {
+      await accountProvisioner({
+        ip,
+        user: {
+          name: "Jenny Tester",
+          email: "jenny@example.com",
+          avatarUrl: "https://example.com/avatar.png",
+          username: "jtester",
+        },
+        team: {
+          name: existingTeam.name,
+          avatarUrl: existingTeam.avatarUrl,
+          subdomain: "example",
+        },
+        authenticationProvider: {
+          name: authenticationProvider.name,
+          providerId: authenticationProvider.providerId,
+        },
+        authentication: {
+          providerId: "123456789",
+          accessToken: "123",
+          scopes: ["read"],
+        },
+      });
+    } catch (err) {
+      error = err;
+    }
+
+    expect(error).toBeTruthy();
+  });
+
+  it("should create a new user in an existing team when the domain is allowed", async () => {
+    const spy = jest.spyOn(WelcomeEmail, "schedule");
+    const { admin, team } = await seed();
+    const authenticationProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = authenticationProviders[0];
+    await TeamDomain.create({
+      teamId: team.id,
+      name: "example.com",
+      createdById: admin.id,
+    });
+
+    const { user, isNewUser } = await accountProvisioner({
+      ip,
+      user: {
+        name: "Jenny Tester",
+        email: "jenny@example.com",
+        avatarUrl: "https://example.com/avatar.png",
+        username: "jtester",
+      },
+      team: {
+        name: team.name,
+        avatarUrl: team.avatarUrl,
+        subdomain: "example",
+      },
+      authenticationProvider: {
+        name: authenticationProvider.name,
+        providerId: authenticationProvider.providerId,
+      },
+      authentication: {
+        providerId: "123456789",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    const authentications = await user.$get("authentications");
+    const auth = authentications[0];
+    expect(auth.accessToken).toEqual("123");
+    expect(auth.scopes.length).toEqual(1);
+    expect(auth.scopes[0]).toEqual("read");
+    expect(user.email).toEqual("jenny@example.com");
+    expect(user.username).toEqual("jtester");
+    expect(isNewUser).toEqual(true);
+    expect(spy).toHaveBeenCalled();
+    // should provision welcome collection
+    const collectionCount = await Collection.count();
+    expect(collectionCount).toEqual(1);
+
+    spy.mockRestore();
+  });
+
   it("should create a new user in an existing team", async () => {
     const spy = jest.spyOn(WelcomeEmail, "schedule");
     const team = await buildTeam();

server/commands/teamCreator.test.ts

@@ -1,4 +1,5 @@
-import { buildTeam } from "@server/test/factories";
+import TeamDomain from "@server/models/TeamDomain";
+import { buildTeam, buildUser } from "@server/test/factories";
 import { flushdb } from "@server/test/support";
 import teamCreator from "./teamCreator";
 
@@ -48,6 +49,15 @@ describe("teamCreator", () => {
   it("should return existing team when within allowed domains", async () => {
     delete process.env.DEPLOYMENT;
     const existing = await buildTeam();
+    const user = await buildUser({
+      teamId: existing.id,
+    });
+    await TeamDomain.create({
+      teamId: existing.id,
+      name: "allowed-domain.com",
+      createdById: user.id,
+    });
+
     const result = await teamCreator({
       name: "Updated name",
       subdomain: "example",
@@ -67,6 +77,34 @@ describe("teamCreator", () => {
     expect(providers.length).toEqual(2);
   });
 
+  it("should error when NOT within allowed domains", async () => {
+    const user = await buildUser();
+    delete process.env.DEPLOYMENT;
+    const existing = await buildTeam();
+    await TeamDomain.create({
+      teamId: existing.id,
+      name: "other-domain.com",
+      createdById: user.id,
+    });
+
+    let error;
+    try {
+      await teamCreator({
+        name: "Updated name",
+        subdomain: "example",
+        domain: "allowed-domain.com",
+        authenticationProvider: {
+          name: "google",
+          providerId: "allowed-domain.com",
+        },
+      });
+    } catch (err) {
+      error = err;
+    }
+
+    expect(error).toBeTruthy();
+  });
+
   it("should return exising team", async () => {
     delete process.env.DEPLOYMENT;
     const authenticationProvider = {

server/commands/teamCreator.ts

@@ -1,10 +1,9 @@
 import invariant from "invariant";
+import { DomainNotAllowedError, MaximumTeamsError } from "@server/errors";
 import Logger from "@server/logging/logger";
 import { APM } from "@server/logging/tracing";
 import { Team, AuthenticationProvider } from "@server/models";
-import { isDomainAllowed } from "@server/utils/authentication";
 import { generateAvatarUrl } from "@server/utils/avatars";
-import { MaximumTeamsError } from "../errors";
 
 type TeamCreatorResult = {
   team: Team;
@@ -60,19 +59,23 @@ async function teamCreator({
     // If the self-hosted installation has a single team and the domain for the
     // new team is allowed then assign the authentication provider to the
     // existing team
-    if (teamCount === 1 && domain && isDomainAllowed(domain)) {
+    if (teamCount === 1 && domain) {
       const team = await Team.findOne();
       invariant(team, "Team should exist");
 
-      authP = await team.$create<AuthenticationProvider>(
-        "authenticationProvider",
-        authenticationProvider
-      );
-      return {
-        authenticationProvider: authP,
-        team,
-        isNewTeam: false,
-      };
+      if (await team.isDomainAllowed(domain)) {
+        authP = await team.$create<AuthenticationProvider>(
+          "authenticationProvider",
+          authenticationProvider
+        );
+        return {
+          authenticationProvider: authP,
+          team,
+          isNewTeam: false,
+        };
+      } else {
+        throw DomainNotAllowedError();
+      }
     }
 
     if (teamCount >= 1) {

server/commands/teamUpdater.ts

@@ -1,9 +1,9 @@
 import { Transaction } from "sequelize";
 import { sequelize } from "@server/database/sequelize";
-import { Event, Team, User } from "@server/models";
+import { Event, Team, TeamDomain, User } from "@server/models";
 
 type TeamUpdaterProps = {
-  params: Partial<Team>;
+  params: Partial<Omit<Team, "allowedDomains">> & { allowedDomains?: string[] };
   ip?: string;
   user: User;
   team: Team;
@@ -22,8 +22,11 @@ const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
     defaultCollectionId,
     defaultUserRole,
     inviteRequired,
+    allowedDomains,
   } = params;
 
+  const transaction: Transaction = await sequelize.transaction();
+
   if (subdomain !== undefined && process.env.SUBDOMAINS_ENABLED === "true") {
     team.subdomain = subdomain === "" ? null : subdomain;
   }
@@ -58,11 +61,50 @@ const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
   if (inviteRequired !== undefined) {
     team.inviteRequired = inviteRequired;
   }
+  if (allowedDomains !== undefined) {
+    const existingAllowedDomains = await TeamDomain.findAll({
+      where: { teamId: team.id },
+      transaction,
+    });
+
+    // Only keep existing domains if they are still in the list of allowed domains
+    const newAllowedDomains = team.allowedDomains.filter((existingTeamDomain) =>
+      allowedDomains.includes(existingTeamDomain.name)
+    );
+
+    // Add new domains
+    const existingDomains = team.allowedDomains.map((x) => x.name);
+    const newDomains = allowedDomains.filter(
+      (newDomain) => newDomain !== "" && !existingDomains.includes(newDomain)
+    );
+    await Promise.all(
+      newDomains.map(async (newDomain) => {
+        newAllowedDomains.push(
+          await TeamDomain.create(
+            {
+              name: newDomain,
+              teamId: team.id,
+              createdById: user.id,
+            },
+            { transaction }
+          )
+        );
+      })
+    );
+
+    // Destroy the existing TeamDomains that were removed
+    const deletedDomains = existingAllowedDomains.filter(
+      (x) => !allowedDomains.includes(x.name)
+    );
+    for (const deletedDomain of deletedDomains) {
+      deletedDomain.destroy({ transaction });
+    }
+
+    team.allowedDomains = newAllowedDomains;
+  }
 
   const changes = team.changed();
 
-  const transaction: Transaction = await sequelize.transaction();
-
   try {
     const savedTeam = await team.save({
       transaction,

server/commands/userCreator.test.ts

@@ -1,5 +1,6 @@
+import { TeamDomain } from "@server/models";
 import { buildUser, buildTeam, buildInvite } from "@server/test/factories";
-import { flushdb } from "@server/test/support";
+import { flushdb, seed } from "@server/test/support";
 import userCreator from "./userCreator";
 
 beforeEach(() => flushdb());
@@ -239,4 +240,68 @@ describe("userCreator", () => {
       "You need an invite to join this team"
     );
   });
+
+  it("should create a user from allowed Domain", async () => {
+    const { admin, team } = await seed();
+    await TeamDomain.create({
+      teamId: team.id,
+      name: "example.com",
+      createdById: admin.id,
+    });
+
+    const authenticationProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = authenticationProviders[0];
+    const result = await userCreator({
+      name: "Test Name",
+      email: "user@example.com",
+      teamId: team.id,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: "fake-service-id",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    const { user, authentication, isNewUser } = result;
+    expect(authentication.accessToken).toEqual("123");
+    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication.scopes[0]).toEqual("read");
+    expect(user.email).toEqual("user@example.com");
+    expect(isNewUser).toEqual(true);
+  });
+
+  it("should reject an user when the domain is not allowed", async () => {
+    const { admin, team } = await seed();
+    await TeamDomain.create({
+      teamId: team.id,
+      name: "other.com",
+      createdById: admin.id,
+    });
+
+    const authenticationProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = authenticationProviders[0];
+    let error;
+
+    try {
+      await userCreator({
+        name: "Bad Domain User",
+        email: "user@example.com",
+        teamId: team.id,
+        ip,
+        authentication: {
+          authenticationProviderId: authenticationProvider.id,
+          providerId: "fake-service-id",
+          accessToken: "123",
+          scopes: ["read"],
+        },
+      });
+    } catch (err) {
+      error = err;
+    }
+
+    expect(error && error.toString()).toContain(
+      "The domain is not allowed for this team"
+    );
+  });
 });

server/commands/userCreator.ts

@@ -1,5 +1,5 @@
 import { Op } from "sequelize";
-import { InviteRequiredError } from "@server/errors";
+import { DomainNotAllowedError, InviteRequiredError } from "@server/errors";
 import { Event, Team, User, UserAuthentication } from "@server/models";
 
 type UserCreatorResult = {
@@ -145,7 +145,7 @@ export default async function userCreator({
 
   try {
     const team = await Team.findByPk(teamId, {
-      attributes: ["defaultUserRole", "inviteRequired"],
+      attributes: ["defaultUserRole", "inviteRequired", "id"],
       transaction,
     });
 
@@ -155,6 +155,13 @@ export default async function userCreator({
       throw InviteRequiredError();
     }
 
+    // If the team settings do not allow this domain,
+    // throw an error and fail user creation.
+    const domain = email.split("@")[1];
+    if (team && !(await team.isDomainAllowed(domain))) {
+      throw DomainNotAllowedError();
+    }
+
     const defaultUserRole = team?.defaultUserRole;
 
     const user = await User.create(