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).
This commit is contained in:
Agent Zero
@@ -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
|
||||
|
||||
@@ -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()
|
||||
)
|
||||
@@ -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(),
|
||||
)
|
||||
@@ -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"),
|
||||
}
|
||||
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user