From 41d1ce1f66b2000fa287abcbd7902678a1de07d4 Mon Sep 17 00:00:00 2001 From: Spitap Date: Sun, 23 Apr 2023 17:13:49 +0200 Subject: [PATCH] Delete imap api v1, better pdfcredentials settings validation --- modoboa/imap_migration/api/v1/__init__.py | 0 modoboa/imap_migration/api/v1/serializers.py | 93 -------------------- modoboa/imap_migration/api/v1/urls.py | 12 --- modoboa/imap_migration/api/v1/viewsets.py | 41 --------- modoboa/imap_migration/api/v2/serializers.py | 80 ++++++++++++++++- modoboa/imap_migration/api/v2/viewsets.py | 38 ++++++-- modoboa/imap_migration/tests.py | 4 +- modoboa/pdfcredentials/api/v2/serializers.py | 28 +++--- modoboa/pdfcredentials/api/v2/tests.py | 15 ++++ modoboa/urls_api_v1.py | 1 - 10 files changed, 143 insertions(+), 169 deletions(-) delete mode 100644 modoboa/imap_migration/api/v1/__init__.py delete mode 100644 modoboa/imap_migration/api/v1/serializers.py delete mode 100644 modoboa/imap_migration/api/v1/urls.py delete mode 100644 modoboa/imap_migration/api/v1/viewsets.py diff --git a/modoboa/imap_migration/api/v1/__init__.py b/modoboa/imap_migration/api/v1/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/modoboa/imap_migration/api/v1/serializers.py b/modoboa/imap_migration/api/v1/serializers.py deleted file mode 100644 index 2a95b9802..000000000 --- a/modoboa/imap_migration/api/v1/serializers.py +++ /dev/null @@ -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 diff --git a/modoboa/imap_migration/api/v1/urls.py b/modoboa/imap_migration/api/v1/urls.py deleted file mode 100644 index c1b429749..000000000 --- a/modoboa/imap_migration/api/v1/urls.py +++ /dev/null @@ -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 diff --git a/modoboa/imap_migration/api/v1/viewsets.py b/modoboa/imap_migration/api/v1/viewsets.py deleted file mode 100644 index 74c8d8481..000000000 --- a/modoboa/imap_migration/api/v1/viewsets.py +++ /dev/null @@ -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 diff --git a/modoboa/imap_migration/api/v2/serializers.py b/modoboa/imap_migration/api/v2/serializers.py index de66c0e62..f86616fdb 100644 --- a/modoboa/imap_migration/api/v2/serializers.py +++ b/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 + diff --git a/modoboa/imap_migration/api/v2/viewsets.py b/modoboa/imap_migration/api/v2/viewsets.py index a1fa254b4..0f52bcd05 100644 --- a/modoboa/imap_migration/api/v2/viewsets.py +++ b/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 diff --git a/modoboa/imap_migration/tests.py b/modoboa/imap_migration/tests.py index 57128d4ad..d87480174 100644 --- a/modoboa/imap_migration/tests.py +++ b/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, diff --git a/modoboa/pdfcredentials/api/v2/serializers.py b/modoboa/pdfcredentials/api/v2/serializers.py index d8f559278..0a0bab668 100644 --- a/modoboa/pdfcredentials/api/v2/serializers.py +++ b/modoboa/pdfcredentials/api/v2/serializers.py @@ -42,18 +42,24 @@ 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.") - ) - if not os.access(value, os.W_OK): - raise serializers.ValidationError( - _("Directory is not writable") - ) - return value + enabled_pdfcredentials = data.get("enabled_pdfcredentials", False) + condition = (enabled_pdfcredentials or + param_tools.get_global_parameter("enabled_pdfcredentials") + ) + 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): diff --git a/modoboa/pdfcredentials/api/v2/tests.py b/modoboa/pdfcredentials/api/v2/tests.py index 68491098c..2c3d79817 100644 --- a/modoboa/pdfcredentials/api/v2/tests.py +++ b/modoboa/pdfcredentials/api/v2/tests.py @@ -164,3 +164,18 @@ 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"]) + # 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) + # Test that we can disable it even if the directory is not right. + self.set_global_parameter("enabled_pdfcredentials", False) + data = {"enabled_pdfcredentials": True} + resp = self.client.put(url, data, format="json") + self.assertEqual(resp.status_code, 200) diff --git a/modoboa/urls_api_v1.py b/modoboa/urls_api_v1.py index 2077cec23..7697ba96b 100644 --- a/modoboa/urls_api_v1.py +++ b/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")) ]