From ff073f8884af40a43c292f633a4954e927e83bb9 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 17 Nov 2021 16:21:55 +0100 Subject: [PATCH] Add confirmation by email when merging DC/FC accounts feat(fci.confirmation_code): add confirmation code to france_connect_informations feat(user_mailer.france_connect_confirmation_code): add confirmation by email mail method/preview/spec, pointing to merge_mail_with_existing_account (reuse existing method) feat(mail_merge): mail merge feat(merge.cannot_use_france_connect): same behaviour as callback clean(fci.confirmation_code): use same token for mail validation as merge feat(resend_france_connect/particulier/merge_confirmation): resend email with link. also enhance some trads, cleanup halfy finished refacto clean(tech): finalize story by plugging merge_with_new_account to email validation fix(deadspec): was removed fix(spec): broken after last refactoring lint(rubocop): space before parenthesis lint(haml-lint): yoohoooo space before = fix(lint): scss now :D Update app/assets/stylesheets/buttons.scss cleanup feat(france_connect): re-add confirm by email, with an option for confirmation by email instead of only confirmation by email fixup! Add confirmation by email when merging DC/FC accounts fix(lint): haml_spec failure --- app/assets/stylesheets/forms.scss | 6 ++ .../france_connect/particulier_controller.rb | 30 +++++++-- app/mailers/user_mailer.rb | 7 +++ .../_password_confirmation.html.haml | 6 +- .../particulier/merge.html.haml | 16 +++-- .../france_connect_merge_confirmation.haml | 20 ++++++ config/routes.rb | 2 + .../particulier_controller_spec.rb | 61 ++++++++++++++++++- spec/mailers/previews/user_mailer_preview.rb | 4 ++ spec/mailers/user_mailer_spec.rb | 10 +++ vendor/assets/stylesheets/franceconnect.scss | 1 + 11 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 app/views/user_mailer/france_connect_merge_confirmation.haml diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index c815ac74d..a5c34814e 100644 --- a/app/assets/stylesheets/forms.scss +++ b/app/assets/stylesheets/forms.scss @@ -556,3 +556,9 @@ [data-reach-combobox-popover] { z-index: 20; } + +.fconnect-form { + input[type=password] { + margin-bottom: 16px; + } +} diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 655ab2cf2..ba20e2943 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -1,6 +1,6 @@ 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] + before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :mail_merge_with_existing_account, :resend_and_renew_merge_confirmation] def login if FranceConnectService.enabled? @@ -20,7 +20,8 @@ class FranceConnect::ParticulierController < ApplicationController fci.associate_user!(fci.email_france_connect) connect_france_connect_particulier(fci.user) else - redirect_to france_connect_particulier_merge_path(fci.create_merge_token!) + merge_token = fci.create_merge_token! + redirect_to france_connect_particulier_merge_path(merge_token) end else user = fci.user @@ -28,7 +29,7 @@ class FranceConnect::ParticulierController < ApplicationController if user.can_france_connect? fci.update(updated_at: Time.zone.now) connect_france_connect_particulier(user) - else + 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 @@ -64,6 +65,20 @@ class FranceConnect::ParticulierController < ApplicationController end end + def mail_merge_with_existing_account + user = User.find_by(email: @fci.email_france_connect.downcase) + if user.can_france_connect? + @fci.update(user: user) + @fci.delete_merge_token! + + flash.notice = "Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés" + 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) @@ -79,13 +94,20 @@ class FranceConnect::ParticulierController < ApplicationController end end + def resend_and_renew_merge_confirmation + merge_token = @fci.create_merge_token! + UserMailer.france_connect_merge_confirmation(@fci.email_france_connect, merge_token).deliver_later + redirect_to france_connect_particulier_merge_path(merge_token), + notice: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" + end + private def securely_retrieve_fci @fci = FranceConnectInformation.find_by(merge_token: merge_token_params) if @fci.nil? || !@fci.valid_for_merge? - flash.alert = 'Votre compte FranceConnect a expiré, veuillez recommencer.' + flash.alert = "Le délai pour fusionner les comptes FranceConnect et #{APPLICATION_NAME} est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes." respond_to do |format| format.html { redirect_to root_path } diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index e5b143566..72db2f384 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -20,6 +20,13 @@ class UserMailer < ApplicationMailer mail(to: requested_email, subject: @subject) end + def france_connect_merge_confirmation(email, merge_token) + @merge_token = merge_token + @subject = "Veuillez confirmer la fusion de compte" + + mail(to: email, subject: @subject) + end + def invite_instructeur(user, reset_password_token) @reset_password_token = reset_password_token @user = user diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index 0bc209dff..ce3cabbf5 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -3,10 +3,14 @@ %br entrez votre mot de passe pour fusionner les comptes -= form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form' do += form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, merge_token = hidden_field_tag :email, email = label_tag :password, 'Mot de passe (8 caractères minimum)' = password_field_tag :password, nil, autocomplete: 'current-password', id: 'password-for-another-account' + .mb-2 + Mot de passe oublié ? + = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: merge_token), method: :post do + Confirmer mon compte par mail = button_tag 'revenir en arrière', type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' = submit_tag 'Fusionner les comptes', class: 'button primary' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index c8f4ae656..3c10ffd73 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -23,22 +23,30 @@ Non .fusion.hidden - %p Pour les fusionner, entrez votre mot de passe + %p Pour fusionner ces comptes, veuillez cliquer sur le lien présent dans le mail que nous venons de vous envoyer. - = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form' do + = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, @fci.merge_token = hidden_field_tag :email, @fci.email_france_connect + = label_tag :password, 'Mot de passe (8 caractères minimum)' - = password_field_tag :password, nil, autocomplete: 'current-password' + = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1' + .mb-2 + Mot de passe oublié ? + = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: @fci.merge_token), method: :post do + Confirmer mon compte par mail + = submit_tag 'Fusionner les comptes', class: 'button primary' + .new-account.hidden %p Donnez-nous alors le mail que #{APPLICATION_NAME} utilisera pour vous contacter = form_tag france_connect_particulier_merge_with_new_account_path, remote: true, class: 'mt-2 form' do = hidden_field_tag :merge_token, @fci.merge_token = label_tag :email, 'Email (nom@site.com)' - = email_field_tag :email + = email_field_tag :email, "", required: true = submit_tag 'Utiliser ce mail', class: 'button primary' + .new-account-password-confirmation.hidden diff --git a/app/views/user_mailer/france_connect_merge_confirmation.haml b/app/views/user_mailer/france_connect_merge_confirmation.haml new file mode 100644 index 000000000..387d72749 --- /dev/null +++ b/app/views/user_mailer/france_connect_merge_confirmation.haml @@ -0,0 +1,20 @@ +- content_for(:title, @subject) + +%p + Bonjour, + +%p + Pour confirmer la fusion de votre compte, veuillez cliquer sur le lien suivant : += round_button 'Je confirme', france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), :primary + +%p + Vous pouvez aussi visiter ce lien : #{link_to france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token)} + +%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}. + +%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" diff --git a/config/routes.rb b/config/routes.rb index 3b4812185..51117675b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -125,6 +125,8 @@ Rails.application.routes.draw 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/:merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account + 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' end diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 92f342022..fb93d508c 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -150,7 +150,7 @@ describe FranceConnect::ParticulierController, type: :controller do else expect(subject).to redirect_to root_path end - expect(flash.alert).to eq('Votre compte FranceConnect a expiré, veuillez recommencer.') + expect(flash.alert).to eq('Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes.') end end end @@ -173,7 +173,7 @@ describe FranceConnect::ParticulierController, type: :controller do it do expect(subject).to redirect_to root_path - expect(flash.alert).to eq('Votre compte FranceConnect a expiré, veuillez recommencer.') + expect(flash.alert).to eq("Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes.") end end end @@ -241,6 +241,54 @@ describe FranceConnect::ParticulierController, type: :controller do end end + describe '#mail_merge_with_existing_account' do + let(:fci) { FranceConnectInformation.create!(user_info) } + let!(:merge_token) { fci.create_merge_token! } + + context 'when the merge_token is ok and the user is found' do + subject { post :mail_merge_with_existing_account, params: { merge_token: fci.merge_token } } + + let!(:user) { create(:user, email: email, password: 'abcdefgh') } + + it 'merges the account, signs in, and delete the merge token' do + subject + fci.reload + + expect(fci.user).to eq(user) + expect(fci.merge_token).to be_nil + expect(controller.current_user).to eq(user) + end + + context 'but the targeted user is an instructeur' do + let!(:user) { create(:instructeur, email: email, password: 'abcdefgh').user } + + it 'redirects to the new session' do + subject + expect(FranceConnectInformation.exists?(fci.id)).to be_falsey + expect(controller.current_user).to be_nil + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path)) + end + end + end + + context 'when the merge_token is not ok' do + subject { post :mail_merge_with_existing_account, params: { 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.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 let(:fci) { FranceConnectInformation.create!(user_info) } let(:merge_token) { fci.create_merge_token! } @@ -280,4 +328,13 @@ describe FranceConnect::ParticulierController, type: :controller do 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 } + expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token)) + end + end end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 436ac435f..fc9782218 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -12,6 +12,10 @@ class UserMailerPreview < ActionMailer::Preview UserMailer.ask_for_merge(user, 'dircab@territoires.gouv.fr') end + def france_connect_merge_confirmation + UserMailer.france_connect_merge_confirmation('new.exemple.fr', '123456') + end + private def user diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 88f4fa2dc..6ea3d81ae 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -25,4 +25,14 @@ RSpec.describe UserMailer, type: :mailer do it { expect(subject.to).to eq([requested_email]) } it { expect(subject.body).to include(requested_email) } end + + describe '.france_connect_merge_confirmation' do + let(:email) { 'new.exemple.fr' } + let(:code) { '123456' } + + subject { described_class.france_connect_merge_confirmation(email, code) } + + it { expect(subject.to).to eq([email]) } + it { expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(merge_token: code)) } + end end diff --git a/vendor/assets/stylesheets/franceconnect.scss b/vendor/assets/stylesheets/franceconnect.scss index 5a74703d4..bddbfbd26 100644 --- a/vendor/assets/stylesheets/franceconnect.scss +++ b/vendor/assets/stylesheets/franceconnect.scss @@ -140,3 +140,4 @@ height: 500px; margin: 60px auto 0 auto; } +