Replace ReverseProxied with werkzeug ProxyFix

- replace ReverseProxied with werkzeug ProxyFix
- trust levels are configurable via PROXYFIX_X_* config attributes
- add tests of ProxyFix configuration
- set X-Forwarded-For and X-Forwarded-Proto to 2

Fixes #112

Assisted-By: Claude Code

.ai_review/project.md

@@ -86,7 +86,7 @@ testdays/
 - **User roles hierarchy**: `none` → `user` → `creator` → `admin`. Admin role implicitly satisfies all other role checks via `User.has_role()`. Anonymous users can never have roles beyond `none`.
 - **Testday metadata format is custom DSL**: Lines starting with `=` are sections, `*` are testcases (`name; url; optional-ulid`), `! ProfileText:` overrides the Profile column label.
 - **HTML escaping is manual**: User input is escaped with `html.escape()` in controllers before DB storage. The `html_escape` validator on forms and `user_displayname_validator` handle this — don't suggest Jinja autoescaping as a replacement.
-- **OpenShift deployment specifics**: `IS_OPENSHIFT` env var activates `ReverseProxied` middleware and expects DB credentials from env vars. Gunicorn `on_starting` hook runs `upgrade_db` for auto-migration.
+- **OpenShift deployment specifics**: `IS_OPENSHIFT` env var activates Werkzeug `ProxyFix` middleware (configured via `PROXYFIX_X_FOR`, `PROXYFIX_X_PROTO`, `PROXYFIX_X_HOST`, `PROXYFIX_X_PORT`, `PROXYFIX_X_PREFIX` config attributes) and expects DB credentials from env vars. Gunicorn `on_starting` hook runs `upgrade_db` for auto-migration.
 
 ### Do NOT Flag
 - `from ..models import *` in controllers — intentional pattern to import all model classes (re-exported via `models/__init__.py`)

testdays/app.py

@@ -11,6 +11,8 @@ from flask_simple_captcha import CAPTCHA
 from git import Repo
 from git.exc import GitCommandError
 
+from werkzeug.middleware.proxy_fix import ProxyFix
+
 from . import config
 from . import extensions
 
@@ -32,24 +34,6 @@ class _WerkzeugRequestFilter(logging.Filter):
         return True
 
 
-class ReverseProxied(object):
-    def __init__(self, app):
-        self.app = app
-
-    def __call__(self, environ, start_response):
-        scheme = environ.get("HTTP_X_FORWARDED_PROTO")
-
-        # For some reason, the current Openshift/Proxy setup is weird, and instead of
-        #   "https" the X-Forwarded-Proto header is set to "https,https"
-        # This works around it, while (hopefully) not fucking up the other instances
-        #   where the value is either unset, or set properly.
-        scheme = scheme.split(",")[0].strip() if scheme and "," in scheme else scheme
-        if scheme:
-            environ["wsgi.url_scheme"] = scheme
-
-        return self.app(environ, start_response)
-
-
 def _load_config_from_env():
     """Build a Config object using the existing 3-layer strategy.
 
@@ -278,10 +262,19 @@ def create_app(config_obj=None):
         return response
 
     # --- Reverse proxy middleware ---
-    # Openshift is reverse-proxied. Maybe change this to:
-    # https://werkzeug.palletsprojects.com/en/3.0.x/middleware/proxy_fix/
     if os.getenv("IS_OPENSHIFT"):
-        app.wsgi_app = ReverseProxied(app.wsgi_app)
+        # ProxyFix is used instead of ReverseProxied because
+        # it correctly handles duplicated values produced by
+        # HAProxy's append mode (e.g. "https,https") without
+        # extra normalization.
+        app.wsgi_app = ProxyFix(
+            app.wsgi_app,
+            x_for=int(app.config["PROXYFIX_X_FOR"]),
+            x_proto=int(app.config["PROXYFIX_X_PROTO"]),
+            x_host=int(app.config["PROXYFIX_X_HOST"]),
+            x_port=int(app.config["PROXYFIX_X_PORT"]),
+            x_prefix=int(app.config["PROXYFIX_X_PREFIX"]),
+        )
 
     if app.config.get("SHOW_DB_URI"):
         app.logger.debug("using DBURI: %s", app.config["SQLALCHEMY_DATABASE_URI"])

testdays/config.py

@@ -45,6 +45,15 @@ class Config(object):
         "forge.fedoraproject.org",
     )
 
+    # ProxyFix settings -- trust levels for X-Forwarded-* headers.
+    # Each value = number of proxies trusted for that header (0 = ignore).
+    # Overridable via env vars or settings.py.
+    PROXYFIX_X_FOR = 2
+    PROXYFIX_X_PROTO = 2
+    PROXYFIX_X_HOST = 0
+    PROXYFIX_X_PORT = 0
+    PROXYFIX_X_PREFIX = 0
+
     @property
     def SQLALCHEMY_DATABASE_URI(self):
         if self.DB_URI:

tests/unit/test_proxy_fix.py

@@ -0,0 +1,96 @@
+"""
+Unit tests for ProxyFix middleware integration.
+
+Verifies that werkzeug's ProxyFix is applied when IS_OPENSHIFT is set,
+config values are passed through, and it is not applied otherwise.
+"""
+
+import os
+from unittest import mock
+
+import pytest
+from jinjax.middleware import ComponentsMiddleware
+from werkzeug.middleware.proxy_fix import ProxyFix
+
+from testdays.app import create_app
+from testdays.config import Testing
+from tests.conftest import TEST_SETTINGS_PATH
+
+
+@pytest.fixture(name="openshift_app")
+def _openshift_app(_app_config):
+    """Create a Flask app with IS_OPENSHIFT set, using the test settings file.
+
+    Depends on the _app_config fixture so the settings file (with DB_URI,
+    OIDC_ENABLED=False, etc.) is written before we load it.
+    """
+    config_obj = Testing()
+    config_obj.update_from_pyfile(str(TEST_SETTINGS_PATH))
+    with mock.patch.dict(os.environ, {"IS_OPENSHIFT": "1"}):
+        yield create_app(config_obj)
+
+
+class TestProxyFixMiddleware:
+    """Tests for ProxyFix middleware application in create_app."""
+
+    def test_proxyfix_applied_when_is_openshift_set(self, openshift_app):
+        """
+        GIVEN IS_OPENSHIFT is set in environment
+        WHEN the app is created
+        THEN ProxyFix should be the outermost middleware
+        """
+        assert isinstance(openshift_app.wsgi_app, ProxyFix)
+
+    def test_proxyfix_not_applied_when_is_openshift_unset(self, app):
+        """
+        GIVEN IS_OPENSHIFT is not set in environment
+        WHEN the app is created
+        THEN ProxyFix should not be in the middleware stack
+        """
+        assert not isinstance(app.wsgi_app, ProxyFix)
+
+    def test_proxyfix_uses_config_defaults(self, openshift_app):
+        """
+        GIVEN IS_OPENSHIFT is set and default config values
+        WHEN the app is created
+        THEN ProxyFix should have x_for=2, x_proto=2, others=0
+        """
+        pf = openshift_app.wsgi_app
+        assert pf.x_for == 2
+        assert pf.x_proto == 2
+        assert pf.x_host == 0
+        assert pf.x_port == 0
+        assert pf.x_prefix == 0
+
+    def test_proxyfix_respects_config_overrides(self, _app_config):
+        """
+        GIVEN IS_OPENSHIFT is set and custom PROXYFIX_* config values
+        WHEN the app is created
+        THEN ProxyFix should use the overridden values
+        """
+        config_obj = Testing()
+        config_obj.update_from_pyfile(str(TEST_SETTINGS_PATH))
+        config_obj.PROXYFIX_X_FOR = 2
+        config_obj.PROXYFIX_X_PROTO = 2
+        config_obj.PROXYFIX_X_HOST = 1
+        config_obj.PROXYFIX_X_PORT = 1
+        config_obj.PROXYFIX_X_PREFIX = 1
+        with mock.patch.dict(os.environ, {"IS_OPENSHIFT": "1"}):
+            app = create_app(config_obj)
+        pf = app.wsgi_app
+        assert pf.x_for == 2
+        assert pf.x_proto == 2
+        assert pf.x_host == 1
+        assert pf.x_port == 1
+        assert pf.x_prefix == 1
+
+    def test_proxyfix_wraps_components_middleware(self, openshift_app):
+        """
+        GIVEN IS_OPENSHIFT is set
+        WHEN the app is created
+        THEN the middleware stack should be:
+             ProxyFix -> ComponentsMiddleware (WhiteNoise)
+        """
+        pf = openshift_app.wsgi_app
+        assert isinstance(pf, ProxyFix)
+        assert isinstance(pf.app, ComponentsMiddleware)