Skip to content

Commit

Permalink
feat: Disallow repeating last 5 passwords. (#7552)
Browse files Browse the repository at this point in the history
We'll store hashes for the last 5 passwords, fetch them all for the user
wanting to change their password, and make sure the password does not
verify against any of the 5 stored hashes.

Includes some password-related UI/UX improvements and refactors. Also
some fixes related to reset password rate limiting (instead of an
unhandled exception), and token expiration on error.

---------

Co-authored-by: Nuno Góis <[email protected]>
  • Loading branch information
chriswk and nunogois authored Jul 9, 2024
1 parent ef3ef87 commit f65afff
Show file tree
Hide file tree
Showing 22 changed files with 324 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { IUser } from 'interfaces/user';
import useAdminUsersApi from 'hooks/api/actions/useAdminUsersApi/useAdminUsersApi';
import { UserAvatar } from 'component/common/UserAvatar/UserAvatar';
import useToast from 'hooks/useToast';
import { formatUnknownError } from 'utils/formatUnknownError';

const StyledUserAvatar = styled(UserAvatar)(({ theme }) => ({
width: theme.spacing(5),
Expand All @@ -37,7 +38,7 @@ const ChangePassword = ({
const [validPassword, setValidPassword] = useState(false);
const { classes: themeStyles } = useThemeStyles();
const { changePassword } = useAdminUsersApi();
const { setToastData } = useToast();
const { setToastData, setToastApiError } = useToast();

const updateField: React.ChangeEventHandler<HTMLInputElement> = (event) => {
setError(undefined);
Expand Down Expand Up @@ -66,8 +67,9 @@ const ChangePassword = ({
type: 'success',
});
} catch (error: unknown) {
console.warn(error);
setError(PASSWORD_FORMAT_MESSAGE);
const formattedError = formatUnknownError(error);
setError(formattedError);
setToastApiError(formattedError);
}
};

Expand Down Expand Up @@ -134,7 +136,7 @@ const ChangePassword = ({
/>
<PasswordMatcher
started={Boolean(data.password && data.confirm)}
matchingPasswords={data.password === data.confirm}
passwordsDoNotMatch={data.password !== data.confirm}
/>
</form>
</Dialogue>
Expand Down
17 changes: 12 additions & 5 deletions frontend/src/component/user/NewUser/NewUser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const NewUser = () => {
const { authDetails } = useAuthDetails();
const { setToastApiError } = useToast();
const navigate = useNavigate();
const [apiError, setApiError] = useState(false);
const [apiError, setApiError] = useState('');
const [email, setEmail] = useState('');
const [name, setName] = useState('');
const {
Expand Down Expand Up @@ -61,10 +61,13 @@ export const NewUser = () => {
if (res.status === OK) {
navigate('/login?reset=true');
} else {
setApiError(true);
setApiError(
'Something went wrong when attempting to update your password. This could be due to unstable internet connectivity. If retrying the request does not work, please try again later.',
);
}
} catch (e) {
setApiError(true);
const error = formatUnknownError(e);
setApiError(error);
}
};

Expand Down Expand Up @@ -199,8 +202,12 @@ export const NewUser = () => {
Set a password for your account.
</Typography>
<ConditionallyRender
condition={apiError && isValidToken}
show={<ResetPasswordError />}
condition={isValidToken}
show={
<ResetPasswordError>
{apiError}
</ResetPasswordError>
}
/>
<ResetPasswordForm onSubmit={onSubmit} />
</>
Expand Down

This file was deleted.

18 changes: 15 additions & 3 deletions frontend/src/component/user/Profile/PasswordTab/PasswordTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,20 @@ export const PasswordTab = () => {
const [oldPassword, setOldPassword] = useState('');
const { changePassword } = usePasswordApi();

const passwordsDoNotMatch = password !== confirmPassword;
const sameAsOldPassword = oldPassword === confirmPassword;
const allPasswordsFilled =
password.length > 0 &&
confirmPassword.length > 0 &&
oldPassword.length > 0;

const hasError =
!allPasswordsFilled || passwordsDoNotMatch || sameAsOldPassword;

const submit = async (e: SyntheticEvent) => {
e.preventDefault();

if (password !== confirmPassword) {
if (hasError) {
return;
} else if (!validPassword) {
setError(PASSWORD_FORMAT_MESSAGE);
Expand Down Expand Up @@ -125,15 +135,17 @@ export const PasswordTab = () => {
/>
<PasswordMatcher
data-loading
started={Boolean(password && confirmPassword)}
matchingPasswords={password === confirmPassword}
started={allPasswordsFilled}
passwordsDoNotMatch={passwordsDoNotMatch}
sameAsOldPassword={sameAsOldPassword}
/>
<Button
data-loading
variant='contained'
color='primary'
type='submit'
onClick={submit}
disabled={hasError}
>
Save
</Button>
Expand Down
19 changes: 11 additions & 8 deletions frontend/src/component/user/ResetPassword/ResetPassword.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import ResetPasswordForm from '../common/ResetPasswordForm/ResetPasswordForm';
import ResetPasswordError from '../common/ResetPasswordError/ResetPasswordError';
import { useAuthResetPasswordApi } from 'hooks/api/actions/useAuthResetPasswordApi/useAuthResetPasswordApi';
import { useAuthDetails } from 'hooks/api/getters/useAuth/useAuthDetails';
import { formatUnknownError } from 'utils/formatUnknownError';

const StyledDiv = styled('div')(({ theme }) => ({
width: '350px',
Expand All @@ -32,20 +33,23 @@ const ResetPassword = () => {
const { authDetails } = useAuthDetails();
const ref = useLoading(loading || actionLoading);
const navigate = useNavigate();
const [hasApiError, setHasApiError] = useState(false);
const [apiError, setApiError] = useState('');
const passwordDisabled = authDetails?.defaultHidden === true;

const onSubmit = async (password: string) => {
try {
const res = await resetPassword({ token, password });
if (res.status === OK) {
navigate('/login?reset=true');
setHasApiError(false);
setApiError('');
} else {
setHasApiError(true);
setApiError(
'Something went wrong when attempting to update your password. This could be due to unstable internet connectivity. If retrying the request does not work, please try again later.',
);
}
} catch (e) {
setHasApiError(true);
const error = formatUnknownError(e);
setApiError(error);
}
};

Expand All @@ -62,10 +66,9 @@ const ResetPassword = () => {
Reset password
</StyledTypography>

<ConditionallyRender
condition={hasApiError}
show={<ResetPasswordError />}
/>
<ResetPasswordError>
{apiError}
</ResetPasswordError>
<ResetPasswordForm onSubmit={onSubmit} />
</>
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { Alert, AlertTitle } from '@mui/material';

const ResetPasswordError = () => {
interface IResetPasswordErrorProps {
children: string;
}

const ResetPasswordError = ({ children }: IResetPasswordErrorProps) => {
if (!children) return null;

return (
<Alert severity='error' data-loading>
<AlertTitle>Unable to reset password</AlertTitle>
Something went wrong when attempting to update your password. This
could be due to unstable internet connectivity. If retrying the
request does not work, please try again later.
{children}
</Alert>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,59 +1,55 @@
import { styled, Typography } from '@mui/material';
import { styled } from '@mui/material';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import CheckIcon from '@mui/icons-material/Check';
import CloseIcon from '@mui/icons-material/Close';

interface IPasswordMatcherProps {
started: boolean;
matchingPasswords: boolean;
passwordsDoNotMatch: boolean;
sameAsOldPassword?: boolean;
}

const StyledMatcherContainer = styled('div')(({ theme }) => ({
position: 'relative',
paddingTop: theme.spacing(0.5),
}));

const StyledMatcher = styled(Typography, {
shouldForwardProp: (prop) => prop !== 'matchingPasswords',
})<{ matchingPasswords: boolean }>(({ theme, matchingPasswords }) => ({
position: 'absolute',
bottom: '-8px',
const StyledMatcher = styled('div', {
shouldForwardProp: (prop) => prop !== 'error',
})<{ error: boolean }>(({ theme, error }) => ({
display: 'flex',
alignItems: 'center',
color: matchingPasswords
? theme.palette.primary.main
: theme.palette.error.main,
lineHeight: 1,
color: error ? theme.palette.error.main : theme.palette.primary.main,
}));

const StyledMatcherCheckIcon = styled(CheckIcon)(({ theme }) => ({
const StyledMatcherCheckIcon = styled(CheckIcon)({
marginRight: '5px',
}));
});

const StyledMatcherErrorIcon = styled(CloseIcon)({
marginRight: '5px',
});

const PasswordMatcher = ({
started,
matchingPasswords,
passwordsDoNotMatch,
sameAsOldPassword = false,
}: IPasswordMatcherProps) => {
const error = passwordsDoNotMatch || sameAsOldPassword;

if (!started) return null;

const label = passwordsDoNotMatch
? 'Passwords do not match'
: sameAsOldPassword
? 'Cannot be the same as the old password'
: 'Passwords match';

return (
<StyledMatcherContainer>
<StyledMatcher data-loading error={error}>
<ConditionallyRender
condition={started}
show={
<StyledMatcher
variant='body2'
data-loading
matchingPasswords={matchingPasswords}
>
<StyledMatcherCheckIcon />{' '}
<ConditionallyRender
condition={matchingPasswords}
show={<Typography> Passwords match</Typography>}
elseShow={
<Typography> Passwords do not match</Typography>
}
/>
</StyledMatcher>
}
/>
</StyledMatcherContainer>
condition={error}
show={<StyledMatcherErrorIcon />}
elseShow={<StyledMatcherCheckIcon />}
/>{' '}
<span>{label}</span>
</StyledMatcher>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const ResetPasswordForm = ({ onSubmit }: IResetPasswordProps) => {

<PasswordMatcher
started={started}
matchingPasswords={matchingPasswords}
passwordsDoNotMatch={!matchingPasswords}
/>
<StyledButton
variant='contained'
Expand Down
Loading

0 comments on commit f65afff

Please sign in to comment.