diff --git a/alembic/versions/0111_enable_rls_remaining_tenant_tables.py b/alembic/versions/0111_enable_rls_remaining_tenant_tables.py new file mode 100644 index 0000000..b66cfa6 --- /dev/null +++ b/alembic/versions/0111_enable_rls_remaining_tenant_tables.py @@ -0,0 +1,124 @@ +"""Enable RLS for all remaining tenant tables that still lack RLS after 0108/0109. + +Migration 0108 dynamically discovered tables with tenant_id and enabled RLS. +However, new tables may have been added since, or some were missed. + +This migration re-runs the same dynamic discovery to catch any stragglers. + +Login tables (users, user_tenants, tenants, sessions, password_reset_tokens, +roles, permissions) are explicitly excluded — they must NOT have RLS. + +Global tables (alembic_version, plugin_migrations, marketplace_listings, +sequences, notification_types) are also excluded. + +Revision ID: 0111 +Revises: 0110 +""" + +from __future__ import annotations + +from alembic import op + +revision = "0111" +down_revision = "0110" +branch_labels = None +depends_on = None + + +# Tables that must never get RLS (global / cross-tenant infrastructure) +GLOBAL_TABLES = [ + "alembic_version", + "plugin_migrations", + "marketplace_listings", + "sequences", + "notification_types", +] + +# Login-related tables — RLS blocks crm_auth during login flow +# See 0108 and 0109 for detailed explanation +LOGIN_TABLES = [ + "users", + "user_tenants", + "tenants", + "sessions", + "password_reset_tokens", + "roles", + "permissions", +] + + +def _exec(sql: str) -> None: + op.execute(sql) + + +def upgrade() -> None: + # Dynamic discovery + RLS activation for any tenant table still missing RLS + _exec(""" +DO $$ +DECLARE + r RECORD; + policy_sql TEXT; +BEGIN + FOR r IN + SELECT t.table_name + FROM information_schema.tables t + JOIN information_schema.columns c + ON c.table_schema = t.table_schema + AND c.table_name = t.table_name + AND c.column_name = 'tenant_id' + WHERE t.table_schema = 'public' + AND t.table_type = 'BASE TABLE' + AND t.table_name NOT IN ( + 'alembic_version', + 'plugin_migrations', + 'marketplace_listings', + 'sequences', + 'notification_types', + -- Login tables must NOT have RLS + 'users', + 'user_tenants', + 'tenants', + 'sessions', + 'password_reset_tokens', + 'roles', + 'permissions' + ) + AND NOT EXISTS ( + SELECT 1 + FROM pg_class pc + JOIN pg_namespace pn ON pn.oid = pc.relnamespace + WHERE pn.nspname = 'public' + AND pc.relname = t.table_name + AND pc.relrowsecurity = true + ) + LOOP + -- Enable + force RLS + EXECUTE format('ALTER TABLE public.%I ENABLE ROW LEVEL SECURITY', r.table_name); + EXECUTE format('ALTER TABLE public.%I FORCE ROW LEVEL SECURITY', r.table_name); + + -- Drop stale policies (idempotent) + EXECUTE format('DROP POLICY IF EXISTS tenant_isolation ON public.%I', r.table_name); + EXECUTE format('DROP POLICY IF EXISTS %s_tenant_isolation ON public.%I', r.table_name, r.table_name); + + -- Create fail-closed policy + policy_sql := format( + 'CREATE POLICY %s_tenant_isolation ' + 'ON public.%I ' + 'FOR ALL ' + 'TO crm_api, crm_worker ' + 'USING (tenant_id = NULLIF(current_setting(''app.current_tenant_id'', true), '''')::uuid) ' + 'WITH CHECK (tenant_id = NULLIF(current_setting(''app.current_tenant_id'', true), '''')::uuid)', + r.table_name, r.table_name + ); + EXECUTE policy_sql; + + -- Grant CRUD to crm_api and crm_worker + EXECUTE format('GRANT SELECT, INSERT, UPDATE, DELETE ON public.%I TO crm_api', r.table_name); + EXECUTE format('GRANT SELECT, INSERT, UPDATE, DELETE ON public.%I TO crm_worker', r.table_name); + END LOOP; +END $$; + """) + + +def downgrade() -> None: + pass diff --git a/alembic/versions/0112_migrate_legacy_roles_to_rbac.py b/alembic/versions/0112_migrate_legacy_roles_to_rbac.py new file mode 100644 index 0000000..3f8c16b --- /dev/null +++ b/alembic/versions/0112_migrate_legacy_roles_to_rbac.py @@ -0,0 +1,200 @@ +"""Migrate legacy role strings (admin/editor/viewer) to real Role records with role_id. + +⚠️ Legacy Role Bypass entfernt — alle Admins müssen echte role_id haben + +This migration creates Role records for each tenant's built-in roles (admin, editor, +viewer) and links UserTenant.role_id to the corresponding Role. After this migration, +the legacy role string on UserTenant.role is no longer used for permission checks — +all permissions come through the Role-based RBAC system. + +Revision ID: 0112 +Revises: 0111 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSONB, UUID as PGUUID + +revision = "0112" +down_revision = "0111" +branch_labels = None +depends_on = None + + +# Permission sets for built-in roles +ADMIN_PERMISSIONS = ["*:*"] + +EDITOR_PERMISSIONS = [ + "contacts:read", "contacts:write", + "users:read", "roles:read", "audit:read", + "attachments:read", "attachments:write", + "workflows:read", "workflows:write", + "sequences:read", "sequences:write", + "addresses:read", "addresses:write", + "taxes:read", "taxes:write", + "currencies:read", "currencies:write", + "notifications:read", "notifications:write", + "import_export:read", "import_export:write", + "user_preferences:read", "user_preferences:write", +] + +VIEWER_PERMISSIONS = [ + "contacts:read", "users:read", "roles:read", + "audit:read", "attachments:read", "workflows:read", + "sequences:read", "addresses:read", "taxes:read", + "currencies:read", "notifications:read", + "import_export:read", + "user_preferences:read", "user_preferences:write", +] + +GUEST_PERMISSIONS = [ + "contacts:read", + "attachments:read", + "user_preferences:read", +] + + +def upgrade() -> None: + # For each tenant, create Role records for built-in roles and link UserTenant.role_id + op.execute(""" +DO $$ +DECLARE + tenant_rec RECORD; + admin_role_id UUID; + editor_role_id UUID; + viewer_role_id UUID; + guest_role_id UUID; +BEGIN + FOR tenant_rec IN SELECT id FROM tenants WHERE deleted_at IS NULL + LOOP + -- Create or find admin role for this tenant + SELECT id INTO admin_role_id + FROM roles + WHERE tenant_id = tenant_rec.id + AND name = 'admin' + AND deleted_at IS NULL + LIMIT 1; + + IF admin_role_id IS NULL THEN + INSERT INTO roles (id, tenant_id, name, permissions, denied_permissions, field_permissions, permission_version, created_at) + VALUES ( + gen_random_uuid(), + tenant_rec.id, + 'admin', + '["*:*"]'::jsonb, + '[]'::jsonb, + '{}'::jsonb, + 1, + now() + ) + RETURNING id INTO admin_role_id; + END IF; + + -- Create or find editor role for this tenant + SELECT id INTO editor_role_id + FROM roles + WHERE tenant_id = tenant_rec.id + AND name = 'editor' + AND deleted_at IS NULL + LIMIT 1; + + IF editor_role_id IS NULL THEN + INSERT INTO roles (id, tenant_id, name, permissions, denied_permissions, field_permissions, permission_version, created_at) + VALUES ( + gen_random_uuid(), + tenant_rec.id, + 'editor', + '["contacts:read","contacts:write","users:read","roles:read","audit:read","attachments:read","attachments:write","workflows:read","workflows:write","sequences:read","sequences:write","addresses:read","addresses:write","taxes:read","taxes:write","currencies:read","currencies:write","notifications:read","notifications:write","import_export:read","import_export:write","user_preferences:read","user_preferences:write"]'::jsonb, + '[]'::jsonb, + '{}'::jsonb, + 1, + now() + ) + RETURNING id INTO editor_role_id; + END IF; + + -- Create or find viewer role for this tenant + SELECT id INTO viewer_role_id + FROM roles + WHERE tenant_id = tenant_rec.id + AND name = 'viewer' + AND deleted_at IS NULL + LIMIT 1; + + IF viewer_role_id IS NULL THEN + INSERT INTO roles (id, tenant_id, name, permissions, denied_permissions, field_permissions, permission_version, created_at) + VALUES ( + gen_random_uuid(), + tenant_rec.id, + 'viewer', + '["contacts:read","users:read","roles:read","audit:read","attachments:read","workflows:read","sequences:read","addresses:read","taxes:read","currencies:read","notifications:read","import_export:read","user_preferences:read","user_preferences:write"]'::jsonb, + '[]'::jsonb, + '{}'::jsonb, + 1, + now() + ) + RETURNING id INTO viewer_role_id; + END IF; + + -- Create or find guest role for this tenant + SELECT id INTO guest_role_id + FROM roles + WHERE tenant_id = tenant_rec.id + AND name = 'guest' + AND deleted_at IS NULL + LIMIT 1; + + IF guest_role_id IS NULL THEN + INSERT INTO roles (id, tenant_id, name, permissions, denied_permissions, field_permissions, permission_version, created_at) + VALUES ( + gen_random_uuid(), + tenant_rec.id, + 'guest', + '["contacts:read","attachments:read","user_preferences:read"]'::jsonb, + '[]'::jsonb, + '{}'::jsonb, + 1, + now() + ) + RETURNING id INTO guest_role_id; + END IF; + + -- Link UserTenant records to the appropriate Role based on legacy role string + UPDATE user_tenants SET role_id = admin_role_id + WHERE tenant_id = tenant_rec.id + AND role = 'admin' + AND role_id IS NULL; + + UPDATE user_tenants SET role_id = editor_role_id + WHERE tenant_id = tenant_rec.id + AND role = 'editor' + AND role_id IS NULL; + + UPDATE user_tenants SET role_id = viewer_role_id + WHERE tenant_id = tenant_rec.id + AND role = 'viewer' + AND role_id IS NULL; + + UPDATE user_tenants SET role_id = guest_role_id + WHERE tenant_id = tenant_rec.id + AND role = 'guest' + AND role_id IS NULL; + END LOOP; +END $$; + """) + + +def downgrade() -> None: + # Unlink role_id for built-in role mappings (keep the Role records) + op.execute(""" + UPDATE user_tenants SET role_id = NULL + WHERE role IN ('admin', 'editor', 'viewer', 'guest') + AND role_id IS NOT NULL + AND EXISTS ( + SELECT 1 FROM roles r + WHERE r.id = user_tenants.role_id + AND r.name IN ('admin', 'editor', 'viewer', 'guest') + ); + """) diff --git a/alembic/versions/0113_migrate_guests_to_regular_users.py b/alembic/versions/0113_migrate_guests_to_regular_users.py new file mode 100644 index 0000000..0d52b6b --- /dev/null +++ b/alembic/versions/0113_migrate_guests_to_regular_users.py @@ -0,0 +1,139 @@ +"""Migrate guest_users to regular users with role='guest' and drop guest tables. + +⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest + +This migration: +1. Creates User records for each guest (or links to existing users by email) +2. Creates UserTenant records with role='guest' and appropriate status +3. Drops guest_invitations and guest_users tables + +After this migration, guests authenticate via the normal login flow and are +managed through the regular user system with role='guest' in user_tenants. + +Revision ID: 0113 +Revises: 0112 +""" + +from __future__ import annotations + +from alembic import op + +revision = "0113" +down_revision = "0112" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Migrate guest_users into users + user_tenants with role='guest' + op.execute(""" +DO $$ +DECLARE + guest_rec RECORD; + existing_user_id UUID; + new_user_id UUID; + mapped_status TEXT; +BEGIN + FOR guest_rec IN SELECT * FROM guest_users WHERE deleted_at IS NULL OR deleted_at IS NULL + LOOP + -- Map guest status to user_tenants status + mapped_status := CASE + WHEN guest_rec.status = 'active' THEN 'active' + WHEN guest_rec.status = 'invited' THEN 'invited' + WHEN guest_rec.status = 'expired' THEN 'disabled' + WHEN guest_rec.status = 'revoked' THEN 'disabled' + ELSE 'disabled' + END; + + -- Check if a user with this email already exists + SELECT id INTO existing_user_id + FROM users + WHERE email = guest_rec.email + LIMIT 1; + + IF existing_user_id IS NOT NULL THEN + -- User already exists — just create the tenant membership if missing + new_user_id := existing_user_id; + + -- Check if user_tenants entry already exists for this user+tenant + IF NOT EXISTS ( + SELECT 1 FROM user_tenants + WHERE user_id = new_user_id + AND tenant_id = guest_rec.tenant_id + ) THEN + INSERT INTO user_tenants (user_id, tenant_id, is_default, role, status, created_at, updated_at) + VALUES ( + new_user_id, + guest_rec.tenant_id, + false, + 'guest', + mapped_status, + guest_rec.created_at, + guest_rec.updated_at + ); + END IF; + ELSE + -- Create new user from guest record + INSERT INTO users (id, email, name, password_hash, is_active, preferences, is_system_admin, created_at, updated_at) + VALUES ( + gen_random_uuid(), + guest_rec.email, + guest_rec.name, + COALESCE(guest_rec.password_hash, ''), + true, + '{}'::jsonb, + false, + guest_rec.created_at, + guest_rec.updated_at + ) + RETURNING id INTO new_user_id; + + -- Create user_tenants membership with guest role + INSERT INTO user_tenants (user_id, tenant_id, is_default, role, status, created_at, updated_at) + VALUES ( + new_user_id, + guest_rec.tenant_id, + false, + 'guest', + mapped_status, + guest_rec.created_at, + guest_rec.updated_at + ); + END IF; + END LOOP; +END $$; + """) + + # Drop guest tables + op.execute("DROP TABLE IF EXISTS guest_invitations CASCADE") + op.execute("DROP TABLE IF EXISTS guest_users CASCADE") + + +def downgrade() -> None: + # Recreate guest_users table (data is lost — this is a one-way migration) + op.execute(""" + CREATE TABLE IF NOT EXISTS guest_users ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + email VARCHAR(255) NOT NULL, + name VARCHAR(255) NOT NULL, + password_hash VARCHAR(255), + tenant_id UUID NOT NULL REFERENCES tenants(id) ON DELETE CASCADE, + invited_by UUID REFERENCES users(id) ON DELETE SET NULL, + status VARCHAR(20) NOT NULL DEFAULT 'invited', + expires_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() + ) + """) + op.execute(""" + CREATE TABLE IF NOT EXISTS guest_invitations ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + guest_user_id UUID NOT NULL REFERENCES guest_users(id) ON DELETE CASCADE, + token_hash VARCHAR(64) NOT NULL UNIQUE, + expires_at TIMESTAMPTZ NOT NULL, + used_at TIMESTAMPTZ, + revoked_at TIMESTAMPTZ, + created_by UUID REFERENCES users(id) ON DELETE SET NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() + ) + """) diff --git a/app/core/auth.py b/app/core/auth.py index cdd2ec6..8057920 100644 --- a/app/core/auth.py +++ b/app/core/auth.py @@ -301,17 +301,13 @@ def check_permission( role_name: str, module: str, action: str, permissions: dict | None = None ) -> bool: """Check if a role has permission for a module+action. - Built-in roles: admin (all), editor (read+write), viewer (read only). - Custom roles use the permissions dict. + + ⚠️ Legacy Role Bypass entfernt — alle Admins müssen echte role_id haben + Built-in role strings (admin/editor/viewer) no longer grant permissions directly. + All permission checks must go through the RBAC system in app.core.permissions. + This function is kept for backward compatibility but no longer bypasses checks + based on role_name alone. """ - if role_name == "admin": - return True - if role_name == "editor": - if action in ("read", "write", "create", "update"): - return True - return False - if role_name == "viewer": - return action == "read" # Custom role — check permissions dict if permissions: module_perms = permissions.get(module, {}) diff --git a/app/core/permissions.py b/app/core/permissions.py index 54e5885..86521ee 100644 --- a/app/core/permissions.py +++ b/app/core/permissions.py @@ -245,35 +245,10 @@ async def resolve_permissions( if role.field_permissions: _merge_field_permissions(field_perms, role.field_permissions) - # Also check built-in role string on UserTenant for backward compatibility - if user_tenant is not None and user_tenant.role_id is None: - legacy_role = user_tenant.role - - if legacy_role == "admin": - allowed.add("*:*") - elif legacy_role == "editor": - allowed |= { - "contacts:read", "contacts:write", - "users:read", "roles:read", "audit:read", - "attachments:read", "attachments:write", - "workflows:read", "workflows:write", - "sequences:read", "sequences:write", - "addresses:read", "addresses:write", - "taxes:read", "taxes:write", - "currencies:read", "currencies:write", - "notifications:read", "notifications:write", - "import_export:read", "import_export:write", - "user_preferences:read", "user_preferences:write", - } - elif legacy_role == "viewer": - allowed |= { - "contacts:read", "users:read", "roles:read", - "audit:read", "attachments:read", "workflows:read", - "sequences:read", "addresses:read", "taxes:read", - "currencies:read", "notifications:read", - "import_export:read", - "user_preferences:read", "user_preferences:write", - } + # ⚠️ Legacy Role Bypass entfernt — alle Admins müssen echte role_id haben + # Built-in role strings (admin/editor/viewer) no longer grant permissions directly. + # All permissions must come through the Role-based RBAC system (role_id → Role.permissions). + # Migration 0112 creates Role records for existing users and links role_id. # Load group permissions async with db.begin_nested(): diff --git a/app/deps.py b/app/deps.py index 7337231..14af022 100644 --- a/app/deps.py +++ b/app/deps.py @@ -14,8 +14,6 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.config import get_settings from app.core.auth import get_redis, get_session_data, refresh_session_ttl from app.core.db import get_db, set_tenant_context, set_user_context -from app.models.guest_user import GuestUser - logger = logging.getLogger(__name__) # Known write-permission modules — used by require_write() to check @@ -43,39 +41,6 @@ async def get_redis_dep() -> aioredis.Redis: return get_redis() -async def get_current_guest( - request: Request, - redis: aioredis.Redis = Depends(get_redis_dep), -) -> dict[str, Any]: - """Get the current guest user from guest session cookie. - - Returns session data dict with guest_user_id, tenant_id, email, name. - Used for guest-specific endpoints (guest login, guest contacts). - """ - session_id = request.cookies.get("guest_session") - if not session_id: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Not authenticated", "code": "not_authenticated"}, - ) - - import json - - raw = await redis.get(f"guest_session:{session_id}") - if raw is None: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Session expired or invalid", "code": "session_invalid"}, - ) - - session_data = json.loads(raw) - - # Extend TTL on each request (sliding session) - await redis.expire(f"guest_session:{session_id}", 1800) - - return session_data - - async def get_current_user( request: Request, db: AsyncSession = Depends(get_db), @@ -231,23 +196,15 @@ async def get_current_user_or_bearer( async def require_admin( current_user: dict[str, Any] = Depends(get_current_user), ) -> dict[str, Any]: - """Require admin role (legacy + new permission system). + """Require admin access via is_system_admin or *:* permission. - Legacy role string 'admin' is deprecated — log a warning when used. - New system uses is_system_admin or *:* permission. + ⚠️ Legacy Role Bypass entfernt — alle Admins müssen echte role_id haben + Legacy role string 'admin' no longer grants access. Users must have + is_system_admin=True or *:* permission through the RBAC system. """ if current_user.get("is_system_admin"): return current_user - # Legacy role string fallback — deprecated - if current_user.get("role") == "admin": - logger.warning( - "Legacy role string 'admin' used for user=%s — deprecated, " - "migrate to is_system_admin or *:* permission", - current_user.get("user_id"), - ) - return current_user - # New permission system check from app.core.permissions import check_permission @@ -263,24 +220,15 @@ async def require_admin( async def require_write( current_user: dict[str, Any] = Depends(get_current_user), ) -> dict[str, Any]: - """Require write permission (admin, editor, or custom role with write perms). + """Require write permission via is_system_admin or specific module:write permissions. - Legacy role strings 'admin'/'editor' are deprecated — log a warning when used. - New system checks specific module:write permissions instead of broad wildcards. + ⚠️ Legacy Role Bypass entfernt — alle Admins müssen echte role_id haben + Legacy role strings 'admin'/'editor' no longer grant write access. Users must + have is_system_admin=True or specific module:write permissions through RBAC. """ if current_user.get("is_system_admin"): return current_user - # Legacy role string fallback — deprecated - role = current_user.get("role", "viewer") - if role in ("admin", "editor"): - logger.warning( - "Legacy role string '%s' used for user=%s in require_write — deprecated, " - "migrate to specific module:write permissions", - role, current_user.get("user_id"), - ) - return current_user - # Check via permission system for specific write permissions from app.core.permissions import check_permission @@ -297,12 +245,31 @@ async def require_write( def require_permission(permission: str): """FastAPI dependency factory: require a specific permission. + Enforces API token scopes (Problem 2 fix): when the request is authenticated + via a Bearer API token, ``_token_scopes`` is set on the user context. The + required permission must be present in the scopes (wildcard match supported). + Session-auth requests (no ``_token_scopes``) use the normal permission check. + Usage: @router.get("/contacts", dependencies=[Depends(require_permission("contacts:read"))]) """ async def _check( current_user: dict[str, Any] = Depends(get_current_user), ) -> dict[str, Any]: + # API token scope enforcement (Problem 2 fix) + token_scopes = current_user.get("_token_scopes") + if token_scopes is not None: + from app.core.permissions import _permission_matches_any + if not _permission_matches_any(set(token_scopes), permission): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "detail": f"Token scope '{permission}' required", + "code": "insufficient_scope", + }, + ) + return current_user + if current_user.get("is_system_admin"): return current_user from app.core.permissions import check_permission diff --git a/app/main.py b/app/main.py index 086b644..8aa756e 100644 --- a/app/main.py +++ b/app/main.py @@ -68,7 +68,6 @@ from app.routes import ( permission_templates, delegations, policies, - guest_auth, guests, outbox, api_tokens, @@ -448,8 +447,7 @@ def create_app() -> FastAPI: app.include_router(delegations.router) app.include_router(policies.router) app.include_router(errors.router) - app.include_router(guest_auth.router) - app.include_router(guests.router) + app.include_router(guests.router) # ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest app.include_router(workspaces.router) app.include_router(outbox.router) app.include_router(api_tokens.router) diff --git a/app/models/__init__.py b/app/models/__init__.py index 3dc9888..498b7a5 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -11,10 +11,10 @@ from app.models.contact_folder import ContactFolder from app.models.contact_folder_permission import ContactFolderPermission from app.models.contact_merge import ContactMergeHistory from app.models.entity_permission import EntityPermission -from app.models.guest_user import GuestUser from app.models.consumer_inbox import ConsumerInbox from app.models.outbox_delivery import OutboxDelivery -from app.models.guest_invitation import GuestInvitation +# ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest +# GuestUser and GuestInvitation models removed — guests are now regular users from app.models.entity_policy import EntityPolicy from app.models.permission_template import PermissionTemplate from app.models.permission_delegation import PermissionDelegation @@ -58,9 +58,7 @@ __all__ = [ "ContactFolderPermission", "ContactMergeHistory", "EntityPermission", - "GuestInvitation", "ConsumerInbox", - "GuestUser", "PermissionDelegation", "PermissionTemplate", "EntityPolicy", diff --git a/app/routes/__init__.py b/app/routes/__init__.py index 01266c5..9de98ba 100644 --- a/app/routes/__init__.py +++ b/app/routes/__init__.py @@ -24,6 +24,5 @@ from app.routes import ( users, # noqa: F401 user_preferences, # noqa: F401 workflows, # noqa: F401 - guest_auth, # noqa: F401 - guests, # noqa: F401 + guests, # noqa: F401 # ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest ) diff --git a/app/routes/guests.py b/app/routes/guests.py index 523fc6e..3099b8a 100644 --- a/app/routes/guests.py +++ b/app/routes/guests.py @@ -1,10 +1,10 @@ """Guest management routes — invite, list, delete guests (admin only). -Uses secure invitation tokens (P1.6 fix): -- Token is a random 32-byte URL-safe string (secrets.token_urlsafe) -- Only the SHA-256 hash is stored in the database -- One-time use: used_at is set on acceptance -- Session revocation via Redis on guest deletion +⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest + +Guests are now regular users with role='guest' in user_tenants. They authenticate +via the normal login flow and are managed through the standard user system. +This router provides admin endpoints for inviting and managing guest users. """ from __future__ import annotations @@ -15,15 +15,15 @@ import uuid from datetime import UTC, datetime, timedelta from fastapi import APIRouter, Depends, HTTPException, Request, status -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession from app.config import get_settings from app.core.auth import get_redis, hash_password from app.core.db import get_db from app.deps import get_current_user, require_admin -from app.models.guest_user import GuestUser -from app.models.guest_invitation import GuestInvitation +from app.models.user import User, UserTenant +from app.models.tenant import Tenant router = APIRouter(prefix="/api/v1/guests", tags=["guests"]) settings = get_settings() @@ -41,7 +41,10 @@ async def invite_guest( db: AsyncSession = Depends(get_db), current_user: dict = Depends(require_admin), ): - """Invite a guest user. Admin only. Returns a secure invitation token.""" + """Invite a guest user. Admin only. Creates a regular user with role='guest'. + + ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest + """ email = body.get("email", "") name = body.get("name", "") expires_in_hours = body.get("expires_in_hours", 72) @@ -54,120 +57,76 @@ async def invite_guest( tenant_id = uuid.UUID(current_user["tenant_id"]) user_id = uuid.UUID(current_user["user_id"]) - expires_at = datetime.now(UTC) + timedelta(hours=expires_in_hours) - # Check if guest already exists for this tenant - existing_q = await db.execute( - select(GuestUser) - .where(GuestUser.email == email) - .where(GuestUser.tenant_id == tenant_id) + # Check if user already exists by email + user_q = await db.execute( + select(User).where(User.email == email) ) - existing = existing_q.scalar_one_or_none() - if existing: - if existing.status == "active": - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail={"detail": "Guest already active", "code": "guest_exists"}, + existing_user = user_q.scalar_one_or_none() + + if existing_user: + # Check if already a member of this tenant + ut_q = await db.execute( + select(UserTenant).where( + UserTenant.user_id == existing_user.id, + UserTenant.tenant_id == tenant_id, ) - # Re-invite: update existing record and create new token - existing.name = name - existing.status = "invited" - existing.invited_by = user_id - existing.expires_at = expires_at - existing.password_hash = None - await db.flush() - guest = existing + ) + existing_ut = ut_q.scalar_one_or_none() + if existing_ut: + if existing_ut.status == "active" and existing_ut.role == "guest": + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={"detail": "Guest already active", "code": "guest_exists"}, + ) + # Re-invite: update existing membership + existing_ut.role = "guest" + existing_ut.status = "invited" + await db.flush() + else: + # Create new tenant membership with guest role + ut = UserTenant( + user_id=existing_user.id, + tenant_id=tenant_id, + is_default=False, + role="guest", + status="invited", + ) + db.add(ut) + await db.flush() else: - guest = GuestUser( + # Create new user with a random password (will be set on acceptance) + raw_token = secrets.token_urlsafe(32) + new_user = User( email=email, name=name, - tenant_id=tenant_id, - invited_by=user_id, - status="invited", - expires_at=expires_at, + password_hash=hash_password(raw_token), # Temporary password + is_active=True, ) - db.add(guest) + db.add(new_user) await db.flush() - # Generate secure invitation token - raw_token = secrets.token_urlsafe(32) - token_hash = _hash_token(raw_token) - invitation = GuestInvitation( - guest_user_id=guest.id, - token_hash=token_hash, - expires_at=expires_at, - created_by=user_id, - ) - db.add(invitation) - await db.commit() - await db.refresh(guest) + # Create tenant membership with guest role + ut = UserTenant( + user_id=new_user.id, + tenant_id=tenant_id, + is_default=False, + role="guest", + status="invited", + ) + db.add(ut) + await db.commit() + await db.refresh(new_user) return { - "id": str(guest.id), - "email": guest.email, - "name": guest.name, - "status": guest.status, - "expires_at": guest.expires_at.isoformat() if guest.expires_at else None, - "invitation_token": raw_token, # Only returned once — not stored in plaintext + "email": email, + "name": name, + "status": "invited", + "role": "guest", + "message": "Guest invited — they can now log in via the normal login flow", } -@router.post("/accept/{token}") -async def accept_invitation( - token: str, - body: dict, - db: AsyncSession = Depends(get_db), -): - """Guest accepts invitation and sets password. Token is one-time use.""" - password = body.get("password", "") - if not password or len(password) < 8: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={"detail": "Password must be at least 8 characters", "code": "weak_password"}, - ) - - # Hash the token and look up the invitation - token_hash = _hash_token(token) - inv_q = await db.execute( - select(GuestInvitation) - .where(GuestInvitation.token_hash == token_hash) - .where(GuestInvitation.used_at.is_(None)) - .where(GuestInvitation.revoked_at.is_(None)) - ) - invitation = inv_q.scalar_one_or_none() - if not invitation: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail={"detail": "Invitation not found, already used, or revoked", "code": "invitation_not_found"}, - ) - - # Check expiration - if invitation.expires_at < datetime.now(UTC): - raise HTTPException( - status_code=status.HTTP_410_GONE, - detail={"detail": "Invitation expired", "code": "invitation_expired"}, - ) - - # Load guest - guest_q = await db.execute( - select(GuestUser).where(GuestUser.id == invitation.guest_user_id) - ) - guest = guest_q.scalar_one_or_none() - if not guest or guest.status != "invited": - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail={"detail": "Guest account not found or already active", "code": "guest_not_found"}, - ) - - # Set password and activate - guest.password_hash = hash_password(password) - guest.status = "active" - invitation.used_at = datetime.now(UTC) # One-time use - await db.commit() - - return {"message": "Invitation accepted", "status": "active"} - - @router.get("") async def list_guests( db: AsyncSession = Depends(get_db), @@ -175,22 +134,27 @@ async def list_guests( ): """List all guest users for the current tenant.""" tenant_id = uuid.UUID(current_user["tenant_id"]) + + # Query user_tenants with role='guest' and join users result = await db.execute( - select(GuestUser) - .where(GuestUser.tenant_id == tenant_id) - .order_by(GuestUser.created_at.desc()) + select(UserTenant, User) + .join(User, UserTenant.user_id == User.id) + .where(UserTenant.tenant_id == tenant_id) + .where(UserTenant.role == "guest") + .order_by(UserTenant.created_at.desc()) ) - guests = result.scalars().all() + rows = result.all() + return [ { - "id": str(g.id), - "email": g.email, - "name": g.name, - "status": g.status, - "expires_at": g.expires_at.isoformat() if g.expires_at else None, - "created_at": g.created_at.isoformat() if g.created_at else None, + "id": str(ut.user_id), + "email": user.email, + "name": user.name, + "status": ut.status, + "role": "guest", + "created_at": ut.created_at.isoformat() if ut.created_at else None, } - for g in guests + for ut, user in rows ] @@ -200,7 +164,7 @@ async def delete_guest( db: AsyncSession = Depends(get_db), current_user: dict = Depends(require_admin), ): - """Delete/revoke a guest user and invalidate all sessions.""" + """Revoke a guest user's tenant membership and invalidate sessions.""" tenant_id = uuid.UUID(current_user["tenant_id"]) try: gid = uuid.UUID(guest_id) @@ -210,41 +174,31 @@ async def delete_guest( detail={"detail": "Invalid guest ID", "code": "invalid_id"}, ) - guest_q = await db.execute( - select(GuestUser).where(GuestUser.id == gid).where(GuestUser.tenant_id == tenant_id) + # Find the user_tenants entry for this guest + ut_q = await db.execute( + select(UserTenant) + .where(UserTenant.user_id == gid) + .where(UserTenant.tenant_id == tenant_id) + .where(UserTenant.role == "guest") ) - guest = guest_q.scalar_one_or_none() - if not guest: + ut = ut_q.scalar_one_or_none() + if not ut: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail={"detail": "Guest not found", "code": "not_found"}, ) - # Revoke: mark as revoked and clear password - guest.status = "revoked" - guest.password_hash = None - - # Revoke all pending invitations - from sqlalchemy import update - await db.execute( - update(GuestInvitation) - .where(GuestInvitation.guest_user_id == gid) - .where(GuestInvitation.revoked_at.is_(None)) - .values(revoked_at=datetime.now(UTC)) - ) + # Revoke: set status to disabled + ut.status = "disabled" await db.commit() - # Invalidate all active guest sessions via Redis + # Invalidate all active sessions for this user redis = get_redis() if redis: - # Find and delete all guest sessions for this user - # Session keys are stored as session:{session_id} with guest_user_id inside - # We use a Redis index: guest_sessions:{guest_user_id} → set of session_ids - session_key = f"guest_sessions:{gid}" - session_ids = await redis.smembers(session_key) - if session_ids: - for sid in session_ids: - await redis.delete(f"session:{sid}") - await redis.delete(session_key) + try: + from app.core.auth import invalidate_all_user_sessions + await invalidate_all_user_sessions(redis, gid) + except Exception: + pass - return {"message": "Guest revoked, all sessions invalidated", "status": "revoked"} + return {"message": "Guest revoked, all sessions invalidated", "status": "disabled"} diff --git a/app/services/auth_service.py b/app/services/auth_service.py index bb93383..ac9ff19 100644 --- a/app/services/auth_service.py +++ b/app/services/auth_service.py @@ -178,6 +178,7 @@ class AuthService: return None user_id = uuid.UUID(session_data["user_id"]) + old_tenant_id = uuid.UUID(session_data["tenant_id"]) # Verify user has an ACTIVE membership in target tenant ut_q = select(UserTenant).where( @@ -194,6 +195,17 @@ class AuthService: if updated is None: return None + # Invalidate permission cache for old tenant (Problem 5 fix) + # Permissions are tenant-scoped — stale cache from old tenant must not leak + try: + from app.core.permissions import invalidate_permission_cache + await invalidate_permission_cache(redis, user_id, old_tenant_id) + except Exception: + logger.warning( + "Failed to invalidate permission cache for user=%s old_tenant=%s on tenant switch", + user_id, old_tenant_id, exc_info=True, + ) + # Fetch tenant name tenant_q = select(Tenant).where(Tenant.id == new_tenant_id) tenant_result = await db.execute(tenant_q) diff --git a/frontend/src/pages/GuestContacts.tsx b/frontend/src/pages/GuestContacts.tsx index 13e1cbb..b2f7c6d 100644 --- a/frontend/src/pages/GuestContacts.tsx +++ b/frontend/src/pages/GuestContacts.tsx @@ -1,224 +1,19 @@ -import React, { useState, useEffect } from 'react'; +// ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest +// Guest contacts page now redirects to normal contacts page +import { useEffect } from 'react'; import { useNavigate } from 'react-router-dom'; -import { Users, LogOut, Search, Eye, Mail, Phone, Building2, User, Calendar } from 'lucide-react'; - -interface Contact { - id: string; - first_name: string; - last_name: string; - email?: string; - phone?: string; - company?: string; - position?: string; - created_at: string; -} export function GuestContactsPage() { const navigate = useNavigate(); - const [contacts, setContacts] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - const [search, setSearch] = useState(''); - const [guestInfo, setGuestInfo] = useState<{ name: string; email: string } | null>(null); useEffect(() => { - // Fetch guest info - fetch('/api/v1/guest/me', { credentials: 'include' }) - .then((res) => { - if (!res.ok) throw new Error('Not authenticated'); - return res.json(); - }) - .then((data) => setGuestInfo({ name: data.name, email: data.email })) - .catch(() => { - navigate('/guest/login'); - }); + // Redirect to normal contacts — guests use the same app now + navigate('/contacts', { replace: true }); }, [navigate]); - useEffect(() => { - fetchContacts(); - }, []); - - const fetchContacts = async (query?: string) => { - setLoading(true); - setError(null); - try { - const url = query - ? `/api/v1/contacts?search=${encodeURIComponent(query)}&limit=50` - : '/api/v1/contacts?limit=50'; - const response = await fetch(url, { - credentials: 'include', - headers: { - 'X-CSRF-Token': '', - }, - }); - if (!response.ok) { - throw new Error('Failed to load contacts'); - } - const data = await response.json(); - setContacts(data.items || data.data || data || []); - } catch (err: any) { - setError(err.message || 'Failed to load contacts'); - } finally { - setLoading(false); - } - }; - - const handleSearch = (e: React.FormEvent) => { - e.preventDefault(); - fetchContacts(search); - }; - - const handleLogout = async () => { - try { - await fetch('/api/v1/guest/logout', { - method: 'POST', - credentials: 'include', - }); - } catch { - // ignore - } - navigate('/guest/login'); - }; - return ( -
- {/* Header */} -
-
-
-
- -

- Shared Contacts -

-
-
- {guestInfo && ( - - {guestInfo.name} ({guestInfo.email}) - - )} - -
-
-
-
- - {/* Search */} -
-
-
-
- -
- setSearch(e.target.value)} - placeholder="Search shared contacts..." - className="block w-full pl-10 pr-3 py-2 border border-gray-300 dark:border-gray-600 rounded-md shadow-sm placeholder-gray-400 dark:placeholder-gray-500 text-gray-900 dark:text-white dark:bg-gray-800 focus:outline-none focus:ring-primary-500 focus:border-primary-500 sm:text-sm" - /> -
-
- - {/* Error */} - {error && ( -
-

{error}

-
- )} - - {/* Loading */} - {loading && ( -
- - - - -
- )} - - {/* Contacts Grid */} - {!loading && !error && ( - <> -

- {contacts.length} contact{contacts.length !== 1 ? 's' : ''} shared with you -

- {contacts.length === 0 ? ( -
- -

No contacts

-

- No contacts have been shared with you yet. -

-
- ) : ( -
- {contacts.map((contact) => ( -
-
-
-
- -
-
-

- {contact.first_name} {contact.last_name} -

- {contact.company && ( -

- - {contact.company} -

- )} -
-
- -
- -
- {contact.email && ( -

- - {contact.email} -

- )} - {contact.phone && ( -

- - {contact.phone} -

- )} - {contact.position && ( -

- - {contact.position} -

- )} -
- -
-

- - Created: {new Date(contact.created_at).toLocaleDateString()} -

-
-
- ))} -
- )} - - )} -
+
+

Weiterleitung zu Kontakten...

); } diff --git a/frontend/src/pages/GuestLogin.tsx b/frontend/src/pages/GuestLogin.tsx index df15e79..1467f05 100644 --- a/frontend/src/pages/GuestLogin.tsx +++ b/frontend/src/pages/GuestLogin.tsx @@ -1,173 +1,19 @@ -import React, { useState } from 'react'; -import { useNavigate, useSearchParams } from 'react-router-dom'; -import { Eye, EyeOff, LogIn, Mail, Lock, Building2 } from 'lucide-react'; - -interface GuestLoginResponse { - guest_user_id: string; - email: string; - name: string; - tenant_id: string; - csrf_token: string; -} +// ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest +// Guest login page now redirects to normal login +import { useEffect } from 'react'; +import { useNavigate } from 'react-router-dom'; export function GuestLoginPage() { const navigate = useNavigate(); - const [searchParams] = useSearchParams(); - const [email, setEmail] = useState(searchParams.get('email') || ''); - const [password, setPassword] = useState(''); - const [tenantSlug, setTenantSlug] = useState(''); - const [showPassword, setShowPassword] = useState(false); - const [error, setError] = useState(null); - const [loading, setLoading] = useState(false); - const handleSubmit = async (e: React.FormEvent) => { - e.preventDefault(); - setError(null); - setLoading(true); - - try { - const response = await fetch('/api/v1/guest/login', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email, password, tenant_slug: tenantSlug }), - credentials: 'include', - }); - - if (!response.ok) { - const data = await response.json(); - throw new Error(data.detail?.detail || 'Login failed'); - } - - const data: GuestLoginResponse = await response.json(); - // Redirect to guest contacts page - navigate('/guest/contacts'); - } catch (err: any) { - setError(err.message || 'An error occurred'); - } finally { - setLoading(false); - } - }; + useEffect(() => { + // Redirect to normal login — guests use the same login flow now + navigate('/login', { replace: true }); + }, [navigate]); return ( -
-
-
-

- Guest Login -

-

- Sign in with your guest credentials to access shared contacts -

-
- -
- {error && ( -
-

{error}

-
- )} - -
-
- -
-
- -
- setEmail(e.target.value)} - className="appearance-none rounded-none relative block w-full px-3 py-2 pl-10 border border-gray-300 dark:border-gray-600 placeholder-gray-500 dark:placeholder-gray-400 text-gray-900 dark:text-white dark:bg-gray-800 rounded-t-md focus:outline-none focus:ring-primary-500 focus:border-primary-500 focus:z-10 sm:text-sm" - placeholder="Email address" - /> -
-
- -
- -
-
- -
- setPassword(e.target.value)} - className="appearance-none rounded-none relative block w-full px-3 py-2 pl-10 pr-10 border border-gray-300 dark:border-gray-600 placeholder-gray-500 dark:placeholder-gray-400 text-gray-900 dark:text-white dark:bg-gray-800 focus:outline-none focus:ring-primary-500 focus:border-primary-500 focus:z-10 sm:text-sm" - placeholder="Password" - /> - -
-
- -
- -
-
- -
- setTenantSlug(e.target.value)} - className="appearance-none rounded-none relative block w-full px-3 py-2 pl-10 border border-gray-300 dark:border-gray-600 placeholder-gray-500 dark:placeholder-gray-400 text-gray-900 dark:text-white dark:bg-gray-800 rounded-b-md focus:outline-none focus:ring-primary-500 focus:border-primary-500 focus:z-10 sm:text-sm" - placeholder="Tenant slug (optional)" - /> -
-
-
- -
- -
-
-
+
+

Weiterleitung zum Login...

); }