tests: functional: Improve creating users in tests

- Do not require a user password when creating a user. Generate a password
  if not provided.
- Before creating a user, delete previous user with the same name.
- Delete user: Do not skip if the user doesn't exist. It avoids the
  situation that some @when tests may silently skip.
- Update openvpn and deluge functional tests assuming passwords were not
  under the tests.

Tests performed:
- users, openvpn and deluge app functional tests pass.

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/deluge/tests/deluge.feature

@@ -15,14 +15,14 @@ Scenario: Enable deluge application
 
 Scenario: User of 'bit-torrent' group
   Given the deluge application is enabled
-  When I create a user named delugeuser with password deluge&456 in group bit-torrent
+  And the user delugeuser in group bit-torrent exists
-  And I'm logged in as the user delugeuser with password deluge&456
+  When I'm logged in as the user delugeuser
   Then the deluge site should be available
 
 Scenario: User not of 'bit-torrent' group
   Given the deluge application is enabled
-  When I create a user named nogroupuser with password somep@ssw6
+  And the user nogroupuser exists
-  And I'm logged in as the user nogroupuser with password somep@ssw6
+  When I'm logged in as the user nogroupuser
   Then the deluge site should not be available
 
 Scenario: Upload a torrent to deluge

plinth/modules/openvpn/tests/openvpn.feature

@@ -19,14 +19,14 @@ Scenario: Download openvpn profile
 
 Scenario: User of 'vpn' group
   Given the openvpn application is enabled
-  When I create a user named vpnuser with password openvpnrock$0 in group vpn
+  And the user vpnuser in group vpn exists
-  And I'm logged in as the user vpnuser with password openvpnrock$0
+  When I'm logged in as the user vpnuser
   Then the openvpn profile should be downloadable
 
 Scenario: User not of 'vpn' group
   Given the openvpn application is enabled
-  When I create a user named nonvpnuser with password whatever@123
+  And the user nonvpnuser exists
-  And I'm logged in as the user nonvpnuser with password whatever@123
+  When I'm logged in as the user nonvpnuser
   Then openvpn app should not be visible on the front page
 
 @backups

plinth/modules/users/tests/test_functional.py

@@ -3,8 +3,6 @@
 Functional, browser based tests for users app.
 """
 
-import random
-import string
 import subprocess
 import urllib
 
@@ -52,52 +50,17 @@ _config_page_title_language_map = {
 }
 
 
+@given(parsers.parse('the admin user {name:w} exists'))
+def admin_user_exists(session_browser, name):
+    if functional.user_exists(session_browser, name):
+        functional.delete_user(session_browser, name)
+    functional.create_user(session_browser, name, groups=['admin'])
+
+
 @given(parsers.parse("the user {name:w} doesn't exist"))
-def new_user_does_not_exist(session_browser, name):
+def user_does_not_exist(session_browser, name):
-    _delete_user(session_browser, name)
+    if functional.user_exists(session_browser, name):
-
+        functional.delete_user(session_browser, name)
-
-@given(parsers.parse('the user {name:w} exists'))
-def 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)
-
-    functional.create_user(session_browser, name, _random_string())
-
-
-@given(
-    parsers.parse('the admin user {name:w} with password {password:w} exists'))
-def 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 user_link:
-        _delete_user(session_browser, name)
-
-    functional.create_user(session_browser, name, password, groups=['admin'])
-
-
-@given(parsers.parse('the user {name:w} with password {password:w} exists'))
-def user_exists_with_password(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)
-
-    functional.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}'))
@@ -118,6 +81,11 @@ def generate_ssh_keys(session_browser, tmp_path_factory):
          str(key_file)])
 
 
+@when(parsers.parse('I create a user named {name:w}'))
+def create_user(session_browser, name):
+    functional.create_user(session_browser, name)
+
+
 @when(parsers.parse('I rename the user {old_name:w} to {new_name:w}'))
 def rename_user(session_browser, old_name, new_name):
     _rename_user(session_browser, old_name, new_name)
@@ -125,7 +93,7 @@ def rename_user(session_browser, old_name, new_name):
 
 @when(parsers.parse('I delete the user {name:w}'))
 def delete_user(session_browser, name):
-    _delete_user(session_browser, name)
+    functional.delete_user(session_browser, name)
 
 
 @when('I change the language to <language>')
@@ -150,11 +118,9 @@ def change_user_ssh_keys(session_browser, ssh_keys, username):
     _set_ssh_keys(session_browser, ssh_keys, username=username)
 
 
-@when(
+@when(parsers.parse('I change my ssh keys to {ssh_keys:w}'))
-    parsers.parse(
+def change_my_ssh_keys(session_browser, ssh_keys):
-        'I change my ssh keys to {ssh_keys:w} with password {password:w}'))
+    _set_ssh_keys(session_browser, ssh_keys)
-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'))
@@ -162,12 +128,9 @@ def set_user_inactive(session_browser, username):
     _set_user_inactive(session_browser, username)
 
 
-@when(
+@when(parsers.parse('I change my password to {new_password:w}'))
-    parsers.parse(
+def change_my_password(session_browser, new_password):
-        'I change my password from {current_password} to {new_password:w}'))
+    _change_password(session_browser, new_password)
-def change_my_password(session_browser, current_password, new_password):
-    _change_password(session_browser, new_password,
-                     current_password=current_password)
 
 
 @when(
@@ -184,23 +147,27 @@ def configure_ssh_keys(session_browser, tmp_path_factory):
     _set_ssh_keys(session_browser, public_key)
 
 
+@then(parsers.parse('I can log in as the user {username:w}'))
+def can_log_in(session_browser, username):
+    functional.login_with_account(session_browser, functional.base_url,
+                                  username)
+    assert len(session_browser.find_by_id('id_user_menu')) > 0
+
+
 @then(
     parsers.parse(
         'I can log in as the user {username:w} with password {password:w}'))
-def can_log_in(session_browser, username, password):
+def can_log_in_with_password(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(
+@then(parsers.parse("I can't log in as the user {username:w}"))
-    parsers.parse(
+def cannot_log_in(session_browser, username):
-        "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)
+                                  username)
     assert len(session_browser.find_by_id('id_user_menu')) == 0
 
 
@@ -247,12 +214,12 @@ def should_not_connect_passwordless_over_ssh(session_browser,
 
 @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)
+    assert functional.user_exists(session_browser, name)
 
 
 @then(parsers.parse('{name:w} should not be listed as a user'))
 def new_user_is_not_listed(session_browser, name):
-    assert not _is_user(session_browser, name)
+    assert not functional.user_exists(session_browser, name)
 
 
 def _rename_user(browser, old_name, new_name):
@@ -265,23 +232,6 @@ def _rename_user(browser, old_name, new_name):
     functional.submit(browser)
 
 
-def _delete_user(browser, name):
-    functional.nav_to_module(browser, 'users')
-    delete_link = browser.find_link_by_href('/plinth/sys/users/' + name +
-                                            '/delete/')
-    if delete_link:
-        with functional.wait_for_page_update(browser):
-            delete_link.first.click()
-        functional.submit(browser)
-
-
-def _is_user(browser, name):
-    functional.nav_to_module(browser, 'users')
-    edit_link = browser.find_link_by_href('/plinth/sys/users/' + name +
-                                          '/edit/')
-    return bool(edit_link)
-
-
 def _set_language(browser, language_code):
     username = functional.config['DEFAULT']['username']
     functional.visit(browser, '/plinth/sys/users/{}/edit/'.format(username))
@@ -308,13 +258,7 @@ def _get_ssh_keys(browser, username=None):
     return browser.find_by_id('id_ssh_keys').text
 
 
-def _random_string(length=8):
+def _set_ssh_keys(browser, ssh_keys, username=None):
-    """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()
@@ -322,10 +266,11 @@ def _set_ssh_keys(browser, ssh_keys, username=None, password=None):
         functional.visit(browser,
                          '/plinth/sys/users/{}/edit/'.format(username))
 
-    password = password or _admin_password
+    current_user = browser.find_by_id('id_user_menu_link').text
+    auth_password = functional.get_password(current_user)
 
     browser.find_by_id('id_ssh_keys').fill(ssh_keys)
-    browser.find_by_id('id_confirm_password').fill(password)
+    browser.find_by_id('id_confirm_password').fill(auth_password)
 
     functional.submit(browser)
 
@@ -339,8 +284,6 @@ def _set_user_inactive(browser, username):
 
 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()
@@ -348,9 +291,12 @@ def _change_password(browser, new_password, current_password=None,
         functional.visit(
             browser, '/plinth/sys/users/{}/change_password/'.format(username))
 
+    current_user = browser.find_by_id('id_user_menu_link').text
+    auth_password = current_password or functional.get_password(current_user)
+
     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)
+    browser.find_by_id('id_confirm_password').fill(auth_password)
     functional.submit(browser)
 
 

plinth/modules/users/tests/users.feature

@@ -12,7 +12,7 @@ Background:
 
 Scenario: Create user
   Given the user alice doesn't exist
-  When I create a user named alice with password secret123secret123
+  When I create a user named alice
   Then alice should be listed as a user
 
 Scenario: Rename user
@@ -20,16 +20,16 @@ Scenario: Rename user
   Given the user bob doesn't exist
   When I rename the user alice to bob
   Then alice should not be listed as a user
-  Then bob should be listed as a user
+  And bob should be listed as a 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
+  Given the user alice exists
-  And I'm logged in as the user alice with password secret123secret123
+  And I'm logged in as the user alice
-  When I change my ssh keys to somekey456 with password secret123secret123
+  When I change my ssh keys to somekey456
   Then my ssh keys should be somekey456
 
 Scenario: Admin users can change other user's ssh keys
@@ -78,15 +78,15 @@ Scenario Outline: Change language
   | None           |
 
 Scenario: Admin users can set other users an inactive
-  Given the user alice with password secret789secret789 exists
+  Given the user alice exists
   When I set the user alice as inactive
-  Then I can't log in as the user alice with password secret789secret789
+  Then I can't log in as the user alice
 
 Scenario: Admin users can change their own password
-  Given the admin user testadmin with password testingtesting123 exists
+  Given the user testadmin in group admin exists
-  And I'm logged in as the user testadmin with password testingtesting123
+  And I'm logged in as the user testadmin
-  When I change my password from testingtesting123 to testingtesting456
+  When I change my password to newpassword456
-  Then I can log in as the user testadmin with password testingtesting456
+  Then I can log in as the user testadmin with password newpassword456
 
 Scenario: Admin user can change other user's password
   Given the user alice exists
@@ -94,10 +94,10 @@ Scenario: Admin user can change other user's password
   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
+  Given the user alice exists
-  And I'm logged in as the user alice with password secret123secret123
+  And I'm logged in as the user alice
-  When I change my password from secret123secret123 to secret456secret456
+  When I change my password to newpassword123
-  Then I can log in as the user alice with password secret456secret456
+  Then I can log in as the user alice with password newpassword123
 
 Scenario: Delete user
   Given the user alice exists

plinth/tests/functional/__init__.py

@@ -144,6 +144,13 @@ def access_url(browser, site_name):
     browser.visit(config['DEFAULT']['url'] + _get_site_url(site_name))
 
 
+def get_password(username):
+    """Get a user password."""
+    if username == config['DEFAULT']['username']:
+        return config['DEFAULT']['password']
+    return 'password@&123_{}'.format(username)
+
+
 def is_available(browser, site_name):
     url_to_visit = config['DEFAULT']['url'] + _get_site_url(site_name)
     browser.visit(url_to_visit)
@@ -245,8 +252,10 @@ def login(browser):
                        config['DEFAULT']['password'])
 
 
-def login_with_account(browser, url, username, password):
+def login_with_account(browser, url, username, password=None):
 
+    if password is None:
+        password = get_password(username)
     # XXX: Find a way to remove the hardcoded jsxc URL
     if '/plinth/' not in browser.url or '/jsxc/jsxc' in browser.url:
         browser.visit(url)
@@ -499,18 +508,37 @@ def get_forwarders(browser):
 ##############################
 
 
-def create_user(browser, name, password, groups=[]):
+def create_user(browser, name, password=None, groups=[]):
     """Create a user with password and user groups."""
     nav_to_module(browser, 'users')
+
+    if password is None:
+        password = get_password(name)
+
     with 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)
+
     for group in groups:
         browser.find_by_id(f'id_groups_{group}').check()
+
     browser.find_by_id('id_confirm_password').fill(
         config['DEFAULT']['password'])
+
+    submit(browser)
+
+
+def delete_user(browser, name):
+    """Delete a user."""
+    nav_to_module(browser, 'users')
+    delete_link = browser.find_link_by_href(
+        f'/plinth/sys/users/{name}/delete/')
+
+    with wait_for_page_update(browser):
+        delete_link.first.click()
     submit(browser)
 
 

plinth/tests/functional/step_definitions.py

@@ -12,7 +12,7 @@ from plinth.tests import functional
 
 
 @given("I'm a logged in user")
-def logged_in_user(session_browser):
+def logged_in(session_browser):
     functional.login(session_browser)
 
 
@@ -161,24 +161,21 @@ def bind_assert_forwarders(session_browser, forwarders):
     assert functional.get_forwarders(session_browser) == forwarders
 
 
-@when(
+@given(parsers.parse('the user {name:w} exists'))
-    parsers.parse('I create a user named {name:w} with password {password:S}'))
+def user_exists(session_browser, name):
-def create_user(session_browser, name, password):
+    if functional.user_exists(session_browser, name):
-    if not functional.user_exists(session_browser, name):
+        functional.delete_user(session_browser, name)
-        functional.create_user(session_browser, name, password)
+    functional.create_user(session_browser, name)
 
 
-@when(
+@given(parsers.parse('the user {name:w} in group {group:S} exists'))
-    parsers.parse('I create a user named {name:w} with password {password:S} '
+def user_in_group_exists(session_browser, name, group):
-                  'in group {group:S}'))
+    if functional.user_exists(session_browser, name):
-def create_user_in_group(session_browser, name, password, group):
+        functional.delete_user(session_browser, name)
-    if not functional.user_exists(session_browser, name):
+    functional.create_user(session_browser, name, groups=[group])
-        functional.create_user(session_browser, name, password, groups=[group])
 
 
-@when(
+@given(parsers.parse("I'm logged in as the user {name:w}"))
-    parsers.parse(
+@when(parsers.parse("I'm logged in as the user {name:w}"))
-        "I'm logged in as the user {username:w} with password {password:S}"))
+def logged_in_user(session_browser, name):
-def logged_in_user_with_account(session_browser, username, password):
+    functional.login_with_account(session_browser, functional.base_url, name)
-    functional.login_with_account(session_browser, functional.base_url,
-                                  username, password)