Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      3858

      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 ?

        Activity

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

        sorry "how" not "who"...

        Show
        Etienne Rozé added a comment - sorry "how" not "who"...
        Hide
        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
        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).
        Martin Dougiamas made changes -
        Field Original Value New Value
        Workflow jira [ 22548 ] MDL Workflow [ 42563 ]
        Martin Dougiamas made changes -
        Workflow MDL Workflow [ 42563 ] MDL Full Workflow [ 70978 ]
        Martin Dougiamas made changes -
        Assignee Martin Dougiamas [ dougiamas ] moodle.com [ moodle.com ]
        Hide
        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
        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.
        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 ]
        Ruslan Kabalin made changes -
        Assignee moodle.com [ moodle.com ] Ruslan Kabalin [ kabalin ]
        Hide
        Ruslan Kabalin added a comment -

        Dan, can you please review it?

        Show
        Ruslan Kabalin added a comment - Dan, can you please review it?
        Ruslan Kabalin made changes -
        Status Open [ 1 ] Waiting for peer review [ 10012 ]
        Hide
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        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
        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
        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
        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
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda added a comment -

        oh, and we need renderers for each message type

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

        sorry for the off-topic noise

        Show
        Petr Škoda added a comment - sorry for the off-topic noise
        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
        Dan Poltawski added a comment -

        Please see Petr's comments about language strings and typos

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

        New branch. Petr's suggestions have been accomodated.

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

        +1

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

        Looks OK for master - please remember testing instructions

        Show
        Dan Poltawski added a comment - Looks OK for master - please remember testing instructions
        Dan Poltawski made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        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).
        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
        Ruslan Kabalin added a comment -

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

        Show
        Ruslan Kabalin added a comment - Testing instructions have been added, the fix has been reviewed. Ready for integration.
        Ruslan Kabalin made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Sam Hemelryk made changes -
        Currently in integration Yes [ 10041 ]
        Sam Hemelryk made changes -
        Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
        Integrator samhemelryk
        Hide
        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
        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
        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 ]
        Michael de Raadt made changes -
        Tester rajeshtaneja
        Rajesh Taneja made changes -
        Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
        Hide
        Rajesh Taneja added a comment -

        Works Great,

        Thanks for fixing this Ruslan

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

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

        And this has landed upstream, finally! Yay!

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



        Closing, ciao

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

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

        Show
        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: