From 21326cfe46b29d33e03eb24185128c9a5c00ebc4 Mon Sep 17 00:00:00 2001 From: Joseph Nuthalapati Date: Thu, 12 Nov 2020 08:09:10 +0530 Subject: [PATCH] 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 Reviewed-by: James Valleroy --- plinth/forms.py | 25 ++++++++++++++++- plinth/modules/openvpn/tests/openvpn.feature | 6 +++++ .../modules/openvpn/tests/test_functional.py | 10 ++++--- plinth/modules/users/forms.py | 2 +- plinth/modules/users/tests/test_functional.py | 21 +++------------ plinth/tests/functional/__init__.py | 27 +++++++++++++++++++ plinth/tests/functional/step_definitions.py | 16 +++++++++++ 7 files changed, 84 insertions(+), 23 deletions(-) diff --git a/plinth/forms.py b/plinth/forms.py index f487b1158..6042ea323 100644 --- a/plinth/forms.py +++ b/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'') 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_ diff --git a/plinth/modules/openvpn/tests/openvpn.feature b/plinth/modules/openvpn/tests/openvpn.feature index a5b151628..180ecd695 100644 --- a/plinth/modules/openvpn/tests/openvpn.feature +++ b/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 diff --git a/plinth/modules/openvpn/tests/test_functional.py b/plinth/modules/openvpn/tests/test_functional.py index 1a1cc4082..07d05b87a 100644 --- a/plinth/modules/openvpn/tests/test_functional.py +++ b/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/') diff --git a/plinth/modules/users/forms.py b/plinth/modules/users/forms.py index 760c9abd1..2d6e3e4ff 100644 --- a/plinth/modules/users/forms.py +++ b/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 ' diff --git a/plinth/modules/users/tests/test_functional.py b/plinth/modules/users/tests/test_functional.py index 7481297de..a54ae822b 100644 --- a/plinth/modules/users/tests/test_functional.py +++ b/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): diff --git a/plinth/tests/functional/__init__.py b/plinth/tests/functional/__init__.py index 03e01a8c9..661da9863 100644 --- a/plinth/tests/functional/__init__.py +++ b/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 diff --git a/plinth/tests/functional/step_definitions.py b/plinth/tests/functional/step_definitions.py index 848531139..bc8740c65 100644 --- a/plinth/tests/functional/step_definitions.py +++ b/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)