From f2438a0e86f8a77a8216abdf16f362b362947d28 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 11 Feb 2026 15:10:32 -0500 Subject: [PATCH] Fix server error from PATCH to inventory source (#16274) Fix server error from PATCH to inventory source, co-authored with Claude opus 4.6 --- awx/api/fields.py | 2 +- .../tests/functional/api/test_inventory.py | 20 ++++++++ awx/main/tests/unit/api/test_fields.py | 49 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 awx/main/tests/unit/api/test_fields.py diff --git a/awx/api/fields.py b/awx/api/fields.py index bf936cb7ba..c53494d9e3 100644 --- a/awx/api/fields.py +++ b/awx/api/fields.py @@ -89,7 +89,7 @@ class DeprecatedCredentialField(serializers.IntegerField): def to_internal_value(self, pk): try: pk = int(pk) - except ValueError: + except (ValueError, TypeError): self.fail('invalid') try: Credential.objects.get(pk=pk) diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index a9a24b4abb..da19d62e66 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -463,6 +463,26 @@ class TestInventorySourceCredential: assert 'Cloud-based inventory sources (such as ec2)' in r.data['credential'][0] assert 'require credentials for the matching cloud service' in r.data['credential'][0] + def test_credential_dict_value_returns_400(self, inventory, admin_user, put): + """Passing a dict for the credential field should return 400, not 500. + + Reproduces a bug where int() raises TypeError on non-scalar types + (dict, list) which was uncaught, resulting in a 500 Internal Server Error. + """ + inv_src = InventorySource.objects.create(name='test-src', inventory=inventory, source='ec2') + r = put( + url=reverse('api:inventory_source_detail', kwargs={'pk': inv_src.pk}), + data={ + 'name': 'test-src', + 'inventory': inventory.pk, + 'source': 'ec2', + 'credential': {'username': 'admin', 'password': 'secret'}, + }, + user=admin_user, + expect=400, + ) + assert r.status_code == 400 + def test_vault_credential_not_allowed(self, project, inventory, vault_credential, admin_user, post): """Vault credentials cannot be associated via the deprecated field""" # TODO: when feature is added, add tests to use the related credentials diff --git a/awx/main/tests/unit/api/test_fields.py b/awx/main/tests/unit/api/test_fields.py new file mode 100644 index 0000000000..d6b6ae49d2 --- /dev/null +++ b/awx/main/tests/unit/api/test_fields.py @@ -0,0 +1,49 @@ +import pytest +from collections import OrderedDict +from unittest import mock + +from rest_framework.exceptions import ValidationError + +from awx.api.fields import DeprecatedCredentialField + + +class TestDeprecatedCredentialField: + """Test that DeprecatedCredentialField handles unexpected input types gracefully.""" + + def test_dict_value_raises_validation_error(self): + """Passing a dict instead of an integer should return a 400 validation error, not a 500 TypeError.""" + field = DeprecatedCredentialField() + with pytest.raises(ValidationError): + field.to_internal_value({"username": "admin", "password": "secret"}) + + def test_ordered_dict_value_raises_validation_error(self): + """Passing an OrderedDict should return a 400 validation error, not a 500 TypeError.""" + field = DeprecatedCredentialField() + with pytest.raises(ValidationError): + field.to_internal_value(OrderedDict([("username", "admin")])) + + def test_list_value_raises_validation_error(self): + """Passing a list should return a 400 validation error, not a 500 TypeError.""" + field = DeprecatedCredentialField() + with pytest.raises(ValidationError): + field.to_internal_value([1, 2, 3]) + + def test_string_value_raises_validation_error(self): + """Passing a non-numeric string should return a 400 validation error.""" + field = DeprecatedCredentialField() + with pytest.raises(ValidationError): + field.to_internal_value("not_a_number") + + @mock.patch('awx.api.fields.Credential.objects') + def test_valid_integer_value_works(self, mock_cred_objects): + """Passing a valid integer PK should work when the credential exists.""" + mock_cred_objects.get.return_value = mock.MagicMock() + field = DeprecatedCredentialField() + assert field.to_internal_value(42) == 42 + + @mock.patch('awx.api.fields.Credential.objects') + def test_valid_string_integer_value_works(self, mock_cred_objects): + """Passing a numeric string PK should work when the credential exists.""" + mock_cred_objects.get.return_value = mock.MagicMock() + field = DeprecatedCredentialField() + assert field.to_internal_value("42") == 42