Project

General

Profile

Actions

Bug #5975

closed

Audit all mail plugins on the Commons

Added by Raymond Hoh about 6 years ago. Updated almost 6 years ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Daniel Jones
Category name:
WordPress Plugins
Target version:
Start date:
2016-09-01
Due date:
% Done:

0%

Estimated time:

Description

Due to the MailPoet issue, I did a quick search through our plugins to see what other plugins are using PHPMailer.

Here are the other plugins bundling their own version of PHPMailer:
- cforms
- threewp-broadcast
- wp-mailinglist

I haven't checked to see if it is possible to override the Sender property in those plugins yet, but wanted to create this issue so we can do so.


Related issues

Related to CUNY Academic Commons - Bug #5971: MailPoet should use commons-no-reply@gc.cuny.edu for bounced emails ResolvedRaymond Hoh2016-08-30

Actions
Actions #1

Updated by Matt Gold about 6 years ago

awesome -- thanks for looking into this, Ray

Actions #2

Updated by Boone Gorges about 6 years ago

  • Target version changed from 1.9.27 to 1.9.28
Actions #3

Updated by Boone Gorges about 6 years ago

  • Target version changed from 1.9.28 to 1.9.29
Actions #4

Updated by Boone Gorges about 6 years ago

  • Target version changed from 1.9.29 to 1.9.30
Actions #5

Updated by Boone Gorges almost 6 years ago

  • Target version changed from 1.9.30 to 1.9.31
Actions #6

Updated by Boone Gorges almost 6 years ago

  • Target version changed from 1.9.31 to 1.9.32
Actions #7

Updated by Boone Gorges almost 6 years ago

  • Related to Bug #5971: MailPoet should use commons-no-reply@gc.cuny.edu for bounced emails added
Actions #8

Updated by Boone Gorges almost 6 years ago

  • Status changed from New to Assigned
  • Assignee set to Daniel Jones
  • Target version changed from 1.9.32 to 1.11

Dan, can you take on this task? For each of the plugins Ray mentions, see whether it's possible to override the Sender property with our own. See #5971 for background.

Actions #9

Updated by Daniel Jones almost 6 years ago

cforms uses an option to store all of its settings, including email. We could use the filter 'option_cforms_settings' to capture the options array and modify it.

I actually wasn't able to find anywhere in the threewp-broadcast plugin that an email was actually sent, except for in the unit tests, so I wasn't able to see if there was a place to intercept and change the from email.

wp-mailinglist also uses get_option to get the from email, although it uses a class method which calls the standard WP method. They do provide their own filter, 'newsletters_get_option', which we could use to change the email.

Is this what you all were looking for?

Actions #10

Updated by Boone Gorges almost 6 years ago

Hi Dan - Thanks for looking into this.

We are not concerned with the From address of the outgoing emails, but the "Return-Path" header, which in PHPMailer is called the Sender property. Plugins that send email using wp_mail() have their PHPMailer instance sent through the 'phpmailer_init' action, which allows us to do this: https://github.com/cuny-academic-commons/cac/blob/1.10.x/wp-content/mu-plugins/cac-functions.php#L799 But plugins like the ones listed on this ticket use their own versions of PHPMailer. So the trick is to see whether there's some way we can tap into their sending process to modify outgoing emails just before their sent. It could be that the answer is "no" in the case of these specific plugins, in which case we could consider sending PRs to the authors to add options or hooks.

Actions #11

Updated by Daniel Jones almost 6 years ago

Okay I see now.

-cforms: Does not expose any way of changing phpmailer's Sender property, but could easily (see cforms/phpmailer/cforms_phpmailer.php).
-wp-mailinglist: Still uses its class's get_option method, which we can filter. It gets its "bounceemail" option.
-threewp-broadcast: I'm still not seeing a place in the codebase where an email is actually sent, but I also don't totally understand what the plugin does. It looks like it does set the Sender property, and sets it to be the same as the From property. Seems like it'd be easy to add a filter.

Actions #12

Updated by Boone Gorges almost 6 years ago

Thanks, Dan.

In cases where it appears that we'll need a filter or action to make the necessary change, would you mind researching to see whether the plugins in question have a public dev repo (probably on GitHub), and then sending PRs?

For wp-mailinglist, what is the default value of the "bounceemail" option?

Actions #13

Updated by Daniel Jones almost 6 years ago

Hey Boone - very sorry for the long delay on this issue.

For cforms: I see a repository on Github that I think is active, but for some reason I'm not actually seeing it in our plugin list in the Dashboard. Should I just try and add in the filter and push the pull request from an clean WP install? Do you also have that issue?

For threewp-broadcast: There is a Github repository, but it hasn't been updated in a couple years. The plugin itself seems to be updated pretty regularly. What would be the best thing to do?

For wp-mailinglist: The default is the "admin_email" option, which looks like right now on my local install. It's something that people can actually set/override themselves if they've activated the plugin.

Actions #14

Updated by Boone Gorges almost 6 years ago

For cforms: I see a repository on Github that I think is active, but for some reason I'm not actually seeing it in our plugin list in the Dashboard. Should I just try and add in the filter and push the pull request from an clean WP install? Do you also have that issue?

Ah. Yes, cforms is installed on the Commons, but is disabled for new sites. See #2029. Let's skip this one.

For threewp-broadcast: There is a Github repository, but it hasn't been updated in a couple years. The plugin itself seems to be updated pretty regularly. What would be the best thing to do?

According to the plugin page, https://bitbucket.org/edward_electric/broadcast/src is the repo. Can you take a look there?

For wp-mailinglist: The default is the "admin_email" option, which looks like right now on my local install. It's something that people can actually set/override themselves if they've activated the plugin.

If it's got a sensible default, I think that's enough.

Actions #15

Updated by Daniel Jones almost 6 years ago

Just created the pull request at the ThreeWP Broadcast Bitbucket repo with a filter for the Sender property. Will let you know when I hear back!

Actions #16

Updated by Daniel Jones almost 6 years ago

Here's what I got back:

"Broadcast doesn't send any mails. The mail class in the sdk should never be used by Wordpress or any plugins other than those that use the Plainview SDK.

Why would that function need to be modified at all?"

I noted above that I didn't a place where the plugin actually sent an email, but I'm familiar enough with the plugin to totally know what it does and what the Plainview SDK is. Do you have any idea?

Actions #17

Updated by Boone Gorges almost 6 years ago

  • Status changed from Assigned to Resolved

I don't really have any idea what the Plainview SDK library is, but I guess if the plugin doesn't send any emails, there's nothing we need to worry about.

I think we can go ahead and close out this ticket, right?

Actions

Also available in: Atom PDF