feat: Authentication provider display (#4332)

* layout * Refactor * wip * Quick changes to make this deployable without full management * test
2022-10-24 17:01:40 -04:00
parent 434bb989cc
commit df46d3754a
13 changed files with 438 additions and 283 deletions
--- a/server/models/AuthenticationProvider.ts
+++ b/server/models/AuthenticationProvider.ts
@@ -1,4 +1,4 @@
-import { Op } from "sequelize";
+import { Op, SaveOptions } from "sequelize";
 import {
  BelongsTo,
  Column,
@@ -97,9 +97,10 @@ class AuthenticationProvider extends Model {
    }
  }

-  disable = async () => {
+  disable = async (options?: SaveOptions<AuthenticationProvider>) => {
    const res = await (this
      .constructor as typeof AuthenticationProvider).findAndCountAll({
+      ...options,
      where: {
        teamId: this.teamId,
        enabled: true,
@@ -111,18 +112,24 @@ class AuthenticationProvider extends Model {
    });

    if (res.count >= 1) {
-      return this.update({
-        enabled: false,
-      });
+      return this.update(
+        {
+          enabled: false,
+        },
+        options
+      );
    } else {
      throw ValidationError("At least one authentication provider is required");
    }
  };

-  enable = () => {
-    return this.update({
-      enabled: true,
-    });
+  enable = (options?: SaveOptions<AuthenticationProvider>) => {
+    return this.update(
+      {
+        enabled: true,
+      },
+      options
+    );
  };
 }

--- a/server/routes/api/authenticationProviders.test.ts
+++ b/server/routes/api/authenticationProviders.test.ts
@@ -7,7 +7,7 @@ const server = getTestServer();
 describe("#authenticationProviders.info", () => {
  it("should return auth provider", async () => {
    const team = await buildTeam();
-    const user = await buildUser({
+    const user = await buildAdmin({
      teamId: team.id,
    });
    const authenticationProviders = await team.$get("authenticationProviders");
@@ -23,7 +23,7 @@ describe("#authenticationProviders.info", () => {
    expect(body.data.isEnabled).toBe(true);
    expect(body.data.isConnected).toBe(true);
    expect(body.policies[0].abilities.read).toBe(true);
-    expect(body.policies[0].abilities.update).toBe(false);
+    expect(body.policies[0].abilities.update).toBe(true);
  });

  it("should require authorization", async () => {
@@ -123,7 +123,7 @@ describe("#authenticationProviders.update", () => {
 describe("#authenticationProviders.list", () => {
  it("should return enabled and available auth providers", async () => {
    const team = await buildTeam();
-    const user = await buildUser({
+    const user = await buildAdmin({
      teamId: team.id,
    });
    const res = await server.post("/api/authenticationProviders.list", {
@@ -133,13 +133,13 @@ describe("#authenticationProviders.list", () => {
    });
    const body = await res.json();
    expect(res.status).toEqual(200);
-    expect(body.data.authenticationProviders.length).toBe(2);
-    expect(body.data.authenticationProviders[0].name).toBe("slack");
-    expect(body.data.authenticationProviders[0].isEnabled).toBe(true);
-    expect(body.data.authenticationProviders[0].isConnected).toBe(true);
-    expect(body.data.authenticationProviders[1].name).toBe("google");
-    expect(body.data.authenticationProviders[1].isEnabled).toBe(false);
-    expect(body.data.authenticationProviders[1].isConnected).toBe(false);
+    expect(body.data.length).toBe(2);
+    expect(body.data[0].name).toBe("slack");
+    expect(body.data[0].isEnabled).toBe(true);
+    expect(body.data[0].isConnected).toBe(true);
+    expect(body.data[1].name).toBe("google");
+    expect(body.data[1].isEnabled).toBe(false);
+    expect(body.data[1].isConnected).toBe(false);
  });

  it("should require authentication", async () => {
--- a/server/routes/api/authenticationProviders.ts
+++ b/server/routes/api/authenticationProviders.ts
@@ -1,4 +1,5 @@
 import Router from "koa-router";
+import { sequelize } from "@server/database/sequelize";
 import auth from "@server/middlewares/authentication";
 import { AuthenticationProvider, Event } from "@server/models";
 import { authorize } from "@server/policies";
@@ -11,81 +12,108 @@ import allAuthenticationProviders from "../auth/providers";

 const router = new Router();

-router.post("authenticationProviders.info", auth(), async (ctx) => {
-  const { id } = ctx.request.body;
-  assertUuid(id, "id is required");
-  const { user } = ctx.state;
-  const authenticationProvider = await AuthenticationProvider.findByPk(id);
-  authorize(user, "read", authenticationProvider);
+router.post(
+  "authenticationProviders.info",
+  auth({ admin: true }),
+  async (ctx) => {
+    const { id } = ctx.request.body;
+    assertUuid(id, "id is required");

-  ctx.body = {
-    data: presentAuthenticationProvider(authenticationProvider),
-    policies: presentPolicies(user, [authenticationProvider]),
-  };
-});
+    const { user } = ctx.state;
+    const authenticationProvider = await AuthenticationProvider.findByPk(id);
+    authorize(user, "read", authenticationProvider);

-router.post("authenticationProviders.update", auth(), async (ctx) => {
-  const { id, isEnabled } = ctx.request.body;
-  assertUuid(id, "id is required");
-  assertPresent(isEnabled, "isEnabled is required");
-  const { user } = ctx.state;
-  const authenticationProvider = await AuthenticationProvider.findByPk(id);
-
-  authorize(user, "update", authenticationProvider);
-  const enabled = !!isEnabled;
-
-  if (enabled) {
-    await authenticationProvider.enable();
-  } else {
-    await authenticationProvider.disable();
+    ctx.body = {
+      data: presentAuthenticationProvider(authenticationProvider),
+      policies: presentPolicies(user, [authenticationProvider]),
+    };
  }
+);

-  await Event.create({
-    name: "authenticationProviders.update",
-    data: {
-      enabled,
-    },
-    modelId: id,
-    teamId: user.teamId,
-    actorId: user.id,
-    ip: ctx.request.ip,
-  });
+router.post(
+  "authenticationProviders.update",
+  auth({ admin: true }),
+  async (ctx) => {
+    const { id, isEnabled } = ctx.request.body;
+    assertUuid(id, "id is required");
+    assertPresent(isEnabled, "isEnabled is required");
+    const { user } = ctx.state;

-  ctx.body = {
-    data: presentAuthenticationProvider(authenticationProvider),
-    policies: presentPolicies(user, [authenticationProvider]),
-  };
-});
+    const authenticationProvider = await sequelize.transaction(
+      async (transaction) => {
+        const authenticationProvider = await AuthenticationProvider.findByPk(
+          id,
+          {
+            transaction,
+            lock: transaction.LOCK.UPDATE,
+          }
+        );

-router.post("authenticationProviders.list", auth(), async (ctx) => {
-  const { user } = ctx.state;
-  authorize(user, "read", user.team);
+        authorize(user, "update", authenticationProvider);
+        const enabled = !!isEnabled;

-  const teamAuthenticationProviders = await user.team.$get(
-    "authenticationProviders"
-  );
+        if (enabled) {
+          await authenticationProvider.enable({ transaction });
+        } else {
+          await authenticationProvider.disable({ transaction });
+        }

-  const otherAuthenticationProviders = allAuthenticationProviders.filter(
-    (p) =>
-      // @ts-expect-error ts-migrate(7006) FIXME: Parameter 't' implicitly has an 'any' type.
-      !teamAuthenticationProviders.find((t) => t.name === p.id) &&
-      p.enabled && // email auth is dealt with separetly right now, although it definitely
-      // wants to be here in the future – we'll need to migrate more data though
-      p.id !== "email"
-  );
+        await Event.create(
+          {
+            name: "authenticationProviders.update",
+            data: {
+              enabled,
+            },
+            modelId: id,
+            teamId: user.teamId,
+            actorId: user.id,
+            ip: ctx.request.ip,
+          },
+          { transaction }
+        );

-  ctx.body = {
-    data: {
-      authenticationProviders: [
-        ...teamAuthenticationProviders.map(presentAuthenticationProvider),
-        ...otherAuthenticationProviders.map((p) => ({
+        return authenticationProvider;
+      }
+    );
+
+    ctx.body = {
+      data: presentAuthenticationProvider(authenticationProvider),
+      policies: presentPolicies(user, [authenticationProvider]),
+    };
+  }
+);
+
+router.post(
+  "authenticationProviders.list",
+  auth({ admin: true }),
+  async (ctx) => {
+    const { user } = ctx.state;
+    authorize(user, "read", user.team);
+
+    const teamAuthenticationProviders = (await user.team.$get(
+      "authenticationProviders"
+    )) as AuthenticationProvider[];
+
+    const data = allAuthenticationProviders
+      .filter((p) => p.id !== "email")
+      .map((p) => {
+        const row = teamAuthenticationProviders.find((t) => t.name === p.id);
+
+        return {
+          id: p.id,
          name: p.id,
+          displayName: p.name,
          isEnabled: false,
          isConnected: false,
-        })),
-      ],
-    },
-  };
-});
+          ...(row ? presentAuthenticationProvider(row) : {}),
+        };
+      })
+      .sort((a) => (a.isEnabled ? -1 : 1));
+
+    ctx.body = {
+      data,
+    };
+  }
+);

 export default router;
--- a/server/routes/auth/providers/index.ts
+++ b/server/routes/auth/providers/index.ts
@@ -3,13 +3,13 @@ import { sortBy } from "lodash";
 import { signin } from "@shared/utils/urlHelpers";
 import { requireDirectory } from "@server/utils/fs";

-interface AuthenticationProviderConfig {
+export type AuthenticationProviderConfig = {
  id: string;
  name: string;
  enabled: boolean;
  authUrl: string;
  router: Router;
-}
+};

 const providers: AuthenticationProviderConfig[] = [];

--- a/server/test/env.ts
+++ b/server/test/env.ts
@@ -7,6 +7,12 @@ env.GOOGLE_CLIENT_ID = "123";
 env.GOOGLE_CLIENT_SECRET = "123";
 env.SLACK_CLIENT_ID = "123";
 env.SLACK_CLIENT_SECRET = "123";
+
+env.AZURE_CLIENT_ID = undefined;
+env.AZURE_CLIENT_SECRET = undefined;
+env.OIDC_CLIENT_ID = undefined;
+env.OIDC_CLIENT_SECRET = undefined;
+
 env.RATE_LIMITER_ENABLED = false;
 env.DEPLOYMENT = undefined;