From da222fdcd05cf752afb21b55b315831a8d12d787 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 20 Feb 2017 11:32:20 +0100 Subject: [PATCH 1/3] Check that resellers can't define unlimited quota. see #1035 --- modoboa/admin/forms/domain.py | 20 +++++++++++++++++--- modoboa/limits/models.py | 5 +++++ modoboa/limits/tests/test_api.py | 10 +++++++++- modoboa/limits/tests/test_import.py | 7 +++++++ modoboa/limits/tests/test_user_limits.py | 19 +++++++++++++++++-- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py index 12db86505..58a5b1f4f 100644 --- a/modoboa/admin/forms/domain.py +++ b/modoboa/admin/forms/domain.py @@ -48,7 +48,7 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): "name", "type", "quota", "default_mailbox_quota", "aliases", "enabled", "enable_dns_checks") - def __init__(self, *args, **kwargs): + def __init__(self, user, *args, **kwargs): self.oldname = None if "instance" in kwargs: self.oldname = kwargs["instance"].name @@ -67,6 +67,7 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): "quota": 3, "default_mailbox_quota": 3 } + self.user = user if len(args) and isinstance(args[0], QueryDict): self._load_from_qdict(args[0], "aliases", DomainNameField) @@ -95,6 +96,17 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): "name", _("A %s with this name already exists") % unicode(label) ) + if self.user.role == "Resellers": + limit = self.user.userobjectlimit_set.get(name="quota") + if limit.max_value != 0: + quota = self.cleaned_data["quota"] + msg = _("You can't define an unlimited quota.") + if quota == 0: + self.add_error("quota", msg) + default_mailbox_quota = self.cleaned_data[ + "default_mailbox_quota"] + if default_mailbox_quota == 0: + self.add_error("default_mailbox_quota", msg) self.aliases = [] for k in cleaned_data.keys(): if not k.startswith("aliases"): @@ -273,7 +285,8 @@ class DomainForm(TabForms): "title": _("General"), "formtpl": "admin/domain_general_form.html", "cls": DomainFormGeneral, - "mandatory": True + "mandatory": True, + "new_args": [request.user] }) cbargs = {"user": self.user} @@ -337,7 +350,8 @@ class DomainWizard(WizardForm): self.add_step( WizardStep( "general", DomainFormGeneral, _("General"), - "admin/domain_general_form.html" + "admin/domain_general_form.html", + [request.user] ) ) results = signals.extra_domain_wizard_steps.send(sender=self.__class__) diff --git a/modoboa/limits/models.py b/modoboa/limits/models.py index 9d3864840..58e3e8c0e 100644 --- a/modoboa/limits/models.py +++ b/modoboa/limits/models.py @@ -10,6 +10,7 @@ from django.utils.translation import ugettext as _ from modoboa.core import models as core_models +from . import lib from . import utils @@ -45,6 +46,10 @@ class ObjectLimitMixin(object): return False field = self.definition["field"] value = getattr(instance, field) + if value == 0: + # Do not allow unlimited value + raise lib.BadLimitValue( + _("You're not allowed to define unlimited values")) try: old_value = instance._loaded_values[field] except (AttributeError, KeyError): diff --git a/modoboa/limits/tests/test_api.py b/modoboa/limits/tests/test_api.py index a35d220f1..08f0aecb2 100644 --- a/modoboa/limits/tests/test_api.py +++ b/modoboa/limits/tests/test_api.py @@ -93,9 +93,17 @@ class APIAdminLimitsTestCase(lib_tests.ModoAPITestCase): self.assertTrue(limit.is_exceeded()) self.assertFalse(quota.is_exceeded()) - data["username"] = "test5.com" + data["name"] = "test5.com" + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 403) + self.assertEqual(response.content, '"Quota: limit reached"') + + data["quota"] = 0 response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 400) + self.assertEqual( + response.content, + '"You\'re not allowed to define unlimited values"') def test_domain_aliases_limit(self): """Check domain aliases limit.""" diff --git a/modoboa/limits/tests/test_import.py b/modoboa/limits/tests/test_import.py index b2813fa6c..0cc11e39a 100644 --- a/modoboa/limits/tests/test_import.py +++ b/modoboa/limits/tests/test_import.py @@ -152,6 +152,13 @@ class UserLimitImportTestCase(LimitImportTestCase): ) self.assertFalse(limit.is_exceeded()) + f = ContentFile("domain; domain2.com; 0; 1; True", name="domains.csv") + response = self.client.post( + reverse("admin:domain_import"), {"sourcefile": f} + ) + self.assertContains( + response, "not allowed to define unlimited values") + f = ContentFile("domain; domain2.com; 2; 1; True", name="domains.csv") response = self.client.post( reverse("admin:domain_import"), {"sourcefile": f} diff --git a/modoboa/limits/tests/test_user_limits.py b/modoboa/limits/tests/test_user_limits.py index 04a4244aa..c871e0b58 100644 --- a/modoboa/limits/tests/test_user_limits.py +++ b/modoboa/limits/tests/test_user_limits.py @@ -76,9 +76,9 @@ class ResourceTestCase(lib_tests.ModoTestCase): reverse("admin:alias_add"), values, status ) - def _create_domain(self, name, status=200, withtpl=False, quota=100): + def _create_domain(self, name, status=200, withtpl=False, **kwargs): values = { - "name": name, "quota": quota, "default_mailbox_quota": 10, + "name": name, "quota": 100, "default_mailbox_quota": 10, "create_dom_admin": False, "create_aliases": False, "stepid": "step3", "type": "domain" } @@ -88,6 +88,7 @@ class ResourceTestCase(lib_tests.ModoTestCase): "dom_admin_username": "admin", "create_aliases": True }) + values.update(kwargs) response = self.ajax_post( reverse("admin:domain_add"), values, status ) @@ -307,6 +308,20 @@ class ResellerTestCase(ResourceTestCase): } self.ajax_post(url, values) + def test_quota_constraints(self): + """Check reseller can't define unlimited quota.""" + response = self._create_domain("domain1.tld", 400, quota=0) + self.assertEqual( + response["form_errors"]["quota"][0], + "You can't define an unlimited quota.") + response = self._create_domain( + "domain1.tld", 400, default_mailbox_quota=0) + self.assertEqual( + response["form_errors"]["default_mailbox_quota"][0], + "You can't define an unlimited quota.") + self.user.userobjectlimit_set.filter(name="quota").update(max_value=0) + response = self._create_domain("domain1.tld", quota=0) + def test_reseller_deletes_domain(self): """Check if all resources are restored after the deletion.""" self._create_domain('domain.tld', withtpl=True) From 840fd73387cbda54acb6430022d55a601c54821c Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 20 Feb 2017 12:18:28 +0100 Subject: [PATCH 2/3] Fixed unit tests. see #1035 --- modoboa/limits/tests/test_api.py | 5 ++++- modoboa/relaydomains/tests.py | 18 +++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/modoboa/limits/tests/test_api.py b/modoboa/limits/tests/test_api.py index 08f0aecb2..74d029abf 100644 --- a/modoboa/limits/tests/test_api.py +++ b/modoboa/limits/tests/test_api.py @@ -96,8 +96,11 @@ class APIAdminLimitsTestCase(lib_tests.ModoAPITestCase): data["name"] = "test5.com" response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 403) - self.assertEqual(response.content, '"Quota: limit reached"') + self.assertEqual(response.content, '"Domains: limit reached"') + self.client.delete( + reverse("external_api:domain-detail", + args=[Domain.objects.get(name="test4.com").pk])) data["quota"] = 0 response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 400) diff --git a/modoboa/relaydomains/tests.py b/modoboa/relaydomains/tests.py index b76606928..1ca7e151f 100644 --- a/modoboa/relaydomains/tests.py +++ b/modoboa/relaydomains/tests.py @@ -18,7 +18,7 @@ from . import models class Operations(object): - def _create_relay_domain(self, name, status=200): + def _create_relay_domain(self, name, status=200, **kwargs): srv, created = models.Service.objects.get_or_create(name='dummy') values = { "name": name, "create_dom_admin": False, "type": "relaydomain", @@ -26,6 +26,7 @@ class Operations(object): "service": srv.id, "enabled": True, "stepid": "step3", "quota": 0, "default_mailbox_quota": 0 } + values.update(kwargs) return self.ajax_post( reverse("admin:domain_add"), values, status @@ -37,7 +38,8 @@ class Operations(object): "name": rdom.domain.name, "target_host": rdom.target_host, "target_port": rdom.target_port, "type": "relaydomain", "service": rdom.service.id, - "quota": 0, "default_mailbox_quota": 0 + "quota": rdom.domain.quota, + "default_mailbox_quota": rdom.domain.default_mailbox_quota } aliases = [alias.name for alias in rdom.domain.domainalias_set.all()] if optype == 'add': @@ -288,13 +290,14 @@ class LimitsTestCase(ModoTestCase, Operations): def setUp(self): """Initialize test.""" super(LimitsTestCase, self).setUp() - self.client.logout() - self.client.login(username='reseller', password='toto') + self.client.force_login(self.user) def test_relay_domains_limit(self): - self._create_relay_domain('relaydomain1.tld') + self._create_relay_domain( + 'relaydomain1.tld', quota=1, default_mailbox_quota=1) self._check_limit('domains', 1, 2) - self._create_relay_domain('relaydomain2.tld') + self._create_relay_domain( + 'relaydomain2.tld', quota=1, default_mailbox_quota=1) self._check_limit('domains', 2, 2) self._create_relay_domain('relaydomain3.tld', 403) self._check_limit('domains', 2, 2) @@ -304,7 +307,8 @@ class LimitsTestCase(ModoTestCase, Operations): self._check_limit('domains', 1, 2) def test_relay_domain_aliases_limit(self): - self._create_relay_domain('relaydomain1.tld') + self._create_relay_domain( + 'relaydomain1.tld', quota=1, default_mailbox_quota=1) self._relay_domain_alias_operation( 'add', 'relaydomain1.tld', 'relay-domain-alias1.tld' ) From a4a9e2597f34670264dc68112f223a468ba66d6c Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 20 Feb 2017 12:45:07 +0100 Subject: [PATCH 3/3] Fixed unit test. see #1035 --- modoboa/limits/tests/test_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modoboa/limits/tests/test_api.py b/modoboa/limits/tests/test_api.py index 74d029abf..58bb15454 100644 --- a/modoboa/limits/tests/test_api.py +++ b/modoboa/limits/tests/test_api.py @@ -80,6 +80,8 @@ class APIAdminLimitsTestCase(lib_tests.ModoAPITestCase): HTTP_AUTHORIZATION='Token ' + self.r_token.key) limit = self.reseller.userobjectlimit_set.get(name="domains") quota = self.reseller.userobjectlimit_set.get(name="quota") + quota.max_value = 3 + quota.save(update_fields=["max_value"]) url = reverse("external_api:domain-list") data = {"name": "test3.com", "quota": 1} response = self.client.post(url, data, format="json")