Address review: raise NotImplementedError in csrf noop, fix mypy errors

Author: JSv4

config/graphql/analysis_mutations.py

@@ -206,3 +206,5 @@ def mutate(root, info, id) -> "DeleteAnalysisMutation":
 
         # Kick off an async task to delete the analysis (as it can be very large)
         delete_analysis_and_annotations_task.si(analysis_pk=analysis_pk).apply_async()
+
+        return DeleteAnalysisMutation(ok=True, message="SUCCESS")

config/graphql/pipeline_queries.py

@@ -5,7 +5,7 @@
 from __future__ import annotations
 
 import logging
-from collections.abc import Sequence
+from collections.abc import Mapping, Sequence
 from typing import Optional
 
 import graphene
@@ -45,7 +45,7 @@ class PipelineQueryMixin:
     def resolve_pipeline_components(
         self,
         info: graphene.ResolveInfo,
-        mimetype: Optional[FileTypeEnum] = None,
+        mimetype: FileTypeEnum | None = None,
     ) -> PipelineComponentsType:
         """
         Resolver for the pipeline_components query.
@@ -60,7 +60,7 @@ def resolve_pipeline_components(
         Returns:
             PipelineComponentsType: The pipeline components grouped by type.
         """
-        components_data: dict[str, Sequence[PipelineComponentDefinition]]
+        components_data: Mapping[str, Sequence[PipelineComponentDefinition]]
         if mimetype:
             mime_type_str = FILE_TYPE_TO_MIME.get(mimetype.value)
             if mime_type_str is None:
@@ -131,7 +131,7 @@ def to_graphql_type(
             defn: PipelineComponentDefinition, component_type: str
         ) -> PipelineComponentType:
             is_enabled = (not enabled_set) or (defn.class_name in enabled_set)
-            settings_schema: Optional[list[ComponentSettingSchemaType]] = None
+            settings_schema: list[ComponentSettingSchemaType] | None = None
             if user.is_superuser:
                 # Get schema augmented with has_value/current_value from DB
                 augmented_schema = settings_instance.get_component_schema(

config/graphql/security.py

@@ -30,9 +30,11 @@
 
 def _csrf_noop_get_response(request: HttpRequest) -> HttpResponse:
     # CsrfViewMiddleware only invokes ``get_response`` when we call
-    # ``__call__``; we only use ``process_view``, so this is never reached.
-    # An empty ``HttpResponse`` keeps the middleware's type contract happy.
-    return HttpResponse()
+    # ``__call__``; we only use ``process_view``, so this is unreachable.
+    raise NotImplementedError(
+        "_csrf_noop_get_response is unreachable: CsrfViewMiddleware.process_view "
+        "does not call get_response."
+    )
 
 
 _csrf_middleware = CsrfViewMiddleware(_csrf_noop_get_response)

opencontractserver/utils/permissioning.py

@@ -22,7 +22,7 @@
 
 
 def set_permissions_for_obj_to_user(
-    user_val: int | str | "UserModel",
+    user_val: int | str | UserModel,
     instance: django.db.models.Model,
     permissions: list[PermissionTypes],
 ) -> None:
@@ -167,7 +167,7 @@ def set_permissions_for_obj_to_user(
             assign_perm(f"{app_name}.publish_{model_name}", user, instance)
 
 
-def get_users_group_ids(user_instance: "UserModel") -> list[str | int]:
+def get_users_group_ids(user_instance: UserModel) -> list[str | int]:
     """
     For a given user, return list of group ids it belongs to.
     """
@@ -206,7 +206,7 @@ def get_permission_id_to_name_map_for_model(
 
 
 def get_users_permissions_for_obj(
-    user: "UserModel",
+    user: UserModel,
     instance: django.db.models.Model,
     include_group_permissions: bool = False,
 ) -> set[str]:
@@ -304,7 +304,7 @@ def get_users_permissions_for_obj(
 
 
 def user_has_permission_for_obj(
-    user_val: int | str | "UserModel",
+    user_val: int | str | UserModel,
     instance: django.db.models.Model,
     permission: PermissionTypes,
     include_group_permissions: bool = False,
@@ -383,10 +383,10 @@ def user_has_permission_for_obj(
                 # For private annotations, permissions are limited by BOTH the source object AND doc+corpus
                 # We need to check the source object permissions match or exceed what's being requested
                 if instance.created_by_analysis_id:
-                    # Check if user has the requested permission level on the analysis
-                    if not user_has_permission_for_obj(
+                    source_analysis = instance.created_by_analysis
+                    if source_analysis is None or not user_has_permission_for_obj(
                         user,
-                        instance.created_by_analysis,
+                        source_analysis,
                         permission,  # Check for the same permission level being requested
                         include_group_permissions=include_group_permissions,
                     ):
@@ -396,10 +396,10 @@ def user_has_permission_for_obj(
                         )
                         return False
                 elif instance.created_by_extract_id:
-                    # Check if user has the requested permission level on the extract
-                    if not user_has_permission_for_obj(
+                    source_extract = instance.created_by_extract
+                    if source_extract is None or not user_has_permission_for_obj(
                         user,
-                        instance.created_by_extract,
+                        source_extract,
                         permission,  # Check for the same permission level being requested
                         include_group_permissions=include_group_permissions,
                     ):
@@ -409,7 +409,11 @@ def user_has_permission_for_obj(
                         )
                         return False
 
-            # Now check document+corpus permissions using the query optimizer
+            # Now check document+corpus permissions using the query optimizer.
+            # An annotation without a parent document has no inheritable scope,
+            # so non-superuser access is denied (the superuser branch is above).
+            if instance.document_id is None:
+                return False
             can_read, can_create, can_update, can_delete, can_comment = (
                 AnnotationQueryOptimizer._compute_effective_permissions(
                     user=user,
@@ -467,8 +471,12 @@ def user_has_permission_for_obj(
                 )
                 return False
 
-            # Relationships inherit permissions from document+corpus
-            # Use the same logic as annotations
+            # Relationships inherit permissions from document+corpus.
+            # Relationships without a document have no inheritable scope, so
+            # we deny all non-superuser access (the superuser short-circuit
+            # above is the only escape).
+            if instance.document_id is None:
+                return False
             can_read, can_create, can_update, can_delete, can_comment = (
                 AnnotationQueryOptimizer._compute_effective_permissions(
                     user=user,