Fix WebSocket authentication to use configurable authentication header

.github/copilot-instructions.md

@@ -61,7 +61,7 @@ frontend/ React 19 + Vite + Tailwind; state via contexts (Chat/WS/Marketplace)
 - Use uv; do not use npm run dev; do not use uvicorn --reload.
 - File naming: avoid generic names (utils.py, helpers.py). Prefer descriptive names; backend/main.py is the entry-point exception.
 - No emojis in code or docs. Prefer files ≤ ~400 lines when practical.
-- Auth assumption: in prod, reverse proxy injects X-Authenticated-User; dev falls back to test user.
+- Auth assumption: in prod, reverse proxy injects X-User-Email (after stripping client headers); dev falls back to test user.
 
 ## Extend by example
 - Add a tool server: edit config/overrides/mcp.json (set groups, transport, url/command, compliance_level). Restart or call discovery on startup.

backend/main.py

@@ -23,6 +23,7 @@
 from core.security_headers_middleware import SecurityHeadersMiddleware
 from core.otel_config import setup_opentelemetry
 from core.utils import sanitize_for_logging
+from core.auth import get_user_from_header
 
 # Import from infrastructure
 from infrastructure.app_factory import app_factory
@@ -186,30 +187,48 @@
     2. Reverse proxy intercepts WebSocket handshake (HTTP Upgrade request)
     3. Reverse proxy delegates to authentication service
     4. Auth service validates JWT/session from cookies or headers
-    5. If valid: Auth service returns X-Authenticated-User header
-    6. Reverse proxy forwards connection to this app with X-Authenticated-User header
+    5. If valid: Auth service returns X-User-Email header
+    6. Reverse proxy forwards connection to this app with X-User-Email header
     7. This app trusts the header (already validated by auth service)
 
     SECURITY REQUIREMENTS:
     - This app MUST ONLY be accessible via reverse proxy
     - Direct public access to this app bypasses authentication
     - Use network isolation to prevent direct access
     - The /login endpoint lives in the separate auth service
+    - Reverse proxy MUST strip client-provided X-User-Email headers before adding its own
+      (otherwise attackers can inject headers: X-User-Email: [email protected])
 
     DEVELOPMENT vs PRODUCTION:
-    - Production: Extracts user from X-Authenticated-User header (set by reverse proxy)
+    - Production: Extracts user from X-User-Email header (set by reverse proxy)
     - Development: Falls back to 'user' query parameter (INSECURE, local only)
 
     See docs/security_architecture.md for complete architecture details.
     """
     await websocket.accept()
 
-    # Basic auth: derive user from query parameters or use test user
-    user_email = websocket.query_params.get('user')
+    # Extract user email using the same authentication flow as HTTP requests
+    # Priority: 1) X-User-Email header (production), 2) query param (dev), 3) test user (dev fallback)
+    config_manager = app_factory.get_config_manager()
+    user_email = None
+
+    # Check X-User-Email header first (consistent with AuthMiddleware)
+    x_email_header = websocket.headers.get('X-User-Email')
+    if x_email_header:
+
+        user_email = get_user_from_header(x_email_header)
+        logger.info(f"WebSocket authenticated via X-User-Email header: {sanitize_for_logging(user_email)}")
+
+    # Fallback to query parameter for backward compatibility (development/testing)
+    if not user_email:
+        user_email = websocket.query_params.get('user')
+        if user_email:
+            logger.info(f"WebSocket authenticated via query parameter: {sanitize_for_logging(user_email)}")
+
+    # Final fallback to test user (development mode only)
     if not user_email:
-        # Fallback to test user or require auth
-        config_manager = app_factory.get_config_manager()
         user_email = config_manager.app_settings.test_user or '[email protected]'
+        logger.info(f"WebSocket using fallback test user: {sanitize_for_logging(user_email)}")
 
     session_id = uuid4()
 

backend/tests/test_issue_access_denied_fix.py

@@ -0,0 +1,136 @@
+"""
+Integration test demonstrating the fix for the access denied issue.
+
+This test simulates the exact scenario from the issue:
+- A file belongs to user '[email protected]'
+- WebSocket connection is authenticated as '[email protected]' via X-User-Email header
+- Attaching the file should succeed (not fail with "Access denied")
+"""
+
+import pytest
+import base64
+import uuid
+from unittest.mock import MagicMock, AsyncMock, patch
+from fastapi.testclient import TestClient
+
+from main import app
+
+
+@pytest.fixture
+def mock_components():
+    """Mock all components needed for the test."""
+    with patch('main.app_factory') as mock_factory:
+        # Mock config
+        mock_config = MagicMock()
+        mock_config.app_settings.test_user = '[email protected]'
+        mock_factory.get_config_manager.return_value = mock_config
+
+        # Mock file manager with S3 client
+        mock_file_manager = MagicMock()
+        mock_s3_client = MagicMock()
+
+        # Simulate a file that belongs to [email protected]
+        async def mock_get_file(user_email, s3_key):
+            """Mock S3 get_file that enforces user prefix check."""
+            # This is the actual check from s3_client.py line 185
+            if not s3_key.startswith(f"users/{user_email}/"):
+                raise Exception("Access denied to file")
+
+            # If user matches, return file metadata
+            return {
+                "key": s3_key,
+                "filename": "mypdf.pdf",
+                "content_base64": base64.b64encode(b"test content").decode(),
+                "content_type": "application/pdf",
+                "size": 100,
+                "etag": "test-etag"
+            }
+
+        mock_s3_client.get_file = AsyncMock(side_effect=mock_get_file)
+        mock_file_manager.s3_client = mock_s3_client
+
+        # Mock chat service
+        mock_chat_service = MagicMock()
+        mock_chat_service.handle_attach_file = AsyncMock(return_value={
+            'type': 'file_attach',
+            'success': True,
+            'filename': 'mypdf.pdf'
+        })
+        mock_chat_service.end_session = MagicMock()
+        mock_factory.create_chat_service.return_value = mock_chat_service
+
+        yield {
+            'factory': mock_factory,
+            'config': mock_config,
+            'file_manager': mock_file_manager,
+            'chat_service': mock_chat_service
+        }
+
+
+def test_issue_scenario_fixed_with_correct_user(mock_components):
+    """
+    Test the exact scenario from the issue, demonstrating the fix.
+    
+    Before fix:
+    - WebSocket would use [email protected] (from fallback)
+    - Attempting to access users/[email protected]/generated/file.pdf would fail
+    - Error: "Access denied: [email protected] attempted to access users/[email protected]/..."
+    
+    After fix:
+    - WebSocket uses [email protected] (from X-User-Email header)
+    - Accessing users/[email protected]/generated/file.pdf succeeds
+    """
+    client = TestClient(app)
+
+    # Simulate the production scenario: reverse proxy sets X-User-Email header
+    actual_user = "[email protected]"
+    file_key = f"users/{actual_user}/generated/1234567890_mypdf.pdf"
+
+    # Connect with X-User-Email header (as set by reverse proxy)
+    with client.websocket_connect("/ws", headers={"X-User-Email": actual_user}):
+        # Verify the connection was created with the correct user
+        call_args = mock_components['factory'].create_chat_service.call_args
+        connection_adapter = call_args[0][0]
+
+        # This should be the actual user, not [email protected]
+        assert connection_adapter.user_email == actual_user, (
+            f"Expected user to be {actual_user}, but got {connection_adapter.user_email}. "
+            "This would cause 'Access denied' errors when accessing user's files."
+        )
+
+
+def test_issue_scenario_would_fail_without_header():
+    """
+    Demonstrate that without the header, the old behavior (test user fallback) occurs.
+    This test shows why the issue existed in the first place.
+    """
+    with patch('main.app_factory') as mock_factory:
+        # Mock config
+        mock_config = MagicMock()
+        mock_config.app_settings.test_user = '[email protected]'
+        mock_factory.get_config_manager.return_value = mock_config
+
+        # Mock chat service
+        mock_chat_service = MagicMock()
+        mock_chat_service.end_session = MagicMock()
+        mock_factory.create_chat_service.return_value = mock_chat_service
+
+        client = TestClient(app)
+
+        # Connect WITHOUT X-User-Email header (simulating old behavior or dev mode)
+        with client.websocket_connect("/ws"):
+            call_args = mock_factory.create_chat_service.call_args
+            connection_adapter = call_args[0][0]
+
+            # Without header, it falls back to test user
+            assert connection_adapter.user_email == '[email protected]', (
+                "Without X-User-Email header, should fall back to test user"
+            )
+
+            # This would cause access denied when trying to access:
+            # users/[email protected]/generated/file.pdf
+            # because connection is authenticated as [email protected]
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])