Skip to content

Commit d5b131a

Browse files
simeng-liCopilot
simeng-li
and
Copilot
authored
refactor(core): has user with normalized phone number match (#7385)
* refactor(core): has user with normalized phone number match replace hasUserWithPhone using new hasHserWithNormalizedPhone * chore: fix typo Co-authored-by: Copilot <[email protected]> * chore: update wording Co-authored-by: Copilot <[email protected]> * fix(core): fix sql condition fix sql condition * chore(test): add additional integration tests add additional integration tests * fix(test): fix integration test fix integration test * chore(core): update jsdoc comments update jsdoc comments --------- Co-authored-by: Copilot <[email protected]>
1 parent cd9cc25 commit d5b131a

23 files changed

+328
-110
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export const createUserLibrary = (queries: Queries) => {
2929
hasUser,
3030
hasUserWithEmail,
3131
hasUserWithId,
32-
hasUserWithPhone,
32+
hasUserWithNormalizedPhone,
3333
hasUserWithIdentity,
3434
findUsersByIds,
3535
updateUserById,
@@ -103,7 +103,7 @@ export const createUserLibrary = (queries: Queries) => {
103103
throw new RequestError({ code: 'user.email_already_in_use', status: 422 });
104104
}
105105

106-
if (primaryPhone && (await hasUserWithPhone(primaryPhone, excludeUserId))) {
106+
if (primaryPhone && (await hasUserWithNormalizedPhone(primaryPhone, excludeUserId))) {
107107
throw new RequestError({ code: 'user.phone_already_in_use', status: 422 });
108108
}
109109

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ export const createUserQueries = (pool: CommonQueryMethods) => {
7979
where lower(${fields.primaryEmail})=lower(${email})
8080
`);
8181

82+
/**
83+
* Find user by phone with exact match.
84+
*/
8285
const findUserByPhone = async (phone: string) =>
8386
pool.maybeOne<User>(sql`
8487
select ${sql.join(Object.values(fields), sql`,`)}
@@ -195,7 +198,7 @@ export const createUserQueries = (pool: CommonQueryMethods) => {
195198
`);
196199

197200
/**
198-
* Find user by phone with exact match.
201+
* Checks if a user exists in the database with an exact match on their phone number.
199202
*/
200203
const hasUserWithPhone = async (phone: string, excludeUserId?: string) =>
201204
pool.exists(sql`
@@ -205,6 +208,51 @@ export const createUserQueries = (pool: CommonQueryMethods) => {
205208
${conditionalSql(excludeUserId, (id) => sql`and ${fields.id}<>${id}`)}
206209
`);
207210

211+
/**
212+
* Checks if a user exists in the database with a phone number that matches the provided
213+
* number in either normalized format or with a leading '0'.
214+
*
215+
* @remarks
216+
* This function normalizes the input phone number to account for variations in formatting.
217+
* It checks for the existence of a user with the same phone number in two formats:
218+
* - Standard international format (e.g., 61412345678)
219+
* - International format with a leading '0' before the local number (e.g., 610412345678)
220+
*
221+
* If the provided phone number is not a valid international format, it falls back to checking
222+
* for an exact match using the `hasUserWithPhone` function.
223+
*
224+
* @param phone - The phone number to check for user existence.
225+
* @param excludeUserId - (Optional) If provided, excludes the user with this ID from the search,
226+
* allowing for updates without false positives.
227+
*
228+
* @example
229+
* // Database contains: 610412345678
230+
* hasUserWithNormalizedPhone(61412345678); // returns: true
231+
*
232+
* @example
233+
* // Database contains: 61412345678
234+
* hasUserWithNormalizedPhone(610412345678); // returns: true
235+
*/
236+
const hasUserWithNormalizedPhone = async (phone: string, excludeUserId?: string) => {
237+
const phoneNumberParser = new PhoneNumberParser(phone);
238+
239+
const { internationalNumber, internationalNumberWithLeadingZero, isValid } = phoneNumberParser;
240+
241+
// If the phone number is not a valid international phone number, find user with exact match.
242+
if (!isValid || !internationalNumber || !internationalNumberWithLeadingZero) {
243+
return hasUserWithPhone(phone, excludeUserId);
244+
}
245+
246+
// Check if the user exists with any of the two formats.
247+
return pool.exists(sql`
248+
select ${fields.primaryPhone}
249+
from ${table}
250+
where (${fields.primaryPhone}=${internationalNumber}
251+
or ${fields.primaryPhone}=${internationalNumberWithLeadingZero})
252+
${conditionalSql(excludeUserId, (id) => sql`and ${fields.id}<>${id}`)}
253+
`);
254+
};
255+
208256
const hasUserWithIdentity = async (target: string, userId: string, excludeUserId?: string) =>
209257
pool.exists(
210258
sql`
@@ -341,6 +389,7 @@ export const createUserQueries = (pool: CommonQueryMethods) => {
341389
hasUserWithId,
342390
hasUserWithEmail,
343391
hasUserWithPhone,
392+
hasUserWithNormalizedPhone,
344393
hasUserWithIdentity,
345394
countUsers,
346395
findUsers,

‎packages/core/src/routes/account/email-and-phone.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export default function emailAndPhoneRoutes<T extends UserRouter>(...args: Route
3131
email: z.string().regex(emailRegEx),
3232
newIdentifierVerificationRecordId: z.string(),
3333
}),
34-
status: [204, 400, 401],
34+
status: [204, 400, 401, 422],
3535
}),
3636
async (ctx, next) => {
3737
const { id: userId, scopes, identityVerified } = ctx.auth;
@@ -123,7 +123,7 @@ export default function emailAndPhoneRoutes<T extends UserRouter>(...args: Route
123123
phone: z.string().regex(phoneRegEx),
124124
newIdentifierVerificationRecordId: z.string(),
125125
}),
126-
status: [204, 400, 401],
126+
status: [204, 400, 401, 422],
127127
}),
128128
async (ctx, next) => {
129129
const { id: userId, scopes, identityVerified } = ctx.auth;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const mockedQueries = {
3434
findUserById: jest.fn(async (id: string) => mockUser),
3535
hasUser: jest.fn(async () => mockHasUser()),
3636
hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
37-
hasUserWithPhone: jest.fn(async () => mockHasUserWithPhone()),
37+
hasUserWithNormalizedPhone: jest.fn(async () => mockHasUserWithPhone()),
3838
updateUserById: jest.fn(
3939
async (_, data: Partial<CreateUser>): Promise<User> => ({
4040
...mockUser,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
3030
hasUser,
3131
updateUserById,
3232
hasUserWithEmail,
33-
hasUserWithPhone,
33+
hasUserWithNormalizedPhone,
3434
},
3535
} = queries;
3636
const {
@@ -191,7 +191,7 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
191191
})
192192
);
193193
assertThat(
194-
!primaryPhone || !(await hasUserWithPhone(primaryPhone)),
194+
!primaryPhone || !(await hasUserWithNormalizedPhone(primaryPhone)),
195195
new RequestError({ code: 'user.phone_already_in_use', status: 422 })
196196
);
197197

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,15 @@ const { mockEsmWithActual } = createMockUtils(jest);
2020
const mockedQueries = {
2121
users: {
2222
findUserById: jest.fn(async (id: string) => mockUser),
23-
hasUser: jest.fn(async () => mockHasUser()),
24-
hasUserWithEmail: jest.fn(async () => mockHasUserWithEmail()),
25-
hasUserWithPhone: jest.fn(async () => mockHasUserWithPhone()),
2623
updateUserById: jest.fn(
2724
async (_, data: Partial<CreateUser>): Promise<User> => ({
2825
...mockUser,
2926
...data,
3027
})
3128
),
32-
deleteUserById: jest.fn(),
33-
deleteUserIdentity: jest.fn(),
3429
},
3530
} satisfies Partial2<Queries>;
3631

37-
const mockHasUser = jest.fn(async () => false);
38-
const mockHasUserWithEmail = jest.fn(async () => false);
39-
const mockHasUserWithPhone = jest.fn(async () => false);
40-
4132
const { findUserById, updateUserById } = mockedQueries.users;
4233

4334
await mockEsmWithActual('../interaction/utils/totp-validation.js', () => ({

‎packages/core/src/routes/experience/classes/experience-interaction.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const mockEmail = '[email protected]';
3333
const userQueries = {
3434
hasActiveUsers: jest.fn().mockResolvedValue(false),
3535
hasUserWithEmail: jest.fn().mockResolvedValue(false),
36-
hasUserWithPhone: jest.fn().mockResolvedValue(false),
36+
hasUserWithNormalizedPhone: jest.fn().mockResolvedValue(false),
3737
hasUserWithIdentity: jest.fn().mockResolvedValue(false),
3838
};
3939
const userLibraries = {

‎packages/core/src/routes/experience/classes/libraries/profile-validator.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ export class ProfileValidator {
1010
constructor(private readonly queries: Queries) {}
1111

1212
public async guardProfileUniquenessAcrossUsers(profile: InteractionProfile = {}) {
13-
const { hasUser, hasUserWithEmail, hasUserWithPhone, hasUserWithIdentity } = this.queries.users;
13+
const { hasUser, hasUserWithEmail, hasUserWithNormalizedPhone, hasUserWithIdentity } =
14+
this.queries.users;
1415
const { userSsoIdentities } = this.queries;
1516

1617
const { username, primaryEmail, primaryPhone, socialIdentity, enterpriseSsoIdentity } = profile;
@@ -37,7 +38,7 @@ export class ProfileValidator {
3738

3839
if (primaryPhone) {
3940
assertThat(
40-
!(await hasUserWithPhone(primaryPhone)),
41+
!(await hasUserWithNormalizedPhone(primaryPhone)),
4142
new RequestError({
4243
code: 'user.phone_already_in_use',
4344
status: 422,

‎packages/core/src/routes/experience/classes/verifications/social-verification.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,16 @@ export class SocialVerification implements IdentifierVerificationRecord<Verifica
258258

259259
if (isNewUser) {
260260
const {
261-
users: { hasUserWithEmail, hasUserWithPhone },
261+
users: { hasUserWithEmail, hasUserWithNormalizedPhone },
262262
} = this.queries;
263263

264264
return {
265265
// Sync the email only if the email is not used by other users
266266
...conditional(primaryEmail && !(await hasUserWithEmail(primaryEmail)) && { primaryEmail }),
267267
// Sync the phone only if the phone is not used by other users
268-
...conditional(primaryPhone && !(await hasUserWithPhone(primaryPhone)) && { primaryPhone }),
268+
...conditional(
269+
primaryPhone && !(await hasUserWithNormalizedPhone(primaryPhone)) && { primaryPhone }
270+
),
269271
...conditional(name && { name }),
270272
...conditional(avatar && { avatar }),
271273
};

‎packages/core/src/routes/interaction/actions/helpers.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const getSocialSyncProfile = async (
5353

5454
/* Parse the user profile from the new linked Social identity */
5555
const parseNewSocialProfile = async (
56-
{ users: { hasUserWithEmail, hasUserWithPhone } }: Queries,
56+
{ users: { hasUserWithEmail, hasUserWithNormalizedPhone } }: Queries,
5757
{ getLogtoConnectorById }: ConnectorLibrary,
5858
socialIdentifier: SocialIdentifier,
5959
user?: User
@@ -78,7 +78,9 @@ const parseNewSocialProfile = async (
7878
// Sync the email only if the email is not used by other users
7979
...conditional(email && !(await hasUserWithEmail(email)) && { primaryEmail: email }),
8080
// Sync the phone only if the phone is not used by other users
81-
...conditional(phone && !(await hasUserWithPhone(phone)) && { primaryPhone: phone }),
81+
...conditional(
82+
phone && !(await hasUserWithNormalizedPhone(phone)) && { primaryPhone: phone }
83+
),
8284
};
8385
}
8486

0 commit comments

Comments
 (0)