fix: security review (#656)

* Add security review doc

* Add tests to reproduce security issues

* Fix security issues

* Fix tests

* Set auth auth backend for tests

* Fix ics api tests

* Fix transcript mutate check

* Update frontent env var names

* Remove permissions doc

server/reflector/db/transcripts.py

@@ -647,6 +647,19 @@ class TranscriptController:
         query = transcripts.delete().where(transcripts.c.recording_id == recording_id)
         await get_database().execute(query)
 
+    @staticmethod
+    def user_can_mutate(transcript: Transcript, user_id: str | None) -> bool:
+        """
+        Returns True if the given user is allowed to modify the transcript.
+
+        Policy:
+        - Anonymous transcripts (user_id is None) cannot be modified via API
+        - Only the owner (matching user_id) can modify their transcript
+        """
+        if transcript.user_id is None:
+            return False
+        return user_id and transcript.user_id == user_id
+
     @asynccontextmanager
     async def transaction(self):
         """

server/reflector/views/rooms.py

@@ -199,6 +199,8 @@ async def rooms_get(
     room = await rooms_controller.get_by_id_for_http(room_id, user_id=user_id)
     if not room:
         raise HTTPException(status_code=404, detail="Room not found")
+    if not room.is_shared and (user_id is None or room.user_id != user_id):
+        raise HTTPException(status_code=403, detail="Room access denied")
     return room
 
 
@@ -229,9 +231,9 @@ async def rooms_get_by_name(
 @router.post("/rooms", response_model=Room)
 async def rooms_create(
     room: CreateRoom,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
 
     return await rooms_controller.add(
         name=room.name,
@@ -256,12 +258,14 @@ async def rooms_create(
 async def rooms_update(
     room_id: str,
     info: UpdateRoom,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     room = await rooms_controller.get_by_id_for_http(room_id, user_id=user_id)
     if not room:
         raise HTTPException(status_code=404, detail="Room not found")
+    if room.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
     values = info.dict(exclude_unset=True)
     await rooms_controller.update(room, values)
     return room
@@ -270,12 +274,14 @@ async def rooms_update(
 @router.delete("/rooms/{room_id}", response_model=DeletionStatus)
 async def rooms_delete(
     room_id: str,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
-    room = await rooms_controller.get_by_id(room_id, user_id=user_id)
+    user_id = user["sub"]
+    room = await rooms_controller.get_by_id(room_id)
     if not room:
         raise HTTPException(status_code=404, detail="Room not found")
+    if room.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
     await rooms_controller.remove_by_id(room.id, user_id=user_id)
     return DeletionStatus(status="ok")
 
@@ -339,16 +345,16 @@ async def rooms_create_meeting(
 @router.post("/rooms/{room_id}/webhook/test", response_model=WebhookTestResult)
 async def rooms_test_webhook(
     room_id: str,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
     """Test webhook configuration by sending a sample payload."""
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
 
     room = await rooms_controller.get_by_id(room_id)
     if not room:
         raise HTTPException(status_code=404, detail="Room not found")
 
-    if user_id and room.user_id != user_id:
+    if room.user_id != user_id:
         raise HTTPException(
             status_code=403, detail="Not authorized to test this room's webhook"
         )

server/reflector/views/transcripts.py

@@ -9,8 +9,6 @@ from pydantic import BaseModel, Field, constr, field_serializer
 
 import reflector.auth as auth
 from reflector.db import get_database
-from reflector.db.meetings import meetings_controller
-from reflector.db.rooms import rooms_controller
 from reflector.db.search import (
     DEFAULT_SEARCH_LIMIT,
     SearchLimit,
@@ -344,12 +342,14 @@ async def transcript_get(
 async def transcript_update(
     transcript_id: str,
     info: UpdateTranscript,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if not transcripts_controller.user_can_mutate(transcript, user_id):
+        raise HTTPException(status_code=403, detail="Not authorized")
     values = info.dict(exclude_unset=True)
     updated_transcript = await transcripts_controller.update(transcript, values)
     return updated_transcript
@@ -358,18 +358,14 @@ async def transcript_update(
 @router.delete("/transcripts/{transcript_id}", response_model=DeletionStatus)
 async def transcript_delete(
     transcript_id: str,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id(transcript_id)
     if not transcript:
         raise HTTPException(status_code=404, detail="Transcript not found")
-
-    if transcript.meeting_id:
-        meeting = await meetings_controller.get_by_id(transcript.meeting_id)
-        room = await rooms_controller.get_by_id(meeting.room_id)
-        if room.is_shared:
-            user_id = None
+    if not transcripts_controller.user_can_mutate(transcript, user_id):
+        raise HTTPException(status_code=403, detail="Not authorized")
 
     await transcripts_controller.remove_by_id(transcript.id, user_id=user_id)
     return DeletionStatus(status="ok")
@@ -443,15 +439,16 @@ async def transcript_post_to_zulip(
     stream: str,
     topic: str,
     include_topics: bool,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ):
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
     if not transcript:
         raise HTTPException(status_code=404, detail="Transcript not found")
-
+    if not transcripts_controller.user_can_mutate(transcript, user_id):
+        raise HTTPException(status_code=403, detail="Not authorized")
     content = get_zulip_message(transcript, include_topics)
 
     message_updated = False

server/reflector/views/transcripts_participants.py

@@ -56,12 +56,14 @@ async def transcript_get_participants(
 async def transcript_add_participant(
     transcript_id: str,
     participant: CreateParticipant,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ) -> Participant:
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if transcript.user_id is not None and transcript.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
 
     # ensure the speaker is unique
     if participant.speaker is not None and transcript.participants is not None:
@@ -101,12 +103,14 @@ async def transcript_update_participant(
     transcript_id: str,
     participant_id: str,
     participant: UpdateParticipant,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ) -> Participant:
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if transcript.user_id is not None and transcript.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
 
     # ensure the speaker is unique
     for p in transcript.participants:
@@ -138,11 +142,13 @@ async def transcript_update_participant(
 async def transcript_delete_participant(
     transcript_id: str,
     participant_id: str,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ) -> DeletionStatus:
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if transcript.user_id is not None and transcript.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
     await transcripts_controller.delete_participant(transcript, participant_id)
     return DeletionStatus(status="ok")

server/reflector/views/transcripts_speaker.py

@@ -35,12 +35,14 @@ class SpeakerMerge(BaseModel):
 async def transcript_assign_speaker(
     transcript_id: str,
     assignment: SpeakerAssignment,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ) -> SpeakerAssignmentStatus:
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if transcript.user_id is not None and transcript.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
 
     if not transcript:
         raise HTTPException(status_code=404, detail="Transcript not found")
@@ -113,12 +115,14 @@ async def transcript_assign_speaker(
 async def transcript_merge_speaker(
     transcript_id: str,
     merge: SpeakerMerge,
-    user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Annotated[auth.UserInfo, Depends(auth.current_user)],
 ) -> SpeakerAssignmentStatus:
-    user_id = user["sub"] if user else None
+    user_id = user["sub"]
     transcript = await transcripts_controller.get_by_id_for_http(
         transcript_id, user_id=user_id
     )
+    if transcript.user_id is not None and transcript.user_id != user_id:
+        raise HTTPException(status_code=403, detail="Not authorized")
 
     if not transcript:
         raise HTTPException(status_code=404, detail="Transcript not found")

server/reflector/views/transcripts_websocket.py

@@ -4,8 +4,11 @@ Transcripts websocket API
 
 """
 
-from fastapi import APIRouter, HTTPException, WebSocket, WebSocketDisconnect
+from typing import Optional
 
+from fastapi import APIRouter, Depends, HTTPException, WebSocket, WebSocketDisconnect
+
+import reflector.auth as auth
 from reflector.db.transcripts import transcripts_controller
 from reflector.ws_manager import get_ws_manager
 
@@ -21,10 +24,12 @@ async def transcript_get_websocket_events(transcript_id: str):
 async def transcript_events_websocket(
     transcript_id: str,
     websocket: WebSocket,
-    # user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
+    user: Optional[auth.UserInfo] = Depends(auth.current_user_optional),
 ):
-    # user_id = user["sub"] if user else None
-    transcript = await transcripts_controller.get_by_id(transcript_id)
+    user_id = user["sub"] if user else None
+    transcript = await transcripts_controller.get_by_id_for_http(
+        transcript_id, user_id=user_id
+    )
     if not transcript:
         raise HTTPException(status_code=404, detail="Transcript not found")