From 8539a6402c2eaca77a0f1ca0c0aa0a421c60952c Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Wed, 29 Jul 2026 12:30:00 +0200 Subject: [PATCH] P1.6: secure guest invitation tokens (secrets.token_urlsafe + SHA-256 hash + one-time use + session revocation) --- .../versions/0062_guest_invitations_secure.py | 51 +++++++ app/models/__init__.py | 2 + app/models/guest_invitation.py | 43 ++++++ app/routes/guests.py | 134 ++++++++++++------ 4 files changed, 186 insertions(+), 44 deletions(-) create mode 100644 alembic/versions/0062_guest_invitations_secure.py create mode 100644 app/models/guest_invitation.py diff --git a/alembic/versions/0062_guest_invitations_secure.py b/alembic/versions/0062_guest_invitations_secure.py new file mode 100644 index 0000000..dc2cb99 --- /dev/null +++ b/alembic/versions/0062_guest_invitations_secure.py @@ -0,0 +1,51 @@ +"""Fix guest invitation security — separate token table. + +Revision ID: 0062 +Revises: 0061 +Create Date: 2026-07-29 + +Problems fixed: +1. Guest UUID was used as invitation token (P1.6) +2. No separate token with sufficient entropy +3. No one-time use tracking +4. No session revocation on guest deletion +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import UUID + +revision = "0062" +down_revision = "0061" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "guest_invitations", + sa.Column("id", UUID(as_uuid=True), primary_key=True, server_default=sa.text("gen_random_uuid()")), + sa.Column("guest_user_id", UUID(as_uuid=True), sa.ForeignKey("guest_users.id", ondelete="CASCADE"), nullable=False), + sa.Column("token_hash", sa.String(64), nullable=False, unique=True, index=True), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("used_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("created_by", UUID(as_uuid=True), sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.text("NOW()"), nullable=False), + ) + op.execute("ALTER TABLE guest_invitations ENABLE ROW LEVEL SECURITY") + op.execute(""" + CREATE POLICY guest_invitations_tenant_isolation ON guest_invitations + FOR ALL + USING ( + EXISTS ( + SELECT 1 FROM guest_users gu + WHERE gu.id = guest_invitations.guest_user_id + AND gu.tenant_id = current_setting('app.current_tenant_id', true)::uuid + ) + ) + """) + + +def downgrade() -> None: + op.drop_table("guest_invitations") diff --git a/app/models/__init__.py b/app/models/__init__.py index f1c3171..5321586 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -12,6 +12,7 @@ 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.guest_invitation import GuestInvitation from app.models.entity_policy import EntityPolicy from app.models.permission_template import PermissionTemplate from app.models.permission_delegation import PermissionDelegation @@ -55,6 +56,7 @@ __all__ = [ "ContactFolderPermission", "ContactMergeHistory", "EntityPermission", + "GuestInvitation", "GuestUser", "PermissionDelegation", "PermissionTemplate", diff --git a/app/models/guest_invitation.py b/app/models/guest_invitation.py new file mode 100644 index 0000000..ff99117 --- /dev/null +++ b/app/models/guest_invitation.py @@ -0,0 +1,43 @@ +"""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/routes/guests.py b/app/routes/guests.py index b115b61..523fc6e 100644 --- a/app/routes/guests.py +++ b/app/routes/guests.py @@ -1,7 +1,16 @@ -"""Guest management routes — invite, list, delete guests (admin only).""" +"""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 +""" from __future__ import annotations +import hashlib +import secrets import uuid from datetime import UTC, datetime, timedelta @@ -14,11 +23,17 @@ 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 router = APIRouter(prefix="/api/v1/guests", tags=["guests"]) settings = get_settings() +def _hash_token(token: str) -> str: + """Hash a token using SHA-256.""" + return hashlib.sha256(token.encode()).hexdigest() + + @router.post("/invite") async def invite_guest( request: Request, @@ -26,10 +41,10 @@ async def invite_guest( db: AsyncSession = Depends(get_db), current_user: dict = Depends(require_admin), ): - """Invite a guest user. Admin only.""" + """Invite a guest user. Admin only. Returns a secure invitation token.""" email = body.get("email", "") name = body.get("name", "") - expires_in_hours = body.get("expires_in_hours", 72) # Default 3 days + expires_in_hours = body.get("expires_in_hours", 72) if not email or not name: raise HTTPException( @@ -39,6 +54,7 @@ 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( @@ -53,32 +69,36 @@ async def invite_guest( status_code=status.HTTP_409_CONFLICT, detail={"detail": "Guest already active", "code": "guest_exists"}, ) - # Re-invite: update existing record + # Re-invite: update existing record and create new token existing.name = name existing.status = "invited" existing.invited_by = user_id - existing.expires_at = datetime.now(UTC) + timedelta(hours=expires_in_hours) + existing.expires_at = expires_at existing.password_hash = None - await db.commit() - await db.refresh(existing) - return { - "id": str(existing.id), - "email": existing.email, - "name": existing.name, - "status": existing.status, - "expires_at": existing.expires_at.isoformat() if existing.expires_at else None, - } + await db.flush() + guest = existing + else: + guest = GuestUser( + email=email, + name=name, + tenant_id=tenant_id, + invited_by=user_id, + status="invited", + expires_at=expires_at, + ) + db.add(guest) + await db.flush() - expires_at = datetime.now(UTC) + timedelta(hours=expires_in_hours) - guest = GuestUser( - email=email, - name=name, - tenant_id=tenant_id, - invited_by=user_id, - status="invited", + # 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(guest) + db.add(invitation) await db.commit() await db.refresh(guest) @@ -88,6 +108,7 @@ async def invite_guest( "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 } @@ -97,7 +118,7 @@ async def accept_invitation( body: dict, db: AsyncSession = Depends(get_db), ): - """Guest accepts invitation and sets password.""" + """Guest accepts invitation and sets password. Token is one-time use.""" password = body.get("password", "") if not password or len(password) < 8: raise HTTPException( @@ -105,36 +126,43 @@ async def accept_invitation( detail={"detail": "Password must be at least 8 characters", "code": "weak_password"}, ) - # Token is the guest user ID - try: - guest_id = uuid.UUID(token) - except ValueError: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={"detail": "Invalid token", "code": "invalid_token"}, - ) - - guest_q = await db.execute( - select(GuestUser).where(GuestUser.id == guest_id).where(GuestUser.status == "invited") + # 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)) ) - guest = guest_q.scalar_one_or_none() - if not guest: + invitation = inv_q.scalar_one_or_none() + if not invitation: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, - detail={"detail": "Invitation not found or already accepted", "code": "invitation_not_found"}, + detail={"detail": "Invitation not found, already used, or revoked", "code": "invitation_not_found"}, ) # Check expiration - if guest.expires_at and guest.expires_at < datetime.now(UTC): - guest.status = "expired" - await db.commit() + 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"} @@ -172,7 +200,7 @@ async def delete_guest( db: AsyncSession = Depends(get_db), current_user: dict = Depends(require_admin), ): - """Delete/revoke a guest user.""" + """Delete/revoke a guest user and invalidate all sessions.""" tenant_id = uuid.UUID(current_user["tenant_id"]) try: gid = uuid.UUID(guest_id) @@ -195,10 +223,28 @@ async def delete_guest( # 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)) + ) await db.commit() - # Invalidate any active guest sessions + # Invalidate all active guest sessions via Redis redis = get_redis() - # We can't easily find all sessions for this guest, but they'll expire naturally + 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) - return {"message": "Guest revoked", "status": "revoked"} + return {"message": "Guest revoked, all sessions invalidated", "status": "revoked"}