From 6864e462bfba49de70d9b05fc2f11a515557005d Mon Sep 17 00:00:00 2001 From: root Date: Wed, 5 Nov 2014 20:22:01 +0000 Subject: [PATCH] Improved validation errors --- TODO.md | 34 +++++++++++++++++- orchestra/apps/accounts/forms.py | 15 +++++--- orchestra/apps/bills/models.py | 6 ++-- orchestra/apps/contacts/models.py | 12 +++++-- orchestra/apps/domains/forms.py | 6 ++-- orchestra/apps/domains/models.py | 19 +++++++---- orchestra/apps/mailboxes/forms.py | 6 ++-- orchestra/apps/payments/models.py | 2 +- orchestra/apps/services/handlers.py | 29 ++++++++++++++++ orchestra/apps/services/models.py | 38 +++++---------------- orchestra/apps/webapps/backends/__init__.py | 2 +- orchestra/apps/webapps/models.py | 9 +++-- orchestra/apps/webapps/settings.py | 4 +-- orchestra/apps/websites/backends/apache.py | 1 + orchestra/apps/websites/models.py | 9 +++-- orchestra/core/validators.py | 12 +++++++ 16 files changed, 145 insertions(+), 59 deletions(-) diff --git a/TODO.md b/TODO.md index 1dea9801..18bdabf1 100644 --- a/TODO.md +++ b/TODO.md @@ -12,7 +12,6 @@ TODO ==== * add `BackendLog` retry action * PHPbBckendMiixin with get_php_ini -* Apache: `IncludeOptional /etc/apache2/extra-vhos[t]/account-site-custom.con[f]` * webmail identities and addresses * user.roles.mailbox its awful when combined with addresses: * address.mailboxes filter by account is crap in admin and api @@ -180,3 +179,36 @@ Remember that, as always with QuerySets, any subsequent chained methods which im * Databases.User add reverse M2M databases widget (like mailbox.addresses) * One domain zone validation for each save, not one per subdomain, maybe on modeladmin.save_related? prevent save on model_related, and save it on save_related() + +* Change permissions periodically on the web server, to ensure security + +* Apache RLimit ? + +* Patch suexec: if user mismatch, check user belongs to suexecusergroup group + +* fuck suexec http://www.litespeedtech.com/support/forum/threads/solved-cloudlinux-php-lsapi-say-no-to-suexec.5812/ + +* http://mail-archives.apache.org/mod_mbox/httpd-dev/201409.mbox/%3C5411FFBE.9050506@loginroot.com%3E ?? + +* Root owned logs on user's home ? + + +* Secondary user home in /home/secondaryuser and simlink to /home/main/webapps/app so it can have private storage? + +* Grant permissions like in webfaction + + +* Secondaryusers home should be under mainuser home. i.e. /home/mainuser/webapps/seconduser_webapp/ +* Make one dedicated CGI user for each account only for CGI execution (fpm/fcgid). Different from the files owner, and without W permissions, so attackers can not inject backdors and malware. +* In most cases we can prevent the creation of files for the CGI users, preventing attackers to upload and executing PHPShells. +* Make main systemuser able to write/read everything on its home, including stuff created by the CGI user and secondary users +* Prevent users from accessing other users home while at the same time allow access Apache/fcgid/fpm and secondary users (x) + +* public_html/webapps directory with root owner and permissions + +* resource min max allocation with validation + +* mailman needs both aliases when address_name is provided (default messages and bounces and all) + +* specify field on ValidationError under model.clean() of form.clean(): ValidationError({'bark_volume': ["Must be louder!",]} + * And raise ValidationError once at the end collecting all errors at once diff --git a/orchestra/apps/accounts/forms.py b/orchestra/apps/accounts/forms.py index b3907a63..1a9eb9ca 100644 --- a/orchestra/apps/accounts/forms.py +++ b/orchestra/apps/accounts/forms.py @@ -17,7 +17,7 @@ def create_account_creation_form(): help_text=_("Designates whether to creates an enabled or disabled related system user. " "Notice that a related system user will be always created.")) }) - for model, key, kwargs, help_text in settings.ACCOUNTS_CREATE_RELATED: + for model, __, kwargs, help_text in settings.ACCOUNTS_CREATE_RELATED: model = get_model(model) field_name = 'create_%s' % model._meta.model_name label = _("Create %s") % model._meta.verbose_name @@ -35,9 +35,10 @@ def create_account_creation_form(): except KeyError: # Previous validation error return + errors = {} systemuser_model = Account.main_systemuser.field.rel.to if systemuser_model.objects.filter(username=account.username).exists(): - raise forms.ValidationError(_("A system user with this name already exists")) + errors['username'] = _("A system user with this name already exists.") for model, key, related_kwargs, __ in settings.ACCOUNTS_CREATE_RELATED: model = get_model(model) kwargs = { @@ -45,9 +46,13 @@ def create_account_creation_form(): } if model.objects.filter(**kwargs).exists(): verbose_name = model._meta.verbose_name - raise forms.ValidationError( - _("A %s with this name already exists") % verbose_name - ) + field_name = 'create_%s' % model._meta.model_name + errors[field] = ValidationError( + _("A %(type)s with this name already exists."), + params={'type': verbose_name}) + if errors: + raise ValidationError(errors) + def save_model(self, account): account.save(active_systemuser=self.cleaned_data['enable_systemuser']) diff --git a/orchestra/apps/bills/models.py b/orchestra/apps/bills/models.py index 52bd6765..f44f2e43 100644 --- a/orchestra/apps/bills/models.py +++ b/orchestra/apps/bills/models.py @@ -41,8 +41,10 @@ class BillContact(models.Model): def clean(self): self.vat = self.vat.strip() self.city = self.city.strip() - validators.validate_vat(self.vat, self.country) - validators.validate_zipcode(self.zipcode, self.country) + validators.all_valid({ + 'vat': (validators.validate_vat, self.vat, self.country), + 'zipcode': (validators.validate_zipcode, self.zipcode, self.country) + }) class BillManager(models.Manager): diff --git a/orchestra/apps/contacts/models.py b/orchestra/apps/contacts/models.py index e2a76e10..afaae3ae 100644 --- a/orchestra/apps/contacts/models.py +++ b/orchestra/apps/contacts/models.py @@ -66,12 +66,18 @@ class Contact(models.Model): self.address = self.address.strip() self.city = self.city.strip() self.country = self.country.strip() + errors = {} if self.address and not (self.city and self.zipcode and self.country): - raise ValidationError(_("City, zipcode and country must be provided when address is provided.")) + errors['__all__'] = _("City, zipcode and country must be provided when address is provided.") if self.zipcode and not self.country: - raise ValidationError(_("Country must be provided when zipcode is provided.")) + errors['country'] = _("Country must be provided when zipcode is provided.") elif self.zipcode and self.country: - validators.validate_zipcode(self.zipcode, self.country) + try: + validators.validate_zipcode(self.zipcode, self.country) + except ValidationError, error: + errors['zipcode'] = error + if errors: + raise ValidationError(errors) accounts.register(Contact) diff --git a/orchestra/apps/domains/forms.py b/orchestra/apps/domains/forms.py index 212af8e9..80be59a5 100644 --- a/orchestra/apps/domains/forms.py +++ b/orchestra/apps/domains/forms.py @@ -21,8 +21,9 @@ class CreateDomainAdminForm(forms.ModelForm): # Fake an account to make django validation happy account_model = self.fields['account']._queryset.model cleaned_data['account'] = account_model() - msg = _("An account should be provided for top domain names") - raise ValidationError(msg) + raise ValidationError({ + 'account': _("An account should be provided for top domain names."), + }) cleaned_data['account'] = top.account return cleaned_data @@ -67,6 +68,7 @@ class CreateDomainAdminForm(forms.ModelForm): # self.save_formset(request, form, formset, change=change) +# TODO do it in admin class RecordInlineFormSet(forms.models.BaseInlineFormSet): def clean(self): """ Checks if everything is consistent """ diff --git a/orchestra/apps/domains/models.py b/orchestra/apps/domains/models.py index 6722601f..27d4a375 100644 --- a/orchestra/apps/domains/models.py +++ b/orchestra/apps/domains/models.py @@ -14,7 +14,7 @@ class Domain(models.Model): help_text=_("Domain or subdomain name.")) account = models.ForeignKey('accounts.Account', verbose_name=_("Account"), related_name='domains', blank=True, help_text=_("Automatically selected for subdomains.")) - top = models.ForeignKey('domains.Domain', null=True, related_name='subdomains', editable=False) + top = models.ForeignKey('domains.Domain', null=True, related_name='subdomain_set', editable=False) serial = models.IntegerField(_("serial"), default=utils.generate_zone_serial, help_text=_("Serial number")) @@ -41,6 +41,10 @@ class Domain(models.Model): # don't cache, don't replace by top_id return not bool(self.top) + @property + def subdomains(self): + return Domain.objects.filter(name__regex='\.%s$' % self.name) + def get_absolute_url(self): return 'http://%s' % self.name @@ -50,7 +54,7 @@ class Domain(models.Model): def get_subdomains(self): """ proxy method, needed for input validation, see helpers.domain_for_validation """ - return self.origin.subdomains.all() + return self.origin.subdomain_set.all() def get_top(self): return type(self).get_top_domain(self.name) @@ -140,8 +144,8 @@ class Domain(models.Model): update = True super(Domain, self).save(*args, **kwargs) if update: - domains = Domain.objects.exclude(pk=self.pk) - for domain in domains.filter(name__endswith=self.name): + for domain in self.subdomains.exclude(pk=self.pk): + # queryset.update() is not used because we want to trigger backend to delete ex-topdomains domain.top = self domain.save(update_fields=['top']) @@ -182,7 +186,7 @@ class Record(models.Model): """ validates record value based on its type """ # validate value self.value = self.value.lower().strip() - mapp = { + choices = { self.MX: validators.validate_mx_record, self.NS: validators.validate_zone_label, self.A: validate_ipv4_address, @@ -192,7 +196,10 @@ class Record(models.Model): self.SRV: validators.validate_srv_record, self.SOA: validators.validate_soa_record, } - mapp[self.type](self.value) + try: + choices[self.type](self.value) + except ValidationError, error: + raise ValidationError({'value': error}) def get_ttl(self): return self.ttl or settings.DOMAINS_DEFAULT_TTL diff --git a/orchestra/apps/mailboxes/forms.py b/orchestra/apps/mailboxes/forms.py index 60d19a7c..55fffed4 100644 --- a/orchestra/apps/mailboxes/forms.py +++ b/orchestra/apps/mailboxes/forms.py @@ -45,7 +45,9 @@ class MailboxForm(forms.ModelForm): filtering = self.cleaned_data['filtering'] custom_filtering = self.cleaned_data['custom_filtering'] if filtering == self._meta.model.CUSTOM and not custom_filtering: - raise forms.ValidationError(_("You didn't provide any custom filtering")) + raise forms.ValidationError({ + 'custom_filtering': _("You didn't provide any custom filtering.") + }) return custom_filtering @@ -69,4 +71,4 @@ class AddressForm(forms.ModelForm): def clean(self): cleaned_data = super(AddressForm, self).clean() if not cleaned_data.get('mailboxes', True) and not cleaned_data['forward']: - raise forms.ValidationError(_("Mailboxes or forward address should be provided")) + raise forms.ValidationError(_("Mailboxes or forward address should be provided.")) diff --git a/orchestra/apps/payments/models.py b/orchestra/apps/payments/models.py index afe52588..7ffcd236 100644 --- a/orchestra/apps/payments/models.py +++ b/orchestra/apps/payments/models.py @@ -117,7 +117,7 @@ class Transaction(models.Model): if not self.pk: amount = self.bill.transactions.exclude(state=self.REJECTED).amount() if amount >= self.bill.total: - raise ValidationError(_("New transactions can not be allocated for this bill")) + raise ValidationError(_("New transactions can not be allocated for this bill.")) def mark_as_processed(self): assert self.state == self.WAITTING_PROCESSING diff --git a/orchestra/apps/services/handlers.py b/orchestra/apps/services/handlers.py index 0dc8405e..d1db77bf 100644 --- a/orchestra/apps/services/handlers.py +++ b/orchestra/apps/services/handlers.py @@ -39,6 +39,35 @@ class ServiceHandler(plugins.Plugin): choices = super(ServiceHandler, cls).get_plugin_choices() return [('', _("Default"))] + choices + def validate_content_type(self, service): + pass + + def validate_match(self, service): + if not service.match: + raise ValidationError(_("Match should be provided.")) + try: + obj = service.content_type.model_class().objects.all()[0] + except IndexError: + return + try: + bool(self.matches(obj)) + except Exception, exception: + name = type(exception).__name__ + message = exception.message + raise ValidationError(': '.join((name, message))) + + def validate_metric(self, service): + try: + obj = service.content_type.model_class().objects.all()[0] + except IndexError: + return + try: + bool(self.get_metric(obj)) + except Exception, exception: + name = type(exception).__name__ + message = exception.message + raise ValidationError(': '.join((name, message))) + def get_content_type(self): if not self.model: return self.content_type diff --git a/orchestra/apps/services/models.py b/orchestra/apps/services/models.py index eacc83f0..26f05852 100644 --- a/orchestra/apps/services/models.py +++ b/orchestra/apps/services/models.py @@ -9,7 +9,7 @@ from django.utils.functional import cached_property from django.utils.module_loading import autodiscover_modules from django.utils.translation import ugettext_lazy as _ -from orchestra.core import caches, services, accounts +from orchestra.core import caches, services, accounts, validators from orchestra.core.validators import validate_name from orchestra.models import queryset @@ -51,7 +51,7 @@ class ContractedPlan(models.Model): def clean(self): if not self.pk and not self.plan.allow_multiples: if ContractedPlan.objects.filter(plan=self.plan, account=self.account).exists(): - raise ValidationError("A contracted plan for this account already exists") + raise ValidationError("A contracted plan for this account already exists.") class RateQuerySet(models.QuerySet): @@ -243,34 +243,12 @@ class Service(models.Model): def clean(self): self.description = self.description.strip() - content_type = self.handler.get_content_type() - if self.content_type != content_type: - ct = str(content_type) - raise ValidationError(_("Content type must be equal to '%s'.") % ct) - if not self.match: - raise ValidationError(_("Match should be provided")) - try: - obj = content_type.model_class().objects.all()[0] - except IndexError: - pass - else: - attr = None - try: - bool(self.handler.matches(obj)) - except Exception as exception: - attr = "Matches" - try: - metric = self.handler.get_metric(obj) - if metric is not None: - int(metric) - except Exception as exception: - attr = "Get metric" - if attr is not None: - name = type(exception).__name__ - message = exception.message - msg = "{0} {1}: {2}".format(attr, name, message) - raise ValidationError(msg) - + validators.all_valid({ + 'content_type': (self.handler.validate_content_type, self), + 'match': (self.handlers.validate_match, self), + 'metric': (self.handlers.validate_metric, self), + }) + def get_pricing_period(self): if self.pricing_period == self.BILLING_PERIOD: return self.billing_period diff --git a/orchestra/apps/webapps/backends/__init__.py b/orchestra/apps/webapps/backends/__init__.py index c3fb9f4c..5c7a0aa2 100644 --- a/orchestra/apps/webapps/backends/__init__.py +++ b/orchestra/apps/webapps/backends/__init__.py @@ -54,7 +54,7 @@ class WebAppServiceMixin(object): return { 'user': webapp.get_username(), 'group': webapp.get_groupname(), - 'app_name': webapp.name, + 'app_name': webapp.get_name(), 'type': webapp.type, 'app_path': webapp.get_path().rstrip('/'), 'banner': self.get_banner(), diff --git a/orchestra/apps/webapps/models.py b/orchestra/apps/webapps/models.py index 2e95d095..83048060 100644 --- a/orchestra/apps/webapps/models.py +++ b/orchestra/apps/webapps/models.py @@ -77,8 +77,13 @@ class WebAppOption(models.Model): """ validates name and value according to WEBAPPS_OPTIONS """ __, regex = settings.WEBAPPS_OPTIONS[self.name] if not re.match(regex, self.value): - msg = _("'%s' does not match %s") - raise ValidationError(msg % (self.value, regex)) + raise ValidationError({ + 'value': ValidationError(_("'%(value)s' does not match %(regex)s."), + params={ + 'value': self.value, + 'regex': regex + }), + }) services.register(WebApp) diff --git a/orchestra/apps/webapps/settings.py b/orchestra/apps/webapps/settings.py index 7170d997..814fca94 100644 --- a/orchestra/apps/webapps/settings.py +++ b/orchestra/apps/webapps/settings.py @@ -24,7 +24,7 @@ WEBAPPS_PHPFPM_POOL_PATH = getattr(settings, 'WEBAPPS_PHPFPM_POOL_PATH', WEBAPPS_FCGID_PATH = getattr(settings, 'WEBAPPS_FCGID_PATH', - '/home/httpd/fcgid/%(user)s-%(app_name)s-wrapper') + '/home/httpd/fcgid/%(user)s/%(app_name)s-wrapper') WEBAPPS_TYPES = getattr(settings, 'WEBAPPS_TYPES', { @@ -190,7 +190,7 @@ WEBAPPS_OPTIONS = getattr(settings, 'WEBAPPS_OPTIONS', { ), 'PHP-suhosin.executor.include.whitelist': ( _("PHP - suhosin.executor.include.whitelist"), - r'^(upload|phar)$' + r'.*$' ), 'PHP-upload_max_filesize': ( _("PHP - upload_max_filesize"), diff --git a/orchestra/apps/websites/backends/apache.py b/orchestra/apps/websites/backends/apache.py index 0888a06e..5d898ddf 100644 --- a/orchestra/apps/websites/backends/apache.py +++ b/orchestra/apps/websites/backends/apache.py @@ -37,6 +37,7 @@ class Apache2Backend(ServiceController): SuexecUserGroup {{ user }} {{ group }}\ {% for line in extra_conf.splitlines %} {{ line | safe }}{% endfor %} + IncludeOptional /etc/apache2/extra-vhos[t]/{{ site_unique_name }}.con[f] """ )) apache_conf = apache_conf.render(Context(context)) diff --git a/orchestra/apps/websites/models.py b/orchestra/apps/websites/models.py index 57de95d8..32d5c946 100644 --- a/orchestra/apps/websites/models.py +++ b/orchestra/apps/websites/models.py @@ -82,8 +82,13 @@ class WebsiteOption(models.Model): """ validates name and value according to WEBSITES_WEBSITEOPTIONS """ __, regex = settings.WEBSITES_OPTIONS[self.name] if not re.match(regex, self.value): - msg = _("'%s' does not match %s") - raise ValidationError(msg % (self.value, regex)) + raise ValidationError({ + 'value': ValidationError(_("'%(value)s' does not match %(regex)s."), + params={ + 'value': self.value, + 'regex': regex + }), + }) class Content(models.Model): diff --git a/orchestra/core/validators.py b/orchestra/core/validators.py index 76f3dbac..b11a722c 100644 --- a/orchestra/core/validators.py +++ b/orchestra/core/validators.py @@ -12,6 +12,18 @@ from IPy import IP from ..utils.python import import_class +def all_valid(kwargs): + """ helper function to merge multiple validators at once """ + errors = {} + for field, validator in kwargs.iteritems(): + try: + validator[0](*validator[1:]) + except ValidationError, error: + errors[field] = error + if errors: + raise ValidationError(errors) + + def validate_ipv4_address(value): msg = _("%s is not a valid IPv4 address") % value try: