fix(arch): externes Audit — 13 Backend-Fixes (Workspace-Modules, Tenant-Manifeste, Lifecycle, Contracts, Permissions)
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
Verifikation: Alle 17 Audit-Findings gegen den Code geprueft — alle bestaetigt. Backend-Lifecycle-Fixes umgesetzt; 4 Frontend-Plugin-Architektur-Punkte als Phase Q in die Roadmap eingeplant. - P1 list_workspaces: Module + User-Counts gebuendelt laden (Editor-Overwrite-Bug) - P1 active-manifests: Tenant-Deaktivierung (tenant_plugin_activation) filtern - P1 uninstall: volle Service-Deactivation VOR registry.uninstall() - P1 ContractRegistry: DB-Aktivstatus-Guard (Restart-Edge-Case) + Re-Activate - P1/P2 Field-Definitions: voller Lifecycle (register/unregister) im Service - P1/P2 Contact-Felddefinitionen (39) ins ContactsPlugin-Manifest verschoben - P1 12 fehlende Permission-Keys registriert (AST-Scan: 0 fehlend) - P2 contact_folder -> ContactsPlugin; ENTITY_PLUGIN_OWNERS wird befuellt - P2 Entity-Permission-Fallback fail-closed statt contacts:read - P2 forgejo_error_reporter is_core=False; DMS is_core=True (ADR-020) - P2 Worker: Contacts-Trash-Cleanup ins Plugin (get_job_modules-Discovery) - P1/P2 DSGVO-Export delegiert an DSAR-Collector (kein Core->Contacts) - P2 False-green Tests korrigiert (or True, veraltete Route-Count-Assertion) Verifikation: tests/test_audit_architecture_fixes.py 17/17; Regressionen gruen (contacts_lifecycle, entity_registry, workspace_scopes, rbac, lifecycle_service); Combo-Order-Test 35/35; Cross-Plugin-Checker 497/0; compileall sauber; ruff auf 7-Error-Baseline. Doku: PROGRESS.md Audit-Section, PLATFORM_ROADMAP.md Phase Q (Q1-Q4), plugin-development-guide.md Lifecycle, permissions.md Katalog.
This commit is contained in:
@@ -0,0 +1,64 @@
|
||||
"""ARQ background jobs for the contacts plugin.
|
||||
|
||||
Registered via ``register_job()`` at import time; the worker discovers this
|
||||
module through ``ContactsPlugin.get_job_modules()`` — the core worker must
|
||||
not import contact models directly (audit P2: hidden core->contacts
|
||||
coupling in the trash cleanup).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
|
||||
from sqlalchemy import delete as sa_delete
|
||||
from sqlalchemy import text as sa_text
|
||||
|
||||
from app.core.job_registry import register_job
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_TRASH_RETENTION_DAYS = 90
|
||||
|
||||
|
||||
async def cleanup_contacts_trash_job(ctx: dict[str, Any]) -> None:
|
||||
"""Permanently delete soft-deleted contacts older than the retention window.
|
||||
|
||||
Runs daily. Iterates per-tenant for RLS compliance.
|
||||
Moved from app.core.worker.cleanup_trash_job (audit P2) so the core
|
||||
worker only handles core-owned entities (entity_attachments).
|
||||
"""
|
||||
from app.core.db import get_worker_session_factory
|
||||
from app.models.contact import Contact
|
||||
|
||||
factory = get_worker_session_factory()
|
||||
async with factory() as db:
|
||||
try:
|
||||
tenant_result = await db.execute(sa_text("SELECT id FROM tenants"))
|
||||
tenant_ids = [row[0] for row in tenant_result]
|
||||
|
||||
cutoff = datetime.now(UTC) - timedelta(days=_TRASH_RETENTION_DAYS)
|
||||
total_deleted = 0
|
||||
for tenant_id in tenant_ids:
|
||||
await db.execute(
|
||||
sa_text("SELECT set_config('app.current_tenant_id', :tid, true)"),
|
||||
{"tid": str(tenant_id)},
|
||||
)
|
||||
result = await db.execute(
|
||||
sa_delete(Contact).where(
|
||||
Contact.deleted_at.is_not(None),
|
||||
Contact.deleted_at < cutoff,
|
||||
)
|
||||
)
|
||||
total_deleted += result.rowcount
|
||||
await db.commit()
|
||||
|
||||
if total_deleted:
|
||||
logger.info("Contacts trash cleanup: permanently deleted %d old contacts", total_deleted)
|
||||
except Exception:
|
||||
logger.error("Contacts trash cleanup failed", exc_info=True)
|
||||
await db.rollback()
|
||||
|
||||
|
||||
register_job("cleanup_contacts_trash", cleanup_contacts_trash_job)
|
||||
@@ -10,6 +10,7 @@ import logging
|
||||
|
||||
from app.plugins.base import BasePlugin
|
||||
from app.plugins.manifest import (
|
||||
FieldDefinition,
|
||||
FrontendDashboardWidget,
|
||||
FrontendMenuItem,
|
||||
FrontendPageRoute,
|
||||
@@ -106,18 +107,70 @@ class ContactsPlugin(BasePlugin):
|
||||
"contacts:write",
|
||||
"contacts:delete",
|
||||
],
|
||||
# Audit P1/P2: contact field definitions are plugin-owned (moved
|
||||
# from CORE_FIELD_DEFINITIONS) — registered at activation time via
|
||||
# register_field_definitions() and removed on deactivation.
|
||||
field_definitions=[
|
||||
FieldDefinition(module="contacts", field="firstname", label="First Name", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="surname", label="Last Name", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="displayname", label="Display Name", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="name", label="Name", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="email_1", label="Email 1", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="email_2", label="Email 2", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="phone_1", label="Phone 1", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="phone_2", label="Phone 2", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="mobilephone", label="Mobile", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="function", label="Position", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="website", label="Website", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="status", label="Status", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="type", label="Type", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="gender", label="Gender", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="suffix", label="Suffix", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="ext_name_line", label="Extra Name Line", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="country", label="Country", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="code", label="Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="accounting_code", label="Accounting Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="vendor_accounting_code", label="Vendor Accounting Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="vat_code", label="VAT Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="fiscal_code", label="Fiscal Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="commerce_code", label="Commerce Code", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="purchase_number", label="Purchase Number", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="bic", label="BIC", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="mailing_street", label="Mailing Street", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="mailing_city", label="Mailing City", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="mailing_postalcode", label="Mailing Postal Code", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="mailing_country", label="Mailing Country", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="visit_street", label="Visit Street", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="visit_city", label="Visit City", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="visit_postalcode", label="Visit Postal Code", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="visit_country", label="Visit Country", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="invoice_street", label="Invoice Street", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="invoice_city", label="Invoice City", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="invoice_postalcode", label="Invoice Postal Code", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="invoice_country", label="Invoice Country", sensitivity="normal"),
|
||||
FieldDefinition(module="contacts", field="notes", label="Notes", sensitivity="sensitive"),
|
||||
FieldDefinition(module="contacts", field="tags", label="Tags", sensitivity="sensitive"),
|
||||
],
|
||||
is_core=True,
|
||||
author="LeoCRM Team",
|
||||
min_app_version="1.0.0",
|
||||
contract_version="1.0.0",
|
||||
)
|
||||
|
||||
def get_job_modules(self) -> list[str]:
|
||||
"""Worker discovers the contacts trash-cleanup job here (audit P2)."""
|
||||
return ["app.plugins.builtins.contacts.jobs"]
|
||||
|
||||
def get_entity_models(self) -> dict[str, type]:
|
||||
from app.models.contact import Contact
|
||||
from app.models.contact_folder import ContactFolder
|
||||
return {
|
||||
"contact": Contact,
|
||||
"contacts": Contact,
|
||||
"company": Contact,
|
||||
# Audit P2: contact_folder is contacts-plugin-owned domain data
|
||||
# (moved from the static core ENTITY_MODELS map).
|
||||
"contact_folder": ContactFolder,
|
||||
}
|
||||
|
||||
async def on_activate(self, db, service_container, event_bus) -> None:
|
||||
|
||||
@@ -60,6 +60,9 @@ class ContractRegistry:
|
||||
cls._instance._contracts: dict[str, Any] = {}
|
||||
cls._instance._loaded: set[str] = set()
|
||||
cls._instance._unregistered: set[str] = set()
|
||||
# Plugins whose DB record says active=False (audit restart edge
|
||||
# case) — marked once at API startup, see main.py lifespan.
|
||||
cls._instance._db_inactive: set[str] = set()
|
||||
return cls._instance
|
||||
|
||||
# ─── registration ───
|
||||
@@ -92,20 +95,51 @@ class ContractRegistry:
|
||||
|
||||
On first access the registry attempts to lazy-load the plugin's
|
||||
``contracts`` module, which will register itself on import.
|
||||
"""
|
||||
if plugin_name in self._contracts:
|
||||
return self._contracts[plugin_name]
|
||||
|
||||
Audit P1 (contract lazy loading): the DB activation state is checked
|
||||
BEFORE serving or lazy-loading. A plugin that was already inactive
|
||||
when the process started never lands in ``_unregistered`` (it was
|
||||
never deactivated at runtime), so the old guard alone let the lazy
|
||||
loader import its contracts module and resurrect the contract.
|
||||
The permission registry mirrors ``PluginModel.active`` at startup,
|
||||
so an inactive plugin fails closed here. When the permission
|
||||
registry is NOT initialized (worker process, early bootstrap)
|
||||
the legacy lazy-load behaviour is kept.
|
||||
"""
|
||||
# Explicitly unregistered (deactivated): never resurrect via
|
||||
# lazy-loading (ARCH-014) — the deactivated contract must stay gone.
|
||||
if plugin_name in self._unregistered:
|
||||
return None
|
||||
|
||||
# DB activation guard (audit restart edge case): plugins whose DB
|
||||
# record was already inactive when the process started never land in
|
||||
# _unregistered (they were never deactivated at runtime), so lazy
|
||||
# loading could resurrect their contracts. main.py marks them once
|
||||
# at startup; activation clears the marker again.
|
||||
if plugin_name in self._db_inactive:
|
||||
return None
|
||||
|
||||
if plugin_name in self._contracts:
|
||||
return self._contracts[plugin_name]
|
||||
|
||||
if plugin_name not in self._loaded:
|
||||
self._try_lazy_load(plugin_name)
|
||||
|
||||
return self._contracts.get(plugin_name)
|
||||
|
||||
def mark_db_inactive(self, plugin_names: set[str]) -> None:
|
||||
"""Mark plugins as DB-inactive (startup, audit restart edge case).
|
||||
|
||||
Called once from main.py lifespan with the names of plugins whose DB
|
||||
record has active=False. get_contract() fails closed for these.
|
||||
"""
|
||||
self._db_inactive.update(plugin_names)
|
||||
|
||||
def mark_plugin_active(self, plugin_name: str) -> None:
|
||||
"""Clear inactive markers (plugin activated/reinstalled at runtime)."""
|
||||
self._db_inactive.discard(plugin_name)
|
||||
self._unregistered.discard(plugin_name)
|
||||
|
||||
def require_contract(self, plugin_name: str) -> Any:
|
||||
"""Like :meth:`get_contract` but raise if unavailable."""
|
||||
contract = self.get_contract(plugin_name)
|
||||
@@ -148,6 +182,7 @@ class ContractRegistry:
|
||||
"""Clear all state — for unit tests only."""
|
||||
self._contracts.clear()
|
||||
self._loaded.clear()
|
||||
self._db_inactive.clear()
|
||||
|
||||
|
||||
# ─── module-level helpers ───
|
||||
|
||||
@@ -20,6 +20,11 @@ class DmsPlugin(BasePlugin):
|
||||
version="1.0.0",
|
||||
display_name="DMS",
|
||||
description="Document management: folder hierarchy, file upload, PDF preview, Collabora edit sessions, internal sharing, search, bulk ops.",
|
||||
# Audit P1/P2 (ADR-020): DMS is a platform core plugin — the core schema
|
||||
# (entity_attachments.files-FK) builds on the DMS files table, so DMS
|
||||
# cannot be deactivated. Declared is_core=True so the registry enforces
|
||||
# this instead of the FK being silently invalid.
|
||||
is_core=True,
|
||||
dependencies=["permissions"],
|
||||
routes=[
|
||||
PluginRouteDef(
|
||||
|
||||
@@ -23,7 +23,9 @@ class ForgejoErrorReporterPlugin(BasePlugin):
|
||||
version="1.0.0",
|
||||
display_name="Forgejo Error Reporter",
|
||||
description="Automatically reports errors to Forgejo as issues. Test environment only.",
|
||||
is_core=True,
|
||||
# Audit P2 (classification): a test/staging-only plugin must be
|
||||
# deactivatable — is_core=True contradicts its own production guard.
|
||||
is_core=False,
|
||||
dependencies=[],
|
||||
events=[],
|
||||
migrations=[],
|
||||
@@ -33,7 +35,8 @@ class ForgejoErrorReporterPlugin(BasePlugin):
|
||||
path="/api/v1/forgejo-error-reporter",
|
||||
module="app.plugins.builtins.forgejo_error_reporter.routes",
|
||||
router_attr="router",
|
||||
),
|
||||
permissions=["system:read"],
|
||||
),
|
||||
],
|
||||
|
||||
author="LeoCRM Team",
|
||||
|
||||
@@ -30,7 +30,12 @@ class PermissionsPlugin(BasePlugin):
|
||||
],
|
||||
events=[],
|
||||
migrations=["0001_initial.sql"],
|
||||
permissions=[],
|
||||
# Audit P1 (permission catalog): routes and settings pages use
|
||||
# permissions:admin / permissions:read — they must be grantable.
|
||||
permissions=[
|
||||
"permissions:read",
|
||||
"permissions:admin",
|
||||
],
|
||||
is_core=True,
|
||||
settings_pages=[
|
||||
FrontendSettingsPage(path='roles', label_key='settings.roles', label='Roles', component='@/pages/SettingsRoles', icon='Shield', order=10, permission='permissions:read'),
|
||||
|
||||
+26
-1
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import logging
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
@@ -852,19 +853,43 @@ class PluginRegistry:
|
||||
|
||||
# ── Active UI Manifests (Phase 3) ──
|
||||
|
||||
async def get_active_manifests(self, db: AsyncSession) -> list[dict[str, Any]]:
|
||||
async def get_active_manifests(
|
||||
self, db: AsyncSession, tenant_id: uuid.UUID | None = None
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Return UI manifests for all active plugins.
|
||||
|
||||
Each entry contains the plugin name and its frontend UI contributions
|
||||
(menu_items, page_routes, detail_tabs, settings_pages, dashboard_widgets).
|
||||
|
||||
Audit P1 (tenant manifests): when *tenant_id* is given, plugins that are
|
||||
deactivated for that tenant (tenant_plugin_activation.is_active=False)
|
||||
are excluded — the manifest output must mirror require_active_plugin()
|
||||
semantics so the UI never offers menus/routes the backend then blocks
|
||||
with 403. No tenant row = default active (same as the API gate).
|
||||
"""
|
||||
result = await db.execute(select(PluginModel).where(PluginModel.active.is_(True)))
|
||||
active_records = {row.name: row for row in result.scalars().all()}
|
||||
|
||||
# Per-tenant deactivations (same table/semantics as deps.require_active_plugin)
|
||||
tenant_disabled: set[str] = set()
|
||||
if tenant_id is not None:
|
||||
from sqlalchemy import text as sa_text
|
||||
|
||||
rows = await db.execute(
|
||||
sa_text(
|
||||
"SELECT plugin_name FROM tenant_plugin_activation "
|
||||
"WHERE tenant_id = :tid AND is_active = false"
|
||||
),
|
||||
{"tid": tenant_id},
|
||||
)
|
||||
tenant_disabled = {row[0] for row in rows}
|
||||
|
||||
manifests: list[dict[str, Any]] = []
|
||||
for name, plugin in self._plugins.items():
|
||||
if name not in active_records:
|
||||
continue
|
||||
if name in tenant_disabled:
|
||||
continue
|
||||
m = plugin.manifest
|
||||
manifests.append(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user