From 773f481e096e093029c61a1ee89fdd85a5b83e15 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Wed, 12 Sep 2018 23:44:16 -0400 Subject: [PATCH 1/4] Fix PIV/CAC and Webauthn errors when disabled **Why**: When the feature flags are disabled the routes are not valid so references to the urls throw an error unless guarded with a feature flag. **How**: Add feature flag checks for all PIV/CAC and webauthn urls --- .../two_factor_authentication/options_controller.rb | 4 ++-- app/presenters/two_factor_options_presenter.rb | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/two_factor_authentication/options_controller.rb b/app/controllers/two_factor_authentication/options_controller.rb index ea27e812c09..1029427b2f2 100644 --- a/app/controllers/two_factor_authentication/options_controller.rb +++ b/app/controllers/two_factor_authentication/options_controller.rb @@ -33,8 +33,8 @@ def process_valid_form 'personal_key' => login_two_factor_personal_key_url, 'sms' => otp_send_url(otp_delivery_selection_form: { otp_delivery_preference: 'sms' }), 'auth_app' => login_two_factor_authenticator_url, - 'piv_cac' => login_two_factor_piv_cac_url, - 'webauthn' => login_two_factor_webauthn_url, + 'piv_cac' => FeatureManagement.piv_cac_enabled? ? login_two_factor_piv_cac_url : nil, + 'webauthn' => FeatureManagement.webauthn_enabled? ? login_two_factor_webauthn_url : nil, } url = factor_to_url[@two_factor_options_form.selection] redirect_to url if url diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index ece7443f26e..4308174e96a 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -38,7 +38,11 @@ def options private def available_2fa_types - %w[sms voice auth_app webauthn] + piv_cac_if_available + %w[sms voice auth_app] + webauthn_if_available + piv_cac_if_available + end + + def webauthn_if_available + FeatureManagement.webauthn_enabled? ? %w[webauthn] : [] end def piv_cac_if_available From b388392d5f48cc3cd9acdaac18bfac358bd32dfb Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Thu, 13 Sep 2018 23:03:28 -0400 Subject: [PATCH 2/4] Added test coverage for piv cac and webauthn disabled --- .../options_controller_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/controllers/two_factor_authentication/options_controller_spec.rb b/spec/controllers/two_factor_authentication/options_controller_spec.rb index b37a3f13794..82f70e9ea3d 100644 --- a/spec/controllers/two_factor_authentication/options_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/options_controller_spec.rb @@ -24,6 +24,17 @@ describe '#create' do before { sign_in_before_2fa } + it 'redirects to login_two_factor_url if sms and piv/cac and webauthn disabled' do + set_piv_cac_webauthn_enabled('false') + + post :create, params: { two_factor_options_form: { selection: 'sms' } } + + expect(response).to redirect_to otp_send_url( \ + otp_delivery_selection_form: { otp_delivery_preference: 'sms' }) + + set_piv_cac_webauthn_enabled('true') + end + it 'redirects to login_two_factor_url if user selects sms' do post :create, params: { two_factor_options_form: { selection: 'sms' } } @@ -80,4 +91,10 @@ post :create, params: { two_factor_options_form: { selection: 'sms' } } end end + + def set_piv_cac_webauthn_enabled(bool) + allow(Figaro.env).to receive(:piv_cac_enabled) { bool } + allow(Figaro.env).to receive(:webauthn_enabled) { bool } + Rails.application.reload_routes! + end end From bc1e4397500337d82de7eba7916b923bd954a05e Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Thu, 13 Sep 2018 23:06:39 -0400 Subject: [PATCH 3/4] Lint --- .../two_factor_authentication/options_controller_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/controllers/two_factor_authentication/options_controller_spec.rb b/spec/controllers/two_factor_authentication/options_controller_spec.rb index 82f70e9ea3d..a64778e0873 100644 --- a/spec/controllers/two_factor_authentication/options_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/options_controller_spec.rb @@ -25,14 +25,15 @@ before { sign_in_before_2fa } it 'redirects to login_two_factor_url if sms and piv/cac and webauthn disabled' do - set_piv_cac_webauthn_enabled('false') + piv_cac_webauthn_enabled('false') post :create, params: { two_factor_options_form: { selection: 'sms' } } expect(response).to redirect_to otp_send_url( \ - otp_delivery_selection_form: { otp_delivery_preference: 'sms' }) + otp_delivery_selection_form: { otp_delivery_preference: 'sms' } + ) - set_piv_cac_webauthn_enabled('true') + piv_cac_webauthn_enabled('true') end it 'redirects to login_two_factor_url if user selects sms' do @@ -92,7 +93,7 @@ end end - def set_piv_cac_webauthn_enabled(bool) + def piv_cac_webauthn_enabled(bool) allow(Figaro.env).to receive(:piv_cac_enabled) { bool } allow(Figaro.env).to receive(:webauthn_enabled) { bool } Rails.application.reload_routes! From 117cce402f0e7ad1a4a8ec2b29e282d878683dd1 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Thu, 13 Sep 2018 23:10:16 -0400 Subject: [PATCH 4/4] Changed description --- .../two_factor_authentication/options_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/two_factor_authentication/options_controller_spec.rb b/spec/controllers/two_factor_authentication/options_controller_spec.rb index a64778e0873..2af1a56d2f1 100644 --- a/spec/controllers/two_factor_authentication/options_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/options_controller_spec.rb @@ -24,7 +24,7 @@ describe '#create' do before { sign_in_before_2fa } - it 'redirects to login_two_factor_url if sms and piv/cac and webauthn disabled' do + it 'redirects to login_two_factor_url for sms with piv/cac and webauthn disabled' do piv_cac_webauthn_enabled('false') post :create, params: { two_factor_options_form: { selection: 'sms' } }