WIP (handle conflicts)

see #832

modoboa/admin/api.py

@@ -44,3 +44,7 @@ class AccountViewSet(viewsets.ModelViewSet):
             .filter(content_type=ContentType.objects.get_for_model(user)) \
             .values_list('object_id', flat=True)
         return core_models.User.objects.filter(pk__in=ids)
+
+    def perform_destroy(self, instance):
+        """Add custom args to delete call."""
+        instance.delete(self.request.user)

modoboa/admin/callbacks.py

@@ -114,12 +114,9 @@ def import_account_mailbox(user, account, row):
             )
         if not user.can_access(domain):
             raise PermDeniedException
-        try:
-            mb = Mailbox.objects.get(address=mailbox, domain=domain)
-        except Mailbox.DoesNotExist:
-            pass
-        else:
-            raise Conflict(_("Mailbox %s already exists" % account.email))
+        if Mailbox.objects.filter(address=mailbox, domain=domain).exists():
+            raise Conflict(
+                _("Mailbox {} already exists").format(account.email))
         if len(row) == 1:
             quota = None
         else:

modoboa/admin/forms/account.py

@@ -12,7 +12,7 @@ from passwords.fields import PasswordField
 from modoboa.core.models import User
 from modoboa.lib import events, parameters
 from modoboa.lib.email_utils import split_mailbox
-from modoboa.lib.exceptions import PermDeniedException, Conflict, NotFound
+from modoboa.lib.exceptions import PermDeniedException, NotFound
 from modoboa.lib import fields as lib_fields
 from modoboa.lib.form_utils import (
     DynamicForm, TabForms, WizardForm, WizardStep
@@ -43,6 +43,7 @@ class AccountFormGeneral(forms.ModelForm):
     password1 = PasswordField(
         label=ugettext_lazy("Password"), widget=forms.widgets.PasswordInput
     )
+
     password2 = PasswordField(
         label=ugettext_lazy("Confirmation"),
         widget=forms.widgets.PasswordInput,
@@ -253,14 +254,6 @@ class AccountFormMail(forms.Form, DynamicForm):
             raise NotFound(_("Domain does not exist"))
         if not user.can_access(domain):
             raise PermDeniedException
-        try:
-            Mailbox.objects.get(address=locpart, domain=domain)
-        except Mailbox.DoesNotExist:
-            pass
-        else:
-            raise Conflict(
-                _("Mailbox %s already exists" % self.cleaned_data["email"])
-            )
         events.raiseEvent("CanCreate", user, "mailboxes")
         self.mb = Mailbox(address=locpart, domain=domain, user=account,
                           use_domain_quota=self.cleaned_data["quota_act"])
@@ -276,7 +269,6 @@ class AccountFormMail(forms.Form, DynamicForm):
               account.username != self.mb.full_address):
             newaddress = account.username
         if newaddress is not None:
-            self.mb.old_full_address = self.mb.full_address
             local_part, domname = split_mailbox(newaddress)
             try:
                 domain = Domain.objects.get(name=domname)
@@ -284,15 +276,15 @@ class AccountFormMail(forms.Form, DynamicForm):
                 raise NotFound(_("Domain does not exist"))
             if not user.can_access(domain):
                 raise PermDeniedException
-            self.mb.rename(local_part, domain)
-
         self.mb.use_domain_quota = self.cleaned_data["quota_act"]
         override_rules = True \
             if not self.mb.quota or user.has_perm("admin.add_domain") \
             else False
         self.mb.set_quota(self.cleaned_data["quota"], override_rules)
         self.mb.save()
-        events.raiseEvent('MailboxModified', self.mb)
+        if newaddress:
+            self.mb.rename(local_part, domain)
+        events.raiseEvent("MailboxModified", self.mb)
 
     def _update_aliases(self, user, account):
         """Update mailbox aliases."""

modoboa/admin/forms/domain.py

@@ -121,28 +121,6 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
                 )
         return cleaned_data
 
-    def update_mailbox_quotas(self, domain):
-        """Update all quota records associated to this domain.
-
-        This method must be called only when a domain gets renamed. As
-        the primary key used for a quota is an email address, rename a
-        domain will change all associated email addresses, so it will
-        change the primary keys used for quotas. The consequence is we
-        can't issue regular UPDATE queries using the .save() method of
-        a Quota instance (it will trigger an INSERT as the primary key
-        has changed).
-
-        So, we use this ugly hack to bypass this behaviour. It is not
-        perfomant at all as it will generate one query per quota
-        record to update.
-        """
-        for q in Quota.objects.filter(username__contains="@%s" % self.oldname):
-            username = q.username.replace(
-                '@%s' % self.oldname, '@%s' % domain.name)
-            Quota.objects.create(
-                username=username, bytes=q.bytes, messages=q.messages)
-            q.delete()
-
     def save(self, user, commit=True, domalias_post_create=False):
         """Custom save method.
 

modoboa/admin/handlers.py

@@ -29,6 +29,12 @@ def update_domain_mailboxes(sender, instance, **kwargs):
         mb.rename_dir(instance.old_mail_homes[mb.pk])
 
 
+@receiver(signals.post_save, sender=models.Mailbox)
+def rename_mailbox(sender, instance, **kwargs):
+    """Check if mailbox must be renamed on local FS."""
+    pass
+
+
 @receiver(signals.post_save, sender=models.DomainAlias)
 def create_alias_for_domainalias(sender, instance, **kwargs):
     """Create a dedicated alias for domain alias."""

modoboa/admin/models/mailbox.py

@@ -15,8 +15,8 @@ import reversion
 from .base import AdminObject
 from .domain import Domain
 from modoboa.core.models import User
-from modoboa.lib import parameters
-from modoboa.lib.exceptions import BadRequest, InternalError
+from modoboa.lib import events, parameters
+from modoboa.lib.exceptions import BadRequest, Conflict, InternalError
 from modoboa.lib.sysutils import exec_cmd
 
 
@@ -96,6 +96,8 @@ class Mailbox(AdminObject):
     def __init__(self, *args, **kwargs):
         super(Mailbox, self).__init__(*args, **kwargs)
         self.__mail_home = None
+        self.old_full_address = self.full_address
+        self.old_mail_home = self.mail_home
 
     def __str__(self):
         return smart_text(self.full_address)
@@ -179,11 +181,12 @@ class Mailbox(AdminObject):
         self._quota_value = instance
 
     def rename_dir(self, old_mail_home):
+        """Rename local directory if needed."""
         hm = parameters.get_admin("HANDLE_MAILBOXES", raise_error=False)
         if hm is None or hm == "no":
             return
         MailboxOperation.objects.create(
-            mailbox=self, type='rename', argument=old_mail_home
+            mailbox=self, type="rename", argument=old_mail_home
         )
 
     def rename(self, address, domain):
@@ -197,7 +200,6 @@ class Mailbox(AdminObject):
         :param Domain domain: the new mailbox's domain
 
         """
-        old_mail_home = self.mail_home
         old_qvalue = self.quota_value
         self.address = address
         self.domain = domain
@@ -206,7 +208,7 @@ class Mailbox(AdminObject):
             messages=old_qvalue.messages
         )
         old_qvalue.delete()
-        self.rename_dir(old_mail_home)
+        self.rename_dir(self.old_mail_home)
 
     def delete_dir(self):
         hm = parameters.get_admin("HANDLE_MAILBOXES", raise_error=False)
@@ -280,8 +282,16 @@ class Mailbox(AdminObject):
     def save(self, *args, **kwargs):
         """Custom save.
 
-        We just make sure a quota record is defined for this mailbox.
+        We check that the address is unique and we make sure a quota
+        record is defined for this mailbox.
+
         """
+        qset = Mailbox.objects.filter(address=self.address, domain=self.domain)
+        if self.pk:
+            qset = qset.exclude(pk=self.pk)
+        if qset.exists():
+            raise Conflict(
+                _("Mailbox {} already exists").format(self))
         if self.quota_value is None:
             self.quota_value, created = Quota.objects.get_or_create(
                 username=self.full_address)

modoboa/admin/serializers.py

@@ -9,7 +9,7 @@ from passwords import validators
 
 from modoboa.admin import models as admin_models
 from modoboa.core import models as core_models
-from modoboa.lib import permissions, email_utils
+from modoboa.lib import permissions, email_utils, fields as lib_fields
 
 from . import models
 
@@ -22,11 +22,17 @@ class DomainSerializer(serializers.ModelSerializer):
         model = models.Domain
         fields = ("pk", "name", "quota", "enabled", "type", )
 
+    def create(self, validated_data):
+        """Set permissions."""
+        domain = models.Domain(**validated_data)
+        domain.save(creator=self.context["request"].user)
+        return domain
+
 
 class MailboxSerializer(serializers.ModelSerializer):
     """Base mailbox serializer."""
 
-    full_address = serializers.EmailField()
+    full_address = lib_fields.DRFEmailFieldUTF8()
 
     class Meta:
         model = models.Mailbox
@@ -89,21 +95,37 @@ class CreateAccountSerializer(AccountSerializer):
             raise serializers.ValidationError({
                 "master_user": _("Not allowed for this role.")
             })
+        if data["role"] == "SimpleUsers":
+            mailbox = data.get("mailbox")
+            if mailbox is None:
+                data["mailbox"] = {
+                    "full_address": data["username"], "use_domain_quota": True
+                }
+            elif mailbox["full_address"] != data["username"]:
+                raise serializers.ValidationError({
+                    "username": _("Must be equal to mailbox full_address")
+                })
         return data
 
     def create(self, validated_data):
-        """Call the set_password method."""
+        """Create appropriate objects."""
+        creator = self.context["request"].user
         mailbox_data = validated_data.pop("mailbox", None)
         role = validated_data.pop("role")
         user = core_models.User(**validated_data)
         user.set_password(validated_data["password"])
-        user.save()
-        user.role = role
+        user.save(creator=creator)
         if mailbox_data:
-            address, domain_name = email_utils.split_mailbox(
-                mailbox_data.pop("full_address"))
+            full_address = mailbox_data.pop("full_address")
+            address, domain_name = email_utils.split_mailbox(full_address)
             domain = get_object_or_404(
                 admin_models.Domain, name=domain_name)
-            admin_models.Mailbox.objects.create(
+            mb = admin_models.Mailbox(
                 user=user, address=address, domain=domain, **mailbox_data)
+            mb.set_quota(
+                mailbox_data.get("quota"), creator.has_perm("admin.add_domain")
+            )
+            mb.save(creator=creator)
+            user.email = full_address
+        user.role = role
         return user

modoboa/admin/tests/__init__.py

@@ -2,7 +2,7 @@
 
 from .account import AccountTestCase, PermissionsTestCase
 from .alias import AliasTestCase
-from .api import APITestCase
+from .api import DomainAPITestCase, AccountAPITestCase
 from .domain import DomainTestCase
 from .domain_alias import DomainAliasTestCase
 from .export import ExportTestCase
@@ -12,9 +12,10 @@ from .password_schemes import PasswordSchemesTestCase
 from .user import ForwardTestCase
 
 __all__ = [
+    'AccountAPITestCase',
     'AccountTestCase',
     'AliasTestCase',
-    'APITestCase',
+    'DomainAPITestCase',
     'DomainTestCase',
     'DomainAliasTestCase',
     'ExportTestCase',

modoboa/admin/tests/account.py

@@ -67,6 +67,31 @@ class AccountTestCase(ModoTestCase):
                 alias__internal=True).exists()
         )
 
+    def test_conflicts(self):
+        """Check if unicity constraints are respected."""
+        values = {
+            "username": "user@test.com",
+            "password1": "Toto1234", "password2": "Toto1234",
+            "role": "SimpleUsers", "quota_act": True,
+            "is_active": True, "email": "user@test.com",
+            "stepid": "step2"
+        }
+        self.ajax_post(reverse("admin:account_add"), values, status=400)
+
+        values.update({"username": "fakeuser@test.com",
+                       "email": "fakeuser@test.com"})
+        self.ajax_post(reverse("admin:account_add"), values)
+        account = User.objects.get(username="fakeuser@test.com")
+        values = {
+            "username": "user@test.com",
+            "role": "SimpleUsers", "quota_act": True,
+            "is_active": True, "email": "user@test.com",
+        }
+        self.ajax_post(
+            reverse("admin:account_change", args=[account.pk]), values,
+            status=400
+        )
+
     def test_utf8_username(self):
         """Create an account with non-ASCII characters."""
         values = dict(

modoboa/admin/tests/api.py

@@ -1,5 +1,7 @@
+# coding: utf-8
 """Admin API related tests."""
 
+import copy
 import json
 
 from django.core.urlresolvers import reverse
@@ -13,14 +15,13 @@ from .. import factories
 from .. import models
 
 
-class APITestCase(ModoAPITestCase):
-
+class DomainAPITestCase(ModoAPITestCase):
     """Check API."""
 
     @classmethod
     def setUpTestData(cls):
         """Create test data."""
-        super(APITestCase, cls).setUpTestData()
+        super(DomainAPITestCase, cls).setUpTestData()
         factories.populate_database()
         cls.da_token = Token.objects.create(
             user=core_models.User.objects.get(username="admin@test.com"))
@@ -71,6 +72,9 @@ class APITestCase(ModoAPITestCase):
 
         response = self.client.put(url, {"name": "test42.com", "quota": 1000})
         self.assertEqual(response.status_code, 200)
+        self.assertTrue(
+            models.Mailbox.objects.filter(
+                address="user", domain__name="test42.com").exists())
 
     def test_delete_domain(self):
         """Try to delete a domain."""
@@ -80,6 +84,28 @@ class APITestCase(ModoAPITestCase):
         self.assertEqual(response.status_code, 204)
         self.assertFalse(models.Domain.objects.filter(pk=domain.pk).exists())
 
+
+class AccountAPITestCase(ModoAPITestCase):
+    """Check Account API."""
+
+    ACCOUNT_DATA = {
+        "username": "fromapi@test.com",
+        "role": "SimpleUsers",
+        "password": "Toto1234",
+        "mailbox": {
+            "full_address": "fromapi@test.com",
+            "quota": 10
+        }
+    }
+
+    @classmethod
+    def setUpTestData(cls):
+        """Create test data."""
+        super(AccountAPITestCase, cls).setUpTestData()
+        factories.populate_database()
+        cls.da_token = Token.objects.create(
+            user=core_models.User.objects.get(username="admin@test.com"))
+
     def test_get_accounts(self):
         """Retrieve a list of accounts."""
         url = reverse("external_api:account-list")
@@ -90,17 +116,8 @@ class APITestCase(ModoAPITestCase):
 
     def test_create_account(self):
         """Try to create a new account."""
-        data = {
-            "username": "fromapi@test.com",
-            "role": "SimpleUsers",
-            "password": "Toto1234",
-            "mailbox": {
-                "full_address": "fromapi@test.com",
-                "quota": 10
-            }
-        }
         url = reverse("external_api:account-list")
-        response = self.client.post(url, data, format="json")
+        response = self.client.post(url, self.ACCOUNT_DATA, format="json")
         self.assertEqual(response.status_code, 201)
 
         account = json.loads(response.content)
@@ -108,17 +125,56 @@ class APITestCase(ModoAPITestCase):
         self.assertIsNot(user, None)
         self.assertIsNot(user.mailbox, None)
 
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        data["username"] = "fromapi_ééé@test.com"
+        data["mailbox"]["full_address"] = data["username"]
+        url = reverse("external_api:account-list")
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 201)
+
+    def test_create_domainadmin_account(self):
+        """Try to create a domain admin."""
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        data["role"] = "DomainAdmins"
+        url = reverse("external_api:account-list")
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 201)
+
+        data["username"] = "domain_admin"
+        del data["mailbox"]
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 201)
+
+    def test_create_account_with_no_mailbox(self):
+        """Try to create a new account."""
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        del data["mailbox"]
+        url = reverse("external_api:account-list")
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 201)
+        account = json.loads(response.content)
+        user = core_models.User.objects.filter(pk=account["pk"]).first()
+        self.assertIsNot(user, None)
+        self.assertIsNot(user.mailbox, None)
+        self.assertEqual(user.mailbox.quota, user.mailbox.domain.quota)
+
+    def test_create_existing_account(self):
+        """Check if unicity is respected."""
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        data["username"] = "user@test.com"
+        url = reverse("external_api:account-list")
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 400)
+
+        data.update({"username": "domainadmin", "role": "DomainAdmins"})
+        data["mailbox"]["full_address"] = "admin@test.com"
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 409)
+
     def test_create_account_bad_password(self):
         """Try to create a new account."""
-        data = {
-            "username": "fromapi@test.com",
-            "role": "SimpleUsers",
-            "password": "toto",  # Bad password
-            "mailbox": {
-                "full_address": "fromapi@test.com",
-                "quota": 10
-            }
-        }
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        data["password"] = "toto"
         url = reverse("external_api:account-list")
         response = self.client.post(url, data, format="json")
         self.assertEqual(response.status_code, 400)
@@ -127,18 +183,23 @@ class APITestCase(ModoAPITestCase):
 
     def test_create_account_bad_master_user(self):
         """Try to create a new account."""
-        data = {
-            "username": "fromapi@test.com",
-            "role": "SimpleUsers",
-            "master_user": True,
-            "password": "Toto1234",
-            "mailbox": {
-                "full_address": "fromapi@test.com",
-                "quota": 10
-            }
-        }
+        data = copy.deepcopy(self.ACCOUNT_DATA)
+        data["master_user"] = True
         url = reverse("external_api:account-list")
         response = self.client.post(url, data, format="json")
         self.assertEqual(response.status_code, 400)
         errors = json.loads(response.content)
         self.assertIn("master_user", errors)
+
+    def test_update_account(self):
+        """Try to update an account."""
+        pass
+
+    def test_delete_account(self):
+        """Try to delete an account."""
+        account = core_models.User.objects.get(username="user@test.com")
+        url = reverse("external_api:account-detail", args=[account.pk])
+        response = self.client.delete(url)
+        self.assertEqual(response.status_code, 204)
+        self.assertFalse(
+            core_models.User.objects.filter(pk=account.pk).exists())

modoboa/core/models.py

@@ -349,6 +349,7 @@ class User(PermissionsMixin):
         self.save()
 
     def post_create(self, creator):
+        """Grant permission on this user to creator."""
         from modoboa.lib.permissions import grant_access_to_object
         grant_access_to_object(creator, self, is_owner=True)
         events.raiseEvent("AccountCreated", self)

modoboa/lib/fields.py

@@ -3,11 +3,12 @@
 from django import forms
 from django.utils.translation import ugettext_lazy
 
+from rest_framework import serializers
+
 from . import validators
 
 
 class DomainNameField(forms.fields.CharField):
-
     """A subclass of CharField that only accepts a valid domain name."""
 
     default_error_messages = {
@@ -22,17 +23,29 @@ class DomainNameField(forms.fields.CharField):
 
 
 class UTF8EmailField(forms.fields.EmailField):
-
     """A subclass of EmailField to support UTF8 addresses."""
 
     default_validators = [validators.validate_utf8_email]
 
 
 class UTF8AndEmptyUserEmailField(forms.fields.EmailField):
-
     """
     A subclass of EmailField to support UTF8 addresses (with or without
     local part).
     """
 
     default_validators = [validators.validate_utf8_and_empty_user_email]
+
+
+class DRFEmailFieldUTF8(serializers.CharField):
+    """Custom DRF email field to support UTF8."""
+
+    default_error_messages = {
+        "invalid": ugettext_lazy("Enter a valid email address.")
+    }
+
+    def __init__(self, **kwargs):
+        super(DRFEmailFieldUTF8, self).__init__(**kwargs)
+        validator = validators.UTF8EmailValidator(
+            message=self.error_messages["invalid"])
+        self.validators.append(validator)

modoboa/lib/middleware.py

@@ -21,21 +21,23 @@ class AjaxLoginRedirect(object):
 
 
 class CommonExceptionCatcher(object):
+    """Modoboa exceptions catcher."""
 
     def process_exception(self, request, exception):
         if not isinstance(exception, ModoboaException):
             return None
 
-        if not request.is_ajax():
-            return _render_error(
-                request, user_context=dict(error=str(exception))
+        if request.is_ajax() or "/api/" in request.path:
+            if exception.http_code is None:
+                return ajax_response(
+                    request, status="ko", respmsg=unicode(exception),
+                    norefresh=True
+                )
+            return render_to_json_response(
+                unicode(exception), status=exception.http_code
             )
-        if exception.http_code is None:
-            return ajax_response(
-                request, status="ko", respmsg=unicode(exception), norefresh=True
-            )
-        return render_to_json_response(
-            unicode(exception), status=exception.http_code
+        return _render_error(
+            request, user_context=dict(error=str(exception))
         )