From 5347c6afebb7c2e6d6936deb1719e9c5741d7c07 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 9 Oct 2022 17:01:57 +0200 Subject: [PATCH 1/5] Merge firstname and lastname info --- share/actionsmap.yml | 45 ++++++++++++++---------- src/tests/test_user-group.py | 25 ++++++++------ src/tools.py | 5 ++- src/user.py | 67 ++++++++++++++++-------------------- 4 files changed, 74 insertions(+), 68 deletions(-) diff --git a/share/actionsmap.yml b/share/actionsmap.yml index 2253cea54..98ae59a7b 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -73,19 +73,28 @@ user: pattern: &pattern_username - !!str ^[a-z0-9_]+$ - "pattern_username" + -F: + full: --fullname + help: The full name of the user. For example 'Camille Dupont' + extra: + ask: ask_fullname + required: False + pattern: &pattern_fullname + - !!str ^([^\W\d_]{1,30}[ ,.'-]{0,3})+$ + - "pattern_fullname" -f: full: --firstname + help: Deprecated. Use --fullname instead. extra: - ask: ask_firstname - required: True + required: False pattern: &pattern_firstname - !!str ^([^\W\d_]{1,30}[ ,.'-]{0,3})+$ - "pattern_firstname" -l: full: --lastname + help: Deprecated. Use --fullname instead. extra: - ask: ask_lastname - required: True + required: False pattern: &pattern_lastname - !!str ^([^\W\d_]{1,30}[ ,.'-]{0,3})+$ - "pattern_lastname" @@ -136,12 +145,19 @@ user: arguments: username: help: Username to update + -F: + full: --fullname + help: The full name of the user. For example 'Camille Dupont' + extra: + pattern: *pattern_fullname -f: full: --firstname + help: Deprecated. Use --fullname instead. extra: pattern: *pattern_firstname -l: full: --lastname + help: Deprecated. Use --fullname instead. extra: pattern: *pattern_lastname -m: @@ -1520,25 +1536,18 @@ tools: required: True -u: full: --username - help: Username for the first (admin) user + help: Username for the first (admin) user. For example 'camille' extra: - ask: ask_username + ask: ask_admin_username pattern: *pattern_username required: True - -f: - full: --firstname - help: Firstname for the first (admin) user + -F: + full: --fullname + help: The full name for the first (admin) user. For example 'Camille Dupont' extra: - ask: ask_firstname + ask: ask_admin_fullname required: True - pattern: *pattern_firstname - -l: - full: --lastname - help: Lastname for the first (admin) user - extra: - ask: ask_lastname - required: True - pattern: *pattern_lastname + pattern: *pattern_fullname -p: full: --password help: YunoHost admin password diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index 1a368ceac..dc65e8ed8 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -38,9 +38,9 @@ def setup_function(function): global maindomain maindomain = _get_maindomain() - user_create("alice", "Alice", "White", maindomain, "test123Ynh", admin=True) - user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") - user_create("jack", "Jack", "Black", maindomain, "test123Ynh") + user_create("alice", maindomain, "test123Ynh", admin=True, fullname="Alice White") + user_create("bob", maindomain, "test123Ynh", fullname="Bob Snow") + user_create("jack", maindomain, "test123Ynh", fullname="Jack Black") user_group_create("dev") user_group_create("apps") @@ -94,7 +94,7 @@ def test_list_groups(): def test_create_user(mocker): with message(mocker, "user_created"): - user_create("albert", "Albert", "Good", maindomain, "test123Ynh") + user_create("albert", maindomain, "test123Ynh", fullname="Albert Good") group_res = user_group_list()["groups"] assert "albert" in user_list()["users"] @@ -211,17 +211,17 @@ def test_del_group(mocker): def test_create_user_with_password_too_simple(mocker): with raiseYunohostError(mocker, "password_listed"): - user_create("other", "Alice", "White", maindomain, "12") + user_create("other", maindomain, "12", fullname="Alice White") def test_create_user_already_exists(mocker): with raiseYunohostError(mocker, "user_already_exists"): - user_create("alice", "Alice", "White", maindomain, "test123Ynh") + user_create("alice", maindomain, "test123Ynh", fullname="Alice White") def test_create_user_with_domain_that_doesnt_exists(mocker): with raiseYunohostError(mocker, "domain_unknown"): - user_create("alice", "Alice", "White", "doesnt.exists", "test123Ynh") + user_create("alice", "doesnt.exists", "test123Ynh", fullname="Alice White") def test_update_user_with_mail_address_already_taken(mocker): @@ -255,7 +255,7 @@ def test_del_group_all_users(mocker): with raiseYunohostError(mocker, "group_cannot_be_deleted"): user_group_delete("all_users") - +/ def test_del_group_that_does_not_exist(mocker): with raiseYunohostError(mocker, "group_unknown"): user_group_delete("doesnt_exist") @@ -271,8 +271,13 @@ def test_update_user(mocker): user_update("alice", firstname="NewName", lastname="NewLast") info = user_info("alice") - assert info["firstname"] == "NewName" - assert info["lastname"] == "NewLast" + assert info["fullname"] == "NewName NewLast" + + with message(mocker, "user_updated"): + user_update("alice", fullname="New2Name New2Last") + + info = user_info("alice") + assert info["fullname"] == "New2Name New2Last" def test_update_group_add_user(mocker): diff --git a/src/tools.py b/src/tools.py index 09574c36e..ecf19cf25 100644 --- a/src/tools.py +++ b/src/tools.py @@ -146,8 +146,7 @@ def tools_postinstall( operation_logger, domain, username, - firstname, - lastname, + fullname, password, ignore_dyndns=False, force_diskspace=False, @@ -226,7 +225,7 @@ def tools_postinstall( domain_add(domain, dyndns) domain_main_domain(domain) - user_create(username, firstname, lastname, domain, password, admin=True) + user_create(username, domain, password, admin=True, fullname=fullname) # Update LDAP admin and create home dir tools_rootpw(password) diff --git a/src/user.py b/src/user.py index e00fa3685..13c806d1c 100644 --- a/src/user.py +++ b/src/user.py @@ -134,15 +134,29 @@ def user_list(fields=None): def user_create( operation_logger, username, - firstname, - lastname, domain, password, + fullname=None, + firstname=None, + lastname=None, mailbox_quota="0", admin=False, from_import=False, ): + if firstname or lastname: + logger.warning("Options --firstname / --lastname of 'yunohost user create' are deprecated. We recommend using --fullname instead.") + + if not fullname.strip(): + if not firstname.strip(): + raise YunohostValidationError("You should specify the fullname of the user using option -F") + lastname = lastname or " " # Stupid hack because LDAP requires the sn/lastname attr, but it accepts a single whitespace... + fullname = f"{firstname} {lastname}".strip() + else: + fullname = fullname.strip() + firstname = fullname.split()[0] + lastname = ' '.join(fullname.split()[1:]) or " " # Stupid hack because LDAP requires the sn/lastname attr, but it accepts a single whitespace... + from yunohost.domain import domain_list, _get_maindomain, _assert_domain_exists from yunohost.hook import hook_callback from yunohost.utils.password import ( @@ -219,9 +233,6 @@ def user_create( uid = str(random.randint(1001, 65000)) uid_guid_found = uid not in all_uid and uid not in all_gid - # Adapt values for LDAP - fullname = f"{firstname} {lastname}" - attr_dict = { "objectClass": [ "mailAccount", @@ -292,14 +303,7 @@ def user_create( @is_unit_operation([("username", "user")]) def user_delete(operation_logger, username, purge=False, from_import=False): - """ - Delete user - Keyword argument: - username -- Username to delete - purge - - """ from yunohost.hook import hook_callback from yunohost.utils.ldap import _get_ldap_interface @@ -357,22 +361,14 @@ def user_update( remove_mailalias=None, mailbox_quota=None, from_import=False, + fullname=None, ): - """ - Update user informations - Keyword argument: - lastname - mail - firstname - add_mailalias -- Mail aliases to add - remove_mailforward -- Mailforward addresses to remove - username -- Username of user to update - add_mailforward -- Mailforward addresses to add - change_password -- New password to set - remove_mailalias -- Mail aliases to remove + if fullname.strip(): + fullname = fullname.strip() + firstname = fullname.split()[0] + lastname = ' '.join(fullname.split()[1:]) or " " # Stupid hack because LDAP requires the sn/lastname attr, but it accepts a single whitespace... - """ from yunohost.domain import domain_list, _get_maindomain from yunohost.app import app_ssowatconf from yunohost.utils.password import ( @@ -402,20 +398,20 @@ def user_update( if firstname: new_attr_dict["givenName"] = [firstname] # TODO: Validate new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - firstname + " " + user["sn"][0] + (firstname + " " + user["sn"][0]).strip() ] env_dict["YNH_USER_FIRSTNAME"] = firstname if lastname: new_attr_dict["sn"] = [lastname] # TODO: Validate new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - user["givenName"][0] + " " + lastname + (user["givenName"][0] + " " + lastname).strip() ] env_dict["YNH_USER_LASTNAME"] = lastname if lastname and firstname: new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - firstname + " " + lastname + (firstname + " " + lastname).strip() ] # change_password is None if user_update is not called to change the password @@ -547,7 +543,7 @@ def user_info(username): ldap = _get_ldap_interface() - user_attrs = ["cn", "mail", "uid", "maildrop", "givenName", "sn", "mailuserquota"] + user_attrs = ["cn", "mail", "uid", "maildrop", "mailuserquota"] if len(username.split("@")) == 2: filter = "mail=" + username @@ -564,8 +560,6 @@ def user_info(username): result_dict = { "username": user["uid"][0], "fullname": user["cn"][0], - "firstname": user["givenName"][0], - "lastname": user["sn"][0], "mail": user["mail"][0], "mail-aliases": [], "mail-forward": [], @@ -859,10 +853,9 @@ def user_import(operation_logger, csvfile, update=False, delete=False): user_update( new_infos["username"], - new_infos["firstname"], - new_infos["lastname"], - new_infos["mail"], - new_infos["password"], + firstname=new_infos["firstname"], + lastname=new_infos["lastname"], + password=new_infos["password"], mailbox_quota=new_infos["mailbox-quota"], mail=new_infos["mail"], add_mailalias=new_infos["mail-alias"], @@ -902,12 +895,12 @@ def user_import(operation_logger, csvfile, update=False, delete=False): try: user_create( user["username"], - user["firstname"], - user["lastname"], user["domain"], user["password"], user["mailbox-quota"], from_import=True, + firstname=user["firstname"], + lastname=user["lastname"], ) update(user) result["created"] += 1 From e64e5b9c1411440a9b4752949014cb24b12be8d3 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 9 Oct 2022 18:02:45 +0200 Subject: [PATCH 2/5] Big oopsie --- src/tests/test_user-group.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index dc65e8ed8..095558d7a 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -255,7 +255,6 @@ def test_del_group_all_users(mocker): with raiseYunohostError(mocker, "group_cannot_be_deleted"): user_group_delete("all_users") -/ def test_del_group_that_does_not_exist(mocker): with raiseYunohostError(mocker, "group_unknown"): user_group_delete("doesnt_exist") From 4822afb9d6d6c1ccf302f01e53e0d2a2646a09b4 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 9 Oct 2022 19:40:44 +0200 Subject: [PATCH 3/5] Fix postinstall test --- .gitlab/ci/install.gitlab-ci.yml | 2 +- .gitlab/ci/test.gitlab-ci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab/ci/install.gitlab-ci.yml b/.gitlab/ci/install.gitlab-ci.yml index 89360c8f8..ecdfecfcd 100644 --- a/.gitlab/ci/install.gitlab-ci.yml +++ b/.gitlab/ci/install.gitlab-ci.yml @@ -26,4 +26,4 @@ install-postinstall: script: - apt-get update -o Acquire::Retries=3 - DEBIAN_FRONTEND=noninteractive SUDO_FORCE_REMOVE=yes apt --assume-yes -o Dpkg::Options::="--force-confold" --allow-downgrades install ./$YNH_BUILD_DIR/*.deb - - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l Mine -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -F 'Syssa Mine' -p the_password --ignore-dyndns --force-diskspace diff --git a/.gitlab/ci/test.gitlab-ci.yml b/.gitlab/ci/test.gitlab-ci.yml index d7ccbc807..8d0d90ded 100644 --- a/.gitlab/ci/test.gitlab-ci.yml +++ b/.gitlab/ci/test.gitlab-ci.yml @@ -34,7 +34,7 @@ full-tests: PYTEST_ADDOPTS: "--color=yes" before_script: - *install_debs - - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l Mine -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -F 'Syssa Mine' -p the_password --ignore-dyndns --force-diskspace script: - python3 -m pytest --cov=yunohost tests/ src/tests/ src/diagnosers/ --junitxml=report.xml - cd tests From e32fe7aa41fff10213d9b516ee88801ee0056aaf Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 9 Oct 2022 20:52:20 +0200 Subject: [PATCH 4/5] Moar oopsies --- src/user.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/user.py b/src/user.py index 13c806d1c..68310f4b4 100644 --- a/src/user.py +++ b/src/user.py @@ -147,7 +147,7 @@ def user_create( if firstname or lastname: logger.warning("Options --firstname / --lastname of 'yunohost user create' are deprecated. We recommend using --fullname instead.") - if not fullname.strip(): + if not fullname or not fullname.strip(): if not firstname.strip(): raise YunohostValidationError("You should specify the fullname of the user using option -F") lastname = lastname or " " # Stupid hack because LDAP requires the sn/lastname attr, but it accepts a single whitespace... @@ -364,7 +364,10 @@ def user_update( fullname=None, ): - if fullname.strip(): + if firstname or lastname: + logger.warning("Options --firstname / --lastname of 'yunohost user create' are deprecated. We recommend using --fullname instead.") + + if fullname and fullname.strip(): fullname = fullname.strip() firstname = fullname.split()[0] lastname = ' '.join(fullname.split()[1:]) or " " # Stupid hack because LDAP requires the sn/lastname attr, but it accepts a single whitespace... @@ -855,7 +858,7 @@ def user_import(operation_logger, csvfile, update=False, delete=False): new_infos["username"], firstname=new_infos["firstname"], lastname=new_infos["lastname"], - password=new_infos["password"], + change_password=new_infos["password"], mailbox_quota=new_infos["mailbox-quota"], mail=new_infos["mail"], add_mailalias=new_infos["mail-alias"], From 42bcd5e6d3ca2e33ee30bcd2f0b7753a3113878d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 9 Oct 2022 21:53:35 +0200 Subject: [PATCH 5/5] Propagate changes to user_create() function in other test modules --- src/tests/test_app_config.py | 2 +- src/tests/test_backuprestore.py | 2 +- src/tests/test_ldapauth.py | 4 ++-- src/tests/test_permission.py | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/test_app_config.py b/src/tests/test_app_config.py index d6cf8045d..db898233d 100644 --- a/src/tests/test_app_config.py +++ b/src/tests/test_app_config.py @@ -102,7 +102,7 @@ def config_app(request): def test_app_config_get(config_app): - user_create("alice", "Alice", "White", _get_maindomain(), "test123Ynh") + user_create("alice", _get_maindomain(), "test123Ynh", fullname="Alice White") assert isinstance(app_config_get(config_app), dict) assert isinstance(app_config_get(config_app, full=True), dict) diff --git a/src/tests/test_backuprestore.py b/src/tests/test_backuprestore.py index 17147f586..adc14b80e 100644 --- a/src/tests/test_backuprestore.py +++ b/src/tests/test_backuprestore.py @@ -77,7 +77,7 @@ def setup_function(function): if "with_permission_app_installed" in markers: assert not app_is_installed("permissions_app") - user_create("alice", "Alice", "White", maindomain, "test123Ynh") + user_create("alice", maindomain, "test123Ynh", fullname="Alice White") with patch.object(os, "isatty", return_value=False): install_app("permissions_app_ynh", "/urlpermissionapp" "&admin=alice") assert app_is_installed("permissions_app") diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index db5229342..f8ad83544 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -19,8 +19,8 @@ def setup_function(function): if os.system("systemctl is-active slapd >/dev/null") != 0: os.system("systemctl start slapd && sleep 3") - user_create("alice", "Alice", "White", maindomain, "Yunohost", admin=True) - user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") + user_create("alice", maindomain, "Yunohost", admin=True, fullname="Alice White") + user_create("bob", maindomain, "test123Ynh", fullname="Bob Snow") def teardown_function(): diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index 379f1cf39..5ba073d96 100644 --- a/src/tests/test_permission.py +++ b/src/tests/test_permission.py @@ -158,8 +158,8 @@ def setup_function(function): socket.getaddrinfo = new_getaddrinfo - user_create("alice", "Alice", "White", maindomain, dummy_password) - user_create("bob", "Bob", "Snow", maindomain, dummy_password) + user_create("alice", maindomain, dummy_password, fullname="Alice White") + user_create("bob", maindomain, dummy_password, fullname="Bob Snow") _permission_create_with_dummy_app( permission="wiki.main", url="/",