test(core): update the resource response guard and integration tests (#3824)

* test(core): update the resouce response guard and integration tests

update the resource response guard and integration tests

* fix(test): fix test

fix resource test

* fix(schemas): add non-empty string type guard (#3826)

* fix(schemas): add non-empty zod guard to the string typed schema field

add non-empty zod guard to the string typed schema field

* fix(schemas): comment out non-empty string guard

comment our non-empty string guard for now

* chore: add change set

add change set

* fix(schemas): remove the guard if has default value
remove the guard if has default value

* refactor(core): replace zod merge using zod extend

replace zod merge using zod extend

.changeset/smooth-icons-fetch.md

@@ -0,0 +1,7 @@
+---
+"@logto/schemas": patch
+---
+
+## Add min length 1 type guard for all string typed db schema fields
+
+Update the `@logto/schemas` zod guard generation method to include a min length of 1 for all the required string typed db fields.

packages/core/src/routes/resource.test.ts

@@ -95,7 +95,7 @@ describe('resource routes', () => {
       .post('/resources')
       .send({ name, indicator, accessTokenTtl });
 
-    expect(response.status).toEqual(200);
+    expect(response.status).toEqual(201);
     expect(response.body).toEqual({
       tenantId: 'fake_tenant',
       id: 'randomId',
@@ -188,7 +188,7 @@ describe('resource routes', () => {
       .post('/resources/foo/scopes')
       .send({ name, description });
 
-    expect(response.status).toEqual(200);
+    expect(response.status).toEqual(201);
     expect(findResourceById).toHaveBeenCalledWith('foo');
     expect(insertScope).toHaveBeenCalledWith({
       id: 'randomId',

packages/core/src/routes/resource.ts

@@ -45,6 +45,8 @@ export default function resourceRoutes<T extends AuthedRouter>(
       query: object({
         includeScopes: string().optional(),
       }),
+      response: Resources.guard.extend({ scopes: Scopes.guard.array().optional() }).array(),
+      status: [200],
     }),
     async (ctx, next) => {
       const { limit, offset, disabled } = ctx.pagination;
@@ -75,6 +77,8 @@ export default function resourceRoutes<T extends AuthedRouter>(
     '/resources',
     koaGuard({
       body: Resources.createGuard.omit({ id: true }),
+      response: Resources.guard.extend({ scopes: Scopes.guard.array().optional() }),
+      status: [201, 422],
     }),
     async (ctx, next) => {
       const { body } = ctx.guard;
@@ -94,6 +98,7 @@ export default function resourceRoutes<T extends AuthedRouter>(
         ...body,
       });
 
+      ctx.status = 201;
       ctx.body = { ...resource, scopes: [] };
 
       return next();
@@ -102,7 +107,11 @@ export default function resourceRoutes<T extends AuthedRouter>(
 
   router.get(
     '/resources/:id',
-    koaGuard({ params: object({ id: string().min(1) }) }),
+    koaGuard({
+      params: object({ id: string().min(1) }),
+      response: Resources.guard,
+      status: [200, 404],
+    }),
     async (ctx, next) => {
       const {
         params: { id },
@@ -120,6 +129,8 @@ export default function resourceRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ id: string().min(1) }),
       body: Resources.createGuard.omit({ id: true, indicator: true }).partial(),
+      response: Resources.guard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -136,7 +147,7 @@ export default function resourceRoutes<T extends AuthedRouter>(
 
   router.delete(
     '/resources/:id',
-    koaGuard({ params: object({ id: string().min(1) }) }),
+    koaGuard({ params: object({ id: string().min(1) }), status: [204, 404] }),
     async (ctx, next) => {
       const { id } = ctx.guard.params;
       await deleteResourceById(id);
@@ -149,7 +160,11 @@ export default function resourceRoutes<T extends AuthedRouter>(
   router.get(
     '/resources/:resourceId/scopes',
     koaPagination(),
-    koaGuard({ params: object({ resourceId: string().min(1) }) }),
+    koaGuard({
+      params: object({ resourceId: string().min(1) }),
+      status: [200, 400],
+      response: Scopes.guard.array(),
+    }),
     async (ctx, next) => {
       const {
         params: { resourceId },
@@ -161,14 +176,14 @@ export default function resourceRoutes<T extends AuthedRouter>(
         async () => {
           const search = parseSearchParamsForSearch(searchParams);
 
-          const [{ count }, roles] = await Promise.all([
+          const [{ count }, scopes] = await Promise.all([
             countScopesByResourceId(resourceId, search),
             searchScopesByResourceId(resourceId, search, limit, offset),
           ]);
 
           // Return totalCount to pagination middleware
           ctx.pagination.totalCount = count;
-          ctx.body = roles;
+          ctx.body = scopes;
 
           return next();
         },
@@ -190,6 +205,8 @@ export default function resourceRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ resourceId: string().min(1) }),
       body: Scopes.createGuard.pick({ name: true, description: true }),
+      response: Scopes.guard,
+      status: [201, 422, 400, 404],
     }),
     async (ctx, next) => {
       const {
@@ -199,6 +216,17 @@ export default function resourceRoutes<T extends AuthedRouter>(
 
       assertThat(!/\s/.test(body.name), 'scope.name_with_space');
 
+      assertThat(
+        await findResourceById(resourceId),
+        new RequestError({
+          code: 'entity.not_exists_with_id',
+          name: 'resource',
+          id: resourceId,
+          resourceId,
+          status: 404,
+        })
+      );
+
       assertThat(
         !(await findScopeByNameAndResourceId(body.name, resourceId)),
         new RequestError({
@@ -208,6 +236,7 @@ export default function resourceRoutes<T extends AuthedRouter>(
         })
       );
 
+      ctx.status = 201;
       ctx.body = await insertScope({
         ...body,
         id: scopeId(),
@@ -222,7 +251,9 @@ export default function resourceRoutes<T extends AuthedRouter>(
     '/resources/:resourceId/scopes/:scopeId',
     koaGuard({
       params: object({ resourceId: string().min(1), scopeId: string().min(1) }),
-      body: Scopes.createGuard.pick({ name: true, description: true }),
+      body: Scopes.createGuard.pick({ name: true, description: true }).partial(),
+      response: Scopes.guard,
+      status: [200, 404, 422],
     }),
     async (ctx, next) => {
       const {
@@ -230,6 +261,17 @@ export default function resourceRoutes<T extends AuthedRouter>(
         body,
       } = ctx.guard;
 
+      assertThat(
+        await findResourceById(resourceId),
+        new RequestError({
+          code: 'entity.not_exists_with_id',
+          name: 'resource',
+          id: resourceId,
+          resourceId,
+          status: 404,
+        })
+      );
+
       if (body.name) {
         assertThat(!/\s/.test(body.name), 'scope.name_with_space');
         assertThat(
@@ -252,10 +294,11 @@ export default function resourceRoutes<T extends AuthedRouter>(
     '/resources/:resourceId/scopes/:scopeId',
     koaGuard({
       params: object({ resourceId: string().min(1), scopeId: string().min(1) }),
+      status: [204, 404],
     }),
     async (ctx, next) => {
       const {
-        params: { resourceId, scopeId },
+        params: { scopeId },
       } = ctx.guard;
 
       await deleteScopeById(scopeId);

packages/integration-tests/src/api/resource.ts

@@ -15,6 +15,8 @@ export const createResource = async (name?: string, indicator?: string) =>
     })
     .json<Resource>();
 
+export const getResources = async () => authedAdminApi.get('resources').json<Resource[]>();
+
 export const getResource = async (resourceId: string, options?: OptionsOfTextResponseBody) =>
   authedAdminApi.get(`resources/${resourceId}`, options).json<Resource>();
 

packages/integration-tests/src/tests/api/resource.scope.test.ts

@@ -33,6 +33,27 @@ describe('scopes', () => {
     expect(response instanceof HTTPError && response.response.statusCode === 422).toBe(true);
   });
 
+  it('should return 404 when create scope with invalid resource id', async () => {
+    const response = await createScope('invalid_resource_id', 'invalid_scope_name').catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
+  });
+
+  it('should return 400 if scope name is empty', async () => {
+    const resource = await createResource();
+
+    const response = await createScope(resource.id, '').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode === 400).toBe(true);
+  });
+
+  it('should return 400 if scope name has empty space', async () => {
+    const resource = await createResource();
+
+    const response = await createScope(resource.id, 'scope id').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode === 400).toBe(true);
+  });
+
   it('should update scope successfully', async () => {
     const resource = await createResource();
     const scope = await createScope(resource.id);
@@ -58,11 +79,37 @@ describe('scopes', () => {
     const createdScope2 = await createScope(resource.id);
     const response = await updateScope(resource.id, createdScope2.id, {
       name: createdScope.name,
-      description: '',
     }).catch((error: unknown) => error);
     expect(response instanceof HTTPError && response.response.statusCode === 422).toBe(true);
   });
 
+  it('should return 400 if update scope name that has empty space', async () => {
+    const resource = await createResource();
+    const scope = await createScope(resource.id);
+    const response = await updateScope(resource.id, scope.id, {
+      name: 'scope name',
+    }).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode === 400).toBe(true);
+  });
+
+  it('should return 404 when update scope with invalid resource id', async () => {
+    const response = await updateScope('invalid_resource_id', 'invalid_scope_id', {
+      name: 'scope',
+    }).catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
+  });
+
+  it('should return 404 when update scope with invalid scope id', async () => {
+    const resource = await createResource();
+
+    const response = await updateScope(resource.id, 'invalid_scope_id', {
+      name: 'scope',
+    }).catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
+  });
+
   it('should delete scope successfully', async () => {
     const resource = await createResource();
     const scope = await createScope(resource.id);

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

@@ -1,7 +1,13 @@
 import { defaultManagementApi } from '@logto/schemas';
 import { HTTPError } from 'got';
 
-import { createResource, getResource, updateResource, deleteResource } from '#src/api/index.js';
+import {
+  createResource,
+  getResource,
+  getResources,
+  updateResource,
+  deleteResource,
+} from '#src/api/index.js';
 import { createResponseWithCode } from '#src/helpers/admin-tenant.js';
 import { generateResourceIndicator, generateResourceName } from '#src/utils.js';
 
@@ -12,6 +18,11 @@ describe('admin console api resources', () => {
     expect(fetchedManagementApiResource).toMatchObject(defaultManagementApi.resource);
   });
 
+  it('should return 404 if resource not found', async () => {
+    const response = await getResource('not_found').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
+  });
+
   it('should create api resource successfully', async () => {
     const resourceName = generateResourceName();
     const resourceIndicator = generateResourceIndicator();
@@ -40,6 +51,20 @@ describe('admin console api resources', () => {
     );
   });
 
+  it('should get resource list successfully', async () => {
+    const resourceName = generateResourceName();
+    const resourceIndicator = generateResourceIndicator();
+
+    // Create first resource
+    await createResource(resourceName, resourceIndicator);
+
+    // Get all resources
+    const resources = await getResources();
+
+    expect(resources.length).toBeGreaterThan(0);
+    expect(resources.findIndex(({ name }) => name === resourceName)).not.toBe(-1);
+  });
+
   it('should update api resource details successfully', async () => {
     const resource = await createResource();
 
@@ -58,6 +83,17 @@ describe('admin console api resources', () => {
     expect(updatedResource.accessTokenTtl).toBe(newAccessTokenTtl);
   });
 
+  it('should throw error when update api resource with invalid payload', async () => {
+    const resource = await createResource();
+
+    const response = await updateResource(resource.id, {
+      // @ts-expect-error for testing
+      name: 123,
+    }).catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode === 400).toBe(true);
+  });
+
   it('should not update api resource indicator', async () => {
     const resource = await createResource();
     const newResourceName = `new_${resource.name}`;
@@ -83,4 +119,9 @@ describe('admin console api resources', () => {
     const response = await getResource(createdResource.id).catch((error: unknown) => error);
     expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
   });
+
+  it('should throw 404 when delete api resource not found', async () => {
+    const response = await deleteResource('dummy_id').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode === 404).toBe(true);
+  });
 });

packages/schemas/src/gen/schema.ts

@@ -45,11 +45,17 @@ export const generateSchema = ({ name, fields }: TableWithType) => {
           )}${conditionalString((nullable || hasDefaultValue) && '.optional()')},`;
         }
 
-        return `  ${camelcase(name)}: z.${
-          isEnum ? `nativeEnum(${type})` : `${type}()`
-        }${conditionalString(isString && maxLength && `.max(${maxLength})`)}${conditionalString(
-          isArray && '.array()'
-        )}${conditionalString(nullable && '.nullable()')}${conditionalString(
+        return `  ${camelcase(name)}: z.${isEnum ? `nativeEnum(${type})` : `${type}()`}${
+          // Non-nullable strings should have a min length of 1
+          conditionalString(
+            isString && !(nullable || hasDefaultValue || name === tenantId) && `.min(1)`
+          )
+        }${
+          // String types value in DB should have a max length
+          conditionalString(isString && maxLength && `.max(${maxLength})`)
+        }${conditionalString(isArray && '.array()')}${conditionalString(
+          nullable && '.nullable()'
+        )}${conditionalString(
           (nullable || hasDefaultValue || name === tenantId) && '.optional()'
         )},`;
       }
@@ -58,19 +64,28 @@ export const generateSchema = ({ name, fields }: TableWithType) => {
     '',
     `const guard: Guard<${modelName}> = z.object({`,
 
-    ...fields.map(({ name, type, isArray, isEnum, nullable, tsType, isString, maxLength }) => {
-      if (tsType) {
-        return `  ${camelcase(name)}: ${camelcase(tsType)}Guard${conditionalString(
+    ...fields.map(
+      // eslint-disable-next-line complexity
+      ({ name, type, isArray, isEnum, nullable, tsType, isString, maxLength, hasDefaultValue }) => {
+        if (tsType) {
+          return `  ${camelcase(name)}: ${camelcase(tsType)}Guard${conditionalString(
+            nullable && '.nullable()'
+          )},`;
+        }
+
+        return `  ${camelcase(name)}: z.${isEnum ? `nativeEnum(${type})` : `${type}()`}${
+          // Non-nullable strings should have a min length of 1
+          conditionalString(
+            isString && !(nullable || hasDefaultValue || name === tenantId) && `.min(1)`
+          )
+        }${
+          // String types value in DB should have a max length
+          conditionalString(isString && maxLength && `.max(${maxLength})`)
+        }${conditionalString(isArray && '.array()')}${conditionalString(
           nullable && '.nullable()'
         )},`;
       }
-
-      return `  ${camelcase(name)}: z.${
-        isEnum ? `nativeEnum(${type})` : `${type}()`
-      }${conditionalString(isString && maxLength && `.max(${maxLength})`)}${conditionalString(
-        isArray && '.array()'
-      )}${conditionalString(nullable && '.nullable()')},`;
-    }),
+    ),
     '  });',
     '',
     `export const ${camelcase(name, {