Skip to content

Commit

Permalink
FIX: Admin change email for user process improvements and fixes (#10755)
Browse files Browse the repository at this point in the history
See https://meta.discourse.org/t/changing-a-users-email/164512 for context.

When admin changes an email for a user, we were incorrectly sending the password reset email to the user's old address. Also the new email does not come into effect until the reset password process is done, so this PR adds some notes to the admin to make this clearer.
  • Loading branch information
martin-brennan committed Sep 28, 2020
1 parent fe86a7c commit 3cd601d
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 10 deletions.
@@ -1,6 +1,6 @@
import I18n from "I18n";
import discourseComputed from "discourse-common/utils/decorators";
import { empty, or } from "@ember/object/computed";
import { empty, or, alias } from "@ember/object/computed";
import Controller from "@ember/controller";
import { propertyEqual } from "discourse/lib/computed";
import EmberObject from "@ember/object";
Expand All @@ -15,6 +15,7 @@ export default Controller.extend({
success: false,
oldEmail: null,
newEmail: null,
successMessage: null,

newEmailEmpty: empty("newEmail"),

Expand All @@ -28,6 +29,8 @@ export default Controller.extend({

unchanged: propertyEqual("newEmailLower", "oldEmail"),

currentUserAdmin: alias("currentUser.admin"),

@discourseComputed("newEmail")
newEmailLower(newEmail) {
return newEmail.toLowerCase().trim();
Expand Down Expand Up @@ -77,7 +80,25 @@ export default Controller.extend({
? this.model.addEmail(this.newEmail)
: this.model.changeEmail(this.newEmail)
).then(
() => this.set("success", true),
() => {
this.set("success", true);

if (this.model.staff) {
this.set(
"successMessage",
I18n.t("user.change_email.success_staff")
);
} else {
if (this.currentUser.admin) {
this.set(
"successMessage",
I18n.t("user.change_email.success_via_admin")
);
} else {
this.set("successMessage", I18n.t("user.change_email.success"));
}
}
},
(e) => {
this.setProperties({ error: true, saving: false });
if (
Expand Down
Expand Up @@ -11,13 +11,7 @@
<div class="control-group">
<div class="controls">
<div class="instructions">
<p>
{{#if model.staff}}
{{i18n "user.change_email.success_staff"}}
{{else}}
{{i18n "user.change_email.success"}}
{{/if}}
</p>
<p>{{ successMessage }}</p>
</div>
</div>
</div>
Expand All @@ -44,6 +38,9 @@
{{i18n "user.email.instructions"}}
{{/if}}
</div>
{{#if currentUserAdmin}}
<p>{{i18n "user.email.admin_note"}}</p>
{{/if}}
</div>
</div>

Expand Down
2 changes: 2 additions & 0 deletions config/locales/client.en.yml
Expand Up @@ -1148,6 +1148,7 @@ en:
taken: "Sorry, that email is not available."
error: "There was an error changing your email. Perhaps that address is already in use?"
success: "We've sent an email to that address. Please follow the confirmation instructions."
success_via_admin: "Because you are changing the email of the user, we assume that they have lost access to their original email account. We sent a reset password email to the new email address for this user. Please note the user must complete the reset password process for the email address change to take effect."
success_staff: "We've sent an email to your current address. Please follow the confirmation instructions."

change_avatar:
Expand Down Expand Up @@ -1190,6 +1191,7 @@ en:
sso_override_instructions: "Email can be updated from SSO provider."
no_secondary: "No secondary emails"
instructions: "Never shown to the public."
admin_note: "Note: An admin user changing another non-admin user's email indicates the user has lost access to their original email account, so a reset password email will be sent to their new address. The user's email will not change until they complete the reset password process."
ok: "We will email you to confirm"
required: "Please enter an email address"
invalid: "Please enter a valid email address"
Expand Down
1 change: 1 addition & 0 deletions lib/email_updater.rb
Expand Up @@ -120,6 +120,7 @@ def update_user_email(old_email, new_email)
else
@user.user_emails.create!(email: new_email)
end
@user.reload

DiscourseEvent.trigger(:user_updated, @user)
@user.set_automatic_groups
Expand Down
4 changes: 3 additions & 1 deletion spec/components/email_updater_spec.rb
Expand Up @@ -49,7 +49,7 @@ def expect_forgot_password_job

it "creates a change request authorizing the new email and immediately confirms it " do
updater.change_to(new_email)
change_req = user.email_change_requests.first
user.reload
expect(user.reload.email).to eq(new_email)
end

Expand All @@ -59,6 +59,8 @@ def expect_forgot_password_job
updater.change_to(new_email)
end
end

expect(EmailToken.where(user: user).last.email).to eq(new_email)
end
end

Expand Down

0 comments on commit 3cd601d

Please sign in to comment.