Merge pull request #2963 from modoboa/Cleaning-and-pdf-qol

Deleted imap api v1, better pdfcredentials settings validation

modoboa/imap_migration/api/v1/serializers.py

@@ -1,93 +0,0 @@
-"""API serializers."""
-
-from rest_framework import serializers
-
-from modoboa.admin import models as admin_models
-
-from modoboa.imap_migration import models
-
-
-class EmailProviderDomainSerializer(serializers.ModelSerializer):
-    """Serializer class for EmailProviderDomain."""
-
-    id = serializers.IntegerField(required=False)
-
-    class Meta:
-        extra_kwargs = {
-            "name": {
-                "validators": []
-            }
-        }
-        fields = ("id", "name", "new_domain")
-        model = models.EmailProviderDomain
-
-
-class EmailProviderSerializer(serializers.ModelSerializer):
-    """Serializer class for EmailProvider."""
-
-    domains = EmailProviderDomainSerializer(many=True, required=False)
-
-    class Meta:
-        fields = "__all__"
-        model = models.EmailProvider
-
-    def create(self, validated_data):
-        """Create provider and domains."""
-        domains = validated_data.pop("domains", None)
-        validated_data.pop("id", None)
-        provider = models.EmailProvider.objects.create(**validated_data)
-        if domains:
-            to_create = []
-            for domain in domains:
-                to_create.append(models.EmailProviderDomain(
-                    provider=provider, **domain))
-            models.EmailProviderDomain.objects.bulk_create(to_create)
-        return provider
-
-    def update(self, instance, validated_data):
-        """Update provider and domains."""
-        domains = validated_data.pop("domains", [])
-        domain_ids = [domain["id"] for domain in domains if "id" in domain]
-        for key, value in validated_data.items():
-            setattr(instance, key, value)
-        instance.save()
-        for domain in instance.domains.all():
-            if domain.id not in domain_ids:
-                domain.delete()
-            else:
-                for updated_domain in domains:
-                    if updated_domain.get("id") != domain.id:
-                        continue
-                    domain.name = updated_domain["name"]
-                    domain.new_domain = updated_domain.get("new_domain")
-                    domain.save()
-                    domains.remove(updated_domain)
-                    break
-        to_create = []
-        for new_domain in domains:
-            to_create.append(models.EmailProviderDomain(
-                name=new_domain["name"],
-                new_domain=new_domain.get("new_domain"),
-                provider=instance
-            ))
-        models.EmailProviderDomain.objects.bulk_create(to_create)
-        return instance
-
-
-class MailboxMigrationSerializer(serializers.ModelSerializer):
-    """Simple mailbox serializer."""
-
-    class Meta:
-        fields = ("id", "address", "user")
-        model = admin_models.Mailbox
-
-
-class MigrationSerializer(serializers.ModelSerializer):
-    """Serializer class for Migration."""
-
-    mailbox = MailboxMigrationSerializer()
-
-    class Meta:
-        depth = 1
-        fields = ("id", "provider", "mailbox", "username")
-        model = models.Migration

modoboa/imap_migration/api/v1/urls.py

@@ -1,12 +0,0 @@
-"""API urls."""
-
-from rest_framework import routers
-
-from . import viewsets
-
-
-router = routers.SimpleRouter()
-router.register(r"email-providers", viewsets.EmailProviderViewSet)
-router.register(r"migrations", viewsets.MigrationViewSet)
-
-urlpatterns = router.urls

modoboa/imap_migration/api/v1/viewsets.py

@@ -1,41 +0,0 @@
-"""API viewsets."""
-
-from rest_framework import filters, mixins, permissions, viewsets
-
-from modoboa.lib.throttle import GetThrottleViewsetMixin
-
-from ... import models
-from . import serializers
-
-
-class EmailProviderViewSet(GetThrottleViewsetMixin, viewsets.ModelViewSet):
-    """ViewSet class for EmailProvider."""
-
-    filter_backends = (filters.SearchFilter, )
-    permission_classes = (
-        permissions.IsAuthenticated,
-        permissions.DjangoModelPermissions
-    )
-    queryset = models.EmailProvider.objects.all().prefetch_related("domains")
-    search_fields = ("name", )
-    serializer_class = serializers.EmailProviderSerializer
-
-
-class MigrationViewSet(GetThrottleViewsetMixin,
-                       mixins.ListModelMixin,
-                       mixins.RetrieveModelMixin,
-                       mixins.DestroyModelMixin,
-                       viewsets.GenericViewSet):
-    """ViewSet class for Migration."""
-
-    filter_backends = (filters.SearchFilter, )
-    permission_classes = (
-        permissions.IsAuthenticated,
-        permissions.DjangoModelPermissions
-    )
-    queryset = models.Migration.objects.all().select_related(
-        "provider", "mailbox__domain")
-    search_fields = (
-        "username", "provider__name", "mailbox__address",
-        "mailbox__domain__name")
-    serializer_class = serializers.MigrationSerializer

modoboa/imap_migration/api/v2/serializers.py

@@ -3,11 +3,12 @@
 from django.utils.translation import gettext_lazy as _
 
 from rest_framework import serializers
-from modoboa.admin.models import Mailbox
-from modoboa.imap_migration.api.v1 import serializers as v1_serializers
 
+from modoboa.admin.models import Mailbox
 from modoboa.admin.models.domain import Domain
 
+from modoboa.imap_migration import models
+
 
 class IMAPMigrationSettingsSerializer(serializers.Serializer):
     """A serializer for global parameters."""
@@ -28,6 +29,73 @@ class IMAPMigrationSettingsSerializer(serializers.Serializer):
                                                   default="")
 
 
+class EmailProviderDomainSerializer(serializers.ModelSerializer):
+    """Serializer class for EmailProviderDomain."""
+
+    id = serializers.IntegerField(required=False)
+
+    class Meta:
+        extra_kwargs = {
+            "name": {
+                "validators": []
+            }
+        }
+        fields = ("id", "name", "new_domain")
+        model = models.EmailProviderDomain
+
+
+class EmailProviderSerializer(serializers.ModelSerializer):
+    """Serializer class for EmailProvider."""
+
+    domains = EmailProviderDomainSerializer(many=True, required=False)
+
+    class Meta:
+        fields = "__all__"
+        model = models.EmailProvider
+
+    def create(self, validated_data):
+        """Create provider and domains."""
+        domains = validated_data.pop("domains", None)
+        validated_data.pop("id", None)
+        provider = models.EmailProvider.objects.create(**validated_data)
+        if domains:
+            to_create = []
+            for domain in domains:
+                to_create.append(models.EmailProviderDomain(
+                    provider=provider, **domain))
+            models.EmailProviderDomain.objects.bulk_create(to_create)
+        return provider
+
+    def update(self, instance, validated_data):
+        """Update provider and domains."""
+        domains = validated_data.pop("domains", [])
+        domain_ids = [domain["id"] for domain in domains if "id" in domain]
+        for key, value in validated_data.items():
+            setattr(instance, key, value)
+        instance.save()
+        for domain in instance.domains.all():
+            if domain.id not in domain_ids:
+                domain.delete()
+            else:
+                for updated_domain in domains:
+                    if updated_domain.get("id") != domain.id:
+                        continue
+                    domain.name = updated_domain["name"]
+                    domain.new_domain = updated_domain.get("new_domain")
+                    domain.save()
+                    domains.remove(updated_domain)
+                    break
+        to_create = []
+        for new_domain in domains:
+            to_create.append(models.EmailProviderDomain(
+                name=new_domain["name"],
+                new_domain=new_domain.get("new_domain"),
+                provider=instance
+            ))
+        models.EmailProviderDomain.objects.bulk_create(to_create)
+        return instance
+
+
 class CheckProviderSerializer(serializers.Serializer):
     """A serializer for checking connection to IMAP server."""
 
@@ -61,7 +129,13 @@ class MailboxSerializer(serializers.ModelSerializer):
         model = Mailbox
 
 
-class MigrationSerializer(v1_serializers.MigrationSerializer):
+class MigrationSerializer(serializers.ModelSerializer):
     """Serializer class for Migration."""
 
     mailbox = MailboxSerializer()
+
+    class Meta:
+        depth = 1
+        fields = ("id", "provider", "mailbox", "username")
+        model = models.Migration
+

modoboa/imap_migration/api/v2/viewsets.py

@@ -4,15 +4,26 @@ import imaplib
 import socket
 import ssl
 
-from rest_framework import response
+from rest_framework import response, filters, mixins, permissions, viewsets
 from rest_framework.decorators import action
 
-from modoboa.imap_migration.api.v1 import viewsets as v1_viewsets
+from modoboa.lib.throttle import GetThrottleViewsetMixin
 
+from ... import models
 from . import serializers
 
 
-class EmailProviderViewSet(v1_viewsets.EmailProviderViewSet):
+class EmailProviderViewSet(GetThrottleViewsetMixin, viewsets.ModelViewSet):
+    """ViewSet class for EmailProvider."""
+
+    filter_backends = (filters.SearchFilter, )
+    permission_classes = (
+        permissions.IsAuthenticated,
+        permissions.DjangoModelPermissions
+    )
+    queryset = models.EmailProvider.objects.all().prefetch_related("domains")
+    search_fields = ("name", )
+    serializer_class = serializers.EmailProviderSerializer
 
     @action(methods=["post"], detail=False)
     def check_connection(self, request, **kwargs):
@@ -27,7 +38,7 @@ class EmailProviderViewSet(v1_viewsets.EmailProviderViewSet):
                 imaplib.IMAP4_SSL(address, port)
             else:
                 imaplib.IMAP4(address, port)
-        except (socket.error, imaplib.IMAP4.error, ssl.SSLError) as error:
+        except (socket.error, imaplib.IMAP4.error, ssl.SSLError):
             return response.Response({'status': 'failed'}, 400)
         return response.Response(status=200)
 
@@ -41,6 +52,21 @@ class EmailProviderViewSet(v1_viewsets.EmailProviderViewSet):
         return response.Response(status=200)
 
 
-class MigrationViewSet(v1_viewsets.MigrationViewSet):
-    """ Change the serializer to add the user id. """
+class MigrationViewSet(GetThrottleViewsetMixin,
+                       mixins.ListModelMixin,
+                       mixins.RetrieveModelMixin,
+                       mixins.DestroyModelMixin,
+                       viewsets.GenericViewSet):
+    """ViewSet class for Migration."""
+
+    filter_backends = (filters.SearchFilter, )
+    permission_classes = (
+        permissions.IsAuthenticated,
+        permissions.DjangoModelPermissions
+    )
+    queryset = models.Migration.objects.all().select_related(
+        "provider", "mailbox__domain")
+    search_fields = (
+        "username", "provider__name", "mailbox__address",
+        "mailbox__domain__name")
     serializer_class = serializers.MigrationSerializer

modoboa/imap_migration/tests.py

@@ -134,7 +134,7 @@ class ViewSetTestCase(DataMixin, ModoAPITestCase):
     """ViewSet related tests."""
 
     def test_create_provider(self):
-        url = reverse("api:emailprovider-list")
+        url = reverse("v2:emailprovider-list")
         data = {
             "name": "Google",
             "address": "imap.google.com",
@@ -156,7 +156,7 @@ class ViewSetTestCase(DataMixin, ModoAPITestCase):
         provider_domain = factories.EmailProviderDomainFactory(
             name="outlook.com")
         url = reverse(
-            "api:emailprovider-detail", args=[provider_domain.provider.pk])
+            "v2:emailprovider-detail", args=[provider_domain.provider.pk])
         data = {
             "id": provider_domain.provider.id,
             "name": provider_domain.provider.name,

modoboa/pdfcredentials/api/v2/serializers.py

@@ -42,18 +42,26 @@ class PDFCredentialsSettingsSerializer(serializers.Serializer):
     imap_connection_security = serializers.ChoiceField(
         choices=CONNECTION_SECURITY_MODES, default="starttls")
 
-    def validate_storage_dir(self, value):
+    def validate(self, data):
         """Check that directory exists."""
-        if value:
-            if not os.path.isdir(value):
-                raise serializers.ValidationError(
-                    _("Directory not found.")
+        enabled_pdfcredentials = data.get("enabled_pdfcredentials", None)
+        condition = (enabled_pdfcredentials or
+                     (enabled_pdfcredentials is None and
+                      param_tools.get_global_parameter("enabled_pdfcredentials")
+                     )
                     )
-            if not os.access(value, os.W_OK):
-                raise serializers.ValidationError(
-                    _("Directory is not writable")
-                    )
-        return value
+        if condition:
+            storage_dir = data.get("storage_dir", None)
+            if storage_dir is not None:
+                if not os.path.isdir(storage_dir):
+                    raise serializers.ValidationError(
+                        _("Directory not found.")
+                        )
+                if not os.access(storage_dir, os.W_OK):
+                    raise serializers.ValidationError(
+                        _("Directory is not writable")
+                        )
+        return data
 
 
 class GetAccountCredentialsSerializer(serializers.Serializer):

modoboa/pdfcredentials/api/v2/tests.py

@@ -164,3 +164,25 @@ class PDFCredentialViewTestCase(ModoAPITestCase):
         """Test storage directory creation."""
         self.set_global_parameter("storage_dir", "/nonexistentdir")
         self._create_account("leon@test.com", expected_status=500)
+
+    def test_directory_check_settings(self):
+        """Test validation for unwritable directory."""
+        self.set_global_parameter("storage_dir", "/nonexistentdir")
+        self.set_global_parameter("enabled_pdfcredentials", False)
+        url = reverse("v2:parameter-detail", args=["pdfcredentials"])
+        data = {
+                "webpanel_url": "http://localhost",
+                "smtp_server_address": "mail.localhost",
+                "imap_server_address": "mail.localhost",
+               }
+
+        # Test that we can't activate it without providing a valid storage dir.
+        data["enabled_pdfcredentials"] = True
+        resp = self.client.put(url, data, format="json")
+        self.assertEqual(resp.status_code, 400)
+        self.assertIn(b"Directory not found", resp.content)
+        # Test that we can disable it even if the directory is not right.
+        self.set_global_parameter("enabled_pdfcredentials", True)
+        data["enabled_pdfcredentials"] = False
+        resp = self.client.put(url, data, format="json")
+        self.assertEqual(resp.status_code, 200)

modoboa/urls_api_v1.py

@@ -9,7 +9,6 @@ app_name = "api"
 urlpatterns = [
     path('', include("modoboa.core.api.v1.urls")),
     path('', include("modoboa.admin.api.v1.urls")),
-    path('', include("modoboa.imap_migration.api.v1.urls")),
     path('', include("modoboa.limits.api.v1.urls")),
     path('', include("modoboa.relaydomains.api.v1.urls"))
 ]