fix: Allow soft deletion of teams (#1754)

* fix: Allow soft deletion of teams

* test: regression specs

server/api/auth.test.js

@@ -0,0 +1,43 @@
+/* eslint-disable flowtype/require-valid-file-annotation */
+import TestServer from "fetch-test-server";
+import app from "../app";
+import { buildUser, buildTeam } from "../test/factories";
+import { flushdb } from "../test/support";
+
+const server = new TestServer(app.callback());
+
+beforeEach(() => flushdb());
+afterAll(() => server.close());
+
+describe("#auth.info", () => {
+  it("should return current authentication", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+
+    const res = await server.post("/api/auth.info", {
+      body: { token: user.getJwtToken() },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.user.name).toBe(user.name);
+    expect(body.data.team.name).toBe(team.name);
+  });
+
+  it("should require the team to not be deleted", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+
+    await team.destroy();
+
+    const res = await server.post("/api/auth.info", {
+      body: { token: user.getJwtToken() },
+    });
+    expect(res.status).toEqual(401);
+  });
+
+  it("should require authentication", async () => {
+    const res = await server.post("/api/auth.info");
+    expect(res.status).toEqual(401);
+  });
+});

server/auth/email.js

@@ -25,6 +25,10 @@ router.post("email", async (ctx) => {
 
   if (user) {
     const team = await Team.findByPk(user.teamId);
+    if (!team) {
+      ctx.redirect(`/?notice=auth-error`);
+      return;
+    }
 
     // If the user matches an email address associated with an SSO
     // signin then just forward them directly to that service's

server/auth/google.js

@@ -1,6 +1,7 @@
 // @flow
 import crypto from "crypto";
 import { OAuth2Client } from "google-auth-library";
+import invariant from "invariant";
 import Router from "koa-router";
 import { capitalize } from "lodash";
 import Sequelize from "sequelize";
@@ -68,15 +69,24 @@ router.get("google.callback", auth({ required: false }), async (ctx) => {
   const cbResponse = await fetch(cbUrl);
   const avatarUrl = cbResponse.status === 200 ? cbUrl : tileyUrl;
 
-  const [team, isFirstUser] = await Team.findOrCreate({
-    where: {
-      googleId,
-    },
-    defaults: {
-      name: teamName,
-      avatarUrl,
-    },
-  });
+  let team, isFirstUser;
+  try {
+    [team, isFirstUser] = await Team.findOrCreate({
+      where: {
+        googleId,
+      },
+      defaults: {
+        name: teamName,
+        avatarUrl,
+      },
+    });
+  } catch (err) {
+    if (err instanceof Sequelize.UniqueConstraintError) {
+      ctx.redirect(`/?notice=auth-error`);
+      return;
+    }
+  }
+  invariant(team, "Team must exist");
 
   try {
     const [user, isFirstSignin] = await User.findOrCreate({

server/auth/slack.js

@@ -1,5 +1,6 @@
 // @flow
 import addHours from "date-fns/add_hours";
+import invariant from "invariant";
 import Router from "koa-router";
 import Sequelize from "sequelize";
 import { slackAuth } from "../../shared/utils/routeHelpers";
@@ -40,15 +41,24 @@ router.get("slack.callback", auth({ required: false }), async (ctx) => {
 
   const data = await Slack.oauthAccess(code);
 
-  const [team, isFirstUser] = await Team.findOrCreate({
-    where: {
-      slackId: data.team.id,
-    },
-    defaults: {
-      name: data.team.name,
-      avatarUrl: data.team.image_88,
-    },
-  });
+  let team, isFirstUser;
+  try {
+    [team, isFirstUser] = await Team.findOrCreate({
+      where: {
+        slackId: data.team.id,
+      },
+      defaults: {
+        name: data.team.name,
+        avatarUrl: data.team.image_88,
+      },
+    });
+  } catch (err) {
+    if (err instanceof Sequelize.UniqueConstraintError) {
+      ctx.redirect(`/?notice=auth-error`);
+      return;
+    }
+  }
+  invariant(team, "Team must exist");
 
   try {
     const [user, isFirstSignin] = await User.findOrCreate({

server/middlewares/authentication.test.js

@@ -1,8 +1,8 @@
 /* eslint-disable flowtype/require-valid-file-annotation */
 import randomstring from "randomstring";
 import { ApiKey } from "../models";
-import { buildUser } from "../test/factories";
-import { flushdb, seed } from "../test/support";
+import { buildUser, buildTeam } from "../test/factories";
+import { flushdb } from "../test/support";
 import auth from "./authentication";
 
 beforeEach(() => flushdb());
@@ -11,7 +11,7 @@ describe("Authentication middleware", () => {
   describe("with JWT", () => {
     it("should authenticate with correct token", async () => {
       const state = {};
-      const { user } = await seed();
+      const user = await buildUser();
       const authMiddleware = auth();
 
       await authMiddleware(
@@ -29,7 +29,7 @@ describe("Authentication middleware", () => {
 
     it("should return error with invalid token", async () => {
       const state = {};
-      const { user } = await seed();
+      const user = await buildUser();
       const authMiddleware = auth();
 
       try {
@@ -52,7 +52,7 @@ describe("Authentication middleware", () => {
   describe("with API key", () => {
     it("should authenticate user with valid API key", async () => {
       const state = {};
-      const { user } = await seed();
+      const user = await buildUser();
       const authMiddleware = auth();
       const key = await ApiKey.create({
         userId: user.id,
@@ -116,7 +116,7 @@ describe("Authentication middleware", () => {
 
   it("should allow passing auth token as a GET param", async () => {
     const state = {};
-    const { user } = await seed();
+    const user = await buildUser();
     const authMiddleware = auth();
 
     await authMiddleware(
@@ -138,7 +138,7 @@ describe("Authentication middleware", () => {
 
   it("should allow passing auth token in body params", async () => {
     const state = {};
-    const { user } = await seed();
+    const user = await buildUser();
     const authMiddleware = auth();
 
     await authMiddleware(
@@ -159,13 +159,14 @@ describe("Authentication middleware", () => {
 
   it("should return an error for suspended users", async () => {
     const state = {};
-    const admin = await buildUser({});
+    const admin = await buildUser();
     const user = await buildUser({
       suspendedAt: new Date(),
       suspendedById: admin.id,
     });
     const authMiddleware = auth();
 
+    let error;
     try {
       await authMiddleware(
         {
@@ -177,11 +178,38 @@ describe("Authentication middleware", () => {
         },
         jest.fn()
       );
-    } catch (e) {
-      expect(e.message).toEqual(
-        "Your access has been suspended by the team admin"
-      );
-      expect(e.errorData.adminEmail).toEqual(admin.email);
+    } catch (err) {
+      error = err;
     }
+    expect(error.message).toEqual(
+      "Your access has been suspended by the team admin"
+    );
+    expect(error.errorData.adminEmail).toEqual(admin.email);
+  });
+
+  it("should return an error for deleted team", async () => {
+    const state = {};
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+
+    await team.destroy();
+
+    const authMiddleware = auth();
+    let error;
+    try {
+      await authMiddleware(
+        {
+          request: {
+            get: jest.fn(() => `Bearer ${user.getJwtToken()}`),
+          },
+          state,
+          cache: {},
+        },
+        jest.fn()
+      );
+    } catch (err) {
+      error = err;
+    }
+    expect(error.message).toEqual("Invalid token");
   });
 });

server/models/Team.js

@@ -69,6 +69,7 @@ const Team = sequelize.define(
     slackData: DataTypes.JSONB,
   },
   {
+    paranoid: true,
     getterMethods: {
       url() {
         if (this.domain) {

server/utils/jwt.js

@@ -2,7 +2,7 @@
 import subMinutes from "date-fns/sub_minutes";
 import JWT from "jsonwebtoken";
 import { AuthenticationError } from "../errors";
-import { User } from "../models";
+import { Team, User } from "../models";
 
 function getJWTPayload(token) {
   let payload;
@@ -28,7 +28,15 @@ export async function getUserForJWT(token: string): Promise<User> {
     }
   }
 
-  const user = await User.findByPk(payload.id);
+  const user = await User.findByPk(payload.id, {
+    include: [
+      {
+        model: Team,
+        as: "team",
+        required: true,
+      },
+    ],
+  });
 
   if (payload.type === "transfer") {
     // If the user has made a single API request since the transfer token was