tbar0970 / jethro-pmm

Jethro Pastoral Ministry Manager
GNU General Public License v3.0
36 stars 26 forks source link

roster_reminder.php should set the envelope From address #705

Open jefft opened 3 years ago

jefft commented 3 years ago

I've just set up the excellent roster_reminder.php script in our instance.

We would like roster emails to from from roster-coordinator@, so that replies to roster notifications go to a real person.

As background: when sending emails, there's an 'envelope from' and then there's the From: header. GMail at least cares very little for what's in the From: header. The envelope from is what becomes the sender address.

Currently rosterreminder.php gives two choices: 1) send email via swiftmailer which uses SMTP* settings in conf.php, 2) send via /usr/bin/sendmail. Option 1 isn't good for us, as we'd need to all Jethro emails as coming from roster-coordinator@. Option 2 would work, except that roster_reminder.php currently doesn't set the envelope from. That would involve using the 5th parameter of mail() to pass a -f flag through to sendmail. This is what I propose doing in the associated patch.

tbar0970 commented 3 years ago

HI Jeff I'm happy to accept the pull request. BUT I would recommend not using the php_mail option which (I believe) was more of a workaround during development.

When the script uses swiftmailer, it uses the SMPT server settings from the system config, but it takes the from address specified in the roster-reminder ini file. - https://github.com/tbar0970/jethro-pmm/blob/68f682ee4735fc8a60d4d8b4aedbfe5fa3d3a42f/scripts/roster_reminder.php#L292

However the envelope-from issue may exist for swiftmailer as well (and be worth fixing). I think there's the need to call ->setSender as well as ->setFrom on the swiftmailer message object.

jefft commented 3 years ago

Thanks. I just tested Swiftmailer (PHP_MAIL=0) again, and yes it can be made to work (i.e. set mail appears to come from EMAIL_FROM address), but only if I first tell GMail to allow it via Settings -> Accounts -> Send mail as -> Add another email address you own (stackoverflow). setSender() is not needed to achieve this.

With /usr/bin/sendmail (PHP_MAIL=1), setting the envelope from (i.e. what my patch does) is still needed regardless of GMail's setting, at least if my setup is typical. I have Postfix as the MTA, with a sasl_passwd entry that maps mail from roster-coordinator@ to be sent via GMail authenticated as roster-coordinator. Without the envelope from I can't do the sasl_passwd mapping.

As for choosing Swiftmailer vs. Postfix to interact with an external SMTP server, I'll take Postfix every time, since:

Basically the things in this article, but s/Jira/Jethro/g.

Swiftmailer has a /usr/bin/sendmail transport, so if you did want to drop direct mail() support and only go through Swiftmailer I'd be fine with that.

tbar0970 commented 3 years ago

Thanks - good points about the benefits of postfix! (for those with root access to their servers, or a server with postfix configured as required).

It appears that if SMTP_SERVER is not set, Jethro will fall back to using Swift_MailTransport, which just calls the PHP mail function, which is a (not fantastic) wrapper around sendmail - https://github.com/tbar0970/jethro-pmm/blob/0698c96d6e45707edc0e04c6444d04c4237b8db4/include/emailer.class.php#L27

However it looks like Swift_MailTransport is (to be) deprecated - https://github.com/swiftmailer/swiftmailer/issues/866 and we should use SendMailTransport instead.

In the specific case of the roster reminder script, the advantage of using SwiftMailer is that the script doesn't assemble the message content itself - it lets SwiftMailer do it, which is what Swiftmailer is good at!

tbar0970 commented 2 years ago

Reopening this so we can think about supporting envelope-from via swiftmailer, and support Swift_SendMail transport

jefft commented 2 years ago

Yes, in production environments this is quite important. Scenario: