From c927e6651d4499606e4ec733b2ee4dfbf42cb32f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 8 Nov 2018 16:13:51 +0100 Subject: [PATCH 01/14] footer: fix extra horizontal pixels being added to the page --- app/assets/stylesheets/new_design/new_footer.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/new_design/new_footer.scss b/app/assets/stylesheets/new_design/new_footer.scss index b17ff187d..75e6df47d 100644 --- a/app/assets/stylesheets/new_design/new_footer.scss +++ b/app/assets/stylesheets/new_design/new_footer.scss @@ -22,7 +22,7 @@ footer { .footer-columns { @extend %horizontal-list; justify-content: flex-start; - margin: 0 -20px; + margin: 0 -15px; } .footer-column { From c7ac43cfe7ec85642f8b561e1cfc63c7f040a48c Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 8 Nov 2018 14:02:41 +0000 Subject: [PATCH 02/14] confirmation: add a dedicated page with confirmation instructions Fix #2586 --- app/assets/images/user/confirmation-email.svg | 1 + .../stylesheets/new_design/_colors.scss | 1 + .../stylesheets/new_design/confirmations.scss | 64 +++++++++++++++++++ .../users/confirmations_controller.rb | 11 +++- .../users/registrations_controller.rb | 27 ++++---- app/views/users/confirmations/new.html.haml | 28 ++++++-- .../users/registrations_controller_spec.rb | 3 +- spec/features/new_user/invite_spec.rb | 2 +- spec/features/sessions/sign_up_spec.rb | 40 ++++++++++-- 9 files changed, 146 insertions(+), 31 deletions(-) create mode 100644 app/assets/images/user/confirmation-email.svg create mode 100644 app/assets/stylesheets/new_design/confirmations.scss diff --git a/app/assets/images/user/confirmation-email.svg b/app/assets/images/user/confirmation-email.svg new file mode 100644 index 000000000..18883ebe4 --- /dev/null +++ b/app/assets/images/user/confirmation-email.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/stylesheets/new_design/_colors.scss b/app/assets/stylesheets/new_design/_colors.scss index b07ace308..a437d8530 100644 --- a/app/assets/stylesheets/new_design/_colors.scss +++ b/app/assets/stylesheets/new_design/_colors.scss @@ -15,4 +15,5 @@ $light-green: lighten($green, 25%); $dark-green: darken($green, 20%); $orange: #F28900; $orange-bg: lighten($orange, 35%); +$yellow: #FEF3B8; $light-yellow: #FFFFDE; diff --git a/app/assets/stylesheets/new_design/confirmations.scss b/app/assets/stylesheets/new_design/confirmations.scss new file mode 100644 index 000000000..7c59e979b --- /dev/null +++ b/app/assets/stylesheets/new_design/confirmations.scss @@ -0,0 +1,64 @@ +@import "colors"; +@import "constants"; + +.devise-confirmations { + .one-column-centered { + max-width: 600px; + } + + .confirmation-icon, + .confirmation-preamble, + .confirmation-instructions, + .confirmation-separator { + font-size: 1.15em; + margin-bottom: $default-padding * 3; + } + + .confirmation-icon { + display: block; + margin-left: auto; + margin-right: auto; + } + + .confirmation-instructions { + color: #000000; + background-color: $yellow; + margin-left: -15px; + margin-right: -15px; + padding: 15px 20px 17px 20px; + } + + .confirmation-separator { + height: 1px; + margin-left: -12px; + margin-right: -12px; + border: none; + border-top: 1px solid #DDDDDD; + } + + .confirmation-resend { + p { + margin-bottom: $default-padding; + } + + .form { + display: flex; + flex-wrap: wrap; + + input, + button { + margin-bottom: $default-spacer; + } + + input[type=email] { + width: auto; + flex-grow: 1; + margin-right: $default-spacer; + } + } + + label { + display: none; + } + } +} diff --git a/app/controllers/users/confirmations_controller.rb b/app/controllers/users/confirmations_controller.rb index 927f44bc9..b60e2deeb 100644 --- a/app/controllers/users/confirmations_controller.rb +++ b/app/controllers/users/confirmations_controller.rb @@ -4,9 +4,10 @@ class Users::ConfirmationsController < Devise::ConfirmationsController layout "new_application" # GET /resource/confirmation/new - # def new - # super - # end + def new + # Allow displaying the user email in the message + self.resource = resource_class.new(email: user_email_param) + end # POST /resource/confirmation # def create @@ -20,6 +21,10 @@ class Users::ConfirmationsController < Devise::ConfirmationsController # protected + def user_email_param + params.permit(user: :email).dig(:user, :email) + end + # The path used after resending confirmation instructions. # def after_resending_confirmation_instructions_path_for(resource_name) # super(resource_name) diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 85964b628..980740240 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -22,18 +22,20 @@ class Users::RegistrationsController < Devise::RegistrationsController # POST /resource def create - user = User.find_by(email: params[:user][:email]) - if user.present? - if user.confirmed? - UserMailer.new_account_warning(user).deliver_later + # Handle existing user trying to sign up again + existing_user = User.find_by(email: params[:user][:email]) + if existing_user.present? + if existing_user.confirmed? + UserMailer.new_account_warning(existing_user).deliver_later + flash.notice = t('devise.registrations.signed_up_but_unconfirmed') + return redirect_to root_path else - user.resend_confirmation_instructions + existing_user.resend_confirmation_instructions + return redirect_to after_inactive_sign_up_path_for(existing_user) end - flash.notice = t('devise.registrations.signed_up_but_unconfirmed') - redirect_to root_path - else - super end + + super end # GET /resource/edit @@ -78,7 +80,8 @@ class Users::RegistrationsController < Devise::RegistrationsController # end # The path used after sign up for inactive accounts. - # def after_inactive_sign_up_path_for(resource) - # super(resource) - # end + def after_inactive_sign_up_path_for(resource) + flash.discard(:notice) # Remove devise's default message (as we have a custom page to explain it) + new_confirmation_path(resource, :user => { email: resource.email }) + end end diff --git a/app/views/users/confirmations/new.html.haml b/app/views/users/confirmations/new.html.haml index 0f7e6534a..83c389e61 100644 --- a/app/views/users/confirmations/new.html.haml +++ b/app/views/users/confirmations/new.html.haml @@ -1,17 +1,31 @@ -- content_for(:title, 'Renvoyer les instructions de confirmation de compte') +- content_for(:title, 'Confirmer votre adresse email') - content_for :footer do = render partial: 'root/footer' -.container.devise-container +.container.devise-container.devise-confirmations .one-column-centered = devise_error_messages! - = form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { class: 'form' }) do |f| + %img.confirmation-icon{ src: image_url("user/confirmation-email.svg"), alt: "" } - %h1 Renvoyer les instructions de confirmation de compte + %p.confirmation-preamble + = succeed '.' do + Avant d’effectuer votre démarche, nous avons besoin de vérifier votre adresse + - if resource.email.present? + %strong= resource.email - = f.label :email, 'Email' - = f.email_field :email, autofocus: true + %p.confirmation-instructions + Ouvrez votre boîte email, et + %strong cliquez sur le lien d’activation + dans le message que vous avez reçu. - = f.submit 'Renvoyer les instructions de confirmation', class: 'button primary' + %hr.confirmation-separator + + .confirmation-resend + %p Si vous n’avez pas reçu notre message, nous pouvons vous le renvoyer. + + = form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { class: 'form' }) do |f| + = f.label :email, 'Email' + = f.email_field :email, placeholder: 'Email', class: 'small', autofocus: true + = f.submit 'Renvoyer un email de confirmation', class: 'button' diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb index 14af91a6a..2f3c80463 100644 --- a/spec/controllers/users/registrations_controller_spec.rb +++ b/spec/controllers/users/registrations_controller_spec.rb @@ -74,8 +74,7 @@ describe Users::RegistrationsController, type: :controller do subject end - it { expect(response).to redirect_to(root_path) } - it { expect(flash.notice).to eq(I18n.t('devise.registrations.signed_up_but_unconfirmed')) } + it { expect(response).to redirect_to(new_user_confirmation_path(user: { email: user[:email] })) } it { expect(UserMailer).not_to have_received(:new_account_warning) } end end diff --git a/spec/features/new_user/invite_spec.rb b/spec/features/new_user/invite_spec.rb index e5774c883..d07efb73d 100644 --- a/spec/features/new_user/invite_spec.rb +++ b/spec/features/new_user/invite_spec.rb @@ -37,7 +37,7 @@ feature 'Invitations' do # Create the account sign_up_with invite.email, user_password - expect(page).to have_content("lien d'activation") + expect(page).to have_content('lien d’activation') # Confirm the account # (The user should be redirected to the dossier they was invited on) diff --git a/spec/features/sessions/sign_up_spec.rb b/spec/features/sessions/sign_up_spec.rb index 54750eb01..efd0929c4 100644 --- a/spec/features/sessions/sign_up_spec.rb +++ b/spec/features/sessions/sign_up_spec.rb @@ -1,15 +1,18 @@ require 'spec_helper' feature 'Signin up:' do + let(:user_email) { generate :user_email } + let(:user_password) { 'testpassword' } + scenario 'a new user can sign-up' do visit root_path click_on 'Connexion' click_on 'Créer un compte' - sign_up_with 'testuser@exemple.fr' - expect(page).to have_content "Nous vous avons envoyé un email contenant un lien d'activation" + sign_up_with user_email, user_password + expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}" - click_confirmation_link_for 'testuser@exemple.fr' + click_confirmation_link_for user_email expect(page).to have_content 'Votre compte a été activé' expect(page).to have_current_path dossiers_path end @@ -25,12 +28,37 @@ feature 'Signin up:' do expect(page).to have_current_path new_user_session_path click_on 'Créer un compte' - sign_up_with 'testuser@exemple.fr' - expect(page).to have_content "Nous vous avons envoyé un email contenant un lien d'activation" + sign_up_with user_email, user_password + expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}" - click_confirmation_link_for 'testuser@exemple.fr' + click_confirmation_link_for user_email expect(page).to have_content 'Votre compte a été activé' expect(page).to have_content procedure.libelle end end + + context 'when a user is not confirmed yet' do + before do + visit root_path + click_on 'Connexion' + click_on 'Créer un compte' + + sign_up_with user_email, user_password + end + + # Ideally, when signing-in with an unconfirmed account, + # the user would be redirected to the "resend email confirmation" page. + # + # However the check for unconfirmed accounts is made by Warden every time a page is loaded – + # and much earlier than SessionsController#create. + # + # For now only test the default behavior (an error message is displayed). + scenario 'they get an error message' do + visit root_path + click_on 'Connexion' + + sign_in_with user_email, user_password + expect(page).to have_content 'Vous devez confirmer votre adresse email pour continuer' + end + end end From 5c921182ea19b7d3f1323283f378a71136e8ad84 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 8 Nov 2018 16:19:17 +0000 Subject: [PATCH 03/14] procedure: rename `mean_instruction_time` to `usual_instruction_time` Ref #2970 --- app/models/procedure.rb | 6 +++--- .../new_user/dossiers/show/_status_overview.html.haml | 8 ++++---- spec/models/procedure_spec.rb | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index c8dd7b625..d8f287ab1 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -304,15 +304,15 @@ class Procedure < ApplicationRecord end end - def mean_traitement_time + def usual_traitement_time mean_time(:en_construction_at, :processed_at) end - def mean_verification_time + def usual_verification_time mean_time(:en_construction_at, :en_instruction_at) end - def mean_instruction_time + def usual_instruction_time mean_time(:en_instruction_at, :processed_at) end diff --git a/app/views/new_user/dossiers/show/_status_overview.html.haml b/app/views/new_user/dossiers/show/_status_overview.html.haml index 167883f63..39f06f614 100644 --- a/app/views/new_user/dossiers/show/_status_overview.html.haml +++ b/app/views/new_user/dossiers/show/_status_overview.html.haml @@ -31,10 +31,10 @@ %strong votre dossier passera directement en instruction / FIXME: remove the custom procedure switch at some point - - if dossier.procedure.mean_verification_time && show_time_means + - if dossier.procedure.usual_verification_time && show_time_means - cache(dossier.procedure, expires_in: 1.week) do %p - Le temps moyen de vérification pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.mean_verification_time)}. + Le temps moyen de vérification pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.usual_verification_time)}. - elsif dossier.en_instruction? .en-instruction @@ -46,10 +46,10 @@ avec le résultat. / FIXME: remove the custom procedure switch at some point - - if dossier.procedure.mean_instruction_time && show_time_means + - if dossier.procedure.usual_instruction_time && show_time_means - cache(dossier.procedure, expires_in: 1.week) do %p - Le temps moyen d’instruction pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.mean_instruction_time)}. + Le temps moyen d’instruction pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.usual_instruction_time)}. - elsif dossier.accepte? .accepte diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 1f039cea0..a96599042 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -705,7 +705,7 @@ describe Procedure do end end - describe '#mean_instruction_time' do + describe '#usual_instruction_time' do let(:procedure) { create(:procedure) } context 'when there is only one dossier' do @@ -719,7 +719,7 @@ describe Procedure do dossier.update(en_instruction_at: instruction_date, processed_at: processed_date) end - it { expect(procedure.mean_instruction_time).to eq(1.day.to_i) } + it { expect(procedure.usual_instruction_time).to eq(1.day.to_i) } end end end From e59bec51ef7b9e434574593c9316df5250e24cf8 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 8 Nov 2018 18:09:27 +0100 Subject: [PATCH 04/14] procedure: use 90th percentile to estimate the completion delay --- app/models/procedure.rb | 12 +++++++----- lib/percentile.rb | 31 ++++++++++++++++++++++++++++++ spec/models/procedure_spec.rb | 36 +++++++++++++++++++++++------------ 3 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 lib/percentile.rb diff --git a/app/models/procedure.rb b/app/models/procedure.rb index d8f287ab1..44984c564 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -1,3 +1,5 @@ +require Rails.root.join('lib', 'percentile') + class Procedure < ApplicationRecord MAX_DUREE_CONSERVATION = 36 @@ -305,15 +307,15 @@ class Procedure < ApplicationRecord end def usual_traitement_time - mean_time(:en_construction_at, :processed_at) + percentile_time(:en_construction_at, :processed_at, 90) end def usual_verification_time - mean_time(:en_construction_at, :en_instruction_at) + percentile_time(:en_construction_at, :en_instruction_at, 90) end def usual_instruction_time - mean_time(:en_instruction_at, :processed_at) + percentile_time(:en_instruction_at, :processed_at, 90) end PATH_AVAILABLE = :available @@ -421,14 +423,14 @@ class Procedure < ApplicationRecord true end - def mean_time(start_attribute, end_attribute) + def percentile_time(start_attribute, end_attribute, p) times = dossiers .state_termine .pluck(start_attribute, end_attribute) .map { |(start_date, end_date)| end_date - start_date } if times.present? - times.sum.fdiv(times.size).ceil + times.percentile(p).ceil end end end diff --git a/lib/percentile.rb b/lib/percentile.rb new file mode 100644 index 000000000..00aa9f59c --- /dev/null +++ b/lib/percentile.rb @@ -0,0 +1,31 @@ +# Adapted from https://github.com/thirtysixthspan/descriptive_statistics + +# Copyright (c) 2010-2014 Derrick Parkhurst (derrick.parkhurst@gmail.com) +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. + +class Array + def percentile(p) + values = self.sort + + if values.empty? + return [] + elsif values.size == 1 + return values.first + elsif p == 100 + return values.last + end + + rank = p / 100.0 * (values.size - 1) + lower, upper = values[rank.floor, 2] + lower + (upper - lower) * (rank - rank.floor) + end +end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index a96599042..ab4fb73d7 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -708,19 +708,31 @@ describe Procedure do describe '#usual_instruction_time' do let(:procedure) { create(:procedure) } - context 'when there is only one dossier' do - let(:dossier) { create(:dossier, procedure: procedure) } - - context 'which is termine' do - before do - dossier.accepte! - processed_date = Time.zone.parse('12/12/2012') - instruction_date = processed_date - 1.day - dossier.update(en_instruction_at: instruction_date, processed_at: processed_date) - end - - it { expect(procedure.usual_instruction_time).to eq(1.day.to_i) } + before do + processed_delays.each do |delay| + dossier = create :dossier, :accepte, procedure: procedure + instruction_date = 1.month.ago + processed_date = instruction_date + delay + dossier.update!(en_instruction_at: instruction_date, processed_at: processed_date) end end + + context 'when there are several processed dossiers' do + let(:processed_delays) { [1.day, 2.days, 2.days, 2.days, 2.days, 3.days, 3.days, 3.days, 3.days, 12.days] } + + it 'returns a time representative of the dossier instruction delay' do + expect(procedure.usual_instruction_time).to be_between(3.days, 4.days) + end + end + + context 'when there is only one processed dossier' do + let(:processed_delays) { [1.day] } + it { expect(procedure.usual_instruction_time).to eq(1.day) } + end + + context 'where there is no processed dossier' do + let(:processed_delays) { [] } + it { expect(procedure.usual_instruction_time).to be_nil } + end end end From 7635aede98add818533a9ab52c1e685fb2f2067f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 12 Nov 2018 08:39:59 +0000 Subject: [PATCH 05/14] dossier: improve wording of estimated time Ref #2970 --- app/views/new_user/dossiers/show/_status_overview.html.haml | 4 ++-- spec/features/new_user/dossier_details_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/new_user/dossiers/show/_status_overview.html.haml b/app/views/new_user/dossiers/show/_status_overview.html.haml index 39f06f614..d1e94d1e2 100644 --- a/app/views/new_user/dossiers/show/_status_overview.html.haml +++ b/app/views/new_user/dossiers/show/_status_overview.html.haml @@ -34,7 +34,7 @@ - if dossier.procedure.usual_verification_time && show_time_means - cache(dossier.procedure, expires_in: 1.week) do %p - Le temps moyen de vérification pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.usual_verification_time)}. + Habituellement, les dossiers de cette démarche sont vérifiés dans un délai de #{distance_of_time_in_words(dossier.procedure.usual_verification_time)}. - elsif dossier.en_instruction? .en-instruction @@ -49,7 +49,7 @@ - if dossier.procedure.usual_instruction_time && show_time_means - cache(dossier.procedure, expires_in: 1.week) do %p - Le temps moyen d’instruction pour cette démarche est de #{distance_of_time_in_words(dossier.procedure.usual_instruction_time)}. + Habituellement, les dossiers de cette démarche sont traités dans un délai de #{distance_of_time_in_words(dossier.procedure.usual_instruction_time)}. - elsif dossier.accepte? .accepte diff --git a/spec/features/new_user/dossier_details_spec.rb b/spec/features/new_user/dossier_details_spec.rb index fd46cac7f..b24744713 100644 --- a/spec/features/new_user/dossier_details_spec.rb +++ b/spec/features/new_user/dossier_details_spec.rb @@ -23,7 +23,7 @@ describe 'Dossier details:' do visit dossier_path(dossier) end - it { expect(page).to have_text("Le temps moyen de vérification pour cette démarche est de 10 jours.") } + it { expect(page).to have_text("Habituellement, les dossiers de cette démarche sont vérifiés dans un délai de 10 jours.") } end context "when the dossier is in instruction" do @@ -34,7 +34,7 @@ describe 'Dossier details:' do visit dossier_path(dossier) end - it { expect(page).to have_text("Le temps moyen d’instruction pour cette démarche est de 2 mois.") } + it { expect(page).to have_text("Habituellement, les dossiers de cette démarche sont traités dans un délai de 2 mois.") } end end From 7a7093503acfccf20ac5c28f4fa1e25daed3e666 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 12 Nov 2018 10:22:21 +0100 Subject: [PATCH 06/14] dossier: avoid the estimated duration to dangle on a new line --- app/assets/stylesheets/new_design/status_overview.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/new_design/status_overview.scss b/app/assets/stylesheets/new_design/status_overview.scss index beede684f..1503d10cb 100644 --- a/app/assets/stylesheets/new_design/status_overview.scss +++ b/app/assets/stylesheets/new_design/status_overview.scss @@ -51,7 +51,7 @@ .brouillon, .en-construction, .en-instruction { - max-width: 600px; + max-width: 650px; margin: auto; } From 2613d05a4fe2b36c5833547979923be68ab9171b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 30 Oct 2018 15:31:13 +0100 Subject: [PATCH 07/14] An admin has always a gestionnaire role --- app/models/administration.rb | 6 ++++++ lib/tasks/2018_10_30_admin_has_gestionnaire.rake | 15 +++++++++++++++ spec/features/admin/connection_spec.rb | 5 ++++- spec/models/administration_spec.rb | 6 ++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 lib/tasks/2018_10_30_admin_has_gestionnaire.rake diff --git a/app/models/administration.rb b/app/models/administration.rb index 1d350b240..8d98bab4d 100644 --- a/app/models/administration.rb +++ b/app/models/administration.rb @@ -19,11 +19,17 @@ class Administration < ApplicationRecord if administrateur.save AdministrationMailer.new_admin_email(administrateur, self).deliver_later administrateur.invite!(id) + User.create({ email: email, password: password, confirmed_at: Time.zone.now }) + + Gestionnaire.create({ + email: email, + password: password + }) end administrateur diff --git a/lib/tasks/2018_10_30_admin_has_gestionnaire.rake b/lib/tasks/2018_10_30_admin_has_gestionnaire.rake new file mode 100644 index 000000000..c78969b78 --- /dev/null +++ b/lib/tasks/2018_10_30_admin_has_gestionnaire.rake @@ -0,0 +1,15 @@ +namespace :'2018_10_30_admin_has_gestionnaire' do + task run: :environment do + admin_without_gestionnaire_ids = Administrateur + .find_by_sql('SELECT administrateurs.id FROM administrateurs LEFT OUTER JOIN gestionnaires ON gestionnaires.email = administrateurs.email WHERE gestionnaires.email IS NULL') + .pluck(:id) + + admin_without_gestionnaire_ids.each do |admin_id| + admin = Administrateur.find(admin_id) + g = Gestionnaire.new + g.email = admin.email + g.encrypted_password = admin.encrypted_password + g.save(validate: false) + end + end +end diff --git a/spec/features/admin/connection_spec.rb b/spec/features/admin/connection_spec.rb index 3b6168311..6b1a0eaf2 100644 --- a/spec/features/admin/connection_spec.rb +++ b/spec/features/admin/connection_spec.rb @@ -1,7 +1,10 @@ require 'spec_helper' feature 'Administrator connection' do - let(:admin) { create(:administrateur) } + let(:email) { 'admin1@admin.com' } + let(:password) { 'mon chien aime les bananes' } + let!(:admin) { create(:administrateur, email: email, password: password) } + let!(:gestionnaire) { create(:gestionnaire, email: email, password: password) } before do visit new_administrateur_session_path end diff --git a/spec/models/administration_spec.rb b/spec/models/administration_spec.rb index 91444ad3b..77f3594f2 100644 --- a/spec/models/administration_spec.rb +++ b/spec/models/administration_spec.rb @@ -20,6 +20,12 @@ describe Administration, type: :model do expect(user).to be_present end + it 'creates a corresponding gestionnaire account for the email' do + subject + gestionnaire = Gestionnaire.find_by(email: valid_email) + expect(gestionnaire).to be_present + end + context 'when there already is a user account with the same email' do before { create(:user, email: valid_email) } it 'still creates an admin account' do From 56905992895c9b9cfb4dd927a4be0c0a7f002393 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 3 Oct 2018 11:11:02 +0200 Subject: [PATCH 08/14] Session: send a mail to confirm gestionnaire login --- .../stylesheets/new_design/link-sent.scss | 30 +++++++ app/controllers/users/sessions_controller.rb | 34 ++++++-- app/mailers/gestionnaire_mailer.rb | 8 ++ app/models/gestionnaire.rb | 15 ++++ .../send_login_token.html.haml | 12 +++ app/views/users/sessions/link_sent.html.haml | 14 ++++ config/routes.rb | 2 + ...pted_login_token_column_to_gestionnaire.rb | 6 ++ db/schema.rb | 4 +- .../users/sessions_controller_spec.rb | 81 ++++++++++++++++--- spec/features/admin/connection_spec.rb | 9 ++- .../new_gestionnaire/gestionnaire_spec.rb | 11 ++- .../previews/gestionnaire_mailer_preview.rb | 4 + spec/support/feature_helpers.rb | 15 +++- 14 files changed, 215 insertions(+), 30 deletions(-) create mode 100644 app/assets/stylesheets/new_design/link-sent.scss create mode 100644 app/views/gestionnaire_mailer/send_login_token.html.haml create mode 100644 app/views/users/sessions/link_sent.html.haml create mode 100644 db/migrate/20181108091339_add_encrypted_login_token_column_to_gestionnaire.rb diff --git a/app/assets/stylesheets/new_design/link-sent.scss b/app/assets/stylesheets/new_design/link-sent.scss new file mode 100644 index 000000000..778d330b1 --- /dev/null +++ b/app/assets/stylesheets/new_design/link-sent.scss @@ -0,0 +1,30 @@ +@import "constants"; +@import "colors"; + +#link-sent { + padding-top: 2 * $default-padding; + padding-bottom: 2 * $default-padding; + text-align: center; + max-width: 600px; + + b { + font-weight: bold; + } + + p { + text-align: left; + margin: 6 * $default-spacer auto; + } + + p.mail { + color: #000000; + background-color: $yellow; + padding: $default-padding; + } + + p.help { + border-top: 1px solid $grey; + padding-top: 6 * $default-spacer; + margin-bottom: 2 * $default-spacer; + } +} diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 5d43d18e6..000d6a7d8 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -23,13 +23,17 @@ class Users::SessionsController < Sessions::SessionsController current_user.update(loged_in_with_france_connect: '') end - if user_signed_in? + if gestionnaire_signed_in? + gestionnaire = current_gestionnaire + + login_token = gestionnaire.login_token! + GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later + + [:user, :gestionnaire, :administrateur].each { |role| sign_out(role) } + + redirect_to link_sent_path(email: gestionnaire.email) + elsif user_signed_in? redirect_to after_sign_in_path_for(:user) - elsif gestionnaire_signed_in? - location = stored_location_for(:gestionnaire) || gestionnaire_procedures_path - redirect_to location - elsif administrateur_signed_in? - redirect_to admin_path else flash.alert = 'Mauvais couple login / mot de passe' new @@ -37,6 +41,10 @@ class Users::SessionsController < Sessions::SessionsController end end + def link_sent + @email = params[:email] + end + # DELETE /resource/sign_out def destroy if gestionnaire_signed_in? @@ -68,6 +76,20 @@ class Users::SessionsController < Sessions::SessionsController redirect_to new_user_session_path end + def sign_in_by_link + gestionnaire = Gestionnaire.find(params[:id]) + if gestionnaire&.login_token_valid?(params[:jeton]) + user = User.find_by(email: gestionnaire.email) + administrateur = Administrateur.find_by(email: gestionnaire.email) + [user, gestionnaire, administrateur].compact.each { |resource| sign_in(resource) } + + redirect_to gestionnaire_procedures_path + else + flash[:alert] = 'Votre lien est invalide ou expiré, veuillez-vous reconnecter.' + redirect_to new_user_session_path + end + end + private def error_procedure diff --git a/app/mailers/gestionnaire_mailer.rb b/app/mailers/gestionnaire_mailer.rb index 105eb7243..a3473f65f 100644 --- a/app/mailers/gestionnaire_mailer.rb +++ b/app/mailers/gestionnaire_mailer.rb @@ -34,4 +34,12 @@ class GestionnaireMailer < ApplicationMailer mail(to: recipient.email, subject: subject) end + + def send_login_token(gestionnaire, login_token) + @gestionnaire_id = gestionnaire.id + @login_token = login_token + subject = "Connexion sécurisée à demarches-simplifiees.fr" + + mail(to: gestionnaire.email, subject: subject) + end end diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 4829a736b..44ef980e6 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -1,6 +1,7 @@ class Gestionnaire < ApplicationRecord include CredentialsSyncableConcern include EmailSanitizableConcern + include ActiveRecord::SecureToken devise :database_authenticatable, :registerable, :async, :recoverable, :rememberable, :trackable, :validatable @@ -144,6 +145,20 @@ class Gestionnaire < ApplicationRecord Dossier.where(id: dossiers_id_with_notifications(dossiers)).group(:procedure_id).count end + def login_token! + login_token = Gestionnaire.generate_unique_secure_token + encrypted_login_token = BCrypt::Password.create(login_token) + update(encrypted_login_token: encrypted_login_token, login_token_created_at: Time.zone.now) + login_token + end + + def login_token_valid?(login_token) + BCrypt::Password.new(encrypted_login_token) == login_token + 30.minutes.ago < login_token_created_at + rescue BCrypt::Errors::InvalidHash + false + end + def dossiers_id_with_notifications(dossiers) dossiers = dossiers.followed_by(self) diff --git a/app/views/gestionnaire_mailer/send_login_token.html.haml b/app/views/gestionnaire_mailer/send_login_token.html.haml new file mode 100644 index 000000000..c2c2374f4 --- /dev/null +++ b/app/views/gestionnaire_mailer/send_login_token.html.haml @@ -0,0 +1,12 @@ +%p + Bonjour, + +%p + Veuillez cliquer sur le lien suivant pour vous connecter sur le site demarches-simplifiees.fr :  + = link_to(sign_in_by_link_url(@gestionnaire_id, jeton: @login_token), sign_in_by_link_url(@gestionnaire_id, jeton: @login_token)) + +%p + Bonne journée, + +%p + L'équipe demarches-simplifiees.fr diff --git a/app/views/users/sessions/link_sent.html.haml b/app/views/users/sessions/link_sent.html.haml new file mode 100644 index 000000000..cd7f78c25 --- /dev/null +++ b/app/views/users/sessions/link_sent.html.haml @@ -0,0 +1,14 @@ +- content_for(:title, 'Lien de connexion par email') + +- content_for :footer do + = render partial: 'root/footer' + +#link-sent.container + = image_tag('user/confirmation-email.svg') + %h1 Encore une petite étape :) + + %p.mail + Ouvrez votre boite email #{@email} puis cliquez sur le lien d'activation du message Connexion sécurisée à demarches-simplifiees.fr. + + %p.help + En cas de difficultés, nous restons joignables sur #{link_to 'contact@demarches-simplifiees.fr', 'mailto:contact@demarches-simplifiees.fr'}. diff --git a/config/routes.rb b/config/routes.rb index 1c5f6ac70..4ba5ac98a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,6 +88,8 @@ Rails.application.routes.draw do devise_scope :user do get '/users/sign_in/demo' => redirect("/users/sign_in") get '/users/no_procedure' => 'users/sessions#no_procedure' + get 'connexion-par-jeton/:id' => 'users/sessions#sign_in_by_link', as: 'sign_in_by_link' + get 'lien-envoye/:email' => 'users/sessions#link_sent', constraints: { email: /.*/ }, as: 'link_sent' end devise_scope :gestionnaire do diff --git a/db/migrate/20181108091339_add_encrypted_login_token_column_to_gestionnaire.rb b/db/migrate/20181108091339_add_encrypted_login_token_column_to_gestionnaire.rb new file mode 100644 index 000000000..9d92083f8 --- /dev/null +++ b/db/migrate/20181108091339_add_encrypted_login_token_column_to_gestionnaire.rb @@ -0,0 +1,6 @@ +class AddEncryptedLoginTokenColumnToGestionnaire < ActiveRecord::Migration[5.2] + def change + add_column :gestionnaires, :encrypted_login_token, :text + add_column :gestionnaires, :login_token_created_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index f366acca0..e6cba3c3a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_10_30_141238) do +ActiveRecord::Schema.define(version: 2018_11_08_091339) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -365,6 +365,8 @@ ActiveRecord::Schema.define(version: 2018_10_30_141238) do t.string "last_sign_in_ip" t.datetime "created_at" t.datetime "updated_at" + t.text "encrypted_login_token" + t.datetime "login_token_created_at" t.index ["email"], name: "index_gestionnaires_on_email", unique: true t.index ["reset_password_token"], name: "index_gestionnaires_on_reset_password_token", unique: true end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index af12c7ce2..0e8eb02ea 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -41,18 +41,25 @@ describe Users::SessionsController, type: :controller do it 'signs gestionnaire in' do post :create, params: { user: { email: gestionnaire.email, password: gestionnaire.password } } - expect(@response.redirect?).to be(true) + + expect(subject).to redirect_to link_sent_path(email: gestionnaire.email) expect(subject.current_user).to be(nil) - expect(subject.current_gestionnaire).to eq(gestionnaire) + expect(subject.current_gestionnaire).to be(nil) expect(subject.current_administrateur).to be(nil) end - it 'signs administrateur in' do - post :create, params: { user: { email: administrateur.email, password: administrateur.password } } - expect(@response.redirect?).to be(true) - expect(subject.current_user).to be(nil) - expect(subject.current_gestionnaire).to be(nil) - expect(subject.current_administrateur).to eq(administrateur) + context 'signs administrateur in' do + # an admin has always an gestionnaire role + before { gestionnaire } + + it 'signs administrateur in' do + post :create, params: { user: { email: administrateur.email, password: administrateur.password } } + + expect(subject).to redirect_to link_sent_path(email: gestionnaire.email) + expect(subject.current_user).to be(nil) + expect(subject.current_gestionnaire).to be(nil) + expect(subject.current_administrateur).to eq(nil) + end end context { @@ -63,10 +70,16 @@ describe Users::SessionsController, type: :controller do it 'signs user + gestionnaire + administrateur in' do post :create, params: { user: { email: administrateur.email, password: administrateur.password } } - expect(@response.redirect?).to be(true) - expect(subject.current_user).to eq(user) - expect(subject.current_gestionnaire).to eq(gestionnaire) - expect(subject.current_administrateur).to eq(administrateur) + + expect(subject).to redirect_to link_sent_path(email: gestionnaire.email) + + # TODO: fix me + # Strange behaviour: sign_out(:user) does not work in spec + # but seems to work in live + # expect(controller.current_user).to be(nil) + + 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) end } @@ -219,4 +232,48 @@ describe Users::SessionsController, type: :controller do end end end + + describe '#sign_in_by_link' do + context 'when the gestionnaire has non other account' do + let(:gestionnaire) { create(:gestionnaire) } + before do + post :sign_in_by_link, params: { id: gestionnaire.id, login_token: login_token } + end + + context 'when the token is valid' do + let(:login_token) { gestionnaire.login_token! } + + it { is_expected.to redirect_to gestionnaire_procedures_path } + it { expect(controller.current_gestionnaire).to eq(gestionnaire) } + end + + context 'when the token is invalid' do + let(:login_token) { 'invalid_token' } + + it { is_expected.to redirect_to new_user_session_path } + it { expect(controller.current_gestionnaire).to be_nil } + end + end + + context 'when the gestionnaire has an user and admin account' do + let(:email) { 'unique@plop.com' } + let(:password) { 'un super mot de passe' } + + let!(:user) { create(:user, email: email, password: password) } + let!(:gestionnaire) { create(:gestionnaire, email: email, password: password) } + let!(:administrateur) { create(:administrateur, email: email, password: password) } + + before do + post :sign_in_by_link, params: { id: gestionnaire.id, login_token: login_token } + end + + context 'when the token is valid' do + let(:login_token) { gestionnaire.login_token! } + + it { expect(controller.current_gestionnaire).to eq(gestionnaire) } + it { expect(controller.current_administrateur).to eq(administrateur) } + it { expect(controller.current_user).to eq(user) } + end + end + end end diff --git a/spec/features/admin/connection_spec.rb b/spec/features/admin/connection_spec.rb index 6b1a0eaf2..31c1230e4 100644 --- a/spec/features/admin/connection_spec.rb +++ b/spec/features/admin/connection_spec.rb @@ -1,23 +1,26 @@ require 'spec_helper' feature 'Administrator connection' do + include ActiveJob::TestHelper + let(:email) { 'admin1@admin.com' } let(:password) { 'mon chien aime les bananes' } let!(:admin) { create(:administrateur, email: email, password: password) } let!(:gestionnaire) { create(:gestionnaire, email: email, password: password) } + before do visit new_administrateur_session_path end + scenario 'administrator is on sign in page' do expect(page).to have_css('#new_user') end context "admin fills form and log in" do before do - page.find_by_id('user_email').set admin.email - page.find_by_id('user_password').set admin.password - page.click_on 'Se connecter' + sign_in_with(email, password, true) end + scenario 'a menu button is available' do expect(page).to have_css('#admin_menu') end diff --git a/spec/features/new_gestionnaire/gestionnaire_spec.rb b/spec/features/new_gestionnaire/gestionnaire_spec.rb index 1461b0ae0..eb1881b05 100644 --- a/spec/features/new_gestionnaire/gestionnaire_spec.rb +++ b/spec/features/new_gestionnaire/gestionnaire_spec.rb @@ -116,7 +116,7 @@ feature 'The gestionnaire part' do log_out - log_in(gestionnaire.email, password) + log_in(gestionnaire.email, password, check_email: false) click_on procedure.libelle click_on dossier.user.email @@ -173,14 +173,13 @@ feature 'The gestionnaire part' do expect(page).to have_text("Dossier envoyé") end - def log_in(email, password) + def log_in(email, password, check_email: true) visit '/' click_on 'Connexion' expect(page).to have_current_path(new_user_session_path) - fill_in 'user_email', with: email - fill_in 'user_password', with: password - click_on 'Se connecter' + sign_in_with(email, password, check_email) + expect(page).to have_current_path(gestionnaire_procedures_path) end @@ -196,7 +195,7 @@ feature 'The gestionnaire part' do end def test_mail(to, content) - mail = ActionMailer::Base.deliveries.first + mail = ActionMailer::Base.deliveries.last expect(mail.to).to match([to]) expect(mail.body.parts.map(&:to_s)).to all(include(content)) end diff --git a/spec/mailers/previews/gestionnaire_mailer_preview.rb b/spec/mailers/previews/gestionnaire_mailer_preview.rb index d38ca04e2..d7707bd22 100644 --- a/spec/mailers/previews/gestionnaire_mailer_preview.rb +++ b/spec/mailers/previews/gestionnaire_mailer_preview.rb @@ -7,4 +7,8 @@ class GestionnaireMailerPreview < ActionMailer::Preview def send_dossier GestionnaireMailer.send_dossier(Gestionnaire.first, Dossier.first, Gestionnaire.last) end + + def send_login_token + GestionnaireMailer.send_login_token(Gestionnaire.first, "token") + end end diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 0f0504446..f0d0f6447 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -17,10 +17,21 @@ module FeatureHelpers dossier end - def sign_in_with(email, password) + def sign_in_with(email, password, sign_in_by_link = false) fill_in :user_email, with: email fill_in :user_password, with: password - click_on 'Se connecter' + + perform_enqueued_jobs do + click_on 'Se connecter' + end + + if sign_in_by_link + mail = ActionMailer::Base.deliveries.last + message = mail.body.parts.join(&:to_s) + login_token = message[/connexion-par-jeton\/(.*)/, 1] + + visit sign_in_by_link_path(login_token) + end end def sign_up_with(email, password = 'testpassword') From 0d8d2de5a68786030c5d5bd02a2911e61bcbe06b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 30 Oct 2018 18:24:29 +0100 Subject: [PATCH 09/14] Session: add trusted_device cookie --- .../administrateurs/activate_controller.rb | 7 +++- .../gestionnaires/activate_controller.rb | 7 +++- app/controllers/users/sessions_controller.rb | 21 +++++++--- app/models/concerns/trusted_device_concern.rb | 25 +++++++++++ .../activate_controller_spec.rb | 20 +++++++++ .../gestionnaires/activate_controller_spec.rb | 20 +++++++++ .../users/sessions_controller_spec.rb | 41 +++++++++++++++++++ 7 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 app/models/concerns/trusted_device_concern.rb create mode 100644 spec/controllers/administrateur/activate_controller_spec.rb create mode 100644 spec/controllers/gestionnaires/activate_controller_spec.rb diff --git a/app/controllers/administrateurs/activate_controller.rb b/app/controllers/administrateurs/activate_controller.rb index d354c0375..5d2654587 100644 --- a/app/controllers/administrateurs/activate_controller.rb +++ b/app/controllers/administrateurs/activate_controller.rb @@ -1,12 +1,17 @@ require 'zxcvbn' class Administrateurs::ActivateController < ApplicationController + include TrustedDeviceConcern + layout "new_application" def new @administrateur = Administrateur.find_inactive_by_token(params[:token]) - if !@administrateur + if @administrateur + # the administrateur activates its account from an email + trust_device + else flash.alert = "Le lien de validation d'administrateur a expiré, #{helpers.contact_link('contactez-nous', tags: 'lien expiré')} pour obtenir un nouveau lien." redirect_to root_path end diff --git a/app/controllers/gestionnaires/activate_controller.rb b/app/controllers/gestionnaires/activate_controller.rb index 873757f47..12fdae36a 100644 --- a/app/controllers/gestionnaires/activate_controller.rb +++ b/app/controllers/gestionnaires/activate_controller.rb @@ -1,10 +1,15 @@ class Gestionnaires::ActivateController < ApplicationController + include TrustedDeviceConcern + layout "new_application" def new @gestionnaire = Gestionnaire.with_reset_password_token(params[:token]) - if !@gestionnaire + if @gestionnaire + # the gestionnaire activates its account from an email + trust_device + else flash.alert = "Le lien de validation du compte instructeur a expiré, #{helpers.contact_link('contactez-nous', tags: 'lien expiré')} pour obtenir un nouveau lien." redirect_to root_path end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 000d6a7d8..9db5a4310 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -1,4 +1,7 @@ class Users::SessionsController < Sessions::SessionsController + include TrustedDeviceConcern + include ActionView::Helpers::DateHelper + layout "new_application" # GET /resource/sign_in @@ -24,14 +27,17 @@ class Users::SessionsController < Sessions::SessionsController end if gestionnaire_signed_in? - gestionnaire = current_gestionnaire + if trusted_device? + redirect_to gestionnaire_procedures_path + else + gestionnaire = current_gestionnaire + login_token = gestionnaire.login_token! + GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later - login_token = gestionnaire.login_token! - GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later + [:user, :gestionnaire, :administrateur].each { |role| sign_out(role) } - [:user, :gestionnaire, :administrateur].each { |role| sign_out(role) } - - redirect_to link_sent_path(email: gestionnaire.email) + redirect_to link_sent_path(email: gestionnaire.email) + end elsif user_signed_in? redirect_to after_sign_in_path_for(:user) else @@ -79,6 +85,9 @@ class Users::SessionsController < Sessions::SessionsController def sign_in_by_link gestionnaire = Gestionnaire.find(params[:id]) if gestionnaire&.login_token_valid?(params[:jeton]) + trust_device + flash.notice = "Merci d’avoir confirmé votre connexion. Votre navigateur est maintenant authentifié pour #{TRUSTED_DEVICE_PERIOD.to_i / ActiveSupport::Duration::SECONDS_PER_DAY} jours." + user = User.find_by(email: gestionnaire.email) administrateur = Administrateur.find_by(email: gestionnaire.email) [user, gestionnaire, administrateur].compact.each { |resource| sign_in(resource) } diff --git a/app/models/concerns/trusted_device_concern.rb b/app/models/concerns/trusted_device_concern.rb new file mode 100644 index 000000000..5aff22882 --- /dev/null +++ b/app/models/concerns/trusted_device_concern.rb @@ -0,0 +1,25 @@ +module TrustedDeviceConcern + extend ActiveSupport::Concern + + TRUSTED_DEVICE_COOKIE_NAME = :trusted_device + TRUSTED_DEVICE_PERIOD = 1.month + + def trust_device + cookies.encrypted[TRUSTED_DEVICE_COOKIE_NAME] = { + value: JSON.generate({ created_at: Time.zone.now }), + expires: TRUSTED_DEVICE_PERIOD, + httponly: true + } + end + + def trusted_device? + trusted_device_cookie.present? && + Time.zone.now - TRUSTED_DEVICE_PERIOD < JSON.parse(trusted_device_cookie)['created_at'] + end + + private + + def trusted_device_cookie + cookies.encrypted[TRUSTED_DEVICE_COOKIE_NAME] + end +end diff --git a/spec/controllers/administrateur/activate_controller_spec.rb b/spec/controllers/administrateur/activate_controller_spec.rb new file mode 100644 index 000000000..a6e99f851 --- /dev/null +++ b/spec/controllers/administrateur/activate_controller_spec.rb @@ -0,0 +1,20 @@ +describe Administrateurs::ActivateController, type: :controller do + describe '#new' do + let(:admin) { create(:administrateur) } + let(:token) { admin.send(:set_reset_password_token) } + + before { allow(controller).to receive(:trust_device) } + + context 'when the token is ok' do + before { get :new, params: { token: token } } + + it { expect(controller).to have_received(:trust_device) } + end + + context 'when the token is bad' do + before { get :new, params: { token: 'bad' } } + + it { expect(controller).not_to have_received(:trust_device) } + end + end +end diff --git a/spec/controllers/gestionnaires/activate_controller_spec.rb b/spec/controllers/gestionnaires/activate_controller_spec.rb new file mode 100644 index 000000000..fa7ce7857 --- /dev/null +++ b/spec/controllers/gestionnaires/activate_controller_spec.rb @@ -0,0 +1,20 @@ +describe Gestionnaires::ActivateController, type: :controller do + describe '#new' do + let(:gestionnaire) { create(:gestionnaire) } + let(:token) { gestionnaire.send(:set_reset_password_token) } + + before { allow(controller).to receive(:trust_device) } + + context 'when the token is ok' do + before { get :new, params: { token: token } } + + it { expect(controller).to have_received(:trust_device) } + end + + context 'when the token is bad' do + before { get :new, params: { token: 'bad' } } + + it { expect(controller).not_to have_received(:trust_device) } + end + end +end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 0e8eb02ea..620bc7968 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -48,6 +48,20 @@ describe Users::SessionsController, type: :controller do expect(subject.current_administrateur).to be(nil) end + context 'when the device is trusted' do + before do + allow(controller).to receive(:trusted_device?).and_return(true) + post :create, params: { user: { email: gestionnaire.email, password: gestionnaire.password } } + end + + it 'directly log the gestionnaire' do + expect(subject).to redirect_to gestionnaire_procedures_path + expect(subject.current_user).to be(nil) + expect(subject.current_gestionnaire).to eq(gestionnaire) + expect(subject.current_administrateur).to be(nil) + end + end + context 'signs administrateur in' do # an admin has always an gestionnaire role before { gestionnaire } @@ -237,6 +251,7 @@ describe Users::SessionsController, type: :controller do context 'when the gestionnaire has non other account' do let(:gestionnaire) { create(:gestionnaire) } before do + allow(controller).to receive(:trust_device) post :sign_in_by_link, params: { id: gestionnaire.id, login_token: login_token } end @@ -245,6 +260,7 @@ describe Users::SessionsController, type: :controller do it { is_expected.to redirect_to gestionnaire_procedures_path } it { expect(controller.current_gestionnaire).to eq(gestionnaire) } + it { expect(controller).to have_received(:trust_device) } end context 'when the token is invalid' do @@ -252,6 +268,7 @@ describe Users::SessionsController, type: :controller do it { is_expected.to redirect_to new_user_session_path } it { expect(controller.current_gestionnaire).to be_nil } + it { expect(controller).not_to have_received(:trust_device) } end end @@ -276,4 +293,28 @@ describe Users::SessionsController, type: :controller do end end end + + describe '#trust_device and #trusted_device?' do + subject { controller.trusted_device? } + + context 'when the trusted cookie is not present' do + it { is_expected.to be false } + end + + context 'when the cookie is outdated' do + before do + Timecop.freeze(Time.zone.now - TrustedDeviceConcern::TRUSTED_DEVICE_PERIOD - 1.minute) + controller.trust_device + Timecop.return + end + + it { is_expected.to be false } + end + + context 'when the cookie is ok' do + before { controller.trust_device } + + it { is_expected.to be true } + end + end end From 87967568b76eae98fdd31f5c6a55e4e03dd87180 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 7 Nov 2018 10:30:17 +0100 Subject: [PATCH 10/14] SessionController: if a admin is login in, redirect to admin page --- app/controllers/users/sessions_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 9db5a4310..2adbdbc39 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -92,7 +92,11 @@ class Users::SessionsController < Sessions::SessionsController administrateur = Administrateur.find_by(email: gestionnaire.email) [user, gestionnaire, administrateur].compact.each { |resource| sign_in(resource) } - redirect_to gestionnaire_procedures_path + if administrateur.present? + redirect_to admin_procedures_path + else + redirect_to gestionnaire_procedures_path + end else flash[:alert] = 'Votre lien est invalide ou expiré, veuillez-vous reconnecter.' redirect_to new_user_session_path From 457bc13c755db422cee1898d70537731736183f3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 8 Nov 2018 14:39:23 +0100 Subject: [PATCH 11/14] =?UTF-8?q?SessionController:=20do=20not=20display?= =?UTF-8?q?=20'Connect=C3=A9'=20when=20a=20login=5Flink=20is=20required?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/users/sessions_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 2adbdbc39..497bc7ce0 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -28,6 +28,7 @@ class Users::SessionsController < Sessions::SessionsController if gestionnaire_signed_in? if trusted_device? + set_flash_message :notice, :signed_in redirect_to gestionnaire_procedures_path else gestionnaire = current_gestionnaire @@ -39,6 +40,7 @@ class Users::SessionsController < Sessions::SessionsController redirect_to link_sent_path(email: gestionnaire.email) end elsif user_signed_in? + set_flash_message :notice, :signed_in redirect_to after_sign_in_path_for(:user) else flash.alert = 'Mauvais couple login / mot de passe' @@ -127,7 +129,6 @@ class Users::SessionsController < Sessions::SessionsController resource.remember_me = remember_me sign_in resource resource.force_sync_credentials - set_flash_message :notice, :signed_in end end end From 36621bffebb3316f231c3906d749a221b6fe34d3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 8 Nov 2018 17:13:48 +0100 Subject: [PATCH 12/14] FlipFlop: enable for gestionnaire --- .../manager/gestionnaires_controller.rb | 14 ++++++++++++++ app/dashboards/gestionnaire_dashboard.rb | 6 ++++-- app/models/gestionnaire.rb | 17 +++++++++++++++++ app/views/fields/features_field/_show.html.haml | 9 ++++++++- config/routes.rb | 1 + ...1929_add_features_column_to_gestionnaires.rb | 5 +++++ db/schema.rb | 3 ++- 7 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20181108151929_add_features_column_to_gestionnaires.rb diff --git a/app/controllers/manager/gestionnaires_controller.rb b/app/controllers/manager/gestionnaires_controller.rb index 1caaf9156..0b83d772e 100644 --- a/app/controllers/manager/gestionnaires_controller.rb +++ b/app/controllers/manager/gestionnaires_controller.rb @@ -6,5 +6,19 @@ module Manager flash[:notice] = "Instructeur réinvité." redirect_to manager_gestionnaire_path(gestionnaire) end + + def enable_feature + gestionnaire = Gestionnaire.find(params[:id]) + + params[:features].each do |key, enable| + if enable + gestionnaire.enable_feature(key.to_sym) + else + gestionnaire.disable_feature(key.to_sym) + end + end + + head :ok + end end end diff --git a/app/dashboards/gestionnaire_dashboard.rb b/app/dashboards/gestionnaire_dashboard.rb index a1666edf7..2f609a677 100644 --- a/app/dashboards/gestionnaire_dashboard.rb +++ b/app/dashboards/gestionnaire_dashboard.rb @@ -14,7 +14,8 @@ class GestionnaireDashboard < Administrate::BaseDashboard updated_at: Field::DateTime, current_sign_in_at: Field::DateTime, dossiers: Field::HasMany, - procedures: Field::HasMany + procedures: Field::HasMany, + features: FeaturesField }.freeze # COLLECTION_ATTRIBUTES @@ -35,7 +36,8 @@ class GestionnaireDashboard < Administrate::BaseDashboard :id, :email, :current_sign_in_at, - :created_at + :created_at, + :features ].freeze # FORM_ATTRIBUTES diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 44ef980e6..7f6fdcf3b 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -205,6 +205,23 @@ class Gestionnaire < ApplicationRecord GestionnaireMailer.invite_gestionnaire(self, reset_password_token).deliver_later end + def feature_enabled?(feature) + Flipflop.feature_set.feature(feature) + features[feature.to_s] + end + + def disable_feature(feature) + Flipflop.feature_set.feature(feature) + features.delete(feature.to_s) + save + end + + def enable_feature(feature) + Flipflop.feature_set.feature(feature) + features[feature.to_s] = true + save + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/app/views/fields/features_field/_show.html.haml b/app/views/fields/features_field/_show.html.haml index d3d864800..27e2741ca 100644 --- a/app/views/fields/features_field/_show.html.haml +++ b/app/views/fields/features_field/_show.html.haml @@ -1,7 +1,14 @@ +:ruby + url = if field.resource.class.name == 'Gestionnaire' + enable_feature_manager_gestionnaire_path(field.resource.id) + else + enable_feature_manager_administrateur_path(field.resource.id) + end + %table#features - Flipflop.feature_set.features.each do |feature| - if !feature.group || feature.group.key != :production %tr %td= feature.title %td - = check_box_tag "enable-feature", "enable", field.data[feature.name], data: { url: enable_feature_manager_administrateur_path(field.resource.id), key: feature.key } + = check_box_tag "enable-feature", "enable", field.data[feature.name], data: { url: url, key: feature.key } diff --git a/config/routes.rb b/config/routes.rb index 4ba5ac98a..caa4e0487 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,6 +26,7 @@ Rails.application.routes.draw do resources :gestionnaires, only: [:index, :show] do post 'reinvite', on: :member + put 'enable_feature', on: :member end resources :dossiers, only: [:show] diff --git a/db/migrate/20181108151929_add_features_column_to_gestionnaires.rb b/db/migrate/20181108151929_add_features_column_to_gestionnaires.rb new file mode 100644 index 000000000..c90c6ebb6 --- /dev/null +++ b/db/migrate/20181108151929_add_features_column_to_gestionnaires.rb @@ -0,0 +1,5 @@ +class AddFeaturesColumnToGestionnaires < ActiveRecord::Migration[5.2] + def change + add_column :gestionnaires, :features, :jsonb, null: false, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index e6cba3c3a..8e642e1f6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_11_08_091339) do +ActiveRecord::Schema.define(version: 2018_11_08_151929) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -367,6 +367,7 @@ ActiveRecord::Schema.define(version: 2018_11_08_091339) do t.datetime "updated_at" t.text "encrypted_login_token" t.datetime "login_token_created_at" + t.jsonb "features", default: {}, null: false t.index ["email"], name: "index_gestionnaires_on_email", unique: true t.index ["reset_password_token"], name: "index_gestionnaires_on_reset_password_token", unique: true end From fb76197404f869dd75de668c9461b222b0235f78 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 8 Nov 2018 17:14:24 +0100 Subject: [PATCH 13/14] SessionController: flipflop trusted device --- app/controllers/users/sessions_controller.rb | 2 +- config/features.rb | 1 + spec/controllers/users/sessions_controller_spec.rb | 2 +- spec/factories/gestionnaire.rb | 6 ++++++ spec/features/admin/connection_spec.rb | 2 +- spec/features/new_gestionnaire/gestionnaire_spec.rb | 2 +- 6 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 497bc7ce0..610ce28b7 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -27,7 +27,7 @@ class Users::SessionsController < Sessions::SessionsController end if gestionnaire_signed_in? - if trusted_device? + if trusted_device? || !current_gestionnaire.feature_enabled?(:enable_email_login_token) set_flash_message :notice, :signed_in redirect_to gestionnaire_procedures_path else diff --git a/config/features.rb b/config/features.rb index 6ee9d203e..6ec364588 100644 --- a/config/features.rb +++ b/config/features.rb @@ -20,6 +20,7 @@ Flipflop.configure do feature :web_hook feature :publish_draft feature :support_form + feature :enable_email_login_token group :production do feature :remote_storage, diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 620bc7968..3fcc5e9de 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -27,7 +27,7 @@ describe Users::SessionsController, type: :controller do let(:password) { 'un super mot de passe' } let(:user) { create(:user, email: email, password: password) } - let(:gestionnaire) { create(:gestionnaire, email: email, password: password) } + let(:gestionnaire) { create(:gestionnaire, :with_trusted_device, email: email, password: password) } let(:administrateur) { create(:administrateur, email: email, password: password) } it 'signs user in' do diff --git a/spec/factories/gestionnaire.rb b/spec/factories/gestionnaire.rb index dc07d61e9..b93610555 100644 --- a/spec/factories/gestionnaire.rb +++ b/spec/factories/gestionnaire.rb @@ -4,4 +4,10 @@ FactoryBot.define do email { generate(:gestionnaire_email) } password { 'password' } end + + trait :with_trusted_device do + after(:create) do |gestionnaire| + gestionnaire.update(features: { "enable_email_login_token" => true }) + end + end end diff --git a/spec/features/admin/connection_spec.rb b/spec/features/admin/connection_spec.rb index 31c1230e4..1647e5bb7 100644 --- a/spec/features/admin/connection_spec.rb +++ b/spec/features/admin/connection_spec.rb @@ -6,7 +6,7 @@ feature 'Administrator connection' do let(:email) { 'admin1@admin.com' } let(:password) { 'mon chien aime les bananes' } let!(:admin) { create(:administrateur, email: email, password: password) } - let!(:gestionnaire) { create(:gestionnaire, email: email, password: password) } + let!(:gestionnaire) { create(:gestionnaire, :with_trusted_device, email: email, password: password) } before do visit new_administrateur_session_path diff --git a/spec/features/new_gestionnaire/gestionnaire_spec.rb b/spec/features/new_gestionnaire/gestionnaire_spec.rb index eb1881b05..e38f6c3f7 100644 --- a/spec/features/new_gestionnaire/gestionnaire_spec.rb +++ b/spec/features/new_gestionnaire/gestionnaire_spec.rb @@ -4,7 +4,7 @@ feature 'The gestionnaire part' do include ActiveJob::TestHelper let(:password) { 'secret_password' } - let!(:gestionnaire) { create(:gestionnaire, password: password) } + let!(:gestionnaire) { create(:gestionnaire, :with_trusted_device, password: password) } let!(:procedure) { create(:procedure, :published, gestionnaires: [gestionnaire]) } let!(:dossier) { create(:dossier, state: Dossier.states.fetch(:en_construction), procedure: procedure) } From 7313ca0675c30ae6dfdb760affaca43eb0e4c88a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 8 Nov 2018 17:53:19 +0100 Subject: [PATCH 14/14] Rake: enable trusted_device for some gestionnaire --- lib/tasks/2018_11_08_activate_trusted_device_for_a-f.rake | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 lib/tasks/2018_11_08_activate_trusted_device_for_a-f.rake diff --git a/lib/tasks/2018_11_08_activate_trusted_device_for_a-f.rake b/lib/tasks/2018_11_08_activate_trusted_device_for_a-f.rake new file mode 100644 index 000000000..fe55cb3ee --- /dev/null +++ b/lib/tasks/2018_11_08_activate_trusted_device_for_a-f.rake @@ -0,0 +1,8 @@ +namespace :'activate_trusted_device_for_a-f' do + task run: :environment do + letters_a_to_f = ('a'..'f').to_a + Gestionnaire + .where("substr(email, 1, 1) IN (?)", letters_a_to_f) + .update_all(features: { "enable_email_login_token" => true }) + end +end