Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Improve UX support for multiple email addresses (#9691)

This commit is contained in:
Dan Ungureanu 2020-06-10 19:11:49 +03:00 committed by GitHub
parent 65dd8e2fa2
commit 5bfe1ee4f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 686 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
app/assets/javascripts/discourse/app/components/email-dropdown.js Normal file
View File

@ -0,0 +1,63 @@
import { action, computed } from "@ember/object";
import { inject as service } from "@ember/service";
import I18n from "I18n";
import DropdownSelectBoxComponent from "select-kit/components/dropdown-select-box";
export default DropdownSelectBoxComponent.extend({
router: service(),
classNames: ["email-dropdown"],
selectKitOptions: {
icon: "wrench",
showFullTitle: false
},
content: computed("email", function() {
const content = [];
if (this.email.primary) {
content.push({
id: "updateEmail",
icon: "pencil-alt",
name: I18n.t("user.email.update_email"),
description: ""
});
}
if (!this.email.primary && this.email.confirmed) {
content.push({
id: "setPrimaryEmail",
icon: "star",
name: I18n.t("user.email.set_primary"),
description: ""
});
}
if (!this.email.primary) {
content.push({
id: "destroyEmail",
icon: "times",
name: I18n.t("user.email.destroy"),
description: ""
});
}
return content;
}),
@action
onChange(id) {
switch (id) {
case "updateEmail":
this.router.transitionTo("preferences.email");
break;
case "setPrimaryEmail":
this.setPrimaryEmail(this.email.email);
break;
case "destroyEmail":
this.destroyEmail(this.email.email);
break;
}
}
});

54
app/assets/javascripts/discourse/app/controllers/preferences/account.js
View File

@ -12,6 +12,7 @@ import { findAll } from "discourse/models/login-method";
import { ajax } from "discourse/lib/ajax";
import { userPath } from "discourse/lib/url";
import logout from "discourse/lib/logout";
import EmberObject from "@ember/object";
// Number of tokens shown by default.
const DEFAULT_AUTH_TOKENS_COUNT = 2;
@ -95,6 +96,39 @@ export default Controller.extend(CanCheckEmails, {
disableConnectButtons: propertyNotEqual("model.id", "currentUser.id"),
@discourseComputed(
"model.email",
"model.secondary_emails.[]",
"model.unconfirmed_emails.[]"
)
emails(primaryEmail, secondaryEmails, unconfirmedEmails) {
const emails = [];
if (primaryEmail) {
emails.push(
EmberObject.create({
email: primaryEmail,
primary: true,
confirmed: true
})
);
}
if (secondaryEmails) {
secondaryEmails.forEach(email => {
emails.push(EmberObject.create({ email, confirmed: true }));
});
}
if (unconfirmedEmails) {
unconfirmedEmails.forEach(email => {
emails.push(EmberObject.create({ email }));
});
}
return emails.sort((a, b) => a.email.localeCompare(b.email));
},
@discourseComputed(
"model.second_factor_enabled",
"canCheckEmails",
@ -149,6 +183,26 @@ export default Controller.extend(CanCheckEmails, {
.catch(popupAjaxError);
},
setPrimaryEmail(email) {
this.model.setPrimaryEmail(email).catch(popupAjaxError);
},
destroyEmail(email) {
this.model.destroyEmail(email);
},
resendConfirmationEmail(email) {
email.set("resending", true);
this.model
.addEmail(email.email)
.then(() => {
email.set("resent", true);
})
.finally(() => {
email.set("resending", false);
});
},
changePassword() {
if (!this.passwordProgress) {
this.set(

23
app/assets/javascripts/discourse/app/controllers/preferences/email.js
View File

@ -7,10 +7,13 @@ import EmberObject from "@ember/object";
import { emailValid } from "discourse/lib/utilities";
export default Controller.extend({
queryParams: ["new"],
taken: false,
saving: false,
error: false,
success: false,
oldEmail: null,
newEmail: null,
newEmailEmpty: empty("newEmail"),
@ -23,16 +26,17 @@ export default Controller.extend({
"invalidEmail"
),
unchanged: propertyEqual("newEmailLower", "currentUser.email"),
unchanged: propertyEqual("newEmailLower", "oldEmail"),
@discourseComputed("newEmail")
newEmailLower(newEmail) {
return newEmail.toLowerCase().trim();
},
@discourseComputed("saving")
saveButtonText(saving) {
@discourseComputed("saving", "new")
saveButtonText(saving, isNew) {
if (saving) return I18n.t("saving");
if (isNew) return I18n.t("user.add_email.add");
return I18n.t("user.change");
},
@ -41,9 +45,9 @@ export default Controller.extend({
return !emailValid(newEmail);
},
@discourseComputed("invalidEmail")
emailValidation(invalidEmail) {
if (invalidEmail) {
@discourseComputed("invalidEmail", "oldEmail", "newEmail")
emailValidation(invalidEmail, oldEmail, newEmail) {
if (invalidEmail && (oldEmail || newEmail)) {
return EmberObject.create({
failed: true,
reason: I18n.t("user.email.invalid")
@ -62,10 +66,13 @@ export default Controller.extend({
},
actions: {
changeEmail() {
saveEmail() {
this.set("saving", true);
return this.model.changeEmail(this.newEmail).then(
return (this.new
? this.model.addEmail(this.newEmail)
: this.model.changeEmail(this.newEmail)
).then(
() => this.set("success", true),
e => {
this.setProperties({ error: true, saving: false });

28
app/assets/javascripts/discourse/app/models/user.js
View File

@ -246,6 +246,13 @@ const User = RestModel.extend({
});
},
addEmail(email) {
return ajax(userPath(`${this.username_lower}/preferences/email`), {
type: "POST",
data: { email }
});
},
changeEmail(email) {
return ajax(userPath(`${this.username_lower}/preferences/email`), {
type: "PUT",
@ -375,6 +382,27 @@ const User = RestModel.extend({
});
},
setPrimaryEmail(email) {
return ajax(userPath(`${this.username}/preferences/primary-email.json`), {
type: "PUT",
data: { email }
}).then(() => {
this.secondary_emails.removeObject(email);
this.secondary_emails.pushObject(this.email);
this.set("email", email);
});
},
destroyEmail(email) {
return ajax(userPath(`${this.username}/preferences/email.json`), {
type: "DELETE",
data: { email }
}).then(() => {
this.secondary_emails.removeObject(email);
this.unconfirmed_emails.removeObject(email);
});
},
changePassword() {
return ajax("/session/forgot_password", {
dataType: "json",

12
app/assets/javascripts/discourse/app/routes/preferences-email.js
View File

@ -13,7 +13,17 @@ export default RestrictedUserRoute.extend({
setupController: function(controller, model) {
controller.reset();
controller.setProperties({ model: model, newEmail: model.get("email") });
controller.setProperties({
model: model,
oldEmail: controller.new ? "" : model.get("email"),
newEmail: controller.new ? "" : model.get("email")
});
},
resetController: function(controller, isExiting) {
if (isExiting) {
controller.set("new", undefined);
}
},
// A bit odd, but if we leave to /preferences we need to re-render that outlet

4
app/assets/javascripts/discourse/app/templates/preferences-email.hbs
View File

@ -3,7 +3,7 @@
<div class="control-group">
<div class="controls">
<h3>{{i18n "user.change_email.title"}}</h3>
<h3>{{i18n (if new "user.add_email.title" "user.change_email.title")}}</h3>
</div>
</div>
@ -51,7 +51,7 @@
<div class="controls">
{{#d-button
class="btn-primary"
action=(action "changeEmail")
action=(action "saveEmail")
type="submit"
disabled=saveDisabled
}}

49
app/assets/javascripts/discourse/app/templates/preferences/account.hbs
View File

@ -46,12 +46,49 @@
<div class="control-group pref-email">
<label class="control-label">{{i18n "user.email.title"}}</label>
{{#if model.email}}
<div class="controls">
<span class="static">{{model.email}}</span>
{{#if model.can_edit_email}}
{{#link-to "preferences.email" class="btn btn-default btn-small btn-icon pad-left no-text"}}{{d-icon "pencil-alt"}}{{/link-to}}
{{/if}}
</div>
{{#if siteSettings.enable_secondary_emails}}
<div class="emails">
{{#each emails as |email|}}
<div class="row email">
{{email-dropdown email=email
setPrimaryEmail=(action "setPrimaryEmail")
destroyEmail=(action "destroyEmail")}}
<div class="email-first">{{email.email}}</div>
<div class="email-second">
{{#if email.primary}}
<span class="primary">{{i18n "user.email.primary_label"}}</span>
{{/if}}
{{#unless email.confirmed}}
<span class="unconfirmed">{{i18n "user.email.unconfirmed_label"}}</span>
&bull;
{{#if email.resending}}
<span>{{i18n "user.email.resending_label"}}</span>
{{else if email.resent}}
<span>{{i18n "user.email.resent_label"}}</span>
{{else}}
<a {{action "resendConfirmationEmail" email}} href>{{i18n "user.email.resend_label"}}</a>
{{/if}}
{{/unless}}
</div>
</div>
{{/each}}
</div>
{{#link-to "preferences.email" (query-params new=1) class="pull-right"}}
{{d-icon "plus"}} {{i18n "user.email.add_email"}}
{{/link-to}}
{{else}}
<div class="controls">
<span class="static">{{model.email}}</span>
{{#if model.can_edit_email}}
{{#link-to "preferences.email" class="btn btn-default btn-small btn-icon pad-left no-text"}}{{d-icon "pencil-alt"}}{{/link-to}}
{{/if}}
</div>
{{/if}}
<div class="instructions">
{{#if siteSettings.sso_overrides_email}}
{{i18n "user.email.sso_override_instructions"}}

50
app/assets/stylesheets/common/base/discourse.scss
View File

@ -640,6 +640,56 @@ table {
display: inline;
}
.pref-email {
.row {
border-bottom: 1px solid #ddd;
margin: 5px 0px;
padding-bottom: 5px;
&:last-child {
border-bottom: 0;
}
}
.email-first {
font-size: 1.1em;
}
.email-second {
color: $primary-medium;
.primary {
color: $success;
font-weight: bold;
}
.unconfirmed {
font-style: italic;
}
}
.email-dropdown {
float: right;
.btn,
.btn:hover {
background: transparent;
.d-icon {
color: $primary-high;
}
}
}
.dropdown-menu {
width: 120px;
& .icon {
margin-top: auto;
}
}
}
.pref-auth-tokens {
.row {
border-bottom: 1px solid #ddd;

66
app/controllers/users_controller.rb
View File

@ -205,6 +205,72 @@ class UsersController < ApplicationController
render json: failed_json, status: 403
end
def update_primary_email
if !SiteSetting.enable_secondary_emails
return render json: failed_json, status: 410
end
params.require(:email)
user = fetch_user_from_params
guardian.ensure_can_edit_email!(user)
old_primary = user.primary_email
if old_primary.email == params[:email]
return render json: success_json
end
new_primary = user.user_emails.find_by(email: params[:email])
if new_primary.blank?
return render json: failed_json.merge(errors: [I18n.t("change_email.doesnt_exist")]), status: 428
end
User.transaction do
old_primary.update!(primary: false)
new_primary.update!(primary: true)
if current_user.staff? && current_user != user
StaffActionLogger.new(current_user).log_update_email(user)
else
UserHistory.create!(action: UserHistory.actions[:update_email], target_user_id: user.id)
end
end
render json: success_json
end
def destroy_email
if !SiteSetting.enable_secondary_emails
return render json: failed_json, status: 410
end
params.require(:email)
user = fetch_user_from_params
guardian.ensure_can_edit!(user)
user_email = user.user_emails.find_by(email: params[:email])
if user_email&.primary
return render json: failed_json, status: 428
end
ActiveRecord::Base.transaction do
if user_email
user_email.destroy
elsif
user.email_change_requests.where(new_email: params[:email]).destroy_all
end
if current_user.staff? && current_user != user
StaffActionLogger.new(current_user).log_destroy_email(user)
else
UserHistory.create(action: UserHistory.actions[:destroy_email], target_user_id: user.id)
end
end
render json: success_json
end
def topic_tracking_state
user = fetch_user_from_params
guardian.ensure_can_edit!(user)

27
app/controllers/users_email_controller.rb
View File

@ -28,12 +28,35 @@ class UsersEmailController < ApplicationController
def index
end
def create
if !SiteSetting.enable_secondary_emails
return render json: failed_json, status: 410
end
params.require(:email)
user = fetch_user_from_params
RateLimiter.new(user, "email-hr-#{request.remote_ip}", 6, 1.hour).performed!
RateLimiter.new(user, "email-min-#{request.remote_ip}", 3, 1.minute).performed!
updater = EmailUpdater.new(guardian: guardian, user: user)
updater.change_to(params[:email], add: true)
if updater.errors.present?
return render_json_error(updater.errors.full_messages)
end
render body: nil
rescue RateLimiter::LimitExceeded
render_json_error(I18n.t("rate_limiter.slow_down"))
end
def update
params.require(:email)
user = fetch_user_from_params
RateLimiter.new(user, "change-email-hr-#{request.remote_ip}", 6, 1.hour).performed!
RateLimiter.new(user, "change-email-min-#{request.remote_ip}", 3, 1.minute).performed!
RateLimiter.new(user, "email-hr-#{request.remote_ip}", 6, 1.hour).performed!
RateLimiter.new(user, "email-min-#{request.remote_ip}", 3, 1.minute).performed!
updater = EmailUpdater.new(guardian: guardian, user: user)
updater.change_to(params[:email])

2
app/jobs/regular/user_email.rb
View File

@ -154,7 +154,7 @@ module Jobs
raise Discourse::InvalidParameters.new("type=#{type}") unless UserNotifications.respond_to?(type)
email_args[:email_token] = email_token if email_token.present?
email_args[:new_email] = user.email if type.to_s == "notify_old_email"
email_args[:new_email] = args[:new_email] || user.email if type.to_s == "notify_old_email" || type.to_s == "notify_old_email_add"
if args[:client_ip] && args[:user_agent]
email_args[:client_ip] = args[:client_ip]

15
app/mailers/user_notifications.rb
View File

@ -63,6 +63,13 @@ class UserNotifications < ActionMailer::Base
new_email: opts[:new_email])
end
def notify_old_email_add(user, opts = {})
build_email(user.email,
template: "user_notifications.notify_old_email_add",
locale: user_locale(user),
new_email: opts[:new_email])
end
def confirm_old_email(user, opts = {})
build_user_email_token_by_template(
"user_notifications.confirm_old_email",
@ -71,6 +78,14 @@ class UserNotifications < ActionMailer::Base
)
end
def confirm_old_email_add(user, opts = {})
build_user_email_token_by_template(
"user_notifications.confirm_old_email_add",
user,
opts[:email_token]
)
end
def confirm_new_email(user, opts = {})
build_user_email_token_by_template(
"user_notifications.confirm_new_email",

1
app/models/email_change_request.rb
View File

@ -5,7 +5,6 @@ class EmailChangeRequest < ActiveRecord::Base
belongs_to :new_email_token, class_name: 'EmailToken'
belongs_to :user
validates :old_email, presence: true
validates :new_email, presence: true, format: { with: EmailValidator.email_regex }
def self.states

4
app/models/user.rb
View File

@ -1212,6 +1212,10 @@ class User < ActiveRecord::Base
self.user_emails.secondary.pluck(:email)
end
def unconfirmed_emails
self.email_change_requests.where.not(change_state: EmailChangeRequest.states[:complete]).pluck(:new_email)
end
RECENT_TIME_READ_THRESHOLD ||= 60.days
def self.preload_recent_time_read(users)

10
app/models/user_history.rb
View File

@ -105,7 +105,10 @@ class UserHistory < ActiveRecord::Base
change_title: 84,
override_upload_secure_status: 85,
page_published: 86,
page_unpublished: 87
page_unpublished: 87,
add_email: 88,
update_email: 89,
destroy_email: 90,
)
end
@ -187,7 +190,10 @@ class UserHistory < ActiveRecord::Base
:api_key_destroy,
:override_upload_secure_status,
:page_published,
:page_unpublished
:page_unpublished,
:add_email,
:update_email,
:destroy_email
]
end

2
app/serializers/user_card_serializer.rb
View File

@ -34,6 +34,8 @@ class UserCardSerializer < BasicUserSerializer
end
attributes :email,
:secondary_emails,
:unconfirmed_emails,
:last_posted_at,
:last_seen_at,
:created_at,

1
app/serializers/web_hook_user_serializer.rb
View File

@ -8,6 +8,7 @@ class WebHookUserSerializer < UserSerializer
end
%i{
unconfirmed_emails
can_edit
can_edit_username
can_edit_email

30
app/services/staff_action_logger.rb
View File

@ -744,6 +744,36 @@ class StaffActionLogger
))
end
def log_add_email(user)
raise Discourse::InvalidParameters.new(:user) unless user
UserHistory.create!(
action: UserHistory.actions[:add_email],
acting_user_id: @admin.id,
target_user_id: user.id
)
end
def log_update_email(user)
raise Discourse::InvalidParameters.new(:user) unless user
UserHistory.create!(
action: UserHistory.actions[:update_email],
acting_user_id: @admin.id,
target_user_id: user.id
)
end
def log_destroy_email(user)
raise Discourse::InvalidParameters.new(:user) unless user
UserHistory.create!(
action: UserHistory.actions[:destroy_email],
acting_user_id: @admin.id,
target_user_id: user.id
)
end
private
def get_changes(changes)

6
app/views/users_email/show_confirm_new_email.html.erb
View File

@ -13,7 +13,11 @@
<% else %>
<h2><%= t 'change_email.authorizing_new.title' %></h2>
<p>
<%= t 'change_email.authorizing_new.description' %>
<% if @change_request&.old_email %>
<%= t 'change_email.authorizing_new.description' %>
<% else %>
<%= t 'change_email.authorizing_new.description_add' %>
<% end %>
</p>
<p>
<%= @to_email %>

19
app/views/users_email/show_confirm_old_email.html.erb
View File

@ -11,12 +11,19 @@
<% else %>
<h2><%= t 'change_email.authorizing_old.title' %></h2>
<p>
<%= t 'change_email.authorizing_old.description' %>
<br>
<br>
<%= t 'change_email.authorizing_old.old_email', email: @from_email %>
<br>
<%= t 'change_email.authorizing_old.new_email', email: @to_email %>
<% if @change_request&.old_email %>
<%= t 'change_email.authorizing_old.description' %>
<br>
<br>
<%= t 'change_email.authorizing_old.old_email', email: @from_email %>
<br>
<%= t 'change_email.authorizing_old.new_email', email: @to_email %>
<% else %>
<%= t 'change_email.authorizing_old.description_add' %>
<br>
<br>
<%= @to_email %>
<% end %>
</p>
<%=form_tag(u_confirm_old_email_path, method: :put) do %>

19
config/locales/client.en.yml
View File

@ -1083,6 +1083,10 @@ en:
taken: "Sorry, that username is taken."
invalid: "That username is invalid. It must only include numbers and letters"
add_email:
title: "Add Email"
add: "add"
change_email:
title: "Change Email"
taken: "Sorry, that email is not available."
@ -1118,8 +1122,16 @@ en:
title: "Email"
primary: "Primary Email"
secondary: "Secondary Emails"
no_secondary: "No secondary emails"
sso_override_instructions: "Email can be updated from SSO provider."
primary_label: "primary"
unconfirmed_label: "unconfirmed"
resend_label: "resend confirmation email"
resending_label: "sending..."
resent_label: "email sent"
update_email: "Change Email"
set_primary: "Set Primary Email"
destroy: "Remove Email"
add_email: "Add Alternate Email"
ssDestroy_override_instructions: "Email can be updated from SSO provider."
instructions: "Never shown to the public."
ok: "We will email you to confirm"
required: "Please enter an email address"
@ -4175,6 +4187,9 @@ en:
override_upload_secure_status: "override upload secure status"
page_published: "page published"
page_unpublished: "page unpublished"
add_email: "add email"
update_email: "update email"
destroy_email: "destroy email"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."

29
config/locales/server.en.yml
View File

@ -845,17 +845,20 @@ en:
confirmed: "Your email has been updated."
please_continue: "Continue to %{site_name}"
error: "There was an error changing your email address. Perhaps the address is already in use?"
doesnt_exist: "That email address is not associated with your account."
error_staged: "There was an error changing your email address. The address is already in use by a staged user."
already_done: "Sorry, this confirmation link is no longer valid. Perhaps your email was already changed?"
confirm: "Confirm"
authorizing_new:
title: "Confirm your new email"
description: "Please confirm you would like your new email address changed to:"
description: "Please confirm you would like your email address changed to:"
description_add: "Please confirm you would like to add an alternate email address:"
authorizing_old:
title: "Change your email address"
description: "Please confirm your email address change"
description_add: "Please confirm you would like to add an alternate email address:"
old_email: "Old email: %{email}"
new_email: "New email: %{email}"
almost_done_title: "Confirming new email address"
@ -3674,6 +3677,18 @@ en:
%{base_url}/u/confirm-old-email/%{email_token}
confirm_old_email_add:
title: "Confirm Old Email (Add)"
subject_template: "[%{email_prefix}] Confirm your current email address"
text_body_template: |
Before we can add a new email address, we need you to confirm that you control
the current email account. After you complete this step, we will have you confirm
the new email address.
Confirm your current email address for %{site_name} by clicking on the following link:
%{base_url}/u/confirm-old-email/%{email_token}
notify_old_email:
title: "Notify Old Email"
subject_template: "[%{email_prefix}] Your email address has been changed"
@ -3686,6 +3701,18 @@ en:
%{new_email}
notify_old_email_add:
title: "Notify Old Email (Add)"
subject_template: "[%{email_prefix}] A new email address has been added"
text_body_template: |
This is an automated message to let you know that an email address for
%{site_name} has been added. If this was done in error, please contact
a site administrator.
Your added email address is:
%{new_email}
signup_after_approval:
title: "Signup After Approval"
subject_template: "You've been approved on %{site_name}!"

3
config/routes.rb
View File

@ -444,12 +444,15 @@ Discourse::Application.routes.draw do
get "#{root_path}/:username/preferences/account" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/profile" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/emails" => "users#preferences", constraints: { username: RouteFormat.username }
put "#{root_path}/:username/preferences/primary-email" => "users#update_primary_email", format: :json, constraints: { username: RouteFormat.username }
delete "#{root_path}/:username/preferences/email" => "users#destroy_email", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/notifications" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/categories" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/users" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/tags" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/interface" => "users#preferences", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/apps" => "users#preferences", constraints: { username: RouteFormat.username }
post "#{root_path}/:username/preferences/email" => "users_email#create", constraints: { username: RouteFormat.username }
put "#{root_path}/:username/preferences/email" => "users_email#update", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/badge_title" => "users#preferences", constraints: { username: RouteFormat.username }
put "#{root_path}/:username/preferences/badge_title" => "users#badge_title", constraints: { username: RouteFormat.username }

4
config/site_settings.yml
View File

@ -1124,6 +1124,10 @@ email:
delete_rejected_email_after_days:
default: 90
max: 36500
enable_secondary_emails:
client: true
default: true
hidden: true
files:
max_image_size_kb:

11
db/post_migrate/20200508141209_allow_null_old_email_on_email_change_requests.rb Normal file
View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class AllowNullOldEmailOnEmailChangeRequests < ActiveRecord::Migration[6.0]
def up
change_column :email_change_requests, :old_email, :string, null: true
end
def down
change_column :email_change_requests, :old_email, :string, null: false
end
end

143
lib/email_updater.rb
View File

@ -5,24 +5,21 @@ class EmailUpdater
attr_reader :user
def self.human_attribute_name(name, options = {})
User.human_attribute_name(name, options)
end
def initialize(guardian: nil, user: nil)
@guardian = guardian
@user = user
end
def self.human_attribute_name(name, options = {})
User.human_attribute_name(name, options)
end
def changing_staff_user_email?
@user.staff?
end
def change_to(email_input)
def change_to(email, add: false)
@guardian.ensure_can_edit_email!(@user)
email = Email.downcase(email_input.strip)
email = Email.downcase(email.strip)
EmailValidator.new(attributes: :email).validate_each(self, :email, email)
return if errors.present?
if existing_user = User.find_by_email(email)
if SiteSetting.hide_email_address_taken
@ -34,31 +31,51 @@ class EmailUpdater
end
end
if errors.blank? && existing_user.nil?
args = {
old_email: @user.email,
new_email: email,
}
return if errors.present? || existing_user.present?
args, email_token = prepare_change_request(args)
@user.email_change_requests.create!(args)
old_email = @user.email if !add
if initiating_admin_changing_another_user_email? && !changing_staff_user_email?
auto_confirm_and_send_password_reset(email_token)
return
end
if @guardian.is_staff? && @guardian.user != @user
StaffActionLogger.new(@guardian.user).log_add_email(@user)
else
UserHistory.create!(action: UserHistory.actions[:add_email], target_user_id: @user.id)
end
if args[:change_state] == EmailChangeRequest.states[:authorizing_new]
send_email(:confirm_new_email, email_token)
elsif args[:change_state] == EmailChangeRequest.states[:authorizing_old]
send_email(:confirm_old_email, email_token)
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
change_req = EmailChangeRequest.find_or_initialize_by(user_id: @user.id, new_email: email)
if change_req.new_record?
change_req.old_email = old_email
change_req.new_email = email
end
if change_req.change_state.blank?
change_req.change_state = if @user.staff?
# Staff users must confirm their old email address first.
EmailChangeRequest.states[:authorizing_old]
else
EmailChangeRequest.states[:authorizing_new]
end
end
if change_req.change_state == EmailChangeRequest.states[:authorizing_old]
change_req.old_email_token = @user.email_tokens.create!(email: @user.email)
send_email(add ? :confirm_old_email_add : :confirm_old_email, change_req.old_email_token)
elsif change_req.change_state == EmailChangeRequest.states[:authorizing_new]
change_req.new_email_token = @user.email_tokens.create!(email: email)
send_email(:confirm_new_email, change_req.new_email_token)
end
change_req.save!
end
def confirm(token)
confirm_result = nil
change_req = nil
User.transaction do
result = EmailToken.atomic_confirm(token)
@ -66,22 +83,26 @@ class EmailUpdater
token = result[:email_token]
@user = token.user
change_req = user.email_change_requests
change_req = @user.email_change_requests
.where('old_email_token_id = :token_id OR new_email_token_id = :token_id', token_id: token.id)
.first
# Simple state machine
case change_req.try(:change_state)
when EmailChangeRequest.states[:authorizing_old]
new_token = user.email_tokens.create(email: change_req.new_email)
change_req.update_columns(change_state: EmailChangeRequest.states[:authorizing_new],
new_email_token_id: new_token.id)
send_email(:confirm_new_email, new_token)
change_req.update!(
change_state: EmailChangeRequest.states[:authorizing_new],
new_email_token: @user.email_tokens.create(email: change_req.new_email)
)
send_email(:confirm_new_email, change_req.new_email_token)
confirm_result = :authorizing_new
when EmailChangeRequest.states[:authorizing_new]
change_req.update_column(:change_state, EmailChangeRequest.states[:complete])
user.primary_email.update!(email: token.email)
user.set_automatic_groups
change_req.update!(change_state: EmailChangeRequest.states[:complete])
if !@user.staff?
# Send an email notification only to users who did not confirm old
# email.
send_email_notification(change_req.old_email, change_req.new_email)
end
update_user_email(change_req.old_email, change_req.new_email)
confirm_result = :complete
end
else
@ -90,22 +111,21 @@ class EmailUpdater
end
end
if confirm_result == :complete && change_req.old_email_token_id.blank?
notify_old(change_req.old_email, token.email)
end
confirm_result || :error
end
protected
def update_user_email(old_email, new_email)
if old_email.present?
@user.user_emails.find_by(email: old_email).update!(email: new_email)
else
@user.user_emails.create!(email: new_email)
end
def notify_old(old_email, new_email)
Jobs.enqueue :critical_user_email,
to_address: old_email,
type: :notify_old_email,
user_id: @user.id
@user.set_automatic_groups
end
protected
def send_email(type, email_token)
Jobs.enqueue :critical_user_email,
to_address: email_token.email,
@ -114,32 +134,11 @@ class EmailUpdater
email_token: email_token.token
end
# regular users or changes requested by admin are always
# authorizing new only (as long as they are not changing their
# own email!)
#
# however even if an admin requests an email change for another
# admin the other admin must always confirm their old email
def prepare_change_request(args)
if changing_staff_user_email?
args[:change_state] = EmailChangeRequest.states[:authorizing_old]
email_token = @user.email_tokens.create!(email: args[:old_email])
args[:old_email_token] = email_token
else
args[:change_state] = EmailChangeRequest.states[:authorizing_new]
email_token = @user.email_tokens.create!(email: args[:new_email])
args[:new_email_token] = email_token
end
[args, email_token]
end
def initiating_admin_changing_another_user_email?
@guardian.is_admin? && @guardian.user != @user
end
def auto_confirm_and_send_password_reset(email_token)
confirm(email_token.token)
reset_email_token = @user.email_tokens.create(email: @user.email)
send_email(:forgot_password, reset_email_token)
def send_email_notification(old_email, new_email)
Jobs.enqueue :critical_user_email,
to_address: @user.email,
type: old_email ? :notify_old_email : :notify_old_email_add,
user_id: @user.id,
new_email: new_email
end
end

1
lib/svg_sprite/svg_sprite.rb
View File

@ -170,6 +170,7 @@ module SvgSprite
"sign-in-alt",
"sign-out-alt",
"signal",
"star",
"step-backward",
"step-forward",
"stream",

85
spec/components/email_updater_spec.rb
View File

@ -42,7 +42,7 @@ describe EmailUpdater do
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
expect(change_req.change_state).to eq(EmailChangeRequest.states[:complete])
expect(user.reload.email).to eq(new_email)
end
it "sends a reset password email to the user so they can set a password for their new email" do
@ -110,44 +110,65 @@ describe EmailUpdater do
let(:user) { Fabricate(:user, email: old_email) }
let(:updater) { EmailUpdater.new(guardian: user.guardian, user: user) }
before do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email))
updater.change_to(new_email)
@change_req = user.email_change_requests.first
end
it "starts the new confirmation process" do
expect(updater.errors).to be_blank
expect(@change_req).to be_present
expect(@change_req.change_state).to eq(EmailChangeRequest.states[:authorizing_new])
expect(@change_req.old_email).to eq(old_email)
expect(@change_req.new_email).to eq(new_email)
expect(@change_req.old_email_token).to be_blank
expect(@change_req.new_email_token.email).to eq(new_email)
end
context 'confirming an invalid token' do
it "produces an error" do
updater.confirm('random')
expect(updater.errors).to be_present
expect(user.reload.email).not_to eq(new_email)
context "changing primary email" do
before do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email))
updater.change_to(new_email)
@change_req = user.email_change_requests.first
end
end
context 'confirming a valid token' do
it "updates the user's email" do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email))
updater.confirm(@change_req.new_email_token.token)
it "starts the new confirmation process" do
expect(updater.errors).to be_blank
expect(user.reload.email).to eq(new_email)
@change_req.reload
expect(@change_req.change_state).to eq(EmailChangeRequest.states[:complete])
expect(@change_req).to be_present
expect(@change_req.change_state).to eq(EmailChangeRequest.states[:authorizing_new])
expect(@change_req.old_email).to eq(old_email)
expect(@change_req.new_email).to eq(new_email)
expect(@change_req.old_email_token).to be_blank
expect(@change_req.new_email_token.email).to eq(new_email)
end
context 'confirming an invalid token' do
it "produces an error" do
updater.confirm('random')
expect(updater.errors).to be_present
expect(user.reload.email).not_to eq(new_email)
end
end
context 'confirming a valid token' do
it "updates the user's email" do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email))
updater.confirm(@change_req.new_email_token.token)
expect(updater.errors).to be_blank
expect(user.reload.email).to eq(new_email)
@change_req.reload
expect(@change_req.change_state).to eq(EmailChangeRequest.states[:complete])
end
end
end
context "adding an email" do
before do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email))
updater.change_to(new_email, add: true)
@change_req = user.email_change_requests.first
end
context 'confirming a valid token' do
it "adds a user email" do
Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email_add, to_address: old_email))
updater.confirm(@change_req.new_email_token.token)
expect(updater.errors).to be_blank
expect(UserEmail.where(user_id: user.id).pluck(:email)).to contain_exactly(user.email, new_email)
@change_req.reload
expect(@change_req.change_state).to eq(EmailChangeRequest.states[:complete])
end
end
end
end
context 'as a staff user' do

46
spec/requests/users_controller_spec.rb
View File

@ -2558,6 +2558,52 @@ describe UsersController do
end
end
describe '#update_primary_email' do
fab!(:user) { Fabricate(:user) }
fab!(:user_email) { user.primary_email }
fab!(:other_email) { Fabricate(:secondary_email, user: user) }
before do
SiteSetting.email_editable = true
sign_in(user)
end
it "changes user's primary email" do
put "/u/#{user.username}/preferences/primary-email.json", params: { email: user_email.email }
expect(response.status).to eq(200)
expect(user_email.reload.primary).to eq(true)
expect(other_email.reload.primary).to eq(false)
put "/u/#{user.username}/preferences/primary-email.json", params: { email: other_email.email }
expect(response.status).to eq(200)
expect(user_email.reload.primary).to eq(false)
expect(other_email.reload.primary).to eq(true)
end
end
describe '#destroy_email' do
fab!(:user) { Fabricate(:user) }
fab!(:user_email) { user.primary_email }
fab!(:other_email) { Fabricate(:secondary_email, user: user) }
before do
SiteSetting.email_editable = true
sign_in(user)
end
it "can destroy secondary emails" do
delete "/u/#{user.username}/preferences/email.json", params: { email: user_email.email }
expect(response.status).to eq(428)
expect(user.reload.user_emails.pluck(:email)).to contain_exactly(user_email.email, other_email.email)
delete "/u/#{user.username}/preferences/email.json", params: { email: other_email.email }
expect(response.status).to eq(200)
expect(user.reload.user_emails.pluck(:email)).to contain_exactly(user_email.email)
end
end
describe '#is_local_username' do
fab!(:user) { Fabricate(:user) }
fab!(:group) { Fabricate(:group, name: "Discourse", mentionable_level: Group::ALIAS_LEVELS[:everyone]) }

15
spec/requests/users_email_controller_spec.rb
View File

@ -261,6 +261,21 @@ describe UsersEmailController do
end
end
describe '#create' do
let(:new_email) { 'bubblegum@adventuretime.ooo' }
it 'has an email token' do
sign_in(user)
expect { post "/u/#{user.username}/preferences/email.json", params: { email: new_email } }
.to change(EmailChangeRequest, :count)
emailChangeRequest = EmailChangeRequest.last
expect(emailChangeRequest.old_email).to eq(nil)
expect(emailChangeRequest.new_email).to eq(new_email)
end
end
describe '#update' do
let(:new_email) { 'bubblegum@adventuretime.ooo' }

2
spec/serializers/web_hook_user_serializer_spec.rb
View File

@ -23,7 +23,7 @@ RSpec.describe WebHookUserSerializer do
it 'should only include the required keys' do
count = serializer.as_json.keys.count
difference = count - 45
difference = count - 46
expect(difference).to eq(0), lambda {
message = (difference < 0 ?