mediawiki: enable/disable public registrations - refactoring & tests

- Add two functional tests to test enabling and disabling public registrations
  for MediaWiki
- Refactor and simplify config-file editing logic in actions script

Signed-off-by: Joseph Nuthalapati <njoseph@thoughtworks.com>

actions/mediawiki

@@ -41,10 +41,11 @@ def parse_arguments():
     subparsers.add_parser('enable', help='Enable MediaWiki')
     subparsers.add_parser('disable', help='Disable MediaWiki')
     subparsers.add_parser('setup', help='Setup MediaWiki')
-    help_pubreg = 'Enable/Disable/Status public user registration.'
+
-    pubreg = subparsers.add_parser('public-registration', help=help_pubreg)
+    help_pub_reg = 'Enable/Disable/Status public user registration.'
-    pubreg.add_argument('command', choices=('true', 'false', 'status'),
+    pub_reg = subparsers.add_parser('public-registrations', help=help_pub_reg)
-                        help=help_pubreg)
+    pub_reg.add_argument('command', choices=('enable', 'disable', 'status'),
+                         help=help_pub_reg)
 
     change_password = subparsers.add_parser('change-password',
                                             help='Change user password')
@@ -92,7 +93,7 @@ def _disable_public_registrations():
 def _disable_anonymous_editing():
     """Edit MediaWiki configuration to allow anonymous users from editing.
 
-    MediaWiki instances get a lot of spam bot typically.
+    MediaWiki instances typically get a lot of spam bots.
     """
     if not grep(r'\$wgGroupPermissions.*edit', CONF_FILE):
         with open(CONF_FILE, 'a') as file_handle:
@@ -134,32 +135,33 @@ def subcommand_disable(_):
     action_utils.service_disable('mediawiki-jobrunner')
 
 
-def subcommand_public_registration(argument):
+def subcommand_public_registrations(arguments):
-    """ Enable public registrations for mediawiki"""
+    """Enable or Disable public registrations for MediaWiki."""
-    with open(CONF_FILE, 'r+') as file_handle:
-        config_line_index = None
-        lines = file_handle.readlines()
-        config_line = "$wgGroupPermissions['*']['createaccount'] = "
-        for i in range(len(lines)):
-            line = lines[i]
-            if line.startswith("$wgGroupPermissions['*']['createaccount']"):
-                config_line_index = i
-                break
 
-        if config_line_index is None:
+    with open(CONF_FILE, 'r') as conf_file:
-            lines.append(config_line)
+        lines = conf_file.readlines()
-            print('disabled')
+
+    def is_pub_reg_line(line):
+        return line.startswith("$wgGroupPermissions['*']['createaccount']")
+
+    if arguments.command == 'status':
+        conf_lines = list(filter(is_pub_reg_line, lines))
+        if conf_lines:
+            print('enabled' if 'true' in conf_lines[0] else 'disabled')
         else:
-            status = lines[config_line_index].strip(' \n').split('=')[1]
+            print('disabled')
-            if argument.command == 'status':
+    else:
-                print(status)
+        with open(CONF_FILE, 'w') as conf_file:
-                return
+            for line in lines:
-            elif status != argument.command:
+                if is_pub_reg_line(line):
-                config_line = config_line + argument.command + '\n'
+                    words = line.split()
-                lines[config_line_index] = config_line
+                    if arguments.command == 'enable':
-        file_handle.seek(0)
+                        words[-1] = 'true;'
-        file_handle.writelines(lines)
+                    else:
-        file_handle.truncate()
+                        words[-1] = 'false;'
+                    conf_file.write(" ".join(words) + '\n')
+                else:
+                    conf_file.write(line)
 
 
 def main():

functional_tests/.gitignore

@@ -194,4 +194,5 @@ tags
 # End of https://www.gitignore.io/api/vim,emacs,macos,python,vagrant
 
 test_plinth/
-geckodriver.log
+geckodriver.log
+.pytest_cache

functional_tests/features/wiki_engine.feature

@@ -32,3 +32,13 @@ Scenario: Disable mediawiki application
   Given the mediawiki application is enabled
   When I disable the mediawiki application
   Then the mediawiki site should not be available
+
+Scenario: Enable public registrations
+  Given the mediawiki application is enabled
+  When I enable mediawiki public registrations
+  Then the mediawiki site should allow creating accounts
+
+Scenario: Disable public registrations
+  Given the mediawiki application is enabled
+  When I disable mediawiki public registrations
+  Then the mediawiki site should not allow creating accounts

functional_tests/step_definitions/application.py

@@ -164,3 +164,13 @@ def verify_nonexistant_share(browser, name):
 @then(parsers.parse('the share {name:w} should not be accessible'))
 def verify_inaccessible_share(browser, name):
     application.verify_inaccessible_share(browser, name)
+
+
+@when(parsers.parse('I enable mediawiki public registrations'))
+def enable_mediawiki_public_registrations(browser):
+    application.enable_mediawiki_public_registrations(browser)
+
+
+@when(parsers.parse('I disable mediawiki public registrations'))
+def disable_mediawiki_public_registrations(browser):
+    application.disable_mediawiki_public_registrations(browser)

functional_tests/step_definitions/site.py

@@ -40,3 +40,13 @@ def access_application(browser, app_name):
         'I should be able to login to coquelicot with password {password:w}'))
 def verify_upload_password(browser, password):
     site.verify_coquelicot_upload_password(browser, password)
+
+
+@then(parsers.parse('the mediawiki site should allow creating accounts'))
+def mediawiki_allows_creating_accounts(browser):
+    site.verify_mediawiki_create_account_link(browser)
+
+
+@then(parsers.parse('the mediawiki site should not allow creating accounts'))
+def mediawiki_does_not_allow_creating_accounts(browser):
+    site.verify_mediawiki_no_create_account_link(browser)

functional_tests/support/application.py

@@ -85,9 +85,11 @@ def install(browser, app_name):
         sleep(2)  # XXX This shouldn't be required.
 
 
-def _change_status(browser, app_name, change_status_to='enabled'):
+def _change_status(browser, app_name, change_status_to='enabled',
+                   checkbox_id=None):
     interface.nav_to_module(browser, get_app_module(app_name))
-    checkbox = browser.find_by_id(get_app_checkbox_id(app_name))
+    checkbox_id = checkbox_id or get_app_checkbox_id(app_name)
+    checkbox = browser.find_by_id(checkbox_id)
     checkbox.check() if change_status_to == 'enabled' else checkbox.uncheck()
     interface.submit(browser, 'form-configuration')
     if app_name in apps_with_loaders:
@@ -216,3 +218,17 @@ def verify_inaccessible_share(browser, name):
     url = '{}/share/{}'.format(default_url, name)
     browser.visit(url)
     eventually(lambda: '/plinth' in browser.url, args=[])
+
+
+def enable_mediawiki_public_registrations(browser):
+    """Enable public registrations in MediaWiki."""
+    interface.nav_to_module(browser, 'mediawiki')
+    _change_status(browser, 'mediawiki', 'enabled',
+                   checkbox_id='id_enable_public_registrations')
+
+
+def disable_mediawiki_public_registrations(browser):
+    """Enable public registrations in MediaWiki."""
+    interface.nav_to_module(browser, 'mediawiki')
+    _change_status(browser, 'mediawiki', 'disabled',
+                   checkbox_id='id_enable_public_registrations')

functional_tests/support/site.py

@@ -57,3 +57,15 @@ def verify_coquelicot_upload_password(browser, password):
     actions.perform()
     assert eventually(browser.is_element_present_by_css,
                       args=['div[style*="display: none;"]'])
+
+
+def verify_mediawiki_create_account_link(browser):
+    browser.visit(config['DEFAULT']['url'] + '/mediawiki')
+    assert eventually(browser.is_element_present_by_id,
+                      args=['pt-createaccount'])
+
+
+def verify_mediawiki_no_create_account_link(browser):
+    browser.visit(config['DEFAULT']['url'] + '/mediawiki')
+    assert eventually(browser.is_element_not_present_by_id,
+                      args=['pt-createaccount'])

plinth/modules/mediawiki/__init__.py

@@ -131,5 +131,5 @@ def diagnose():
 def get_public_registration_status():
     """Return whether public registration is enabled."""
     output = actions.superuser_run('mediawiki',
-                                   ['public-registration', 'status'])
+                                   ['public-registrations', 'status'])
-    return output.strip() == 'true'
+    return output.strip() == 'enabled'

plinth/modules/mediawiki/forms.py

@@ -26,14 +26,13 @@ from plinth.forms import ServiceForm
 
 class MediaWikiForm(ServiceForm):  # pylint: disable=W0232
     """MediaWiki configuration form."""
-    password = forms.CharField(label=_('Administrator Password'), help_text=_(
+    password = forms.CharField(
-        'Set a new password for MediaWiki\'s administrator account (admin). '
+        label=_('Administrator Password'), help_text=_(
-        'Leave this field blank to keep the current password.'),
+            'Set a new password for MediaWiki\'s administrator account '
-                               required=False, widget=forms.PasswordInput)
+            '(admin). Leave this field blank to keep the current password.'),
-    enable_public_registration = forms.BooleanField(
+        required=False, widget=forms.PasswordInput)
-        label=_('Enable Public Registration'),
-        required=False,
-        help_text=_(
-            'Disabling public registration means that you restrict account '
-            'creation from outside the freedombox'))
 
+    enable_public_registrations = forms.BooleanField(
+        label=_('Enable public registrations'), required=False, help_text=_(
+            'If enabled, anyone on the internet will be able to '
+            'create an account on your MediaWiki instance.'))

plinth/modules/mediawiki/views.py

@@ -26,8 +26,8 @@ from django.utils.translation import ugettext as _
 from plinth import actions, views
 from plinth.modules import mediawiki
 
-from .forms import MediaWikiForm
 from . import get_public_registration_status
+from .forms import MediaWikiForm
 
 logger = logging.getLogger(__name__)
 
@@ -46,7 +46,7 @@ class MediaWikiServiceView(views.ServiceView):
         """Return the values to fill in the form."""
         initial = super().get_initial()
         initial.update({
-            'enable_public_registration': get_public_registration_status()
+            'enable_public_registrations': get_public_registration_status()
         })
         return initial
 
@@ -54,14 +54,19 @@ class MediaWikiServiceView(views.ServiceView):
         """Apply the changes submitted in the form."""
         old_config = self.get_initial()
         new_config = form.cleaned_data
-        app_same = old_config['is_enabled'] == new_config['is_enabled']
+
-        pubreg_same = old_config['enable_public_registration'] == \
+        def is_unchanged(key):
-                      new_config['enable_public_registration']
+            return old_config[key] == new_config[key]
+
+        app_same = is_unchanged('is_enabled')
+        pub_reg_same = is_unchanged('enable_public_registrations')
+
         if new_config['password']:
             actions.superuser_run('mediawiki', ['change-password'],
                                   input=new_config['password'].encode())
             messages.success(self.request, _('Password updated'))
-        if app_same and pubreg_same:
+
+        if app_same and pub_reg_same:
             if not self.request._messages._queued_messages:
                 messages.info(self.request, _('Setting unchanged'))
         elif not app_same:
@@ -72,17 +77,17 @@ class MediaWikiServiceView(views.ServiceView):
                 self.service.disable()
                 messages.success(self.request, _('Application disabled'))
 
-        if not pubreg_same:
+        if not pub_reg_same:
             # note action public-registration restarts, if running now
-            if new_config['enable_public_registration']:
+            if new_config['enable_public_registrations']:
                 actions.superuser_run('mediawiki',
-                                      ['public-registration', 'true'])
+                                      ['public-registrations', 'enable'])
                 messages.success(self.request,
-                                 _('Public registration enabled'))
+                                 _('Public registrations enabled'))
             else:
                 actions.superuser_run('mediawiki',
-                                      ['public-registration', 'false'])
+                                      ['public-registrations', 'disable'])
                 messages.success(self.request,
-                                 _('Public registration disabled'))
+                                 _('Public registrations disabled'))
 
         return super().form_valid(form)