From 2d09af35441732602abf59b86cf87a4ac2820681 Mon Sep 17 00:00:00 2001 From: networkinss Date: Sun, 1 Mar 2026 07:19:23 +0100 Subject: [PATCH 1/2] fix(core-users): distinguish validation error reasons for email and username Default validators returned false for both invalid format and duplicate entry. Callers threw a single generic error for both, causing the Admin UI to show a misleading message even when the real problem was a duplicate. defaultValidateEmail and defaultValidateUsername now return a typed ValidationResult object instead of a boolean. The four call sites in configureUsersModule propagate the specific reason as the Error cause: EMAIL_FORMAT_INVALID, EMAIL_ALREADY_EXISTS, USERNAME_TOO_SHORT, USERNAME_ALREADY_EXISTS. errors.ts gains EmailFormatInvalidError and UsernameTooShortError, and the previously merged messages on EmailAlreadyExistsError and UsernameAlreadyExistsError are made specific. --- packages/api/src/errors.ts | 14 +++++++-- .../src/module/configureUsersModule.ts | 20 +++++++----- packages/core-users/src/users-settings.ts | 31 +++++++++++-------- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/packages/api/src/errors.ts b/packages/api/src/errors.ts index a445630fe..1ff32c0f5 100644 --- a/packages/api/src/errors.ts +++ b/packages/api/src/errors.ts @@ -241,14 +241,24 @@ export const CyclicProductBundlingNotSupportedError = createError( 'Cyclic bundling detected, make sure bundled product is not the same as the bundle product itself', ); +export const EmailFormatInvalidError = createError( + 'EmailFormatInvalid', + 'Email address format is invalid', +); + export const EmailAlreadyExistsError = createError( 'EmailAlreadyExists', - 'Email already exists or is invalid', + 'Email address already exists', +); + +export const UsernameTooShortError = createError( + 'UsernameTooShort', + 'Username must be at least 3 characters', ); export const UsernameAlreadyExistsError = createError( 'UsernameAlreadyExists', - 'Username already exists or is invalid', + 'Username already exists', ); export const UsernameOrEmailRequiredError = createError( diff --git a/packages/core-users/src/module/configureUsersModule.ts b/packages/core-users/src/module/configureUsersModule.ts index 3438c5847..644c9f38f 100644 --- a/packages/core-users/src/module/configureUsersModule.ts +++ b/packages/core-users/src/module/configureUsersModule.ts @@ -324,8 +324,9 @@ export const configureUsersModule = async (moduleInput: ModuleInput = {}; if (email) { - if (!(await userSettings.validateEmail(email))) { - throw new Error(`E-Mail address ${email} is invalid`, { cause: 'EMAIL_INVALID' }); + const emailResult = await userSettings.validateEmail(email); + if (!emailResult.valid) { + throw new Error(`E-Mail address ${email} is invalid`, { cause: emailResult.reason }); } } @@ -353,8 +354,9 @@ export const configureUsersModule = async (moduleInput: ModuleInput { - if (!(await userSettings.validateEmail(address))) { - throw new Error(`E-Mail address ${address} is invalid`, { cause: 'EMAIL_INVALID' }); + const emailResult = await userSettings.validateEmail(address); + if (!emailResult.valid) { + throw new Error(`E-Mail address ${address} is invalid`, { cause: emailResult.reason }); } await this.updateUser( { _id: userId, 'emails.address': { $not: insensitiveTrimmedRegexOperator(address) } }, @@ -686,8 +689,9 @@ export const configureUsersModule = async (moduleInput: ModuleInput Date; - validateEmail: (email: string) => Promise; - validateUsername: (username: string) => Promise; + validateEmail: (email: string) => Promise; + validateUsername: (username: string) => Promise; validateNewUser: (user: UserRegistrationData) => Promise; validatePassword: (password: string) => Promise; configureSettings: (options: UserSettingsOptions, db: mongodb.Db) => void; } -export type UserSettingsOptions = Omit, 'configureSettings'>; +export type UserSettingsOptions = Omit, 'configureSettings' | 'validateEmail' | 'validateUsername'> & { + validateEmail?: (email: string) => Promise; + validateUsername?: (username: string) => Promise; +}; const defaultAutoMessagingAfterUserCreation = true; const defaultMergeUserCartsOnLogin = true; @@ -56,8 +61,8 @@ export const userSettings: UserSettings = { mergeUserCartsOnLogin: defaultMergeUserCartsOnLogin, earliestValidTokenDate: defaultEarliestValidTokenDate, validateNewUser: defaultValidateNewUser, - validateEmail: () => Promise.resolve(true), - validateUsername: () => Promise.resolve(true), + validateEmail: () => Promise.resolve({ valid: true } as ValidationResult), + validateUsername: () => Promise.resolve({ valid: true } as ValidationResult), validatePassword: () => Promise.resolve(true), configureSettings: ( @@ -72,21 +77,21 @@ export const userSettings: UserSettings = { }, db: mongodb.Db, ) => { - const defaultValidateEmail = async (rawEmail: string) => { - if (!rawEmail?.includes?.('@')) return false; + const defaultValidateEmail = async (rawEmail: string): Promise => { + if (!rawEmail?.includes?.('@')) return { valid: false, reason: 'EMAIL_FORMAT_INVALID' }; const emailAlreadyExists = await db .collection('users') .countDocuments({ 'emails.address': insensitiveTrimmedRegexOperator(rawEmail) }, { limit: 1 }); - if (emailAlreadyExists) return false; - return true; + if (emailAlreadyExists) return { valid: false, reason: 'EMAIL_ALREADY_EXISTS' }; + return { valid: true }; }; - const defaultValidateUsername = async (rawUsername: string) => { - if (rawUsername?.length < 3) return false; + const defaultValidateUsername = async (rawUsername: string): Promise => { + if (rawUsername?.length < 3) return { valid: false, reason: 'USERNAME_TOO_SHORT' }; const usernameAlreadyExists = await db .collection('users') .countDocuments({ username: insensitiveTrimmedRegexOperator(rawUsername) }, { limit: 1 }); - if (usernameAlreadyExists) return false; - return true; + if (usernameAlreadyExists) return { valid: false, reason: 'USERNAME_ALREADY_EXISTS' }; + return { valid: true }; }; userSettings.mergeUserCartsOnLogin = mergeUserCartsOnLogin ?? defaultMergeUserCartsOnLogin; From d39813a40733d60890de5e606e97dca7494a3548 Mon Sep 17 00:00:00 2001 From: networkinss Date: Sun, 1 Mar 2026 07:59:29 +0100 Subject: [PATCH 2/2] fix(examples/oidc): update validateEmail to return ValidationResult Return { valid: true } instead of a bare boolean to match the ValidationResult type introduced by fix/distinguish-validation-error-reasons. --- examples/oidc/boot.ts | 2 +- .../core-users/src/users-settings.test.ts | 83 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 packages/core-users/src/users-settings.test.ts diff --git a/examples/oidc/boot.ts b/examples/oidc/boot.ts index 51d7f3edf..f820181d1 100644 --- a/examples/oidc/boot.ts +++ b/examples/oidc/boot.ts @@ -50,7 +50,7 @@ try { * return false; * } */ - validateEmail: async () => true, + validateEmail: async () => ({ valid: true as const }), }, }, adminUiConfig: { diff --git a/packages/core-users/src/users-settings.test.ts b/packages/core-users/src/users-settings.test.ts new file mode 100644 index 000000000..69ef215b1 --- /dev/null +++ b/packages/core-users/src/users-settings.test.ts @@ -0,0 +1,83 @@ +import { describe, it, before } from 'node:test'; +import assert from 'node:assert'; +import { userSettings } from './users-settings.ts'; + +const makeDb = (count: number) => + ({ + collection: () => ({ + countDocuments: async () => count, + }), + }) as any; + +describe('defaultValidateEmail', () => { + describe('format validation', () => { + before(() => { + userSettings.configureSettings({}, makeDb(0)); + }); + + it('returns EMAIL_FORMAT_INVALID when email has no @', async () => { + const result = await userSettings.validateEmail('notanemail'); + assert.deepStrictEqual(result, { valid: false, reason: 'EMAIL_FORMAT_INVALID' }); + }); + + it('returns EMAIL_FORMAT_INVALID for empty string', async () => { + const result = await userSettings.validateEmail(''); + assert.deepStrictEqual(result, { valid: false, reason: 'EMAIL_FORMAT_INVALID' }); + }); + + it('returns valid: true for a correctly formatted new email', async () => { + const result = await userSettings.validateEmail('new@example.com'); + assert.deepStrictEqual(result, { valid: true }); + }); + }); + + describe('duplicate detection', () => { + before(() => { + userSettings.configureSettings({}, makeDb(1)); + }); + + it('returns EMAIL_ALREADY_EXISTS when email is taken', async () => { + const result = await userSettings.validateEmail('existing@example.com'); + assert.deepStrictEqual(result, { valid: false, reason: 'EMAIL_ALREADY_EXISTS' }); + }); + }); +}); + +describe('defaultValidateUsername', () => { + describe('length validation', () => { + before(() => { + userSettings.configureSettings({}, makeDb(0)); + }); + + it('returns USERNAME_TOO_SHORT for a 2-character username', async () => { + const result = await userSettings.validateUsername('ab'); + assert.deepStrictEqual(result, { valid: false, reason: 'USERNAME_TOO_SHORT' }); + }); + + it('returns USERNAME_TOO_SHORT for empty string', async () => { + const result = await userSettings.validateUsername(''); + assert.deepStrictEqual(result, { valid: false, reason: 'USERNAME_TOO_SHORT' }); + }); + + it('returns valid: true for a username of exactly 3 characters', async () => { + const result = await userSettings.validateUsername('abc'); + assert.deepStrictEqual(result, { valid: true }); + }); + + it('returns valid: true for a longer valid username', async () => { + const result = await userSettings.validateUsername('newuser'); + assert.deepStrictEqual(result, { valid: true }); + }); + }); + + describe('duplicate detection', () => { + before(() => { + userSettings.configureSettings({}, makeDb(1)); + }); + + it('returns USERNAME_ALREADY_EXISTS when username is taken', async () => { + const result = await userSettings.validateUsername('existinguser'); + assert.deepStrictEqual(result, { valid: false, reason: 'USERNAME_ALREADY_EXISTS' }); + }); + }); +});