Skip to content

Commit 51604ce

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

File tree

5 files changed

+259
-2
lines changed

5 files changed

+259
-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: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { faker } from '@faker-js/faker';
2+
import type { APIResponse } from '@playwright/test';
3+
import type { IUser } from '@rocket.chat/core-typings';
4+
5+
import type { BaseTest } from './test';
6+
import { DEFAULT_USER_CREDENTIALS } from '../config/constants';
7+
8+
export interface ICreateUserOptions {
9+
username?: string;
10+
email?: string;
11+
name?: string;
12+
password?: string;
13+
roles?: string[];
14+
data?: Record<string, any>;
15+
}
16+
17+
export interface ITestUser {
18+
response: APIResponse;
19+
data: IUser & { username: string };
20+
deleted: boolean;
21+
delete: () => Promise<APIResponse | undefined>;
22+
markAsDeleted: () => void;
23+
}
24+
25+
/**
26+
* Creates a test user with optional customizations
27+
*/
28+
export async function createTestUser(api: BaseTest['api'], options: ICreateUserOptions = {}): Promise<ITestUser> {
29+
const userData = {
30+
email: options.email || faker.internet.email(),
31+
name: options.name || faker.person.fullName(),
32+
password: options.password || DEFAULT_USER_CREDENTIALS.password,
33+
username: options.username || `test-user-${faker.string.uuid()}`,
34+
roles: options.roles || ['user'],
35+
...options.data,
36+
};
37+
38+
const response = await api.post('/users.create', userData);
39+
40+
if (response.status() !== 200) {
41+
throw new Error(`Failed to create user: ${response.status()}, response: ${await response.text()}`);
42+
}
43+
44+
const { user } = await response.json();
45+
46+
return {
47+
response,
48+
data: user,
49+
deleted: false,
50+
markAsDeleted(this: ITestUser) {
51+
this.deleted = true;
52+
},
53+
async delete(this: ITestUser) {
54+
if (this.deleted) {
55+
return;
56+
}
57+
58+
const response = await api.post('/users.delete', { userId: user._id });
59+
this.markAsDeleted();
60+
return response;
61+
},
62+
};
63+
}
64+
65+
/**
66+
* Creates multiple test users at once
67+
*/
68+
export async function createTestUsers(api: BaseTest['api'], count: number, options: ICreateUserOptions = {}): Promise<ITestUser[]> {
69+
const promises = Array.from({ length: count }, (_, i) =>
70+
createTestUser(api, {
71+
...options,
72+
username: options.username ? `${options.username}-${i + 1}` : undefined,
73+
}),
74+
);
75+
76+
return Promise.all(promises);
77+
}

‎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)
⚡