refact!: handle scope exceptions with DRF exc handler

chord_metadata_service/authz/tests/test_viewset.py

@@ -7,6 +7,7 @@
 from chord_metadata_service.authz.tests.helpers import AuthzAPITestCase
 from chord_metadata_service.authz.viewset import BentoAuthzScopedModelGenericListViewSet
 from chord_metadata_service.chord.tests.helpers import ProjectTestCase
+from chord_metadata_service.discovery.exceptions import DiscoveryScopeException
 from chord_metadata_service.phenopackets import models as ph_m
 from chord_metadata_service.phenopackets.tests import constants as ph_c
 
@@ -40,13 +41,15 @@ async def test_obj_is_in_request_scope(self):
         mock_req.GET["project"] = "does-not-exist"
         mock_drf_req = DrfRequest(mock_req)
 
-        self.assertFalse(await TestNotImplViewSet.obj_is_in_request_scope(mock_drf_req, self.individual))
+        with self.assertRaises(DiscoveryScopeException):
+            await TestNotImplViewSet.obj_is_in_request_scope(mock_drf_req, self.individual)
 
         mock_req_2 = HttpRequest()
         mock_req_2.GET["project"] = str(uuid.uuid4())
         mock_drf_req_2 = DrfRequest(mock_req_2)
 
-        self.assertFalse(await TestNotImplViewSet.obj_is_in_request_scope(mock_drf_req_2, self.individual))
+        with self.assertRaises(DiscoveryScopeException):
+            await TestNotImplViewSet.obj_is_in_request_scope(mock_drf_req_2, self.individual)
 
     async def test_request_has_data_type_permissions(self):
         vs = TestNotImplViewSet()
@@ -74,10 +77,12 @@ async def test_request_has_data_type_permissions_scope_dne(self):
 
         vs = TestNotImplViewSet()
 
-        self.assertFalse(await vs.request_has_data_type_permissions(mock_drf_req, None))
+        with self.assertRaises(DiscoveryScopeException):
+            await vs.request_has_data_type_permissions(mock_drf_req, None)
 
         mock_req_2 = HttpRequest()
         mock_req_2.GET["project"] = str(uuid.uuid4())
         mock_drf_req_2 = DrfRequest(mock_req_2)
 
-        self.assertFalse(await vs.request_has_data_type_permissions(mock_drf_req_2, None))
+        with self.assertRaises(DiscoveryScopeException):
+            await vs.request_has_data_type_permissions(mock_drf_req_2, None)

chord_metadata_service/authz/viewset.py

@@ -34,10 +34,9 @@ def get_queryset(self):
 
     @staticmethod
     async def obj_is_in_request_scope(request: DrfRequest, obj: BaseScopeableModel) -> bool:
-        try:
-            return await obj.scope_contains_object(await get_request_discovery_scope(request))
-        except DiscoveryScopeException:  # project/dataset does not exist, or non-UUID request for a project/dataset
-            return False
+        # DiscoveryScopeException - project/dataset does not exist, or non-UUID request for a project/dataset
+        #  - will be an API exception and handled by the katsu exception handler
+        return await obj.scope_contains_object(await get_request_discovery_scope(request))
 
     def permission_from_request(self, request: DrfRequest) -> Permission | None:
         if self.action in ("list", "retrieve"):
@@ -53,10 +52,9 @@ def permission_from_request(self, request: DrfRequest) -> Permission | None:
     async def request_has_data_type_permissions(
         self, request: DrfRequest, scope: ValidatedDiscoveryScope | None = None
     ):
-        try:
-            scope_: ValidatedDiscoveryScope = scope or await get_request_discovery_scope(request)
-        except DiscoveryScopeException:  # project/dataset does not exist, or non-UUID request for a project/dataset
-            return False
+        # DiscoveryScopeException - project/dataset does not exist, or non-UUID request for a project/dataset
+        #  - will be an API exception and handled by the katsu exception handler
+        scope_: ValidatedDiscoveryScope = scope or await get_request_discovery_scope(request)
 
         p: Permission | None = self.permission_from_request(request)
         if p is None:

chord_metadata_service/discovery/exceptions.py

@@ -2,8 +2,14 @@
     "DiscoveryScopeException",
 ]
 
+from rest_framework import status
+from rest_framework.exceptions import APIException
 
-class DiscoveryScopeException(Exception):
+
+class DiscoveryScopeException(APIException):
+    status_code = status.HTTP_400_BAD_REQUEST
+    default_detail = "Error validating discovery scope (does not exist)"
+    default_code = "bad_request"
 
     def __init__(self, dataset_id: str | None = None, project_id: str | None = None, *args) -> None:
         self.dataset_id = dataset_id
@@ -18,4 +24,4 @@ def __init__(self, dataset_id: str | None = None, project_id: str | None = None,
             message = message.format("project", project_id)
         self.message = {"message": message}
 
-        super().__init__(*args)
+        super().__init__(*args, detail=message)

chord_metadata_service/experiments/tests/test_api.py

@@ -76,9 +76,10 @@ def test_get_experiments_scoped_forbidden(self):
         r = self.one_no_authz_get(f"/api/experiments?project={self.p.identifier}")
         self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
 
-        # not found, yields 403 even "with auto"
+    def test_get_experiments_scope_not_found(self):
+        # not found, yields scope bad request
         r = self.one_authz_get(f"/api/experiments?project={uuid.uuid4()}")
-        self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
 
     def test_get_experiment_one(self):
         response = self.one_authz_get('/api/experiments/katsu.experiment:1')
@@ -133,9 +134,10 @@ def test_get_experiment_results_scoped_forbidden(self):
         r = self.one_no_authz_get(f"/api/experimentresults?project={self.p.identifier}")
         self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
 
-        # not found, yields 403 even "with auto"
+    def test_get_experiment_results_scope_not_found(self):
+        # not found (bad request for scope)
         r = self.one_authz_get(f"/api/experimentresults?project={uuid.uuid4()}")
-        self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
 
     def test_filter_experiment_results(self):
         response = self.one_authz_get('/api/experimentresults?file_format=vcf')

chord_metadata_service/metadata/settings.py

@@ -271,6 +271,7 @@ def get_secret(path):
     'DEFAULT_PERMISSION_CLASSES': ['chord_metadata_service.authz.permissions.BentoDeferToHandler'],
     'DEFAULT_SCHEMA_CLASS': 'drf_spectacular.openapi.AutoSchema',
     'DEFAULT_FILTER_BACKENDS': ['django_filters.rest_framework.DjangoFilterBackend'],
+    'EXCEPTION_HANDLER': 'chord_metadata_service.restapi.exception_handler.katsu_exception_handler',
     'JSON_UNDERSCOREIZE': {
         'no_underscore_before_number': True
     }

chord_metadata_service/resources/tests/test_api.py

@@ -72,11 +72,11 @@ def test_list_resources_basic(self):
     def test_list_resources_scope_dne(self):
         res = self.one_authz_get(f"{self.url}?project=does-not-exist")
         # non-UUID - triggers scope error when handling permissions:
-        self.assertEqual(res.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
 
         res = self.one_authz_get(f"{self.url}?project={uuid.uuid4()}")
         # does not exist - triggers scope error when handling permissions:
-        self.assertEqual(res.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
 
     def test_list_resources_forbidden(self):
         response = self.one_no_authz_get(self.url)

chord_metadata_service/restapi/exception_handler.py

@@ -0,0 +1,17 @@
+from rest_framework.views import exception_handler
+
+from chord_metadata_service.authz.middleware import authz_middleware
+from chord_metadata_service.discovery.exceptions import DiscoveryScopeException
+
+__all__ = ["katsu_exception_handler"]
+
+
+def katsu_exception_handler(exc, context):
+    # Start with default DRF exception handler
+    response = exception_handler(exc, context)
+
+    if isinstance(exc, DiscoveryScopeException):
+        # Allow scope exception responses through the authz middleware (mark them as authorized)
+        authz_middleware.mark_authz_done(context["request"])
+
+    return response