feat: allow external SSO methods to log into teams as long as emails match (#3813)

* wip

* wip

* fix comments

* better separation of conerns

* fix up tests

* fix semantics

* fixup tsc

* fix some tests

* the old semantics were easier to use

* add db:reset to scripts

* explicitly throw for unauthorized external authorization

* fix minor bug

* add additional tests for user creator and team creator

* yank the email matching logic out of teamcreator

* renaming

* fix type and test errors

* adds test to ensure that accountProvisioner works with email matching

* remove only

* fix comments

* recreate changes to allow self hosted to make teams

package.json

@@ -22,6 +22,7 @@
     "db:create-migration": "sequelize migration:create",
     "db:migrate": "sequelize db:migrate",
     "db:rollback": "sequelize db:migrate:undo",
+    "db:reset": "sequelize db:drop && sequelize db:create && sequelize db:migrate",
     "upgrade": "git fetch && git pull && yarn install && yarn heroku-postbuild",
     "test": "yarn test:app && yarn test:server",
     "test:app": "jest --config=app/.jestconfig.json --runInBand --forceExit",

server/commands/accountProvisioner.test.ts

@@ -108,6 +108,44 @@ describe("accountProvisioner", () => {
     spy.mockRestore();
   });
 
+  it("should allow authentication by email matching", async () => {
+    const existingTeam = await buildTeam();
+    const providers = await existingTeam.$get("authenticationProviders");
+    const authenticationProvider = providers[0];
+    const userWithoutAuth = await buildUser({
+      email: "email@example.com",
+      teamId: existingTeam.id,
+      authentications: [],
+    });
+
+    const { user, isNewUser, isNewTeam } = await accountProvisioner({
+      ip,
+      user: {
+        name: userWithoutAuth.name,
+        email: "email@example.com",
+        avatarUrl: userWithoutAuth.avatarUrl,
+      },
+      team: {
+        name: existingTeam.name,
+        avatarUrl: existingTeam.avatarUrl,
+        subdomain: "example",
+      },
+      authenticationProvider: {
+        name: authenticationProvider.name,
+        providerId: authenticationProvider.providerId,
+      },
+      authentication: {
+        providerId: "anything",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    expect(user.id).toEqual(userWithoutAuth.id);
+    expect(isNewTeam).toEqual(false);
+    expect(isNewUser).toEqual(false);
+    expect(user.authentications.length).toEqual(0);
+  });
+
   it("should throw an error when authentication provider is disabled", async () => {
     const existingTeam = await buildTeam();
     const providers = await existingTeam.$get("authenticationProviders");

server/commands/accountProvisioner.ts

@@ -8,9 +8,9 @@ import {
   AuthenticationProviderDisabledError,
 } from "@server/errors";
 import { APM } from "@server/logging/tracing";
-import { Collection, Team, User } from "@server/models";
+import { AuthenticationProvider, Collection, Team, User } from "@server/models";
-import teamCreator from "./teamCreator";
+import teamProvisioner from "./teamProvisioner";
-import userCreator from "./userCreator";
+import userProvisioner from "./userProvisioner";
 
 type Props = {
   ip: string;
@@ -21,7 +21,7 @@ type Props = {
     username?: string;
   };
   team: {
-    id?: string;
+    teamId?: string;
     name: string;
     domain?: string;
     subdomain: string;
@@ -55,15 +55,45 @@ async function accountProvisioner({
   authentication: authenticationParams,
 }: Props): Promise<AccountProvisionerResult> {
   let result;
+  let emailMatchOnly;
 
   try {
-    result = await teamCreator({
+    result = await teamProvisioner({
       ...teamParams,
       authenticationProvider: authenticationProviderParams,
       ip,
     });
   } catch (err) {
-    throw InvalidAuthenticationError(err.message);
+    // The account could not be provisioned for the provided teamId
+    // check to see if we can try authentication using email matching only
+    if (err.id === "invalid_authentication") {
+      const authenticationProvider = await AuthenticationProvider.findOne({
+        where: {
+          name: authenticationProviderParams.name, // example: "google"
+          teamId: teamParams.teamId,
+        },
+        include: [
+          {
+            model: Team,
+            as: "team",
+            required: true,
+          },
+        ],
+      });
+
+      if (authenticationProvider) {
+        emailMatchOnly = true;
+        result = {
+          authenticationProvider,
+          team: authenticationProvider.team,
+          isNewTeam: false,
+        };
+      }
+    }
+
+    if (!result) {
+      throw InvalidAuthenticationError(err.message);
+    }
   }
 
   invariant(result, "Team creator result must exist");
@@ -74,20 +104,21 @@ async function accountProvisioner({
   }
 
   try {
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: userParams.name,
       email: userParams.email,
       username: userParams.username,
       isAdmin: isNewTeam || undefined,
       avatarUrl: userParams.avatarUrl,
       teamId: team.id,
+      emailMatchOnly,
       ip,
       authentication: {
+        authenticationProviderId: authenticationProvider.id,
         ...authenticationParams,
         expiresAt: authenticationParams.expiresIn
           ? new Date(Date.now() + authenticationParams.expiresIn * 1000)
           : undefined,
-        authenticationProviderId: authenticationProvider.id,
       },
     });
     const { isNewUser, user } = result;

server/commands/teamProvisioner.test.ts

@@ -2,16 +2,16 @@ import env from "@server/env";
 import TeamDomain from "@server/models/TeamDomain";
 import { buildTeam, buildUser } from "@server/test/factories";
 import { flushdb } from "@server/test/support";
-import teamCreator from "./teamCreator";
+import teamProvisioner from "./teamProvisioner";
 
 beforeEach(() => flushdb());
 
-describe("teamCreator", () => {
+describe("teamProvisioner", () => {
   const ip = "127.0.0.1";
 
   it("should create team and authentication provider", async () => {
     env.DEPLOYMENT = "hosted";
-    const result = await teamCreator({
+    const result = await teamProvisioner({
       name: "Test team",
       subdomain: "example",
       avatarUrl: "http://example.com/logo.png",
@@ -35,7 +35,7 @@ describe("teamCreator", () => {
     await buildTeam({
       subdomain: "myteam",
     });
-    const result = await teamCreator({
+    const result = await teamProvisioner({
       name: "Test team",
       subdomain: "myteam",
       avatarUrl: "http://example.com/logo.png",
@@ -58,7 +58,7 @@ describe("teamCreator", () => {
     await buildTeam({
       subdomain: "myteam1",
     });
-    const result = await teamCreator({
+    const result = await teamProvisioner({
       name: "Test team",
       subdomain: "myteam",
       avatarUrl: "http://example.com/logo.png",
@@ -72,10 +72,63 @@ describe("teamCreator", () => {
     expect(result.team.subdomain).toEqual("myteam2");
   });
 
+  it("should return existing team", async () => {
+    env.DEPLOYMENT = "hosted";
+    const authenticationProvider = {
+      name: "google",
+      providerId: "example.com",
+    };
+    const existing = await buildTeam({
+      subdomain: "example",
+      authenticationProviders: [authenticationProvider],
+    });
+    const result = await teamProvisioner({
+      name: "Updated name",
+      subdomain: "example",
+      authenticationProvider,
+      ip,
+    });
+    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);
+  });
+
+  it("should error on mismatched team and authentication provider", async () => {
+    env.DEPLOYMENT = "hosted";
+    const exampleTeam = await buildTeam({
+      subdomain: "example",
+      authenticationProviders: [
+        {
+          name: "google",
+          providerId: "example.com",
+        },
+      ],
+    });
+
+    let error;
+    try {
+      await teamProvisioner({
+        teamId: exampleTeam.id,
+        name: "name",
+        subdomain: "other",
+        authenticationProvider: {
+          name: "google",
+          providerId: "other.com",
+        },
+        ip,
+      });
+    } catch (e) {
+      error = e;
+    }
+    expect(error.id).toEqual("invalid_authentication");
+  });
+
   describe("self hosted", () => {
     it("should allow creating first team", async () => {
       env.DEPLOYMENT = undefined;
-      const { team, isNewTeam } = await teamCreator({
+      const { team, isNewTeam } = await teamProvisioner({
         name: "Test team",
         subdomain: "example",
         avatarUrl: "http://example.com/logo.png",
@@ -96,7 +149,7 @@ describe("teamCreator", () => {
       let error;
 
       try {
-        await teamCreator({
+        await teamProvisioner({
           name: "Test team",
           subdomain: "example",
           avatarUrl: "http://example.com/logo.png",
@@ -124,7 +177,7 @@ describe("teamCreator", () => {
         name: "allowed-domain.com",
         createdById: user.id,
       });
-      const result = await teamCreator({
+      const result = await teamProvisioner({
         name: "Updated name",
         subdomain: "example",
         domain: "allowed-domain.com",
@@ -158,7 +211,7 @@ describe("teamCreator", () => {
 
       let error;
       try {
-        await teamCreator({
+        await teamProvisioner({
           name: "Updated name",
           subdomain: "example",
           domain: "other-domain.com",
@@ -175,7 +228,7 @@ describe("teamCreator", () => {
       expect(error).toBeTruthy();
     });
 
-    it("should return exising team", async () => {
+    it("should return existing team", async () => {
       env.DEPLOYMENT = undefined;
       const authenticationProvider = {
         name: "google",
@@ -185,7 +238,7 @@ describe("teamCreator", () => {
         subdomain: "example",
         authenticationProviders: [authenticationProvider],
       });
-      const result = await teamCreator({
+      const result = await teamProvisioner({
         name: "Updated name",
         subdomain: "example",
         authenticationProvider,

server/commands/teamProvisioner.ts

@@ -1,8 +1,8 @@
 import { sequelize } from "@server/database/sequelize";
 import env from "@server/env";
 import {
-  InvalidAuthenticationError,
   DomainNotAllowedError,
+  InvalidAuthenticationError,
   MaximumTeamsError,
 } from "@server/errors";
 import Logger from "@server/logging/Logger";
@@ -10,14 +10,14 @@ import { APM } from "@server/logging/tracing";
 import { Team, AuthenticationProvider, Event } from "@server/models";
 import { generateAvatarUrl } from "@server/utils/avatars";
 
-type TeamCreatorResult = {
+type TeamProvisionerResult = {
   team: Team;
   authenticationProvider: AuthenticationProvider;
   isNewTeam: boolean;
 };
 
 type Props = {
-  id?: string;
+  teamId?: string;
   name: string;
   domain?: string;
   subdomain: string;
@@ -29,18 +29,18 @@ type Props = {
   ip: string;
 };
 
-async function teamCreator({
+async function teamProvisioner({
-  id,
+  teamId,
   name,
   domain,
   subdomain,
   avatarUrl,
   authenticationProvider,
   ip,
-}: Props): Promise<TeamCreatorResult> {
+}: Props): Promise<TeamProvisionerResult> {
   let authP = await AuthenticationProvider.findOne({
-    where: id
+    where: teamId
-      ? { ...authenticationProvider, teamId: id }
+      ? { ...authenticationProvider, teamId }
       : authenticationProvider,
     include: [
       {
@@ -59,11 +59,9 @@ async function teamCreator({
       team: authP.team,
       isNewTeam: false,
     };
-  }
+  } else if (teamId) {
-  // A team id was provided but no auth provider was found matching those credentials
+    // The user is attempting to log into a team with an unfamiliar SSO provider
-  // The user is attempting to log into a team with an incorrect SSO - fail the login
+    throw InvalidAuthenticationError();
-  else if (id) {
-    throw InvalidAuthenticationError("incorrect authentication credentials");
   }
 
   // This team has never been seen before, if self hosted the logic is different
@@ -176,5 +174,5 @@ async function provisionSubdomain(team: Team, requestedSubdomain: string) {
 
 export default APM.traceFunction({
   serviceName: "command",
-  spanName: "teamCreator",
+  spanName: "teamProvisioner",
-})(teamCreator);
+})(teamProvisioner);

server/commands/userProvisioner.test.ts

@@ -2,20 +2,20 @@ import { v4 as uuidv4 } from "uuid";
 import { TeamDomain } from "@server/models";
 import { buildUser, buildTeam, buildInvite } from "@server/test/factories";
 import { flushdb, seed } from "@server/test/support";
-import userCreator from "./userCreator";
+import userProvisioner from "./userProvisioner";
 
 beforeEach(() => flushdb());
 
-describe("userCreator", () => {
+describe("userProvisioner", () => {
   const ip = "127.0.0.1";
 
-  it("should update exising user and authentication", async () => {
+  it("should update existing user and authentication", async () => {
     const existing = await buildUser();
     const authentications = await existing.$get("authentications");
     const existingAuth = authentications[0];
     const newEmail = "test@example.com";
     const newUsername = "tname";
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: existing.name,
       email: newEmail,
       username: newUsername,
@@ -30,9 +30,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
     expect(user.email).toEqual(newEmail);
     expect(user.username).toEqual(newUsername);
     expect(isNewUser).toEqual(false);
@@ -50,7 +51,7 @@ describe("userCreator", () => {
       authentications: [],
     });
 
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: existing.name,
       email,
       username: "new-username",
@@ -65,9 +66,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
 
     const authentications = await user.$get("authentications");
     expect(authentications.length).toEqual(1);
@@ -85,7 +87,7 @@ describe("userCreator", () => {
       teamId: team.id,
     });
 
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: existing.name,
       email,
       username: "new-username",
@@ -100,9 +102,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
 
     const authentications = await user.$get("authentications");
     expect(authentications.length).toEqual(1);
@@ -115,7 +118,7 @@ describe("userCreator", () => {
     const existingAuth = authentications[0];
     const newEmail = "test@example.com";
     await existing.destroy();
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: "Test Name",
       email: "test@example.com",
       teamId: existing.teamId,
@@ -128,9 +131,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
     expect(user.email).toEqual(newEmail);
     expect(isNewUser).toEqual(true);
   });
@@ -142,13 +146,13 @@ describe("userCreator", () => {
     let error;
 
     try {
-      await userCreator({
+      await userProvisioner({
         name: "Test Name",
         email: "test@example.com",
         teamId: existing.teamId,
         ip,
         authentication: {
-          authenticationProviderId: "example.org",
+          authenticationProviderId: uuidv4(),
           providerId: existingAuth.providerId,
           accessToken: "123",
           scopes: ["read"],
@@ -165,7 +169,7 @@ describe("userCreator", () => {
     const team = await buildTeam();
     const authenticationProviders = await team.$get("authenticationProviders");
     const authenticationProvider = authenticationProviders[0];
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: "Test Name",
       email: "test@example.com",
       username: "tname",
@@ -179,9 +183,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
     expect(user.email).toEqual("test@example.com");
     expect(user.username).toEqual("tname");
     expect(user.isAdmin).toEqual(false);
@@ -195,7 +200,7 @@ describe("userCreator", () => {
     });
     const authenticationProviders = await team.$get("authenticationProviders");
     const authenticationProvider = authenticationProviders[0];
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: "Test Name",
       email: "test@example.com",
       username: "tname",
@@ -219,7 +224,7 @@ describe("userCreator", () => {
     });
     const authenticationProviders = await team.$get("authenticationProviders");
     const authenticationProvider = authenticationProviders[0];
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: "Test Name",
       email: "test@example.com",
       username: "tname",
@@ -236,7 +241,7 @@ describe("userCreator", () => {
     expect(tname.username).toEqual("tname");
     expect(tname.isAdmin).toEqual(false);
     expect(tname.isViewer).toEqual(true);
-    const tname2Result = await userCreator({
+    const tname2Result = await userProvisioner({
       name: "Test2 Name",
       email: "tes2@example.com",
       username: "tname2",
@@ -264,7 +269,7 @@ describe("userCreator", () => {
     });
     const authenticationProviders = await team.$get("authenticationProviders");
     const authenticationProvider = authenticationProviders[0];
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: invite.name,
       email: "invite@ExamPle.com",
       teamId: invite.teamId,
@@ -277,13 +282,46 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
     expect(user.email).toEqual(invite.email);
     expect(isNewUser).toEqual(true);
   });
 
+  it("should create a user from an invited user using email match", async () => {
+    const externalUser = await buildUser({
+      email: "external@example.com",
+    });
+
+    const team = await buildTeam({ inviteRequired: true });
+    const invite = await buildInvite({
+      teamId: team.id,
+      email: externalUser.email,
+    });
+
+    const authenticationProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = authenticationProviders[0];
+    const result = await userProvisioner({
+      name: invite.name,
+      email: "external@ExamPle.com", // ensure that email is case insensistive
+      teamId: invite.teamId,
+      emailMatchOnly: true,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: "whatever",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    const { user, authentication, isNewUser } = result;
+    expect(authentication).toEqual(null);
+    expect(user.id).toEqual(invite.id);
+    expect(isNewUser).toEqual(true);
+  });
+
   it("should reject an uninvited user when invites are required", async () => {
     const team = await buildTeam({ inviteRequired: true });
 
@@ -292,7 +330,7 @@ describe("userCreator", () => {
     let error;
 
     try {
-      await userCreator({
+      await userProvisioner({
         name: "Uninvited User",
         email: "invite@ExamPle.com",
         teamId: team.id,
@@ -323,7 +361,7 @@ describe("userCreator", () => {
 
     const authenticationProviders = await team.$get("authenticationProviders");
     const authenticationProvider = authenticationProviders[0];
-    const result = await userCreator({
+    const result = await userProvisioner({
       name: "Test Name",
       email: "user@example-company.com",
       teamId: team.id,
@@ -336,9 +374,10 @@ describe("userCreator", () => {
       },
     });
     const { user, authentication, isNewUser } = result;
-    expect(authentication.accessToken).toEqual("123");
+    expect(authentication).toBeDefined();
-    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication?.accessToken).toEqual("123");
-    expect(authentication.scopes[0]).toEqual("read");
+    expect(authentication?.scopes.length).toEqual(1);
+    expect(authentication?.scopes[0]).toEqual("read");
     expect(user.email).toEqual("user@example-company.com");
     expect(isNewUser).toEqual(true);
   });
@@ -356,7 +395,7 @@ describe("userCreator", () => {
     let error;
 
     try {
-      await userCreator({
+      await userProvisioner({
         name: "Bad Domain User",
         email: "user@example.com",
         teamId: team.id,

server/commands/userProvisioner.ts

@@ -1,12 +1,16 @@
 import { sequelize } from "@server/database/sequelize";
 import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail";
-import { DomainNotAllowedError, InviteRequiredError } from "@server/errors";
+import {
+  DomainNotAllowedError,
+  InvalidAuthenticationError,
+  InviteRequiredError,
+} from "@server/errors";
 import { Event, Team, User, UserAuthentication } from "@server/models";
 
-type UserCreatorResult = {
+type UserProvisionerResult = {
   user: User;
   isNewUser: boolean;
-  authentication: UserAuthentication;
+  authentication: UserAuthentication | null;
 };
 
 type Props = {
@@ -16,6 +20,7 @@ type Props = {
   isAdmin?: boolean;
   avatarUrl?: string | null;
   teamId: string;
+  emailMatchOnly?: boolean;
   ip: string;
   authentication: {
     authenticationProviderId: string;
@@ -27,17 +32,18 @@ type Props = {
   };
 };
 
-export default async function userCreator({
+export default async function userProvisioner({
   name,
   email,
   username,
   isAdmin,
+  emailMatchOnly,
   avatarUrl,
   teamId,
   authentication,
   ip,
-}: Props): Promise<UserCreatorResult> {
+}: Props): Promise<UserProvisionerResult> {
-  const { authenticationProviderId, providerId, ...rest } = authentication;
+  const { providerId, authenticationProviderId, ...rest } = authentication;
 
   const auth = await UserAuthentication.findOne({
     where: {
@@ -87,9 +93,8 @@ export default async function userCreator({
     await auth.destroy();
   }
 
-  // A `user` record might exist in the form of an invite even if there is no
+  // A `user` record may exist even if there is no existing authentication record.
-  // existing authentication record that matches. In Outline an invite is a
+  // This is either an invite or a user that's external to the team
-  // shell user record.
   const existingUser = await User.scope([
     "withAuthentications",
     "withTeam",
@@ -102,12 +107,11 @@ export default async function userCreator({
     },
   });
 
-  // We have an existing invite for his user, so we need to update it with our
+  // We have an existing user, so we need to update it with our
-  // new details, link up the authentication method, and count this as a new
+  // new details and count this as a new user creation.
-  // user creation.
   if (existingUser) {
     // A `user` record might exist in the form of an invite.
-    // In Outline an invite is a shell user record with no authentication method
+    // An invite is a shell user record with no authentication method
     // that's never been active before.
     const isInvite = existingUser.isInvited;
 
@@ -145,6 +149,12 @@ export default async function userCreator({
         }
       );
 
+      // We don't want to associate a user auth with the auth provider
+      // if we're doing a simple email match, so early return here
+      if (emailMatchOnly) {
+        return null;
+      }
+
       return await existingUser.$create<UserAuthentication>(
         "authentication",
         authentication,
@@ -171,9 +181,16 @@ export default async function userCreator({
       authentication: auth,
       isNewUser: isInvite,
     };
+  } else if (emailMatchOnly) {
+    // There's no existing invite or user that matches the external auth email
+    // This is simply unauthorized
+    throw InvalidAuthenticationError();
   }
 
+  //
   // No auth, no user – this is an entirely new sign in.
+  //
+
   const transaction = await User.sequelize!.transaction();
 
   try {

server/routes/auth/providers/azure.ts

@@ -4,6 +4,7 @@ import jwt from "jsonwebtoken";
 import type { Context } from "koa";
 import Router from "koa-router";
 import { Profile } from "passport";
+import { slugifyDomain } from "@shared/utils/domains";
 import accountProvisioner, {
   AccountProvisionerResult,
 } from "@server/commands/accountProvisioner";
@@ -95,12 +96,13 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) {
         const team = await getTeamFromContext(ctx);
 
         const domain = email.split("@")[1];
-        const subdomain = domain.split(".")[0];
+        const subdomain = slugifyDomain(domain);
+
         const teamName = organization.displayName;
         const result = await accountProvisioner({
           ip: ctx.ip,
           team: {
-            id: team?.id,
+            teamId: team?.id,
             name: teamName,
             domain,
             subdomain,

server/routes/auth/providers/google.ts

@@ -11,7 +11,6 @@ import accountProvisioner, {
 import env from "@server/env";
 import {
   GmailAccountCreationError,
-  InviteRequiredError,
   TeamDomainRequiredError,
 } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
@@ -19,7 +18,7 @@ import { User } from "@server/models";
 import { StateStore, getTeamFromContext } from "@server/utils/passport";
 
 const router = new Router();
-const providerName = "google";
+const GOOGLE = "google";
 const scopes = [
   "https://www.googleapis.com/auth/userinfo.profile",
   "https://www.googleapis.com/auth/userinfo.email",
@@ -34,7 +33,7 @@ type GoogleProfile = Profile & {
   email: string;
   picture: string;
   _json: {
-    hd: string;
+    hd?: string;
   };
 };
 
@@ -63,87 +62,66 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) {
         ) => void
       ) {
         try {
-          let result;
-
           // "domain" is the Google Workspaces domain
           const domain = profile._json.hd;
           const team = await getTeamFromContext(ctx);
 
-          // Existence of domain means this is a Google Workspaces account
+          // No profile domain means personal gmail account
-          // so we'll attempt to provision an account (team and user)
+          // No team implies the request came from the apex domain
-          if (domain) {
+          // This combination is always an error
-            // remove the TLD and form a subdomain from the remaining
+          if (!domain && !team) {
-            // subdomains of the form "foo.bar.com" are allowed as primary Google Workspaces domains
+            const userExists = await User.count({
-            // see https://support.google.com/nonprofits/thread/19685140/using-a-subdomain-as-a-primary-domain
+              where: { email: profile.email.toLowerCase() },
-            const subdomain = slugifyDomain(domain);
-            const teamName = capitalize(subdomain);
-
-            // Request a larger size profile picture than the default by tweaking
-            // the query parameter.
-            const avatarUrl = profile.picture.replace("=s96-c", "=s128-c");
-
-            // if a team can be inferred, we assume the user is only interested in signing into
-            // that team in particular; otherwise, we will do a best effort at finding their account
-            // or provisioning a new one (within AccountProvisioner)
-            result = await accountProvisioner({
-              ip: ctx.ip,
-              team: {
-                id: team?.id,
-                name: teamName,
-                domain,
-                subdomain,
-              },
-              user: {
-                email: profile.email,
-                name: profile.displayName,
-                avatarUrl,
-              },
-              authenticationProvider: {
-                name: providerName,
-                providerId: domain,
-              },
-              authentication: {
-                providerId: profile.id,
-                accessToken,
-                refreshToken,
-                expiresIn: params.expires_in,
-                scopes,
-              },
-            });
-          } else {
-            // No domain means it's a personal Gmail account
-            // We only allow sign-in to existing user accounts with these
-            if (!team) {
-              // No team usually means this is the apex domain
-              // Throw different errors depending on whether we think the user is
-              // trying to create a new account, or log-in to an existing one
-              const userExists = await User.count({
-                where: { email: profile.email.toLowerCase() },
-              });
-
-              if (!userExists) {
-                throw GmailAccountCreationError();
-              }
-
-              throw TeamDomainRequiredError();
-            }
-
-            const user = await User.findOne({
-              where: { teamId: team.id, email: profile.email.toLowerCase() },
             });
 
-            if (!user) {
+            // Users cannot create a team with personal gmail accounts
-              throw InviteRequiredError();
+            if (!userExists) {
+              throw GmailAccountCreationError();
             }
 
-            result = {
+            // To log-in with a personal account, users must specify a team subdomain
-              user,
+            throw TeamDomainRequiredError();
-              team,
-              isNewUser: false,
-              isNewTeam: false,
-            };
           }
 
+          // remove the TLD and form a subdomain from the remaining
+          // subdomains of the form "foo.bar.com" are allowed as primary Google Workspaces domains
+          // see https://support.google.com/nonprofits/thread/19685140/using-a-subdomain-as-a-primary-domain
+          const subdomain = domain ? slugifyDomain(domain) : "";
+          const teamName = capitalize(subdomain);
+
+          // Request a larger size profile picture than the default by tweaking
+          // the query parameter.
+          const avatarUrl = profile.picture.replace("=s96-c", "=s128-c");
+
+          // if a team can be inferred, we assume the user is only interested in signing into
+          // that team in particular; otherwise, we will do a best effort at finding their account
+          // or provisioning a new one (within AccountProvisioner)
+          const result = await accountProvisioner({
+            ip: ctx.ip,
+            team: {
+              teamId: team?.id,
+              name: teamName,
+              domain,
+              subdomain,
+            },
+            user: {
+              email: profile.email,
+              name: profile.displayName,
+              avatarUrl,
+            },
+            authenticationProvider: {
+              name: GOOGLE,
+              providerId: domain ?? "",
+            },
+            authentication: {
+              providerId: profile.id,
+              accessToken,
+              refreshToken,
+              expiresIn: params.expires_in,
+              scopes,
+            },
+          });
+
           return done(null, result.user, result);
         } catch (err) {
           return done(err, null);
@@ -154,13 +132,13 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) {
 
   router.get(
     "google",
-    passport.authenticate(providerName, {
+    passport.authenticate(GOOGLE, {
       accessType: "offline",
       prompt: "select_account consent",
     })
   );
 
-  router.get("google.callback", passportMiddleware(providerName));
+  router.get("google.callback", passportMiddleware(GOOGLE));
 }
 
 export default router;

server/routes/auth/providers/oidc.ts

@@ -97,7 +97,7 @@ if (env.OIDC_CLIENT_ID && env.OIDC_CLIENT_SECRET) {
           const result = await accountProvisioner({
             ip: ctx.ip,
             team: {
-              id: team?.id,
+              teamId: team?.id,
               // https://github.com/outline/outline/pull/2388#discussion_r681120223
               name: "Wiki",
               domain,

server/routes/auth/providers/slack.ts

@@ -79,7 +79,7 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
         const result = await accountProvisioner({
           ip: ctx.ip,
           team: {
-            id: team?.id,
+            teamId: team?.id,
             name: profile.team.name,
             subdomain: profile.team.domain,
             avatarUrl: profile.team.image_230,