From 95e4060f15abcf40db9478e4b6f8d4bfcde56050 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Fri, 19 Apr 2019 02:11:09 -0400 Subject: [PATCH] ProxyFix changes (#960) * Fixes insufficiently documented changes in ProxyFix * https://github.com/pallets/werkzeug/issues/1484 * Pin `Werkzeug==0.15.2` in requirements.txt * Allow exports test to parallelize --- CTFd/__init__.py | 23 ++++++++++++++++++++--- CTFd/config.py | 6 ++++++ requirements.txt | 1 + tests/test_config.py | 35 +++++++++++++++++++++++++++++++++++ tests/utils/test_exports.py | 8 ++++---- 5 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 tests/test_config.py diff --git a/CTFd/__init__.py b/CTFd/__init__.py index ca238a8..ef93830 100644 --- a/CTFd/__init__.py +++ b/CTFd/__init__.py @@ -5,7 +5,7 @@ from distutils.version import StrictVersion from flask import Flask, Request from flask_migrate import upgrade, stamp from werkzeug.utils import cached_property -from werkzeug.contrib.fixers import ProxyFix +from werkzeug.middleware.proxy_fix import ProxyFix from jinja2 import FileSystemLoader from jinja2.sandbox import SandboxedEnvironment from six.moves import input @@ -153,8 +153,25 @@ def create_app(config='CTFd.config.Config'): cache.init_app(app) app.cache = cache - if app.config.get('REVERSE_PROXY'): - app.wsgi_app = ProxyFix(app.wsgi_app) + reverse_proxy = app.config.get('REVERSE_PROXY') + if reverse_proxy: + if ',' in reverse_proxy: + proxyfix_args = [int(i) for i in reverse_proxy.split(',')] + app.wsgi_app = ProxyFix( + app.wsgi_app, + None, + *proxyfix_args + ) + else: + app.wsgi_app = ProxyFix( + app.wsgi_app, + num_proxies=None, + x_for=1, + x_proto=1, + x_host=1, + x_port=1, + x_prefix=1 + ) version = utils.get_config('ctf_version') diff --git a/CTFd/config.py b/CTFd/config.py index 13e2a60..1f3557a 100644 --- a/CTFd/config.py +++ b/CTFd/config.py @@ -194,6 +194,12 @@ class Config(object): REVERSE_PROXY: Specifies whether CTFd is behind a reverse proxy or not. Set to True if using a reverse proxy like nginx. + You can also specify a comma seperated set of numbers specifying the reverse proxy configuration settings. + + See https://werkzeug.palletsprojects.com/en/0.15.x/middleware/proxy_fix/#werkzeug.middleware.proxy_fix.ProxyFix. + For example to configure `x_for=1, x_proto=1, x_host=1, x_port=1, x_prefix=1` specify `1,1,1,1,1`. + + Alternatively if you specify `true` CTFd will default to the above behavior with all proxy settings set to 1. TEMPLATES_AUTO_RELOAD: Specifies whether Flask should check for modifications to templates and reload them automatically. diff --git a/requirements.txt b/requirements.txt index 371f649..bb35b58 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,5 @@ Flask==1.0.2 +Werkzeug==0.15.2 Flask-SQLAlchemy==2.3.2 Flask-Session==0.3.1 Flask-Caching==1.4.0 diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..972043b --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +from tests.helpers import ( + create_ctfd, + destroy_ctfd, +) +from CTFd.config import TestingConfig + + +def test_reverse_proxy_config(): + """Test that REVERSE_PROXY configuration behaviors properly""" + class ReverseProxyConfig(TestingConfig): + REVERSE_PROXY = '1,2,3,4' + + app = create_ctfd(config=ReverseProxyConfig) + with app.app_context(): + assert app.wsgi_app.x_for == 1 + assert app.wsgi_app.x_proto == 2 + assert app.wsgi_app.x_host == 3 + assert app.wsgi_app.x_port == 4 + assert app.wsgi_app.x_prefix == 0 + destroy_ctfd(app) + + class ReverseProxyConfig(TestingConfig): + REVERSE_PROXY = 'true' + + app = create_ctfd(config=ReverseProxyConfig) + with app.app_context(): + assert app.wsgi_app.x_for == 1 + assert app.wsgi_app.x_proto == 1 + assert app.wsgi_app.x_host == 1 + assert app.wsgi_app.x_port == 1 + assert app.wsgi_app.x_prefix == 1 + destroy_ctfd(app) diff --git a/tests/utils/test_exports.py b/tests/utils/test_exports.py index b0ecfbd..a937a6a 100644 --- a/tests/utils/test_exports.py +++ b/tests/utils/test_exports.py @@ -36,9 +36,9 @@ def test_export_ctf(): app.db.session.commit() backup = export_ctf() - with open('export.zip', 'wb') as f: + with open('export.test_export_ctf.zip', 'wb') as f: f.write(backup.read()) - os.remove('export.zip') + os.remove('export.test_export_ctf.zip') destroy_ctfd(app) @@ -61,7 +61,7 @@ def test_import_ctf(): backup = export_ctf() - with open('export.zip', 'wb') as f: + with open('export.test_import_ctf.zip', 'wb') as f: f.write(backup.read()) destroy_ctfd(app) @@ -69,7 +69,7 @@ def test_import_ctf(): # TODO: These databases should work but they don't... if not app.config.get('SQLALCHEMY_DATABASE_URI').startswith('sqlite'): with app.app_context(): - import_ctf('export.zip') + import_ctf('export.test_import_ctf.zip') if not app.config.get('SQLALCHEMY_DATABASE_URI').startswith('postgres'): # TODO: Dig deeper into why Postgres fails here