fix: Email auth should allow same guest user on multiple subdomains (#2252)

* test: Add email auth tests to establish current state of system

* fix: Update logic to account for dupe emails used between subdomains

* test

* test

server/auth/providers/email.js

@@ -2,12 +2,15 @@
 import { subMinutes } from "date-fns";
 import Router from "koa-router";
 import { find } from "lodash";
+import { parseDomain, isCustomSubdomain } from "../../../shared/utils/domains";
 import { AuthorizationError } from "../../errors";
 import mailer, { sendEmail } from "../../mailer";
+import errorHandling from "../../middlewares/errorHandling";
 import methodOverride from "../../middlewares/methodOverride";
 import validation from "../../middlewares/validation";
 import { User, Team } from "../../models";
 import { signIn } from "../../utils/authentication";
+import { isCustomDomain } from "../../utils/domains";
 import { getUserForEmailSigninToken } from "../../utils/jwt";
 
 const router = new Router();
@@ -20,19 +23,45 @@ export const config = {
 router.use(methodOverride());
 router.use(validation());
 
-router.post("email", async (ctx) => {
+router.post("email", errorHandling(), async (ctx) => {
   const { email } = ctx.body;
 
   ctx.assertEmail(email, "email is required");
 
-  const user = await User.scope("withAuthentications").findOne({
+  const users = await User.scope("withAuthentications").findAll({
     where: { email: email.toLowerCase() },
   });
 
-  if (user) {
-    const team = await Team.scope("withAuthenticationProviders").findByPk(
-      user.teamId
-    );
+  if (users.length) {
+    let team;
+
+    if (isCustomDomain(ctx.request.hostname)) {
+      team = await Team.scope("withAuthenticationProviders").findOne({
+        where: { domain: ctx.request.hostname },
+      });
+    }
+
+    if (
+      process.env.SUBDOMAINS_ENABLED === "true" &&
+      isCustomSubdomain(ctx.request.hostname) &&
+      !isCustomDomain(ctx.request.hostname)
+    ) {
+      const domain = parseDomain(ctx.request.hostname);
+      const subdomain = domain ? domain.subdomain : undefined;
+      team = await Team.scope("withAuthenticationProviders").findOne({
+        where: { subdomain },
+      });
+    }
+
+    const user =
+      users.find((user) => team && user.teamId === team.id) || users[0];
+
+    if (!team) {
+      team = await Team.scope("withAuthenticationProviders").findByPk(
+        user.teamId
+      );
+    }
+
     if (!team) {
       ctx.redirect(`/?notice=auth-error`);
       return;

server/auth/providers/email.test.js

@@ -0,0 +1,156 @@
+// @flow
+import TestServer from "fetch-test-server";
+import app from "../../app";
+import mailer from "../../mailer";
+import { buildUser, buildGuestUser, buildTeam } from "../../test/factories";
+import { flushdb } from "../../test/support";
+
+const server = new TestServer(app.callback());
+
+jest.mock("../../mailer");
+
+beforeEach(async () => {
+  await flushdb();
+
+  // $FlowFixMe – does not understand Jest mocks
+  mailer.signin.mockReset();
+});
+afterAll(() => server.close());
+
+describe("email", () => {
+  it("should require email param", async () => {
+    const res = await server.post("/auth/email", {
+      body: {},
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(400);
+    expect(body.error).toEqual("validation_error");
+    expect(body.ok).toEqual(false);
+  });
+
+  it("should respond with redirect location when user is SSO enabled", async () => {
+    const user = await buildUser();
+
+    const res = await server.post("/auth/email", {
+      body: { email: user.email },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.redirect).toMatch("slack");
+    expect(mailer.signin).not.toHaveBeenCalled();
+  });
+
+  it("should respond with success when user is not SSO enabled", async () => {
+    const user = await buildGuestUser();
+
+    const res = await server.post("/auth/email", {
+      body: { email: user.email },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.success).toEqual(true);
+    expect(mailer.signin).toHaveBeenCalled();
+  });
+
+  it("should respond with success regardless of whether successful to prevent crawling email logins", async () => {
+    const res = await server.post("/auth/email", {
+      body: { email: "user@example.com" },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.success).toEqual(true);
+    expect(mailer.signin).not.toHaveBeenCalled();
+  });
+
+  describe("with multiple users matching email", () => {
+    it("should default to current subdomain with SSO", async () => {
+      process.env.URL = "http://localoutline.com";
+      process.env.SUBDOMAINS_ENABLED = "true";
+
+      const email = "sso-user@example.org";
+      const team = await buildTeam({
+        subdomain: "example",
+      });
+
+      await buildGuestUser({ email });
+      await buildUser({ email, teamId: team.id });
+
+      const res = await server.post("/auth/email", {
+        body: { email },
+        headers: { host: "example.localoutline.com" },
+      });
+      const body = await res.json();
+
+      expect(res.status).toEqual(200);
+      expect(body.redirect).toMatch("slack");
+      expect(mailer.signin).not.toHaveBeenCalled();
+    });
+
+    it("should default to current subdomain with guest email", async () => {
+      process.env.URL = "http://localoutline.com";
+      process.env.SUBDOMAINS_ENABLED = "true";
+
+      const email = "guest-user@example.org";
+      const team = await buildTeam({
+        subdomain: "example",
+      });
+
+      await buildUser({ email });
+      await buildGuestUser({ email, teamId: team.id });
+
+      const res = await server.post("/auth/email", {
+        body: { email },
+        headers: { host: "example.localoutline.com" },
+      });
+      const body = await res.json();
+
+      expect(res.status).toEqual(200);
+      expect(body.success).toEqual(true);
+      expect(mailer.signin).toHaveBeenCalled();
+    });
+
+    it("should default to custom domain with SSO", async () => {
+      const email = "sso-user-2@example.org";
+      const team = await buildTeam({
+        domain: "docs.mycompany.com",
+      });
+
+      await buildGuestUser({ email });
+      await buildUser({ email, teamId: team.id });
+
+      const res = await server.post("/auth/email", {
+        body: { email },
+        headers: { host: "docs.mycompany.com" },
+      });
+      const body = await res.json();
+
+      expect(res.status).toEqual(200);
+      expect(body.redirect).toMatch("slack");
+      expect(mailer.signin).not.toHaveBeenCalled();
+    });
+
+    it("should default to custom domain with guest email", async () => {
+      const email = "guest-user-2@example.org";
+      const team = await buildTeam({
+        domain: "docs.mycompany.com",
+      });
+
+      await buildUser({ email });
+      await buildGuestUser({ email, teamId: team.id });
+
+      const res = await server.post("/auth/email", {
+        body: { email },
+        headers: { host: "docs.mycompany.com" },
+      });
+      const body = await res.json();
+
+      expect(res.status).toEqual(200);
+      expect(body.success).toEqual(true);
+      expect(mailer.signin).toHaveBeenCalled();
+    });
+  });
+});

server/migrations/20170712055148-non-unique-email.js

@@ -1,15 +1,7 @@
 module.exports = {
   up: async (queryInterface, Sequelize) => {
-    await queryInterface.changeColumn('users', 'email', {
-      type: Sequelize.STRING,
-      unique: false,
-      allowNull: false,
-    });
-    await queryInterface.changeColumn('users', 'username', {
-      type: Sequelize.STRING,
-      unique: false,
-      allowNull: false,
-    });
+    await queryInterface.removeConstraint('users', 'users_email_key', {})
+    await queryInterface.removeConstraint('users', 'users_username_key', {})
   },
 
   down: async (queryInterface, Sequelize) => {

server/test/factories.js

@@ -68,6 +68,23 @@ export function buildEvent(overrides: Object = {}) {
   });
 }
 
+export async function buildGuestUser(overrides: Object = {}) {
+  if (!overrides.teamId) {
+    const team = await buildTeam();
+    overrides.teamId = team.id;
+  }
+
+  count++;
+
+  return User.create({
+    email: `user${count}@example.com`,
+    name: `User ${count}`,
+    createdAt: new Date("2018-01-01T00:00:00.000Z"),
+    lastActiveAt: new Date("2018-01-01T00:00:00.000Z"),
+    ...overrides,
+  });
+}
+
 export async function buildUser(overrides: Object = {}) {
   if (!overrides.teamId) {
     const team = await buildTeam();

server/test/support.js

@@ -3,17 +3,15 @@ import { v4 as uuidv4 } from "uuid";
 import { User, Document, Collection, Team } from "../models";
 import { sequelize } from "../sequelize";
 
-export function flushdb() {
-  const sql = sequelize.getQueryInterface();
-  const tables = Object.keys(sequelize.models).map((model) => {
-    const n = sequelize.models[model].getTableName();
-    return sql.queryGenerator.quoteTable(
-      typeof n === "string" ? n : n.tableName
-    );
-  });
+const sql = sequelize.getQueryInterface();
+const tables = Object.keys(sequelize.models).map((model) => {
+  const n = sequelize.models[model].getTableName();
+  return sql.queryGenerator.quoteTable(typeof n === "string" ? n : n.tableName);
+});
+const flushQuery = `TRUNCATE ${tables.join(", ")}`;
 
-  const query = `TRUNCATE ${tables.join(", ")} CASCADE`;
-  return sequelize.query(query);
+export function flushdb() {
+  return sequelize.query(flushQuery);
 }
 
 export const seed = async () => {