Zod schemas for routes under /plugins (outline#6378)

* fix: schema for slack routes * fix: slack.post * fix: email
FigPedro · Jan 13, 2024 · 3561b79 · 3561b79
1 parent 7e61a51
commit 3561b79
Show file tree

Hide file tree

Showing 7 changed files with 185 additions and 76 deletions.
diff --git a/plugins/email/server/auth/email.test.ts b/plugins/email/server/auth/email.test.ts
@@ -8,6 +8,15 @@ import { getTestServer } from "@server/test/support";
 const server = getTestServer();
 
 describe("email", () => {
+  it("should fail with status 400 bad request if email is invalid", async () => {
+    const res = await server.post("/auth/email", {
+      body: { email: "invalid" },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual("email: Invalid email");
+  });
+
   it("should require email param", async () => {
     const res = await server.post("/auth/email", {
       body: {},

diff --git a/plugins/email/server/auth/email.ts b/plugins/email/server/auth/email.ts
@@ -1,26 +1,28 @@
 import Router from "koa-router";
-import { Client, NotificationEventType } from "@shared/types";
+import { NotificationEventType } from "@shared/types";
 import { parseDomain } from "@shared/utils/domains";
 import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail";
 import SigninEmail from "@server/emails/templates/SigninEmail";
 import WelcomeEmail from "@server/emails/templates/WelcomeEmail";
 import env from "@server/env";
 import { AuthorizationError } from "@server/errors";
 import { rateLimiter } from "@server/middlewares/rateLimiter";
+import validate from "@server/middlewares/validate";
 import { User, Team } from "@server/models";
+import { APIContext } from "@server/types";
 import { RateLimiterStrategy } from "@server/utils/RateLimiter";
 import { signIn } from "@server/utils/authentication";
 import { getUserForEmailSigninToken } from "@server/utils/jwt";
-import { assertEmail, assertPresent } from "@server/validation";
+import * as T from "./schema";
 
 const router = new Router();
 
 router.post(
   "email",
   rateLimiter(RateLimiterStrategy.TenPerHour),
-  async (ctx) => {
-    const { email, client } = ctx.request.body;
-    assertEmail(email, "email is required");
+  validate(T.EmailSchema),
+  async (ctx: APIContext<T.EmailReq>) => {
+    const { email, client } = ctx.input.body;
 
     const domain = parseDomain(ctx.request.hostname);
 
@@ -71,7 +73,7 @@ router.post(
       to: user.email,
       token: user.getEmailSigninToken(),
       teamUrl: team.url,
-      client: client === Client.Desktop ? Client.Desktop : Client.Web,
+      client,
     }).schedule();
 
     user.lastSigninEmailSentAt = new Date();
@@ -84,52 +86,57 @@ router.post(
   }
 );
 
-router.get("email.callback", async (ctx) => {
-  const { token, client } = ctx.request.query;
-  assertPresent(token, "token is required");
+router.get(
+  "email.callback",
+  validate(T.EmailCallbackSchema),
+  async (ctx: APIContext<T.EmailCallbackReq>) => {
+    const { token, client } = ctx.input.query;
 
-  let user!: User;
+    let user!: User;
 
-  try {
-    user = await getUserForEmailSigninToken(token as string);
-  } catch (err) {
-    ctx.redirect(`/?notice=expired-token`);
-    return;
-  }
-
-  if (!user.team.emailSigninEnabled) {
-    return ctx.redirect("/?notice=auth-error");
-  }
+    try {
+      user = await getUserForEmailSigninToken(token as string);
+    } catch (err) {
+      ctx.redirect(`/?notice=expired-token`);
+      return;
+    }
 
-  if (user.isSuspended) {
-    return ctx.redirect("/?notice=user-suspended");
-  }
+    if (!user.team.emailSigninEnabled) {
+      return ctx.redirect("/?notice=auth-error");
+    }
 
-  if (user.isInvited) {
-    await new WelcomeEmail({
-      to: user.email,
-      teamUrl: user.team.url,
-    }).schedule();
+    if (user.isSuspended) {
+      return ctx.redirect("/?notice=user-suspended");
+    }
 
-    const inviter = await user.$get("invitedBy");
-    if (inviter?.subscribedToEventType(NotificationEventType.InviteAccepted)) {
-      await new InviteAcceptedEmail({
-        to: inviter.email,
-        inviterId: inviter.id,
-        invitedName: user.name,
+    if (user.isInvited) {
+      await new WelcomeEmail({
+        to: user.email,
         teamUrl: user.team.url,
       }).schedule();
+
+      const inviter = await user.$get("invitedBy");
+      if (
+        inviter?.subscribedToEventType(NotificationEventType.InviteAccepted)
+      ) {
+        await new InviteAcceptedEmail({
+          to: inviter.email,
+          inviterId: inviter.id,
+          invitedName: user.name,
+          teamUrl: user.team.url,
+        }).schedule();
+      }
     }
-  }
 
-  // set cookies on response and redirect to team subdomain
-  await signIn(ctx, "email", {
-    user,
-    team: user.team,
-    isNewTeam: false,
-    isNewUser: false,
-    client: client === Client.Desktop ? Client.Desktop : Client.Web,
-  });
-});
+    // set cookies on response and redirect to team subdomain
+    await signIn(ctx, "email", {
+      user,
+      team: user.team,
+      isNewTeam: false,
+      isNewUser: false,
+      client,
+    });
+  }
+);
 
 export default router;
diff --git a/plugins/email/server/auth/schema.ts b/plugins/email/server/auth/schema.ts
@@ -0,0 +1,21 @@
+import { z } from "zod";
+import { Client } from "@shared/types";
+import { BaseSchema } from "@server/routes/api/schema";
+
+export const EmailSchema = BaseSchema.extend({
+  body: z.object({
+    email: z.string().email(),
+    client: z.nativeEnum(Client).default(Client.Web),
+  }),
+});
+
+export type EmailReq = z.infer<typeof EmailSchema>;
+
+export const EmailCallbackSchema = BaseSchema.extend({
+  query: z.object({
+    token: z.string(),
+    client: z.nativeEnum(Client).default(Client.Web),
+  }),
+});
+
+export type EmailCallbackReq = z.infer<typeof EmailCallbackSchema>;
diff --git a/plugins/slack/server/auth/schema.ts b/plugins/slack/server/auth/schema.ts
@@ -0,0 +1,31 @@
+import isEmpty from "lodash/isEmpty";
+import { z } from "zod";
+import { BaseSchema } from "@server/routes/api/schema";
+
+export const SlackCommandsSchema = BaseSchema.extend({
+  query: z
+    .object({
+      code: z.string().nullish(),
+      state: z.string().uuid().nullish(),
+      error: z.string().nullish(),
+    })
+    .refine((req) => !(isEmpty(req.code) && isEmpty(req.error)), {
+      message: "one of code or error is required",
+    }),
+});
+
+export type SlackCommandsReq = z.infer<typeof SlackCommandsSchema>;
+
+export const SlackPostSchema = BaseSchema.extend({
+  query: z
+    .object({
+      code: z.string().nullish(),
+      state: z.string().uuid().nullish(),
+      error: z.string().nullish(),
+    })
+    .refine((req) => !(isEmpty(req.code) && isEmpty(req.error)), {
+      message: "one of code or error is required",
+    }),
+});
+
+export type SlackPostReq = z.infer<typeof SlackPostSchema>;
diff --git a/plugins/slack/server/auth/slack.test.ts b/plugins/slack/server/auth/slack.test.ts
@@ -0,0 +1,39 @@
+import { getTestServer } from "@server/test/support";
+
+const server = getTestServer();
+
+describe("#slack.commands", () => {
+  it("should fail with status 400 bad request if query param state is not a uuid", async () => {
+    const res = await server.get("/auth/slack.commands?state=123");
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual("state: Invalid uuid");
+  });
+
+  it("should fail with status 400 bad request when both code and error are missing in query params", async () => {
+    const res = await server.get(
+      "/auth/slack.commands?state=182d14d5-0dbd-4521-ac52-25484c25c96e"
+    );
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual("query: one of code or error is required");
+  });
+});
+
+describe("#slack.post", () => {
+  it("should fail with status 400 bad request if query param state is not a uuid", async () => {
+    const res = await server.get("/auth/slack.post?state=123");
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual("state: Invalid uuid");
+  });
+
+  it("should fail with status 400 bad request when both code and error are missing in query params", async () => {
+    const res = await server.get(
+      "/auth/slack.post?state=182d14d5-0dbd-4521-ac52-25484c25c96e"
+    );
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual("query: one of code or error is required");
+  });
+});
diff --git a/plugins/slack/server/auth/slack.ts b/plugins/slack/server/auth/slack.ts
@@ -9,21 +9,22 @@ import accountProvisioner from "@server/commands/accountProvisioner";
 import env from "@server/env";
 import auth from "@server/middlewares/authentication";
 import passportMiddleware from "@server/middlewares/passport";
+import validate from "@server/middlewares/validate";
 import {
   IntegrationAuthentication,
   Collection,
   Integration,
   Team,
   User,
 } from "@server/models";
-import { AppContext, AuthenticationResult } from "@server/types";
+import { APIContext, AuthenticationResult } from "@server/types";
 import {
   getClientFromContext,
   getTeamFromContext,
   StateStore,
 } from "@server/utils/passport";
-import { assertPresent, assertUuid } from "@server/validation";
 import * as Slack from "../slack";
+import * as T from "./schema";
 
 type SlackProfile = Profile & {
   team: {
@@ -132,10 +133,10 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
     auth({
       optional: true,
     }),
-    async (ctx: AppContext) => {
-      const { code, state, error } = ctx.request.query;
+    validate(T.SlackCommandsSchema),
+    async (ctx: APIContext<T.SlackCommandsReq>) => {
+      const { code, state: teamId, error } = ctx.input.query;
       const { user } = ctx.state.auth;
-      assertPresent(code || error, "code is required");
 
       if (error) {
         ctx.redirect(integrationSettingsPath(`slack?error=${error}`));
@@ -146,9 +147,9 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
       // access authentication for subdomains. We must forward to the appropriate
       // subdomain to complete the oauth flow
       if (!user) {
-        if (state) {
+        if (teamId) {
           try {
-            const team = await Team.findByPk(String(state), {
+            const team = await Team.findByPk(teamId, {
               rejectOnEmpty: true,
             });
             return redirectOnClient(
@@ -168,7 +169,8 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
       }
 
       const endpoint = `${env.URL}/auth/slack.commands`;
-      const data = await Slack.oauthAccess(String(code), endpoint);
+      // validation middleware ensures that code is non-null at this point
+      const data = await Slack.oauthAccess(code!, endpoint);
       const authentication = await IntegrationAuthentication.create({
         service: IntegrationService.Slack,
         userId: user.id,
@@ -195,14 +197,10 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
     auth({
       optional: true,
     }),
-    async (ctx: AppContext) => {
-      const { code, error, state } = ctx.request.query;
+    validate(T.SlackPostSchema),
+    async (ctx: APIContext<T.SlackPostReq>) => {
+      const { code, error, state: collectionId } = ctx.input.query;
       const { user } = ctx.state.auth;
-      assertPresent(code || error, "code is required");
-
-      // FIX ME! What about having zod like schema in place here?
-      const collectionId = state as string;
-      assertUuid(collectionId, "collectionId must be an uuid");
 
       if (error) {
         ctx.redirect(integrationSettingsPath(`slack?error=${error}`));
@@ -213,29 +211,33 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
       // access authentication for subdomains. We must forward to the
       // appropriate subdomain to complete the oauth flow
       if (!user) {
-        try {
-          const collection = await Collection.findOne({
-            where: {
-              id: String(state),
-            },
-            rejectOnEmpty: true,
-          });
-          const team = await Team.findByPk(collection.teamId, {
-            rejectOnEmpty: true,
-          });
-          return redirectOnClient(
-            ctx,
-            `${team.url}/auth/slack.post?${ctx.request.querystring}`
-          );
-        } catch (err) {
+        if (collectionId) {
+          try {
+            const collection = await Collection.findByPk(collectionId, {
+              rejectOnEmpty: true,
+            });
+            const team = await Team.findByPk(collection.teamId, {
+              rejectOnEmpty: true,
+            });
+            return redirectOnClient(
+              ctx,
+              `${team.url}/auth/slack.post?${ctx.request.querystring}`
+            );
+          } catch (err) {
+            return ctx.redirect(
+              integrationSettingsPath(`slack?error=unauthenticated`)
+            );
+          }
+        } else {
           return ctx.redirect(
             integrationSettingsPath(`slack?error=unauthenticated`)
           );
         }
       }
 
       const endpoint = `${env.URL}/auth/slack.post`;
-      const data = await Slack.oauthAccess(code as string, endpoint);
+      // validation middleware ensures that code is non-null at this point
+      const data = await Slack.oauthAccess(code!, endpoint);
       const authentication = await IntegrationAuthentication.create({
         service: IntegrationService.Slack,
         userId: user.id,

diff --git a/server/models/helpers/AuthenticationHelper.ts b/server/models/helpers/AuthenticationHelper.ts
@@ -32,7 +32,7 @@ export default class AuthenticationHelper {
     const rootDir = env.ENVIRONMENT === "test" ? "" : "build";
 
     glob
-      .sync(path.join(rootDir, "plugins/*/server/auth/!(*.test).[jt]s"))
+      .sync(path.join(rootDir, "plugins/*/server/auth/!(*.test|schema).[jt]s"))
       .forEach((filePath: string) => {
         const { default: authProvider, name } = require(path.join(
           process.cwd(),