Skip to content

Commit 4a854dd

Browse files
darcyYedarcyYe
committed
feat: add phone number validation to user APIs
1 parent 393cf44 commit 4a854dd

30 files changed

+251
-137
lines changed

‎packages/console/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
"jest-transformer-svg": "^2.0.0",
8787
"just-kebab-case": "^4.2.0",
8888
"ky": "^1.2.3",
89-
"libphonenumber-js": "^1.10.51",
89+
"libphonenumber-js": "^1.11.1",
9090
"lint-staged": "^15.0.0",
9191
"nanoid": "^5.0.1",
9292
"overlayscrollbars": "^2.0.2",

‎packages/console/src/pages/UserDetails/UserSettings/index.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { emailRegEx, usernameRegEx } from '@logto/core-kit';
22
import type { User } from '@logto/schemas';
33
import { parsePhoneNumber } from '@logto/shared/universal';
44
import { conditionalString, trySafe } from '@silverhand/essentials';
5-
import { parsePhoneNumberWithError } from 'libphonenumber-js';
5+
import { parsePhoneNumberWithError } from 'libphonenumber-js/mobile';
66
import { useForm, useController } from 'react-hook-form';
77
import { toast } from 'react-hot-toast';
88
import { useTranslation } from 'react-i18next';

‎packages/core/src/libraries/user.test.ts

+22-9
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,15 @@ describe('verifyUserPassword()', () => {
186186
};
187187
it('migrates password to Argon2', async () => {
188188
await verifyUserPassword(user, 'password');
189-
expect(updateUserById).toHaveBeenCalledWith(user.id, {
190-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
191-
passwordEncrypted: expect.stringContaining('argon2'),
192-
passwordEncryptionMethod: UsersPasswordEncryptionMethod.Argon2i,
193-
});
189+
expect(updateUserById).toHaveBeenCalledWith(
190+
user.id,
191+
{
192+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
193+
passwordEncrypted: expect.stringContaining('argon2'),
194+
passwordEncryptionMethod: UsersPasswordEncryptionMethod.Argon2i,
195+
},
196+
undefined
197+
);
194198
});
195199
});
196200
});
@@ -220,17 +224,26 @@ describe('addUserMfaVerification()', () => {
220224
beforeAll(() => {
221225
jest.useFakeTimers();
222226
jest.setSystemTime(new Date(createdAt));
227+
jest.clearAllMocks();
223228
});
224229

225230
afterAll(() => {
226231
jest.useRealTimers();
227232
});
228233

229234
it('update user with new mfa verification', async () => {
230-
await addUserMfaVerification(mockUser.id, { type: MfaFactor.TOTP, secret: 'secret' });
231-
expect(updateUserById).toHaveBeenCalledWith(mockUser.id, {
232-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
233-
mfaVerifications: [{ type: MfaFactor.TOTP, key: 'secret', id: expect.anything(), createdAt }],
235+
await addUserMfaVerification(mockUser.id, {
236+
type: MfaFactor.TOTP,
237+
secret: 'secret',
234238
});
239+
expect(updateUserById).toHaveBeenCalledWith(
240+
mockUser.id,
241+
{
242+
mfaVerifications: [
243+
{ type: MfaFactor.TOTP, key: 'secret', id: expect.anything(), createdAt },
244+
],
245+
},
246+
undefined
247+
);
235248
});
236249
});

‎packages/core/src/libraries/user.ts

+32-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { User, CreateUser, Scope, BindMfa, MfaVerification } from '@logto/s
22
import { MfaFactor, Users, UsersPasswordEncryptionMethod } from '@logto/schemas';
33
import { generateStandardShortId, generateStandardId } from '@logto/shared';
44
import type { Nullable } from '@silverhand/essentials';
5-
import { deduplicate } from '@silverhand/essentials';
5+
import { deduplicate, conditional } from '@silverhand/essentials';
66
import { argon2Verify, bcryptVerify, md5, sha1, sha256 } from 'hash-wasm';
77
import pRetry from 'p-retry';
88

@@ -14,6 +14,7 @@ import type Queries from '#src/tenants/Queries.js';
1414
import assertThat from '#src/utils/assert-that.js';
1515
import { encryptPassword } from '#src/utils/password.js';
1616
import type { OmitAutoSetFields } from '#src/utils/sql.js';
17+
import { getValidPhoneNumber } from '#src/utils/user.js';
1718

1819
export const encryptUserPassword = async (
1920
password: string
@@ -82,7 +83,7 @@ export const createUserLibrary = (queries: Queries) => {
8283
hasUserWithId,
8384
hasUserWithPhone,
8485
findUsersByIds,
85-
updateUserById,
86+
updateUserById: updateUserByIdQuery,
8687
findUserById,
8788
},
8889
usersRoles: { findUsersRolesByRoleId, findUsersRolesByUserId },
@@ -105,18 +106,45 @@ export const createUserLibrary = (queries: Queries) => {
105106
{ retries, factor: 0 } // No need for exponential backoff
106107
);
107108

109+
const updateUserById = async (
110+
id: string,
111+
set: Partial<OmitAutoSetFields<CreateUser>>,
112+
jsonbMode?: 'replace' | 'merge'
113+
) => {
114+
const validPhoneNumber = conditional(
115+
'primaryPhone' in set &&
116+
typeof set.primaryPhone === 'string' &&
117+
getValidPhoneNumber(set.primaryPhone)
118+
);
119+
120+
return updateUserByIdQuery(
121+
id,
122+
{ ...set, ...conditional(validPhoneNumber && { primaryPhone: validPhoneNumber }) },
123+
jsonbMode
124+
);
125+
};
126+
108127
const insertUser = async (data: OmitAutoSetFields<CreateUser>, additionalRoleNames: string[]) => {
109128
const roleNames = deduplicate([...EnvSet.values.userDefaultRoleNames, ...additionalRoleNames]);
110129
const roles = await findRolesByRoleNames(roleNames);
111130

112131
assertThat(roles.length === roleNames.length, 'role.default_role_missing');
113132

133+
const validPhoneNumber = conditional(
134+
'primaryPhone' in data &&
135+
typeof data.primaryPhone === 'string' &&
136+
getValidPhoneNumber(data.primaryPhone)
137+
);
138+
114139
return pool.transaction(async (connection) => {
115140
const insertUserQuery = buildInsertIntoWithPool(connection)(Users, {
116141
returning: true,
117142
});
118143

119-
const user = await insertUserQuery(data);
144+
const user = await insertUserQuery({
145+
...data,
146+
...conditional(validPhoneNumber && { primaryPhone: validPhoneNumber }),
147+
});
120148

121149
if (roles.length > 0) {
122150
const { insertUsersRoles } = createUsersRolesQueries(connection);
@@ -279,5 +307,6 @@ export const createUserLibrary = (queries: Queries) => {
279307
findUserRoles,
280308
addUserMfaVerification,
281309
verifyUserPassword,
310+
updateUserById,
282311
};
283312
};

‎packages/core/src/routes-me/social.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ export default function socialRoutes<T extends AuthedMeRouter>(
2222
) {
2323
const {
2424
queries: {
25-
users: { findUserById, updateUserById, deleteUserIdentity, hasUserWithIdentity },
25+
users: { findUserById, deleteUserIdentity, hasUserWithIdentity },
2626
signInExperiences: { findDefaultSignInExperience },
2727
},
28+
libraries: {
29+
users: { updateUserById },
30+
},
2831
connectors: { getLogtoConnectors, getLogtoConnectorById },
2932
} = tenant;
3033

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ export default function userRoutes<T extends AuthedMeRouter>(
1717
) {
1818
const {
1919
queries: {
20-
users: { findUserById, updateUserById },
20+
users: { findUserById },
2121
},
2222
libraries: {
23-
users: { checkIdentifierCollision, verifyUserPassword },
23+
users: { checkIdentifierCollision, verifyUserPassword, updateUserById },
2424
verificationStatuses: { createVerificationStatus, checkVerificationStatus },
2525
},
2626
} = tenant;

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,6 @@ const mockedQueries = {
3434
hasUser: jest.fn(async () => mockHasUser()),
3535
hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
3636
hasUserWithPhone: jest.fn(async () => mockHasUserWithPhone()),
37-
updateUserById: jest.fn(
38-
async (_, data: Partial<CreateUser>): Promise<User> => ({
39-
...mockUser,
40-
...data,
41-
})
42-
),
4337
deleteUserById: jest.fn(),
4438
deleteUserIdentity: jest.fn(),
4539
},
@@ -66,8 +60,7 @@ const mockHasUserWithEmail = jest.fn(async () => false);
6660
const mockHasUserWithPhone = jest.fn(async () => false);
6761

6862
const { revokeInstanceByUserId } = mockedQueries.oidcModelInstances;
69-
const { hasUser, findUserById, updateUserById, deleteUserIdentity, deleteUserById } =
70-
mockedQueries.users;
63+
const { hasUser, findUserById, deleteUserIdentity, deleteUserById } = mockedQueries.users;
7164

7265
const { encryptUserPassword } = await mockEsmWithActual('#src/libraries/user.js', () => ({
7366
encryptUserPassword: jest.fn(() => ({
@@ -86,8 +79,16 @@ const usersLibraries = {
8679
})
8780
),
8881
verifyUserPassword,
82+
updateUserById: jest.fn(
83+
async (_, data: Partial<CreateUser>): Promise<User> => ({
84+
...mockUser,
85+
...data,
86+
})
87+
),
8988
} satisfies Partial<Libraries['users']>;
9089

90+
const { updateUserById } = usersLibraries;
91+
9192
const adminUserRoutes = await pickDefault(import('./basics.js'));
9293

9394
describe('adminUserRoutes', () => {

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

+8-10
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,17 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
2222
const [router, { queries, libraries }] = args;
2323
const {
2424
oidcModelInstances: { revokeInstanceByUserId },
25-
users: {
26-
deleteUserById,
27-
findUserById,
28-
hasUser,
29-
updateUserById,
30-
hasUserWithEmail,
31-
hasUserWithPhone,
32-
},
25+
users: { deleteUserById, findUserById, hasUser, hasUserWithEmail, hasUserWithPhone },
3326
userSsoIdentities,
3427
} = queries;
3528
const {
36-
users: { checkIdentifierCollision, generateUserId, insertUser, verifyUserPassword },
29+
users: {
30+
checkIdentifierCollision,
31+
generateUserId,
32+
insertUser,
33+
verifyUserPassword,
34+
updateUserById,
35+
},
3736
} = libraries;
3837

3938
router.get(
@@ -348,7 +347,6 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
348347

349348
ctx.body = pick(user, ...userInfoSelectFields);
350349

351-
// eslint-disable-next-line max-lines
352350
return next();
353351
}
354352
);

‎packages/core/src/routes/admin-user/mfa-verifications.test.ts

+22-19
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@ const mockedQueries = {
2222
hasUser: jest.fn(async () => mockHasUser()),
2323
hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
2424
hasUserWithPhone: jest.fn(async () => mockHasUserWithPhone()),
25-
updateUserById: jest.fn(
26-
async (_, data: Partial<CreateUser>): Promise<User> => ({
27-
...mockUser,
28-
...data,
29-
})
30-
),
3125
deleteUserById: jest.fn(),
3226
deleteUserIdentity: jest.fn(),
3327
},
@@ -37,7 +31,7 @@ const mockHasUser = jest.fn(async () => false);
3731
const mockHasUserWithEmail = jest.fn(async () => false);
3832
const mockHasUserWithPhone = jest.fn(async () => false);
3933

40-
const { findUserById, updateUserById } = mockedQueries.users;
34+
const { findUserById } = mockedQueries.users;
4135

4236
await mockEsmWithActual('../interaction/utils/totp-validation.js', () => ({
4337
generateTotpSecret: jest.fn().mockReturnValue('totp_secret'),
@@ -46,22 +40,31 @@ await mockEsmWithActual('../interaction/utils/backup-code-validation.js', () =>
4640
generateBackupCodes: jest.fn().mockReturnValue(['code']),
4741
}));
4842

49-
const usersLibraries = {
50-
generateUserId: jest.fn(async () => 'fooId'),
51-
insertUser: jest.fn(
52-
async (user: CreateUser): Promise<User> => ({
53-
...mockUser,
54-
...removeUndefinedKeys(user), // No undefined values will be returned from database
55-
})
56-
),
57-
} satisfies Partial<Libraries['users']>;
43+
const mockLibraries = {
44+
users: {
45+
generateUserId: jest.fn(async () => 'fooId'),
46+
insertUser: jest.fn(
47+
async (user: CreateUser): Promise<User> => ({
48+
...mockUser,
49+
...removeUndefinedKeys(user), // No undefined values will be returned from database
50+
})
51+
),
52+
updateUserById: jest.fn(
53+
async (_, data: Partial<CreateUser>): Promise<User> => ({
54+
...mockUser,
55+
...data,
56+
})
57+
),
58+
addUserMfaVerification: jest.fn(),
59+
},
60+
} satisfies Partial2<Libraries>;
61+
62+
const { updateUserById } = mockLibraries.users;
5863

5964
const adminUserRoutes = await pickDefault(import('./mfa-verifications.js'));
6065

6166
describe('adminUserRoutes', () => {
62-
const tenantContext = new MockTenant(undefined, mockedQueries, undefined, {
63-
users: usersLibraries,
64-
});
67+
const tenantContext = new MockTenant(undefined, mockedQueries, undefined, mockLibraries);
6568
const userRequest = createRequester({ authedRoutes: adminUserRoutes, tenantContext });
6669

6770
afterEach(() => {

‎packages/core/src/routes/admin-user/mfa-verifications.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ export default function adminUserMfaVerificationsRoutes<T extends ManagementApiR
2121
{
2222
queries,
2323
libraries: {
24-
users: { addUserMfaVerification },
24+
users: { addUserMfaVerification, updateUserById },
2525
},
2626
},
2727
] = args;
2828
const {
29-
users: { findUserById, updateUserById },
29+
users: { findUserById },
3030
} = queries;
3131

3232
router.get(

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ const mockedQueries = {
3232
}
3333
return mockUser;
3434
}),
35-
updateUserById: jest.fn(
36-
async (_, data: Partial<CreateUser>): Promise<User> => ({
37-
...mockUser,
38-
...data,
39-
})
40-
),
4135
hasUserWithIdentity: mockHasUserWithIdentity,
4236
deleteUserById: jest.fn(),
4337
deleteUserIdentity: jest.fn(),
@@ -52,6 +46,12 @@ const usersLibraries = {
5246
...user,
5347
})
5448
),
49+
updateUserById: jest.fn(
50+
async (_, data: Partial<CreateUser>): Promise<User> => ({
51+
...mockUser,
52+
...data,
53+
})
54+
),
5555
} satisfies Partial<Libraries['users']>;
5656

5757
const mockGetLogtoConnectors = jest.fn(async () => mockLogtoConnectorList);
@@ -73,7 +73,8 @@ const mockedConnectors = {
7373
},
7474
};
7575

76-
const { findUserById, updateUserById, deleteUserIdentity } = mockedQueries.users;
76+
const { findUserById, deleteUserIdentity } = mockedQueries.users;
77+
const { updateUserById } = usersLibraries;
7778

7879
const adminUserSocialRoutes = await pickDefault(import('./social.js'));
7980

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ export default function adminUserSocialRoutes<T extends ManagementApiRouter>(
2020
) {
2121
const {
2222
queries: {
23-
users: { findUserById, updateUserById, hasUserWithIdentity, deleteUserIdentity },
23+
users: { findUserById, hasUserWithIdentity, deleteUserIdentity },
24+
},
25+
libraries: {
26+
users: { updateUserById },
2427
},
2528
connectors: { getLogtoConnectorById },
2629
} = tenant;

0 commit comments

Comments
 (0)