From b57e108e84547d540fc172053520a74f151b48b7 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 20 Nov 2019 18:31:25 +1100 Subject: [PATCH] FEATURE: improve email change workflow - Show old and new email address during the process - Ensure correct user is logged on when attempting to make email changes - Support reloading a page during the email reset process without resubmit of form - Improve tests - Fixed issue where redirect back to site was not linking correctly in subfolder setups Internal refactor of single action into 4 distinct actions that are simpler to reason about. This also removes the step that logs on an account after you confirm an email change, since it is no longer needed which leaves us with safer internals. This left me no choice but to amend translations cause the old route was removed. --- app/controllers/application_controller.rb | 39 ++-- app/controllers/users_email_controller.rb | 169 +++++++++++++++--- app/views/users_email/confirm.html.erb | 47 ----- .../show_confirm_new_email.html.erb | 51 ++++++ .../show_confirm_old_email.html.erb | 27 +++ config/locales/server.ar.yml | 4 +- config/locales/server.ca.yml | 4 +- config/locales/server.de.yml | 4 +- config/locales/server.el.yml | 4 +- config/locales/server.en.yml | 19 +- config/locales/server.es.yml | 4 +- config/locales/server.fa_IR.yml | 4 +- config/locales/server.fi.yml | 4 +- config/locales/server.fr.yml | 4 +- config/locales/server.he.yml | 4 +- config/locales/server.hy.yml | 2 +- config/locales/server.it.yml | 4 +- config/locales/server.ja.yml | 2 +- config/locales/server.pl_PL.yml | 4 +- config/locales/server.pt_BR.yml | 4 +- config/locales/server.ru.yml | 4 +- config/locales/server.sl.yml | 4 +- config/locales/server.sq.yml | 2 +- config/locales/server.uk.yml | 2 +- config/locales/server.ur.yml | 4 +- config/locales/server.zh_CN.yml | 4 +- config/locales/server.zh_TW.yml | 4 +- config/routes.rb | 9 +- spec/requests/users_email_controller_spec.rb | 157 ++++++++++------ 29 files changed, 409 insertions(+), 185 deletions(-) delete mode 100644 app/views/users_email/confirm.html.erb create mode 100644 app/views/users_email/show_confirm_new_email.html.erb create mode 100644 app/views/users_email/show_confirm_old_email.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d0924e76e35..592a80cdfc5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -687,6 +687,25 @@ class ApplicationController < ActionController::Base request.original_url unless request.original_url =~ /uploads/ end + def redirect_to_login + dont_cache_page + + if SiteSetting.enable_sso? + # save original URL in a session so we can redirect after login + session[:destination_url] = destination_url + redirect_to path('/session/sso') + elsif !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1 && !cookies[:authentication_data] + # Only one authentication provider, direct straight to it. + # If authentication_data is present, then we are halfway though registration. Don't redirect offsite + cookies[:destination_url] = destination_url + redirect_to path("/auth/#{Discourse.enabled_authenticators.first.name}") + else + # save original URL in a cookie (javascript redirects after login in this case) + cookies[:destination_url] = destination_url + redirect_to path("/login") + end + end + def redirect_to_login_if_required return if request.format.json? && is_api? @@ -715,24 +734,8 @@ class ApplicationController < ActionController::Base if !current_user && SiteSetting.login_required? flash.keep - dont_cache_page - - if SiteSetting.enable_sso? - # save original URL in a session so we can redirect after login - session[:destination_url] = destination_url - redirect_to path('/session/sso') - return - elsif !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1 && !cookies[:authentication_data] - # Only one authentication provider, direct straight to it. - # If authentication_data is present, then we are halfway though registration. Don't redirect offsite - cookies[:destination_url] = destination_url - redirect_to path("/auth/#{Discourse.enabled_authenticators.first.name}") - else - # save original URL in a cookie (javascript redirects after login in this case) - cookies[:destination_url] = destination_url - redirect_to path("/login") - return - end + redirect_to_login + return end check_totp = current_user && diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb index f74f60a94a2..aaead3bf624 100644 --- a/app/controllers/users_email_controller.rb +++ b/app/controllers/users_email_controller.rb @@ -4,8 +4,26 @@ class UsersEmailController < ApplicationController requires_login only: [:index, :update] - skip_before_action :check_xhr, only: [:confirm] - skip_before_action :redirect_to_login_if_required, only: [:confirm] + skip_before_action :check_xhr, only: [ + :confirm_old_email, + :show_confirm_old_email, + :confirm_new_email, + :show_confirm_new_email + ] + + skip_before_action :redirect_to_login_if_required, only: [ + :confirm_old_email, + :show_confirm_old_email, + :confirm_new_email, + :show_confirm_new_email + ] + + before_action :require_login, only: [ + :confirm_old_email, + :show_confirm_old_email, + :confirm_new_email, + :show_confirm_new_email + ] def index end @@ -29,38 +47,141 @@ class UsersEmailController < ApplicationController render_json_error(I18n.t("rate_limiter.slow_down")) end - def confirm - expires_now + def confirm_new_email + load_change_request(:new) - token = EmailToken.confirmable(params[:token]) - user = token&.user + if @change_request&.change_state != EmailChangeRequest.states[:authorizing_new] + @error = I18n.t("change_email.already_done") + end - change_request = - if user - user.email_change_requests.where(new_email_token_id: token.id).first - end + redirect_url = path("/u/confirm-new-email/#{params[:token]}") - if change_request&.change_state == EmailChangeRequest.states[:authorizing_new] && - user.totp_enabled? && !user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i) + if !@error && @user.totp_enabled? && !@user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i) + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + flash[:invalid_second_factor] = true + redirect_to redirect_url + return + end - @update_result = :invalid_second_factor - @backup_codes_enabled = true if user.backup_codes_enabled? - - if params[:second_factor_token].present? - RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! - @show_invalid_second_factor_error = true - end - else + if !@error updater = EmailUpdater.new - @update_result = updater.confirm(params[:token]) - - if @update_result == :complete + if updater.confirm(params[:token]) == :complete updater.user.user_stat.reset_bounce_score! - log_on_user(updater.user) + else + @error = I18n.t("change_email.already_done") end end + if @error + flash[:error] = @error + redirect_to redirect_url + else + redirect_to "#{redirect_url}?done=true" + end + end + + def show_confirm_new_email + load_change_request(:new) + + if params[:done].to_s == "true" + @done = true + end + + if @change_request&.change_state != EmailChangeRequest.states[:authorizing_new] + @error = I18n.t("change_email.already_done") + end + + @show_invalid_second_factor_error = flash[:invalid_second_factor] + + if !@error + if @user.totp_enabled? + @backup_codes_enabled = @user.backup_codes_enabled? + if params[:show_backup].to_s == "true" && @backup_codes_enabled + @show_backup_codes = true + else + @show_second_factor = true + end + end + + @to_email = @change_request.new_email + end + render layout: 'no_ember' end + def confirm_old_email + load_change_request(:old) + + if @change_request&.change_state != EmailChangeRequest.states[:authorizing_old] + @error = I18n.t("change_email.already_done") + end + + redirect_url = path("/u/confirm-old-email/#{params[:token]}") + + if !@error + updater = EmailUpdater.new + if updater.confirm(params[:token]) != :authorizing_new + @error = I18n.t("change_email.already_done") + end + end + + if @error + flash[:error] = @error + redirect_to redirect_url + else + redirect_to "#{redirect_url}?done=true" + end + end + + def show_confirm_old_email + load_change_request(:old) + + if @change_request&.change_state != EmailChangeRequest.states[:authorizing_old] + @error = I18n.t("change_email.already_done") + end + + if params[:done].to_s == "true" + @almost_done = true + end + + if !@error + @from_email = @user.email + @to_email = @change_request.new_email + end + + render layout: 'no_ember' + end + + private + + def load_change_request(type) + expires_now + + @token = EmailToken.confirmable(params[:token]) + + if @token + if type == :old + @change_request = @token.user&.email_change_requests.where(old_email_token_id: @token.id).first + elsif type == :new + @change_request = @token.user&.email_change_requests.where(new_email_token_id: @token.id).first + end + end + + @user = @token&.user + + if (!@user || !@change_request) + @error = I18n.t("change_email.already_done") + end + + if current_user.id != @user&.id + @error = I18n.t 'change_email.wrong_account_error' + end + end + + def require_login + if !current_user + redirect_to_login + end + end + end diff --git a/app/views/users_email/confirm.html.erb b/app/views/users_email/confirm.html.erb deleted file mode 100644 index 69d63eecce7..00000000000 --- a/app/views/users_email/confirm.html.erb +++ /dev/null @@ -1,47 +0,0 @@ -
- <% if @update_result == :authorizing_new %> -

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

-
-

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

- <% elsif @update_result == :complete %> -

<%= t 'change_email.confirmed' %>

-
- <%= t('change_email.please_continue', site_name: SiteSetting.title) %> - <% elsif @update_result == :invalid_second_factor%> - <% if !params[:show_backup] || params[:show_backup] == "false" %> -
-

<%= t('login.second_factor_title') %>

-
- <%=form_tag({}, method: :put) do %> - <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> -
<%= render 'common/second_factor_text_field' %>
- <% if @show_invalid_second_factor_error %> -
<%= t('login.invalid_second_factor_code') %>
- <% end %> - <%= submit_tag t('submit'), class: "btn btn-primary" %> - <% end %> -
- <% if @backup_codes_enabled %> - <%= link_to t("login.second_factor_toggle.backup_code"), show_backup: "true" %> - <% end %> - <% end %> - - <% if @backup_codes_enabled && params[:show_backup] == "true" %> -
-

<%= t('login.second_factor_backup_title') %>

-
- <%= form_tag({}, method: :put) do%> - <%= label_tag(:second_factor_token, t("login.second_factor_backup_description")) %> -
<%= render 'common/second_factor_backup_input' %>
- <%= submit_tag(t("submit"), class: "btn btn-primary") %> - <%end%> - -
- <%= link_to t("login.second_factor_toggle.totp"), show_backup: "false" %> - <%end%> - <% else %> -
- <%=t 'change_email.already_done' %> -
- <% end %> -
diff --git a/app/views/users_email/show_confirm_new_email.html.erb b/app/views/users_email/show_confirm_new_email.html.erb new file mode 100644 index 00000000000..fe14cf53df2 --- /dev/null +++ b/app/views/users_email/show_confirm_new_email.html.erb @@ -0,0 +1,51 @@ +
+ <% if @done %> +

+ <%= t 'change_email.confirmed' %> +

+

+ "><%= t('change_email.please_continue', site_name: SiteSetting.title) %> +

+ <% elsif @error %> +
+ <%= @error %> +
+ <% else %> +

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

+

+ <%= t 'change_email.authorizing_new.description' %> +

+

+ <%= @to_email %> +

+ + <%=form_tag(u_confirm_new_email_path, method: :put) do %> + <%= hidden_field_tag 'token', @token.token %> + + <% if @show_backup_codes %> +
+

<%= t('login.second_factor_backup_title') %>

+ <%= label_tag(:second_factor_token, t("login.second_factor_backup_description")) %> +
<%= render 'common/second_factor_backup_input' %>
+ <%= submit_tag(t("submit"), class: "btn btn-primary") %> +
+ <%= link_to t("login.second_factor_toggle.totp"), show_backup: "false" %> + <% elsif @show_second_factor %> +
+

<%= t('login.second_factor_title') %>

+ <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> +
<%= render 'common/second_factor_text_field' %>
+ <% if @show_invalid_second_factor_error %> +
<%= t('login.invalid_second_factor_code') %>
+ <% end %> + <%= submit_tag t('submit'), class: "btn btn-primary" %> +
+ <% if @backup_codes_enabled %> + <%= link_to t("login.second_factor_toggle.backup_code"), show_backup: "true" %> + <% end %> + <% else %> + <%= submit_tag t('change_email.confirm'), class: "btn btn-primary" %> + <% end %> + <%end%> + <% end%> +
diff --git a/app/views/users_email/show_confirm_old_email.html.erb b/app/views/users_email/show_confirm_old_email.html.erb new file mode 100644 index 00000000000..6d810931860 --- /dev/null +++ b/app/views/users_email/show_confirm_old_email.html.erb @@ -0,0 +1,27 @@ +
+ <% if @almost_done %> +

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

+

+ <%= t 'change_email.authorizing_old.almost_done_description' %> +

+ <% elsif @error %> +
+ <%= @error %> +
+ <% 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 %> +

+ + <%=form_tag(u_confirm_old_email_path, method: :put) do %> + <%= hidden_field_tag 'token', @token.token %> + <%= submit_tag t('change_email.confirm'), class: "btn btn-primary" %> + <% end %> + <% end %> +
diff --git a/config/locales/server.ar.yml b/config/locales/server.ar.yml index f2adf79e850..ac62eb1e1d0 100644 --- a/config/locales/server.ar.yml +++ b/config/locales/server.ar.yml @@ -1456,7 +1456,7 @@ ar: text_body_template: | أكد عنوان بريدك الإلكتروني لـ %{site_name} بالضغط على الرابط التالي : - %{email_token}/u/authorize-email/%{base_url} + %{email_token}/u/confirm-new-email/%{base_url} confirm_old_email: subject_template: "أكّد عنوان بريد الإلكتروني الحالي %{email_prefix}" text_body_template: | @@ -1466,7 +1466,7 @@ ar: أكّد عنوان بريدك الإلكتروني الحالي لـ %{site_name} بالضغط على الرابط التالي : - %{email_token}/u/authorize-email/%{base_url} + %{email_token}/u/confirm-old-email/%{base_url} notify_old_email: subject_template: "عنوان بريد الإلكتروني تم تغييرة %{email_prefix}" signup_after_approval: diff --git a/config/locales/server.ca.yml b/config/locales/server.ca.yml index 449d71503c1..b33c23a523a 100644 --- a/config/locales/server.ca.yml +++ b/config/locales/server.ca.yml @@ -2745,7 +2745,7 @@ ca: text_body_template: | Confirmeu la vostra nova adreça de correu per a %{site_name} fent clic en l'enllaç següent: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Confirmeu l'adreça de correu antiga" subject_template: "[%{email_prefix}] Confirmeu la vostra adreça de correu actual" @@ -2756,7 +2756,7 @@ ca: Confirmeu la vostra adreça actual per a %{site_name} fent clic en l'enllaç següent: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Notifica l'adreça de correu antiga" subject_template: "[%{email_prefix}] La vostra adreça de correu ha canviat" diff --git a/config/locales/server.de.yml b/config/locales/server.de.yml index b08f8f19742..13f4b63a4a5 100644 --- a/config/locales/server.de.yml +++ b/config/locales/server.de.yml @@ -3019,7 +3019,7 @@ de: text_body_template: | Bestätige deine neue E-Mail-Adresse für %{site_name}, indem du dem diesem Link folgst: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "E-Mail-Adresse bestätigen (an alte)" subject_template: "[%{email_prefix}] Bestätige deine aktuelle E-Mail-Adresse" @@ -3028,7 +3028,7 @@ de: Bestätige deine aktuelle E-Mail-Adresse für %{site_name}, indem du diesem Link folgst: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Benachrichtigung an alte E-Mail-Adresse" subject_template: "[%{email_prefix}] Deine E-Mail-Adresse wurde geändert" diff --git a/config/locales/server.el.yml b/config/locales/server.el.yml index 0e57b3621e2..7c18fa00693 100644 --- a/config/locales/server.el.yml +++ b/config/locales/server.el.yml @@ -2084,7 +2084,7 @@ el: Επικυρώστε την νέα σας διεύθυνση email στην %{site_name} κάνοντας κλικ στον παρακάτω σύνδεσμο: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Επιβεβαίωση παλιάς διεύθυνσης email" subject_template: "[%{email_prefix}] Επικυρώστε την νέα σας διεύθυνση email" @@ -2098,7 +2098,7 @@ el: Επιβεβαιώστε την τρέχουσα διεύθυνση email στην%{site_name} κάνοντας κλικ στον παρακάτω σύνδεσμο: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Ειδοποίηση παλιάς διεύθυνσης email" subject_template: "[%{email_prefix}] Η διεύθυνση email σας έχει αλλαχθεί" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index acfd572e8d0..c93fb0cb285 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -805,14 +805,25 @@ en: unknown: "unknown operating system" change_email: + wrong_account_error: "You are logged in the wrong account, please log out and try again." 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?" 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:" + authorizing_old: - title: "Thanks for confirming your current email address" - description: "We're now emailing your new address for confirmation." + title: "Change your email address" + description: "Please confirm your email address change" + old_email: "Old email: %{email}" + new_email: "New email: %{email}" + almost_done_title: "Confirming new email address" + almost_done_description: "We have sent an email to your new email address to confirm the change!" associated_accounts: revoke_failed: "Failed to revoke your account with %{provider_name}." @@ -3545,7 +3556,7 @@ en: text_body_template: | Confirm your new email address for %{site_name} by clicking on the following link: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Confirm Old Email" @@ -3557,7 +3568,7 @@ en: Confirm your current email address for %{site_name} by clicking on the following link: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Notify Old Email" diff --git a/config/locales/server.es.yml b/config/locales/server.es.yml index 2d8687676e0..7c2ed250332 100644 --- a/config/locales/server.es.yml +++ b/config/locales/server.es.yml @@ -3144,7 +3144,7 @@ es: text_body_template: | Confirma tu nueva dirección de correo electrónico para %{site_name} haciendo clic en el siguiente enlace: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Confirmar correo electrónico antiguo" subject_template: "[%{email_prefix}] Confirma tu dirección actual de correo electrónico" @@ -3155,7 +3155,7 @@ es: Confirma tu correo electrónico actual para %{site_name} haciendo clic en el siguiente enlace: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} notify_old_email: title: "Antiguo correo electrónico de notificaciones" subject_template: "[%{email_prefix}] Tu dirección de correo electrónico ha sido cambiada" diff --git a/config/locales/server.fa_IR.yml b/config/locales/server.fa_IR.yml index 4179077907c..354e22fed28 100644 --- a/config/locales/server.fa_IR.yml +++ b/config/locales/server.fa_IR.yml @@ -1896,7 +1896,7 @@ fa_IR: text_body_template: | ایمیل جدید خود را برای %{site_name} با کلیک روی لینک زیر تایید کنید: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "تایید ایمیل قبلی" subject_template: "[%{email_prefix}] ایمیل فعلی خود را تایید کنید" @@ -1907,7 +1907,7 @@ fa_IR: ایمیل فعلی خود در سایت %{site_name} را با کلیکل روی لینک زیر تعیید کنید: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "اعلام ایمیل قبلی" subject_template: "[%{email_prefix}] ایمیل شما تغییر کرده است" diff --git a/config/locales/server.fi.yml b/config/locales/server.fi.yml index 4b617760114..5d202e566d3 100644 --- a/config/locales/server.fi.yml +++ b/config/locales/server.fi.yml @@ -2879,7 +2879,7 @@ fi: text_body_template: | Vahvista uusi sähköpostiosoitteesi sivustolla %{site_name} klikkaamalla linkkiä: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Vahvista vanha sähköpostiosoite" subject_template: "[%{email_prefix}] Vahvista nykyinen sähköpostiosoitteesi" @@ -2888,7 +2888,7 @@ fi: Vahvista nykyinen sähköpostiosoitteesi sivustolla %{site_name} klikkaamalla linkkiä: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Ilmoita vanhaan sähköpostiosoitteeseen" subject_template: "[%{email_prefix}] Sähköpostiosoitteesi on vaihdettu" diff --git a/config/locales/server.fr.yml b/config/locales/server.fr.yml index aa6de0aa519..5d2e352f736 100644 --- a/config/locales/server.fr.yml +++ b/config/locales/server.fr.yml @@ -3009,7 +3009,7 @@ fr: text_body_template: | Confirmez votre nouvelle adresse email pour %{site_name} en cliquant sur le lien suivant : - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Confirmez votre ancienne adresse email" subject_template: "[%{email_prefix}] Confirmez votre adresse email actuelle" @@ -3018,7 +3018,7 @@ fr: Confirmez votre adresse email actuelle pour %{site_name} en cliquant sur le lien suivant : - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Notifier l'ancienne adresse courriel" subject_template: "[%{email_prefix}] Votre adresse email a été modifié" diff --git a/config/locales/server.he.yml b/config/locales/server.he.yml index 097f22db589..ac476139b73 100644 --- a/config/locales/server.he.yml +++ b/config/locales/server.he.yml @@ -3259,7 +3259,7 @@ he: text_body_template: | אשרו את כתובת המייל החדשה שלכם עבור %{site_name} על ידי לחיצה על הקישור הבא: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "אישור מייל ישן" subject_template: "[%{email_prefix}] אשרו את כתובת המייל הנוכחית שלכם" @@ -3269,7 +3269,7 @@ he: אשרו את כתובת המייל הנוכחית עבור %{site_name} על ידי לחיצה על הקישור הבא: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "התראת דוא״ל ישן" subject_template: "[%{email_prefix}] כתובת הדוא״ל שלך הוחלפה" diff --git a/config/locales/server.hy.yml b/config/locales/server.hy.yml index 91f214c20fa..30db6aa4d01 100644 --- a/config/locales/server.hy.yml +++ b/config/locales/server.hy.yml @@ -2701,7 +2701,7 @@ hy: text_body_template: | Հաստատեք Ձեր նոր էլ. հասցեն %{site_name} -ի համար՝ սեղմելով հետևյալ հղումը. - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Հաստատել Հին Էլ. Հասցեն" subject_template: "[%{email_prefix}] Հաստատել Ձեր ընթացիկ էլ. հասցեն" diff --git a/config/locales/server.it.yml b/config/locales/server.it.yml index 2c52e5446e3..faf3d8a1b36 100644 --- a/config/locales/server.it.yml +++ b/config/locales/server.it.yml @@ -2862,7 +2862,7 @@ it: text_body_template: | Conferma il tuo nuovo indirizzo email su %{site_name} cliccando il seguente collegamento: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Conferma Vecchia Email" subject_template: "[%{email_prefix}] Conferma il tuo attuale indirizzo email" @@ -2873,7 +2873,7 @@ it: Conferma il tuo attuale indirizzo email su %{site_name} cliccando il seguente collegamento: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Notifica Vecchia Email" subject_template: "[%{email_prefix}] Il tuo indirizzo email è stato cambiato" diff --git a/config/locales/server.ja.yml b/config/locales/server.ja.yml index 529d544a953..dc9ca9e287c 100644 --- a/config/locales/server.ja.yml +++ b/config/locales/server.ja.yml @@ -1300,7 +1300,7 @@ ja: text_body_template: | %{site_name}への新しいメールアドレスを下のリンクから確認してください。 - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "古いメールの確認" subject_template: "[%{email_prefix}]現在のメールアドレスの確認" diff --git a/config/locales/server.pl_PL.yml b/config/locales/server.pl_PL.yml index 75f2d442316..39c273adf25 100644 --- a/config/locales/server.pl_PL.yml +++ b/config/locales/server.pl_PL.yml @@ -2313,7 +2313,7 @@ pl_PL: text_body_template: | Potwierdź swój nowy adres email dla %{site_name} poprzez kliknięcia na następujący link: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Potwierdź stary email" subject_template: "[%{site_name}] Potwierdź aktualny adres email" @@ -2322,7 +2322,7 @@ pl_PL: Potwierdź obecny adres email dla %{site_name}poprzez naciśnięcie na następujący link: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Powiadom Stary Email" subject_template: "[%{site_name}] Twój adres email został zmieniony" diff --git a/config/locales/server.pt_BR.yml b/config/locales/server.pt_BR.yml index b2fe3bbc579..d2bfcc8549e 100644 --- a/config/locales/server.pt_BR.yml +++ b/config/locales/server.pt_BR.yml @@ -2925,11 +2925,11 @@ pt_BR: text_body_template: | Confirme seu novo endereço de e-mail para %{site_name} clicando no seguinte link: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Confirmar Antigo e-mail" subject_template: "[%{email_prefix}] Confirme seu endereço de e-mail atual" - text_body_template: "Antes que possamos alterar seu endereço de e-mail, precisamos que você confirme que você controla a conta de e-mail atual. \nDepois de concluir esta etapa, você terá que confirmar\no novo endereço de e-mail.\n\nConfirme seu endereço de e-mail atual para %{site_name} clicando no seguinte link:\n\n%{base_url}/u/authorize-email/%{email_token}\n" + text_body_template: "Antes que possamos alterar seu endereço de e-mail, precisamos que você confirme que você controla a conta de e-mail atual. \nDepois de concluir esta etapa, você terá que confirmar\no novo endereço de e-mail.\n\nConfirme seu endereço de e-mail atual para %{site_name} clicando no seguinte link:\n\n%{base_url}/u/confirm-old-email/%{email_token}\n" notify_old_email: title: "Notificar e-mail antigo" subject_template: "[%{email_prefix}] Seu endereço de e-mail foi alterado" diff --git a/config/locales/server.ru.yml b/config/locales/server.ru.yml index d4ee59168d3..ac71a206914 100644 --- a/config/locales/server.ru.yml +++ b/config/locales/server.ru.yml @@ -2217,7 +2217,7 @@ ru: text_body_template: | Подтвердите ваш новый адрес e-mail почты для %{site_name} нажав на следующую ссылку: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: text_body_template: | Прежде чем мы сможем изменить ваш адрес электронной почты, нам нужно, чтобы вы подтвердили, что вы контролируете @@ -2226,7 +2226,7 @@ ru: Подтвердите свой текущий адрес e-mail почты для %{site_name} нажав на следующую ссылку: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} signup_after_approval: title: "Регистрация После Утверждения" subject_template: "Ваша учетная запись на сайте %{site_name} одобрена!" diff --git a/config/locales/server.sl.yml b/config/locales/server.sl.yml index 6a5711a1419..fe4a8618d21 100644 --- a/config/locales/server.sl.yml +++ b/config/locales/server.sl.yml @@ -1659,7 +1659,7 @@ sl: text_body_template: | Potrdite vaš nov e-naslov pri %{site_name} tako da sledite povezavi: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Potrdite star e-naslov" subject_template: "[%{email_prefix}] Potrdite vaš trenutni e-naslov" @@ -1668,7 +1668,7 @@ sl: Potrdite vaš trenutni e-naslov pri %{site_name} tako da sledite povezavi: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "Obvesti stari e-naslov" subject_template: "[%{email_prefix}] Vaš e-naslov je bil spremenjen" diff --git a/config/locales/server.sq.yml b/config/locales/server.sq.yml index 6130e9b8ad5..f93a1297a0d 100644 --- a/config/locales/server.sq.yml +++ b/config/locales/server.sq.yml @@ -1163,7 +1163,7 @@ sq: text_body_template: | Konfirmoni adresën tuaj të re të emailit për "%{site_name}" duke klikuar linkun më poshtë: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: subject_template: "[%{email_prefix}] Konfirmoni adresën e tanishme të emailit " notify_old_email: diff --git a/config/locales/server.uk.yml b/config/locales/server.uk.yml index 756ae8b7293..12c4680b488 100644 --- a/config/locales/server.uk.yml +++ b/config/locales/server.uk.yml @@ -2434,7 +2434,7 @@ uk: text_body_template: | Підтвердіть свою нову електронну адресу для %{site_name} натиснувши наступне посилання: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "Підтвердіть стару електронну пошту" subject_template: "[%{email_prefix}] Підтвердіть свою поточну адресу електроної пошти" diff --git a/config/locales/server.ur.yml b/config/locales/server.ur.yml index 42da5aa0238..9036cb9ed66 100644 --- a/config/locales/server.ur.yml +++ b/config/locales/server.ur.yml @@ -3033,7 +3033,7 @@ ur: text_body_template: | %{site_name} پر اپنے نئے ای میل ایڈریس کی تصدیق کرنے کیلئے مندرجہ ذیل لِنک پر کلِک کریں: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "پرانی ای میل تصدیق" subject_template: "[%{email_prefix}] اپنا موجودہ ای میل ایڈریس تصدیق کریں" @@ -3044,7 +3044,7 @@ ur: %{site_name} پر اپنے موجودہ ای میل ایڈریس کی تصدیق کرنے کیلئے مندرجہ ذیل لِنک پر کلک کریں: - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "پرانا ای میل مطلع" subject_template: "[%{email_prefix}] آپ کا ای میل ایڈریس تبدیل ہوگیا ہے" diff --git a/config/locales/server.zh_CN.yml b/config/locales/server.zh_CN.yml index e7d2150d2a9..52c35fff8f7 100644 --- a/config/locales/server.zh_CN.yml +++ b/config/locales/server.zh_CN.yml @@ -3013,7 +3013,7 @@ zh_CN: text_body_template: | 点击下面的链接来确认你在%{site_name}上的新电子邮箱地址： - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "确认旧邮箱" subject_template: "[%{email_prefix}] 确认你现在的电子邮箱地址" @@ -3022,7 +3022,7 @@ zh_CN: 点击下面的链接来确认你在%{site_name}正使用的邮件： - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "通知旧邮箱" subject_template: "[%{email_prefix}] 你的邮箱已经修改成功" diff --git a/config/locales/server.zh_TW.yml b/config/locales/server.zh_TW.yml index e530144e023..27cada2b1c0 100644 --- a/config/locales/server.zh_TW.yml +++ b/config/locales/server.zh_TW.yml @@ -2810,7 +2810,7 @@ zh_TW: text_body_template: | 點擊以下連結，確認你 %{site_name} 的新郵件地址： - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-new-email/%{email_token} confirm_old_email: title: "確認原郵件地址" subject_template: "[%{email_prefix}] 確認你的現行郵件地址" @@ -2820,7 +2820,7 @@ zh_TW: 點擊下面的連結，以確認你當前在 %{site_name} 的郵件地址： - %{base_url}/u/authorize-email/%{email_token} + %{base_url}/u/confirm-old-email/%{email_token} notify_old_email: title: "通知原郵件地址" subject_template: "[%{email_prefix}] 已變更你的郵件地址" diff --git a/config/routes.rb b/config/routes.rb index eb1bab02127..7fd6e9cef81 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -408,8 +408,13 @@ Discourse::Application.routes.draw do put "#{root_path}/password-reset/:token" => "users#password_reset" get "#{root_path}/activate-account/:token" => "users#activate_account" put({ "#{root_path}/activate-account/:token" => "users#perform_account_activation" }.merge(index == 1 ? { as: 'perform_activate_account' } : {})) - get "#{root_path}/authorize-email/:token" => "users_email#confirm" - put "#{root_path}/authorize-email/:token" => "users_email#confirm" + + get "#{root_path}/confirm-old-email/:token" => "users_email#show_confirm_old_email" + put "#{root_path}/confirm-old-email" => "users_email#confirm_old_email" + + get "#{root_path}/confirm-new-email/:token" => "users_email#show_confirm_new_email" + put "#{root_path}/confirm-new-email" => "users_email#confirm_new_email" + get({ "#{root_path}/confirm-admin/:token" => "users#confirm_admin", constraints: { token: /[0-9a-f]+/ } diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb index d2512e08d5c..55bd65c62f5 100644 --- a/spec/requests/users_email_controller_spec.rb +++ b/spec/requests/users_email_controller_spec.rb @@ -4,72 +4,65 @@ require 'rails_helper' describe UsersEmailController do - describe '#confirm' do + fab!(:user) { Fabricate(:user) } + fab!(:moderator) { Fabricate(:moderator) } + + describe "#confirm-new-email" do + it 'redirects to login for signed out accounts' do + get "/u/confirm-new-email/asdfasdf" + + expect(response.status).to eq(302) + expect(response.redirect_url).to eq("http://test.localhost/login") + end + it 'errors out for invalid tokens' do - get "/u/authorize-email/asdfasdf" + sign_in(user) + + get "/u/confirm-new-email/asdfasdf" expect(response.status).to eq(200) expect(response.body).to include(I18n.t('change_email.already_done')) end - context 'valid old address token' do - fab!(:user) { Fabricate(:moderator) } - let(:updater) { EmailUpdater.new(user.guardian, user) } + it 'does not change email if accounts mismatch' do + updater = EmailUpdater.new(user.guardian, user) + updater.change_to('new.n.cool@example.com') - before do - updater.change_to('new.n.cool@example.com') - end + old_email = user.email - it 'confirms with a correct token' do - get "/u/authorize-email/#{user.email_tokens.last.token}" + sign_in(moderator) - expect(response.status).to eq(200) + put "/u/confirm-new-email", params: { + token: "#{user.email_tokens.last.token}" + } - body = CGI.unescapeHTML(response.body) - - expect(body) - .to include(I18n.t('change_email.authorizing_old.title')) - - expect(body) - .to include(I18n.t('change_email.authorizing_old.description')) - end + user.reload + expect(user.email).to eq(old_email) end - context 'valid new address token' do - fab!(:user) { Fabricate(:user) } + context "with a valid user" do let(:updater) { EmailUpdater.new(user.guardian, user) } before do + sign_in(user) updater.change_to('new.n.cool@example.com') end it 'confirms with a correct token' do user.user_stat.update_columns(bounce_score: 42, reset_bounce_score_after: 1.week.from_now) - events = DiscourseEvent.track_events do - get "/u/authorize-email/#{user.email_tokens.last.token}" - end + put "/u/confirm-new-email", params: { + token: "#{user.email_tokens.last.token}" + } - expect(events.map { |event| event[:event_name] }).to include( - :user_logged_in, :user_first_logged_in - ) - - expect(response.status).to eq(200) - expect(response.body).to include(I18n.t('change_email.confirmed')) + expect(response.status).to eq(302) + expect(response.redirect_url).to include("done") user.reload expect(user.user_stat.bounce_score).to eq(0) expect(user.user_stat.reset_bounce_score_after).to eq(nil) - end - - it 'automatically adds the user to a group when the email matches' do - group = Fabricate(:group, automatic_membership_email_domains: "example.com") - - get "/u/authorize-email/#{user.email_tokens.last.token}" - - expect(response.status).to eq(200) - expect(group.reload.users.include?(user)).to eq(true) + expect(user.email).to eq("new.n.cool@example.com") end context 'second factor required' do @@ -77,7 +70,7 @@ describe UsersEmailController do fab!(:backup_code) { Fabricate(:user_second_factor_backup, user: user) } it 'requires a second factor token' do - get "/u/authorize-email/#{user.email_tokens.last.token}" + get "/u/confirm-new-email/#{user.email_tokens.last.token}" expect(response.status).to eq(200) @@ -88,7 +81,7 @@ describe UsersEmailController do end it 'requires a backup token' do - get "/u/authorize-email/#{user.email_tokens.last.token}?show_backup=true" + get "/u/confirm-new-email/#{user.email_tokens.last.token}?show_backup=true" expect(response.status).to eq(200) @@ -98,34 +91,94 @@ describe UsersEmailController do end it 'adds an error on a second factor attempt' do - get "/u/authorize-email/#{user.email_tokens.last.token}", params: { + put "/u/confirm-new-email", params: { + token: user.email_tokens.last.token, second_factor_token: "000000", second_factor_method: UserSecondFactor.methods[:totp] } - expect(response.status).to eq(200) - expect(response.body).to include(I18n.t("login.invalid_second_factor_code")) + expect(response.status).to eq(302) + expect(flash[:invalid_second_factor]).to eq(true) end it 'confirms with a correct second token' do - get "/u/authorize-email/#{user.email_tokens.last.token}", params: { + put "/u/confirm-new-email", params: { second_factor_token: ROTP::TOTP.new(second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:totp] + second_factor_method: UserSecondFactor.methods[:totp], + token: user.email_tokens.last.token } - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = response.body - - expect(response_body).not_to include(I18n.t("login.second_factor_title")) - expect(response_body).not_to include(I18n.t("login.invalid_second_factor_code")) + user.reload + expect(user.email).to eq("new.n.cool@example.com") end end end end + describe '#confirm-old-email' do + + it 'redirects to login for signed out accounts' do + get "/u/confirm-old-email/asdfasdf" + + expect(response.status).to eq(302) + expect(response.redirect_url).to eq("http://test.localhost/login") + end + + it 'errors out for invalid tokens' do + sign_in(user) + + get "/u/confirm-old-email/asdfasdf" + + expect(response.status).to eq(200) + expect(response.body).to include(I18n.t('change_email.already_done')) + end + + it 'bans change when accounts do not match' do + + sign_in(user) + updater = EmailUpdater.new(moderator.guardian, moderator) + updater.change_to('new.n.cool@example.com') + + get "/u/confirm-old-email/#{moderator.email_tokens.last.token}" + + expect(response.status).to eq(200) + expect(body).to include("alert-error") + end + + context 'valid old address token' do + + it 'confirms with a correct token' do + # NOTE: only moderators need to confirm both old and new + sign_in(moderator) + updater = EmailUpdater.new(moderator.guardian, moderator) + updater.change_to('new.n.cool@example.com') + + get "/u/confirm-old-email/#{moderator.email_tokens.last.token}" + + expect(response.status).to eq(200) + + body = CGI.unescapeHTML(response.body) + + expect(body) + .to include(I18n.t('change_email.authorizing_old.title')) + + expect(body) + .to include(I18n.t('change_email.authorizing_old.description')) + + put "/u/confirm-old-email", params: { + token: moderator.email_tokens.last.token + } + + expect(response.status).to eq(302) + expect(response.redirect_url).to include("done=true") + + end + end + end + describe '#update' do - fab!(:user) { Fabricate(:user) } let(:new_email) { 'bubblegum@adventuretime.ooo' } it "requires you to be logged in" do