FIX: Admin change email for user process improvements and fixes (#10755)

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.
This commit is contained in:
Martin Brennan 2020-09-29 09:45:45 +10:00 committed by GitHub
parent fe86a7c7c8
commit 3cd601dcc9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 33 additions and 10 deletions

View File

@ -1,6 +1,6 @@
import I18n from "I18n"; import I18n from "I18n";
import discourseComputed from "discourse-common/utils/decorators"; 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 Controller from "@ember/controller";
import { propertyEqual } from "discourse/lib/computed"; import { propertyEqual } from "discourse/lib/computed";
import EmberObject from "@ember/object"; import EmberObject from "@ember/object";
@ -15,6 +15,7 @@ export default Controller.extend({
success: false, success: false,
oldEmail: null, oldEmail: null,
newEmail: null, newEmail: null,
successMessage: null,
newEmailEmpty: empty("newEmail"), newEmailEmpty: empty("newEmail"),
@ -28,6 +29,8 @@ export default Controller.extend({
unchanged: propertyEqual("newEmailLower", "oldEmail"), unchanged: propertyEqual("newEmailLower", "oldEmail"),
currentUserAdmin: alias("currentUser.admin"),
@discourseComputed("newEmail") @discourseComputed("newEmail")
newEmailLower(newEmail) { newEmailLower(newEmail) {
return newEmail.toLowerCase().trim(); return newEmail.toLowerCase().trim();
@ -77,7 +80,25 @@ export default Controller.extend({
? this.model.addEmail(this.newEmail) ? this.model.addEmail(this.newEmail)
: this.model.changeEmail(this.newEmail) : this.model.changeEmail(this.newEmail)
).then( ).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) => { (e) => {
this.setProperties({ error: true, saving: false }); this.setProperties({ error: true, saving: false });
if ( if (

View File

@ -11,13 +11,7 @@
<div class="control-group"> <div class="control-group">
<div class="controls"> <div class="controls">
<div class="instructions"> <div class="instructions">
<p> <p>{{ successMessage }}</p>
{{#if model.staff}}
{{i18n "user.change_email.success_staff"}}
{{else}}
{{i18n "user.change_email.success"}}
{{/if}}
</p>
</div> </div>
</div> </div>
</div> </div>
@ -44,6 +38,9 @@
{{i18n "user.email.instructions"}} {{i18n "user.email.instructions"}}
{{/if}} {{/if}}
</div> </div>
{{#if currentUserAdmin}}
<p>{{i18n "user.email.admin_note"}}</p>
{{/if}}
</div> </div>
</div> </div>

View File

@ -1148,6 +1148,7 @@ en:
taken: "Sorry, that email is not available." taken: "Sorry, that email is not available."
error: "There was an error changing your email. Perhaps that address is already in use?" 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: "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." success_staff: "We've sent an email to your current address. Please follow the confirmation instructions."
change_avatar: change_avatar:
@ -1190,6 +1191,7 @@ en:
sso_override_instructions: "Email can be updated from SSO provider." sso_override_instructions: "Email can be updated from SSO provider."
no_secondary: "No secondary emails" no_secondary: "No secondary emails"
instructions: "Never shown to the public." 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" ok: "We will email you to confirm"
required: "Please enter an email address" required: "Please enter an email address"
invalid: "Please enter a valid email address" invalid: "Please enter a valid email address"

View File

@ -120,6 +120,7 @@ class EmailUpdater
else else
@user.user_emails.create!(email: new_email) @user.user_emails.create!(email: new_email)
end end
@user.reload
DiscourseEvent.trigger(:user_updated, @user) DiscourseEvent.trigger(:user_updated, @user)
@user.set_automatic_groups @user.set_automatic_groups

View File

@ -49,7 +49,7 @@ describe EmailUpdater do
it "creates a change request authorizing the new email and immediately confirms it " do it "creates a change request authorizing the new email and immediately confirms it " do
updater.change_to(new_email) updater.change_to(new_email)
change_req = user.email_change_requests.first user.reload
expect(user.reload.email).to eq(new_email) expect(user.reload.email).to eq(new_email)
end end
@ -59,6 +59,8 @@ describe EmailUpdater do
updater.change_to(new_email) updater.change_to(new_email)
end end
end end
expect(EmailToken.where(user: user).last.email).to eq(new_email)
end end
end end