From 098c996b4dc129092290a0fb1d68bd00ed08d224 Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Thu, 20 Feb 2025 23:01:42 +0530 Subject: [PATCH 01/10] added UI validation for existing email during user registration --- apps/meteor/app/api/server/v1/users.ts | 5 ++++- packages/web-ui-registration/src/RegisterForm.tsx | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index 7858a236f8685..a35166cdc4d39 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -36,6 +36,7 @@ import { sendConfirmationEmail } from '../../../../server/methods/sendConfirmati import { getUserForCheck, emailCheck } from '../../../2fa/server/code'; import { resetTOTP } from '../../../2fa/server/functions/resetTOTP'; import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import {checkEmailAvailability} from '../../../lib/server/functions/checkEmailAvailability'; import { checkUsernameAvailability, checkUsernameAvailabilityWithValidation, @@ -649,7 +650,9 @@ API.v1.addRoute( if (!(await checkUsernameAvailability(this.bodyParams.username))) { return API.v1.failure('Username is already in use'); } - + if(!(await checkEmailAvailability(this.bodyParams.email))){ + return API.v1.failure('Email is already in use'); + } if (this.bodyParams.customFields) { try { await validateCustomFields(this.bodyParams.customFields); diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index dcf32c72d4180..799cbe27b53ae 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -93,7 +93,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo if (error.errorType === 'error-user-already-exists') { setError('username', { type: 'user-already-exists', message: t('registration.component.form.usernameAlreadyExists') }); } - if (/Email already exists/.test(error.error)) { + if (/Email is already in use/.test(error.error)) { setError('email', { type: 'email-already-exists', message: t('registration.component.form.emailAlreadyExists') }); } if (/Username is already in use/.test(error.error)) { From 2401105c850adaf7224d2d86b75cab1c1be98148 Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Thu, 20 Feb 2025 23:24:44 +0530 Subject: [PATCH 02/10] Add changeset for email validation fix --- .changeset/open-toes-vanish.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/open-toes-vanish.md diff --git a/.changeset/open-toes-vanish.md b/.changeset/open-toes-vanish.md new file mode 100644 index 0000000000000..fc98067aa24f5 --- /dev/null +++ b/.changeset/open-toes-vanish.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/meteor': patch +'@rocket.chat/web-ui-registration': patch +--- + +Fixes email validation in the registration form to prevent users from registering with an already existing email address. From 0322f63caffcdad492b94363a4198205f509f56c Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Mon, 24 Feb 2025 21:59:08 +0530 Subject: [PATCH 03/10] fixed lint errors --- apps/meteor/app/api/server/v1/users.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index a35166cdc4d39..ef54ceee85aad 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -36,7 +36,7 @@ import { sendConfirmationEmail } from '../../../../server/methods/sendConfirmati import { getUserForCheck, emailCheck } from '../../../2fa/server/code'; import { resetTOTP } from '../../../2fa/server/functions/resetTOTP'; import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; -import {checkEmailAvailability} from '../../../lib/server/functions/checkEmailAvailability'; +import { checkEmailAvailability } from '../../../lib/server/functions/checkEmailAvailability'; import { checkUsernameAvailability, checkUsernameAvailabilityWithValidation, @@ -650,7 +650,7 @@ API.v1.addRoute( if (!(await checkUsernameAvailability(this.bodyParams.username))) { return API.v1.failure('Username is already in use'); } - if(!(await checkEmailAvailability(this.bodyParams.email))){ + if (!(await checkEmailAvailability(this.bodyParams.email))) { return API.v1.failure('Email is already in use'); } if (this.bodyParams.customFields) { From 1766d36c9cfc2cb391ee52b6fd4260b9f26b7686 Mon Sep 17 00:00:00 2001 From: Anshul Date: Wed, 26 Feb 2025 00:17:41 +0530 Subject: [PATCH 04/10] Update users.ts --- apps/meteor/app/api/server/v1/users.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index ef54ceee85aad..4a922eaac9d5d 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -651,7 +651,7 @@ API.v1.addRoute( return API.v1.failure('Username is already in use'); } if (!(await checkEmailAvailability(this.bodyParams.email))) { - return API.v1.failure('Email is already in use'); + return API.v1.failure('Email already exists'); } if (this.bodyParams.customFields) { try { From 4d92b922b24867e2ac054f5ed3bddca0eb5106e7 Mon Sep 17 00:00:00 2001 From: Anshul Date: Wed, 26 Feb 2025 00:19:49 +0530 Subject: [PATCH 05/10] Update RegisterForm.tsx --- packages/web-ui-registration/src/RegisterForm.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index 799cbe27b53ae..7263c98597c32 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -96,7 +96,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo if (/Email is already in use/.test(error.error)) { setError('email', { type: 'email-already-exists', message: t('registration.component.form.emailAlreadyExists') }); } - if (/Username is already in use/.test(error.error)) { + if (/Email already exists/.test(error.error)) { setError('username', { type: 'username-already-exists', message: t('registration.component.form.userAlreadyExist') }); } if (/The username provided is not valid/.test(error.error)) { From 29b97c3af37d8c3c3d02c287a1dc49a3d28144a2 Mon Sep 17 00:00:00 2001 From: Anshul Date: Wed, 26 Feb 2025 00:28:30 +0530 Subject: [PATCH 06/10] Update RegisterForm.tsx --- packages/web-ui-registration/src/RegisterForm.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index 7263c98597c32..dcf32c72d4180 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -93,10 +93,10 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo if (error.errorType === 'error-user-already-exists') { setError('username', { type: 'user-already-exists', message: t('registration.component.form.usernameAlreadyExists') }); } - if (/Email is already in use/.test(error.error)) { + if (/Email already exists/.test(error.error)) { setError('email', { type: 'email-already-exists', message: t('registration.component.form.emailAlreadyExists') }); } - if (/Email already exists/.test(error.error)) { + if (/Username is already in use/.test(error.error)) { setError('username', { type: 'username-already-exists', message: t('registration.component.form.userAlreadyExist') }); } if (/The username provided is not valid/.test(error.error)) { From 06b34a853bbb64f27d92f8368025d90deebee940 Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Thu, 13 Mar 2025 20:49:09 +0530 Subject: [PATCH 07/10] test: added e2e test for duplicate email validation in user registration --- apps/meteor/tests/e2e/register.spec.ts | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/apps/meteor/tests/e2e/register.spec.ts b/apps/meteor/tests/e2e/register.spec.ts index 0e371aa25d972..805a9c097f928 100644 --- a/apps/meteor/tests/e2e/register.spec.ts +++ b/apps/meteor/tests/e2e/register.spec.ts @@ -134,6 +134,39 @@ test.describe.parallel('register', () => { expect(results.violations).toEqual([]); }); + + test('should not allow registration with an already registered email', async ({ page }) => { + const email = faker.internet.email(); + await test.step('Register a new user successfully', async () => { + await page.goto('/home'); + await poRegistration.goToRegister.click(); + await poRegistration.inputName.fill(faker.person.firstName()); + await poRegistration.inputEmail.fill(email); + await poRegistration.username.fill(faker.internet.userName()); + await poRegistration.inputPassword.fill('any_password'); + await poRegistration.inputPasswordConfirm.fill('any_password'); + + await poRegistration.btnRegister.click(); + + await page.locator('button[title="User menu"]').click(); + await page.locator('div.rcx-option__content', { hasText: 'Logout' }).click(); + }); + + await test.step('Attempt registration with the same email', async () => { + await poRegistration.goToRegister.click(); + await poRegistration.inputName.fill(faker.person.firstName()); + await poRegistration.inputEmail.fill(email); + await poRegistration.username.fill(faker.internet.userName()); + await poRegistration.inputPassword.fill('any_password'); + await poRegistration.inputPasswordConfirm.fill('any_password'); + + await poRegistration.btnRegister.click(); + + const errorMessageLocator = page.locator('[aria-live="assertive"].rcx-field__error'); + await expect(errorMessageLocator).toBeVisible(); + await expect(errorMessageLocator).toHaveText('Email already exists'); + }); + }); }); test.describe('Registration for secret password', async () => { From 5748771d7de9b5a64fd064a5df34815fecdae623 Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Fri, 14 Mar 2025 02:17:38 +0530 Subject: [PATCH 08/10] test:added api for signup, specific locator and separate test block --- apps/meteor/tests/e2e/register.spec.ts | 36 +++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/apps/meteor/tests/e2e/register.spec.ts b/apps/meteor/tests/e2e/register.spec.ts index 805a9c097f928..d0b62d1b6f0dd 100644 --- a/apps/meteor/tests/e2e/register.spec.ts +++ b/apps/meteor/tests/e2e/register.spec.ts @@ -1,5 +1,5 @@ import { faker } from '@faker-js/faker'; - +import {request} from '../data/api-data'; import { Utils, Registration } from './page-objects'; import { test, expect } from './utils/test'; @@ -134,25 +134,25 @@ test.describe.parallel('register', () => { expect(results.violations).toEqual([]); }); + }); + test.describe('Registration form validation', async () => { + test.beforeEach(async ({ page }) => { + poRegistration = new Registration(page); + poUtils = new Utils(page); + }); test('should not allow registration with an already registered email', async ({ page }) => { const email = faker.internet.email(); - await test.step('Register a new user successfully', async () => { - await page.goto('/home'); - await poRegistration.goToRegister.click(); - await poRegistration.inputName.fill(faker.person.firstName()); - await poRegistration.inputEmail.fill(email); - await poRegistration.username.fill(faker.internet.userName()); - await poRegistration.inputPassword.fill('any_password'); - await poRegistration.inputPasswordConfirm.fill('any_password'); - - await poRegistration.btnRegister.click(); - - await page.locator('button[title="User menu"]').click(); - await page.locator('div.rcx-option__content', { hasText: 'Logout' }).click(); + await request.post('/api/v1/users.register') + .set('Content-Type', 'application/json') + .send({ + name: faker.person.firstName(), + email: email, + username: faker.internet.userName(), + pass: "any_password", }); - await test.step('Attempt registration with the same email', async () => { + await page.goto('/home'); await poRegistration.goToRegister.click(); await poRegistration.inputName.fill(faker.person.firstName()); await poRegistration.inputEmail.fill(email); @@ -162,12 +162,12 @@ test.describe.parallel('register', () => { await poRegistration.btnRegister.click(); - const errorMessageLocator = page.locator('[aria-live="assertive"].rcx-field__error'); + const errorMessageLocator = page.locator('span.rcx-field__error').filter({ hasText: 'Email already exists' }); await expect(errorMessageLocator).toBeVisible(); await expect(errorMessageLocator).toHaveText('Email already exists'); }); }); - }); + }) test.describe('Registration for secret password', async () => { test.beforeEach(async ({ api, page }) => { @@ -226,4 +226,4 @@ test.describe.parallel('register', () => { await expect(page.locator('role=heading[level=2][name="The URL provided is invalid."]')).toBeVisible(); }); }); -}); +}); \ No newline at end of file From e76f770985c853e0d8f9737f6f05299c267173cb Mon Sep 17 00:00:00 2001 From: Anxhul10 Date: Fri, 14 Mar 2025 02:28:39 +0530 Subject: [PATCH 09/10] fixed lint errors --- apps/meteor/tests/e2e/register.spec.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/apps/meteor/tests/e2e/register.spec.ts b/apps/meteor/tests/e2e/register.spec.ts index d0b62d1b6f0dd..1fbba1b77864f 100644 --- a/apps/meteor/tests/e2e/register.spec.ts +++ b/apps/meteor/tests/e2e/register.spec.ts @@ -1,6 +1,7 @@ import { faker } from '@faker-js/faker'; -import {request} from '../data/api-data'; + import { Utils, Registration } from './page-objects'; +import { request } from '../data/api-data'; import { test, expect } from './utils/test'; test.describe.parallel('register', () => { @@ -143,13 +144,11 @@ test.describe.parallel('register', () => { }); test('should not allow registration with an already registered email', async ({ page }) => { const email = faker.internet.email(); - await request.post('/api/v1/users.register') - .set('Content-Type', 'application/json') - .send({ + await request.post('/api/v1/users.register').set('Content-Type', 'application/json').send({ name: faker.person.firstName(), - email: email, + email, username: faker.internet.userName(), - pass: "any_password", + pass: 'any_password', }); await test.step('Attempt registration with the same email', async () => { await page.goto('/home'); @@ -167,7 +166,7 @@ test.describe.parallel('register', () => { await expect(errorMessageLocator).toHaveText('Email already exists'); }); }); - }) + }); test.describe('Registration for secret password', async () => { test.beforeEach(async ({ api, page }) => { @@ -226,4 +225,4 @@ test.describe.parallel('register', () => { await expect(page.locator('role=heading[level=2][name="The URL provided is invalid."]')).toBeVisible(); }); }); -}); \ No newline at end of file +}); From 42747587147b6873b6fbc297e665d26b40d32f66 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Wed, 18 Jun 2025 14:53:42 -0300 Subject: [PATCH 10/10] fix: error locators --- apps/meteor/tests/e2e/register.spec.ts | 33 +++++++++++-------- .../web-ui-registration/src/RegisterForm.tsx | 12 +++---- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/apps/meteor/tests/e2e/register.spec.ts b/apps/meteor/tests/e2e/register.spec.ts index 1fbba1b77864f..bf0e8029654c5 100644 --- a/apps/meteor/tests/e2e/register.spec.ts +++ b/apps/meteor/tests/e2e/register.spec.ts @@ -13,7 +13,8 @@ test.describe.parallel('register', () => { poRegistration = new Registration(page); poUtils = new Utils(page); }); - test('Successfully Registration flow', async ({ page }) => { + + test('should complete the registration flow', async ({ page }) => { await test.step('expect trigger a validation error if no data is provided on register', async () => { await page.goto('/home'); await poRegistration.goToRegister.click(); @@ -44,24 +45,27 @@ test.describe.parallel('register', () => { }); }); - test.describe('Registration without Account confirmation password set', async () => { + test.describe('should complete registration without account confirmation password set', async () => { test.beforeEach(async ({ api }) => { const result = await api.post('/settings/Accounts_RequirePasswordConfirmation', { value: false }); await expect(result.ok()).toBeTruthy(); }); + test.beforeEach(async ({ page }) => { await page.goto('/home'); await poRegistration.goToRegister.click(); }); + test.afterEach(async ({ api }) => { const result = await api.post('/settings/Accounts_RequirePasswordConfirmation', { value: true, }); + await expect(result.ok()).toBeTruthy(); }); - test('expect to register a user without password confirmation', async () => { + test('should register a user without password confirmation', async () => { await test.step('expect to not have password confirmation field', async () => { await expect(poRegistration.inputPasswordConfirm).toBeHidden(); }); @@ -78,12 +82,13 @@ test.describe.parallel('register', () => { }); }); - test.describe('Registration with manually confirmation enabled', async () => { + test.describe('should complete registration with manually confirmation enabled', async () => { test.beforeEach(async ({ api }) => { const result = await api.post('/settings/Accounts_ManuallyApproveNewUsers', { value: true }); await expect(result.ok()).toBeTruthy(); }); + test.beforeEach(async ({ page }) => { poRegistration = new Registration(page); @@ -98,7 +103,7 @@ test.describe.parallel('register', () => { await expect(result.ok()).toBeTruthy(); }); - test('it should expect to have a textbox asking the reason for the registration', async () => { + test('should expect to have a textbox asking the reason for the registration', async () => { await test.step('expect to have a textbox asking the reason for the registration', async () => { await expect(poRegistration.inputReason).toBeVisible(); }); @@ -120,7 +125,7 @@ test.describe.parallel('register', () => { await api.post('/settings/Accounts_RegistrationForm', { value: 'Public' }); }); - test('It should expect a message warning that registration is disabled', async ({ page }) => { + test('should expect a message warning that registration is disabled', async ({ page }) => { await page.goto('/home'); await poRegistration.goToRegister.click(); await expect(poRegistration.registrationDisabledCallout).toBeVisible(); @@ -142,6 +147,7 @@ test.describe.parallel('register', () => { poRegistration = new Registration(page); poUtils = new Utils(page); }); + test('should not allow registration with an already registered email', async ({ page }) => { const email = faker.internet.email(); await request.post('/api/v1/users.register').set('Content-Type', 'application/json').send({ @@ -150,6 +156,7 @@ test.describe.parallel('register', () => { username: faker.internet.userName(), pass: 'any_password', }); + await test.step('Attempt registration with the same email', async () => { await page.goto('/home'); await poRegistration.goToRegister.click(); @@ -158,12 +165,9 @@ test.describe.parallel('register', () => { await poRegistration.username.fill(faker.internet.userName()); await poRegistration.inputPassword.fill('any_password'); await poRegistration.inputPasswordConfirm.fill('any_password'); - await poRegistration.btnRegister.click(); - const errorMessageLocator = page.locator('span.rcx-field__error').filter({ hasText: 'Email already exists' }); - await expect(errorMessageLocator).toBeVisible(); - await expect(errorMessageLocator).toHaveText('Email already exists'); + await expect(page.getByRole('alert').filter({ hasText: 'Email already exists' })).toBeVisible(); }); }); }); @@ -182,7 +186,7 @@ test.describe.parallel('register', () => { await expect(result.ok()).toBeTruthy(); }); - test('It should expect a message warning that registration is disabled', async ({ page }) => { + test('should expect a message warning that registration is disabled', async ({ page }) => { await page.goto('/home'); await poRegistration.goToRegister.click(); await expect(poRegistration.registrationDisabledCallout).toBeVisible(); @@ -192,14 +196,15 @@ test.describe.parallel('register', () => { test.beforeEach(async ({ page }) => { await page.goto('/register/invalid_secret'); }); - test('It should expect a invalid page informing that the secret password is invalid', async ({ page }) => { + + test('should expect a invalid page informing that the secret password is invalid', async ({ page }) => { await expect(page.locator('role=heading[level=2][name="The URL provided is invalid."]')).toBeVisible({ timeout: 10000, }); }); }); - test('It should register a user if the right secret password is provided', async ({ page }) => { + test('should register a user if the right secret password is provided', async ({ page }) => { await page.goto('/register/secret'); await page.waitForSelector('role=form'); await poRegistration.inputName.fill(faker.person.firstName()); @@ -219,7 +224,7 @@ test.describe.parallel('register', () => { await expect(result.ok()).toBeTruthy(); }); - test('It should show an invalid page informing that the url is not valid', async ({ page }) => { + test('should show an invalid page informing that the url is not valid', async ({ page }) => { await page.goto('/register/secret'); await page.waitForSelector('role=heading[level=2]'); await expect(page.locator('role=heading[level=2][name="The URL provided is invalid."]')).toBeVisible(); diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index dcf32c72d4180..9b7f3e548d3eb 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -158,7 +158,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors.name && ( - + {errors.name.message} )} @@ -185,7 +185,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors.email && ( - + {errors.email.message} )} @@ -208,7 +208,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors.username && ( - + {errors.username.message} )} @@ -232,7 +232,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors?.password && ( - + {errors.password.message} )} @@ -260,7 +260,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors.passwordConfirmation && ( - + {errors.passwordConfirmation.message} )} @@ -284,7 +284,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo /> {errors.reason && ( - + {errors.reason.message} )}