I recently went through the emails:task and related code with the aim of testing each of the failure codepaths and touching up the error text.
I also discovered that the effects of setting DISCOURSE_SMTP_ENABLE_START_TLS=false are (mostly) nil. Setting this didn’t actually disable STARTTLS and in fact, it can coexist just fine with tls at connect time (DISCOURSE_SMTP_FORCE_TLS=true).
Before I merge this though, I think an admin warning in the dashboard for when DISCOURSE_SMTP_ENABLE_STARTTLS=false is being set is suitable. I imagine there’s at least one self-hoster out there who has set this but doesn’t need it and is actually relying on STARTTLS.
That sounds like good work! One thing that I (think I’ve) noticed is that the rake task doesn’t actually use the same code as the actual sending does (like from the /admin/email test page). I’m pretty sure that I’ve had a case where it worked in the UX but not the rake task (or maybe it was the opposite?)
While it’s fresh on your mind, it you could see that at least when it actually sends it does so using the same code that Discourse does, that would be great.
@supermathie is this worth PMing every admin on every site that has this variable set? Our current problem checks system will do that, and I’m not sure it is warranted here, given that for the most part, this has a nil effect. ideally, I’d want to only show this on the dashboard without pinging admin users, not sure our current problem checks structure supports that use case.
I think it is - I find it extremely likely that given how confused many admins are around email setup, someone has this variable set when they actually depend on starttls.
Nobody should have it set, probably.
Is rather have a spurious warning than silently break someone’s email setup.
The alternative is removing the check and neutering the variable so it does nothing at all.
It would be nice if the warning didn’t show up when the outgoing SMTP server is the localhost (ie matches the Discourse domain name) as TLS between the Docker container and the host isn’t needed as they are the same machine.
