From 49b8433c564647b4f80e8d70dd838753ed87d835 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 25 Oct 2023 23:33:44 +0200 Subject: [PATCH] Fix confusing screen when visiting a confirmation link for an already-confirmed email (#27368) --- .../auth/confirmations_controller.rb | 8 +++++- app/views/auth/confirmations/new.html.haml | 28 +++++++++++++++---- app/views/auth/shared/_progress.html.haml | 6 ++-- config/locales/en.yml | 8 ++++++ spec/features/captcha_spec.rb | 8 ++++++ 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 667e8eb0635..05e4605f4e6 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -39,6 +39,12 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController show end + def redirect_to_app? + truthy_param?(:redirect_to_app) + end + + helper_method :redirect_to_app? + private def require_captcha_if_needed! @@ -82,7 +88,7 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController end def after_confirmation_path_for(_resource_name, user) - if user.created_by_application && truthy_param?(:redirect_to_app) + if user.created_by_application && redirect_to_app? user.created_by_application.confirmation_redirect_uri elsif user_signed_in? web_url('start') diff --git a/app/views/auth/confirmations/new.html.haml b/app/views/auth/confirmations/new.html.haml index a98257873ce..0cb82a1f86a 100644 --- a/app/views/auth/confirmations/new.html.haml +++ b/app/views/auth/confirmations/new.html.haml @@ -1,13 +1,29 @@ - content_for :page_title do = t('auth.resend_confirmation') -= simple_form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post }) do |f| - = render 'shared/error_messages', object: resource +- if resource.errors.of_kind?(:email, :already_confirmed) + .simple_form + = render 'auth/shared/progress', stage: resource.approved? ? 'completed' : 'confirmed' - .fields-group - = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label': t('simple_form.labels.defaults.email') }, readonly: current_user.present?, hint: current_user.present? && t('auth.confirmations.wrong_email_hint') + - if resource.approved? + %h1.title= t('auth.confirmations.welcome_title', name: resource.account.username) + %p.lead= t('auth.confirmations.registration_complete', domain: site_hostname) + - if resource.created_by_application && redirect_to_app? + - app = resource.created_by_application + %p.lead= t('auth.confirmations.redirect_to_app_html', app_name: app.name, clicking_this_link: link_to(t('auth.confirmations.clicking_this_link'), app.confirmation_redirect_uri)) + - else + %p.lead= t('auth.confirmations.proceed_to_login_html', login_link: link_to_login(t('auth.confirmations.login_link'))) + - else + %h1.title= t('auth.confirmations.awaiting_review_title') + %p.lead= t('auth.confirmations.awaiting_review', domain: site_hostname) +- else + = simple_form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post }) do |f| + = render 'shared/error_messages', object: resource - .actions - = f.button :button, t('auth.resend_confirmation'), type: :submit + .fields-group + = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label': t('simple_form.labels.defaults.email') }, readonly: current_user.present?, hint: current_user.present? && t('auth.confirmations.wrong_email_hint') + + .actions + = f.button :button, t('auth.resend_confirmation'), type: :submit .form-footer= render 'auth/shared/links' diff --git a/app/views/auth/shared/_progress.html.haml b/app/views/auth/shared/_progress.html.haml index 578f62fa9c7..8fb33854194 100644 --- a/app/views/auth/shared/_progress.html.haml +++ b/app/views/auth/shared/_progress.html.haml @@ -1,4 +1,4 @@ -- progress_index = { rules: 0, details: 1, confirm: 2 }[stage.to_sym] +- progress_index = { rules: 0, details: 1, confirm: 2, confirmed: 3, completed: 4 }[stage.to_sym] %ol.progress-tracker %li{ class: progress_index.positive? ? 'completed' : 'active' } @@ -20,6 +20,8 @@ .label= t('auth.progress.confirm') - if approved_registrations? %li.separator{ class: progress_index > 2 ? 'completed' : nil } - %li + %li{ class: [progress_index > 3 && 'completed', progress_index == 3 && 'active'] } .circle + - if progress_index > 3 + = check_icon .label= t('auth.progress.review') diff --git a/config/locales/en.yml b/config/locales/en.yml index e8e0f21e1c3..c298c47d351 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1041,6 +1041,14 @@ en: hint_html: Just one more thing! We need to confirm you're a human (this is so we can keep the spam out!). Solve the CAPTCHA below and click "Continue". title: Security check confirmations: + awaiting_review: Your e-mail address is confirmed! The %{domain} staff is now reviewing your registration. You will receive an e-mail if they approve your account! + awaiting_review_title: Your registration is being reviewed + clicking_this_link: clicking this link + login_link: log in + proceed_to_login_html: You can now proceed to %{login_link}. + redirect_to_app_html: You should have been redirected to the %{app_name} app. If that did not happen, try %{clicking_this_link} or manually return to the app. + registration_complete: Your registration on %{domain} is now complete! + welcome_title: Welcome, %{name}! wrong_email_hint: If that e-mail address is not correct, you can change it in account settings. delete_account: Delete account delete_account_html: If you wish to delete your account, you can proceed here. You will be asked for confirmation. diff --git a/spec/features/captcha_spec.rb b/spec/features/captcha_spec.rb index 6ccf066208f..a5c5a44aa62 100644 --- a/spec/features/captcha_spec.rb +++ b/spec/features/captcha_spec.rb @@ -30,6 +30,14 @@ describe 'email confirmation flow when captcha is enabled' do click_button I18n.t('challenge.confirm') expect(user.reload.confirmed?).to be true expect(page).to have_current_path(/\A#{client_app.confirmation_redirect_uri}/, url: true) + + # Browsers will generally reload the original page upon redirection + # to external handlers, so test this as well + visit "/auth/confirmation?confirmation_token=#{user.confirmation_token}&redirect_to_app=true" + + # It presents a page with a link to the app callback + expect(page).to have_content(I18n.t('auth.confirmations.registration_complete', domain: 'cb6e6126.ngrok.io')) + expect(page).to have_link(I18n.t('auth.confirmations.clicking_this_link'), href: client_app.confirmation_redirect_uri) end end end