-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
O3-1409: Support for users changing passwords #43
base: main
Are you sure you want to change the base?
Changes from 3 commits
cb4692f
70a8f2d
cae39ea
6159748
a2feec6
31a16d5
12e1898
3e17fb6
c7431e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||
import React, { useCallback } from 'react'; | ||||||
import { useTranslation } from 'react-i18next'; | ||||||
import { SwitcherItem } from '@carbon/react'; | ||||||
import { Settings } from '@carbon/react/icons'; | ||||||
import { showModal } from '@openmrs/esm-framework'; | ||||||
import styles from './change-password-button.scss'; | ||||||
|
||||||
export interface ChangePasswordLinkProps {} | ||||||
|
||||||
const ChangePasswordLink: React.FC<ChangePasswordLinkProps> = () => { | ||||||
const { t } = useTranslation(); | ||||||
|
||||||
const launchChangePasswordModal = useCallback(() => { | ||||||
showModal('change-password-modal'); | ||||||
}, []); | ||||||
|
||||||
return ( | ||||||
<> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we don't need this outer React fragment. |
||||||
<SwitcherItem aria-label="Switcher Container"> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
<div className={styles.changePasswordButton} role="button" onClick={launchChangePasswordModal} tabIndex={0}> | ||||||
<Settings size={20} /> | ||||||
<p>{t('changePassword', 'Change Password')}</p> | ||||||
</div> | ||||||
</SwitcherItem> | ||||||
</> | ||||||
); | ||||||
}; | ||||||
|
||||||
export default ChangePasswordLink; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
.changePasswordButton { | ||
width: 16rem; | ||
display: inline-flex; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import React from 'react'; | ||
import ChangePasswordButton from './change-password-button.component'; | ||
import { render, screen } from '@testing-library/react'; | ||
import { showModal } from '@openmrs/esm-framework'; | ||
import userEvent from '@testing-library/user-event'; | ||
|
||
const showModalMock = showModal as jest.Mock; | ||
|
||
describe('<ChangePasswordButton/>', () => { | ||
beforeEach(() => { | ||
render(<ChangePasswordButton />); | ||
}); | ||
|
||
it('should display the `Change Password` button', async () => { | ||
const user = userEvent.setup(); | ||
const changePasswordButton = await screen.findByRole('button', { | ||
name: /Change Password/i, | ||
}); | ||
|
||
await user.click(changePasswordButton); | ||
|
||
expect(showModalMock).toHaveBeenCalledWith('change-password-modal'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,197 @@ | ||||||
import React, { useCallback, useEffect, useRef, useState } from 'react'; | ||||||
import { useTranslation } from 'react-i18next'; | ||||||
import { Button, ModalBody, ModalFooter, ModalHeader, Tile, PasswordInput, Form, Layer } from '@carbon/react'; | ||||||
import { navigate, showSnackbar } from '@openmrs/esm-framework'; | ||||||
import styles from './change-password-modal.scss'; | ||||||
import { performPasswordChange } from './change-password-model.resource'; | ||||||
|
||||||
interface ChangePasswordModalProps { | ||||||
close(): void; | ||||||
} | ||||||
|
||||||
export default function ChangePasswordModal({ close }: ChangePasswordModalProps) { | ||||||
const { t } = useTranslation(); | ||||||
const [isSavingPassword, setIsSavingPassword] = useState(false); | ||||||
const oldPasswordInputRef = useRef<HTMLInputElement>(null); | ||||||
const newPasswordInputRef = useRef<HTMLInputElement>(null); | ||||||
const confirmPasswordInputRef = useRef<HTMLInputElement>(null); | ||||||
const formRef = useRef<HTMLFormElement>(null); | ||||||
const [newPasswordError, setNewPasswordErr] = useState(''); | ||||||
const [oldPasswordError, setOldPasswordErr] = useState(''); | ||||||
const [confirmPasswordError, setConfirmPasswordError] = useState(''); | ||||||
const [isOldPasswordInvalid, setIsOldPasswordInvalid] = useState<boolean>(true); | ||||||
const [isNewPasswordInvalid, setIsNewPasswordInvalid] = useState<boolean>(true); | ||||||
const [isConfirmPasswordInvalid, setIsConfirmPasswordInvalid] = useState<boolean>(true); | ||||||
const [passwordInput, setPasswordInput] = useState({ | ||||||
oldPassword: '', | ||||||
newPassword: '', | ||||||
confirmPassword: '', | ||||||
}); | ||||||
|
||||||
const handleValidation = useCallback( | ||||||
(passwordInputValue, passwordInputFieldName) => { | ||||||
if (passwordInputFieldName === 'newPassword') { | ||||||
const uppercaseRegExp = /(?=.*?[A-Z])/; | ||||||
const lowercaseRegExp = /(?=.*?[a-z])/; | ||||||
const digitsRegExp = /(?=.*?[0-9])/; | ||||||
const minLengthRegExp = /.{8,}/; | ||||||
const passwordLength = passwordInputValue.length; | ||||||
const uppercasePassword = uppercaseRegExp.test(passwordInputValue); | ||||||
const lowercasePassword = lowercaseRegExp.test(passwordInputValue); | ||||||
const digitsPassword = digitsRegExp.test(passwordInputValue); | ||||||
const minLengthPassword = minLengthRegExp.test(passwordInputValue); | ||||||
let errMsg = ''; | ||||||
if (passwordLength === 0) { | ||||||
errMsg = t('passwordIsEmpty', 'Password is empty'); | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (!uppercasePassword) { | ||||||
errMsg = t('atLeastOneUppercase', 'At least one Uppercase'); | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (!lowercasePassword) { | ||||||
errMsg = t('atLeastOneLowercase', 'At least one Lowercase'); | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (!digitsPassword) { | ||||||
errMsg = t('atLeastOneDigit', 'At least one digit'); | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (!minLengthPassword) { | ||||||
errMsg = t('minimum8Characters', 'Minimum 8 characters'); | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (passwordInput.oldPassword.length > 0 && passwordInput.newPassword === passwordInput.oldPassword) { | ||||||
errMsg = t('newPasswordMustNotBeTheSameAsOld', 'New password must not be the same as password'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} else { | ||||||
errMsg = ''; | ||||||
setIsNewPasswordInvalid(false); | ||||||
} | ||||||
setNewPasswordErr(errMsg); | ||||||
} else if ( | ||||||
passwordInputFieldName === 'confirmPassword' || | ||||||
(passwordInputFieldName === 'newPassword' && passwordInput.confirmPassword.length > 0) | ||||||
) { | ||||||
if (passwordInput.confirmPassword !== passwordInput.newPassword) { | ||||||
setConfirmPasswordError(t('confirmPasswordMustBeTheSameAsNew', 'Confirm password must be the same as new')); | ||||||
} else { | ||||||
setConfirmPasswordError(''); | ||||||
setIsConfirmPasswordInvalid(false); | ||||||
} | ||||||
} else { | ||||||
if (passwordInput.newPassword.length > 0 && passwordInput.newPassword === passwordInput.oldPassword) { | ||||||
setOldPasswordErr(t('oldPasswordMustNotBeTheSameAsNew', 'Old password must not be the same as new')); | ||||||
} else { | ||||||
setOldPasswordErr(''); | ||||||
setIsOldPasswordInvalid(false); | ||||||
} | ||||||
} | ||||||
}, | ||||||
[passwordInput.confirmPassword, passwordInput.newPassword, passwordInput.oldPassword, t], | ||||||
); | ||||||
|
||||||
useEffect(() => { | ||||||
if (passwordInput.oldPassword !== '') { | ||||||
handleValidation(passwordInput.oldPassword, 'oldPassword'); | ||||||
} | ||||||
if (passwordInput.newPassword !== '') { | ||||||
handleValidation(passwordInput.newPassword, 'newPassword'); | ||||||
} | ||||||
if (passwordInput.confirmPassword !== '') { | ||||||
handleValidation(passwordInput.confirmPassword, 'confirmPassword'); | ||||||
} | ||||||
}, [handleValidation, passwordInput]); | ||||||
Comment on lines
+83
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the best client-side validation experience, we should probably use RHF and Zod here similar to how we're validating the Vitals and Biometrics form (and various other vanilla React forms in O3). |
||||||
|
||||||
const handlePasswordChange = (event) => { | ||||||
const passwordInputValue = event.target.value.trim(); | ||||||
const passwordInputFieldName = event.target.name; | ||||||
const NewPasswordInput = { ...passwordInput, [passwordInputFieldName]: passwordInputValue }; | ||||||
setPasswordInput(NewPasswordInput); | ||||||
}; | ||||||
|
||||||
const handleSubmit = useCallback( | ||||||
async (evt: React.FormEvent<HTMLFormElement>) => { | ||||||
evt.preventDefault(); | ||||||
setIsSavingPassword(true); | ||||||
performPasswordChange(passwordInput.oldPassword, passwordInput.confirmPassword) | ||||||
.then(() => { | ||||||
close(); | ||||||
navigate({ to: `\${openmrsSpaBase}/logout` }); | ||||||
showSnackbar({ | ||||||
isLowContrast: true, | ||||||
kind: 'success', | ||||||
subtitle: t('userPasswordUpdated', 'User password updated successfully'), | ||||||
title: t('userPassword', 'User password'), | ||||||
jnsereko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}); | ||||||
}) | ||||||
.catch((error) => { | ||||||
setIsSavingPassword(false); | ||||||
showSnackbar({ | ||||||
title: t('invalidPasswordCredentials', 'Invalid password provided'), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the only reason why this might fail be because an invalid password was provided? It might be more useful to show a more generic error message along the lines of |
||||||
kind: 'error', | ||||||
isLowContrast: false, | ||||||
subtitle: error?.message, | ||||||
}); | ||||||
}); | ||||||
}, | ||||||
[close, passwordInput.confirmPassword, passwordInput.oldPassword, t], | ||||||
); | ||||||
|
||||||
return ( | ||||||
<> | ||||||
<ModalHeader closeModal={close} title={t('changePassword', 'Change Password')} /> | ||||||
<ModalBody> | ||||||
<Tile> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for Tile here. It adds unnecessary margin to the modal body. |
||||||
<Form onSubmit={handleSubmit} ref={formRef}> | ||||||
<div className={styles['input-group']}> | ||||||
<Layer> | ||||||
<PasswordInput | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's likely we need a Carbon |
||||||
id="oldPassword" | ||||||
invalid={oldPasswordError.length > 0} | ||||||
invalidText={oldPasswordError} | ||||||
labelText={t('oldPassword', 'Old Password')} | ||||||
name="oldPassword" | ||||||
value={passwordInput.oldPassword} | ||||||
onChange={handlePasswordChange} | ||||||
ref={oldPasswordInputRef} | ||||||
required | ||||||
showPasswordLabel="Show old password" | ||||||
/> | ||||||
</Layer> | ||||||
<Layer> | ||||||
<PasswordInput | ||||||
id="newPassword" | ||||||
invalid={newPasswordError.length > 0} | ||||||
invalidText={newPasswordError} | ||||||
labelText={t('newPassword', 'New Password')} | ||||||
name="newPassword" | ||||||
value={passwordInput.newPassword} | ||||||
onChange={handlePasswordChange} | ||||||
ref={newPasswordInputRef} | ||||||
required | ||||||
showPasswordLabel="Show new password" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider providing detailed helper text below the fields listing any requirements related to the data format, such as types of characters allowed per the password input usage guidelines. |
||||||
/> | ||||||
</Layer> | ||||||
<Layer> | ||||||
<PasswordInput | ||||||
id="confirmPassword" | ||||||
invalid={confirmPasswordError.length > 0} | ||||||
invalidText={confirmPasswordError} | ||||||
labelText={t('confirmPassword', 'Confirm Password')} | ||||||
name="confirmPassword" | ||||||
value={passwordInput.confirmPassword} | ||||||
onChange={handlePasswordChange} | ||||||
ref={confirmPasswordInputRef} | ||||||
required | ||||||
showPasswordLabel="Show confirm password" | ||||||
/> | ||||||
</Layer> | ||||||
</div> | ||||||
</Form> | ||||||
</Tile> | ||||||
</ModalBody> | ||||||
<ModalFooter> | ||||||
<Button kind="secondary" onClick={close}> | ||||||
{t('cancel', 'Cancel')} | ||||||
</Button> | ||||||
<Button | ||||||
type="submit" | ||||||
onClick={handleSubmit} | ||||||
disabled={isSavingPassword || isNewPasswordInvalid || isConfirmPasswordInvalid || isOldPasswordInvalid} | ||||||
> | ||||||
{t('updatePassword', 'Update Password')} | ||||||
</Button> | ||||||
</ModalFooter> | ||||||
</> | ||||||
); | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
.input-group { | ||
:global(.cds--label) { | ||
margin-top: 1rem; | ||
} | ||
|
||
:global(.cds--text-input) { | ||
background-color: white; | ||
} | ||
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Layer instead of this override. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import React from 'react'; | ||
import { render, screen } from '@testing-library/react'; | ||
import ChangePasswordModal from './change-password-modal.component'; | ||
import userEvent from '@testing-library/user-event'; | ||
|
||
describe(`Change Password Modal`, () => { | ||
it('should change user locale', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be something about changing the password, not the locale? |
||
const user = userEvent.setup(); | ||
|
||
render(<ChangePasswordModal close={jest.fn()} />); | ||
expect(screen.getByRole('button', { name: /Apply/ })).toBeDisabled(); | ||
|
||
await user.type(screen.getByLabelText(/Old Password/i), 'my-password'); | ||
await user.type(screen.getByLabelText(/New Password/i), 'my-password'); | ||
await user.type(screen.getByLabelText(/Confirm Password/i), 'my-password'); | ||
await user.click(screen.getByRole('button', { name: /Apply/i })); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,16 @@ | ||||||||||
import { openmrsFetch } from '@openmrs/esm-framework'; | ||||||||||
|
||||||||||
export async function performPasswordChange(oldPassword: string, newPassword: string) { | ||||||||||
return openmrsFetch(`/ws/rest/v1/password`, { | ||||||||||
headers: { | ||||||||||
'Content-Type': 'application/json', | ||||||||||
}, | ||||||||||
method: 'POST', | ||||||||||
body: { | ||||||||||
oldPassword: oldPassword, | ||||||||||
newPassword: newPassword, | ||||||||||
}, | ||||||||||
}).then((res) => { | ||||||||||
return res; | ||||||||||
}); | ||||||||||
Comment on lines
+13
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,23 @@ | ||
{ | ||
"atLeastOneDigit": "رقم واحد على الأقل", | ||
"atLeastOneLowercase": "حرف صغير واحد على الأقل", | ||
"atLeastOneUppercase": "حرف كبير واحد على الأقل", | ||
"cancel": "يلغي", | ||
"changePassword": "تغيير كلمة المرور", | ||
"config": "الإعدادات", | ||
"confirmPassword": "تأكيد كلمة المرور", | ||
"confirmPasswordMustBeTheSameAsNew": "تأكيد كلمة المرور يجب أن تكون هي نفسها الجديدة", | ||
"invalidPasswordCredentials": "", | ||
"legacyAdmin": "إدارة النظام القديم", | ||
"systemAdmin": "إدارة النظام" | ||
"minimum8Characters": "الحد الأدنى 8 أحرف", | ||
"newPassword": "كلمة المرور الجديدة", | ||
"newPasswordMustNotBeTheSameAsOld": "كلمة المرور الجديدة يجب ألا تكون نفس كلمة المرور", | ||
"oldPassword": "كلمة المرور القديمة", | ||
"oldPasswordMustNotBeTheSameAsNew": "يجب ألا تكون كلمة المرور القديمة هي نفس كلمة المرور الجديدة", | ||
"passwordIsEmpty": "كلمة المرور فارغة", | ||
"systemAdmin": "إدارة النظام", | ||
"updatePassword": "تطوير كلمة السر", | ||
"userPassword": "كلمة مرور المستخدم", | ||
"userPasswordUpdated": "تم تحديث كلمة مرور المستخدم بنجاح" | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,5 +1,23 @@ | ||||
{ | ||||
"atLeastOneDigit": "At least one digit", | ||||
"atLeastOneLowercase": "At least one Lowercase", | ||||
"atLeastOneUppercase": "At least one Uppercase", | ||||
"cancel": "Cancel", | ||||
"changePassword": "Change Password", | ||||
"config": "Configurations", | ||||
"confirmPassword": "Confirm Password", | ||||
"confirmPasswordMustBeTheSameAsNew": "Confirm password must be the same as new", | ||||
"invalidPasswordCredentials": "", | ||||
"legacyAdmin": "Legacy Admin", | ||||
"systemAdmin": "System Administration" | ||||
"minimum8Characters": "Minimum 8 characters", | ||||
"newPassword": "New Password", | ||||
"newPasswordMustNotBeTheSameAsOld": "New password must not be the same as password", | ||||
"oldPassword": "Old Password", | ||||
"oldPasswordMustNotBeTheSameAsNew": "Old password must not be the same as new", | ||||
"passwordIsEmpty": "Password is empty", | ||||
"systemAdmin": "System Administration", | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This translation already exists. Same applies in ar.json. |
||||
"updatePassword": "Update Password", | ||||
"userPassword": "User password", | ||||
"userPasswordUpdated": "User password updated successfully" | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't used anywhere. Might as well remove it.