Moodle
  1. Moodle
  2. MDL-13811

Add a confirmation step when a user changes their own email address in their profile.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.8.6, 1.9.2
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      Currently there is no process and any new email address is accepted.

      I think we could do it like this instead:

      • User edits profile and submits form
      • If email is different then:
      • Do NOT update the real profile email yet.
      • Save that new email in a user preference together with a random key.
      • Send an email to the NEW address with instructions and a link containing the secret key.
        eg http://moodle.org/user/emailchange.php?id=y73nj3bh3b3m7678bbhbhbhbh3bh34
      • User finds the email, clicks the link, and a script:
      • verifies the secret key,
      • changes the profile email to the new one and
      • deletes the user preference

        Gliffy Diagrams

        1. MDL-13811_002.patch
          8 kB
          Nicolas Connault
        2. MDL-13811_003.patch
          9 kB
          Nicolas Connault
        3. MDL-13811_004.patch
          8 kB
          Nicolas Connault
        4. MDL-13811_005.patch
          9 kB
          Nicolas Connault
        5. MDL-13811.patch
          8 kB
          Nicolas Connault
        1. MDL-13811.png
          102 kB
        2. MDL-13811b.png
          88 kB

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment - - edited

            the only troubly is how to inform user that:

            • email change is pending (some info in edit form)
            • email will not be changed immediately (info in form)

            Also there should be an option to cancel the change.
            Users which are allowed to edit profiles (such as admins) should not be asked for confirmation.
            Maybe a resend option.

            Looking forward to patch, this will be a great improvement

            Show
            Petr Skoda added a comment - - edited the only troubly is how to inform user that: email change is pending (some info in edit form) email will not be changed immediately (info in form) Also there should be an option to cancel the change. Users which are allowed to edit profiles (such as admins) should not be asked for confirmation. Maybe a resend option. Looking forward to patch, this will be a great improvement
            Hide
            Petr Skoda added a comment -

            This could be implemented by adding two different form fields and removing the one that is not used in definition_after_data() - one for old style change, the other for change with confirmation.

            Show
            Petr Skoda added a comment - This could be implemented by adding two different form fields and removing the one that is not used in definition_after_data() - one for old style change, the other for change with confirmation.
            Hide
            Martin Dougiamas added a comment -

            Agreed that people with the ability to edit all profiles don't need this.

            For others, I don't think we need to maintain or show anything about the pending change in the main form interface, if we do it like this:

            When you hit submit:

            iff the email was changed then you see the message telling you that the email will not be changed to the new value until you read the email at that address and click on the link found there, with a continue link.

            if the email was not changed then just go straight to the profile page as we do now.

            Since nothing happens until they press the link in the email there is no need to do anything special for resending etc.

            Show
            Martin Dougiamas added a comment - Agreed that people with the ability to edit all profiles don't need this. For others, I don't think we need to maintain or show anything about the pending change in the main form interface, if we do it like this: When you hit submit: iff the email was changed then you see the message telling you that the email will not be changed to the new value until you read the email at that address and click on the link found there, with a continue link. if the email was not changed then just go straight to the profile page as we do now. Since nothing happens until they press the link in the email there is no need to do anything special for resending etc.
            Hide
            Nicolas Connault added a comment -

            Attaching patch

            Show
            Nicolas Connault added a comment - Attaching patch
            Hide
            Nicolas Connault added a comment -

            Version 2 of the patch attached.

            Show
            Nicolas Connault added a comment - Version 2 of the patch attached.
            Hide
            Nicolas Connault added a comment -

            Ignore the last file, try number 3 instead!

            Show
            Nicolas Connault added a comment - Ignore the last file, try number 3 instead!
            Hide
            Petr Skoda added a comment - - edited

            1/ we must add email duplicate test into user/emailupdate.php just before saving the new one - the timing could work against us there
            2/ admin option to turn it off
            3/ print_continue("$CFG->wwwroot/user/view.php?id=$user->id", true); – true does not print the continue button
            4/ name of site in email header missing
            5/ missing name of user in that email too
            6/ I think the key may be shorter - 20 chars?
            7/ oh! print_simple_box() is deprecated - use the alternatives instead

            Show
            Petr Skoda added a comment - - edited 1/ we must add email duplicate test into user/emailupdate.php just before saving the new one - the timing could work against us there 2/ admin option to turn it off 3/ print_continue("$CFG->wwwroot/user/view.php?id=$user->id", true); – true does not print the continue button 4/ name of site in email header missing 5/ missing name of user in that email too 6/ I think the key may be shorter - 20 chars? 7/ oh! print_simple_box() is deprecated - use the alternatives instead
            Hide
            Helen Foster added a comment -

            Hi, how is this going?

            Show
            Helen Foster added a comment - Hi, how is this going?
            Hide
            Nicolas Connault added a comment -

            Attached a patch with all the fixes Petr suggested

            Show
            Nicolas Connault added a comment - Attached a patch with all the fixes Petr suggested
            Hide
            Petr Skoda added a comment -

            thanks, I will review it on Wednesday

            Show
            Petr Skoda added a comment - thanks, I will review it on Wednesday
            Hide
            Petr Skoda added a comment -

            for some reason the patch does not apply for me to current 19x

            1/ missing $a->fullname = fullname($user, true); in auth_emailupdatemessage
            2/ there should be some info when editing profile that email change is pending

            the rest seems ok

            Show
            Petr Skoda added a comment - for some reason the patch does not apply for me to current 19x 1/ missing $a->fullname = fullname($user, true); in auth_emailupdatemessage 2/ there should be some info when editing profile that email change is pending the rest seems ok
            Hide
            Nicolas Connault added a comment -

            Fixed in 1.9: Also added a "Cancel email change" link and an admin option in Security -> Site policies section

            Show
            Nicolas Connault added a comment - Fixed in 1.9: Also added a "Cancel email change" link and an admin option in Security -> Site policies section
            Hide
            Petr Skoda added a comment -

            oops, this does not work because useredit_update_user_preference() was not intended for this

            working on a fix...

            Show
            Petr Skoda added a comment - oops, this does not work because useredit_update_user_preference() was not intended for this working on a fix...
            Hide
            Petr Skoda added a comment - - edited

            had to tweak it a bit, tested, works for me, yay!
            thanks everybody

            Show
            Petr Skoda added a comment - - edited had to tweak it a bit, tested, works for me, yay! thanks everybody
            Hide
            Jérôme Mouneyrac added a comment -

            there are some logs displayed on the top of the confirmation page (1.9):

            SMTP -> get_lines(): $data was ""
            SMTP -> get_lines(): $str is "220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP
            "
            SMTP -> get_lines(): $data is "220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP
            "
            SMTP -> FROM SERVER:
            220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP
            ...........

            Show
            Jérôme Mouneyrac added a comment - there are some logs displayed on the top of the confirmation page (1.9): SMTP -> get_lines(): $data was "" SMTP -> get_lines(): $str is "220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP " SMTP -> get_lines(): $data is "220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP " SMTP -> FROM SERVER: 220 outbound.icp-qv1-irony-out4.iinet.net.au ESMTP ...........
            Hide
            Anthony Borrow added a comment -

            Where are you seeing these? I just tested with the latest from CVS and all seems to be well. I tested with both sendmail and SMTP server. I'm curious what might be causing this. Peace - Anthony

            Show
            Anthony Borrow added a comment - Where are you seeing these? I just tested with the latest from CVS and all seems to be well. I tested with both sendmail and SMTP server. I'm curious what might be causing this. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            Is this what Jerome means by confirmation page?

            Show
            Anthony Borrow added a comment - Is this what Jerome means by confirmation page?
            Hide
            Anthony Borrow added a comment -

            here is the confirmation after clicking on the link (w/smtp server)

            Show
            Anthony Borrow added a comment - here is the confirmation after clicking on the link (w/smtp server)
            Hide
            Jérôme Mouneyrac added a comment -

            Anthony, it's apparently in lib/phpmailer/class.smtp.php, there is echo "SMTP -> get_lines(): \$data was \"$data\"" ...
            I keep going to test this issue and I'll write a bug for the log stuff if needed.

            Show
            Jérôme Mouneyrac added a comment - Anthony, it's apparently in lib/phpmailer/class.smtp.php, there is echo "SMTP -> get_lines(): \$data was \"$data\"" ... I keep going to test this issue and I'll write a bug for the log stuff if needed.
            Hide
            Jérôme Mouneyrac added a comment -

            tested on 1.9, 1.8 and Head

            My tests:
            When option is turned off:

            • student can change his email address

            When option is turned on

            • admin can change email address
            • student needs to confirm by email (try accept and cancel)
            • if email address has been taken by someone else before validation, an error message appears when student click on the email confirmation link

            Remarks:

            • In 1.8, the option is located in the "Module Security" folder. In 1.9 and HEAD it's in Site Policies. It would be better to be consistent between version.
            • If I don't set any SMPT server and turn the option on, a student can change the email address, then he gets an error (not very friendly on a blank page), and the profil page says that Moodle is waiting for a confirmation. However no email was sent. The student has to cancel the change. (behaviour tested on 1.9, 1.8 and 2.0)
            • my previous comment (08/Jul/08 11:16 AM) concerns only 1.9. These SMTP logs appear on the page displayed just after clicking on Update on the profil page. I'll write a new issue for that.

            Otherwise all works fine
            If the remarks are not going to be fixed, you can close the issue. Thanks.

            Show
            Jérôme Mouneyrac added a comment - tested on 1.9, 1.8 and Head My tests: When option is turned off: student can change his email address When option is turned on admin can change email address student needs to confirm by email (try accept and cancel) if email address has been taken by someone else before validation, an error message appears when student click on the email confirmation link Remarks: In 1.8, the option is located in the "Module Security" folder. In 1.9 and HEAD it's in Site Policies. It would be better to be consistent between version. If I don't set any SMPT server and turn the option on, a student can change the email address, then he gets an error (not very friendly on a blank page), and the profil page says that Moodle is waiting for a confirmation. However no email was sent. The student has to cancel the change. (behaviour tested on 1.9, 1.8 and 2.0) my previous comment (08/Jul/08 11:16 AM) concerns only 1.9. These SMTP logs appear on the page displayed just after clicking on Update on the profil page. I'll write a new issue for that. Otherwise all works fine If the remarks are not going to be fixed, you can close the issue. Thanks.
            Hide
            Jérôme Mouneyrac added a comment -

            about SMTP logs, it's not a bug, I had SMTP debug turned on in administration. Oops...

            Show
            Jérôme Mouneyrac added a comment - about SMTP logs, it's not a bug, I had SMTP debug turned on in administration. Oops...
            Hide
            Petr Skoda added a comment -

            I have moved the setting to correct category.

            Show
            Petr Skoda added a comment - I have moved the setting to correct category.
            Hide
            Petr Skoda added a comment -

            there is a die("could not send email!"); in user/edit.php, that should help diagnose smtp sending problems for now

            thanks for the review, reclosing

            Show
            Petr Skoda added a comment - there is a die("could not send email!"); in user/edit.php, that should help diagnose smtp sending problems for now thanks for the review, reclosing
            Hide
            Jérôme Mouneyrac added a comment -

            Ok 1.8, option setting is now in Sites Policies. All good.

            Show
            Jérôme Mouneyrac added a comment - Ok 1.8, option setting is now in Sites Policies. All good.
            Hide
            Martin Dougiamas added a comment -

            I made a little fix to make this work even when that user's email is currently disabled.

            Show
            Martin Dougiamas added a comment - I made a little fix to make this work even when that user's email is currently disabled.
            Hide
            Petr Skoda added a comment -

            enabled by default now

            Show
            Petr Skoda added a comment - enabled by default now

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: