From 8060505baabb2d15b75c2d571f553d87013d0850 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Thu, 6 Aug 2026 11:46:00 +0200 Subject: [PATCH] refactor(cleanup): remove dead GuestUser/GuestInvitation code and fix test imports Deleted: - app/models/guest_user.py - app/models/guest_invitation.py - app/routes/guest_auth.py (was orphaned, not imported) - tests/test_guest_auth.py (tested removed guest auth system) Modified: - app/models/__init__.py: removed stale GuestUser/GuestInvitation comments - app/services/entity_permission_service.py: updated guest permission comment - tests/test_permission_system_live.py: replaced GuestUser with User+UserTenant(role=guest), changed principal_type from "guest" to "user", switched guest test from /api/v1/guest/login to regular /api/v1/auth/login endpoint Frontend: no guest components found, nothing to clean up. Alembic migrations: historical migrations referencing guest_users/guest_invitations tables are left intact (they document DB history). --- app/models/__init__.py | 2 - app/models/guest_invitation.py | 43 ------ app/models/guest_user.py | 60 -------- app/routes/guest_auth.py | 169 ---------------------- app/services/entity_permission_service.py | 4 +- tests/test_guest_auth.py | 75 ---------- tests/test_permission_system_live.py | 37 ++--- 7 files changed, 14 insertions(+), 376 deletions(-) delete mode 100644 app/models/guest_invitation.py delete mode 100644 app/models/guest_user.py delete mode 100644 app/routes/guest_auth.py delete mode 100644 tests/test_guest_auth.py diff --git a/app/models/__init__.py b/app/models/__init__.py index 498b7a5..62b7b19 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -13,8 +13,6 @@ from app.models.contact_merge import ContactMergeHistory from app.models.entity_permission import EntityPermission from app.models.consumer_inbox import ConsumerInbox from app.models.outbox_delivery import OutboxDelivery -# ⚠️ 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 diff --git a/app/models/guest_invitation.py b/app/models/guest_invitation.py deleted file mode 100644 index ff99117..0000000 --- a/app/models/guest_invitation.py +++ /dev/null @@ -1,43 +0,0 @@ -"""Guest invitation model — secure token-based invitations.""" - -from __future__ import annotations - -import uuid -from datetime import datetime - -from sqlalchemy import DateTime, ForeignKey, String, func -from sqlalchemy.dialects.postgresql import UUID as PGUUID -from sqlalchemy.orm import Mapped, mapped_column - -from app.core.db import Base - - -class GuestInvitation(Base): - """Secure invitation token for guest users. - - Token is a random 32-byte URL-safe string. - Only the hash is stored in the database. - One-time use: used_at is set on acceptance. - """ - - __tablename__ = "guest_invitations" - - id: Mapped[uuid.UUID] = mapped_column( - PGUUID(as_uuid=True), primary_key=True, default=uuid.uuid4 - ) - guest_user_id: Mapped[uuid.UUID] = mapped_column( - PGUUID(as_uuid=True), - ForeignKey("guest_users.id", ondelete="CASCADE"), - nullable=False, - index=True, - ) - token_hash: Mapped[str] = mapped_column(String(64), nullable=False, unique=True, index=True) - expires_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) - used_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True, default=None) - revoked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True, default=None) - created_by: Mapped[uuid.UUID | None] = mapped_column( - PGUUID(as_uuid=True), ForeignKey("users.id", ondelete="SET NULL"), nullable=True - ) - created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.now() - ) diff --git a/app/models/guest_user.py b/app/models/guest_user.py deleted file mode 100644 index 2e4d316..0000000 --- a/app/models/guest_user.py +++ /dev/null @@ -1,60 +0,0 @@ -"""Guest User model — for time-limited guest access via entity permissions.""" - -from __future__ import annotations - -import uuid -from datetime import datetime - -from sqlalchemy import DateTime, ForeignKey, Index, String, func -from sqlalchemy.dialects.postgresql import UUID as PGUUID -from sqlalchemy.orm import Mapped, mapped_column - -from app.core.db import Base, TenantMixin - - -class GuestUser(Base, TenantMixin): - """Guest user with time-limited access to shared entities. - - Guests are invited by tenant admins and can only access entities - that have explicit entity_permissions with principal_type='guest'. - """ - - __tablename__ = "guest_users" - __table_args__ = ( - Index("ix_guest_users_email_tenant", "email", "tenant_id", unique=True), - Index("ix_guest_users_status", "status", "tenant_id"), - Index("ix_guest_users_invited_by", "invited_by"), - ) - - id: Mapped[uuid.UUID] = mapped_column( - PGUUID(as_uuid=True), primary_key=True, default=uuid.uuid4 - ) - email: Mapped[str] = mapped_column(String(255), nullable=False) - name: Mapped[str] = mapped_column(String(255), nullable=False) - password_hash: Mapped[str | None] = mapped_column( - String(255), nullable=True, default=None - ) - tenant_id: Mapped[uuid.UUID] = mapped_column( - PGUUID(as_uuid=True), - ForeignKey("tenants.id", ondelete="CASCADE"), - nullable=False, - index=True, - ) - invited_by: Mapped[uuid.UUID | None] = mapped_column( - PGUUID(as_uuid=True), - ForeignKey("users.id", ondelete="SET NULL"), - nullable=True, - ) - status: Mapped[str] = mapped_column( - String(20), nullable=False, default="invited" - ) # 'invited', 'active', 'expired', 'revoked' - expires_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), nullable=True, default=None - ) - created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.now() - ) - updated_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.now(), - onupdate=func.now(), - ) diff --git a/app/routes/guest_auth.py b/app/routes/guest_auth.py deleted file mode 100644 index 76a9d0c..0000000 --- a/app/routes/guest_auth.py +++ /dev/null @@ -1,169 +0,0 @@ -"""Guest Auth routes — login, logout for guest users.""" - -from __future__ import annotations - -import uuid -from datetime import UTC, datetime, timedelta - -from fastapi import APIRouter, Depends, HTTPException, Request, Response, status -from pydantic import BaseModel, EmailStr -from sqlalchemy import select -from sqlalchemy.ext.asyncio import AsyncSession - -from app.config import get_settings -from app.core.auth import get_redis, hash_password, verify_password -from app.core.db import get_db -from app.deps import get_current_guest -from app.models.guest_user import GuestUser -from app.models.tenant import Tenant - -router = APIRouter(prefix="/api/v1/guest", tags=["guest-auth"]) -settings = get_settings() - - -class GuestLoginRequest(BaseModel): - """Schema for guest login request.""" - email: EmailStr - password: str - tenant_slug: str - - -@router.post("/login") -async def guest_login( - request: Request, - body: GuestLoginRequest, - db: AsyncSession = Depends(get_db), -): - """Guest login with email+password. Sets guest session cookie.""" - email = body.email - password = body.password - tenant_slug = body.tenant_slug - - # Find guest user by email — tenant_slug is required to prevent cross-tenant enumeration - tenant_q = await db.execute( - select(Tenant).where(Tenant.slug == tenant_slug) - ) - tenant = tenant_q.scalar_one_or_none() - if not tenant: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Invalid credentials", "code": "invalid_credentials"}, - ) - tenant_id = tenant.id - - # Find guest with tenant context - guest_q = await db.execute( - select(GuestUser) - .where(GuestUser.email == email) - .where(GuestUser.tenant_id == tenant_id) - .where(GuestUser.status == "active") - ) - guest = guest_q.scalar_one_or_none() - - if not guest or not guest.password_hash: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Invalid credentials", "code": "invalid_credentials"}, - ) - - if not verify_password(password, guest.password_hash): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Invalid credentials", "code": "invalid_credentials"}, - ) - - # Check expiration - if guest.expires_at and guest.expires_at < datetime.now(UTC): - guest.status = "expired" - await db.commit() - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail={"detail": "Guest access expired", "code": "guest_expired"}, - ) - - # Create guest session in Redis - redis = get_redis() - session_id = str(uuid.uuid4()) - csrf_token = str(uuid.uuid4()) - session_data = { - "guest_user_id": str(guest.id), - "tenant_id": str(tenant_id), - "email": guest.email, - "name": guest.name, - "csrf_token": csrf_token, - "is_guest": True, - } - import json - - await redis.setex( - f"guest_session:{session_id}", - 1800, # 30 min TTL - json.dumps(session_data), - ) - # Track session in guest index for revocation (P1.6 fix) - await redis.sadd(f"guest_sessions:{guest.id}", session_id) - await redis.expire(f"guest_sessions:{guest.id}", 1800) - - from fastapi.responses import JSONResponse - - resp = JSONResponse( - status_code=status.HTTP_200_OK, - content={ - "guest_user_id": str(guest.id), - "email": guest.email, - "name": guest.name, - "tenant_id": str(tenant_id), - "csrf_token": csrf_token, - }, - ) - resp.set_cookie( - key="guest_session", - value=session_id, - httponly=True, - secure=settings.session_cookie_secure, - samesite=settings.session_cookie_samesite, - max_age=1800, - path="/", - ) - return resp - - -@router.post("/logout") -async def guest_logout( - request: Request, -): - """Logout — invalidate guest session, clear cookie.""" - session_id = request.cookies.get("guest_session") - if session_id: - redis = get_redis() - # Remove from guest sessions index (P1.6 fix) - guest_data = await redis.get(f"guest_session:{session_id}") - if guest_data: - import json - data = json.loads(guest_data) - gid = data.get("guest_user_id") - if gid: - await redis.srem(f"guest_sessions:{gid}", session_id) - await redis.delete(f"guest_session:{session_id}") - - from fastapi.responses import JSONResponse - - resp = JSONResponse( - status_code=status.HTTP_200_OK, - content={"message": "Logged out"}, - ) - resp.delete_cookie("guest_session", path="/") - return resp - - -@router.get("/me") -async def guest_me( - current_guest: dict = Depends(get_current_guest), -): - """Get current guest user info.""" - return { - "guest_user_id": current_guest.get("guest_user_id"), - "email": current_guest.get("email"), - "name": current_guest.get("name"), - "tenant_id": current_guest.get("tenant_id"), - } diff --git a/app/services/entity_permission_service.py b/app/services/entity_permission_service.py index 22b8fdb..3f29c96 100644 --- a/app/services/entity_permission_service.py +++ b/app/services/entity_permission_service.py @@ -485,8 +485,8 @@ async def get_effective_access( EntityPermission.principal_id == role_id, ) ) - # Guest permission — only if user_id is actually a guest_user_id - # (Guest users have no groups/roles, only direct entity_permissions) + # Guest permission — guests are now regular users with role=guest + # (Guest users may have no groups/roles, only direct entity_permissions) principal_conditions.append( and_( EntityPermission.principal_type == "guest", diff --git a/tests/test_guest_auth.py b/tests/test_guest_auth.py deleted file mode 100644 index 7274497..0000000 --- a/tests/test_guest_auth.py +++ /dev/null @@ -1,75 +0,0 @@ -"""Tests for guest authentication — login, logout, tenant isolation. - -Security-critical: Guest users must only access their assigned tenant. -Cross-tenant enumeration must be prevented. -""" - -from __future__ import annotations - -import pytest -from httpx import AsyncClient - -from tests.conftest import ORIGIN_HEADER, seed_tenant_and_users, login_client - - -@pytest.mark.asyncio -class TestGuestAuth: - """Guest authentication and authorization tests.""" - - async def test_guest_login_requires_tenant_slug(self, client: AsyncClient, db_session): - """Guest login without tenant_slug should return 422 (validation error).""" - resp = await client.post( - "/api/v1/guest/login", - json={"email": "guest@test.de", "password": "TestPass123!"}, - headers=ORIGIN_HEADER, - ) - # Pydantic validation error — tenant_slug is required - assert resp.status_code == 422 - - async def test_guest_login_invalid_tenant_returns_401(self, client: AsyncClient, db_session): - """Guest login with non-existent tenant_slug returns 401.""" - resp = await client.post( - "/api/v1/guest/login", - json={"email": "guest@test.de", "password": "TestPass123!", "tenant_slug": "nonexistent"}, - headers=ORIGIN_HEADER, - ) - assert resp.status_code == 401 - - async def test_guest_login_invalid_credentials(self, client: AsyncClient, db_session): - """Guest login with valid tenant but wrong password returns 401.""" - # Seed data - seed = await seed_tenant_and_users(db_session) - - resp = await client.post( - "/api/v1/guest/login", - json={"email": "nobody@test.de", "password": "WrongPass!", "tenant_slug": "tenant-a"}, - headers=ORIGIN_HEADER, - ) - assert resp.status_code == 401 - - async def test_guest_login_valid_email_format_validation(self, client: AsyncClient, db_session): - """Guest login with invalid email format returns 422.""" - resp = await client.post( - "/api/v1/guest/login", - json={"email": "not-an-email", "password": "TestPass123!", "tenant_slug": "tenant-a"}, - headers=ORIGIN_HEADER, - ) - assert resp.status_code == 422 - - async def test_guest_login_missing_password(self, client: AsyncClient, db_session): - """Guest login with empty password returns 401.""" - resp = await client.post( - "/api/v1/guest/login", - json={"email": "guest@test.de", "password": "", "tenant_slug": "tenant-a"}, - headers=ORIGIN_HEADER, - ) - assert resp.status_code in (401, 422) - - async def test_guest_logout_without_session(self, client: AsyncClient, db_session): - """Guest logout without active session returns 200 (idempotent).""" - resp = await client.post( - "/api/v1/guest/logout", - headers=ORIGIN_HEADER, - ) - # Guest logout is idempotent — returns 200 even without session - assert resp.status_code in (200, 401, 403, 400) diff --git a/tests/test_permission_system_live.py b/tests/test_permission_system_live.py index 5f5b1ac..fa0917c 100644 --- a/tests/test_permission_system_live.py +++ b/tests/test_permission_system_live.py @@ -47,7 +47,6 @@ from app.models.role import Role from app.models.tenant import Tenant from app.models.user import User, UserTenant from app.models.entity_permission import EntityPermission -from app.models.guest_user import GuestUser from app.models.group import Group, UserGroup from app.plugins.registry import reset_registry_for_testing # noqa: F401 from app.services.plugin_service import reset_plugin_service_for_testing # noqa: F401 @@ -262,13 +261,13 @@ async def seed_full_data(db: AsyncSession) -> dict[str, Any]: is_active=True, preferences={}, ) - # Guest user in tenant A - guest_a = GuestUser( + # Guest user in tenant A (now a regular User with role=guest) + guest_a = User( email="guest@tenanta.com", name="Guest A", password_hash=hash_password("TestPass123!"), - tenant_id=tenant_a.id, - status="active", + is_active=True, + preferences={}, ) db.add_all([admin_a, editor_a, viewer_a, admin_b, orphan_user, guest_a]) await db.flush() @@ -280,7 +279,9 @@ async def seed_full_data(db: AsyncSession) -> dict[str, Any]: ut4 = UserTenant(user_id=admin_b.id, tenant_id=tenant_b.id, is_default=True, role="admin") # Admin A is also member of tenant B (multi-tenant) ut5 = UserTenant(user_id=admin_a.id, tenant_id=tenant_b.id, is_default=False, role="admin") - db.add_all([ut1, ut2, ut3, ut4, ut5]) + # Guest tenant membership with role=guest + ut6 = UserTenant(user_id=guest_a.id, tenant_id=tenant_a.id, is_default=True, role="guest") + db.add_all([ut1, ut2, ut3, ut4, ut5, ut6]) await db.flush() # Custom role with limited permissions in tenant A @@ -368,7 +369,7 @@ async def seed_full_data(db: AsyncSession) -> dict[str, Any]: tenant_id=tenant_a.id, entity_type="contact", entity_id=contact_a1.id, - principal_type="guest", + principal_type="user", principal_id=guest_a.id, permission_level="read", created_by=admin_a.id, @@ -862,26 +863,12 @@ async def test_scenario_guest_access(client: AsyncClient, db_session: AsyncSessi """Guest user can only see entities shared with them.""" seed = await seed_full_data(db_session) - # Guest login - resp = await client.post( - "/api/v1/guest/login", - json={ - "email": "guest@tenanta.com", - "password": "TestPass123!", - "tenant_slug": "tenant-a", - }, - headers=ORIGIN_HEADER, - ) - assert resp.status_code == 200, f"Guest login failed: {resp.status_code} {resp.text}" + # Guest login (now uses regular auth endpoint) + await login(client, "guest@tenanta.com", tenant_slug="tenant-a") # Guest should be able to access shared contact (contact_a1 shared with guest) - # Note: guest endpoints may differ from regular contact endpoints - # Test guest-specific contact access if available - # If no guest contact endpoint exists, verify guest session is valid - guest_session = resp.json() - assert guest_session.get("guest_user_id") or guest_session.get("user_id"), ( - f"Guest session should have user ID: {guest_session}" - ) + resp = await client.get("/api/v1/contacts") + assert resp.status_code == 200, f"Guest contacts access failed: {resp.status_code} {resp.text}" @pytest.mark.asyncio