From 5c50598eb1bc14b5c063ef5cc94f5b17996a9a1a Mon Sep 17 00:00:00 2001 From: Sarath Nair <91882341+sarath-nair-sonarsource@users.noreply.github.com> Date: Fri, 6 Sep 2024 12:10:34 +0200 Subject: [PATCH] SONAR-22936 Update user password complexity (#11694) --- .../js/apps/users/__tests__/UsersApp-it.tsx | 43 +++++++++-------- .../js/apps/users/components/PasswordForm.tsx | 47 ++++--------------- .../components/common/UserPasswordInput.tsx | 4 +- .../resources/org/sonar/l10n/core.properties | 1 + 4 files changed, 35 insertions(+), 60 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx b/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx index 58dd60c1e49..d6d7940c7df 100644 --- a/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx +++ b/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx @@ -137,14 +137,12 @@ const ui = { loginInput: byRole('textbox', { name: /login/ }), userNameInput: byRole('textbox', { name: /name/ }), emailInput: byRole('textbox', { name: /email/ }), - passwordInput: byLabelText(/password/), - confirmPasswordInput: byLabelText(/confirm_password/), + passwordInput: byLabelText(/^password/), dialogSCMInputs: byRole('textbox', { name: /users.create_user.scm_account/ }), dialogSCMInput: (value?: string) => byRole('textbox', { name: `users.create_user.scm_account_${value ? `x.${value}` : 'new'}` }), oldPassword: byLabelText('my_profile.password.old', { selector: 'input', exact: false }), - newPassword: byLabelText('my_profile.password.new', { selector: 'input', exact: false }), - confirmPassword: byLabelText('my_profile.password.confirm', { selector: 'input', exact: false }), + confirmPassword: byLabelText(/confirm_password\*/i), tokenNameInput: byRole('textbox', { name: 'users.tokens.name' }), deleteUserCheckbox: byRole('checkbox', { name: 'users.delete_user' }), githubProvisioningPending: byText(/synchronization_pending/), @@ -265,8 +263,8 @@ describe('in non managed mode', () => { await user.type(ui.loginInput.get(), 'Login'); await user.type(ui.userNameInput.get(), 'Jack'); - await user.type(ui.passwordInput.getAll()[0], 'P@ssword12345'); - await user.type(ui.passwordInput.getAll()[1], 'P@ssword12345'); + await user.type(ui.passwordInput.get(), 'P@ssword12345'); + await user.type(ui.confirmPassword.get(), 'P@ssword12345'); // Add SCM account expect(ui.dialogSCMInputs.queryAll()).toHaveLength(0); await user.click(ui.scmAddButton.get()); @@ -421,10 +419,19 @@ describe('in non managed mode', () => { expect(ui.changeButton.get()).toBeDisabled(); - await user.type(ui.oldPassword.get(), '123'); - await user.type(ui.newPassword.get(), '1234'); - await user.type(ui.confirmPassword.get(), '1234'); + // changes password + await user.type(ui.oldPassword.get(), 'test'); + await user.type(ui.passwordInput.get(), 'AveryStrongP@55'); + await user.type(ui.confirmPassword.get(), 'AveryStrongP@55'); + await user.click(ui.changeButton.get()); + expect(ui.dialogPasswords.query()).not.toBeInTheDocument(); + // cannot change password since old password is wrong + await user.click(await ui.aliceUpdateButton.find()); + await user.click(await byText('my_profile.password.title').find()); + await user.type(ui.oldPassword.get(), 'test'); + await user.type(ui.passwordInput.get(), 'AveryStrongP@556'); + await user.type(ui.confirmPassword.get(), 'AveryStrongP@556'); expect(ui.changeButton.get()).toBeEnabled(); expect( screen.queryByText(`user.${ChangePasswordResults.OldPasswordIncorrect}`), @@ -434,12 +441,13 @@ describe('in non managed mode', () => { await ui.dialogPasswords.byText(`user.${ChangePasswordResults.OldPasswordIncorrect}`).find(), ).toBeInTheDocument(); + // cannot change password since new and old password is same await user.clear(ui.oldPassword.get()); - await user.clear(ui.newPassword.get()); + await user.clear(ui.passwordInput.get()); await user.clear(ui.confirmPassword.get()); - await user.type(ui.oldPassword.get(), 'test'); - await user.type(ui.newPassword.get(), 'test'); - await user.type(ui.confirmPassword.get(), 'test'); + await user.type(ui.oldPassword.get(), 'AveryStrongP@55'); + await user.type(ui.passwordInput.get(), 'AveryStrongP@55'); + await user.type(ui.confirmPassword.get(), 'AveryStrongP@55'); expect( screen.queryByText(`user.${ChangePasswordResults.NewPasswordSameAsOld}`), @@ -448,15 +456,6 @@ describe('in non managed mode', () => { expect( await screen.findByText(`user.${ChangePasswordResults.NewPasswordSameAsOld}`), ).toBeInTheDocument(); - - await user.clear(ui.newPassword.get()); - await user.clear(ui.confirmPassword.get()); - await user.type(ui.newPassword.get(), 'test2'); - await user.type(ui.confirmPassword.get(), 'test2'); - - await user.click(ui.changeButton.get()); - - expect(ui.dialogPasswords.query()).not.toBeInTheDocument(); }); it('should not allow to update non-local user', async () => { diff --git a/server/sonar-web/src/main/js/apps/users/components/PasswordForm.tsx b/server/sonar-web/src/main/js/apps/users/components/PasswordForm.tsx index 2dd07421c0b..f01a3cd79f6 100644 --- a/server/sonar-web/src/main/js/apps/users/components/PasswordForm.tsx +++ b/server/sonar-web/src/main/js/apps/users/components/PasswordForm.tsx @@ -23,6 +23,7 @@ import { FlagMessage, FormField, InputField, Modal, addGlobalSuccessMessage } fr import * as React from 'react'; import { changePassword } from '../../../api/users'; import { CurrentUserContext } from '../../../app/components/current-user/CurrentUserContext'; +import UserPasswordInput from '../../../components/common/UserPasswordInput'; import { translate } from '../../../helpers/l10n'; import { ChangePasswordResults, RestUserDetailed, isLoggedIn } from '../../../types/users'; @@ -33,15 +34,17 @@ interface Props { const PASSWORD_FORM_ID = 'user-password-form'; -export default function PasswordForm(props: Props) { +export default function PasswordForm(props: Readonly) { const { user } = props; - const [confirmPassword, setConfirmPassword] = React.useState(''); const [errorTranslationKey, setErrorTranslationKey] = React.useState( undefined, ); + const [newPassword, setNewPassword] = React.useState<{ isValid: boolean; value: string }>({ + value: '', + isValid: false, + }); - const [newPassword, setNewPassword] = React.useState(''); const [oldPassword, setOldPassword] = React.useState(''); const [submitting, setSubmitting] = React.useState(false); @@ -62,12 +65,12 @@ export default function PasswordForm(props: Props) { const handleChangePassword = (event: React.SyntheticEvent) => { event.preventDefault(); - if (newPassword.length > 0 && newPassword === confirmPassword) { + if (newPassword.isValid) { setSubmitting(true); changePassword({ login: user.login, - password: newPassword, + password: newPassword.value, previousPassword: oldPassword, }).then(() => { addGlobalSuccessMessage(translate('my_profile.password.changed')); @@ -114,37 +117,7 @@ export default function PasswordForm(props: Props) { )} - - setNewPassword(event.currentTarget.value)} - required - type="password" - value={newPassword} - size="full" - /> - - - - - setConfirmPassword(event.currentTarget.value)} - required - type="password" - value={confirmPassword} - size="full" - /> - - + } onClose={props.onClose} @@ -152,7 +125,7 @@ export default function PasswordForm(props: Props) { primaryButton={