feat(#357): custom_field_definitions generisch — W4b-Muster (422/403-Entity-Checks, {items,total}-Shape, ACL-Fix), zentrale Helper, Plural-Ableitungs-Fix

app/routes/custom_field_definitions.py

@@ -1,4 +1,10 @@
-"""API routes for CustomFieldDefinition CRUD."""
+"""API routes for CustomFieldDefinition CRUD — generic across entities.
+
+Paket 4 (#357): applies the W4b pattern — entity validation (422) and
+owner-module read check (403) via ENTITY_PLUGIN_OWNERS, central helpers
+from entity_permission_service, and the {items, total} list shape all
+frontend consumers read.
+"""
 
 from __future__ import annotations
 
@@ -11,17 +17,22 @@ from app.core.db import get_db
 from app.deps import get_current_user, require_permission
 from app.schemas.custom_field_definition import (
     CustomFieldDefinitionCreate,
+    CustomFieldDefinitionListResponse,
     CustomFieldDefinitionResponse,
     CustomFieldDefinitionUpdate,
 )
 from app.services import custom_field_service
+from app.services.entity_permission_service import (
+    check_entity_read_permission,
+    validate_entity_type,
+)
 
 router = APIRouter(prefix="/api/v1/custom-fields", tags=["custom-fields-definitions"])
 
 
 @router.get(
     "/definitions",
-    response_model=list[CustomFieldDefinitionResponse],
+    response_model=CustomFieldDefinitionListResponse,
     dependencies=[Depends(require_permission("custom_fields:read"))],
 )
 async def list_definitions(
@@ -29,10 +40,28 @@ async def list_definitions(
     db: AsyncSession = Depends(get_db),
     current_user: dict = Depends(get_current_user),
 ):
-    """List all active custom field definitions for the current tenant."""
+    """List all active custom field definitions for the current tenant.
+
+    Returns ``{items, total}`` — the shape the frontend API client and
+    all six consumers (CustomFields page, ContactsList, SortPanel,
+    GroupPanel, FilterPanel) read.
+    """
     tenant_id = uuid.UUID(current_user["tenant_id"])
-    definitions = await custom_field_service.list_definitions(db, tenant_id, entity=entity)
-    return definitions
+    if entity:
+        validate_entity_type(entity)
+        check_entity_read_permission(current_user, entity)
+
+    definitions = await custom_field_service.list_definitions(
+        db,
+        tenant_id,
+        entity=entity,
+        user_id=uuid.UUID(current_user["user_id"]),
+        is_system_admin=current_user.get("is_system_admin", False),
+    )
+    return CustomFieldDefinitionListResponse(
+        items=[CustomFieldDefinitionResponse.model_validate(d) for d in definitions],
+        total=len(definitions),
+    )
 
 
 @router.post(
@@ -46,7 +75,16 @@ async def create_definition(
     db: AsyncSession = Depends(get_db),
     current_user: dict = Depends(get_current_user),
 ):
-    """Create a new custom field definition."""
+    """Create a new custom field definition.
+
+    W4b: the entity must be a registered entity type (422) and the user
+    needs the owning module's read permission (e.g. contacts:read) on top
+    of custom_fields:write — writing definitions for modules you cannot
+    even read would bypass module isolation.
+    """
+    validate_entity_type(body.entity)
+    check_entity_read_permission(current_user, body.entity)
+
     tenant_id = uuid.UUID(current_user["tenant_id"])
     user_id = uuid.UUID(current_user["user_id"])
     definition = await custom_field_service.create_definition(
@@ -69,6 +107,7 @@ async def update_definition(
     """Update an existing custom field definition."""
     tenant_id = uuid.UUID(current_user["tenant_id"])
     user_id = uuid.UUID(current_user["user_id"])
+    is_admin = current_user.get("is_system_admin", False)
     try:
         def_id = uuid.UUID(definition_id)
     except (ValueError, TypeError):
@@ -79,11 +118,16 @@ async def update_definition(
     if not update_data:
         raise HTTPException(400, detail={"detail": "No fields to update", "code": "no_updates"})
 
-    definition = await custom_field_service.update_definition(
-        db, tenant_id, def_id, update_data, user_id=user_id
-    )
+    try:
+        definition = await custom_field_service.update_definition(
+            db, tenant_id, def_id, update_data, user_id=user_id, is_system_admin=is_admin
+        )
+    except PermissionError as e:
+        raise HTTPException(status_code=403, detail={"detail": str(e), "code": "forbidden"}) from e
     if definition is None:
         raise HTTPException(404, detail={"detail": "Definition not found", "code": "not_found"})
+    # W4b: changing a definition still requires read access to its entity module
+    check_entity_read_permission(current_user, definition.entity)
     return definition
 
 
@@ -99,12 +143,19 @@ async def delete_definition(
 ):
     """Delete a custom field definition."""
     tenant_id = uuid.UUID(current_user["tenant_id"])
+    user_id = uuid.UUID(current_user["user_id"])
+    is_admin = current_user.get("is_system_admin", False)
     try:
         def_id = uuid.UUID(definition_id)
     except (ValueError, TypeError):
         raise HTTPException(400, detail={"detail": "Invalid definition_id", "code": "invalid_id"}) from None
 
-    deleted = await custom_field_service.delete_definition(db, tenant_id, def_id)
+    try:
+        deleted = await custom_field_service.delete_definition(
+            db, tenant_id, def_id, user_id=user_id, is_system_admin=is_admin
+        )
+    except PermissionError as e:
+        raise HTTPException(status_code=403, detail={"detail": str(e), "code": "forbidden"}) from e
     if not deleted:
         raise HTTPException(404, detail={"detail": "Definition not found", "code": "not_found"})
     return None

app/routes/saved_filters.py

@@ -18,37 +18,15 @@ from sqlalchemy.ext.asyncio import AsyncSession
 from app.core.db import get_db
 from app.deps import get_current_user
 from app.models.saved_filter import SavedFilter
+from app.services.entity_permission_service import (
+    check_entity_read_permission as _check_entity_read,
+)
+from app.services.entity_permission_service import (
+    validate_entity_type as _validate_entity_type,
+)
 
 router = APIRouter(prefix="/api/v1/saved-filters", tags=["saved-filters"])
 
-VALID_ENTITY_TYPES = None  # Dynamic — validated against ENTITY_MODELS at runtime
-
-
-def _validate_entity_type(entity_type: str) -> None:
-    """Validate entity_type against ENTITY_MODELS. Raises HTTPException if invalid."""
-    from app.services.entity_permission_service import ENTITY_MODELS
-    if entity_type not in ENTITY_MODELS:
-        from fastapi import HTTPException
-        valid = sorted(ENTITY_MODELS.keys())
-        raise HTTPException(422, detail={
-            "detail": f"Invalid entity_type: {entity_type}",
-            "code": "invalid_entity_type",
-            "valid_types": valid,
-        })
-
-
-def _check_entity_read(current_user: dict, entity_type: str) -> None:
-    """Check that the user has read permission for the entity type."""
-    from app.core.permissions import check_permission
-    from app.services.entity_permission_service import get_entity_read_permission
-
-    perm = get_entity_read_permission(entity_type)
-    if not check_permission(current_user, perm):
-        raise HTTPException(403, detail={
-            "detail": f"Permission '{perm}' required",
-            "code": "forbidden",
-        })
-
 
 class SavedFilterCreate(BaseModel):
     """Schema for creating a saved filter."""

app/routes/saved_views.py

@@ -18,37 +18,15 @@ from sqlalchemy.ext.asyncio import AsyncSession
 from app.core.db import get_db
 from app.deps import get_current_user
 from app.models.saved_view import SavedView
+from app.services.entity_permission_service import (
+    check_entity_read_permission as _check_entity_read,
+)
+from app.services.entity_permission_service import (
+    validate_entity_type as _validate_entity_type,
+)
 
 router = APIRouter(prefix="/api/v1/saved-views", tags=["saved-views"])
 
-VALID_ENTITY_TYPES = None  # Dynamic — validated against ENTITY_MODELS at runtime
-
-
-def _validate_entity_type(entity_type: str) -> None:
-    """Validate entity_type against ENTITY_MODELS. Raises HTTPException if invalid."""
-    from app.services.entity_permission_service import ENTITY_MODELS
-    if entity_type not in ENTITY_MODELS:
-        from fastapi import HTTPException
-        valid = sorted(ENTITY_MODELS.keys())
-        raise HTTPException(422, detail={
-            "detail": f"Invalid entity_type: {entity_type}",
-            "code": "invalid_entity_type",
-            "valid_types": valid,
-        })
-
-
-def _check_entity_read(current_user: dict, entity_type: str) -> None:
-    """Check that the user has read permission for the entity type."""
-    from app.core.permissions import check_permission
-    from app.services.entity_permission_service import get_entity_read_permission
-
-    perm = get_entity_read_permission(entity_type)
-    if not check_permission(current_user, perm):
-        raise HTTPException(403, detail={
-            "detail": f"Permission '{perm}' required",
-            "code": "forbidden",
-        })
-
 
 class SavedViewCreate(BaseModel):
     """Schema for creating a saved view."""

app/schemas/custom_field_definition.py

@@ -62,3 +62,10 @@ class CustomFieldDefinitionResponse(BaseModel):
     updated_by: uuid.UUID | None = None
     created_at: datetime
     updated_at: datetime
+
+
+class CustomFieldDefinitionListResponse(BaseModel):
+    """List response — {items, total} shape consumed by the frontend."""
+
+    items: list[CustomFieldDefinitionResponse]
+    total: int

app/services/entity_permission_service.py

@@ -84,10 +84,15 @@ def get_entity_read_permission(entity_type: str) -> str:
     owner = ENTITY_PLUGIN_OWNERS.get(entity_type)
     if owner:
         return f"{owner}:read"
-    # Core entities: derive from module name (e.g. workflows → workflows:read)
-    module = entity_type.rstrip("s")
-    candidates = [k for k in _core_module_keys(module, "read")]
-    return candidates[0] if candidates else "contacts:read"
+    # Core entities: derive from the module name in CORE_PERMISSIONS. Modules
+    # are mostly plural ("workflows", "addresses") while entity types are
+    # mostly singular ("workflow", "address") — try exact, singular and
+    # plural forms before the contacts:read fallback.
+    for module in (entity_type, entity_type.rstrip("s"), f"{entity_type}s", f"{entity_type}es"):
+        candidates = _core_module_keys(module, "read")
+        if candidates:
+            return candidates[0]
+    return "contacts:read"
 
 
 def _core_module_keys(module: str, action: str) -> list[str]:
@@ -100,6 +105,41 @@ def _core_module_keys(module: str, action: str) -> list[str]:
         if p.get("module") == module and p["key"].endswith(f":{action}")
     ]
 
+
+def validate_entity_type(entity_type: str) -> None:
+    """Validate entity_type against ENTITY_MODELS (W4b pattern).
+
+    Central helper for entity-typed CRUD (saved filters/views,
+    custom field definitions). Raises fastapi HTTPException 422
+    with the valid types so clients can self-correct.
+    """
+    if entity_type not in ENTITY_MODELS:
+        from fastapi import HTTPException
+
+        raise HTTPException(422, detail={
+            "detail": f"Invalid entity_type: {entity_type}",
+            "code": "invalid_entity_type",
+            "valid_types": sorted(ENTITY_MODELS.keys()),
+        })
+
+
+def check_entity_read_permission(current_user: dict, entity_type: str) -> None:
+    """Check that the user may read the entity type's owning module (W4b).
+
+    Raises fastapi HTTPException 403 when the derived module read
+    permission (e.g. contacts:read, workflows:read) is missing.
+    """
+    from fastapi import HTTPException
+
+    from app.core.permissions import check_permission
+
+    perm = get_entity_read_permission(entity_type)
+    if not check_permission(current_user, perm):
+        raise HTTPException(403, detail={
+            "detail": f"Permission '{perm}' required",
+            "code": "forbidden",
+        })
+
 # Core models with OwnedMixin (Phase 2 additions)
 try:
     from app.models.entity_attachment import EntityAttachment
@@ -130,6 +170,7 @@ def register_entity_model(
 def unregister_entity_model(entity_type: str) -> None:
     """Unregister an entity model (called during plugin deactivation)."""
     ENTITY_MODELS.pop(entity_type, None)
+    ENTITY_PLUGIN_OWNERS.pop(entity_type, None)
 
 
 def _get_entity_model(entity_type: str) -> type: