test(core): add api response guard and error case tests to admin user (#3809)

test(core): add api response guard and error case tests to admin user api

packages/core/src/routes-me/user.ts

@@ -1,5 +1,4 @@
 import { emailRegEx, passwordRegEx, usernameRegEx } from '@logto/core-kit';
-import type { UserProfileResponse } from '@logto/schemas';
 import { userInfoSelectFields, jsonObjectGuard } from '@logto/schemas';
 import { conditional, pick } from '@silverhand/essentials';
 import { literal, object, string } from 'zod';
@@ -31,7 +30,7 @@ export default function userRoutes<T extends AuthedMeRouter>(
 
     const user = await findUserById(userId);
 
-    const responseData: UserProfileResponse = {
+    const responseData = {
       ...pick(user, ...userInfoSelectFields),
       ...conditional(user.passwordEncrypted && { hasPassword: Boolean(user.passwordEncrypted) }),
     };

packages/core/src/routes/admin-user-search.test.ts

@@ -0,0 +1,88 @@
+import type { CreateUser, Role, User } from '@logto/schemas';
+import { userInfoSelectFields } from '@logto/schemas';
+import { pickDefault } from '@logto/shared/esm';
+import { pick } from '@silverhand/essentials';
+
+import { mockUser, mockUserList, mockUserListResponse } from '#src/__mocks__/index.js';
+import type Libraries from '#src/tenants/Libraries.js';
+import type Queries from '#src/tenants/Queries.js';
+import { MockTenant, type Partial2 } from '#src/test-utils/tenant.js';
+import { createRequester } from '#src/utils/test-utils.js';
+
+const { jest } = import.meta;
+
+const filterUsersWithSearch = (users: User[], search: string) =>
+  users.filter((user) =>
+    [user.username, user.primaryEmail, user.primaryPhone, user.name].some((value) =>
+      value ? !value.includes(search) : false
+    )
+  );
+
+const mockedQueries = {
+  users: {
+    countUsers: jest.fn(async (search) => ({
+      count: search
+        ? filterUsersWithSearch(mockUserList, String(search)).length
+        : mockUserList.length,
+    })),
+    findUsers: jest.fn(
+      async (limit, offset, search): Promise<User[]> =>
+        // For testing, type should be `Search` but we use `string` in `filterUsersWithSearch()` here
+        // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+        search ? filterUsersWithSearch(mockUserList, String(search)) : mockUserList
+    ),
+  },
+  roles: {
+    findRolesByRoleNames: jest.fn(
+      async (): Promise<Role[]> => [
+        { tenantId: 'fake_tenant', id: 'role_id', name: 'admin', description: 'none' },
+      ]
+    ),
+  },
+  usersRoles: {
+    deleteUsersRolesByUserIdAndRoleId: jest.fn(),
+  },
+} satisfies Partial2<Queries>;
+
+const usersLibraries = {
+  generateUserId: jest.fn(async () => 'fooId'),
+  insertUser: jest.fn(
+    async (user: CreateUser): Promise<User> => ({
+      ...mockUser,
+      ...user,
+    })
+  ),
+} satisfies Partial<Libraries['users']>;
+
+const adminUserRoutes = await pickDefault(import('./admin-user-search.js'));
+
+describe('adminUserRoutes', () => {
+  const tenantContext = new MockTenant(undefined, mockedQueries, undefined, {
+    users: usersLibraries,
+  });
+  const userRequest = createRequester({ authedRoutes: adminUserRoutes, tenantContext });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('GET /users', async () => {
+    const response = await userRequest.get('/users');
+    expect(response.status).toEqual(200);
+    expect(response.body).toEqual(mockUserListResponse);
+    expect(response.header).toHaveProperty('total-number', `${mockUserList.length}`);
+  });
+
+  it('GET /users should return matched data', async () => {
+    const search = 'foo';
+    const response = await userRequest.get('/users').send({ search });
+    expect(response.status).toEqual(200);
+    expect(response.body).toEqual(
+      filterUsersWithSearch(mockUserList, search).map((user) => pick(user, ...userInfoSelectFields))
+    );
+    expect(response.header).toHaveProperty(
+      'total-number',
+      `${filterUsersWithSearch(mockUserList, search).length}`
+    );
+  });
+});

packages/core/src/routes/admin-user-search.ts

@@ -0,0 +1,61 @@
+import { userInfoSelectFields, userProfileResponseGuard } from '@logto/schemas';
+import { pick, tryThat } from '@silverhand/essentials';
+
+import RequestError from '#src/errors/RequestError/index.js';
+import koaGuard from '#src/middleware/koa-guard.js';
+import koaPagination from '#src/middleware/koa-pagination.js';
+import { parseSearchParamsForSearch } from '#src/utils/search.js';
+
+import type { AuthedRouter, RouterInitArgs } from './types.js';
+
+export default function adminUserSearchRoutes<T extends AuthedRouter>(
+  ...[router, { queries }]: RouterInitArgs<T>
+) {
+  const {
+    users: { findUsers, countUsers },
+    usersRoles: { findUsersRolesByRoleId },
+  } = queries;
+
+  router.get(
+    '/users',
+    koaPagination(),
+    koaGuard({
+      response: userProfileResponseGuard.array(),
+      status: [200, 400],
+    }),
+    async (ctx, next) => {
+      const { limit, offset } = ctx.pagination;
+      const { searchParams } = ctx.request.URL;
+
+      return tryThat(
+        async () => {
+          const search = parseSearchParamsForSearch(searchParams);
+          const excludeRoleId = searchParams.get('excludeRoleId');
+          const excludeUsersRoles = excludeRoleId
+            ? await findUsersRolesByRoleId(excludeRoleId)
+            : [];
+          const excludeUserIds = excludeUsersRoles.map(({ userId }) => userId);
+
+          const [{ count }, users] = await Promise.all([
+            countUsers(search, excludeUserIds),
+            findUsers(limit, offset, search, excludeUserIds),
+          ]);
+
+          ctx.pagination.totalCount = count;
+          ctx.body = users.map((user) => pick(user, ...userInfoSelectFields));
+
+          return next();
+        },
+        (error) => {
+          if (error instanceof TypeError) {
+            throw new RequestError(
+              { code: 'request.invalid_input', details: error.message },
+              error
+            );
+          }
+          throw error;
+        }
+      );
+    }
+  );
+}

packages/core/src/routes/admin-user.test.ts

@@ -1,15 +1,7 @@
-/* eslint-disable max-lines */
 import type { CreateUser, Role, SignInExperience, User } from '@logto/schemas';
-import { userInfoSelectFields } from '@logto/schemas';
 import { createMockUtils, pickDefault } from '@logto/shared/esm';
-import { pick } from '@silverhand/essentials';
 
-import {
-  mockUser,
-  mockUserList,
-  mockUserListResponse,
-  mockUserResponse,
-} from '#src/__mocks__/index.js';
+import { mockUser, mockUserResponse } from '#src/__mocks__/index.js';
 import RequestError from '#src/errors/RequestError/index.js';
 import type Libraries from '#src/tenants/Libraries.js';
 import type Queries from '#src/tenants/Queries.js';
@@ -43,17 +35,6 @@ const mockedQueries = {
     ),
   },
   users: {
-    countUsers: jest.fn(async (search) => ({
-      count: search
-        ? filterUsersWithSearch(mockUserList, String(search)).length
-        : mockUserList.length,
-    })),
-    findUsers: jest.fn(
-      async (limit, offset, search): Promise<User[]> =>
-        // For testing, type should be `Search` but we use `string` in `filterUsersWithSearch()` here
-        // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-        search ? filterUsersWithSearch(mockUserList, String(search)) : mockUserList
-    ),
     findUserById: jest.fn(async (id: string) => mockUser),
     hasUser: jest.fn(async () => mockHasUser()),
     hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
@@ -120,26 +101,6 @@ describe('adminUserRoutes', () => {
     jest.clearAllMocks();
   });
 
-  it('GET /users', async () => {
-    const response = await userRequest.get('/users');
-    expect(response.status).toEqual(200);
-    expect(response.body).toEqual(mockUserListResponse);
-    expect(response.header).toHaveProperty('total-number', `${mockUserList.length}`);
-  });
-
-  it('GET /users should return matched data', async () => {
-    const search = 'foo';
-    const response = await userRequest.get('/users').send({ search });
-    expect(response.status).toEqual(200);
-    expect(response.body).toEqual(
-      filterUsersWithSearch(mockUserList, search).map((user) => pick(user, ...userInfoSelectFields))
-    );
-    expect(response.header).toHaveProperty(
-      'total-number',
-      `${filterUsersWithSearch(mockUserList, search).length}`
-    );
-  });
-
   it('GET /users/:userId', async () => {
     const response = await userRequest.get('/users/foo');
     expect(response.status).toEqual(200);
@@ -474,4 +435,3 @@ describe('adminUserRoutes', () => {
     expect(deleteUserIdentity).toHaveBeenCalledWith(arbitraryUserId, arbitraryTarget);
   });
 });
-/* eslint-enable max-lines */

packages/core/src/routes/admin-user.ts

@@ -1,14 +1,12 @@
 import { emailRegEx, passwordRegEx, phoneRegEx, usernameRegEx } from '@logto/core-kit';
-import { jsonObjectGuard, userInfoSelectFields } from '@logto/schemas';
-import { conditional, has, pick, tryThat } from '@silverhand/essentials';
+import { jsonObjectGuard, userInfoSelectFields, userProfileResponseGuard } from '@logto/schemas';
+import { conditional, has, pick } from '@silverhand/essentials';
 import { boolean, literal, object, string } from 'zod';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js';
 import koaGuard from '#src/middleware/koa-guard.js';
-import koaPagination from '#src/middleware/koa-pagination.js';
 import assertThat from '#src/utils/assert-that.js';
-import { parseSearchParamsForSearch } from '#src/utils/search.js';
 
 import type { AuthedRouter, RouterInitArgs } from './types.js';
 
@@ -20,54 +18,23 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     users: {
       deleteUserById,
       deleteUserIdentity,
-      findUsers,
-      countUsers,
       findUserById,
       hasUser,
       updateUserById,
       hasUserWithEmail,
       hasUserWithPhone,
     },
-    usersRoles: { findUsersRolesByRoleId },
   } = queries;
   const {
-    users: { checkIdentifierCollision, generateUserId, insertUser, findUsersByRoleName },
+    users: { checkIdentifierCollision, generateUserId, insertUser },
   } = libraries;
 
-  router.get('/users', koaPagination(), async (ctx, next) => {
-    const { limit, offset } = ctx.pagination;
-    const { searchParams } = ctx.request.URL;
-
-    return tryThat(
-      async () => {
-        const search = parseSearchParamsForSearch(searchParams);
-        const excludeRoleId = searchParams.get('excludeRoleId');
-        const excludeUsersRoles = excludeRoleId ? await findUsersRolesByRoleId(excludeRoleId) : [];
-        const excludeUserIds = excludeUsersRoles.map(({ userId }) => userId);
-
-        const [{ count }, users] = await Promise.all([
-          countUsers(search, excludeUserIds),
-          findUsers(limit, offset, search, excludeUserIds),
-        ]);
-
-        ctx.pagination.totalCount = count;
-        ctx.body = users.map((user) => pick(user, ...userInfoSelectFields));
-
-        return next();
-      },
-      (error) => {
-        if (error instanceof TypeError) {
-          throw new RequestError({ code: 'request.invalid_input', details: error.message }, error);
-        }
-        throw error;
-      }
-    );
-  });
-
   router.get(
     '/users/:userId',
     koaGuard({
       params: object({ userId: string() }),
+      response: userProfileResponseGuard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -87,6 +54,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ userId: string() }),
       response: jsonObjectGuard,
+      status: [200],
     }),
     async (ctx, next) => {
       const {
@@ -106,6 +74,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
       params: object({ userId: string() }),
       body: object({ customData: jsonObjectGuard }),
       response: jsonObjectGuard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -135,6 +104,8 @@ export default function adminUserRoutes<T extends AuthedRouter>(
         password: string().regex(passwordRegEx),
         name: string(),
       }).partial(),
+      response: userProfileResponseGuard,
+      status: [200, 404, 422],
     }),
     async (ctx, next) => {
       const { primaryEmail, primaryPhone, username, password, name } = ctx.guard.body;
@@ -155,7 +126,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
       );
       assertThat(
         !primaryPhone || !(await hasUserWithPhone(primaryPhone)),
-        new RequestError({ code: 'user.phone_already_in_use' })
+        new RequestError({ code: 'user.phone_already_in_use', status: 422 })
       );
 
       const id = await generateUserId();
@@ -190,6 +161,8 @@ export default function adminUserRoutes<T extends AuthedRouter>(
         avatar: string().url().or(literal('')).nullable(),
         customData: jsonObjectGuard,
       }).partial(),
+      response: userProfileResponseGuard,
+      status: [200, 404, 422],
     }),
     async (ctx, next) => {
       const {
@@ -212,6 +185,8 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ userId: string() }),
       body: object({ password: string().regex(passwordRegEx) }),
+      response: userProfileResponseGuard,
+      status: [200, 422],
     }),
     async (ctx, next) => {
       const {
@@ -239,6 +214,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ userId: string() }),
       body: object({ password: string() }),
+      status: [204],
     }),
     async (ctx, next) => {
       const {
@@ -260,7 +236,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ userId: string() }),
       response: object({ hasPassword: boolean() }),
-      status: [200],
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const { userId } = ctx.guard.params;
@@ -279,6 +255,8 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     koaGuard({
       params: object({ userId: string() }),
       body: object({ isSuspended: boolean() }),
+      response: userProfileResponseGuard,
+      status: [200, 404],
     }),
     async (ctx, next) => {
       const {
@@ -306,6 +284,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(
     '/users/:userId',
     koaGuard({
       params: object({ userId: string() }),
+      status: [204, 400, 404],
     }),
     async (ctx, next) => {
       const {
@@ -326,7 +305,11 @@ export default function adminUserRoutes<T extends AuthedRouter>(
 
   router.delete(
     '/users/:userId/identities/:target',
-    koaGuard({ params: object({ userId: string(), target: string() }) }),
+    koaGuard({
+      params: object({ userId: string(), target: string() }),
+      response: userProfileResponseGuard,
+      status: [200, 404],
+    }),
     async (ctx, next) => {
       const {
         params: { userId, target },

packages/core/src/routes/init.ts

@@ -10,6 +10,7 @@ import type TenantContext from '#src/tenants/TenantContext.js';
 import koaAuth from '../middleware/koa-auth/index.js';
 
 import adminUserRoleRoutes from './admin-user-role.js';
+import adminUserSearchRoutes from './admin-user-search.js';
 import adminUserRoutes from './admin-user.js';
 import applicationRoutes from './application.js';
 import authnRoutes from './authn.js';
@@ -43,6 +44,7 @@ const createRouters = (tenant: TenantContext) => {
   resourceRoutes(managementRouter, tenant);
   signInExperiencesRoutes(managementRouter, tenant);
   adminUserRoutes(managementRouter, tenant);
+  adminUserSearchRoutes(managementRouter, tenant);
   adminUserRoleRoutes(managementRouter, tenant);
   logRoutes(managementRouter, tenant);
   roleRoutes(managementRouter, tenant);

packages/core/src/routes/role.ts

@@ -1,5 +1,5 @@
 import type { RoleResponse } from '@logto/schemas';
-import { userInfoSelectFields, userInfoResponseGuard, Roles, Users } from '@logto/schemas';
+import { userInfoSelectFields, userProfileResponseGuard, Roles, Users } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { pick, tryThat } from '@silverhand/essentials';
 import { object, string, z, number } from 'zod';
@@ -220,7 +220,7 @@ export default function roleRoutes<T extends AuthedRouter>(
     koaPagination(),
     koaGuard({
       params: object({ id: string().min(1) }),
-      response: userInfoResponseGuard.array(),
+      response: userProfileResponseGuard.array(),
       status: [200, 400, 404],
     }),
     async (ctx, next) => {

packages/integration-tests/src/tests/api/admin-user.roles.test.ts

@@ -3,6 +3,7 @@ import { HTTPError } from 'got';
 
 import { assignRolesToUser, getUserRoles, deleteRoleFromUser } from '#src/api/index.js';
 import { createRole } from '#src/api/role.js';
+import { createResponseWithCode } from '#src/helpers/admin-tenant.js';
 import { createUserByAdmin } from '#src/helpers/index.js';
 
 describe('admin console user management (roles)', () => {
@@ -22,6 +23,16 @@ describe('admin console user management (roles)', () => {
     expect(roles[0]).toHaveProperty('id', role.id);
   });
 
+  it('should fail when assign duplicated role to user', async () => {
+    const user = await createUserByAdmin();
+    const role = await createRole();
+
+    await assignRolesToUser(user.id, [role.id]);
+    await expect(assignRolesToUser(user.id, [role.id])).rejects.toMatchObject(
+      createResponseWithCode(422)
+    );
+  });
+
   it('should delete role from user successfully', async () => {
     const user = await createUserByAdmin();
 

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

@@ -7,7 +7,6 @@ import {
 } from '#src/__mocks__/connectors-mock.js';
 import {
   getUser,
-  getUsers,
   updateUser,
   deleteUser,
   updateUserPassword,
@@ -16,22 +15,40 @@ import {
   updateConnectorConfig,
   deleteConnectorById,
 } from '#src/api/index.js';
+import { createResponseWithCode } from '#src/helpers/admin-tenant.js';
 import { createUserByAdmin } from '#src/helpers/index.js';
 import { createNewSocialUserWithUsernameAndPassword } from '#src/helpers/interactions.js';
+import { generateUsername, generateEmail, generatePhone, generatePassword } from '#src/utils.js';
 
 describe('admin console user management', () => {
-  it('should create user successfully', async () => {
+  it('should create and get user successfully', async () => {
     const user = await createUserByAdmin();
 
     const userDetails = await getUser(user.id);
     expect(userDetails.id).toBe(user.id);
   });
 
-  it('should get user list successfully', async () => {
-    await createUserByAdmin();
-    const users = await getUsers();
+  it('should fail when create user with conflict identifiers', async () => {
+    const [username, password, email, phone] = [
+      generateUsername(),
+      generatePassword(),
+      generateEmail(),
+      generatePhone(),
+    ];
+    await createUserByAdmin(username, password, email, phone);
+    await expect(createUserByAdmin(username, password)).rejects.toMatchObject(
+      createResponseWithCode(422)
+    );
+    await expect(createUserByAdmin(undefined, undefined, email)).rejects.toMatchObject(
+      createResponseWithCode(422)
+    );
+    await expect(createUserByAdmin(undefined, undefined, undefined, phone)).rejects.toMatchObject(
+      createResponseWithCode(422)
+    );
+  });
 
-    expect(users.length).not.toBeLessThan(1);
+  it('should fail when get user by invalid id', async () => {
+    await expect(getUser('invalid-user-id')).rejects.toMatchObject(createResponseWithCode(404));
   });
 
   it('should update userinfo successfully', async () => {
@@ -50,6 +67,15 @@ describe('admin console user management', () => {
     expect(updatedUser).toMatchObject(newUserData);
   });
 
+  it('should fail when update userinfo with conflict identifiers', async () => {
+    const user = await createUserByAdmin();
+    const anotherUser = await createUserByAdmin();
+
+    await expect(updateUser(user.id, { username: anotherUser.username })).rejects.toMatchObject(
+      createResponseWithCode(422)
+    );
+  });
+
   it('should delete user successfully', async () => {
     const user = await createUserByAdmin();
 

packages/schemas/src/types/user.ts

@@ -1,6 +1,6 @@
+import { z } from 'zod';
+
 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',
@@ -17,27 +17,17 @@ export const userInfoSelectFields = Object.freeze([
   'isSuspended',
 ] as const);
 
-export type UserInfo<Keys extends keyof User = (typeof userInfoSelectFields)[number]> = Pick<
-  User,
-  Keys
->;
+export const userInfoGuard = Users.guard.pick(
+  Object.fromEntries(userInfoSelectFields.map((key) => [key, true]))
+);
 
-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 UserInfo = z.infer<typeof userInfoGuard>;
+
+export const userProfileResponseGuard = userInfoGuard.extend({
+  hasPassword: z.boolean().optional(),
 });
 
-export type UserProfileResponse = UserInfo & { hasPassword?: boolean };
+export type UserProfileResponse = z.infer<typeof userProfileResponseGuard>;
 
 /** Internal read-only roles for user tenants. */
 export enum InternalRole {