From bef009659db45368f1908fabd0bac5d8bd4f7591 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Thu, 15 Jun 2017 12:24:15 -0400 Subject: [PATCH] Add value type validation to sublist (un)attach --- awx/api/generics.py | 16 ++++++++++++- awx/main/tests/unit/api/test_generics.py | 29 ++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/awx/api/generics.py b/awx/api/generics.py index 11304c14e3..2bf09211c9 100644 --- a/awx/api/generics.py +++ b/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): diff --git a/awx/main/tests/unit/api/test_generics.py b/awx/main/tests/unit/api/test_generics.py index 9f3746c043..82647460e0 100644 --- a/awx/main/tests/unit/api/test_generics.py +++ b/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()