[Django] #20743: Support keyfile/certfile in SMTP connections
Django
---------------------------------------+------------------------
Reporter: claudep | Owner: nobody
Type: New feature | Status: new
Component: Core (Mail) | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
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
[http://docs.python.org/2/library/smtplib.html#smtplib.SMTP.starttls
`SMTP.starttls`] or
[http://docs.python.org/2/library/smtplib.html#smtplib.SMTP_SSL
`smtplib.SMTP_SSL`].
Is there another way than adding two new settings?
--
Ticket URL: <https://code.djangoproject.com/ticket/20743>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: claudep | Owner: andialbrecht
Type: New feature | Status: assigned
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andialbrecht):
* owner: nobody => andialbrecht
* status: new => assigned
Comment:
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'm 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:1>
Django
Reporter: claudep | Owner: andialbrecht
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andialbrecht):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:2>
Django
Reporter: claudep | Owner: andialbrecht
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by gchp):
This may be somewhat related: #22734. The changes in this patch will need
to be refactored as part of that ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:3>
Django
Reporter: claudep | Owner: andialbrecht
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
To avoid the new setting, we could use the same technique that we used for
adding
[https://docs.djangoproject.com/en/dev/topics/email/#django.core.mail.backends.smtp.EmailBackend.timeout
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:4>
Django
Reporter: claudep | Owner: timgraham
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* owner: andialbrecht => timgraham
Comment:
#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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:5>
Django
Reporter: claudep | Owner: timgraham
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"00535e8e6b402cab29ea3dbb7b4cc470f9839678"]:
{{{
#!CommitTicketReference repository=""
revision="00535e8e6b402cab29ea3dbb7b4cc470f9839678"
Fixed #20743 -- Added support for keyfile/certfile in SMTP connections.
Thanks jwmayfield, serg.partizan, and Wojciech Banaś for work on the
patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20743#comment:6>