Wizard: Add password validation in User step

This commit adds password validation to the User step:
- Moved password validation logic to a separate `checkPasswordValidity` function, returning detailed results (strength and validation state).
- Simplified validation in `PasswordValidatedInput` by using `checkPasswordValidity` results directly.
- Added dynamic password strength indicator showing success or error based on requirements.
- Integrated environment-specific validation messages (e.g., for Azure).
- Improved code separation between presentation and validation for better maintainability.
- Unit tests: Adds tests for invalid passwords, covering both default and Azure cases.
This commit is contained in:
Michal Gold 2025-02-04 15:58:54 +02:00 committed by Klara Simickova
parent ef9510327d
commit 265ba2ac78
7 changed files with 332 additions and 113 deletions

View file

@ -492,6 +492,7 @@ const CreateImageWizard = ({ isEdit }: CreateImageWizardProps) => {
key="wizard-users"
navItem={customStatusNavItem}
isHidden={!isUsersEnabled}
status={usersValidation.disabledNext ? 'error' : 'default'}
footer={
<CustomWizardFooter
disableNext={usersValidation.disabledNext}

View file

@ -7,11 +7,7 @@ import {
TextAreaProps,
TextInput,
TextInputProps,
Button,
InputGroup,
InputGroupItem,
} from '@patternfly/react-core';
import { EyeIcon, EyeSlashIcon } from '@patternfly/react-icons';
import type { StepValidation } from './utilities/useValidation';
@ -60,57 +56,6 @@ type ErrorMessageProps = {
type ValidationResult = 'default' | 'success' | 'error';
export const HookPasswordValidatedInput = ({
ariaLabel,
placeholder,
dataTestId,
value,
ouiaId,
stepValidation,
fieldName,
onChange,
warning = undefined,
inputType,
isDisabled,
}: HookValidatedInputPropTypes) => {
const [isPasswordVisible, setIsPasswordVisible] = useState(false);
const togglePasswordVisibility = () => {
setIsPasswordVisible(!isPasswordVisible);
};
return (
<>
<InputGroup>
<InputGroupItem isFill>
<HookValidatedInput
type={isPasswordVisible ? 'text' : 'password'}
ouiaId={ouiaId || ''}
data-testid={dataTestId}
value={value}
onChange={onChange!}
stepValidation={stepValidation}
ariaLabel={ariaLabel || ''}
fieldName={fieldName}
placeholder={placeholder || ''}
inputType={inputType || 'textInput'}
warning={warning || ''}
isDisabled={isDisabled || false}
/>
</InputGroupItem>
<InputGroupItem>
<Button
variant="control"
onClick={togglePasswordVisibility}
aria-label={isPasswordVisible ? 'Show password' : 'Hide password'}
>
{isPasswordVisible ? <EyeSlashIcon /> : <EyeIcon />}
</Button>
</InputGroupItem>
</InputGroup>
</>
);
};
export const ValidatedInputAndTextArea = ({
value,
stepValidation,

View file

@ -16,11 +16,9 @@ import {
setUserAdministratorByIndex,
removeUser,
} from '../../../../../store/wizardSlice';
import { PasswordValidatedInput } from '../../../utilities/PasswordValidatedInput';
import { useUsersValidation } from '../../../utilities/useValidation';
import {
HookPasswordValidatedInput,
ValidatedInputAndTextArea,
} from '../../../ValidatedInput';
import { ValidatedInputAndTextArea } from '../../../ValidatedInput';
const UserInfo = () => {
const dispatch = useAppDispatch();
const index = 0;
@ -41,7 +39,7 @@ const UserInfo = () => {
};
const handlePasswordChange = (
_event: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>,
_event: React.FormEvent<HTMLInputElement>,
value: string
) => {
dispatch(setUserPasswordByIndex({ index: index, password: value }));
@ -81,16 +79,12 @@ const UserInfo = () => {
fieldName="userName"
/>
</FormGroup>
<FormGroup isRequired label="Password">
<HookPasswordValidatedInput
ariaLabel="blueprint user password"
value={userPassword || ''}
onChange={(_e, value) => handlePasswordChange(_e, value)}
placeholder="Enter password"
stepValidation={stepValidation}
fieldName="userPassword"
/>
</FormGroup>
<PasswordValidatedInput
value={userPassword || ''}
ariaLabel="blueprint user password"
placeholder="Enter password"
onChange={(_e, value) => handlePasswordChange(_e, value)}
/>
<FormGroup isRequired label="SSH key">
<ValidatedInputAndTextArea
inputType={'textArea'}

View file

@ -0,0 +1,95 @@
import React, { useState } from 'react';
import {
FormGroup,
FormHelperText,
HelperText,
HelperTextItem,
InputGroup,
InputGroupItem,
TextInput,
Button,
TextInputProps,
} from '@patternfly/react-core';
import { EyeIcon, EyeSlashIcon } from '@patternfly/react-icons';
import { checkPasswordValidity } from './useValidation';
import { useAppSelector } from '../../../store/hooks';
import { selectImageTypes } from '../../../store/wizardSlice';
type ValidatedPasswordInput = TextInputProps & {
value: string;
placeholder: string;
ariaLabel: string;
onChange: (event: React.FormEvent<HTMLInputElement>, value: string) => void;
};
export const PasswordValidatedInput = ({
value,
placeholder,
ariaLabel,
onChange,
}: ValidatedPasswordInput) => {
const environments = useAppSelector(selectImageTypes);
const [isPasswordVisible, setIsPasswordVisible] = useState(false);
const { validationState, strength } = checkPasswordValidity(
value,
environments.includes('azure')
);
const { ruleLength, ruleCharacters } = validationState;
const togglePasswordVisibility = () => {
setIsPasswordVisible(!isPasswordVisible);
};
const passStrLabel = (
<HelperText>
<HelperTextItem variant={strength.variant} icon={strength.icon}>
{strength.text}
</HelperTextItem>
</HelperText>
);
return (
<FormGroup label="Password" isRequired labelInfo={passStrLabel}>
<>
<InputGroup>
<InputGroupItem isFill>
<TextInput
isRequired
type={isPasswordVisible ? 'text' : 'password'}
value={value}
onChange={onChange}
aria-label={ariaLabel}
placeholder={placeholder}
/>
</InputGroupItem>
<InputGroupItem>
<Button
variant="control"
onClick={togglePasswordVisibility}
aria-label={isPasswordVisible ? 'Hide password' : 'Show password'}
>
{isPasswordVisible ? <EyeSlashIcon /> : <EyeIcon />}
</Button>
</InputGroupItem>
</InputGroup>
</>
<FormHelperText>
<HelperText component="ul">
<HelperTextItem variant={ruleLength} component="li" hasIcon>
Password must be at least 6 characters long.
</HelperTextItem>
{environments.includes('azure') && (
<HelperTextItem variant={ruleCharacters} component="li" hasIcon>
Must include at least 3 of the following: lowercase letters,
uppercase letters, numbers, symbols.
</HelperTextItem>
)}
</HelperText>
</FormHelperText>
</FormGroup>
);
};

View file

@ -1,4 +1,6 @@
import { useEffect, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { CheckCircleIcon } from '@patternfly/react-icons';
import { UNIQUE_VALIDATION_DELAY } from '../../../constants';
import { useLazyGetBlueprintsQuery } from '../../../store/backendApi';
@ -31,9 +33,11 @@ import {
selectLanguages,
selectKeyboard,
selectTimezone,
selectImageTypes,
} from '../../../store/wizardSlice';
import { keyboardsList } from '../steps/Locale/keyboardsList';
import { languagesList } from '../steps/Locale/languagesList';
import { HelperTextVariant } from '../steps/Packages/components/CustomHelperText';
import { timezones } from '../steps/Timezone/timezonesList';
import {
getDuplicateMountPoints,
@ -70,6 +74,7 @@ export function useIsBlueprintValid(): boolean {
const services = useServicesValidation();
const firstBoot = useFirstBootValidation();
const details = useDetailsValidation();
const users = useUsersValidation();
return (
!registration.disabledNext &&
!filesystem.disabledNext &&
@ -81,10 +86,32 @@ export function useIsBlueprintValid(): boolean {
!firewall.disabledNext &&
!services.disabledNext &&
!firstBoot.disabledNext &&
!details.disabledNext
!details.disabledNext &&
!users.disabledNext
);
}
type ValidationFlags = {
isUserNameValidValue: boolean;
isSshKeyValidValue: boolean;
isPasswordValidValue: boolean;
};
type PasswordValidationResult = {
isValid: boolean;
strength: {
variant: HelperTextVariant;
icon: JSX.Element | null;
text: string;
};
validationState: ValidationState;
};
type ValidationState = {
ruleLength: HelperTextVariant;
ruleCharacters: HelperTextVariant;
};
export function useRegistrationValidation(): StepValidation {
const registrationType = useAppSelector(selectRegistrationType);
const activationKey = useAppSelector(selectActivationKey);
@ -380,8 +407,23 @@ export function useServicesValidation(): StepValidation {
};
}
const getUserNameErrorMsg = (userName: string): string => {
if (userName && !isUserNameValid(userName)) {
return 'Invalid user name';
}
return '';
};
const getSshKeyErrorMsg = (userSshKey: string): string => {
if (userSshKey && !isSshKeyValid(userSshKey)) {
return 'Invalid SSH key';
}
return '';
};
export function useUsersValidation(): StepValidation {
const index = 0;
const environments = useAppSelector(selectImageTypes);
const userNameSelector = selectUserNameByIndex(index);
const userName = useAppSelector(userNameSelector);
const userPasswordSelector = selectUserPasswordByIndex(index);
@ -389,28 +431,119 @@ export function useUsersValidation(): StepValidation {
const userSshKeySelector = selectUserSshKeyByIndex(index);
const userSshKey = useAppSelector(userSshKeySelector);
const users = useAppSelector(selectUsers);
const userNameError = getUserNameErrorMsg(userName);
const sshKeyError = getSshKeyErrorMsg(userSshKey);
const { isUserNameValidValue, isSshKeyValidValue, isPasswordValidValue } =
calculateValidationFlags(
userName,
userPassword,
userSshKey,
environments.includes('azure')
);
const canProceed =
// Case 1: there is no users
users.length === 0 ||
// Case 2: All fields are empty
(userName === '' && userPassword === '' && userSshKey === '') ||
// Case 3: userName is valid and SshKey is valid
(userName &&
isUserNameValid(userName) &&
userSshKey &&
isSshKeyValid(userSshKey));
// Case 2: userName is valid and SshKey or Password is valid
(isUserNameValidValue && (isSshKeyValidValue || isPasswordValidValue));
return {
errors: {
userName:
userName && !isUserNameValid(userName) ? 'Invalid user name' : '',
userSshKey:
userSshKey && !isSshKeyValid(userSshKey) ? 'Invalid SSH key' : '',
userName: userNameError,
userSshKey: sshKeyError,
},
disabledNext: !canProceed,
};
}
const calculateValidationFlags = (
userName: string,
userPassword: string,
userSshKey: string,
isAzure: boolean
): ValidationFlags => {
const isUserNameValidValue = !!userName && isUserNameValid(userName);
const isSshKeyValidValue = !!userSshKey && isSshKeyValid(userSshKey);
const isPasswordValidValue =
!!userPassword && checkPasswordValidity(userPassword, isAzure).isValid;
return { isUserNameValidValue, isSshKeyValidValue, isPasswordValidValue };
};
export const checkPasswordValidity = (
password: string,
isAzure: boolean
): PasswordValidationResult => {
if (!password) {
return {
isValid: false,
strength: getStrength(0, 0, false),
validationState: {
ruleLength: 'indeterminate',
ruleCharacters: 'indeterminate',
},
};
}
const isEncrypted = /^\$([^$]+)\$/.test(password);
if (isEncrypted) {
return {
isValid: true,
strength: getStrength(0, 0, false),
validationState: {
ruleLength: 'success',
ruleCharacters: 'success',
},
};
}
const trimmedValue = password.trim();
const isLengthValid = trimmedValue.length >= 6 && trimmedValue.length <= 128;
const { rulesCount, strCount } = countCharacterTypes(password);
const validationState: ValidationState = {
ruleLength: isLengthValid ? 'success' : 'error',
ruleCharacters: rulesCount >= 3 ? 'success' : 'error',
};
return {
isValid: isLengthValid,
strength: getStrength(strCount, rulesCount, isAzure),
validationState: validationState,
};
};
const getStrength = (
strCount: number,
rulesCount: number,
isAzure: boolean
): PasswordValidationResult['strength'] => {
const isValidStrength = isAzure
? strCount >= 6 && rulesCount >= 3
: strCount >= 6;
return isValidStrength
? { variant: 'success', icon: <CheckCircleIcon />, text: 'Strong' }
: { variant: 'default', icon: null, text: '' };
};
const countCharacterTypes = (value: string) => {
const lowercaseCount = (value.match(/[a-z]/g) || []).length;
const uppercaseCount = (value.match(/[A-Z]/g) || []).length;
const digitsCount = (value.match(/\d/g) || []).length;
const specialCount = (value.match(/\W/g) || []).length;
const rulesCount = [
lowercaseCount,
uppercaseCount,
digitsCount,
specialCount,
].filter((count) => count > 0).length;
const strCount = lowercaseCount + uppercaseCount + digitsCount + specialCount;
return { rulesCount, strCount };
};
export function useDetailsValidation(): StepValidation {
const name = useAppSelector(selectBlueprintName);
const description = useAppSelector(selectBlueprintDescription);

View file

@ -19,6 +19,30 @@ const getAzureSourceDropdown = async () => {
return sourceDropdown;
};
export const addTargetEnvAzure = async () => {
const user = userEvent.setup();
expect(
await screen.findByRole('radio', {
name: /use an account configured from sources\./i,
})
).toHaveFocus();
const azureSourceDropdown = await getAzureSourceDropdown();
await waitFor(() => user.click(azureSourceDropdown));
const azureSource = await screen.findByRole('option', {
name: /azureSource1/i,
});
await waitFor(() => user.click(azureSource));
const resourceGroupDropdown = await screen.findByPlaceholderText(
/select resource group/i
);
await waitFor(() => user.click(resourceGroupDropdown));
await waitFor(async () =>
user.click(await screen.findByLabelText('Resource group myResourceGroup1'))
);
await clickNext();
};
const selectAllEnvironments = async () => {
const user = userEvent.setup();
@ -119,28 +143,7 @@ describe('Keyboard accessibility', () => {
await clickNext();
// Target environment azure
expect(
await screen.findByRole('radio', {
name: /use an account configured from sources\./i,
})
).toHaveFocus();
const azureSourceDropdown = await getAzureSourceDropdown();
await waitFor(() => user.click(azureSourceDropdown));
const azureSource = await screen.findByRole('option', {
name: /azureSource1/i,
});
await waitFor(() => user.click(azureSource));
const resourceGroupDropdown = await screen.findByPlaceholderText(
/select resource group/i
);
await waitFor(() => user.click(resourceGroupDropdown));
await waitFor(async () =>
user.click(
await screen.findByLabelText('Resource group myResourceGroup1')
)
);
await clickNext();
await addTargetEnvAzure();
// Registration
await screen.findByText(

View file

@ -5,6 +5,7 @@ import { userEvent } from '@testing-library/user-event';
import { CREATE_BLUEPRINT, EDIT_BLUEPRINT } from '../../../../../constants';
import { mockBlueprintIds } from '../../../../fixtures/blueprints';
import { usersCreateBlueprintRequest } from '../../../../fixtures/editMode';
import { addTargetEnvAzure } from '../../CreateImageWizard.test';
import {
blueprintRequest,
clickBack,
@ -13,7 +14,6 @@ import {
getNextButton,
interceptBlueprintRequest,
interceptEditBlueprintRequest,
openAndDismissSaveAndBuildModal,
renderEditMode,
verifyCancelButton,
} from '../../wizardTestUtils';
@ -26,6 +26,8 @@ import {
let router: RemixRouter | undefined = undefined;
const validUserName = 'best';
const validSshKey = 'ssh-rsa d';
const validPassword = 'validPassword';
const invalidPassword = 'inval';
const goToUsersStep = async () => {
await clickNext();
@ -88,6 +90,13 @@ const addUserName = async (userName: string) => {
await waitFor(() => expect(enterUserName).toHaveValue(userName));
};
const addPassword = async (mockPassword: string) => {
const user = userEvent.setup();
const enterPassword = screen.getByPlaceholderText(/enter password/i);
await waitFor(() => user.type(enterPassword, mockPassword));
await waitFor(() => expect(enterPassword).toHaveValue(mockPassword));
};
describe('Step Users', () => {
beforeEach(async () => {
vi.clearAllMocks();
@ -159,10 +168,51 @@ describe('Step Users', () => {
const invalidUserMessage = screen.getByText(/invalid ssh key/i);
await waitFor(() => expect(invalidUserMessage));
});
test('try to create Azure image with invalid password', async () => {
const user = userEvent.setup();
await renderCreateMode();
await waitFor(() => user.click(screen.getByTestId('upload-azure')));
await clickNext();
await addTargetEnvAzure();
await clickRegisterLater();
await goToUsersStep();
await clickAddUser();
await addUserName(validUserName);
await addPassword(invalidPassword);
const invalidUserMessage = screen.getByText(
/Password must be at least 6 characters long./i
);
const warningUserMessage = screen.getByText(
/Must include at least 3 of the following: lowercase letters, uppercase letters, numbers, symbols./i
);
await waitFor(() => expect(invalidUserMessage));
await waitFor(() => expect(warningUserMessage));
const nextButton = await getNextButton();
await waitFor(() => expect(nextButton).toBeDisabled());
});
test('with invalid password', async () => {
await renderCreateMode();
await goToRegistrationStep();
await clickRegisterLater();
await goToUsersStep();
await clickAddUser();
await addUserName(validUserName);
await addPassword(invalidPassword);
const invalidUserMessage = screen.getByText(
/Password must be at least 6 characters long./i
);
await waitFor(() => expect(invalidUserMessage));
const nextButton = await getNextButton();
await waitFor(() => expect(nextButton).toBeDisabled());
});
});
describe('User request generated correctly', () => {
test('with valid name, ssh key and checked Administrator checkbox', async () => {
test('create image with valid name, password, ssh key and checked Administrator checkbox', async () => {
const user = userEvent.setup();
await renderCreateMode();
await goToRegistrationStep();
@ -171,16 +221,14 @@ describe('User request generated correctly', () => {
await clickAddUser();
await addUserName(validUserName);
await addSshKey(validSshKey);
await addPassword(validPassword);
const nextButton = await getNextButton();
await waitFor(() => expect(nextButton).toBeEnabled());
const isAdmin = screen.getByRole('checkbox', {
name: /administrator/i,
});
user.click(isAdmin);
await waitFor(() => user.click(isAdmin));
await goToReviewStep();
// informational modal pops up in the first test only as it's tied
// to a 'imageBuilder.saveAndBuildModalSeen' variable in localStorage
await openAndDismissSaveAndBuildModal();
const receivedRequest = await interceptBlueprintRequest(CREATE_BLUEPRINT);
const expectedRequest = {
...blueprintRequest,
@ -189,6 +237,7 @@ describe('User request generated correctly', () => {
{
name: 'best',
ssh_key: 'ssh-rsa d',
password: validPassword,
groups: ['wheel'],
},
],
@ -234,7 +283,6 @@ describe('Users edit mode', () => {
const receivedRequest = await interceptEditBlueprintRequest(
`${EDIT_BLUEPRINT}/${id}`
);
const expectedRequest = usersCreateBlueprintRequest;
expect(receivedRequest).toEqual(expectedRequest);
expect(receivedRequest).toEqual(usersCreateBlueprintRequest);
});
});