Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/nervous-clouds-carry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/ui-client': minor
'@rocket.chat/meteor': minor
---

Enables the password policy by default to ensure security by default and alters SetupWizard to handle errors
4 changes: 2 additions & 2 deletions apps/meteor/server/settings/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ export const createAccountSettings = () =>
});

await this.section('Password_Policy', async function () {
await this.add('Accounts_Password_Policy_Enabled', false, {
await this.add('Accounts_Password_Policy_Enabled', true, {
type: 'boolean',
public: true,
});
Expand All @@ -820,7 +820,7 @@ export const createAccountSettings = () =>
public: true,
};

await this.add('Accounts_Password_Policy_MinLength', 7, {
await this.add('Accounts_Password_Policy_MinLength', 14, {
type: 'int',
public: true,
enableQuery,
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/tests/data/user.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Credentials } from '@rocket.chat/api-client';
import type { IUser } from '@rocket.chat/core-typings';

export const password = 'rocket.chat';
export const password = 'R0ck3t.ch@tP@ssw0rd1234.!';
export const adminUsername = 'rocketchat.internal.admin.test';
export const adminEmail = `${adminUsername}@rocket.chat`;
export const adminPassword = adminUsername;
Expand Down
14 changes: 12 additions & 2 deletions apps/meteor/tests/e2e/account-security.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,32 @@ import { test, expect } from './utils/test';

test.use({ storageState: Users.admin.state });

const RANDOM_PASSWORD = faker.string.alphanumeric(10);
const RANDOM_PASSWORD = faker.helpers
.shuffle([
faker.string.alpha({ casing: 'upper' }),
faker.string.alpha({ casing: 'lower' }),
faker.string.numeric(),
faker.string.symbol(),
faker.string.alphanumeric(10),
])
.join('');

test.describe.serial('account-security', () => {
let poAccountSecurity: AccountSecurity;

test.beforeEach(async ({ page }) => {
test.beforeEach(async ({ page, api }) => {
poAccountSecurity = new AccountSecurity(page);
await page.goto('/account/security');
await page.waitForSelector('#main-content');
await setSettingValueById(api, 'Accounts_Password_Policy_Enabled', false);
});

test.afterAll(async ({ api }) =>
Promise.all([
setSettingValueById(api, 'Accounts_AllowPasswordChange', true),
setSettingValueById(api, 'Accounts_TwoFactorAuthentication_Enabled', true),
setSettingValueById(api, 'E2E_Enable', false),
setSettingValueById(api, 'Accounts_Password_Policy_Enabled', true),
]),
);

Expand Down
8 changes: 4 additions & 4 deletions apps/meteor/tests/e2e/administration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ test.describe.parallel('administration', () => {
await poAdminUsers.editUser.inputName.fill(faker.person.firstName());
await poAdminUsers.editUser.inputUserName.fill(faker.internet.userName());
await poAdminUsers.editUser.inputSetManually.click();
await poAdminUsers.editUser.inputPassword.fill('any_password');
await poAdminUsers.editUser.inputConfirmPassword.fill('any_password');
await poAdminUsers.editUser.inputPassword.fill('P@ssw0rd1234.!');
await poAdminUsers.editUser.inputConfirmPassword.fill('P@ssw0rd1234.!');
await expect(poAdminUsers.editUser.userRole).toBeVisible();
await poAdminUsers.editUser.btnAddUser.click();
});
Expand All @@ -95,8 +95,8 @@ test.describe.parallel('administration', () => {
await poAdminUsers.editUser.inputUserName.type(username);
await poAdminUsers.editUser.inputEmail.type(faker.internet.email());
await poAdminUsers.editUser.inputSetManually.click();
await poAdminUsers.editUser.inputPassword.type('any_password');
await poAdminUsers.editUser.inputConfirmPassword.type('any_password');
await poAdminUsers.editUser.inputPassword.type('P@ssw0rd1234.!');
await poAdminUsers.editUser.inputConfirmPassword.type('P@ssw0rd1234.!');
await expect(poAdminUsers.editUser.userRole).toBeVisible();
await expect(poAdminUsers.editUser.joinDefaultChannels).toBeVisible();
await poAdminUsers.editUser.btnAddUser.click();
Expand Down
18 changes: 9 additions & 9 deletions apps/meteor/tests/e2e/register.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ test.describe.parallel('register', () => {
await poRegistration.inputName.fill(faker.person.firstName());
await poRegistration.inputEmail.fill(faker.internet.email());
await poRegistration.username.fill(faker.internet.userName());
await poRegistration.inputPassword.fill('any_password');
await poRegistration.inputPasswordConfirm.fill('any_password_2');
await poRegistration.inputPassword.fill('P@ssw0rd1234.!');
await poRegistration.inputPasswordConfirm.fill('Password1235.!');
await poRegistration.btnRegister.click();

await expect(poRegistration.inputPasswordConfirm).toBeInvalid();
});

await test.step('expect successfully register a new user', async () => {
await poRegistration.inputPasswordConfirm.fill('any_password');
await poRegistration.inputPasswordConfirm.fill('P@ssw0rd1234.!');
await poRegistration.btnRegister.click();
await poAuth.waitForDisplay();
});
Expand Down Expand Up @@ -74,7 +74,7 @@ test.describe.parallel('register', () => {
await poRegistration.inputName.fill(faker.person.firstName());
await poRegistration.inputEmail.fill(faker.internet.email());
await poRegistration.username.fill(faker.internet.userName());
await poRegistration.inputPassword.fill('any_password');
await poRegistration.inputPassword.fill('P@ssw0rd1234.!');

await poRegistration.btnRegister.click();
await poAuth.waitForDisplay();
Expand Down Expand Up @@ -149,7 +149,7 @@ test.describe.parallel('register', () => {
name: faker.person.firstName(),
email,
username: faker.internet.userName(),
pass: 'any_password',
pass: 'P@ssw0rd1234.!',
});

await test.step('Attempt registration with the same email', async () => {
Expand All @@ -158,8 +158,8 @@ test.describe.parallel('register', () => {
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.inputPassword.fill('P@ssw0rd1234.!');
await poRegistration.inputPasswordConfirm.fill('P@ssw0rd1234.!');
await poRegistration.btnRegister.click();

await expect(page.getByRole('alert').filter({ hasText: 'Email already exists' })).toBeVisible();
Expand Down Expand Up @@ -203,8 +203,8 @@ test.describe.parallel('register', () => {
await poRegistration.inputName.fill(faker.person.firstName());
await poRegistration.inputEmail.fill(faker.internet.email());
await poRegistration.username.fill(faker.internet.userName());
await poRegistration.inputPassword.fill('any_password');
await poRegistration.inputPasswordConfirm.fill('any_password');
await poRegistration.inputPassword.fill('P@ssw0rd1234.!');
await poRegistration.inputPasswordConfirm.fill('P@ssw0rd1234.!');
await poRegistration.btnRegister.click();
await poAuth.waitForDisplay();
});
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/tests/e2e/reset-password.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ test.describe.parallel('Reset Password', () => {
});

test('should confirm password be invalid', async () => {
await poRegistration.inputPassword.fill('123456');
await poRegistration.inputPasswordConfirm.fill('123455');
await poRegistration.inputPassword.fill('P@ssw0rd1234.!');
await poRegistration.inputPasswordConfirm.fill('Password4321.!');
await poRegistration.btnReset.click();
await expect(poRegistration.inputPasswordConfirm).toBeInvalid();
});
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/user-required-password-change.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ test.describe('User - Password change required', () => {

test.beforeAll(async ({ api }) => {
settingDefaultValue = await getSettingValueById(api, 'Accounts_RequirePasswordConfirmation');
await setSettingValueById(api, 'Accounts_Password_Policy_Enabled', false);
await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', true);
userRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: true } });
userNotRequiringPasswordChange = await createTestUser(api, { data: { requirePasswordChange: false } });
Expand All @@ -38,6 +39,7 @@ test.describe('User - Password change required', () => {
userRequiringPasswordChange.delete(),
userNotRequiringPasswordChange.delete(),
userNotAbleToLogin.delete(),
setSettingValueById(api, 'Accounts_Password_Policy_Enabled', true),
]);
});

Expand Down
22 changes: 10 additions & 12 deletions apps/meteor/tests/end-to-end/api/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ describe('[Users]', () => {
email,
name: 'name',
username,
pass: 'test',
pass: 'P@ssw0rd1234.!',
})
.expect('Content-Type', 'application/json')
.expect(200)
Expand All @@ -838,7 +838,7 @@ describe('[Users]', () => {
email,
name: 'name',
username: 'test$username<>',
pass: 'test',
pass: 'P@ssw0rd1234.!',
})
.expect('Content-Type', 'application/json')
.expect(400)
Expand All @@ -856,7 +856,7 @@ describe('[Users]', () => {
email,
name: 'name',
username,
pass: 'test',
pass: 'P@ssw0rd1234.!',
})
.expect('Content-Type', 'application/json')
.expect(400)
Expand All @@ -873,7 +873,7 @@ describe('[Users]', () => {
email,
name: '</\\name>',
username,
pass: 'test',
pass: 'P@ssw0rd1234.!',
})
.expect('Content-Type', 'application/json')
.expect(400)
Expand Down Expand Up @@ -1105,7 +1105,7 @@ describe('[Users]', () => {
email: `me-${Date.now()}@email.com`,
name: 'testuser',
username: ufsUsername,
password: '1234',
password,
});

await request
Expand Down Expand Up @@ -2171,7 +2171,7 @@ describe('[Users]', () => {
.send({
userId: targetUser._id,
data: {
password: 'itsnotworking',
password: '1tsn0tw0rkingP@ssw0rd1234.!',
},
})
.expect('Content-Type', 'application/json')
Expand All @@ -2193,7 +2193,7 @@ describe('[Users]', () => {
.send({
userId: targetUser._id,
data: {
password: 'itsnotworking',
password: '1tsn0tw0rkingP@ssw0rd1234.!',
},
})
.expect('Content-Type', 'application/json')
Expand Down Expand Up @@ -2700,7 +2700,7 @@ describe('[Users]', () => {
.set(credentials)
.send({
data: {
newPassword: 'the new pass',
newPassword: '1Tsn3wP@ssw0rd1234.!',
},
})
.expect('Content-Type', 'application/json')
Expand Down Expand Up @@ -2851,7 +2851,7 @@ describe('[Users]', () => {
.set(credentials)
.send({
data: {
newPassword: 'MyNewPassw0rd',
newPassword: '1Tsn3wP@ssw0rd1234.!',
},
})
.expect('Content-Type', 'application/json')
Expand Down Expand Up @@ -2891,13 +2891,11 @@ describe('[Users]', () => {
describe('[Password Policy]', () => {
before(async () => {
await updateSetting('Accounts_AllowPasswordChange', true);
await updateSetting('Accounts_Password_Policy_Enabled', true);
await updateSetting('Accounts_TwoFactorAuthentication_Enabled', false);
});

after(async () => {
await updateSetting('Accounts_AllowPasswordChange', true);
await updateSetting('Accounts_Password_Policy_Enabled', false);
await updateSetting('Accounts_TwoFactorAuthentication_Enabled', true);
});

Expand Down Expand Up @@ -3086,7 +3084,7 @@ describe('[Users]', () => {
.send({
data: {
currentPassword,
newPassword: '123Abc@!',
newPassword: '1Tsn3wP@ssw0rd1234.!',
},
})
.expect('Content-Type', 'application/json')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@ it('should render no policy if its disabled ', () => {

it('should render no policy if its enabled but empty', async () => {
render(<PasswordVerifier password='asasdfafdgsdffdf' />, {
wrapper: mockAppRoot().build(),
wrapper: mockAppRoot()
.withSetting('Accounts_Password_Policy_Enabled', true)
.withSetting('Accounts_Password_Policy_MinLength', -1)
.withSetting('Accounts_Password_Policy_MaxLength', -1)
.withSetting('Accounts_Password_Policy_ForbidRepeatingCharacters', false)
.withSetting('Accounts_Password_Policy_ForbidRepeatingCharactersCount', 3)
.withSetting('Accounts_Password_Policy_AtLeastOneLowercase', false)
.withSetting('Accounts_Password_Policy_AtLeastOneUppercase', false)
.withSetting('Accounts_Password_Policy_AtLeastOneNumber', false)
.withSetting('Accounts_Password_Policy_AtLeastOneSpecialCharacter', false)
.build(),
});

await waitFor(() => {
Expand Down
40 changes: 37 additions & 3 deletions packages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { AdminInfoPage } from '@rocket.chat/onboarding-ui';
import { escapeRegExp } from '@rocket.chat/string-helpers';
import { useSetting } from '@rocket.chat/ui-contexts';
import { useSetting, useVerifyPassword, usePasswordPolicy, usePasswordPolicyOptions } from '@rocket.chat/ui-contexts';
import type { ReactElement, ComponentProps } from 'react';
import { useMemo } from 'react';
import { I18nextProvider, useTranslation } from 'react-i18next';

import { useSetupWizardContext } from '../contexts/SetupWizardContext';
Expand All @@ -18,6 +19,26 @@ const AdminInfoStep = (): ReactElement => {

const { currentStep, validateEmail, registerAdminUser, maxSteps } = useSetupWizardContext();

const passwordPolicyOptions = usePasswordPolicyOptions();
const validatePasswordPolicy = usePasswordPolicy(passwordPolicyOptions);

const passwordPolicyValidations = useVerifyPassword('');
const passwordRulesHint = useMemo(() => {
if (!passwordPolicyValidations.validations || passwordPolicyValidations.validations.length === 0) {
return '';
}

return passwordPolicyValidations.validations
.map((validation) => {
const labelKey = `${validation.name}-label` as const;
if ('limit' in validation) {
return t(labelKey, { limit: validation.limit });
}
return t(labelKey);
})
.join(', ');
}, [passwordPolicyValidations.validations, t]);

// TODO: check if username exists
const validateUsername = (username: string): boolean | string => {
if (!usernameRegExp.test(username) || hasBlockedName(username)) {
Expand All @@ -27,15 +48,28 @@ const AdminInfoStep = (): ReactElement => {
return true;
};

const validatePassword = (password: string): boolean | string => {
if (!password || password.length === 0) {
return t('Required_field', { field: t('Password') });
}

const passwordValidation = validatePasswordPolicy(password);
if (!passwordValidation.valid) {
return t('Password_must_meet_the_complexity_requirements');
}

return true;
};

const handleSubmit: ComponentProps<typeof AdminInfoPage>['onSubmit'] = async (data) => {
registerAdminUser(data);
};

return (
<I18nextProvider i18n={i18n} defaultNS='onboarding'>
<AdminInfoPage
validatePassword={(password) => password.length > 0}
passwordRulesHint=''
validatePassword={validatePassword}
passwordRulesHint={passwordRulesHint}
validateUsername={validateUsername}
validateEmail={validateEmail}
currentStep={currentStep}
Expand Down
28 changes: 28 additions & 0 deletions packages/ui-contexts/src/hooks/usePasswordPolicyOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import type { PasswordPolicyOptions } from '@rocket.chat/password-policies';

import { useSetting } from './useSetting';

export const usePasswordPolicyOptions = (): PasswordPolicyOptions => {
const enabled = useSetting('Accounts_Password_Policy_Enabled', true);
const minLength = useSetting('Accounts_Password_Policy_MinLength', 14);
const maxLength = useSetting('Accounts_Password_Policy_MaxLength', -1);
const forbidRepeatingCharacters = useSetting('Accounts_Password_Policy_ForbidRepeatingCharacters', true);
const forbidRepeatingCharactersCount = useSetting('Accounts_Password_Policy_ForbidRepeatingCharactersCount', 3);
const mustContainAtLeastOneLowercase = useSetting('Accounts_Password_Policy_AtLeastOneLowercase', true);
const mustContainAtLeastOneUppercase = useSetting('Accounts_Password_Policy_AtLeastOneUppercase', true);
const mustContainAtLeastOneNumber = useSetting('Accounts_Password_Policy_AtLeastOneNumber', true);
const mustContainAtLeastOneSpecialCharacter = useSetting('Accounts_Password_Policy_AtLeastOneSpecialCharacter', true);

return {
enabled,
minLength,
maxLength,
forbidRepeatingCharacters,
forbidRepeatingCharactersCount,
mustContainAtLeastOneLowercase,
mustContainAtLeastOneUppercase,
mustContainAtLeastOneNumber,
mustContainAtLeastOneSpecialCharacter,
throwError: false,
};
};
Loading
Loading