From b8423a3464202a9d43a297066f8dae51ca819d53 Mon Sep 17 00:00:00 2001 From: "Phil V." Date: Wed, 23 Sep 2026 09:50:11 +0300 Subject: [PATCH 1/4] Validate sub unit types for entity types to prevent recursion --- iaso/api/org_unit_types/serializers.py | 102 ++++++++++++++++--------- 1 file changed, 64 insertions(+), 38 deletions(-) diff --git a/iaso/api/org_unit_types/serializers.py b/iaso/api/org_unit_types/serializers.py index 4befaec5c7..913566a61f 100644 --- a/iaso/api/org_unit_types/serializers.py +++ b/iaso/api/org_unit_types/serializers.py @@ -11,18 +11,36 @@ from ..projects.serializers import ProjectSerializer -def get_parents(type_id): - parents_ids = [] - if type_id is not None: - queryset = OrgUnitType.objects.filter(sub_unit_types__id=type_id) - for parent in queryset.all(): - if parent.id not in parents_ids: - parents_ids.append(parent.id) - great_parents = get_parents(parent.id) - for great_parent_id in great_parents: - if great_parent_id not in parents_ids: - parents_ids.append(great_parent_id) - return parents_ids +def has_cycle_after_adding(node_id, proposed_children, relation_name): + """ + Returns True if adding proposed_children to node_id's relation_name + creates a cycle/loop. + """ + if not node_id: + return False + + proposed_children_ids = [c.id for c in proposed_children] + if node_id in proposed_children_ids: + return True + + visited = set(proposed_children_ids) + queue = list(proposed_children_ids) + + while queue: + curr_id = queue.pop(0) + if curr_id == node_id: + return True + try: + curr_node = OrgUnitType.objects.get(pk=curr_id) + children_field = getattr(curr_node, relation_name) + for child_id in children_field.values_list("id", flat=True): + if child_id not in visited: + visited.add(child_id) + queue.append(child_id) + except OrgUnitType.DoesNotExist: + continue + + return False def validate_reference_forms(data): @@ -109,21 +127,25 @@ def get_sub_unit_types(self, obj: OrgUnitType): ).data def validate(self, data: typing.Mapping): - parents = get_parents(self.context["request"].data.get("id", None)) + org_unit_type_id = self.instance.id if self.instance else self.context["request"].data.get("id") # validate sub org unit type - sub_types_errors = [] - for sub_type in data.get("sub_unit_types", []): - if sub_type.id in parents: - sub_types_errors.append(sub_type.name) - if len(sub_types_errors) > 0: - raise serializers.ValidationError({"sub_unit_type_ids": sub_types_errors}) + if "sub_unit_types" in data: + if has_cycle_after_adding(org_unit_type_id, data["sub_unit_types"], "sub_unit_types"): + raise serializers.ValidationError( + {"sub_unit_type_ids": ["A loop was detected in the sub-unit types hierarchy."]} + ) # validate sub org unit type allowed to be created - create_sub_types_errors = [] - for sub_type in data.get("allow_creating_sub_unit_types", []): - if sub_type.id in parents: - create_sub_types_errors.append(sub_type.name) - if len(create_sub_types_errors) > 0: - raise serializers.ValidationError({"allow_creating_sub_unit_type_ids": create_sub_types_errors}) + if "allow_creating_sub_unit_types" in data: + if has_cycle_after_adding( + org_unit_type_id, data["allow_creating_sub_unit_types"], "allow_creating_sub_unit_types" + ): + raise serializers.ValidationError( + { + "allow_creating_sub_unit_type_ids": [ + "A loop was detected in the allowed sub-unit types creation hierarchy." + ] + } + ) # validate projects (access check) for project in data.get("projects", []): if self.context["request"].user.iaso_profile.account != project.account: @@ -241,21 +263,25 @@ def get_allow_creating_sub_unit_types(self, obj: OrgUnitType): ).data def validate(self, data: typing.Mapping): - parents = get_parents(self.context["request"].data.get("id", None)) + org_unit_type_id = self.instance.id if self.instance else self.context["request"].data.get("id") # validate sub org unit type - sub_types_errors = [] - for sub_type in data.get("sub_unit_types", []): - if sub_type.id in parents: - sub_types_errors.append(sub_type.name) - if len(sub_types_errors) > 0: - raise serializers.ValidationError({"sub_unit_type_ids": sub_types_errors}) + if "sub_unit_types" in data: + if has_cycle_after_adding(org_unit_type_id, data["sub_unit_types"], "sub_unit_types"): + raise serializers.ValidationError( + {"sub_unit_type_ids": ["A loop was detected in the sub-unit types hierarchy."]} + ) # validate sub org unit type allowed to be created - create_sub_types_errors = [] - for sub_type in data.get("allow_creating_sub_unit_types", []): - if sub_type.id in parents: - create_sub_types_errors.append(sub_type.name) - if len(create_sub_types_errors) > 0: - raise serializers.ValidationError({"allow_creating_sub_unit_type_ids": create_sub_types_errors}) + if "allow_creating_sub_unit_types" in data: + if has_cycle_after_adding( + org_unit_type_id, data["allow_creating_sub_unit_types"], "allow_creating_sub_unit_types" + ): + raise serializers.ValidationError( + { + "allow_creating_sub_unit_type_ids": [ + "A loop was detected in the allowed sub-unit types creation hierarchy." + ] + } + ) # validate projects (access check) for project in data.get("projects", []): if self.context["request"].user.iaso_profile.account != project.account: From ad6470bde0b85d8e8f8c4eea1c475493c5d838db Mon Sep 17 00:00:00 2001 From: "Phil V." Date: Wed, 23 Sep 2026 09:53:34 +0300 Subject: [PATCH 2/4] Prevent infinite recursion in hierarchy serializer --- iaso/api/org_unit_types/serializers.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/iaso/api/org_unit_types/serializers.py b/iaso/api/org_unit_types/serializers.py index 913566a61f..fe976ad6fa 100644 --- a/iaso/api/org_unit_types/serializers.py +++ b/iaso/api/org_unit_types/serializers.py @@ -310,8 +310,17 @@ class Meta: def get_sub_unit_types(self, obj): """Recursively serialize sub_unit_types to build complete hierarchy""" + visited = self.context.setdefault("visited_org_unit_types", set()) + if obj.id in visited: + return [] + + child_visited = set(visited) + child_visited.add(obj.id) + child_context = {**self.context, "visited_org_unit_types": child_visited} + sub_types = obj.sub_unit_types.all() - return OrgUnitTypeHierarchySerializer(sub_types, many=True, context=self.context).data + sub_types = [t for t in sub_types if t.id not in child_visited] + return OrgUnitTypeHierarchySerializer(sub_types, many=True, context=child_context).data class OrgUnitTypesDropdownSerializer(serializers.ModelSerializer): From 02933f57ba87284c75edc09f6ed60818aa8a7884 Mon Sep 17 00:00:00 2001 From: "Phil V." Date: Wed, 23 Sep 2026 10:02:54 +0300 Subject: [PATCH 3/4] Add option to prevent dynamic fields recursion --- dynamic_fields/serializer.py | 2 ++ iaso/api/org_unit_types/serializers.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dynamic_fields/serializer.py b/dynamic_fields/serializer.py index 8b87277aae..4e35fa04c4 100644 --- a/dynamic_fields/serializer.py +++ b/dynamic_fields/serializer.py @@ -53,6 +53,8 @@ def to_representation(self, instance): return super().to_representation(instance) def get_fields_value_from_query(self): + if self.context.get("ignore_dynamic_fields", False): + return [] if self.context.get("request", None): if self.string_field: return [ diff --git a/iaso/api/org_unit_types/serializers.py b/iaso/api/org_unit_types/serializers.py index fe976ad6fa..820a9a58ed 100644 --- a/iaso/api/org_unit_types/serializers.py +++ b/iaso/api/org_unit_types/serializers.py @@ -123,7 +123,7 @@ def get_sub_unit_types(self, obj: OrgUnitType): unit_types, fields=["id", "name", "short_name", "depth", "created_at", "updated_at"], many=True, - context=self.context, + context={**self.context, "ignore_dynamic_fields": True}, ).data def validate(self, data: typing.Mapping): @@ -245,7 +245,7 @@ def get_sub_unit_types(self, obj: OrgUnitType): unit_types, fields=["id", "name", "short_name", "depth", "created_at", "updated_at"], many=True, - context=self.context, + context={**self.context, "ignore_dynamic_fields": True}, ).data def get_allow_creating_sub_unit_types(self, obj: OrgUnitType): @@ -259,7 +259,7 @@ def get_allow_creating_sub_unit_types(self, obj: OrgUnitType): unit_types, fields=["id", "name", "short_name", "depth", "created_at", "updated_at"], many=True, - context=self.context, + context={**self.context, "ignore_dynamic_fields": True}, ).data def validate(self, data: typing.Mapping): From fd386ec1c1eb267506bec6389b2f094dd4f92bf4 Mon Sep 17 00:00:00 2001 From: "Phil V." Date: Wed, 23 Sep 2026 10:12:32 +0300 Subject: [PATCH 4/4] Test coverage --- iaso/tests/api/test_org_unit_types_v2.py | 65 ++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/iaso/tests/api/test_org_unit_types_v2.py b/iaso/tests/api/test_org_unit_types_v2.py index de25ff6ab3..da3806145d 100644 --- a/iaso/tests/api/test_org_unit_types_v2.py +++ b/iaso/tests/api/test_org_unit_types_v2.py @@ -776,3 +776,68 @@ def test_num_queries_retrieve(self): response = self.client.get(f"{self.BASE_URL}{self.org_unit_type_1.id}/") self.assertJSONResponse(response, status.HTTP_200_OK) + + def test_prevent_loop_sub_unit_types(self): + """Test that `sub_unit_types` is validated to prevent infinite recursion loops.""" + self.client.force_authenticate(self.jane) + self.org_unit_type_1.sub_unit_types.set([self.org_unit_type_2]) + + response = self.client.put( + f"{self.BASE_URL}{self.org_unit_type_2.id}/", + data={ + "name": self.org_unit_type_2.name, + "short_name": self.org_unit_type_2.short_name, + "project_ids": [self.ead.id], + "sub_unit_type_ids": [self.org_unit_type_1.id], + "allow_creating_sub_unit_type_ids": [], + }, + format="json", + ) + data = self.assertJSONResponse(response, status.HTTP_400_BAD_REQUEST) + self.assertIn("sub_unit_type_ids", data) + + def test_prevent_loop_allow_creating_sub_unit_types(self): + """Test that `allow_creating_sub_unit_types` is validated to prevent infinite recursion loops.""" + self.client.force_authenticate(self.jane) + self.org_unit_type_1.allow_creating_sub_unit_types.set([self.org_unit_type_2]) + + response = self.client.put( + f"{self.BASE_URL}{self.org_unit_type_2.id}/", + data={ + "name": self.org_unit_type_2.name, + "short_name": self.org_unit_type_2.short_name, + "project_ids": [self.ead.id], + "sub_unit_type_ids": [], + "allow_creating_sub_unit_type_ids": [self.org_unit_type_1.id], + }, + format="json", + ) + data = self.assertJSONResponse(response, status.HTTP_400_BAD_REQUEST) + self.assertIn("allow_creating_sub_unit_type_ids", data) + + def test_serialization_resilience_to_existing_loops(self): + """Test that if a cycle exists, querying the api doesn't throw a recursion error.""" + self.client.force_authenticate(self.jane) + self.org_unit_type_1.sub_unit_types.set([self.org_unit_type_2]) + self.org_unit_type_2.sub_unit_types.set([self.org_unit_type_1]) + + response = self.client.get(f"{self.BASE_URL}?fields=id,name,sub_unit_types") + self.assertJSONResponse(response, status.HTTP_200_OK) + + def test_serialization_resilience_to_existing_loops_allow_creating_sub_unit_types(self): + """Test that if a cycle exists on allow_creating_sub_unit_types, querying the api doesn't throw a recursion error.""" + self.client.force_authenticate(self.jane) + self.org_unit_type_1.allow_creating_sub_unit_types.set([self.org_unit_type_2]) + self.org_unit_type_2.allow_creating_sub_unit_types.set([self.org_unit_type_1]) + + response = self.client.get(f"{self.BASE_URL}?fields=id,name,allow_creating_sub_unit_types") + self.assertJSONResponse(response, status.HTTP_200_OK) + + def test_hierarchy_resilience_to_existing_loops(self): + """Test that if a cycle exists, querying the hierarchy endpoint doesn't throw a recursion error.""" + self.client.force_authenticate(self.jane) + self.org_unit_type_1.sub_unit_types.set([self.org_unit_type_2]) + self.org_unit_type_2.sub_unit_types.set([self.org_unit_type_1]) + + response = self.client.get(f"{self.BASE_URL}{self.org_unit_type_1.id}/hierarchy/") + self.assertJSONResponse(response, status.HTTP_200_OK)