Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change email notification to use site from address (Fix #9261) #13518

Merged
merged 1 commit into from
May 26, 2017
Merged

Change email notification to use site from address (Fix #9261) #13518

merged 1 commit into from
May 26, 2017

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Jan 8, 2017

Pull Request for Issue #9261

Summary of Changes

The email notification sent when sending a private message uses the "from" user's email address as the sender which can cause some sending failures (see referenced issue for examples). This changes the process to set the "from" user as the reply-to address instead.

As well, proper error checks will be made when setting the reply-to and recipient addresses and the message will not be sent if this step fails. Additionally, a message will be displayed if sending

Concurrent with #13504 the log statements are wrapped in try/catch.

Testing Instructions

Pre-patch, an email message sent via the "Private Message" feature of com_messages will have the "from" address set as the sending user. Post-patch, the message will use the site's configured from address with the sending user set as the reply-to address.

Documentation Changes Required

N/A

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Jan 8, 2017
@RonakParmar
Copy link

I have tested this item ✅ successfully on db2e207


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13518.

@brianteeman
Copy link
Contributor

As pointed out in #16107 the site email address is not a required or a validated field


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13518.

@mbabker
Copy link
Contributor Author

mbabker commented May 19, 2017

Still a better option than potentially spoofing someone's email address as if it's configured properly that would be valid data from the site owner.

@brianteeman
Copy link
Contributor

i agree i was just pointing it out and that with this PR there is even more reason to merge #16107

@Quy
Copy link
Contributor

Quy commented May 19, 2017

Somewhat related: Our forms were setup to use the user's email for the FROM, and some got bounced/rejected:

550 5.1.0 ###.###.###.### is not allowed to send from <example.com> per it's SPF Record. Please inspect your SPF settings, and try again. IB508 http://x.co/srbounce

Now FROM is the site's email and Reply-to is the user's email.

@Quy
Copy link
Contributor

Quy commented May 20, 2017

Why add REPLY-TO? The email notification is generic. There is no context to the message to know what to reply to.

Subject: A new private message has arrived from Acme Corp
Message: Please log in to http://www.example.com/administrator/index.php?option=com_messages&view=message&message_id=2 to read your message.

@brianteeman
Copy link
Contributor

@Quy you are probably correct but I guess someone could be overriding the message

@Quy
Copy link
Contributor

Quy commented May 25, 2017

I have tested this item ✅ successfully on db2e207


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13518.

@joomla-cms-bot joomla-cms-bot removed Language Change This is for Translators PR-staging labels May 25, 2017
@ghost
Copy link

ghost commented May 25, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 25, 2017
@rdeutz rdeutz merged commit 1f6b7d0 into joomla:staging May 26, 2017
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging and removed RTC This Pull Request is Ready To Commit labels May 26, 2017
@rdeutz rdeutz added this to the Joomla 3.7.3 milestone May 26, 2017
@mbabker mbabker deleted the fix-9261 branch May 26, 2017 17:53
izharaazmi added a commit to izharaazmi/joomla-cms that referenced this pull request May 31, 2017
* staging: (1779 commits)
  Deprecate modules correction (joomla#16362)
  Removed unnecessary code in com_content (joomla#14628)
  Stop installation if minimum requirement isn't met. (joomla#15890)
  Deprecate parts of com_modules (joomla#16152)
  LICENSE.txt (joomla#16317)
  [a11y] [com_fields] icons in modals (joomla#15047)
  Load jQuery in associations edit layout (joomla#16240)
  HHVM was removed. Remove from travis conditionals (joomla#16281)
  Fix Stylesheet Mime type keeping b/c (joomla#16284)
  re-merge joomla#15068 and joomla#16256
  Add showon attribute to add mailto link parameter (joomla#16282)
  Fix notices on Contact form (joomla#16279)
  Updating dutch TinyMCE files
  Media upload form margin (joomla#16253)
  Changes to display atom feeds correctly (joomla#16105)
  admin mod_latest (joomla#16277)
  Add truncate class and implement in popular articles module (joomla#16257)
  Using the "Special:MyLanguage" tag for links pointing to docs.joomla.org (joomla#15858)
  Change email notification to use site from address (Fix joomla#9261) (joomla#13518)
  Fix double encode ampersand in contact select list value (joomla#16268)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants