Skip to content

Commit 13d885a

Browse files
committed
feat: add phone number validation to user APIs
1 parent 1b2359b commit 13d885a

30 files changed

+243
-134
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 },
@@ -106,18 +107,45 @@ export const createUserLibrary = (queries: Queries) => {
106107
{ retries, factor: 0 } // No need for exponential backoff
107108
);
108109

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

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

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

120-
const user = await insertUserQuery(data);
145+
const user = await insertUserQuery({
146+
...data,
147+
...conditional(validPhoneNumber && { primaryPhone: validPhoneNumber }),
148+
});
121149

122150
if (roles.length > 0) {
123151
const { insertUsersRoles } = createUsersRolesQueries(connection);
@@ -289,5 +317,6 @@ export const createUserLibrary = (queries: Queries) => {
289317
addUserMfaVerification,
290318
verifyUserPassword,
291319
signOutUser,
320+
updateUserById,
292321
};
293322
};

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

+10-8
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ const mockedQueries = {
3333
hasUser: jest.fn(async () => mockHasUser()),
3434
hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
3535
hasUserWithPhone: jest.fn(async () => mockHasUserWithPhone()),
36-
updateUserById: jest.fn(
37-
async (_, data: Partial<CreateUser>): Promise<User> => ({
38-
...mockUser,
39-
...data,
40-
})
41-
),
4236
deleteUserById: jest.fn(),
4337
deleteUserIdentity: jest.fn(),
4438
},
@@ -64,8 +58,8 @@ const mockHasUser = jest.fn(async () => false);
6458
const mockHasUserWithEmail = jest.fn(async () => false);
6559
const mockHasUserWithPhone = jest.fn(async () => false);
6660

67-
const { hasUser, findUserById, updateUserById, deleteUserIdentity, deleteUserById } =
68-
mockedQueries.users;
61+
const { revokeInstanceByUserId } = mockedQueries.oidcModelInstances;
62+
const { hasUser, findUserById, deleteUserIdentity, deleteUserById } = mockedQueries.users;
6963

7064
const { encryptUserPassword } = await mockEsmWithActual('#src/libraries/user.js', () => ({
7165
encryptUserPassword: jest.fn(() => ({
@@ -86,8 +80,16 @@ const usersLibraries = {
8680
),
8781
verifyUserPassword,
8882
signOutUser,
83+
updateUserById: jest.fn(
84+
async (_, data: Partial<CreateUser>): Promise<User> => ({
85+
...mockUser,
86+
...data,
87+
})
88+
),
8989
} satisfies Partial<Libraries['users']>;
9090

91+
const { updateUserById } = usersLibraries;
92+
9193
const adminUserRoutes = await pickDefault(import('./basics.js'));
9294

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

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

+3-8
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,8 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
2222
) {
2323
const [router, { queries, libraries }] = args;
2424
const {
25-
users: {
26-
deleteUserById,
27-
findUserById,
28-
hasUser,
29-
updateUserById,
30-
hasUserWithEmail,
31-
hasUserWithPhone,
32-
},
25+
oidcModelInstances: { revokeInstanceByUserId },
26+
users: { deleteUserById, findUserById, hasUser, hasUserWithEmail, hasUserWithPhone },
3327
userSsoIdentities,
3428
} = queries;
3529
const {
@@ -39,6 +33,7 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
3933
insertUser,
4034
verifyUserPassword,
4135
signOutUser,
36+
updateUserById,
4237
},
4338
} = libraries;
4439

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;

packages/core/src/routes/interaction/actions/submit-interaction.mfa.test.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,19 @@ const userQueries = {
4444
identities: { google: { userId: 'googleId', details: {} } },
4545
mfaVerifications: [],
4646
}),
47-
updateUserById: jest.fn(),
4847
hasActiveUsers: jest.fn().mockResolvedValue(true),
4948
hasUserWithEmail: jest.fn().mockResolvedValue(false),
5049
hasUserWithPhone: jest.fn().mockResolvedValue(false),
5150
};
5251

53-
const { hasActiveUsers, updateUserById } = userQueries;
52+
const { hasActiveUsers } = userQueries;
5453

55-
const userLibraries = { generateUserId: jest.fn().mockResolvedValue('uid'), insertUser: jest.fn() };
56-
const { generateUserId, insertUser } = userLibraries;
54+
const userLibraries = {
55+
generateUserId: jest.fn().mockResolvedValue('uid'),
56+
updateUserById: jest.fn(),
57+
insertUser: jest.fn(),
58+
};
59+
const { generateUserId, insertUser, updateUserById } = userLibraries;
5760

5861
const submitInteraction = await pickDefault(import('./submit-interaction.js'));
5962
const now = Date.now();

0 commit comments

Comments
 (0)