Opened 7 years ago

Closed 6 years ago

#861 closed enhancement (fixed)

Use STARTTLS when sending emails

Reported by: Matt Molyneaux Owned by:
Priority: minor Milestone: 0.7.0
Component: programming Keywords: mail, smtp
Cc: Parent Tickets:

Description

Currently MediaGoblin only has the option of using plain SMTP or SMTP over SSL (which has deprecated for some time now). Using STARTTLS is completely unsupported.

These changes address this issue by attempting to use the STARTTLS command after EHLO/HELO. By default it will carry on as normal if STARTTLS command is unavailable, this can be changed by setting email_smtp_force_tls in the config.

Before this can be considered for merging there are two things that need to be taken care of:

  • Unit test - I have no idea how to change email_smtp_force_tls at runtime, so I can't test this config option in both states.
  • Documentation - I can't see where email_smtp_* are documented. In particular, settings email_smtp_use_ssl and email_smtp_force_tls might confuse users

Patches can be found on the "starttls" branch of this git repo: https://gitorious.org/mediagoblin/mediagoblin-starttls/commits/starttls

Subtickets

Attachments (1)

861-starttls.patch (3.2 KB) - added by Jessica Tallon 6 years ago.
Patch to add documentation and unit tests

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by Matt Molyneaux

Owner: Matt Molyneaux deleted
Status: newreview

comment:2 Changed 6 years ago by Matt Molyneaux

Milestone: 0.7.0

comment:3 Changed 6 years ago by Matt Molyneaux

Rebased branch on 8917ffb1e73ac8ed0fc825113593e5e5ca9b4573 (current HEAD)

comment:4 Changed 6 years ago by Christopher Allan Webber

Merged!

But stupidly, I merged it before realizing that there were those two things above that needed addressing!

comment:5 Changed 6 years ago by Matt Molyneaux

The documentation issue appears to have already been reported as #734

comment:6 Changed 6 years ago by Jessica Tallon

Owner: set to Jessica Tallon
Status: reviewin_progress

Changed 6 years ago by Jessica Tallon

Attachment: 861-starttls.patch added

Patch to add documentation and unit tests

comment:7 Changed 6 years ago by Jessica Tallon

I have attached a patch which adds documentation and unit tests to cover the addition of the force email_smtp_force_starttls option (renamed from email_smtp_force_tls to reduce ambiguity).

comment:8 Changed 6 years ago by Matt Molyneaux

Ah yes, I probably should have named it that way in the first place. Good catch :)

comment:9 Changed 6 years ago by Jessica Tallon

Owner: Jessica Tallon deleted
Resolution: fixed
Status: in_progressclosed

This is fixed as of 7ffd4cf.

Note: See TracTickets for help on using tickets.