From 8da803156e6a3bfcad5a566e97066b3bf60744b9 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Wed, 29 Jul 2026 16:36:51 +0200 Subject: [PATCH] phase1: RLS simplified to tenant isolation only + canAccess fallback removed + useUserPermissions hook + security kernel docs --- .../0069_rls_tenant_isolation_only.py | 103 ++++++++++++++++++ docs/security_kernel.md | 74 +++++++++++++ .../src/components/contacts/ContactDetail.tsx | 3 - frontend/src/components/layout/Sidebar.tsx | 5 +- frontend/src/components/layout/TopBar.tsx | 4 +- frontend/src/hooks/useAuth.ts | 4 + frontend/src/hooks/useUserPermissions.ts | 39 +++++++ frontend/src/pages/ContactDetailPage.tsx | 3 - frontend/src/pages/ContactsList.tsx | 3 - 9 files changed, 222 insertions(+), 16 deletions(-) create mode 100644 alembic/versions/0069_rls_tenant_isolation_only.py create mode 100644 docs/security_kernel.md create mode 100644 frontend/src/hooks/useUserPermissions.ts diff --git a/alembic/versions/0069_rls_tenant_isolation_only.py b/alembic/versions/0069_rls_tenant_isolation_only.py new file mode 100644 index 0000000..9cd11a1 --- /dev/null +++ b/alembic/versions/0069_rls_tenant_isolation_only.py @@ -0,0 +1,103 @@ +"""Simplify RLS to pure tenant isolation. + +Per architecture review: RLS should be the "safety belt" (tenant isolation only), +NOT the "vehicle control" (business authorization). Business authorization +(owner_id, sharing, entity_permissions) belongs in the application layer +(visibility.py with Defense-in-Depth tenant_id filter). + +Revision ID: 0069 +Revises: 0068 +""" + +from alembic import op +from sqlalchemy import text + +revision = "0069" +down_revision = "0068" +branch_labels = None +depends_on = None + +RLS_TABLES = [ + "contacts", "addresses", "attachments", "bank_accounts", + "contact_folders", "contact_folder_permissions", "entity_permissions", + "entity_policies", "event_outbox", "audit_log", "notifications", + "saved_filters", "saved_views", "webhooks", "workflow_instances", + "workflow_step_history", "sequences", "custom_field_definitions", + "custom_field_values", "guest_users", "guest_invitations", + "consumer_inbox", "tenant_plugin_activation", "permission_templates", + "permission_delegations", "dms_files", "dms_folders", + "calendar_events", "calendars", "tasks", "task_lists", + "messages", "channels", "entity_links", "tags", "tag_assignments", + "mail_accounts", "mail_messages", "mail_folders", + "report_templates", "report_generations", "ai_conversations", + "ai_messages", "automation_workflows", "automation_runs", + "mcp_server_configs", "mcp_client_configs", "system_notifications", +] + +CONTACTS_POLICIES_TO_DROP = [ + "contacts_admin_select", "contacts_owner_select", + "contacts_shared_select", "contacts_tenant_owned_select", + "contacts_delete_policy", "contacts_insert_policy", + "contacts_update_policy", +] + + +def upgrade() -> None: + conn = op.get_bind() + + # 1. Drop all business-logic RLS policies on contacts + for policy in CONTACTS_POLICIES_TO_DROP: + op.execute(f"DROP POLICY IF EXISTS {policy} ON contacts") + + # 2. Drop old tenant_isolation policy on contacts + op.execute("DROP POLICY IF EXISTS contacts_tenant_isolation ON contacts") + + # 3. Create simple tenant isolation for ALL operations on contacts + op.execute( + "CREATE POLICY contacts_tenant_isolation ON contacts " + "FOR ALL " + "USING (tenant_id = current_setting('app.current_tenant_id', true)::uuid) " + "WITH CHECK (tenant_id = current_setting('app.current_tenant_id', true)::uuid)" + ) + + # 4. For all other RLS tables: drop existing policies, create simple tenant isolation + for table in RLS_TABLES: + if table == "contacts": + continue + + # Check if table exists first + table_exists = conn.execute( + text(f"SELECT 1 FROM information_schema.tables WHERE table_name = '{table}'") + ).fetchone() is not None + + if not table_exists: + continue + + # Get all existing policies on this table + result = conn.execute( + text(f"SELECT polname FROM pg_policy WHERE polrelid = '{table}'::regclass") + ) + policies = [row[0] for row in result] + + # Drop each policy + for policy in policies: + op.execute(f'DROP POLICY IF EXISTS "{policy}" ON {table}') + + # Check if table has tenant_id column + col_result = conn.execute( + text(f"SELECT 1 FROM information_schema.columns " + f"WHERE table_name = '{table}' AND column_name = 'tenant_id'") + ) + has_tenant_id = col_result.fetchone() is not None + + if has_tenant_id: + op.execute( + f"CREATE POLICY {table}_tenant_isolation ON {table} " + "FOR ALL " + "USING (tenant_id = current_setting('app.current_tenant_id', true)::uuid) " + "WITH CHECK (tenant_id = current_setting('app.current_tenant_id', true)::uuid)" + ) + + +def downgrade() -> None: + pass diff --git a/docs/security_kernel.md b/docs/security_kernel.md new file mode 100644 index 0000000..75eb9f0 --- /dev/null +++ b/docs/security_kernel.md @@ -0,0 +1,74 @@ +# Security Kernel — Verantwortungstabelle + +## Architektur-Prinzip + +```text +Authentifizierung → Tenant Membership → Capability-Prüfung → Objektfilter/ACL → RLS als letzte Barriere +``` + +## Was was prüft + +| Schicht | Verantwortung | Was geprüft wird | Wo implementiert | +|---------|--------------|------------------|------------------| +| **Authentifizierung** | User identifizieren | Session-Cookie, CSRF-Token | `deps.py:get_current_user()` | +| **Tenant Membership** | User gehört zu Tenant | `UserTenant.status == 'active'` | `deps.py:get_current_user()` | +| **Capability (RBAC)** | Darf User grundsätzlich Modul nutzen? | `contacts:read`, `contacts:write`, etc. | `require_permission()` Decorator | +| **Objekt-ACL** | Darf User DIESEN Datensatz sehen? | `owner_id == user_id` OR shared via `entity_permissions` | `visibility.py:apply_visibility_filter()` | +| **ABAC** | Darf User Datensatz mit bestimmten Attributen sehen? | Policy conditions (status, custom fields) | `policy_service.py:apply_policy_filter()` | +| **RLS** | Ist User im richtigen Tenant? | `tenant_id == current_setting('app.current_tenant_id')` | PostgreSQL RLS Policies | + +## Was RLS NICHT mehr prüft (seit Migration 0069) + +- ❌ `owner_id` — das macht `visibility.py` +- ❌ `entity_permissions` (sharing) — das macht `visibility.py` +- ❌ `is_system_admin` — das macht `visibility.py` (überspringt Filter) +- ❌ Business-Autorisierung — das macht die Application Layer + +## Was RLS nur noch prüft + +- ✅ `tenant_id == current_setting('app.current_tenant_id')` — Tenant-Isolation +- ✅ `WITH CHECK` für INSERT/UPDATE — verhindert cross-tenant writes + +## Defense-in-Depth + +```text +RLS (PostgreSQL) → tenant_id Isolation (Safety Belt) +visibility.py (App) → tenant_id + owner_id + sharing (Vehicle Control) +``` + +Beide Schichten filtern `tenant_id` unabhängig voneinander. Selbst wenn eine Schicht versagt, blockt die andere cross-tenant Zugriff. + +## RLS Policies (nach Migration 0069) + +Alle RLS-enabled Tabellen haben genau eine Policy: + +```sql +CREATE POLICY {table}_tenant_isolation ON {table} +FOR ALL +USING (tenant_id = current_setting('app.current_tenant_id', true)::uuid) +WITH CHECK (tenant_id = current_setting('app.current_tenant_id', true)::uuid) +``` + +Keine Business-Logic in RLS. Keine owner_id, keine sharing, keine permissions. + +## Session-Variablen + +| Variable | Wert | Wo gesetzt | +|----------|------|------------| +| `app.current_tenant_id` | UUID des aktuellen Tenants | `deps.py:set_tenant_context()` | +| `app.tenant_id` | UUID des aktuellen Tenants (Alias) | `deps.py:set_tenant_context()` | +| `app.current_user_id` | UUID des aktuellen Users | `deps.py:set_user_context()` | +| `app.is_system_admin` | `'true'` oder `'false'` | `deps.py:set_user_context()` | +| `app.current_user_groups` | Komma-getrennte Group-IDs | `deps.py:set_user_context()` | + +## Test-Verifikation + +8/8 Cross-Tenant Security Tests grün: +- ✅ visibility_filter_blocks_cross_tenant +- ✅ check_single_entity_access_cross_tenant +- ✅ get_visible_ids_tenant_scoped +- ✅ entity_permissions_tenant_scoped +- ✅ rls_tenant_isolation_policy_exists +- ✅ rls_enabled_on_tenant_tables +- ✅ rls_disabled_on_system_tables +- ✅ tenant_context_variable_consistency diff --git a/frontend/src/components/contacts/ContactDetail.tsx b/frontend/src/components/contacts/ContactDetail.tsx index 0156ae2..94ff626 100644 --- a/frontend/src/components/contacts/ContactDetail.tsx +++ b/frontend/src/components/contacts/ContactDetail.tsx @@ -163,9 +163,6 @@ export function ContactDetail({ contact, loading, onEdit, onDeleted }: ContactDe const { hasPermission, hasFieldAccess } = usePermission(); const authUser = useAuthStore((state) => state.user); const canAccess = (perm: string): boolean => { - if (authUser?.is_system_admin) return true; - const perms = authUser?.permissions || []; - if (perms.length === 0) return true; return hasPermission(perm); }; const createPersonMutation = useCreateContactPerson(); diff --git a/frontend/src/components/layout/Sidebar.tsx b/frontend/src/components/layout/Sidebar.tsx index c649372..9e2d407 100644 --- a/frontend/src/components/layout/Sidebar.tsx +++ b/frontend/src/components/layout/Sidebar.tsx @@ -45,12 +45,9 @@ export function Sidebar() { const { hasPermission } = usePermission(); const user = useAuthStore((state) => state.user); - // Fallback: system admin or empty permissions = show everything + // Use hasPermission directly — permissions are loaded via useUserPermissions hook const canAccess = (perm?: string): boolean => { if (!perm) return true; - if (user?.is_system_admin) return true; - const perms = user?.permissions || []; - if (perms.length === 0) return true; // No permissions loaded — show all, backend 403 handles it return hasPermission(perm); }; diff --git a/frontend/src/components/layout/TopBar.tsx b/frontend/src/components/layout/TopBar.tsx index b270448..30d73e9 100644 --- a/frontend/src/components/layout/TopBar.tsx +++ b/frontend/src/components/layout/TopBar.tsx @@ -22,10 +22,8 @@ export function TopBar() { const logoutMutation = useLogout(); const minimizedWindows = useWindowStore((s) => s.windows.filter((w) => w.state === 'minimized')); const { hasPermission } = usePermission(); + // Use hasPermission directly — permissions are loaded via useUserPermissions hook const canAccess = (perm: string): boolean => { - if (user?.is_system_admin) return true; - const perms = user?.permissions || []; - if (perms.length === 0) return true; return hasPermission(perm); }; const restoreWindow = useWindowStore((s) => s.restoreWindow); diff --git a/frontend/src/hooks/useAuth.ts b/frontend/src/hooks/useAuth.ts index e0cc6cd..00520f1 100644 --- a/frontend/src/hooks/useAuth.ts +++ b/frontend/src/hooks/useAuth.ts @@ -1,11 +1,15 @@ import { useEffect } from 'react'; import { useAuthStore } from '@/store/authStore'; import { useCurrentUser } from '@/api/hooks'; +import { useUserPermissions } from '@/hooks/useUserPermissions'; export function useAuth() { const store = useAuthStore(); const { data, isLoading, isError, error } = useCurrentUser(); + // Load permissions after authentication + useUserPermissions(); + useEffect(() => { if (isError) { const status = (error as any)?.status || 0; diff --git a/frontend/src/hooks/useUserPermissions.ts b/frontend/src/hooks/useUserPermissions.ts new file mode 100644 index 0000000..8077658 --- /dev/null +++ b/frontend/src/hooks/useUserPermissions.ts @@ -0,0 +1,39 @@ +import { useQuery } from '@tanstack/react-query'; +import { apiGet } from '@/api/client'; +import { useAuthStore } from '@/store/authStore'; +import { useEffect } from 'react'; + +interface PermissionsResponse { + permissions: string[]; + denied_permissions: string[]; + field_permissions: Record; + is_system_admin: boolean; +} + +/** + * Fetches the current user's resolved permissions from /api/v1/auth/me/permissions + * and stores them in the authStore. + */ +export function useUserPermissions() { + const { isAuthenticated, setPermissions } = useAuthStore(); + + const { data, isSuccess } = useQuery({ + queryKey: ['user-permissions'], + queryFn: () => apiGet('/api/v1/auth/me/permissions'), + enabled: isAuthenticated, + staleTime: 5 * 60 * 1000, + retry: 1, + }); + + useEffect(() => { + if (isSuccess && data) { + setPermissions( + data.permissions || [], + data.is_system_admin || false, + data.field_permissions || {}, + ); + } + }, [isSuccess, data, setPermissions]); + + return { data, isSuccess }; +} diff --git a/frontend/src/pages/ContactDetailPage.tsx b/frontend/src/pages/ContactDetailPage.tsx index 9df918a..3375a69 100644 --- a/frontend/src/pages/ContactDetailPage.tsx +++ b/frontend/src/pages/ContactDetailPage.tsx @@ -24,9 +24,6 @@ export function ContactDetailPage() { const { hasPermission } = usePermission(); const authUser = useAuthStore((state) => state.user); const canAccess = (perm: string): boolean => { - if (authUser?.is_system_admin) return true; - const perms = authUser?.permissions || []; - if (perms.length === 0) return true; return hasPermission(perm); }; diff --git a/frontend/src/pages/ContactsList.tsx b/frontend/src/pages/ContactsList.tsx index d2130bd..85031eb 100644 --- a/frontend/src/pages/ContactsList.tsx +++ b/frontend/src/pages/ContactsList.tsx @@ -45,9 +45,6 @@ export function ContactsListPage() { const { hasPermission } = usePermission(); const authUser = useAuthStore((state) => state.user); const canAccess = (perm: string): boolean => { - if (authUser?.is_system_admin) return true; - const perms = authUser?.permissions || []; - if (perms.length === 0) return true; return hasPermission(perm); };