diff --git a/.gitlab/ci/install.gitlab-ci.yml b/.gitlab/ci/install.gitlab-ci.yml index e2662e9e2..89360c8f8 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 -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l 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 519ae427a..d7ccbc807 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 -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l Mine -p the_password --ignore-dyndns --force-diskspace script: - python3 -m pytest --cov=yunohost tests/ src/tests/ src/diagnosers/ --junitxml=report.xml - cd tests diff --git a/conf/postfix/main.cf b/conf/postfix/main.cf index 13b68cafa..19b40aefb 100644 --- a/conf/postfix/main.cf +++ b/conf/postfix/main.cf @@ -102,7 +102,7 @@ message_size_limit = 35914708 virtual_mailbox_domains = ldap:/etc/postfix/ldap-domains.cf virtual_mailbox_maps = ldap:/etc/postfix/ldap-accounts.cf virtual_mailbox_base = -virtual_alias_maps = ldap:/etc/postfix/ldap-aliases.cf +virtual_alias_maps = ldap:/etc/postfix/ldap-aliases.cf,ldap:/etc/postfix/ldap-groups.cf virtual_alias_domains = virtual_minimum_uid = 100 virtual_uid_maps = static:vmail diff --git a/conf/postfix/plain/ldap-groups.cf b/conf/postfix/plain/ldap-groups.cf new file mode 100644 index 000000000..dbf768641 --- /dev/null +++ b/conf/postfix/plain/ldap-groups.cf @@ -0,0 +1,9 @@ +server_host = localhost +server_port = 389 +search_base = dc=yunohost,dc=org +query_filter = (&(objectClass=groupOfNamesYnh)(mail=%s)) +exclude_internal = yes +search_timeout = 30 +scope = sub +result_attribute = memberUid, mail +terminal_result_attribute = memberUid diff --git a/conf/slapd/config.ldif b/conf/slapd/config.ldif index e1fe3b1b5..249422950 100644 --- a/conf/slapd/config.ldif +++ b/conf/slapd/config.ldif @@ -130,7 +130,6 @@ olcSuffix: dc=yunohost,dc=org # admin entry below # These access lines apply to database #1 only olcAccess: {0}to attrs=userPassword,shadowLastChange - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write by anonymous auth by self write @@ -140,7 +139,6 @@ olcAccess: {0}to attrs=userPassword,shadowLastChange # owning it if they are authenticated. # Others should be able to see it. olcAccess: {1}to attrs=cn,gecos,givenName,mail,maildrop,displayName,sn - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write by self write by * read @@ -160,9 +158,7 @@ olcAccess: {2}to dn.base="" # The admin dn has full write access, everyone else # can read everything. olcAccess: {3}to * - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write - by group/groupOfNames/member.exact="cn=admin,ou=groups,dc=yunohost,dc=org" write by * read # olcAddContentAcl: FALSE diff --git a/conf/slapd/db_init.ldif b/conf/slapd/db_init.ldif index be0181dfe..95b9dd936 100644 --- a/conf/slapd/db_init.ldif +++ b/conf/slapd/db_init.ldif @@ -5,15 +5,6 @@ objectClass: organization o: yunohost.org dc: yunohost -dn: cn=admin,ou=sudo,dc=yunohost,dc=org -cn: admin -objectClass: sudoRole -objectClass: top -sudoCommand: ALL -sudoUser: admin -sudoOption: !authenticate -sudoHost: ALL - dn: ou=users,dc=yunohost,dc=org objectClass: organizationalUnit objectClass: top @@ -39,28 +30,31 @@ objectClass: organizationalUnit objectClass: top ou: groups +dn: cn=admins,ou=sudo,dc=yunohost,dc=org +cn: admins +objectClass: sudoRole +objectClass: top +sudoCommand: ALL +sudoUser: %admins +sudoHost: ALL + dn: ou=sudo,dc=yunohost,dc=org objectClass: organizationalUnit objectClass: top ou: sudo -dn: cn=admin,dc=yunohost,dc=org -objectClass: organizationalRole -objectClass: posixAccount -objectClass: simpleSecurityObject -cn: admin -uid: admin -uidNumber: 1007 -gidNumber: 1007 -homeDirectory: /home/admin -loginShell: /bin/bash -userPassword: yunohost - dn: cn=admins,ou=groups,dc=yunohost,dc=org objectClass: posixGroup objectClass: top -memberUid: admin +objectClass: groupOfNamesYnh +objectClass: mailGroup gidNumber: 4001 +mail: root +mail: admin +mail: admins +mail: webmaster +mail: postmaster +mail: abuse cn: admins dn: cn=all_users,ou=groups,dc=yunohost,dc=org diff --git a/conf/slapd/mailserver.ldif b/conf/slapd/mailserver.ldif index 849d1d9e1..09f5c64cc 100644 --- a/conf/slapd/mailserver.ldif +++ b/conf/slapd/mailserver.ldif @@ -89,4 +89,7 @@ olcObjectClasses: ( 1.3.6.1.4.1.40328.1.1.2.3 NAME 'mailGroup' SUP top AUXILIARY DESC 'Mail Group' MUST ( mail ) + MAY ( + mailalias $ maildrop + ) ) diff --git a/hooks/conf_regen/01-yunohost b/hooks/conf_regen/01-yunohost index dc0bfc689..aab617c2c 100755 --- a/hooks/conf_regen/01-yunohost +++ b/hooks/conf_regen/01-yunohost @@ -42,7 +42,7 @@ do_init_regen() { # Backup folders mkdir -p /home/yunohost.backup/archives chmod 750 /home/yunohost.backup/archives - chown root:root /home/yunohost.backup/archives # This is later changed to admin:root once admin user exists + chown root:root /home/yunohost.backup/archives # This is later changed to root:admins once the admins group exists # Empty ssowat json persistent conf echo "{}" >'/etc/ssowat/conf.json.persistent' @@ -168,12 +168,11 @@ do_post_regen() { # Enfore permissions # ###################### - chmod 750 /home/admin - chmod 750 /home/yunohost.backup - chmod 750 /home/yunohost.backup/archives + chmod 770 /home/yunohost.backup + chmod 770 /home/yunohost.backup/archives chmod 700 /var/cache/yunohost - chown admin:root /home/yunohost.backup - chown admin:root /home/yunohost.backup/archives + chown root:admins /home/yunohost.backup + chown root:admins /home/yunohost.backup/archives chown root:root /var/cache/yunohost # NB: x permission for 'others' is important for ssl-cert (and maybe mdns), otherwise slapd will fail to start because can't access the certs diff --git a/hooks/conf_regen/06-slapd b/hooks/conf_regen/06-slapd index 1cc1052b7..aefbca3f4 100755 --- a/hooks/conf_regen/06-slapd +++ b/hooks/conf_regen/06-slapd @@ -58,14 +58,6 @@ EOF nscd -i passwd || true systemctl restart slapd - - # We don't use mkhomedir_helper because 'admin' may not be recognized - # when this script is ran in a chroot (e.g. ISO install) - # We also refer to admin as uid 1007 for the same reason - if [ ! -d /home/admin ]; then - cp -r /etc/skel /home/admin - chown -R 1007:1007 /home/admin - fi } _regenerate_slapd_conf() { @@ -172,22 +164,6 @@ objectClass: top" echo "Reloading slapd" systemctl force-reload slapd - - # on slow hardware/vm this regen conf would exit before the admin user that - # is stored in ldap is available because ldap seems to slow to restart - # so we'll wait either until we are able to log as admin or until a timeout - # is reached - # we need to do this because the next hooks executed after this one during - # postinstall requires to run as admin thus breaking postinstall on slow - # hardware which mean yunohost can't be correctly installed on those hardware - # and this sucks - # wait a maximum time of 5 minutes - # yes, force-reload behave like a restart - number_of_wait=0 - while ! su admin -c '' && ((number_of_wait < 60)); do - sleep 5 - ((number_of_wait += 1)) - done } do_$1_regen ${@:2} diff --git a/locales/en.json b/locales/en.json index e351993b3..74b62408e 100644 --- a/locales/en.json +++ b/locales/en.json @@ -6,7 +6,6 @@ "admin_password": "Administration password", "admin_password_change_failed": "Unable to change password", "admin_password_changed": "The administration password was changed", - "admin_password_too_long": "Please choose a password shorter than 127 characters", "already_up_to_date": "Nothing to do. Everything is already up-to-date.", "app_action_broke_system": "This action seems to have broken these important services: {services}", "app_action_cannot_be_ran_because_required_services_down": "These required services should be running to run this action: {services}. Try restarting them to continue (and possibly investigate why they are down).", @@ -430,6 +429,7 @@ "invalid_number_min": "Must be greater than {min}", "invalid_password": "Invalid password", "invalid_regex": "Invalid regex:'{regex}'", + "invalid_credentials": "Invalid password or username", "ip6tables_unavailable": "You cannot play with ip6tables here. You are either in a container or your kernel does not support it", "iptables_unavailable": "You cannot play with iptables here. You are either in a container or your kernel does not support it", "ldap_attribute_already_exists": "LDAP attribute '{attribute}' already exists with value '{value}'", @@ -551,6 +551,7 @@ "password_too_simple_2": "The password needs to be at least 8 characters long and contain a digit, upper and lower characters", "password_too_simple_3": "The password needs to be at least 8 characters long and contain a digit, upper, lower and special characters", "password_too_simple_4": "The password needs to be at least 12 characters long and contain a digit, upper, lower and special characters", + "password_too_long": "Please choose a password shorter than 127 characters", "pattern_backup_archive_name": "Must be a valid filename with max 30 characters, alphanumeric and -_. characters only", "pattern_domain": "Must be a valid domain name (e.g. my-domain.org)", "pattern_email": "Must be a valid e-mail address, without '+' symbol (e.g. someone@example.com)", @@ -701,5 +702,5 @@ "yunohost_configured": "YunoHost is now configured", "yunohost_installing": "Installing YunoHost...", "yunohost_not_installed": "YunoHost is not correctly installed. Please run 'yunohost tools postinstall'", - "yunohost_postinstall_end_tip": "The post-install completed! To finalize your setup, please consider:\n - adding a first user through the 'Users' section of the webadmin (or 'yunohost user create ' in command-line);\n - diagnose potential issues through the 'Diagnosis' section of the webadmin (or 'yunohost diagnosis run' in command-line);\n - reading the 'Finalizing your setup' and 'Getting to know YunoHost' parts in the admin documentation: https://yunohost.org/admindoc." -} \ No newline at end of file + "yunohost_postinstall_end_tip": "The post-install completed! To finalize your setup, please consider:\n - diagnose potential issues through the 'Diagnosis' section of the webadmin (or 'yunohost diagnosis run' in command-line);\n - reading the 'Finalizing your setup' and 'Getting to know YunoHost' parts in the admin documentation: https://yunohost.org/admindoc." +} diff --git a/share/actionsmap.yml b/share/actionsmap.yml index f15b363e4..c54c0972c 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -1445,10 +1445,10 @@ tools: category_help: Specific tools actions: - ### tools_adminpw() - adminpw: - action_help: Change password of admin and root users - api: PUT /adminpw + ### tools_rootpw() + rootpw: + action_help: Change root password + api: PUT /rootpw arguments: -n: full: --new-password @@ -1483,6 +1483,25 @@ tools: ask: ask_main_domain pattern: *pattern_domain required: True + -u: + full: --username + help: Username for the first (admin) user + extra: + ask: ask_username + pattern: *pattern_username + required: True + -f: + full: --firstname + extra: + ask: ask_firstname + required: True + pattern: *pattern_firstname + -l: + full: --lastname + extra: + ask: ask_lastname + required: True + pattern: *pattern_lastname -p: full: --password help: YunoHost admin password @@ -1494,14 +1513,10 @@ tools: --ignore-dyndns: help: Do not subscribe domain to a DynDNS service action: store_true - --force-password: - help: Use this if you really want to set a weak password - action: store_true --force-diskspace: help: Use this if you really want to install YunoHost on a setup with less than 10 GB on the root filesystem action: store_true - ### tools_update() update: action_help: YunoHost update diff --git a/src/authenticators/ldap_admin.py b/src/authenticators/ldap_admin.py index 872dd3c8d..a7fc18da6 100644 --- a/src/authenticators/ldap_admin.py +++ b/src/authenticators/ldap_admin.py @@ -11,37 +11,64 @@ from moulinette.authentication import BaseAuthenticator from moulinette.utils.text import random_ascii from yunohost.utils.error import YunohostError, YunohostAuthenticationError - -logger = logging.getLogger("yunohost.authenticators.ldap_admin") +from yunohost.utils.ldap import _get_ldap_interface session_secret = random_ascii() +logger = logging.getLogger("yunohost.authenticators.ldap_admin") +LDAP_URI = "ldap://localhost:389" +ADMIN_GROUP = "cn=admins,ou=groups" +AUTH_DN = "uid={uid},ou=users,dc=yunohost,dc=org" class Authenticator(BaseAuthenticator): name = "ldap_admin" def __init__(self, *args, **kwargs): - self.uri = "ldap://localhost:389" - self.basedn = "dc=yunohost,dc=org" - self.admindn = "cn=admin,dc=yunohost,dc=org" + pass def _authenticate_credentials(self, credentials=None): - # TODO : change authentication format - # to support another dn to support multi-admins + try: + admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", []) + except ldap.SERVER_DOWN: + # ldap is down, attempt to restart it before really failing + logger.warning(m18n.n("ldap_server_is_down_restart_it")) + os.system("systemctl restart slapd") + time.sleep(10) # waits 10 secondes so we are sure that slapd has restarted + + # Force-reset existing LDAP interface + from yunohost.utils import ldap as ldaputils + ldaputils._ldap_interface = None + + try: + admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", []) + except ldap.SERVER_DOWN: + raise YunohostError("ldap_server_down") + + try: + uid, password = credentials.split(":", 1) + except ValueError: + raise YunohostError("invalid_credentials") + + # Here we're explicitly using set() which are handled as hash tables + # and should prevent timing attacks to find out the admin usernames? + if uid not in set(admins): + raise YunohostError("invalid_credentials") + + dn = AUTH_DN.format(uid=uid) def _reconnect(): con = ldap.ldapobject.ReconnectLDAPObject( - self.uri, retry_max=10, retry_delay=0.5 + LDAP_URI, retry_max=10, retry_delay=0.5 ) - con.simple_bind_s(self.admindn, credentials) + con.simple_bind_s(dn, password) return con try: con = _reconnect() except ldap.INVALID_CREDENTIALS: - raise YunohostError("invalid_password") + raise YunohostError("invalid_credentials") except ldap.SERVER_DOWN: # ldap is down, attempt to restart it before really failing logger.warning(m18n.n("ldap_server_is_down_restart_it")) @@ -61,11 +88,8 @@ class Authenticator(BaseAuthenticator): logger.warning("Error during ldap authentication process: %s", e) raise else: - if who != self.admindn: - raise YunohostError( - f"Not logged with the appropriate identity ? Found {who}, expected {self.admindn} !?", - raw_msg=True, - ) + if who != dn: + raise YunohostError(f"Not logged with the appropriate identity ? Found {who}, expected {dn} !?", raw_msg=True) finally: # Free the connection, we don't really need it to keep it open as the point is only to check authentication... if con: diff --git a/src/backup.py b/src/backup.py index 90a843b6e..a3b5ec3a0 100644 --- a/src/backup.py +++ b/src/backup.py @@ -346,7 +346,7 @@ class BackupManager: # FIXME replace isdir by exists ? manage better the case where the path # exists if not os.path.isdir(self.work_dir): - mkdir(self.work_dir, 0o750, parents=True, uid="admin") + mkdir(self.work_dir, 0o750, parents=True) elif self.is_tmp_work_dir: logger.debug( @@ -362,7 +362,7 @@ class BackupManager: # we're in /home/yunohost.backup/tmp so that should be okay... # c.f. method clean() which also does this) rm(self.work_dir, recursive=True, force=True) - mkdir(self.work_dir, 0o750, parents=True, uid="admin") + mkdir(self.work_dir, 0o750, parents=True) # # Backup target management # @@ -1897,7 +1897,7 @@ class CopyBackupMethod(BackupMethod): dest_parent = os.path.dirname(dest) if not os.path.exists(dest_parent): - mkdir(dest_parent, 0o700, True, uid="admin") + mkdir(dest_parent, 0o700, True) if os.path.isdir(source): shutil.copytree(source, dest) @@ -1959,7 +1959,7 @@ class TarBackupMethod(BackupMethod): """ if not os.path.exists(self.repo): - mkdir(self.repo, 0o750, parents=True, uid="admin") + mkdir(self.repo, 0o750, parents=True) # Check free space in output self._check_is_enough_free_space() @@ -2641,9 +2641,9 @@ def _create_archive_dir(): if os.path.lexists(ARCHIVES_PATH): raise YunohostError("backup_output_symlink_dir_broken", path=ARCHIVES_PATH) - # Create the archive folder, with 'admin' as owner, such that + # Create the archive folder, with 'admins' as groupowner, such that # people can scp archives out of the server - mkdir(ARCHIVES_PATH, mode=0o750, parents=True, uid="admin", gid="root") + mkdir(ARCHIVES_PATH, mode=0o770, parents=True, gid="admins") def _call_for_each_path(self, callback, csv_path=None): diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py new file mode 100644 index 000000000..3fa9a2325 --- /dev/null +++ b/src/migrations/0026_new_admins_group.py @@ -0,0 +1,111 @@ +from moulinette.utils.log import getActionLogger + +from yunohost.tools import Migration + +logger = getActionLogger("yunohost.migration") + +################################################### +# Tools used also for restoration +################################################### + + +class MyMigration(Migration): + """ + Add new permissions around SSH/SFTP features + """ + + introduced_in_version = "11.1" # FIXME? + dependencies = [] + + ldap_migration_started = False + + @Migration.ldap_migration + def run(self, *args): + + from yunohost.user import user_list, user_info, user_group_update, user_update + from yunohost.utils.ldap import _get_ldap_interface + from yunohost.permission import permission_sync_to_user + + ldap = _get_ldap_interface() + + all_users = user_list()["users"].keys() + new_admin_user = None + for user in all_users: + if any(alias.startswith("root@") for alias in user_info(user).get("mail-aliases", [])): + new_admin_user = user + break + + self.ldap_migration_started = True + + if new_admin_user: + aliases = user_info(new_admin_user).get("mail-aliases", []) + old_admin_aliases_to_remove = [alias for alias in aliases if any(alias.startswith(a) for a in ["root@", "admin@", "admins@", "webmaster@", "postmaster@", "abuse@"])] + + user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) + + admin_hashs = ldap.search("cn=admin", attrs={"userPassword"})[0]["userPassword"] + + stuff_to_delete = [ + "cn=admin,ou=sudo", + "cn=admin", + "cn=admins,ou=groups", + ] + + for stuff in stuff_to_delete: + if ldap.search(stuff): + ldap.remove(stuff) + + ldap.add( + "cn=admins,ou=sudo", + { + "cn": ["admins"], + "objectClass": ["top", "sudoRole"], + "sudoCommand": ["ALL"], + "sudoUser": ["%admins"], + "sudoHost": ["ALL"], + } + ) + + ldap.add( + "cn=admins,ou=groups", + { + "cn": ["admins"], + "objectClass": ["top", "posixGroup", "groupOfNamesYnh", "mailGroup"], + "gidNumber": ["4001"], + "mail": ["root", "admin", "admins", "webmaster", "postmaster", "abuse"], + } + ) + + permission_sync_to_user() + + if new_admin_user: + user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) + + # Re-add admin as a regular user + attr_dict = { + "objectClass": [ + "mailAccount", + "inetOrgPerson", + "posixAccount", + "userPermissionYnh", + ], + "givenName": ["Admin"], + "sn": ["Admin"], + "displayName": ["Admin"], + "cn": ["Admin"], + "uid": ["admin"], + "mail": "admin_legacy", + "maildrop": ["admin"], + "mailuserquota": ["0"], + "userPassword": admin_hashs, + "gidNumber": ["1007"], + "uidNumber": ["1007"], + "homeDirectory": ["/home/admin"], + "loginShell": ["/bin/bash"], + } + ldap.add("uid=admin,ou=users", attr_dict) + user_group_update(groupname="admins", add="admin", sync_perm=True) + + + def run_after_system_restore(self): + self.run() diff --git a/src/ssh.py b/src/ssh.py index b89dc6c8e..63b122e76 100644 --- a/src/ssh.py +++ b/src/ssh.py @@ -158,15 +158,6 @@ def _get_user_for_ssh(username, attrs=None): "home_path": root_unix.pw_dir, } - if username == "admin": - admin_unix = pwd.getpwnam("admin") - return { - "username": "admin", - "fullname": "", - "mail": "", - "home_path": admin_unix.pw_dir, - } - # TODO escape input using https://www.python-ldap.org/doc/html/ldap-filter.html from yunohost.utils.ldap import _get_ldap_interface diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index a95dea443..db5229342 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -2,7 +2,8 @@ import pytest import os from yunohost.authenticators.ldap_admin import Authenticator as LDAPAuth -from yunohost.tools import tools_adminpw +from yunohost.user import user_create, user_list, user_update, user_delete +from yunohost.domain import _get_maindomain from moulinette import m18n from moulinette.core import MoulinetteError @@ -10,19 +11,52 @@ from moulinette.core import MoulinetteError def setup_function(function): - if os.system("systemctl is-active slapd") != 0: + for u in user_list()["users"]: + user_delete(u, purge=True) + + maindomain = _get_maindomain() + + if os.system("systemctl is-active slapd >/dev/null") != 0: os.system("systemctl start slapd && sleep 3") - tools_adminpw("yunohost", check_strength=False) + user_create("alice", "Alice", "White", maindomain, "Yunohost", admin=True) + user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") + + +def teardown_function(): + + os.system("systemctl is-active slapd >/dev/null || systemctl start slapd; sleep 5") + + for u in user_list()["users"]: + user_delete(u, purge=True) def test_authenticate(): - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") + + +def test_authenticate_with_no_user(): + + with pytest.raises(MoulinetteError): + LDAPAuth().authenticate_credentials(credentials="Yunohost") + + with pytest.raises(MoulinetteError): + LDAPAuth().authenticate_credentials(credentials=":Yunohost") + + +def test_authenticate_with_user_who_is_not_admin(): + + with pytest.raises(MoulinetteError) as exception: + LDAPAuth().authenticate_credentials(credentials="bob:test123Ynh") + + translation = m18n.n("invalid_password") + expected_msg = translation.format() + assert expected_msg in str(exception) def test_authenticate_with_wrong_password(): with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="bad_password_lul") + LDAPAuth().authenticate_credentials(credentials="alice:bad_password_lul") translation = m18n.n("invalid_password") expected_msg = translation.format() @@ -30,30 +64,22 @@ def test_authenticate_with_wrong_password(): def test_authenticate_server_down(mocker): - os.system("systemctl stop slapd && sleep 3") + os.system("systemctl stop slapd && sleep 5") - # Now if slapd is down, moulinette tries to restart it - mocker.patch("os.system") - mocker.patch("time.sleep") - with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="yunohost") - - translation = m18n.n("ldap_server_down") - expected_msg = translation.format() - assert expected_msg in str(exception) + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") def test_authenticate_change_password(): - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") - tools_adminpw("plopette", check_strength=False) + user_update("alice", change_password="plopette") with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") translation = m18n.n("invalid_password") expected_msg = translation.format() assert expected_msg in str(exception) - LDAPAuth().authenticate_credentials(credentials="plopette") + LDAPAuth().authenticate_credentials(credentials="alice:plopette") diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index f2bff5507..379f1cf39 100644 --- a/src/tests/test_permission.py +++ b/src/tests/test_permission.py @@ -109,7 +109,7 @@ def clean_user_groups_permission(): user_delete(u) for g in user_group_list()["groups"]: - if g not in ["all_users", "visitors"]: + if g not in ["all_users", "visitors", "admins"]: user_group_delete(g) for p in user_permission_list()["permissions"]: diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index e561118e0..1a368ceac 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -11,7 +11,6 @@ from yunohost.user import ( user_import, user_export, FIELDS_FOR_IMPORT, - FIRST_ALIASES, user_group_list, user_group_create, user_group_delete, @@ -29,7 +28,7 @@ def clean_user_groups(): user_delete(u, purge=True) for g in user_group_list()["groups"]: - if g not in ["all_users", "visitors"]: + if g not in ["all_users", "visitors", "admins"]: user_group_delete(g) @@ -39,7 +38,7 @@ def setup_function(function): global maindomain maindomain = _get_maindomain() - user_create("alice", "Alice", "White", maindomain, "test123Ynh") + user_create("alice", "Alice", "White", maindomain, "test123Ynh", admin=True) user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") user_create("jack", "Jack", "Black", maindomain, "test123Ynh") @@ -80,6 +79,7 @@ def test_list_groups(): assert "alice" in res assert "bob" in res assert "jack" in res + assert "alice" in res["admins"]["members"] for u in ["alice", "bob", "jack"]: assert u in res assert u in res[u]["members"] @@ -175,10 +175,9 @@ def test_import_user(mocker): def test_export_user(mocker): result = user_export() - aliases = ",".join([alias + maindomain for alias in FIRST_ALIASES]) should_be = ( "username;firstname;lastname;password;mail;mail-alias;mail-forward;mailbox-quota;groups\r\n" - f"alice;Alice;White;;alice@{maindomain};{aliases};;0;dev\r\n" + f"alice;Alice;White;;alice@{maindomain};;;0;admins,dev\r\n" f"bob;Bob;Snow;;bob@{maindomain};;;0;apps\r\n" f"jack;Jack;Black;;jack@{maindomain};;;0;" ) diff --git a/src/tools.py b/src/tools.py index bf2a72ede..ccc2b4a32 100644 --- a/src/tools.py +++ b/src/tools.py @@ -19,10 +19,6 @@ """ -""" yunohost_tools.py - - Specific tools -""" import re import os import subprocess @@ -34,7 +30,7 @@ from typing import List from moulinette import Moulinette, m18n from moulinette.utils.log import getActionLogger from moulinette.utils.process import call_async_output -from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm +from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm, chown from yunohost.app import app_upgrade, app_list from yunohost.app_catalog import ( @@ -64,14 +60,8 @@ def tools_versions(): return ynh_packages_version() -def tools_adminpw(new_password, check_strength=True): - """ - Change admin password +def tools_rootpw(new_password, check_strength=True): - Keyword argument: - new_password - - """ from yunohost.user import _hash_user_password from yunohost.utils.password import ( assert_password_is_strong_enough, @@ -79,48 +69,31 @@ def tools_adminpw(new_password, check_strength=True): ) import spwd + assert_password_is_compatible(new_password) if check_strength: assert_password_is_strong_enough("admin", new_password) - assert_password_is_compatible(new_password) - new_hash = _hash_user_password(new_password) - from yunohost.utils.ldap import _get_ldap_interface - - ldap = _get_ldap_interface() - + # Write as root password try: - ldap.update( - "cn=admin", - {"userPassword": [new_hash]}, - ) - except Exception as e: - logger.error(f"unable to change admin password : {e}") - raise YunohostError("admin_password_change_failed") - else: - # Write as root password - try: - hash_root = spwd.getspnam("root").sp_pwd + hash_root = spwd.getspnam("root").sp_pwd - with open("/etc/shadow", "r") as before_file: - before = before_file.read() + with open("/etc/shadow", "r") as before_file: + before = before_file.read() - with open("/etc/shadow", "w") as after_file: - after_file.write( - before.replace( - "root:" + hash_root, "root:" + new_hash.replace("{CRYPT}", "") - ) + with open("/etc/shadow", "w") as after_file: + after_file.write( + before.replace( + "root:" + hash_root, "root:" + new_hash.replace("{CRYPT}", "") ) - # An IOError may be thrown if for some reason we can't read/write /etc/passwd - # A KeyError could also be thrown if 'root' is not in /etc/passwd in the first place (for example because no password defined ?) - # (c.f. the line about getspnam) - except (IOError, KeyError): - logger.warning(m18n.n("root_password_desynchronized")) - return - - logger.info(m18n.n("root_password_replaced_by_admin_password")) - logger.success(m18n.n("admin_password_changed")) + ) + # An IOError may be thrown if for some reason we can't read/write /etc/passwd + # A KeyError could also be thrown if 'root' is not in /etc/passwd in the first place (for example because no password defined ?) + # (c.f. the line about getspnam) + except (IOError, KeyError): + logger.warning(m18n.n("root_password_desynchronized")) + return def tools_maindomain(new_main_domain=None): @@ -172,21 +145,14 @@ def _set_hostname(hostname, pretty_hostname=None): def tools_postinstall( operation_logger, domain, + username, + firstname, + lastname, password, ignore_dyndns=False, - force_password=False, force_diskspace=False, ): - """ - YunoHost post-install - Keyword argument: - domain -- YunoHost main domain - ignore_dyndns -- Do not subscribe domain to a DynDNS service (only - needed for nohost.me, noho.st domains) - password -- YunoHost admin password - - """ from yunohost.dyndns import _dyndns_available from yunohost.utils.dns import is_yunohost_dyndns_domain from yunohost.utils.password import ( @@ -194,6 +160,7 @@ def tools_postinstall( assert_password_is_compatible, ) from yunohost.domain import domain_main_domain + from yunohost.user import user_create import psutil # Do some checks at first @@ -220,9 +187,7 @@ def tools_postinstall( # Check password assert_password_is_compatible(password) - - if not force_password: - assert_password_is_strong_enough("admin", password) + assert_password_is_strong_enough("admin", password) # If this is a nohost.me/noho.st, actually check for availability if not ignore_dyndns and is_yunohost_dyndns_domain(domain): @@ -258,8 +223,10 @@ def tools_postinstall( domain_add(domain, dyndns) domain_main_domain(domain) + user_create(username, firstname, lastname, domain, password, admin=True) + # Update LDAP admin and create home dir - tools_adminpw(password, check_strength=not force_password) + tools_rootpw(password) # Enable UPnP silently and reload firewall firewall_upnp("enable", no_refresh=True) @@ -971,7 +938,7 @@ class Migration: def description(self): return m18n.n(f"migration_description_{self.id}") - def ldap_migration(self, run): + def ldap_migration(run): def func(self): # Backup LDAP before the migration @@ -999,22 +966,28 @@ class Migration: try: run(self, backup_folder) except Exception: - logger.warning( - m18n.n("migration_ldap_migration_failed_trying_to_rollback") - ) - os.system("systemctl stop slapd") - # To be sure that we don't keep some part of the old config - rm("/etc/ldap/slapd.d", force=True, recursive=True) - cp(f"{backup_folder}/ldap_config", "/etc/ldap", recursive=True) - cp(f"{backup_folder}/ldap_db", "/var/lib/ldap", recursive=True) - cp( - f"{backup_folder}/apps_settings", - "/etc/yunohost/apps", - recursive=True, - ) - os.system("systemctl start slapd") - rm(backup_folder, force=True, recursive=True) - logger.info(m18n.n("migration_ldap_rollback_success")) + if self.ldap_migration_started: + logger.warning( + m18n.n("migration_ldap_migration_failed_trying_to_rollback") + ) + os.system("systemctl stop slapd") + # To be sure that we don't keep some part of the old config + rm("/etc/ldap", force=True, recursive=True) + cp(f"{backup_folder}/ldap_config", "/etc/ldap", recursive=True) + chown("/etc/ldap/schema/", "openldap", "openldap", recursive=True) + chown("/etc/ldap/slapd.d/", "openldap", "openldap", recursive=True) + rm("/var/lib/ldap", force=True, recursive=True) + cp(f"{backup_folder}/ldap_db", "/var/lib/ldap", recursive=True) + rm("/etc/yunohost/apps", force=True, recursive=True) + chown("/var/lib/ldap/", "openldap", recursive=True) + cp( + f"{backup_folder}/apps_settings", + "/etc/yunohost/apps", + recursive=True, + ) + os.system("systemctl start slapd") + rm(backup_folder, force=True, recursive=True) + logger.info(m18n.n("migration_ldap_rollback_success")) raise else: rm(backup_folder, force=True, recursive=True) diff --git a/src/user.py b/src/user.py index 81b20f15f..e00fa3685 100644 --- a/src/user.py +++ b/src/user.py @@ -56,7 +56,7 @@ FIELDS_FOR_IMPORT = { "groups": r"^|([a-z0-9_]+(,?[a-z0-9_]+)*)$", } -FIRST_ALIASES = ["root@", "admin@", "webmaster@", "postmaster@", "abuse@"] +ADMIN_ALIASES = ["root@", "admin@", "admins", "webmaster@", "postmaster@", "abuse@"] def user_list(fields=None): @@ -139,6 +139,7 @@ def user_create( domain, password, mailbox_quota="0", + admin=False, from_import=False, ): @@ -152,7 +153,7 @@ def user_create( # Ensure compatibility and sufficiently complex password assert_password_is_compatible(password) - assert_password_is_strong_enough("user", password) + assert_password_is_strong_enough("admin" if admin else "user", password) # Validate domain used for email address/xmpp account if domain is None: @@ -194,9 +195,10 @@ def user_create( raise YunohostValidationError("system_username_exists") main_domain = _get_maindomain() - aliases = [alias + main_domain for alias in FIRST_ALIASES] + # FIXME: should forbit root@any.domain, not just main domain? + admin_aliases = [alias + main_domain for alias in ADMIN_ALIASES] - if mail in aliases: + if mail in admin_aliases: raise YunohostValidationError("mail_unavailable") if not from_import: @@ -206,6 +208,11 @@ def user_create( all_uid = {str(x.pw_uid) for x in pwd.getpwall()} all_gid = {str(x.gr_gid) for x in grp.getgrall()} + # Prevent users from obtaining uid 1007 which is the uid of the legacy admin, + # and there could be a edge case where a new user becomes owner of an old, removed admin user + all_uid.add("1007") + all_gid.add("1007") + uid_guid_found = False while not uid_guid_found: # LXC uid number is limited to 65536 by default @@ -237,10 +244,6 @@ def user_create( "loginShell": ["/bin/bash"], } - # If it is the first user, add some aliases - if not ldap.search(base="ou=users", filter="uid=*"): - attr_dict["mail"] = [attr_dict["mail"]] + aliases - try: ldap.add(f"uid={username},ou=users", attr_dict) except Exception as e: @@ -266,6 +269,8 @@ def user_create( # Create group for user and add to group 'all_users' user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False) user_group_update(groupname="all_users", add=username, force=True, sync_perm=True) + if admin: + user_group_update(groupname="admins", add=username, sync_perm=True) # Trigger post_user_create hooks env_dict = { @@ -381,7 +386,7 @@ def user_update( # Populate user informations ldap = _get_ldap_interface() - attrs_to_fetch = ["givenName", "sn", "mail", "maildrop"] + attrs_to_fetch = ["givenName", "sn", "mail", "maildrop", "memberOf"] result = ldap.search( base="ou=users", filter="uid=" + username, @@ -422,16 +427,17 @@ def user_update( change_password = Moulinette.prompt( m18n.n("ask_password"), is_password=True, confirm=True ) + # Ensure compatibility and sufficiently complex password assert_password_is_compatible(change_password) - assert_password_is_strong_enough("user", change_password) + is_admin = "cn=admins,ou=groups,dc=yunohost,dc=org" in user["memberOf"] + assert_password_is_strong_enough("admin" if is_admin else "user", change_password) new_attr_dict["userPassword"] = [_hash_user_password(change_password)] env_dict["YNH_USER_PASSWORD"] = change_password if mail: main_domain = _get_maindomain() - aliases = [alias + main_domain for alias in FIRST_ALIASES] # If the requested mail address is already as main address or as an alias by this user if mail in user["mail"]: @@ -446,6 +452,9 @@ def user_update( raise YunohostError( "mail_domain_unknown", domain=mail[mail.find("@") + 1 :] ) + + # FIXME: should also forbid root@any.domain and not just the main domain + aliases = [alias + main_domain for alias in ADMIN_ALIASES] if mail in aliases: raise YunohostValidationError("mail_unavailable") @@ -1071,7 +1080,7 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True): # # We also can't delete "all_users" because that's a special group... existing_users = list(user_list()["users"].keys()) - undeletable_groups = existing_users + ["all_users", "visitors"] + undeletable_groups = existing_users + ["all_users", "visitors", "admins"] if groupname in undeletable_groups and not force: raise YunohostValidationError("group_cannot_be_deleted", group=groupname) diff --git a/src/utils/config.py b/src/utils/config.py index d2bd5bc36..57a33606e 100644 --- a/src/utils/config.py +++ b/src/utils/config.py @@ -1184,6 +1184,8 @@ class UserQuestion(Question): ) if self.default is None: + # FIXME: this code is obsolete with the new admins group + # Should be replaced by something like "any first user we find in the admin group" root_mail = "root@%s" % _get_maindomain() for user in self.choices.keys(): if root_mail in user_info(user).get("mail-aliases", []): diff --git a/src/utils/ldap.py b/src/utils/ldap.py index 98c0fecf7..627ab4e7a 100644 --- a/src/utils/ldap.py +++ b/src/utils/ldap.py @@ -84,7 +84,7 @@ class LDAPInterface: def connect(self): def _reconnect(): con = ldap.ldapobject.ReconnectLDAPObject( - self.uri, retry_max=10, retry_delay=0.5 + self.uri, retry_max=10, retry_delay=2 ) con.sasl_non_interactive_bind_s("EXTERNAL") return con @@ -145,6 +145,8 @@ class LDAPInterface: try: result = self.con.search_s(base, ldap.SCOPE_SUBTREE, filter, attrs) + except ldap.SERVER_DOWN as e: + raise e except Exception as e: raise MoulinetteError( "error during LDAP search operation with: base='%s', "