From 84e4b6c16308397ac3faf3f0f744e2acbc03bf62 Mon Sep 17 00:00:00 2001 From: Anthony Date: Fri, 21 Nov 2025 04:55:46 +0000 Subject: [PATCH 1/3] Fix WebSocket authentication to use configurable header WebSocket connections were falling back to test user instead of reading the authenticated user from the configured authentication header set by the reverse proxy. This caused file access checks to fail with 'Access denied: test@test.com attempted to access users/{actual_user}/...' when users tried to load their own files. Changes: - WebSocket endpoint now uses config.app_settings.auth_user_header - Consistent with HTTP endpoints via AuthMiddleware - Falls back to query parameter for dev/test compatibility - Added logging to identify authentication source Tests: - test_websocket_auth_header.py: Header-based auth, query param fallback - test_issue_access_denied_fix.py: Integration test for the bug scenario - test_security_header_injection.py: Header injection security tests Documentation: - docs/example/nginx.config: Production-ready nginx configuration - docs/example/reverse-proxy-examples.md: Nginx/Apache setup guide - Updated security architecture with header injection prevention Fixes #45 --- .github/copilot-instructions.md | 2 +- backend/main.py | 27 +- backend/tests/test_issue_access_denied_fix.py | 136 ++++++++ .../tests/test_security_header_injection.py | 191 +++++++++++ backend/tests/test_websocket_auth_header.py | 109 +++++++ docs/archive/security_architecture.md | 82 ++++- docs/example/nginx.config | 164 ++++++++++ docs/example/reverse-proxy-examples.md | 308 ++++++++++++++++++ 8 files changed, 997 insertions(+), 22 deletions(-) create mode 100644 backend/tests/test_issue_access_denied_fix.py create mode 100644 backend/tests/test_security_header_injection.py create mode 100644 backend/tests/test_websocket_auth_header.py create mode 100644 docs/example/nginx.config create mode 100644 docs/example/reverse-proxy-examples.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4249631..2869fc8 100644 --- a/.github/copilot-instructions.md +++ b/.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. diff --git a/backend/main.py b/backend/main.py index 7bf2ce1..c87a193 100644 --- a/backend/main.py +++ b/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 @@ -209,6 +210,8 @@ async def websocket_endpoint(websocket: WebSocket): - 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: admin@company.com) DEVELOPMENT vs PRODUCTION: - Production: Extracts user from configured auth header (set by reverse proxy) @@ -218,12 +221,28 @@ async def websocket_endpoint(websocket: WebSocket): """ 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) configured auth header (production), 2) query param (dev), 3) test user (dev fallback) + config_manager = app_factory.get_config_manager() + user_email = None + + # Check configured auth header first (consistent with AuthMiddleware) + auth_header_name = config_manager.app_settings.auth_user_header + x_email_header = websocket.headers.get(auth_header_name) + if x_email_header: + user_email = get_user_from_header(x_email_header) + logger.info("WebSocket authenticated via %s header: %s", auth_header_name, 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("WebSocket authenticated via query parameter: %s", 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 'test@test.com' + logger.info(f"WebSocket using fallback test user: {sanitize_for_logging(user_email)}") session_id = uuid4() diff --git a/backend/tests/test_issue_access_denied_fix.py b/backend/tests/test_issue_access_denied_fix.py new file mode 100644 index 0000000..9c37813 --- /dev/null +++ b/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 'agarlan@sandia.gov' +- WebSocket connection is authenticated as 'agarlan@sandia.gov' via X-User-Email header +- Attaching the file should succeed (not fail with "Access denied") +""" + +import base64 +import pytest +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 = 'test@test.com' + mock_config.app_settings.auth_user_header = 'X-User-Email' + 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 agarlan@sandia.gov + 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 test@test.com (from fallback) + - Attempting to access users/agarlan@sandia.gov/generated/file.pdf would fail + - Error: "Access denied: test@test.com attempted to access users/agarlan@sandia.gov/..." + + After fix: + - WebSocket uses agarlan@sandia.gov (from X-User-Email header) + - Accessing users/agarlan@sandia.gov/generated/file.pdf succeeds + """ + client = TestClient(app) + + # Simulate the production scenario: reverse proxy sets X-User-Email header + actual_user = "agarlan@sandia.gov" + + # 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 test@test.com + 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 = 'test@test.com' + mock_config.app_settings.auth_user_header = 'X-User-Email' + 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 == 'test@test.com', ( + "Without X-User-Email header, should fall back to test user" + ) + + # This would cause access denied when trying to access: + # users/agarlan@sandia.gov/generated/file.pdf + # because connection is authenticated as test@test.com + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/backend/tests/test_security_header_injection.py b/backend/tests/test_security_header_injection.py new file mode 100644 index 0000000..6ed1dd0 --- /dev/null +++ b/backend/tests/test_security_header_injection.py @@ -0,0 +1,191 @@ +""" +Test header injection vulnerabilities. + +This test suite demonstrates the header injection attack vector and +documents why reverse proxy configuration is critical. +""" +import pytest +from fastapi.testclient import TestClient +from main import app + + +client = TestClient(app) + + +def test_direct_access_header_injection_vulnerability(): + """ + SECURITY WARNING: This test demonstrates a CRITICAL vulnerability. + + When the app is accessed DIRECTLY (bypassing reverse proxy), + attackers can inject X-User-Email headers to impersonate any user. + + This test documents the vulnerability. In production: + - Main app MUST be network-isolated (not publicly accessible) + - ALL traffic MUST go through reverse proxy + - Reverse proxy MUST strip client X-User-Email headers + + This test will PASS because the app is designed to trust headers + when behind a properly configured reverse proxy. The test serves + as documentation of the security requirement. + """ + # Attacker tries to impersonate admin by injecting header + response = client.get( + "/api/config", + headers={"X-User-Email": "attacker-pretending-to-be-admin@evil.com"} + ) + + # In direct access mode (no proxy), the app trusts this header + # This is why network isolation is CRITICAL + assert response.status_code == 200 + + # The app will treat this as a legitimate request from the attacker's email + # In production, this request should NEVER reach the app (network isolation) + + +def test_websocket_header_injection_vulnerability(): + """ + Demonstrates WebSocket header injection vulnerability with direct access. + + This shows why the reverse proxy MUST strip X-User-Email headers + before adding the authenticated user's header. + """ + # Attacker connects with injected header + with client.websocket_connect( + "/ws", + headers={"X-User-Email": "attacker@evil.com"} + ) as websocket: + # Connection succeeds because app trusts the header + # This is the EXPECTED behavior when behind a proxy that strips headers + # This is VULNERABLE behavior when directly accessible + + # Send a test message + websocket.send_json({ + "type": "chat", + "content": "test message" + }) + + # The WebSocket will use "attacker@evil.com" as the user + # This demonstrates why network isolation is critical + + +def test_multiple_headers_first_wins(): + """ + Demonstrates the danger of improperly configured reverse proxies. + + If the reverse proxy adds X-User-Email without stripping the client's + version first, both headers arrive. Most frameworks (including FastAPI) + return the FIRST header, allowing the attacker to win. + + Proper nginx config: + proxy_set_header X-User-Email ""; # Strip first! + proxy_set_header X-User-Email $authenticated_user; # Then add + + Vulnerable nginx config: + proxy_set_header X-User-Email $authenticated_user; # Only adds, doesn't strip! + """ + # Simulate what happens when proxy doesn't strip headers + # We can't easily test multiple headers with TestClient, + # but we document the expected behavior + + # When client sends: X-User-Email: attacker@evil.com + # And proxy adds: X-User-Email: realuser@example.com + # The app receives BOTH headers + + # FastAPI's request.headers.get() returns the FIRST occurrence + # So the attacker's header would win! + + # This test documents the requirement for header stripping + assert True, "Documented: Proxy must strip headers first" + + +@pytest.mark.skip(reason="Requires production environment with reverse proxy") +def test_production_header_stripping(): + """ + Test to run in production/staging to verify header stripping works. + + This test should be run manually against the actual deployment to verify + that the reverse proxy properly strips client-provided headers. + + Usage: + 1. Deploy to staging/production with reverse proxy + 2. Get a valid authentication token/cookie + 3. Run this test against the deployed URL + 4. Verify logs show the REAL user, not the injected one + + Expected behavior: + - Request includes malicious X-User-Email header + - Reverse proxy strips it + - Reverse proxy adds real authenticated user header + - Backend receives only the real user header + - Logs confirm backend saw the real user + """ + import os + import requests + + deployment_url = os.getenv("PRODUCTION_URL") + auth_cookie = os.getenv("VALID_AUTH_COOKIE") + + if not deployment_url or not auth_cookie: + pytest.skip("Set PRODUCTION_URL and VALID_AUTH_COOKIE env vars") + + # Try to inject a malicious header + response = requests.get( + f"{deployment_url}/api/config", + headers={"X-User-Email": "attacker@evil.com"}, + cookies={"session": auth_cookie} + ) + + assert response.status_code == 200 + + # Manual verification required: + # Check backend logs to confirm it received the REAL user from auth, + # not the injected "attacker@evil.com" + print("✓ Request succeeded") + print("⚠ MANUAL VERIFICATION REQUIRED:") + print(" Check backend logs to confirm user was NOT 'attacker@evil.com'") + print(" The backend should have received the real authenticated user") + + +def test_header_injection_documentation(): + """ + Documentation test: Lists all security requirements for production deployment. + + This test always passes but serves as executable documentation of the + security requirements needed to prevent header injection attacks. + """ + security_requirements = [ + "Main app MUST be network-isolated (not publicly accessible)", + "ALL traffic MUST flow through reverse proxy", + "Reverse proxy MUST strip client-provided X-User-Email headers", + "Reverse proxy MUST add X-User-Email header AFTER stripping client headers", + "Direct access to main app ports MUST be blocked by firewall/VPC", + "Nginx config MUST include: proxy_set_header X-User-Email '' before setting it", + "Apache config MUST include: RequestHeader unset X-User-Email before setting it", + "Network isolation MUST be tested (attempt direct access should fail)", + "Header injection test MUST be run in production (test_production_header_stripping)", + "Deployment checklist in docs/reverse-proxy-examples.md MUST be completed", + ] + + for i, requirement in enumerate(security_requirements, 1): + print(f"{i}. {requirement}") + + assert True, "Security requirements documented" + + +# Additional test to verify the current behavior +def test_x_user_email_header_is_used(): + """ + Verifies that X-User-Email header is properly extracted. + + This is the expected behavior when behind a properly configured proxy. + """ + test_user = "alice@example.com" + + response = client.get( + "/api/config", + headers={"X-User-Email": test_user} + ) + + assert response.status_code == 200 + # The middleware should have processed this header + # In production, this header comes from the reverse proxy, not the client diff --git a/backend/tests/test_websocket_auth_header.py b/backend/tests/test_websocket_auth_header.py new file mode 100644 index 0000000..da982e0 --- /dev/null +++ b/backend/tests/test_websocket_auth_header.py @@ -0,0 +1,109 @@ +""" +Tests for WebSocket authentication using the configured authentication header. + +These tests validate that the backend correctly extracts the user email from the +configured authentication header (default: X-User-Email) for WebSocket connections, +which is critical for the production authentication flow where the reverse proxy +sets this header. The tests also ensure that fallback mechanisms (query parameter, +test user from config) work as expected, and that the header takes precedence when +both are present. +""" + +import pytest +from fastapi.testclient import TestClient +from unittest.mock import AsyncMock, MagicMock, patch + +from main import app + + +@pytest.fixture +def mock_app_factory(): + """Mock app factory to avoid initializing full application.""" + with patch('main.app_factory') as mock_factory: + # Mock config manager + mock_config = MagicMock() + mock_config.app_settings.test_user = 'test@test.com' + mock_config.app_settings.debug_mode = False + mock_config.app_settings.auth_user_header = 'X-User-Email' + mock_factory.get_config_manager.return_value = mock_config + + # Mock chat service + mock_chat_service = MagicMock() + mock_chat_service.handle_chat_message = AsyncMock(return_value={}) + mock_chat_service.handle_attach_file = AsyncMock(return_value={'type': 'file_attach', 'success': True}) + mock_chat_service.end_session = MagicMock() + mock_factory.create_chat_service.return_value = mock_chat_service + + yield mock_factory + + +def test_websocket_uses_x_user_email_header(mock_app_factory): + """Test that WebSocket connection uses X-User-Email header for authentication.""" + client = TestClient(app) + + # Connect with X-User-Email header + with client.websocket_connect("/ws", headers={"X-User-Email": "alice@example.com"}) as websocket: + # Send a test message + websocket.send_json({"type": "attach_file", "s3_key": "users/alice@example.com/test.txt"}) + + # Verify that the connection was created with the correct user from header + # The user_email should be extracted from X-User-Email header + call_args = mock_app_factory.create_chat_service.call_args + connection_adapter = call_args[0][0] # First positional argument + + # The connection adapter should have been created with alice@example.com + assert connection_adapter.user_email == "alice@example.com" + + +def test_websocket_fallback_to_query_param(mock_app_factory): + """Test that WebSocket falls back to query parameter if header not present.""" + client = TestClient(app) + + # Connect without header but with query param + with client.websocket_connect("/ws?user=bob@example.com") as websocket: + # Send a test message + websocket.send_json({"type": "attach_file", "s3_key": "users/bob@example.com/test.txt"}) + + # Get the chat service instance + call_args = mock_app_factory.create_chat_service.call_args + connection_adapter = call_args[0][0] + + # Should use query param + assert connection_adapter.user_email == "bob@example.com" + + +def test_websocket_fallback_to_test_user(mock_app_factory): + """Test that WebSocket falls back to test user if neither header nor query param present.""" + client = TestClient(app) + + # Connect without header or query param + with client.websocket_connect("/ws") as websocket: + # Send a test message + websocket.send_json({"type": "attach_file", "s3_key": "users/test@test.com/test.txt"}) + + # Get the chat service instance + call_args = mock_app_factory.create_chat_service.call_args + connection_adapter = call_args[0][0] + + # Should use test user from config + assert connection_adapter.user_email == "test@test.com" + + +def test_websocket_header_takes_precedence_over_query_param(mock_app_factory): + """Test that X-User-Email header takes precedence over query parameter.""" + client = TestClient(app) + + # Connect with both header and query param (header should win) + with client.websocket_connect( + "/ws?user=wrong@example.com", + headers={"X-User-Email": "correct@example.com"} + ) as websocket: + # Send a test message + websocket.send_json({"type": "attach_file", "s3_key": "users/correct@example.com/test.txt"}) + + # Get the chat service instance + call_args = mock_app_factory.create_chat_service.call_args + connection_adapter = call_args[0][0] + + # Should use header, not query param + assert connection_adapter.user_email == "correct@example.com" diff --git a/docs/archive/security_architecture.md b/docs/archive/security_architecture.md index a6e292c..25278b4 100644 --- a/docs/archive/security_architecture.md +++ b/docs/archive/security_architecture.md @@ -46,7 +46,7 @@ Internet → Reverse Proxy → Authentication Service 2. Reverse Proxy → Auth Service (validates during handshake) 3. If invalid → Connection rejected (HTTP 401) 4. If valid → Auth Service returns user identity header -5. Reverse Proxy → Main App (with X-Authenticated-User header) +5. Reverse Proxy → Main App (with X-User-Email header) 6. Main App accepts WebSocket connection 7. All subsequent messages occur over established connection ``` @@ -61,7 +61,7 @@ Internet → Reverse Proxy → Authentication Service ### Header-Based Trust -The main application trusts the `X-Authenticated-User` header because: +The main application trusts the `X-User-Email` header because: 1. **Network Isolation**: Main app is not publicly accessible 2. **Single Entry Point**: Only reverse proxy can reach main app @@ -73,7 +73,7 @@ The main application trusts the `X-Authenticated-User` header because: When examining this codebase in isolation, the WebSocket endpoint appears to lack authentication: ```python -user_email = websocket.headers.get('X-Authenticated-User') +user_email = websocket.headers.get('X-User-Email') ``` This is **intentional by design**. The security controls exist in the infrastructure layer, not the application layer. @@ -104,7 +104,7 @@ Production deployments MUST: 1. Deploy reverse proxy with auth delegation 2. Deploy separate authentication service 3. Isolate main app from public access -4. Configure reverse proxy to set X-Authenticated-User header +4. Configure reverse proxy to set X-User-Email header (after stripping client headers) 5. Never expose main app ports publicly ### Example Network Configuration @@ -227,12 +227,43 @@ Since WebSocket authentication happens only at handshake: ### Header Injection Prevention -**Risk:** If main app is publicly accessible, attackers can inject headers +**Risk:** Attackers can inject X-User-Email headers to impersonate users -**Mitigation:** -- Network isolation (main app not reachable publicly) -- Reverse proxy strips client-provided headers -- Only reverse proxy can set X-Authenticated-User +**Attack Scenario:** +```bash +# Attacker sends malicious header +curl -H "X-User-Email: admin@company.com" https://your-app.com/api/config +``` + +**Two-Layer Mitigation Required:** + +1. **Network Isolation** (Primary Defense) + - Main app MUST NOT be publicly accessible + - Only reverse proxy can reach main app + - Use Docker networks, VPCs, or firewall rules + +2. **Header Stripping** (Defense in Depth) + - Reverse proxy MUST strip client X-User-Email headers + - Then add authenticated user header + - Prevents injection even if proxy is misconfigured + +**Critical:** Even with a reverse proxy, if it doesn't strip client headers first, attackers can inject headers that arrive before the proxy's header (first header wins in most frameworks). + +**Nginx Example (Secure):** +```nginx +location /ws { + auth_request /auth/validate; + auth_request_set $authenticated_user $upstream_http_x_user_email; + + # CRITICAL: Strip client headers first + proxy_set_header X-User-Email ""; + proxy_set_header X-User-Email $authenticated_user; + + proxy_pass http://main-app:8000; +} +``` + +See `docs/reverse-proxy-examples.md` for complete configuration examples. ### Defense in Depth @@ -249,16 +280,33 @@ Additional security layers: Before deploying to production: -- [ ] Main application is NOT publicly accessible -- [ ] Reverse proxy is configured with auth delegation +**Network Security:** +- [ ] Main application is NOT publicly accessible (test: `curl http://main-app:8000` should timeout from internet) +- [ ] Reverse proxy is the only public-facing component +- [ ] Network isolation is enforced (Docker networks, VPCs, firewall rules) +- [ ] Direct access to main app ports is blocked (verify with nmap/telnet from outside network) + +**Authentication & Headers:** +- [ ] Reverse proxy is configured with auth delegation to auth service - [ ] Authentication service is deployed and tested -- [ ] Network isolation is enforced (firewall rules, VPC, etc.) -- [ ] TLS certificates are valid and renewed -- [ ] WebSocket upgrade is properly proxied -- [ ] X-Authenticated-User header is set by reverse proxy -- [ ] Client-provided headers are stripped +- [ ] X-User-Email header is set by reverse proxy after authentication +- [ ] **CRITICAL:** Client-provided X-User-Email headers are explicitly stripped before proxy adds its own + - Nginx: Verify `proxy_set_header X-User-Email "";` appears BEFORE setting the authenticated header + - Apache: Verify `RequestHeader unset X-User-Email` appears BEFORE setting the authenticated header +- [ ] Header injection test passed (run `pytest backend/tests/test_security_header_injection.py::test_production_header_stripping`) +- [ ] Backend logs confirm authenticated user is received (not client-provided values) + +**SSL/TLS & WebSocket:** +- [ ] TLS certificates are valid and auto-renewal is configured +- [ ] WebSocket upgrade is properly proxied (test WebSocket connection through proxy) +- [ ] WebSocket authentication works during handshake (test with invalid credentials) + +**Monitoring & Testing:** - [ ] Logging and monitoring are configured - [ ] Token expiration and refresh are tested +- [ ] Review security architecture documentation (docs/archive/security_architecture.md) +- [ ] Review reverse proxy configuration examples (docs/reverse-proxy-examples.md) +- [ ] Security tests pass: `pytest backend/tests/test_security_*.py` ## Testing Authentication @@ -288,7 +336,7 @@ Before deploying to production: curl -i --no-buffer \ -H "Connection: Upgrade" \ -H "Upgrade: websocket" \ - -H "X-Authenticated-User: attacker@example.com" \ + -H "X-User-Email: attacker@example.com" \ http://main-app:8000/ws # Should NOT be reachable from outside network ``` diff --git a/docs/example/nginx.config b/docs/example/nginx.config new file mode 100644 index 0000000..ebbd703 --- /dev/null +++ b/docs/example/nginx.config @@ -0,0 +1,164 @@ +# Atlas UI 3 - Secure Nginx Configuration +# Last Updated: 2025-11-10 +# +# SECURITY REQUIREMENTS: +# - This configuration includes header stripping to prevent header injection attacks +# - Each location that proxies to main-app MUST strip X-User-Email before setting it +# - Without header stripping, attackers can send X-User-Email: admin@company.com and bypass auth +# +# For more details, see docs/reverse-proxy-examples.md + +# Upstream names now use the APP_NAME prefix +upstream __APP_NAME__-main-app { + server __APP_NAME__-main-app:8000; +} + +upstream __APP_NAME__-auth-app { + server __APP_NAME__-main-app:5000; +} + +server { + listen 8080 default_server; + listen [::]:8080 default_server; + server_tokens off; + + # Don't add the internal port (8080) to redirects + port_in_redirect off; + + # Route /login and /getatoken (case-insensitive) to auth-app + # No authentication required for login endpoints + location ~* ^/(login|getatoken)$ { + add_header X-Debug-Location "auth-regex-block" always; + proxy_pass http://__APP_NAME__-auth-app; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_http_version 1.1; + proxy_set_header Connection ""; + } + + # Internal location for auth check + # This endpoint validates authentication and returns X-User-Email header + location = /auth-check { + add_header X-Debug-Location "auth-check-block" always; + internal; + proxy_pass http://__APP_NAME__-auth-app/auth; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header X-Original-URI $request_uri; + proxy_set_header Cookie $http_cookie; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_http_version 1.1; + proxy_set_header Connection ""; + } + + # File download endpoint - requires auth, allows internal IPs + location /api/files/download/ { + # Allow internal IP ranges (optional - remove if not needed) + # satisfy any; + allow 127.0.0.1; + allow ::1; + allow 10.0.0.0/8; + allow 172.16.0.0/12; + allow 192.168.0.0/16; + deny all; + + # Authenticate and get user email + auth_request /auth-check; + auth_request_set $auth_user_email $upstream_http_x_user_email; + error_page 401 = @do_login_redirect; + + # Proxy to main app + proxy_pass http://__APP_NAME__-main-app; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + + # CRITICAL SECURITY: Strip client-provided X-User-Email header first + # Without this, attackers can inject headers to impersonate users + proxy_set_header X-User-Email ""; + # Now set the authenticated user from auth service + proxy_set_header X-User-Email $auth_user_email; + + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_http_version 1.1; + proxy_set_header Connection ""; + } + + # Everything else to main-app, requires authentication + # This includes WebSocket connections to /ws + location / { + add_header X-Debug-Location "main-location-block" always; + + # Authenticate and get user email + auth_request /auth-check; + error_page 401 = @do_login_redirect; + auth_request_set $auth_user_email $upstream_http_x_user_email; + + # Proxy to main app + proxy_pass http://__APP_NAME__-main-app; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + + # CRITICAL SECURITY: Strip client-provided X-User-Email header first + # This prevents header injection attacks where clients send: + # X-User-Email: admin@company.com + # Without stripping, both the client's header AND the auth header arrive, + # and the client's header (first) wins in most frameworks. + proxy_set_header X-User-Email ""; + # Now set the authenticated user from auth service + proxy_set_header X-User-Email $auth_user_email; + + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_http_version 1.1; + + # WebSocket support + proxy_set_header Connection "upgrade"; + proxy_set_header Upgrade $http_upgrade; + proxy_read_timeout 86400; # 24 hours for long-lived WebSocket connections + } + + # Named location to handle the redirect for 401 errors from auth_request + location @do_login_redirect { + return 302 /login; + } +} + +# SECURITY VERIFICATION CHECKLIST: +# ================================ +# Before deploying this configuration: +# +# 1. Verify header stripping is in place: +# - Search for "proxy_set_header X-User-Email" in this file +# - Confirm EVERY occurrence is preceded by: proxy_set_header X-User-Email ""; +# - There should be TWO pairs: one in /api/files/download/, one in / +# +# 2. Test header injection protection: +# curl -H "X-User-Email: attacker@evil.com" \ +# -H "Cookie: valid_session" \ +# https://your-domain.com/api/config +# Then check backend logs - should see REAL user, not "attacker@evil.com" +# +# 3. Test WebSocket authentication: +# - Connect to wss://your-domain.com/ws with valid auth +# - Verify connection succeeds and user is correct +# - Try with invalid auth, verify connection is rejected (401) +# +# 4. Test direct access is blocked: +# curl http://__APP_NAME__-main-app:8000/api/config +# Should timeout or connection refused (network isolation) +# +# 5. Verify auth service is working: +# - Access /login endpoint +# - Verify it returns login page +# - Test valid/invalid credentials +# +# For detailed security documentation, see: +# - docs/reverse-proxy-examples.md +# - docs/archive/security_architecture.md +# - backend/tests/test_security_header_injection.py diff --git a/docs/example/reverse-proxy-examples.md b/docs/example/reverse-proxy-examples.md new file mode 100644 index 0000000..ed49e3f --- /dev/null +++ b/docs/example/reverse-proxy-examples.md @@ -0,0 +1,308 @@ +# Reverse Proxy Configuration Examples + +**Last Updated: 2025-11-10** + +This document provides secure configuration examples for deploying Atlas UI 3 behind a reverse proxy with proper authentication header handling. + +## Critical Security Requirement + +**The reverse proxy MUST strip client-provided authentication headers before adding its own.** + +Without this, attackers can inject headers like `X-User-Email: admin@company.com` and bypass authentication even when the proxy is properly configured. + +## Nginx Configuration + +### Secure Example (RECOMMENDED ✅) + +```nginx +# Atlas UI 3 - Secure Configuration +upstream atlas_backend { + server main-app:8000; +} + +upstream auth_service { + server auth-service:8001; +} + +server { + listen 443 ssl http2; + server_name your-domain.com; + + ssl_certificate /etc/ssl/certs/your-domain.crt; + ssl_certificate_key /etc/ssl/private/your-domain.key; + + # WebSocket endpoint with authentication + location /ws { + # STEP 1: Authenticate via auth service + auth_request /auth/validate; + auth_request_set $authenticated_user $upstream_http_x_user_email; + + # STEP 2: CRITICAL - Strip any X-User-Email headers from client + # This prevents header injection attacks + proxy_set_header X-User-Email ""; + + # STEP 3: Set X-User-Email from authenticated user only + proxy_set_header X-User-Email $authenticated_user; + + # Standard WebSocket proxy settings + proxy_pass http://atlas_backend; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection "upgrade"; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + + # Timeouts for long-lived connections + proxy_read_timeout 3600s; + proxy_send_timeout 3600s; + } + + # HTTP API endpoints with authentication + location /api/ { + auth_request /auth/validate; + auth_request_set $authenticated_user $upstream_http_x_user_email; + + # CRITICAL: Strip client headers before adding authenticated header + proxy_set_header X-User-Email ""; + proxy_set_header X-User-Email $authenticated_user; + + proxy_pass http://atlas_backend; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + } + + # Admin endpoints with authentication + location /admin/ { + auth_request /auth/validate; + auth_request_set $authenticated_user $upstream_http_x_user_email; + + proxy_set_header X-User-Email ""; + proxy_set_header X-User-Email $authenticated_user; + + proxy_pass http://atlas_backend; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + } + + # Static files (no auth required) + location / { + proxy_pass http://atlas_backend; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + } + + # Internal auth validation endpoint (not exposed to clients) + location = /auth/validate { + internal; + proxy_pass http://auth_service/validate; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header X-Original-URI $request_uri; + proxy_set_header Cookie $http_cookie; + proxy_set_header Authorization $http_authorization; + } +} +``` + +### Vulnerable Example (DO NOT USE ❌) + +```nginx +# VULNERABLE: Missing header stripping! +location /ws { + auth_request /auth/validate; + auth_request_set $authenticated_user $upstream_http_x_user_email; + + # DANGER: Only adds header without clearing client's version + # If client sends X-User-Email: attacker@evil.com, BOTH headers arrive! + proxy_set_header X-User-Email $authenticated_user; # ❌ INSECURE + + proxy_pass http://atlas_backend; + # ... rest of config +} +``` + +**Why this is vulnerable:** +1. Client sends: `X-User-Email: admin@evil.com` +2. Proxy adds: `X-User-Email: realuser@example.com` +3. Backend receives BOTH headers +4. `request.headers.get('X-User-Email')` returns the FIRST one (attacker's!) + +## Apache Configuration + +### Secure Example (RECOMMENDED ✅) + +```apache + + ServerName your-domain.com + + SSLEngine on + SSLCertificateFile /etc/ssl/certs/your-domain.crt + SSLCertificateKeyFile /etc/ssl/private/your-domain.key + + # Enable WebSocket proxying + RewriteEngine On + RewriteCond %{HTTP:Upgrade} =websocket [NC] + RewriteRule ^/ws(.*)$ ws://main-app:8000/ws$1 [P,L] + + # WebSocket location + + # Authenticate via external service + AuthType Bearer + AuthName "Atlas UI Authentication" + AuthBasicProvider external-auth-provider + Require valid-user + + # CRITICAL: Remove client-provided header + RequestHeader unset X-User-Email + + # Set header from authenticated user + RequestHeader set X-User-Email %{REMOTE_USER}e + + ProxyPass http://main-app:8000/ws + ProxyPassReverse http://main-app:8000/ws + + # WebSocket settings + ProxyPreserveHost On + ProxyTimeout 3600 + + + # API endpoints + + AuthType Bearer + AuthName "Atlas UI Authentication" + AuthBasicProvider external-auth-provider + Require valid-user + + RequestHeader unset X-User-Email + RequestHeader set X-User-Email %{REMOTE_USER}e + + ProxyPass http://main-app:8000/api/ + ProxyPassReverse http://main-app:8000/api/ + + + # Static files (no auth) + + ProxyPass http://main-app:8000/ + ProxyPassReverse http://main-app:8000/ + + +``` + +## Testing Header Injection Prevention + +### Test 1: Verify Header Stripping + +This test verifies that client-provided headers are stripped: + +```bash +# Try to inject a malicious header through the proxy +curl -i \ + -H "X-User-Email: attacker@evil.com" \ + -H "Cookie: valid_session_token_here" \ + https://your-domain.com/api/config + +# Expected: Should work, but backend receives legitimate user from auth +# Check logs to confirm backend saw the REAL user, not "attacker@evil.com" +``` + +### Test 2: Verify Direct Access is Blocked + +```bash +# Try to connect directly to the backend (bypassing proxy) +curl -i \ + -H "X-User-Email: admin@company.com" \ + http://main-app:8000/api/config + +# Expected: Connection refused or timeout (network isolation) +``` + +### Test 3: WebSocket Header Injection Test + +```python +import websocket +import json + +# Try to inject header during WebSocket handshake +ws = websocket.WebSocket() +ws.connect( + "wss://your-domain.com/ws", + header=["X-User-Email: attacker@evil.com"], + cookie="valid_session_token_here" +) + +# Send a message that reveals the user +ws.send(json.dumps({"type": "chat", "content": "Who am I?"})) +response = json.loads(ws.recv()) + +# Check logs: backend should see the REAL user from auth, not the injected one +ws.close() +``` + +## Deployment Checklist + +Before deploying to production, verify: + +- [ ] Reverse proxy configuration includes explicit header stripping (`proxy_set_header X-User-Email ""` or `RequestHeader unset X-User-Email`) +- [ ] Auth service properly validates credentials before setting header +- [ ] Direct access to main app is blocked at network level (test with curl) +- [ ] WebSocket connections are properly authenticated during handshake +- [ ] Header injection test confirms client headers are stripped (see Test 1 above) +- [ ] Logs confirm backend receives authenticated user, not client-provided headers +- [ ] SSL/TLS is properly configured with valid certificates +- [ ] WebSocket upgrade is working correctly through the proxy + +## Common Mistakes + +### 1. Forgetting to Strip Headers +```nginx +# WRONG: Only adding header +proxy_set_header X-User-Email $authenticated_user; + +# RIGHT: Strip first, then add +proxy_set_header X-User-Email ""; +proxy_set_header X-User-Email $authenticated_user; +``` + +### 2. Wrong Header Order +```nginx +# WRONG: Set then clear (clears the authenticated header!) +proxy_set_header X-User-Email $authenticated_user; +proxy_set_header X-User-Email ""; + +# RIGHT: Clear then set +proxy_set_header X-User-Email ""; +proxy_set_header X-User-Email $authenticated_user; +``` + +### 3. Not Testing Header Injection +Always run the header injection tests above to verify your configuration is secure. + +### 4. Exposing Backend Ports +```yaml +# WRONG: Exposing backend port publicly +services: + main-app: + ports: + - "8000:8000" # ❌ Bypasses proxy! + +# RIGHT: Only expose internally +services: + main-app: + expose: + - "8000" # ✅ Internal network only +``` + +## Additional Resources + +- [Nginx ngx_http_auth_request_module](https://nginx.org/en/docs/http/ngx_http_auth_request_module.html) +- [Apache mod_headers](https://httpd.apache.org/docs/current/mod/mod_headers.html) +- [WebSocket RFC 6455](https://tools.ietf.org/html/rfc6455) +- [OWASP Header Injection Prevention](https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html) From 5397f39ca644236a7aa24eb581f83e3e56ba0fb2 Mon Sep 17 00:00:00 2001 From: Anthony Date: Fri, 21 Nov 2025 05:05:55 +0000 Subject: [PATCH 2/3] feat(ws): enforce proxy secret authentication on WebSocket connections Add mandatory proxy secret check in websocket_endpoint before accepting WebSocket connections, ensuring consistency with HTTP AuthMiddleware security. Raise WebSocketException for invalid secrets to reject unauthorized access. Introduce WebSocketException import and sanitize logging for auth headers and user emails to prevent potential information leakage. This enhances overall system security in production environments. --- backend/main.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/backend/main.py b/backend/main.py index c87a193..d7cd23f 100644 --- a/backend/main.py +++ b/backend/main.py @@ -9,7 +9,7 @@ from pathlib import Path from uuid import uuid4 -from fastapi import FastAPI, WebSocket, WebSocketDisconnect +from fastapi import FastAPI, WebSocket, WebSocketDisconnect, WebSocketException from fastapi.staticfiles import StaticFiles from fastapi.responses import FileResponse from dotenv import load_dotenv @@ -219,11 +219,27 @@ async def websocket_endpoint(websocket: WebSocket): See docs/security_architecture.md for complete architecture details. """ - await websocket.accept() - # Extract user email using the same authentication flow as HTTP requests # Priority: 1) configured auth header (production), 2) query param (dev), 3) test user (dev fallback) config_manager = app_factory.get_config_manager() + + # WebSocket connections must present the shared proxy secret (same as AuthMiddleware) + if ( + config_manager.app_settings.proxy_secret_enabled + and config_manager.app_settings.proxy_secret + and not config_manager.app_settings.debug_mode + ): + proxy_secret_header = config_manager.app_settings.proxy_secret_header + proxy_secret_value = websocket.headers.get(proxy_secret_header) + if proxy_secret_value != config_manager.app_settings.proxy_secret: + logger.warning( + "WS proxy secret mismatch on %s", + sanitize_for_logging(websocket.client) + ) + raise WebSocketException(code=1008, reason="Invalid proxy secret") + + await websocket.accept() + user_email = None # Check configured auth header first (consistent with AuthMiddleware) @@ -231,7 +247,11 @@ async def websocket_endpoint(websocket: WebSocket): x_email_header = websocket.headers.get(auth_header_name) if x_email_header: user_email = get_user_from_header(x_email_header) - logger.info("WebSocket authenticated via %s header: %s", auth_header_name, sanitize_for_logging(user_email)) + logger.info( + "WebSocket authenticated via %s header: %s", + sanitize_for_logging(auth_header_name), + sanitize_for_logging(user_email) + ) # Fallback to query parameter for backward compatibility (development/testing) if not user_email: From eb02ce4cbda68c463f61d724b20a17bd38399c35 Mon Sep 17 00:00:00 2001 From: Anthony Date: Fri, 21 Nov 2025 05:43:05 +0000 Subject: [PATCH 3/3] test(ws): add tests for WebSocket authentication header handling --- backend/main.py | 2 +- backend/tests/test_websocket_auth_header.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/main.py b/backend/main.py index d7cd23f..5de95e6 100644 --- a/backend/main.py +++ b/backend/main.py @@ -225,7 +225,7 @@ async def websocket_endpoint(websocket: WebSocket): # WebSocket connections must present the shared proxy secret (same as AuthMiddleware) if ( - config_manager.app_settings.proxy_secret_enabled + config_manager.app_settings.feature_proxy_secret_enabled and config_manager.app_settings.proxy_secret and not config_manager.app_settings.debug_mode ): diff --git a/backend/tests/test_websocket_auth_header.py b/backend/tests/test_websocket_auth_header.py index da982e0..09ddbd5 100644 --- a/backend/tests/test_websocket_auth_header.py +++ b/backend/tests/test_websocket_auth_header.py @@ -25,6 +25,7 @@ def mock_app_factory(): mock_config.app_settings.test_user = 'test@test.com' mock_config.app_settings.debug_mode = False mock_config.app_settings.auth_user_header = 'X-User-Email' + mock_config.app_settings.feature_proxy_secret_enabled = False mock_factory.get_config_manager.return_value = mock_config # Mock chat service