From e8efa4213a4495cc760106c81889b9a80ba856ae Mon Sep 17 00:00:00 2001 From: Agate Date: Mon, 4 May 2020 12:02:08 +0200 Subject: [PATCH] Fix #1085: Make URL-building logic more resilient against reverse proxy misconfiguration --- api/config/settings/common.py | 7 ++++ api/funkwhale_api/common/apps.py | 2 + api/funkwhale_api/common/utils.py | 25 ++++++++++++ api/tests/common/test_utils.py | 61 ++++++++++++++++++++++++++++ changes/changelog.d/1085.enhancement | 1 + 5 files changed, 96 insertions(+) create mode 100644 changes/changelog.d/1085.enhancement diff --git a/api/config/settings/common.py b/api/config/settings/common.py index b55cfe84a..675d3e8ce 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -1302,3 +1302,10 @@ PODCASTS_RSS_FEED_MAX_ITEMS = env.int("PODCASTS_RSS_FEED_MAX_ITEMS", default=250 """ Maximum number of RSS items to load in each podcast feed. """ + +IGNORE_FORWARDED_HOST_AND_PROTO = env.bool( + "IGNORE_FORWARDED_HOST_AND_PROTO", default=True +) +""" +Use :attr:`FUNKWHALE_HOSTNAME` and :attr:`FUNKWHALE_PROTOCOL ` instead of request header. +""" diff --git a/api/funkwhale_api/common/apps.py b/api/funkwhale_api/common/apps.py index cd671be29..7d94695a1 100644 --- a/api/funkwhale_api/common/apps.py +++ b/api/funkwhale_api/common/apps.py @@ -1,6 +1,7 @@ from django.apps import AppConfig, apps from . import mutations +from . import utils class CommonConfig(AppConfig): @@ -11,3 +12,4 @@ class CommonConfig(AppConfig): app_names = [app.name for app in apps.app_configs.values()] mutations.registry.autodiscover(app_names) + utils.monkey_patch_request_build_absolute_uri() diff --git a/api/funkwhale_api/common/utils.py b/api/funkwhale_api/common/utils.py index b721bdf7b..0a38ef05f 100644 --- a/api/funkwhale_api/common/utils.py +++ b/api/funkwhale_api/common/utils.py @@ -1,6 +1,7 @@ import datetime from django.core.files.base import ContentFile +from django.http import request from django.utils.deconstruct import deconstructible import bleach.sanitizer @@ -433,3 +434,27 @@ def update_modification_date(obj, field="modification_date", date=None): obj.__class__.objects.filter(pk=obj.pk).update(**{field: date}) return date + + +def monkey_patch_request_build_absolute_uri(): + """ + Since we have FUNKWHALE_HOSTNAME and PROTOCOL hardcoded in settings, we can + override django's multisite logic which can break when reverse proxy aren't configured + properly. + """ + builtin_scheme = request.HttpRequest.scheme + + def scheme(self): + if settings.IGNORE_FORWARDED_HOST_AND_PROTO: + return settings.FUNKWHALE_PROTOCOL + return builtin_scheme.fget(self) + + builtin_get_host = request.HttpRequest.get_host + + def get_host(self): + if settings.IGNORE_FORWARDED_HOST_AND_PROTO: + return settings.FUNKWHALE_HOSTNAME + return builtin_get_host(self) + + request.HttpRequest.scheme = property(scheme) + request.HttpRequest.get_host = get_host diff --git a/api/tests/common/test_utils.py b/api/tests/common/test_utils.py index af2b25207..ffe9eae3b 100644 --- a/api/tests/common/test_utils.py +++ b/api/tests/common/test_utils.py @@ -197,3 +197,64 @@ def test_attach_file_content(factories, r_mock): assert new_attachment.file.read() == b"content" assert new_attachment.url is None assert new_attachment.mimetype == data["mimetype"] + + +@pytest.mark.parametrize( + "ignore, hostname, protocol, meta, path, expected", + [ + ( + False, + "test.hostname", + "http", + { + "HTTP_X_FORWARDED_HOST": "real.hostname", + "HTTP_X_FORWARDED_PROTO": "https", + }, + "/hello", + "https://real.hostname/hello", + ), + ( + False, + "test.hostname", + "http", + { + "HTTP_X_FORWARDED_HOST": "real.hostname", + "HTTP_X_FORWARDED_PROTO": "http", + }, + "/hello", + "http://real.hostname/hello", + ), + ( + True, + "test.hostname", + "http", + { + "HTTP_X_FORWARDED_HOST": "real.hostname", + "HTTP_X_FORWARDED_PROTO": "https", + }, + "/hello", + "http://test.hostname/hello", + ), + ( + True, + "test.hostname", + "https", + { + "HTTP_X_FORWARDED_HOST": "real.hostname", + "HTTP_X_FORWARDED_PROTO": "http", + }, + "/hello", + "https://test.hostname/hello", + ), + ], +) +def test_monkey_patch_request_build_absolute_uri( + ignore, hostname, protocol, meta, path, expected, fake_request, settings +): + settings.IGNORE_FORWARDED_HOST_AND_PROTO = ignore + settings.ALLOWED_HOSTS = "*" + settings.FUNKWHALE_HOSTNAME = hostname + settings.FUNKWHALE_PROTOCOL = protocol + request = fake_request.get("/", **meta) + + assert request.build_absolute_uri(path) == expected diff --git a/changes/changelog.d/1085.enhancement b/changes/changelog.d/1085.enhancement new file mode 100644 index 000000000..49cbee6f8 --- /dev/null +++ b/changes/changelog.d/1085.enhancement @@ -0,0 +1 @@ +Make URL-building logic more resilient against reverse proxy misconfiguration (#1085)