Changing a user's email

RGJ · September 18, 2020, 10:17pm

I am a bit confused by the process when an admin changes a users email address.

Some things I just don’t understand, and there is one bug, (that is why I’m posting this in bug and not in support)

According to this pull request it should work like this.

When admin changes a user’s email from the preferences page of that user:

The user will not be sent an email to confirm that their email is changing. They will be sent a reset password email so they can set the password for their account at the new email address.

The user will still be sent an email to their old email to inform them that it was changed.

#1 I do not understand why a password reset email is being sent (“so they can set the password for their account”). They don’t need to change their password? And the user experience is confusing - the user does not expect an password reset email, and there is no accompanying text, it just says “Somebody asked to reset your password on [name of forum]”.

#2 That password reset email is sent to the old address instead of the new email address.

Even though the user email is updated in update_user_email on line 46, the @user object is not reloaded and still contains the old email address.

github.com

discourse/discourse/blob/main/lib/email_updater.rb#L44-L49


      
          return if errors.present? || existing_user.present?
          
          if @guardian.is_staff? && @guardian.user != @user
            StaffActionLogger.new(@guardian.user).log_add_email(@user)
          else
            UserHistory.create!(action: UserHistory.actions[:add_email], acting_user_id: @user.id)

#3 If the admin is the acting user, and the user that is acted upon is not staff, there is no confirmation email sent per the above specification. Nevertheless, after changing the email address, the admin gets the following success message: “We’ve sent an email to that address. Please follow the confirmation instructions”

#4 Why does the user not need to confirm their new email address? The pull request refers to this topic but it seems like there are many posts missing from it. But the topic does still mention " For a normal user, the only email address that has to be verified is the NEW email address" EDIT oh wait, see #6 / #7.

#5 This process where an admin changes the users email is typically used when the old email address is not accessible any more (I assume?) Why is there still a notification being sent to the old address?

#6 When this user tries to log in they get a popup

You can’t log in yet. We previously sent an activation email to you at old email address . Please follow the instructions in that email to activate your account.

there has not been such an email
the old email address is mentioned

Pressing the Resend button says

We sent another activation email to you at new email address. It might take a few minutes for it to arrive; be sure to check your spam folder.

#7 That activation email indeed arrives at the new email address and is titled “confirm your new account” (and not “confirm your new email address”)

Shouldn’t this just be :

One email is sent to the new email address, stating "your email address has been changed by [admin name]. Please click the following link to confirm [link].

Edit: #8 the email address can be changed by an admin from the users public profile (/u/username) but not from the admin page for that user (/admin/users/id/username). This is counterintuitive.

codinghorror · September 21, 2020, 12:17am

Can we repro this @tshenry? Did we regress here?

tshenry · September 25, 2020, 12:40am

I’ll start by laying out the current flow as I am seeing it happen (most, if not all, or this aligns with what @RGJ outlined):

Admin goes to a non-staff user’s preferences and changes their email address:

Screen Shot 2020-09-24 at 4.16.04 PM1758×590 35.2 KB

Once submitted, the admin sees the following message:

We’ve sent an email to that address. Please follow the confirmation instructions.
The above message does not appear accurate since TWO emails are sent to the old email address:

[Demo] Your email address has been changed

This is an automated message to let you know that your email address for
Demo has been changed. If this was done in error, please contact
a site administrator.

Your email address has been changed to:

new@email.com

[Demo] Password reset

Somebody asked to reset your password on Demo.

If it was not you, you can safely ignore this email.

Click the following link to choose a new password:
https://example.discourse.site/u/password-reset/74d53d7d2cf20dsbc360614844c653s2
I tested three separate scenarios from this point. Each bullet point represents its own scenario:
- The user has access to the old email and follows the reset password link. Upon updating their password, they are logged in. From there, they can log in with their username or the OLD email address. The new email does not seem to be in place at this point.
- The user tries to log in with their username or the new email address and receives this:
  
  Screen Shot 2020-09-24 at 4.38.01 PM1418×394 35.5 KB
  
  Option 1: Hitting resend will display the following message (note it mentions the new email address this time):
  
  Screen Shot 2020-09-24 at 4.59.42 PM1418×390 27.8 KB
  
  An email does indeed get sent to the the new email address:
  
  [Demo] Confirm your new account
  
  Welcome to Demo!
  
  Click the following link to confirm and activate your new account:
  https://example.discourse.site/u/activate-account/74d53d7d2cf20dsbc360614844c653s2
  
  If the above link is not clickable, try copying and pasting it into the address bar of your web browser.
  
  Following the link goes through some of the pages for activating a new account. In the end the user successfully logs in. From this point the user can log in with their username or the OLD email address. The new email does not seem to be in place at this point.
  
  Option 2: Hitting the change email address button shows the following. The Update Email Address button is disabled when the new email address is in the text field, implying the new email is already active (but that doesn’t appear to be true).
  
  Screen Shot 2020-09-24 at 5.17.31 PM938×492 27.2 KB
- The user initiates a password reset. The email goes to the new email address and the user can log in with the link. As with the other scenarios, the user can log in with their username or the OLD email address. The new email does not seem to be in place at this point.

So definitely repro’ing stuff here. It’s a tricky thing to write out clearly, but hopefully between the OP and this outline, it will make sense. There certainly appear to be some things that need fixing.

@martin I know you’ve tinkered with this part of core in the past. Would you be able to weigh in here when you have a moment?

codinghorror · September 25, 2020, 12:54am

Why would this have regressed?

edit: I can also confirm it has regressed. When editing a regular user’s email, the confirmation and such are sent to the OLD email. That’s not how this worked in the past…

martin · September 27, 2020, 11:11pm

That feeling of uncomfortable familiarity when you are reading the quoted pull request description and realise you are the culprit…

Thanks for the detailed instructions @tshenry and @RGJ I will put this to the top of my list to fix this week.

martin · September 28, 2020, 2:48am

Alright I have my head around this now, and I looked into the old topic deleted comments for history. I unearthed this from @sam which I now remember:

The admin resetting email case is so different, it is more admin resetting email and password. Cause if user had access to account they can do it all using self service

So we are saying that because admin is the one changing the email there should be a reset password email sent, because if the user had access the old email they could just…log in and do it themselves? But the reset password email also acts as a confirmation. Without finishing the reset password process (which is impossible right now because it is sent to the old email) the new email is not “confirmed” which is what makes this modal appear:

The issue where the reset password email is sent to the old address which is easily fixed and we will at least get into a state where the reset process can be followed:

RGJ:

Even though the user email is updated in update_user_email on line 46, the @user object is not reloaded and still contains the old email address.

github.com

discourse/discourse/blob/master/lib/email_updater.rb#L44-L49
if @guardian.is_staff? && !@user.staff?
  send_email_notification(@user.email, email)
  update_user_email(old_email, email)
  send_email(:forgot_password, @user.email_tokens.create!(email: @user.email))
  return
end

Also, because the reset password email is currently sent to the old email when it is confirmed it confirms the wrong address and sets the user’s email back to the old one.

I will change the messaging for the admin that is changing the email to make it clear to them that the user must click the link in the new email and change their password for the change to take full effect (and fix the wrong email issue too).

RGJ · September 28, 2020, 11:28am

Wait. I do not understand this.

There is a difference between a user being able to access the old email and a user requiring a password reset for their Discourse account. The first does absolutely not imply the second, these are totally different situations?

A large number of email changes by admin are also because the user does not know how to do it, or because the admin needs to temporarily lift the email_editable = false restriction.

I find it very confusing how a password reset doubles as an email confirmation. Personally I wouldn’t even respond to the password reset, I didn’t ask for it, and I would not realize that it was a necessary confirmation step (and I think it isn’t, a regular confirmation mail would suffice?)

Steve_Pavlina · September 28, 2020, 4:47pm

Might be related:

When one of my forum users tries to reset his password today (running latest version of Discourse as of this morning), he gets the email but then an error from following the email link:

He gets this error in multiple browsers and isn’t using an ad blocker.

When I go to the Preferences page for his account and click “Send Password Reset Email”, I get an error message there too:

Screen Shot 2020-09-28 at 9.44.10 AM

Before it shows “(error)” next to the button, it briefly flashes “(sending email)”. It looks like no email was sent though. I can verify that other forum emails are sending normally today.

This feature was previously working fine… seems to have broken within the past week sometime.

codinghorror · September 28, 2020, 4:54pm

Check your Discourse error logs in the web browser when logged in as an admin, there should be an error report there with more detail.

Steve_Pavlina · September 28, 2020, 5:05pm

Here’s the error entry:

398 Job exception: The specified copy source is larger than the maximum allowable size for a copy source: 5368709120

aws-sdk-core-3.99.1/lib/seahorse/client/plugins/raise_response_errors.rb:15:in `call'

aws-sdk-s3-1.66.0/lib/aws-sdk-s3/plugins/sse_cpk.rb:22:in `call'

aws-sdk-s3-1.66.0/lib/aws-sdk-s3/plugins/dualstack.rb:26:in `call'

aws-sdk-s3-1.66.0/lib/aws-sdk-s3/plugins/accelerate.rb:35:in `call'

aws-sdk-core-3.99.1/lib/aws-sdk-core/plugins/jsonvalue_converter.rb:20:in `call'

aws-sdk-core-3.99.1/lib/aws-sdk-core/plugins/idempotency_token.rb:17:in `call'

aws-sdk-core-3.99.1/lib/aws-sdk-core/plugins/param_converter.rb:24:in `call'

aws-sdk-core-3.99.1/lib/aws-sdk-core/plugins/response_paging.rb:10:in `call'

aws-sdk-core-3.99.1/lib/seahorse/client/plugins/response_target.rb:23:in `call'

aws-sdk-core-3.99.1/lib/seahorse/client/request.rb:70:in `send_request'

aws-sdk-s3-1.66.0/lib/aws-sdk-s3/client.rb:1108:in `copy_object'

/var/www/discourse/lib/backup_restore/s3_backup_store.rb:61:in `block in vacate_legacy_prefix'

/var/www/discourse/lib/backup_restore/s3_backup_store.rb:60:in `each'

/var/www/discourse/lib/backup_restore/s3_backup_store.rb:60:in `vacate_legacy_prefix'

/var/www/discourse/app/jobs/onceoff/vacate_legacy_prefix_backups.rb:7:in `execute_onceoff'

/var/www/discourse/app/jobs/onceoff/onceoff.rb:25:in `execute'

/var/www/discourse/app/jobs/base.rb:232:in `block (2 levels) in perform'

rails_multisite-2.5.0/lib/rails_multisite/connection_management.rb:76:in `with_connection'

/var/www/discourse/app/jobs/base.rb:221:in `block in perform'

/var/www/discourse/app/jobs/base.rb:217:in `each'

/var/www/discourse/app/jobs/base.rb:217:in `perform'

sidekiq-6.1.2/lib/sidekiq/processor.rb:196:in `execute_job'

sidekiq-6.1.2/lib/sidekiq/processor.rb:164:in `block (2 levels) in process'

sidekiq-6.1.2/lib/sidekiq/middleware/chain.rb:138:in `block in invoke'

/var/www/discourse/lib/sidekiq/pausable.rb:138:in `call'

sidekiq-6.1.2/lib/sidekiq/middleware/chain.rb:140:in `block in invoke'

sidekiq-6.1.2/lib/sidekiq/middleware/chain.rb:143:in `invoke'

sidekiq-6.1.2/lib/sidekiq/processor.rb:163:in `block in process'

sidekiq-6.1.2/lib/sidekiq/processor.rb:136:in `block (6 levels) in dispatch'

sidekiq-6.1.2/lib/sidekiq/job_retry.rb:111:in `local'

sidekiq-6.1.2/lib/sidekiq/processor.rb:135:in `block (5 levels) in dispatch'

sidekiq-6.1.2/lib/sidekiq.rb:38:in `block in <module:Sidekiq>'

sidekiq-6.1.2/lib/sidekiq/processor.rb:131:in `block (4 levels) in dispatch'

sidekiq-6.1.2/lib/sidekiq/processor.rb:257:in `stats'

sidekiq-6.1.2/lib/sidekiq/processor.rb:126:in `block (3 levels) in dispatch'

sidekiq-6.1.2/lib/sidekiq/job_logger.rb:13:in `call'

sidekiq-6.1.2/lib/sidekiq/processor.rb:125:in `block (2 levels) in dispatch'

sidekiq-6.1.2/lib/sidekiq/job_retry.rb:78:in `global'

sidekiq-6.1.2/lib/sidekiq/processor.rb:124:in `block in dispatch'

sidekiq-6.1.2/lib/sidekiq/logger.rb:10:in `with'

sidekiq-6.1.2/lib/sidekiq/job_logger.rb:33:in `prepare'

sidekiq-6.1.2/lib/sidekiq/processor.rb:123:in `dispatch'

sidekiq-6.1.2/lib/sidekiq/processor.rb:162:in `process'

sidekiq-6.1.2/lib/sidekiq/processor.rb:78:in `process_one'

sidekiq-6.1.2/lib/sidekiq/processor.rb:68:in `run'

sidekiq-6.1.2/lib/sidekiq/util.rb:15:in `watchdog'

sidekiq-6.1.2/lib/sidekiq/util.rb:24:in `block in safe_thread'

RGJ · September 28, 2020, 5:28pm

No, that is absolutely unrelated.
Try to clear the logs and force this error to happen, then check the logs again.

Steve_Pavlina · September 28, 2020, 6:22pm

Then the logs remain blank after the error occurs.

martin · September 28, 2020, 11:44pm

I can see where you are coming from, to me the reset password as confirmation thing tripped me up yesterday. I feel this could be a secondary option for the admin when changing a user’s email, checking a box saying “Reset the user’s password as well”. I am going to merge the PR I have for a fix as is because the process is totally broken right now.

I would like @sam to weigh in on a new process/flow because Sam originally talked about the reasoning behind the reset password flow:

Admin changes user’s email. They have the option to reset their password at the same time.
The user receives an email at their new address asking for confirmation to change their email.
- If they say Yes, then change the email. We send an email to their old address saying the email has changed.
- If they say No, do nothing.
If the admin had specified they wanted a reset in 1., then as soon as the user confirms their email change they get a reset password email at the new address.

I think this would be a lot clearer, and the reset password would have nothing to do with confirming the email change.

codinghorror · September 28, 2020, 11:48pm

Yes I cannot see why these two things would be related at all?

IAmGav · September 29, 2020, 12:56am

i see a nice 1 new merged commit that people would be happy about

https://github.com/discourse/discourse/pull/10755

martin · September 29, 2020, 1:03am

Thanks, this will just merge a fix for the “completely broken” state of affairs. Another PR to follow!

It was just done this way from discussions in the previous topic with Sam. I will go ahead with the new process to lift the veil of confusion and get rid of the link between the unrelated things.

martin · October 7, 2020, 3:03am

I just merged this PR which does the below:

https://github.com/discourse/discourse/pull/10830

Changes the admin change email for user flow so the user is sent an email to confirm the change
We now record who the email change request was requested by
If the requested by user is admin and not the user we note this in the email sent to the user
We also make the confirm change email route open to anonymous users, so it can be clicked by the user even if they do not have access to their account. If there is a logged in user we make sure the confirmation matches the current user.

Hopefully this makes the process make a lot more sense!

Topic		Replies	Views
On email address change second confirm fails due to email customization Support	20	1784	December 26, 2019
Different password reset for wrong username/email Feature	65	27651	August 4, 2023
Change email for SSO user? SSO	35	9314	September 9, 2024
Additional email address per user account support Feature	48	10242	March 18, 2021
Two emails for one user Feature	49	14806	June 20, 2017

Changing a user's email

[Demo] Your email address has been changed

[Demo] Password reset

[Demo] Confirm your new account

Related Topics