fix: Self-hosted should show signin options for all configured authentication methods (#2986)

server/commands/accountProvisioner.test.ts

@@ -1,4 +1,5 @@
 import WelcomeEmail from "@server/emails/templates/WelcomeEmail";
+import env from "@server/env";
 import { TeamDomain } from "@server/models";
 import Collection from "@server/models/Collection";
 import UserAuthentication from "@server/models/UserAuthentication";
@@ -14,6 +15,8 @@ describe("accountProvisioner", () => {
   const ip = "127.0.0.1";
 
   it("should create a new user and team", async () => {
+    env.DEPLOYMENT = "hosted";
+
     const spy = jest.spyOn(WelcomeEmail, "schedule");
     const { user, team, isNewTeam, isNewUser } = await accountProvisioner({
       ip,
@@ -286,4 +289,81 @@ describe("accountProvisioner", () => {
 
     spy.mockRestore();
   });
+
+  describe("self hosted", () => {
+    it("should fail if existing team and domain not in allowed list", async () => {
+      env.DEPLOYMENT = undefined;
+      let error;
+      const team = await buildTeam();
+
+      try {
+        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: "google",
+            providerId: "example.com",
+          },
+          authentication: {
+            providerId: "123456789",
+            accessToken: "123",
+            scopes: ["read"],
+          },
+        });
+      } catch (err) {
+        error = err;
+      }
+
+      expect(error.message).toEqual(
+        "The maximum number of teams has been reached"
+      );
+    });
+
+    it("should always use existing team if self-hosted", async () => {
+      env.DEPLOYMENT = undefined;
+
+      const team = await buildTeam();
+      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",
+          domain: "allowed-domain.com",
+        },
+        authenticationProvider: {
+          name: "google",
+          providerId: "allowed-domain.com",
+        },
+        authentication: {
+          providerId: "123456789",
+          accessToken: "123",
+          scopes: ["read"],
+        },
+      });
+
+      expect(user.teamId).toEqual(team.id);
+      expect(user.username).toEqual("jtester");
+      expect(isNewUser).toEqual(true);
+
+      const providers = await team.$get("authenticationProviders");
+      expect(providers.length).toEqual(2);
+    });
+  });
 });

server/commands/teamCreator.test.ts

@@ -8,6 +8,7 @@ beforeEach(() => flushdb());
 
 describe("teamCreator", () => {
   it("should create team and authentication provider", async () => {
+    env.DEPLOYMENT = "hosted";
     const result = await teamCreator({
       name: "Test team",
       subdomain: "example",
@@ -25,72 +26,41 @@ describe("teamCreator", () => {
     expect(isNewTeam).toEqual(true);
   });
 
-  it("should not allow creating multiple teams in installation", async () => {
-    env.DEPLOYMENT = undefined;
-    await buildTeam();
-    let error;
+  describe("self hosted", () => {
+    it("should not allow creating multiple teams in installation", async () => {
+      env.DEPLOYMENT = undefined;
+      await buildTeam();
+      let error;
 
-    try {
-      await teamCreator({
-        name: "Test team",
-        subdomain: "example",
-        avatarUrl: "http://example.com/logo.png",
-        authenticationProvider: {
-          name: "google",
-          providerId: "example.com",
-        },
+      try {
+        await teamCreator({
+          name: "Test team",
+          subdomain: "example",
+          avatarUrl: "http://example.com/logo.png",
+          authenticationProvider: {
+            name: "google",
+            providerId: "example.com",
+          },
+        });
+      } catch (err) {
+        error = err;
+      }
+
+      expect(error).toBeTruthy();
+    });
+
+    it("should return existing team when within allowed domains", async () => {
+      env.DEPLOYMENT = undefined;
+      const existing = await buildTeam();
+      const user = await buildUser({
+        teamId: existing.id,
       });
-    } catch (err) {
-      error = err;
-    }
-
-    expect(error).toBeTruthy();
-  });
-
-  it("should return existing team when within allowed domains", async () => {
-    env.DEPLOYMENT = undefined;
-    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",
-      domain: "allowed-domain.com",
-      authenticationProvider: {
-        name: "google",
-        providerId: "allowed-domain.com",
-      },
-    });
-    const { team, authenticationProvider, isNewTeam } = result;
-    expect(team.id).toEqual(existing.id);
-    expect(team.name).toEqual(existing.name);
-    expect(authenticationProvider.name).toEqual("google");
-    expect(authenticationProvider.providerId).toEqual("allowed-domain.com");
-    expect(isNewTeam).toEqual(false);
-    const providers = await team.$get("authenticationProviders");
-    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({
+      await TeamDomain.create({
+        teamId: existing.id,
+        name: "allowed-domain.com",
+        createdById: user.id,
+      });
+      const result = await teamCreator({
         name: "Updated name",
         subdomain: "example",
         domain: "allowed-domain.com",
@@ -99,32 +69,66 @@ describe("teamCreator", () => {
           providerId: "allowed-domain.com",
         },
       });
-    } catch (err) {
-      error = err;
-    }
-
-    expect(error).toBeTruthy();
-  });
-
-  it("should return exising team", async () => {
-    env.DEPLOYMENT = undefined;
-    const authenticationProvider = {
-      name: "google",
-      providerId: "example.com",
-    };
-    const existing = await buildTeam({
-      subdomain: "example",
-      authenticationProviders: [authenticationProvider],
+      const { team, authenticationProvider, isNewTeam } = result;
+      expect(team.id).toEqual(existing.id);
+      expect(team.name).toEqual(existing.name);
+      expect(authenticationProvider.name).toEqual("google");
+      expect(authenticationProvider.providerId).toEqual("allowed-domain.com");
+      expect(isNewTeam).toEqual(false);
+      const providers = await team.$get("authenticationProviders");
+      expect(providers.length).toEqual(2);
     });
-    const result = await teamCreator({
-      name: "Updated name",
-      subdomain: "example",
-      authenticationProvider,
+
+    it("should error when NOT within allowed domains", async () => {
+      env.DEPLOYMENT = undefined;
+      const existing = await buildTeam();
+      const user = await buildUser({
+        teamId: existing.id,
+      });
+      await TeamDomain.create({
+        teamId: existing.id,
+        name: "allowed-domain.com",
+        createdById: user.id,
+      });
+
+      let error;
+      try {
+        await teamCreator({
+          name: "Updated name",
+          subdomain: "example",
+          domain: "other-domain.com",
+          authenticationProvider: {
+            name: "google",
+            providerId: "other-domain.com",
+          },
+        });
+      } catch (err) {
+        error = err;
+      }
+
+      expect(error).toBeTruthy();
+    });
+
+    it("should return exising team", async () => {
+      env.DEPLOYMENT = undefined;
+      const authenticationProvider = {
+        name: "google",
+        providerId: "example.com",
+      };
+      const existing = await buildTeam({
+        subdomain: "example",
+        authenticationProviders: [authenticationProvider],
+      });
+      const result = await teamCreator({
+        name: "Updated name",
+        subdomain: "example",
+        authenticationProvider,
+      });
+      const { team, isNewTeam } = result;
+      expect(team.id).toEqual(existing.id);
+      expect(team.name).toEqual(existing.name);
+      expect(team.subdomain).toEqual("example");
+      expect(isNewTeam).toEqual(false);
     });
-    const { team, isNewTeam } = result;
-    expect(team.id).toEqual(existing.id);
-    expect(team.name).toEqual(existing.name);
-    expect(team.subdomain).toEqual("example");
-    expect(isNewTeam).toEqual(false);
   });
 });

server/commands/teamCreator.ts

@@ -1,4 +1,3 @@
-import invariant from "invariant";
 import env from "@server/env";
 import { DomainNotAllowedError, MaximumTeamsError } from "@server/errors";
 import Logger from "@server/logging/Logger";
@@ -55,15 +54,12 @@ async function teamCreator({
   // to the multi-tenant version, we want to restrict to a single team that MAY
   // have multiple authentication providers
   if (env.DEPLOYMENT !== "hosted") {
-    const teamCount = await Team.count();
+    const team = await Team.findOne();
 
     // 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) {
-      const team = await Team.findOne();
-      invariant(team, "Team should exist");
-
+    if (team && domain) {
       if (await team.isDomainAllowed(domain)) {
         authP = await team.$create<AuthenticationProvider>(
           "authenticationProvider",
@@ -79,9 +75,7 @@ async function teamCreator({
       }
     }
 
-    if (teamCount >= 1) {
-      throw MaximumTeamsError();
-    }
+    throw MaximumTeamsError();
   }
 
   // If the service did not provide a logo/avatar then we attempt to generate