Merge pull request #3039 from modoboa/improved-csv-import

Improved alias management

frontend/src/components/identities/AccountAliasForm.vue

@@ -15,7 +15,7 @@
       />
   </validation-provider>
   <v-chip
-    v-for="(alias, index) in aliases"
+    v-for="(alias, index) in form.aliases"
     :key="index"
     class="mr-2 mt-2"
     close
@@ -37,19 +37,25 @@ export default {
   },
   data () {
     return {
-      currentAlias: '',
-      aliases: []
+      form: {
+        mailbox: {},
+        aliases: []
+      },
+      currentAlias: ''
     }
   },
   methods: {
     reset () {
       this.currentAlias = ''
-      this.aliases = []
+      this.form.aliases = []
+    },
+    update () {
+      this.$emit('input', this.form)
     },
     async addAlias () {
       try {
-        await accounts.validate({ aliases: [this.currentAlias] })
-        this.aliases.push(this.currentAlias)
+        await accounts.validate({ aliases: [this.currentAlias], mailbox: this.form.mailbox })
+        this.form.aliases.push(this.currentAlias)
         this.$refs.aliasField.reset()
       } catch (error) {
         let errorMsg = null
@@ -62,15 +68,20 @@ export default {
       }
     },
     removeAlias (index) {
-      this.aliases.splice(index, 1)
+      this.form.aliases.splice(index, 1)
       this.update()
-    },
-    update () {
-      this.$emit('input', this.aliases)
     }
   },
   mounted () {
-    this.aliases = [...this.value]
+    this.form = { ...this.value }
+  },
+  watch: {
+    value: {
+      handler (newValue) {
+        this.form = { ...newValue }
+      },
+      deep: true
+    }
   }
 }
 </script>

frontend/src/components/identities/AccountCreationForm.vue

@@ -20,7 +20,7 @@
     <account-mailbox-form :ref="`form_${step}`" v-model="account" />
   </template>
   <template v-slot:form.aliases="{ step }">
-    <account-alias-form v-if="step >= 4" :ref="`form_${step}`" v-model="account.aliases" />
+    <account-alias-form v-if="step >= 4" :ref="`form_${step}`" v-model="account" />
   </template>
   <template v-slot:item.random_password="{ item }">
     <template v-if="item.value">

frontend/src/components/identities/AccountForm.vue

@@ -103,6 +103,37 @@
         <account-mailbox-form ref="mailboxForm" v-model="editedAccount" />
       </v-expansion-panel-content>
     </v-expansion-panel>
+    <v-expansion-panel v-if="usernameIsEmail">
+      <v-expansion-panel-header v-slot="{ open }">
+        <v-row no-gutters>
+          <v-col cols="4">
+            <translate>Alias</translate>
+          </v-col>
+          <v-col
+            cols="8"
+            class="text--secondary"
+            >
+            <v-fade-transition leave-absolute>
+              <span v-if="open"></span>
+              <v-row
+                v-else
+                no-gutters
+                style="width: 100%"
+                >
+                <template v-if="account.aliases">
+                  <v-col cols="6">
+                    <translate class="mr-2">Aliases: </translate> <translate>Number of associated alias: </translate> {{ account.aliases.length }}
+                  </v-col>
+                </template>
+              </v-row>
+            </v-fade-transition>
+          </v-col>
+        </v-row>
+      </v-expansion-panel-header>
+      <v-expansion-panel-content>
+        <account-alias-form ref="aliasForm" v-model="editedAccount" />
+      </v-expansion-panel-content>
+    </v-expansion-panel>
     <v-expansion-panel
       v-if="limitsConfig.params && limitsConfig.params.enable_admin_limits && account.role !== 'SimpleUsers' && account.role !== 'SuperAdmins'"
       >
@@ -135,6 +166,7 @@ import { bus } from '@/main'
 import accounts from '@/api/accounts'
 import AccountGeneralForm from './AccountGeneralForm'
 import AccountMailboxForm from './AccountMailboxForm'
+import AccountAliasForm from './AccountAliasForm'
 import AccountRoleForm from './AccountRoleForm'
 import ResourcesForm from '@/components/tools/ResourcesForm'
 import parameters from '@/api/parameters'
@@ -143,6 +175,7 @@ export default {
   components: {
     AccountGeneralForm,
     AccountMailboxForm,
+    AccountAliasForm,
     AccountRoleForm,
     ResourcesForm
   },

frontend/src/components/identities/AliasCreationForm.vue

@@ -101,7 +101,14 @@ export default {
       if (data.recipients.length === 0) {
         delete data.recipients
       }
-      return aliases.validate(data)
+      const validation = aliases.validate(data)
+      validation.catch(error => {
+        if (error.response.status === 409 && error.response.data.id !== undefined) {
+          bus.$emit('notification', { msg: this.$gettext('Alias already exists, redirecting to edit page'), type: 'warning' })
+          this.$router.push({ name: 'AliasEdit', params: { id: error.response.data.id } })
+        }
+      })
+      return validation
     },
     submit () {
       aliases.create(this.alias).then(resp => {

frontend/src/components/tools/ImportForm.vue

@@ -29,13 +29,19 @@
           dense
           />
       </validation-provider>
-      <label class="m-label"><translate>Separator</translate></label>
-      <v-text-field
-        v-model="form.sepchar"
-        :error-messages="errors"
-        outlined
-        dense
-        />
+      <validation-provider
+        v-slot="{ errors }"
+        name="name"
+        rules="required"
+        >
+        <label class="m-label"><translate>Separator</translate></label>
+        <v-text-field
+          v-model="form.sepchar"
+          :error-messages="errors"
+          outlined
+          dense
+          />
+      </validation-provider>
       <v-switch
         v-model="form.continue_if_exists"
         :label="'Continue on error'|translate"

frontend/src/mixins/importExport.js

@@ -18,6 +18,11 @@ export const importExportMixin = {
         } else {
           bus.$emit('notification', { msg: resp.data.message, type: 'error' })
         }
+      }).catch(error => {
+        bus.$emit('notification', {
+          msg: this.$gettext('CSV seems to be badly formatted'),
+          type: 'error'
+        })
       })
     }
   }

frontend/src/views/identities/Identities.vue

@@ -62,12 +62,12 @@
       :title="'Import identities'|translate"
       @beforeSubmit="prepareData"
       @submit="importIdentities"
-      @close="showImportForm = false"
+      @close="closeImportForm"
       >
       <template v-slot:help>
         <ul>
           <li><em>account; loginname; password; first name; last name; enabled; group; address; quota; [, domain, ...]</em></li>
-          <li><em>alias; address; enabled; recipient; recipient; ...</em></li>
+          <li><em>alias; address; enabled; recipient; [more recipients; ...]</em></li>
         </ul>
       </template>
       <template v-slot:extraFields="{ form }">
@@ -124,10 +124,16 @@ export default {
       })
     },
     prepareData (data) {
-      data.append('crypt_passwords', this.form.crypt_passwords)
+      if (this.form !== undefined) {
+        data.append('crypt_passwords', this.form.crypt_passwords)
+      }
     },
     importIdentities (data) {
       this.importContent(identities, data)
+    },
+    closeImportForm () {
+      this.showImportForm = false
+      this.updateIdentities()
     }
   }
 }

modoboa/admin/api/v2/serializers.py

@@ -446,7 +446,7 @@ class WritableAccountSerializer(v1_serializers.WritableAccountSerializer):
         aliases = []
         for alias in value:
             localpart, domain = models.validate_alias_address(
-                alias, self.context["request"].user)
+                alias, self.context["request"].user, ignore_existing=True)
             aliases.append({"localpart": localpart, "domain": domain})
         return aliases
 
@@ -543,12 +543,12 @@ class WritableAccountSerializer(v1_serializers.WritableAccountSerializer):
         self.set_permissions(user, domains)
         if aliases:
             for alias in aliases:
-                models.Alias.objects.create(
-                    creator=creator,
-                    domain=alias["domain"],
+                models.Alias.objects.modify_or_create(
                     address="{}@{}".format(alias["localpart"], alias["domain"]),
-                    recipients=[user.username]
-                )
+                    recipients=[user.username],
+                    creator=creator,
+                    domain=alias["domain"]
+                    )
         return user
 
     def update(self, instance, validated_data):
@@ -556,6 +556,7 @@ class WritableAccountSerializer(v1_serializers.WritableAccountSerializer):
         mailbox_data = validated_data.pop("mailbox", None)
         password = validated_data.pop("password", None)
         domains = validated_data.pop("domains", [])
+        aliases = validated_data.pop("aliases", None)
         for key, value in validated_data.items():
             setattr(instance, key, value)
         if password:
@@ -571,6 +572,28 @@ class WritableAccountSerializer(v1_serializers.WritableAccountSerializer):
                 mailbox_data["full_address"] = validated_data["username"]
                 instance.email = validated_data["username"]
                 self._create_mailbox(creator, instance, mailbox_data)
+        if aliases is not None and "username" in validated_data:
+            # We create a list to keep track of alias we need to delete
+            alias_recipients = instance.mailbox.alias_addresses
+            for alias in aliases:
+                address = "{}@{}".format(alias["localpart"], alias["domain"])
+                models.Alias.objects.modify_or_create(
+                    address=address,
+                    recipients=[validated_data["username"]],
+                    creator=creator,
+                    domain=alias["domain"]
+                )
+                try:
+                    alias_recipients.remove(address)
+                except ValueError:
+                    continue
+            for alias_address in alias_recipients:
+                alias = models.Alias.objects.filter(
+                    address=alias_address, internal=False)
+                if alias.exists():
+                    alias.first().remove_recipient_or_delete(
+                        validated_data["username"]
+                    )
         instance.save()
         resources = validated_data.get("resources")
         if resources:

modoboa/admin/api/v2/tests.py

@@ -253,7 +253,10 @@ class AccountViewSetTestCase(ModoAPITestCase):
             "password": "Toto12345",
             "mailbox": {
                 "quota": 10
-            }
+            },
+            "aliases": [
+                "aliasupdate1@test.com"
+            ]
         }
         resp = self.client.patch(url, data, format="json")
         self.assertEqual(resp.status_code, 200)
@@ -304,6 +307,80 @@ class AccountViewSetTestCase(ModoAPITestCase):
         self.assertEqual(account.mailbox.message_limit, None)
         self.assertEqual(account.mailbox.is_send_only, False)
 
+    def test_update_aliases(self):
+        account = core_models.User.objects.get(username="user@test.com")
+        url = reverse("v2:account-detail", args=[account.pk])
+        data = {
+            "username": "user@test.com",
+            "role": "SimpleUsers",
+            "password": "Toto12345",
+            "mailbox": {
+                "quota": 10
+            },
+            "aliases": [
+                "aliasupdate1@test.com"
+            ]
+        }
+        resp = self.client.patch(url, data, format="json")
+        self.assertEqual(resp.status_code, 200)
+        self.assertTrue(
+            models.Alias.objects.filter(address="aliasupdate1@test.com").exists())
+
+        # Create an alias for another user
+        url = reverse("v2:account-list")
+        data = {
+            "username": "toto@test.com",
+            "role": "SimpleUsers",
+            "mailbox": {
+                "use_domain_quota": True
+            },
+            "password": "Toto12345",
+            "language": "fr",
+            "aliases": ["totoalias@test.com"]
+        }
+        resp = self.client.post(url, data, format="json")
+        self.assertEqual(resp.status_code, 201)
+
+        # Try updating existing account with this newly created alias
+        url = reverse("v2:account-detail", args=[account.pk])
+        data = {
+            "username": "user@test.com",
+            "role": "SimpleUsers",
+            "password": "Toto12345",
+            "mailbox": {
+                "quota": 10
+            },
+            "aliases": [
+                "totoalias@test.com",
+                "aliasupdate1@test.com",
+            ]
+        }
+        resp = self.client.patch(url, data, format="json")
+        self.assertEqual(resp.status_code, 200)
+        alias = models.Alias.objects.filter(address="totoalias@test.com")
+        self.assertTrue(alias.exists())
+        alias_recipients = list(alias.first().recipients)
+        self.assertIn("toto@test.com", alias_recipients)
+        self.assertIn("user@test.com", alias_recipients)
+
+        # Try deleting the aliases
+        data = {
+            "username": "user@test.com",
+            "role": "SimpleUsers",
+            "mailbox": {
+                "quota": 10
+            },
+            "aliases": []
+        }
+        resp = self.client.patch(url, data, format="json")
+        self.assertEqual(resp.status_code, 200)
+        self.assertFalse(
+            models.Alias.objects.filter(address="aliasupdate1@test.com").exists())
+        alias_recipients = list(models.Alias.objects.filter(
+            address="totoalias@test.com").first().recipients
+        )
+        self.assertEqual(alias_recipients, ["toto@test.com"])
+
     def test_update_admin(self):
         account = core_models.User.objects.get(username="admin")
         url = reverse("v2:account-detail", args=[account.pk])
@@ -458,7 +535,10 @@ class AliasViewSetTestCase(ModoAPITestCase):
 
         data = {"address": "alias@test.com"}
         resp = self.client.post(url, data, format="json")
-        self.assertEqual(resp.status_code, 400)
+        self.assertEqual(resp.status_code, 409)
+
+        al_id = models.Alias.objects.get(address="alias@test.com").pk
+        self.assertEqual(resp.json()["id"], al_id)
 
         data = {"address": "alias2@test.com"}
         resp = self.client.post(url, data, format="json")

modoboa/admin/api/v2/viewsets.py

@@ -18,6 +18,7 @@ from modoboa.lib import pagination
 from modoboa.lib import renderers as lib_renderers
 from modoboa.lib import viewsets as lib_viewsets
 from modoboa.lib.throttle import GetThrottleViewsetMixin
+from modoboa.lib.exceptions import AliasExists
 
 from ... import lib
 from ... import models
@@ -163,7 +164,6 @@ class AccountViewSet(v1_viewsets.AccountViewSet):
     filter_backends = (filters.SearchFilter, dj_filters.DjangoFilterBackend)
     filterset_class = AccountFilterSet
 
-
     def get_serializer_class(self):
         if self.action in ["create", "validate", "update", "partial_update"]:
             return serializers.WritableAccountSerializer
@@ -264,7 +264,13 @@ class AliasViewSet(v1_viewsets.AliasViewSet):
             data=request.data,
             context=self.get_serializer_context(),
             partial=True)
-        serializer.is_valid(raise_exception=True)
+        try:
+            serializer.is_valid(raise_exception=True)
+        except AliasExists as e:
+            return response.Response(
+                data={"id": e.alias_id, "status": _("This alias already exists")},
+                status=409
+                )
         return response.Response(status=204)
 
     @action(methods=["get"], detail=False)

modoboa/admin/lib.py

@@ -166,15 +166,15 @@ def _import_alias(user, row, **kwargs):
 
 
 def import_alias(user, row, formopts):
-    _import_alias(user, row, expected_elements=4)
+    _import_alias(user, row, expected_elements=4, formopts=formopts)
 
 
 def import_forward(user, row, formopts):
-    _import_alias(user, row, expected_elements=4)
+    _import_alias(user, row, expected_elements=4, formopts=formopts)
 
 
 def import_dlist(user, row, formopts):
-    _import_alias(user, row)
+    _import_alias(user, row, formopts=formopts)
 
 
 def get_dns_resolver():

modoboa/admin/management/commands/subcommands/_import.py

@@ -7,6 +7,7 @@ import os
 import progressbar
 from chardet.universaldetector import UniversalDetector
 
+from django.core.exceptions import ValidationError
 from django.core.management.base import BaseCommand, CommandError
 from django.db import transaction
 from django.utils.translation import gettext as _
@@ -72,8 +73,8 @@ class ImportCommand(BaseCommand):
                     if options["continue_if_exists"]:
                         continue
                     raise CommandError(
-                        "Object already exists: {}".format(
-                            options["sepchar"].join(row[:2])))
+                        "Object already exists at line {}: {}".format(
+                            i+1, options["sepchar"].join(row[:2])))
                 i += 1
                 pbar.update(i)
 

modoboa/admin/models/__init__.py

@@ -1,7 +1,10 @@
 """Admin models."""
 
 from .alarm import Alarm
-from .alias import Alias, AliasRecipient, validate_alias_address
+from .alias import (
+    Alias,
+    AliasRecipient,
+    validate_alias_address)
 from .base import AdminObject
 from .domain import Domain
 from .domain_alias import DomainAlias

modoboa/admin/models/alias.py

@@ -16,7 +16,7 @@ from reversion import revisions as reversion
 from modoboa.core import signals as core_signals
 from modoboa.lib.email_utils import split_mailbox
 from modoboa.lib.exceptions import (
-    BadRequest, Conflict, NotFound, ModoboaException
+    BadRequest, Conflict, NotFound, ModoboaException, AliasExists
 )
 from .. import signals
 from .base import AdminObject
@@ -24,7 +24,9 @@ from .domain import Domain
 from .mailbox import Mailbox
 
 
-def validate_alias_address(address, creator, internal=False, instance=None):
+def validate_alias_address(
+        address, creator, internal=False, instance=None, ignore_existing=False
+        ):
     """Check if the given alias address can be created by creator."""
     local_part, domain = split_mailbox(address)
     domain = Domain.objects.filter(name=domain).first()
@@ -33,8 +35,13 @@ def validate_alias_address(address, creator, internal=False, instance=None):
     if not creator.can_access(domain):
         raise ValidationError(_("Permission denied."))
     if not instance or instance.address != address:
-        if Alias.objects.filter(address=address, internal=internal).exists():
-            raise ValidationError(_("An alias with this name already exists."))
+        alias = Alias.objects.filter(address=address, internal=internal)
+        condition = (
+            alias.exists()
+            and not ignore_existing
+        )
+        if condition:
+            raise AliasExists(alias.first().pk)
     if instance is None:
         try:
             # Check creator limits
@@ -63,6 +70,20 @@ class AliasManager(models.Manager):
             alias.set_recipients(recipients)
         return alias
 
+    def modify_or_create(self, address, recipients, creator, domain):
+        """Add recipient if the alias already exists or create it."""
+
+        alias = self.get_queryset().filter(address=address, internal=False)
+        if alias.exists():
+            alias.first().add_recipients(recipients)
+            return
+        self.create(
+            creator=creator,
+            domain=domain,
+            address=address,
+            recipients=recipients
+        )
+
 
 class Alias(AdminObject):
     """Mailbox alias."""
@@ -187,6 +208,21 @@ class Alias(AdminObject):
                     kwargs["r_mailbox"] = rcpt
             AliasRecipient(**kwargs).save()
 
+    def remove_recipient_or_delete(self, recipient_to_delete: str):
+        """
+        Delete the selected recipient from this alias
+        or delete the whole alias if only one is left.
+        """
+        alias_recipients = list(self.recipients)
+        if recipient_to_delete in alias_recipients:
+            if len(alias_recipients) == 1:
+                # Only recipient, we delete the Alias
+                self.delete()
+            else:
+                alias_recipients.remove(recipient_to_delete)
+                self.set_recipients(alias_recipients)
+                self.save()
+
     def set_recipients(self, address_list):
         """Set recipients for this alias."""
 
@@ -209,12 +245,33 @@ class Alias(AdminObject):
         """Return the number of recipients of this alias."""
         return self.aliasrecipient_set.count()
 
-    def from_csv(self, user, row, expected_elements=5):
+    def from_csv(self,
+                 user,
+                 row,
+                 expected_elements=5,
+                 formopts=False,
+                 **kwargs):
         """Create a new alias from a CSV file entry."""
         if len(row) < expected_elements:
             raise BadRequest(_("Invalid line: {}").format(row))
         self.address = row[1].strip().lower()
-        local_part, self.domain = validate_alias_address(self.address, user)
+        try:
+            local_part, self.domain = validate_alias_address(self.address, user)
+        except AliasExists as e:
+            # If continue_if_exists is true, we simply update the alias
+            # Else we throw the conflict issue
+            continue_if_exists = False
+            if formopts:
+                continue_if_exists = formopts.get("continue_if_exists", False)
+            if continue_if_exists:
+                alias = Alias.objects.filter(address=self.address,
+                                             internal=False).first()
+                alias.add_recipients([raddress.strip() for raddress in row[3:]])
+                alias.save()
+                return
+            else:
+                raise e
+
         self.enabled = (row[2].strip().lower() in ["true", "1", "yes", "y"])
         self.save()
         self.set_recipients([raddress.strip() for raddress in row[3:]])

modoboa/admin/tests/test_data/import_aliases.csv

@@ -0,0 +1,2 @@
+
+alias; alias1@test.com; True; truc@test.com

modoboa/admin/tests/test_import_.py

@@ -418,3 +418,26 @@ dómain; dómªin2.com; 1000; 100; True
         self.assertEqual(dom.default_mailbox_quota, 200)
         self.assertTrue(dom.enabled)
         self.assertTrue(admin.is_owner(dom))
+
+    def test_import_aliases(self):
+        f = ContentFile("""
+account; user1@test.com; toto; User; One; True; SimpleUsers; user1@test.com; 0
+account; Truc@test.com; toto; René; Truc; True; DomainAdmins; truc@test.com; 5; test.com
+alias; alias1@test.com; True; user1@test.com
+""", name="identities.csv")  # NOQA:E501
+        resp = self.client.post(
+            reverse("admin:identity_import"),
+            {"sourcefile": f, "crypt_password": True}
+        )
+        self.assertEqual(resp.status_code, 200)
+
+        test_file = os.path.join(
+            os.path.dirname(__file__),
+            "test_data/import_aliases.csv"
+        )
+        with self.assertRaises(CommandError) as cm:
+            call_command("modo", "import", test_file)
+            ex_message = cm.exception.messages
+            self.assertTrue(ex_message.startswith("Object already exists"))
+
+        call_command("modo", "import", "--continue-if-exists", test_file)

modoboa/core/commands/templates/urls.py.tpl

@@ -1,4 +1,4 @@
-from django.conf.urls import include, path
+from django.urls import include, path
 
 urlpatterns = [
     path(r'', include('modoboa.urls')),

modoboa/core/extensions.py

@@ -1,7 +1,7 @@
 """Extension management."""
 
 from django.conf import settings
-from django.conf.urls import include
+from django.urls import include
 from django.urls import re_path
 from django.utils.encoding import smart_str
 

modoboa/lib/exceptions.py

@@ -52,6 +52,16 @@ class Conflict(ModoboaException):
     http_code = 409
 
 
+class AliasExists(Conflict):
+    """
+    Use this exception to indicate that the requested alias already exists
+    and that it should be updated instead of created.
+    """
+
+    def __init__(self, alias_id):
+        self.alias_id = alias_id
+
+
 class PermDeniedException(ModoboaException):
     """
     Use this exception when a user tries to do something he is not