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 encrypting backend mixin and mix it in with Django's built-in backends #39

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

blag
Copy link
Contributor

@blag blag commented Mar 22, 2017

This is my first attempt at this, so feedback and criticism is more than welcome!

This is extremely similar to email_extras.utils.send_mail, but it's a mixin for mail backends.

The main problem I have with django-email-extras is that third-party app developers have to explicitly opt-in to using it by calling our send_mail function. I am using django-accounts and django-allauth to handle user registration/login/forgotten passwords, and they use Django's built-in one from django.core.mail.

I think encryption should be as easy to implement and use as possible (while still remaining actually secure), so this is an attempt in that direction. With this backend configured all mail Django sends will be sent through this backend, and opportunistically encrypted along the way (if the user has uploaded a key).

I also mixed it in with Django's built-in backends, so there's Encrypting*EmailBackend for the Console, Locmem, Filebased, and Smtp backends.

I am explicitly not adding code to upload the key to keyservers because python-gnupg does not yet support generating key revocation certificates, so I don't want users to upload keys they can't easily revoke. I added code to the new email_signing_key management command to automatically upload the key to one of more specified keyservers.

TODO:

  • Add management command/s for generating signing key? uploading to keyservers? with flag to skip if already exists?
  • Optionally sign outgoing email with generated key
  • Remove more specific exceptions and add a configurable exception handler for failed messages. I think this is a better way to handle things than simply throwing exceptions, because third party apps won't be expecting failed encryption exceptions or properly handle them.
  • Add user friendly messages when SIGNING_KEY_FINGERPRINT specified a key that doesn't exist
  • Document the process for how to add a signing key:
    1. Adjust EMAIL_EXTRAS_SIGNING_KEY_DATA
    2. Fire up server, browse to admin for keys
    3. Hit the "Generate signing key" button Run the email_signing_key command to generate a signing key
    4. Copy displayed fingerprint
    5. Set EMAIL_EXTRAS_SIGNING_KEY_FINGERPRINT to copied fingerprint
    6. Restart server

Edit: Thought of more/better ways to go about things.

@blag blag force-pushed the encrypting-backend branch 7 times, most recently from 345283f to a61cccb Compare March 27, 2017 22:23
@blag
Copy link
Contributor Author

blag commented Mar 27, 2017

@stephenmcd I'm now happy with this PR. Do you mind reviewing it? You might also be interested, @theithec.

And I'd like to add tests to the repo. Do you mind if I tack them onto the end of this?

Thanks!

@stephenmcd
Copy link
Owner

Do you mind reviewing it?

No time at the moment sorry.

@blag blag force-pushed the encrypting-backend branch 2 times, most recently from fd943ac to d197e18 Compare March 30, 2017 12:00
@blag blag mentioned this pull request Mar 30, 2017
15 tasks
@blag blag force-pushed the encrypting-backend branch 10 times, most recently from 6e3146f to ec1cf6a Compare April 2, 2017 22:54
@blag
Copy link
Contributor Author

blag commented Apr 9, 2017

Rebasing on master now that #41 is merged...

@blag blag force-pushed the encrypting-backend branch from ec1cf6a to d19d9bb Compare April 9, 2017 23:51
@blag
Copy link
Contributor Author

blag commented Apr 9, 2017

Done rebasing.

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.

2 participants