refact!: define a "scopeable model" base class + modelviewset scoping

chord_metadata_service/authz/permissions.py

@@ -1,12 +1,18 @@
+from asgiref.sync import async_to_sync
 from django.conf import settings
 from rest_framework.permissions import BasePermission, SAFE_METHODS
+from rest_framework.request import Request as DrfRequest
+
+from chord_metadata_service.discovery.scopeable_model import BaseScopeableModel
+
 from .middleware import authz_middleware
 
 
 __all__ = [
     "BentoAllowAny",
     "BentoAllowAnyReadOnly",
     "BentoDeferToHandler",
+    "BentoDataTypePermission",
     "ReadOnly",
     "OverrideOrSuperUserOnly",
 ]
@@ -36,6 +42,22 @@
         return True  # we return true, like AllowAny, but we don't mark authz as done - so we defer it to the handler
 
 
+class BentoDataTypePermission(BasePermission):
+    @async_to_sync
+    async def has_permission(self, request: DrfRequest, view):
+        # view: BentoAuthzModelViewSet (cannot annotate due to circular import)
+        if view.data_type is None:
+            raise NotImplementedError("BentoAuthzModelViewSet DATA_TYPE must be set")
+        return await view.request_has_data_type_permissions(request)
+
+    @async_to_sync
+    async def has_object_permission(self, request: DrfRequest, view, obj: BaseScopeableModel):
+        # view: BentoAuthzModelViewSet (cannot annotate due to circular import)
+        # if this is called, has_data_type_permission has already been called and handled the overall action type
+        # TODO: eliminate duplicate scope check somehow without enabling permissions on objects outside of scope
+        return await view.obj_is_in_request_scope(request, obj)
+
+
 class ReadOnly(BasePermission):
     def has_permission(self, request, view):
         return request.method in SAFE_METHODS

chord_metadata_service/authz/tests/helpers.py

@@ -1,6 +1,8 @@
+import json
+
 from aioresponses import aioresponses
 from bento_lib.auth.types import EvaluationResultMatrix
-from rest_framework.test import APITestCase
+from rest_framework.test import APITransactionTestCase
 from typing import Literal
 
 from ..types import DataPermissionsDict
@@ -26,7 +28,7 @@ def mock_authz_eval_result(m: aioresponses, result: EvaluationResultMatrix | lis
 DTAccessLevel = Literal["none", "bool", "counts", "full"]
 
 
-class AuthzAPITestCase(APITestCase):
+class AuthzAPITestCase(APITransactionTestCase):
     # data type permissions: bool, counts, data
     dt_none_eval_res = [[False, False, False]]
     dt_bool_eval_res = [[True, False, False]]
@@ -42,38 +44,51 @@ class AuthzAPITestCase(APITestCase):
 
     # ------------------------------------------------------------------------------------------------------------------
 
-    def _one_authz_post(self, authz_res: bool, url: str, *args, **kwargs):
+    def _one_authz_generic(
+        self, method: Literal["get", "post", "put", "delete"], authz_res: bool, url: str, *args, **kwargs
+    ):
+        if "json" in kwargs:
+            kwargs["data"] = json.dumps(kwargs["json"])
+            del kwargs["json"]
+
+        if method in ("post", "put") and "format" not in kwargs:
+            kwargs["content_type"] = "application/json"
+
         with aioresponses() as m:
             mock_authz_eval_one_result(m, authz_res)
-            return self.client.post(url, *args, content_type="application/json", **kwargs)
+            return getattr(self.client, method)(url, *args, **kwargs)
+
+    def _one_authz_get(self, authz_res: bool, url: str, *args, **kwargs):
+        return self._one_authz_generic("get", authz_res, url, *args, **kwargs)
+
+    def one_authz_get(self, url: str, *args, **kwargs):
+        """Mocks a single True response from the authorization service and executes a GET request."""
+        return self._one_authz_get(True, url, *args, **kwargs)
+
+    def one_no_authz_get(self, url: str, *args, **kwargs):
+        """Mocks a single False response from the authorization service and executes a GET request."""
+        return self._one_authz_get(False, url, *args, **kwargs)
+
+    def _one_authz_post(self, authz_res: bool, url: str, *args, **kwargs):
+        return self._one_authz_generic("post", authz_res, url, *args, **kwargs)
 
     def one_authz_post(self, url: str, *args, **kwargs):
-        """
-        Mocks a single True response from the authorization service and executes a JSON POST request.
-        """
+        """Mocks a single True response from the authorization service and executes a JSON POST request."""
         return self._one_authz_post(True, url, *args, **kwargs)
 
     def one_no_authz_post(self, url: str, *args, **kwargs):
-        """
-        Mocks a single False response from the authorization service and executes a JSON POST request.
-        """
+        """Mocks a single False response from the authorization service and executes a JSON POST request."""
         return self._one_authz_post(False, url, *args, **kwargs)
 
     def _one_authz_put(self, authz_res: bool, url: str, *args, **kwargs):
-        with aioresponses() as m:
-            mock_authz_eval_one_result(m, authz_res)
-            return self.client.put(url, *args, content_type="application/json", **kwargs)
+        return self._one_authz_generic("put", authz_res, url, *args, **kwargs)
 
     def one_authz_put(self, url: str, *args, **kwargs):
-        """
-        Mocks a single True response from the authorization service and executes a JSON PUT request.
-        """
+        """Mocks a single True response from the authorization service and executes a JSON PUT request."""
         return self._one_authz_put(True, url, *args, **kwargs)
 
     def one_no_authz_put(self, url: str, *args, **kwargs):
-        """
-        Mocks a single False response from the authorization service and executes a JSON PUT request.
-        """
+        """Mocks a single False response from the authorization service and executes a JSON PUT request."""
         return self._one_authz_put(False, url, *args, **kwargs)
 
     def _one_authz_patch(self, authz_res: bool, url: str, *args, **kwargs):

chord_metadata_service/authz/viewset.py

@@ -0,0 +1,58 @@
+from bento_lib.auth.permissions import P_QUERY_DATA, Permission, P_INGEST_DATA, P_DELETE_DATA
+from rest_framework import viewsets
+from rest_framework.request import Request as DrfRequest
+
+from chord_metadata_service.discovery.scope import get_request_discovery_scope, INSTANCE_SCOPE, ValidatedDiscoveryScope
+from chord_metadata_service.discovery.scopeable_model import BaseScopeableModel
+
+from .permissions import BentoDataTypePermission
+from .middleware import authz_middleware
+
+__all__ = [
+    "BentoAuthzModelViewSet",
+]
+
+
+class BentoAuthzModelViewSet(viewsets.ModelViewSet):
+    data_type: str | None = None
+    scope_enabled: bool = False  # must be set to True in order to get correctly-scoped permissions
+
+    permission_classes = (BentoDataTypePermission,)
+
+    async def _get_scope_for_request(self, request: DrfRequest) -> ValidatedDiscoveryScope:
+        if self.scope_enabled:
+            return await get_request_discovery_scope(request)
+        else:
+            return INSTANCE_SCOPE
+
+    async def obj_is_in_request_scope(self, request: DrfRequest, obj: BaseScopeableModel) -> bool:
+        scope = await self._get_scope_for_request(request)
+        return await obj.scope_contains_object_async(scope)
+
+    def permission_from_request(self, request: DrfRequest) -> Permission | None:
+        if self.action in ("list", "retrieve"):
+            return P_QUERY_DATA
+        elif self.action in ("create", "update"):
+            return P_INGEST_DATA
+        elif self.action == "destroy":
+            return P_DELETE_DATA
+        else:
+            return None
+
+    async def request_has_data_type_permissions(
+        self, request: DrfRequest, scope: ValidatedDiscoveryScope | None = None
+    ):
+        # We MUST specifically mark view sets as scope-enabled (which means their queryset handles scope correctly);
+        # otherwise, we cannot scope into a specific project/dataset and must use the whole instance as the scope.
+        # Otherwise, we can could leak data from other projects/datasets.
+        # TODO: there must be a better way to enforce this without manual flagging
+
+        _scope: ValidatedDiscoveryScope = scope or await self._get_scope_for_request(request)
+
+        p: Permission | None = self.permission_from_request(request)
+        if p is None:
+            return False
+
+        return await authz_middleware.async_evaluate_one(
+            request, _scope.as_authz_resource(data_type=self.data_type), p, mark_authz_done=True
+        )

chord_metadata_service/chord/api_views.py

@@ -36,7 +36,6 @@
     ProjectSerializer,
     DatasetSerializer,
 )
-from .filters import AuthorizedDatasetFilter
 
 logger = logging.getLogger(__name__)
 
@@ -135,7 +134,6 @@ class DatasetViewSet(CHORDPublicModelViewSet):
     """
 
     filter_backends = [DjangoFilterBackend]
-    filterset_class = AuthorizedDatasetFilter
     lookup_url_kwarg = "dataset_id"
 
     serializer_class = DatasetSerializer

chord_metadata_service/chord/models.py

@@ -3,12 +3,12 @@
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.utils import timezone
+from chord_metadata_service.discovery.schemas import DISCOVERY_SCHEMA
 from chord_metadata_service.patients.models import Individual
 from chord_metadata_service.phenopackets.models import Biosample, Phenopacket
 from chord_metadata_service.resources.models import Resource
 from chord_metadata_service.restapi.validators import JsonSchemaValidator
 from chord_metadata_service.restapi.models import SchemaType
-from chord_metadata_service.discovery.schemas import DISCOVERY_SCHEMA
 
 
 __all__ = ["Project", "Dataset", "ProjectJsonSchema"]

chord_metadata_service/chord/tests/helpers.py

@@ -1,13 +1,11 @@
-import json
-
 from django.db.models import Model
 from django.test import TestCase
 from django.urls import reverse
 
 from chord_metadata_service.authz.tests.helpers import AuthzAPITestCase
 from chord_metadata_service.chord.models import Dataset, Project
 from chord_metadata_service.chord.tests.constants import VALID_DATA_USE_1, VALID_PROJECT_1
-from chord_metadata_service.discovery.utils import ValidatedDiscoveryScope
+from chord_metadata_service.discovery.scope import ValidatedDiscoveryScope
 from chord_metadata_service.restapi.utils import remove_computed_properties
 
 
@@ -79,5 +77,5 @@ def assert_model_fields_equal(self, db_obj: Model, ground_truth: dict,
 class AuthzAPITestCaseWithProjectJSON(AuthzAPITestCase):
     def setUp(self) -> None:
         super().setUp()
-        r = self.one_authz_post(reverse("project-list"), data=json.dumps(VALID_PROJECT_1))
+        r = self.one_authz_post(reverse("project-list"), json=VALID_PROJECT_1)
         self.project = r.json()

chord_metadata_service/chord/tests/test_api.py

@@ -222,7 +222,7 @@ def test_resources(self):
             "iri_prefix": "http://purl.obolibrary.org/obo/NCBITaxon_",
         }
 
-        r = self.client.post("/api/resources", data=json.dumps(resource), content_type="application/json")
+        r = self.one_authz_post("/api/resources", json=resource)
         self.assertEqual(r.status_code, status.HTTP_201_CREATED)
 
         r = self.one_authz_post(
@@ -263,7 +263,7 @@ def setUp(self):
         }
 
     def test_update_dataset(self):
-        r = self.one_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
+        r = self.one_authz_put(f"/api/datasets/{self.dataset.identifier}", json=self.valid_update)
         self.assertEqual(r.status_code, status.HTTP_200_OK)
         self.dataset.refresh_from_db()
         self.assertEqual(self.dataset.title, self.valid_update["title"])
@@ -306,11 +306,11 @@ def test_update_dataset_bad_dats_json(self):
         )
 
     def test_update_dataset_forbidden(self):
-        r = self.one_no_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
+        r = self.one_no_authz_put(f"/api/datasets/{self.dataset.identifier}", json=self.valid_update)
         self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_update_dataset_not_found(self):
-        r = self.one_authz_put(f"/api/datasets/{uuid.uuid4()}", data=json.dumps(self.valid_update))
+        r = self.one_authz_put(f"/api/datasets/{uuid.uuid4()}", json=self.valid_update)
         self.assertEqual(r.status_code, status.HTTP_404_NOT_FOUND)
 
 

chord_metadata_service/chord/tests/test_api_data_types.py

@@ -5,8 +5,8 @@
 from rest_framework import status
 
 from chord_metadata_service.authz.tests.helpers import AuthzAPITestCase, PermissionsTestCaseMixin
+from chord_metadata_service.discovery.scope import get_discovery_scope
 from chord_metadata_service.discovery.tests.constants import DISCOVERY_CONFIG_TEST
-from chord_metadata_service.discovery.utils import get_discovery_scope
 from chord_metadata_service.phenopackets.tests.helpers import PhenoTestCase
 
 from ..data_types import DATA_TYPE_EXPERIMENT, DATA_TYPE_PHENOPACKET, DATA_TYPES

chord_metadata_service/chord/views_data_types.py

@@ -20,11 +20,8 @@
 from chord_metadata_service.cleanup import run_all_cleanup
 from chord_metadata_service.discovery.censorship import thresholded_count
 from chord_metadata_service.discovery.exceptions import DiscoveryScopeException
-from chord_metadata_service.discovery.utils import (
-    get_discovery_data_type_permissions,
-    ValidatedDiscoveryScope,
-    get_request_discovery_scope,
-)
+from chord_metadata_service.discovery.scope import ValidatedDiscoveryScope, get_request_discovery_scope
+from chord_metadata_service.discovery.utils import get_discovery_data_type_permissions
 from chord_metadata_service.experiments.models import Experiment
 from chord_metadata_service.logger import logger
 from chord_metadata_service.phenopackets.models import Phenopacket

chord_metadata_service/chord/views_search.py

@@ -26,7 +26,7 @@
 from chord_metadata_service.authz.helpers import get_data_type_query_permissions
 from chord_metadata_service.authz.permissions import BentoAllowAny, OverrideOrSuperUserOnly, ReadOnly
 
-from chord_metadata_service.discovery.utils import ValidatedDiscoveryScope
+from chord_metadata_service.discovery.scope import ValidatedDiscoveryScope
 
 from chord_metadata_service.experiments.api_views import EXPERIMENT_SELECT_REL, EXPERIMENT_PREFETCH
 from chord_metadata_service.experiments.models import Experiment