✅ [#4398] Fix coverage for Objects API ownership validation

open-formulieren · Dec 2, 2024 · 1cd37e4 · 1cd37e4
1 parent 22b6430
commit 1cd37e4
Show file tree

Hide file tree

Showing 3 changed files with 294 additions and 15 deletions.
diff --git a/src/openforms/contrib/objects_api/ownership_validation.py b/src/openforms/contrib/objects_api/ownership_validation.py
@@ -24,7 +24,6 @@ def validate_object_ownership(
     client: ObjectsClient,
     object_attribute: list[str],
     plugin: BasePrefillPlugin | BaseRegistrationPlugin,
-    raise_exception: bool = True,
 ) -> None:
     """
     Function to check whether the user associated with a Submission is the owner
@@ -52,23 +51,19 @@ def validate_object_ownership(
             "Something went wrong while trying to retrieve "
             "object for initial_data_reference"
         )
-        if raise_exception:
-            raise PermissionDenied from e
+        raise PermissionDenied from e
 
-    if not object:
-        # If the object cannot be found, we cannot consider the ownership check failed
-        # because it is not verified that the user is not the owner
-        logger.warning(
-            "Could not find object for initial_data_reference: %s",
-            submission.initial_data_reference,
+    if not object_attribute:
+        logger.exception(
+            "No path for auth value configured: %s, cannot perform ownership check",
+            object_attribute,
+        )
+        raise PermissionDenied(
+            "Could not verify if user is owner of the referenced object"
         )
-        if raise_exception:
-            raise PermissionDenied("Could not find object for initial_data_reference")
-        else:
-            return
 
     try:
-        auth_value = glom(object["record"]["data"], Path(*object_attribute))
+        auth_value = glom(object["record"]["data"], Path(*(object_attribute or [])))
     except PathAccessError as e:
         logger.exception(
             "Could not retrieve auth value for path %s, it could be incorrectly configured",

diff --git a/...ValidatorTests.test_ownership_check_fails_if_auth_attribute_path_is_badly_configured.yaml b/...ValidatorTests.test_ownership_check_fails_if_auth_attribute_path_is_badly_configured.yaml
@@ -0,0 +1,202 @@
+interactions:
+- request:
+    body: '{"type": "http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e",
+      "record": {"typeVersion": 1, "data": {"nested": {"bsn": "111222333"}, "foo":
+      "bar"}, "startAt": "2024-11-29"}}'
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate, br
+      Authorization:
+      - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      Content-Length:
+      - '206'
+      Content-Type:
+      - application/json
+      User-Agent:
+      - python-requests/2.32.2
+    method: POST
+    uri: http://localhost:8002/api/v2/objects
+  response:
+    body:
+      string: '{"url":"http://objects-web:8000/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c","uuid":"c614f674-04dc-435f-a801-99277148b69c","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"nested":{"bsn":"111222333"},"foo":"bar"},"geometry":null,"startAt":"2024-11-29","endAt":null,"registrationAt":"2024-11-29","correctionFor":null,"correctedBy":null}}'
+    headers:
+      Allow:
+      - GET, POST, HEAD, OPTIONS
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      Content-Length:
+      - '433'
+      Content-Type:
+      - application/json
+      Cross-Origin-Opener-Policy:
+      - same-origin
+      Date:
+      - Fri, 29 Nov 2024 09:50:58 GMT
+      Location:
+      - http://localhost:8002/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c
+      Referrer-Policy:
+      - same-origin
+      Server:
+      - nginx/1.27.0
+      Vary:
+      - origin
+      X-Content-Type-Options:
+      - nosniff
+      X-Frame-Options:
+      - DENY
+    status:
+      code: 201
+      message: Created
+- request:
+    body: null
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate, br
+      Authorization:
+      - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      User-Agent:
+      - python-requests/2.32.2
+    method: GET
+    uri: http://localhost:8002/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c
+  response:
+    body:
+      string: '{"url":"http://objects-web:8000/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c","uuid":"c614f674-04dc-435f-a801-99277148b69c","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"foo":"bar","nested":{"bsn":"111222333"}},"geometry":null,"startAt":"2024-11-29","endAt":null,"registrationAt":"2024-11-29","correctionFor":null,"correctedBy":null}}'
+    headers:
+      Allow:
+      - GET, PUT, PATCH, DELETE, HEAD, OPTIONS
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      Content-Length:
+      - '433'
+      Content-Type:
+      - application/json
+      Cross-Origin-Opener-Policy:
+      - same-origin
+      Date:
+      - Fri, 29 Nov 2024 09:50:58 GMT
+      Referrer-Policy:
+      - same-origin
+      Server:
+      - nginx/1.27.0
+      Vary:
+      - origin
+      X-Content-Type-Options:
+      - nosniff
+      X-Frame-Options:
+      - DENY
+    status:
+      code: 200
+      message: OK
+- request:
+    body: null
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate, br
+      Authorization:
+      - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      User-Agent:
+      - python-requests/2.32.2
+    method: GET
+    uri: http://localhost:8002/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c
+  response:
+    body:
+      string: '{"url":"http://objects-web:8000/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c","uuid":"c614f674-04dc-435f-a801-99277148b69c","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"foo":"bar","nested":{"bsn":"111222333"}},"geometry":null,"startAt":"2024-11-29","endAt":null,"registrationAt":"2024-11-29","correctionFor":null,"correctedBy":null}}'
+    headers:
+      Allow:
+      - GET, PUT, PATCH, DELETE, HEAD, OPTIONS
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      Content-Length:
+      - '433'
+      Content-Type:
+      - application/json
+      Cross-Origin-Opener-Policy:
+      - same-origin
+      Date:
+      - Fri, 29 Nov 2024 09:50:58 GMT
+      Referrer-Policy:
+      - same-origin
+      Server:
+      - nginx/1.27.0
+      Vary:
+      - origin
+      X-Content-Type-Options:
+      - nosniff
+      X-Frame-Options:
+      - DENY
+    status:
+      code: 200
+      message: OK
+- request:
+    body: null
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate, br
+      Authorization:
+      - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      User-Agent:
+      - python-requests/2.32.2
+    method: GET
+    uri: http://localhost:8002/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c
+  response:
+    body:
+      string: '{"url":"http://objects-web:8000/api/v2/objects/c614f674-04dc-435f-a801-99277148b69c","uuid":"c614f674-04dc-435f-a801-99277148b69c","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"foo":"bar","nested":{"bsn":"111222333"}},"geometry":null,"startAt":"2024-11-29","endAt":null,"registrationAt":"2024-11-29","correctionFor":null,"correctedBy":null}}'
+    headers:
+      Allow:
+      - GET, PUT, PATCH, DELETE, HEAD, OPTIONS
+      Connection:
+      - keep-alive
+      Content-Crs:
+      - EPSG:4326
+      Content-Length:
+      - '433'
+      Content-Type:
+      - application/json
+      Cross-Origin-Opener-Policy:
+      - same-origin
+      Date:
+      - Fri, 29 Nov 2024 09:50:58 GMT
+      Referrer-Policy:
+      - same-origin
+      Server:
+      - nginx/1.27.0
+      Vary:
+      - origin
+      X-Content-Type-Options:
+      - nosniff
+      X-Frame-Options:
+      - DENY
+    status:
+      code: 200
+      message: OK
+version: 1
diff --git a/src/openforms/contrib/objects_api/tests/test_ownership_validation.py b/src/openforms/contrib/objects_api/tests/test_ownership_validation.py
@@ -4,7 +4,7 @@
 from django.core.exceptions import PermissionDenied
 from django.test import TestCase, override_settings, tag
 
-from requests.exceptions import RequestException
+from requests.exceptions import HTTPError, RequestException
 
 from openforms.authentication.service import AuthAttribute
 from openforms.contrib.objects_api.clients import get_objects_client
@@ -176,6 +176,56 @@ def test_user_is_not_owner_of_object_nested_auth_attribute(self):
             str(cm.exception), "User is not the owner of the referenced object"
         )
 
+    @tag("gh-4398")
+    def test_ownership_check_fails_if_auth_attribute_path_is_badly_configured(self):
+        with get_objects_client(self.objects_api_group_used) as client:
+            object = client.create_object(
+                record_data=prepare_data_for_registration(
+                    data={"nested": {"bsn": "111222333"}, "foo": "bar"},
+                    objecttype_version=1,
+                ),
+                objecttype_url="http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e",
+            )
+            object_ref = object["uuid"]
+
+        submission = SubmissionFactory.create(
+            auth_info__value="123456782",
+            auth_info__attribute=AuthAttribute.bsn,
+            initial_data_reference=object_ref,
+        )
+
+        # The backend that should be used to perform the check
+        FormRegistrationBackendFactory.create(
+            form=submission.form,
+            backend="objects_api",
+            options={
+                "version": 2,
+                "objecttype": "3edfdaf7-f469-470b-a391-bb7ea015bd6f",
+                "objects_api_group": self.objects_api_group_used.pk,
+                "objecttype_version": 1,
+            },
+        )
+
+        with self.subTest("empty path"):
+            with get_objects_client(self.objects_api_group_used) as client:
+                with self.assertRaises(PermissionDenied) as cm:
+                    validate_object_ownership(submission, client, [], PLUGIN)
+            self.assertEqual(
+                str(cm.exception),
+                "Could not verify if user is owner of the referenced object",
+            )
+
+        with self.subTest("non existent path"):
+            with get_objects_client(self.objects_api_group_used) as client:
+                with self.assertRaises(PermissionDenied) as cm:
+                    validate_object_ownership(
+                        submission, client, ["this", "does", "not", "exist"], PLUGIN
+                    )
+            self.assertEqual(
+                str(cm.exception),
+                "Could not verify if user is owner of the referenced object",
+            )
+
     @tag("gh-4398")
     @patch(
         "openforms.contrib.objects_api.clients.objects.ObjectsClient.get_object",
@@ -208,6 +258,38 @@ def test_request_exception_when_doing_permission_check(self, mock_get_object):
             with get_objects_client(self.objects_api_group_used) as client:
                 validate_object_ownership(submission, client, ["bsn"], PLUGIN)
 
+    @tag("gh-4398")
+    @patch(
+        "openforms.contrib.objects_api.clients.objects.ObjectsClient.get_object",
+        side_effect=HTTPError("404"),
+    )
+    def test_object_not_found_when_doing_permission_check(self, mock_get_object):
+        """
+        If the object could not be fetched due to request errors, the ownership check
+        should fail
+        """
+        submission = SubmissionFactory.create(
+            auth_info__value="111222333",
+            auth_info__attribute=AuthAttribute.bsn,
+            initial_data_reference=self.object_ref,
+        )
+
+        # The backend that should be used to perform the check
+        FormRegistrationBackendFactory.create(
+            form=submission.form,
+            backend="objects_api",
+            options={
+                "version": 2,
+                "objecttype": "3edfdaf7-f469-470b-a391-bb7ea015bd6f",
+                "objects_api_group": self.objects_api_group_used.pk,
+                "objecttype_version": 1,
+            },
+        )
+
+        with self.assertRaises(PermissionDenied):
+            with get_objects_client(self.objects_api_group_used) as client:
+                validate_object_ownership(submission, client, ["bsn"], PLUGIN)
+
     @tag("gh-4398")
     def test_no_backends_configured_does_not_raise_error(
         self,