Bug 8085

Summary: Make OJS more SPF-compatible in email sending
Product: OMP Reporter: Alec Smecher <alec>
Component: GeneralAssignee: PKP Support <pkp-support>
Status: NEW ---    
Severity: normal CC: anne.durand, bdgregg, ctgraham, info, jmacgreg
Priority: P3    
Version: 1.2   
Hardware: All   
OS: All   
Version Reported In: Also Affects:
Bug Depends on:    
Bug Blocks: 8966    
Attachments: Patch against OJS 2.4.x
Patch against OJS 2.4.x
Patch against OJS 2.3.x
Patch against OJS 2.4.x

Description Alec Smecher 2013-01-17 15:16:02 PST
Make OJS more SPF-compatible in email sending:
- Use Reply-To where we're currently using From
- Add an email header field to journal setup and all emails
- Add a new default setting for the header field
Comment 1 Alec Smecher 2013-01-17 15:20:02 PST
SPF email tune-ups
https://github.com/pkp/pkp-lib/commit/4acff7d16280343287c6ed30ea4f055f75f6b0a6
Comment 2 Alec Smecher 2013-01-17 15:20:03 PST
SPF email tune-ups
https://github.com/pkp/ojs/commit/2a4f6775b466c09b67346fced7358b5e971b4bd7
Comment 3 Alec Smecher 2013-01-23 12:19:14 PST
Created attachment 3903 [details]
Patch against OJS 2.4.x
Comment 4 Alec Smecher 2013-01-23 12:20:38 PST
Created attachment 3904 [details]
Patch against OJS 2.4.x

(Prior patch contained submodule hashes)
Comment 5 Alec Smecher 2013-01-23 12:32:54 PST
Created attachment 3905 [details]
Patch against OJS 2.3.x
Comment 6 Alec Smecher 2013-01-23 14:00:07 PST
Transferring over to James for some testing feedback.
Comment 8 Alec Smecher 2013-04-30 15:16:03 PDT
Ported to OMP master. Needs porting to OCS.
Comment 10 Alec Smecher 2013-05-01 15:42:35 PDT
SPF to OMP
https://github.com/pkp/pkp-lib/commit/196c11249d6c7a5279bcefe845f764649ae0
d73c
Comment 11 Alec Smecher 2014-05-22 14:09:36 PDT
Created attachment 4028 [details]
Patch against OJS 2.4.x
Comment 12 Clinton Graham 2014-08-20 06:39:09 PDT
I'm not following why in the 2.4.x patch some cases replace the setFrom($x,$y) with an equivalent setReplyTo($x,$y) [c.f. pages/login/LoginHandler.inc.php or lib/pkp/classes/notification/PKPNotificationManager.inc.php] and some cases replace the setFrom($x,$y) with a setReplyTo(null) [c.f. classes/author/form/submit/AuthorSubmitStep5Form.inc.php or plugins/paymethod/manual/ManualPaymentPlugin.inc.php].
Comment 13 Alec Smecher 2014-08-20 08:51:54 PDT
Clinton, in the MailTemplate.inc.php section of the patch you can see that the reply-to is set by default to the current user. In some cases that's not desired behavior -- e.g. when sending an article confirmation the current user is the author, but the reply-to should be the journal contact. The setReplyTo(null) call removes the default in those cases.
Comment 14 Clinton Graham 2014-08-20 12:52:53 PDT
Got it.

So FROM is the journal contact (or the site contact if not in a journal context).

And REPLY-TO is the user (or otherwise specified address per the calling method).

This only covers SPF if the journal's contact is an address at the journal's host, correct?  In the case where the journal contact is an off-host address, I think we would want the FROM to be a noreply at the host, and the REPLY-TO to use the FROM/REPLY-TO logic as above.  I would anticipate:
- a sitewide configuration to force all journals to use a single "noreply" sender.
--  when enabled, FROM would be $NOREPLY and REPLY_TO would be ($REPLY_TO ? $REPLY_TO : $FROM)
-- when disabled, the logic in this patch would control

- a plugin allowing a per-journal email configuration (per separate discussion)
-- this would provide an alternate to the above.

Any thoughts?

And a final question: Any reason not to move the email configuration out of config.inc.php and into the web-based site administration?
Comment 15 Alec Smecher 2014-08-21 11:06:23 PDT
I actually haven't seen journals using no-reply addresses before -- they tend to use address for which they know SPF delivery via the SMTP config will work. Some users set up SMTP via services like gmail, which automatically set the "From" address to the user account that's being used to send.
Comment 16 AD 2014-09-02 05:17:59 PDT
To use the current user into the From field, we had to modify the code of MailTemplate.inc.php as following :


               if ($user) {
                        $this->setReplyTo($user->getEmail(), $user->getFullName());
                        $this->setFrom($user->getEmail(), $user->getFullName());
                } elseif (is_null($journal) || is_null($journal->getSetting('contactEmail'))) {
                        $site =& Request::getSite();
                        $this->setFrom($site->getLocalizedContactEmail(), $site->getLocalizedContactName());

                } else {
                        $this->setFrom($journal->getSetting('contactEmail'), $journal->getSetting('contactName'));
                }
Comment 17 Alec Smecher 2014-09-05 07:45:03 PDT
Pull request opened (not merged):
Add SPF enhancements
https://github.com/pkp/pkp-lib/pull/164
Comment 18 Alec Smecher 2014-09-05 07:48:02 PDT
Pull request opened (not merged):
Add SPF enhancements
https://github.com/pkp/ojs/pull/279
Comment 19 Alec Smecher 2014-09-11 11:59:40 PDT
Moved to OJS 2.4.6, as we're just about to release OJS 2.4.5.
Comment 20 Alec Smecher 2014-09-11 13:28:03 PDT
Pull request assigned (not merged):
Add SPF enhancements
https://github.com/pkp/ojs/pull/279
Comment 21 Alec Smecher 2014-09-18 08:25:03 PDT
Pull request closed (not merged):
Add SPF enhancements
https://github.com/pkp/pkp-lib/pull/164
Comment 22 Alec Smecher 2014-09-18 08:26:03 PDT
Pull request closed (not merged):
Add SPF enhancements
https://github.com/pkp/ojs/pull/279
Comment 23 Alec Smecher 2014-09-18 08:38:17 PDT
Note: The attached pull requests were automatically closed by github.com during the branch rename from ojs-stable-2_4 to ojs-dev-2_4. They should still be considered.
Comment 24 Alec Smecher 2014-09-18 08:45:02 PDT
Pull request opened (not merged):
Add spf enhancements
https://github.com/pkp/pkp-lib/pull/180
Comment 25 Alec Smecher 2014-10-21 06:06:03 PDT
Pull request synchronize (not merged):
Add spf enhancements
https://github.com/pkp/pkp-lib/pull/180
Comment 26 Alec Smecher 2014-10-21 06:12:03 PDT
Pull request opened (not merged):
Add SPF enhancements ##ulsdevteam/add-spf-enhancements##
https://github.com/pkp/ojs/pull/337
Comment 27 Alec Smecher 2014-10-21 07:24:03 PDT
Pull request synchronize (not merged):
Add SPF enhancements ##ulsdevteam/add-spf-enhancements##
https://github.com/pkp/ojs/pull/337
Comment 28 Alec Smecher 2014-10-29 05:39:02 PDT
Pull request synchronize (not merged):
Add spf enhancements
https://github.com/pkp/pkp-lib/pull/180
Comment 29 Alec Smecher 2014-10-29 05:46:02 PDT
Pull request synchronize (not merged):
Add SPF enhancements ##ulsdevteam/add-spf-enhancements##
https://github.com/pkp/ojs/pull/337
Comment 30 Alec Smecher 2014-10-30 12:22:23 PDT
*** Bug 8966 has been marked as a duplicate of this bug. ***
Comment 31 Clinton Graham 2014-10-30 14:10:03 PDT
allow for a default email sender to be forced in configuration multidomain-install DMARC compatibility option
https://github.com/pkp/pkp-lib/commit/4380a2850163de8b706c3215983b604d260ec802
Comment 33 Alec Smecher 2014-10-30 14:10:03 PDT
Pull request closed (merged):
Add SPF enhancements ##ulsdevteam/add-spf-enhancements##
https://github.com/pkp/ojs/pull/337
Comment 34 Clinton Graham 2014-10-30 14:10:03 PDT
*8966* patch OJS for SPF-compatibility, c.f. http://pkp.sfu.ca/bugzilla/attachment.cgi?id=4028 ##ulsdevteam/add-spf-enhancements##
https://github.com/pkp/ojs/commit/bdcc2896a8f46cf29fcbcd883e4304b522e62d7b
Comment 35 Alec Smecher 2014-10-30 14:10:03 PDT
Pull request closed (merged):
Add spf enhancements
https://github.com/pkp/pkp-lib/pull/180
Comment 36 Alec Smecher 2014-10-30 14:10:37 PDT
Committed to OJS pre-2.4.6; needs forward-porting to OMP/OJS master.