From fed2eddf072d7902f759561ed5d36d2033f1a8ac Mon Sep 17 00:00:00 2001
From: AlanCoding <arominge@redhat.com>
Date: Mon, 10 Jul 2017 09:12:02 -0400
Subject: [PATCH] move circular group association validation to view

---
 awx/api/views.py                            | 10 ++++++++++
 awx/main/access.py                          |  8 --------
 awx/main/tests/functional/api/test_group.py | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 8 deletions(-)
 create mode 100644 awx/main/tests/functional/api/test_group.py

diff --git a/awx/api/views.py b/awx/api/views.py
index 3554315174..ed03cde6c6 100644
--- a/awx/api/views.py
+++ b/awx/api/views.py
@@ -2182,6 +2182,16 @@ class GroupChildrenList(ControlledByScmMixin, EnforceParentRelationshipMixin, Su
         parent.delete()
         return Response(status=status.HTTP_204_NO_CONTENT)
 
+    def is_valid_relation(self, parent, sub, created=False):
+        # Prevent any cyclical group associations.
+        parent_pks = set(parent.all_parents.values_list('pk', flat=True))
+        parent_pks.add(parent.pk)
+        child_pks = set(sub.all_children.values_list('pk', flat=True))
+        child_pks.add(sub.pk)
+        if parent_pks & child_pks:
+            return {'error': _('Cyclical Group association.')}
+        return None
+
 
 class GroupPotentialChildrenList(SubListAPIView):
 
diff --git a/awx/main/access.py b/awx/main/access.py
index 417421e5f4..d941a2bbf6 100644
--- a/awx/main/access.py
+++ b/awx/main/access.py
@@ -730,14 +730,6 @@ class GroupAccess(BaseAccess):
         # Prevent assignments between different inventories.
         if obj.inventory != sub_obj.inventory:
             raise ParseError(_('Cannot associate two items from different inventories.'))
-        # Prevent group from being assigned as its own (grand)child.
-        if type(obj) == type(sub_obj):
-            parent_pks = set(obj.all_parents.values_list('pk', flat=True))
-            parent_pks.add(obj.pk)
-            child_pks = set(sub_obj.all_children.values_list('pk', flat=True))
-            child_pks.add(sub_obj.pk)
-            if parent_pks & child_pks:
-                return False
         return True
 
     def can_delete(self, obj):
diff --git a/awx/main/tests/functional/api/test_group.py b/awx/main/tests/functional/api/test_group.py
new file mode 100644
index 0000000000..1498cfa2be
--- /dev/null
+++ b/awx/main/tests/functional/api/test_group.py
@@ -0,0 +1,15 @@
+import pytest
+
+from awx.api.versioning import reverse
+
+from awx.main.models import Group
+
+
+@pytest.mark.django_db
+def test_cyclical_association_prohibited(post, inventory, admin_user):
+    parent = Group.objects.create(inventory=inventory, name='parent_group')
+    child = parent.children.create(inventory=inventory, name='child_group')
+    # Attempt to make parent a child of the child
+    url = reverse('api:group_children_list', kwargs={'pk': child.id})
+    response = post(url, dict(id=parent.id), admin_user, expect=400)
+    assert 'cyclical' in response.data['error'].lower()