Validate user account value

src/components/review/ReviewConfiguration.jsx

@@ -144,7 +144,7 @@ export const ReviewConfiguration = ({ deviceData, diskSelection, language, local
                         {_("Account")}
                     </DescriptionListTerm>
                     <DescriptionListDescription id={idPrefix + "-target-system-account"}>
-                        {accounts.fullName ? `${accounts.fullName} (${accounts.userAccount})` : accounts.userAccount}
+                        {accounts.fullName ? `${accounts.fullName} (${accounts.userName})` : accounts.userName}
                     </DescriptionListDescription>
                 </DescriptionListGroup>
             </ReviewDescriptionList>

src/components/users/Accounts.jsx

@@ -19,10 +19,15 @@ import cockpit from "cockpit";
 import React, { useState, useEffect } from "react";
 import * as python from "python.js";
 import encryptUserPw from "../../scripts/encrypt-user-pw.py";
+import { debounce } from "throttle-debounce";
 
 import {
     Form,
     FormGroup,
+    FormHelperText,
+    HelperText,
+    HelperTextItem,
+    InputGroup,
     TextInput,
     Title,
 } from "@patternfly/react-core";
@@ -35,13 +40,13 @@ const _ = cockpit.gettext;
 
 export function getAccountsState (
     fullName = "",
-    userAccount = "",
+    userName = "",
     password = "",
     confirmPassword = "",
 ) {
     return {
         fullName,
-        userAccount,
+        userName,
         password,
         confirmPassword,
     };
@@ -54,14 +59,41 @@ export const cryptUserPassword = async (password) => {
 
 export const accountsToDbusUsers = (accounts) => {
     return [{
-        name: cockpit.variant("s", accounts.userAccount || ""),
+        name: cockpit.variant("s", accounts.userName || ""),
         gecos: cockpit.variant("s", accounts.fullName || ""),
         password: cockpit.variant("s", accounts.password || ""),
         "is-crypted": cockpit.variant("b", true),
         groups: cockpit.variant("as", ["wheel"]),
     }];
 };
 
+const reservedNames = [
+    "root",
+    "bin",
+    "daemon",
+    "adm",
+    "lp",
+    "sync",
+    "shutdown",
+    "halt",
+    "mail",
+    "operator",
+    "games",
+    "ftp",
+    "nobody",
+    "home",
+    "system",
+];
+
+const isUserNameWithInvalidCharacters = (userName) => {
+    return (
+        userName === "." ||
+        userName === ".." ||
+        userName.match(/^[0-9]+$/) ||
+        !userName.match(/^[A-Za-z0-9._][A-Za-z0-9._-]{0,30}([A-Za-z0-9._-]|\$)?$/)
+    );
+};
+
 const CreateAccount = ({
     idPrefix,
     passwordPolicy,
@@ -70,14 +102,39 @@ const CreateAccount = ({
     setAccounts,
 }) => {
     const [fullName, setFullName] = useState(accounts.fullName);
-    const [userAccount, setUserAccount] = useState(accounts.userAccount);
+    const [_userName, _setUserName] = useState(accounts.userName);
+    const [userName, setUserName] = useState(accounts.userName);
+    const [userNameInvalidHint, setUserNameInvalidHint] = useState("");
+    const [isUserNameValid, setIsUserNameValid] = useState(null);
     const [password, setPassword] = useState(accounts.password);
     const [confirmPassword, setConfirmPassword] = useState(accounts.confirmPassword);
     const [isPasswordValid, setIsPasswordValid] = useState(false);
 
     useEffect(() => {
-        setIsUserValid(isPasswordValid && userAccount.length > 0);
-    }, [setIsUserValid, isPasswordValid, userAccount]);
+        debounce(300, () => setUserName(_userName))();
+    }, [_userName, setUserName]);
+
+    useEffect(() => {
+        setIsUserValid(isPasswordValid && isUserNameValid);
+    }, [setIsUserValid, isPasswordValid, isUserNameValid]);
+
+    useEffect(() => {
+        let valid = true;
+        setUserNameInvalidHint("");
+        if (userName.length === 0) {
+            valid = null;
+        } else if (userName.length > 32) {
+            valid = false;
+            setUserNameInvalidHint(_("User names must be shorter than 33 characters"));
+        } else if (reservedNames.includes(userName)) {
+            valid = false;
+            setUserNameInvalidHint(_("User name must not be a reserved word"));
+        } else if (isUserNameWithInvalidCharacters(userName)) {
+            valid = false;
+            setUserNameInvalidHint(cockpit.format(_("User name may only contain: letters from a-z, digits 0-9, dash $0, period $1, underscore $2"), "-", ".", "_"));
+        }
+        setIsUserNameValid(valid);
+    }, [userName]);
 
     const passphraseForm = (
         <PasswordFormFields
@@ -95,8 +152,10 @@ const CreateAccount = ({
     );
 
     useEffect(() => {
-        setAccounts(ac => ({ ...ac, fullName, userAccount, password, confirmPassword }));
-    }, [setAccounts, fullName, userAccount, password, confirmPassword]);
+        setAccounts(ac => ({ ...ac, fullName, userName, password, confirmPassword }));
+    }, [setAccounts, fullName, userName, password, confirmPassword]);
+
+    const userNameValidated = isUserNameValid === null ? "default" : isUserNameValid ? "success" : "error";
 
     return (
         <Form
@@ -121,14 +180,25 @@ const CreateAccount = ({
                 />
             </FormGroup>
             <FormGroup
-              label={_("User account")}
-              fieldId={idPrefix + "-user-account"}
+              label={_("User name")}
+              fieldId={idPrefix + "-user-name"}
             >
-                <TextInput
-                  id={idPrefix + "-user-account"}
-                  value={userAccount}
-                  onChange={(_event, val) => setUserAccount(val)}
-                />
+                <InputGroup id={idPrefix + "-user-name-input-group"}>
+                    <TextInput
+                      id={idPrefix + "-user-name"}
+                      value={_userName}
+                      onChange={(_event, val) => _setUserName(val)}
+                      validated={userNameValidated}
+                    />
+                </InputGroup>
+                {userNameValidated === "error" &&
+                <FormHelperText>
+                    <HelperText>
+                        <HelperTextItem variant={userNameValidated}>
+                            {userNameInvalidHint}
+                        </HelperTextItem>
+                    </HelperText>
+                </FormHelperText>}
             </FormGroup>
             {passphraseForm}
         </Form>

src/components/users/Accounts.scss

@@ -8,3 +8,7 @@
   flex-direction: column;
   align-items: flex-start;
 }
+
+#accounts-create-account-user-name-input-group {
+  width: min(39ch, 100%);
+}

test/check-users

@@ -18,8 +18,10 @@
 import anacondalib
 
 from installer import Installer
-from users import Users, CREATE_ACCOUNT_ID_PREFIX, create_user
-from testlib import nondestructive, test_main  # pylint: disable=import-error
+from password import Password
+from review import Review
+from users import Users, CREATE_ACCOUNT_ID_PREFIX, create_user, dbus_reset_users
+from testlib import nondestructive, test_main, sit  # pylint: disable=import-error
 
 @nondestructive
 class TestUsers(anacondalib.VirtInstallMachineCase):
@@ -31,6 +33,8 @@ class TestUsers(anacondalib.VirtInstallMachineCase):
         i = Installer(b, self.machine)
         u = Users(b, self.machine)
 
+        self.addCleanup(lambda: dbus_reset_users(self.machine))
+
         i.open()
 
         i.reach(i.steps.ACCOUNTS)
@@ -46,5 +50,145 @@ class TestUsers(anacondalib.VirtInstallMachineCase):
         self.assertIn('"groups" as 1 "wheel"', users)
         self.assertIn('"is-crypted" b true', users)
 
+    @staticmethod
+    def _test_user_name(user, installer, valid, invalid):
+        installer.check_next_disabled(disabled=False)
+
+        n_valid = len(valid) - 1
+        n_invalid = len(invalid) - 1
+
+        # Set valid and invalid value in turns and check the next button
+        for i in range(max(n_valid, n_invalid)):
+            i_valid = min(i, n_valid)
+            i_invalid = min(i, n_invalid)
+            user.set_user_name(invalid[i_invalid])
+            installer.check_next_disabled()
+            user.set_user_name(valid[i_valid])
+            installer.check_next_disabled(disabled=False)
+
+    def testAccessAndValidation(self):
+        b = self.browser
+        i = Installer(b, self.machine)
+        p = Password(b, CREATE_ACCOUNT_ID_PREFIX)
+        u = Users(b, self.machine)
+
+        self.addCleanup(lambda: dbus_reset_users(self.machine))
+
+        i.open()
+
+        i.reach(i.steps.ACCOUNTS)
+
+        # Fill in valid values
+        password = "password"
+        full_name = "Full Tester"
+        user_name = "tester"
+
+        p.set_password(password)
+        p.set_password_confirm(password)
+        u.set_full_name(full_name)
+        u.set_user_name(user_name)
+
+        p.check_password(password)
+        p.check_password_confirm(password)
+        u.check_full_name(full_name)
+        u.check_user_name(user_name)
+        i.check_next_disabled(disabled=False)
+
+        # Test that you can move forward and going back keeps values
+        i.next()
+        i.back()
+        p.check_password(password)
+        p.check_password_confirm(password)
+        u.check_full_name(full_name)
+        u.check_user_name(user_name)
+
+        # Test that moving back and forward keeps values
+        i.back()
+        i.next()
+        p.check_password(password)
+        p.check_password_confirm(password)
+        u.check_full_name(full_name)
+        u.check_user_name(user_name)
+
+        # Test full name validation
+        u.set_full_name("")
+        i.check_next_disabled(disabled=False)
+        u.set_full_name(full_name)
+
+        # Test account validation
+        # FIXME this should be tested by unit tests?
+        invalid_user_names = [
+            "",
+            # reserved
+            "root", "system", "daemon", "home",
+            "33333",
+            ".",
+            "..",
+            "-tester",
+            "123",
+            "$",
+            "$tester",
+            "tester?",
+            "longer_than_32_characteeeeeeeeeeeeeeeeeeers",
+        ]
+        valid_user_names = [
+            "tester-",
+            "...",
+            "12.3",
+            "tester1",
+            "tester$",
+            "test-er",
+            "test_er",
+            "test.er",
+            "_tester",
+            ".tester",
+            "_",
+        ]
+        self._test_user_name(u, i, valid_user_names, invalid_user_names)
+        u.set_user_name(user_name)
+
+        # Test password validation
+        # No password set
+        p.set_password("")
+        p.set_password_confirm("")
+        p.check_pw_rule("length", "indeterminate")
+        p.check_pw_rule("match", "indeterminate")
+        i.check_next_disabled()
+        # Start which pw which is too short
+        p.set_password("abcd")
+        p.check_pw_strength(None)
+        i.check_next_disabled()
+        p.check_pw_rule("length", "error")
+        p.check_pw_rule("match", "error")
+        # Make the pw 6 chars long
+        p.set_password("ef", append=True, value_check=False)
+        i.check_next_disabled()
+        p.check_password("abcdef")
+        p.check_pw_rule("length", "success")
+        p.check_pw_rule("match", "error")
+        p.check_pw_strength("weak")
+        # Set the password confirm
+        p.set_password_confirm("abcde")
+        p.check_pw_rule("match", "error")
+        p.set_password_confirm("abcdef")
+        p.check_pw_rule("match", "success")
+        p.check_pw_rule("length", "success")
+        p.check_pw_strength("weak")
+        i.check_next_disabled(disabled=False)
+
+        # Test password strength
+        p.set_password("Rwce82ybF7dXtCzFumanchu!!!!!!!!")
+        p.check_pw_strength("strong")
+
+        # Test review values
+        p.set_password(password)
+        p.set_password_confirm(password)
+        u.set_full_name(full_name)
+        u.set_user_name(user_name)
+        r = Review(b)
+        i.reach(i.steps.REVIEW)
+        r.check_account(f"{full_name} ({user_name})")
+
+
 if __name__ == '__main__':
     test_main()

test/helpers/users.py

@@ -61,9 +61,24 @@ def __init__(self, browser, machine):
         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)
+    def set_user_name(self, user_name, append=False, value_check=True):
+        sel = "#accounts-create-account-user-name"
+        self.browser.set_input_text(sel, user_name, append=append, value_check=value_check)
+
+    @log_step(snapshot_before=True)
+    def check_user_name(self, user_name):
+        sel = "#accounts-create-account-user-name"
+        self.browser.wait_val(sel, user_name)
+
+    @log_step(snapshot_before=True)
+    def set_full_name(self, full_name, append=False, value_check=True):
+        sel = "#accounts-create-account-full-name"
+        self.browser.set_input_text(sel, full_name, append=append, value_check=value_check)
+
+    @log_step(snapshot_before=True)
+    def check_full_name(self, full_name):
+        sel = "#accounts-create-account-full-name"
+        self.browser.wait_val(sel, full_name)
 
 
 def create_user(browser, machine):
@@ -73,7 +88,7 @@ def create_user(browser, machine):
     password = "password"
     p.set_password(password)
     p.set_password_confirm(password)
-    u.set_user_account("tester")
+    u.set_user_name("tester")
 
 
 def dbus_reset_users(machine):