Merge pull request #3380 from logto-io/gao-add-response-guard

refactor(core): add response and status guard for hooks apis

packages/core/src/middleware/koa-guard.test.ts

@@ -1,6 +1,7 @@
 import { createMockUtils } from '@logto/shared/esm';
 import { z } from 'zod';
 
+import ServerError from '#src/errors/ServerError/index.js';
 import { emptyMiddleware, createContextWithRouteParameters } from '#src/utils/test-utils.js';
 
 const { jest } = import.meta;
@@ -11,12 +12,12 @@ const { default: koaGuard, isGuardMiddleware } = await import('./koa-guard.js');
 
 describe('koaGuardMiddleware', () => {
   describe('isGuardMiddleware', () => {
-    it('isGuardMiddleware return false if name not match', () => {
+    it('should return false if name not match', () => {
       const fooMiddleware = jest.fn();
       expect(isGuardMiddleware(fooMiddleware)).toEqual(false);
     });
 
-    it('isGuardMiddleware return true if name is guardMiddleware & has config property', () => {
+    it('should return true if name is guardMiddleware & has config property', () => {
       // eslint-disable-next-line unicorn/consistent-function-scoping
       const guardMiddleware = () => ({});
 
@@ -26,7 +27,7 @@ describe('koaGuardMiddleware', () => {
       expect(isGuardMiddleware(guardMiddleware)).toBe(true);
     });
 
-    it('isGuardMiddleware  return false if name is name is guardMiddleware but has no config property', () => {
+    it('should return false if name is name is guardMiddleware but has no config property', () => {
       // eslint-disable-next-line unicorn/consistent-function-scoping
       const guardMiddleware = () => ({});
 
@@ -43,10 +44,10 @@ describe('koaGuardMiddleware', () => {
       foo: z.string(),
     });
 
-    // Use to bypass the context type assert
+    /** Use to bypass the context type assert. */
     const defaultGuard = { body: undefined, query: undefined, params: undefined, files: undefined };
 
-    it('invalid body type should throw', async () => {
+    it('should throw when body type is invalid', async () => {
       const ctx = {
         ...baseCtx,
         request: {
@@ -63,10 +64,9 @@ describe('koaGuardMiddleware', () => {
       await expect(koaGuard({ body: FooGuard })(ctx, next)).rejects.toThrow();
     });
 
-    it('invalid query type should throw', async () => {
+    it('should throw when query type is invalid', async () => {
       const ctx = {
         ...baseCtx,
-
         request: {
           ...baseCtx.request,
           query: {},
@@ -81,7 +81,60 @@ describe('koaGuardMiddleware', () => {
       await expect(koaGuard({ query: FooGuard })(ctx, next)).rejects.toThrow();
     });
 
-    it('invalid params type should throw', async () => {
+    it('should throw when files type is invalid', async () => {
+      const ctx = {
+        ...baseCtx,
+        request: {
+          ...baseCtx.request,
+          files: {},
+        },
+        params: {},
+        guard: {
+          ...defaultGuard,
+          files: { foo: '1' },
+        },
+      };
+
+      await expect(koaGuard({ files: FooGuard })(ctx, next)).rejects.toThrow();
+    });
+
+    it('should throw when response type is invalid', async () => {
+      const ctx = {
+        ...baseCtx,
+        request: {
+          ...baseCtx.request,
+          body: { foo: '1' },
+        },
+        params: {},
+        body: {},
+        guard: {
+          ...defaultGuard,
+          body: { foo: '1' },
+        },
+      };
+
+      // @ts-expect-error
+      await expect(koaGuard({ body: FooGuard, response: FooGuard })(ctx, next)).rejects.toThrow(
+        ServerError
+      );
+    });
+
+    it('should throw when status is invalid', async () => {
+      const ctx = {
+        ...baseCtx,
+        params: {},
+        body: {},
+        guard: {},
+        response: { status: 301 },
+      };
+
+      // @ts-expect-error
+      await expect(koaGuard({ status: 200 })(ctx, next)).rejects.toThrow(ServerError);
+      // @ts-expect-error
+      await expect(koaGuard({ status: [200, 204] })(ctx, next)).rejects.toThrow(ServerError);
+    });
+
+    it('should throw when params type is invalid', async () => {
       const ctx = {
         ...baseCtx,
         params: {},
@@ -94,7 +147,7 @@ describe('koaGuardMiddleware', () => {
       await expect(koaGuard({ params: FooGuard })(ctx, next)).rejects.toThrow();
     });
 
-    it('valid body, query, params should pass', async () => {
+    it('should pass when all data are valid', async () => {
       const ctx = {
         ...baseCtx,
         request: {
@@ -106,6 +159,8 @@ describe('koaGuardMiddleware', () => {
             foo: '2',
           },
         },
+        body: { foo: '4' },
+        response: { status: 200 },
         params: {
           foo: '1',
         },
@@ -117,7 +172,15 @@ describe('koaGuardMiddleware', () => {
         },
       };
 
-      await koaGuard({ params: FooGuard, query: FooGuard, body: FooGuard })(ctx, next);
+      await koaGuard({
+        params: FooGuard,
+        query: FooGuard,
+        body: FooGuard,
+        response: FooGuard,
+        status: [200, 204],
+        // @ts-expect-error
+      })(ctx, next);
+      expect(ctx.body).toHaveProperty('foo', '4');
       expect(ctx.guard.body).toHaveProperty('foo', '3');
       expect(ctx.guard.query).toHaveProperty('foo', '2');
       expect(ctx.guard.params).toHaveProperty('foo', '1');

packages/core/src/middleware/koa-guard.ts

@@ -10,11 +10,47 @@ import RequestError from '#src/errors/RequestError/index.js';
 import ServerError from '#src/errors/ServerError/index.js';
 import assertThat from '#src/utils/assert-that.js';
 
+/** Configure what and how to guard. */
 export type GuardConfig<QueryT, BodyT, ParametersT, ResponseT, FilesT> = {
+  /**
+   * Guard query parameters after `?` in the URL.
+   *
+   * Normally you need to use a "string-string" dictionary guard for wrapping key-value pairs
+   * since query parameter values will be always parsed as strings.
+   *
+   * @example
+   * // e.g. parse '?key1=foo'
+   * z.object({ key1: z.string() })
+   */
   query?: ZodType<QueryT>;
+  /**
+   * Guard JSON request body. You can treat the body like a normal object.
+   *
+   * @example
+   * z.object({
+   *   key1: z.string(),
+   *   key2: z.object({ key3: z.number() }).array(),
+   * })
+   */
   body?: ZodType<BodyT>;
+  /**
+   * Guard `koa-router` path parameters (i.e. `ctx.params`).
+   *
+   * @example
+   * // e.g. parse '/foo/:key1'
+   * z.object({ key1: z.string() })
+   */
   params?: ZodType<ParametersT>;
+  /**
+   * Guard response body.
+   *
+   * @example z.object({ key1: z.string() })
+   */
   response?: ZodType<ResponseT>;
+  /**
+   * Guard response status code. It produces a `ServerError` (500)
+   * if the response does not satisfy any of the given value(s).
+   */
   status?: number | number[];
   files?: ZodType<FilesT>;
 };
@@ -113,13 +149,9 @@ export default function koaGuard<
       GuardResponseT
     >
   > = async function (ctx, next) {
-    if (body ?? files) {
-      return koaBody<StateT, ContextT>({ multipart: Boolean(files) })(ctx, async () =>
-        guard(ctx, next)
-      );
-    }
-
-    await guard(ctx, next);
+    await (body ?? files
+      ? koaBody<StateT, ContextT>({ multipart: Boolean(files) })(ctx, async () => guard(ctx, next))
+      : guard(ctx, next));
 
     if (status !== undefined) {
       assertThat(

packages/core/src/routes/hook.ts

@@ -11,15 +11,19 @@ export default function hookRoutes<T extends AuthedRouter>(
 ) {
   const { findAllHooks, findHookById, insertHook, updateHookById, deleteHookById } = queries.hooks;
 
-  router.get('/hooks', async (ctx, next) => {
-    ctx.body = await findAllHooks();
+  router.get(
+    '/hooks',
+    koaGuard({ response: Hooks.guard.array(), status: 200 }),
+    async (ctx, next) => {
+      ctx.body = await findAllHooks();
 
-    return next();
-  });
+      return next();
+    }
+  );
 
   router.post(
     '/hooks',
-    koaGuard({ body: Hooks.createGuard.omit({ id: true }) }),
+    koaGuard({ body: Hooks.createGuard.omit({ id: true }), response: Hooks.guard, status: 200 }),
     async (ctx, next) => {
       ctx.body = await insertHook({
         id: generateStandardId(),
@@ -32,7 +36,11 @@ export default function hookRoutes<T extends AuthedRouter>(
 
   router.get(
     '/hooks/:id',
-    koaGuard({ params: z.object({ id: z.string().min(1) }) }),
+    koaGuard({
+      params: z.object({ id: z.string().min(1) }),
+      response: Hooks.guard,
+      status: [200, 404],
+    }),
     async (ctx, next) => {
       const {
         params: { id },
@@ -49,6 +57,8 @@ export default function hookRoutes<T extends AuthedRouter>(
     koaGuard({
       params: z.object({ id: z.string().min(1) }),
       body: Hooks.createGuard.omit({ id: true }).partial(),
+      response: Hooks.guard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -64,7 +74,7 @@ export default function hookRoutes<T extends AuthedRouter>(
 
   router.delete(
     '/hooks/:id',
-    koaGuard({ params: z.object({ id: z.string().min(1) }) }),
+    koaGuard({ params: z.object({ id: z.string().min(1) }), status: [204, 404] }),
     async (ctx, next) => {
       const { id } = ctx.guard.params;
       await deleteHookById(id);

packages/integration-tests/src/tests/api/hooks.test.ts

@@ -33,7 +33,7 @@ describe('hooks', () => {
     await close();
   });
 
-  it('should be able to create, query, and delete a hook', async () => {
+  it('should be able to create, query, update, and delete a hook', async () => {
     const payload = createPayload(HookEvent.PostRegister);
     const created = await authedAdminApi.post('hooks', { json: payload }).json<Hook>();
 
@@ -42,6 +42,11 @@ describe('hooks', () => {
 
     expect(await authedAdminApi.get('hooks').json<Hook[]>()).toContainEqual(created);
     expect(await authedAdminApi.get(`hooks/${created.id}`).json<Hook>()).toEqual(created);
+    expect(
+      await authedAdminApi
+        .patch(`hooks/${created.id}`, { json: { event: HookEvent.PostSignIn } })
+        .json<Hook>()
+    ).toEqual({ ...created, event: HookEvent.PostSignIn });
     expect(await authedAdminApi.delete(`hooks/${created.id}`)).toHaveProperty('statusCode', 204);
     await expect(authedAdminApi.get(`hooks/${created.id}`)).rejects.toHaveProperty(
       'response.statusCode',