fix(authorization): perform_create reused serializer.instance + adapt rest_framework tests

Two viewsets (dojo/organization/api/views.py and dojo/api_v2/views.py
ProductTypeViewSet) used a leftover RBAC-era pattern after
serializer.save():

    member.product_type = Product_Type(**serializer.data)

That re-constructs a *second* Product_Type instance and assigns it as
the owner-member's FK — semantically wrong (the member should point
at the just-saved row) and now broken under legacy because
serializer.data includes authorized_users (a M2M) which Django
forbids in __init__ kwargs:

    TypeError: Direct assignment to the forward side of a many-to-many set
    is prohibited. Use authorized_users.set() instead.

Replace both with a clean Product_Type_Member.objects.create() that
points at serializer.instance.

In unittests/test_rest_framework.py the API tests assert that
user_has_permission was called with self.permission_create /
self.permission_update / self.permission_delete (Permissions enum
members). The actual API calls now pass action strings ("add",
"edit", "delete", ...). Funnel through permission_to_action() in the
five assertion sites + the inline Permissions.X assertion args inside
ImportScanTest / ReimportScanTest / OrganizationTest / ProductTypeTest
so the existing per-class permission_create / permission_update /
permission_delete attribute pattern keeps working.

Brings test_rest_framework from 116 failures to 10 (the rest are
unrelated: notes/files/profile fixture quirks).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: devGregA

dojo/api_v2/views.py

@@ -2091,14 +2091,14 @@ def get_queryset(self):
     # Overwrite perfom_create of CreateModelMixin to add current user as owner
     def perform_create(self, serializer):
         serializer.save()
-        product_type_data = serializer.data
-        product_type_data.pop("authorization_groups")
-        product_type_data.pop("members")
-        member = Product_Type_Member()
-        member.user = self.request.user
-        member.product_type = Product_Type(**product_type_data)
-        member.role = Role.objects.get(is_owner=True)
-        member.save()
+        # Reuse the just-saved instance instead of re-constructing one from
+        # serializer.data; the latter passes authorized_users (a M2M) into
+        # __init__ which raises under the legacy authorization model.
+        Product_Type_Member.objects.create(
+            user=self.request.user,
+            product_type=serializer.instance,
+            role=Role.objects.get(is_owner=True),
+        )
 
     def destroy(self, request, *args, **kwargs):
         instance = self.get_object()

dojo/organization/api/views.py

@@ -50,17 +50,16 @@ def get_queryset(self):
     # Overwrite perfom_create of CreateModelMixin to add current user as owner
     def perform_create(self, serializer):
         serializer.save()
-        product_type_data = serializer.data
-        product_type_data.pop("authorization_groups")
-        product_type_data.pop("members")
-        # Manage custom fields separately with default fields of false
-        product_type_data["critical_product"] = product_type_data.pop("critical_asset", False)
-        product_type_data["key_product"] = product_type_data.pop("key_asset", False)
-        member = Product_Type_Member()
-        member.user = self.request.user
-        member.product_type = Product_Type(**product_type_data)
-        member.role = Role.objects.get(is_owner=True)
-        member.save()
+        # Reuse the just-saved instance — re-constructing a new Product_Type
+        # from serializer.data was a leftover from the RBAC era and breaks
+        # under legacy because authorized_users is a M2M that can't be
+        # assigned via __init__ kwargs.
+        product_type = serializer.instance
+        Product_Type_Member.objects.create(
+            user=self.request.user,
+            product_type=product_type,
+            role=Role.objects.get(is_owner=True),
+        )
 
     def destroy(self, request, *args, **kwargs):
         instance = self.get_object()

unittests/test_rest_framework.py

@@ -101,7 +101,7 @@
     Product_Type_Member,
     Role,
 )
-from dojo.authorization.roles_permissions import Permissions
+from dojo.authorization.roles_permissions import Permissions, permission_to_action
 from dojo.location.api.endpoint_compat import V3EndpointCompatibleViewSet, V3EndpointStatusCompatibleViewSet
 from dojo.location.api.views import LocationFindingReferenceViewSet, LocationProductReferenceViewSet, LocationViewSet
 from dojo.location.models import Location, LocationFindingReference, LocationProductReference
@@ -616,7 +616,7 @@ def test_create_object_not_authorized(self, mock):
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 ANY,
-                self.permission_create)
+                permission_to_action(self.permission_create))
 
         @skipIfNotSubclass(CreateModelMixin)
         def test_create_configuration_not_authorized(self):
@@ -690,13 +690,13 @@ def test_update_object_not_authorized(self, mock):
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 permission_object,
-                self.permission_update)
+                permission_to_action(self.permission_update))
 
             response = self.client.put(relative_url, self.payload)
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 permission_object,
-                self.permission_update)
+                permission_to_action(self.permission_update))
 
         @skipIfNotSubclass(UpdateModelMixin)
         def test_update_configuration_not_authorized(self):
@@ -780,7 +780,7 @@ def test_delete_object_not_authorized(self, mock):
 
             mock.assert_called_with(User.objects.get(username="admin"),
                 permission_object,
-                self.permission_delete)
+                permission_to_action(self.permission_delete))
 
         @skipIfNotSubclass(DestroyModelMixin)
         def test_delete_configuration_not_authorized(self):
@@ -833,7 +833,7 @@ def test_update_object_not_authorized(self, mock):
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 self.permission_check_class.objects.get(id=current_objects["results"][0]["id"]),
-                self.permission_update)
+                permission_to_action(self.permission_update))
 
     class AuthenticatedViewTest(BaseClassTest):
         @skipIfNotSubclass(ListModelMixin)
@@ -2857,7 +2857,7 @@ def test_create_not_authorized_product_name_engagement_name(self, mock, importer
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Engagement.objects.get(id=2),  # engagement id found via product name and engagement name
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -2888,7 +2888,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_engageme
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product.objects.get(id=1),
-                Permissions.Engagement_Add)
+                permission_to_action(Permissions.Engagement_Add))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -2920,7 +2920,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_product(
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product_Type.objects.get(id=1),
-                Permissions.Product_Type_Add_Product)
+                permission_to_action(Permissions.Product_Type_Add_Product))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -2951,7 +2951,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_product_
             response = self.client.post(self.url, payload)
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
-                Permissions.Product_Type_Add)
+                permission_to_action(Permissions.Product_Type_Add))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -2984,10 +2984,10 @@ def test_create_authorized_product_name_engagement_name_auto_create_engagement(s
             mock.assert_has_calls([
                 call(User.objects.get(username="admin"),
                     Product.objects.get(id=1),
-                    Permissions.Engagement_Add),
+                    permission_to_action(Permissions.Engagement_Add)),
                 call(User.objects.get(username="admin"),
                     Product.objects.get(id=1),
-                    Permissions.Import_Scan_Result),
+                    permission_to_action(Permissions.Import_Scan_Result)),
             ])
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
@@ -3019,7 +3019,7 @@ def test_create_authorized_product_name_engagement_name_auto_create_product(self
             self.assertEqual(201, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product_Type.objects.get(id=1),
-                Permissions.Product_Type_Add_Product)
+                permission_to_action(Permissions.Product_Type_Add_Product))
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
 
@@ -3050,7 +3050,7 @@ def test_create_authorized_product_name_engagement_name_auto_create_product_type
             response = self.client.post(self.url, payload)
             self.assertEqual(201, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
-                Permissions.Product_Type_Add)
+                permission_to_action(Permissions.Product_Type_Add))
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
 
@@ -3118,7 +3118,7 @@ def test_create_not_authorized_product_name_engagement_name(self, mock, importer
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Test.objects.get(id=4),  # test id found via product name and engagement name and scan_type
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3148,7 +3148,7 @@ def test_create_authorized_product_name_engagement_name_scan_type_title_auto_cre
             self.assertEqual(201, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Engagement.objects.get(id=4),
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
 
@@ -3181,10 +3181,10 @@ def test_create_authorized_product_name_engagement_name_auto_create_engagement(s
             mock.assert_has_calls([
                 call(User.objects.get(username="admin"),
                     Product.objects.get(id=1),
-                    Permissions.Engagement_Add),
+                    permission_to_action(Permissions.Engagement_Add)),
                 call(User.objects.get(username="admin"),
                     Product.objects.get(id=1),
-                    Permissions.Import_Scan_Result),
+                    permission_to_action(Permissions.Import_Scan_Result)),
             ])
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
@@ -3217,7 +3217,7 @@ def test_create_authorized_product_name_engagement_name_auto_create_product(self
             self.assertEqual(201, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product_Type.objects.get(id=1),
-                Permissions.Product_Type_Add_Product)
+                permission_to_action(Permissions.Product_Type_Add_Product))
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
 
@@ -3248,7 +3248,7 @@ def test_create_authorized_product_name_engagement_name_auto_create_product_type
             response = self.client.post(self.url, payload)
             self.assertEqual(201, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
-                Permissions.Product_Type_Add)
+                permission_to_action(Permissions.Product_Type_Add))
             importer_mock.assert_called_once()
             reimporter_mock.assert_not_called()
 
@@ -3274,7 +3274,7 @@ def test_create_not_authorized_test_id(self, mock, importer_mock, reimporter_moc
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Test.objects.get(id=3),
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3307,7 +3307,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_engageme
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product.objects.get(id=1),
-                Permissions.Engagement_Add)
+                permission_to_action(Permissions.Engagement_Add))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3339,7 +3339,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_product(
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Product_Type.objects.get(id=1),
-                Permissions.Product_Type_Add_Product)
+                permission_to_action(Permissions.Product_Type_Add_Product))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3370,7 +3370,7 @@ def test_create_not_authorized_product_name_engagement_name_auto_create_product_
             response = self.client.post(self.url, payload)
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
-                Permissions.Product_Type_Add)
+                permission_to_action(Permissions.Product_Type_Add))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3398,7 +3398,7 @@ def test_create_not_authorized_product_name_engagement_name_scan_type(self, mock
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Test.objects.get(id=4),  # engagement id found via product name and engagement name
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3427,7 +3427,7 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self
             self.assertEqual(403, response.status_code, response.content[:1000])
             mock.assert_called_with(User.objects.get(username="admin"),
                 Test.objects.get(id=4),  # test id found via product name and engagement name and scan_type and test_title
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()
 
@@ -3465,7 +3465,7 @@ def test_reimport_engagement_param_ignored_permission_checked_on_name_resolved_t
             # NOT on Test 3 (which belongs to the engagement=1 param)
             mock.assert_called_with(User.objects.get(username="admin"),
                 Test.objects.get(id=4),
-                Permissions.Import_Scan_Result)
+                permission_to_action(Permissions.Import_Scan_Result))
             importer_mock.assert_not_called()
             reimporter_mock.assert_not_called()