Skip to content

Commit

Permalink
Merge pull request #17724 from mozilla/FXA-10477-10481
Browse files Browse the repository at this point in the history
fix(settings): Improve code and key input for reset password
  • Loading branch information
vpomerleau authored Oct 3, 2024
2 parents a3c4432 + ffcdd10 commit afc30f0
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 41 deletions.
2 changes: 0 additions & 2 deletions packages/fxa-settings/src/components/FormVerifyTotp/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const FormVerifyTotp = ({
''
);
e.target.value = filteredCode;
console.log(e.target.value.length);
e.target.value.length === codeLength
? setIsSubmitDisabled(false)
: setIsSubmitDisabled(true);
Expand All @@ -65,7 +64,6 @@ const FormVerifyTotp = ({
} else if (!isSubmitDisabled) {
await verifyCode(code);
}
setIsSubmitDisabled(false);
};

const getDisabledButtonTitle = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import RecoveryKeySetupHint, { MAX_HINT_LENGTH } from '.';
import { viewName } from '../Settings/PageRecoveryKeyCreate';
import { renderWithRouter, MOCK_ACCOUNT } from '../../models/mocks';
import { AuthUiErrorNos } from '../../lib/auth-errors/auth-errors';
import userEvent from '@testing-library/user-event';

const gqlUnexpectedError: any = AuthUiErrorNos[999];

Expand Down Expand Up @@ -99,31 +100,15 @@ describe('RecoveryKeySetupHint', () => {
});
});

it('displays error tooltip if the hint is too long', async () => {
it('limits the input to a max length', async () => {
const user = userEvent.setup();
renderWithContext(accountWithSuccess);
const hintValueTooLong = 'a'.repeat(MAX_HINT_LENGTH + 5);
const textInput = screen.getByRole('textbox', {
name: 'Enter a hint (optional)',
});
const submitButton = screen.getByText('Finish');
fireEvent.input(textInput, {
target: { value: hintValueTooLong },
});
await waitFor(() => {
expect(textInput).toHaveValue(hintValueTooLong);
});
fireEvent.click(submitButton);
await waitFor(() => {
expect(screen.getByTestId('tooltip')).toHaveTextContent(
'The hint must contain fewer than 255 characters.'
);
expect(accountWithSuccess.updateRecoveryKeyHint).not.toBeCalled();
expect(logViewEvent).not.toBeCalledWith(
`flow.${viewName}`,
'create-hint.submit'
);
expect(navigateForward).not.toBeCalled();
});
await waitFor(() => user.type(textInput, hintValueTooLong));
expect(textInput).toHaveValue('a'.repeat(MAX_HINT_LENGTH));
});

it('logs an error if saving a valid hint failed', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const RecoveryKeySetupHint = ({
}: RecoveryKeySetupHintProps) => {
const [bannerText, setBannerText] = useState<string>();
const [hintError, setHintError] = useState<string>();
const [isLoading, setIsLoading] = useState(false);
const [isSubmitDisabled, setIsSubmitDisabled] = useState(false);
const ftlMsgResolver = useFtlMsgResolver();

const { control, getValues, handleSubmit, register } = useForm<FormData>({
Expand Down Expand Up @@ -67,7 +67,7 @@ export const RecoveryKeySetupHint = ({
};

const onSubmit = async ({ hint }: FormData) => {
setIsLoading(true);
setIsSubmitDisabled(true);
const trimmedHint = hint.trim();

if (trimmedHint.length === 0) {
Expand Down Expand Up @@ -98,8 +98,6 @@ export const RecoveryKeySetupHint = ({
}
setBannerText(localizedError);
logViewEvent(`flow.${viewName}`, 'create-hint.fail', e);
} finally {
setIsLoading(false);
}
}
}
Expand All @@ -116,6 +114,7 @@ export const RecoveryKeySetupHint = ({
defaultValue: getValues().hint,
});
const isTooLong: boolean = hint.length > MAX_HINT_LENGTH;

return (
<p
className={classNames('text-end text-xs mt-2', {
Expand Down Expand Up @@ -159,7 +158,9 @@ export const RecoveryKeySetupHint = ({
onChange={() => {
setHintError(undefined);
setBannerText(undefined);
isSubmitDisabled && setIsSubmitDisabled(false);
}}
maxLength={MAX_HINT_LENGTH}
{...{ errorText: hintError }}
/>
</FtlMsg>
Expand All @@ -168,7 +169,7 @@ export const RecoveryKeySetupHint = ({
<button
className="cta-primary cta-xl w-full mt-6 mb-4"
type="submit"
disabled={isLoading}
disabled={isSubmitDisabled}
>
Finish
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ const AccountRecoveryConfirmKeyContainer = (_: RouteComponentProps) => {
ftlMsgResolver,
error
);
setIsSubmitDisabled(false);
setErrorMessage(localizedBannerMessage);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('AccountRecoveryConfirmKey', () => {
expect(submitButton).toBeDisabled();
});

it('with more than 32 characters', async () => {
it('with more than 32 characters extra characters are truncated', async () => {
const user = userEvent.setup();
renderWithLocalizationProvider(
<Subject verifyRecoveryKey={mockVerifyRecoveryKey} />
Expand All @@ -127,7 +127,8 @@ describe('AccountRecoveryConfirmKey', () => {
const input = screen.getByRole('textbox');

await waitFor(() => user.type(input, `${MOCK_RECOVERY_KEY}abc`));
expect(submitButton).toBeDisabled();
expect(input).toHaveValue(MOCK_RECOVERY_KEY);
expect(submitButton).toBeEnabled();
});

it('with invalid Crockford base32', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import { Constants } from '../../../lib/constants';
// TODO in FXA-7894 use sensitive data client to pass sensitive data
// Depends on FXA-7400

const RECOVERY_KEY_LENGTH = 32;

const AccountRecoveryConfirmKey = ({
accountResetToken,
code,
Expand Down Expand Up @@ -78,18 +80,42 @@ const AccountRecoveryConfirmKey = ({
}
};

const truncateCode = (code: string) => {
// Truncate any characters beyond the expected length of the recovery key (keeping spaces but ignoring them in the count).
let count = 0;
let truncatedCode = '';
for (let i = 0; i < code.length; i++) {
if (count === RECOVERY_KEY_LENGTH) {
break;
}
if (code[i].match(/[a-zA-Z0-9]/)) {
count++;
}
truncatedCode += code[i];
}
return truncatedCode;
};

const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
if (errorMessage) {
setErrorMessage('');
}
// strip out any characters other than letters, numbers, spaces
// and only allow single spaces

const filteredCode = e.target.value
// filter out any characters other than alphanumeric characters and spaces
.replace(/[^a-zA-Z0-9\s]/g, '')
// only allow single spaces
.replace(/\s{2,}/g, ' ');
// only count letters and numbers to assess if submit should be disabled
setIsSubmitDisabled(removeSpaces(filteredCode).length !== 32);
e.target.value = filteredCode;

// truncate any characters beyond the expected length of the recovery key (keeping spaces but ignoring them in the count)
const truncatedCode = truncateCode(filteredCode);

// update the input value to reflect the filtered and truncated code
e.target.value = truncatedCode;

// check if submit should be enabled/disabled
const codeLengthWithoutSpaces = removeSpaces(truncatedCode).length;
setIsSubmitDisabled(codeLengthWithoutSpaces !== RECOVERY_KEY_LENGTH);
};

const ControlledCharacterCount = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ const ResetPasswordContainer = ({

let localizedErrorMessage = '';

console.log(
queryParamsToMetricsContext(
flowQueryParams as unknown as Record<string, string>
)
);

const requestResetPasswordCode = async (email: string) => {
const metricsContext = queryParamsToMetricsContext(
flowQueryParams as unknown as Record<string, string>
Expand Down

0 comments on commit afc30f0

Please sign in to comment.