HTTPS #1581

josenavas · 2015-12-18T23:15:05Z

This PR enables HTTPS in the system.

This is adding default certificates and keys to the system. Since these are public, I highly recommend to not use these in a public system open to everybody. However, I think they should be ok to use in personal laptops for development and/or when users install Qiita in their systems.

If anybody sees a potential security problem by using these ones, please let me know and I will add code to generate those certificates.

I will add admin documentation in how to create these certificates and use them in a PR with the other documentation.

Please, test this in your machine using the certificates that are included. If they do not work, let me know so I can add the code rather than include them in the repository.

… modifying configuration file

antgonza · 2015-12-18T23:24:38Z

qiita_core/support_files/config_test.cfg

Space between # W

Done, also fixed a couple of similar errors in this file

squirrelo · 2015-12-21T18:40:45Z

qiita_core/support_files/server.key

Have these been tested when serving from other systems? I want to make sure they don't trigger HTTPS error pages.

When I try and access any page on the system, I get hte following error:

WARNING:tornado.general:SSL Error on 61 ('::1', 58512, 0, 0): [SSL: HTTP_REQUEST] http request (_ssl.c:590)

So these may not be good universally.

I couldn't test this in another system - that is what I exposed in the PR comments.

The error that you're seeing is because you are using http rather than https. Make sure to type https - in the live system the redirect is done through nginx, but in your local machine you need to type the https. Also make sure that your config file includes the https in the base_url, so when redirects happen in the system it works as expected.

Done, and I now get the error I was expecting from chrome:

Your connection is not private Attackers might be trying to steal your information from localhost (for example, passwords, messages, or credit cards). NET::ERR_CERT_AUTHORITY_INVALID

Can you post a screenshot of the error rather than copy paste? I would like to see better the error from chrome.

Thanks,

@biocore/qiita-dev can I have some comments here? The only alternative then is to ask each of our users to ask for a certificate to an authority - which can take some time and I don't think this is what we need to do. Without this, we can't use https neither oauth2.

I think that we good documentation this is not a problem - and we let the user know that is not a problem.

@squirrelo If you don't agree with this, please provide an alternative and constructive feedback.

We should really default back to HTTP if we don't have a certificate provided, possibly with a warning when you start the webserver. This allows users to use standard HTTP when it's on their laptop and safe, and HTTPS in larger deploys.

This issue is a complicated one, on the one hand we want to maintain the installation process as simple as possible, but on the other hand we also want to have a consistent system that isn't raising flags and worrying our users. It almost feels like these two are incompatible, so these are the possible partial solutions I can think of:

Document this situation as part of the installation procedure, something along the lines of " ... you'll need to provide authorized and validated certificates, otherwise you may .... XXXXX".

Add a permanent warning to the system i.e. a bootstrap alert that never leaves the screen saying " your certificates are not xxxxx, to fix this xxxxx, or read a discussion on why this xxxxxxx".

Check the system before starting the webserver and halting execution if valid certificates are not found, this could be bypassed with a flag maybe something like --i-dont-care-about-security- or --local-system.

Add an installation manager that requires valid certificates to be present at installation time, so that the situation shown above cannot be reached.

The solution should probably be a combination of some of the points above.

Just discussed some of this information with Jeff. Few points:

Valid certificates are not free: you have to buy them and their prices can be around ~$50. Even if it was $1 I don't think it is ok to ask our users running Qiita in their local machine to buy some certificate just because chrome complains about it.

The goal of a certificate is to let the user that the server is trusted - if we have documentation explaining that the server is your own machine, you can trust it as much as you trust your own machine.

There is no way of generating self-signed certificates that are recognized by the entire world unless you buy them. However, you can add an exception to your browser so it accepts such certificate.

About the comment of defaulting to HTTP: the entire work to use HTTPS has beed done so the RESTapi/plugins work as expected communicating through OAuth2 - if we default to HTTP, we can't use any of the plugins as they're authenticating through OAuth2, which requires HTTPS.

My vote goes top add detailed documentation in which we specify that if you are running locally in your machine, you will receive that warning the first time that you log in to the server (per @squirrelo 's observation, maybe every time that you start the server - I did not have that problem). Then, also add documentation that if you want to make your own Qiita server, you should get your own valid certificates from a Certificate Authority.

I think the solution is to improve documentation and/or try to do some of these suggestions: http://superuser.com/questions/772762/how-can-i-disable-security-checks-for-localhost

squirrelo · 2015-12-21T19:17:45Z

Some comments.

squirrelo · 2015-12-21T19:27:11Z

Running tests on my system for the plugins, I get that httpretty isn't installed. I also don't see it in the setup.py.

squirrelo · 2015-12-21T19:31:08Z

Sorry, cat on keyboard.

josenavas · 2015-12-21T21:59:45Z

I am hesitant to add httpretty to the setup.py
httpretty is required by the plugin, not by the main Qiita - the only reason why the plugin is in the same repository is for convenience in early development stages of the plugin, but it should be branched of to its own repository soon, once we can make the interfaces a bit more stable. An alternative that I was managing was to add it's own setup.py in the qiita_plugins/target_gene folder, what do others think about this? Looks like a good solution?

squirrelo · 2015-12-21T22:36:58Z

qiita_plugins/target_gene/tgp/tests/test_qiita_client.py

As has been stated in the past, try/finally should never be in tests. Please move this into the setUp/tearDown.

Good call - done

josenavas · 2015-12-23T03:59:29Z

Is there anything missing in this PR? The creation of the setup.py for the plugin is done here: #1587

antgonza · 2015-12-24T14:09:24Z

httpretty on setup.py in qiita_plugins/target_gene sounds like a good idea but open to other suggestions, the only requirement: one single command to install everything. I think it will be fine to have multiple commands once we have separate repos.

antgonza · 2015-12-24T14:17:56Z

qiita_plugins/target_gene/tgp/qiita_client.py

should this use path join?

I disagree as those are not paths and, although we don't support it, on windows it will not work.

antgonza · 2015-12-24T14:19:55Z

2 comments then 👍

josenavas · 2016-01-03T12:58:23Z

Can we address the setup.py comment in the other PR?

antgonza · 2016-01-03T15:00:17Z

Just to keep moving, lets do the setup.py in other PR but please open it ASAP.

HTTPS

josenavas · 2016-01-03T15:53:18Z

@antgonza that PR is already open: #1587

antgonza · 2016-01-03T15:53:56Z

Thanks for the clarification.

josenavas added 8 commits December 18, 2015 12:02

Adding QiitaClient class

c716574

Adding QiitaClient tests

28f9697

Using wss for websockets

15a6ee7

Using https in the tornado webserver, adding default certificates and…

7dbac08

… modifying configuration file

Update plugin to use https rather than http

e7b541f

Type on config file

c9a3da0

Removing prints

f60a98d

Fixing small bug

8739635

antgonza reviewed Dec 18, 2015
View reviewed changes

josenavas added 2 commits December 18, 2015 16:21

Adding documentation to the QiitaClient object

18fe70c

Addressing comments

1337746

squirrelo reviewed Dec 21, 2015
View reviewed changes

squirrelo closed this Dec 21, 2015

squirrelo reopened this Dec 21, 2015

Addressing comments

d6f63dd

Adding verbose comment to the _verify attribute in QiitaClient

0b848d8

squirrelo reviewed Dec 21, 2015
View reviewed changes

Removing try/except

e28153b

josenavas mentioned this pull request Dec 23, 2015

Artifact plugin setup #1587

Merged

antgonza reviewed Dec 24, 2015
View reviewed changes

antgonza added a commit that referenced this pull request Jan 3, 2016

Merge pull request #1581 from josenavas/artifact-https

19dfe07

HTTPS

antgonza merged commit 19dfe07 into qiita-spots:artifact Jan 3, 2016

Uh oh!

HTTPS #1581

HTTPS #1581

Uh oh!

Conversation

josenavas commented Dec 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

squirrelo commented Dec 21, 2015

Uh oh!

squirrelo commented Dec 21, 2015

Uh oh!

squirrelo commented Dec 21, 2015

Uh oh!

josenavas commented Dec 21, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josenavas commented Dec 23, 2015

Uh oh!

antgonza commented Dec 24, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antgonza commented Dec 24, 2015

Uh oh!

josenavas commented Jan 3, 2016

Uh oh!

antgonza commented Jan 3, 2016

Uh oh!

josenavas commented Jan 3, 2016

Uh oh!

antgonza commented Jan 3, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants