From bee9a108c5963d5b1d5993c8aed6358180ff41af Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 1 Feb 2019 17:17:10 +0100 Subject: [PATCH 1/8] split login and trusted_device logic --- app/controllers/application_controller.rb | 29 ++++++ app/controllers/users/sessions_controller.rb | 39 +++----- app/models/concerns/trusted_device_concern.rb | 7 ++ .../application_controller_spec.rb | 51 ++++++++++ .../sessions/sessions_controller_spec.rb | 1 + .../users/sessions_controller_spec.rb | 98 +++++++++---------- 6 files changed, 143 insertions(+), 82 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3b1243d20..175cb05a9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,6 @@ class ApplicationController < ActionController::Base + include TrustedDeviceConcern + MAINTENANCE_MESSAGE = 'Le site est actuellement en maintenance. Il sera à nouveau disponible dans un court instant.' # Prevent CSRF attacks by raising an exception. @@ -6,6 +8,7 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception, if: -> { !Rails.env.test? } before_action :load_navbar_left_pannel_partial_url before_action :set_raven_context + before_action :redirect_if_untrusted before_action :authorize_request_for_profiler before_action :reject, if: -> { Flipflop.maintenance_mode? } @@ -151,4 +154,30 @@ class ApplicationController < ActionController::Base redirect_to root_path end end + + def redirect_if_untrusted + if gestionnaire_signed_in? && + sensitive_path && + current_gestionnaire.feature_enabled?(:enable_email_login_token) && + !trusted_device? + + send_login_token_or_bufferize(current_gestionnaire) + redirect_to link_sent_path(email: current_gestionnaire.email) + end + end + + def sensitive_path + path = request.path_info + + if path == '/' || + path == '/users/sign_out' || + path.start_with?('/connexion-par-jeton') || + path.start_with?('/api/') || + path.start_with?('/lien-envoye') + + false + else + true + end + end end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index ee94aed4a..b02263499 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -23,20 +23,7 @@ class Users::SessionsController < Sessions::SessionsController current_user.update(loged_in_with_france_connect: nil) end - if gestionnaire_signed_in? - if trusted_device? || !current_gestionnaire.feature_enabled?(:enable_email_login_token) - set_flash_message :notice, :signed_in - redirect_to after_sign_in_path_for(:user) - else - gestionnaire = current_gestionnaire - - send_login_token_or_bufferize(gestionnaire) - - [:user, :gestionnaire, :administrateur].each { |role| sign_out(role) } - - redirect_to link_sent_path(email: gestionnaire.email) - end - elsif user_signed_in? + if gestionnaire_signed_in? || user_signed_in? set_flash_message :notice, :signed_in redirect_to after_sign_in_path_for(:user) else @@ -87,28 +74,24 @@ class Users::SessionsController < Sessions::SessionsController 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) } - # redirect to procedure'url if stored by store_location_for(:user) in dossiers_controller # redirect to root_path otherwise - redirect_to after_sign_in_path_for(:user) + + if gestionnaire_signed_in? + redirect_to after_sign_in_path_for(:user) + else + redirect_to new_user_session_path + end else - flash[:alert] = 'Votre lien est invalide ou expiré, veuillez-vous reconnecter.' - redirect_to new_user_session_path + flash[:alert] = 'Votre lien est invalide ou expiré, un nouveau vient de vous être envoyé.' + + send_login_token_or_bufferize(gestionnaire) + redirect_to link_sent_path(email: gestionnaire.email) end end 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 try_to_authenticate(klass, remember_me = false) resource = klass.find_for_database_authentication(email: params[:user][:email]) diff --git a/app/models/concerns/trusted_device_concern.rb b/app/models/concerns/trusted_device_concern.rb index 9928adfa1..258fc15fe 100644 --- a/app/models/concerns/trusted_device_concern.rb +++ b/app/models/concerns/trusted_device_concern.rb @@ -17,6 +17,13 @@ module TrustedDeviceConcern (Time.zone.now - TRUSTED_DEVICE_PERIOD) < trusted_device_cookie_created_at end + 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 + private def trusted_device_cookie_created_at diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 0545d786b..cd8dbf75d 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -9,6 +9,7 @@ describe ApplicationController, type: :controller do .map(&:filter) expect(before_actions).to include(:set_raven_context) + expect(before_actions).to include(:redirect_if_untrusted) end end @@ -145,4 +146,54 @@ describe ApplicationController, type: :controller do it { expect(flash[:alert]).to eq(ApplicationController::MAINTENANCE_MESSAGE) } end end + + describe '#redirect_if_unstrusted' do + let(:current_gestionnaire) { create(:gestionnaire) } + + before do + allow(current_gestionnaire).to receive(:feature_enabled?).and_return(feature_enabled) + allow(@controller).to receive(:current_gestionnaire).and_return(current_gestionnaire) + + allow(@controller).to receive(:redirect_to) + allow(@controller).to receive(:trusted_device?).and_return(trusted_device) + allow(@controller).to receive(:gestionnaire_signed_in?).and_return(gestionnaire_signed_in) + allow(@controller).to receive(:sensitive_path).and_return(sensitive_path) + allow(@controller).to receive(:send_login_token_or_bufferize) + end + + subject { @controller.send(:redirect_if_untrusted) } + + context 'when the path is sensitive' do + let(:sensitive_path) { true } + + context 'when the gestionnaire is signed_in' do + let(:gestionnaire_signed_in) { true } + + context 'when the feature is activated' do + let(:feature_enabled) { true } + + context 'when the device is trusted' do + let(:trusted_device) { true } + + before { subject } + + it { expect(@controller).not_to have_received(:redirect_to) } + end + end + + context 'when the feature is activated' do + let(:feature_enabled) { true } + + context 'when the device is not trusted' do + let(:trusted_device) { false } + + before { subject } + + it { expect(@controller).to have_received(:redirect_to) } + it { expect(@controller).to have_received(:send_login_token_or_bufferize) } + end + end + end + end + end end diff --git a/spec/controllers/sessions/sessions_controller_spec.rb b/spec/controllers/sessions/sessions_controller_spec.rb index c0f5efc14..5d8e5839d 100644 --- a/spec/controllers/sessions/sessions_controller_spec.rb +++ b/spec/controllers/sessions/sessions_controller_spec.rb @@ -40,6 +40,7 @@ describe Sessions::SessionsController, type: :controller do @request.env["devise.mapping"] = Devise.mappings[:gestionnaire] allow_any_instance_of(described_class).to receive(:gestionnaire_signed_in?).and_return(true) + allow_any_instance_of(described_class).to receive(:current_gestionnaire).and_return(gestionnaire) end it 'calls sign out for gestionnaire' do diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index efa388790..cdab44c91 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -28,31 +28,15 @@ describe Users::SessionsController, type: :controller do context 'when the device is not trusted' do let(:trusted_device) { false } - it 'redirects to the confirmation link path' do + it 'redirects to the root path' do subject - expect(controller).to redirect_to link_sent_path(email: email) + expect(controller).to redirect_to(root_path) - # do not know why, should be test related expect(controller.current_user).to eq(user) - - 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 + expect(controller.current_gestionnaire).to eq(gestionnaire) + expect(controller.current_administrateur).to eq(administrateur) + expect(user.loged_in_with_france_connect).to eq(nil) end end @@ -69,7 +53,6 @@ describe Users::SessionsController, type: :controller do 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 @@ -193,49 +176,56 @@ describe Users::SessionsController, type: :controller do context 'when the gestionnaire has non other account' do let(:gestionnaire) { create(:gestionnaire) } let!(:good_jeton) { gestionnaire.login_token! } + let(:logged) { false } before do + if logged + sign_in gestionnaire + end allow(controller).to receive(:trust_device) + allow(controller).to receive(:send_login_token_or_bufferize) post :sign_in_by_link, params: { id: gestionnaire.id, jeton: jeton } end - context 'when the token is valid' do - let(:jeton) { good_jeton } + context 'when the gestionnaire is not logged in' do + context 'when the token is valid' do + let(:jeton) { good_jeton } - # TODO when the gestionnaire has no other account, and the token is valid, and the user signing in was not starting a demarche, - # redirect to root_path, then redirect to gestionnaire_procedures_path (see root_controller) - it { is_expected.to redirect_to root_path } - it { expect(controller.current_gestionnaire).to eq(gestionnaire) } - it { expect(controller).to have_received(:trust_device) } + it { is_expected.to redirect_to new_user_session_path } + it { expect(controller.current_gestionnaire).to be_nil } + it { expect(controller).to have_received(:trust_device) } + end + + context 'when the token is invalid' do + let(:jeton) { 'invalid_token' } + + it { is_expected.to redirect_to link_sent_path(email: gestionnaire.email) } + it { expect(controller.current_gestionnaire).to be_nil } + it { expect(controller).not_to have_received(:trust_device) } + it { expect(controller).to have_received(:send_login_token_or_bufferize) } + end end - context 'when the token is invalid' do - let(:jeton) { 'invalid_token' } + context 'when the gestionnaire is logged in' do + let(:logged) { true } - 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 + context 'when the token is valid' do + let(:jeton) { good_jeton } - context 'when the gestionnaire has an user and admin account' do - let(:email) { 'unique@plop.com' } - let(:password) { 'un super mot de passe' } + # redirect to root_path, then redirect to gestionnaire_procedures_path (see root_controller) + it { is_expected.to redirect_to root_path } + it { expect(controller.current_gestionnaire).to eq(gestionnaire) } + it { expect(controller).to have_received(:trust_device) } + end - let!(:user) { create(:user, email: email, password: password) } - let!(:administrateur) { create(:administrateur, email: email, password: password) } - let(:gestionnaire) { administrateur.gestionnaire } + context 'when the token is invalid' do + let(:jeton) { 'invalid_token' } - before do - post :sign_in_by_link, params: { id: gestionnaire.id, jeton: jeton } - end - - context 'when the token is valid' do - let(:jeton) { 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) } + it { is_expected.to redirect_to link_sent_path(email: gestionnaire.email) } + it { expect(controller.current_gestionnaire).to eq(gestionnaire) } + it { expect(controller).not_to have_received(:trust_device) } + it { expect(controller).to have_received(:send_login_token_or_bufferize) } + end end end end @@ -250,7 +240,7 @@ describe Users::SessionsController, type: :controller do context 'when the cookie is outdated' do before do Timecop.freeze(Time.zone.now - TrustedDeviceConcern::TRUSTED_DEVICE_PERIOD - 1.minute) - controller.trust_device + controller.trust_device(Time.zone.now) Timecop.return end @@ -258,7 +248,7 @@ describe Users::SessionsController, type: :controller do end context 'when the cookie is ok' do - before { controller.trust_device } + before { controller.trust_device(Time.zone.now) } it { is_expected.to be true } end From b9b83cca3a7f9153b115e47232144614b3c8c055 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 1 Feb 2019 18:11:55 +0100 Subject: [PATCH 2/8] use multiple trusted_device_token --- app/models/gestionnaire.rb | 21 +++++++++---------- app/models/trusted_device_token.rb | 4 ++++ ...0201164951_create_trusted_device_tokens.rb | 11 ++++++++++ db/schema.rb | 10 +++++++++ 4 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 app/models/trusted_device_token.rb create mode 100644 db/migrate/20190201164951_create_trusted_device_tokens.rb diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 7ac39c9e5..df3284403 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -1,7 +1,6 @@ class Gestionnaire < ApplicationRecord include CredentialsSyncableConcern include EmailSanitizableConcern - include ActiveRecord::SecureToken LOGIN_TOKEN_VALIDITY = 45.minutes LOGIN_TOKEN_YOUTH = 15.minutes @@ -20,6 +19,7 @@ class Gestionnaire < ApplicationRecord has_many :followed_dossiers, through: :follows, source: :dossier has_many :avis has_many :dossiers_from_avis, through: :avis, source: :dossier + has_many :trusted_device_tokens def visible_procedures procedures.merge(Procedure.avec_lien.or(Procedure.archivees)) @@ -136,17 +136,15 @@ class Gestionnaire < ApplicationRecord 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 + trusted_device_token = trusted_device_tokens.create + trusted_device_token.token end def login_token_valid?(login_token) - BCrypt::Password.new(encrypted_login_token) == login_token && - LOGIN_TOKEN_VALIDITY.ago < login_token_created_at - rescue BCrypt::Errors::InvalidHash - false + trusted_device_token = trusted_device_tokens.find_by(token: login_token) + + trusted_device_token.present? && + LOGIN_TOKEN_VALIDITY.ago < trusted_device_token.created_at end def dossiers_id_with_notifications(dossiers) @@ -213,8 +211,9 @@ class Gestionnaire < ApplicationRecord end def young_login_token? - login_token_created_at.present? && - LOGIN_TOKEN_YOUTH.ago < login_token_created_at + trusted_device_token = trusted_device_tokens.order(created_at: :desc).first + trusted_device_token.present? && + LOGIN_TOKEN_YOUTH.ago < trusted_device_token.created_at end private diff --git a/app/models/trusted_device_token.rb b/app/models/trusted_device_token.rb new file mode 100644 index 000000000..848a9c689 --- /dev/null +++ b/app/models/trusted_device_token.rb @@ -0,0 +1,4 @@ +class TrustedDeviceToken < ApplicationRecord + belongs_to :gestionnaire + has_secure_token +end diff --git a/db/migrate/20190201164951_create_trusted_device_tokens.rb b/db/migrate/20190201164951_create_trusted_device_tokens.rb new file mode 100644 index 000000000..c53bec685 --- /dev/null +++ b/db/migrate/20190201164951_create_trusted_device_tokens.rb @@ -0,0 +1,11 @@ +class CreateTrustedDeviceTokens < ActiveRecord::Migration[5.2] + def change + create_table :trusted_device_tokens do |t| + t.string :token, null: false + t.references :gestionnaire, foreign_key: true + + t.timestamps + end + add_index :trusted_device_tokens, :token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 74265b893..05ee04c74 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -520,6 +520,15 @@ ActiveRecord::Schema.define(version: 2019_02_13_144145) do t.string "version", null: false end + create_table "trusted_device_tokens", force: :cascade do |t| + t.string "token", null: false + t.bigint "gestionnaire_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["gestionnaire_id"], name: "index_trusted_device_tokens_on_gestionnaire_id" + t.index ["token"], name: "index_trusted_device_tokens_on_token", unique: true + end + create_table "types_de_champ", id: :serial, force: :cascade do |t| t.string "libelle" t.string "type_champ" @@ -611,6 +620,7 @@ ActiveRecord::Schema.define(version: 2019_02_13_144145) do add_foreign_key "received_mails", "procedures" add_foreign_key "refused_mails", "procedures" add_foreign_key "services", "administrateurs" + add_foreign_key "trusted_device_tokens", "gestionnaires" add_foreign_key "types_de_champ", "types_de_champ", column: "parent_id" add_foreign_key "without_continuation_mails", "procedures" end From 23db8a160c41c94b443e3f8a934c891829e08416 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Sat, 2 Feb 2019 22:16:11 +0100 Subject: [PATCH 3/8] move token validity to trusted_device_token --- app/controllers/users/sessions_controller.rb | 6 +++++- app/models/gestionnaire.rb | 8 -------- app/models/trusted_device_token.rb | 6 ++++++ spec/models/gestionnaire_spec.rb | 20 -------------------- spec/models/trusted_device_token_spec.rb | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 29 deletions(-) create mode 100644 spec/models/trusted_device_token_spec.rb diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index b02263499..bd7dcd879 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -70,7 +70,11 @@ class Users::SessionsController < Sessions::SessionsController def sign_in_by_link gestionnaire = Gestionnaire.find(params[:id]) - if gestionnaire&.login_token_valid?(params[:jeton]) + trusted_device_token = gestionnaire + .trusted_device_tokens + .find_by(token: params[:jeton]) + + if trusted_device_token&.token_valid? 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." diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index df3284403..7b35b5f92 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -2,7 +2,6 @@ class Gestionnaire < ApplicationRecord include CredentialsSyncableConcern include EmailSanitizableConcern - LOGIN_TOKEN_VALIDITY = 45.minutes LOGIN_TOKEN_YOUTH = 15.minutes devise :database_authenticatable, :registerable, :async, @@ -140,13 +139,6 @@ class Gestionnaire < ApplicationRecord trusted_device_token.token end - def login_token_valid?(login_token) - trusted_device_token = trusted_device_tokens.find_by(token: login_token) - - trusted_device_token.present? && - LOGIN_TOKEN_VALIDITY.ago < trusted_device_token.created_at - end - def dossiers_id_with_notifications(dossiers) dossiers = dossiers.followed_by(self) diff --git a/app/models/trusted_device_token.rb b/app/models/trusted_device_token.rb index 848a9c689..01071ead9 100644 --- a/app/models/trusted_device_token.rb +++ b/app/models/trusted_device_token.rb @@ -1,4 +1,10 @@ class TrustedDeviceToken < ApplicationRecord + LOGIN_TOKEN_VALIDITY = 45.minutes + belongs_to :gestionnaire has_secure_token + + def token_valid? + LOGIN_TOKEN_VALIDITY.ago < created_at + end end diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 344ee1683..493d4a3a1 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -392,26 +392,6 @@ describe Gestionnaire, type: :model do end end - describe '#login_token_valid?' do - let!(:gestionnaire) { create(:gestionnaire) } - let!(:good_token) { gestionnaire.login_token! } - - it { expect(gestionnaire.login_token_valid?(good_token)).to be true } - it { expect(gestionnaire.login_token_valid?('bad_token')).to be false } - - context 'when the token as expired' do - before { gestionnaire.update(login_token_created_at: (Gestionnaire::LOGIN_TOKEN_VALIDITY + 1.minute).ago) } - - it { expect(gestionnaire.login_token_valid?(good_token)).to be false } - end - - context 'when the gestionnaire does not have a token' do - before { gestionnaire.update(encrypted_login_token: nil) } - - it { expect(gestionnaire.login_token_valid?(nil)).to be false } - end - end - describe '#young_login_token?' do let!(:gestionnaire) { create(:gestionnaire) } diff --git a/spec/models/trusted_device_token_spec.rb b/spec/models/trusted_device_token_spec.rb new file mode 100644 index 000000000..4557bf439 --- /dev/null +++ b/spec/models/trusted_device_token_spec.rb @@ -0,0 +1,15 @@ +RSpec.describe TrustedDeviceToken, type: :model do + describe '#token_valid?' do + let(:token) { TrustedDeviceToken.create } + + context 'when the token is create after login_token_validity' do + it { expect(token.token_valid?).to be true } + end + + context 'when the token is create before login_token_validity' do + before { token.update(created_at: (TrustedDeviceToken::LOGIN_TOKEN_VALIDITY + 1.minute).ago) } + + it { expect(token.token_valid?).to be false } + end + end +end From d664f130fd60aa178480465a3f78c8ecbfc2e75b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Feb 2019 11:04:55 +0100 Subject: [PATCH 4/8] trustedDeviceToken: move token youth --- app/models/gestionnaire.rb | 5 +---- app/models/trusted_device_token.rb | 5 +++++ spec/models/gestionnaire_spec.rb | 4 ++-- spec/models/trusted_device_token_spec.rb | 14 ++++++++++++++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 7b35b5f92..9a038586d 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -2,8 +2,6 @@ class Gestionnaire < ApplicationRecord include CredentialsSyncableConcern include EmailSanitizableConcern - LOGIN_TOKEN_YOUTH = 15.minutes - devise :database_authenticatable, :registerable, :async, :recoverable, :rememberable, :trackable, :validatable @@ -204,8 +202,7 @@ class Gestionnaire < ApplicationRecord def young_login_token? trusted_device_token = trusted_device_tokens.order(created_at: :desc).first - trusted_device_token.present? && - LOGIN_TOKEN_YOUTH.ago < trusted_device_token.created_at + trusted_device_token&.token_young? end private diff --git a/app/models/trusted_device_token.rb b/app/models/trusted_device_token.rb index 01071ead9..4b485f832 100644 --- a/app/models/trusted_device_token.rb +++ b/app/models/trusted_device_token.rb @@ -1,5 +1,6 @@ class TrustedDeviceToken < ApplicationRecord LOGIN_TOKEN_VALIDITY = 45.minutes + LOGIN_TOKEN_YOUTH = 15.minutes belongs_to :gestionnaire has_secure_token @@ -7,4 +8,8 @@ class TrustedDeviceToken < ApplicationRecord def token_valid? LOGIN_TOKEN_VALIDITY.ago < created_at end + + def token_young? + LOGIN_TOKEN_YOUTH.ago < created_at + end end diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 493d4a3a1..5385e246c 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -403,13 +403,13 @@ describe Gestionnaire, type: :model do end context 'when the token is a bit old' do - before { gestionnaire.update(login_token_created_at: (Gestionnaire::LOGIN_TOKEN_YOUTH + 1.minute).ago) } + before { gestionnaire.trusted_device_tokens.first.update(created_at: (TrustedDeviceToken::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 } + it { expect(gestionnaire.young_login_token?).to be_falsey } end end diff --git a/spec/models/trusted_device_token_spec.rb b/spec/models/trusted_device_token_spec.rb index 4557bf439..ed5ae1e51 100644 --- a/spec/models/trusted_device_token_spec.rb +++ b/spec/models/trusted_device_token_spec.rb @@ -12,4 +12,18 @@ RSpec.describe TrustedDeviceToken, type: :model do it { expect(token.token_valid?).to be false } end end + + describe '#token_young?' do + let(:token) { TrustedDeviceToken.create } + + context 'when the token is create after login_token_youth' do + it { expect(token.token_young?).to be true } + end + + context 'when the token is create before login_token_youth' do + before { token.update(created_at: (TrustedDeviceToken::LOGIN_TOKEN_YOUTH + 1.minute).ago) } + + it { expect(token.token_young?).to be false } + end + end end From 7de3a18fd1c1e25d2212b34a37076c72f06b1e9e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Feb 2019 11:57:50 +0100 Subject: [PATCH 5/8] valid period depend on trusted_device_token.created_at --- app/controllers/users/sessions_controller.rb | 7 +++++-- app/models/concerns/trusted_device_concern.rb | 6 +++--- spec/controllers/users/sessions_controller_spec.rb | 5 ++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index bd7dcd879..783043bcf 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -75,8 +75,11 @@ class Users::SessionsController < Sessions::SessionsController .find_by(token: params[:jeton]) if trusted_device_token&.token_valid? - 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." + trust_device(trusted_device_token.created_at) + + period = ((trusted_device_token.created_at + TRUSTED_DEVICE_PERIOD) - Time.zone.now).to_i / ActiveSupport::Duration::SECONDS_PER_DAY + + flash.notice = "Merci d’avoir confirmé votre connexion. Votre navigateur est maintenant authentifié pour #{period} jours." # redirect to procedure'url if stored by store_location_for(:user) in dossiers_controller # redirect to root_path otherwise diff --git a/app/models/concerns/trusted_device_concern.rb b/app/models/concerns/trusted_device_concern.rb index 258fc15fe..800c09353 100644 --- a/app/models/concerns/trusted_device_concern.rb +++ b/app/models/concerns/trusted_device_concern.rb @@ -4,10 +4,10 @@ module TrustedDeviceConcern TRUSTED_DEVICE_COOKIE_NAME = :trusted_device TRUSTED_DEVICE_PERIOD = 1.month - def trust_device + def trust_device(start_at) cookies.encrypted[TRUSTED_DEVICE_COOKIE_NAME] = { - value: JSON.generate({ created_at: Time.zone.now }), - expires: TRUSTED_DEVICE_PERIOD, + value: JSON.generate({ created_at: start_at }), + expires: start_at + TRUSTED_DEVICE_PERIOD, httponly: true } end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index cdab44c91..c84415207 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -239,9 +239,8 @@ describe Users::SessionsController, type: :controller do context 'when the cookie is outdated' do before do - Timecop.freeze(Time.zone.now - TrustedDeviceConcern::TRUSTED_DEVICE_PERIOD - 1.minute) - controller.trust_device(Time.zone.now) - Timecop.return + emission_date = Time.zone.now - TrustedDeviceConcern::TRUSTED_DEVICE_PERIOD - 1.minute + controller.trust_device(emission_date) end it { is_expected.to be false } From c16e30442aaed2dc06b54b8fbcae1c9c7ec51c9d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 6 Feb 2019 20:51:04 +0100 Subject: [PATCH 6/8] save path before redirect to link_sent_path --- app/controllers/application_controller.rb | 4 ++++ spec/controllers/application_controller_spec.rb | 2 ++ 2 files changed, 6 insertions(+) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 175cb05a9..4bcaa901a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -161,6 +161,10 @@ class ApplicationController < ActionController::Base current_gestionnaire.feature_enabled?(:enable_email_login_token) && !trusted_device? + # return at this location + # after the device is trusted + store_location_for(:user, request.fullpath) + send_login_token_or_bufferize(current_gestionnaire) redirect_to link_sent_path(email: current_gestionnaire.email) end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index cd8dbf75d..ce5777b40 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -159,6 +159,7 @@ describe ApplicationController, type: :controller do allow(@controller).to receive(:gestionnaire_signed_in?).and_return(gestionnaire_signed_in) allow(@controller).to receive(:sensitive_path).and_return(sensitive_path) allow(@controller).to receive(:send_login_token_or_bufferize) + allow(@controller).to receive(:store_location_for) end subject { @controller.send(:redirect_if_untrusted) } @@ -191,6 +192,7 @@ describe ApplicationController, type: :controller do it { expect(@controller).to have_received(:redirect_to) } it { expect(@controller).to have_received(:send_login_token_or_bufferize) } + it { expect(@controller).to have_received(:store_location_for) } end end end From 47e3b57e81813496bddd58c3bcc926577713afd3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 12 Feb 2019 17:35:19 +0100 Subject: [PATCH 7/8] TrustedDeviceToken: valid for one week --- app/models/trusted_device_token.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/trusted_device_token.rb b/app/models/trusted_device_token.rb index 4b485f832..951ccbe26 100644 --- a/app/models/trusted_device_token.rb +++ b/app/models/trusted_device_token.rb @@ -1,5 +1,5 @@ class TrustedDeviceToken < ApplicationRecord - LOGIN_TOKEN_VALIDITY = 45.minutes + LOGIN_TOKEN_VALIDITY = 1.week LOGIN_TOKEN_YOUTH = 15.minutes belongs_to :gestionnaire From 0b8619be77c017a694758a063c5be09e1b55e6a8 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 18 Feb 2019 17:05:30 +0100 Subject: [PATCH 8/8] Gestionnaire: login_token! -> create_trusted_device_token --- app/models/concerns/trusted_device_concern.rb | 2 +- app/models/gestionnaire.rb | 2 +- spec/controllers/users/sessions_controller_spec.rb | 2 +- spec/models/gestionnaire_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/trusted_device_concern.rb b/app/models/concerns/trusted_device_concern.rb index 800c09353..f7dfb8e92 100644 --- a/app/models/concerns/trusted_device_concern.rb +++ b/app/models/concerns/trusted_device_concern.rb @@ -19,7 +19,7 @@ module TrustedDeviceConcern def send_login_token_or_bufferize(gestionnaire) if !gestionnaire.young_login_token? - login_token = gestionnaire.login_token! + login_token = gestionnaire.create_trusted_device_token GestionnaireMailer.send_login_token(gestionnaire, login_token).deliver_later end end diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 9a038586d..82516d4de 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -132,7 +132,7 @@ class Gestionnaire < ApplicationRecord Dossier.where(id: dossiers_id_with_notifications(dossiers)).group(:procedure_id).count end - def login_token! + def create_trusted_device_token trusted_device_token = trusted_device_tokens.create trusted_device_token.token end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c84415207..e644fa74a 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -175,7 +175,7 @@ describe Users::SessionsController, type: :controller do describe '#sign_in_by_link' do context 'when the gestionnaire has non other account' do let(:gestionnaire) { create(:gestionnaire) } - let!(:good_jeton) { gestionnaire.login_token! } + let!(:good_jeton) { gestionnaire.create_trusted_device_token } let(:logged) { false } before do diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 5385e246c..2a28e5f6e 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -396,7 +396,7 @@ describe Gestionnaire, type: :model do let!(:gestionnaire) { create(:gestionnaire) } context 'when there is a token' do - let!(:good_token) { gestionnaire.login_token! } + let!(:good_token) { gestionnaire.create_trusted_device_token } context 'when the token has just been created' do it { expect(gestionnaire.young_login_token?).to be true }