P1.6: secure guest invitation tokens (secrets.token_urlsafe + SHA-256 hash + one-time use + session revocation)
This commit is contained in:
@@ -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")
|
||||||
@@ -12,6 +12,7 @@ from app.models.contact_folder_permission import ContactFolderPermission
|
|||||||
from app.models.contact_merge import ContactMergeHistory
|
from app.models.contact_merge import ContactMergeHistory
|
||||||
from app.models.entity_permission import EntityPermission
|
from app.models.entity_permission import EntityPermission
|
||||||
from app.models.guest_user import GuestUser
|
from app.models.guest_user import GuestUser
|
||||||
|
from app.models.guest_invitation import GuestInvitation
|
||||||
from app.models.entity_policy import EntityPolicy
|
from app.models.entity_policy import EntityPolicy
|
||||||
from app.models.permission_template import PermissionTemplate
|
from app.models.permission_template import PermissionTemplate
|
||||||
from app.models.permission_delegation import PermissionDelegation
|
from app.models.permission_delegation import PermissionDelegation
|
||||||
@@ -55,6 +56,7 @@ __all__ = [
|
|||||||
"ContactFolderPermission",
|
"ContactFolderPermission",
|
||||||
"ContactMergeHistory",
|
"ContactMergeHistory",
|
||||||
"EntityPermission",
|
"EntityPermission",
|
||||||
|
"GuestInvitation",
|
||||||
"GuestUser",
|
"GuestUser",
|
||||||
"PermissionDelegation",
|
"PermissionDelegation",
|
||||||
"PermissionTemplate",
|
"PermissionTemplate",
|
||||||
|
|||||||
@@ -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()
|
||||||
|
)
|
||||||
+90
-44
@@ -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
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import hashlib
|
||||||
|
import secrets
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import UTC, datetime, timedelta
|
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.core.db import get_db
|
||||||
from app.deps import get_current_user, require_admin
|
from app.deps import get_current_user, require_admin
|
||||||
from app.models.guest_user import GuestUser
|
from app.models.guest_user import GuestUser
|
||||||
|
from app.models.guest_invitation import GuestInvitation
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/v1/guests", tags=["guests"])
|
router = APIRouter(prefix="/api/v1/guests", tags=["guests"])
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
|
|
||||||
|
|
||||||
|
def _hash_token(token: str) -> str:
|
||||||
|
"""Hash a token using SHA-256."""
|
||||||
|
return hashlib.sha256(token.encode()).hexdigest()
|
||||||
|
|
||||||
|
|
||||||
@router.post("/invite")
|
@router.post("/invite")
|
||||||
async def invite_guest(
|
async def invite_guest(
|
||||||
request: Request,
|
request: Request,
|
||||||
@@ -26,10 +41,10 @@ async def invite_guest(
|
|||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: dict = Depends(require_admin),
|
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", "")
|
email = body.get("email", "")
|
||||||
name = body.get("name", "")
|
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:
|
if not email or not name:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@@ -39,6 +54,7 @@ async def invite_guest(
|
|||||||
|
|
||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
user_id = uuid.UUID(current_user["user_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
|
# Check if guest already exists for this tenant
|
||||||
existing_q = await db.execute(
|
existing_q = await db.execute(
|
||||||
@@ -53,32 +69,36 @@ async def invite_guest(
|
|||||||
status_code=status.HTTP_409_CONFLICT,
|
status_code=status.HTTP_409_CONFLICT,
|
||||||
detail={"detail": "Guest already active", "code": "guest_exists"},
|
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.name = name
|
||||||
existing.status = "invited"
|
existing.status = "invited"
|
||||||
existing.invited_by = user_id
|
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
|
existing.password_hash = None
|
||||||
await db.commit()
|
await db.flush()
|
||||||
await db.refresh(existing)
|
guest = existing
|
||||||
return {
|
else:
|
||||||
"id": str(existing.id),
|
guest = GuestUser(
|
||||||
"email": existing.email,
|
email=email,
|
||||||
"name": existing.name,
|
name=name,
|
||||||
"status": existing.status,
|
tenant_id=tenant_id,
|
||||||
"expires_at": existing.expires_at.isoformat() if existing.expires_at else None,
|
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)
|
# Generate secure invitation token
|
||||||
guest = GuestUser(
|
raw_token = secrets.token_urlsafe(32)
|
||||||
email=email,
|
token_hash = _hash_token(raw_token)
|
||||||
name=name,
|
invitation = GuestInvitation(
|
||||||
tenant_id=tenant_id,
|
guest_user_id=guest.id,
|
||||||
invited_by=user_id,
|
token_hash=token_hash,
|
||||||
status="invited",
|
|
||||||
expires_at=expires_at,
|
expires_at=expires_at,
|
||||||
|
created_by=user_id,
|
||||||
)
|
)
|
||||||
db.add(guest)
|
db.add(invitation)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(guest)
|
await db.refresh(guest)
|
||||||
|
|
||||||
@@ -88,6 +108,7 @@ async def invite_guest(
|
|||||||
"name": guest.name,
|
"name": guest.name,
|
||||||
"status": guest.status,
|
"status": guest.status,
|
||||||
"expires_at": guest.expires_at.isoformat() if guest.expires_at else None,
|
"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,
|
body: dict,
|
||||||
db: AsyncSession = Depends(get_db),
|
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", "")
|
password = body.get("password", "")
|
||||||
if not password or len(password) < 8:
|
if not password or len(password) < 8:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@@ -105,36 +126,43 @@ async def accept_invitation(
|
|||||||
detail={"detail": "Password must be at least 8 characters", "code": "weak_password"},
|
detail={"detail": "Password must be at least 8 characters", "code": "weak_password"},
|
||||||
)
|
)
|
||||||
|
|
||||||
# Token is the guest user ID
|
# Hash the token and look up the invitation
|
||||||
try:
|
token_hash = _hash_token(token)
|
||||||
guest_id = uuid.UUID(token)
|
inv_q = await db.execute(
|
||||||
except ValueError:
|
select(GuestInvitation)
|
||||||
raise HTTPException(
|
.where(GuestInvitation.token_hash == token_hash)
|
||||||
status_code=status.HTTP_400_BAD_REQUEST,
|
.where(GuestInvitation.used_at.is_(None))
|
||||||
detail={"detail": "Invalid token", "code": "invalid_token"},
|
.where(GuestInvitation.revoked_at.is_(None))
|
||||||
)
|
|
||||||
|
|
||||||
guest_q = await db.execute(
|
|
||||||
select(GuestUser).where(GuestUser.id == guest_id).where(GuestUser.status == "invited")
|
|
||||||
)
|
)
|
||||||
guest = guest_q.scalar_one_or_none()
|
invitation = inv_q.scalar_one_or_none()
|
||||||
if not guest:
|
if not invitation:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_404_NOT_FOUND,
|
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
|
# Check expiration
|
||||||
if guest.expires_at and guest.expires_at < datetime.now(UTC):
|
if invitation.expires_at < datetime.now(UTC):
|
||||||
guest.status = "expired"
|
|
||||||
await db.commit()
|
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_410_GONE,
|
status_code=status.HTTP_410_GONE,
|
||||||
detail={"detail": "Invitation expired", "code": "invitation_expired"},
|
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.password_hash = hash_password(password)
|
||||||
guest.status = "active"
|
guest.status = "active"
|
||||||
|
invitation.used_at = datetime.now(UTC) # One-time use
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
|
||||||
return {"message": "Invitation accepted", "status": "active"}
|
return {"message": "Invitation accepted", "status": "active"}
|
||||||
@@ -172,7 +200,7 @@ async def delete_guest(
|
|||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: dict = Depends(require_admin),
|
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"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
try:
|
try:
|
||||||
gid = uuid.UUID(guest_id)
|
gid = uuid.UUID(guest_id)
|
||||||
@@ -195,10 +223,28 @@ async def delete_guest(
|
|||||||
# Revoke: mark as revoked and clear password
|
# Revoke: mark as revoked and clear password
|
||||||
guest.status = "revoked"
|
guest.status = "revoked"
|
||||||
guest.password_hash = None
|
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()
|
await db.commit()
|
||||||
|
|
||||||
# Invalidate any active guest sessions
|
# Invalidate all active guest sessions via Redis
|
||||||
redis = get_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"}
|
||||||
|
|||||||
Reference in New Issue
Block a user