anoved / OctoPrint-EmailNotifier

Receive email notifications when OctoPrint jobs are complete. Currently broken. Please fork and fix!
GNU Affero General Public License v3.0
16 stars 35 forks source link

Add a password field to the UI #12

Open cweagans opened 8 years ago

cweagans commented 8 years ago

Don't save it in the octoprint settings, obviously. Basically, just automate the extra step that users need to take to save the SMTP password to a keychain. This field would be empty in the UI. A user can put their password in and save the form, and then the next time the form is rendered, the field is still empty. Users only put their password in to change it.

As an aside, I run OctoPrint via OctoPi, and the normal keyring methods were unavailable in the default distribution (there is no desktop environment or anything installed), so I had to install keyrings.alt. Might be useful to make a note about for users (also in the UI), or even better, add it as a dependency of this plugin. I imagine that the better keyring methods would be used first, and then it would fall back to keyrings.alt if no better methods were available, but I guess that would need some testing.

djvaporize commented 8 years ago

The instructions on the readme works ok for me without installing additional modules:

$ ~/oprint/bin/python
>>> import yagmail
>>> yagmail.register("SMTP username", "SMTP password")

However, I did thought about adding a password field here. The reason why I didn't is because of security reasons as octoprint/octopi (at least on my configuration) does not have SSL. Certainly, the password will be sent over the network in cleartext. Granted, I think (hope) most users are using octoprint in a local network but who knows (and not over open WiFI/WEP). In addition, it is not clear how many have it opened to the internet.

The other idea I had was to use a public/private keypair on the backend and clientside. That is, the backend python will generate a random key pair, the client side javascript will use the public key and encrypt and encode the transmission--the private key will never be in transit. A little over the top paranoia but I think it is unethical to have cleartext passwords in this day in age. Quite frankly too much trouble to implement and best to just use SSL.

Cheers!

cweagans commented 8 years ago

I think it's safe to assume two things here:

  1. Most Octoprint installations will be in a local network and not exposed publicly to the internet; and
  2. Most Octoprint installations that ARE exposed to the internet will be served via TLS.

I don't think making a plugin more difficult to use because the user may have done a poor job with their Octoprint deployment is a good idea, personally. You simply can't protect users from their own stupidity :)

Also, OctoPI comes with a self-signed certificate OOTB and can be reached via HTTPS.

All of that said, I think it's safe to solve this with some text that's displayed to the user inline on that form. Something like "Warning: Do not put your SMTP server password in this field unless you're on a local network or you're accessing Octoprint via HTTPS." I bet you could even detect the protocol on the client side and hide it if it's HTTPS.

The public/private keypair seems like overkill. Would not recommend.

djvaporize commented 8 years ago

Ah okay. I think that can be done. Detecting the https seems to be possible. Do you think it should detect and display the warning? And not display the warning when there is HTTPS ?

cweagans commented 8 years ago

I'd say don't show the warning at all via HTTPS, and then for non-HTTPS on anything other than a private IP range or octopi.local, display "Warning: It looks like you're accessing a public OctoPrint instance via an unsecured HTTP connection. Use of the SMTP password field is not recommended. Instead, manually register your SMTP password per the documentation that can be found here." (and link to the right page)

That should be enough to at least make people aware of what they might be exposing.