AppFlowy-Cloud icon indicating copy to clipboard operation
AppFlowy-Cloud copied to clipboard

[Bug] Appflowy cloud cannot send mail via port 25

Open apollo13 opened this issue 1 year ago • 3 comments

Describe the bug Appflowy solely supports configuration via APPFLOWY_MAILER_SMTP_HOST and sends to port 465 by default. Would be great if that were changeable (Ie similar to gotrue which has a _PORT variable or supporting host:port).

apollo13 avatar May 08 '24 14:05 apollo13

I recently stumbled into this strangeness too.

Problem

The GOTRUE_SMTP mailer will expectedly choose wether to use smtps or plain smtp, with optional STARTTLS. This comes via gotrue's use of github.com/netlify/mailme and in turn gopkg.in/gomail.v2's switch on port 465 (by reading GOTRUE_SMTP_PORT): https://github.com/go-gomail/gomail/blob/81ebce5c23dfd25c6c67194b37d3dd3f338c98b1/smtp.go#L46

But the APPFLOWY_MAILER_SMTP will always choose encrypted smtps. This leads to quite the confusion when you use the same configuration for both. I.e. if not using port 465 you can't get to a working setup -- one of the mailers will always timeout (trying plain smtp on a TLS connection) or write invalid commands (trying a TLS header on a plain connection).

Workaround

Currently we have two options: use port 465 for both mailers or use different ports for the two mailers, smtp for gotrue (the only choice when not using port 465) and smtps for the AppFlowy-Cloud mailer (disregarding what port you choose here).

There should be a warning in the config template that TLS ist always required for the AppFlowy-Cloud mailer.

Solution

The actual problem is in the use of AsyncSmtpTransport.relay() versus AsyncSmtpTransport.starttls_relay() https://github.com/AppFlowy-IO/AppFlowy-Cloud/blob/6972f9c4ab9f5f405fec0f251168b557443d6bdb/src/mailer.rs#L29

We could change to only use AsyncSmtpTransport.relay() if smtp_port is 465 and otherwise use AsyncSmtpTransport.starttls_relay(). (perhaps even with smtp_port % 1000 == 465 as operators might well choose ports like e.g. 1465 for smtps.)

But getting rid of this magic in favor of a detailed explicit configuration with from_url() would be best. The example shows a nice choice of options:

scheme tls parameter example remarks
smtps - smtps://smtp.example.com SMTP over TLS, recommended method
smtp required smtp://smtp.example.com?tls=required SMTP with STARTTLS required, when SMTP over TLS is not available
smtp opportunistic smtp://smtp.example.com?tls=opportunistic SMTP with optionally STARTTLS when supported by the server. Caution: this method is vulnerable to a man-in-the-middle attack. Not recommended for production use.
smtp - smtp://smtp.example.com Unencrypted SMTP, not recommended for production use.

I'm happy to provide a PR if there is consensus on the direction to take.

zuckschwerdt avatar Aug 30 '24 18:08 zuckschwerdt

a PR will be more than welcome my mailer only support starttls .. same behavior as gotrue will be great for consistency too

thomnico avatar Sep 11 '24 16:09 thomnico

We would need figure out wether to use a simple condition like smtp_port == 465, something more practical like smtp_port % 1000 == 465 (my smtps is on 1465) or switch things to the from_url() approach detailed above.

zuckschwerdt avatar Sep 11 '24 16:09 zuckschwerdt

  1. What are their needs?
    1. Currently, it is assumed that the SMTP server supports TLS. However, in many cases, STARTTLS is used as opposed to TLS. There should be an option to allow user to support STARTTLS.
  2. What challenges exist with our current solution?
    1. Implementation and fix has been proposed in the issue itself, so we just need to expose an option to allow users to support STARTTLS instead of TLS.
  3. What percentage of self-hosters might face the same issue?
    1. Small percentage, since most users are able to use TLS instead of STARTTLS. Though there are a sizable of users who complained that the deployment doesn’t work due to using a different SMTP port that support STARTTLS.

annieappflowy avatar Dec 17 '24 03:12 annieappflowy

Fixed in https://github.com/AppFlowy-IO/AppFlowy-Cloud/pull/1078 .

khorshuheng avatar Jan 02 '25 03:01 khorshuheng