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
    • Rank:
      36983

      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
      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 Škoda 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 Škoda 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 Škoda 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 Škoda 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 Škoda 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 Škoda 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 Škoda added a comment -

          thanks, I will review it on Wednesday

          Show
          Petr Škoda added a comment - thanks, I will review it on Wednesday
          Hide
          Petr Škoda 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 Škoda 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 Škoda added a comment -

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

          working on a fix...

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

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

          Show
          Petr Škoda 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 Škoda added a comment -

          I have moved the setting to correct category.

          Show
          Petr Škoda added a comment - I have moved the setting to correct category.
          Hide
          Petr Škoda 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 Škoda 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 Škoda added a comment -

          enabled by default now

          Show
          Petr Škoda added a comment - enabled by default now

            People

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

              Dates

              • Created:
                Updated:
                Resolved: