From eaef5c7e39f71707dd7be60d09bdd1df08375be9 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 31 Jul 2024 17:50:14 +0200 Subject: [PATCH] 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