From 3cc0cb74a6b7c409ad12b4940dd60ed1e719c8c2 Mon Sep 17 00:00:00 2001 From: fonfon Date: Fri, 23 Jan 2015 10:11:46 +0000 Subject: [PATCH] Prevent adding existing or predefined services and improved form error handling --- actions/pagekite | 3 +- plinth/__main__.py | 3 + plinth/modules/pagekite/forms.py | 87 +++++++++++++++---- .../templates/pagekite_custom_services.html | 42 +++++---- .../templates/pagekite_default_services.html | 8 +- plinth/modules/pagekite/util.py | 6 +- plinth/modules/pagekite/views.py | 14 +-- 7 files changed, 113 insertions(+), 50 deletions(-) diff --git a/actions/pagekite b/actions/pagekite index 33c11647c..0a9024158 100755 --- a/actions/pagekite +++ b/actions/pagekite @@ -98,7 +98,8 @@ def subcommand_is_running(_): def subcommand_start_and_enable(_): aug.remove(PATHS['abort_not_configured']) aug.save() - _service('start') + # 'start' alone sometimes fails, even if the service is not running + _service('restart') print 'enabled' diff --git a/plinth/__main__.py b/plinth/__main__.py index 7ceacca80..0c4a253a1 100644 --- a/plinth/__main__.py +++ b/plinth/__main__.py @@ -222,6 +222,9 @@ def configure_django(): 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'plinth.modules.first_boot.middleware.FirstBootMiddleware', ), + MESSAGE_TAGS = { + messages_constants.ERROR: 'danger' + }, ROOT_URLCONF='plinth.urls', SECURE_PROXY_SSL_HEADER=secure_proxy_ssl_header, SESSION_ENGINE='django.contrib.sessions.backends.file', diff --git a/plinth/modules/pagekite/forms.py b/plinth/modules/pagekite/forms.py index 1ce227e18..2df7f00c8 100644 --- a/plinth/modules/pagekite/forms.py +++ b/plinth/modules/pagekite/forms.py @@ -18,11 +18,13 @@ from gettext import gettext as _ import logging +import copy from django import forms from django.contrib import messages from django.core import validators from actions.pagekite_util import convert_service_to_string +from plinth.errors import ActionError from .util import PREDEFINED_SERVICES, _run, get_kite_details, KITE_NAME, \ KITE_SECRET, BACKEND_HOST @@ -52,7 +54,7 @@ class ConfigurationForm(forms.Form): kite_name = TrimmedCharField( label=_('Kite name'), - help_text=_('Example: mybox1-myacc.pagekite.me'), + help_text=_('Example: mybox.pagekite.me'), validators=[ validators.RegexValidator(r'^[\w-]{1,63}(\.[\w-]{1,63})*$', _('Invalid kite name'))]) @@ -120,22 +122,23 @@ class DefaultServiceForm(forms.Form): .format(name=service_name)) -class CustomServiceForm(forms.Form): - """Form to add/delete a custom service""" +class BaseCustomServiceForm(forms.Form): + """Basic form functionality to handle a custom service""" choices = [("http", "http"), ("https", "https"), ("raw", "raw")] protocol = forms.ChoiceField(choices=choices, label="protocol") frontend_port = forms.IntegerField(min_value=0, max_value=65535, - label="external (frontend) port") + label="external (frontend) port", + required=True) backend_port = forms.IntegerField(min_value=0, max_value=65535, label="internal (freedombox) port") subdomains = forms.BooleanField(label="Enable Subdomains", required=False) - def convert_form_data_to_service_string(self, formdata): - """Prepare the user form input for being passed on to the action + def convert_formdata_to_service(self, formdata): + """Add information to make a service out of the form data""" + # convert integers to str (to compare values with DEFAULT_SERVICES) + for field in ('frontend_port', 'backend_port'): + formdata[field] = str(formdata[field]) - 1. add all information that a 'service' is expected to have - 2. convert the service to a service_string - """ # set kitename and kitesecret if not already set if 'kitename' not in formdata: if 'subdomains' in formdata and formdata['subdomains']: @@ -145,22 +148,70 @@ class CustomServiceForm(forms.Form): if 'secret' not in formdata: formdata['secret'] = KITE_SECRET - # merge protocol and frontend_port to one entry (protocol) + # merge protocol and frontend_port back to one entry (protocol) if 'frontend_port' in formdata: - if str(formdata['frontend_port']) not in formdata['protocol']: + if formdata['frontend_port'] not in formdata['protocol']: formdata['protocol'] = "%s/%s" % (formdata['protocol'], formdata['frontend_port']) if 'backend_host' not in formdata: formdata['backend_host'] = BACKEND_HOST - return convert_service_to_string(formdata) + return formdata - def save(self, request): - service = self.convert_form_data_to_service_string(self.cleaned_data) - _run(['add-service', '--service', service]) - messages.success(request, _('Added custom service')) + +class DeleteCustomServiceForm(BaseCustomServiceForm): def delete(self, request): - service = self.convert_form_data_to_service_string(self.cleaned_data) - _run(['remove-service', '--service', service]) + service = self.convert_formdata_to_service(self.cleaned_data) + service_string = convert_service_to_string(service) + _run(['remove-service', '--service', service_string]) messages.success(request, _('Deleted custom service')) + + +class AddCustomServiceForm(BaseCustomServiceForm): + """Adds the save() method and validation to not add predefined services""" + + def matches_predefined_service(self, formdata): + """Returns whether the user input matches a predefined service""" + service = self.convert_formdata_to_service(formdata) + match_found = False + for predefined_service_obj in PREDEFINED_SERVICES.values(): + # manually add the port to compare predefined with custom services + # that's due to the (sometimes) implicit port in the configuration + predefined_service = copy.copy(predefined_service_obj['params']) + if predefined_service['protocol'] == 'http': + predefined_service['protocol'] = 'http/80' + elif predefined_service['protocol'] == 'https': + predefined_service['protocol'] = 'https/443' + + # The formdata service has additional keys, so we can't compare + # the dicts directly. + # instead look whether predefined_service is a subset of service + if all(service[k] == v for k, v in predefined_service.items()): + match_found = True + break + return match_found + + def clean(self): + cleaned_data = super(AddCustomServiceForm, self).clean() + try: + is_predefined = self.matches_predefined_service(cleaned_data) + except KeyError: + is_predefined = False + if is_predefined: + msg = _("""This service is available as a default service. Please + use the 'Default Services' page to enable it.""") + raise forms.ValidationError(msg) + return cleaned_data + + def save(self, request): + service = self.convert_formdata_to_service(self.cleaned_data) + service_string = convert_service_to_string(service) + try: + _run(['add-service', '--service', service_string]) + messages.success(request, _('Added custom service')) + except ActionError as exception: + if "already exists" in str(exception): + messages.error(request, _('This service already exists')) + else: + raise diff --git a/plinth/modules/pagekite/templates/pagekite_custom_services.html b/plinth/modules/pagekite/templates/pagekite_custom_services.html index 6f41e5f97..5efec6ef8 100644 --- a/plinth/modules/pagekite/templates/pagekite_custom_services.html +++ b/plinth/modules/pagekite/templates/pagekite_custom_services.html @@ -24,32 +24,43 @@ {% block page_head %} {% endblock %} {% block content %} -

Custom Services

-
+
+
+

Create a custom service

+ {{ form|bootstrap_horizontal:'col-lg-6' }} + {% csrf_token %} +
+
+ +
+
+
+
+ +

Existing custom services

{% if not custom_services %} You don't have any Custom Services enabled @@ -65,13 +76,15 @@ {% else %} {{ service_url }} {% endif %} +
+ connected to {{ service.backend_host }}:{{ service.backend_port }}
{% csrf_token %} - {{ service.form }} + {{ service.form.as_p }}
-
- -

Create a custom service

- {{ form|bootstrap_horizontal:'col-lg-6' }} - {% csrf_token %} - - -
-
{% endblock %} diff --git a/plinth/modules/pagekite/templates/pagekite_default_services.html b/plinth/modules/pagekite/templates/pagekite_default_services.html index 0e0db1603..f9654efe8 100644 --- a/plinth/modules/pagekite/templates/pagekite_default_services.html +++ b/plinth/modules/pagekite/templates/pagekite_default_services.html @@ -38,12 +38,10 @@ {% block content %} -

Default Services

-
diff --git a/plinth/modules/pagekite/util.py b/plinth/modules/pagekite/util.py index 64118076e..9fac5b859 100644 --- a/plinth/modules/pagekite/util.py +++ b/plinth/modules/pagekite/util.py @@ -28,7 +28,11 @@ LOGGER = logging.getLogger(__name__) KITE_NAME = '@kitename' KITE_SECRET = '@kitesecret' BACKEND_HOST = 'localhost' -# predefined services show up in the PredefinedServiceForm as checkbox + +# Predefined services are used to build the PredefinedServiceForm +# +# ATTENTION: When changing the params, make sure that the AddCustomServiceForm +# still recognizes when you try to add a service equal to a predefined one PREDEFINED_SERVICES = { 'http': { 'params': {'protocol': 'http', diff --git a/plinth/modules/pagekite/views.py b/plinth/modules/pagekite/views.py index 319800c8c..adfb0bcb5 100644 --- a/plinth/modules/pagekite/views.py +++ b/plinth/modules/pagekite/views.py @@ -26,7 +26,9 @@ from django.views.generic.edit import FormView from plinth import package from .util import get_pagekite_config, get_pagekite_services, \ get_kite_details, prepare_service_for_display -from .forms import ConfigurationForm, DefaultServiceForm, CustomServiceForm +from .forms import ConfigurationForm, DefaultServiceForm, \ + AddCustomServiceForm, DeleteCustomServiceForm + subsubmenu = [{'url': reverse_lazy('pagekite:index'), 'text': _('About PageKite')}, @@ -65,7 +67,7 @@ class ContextMixin(object): class DeleteServiceView(ContextMixin, View): def post(self, request): - form = CustomServiceForm(request.POST) + form = DeleteCustomServiceForm(request.POST) if form.is_valid(): form.delete(request) return HttpResponseRedirect(reverse('pagekite:custom-services')) @@ -79,7 +81,7 @@ class CustomServiceView(ContextMixin, TemplateView): **kwargs) unused, custom_services = get_pagekite_services() for service in custom_services: - service['form'] = CustomServiceForm(initial=service) + service['form'] = AddCustomServiceForm(initial=service) context['custom_services'] = [prepare_service_for_display(service) for service in custom_services] context.update(get_kite_details()) @@ -87,15 +89,15 @@ class CustomServiceView(ContextMixin, TemplateView): def get(self, request, *args, **kwargs): context = self.get_context_data(**kwargs) - form = CustomServiceForm(prefix="custom") + form = AddCustomServiceForm(prefix="custom") context['form'] = form return self.render_to_response(context) def post(self, request): - form = CustomServiceForm(request.POST, prefix="custom") + form = AddCustomServiceForm(request.POST, prefix="custom") if form.is_valid(): form.save(request) - form = CustomServiceForm(prefix="custom") + form = AddCustomServiceForm(prefix="custom") context = self.get_context_data() context['form'] = form