From a748fc85bbaa8eb0ffb359d7ff3ef931c7b909af Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Wed, 4 Sep 2024 13:22:44 +0800 Subject: [PATCH] fix(core): add `hasPassword` field to user API response (#6543) --- .changeset/polite-bats-learn.md | 5 +++ packages/core/src/__mocks__/user.ts | 9 ++-- packages/core/src/routes/admin-user/basics.ts | 26 +++++------ .../core/src/routes/admin-user/search.test.ts | 8 ++-- packages/core/src/routes/admin-user/search.ts | 12 ++---- packages/core/src/routes/admin-user/social.ts | 6 +-- packages/core/src/routes/role.user.ts | 8 ++-- packages/core/src/utils/user.ts | 43 ++++++++++++++++++- .../integration-tests/src/helpers/user.ts | 8 ++-- .../src/tests/api/admin-user.test.ts | 6 ++- .../organization/organization-user.test.ts | 12 +++++- 11 files changed, 99 insertions(+), 44 deletions(-) create mode 100644 .changeset/polite-bats-learn.md diff --git a/.changeset/polite-bats-learn.md b/.changeset/polite-bats-learn.md new file mode 100644 index 000000000..5dab18a9d --- /dev/null +++ b/.changeset/polite-bats-learn.md @@ -0,0 +1,5 @@ +--- +"@logto/core": patch +--- + +fix: add `hasPassword` field to user API response diff --git a/packages/core/src/__mocks__/user.ts b/packages/core/src/__mocks__/user.ts index 14bfe03f1..5489b1bde 100644 --- a/packages/core/src/__mocks__/user.ts +++ b/packages/core/src/__mocks__/user.ts @@ -1,6 +1,7 @@ import type { User } from '@logto/schemas'; -import { MfaFactor, userInfoSelectFields, UsersPasswordEncryptionMethod } from '@logto/schemas'; -import { pick } from '@silverhand/essentials'; +import { MfaFactor, UsersPasswordEncryptionMethod } from '@logto/schemas'; + +import { transpileUserProfileResponse } from '../utils/user.js'; export const mockUser: User = { tenantId: 'fake_tenant', @@ -56,7 +57,7 @@ export const mockUserWithMfaVerifications: User = { mfaVerifications: [mockUserTotpMfaVerification], }; -export const mockUserResponse = pick(mockUser, ...userInfoSelectFields); +export const mockUserResponse = transpileUserProfileResponse(mockUser); export const mockPasswordEncrypted = 'a1b2c3'; export const mockUserWithPassword: User = { @@ -191,4 +192,4 @@ export const mockUserList: User[] = [ }, ]; -export const mockUserListResponse = mockUserList.map((user) => pick(user, ...userInfoSelectFields)); +export const mockUserListResponse = mockUserList.map((user) => transpileUserProfileResponse(user)); diff --git a/packages/core/src/routes/admin-user/basics.ts b/packages/core/src/routes/admin-user/basics.ts index b889655ea..1a8d794f3 100644 --- a/packages/core/src/routes/admin-user/basics.ts +++ b/packages/core/src/routes/admin-user/basics.ts @@ -3,11 +3,10 @@ import { emailRegEx, phoneRegEx, usernameRegEx } from '@logto/core-kit'; import { UsersPasswordEncryptionMethod, jsonObjectGuard, - userInfoSelectFields, userProfileGuard, userProfileResponseGuard, } from '@logto/schemas'; -import { conditional, pick, yes } from '@silverhand/essentials'; +import { conditional, yes } from '@silverhand/essentials'; import { boolean, literal, nativeEnum, object, string } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; @@ -16,6 +15,7 @@ import { encryptUserPassword } from '#src/libraries/user.utils.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; +import { transpileUserProfileResponse } from '../../utils/user.js'; import type { ManagementApiRouter, RouterInitArgs } from '../types.js'; export default function adminUserBasicsRoutes( @@ -54,20 +54,16 @@ export default function adminUserBasicsRoutes( async (ctx, next) => { const { params: { userId }, - query: { includeSsoIdentities }, + query: { includeSsoIdentities = 'false' }, } = ctx.guard; const user = await findUserById(userId); - ctx.body = { - ...pick(user, ...userInfoSelectFields), - ...conditional( - includeSsoIdentities && - yes(includeSsoIdentities) && { - ssoIdentities: await findUserSsoIdentities(userId), - } + ctx.body = transpileUserProfileResponse(user, { + ssoIdentities: conditional( + yes(includeSsoIdentities) && [...(await findUserSsoIdentities(userId))] ), - }; + }); return next(); } @@ -221,7 +217,7 @@ export default function adminUserBasicsRoutes( [] ); - ctx.body = pick(user, ...userInfoSelectFields); + ctx.body = transpileUserProfileResponse(user); return next(); } ); @@ -252,7 +248,7 @@ export default function adminUserBasicsRoutes( await checkIdentifierCollision(body, userId); const updatedUser = await updateUserById(userId, body, 'replace'); - ctx.body = pick(updatedUser, ...userInfoSelectFields); + ctx.body = transpileUserProfileResponse(updatedUser); return next(); } @@ -281,7 +277,7 @@ export default function adminUserBasicsRoutes( passwordEncryptionMethod, }); - ctx.body = pick(user, ...userInfoSelectFields); + ctx.body = transpileUserProfileResponse(user); return next(); } @@ -352,7 +348,7 @@ export default function adminUserBasicsRoutes( await signOutUser(user.id); } - ctx.body = pick(user, ...userInfoSelectFields); + ctx.body = transpileUserProfileResponse(user); return next(); } diff --git a/packages/core/src/routes/admin-user/search.test.ts b/packages/core/src/routes/admin-user/search.test.ts index 2d33d5c31..11a743c84 100644 --- a/packages/core/src/routes/admin-user/search.test.ts +++ b/packages/core/src/routes/admin-user/search.test.ts @@ -1,7 +1,7 @@ import type { CreateUser, Role, User } from '@logto/schemas'; -import { userInfoSelectFields, RoleType } from '@logto/schemas'; +import { RoleType } from '@logto/schemas'; import { pickDefault } from '@logto/shared/esm'; -import { pick, removeUndefinedKeys } from '@silverhand/essentials'; +import { removeUndefinedKeys } from '@silverhand/essentials'; import { mockUser, mockUserList, mockUserListResponse } from '#src/__mocks__/index.js'; import { type InsertUserResult } from '#src/libraries/user.js'; @@ -10,6 +10,8 @@ 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'; +import { transpileUserProfileResponse } from '../../utils/user.js'; + const { jest } = import.meta; const filterUsersWithSearch = (users: User[], search: string) => @@ -86,7 +88,7 @@ describe('adminUserRoutes', () => { 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)) + filterUsersWithSearch(mockUserList, search).map((user) => transpileUserProfileResponse(user)) ); expect(response.header).toHaveProperty( 'total-number', diff --git a/packages/core/src/routes/admin-user/search.ts b/packages/core/src/routes/admin-user/search.ts index 0f23168f7..73c727785 100644 --- a/packages/core/src/routes/admin-user/search.ts +++ b/packages/core/src/routes/admin-user/search.ts @@ -1,10 +1,5 @@ -import { - OrganizationUserRelations, - UsersRoles, - userInfoSelectFields, - userProfileResponseGuard, -} from '@logto/schemas'; -import { type Nullable, pick, tryThat } from '@silverhand/essentials'; +import { OrganizationUserRelations, UsersRoles, userProfileResponseGuard } from '@logto/schemas'; +import { type Nullable, tryThat } from '@silverhand/essentials'; import RequestError from '#src/errors/RequestError/index.js'; import koaGuard from '#src/middleware/koa-guard.js'; @@ -12,6 +7,7 @@ import koaPagination from '#src/middleware/koa-pagination.js'; import { type UserConditions } from '#src/queries/user.js'; import { parseSearchParamsForSearch } from '#src/utils/search.js'; +import { transpileUserProfileResponse } from '../../utils/user.js'; import type { ManagementApiRouter, RouterInitArgs } from '../types.js'; const getQueryRelation = ( @@ -82,7 +78,7 @@ export default function adminUserSearchRoutes( ]); ctx.pagination.totalCount = count; - ctx.body = users.map((user) => pick(user, ...userInfoSelectFields)); + ctx.body = users.map((user) => transpileUserProfileResponse(user)); return next(); }, diff --git a/packages/core/src/routes/admin-user/social.ts b/packages/core/src/routes/admin-user/social.ts index 888d05130..35ac73832 100644 --- a/packages/core/src/routes/admin-user/social.ts +++ b/packages/core/src/routes/admin-user/social.ts @@ -3,16 +3,16 @@ import { ConnectorType, identityGuard, identitiesGuard, - userInfoSelectFields, userProfileResponseGuard, } from '@logto/schemas'; -import { has, pick } from '@silverhand/essentials'; +import { has } from '@silverhand/essentials'; import { object, record, string, unknown } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; +import { transpileUserProfileResponse } from '../../utils/user.js'; import type { ManagementApiRouter, RouterInitArgs } from '../types.js'; export default function adminUserSocialRoutes( @@ -149,7 +149,7 @@ export default function adminUserSocialRoutes( } const updatedUser = await deleteUserIdentity(userId, target); - ctx.body = pick(updatedUser, ...userInfoSelectFields); + ctx.body = transpileUserProfileResponse(updatedUser); return next(); } diff --git a/packages/core/src/routes/role.user.ts b/packages/core/src/routes/role.user.ts index 2e7c5f6f9..9c74dd035 100644 --- a/packages/core/src/routes/role.user.ts +++ b/packages/core/src/routes/role.user.ts @@ -1,6 +1,6 @@ -import { UsersRoles, userInfoSelectFields, userProfileResponseGuard } from '@logto/schemas'; +import { UsersRoles, userProfileResponseGuard } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; -import { pick, tryThat } from '@silverhand/essentials'; +import { tryThat } from '@silverhand/essentials'; import { object, string } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; @@ -9,6 +9,8 @@ import koaPagination from '#src/middleware/koa-pagination.js'; import { type UserConditions } from '#src/queries/user.js'; import { parseSearchParamsForSearch } from '#src/utils/search.js'; +import { transpileUserProfileResponse } from '../utils/user.js'; + import type { ManagementApiRouter, RouterInitArgs } from './types.js'; export default function roleUserRoutes( @@ -59,7 +61,7 @@ export default function roleUserRoutes( ]); ctx.pagination.totalCount = count; - ctx.body = users.map((user) => pick(user, ...userInfoSelectFields)); + ctx.body = users.map((user) => transpileUserProfileResponse(user)); return next(); }, diff --git a/packages/core/src/utils/user.ts b/packages/core/src/utils/user.ts index d7d5df661..43f911de4 100644 --- a/packages/core/src/utils/user.ts +++ b/packages/core/src/utils/user.ts @@ -1,4 +1,12 @@ -import { MfaFactor, type User, type UserMfaVerificationResponse } from '@logto/schemas'; +import { + MfaFactor, + userInfoSelectFields, + type UserProfileResponse, + type UserSsoIdentity, + type User, + type UserMfaVerificationResponse, +} from '@logto/schemas'; +import { pick } from '@silverhand/essentials'; export const transpileUserMfaVerifications = ( mfaVerifications: User['mfaVerifications'] @@ -21,3 +29,36 @@ export const transpileUserMfaVerifications = ( return { id, createdAt, type }; }); }; + +type ExtraUserInfo = { + ssoIdentities?: UserSsoIdentity[]; +}; + +/** + * Transforms user data into a user profile response format + * + * This function is used when API endpoints return user profile information, + * converting the internal user data model to an external user profile response format. + * + * Main purposes: + * + * 1. Selectively return user information fields + * 2. Add additional user-related information (e.g., SSO identities) + * 3. Handle password-related information + * + * @param user - Internal user data model + * @param extraInfo - Additional user-related information, such as SSO identities + * @returns Formatted user profile response object + */ +export const transpileUserProfileResponse = ( + user: User, + extraInfo: ExtraUserInfo = {} +): UserProfileResponse => { + const { ssoIdentities } = extraInfo; + + return { + ...pick(user, ...userInfoSelectFields), + hasPassword: Boolean(user.passwordEncrypted), + ...(ssoIdentities && { ssoIdentities }), + }; +}; diff --git a/packages/integration-tests/src/helpers/user.ts b/packages/integration-tests/src/helpers/user.ts index 86514d4bc..13342000c 100644 --- a/packages/integration-tests/src/helpers/user.ts +++ b/packages/integration-tests/src/helpers/user.ts @@ -1,4 +1,4 @@ -import { type User } from '@logto/schemas'; +import { type UserProfileResponse } from '@logto/schemas'; import { trySafe } from '@silverhand/essentials'; import { type CreateUserPayload, createUser, deleteUser } from '#src/api/index.js'; @@ -51,13 +51,13 @@ export const generateNewUser = async (options: }; export class UserApiTest { - #users: User[] = []; + #users: UserProfileResponse[] = []; - get users(): User[] { + get users(): UserProfileResponse[] { return this.#users; } - async create(data: CreateUserPayload): Promise { + async create(data: CreateUserPayload): Promise { const user = await createUser(data); // eslint-disable-next-line @silverhand/fp/no-mutating-methods this.users.push(user); diff --git a/packages/integration-tests/src/tests/api/admin-user.test.ts b/packages/integration-tests/src/tests/api/admin-user.test.ts index 9ee8ec7f1..e81ec5b96 100644 --- a/packages/integration-tests/src/tests/api/admin-user.test.ts +++ b/packages/integration-tests/src/tests/api/admin-user.test.ts @@ -201,7 +201,11 @@ describe('admin console user management', () => { it('should update user password successfully', async () => { const { updatedAt, ...rest } = await createUserByAdmin(); const userEntity = await updateUserPassword(rest.id, 'new_password'); - expect(userEntity).toMatchObject(rest); + expect(userEntity).toMatchObject({ + ...rest, + // Since the password is updated, the hasPassword field will be true. + hasPassword: true, + }); expect(userEntity.updatedAt).toBeGreaterThan(updatedAt); }); diff --git a/packages/integration-tests/src/tests/api/organization/organization-user.test.ts b/packages/integration-tests/src/tests/api/organization/organization-user.test.ts index 26604555f..d08d3e66b 100644 --- a/packages/integration-tests/src/tests/api/organization/organization-user.test.ts +++ b/packages/integration-tests/src/tests/api/organization/organization-user.test.ts @@ -47,7 +47,11 @@ describe('organization user APIs', () => { it('should be able to get organization users with search', async () => { const organizationId = organizationApi.organizations[0]!.id; const username = generateTestName(); - const createdUser = await userApi.create({ username }); + /** + * Exclude `hasPassword` field since the user type returned by the organization user API is not `userProfileResponse`. + * So the `hasPassword` field will not be included in the user object. + */ + const { hasPassword, ...createdUser } = await userApi.create({ username }); await organizationApi.addUsers(organizationId, [createdUser.id]); const [users] = await organizationApi.getUsers(organizationId, { @@ -59,7 +63,11 @@ describe('organization user APIs', () => { it('should be able to get organization users with their roles', async () => { const organizationId = organizationApi.organizations[0]!.id; - const user = userApi.users[0]!; + /** + * Exclude `hasPassword` field since the user type returned by the organization user API is not `userProfileResponse`. + * So the `hasPassword` field will not be included in the user object. + */ + const { hasPassword, ...user } = userApi.users[0]!; const roles = await Promise.all([ organizationApi.roleApi.create({ name: generateTestName() }),