diff --git a/app/assets/javascripts/discourse/app/components/email-dropdown.js b/app/assets/javascripts/discourse/app/components/email-dropdown.js new file mode 100644 index 00000000000..1a4046b2402 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/email-dropdown.js @@ -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; + } + } +}); diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/account.js b/app/assets/javascripts/discourse/app/controllers/preferences/account.js index 0a6f54c2800..6eecf3e7e94 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/account.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/account.js @@ -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( diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/email.js b/app/assets/javascripts/discourse/app/controllers/preferences/email.js index f79e4d15d10..3365faf0f21 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/email.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/email.js @@ -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 }); diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 113b592c277..0f67d613b23 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -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", diff --git a/app/assets/javascripts/discourse/app/routes/preferences-email.js b/app/assets/javascripts/discourse/app/routes/preferences-email.js index c3fb268c4d6..f2d070ad009 100644 --- a/app/assets/javascripts/discourse/app/routes/preferences-email.js +++ b/app/assets/javascripts/discourse/app/routes/preferences-email.js @@ -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 diff --git a/app/assets/javascripts/discourse/app/templates/preferences-email.hbs b/app/assets/javascripts/discourse/app/templates/preferences-email.hbs index 1c561e265f1..447d220403c 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences-email.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences-email.hbs @@ -3,7 +3,7 @@
-

{{i18n "user.change_email.title"}}

+

{{i18n (if new "user.add_email.title" "user.change_email.title")}}

@@ -51,7 +51,7 @@
{{#d-button class="btn-primary" - action=(action "changeEmail") + action=(action "saveEmail") type="submit" disabled=saveDisabled }} diff --git a/app/assets/javascripts/discourse/app/templates/preferences/account.hbs b/app/assets/javascripts/discourse/app/templates/preferences/account.hbs index cd931ff10e0..47df8c73fa9 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/account.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/account.hbs @@ -46,12 +46,49 @@
{{#if model.email}} -
- {{model.email}} - {{#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}} -
+ {{#if siteSettings.enable_secondary_emails}} +
+ {{#each emails as |email|}} + + {{/each}} +
+ + {{#link-to "preferences.email" (query-params new=1) class="pull-right"}} + {{d-icon "plus"}} {{i18n "user.email.add_email"}} + {{/link-to}} + {{else}} +
+ {{model.email}} + {{#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}} +
+ {{/if}} +
{{#if siteSettings.sso_overrides_email}} {{i18n "user.email.sso_override_instructions"}} diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index fd6dfef8de3..1dcb5e858fd 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -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; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 57a2f7a400f..1ca9d9b99e9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -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) diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb index ae7b4c56230..898ddd6897a 100644 --- a/app/controllers/users_email_controller.rb +++ b/app/controllers/users_email_controller.rb @@ -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]) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index bee715099d2..1197499c417 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -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] diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 6ccb86c8602..184c62b1ec1 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -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", diff --git a/app/models/email_change_request.rb b/app/models/email_change_request.rb index f54df4bf23e..d5fd0e57d9c 100644 --- a/app/models/email_change_request.rb +++ b/app/models/email_change_request.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 3caf839a2de..15ed2fb531a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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) diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 49a545b8156..26e7b8ab739 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -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 diff --git a/app/serializers/user_card_serializer.rb b/app/serializers/user_card_serializer.rb index c453dff2e4d..e3ec9357fb1 100644 --- a/app/serializers/user_card_serializer.rb +++ b/app/serializers/user_card_serializer.rb @@ -34,6 +34,8 @@ class UserCardSerializer < BasicUserSerializer end attributes :email, + :secondary_emails, + :unconfirmed_emails, :last_posted_at, :last_seen_at, :created_at, diff --git a/app/serializers/web_hook_user_serializer.rb b/app/serializers/web_hook_user_serializer.rb index 31d178b1a09..d431b213e1e 100644 --- a/app/serializers/web_hook_user_serializer.rb +++ b/app/serializers/web_hook_user_serializer.rb @@ -8,6 +8,7 @@ class WebHookUserSerializer < UserSerializer end %i{ + unconfirmed_emails can_edit can_edit_username can_edit_email diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 6802be34622..0181a558745 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -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) diff --git a/app/views/users_email/show_confirm_new_email.html.erb b/app/views/users_email/show_confirm_new_email.html.erb index 45a3f2a7d21..ba5102e13d0 100644 --- a/app/views/users_email/show_confirm_new_email.html.erb +++ b/app/views/users_email/show_confirm_new_email.html.erb @@ -13,7 +13,11 @@ <% else %>

<%= t 'change_email.authorizing_new.title' %>

- <%= 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 %>

<%= @to_email %> diff --git a/app/views/users_email/show_confirm_old_email.html.erb b/app/views/users_email/show_confirm_old_email.html.erb index 6d810931860..9fc65856cb6 100644 --- a/app/views/users_email/show_confirm_old_email.html.erb +++ b/app/views/users_email/show_confirm_old_email.html.erb @@ -11,12 +11,19 @@ <% else %>

<%= t 'change_email.authorizing_old.title' %>

- <%= t 'change_email.authorizing_old.description' %> -
-
- <%= t 'change_email.authorizing_old.old_email', email: @from_email %> -
- <%= t 'change_email.authorizing_old.new_email', email: @to_email %> + <% if @change_request&.old_email %> + <%= t 'change_email.authorizing_old.description' %> +
+
+ <%= t 'change_email.authorizing_old.old_email', email: @from_email %> +
+ <%= t 'change_email.authorizing_old.new_email', email: @to_email %> + <% else %> + <%= t 'change_email.authorizing_old.description_add' %> +
+
+ <%= @to_email %> + <% end %>

<%=form_tag(u_confirm_old_email_path, method: :put) do %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 2de77fce976..104edd8793c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -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." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index eec321b8d81..d2e875d1177 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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}!" diff --git a/config/routes.rb b/config/routes.rb index 96b81c8e260..0de10003e56 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 } diff --git a/config/site_settings.yml b/config/site_settings.yml index e240fe35261..08e16c94f1f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -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: diff --git a/db/post_migrate/20200508141209_allow_null_old_email_on_email_change_requests.rb b/db/post_migrate/20200508141209_allow_null_old_email_on_email_change_requests.rb new file mode 100644 index 00000000000..b33b5c70585 --- /dev/null +++ b/db/post_migrate/20200508141209_allow_null_old_email_on_email_change_requests.rb @@ -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 diff --git a/lib/email_updater.rb b/lib/email_updater.rb index b3f4371c989..23f0a5c09bf 100644 --- a/lib/email_updater.rb +++ b/lib/email_updater.rb @@ -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 diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index e91acfe138a..d0b04702383 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -170,6 +170,7 @@ module SvgSprite "sign-in-alt", "sign-out-alt", "signal", + "star", "step-backward", "step-forward", "stream", diff --git a/spec/components/email_updater_spec.rb b/spec/components/email_updater_spec.rb index b2cae4d24d2..046e8b04beb 100644 --- a/spec/components/email_updater_spec.rb +++ b/spec/components/email_updater_spec.rb @@ -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 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index fed5b3c726f..e6295dcb182 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -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]) } diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb index 257a431caf4..92cd1570442 100644 --- a/spec/requests/users_email_controller_spec.rb +++ b/spec/requests/users_email_controller_spec.rb @@ -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' } diff --git a/spec/serializers/web_hook_user_serializer_spec.rb b/spec/serializers/web_hook_user_serializer_spec.rb index a019dff350a..0059639054f 100644 --- a/spec/serializers/web_hook_user_serializer_spec.rb +++ b/spec/serializers/web_hook_user_serializer_spec.rb @@ -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 ?