Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8, 1.8.1, 1.8.2, 1.8.3, 1.9, 2.0, 2.1, 2.2, 2.3
    • Fix Version/s: 2.3
    • Component/s: General, Messages
    • Labels:
      None
    • Testing Instructions:
      Hide

      The best way to test it is probably set up mailing through Gmail:

      1. Set the following in Site Admin -> Plugin -> Message outputs -> email:
        SMTP hosts: smtp.gmail.com:465
        SMTP secure: ssl
        SMTP username: Your email address @gmail.com or your own domain if using Google Apps
        SMTP password: password for the above email account (if using two-step authorization, generate an application-specific password)
      2. Try sending message to the user that has email you are able to access (make sure that user messaging preferences allow receiving personal messaged by email and site messaging preferences for private messages are not set to disallowed).
      3. Ensure that the message has been delivered
      Show
      The best way to test it is probably set up mailing through Gmail: Set the following in Site Admin -> Plugin -> Message outputs -> email: SMTP hosts: smtp.gmail.com:465 SMTP secure: ssl SMTP username: Your email address @gmail.com or your own domain if using Google Apps SMTP password: password for the above email account (if using two-step authorization, generate an application-specific password) Try sending message to the user that has email you are able to access (make sure that user messaging preferences allow receiving personal messaged by email and site messaging preferences for private messages are not set to disallowed). Ensure that the message has been delivered
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-11378-master-2

      Description

      This thread describe who use ssl with smtp to send mails : http://moodle.org/mod/forum/discuss.php?d=51833
      But it is a little hack

      Why not a parameter to give this possibility more easily ?

        Gliffy Diagrams

          Activity

          testeur Etienne Rozé created issue -
          Hide
          testeur Etienne Rozé added a comment -

          sorry "how" not "who"...

          Show
          testeur Etienne Rozé added a comment - sorry "how" not "who"...
          Hide
          bletvaska mirek added a comment -

          i was using 'the hack' for long time and today after upgrading to the 1.9.7 i decided to take a closer look. so - this solution solves the problem. it accepts the following urls for smtp server: SSLsmtp.host.com[:PORT]

          first change goes to lib/phpmailer/class.phpmailer.php the first if statement which follows the while statemnet on the line 549 was replaced with this:

          preg_match ( '/(?<host>((ssl|http):\/\/)?(\w*\.)\w)(?<port>\d+))?$/', $hosts[$index], $groups );
          $host = $groups["host"];
          $port = empty($groups["port"]) ? $this->Port : $groups["port"];

          so this is responsible for the parsing the smtp url specified in the configuration form. then - just for the allowing smtp user to authenticate, add line 4264 in to the file lib/moodlelib.php

          $mail->Sender = $CFG->smtpuser;

          and - thats it. hopefuly - this helps to finaly enable this option (without any other kind of hacking).

          Show
          bletvaska mirek added a comment - i was using 'the hack' for long time and today after upgrading to the 1.9.7 i decided to take a closer look. so - this solution solves the problem. it accepts the following urls for smtp server: SSL smtp.host.com [:PORT] first change goes to lib/phpmailer/class.phpmailer.php the first if statement which follows the while statemnet on the line 549 was replaced with this: preg_match ( '/(?<host>((ssl|http):\/\/)?(\w*\.) \w )( ?<port>\d+))?$/', $hosts [$index] , $groups ); $host = $groups ["host"] ; $port = empty($groups ["port"] ) ? $this->Port : $groups ["port"] ; so this is responsible for the parsing the smtp url specified in the configuration form. then - just for the allowing smtp user to authenticate, add line 4264 in to the file lib/moodlelib.php $mail->Sender = $CFG->smtpuser; and - thats it. hopefuly - this helps to finaly enable this option (without any other kind of hacking).
          dougiamas Martin Dougiamas made changes -
          Field Original Value New Value
          Workflow jira [ 22548 ] MDL Workflow [ 42563 ]
          dougiamas Martin Dougiamas made changes -
          Workflow MDL Workflow [ 42563 ] MDL Full Workflow [ 70978 ]
          dougiamas Martin Dougiamas made changes -
          Assignee Martin Dougiamas [ dougiamas ] moodle.com [ moodle.com ]
          Hide
          kabalin Ruslan Kabalin added a comment -

          Given that we have SMTPSecure class variable in phpmailer, this improvement is pretty trivial. The attached patch works with 2.1 onwards (MDL-27171) and can be cherry picked to all relevant branches.

          Show
          kabalin Ruslan Kabalin added a comment - Given that we have SMTPSecure class variable in phpmailer, this improvement is pretty trivial. The attached patch works with 2.1 onwards ( MDL-27171 ) and can be cherry picked to all relevant branches.
          kabalin Ruslan Kabalin made changes -
          Pull Master Diff URL https://git.luns.net.uk/?p=moodle.git;a=commitdiff;h=b22d2a88209c2e07a9e7204c1c1a41b488880822
          Pull Master Branch MDL-11378-master-1
          Fix Version/s 2.3 [ 10657 ]
          Fix Version/s 2.2 [ 10656 ]
          Fix Version/s 2.1 [ 10370 ]
          Peer reviewer poltawski
          Pull from Repository git://git.luns.net.uk/moodle
          Affects Version/s 2.2 [ 10656 ]
          Affects Version/s 2.1 [ 10370 ]
          Affects Version/s 2.3 [ 10657 ]
          Component/s Messages [ 10084 ]
          kabalin Ruslan Kabalin made changes -
          Assignee moodle.com [ moodle.com ] Ruslan Kabalin [ kabalin ]
          Hide
          kabalin Ruslan Kabalin added a comment -

          Dan, can you please review it?

          Show
          kabalin Ruslan Kabalin added a comment - Dan, can you please review it?
          kabalin Ruslan Kabalin made changes -
          Status Open [ 1 ] Waiting for peer review [ 10012 ]
          Hide
          poltawski Dan Poltawski added a comment -

          Hmm, my immediate thought was that since this is a niche feature maybe we could just use a url protocol handler (like ssl://foo.bar.com/) rather than adding yet another setting. But I suppose thats fairly ugly.

          BTW - does phpmailer not just try TLS when it can? This would seem more sensible

          Show
          poltawski Dan Poltawski added a comment - Hmm, my immediate thought was that since this is a niche feature maybe we could just use a url protocol handler (like ssl://foo.bar.com/) rather than adding yet another setting. But I suppose thats fairly ugly. BTW - does phpmailer not just try TLS when it can? This would seem more sensible
          Hide
          skodak Petr Skoda added a comment -
          • "requres" typo
          • is it really necessary to translate things like "SSL", "TLS", "None"?

          (arrgh, I do not understand why the SMTP configuration was moved to messaging plugin and keeping the sending functionality in core, this is messed up - core MUST NOT use hardcoded plugin names and their settings, everything in /message/* keeps driving me crazy, sorry)

          Show
          skodak Petr Skoda added a comment - "requres" typo is it really necessary to translate things like "SSL", "TLS", "None"? (arrgh, I do not understand why the SMTP configuration was moved to messaging plugin and keeping the sending functionality in core, this is messed up - core MUST NOT use hardcoded plugin names and their settings, everything in /message/* keeps driving me crazy, sorry)
          Hide
          kabalin Ruslan Kabalin added a comment -

          Thanks Dan, ssl://foo.bar.com/ might work actually, tls:// will not work for sure as it needs to call StartTLS(), which will be called if SMTPSecure setting is set.

          BTW - does phpmailer not just try TLS when it can? This would seem more sensible

          No, it does not, it checks for SMTPSecure value and goes from there. See SmtpConnect() in class.phpmailer.php.

          Show
          kabalin Ruslan Kabalin added a comment - Thanks Dan, ssl://foo.bar.com/ might work actually, tls:// will not work for sure as it needs to call StartTLS(), which will be called if SMTPSecure setting is set. BTW - does phpmailer not just try TLS when it can? This would seem more sensible No, it does not, it checks for SMTPSecure value and goes from there. See SmtpConnect() in class.phpmailer.php.
          Hide
          poltawski Dan Poltawski added a comment -

          (I am just commenting, not looking at the code in detail here... sat on sofa)

          My point was that we ourselves could do this interpretation of the server rather than passing on directly to phpmailer in order to reduce settings bloat for a feature most admins won't consider.

          There is another reason to use protocol handlers actually - the smtpservers setting can use multiple servers - we would be enforcing one setting for all servers. This may not be a bad thing..

          Show
          poltawski Dan Poltawski added a comment - (I am just commenting, not looking at the code in detail here... sat on sofa) My point was that we ourselves could do this interpretation of the server rather than passing on directly to phpmailer in order to reduce settings bloat for a feature most admins won't consider. There is another reason to use protocol handlers actually - the smtpservers setting can use multiple servers - we would be enforcing one setting for all servers. This may not be a bad thing..
          Hide
          kabalin Ruslan Kabalin added a comment -

          (arrgh, I do not understand why the SMTP configuration was moved to messaging plugin and keeping the sending functionality in core, this is messed up - core MUST NOT use hardcoded plugin names and their settings, everything in /message/* keeps driving me crazy, sorry)

          Indeed, some bits like email_to_user, get_mailer, moodle_phpmailer are in core, but the dispatching bit is in the plugin (message_output_email::send_message). It might be a good idea to move everything email related to output plugin. The reason of maintaining messaging plugins infrastructure is to allow flexibility of adding new providers and processors, and having all messaging-related settings in one place.

          Show
          kabalin Ruslan Kabalin added a comment - (arrgh, I do not understand why the SMTP configuration was moved to messaging plugin and keeping the sending functionality in core, this is messed up - core MUST NOT use hardcoded plugin names and their settings, everything in /message/* keeps driving me crazy, sorry) Indeed, some bits like email_to_user, get_mailer, moodle_phpmailer are in core, but the dispatching bit is in the plugin (message_output_email::send_message). It might be a good idea to move everything email related to output plugin. The reason of maintaining messaging plugins infrastructure is to allow flexibility of adding new providers and processors, and having all messaging-related settings in one place.
          Hide
          skodak Petr Skoda added a comment -

          yeah, there are some corner cases that need to be ironed out (sending emails to non-users that have only email, signup, performance, ...) - that is not going to happen without a devoted maintainer unfortunately

          Show
          skodak Petr Skoda added a comment - yeah, there are some corner cases that need to be ironed out (sending emails to non-users that have only email, signup, performance, ...) - that is not going to happen without a devoted maintainer unfortunately
          Hide
          skodak Petr Skoda added a comment -

          oh, and we need renderers for each message type

          Show
          skodak Petr Skoda added a comment - oh, and we need renderers for each message type
          Hide
          skodak Petr Skoda added a comment -

          sorry for the off-topic noise

          Show
          skodak Petr Skoda added a comment - sorry for the off-topic noise
          poltawski Dan Poltawski made changes -
          Original Estimate 0 minutes [ 0 ]
          Remaining Estimate 0 minutes [ 0 ]
          Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
          Hide
          poltawski Dan Poltawski added a comment -

          Please see Petr's comments about language strings and typos

          Show
          poltawski Dan Poltawski added a comment - Please see Petr's comments about language strings and typos
          poltawski Dan Poltawski made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Hide
          kabalin Ruslan Kabalin added a comment -

          New branch. Petr's suggestions have been accomodated.

          Show
          kabalin Ruslan Kabalin added a comment - New branch. Petr's suggestions have been accomodated.
          kabalin Ruslan Kabalin made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Hide
          skodak Petr Skoda added a comment -

          +1

          Show
          skodak Petr Skoda added a comment - +1
          poltawski Dan Poltawski made changes -
          Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
          Hide
          poltawski Dan Poltawski added a comment -

          Looks OK for master - please remember testing instructions

          Show
          poltawski Dan Poltawski added a comment - Looks OK for master - please remember testing instructions
          poltawski Dan Poltawski made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          kabalin Ruslan Kabalin made changes -
          Testing Instructions The best way to test it is probably set up mailing through Gmail:
          # Set the following in Site Admin -> Plugin -> Message outputs -> email:
          *SMTP hosts:* smtp.gmail.com:465
          *SMTP secure:* ssl
          *SMTP username:* Your email address @gmail.com or your own domain if using Google Apps
          *SMTP password:* password for the above email account (if using two-step authorization, generate an application-specific password)
          # Try sending message to the user that has email you are able to control (make sure that user messaging preferences allow receiving personal messaged by email and site messaging preferences for private messages are not set to disallowed).
          kabalin Ruslan Kabalin made changes -
          Testing Instructions The best way to test it is probably set up mailing through Gmail:
          # Set the following in Site Admin -> Plugin -> Message outputs -> email:
          *SMTP hosts:* smtp.gmail.com:465
          *SMTP secure:* ssl
          *SMTP username:* Your email address @gmail.com or your own domain if using Google Apps
          *SMTP password:* password for the above email account (if using two-step authorization, generate an application-specific password)
          # Try sending message to the user that has email you are able to control (make sure that user messaging preferences allow receiving personal messaged by email and site messaging preferences for private messages are not set to disallowed).
          The best way to test it is probably set up mailing through Gmail:
          # Set the following in Site Admin -> Plugin -> Message outputs -> email:
          *SMTP hosts:* smtp.gmail.com:465
          *SMTP secure:* ssl
          *SMTP username:* Your email address @gmail.com or your own domain if using Google Apps
          *SMTP password:* password for the above email account (if using two-step authorization, generate an application-specific password)
          # Try sending message to the user that has email you are able to access (make sure that user messaging preferences allow receiving personal messaged by email and site messaging preferences for private messages are not set to disallowed).
          # Ensure that the message has been delivered
          Hide
          kabalin Ruslan Kabalin added a comment -

          Testing instructions have been added, the fix has been reviewed. Ready for integration.

          Show
          kabalin Ruslan Kabalin added a comment - Testing instructions have been added, the fix has been reviewed. Ready for integration.
          kabalin Ruslan Kabalin made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          samhemelryk Sam Hemelryk made changes -
          Currently in integration Yes [ 10041 ]
          samhemelryk Sam Hemelryk made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator samhemelryk
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Ruslan, this has been integrated now.
          Just noting as it is a new feature this has been integrated on master only.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Ruslan, this has been integrated now. Just noting as it is a new feature this has been integrated on master only. Cheers Sam
          samhemelryk Sam Hemelryk made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Fix Version/s 2.1 [ 10370 ]
          Fix Version/s 2.2 [ 10656 ]
          salvetore Michael de Raadt made changes -
          Tester rajeshtaneja
          rajeshtaneja Rajesh Taneja made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Works Great,

          Thanks for fixing this Ruslan

          Show
          rajeshtaneja Rajesh Taneja added a comment - Works Great, Thanks for fixing this Ruslan
          rajeshtaneja Rajesh Taneja made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          kabalin Ruslan Kabalin added a comment -

          Show
          kabalin Ruslan Kabalin added a comment -
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          And this has landed upstream, finally! Yay!

          תודה רבה && شكرا جزيلا



          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao
          stronk7 Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 29/Mar/12
          Hide
          tsala Helen Foster added a comment -

          Ruslan, thanks for this new setting which is now documented in the 2.3 wiki:

          Show
          tsala Helen Foster added a comment - Ruslan, thanks for this new setting which is now documented in the 2.3 wiki: http://docs.moodle.org/23/en/Messaging_settings#SMTP_security

            People

            • Votes:
              3 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12