Skip to content

Commit 3656264

Browse files
jessicaschellycardoso
jessicaschelly
and
cardoso
authored
fix: redirect on required password change (#36381)
Co-authored-by: Matheus Cardoso <5606812+cardoso@users.noreply.github.com>
1 parent b7f4874 commit 3656264

File tree

5 files changed

+184
-2
lines changed

5 files changed

+184
-2
lines changed

‎.changeset/stupid-fishes-turn.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
---
4+
5+
Fixes redirection not being triggered after a required password change

‎apps/meteor/server/methods/setUserPassword.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Meteor } from 'meteor/meteor';
66
import type { UpdateResult } from 'mongodb';
77

88
import { passwordPolicy } from '../../app/lib/server';
9+
import { notifyOnUserChange } from '../../app/lib/server/lib/notifyListener';
910
import { compareUserPassword } from '../lib/compareUserPassword';
1011

1112
declare module '@rocket.chat/ddp-client' {
@@ -52,6 +53,14 @@ Meteor.methods<ServerMethods>({
5253
logout: false,
5354
});
5455

55-
return Users.unsetRequirePasswordChange(userId);
56+
const update = await Users.unsetRequirePasswordChange(userId);
57+
58+
void notifyOnUserChange({
59+
clientAction: 'updated',
60+
id: userId,
61+
diff: { requirePasswordChange: false, requirePasswordChangeReason: false },
62+
});
63+
64+
return update;
5665
},
5766
});

‎apps/meteor/tests/e2e/user-required-password-change.spec.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import { faker } from '@faker-js/faker';
2+
3+
import { DEFAULT_USER_CREDENTIALS } from './config/constants';
4+
import { Registration } from './page-objects';
5+
import { HomeSidenav } from './page-objects/fragments/home-sidenav';
6+
import { getSettingValueById, setSettingValueById } from './utils';
7+
import { test, expect } from './utils/test';
8+
import type { ITestUser } from './utils/user-helpers';
9+
import { createTestUser } from './utils/user-helpers';
10+
11+
test.describe('User - Password change required', () => {
12+
let poRegistration: Registration;
13+
let poSidenav: HomeSidenav;
14+
let userRequiringPasswordChange: ITestUser;
15+
let userNotRequiringPasswordChange: ITestUser;
16+
let userNotAbleToLogin: ITestUser;
17+
let settingDefaultValue: unknown;
18+
19+
test.beforeAll(async ({ api }) => {
20+
settingDefaultValue = await getSettingValueById(api, 'Accounts_RequirePasswordConfirmation');
21+
await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', true);
22+
userRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: true } });
23+
userNotRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: false } });
24+
userNotAbleToLogin = await createTestUser(api, { data: { requirePasswordChange: true } });
25+
});
26+
27+
test.beforeEach(async ({ page }) => {
28+
poRegistration = new Registration(page);
29+
poSidenav = new HomeSidenav(page);
30+
await page.goto('/home');
31+
});
32+
33+
test.afterAll(async ({ api }) => {
34+
await Promise.all([
35+
setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', settingDefaultValue),
36+
userRequiringPasswordChange.delete(),
37+
userNotRequiringPasswordChange.delete(),
38+
userNotAbleToLogin.delete(),
39+
]);
40+
});
41+
42+
test('should redirect to home after successful password change for new user', async ({ page }) => {
43+
await test.step('login with temporary password', async () => {
44+
await poRegistration.username.fill(userRequiringPasswordChange.data.username);
45+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
46+
await poRegistration.btnLogin.click();
47+
});
48+
49+
await test.step('should be redirected to password change screen', async () => {
50+
await expect(poRegistration.inputPassword).toBeVisible();
51+
await expect(poRegistration.inputPasswordConfirm).toBeVisible();
52+
});
53+
54+
await test.step('enter new password and confirm', async () => {
55+
const newPassword = faker.internet.password();
56+
await poRegistration.inputPassword.fill(newPassword);
57+
await poRegistration.inputPasswordConfirm.fill(newPassword);
58+
});
59+
60+
await test.step('click reset button', async () => {
61+
await poRegistration.btnReset.click();
62+
});
63+
64+
await test.step('verify password reset form is not visible', async () => {
65+
await page.waitForURL('/home');
66+
await expect(poRegistration.inputPassword).not.toBeVisible();
67+
await expect(poRegistration.inputPasswordConfirm).not.toBeVisible();
68+
});
69+
70+
await test.step('verify user is properly logged in', async () => {
71+
await expect(poSidenav.userProfileMenu).toBeVisible();
72+
await expect(poSidenav.sidebarChannelsList).toBeVisible();
73+
});
74+
});
75+
76+
test('should show error when password confirmation does not match', async () => {
77+
await test.step('login with temporary password', async () => {
78+
await poRegistration.username.fill(userNotAbleToLogin.data.username);
79+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
80+
await poRegistration.btnLogin.click();
81+
});
82+
83+
await test.step('should be redirected to password change screen', async () => {
84+
await expect(poRegistration.inputPassword).toBeVisible();
85+
await expect(poRegistration.inputPasswordConfirm).toBeVisible();
86+
});
87+
88+
await test.step('enter different passwords in password and confirm fields', async () => {
89+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
90+
await poRegistration.inputPasswordConfirm.fill(faker.internet.password());
91+
});
92+
93+
await test.step('attempt to submit password change', async () => {
94+
await poRegistration.btnReset.click();
95+
});
96+
97+
await test.step('verify error message appears for password mismatch', async () => {
98+
await expect(poRegistration.inputPasswordConfirm).toBeInvalid();
99+
});
100+
});
101+
102+
test('should show error when user tries to use the same password as current', async () => {
103+
await test.step('login with temporary password', async () => {
104+
await poRegistration.username.fill(userNotAbleToLogin.data.username);
105+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
106+
await poRegistration.btnLogin.click();
107+
});
108+
109+
await test.step('should be redirected to password change screen', async () => {
110+
await expect(poRegistration.inputPassword).toBeVisible();
111+
await expect(poRegistration.inputPasswordConfirm).toBeVisible();
112+
});
113+
114+
await test.step('enter the same password as current password', async () => {
115+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
116+
await poRegistration.inputPasswordConfirm.fill(DEFAULT_USER_CREDENTIALS.password);
117+
});
118+
119+
await test.step('attempt to submit password change', async () => {
120+
await poRegistration.btnReset.click();
121+
});
122+
123+
await test.step('verify error message appears for same password', async () => {
124+
await expect(poRegistration.inputPassword).toBeInvalid();
125+
});
126+
});
127+
});
128+
129+
test.describe('User - Password change not required', () => {
130+
let poRegistration: Registration;
131+
let poSidenav: HomeSidenav;
132+
let userNotRequiringPasswordChange: ITestUser;
133+
let settingDefaultValue: unknown;
134+
135+
test.beforeAll(async ({ api }) => {
136+
settingDefaultValue = await getSettingValueById(api, 'Accounts_RequirePasswordConfirmation');
137+
userNotRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: false } });
138+
});
139+
140+
test.beforeEach(async ({ page }) => {
141+
poRegistration = new Registration(page);
142+
poSidenav = new HomeSidenav(page);
143+
await page.goto('/home');
144+
});
145+
146+
test.afterAll(async ({ api }) => {
147+
await Promise.all([
148+
setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', settingDefaultValue),
149+
userNotRequiringPasswordChange.delete(),
150+
]);
151+
});
152+
153+
test('should not require password change if the requirePasswordChange is disabled', async ({ page }) => {
154+
await test.step('login with user not requiring password change', async () => {
155+
await poRegistration.username.fill(userNotRequiringPasswordChange.data.username);
156+
await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);
157+
await poRegistration.btnLogin.click();
158+
});
159+
160+
await test.step('verify user is properly logged in', async () => {
161+
await page.waitForURL('/home');
162+
await expect(poSidenav.userProfileMenu).toBeVisible();
163+
await expect(poSidenav.sidebarChannelsList).toBeVisible();
164+
});
165+
});
166+
});

‎apps/meteor/tests/e2e/utils/user-helpers.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export interface ICreateUserOptions {
1111
name?: string;
1212
password?: string;
1313
roles?: string[];
14+
data?: Record<string, any>;
1415
}
1516

1617
export interface ITestUser {
@@ -31,6 +32,7 @@ export async function createTestUser(api: BaseTest['api'], options: ICreateUserO
3132
password: options.password || DEFAULT_USER_CREDENTIALS.password,
3233
username: options.username || `test-user-${faker.string.uuid()}`,
3334
roles: options.roles || ['user'],
35+
...options.data,
3436
};
3537

3638
const response = await api.post('/users.create', userData);

‎packages/ui-contexts/src/hooks/useMethod.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ type ServerMethodFunction<MethodName extends ServerMethodName> = (
77
...args: ServerMethodParameters<MethodName>
88
) => Promise<ServerMethodReturn<MethodName>>;
99

10-
/* @deprecated prefer the use of api endpoints (useEndpoint) */
10+
/** @deprecated prefer the use of api endpoints (useEndpoint) */
1111
export const useMethod = <MethodName extends keyof ServerMethods>(methodName: MethodName): ServerMethodFunction<MethodName> => {
1212
const { callMethod } = useContext(ServerContext);
1313

0 commit comments

Comments
 (0)