Skip to content

Commit

Permalink
Fix #934 infinite redirects with SSL HTTPS & NGIX
Browse files Browse the repository at this point in the history
Fixes two issues.

First, updates NGINX configuration files to pass $request_uri
port of URL from port 80 to port 443. Failing to pass $request_uri
tosses user (and non-users with invites) to home page rather than
requested url.

Second, and more signficantly, scheme `https` in `govready-url`
parameter was also setting `SECURE_SSL_REDIRECT` at the Django app
causing infinite redirects behind an NGINX reverse proxy that was
terminating the SSL connection and passing to local `http://localhost:8000`.
Fix was to let `SECURE_SSL_REDIRECT` remain its default `False` and
add new parameter `secure_ssl_redirect` for situations when deployment
called on Django to handle redirect.
  • Loading branch information
govreadydeploy committed Jun 12, 2020
1 parent 042e201 commit 64dce1b
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 12 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
GovReady-Q Release Notes
========================

v0.9.1.22.7 (June 12, 2020)
---------------------------

**Deployment fix**

* Remove conflict leading to infinite redirects when terminating SSL at a reverse proxy caused by SECURE_SSL_REDIRECT being set to `True` in `settings.py` telling Django to also redirect insecure `https` connections on the same server. Introduces optional new `secure_ssl_redirect`
parameter setting for `local/enviornment.json` file for deployments where Django redirect should be used. See: https://github.com/GovReady/govready-q/issues/934.

# Force Django to redirect non-secure SSL connections to secure ssl connections
# NOTE: Setting to True while simultaneously using a http proxy like NGINX
# that is also redirecting can lead to infinite redirects.
# See: https://docs.djangoproject.com/en/3.0/ref/settings/#secure-ssl-redirect
# SECURE_SSL_REDIRECT is False by default.

**Documentation fix**

* Update documentation on NGINX deployments to include `$request_uri` when redirecting from port 80 to port 443.
* Update documentation to explain new `secure_ssl_redirect` parameter.

v0.9.1.22.6 (June 10, 2020)
---------------------------

Expand Down
3 changes: 1 addition & 2 deletions VERSION
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
v0.9.1.22.6

v0.9.1.22.7
10 changes: 7 additions & 3 deletions docs/source/installation-guide/Environment.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,18 @@ launching a container with GovReady-Q.
- ``secret-key`` - used to make instance more secure by contributing a
salt value to generating various random strings and hashes. Do not
share.
- ``gr-pdf-generator`` - specifies the library/process used to generate PDFs,
- ``gr-pdf-generator``: specifies the library/process used to generate PDFs,
options are `off` and `wkhtmltopdf` and default is `None`.
- ``gr-img-generator`` - specifies the library/process used to generate images and thumbnails,
- ``gr-img-generator``: specifies the library/process used to generate images and thumbnails,
options are `off` and `wkhtmltopdf` and default is `None`.
- ``single-organization``: used to enforce single organization mode
with “main” as subdomain of default organization instead of
multi-tenant with different subdomains for different organizations.
- ``static``: used to prepend a root path to the default ``/static/``
- ``secure_ssl_redirect``: tells Django to redirect any non-secure connections to secure
HTTPS (SSL) connections when terminating SSL at Django and connection scheme is ``https``;
setting ``secure_ssl_redirect`` to ``true`` behind a reverse proxy that is also managing
redirects can cause infinite redirects.
- ``static`` used to prepend a root path to the default ``/static/``
URL path for accessing static assets.
- ``syslog``: used to set the host and port of a syslog-compatible log
message sink. (Default: None.)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,14 +575,16 @@ be replaced with your domain.
Example - HTTPS on 443 and HTTP on 80 redirecting to HTTPS on 443
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The below example shows a basic version of ``/nginx/sites-available/nginx-govready-q.conf`` redirecting port 80 to 443.
The below example shows a basic version of ``/nginx/sites-available/nginx-govready-q.conf`` redirecting port 80 to 443
while passing the path to the requested files along with the redirect.

.. code:: text
server {
listen 80;
server_name example.com;
return 302 https://example.com;
return 302 https://$server_name:443$request_uri;
}
server {
Expand All @@ -606,6 +608,10 @@ The below example shows a basic version of ``/nginx/sites-available/nginx-govrea
port 80 from ``/etc/nginx/sites-enabled/``. Failure to remove the default configuration
file will create two conflicting NGINX servers listening on port 80.

.. warning::
It is important to include the ``$request_uri`` in any redirect of the URL for the redirected
user to be routed to the request page.

Example - Listening both HTTP on 80 and HTTPS on 443
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -647,7 +653,7 @@ on both ports because we want logins to GovReady-Q to be encrypted.
# listen [::]:80;
listen 80;
server_name example.com;
return 301 https://example.com;
return 302 https://$server_name:443$request_uri;
}
server {
Expand Down Expand Up @@ -687,7 +693,6 @@ on both ports because we want logins to GovReady-Q to be encrypted.
}
}
.. note::
Some code for creating and using a self-generated certificated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
server {
# listen [::]:80;
listen 80;
server_name example.com;
return 301 https://example.com;
return 302 https://$server_name:443$request_uri;
}

server {
Expand Down
14 changes: 13 additions & 1 deletion siteapp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def make_secret_key():
from urllib.parse import urlparse
GOVREADY_URL = urlparse(environment.get("govready-url",""))


# Set Django's ALLOWED_HOSTS parameter.
# Use the deprecated 'host' environment parameter and preferred 'govready-url'.
# The port is used in SITE_ROOT_URL must be removed from ALLOWED_HOSTS.
Expand Down Expand Up @@ -344,9 +345,16 @@ def make_secret_key():
SESSION_COOKIE_SECURE = True
CSRF_COOKIE_HTTPONLY = True
CSRF_COOKIE_SECURE = True
SECURE_SSL_REDIRECT = True
SECURE_HSTS_SECONDS = 31536000
SECURE_HSTS_INCLUDE_SUBDOMAINS = True
if environment.get("secure_ssl_redirect", False):
# Force Django to redirect non-secure SSL connections to secure SSL connections
# NOTE: Setting to True while simultaneously using a http proxy like NGINX
# that is also redirecting can lead to infinite redirects.
# See: https://docs.djangoproject.com/en/3.0/ref/settings/#secure-ssl-redirect
# SECURE_SSL_REDIRECT is False by default.
SECURE_SSL_REDIRECT = True
print("INFO: SECURE_SSL_REDIRECT is set to 'True'. May cause infinite redirects behind reverse proxies.")
else:
print("INFO: Connection scheme is 'http'.")
# Silence some checks about HTTPS.
Expand All @@ -357,6 +365,10 @@ def make_secret_key():
'security.W016', # CSRF_COOKIE_SECURE not set
]


SECURE_SSL_REDIRECT = False


# Other security headers.
SECURE_BROWSER_XSS_FILTER = True
SECURE_CONTENT_TYPE_NOSNIFF = True
Expand Down

0 comments on commit 64dce1b

Please sign in to comment.