From fb5968bf255b0b22277b31d8427f87e68005e251 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 1 Jul 2024 14:37:33 +0000 Subject: [PATCH 01/22] =?UTF-8?q?France=20connect=20particulier=20controll?= =?UTF-8?q?er=20:=20permet=20de=20g=C3=A9rer=20le=20cas=20ou=20le=20mail?= =?UTF-8?q?=20n'est=20pas=20connu=20de=20DS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kara Diaby --- .../france_connect/particulier_controller.rb | 76 +++++++++++++++++-- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index fe3c57dee..3eb3c494d 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -2,8 +2,9 @@ class FranceConnect::ParticulierController < ApplicationController before_action :redirect_to_login_if_fc_aborted, only: [:callback] - before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation] + before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation, :associate_user] before_action :securely_retrieve_fci_from_email_merge_token, only: [:mail_merge_with_existing_account] + before_action :set_user_by_confirmation_token, only: [:resend_confirmation, :post_resend_confirmation, :confirm_email, :connect_france_connect_particulier_redirect] def login if FranceConnectService.enabled? @@ -20,8 +21,9 @@ class FranceConnect::ParticulierController < ApplicationController preexisting_unlinked_user = User.find_by(email: sanitize(fci.email_france_connect)) if preexisting_unlinked_user.nil? - fci.associate_user!(fci.email_france_connect) - connect_france_connect_particulier(fci.user) + merge_token = fci.create_merge_token! + render :choose_email, locals: { france_connect_email: fci.email_france_connect, merge_token: } + elsif !preexisting_unlinked_user.can_france_connect? fci.destroy redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) @@ -35,7 +37,7 @@ class FranceConnect::ParticulierController < ApplicationController if user.can_france_connect? fci.update(updated_at: Time.zone.now) connect_france_connect_particulier(user) - else # same behaviour as redirect nicely with message when instructeur/administrateur + else fci.destroy redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) end @@ -46,6 +48,18 @@ class FranceConnect::ParticulierController < ApplicationController redirect_france_connect_error_connection end + def associate_user + email = use_fc_email? ? @fci.email_france_connect : params[:alternative_email] + @fci.associate_user!(email) + user = @fci.user + + @fci.delete_merge_token! + sign_only(user) + + destination_path = destination_path(user) + render :confirmation_sent, locals: { email:, destination_path: } + end + def merge end @@ -75,7 +89,7 @@ class FranceConnect::ParticulierController < ApplicationController if user.can_france_connect? @fci.update(user: user) @fci.delete_merge_token! - + user.update(email_verified_at: Time.zone.now) flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) connect_france_connect_particulier(user) else # same behaviour as redirect nicely with message when instructeur/administrateur @@ -90,8 +104,8 @@ class FranceConnect::ParticulierController < ApplicationController if user.nil? @fci.associate_user!(sanitized_email_params) @fci.delete_merge_token! - - flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) + @fci.send_custom_confirmation_instructions(@fci.user) + flash.notice = t('france_connect.particulier.flash.connection_done_verify_email', application_name: Current.application_name) connect_france_connect_particulier(@fci.user) else @email = sanitized_email_params @@ -113,8 +127,56 @@ class FranceConnect::ParticulierController < ApplicationController notice: t('france_connect.particulier.flash.confirmation_mail_sent') end + def confirm_email + if @user.confirmation_sent_at && 2.days.ago < @user.confirmation_sent_at + @user.update(email_verified_at: Time.zone.now, confirmation_token: nil) + @user.after_confirmation + redirect_to stored_location_for(@user) || root_path(@user), notice: 'Votre email est bien vérifié' + else + redirect_to france_connect_resend_confirmation_path(token: @user.confirmation_token), alert: "Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé." + end + end + + def resend_confirmation + if @user.email_verified_at + redirect_to root_path, alert: 'Email déjà confirmé.' + end + end + + def post_resend_confirmation + if !@user.email_verified_at + fci = FranceConnectInformation.find_by(user: @user) + fci.send_custom_confirmation_instructions(@user) + redirect_to root_path(@user), notice: "Un nouveau lien de confirmation vous a été envoyé par mail." + else + redirect_to root_path(@user), alert: "Adresse email non trouvée ou déjà confirmée." + end + end + private + def set_user_by_confirmation_token + @user = User.find_by(confirmation_token: params[:token]) + + if @user.nil? + redirect_to root_path, alert: 'Utilisateur non trouvé' and return + end + + if user_signed_in? && current_user != @user + sign_out current_user + redirect_to new_user_session_path, alert: "Veuillez vous connecter avec le compte associé à ce lien de confirmation." + end + end + + def use_fc_email? = cast_bool(params[:use_france_connect_email]) + + def sign_only(user) + sign_out(user) if user_signed_in? + sign_in(user) + end + + def destination_path(user) = stored_location_for(user) || root_path(user) + def securely_retrieve_fci_from_email_merge_token @fci = FranceConnectInformation.find_by(email_merge_token: email_merge_token_params) From c51cf7e04896c857f63c0cc4ae2c95de5197f88a Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 1 Jul 2024 14:38:47 +0000 Subject: [PATCH 02/22] Layout : ajoute les vues pour confirmer le mail fc ou en choisir un autre --- .../email_france_connect_controller.ts | 32 +++++++++++++++++ .../particulier/choose_email.html.haml | 34 +++++++++++++++++++ .../particulier/confirmation_sent.html.haml | 8 +++++ .../particulier/merge.html.haml | 27 ++++++++------- .../particulier/resend_confirmation.html.haml | 11 ++++++ ...custom_confirmation_instructions.html.haml | 22 ++++++++++++ 6 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 app/javascript/controllers/email_france_connect_controller.ts create mode 100644 app/views/france_connect/particulier/choose_email.html.haml create mode 100644 app/views/france_connect/particulier/confirmation_sent.html.haml create mode 100644 app/views/france_connect/particulier/resend_confirmation.html.haml create mode 100644 app/views/user_mailer/custom_confirmation_instructions.html.haml diff --git a/app/javascript/controllers/email_france_connect_controller.ts b/app/javascript/controllers/email_france_connect_controller.ts new file mode 100644 index 000000000..6512c4982 --- /dev/null +++ b/app/javascript/controllers/email_france_connect_controller.ts @@ -0,0 +1,32 @@ +import { ApplicationController } from './application_controller'; + +export class EmailFranceConnectController extends ApplicationController { + static targets = ['useFranceConnectEmail', 'emailField']; + + emailFieldTarget!: HTMLInputElement; + useFranceConnectEmailTargets!: HTMLInputElement[]; + + connect() { + this.triggerEmailField(); + } + + triggerEmailField() { + const checkedTarget = this.useFranceConnectEmailTargets.find( + (target) => target.checked + ); + + const inputElement = this.emailFieldTarget.querySelector( + 'input' + ) as HTMLInputElement; + + if (checkedTarget && checkedTarget.value === 'false') { + this.emailFieldTarget.classList.remove('fr-hidden'); + inputElement.setAttribute('required', ''); + this.emailFieldTarget.required = true; + } else { + this.emailFieldTarget.classList.add('fr-hidden'); + inputElement.removeAttribute('required'); + inputElement.value = ''; + } + } +} diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml new file mode 100644 index 000000000..f72f970dc --- /dev/null +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -0,0 +1,34 @@ +.fr-container + %h1.text-center.mt-1= t('.choose_email_contact') + + %p= t('.greetings') + + %p= t('.intro_html', email: france_connect_email) + + %p= t('.use_email_for_notifications') + + = form_with url: france_connect_particulier_associate_user_path, method: :post, data: { controller: "email-france-connect" } do |f| + = hidden_field_tag :france_connect_email, france_connect_email + = hidden_field_tag :merge_token, merge_token + + %fieldset.fr-fieldset + %legend.fr-fieldset__legend + .fr-fieldset__element.fr-fieldset__element--inline + .fr-radio-group + = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } + %label.fr-label{ for: 'use_france_connect_email_yes' } + = t('.yes') + .fr-fieldset__element.fr-fieldset__element--inline + .fr-radio-group + = f.radio_button :use_france_connect_email, false, id: 'use_france_connect_email_no', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } + %label.fr-label{ for: 'use_france_connect_email_no' } + = t('.no') + + .fr-fieldset.fr-w-30v + .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField" } } + = f.label :alternative_email, t('.alternative_email'), class: "fr-label" + %span.fr-hint-text.mb-1= t('activerecord.attributes.user.hints.email') + = f.email_field :alternative_email, class: "fr-input" + + %div + = f.submit t('.confirm'), class: 'fr-btn' diff --git a/app/views/france_connect/particulier/confirmation_sent.html.haml b/app/views/france_connect/particulier/confirmation_sent.html.haml new file mode 100644 index 000000000..d167d2b01 --- /dev/null +++ b/app/views/france_connect/particulier/confirmation_sent.html.haml @@ -0,0 +1,8 @@ +.fr-container + %h1.text-center.mt-1= t('.confirmation_sent_by_email') + + %p= t('.intro', email: email) + + %p= t('.click_the_link_in_the_email') + + = link_to t('.continue'), destination_path, class: 'fr-btn' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index 0d7b98f3a..b7dcd4b40 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -1,20 +1,21 @@ = content_for :title, "Fusion des comptes FC et #{Current.application_name}" -.container +.fr-container %h1.page-title= t('.title', application_name: Current.application_name) %p= t('.subtitle_html', email: @fci.email_france_connect, application_name: Current.application_name) - .form.mt-2 - %label= t('.label_select_merge_flow', email: @fci.email_france_connect) - %fieldset.radios - %label{ onclick: "DS.showFusion(event);" } - = radio_button_tag :value, true, false, autocomplete: "off", id: 'it-is-mine' - = t('utils.yes') + %fieldset.fr-fieldset + %legend.fr-fieldset__legend= t('.label_select_merge_flow', email: @fci.email_france_connect) + .fr-fieldset__element.fr-fieldset__element--inline + .fr-radio-group + %input{ type: 'radio', id: 'it-is-mine', name: 'value', value: 'true', autocomplete: "off", onclick: "DS.showFusion(event);" } + %label{ for: 'it-is-mine' }= t('utils.yes') + .fr-fieldset__element.fr-fieldset__element--inline + .fr-radio-group + %input{ type: 'radio', id: 'it-is-not-mine', name: 'value', value: 'false', autocomplete: "off", onclick: "DS.showNewAccount(event);" } + %label{ for: 'it-is-not-mine' }= t('utils.no') - %label{ onclick: "DS.showNewAccount(event);" } - = radio_button_tag :value, false, false, autocomplete: "off", id: 'it-is-not-mine' - = t('utils.no') .fusion.hidden %p= t('.title_fill_in_password') @@ -38,9 +39,9 @@ = form_tag france_connect_particulier_merge_with_new_account_path, data: { turbo: true }, class: 'mt-2 form' do = hidden_field_tag :merge_token, @fci.merge_token, id: dom_id(@fci, :new_account_merge_token) - = label_tag :email, t('views.registrations.new.email_label'), for: dom_id(@fci, :new_account_email) - = email_field_tag :email, "", required: true, id: dom_id(@fci, :new_account_email) - = submit_tag t('.button_use_this_email'), class: 'button primary' + = label_tag :email, t('views.registrations.new.email_label'), for: dom_id(@fci, :new_account_email), class: 'fr-label' + = email_field_tag :email, "", required: true, id: dom_id(@fci, :new_account_email), class: 'mb-1 fr-input' + = submit_tag t('.button_use_this_email'), class: 'fr-btn' #new-account-password-confirmation.hidden diff --git a/app/views/france_connect/particulier/resend_confirmation.html.haml b/app/views/france_connect/particulier/resend_confirmation.html.haml new file mode 100644 index 000000000..bc03ca5c7 --- /dev/null +++ b/app/views/france_connect/particulier/resend_confirmation.html.haml @@ -0,0 +1,11 @@ +.fr-container + %h1.text-center.mt-1 Renvoyer le lien de confirmation + + %p Bonjour #{@user.email} + + %p Cliquez sur le bouton ci-dessous pour recevoir un nouveau lien de confirmation à votre adresse email. + + = form_with url: france_connect_post_resend_confirmation_path, method: :post, local: true do |form| + = form.hidden_field :token, value: @user.confirmation_token + .form-group + = form.submit "Renvoyer le lien de confirmation", class: 'fr-btn' diff --git a/app/views/user_mailer/custom_confirmation_instructions.html.haml b/app/views/user_mailer/custom_confirmation_instructions.html.haml new file mode 100644 index 000000000..c9ac6a792 --- /dev/null +++ b/app/views/user_mailer/custom_confirmation_instructions.html.haml @@ -0,0 +1,22 @@ +- content_for(:title, 'Confirmez votre email') +%p + Bonjour + = @user.email + ! + +%p + Veuillez confirmer votre email en cliquant sur le lien ci-dessous: + = round_button 'Je confirme', france_connect_confirm_email_url(@token), :primary + + +%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::CONFIRMATION_EMAIL_VALIDITY)}. + +%p + Tant que vous n'aurez pas confirmé votre email, vous ne recevrez aucune notification sur l'avancement de vos dossiers. + +%p + Si vous n’êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à + = succeed '.' do + = mail_to CONTACT_EMAIL + += render partial: "layouts/mailers/signature" From f531ba65f29cd6f5aecf5be0a899bd45dc9bd755 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 1 Jul 2024 14:39:23 +0000 Subject: [PATCH 03/22] locales --- config/locales/en.yml | 17 +++++++++++++++++ config/locales/fr.yml | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index d3570abaf..a73a0b449 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -150,6 +150,8 @@ en: subtitle_two: "Additional notes" content_html: "

The documentation pages are managed by a third-party tool. They are not fully accessible.

FAQ management was delegated to a third-party tool. It was reintegrated into the platform in May 2024 and has not yet been audited.

" + + preparation: title: "Preparation of this accessibility declaration" intro: "This declaration was drawn up on 27 April 2022. It was updated on 14 June 2024." @@ -879,6 +881,20 @@ en: to_follow: to follow france_connect: particulier: + choose_email: + intro_html: "Your FranceConnect account uses %{email}r as the contact email." + use_email_for_notifications: "Would you like to use it to receive notifications regarding the progress of your cases?" + 'yes': "Yes" + 'no': "No" + confirm: "Confirm" + choose_email_contact: "Choose your contact email" + alternative_email: "Please provide the email to use for contacting you." + greetings: "Hello," + confirmation_sent: + confirmation_sent_by_email: "Confirmation sent by email" + intro: "A confirmation email has been sent to your address %{email}" + click_the_link_in_the_email: "Please click the link in the email to confirm your account and connect with France Connect in the future." + continue: "Continue" password_confirmation: back: 'back to previous step' already_exists: An account with %{email} already existis on %{application_name} @@ -896,6 +912,7 @@ en: confirmation_mail_sent: "An email with the confirmation link has been sent, please click on the link." invalid_password: "The password is not correct." connection_done: "The accounts for FranceConnect and %{application_name} are now merged." + connection_done_verify_email: "The FranceConnect and %{application_name} accounts are now merged. Please confirm your email by clicking on the link you received in your email." merger_token_expired: "Le delay to merge your FranceConnect and %{application_name} accounts is expired. Please retry." shared: procedures: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index a6e214613..f209df748 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -935,6 +935,20 @@ fr: ministeres: Ministères france_connect: particulier: + choose_email: + intro_html: "Votre compte FranceConnect utilise %{email} comme email de contact." + use_email_for_notifications: Souhaitez-vous l'utiliser pour recevoir les notifications concernant l'avancement de vos dossiers ? + 'yes': Oui + 'no': Non + confirm: Confirmer + choose_email_contact: Choisissez votre e-mail de contact + alternative_email: Veuillez nous fournir l'email à utiliser pour vous contacter. + greetings: Bonjour, + confirmation_sent: + confirmation_sent_by_email: Confirmation envoyée par mail + intro: Un mail de confirmation a été envoyé à votre adresse %{email} + click_the_link_in_the_email: Vous devez impérativement cliquer sur le lien du mail pour activer votre adresse et recevoir les notifications sur l'avancement de vos dossiers. + continue: Continuer password_confirmation: back: 'revenir en arrière' already_exists: Le compte %{email} existe déjà sur %{application_name} @@ -952,6 +966,7 @@ fr: confirmation_mail_sent: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" invalid_password: "Mauvais mot de passe" connection_done: "Les comptes FranceConnect et %{application_name} sont à présent fusionnés" + connection_done_verify_email: "Les comptes FranceConnect et %{application_name} sont maintenant fusionnés. Veuillez confirmer votre e-mail en cliquant sur le lien que vous avez reçu par mail" merger_token_expired: "Le délai pour fusionner les comptes FranceConnect et %{application_name} est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes." groupe_gestionnaires: flash: From 9c41c9a08f957763243e6656fab2a286acf50f1a Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 1 Jul 2024 14:41:02 +0000 Subject: [PATCH 04/22] Model FC : au lieu de valider le mail on envoie une demande de confirmation par mail --- app/models/france_connect_information.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index e44324e38..63cb341dd 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -2,6 +2,7 @@ class FranceConnectInformation < ApplicationRecord MERGE_VALIDITY = 15.minutes + CONFIRMATION_EMAIL_VALIDITY = 2.days belongs_to :user, optional: true @@ -14,9 +15,9 @@ class FranceConnectInformation < ApplicationRecord password: Devise.friendly_token[0, 20], confirmed_at: Time.zone.now ) - user.after_confirmation + send_custom_confirmation_instructions(user) rescue ActiveRecord::RecordNotUnique - # ignore this exception because we check before is user is nil. + # ignore this exception because we check before if user is nil. # exception can be raised in race conditions, when FranceConnect calls callback 2 times. # At the 2nd call, user is nil but exception is raised at the creation of the user # because the first call has already created a user @@ -26,6 +27,12 @@ class FranceConnectInformation < ApplicationRecord touch # needed to update updated_at column end + def send_custom_confirmation_instructions(user) + token = SecureRandom.hex(10) + user.update!(confirmation_token: token, confirmation_sent_at: Time.zone.now) + UserMailer.custom_confirmation_instructions(user, token).deliver_later + end + def create_merge_token! merge_token = SecureRandom.uuid update(merge_token:, merge_token_created_at: Time.zone.now) From 6a9c489c7f7d7d815d26c8b65c8c3de51e6ab548 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 1 Jul 2024 14:41:13 +0000 Subject: [PATCH 05/22] routes --- config/routes.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 06aecc577..70495e4a7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -190,9 +190,13 @@ Rails.application.routes.draw do get 'particulier/callback' => 'particulier#callback' get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge get 'particulier/mail_merge_with_existing_account/:email_merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account + get 'confirm_email/:token', to: 'particulier#confirm_email', as: :confirm_email + get 'resend_confirmation', to: 'particulier#resend_confirmation', as: 'resend_confirmation' + post 'post_resend_confirmation', to: 'particulier#post_resend_confirmation', as: 'post_resend_confirmation' post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account' post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account' + post 'particulier/associate_user', to: 'particulier#associate_user', as: :particulier_associate_user end namespace :agent_connect do From d6defce1370c09c628ad3518e1d2359bf7f66bce Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Tue, 2 Jul 2024 12:05:06 +0000 Subject: [PATCH 06/22] =?UTF-8?q?Ajoute=20un=20mailer=20pour=20l'envoi=20d?= =?UTF-8?q?u=20lien=20de=20confirmation=20de=20mail=20customis=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/mailers/user_mailer.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 3b1be799e..9a0a3afe1 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -36,6 +36,12 @@ class UserMailer < ApplicationMailer mail(to: email, subject: @subject) end + def custom_confirmation_instructions(user, token) + @user = user + @token = token + mail(to: @user.email, subject: 'Confirmez votre email') + end + def invite_instructeur(user, reset_password_token) @reset_password_token = reset_password_token @user = user @@ -139,7 +145,8 @@ class UserMailer < ApplicationMailer 'france_connect_merge_confirmation', "new_account_warning", "ask_for_merge", - "invite_instructeur" + "invite_instructeur", + "custom_confirmation_instructions" ].include?(action_name) end end From a4b8d816de3beb13f284157c82f3587831f9bc9e Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Thu, 4 Jul 2024 10:47:53 +0000 Subject: [PATCH 07/22] Tests --- .../particulier_controller_spec.rb | 335 ++++++++++++++++-- spec/mailers/user_mailer_spec.rb | 25 ++ .../models/france_connect_information_spec.rb | 69 +++- spec/models/user_spec.rb | 6 - .../france_connect_particulier_spec.rb | 36 +- spec/system/users/dossier_prefill_get_spec.rb | 10 +- .../system/users/dossier_prefill_post_spec.rb | 8 +- 7 files changed, 439 insertions(+), 50 deletions(-) diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 4d56f7da0..6fe560588 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -89,23 +89,9 @@ describe FranceConnect::ParticulierController, type: :controller do let(:fc_user) { nil } context 'and no user with the same email exists' do - it 'creates an user with the same email and log in' do - expect { subject }.to change { User.count }.by(1) - - user = User.last - - expect(user.email).to eq(email.downcase) - expect(controller.current_user).to eq(user) - expect(response).to redirect_to(root_path) - end - - context 'when invites are pending' do - let!(:invite) { create(:invite, email: email, user: nil) } - it 'links pending invites' do - expect(invite.reload.user).to eq(nil) - subject - expect(invite.reload.user).to eq(User.last) - end + it 'render the choose email template to select good email' do + expect { subject }.to change { User.count }.by(0) + expect(subject).to render_template(:choose_email) end end @@ -134,15 +120,7 @@ describe FranceConnect::ParticulierController, type: :controller do context 'when france_connect_particulier_id does not exist in database' do it { expect { subject }.to change { FranceConnectInformation.count }.by(1) } - describe 'FranceConnectInformation attributs' do - let(:stored_fci) { FranceConnectInformation.last } - - before { subject } - - it { expect(stored_fci).to have_attributes(user_info.merge(birthdate: Time.zone.parse(birthdate).to_datetime)) } - end - - it { is_expected.to redirect_to(root_path) } + it { is_expected.to render_template(:choose_email) } end end @@ -158,6 +136,311 @@ describe FranceConnect::ParticulierController, type: :controller do end end + describe '#associate_user' do + subject { post :associate_user, params: { use_france_connect_email: use_france_connect_email, alternative_email: alternative_email, merge_token: merge_token } } + + let(:fci) { FranceConnectInformation.new(user_info) } + let(:use_france_connect_email) { true } + let(:alternative_email) { 'alt@example.com' } + let(:merge_token) { 'valid_merge_token' } + + before do + allow_any_instance_of(ApplicationController).to receive(:session).and_return({ merge_token: merge_token }) + end + + context 'when we are using france connect email' do + let(:fci) { instance_double('FranceConnectInformation') } + let(:email) { 'fc_email@example.com' } + let(:user) { instance_double('User') } + let(:destination_path) { '/some_path' } + let(:merge_token) { 'some_token' } + + before do + allow(controller).to receive(:securely_retrieve_fci).and_return(fci) + controller.instance_variable_set(:@fci, fci) + allow(fci).to receive(:email_france_connect).and_return(email) + allow(fci).to receive(:associate_user!) + allow(fci).to receive(:user).and_return(user) + allow(fci).to receive(:delete_merge_token!) + allow(controller).to receive(:use_fc_email?).and_return(true) + allow(controller).to receive(:sign_only) + allow(controller).to receive(:destination_path).and_return(destination_path) + end + + subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } } + + it 'renders the confirmation_sent template' do + subject + expect(response).to render_template(:confirmation_sent) + end + + it 'performs all expected steps' do + expect(fci).to receive(:associate_user!).with(email) + expect(fci).to receive(:delete_merge_token!) + expect(controller).to receive(:sign_only).with(user) + expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path }) + + subject + end + end + + context 'when france connect information is missing or invalid' do + let(:merge_token) { 'invalid_token' } + + before do + allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(nil) + allow(controller).to receive(:merge_token_params).and_return(merge_token) + end + + it 'redirects to root_path with an alert' do + subject + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq("Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes.") + end + end + + context 'when @fci is not valid for merge' do + before do + allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) + allow(fci).to receive(:valid_for_merge?).and_return(false) + end + + it 'redirects to root_path with an alert' do + subject + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes.') + end + end + + context 'when associating the user succeeds' do + let(:fci) { instance_double('FranceConnectInformation') } + let(:email) { 'user@example.com' } + let(:user) { instance_double('User', id: 1) } + let(:destination_path) { '/' } + + before do + allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) + allow(fci).to receive(:valid_for_merge?).and_return(true) + allow(fci).to receive(:email_france_connect).and_return(email) + allow(fci).to receive(:associate_user!) + allow(fci).to receive(:user).and_return(user) + allow(fci).to receive(:delete_merge_token!) + allow(controller).to receive(:use_fc_email?).and_return(true) + allow(controller).to receive(:sign_only) + allow(controller).to receive(:destination_path).and_return(destination_path) + end + + it 'calls associate_user! with the correct email' do + expect(fci).to receive(:associate_user!).with(email) + subject + end + + it 'deletes the merge token' do + expect(fci).to receive(:delete_merge_token!) + subject + end + + it 'signs in the user' do + expect(controller).to receive(:sign_only).with(user) + subject + end + + it 'renders the confirmation sent template with correct locals' do + expect(controller).to receive(:render).with( + :confirmation_sent, + locals: { email: email, destination_path: destination_path } + ) + subject + end + end + end + + describe '#confirm_email' do + let!(:user) { create(:user) } + let!(:fci) { create(:france_connect_information, user: user) } + + before do + sign_in(user) + fci.send_custom_confirmation_instructions(user) + user.reload + end + + let!(:expired_user_confirmation) do + user = create(:user) + fci = create(:france_connect_information, user: user) + token = SecureRandom.hex(10) + user.update!(confirmation_token: token, confirmation_sent_at: 3.days.ago) + user + end + + context 'when the confirmation token is valid' do + before do + get :confirm_email, params: { token: user.confirmation_token } + user.reload + end + + it 'updates the email_verified_at and confirmation_token of the user' do + expect(user.email_verified_at).to be_present + expect(user.confirmation_token).to be_nil + end + + it 'redirects to the stored location or root path with a success notice' do + expect(response).to redirect_to(root_path(user)) + expect(flash[:notice]).to eq('Votre email est bien vérifié') + end + + it 'calls after_confirmation on the user' do + expect(user).to receive(:after_confirmation).and_call_original + user.after_confirmation + end + end + + context 'when invites are pending' do + let!(:invite) { create(:invite, email: user.email, user: nil) } + + it 'links pending invites' do + get :confirm_email, params: { token: user.confirmation_token } + invite.reload + expect(invite.user).to eq(user) + end + end + + context 'when the confirmation token is expired' do + before do + sign_in(expired_user_confirmation) + get :confirm_email, params: { token: expired_user_confirmation.confirmation_token } + end + + it 'redirects to the resend confirmation path with an alert' do + expect(response).to redirect_to(france_connect_resend_confirmation_path(token: expired_user_confirmation.confirmation_token)) + expect(flash[:alert]).to eq('Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé.') + end + end + + context 'GET #resend_confirmation' do + let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } + + context 'when the token is valid' do + it 'assigns @user and renders the form' do + get :resend_confirmation, params: { token: user.confirmation_token } + expect(assigns(:user)).to eq(user) + expect(response).to render_template(:resend_confirmation) + end + end + + context 'when the email is already confirmed' do + let(:confirmed_user) { create(:user, email: 'confirmed@example.com', email_verified_at: Time.zone.now, confirmation_token: nil) } + + before do + allow(controller).to receive(:set_user_by_confirmation_token) do + controller.instance_variable_set(:@user, confirmed_user) + end + end + + it 'redirects to root path with an alert about already confirmed email' do + get :resend_confirmation, params: { email: confirmed_user.email } + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Email déjà confirmé.') + end + end + + context 'when the token is invalid' do + before do + get :resend_confirmation, params: { token: 'invalid_token' } + end + + it 'sets @user to nil and redirects to root path with an alert' do + expect(assigns(:user)).to be_nil + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Utilisateur non trouvé') + end + end + + context 'when the user is not found' do + before do + allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil) + end + + it 'redirects to root_path with an alert' do + get :resend_confirmation, params: { token: 'invalid_token' } + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Utilisateur non trouvé') + end + end + + context 'when a different user is signed in' do + let(:current_user) { create(:user) } + let(:target_user) { create(:user, confirmation_token: 'valid_token') } + + before do + sign_in current_user + allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(target_user) + end + + it 'signs out the current user and redirects to new_user_session_path with an alert' do + get :resend_confirmation, params: { token: 'valid_token' } + expect(controller.current_user).to be_nil + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq("Veuillez vous connecter avec le compte associé à ce lien de confirmation.") + end + end + end + + context 'POST #post_resend_confirmation' do + let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } + let(:fci) { create(:france_connect_information, user: user) } + + before do + allow(FranceConnectInformation).to receive(:find_by).with(user: user).and_return(fci) + allow(controller).to receive(:set_user_by_confirmation_token).and_call_original + end + + context 'when the user exists and email is not verified' do + before do + allow(controller).to receive(:set_user_by_confirmation_token) do + controller.instance_variable_set(:@user, user) + end + end + + it 'sends custom confirmation instructions and redirects with notice' do + expect(fci).to receive(:send_custom_confirmation_instructions).with(user) + post :post_resend_confirmation, params: { token: user.confirmation_token } + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Un nouveau lien de confirmation vous a été envoyé par mail.') + end + end + + context 'when the user does not exist' do + before do + allow(User).to receive(:find_by).with(confirmation_token: 'non_existent_token').and_return(nil) + allow(controller).to receive(:set_user_by_confirmation_token).and_call_original + end + + it 'redirects with alert for non-existent token' do + post :post_resend_confirmation, params: { token: 'non_existent_token' } + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Utilisateur non trouvé') + end + end + + context 'when the email is already verified' do + let(:verified_user) { create(:user, email: 'verified@example.com', email_verified_at: Time.zone.now, confirmation_token: 'verified_token') } + + before do + allow(controller).to receive(:set_user_by_confirmation_token) do + controller.instance_variable_set(:@user, verified_user) + end + end + + it 'redirects with alert for already verified email' do + post :post_resend_confirmation, params: { token: verified_user.confirmation_token } + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Adresse email non trouvée ou déjà confirmée.') + end + end + end + end + RSpec.shared_examples "a method that needs a valid merge token" do context 'when the merge token is invalid' do before do diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index b91ca8927..151907c72 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -101,6 +101,31 @@ RSpec.describe UserMailer, type: :mailer do end end + describe '#custom_confirmation_instructions' do + let(:user) { create(:user, email: 'user@example.com') } + let(:token) { 'confirmation_token_123' } + let(:mail) { UserMailer.custom_confirmation_instructions(user, token) } + + it 'renders the headers' do + expect(mail.subject).to eq('Confirmez votre email') + expect(mail.to).to eq([user.email]) + expect(mail.from).to eq(['contact@demarches-simplifiees.fr']) + end + + it 'renders the body' do + expect(mail.body.encoded).to match(user.email) + expect(mail.body.encoded).to match(token) + end + + it 'assigns @user' do + expect(mail.body.encoded).to match(user.email) + end + + it 'assigns @token' do + expect(mail.body.encoded).to include(token) + end + end + describe '.send_archive' do let(:procedure) { create(:procedure) } let(:archive) { create(:archive) } diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 688cfd6be..53573c126 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -10,18 +10,71 @@ describe FranceConnectInformation, type: :model do end describe 'associate_user!' do - context 'when there is no user with same email' do - let(:email) { 'A@email.com' } - let(:fci) { build(:france_connect_information) } + let(:email) { 'A@email.com' } + let(:fci) { build(:france_connect_information) } - subject { fci.associate_user!(email) } + subject { fci.associate_user!(email) } - it { expect { subject }.to change(User, :count).by(1) } + context 'when there is no user with the same email' do + it 'creates a new user' do + expect { subject }.to change(User, :count).by(1) + end - it do + it 'sets the correct attributes on the user' do subject - expect(fci.user.email).to eq('a@email.com') - expect(fci.user.email_verified_at).to be_present + user = User.find_by(email: email.downcase) + expect(user).not_to be_nil + expect(user.confirmed_at).to be_present + end + + it 'sends custom confirmation instructions' do + expect(UserMailer).to receive(:custom_confirmation_instructions).and_call_original + subject + end + + it 'associates the user with the FranceConnectInformation' do + subject + expect(fci.reload.user.email).to eq(email.downcase) + end + end + + context 'when a user with the same email already exists due to race condition' do + let!(:existing_user) { create(:user, email: email.downcase) } + let!(:fci) { create(:france_connect_information) } # Assurez-vous que fci est créé et sauvegardé + + before do + call_count = 0 + allow(User).to receive(:create!).and_wrap_original do + call_count += 1 + if call_count == 1 + raise ActiveRecord::RecordNotUnique + else + existing_user + end + end + allow(fci).to receive(:send_custom_confirmation_instructions) + end + + it 'raises an error' do + expect { fci.associate_user!(email) }.to raise_error(NoMethodError) + end + + it 'does not create a new user' do + expect { + begin + fci.associate_user!(email) + rescue NoMethodError + end + }.to_not change(User, :count) + end + + it 'does not associate with any user' do + expect(fci.user).to be_nil + begin + fci.associate_user!(email) + rescue NoMethodError + end + expect(fci.reload.user).to be_nil end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8e33e8951..3ad7b89dd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -18,12 +18,6 @@ describe User, type: :model do user.confirm expect(user.reload.invites.size).to eq(2) end - - it 'verifies its email' do - expect(user.email_verified_at).to be_nil - user.confirm - expect(user.email_verified_at).to be_present - end end describe '#owns?' do diff --git a/spec/system/france_connect/france_connect_particulier_spec.rb b/spec/system/france_connect/france_connect_particulier_spec.rb index a78b01655..60f1c2c91 100644 --- a/spec/system/france_connect/france_connect_particulier_spec.rb +++ b/spec/system/france_connect/france_connect_particulier_spec.rb @@ -43,9 +43,28 @@ describe 'France Connect Particulier Connexion' do before { page.find('.fr-connect').click } scenario 'he is redirected to user dossiers page' do - expect(page).to have_content('Dossiers') + expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) + find('#use_france_connect_email_yes').click + click_on 'Confirmer' + expect(page).to have_content("Confirmation envoyée") + click_on 'Continuer' expect(User.find_by(email: email)).not_to be nil end + + scenario 'he can choose not to use FranceConnect email and input an alternative email' do + expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_selector("#use_france_connect_email_no", visible: false, wait: 10) + + find('#use_france_connect_email_no').click + expect(page).to have_selector("input[name='alternative_email']", visible: true, wait: 10) + + fill_in 'alternative_email', with: 'alternative@example.com' + click_on 'Confirmer' + expect(page).to have_content("Confirmation envoyée") + click_on 'Continuer' + expect(User.find_by(email: 'alternative@example.com')).not_to be nil + end end context 'and an user exists with the same email' do @@ -79,17 +98,18 @@ describe 'France Connect Particulier Connexion' do let!(:another_user) { create(:user, email: 'an_existing_email@a.com', password: SECURE_PASSWORD) } scenario 'it uses another email that belongs to another account' do - page.find('#it-is-not-mine').click - fill_in 'email', with: 'an_existing_email@a.com' - click_on 'Utiliser ce mail' + find('label[for="it-is-not-mine"]').click - expect(page).to have_css('#password-for-another-account', visible: true) + expect(page).to have_css('.new-account', visible: true) - within '#new-account-password-confirmation' do - fill_in 'password', with: SECURE_PASSWORD - click_on 'Fusionner les comptes' + within '.new-account' do + fill_in 'email', with: 'an_existing_email@a.com' + click_on 'Utiliser ce mail' end + fill_in 'password-for-another-account', with: SECURE_PASSWORD + last_button = all('input[type="submit"][value="Fusionner les comptes"]').last.click + puts last_button.click expect(page).to have_content('Dossiers') end end diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index 74535c731..f00ffae34 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -191,7 +191,15 @@ describe 'Prefilling a dossier (with a GET request):', js: true do page.find('.fr-connect').click - click_on "Poursuivre mon dossier prérempli" + expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) + page.execute_script('document.getElementById("use_france_connect_email_yes").click()') + + click_on 'Confirmer' + expect(page).to have_content("Confirmation envoyée") + click_on 'Continuer' + expect(page).to have_content('Vous avez un dossier prérempli') + find('.fr-btn.fr-mb-2w', text: 'Poursuivre mon dossier prérempli', wait: 10).click end end end diff --git a/spec/system/users/dossier_prefill_post_spec.rb b/spec/system/users/dossier_prefill_post_spec.rb index 3fa134714..0ca1baf1c 100644 --- a/spec/system/users/dossier_prefill_post_spec.rb +++ b/spec/system/users/dossier_prefill_post_spec.rb @@ -142,9 +142,15 @@ describe 'Prefilling a dossier (with a POST request):', js: true do allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(build(:france_connect_information)) page.find('.fr-connect').click + expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) + page.execute_script('document.getElementById("use_france_connect_email_yes").click()') + click_on 'Confirmer' + expect(page).to have_content("Confirmation envoyée") + click_on 'Continuer' expect(page).to have_content('Vous avez un dossier prérempli') - click_on 'Poursuivre mon dossier prérempli' + find('.fr-btn.fr-mb-2w', text: 'Poursuivre mon dossier prérempli', wait: 10).click end end end From 337f88575dcac191b5ffc1dcb5c03e3179089a1d Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Thu, 4 Jul 2024 22:03:20 +0000 Subject: [PATCH 08/22] =?UTF-8?q?Modifications=20sur=20la=20m=C3=A9thode?= =?UTF-8?q?=20after=5Fconfirmation=20dans=20le=20mod=C3=A8le=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/user.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index c25b90712..4936fac90 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -65,7 +65,6 @@ class User < ApplicationRecord # Callback provided by Devise def after_confirmation - update!(email_verified_at: Time.zone.now) link_invites! end From fe69ada7d96e11ab7d0063cbd09c736c6756cf50 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 24 Jul 2024 15:43:46 +0200 Subject: [PATCH 09/22] a11y(particulier/choose_email): no empty legend, ensure to link fieldset with legend --- app/views/france_connect/particulier/choose_email.html.haml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index f72f970dc..a1842f3d7 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -5,14 +5,12 @@ %p= t('.intro_html', email: france_connect_email) - %p= t('.use_email_for_notifications') - = form_with url: france_connect_particulier_associate_user_path, method: :post, data: { controller: "email-france-connect" } do |f| = hidden_field_tag :france_connect_email, france_connect_email = hidden_field_tag :merge_token, merge_token - %fieldset.fr-fieldset - %legend.fr-fieldset__legend + %fieldset.fr-fieldset{ aria: { labelledby: 'notifications-or-not-notification' } } + %legend.fr-fieldset__legend#notifications-or-not-notification= t('.use_email_for_notifications') .fr-fieldset__element.fr-fieldset__element--inline .fr-radio-group = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } From 828f491c14b077f8342535e708c5010d793f18e6 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 24 Jul 2024 15:57:26 +0200 Subject: [PATCH 10/22] feat(france_connect#choose_email): prevent bad email --- .../particulier/choose_email.html.haml | 19 +++++++++++++++---- config/locales/en.yml | 5 +++-- config/locales/fr.yml | 7 +++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index a1842f3d7..b540c52cb 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -15,18 +15,29 @@ .fr-radio-group = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } %label.fr-label{ for: 'use_france_connect_email_yes' } - = t('.yes') + = t('utils.yes') .fr-fieldset__element.fr-fieldset__element--inline .fr-radio-group = f.radio_button :use_france_connect_email, false, id: 'use_france_connect_email_no', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } %label.fr-label{ for: 'use_france_connect_email_no' } - = t('.no') + = t('utils.no') .fr-fieldset.fr-w-30v - .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField" } } + .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } = f.label :alternative_email, t('.alternative_email'), class: "fr-label" %span.fr-hint-text.mb-1= t('activerecord.attributes.user.hints.email') - = f.email_field :alternative_email, class: "fr-input" + = f.email_field :alternative_email, class: "fr-input", data: { action: "blur->email-input#checkEmail", 'email-input-target': 'input' } + + .suspect-email.hidden{ data: { "email-input-target": 'ariaRegion'}, aria: { live: 'off' } } + = render Dsfr::AlertComponent.new(title: t('utils.email_suggest.wanna_say'), state: :info, heading_level: :div) do |c| + - c.with_body do + %p{ data: { "email-input-target": 'suggestion'} } exemple@gmail.com  ? + %p + = button_tag type: 'button', class: 'fr-btn fr-btn--sm fr-mr-3w', data: { action: 'click->email-input#accept'} do + = t('utils.yes') + = button_tag type: 'button', class: 'fr-btn fr-btn--sm', data: { action: 'click->email-input#discard'} do + = t('utils.no') + %div = f.submit t('.confirm'), class: 'fr-btn' diff --git a/config/locales/en.yml b/config/locales/en.yml index a73a0b449..4725ab107 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -47,6 +47,9 @@ en: utils: 'yes': 'Yes' 'no': 'No' + email_suggest: + wanna_say: 'Do you mean to say ?' + deconnexion: "Log out" pj: "Attachments" asterisk_html: "Fields marked by an asterisk ( required ) are mandatory." @@ -884,8 +887,6 @@ en: choose_email: intro_html: "Your FranceConnect account uses %{email}r as the contact email." use_email_for_notifications: "Would you like to use it to receive notifications regarding the progress of your cases?" - 'yes': "Yes" - 'no': "No" confirm: "Confirm" choose_email_contact: "Choose your contact email" alternative_email: "Please provide the email to use for contacting you." diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f209df748..93e40c552 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -37,6 +37,9 @@ fr: utils: 'yes': Oui 'no': Non + email_suggest: + wanna_say: 'Voulez-vous dire ?' + i_dont_know: Je ne sais pas deconnexion: "Déconnexion" pj: "Pièces jointes" @@ -938,12 +941,12 @@ fr: choose_email: intro_html: "Votre compte FranceConnect utilise %{email} comme email de contact." use_email_for_notifications: Souhaitez-vous l'utiliser pour recevoir les notifications concernant l'avancement de vos dossiers ? - 'yes': Oui - 'no': Non confirm: Confirmer choose_email_contact: Choisissez votre e-mail de contact alternative_email: Veuillez nous fournir l'email à utiliser pour vous contacter. greetings: Bonjour, + email_suggest: + wanna_say: 'Voulez-vous dire ?' confirmation_sent: confirmation_sent_by_email: Confirmation envoyée par mail intro: Un mail de confirmation a été envoyé à votre adresse %{email} From 88a4619dcb1e1afb774c67d275b41b22199eeb13 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 24 Jul 2024 16:36:16 +0200 Subject: [PATCH 11/22] feat(particulier#associate_user): catch email already taken and redirect to reset password --- .../france_connect/particulier_controller.rb | 6 ++++++ config/locales/en.yml | 3 +++ config/locales/fr.yml | 2 ++ .../particulier_controller_spec.rb | 21 +++++++++++++++++++ 4 files changed, 32 insertions(+) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 3eb3c494d..6f9cbda83 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -58,6 +58,12 @@ class FranceConnect::ParticulierController < ApplicationController destination_path = destination_path(user) render :confirmation_sent, locals: { email:, destination_path: } + rescue ActiveRecord::RecordInvalid => e + if e.record.errors.where(:email, :taken) + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path) + else + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.unknown_error') + end end def merge diff --git a/config/locales/en.yml b/config/locales/en.yml index 4725ab107..c7341c757 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -751,6 +751,9 @@ en: # # etablissement_fail: 'Désolé, nous n’avons pas réussi à enregistrer l’établissement correspondant à ce numéro SIRET' france_connect: connexion: "Error trying to connect to France Connect." + forbidden_html: "Only citizen can use FranceConnect. As an instructor or administrator, you should reset your password." + email_taken: "This email is already in use. You should reset your password." + unknown_error: "An error occured, please retry." evil_regexp: The regular expression you have entered is potentially dangerous and could lead to performance issues. mismatch_regexp: The provided example must match the regular expression syntax_error_regexp: The syntax of the regular expression is invalid diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 93e40c552..8fb2c88b2 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -757,6 +757,8 @@ fr: france_connect: connexion: "Erreur lors de la connexion à France Connect." forbidden_html: "Seul-e-s les usagers peuvent se connecter via France Connect. En tant qu’instructeur ou administrateur, nous vous invitons à réininitialiser votre mot de passe." + email_taken: "Cet email est déjà utilisé. Nous vous invitons à réininitialiser votre mot de passe." + unknown_error: "Une erreure est survenue. Veuillez réessayer." evil_regexp: L'expression régulière que vous avez entrée est potentiellement dangereuse et pourrait entraîner des problèmes de performance mismatch_regexp: L'exemple doit correspondre à l'expression régulière fournie syntax_error_regexp: La syntaxe de l'expression régulière n'est pas valide diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 6fe560588..5eb8c0165 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -253,6 +253,27 @@ describe FranceConnect::ParticulierController, type: :controller do subject end end + + context 'when associating the user conflict with existing one' do + let(:fci) { instance_double('FranceConnectInformation') } + let(:email) { 'user@example.com' } + let(:user) { instance_double('User', id: 1) } + let(:destination_path) { '/' } + + before do + create(:user, email:) + invalid_user = build(:user, email:) + allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) + allow(fci).to receive(:valid_for_merge?).and_return(true) + allow(fci).to receive(:email_france_connect).and_return(email) + invalid_user.valid? + allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(invalid_user)) + end + + it 'fails' do + subject + end + end end describe '#confirm_email' do From 03d425859d399274806651e65fd13579e58ef2b6 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 24 Jul 2024 16:57:08 +0200 Subject: [PATCH 12/22] feat(particulier#confirm_email): nicer --- .../particulier/confirmation_sent.html.haml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/views/france_connect/particulier/confirmation_sent.html.haml b/app/views/france_connect/particulier/confirmation_sent.html.haml index d167d2b01..23ef74732 100644 --- a/app/views/france_connect/particulier/confirmation_sent.html.haml +++ b/app/views/france_connect/particulier/confirmation_sent.html.haml @@ -1,8 +1,12 @@ .fr-container - %h1.text-center.mt-1= t('.confirmation_sent_by_email') + .fr-col-12.fr-col-md-6.fr-col-offset-md-3 + %h1.fr-mt-6w.fr-h2.center= t('.confirmation_sent_by_email') - %p= t('.intro', email: email) + %p.center{ aria: { hidden: true } }= image_tag("user/confirmation-email.svg", alt: t('views.confirmation.new.image_alt')) - %p= t('.click_the_link_in_the_email') + = render Dsfr::AlertComponent.new(title: '', state: :info, heading_level: 'h2', extra_class_names: 'fr-mt-6w fr-mb-3w') do |c| + - c.with_body do + %p= t('.intro', email: email) + %p= t('.click_the_link_in_the_email') - = link_to t('.continue'), destination_path, class: 'fr-btn' + %p.center= link_to t('.continue'), destination_path, class: 'fr-btn' From 3cd5d778ca919124e3721379479733cd696987c2 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 24 Jul 2024 17:08:29 +0200 Subject: [PATCH 13/22] a11y(particulier/merge): no empty legend, ensure to link fieldset with legend --- app/views/france_connect/particulier/merge.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index b7dcd4b40..127d3f96f 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -5,8 +5,8 @@ %p= t('.subtitle_html', email: @fci.email_france_connect, application_name: Current.application_name) - %fieldset.fr-fieldset - %legend.fr-fieldset__legend= t('.label_select_merge_flow', email: @fci.email_france_connect) + %fieldset.fr-fieldset{ aria: { labelledby: 'merge-account' } } + %legend.fr-fieldset__legend#merge-account= t('.label_select_merge_flow', email: @fci.email_france_connect) .fr-fieldset__element.fr-fieldset__element--inline .fr-radio-group %input{ type: 'radio', id: 'it-is-mine', name: 'value', value: 'true', autocomplete: "off", onclick: "DS.showFusion(event);" } From b6d0502f39c2848f807d6746a5c541685549a3ab Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Thu, 25 Jul 2024 11:24:43 +0000 Subject: [PATCH 14/22] =?UTF-8?q?modifications=20apr=C3=A8s=20derni=C3=A8r?= =?UTF-8?q?e=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../france_connect/particulier_controller.rb | 39 ++- app/models/france_connect_information.rb | 1 - .../particulier/resend_confirmation.html.haml | 11 - config/locales/en.yml | 5 + config/locales/fr.yml | 7 +- config/routes.rb | 4 +- .../particulier_controller_spec.rb | 234 +++++++++--------- .../models/france_connect_information_spec.rb | 5 - 8 files changed, 147 insertions(+), 159 deletions(-) delete mode 100644 app/views/france_connect/particulier/resend_confirmation.html.haml diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 6f9cbda83..6ffdd2455 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -4,7 +4,7 @@ class FranceConnect::ParticulierController < ApplicationController before_action :redirect_to_login_if_fc_aborted, only: [:callback] before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation, :associate_user] before_action :securely_retrieve_fci_from_email_merge_token, only: [:mail_merge_with_existing_account] - before_action :set_user_by_confirmation_token, only: [:resend_confirmation, :post_resend_confirmation, :confirm_email, :connect_france_connect_particulier_redirect] + before_action :set_user_by_confirmation_token, only: [:confirm_email] def login if FranceConnectService.enabled? @@ -50,14 +50,15 @@ class FranceConnect::ParticulierController < ApplicationController def associate_user email = use_fc_email? ? @fci.email_france_connect : params[:alternative_email] + @fci.associate_user!(email) user = @fci.user + @fci.send_custom_confirmation_instructions(user) @fci.delete_merge_token! sign_only(user) - destination_path = destination_path(user) - render :confirmation_sent, locals: { email:, destination_path: } + render :confirmation_sent, locals: { email:, destination_path: destination_path(user) } rescue ActiveRecord::RecordInvalid => e if e.record.errors.where(:email, :taken) redirect_to new_user_session_path, alert: t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path) @@ -134,28 +135,20 @@ class FranceConnect::ParticulierController < ApplicationController end def confirm_email - if @user.confirmation_sent_at && 2.days.ago < @user.confirmation_sent_at + if @user.confirmation_sent_at && @user.confirmation_sent_at > 2.days.ago @user.update(email_verified_at: Time.zone.now, confirmation_token: nil) @user.after_confirmation - redirect_to stored_location_for(@user) || root_path(@user), notice: 'Votre email est bien vérifié' - else - redirect_to france_connect_resend_confirmation_path(token: @user.confirmation_token), alert: "Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé." + redirect_to destination_path(@user), notice: I18n.t('france_connect.particulier.flash.email_confirmed') + return end - end - def resend_confirmation - if @user.email_verified_at - redirect_to root_path, alert: 'Email déjà confirmé.' - end - end + fci = FranceConnectInformation.find_by(user: @user) - def post_resend_confirmation - if !@user.email_verified_at - fci = FranceConnectInformation.find_by(user: @user) + if fci fci.send_custom_confirmation_instructions(@user) - redirect_to root_path(@user), notice: "Un nouveau lien de confirmation vous a été envoyé par mail." + redirect_to root_path, notice: I18n.t('france_connect.particulier.flash.confirmation_mail_resent') else - redirect_to root_path(@user), alert: "Adresse email non trouvée ou déjà confirmée." + redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error') end end @@ -165,12 +158,12 @@ class FranceConnect::ParticulierController < ApplicationController @user = User.find_by(confirmation_token: params[:token]) if @user.nil? - redirect_to root_path, alert: 'Utilisateur non trouvé' and return + redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.user_not_found') and return end if user_signed_in? && current_user != @user sign_out current_user - redirect_to new_user_session_path, alert: "Veuillez vous connecter avec le compte associé à ce lien de confirmation." + redirect_to new_user_session_path, alert: I18n.t('france_connect.particulier.flash.redirect_new_user_session') end end @@ -187,7 +180,7 @@ class FranceConnect::ParticulierController < ApplicationController @fci = FranceConnectInformation.find_by(email_merge_token: email_merge_token_params) if @fci.nil? || !@fci.valid_for_email_merge? - flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) + flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) redirect_to root_path else @@ -199,7 +192,7 @@ class FranceConnect::ParticulierController < ApplicationController @fci = FranceConnectInformation.find_by(merge_token: merge_token_params) if @fci.nil? || !@fci.valid_for_merge? - flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) + flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) redirect_to root_path end @@ -220,7 +213,7 @@ class FranceConnect::ParticulierController < ApplicationController user.update_attribute('loged_in_with_france_connect', User.loged_in_with_france_connects.fetch(:particulier)) - redirect_to stored_location_for(current_user) || root_path(current_user) + redirect_to destination_path(current_user) end def redirect_france_connect_error_connection diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 63cb341dd..8761dd5d1 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -15,7 +15,6 @@ class FranceConnectInformation < ApplicationRecord password: Devise.friendly_token[0, 20], confirmed_at: Time.zone.now ) - send_custom_confirmation_instructions(user) rescue ActiveRecord::RecordNotUnique # ignore this exception because we check before if user is nil. # exception can be raised in race conditions, when FranceConnect calls callback 2 times. diff --git a/app/views/france_connect/particulier/resend_confirmation.html.haml b/app/views/france_connect/particulier/resend_confirmation.html.haml deleted file mode 100644 index bc03ca5c7..000000000 --- a/app/views/france_connect/particulier/resend_confirmation.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -.fr-container - %h1.text-center.mt-1 Renvoyer le lien de confirmation - - %p Bonjour #{@user.email} - - %p Cliquez sur le bouton ci-dessous pour recevoir un nouveau lien de confirmation à votre adresse email. - - = form_with url: france_connect_post_resend_confirmation_path, method: :post, local: true do |form| - = form.hidden_field :token, value: @user.confirmation_token - .form-group - = form.submit "Renvoyer le lien de confirmation", class: 'fr-btn' diff --git a/config/locales/en.yml b/config/locales/en.yml index c7341c757..c42e6453e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -914,6 +914,11 @@ en: link_confirm_by_email: Confirm by receiving an email flash: confirmation_mail_sent: "An email with the confirmation link has been sent, please click on the link." + confirmation_mail_resent: "Confirmation link expired. A new link has been sent by email." + confirmation_mail_resent_error: "An unexpected error has occurred. Please contact support if the problem persists." + redirect_new_user_session: "You have been disconnected from your previous account. Please click on the confirmation link again." + email_confirmed: "Your email is confirmed" + user_not_found: "User not found" invalid_password: "The password is not correct." connection_done: "The accounts for FranceConnect and %{application_name} are now merged." connection_done_verify_email: "The FranceConnect and %{application_name} accounts are now merged. Please confirm your email by clicking on the link you received in your email." diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 8fb2c88b2..2ce66e013 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -969,7 +969,12 @@ fr: link_confirm_by_email: Confirmer mon compte par email flash: confirmation_mail_sent: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" - invalid_password: "Mauvais mot de passe" + confirmation_mail_resent: "Le lien de confirmation a expiré. Un nouveau lien de confirmation vous a été envoyé par email." + confirmation_mail_resent_error: "Une erreur inattendue est survenue. Veuillez contacter le support si le problème persiste." + redirect_new_user_session: "Vous avez été déconnecté de votre précédent compte. Veuillez cliquer à nouveau sur le lien de confirmation." + email_confirmed: 'Votre email est bien vérifié' + user_not_found: 'Utilisateur non trouvé' + invalid_password: "Mot de passe incorrect" connection_done: "Les comptes FranceConnect et %{application_name} sont à présent fusionnés" connection_done_verify_email: "Les comptes FranceConnect et %{application_name} sont maintenant fusionnés. Veuillez confirmer votre e-mail en cliquant sur le lien que vous avez reçu par mail" merger_token_expired: "Le délai pour fusionner les comptes FranceConnect et %{application_name} est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes." diff --git a/config/routes.rb b/config/routes.rb index 70495e4a7..0a82f4c72 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -191,12 +191,10 @@ Rails.application.routes.draw do get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge get 'particulier/mail_merge_with_existing_account/:email_merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account get 'confirm_email/:token', to: 'particulier#confirm_email', as: :confirm_email - get 'resend_confirmation', to: 'particulier#resend_confirmation', as: 'resend_confirmation' - post 'post_resend_confirmation', to: 'particulier#post_resend_confirmation', as: 'post_resend_confirmation' post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account' post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account' - post 'particulier/associate_user', to: 'particulier#associate_user', as: :particulier_associate_user + post 'particulier/associate_user' end namespace :agent_connect do diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 5eb8c0165..cf50c71f4 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -162,6 +162,7 @@ describe FranceConnect::ParticulierController, type: :controller do allow(fci).to receive(:associate_user!) allow(fci).to receive(:user).and_return(user) allow(fci).to receive(:delete_merge_token!) + allow(fci).to receive(:send_custom_confirmation_instructions) allow(controller).to receive(:use_fc_email?).and_return(true) allow(controller).to receive(:sign_only) allow(controller).to receive(:destination_path).and_return(destination_path) @@ -169,18 +170,50 @@ describe FranceConnect::ParticulierController, type: :controller do subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } } - it 'renders the confirmation_sent template' do - subject - expect(response).to render_template(:confirmation_sent) + context 'when association is successful' do + it 'renders the confirmation_sent template' do + subject + expect(response).to render_template(:confirmation_sent) + end + + it 'performs all expected steps' do + expect(fci).to receive(:associate_user!).with(email) + expect(fci).to receive(:send_custom_confirmation_instructions).with(user) + expect(fci).to receive(:delete_merge_token!) + expect(controller).to receive(:sign_only).with(user) + expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path }) + + subject + end end - it 'performs all expected steps' do - expect(fci).to receive(:associate_user!).with(email) - expect(fci).to receive(:delete_merge_token!) - expect(controller).to receive(:sign_only).with(user) - expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path }) + context 'when association fails due to taken email' do + before do + allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(User.new)) + allow_any_instance_of(User).to receive_message_chain(:errors, :where).and_return(['Some error']) + end - subject + it 'redirects to new user session path with taken email alert' do + subject + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path)) + end + end + + context 'when association fails due to unknown error' do + let(:user) { User.new } + let(:error) { ActiveRecord::RecordInvalid.new(user) } + + before do + allow(fci).to receive(:associate_user!).and_raise(error) + allow(user.errors).to receive(:where).with(:email, :taken).and_return(nil) + end + + it 'redirects to new user session path with unknown error alert' do + subject + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.unknown_error')) + end end end @@ -217,24 +250,34 @@ describe FranceConnect::ParticulierController, type: :controller do let(:email) { 'user@example.com' } let(:user) { instance_double('User', id: 1) } let(:destination_path) { '/' } + let(:merge_token) { 'sample_merge_token' } before do - allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) - allow(fci).to receive(:valid_for_merge?).and_return(true) + allow(controller).to receive(:securely_retrieve_fci) do + controller.instance_variable_set(:@fci, fci) + end allow(fci).to receive(:email_france_connect).and_return(email) allow(fci).to receive(:associate_user!) allow(fci).to receive(:user).and_return(user) allow(fci).to receive(:delete_merge_token!) + allow(fci).to receive(:send_custom_confirmation_instructions) allow(controller).to receive(:use_fc_email?).and_return(true) allow(controller).to receive(:sign_only) allow(controller).to receive(:destination_path).and_return(destination_path) end + subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } } + it 'calls associate_user! with the correct email' do expect(fci).to receive(:associate_user!).with(email) subject end + it 'sends custom confirmation instructions' do + expect(fci).to receive(:send_custom_confirmation_instructions).with(user) + subject + end + it 'deletes the merge token' do expect(fci).to receive(:delete_merge_token!) subject @@ -327,137 +370,98 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when the confirmation token is expired' do + let(:expired_user_confirmation) do + create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago) + end + before do - sign_in(expired_user_confirmation) - get :confirm_email, params: { token: expired_user_confirmation.confirmation_token } + allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) + allow(controller).to receive(:user_signed_in?).and_return(false) + allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(nil) end - it 'redirects to the resend confirmation path with an alert' do - expect(response).to redirect_to(france_connect_resend_confirmation_path(token: expired_user_confirmation.confirmation_token)) - expect(flash[:alert]).to eq('Lien de confirmation expiré. Un nouveau lien de confirmation a été envoyé.') - end - end + it 'redirects to root path with an alert when FranceConnectInformation is not found' do + get :confirm_email, params: { token: 'expired_token' } - context 'GET #resend_confirmation' do - let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } - - context 'when the token is valid' do - it 'assigns @user and renders the form' do - get :resend_confirmation, params: { token: user.confirmation_token } - expect(assigns(:user)).to eq(user) - expect(response).to render_template(:resend_confirmation) - end + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error')) end - context 'when the email is already confirmed' do - let(:confirmed_user) { create(:user, email: 'confirmed@example.com', email_verified_at: Time.zone.now, confirmation_token: nil) } + context 'when FranceConnectInformation exists' do + let(:france_connect_information) { instance_double(FranceConnectInformation) } before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, confirmed_user) - end + allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(france_connect_information) + allow(france_connect_information).to receive(:send_custom_confirmation_instructions) end - it 'redirects to root path with an alert about already confirmed email' do - get :resend_confirmation, params: { email: confirmed_user.email } + it 'resends the confirmation email and redirects to root path with a notice' do + expect(france_connect_information).to receive(:send_custom_confirmation_instructions).with(expired_user_confirmation) + + get :confirm_email, params: { token: 'expired_token' } + expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Email déjà confirmé.') - end - end - - context 'when the token is invalid' do - before do - get :resend_confirmation, params: { token: 'invalid_token' } - end - - it 'sets @user to nil and redirects to root path with an alert' do - expect(assigns(:user)).to be_nil - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Utilisateur non trouvé') - end - end - - context 'when the user is not found' do - before do - allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil) - end - - it 'redirects to root_path with an alert' do - get :resend_confirmation, params: { token: 'invalid_token' } - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Utilisateur non trouvé') - end - end - - context 'when a different user is signed in' do - let(:current_user) { create(:user) } - let(:target_user) { create(:user, confirmation_token: 'valid_token') } - - before do - sign_in current_user - allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(target_user) - end - - it 'signs out the current user and redirects to new_user_session_path with an alert' do - get :resend_confirmation, params: { token: 'valid_token' } - expect(controller.current_user).to be_nil - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to eq("Veuillez vous connecter avec le compte associé à ce lien de confirmation.") + expect(flash[:notice]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent')) end end end - context 'POST #post_resend_confirmation' do - let(:user) { create(:user, email: 'test@example.com', email_verified_at: nil, confirmation_token: 'valid_token') } - let(:fci) { create(:france_connect_information, user: user) } + context 'when a different user is signed in' do + let(:expired_user_confirmation) do + create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago) + end + let(:current_user) { create(:user) } before do - allow(FranceConnectInformation).to receive(:find_by).with(user: user).and_return(fci) - allow(controller).to receive(:set_user_by_confirmation_token).and_call_original + allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) + allow(controller).to receive(:user_signed_in?).and_return(true) + allow(controller).to receive(:current_user).and_return(current_user) end - context 'when the user exists and email is not verified' do - before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, user) - end - end + it 'signs out the current user and redirects to sign in path' do + expect(controller).to receive(:sign_out).with(current_user) - it 'sends custom confirmation instructions and redirects with notice' do - expect(fci).to receive(:send_custom_confirmation_instructions).with(user) - post :post_resend_confirmation, params: { token: user.confirmation_token } - expect(response).to redirect_to(root_path) - expect(flash[:notice]).to eq('Un nouveau lien de confirmation vous a été envoyé par mail.') - end + get :confirm_email, params: { token: 'expired_token' } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.redirect_new_user_session')) end - context 'when the user does not exist' do - before do - allow(User).to receive(:find_by).with(confirmation_token: 'non_existent_token').and_return(nil) - allow(controller).to receive(:set_user_by_confirmation_token).and_call_original - end + it 'does not process the confirmation' do + expect(FranceConnectInformation).not_to receive(:find_by) + expect_any_instance_of(FranceConnectInformation).not_to receive(:send_custom_confirmation_instructions) - it 'redirects with alert for non-existent token' do - post :post_resend_confirmation, params: { token: 'non_existent_token' } - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Utilisateur non trouvé') - end + get :confirm_email, params: { token: 'expired_token' } end + end + end - context 'when the email is already verified' do - let(:verified_user) { create(:user, email: 'verified@example.com', email_verified_at: Time.zone.now, confirmation_token: 'verified_token') } + describe '#set_user_by_confirmation_token' do + let(:current_user) { create(:user) } + let(:confirmation_user) { create(:user, confirmation_token: 'valid_token') } - before do - allow(controller).to receive(:set_user_by_confirmation_token) do - controller.instance_variable_set(:@user, verified_user) - end - end + before do + sign_in current_user + allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(confirmation_user) + end - it 'redirects with alert for already verified email' do - post :post_resend_confirmation, params: { token: verified_user.confirmation_token } - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Adresse email non trouvée ou déjà confirmée.') - end + it 'signs out current user and redirects to new session path when users do not match' do + expect(controller).to receive(:sign_out).with(current_user) + + get :confirm_email, params: { token: 'valid_token' } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.redirect_new_user_session')) + end + + context 'when user is not found' do + it 'redirects to root path with user not found alert' do + allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil) + + get :confirm_email, params: { token: 'invalid_token' } + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.user_not_found')) end end end diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 53573c126..aaefe9da5 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -27,11 +27,6 @@ describe FranceConnectInformation, type: :model do expect(user.confirmed_at).to be_present end - it 'sends custom confirmation instructions' do - expect(UserMailer).to receive(:custom_confirmation_instructions).and_call_original - subject - end - it 'associates the user with the FranceConnectInformation' do subject expect(fci.reload.user.email).to eq(email.downcase) From c0970693f360d51262c637e31cd9847d1699dfe3 Mon Sep 17 00:00:00 2001 From: mfo Date: Fri, 26 Jul 2024 14:04:23 +0200 Subject: [PATCH 15/22] fix(associate_user.with_existing_one): does not leak existing email when trying to choose an alternative email with france connect --- .../france_connect/particulier_controller.rb | 5 +++- config/locales/en.yml | 1 - config/locales/fr.yml | 1 - .../particulier_controller_spec.rb | 26 +++++++------------ 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 6ffdd2455..8b536a01a 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -61,7 +61,10 @@ class FranceConnect::ParticulierController < ApplicationController render :confirmation_sent, locals: { email:, destination_path: destination_path(user) } rescue ActiveRecord::RecordInvalid => e if e.record.errors.where(:email, :taken) - redirect_to new_user_session_path, alert: t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path) + user = User.find_by(email: e.record.email) + @fci.send_custom_confirmation_instructions(user) + @fci.delete_merge_token! + render :confirmation_sent, locals: { email: user.email, destination_path: destination_path(user) } else redirect_to new_user_session_path, alert: t('errors.messages.france_connect.unknown_error') end diff --git a/config/locales/en.yml b/config/locales/en.yml index c42e6453e..53ce483bd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -752,7 +752,6 @@ en: france_connect: connexion: "Error trying to connect to France Connect." forbidden_html: "Only citizen can use FranceConnect. As an instructor or administrator, you should reset your password." - email_taken: "This email is already in use. You should reset your password." unknown_error: "An error occured, please retry." evil_regexp: The regular expression you have entered is potentially dangerous and could lead to performance issues. mismatch_regexp: The provided example must match the regular expression diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 2ce66e013..40ecb5527 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -757,7 +757,6 @@ fr: france_connect: connexion: "Erreur lors de la connexion à France Connect." forbidden_html: "Seul-e-s les usagers peuvent se connecter via France Connect. En tant qu’instructeur ou administrateur, nous vous invitons à réininitialiser votre mot de passe." - email_taken: "Cet email est déjà utilisé. Nous vous invitons à réininitialiser votre mot de passe." unknown_error: "Une erreure est survenue. Veuillez réessayer." evil_regexp: L'expression régulière que vous avez entrée est potentiellement dangereuse et pourrait entraîner des problèmes de performance mismatch_regexp: L'exemple doit correspondre à l'expression régulière fournie diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index cf50c71f4..099d2d01f 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -187,19 +187,6 @@ describe FranceConnect::ParticulierController, type: :controller do end end - context 'when association fails due to taken email' do - before do - allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(User.new)) - allow_any_instance_of(User).to receive_message_chain(:errors, :where).and_return(['Some error']) - end - - it 'redirects to new user session path with taken email alert' do - subject - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.email_taken', reset_link: new_user_password_path)) - end - end - context 'when association fails due to unknown error' do let(:user) { User.new } let(:error) { ActiveRecord::RecordInvalid.new(user) } @@ -297,14 +284,13 @@ describe FranceConnect::ParticulierController, type: :controller do end end - context 'when associating the user conflict with existing one' do + context 'when associate_user uses an email of an existing user' do let(:fci) { instance_double('FranceConnectInformation') } let(:email) { 'user@example.com' } let(:user) { instance_double('User', id: 1) } let(:destination_path) { '/' } - + let(:existing_user) { create(:user, email:) } before do - create(:user, email:) invalid_user = build(:user, email:) allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) allow(fci).to receive(:valid_for_merge?).and_return(true) @@ -313,7 +299,13 @@ describe FranceConnect::ParticulierController, type: :controller do allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(invalid_user)) end - it 'fails' do + it 'sends confirmation to existing user' do + expect(controller).to receive(:render).with( + :confirmation_sent, + locals: { email: email, destination_path: destination_path } + ) + expect(fci).to receive(:send_custom_confirmation_instructions).with(existing_user) + expect(fci).to receive(:delete_merge_token!) subject end end From 94be5994013a5412b708bd9c8ce74cd4630ccef2 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 30 Jul 2024 15:36:09 +0200 Subject: [PATCH 16/22] add requested_email column to france_connect_information table --- ...ative_email_column_to_france_connect_information_table.rb | 5 +++++ db/schema.rb | 1 + 2 files changed, 6 insertions(+) create mode 100644 db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb diff --git a/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb b/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb new file mode 100644 index 000000000..0ff086128 --- /dev/null +++ b/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb @@ -0,0 +1,5 @@ +class AddAlternativeEmailColumnToFranceConnectInformationTable < ActiveRecord::Migration[7.0] + def change + add_column :france_connect_informations, :requested_email, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index bbef3938d..f7ba84f6b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -712,6 +712,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_29_141049) do t.string "given_name" t.string "merge_token" t.datetime "merge_token_created_at", precision: nil + t.string "requested_email" t.datetime "updated_at", precision: nil, null: false t.integer "user_id" t.index ["email_merge_token"], name: "index_france_connect_informations_on_email_merge_token" From eaef5c7e39f71707dd7be60d09bdd1df08375be9 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 31 Jul 2024 17:50:14 +0200 Subject: [PATCH 17/22] readapt name and logic --- .../france_connect/particulier_controller.rb | 214 ++++------ app/models/france_connect_information.rb | 25 +- .../_password_confirmation.html.haml | 2 +- .../particulier/choose_email.html.haml | 40 +- .../particulier/merge.html.haml | 15 +- .../france_connect_merge_confirmation.haml | 2 +- config/locales/en.yml | 2 - config/locales/fr.yml | 2 - config/routes.rb | 13 +- .../particulier_controller_spec.rb | 367 +++--------------- spec/mailers/user_mailer_spec.rb | 2 +- .../models/france_connect_information_spec.rb | 10 +- .../france_connect_particulier_spec.rb | 22 +- spec/system/users/dossier_prefill_get_spec.rb | 6 +- 14 files changed, 210 insertions(+), 512 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 8b536a01a..e0a7740de 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -2,8 +2,8 @@ class FranceConnect::ParticulierController < ApplicationController before_action :redirect_to_login_if_fc_aborted, only: [:callback] - before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :resend_and_renew_merge_confirmation, :associate_user] - before_action :securely_retrieve_fci_from_email_merge_token, only: [:mail_merge_with_existing_account] + before_action :securely_retrieve_fci, only: [:merge_using_fc_email, :merge_using_password, :send_email_merge_request] + before_action :securely_retrieve_fci_from_email_merge_token, only: [:merge_using_email_link] before_action :set_user_by_confirmation_token, only: [:confirm_email] def login @@ -15,130 +15,98 @@ class FranceConnect::ParticulierController < ApplicationController end def callback - fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code]) + @fci = FranceConnectService.find_or_retrieve_france_connect_information(params[:code]) - if fci.user.nil? - preexisting_unlinked_user = User.find_by(email: sanitize(fci.email_france_connect)) + if @fci.user.nil? + preexisting_unlinked_user = User.find_by(email: sanitize(@fci.email_france_connect)) if preexisting_unlinked_user.nil? - merge_token = fci.create_merge_token! - render :choose_email, locals: { france_connect_email: fci.email_france_connect, merge_token: } - - elsif !preexisting_unlinked_user.can_france_connect? - fci.destroy - redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) + @fci.create_merge_token! + render :choose_email + elsif preexisting_unlinked_user.can_france_connect? + @fci.create_merge_token! + render :merge else - merge_token = fci.create_merge_token! - redirect_to france_connect_particulier_merge_path(merge_token) + destroy_fci_and_redirect_to_login(@fci) end else - user = fci.user - - if user.can_france_connect? - fci.update(updated_at: Time.zone.now) - connect_france_connect_particulier(user) + if @fci.user.can_france_connect? + @fci.update(updated_at: Time.zone.now) + connect_france_connect_particulier(@fci.user) else - fci.destroy - redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) + destroy_fci_and_redirect_to_login(@fci) end end rescue Rack::OAuth2::Client::Error => e Rails.logger.error e.message - redirect_france_connect_error_connection + redirect_to(new_user_session_path, alert: t('errors.messages.france_connect.connexion')) end - def associate_user - email = use_fc_email? ? @fci.email_france_connect : params[:alternative_email] + def send_email_merge_request + @fci.update(requested_email: sanitized_email_params) - @fci.associate_user!(email) - user = @fci.user - - @fci.send_custom_confirmation_instructions(user) - @fci.delete_merge_token! - sign_only(user) - - render :confirmation_sent, locals: { email:, destination_path: destination_path(user) } - rescue ActiveRecord::RecordInvalid => e - if e.record.errors.where(:email, :taken) - user = User.find_by(email: e.record.email) - @fci.send_custom_confirmation_instructions(user) - @fci.delete_merge_token! - render :confirmation_sent, locals: { email: user.email, destination_path: destination_path(user) } - else - redirect_to new_user_session_path, alert: t('errors.messages.france_connect.unknown_error') - end - end - - def merge - end - - def merge_with_existing_account - user = User.find_by(email: sanitized_email_params) - - if user.present? && user.valid_for_authentication? { user.valid_password?(password_params) } - if !user.can_france_connect? - flash.alert = t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) - - redirect_to root_path - else - @fci.update(user: user) - @fci.delete_merge_token! - @fci.delete_email_merge_token! - - flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) - connect_france_connect_particulier(user) - end - else - flash.alert = t('france_connect.particulier.flash.invalid_password') - end - end - - def mail_merge_with_existing_account - user = User.find_by(email: sanitize(@fci.email_france_connect.downcase)) - if user.can_france_connect? - @fci.update(user: user) - @fci.delete_merge_token! - user.update(email_verified_at: Time.zone.now) - flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) - connect_france_connect_particulier(user) - else # same behaviour as redirect nicely with message when instructeur/administrateur - @fci.destroy - redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) - end - end - - def merge_with_new_account - user = User.find_by(email: sanitized_email_params) - - if user.nil? - @fci.associate_user!(sanitized_email_params) - @fci.delete_merge_token! - @fci.send_custom_confirmation_instructions(@fci.user) - flash.notice = t('france_connect.particulier.flash.connection_done_verify_email', application_name: Current.application_name) - connect_france_connect_particulier(@fci.user) - else - @email = sanitized_email_params - @merge_token = merge_token_params - end - end - - def resend_and_renew_merge_confirmation @fci.create_email_merge_token! UserMailer.france_connect_merge_confirmation( - @fci.email_france_connect, + sanitized_email_params, @fci.email_merge_token, @fci.email_merge_token_created_at ) .deliver_later - merge_token = @fci.create_merge_token! - redirect_to france_connect_particulier_merge_path(merge_token), - notice: t('france_connect.particulier.flash.confirmation_mail_sent') + redirect_to root_path, notice: t('france_connect.particulier.flash.confirmation_mail_sent') end + def merge_using_fc_email + @fci.safely_associate_user!(@fci.email_france_connect) + + sign_in(@fci.user) + + @fci.send_custom_confirmation_instructions + + render :confirmation_sent, locals: { email: @fci.email_france_connect, destination_path: destination_path(@fci.user) } + end + + def merge_using_password + user = User.find_by(email: sanitize(@fci.email_france_connect)) + + if user.present? && !user.can_france_connect? + return destroy_fci_and_redirect_to_login(@fci) + end + + if user.present? && user.valid_for_authentication? { user.valid_password?(params[:password]) } + @fci.safely_update_user(user:) + + flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) + connect_france_connect_particulier(user) + else + flash.alert = t('france_connect.particulier.flash.invalid_password') + end + end + + def merge_using_email_link + user = User.find_by(email: @fci.requested_email) + + if user.present? && !user.can_france_connect? + return destroy_fci_and_redirect_to_login(@fci) + end + + if user.nil? + @fci.safely_associate_user!(@fci.requested_email) + else + @fci.safely_update_user(user:) + end + + @fci.user.update(email_verified_at: Time.zone.now) + + flash.notice = t('france_connect.particulier.flash.connection_done', application_name: Current.application_name) + connect_france_connect_particulier(@fci.user) + end + + # TODO mutualiser avec le controller Users::ActivateController + # pour toute la partie de confirmation de compte def confirm_email - if @user.confirmation_sent_at && @user.confirmation_sent_at > 2.days.ago + if @user.confirmation_sent_at && 2.days.ago < @user.confirmation_sent_at @user.update(email_verified_at: Time.zone.now, confirmation_token: nil) @user.after_confirmation redirect_to destination_path(@user), notice: I18n.t('france_connect.particulier.flash.email_confirmed') @@ -148,7 +116,7 @@ class FranceConnect::ParticulierController < ApplicationController fci = FranceConnectInformation.find_by(user: @user) if fci - fci.send_custom_confirmation_instructions(@user) + fci.send_custom_confirmation_instructions redirect_to root_path, notice: I18n.t('france_connect.particulier.flash.confirmation_mail_resent') else redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.confirmation_mail_resent_error') @@ -161,26 +129,19 @@ class FranceConnect::ParticulierController < ApplicationController @user = User.find_by(confirmation_token: params[:token]) if @user.nil? - redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.user_not_found') and return + return redirect_to root_path, alert: I18n.t('france_connect.particulier.flash.user_not_found') end if user_signed_in? && current_user != @user - sign_out current_user + sign_out :user redirect_to new_user_session_path, alert: I18n.t('france_connect.particulier.flash.redirect_new_user_session') end end - def use_fc_email? = cast_bool(params[:use_france_connect_email]) - - def sign_only(user) - sign_out(user) if user_signed_in? - sign_in(user) - end - def destination_path(user) = stored_location_for(user) || root_path(user) def securely_retrieve_fci_from_email_merge_token - @fci = FranceConnectInformation.find_by(email_merge_token: email_merge_token_params) + @fci = FranceConnectInformation.find_by(email_merge_token: params[:email_merge_token]) if @fci.nil? || !@fci.valid_for_email_merge? flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) @@ -192,7 +153,7 @@ class FranceConnect::ParticulierController < ApplicationController end def securely_retrieve_fci - @fci = FranceConnectInformation.find_by(merge_token: merge_token_params) + @fci = FranceConnectInformation.find_by(merge_token: params[:merge_token]) if @fci.nil? || !@fci.valid_for_merge? flash.alert = I18n.t('france_connect.particulier.flash.merger_token_expired', application_name: Current.application_name) @@ -207,11 +168,13 @@ class FranceConnect::ParticulierController < ApplicationController end end - def connect_france_connect_particulier(user) - if user_signed_in? - sign_out :user - end + def destroy_fci_and_redirect_to_login(fci) + fci.destroy + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) + end + def connect_france_connect_particulier(user) + sign_out :user if user_signed_in? sign_in user user.update_attribute('loged_in_with_france_connect', User.loged_in_with_france_connects.fetch(:particulier)) @@ -219,23 +182,6 @@ class FranceConnect::ParticulierController < ApplicationController redirect_to destination_path(current_user) end - def redirect_france_connect_error_connection - flash.alert = t('errors.messages.france_connect.connexion') - redirect_to(new_user_session_path) - end - - def merge_token_params - params[:merge_token] - end - - def email_merge_token_params - params[:email_merge_token] - end - - def password_params - params[:password] - end - def sanitized_email_params sanitize(params[:email]) end diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 8761dd5d1..82fad522c 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -8,7 +8,7 @@ class FranceConnectInformation < ApplicationRecord validates :france_connect_particulier_id, presence: true, allow_blank: false, allow_nil: false - def associate_user!(email) + def safely_associate_user!(email) begin user = User.create!( email: email.downcase, @@ -22,11 +22,18 @@ class FranceConnectInformation < ApplicationRecord # because the first call has already created a user end + clean_tokens_and_requested_email update_attribute('user_id', user.id) - touch # needed to update updated_at column + save! end - def send_custom_confirmation_instructions(user) + def safely_update_user(user:) + self.user = user + clean_tokens_and_requested_email + save! + end + + def send_custom_confirmation_instructions token = SecureRandom.hex(10) user.update!(confirmation_token: token, confirmation_sent_at: Time.zone.now) UserMailer.custom_confirmation_instructions(user, token).deliver_later @@ -54,14 +61,18 @@ class FranceConnectInformation < ApplicationRecord (MERGE_VALIDITY.ago < email_merge_token_created_at) && user_id.nil? end - def delete_merge_token! - update(merge_token: nil, merge_token_created_at: nil) - end - def delete_email_merge_token! update(email_merge_token: nil, email_merge_token_created_at: nil) end + def clean_tokens_and_requested_email + self.merge_token = nil + self.merge_token_created_at = nil + self.email_merge_token = nil + self.email_merge_token_created_at = nil + self.requested_email = nil + end + def full_name [given_name, family_name].compact.join(" ") end diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index fd48f3619..3e23ee608 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -10,7 +10,7 @@ = password_field_tag :password, nil, autocomplete: 'current-password', id: 'password-for-another-account' .mb-2 = t('views.users.sessions.new.reset_password') - = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: merge_token), method: :post do + = link_to france_connect_particulier_send_email_merge_path(merge_token: merge_token), method: :post do = t('france_connect.particulier.merge.link_confirm_by_email') = button_tag t('.back'), type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' = submit_tag t('france_connect.particulier.merge.button_merge'), class: 'button primary' diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index b540c52cb..2ef702263 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -3,30 +3,24 @@ %p= t('.greetings') - %p= t('.intro_html', email: france_connect_email) + %p= t('.intro_html', email: @fci.email_france_connect) - = form_with url: france_connect_particulier_associate_user_path, method: :post, data: { controller: "email-france-connect" } do |f| - = hidden_field_tag :france_connect_email, france_connect_email - = hidden_field_tag :merge_token, merge_token + %p= t('.use_email_for_notifications') - %fieldset.fr-fieldset{ aria: { labelledby: 'notifications-or-not-notification' } } - %legend.fr-fieldset__legend#notifications-or-not-notification= t('.use_email_for_notifications') - .fr-fieldset__element.fr-fieldset__element--inline - .fr-radio-group - = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } - %label.fr-label{ for: 'use_france_connect_email_yes' } - = t('utils.yes') - .fr-fieldset__element.fr-fieldset__element--inline - .fr-radio-group - = f.radio_button :use_france_connect_email, false, id: 'use_france_connect_email_no', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } - %label.fr-label{ for: 'use_france_connect_email_no' } - = t('utils.no') + = button_to t('utils.yes'), + france_connect_particulier_merge_using_fc_email_path, + params: { merge_token: @fci.merge_token }, + class: 'fr-btn', + id: 'use_fc_email' - .fr-fieldset.fr-w-30v - .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } - = f.label :alternative_email, t('.alternative_email'), class: "fr-label" + .fr-fieldset.fr-w-30v.fr-mt-2w + = form_with url: france_connect_particulier_send_email_merge_request_path do |f| + = hidden_field_tag :merge_token, @fci.merge_token + + .fr-fieldset__element.fr-fieldset__element--inline{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } + = f.label :email, t('.alternative_email'), class: "fr-label" %span.fr-hint-text.mb-1= t('activerecord.attributes.user.hints.email') - = f.email_field :alternative_email, class: "fr-input", data: { action: "blur->email-input#checkEmail", 'email-input-target': 'input' } + = f.email_field :email, class: "fr-input", data: { action: "blur->email-input#checkEmail", 'email-input-target': 'input' } .suspect-email.hidden{ data: { "email-input-target": 'ariaRegion'}, aria: { live: 'off' } } = render Dsfr::AlertComponent.new(title: t('utils.email_suggest.wanna_say'), state: :info, heading_level: :div) do |c| @@ -37,7 +31,5 @@ = t('utils.yes') = button_tag type: 'button', class: 'fr-btn fr-btn--sm', data: { action: 'click->email-input#discard'} do = t('utils.no') - - - %div - = f.submit t('.confirm'), class: 'fr-btn' + %div + = f.submit t('.confirm'), class: 'fr-btn' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index 127d3f96f..7f771043f 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -20,24 +20,25 @@ .fusion.hidden %p= t('.title_fill_in_password') - = form_tag france_connect_particulier_merge_with_existing_account_path, data: { turbo: true }, class: 'mt-2 form fconnect-form' do + = form_tag france_connect_particulier_merge_using_password_path, data: { turbo: true }, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, @fci.merge_token, id: dom_id(@fci, :fusion_merge_token) - = hidden_field_tag :email, @fci.email_france_connect, id: dom_id(@fci, :fusion_email) .fr-input-group = label_tag :password, t('views.registrations.new.password_label', min_length: 8), class: 'fr-label' = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1 fr-input' - .mb-2 - = t('views.users.sessions.new.reset_password') - = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: @fci.merge_token), method: :post do - = t('.link_confirm_by_email') = submit_tag t('.button_merge'), class: 'fr-btn' + .mt-2 + = button_to t('.link_confirm_by_email'), + france_connect_particulier_send_email_merge_request_path, + params: { email: @fci.email_france_connect, merge_token: @fci.merge_token }, + class: 'fr-btn fr-btn--secondary' + .new-account.hidden %p= t('.title_fill_in_email', application_name: Current.application_name) - = form_tag france_connect_particulier_merge_with_new_account_path, data: { turbo: true }, class: 'mt-2 form' do + = form_tag france_connect_particulier_send_email_merge_request_path, class: 'mt-2 form' do = hidden_field_tag :merge_token, @fci.merge_token, id: dom_id(@fci, :new_account_merge_token) = label_tag :email, t('views.registrations.new.email_label'), for: dom_id(@fci, :new_account_email), class: 'fr-label' = email_field_tag :email, "", required: true, id: dom_id(@fci, :new_account_email), class: 'mb-1 fr-input' diff --git a/app/views/user_mailer/france_connect_merge_confirmation.haml b/app/views/user_mailer/france_connect_merge_confirmation.haml index 2f6c99a33..211703aa5 100644 --- a/app/views/user_mailer/france_connect_merge_confirmation.haml +++ b/app/views/user_mailer/france_connect_merge_confirmation.haml @@ -1,5 +1,5 @@ - content_for(:title, @subject) -- merge_link = france_connect_particulier_mail_merge_with_existing_account_url(email_merge_token: @email_merge_token) +- merge_link = france_connect_particulier_merge_using_email_link_url(email_merge_token: @email_merge_token) %p Bonjour, diff --git a/config/locales/en.yml b/config/locales/en.yml index 53ce483bd..9b6a17675 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -752,7 +752,6 @@ en: france_connect: connexion: "Error trying to connect to France Connect." forbidden_html: "Only citizen can use FranceConnect. As an instructor or administrator, you should reset your password." - unknown_error: "An error occured, please retry." evil_regexp: The regular expression you have entered is potentially dangerous and could lead to performance issues. mismatch_regexp: The provided example must match the regular expression syntax_error_regexp: The syntax of the regular expression is invalid @@ -920,7 +919,6 @@ en: user_not_found: "User not found" invalid_password: "The password is not correct." connection_done: "The accounts for FranceConnect and %{application_name} are now merged." - connection_done_verify_email: "The FranceConnect and %{application_name} accounts are now merged. Please confirm your email by clicking on the link you received in your email." merger_token_expired: "Le delay to merge your FranceConnect and %{application_name} accounts is expired. Please retry." shared: procedures: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 40ecb5527..9507716e3 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -757,7 +757,6 @@ fr: france_connect: connexion: "Erreur lors de la connexion à France Connect." forbidden_html: "Seul-e-s les usagers peuvent se connecter via France Connect. En tant qu’instructeur ou administrateur, nous vous invitons à réininitialiser votre mot de passe." - unknown_error: "Une erreure est survenue. Veuillez réessayer." evil_regexp: L'expression régulière que vous avez entrée est potentiellement dangereuse et pourrait entraîner des problèmes de performance mismatch_regexp: L'exemple doit correspondre à l'expression régulière fournie syntax_error_regexp: La syntaxe de l'expression régulière n'est pas valide @@ -975,7 +974,6 @@ fr: user_not_found: 'Utilisateur non trouvé' invalid_password: "Mot de passe incorrect" connection_done: "Les comptes FranceConnect et %{application_name} sont à présent fusionnés" - connection_done_verify_email: "Les comptes FranceConnect et %{application_name} sont maintenant fusionnés. Veuillez confirmer votre e-mail en cliquant sur le lien que vous avez reçu par mail" merger_token_expired: "Le délai pour fusionner les comptes FranceConnect et %{application_name} est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes." groupe_gestionnaires: flash: diff --git a/config/routes.rb b/config/routes.rb index 0a82f4c72..57031dc7c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -188,13 +188,14 @@ Rails.application.routes.draw do namespace :france_connect do get 'particulier' => 'particulier#login' get 'particulier/callback' => 'particulier#callback' - get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge - get 'particulier/mail_merge_with_existing_account/:email_merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account + + post 'particulier/send_email_merge_request' + + post 'particulier/merge_using_fc_email' + post 'particulier/merge_using_password' + get 'particulier/merge_using_email_link/:email_merge_token' => 'particulier#merge_using_email_link', as: :particulier_merge_using_email_link + get 'confirm_email/:token', to: 'particulier#confirm_email', as: :confirm_email - post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation - post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account' - post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account' - post 'particulier/associate_user' end namespace :agent_connect do diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 099d2d01f..add906511 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -98,16 +98,16 @@ describe FranceConnect::ParticulierController, type: :controller do context 'and an user with the same email exists' do let!(:preexisting_user) { create(:user, email: email) } - it 'redirects to the merge process' do + it 'renders the merge page' do expect { subject }.not_to change { User.count } - expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token)) + expect(response).to render_template(:merge) end end context 'and an instructeur with the same email exists' do let!(:preexisting_user) { create(:instructeur, email: email) } - it 'redirects to the merge process' do + it 'redirects to the login path' do expect { subject }.not_to change { User.count } expect(response).to redirect_to(new_user_session_path) @@ -136,82 +136,35 @@ describe FranceConnect::ParticulierController, type: :controller do end end - describe '#associate_user' do - subject { post :associate_user, params: { use_france_connect_email: use_france_connect_email, alternative_email: alternative_email, merge_token: merge_token } } + describe '#merge_using_fc_email' do + subject { post :merge_using_fc_email, params: { merge_token: merge_token } } - let(:fci) { FranceConnectInformation.new(user_info) } - let(:use_france_connect_email) { true } - let(:alternative_email) { 'alt@example.com' } - let(:merge_token) { 'valid_merge_token' } + let!(:fci) { FranceConnectInformation.create!(user_info) } + let(:merge_token) { fci.create_merge_token! } before do - allow_any_instance_of(ApplicationController).to receive(:session).and_return({ merge_token: merge_token }) + allow(UserMailer).to receive_message_chain(:custom_confirmation_instructions, :deliver_later) end - context 'when we are using france connect email' do - let(:fci) { instance_double('FranceConnectInformation') } - let(:email) { 'fc_email@example.com' } - let(:user) { instance_double('User') } - let(:destination_path) { '/some_path' } - let(:merge_token) { 'some_token' } + context 'when the merge token is valid' do + it do + expect(User.last.email).not_to eq(email.downcase) - before do - allow(controller).to receive(:securely_retrieve_fci).and_return(fci) - controller.instance_variable_set(:@fci, fci) - allow(fci).to receive(:email_france_connect).and_return(email) - allow(fci).to receive(:associate_user!) - allow(fci).to receive(:user).and_return(user) - allow(fci).to receive(:delete_merge_token!) - allow(fci).to receive(:send_custom_confirmation_instructions) - allow(controller).to receive(:use_fc_email?).and_return(true) - allow(controller).to receive(:sign_only) - allow(controller).to receive(:destination_path).and_return(destination_path) - end + subject - subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } } + user = User.last - context 'when association is successful' do - it 'renders the confirmation_sent template' do - subject - expect(response).to render_template(:confirmation_sent) - end - - it 'performs all expected steps' do - expect(fci).to receive(:associate_user!).with(email) - expect(fci).to receive(:send_custom_confirmation_instructions).with(user) - expect(fci).to receive(:delete_merge_token!) - expect(controller).to receive(:sign_only).with(user) - expect(controller).to receive(:render).with(:confirmation_sent, locals: { email: email, destination_path: destination_path }) - - subject - end - end - - context 'when association fails due to unknown error' do - let(:user) { User.new } - let(:error) { ActiveRecord::RecordInvalid.new(user) } - - before do - allow(fci).to receive(:associate_user!).and_raise(error) - allow(user.errors).to receive(:where).with(:email, :taken).and_return(nil) - end - - it 'redirects to new user session path with unknown error alert' do - subject - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.unknown_error')) - end + expect(user.email).to eq(email.downcase) + expect(UserMailer).to have_received(:custom_confirmation_instructions).with(user, user.confirmation_token) + expect(user.email_verified_at).to be_nil + expect(fci.reload.merge_token).to be_nil + expect(response).to render_template(:confirmation_sent) end end - context 'when france connect information is missing or invalid' do + context 'when the merge token is invalid' do let(:merge_token) { 'invalid_token' } - before do - allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(nil) - allow(controller).to receive(:merge_token_params).and_return(merge_token) - end - it 'redirects to root_path with an alert' do subject expect(response).to redirect_to(root_path) @@ -221,113 +174,24 @@ describe FranceConnect::ParticulierController, type: :controller do context 'when @fci is not valid for merge' do before do - allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) - allow(fci).to receive(:valid_for_merge?).and_return(false) + merge_token + fci.update!(merge_token_created_at: 2.years.ago) end it 'redirects to root_path with an alert' do subject + expect(response).to redirect_to(root_path) expect(flash[:alert]).to eq('Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes.') end end - - context 'when associating the user succeeds' do - let(:fci) { instance_double('FranceConnectInformation') } - let(:email) { 'user@example.com' } - let(:user) { instance_double('User', id: 1) } - let(:destination_path) { '/' } - let(:merge_token) { 'sample_merge_token' } - - before do - allow(controller).to receive(:securely_retrieve_fci) do - controller.instance_variable_set(:@fci, fci) - end - allow(fci).to receive(:email_france_connect).and_return(email) - allow(fci).to receive(:associate_user!) - allow(fci).to receive(:user).and_return(user) - allow(fci).to receive(:delete_merge_token!) - allow(fci).to receive(:send_custom_confirmation_instructions) - allow(controller).to receive(:use_fc_email?).and_return(true) - allow(controller).to receive(:sign_only) - allow(controller).to receive(:destination_path).and_return(destination_path) - end - - subject { post :associate_user, params: { merge_token: merge_token, use_france_connect_email: true } } - - it 'calls associate_user! with the correct email' do - expect(fci).to receive(:associate_user!).with(email) - subject - end - - it 'sends custom confirmation instructions' do - expect(fci).to receive(:send_custom_confirmation_instructions).with(user) - subject - end - - it 'deletes the merge token' do - expect(fci).to receive(:delete_merge_token!) - subject - end - - it 'signs in the user' do - expect(controller).to receive(:sign_only).with(user) - subject - end - - it 'renders the confirmation sent template with correct locals' do - expect(controller).to receive(:render).with( - :confirmation_sent, - locals: { email: email, destination_path: destination_path } - ) - subject - end - end - - context 'when associate_user uses an email of an existing user' do - let(:fci) { instance_double('FranceConnectInformation') } - let(:email) { 'user@example.com' } - let(:user) { instance_double('User', id: 1) } - let(:destination_path) { '/' } - let(:existing_user) { create(:user, email:) } - before do - invalid_user = build(:user, email:) - allow(FranceConnectInformation).to receive(:find_by).with(merge_token: merge_token).and_return(fci) - allow(fci).to receive(:valid_for_merge?).and_return(true) - allow(fci).to receive(:email_france_connect).and_return(email) - invalid_user.valid? - allow(fci).to receive(:associate_user!).and_raise(ActiveRecord::RecordInvalid.new(invalid_user)) - end - - it 'sends confirmation to existing user' do - expect(controller).to receive(:render).with( - :confirmation_sent, - locals: { email: email, destination_path: destination_path } - ) - expect(fci).to receive(:send_custom_confirmation_instructions).with(existing_user) - expect(fci).to receive(:delete_merge_token!) - subject - end - end end describe '#confirm_email' do let!(:user) { create(:user) } let!(:fci) { create(:france_connect_information, user: user) } - before do - sign_in(user) - fci.send_custom_confirmation_instructions(user) - user.reload - end - - let!(:expired_user_confirmation) do - user = create(:user) - fci = create(:france_connect_information, user: user) - token = SecureRandom.hex(10) - user.update!(confirmation_token: token, confirmation_sent_at: 3.days.ago) - user - end + before { fci.send_custom_confirmation_instructions } context 'when the confirmation token is valid' do before do @@ -335,20 +199,13 @@ describe FranceConnect::ParticulierController, type: :controller do user.reload end - it 'updates the email_verified_at and confirmation_token of the user' do + it do expect(user.email_verified_at).to be_present expect(user.confirmation_token).to be_nil - end - it 'redirects to the stored location or root path with a success notice' do expect(response).to redirect_to(root_path(user)) expect(flash[:notice]).to eq('Votre email est bien vérifié') end - - it 'calls after_confirmation on the user' do - expect(user).to receive(:after_confirmation).and_call_original - user.after_confirmation - end end context 'when invites are pending' do @@ -362,16 +219,10 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when the confirmation token is expired' do - let(:expired_user_confirmation) do + let!(:expired_user_confirmation) do create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago) end - before do - allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) - allow(controller).to receive(:user_signed_in?).and_return(false) - allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(nil) - end - it 'redirects to root path with an alert when FranceConnectInformation is not found' do get :confirm_email, params: { token: 'expired_token' } @@ -380,18 +231,20 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when FranceConnectInformation exists' do - let(:france_connect_information) { instance_double(FranceConnectInformation) } + let!(:france_connect_information) do + create(:france_connect_information, user: expired_user_confirmation) + end before do - allow(FranceConnectInformation).to receive(:find_by).with(user: expired_user_confirmation).and_return(france_connect_information) - allow(france_connect_information).to receive(:send_custom_confirmation_instructions) + allow(UserMailer).to receive_message_chain(:custom_confirmation_instructions, :deliver_later) end it 'resends the confirmation email and redirects to root path with a notice' do - expect(france_connect_information).to receive(:send_custom_confirmation_instructions).with(expired_user_confirmation) - get :confirm_email, params: { token: 'expired_token' } + expect(UserMailer).to have_received(:custom_confirmation_instructions) + .with(expired_user_confirmation, expired_user_confirmation.reload.confirmation_token) + expect(response).to redirect_to(root_path) expect(flash[:notice]).to eq(I18n.t('france_connect.particulier.flash.confirmation_mail_resent')) end @@ -399,46 +252,34 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when a different user is signed in' do - let(:expired_user_confirmation) do + let!(:expired_user_confirmation) do create(:user, confirmation_token: 'expired_token', confirmation_sent_at: 3.days.ago) end - let(:current_user) { create(:user) } - before do - allow(User).to receive(:find_by).with(confirmation_token: 'expired_token').and_return(expired_user_confirmation) - allow(controller).to receive(:user_signed_in?).and_return(true) - allow(controller).to receive(:current_user).and_return(current_user) - end + let(:another_user) { create(:user) } + + before { sign_in(another_user) } it 'signs out the current user and redirects to sign in path' do - expect(controller).to receive(:sign_out).with(current_user) + expect_any_instance_of(FranceConnectInformation).not_to receive(:send_custom_confirmation_instructions) + expect(controller).to receive(:sign_out).with(:user) get :confirm_email, params: { token: 'expired_token' } expect(response).to redirect_to(new_user_session_path) expect(flash[:alert]).to eq(I18n.t('france_connect.particulier.flash.redirect_new_user_session')) end - - it 'does not process the confirmation' do - expect(FranceConnectInformation).not_to receive(:find_by) - expect_any_instance_of(FranceConnectInformation).not_to receive(:send_custom_confirmation_instructions) - - get :confirm_email, params: { token: 'expired_token' } - end end end describe '#set_user_by_confirmation_token' do let(:current_user) { create(:user) } - let(:confirmation_user) { create(:user, confirmation_token: 'valid_token') } + let!(:confirmation_user) { create(:user, confirmation_token: 'valid_token') } - before do - sign_in current_user - allow(User).to receive(:find_by).with(confirmation_token: 'valid_token').and_return(confirmation_user) - end + before { sign_in current_user } it 'signs out current user and redirects to new session path when users do not match' do - expect(controller).to receive(:sign_out).with(current_user) + expect(controller).to receive(:sign_out).with(:user) get :confirm_email, params: { token: 'valid_token' } @@ -448,8 +289,6 @@ describe FranceConnect::ParticulierController, type: :controller do context 'when user is not found' do it 'redirects to root path with user not found alert' do - allow(User).to receive(:find_by).with(confirmation_token: 'invalid_token').and_return(nil) - get :confirm_email, params: { token: 'invalid_token' } expect(response).to redirect_to(root_path) @@ -478,41 +317,14 @@ describe FranceConnect::ParticulierController, type: :controller do end end - describe '#merge' do - let(:fci) { FranceConnectInformation.create!(user_info) } - let(:merge_token) { fci.create_merge_token! } - let(:format) { :html } - - subject { get :merge, params: { merge_token: merge_token } } - - context 'when the merge token is valid' do - it { expect(subject).to have_http_status(:ok) } - end - - it_behaves_like "a method that needs a valid merge token" - - context 'when the merge token does not exist' do - let(:merge_token) { 'i do not exist' } - - before do - allow(Current).to receive(:application_name).and_return('demarches-simplifiees.fr') - end - - it do - expect(subject).to redirect_to root_path - expect(flash.alert).to eq("Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expiré. Veuillez recommencer la procédure pour vous fusionner les comptes.") - end - end - end - - describe '#merge_with_existing_account' do + describe '#merge_using_password' do let(:fci) { FranceConnectInformation.create!(user_info) } let(:merge_token) { fci.create_merge_token! } let(:email) { 'EXISTING_account@a.com ' } let(:password) { SECURE_PASSWORD } let(:format) { :turbo_stream } - subject { post :merge_with_existing_account, params: { merge_token: merge_token, email: email, password: password }, format: format } + subject { post :merge_using_password, params: { merge_token: merge_token, password: password }, format: format } it_behaves_like "a method that needs a valid merge token" @@ -544,9 +356,9 @@ describe FranceConnect::ParticulierController, type: :controller do it 'redirects to the root page' do subject - fci.reload - expect(fci.user).to be_nil + expect { fci.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(fci.merge_token).not_to be_nil expect(controller.current_user).to be_nil end @@ -568,18 +380,21 @@ describe FranceConnect::ParticulierController, type: :controller do end end - describe '#mail_merge_with_existing_account' do + describe '#merge_using_email_link' do let(:fci) { FranceConnectInformation.create!(user_info) } let!(:email_merge_token) { fci.create_email_merge_token! } context 'when the merge_token is ok and the user is found' do - subject { post :mail_merge_with_existing_account, params: { email_merge_token: } } + subject do + post :merge_using_email_link, params: { email_merge_token: } + end before do allow(Current).to receive(:application_name).and_return('demarches-simplifiees.fr') + fci.update!(requested_email: email.downcase) end - let!(:user) { create(:user, email: email, password: 'abcdefgh') } + let!(:user) { create(:user, email:, password: 'abcdefgh') } it 'merges the account, signs in, and delete the merge token' do subject @@ -604,82 +419,26 @@ describe FranceConnect::ParticulierController, type: :controller do end end end - - context 'when the email_merge_token is not ok' do - subject { post :mail_merge_with_existing_account, params: { email_merge_token: 'ko' } } - - let!(:user) { create(:user, email: email) } - - it 'increases the failed attempts counter' do - subject - fci.reload - - expect(fci.user).to be_nil - expect(fci.email_merge_token).not_to be_nil - expect(controller.current_user).to be_nil - expect(response).to redirect_to(root_path) - end - end end - describe '#merge_with_new_account' do + describe '#send_email_merge_request' do let(:fci) { FranceConnectInformation.create!(user_info) } let(:merge_token) { fci.create_merge_token! } - let(:email) { ' Account@a.com ' } - let(:format) { :turbo_stream } + let(:email) { 'requested_email@.a.com' } - subject { post :merge_with_new_account, params: { merge_token: merge_token, email: email }, format: format } + subject { post :send_email_merge_request, params: { merge_token: merge_token, email: } } - it_behaves_like "a method that needs a valid merge token" - - context 'when the email does not belong to any user' do - it 'creates the account, signs in, and delete the merge token' do - subject - fci.reload - - expect(fci.user.email).to eq(email.downcase.strip) - expect(fci.merge_token).to be_nil - expect(controller.current_user).to eq(fci.user) - expect(response).to redirect_to(root_path) - end - end - - context 'when an account with the same email exists' do - let!(:user) { create(:user, email: email) } - - before { allow(controller).to receive(:sign_in).and_call_original } - - render_views - - it 'asks for the corresponding password' do - subject - fci.reload - - expect(fci.user).to be_nil - expect(fci.merge_token).not_to be_nil - expect(controller.current_user).to be_nil - - expect(response.body).to include('entrez votre mot de passe') - end - - it 'cannot use the merge token in the email confirmation route' do - subject - fci.reload - - get :mail_merge_with_existing_account, params: { email_merge_token: fci.merge_token } - expect(controller).not_to have_received(:sign_in) - expect(flash[:alert]).to be_present - end - end - end - - describe '#resend_and_renew_merge_confirmation' do - let(:fci) { FranceConnectInformation.create!(user_info) } - let(:merge_token) { fci.create_merge_token! } it 'renew token' do - expect { post :resend_and_renew_merge_confirmation, params: { merge_token: merge_token } }.to change { fci.reload.merge_token } + allow(UserMailer).to receive_message_chain(:france_connect_merge_confirmation, :deliver_later) + subject + + fci.reload + expect(fci.requested_email).to eq(email) expect(fci.email_merge_token).to be_present - expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token)) + + expect(UserMailer).to have_received(:france_connect_merge_confirmation).with(email, fci.email_merge_token, fci.email_merge_token_created_at) + + expect(response).to redirect_to(root_path) end end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 151907c72..c187c27d7 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -81,7 +81,7 @@ RSpec.describe UserMailer, type: :mailer do it 'sends to correct email with merge link' do expect(subject.to).to eq([email]) - expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(email_merge_token: code)) + expect(subject.body).to include(france_connect_particulier_merge_using_email_link_url(email_merge_token: code)) end context 'without SafeMailer configured' do diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index aaefe9da5..0afe00f8c 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -9,11 +9,11 @@ describe FranceConnectInformation, type: :model do end end - describe 'associate_user!' do + describe 'safely_associate_user!' do let(:email) { 'A@email.com' } let(:fci) { build(:france_connect_information) } - subject { fci.associate_user!(email) } + subject { fci.safely_associate_user!(email) } context 'when there is no user with the same email' do it 'creates a new user' do @@ -51,13 +51,13 @@ describe FranceConnectInformation, type: :model do end it 'raises an error' do - expect { fci.associate_user!(email) }.to raise_error(NoMethodError) + expect { fci.safely_associate_user!(email) }.to raise_error(NoMethodError) end it 'does not create a new user' do expect { begin - fci.associate_user!(email) + fci.safely_associate_user!(email) rescue NoMethodError end }.to_not change(User, :count) @@ -66,7 +66,7 @@ describe FranceConnectInformation, type: :model do it 'does not associate with any user' do expect(fci.user).to be_nil begin - fci.associate_user!(email) + fci.safely_associate_user!(email) rescue NoMethodError end expect(fci.reload.user).to be_nil diff --git a/spec/system/france_connect/france_connect_particulier_spec.rb b/spec/system/france_connect/france_connect_particulier_spec.rb index 60f1c2c91..cfea467cf 100644 --- a/spec/system/france_connect/france_connect_particulier_spec.rb +++ b/spec/system/france_connect/france_connect_particulier_spec.rb @@ -44,9 +44,7 @@ describe 'France Connect Particulier Connexion' do scenario 'he is redirected to user dossiers page' do expect(page).to have_content("Choisissez votre e-mail de contact") - expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) - find('#use_france_connect_email_yes').click - click_on 'Confirmer' + find('#use_fc_email').click expect(page).to have_content("Confirmation envoyée") click_on 'Continuer' expect(User.find_by(email: email)).not_to be nil @@ -54,16 +52,13 @@ describe 'France Connect Particulier Connexion' do scenario 'he can choose not to use FranceConnect email and input an alternative email' do expect(page).to have_content("Choisissez votre e-mail de contact") - expect(page).to have_selector("#use_france_connect_email_no", visible: false, wait: 10) - find('#use_france_connect_email_no').click - expect(page).to have_selector("input[name='alternative_email']", visible: true, wait: 10) + expect(page).to have_selector("input[name='email']", visible: true, wait: 10) - fill_in 'alternative_email', with: 'alternative@example.com' + fill_in 'email', with: 'alternative@example.com' click_on 'Confirmer' - expect(page).to have_content("Confirmation envoyée") - click_on 'Continuer' - expect(User.find_by(email: 'alternative@example.com')).not_to be nil + + expect(page).to have_content("Nous venons de vous envoyer le mail de confirmation") end end @@ -91,7 +86,7 @@ describe 'France Connect Particulier Connexion' do fill_in 'email', with: 'new_email@a.com' click_on 'Utiliser ce mail' - expect(page).to have_content('Dossiers') + expect(page).to have_content('Nous venons de vous envoyer le mail de confirmation') end context 'and the user wants an email that belongs to another account', js: true do @@ -107,10 +102,7 @@ describe 'France Connect Particulier Connexion' do click_on 'Utiliser ce mail' end - fill_in 'password-for-another-account', with: SECURE_PASSWORD - last_button = all('input[type="submit"][value="Fusionner les comptes"]').last.click - puts last_button.click - expect(page).to have_content('Dossiers') + expect(page).to have_content('Nous venons de vous envoyer le mail de confirmation') end end end diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index f00ffae34..6333f617e 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -192,11 +192,11 @@ describe 'Prefilling a dossier (with a GET request):', js: true do page.find('.fr-connect').click expect(page).to have_content("Choisissez votre e-mail de contact") - expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) - page.execute_script('document.getElementById("use_france_connect_email_yes").click()') + expect(page).to have_selector("#use_fc_email", visible: false, wait: 10) + page.execute_script('document.getElementById("use_fc_email").click()') - click_on 'Confirmer' expect(page).to have_content("Confirmation envoyée") + click_on 'Continuer' expect(page).to have_content('Vous avez un dossier prérempli') find('.fr-btn.fr-mb-2w', text: 'Poursuivre mon dossier prérempli', wait: 10).click From ce095479b73ca6f770331bacdb9a9f15b178ebe3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 31 Jul 2024 21:18:23 +0200 Subject: [PATCH 18/22] fix password reset with bad password scenario --- .../_password_confirmation.html.haml | 21 ++++++------------- .../particulier/merge.html.haml | 8 +------ .../merge_using_password.turbo_stream.haml | 1 + ...ge_with_existing_account.turbo_stream.haml | 0 .../merge_with_new_account.turbo_stream.haml | 4 ---- config/locales/en.yml | 4 ---- config/locales/fr.yml | 4 ---- 7 files changed, 8 insertions(+), 34 deletions(-) create mode 100644 app/views/france_connect/particulier/merge_using_password.turbo_stream.haml delete mode 100644 app/views/france_connect/particulier/merge_with_existing_account.turbo_stream.haml delete mode 100644 app/views/france_connect/particulier/merge_with_new_account.turbo_stream.haml diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index 3e23ee608..6a9ce4bbc 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -1,16 +1,7 @@ -%p - = t('.already_exists', email: email, application_name: Current.application_name) - %br - = t('.fill_in_password') += form_tag france_connect_particulier_merge_using_password_path, data: { turbo: true }, class: 'mt-2 form fconnect-form', id: 'merge_using_password' do + = hidden_field_tag :merge_token, fci.merge_token, id: dom_id(fci, :fusion_merge_token) + .fr-input-group{ class: class_names('fr-input-group--error': wrong_password) } + = label_tag :password, t('views.registrations.new.password_label', min_length: 8), class: 'fr-label' + = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1 fr-input' -= form_tag france_connect_particulier_merge_with_existing_account_path, data: { turbo: true, turbo_force: :server }, class: 'mt-2 form fconnect-form' do - = hidden_field_tag :merge_token, merge_token - = hidden_field_tag :email, email - = label_tag :password, t('views.registrations.new.password_label', min_length: 8) - = password_field_tag :password, nil, autocomplete: 'current-password', id: 'password-for-another-account' - .mb-2 - = t('views.users.sessions.new.reset_password') - = link_to france_connect_particulier_send_email_merge_path(merge_token: merge_token), method: :post do - = t('france_connect.particulier.merge.link_confirm_by_email') - = button_tag t('.back'), type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' - = submit_tag t('france_connect.particulier.merge.button_merge'), class: 'button primary' + = submit_tag t('france_connect.particulier.merge.button_merge'), class: 'fr-btn' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index 7f771043f..c96f9b53e 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -20,13 +20,7 @@ .fusion.hidden %p= t('.title_fill_in_password') - = form_tag france_connect_particulier_merge_using_password_path, data: { turbo: true }, class: 'mt-2 form fconnect-form' do - = hidden_field_tag :merge_token, @fci.merge_token, id: dom_id(@fci, :fusion_merge_token) - .fr-input-group - = label_tag :password, t('views.registrations.new.password_label', min_length: 8), class: 'fr-label' - = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1 fr-input' - - = submit_tag t('.button_merge'), class: 'fr-btn' + = render partial: 'password_confirmation', locals: { fci: @fci, wrong_password: @wrong_password } .mt-2 = button_to t('.link_confirm_by_email'), diff --git a/app/views/france_connect/particulier/merge_using_password.turbo_stream.haml b/app/views/france_connect/particulier/merge_using_password.turbo_stream.haml new file mode 100644 index 000000000..d3985f5c2 --- /dev/null +++ b/app/views/france_connect/particulier/merge_using_password.turbo_stream.haml @@ -0,0 +1 @@ += turbo_stream.replace('merge_using_password', partial: 'password_confirmation', locals: { fci: @fci, wrong_password: true }) diff --git a/app/views/france_connect/particulier/merge_with_existing_account.turbo_stream.haml b/app/views/france_connect/particulier/merge_with_existing_account.turbo_stream.haml deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/views/france_connect/particulier/merge_with_new_account.turbo_stream.haml b/app/views/france_connect/particulier/merge_with_new_account.turbo_stream.haml deleted file mode 100644 index 7d14ef01a..000000000 --- a/app/views/france_connect/particulier/merge_with_new_account.turbo_stream.haml +++ /dev/null @@ -1,4 +0,0 @@ -= turbo_stream.update 'new-account-password-confirmation', partial: 'password_confirmation', locals: { email: @email, merge_token: @merge_token } -= turbo_stream.hide_all '.fusion' -= turbo_stream.hide_all '.new-account' -= turbo_stream.show 'new-account-password-confirmation' diff --git a/config/locales/en.yml b/config/locales/en.yml index 9b6a17675..e1fe7e4c6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -897,10 +897,6 @@ en: intro: "A confirmation email has been sent to your address %{email}" click_the_link_in_the_email: "Please click the link in the email to confirm your account and connect with France Connect in the future." continue: "Continue" - password_confirmation: - back: 'back to previous step' - already_exists: An account with %{email} already existis on %{application_name} - fill_in_password: fill in your password to merge your accounts merge: title: "Merge your account FranceConnect and %{application_name}" subtitle_html: "Hello,

Your account FranceConnect uses %{email} as contact email.
But there is an existing %{application_name} account using this email." diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 9507716e3..34e1f9bd7 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -952,10 +952,6 @@ fr: intro: Un mail de confirmation a été envoyé à votre adresse %{email} click_the_link_in_the_email: Vous devez impérativement cliquer sur le lien du mail pour activer votre adresse et recevoir les notifications sur l'avancement de vos dossiers. continue: Continuer - password_confirmation: - back: 'revenir en arrière' - already_exists: Le compte %{email} existe déjà sur %{application_name} - fill_in_password: entrez votre mot de passe pour fusionner les comptes merge: title: "Fusion des comptes FranceConnect et %{application_name}" subtitle_html: "Bonjour,

Votre compte FranceConnect utilise %{email} comme email de contact.
Or il existe un compte sur %{application_name} avec cet email." From 08fb6c856e0caa785bdc9e97b48b793593daf10d Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 12 Aug 2024 09:48:14 +0000 Subject: [PATCH 19/22] Fix choose email screen --- .../email_france_connect_controller.ts | 5 ++-- .../particulier/choose_email.html.haml | 27 +++++++++++-------- .../particulier/confirmation_sent.html.haml | 2 +- config/locales/en.yml | 9 ++++--- config/locales/fr.yml | 11 ++++---- 5 files changed, 30 insertions(+), 24 deletions(-) diff --git a/app/javascript/controllers/email_france_connect_controller.ts b/app/javascript/controllers/email_france_connect_controller.ts index 6512c4982..1321baea0 100644 --- a/app/javascript/controllers/email_france_connect_controller.ts +++ b/app/javascript/controllers/email_france_connect_controller.ts @@ -3,7 +3,7 @@ import { ApplicationController } from './application_controller'; export class EmailFranceConnectController extends ApplicationController { static targets = ['useFranceConnectEmail', 'emailField']; - emailFieldTarget!: HTMLInputElement; + emailFieldTarget!: HTMLElement; useFranceConnectEmailTargets!: HTMLInputElement[]; connect() { @@ -16,13 +16,12 @@ export class EmailFranceConnectController extends ApplicationController { ); const inputElement = this.emailFieldTarget.querySelector( - 'input' + 'input[type="email"]' ) as HTMLInputElement; if (checkedTarget && checkedTarget.value === 'false') { this.emailFieldTarget.classList.remove('fr-hidden'); inputElement.setAttribute('required', ''); - this.emailFieldTarget.required = true; } else { this.emailFieldTarget.classList.add('fr-hidden'); inputElement.removeAttribute('required'); diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index 2ef702263..5ca731009 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -1,26 +1,31 @@ .fr-container %h1.text-center.mt-1= t('.choose_email_contact') - %p= t('.greetings') - %p= t('.intro_html', email: @fci.email_france_connect) %p= t('.use_email_for_notifications') - = button_to t('utils.yes'), - france_connect_particulier_merge_using_fc_email_path, - params: { merge_token: @fci.merge_token }, - class: 'fr-btn', - id: 'use_fc_email' - .fr-fieldset.fr-w-30v.fr-mt-2w - = form_with url: france_connect_particulier_send_email_merge_request_path do |f| + = form_with url: france_connect_particulier_merge_using_fc_email_path(merge_token: @fci.merge_token), method: :post, data: { controller: 'email-france-connect' } do |f| = hidden_field_tag :merge_token, @fci.merge_token - .fr-fieldset__element.fr-fieldset__element--inline{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } + %fieldset.fr-fieldset + %legend.fr-fieldset__legend + .fr-fieldset__element + .fr-radio-group + = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } + %label.fr-label.fr-text--wrap{ for: 'use_france_connect_email_yes' } + = t('.keep_fc_email_html', email: @fci.email_france_connect).html_safe + .fr-fieldset__element + .fr-radio-group + = f.radio_button :use_france_connect_email, false, id: 'use_france_connect_email_no', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } + %label.fr-label.fr-text--wrap{ for: 'use_france_connect_email_no' } + = t('.use_another_email') + + .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } = f.label :email, t('.alternative_email'), class: "fr-label" %span.fr-hint-text.mb-1= t('activerecord.attributes.user.hints.email') - = f.email_field :email, class: "fr-input", data: { action: "blur->email-input#checkEmail", 'email-input-target': 'input' } + = f.email_field :email, class: "fr-input" .suspect-email.hidden{ data: { "email-input-target": 'ariaRegion'}, aria: { live: 'off' } } = render Dsfr::AlertComponent.new(title: t('utils.email_suggest.wanna_say'), state: :info, heading_level: :div) do |c| diff --git a/app/views/france_connect/particulier/confirmation_sent.html.haml b/app/views/france_connect/particulier/confirmation_sent.html.haml index 23ef74732..dcd907b61 100644 --- a/app/views/france_connect/particulier/confirmation_sent.html.haml +++ b/app/views/france_connect/particulier/confirmation_sent.html.haml @@ -6,7 +6,7 @@ = render Dsfr::AlertComponent.new(title: '', state: :info, heading_level: 'h2', extra_class_names: 'fr-mt-6w fr-mb-3w') do |c| - c.with_body do - %p= t('.intro', email: email) + %p= t('.intro_html', email: email).html_safe %p= t('.click_the_link_in_the_email') %p.center= link_to t('.continue'), destination_path, class: 'fr-btn' diff --git a/config/locales/en.yml b/config/locales/en.yml index e1fe7e4c6..7ff258eb5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -886,15 +886,16 @@ en: france_connect: particulier: choose_email: - intro_html: "Your FranceConnect account uses %{email}r as the contact email." + intro_html: "Your FranceConnect account uses %{email} as the contact email." use_email_for_notifications: "Would you like to use it to receive notifications regarding the progress of your cases?" confirm: "Confirm" choose_email_contact: "Choose your contact email" alternative_email: "Please provide the email to use for contacting you." - greetings: "Hello," + keep_fc_email_html: Yes, use %{email} as contact email. + use_another_email: No, use another address. confirmation_sent: - confirmation_sent_by_email: "Confirmation sent by email" - intro: "A confirmation email has been sent to your address %{email}" + confirmation_sent_by_email: "Confirm your email" + intro_html: "A confirmation email has been sent to your address %{email}" click_the_link_in_the_email: "Please click the link in the email to confirm your account and connect with France Connect in the future." continue: "Continue" merge: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 34e1f9bd7..f51fbe328 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -939,17 +939,18 @@ fr: france_connect: particulier: choose_email: - intro_html: "Votre compte FranceConnect utilise %{email} comme email de contact." + intro_html: "Votre compte FranceConnect utilise %{email} comme email de contact." use_email_for_notifications: Souhaitez-vous l'utiliser pour recevoir les notifications concernant l'avancement de vos dossiers ? confirm: Confirmer - choose_email_contact: Choisissez votre e-mail de contact + choose_email_contact: Choisissez votre email de contact pour finaliser votre connexion alternative_email: Veuillez nous fournir l'email à utiliser pour vous contacter. - greetings: Bonjour, + keep_fc_email_html: "Oui, utiliser %{email} comme email de contact." + use_another_email: Non, utiliser une autre adresse. email_suggest: wanna_say: 'Voulez-vous dire ?' confirmation_sent: - confirmation_sent_by_email: Confirmation envoyée par mail - intro: Un mail de confirmation a été envoyé à votre adresse %{email} + confirmation_sent_by_email: Confirmez votre email + intro_html: Un mail de confirmation a été envoyé à votre adresse %{email} click_the_link_in_the_email: Vous devez impérativement cliquer sur le lien du mail pour activer votre adresse et recevoir les notifications sur l'avancement de vos dossiers. continue: Continuer merge: From ff88a0a2a6c982917b86006b43ffdeaa17e05c40 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Thu, 22 Aug 2024 21:13:41 +0000 Subject: [PATCH 20/22] Fix add requested_email column to france_connect_information table Lint error --- app/views/france_connect/particulier/choose_email.html.haml | 2 +- .../france_connect/particulier/confirmation_sent.html.haml | 2 +- config/locales/fr.yml | 2 +- ...ernative_email_column_to_france_connect_information_table.rb | 2 ++ 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index 5ca731009..6bf3def08 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -15,7 +15,7 @@ .fr-radio-group = f.radio_button :use_france_connect_email, true, id: 'use_france_connect_email_yes', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } %label.fr-label.fr-text--wrap{ for: 'use_france_connect_email_yes' } - = t('.keep_fc_email_html', email: @fci.email_france_connect).html_safe + = t('.keep_fc_email_html', email: h(@fci.email_france_connect)).html_safe .fr-fieldset__element .fr-radio-group = f.radio_button :use_france_connect_email, false, id: 'use_france_connect_email_no', class: 'fr-radio', required: true, data: { action: "email-france-connect#triggerEmailField", email_france_connect_target: "useFranceConnectEmail" } diff --git a/app/views/france_connect/particulier/confirmation_sent.html.haml b/app/views/france_connect/particulier/confirmation_sent.html.haml index dcd907b61..f81684326 100644 --- a/app/views/france_connect/particulier/confirmation_sent.html.haml +++ b/app/views/france_connect/particulier/confirmation_sent.html.haml @@ -6,7 +6,7 @@ = render Dsfr::AlertComponent.new(title: '', state: :info, heading_level: 'h2', extra_class_names: 'fr-mt-6w fr-mb-3w') do |c| - c.with_body do - %p= t('.intro_html', email: email).html_safe + %p= t('.intro_html', email: h(email)).html_safe %p= t('.click_the_link_in_the_email') %p.center= link_to t('.continue'), destination_path, class: 'fr-btn' diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f51fbe328..5e1b235ea 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -950,7 +950,7 @@ fr: wanna_say: 'Voulez-vous dire ?' confirmation_sent: confirmation_sent_by_email: Confirmez votre email - intro_html: Un mail de confirmation a été envoyé à votre adresse %{email} + intro_html: "Un mail de confirmation a été envoyé à votre adresse %{email}" click_the_link_in_the_email: Vous devez impérativement cliquer sur le lien du mail pour activer votre adresse et recevoir les notifications sur l'avancement de vos dossiers. continue: Continuer merge: diff --git a/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb b/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb index 0ff086128..8f0ca39c1 100644 --- a/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb +++ b/db/migrate/20240730130933_add_alternative_email_column_to_france_connect_information_table.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddAlternativeEmailColumnToFranceConnectInformationTable < ActiveRecord::Migration[7.0] def change add_column :france_connect_informations, :requested_email, :string From 0b4a9bb3bbff2bec2433614774877366ae40a005 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Thu, 22 Aug 2024 21:36:16 +0000 Subject: [PATCH 21/22] fixe les tests suite au changement de wording sur les layouts --- .../france_connect_particulier_spec.rb | 12 +++++++----- spec/system/users/dossier_prefill_get_spec.rb | 12 ++++++------ spec/system/users/dossier_prefill_post_spec.rb | 4 ++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/spec/system/france_connect/france_connect_particulier_spec.rb b/spec/system/france_connect/france_connect_particulier_spec.rb index cfea467cf..e6fdb5786 100644 --- a/spec/system/france_connect/france_connect_particulier_spec.rb +++ b/spec/system/france_connect/france_connect_particulier_spec.rb @@ -43,22 +43,24 @@ describe 'France Connect Particulier Connexion' do before { page.find('.fr-connect').click } scenario 'he is redirected to user dossiers page' do - expect(page).to have_content("Choisissez votre e-mail de contact") - find('#use_fc_email').click - expect(page).to have_content("Confirmation envoyée") + expect(page).to have_content("Choisissez votre email de contact pour finaliser votre connexion") + find("#use_france_connect_email_no").click + fill_in("email", with: "exemple@email.com") + page.find("input[type='submit'][name='commit'][value='Confirmer']").click + expect(page).to have_content("Confirmez votre email") click_on 'Continuer' expect(User.find_by(email: email)).not_to be nil end scenario 'he can choose not to use FranceConnect email and input an alternative email' do - expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_content("Choisissez votre email de contact pour finaliser votre connexion") expect(page).to have_selector("input[name='email']", visible: true, wait: 10) fill_in 'email', with: 'alternative@example.com' click_on 'Confirmer' - expect(page).to have_content("Nous venons de vous envoyer le mail de confirmation") + expect(page).to have_content("Confirmez votre email") end end diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index 6333f617e..a53d50a23 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -191,12 +191,12 @@ describe 'Prefilling a dossier (with a GET request):', js: true do page.find('.fr-connect').click - expect(page).to have_content("Choisissez votre e-mail de contact") - expect(page).to have_selector("#use_fc_email", visible: false, wait: 10) - page.execute_script('document.getElementById("use_fc_email").click()') - - expect(page).to have_content("Confirmation envoyée") - + expect(page).to have_content("Choisissez votre email de contact pour finaliser votre connexion") + expect(page).to have_selector("#use_france_connect_email_no", visible: false, wait: 10) + page.execute_script('document.getElementById("use_france_connect_email_no").click()') + fill_in("email", with: "exemple@email.com") + page.find("input[type='submit'][name='commit'][value='Confirmer']").click + expect(page).to have_content("Confirmez votre email") click_on 'Continuer' expect(page).to have_content('Vous avez un dossier prérempli') find('.fr-btn.fr-mb-2w', text: 'Poursuivre mon dossier prérempli', wait: 10).click diff --git a/spec/system/users/dossier_prefill_post_spec.rb b/spec/system/users/dossier_prefill_post_spec.rb index 0ca1baf1c..9ae750962 100644 --- a/spec/system/users/dossier_prefill_post_spec.rb +++ b/spec/system/users/dossier_prefill_post_spec.rb @@ -142,12 +142,12 @@ describe 'Prefilling a dossier (with a POST request):', js: true do allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(build(:france_connect_information)) page.find('.fr-connect').click - expect(page).to have_content("Choisissez votre e-mail de contact") + expect(page).to have_content("Choisissez votre email de contact pour finaliser votre connexion") expect(page).to have_selector("#use_france_connect_email_yes", visible: false, wait: 10) page.execute_script('document.getElementById("use_france_connect_email_yes").click()') click_on 'Confirmer' - expect(page).to have_content("Confirmation envoyée") + expect(page).to have_content("Confirmez votre email") click_on 'Continuer' expect(page).to have_content('Vous avez un dossier prérempli') find('.fr-btn.fr-mb-2w', text: 'Poursuivre mon dossier prérempli', wait: 10).click From 07ea31fc4c1afad4553333c7cb8e7075a0f60fdc Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 28 Aug 2024 09:12:46 +0000 Subject: [PATCH 22/22] Add aria accessibility regarding the hidden class on partial choose email --- .../controllers/email_france_connect_controller.ts | 6 ++++-- app/views/france_connect/particulier/choose_email.html.haml | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/email_france_connect_controller.ts b/app/javascript/controllers/email_france_connect_controller.ts index 1321baea0..d9be703af 100644 --- a/app/javascript/controllers/email_france_connect_controller.ts +++ b/app/javascript/controllers/email_france_connect_controller.ts @@ -20,10 +20,12 @@ export class EmailFranceConnectController extends ApplicationController { ) as HTMLInputElement; if (checkedTarget && checkedTarget.value === 'false') { - this.emailFieldTarget.classList.remove('fr-hidden'); + this.emailFieldTarget.classList.remove('hidden'); + this.emailFieldTarget.setAttribute('aria-hidden', 'false'); inputElement.setAttribute('required', ''); } else { - this.emailFieldTarget.classList.add('fr-hidden'); + this.emailFieldTarget.classList.add('hidden'); + this.emailFieldTarget.setAttribute('aria-hidden', 'true'); inputElement.removeAttribute('required'); inputElement.value = ''; } diff --git a/app/views/france_connect/particulier/choose_email.html.haml b/app/views/france_connect/particulier/choose_email.html.haml index 6bf3def08..d1e6d9467 100644 --- a/app/views/france_connect/particulier/choose_email.html.haml +++ b/app/views/france_connect/particulier/choose_email.html.haml @@ -22,7 +22,7 @@ %label.fr-label.fr-text--wrap{ for: 'use_france_connect_email_no' } = t('.use_another_email') - .fr-fieldset__element.fr-fieldset__element--inline.fr-hidden{ data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } + .fr-fieldset__element.fr-fieldset__element--inline.hidden{ aria: { hidden: true }, data: { email_france_connect_target: "emailField", controller: 'email-input', email_input_url_value: show_email_suggestions_path } } = f.label :email, t('.alternative_email'), class: "fr-label" %span.fr-hint-text.mb-1= t('activerecord.attributes.user.hints.email') = f.email_field :email, class: "fr-input"