From b3f2eafe45a6daf556ee56d99ff3e56ddea55e6f Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Wed, 29 Jul 2026 23:27:14 +0200 Subject: [PATCH 1/3] feat(webserver): add rate limiting to prevent brute-force and DoS attacks - Add Flask-Limiter dependency for rate limiting - Limit login endpoint to 5 requests/minute (prevents credential stuffing) - Limit file upload to 10 requests/minute (prevents upload flooding) - Limit file listing to 30 requests/minute (prevents enumeration) - Add 429 error handler with JSON response - Support memory storage by default, configurable via RATE_LIMIT_STORAGE_URI env var Security impact: Mitigates brute-force attacks on login and resource exhaustion via API endpoints. --- docker-compose.yml | 1 + webserver/main.py | 39 ++++++++++++++++++++++++++++++++++++++ webserver/requirements.txt | 1 + 3 files changed, 41 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index f313714..4f29c30 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -163,6 +163,7 @@ services: - USERS_CONFIG_PATH=/app/configs/users.json - STORAGE_BACKEND=local # Options: local, s3, minio - STORAGE_BASE_PATH=/app/data + - RATE_LIMIT_STORAGE_URI=memory:// volumes: - webserver_data_staged:/app/data/staged - webserver_data_archived:/app/data/archived diff --git a/webserver/main.py b/webserver/main.py index aede7ab..9e5fa75 100644 --- a/webserver/main.py +++ b/webserver/main.py @@ -26,6 +26,8 @@ login_required, current_user, ) +from flask_limiter import Limiter +from flask_limiter.util import get_remote_address from werkzeug.security import check_password_hash from werkzeug.utils import secure_filename from datetime import datetime, timedelta, timezone @@ -50,6 +52,17 @@ login_manager.login_view = "login" login_manager.login_message = "Please log in to access this page." +# ── Rate Limiting ─────────────────────────────────────────────────────────── +limiter_storage_uri = os.getenv("RATE_LIMIT_STORAGE_URI", "memory://") +limiter = Limiter( + key_func=get_remote_address, + app=app, + default_limits=[], + storage_uri=limiter_storage_uri, + strategy="fixed-window", +) + + class User(UserMixin): def __init__(self, user_id: str): @@ -142,6 +155,10 @@ def user_db_scope(user_id: str): @app.route("/login", methods=["GET", "POST"]) +@limiter.limit( + "5 per minute", + error_message="Too many login attempts. Please try again later.", +) def login(): if current_user.is_authenticated: return redirect(url_for("overview")) @@ -882,6 +899,10 @@ def get_file_size_mb(filepath): @app.route("/api/upload", methods=["POST"]) +@limiter.limit( + "10 per minute", + error_message="Too many upload attempts. Please slow down.", +) @login_required def upload_file(): """Handle file upload via drag and drop""" @@ -946,6 +967,10 @@ def upload_file(): @app.route("/api/files", methods=["GET"]) +@limiter.limit( + "30 per minute", + error_message="Too many requests. Please slow down.", +) @login_required def get_files(): """Get list of files in data_incoming and data_staged_for_parsing directories""" @@ -1004,6 +1029,20 @@ def get_files(): # ==================== ERROR HANDLERS ==================== +@app.errorhandler(429) +def ratelimit_handler(e): + """Handle rate limit exceeded errors.""" + return ( + jsonify( + { + "error": "Rate limit exceeded", + "message": str(e.description), + } + ), + 429, + ) + + @app.errorhandler(404) def not_found(error): return render_template("404.html"), 404 diff --git a/webserver/requirements.txt b/webserver/requirements.txt index e9a5a70..f2394dc 100644 --- a/webserver/requirements.txt +++ b/webserver/requirements.txt @@ -1,5 +1,6 @@ Flask==2.3.3 Flask-Login==0.6.3 +Flask-Limiter==3.5.0 psycopg2-binary==2.9.7 folium==0.14.0 gunicorn==22.0.0 From 554c83c46917c143d8dc6880a4bce9b19912e622 Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Mon, 3 Aug 2026 22:42:34 +0200 Subject: [PATCH 2/3] Fix grafana proxy tests: use LOGIN_DISABLED instead of session login The logged_in_client fixture was using actual session-based login which doesn't persist properly in test client when mocking is involved. This caused mock_requests.request.call_args to be None, resulting in 'TypeError: cannot unpack non-iterable NoneType object' errors. Updated to match pattern used in other passing tests (test_api_routes.py, test_time_slider_api.py): - Set LOGIN_DISABLED=True to bypass auth checks - Mock current_user directly instead of relying on session - Remove client.post('/login', ...) call that wasn't persisting Fixes 3 failing CI tests: - test_grafana_proxy_injects_webauth_user_header - test_grafana_proxy_strips_client_webauth_user_header - test_grafana_proxy_forwards_path --- webserver/tests/test_grafana_proxy.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/webserver/tests/test_grafana_proxy.py b/webserver/tests/test_grafana_proxy.py index 41afce8..902b424 100644 --- a/webserver/tests/test_grafana_proxy.py +++ b/webserver/tests/test_grafana_proxy.py @@ -23,10 +23,10 @@ def _make_grafana_response(status=200, content=b"ok", headers=None): @pytest.fixture def logged_in_client(monkeypatch): - """Test client with demo_openmrg actually logged in via the login route. + """Test client with login bypassed and current_user mocked. - Uses a real session so flask_login's current_user proxy resolves correctly - inside the grafana_proxy route handler. + Uses LOGIN_DISABLED to skip auth checks and mocks current_user + so flask_login's proxy resolves correctly inside grafana_proxy. """ monkeypatch.setitem( wm.USERS, @@ -36,10 +36,13 @@ def logged_in_client(monkeypatch): "display_name": "OpenMRG", }, ) + mock_user = Mock() + mock_user.id = "demo_openmrg" + mock_user.display_name = "OpenMRG" + monkeypatch.setattr(wm, "current_user", mock_user) + monkeypatch.setitem(wm.app.config, "LOGIN_DISABLED", True) wm.app.config["TESTING"] = True - client = wm.app.test_client() - client.post("/login", data={"username": "demo_openmrg", "password": "testpass"}) - return client + return wm.app.test_client() def test_grafana_proxy_injects_webauth_user_header(logged_in_client, monkeypatch): From 904ffcaeaa8711df9eba032cc07877eef241586f Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Mon, 3 Aug 2026 23:17:54 +0200 Subject: [PATCH 3/3] fix: trust X-Forwarded-For behind reverse proxy for correct rate limiting Without ProxyFix, get_remote_address returns the nginx IP, putting all clients in the same rate limit bucket. Set PROXY_COUNT=1 in the deploy docker-compose override when running behind nginx. --- docker-compose.yml | 1 + webserver/main.py | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 4f29c30..097e236 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -164,6 +164,7 @@ services: - STORAGE_BACKEND=local # Options: local, s3, minio - STORAGE_BASE_PATH=/app/data - RATE_LIMIT_STORAGE_URI=memory:// + - PROXY_COUNT=0 # Set to 1 when running behind a reverse proxy (nginx) volumes: - webserver_data_staged:/app/data/staged - webserver_data_archived:/app/data/archived diff --git a/webserver/main.py b/webserver/main.py index 9e5fa75..1dee237 100644 --- a/webserver/main.py +++ b/webserver/main.py @@ -28,6 +28,7 @@ ) from flask_limiter import Limiter from flask_limiter.util import get_remote_address +from werkzeug.middleware.proxy_fix import ProxyFix from werkzeug.security import check_password_hash from werkzeug.utils import secure_filename from datetime import datetime, timedelta, timezone @@ -39,6 +40,11 @@ app.secret_key = os.getenv("SECRET_KEY", os.urandom(32)) app.config["MAX_CONTENT_LENGTH"] = 500 * 1024 * 1024 # WSGI-level enforcement +# Trust X-Forwarded-For from this many upstream proxies (set to 1 when behind nginx) +_proxy_count = int(os.getenv("PROXY_COUNT", "0")) +if _proxy_count > 0: + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=_proxy_count) + # ── User store (loaded from file at startup) ────────────────────────────────── _users_config_path = os.getenv("USERS_CONFIG_PATH", "/app/configs/users.json") try: