test(core): add exception cases for role api and scope api (#3802)

* feat(core): add roles api guard

add roles api guard

* feat(core): add scope api response guard

add scope api response guard

* test(core): add exception cases for role api integration tests

add exception cases for role api integration tests

* fix(console): fix lint error

fix lint error

* fix(core): remove guard status code

remove guard status code

* fix(core): resolve comments

resolve comments

* fix(core): remove useless 401,403 code guard

remove useless 401,403 code guard

* fix(core): fix swagger 422 guard error

fix swagger 422 guard error

packages/console/src/pages/Profile/components/LinkAccountSection/index.tsx

@@ -62,7 +62,7 @@ function LinkAccountSection({ user, connectors, onUpdate }: Props) {
 
     return connectors.map(({ id, name, logo, logoDark, target }) => {
       const logoSrc = theme === Theme.Dark && logoDark ? logoDark : logo;
-      const relatedUserDetails = user.identities?.[target]?.details;
+      const relatedUserDetails = user.identities[target]?.details;
 
       const socialUserInfo = socialUserInfoGuard.safeParse(relatedUserDetails);
       const hasLinked = socialUserInfo.success;

packages/core/src/routes/dashboard.ts

@@ -27,7 +27,7 @@ export default function dashboardRoutes<T extends AuthedRouter>(
       response: object({
         totalUserCount: number(),
       }),
-      status: [200, 401, 403],
+      status: [200],
     }),
     async (ctx, next) => {
       const { count: totalUserCount } = await countUsers();
@@ -41,7 +41,7 @@ export default function dashboardRoutes<T extends AuthedRouter>(
     '/dashboard/users/new',
     koaGuard({
       response: getNewUsersResponseGuard,
-      status: [200, 401, 403],
+      status: [200],
     }),
     async (ctx, next) => {
       const today = Date.now();
@@ -88,7 +88,7 @@ export default function dashboardRoutes<T extends AuthedRouter>(
     koaGuard({
       query: object({ date: string().regex(dateRegex).optional() }),
       response: getActiveUsersResponseGuard,
-      status: [200, 401, 403],
+      status: [200],
     }),
     async (ctx, next) => {
       const {

packages/core/src/routes/role.scope.ts

@@ -1,5 +1,5 @@
 import type { Scope, ScopeResponse } from '@logto/schemas';
-import { scopeResponseGuard } from '@logto/schemas';
+import { scopeResponseGuard, Scopes } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { tryThat } from '@silverhand/essentials';
 import { object, string } from 'zod';
@@ -42,6 +42,7 @@ export default function roleScopeRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ id: string().min(1) }),
       response: scopeResponseGuard.array(),
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -94,7 +95,9 @@ export default function roleScopeRoutes<T extends AuthedRouter>(
     '/roles/:id/scopes',
     koaGuard({
       params: object({ id: string().min(1) }),
-      body: object({ scopeIds: string().min(1).array() }),
+      body: object({ scopeIds: string().min(1).array().nonempty() }),
+      response: Scopes.guard.array(),
+      status: [200, 404, 422],
     }),
     async (ctx, next) => {
       const {
@@ -133,6 +136,7 @@ export default function roleScopeRoutes<T extends AuthedRouter>(
     '/roles/:id/scopes/:scopeId',
     koaGuard({
       params: object({ id: string().min(1), scopeId: string().min(1) }),
+      status: [204, 404],
     }),
     async (ctx, next) => {
       const {

packages/core/src/routes/role.ts

@@ -1,8 +1,8 @@
 import type { RoleResponse } from '@logto/schemas';
-import { userInfoSelectFields, Roles } from '@logto/schemas';
+import { userInfoSelectFields, userInfoResponseGuard, Roles, Users } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { pick, tryThat } from '@silverhand/essentials';
-import { object, string, z } from 'zod';
+import { object, string, z, number } from 'zod';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import koaGuard from '#src/middleware/koa-guard.js';
@@ -41,50 +41,74 @@ export default function roleRoutes<T extends AuthedRouter>(
 
   router.use('/roles(/.*)?', koaRoleRlsErrorHandler());
 
-  router.get('/roles', koaPagination(), async (ctx, next) => {
+  router.get(
-    const { limit, offset } = ctx.pagination;
+    '/roles',
-    const { searchParams } = ctx.request.URL;
+    koaPagination(),
-
+    koaGuard({
-    return tryThat(
+      response: Roles.guard
-      async () => {
+        .merge(
-        const search = parseSearchParamsForSearch(searchParams);
+          object({
-        const excludeUserId = searchParams.get('excludeUserId');
+            usersCount: number(),
-        const usersRoles = excludeUserId ? await findUsersRolesByUserId(excludeUserId) : [];
+            featuredUsers: Users.guard
-        const excludeRoleIds = usersRoles.map(({ roleId }) => roleId);
+              .pick({
-
+                avatar: true,
-        const [{ count }, roles] = await Promise.all([
+                id: true,
-          countRoles(search, { excludeRoleIds }),
+                name: true,
-          findRoles(search, limit, offset, { excludeRoleIds }),
+              })
-        ]);
+              .array(),
-
-        const rolesResponse: RoleResponse[] = await Promise.all(
-          roles.map(async (role) => {
-            const { count } = await countUsersRolesByRoleId(role.id);
-            const usersRoles = await findUsersRolesByRoleId(role.id, 3);
-            const users = await findUsersByIds(usersRoles.map(({ userId }) => userId));
-
-            return {
-              ...role,
-              usersCount: count,
-              featuredUsers: users.map(({ id, avatar, name }) => ({ id, avatar, name })),
-            };
           })
-        );
+        )
+        .array(),
+      status: [200, 400],
+    }),
+    async (ctx, next) => {
+      const { limit, offset } = ctx.pagination;
+      const { searchParams } = ctx.request.URL;
 
-        // Return totalCount to pagination middleware
+      return tryThat(
-        ctx.pagination.totalCount = count;
+        async () => {
-        ctx.body = rolesResponse;
+          const search = parseSearchParamsForSearch(searchParams);
+          const excludeUserId = searchParams.get('excludeUserId');
+          const usersRoles = excludeUserId ? await findUsersRolesByUserId(excludeUserId) : [];
+          const excludeRoleIds = usersRoles.map(({ roleId }) => roleId);
 
-        return next();
+          const [{ count }, roles] = await Promise.all([
-      },
+            countRoles(search, { excludeRoleIds }),
-      (error) => {
+            findRoles(search, limit, offset, { excludeRoleIds }),
-        if (error instanceof TypeError) {
+          ]);
-          throw new RequestError({ code: 'request.invalid_input', details: error.message }, error);
+
+          const rolesResponse: RoleResponse[] = await Promise.all(
+            roles.map(async (role) => {
+              const { count } = await countUsersRolesByRoleId(role.id);
+              const usersRoles = await findUsersRolesByRoleId(role.id, 3);
+              const users = await findUsersByIds(usersRoles.map(({ userId }) => userId));
+
+              return {
+                ...role,
+                usersCount: count,
+                featuredUsers: users.map(({ id, avatar, name }) => ({ id, avatar, name })),
+              };
+            })
+          );
+
+          // Return totalCount to pagination middleware
+          ctx.pagination.totalCount = count;
+          ctx.body = rolesResponse;
+
+          return next();
+        },
+        (error) => {
+          if (error instanceof TypeError) {
+            throw new RequestError(
+              { code: 'request.invalid_input', details: error.message },
+              error
+            );
+          }
+          throw error;
         }
-        throw error;
+      );
-      }
+    }
-    );
+  );
-  });
 
   router.post(
     '/roles',
@@ -92,6 +116,8 @@ export default function roleRoutes<T extends AuthedRouter>(
       body: Roles.createGuard
         .omit({ id: true })
         .extend({ scopeIds: z.string().min(1).array().optional() }),
+      status: [200, 422],
+      response: Roles.guard,
     }),
     async (ctx, next) => {
       const { body } = ctx.guard;
@@ -128,6 +154,8 @@ export default function roleRoutes<T extends AuthedRouter>(
     '/roles/:id',
     koaGuard({
       params: object({ id: string().min(1) }),
+      response: Roles.guard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -145,6 +173,8 @@ export default function roleRoutes<T extends AuthedRouter>(
     koaGuard({
       body: Roles.createGuard.pick({ name: true, description: true }).partial(),
       params: object({ id: string().min(1) }),
+      response: Roles.guard,
+      status: [200, 404, 422],
     }),
     async (ctx, next) => {
       const {
@@ -172,6 +202,7 @@ export default function roleRoutes<T extends AuthedRouter>(
     '/roles/:id',
     koaGuard({
       params: object({ id: string().min(1) }),
+      status: [204, 404],
     }),
     async (ctx, next) => {
       const {
@@ -189,6 +220,8 @@ export default function roleRoutes<T extends AuthedRouter>(
     koaPagination(),
     koaGuard({
       params: object({ id: string().min(1) }),
+      response: userInfoResponseGuard.array(),
+      status: [200, 400, 404],
     }),
     async (ctx, next) => {
       const {
@@ -232,7 +265,8 @@ export default function roleRoutes<T extends AuthedRouter>(
     '/roles/:id/users',
     koaGuard({
       params: object({ id: string().min(1) }),
-      body: object({ userIds: string().min(1).array() }),
+      body: object({ userIds: string().min(1).array().nonempty() }),
+      status: [201, 404, 422],
     }),
     async (ctx, next) => {
       const {
@@ -265,6 +299,7 @@ export default function roleRoutes<T extends AuthedRouter>(
     '/roles/:id/users/:userId',
     koaGuard({
       params: object({ id: string().min(1), userId: string().min(1) }),
+      status: [204, 404],
     }),
     async (ctx, next) => {
       const {

packages/core/src/utils/http.ts

@@ -33,6 +33,7 @@ export const codeToMessage: Record<number, string> = Object.freeze({
   416: 'Requested Range Not Satisfiable',
   417: 'Expectation Failed',
   418: "I'm a teapot",
+  422: 'Unprocessable Content',
   429: 'Too Many Requests',
   500: 'Internal Server Error',
   501: 'Not Implemented',

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

@@ -22,6 +22,17 @@ describe('roles scopes', () => {
     expect(scopes[0]).toHaveProperty('id', scope.id);
   });
 
+  it('should return 404 if role not found', async () => {
+    const response = await getRoleScopes('not-found').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should return empty if role has no scopes', async () => {
+    const role = await createRole();
+    const scopes = await getRoleScopes(role.id);
+    expect(scopes.length).toBe(0);
+  });
+
   it('should assign scopes to role successfully', async () => {
     const role = await createRole();
     const resource = await createResource();
@@ -32,6 +43,47 @@ describe('roles scopes', () => {
     expect(scopes.length).toBe(2);
   });
 
+  it('should fail when try to assign empty scopes', async () => {
+    const role = await createRole();
+    const response = await assignScopesToRole([], role.id).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(400);
+  });
+
+  it('should fail with invalid scope input', async () => {
+    const role = await createRole();
+    const response = await assignScopesToRole([''], role.id).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(400);
+  });
+
+  it('should fail if role not found', async () => {
+    const resource = await createResource();
+    const scope = await createScope(resource.id);
+    const response = await assignScopesToRole([scope.id], 'not-found').catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail if scope not found', async () => {
+    const role = await createRole();
+    const response = await assignScopesToRole(['not-found'], role.id).catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail if scope already assigned to role', async () => {
+    const role = await createRole();
+    const resource = await createResource();
+    const scope1 = await createScope(resource.id);
+    const scope2 = await createScope(resource.id);
+    await assignScopesToRole([scope1.id], role.id);
+    const response = await assignScopesToRole([scope1.id, scope2.id], role.id).catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(422);
+  });
+
   it('should remove scope from role successfully', async () => {
     const role = await createRole();
     const resource = await createResource();
@@ -46,6 +98,23 @@ describe('roles scopes', () => {
     expect(newScopes.length).toBe(0);
   });
 
+  it('should fail when try to remove scope from role that is not assigned', async () => {
+    const role = await createRole();
+    const resource = await createResource();
+    const scope = await createScope(resource.id);
+    const response = await deleteScopeFromRole(scope.id, role.id).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail when try to remove scope from role that is not found', async () => {
+    const resource = await createResource();
+    const scope = await createScope(resource.id);
+    const response = await deleteScopeFromRole(scope.id, 'not-found').catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
   it('should fail when try to assign a scope to an internal role', async () => {
     const resource = await createResource();
     const scope = await createScope(resource.id);

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

@@ -65,6 +65,12 @@ describe('roles', () => {
     expect(role.description).toBe(createdRole.description);
   });
 
+  it('should return 404 if role does not exist', async () => {
+    const response = await getRole('non_existent_role').catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
   it('should update role details successfully', async () => {
     const role = await createRole();
 
@@ -91,6 +97,23 @@ describe('roles', () => {
     expect(response instanceof HTTPError && response.response.statusCode).toBe(422);
   });
 
+  it('should fail when update a non-existent role', async () => {
+    const response = await updateRole('non_existent_role', {
+      name: 'new_name',
+    }).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail when try to update an internal role', async () => {
+    const role = await createRole();
+
+    const response = await updateRole(role.id, {
+      name: '#internal:foo',
+    }).catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(403);
+  });
+
   it('should delete role successfully', async () => {
     const role = await createRole();
 
@@ -99,4 +122,10 @@ describe('roles', () => {
     const response = await getRole(role.id).catch((error: unknown) => error);
     expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
   });
+
+  it('should return 404 if role does not exist', async () => {
+    const response = await deleteRole('non_existent_role').catch((error: unknown) => error);
+
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
 });

packages/integration-tests/src/tests/api/role.user.test.ts

@@ -1,3 +1,5 @@
+import { HTTPError } from 'got';
+
 import { createUser } from '#src/api/index.js';
 import { assignUsersToRole, createRole, deleteUserFromRole, getRoleUsers } from '#src/api/role.js';
 import { generateNewUserProfile } from '#src/helpers/user.js';
@@ -13,6 +15,11 @@ describe('roles users', () => {
     expect(users[0]).toHaveProperty('id', user.id);
   });
 
+  it('should return 404 if role not found', async () => {
+    const response = await getRoleUsers('not-found').catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
   it('should assign users to role successfully', async () => {
     const role = await createRole();
     const user1 = await createUser(generateNewUserProfile({}));
@@ -23,6 +30,34 @@ describe('roles users', () => {
     expect(users.length).toBe(2);
   });
 
+  it('should fail when try to assign empty users', async () => {
+    const role = await createRole();
+    const response = await assignUsersToRole([], role.id).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(400);
+  });
+
+  it('should fail with invalid user input', async () => {
+    const role = await createRole();
+    const response = await assignUsersToRole([''], role.id).catch((error: unknown) => error);
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(400);
+  });
+
+  it('should fail if role not found', async () => {
+    const user = await createUser(generateNewUserProfile({}));
+    const response = await assignUsersToRole([user.id], 'not-found').catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail if user not found', async () => {
+    const role = await createRole();
+    const response = await assignUsersToRole(['not-found'], role.id).catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
   it('should remove user from role successfully', async () => {
     const role = await createRole();
     const user = await createUser(generateNewUserProfile({}));
@@ -35,4 +70,20 @@ describe('roles users', () => {
     const newUsers = await getRoleUsers(role.id);
     expect(newUsers.length).toBe(0);
   });
+
+  it('should fail if role not found when trying to remove user from role', async () => {
+    const user = await createUser(generateNewUserProfile({}));
+    const response = await deleteUserFromRole(user.id, 'not-found').catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
+
+  it('should fail if user not found when trying to  remove user from role', async () => {
+    const role = await createRole();
+    const response = await deleteUserFromRole('not-found', role.id).catch(
+      (error: unknown) => error
+    );
+    expect(response instanceof HTTPError && response.response.statusCode).toBe(404);
+  });
 });

packages/schemas/src/types/user.ts

@@ -1,4 +1,6 @@
-import type { CreateUser } from '../db-entries/index.js';
+import { Users } from '../db-entries/index.js';
+import type { User } from '../db-entries/index.js';
+import { type CreateGuard } from '../index.js';
 
 export const userInfoSelectFields = Object.freeze([
   'id',
@@ -15,11 +17,26 @@ export const userInfoSelectFields = Object.freeze([
   'isSuspended',
 ] as const);
 
-export type UserInfo<Keys extends keyof CreateUser = (typeof userInfoSelectFields)[number]> = Pick<
+export type UserInfo<Keys extends keyof User = (typeof userInfoSelectFields)[number]> = Pick<
-  CreateUser,
+  User,
   Keys
 >;
 
+export const userInfoResponseGuard: CreateGuard<UserInfo> = Users.guard.pick({
+  id: true,
+  username: true,
+  primaryEmail: true,
+  primaryPhone: true,
+  name: true,
+  avatar: true,
+  customData: true,
+  identities: true,
+  lastSignInAt: true,
+  createdAt: true,
+  applicationId: true,
+  isSuspended: true,
+});
+
 export type UserProfileResponse = UserInfo & { hasPassword?: boolean };
 
 /** Internal read-only roles for user tenants. */