Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CLI params for TLS cert and key - serves over HTTPS #1354

Merged
merged 4 commits into from Apr 27, 2020

Conversation

mattdodge
Copy link
Contributor

@mattdodge mattdodge commented Apr 25, 2020

This adds support for serving the web UI over HTTPS. This shouldn't be the default behavior probably but if you have a cert I think you should be able to use it. Especially if basic auth is enabled.

This was first attempted and closed several years ago in #282. I'm hoping that it can be readdressed today though. Most of the web is moving to HTTPS and I believe anything that ships a web server should also come with the ability to run it securely - especially with a package as widely used as Locust is.

Comments/feedback/suggestions welcome, happy to iterate and make this work however we can! I love the Locust project and am happy to help improve it.

@mattdodge mattdodge mentioned this pull request Apr 25, 2020
@codecov
Copy link

codecov bot commented Apr 25, 2020

Codecov Report

Merging #1354 into master will increase coverage by 0.01%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1354      +/-   ##
==========================================
+ Coverage   80.67%   80.69%   +0.01%     
==========================================
  Files          24       24              
  Lines        2251     2258       +7     
  Branches      345      346       +1     
==========================================
+ Hits         1816     1822       +6     
- Misses        343      344       +1     
  Partials       92       92              
Impacted Files Coverage Δ
locust/main.py 20.00% <0.00%> (-0.11%) ⬇️
locust/argument_parser.py 74.31% <100.00%> (+0.48%) ⬆️
locust/env.py 95.55% <100.00%> (ø)
locust/web.py 91.20% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07d4d95...eb19c57. Read the comment docs.

@@ -125,5 +125,5 @@ def create_web_ui(self, host="", port=8089, auth_credentials=None):
:param port: Port that the web server should listen to
:param auth_credentials: If provided (in format "username:password") basic auth will be enabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add docstrings for the tls_cert and tls_cert params to the create_web_ui() method? (Since this method is part of the public API, and included in the API docs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops yeah sure thing, missed this one.

@heyman
Copy link
Member

heyman commented Apr 27, 2020

This looks good!

Do we need to say something about the expected format of the cert and key, or is that standardised nowadays?

@mattdodge
Copy link
Contributor Author

I'm certainly no expert but I think most cert types are handled fine out of the box. I tested with an existing LetsEncrypt cert I had, a new cert using mkcert, and also the self-signed certs from the unit test. It seems like gevent is using ssl.wrap_socket under the hood which should be pretty flexible.

@heyman
Copy link
Member

heyman commented Apr 27, 2020

Ok, great! Thanks!

@heyman heyman merged commit 5ef31a5 into locustio:master Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants