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

[4.4] Add missing escaping of unsafe tags in mail copy to sender and notification to admin #43978

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Aug 25, 2024

Pull Request for Issue #43971 (partly) .

Alternative to PR #43981 .

Summary of Changes

This pull request fixes the following part of issue #43971 :

However, when looking at the copy sent to the sender, everything still works as expected. It seems that a filter is being applied, but only partially. There appear to be inconsistencies, as the mail copy functions as before, but the recipient’s version does not.

In addition, it fixes a similar issue for user registration: In the email to the user, the unsafe mail template tags {...} are escaped, but in the notification mails sent out to all superadmins (i.e. users with user creating permissions and receiving system emails) they are not escaped.

In opposite to my other PR #43981 , this PR here does not fix the part of the mentioned issue regarding custom fields.

Testing Instructions

In principle this PR could be checked by code review.

In addition you can follow the instructions below to check that nothing is broken, i.e. works as well as before.

Code review

  1. In file components/com_contact/src/Controller/ContactController.php in line 274 just before the $mailer->send call the unsafe tags are added:
$mailer->addUnsafeTags(['name', 'email', 'body', 'customfields']);

Then a new mailer is used for sending the copy a few lines below. The mailer uses the same template, so the same unsafe tags should be added before line 283 with the $mailer->send call for the copy. That's currently missing. This PR adds that.

  1. In file components/com_users/src/Model/RegistrationModel.php in line 511 just before the $mailer->send call the unsafe tags are added:
$mailer->addUnsafeTags(['username', 'password_clear', 'name']);

Later below for the notification mail to the admins, unsafe tags should be added before line 558, but they are currently missing.
Because for that email a different mail template is used which does not use the {password_clear} tag, the line should be:

$mailer->addUnsafeTags(['username', 'name']);

This PR adds that.

Test 1: Contact form email copy to sender

Create a contact linked to a user and a single contact menu item for that contact.

Enable the "Send Copy to Submitter" option in the mail options of the menu item.

Go to the menu item on the site and send an email with the contact form, having entered a valid email address for the sender and the "Send a copy to yourself" check box checked.

Check the email copy to the sender in your email client.

Test 2: Notification email copy to superadmins on user registration

Enable user registration on your site.

Make sure you (superadmin) get notification emails.

Register a new user on the site.

Check the notification email for you (superadmin) in your email client.

Actual result BEFORE applying this Pull Request

Test 1: Contact form email copy to sender

Works as expected.

Test 2: Notification email copy to superadmins on user registration

Works as expected.

Expected result AFTER applying this Pull Request

Test 1: Contact form email copy to sender

Still works as expected.

Test 2: Notification email copy to superadmins on user registration

Still works as expected.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@SniperSister
Copy link
Contributor

I have tested this item ✅ successfully on 8c455e3


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

@brianteeman
Copy link
Contributor

mail copy to sender really should be removed completely as its just a means of sending spam

@richard67
Copy link
Member Author

mail copy to sender really should be removed completely as its just a means of sending spam

@brianteeman Possibly ... but that's beyond the scope of this PR.

@richard67
Copy link
Member Author

Meanwhile I've created an alternative PR - or if this here gets merged an additional one - which contains the changes from this PR here plus removes the customfields from the unsafe tags and so fixes the issue completely: #43981

Please review or test that one, too.

@richard67
Copy link
Member Author

Closing in favour of #43981 .

@richard67 richard67 closed this Aug 26, 2024
@richard67 richard67 deleted the 4.4-dev-fix-mail-templates-2024-08-24 branch August 26, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants