plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
248 stars 188 forks source link

feat: extends sendmail wrapper #3941

Closed mamico closed 5 months ago

mamico commented 5 months ago

This change enables the use of new MailHost features, e.g. implicit_tls.

https://github.com/zopefoundation/Products.MailHost/pull/47 https://github.com/zopefoundation/zope.sendmail/pull/54

mister-roboto commented 5 months ago

@mamico thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

mamico commented 5 months ago

more info here: https://community.plone.org/t/configuring-plone-email-smtp-when-ssl-is-required/17075/7

manually tested against different smtp server configuration with

docker-compose.yml

version: '3'

services:
  smtp4dev:
    image: rnwood/smtp4dev:v3
    ports:
      - '1080:80'
      - '1025:25'
      - '1143:143'
    volumes:
      - smtp4dev-data:/smtp4dev
    environment:
      # Specifies the TLS mode to use. None=Off. StartTls=On demand if client supports STARTTLS.
      # ImplicitTls=TLS as soon as connection is established.
      - ServerOptions__TlsMode=None
      # - ServerOptions__TlsMode=StartTls
      # - ServerOptions__TlsMode=ImplicitTls
volumes:
  smtp4dev-data:

zope.sendmail >= 6.2 Products.MailHost branch https://github.com/zopefoundation/Products.MailHost/tree/implicit_tls

mamico commented 5 months ago

@jenkins-plone-org please run jobs

mamico commented 5 months ago

Just checking if I understand the code correctly: this is not dependent on your open PR in Products.MailHost, right? It can be used with the latest release, and then once your PR is merged, the wrapper will automatically support the new implicit_tls option.

Correct.

  • It would be nice to have all Products.MailHost options that are in the ZMI also available in the Plone control panel (Classic and Volto). But the current control panel is already easily misconfigured if you don't know what you are doing.
  • Alternatively, I wonder if on submit of the control panel we could automatically set the various TLS options in the ZMI depending on the port number.

I personally don't like the current behavior of having the same configuration in the mailhost and plone registry, however that's how it is. In any case we can implement your second option and let people tweaking the mailhost tool for non-standard use cases. And anyway, as you said, not in this pr

MrTango commented 3 months ago

It would be nice to have all Products.MailHost options that are in the ZMI also available in the Plone control panel (Classic and Volto). But the current control panel is already easily misconfigured if you don't know what you are doing. Alternatively, I wonder if on submit of the control panel we could automatically set the various TLS options in the ZMI depending on the port number.

just hit this patch. But in general, i don't like the patch at all. What about synching the control panel settings into the mailhost object? This way the only truth is in the MailHost object and guess hat you can have more than one in Plone. With this patch, you can't really because the settings are hard wired to the control panel. Now i have to patch the patch, have a solution which sounds like it sounds :(.

mauritsvanrees commented 3 months ago

Alternative idea: in Products/MailHost/MailHost.py patch _makeMailer. Do more or less the same patch as we do now, but only do it if self.id == 'MailHost'. Then the patch should not affect alternative MailHost instances that you setup.