{% url 'users:change_password' object.username as change_password_url %} diff --git a/plinth/modules/users/tests/test_actions.py b/plinth/modules/users/tests/test_actions.py index 6a1d54645..b88404476 100644 --- a/plinth/modules/users/tests/test_actions.py +++ b/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( - current_directory / '..' / '..' / '..' / '..' / 'actions' / 'users') + return str(current_directory / '..' / '..' / '..' / '..' / 'actions' / + '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['stderr'] = kwargs.get('stderr', subprocess.DEVNULL) + kwargs['stdout'] = kwargs.get('stdout', subprocess.PIPE) + 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], - stdout=subprocess.PIPE) + process = _call_action(['get-user-groups', username]) 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()) diff --git a/plinth/modules/users/tests/test_functional.py b/plinth/modules/users/tests/test_functional.py index fc642dbab..7481297de 100644 --- a/plinth/modules/users/tests/test_functional.py +++ b/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: - create_user(session_browser, name, 'secret123') + if user_link: + _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 ') +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 ') +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 ') -def change_language(session_browser, language): - _set_language(session_browser, _language_codes[language]) - - -@then('Plinth language should be ') -def plinth_language_should_be(session_browser, language): - assert _check_language(session_browser, _language_codes[language]) - - -def _create_user(browser, name, password): +def _create_user(browser, name, password, is_admin=False): 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' + ]) diff --git a/plinth/modules/users/tests/test_views.py b/plinth/modules/users/tests/test_views.py index 97d717908..467604990 100644 --- a/plinth/modules/users/tests/test_views.py +++ b/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}) diff --git a/plinth/modules/users/tests/users.feature b/plinth/modules/users/tests/users.feature index ddb70ab14..4e1a5eea2 100644 --- a/plinth/modules/users/tests/users.feature +++ b/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 - Then alice should not be listed as a user + When I change the ssh keys to alicesomekey123 for the user alice + 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 @@ -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 diff --git a/plinth/modules/users/views.py b/plinth/modules/users/views.py index d51014a1c..3cb9a365c 100644 --- a/plinth/modules/users/views.py +++ b/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) diff --git a/plinth/templates/base.html b/plinth/templates/base.html index 6934f4b55..0252d30f8 100644 --- a/plinth/templates/base.html +++ b/plinth/templates/base.html @@ -146,7 +146,7 @@ {% include "help-menu.html" %} - + @@ -156,13 +156,15 @@ - {% trans "Edit" %} - {% trans "Change password" %} diff --git a/plinth/tests/config.py b/plinth/tests/config.py index 22700aebb..9391c2993 100644 --- a/plinth/tests/config.py +++ b/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/ +# 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 diff --git a/plinth/tests/functional/__init__.py b/plinth/tests/functional/__init__.py index ec73a0be9..e5c6f8c78 100644 --- a/plinth/tests/functional/__init__.py +++ b/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/') - if len(apps_link): - return + user_menu = browser.find_by_id('id_user_menu') + + 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: diff --git a/plinth/tests/functional/config.ini b/plinth/tests/functional/config.ini index 6f60c99c4..8f87c8ae3 100644 --- a/plinth/tests/functional/config.ini +++ b/plinth/tests/functional/config.ini @@ -2,4 +2,5 @@ url = https://localhost:4430 username = tester password = testingtesting +ssh_port = 2222 samba_port = 4450 diff --git a/plinth/utils.py b/plinth/utils.py index a2dfcb6be..2d01a5777 100644 --- a/plinth/utils.py +++ b/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))