diff --git a/modoboa/admin/api.py b/modoboa/admin/api.py index 7d1a2c83b..d36184980 100644 --- a/modoboa/admin/api.py +++ b/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) diff --git a/modoboa/admin/callbacks.py b/modoboa/admin/callbacks.py index 95c21da90..b894673c0 100644 --- a/modoboa/admin/callbacks.py +++ b/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: diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py index c7c61ecca..9bdd251c3 100644 --- a/modoboa/admin/forms/account.py +++ b/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.""" diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py index 86234695a..9bd579cd2 100644 --- a/modoboa/admin/forms/domain.py +++ b/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. diff --git a/modoboa/admin/handlers.py b/modoboa/admin/handlers.py index 04e3b2175..422177987 100644 --- a/modoboa/admin/handlers.py +++ b/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.""" diff --git a/modoboa/admin/models/mailbox.py b/modoboa/admin/models/mailbox.py index 5388ab3b2..3c214691f 100644 --- a/modoboa/admin/models/mailbox.py +++ b/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) diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index ff8066a6f..2b79f2e5e 100644 --- a/modoboa/admin/serializers.py +++ b/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 diff --git a/modoboa/admin/tests/__init__.py b/modoboa/admin/tests/__init__.py index b950e511d..de79ff43c 100644 --- a/modoboa/admin/tests/__init__.py +++ b/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', diff --git a/modoboa/admin/tests/account.py b/modoboa/admin/tests/account.py index c05b6e856..83a682f6e 100644 --- a/modoboa/admin/tests/account.py +++ b/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( diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index 9a0978534..27adabf96 100644 --- a/modoboa/admin/tests/api.py +++ b/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()) diff --git a/modoboa/core/models.py b/modoboa/core/models.py index 2c0d6ad6f..82621b172 100644 --- a/modoboa/core/models.py +++ b/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) diff --git a/modoboa/lib/fields.py b/modoboa/lib/fields.py index d38322d21..a797c055f 100644 --- a/modoboa/lib/fields.py +++ b/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) diff --git a/modoboa/lib/middleware.py b/modoboa/lib/middleware.py index b6da7c8c9..aa971bd1a 100644 --- a/modoboa/lib/middleware.py +++ b/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)) )