Add preload option #7
Conversation
adelevie
force-pushed
the
add-preload-option
branch
from
46514b4 to
2e34ed7
Compare
|
Good call. I'll remove and gitignore it. |
| @@ -48,8 +48,26 @@ You can pass some keyword arguments to `sslify` to control its behavior: | |||
| * `proxy_header` (default: `X-Forwarded-Proto`) - for services behind a proxy, | |||
| this is the name of the header that contains the *real* request scheme. | |||
|
|
|||
| * `preload` (default: 'False') - adds the "preload" directive. | |||
There was a problem hiding this comment.
How about something like this to give a bit more context?
adds the `preload` directive, which is required for inclusion on the [HSTS preload list](https://hstspreload.appspot.com/).
| @@ -0,0 +1,10 @@ | |||
| FROM python:3 | |||
There was a problem hiding this comment.
Would you mind separating the Docker part out of this PR? :-)
| env['wsgi.url_scheme'] = 'https' | ||
| app_iter, status, headers = run_wsgi_app(app, env) | ||
| assert status == '200 OK' | ||
| assert 'preload' in headers['Strict-Transport-Security'] |
There was a problem hiding this comment.
I think it might be best to validate the entire header, rather than just that the "preload" string exists in it? (Like the test above does)
For example the header max-age=123 preload would pass this test, but be ignored by browsers (due to lack of ; delimiter) according to the spec:
https://tools.ietf.org/html/rfc6797#section-6.1
In addition it may be worth also adding a second test, that confirms both subdomains=True and preload=True can be set at the same time (named something like test_hsts_all_options_combined()).
|
Thanks for the review and feedback, @edmorley. Might take me a week or so to get around to these. |
Depends on #6, but I probably could separate the two if you don't want the Docker PR.