Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30960

No option in email settings to speccify SSL or TLS (SMTPSecure property of PHPMailer)

    Details

    • Testing Instructions:
      Hide

      1. find a SMTP server that accepts TLS or SSL only
      2. specify the full SMTP hostname:port in Email settings, e.g.:smtp.example.com:587
      3. open up /login/forgot_password.php in browser and trigger a password reset email
      4. the cannotmailconfirm error show up

      Show
      1. find a SMTP server that accepts TLS or SSL only 2. specify the full SMTP hostname:port in Email settings, e.g.:smtp.example.com:587 3. open up /login/forgot_password.php in browser and trigger a password reset email 4. the cannotmailconfirm error show up
    • Workaround:
      Hide

      in moodle 2.2, editing get_mailer() function in /lib/moodlelib.php to add:
      $mailer->SMTPSecure = "tls"; // or "ssl";
      after $mailer->IsSMTP();

      Similar fixes can also be done directly in the /lib/PHPMailer files to force SSL/TLS.

      Show
      in moodle 2.2, editing get_mailer() function in /lib/moodlelib.php to add: $mailer->SMTPSecure = "tls"; // or "ssl"; after $mailer->IsSMTP(); Similar fixes can also be done directly in the /lib/PHPMailer files to force SSL/TLS.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_30_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30960_master

      Description

      There is no option to specify the SMTPSecure property of PHPMailer in the admin email settings section. Specifying just the :port by itself is insufficient to trigger PHPMailer to use SSL/TLS connection prefix and will not initiate the connection correctly.

      If I remember correctly, this has been an issue since early 1.x (1.6?).

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          This option could be valuable. The specification of the mail server needs to be reviewed.

          Another workaround is to specify the protocol in the hostname of the server.

          Show
          salvetore Michael de Raadt added a comment - This option could be valuable. The specification of the mail server needs to be reviewed. Another workaround is to specify the protocol in the hostname of the server.
          Hide
          smaclean Shaun Maclean added a comment -

          Moodle 2.2.2+ (I suspect line numbers may change depending on precise build version)

          In order to use Office365 smtp for outbound moodle mails using workaround specified above:
          In Function get_mailer (line 4885 in my version)
          find line containing: $mailer->IsSMTP(); // use SMTP directly (line 4948 in my version)
          add a line below containing: $mailer->SMTPSecure = "tls"; (inserted at 4949 in my version)

          Adjust the following as appropriate for your office 365 account / moodle email account
          In plugins > message outputs > email
          SMTP hosts = pod12345.outlook.com:587
          SMTP username = yourmoodlenoreplyaccount@youremaildomain.as.appropriate
          SMTP password = yourmoodlenoreplyaccountpassword

          I also set the No-Reply address the same as the SMTP username

          note that specifying tls://pod12345.outlook.com:587 did not make it work, thus requiring the $mailer->SMTPSecure = "tls"; tweak.

          Show
          smaclean Shaun Maclean added a comment - Moodle 2.2.2+ (I suspect line numbers may change depending on precise build version) In order to use Office365 smtp for outbound moodle mails using workaround specified above: In Function get_mailer (line 4885 in my version) find line containing: $mailer->IsSMTP(); // use SMTP directly (line 4948 in my version) add a line below containing: $mailer->SMTPSecure = "tls"; (inserted at 4949 in my version) Adjust the following as appropriate for your office 365 account / moodle email account In plugins > message outputs > email SMTP hosts = pod12345.outlook.com:587 SMTP username = yourmoodlenoreplyaccount@youremaildomain.as.appropriate SMTP password = yourmoodlenoreplyaccountpassword I also set the No-Reply address the same as the SMTP username note that specifying tls://pod12345.outlook.com:587 did not make it work, thus requiring the $mailer->SMTPSecure = "tls"; tweak.
          Hide
          andyjdavis Andrew Davis added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          andyjdavis Andrew Davis added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
          Hide
          danielneis Daniel Neis added a comment -

          Hello,

          this is still true for 2.9
          I am using SSL only, authenticated only mail at moodle.novoaeon.com.br and had to change lib/phpmailer/class.phpmailer.php to make things works.
          Here is a link for the patch:

          https://github.com/danielneis/moodle/commit/7bea62c0367e378e95abdba0990a3aa281e84923

          Hope that you like =)

          Show
          danielneis Daniel Neis added a comment - Hello, this is still true for 2.9 I am using SSL only, authenticated only mail at moodle.novoaeon.com.br and had to change lib/phpmailer/class.phpmailer.php to make things works. Here is a link for the patch: https://github.com/danielneis/moodle/commit/7bea62c0367e378e95abdba0990a3aa281e84923 Hope that you like =)
          Hide
          marina Marina Glancy added a comment - - edited

          Hi Daniel,
          thanks for sharing it. We usually try to to modify 3rd party libraries unless necessary. Can you confirm please that this change can not be done in moodle_phpmailer.php instead?

          P.S. you have some stray spaces in front of <?php, it will break all file downloads

          Show
          marina Marina Glancy added a comment - - edited Hi Daniel, thanks for sharing it. We usually try to to modify 3rd party libraries unless necessary. Can you confirm please that this change can not be done in moodle_phpmailer.php instead? P.S. you have some stray spaces in front of <?php, it will break all file downloads
          Hide
          danielneis Daniel Neis added a comment -

          Hello,

          i got it working on lib/phpmailer/moodle_phpmailer.php , but i had to copy the entire function.

          Kind regards,
          Daniel

          Show
          danielneis Daniel Neis added a comment - Hello, i got it working on lib/phpmailer/moodle_phpmailer.php , but i had to copy the entire function. Kind regards, Daniel
          Hide
          cibot CiBoT added a comment -

          Code verified against automated checks with warnings.

          Checked MDL-30960 using repository: https://github.com/danielneis/moodle

          More information about this report

          Show
          cibot CiBoT added a comment - Code verified against automated checks with warnings. Checked MDL-30960 using repository: https://github.com/danielneis/moodle master (0 errors / 1 warnings) [branch: MDL-30960-master | CI Job ] phplint (0/0) , php (0/0) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (0/0) , savepoint (0/0) , thirdparty (0/1) , More information about this report
          Hide
          marina Marina Glancy added a comment - - edited

          Hello Daniel,
          I can't test your patch at the moment but reviewing the code I don't understand the purpose of the change.
          The variable $port is assigned again in line 229 and it overwrites whatever you set there.

          Your previous commit removed this assignment: https://github.com/danielneis/moodle/commit/7bea62c0367e378e95abdba0990a3aa281e84923#diff-373074cc6b99e47fb143890641dee0eaL1320

          This is what I meant in my previous review:

          public function smtpConnect($options = array()) {
              if ($this->SMTPSecure == 'tls') || ($this->SMTPSecure == 'ssl')) {
                  $this->Port = 587;
              }
              parent::smtpConnect($options);
          }
          

          but in any case - this patch only changes the default value for the port. If port is specified in the url, it would be overridden anyway.

          I'm not really sure what you are trying to fix here. Do you still experience the problem outlined in this issue description?

          Show
          marina Marina Glancy added a comment - - edited Hello Daniel, I can't test your patch at the moment but reviewing the code I don't understand the purpose of the change. The variable $port is assigned again in line 229 and it overwrites whatever you set there. Your previous commit removed this assignment: https://github.com/danielneis/moodle/commit/7bea62c0367e378e95abdba0990a3aa281e84923#diff-373074cc6b99e47fb143890641dee0eaL1320 This is what I meant in my previous review: public function smtpConnect($options = array()) { if ($this->SMTPSecure == 'tls') || ($this->SMTPSecure == 'ssl')) { $this->Port = 587; } parent::smtpConnect($options); } but in any case - this patch only changes the default value for the port. If port is specified in the url, it would be overridden anyway. I'm not really sure what you are trying to fix here. Do you still experience the problem outlined in this issue description?
          Hide
          danielneis Daniel Neis added a comment -

          Hello,

          I've re-tested and really, the only missing part is

            public $AuthType = 'PLAIN';
          

          Better yet, it should be a configuration setting.

          Show
          danielneis Daniel Neis added a comment - Hello, I've re-tested and really, the only missing part is public $AuthType = 'PLAIN'; Better yet, it should be a configuration setting.
          Hide
          marina Marina Glancy added a comment -

          Hi Daniel, this makes sense. Although here we are again where we started - can you make this change to the Moodle wrapper instead of the library file?

          Show
          marina Marina Glancy added a comment - Hi Daniel, this makes sense. Although here we are again where we started - can you make this change to the Moodle wrapper instead of the library file?
          Hide
          danielneis Daniel Neis added a comment -

          Oh, yes, of course, I'm sorry.

          I've moved the changes to the Moodle wrapper and also made it a config setting at message/output/email/settings.php
          Hope you like =)

          Kind regards,
          Daniel

          Show
          danielneis Daniel Neis added a comment - Oh, yes, of course, I'm sorry. I've moved the changes to the Moodle wrapper and also made it a config setting at message/output/email/settings.php Hope you like =) Kind regards, Daniel
          Hide
          marina Marina Glancy added a comment - - edited

          Thanks Daniel!
          Can you please rebase the branch over master instead of 2.9, 2.9 is already released and we can not integrate improvements into it.

          Please note that $CFG->smtpauthtype will always be set. The default value '' that you set in message/output/email/settings.php is not in the list of options - this is not correct, it means that installed vs. upgraded site behaviour will be different. When moodle is installed all configuration settings are filled with their default values. When site is upgraded via web interface, new settings appear on the confirmation screen and by default it will use the first value from the list of options (because there is no empty string in the options)

          Show
          marina Marina Glancy added a comment - - edited Thanks Daniel! Can you please rebase the branch over master instead of 2.9, 2.9 is already released and we can not integrate improvements into it. Please note that $CFG->smtpauthtype will always be set. The default value '' that you set in message/output/email/settings.php is not in the list of options - this is not correct, it means that installed vs. upgraded site behaviour will be different. When moodle is installed all configuration settings are filled with their default values. When site is upgraded via web interface, new settings appear on the confirmation screen and by default it will use the first value from the list of options (because there is no empty string in the options)
          Hide
          danielneis Daniel Neis added a comment -

          ok, thanks for the review.
          i've rebased the branch and removed the isset() call.

          Show
          danielneis Daniel Neis added a comment - ok, thanks for the review. i've rebased the branch and removed the isset() call.
          Hide
          marina Marina Glancy added a comment -

          Thanks Daniel, looks good. The (!empty()) check is still unnecessary because it never can be empty but it does not really change anything and I don't want to bounce the issue back again. Submitting for integration.

          To integrator: note that now the default value is 'LOGIN' instead of '' as it used to be. However the empty value was substituted with LOGIN anyway here https://github.com/moodle/moodle/blob/master/lib/phpmailer/class.smtp.php#L350 , therefore the functionality has not changed

          Show
          marina Marina Glancy added a comment - Thanks Daniel, looks good. The (!empty()) check is still unnecessary because it never can be empty but it does not really change anything and I don't want to bounce the issue back again. Submitting for integration. To integrator: note that now the default value is 'LOGIN' instead of '' as it used to be. However the empty value was substituted with LOGIN anyway here https://github.com/moodle/moodle/blob/master/lib/phpmailer/class.smtp.php#L350 , therefore the functionality has not changed
          Hide
          marina Marina Glancy added a comment -

          TO INTEGRATOR: This change needs version bump but I know that issue will be waiting for the end of on-sync and version number will be changed by then. Can you please add version bump yourself?

          Show
          marina Marina Glancy added a comment - TO INTEGRATOR: This change needs version bump but I know that issue will be waiting for the end of on-sync and version number will be changed by then. Can you please add version bump yourself?
          Hide
          dmonllao David Monllaó added a comment -

          Thanks Daniel. Adding integration_held label as we are still on-sync period. Will be picked for the next integration cycle.

          Show
          dmonllao David Monllaó added a comment - Thanks Daniel. Adding integration_held label as we are still on-sync period. Will be picked for the next integration cycle.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Now that the on-sync period has ended, this issue is being un-held and will be processed normally as part of the integration queue. Thanks for your patience.

          Note to integrators: This requires a version bump, don't forget it.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Now that the on-sync period has ended, this issue is being un-held and will be processed normally as part of the integration queue. Thanks for your patience. Note to integrators: This requires a version bump, don't forget it.
          Hide
          poltawski Dan Poltawski added a comment -

          Integrated to master only - thanks a lot Daniel!

          Show
          poltawski Dan Poltawski added a comment - Integrated to master only - thanks a lot Daniel!
          Hide
          poltawski Dan Poltawski added a comment -

          ps. I think this issue would be a good candidate for a backport request https://docs.moodle.org/dev/Integration_Review#Process_for_requesting_a_non_bug-fix_backport

          Show
          poltawski Dan Poltawski added a comment - ps. I think this issue would be a good candidate for a backport request https://docs.moodle.org/dev/Integration_Review#Process_for_requesting_a_non_bug-fix_backport
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Works as described

          Show
          ankit_frenz Ankit Agarwal added a comment - Works as described
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your time and work!

          The roots of all goodness lie
          in the soil of appreciation for goodness.
          – Dalai Lama XIV

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your time and work! The roots of all goodness lie in the soil of appreciation for goodness. – Dalai Lama XIV

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Nov/15