Add value type validation to sublist (un)attach

awx/api/generics.py

@@ -461,13 +461,24 @@ class SubListCreateAttachDetachAPIView(SubListCreateAPIView):
         })
         return d
 
+    def attach_validate(self, request):
+        sub_id = request.data.get('id', None)
+        res = None
+        if sub_id and not isinstance(sub_id, int):
+            data = dict(msg=_('"id" field must be an integer.'))
+            res = Response(data, status=status.HTTP_400_BAD_REQUEST)
+        return (sub_id, res)
+
     def attach(self, request, *args, **kwargs):
         created = False
         parent = self.get_parent_object()
         relationship = getattrd(parent, self.relationship)
-        sub_id = request.data.get('id', None)
         data = request.data
 
+        sub_id, res = self.attach_validate(request)
+        if res:
+            return res
+
         # Create the sub object if an ID is not provided.
         if not sub_id:
             response = self.create(request, *args, **kwargs)
@@ -515,6 +526,9 @@ class SubListCreateAttachDetachAPIView(SubListCreateAPIView):
         if not sub_id:
             data = dict(msg=_('"id" is required to disassociate'))
             res = Response(data, status=status.HTTP_400_BAD_REQUEST)
+        elif not isinstance(sub_id, int):
+            data = dict(msg=_('"id" field must be an integer.'))
+            res = Response(data, status=status.HTTP_400_BAD_REQUEST)
         return (sub_id, res)
 
     def unattach_by_id(self, request, sub_id):

awx/main/tests/unit/api/test_generics.py

@@ -58,6 +58,23 @@ def parent_relationship_factory(mocker):
 # TODO: Test create and associate failure (i.e. id doesn't exist, record already exists, permission denied)
 # TODO: Mock and check return (Response)
 class TestSubListCreateAttachDetachAPIView:
+    def test_attach_validate_ok(self, mocker):
+        mock_request = mocker.MagicMock(data=dict(id=1))
+        serializer = SubListCreateAttachDetachAPIView()
+
+        (sub_id, res) = serializer.attach_validate(mock_request)
+
+        assert sub_id == 1
+        assert res is None
+
+    def test_attach_validate_invalid_type(self, mocker):
+        mock_request = mocker.MagicMock(data=dict(id='foobar'))
+        serializer = SubListCreateAttachDetachAPIView()
+
+        (sub_id, res) = serializer.attach_validate(mock_request)
+
+        assert type(res) is Response
+
     def test_attach_create_and_associate(self, mocker, get_object_or_400, parent_relationship_factory, mock_response_new):
         (serializer, mock_parent_relationship) = parent_relationship_factory(SubListCreateAttachDetachAPIView, 'wife')
         create_return_value = mocker.MagicMock(status_code=status.HTTP_201_CREATED)
@@ -91,7 +108,15 @@ class TestSubListCreateAttachDetachAPIView:
 
         assert sub_id == 1
         assert res is None
-    
+
+    def test_unattach_validate_invalid_type(self, mocker):
+        mock_request = mocker.MagicMock(data=dict(id='foobar'))
+        serializer = SubListCreateAttachDetachAPIView()
+
+        (sub_id, res) = serializer.unattach_validate(mock_request)
+
+        assert type(res) is Response
+
     def test_unattach_validate_missing_id(self, mocker):
         mock_request = mocker.MagicMock(data=dict())
         serializer = SubListCreateAttachDetachAPIView()
@@ -100,7 +125,7 @@ class TestSubListCreateAttachDetachAPIView:
 
         assert sub_id is None
         assert type(res) is Response
- 
+
     def test_unattach_by_id_ok(self, mocker, parent_relationship_factory, get_object_or_400):
         (serializer, mock_parent_relationship) = parent_relationship_factory(SubListCreateAttachDetachAPIView, 'wife')
         mock_request = mocker.MagicMock()