-
Notifications
You must be signed in to change notification settings - Fork 663
✨feat: Add conversation share #3308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a1c7c39
9d8398f
9c7f0b1
9efe4ce
cc70957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| import logging | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Auth Check get_conversation_share_endpoint, download_share_asset_endpoint, preview_share_asset_endpoint have NO authentication checks. Anyone with share_token can access conversations and download assets. Add rate limiting and optional password protection.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Expire Time Validation CreateConversationShareRequest accepts expire_time without validation. User could set year 3000 creating permanent shares. Reject expire_time > 1 year and < now.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No Share Revocation Endpoint revoke_conversation_share exists in DB layer but NO API endpoint calls it. Users cannot revoke shares. Add DELETE /share/{share_token} with auth check.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Rate Limiting create_conversation_share_endpoint has no rate limiting. Malicious user could create thousands of shares. Add per-user and per-conversation rate limits.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Snapshot Content Exposure snapshot_json stores full conversation history returned to unauthenticated users. Ensure sensitive data (API keys, internal URLs, debug info) is filtered before storing. |
||
| from datetime import datetime | ||
| from http import HTTPStatus | ||
| from typing import Any, Dict, List, Optional, Tuple | ||
|
|
||
| from fastapi import APIRouter, Header, HTTPException, Path as PathParam, Query | ||
| from fastapi.responses import StreamingResponse | ||
| from pydantic import BaseModel | ||
| from starlette.background import BackgroundTask | ||
|
|
||
| from consts.exceptions import FileTooLargeException, NotFoundException, UnsupportedFileTypeException | ||
| from services.conversation_share_service import ( | ||
| create_share_snapshot_service, | ||
| get_share_asset_service, | ||
| get_share_snapshot_service, | ||
| ) | ||
| from services.file_management_service import get_file_stream_impl, get_preview_stream, resolve_preview_file | ||
| from utils.auth_utils import get_current_user_id | ||
|
|
||
| from .file_management_app import build_content_disposition_header | ||
|
|
||
| logger = logging.getLogger("conversation_share_app") | ||
|
|
||
| router = APIRouter(prefix="/share") | ||
|
|
||
|
|
||
| class CreateConversationShareRequest(BaseModel): | ||
| mode: str = "selected" | ||
| selected_user_message_ids: Optional[List[int]] = None | ||
| expire_time: Optional[datetime] = None | ||
|
|
||
|
|
||
| def _parse_range_header(range_header: Optional[str], total_size: int) -> Optional[Tuple[int, int]]: | ||
| if not range_header: | ||
| return None | ||
| if not range_header.startswith("bytes="): | ||
| return None | ||
|
|
||
| range_value = range_header[len("bytes="):].strip() | ||
| if "," in range_value: | ||
| return None | ||
|
|
||
| start_text, _, end_text = range_value.partition("-") | ||
| try: | ||
| if start_text: | ||
| start = int(start_text) | ||
| end = int(end_text) if end_text else total_size - 1 | ||
| else: | ||
| suffix_length = int(end_text) | ||
| if suffix_length <= 0: | ||
| return None | ||
| start = max(total_size - suffix_length, 0) | ||
| end = total_size - 1 | ||
| except ValueError: | ||
| return None | ||
|
|
||
| if start < 0 or end < start or start >= total_size: | ||
| return None | ||
| return start, min(end, total_size - 1) | ||
|
|
||
|
|
||
| @router.post("/conversation/{conversation_id}") | ||
| async def create_conversation_share_endpoint( | ||
| request: CreateConversationShareRequest, | ||
| conversation_id: int, | ||
| authorization: Optional[str] = Header(None), | ||
|
Check warning on line 66 in backend/apps/conversation_share_app.py
|
||
| ): | ||
| try: | ||
| user_id, tenant_id = get_current_user_id(authorization) | ||
| result = create_share_snapshot_service( | ||
| conversation_id=conversation_id, | ||
| user_id=user_id, | ||
| tenant_id=tenant_id, | ||
| mode=request.mode, | ||
| selected_user_message_ids=request.selected_user_message_ids, | ||
| expire_time=request.expire_time, | ||
| ) | ||
| result["url"] = f"/share/{result['share_id']}" | ||
| return {"code": 0, "message": "success", "data": result} | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail=str(e)) | ||
| except Exception as e: | ||
| logger.error("Failed to create conversation share: %s", str(e), exc_info=True) | ||
|
Check failure on line 83 in backend/apps/conversation_share_app.py
|
||
| raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to create share") | ||
|
|
||
|
|
||
| @router.get("/{share_token}") | ||
| async def get_conversation_share_endpoint(share_token: str): | ||
| try: | ||
| return { | ||
| "code": 0, | ||
| "message": "success", | ||
| "data": get_share_snapshot_service(share_token), | ||
| } | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail=str(e)) | ||
| except Exception as e: | ||
| logger.error("Failed to get conversation share: %s", str(e), exc_info=True) | ||
|
Check failure on line 98 in backend/apps/conversation_share_app.py
|
||
| raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to get share") | ||
|
|
||
|
|
||
| @router.get("/{share_token}/assets/{asset_id}/download") | ||
| async def download_share_asset_endpoint( | ||
| share_token: str, | ||
| asset_id: str, | ||
| filename: Optional[str] = Query(None), | ||
|
Check warning on line 106 in backend/apps/conversation_share_app.py
|
||
| ): | ||
| try: | ||
| asset = get_share_asset_service(share_token, asset_id) | ||
| object_name = asset["object_name"] | ||
| file_stream, content_type = await get_file_stream_impl(object_name=object_name) | ||
| download_filename = filename or asset.get("filename") or object_name.rsplit("/", 1)[-1] | ||
| return StreamingResponse( | ||
| file_stream, | ||
| media_type=content_type, | ||
| headers={ | ||
| "Content-Disposition": build_content_disposition_header(download_filename), | ||
| "Cache-Control": "public, max-age=3600", | ||
| "ETag": f'"share-{share_token}-{asset_id}"', | ||
| }, | ||
| ) | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail=str(e)) | ||
| except Exception as e: | ||
| logger.error("Failed to download share asset: %s", str(e), exc_info=True) | ||
|
Check failure on line 125 in backend/apps/conversation_share_app.py
|
||
| raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to download asset") | ||
|
|
||
|
|
||
| @router.get("/{share_token}/assets/{asset_id}/preview") | ||
| async def preview_share_asset_endpoint( | ||
| share_token: str, | ||
| asset_id: str, | ||
| filename: Optional[str] = Query(None), | ||
|
Check warning on line 133 in backend/apps/conversation_share_app.py
|
||
| range_header: Optional[str] = Header(None, alias="range"), | ||
|
Check warning on line 134 in backend/apps/conversation_share_app.py
|
||
| ): | ||
| try: | ||
| asset = get_share_asset_service(share_token, asset_id) | ||
| object_name = asset["object_name"] | ||
| actual_name, content_type, total_size = await resolve_preview_file(object_name=object_name) | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail=str(e)) | ||
| except FileTooLargeException as e: | ||
| raise HTTPException(status_code=HTTPStatus.REQUEST_ENTITY_TOO_LARGE, detail=str(e)) | ||
| except NotFoundException as e: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail=str(e)) | ||
| except UnsupportedFileTypeException as e: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail=str(e)) | ||
| except Exception as e: | ||
| logger.error("Failed to resolve share asset preview: %s", str(e), exc_info=True) | ||
|
Check failure on line 149 in backend/apps/conversation_share_app.py
|
||
| raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to preview asset") | ||
|
|
||
| display_filename = filename or asset.get("filename") or object_name.rsplit("/", 1)[-1] | ||
| common_headers = { | ||
| "Content-Disposition": build_content_disposition_header(display_filename, inline=True), | ||
| "Accept-Ranges": "bytes", | ||
| "Cache-Control": "public, max-age=3600", | ||
| "ETag": f'"share-{share_token}-{asset_id}"', | ||
| } | ||
|
|
||
| if total_size == 0: | ||
| return StreamingResponse( | ||
| iter([]), | ||
| status_code=HTTPStatus.OK, | ||
| media_type=content_type, | ||
| headers={**common_headers, "Content-Length": "0"}, | ||
| ) | ||
|
|
||
| parsed_range = _parse_range_header(range_header, total_size) if range_header else None | ||
| if range_header and parsed_range is None: | ||
| return StreamingResponse( | ||
| iter([]), | ||
| status_code=HTTPStatus.REQUESTED_RANGE_NOT_SATISFIABLE, | ||
| headers={"Content-Range": f"bytes */{total_size}"}, | ||
| ) | ||
|
|
||
| try: | ||
| if parsed_range: | ||
| start, end = parsed_range | ||
| stream = get_preview_stream(actual_name, start, end) | ||
| return StreamingResponse( | ||
| stream.iter_chunks(chunk_size=64 * 1024), | ||
| status_code=HTTPStatus.PARTIAL_CONTENT, | ||
| media_type=content_type, | ||
| background=BackgroundTask(stream.close), | ||
| headers={ | ||
| **common_headers, | ||
| "Content-Range": f"bytes {start}-{end}/{total_size}", | ||
| "Content-Length": str(end - start + 1), | ||
| }, | ||
| ) | ||
|
|
||
| stream = get_preview_stream(actual_name) | ||
| return StreamingResponse( | ||
| stream.iter_chunks(chunk_size=64 * 1024), | ||
| status_code=HTTPStatus.OK, | ||
| media_type=content_type, | ||
| background=BackgroundTask(stream.close), | ||
| headers={**common_headers, "Content-Length": str(total_size)}, | ||
| ) | ||
| except Exception as e: | ||
| logger.error("Failed to stream share asset preview: %s", str(e), exc_info=True) | ||
|
Check failure on line 201 in backend/apps/conversation_share_app.py
|
||
| raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to stream asset") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| from datetime import datetime | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weak Share Token Share token uses uuid.uuid4().hex + uuid.uuid4().hex[:16]. For public URLs granting file access, use secrets.token_urlsafe(32) for 256 bits of entropy.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asset ID Enumeration get_share_asset_service validates share_token is active and asset_id exists but asset_ids use same uuid4 pattern. Attacker could enumerate asset_ids across shares. |
||
| from typing import Any, Dict, List, Optional | ||
|
|
||
| from sqlalchemy import select, update | ||
|
|
||
| from database.client import as_dict, filter_property, get_db_session | ||
| from database.db_models import ConversationShare, ConversationShareAsset | ||
|
|
||
|
|
||
| def create_conversation_share(share_data: Dict[str, Any], user_id: str) -> Dict[str, Any]: | ||
| with get_db_session() as session: | ||
| payload = filter_property(share_data, ConversationShare) | ||
| payload["created_by"] = user_id | ||
| payload["updated_by"] = user_id | ||
| record = ConversationShare(**payload) | ||
| session.add(record) | ||
| session.flush() | ||
| session.refresh(record) | ||
| return as_dict(record) | ||
|
|
||
|
|
||
| def create_conversation_share_assets( | ||
| share_token: str, | ||
| assets: List[Dict[str, Any]], | ||
| user_id: str, | ||
| ) -> List[Dict[str, Any]]: | ||
| if not assets: | ||
| return [] | ||
|
|
||
| with get_db_session() as session: | ||
| records = [] | ||
| for asset in assets: | ||
| payload = filter_property(asset, ConversationShareAsset) | ||
| payload["share_token"] = share_token | ||
| payload["created_by"] = user_id | ||
| payload["updated_by"] = user_id | ||
| record = ConversationShareAsset(**payload) | ||
| session.add(record) | ||
| records.append(record) | ||
| session.flush() | ||
| for record in records: | ||
| session.refresh(record) | ||
| return [as_dict(record) for record in records] | ||
|
|
||
|
|
||
| def get_active_conversation_share(share_token: str) -> Optional[Dict[str, Any]]: | ||
| with get_db_session() as session: | ||
| stmt = select(ConversationShare).where( | ||
| ConversationShare.share_token == share_token, | ||
| ConversationShare.delete_flag == "N", | ||
| ConversationShare.status == "active", | ||
| ) | ||
| record = session.scalars(stmt).first() | ||
| if record is None: | ||
| return None | ||
|
|
||
| data = as_dict(record) | ||
| expire_time = data.get("expire_time") | ||
| if expire_time: | ||
| if isinstance(expire_time, str): | ||
| expire_time = datetime.fromisoformat(expire_time) | ||
| if expire_time < datetime.now(): | ||
| return None | ||
| return data | ||
|
|
||
|
|
||
| def get_share_asset(share_token: str, asset_id: str) -> Optional[Dict[str, Any]]: | ||
| with get_db_session() as session: | ||
| stmt = select(ConversationShareAsset).where( | ||
| ConversationShareAsset.share_token == share_token, | ||
| ConversationShareAsset.asset_id == asset_id, | ||
| ConversationShareAsset.delete_flag == "N", | ||
| ) | ||
| record = session.scalars(stmt).first() | ||
| return None if record is None else as_dict(record) | ||
|
|
||
|
|
||
| def revoke_conversation_share(share_token: str, user_id: str) -> bool: | ||
| with get_db_session() as session: | ||
| stmt = update(ConversationShare).where( | ||
| ConversationShare.share_token == share_token, | ||
| ConversationShare.created_by == user_id, | ||
| ConversationShare.delete_flag == "N", | ||
| ).values(status="revoked", updated_by=user_id) | ||
| result = session.execute(stmt) | ||
| return result.rowcount > 0 | ||
Uh oh!
There was an error while loading. Please reload this page.