From 9adebfa02dd0618b4b070c42f24fd7ca6d957e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9ssica=20Souza?= Date: Fri, 11 Jul 2025 10:06:37 -0300 Subject: [PATCH] fix: redirect on required password change (#36381) Co-authored-by: Matheus Cardoso <5606812+cardoso@users.noreply.github.com> --- .changeset/stupid-fishes-turn.md | 5 + apps/meteor/server/methods/setUserPassword.ts | 11 +- .../e2e/user-required-password-change.spec.ts | 166 ++++++++++++++++++ apps/meteor/tests/e2e/utils/user-helpers.ts | 2 + packages/ui-contexts/src/hooks/useMethod.ts | 2 +- 5 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 .changeset/stupid-fishes-turn.md create mode 100644 apps/meteor/tests/e2e/user-required-password-change.spec.ts diff --git a/.changeset/stupid-fishes-turn.md b/.changeset/stupid-fishes-turn.md new file mode 100644 index 00000000000..2916ce35938 --- /dev/null +++ b/.changeset/stupid-fishes-turn.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixes redirection not being triggered after a required password change diff --git a/apps/meteor/server/methods/setUserPassword.ts b/apps/meteor/server/methods/setUserPassword.ts index e17fa09baf4..9e37c7ee558 100644 --- a/apps/meteor/server/methods/setUserPassword.ts +++ b/apps/meteor/server/methods/setUserPassword.ts @@ -6,6 +6,7 @@ import { Meteor } from 'meteor/meteor'; import type { UpdateResult } from 'mongodb'; import { passwordPolicy } from '../../app/lib/server'; +import { notifyOnUserChange } from '../../app/lib/server/lib/notifyListener'; import { compareUserPassword } from '../lib/compareUserPassword'; declare module '@rocket.chat/ddp-client' { @@ -52,6 +53,14 @@ Meteor.methods({ logout: false, }); - return Users.unsetRequirePasswordChange(userId); + const update = await Users.unsetRequirePasswordChange(userId); + + void notifyOnUserChange({ + clientAction: 'updated', + id: userId, + diff: { requirePasswordChange: false, requirePasswordChangeReason: false }, + }); + + return update; }, }); diff --git a/apps/meteor/tests/e2e/user-required-password-change.spec.ts b/apps/meteor/tests/e2e/user-required-password-change.spec.ts new file mode 100644 index 00000000000..0135d21d67d --- /dev/null +++ b/apps/meteor/tests/e2e/user-required-password-change.spec.ts @@ -0,0 +1,166 @@ +import { faker } from '@faker-js/faker'; + +import { DEFAULT_USER_CREDENTIALS } from './config/constants'; +import { Registration } from './page-objects'; +import { HomeSidenav } from './page-objects/fragments/home-sidenav'; +import { getSettingValueById, setSettingValueById } from './utils'; +import { test, expect } from './utils/test'; +import type { ITestUser } from './utils/user-helpers'; +import { createTestUser } from './utils/user-helpers'; + +test.describe('User - Password change required', () => { + let poRegistration: Registration; + let poSidenav: HomeSidenav; + let userRequiringPasswordChange: ITestUser; + let userNotRequiringPasswordChange: ITestUser; + let userNotAbleToLogin: ITestUser; + let settingDefaultValue: unknown; + + test.beforeAll(async ({ api }) => { + settingDefaultValue = await getSettingValueById(api, 'Accounts_RequirePasswordConfirmation'); + await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', true); + userRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: true } }); + userNotRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: false } }); + userNotAbleToLogin = await createTestUser(api, { data: { requirePasswordChange: true } }); + }); + + test.beforeEach(async ({ page }) => { + poRegistration = new Registration(page); + poSidenav = new HomeSidenav(page); + await page.goto('/home'); + }); + + test.afterAll(async ({ api }) => { + await Promise.all([ + setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', settingDefaultValue), + userRequiringPasswordChange.delete(), + userNotRequiringPasswordChange.delete(), + userNotAbleToLogin.delete(), + ]); + }); + + test('should redirect to home after successful password change for new user', async ({ page }) => { + await test.step('login with temporary password', async () => { + await poRegistration.username.fill(userRequiringPasswordChange.data.username); + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.btnLogin.click(); + }); + + await test.step('should be redirected to password change screen', async () => { + await expect(poRegistration.inputPassword).toBeVisible(); + await expect(poRegistration.inputPasswordConfirm).toBeVisible(); + }); + + await test.step('enter new password and confirm', async () => { + const newPassword = faker.internet.password(); + await poRegistration.inputPassword.fill(newPassword); + await poRegistration.inputPasswordConfirm.fill(newPassword); + }); + + await test.step('click reset button', async () => { + await poRegistration.btnReset.click(); + }); + + await test.step('verify password reset form is not visible', async () => { + await page.waitForURL('/home'); + await expect(poRegistration.inputPassword).not.toBeVisible(); + await expect(poRegistration.inputPasswordConfirm).not.toBeVisible(); + }); + + await test.step('verify user is properly logged in', async () => { + await expect(poSidenav.userProfileMenu).toBeVisible(); + await expect(poSidenav.sidebarChannelsList).toBeVisible(); + }); + }); + + test('should show error when password confirmation does not match', async () => { + await test.step('login with temporary password', async () => { + await poRegistration.username.fill(userNotAbleToLogin.data.username); + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.btnLogin.click(); + }); + + await test.step('should be redirected to password change screen', async () => { + await expect(poRegistration.inputPassword).toBeVisible(); + await expect(poRegistration.inputPasswordConfirm).toBeVisible(); + }); + + await test.step('enter different passwords in password and confirm fields', async () => { + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.inputPasswordConfirm.fill(faker.internet.password()); + }); + + await test.step('attempt to submit password change', async () => { + await poRegistration.btnReset.click(); + }); + + await test.step('verify error message appears for password mismatch', async () => { + await expect(poRegistration.inputPasswordConfirm).toBeInvalid(); + }); + }); + + test('should show error when user tries to use the same password as current', async () => { + await test.step('login with temporary password', async () => { + await poRegistration.username.fill(userNotAbleToLogin.data.username); + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.btnLogin.click(); + }); + + await test.step('should be redirected to password change screen', async () => { + await expect(poRegistration.inputPassword).toBeVisible(); + await expect(poRegistration.inputPasswordConfirm).toBeVisible(); + }); + + await test.step('enter the same password as current password', async () => { + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.inputPasswordConfirm.fill(DEFAULT_USER_CREDENTIALS.password); + }); + + await test.step('attempt to submit password change', async () => { + await poRegistration.btnReset.click(); + }); + + await test.step('verify error message appears for same password', async () => { + await expect(poRegistration.inputPassword).toBeInvalid(); + }); + }); +}); + +test.describe('User - Password change not required', () => { + let poRegistration: Registration; + let poSidenav: HomeSidenav; + let userNotRequiringPasswordChange: ITestUser; + let settingDefaultValue: unknown; + + test.beforeAll(async ({ api }) => { + settingDefaultValue = await getSettingValueById(api, 'Accounts_RequirePasswordConfirmation'); + userNotRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: false } }); + }); + + test.beforeEach(async ({ page }) => { + poRegistration = new Registration(page); + poSidenav = new HomeSidenav(page); + await page.goto('/home'); + }); + + test.afterAll(async ({ api }) => { + await Promise.all([ + setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', settingDefaultValue), + userNotRequiringPasswordChange.delete(), + ]); + }); + + test('should not require password change if the requirePasswordChange is disabled', async ({ page }) => { + await test.step('login with user not requiring password change', async () => { + await poRegistration.username.fill(userNotRequiringPasswordChange.data.username); + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.btnLogin.click(); + }); + + await test.step('verify user is properly logged in', async () => { + await page.waitForURL('/home'); + await expect(poSidenav.userProfileMenu).toBeVisible(); + await expect(poSidenav.sidebarChannelsList).toBeVisible(); + }); + }); +}); diff --git a/apps/meteor/tests/e2e/utils/user-helpers.ts b/apps/meteor/tests/e2e/utils/user-helpers.ts index 329b7b83aaa..267fbc6b6b9 100644 --- a/apps/meteor/tests/e2e/utils/user-helpers.ts +++ b/apps/meteor/tests/e2e/utils/user-helpers.ts @@ -11,6 +11,7 @@ export interface ICreateUserOptions { name?: string; password?: string; roles?: string[]; + data?: Record; } export interface ITestUser { @@ -31,6 +32,7 @@ export async function createTestUser(api: BaseTest['api'], options: ICreateUserO password: options.password || DEFAULT_USER_CREDENTIALS.password, username: options.username || `test-user-${faker.string.uuid()}`, roles: options.roles || ['user'], + ...options.data, }; const response = await api.post('/users.create', userData); diff --git a/packages/ui-contexts/src/hooks/useMethod.ts b/packages/ui-contexts/src/hooks/useMethod.ts index a9f1513ed2f..ea846616265 100644 --- a/packages/ui-contexts/src/hooks/useMethod.ts +++ b/packages/ui-contexts/src/hooks/useMethod.ts @@ -7,7 +7,7 @@ type ServerMethodFunction = ( ...args: ServerMethodParameters ) => Promise>; -/* @deprecated prefer the use of api endpoints (useEndpoint) */ +/** @deprecated prefer the use of api endpoints (useEndpoint) */ export const useMethod = (methodName: MethodName): ServerMethodFunction => { const { callMethod } = useContext(ServerContext);