Move ssh commands into a subcategory of 'user' + fix a few bugs (#445)

* Move all ssh commands to a subcategory in user...

* [fix] Actionmap didn't match functions input ?

* [fix] Invalidate nscd cache to propagate new loginShell

* Consistency of list-keys even if there's no key..

data/actionsmap/yunohost.yml

@@ -203,30 +203,6 @@ user:
                     extra:
                         pattern: *pattern_mailbox_quota
 
-        ### ssh_user_enable_ssh()
-        allow-ssh:
-            action_help: Allow the user to uses ssh
-            api: POST /ssh/user/enable-ssh
-            configuration:
-                authenticate: all
-            arguments:
-                username:
-                    help: Username of the user
-                    extra:
-                        pattern: *pattern_username
-
-        ### ssh_user_disable_ssh()
-        disallow-ssh:
-            action_help: Disallow the user to uses ssh
-            api: POST /ssh/user/disable-ssh
-            configuration:
-                authenticate: all
-            arguments:
-                username:
-                    help: Username of the user
-                    extra:
-                        pattern: *pattern_username
-
         ### user_info()
         info:
             action_help: Get user information
@@ -238,6 +214,78 @@ user:
                 username:
                     help: Username or email to get information
 
+    subcategories:
+
+        ssh:
+            subcategory_help: Manage ssh access
+            actions:
+                ### user_ssh_enable()
+                allow:
+                    action_help: Allow the user to uses ssh
+                    api: POST /users/ssh/enable
+                    configuration:
+                        authenticate: all
+                    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
+                    configuration:
+                        authenticate: all
+                    arguments:
+                        username:
+                            help: Username of the user
+                            extra:
+                                pattern: *pattern_username
+
+                ### user_ssh_keys_list()
+                list-keys:
+                    action_help: Show user's authorized ssh keys
+                    api: GET /users/ssh/keys
+                    configuration:
+                        authenticate: all
+                    arguments:
+                        username:
+                            help: Username of the user
+                            extra:
+                                pattern: *pattern_username
+
+                ### user_ssh_keys_add()
+                add-key:
+                    action_help: Add a new authorized ssh key for this user
+                    api: POST /users/ssh/key
+                    configuration:
+                        authenticate: all
+                    arguments:
+                        username:
+                            help: Username of the user
+                            extra:
+                                pattern: *pattern_username
+                        key:
+                            help: The key to be added
+                        -c:
+                            full: --comment
+                            help: Optionnal comment about the key
+
+                ### user_ssh_keys_remove()
+                remove-key:
+                    action_help: Remove an authorized ssh key for this user
+                    api: DELETE /users/ssh/key
+                    configuration:
+                        authenticate: all
+                    arguments:
+                        username:
+                            help: Username of the user
+                            extra:
+                                pattern: *pattern_username
+                        key:
+                            help: The key to be removed
+
 
 #############################
 #          Domain           #
@@ -1349,74 +1397,6 @@ dyndns:
             api: DELETE /dyndns/cron
 
 
-#############################
-#            SSH            #
-#############################
-ssh:
-    category_help: Manage ssh keys and access
-    actions: {}
-    subcategories:
-        authorized-keys:
-            subcategory_help: Manage user's authorized ssh keys
-
-            actions:
-                ### ssh_authorized_keys_list()
-                list:
-                    action_help: Show user's authorized ssh keys
-                    api: GET /ssh/authorized-keys
-                    configuration:
-                        authenticate: all
-                    arguments:
-                        username:
-                            help: Username of the user
-                            extra:
-                                pattern: *pattern_username
-
-                ### ssh_authorized_keys_add()
-                add:
-                    action_help: Add a new authorized ssh key for this user
-                    api: POST /ssh/authorized-keys
-                    configuration:
-                        authenticate: all
-                    arguments:
-                        username:
-                            help: Username of the user
-                            extra:
-                                pattern: *pattern_username
-                        -u:
-                            full: --public
-                            help: Public key
-                            extra:
-                                required: True
-                        -i:
-                            full: --private
-                            help: Private key
-                            extra:
-                                required: True
-                        -n:
-                            full: --name
-                            help: Key name
-                            extra:
-                                required: True
-
-                ### ssh_authorized_keys_remove()
-                remove:
-                    action_help: Remove an authorized ssh key for this user
-                    api: DELETE /ssh/authorized-keys
-                    configuration:
-                        authenticate: all
-                    arguments:
-                        username:
-                            help: Username of the user
-                            extra:
-                                pattern: *pattern_username
-                        -k:
-                            full: --key
-                            help: Key as a string
-                            extra:
-                                required: True
-
-
 #############################
 #           Tools           #
 #############################

src/yunohost/ssh.py

@@ -1,13 +1,57 @@
 # encoding: utf-8
 
+import re
 import os
+import errno
+import pwd
+import subprocess
 
+from moulinette import m18n
+from moulinette.core import MoulinetteError
 from moulinette.utils.filesystem import read_file, write_to_file, chown, chmod, mkdir
 
-from yunohost.user import _get_user_for_ssh
+SSHD_CONFIG_PATH = "/etc/ssh/sshd_config"
 
 
-def ssh_authorized_keys_list(auth, username):
+def user_ssh_allow(auth, 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(auth, username):
+        raise MoulinetteError(errno.EINVAL, m18n.n('user_unknown', user=username))
+
+    auth.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(auth, 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(auth, username):
+        raise MoulinetteError(errno.EINVAL, m18n.n('user_unknown', user=username))
+
+    auth.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(auth, username):
     user = _get_user_for_ssh(auth, username, ["homeDirectory"])
     if not user:
         raise Exception("User with username '%s' doesn't exists" % username)
@@ -15,7 +59,7 @@ def ssh_authorized_keys_list(auth, username):
     authorized_keys_file = os.path.join(user["homeDirectory"][0], ".ssh", "authorized_keys")
 
     if not os.path.exists(authorized_keys_file):
-        return []
+        return {"keys": []}
 
     keys = []
     last_comment = ""
@@ -40,7 +84,7 @@ def ssh_authorized_keys_list(auth, username):
     return {"keys": keys}
 
 
-def ssh_authorized_keys_add(auth, username, key, comment):
+def user_ssh_add_key(auth, username, key, comment):
     user = _get_user_for_ssh(auth, username, ["homeDirectory", "uid"])
     if not user:
         raise Exception("User with username '%s' doesn't exists" % username)
@@ -74,8 +118,8 @@ def ssh_authorized_keys_add(auth, username, key, comment):
     write_to_file(authorized_keys_file, authorized_keys_content)
 
 
-def ssh_authorized_keys_remove(auth, username, key):
+def user_ssh_remove_key(auth, username, key):
-    user = _get_user(auth, username, ["homeDirectory", "uid"])
+    user = _get_user_for_ssh(auth, username, ["homeDirectory", "uid"])
     if not user:
         raise Exception("User with username '%s' doesn't exists" % username)
 
@@ -100,3 +144,60 @@ def ssh_authorized_keys_remove(auth, username, key):
     authorized_keys_content = authorized_keys_content.replace(key, "")
 
     write_to_file(authorized_keys_file, authorized_keys_content)
+
+#
+# Helpers
+#
+
+
+def _get_user_for_ssh(auth, username, attrs=None):
+    def ssh_root_login_status(auth):
+        # XXX temporary placed here for when the ssh_root commands are integrated
+        # extracted from https://github.com/YunoHost/yunohost/pull/345
+        # XXX should we support all the options?
+        # this is the content of "man sshd_config"
+        # PermitRootLogin
+        #     Specifies whether root can log in using ssh(1).  The argument must be
+        #     “yes”, “without-password”, “forced-commands-only”, or “no”.  The
+        #     default is “yes”.
+        sshd_config_content = read_file(SSHD_CONFIG_PATH)
+
+        if re.search("^ *PermitRootLogin +(no|forced-commands-only) *$",
+                     sshd_config_content, re.MULTILINE):
+            return {"PermitRootLogin": False}
+
+        return {"PermitRootLogin": True}
+
+    if username == "root":
+        root_unix = pwd.getpwnam("root")
+        return {
+            'username': 'root',
+            'fullname': '',
+            'mail': '',
+            'ssh_allowed': ssh_root_login_status(auth)["PermitRootLogin"],
+            'shell': root_unix.pw_shell,
+            'home_path': root_unix.pw_dir,
+        }
+
+    if username == "admin":
+        admin_unix = pwd.getpwnam("admin")
+        return {
+            'username': 'admin',
+            'fullname': '',
+            'mail': '',
+            'ssh_allowed': admin_unix.pw_shell.strip() != "/bin/false",
+            'shell': admin_unix.pw_shell,
+            'home_path': admin_unix.pw_dir,
+        }
+
+    # TODO escape input using https://www.python-ldap.org/doc/html/ldap-filter.html
+    user = auth.search('ou=users,dc=yunohost,dc=org',
+                       '(&(objectclass=person)(uid=%s))' % username,
+                       attrs)
+
+    assert len(user) in (0, 1)
+
+    if not user:
+        return None
+
+    return user[0]

src/yunohost/user.py

@@ -41,9 +41,6 @@ from yunohost.service import service_status
 
 logger = getActionLogger('yunohost.user')
 
-SSHD_CONFIG_PATH = "/etc/ssh/sshd_config"
-
-
 def user_list(auth, fields=None):
     """
     List users
@@ -446,36 +443,30 @@ def user_info(auth, username):
     else:
         raise MoulinetteError(167, m18n.n('user_info_failed'))
 
+#
+# SSH subcategory
+#
+#
+import yunohost.ssh
 
-def user_allow_ssh(auth, username):
+def user_ssh_allow(auth, username):
-    """
+    return yunohost.ssh.user_ssh_allow(auth, username)
-    Allow YunoHost user connect as ssh.
 
-    Keyword argument:
+def user_ssh_disallow(auth, username):
-        username -- User username
+    return yunohost.ssh.user_ssh_disallow(auth, username)
-    """
-    # TODO it would be good to support different kind of shells
 
-    if not _get_user_for_ssh(auth, username):
+def user_ssh_list_keys(auth, username):
-        raise MoulinetteError(errno.EINVAL, m18n.n('user_unknown', user=username))
+    return yunohost.ssh.user_ssh_list_keys(auth, username)
 
-    auth.update('uid=%s,ou=users' % username, {'loginShell': '/bin/bash'})
+def user_ssh_add_key(auth, username, key, comment):
+    return yunohost.ssh.user_ssh_add_key(auth, username, key, comment)
 
+def user_ssh_remove_key(auth, username, key):
+    return yunohost.ssh.user_ssh_remove_key(auth, username, key)
 
-def user_disallow_ssh(auth, username):
+#
-    """
+# End SSH subcategory
-    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(auth, username) :
-        raise MoulinetteError(errno.EINVAL, m18n.n('user_unknown', user=username))
-
-    auth.update('uid=%s,ou=users' % username, {'loginShell': '/bin/false'})
-
 
 def _convertSize(num, suffix=''):
     for unit in ['K', 'M', 'G', 'T', 'P', 'E', 'Z']:
@@ -514,54 +505,4 @@ def _hash_user_password(password):
     return '{CRYPT}' + crypt.crypt(str(password), salt)
 
 
-def _get_user_for_ssh(auth, username, attrs=None):
-    def ssh_root_login_status(auth):
-        # XXX temporary placed here for when the ssh_root commands are integrated
-        # extracted from https://github.com/YunoHost/yunohost/pull/345
-        # XXX should we support all the options?
-        # this is the content of "man sshd_config"
-        # PermitRootLogin
-        #     Specifies whether root can log in using ssh(1).  The argument must be
-        #     “yes”, “without-password”, “forced-commands-only”, or “no”.  The
-        #     default is “yes”.
-        sshd_config_content = read_file(SSHD_CONFIG_PATH)
 
-        if re.search("^ *PermitRootLogin +(no|forced-commands-only) *$",
-                     sshd_config_content, re.MULTILINE):
-            return {"PermitRootLogin": False}
-
-        return {"PermitRootLogin": True}
-
-    if username == "root":
-        root_unix = pwd.getpwnam("root")
-        return {
-            'username': 'root',
-            'fullname': '',
-            'mail': '',
-            'ssh_allowed': ssh_root_login_status(auth)["PermitRootLogin"],
-            'shell': root_unix.pw_shell,
-            'home_path': root_unix.pw_dir,
-        }
-
-    if username == "admin":
-        admin_unix = pwd.getpwnam("admin")
-        return {
-            'username': 'admin',
-            'fullname': '',
-            'mail': '',
-            'ssh_allowed': admin_unix.pw_shell.strip() != "/bin/false",
-            'shell': admin_unix.pw_shell,
-            'home_path': admin_unix.pw_dir,
-        }
-
-    # TODO escape input using https://www.python-ldap.org/doc/html/ldap-filter.html
-    user = auth.search('ou=users,dc=yunohost,dc=org',
-                       '(&(objectclass=person)(uid=%s))' % username,
-                       attrs)
-
-    assert len(user) in (0, 1)
-
-    if not user:
-        return None
-
-    return user[0]