From 29fff9ee686b59a0546bdf487db54ba6d411658a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 10 Jan 2019 10:41:03 +0100 Subject: [PATCH 1/3] [fix #3269] bufferize login token email --- app/controllers/users/sessions_controller.rb | 11 +++- app/models/gestionnaire.rb | 6 ++ .../users/sessions_controller_spec.rb | 57 +++++++++++++------ spec/models/gestionnaire_spec.rb | 21 +++++++ 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 477cdc85e..06a7714d8 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -30,8 +30,8 @@ class Users::SessionsController < Sessions::SessionsController redirect_to after_sign_in_path_for(:user) else gestionnaire = current_gestionnaire - login_token = gestionnaire.login_token! - GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later + + send_login_token_or_bufferize(gestionnaire) [:user, :gestionnaire, :administrateur].each { |role| sign_out(role) } @@ -103,6 +103,13 @@ class Users::SessionsController < Sessions::SessionsController private + def send_login_token_or_bufferize(gestionnaire) + if !gestionnaire.young_login_token? + login_token = gestionnaire.login_token! + GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later + end + end + def error_procedure session["user_return_to"] = nil flash.alert = t('errors.messages.procedure_not_found') diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 9b6a8ece8..b62fee78d 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -4,6 +4,7 @@ class Gestionnaire < ApplicationRecord include ActiveRecord::SecureToken LOGIN_TOKEN_VALIDITY = 30.minutes + LOGIN_TOKEN_YOUTH = 15.minutes devise :database_authenticatable, :registerable, :async, :recoverable, :rememberable, :trackable, :validatable @@ -211,6 +212,11 @@ class Gestionnaire < ApplicationRecord save end + def young_login_token? + login_token_created_at.present? && + LOGIN_TOKEN_YOUTH.ago < login_token_created_at + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c72a70c33..a5ef7a201 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -17,6 +17,10 @@ describe Users::SessionsController, type: :controller do before do allow(controller).to receive(:trusted_device?).and_return(trusted_device) + allow(GestionnaireMailer).to receive(:send_login_token).and_return(double(deliver_later: true)) + end + + subject do post :create, params: { user: { email: email, password: send_password } } user.reload end @@ -25,28 +29,47 @@ describe Users::SessionsController, type: :controller do let(:trusted_device) { false } it 'redirects to the confirmation link path' do - expect(subject).to redirect_to link_sent_path(email: email) + subject + + expect(controller).to redirect_to link_sent_path(email: email) # do not know why, should be test related - expect(subject.current_user).to eq(user) + expect(controller.current_user).to eq(user) - expect(subject.current_gestionnaire).to be(nil) - expect(subject.current_administrateur).to be(nil) - expect(user.reload.loged_in_with_france_connect).to be(nil) + expect(controller.current_gestionnaire).to be(nil) + expect(controller.current_administrateur).to be(nil) + expect(user.loged_in_with_france_connect).to be(nil) + expect(GestionnaireMailer).to have_received(:send_login_token) + end + + context 'and the user try to connect multiple times in a short period' do + before do + allow_any_instance_of(Gestionnaire).to receive(:young_login_token?).and_return(true) + allow(GestionnaireMailer).to receive(:send_login_token) + end + + it 'does not renew nor send a new login token' do + subject + + expect(GestionnaireMailer).not_to have_received(:send_login_token) + end end end context 'when the device is trusted' do it 'signs in as user, gestionnaire and adminstrateur' do - expect(@response.redirect?).to be(true) - expect(subject).not_to redirect_to link_sent_path(email: email) - # TODO when signing in as non-administrateur, and not starting a demarche, log in to gestionnaire path - # expect(subject).to redirect_to gestionnaire_procedures_path + subject - expect(subject.current_user).to eq(user) - expect(subject.current_gestionnaire).to eq(gestionnaire) - expect(subject.current_administrateur).to eq(administrateur) + expect(response.redirect?).to be(true) + expect(controller).not_to redirect_to link_sent_path(email: email) + # TODO when signing in as non-administrateur, and not starting a demarche, log in to gestionnaire path + # expect(controller).to redirect_to gestionnaire_procedures_path + + expect(controller.current_user).to eq(user) + expect(controller.current_gestionnaire).to eq(gestionnaire) + expect(controller.current_administrateur).to eq(administrateur) expect(user.loged_in_with_france_connect).to be(nil) + expect(GestionnaireMailer).not_to have_received(:send_login_token) end end @@ -54,10 +77,12 @@ describe Users::SessionsController, type: :controller do let(:send_password) { 'wrong_password' } it 'fails to sign in with bad credentials' do - expect(@response.unauthorized?).to be(true) - expect(subject.current_user).to be(nil) - expect(subject.current_gestionnaire).to be(nil) - expect(subject.current_administrateur).to be(nil) + subject + + expect(response.unauthorized?).to be(true) + expect(controller.current_user).to be(nil) + expect(controller.current_gestionnaire).to be(nil) + expect(controller.current_administrateur).to be(nil) end end end diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 904c2eaab..344ee1683 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -412,6 +412,27 @@ describe Gestionnaire, type: :model do end end + describe '#young_login_token?' do + let!(:gestionnaire) { create(:gestionnaire) } + + context 'when there is a token' do + let!(:good_token) { gestionnaire.login_token! } + + context 'when the token has just been created' do + it { expect(gestionnaire.young_login_token?).to be true } + end + + context 'when the token is a bit old' do + before { gestionnaire.update(login_token_created_at: (Gestionnaire::LOGIN_TOKEN_YOUTH + 1.minute).ago) } + it { expect(gestionnaire.young_login_token?).to be false } + end + end + + context 'when there are no token' do + it { expect(gestionnaire.young_login_token?).to be false } + end + end + private def assign(procedure_to_assign) From eb6ef1eb46a4128bc4e18791c29859da8dfd7dbd Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 10 Jan 2019 12:01:07 +0100 Subject: [PATCH 2/3] Gestionnaire: increase validity token to 45 minutes --- app/models/gestionnaire.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index b62fee78d..7ac39c9e5 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -3,7 +3,7 @@ class Gestionnaire < ApplicationRecord include EmailSanitizableConcern include ActiveRecord::SecureToken - LOGIN_TOKEN_VALIDITY = 30.minutes + LOGIN_TOKEN_VALIDITY = 45.minutes LOGIN_TOKEN_YOUTH = 15.minutes devise :database_authenticatable, :registerable, :async, From e55fe2bb912ca7a4fbbe17b2834ffa08236dba83 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 10 Jan 2019 12:06:19 +0100 Subject: [PATCH 3/3] link_sent: add sentence to warn max email delay --- app/views/users/sessions/link_sent.html.haml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/users/sessions/link_sent.html.haml b/app/views/users/sessions/link_sent.html.haml index cd7f78c25..264b09955 100644 --- a/app/views/users/sessions/link_sent.html.haml +++ b/app/views/users/sessions/link_sent.html.haml @@ -9,6 +9,9 @@ %p.mail Ouvrez votre boite email #{@email} puis cliquez sur le lien d'activation du message Connexion sécurisée à demarches-simplifiees.fr. + %br + %br + Attention, ce message peut mettre jusqu'à 15 minutes pour arriver. %p.help En cas de difficultés, nous restons joignables sur #{link_to 'contact@demarches-simplifiees.fr', 'mailto:contact@demarches-simplifiees.fr'}.