Merge pull request #606 from YunoHost/sftp_permission

Add SFTP / SSH permissions

data/actionsmap/yunohost.yml

@@ -379,25 +379,6 @@ user:
         ssh:
             subcategory_help: Manage ssh access
             actions:
-                ### user_ssh_enable()
-                allow:
-                    action_help: Allow the user to uses ssh
-                    api: POST /users/ssh/enable
-                    arguments:
-                        username:
-                            help: Username of the user
-                            extra:
-                                pattern: *pattern_username
-
-                ### user_ssh_disable()
-                disallow:
-                    action_help: Disallow the user to uses ssh
-                    api: POST /users/ssh/disable
-                    arguments:
-                        username:
-                            help: Username of the user
-                            extra:
-                                pattern: *pattern_username
 
                 ### user_ssh_keys_list()
                 list-keys:
@@ -436,7 +417,6 @@ user:
                         key:
                             help: The key to be removed
 
-
 #############################
 #          Domain           #
 #############################

data/hooks/conf_regen/03-ssh

@@ -25,9 +25,7 @@ do_pre_regen() {
 
     # Support different strategy for security configurations
     export compatibility="$(yunohost settings get 'security.ssh.compatibility')"
-
     export port="$(yunohost settings get 'security.ssh.port')"
-
     export ssh_keys
     export ipv6_enabled
     ynh_render_template "sshd_config" "${pending_dir}/etc/ssh/sshd_config"

data/hooks/diagnosis/70-regenconf.py

@@ -1,10 +1,12 @@
 #!/usr/bin/env python
 
 import os
+import re
 
+from yunohost.settings import settings_get
 from yunohost.diagnosis import Diagnoser
 from yunohost.regenconf import _get_regenconf_infos, _calculate_hash
-
+from moulinette.utils.filesystem import read_file
 
 class RegenconfDiagnoser(Diagnoser):
 
@@ -35,6 +37,31 @@ class RegenconfDiagnoser(Diagnoser):
                     details=["diagnosis_regenconf_manually_modified_details"],
                 )
 
+        if any(f["path"] == '/etc/ssh/sshd_config' for f in regenconf_modified_files) \
+            and os.system("grep -q '^ *AllowGroups\\|^ *AllowUsers' /etc/ssh/sshd_config") != 0:
+                yield dict(
+                   meta={
+                       "test": "sshd_config_insecure"
+                   },
+                   status="ERROR",
+                   summary="diagnosis_sshd_config_insecure",
+                )
+
+        # Check consistency between actual ssh port in sshd_config vs. setting
+        ssh_port_setting = settings_get('security.ssh.port')
+        ssh_port_line = re.findall(
+            r"\bPort *([0-9]{2,5})\b", read_file("/etc/ssh/sshd_config")
+        )
+        if len(ssh_port_line) == 1 and int(ssh_port_line[0]) != ssh_port_setting:
+            yield dict(
+               meta={
+                   "test": "sshd_config_port_inconsistency"
+               },
+               status="WARNING",
+               summary="diagnosis_sshd_config_inconsistent",
+               details=["diagnosis_sshd_config_inconsistent_details"],
+            )
+
     def manually_modified_files(self):
 
         for category, infos in _get_regenconf_infos().items():

data/other/ldap_scheme.yml

@@ -89,3 +89,25 @@ depends_children:
         label: "XMPP"
         showTile: "FALSE"
         isProtected: "TRUE"
+    cn=ssh.main,ou=permission:
+        cn: ssh.main
+        gidNumber: "5003"
+        objectClass:
+            - posixGroup
+            - permissionYnh
+        groupPermission: []
+        authHeader: "FALSE"
+        label: "SSH"
+        showTile: "FALSE"
+        isProtected: "TRUE"
+    cn=sftp.main,ou=permission:
+        cn: sftp.main
+        gidNumber: "5004"
+        objectClass:
+            - posixGroup
+            - permissionYnh
+        groupPermission: []
+        authHeader: "FALSE"
+        label: "SFTP"
+        showTile: "FALSE"
+        isProtected: "TRUE"

data/templates/ssh/sshd_config

@@ -64,20 +64,31 @@ PrintLastLog yes
 ClientAliveInterval 60
 AcceptEnv LANG LC_*
 
+# Disallow user without ssh or sftp permissions
+AllowGroups ssh.main sftp.main admins root
+
+# Allow users to create tunnels or forwarding
+AllowTcpForwarding yes
+AllowStreamLocalForwarding yes
+PermitTunnel yes
+PermitUserRC yes
+
 # SFTP stuff
 Subsystem sftp internal-sftp
 
-# Forbid users from using their account SSH as a VPN (even if SSH login is disabled)
-AllowTcpForwarding no
-AllowStreamLocalForwarding no
-
-# Disable .ssh/rc, which could be edited (e.g. from Nextcloud or whatever) by users to execute arbitrary commands even if SSH login is disabled
-PermitUserRC no
-
-Match User admin,root
-    AllowTcpForwarding yes
-    AllowStreamLocalForwarding yes
-    PermitUserRC yes
+# Apply following instructions to user with sftp perm only
+Match Group sftp.main,!ssh.main
+    ForceCommand internal-sftp
+    # We can't restrict to /home/%u because the chroot base must be owned by root
+    # So we chroot only on /home
+    # See https://serverfault.com/questions/584986/bad-ownership-or-modes-for-chroot-directory-component
+    ChrootDirectory /home
+    # Forbid SFTP users from using their account SSH as a VPN (even if SSH login is disabled)
+    AllowTcpForwarding no
+    AllowStreamLocalForwarding no
+    PermitTunnel no
+    # Disable .ssh/rc, which could be edited (e.g. from Nextcloud or whatever) by users to execute arbitrary commands even if SSH login is disabled
+    PermitUserRC no
 
 
 # root login is allowed on local networks
@@ -86,4 +97,4 @@ Match User admin,root
 # If the server is a VPS, it's expected that the owner of the
 # server has access to a web console through which to log in.
 Match Address 192.168.0.0/16,10.0.0.0/8,172.16.0.0/12,169.254.0.0/16,fe80::/10,fd00::/8
-        PermitRootLogin yes
+    PermitRootLogin yes

locales/en.json

@@ -269,6 +269,9 @@
     "diagnosis_unknown_categories": "The following categories are unknown: {categories}",
     "diagnosis_never_ran_yet": "It looks like this server was setup recently and there's no diagnosis report to show yet. You should start by running a full diagnosis, either from the webadmin or using 'yunohost diagnosis run' from the command line.",
     "diagnosis_processes_killed_by_oom_reaper": "Some processes were recently killed by the system because it ran out of memory. This is typically symptomatic of a lack of memory on the system or of a process that ate up to much memory. Summary of the processes killed:\n{kills_summary}",
+    "diagnosis_sshd_config_insecure": "The SSH configuration appears to have been manually modified, and is insecure because it contains no 'AllowGroups' or 'AllowUsers' directive to limit access to authorized users.",
+    "diagnosis_sshd_config_inconsistent": "It looks like the SSH port was manually modified in /etc/ssh/sshd_config. Since Yunohost 4.2, a new global setting 'security.ssh.port' is available to avoid manually editing the configuration.",
+    "diagnosis_sshd_config_inconsistent_details": "Please run <cmd>yunohost settings set security.ssh.port -v YOUR_SSH_PORT</cmd> to define the SSH port, and check <cmd>yunohost tools regen-conf ssh --dry-run --with-diff</cmd> and <cmd>yunohost tools regen-conf ssh --force</cmd> to reset your conf to the Yunohost recommendation.",
     "domain_cannot_remove_main": "You cannot remove '{domain:s}' since it's the main domain, you first need to set another domain as the main domain using 'yunohost domain main-domain -n <another-domain>'; here is the list of candidate domains: {other_domains:s}",
     "domain_cannot_add_xmpp_upload": "You cannot add domains starting with 'xmpp-upload.'. This kind of name is reserved for the XMPP upload feature integrated in YunoHost.",
     "domain_cannot_remove_main_add_new_one": "You cannot remove '{domain:s}' since it's the main domain and your only domain, you need to first add another domain using 'yunohost domain add <another-domain.com>', then set is as the main domain using 'yunohost domain main-domain -n <another-domain.com>' and then you can remove the domain '{domain:s}' using 'yunohost domain remove {domain:s}'.'",
@@ -424,6 +427,11 @@
     "migration_description_0017_postgresql_9p6_to_11": "Migrate databases from PostgreSQL 9.6 to 11",
     "migration_description_0018_xtable_to_nftable": "Migrate old network traffic rules to the new nftable system",
     "migration_description_0019_extend_permissions_features": "Extend/rework the app permission management system",
+    "migration_description_0020_ssh_sftp_permissions": "Add SSH and SFTP permissions support",
+    "migration_ldap_backup_before_migration": "Creating a backup of LDAP database and apps settings prior to the actual migration.",
+    "migration_ldap_can_not_backup_before_migration": "The backup of the system could not be completed before the migration failed. Error: {error:s}",
+    "migration_ldap_migration_failed_trying_to_rollback": "Could not migrate... trying to roll back the system.",
+    "migration_ldap_rollback_success": "System rolled back.",
     "migration_update_LDAP_schema": "Updating LDAP schema...",
     "migration_0015_start" : "Starting migration to Buster",
     "migration_0015_patching_sources_list": "Patching the sources.lists...",
@@ -445,10 +453,6 @@
     "migration_0018_failed_to_migrate_iptables_rules": "Failed to migrate legacy iptables rules to nftables: {error}",
     "migration_0018_failed_to_reset_legacy_rules": "Failed to reset legacy iptables rules: {error}",
     "migration_0019_add_new_attributes_in_ldap": "Add new attributes for permissions in LDAP database",
-    "migration_0019_backup_before_migration": "Creating a backup of LDAP database and apps settings prior to the actual migration.",
-    "migration_0019_can_not_backup_before_migration": "The backup of the system could not be completed before the migration failed. Error: {error:s}",
-    "migration_0019_migration_failed_trying_to_rollback": "Could not migrate... trying to roll back the system.",
-    "migration_0019_rollback_success": "System rolled back.",
     "migration_0019_slapd_config_will_be_overwritten": "It looks like you manually edited the slapd configuration. For this critical migration, YunoHost needs to force the update of the slapd configuration. The original files will be backuped in {conf_backup_folder}.",
     "migrations_already_ran": "Those migrations are already done: {ids}",
     "migrations_cant_reach_migration_file": "Could not access migrations files at the path '%s'",
@@ -497,6 +501,7 @@
     "permission_created": "Permission '{permission:s}' created",
     "permission_creation_failed": "Could not create permission '{permission}': {error}",
     "permission_currently_allowed_for_all_users": "This permission is currently granted to all users in addition to other groups. You probably want to either remove the 'all_users' permission or remove the other groups it is currently granted to.",
+    "permission_cant_add_to_all_users": "The permission {permission} can not be added to all users.",
     "permission_deleted": "Permission '{permission:s}' deleted",
     "permission_deletion_failed": "Could not delete permission '{permission}': {error}",
     "permission_not_found": "Permission '{permission:s}' not found",

src/yunohost/data_migrations/0019_extend_permissions_features.py

@@ -1,8 +1,4 @@
-import time
-import os
-
 from moulinette import m18n
-from yunohost.utils.error import YunohostError
 from moulinette.utils.log import getActionLogger
 
 from yunohost.tools import Migration
@@ -17,7 +13,14 @@ class MyMigration(Migration):
     Add protected attribute in LDAP permission
     """
 
-    required = True
+    @Migration.ldap_migration
+    def run(self, backup_folder):
+
+        # Update LDAP database
+        self.add_new_ldap_attributes()
+
+        # Migrate old settings
+        migrate_legacy_permission_settings()
 
     def add_new_ldap_attributes(self):
 
@@ -84,7 +87,7 @@ class MyMigration(Migration):
         # Update LDAP database
         self.add_new_ldap_attributes()
 
-    def run_before_system_restore(self, app_id):
+    def run_before_app_restore(self, app_id):
         from yunohost.app import app_setting
         from yunohost.utils.legacy import migrate_legacy_permission_settings
 
@@ -102,56 +105,3 @@ class MyMigration(Migration):
             for setting in legacy_permission_settings
         ):
             migrate_legacy_permission_settings(app=app_id)
-
-
-    def run(self):
-
-        # FIXME : what do we really want to do here ...
-        # Imho we should just force-regen the conf in all case, and maybe
-        # just display a warning if we detect that the conf was manually modified
-
-        # Backup LDAP and the apps settings before to do the migration
-        logger.info(m18n.n("migration_0019_backup_before_migration"))
-        try:
-            backup_folder = "/home/yunohost.backup/premigration/" + time.strftime(
-                "%Y%m%d-%H%M%S", time.gmtime()
-            )
-            os.makedirs(backup_folder, 0o750)
-            os.system("systemctl stop slapd")
-            os.system("cp -r --preserve /etc/ldap %s/ldap_config" % backup_folder)
-            os.system("cp -r --preserve /var/lib/ldap %s/ldap_db" % backup_folder)
-            os.system(
-                "cp -r --preserve /etc/yunohost/apps %s/apps_settings" % backup_folder
-            )
-        except Exception as e:
-            raise YunohostError(
-                "migration_0019_can_not_backup_before_migration", error=e
-            )
-        finally:
-            os.system("systemctl start slapd")
-
-        try:
-            # Update LDAP database
-            self.add_new_ldap_attributes()
-
-            # Migrate old settings
-            migrate_legacy_permission_settings()
-
-        except Exception:
-            logger.warn(m18n.n("migration_0019_migration_failed_trying_to_rollback"))
-            os.system("systemctl stop slapd")
-            os.system(
-                "rm -r /etc/ldap/slapd.d"
-            )  # To be sure that we don't keep some part of the old config
-            os.system("cp -r --preserve %s/ldap_config/. /etc/ldap/" % backup_folder)
-            os.system("cp -r --preserve %s/ldap_db/. /var/lib/ldap/" % backup_folder)
-            os.system(
-                "cp -r --preserve %s/apps_settings/. /etc/yunohost/apps/"
-                % backup_folder
-            )
-            os.system("systemctl start slapd")
-            os.system("rm -r " + backup_folder)
-            logger.info(m18n.n("migration_0019_rollback_success"))
-            raise
-        else:
-            os.system("rm -r " + backup_folder)

src/yunohost/data_migrations/0020_ssh_sftp_permissions.py

@@ -0,0 +1,69 @@
+import subprocess
+import os
+
+from moulinette import m18n
+from moulinette.utils.log import getActionLogger
+from moulinette.utils.filesystem import read_yaml
+
+from yunohost.tools import Migration
+from yunohost.permission import user_permission_update, permission_sync_to_user
+from yunohost.regenconf import manually_modified_files
+
+logger = getActionLogger('yunohost.migration')
+
+###################################################
+# Tools used also for restoration
+###################################################
+
+
+class MyMigration(Migration):
+    """
+        Add new permissions around SSH/SFTP features
+    """
+
+    introduced_in_version = "4.2.2"
+    dependencies = ["extend_permissions_features"]
+
+    @Migration.ldap_migration
+    def run(self, *args):
+
+        from yunohost.utils.ldap import _get_ldap_interface
+        ldap = _get_ldap_interface()
+
+        existing_perms_raw = ldap.search("ou=permission,dc=yunohost,dc=org", "(objectclass=permissionYnh)", ["cn"])
+        existing_perms = [perm['cn'][0] for perm in existing_perms_raw]
+
+        # Add SSH and SFTP permissions
+        ldap_map = read_yaml('/usr/share/yunohost/yunohost-config/moulinette/ldap_scheme.yml')
+
+        if "sftp.main" not in existing_perms:
+            ldap.add("cn=sftp.main,ou=permission", ldap_map['depends_children']["cn=sftp.main,ou=permission"])
+
+        if "ssh.main" not in existing_perms:
+            ldap.add("cn=ssh.main,ou=permission", ldap_map['depends_children']["cn=ssh.main,ou=permission"])
+
+            # Add a bash terminal to each users
+            users = ldap.search('ou=users,dc=yunohost,dc=org', filter="(loginShell=*)", attrs=["dn", "uid", "loginShell"])
+            for user in users:
+                if user['loginShell'][0] == '/bin/false':
+                    dn = user['dn'][0].replace(',dc=yunohost,dc=org', '')
+                    ldap.update(dn, {'loginShell': ['/bin/bash']})
+                else:
+                    user_permission_update("ssh.main", add=user["uid"][0], sync_perm=False)
+
+            permission_sync_to_user()
+
+            # Somehow this is needed otherwise the PAM thing doesn't forget about the
+            # old loginShell value ?
+            subprocess.call(['nscd', '-i', 'passwd'])
+
+        if '/etc/ssh/sshd_config' in manually_modified_files() \
+            and os.system("grep -q '^ *AllowGroups\\|^ *AllowUsers' /etc/ssh/sshd_config") != 0:
+                logger.error(m18n.n('diagnosis_sshd_config_insecure'))
+
+    def run_after_system_restore(self):
+        self.run()
+
+    def run_before_app_restore(self, app_id):
+        # Nothing to do during app backup restore for this migration
+        pass

src/yunohost/permission.py

@@ -188,6 +188,10 @@ def user_permission_update(
     ) and not force:
         raise YunohostValidationError("permission_protected", permission=permission)
 
+    # Refuse to add "all_users" to ssh/sftp permissions
+    if permission.split(".")[0] in ["ssh", "sftp"] and (add and "all_users" in add) and not force:
+        raise YunohostValidationError("permission_cant_add_to_all_users", permission=permission)
+
     # Fetch currently allowed groups for this permission
 
     current_allowed_groups = existing_permission["allowed"]

src/yunohost/ssh.py

@@ -3,7 +3,6 @@
 import re
 import os
 import pwd
-import subprocess
 
 from yunohost.utils.error import YunohostValidationError
 from moulinette.utils.filesystem import read_file, write_to_file, chown, chmod, mkdir
@@ -11,54 +10,10 @@ from moulinette.utils.filesystem import read_file, write_to_file, chown, chmod,
 SSHD_CONFIG_PATH = "/etc/ssh/sshd_config"
 
 
-def user_ssh_allow(username):
-    """
-    Allow YunoHost user connect as ssh.
-
-    Keyword argument:
-        username -- User username
-    """
-    # TODO it would be good to support different kind of shells
-
-    if not _get_user_for_ssh(username):
-        raise YunohostValidationError("user_unknown", user=username)
-
-    from yunohost.utils.ldap import _get_ldap_interface
-
-    ldap = _get_ldap_interface()
-    ldap.update("uid=%s,ou=users" % username, {"loginShell": ["/bin/bash"]})
-
-    # Somehow this is needed otherwise the PAM thing doesn't forget about the
-    # old loginShell value ?
-    subprocess.call(["nscd", "-i", "passwd"])
-
-
-def user_ssh_disallow(username):
-    """
-    Disallow YunoHost user connect as ssh.
-
-    Keyword argument:
-        username -- User username
-    """
-    # TODO it would be good to support different kind of shells
-
-    if not _get_user_for_ssh(username):
-        raise YunohostValidationError("user_unknown", user=username)
-
-    from yunohost.utils.ldap import _get_ldap_interface
-
-    ldap = _get_ldap_interface()
-    ldap.update("uid=%s,ou=users" % username, {"loginShell": ["/bin/false"]})
-
-    # Somehow this is needed otherwise the PAM thing doesn't forget about the
-    # old loginShell value ?
-    subprocess.call(["nscd", "-i", "passwd"])
-
-
 def user_ssh_list_keys(username):
     user = _get_user_for_ssh(username, ["homeDirectory"])
     if not user:
-        raise Exception("User with username '%s' doesn't exists" % username)
+        raise YunohostValidationError("user_unknown", user=username)
 
     authorized_keys_file = os.path.join(
         user["homeDirectory"][0], ".ssh", "authorized_keys"
@@ -95,7 +50,7 @@ def user_ssh_list_keys(username):
 def user_ssh_add_key(username, key, comment):
     user = _get_user_for_ssh(username, ["homeDirectory", "uid"])
     if not user:
-        raise Exception("User with username '%s' doesn't exists" % username)
+        raise YunohostValidationError("user_unknown", user=username)
 
     authorized_keys_file = os.path.join(
         user["homeDirectory"][0], ".ssh", "authorized_keys"
@@ -135,21 +90,26 @@ def user_ssh_add_key(username, key, comment):
 def user_ssh_remove_key(username, key):
     user = _get_user_for_ssh(username, ["homeDirectory", "uid"])
     if not user:
-        raise Exception("User with username '%s' doesn't exists" % username)
+        raise YunohostValidationError("user_unknown", user=username)
 
     authorized_keys_file = os.path.join(
         user["homeDirectory"][0], ".ssh", "authorized_keys"
     )
 
     if not os.path.exists(authorized_keys_file):
-        raise Exception(
-            "this key doesn't exists ({} dosesn't exists)".format(authorized_keys_file)
+        raise YunohostValidationError(
+            "this key doesn't exists ({} dosesn't exists)".format(authorized_keys_file),
+            raw_msg=True
         )
 
     authorized_keys_content = read_file(authorized_keys_file)
 
     if key not in authorized_keys_content:
-        raise Exception("Key '{}' is not present in authorized_keys".format(key))
+        raise YunohostValidationError(
+            "Key '{}' is not present in authorized_keys".format(key),
+            raw_msg=True
+        )
+
 
     # don't delete the previous comment because we can't verify if it's legit
 
@@ -196,8 +156,6 @@ def _get_user_for_ssh(username, attrs=None):
             "username": "root",
             "fullname": "",
             "mail": "",
-            "ssh_allowed": ssh_root_login_status()["PermitRootLogin"],
-            "shell": root_unix.pw_shell,
             "home_path": root_unix.pw_dir,
         }
 
@@ -207,8 +165,6 @@ def _get_user_for_ssh(username, attrs=None):
             "username": "admin",
             "fullname": "",
             "mail": "",
-            "ssh_allowed": admin_unix.pw_shell.strip() != "/bin/false",
-            "shell": admin_unix.pw_shell,
             "home_path": admin_unix.pw_dir,
         }
 

src/yunohost/tests/test_apps_arguments_parsing.py

@@ -1206,7 +1206,6 @@ def test_parse_args_in_yunohost_format_user_empty():
         "some_user": {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1232,7 +1231,6 @@ def test_parse_args_in_yunohost_format_user():
         username: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1261,7 +1259,6 @@ def test_parse_args_in_yunohost_format_user_two_users():
         username: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1269,7 +1266,6 @@ def test_parse_args_in_yunohost_format_user_two_users():
         other_user: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "z@ynh.local",
             "fullname": "john doe",
@@ -1304,7 +1300,6 @@ def test_parse_args_in_yunohost_format_user_two_users_wrong_answer():
         username: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1312,7 +1307,6 @@ def test_parse_args_in_yunohost_format_user_two_users_wrong_answer():
         other_user: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "z@ynh.local",
             "fullname": "john doe",
@@ -1339,7 +1333,6 @@ def test_parse_args_in_yunohost_format_user_two_users_no_default():
         username: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1347,7 +1340,6 @@ def test_parse_args_in_yunohost_format_user_two_users_no_default():
         other_user: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "z@ynh.local",
             "fullname": "john doe",
@@ -1369,7 +1361,6 @@ def test_parse_args_in_yunohost_format_user_two_users_default_input():
         username: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "p@ynh.local",
             "fullname": "the first name the last name",
@@ -1377,7 +1368,6 @@ def test_parse_args_in_yunohost_format_user_two_users_default_input():
         other_user: {
             "ssh_allowed": False,
             "username": "some_user",
-            "shell": "/bin/false",
             "mailbox-quota": "0",
             "mail": "z@ynh.local",
             "fullname": "john doe",

src/yunohost/tools.py

@@ -28,6 +28,7 @@ import os
 import yaml
 import subprocess
 import pwd
+import time
 from importlib import import_module
 from packaging import version
 
@@ -1121,9 +1122,15 @@ def _tools_migrations_run_after_system_restore(backup_version):
 
     all_migrations = _get_migrations_list()
 
+    current_version = version.parse(ynh_packages_version()["yunohost"]["version"])
+    backup_version = version.parse(backup_version)
+
+    if backup_version == current_version:
+        return
+
     for migration in all_migrations:
         if hasattr(migration, "introduced_in_version") \
-           and version.parse(migration.introduced_in_version) > version.parse(backup_version) \
+           and version.parse(migration.introduced_in_version) > backup_version \
            and hasattr(migration, "run_after_system_restore"):
             try:
                 logger.info(m18n.n("migrations_running_forward", id=migration.id))
@@ -1140,9 +1147,15 @@ def _tools_migrations_run_before_app_restore(backup_version, app_id):
 
     all_migrations = _get_migrations_list()
 
+    current_version = version.parse(ynh_packages_version()["yunohost"]["version"])
+    backup_version = version.parse(backup_version)
+
+    if backup_version == current_version:
+        return
+
     for migration in all_migrations:
         if hasattr(migration, "introduced_in_version") \
-           and version.parse(migration.introduced_in_version) > version.parse(backup_version) \
+           and version.parse(migration.introduced_in_version) > backup_version \
            and hasattr(migration, "run_before_app_restore"):
             try:
                 logger.info(m18n.n("migrations_running_forward", id=migration.id))
@@ -1154,7 +1167,6 @@ def _tools_migrations_run_before_app_restore(backup_version, app_id):
                 logger.error(msg, exc_info=1)
                 raise
 
-
 class Migration(object):
 
     # Those are to be implemented by daughter classes
@@ -1179,3 +1191,44 @@ class Migration(object):
     @property
     def description(self):
         return m18n.n("migration_description_%s" % self.id)
+
+    def ldap_migration(run):
+
+        def func(self):
+
+            # Backup LDAP before the migration
+            logger.info(m18n.n("migration_ldap_backup_before_migration"))
+            try:
+                backup_folder = "/home/yunohost.backup/premigration/" + time.strftime(
+                    "%Y%m%d-%H%M%S", time.gmtime()
+                )
+                os.makedirs(backup_folder, 0o750)
+                os.system("systemctl stop slapd")
+                os.system(f"cp -r --preserve /etc/ldap {backup_folder}/ldap_config")
+                os.system(f"cp -r --preserve /var/lib/ldap {backup_folder}/ldap_db")
+                os.system(f"cp -r --preserve /etc/yunohost/apps {backup_folder}/apps_settings")
+            except Exception as e:
+                raise YunohostError(
+                    "migration_ldap_can_not_backup_before_migration", error=str(e)
+                )
+            finally:
+                os.system("systemctl start slapd")
+
+            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
+                os.system("rm -r /etc/ldap/slapd.d")
+                os.system(f"cp -r --preserve {backup_folder}/ldap_config/. /etc/ldap/")
+                os.system(f"cp -r --preserve {backup_folder}/ldap_db/. /var/lib/ldap/")
+                os.system(f"cp -r --preserve {backup_folder}/apps_settings/. /etc/yunohost/apps/")
+                os.system("systemctl start slapd")
+                os.system(f"rm -r {backup_folder}")
+                logger.info(m18n.n("migration_ldap_rollback_success"))
+                raise
+            else:
+                os.system(f"rm -r {backup_folder}")
+
+        return func

src/yunohost/user.py

@@ -53,7 +53,6 @@ def user_list(fields=None):
         "cn": "fullname",
         "mail": "mail",
         "maildrop": "mail-forward",
-        "loginShell": "shell",
         "homeDirectory": "home_path",
         "mailuserquota": "mailbox-quota",
     }
@@ -69,7 +68,7 @@ def user_list(fields=None):
             else:
                 raise YunohostError("field_invalid", attr)
     else:
-        attrs = ["uid", "cn", "mail", "mailuserquota", "loginShell"]
+        attrs = ["uid", "cn", "mail", "mailuserquota"]
 
     ldap = _get_ldap_interface()
     result = ldap.search(
@@ -82,12 +81,6 @@ def user_list(fields=None):
         entry = {}
         for attr, values in user.items():
             if values:
-                if attr == "loginShell":
-                    if values[0].strip() == "/bin/false":
-                        entry["ssh_allowed"] = False
-                    else:
-                        entry["ssh_allowed"] = True
-
                 entry[user_attrs[attr]] = values[0]
 
         uid = entry[user_attrs["uid"]]
@@ -206,7 +199,7 @@ def user_create(
         "gidNumber": [uid],
         "uidNumber": [uid],
         "homeDirectory": ["/home/" + username],
-        "loginShell": ["/bin/false"],
+        "loginShell": ["/bin/bash"],
     }
 
     # If it is the first user, add some aliases
@@ -947,14 +940,6 @@ def user_permission_info(permission):
 import yunohost.ssh
 
 
-def user_ssh_allow(username):
-    return yunohost.ssh.user_ssh_allow(username)
-
-
-def user_ssh_disallow(username):
-    return yunohost.ssh.user_ssh_disallow(username)
-
-
 def user_ssh_list_keys(username):
     return yunohost.ssh.user_ssh_list_keys(username)