Merge pull request #536 from bento-platform/refact/scopeable-model-base-class

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

Author: davidlougheed

.env-sample

@@ -13,14 +13,4 @@ export POSTGRES_PORT=
 
 # CHORD-specific
 export CHORD_URL=
-export CHORD_PERMISSIONS=
 export SERVICE_ID=
-
-# CanDIG-specific
-export INSIDE_CANDIG=true
-export CANDIG_AUTHORIZATION=OPA
-export CANDIG_OPA_URL=http://0.0.0.0:8181
-export CACHE_TIME=0
-export ROOT_CA=
-export CANDIG_OPA_VERSION=
-export PERMISSIONS_SECRET=

README.md

@@ -23,7 +23,6 @@ A Phenopackets-based clinical and phenotypic metadata service for the Bento plat
   - [Standalone PostGres db and AdMiner](#standalone-postgres-db-and-adminer)
   - [Authentication](#authentication)
     - [Note on Permissions](#note-on-permissions)
-    - [Authorization inside CanDIG](#authorization-inside-candig)
   - [Developing](#developing)
     - [Branching](#branching)
     - [Tests](#tests)
@@ -153,16 +152,6 @@ POSTGRES_PORT=5432
 # CHORD/Bento-specific variables:
 #  - If set, used for setting an allowed host & other API-calling purposes
 CHORD_URL=
-#  - If true, will enforce permissions. Do not run with this not set to true in production! 
-#    Defaults to (not DEBUG)
-CHORD_PERMISSIONS=
-
-# CanDIG-specific variables:
-CANDIG_AUTHORIZATION=
-CANDIG_OPA_URL=
-CANDIG_OPA_SECRET=
-CANDIG_OPA_SITE_ADMIN_KEY=
-INSIDE_CANDIG=
 ```
 
 ## Standalone Postgres db and Adminer
@@ -223,15 +212,6 @@ functions as follows:
 This can be turned off with the `CHORD_PERMISSIONS` environment variable and/or
 Django setting, or with the `AUTH_OVERRIDE` Django setting.
 
-### Authorization inside CanDIG
-
-When ran inside the CanDIG context, to properly implement authorization you'll
-have to do the following:
-
-1. Make sure the CHORD_PERMISSIONS is set to "false".
-2. Set CANDIG_AUTHORIZATION to "OPA".
-3. Configure CANDIG_OPA_URL and CANDIG_OPA_SECRET.
-
 
 ## Developing
 

chord_metadata_service/authz/middleware.py

@@ -1,5 +1,3 @@
-import re
-
 from bento_lib.auth.middleware.django import DjangoAuthMiddleware
 from django.conf import settings
 
@@ -10,32 +8,10 @@
     "AuthzMiddleware",
 ]
 
-pattern_get = re.compile(r"^GET$")
-
-# --- List of patterns to apply authz middleware to --------------------------------------------------------------------
-#  - Note: as we gradually roll out authz across Katus, this list will expand. Anything not covered here is assumed to
-#          be protected by the gateway.
-include_pattern_public = (
-    re.compile(r"^(GET|POST|PUT|DELETE)$"),
-    re.compile(r"^/api/(projects|datasets|public|public_overview|public_search_fields|public_rules)$"),
-)
-include_pattern_workflows = (pattern_get, re.compile(r"^(/workflows$|/workflows/)"))
-include_pattern_si = (pattern_get, re.compile(r"^/service-info"))
-include_pattern_schemas = (pattern_get, re.compile(r"^/schemas/.+$"))
-include_pattern_schema_types = (pattern_get, re.compile(r"^/extra_properties_schema_types$"))
-# ----------------------------------------------------------------------------------------------------------------------
-
 authz_middleware = DjangoAuthMiddleware(
     bento_authz_service_url=settings.BENTO_AUTHZ_SERVICE_URL,
     debug_mode=settings.DEBUG,
     enabled=settings.BENTO_AUTHZ_ENABLED,
-    include_request_patterns=(
-        include_pattern_public,
-        include_pattern_workflows,
-        include_pattern_si,
-        include_pattern_schemas,
-        include_pattern_schema_types,
-    ),
     logger=logger,
 )
 

chord_metadata_service/authz/permissions.py

@@ -1,14 +1,17 @@
-from django.conf import settings
+from asgiref.sync import async_to_sync
 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",
-    "ReadOnly",
-    "OverrideOrSuperUserOnly",
+    "BentoDataTypePermission",
 ]
 
 
@@ -36,13 +39,17 @@ def has_permission(self, _request, _view):
         return True  # we return true, like AllowAny, but we don't mark authz as done - so we defer it to the handler
 
 
-class ReadOnly(BasePermission):
-    def has_permission(self, request, view):
-        return request.method in SAFE_METHODS
-
-
-class OverrideOrSuperUserOnly(BasePermission):
-    def has_permission(self, request, view):
-        # If in CHORD production, is_superuser will be set by remote user headers.
-        # TODO: Configuration: Allow configurable read-only APIs or other external access
-        return settings.AUTH_OVERRIDE or request.user.is_superuser
+class BentoDataTypePermission(BasePermission):
+    @async_to_sync
+    async def has_permission(self, request: DrfRequest, view) -> bool:
+        # view: BentoAuthzScopedModelViewSet (cannot annotate due to circular import)
+        if view.data_type is None:
+            raise NotImplementedError("BentoAuthzScopedModelViewSet 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: BentoAuthzScopedModelViewSet (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)

chord_metadata_service/authz/tests/helpers.py

@@ -1,32 +1,24 @@
+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
 
 
 __all__ = [
-    "mock_authz_eval_one_result",
-    "mock_authz_eval_result",
     "DTAccessLevel",
     "AuthzAPITestCase",
     "PermissionsTestCaseMixin",
 ]
 
 
-def mock_authz_eval_one_result(m: aioresponses, result: bool):
-    m.post("http://authz.local/policy/evaluate", payload={"result": [[result]]})
-
-
-def mock_authz_eval_result(m: aioresponses, result: EvaluationResultMatrix | list[list[bool]]):
-    m.post("http://authz.local/policy/evaluate", payload={"result": result})
-
-
 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,44 +34,65 @@ class AuthzAPITestCase(APITestCase):
 
     # ------------------------------------------------------------------------------------------------------------------
 
-    def _one_authz_post(self, authz_res: bool, url: str, *args, **kwargs):
+    @staticmethod
+    def mock_authz_eval_one_result(m: aioresponses, result: bool):
+        m.post("http://authz.local/policy/evaluate", payload={"result": [[result]]})
+
+    @staticmethod
+    def mock_authz_eval_result(m: aioresponses, result: EvaluationResultMatrix | list[list[bool]]):
+        m.post("http://authz.local/policy/evaluate", payload={"result": result})
+
+    # ------------------------------------------------------------------------------------------------------------------
+
+    def _one_authz_generic(
+        self, method: Literal["get", "post", "put", "patch", "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", "patch") 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)
+            self.mock_authz_eval_one_result(m, authz_res)
+            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):
-        with aioresponses() as m:
-            mock_authz_eval_one_result(m, authz_res)
-            return self.client.patch(url, *args, content_type="application/json", **kwargs)
+        return self._one_authz_generic("patch", authz_res, url, *args, **kwargs)
 
     def one_authz_patch(self, url: str, *args, **kwargs):
         """
@@ -89,12 +102,12 @@ def one_authz_patch(self, url: str, *args, **kwargs):
 
     def _one_authz_delete(self, authz_res: bool, url: str, *args, **kwargs):
         with aioresponses() as m:
-            mock_authz_eval_one_result(m, authz_res)
+            self.mock_authz_eval_one_result(m, authz_res)
             return self.client.delete(url, *args, **kwargs)
 
     async def _async_one_authz_delete(self, authz_res: bool, url: str, *args, **kwargs):
         with aioresponses() as m:
-            mock_authz_eval_one_result(m, authz_res)
+            self.mock_authz_eval_one_result(m, authz_res)
             return await self.async_client.delete(url, *args, **kwargs)
 
     def one_authz_delete(self, url: str, *args, **kwargs):
@@ -119,12 +132,12 @@ def one_no_authz_delete(self, url: str, *args, **kwargs):
 
     def dt_get(self, level: Literal["none", "bool", "counts", "full"], url: str, *args, **kwargs):
         with aioresponses() as m:
-            mock_authz_eval_result(m, self.dt_levels[level])  # data type permissions: bool, counts, data
+            self.mock_authz_eval_result(m, self.dt_levels[level])  # data type permissions: bool, counts, data
             return self.client.get(url, *args, **kwargs)
 
     def dt_post(self, level: Literal["none", "bool", "counts", "full"], url: str, *args, **kwargs):
         with aioresponses() as m:
-            mock_authz_eval_result(m, self.dt_levels[level])  # data type permissions: bool, counts, data
+            self.mock_authz_eval_result(m, self.dt_levels[level])  # data type permissions: bool, counts, data
             return self.client.post(url, *args, **kwargs)
 
     def dt_authz_none_get(self, url: str, *args, **kwargs):
@@ -136,6 +149,9 @@ def dt_authz_bool_get(self, url: str, *args, **kwargs):
     def dt_authz_counts_get(self, url: str, *args, **kwargs):
         return self.dt_get("counts", url, *args, **kwargs)
 
+    def dt_authz_counts_post(self, url: str, *args, **kwargs):
+        return self.dt_post("counts", url, *args, **kwargs)
+
     def dt_authz_full_get(self, url: str, *args, **kwargs):
         return self.dt_get("full", url, *args, **kwargs)
 

chord_metadata_service/authz/tests/test_viewset.py

@@ -0,0 +1,88 @@
+import uuid
+
+from aioresponses import aioresponses
+from django.http.request import HttpRequest
+from rest_framework.request import Request as DrfRequest
+
+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
+
+
+class TestNotImplViewSet(BentoAuthzScopedModelGenericListViewSet):
+    pass
+
+
+class AuthzBaseViewsetTest(AuthzAPITestCase, ProjectTestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.individual = ph_m.Individual.objects.create(**ph_c.VALID_INDIVIDUAL_1)
+
+        self.mock_project_req = HttpRequest()
+        self.mock_project_req.GET["project"] = str(self.project.identifier)
+        self.mock_project_drf_req = DrfRequest(self.mock_project_req)
+
+    def test_get_queryset_not_impl(self):
+        with self.assertRaises(NotImplementedError):
+            TestNotImplViewSet().get_queryset()
+
+    def test_permission_from_request_none(self):
+        vs = TestNotImplViewSet()
+        vs.action = "fubar"
+        mock_drf_req = DrfRequest(HttpRequest())
+        self.assertIsNone(vs.permission_from_request(mock_drf_req))
+
+    async def test_obj_is_in_request_scope(self):
+        mock_req = HttpRequest()
+        mock_req.GET["project"] = "does-not-exist"
+        mock_drf_req = DrfRequest(mock_req)
+
+        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)
+
+        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()
+        vs.action = "list"
+        with aioresponses() as m:
+            self.mock_authz_eval_one_result(m, True)
+            self.assertTrue(await vs.request_has_data_type_permissions(self.mock_project_drf_req, None))
+
+    async def test_request_has_data_type_permissions_false(self):
+        vs = TestNotImplViewSet()
+        vs.action = "list"
+        with aioresponses() as m:
+            self.mock_authz_eval_one_result(m, False)
+            self.assertFalse(await vs.request_has_data_type_permissions(self.mock_project_drf_req, None))
+
+    async def test_request_has_data_type_permissions_action_dne(self):
+        vs = TestNotImplViewSet()
+        vs.action = "does-not-exist"  # no permissions implemented for this action
+        self.assertFalse(await vs.request_has_data_type_permissions(self.mock_project_drf_req, None))
+
+    async def test_request_has_data_type_permissions_scope_dne(self):
+        mock_req = HttpRequest()
+        mock_req.GET["project"] = "does-not-exist"
+        mock_drf_req = DrfRequest(mock_req)
+
+        vs = TestNotImplViewSet()
+
+        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)
+
+        with self.assertRaises(DiscoveryScopeException):
+            await vs.request_has_data_type_permissions(mock_drf_req_2, None)