users: Require admin credentials when creating or editing a user

This change prevents the plinth user to become a superuser without
knowing an admin password.

Users module and action script:
- User credentials are now required for the subcommands: create-user,
  set-user-password, add-user-to-group (if the group is admin),
  remove-user-from-group (if the group is admin), set-user-status,
  remove-user (if the removed user is the last admin user.
  Note: the web UI doesn't allow to delete last admin user).
- subcommand remove-users requires authentication if the user is last
  admin user. Password must be provided through standard input.
- subcommand remove-group: do not allow to remove group 'admin'
- User credentials must be provided using the argument
  --auth-user and a passsword must be provided through standard input.
- If there are no users in the admin group, no admin password is
  required and if the --auth-user argument is required, it can be an
  empty string.

Users web UI:
- An admin needs to enter current password to create and edit a user
  and to change user's password.
- Show more detailed error text on exceptions when submitting forms.
- Show page title on the edit and create user pages.

Users unit and functional tests:
- Added a configuration parameters to the pytest configuration file
  to set current admin user/password.
- Added a configuration parameter 'ssh_port' to the functional tests.
  You can overwrite this with the FREEDOMBOX_SSH_PORT environment
  variable. Modified HACKING.md accordingly.
- Added an unit test:
     - test changing the password as a non-admin user.
     - test invalid admin password input.
     - test that removing the admin group fails.
- Capture stdout and stderr in the unit tests when calling an action
  script to be able to see more info on exceptions.
- Added functional tests for setting ssh keys and changing passwords
  for admin and non-admin users.
- Added a functional test for setting a user as active/inactive.

Changes during review [sunil]:
- Move uncommon functional step definitions to users module from global. This is
  keep the common functional step definitions to minimal level and promote when
  needed.
- Minor styling changes, flake8 fixes.
- Don't require pampy module when running non-admin tests. This allows tests to
  be run from outside the container on the host machine without python3-pam
  installed.
- Call the confirm password field 'Authorization Password'. This avoid confusion
  with a very common field 'Confirm Password' which essentially means retype
  your password to ensure you didn't get it wrong. Add label explaining why the
  field exists.
- Don't hard-code /tmp path in test_actions.py. Use tmp_path_factory fixture
  provided by pytest.
- Remove unused _get_password_hash() from actions/users.
- Undo splitting ldapgid output before parsing. It does not seem correct and
  could introduce problems when field values contain spaces.

Tests performed:
- No failed unit tests (run with and without sudo).
- All 'users' functional tests pass.
- Creating an admin user during the first boot wizard succeeds.
- Creating a user using the web UI with an empty or wrong admin
  password fails and with the correct admin password succeeds.
- Editing a user using the web UI with an empty or wrong admin
  password fails and with the correct admin password succeeds.
- Changing user's password using the web UI with an empty or wrong
  admin password fails and with the correct admin password succeeds.
- Above mentioned user action script commands can't be run without
  correct credentials.
- Adding the daemon user to the freedombox-share group succeeds when
  installing certain apps (deluge, mldonkey, syncthing, transmission).

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
[sunil: Move uncommon functional step definitions to users module from global]
[sunil: Minor styling changes, flake8 fixes]
[sunil: Don't require pampy module when running non-admin tests]
[sunil: Call the confirm password field 'Authorization Password']
[sunil: Don't hard-code /tmp path in test_actions.py]
[sunil: Remove unused _get_password_hash() from actions/users]
[sunil: Undo splitting ldapgid output before parsing]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>

HACKING.md

@@ -349,7 +349,7 @@ tests will create the required user using FreedomBox's first boot process.
 **When inside a container/VM you will need to target the guest**
 
 ```bash
-guest$ export FREEDOMBOX_URL=https://localhost FREEDOMBOX_SAMBA_PORT=445
+guest$ export FREEDOMBOX_URL=https://localhost FREEDOMBOX_SSH_PORT=22 FREEDOMBOX_SAMBA_PORT=445
 ```
 
 You will be running `py.test-3`.

actions/users

@@ -14,8 +14,9 @@ import sys
 
 import augeas
 
-from plinth import action_utils
+from plinth import action_utils, utils
 
+INPUT_LINES = None
 ACCESS_CONF = '/etc/security/access.conf'
 LDAPSCRIPTS_CONF = '/etc/ldapscripts/freedombox-ldapscripts.conf'
 
@@ -31,10 +32,13 @@ def parse_arguments():
     subparser = subparsers.add_parser('create-user',
                                       help='Create an LDAP user')
     subparser.add_argument('username', help='Name of the LDAP user to create')
+    subparser.add_argument('--auth-user', required=True)
 
     subparser = subparsers.add_parser('remove-user',
                                       help='Delete an LDAP user')
-    subparser.add_argument('username', help='Name of the LDAP user to delete')
+    subparser.add_argument(
+        'username', help='Name of the LDAP user to delete. If the username is '
+        'the last admin user, a password should be provided through STDIN.')
 
     subparser = subparsers.add_parser('rename-user',
                                       help='Rename an LDAP user')
@@ -45,6 +49,7 @@ def parse_arguments():
                                       help='Set the password of an LDAP user')
     subparser.add_argument(
         'username', help='Name of the LDAP user to set the password for')
+    subparser.add_argument('--auth-user', required=True)
 
     subparser = subparsers.add_parser('create-group',
                                       help='Create an LDAP group')
@@ -65,12 +70,14 @@ def parse_arguments():
                                       help='Add an LDAP user to an LDAP group')
     subparser.add_argument('username', help='LDAP user to add to group')
     subparser.add_argument('groupname', help='LDAP group to add the user to')
+    subparser.add_argument('--auth-user', required=False)
 
     subparser = subparsers.add_parser('set-user-status',
                                       help='Set user as active or inactive')
     subparser.add_argument('username', help='User to change status')
     subparser.add_argument('status', choices=['active', 'inactive'],
                            help='New status of the user')
+    subparser.add_argument('--auth-user', required=True)
 
     subparser = subparsers.add_parser(
         'remove-user-from-group',
@@ -78,6 +85,7 @@ def parse_arguments():
     subparser.add_argument('username', help='LDAP user to remove from group')
     subparser.add_argument('groupname',
                            help='LDAP group to remove the user from')
+    subparser.add_argument('--auth-user', required=False)
 
     help_get_group_users = 'Get the list of all users in an LDAP group'
     subparser = subparsers.add_parser('get-group-users',
@@ -90,6 +98,38 @@ def parse_arguments():
     return parser.parse_args()
 
 
+def validate_user(username, must_be_admin=True):
+    """Validate a user."""
+    if must_be_admin:
+        admins = get_admin_users()
+
+        if not admins:
+            # any user is valid
+            return
+
+        if not username:
+            msg = 'Argument --auth-user is required'
+            raise argparse.ArgumentTypeError(msg)
+
+        if username not in admins:
+            msg = '"{}" is not authorized to perform this action'.format(
+                username)
+            raise argparse.ArgumentTypeError(msg)
+
+    if not username:
+        msg = 'Argument --auth-user is required'
+        raise argparse.ArgumentTypeError(msg)
+
+    validate_password(username)
+
+
+def validate_password(username):
+    """Raise an error if the user password is invalid."""
+    password = read_password(last=True)
+    if not utils.is_authenticated_user(username, password):
+        raise argparse.ArgumentTypeError("Invalid credentials")
+
+
 def subcommand_first_setup(_):
     """Perform initial setup of LDAP."""
     # Avoid reconfiguration of slapd during module upgrades, because
@@ -243,18 +283,36 @@ def disconnect_samba_user(username):
             raise
 
 
-def read_password():
+def get_input_lines():
-    """Read the password from stdin."""
+    """Return list of input lines from stdin."""
-    return ''.join(sys.stdin)
+    global INPUT_LINES
+    if INPUT_LINES is None:
+        INPUT_LINES = [line.strip() for line in sys.stdin]
+    return INPUT_LINES
+
+
+def read_password(last=False):
+    """Return the password.
+
+    Set last=True to read password from last line of the input.
+
+    """
+    line = -1 if last else 0
+    return get_input_lines()[line]
 
 
 def subcommand_create_user(arguments):
     """Create an LDAP user, set password and flush cache."""
-    _run(['ldapadduser', arguments.username, 'users'])
+    username = arguments.username
+    auth_user = arguments.auth_user
+
+    validate_user(auth_user)
+
+    _run(['ldapadduser', username, 'users'])
     password = read_password()
-    set_user_password(arguments.username, password)
+    set_user_password(username, password)
     flush_cache()
-    set_samba_user(arguments.username, password)
+    set_samba_user(username, password)
 
 
 def subcommand_remove_user(arguments):
@@ -262,6 +320,10 @@ def subcommand_remove_user(arguments):
     username = arguments.username
     groups = get_user_groups(username)
 
+    # require authentication if the user is last admin user
+    if get_group_users('admin') == [username]:
+        validate_password(username)
+
     delete_samba_user(username)
 
     for group in groups:
@@ -314,9 +376,58 @@ def set_samba_user(username, password):
 
 def subcommand_set_user_password(arguments):
     """Set a user's password."""
+    username = arguments.username
+    auth_user = arguments.auth_user
+
+    must_be_admin = username != auth_user
+    validate_user(auth_user, must_be_admin=must_be_admin)
+
     password = read_password()
-    set_user_password(arguments.username, password)
+    set_user_password(username, password)
-    set_samba_user(arguments.username, password)
+    set_samba_user(username, password)
+
+
+def get_admin_users():
+    """Returns list of members in the admin group.
+
+    Raises an error if the slapd service is not running.
+
+    """
+    admin_users = []
+
+    try:
+        output = subprocess.check_output([
+            'ldapsearch', '-LLL', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///',
+            '-o', 'ldif-wrap=no', '-s', 'base', '-b',
+            'cn=admin,ou=groups,dc=thisbox', 'memberUid'
+        ]).decode()
+    except subprocess.CalledProcessError as error:
+        if error.returncode == 32:
+            # no entries found
+            return []
+        raise
+
+    for line in output.splitlines():
+        if line.startswith('memberUid: '):
+            user = line.split('memberUid: ', 1)[1].strip()
+            admin_users.append(user)
+
+    return admin_users
+
+
+def get_group_users(groupname):
+    """Returns list of members in the group."""
+    try:
+        process = _run(['ldapgid', '-P', groupname], stdout=subprocess.PIPE)
+    except subprocess.CalledProcessError:
+        return []  # Group does not exist
+
+    output = process.stdout.decode()
+    # extract users from output, example: 'admin:*:10001:user1,user2'
+    users = output.rsplit(':')[-1].strip().split(',')
+    if users == ['']:
+        return []
+    return users
 
 
 def get_user_groups(username):
@@ -371,10 +482,14 @@ def subcommand_create_group(arguments):
 
 def subcommand_remove_group(arguments):
     """Remove an LDAP group."""
-    if group_exists(arguments.groupname):
+    groupname = arguments.groupname
-        _run(['ldapdeletegroup', arguments.groupname])
 
-    flush_cache()
+    if groupname == 'admin':
+        raise argparse.ArgumentTypeError("Can't remove the group 'admin'")
+
+    if group_exists(groupname):
+        _run(['ldapdeletegroup', groupname])
+        flush_cache()
 
 
 def add_user_to_group(username, groupname):
@@ -385,7 +500,12 @@ def add_user_to_group(username, groupname):
 
 def subcommand_add_user_to_group(arguments):
     """Add an LDAP user to an LDAP group."""
-    add_user_to_group(arguments.username, arguments.groupname)
+    groupname = arguments.groupname
+
+    if groupname == 'admin':
+        validate_user(arguments.auth_user)
+
+    add_user_to_group(arguments.username, groupname)
     flush_cache()
 
 
@@ -396,31 +516,31 @@ def remove_user_from_group(username, groupname):
 
 def subcommand_remove_user_from_group(arguments):
     """Remove an LDAP user from an LDAP group."""
-    remove_user_from_group(arguments.username, arguments.groupname)
+    username = arguments.username
+    groupname = arguments.groupname
+
+    if groupname == 'admin':
+        validate_user(arguments.auth_user)
+
+    remove_user_from_group(username, groupname)
     flush_cache()
-    if arguments.groupname == 'freedombox-share':
+    if groupname == 'freedombox-share':
-        disconnect_samba_user(arguments.username)
+        disconnect_samba_user(username)
 
 
 def subcommand_get_group_users(arguments):
     """Get the list of users of an LDAP group."""
-    try:
+    for user in get_group_users(arguments.groupname):
-        process = _run(['ldapgid', '-P', arguments.groupname],
+        print(user)
-                       stdout=subprocess.PIPE)
-    except subprocess.CalledProcessError:
-        return  # Group does not exist, return empty list
-
-    output = process.stdout.decode()
-    users = output.rsplit(':')[-1]
-    if users:
-        for user in users.strip().split(','):
-            print(user)
 
 
 def subcommand_set_user_status(arguments):
     """Set the status of the user."""
     username = arguments.username
     status = arguments.status
+    auth_user = arguments.auth_user
+
+    validate_user(auth_user)
 
     if status == 'active':
         flag = '-e'

plinth/modules/users/forms.py

@@ -6,6 +6,7 @@ import re
 from django import forms
 from django.contrib import auth, messages
 from django.contrib.auth.forms import SetPasswordForm, UserCreationForm
+from django.contrib.auth.hashers import check_password
 from django.contrib.auth.models import Group, User
 from django.core import validators
 from django.core.exceptions import ValidationError
@@ -71,9 +72,27 @@ USERNAME_FIELD = forms.CharField(
                             'letters, digits and @/./-/_ only.'))
 
 
+class PasswordConfirmForm(forms.Form):
+    """Password confirmation form."""
+    confirm_password = forms.CharField(
+        widget=forms.PasswordInput,
+        label=ugettext_lazy('Authorization Password'), help_text=ugettext_lazy(
+            'Enter your current password to authorize account modifications.'))
+
+    def clean_confirm_password(self):
+        """Check that current user's password matches."""
+        confirm_password = self.cleaned_data['confirm_password']
+        password_matches = check_password(confirm_password,
+                                          self.request.user.password)
+        if not password_matches:
+            raise ValidationError(_('Invalid password.'), code='invalid')
+
+        return confirm_password
+
+
 class CreateUserForm(ValidNewUsernameCheckMixin,
                      plinth.forms.LanguageSelectionFormMixin,
-                     UserCreationForm):
+                     PasswordConfirmForm, UserCreationForm):
     """Custom user create form.
 
     Include options to add user to groups.
@@ -95,7 +114,8 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
 
     class Meta(UserCreationForm.Meta):
         """Metadata to control automatic form building."""
-        fields = ('username', 'password1', 'password2', 'groups', 'language')
+        fields = ('username', 'password1', 'password2', 'groups', 'language',
+                  'confirm_password')
 
     def __init__(self, request, *args, **kwargs):
         """Initialize the form with extra request argument."""
@@ -104,7 +124,7 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
         self.fields['username'].widget.attrs.update({
             'autofocus': 'autofocus',
             'autocapitalize': 'none',
-            'autocomplete': 'username'
+            'autocomplete': 'username',
         })
 
     def save(self, commit=True):
@@ -114,26 +134,34 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
         if commit:
             user.userprofile.language = self.cleaned_data['language']
             user.userprofile.save()
+            auth_username = self.request.user.username
+            confirm_password = self.cleaned_data['confirm_password']
 
+            process_input = '{0}\n{1}'.format(self.cleaned_data['password1'],
+                                              confirm_password).encode()
             try:
-                actions.superuser_run(
+                actions.superuser_run('users', [
-                    'users',
+                    'create-user',
-                    ['create-user', user.get_username()],
+                    user.get_username(), '--auth-user', auth_username
-                    input=self.cleaned_data['password1'].encode())
+                ], input=process_input)
-            except ActionError:
+            except ActionError as error:
-                messages.error(self.request, _('Creating LDAP user failed.'))
+                messages.error(
+                    self.request,
+                    _('Creating LDAP user failed: {error}'.format(
+                        error=error)))
 
             for group in self.cleaned_data['groups']:
                 try:
-                    actions.superuser_run(
+                    actions.superuser_run('users', [
-                        'users',
+                        'add-user-to-group',
-                        ['add-user-to-group',
+                        user.get_username(), group, '--auth-user',
-                         user.get_username(), group])
+                        auth_username
-                except ActionError:
+                    ], input=confirm_password.encode())
+                except ActionError as error:
                     messages.error(
                         self.request,
-                        _('Failed to add new user to {group} group.').format(
+                        _('Failed to add new user to {group} group: {error}').
-                            group=group))
+                        format(group=group, error=error))
 
                 group_object, created = Group.objects.get_or_create(name=group)
                 group_object.user_set.add(user)
@@ -141,7 +169,7 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
         return user
 
 
-class UserUpdateForm(ValidNewUsernameCheckMixin,
+class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
                      plinth.forms.LanguageSelectionFormMixin, forms.ModelForm):
     """When user info is changed, also updates LDAP user."""
     username = USERNAME_FIELD
@@ -158,7 +186,8 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
 
     class Meta:
         """Metadata to control automatic form building."""
-        fields = ('username', 'groups', 'ssh_keys', 'language', 'is_active')
+        fields = ('username', 'groups', 'ssh_keys', 'language', 'is_active',
+                  'confirm_password')
         model = User
         widgets = {
             'groups': plinth.forms.CheckboxSelectMultipleWithReadOnly(),
@@ -210,9 +239,11 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
         user = super(UserUpdateForm, self).save(commit=False)
         # Profile is auto saved with user object
         user.userprofile.language = self.cleaned_data['language']
+        auth_username = self.request.user.username
+        confirm_password = self.cleaned_data['confirm_password']
 
         # If user is updating their own profile then only translate the pages
-        if self.username == self.request.user.username:
+        if self.username == auth_username:
             set_language(self.request, None, user.userprofile.language)
 
         if commit:
@@ -240,8 +271,9 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
                     try:
                         actions.superuser_run('users', [
                             'remove-user-from-group',
-                            user.get_username(), old_group
+                            user.get_username(), old_group, '--auth-user',
-                        ])
+                            auth_username
+                        ], input=confirm_password.encode())
                     except ActionError:
                         messages.error(self.request,
                                        _('Failed to remove user from group.'))
@@ -251,18 +283,23 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
                     try:
                         actions.superuser_run('users', [
                             'add-user-to-group',
-                            user.get_username(), new_group
+                            user.get_username(), new_group, '--auth-user',
-                        ])
+                            auth_username
+                        ], input=confirm_password.encode())
                     except ActionError:
                         messages.error(self.request,
                                        _('Failed to add user to group.'))
 
             try:
                 actions.superuser_run('ssh', [
-                    'set-keys', '--username',
+                    'set-keys',
-                    user.get_username(), '--keys',
+                    '--username',
-                    self.cleaned_data['ssh_keys'].strip()
+                    user.get_username(),
-                ])
+                    '--keys',
+                    self.cleaned_data['ssh_keys'].strip(),
+                    '--auth-user',
+                    auth_username,
+                ], input=confirm_password.encode())
             except ActionError:
                 messages.error(self.request, _('Unable to set SSH keys.'))
 
@@ -273,10 +310,13 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
                 else:
                     status = 'inactive'
                 try:
-                    actions.superuser_run(
+                    actions.superuser_run('users', [
-                        'users',
+                        'set-user-status',
-                        ['set-user-status',
+                        user.get_username(),
-                         user.get_username(), status])
+                        status,
+                        '--auth-user',
+                        auth_username,
+                    ], input=confirm_password.encode())
                 except ActionError:
                     messages.error(self.request,
                                    _('Failed to change user status.'))
@@ -297,7 +337,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
         return cleaned_data
 
 
-class UserChangePasswordForm(SetPasswordForm):
+class UserChangePasswordForm(PasswordConfirmForm, SetPasswordForm):
     """Custom form that also updates password for LDAP users."""
 
     def __init__(self, request, *args, **kwargs):
@@ -310,12 +350,16 @@ class UserChangePasswordForm(SetPasswordForm):
     def save(self, commit=True):
         """Save the user model and change LDAP password as well."""
         user = super(UserChangePasswordForm, self).save(commit)
+        auth_username = self.request.user.username
         if commit:
+            process_input = '{0}\n{1}'.format(
+                self.cleaned_data['new_password1'],
+                self.cleaned_data['confirm_password']).encode()
             try:
-                actions.superuser_run(
+                actions.superuser_run('users', [
-                    'users', ['set-user-password',
+                    'set-user-password',
-                              user.get_username()],
+                    user.get_username(), '--auth-user', auth_username
-                    input=self.cleaned_data['new_password1'].encode())
+                ], input=process_input)
             except ActionError:
                 messages.error(self.request,
                                _('Changing LDAP user password failed.'))
@@ -341,19 +385,25 @@ class FirstBootForm(ValidNewUsernameCheckMixin, auth.forms.UserCreationForm):
             try:
                 actions.superuser_run(
                     'users',
-                    ['create-user', user.get_username()],
+                    ['create-user',
+                     user.get_username(), '--auth-user', ''],
                     input=self.cleaned_data['password1'].encode())
-            except ActionError:
+            except ActionError as error:
-                messages.error(self.request, _('Creating LDAP user failed.'))
+                messages.error(
+                    self.request,
+                    _('Creating LDAP user failed: {error}'.format(
+                        error=error)))
 
             try:
                 actions.superuser_run(
                     'users',
                     ['add-user-to-group',
                      user.get_username(), 'admin'])
-            except ActionError:
+            except ActionError as error:
-                messages.error(self.request,
+                messages.error(
-                               _('Failed to add new user to admin group.'))
+                    self.request,
+                    _('Failed to add new user to admin group: {error}'.format(
+                        error=error)))
 
             # Create initial Django groups
             for group_choice in UsersAndGroups.get_group_choices():
@@ -368,9 +418,11 @@ class FirstBootForm(ValidNewUsernameCheckMixin, auth.forms.UserCreationForm):
             # Restrict console login to users in admin or sudo group
             try:
                 set_restricted_access(True)
-            except Exception:
+            except Exception as error:
-                messages.error(self.request,
+                messages.error(
-                               _('Failed to restrict console access.'))
+                    self.request,
+                    _('Failed to restrict console access: {error}'.format(
+                        error=error)))
 
         return user
 

plinth/modules/users/templates/users_create.html

@@ -8,6 +8,8 @@
 
 {% block content %}
 
+  <h3>{%trans "Create User" %}</h3>
+
   <form class="form" method="post">
     {% csrf_token %}
 

plinth/modules/users/templates/users_update.html

@@ -7,7 +7,11 @@
 {% load i18n %}
 
 {% block content %}
-  <h3>{{ object.username }}</h3>
+  <h3>
+    {% blocktrans trimmed with username=object.username %}
+      Edit User <em>{{ username }}</em>
+    {% endblocktrans %}
+  </h3>
 
   <p>
     {% url 'users:change_password' object.username as change_password_url %}

plinth/modules/users/tests/test_actions.py

@@ -16,6 +16,7 @@ import pytest
 
 from plinth import action_utils
 from plinth.modules import security
+from plinth.tests import config as test_config
 
 _cleanup_users = None
 _cleanup_groups = None
@@ -74,8 +75,8 @@ def _try_login_to_ssh(username, password, returncode=0):
 def _action_file():
     """Return the path to the 'users' actions file."""
     current_directory = pathlib.Path(__file__).parent
-    return str(
+    return str(current_directory / '..' / '..' / '..' / '..' / 'actions' /
-        current_directory / '..' / '..' / '..' / '..' / 'actions' / 'users')
+               'users')
 
 
 @pytest.fixture(name='disable_restricted_access', autouse=True)
@@ -111,8 +112,8 @@ def fixture_auto_cleanup_users_groups(needs_root, load_cfg):
 
 def _call_action(arguments, **kwargs):
     """Call the action script."""
-    kwargs['stdout'] = kwargs.get('stdout', subprocess.DEVNULL)
+    kwargs['stdout'] = kwargs.get('stdout', subprocess.PIPE)
-    kwargs['stderr'] = kwargs.get('stderr', subprocess.DEVNULL)
+    kwargs['stderr'] = kwargs.get('stderr', subprocess.PIPE)
     kwargs['check'] = kwargs.get('check', True)
     return subprocess.run([_action_file()] + arguments, **kwargs)
 
@@ -120,13 +121,19 @@ def _call_action(arguments, **kwargs):
 def _create_user(username=None, groups=None):
     """Call the action script for creating a new user."""
     username = username or _random_string()
-    password = _random_string()
+    password = username + '_passwd'
+    admin_user, admin_password = _get_admin_user_password()
 
-    _call_action(['create-user', username], input=password.encode())
+    process_input = "{0}\n{1}".format(password, admin_password).encode()
+    _call_action(['create-user', '--auth-user', admin_user, username],
+                 input=process_input)
 
     if groups:
         for group in groups:
-            _call_action(['add-user-to-group', username, group])
+            admin_user, admin_password = _get_admin_user_password()
+            _call_action([
+                'add-user-to-group', '--auth-user', admin_user, username, group
+            ], input=admin_password.encode())
             if group != 'admin':
                 _cleanup_groups.add(group)
 
@@ -136,29 +143,54 @@ def _create_user(username=None, groups=None):
 
 def _delete_user(username):
     """Utility to delete an LDAP and Samba user"""
-    _call_action(['remove-user', username])
+    process_input = None
+    if _get_group_users('admin') == [username]:
+        _, admin_password = _get_admin_user_password()
+        process_input = admin_password.encode()
+    _call_action(['remove-user', username], input=process_input)
+
+
+def _get_admin_user_password():
+    """Return an admin username and password."""
+
+    admin_users = _get_group_users('admin')
+
+    if not admin_users:
+        return ('', '')
+
+    if test_config.admin_username in admin_users:
+        return (test_config.admin_username, test_config.admin_password)
+
+    return (admin_users[0], admin_users[0] + '_passwd')
 
 
 def _rename_user(old_username, new_username=None):
     """Rename a user."""
     new_username = new_username or _random_string()
+
     _call_action(['rename-user', old_username, new_username])
     _cleanup_users.remove(old_username)
     _cleanup_users.add(new_username)
     return new_username
 
 
+def _get_group_users(group):
+    """Return the list of members in a group."""
+    process = _call_action(['get-group-users', group])
+    return process.stdout.decode().split()
+
+
 def _get_user_groups(username):
     """Return the list of groups for a user."""
-    process = _call_action(['get-user-groups', username],
+    process = _call_action(['get-user-groups', username])
-                           stdout=subprocess.PIPE)
     return process.stdout.decode().split()
 
 
 def _create_group(groupname=None):
     groupname = groupname or _random_string()
     _call_action(['create-group', groupname])
-    _cleanup_groups.add(groupname)
+    if groupname != 'admin':
+        _cleanup_groups.add(groupname)
     return groupname
 
 
@@ -181,7 +213,13 @@ def test_change_user_password():
     username, old_password = _create_user(groups=['admin'])
     old_password_hash = _get_password_hash(username)
     new_password = 'pass $123'
-    _call_action(['set-user-password', username], input=new_password.encode())
+
+    admin_user, admin_password = _get_admin_user_password()
+
+    process_input = "{0}\n{1}".format(new_password, admin_password).encode()
+    _call_action(['set-user-password', username, '--auth-user', admin_user],
+                 input=process_input)
+
     new_password_hash = _get_password_hash(username)
     assert old_password_hash != new_password_hash
 
@@ -191,6 +229,38 @@ def test_change_user_password():
     assert _try_login_to_ssh(username, new_password)
 
 
+def test_change_password_as_non_admin_user():
+    """Test changing user password as a non-admin user."""
+    username, old_password = _create_user()
+    old_password_hash = _get_password_hash(username)
+    new_password = 'pass $123'
+
+    process_input = "{0}\n{1}".format(new_password, old_password).encode()
+    _call_action(['set-user-password', username, '--auth-user', username],
+                 input=process_input)
+
+    new_password_hash = _get_password_hash(username)
+    assert old_password_hash != new_password_hash
+
+    # User can login to ssh using new password but not the old password.
+    # sshpass gives a return code of 5 if the password is incorrect.
+    assert _try_login_to_ssh(username, old_password, returncode=5)
+    assert _try_login_to_ssh(username, new_password)
+
+
+def test_change_other_users_password_as_non_admin():
+    """Test that changing other user's password as a non-admin user fails."""
+    username1, password1 = _create_user()
+    username2, _ = _create_user()
+    new_password = 'pass $123'
+
+    process_input = "{0}\n{1}".format(new_password, password1).encode()
+    with pytest.raises(subprocess.CalledProcessError):
+        _call_action(
+            ['set-user-password', username2, '--auth-user', username1],
+            input=process_input)
+
+
 def test_set_password_for_non_existent_user():
     """Test setting password for a non-existent user."""
     non_existent_user = _random_string()
@@ -202,6 +272,9 @@ def test_set_password_for_non_existent_user():
 
 def test_rename_user():
     """Test whether renaming a user works."""
+    # create an admin user to create other users with
+    _create_user(groups=['admin'])
+
     old_username, password = _create_user(groups=['admin', _random_string()])
     old_groups = _get_user_groups(old_username)
 
@@ -269,24 +342,40 @@ def test_groups():
     assert _is_exit_zero([_action_file(), 'remove-group', groupname])
 
 
+def test_delete_admin_group_fails():
+    """Test that deleting the admin group fails."""
+    groupname = 'admin'
+    _create_group('admin')
+
+    assert not _is_exit_zero([_action_file(), 'remove-group', groupname])
+
+
 def test_user_group_interactions():
     """Test adding/removing user from a groups."""
     group1 = _random_string()
     user1, _ = _create_user(groups=[group1])
     assert [group1] == _get_user_groups(user1)
 
+    admin_user, admin_password = _get_admin_user_password()
+
     # add-user-to-group is not idempotent
     with pytest.raises(subprocess.CalledProcessError):
-        _call_action(['add-user-to-group', user1, group1])
+        _call_action(
+            ['add-user-to-group', '--auth-user', admin_user, user1, group1],
+            input=admin_password.encode())
 
     # The same user can be added to other new groups
     group2 = _random_string()
     _create_group(group2)
-    _call_action(['add-user-to-group', user1, group2])
+    _call_action(
+        ['add-user-to-group', '--auth-user', admin_user, user1, group2],
+        input=admin_password.encode())
 
     # Adding a user to a non-existent group creates the group
     group3 = _random_string()
-    _call_action(['add-user-to-group', user1, group3])
+    _call_action(
+        ['add-user-to-group', '--auth-user', admin_user, user1, group3],
+        input=admin_password.encode())
     _cleanup_groups.add(group3)
 
     # The expected groups got created and the user is part of them.
@@ -295,7 +384,10 @@ def test_user_group_interactions():
 
     # Remove user from group
     group_to_remove_from = random.choice(expected_groups)
-    _call_action(['remove-user-from-group', user1, group_to_remove_from])
+    _call_action([
+        'remove-user-from-group', '--auth-user', admin_user, user1,
+        group_to_remove_from
+    ], input=admin_password.encode())
 
     # User is no longer in the group that they're removed from
     expected_groups.remove(group_to_remove_from)
@@ -305,4 +397,7 @@ def test_user_group_interactions():
     random_group = _random_string()
     _create_group(random_group)
     with pytest.raises(subprocess.CalledProcessError):
-        _call_action(['remove-user-from-group', user1, random_group])
+        _call_action([
+            'remove-user-from-group', '--auth-user', admin_user, user1,
+            random_group
+        ], input=admin_password.encode())

plinth/modules/users/tests/test_functional.py

@@ -3,10 +3,18 @@
 Functional, browser based tests for users app.
 """
 
+import random
+import string
+import subprocess
+import urllib
+
+import pytest
 from pytest_bdd import given, parsers, scenarios, then, when
 
 from plinth.tests import functional
 
+_admin_password = functional.config['DEFAULT']['password']
+
 scenarios('users.feature')
 
 _language_codes = {
@@ -51,11 +59,63 @@ def new_user_does_not_exist(session_browser, name):
 
 @given(parsers.parse('the user {name:w} exists'))
 def test_user_exists(session_browser, name):
+    functional.nav_to_module(session_browser, 'users')
+    user_link = session_browser.find_link_by_href(
+        '/plinth/sys/users/{}/edit/'.format(name))
+
+    if user_link:
+        _delete_user(session_browser, name)
+
+    _create_user(session_browser, name, _random_string())
+
+
+@given(
+    parsers.parse('the admin user {name:w} with password {password:w} exists'))
+def test_admin_user_exists(session_browser, name, password):
     functional.nav_to_module(session_browser, 'users')
     user_link = session_browser.find_link_by_href('/plinth/sys/users/' + name +
                                                   '/edit/')
-    if not user_link:
+    if user_link:
-        create_user(session_browser, name, 'secret123')
+        _delete_user(session_browser, name)
+
+    _create_user(session_browser, name, password, is_admin=True)
+
+
+@given(parsers.parse('the user {name:w} with password {password:w} exists'))
+def user_exists(session_browser, name, password):
+    functional.nav_to_module(session_browser, 'users')
+    user_link = session_browser.find_link_by_href('/plinth/sys/users/' + name +
+                                                  '/edit/')
+    if user_link:
+        _delete_user(session_browser, name)
+
+    _create_user(session_browser, name, password)
+
+
+@given(
+    parsers.parse(
+        "I'm logged in as the user {username:w} with password {password:w}"))
+def logged_in_user_with_account(session_browser, username, password):
+    functional.login_with_account(session_browser, functional.base_url,
+                                  username, password)
+
+
+@given(parsers.parse('the ssh keys are {ssh_keys:w}'))
+def ssh_keys(session_browser, ssh_keys):
+    _set_ssh_keys(session_browser, ssh_keys)
+
+
+@given('the client has a ssh key')
+def generate_ssh_keys(session_browser, tmp_path_factory):
+    key_file = tmp_path_factory.getbasetemp() / 'users-ssh.key'
+    try:
+        key_file.unlink()
+    except FileNotFoundError:
+        pass
+
+    subprocess.check_call(
+        ['ssh-keygen', '-t', 'ed25519', '-N', '', '-q', '-f',
+         str(key_file)])
 
 
 @when(
@@ -74,6 +134,123 @@ def delete_user(session_browser, name):
     _delete_user(session_browser, name)
 
 
+@when('I change the language to <language>')
+def change_language(session_browser, language):
+    _set_language(session_browser, _language_codes[language])
+
+
+@when(parsers.parse('I change the ssh keys to {ssh_keys:w}'))
+def change_ssh_keys(session_browser, ssh_keys):
+    _set_ssh_keys(session_browser, ssh_keys)
+
+
+@when('I remove the ssh keys')
+def remove_ssh_keys(session_browser):
+    _set_ssh_keys(session_browser, '')
+
+
+@when(
+    parsers.parse(
+        'I change the ssh keys to {ssh_keys:w} for the user {username:w}'))
+def change_user_ssh_keys(session_browser, ssh_keys, username):
+    _set_ssh_keys(session_browser, ssh_keys, username=username)
+
+
+@when(
+    parsers.parse(
+        'I change my ssh keys to {ssh_keys:w} with password {password:w}'))
+def change_my_ssh_keys(session_browser, ssh_keys, password):
+    _set_ssh_keys(session_browser, ssh_keys, password=password)
+
+
+@when(parsers.parse('I set the user {username:w} as inactive'))
+def set_user_inactive(session_browser, username):
+    _set_user_inactive(session_browser, username)
+
+
+@when(
+    parsers.parse(
+        'I change my password from {current_password} to {new_password:w}'))
+def change_my_password(session_browser, current_password, new_password):
+    _change_password(session_browser, new_password,
+                     current_password=current_password)
+
+
+@when(
+    parsers.parse(
+        'I change the user {username:w} password to {new_password:w}'))
+def change_other_user_password(session_browser, username, new_password):
+    _change_password(session_browser, new_password, username=username)
+
+
+@when('I configure the ssh keys')
+def configure_ssh_keys(session_browser, tmp_path_factory):
+    public_key_file = tmp_path_factory.getbasetemp() / 'users-ssh.key.pub'
+    public_key = public_key_file.read_text()
+    _set_ssh_keys(session_browser, public_key)
+
+
+@then(
+    parsers.parse(
+        'I can log in as the user {username:w} with password {password:w}'))
+def can_log_in(session_browser, username, password):
+    functional.visit(session_browser, '/plinth/accounts/logout/')
+    functional.login_with_account(session_browser, functional.base_url,
+                                  username, password)
+    assert len(session_browser.find_by_id('id_user_menu')) > 0
+
+
+@then(
+    parsers.parse(
+        "I can't log in as the user {username:w} with password {password:w}"))
+def cannot_log_in(session_browser, username, password):
+    functional.visit(session_browser, '/plinth/accounts/logout/')
+    functional.login_with_account(session_browser, functional.base_url,
+                                  username, password)
+    assert len(session_browser.find_by_id('id_user_menu')) == 0
+
+
+@then('Plinth language should be <language>')
+def plinth_language_should_be(session_browser, language):
+    assert _check_language(session_browser, _language_codes[language])
+
+
+@then(parsers.parse('the ssh keys should be {ssh_keys:w}'))
+def ssh_keys_match(session_browser, ssh_keys):
+    assert _get_ssh_keys(session_browser) == ssh_keys
+
+
+@then('the ssh keys should be removed')
+def ssh_keys_should_be_removed(session_browser, ssh_keys):
+    assert _get_ssh_keys(session_browser) == ''
+
+
+@then(
+    parsers.parse(
+        'the ssh keys should be {ssh_keys:w} for the user {username:w}'))
+def ssh_keys_match_for_user(session_browser, ssh_keys, username):
+    assert _get_ssh_keys(session_browser, username=username) == ssh_keys
+
+
+@then(parsers.parse('my ssh keys should be {ssh_keys:w}'))
+def my_ssh_keys_match(session_browser, ssh_keys):
+    assert _get_ssh_keys(session_browser) == ssh_keys
+
+
+@then('the client should be able to connect passwordless over ssh')
+def should_connect_passwordless_over_ssh(session_browser, tmp_path_factory):
+    key_file = tmp_path_factory.getbasetemp() / 'users-ssh.key'
+    _try_login_to_ssh(key_file=key_file)
+
+
+@then("the client shouldn't be able to connect passwordless over ssh")
+def should_not_connect_passwordless_over_ssh(session_browser,
+                                             tmp_path_factory):
+    key_file = tmp_path_factory.getbasetemp() / 'users-ssh.key'
+    with pytest.raises(subprocess.CalledProcessError):
+        _try_login_to_ssh(key_file=key_file)
+
+
 @then(parsers.parse('{name:w} should be listed as a user'))
 def new_user_is_listed(session_browser, name):
     assert _is_user(session_browser, name)
@@ -84,23 +261,16 @@ def new_user_is_not_listed(session_browser, name):
     assert not _is_user(session_browser, name)
 
 
-@when('I change the language to <language>')
+def _create_user(browser, name, password, is_admin=False):
-def change_language(session_browser, language):
-    _set_language(session_browser, _language_codes[language])
-
-
-@then('Plinth language should be <language>')
-def plinth_language_should_be(session_browser, language):
-    assert _check_language(session_browser, _language_codes[language])
-
-
-def _create_user(browser, name, password):
     functional.nav_to_module(browser, 'users')
     with functional.wait_for_page_update(browser):
         browser.find_link_by_href('/plinth/sys/users/create/').first.click()
     browser.find_by_id('id_username').fill(name)
     browser.find_by_id('id_password1').fill(password)
     browser.find_by_id('id_password2').fill(password)
+    if is_admin:
+        browser.find_by_id('id_groups_0').check()
+    browser.find_by_id('id_confirm_password').fill(_admin_password)
     functional.submit(browser)
 
 
@@ -110,6 +280,7 @@ def _rename_user(browser, old_name, new_name):
         browser.find_link_by_href('/plinth/sys/users/' + old_name +
                                   '/edit/').first.click()
     browser.find_by_id('id_username').fill(new_name)
+    browser.find_by_id('id_confirm_password').fill(_admin_password)
     functional.submit(browser)
 
 
@@ -135,6 +306,7 @@ def _set_language(browser, language_code):
     functional.visit(browser, '/plinth/sys/users/{}/edit/'.format(username))
     browser.find_by_xpath('//select[@id="id_language"]//option[@value="' +
                           language_code + '"]').first.click()
+    browser.find_by_id('id_confirm_password').fill(_admin_password)
     functional.submit(browser)
 
 
@@ -142,3 +314,73 @@ def _check_language(browser, language_code):
     functional.nav_to_module(browser, 'config')
     return browser.find_by_css('.app-titles').first.find_by_tag(
         'h2').first.value == _config_page_title_language_map[language_code]
+
+
+def _get_ssh_keys(browser, username=None):
+    functional.visit(browser, '/plinth/')
+    if username is None:
+        browser.find_by_id('id_user_menu').click()
+        browser.find_by_id('id_user_edit_menu').click()
+    else:
+        functional.visit(browser,
+                         '/plinth/sys/users/{}/edit/'.format(username))
+    return browser.find_by_id('id_ssh_keys').text
+
+
+def _random_string(length=8):
+    """Return a random string created from lower case ascii."""
+    return ''.join(
+        [random.choice(string.ascii_lowercase) for _ in range(length)])
+
+
+def _set_ssh_keys(browser, ssh_keys, username=None, password=None):
+    if username is None:
+        browser.find_by_id('id_user_menu').click()
+        browser.find_by_id('id_user_edit_menu').click()
+    else:
+        functional.visit(browser,
+                         '/plinth/sys/users/{}/edit/'.format(username))
+
+    password = password or _admin_password
+
+    browser.find_by_id('id_ssh_keys').fill(ssh_keys)
+    browser.find_by_id('id_confirm_password').fill(password)
+
+    functional.submit(browser)
+
+
+def _set_user_inactive(browser, username):
+    functional.visit(browser, '/plinth/sys/users/{}/edit/'.format(username))
+    browser.find_by_id('id_is_active').uncheck()
+    browser.find_by_id('id_confirm_password').fill(_admin_password)
+    functional.submit(browser)
+
+
+def _change_password(browser, new_password, current_password=None,
+                     username=None):
+    current_password = current_password or _admin_password
+
+    if username is None:
+        browser.find_by_id('id_user_menu').click()
+        browser.find_by_id('id_change_password_menu').click()
+    else:
+        functional.visit(
+            browser, '/plinth/sys/users/{}/change_password/'.format(username))
+
+    browser.find_by_id('id_new_password1').fill(new_password)
+    browser.find_by_id('id_new_password2').fill(new_password)
+    browser.find_by_id('id_confirm_password').fill(current_password)
+    functional.submit(browser)
+
+
+def _try_login_to_ssh(key_file=None):
+    user = functional.config['DEFAULT']['username']
+    hostname = urllib.parse.urlparse(
+        functional.config['DEFAULT']['url']).hostname
+    port = functional.config['DEFAULT']['ssh_port']
+
+    subprocess.check_call([
+        'ssh', '-p', port, '-i', key_file, '-q', '-o',
+        'StrictHostKeyChecking=no', '-o', 'UserKnownHostsFile=/dev/null', '-o',
+        'BatchMode=yes', '{0}@{1}'.format(user, hostname), '/bin/true'
+    ])

plinth/modules/users/tests/test_views.py

@@ -67,6 +67,7 @@ def make_request(request, view, as_admin=True, **kwargs):
 
     user = User.objects.create(username='tester')
     admin_user = User.objects.create(username='admin')
+    admin_user.set_password('adminpassword')
 
     request.user = admin_user if as_admin else user
 
@@ -99,7 +100,8 @@ def test_create_user_view(rf, username):
     form_data = {
         'username': username,
         'password1': password,
-        'password2': password
+        'password2': password,
+        'confirm_password': 'adminpassword',
     }
 
     request = rf.post(urls.reverse('users:create'), data=form_data)
@@ -111,6 +113,31 @@ def test_create_user_view(rf, username):
     assert response.url == urls.reverse('users:index')
 
 
+@pytest.mark.parametrize('password,error', [
+    ('', {
+        'confirm_password': ['This field is required.']
+    }),
+    ('wrong_password', {
+        'confirm_password': ['Invalid password.']
+    }),
+])
+def test_create_user_invalid_admin_view(rf, password, error):
+    """Test that user creation with an invalid admin password fails."""
+    user_password = 'testingtesting'
+    form_data = {
+        'username': 'test-new',
+        'password1': user_password,
+        'password2': user_password,
+        'confirm_password': password,
+    }
+
+    request = rf.post(urls.reverse('users:create'), data=form_data)
+    view = views.UserCreate.as_view()
+    response, messages = make_request(request, view)
+
+    assert response.context_data['form'].errors == error
+
+
 def test_create_user_form_view(rf):
     """Test that a username field on create form has correct attributes."""
     request = rf.get(urls.reverse('users:create'))
@@ -133,7 +160,8 @@ def test_create_user_invalid_username_view(rf, username):
     form_data = {
         'username': username,
         'password1': 'testingtesting',
-        'password2': 'testingtesting'
+        'password2': 'testingtesting',
+        'confirm_password': 'adminpassword',
     }
 
     request = rf.post(urls.reverse('users:create'), data=form_data)
@@ -157,7 +185,8 @@ def test_create_user_taken_or_reserved_username_view(rf, username, error):
     form_data = {
         'username': username,
         'password1': 'testingtesting',
-        'password2': 'testingtesting'
+        'password2': 'testingtesting',
+        'confirm_password': 'adminpassword',
     }
 
     request = rf.post(urls.reverse('users:create'), data=form_data)
@@ -175,7 +204,8 @@ def test_update_user_view(rf):
     form_data = {
         'username': new_username,
         'password1': 'testingtesting',
-        'password2': 'testingtesting'
+        'password2': 'testingtesting',
+        'confirm_password': 'adminpassword',
     }
 
     url = urls.reverse('users:edit', kwargs={'slug': user})
@@ -189,12 +219,40 @@ def test_update_user_view(rf):
                                         kwargs={'slug': new_username})
 
 
+@pytest.mark.parametrize('password,error', [
+    ('', {
+        'confirm_password': ['This field is required.']
+    }),
+    ('wrong_password', {
+        'confirm_password': ['Invalid password.']
+    }),
+])
+def test_update_user_invalid_admin_view(rf, password, error):
+    """Test that updating username with an invalid admin password fails."""
+    user = 'tester'
+    new_username = 'tester-renamed'
+    form_data = {
+        'username': new_username,
+        'password1': 'testingtesting',
+        'password2': 'testingtesting',
+        'confirm_password': password,
+    }
+
+    url = urls.reverse('users:edit', kwargs={'slug': user})
+    request = rf.post(url, data=form_data)
+    view = views.UserUpdate.as_view()
+    response, messages = make_request(request, view, as_admin=True, slug=user)
+
+    assert response.context_data['form'].errors == error
+
+
 def test_update_user_without_permissions_view(rf):
     """Test that updating other user as non-admin user raises exception."""
     form_data = {
         'username': 'admin-renamed',
         'password1': 'testingtesting',
-        'password2': 'testingtesting'
+        'password2': 'testingtesting',
+        'confirm_password': 'adminpassword',
     }
 
     url = urls.reverse('users:edit', kwargs={'slug': 'admin'})
@@ -224,7 +282,8 @@ def test_user_change_password_view(rf):
     user = 'admin'
     form_data = {
         'new_password1': 'testingtesting2',
-        'new_password2': 'testingtesting2'
+        'new_password2': 'testingtesting2',
+        'confirm_password': 'adminpassword',
     }
 
     url = urls.reverse('users:change_password', kwargs={'slug': user})
@@ -237,6 +296,32 @@ def test_user_change_password_view(rf):
     assert response.url == urls.reverse('users:edit', kwargs={'slug': user})
 
 
+@pytest.mark.parametrize('password,error', [
+    ('', {
+        'confirm_password': ['This field is required.']
+    }),
+    ('wrong_password', {
+        'confirm_password': ['Invalid password.']
+    }),
+])
+def test_user_change_password_invalid_admin_view(rf, password, error):
+    """Test that changing password with an invalid admin password fails."""
+    user = 'admin'
+    form_data = {
+        'new_password1': 'testingtesting2',
+        'new_password2': 'testingtesting2',
+        'confirm_password': password,
+    }
+
+    url = urls.reverse('users:change_password', kwargs={'slug': user})
+    request = rf.post(url, data=form_data)
+    view = views.UserChangePassword.as_view()
+    response, messages = make_request(request, view, as_admin=True, slug=user)
+
+    assert response.context_data['form'].errors == error
+    assert response.status_code == 200
+
+
 def test_user_change_password_without_permissions_view(rf):
     """
     Test that changing other user password as a non-admin user raises
@@ -245,7 +330,8 @@ def test_user_change_password_without_permissions_view(rf):
     user = 'admin'
     form_data = {
         'new_password1': 'adminadmin2',
-        'new_password2': 'adminadmin2'
+        'new_password2': 'adminadmin2',
+        'confirm_password': 'adminpassword',
     }
 
     url = urls.reverse('users:change_password', kwargs={'slug': user})

plinth/modules/users/tests/users.feature

@@ -2,11 +2,6 @@
 
 # TODO Scenario: Add user to wiki group
 # TODO Scenario: Remove user from wiki group
-# TODO Scenario: Set user SSH key
-# TODO Scenario: Clear user SSH key
-# TODO Scenario: Make user inactive
-# TODO Scenario: Make user active
-# TODO Scenario: Change user password
 
 @system @essential @users
 Feature: Users and Groups
@@ -27,10 +22,39 @@ Scenario: Rename user
   Then alice should not be listed as a user
   Then bob should be listed as a user
 
-Scenario: Delete user
+Scenario: Admin users can change their own ssh keys
+  When I change the ssh keys to somekey123
+  Then the ssh keys should be somekey123
+
+Scenario: Non-admin users can change their own ssh keys
+  Given the user alice with password secret123secret123 exists
+  And I'm logged in as the user alice with password secret123secret123
+  When I change my ssh keys to somekey456 with password secret123secret123
+  Then my ssh keys should be somekey456
+
+Scenario: Admin users can change other user's ssh keys
   Given the user alice exists
-  When I delete the user alice
+  When I change the ssh keys to alicesomekey123 for the user alice
-  Then alice should not be listed as a user
+  Then the ssh keys should be alicesomekey123 for the user alice
+
+Scenario: Users can remove ssh keys
+  Given the ssh keys are somekey123
+  When I remove the ssh keys
+  Then the ssh keys should be removed
+
+Scenario: Users can connect passwordless over ssh if the keys are set
+  Given the ssh application is enabled
+  And the client has a ssh key
+  When I configure the ssh keys
+  Then the client should be able to connect passwordless over ssh
+
+Scenario: Users can't connect passwordless over ssh if the keys aren't set
+  Given the ssh application is enabled
+  And the client has a ssh key
+  And the ssh keys are configured
+  When I remove the ssh keys
+  Then the client shouldn't be able to connect passwordless over ssh
+
 
 Scenario Outline: Change language
   When I change the language to <language>
@@ -52,3 +76,30 @@ Scenario Outline: Change language
   | Türkçe         |
   | 简体中文       |
   | None           |
+
+Scenario: Admin users can set other users an inactive
+  Given the user alice with password secret789secret789 exists
+  When I set the user alice as inactive
+  Then I can't log in as the user alice with password secret789secret789
+
+Scenario: Admin users can change their own password
+  Given the admin user testadmin with password testingtesting123 exists
+  And I'm logged in as the user testadmin with password testingtesting123
+  When I change my password from testingtesting123 to testingtesting456
+  Then I can log in as the user testadmin with password testingtesting456
+
+Scenario: Admin user can change other user's password
+  Given the user alice exists
+  When I change the user alice password to secretsecret567
+  Then I can log in as the user alice with password secretsecret567
+
+Scenario: Non-admin users can change their own password
+  Given the user alice with password secret123secret123 exists
+  And I'm logged in as the user alice with password secret123secret123
+  When I change my password from secret123secret123 to secret456secret456
+  Then I can log in as the user alice with password secret456secret456
+
+Scenario: Delete user
+  Given the user alice exists
+  When I delete the user alice
+  Then alice should not be listed as a user

plinth/modules/users/views.py

@@ -25,6 +25,7 @@ from .forms import (CreateUserForm, FirstBootForm, UserChangePasswordForm,
 
 class ContextMixin(object):
     """Mixin to add 'title' to the template context."""
+
     def get_context_data(self, **kwargs):
         """Add self.title to template context."""
         context = super(ContextMixin, self).get_context_data(**kwargs)

plinth/templates/base.html

@@ -146,7 +146,7 @@
 
               {% include "help-menu.html" %}
 
-              <li class="dropdown">
+              <li id="id_user_menu" class="dropdown">
                 <a href="{% url 'users:edit' request.user.username %}"
                    class="dropdown-toggle" data-toggle="dropdown"
                    role="button" aria-expanded="false">
@@ -156,13 +156,15 @@
                 </a>
                 <ul class="dropdown-menu" role="menu">
                   <li>
-                    <a href="{% url 'users:edit' request.user.username %}"
+                    <a id="id_user_edit_menu"
+                       href="{% url 'users:edit' request.user.username %}"
                        title="{% trans "Edit"%}">
                       {% trans "Edit" %}
                     </a>
                   </li>
                   <li>
-                    <a href="{% url 'users:change_password' request.user.username %}"
+                    <a id='id_change_password_menu'
+                       href="{% url 'users:change_password' request.user.username %}"
                        title="{% trans "Change password" %}">
                       {% trans "Change password" %}
                     </a>

plinth/tests/config.py

@@ -14,6 +14,12 @@ backups_ssh_password = None
 backups_ssh_keyfile = None
 backups_ssh_repo_uuid = 'plinth_test_sshfs'  # will be mounted to /media/<uuid>
 
+# An existing admin account for privileged actions. If this admin account
+# doesn't exist and no other admin accounts exist, a random admin account is
+# created and deleted afterwards by the tests.
+admin_username = 'tester'
+admin_password = 'testingtesting'
+
 # Import config_local to override the default variables
 try:
     from .config_local import *  # noqa, pylint: disable=unused-import

plinth/tests/functional/__init__.py

@@ -22,6 +22,8 @@ config.read(pathlib.Path(__file__).with_name('config.ini'))
 
 config['DEFAULT']['url'] = os.environ.get('FREEDOMBOX_URL',
                                           config['DEFAULT']['url']).rstrip('/')
+config['DEFAULT']['ssh_port'] = os.environ.get('FREEDOMBOX_SSH_PORT',
+                                               config['DEFAULT']['ssh_port'])
 config['DEFAULT']['samba_port'] = os.environ.get(
     'FREEDOMBOX_SAMBA_PORT', config['DEFAULT']['samba_port'])
 
@@ -246,9 +248,13 @@ def login_with_account(browser, url, username, password):
     if '/plinth/' not in browser.url or '/jsxc/jsxc' in browser.url:
         browser.visit(url)
 
-    apps_link = browser.find_link_by_href('/plinth/apps/')
+    user_menu = browser.find_by_id('id_user_menu')
-    if len(apps_link):
+
-        return
+    if len(user_menu):
+        if user_menu.text == username:
+            return
+
+        visit(browser, '/plinth/accounts/logout/')
 
     login_button = browser.find_link_by_href('/plinth/accounts/login/')
     if login_button:

plinth/tests/functional/config.ini

@@ -2,4 +2,5 @@
 url = https://localhost:4430
 username = tester
 password = testingtesting
+ssh_port = 2222
 samba_port = 4450

plinth/utils.py

@@ -12,7 +12,6 @@ import string
 from distutils.version import LooseVersion
 
 import markupsafe
-import pam
 import ruamel.yaml
 from django.utils.functional import lazy
 
@@ -68,6 +67,7 @@ def is_user_admin(request, cached=False):
 
 class YAMLFile(object):
     """A context management class for updating YAML files"""
+
     def __init__(self, yaml_file):
         """Return a context object for the YAML file.
 
@@ -161,5 +161,6 @@ def is_axes_old():
 
 def is_authenticated_user(username, password):
     """Return true if the user authentication succeeds."""
+    import pam  # Minimize dependencies for running tests
     pam_authenticator = pam.pam()
     return bool(pam_authenticator.authenticate(username, password))