From 8e348ef6961ef7b34b1d044ae48956b2d0c2907d Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Tue, 16 Feb 2016 16:02:09 +0100 Subject: [PATCH 01/10] WIP. see #832 --- modoboa/admin/api.py | 24 ++++++++ modoboa/admin/callbacks.py | 37 ----------- modoboa/admin/forms/account.py | 2 +- modoboa/admin/forms/domain.py | 2 +- modoboa/admin/serializers.py | 95 +++++++++++++++++++++++++++++ modoboa/admin/tests/api.py | 65 ++++++++++++++++++++ modoboa/admin/urls_api.py | 1 + modoboa/core/__init__.py | 36 ++++++++++- modoboa/core/models.py | 18 +++++- modoboa/lib/permissions.py | 21 +++++-- modoboa/limits/general_callbacks.py | 31 +--------- 11 files changed, 255 insertions(+), 77 deletions(-) diff --git a/modoboa/admin/api.py b/modoboa/admin/api.py index 2235ba2f1..7d1a2c83b 100644 --- a/modoboa/admin/api.py +++ b/modoboa/admin/api.py @@ -1,8 +1,12 @@ """Admin API.""" +from django.contrib.contenttypes.models import ContentType + from rest_framework import viewsets from rest_framework.permissions import DjangoModelPermissions +from modoboa.core import models as core_models + from . import models from . import serializers @@ -20,3 +24,23 @@ class DomainViewSet(viewsets.ModelViewSet): def perform_destroy(self, instance): """Add custom args to delete call.""" instance.delete(self.request.user) + + +class AccountViewSet(viewsets.ModelViewSet): + """ViewSet for User/Mailbox.""" + + permission_classes = [DjangoModelPermissions, ] + + def get_serializer_class(self): + """Return a serializer.""" + if self.request.method == "GET": + return serializers.AccountSerializer + return serializers.CreateAccountSerializer + + def get_queryset(self): + """Filter queryset based on current user.""" + user = self.request.user + ids = user.objectaccess_set \ + .filter(content_type=ContentType.objects.get_for_model(user)) \ + .values_list('object_id', flat=True) + return core_models.User.objects.filter(pk__in=ids) diff --git a/modoboa/admin/callbacks.py b/modoboa/admin/callbacks.py index f0983c875..95c21da90 100644 --- a/modoboa/admin/callbacks.py +++ b/modoboa/admin/callbacks.py @@ -12,43 +12,6 @@ from .models import ( ) -PERMISSIONS = { - "DomainAdmins": [ - ["core", "user", "add_user"], - ["core", "user", "change_user"], - ["core", "user", "delete_user"], - ["admin", "domain", "view_domains"], - ["admin", "domain", "view_domain"], - ["admin", "mailbox", "add_mailbox"], - ["admin", "mailbox", "change_mailbox"], - ["admin", "mailbox", "delete_mailbox"], - ["admin", "alias", "add_alias"], - ["admin", "alias", "change_alias"], - ["admin", "alias", "delete_alias"], - ["admin", "mailbox", "view_mailboxes"], - ["admin", "alias", "view_aliases"], - ] -} - - -@events.observe("GetExtraRoles") -def extra_roles(user, account): - """Return new roles.""" - filters = events.raiseQueryEvent( - "UserCanSetRole", user, "DomainAdmins", account - ) - if user.has_perm("admin.add_domain") and \ - (not filters or True in filters): - return [("DomainAdmins", _("Domain administrator"))] - return [] - - -@events.observe("GetExtraRolePermissions") -def extra_permissions(rolename): - """Return extra permissions for :kw:`rolename`.""" - return PERMISSIONS.get(rolename, []) - - @events.observe("AdminMenuDisplay") def admin_menu(target, user): if target != "top_menu": diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py index 312a2e7d4..c7c61ecca 100644 --- a/modoboa/admin/forms/account.py +++ b/modoboa/admin/forms/account.py @@ -160,7 +160,7 @@ class AccountFormGeneral(forms.ModelForm): and self.cleaned_data["password1"] != "": account.set_password(self.cleaned_data["password1"]) account.save() - account.set_role(self.cleaned_data["role"]) + account.role = self.cleaned_data["role"] return account diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py index 7d4806168..86234695a 100644 --- a/modoboa/admin/forms/domain.py +++ b/modoboa/admin/forms/domain.py @@ -250,7 +250,7 @@ class DomainFormOptions(forms.Form): da = User(username=username, email=username, is_active=True) da.set_password("password") da.save() - da.set_role("DomainAdmins") + da.role = "DomainAdmins" da.post_create(user) mb = Mailbox( diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index ca1a0ea4f..ff8066a6f 100644 --- a/modoboa/admin/serializers.py +++ b/modoboa/admin/serializers.py @@ -1,6 +1,15 @@ """Admin serializers.""" +from django.core.exceptions import ValidationError +from django.shortcuts import get_object_or_404 +from django.utils.translation import ugettext as _ + from rest_framework import serializers +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 . import models @@ -12,3 +21,89 @@ class DomainSerializer(serializers.ModelSerializer): class Meta: model = models.Domain fields = ("pk", "name", "quota", "enabled", "type", ) + + +class MailboxSerializer(serializers.ModelSerializer): + """Base mailbox serializer.""" + + full_address = serializers.EmailField() + + class Meta: + model = models.Mailbox + fields = ("full_address", "use_domain_quota", "quota", ) + + +class AccountSerializer(serializers.ModelSerializer): + """Base account serializer.""" + + role = serializers.SerializerMethodField() + mailbox = MailboxSerializer(required=False) + + class Meta: + model = core_models.User + fields = ( + "pk", "username", "first_name", "last_name", "is_active", + "master_user", "mailbox", "role", "language", "phone_number", + "secondary_email", + ) + + def __init__(self, *args, **kwargs): + """Adapt fields to current user.""" + super(AccountSerializer, self).__init__(*args, **kwargs) + user = self.context["request"].user + if not user.is_superuser: + del self.fields["master_user"] + + def get_role(self, account): + """Return role.""" + return account.group + + +class CreateAccountSerializer(AccountSerializer): + """Serializer to create account.""" + + class Meta(AccountSerializer.Meta): + fields = AccountSerializer.Meta.fields + ( + "password", ) + + def __init__(self, *args, **kwargs): + """Adapt fields to current user.""" + super(CreateAccountSerializer, self).__init__(*args, **kwargs) + user = self.context["request"].user + self.fields["role"] = serializers.ChoiceField( + choices=permissions.get_account_roles(user)) + + def validate_password(self, value): + """Check password constraints.""" + try: + validators.validate_length(value) + validators.complexity(value) + except ValidationError as exc: + raise serializers.ValidationError(exc.messages[0]) + return value + + def validate(self, data): + """Check constraints.""" + master_user = data.get("master_user", False) + if master_user and data["role"] != "SuperAdmins": + raise serializers.ValidationError({ + "master_user": _("Not allowed for this role.") + }) + return data + + def create(self, validated_data): + """Call the set_password method.""" + 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 + if mailbox_data: + address, domain_name = email_utils.split_mailbox( + mailbox_data.pop("full_address")) + domain = get_object_or_404( + admin_models.Domain, name=domain_name) + admin_models.Mailbox.objects.create( + user=user, address=address, domain=domain, **mailbox_data) + return user diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index d6aa4c9b4..9a0978534 100644 --- a/modoboa/admin/tests/api.py +++ b/modoboa/admin/tests/api.py @@ -1,5 +1,7 @@ """Admin API related tests.""" +import json + from django.core.urlresolvers import reverse from rest_framework.authtoken.models import Token @@ -77,3 +79,66 @@ class APITestCase(ModoAPITestCase): response = self.client.delete(url) self.assertEqual(response.status_code, 204) self.assertFalse(models.Domain.objects.filter(pk=domain.pk).exists()) + + def test_get_accounts(self): + """Retrieve a list of accounts.""" + url = reverse("external_api:account-list") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + response = json.loads(response.content) + self.assertEqual(len(response), 5) + + 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") + 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) + + 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 + } + } + 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("password", errors) + + 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 + } + } + 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) diff --git a/modoboa/admin/urls_api.py b/modoboa/admin/urls_api.py index b3e1c33fd..f83d0d378 100644 --- a/modoboa/admin/urls_api.py +++ b/modoboa/admin/urls_api.py @@ -7,5 +7,6 @@ from . import api router = routers.SimpleRouter() router.register(r"domains", api.DomainViewSet, base_name="domain") +router.register(r"accounts", api.AccountViewSet, base_name="account") urlpatterns = router.urls diff --git a/modoboa/core/__init__.py b/modoboa/core/__init__.py index b345b0939..adbda48b9 100644 --- a/modoboa/core/__init__.py +++ b/modoboa/core/__init__.py @@ -6,7 +6,41 @@ from modoboa.lib import events PERMISSIONS = { - "SimpleUsers": [] + "SimpleUsers": [], + "DomainAdmins": [ + ["core", "user", "add_user"], + ["core", "user", "change_user"], + ["core", "user", "delete_user"], + ["admin", "domain", "view_domains"], + ["admin", "domain", "view_domain"], + ["admin", "mailbox", "add_mailbox"], + ["admin", "mailbox", "change_mailbox"], + ["admin", "mailbox", "delete_mailbox"], + ["admin", "alias", "add_alias"], + ["admin", "alias", "change_alias"], + ["admin", "alias", "delete_alias"], + ["admin", "mailbox", "view_mailboxes"], + ["admin", "alias", "view_aliases"], + ], + "Resellers": [ + ["core", "user", "add_user"], + ["core", "user", "change_user"], + ["core", "user", "delete_user"], + ["admin", "domain", "view_domains"], + ["admin", "domain", "view_domain"], + ["admin", "mailbox", "add_mailbox"], + ["admin", "mailbox", "change_mailbox"], + ["admin", "mailbox", "delete_mailbox"], + ["admin", "alias", "add_alias"], + ["admin", "alias", "change_alias"], + ["admin", "alias", "delete_alias"], + ["admin", "mailbox", "view_mailboxes"], + ["admin", "alias", "view_aliases"], + ["admin", "domain", "view_domains"], + ["admin", "domain", "add_domain"], + ["admin", "domain", "change_domain"], + ["admin", "domain", "delete_domain"] + ] } diff --git a/modoboa/core/models.py b/modoboa/core/models.py index 519546806..2c0d6ad6f 100644 --- a/modoboa/core/models.py +++ b/modoboa/core/models.py @@ -234,6 +234,7 @@ class User(PermissionsMixin): @property def group(self): + """FIXME: DEPRECATED""" if self.is_superuser: return "SuperAdmins" try: @@ -312,7 +313,18 @@ class User(PermissionsMixin): return True return False - def set_role(self, role): + @property + def role(self): + """Return user role.""" + if self.is_superuser: + return "SuperAdmins" + try: + return self.groups.all()[0].name + except IndexError: + return "---" + + @role.setter + def role(self, role): """Set administrative role for this account :param string role: the role to set @@ -407,7 +419,7 @@ class User(PermissionsMixin): self.last_name = row[4].strip() self.is_active = (row[5].strip() in ["True", "1", "yes", "y"]) self.save() - self.set_role(desired_role) + self.role = desired_role self.post_create(user) if len(row) < 8: return @@ -449,7 +461,7 @@ def populate_callback(user, group='SimpleUsers'): from modoboa.lib.permissions import grant_access_to_object sadmins = User.objects.filter(is_superuser=True) - user.set_role(group) + user.role = group user.post_create(sadmins[0]) for su in sadmins[1:]: grant_access_to_object(su, user) diff --git a/modoboa/lib/permissions.py b/modoboa/lib/permissions.py index fe5fd62e1..da6ac8d49 100644 --- a/modoboa/lib/permissions.py +++ b/modoboa/lib/permissions.py @@ -2,11 +2,16 @@ from django.contrib.auth.models import Group, Permission from django.contrib.contenttypes.models import ContentType -from django.utils.translation import ugettext as _ +from django.utils.translation import ugettext_lazy from modoboa.core.models import ObjectAccess, User from modoboa.lib import events +SIMPLEUSERS_ROLE = ("SimpleUsers", ugettext_lazy("Simple user")) +DOMAINADMINS_ROLE = ("DomainAdmins", ugettext_lazy("Domain administrator")) +RESELLERS_ROLE = ("Resellers", ugettext_lazy("Reseller")) +SUPERADMINS_ROLE = ("SuperAdmins", ugettext_lazy("Super administrator")) + def get_account_roles(user, account=None): """Return the list of available account roles. @@ -18,11 +23,17 @@ def get_account_roles(user, account=None): :param ``User`` account: account beeing modified (None on creation) :return: list of strings """ - std_roles = [("SimpleUsers", _("Simple user"))] + result = [SIMPLEUSERS_ROLE] + filters = events.raiseQueryEvent( + "UserCanSetRole", user, "DomainAdmins", account + ) + if user.has_perm("admin.add_domain") and \ + (not filters or True in filters): + result += [DOMAINADMINS_ROLE] if user.is_superuser: - std_roles += [("SuperAdmins", _("Super administrator"))] - std_roles += events.raiseQueryEvent("GetExtraRoles", user, account) - return sorted(std_roles, key=lambda role: role[1]) + result += [RESELLERS_ROLE, SUPERADMINS_ROLE] + result += events.raiseQueryEvent("GetExtraRoles", user, account) + return sorted(result, key=lambda role: role[1]) def grant_access_to_object(user, obj, is_owner=False): diff --git a/modoboa/limits/general_callbacks.py b/modoboa/limits/general_callbacks.py index c9b406c15..e1cac86e1 100644 --- a/modoboa/limits/general_callbacks.py +++ b/modoboa/limits/general_callbacks.py @@ -4,37 +4,10 @@ from django.utils.translation import ugettext as _ from modoboa.lib import events -from modoboa.admin.callbacks import PERMISSIONS as ADMIN_PERMS - from .forms import ResourcePoolForm from .models import LimitTemplates -PERMISSIONS = { - "Resellers": ( - ADMIN_PERMS.get("DomainAdmins") + - [["admin", "domain", "view_domains"], - ["admin", "domain", "add_domain"], - ["admin", "domain", "change_domain"], - ["admin", "domain", "delete_domain"]] - ) -} - - -@events.observe("GetExtraRoles") -def get_extra_roles(user, account): - """Return additional roles.""" - if user.is_superuser: - return [("Resellers", _("Reseller")), ] - return [] - - -@events.observe("GetExtraRolePermissions") -def extra_permissions(rolename): - """Return extra permissions for Resellers.""" - return PERMISSIONS.get(rolename, []) - - @events.observe("ExtraAdminContent") def display_pool_usage(user, target, currentpage): from django.template.loader import render_to_string @@ -49,8 +22,8 @@ def display_pool_usage(user, target, currentpage): names = [ tpl[0] for tpl in LimitTemplates().templates if tpl[0] not in ["domain_admins_limit", "mailboxes_limit", - "mailbox_aliases_limit"] - and (len(tpl) == 3 or tpl[3] == user.group) + "mailbox_aliases_limit"] and + (len(tpl) == 3 or tpl[3] == user.group) ] limits = user.limitspool.limit_set.filter(name__in=names, maxvalue__gt=0) From 000aba6569e96975665897dabb5a354567cb48e9 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 22 Feb 2016 09:33:18 +0100 Subject: [PATCH 02/10] WIP (handle conflicts) see #832 --- modoboa/admin/api.py | 4 + modoboa/admin/callbacks.py | 9 +-- modoboa/admin/forms/account.py | 18 ++--- modoboa/admin/forms/domain.py | 22 ------ modoboa/admin/handlers.py | 6 ++ modoboa/admin/models/mailbox.py | 22 ++++-- modoboa/admin/serializers.py | 38 ++++++++-- modoboa/admin/tests/__init__.py | 5 +- modoboa/admin/tests/account.py | 25 +++++++ modoboa/admin/tests/api.py | 125 ++++++++++++++++++++++++-------- modoboa/core/models.py | 1 + modoboa/lib/fields.py | 19 ++++- modoboa/lib/middleware.py | 20 ++--- 13 files changed, 213 insertions(+), 101 deletions(-) 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)) ) From b6f1282c7edbd9d4965577df2572d524fc15d923 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 2 Mar 2016 15:57:09 +0100 Subject: [PATCH 03/10] WIP see #832 --- modoboa/admin/forms/account.py | 30 +++-------------- modoboa/admin/models/mailbox.py | 38 +++++++++++++++++++--- modoboa/admin/serializers.py | 45 +++++++++++++++++++------- modoboa/admin/static/admin/js/admin.js | 5 +++ modoboa/admin/tests/account.py | 5 +-- modoboa/admin/tests/api.py | 21 +++++++++++- 6 files changed, 99 insertions(+), 45 deletions(-) diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py index 9bdd251c3..22729bf5f 100644 --- a/modoboa/admin/forms/account.py +++ b/modoboa/admin/forms/account.py @@ -124,6 +124,7 @@ class AccountFormGeneral(forms.ModelForm): return self.cleaned_data["role"] def clean_username(self): + """username must be a valid email address for simple users.""" if "role" not in self.cleaned_data: return self.cleaned_data["username"] if self.cleaned_data["role"] != "SimpleUsers": @@ -261,31 +262,6 @@ class AccountFormMail(forms.Form, DynamicForm): user.has_perm("admin.add_domain")) self.mb.save(creator=user) - def update_mailbox(self, user, account): - newaddress = None - if self.cleaned_data["email"] != self.mb.full_address: - newaddress = self.cleaned_data["email"] - elif (account.group == "SimpleUsers" and - account.username != self.mb.full_address): - newaddress = account.username - if newaddress is not None: - local_part, domname = split_mailbox(newaddress) - try: - domain = Domain.objects.get(name=domname) - except Domain.DoesNotExist: - raise NotFound(_("Domain does not exist")) - if not user.can_access(domain): - raise PermDeniedException - 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() - if newaddress: - self.mb.rename(local_part, domain) - events.raiseEvent("MailboxModified", self.mb) - def _update_aliases(self, user, account): """Update mailbox aliases.""" aliases = [] @@ -334,7 +310,9 @@ class AccountFormMail(forms.Form, DynamicForm): if not hasattr(self, "mb") or self.mb is None: self.create_mailbox(user, account) else: - self.update_mailbox(user, account) + self.cleaned_data["use_domain_quota"] = ( + self.cleaned_data["quota_act"]) + self.mb.update_from_dict(user, self.cleaned_data) events.raiseEvent( 'SaveExtraFormFields', 'mailform', self.mb, self.cleaned_data ) diff --git a/modoboa/admin/models/mailbox.py b/modoboa/admin/models/mailbox.py index 3c214691f..a061042e1 100644 --- a/modoboa/admin/models/mailbox.py +++ b/modoboa/admin/models/mailbox.py @@ -16,7 +16,8 @@ from .base import AdminObject from .domain import Domain from modoboa.core.models import User from modoboa.lib import events, parameters -from modoboa.lib.exceptions import BadRequest, Conflict, InternalError +from modoboa.lib import exceptions as lib_exceptions +from modoboa.lib.email_utils import split_mailbox from modoboa.lib.sysutils import exec_cmd @@ -144,7 +145,7 @@ class Mailbox(AdminObject): "doveadm user %s -f home" % self.full_address, **options ) if code: - raise InternalError( + raise lib_exceptions.InternalError( _("Failed to retrieve mailbox location (%s)" % output)) self.__mail_home = output.strip() return self.__mail_home @@ -233,14 +234,14 @@ class Mailbox(AdminObject): else: self.quota = 0 elif int(value) > self.domain.quota and not override_rules: - raise BadRequest( + raise lib_exceptions.BadRequest( _("Quota is greater than the allowed domain's limit (%dM)" % self.domain.quota) ) else: self.quota = value if not self.quota and self.domain.quota and not override_rules: - raise BadRequest(_("A quota is required")) + raise lib_exceptions.BadRequest(_("A quota is required")) def get_quota(self): """Get quota limit. @@ -279,6 +280,33 @@ class Mailbox(AdminObject): grant_access_to_object(admin, self) grant_access_to_object(admin, self.user) + def update_from_dict(self, user, values): + """Update mailbox from a dictionary.""" + newaddress = None + if values["email"] != self.full_address: + newaddress = values["email"] + elif (self.user.group == "SimpleUsers" and + self.user.username != self.full_address): + newaddress = self.user.username + if newaddress is not None: + local_part, domname = split_mailbox(newaddress) + try: + domain = Domain.objects.get(name=domname) + except Domain.DoesNotExist: + raise lib_exceptions.NotFound(_("Domain does not exist")) + if not user.can_access(domain): + raise lib_exceptions.PermDeniedException + if "use_domain_quota" in values: + self.use_domain_quota = values["use_domain_quota"] + override_rules = True \ + if not self.quota or user.has_perm("admin.add_domain") \ + else False + self.set_quota(values["quota"], override_rules) + if newaddress: + self.rename(local_part, domain) + self.save() + events.raiseEvent("MailboxModified", self) + def save(self, *args, **kwargs): """Custom save. @@ -290,7 +318,7 @@ class Mailbox(AdminObject): if self.pk: qset = qset.exclude(pk=self.pk) if qset.exists(): - raise Conflict( + raise lib_exceptions.Conflict( _("Mailbox {} already exists").format(self)) if self.quota_value is None: self.quota_value, created = Quota.objects.get_or_create( diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index 2b79f2e5e..ed66ffb24 100644 --- a/modoboa/admin/serializers.py +++ b/modoboa/admin/serializers.py @@ -107,6 +107,21 @@ class CreateAccountSerializer(AccountSerializer): }) return data + def _create_mailbox(self, creator, account, data): + """Create a new Mailbox instance.""" + full_address = data.pop("full_address") + address, domain_name = email_utils.split_mailbox(full_address) + domain = get_object_or_404( + admin_models.Domain, name=domain_name) + mb = admin_models.Mailbox( + user=account, address=address, domain=domain, **data) + mb.set_quota( + data.get("quota"), creator.has_perm("admin.add_domain") + ) + mb.save(creator=creator) + account.email = full_address + return mb + def create(self, validated_data): """Create appropriate objects.""" creator = self.context["request"].user @@ -116,16 +131,24 @@ class CreateAccountSerializer(AccountSerializer): user.set_password(validated_data["password"]) user.save(creator=creator) if mailbox_data: - 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) - 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 + self._create_mailbox(creator, user, mailbox_data) user.role = role return user + + def update(self, instance, validated_data): + """Update account and associated objects.""" + mailbox_data = validated_data.pop("mailbox") + password = validated_data.pop("password") + for key, value in validated_data.items(): + setattr(instance, key, value) + instance.set_password(password) + instance.save() + if mailbox_data: + creator = self.context["request"].user + if instance.mailbox: + # FIXME: compat, to remove. + mailbox_data["email"] = mailbox_data["full_address"] + instance.mailbox.update_from_dict(creator, mailbox_data) + else: + self._create_mailbox(creator, instance, mailbox_data) + return instance diff --git a/modoboa/admin/static/admin/js/admin.js b/modoboa/admin/static/admin/js/admin.js index b35e5caa8..0e91c09e2 100644 --- a/modoboa/admin/static/admin/js/admin.js +++ b/modoboa/admin/static/admin/js/admin.js @@ -6,6 +6,11 @@ * @classdesc This class contains utility methods used by the admin * application. */ + +/*global + $ Listing +*/ + var Admin = function(options) { Listing.call(this, options); }; diff --git a/modoboa/admin/tests/account.py b/modoboa/admin/tests/account.py index 83a682f6e..1ea5457e8 100644 --- a/modoboa/admin/tests/account.py +++ b/modoboa/admin/tests/account.py @@ -44,11 +44,12 @@ class AccountTestCase(ModoTestCase): alias__internal=True).exists() ) - values["username"] = "pouet@test.com" + values.update({ + "username": "pouet@test.com", "email": "pouet@test.com"}) self.ajax_post( reverse("admin:account_change", args=[account.id]), values ) - mb = models.Mailbox.objects.get(pk=mb.id) + mb = models.Mailbox.objects.get(pk=mb.pk) self.assertEqual(mb.full_address, "pouet@test.com") self.assertEqual(mb.quota_value.username, "pouet@test.com") # Check if self alias has been updated diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index 27adabf96..358d2c972 100644 --- a/modoboa/admin/tests/api.py +++ b/modoboa/admin/tests/api.py @@ -124,6 +124,8 @@ class AccountAPITestCase(ModoAPITestCase): user = core_models.User.objects.filter(pk=account["pk"]).first() self.assertIsNot(user, None) self.assertIsNot(user.mailbox, None) + domadmin = core_models.User.objects.get(username="admin@test.com") + self.assertTrue(domadmin.can_access(user)) data = copy.deepcopy(self.ACCOUNT_DATA) data["username"] = "fromapi_ééé@test.com" @@ -193,13 +195,30 @@ class AccountAPITestCase(ModoAPITestCase): def test_update_account(self): """Try to update an account.""" - pass + account = core_models.User.objects.get(username="user@test.com") + url = reverse("external_api:account-detail", args=[account.pk]) + data = { + "username": "fromapi@test.com", + "role": account.role, + "password": "Toto1234", + "mailbox": { + "full_address": "fromapi@test.com", + "quota": account.mailbox.quota + } + } + response = self.client.put(url, data, format="json") + self.assertEqual(response.status_code, 200) + account.refresh_from_db() + self.assertEqual(account.email, account.mailbox.full_address) def test_delete_account(self): """Try to delete an account.""" account = core_models.User.objects.get(username="user@test.com") + domadmin = core_models.User.objects.get(username="admin@test.com") + self.assertTrue(domadmin.can_access(account)) 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()) + self.assertFalse(domadmin.can_access(account)) From f2d5dd229ff1ce12ff0c3c8d68b3da8fa06a64f6 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 2 Mar 2016 16:15:09 +0100 Subject: [PATCH 04/10] Increase coverage. see #832 --- modoboa/admin/models/mailbox.py | 5 ++--- modoboa/admin/serializers.py | 2 +- modoboa/admin/tests/account.py | 3 +-- modoboa/admin/tests/api.py | 16 ++++++++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/modoboa/admin/models/mailbox.py b/modoboa/admin/models/mailbox.py index a061042e1..1cd620197 100644 --- a/modoboa/admin/models/mailbox.py +++ b/modoboa/admin/models/mailbox.py @@ -290,9 +290,8 @@ class Mailbox(AdminObject): newaddress = self.user.username if newaddress is not None: local_part, domname = split_mailbox(newaddress) - try: - domain = Domain.objects.get(name=domname) - except Domain.DoesNotExist: + domain = Domain.objects.filter(name=domname).first() + if domain is None: raise lib_exceptions.NotFound(_("Domain does not exist")) if not user.can_access(domain): raise lib_exceptions.PermDeniedException diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index ed66ffb24..5acfe858e 100644 --- a/modoboa/admin/serializers.py +++ b/modoboa/admin/serializers.py @@ -146,7 +146,7 @@ class CreateAccountSerializer(AccountSerializer): if mailbox_data: creator = self.context["request"].user if instance.mailbox: - # FIXME: compat, to remove. + # FIXME: compat, to remove ASAP. mailbox_data["email"] = mailbox_data["full_address"] instance.mailbox.update_from_dict(creator, mailbox_data) else: diff --git a/modoboa/admin/tests/account.py b/modoboa/admin/tests/account.py index 1ea5457e8..6a7a2e1ba 100644 --- a/modoboa/admin/tests/account.py +++ b/modoboa/admin/tests/account.py @@ -44,8 +44,7 @@ class AccountTestCase(ModoTestCase): alias__internal=True).exists() ) - values.update({ - "username": "pouet@test.com", "email": "pouet@test.com"}) + values.update({"username": "pouet@test.com"}) self.ajax_post( reverse("admin:account_change", args=[account.id]), values ) diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index 358d2c972..2724072c2 100644 --- a/modoboa/admin/tests/api.py +++ b/modoboa/admin/tests/api.py @@ -211,6 +211,22 @@ class AccountAPITestCase(ModoAPITestCase): account.refresh_from_db() self.assertEqual(account.email, account.mailbox.full_address) + def test_update_account_wrong_address(self): + """Try to update an account.""" + account = core_models.User.objects.get(username="user@test.com") + url = reverse("external_api:account-detail", args=[account.pk]) + data = { + "username": "fromapi@test3.com", + "role": account.role, + "password": "Toto1234", + "mailbox": { + "full_address": "fromapi@test3.com", + "quota": account.mailbox.quota + } + } + response = self.client.put(url, data, format="json") + self.assertEqual(response.status_code, 404) + def test_delete_account(self): """Try to delete an account.""" account = core_models.User.objects.get(username="user@test.com") From 62c16d1293be7d3ba03979855ccf78b3ba5b3d79 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 2 Mar 2016 16:46:47 +0100 Subject: [PATCH 05/10] Remove useless handler. see #832 --- modoboa/admin/handlers.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modoboa/admin/handlers.py b/modoboa/admin/handlers.py index 422177987..04e3b2175 100644 --- a/modoboa/admin/handlers.py +++ b/modoboa/admin/handlers.py @@ -29,12 +29,6 @@ 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.""" From 882c137d60a8eb0ec2d684f6fa53b65008b32704 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 2 Mar 2016 17:15:47 +0100 Subject: [PATCH 06/10] Add unit tests. see #832 --- modoboa/admin/models/mailbox.py | 4 ++-- modoboa/admin/serializers.py | 3 +++ modoboa/admin/tests/api.py | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/modoboa/admin/models/mailbox.py b/modoboa/admin/models/mailbox.py index 1cd620197..c201bc7b9 100644 --- a/modoboa/admin/models/mailbox.py +++ b/modoboa/admin/models/mailbox.py @@ -235,8 +235,8 @@ class Mailbox(AdminObject): self.quota = 0 elif int(value) > self.domain.quota and not override_rules: raise lib_exceptions.BadRequest( - _("Quota is greater than the allowed domain's limit (%dM)" - % self.domain.quota) + _("Quota is greater than the allowed domain's limit (%dM)") + % self.domain.quota ) else: self.quota = value diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index 5acfe858e..730fe9d31 100644 --- a/modoboa/admin/serializers.py +++ b/modoboa/admin/serializers.py @@ -113,6 +113,9 @@ class CreateAccountSerializer(AccountSerializer): address, domain_name = email_utils.split_mailbox(full_address) domain = get_object_or_404( admin_models.Domain, name=domain_name) + if not creator.can_access(domain): + raise serializers.ValidationError({ + "domain": _("Permission denied.")}) mb = admin_models.Mailbox( user=account, address=address, domain=domain, **data) mb.set_quota( diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index 2724072c2..a060d96a6 100644 --- a/modoboa/admin/tests/api.py +++ b/modoboa/admin/tests/api.py @@ -183,6 +183,24 @@ class AccountAPITestCase(ModoAPITestCase): errors = json.loads(response.content) self.assertIn("password", errors) + def test_create_account_as_domadmin(self): + """As DomainAdmin, try to create a new account.""" + self.client.credentials( + HTTP_AUTHORIZATION='Token ' + self.da_token.key) + data = copy.deepcopy(self.ACCOUNT_DATA) + data["mailbox"]["quota"] = 20 + url = reverse("external_api:account-list") + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 400) + data["username"] = "fromapi@test2.com" + data["mailbox"].update({"full_address": "fromapi@test2.com", + "quota": 10}) + 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("domain", errors) + def test_create_account_bad_master_user(self): """Try to create a new account.""" data = copy.deepcopy(self.ACCOUNT_DATA) From 5f9bc8a4e283b460797d10b6ab32bc7d310d3132 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 7 Mar 2016 09:13:39 +0100 Subject: [PATCH 07/10] Refactor. see #832 --- modoboa/admin/forms/account.py | 20 +++++++++++--------- modoboa/admin/forms/domain.py | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py index 22729bf5f..10aa6ee22 100644 --- a/modoboa/admin/forms/account.py +++ b/modoboa/admin/forms/account.py @@ -232,7 +232,13 @@ class AccountFormMail(forms.Form, DynamicForm): def clean_email(self): """Ensure lower case emails""" - return self.cleaned_data["email"].lower() + email = self.cleaned_data["email"].lower() + self.locpart, domname = split_mailbox(email) + try: + self.domain = Domain.objects.get(name=domname) + except Domain.DoesNotExist: + raise forms.ValidationError(_("Domain does not exist")) + return email def clean(self): """Custom fields validation. @@ -248,16 +254,12 @@ class AccountFormMail(forms.Form, DynamicForm): def create_mailbox(self, user, account): """Create a mailbox associated to :kw:`account`.""" - locpart, domname = split_mailbox(self.cleaned_data["email"]) - try: - domain = Domain.objects.get(name=domname) - except Domain.DoesNotExist: - raise NotFound(_("Domain does not exist")) - if not user.can_access(domain): + if not user.can_access(self.domain): raise PermDeniedException events.raiseEvent("CanCreate", user, "mailboxes") - self.mb = Mailbox(address=locpart, domain=domain, user=account, - use_domain_quota=self.cleaned_data["quota_act"]) + self.mb = Mailbox( + address=self.locpart, domain=self.domain, user=account, + use_domain_quota=self.cleaned_data["quota_act"]) self.mb.set_quota(self.cleaned_data["quota"], user.has_perm("admin.add_domain")) self.mb.save(creator=user) diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py index 9bd579cd2..aa11113a6 100644 --- a/modoboa/admin/forms/domain.py +++ b/modoboa/admin/forms/domain.py @@ -16,7 +16,7 @@ from modoboa.lib.web_utils import render_to_json_response from ..lib import check_if_domain_exists from ..models import ( - Domain, DomainAlias, Mailbox, Alias, Quota + Domain, DomainAlias, Mailbox, Alias ) DOMAIN_TYPES = [ From 0ff132014acb088d15617cbca0141724f47e934b Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 7 Mar 2016 09:20:33 +0100 Subject: [PATCH 08/10] Do not check domain if not needed. see #832 --- modoboa/admin/forms/account.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py index 10aa6ee22..d360cb2d0 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, NotFound +from modoboa.lib.exceptions import PermDeniedException from modoboa.lib import fields as lib_fields from modoboa.lib.form_utils import ( DynamicForm, TabForms, WizardForm, WizardStep @@ -234,6 +234,8 @@ class AccountFormMail(forms.Form, DynamicForm): """Ensure lower case emails""" email = self.cleaned_data["email"].lower() self.locpart, domname = split_mailbox(email) + if not domname: + return email try: self.domain = Domain.objects.get(name=domname) except Domain.DoesNotExist: From a92a3724dae1ee7aa30095c0514ee806a729bb97 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Tue, 8 Mar 2016 18:48:46 +0100 Subject: [PATCH 09/10] Add /exists/ service. see #832 --- modoboa/admin/api.py | 18 +++++++++++++++++- modoboa/admin/serializers.py | 10 ++++++++-- modoboa/admin/tests/api.py | 16 ++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/modoboa/admin/api.py b/modoboa/admin/api.py index d36184980..bfe452cf7 100644 --- a/modoboa/admin/api.py +++ b/modoboa/admin/api.py @@ -3,7 +3,10 @@ from django.contrib.contenttypes.models import ContentType from rest_framework import viewsets +from rest_framework.decorators import list_route +from rest_framework.exceptions import ParseError from rest_framework.permissions import DjangoModelPermissions +from rest_framework.response import Response from modoboa.core import models as core_models @@ -35,7 +38,7 @@ class AccountViewSet(viewsets.ModelViewSet): """Return a serializer.""" if self.request.method == "GET": return serializers.AccountSerializer - return serializers.CreateAccountSerializer + return serializers.WritableAccountSerializer def get_queryset(self): """Filter queryset based on current user.""" @@ -48,3 +51,16 @@ class AccountViewSet(viewsets.ModelViewSet): def perform_destroy(self, instance): """Add custom args to delete call.""" instance.delete(self.request.user) + + @list_route() + def exists(self, request): + """Check if account exists.""" + email = request.GET.get("email") + if not email: + raise ParseError("email not provided") + if not core_models.User.objects.filter(email=email).exists(): + data = {"exists": False} + else: + data = {"exists": True} + serializer = serializers.AccountExistsSerializer(data) + return Response(serializer.data) diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py index 730fe9d31..473395656 100644 --- a/modoboa/admin/serializers.py +++ b/modoboa/admin/serializers.py @@ -65,7 +65,13 @@ class AccountSerializer(serializers.ModelSerializer): return account.group -class CreateAccountSerializer(AccountSerializer): +class AccountExistsSerializer(serializers.Serializer): + """Simple serializer used with existence checks.""" + + exists = serializers.BooleanField() + + +class WritableAccountSerializer(AccountSerializer): """Serializer to create account.""" class Meta(AccountSerializer.Meta): @@ -74,7 +80,7 @@ class CreateAccountSerializer(AccountSerializer): def __init__(self, *args, **kwargs): """Adapt fields to current user.""" - super(CreateAccountSerializer, self).__init__(*args, **kwargs) + super(WritableAccountSerializer, self).__init__(*args, **kwargs) user = self.context["request"].user self.fields["role"] = serializers.ChoiceField( choices=permissions.get_account_roles(user)) diff --git a/modoboa/admin/tests/api.py b/modoboa/admin/tests/api.py index a060d96a6..7c610f774 100644 --- a/modoboa/admin/tests/api.py +++ b/modoboa/admin/tests/api.py @@ -256,3 +256,19 @@ class AccountAPITestCase(ModoAPITestCase): self.assertFalse( core_models.User.objects.filter(pk=account.pk).exists()) self.assertFalse(domadmin.can_access(account)) + + def test_account_exists(self): + """Validate /exists/ service.""" + url = reverse("external_api:account-exists") + response = self.client.get( + "{}?email={}".format(url, "user@test.com")) + self.assertEqual(response.status_code, 200) + content = json.loads(response.content) + self.assertTrue(content["exists"]) + response = self.client.get( + "{}?email={}".format(url, "pipo@test.com")) + self.assertEqual(response.status_code, 200) + content = json.loads(response.content) + self.assertFalse(content["exists"]) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) From 358c4fe1a4ef21f558719d98253eb75b126f391b Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 9 Mar 2016 10:47:20 +0100 Subject: [PATCH 10/10] Force export order. see #832 --- modoboa/admin/models/alias.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modoboa/admin/models/alias.py b/modoboa/admin/models/alias.py index 7964b1dee..ac446b8c8 100644 --- a/modoboa/admin/models/alias.py +++ b/modoboa/admin/models/alias.py @@ -147,7 +147,10 @@ class Alias(AdminObject): @property def recipients(self): """Return the recipient list.""" - return self.aliasrecipient_set.values_list("address", flat=True) + return ( + self.aliasrecipient_set.order_by("address") + .values_list("address", flat=True) + ) @property def recipients_count(self):