From bdcb30d33301565065ad4d0af91faac0a6a0099a Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Mon, 24 Jun 2019 14:51:15 +0200 Subject: [PATCH 1/6] Reject host names that are already in use as cnames. --- mreg/api/v1/tests/tests.py | 15 +++++++++++++++ mreg/api/v1/views.py | 17 ++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/mreg/api/v1/tests/tests.py b/mreg/api/v1/tests/tests.py index 0ecc88bd..6385fe0c 100644 --- a/mreg/api/v1/tests/tests.py +++ b/mreg/api/v1/tests/tests.py @@ -439,6 +439,14 @@ def test_hosts_post_409_conflict_name(self): """"Posting a new host with a name already in use should return 409""" self.assert_post_and_409('/hosts/', self.post_data_name) + def test_hosts_post_409_conflict_cname(self): + """"Posting a new host with a name already in use as cname should return 409""" + cname_data = {'name': 'host3.example.org', "host": self.host_one.id} + response = self.assert_post_and_201('/cnames/', cname_data) + conflicting_host_data = {'name': 'host3.example.org', "ipaddress": '127.0.0.3', + 'contact': 'hostmaster@example.org'} + self.assert_post_and_409('/hosts/', conflicting_host_data) + def test_hosts_patch_204_no_content(self): """Patching an existing and valid entry should return 204 and Location""" response = self.assert_patch_and_204('/hosts/%s' % self.host_one.name, self.patch_data) @@ -468,6 +476,13 @@ def test_hosts_patch_409_conflict_name(self): """Patching an entry with a name that already exists should return 409""" self.assert_patch_and_409('/hosts/%s' % self.host_one.name, {'name': self.host_two.name}) + def test_hosts_patch_409_conflict_cname(self): + """"Patching an entry host with a name already in use as cname should return 409""" + cname_data = {'name': 'host3.example.org', "host": self.host_one.id} + response = self.assert_post_and_201('/cnames/', cname_data) + conflicting_host_data = {'name': 'host3.example.org'} + self.assert_patch_and_409('/hosts/%s' % self.host_one.name, conflicting_host_data) + class APIHostsTestCaseAsAdminuser(APIHostsTestCase): """Same tests as in APIHostsTestCase, only test as admin and not super""" diff --git a/mreg/api/v1/views.py b/mreg/api/v1/views.py index 668db42d..1715536f 100644 --- a/mreg/api/v1/views.py +++ b/mreg/api/v1/views.py @@ -43,6 +43,15 @@ from .zonefile import ZoneFile +def cname_conflict(cname): + try: + conflicting_cname = Cname.objects.get(name=cname) + # No exception, means such Cname exists + return True + except Cname.DoesNotExist: + return False + + # These filtersets are used for applying generic filtering to all objects. class CnameFilterSet(ModelFilterSet): class Meta: @@ -309,7 +318,10 @@ def post(self, request, *args, **kwargs): if self.queryset.filter(name=request.data["name"]).exists(): content = {'ERROR': 'name already in use'} return Response(content, status=status.HTTP_409_CONFLICT) - + elif cname_conflict(request.data["name"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) + hostdata = request.data.copy() if 'ipaddress' in hostdata: @@ -358,6 +370,9 @@ def patch(self, request, *args, **kwargs): if self.get_queryset().filter(name=request.data["name"]).exists(): content = {'ERROR': 'name already in use'} return Response(content, status=status.HTTP_409_CONFLICT) + elif cname_conflict(request.data["name"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) return super().patch(request, *args, **kwargs) From 56fa5f4f0cbc7fc3832bb136fdd236cde312365c Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Mon, 24 Jun 2019 15:23:39 +0200 Subject: [PATCH 2/6] Reject NAPTRs if the name is taken by cname. --- mreg/api/v1/tests/tests.py | 25 +++++++++++++++++++++++-- mreg/api/v1/views.py | 16 ++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/mreg/api/v1/tests/tests.py b/mreg/api/v1/tests/tests.py index 6385fe0c..2f00ce7e 100644 --- a/mreg/api/v1/tests/tests.py +++ b/mreg/api/v1/tests/tests.py @@ -442,7 +442,7 @@ def test_hosts_post_409_conflict_name(self): def test_hosts_post_409_conflict_cname(self): """"Posting a new host with a name already in use as cname should return 409""" cname_data = {'name': 'host3.example.org', "host": self.host_one.id} - response = self.assert_post_and_201('/cnames/', cname_data) + self.assert_post_and_201('/cnames/', cname_data) conflicting_host_data = {'name': 'host3.example.org', "ipaddress": '127.0.0.3', 'contact': 'hostmaster@example.org'} self.assert_post_and_409('/hosts/', conflicting_host_data) @@ -479,7 +479,7 @@ def test_hosts_patch_409_conflict_name(self): def test_hosts_patch_409_conflict_cname(self): """"Patching an entry host with a name already in use as cname should return 409""" cname_data = {'name': 'host3.example.org', "host": self.host_one.id} - response = self.assert_post_and_201('/cnames/', cname_data) + self.assert_post_and_201('/cnames/', cname_data) conflicting_host_data = {'name': 'host3.example.org'} self.assert_patch_and_409('/hosts/%s' % self.host_one.name, conflicting_host_data) @@ -693,6 +693,27 @@ def test_naptr_zone_autoupdate_delete(self): self.zone.refresh_from_db() self.assertTrue(self.zone.updated) + def test_naptr_post_409_conflict_cname(self): + cname_data = {'name': 'replacement.example.org', "host": self.host.id} + self.assert_post_and_201('/cnames/', cname_data) + conflicting_naptr_data = {'host': self.host.id, + 'preference': 10, + 'order': 20, + 'flag': 'a', + 'service': 'SERVICE', + 'regex': r'1(.*@example.org)', + 'replacement': 'replacement.example.org' + } + self.assert_post_and_409('/naptrs/', conflicting_naptr_data) + + def test_naptr_patch_409_conflict_cname(self): + cname_data = {'name': 'replacement1.example.org', "host": self.host.id} + self.assert_post_and_201('/cnames/', cname_data) + self.test_naptr_post() + naptrs = self.assert_get("/naptrs/").json()['results'] + conflicting_naptr_data = {'replacement': 'replacement1.example.org'} + self.assert_patch_and_409("/naptrs/{}".format(naptrs[0]['id']), conflicting_naptr_data) + class APIPtrOverrideTestcase(MregAPITestCase): """Test PtrOverride records.""" diff --git a/mreg/api/v1/views.py b/mreg/api/v1/views.py index 1715536f..2504c601 100644 --- a/mreg/api/v1/views.py +++ b/mreg/api/v1/views.py @@ -458,6 +458,14 @@ def get_queryset(self): qs = super().get_queryset() return NaptrFilterSet(data=self.request.GET, queryset=qs).filter() + def post(self, request, *args, **kwargs): + if "replacement" in request.data: + if cname_conflict(request.data["replacement"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) + + return super().post(request, *args, **kwargs) + class NaptrDetail(HostPermissionsUpdateDestroy, MregRetrieveUpdateDestroyAPIView): @@ -475,6 +483,14 @@ class NaptrDetail(HostPermissionsUpdateDestroy, queryset = Naptr.objects.all() serializer_class = NaptrSerializer + def patch(self, request, *args, **kwargs): + if "replacement" in request.data: + if cname_conflict(request.data["replacement"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) + + return super().patch(request, *args, **kwargs) + class NameServerList(HostPermissionsListCreateAPIView): """ From cc242a039aa9b8cd7407c47d7952afe79a88f726 Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Tue, 25 Jun 2019 10:59:09 +0200 Subject: [PATCH 3/6] cname conflict checks for SRV records --- mreg/api/v1/views.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/mreg/api/v1/views.py b/mreg/api/v1/views.py index 2504c601..83629482 100644 --- a/mreg/api/v1/views.py +++ b/mreg/api/v1/views.py @@ -45,7 +45,7 @@ def cname_conflict(cname): try: - conflicting_cname = Cname.objects.get(name=cname) + Cname.objects.get(name=cname) # No exception, means such Cname exists return True except Cname.DoesNotExist: @@ -606,6 +606,14 @@ def get_queryset(self): qs = super().get_queryset() return SrvFilterSet(data=self.request.GET, queryset=qs).filter() + def post(self, request, *args, **kwargs): + if "name" in request.data: + if cname_conflict(request.data["name"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) + + return super().post(request, *args, **kwargs) + class SrvDetail(HostPermissionsUpdateDestroy, MregRetrieveUpdateDestroyAPIView): @@ -622,6 +630,14 @@ class SrvDetail(HostPermissionsUpdateDestroy, queryset = Srv.objects.all() serializer_class = SrvSerializer + def patch(self, request, *args, **kwargs): + if "name" in request.data: + if cname_conflict(request.data["name"]): + content = {'ERROR': 'name already in use as cname'} + return Response(content, status=status.HTTP_409_CONFLICT) + + return super().patch(request, *args, **kwargs) + def _overlap_check(range, exclude=None): try: From b4374551bd5aa7f2033d9ffb9f8b6a570f2afbe1 Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Tue, 25 Jun 2019 11:48:16 +0200 Subject: [PATCH 4/6] Quotes consistency adjustments. --- mreg/api/v1/tests/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mreg/api/v1/tests/tests.py b/mreg/api/v1/tests/tests.py index 2f00ce7e..b21f7cc8 100644 --- a/mreg/api/v1/tests/tests.py +++ b/mreg/api/v1/tests/tests.py @@ -441,9 +441,9 @@ def test_hosts_post_409_conflict_name(self): def test_hosts_post_409_conflict_cname(self): """"Posting a new host with a name already in use as cname should return 409""" - cname_data = {'name': 'host3.example.org', "host": self.host_one.id} + cname_data = {'name': 'host3.example.org', 'host': self.host_one.id} self.assert_post_and_201('/cnames/', cname_data) - conflicting_host_data = {'name': 'host3.example.org', "ipaddress": '127.0.0.3', + conflicting_host_data = {'name': 'host3.example.org', 'ipaddress': '127.0.0.3', 'contact': 'hostmaster@example.org'} self.assert_post_and_409('/hosts/', conflicting_host_data) @@ -478,7 +478,7 @@ def test_hosts_patch_409_conflict_name(self): def test_hosts_patch_409_conflict_cname(self): """"Patching an entry host with a name already in use as cname should return 409""" - cname_data = {'name': 'host3.example.org', "host": self.host_one.id} + cname_data = {'name': 'host3.example.org', 'host': self.host_one.id} self.assert_post_and_201('/cnames/', cname_data) conflicting_host_data = {'name': 'host3.example.org'} self.assert_patch_and_409('/hosts/%s' % self.host_one.name, conflicting_host_data) From 7f9f61fbb97b890c7618803fcd898b06a3f961fc Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Tue, 25 Jun 2019 11:53:29 +0200 Subject: [PATCH 5/6] Revert "cname conflict checks for SRV records" This reverts commit cc242a039aa9b8cd7407c47d7952afe79a88f726. --- mreg/api/v1/views.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/mreg/api/v1/views.py b/mreg/api/v1/views.py index 83629482..2504c601 100644 --- a/mreg/api/v1/views.py +++ b/mreg/api/v1/views.py @@ -45,7 +45,7 @@ def cname_conflict(cname): try: - Cname.objects.get(name=cname) + conflicting_cname = Cname.objects.get(name=cname) # No exception, means such Cname exists return True except Cname.DoesNotExist: @@ -606,14 +606,6 @@ def get_queryset(self): qs = super().get_queryset() return SrvFilterSet(data=self.request.GET, queryset=qs).filter() - def post(self, request, *args, **kwargs): - if "name" in request.data: - if cname_conflict(request.data["name"]): - content = {'ERROR': 'name already in use as cname'} - return Response(content, status=status.HTTP_409_CONFLICT) - - return super().post(request, *args, **kwargs) - class SrvDetail(HostPermissionsUpdateDestroy, MregRetrieveUpdateDestroyAPIView): @@ -630,14 +622,6 @@ class SrvDetail(HostPermissionsUpdateDestroy, queryset = Srv.objects.all() serializer_class = SrvSerializer - def patch(self, request, *args, **kwargs): - if "name" in request.data: - if cname_conflict(request.data["name"]): - content = {'ERROR': 'name already in use as cname'} - return Response(content, status=status.HTTP_409_CONFLICT) - - return super().patch(request, *args, **kwargs) - def _overlap_check(range, exclude=None): try: From c985a52109d2ec8a2a6967fc0d911d2be1162cf8 Mon Sep 17 00:00:00 2001 From: Dmytro Karpenko Date: Tue, 25 Jun 2019 11:55:33 +0200 Subject: [PATCH 6/6] conflicting_cname is not needed. --- mreg/api/v1/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mreg/api/v1/views.py b/mreg/api/v1/views.py index 2504c601..a0300463 100644 --- a/mreg/api/v1/views.py +++ b/mreg/api/v1/views.py @@ -45,7 +45,7 @@ def cname_conflict(cname): try: - conflicting_cname = Cname.objects.get(name=cname) + Cname.objects.get(name=cname) # No exception, means such Cname exists return True except Cname.DoesNotExist: