diff --git a/src/components/review/ReviewConfiguration.jsx b/src/components/review/ReviewConfiguration.jsx
index a597759717..78cbb7d95a 100644
--- a/src/components/review/ReviewConfiguration.jsx
+++ b/src/components/review/ReviewConfiguration.jsx
@@ -144,7 +144,7 @@ export const ReviewConfiguration = ({ deviceData, diskSelection, language, local
{_("Account")}
- {accounts.fullName ? `${accounts.fullName} (${accounts.userAccount})` : accounts.userAccount}
+ {accounts.fullName ? `${accounts.fullName} (${accounts.userName})` : accounts.userName}
diff --git a/src/components/users/Accounts.jsx b/src/components/users/Accounts.jsx
index bce838273b..8f28b363e7 100644
--- a/src/components/users/Accounts.jsx
+++ b/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,7 +59,7 @@ 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),
@@ -62,6 +67,33 @@ export const accountsToDbusUsers = (accounts) => {
}];
};
+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 = (
{
- 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 (
- setUserAccount(val)}
- />
+
+ _setUserName(val)}
+ validated={userNameValidated}
+ />
+
+ {userNameValidated === "error" &&
+
+
+
+ {userNameInvalidHint}
+
+
+ }
{passphraseForm}
diff --git a/src/components/users/Accounts.scss b/src/components/users/Accounts.scss
index dcc4da3b88..ad27a615ce 100644
--- a/src/components/users/Accounts.scss
+++ b/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%);
+}
diff --git a/test/check-users b/test/check-users
index 59fe08e848..5b52ad2542 100755
--- a/test/check-users
+++ b/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()
diff --git a/test/helpers/users.py b/test/helpers/users.py
index e6a53f2dd3..b2af000ce3 100644
--- a/test/helpers/users.py
+++ b/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):
diff --git a/test/reference b/test/reference
index cc6e86c9a4..c326dfef87 160000
--- a/test/reference
+++ b/test/reference
@@ -1 +1 @@
-Subproject commit cc6e86c9a430de95a580bd275ef01659240d1af4
+Subproject commit c326dfef87adf3b9c9f3f4d72a4a10f8f386f676