From ad4abc5167399868a2808999c21870a807c9237c Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 16 May 2023 14:59:03 +0200 Subject: [PATCH 1/2] Adds docker-compose dev options to force HTTPS settings and DEBUG mode through environment --- dockerfiles/settings/proxito.py | 5 +++++ docs/dev/install.rst | 4 +++- readthedocs/core/unresolver.py | 11 ++++++----- readthedocs/settings/base.py | 2 ++ readthedocs/settings/docker_compose.py | 19 +++++++++++++++++++ readthedocs/storage/mixins.py | 16 ++++++++++++---- readthedocs/storage/s3_storage.py | 5 +++-- 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/dockerfiles/settings/proxito.py b/dockerfiles/settings/proxito.py index 111e35b21b6..bd8595319b8 100644 --- a/dockerfiles/settings/proxito.py +++ b/dockerfiles/settings/proxito.py @@ -1,3 +1,5 @@ +import os + from readthedocs.settings.proxito.base import CommunityProxitoSettingsMixin from .docker_compose import DockerBaseSettings @@ -19,5 +21,8 @@ def DEBUG_TOOLBAR_CONFIG(self): 'SHOW_TOOLBAR_CALLBACK': lambda request: False, } + if os.environ.get("RTD_FORCE_HTTPS"): + PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK = True + ProxitoDevSettings.load_settings(__name__) diff --git a/docs/dev/install.rst b/docs/dev/install.rst index aa3941c0f3d..a4ad1444689 100644 --- a/docs/dev/install.rst +++ b/docs/dev/install.rst @@ -114,7 +114,9 @@ save some work while typing docker compose commands. This section explains these * ``--init`` is used the first time this command is ran to run initial migrations, create an admin user, etc * ``--no-reload`` makes all celery processes and django runserver to use no reload and do not watch for files changes - * ``--ngrok`` is useful when it's required to access the local instance from outside (e.g. GitHub webhook) + * ``--no-django-debug`` runs all containers with ``DEBUG=False`` + * ``--http-domain`` configures an external domain for the environment (useful for Ngrok or other https proxy) + * ``--https`` if using an HTTPS proxy, you may need to force the ``https://`` protocol for settings that otherwise automatically detect it as ``http://`` * ``--ext-theme`` to use the new dashboard templates * ``--webpack`` to start the Webpack dev server for the new dashboard templates diff --git a/readthedocs/core/unresolver.py b/readthedocs/core/unresolver.py index 508d43ceb0b..590ad1e6600 100644 --- a/readthedocs/core/unresolver.py +++ b/readthedocs/core/unresolver.py @@ -485,11 +485,12 @@ def unresolve_domain(self, domain): log.info("Invalid format of external versions domain.", domain=domain) raise InvalidExternalDomainError(domain=domain) - if public_domain in domain or external_domain in domain: - # NOTE: This can catch some possibly valid domains (docs.readthedocs.io.com) - # for example, but these might be phishing, so let's block them for now. - log.warning("Weird variation of our domain.", domain=domain) - raise SuspiciousHostnameError(domain=domain) + if not getattr(settings, "PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK", False): + if public_domain in domain or external_domain in domain: + # NOTE: This can catch some possibly valid domains (docs.readthedocs.io.com) + # for example, but these might be phishing, so let's block them for now. + log.warning("Weird variation of our domain.", domain=domain) + raise SuspiciousHostnameError(domain=domain) # Custom domain. domain_object = ( diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 139c2279fdb..ddc6150e722 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -1053,6 +1053,8 @@ def DOCKER_LIMITS(self): RTD_SPAM_THRESHOLD_DELETE_PROJECT = 1000 RTD_SPAM_MAX_SCORE = 9999 + PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK = False + CACHEOPS_ENABLED = False CACHEOPS_TIMEOUT = 60 * 60 # seconds CACHEOPS_OPS = {'get', 'fetch'} diff --git a/readthedocs/settings/docker_compose.py b/readthedocs/settings/docker_compose.py index 84afdea85bc..cf6d297b478 100644 --- a/readthedocs/settings/docker_compose.py +++ b/readthedocs/settings/docker_compose.py @@ -8,6 +8,8 @@ class DockerBaseSettings(CommunityBaseSettings): """Settings for local development with Docker""" + DEBUG = bool(os.environ.get('RTD_DJANGO_DEBUG', True)) + DOCKER_ENABLE = True RTD_DOCKER_COMPOSE = True RTD_DOCKER_COMPOSE_VOLUME = 'community_build-user-builds' @@ -30,6 +32,10 @@ class DockerBaseSettings(CommunityBaseSettings): # In the local docker environment, nginx should be trusted to set the host correctly USE_X_FORWARDED_HOST = True + # Assume running on forwarded https + if os.environ.get("RTD_FORCE_HTTPS"): + SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") + MULTIPLE_BUILD_SERVERS = ['build'] # https://docs.docker.com/engine/reference/commandline/run/#add-entries-to-container-hosts-file---add-host @@ -149,6 +155,13 @@ def DATABASES(self): # noqa } ACCOUNT_EMAIL_VERIFICATION = "none" + + # Assume running on forwarded https, needs a special option for socialauth because + # it detects HTTPS from the request automatically, and we may be running the app + # on :80 behind a :443 proxy. + if os.environ.get("RTD_FORCE_HTTPS"): + ACCOUNT_DEFAULT_HTTP_PROTOCOL = "https" + SESSION_COOKIE_DOMAIN = None CACHES = { 'default': { @@ -193,6 +206,12 @@ def DATABASES(self): # noqa AWS_S3_ENDPOINT_URL = 'http://storage:9000/' AWS_QUERYSTRING_AUTH = False + # Force the protocol for generated URLs to be https://, otherwise + # http:// is used because the storage server is running in a http + # container. + if os.environ.get("RTD_FORCE_HTTPS"): + S3_STATIC_STORAGE_OVERRIDE_PROTOCOL = "https" + RTD_SAVE_BUILD_COMMANDS_TO_STORAGE = True RTD_BUILD_COMMANDS_STORAGE = 'readthedocs.storage.s3_storage.S3BuildCommandsStorage' BUILD_COLD_STORAGE_URL = 'http://storage:9000/builds' diff --git a/readthedocs/storage/mixins.py b/readthedocs/storage/mixins.py index 7e0b5794301..e0c9b6031ca 100644 --- a/readthedocs/storage/mixins.py +++ b/readthedocs/storage/mixins.py @@ -6,21 +6,29 @@ class OverrideHostnameMixin: """ - Override the hostname when outputting URLs. + Override the hostname or protocol when outputting URLs. This is useful for use with a CDN or when proxying outside of Blob Storage See: https://github.com/jschneier/django-storages/pull/658 """ - override_hostname = None # Just the hostname without scheme (eg. 'assets.readthedocs.org') + override_hostname = ( + None # use the hostname without scheme (eg. 'assets.readthedocs.org') + ) + override_protocol = ( + None # set to "http" or "https". None = inherit automatic setting. + ) def url(self, *args, **kwargs): url = super().url(*args, **kwargs) - if self.override_hostname: + if self.override_hostname or self.override_protocol: parts = list(urlsplit(url)) - parts[1] = self.override_hostname + if self.override_protocol: + parts[0] = self.override_protocol + if self.override_hostname: + parts[1] = self.override_hostname url = urlunsplit(parts) return url diff --git a/readthedocs/storage/s3_storage.py b/readthedocs/storage/s3_storage.py index 48d40245720..d7f13f8f427 100644 --- a/readthedocs/storage/s3_storage.py +++ b/readthedocs/storage/s3_storage.py @@ -77,8 +77,9 @@ def __init__(self, *args, **kwargs): class S3StaticStorageMixin: - bucket_name = getattr(settings, 'S3_STATIC_STORAGE_BUCKET', None) - override_hostname = getattr(settings, 'S3_STATIC_STORAGE_OVERRIDE_HOSTNAME', None) + bucket_name = getattr(settings, "S3_STATIC_STORAGE_BUCKET", None) + override_hostname = getattr(settings, "S3_STATIC_STORAGE_OVERRIDE_HOSTNAME", None) + override_protocol = getattr(settings, "S3_STATIC_STORAGE_OVERRIDE_PROTOCOL", None) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) From 76cf10ad34c6aa104135d655f48ae7ba053c741f Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Mon, 5 Jun 2023 16:15:23 +0200 Subject: [PATCH 2/2] Remove settings that support https and disables suspicious host checking on Proxito --- dockerfiles/settings/proxito.py | 5 ----- docs/dev/install.rst | 5 +++-- readthedocs/core/unresolver.py | 11 +++++------ readthedocs/settings/base.py | 2 -- readthedocs/settings/docker_compose.py | 16 ---------------- readthedocs/storage/mixins.py | 14 ++++---------- readthedocs/storage/s3_storage.py | 1 - 7 files changed, 12 insertions(+), 42 deletions(-) diff --git a/dockerfiles/settings/proxito.py b/dockerfiles/settings/proxito.py index bd8595319b8..111e35b21b6 100644 --- a/dockerfiles/settings/proxito.py +++ b/dockerfiles/settings/proxito.py @@ -1,5 +1,3 @@ -import os - from readthedocs.settings.proxito.base import CommunityProxitoSettingsMixin from .docker_compose import DockerBaseSettings @@ -21,8 +19,5 @@ def DEBUG_TOOLBAR_CONFIG(self): 'SHOW_TOOLBAR_CALLBACK': lambda request: False, } - if os.environ.get("RTD_FORCE_HTTPS"): - PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK = True - ProxitoDevSettings.load_settings(__name__) diff --git a/docs/dev/install.rst b/docs/dev/install.rst index a4ad1444689..5f6ec948ded 100644 --- a/docs/dev/install.rst +++ b/docs/dev/install.rst @@ -115,8 +115,9 @@ save some work while typing docker compose commands. This section explains these * ``--no-reload`` makes all celery processes and django runserver to use no reload and do not watch for files changes * ``--no-django-debug`` runs all containers with ``DEBUG=False`` - * ``--http-domain`` configures an external domain for the environment (useful for Ngrok or other https proxy) - * ``--https`` if using an HTTPS proxy, you may need to force the ``https://`` protocol for settings that otherwise automatically detect it as ``http://`` + * ``--http-domain`` configures an external domain for the environment (useful for Ngrok or other http proxy). + Note that https proxies aren't supported. + There will also be issues with "suspicious domain" failures on Proxito. * ``--ext-theme`` to use the new dashboard templates * ``--webpack`` to start the Webpack dev server for the new dashboard templates diff --git a/readthedocs/core/unresolver.py b/readthedocs/core/unresolver.py index 590ad1e6600..508d43ceb0b 100644 --- a/readthedocs/core/unresolver.py +++ b/readthedocs/core/unresolver.py @@ -485,12 +485,11 @@ def unresolve_domain(self, domain): log.info("Invalid format of external versions domain.", domain=domain) raise InvalidExternalDomainError(domain=domain) - if not getattr(settings, "PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK", False): - if public_domain in domain or external_domain in domain: - # NOTE: This can catch some possibly valid domains (docs.readthedocs.io.com) - # for example, but these might be phishing, so let's block them for now. - log.warning("Weird variation of our domain.", domain=domain) - raise SuspiciousHostnameError(domain=domain) + if public_domain in domain or external_domain in domain: + # NOTE: This can catch some possibly valid domains (docs.readthedocs.io.com) + # for example, but these might be phishing, so let's block them for now. + log.warning("Weird variation of our domain.", domain=domain) + raise SuspiciousHostnameError(domain=domain) # Custom domain. domain_object = ( diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index ddc6150e722..139c2279fdb 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -1053,8 +1053,6 @@ def DOCKER_LIMITS(self): RTD_SPAM_THRESHOLD_DELETE_PROJECT = 1000 RTD_SPAM_MAX_SCORE = 9999 - PROXITO_DEV_DISABLE_SUSPICIOUS_HOST_CHECK = False - CACHEOPS_ENABLED = False CACHEOPS_TIMEOUT = 60 * 60 # seconds CACHEOPS_OPS = {'get', 'fetch'} diff --git a/readthedocs/settings/docker_compose.py b/readthedocs/settings/docker_compose.py index cf6d297b478..46a8885eb29 100644 --- a/readthedocs/settings/docker_compose.py +++ b/readthedocs/settings/docker_compose.py @@ -32,10 +32,6 @@ class DockerBaseSettings(CommunityBaseSettings): # In the local docker environment, nginx should be trusted to set the host correctly USE_X_FORWARDED_HOST = True - # Assume running on forwarded https - if os.environ.get("RTD_FORCE_HTTPS"): - SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") - MULTIPLE_BUILD_SERVERS = ['build'] # https://docs.docker.com/engine/reference/commandline/run/#add-entries-to-container-hosts-file---add-host @@ -156,12 +152,6 @@ def DATABASES(self): # noqa ACCOUNT_EMAIL_VERIFICATION = "none" - # Assume running on forwarded https, needs a special option for socialauth because - # it detects HTTPS from the request automatically, and we may be running the app - # on :80 behind a :443 proxy. - if os.environ.get("RTD_FORCE_HTTPS"): - ACCOUNT_DEFAULT_HTTP_PROTOCOL = "https" - SESSION_COOKIE_DOMAIN = None CACHES = { 'default': { @@ -206,12 +196,6 @@ def DATABASES(self): # noqa AWS_S3_ENDPOINT_URL = 'http://storage:9000/' AWS_QUERYSTRING_AUTH = False - # Force the protocol for generated URLs to be https://, otherwise - # http:// is used because the storage server is running in a http - # container. - if os.environ.get("RTD_FORCE_HTTPS"): - S3_STATIC_STORAGE_OVERRIDE_PROTOCOL = "https" - RTD_SAVE_BUILD_COMMANDS_TO_STORAGE = True RTD_BUILD_COMMANDS_STORAGE = 'readthedocs.storage.s3_storage.S3BuildCommandsStorage' BUILD_COLD_STORAGE_URL = 'http://storage:9000/builds' diff --git a/readthedocs/storage/mixins.py b/readthedocs/storage/mixins.py index e0c9b6031ca..5820d3d92fd 100644 --- a/readthedocs/storage/mixins.py +++ b/readthedocs/storage/mixins.py @@ -6,7 +6,7 @@ class OverrideHostnameMixin: """ - Override the hostname or protocol when outputting URLs. + Override the hostname when outputting URLs. This is useful for use with a CDN or when proxying outside of Blob Storage @@ -14,21 +14,15 @@ class OverrideHostnameMixin: """ override_hostname = ( - None # use the hostname without scheme (eg. 'assets.readthedocs.org') - ) - override_protocol = ( - None # set to "http" or "https". None = inherit automatic setting. + None # Just the hostname without scheme (eg. 'assets.readthedocs.org') ) def url(self, *args, **kwargs): url = super().url(*args, **kwargs) - if self.override_hostname or self.override_protocol: + if self.override_hostname: parts = list(urlsplit(url)) - if self.override_protocol: - parts[0] = self.override_protocol - if self.override_hostname: - parts[1] = self.override_hostname + parts[1] = self.override_hostname url = urlunsplit(parts) return url diff --git a/readthedocs/storage/s3_storage.py b/readthedocs/storage/s3_storage.py index d7f13f8f427..a1d51a9aba7 100644 --- a/readthedocs/storage/s3_storage.py +++ b/readthedocs/storage/s3_storage.py @@ -79,7 +79,6 @@ class S3StaticStorageMixin: bucket_name = getattr(settings, "S3_STATIC_STORAGE_BUCKET", None) override_hostname = getattr(settings, "S3_STATIC_STORAGE_OVERRIDE_HOSTNAME", None) - override_protocol = getattr(settings, "S3_STATIC_STORAGE_OVERRIDE_PROTOCOL", None) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs)