Problem/Motivation

Wondering if there is some sort of security issue with allowing MN to set the From address. By this i mean the real From address. For some reason the MN code refers to the "reply to" value as "from". I don't think this is a standard use of this term and it doesn't match with Drupal's core MailManagerInterface:

public function mail($module, $key, $to, $langcode, $params = [], $reply = NULL, $send = TRUE);

where MN's $from variable is what is sent to mail() as $reply.

I know there are other modules (like Business Rules) which set the From address as the site's address unless the rule is set to use something else.

To set the From address the Email->deliver() method simply needs to set the From header. This can be transferred now from the $notifier->send() method like this:

    $headers['From'] = 'from@example.com';
    $options = [
      'mail' => 'to@example.com',
      'context' => $policy,
      'output' => [
        'headers' => $headers,
      ],
      'from' => 'REPLY@example.com',
    ];

    $notifier->send($message, $options);

The only issue is that the deliver method doesn't pass along the rest of $output (should really be called $params to be consistent with core MailInterface) values sent along.

Perhaps some reason this isn't supported; but i'll post a patch to support this. Would be great to rename the variables to better line up with core variables; but i'll leave that for now.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 allow-sending-headers-3211901-8.patch2.03 KBmjmorley
#7 allow-sending-headers-3211901-7.patch2.14 KBmjmorley
#6 allow-sending-headers-3211901-6.patch1.97 KBmjmorley
#2 allow-sending-headers-3211901-1.patch1.88 KBliquidcms

Issue fork message_notify-3211901

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

liquidcms created an issue. See original summary.

liquidcms’s picture

liquidcms commented
StatusFileSize
new allow-sending-headers-3211901-1.patch1.88 KB

I tried to clean up some of the variables used in Email->deliver(); but some of these are part of the module's external API so can't fix these without breaking people's code. But best possible to try to line this up with Druapl's mail function and make this less confusing. This patch also allows passing in mail headers to allow setting CC, BCC and From (the real From not Reply as it is called in the module).

liquidcms’s picture

liquidcms commented

the above patch would then let me use message notify send like this:

  private function sendEmail($form_state) {
    $policy = Node::load($form_state->getValues()['policy_id']);
    $notifier = \Drupal::service('message_notify.sender');
    // Create a message with node author as message creator.
    $message = Message::create(['template' => 'test_email', 'uid' => $policy->getOwnerId()]);
    $message->set('field_message_header', $form_state->getValues()['header_text']);
    $message->save();

    $headers['From'] = 'from@example.com';
    $headers['Cc'] = 'cc@example.com';

    $attachment = array(
      'filepath' => $attached_file_uri,   // like public://some-file.pdf
      'filename' => 'some-file.pdf',
      'filemime' => 'application/pdf'
    );
   
    $options = [
      'mail' => 'to@example.com',    // really should be 'to'
      'context' => $policy,
      'params' => [
        'headers' => $headers,
        'attachments' => [$attachment],  // if using Mimemail
      ],
      'from' => 'reply@example.com',   // really should be 'reply'
    ];

    $notifier->send($message, $options);
  }
liquidcms’s picture

liquidcms commented
Title: Allow setting From address. » Allow setting From, CC, BCC and adding attachments.
liquidcms’s picture

liquidcms commented
Status: Active » Needs review
mjmorley’s picture

mjmorley commented
StatusFileSize
new allow-sending-headers-3211901-6.patch1.97 KB

The patch wasn't applying cleanly for me, so I've updated the patch but using the same changes as made in #2
This applies cleanly and fixes the issue at hand, I'm now able to set headers in the config to use in message notify.

mjmorley’s picture

mjmorley commented
StatusFileSize
new allow-sending-headers-3211901-7.patch2.14 KB

I noticed an error would occur if $this->configuration['params'] was ever null. So I have amended the patch to do a check before merging with the $params variable.

mjmorley’s picture

mjmorley commented
StatusFileSize
new allow-sending-headers-3211901-8.patch2.03 KB

Updated the above patch to use isset() to avoid a warning

gorkagr made their first commit to this issue’s fork.

gorkagr opened merge request !6

gorkagr’s picture

gorkagr commented

Hi all

Thanks for the patch. I just needed to send cc/bcc when creating a message and I saw it created.

I have added some code in the hook_mail() part, as the message was not sent to the cc/bcc without that part (at least for me).
As in the examples, it is written in general 'Bcc' and 'Cc', but I have checked that for drupal to work, it needs to be 'cc' and 'bcc'; therefore the strtolower(). After submitting the patch i have seen from @liquidcms that there is also the option for 'From', but i havent tested that header.

Best

bluegeek9 made their first commit to this issue’s fork.