From 95107cf9f81855daace01e684266a02b4e2d5d33 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 20 Oct 2023 10:41:29 +0200 Subject: [PATCH 01/19] webui: use dynamic rules in password form component --- .../src/components/storage/DiskEncryption.jsx | 149 ++++++++++-------- ui/webui/test/check-storage | 10 +- 2 files changed, 88 insertions(+), 71 deletions(-) diff --git a/ui/webui/src/components/storage/DiskEncryption.jsx b/ui/webui/src/components/storage/DiskEncryption.jsx index 04ad47f0f50..2b5ee948e2b 100644 --- a/ui/webui/src/components/storage/DiskEncryption.jsx +++ b/ui/webui/src/components/storage/DiskEncryption.jsx @@ -91,6 +91,34 @@ const getStrengthLevels = (minQualility, isStrict) => { return levels; }; +const rules = [ + { + id: "length", + text: (policy) => cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v), + check: (policy, password) => password.length >= policy["min-length"].v, + isError: true, + }, + { + id: "ascii", + text: (policy) => _("The passphrase you have provided contains non-ASCII characters. You may not be able to switch between keyboard layouts when typing it."), + check: (policy, password) => password.length > 0 && /^[\x20-\x7F]*$/.test(password), + isError: false, + }, +]; + +const getRuleResults = (rules, policy, password) => { + return rules.map(rule => { + return { + id: rule.id, + text: rule.text(policy, password), + isSatisfied: password.length > 0 ? rule.check(policy, password) : null, + isError: rule.isError + }; + }); +}; + +const rulesSatisfied = ruleResults => ruleResults.every(r => r.isSatisfied || !r.isError); + export function getStorageEncryptionState (password = "", confirmPassword = "", encrypt = false) { return { password, confirmPassword, encrypt }; } @@ -112,36 +140,68 @@ const passwordStrengthLabel = (idPrefix, strength, strengthLevels) => { const PasswordFormFields = ({ idPrefix, policy, - password, + initialPassword, passwordLabel, onChange, - passwordConfirm, - passwordConfirmLabel, + initialConfirmPassword, + confirmPasswordLabel, onConfirmChange, passwordStrength, - ruleLength, - ruleConfirmMatches, - ruleAscii, + rules, strengthLevels, }) => { const [passwordHidden, setPasswordHidden] = useState(true); const [confirmHidden, setConfirmHidden] = useState(true); - const [_password, _setPassword] = useState(password); - const [_passwordConfirm, _setPasswordConfirm] = useState(passwordConfirm); + const [_password, _setPassword] = useState(initialPassword); + const [_confirmPassword, _setConfirmPassword] = useState(initialConfirmPassword); + const [password, setPassword] = useState(initialPassword); + const [confirmPassword, setConfirmPassword] = useState(initialConfirmPassword); useEffect(() => { - debounce(300, () => onChange(_password))(); + debounce(300, () => { setPassword(_password); onChange(_password) })(); }, [_password, onChange]); useEffect(() => { - debounce(300, () => onConfirmChange(_passwordConfirm))(); - }, [_passwordConfirm, onConfirmChange]); + debounce(300, () => { setConfirmPassword(_confirmPassword); onConfirmChange(_confirmPassword) })(); + }, [_confirmPassword, onConfirmChange]); + + const ruleResults = useMemo(() => { + return getRuleResults(rules, policy, password); + }, [policy, password, rules]); + + const ruleConfirmMatches = useMemo(() => { + return password.length > 0 ? password === confirmPassword : null; + }, [password, confirmPassword]); + + const ruleHelperItems = ruleResults.map(rule => { + console.log(rule); + let variant = rule.isSatisfied === null ? "indeterminate" : rule.isSatisfied ? "success" : "error"; + if (!rule.isError) { + if (rule.isSatisfied || rule.isSatisfied === null) { + return null; + } + variant = "warning"; + } + return ( + + {rule.text} + + ); + }); + + const ruleConfirmVariant = ruleConfirmMatches === null ? "indeterminate" : ruleConfirmMatches ? "success" : "error"; return ( <> @@ -164,34 +224,18 @@ const PasswordFormFields = ({ - - {cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v)} - - {ruleAscii && - - {_("The passphrase you have provided contains non-ASCII characters. You may not be able to switch between keyboard layouts when typing it.")} - } + {ruleHelperItems} _setPasswordConfirm(val)} + value={_confirmPassword} + onChange={(_event, val) => _setConfirmPassword(val)} id={idPrefix + "-password-confirm-field"} /> @@ -210,7 +254,7 @@ const PasswordFormFields = ({ {_("Passphrases must match")} @@ -236,18 +280,6 @@ const isValidStrength = (strength, strengthLevels) => { return level ? level.valid : false; }; -const getRuleLength = (password, minLength) => { - let ruleState = "indeterminate"; - if (password.length > 0 && password.length < minLength) { - ruleState = "error"; - } else if (password.length >= minLength) { - ruleState = "success"; - } - return ruleState; -}; - -const getRuleConfirmMatches = (password, confirm) => (password.length > 0 ? (password === confirm ? "success" : "error") : "indeterminate"); - const CheckDisksSpinner = ( {_("Checking storage configuration")}} icon={} headingLevel="h4" /> @@ -275,18 +307,6 @@ export const DiskEncryption = ({ const isEncrypted = storageEncryption.encrypt; const luksPolicy = passwordPolicies.luks; - const ruleConfirmMatches = useMemo(() => { - return getRuleConfirmMatches(password, confirmPassword); - }, [password, confirmPassword]); - - const ruleLength = useMemo(() => { - return luksPolicy && getRuleLength(password, luksPolicy["min-length"].v); - }, [password, luksPolicy]); - - const ruleAscii = useMemo(() => { - return password.length > 0 && !/^[\x20-\x7F]*$/.test(password); - }, [password]); - const strengthLevels = useMemo(() => { return luksPolicy && getStrengthLevels(luksPolicy["min-quality"].v, luksPolicy["is-strict"].v); }, [luksPolicy]); @@ -305,14 +325,12 @@ export const DiskEncryption = ({ { const updateValidity = (isEncrypted) => { const passphraseValid = ( - ruleLength === "success" && - ruleConfirmMatches === "success" && isValidStrength(passwordStrength, strengthLevels) ); setIsFormValid(!isEncrypted || passphraseValid); }; updateValidity(isEncrypted); - }, [setIsFormValid, isEncrypted, ruleConfirmMatches, ruleLength, passwordStrength, strengthLevels]); + }, [setIsFormValid, isEncrypted, passwordStrength, strengthLevels]); useEffect(() => { setStorageEncryption(se => ({ ...se, password })); diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 7f110632b78..3e4b25db145 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -165,7 +165,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): ) # No password set - s.check_pw_rule("min-chars", "indeterminate") + s.check_pw_rule("length", "indeterminate") s.check_pw_rule("match", "indeterminate") i.check_next_disabled() @@ -173,21 +173,21 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): s.set_password("abcd") s.check_pw_strength(None) i.check_next_disabled() - s.check_pw_rule("min-chars", "error") + s.check_pw_rule("length", "error") s.check_pw_rule("match", "error") # Make the pw 8 chars long s.set_password("efgh", append=True, value_check=False) i.check_next_disabled() s.check_password("abcdefgh") - s.check_pw_rule("min-chars", "success") + s.check_pw_rule("length", "success") s.check_pw_rule("match", "error") s.check_pw_strength("weak") # Non-ASCII password s.set_password(8 * "š") s.check_password(8 * "š") - s.check_pw_rule("min-chars", "success") + s.check_pw_rule("length", "success") s.check_pw_rule("match", "error") s.check_pw_rule("ascii", "warning") s.check_pw_strength("weak") @@ -201,7 +201,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): s.check_pw_rule("match", "error") s.set_password_confirm("abcdefgh") s.check_pw_rule("match", "success") - s.check_pw_rule("min-chars", "success") + s.check_pw_rule("length", "success") s.check_pw_strength("weak") s.check_password("abcdefgh") s.check_password_confirm("abcdefgh") From 5f40775209ea9a3cf399694ee7ed6c7e466fbdac Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 23 Oct 2023 12:38:57 +0200 Subject: [PATCH 02/19] webui: move also password strength logic into pw form component --- .../src/components/storage/DiskEncryption.jsx | 57 ++++++++----------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/ui/webui/src/components/storage/DiskEncryption.jsx b/ui/webui/src/components/storage/DiskEncryption.jsx index 2b5ee948e2b..4a30ffd7908 100644 --- a/ui/webui/src/components/storage/DiskEncryption.jsx +++ b/ui/webui/src/components/storage/DiskEncryption.jsx @@ -136,7 +136,6 @@ const passwordStrengthLabel = (idPrefix, strength, strengthLevels) => { } }; -// TODO create strengthLevels object with methods passed to the component ? const PasswordFormFields = ({ idPrefix, policy, @@ -146,9 +145,8 @@ const PasswordFormFields = ({ initialConfirmPassword, confirmPasswordLabel, onConfirmChange, - passwordStrength, rules, - strengthLevels, + setIsValid, }) => { const [passwordHidden, setPasswordHidden] = useState(true); const [confirmHidden, setConfirmHidden] = useState(true); @@ -156,6 +154,7 @@ const PasswordFormFields = ({ const [_confirmPassword, _setConfirmPassword] = useState(initialConfirmPassword); const [password, setPassword] = useState(initialPassword); const [confirmPassword, setConfirmPassword] = useState(initialConfirmPassword); + const [passwordStrength, setPasswordStrength] = useState(""); useEffect(() => { debounce(300, () => { setPassword(_password); onChange(_password) })(); @@ -197,6 +196,26 @@ const PasswordFormFields = ({ const ruleConfirmVariant = ruleConfirmMatches === null ? "indeterminate" : ruleConfirmMatches ? "success" : "error"; + const strengthLevels = useMemo(() => { + return policy && getStrengthLevels(policy["min-quality"].v, policy["is-strict"].v); + }, [policy]); + + useEffect(() => { + const updatePasswordStrength = async () => { + const _passwordStrength = await getPasswordStrength(password, strengthLevels); + setPasswordStrength(_passwordStrength); + }; + updatePasswordStrength(); + }, [password, strengthLevels]); + + useEffect(() => { + setIsValid( + rulesSatisfied(ruleResults) && + ruleConfirmMatches && + isValidStrength(passwordStrength, strengthLevels) + ); + }, [setIsValid, ruleResults, ruleConfirmMatches, passwordStrength, strengthLevels]); + return ( <> { const [password, setPassword] = useState(storageEncryption.password); const [confirmPassword, setConfirmPassword] = useState(storageEncryption.confirmPassword); - const [passwordStrength, setPasswordStrength] = useState(""); const isEncrypted = storageEncryption.encrypt; const luksPolicy = passwordPolicies.luks; - const strengthLevels = useMemo(() => { - return luksPolicy && getStrengthLevels(luksPolicy["min-quality"].v, luksPolicy["is-strict"].v); - }, [luksPolicy]); - const encryptedDevicesCheckbox = content => ( ); useEffect(() => { - if (!strengthLevels) { - return; - } - - const updatePasswordStrength = async () => { - const _passwordStrength = await getPasswordStrength(password, strengthLevels); - setPasswordStrength(_passwordStrength); - }; - updatePasswordStrength(); - }, [password, strengthLevels]); - - // TODO: use the state set from the child - useEffect(() => { - const updateValidity = (isEncrypted) => { - const passphraseValid = ( - isValidStrength(passwordStrength, strengthLevels) - ); - setIsFormValid(!isEncrypted || passphraseValid); - }; - - updateValidity(isEncrypted); - }, [setIsFormValid, isEncrypted, passwordStrength, strengthLevels]); + setIsFormValid(!isEncrypted); + }, [setIsFormValid, isEncrypted]); useEffect(() => { setStorageEncryption(se => ({ ...se, password })); From 17901f1738de848359cb7388351d6ec92242ac99 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 23 Oct 2023 12:53:01 +0200 Subject: [PATCH 03/19] webui: move pasword form component into a separate file --- ui/webui/src/components/Password.jsx | 280 ++++++++++++++++++ .../src/components/storage/DiskEncryption.jsx | 255 +--------------- 2 files changed, 286 insertions(+), 249 deletions(-) create mode 100644 ui/webui/src/components/Password.jsx diff --git a/ui/webui/src/components/Password.jsx b/ui/webui/src/components/Password.jsx new file mode 100644 index 00000000000..e8690e5a8e6 --- /dev/null +++ b/ui/webui/src/components/Password.jsx @@ -0,0 +1,280 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with This program; If not, see . + */ + +import cockpit from "cockpit"; +import React, { useState, useEffect, useMemo } from "react"; +import { debounce } from "throttle-debounce"; + +import { + Button, + FormGroup, + FormHelperText, + HelperText, + HelperTextItem, + InputGroup, + InputGroupItem, + TextInput, +} from "@patternfly/react-core"; + +// eslint-disable-next-line camelcase +import { password_quality } from "cockpit-components-password.jsx"; +import { + ExclamationCircleIcon, + ExclamationTriangleIcon, + CheckCircleIcon, + EyeIcon, + EyeSlashIcon +} from "@patternfly/react-icons"; + +const _ = cockpit.gettext; + +export const ruleLength = { + id: "length", + text: (policy) => cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v), + check: (policy, password) => password.length >= policy["min-length"].v, + isError: true, +}; + +/* Calculate the password quality levels based on the password policy + * If the policy specifies a 'is-strict' rule anything bellow the minimum specified by the policy + * is considered invalid + * @param {int} minQualility - the minimum quality level + * @return {array} - the password strengh levels + */ +const getStrengthLevels = (minQualility, isStrict) => { + const levels = [{ + id: "weak", + label: _("Weak"), + variant: "error", + icon: , + lower_bound: 0, + higher_bound: minQualility - 1, + valid: !isStrict, + }]; + + if (minQualility <= 69) { + levels.push({ + id: "medium", + label: _("Medium"), + variant: "warning", + icon: , + lower_bound: minQualility, + higher_bound: 69, + valid: true, + }); + } + + levels.push({ + id: "strong", + label: _("Strong"), + variant: "success", + icon: , + lower_bound: Math.max(70, minQualility), + higher_bound: 100, + valid: true, + }); + + return levels; +}; + +const getRuleResults = (rules, policy, password) => { + return rules.map(rule => { + return { + id: rule.id, + text: rule.text(policy, password), + isSatisfied: password.length > 0 ? rule.check(policy, password) : null, + isError: rule.isError + }; + }); +}; + +const rulesSatisfied = ruleResults => ruleResults.every(r => r.isSatisfied || !r.isError); + +const passwordStrengthLabel = (idPrefix, strength, strengthLevels) => { + const level = strengthLevels.filter(l => l.id === strength)[0]; + if (level) { + return ( + + + {level.label} + + + ); + } +}; + +export const PasswordFormFields = ({ + idPrefix, + policy, + initialPassword, + passwordLabel, + onChange, + initialConfirmPassword, + confirmPasswordLabel, + onConfirmChange, + rules, + setIsValid, +}) => { + const [passwordHidden, setPasswordHidden] = useState(true); + const [confirmHidden, setConfirmHidden] = useState(true); + const [_password, _setPassword] = useState(initialPassword); + const [_confirmPassword, _setConfirmPassword] = useState(initialConfirmPassword); + const [password, setPassword] = useState(initialPassword); + const [confirmPassword, setConfirmPassword] = useState(initialConfirmPassword); + const [passwordStrength, setPasswordStrength] = useState(""); + + useEffect(() => { + debounce(300, () => { setPassword(_password); onChange(_password) })(); + }, [_password, onChange]); + + useEffect(() => { + debounce(300, () => { setConfirmPassword(_confirmPassword); onConfirmChange(_confirmPassword) })(); + }, [_confirmPassword, onConfirmChange]); + + const ruleResults = useMemo(() => { + return getRuleResults(rules, policy, password); + }, [policy, password, rules]); + + const ruleConfirmMatches = useMemo(() => { + return password.length > 0 ? password === confirmPassword : null; + }, [password, confirmPassword]); + + const ruleHelperItems = ruleResults.map(rule => { + let variant = rule.isSatisfied === null ? "indeterminate" : rule.isSatisfied ? "success" : "error"; + if (!rule.isError) { + if (rule.isSatisfied || rule.isSatisfied === null) { + return null; + } + variant = "warning"; + } + return ( + + {rule.text} + + ); + }); + + const ruleConfirmVariant = ruleConfirmMatches === null ? "indeterminate" : ruleConfirmMatches ? "success" : "error"; + + const strengthLevels = useMemo(() => { + return policy && getStrengthLevels(policy["min-quality"].v, policy["is-strict"].v); + }, [policy]); + + useEffect(() => { + const updatePasswordStrength = async () => { + const _passwordStrength = await getPasswordStrength(password, strengthLevels); + setPasswordStrength(_passwordStrength); + }; + updatePasswordStrength(); + }, [password, strengthLevels]); + + useEffect(() => { + setIsValid( + rulesSatisfied(ruleResults) && + ruleConfirmMatches && + isValidStrength(passwordStrength, strengthLevels) + ); + }, [setIsValid, ruleResults, ruleConfirmMatches, passwordStrength, strengthLevels]); + + return ( + <> + + + + _setPassword(val)} + id={idPrefix + "-password-field"} + /> + + + + + + + + {ruleHelperItems} + + + + + + _setConfirmPassword(val)} + id={idPrefix + "-password-confirm-field"} + /> + + + + + + + + + {_("Passphrases must match")} + + + + + + ); +}; + +const getPasswordStrength = async (password, strengthLevels) => { + // In case of unacceptable password just return 0 + const force = true; + const quality = await password_quality(password, force); + const level = strengthLevels.filter(l => l.lower_bound <= quality.value && l.higher_bound >= quality.value)[0]; + return level.id; +}; + +const isValidStrength = (strength, strengthLevels) => { + const level = strengthLevels.filter(l => l.id === strength)[0]; + + return level ? level.valid : false; +}; diff --git a/ui/webui/src/components/storage/DiskEncryption.jsx b/ui/webui/src/components/storage/DiskEncryption.jsx index 4a30ffd7908..b9e094fe4a1 100644 --- a/ui/webui/src/components/storage/DiskEncryption.jsx +++ b/ui/webui/src/components/storage/DiskEncryption.jsx @@ -16,80 +16,26 @@ */ import cockpit from "cockpit"; -import React, { useState, useEffect, useMemo } from "react"; -import { debounce } from "throttle-debounce"; +import React, { useState, useEffect } from "react"; import { - Button, Checkbox, EmptyState, + EmptyStateHeader, EmptyStateIcon, + EmptyStateFooter, Form, - FormGroup, - FormHelperText, - HelperText, - HelperTextItem, - InputGroup, Spinner, - TextInput, TextContent, TextVariants, - Text, EmptyStateHeader, EmptyStateFooter, InputGroupItem, + Text, } from "@patternfly/react-core"; -// eslint-disable-next-line camelcase -import { password_quality } from "cockpit-components-password.jsx"; -import EyeIcon from "@patternfly/react-icons/dist/esm/icons/eye-icon"; -import EyeSlashIcon from "@patternfly/react-icons/dist/esm/icons/eye-slash-icon"; -import ExclamationCircleIcon from "@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon"; -import ExclamationTriangleIcon from "@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon"; -import CheckCircleIcon from "@patternfly/react-icons/dist/esm/icons/check-circle-icon"; - import "./DiskEncryption.scss"; -const _ = cockpit.gettext; - -/* Calculate the password quality levels based on the password policy - * If the policy specifies a 'is-strict' rule anything bellow the minimum specified by the policy - * is considered invalid - * @param {int} minQualility - the minimum quality level - * @return {array} - the password strengh levels - */ -const getStrengthLevels = (minQualility, isStrict) => { - const levels = [{ - id: "weak", - label: _("Weak"), - variant: "error", - icon: , - lower_bound: 0, - higher_bound: minQualility - 1, - valid: !isStrict, - }]; - - if (minQualility <= 69) { - levels.push({ - id: "medium", - label: _("Medium"), - variant: "warning", - icon: , - lower_bound: minQualility, - higher_bound: 69, - valid: true, - }); - } - - levels.push({ - id: "strong", - label: _("Strong"), - variant: "success", - icon: , - lower_bound: Math.max(70, minQualility), - higher_bound: 100, - valid: true, - }); +import { PasswordFormFields } from "../Password.jsx"; - return levels; -}; +const _ = cockpit.gettext; const rules = [ { @@ -106,199 +52,10 @@ const rules = [ }, ]; -const getRuleResults = (rules, policy, password) => { - return rules.map(rule => { - return { - id: rule.id, - text: rule.text(policy, password), - isSatisfied: password.length > 0 ? rule.check(policy, password) : null, - isError: rule.isError - }; - }); -}; - -const rulesSatisfied = ruleResults => ruleResults.every(r => r.isSatisfied || !r.isError); - export function getStorageEncryptionState (password = "", confirmPassword = "", encrypt = false) { return { password, confirmPassword, encrypt }; } -const passwordStrengthLabel = (idPrefix, strength, strengthLevels) => { - const level = strengthLevels.filter(l => l.id === strength)[0]; - if (level) { - return ( - - - {level.label} - - - ); - } -}; - -const PasswordFormFields = ({ - idPrefix, - policy, - initialPassword, - passwordLabel, - onChange, - initialConfirmPassword, - confirmPasswordLabel, - onConfirmChange, - rules, - setIsValid, -}) => { - const [passwordHidden, setPasswordHidden] = useState(true); - const [confirmHidden, setConfirmHidden] = useState(true); - const [_password, _setPassword] = useState(initialPassword); - const [_confirmPassword, _setConfirmPassword] = useState(initialConfirmPassword); - const [password, setPassword] = useState(initialPassword); - const [confirmPassword, setConfirmPassword] = useState(initialConfirmPassword); - const [passwordStrength, setPasswordStrength] = useState(""); - - useEffect(() => { - debounce(300, () => { setPassword(_password); onChange(_password) })(); - }, [_password, onChange]); - - useEffect(() => { - debounce(300, () => { setConfirmPassword(_confirmPassword); onConfirmChange(_confirmPassword) })(); - }, [_confirmPassword, onConfirmChange]); - - const ruleResults = useMemo(() => { - return getRuleResults(rules, policy, password); - }, [policy, password, rules]); - - const ruleConfirmMatches = useMemo(() => { - return password.length > 0 ? password === confirmPassword : null; - }, [password, confirmPassword]); - - const ruleHelperItems = ruleResults.map(rule => { - console.log(rule); - let variant = rule.isSatisfied === null ? "indeterminate" : rule.isSatisfied ? "success" : "error"; - if (!rule.isError) { - if (rule.isSatisfied || rule.isSatisfied === null) { - return null; - } - variant = "warning"; - } - return ( - - {rule.text} - - ); - }); - - const ruleConfirmVariant = ruleConfirmMatches === null ? "indeterminate" : ruleConfirmMatches ? "success" : "error"; - - const strengthLevels = useMemo(() => { - return policy && getStrengthLevels(policy["min-quality"].v, policy["is-strict"].v); - }, [policy]); - - useEffect(() => { - const updatePasswordStrength = async () => { - const _passwordStrength = await getPasswordStrength(password, strengthLevels); - setPasswordStrength(_passwordStrength); - }; - updatePasswordStrength(); - }, [password, strengthLevels]); - - useEffect(() => { - setIsValid( - rulesSatisfied(ruleResults) && - ruleConfirmMatches && - isValidStrength(passwordStrength, strengthLevels) - ); - }, [setIsValid, ruleResults, ruleConfirmMatches, passwordStrength, strengthLevels]); - - return ( - <> - - - - _setPassword(val)} - id={idPrefix + "-password-field"} - /> - - - - - - - - {ruleHelperItems} - - - - - - _setConfirmPassword(val)} - id={idPrefix + "-password-confirm-field"} - /> - - - - - - - - - {_("Passphrases must match")} - - - - - - ); -}; - -const getPasswordStrength = async (password, strengthLevels) => { - // In case of unacceptable password just return 0 - const force = true; - const quality = await password_quality(password, force); - const level = strengthLevels.filter(l => l.lower_bound <= quality.value && l.higher_bound >= quality.value)[0]; - return level.id; -}; - -const isValidStrength = (strength, strengthLevels) => { - const level = strengthLevels.filter(l => l.id === strength)[0]; - - return level ? level.valid : false; -}; - const CheckDisksSpinner = ( {_("Checking storage configuration")}} icon={} headingLevel="h4" /> From d7b83f7d55bc57ecf71a1cc763e4f0e5ab619c12 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 16 Oct 2023 17:14:28 +0200 Subject: [PATCH 04/19] webui: add a simple Create Account screen --- ui/webui/src/components/AnacondaWizard.jsx | 6 + ui/webui/src/components/users/Accounts.jsx | 119 ++++++++++++++++++++ ui/webui/src/components/users/Accounts.scss | 4 + ui/webui/test/check-storage | 6 +- ui/webui/test/helpers/installer.py | 6 +- 5 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 ui/webui/src/components/users/Accounts.jsx create mode 100644 ui/webui/src/components/users/Accounts.scss diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index eb6beff0d8a..32d03fd3d16 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -39,6 +39,7 @@ import { getDefaultScenario } from "./storage/InstallationScenario.jsx"; import { MountPointMapping, getPageProps as getMountPointMappingProps } from "./storage/MountPointMapping.jsx"; import { DiskEncryption, getStorageEncryptionState, getPageProps as getDiskEncryptionProps } from "./storage/DiskEncryption.jsx"; import { InstallationLanguage, getPageProps as getInstallationLanguageProps } from "./localization/InstallationLanguage.jsx"; +import { Accounts, getPageProps as getAccountsProps } from "./users/Accounts.jsx"; import { InstallationProgress } from "./installation/InstallationProgress.jsx"; import { ReviewConfiguration, ReviewConfigurationConfirmModal, getPageProps as getReviewConfigurationProps } from "./review/ReviewConfiguration.jsx"; import { exitGui } from "../helpers/exit.js"; @@ -143,6 +144,11 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim ...getDiskEncryptionProps({ storageScenarioId }) }] }, + { + component: Accounts, + data: {}, + ...getAccountsProps() + }, { component: ReviewConfiguration, data: { diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx new file mode 100644 index 00000000000..a52b43c0981 --- /dev/null +++ b/ui/webui/src/components/users/Accounts.jsx @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with This program; If not, see . + */ + +import cockpit from "cockpit"; +import React, { useState, useEffect } from "react"; + +import { + Form, + FormGroup, + TextInput, + Title, +} from "@patternfly/react-core"; + +import "./Accounts.scss"; + +const _ = cockpit.gettext; + +const CreateAccount = ({ + idPrefix, +}) => { + const [fullName, setFullName] = useState(); + const [userAccount, setUserAccount] = useState(); + const [password, setPassword] = useState(); + const [confirmPassword, setConfirmPassword] = useState(); + + return ( +
+ + {_("Create account")} + + {_("This account will have administration priviledge with sudo.")} + + setFullName(val)} + /> + + + setUserAccount(val)} + /> + + + setPassword} + /> + + + setConfirmPassword} + /> + +
+ ); +}; + +export const Accounts = ({ + idPrefix, + setIsFormValid, +}) => { + useEffect(() => { + setIsFormValid(true); + }, [setIsFormValid]); + + return ( + <> + + + ); +}; + +export const getPageProps = () => { + return ({ + id: "accounts", + label: _("Create Account"), + title: null, + }); +}; diff --git a/ui/webui/src/components/users/Accounts.scss b/ui/webui/src/components/users/Accounts.scss new file mode 100644 index 00000000000..f53ec05624c --- /dev/null +++ b/ui/webui/src/components/users/Accounts.scss @@ -0,0 +1,4 @@ +// Span disk encryption password fields to take slightly more width than the default +#accounts-create-account { + width: min(60ch, 100%); +} diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 3e4b25db145..3159a094f50 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -295,10 +295,11 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): for _ in range(10): s.dbus_create_partitioning("AUTOMATIC") - # Go back to the previous page and re-enter the review screen. + # Go back to the Disk Configuration page and re-enter the review screen. # This should create again a new partitioning object and apply it # no matter how many partitioning objects were created before i.back() + i.back() i.reach(i.steps.REVIEW) new_applied_partitioning = s.dbus_get_applied_partitioning() new_created_partitioning = s.dbus_get_created_partitioning() @@ -422,6 +423,7 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) applied_partitioning = s.dbus_get_applied_partitioning() # When adding a new partition a new partitioning should be created + i.back() i.back(previous_page=i.steps.CUSTOM_MOUNT_POINT) i.back() @@ -788,6 +790,7 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) r.check_disk_row(dev, 3, "home, 15.0 GB: mount, /home") r.check_disk_row_not_present(dev, f"unused") + i.back() i.back(previous_page=i.steps.CUSTOM_MOUNT_POINT) i.back() @@ -950,6 +953,7 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) r.check_disk_row(disk, 3, f"{vgname}-home, 8.12 GB: mount, /home") r.check_disk_row(disk, 4, f"{vgname}-swap, 902 MB: mount, swap") + i.back() i.back(previous_page=i.steps.CUSTOM_MOUNT_POINT) # remove the /home row and check that row 3 is now swap diff --git a/ui/webui/test/helpers/installer.py b/ui/webui/test/helpers/installer.py index 9867f21c230..80163150bb9 100644 --- a/ui/webui/test/helpers/installer.py +++ b/ui/webui/test/helpers/installer.py @@ -25,14 +25,16 @@ class InstallerSteps(UserDict): CUSTOM_MOUNT_POINT = "mount-point-mapping" DISK_CONFIGURATION = "disk-configuration" DISK_ENCRYPTION = "disk-encryption" + ACCOUNTS = "accounts" REVIEW = "installation-review" PROGRESS = "installation-progress" _steps_jump = {} _steps_jump[WELCOME] = INSTALLATION_METHOD _steps_jump[INSTALLATION_METHOD] = [DISK_ENCRYPTION, CUSTOM_MOUNT_POINT] - _steps_jump[DISK_ENCRYPTION] = REVIEW - _steps_jump[CUSTOM_MOUNT_POINT] = REVIEW + _steps_jump[DISK_ENCRYPTION] = ACCOUNTS + _steps_jump[CUSTOM_MOUNT_POINT] = ACCOUNTS + _steps_jump[ACCOUNTS] = REVIEW _steps_jump[REVIEW] = PROGRESS _steps_jump[PROGRESS] = [] From 69d9fdc64b800fd873555f04b1cb7ed8941e668b Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 23 Oct 2023 13:25:29 +0200 Subject: [PATCH 05/19] webui: use password form component for Create Account screen --- ui/webui/src/components/AnacondaWizard.jsx | 4 +- ui/webui/src/components/users/Accounts.jsx | 66 ++++++++++++++-------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index 32d03fd3d16..4657d9a853a 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -146,7 +146,9 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim }, { component: Accounts, - data: {}, + data: { + passwordPolicies: runtimeData.passwordPolicies, + }, ...getAccountsProps() }, { diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index a52b43c0981..b8396463839 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -27,15 +27,48 @@ import { import "./Accounts.scss"; +import { PasswordFormFields } from "../Password.jsx"; + const _ = cockpit.gettext; +const rules = [ + { + id: "length", + text: (policy) => cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v), + check: (policy, password) => password.length >= policy["min-length"].v, + isWarning: false, + }, +]; + const CreateAccount = ({ idPrefix, + passwordPolicy, + setIsUserValid, }) => { const [fullName, setFullName] = useState(); const [userAccount, setUserAccount] = useState(); - const [password, setPassword] = useState(); - const [confirmPassword, setConfirmPassword] = useState(); + const [password, setPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [isPasswordValid, setIsPasswordValid] = useState(false); + + useEffect(() => { + setIsUserValid(isPasswordValid); + }, [setIsUserValid, isPasswordValid]); + + const passphraseForm = ( + + ); return (
setUserAccount(val)} /> - - setPassword} - /> - - - setConfirmPassword} - /> - + {passphraseForm}
); }; @@ -96,15 +110,19 @@ const CreateAccount = ({ export const Accounts = ({ idPrefix, setIsFormValid, + passwordPolicies, }) => { + const [isUserValid, setIsUserValid] = useState(); useEffect(() => { - setIsFormValid(true); - }, [setIsFormValid]); + setIsFormValid(isUserValid); + }, [setIsFormValid, isUserValid]); return ( <> ); From b01f27b13ae85507536783b1760b30b27a0cccc4 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 23 Oct 2023 14:44:53 +0200 Subject: [PATCH 06/19] webui: share length password rule between users and disk encryption --- .../src/components/storage/DiskEncryption.jsx | 24 +++++++------------ ui/webui/src/components/users/Accounts.jsx | 13 ++-------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/ui/webui/src/components/storage/DiskEncryption.jsx b/ui/webui/src/components/storage/DiskEncryption.jsx index b9e094fe4a1..786427c714a 100644 --- a/ui/webui/src/components/storage/DiskEncryption.jsx +++ b/ui/webui/src/components/storage/DiskEncryption.jsx @@ -33,24 +33,16 @@ import { import "./DiskEncryption.scss"; -import { PasswordFormFields } from "../Password.jsx"; +import { PasswordFormFields, ruleLength } from "../Password.jsx"; const _ = cockpit.gettext; -const rules = [ - { - id: "length", - text: (policy) => cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v), - check: (policy, password) => password.length >= policy["min-length"].v, - isError: true, - }, - { - id: "ascii", - text: (policy) => _("The passphrase you have provided contains non-ASCII characters. You may not be able to switch between keyboard layouts when typing it."), - check: (policy, password) => password.length > 0 && /^[\x20-\x7F]*$/.test(password), - isError: false, - }, -]; +const ruleAscii = { + id: "ascii", + text: (policy) => _("The passphrase you have provided contains non-ASCII characters. You may not be able to switch between keyboard layouts when typing it."), + check: (policy, password) => password.length > 0 && /^[\x20-\x7F]*$/.test(password), + isError: false, +}; export function getStorageEncryptionState (password = "", confirmPassword = "", encrypt = false) { return { password, confirmPassword, encrypt }; @@ -100,7 +92,7 @@ export const DiskEncryption = ({ passwordLabel={_("Passphrase")} initialConfirmPassword={confirmPassword} confirmPasswordLabel={_("Confirm passphrase")} - rules={rules} + rules={[ruleLength, ruleAscii]} onChange={setPassword} onConfirmChange={setConfirmPassword} setIsValid={setIsFormValid} diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index b8396463839..4a1707cda69 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -27,19 +27,10 @@ import { import "./Accounts.scss"; -import { PasswordFormFields } from "../Password.jsx"; +import { PasswordFormFields, ruleLength } from "../Password.jsx"; const _ = cockpit.gettext; -const rules = [ - { - id: "length", - text: (policy) => cockpit.format(_("Must be at least $0 characters"), policy["min-length"].v), - check: (policy, password) => password.length >= policy["min-length"].v, - isWarning: false, - }, -]; - const CreateAccount = ({ idPrefix, passwordPolicy, @@ -63,7 +54,7 @@ const CreateAccount = ({ passwordLabel={_("Passphrase")} initialConfirmPassword={confirmPassword} confirmPasswordLabel={_("Confirm passphrase")} - rules={rules} + rules={[ruleLength]} onChange={setPassword} onConfirmChange={setConfirmPassword} setIsValid={setIsPasswordValid} From f0453c1e3eca6f2184aefd19f30e63db9caa9968 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 24 Oct 2023 08:57:13 +0200 Subject: [PATCH 07/19] webui: add simplest user name check to Create Accounts --- ui/webui/src/components/users/Accounts.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 4a1707cda69..09ddb51f6f3 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -36,15 +36,15 @@ const CreateAccount = ({ passwordPolicy, setIsUserValid, }) => { - const [fullName, setFullName] = useState(); - const [userAccount, setUserAccount] = useState(); + const [fullName, setFullName] = useState(""); + const [userAccount, setUserAccount] = useState(""); const [password, setPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); const [isPasswordValid, setIsPasswordValid] = useState(false); useEffect(() => { - setIsUserValid(isPasswordValid); - }, [setIsUserValid, isPasswordValid]); + setIsUserValid(isPasswordValid && userAccount.length > 0); + }, [setIsUserValid, isPasswordValid, userAccount]); const passphraseForm = ( Date: Tue, 24 Oct 2023 11:45:48 +0200 Subject: [PATCH 08/19] webui: keep the state of Create Account UI We could maybe also use accounts structure directly without the local state for password, fullname, etc. I don't see advantage in using a reducer here. Using global state seems inapropriate to me (seems to be used more for backend state, or we should create one for UI - for example storageEcnryption could go there). Storing the state in backend doesn't seem like a good option, especially given it is kind of write-only - we are storing passwords encrypted by frontend. --- ui/webui/src/components/AnacondaWizard.jsx | 9 +++++- ui/webui/src/components/users/Accounts.jsx | 32 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index 4657d9a853a..63605567899 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -39,7 +39,7 @@ import { getDefaultScenario } from "./storage/InstallationScenario.jsx"; import { MountPointMapping, getPageProps as getMountPointMappingProps } from "./storage/MountPointMapping.jsx"; import { DiskEncryption, getStorageEncryptionState, getPageProps as getDiskEncryptionProps } from "./storage/DiskEncryption.jsx"; import { InstallationLanguage, getPageProps as getInstallationLanguageProps } from "./localization/InstallationLanguage.jsx"; -import { Accounts, getPageProps as getAccountsProps } from "./users/Accounts.jsx"; +import { Accounts, getPageProps as getAccountsProps, getAccountsState } from "./users/Accounts.jsx"; import { InstallationProgress } from "./installation/InstallationProgress.jsx"; import { ReviewConfiguration, ReviewConfigurationConfirmModal, getPageProps as getReviewConfigurationProps } from "./review/ReviewConfiguration.jsx"; import { exitGui } from "../helpers/exit.js"; @@ -64,6 +64,7 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim const [stepNotification, setStepNotification] = useState(); const [storageEncryption, setStorageEncryption] = useState(getStorageEncryptionState()); const [storageScenarioId, setStorageScenarioId] = useState(window.sessionStorage.getItem("storage-scenario-id") || getDefaultScenario().id); + const [accounts, setAccounts] = useState(getAccountsState()); const [showWizard, setShowWizard] = useState(true); const osRelease = useContext(OsReleaseContext); const isBootIso = useContext(SystemTypeContext) === "BOOT_ISO"; @@ -147,6 +148,8 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim { component: Accounts, data: { + accounts, + setAccounts, passwordPolicies: runtimeData.passwordPolicies, }, ...getAccountsProps() @@ -270,6 +273,7 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim stepsOrder={stepsOrder} storageEncryption={storageEncryption} storageScenarioId={storageScenarioId} + accounts={accounts} />} hideClose mainAriaLabel={`${title} content`} @@ -296,6 +300,7 @@ const Footer = ({ stepsOrder, storageEncryption, storageScenarioId, + accounts, }) => { const [nextWaitsConfirmation, setNextWaitsConfirmation] = useState(false); const [quitWaitsConfirmation, setQuitWaitsConfirmation] = useState(false); @@ -346,6 +351,8 @@ const Footer = ({ setStepNotification(); }, }); + } else if (activeStep.id === "accounts") { + onNext(); } else { onNext(); } diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 09ddb51f6f3..02fb47c9e84 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -31,15 +31,31 @@ import { PasswordFormFields, ruleLength } from "../Password.jsx"; const _ = cockpit.gettext; +export function getAccountsState ( + fullName = "", + userAccount = "", + password = "", + confirmPassword = "", +) { + return { + fullName, + userAccount, + password, + confirmPassword, + }; +} + const CreateAccount = ({ idPrefix, passwordPolicy, setIsUserValid, + accounts, + setAccounts, }) => { - const [fullName, setFullName] = useState(""); - const [userAccount, setUserAccount] = useState(""); - const [password, setPassword] = useState(""); - const [confirmPassword, setConfirmPassword] = useState(""); + const [fullName, setFullName] = useState(accounts.fullName); + const [userAccount, setUserAccount] = useState(accounts.userAccount); + const [password, setPassword] = useState(accounts.password); + const [confirmPassword, setConfirmPassword] = useState(accounts.confirmPassword); const [isPasswordValid, setIsPasswordValid] = useState(false); useEffect(() => { @@ -61,6 +77,10 @@ const CreateAccount = ({ /> ); + useEffect(() => { + setAccounts(ac => ({ ...ac, fullName, userAccount, password, confirmPassword })); + }, [setAccounts, fullName, userAccount, password, confirmPassword]); + return (
{ const [isUserValid, setIsUserValid] = useState(); useEffect(() => { @@ -114,6 +136,8 @@ export const Accounts = ({ idPrefix={idPrefix} passwordPolicy={passwordPolicies.user} setIsUserValid={setIsUserValid} + accounts={accounts} + setAccounts={setAccounts} /> ); From 143740d339fa9c625b384bc9b90520e71d7523a6 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 24 Oct 2023 13:42:35 +0200 Subject: [PATCH 09/19] webui: make partitioning reset on going back more robust It needs perhaps some refactoring or abstractions but not in scope of this PR and perhaps a bit later when the use cases for step objects are more clear. --- ui/webui/src/components/AnacondaWizard.jsx | 16 +++++++++++----- ui/webui/test/check-storage | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index 63605567899..c1ebc0b635d 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -189,10 +189,14 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim const firstStepId = stepsOrder.filter(step => !step.isHidden)[0].id; const currentStepId = path[0] || firstStepId; + const isStepFollowedBy = (earlierStepId, laterStepId) => { + const earlierStepIdx = flattenedStepsIds.findIndex(s => s === earlierStepId); + const laterStepIdx = flattenedStepsIds.findIndex(s => s === laterStepId); + return earlierStepIdx < laterStepIdx; + }; + const canJumpToStep = (stepId, currentStepId) => { - const stepIdx = flattenedStepsIds.findIndex(s => s === stepId); - const currentStepIdx = flattenedStepsIds.findIndex(s => s === currentStepId); - return stepIdx <= currentStepIdx; + return stepId === currentStepId || isStepFollowedBy(stepId, currentStepId); }; const createSteps = (stepsOrder) => { @@ -235,8 +239,10 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim setIsFormValid(false); } - // Reset the applied partitioning when going back from review page - if (prevStep.prevId === "installation-review") { + // Reset the applied partitioning when going back from a step after creating partitioning to a step + // before creating partitioning. + if ((prevStep.prevId === "accounts" || isStepFollowedBy("accounts", prevStep.prevId)) && + isStepFollowedBy(newStep.id, "accounts")) { setIsFormDisabled(true); resetPartitioning() .then( diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 3159a094f50..dae2bebab50 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -307,7 +307,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): self.assertEqual(len(created_partitioning) + 11, len(new_created_partitioning)) self.assertEqual(new_applied_partitioning, new_created_partitioning[-1]) - # The applied partitioning should be reset when going back at any step from review page + # The applied partitioning should be reset also when going back to installation method i.click_step_on_sidebar(i.steps.INSTALLATION_METHOD) new_applied_partitioning = s.dbus_get_applied_partitioning() self.assertEqual(new_applied_partitioning, "") From 86e8edb2d132ff14d78b67089f3e1fc4ae5c1c0b Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Wed, 25 Oct 2023 11:18:05 +0200 Subject: [PATCH 10/19] webui: apply the created user to the backend --- ui/webui/src/apis/users.js | 56 ++++++++++++++++++++++ ui/webui/src/components/AnacondaWizard.jsx | 6 ++- ui/webui/src/components/app.jsx | 2 + ui/webui/src/components/users/Accounts.jsx | 9 ++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 ui/webui/src/apis/users.js diff --git a/ui/webui/src/apis/users.js b/ui/webui/src/apis/users.js new file mode 100644 index 00000000000..a4d2bc6cc5a --- /dev/null +++ b/ui/webui/src/apis/users.js @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with This program; If not, see . + */ +import cockpit from "cockpit"; +import { _setProperty } from "./helpers.js"; + +const OBJECT_PATH = "/org/fedoraproject/Anaconda/Modules/Users"; +const INTERFACE_NAME = "org.fedoraproject.Anaconda.Modules.Users"; + +const setProperty = (...args) => { + return _setProperty(UsersClient, OBJECT_PATH, INTERFACE_NAME, ...args); +}; + +export class UsersClient { + constructor (address) { + if (UsersClient.instance && (!address || UsersClient.instance.address === address)) { + return UsersClient.instance; + } + + UsersClient.instance?.client.close(); + + UsersClient.instance = this; + + this.client = cockpit.dbus( + INTERFACE_NAME, + { superuser: "try", bus: "none", address } + ); + this.address = address; + } + + init () { + this.client.addEventListener( + "close", () => console.error("Users client closed") + ); + } +} + +/** + * @param {Array.} users An array of user objects + */ +export const setUsers = (users) => { + return setProperty("Users", cockpit.variant("aa{sv}", users)); +}; diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index c1ebc0b635d..a38c4e1a8cd 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -39,7 +39,7 @@ import { getDefaultScenario } from "./storage/InstallationScenario.jsx"; import { MountPointMapping, getPageProps as getMountPointMappingProps } from "./storage/MountPointMapping.jsx"; import { DiskEncryption, getStorageEncryptionState, getPageProps as getDiskEncryptionProps } from "./storage/DiskEncryption.jsx"; import { InstallationLanguage, getPageProps as getInstallationLanguageProps } from "./localization/InstallationLanguage.jsx"; -import { Accounts, getPageProps as getAccountsProps, getAccountsState } from "./users/Accounts.jsx"; +import { Accounts, getPageProps as getAccountsProps, getAccountsState, accountsToDbusUsers } from "./users/Accounts.jsx"; import { InstallationProgress } from "./installation/InstallationProgress.jsx"; import { ReviewConfiguration, ReviewConfigurationConfirmModal, getPageProps as getReviewConfigurationProps } from "./review/ReviewConfiguration.jsx"; import { exitGui } from "../helpers/exit.js"; @@ -51,6 +51,9 @@ import { applyStorage, resetPartitioning, } from "../apis/storage_partitioning.js"; +import { + setUsers, +} from "../apis/users.js"; import { SystemTypeContext, OsReleaseContext } from "./Common.jsx"; const _ = cockpit.gettext; @@ -358,6 +361,7 @@ const Footer = ({ }, }); } else if (activeStep.id === "accounts") { + setUsers(accountsToDbusUsers(accounts)); onNext(); } else { onNext(); diff --git a/ui/webui/src/components/app.jsx b/ui/webui/src/components/app.jsx index ea6939cdb90..564d4d46e7d 100644 --- a/ui/webui/src/components/app.jsx +++ b/ui/webui/src/components/app.jsx @@ -36,6 +36,7 @@ import { StorageClient, initDataStorage, startEventMonitorStorage } from "../api import { PayloadsClient } from "../apis/payloads"; import { RuntimeClient, initDataRuntime, startEventMonitorRuntime } from "../apis/runtime"; import { NetworkClient, initDataNetwork, startEventMonitorNetwork } from "../apis/network.js"; +import { UsersClient } from "../apis/users"; import { setCriticalErrorAction } from "../actions/miscellaneous-actions.js"; @@ -78,6 +79,7 @@ export const Application = () => { new RuntimeClient(address), new BossClient(address), new NetworkClient(address), + new UsersClient(address), ]; clients.forEach(c => c.init()); diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 02fb47c9e84..4a585231cc4 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -45,6 +45,15 @@ export function getAccountsState ( }; } +export const accountsToDbusUsers = (accounts) => { + return [{ + name: cockpit.variant("s", accounts.userAccount || ""), + gecos: cockpit.variant("s", accounts.fullName || ""), + password: cockpit.variant("s", accounts.password || ""), + "is-crypted": cockpit.variant("b", false), + }]; +}; + const CreateAccount = ({ idPrefix, passwordPolicy, From 4021cc102ddb955977eaba2479c8267a95d26c6d Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 31 Oct 2023 13:37:46 +0100 Subject: [PATCH 11/19] webui: make created user administarator by default --- ui/webui/src/components/users/Accounts.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 4a585231cc4..4f9c1768e37 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -51,6 +51,7 @@ export const accountsToDbusUsers = (accounts) => { gecos: cockpit.variant("s", accounts.fullName || ""), password: cockpit.variant("s", accounts.password || ""), "is-crypted": cockpit.variant("b", false), + groups: cockpit.variant("as", ["wheel"]), }]; }; From d7af610a1b79ffedccb83279bfce84b1e1263067 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Wed, 1 Nov 2023 11:28:36 +0100 Subject: [PATCH 12/19] webui: hide user screen on live images Should be hidden on Fedora WS, for now we handle this by checking isBootIso value. In the future we should use product/variant from configuration. --- ui/webui/src/components/AnacondaWizard.jsx | 2 +- ui/webui/src/components/users/Accounts.jsx | 3 ++- ui/webui/test/check-basic | 2 +- ui/webui/test/helpers/installer.py | 6 ++++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index a38c4e1a8cd..9ae1ae3d446 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -155,7 +155,7 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, runtim setAccounts, passwordPolicies: runtimeData.passwordPolicies, }, - ...getAccountsProps() + ...getAccountsProps({ isBootIso }) }, { component: ReviewConfiguration, diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 4f9c1768e37..475f2db06a6 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -153,10 +153,11 @@ export const Accounts = ({ ); }; -export const getPageProps = () => { +export const getPageProps = ({ isBootIso }) => { return ({ id: "accounts", label: _("Create Account"), + isHidden: !isBootIso, title: null, }); }; diff --git a/ui/webui/test/check-basic b/ui/webui/test/check-basic index 7056e69f27b..7d44ce30633 100755 --- a/ui/webui/test/check-basic +++ b/ui/webui/test/check-basic @@ -93,7 +93,7 @@ class TestBasic(anacondalib.VirtInstallMachineCase): b.wait_visible(f"#installation-back-btn:not([aria-disabled={True}]") # For live media in the review screen language details should still be displayed - i.reach(i.steps.REVIEW) + i.reach(i.steps.REVIEW, hidden_steps=[i.steps.ACCOUNTS]) r.check_language("English (United States)") def testAboutModal(self): diff --git a/ui/webui/test/helpers/installer.py b/ui/webui/test/helpers/installer.py index 80163150bb9..5a3402127e4 100644 --- a/ui/webui/test/helpers/installer.py +++ b/ui/webui/test/helpers/installer.py @@ -60,7 +60,8 @@ def begin_installation(self, should_fail=False, confirm_erase=True): else: self.wait_current_page(self.steps._steps_jump[current_page]) - def reach(self, target_page): + def reach(self, target_page, hidden_steps=None): + hidden_steps = hidden_steps or [] path = [] prev_pages = [target_page] current_page = self.get_current_page() @@ -72,7 +73,8 @@ def reach(self, target_page): while self.get_current_page() != target_page: next_page = path.pop() - self.next(next_page=next_page) + if next_page not in hidden_steps: + self.next(next_page=next_page) @log_step() def next(self, should_fail=False, next_page=""): From 1593bcde6eea604ff63ea1e8933a3b12c2d1f46f Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Thu, 2 Nov 2023 10:37:12 +0100 Subject: [PATCH 13/19] webui: add simple test for users screen --- ui/webui/src/components/users/Accounts.jsx | 16 ++--- ui/webui/test/check-storage | 72 +++++++++++---------- ui/webui/test/check-users | 60 ++++++++++++++++++ ui/webui/test/helpers/password.py | 74 ++++++++++++++++++++++ ui/webui/test/helpers/storage.py | 45 ------------- ui/webui/test/helpers/users.py | 65 +++++++++++++++++++ 6 files changed, 245 insertions(+), 87 deletions(-) create mode 100755 ui/webui/test/check-users create mode 100644 ui/webui/test/helpers/password.py create mode 100644 ui/webui/test/helpers/users.py diff --git a/ui/webui/src/components/users/Accounts.jsx b/ui/webui/src/components/users/Accounts.jsx index 475f2db06a6..8ab65260aea 100644 --- a/ui/webui/src/components/users/Accounts.jsx +++ b/ui/webui/src/components/users/Accounts.jsx @@ -74,7 +74,7 @@ const CreateAccount = ({ const passphraseForm = ( {_("Create account")} {_("This account will have administration priviledge with sudo.")} setFullName(val)} /> setUserAccount(val)} /> @@ -143,7 +143,7 @@ export const Accounts = ({ return ( <> . + +import anacondalib + +from installer import Installer +from password import Password +from users import Users, CREATE_ACCOUNT_ID_PREFIX +from testlib import nondestructive, test_main # pylint: disable=import-error + +@nondestructive +class TestUsers(anacondalib.VirtInstallMachineCase): + def setUp(self): + super().setUp() + + def testBasic(self): + b = self.browser + i = Installer(b, self.machine) + p = Password(b, CREATE_ACCOUNT_ID_PREFIX) + u = Users(b, self.machine) + + i.open() + + i.reach(i.steps.ACCOUNTS) + + password = "password" + p.set_password(password) + p.set_password_confirm(password) + u.set_user_account("tester") + + b.assert_pixels( + "#app", + "users-step-basic", + ignore=["#betanag-icon"], + ) + + self.addCleanup(u.dbus_clear_users) + i.reach(i.steps.REVIEW) + + users = u.dbus_get_users() + self.assertIn('"groups" as 1 "wheel"', users) + self.assertIn('"is-crypted" b false', users) + self.assertIn('"password" s "password"', users) + +if __name__ == '__main__': + test_main() diff --git a/ui/webui/test/helpers/password.py b/ui/webui/test/helpers/password.py new file mode 100644 index 00000000000..55324bfc67e --- /dev/null +++ b/ui/webui/test/helpers/password.py @@ -0,0 +1,74 @@ +#!/usr/bin/python3 +# +# Copyright (C) 2022 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; If not, see . + +import os +import sys + +HELPERS_DIR = os.path.dirname(__file__) +sys.path.append(HELPERS_DIR) + +from step_logger import log_step + +class Password(): + def __init__(self, browser, id_prefix): + self.browser = browser + self.id_prefix = id_prefix + + @log_step(snapshot_before=True) + def check_pw_rule(self, rule, value): + sel = f"#{self.id_prefix}-password-rule-" + rule + cls_value = "pf-m-" + value + self.browser.wait_visible(sel) + self.browser.wait_attr_contains(sel, "class", cls_value) + + @log_step(snapshot_before=True) + def set_password(self, password, append=False, value_check=True): + sel = f"#{self.id_prefix}-password-field" + self.browser.set_input_text(sel, password, append=append, value_check=value_check) + + @log_step(snapshot_before=True) + def check_password(self, password): + sel = f"#{self.id_prefix}-password-field" + self.browser.wait_val(sel, password) + + @log_step(snapshot_before=True) + def set_password_confirm(self, password): + sel = f"#{self.id_prefix}-password-confirm-field" + self.browser.set_input_text(sel, password) + + @log_step(snapshot_before=True) + def check_password_confirm(self, password): + sel = f"#{self.id_prefix}-password-confirm-field" + self.browser.wait_val(sel, password) + + @log_step(snapshot_before=True) + def check_pw_strength(self, strength): + sel = f"#{self.id_prefix}-password-strength-label" + + if strength is None: + self.browser.wait_not_present(sel) + return + + variant = "" + if strength == "weak": + variant = "error" + elif strength == "medium": + variant = "warning" + elif strength == "strong": + variant = "success" + + self.browser.wait_attr_contains(sel, "class", "pf-m-" + variant) diff --git a/ui/webui/test/helpers/storage.py b/ui/webui/test/helpers/storage.py index 2274b7fa990..1ef70954ff4 100644 --- a/ui/webui/test/helpers/storage.py +++ b/ui/webui/test/helpers/storage.py @@ -134,51 +134,6 @@ def set_encryption_selected(self, selected): sel = "#disk-encryption-encrypt-devices" self.browser.set_checked(sel, selected) - @log_step(snapshot_before=True) - def check_pw_rule(self, rule, value): - sel = "#disk-encryption-password-rule-" + rule - cls_value = "pf-m-" + value - self.browser.wait_visible(sel) - self.browser.wait_attr_contains(sel, "class", cls_value) - - @log_step(snapshot_before=True) - def set_password(self, password, append=False, value_check=True): - sel = "#disk-encryption-password-field" - self.browser.set_input_text(sel, password, append=append, value_check=value_check) - - @log_step(snapshot_before=True) - def check_password(self, password): - sel = "#disk-encryption-password-field" - self.browser.wait_val(sel, password) - - @log_step(snapshot_before=True) - def set_password_confirm(self, password): - sel = "#disk-encryption-password-confirm-field" - self.browser.set_input_text(sel, password) - - @log_step(snapshot_before=True) - def check_password_confirm(self, password): - sel = "#disk-encryption-password-confirm-field" - self.browser.wait_val(sel, password) - - @log_step(snapshot_before=True) - def check_pw_strength(self, strength): - sel = "#disk-encryption-password-strength-label" - - if strength is None: - self.browser.wait_not_present(sel) - return - - variant = "" - if strength == "weak": - variant = "error" - elif strength == "medium": - variant = "warning" - elif strength == "strong": - variant = "success" - - self.browser.wait_attr_contains(sel, "class", "pf-m-" + variant) - class StorageUtils(StorageDestination): def __init__(self, browser, machine): diff --git a/ui/webui/test/helpers/users.py b/ui/webui/test/helpers/users.py new file mode 100644 index 00000000000..eec3979206c --- /dev/null +++ b/ui/webui/test/helpers/users.py @@ -0,0 +1,65 @@ +#!/usr/bin/python3 +# +# Copyright (C) 2022 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; If not, see . + +import os +import sys + +HELPERS_DIR = os.path.dirname(__file__) +sys.path.append(HELPERS_DIR) + +from step_logger import log_step + + +USERS_SERVICE = "org.fedoraproject.Anaconda.Modules.Users" +USERS_INTERFACE = USERS_SERVICE +USERS_OBJECT_PATH = "/org/fedoraproject/Anaconda/Modules/Users" + +CREATE_ACCOUNT_ID_PREFIX = "accounts-create-account" + + +class UsersDBus(): + def __init__(self, machine): + self.machine = machine + self._bus_address = self.machine.execute("cat /run/anaconda/bus.address") + + def dbus_get_users(self): + ret = self.machine.execute(f'busctl --address="{self._bus_address}" \ + get-property \ + {USERS_SERVICE} \ + {USERS_OBJECT_PATH} \ + {USERS_INTERFACE} Users') + + return ret + + def dbus_clear_users(self): + self.machine.execute(f'busctl --address="{self._bus_address}" \ + set-property \ + {USERS_SERVICE} \ + {USERS_OBJECT_PATH} \ + {USERS_INTERFACE} Users aa{{sv}} 0') + + +class Users(UsersDBus): + def __init__(self, browser, machine): + self.browser = browser + + UsersDBus.__init__(self, machine) + + @log_step(snapshot_before=True) + def set_user_account(self, user_account, append=False, value_check=True): + sel = "#accounts-create-account-user-account" + self.browser.set_input_text(sel, user_account, append=append, value_check=value_check) From 7865158f0ad48e06d86d5d15328dd12defc167a4 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Thu, 2 Nov 2023 13:51:06 +0100 Subject: [PATCH 14/19] webui: add users screen to tests for sidebar navigation --- ui/webui/test/check-basic | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/webui/test/check-basic b/ui/webui/test/check-basic index 7d44ce30633..5a38f97c9cd 100755 --- a/ui/webui/test/check-basic +++ b/ui/webui/test/check-basic @@ -65,6 +65,7 @@ class TestBasic(anacondalib.VirtInstallMachineCase): # Test going back steps = [ + i.steps.ACCOUNTS, i.steps.DISK_CONFIGURATION, i.steps.DISK_ENCRYPTION, i.steps.DISK_CONFIGURATION, From 3d232999ad996bf283a43a2e5a4426a28435eccb Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Thu, 2 Nov 2023 11:51:44 +0100 Subject: [PATCH 15/19] webui: allow to create user more easily for reaching a step in test --- ui/webui/test/check-basic | 7 +++++-- ui/webui/test/check-progress | 4 +++- ui/webui/test/check-review | 11 ++++++++--- ui/webui/test/check-storage | 29 ++++++++++++++++++++--------- ui/webui/test/check-users | 13 ++----------- ui/webui/test/helpers/installer.py | 4 +++- ui/webui/test/helpers/users.py | 14 ++++++++++++++ 7 files changed, 55 insertions(+), 27 deletions(-) diff --git a/ui/webui/test/check-basic b/ui/webui/test/check-basic index 5a38f97c9cd..f4a3fdfa3b8 100755 --- a/ui/webui/test/check-basic +++ b/ui/webui/test/check-basic @@ -21,6 +21,7 @@ from installer import Installer from language import Language from review import Review from storage import Storage +from users import create_user from testlib import nondestructive, test_main, wait, Error # pylint: disable=import-error from utils import pretend_live_iso, get_pretty_name @@ -44,7 +45,8 @@ class TestBasic(anacondalib.VirtInstallMachineCase): # Do not start the installation in non-destructive tests as this performs non revertible changes # with the pages basically empty of common elements (as those are provided by the top-level installer widget) # we at least iterate over them to check this works as expected - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # Ensure that the 'actual' UI process is running/ self.assertIn("/usr/libexec/webui-desktop", m.execute("ps aux")) @@ -61,7 +63,8 @@ class TestBasic(anacondalib.VirtInstallMachineCase): # Test that clicking on current step does not break navigation i.click_step_on_sidebar() - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # Test going back steps = [ diff --git a/ui/webui/test/check-progress b/ui/webui/test/check-progress index feb43775695..56dfafcfc0f 100755 --- a/ui/webui/test/check-progress +++ b/ui/webui/test/check-progress @@ -20,6 +20,7 @@ import anacondalib from installer import Installer from progress import Progress from storage import Storage +from users import create_user from utils import add_public_key, get_pretty_name from testlib import test_main # pylint: disable=import-error @@ -40,7 +41,8 @@ class TestInstallationProgress(anacondalib.VirtInstallMachineCase): i.open() - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self, cleanup=False)}) i.begin_installation() # the warning can take longer to show up than the regular wait_in_text() timeout diff --git a/ui/webui/test/check-review b/ui/webui/test/check-review index e401e600bb2..18d546f664a 100755 --- a/ui/webui/test/check-review +++ b/ui/webui/test/check-review @@ -22,6 +22,7 @@ from storage import Storage # pylint: disable=import-error from review import Review from language import Language from progress import Progress +from users import create_user from utils import add_public_key from testlib import nondestructive, test_main # pylint: disable=import-error @@ -40,7 +41,9 @@ class TestReview(anacondalib.VirtInstallMachineCase): # After clicking 'Next' on the storage step, partitioning is done, thus changing the available space on the disk # Since this is a non-destructive test we need to make sure the we reset partitioning to how it was before the test started self.addCleanup(s.dbus_reset_partitioning) - i.reach(i.steps.REVIEW) + + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # check language is shown r.check_language("English (United States)") @@ -71,7 +74,8 @@ class TestReview(anacondalib.VirtInstallMachineCase): i.open() - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) i.begin_installation(should_fail=True, confirm_erase=False) def testUnknownLanguage(self): @@ -85,7 +89,8 @@ class TestReview(anacondalib.VirtInstallMachineCase): # Set language to Macedonian that is now in the installer translated languages # Go to review page after it l.dbus_set_language("mk_MK.UTF-8") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # test the macedonian language selected r.check_language("mk_MK.UTF-8") diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index da3ed70cdfa..607abf3ff1c 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -21,6 +21,7 @@ from installer import Installer from storage import Storage from review import Review from password import Password +from users import create_user from testlib import nondestructive, test_main # pylint: disable=import-error from storagelib import StorageHelpers # pylint: disable=import-error from utils import pretend_live_iso @@ -281,7 +282,8 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): # Go to Review step i.open() - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # Read partitioning data after we went to Review step new_applied_partitioning = s.dbus_get_applied_partitioning() @@ -415,7 +417,9 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) "mount-point-mapping-table", ) - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + # verify review screen r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -540,7 +544,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_mountpoint(3, "/home") s.check_mountpoint_row(3, "/home", f"{dev2}1", False, "xfs") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # verify review screen disk = "vda" @@ -626,7 +631,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) b.wait_in_text(f"{selector} .pf-v5-c-select__toggle-text", "luks") s.check_mountpoint_row_format_type(2, "xfs") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) r.check_in_disk_row(dev1, 2, "luks-") @@ -672,7 +678,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_device(2, "encryptedraid") s.check_mountpoint_row_format_type(2, "xfs") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -724,7 +731,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) b.wait_in_text(f"{selector} .pf-v5-c-select__toggle-text", "luks") s.check_mountpoint_row_format_type(2, "xfs") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -784,7 +792,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_reformat(1) s.check_mountpoint_row_reformat(1, True) - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # verify review screen r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -946,7 +955,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_reformat(1) s.check_mountpoint_row_reformat(1, True) - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # verify review screen disk = "vda" @@ -1035,7 +1045,8 @@ class TestStorageMountPointsEFI(anacondalib.VirtInstallMachineCase): s.select_mountpoint_row_device(3, f"{dev}3") s.check_mountpoint_row_format_type(3, "xfs") - i.reach(i.steps.REVIEW) + # Create user on the first pass through users step + i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) # verify review screen r.check_disk(dev, "16.1 GB vda (0x1af4)") diff --git a/ui/webui/test/check-users b/ui/webui/test/check-users index cdbf4e00aa2..a2248063263 100755 --- a/ui/webui/test/check-users +++ b/ui/webui/test/check-users @@ -18,8 +18,7 @@ import anacondalib from installer import Installer -from password import Password -from users import Users, CREATE_ACCOUNT_ID_PREFIX +from users import Users, CREATE_ACCOUNT_ID_PREFIX, create_user from testlib import nondestructive, test_main # pylint: disable=import-error @nondestructive @@ -30,25 +29,17 @@ class TestUsers(anacondalib.VirtInstallMachineCase): def testBasic(self): b = self.browser i = Installer(b, self.machine) - p = Password(b, CREATE_ACCOUNT_ID_PREFIX) u = Users(b, self.machine) i.open() i.reach(i.steps.ACCOUNTS) - - password = "password" - p.set_password(password) - p.set_password_confirm(password) - u.set_user_account("tester") - + create_user(self) b.assert_pixels( "#app", "users-step-basic", ignore=["#betanag-icon"], ) - - self.addCleanup(u.dbus_clear_users) i.reach(i.steps.REVIEW) users = u.dbus_get_users() diff --git a/ui/webui/test/helpers/installer.py b/ui/webui/test/helpers/installer.py index 5a3402127e4..16989aa4f46 100644 --- a/ui/webui/test/helpers/installer.py +++ b/ui/webui/test/helpers/installer.py @@ -60,7 +60,7 @@ def begin_installation(self, should_fail=False, confirm_erase=True): else: self.wait_current_page(self.steps._steps_jump[current_page]) - def reach(self, target_page, hidden_steps=None): + def reach(self, target_page, hidden_steps=None, step_callbacks={}): hidden_steps = hidden_steps or [] path = [] prev_pages = [target_page] @@ -75,6 +75,8 @@ def reach(self, target_page, hidden_steps=None): next_page = path.pop() if next_page not in hidden_steps: self.next(next_page=next_page) + if next_page in step_callbacks: + step_callbacks[next_page]() @log_step() def next(self, should_fail=False, next_page=""): diff --git a/ui/webui/test/helpers/users.py b/ui/webui/test/helpers/users.py index eec3979206c..46a39be9177 100644 --- a/ui/webui/test/helpers/users.py +++ b/ui/webui/test/helpers/users.py @@ -21,6 +21,7 @@ HELPERS_DIR = os.path.dirname(__file__) sys.path.append(HELPERS_DIR) +from password import Password from step_logger import log_step @@ -63,3 +64,16 @@ def __init__(self, browser, machine): def set_user_account(self, user_account, append=False, value_check=True): sel = "#accounts-create-account-user-account" self.browser.set_input_text(sel, user_account, append=append, value_check=value_check) + + +def create_user(test, cleanup=True): + p = Password(test.browser, CREATE_ACCOUNT_ID_PREFIX) + u = Users(test.browser, test.machine) + + password = "password" + p.set_password(password) + p.set_password_confirm(password) + u.set_user_account("tester") + + if cleanup: + test.addCleanup(u.dbus_clear_users) From 3bf083ad86de46a119a1cd9c7a221ca76127fff7 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 3 Nov 2023 11:01:01 +0100 Subject: [PATCH 16/19] webui: create required user when reaching a test step by default --- ui/webui/test/check-basic | 10 ++++---- ui/webui/test/check-progress | 4 +--- ui/webui/test/check-review | 14 +++++------ ui/webui/test/check-storage | 38 +++++++++++++++--------------- ui/webui/test/check-users | 2 +- ui/webui/test/helpers/installer.py | 11 ++++++--- ui/webui/test/helpers/users.py | 11 +++++---- 7 files changed, 47 insertions(+), 43 deletions(-) diff --git a/ui/webui/test/check-basic b/ui/webui/test/check-basic index f4a3fdfa3b8..3b2f5021169 100755 --- a/ui/webui/test/check-basic +++ b/ui/webui/test/check-basic @@ -21,7 +21,7 @@ from installer import Installer from language import Language from review import Review from storage import Storage -from users import create_user +from users import dbus_reset_users from testlib import nondestructive, test_main, wait, Error # pylint: disable=import-error from utils import pretend_live_iso, get_pretty_name @@ -45,8 +45,8 @@ class TestBasic(anacondalib.VirtInstallMachineCase): # Do not start the installation in non-destructive tests as this performs non revertible changes # with the pages basically empty of common elements (as those are provided by the top-level installer widget) # we at least iterate over them to check this works as expected - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # Ensure that the 'actual' UI process is running/ self.assertIn("/usr/libexec/webui-desktop", m.execute("ps aux")) @@ -63,8 +63,8 @@ class TestBasic(anacondalib.VirtInstallMachineCase): # Test that clicking on current step does not break navigation i.click_step_on_sidebar() - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # Test going back steps = [ diff --git a/ui/webui/test/check-progress b/ui/webui/test/check-progress index 56dfafcfc0f..feb43775695 100755 --- a/ui/webui/test/check-progress +++ b/ui/webui/test/check-progress @@ -20,7 +20,6 @@ import anacondalib from installer import Installer from progress import Progress from storage import Storage -from users import create_user from utils import add_public_key, get_pretty_name from testlib import test_main # pylint: disable=import-error @@ -41,8 +40,7 @@ class TestInstallationProgress(anacondalib.VirtInstallMachineCase): i.open() - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self, cleanup=False)}) + i.reach(i.steps.REVIEW) i.begin_installation() # the warning can take longer to show up than the regular wait_in_text() timeout diff --git a/ui/webui/test/check-review b/ui/webui/test/check-review index 18d546f664a..7ec8392ea1d 100755 --- a/ui/webui/test/check-review +++ b/ui/webui/test/check-review @@ -22,7 +22,7 @@ from storage import Storage # pylint: disable=import-error from review import Review from language import Language from progress import Progress -from users import create_user +from users import dbus_reset_users from utils import add_public_key from testlib import nondestructive, test_main # pylint: disable=import-error @@ -42,8 +42,8 @@ class TestReview(anacondalib.VirtInstallMachineCase): # Since this is a non-destructive test we need to make sure the we reset partitioning to how it was before the test started self.addCleanup(s.dbus_reset_partitioning) - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # check language is shown r.check_language("English (United States)") @@ -74,8 +74,8 @@ class TestReview(anacondalib.VirtInstallMachineCase): i.open() - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) i.begin_installation(should_fail=True, confirm_erase=False) def testUnknownLanguage(self): @@ -89,8 +89,8 @@ class TestReview(anacondalib.VirtInstallMachineCase): # Set language to Macedonian that is now in the installer translated languages # Go to review page after it l.dbus_set_language("mk_MK.UTF-8") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # test the macedonian language selected r.check_language("mk_MK.UTF-8") diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 607abf3ff1c..3b43bdcf775 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -19,9 +19,9 @@ import anacondalib from installer import Installer from storage import Storage +from users import dbus_reset_users from review import Review from password import Password -from users import create_user from testlib import nondestructive, test_main # pylint: disable=import-error from storagelib import StorageHelpers # pylint: disable=import-error from utils import pretend_live_iso @@ -282,8 +282,8 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): # Go to Review step i.open() - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # Read partitioning data after we went to Review step new_applied_partitioning = s.dbus_get_applied_partitioning() @@ -417,8 +417,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) "mount-point-mapping-table", ) - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # verify review screen @@ -544,8 +544,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_mountpoint(3, "/home") s.check_mountpoint_row(3, "/home", f"{dev2}1", False, "xfs") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # verify review screen disk = "vda" @@ -631,8 +631,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) b.wait_in_text(f"{selector} .pf-v5-c-select__toggle-text", "luks") s.check_mountpoint_row_format_type(2, "xfs") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) r.check_in_disk_row(dev1, 2, "luks-") @@ -678,8 +678,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_device(2, "encryptedraid") s.check_mountpoint_row_format_type(2, "xfs") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -731,8 +731,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) b.wait_in_text(f"{selector} .pf-v5-c-select__toggle-text", "luks") s.check_mountpoint_row_format_type(2, "xfs") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -792,8 +792,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_reformat(1) s.check_mountpoint_row_reformat(1, True) - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # verify review screen r.check_disk(dev, "16.1 GB vda (0x1af4)") @@ -955,8 +955,8 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase, StorageHelpers) s.select_mountpoint_row_reformat(1) s.check_mountpoint_row_reformat(1, True) - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # verify review screen disk = "vda" @@ -1045,8 +1045,8 @@ class TestStorageMountPointsEFI(anacondalib.VirtInstallMachineCase): s.select_mountpoint_row_device(3, f"{dev}3") s.check_mountpoint_row_format_type(3, "xfs") - # Create user on the first pass through users step - i.reach(i.steps.REVIEW, step_callbacks={i.steps.ACCOUNTS: lambda: create_user(self)}) + self.addCleanup(lambda: dbus_reset_users(self.machine)) + i.reach(i.steps.REVIEW) # verify review screen r.check_disk(dev, "16.1 GB vda (0x1af4)") diff --git a/ui/webui/test/check-users b/ui/webui/test/check-users index a2248063263..5d6d3601db0 100755 --- a/ui/webui/test/check-users +++ b/ui/webui/test/check-users @@ -34,7 +34,7 @@ class TestUsers(anacondalib.VirtInstallMachineCase): i.open() i.reach(i.steps.ACCOUNTS) - create_user(self) + create_user(b, self.machine) b.assert_pixels( "#app", "users-step-basic", diff --git a/ui/webui/test/helpers/installer.py b/ui/webui/test/helpers/installer.py index 16989aa4f46..7c0e57cc1db 100644 --- a/ui/webui/test/helpers/installer.py +++ b/ui/webui/test/helpers/installer.py @@ -18,6 +18,8 @@ from time import sleep from step_logger import log_step +from users import create_user + class InstallerSteps(UserDict): WELCOME = "installation-language" @@ -38,6 +40,9 @@ class InstallerSteps(UserDict): _steps_jump[REVIEW] = PROGRESS _steps_jump[PROGRESS] = [] + _steps_callbacks = {} + _steps_callbacks[ACCOUNTS] = create_user + class Installer(): def __init__(self, browser, machine): self.browser = browser @@ -60,7 +65,7 @@ def begin_installation(self, should_fail=False, confirm_erase=True): else: self.wait_current_page(self.steps._steps_jump[current_page]) - def reach(self, target_page, hidden_steps=None, step_callbacks={}): + def reach(self, target_page, hidden_steps=None): hidden_steps = hidden_steps or [] path = [] prev_pages = [target_page] @@ -75,8 +80,8 @@ def reach(self, target_page, hidden_steps=None, step_callbacks={}): next_page = path.pop() if next_page not in hidden_steps: self.next(next_page=next_page) - if next_page in step_callbacks: - step_callbacks[next_page]() + if next_page in self.steps._steps_callbacks: + self.steps._steps_callbacks[next_page](self.browser, self.machine) @log_step() def next(self, should_fail=False, next_page=""): diff --git a/ui/webui/test/helpers/users.py b/ui/webui/test/helpers/users.py index 46a39be9177..e6a53f2dd32 100644 --- a/ui/webui/test/helpers/users.py +++ b/ui/webui/test/helpers/users.py @@ -66,14 +66,15 @@ def set_user_account(self, user_account, append=False, value_check=True): self.browser.set_input_text(sel, user_account, append=append, value_check=value_check) -def create_user(test, cleanup=True): - p = Password(test.browser, CREATE_ACCOUNT_ID_PREFIX) - u = Users(test.browser, test.machine) +def create_user(browser, machine): + p = Password(browser, CREATE_ACCOUNT_ID_PREFIX) + u = Users(browser, machine) password = "password" p.set_password(password) p.set_password_confirm(password) u.set_user_account("tester") - if cleanup: - test.addCleanup(u.dbus_clear_users) + +def dbus_reset_users(machine): + UsersDBus(machine).dbus_clear_users() From 415843e9839897090997795c322314e77ac47250 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 3 Nov 2023 12:35:27 +0100 Subject: [PATCH 17/19] webui: update end2end tests for the new users screen --- ui/webui/test/check-storage | 15 ++++++--------- ui/webui/test/end2end/storage_encryption.py | 10 +++++----- ui/webui/test/end2end/wizard_navigation.py | 2 ++ ui/webui/test/helpers/end2end.py | 8 ++++++++ ui/webui/test/helpers/storage.py | 6 ++++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 3b43bdcf775..9cda4588eed 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -30,13 +30,10 @@ from utils import pretend_live_iso @nondestructive class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): efi = False - encryption_id_prefix = "disk-encryption" - def set_valid_password(self, password="abcdefgh"): - p = Password(self.browser, self.encryption_id_prefix) - - p.set_password(password) - p.set_password_confirm(password) + def set_valid_password(self, password_ui, password="abcdefgh"): + password_ui.set_password(password) + password_ui.set_password_confirm(password) def testLocalStandardDisks(self): b = self.browser @@ -136,7 +133,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): b = self.browser i = Installer(b, self.machine) s = Storage(b, self.machine) - p = Password(b, self.encryption_id_prefix) + p = Password(b, s.encryption_id_prefix) i.open() # Language selection @@ -221,7 +218,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): b = self.browser i = Installer(b, self.machine) s = Storage(b, self.machine) - p = Password(b, self.encryption_id_prefix) + p = Password(b, s.encryption_id_prefix) i.open() # Language selection @@ -248,7 +245,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers): s.check_encryption_selected(encrypt) # Set valid password - self.set_valid_password() + self.set_valid_password(p) # Verify that the password is saved when moving forward and back i.next() diff --git a/ui/webui/test/end2end/storage_encryption.py b/ui/webui/test/end2end/storage_encryption.py index 703c163e765..caa69c62605 100755 --- a/ui/webui/test/end2end/storage_encryption.py +++ b/ui/webui/test/end2end/storage_encryption.py @@ -38,11 +38,11 @@ def configure_storage_encryption(self): self._storage.set_encryption_selected(True) self._storage.check_encryption_selected(True) - self._storage.set_password(self.luks_pass) - self._storage.check_password(self.luks_pass) - self._storage.set_password_confirm(self.luks_pass) - self._storage.check_password_confirm(self.luks_pass) - self._storage.check_pw_strength('weak') + self._storage_password.set_password(self.luks_pass) + self._storage_password.check_password(self.luks_pass) + self._storage_password.set_password_confirm(self.luks_pass) + self._storage_password.check_password_confirm(self.luks_pass) + self._storage_password.check_pw_strength('weak') @log_step() def post_install_step(self): diff --git a/ui/webui/test/end2end/wizard_navigation.py b/ui/webui/test/end2end/wizard_navigation.py index 0d24fd4d944..d1bbf239fcd 100755 --- a/ui/webui/test/end2end/wizard_navigation.py +++ b/ui/webui/test/end2end/wizard_navigation.py @@ -45,6 +45,8 @@ def test_wizard_navigation(self): self._installer.next() self.configure_storage_encryption() self._installer.next() + self.check_users_screen() + self._installer.next() # Finish installation self.check_review_screen() self._installer.begin_installation() diff --git a/ui/webui/test/helpers/end2end.py b/ui/webui/test/helpers/end2end.py index 02a2ccef42c..65008fbf8c5 100644 --- a/ui/webui/test/helpers/end2end.py +++ b/ui/webui/test/helpers/end2end.py @@ -30,6 +30,8 @@ from storage import Storage from review import Review from progress import Progress +from password import Password +from users import create_user from utils import add_public_key from testlib import MachineCase # pylint: disable=import-error from machine_install import VirtInstallMachine @@ -44,6 +46,7 @@ def setUp(self): self._installer = Installer(self.browser, self.machine) self._language = Language(self.browser, self.machine) self._storage = Storage(self.browser, self.machine) + self._storage_password = Password(self.browser, self._storage.encryption_id_prefix) self._review = Review(self.browser) self._progress = Progress(self.browser) self.__installation_finished = False @@ -75,6 +78,9 @@ def configure_storage_encryption(self): def check_review_screen(self): pass + def check_users_screen(self): + create_user(self.browser, self.machine) + def monitor_progress(self): self._progress.wait_done() @@ -101,6 +107,8 @@ def run_integration_test(self): self._installer.next() self.configure_storage_encryption() self._installer.next() + self.check_users_screen() + self._installer.next() self.check_review_screen() self._installer.begin_installation() self.monitor_progress() diff --git a/ui/webui/test/helpers/storage.py b/ui/webui/test/helpers/storage.py index 1ef70954ff4..604998fe8cd 100644 --- a/ui/webui/test/helpers/storage.py +++ b/ui/webui/test/helpers/storage.py @@ -117,13 +117,15 @@ def check_disk_visible(self, disk, visible=True): class StorageEncryption(): + encryption_id_prefix = "disk-encryption" + def __init__(self, browser, machine): self.browser = browser self.machine = machine @log_step(snapshot_before=True) def check_encryption_selected(self, selected): - sel = "#disk-encryption-encrypt-devices" + sel = f"#{self.encryption_id_prefix}-encrypt-devices" if selected: self.browser.wait_visible(sel + ':checked') else: @@ -131,7 +133,7 @@ def check_encryption_selected(self, selected): @log_step(snapshot_before=True) def set_encryption_selected(self, selected): - sel = "#disk-encryption-encrypt-devices" + sel = f"#{self.encryption_id_prefix}-encrypt-devices" self.browser.set_checked(sel, selected) From 1588dad011eebe7cfdf4d65d6d4a6a697914bd23 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 6 Nov 2023 10:04:21 +0100 Subject: [PATCH 18/19] webui: fix password strength indicator layout in horizontal form --- ui/webui/src/components/storage/DiskEncryption.scss | 2 +- ui/webui/src/components/users/Accounts.scss | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ui/webui/src/components/storage/DiskEncryption.scss b/ui/webui/src/components/storage/DiskEncryption.scss index bb1272cb4af..1d908619860 100644 --- a/ui/webui/src/components/storage/DiskEncryption.scss +++ b/ui/webui/src/components/storage/DiskEncryption.scss @@ -1,4 +1,4 @@ -// Span disk encryption password fields to take slightly more width than the default +// Limit the width of input fields #disk-encryption-encrypt-devices ~ .pf-v5-c-check__body { width: min(60ch, 100%); } diff --git a/ui/webui/src/components/users/Accounts.scss b/ui/webui/src/components/users/Accounts.scss index f53ec05624c..dcc4da3b88f 100644 --- a/ui/webui/src/components/users/Accounts.scss +++ b/ui/webui/src/components/users/Accounts.scss @@ -1,4 +1,10 @@ -// Span disk encryption password fields to take slightly more width than the default +// Limit the width of input fields #accounts-create-account { width: min(60ch, 100%); } + +// FIXME: Undo when fixed upstream: https://github.com/patternfly/patternfly/issues/6032 +#accounts-create-account .pf-v5-c-form__group-label.pf-m-info { + flex-direction: column; + align-items: flex-start; +} From fb8257c5924035af2b3db805232a4b315d40725b Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 31 Oct 2023 11:50:50 +0100 Subject: [PATCH 19/19] webui: update pixel test images --- ui/webui/test/reference | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/webui/test/reference b/ui/webui/test/reference index e05382c968a..926f5923b79 160000 --- a/ui/webui/test/reference +++ b/ui/webui/test/reference @@ -1 +1 @@ -Subproject commit e05382c968a80a76c717eca43882e12f982bff22 +Subproject commit 926f5923b7994a2858fb0b104936989ad60b5bd4