openvpn: Add functional tests for user group "vpn"

This is the first functional test which tests the permissions of a
non-administrator user in a group.

Some changes had to be made in the form shown in users module for this
to work. The id of each checkbox in the "Permissions" section is now
predictable based on the name of the group. For example, the id of the
checkbox for the group "vpn" is `id_group_vpn`. Changes are also made in
`CheckboxSelectMultipleReadOnly` form class for consistency, though it
is not being used by this functional test.

Some utility functions for functional tests have been moved out of users
module to be usable by other app modules for testing group permissions.

One additional utility function to skip creating user if it already
exists has been added. Not using this function wouldn't break the test
but using it saves some time.

Changed password format string to use `S` instead of `w` to support
special characters in password.

Signed-off-by: Joseph Nuthalapati <njoseph@riseup.net>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/forms.py

@@ -27,6 +27,7 @@ class DomainSelectionForm(forms.Form):
     """Form for selecting a domain name to be used for
     distributed federated applications
     """
+
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
@@ -80,6 +81,10 @@ class LanguageSelectionForm(LanguageSelectionFormMixin, forms.Form):
     language = LanguageSelectionFormMixin.language
 
 
+def _get_value_in_parens(string):
+    return string[string.find("(") + 1:string.find(")")]
+
+
 class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
     """
     Subclass of Django's CheckboxSelectMultiple widget that allows setting
@@ -89,6 +94,7 @@ class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
 
     Derived from https://djangosnippets.org/snippets/2786/
     """
+
     def render(self, name, value, attrs=None, choices=(), renderer=None):
         if value is None:
             value = []
@@ -105,7 +111,9 @@ class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
                 if dict.get(option_label, 'readonly'):
                     final_attrs = dict(final_attrs, readonly='readonly')
                 option_label = option_label['label']
-            final_attrs = dict(final_attrs, id='{}_{}'.format(attrs['id'], i))
+            group_name = _get_value_in_parens(option_label)
+            final_attrs = dict(final_attrs,
+                               id='{}_{}'.format(attrs['id'], group_name))
             label_for = u' for="{}"'.format(final_attrs['id'])
             cb = CheckboxInput(final_attrs,
                                check_test=lambda value: value in str_values)
@@ -114,3 +122,18 @@ class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
                           (label_for, rendered_cb, option_label))
         output.append(u'</ul>')
         return mark_safe(u'\n'.join(output))
+
+
+class CheckboxSelectMultiple(forms.widgets.CheckboxSelectMultiple):
+    """CheckboxSelectMultiple with ids named after choices."""
+
+    def id_for_label(self, id_, index=None):
+        """
+        Make ids looks like id_groups_groupname where
+        a choice is like ('groupname', 'Group description').
+        """
+        if index is None:
+            return ''
+        if id_ and self.add_id_index:
+            id_ = '%s_%s' % (id_, list(self.choices)[int(index)][0])
+        return id_

plinth/modules/openvpn/tests/openvpn.feature

@@ -17,6 +17,12 @@ Scenario: Download openvpn profile
   Given the openvpn application is enabled
   Then the openvpn profile should be downloadable
 
+Scenario: OpenVPN user group
+  Given the openvpn application is enabled
+  When I create a user named vpnuser with password openvpnrock$0 in group vpn
+  And I'm logged in as the user vpnuser with password openvpnrock$0
+  Then the openvpn profile should be downloadable
+
 @backups
 Scenario: Backup and restore openvpn
   Given the openvpn application is enabled

plinth/modules/openvpn/tests/test_functional.py

@@ -28,7 +28,9 @@ def openvpn_profile_download_compare(session_browser,
 
 
 def _download_profile(browser):
-    """Download the current user's profile into a file and return path."""
-    functional.nav_to_module(browser, 'openvpn')
-    url = browser.find_by_css('.form-profile')['action']
-    return functional.download_file(browser, url)
+    """Return the content of the current user's OpenVPN profile."""
+    default_url = functional.config['DEFAULT']['URL']
+    browser.visit(default_url)
+    browser.click_link_by_href('?selected=shortcut-openvpn')
+    return functional.download_file(
+        browser, f'{default_url}/plinth/apps/openvpn/profile/')

plinth/modules/users/forms.py

@@ -101,7 +101,7 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
     groups = forms.MultipleChoiceField(
         choices=UsersAndGroups.get_group_choices,
         label=ugettext_lazy('Permissions'), required=False,
-        widget=forms.CheckboxSelectMultiple, help_text=ugettext_lazy(
+        widget=plinth.forms.CheckboxSelectMultiple, help_text=ugettext_lazy(
             'Select which services should be available to the new '
             'user. The user will be able to log in to services that '
             'support single sign-on through LDAP, if they are in the '

plinth/modules/users/tests/test_functional.py

@@ -66,7 +66,7 @@ def test_user_exists(session_browser, name):
     if user_link:
         _delete_user(session_browser, name)
 
-    _create_user(session_browser, name, _random_string())
+    functional.create_user(session_browser, name, _random_string())
 
 
 @given(
@@ -78,7 +78,7 @@ def test_admin_user_exists(session_browser, name, password):
     if user_link:
         _delete_user(session_browser, name)
 
-    _create_user(session_browser, name, password, is_admin=True)
+    functional.create_user(session_browser, name, password, groups=['admin'])
 
 
 @given(parsers.parse('the user {name:w} with password {password:w} exists'))
@@ -89,7 +89,7 @@ def user_exists(session_browser, name, password):
     if user_link:
         _delete_user(session_browser, name)
 
-    _create_user(session_browser, name, password)
+    functional.create_user(session_browser, name, password)
 
 
 @given(
@@ -121,7 +121,7 @@ def generate_ssh_keys(session_browser, tmp_path_factory):
 @when(
     parsers.parse('I create a user named {name:w} with password {password:w}'))
 def create_user(session_browser, name, password):
-    _create_user(session_browser, name, password)
+    functional.create_user(session_browser, name, password)
 
 
 @when(parsers.parse('I rename the user {old_name:w} to {new_name:w}'))
@@ -261,19 +261,6 @@ def new_user_is_not_listed(session_browser, name):
     assert not _is_user(session_browser, name)
 
 
-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)
-
-
 def _rename_user(browser, old_name, new_name):
     functional.nav_to_module(browser, 'users')
     with functional.wait_for_page_update(browser):

plinth/tests/functional/__init__.py

@@ -488,3 +488,30 @@ def get_forwarders(browser):
     """Return the forwarders list (space separated) in bind configuration."""
     nav_to_module(browser, 'bind')
     return browser.find_by_name('forwarders').first.value
+
+
+##############################
+# Users and Groups utilities #
+##############################
+
+
+def create_user(browser, name, password, groups=[]):
+    """Create a user with password and user groups."""
+    nav_to_module(browser, 'users')
+    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 user_exists(browser, name):
+    """Check if a user with a given name exists."""
+    nav_to_module(browser, 'users')
+    links = browser.find_link_by_href(f'/plinth/sys/users/{name}/edit/')
+    return len(links) == 1

plinth/tests/functional/step_definitions.py

@@ -159,3 +159,19 @@ def bind_set_forwarders(session_browser, forwarders):
 @then(parsers.parse('bind forwarders should be {forwarders}'))
 def bind_assert_forwarders(session_browser, forwarders):
     assert functional.get_forwarders(session_browser) == forwarders
+
+
+@when(
+    parsers.parse('I create a user named {name:w} with password {password:S} '
+                  'in group {group:w}'))
+def create_user(session_browser, name, password, group):
+    if not functional.user_exists(session_browser, name):
+        functional.create_user(session_browser, name, password, groups=[group])
+
+
+@when(
+    parsers.parse(
+        "I'm logged in as the user {username:w} with password {password:S}"))
+def logged_in_user_with_account(session_browser, username, password):
+    functional.login_with_account(session_browser, functional.base_url,
+                                  username, password)