[GitHub] [airflow] ashb commented on a change in pull request #8157: Reorder middleware - ProxyFix and BaseUrl

[GitBox <gi...@apache.org>]

ashb commented on a change in pull request #8157: Reorder middleware - ProxyFix and BaseUrl
URL: https://github.com/apache/airflow/pull/8157#discussion_r404758180
 
 

 ##########
 File path: tests/www/test_app.py
 ##########
 @@ -17,41 +17,185 @@
 # under the License.
 
 import unittest
+from unittest import mock
 
-from werkzeug.middleware.proxy_fix import ProxyFix
+from werkzeug.routing import Rule
+from werkzeug.test import create_environ
+from werkzeug.wrappers import Response
 
-from airflow.settings import Session
 from airflow.www import app as application
 from tests.test_utils.config import conf_vars
 
 
 class TestApp(unittest.TestCase):
-    @conf_vars({('webserver', 'enable_proxy_fix'): 'False'})
-    def test_constructor_no_proxyfix(self):
-        app, _ = application.create_app(session=Session, testing=True)
-        self.assertFalse(isinstance(app.wsgi_app, ProxyFix))
-
-    @conf_vars({('webserver', 'enable_proxy_fix'): 'True'})
-    def test_constructor_proxyfix(self):
-        app, _ = application.create_app(session=Session, testing=True)
-        self.assertTrue(isinstance(app.wsgi_app, ProxyFix))
-        self.assertEqual(app.wsgi_app.x_for, 1)
-        self.assertEqual(app.wsgi_app.x_proto, 1)
-        self.assertEqual(app.wsgi_app.x_host, 1)
-        self.assertEqual(app.wsgi_app.x_port, 1)
-        self.assertEqual(app.wsgi_app.x_prefix, 1)
-
-    @conf_vars({('webserver', 'enable_proxy_fix'): 'True',
-                ('webserver', 'proxy_fix_x_for'): '3',
-                ('webserver', 'proxy_fix_x_proto'): '4',
-                ('webserver', 'proxy_fix_x_host'): '5',
-                ('webserver', 'proxy_fix_x_port'): '6',
-                ('webserver', 'proxy_fix_x_prefix'): '7'})
-    def test_constructor_proxyfix_args(self):
-        app, _ = application.create_app(session=Session, testing=True)
-        self.assertTrue(isinstance(app.wsgi_app, ProxyFix))
-        self.assertEqual(app.wsgi_app.x_for, 3)
-        self.assertEqual(app.wsgi_app.x_proto, 4)
-        self.assertEqual(app.wsgi_app.x_host, 5)
-        self.assertEqual(app.wsgi_app.x_port, 6)
-        self.assertEqual(app.wsgi_app.x_prefix, 7)
+
+    @conf_vars({
+        ('webserver', 'enable_proxy_fix'): 'True',
+        ('webserver', 'proxy_fix_x_for'): '1',
+        ('webserver', 'proxy_fix_x_proto'): '1',
+        ('webserver', 'proxy_fix_x_host'): '1',
+        ('webserver', 'proxy_fix_x_port'): '1',
+        ('webserver', 'proxy_fix_x_prefix'): '1'
+    })
+    @mock.patch("airflow.www.app.app", None)
+    @mock.patch("airflow.www.app.appbuilder", None)
+    def test_should_respect_proxy_fix(self):
+        app = application.cached_app(testing=True)
+        flask_app = next(iter(app.app.mounts.values()))
+        flask_app.url_map.add(Rule("/debug", endpoint="debug"))
+
+        def debug_view():
+            from flask import request
+
+            # Should respect HTTP_X_FORWARDED_FOR
+            self.assertEqual(request.remote_addr, '192.168.0.1')
+            # Should respect HTTP_X_FORWARDED_PROTO, HTTP_X_FORWARDED_HOST, HTTP_X_FORWARDED_PORT,
+            # HTTP_X_FORWARDED_PREFIX
+            self.assertEqual(request.url, 'https://valid:445/proxy-prefix/debug')
 
 Review comment:
   Does this work in practice without base_url set, or are all the links and stylesheets pulled in at the wrong URLs?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services