From 73e2e2242e6114da603f43102de9db0ad209f40b Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 25 Jul 2022 14:27:18 -0400 Subject: [PATCH 1/8] changelog: Bug Fixes,Secure Header allows CSRF redirect, LG-6992 --- app/controllers/users/additional_mfa_required_controller.rb | 1 + app/controllers/users/mfa_selection_controller.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/controllers/users/additional_mfa_required_controller.rb b/app/controllers/users/additional_mfa_required_controller.rb index 985b682d351..20e443f5617 100644 --- a/app/controllers/users/additional_mfa_required_controller.rb +++ b/app/controllers/users/additional_mfa_required_controller.rb @@ -1,5 +1,6 @@ module Users class AdditionalMfaRequiredController < ApplicationController + include SecureHeadersConcern extend ActiveSupport::Concern def show diff --git a/app/controllers/users/mfa_selection_controller.rb b/app/controllers/users/mfa_selection_controller.rb index 16a0f5f607f..5dd11276c2b 100644 --- a/app/controllers/users/mfa_selection_controller.rb +++ b/app/controllers/users/mfa_selection_controller.rb @@ -1,6 +1,7 @@ module Users class MfaSelectionController < ApplicationController include UserAuthenticator + include SecureHeadersConcern include MfaSetupConcern before_action :authenticate_user From d419178f83c7c1e35dcef976078286e95278d50f Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Jul 2022 09:37:40 -0400 Subject: [PATCH 2/8] changelog: Bug Fixes, Authentication, add CSR concern to new controllers (LG-6992) --- app/controllers/mfa_confirmation_controller.rb | 1 + app/controllers/users/additional_mfa_required_controller.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/controllers/mfa_confirmation_controller.rb b/app/controllers/mfa_confirmation_controller.rb index ee5db497983..c29de0356bf 100644 --- a/app/controllers/mfa_confirmation_controller.rb +++ b/app/controllers/mfa_confirmation_controller.rb @@ -1,5 +1,6 @@ class MfaConfirmationController < ApplicationController include MfaSetupConcern + include SecureHeadersConcern before_action :confirm_two_factor_authenticated def show diff --git a/app/controllers/users/additional_mfa_required_controller.rb b/app/controllers/users/additional_mfa_required_controller.rb index 20e443f5617..4e033a58fc8 100644 --- a/app/controllers/users/additional_mfa_required_controller.rb +++ b/app/controllers/users/additional_mfa_required_controller.rb @@ -3,6 +3,8 @@ class AdditionalMfaRequiredController < ApplicationController include SecureHeadersConcern extend ActiveSupport::Concern + before_action :confirm_user_fully_uathenticated + def show @content = AdditionalMfaRequiredPresenter.new(current_user: current_user) analytics.non_restricted_mfa_required_prompt_visited From 2393e7595ec90f69670a8e57026626dd3c6edf32 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Jul 2022 13:13:24 -0400 Subject: [PATCH 3/8] additional mfa required --- app/controllers/users/additional_mfa_required_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/users/additional_mfa_required_controller.rb b/app/controllers/users/additional_mfa_required_controller.rb index 4e033a58fc8..b79b8b93c45 100644 --- a/app/controllers/users/additional_mfa_required_controller.rb +++ b/app/controllers/users/additional_mfa_required_controller.rb @@ -27,5 +27,11 @@ def skip def enforcement_date @enforcement_date ||= IdentityConfig.store.kantara_restriction_enforcement_date end + + def confirm_user_fully_uathenticated + unless user_fully_authenticated? + return confirm_two_factor_authenticated(sp_session[:request_id]) + end + end end end From 5e9eaabe9bec9ed8bae02b3fee6fd0de7b71225b Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 26 Jul 2022 12:42:27 -0500 Subject: [PATCH 4/8] add failing spec --- .../additional_mfa_required_controller.rb | 4 ++-- ...additional_mfa_required_controller_spec.rb | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/controllers/users/additional_mfa_required_controller.rb b/app/controllers/users/additional_mfa_required_controller.rb index b79b8b93c45..a451a659104 100644 --- a/app/controllers/users/additional_mfa_required_controller.rb +++ b/app/controllers/users/additional_mfa_required_controller.rb @@ -3,7 +3,7 @@ class AdditionalMfaRequiredController < ApplicationController include SecureHeadersConcern extend ActiveSupport::Concern - before_action :confirm_user_fully_uathenticated + before_action :confirm_user_fully_authenticated def show @content = AdditionalMfaRequiredPresenter.new(current_user: current_user) @@ -28,7 +28,7 @@ def enforcement_date @enforcement_date ||= IdentityConfig.store.kantara_restriction_enforcement_date end - def confirm_user_fully_uathenticated + def confirm_user_fully_authenticated unless user_fully_authenticated? return confirm_two_factor_authenticated(sp_session[:request_id]) end diff --git a/spec/controllers/users/additional_mfa_required_controller_spec.rb b/spec/controllers/users/additional_mfa_required_controller_spec.rb index 1c3083ef9d8..b3bb1dda471 100644 --- a/spec/controllers/users/additional_mfa_required_controller_spec.rb +++ b/spec/controllers/users/additional_mfa_required_controller_spec.rb @@ -1,21 +1,28 @@ require 'rails_helper' describe Users::AdditionalMfaRequiredController do + render_views let(:user) { build(:user, :with_phone) } before do allow(IdentityConfig.store). to receive(:kantara_2fa_phone_existing_user_restriction). and_return(true) - stub_sign_in(user) end describe '#show' do it 'presents the additional mfa required prompt page.' do + stub_sign_in(user) get :show expect(response.status).to eq 200 end + + it 'does not allow unauthenticated users' do + get :show + + expect(response).to redirect_to(new_user_session_path) + end end describe '#skip' do @@ -24,8 +31,10 @@ allow(IdentityConfig.store).to receive(:kantara_restriction_enforcement_date). and_return(enforcement_date) end + context 'before enforcement date' do it 'should redirect to user signin' do + stub_sign_in(user) post :skip expect(response).to redirect_to account_url end @@ -35,18 +44,26 @@ let(:enforcement_date) { Time.zone.today - 6.days } it 'should redirect user to sign in' do + stub_sign_in(user) post :skip expect(response).to redirect_to account_url end it 'should add sign in attribute to users' do + stub_sign_in(user) post :skip user.reload expect(user.non_restricted_mfa_required_prompt_skip_date). to eq Time.zone.today end + + it 'does not allow unauthenticated users' do + post :skip + + expect(response).to redirect_to(new_user_session_path) + end end end end From b42a273515964d5496b0a19e8b456d82a5ed893d Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Jul 2022 17:14:31 -0400 Subject: [PATCH 5/8] make sure that user session is included --- app/controllers/concerns/mfa_setup_concern.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 267868c0ef3..28e7604498e 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -58,7 +58,8 @@ def mfa_context end def suggest_second_mfa? - mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 + return false unless user_session[:mfa_selections] + mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 end def mfa_selection_count From 9fd203546598b9451d0a55bf89d3e90b925b733a Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Jul 2022 17:22:17 -0400 Subject: [PATCH 6/8] remove header from mfa confirmation page --- app/controllers/mfa_confirmation_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/mfa_confirmation_controller.rb b/app/controllers/mfa_confirmation_controller.rb index c29de0356bf..ee5db497983 100644 --- a/app/controllers/mfa_confirmation_controller.rb +++ b/app/controllers/mfa_confirmation_controller.rb @@ -1,6 +1,5 @@ class MfaConfirmationController < ApplicationController include MfaSetupConcern - include SecureHeadersConcern before_action :confirm_two_factor_authenticated def show From 509c9cc8622b001bfa70c99887507aa43d857bda Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 27 Jul 2022 08:31:17 -0500 Subject: [PATCH 7/8] remove render_views --- .../controllers/users/additional_mfa_required_controller_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/controllers/users/additional_mfa_required_controller_spec.rb b/spec/controllers/users/additional_mfa_required_controller_spec.rb index b3bb1dda471..a7d0ee4d59d 100644 --- a/spec/controllers/users/additional_mfa_required_controller_spec.rb +++ b/spec/controllers/users/additional_mfa_required_controller_spec.rb @@ -1,7 +1,6 @@ require 'rails_helper' describe Users::AdditionalMfaRequiredController do - render_views let(:user) { build(:user, :with_phone) } before do From c23082fb3113445a31e26d996b76fa0df7787f20 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 27 Jul 2022 09:21:21 -0500 Subject: [PATCH 8/8] remove trailing space --- app/controllers/concerns/mfa_setup_concern.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 28e7604498e..20277e292a5 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -59,7 +59,7 @@ def mfa_context def suggest_second_mfa? return false unless user_session[:mfa_selections] - mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 + mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 end def mfa_selection_count