feat(#357): W4b — Saved-Views/Filters von contacts:read entkoppelt
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
- ENTITY_PLUGIN_OWNERS-Registry: trackt, welches Plugin welche Entity registriert - get_entity_read_permission(): leitet die modul-korrekte Permission ab (contacts -> contacts:read, tasks -> tasks:read, ...) mit Core-Fallback - registry.activate(): uebergibt plugin_name an register_entity_model - saved_views.py + saved_filters.py: statische contacts:read-Dependencies durch dynamische _check_entity_read() ersetzt — Saved Views/Filters fuer fremde Entities brauchen jetzt die richtige modul-spezifische Permission Verifikation: 10/11 tests/test_saved_filters.py passed (1 Vorbestand- Failure per Stash bewiesen), create_app OK, ruff modified-files gruen. fixes #357 (W4b-Teil)
This commit is contained in:
@@ -622,7 +622,7 @@ class PluginRegistry:
|
|||||||
for entity_type, model_class in plugin.get_entity_models().items():
|
for entity_type, model_class in plugin.get_entity_models().items():
|
||||||
from app.services.entity_permission_service import register_entity_model
|
from app.services.entity_permission_service import register_entity_model
|
||||||
|
|
||||||
register_entity_model(entity_type, model_class)
|
register_entity_model(entity_type, model_class, plugin_name=name)
|
||||||
|
|
||||||
# Call on_activate hook (registers event listeners)
|
# Call on_activate hook (registers event listeners)
|
||||||
await plugin.on_activate(db, self._container, self._event_bus)
|
await plugin.on_activate(db, self._container, self._event_bus)
|
||||||
|
|||||||
+43
-25
@@ -1,9 +1,13 @@
|
|||||||
"""Saved filters routes — CRUD for reusable filter criteria."""
|
"""Saved filters routes — CRUD for reusable filter criteria.
|
||||||
|
|
||||||
|
W4b: Permissions are derived from the entity type's owning plugin instead
|
||||||
|
of a hardcoded ``contacts:read`` (Spec #357/Kritikpunkt 13).
|
||||||
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import UTC
|
from datetime import UTC, datetime
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query, Response, status
|
from fastapi import APIRouter, Depends, HTTPException, Query, Response, status
|
||||||
@@ -12,7 +16,7 @@ from sqlalchemy import select
|
|||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.core.db import get_db
|
from app.core.db import get_db
|
||||||
from app.deps import get_current_user, require_permission
|
from app.deps import get_current_user
|
||||||
from app.models.saved_filter import SavedFilter
|
from app.models.saved_filter import SavedFilter
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/v1/saved-filters", tags=["saved-filters"])
|
router = APIRouter(prefix="/api/v1/saved-filters", tags=["saved-filters"])
|
||||||
@@ -33,6 +37,19 @@ def _validate_entity_type(entity_type: str) -> None:
|
|||||||
})
|
})
|
||||||
|
|
||||||
|
|
||||||
|
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):
|
class SavedFilterCreate(BaseModel):
|
||||||
"""Schema for creating a saved filter."""
|
"""Schema for creating a saved filter."""
|
||||||
name: str = Field(..., min_length=1, max_length=100)
|
name: str = Field(..., min_length=1, max_length=100)
|
||||||
@@ -68,6 +85,9 @@ async def list_saved_filters(
|
|||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
|
if entity_type:
|
||||||
|
_check_entity_read(current_user, entity_type)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
query = select(SavedFilter).where(
|
query = select(SavedFilter).where(
|
||||||
SavedFilter.tenant_id == tenant_id,
|
SavedFilter.tenant_id == tenant_id,
|
||||||
@@ -85,7 +105,7 @@ async def list_saved_filters(
|
|||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
raise HTTPException(status_code=403, detail=str(e)) from e
|
||||||
|
|
||||||
|
|
||||||
@router.post("", status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_permission("contacts:read"))])
|
@router.post("", status_code=status.HTTP_201_CREATED)
|
||||||
async def create_saved_filter(
|
async def create_saved_filter(
|
||||||
body: SavedFilterCreate,
|
body: SavedFilterCreate,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
@@ -93,6 +113,7 @@ async def create_saved_filter(
|
|||||||
):
|
):
|
||||||
"""Create a new saved filter for the current user."""
|
"""Create a new saved filter for the current user."""
|
||||||
_validate_entity_type(body.entity_type)
|
_validate_entity_type(body.entity_type)
|
||||||
|
_check_entity_read(current_user, body.entity_type)
|
||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
@@ -124,7 +145,7 @@ async def create_saved_filter(
|
|||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
raise HTTPException(status_code=403, detail=str(e)) from e
|
||||||
|
|
||||||
|
|
||||||
@router.delete("/{filter_id}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_permission("contacts:read"))])
|
@router.delete("/{filter_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
async def delete_saved_filter(
|
async def delete_saved_filter(
|
||||||
filter_id: str,
|
filter_id: str,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
@@ -135,26 +156,23 @@ async def delete_saved_filter(
|
|||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
try:
|
try:
|
||||||
try:
|
fid = uuid.UUID(filter_id)
|
||||||
fid = uuid.UUID(filter_id)
|
except (ValueError, TypeError):
|
||||||
except (ValueError, TypeError):
|
raise HTTPException(400, detail={"detail": "Invalid filter_id", "code": "invalid_id"}) from None
|
||||||
raise HTTPException(400, detail={"detail": "Invalid filter_id", "code": "invalid_id"}) from None
|
|
||||||
|
|
||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
select(SavedFilter).where(
|
select(SavedFilter).where(
|
||||||
SavedFilter.id == fid,
|
SavedFilter.id == fid,
|
||||||
SavedFilter.tenant_id == tenant_id,
|
SavedFilter.tenant_id == tenant_id,
|
||||||
SavedFilter.user_id == user_id,
|
SavedFilter.user_id == user_id,
|
||||||
SavedFilter.deleted_at.is_(None),
|
SavedFilter.deleted_at.is_(None),
|
||||||
)
|
|
||||||
)
|
)
|
||||||
saved = result.scalar_one_or_none()
|
)
|
||||||
if saved is None:
|
saved = result.scalar_one_or_none()
|
||||||
raise HTTPException(404, detail={"detail": "Saved filter not found", "code": "not_found"})
|
if saved is None:
|
||||||
|
raise HTTPException(404, detail={"detail": "Saved filter not found", "code": "not_found"})
|
||||||
|
|
||||||
from datetime import datetime
|
_check_entity_read(current_user, saved.entity_type)
|
||||||
saved.deleted_at = datetime.now(UTC)
|
saved.deleted_at = datetime.now(UTC)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
return Response(status_code=status.HTTP_204_NO_CONTENT)
|
return Response(status_code=status.HTTP_204_NO_CONTENT)
|
||||||
except PermissionError as e:
|
|
||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
|
||||||
|
|||||||
+46
-27
@@ -1,18 +1,22 @@
|
|||||||
"""Saved views routes — CRUD for reusable view configurations."""
|
"""Saved views routes — CRUD for reusable view configurations.
|
||||||
|
|
||||||
|
W4b: Permissions are derived from the entity type's owning plugin instead
|
||||||
|
of a hardcoded ``contacts:read`` (Spec #357/Kritikpunkt 13).
|
||||||
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import UTC
|
from datetime import UTC, datetime
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query, Response, status
|
from fastapi import APIRouter, Depends, HTTPException, Query, status
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.core.db import get_db
|
from app.core.db import get_db
|
||||||
from app.deps import get_current_user, require_permission
|
from app.deps import get_current_user
|
||||||
from app.models.saved_view import SavedView
|
from app.models.saved_view import SavedView
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/v1/saved-views", tags=["saved-views"])
|
router = APIRouter(prefix="/api/v1/saved-views", tags=["saved-views"])
|
||||||
@@ -33,6 +37,19 @@ def _validate_entity_type(entity_type: str) -> None:
|
|||||||
})
|
})
|
||||||
|
|
||||||
|
|
||||||
|
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):
|
class SavedViewCreate(BaseModel):
|
||||||
"""Schema for creating a saved view."""
|
"""Schema for creating a saved view."""
|
||||||
name: str = Field(..., min_length=1, max_length=100)
|
name: str = Field(..., min_length=1, max_length=100)
|
||||||
@@ -68,6 +85,9 @@ async def list_saved_views(
|
|||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
|
if entity_type:
|
||||||
|
_check_entity_read(current_user, entity_type)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
query = select(SavedView).where(
|
query = select(SavedView).where(
|
||||||
SavedView.tenant_id == tenant_id,
|
SavedView.tenant_id == tenant_id,
|
||||||
@@ -85,7 +105,7 @@ async def list_saved_views(
|
|||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
raise HTTPException(status_code=403, detail=str(e)) from e
|
||||||
|
|
||||||
|
|
||||||
@router.post("", status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_permission("contacts:read"))])
|
@router.post("", status_code=status.HTTP_201_CREATED)
|
||||||
async def create_saved_view(
|
async def create_saved_view(
|
||||||
body: SavedViewCreate,
|
body: SavedViewCreate,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
@@ -93,6 +113,7 @@ async def create_saved_view(
|
|||||||
):
|
):
|
||||||
"""Create a new saved view for the current user."""
|
"""Create a new saved view for the current user."""
|
||||||
_validate_entity_type(body.entity_type)
|
_validate_entity_type(body.entity_type)
|
||||||
|
_check_entity_read(current_user, body.entity_type)
|
||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
@@ -124,7 +145,7 @@ async def create_saved_view(
|
|||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
raise HTTPException(status_code=403, detail=str(e)) from e
|
||||||
|
|
||||||
|
|
||||||
@router.put("/{view_id}", dependencies=[Depends(require_permission("contacts:read"))])
|
@router.put("/{view_id}")
|
||||||
async def update_saved_view(
|
async def update_saved_view(
|
||||||
view_id: str,
|
view_id: str,
|
||||||
body: SavedViewUpdate,
|
body: SavedViewUpdate,
|
||||||
@@ -153,6 +174,8 @@ async def update_saved_view(
|
|||||||
if saved is None:
|
if saved is None:
|
||||||
raise HTTPException(404, detail={"detail": "Saved view not found", "code": "not_found"})
|
raise HTTPException(404, detail={"detail": "Saved view not found", "code": "not_found"})
|
||||||
|
|
||||||
|
_check_entity_read(current_user, saved.entity_type)
|
||||||
|
|
||||||
if body.name is not None:
|
if body.name is not None:
|
||||||
saved.name = body.name
|
saved.name = body.name
|
||||||
if body.view_config is not None:
|
if body.view_config is not None:
|
||||||
@@ -163,7 +186,7 @@ async def update_saved_view(
|
|||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
raise HTTPException(status_code=403, detail=str(e)) from e
|
||||||
|
|
||||||
|
|
||||||
@router.delete("/{view_id}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_permission("contacts:read"))])
|
@router.delete("/{view_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
async def delete_saved_view(
|
async def delete_saved_view(
|
||||||
view_id: str,
|
view_id: str,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
@@ -174,26 +197,22 @@ async def delete_saved_view(
|
|||||||
user_id = uuid.UUID(current_user["user_id"])
|
user_id = uuid.UUID(current_user["user_id"])
|
||||||
|
|
||||||
try:
|
try:
|
||||||
try:
|
vid = uuid.UUID(view_id)
|
||||||
vid = uuid.UUID(view_id)
|
except (ValueError, TypeError):
|
||||||
except (ValueError, TypeError):
|
raise HTTPException(400, detail={"detail": "Invalid view_id", "code": "invalid_id"}) from None
|
||||||
raise HTTPException(400, detail={"detail": "Invalid view_id", "code": "invalid_id"}) from None
|
|
||||||
|
|
||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
select(SavedView).where(
|
select(SavedView).where(
|
||||||
SavedView.id == vid,
|
SavedView.id == vid,
|
||||||
SavedView.tenant_id == tenant_id,
|
SavedView.tenant_id == tenant_id,
|
||||||
SavedView.user_id == user_id,
|
SavedView.user_id == user_id,
|
||||||
SavedView.deleted_at.is_(None),
|
SavedView.deleted_at.is_(None),
|
||||||
)
|
|
||||||
)
|
)
|
||||||
saved = result.scalar_one_or_none()
|
)
|
||||||
if saved is None:
|
saved = result.scalar_one_or_none()
|
||||||
raise HTTPException(404, detail={"detail": "Saved view not found", "code": "not_found"})
|
if saved is None:
|
||||||
|
raise HTTPException(404, detail={"detail": "Saved view not found", "code": "not_found"})
|
||||||
|
|
||||||
from datetime import datetime
|
_check_entity_read(current_user, saved.entity_type)
|
||||||
saved.deleted_at = datetime.now(UTC)
|
saved.deleted_at = datetime.now(UTC)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
return Response(status_code=status.HTTP_204_NO_CONTENT)
|
|
||||||
except PermissionError as e:
|
|
||||||
raise HTTPException(status_code=403, detail=str(e)) from e
|
|
||||||
|
|||||||
@@ -69,6 +69,37 @@ ENTITY_MODELS: dict[str, type] = {
|
|||||||
"contact_folder": ContactFolder,
|
"contact_folder": ContactFolder,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# W4b: Tracks which plugin registered which entity_type — used to derive
|
||||||
|
# the correct module permission (e.g. contacts:read for contacts entities).
|
||||||
|
ENTITY_PLUGIN_OWNERS: dict[str, str] = {}
|
||||||
|
|
||||||
|
|
||||||
|
def get_entity_read_permission(entity_type: str) -> str:
|
||||||
|
"""Derive the module read permission for an entity type.
|
||||||
|
|
||||||
|
W4b: Saved views/filters must respect the owning plugin's permission
|
||||||
|
instead of a hardcoded contacts:read. Falls back to contacts:read for
|
||||||
|
unknown entities (backward compat, pre-plugin behavior).
|
||||||
|
"""
|
||||||
|
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"
|
||||||
|
|
||||||
|
|
||||||
|
def _core_module_keys(module: str, action: str) -> list[str]:
|
||||||
|
"""Find a core permission key matching module+action (lazy import safe)."""
|
||||||
|
from app.core.permission_registry import CORE_PERMISSIONS
|
||||||
|
|
||||||
|
return [
|
||||||
|
p["key"]
|
||||||
|
for p in CORE_PERMISSIONS
|
||||||
|
if p.get("module") == module and p["key"].endswith(f":{action}")
|
||||||
|
]
|
||||||
|
|
||||||
# Core models with OwnedMixin (Phase 2 additions)
|
# Core models with OwnedMixin (Phase 2 additions)
|
||||||
try:
|
try:
|
||||||
from app.models.entity_attachment import EntityAttachment
|
from app.models.entity_attachment import EntityAttachment
|
||||||
@@ -85,9 +116,15 @@ except ImportError:
|
|||||||
# at activation time in main.py:lifespan(). No hardcoded plugin imports here.
|
# at activation time in main.py:lifespan(). No hardcoded plugin imports here.
|
||||||
|
|
||||||
|
|
||||||
def register_entity_model(entity_type: str, model_class: type) -> None:
|
def register_entity_model(
|
||||||
|
entity_type: str,
|
||||||
|
model_class: type,
|
||||||
|
plugin_name: str | None = None,
|
||||||
|
) -> None:
|
||||||
"""Register an entity model dynamically (called during plugin activation)."""
|
"""Register an entity model dynamically (called during plugin activation)."""
|
||||||
ENTITY_MODELS[entity_type] = model_class
|
ENTITY_MODELS[entity_type] = model_class
|
||||||
|
if plugin_name:
|
||||||
|
ENTITY_PLUGIN_OWNERS[entity_type] = plugin_name
|
||||||
|
|
||||||
|
|
||||||
def unregister_entity_model(entity_type: str) -> None:
|
def unregister_entity_model(entity_type: str) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user