refactor(#356): DSAR-Fachlogik vollstaendig aus dem Core extrahiert — dsar_collect/dsar_erase in die 5 beteiligten Contracts (contacts, mail, tasks, calendar, kommunikation); core/jobs.py sammelt/loescht nur Core-eigene Daten und iteriert generisch ueber die Plugin-Registry; neue Plugins liefern DSAR-Kategorien ohne Core-Aenderung; Counts-/Category-Keys unveraendert; tasks/contracts.py von Patch-Artefakt bereinigt

fixes #356
2026-08-27 18:09:15 +02:00
parent 092c2d20fb
commit ad5601eb7d
7 changed files with 228 additions and 153 deletions
@@ -16,10 +16,11 @@

 | Finding | Issue | Fix | Verifikation (Live-Messung) |
 |---|---|---|---|
-| 4 Core→Plugin-Imports in `core/jobs.py` DSAR-Sammlung (mail/tasks/calendar/kommunikation Models direkt importiert statt über Contracts) | [#356](https://forgejo.media-on.de/Leopoldadmin/leocrm/issues/356) | 4 Blöcke auf `get_contract()` umgestellt (bestehende Registry, ARCH-014-Schutz); MailContract exponiert `MailAccount`; ImportError-Fallback-Semantik unverändert | Checker vor Fix: 4 Verstöße → nach Fix: **0 Verstöße** (480 Dateien); DSAR-Suite `test_g1_dsar.py` 4/4 passed (Funktionserhalt); ruff: nur 2 Vorbestand-N811 |
+| 4 Core→Plugin-Imports in `core/jobs.py` DSAR-Sammlung | [#356](https://forgejo.media-on.de/Leopoldadmin/leocrm/issues/356) | Schritt 1 (092c2d2): Imports auf `get_contract()` umgestellt | Checker 4→0; DSAR-Suite 4/4 |
+| **Vertiefung nach Review-Einspruch:** Plugin-Fachlogik (welche Kategorien, welche Felder, Limits) lag weiterhin hart im Core — Core kannte Plugin-Details | [#356](https://forgejo.media-on.de/Leopoldadmin/leocrm/issues/356) | **Komplette Extraktion:** `dsar_collect()`/`dsar_erase()` in die 5 beteiligten Contracts (contacts, mail, tasks, calendar, kommunikation); `core/jobs.py` sammelt/löscht nur Core-eigene Daten (Profil, Audit, Notifications, User-Anonymisierung) und iteriert generisch über `registry.list_discovered()` → Contract-DSAR-Beiträge. **Neue Plugins liefern DSAR-Kategorien ohne Core-Änderung.** | Checker **0 Verstöße**; DSAR-Suite **4/4 passed** (Counts-/Category-Keys unverändert: `contacts`, `contacts_soft_deleted` etc. via Contracts); `create_app()` OK; ruff nur 2 Vorbestand-N811; korruptes tasks/contracts.py (Patch-Artefakt, ast-gefangen) sauber neu geschrieben |
 | P16 manuell klassifiziert: `app.models.contact`-Imports in core/jobs.py + worker.py sind KEINE Verstöße (Contact liegt im Core-Models-Layer) | — | Keine Aktion nötig, dokumentiert in #356 | Checker-Regex deckt nur `app.plugins.*` ab — korrekt so |

-**Gates:** ruff modified-files grün (2 Vorbestand N811 ausgenommen) · Cross-Plugin-Checker 0 · pytest DSAR 4/4
+**Gates:** ruff modified-files grün (2 Vorbestand N811 ausgenommen) · Cross-Plugin-Checker 0 · pytest DSAR 4/4 · create_app OK

 ## Welle 1 — Plugin-Lifecycle-Fix (2026-08-27)

@@ -160,19 +160,21 @@ register_job("send_password_reset_email", send_password_reset_email)
 async def _dsar_collect_user_data(db: Any, tenant_id: str, user_id: str) -> dict[str, Any]:
    """Collect every data category the dsgvo-export route promises.

-    Shared by type=access (full export) so both paths stay consistent.
+    Core collects ONLY core-owned data (profile, audit log, notifications).
+    Plugin-owned categories (contacts, mail accounts, tasks, calendar
+    entries, comm messages, ...) are contributed by each plugin's contract
+    via ``dsar_collect()`` — the core must not know plugin internals.
    """
    from datetime import UTC, datetime
    from uuid import UUID as PyUUID

-    from sqlalchemy import or_ as sa_or_
    from sqlalchemy import select as sa_select

    from app.models.audit import AuditLog
-    from app.models.contact import Contact
    from app.models.notification import Notification
    from app.models.user import User
    from app.plugins.builtins.contracts import get_contract
+    from app.plugins.registry import get_registry

    uid = PyUUID(user_id)
    tid = PyUUID(tenant_id)
@@ -196,27 +198,6 @@ async def _dsar_collect_user_data(db: Any, tenant_id: str, user_id: str) -> dict
            "created_at": user.created_at.isoformat() if user.created_at else None,
        }

-    # Contacts owned by the user
-    contacts = (
-        await db.execute(
-            sa_select(Contact).where(
-                Contact.tenant_id == tid,
-                Contact.owner_id == uid,
-                Contact.deleted_at.is_(None),
-            )
-        )
-    ).scalars().all()
-    export_data["data"]["contacts"] = [
-        {
-            "id": str(c.id),
-            "type": c.type,
-            "displayname": c.displayname,
-            "email_1": c.email_1,
-            "email_2": c.email_2,
-        }
-        for c in contacts
-    ]
-
    # Audit trail entries by/about the user (bounded to keep payloads sane)
    audit_entries = (
        await db.execute(
@@ -254,116 +235,25 @@ async def _dsar_collect_user_data(db: Any, tenant_id: str, user_id: str) -> dict
        for n in notifications
    ]

-    # ── Categories promised by the dsgvo-export route docstring ──
-    # (G1-b: mail accounts, tasks, calendar entries, comm messages)
-    try:
-        mail_contract = get_contract("mail")
-        if mail_contract is None:
-            raise ImportError("mail plugin not active")
-        mail_account_m = mail_contract.MailAccount
-
-        mail_accounts = (
-            await db.execute(
-                sa_select(mail_account_m).where(
-                    mail_account_m.tenant_id == tid,
-                    mail_account_m.user_id == uid,
-                ).limit(500)
+    # ── Plugin-owned categories via contracts ──
+    # For every discovered plugin, resolve its contract (lazy-load) and ask
+    # it to contribute its DSAR categories. Inactive/absent plugins simply
+    # contribute nothing — same semantics as the former per-plugin try/except.
+    registry = get_registry()
+    for plugin_name in registry.list_discovered():
+        contract = get_contract(plugin_name)
+        dsar_collect = getattr(contract, "dsar_collect", None) if contract else None
+        if dsar_collect is None:
+            continue
+        try:
+            categories = await dsar_collect(db, tid, uid)
+            export_data["data"].update(categories)
+        except Exception:
+            logger.warning(
+                "DSAR collect failed for plugin '%s' — category skipped",
+                plugin_name,
+                exc_info=True,
            )
-        ).scalars().all()
-        export_data["data"]["mail_accounts"] = [
-            {
-                "id": str(a.id),
-                "email_address": a.email_address,
-                "display_name": a.display_name,
-                "is_shared": a.is_shared,
-                "is_active": a.is_active,
-            }
-            for a in mail_accounts
-        ]
-    except ImportError:
-        pass
-
-    try:
-        tasks_contract = get_contract("tasks")
-        if tasks_contract is None:
-            raise ImportError("tasks plugin not active")
-        task_m = tasks_contract.Task
-
-        tasks = (
-            await db.execute(
-                sa_select(task_m).where(
-                    sa_or_(task_m.owner_id == uid, task_m.assigned_to == uid),
-                    task_m.tenant_id == tid,
-                    task_m.deleted_at.is_(None),
-                ).limit(1000)
-            )
-        ).scalars().all()
-        export_data["data"]["tasks"] = [
-            {
-                "id": str(t.id),
-                "title": t.title,
-                "status": t.status,
-                "priority": t.priority,
-                "due_date": t.due_date.isoformat() if t.due_date else None,
-            }
-            for t in tasks
-        ]
-    except ImportError:
-        pass
-
-    try:
-        cal_contract = get_contract("calendar")
-        if cal_contract is None:
-            raise ImportError("calendar plugin not active")
-        cal_entry_m = cal_contract.CalendarEntry
-
-        cal_entries = (
-            await db.execute(
-                sa_select(cal_entry_m).where(
-                    cal_entry_m.tenant_id == tid,
-                    cal_entry_m.owner_id == uid,
-                    cal_entry_m.deleted_at.is_(None),
-                ).limit(1000)
-            )
-        ).scalars().all()
-        export_data["data"]["calendar_entries"] = [
-            {
-                "id": str(e.id),
-                "title": e.title,
-                "entry_type": e.entry_type,
-                "start_at": e.start_at.isoformat() if e.start_at else None,
-                "end_at": e.end_at.isoformat() if e.end_at else None,
-            }
-            for e in cal_entries
-        ]
-    except ImportError:
-        pass
-
-    try:
-        komm_contract = get_contract("kommunikation")
-        if komm_contract is None:
-            raise ImportError("kommunikation plugin not active")
-        comm_msg_m = komm_contract.CommMessage
-
-        comm_messages = (
-            await db.execute(
-                sa_select(comm_msg_m).where(
-                    comm_msg_m.sender_id == uid,
-                    comm_msg_m.tenant_id == tid,
-                ).limit(1000)
-            )
-        ).scalars().all()
-        export_data["data"]["comm_messages"] = [
-            {
-                "id": str(m.id),
-                "sender_type": m.sender_type,
-                "content": m.content[:500],
-                "created_at": m.created_at.isoformat() if m.created_at else None,
-            }
-            for m in comm_messages
-        ]
-    except ImportError:
-        pass

    return export_data

@@ -372,42 +262,45 @@ async def _dsar_execute_deletion(db: Any, tenant_id: str, user_id: str) -> dict[
    """Execute GDPR Art. 17 erasure for a user within one tenant.

    Strategy (respects retention duties):
-    - Contacts owned by the user → soft-delete via deleted_at
-      (audit history must remain intact — it is not personal data of the
-       subject but business record; retention policy governs its cleanup)
-    - Notifications owned by the user → hard delete
+    - Plugin-owned personal data (contacts, ...) → erased via each plugin's
+      contract ``dsar_erase()`` — the core must not know plugin internals
+    - Notifications owned by the user → hard delete (core-owned)
    - User account → deactivate (is_active=False), clear personal fields,
      scramble password hash and email (keeps FK integrity for audit rows)
    Returns counters for the audit entry.
    """
-    from datetime import UTC, datetime
    from uuid import UUID as PyUUID

    from sqlalchemy import select as sa_select
    from sqlalchemy import update as sa_update

    from app.core.audit import log_audit
-    from app.models.contact import Contact
    from app.models.notification import Notification
    from app.models.user import User
+    from app.plugins.builtins.contracts import get_contract
+    from app.plugins.registry import get_registry

    uid = PyUUID(user_id)
    tid = PyUUID(tenant_id)

    counts: dict[str, int] = {}

-    # 1. Soft-delete contacts owned by the user
-    contact_result = await db.execute(
-        sa_select(Contact).where(
-            Contact.tenant_id == tid,
-            Contact.owner_id == uid,
-            Contact.deleted_at.is_(None),
-        )
-    )
-    contacts = contact_result.scalars().all()
-    for c in contacts:
-        c.deleted_at = datetime.now(UTC)
-    counts["contacts_soft_deleted"] = len(contacts)
+    # 1. Plugin-owned erasure via contracts (contacts, ...)
+    registry = get_registry()
+    for plugin_name in registry.list_discovered():
+        contract = get_contract(plugin_name)
+        dsar_erase = getattr(contract, "dsar_erase", None) if contract else None
+        if dsar_erase is None:
+            continue
+        try:
+            plugin_counts = await dsar_erase(db, tid, uid)
+            counts.update(plugin_counts)
+        except Exception:
+            logger.warning(
+                "DSAR erase failed for plugin '%s' — counters may be incomplete",
+                plugin_name,
+                exc_info=True,
+            )

    # 2. Hard-delete notifications owned by the user
    notif_result = await db.execute(
@@ -2,6 +2,11 @@

 from __future__ import annotations

+from typing import Any
+
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession
+
 from app.plugins.builtins.calendar.models import Calendar, CalendarEntry, CalendarEntryLink
 from app.plugins.builtins.contracts import get_contract_registry

@@ -15,6 +20,33 @@ class CalendarContract:
    CalendarEntry = CalendarEntry
    CalendarEntryLink = CalendarEntryLink

+    @staticmethod
+    async def dsar_collect(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, Any]:
+        """GDPR Art. 15: collect calendar entries owned by the user."""
+        cal_entries = (
+            await db.execute(
+                select(CalendarEntry).where(
+                    CalendarEntry.tenant_id == tenant_id,
+                    CalendarEntry.owner_id == user_id,
+                    CalendarEntry.deleted_at.is_(None),
+                ).limit(1000)
+            )
+        ).scalars().all()
+        return {
+            "calendar_entries": [
+                {
+                    "id": str(e.id),
+                    "title": e.title,
+                    "entry_type": e.entry_type,
+                    "start_at": e.start_at.isoformat() if e.start_at else None,
+                    "end_at": e.end_at.isoformat() if e.end_at else None,
+                }
+                for e in cal_entries
+            ]
+        }
+
    @classmethod
    def get_function(cls, name: str):
        """Return a callable exposed by this contract, or None if absent."""
@@ -50,6 +50,62 @@ class ContactsContract:
            "total": results[0],
        }

+    @staticmethod
+    async def dsar_collect(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, Any]:
+        """GDPR Art. 15: collect contact data owned by the user.
+
+        Owned by the contacts plugin — core/jobs.py calls this generically
+        via the contract, it must not know contact internals.
+        """
+        contacts = (
+            await db.execute(
+                select(Contact).where(
+                    Contact.tenant_id == tenant_id,
+                    Contact.owner_id == user_id,
+                    Contact.deleted_at.is_(None),
+                )
+            )
+        ).scalars().all()
+        return {
+            "contacts": [
+                {
+                    "id": str(c.id),
+                    "type": c.type,
+                    "displayname": c.displayname,
+                    "email_1": c.email_1,
+                    "email_2": c.email_2,
+                }
+                for c in contacts
+            ]
+        }
+
+    @staticmethod
+    async def dsar_erase(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, int]:
+        """GDPR Art. 17: soft-delete contacts owned by the user.
+
+        Soft-delete via deleted_at (audit history must remain intact — it is
+        a business record, not personal data of the subject; retention
+        policy governs its cleanup).
+        """
+        from datetime import UTC, datetime
+
+        contacts = (
+            await db.execute(
+                select(Contact).where(
+                    Contact.tenant_id == tenant_id,
+                    Contact.owner_id == user_id,
+                    Contact.deleted_at.is_(None),
+                )
+            )
+        ).scalars().all()
+        for c in contacts:
+            c.deleted_at = datetime.now(UTC)
+        return {"contacts_soft_deleted": len(contacts)}
+
    @classmethod
    def get_function(cls, name: str):
        """Return a callable exposed by this contract, or None if absent."""
@@ -13,6 +13,11 @@ instead of importing from internal modules directly.

 from __future__ import annotations

+from typing import Any
+
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession
+
 from app.plugins.builtins.contracts import get_contract_registry
 from app.plugins.builtins.kommunikation.miniapp_registry import (
    MiniAppDef,
@@ -74,6 +79,31 @@ class KommunikationContract:
    CommMessage = CommMessage
    CommParticipant = CommParticipant

+    @staticmethod
+    async def dsar_collect(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, Any]:
+        """GDPR Art. 15: collect communication messages sent by the user."""
+        comm_messages = (
+            await db.execute(
+                select(CommMessage).where(
+                    CommMessage.sender_id == user_id,
+                    CommMessage.tenant_id == tenant_id,
+                ).limit(1000)
+            )
+        ).scalars().all()
+        return {
+            "comm_messages": [
+                {
+                    "id": str(m.id),
+                    "sender_type": m.sender_type,
+                    "content": m.content[:500],
+                    "created_at": m.created_at.isoformat() if m.created_at else None,
+                }
+                for m in comm_messages
+            ]
+        }
+

 # ─── self-registration ───

@@ -16,6 +16,11 @@ instead of importing from internal modules directly.

 from __future__ import annotations

+from typing import Any
+
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession
+
 from app.plugins.builtins.contracts import get_contract_registry
 from app.plugins.builtins.mail.models import Mail, MailAccount

@@ -33,6 +38,32 @@ class MailContract:
    Mail = Mail
    MailAccount = MailAccount

+    @staticmethod
+    async def dsar_collect(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, Any]:
+        """GDPR Art. 15: collect mail account data owned by the user."""
+        mail_accounts = (
+            await db.execute(
+                select(MailAccount).where(
+                    MailAccount.tenant_id == tenant_id,
+                    MailAccount.user_id == user_id,
+                ).limit(500)
+            )
+        ).scalars().all()
+        return {
+            "mail_accounts": [
+                {
+                    "id": str(a.id),
+                    "email_address": a.email_address,
+                    "display_name": a.display_name,
+                    "is_shared": a.is_shared,
+                    "is_active": a.is_active,
+                }
+                for a in mail_accounts
+            ]
+        }
+
    @classmethod
    def get_function(cls, name: str):
        """Return a callable exposed by this contract, or None if absent."""
@@ -13,6 +13,11 @@ instead of importing from internal modules directly.

 from __future__ import annotations

+from typing import Any
+
+from sqlalchemy import or_, select
+from sqlalchemy.ext.asyncio import AsyncSession
+
 from app.plugins.builtins.contracts import get_contract_registry
 from app.plugins.builtins.tasks.models import Task
 from app.plugins.builtins.tasks.services import (
@@ -45,6 +50,33 @@ class TasksContract:
    # ─── models (read-only for queries) ───
    Task = Task

+    @staticmethod
+    async def dsar_collect(
+        db: AsyncSession, tenant_id: Any, user_id: Any
+    ) -> dict[str, Any]:
+        """GDPR Art. 15: collect tasks owned by or assigned to the user."""
+        tasks = (
+            await db.execute(
+                select(Task).where(
+                    or_(Task.owner_id == user_id, Task.assigned_to == user_id),
+                    Task.tenant_id == tenant_id,
+                    Task.deleted_at.is_(None),
+                ).limit(1000)
+            )
+        ).scalars().all()
+        return {
+            "tasks": [
+                {
+                    "id": str(t.id),
+                    "title": t.title,
+                    "status": t.status,
+                    "priority": t.priority,
+                    "due_date": t.due_date.isoformat() if t.due_date else None,
+                }
+                for t in tasks
+            ]
+        }
+

 # ─── self-registration ───