Opened 11 years ago

Closed 10 years ago

#20743 closed New feature (fixed)

Support keyfile/certfile in SMTP connections

Reported by: Claude Paroz Owned by: Tim Graham
Component: Core (Mail) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This was initially included in a patch on #13142 (https://github.com/django/django/pull/347), but the SSL support was added in commit for #17471.

keyfile/certfile are optional parameters to either `SMTP.starttls` or `smtplib.SMTP_SSL`.

Is there another way than adding two new settings?

Change History (6)

comment:1 by Andi Albrecht, 10 years ago

Owner: changed from nobody to Andi Albrecht
Status: newassigned

In my branch at github I've incorporated the previous work found in #13142: https://github.com/andialbrecht/django/tree/ticket_20743

This change uses the approach to add two new settings. I thought a bit about this, but didn't found a better solution than adding new settings. However, one approach would be to group all SMTP settings using a dictionary similar to cache or database settings, but this would be a more major change.

In addition to the previous work I've added support for keyfile/certfile when using TLS, added a note regarding certification checks (or the lack of) in the docs and added some minimal tests. I was wondering if we could find a clever approach to test the actual use of certfile/keyfile when initializing SMTP_SSL/calling starttls without some dirty mocking of the standard library. If anyone has a suggestion I'd be happy to add such tests to this patch.

It'd be nice if a core dev could have a preliminary look at the commit in my branch. I'd send then a pull request if it goes in the right direction.

Last edited 10 years ago by Andi Albrecht (previous) (diff)

comment:2 by Andi Albrecht, 10 years ago

Here's the pull request: https://github.com/django/django/pull/2709

All tests pass under SQLite.

I'm open for discussions regarding the two new settings. It would be good to avoid them, if possible. But the both files need to be configured somewhere.

comment:3 by Greg Chapple, 10 years ago

This may be somewhat related: #22734. The changes in this patch will need to be refactored as part of that ticket.

Version 0, edited 10 years ago by Greg Chapple (next)

comment:4 by Tim Graham, 10 years ago

Has patch: set
Patch needs improvement: set

To avoid the new setting, we could use the same technique that we used for adding timeout in 1.7, but only if we go that route for resolving #22734. Otherwise, we can go with whatever solution we adopt in #22734.

Marking this as "Patch needs improvement" for now. Once #22734 is resolved and this is updated, please uncheck.

comment:5 by Tim Graham, 10 years ago

Owner: changed from Andi Albrecht to Tim Graham

#22734 is a won't fix and the consensus is that it's fine to add these settings. I'll review and commit the PR.

comment:6 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 00535e8e6b402cab29ea3dbb7b4cc470f9839678:

Fixed #20743 -- Added support for keyfile/certfile in SMTP connections.

Thanks jwmayfield, serg.partizan, and Wojciech Banaś for work on the patch.

Note: See TracTickets for help on using tickets.
Back to Top