From c86105316fd785dfc950b2e64b8f7967e5f63779 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 11 Mar 2022 16:15:49 -0600 Subject: [PATCH 1/3] Create common interface for accessing PII bundle in session changelog: Internal, Data, Create common interface for accessing PII bundle in session --- app/controllers/application_controller.rb | 2 +- app/controllers/idv/gpo_controller.rb | 4 +++- app/controllers/idv/sessions_controller.rb | 2 +- .../openid_connect/authorization_controller.rb | 2 +- app/controllers/saml_idp_controller.rb | 3 ++- app/controllers/sign_up/completions_controller.rb | 3 ++- app/controllers/users/passwords_controller.rb | 3 ++- app/services/pii/cacher.rb | 15 ++++++++++++--- app/services/pii/session_store.rb | 3 ++- spec/controllers/saml_idp_controller_spec.rb | 3 ++- .../users/passwords_controller_spec.rb | 2 +- 11 files changed, 29 insertions(+), 13 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6acfa0e0822..212e97d297a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -193,7 +193,7 @@ def fix_broken_personal_key_url flash[:info] = t('account.personal_key.needs_new') - pii_unlocked = user_session[:decrypted_pii].present? + pii_unlocked = Pii::Cacher.new(current_user, user_session).fetch.present? if pii_unlocked cacher = Pii::Cacher.new(current_user, user_session) diff --git a/app/controllers/idv/gpo_controller.rb b/app/controllers/idv/gpo_controller.rb index 08a8edac004..71e603ab724 100644 --- a/app/controllers/idv/gpo_controller.rb +++ b/app/controllers/idv/gpo_controller.rb @@ -74,7 +74,9 @@ def non_address_pii end def pii_to_h - JSON.parse(user_session[:decrypted_pii]) + JSON.parse( + Pii::Cacher.new(current_user, user_session).fetch_string, + ) end def resolution_success(hash) diff --git a/app/controllers/idv/sessions_controller.rb b/app/controllers/idv/sessions_controller.rb index 421148c233f..206b7a92f51 100644 --- a/app/controllers/idv/sessions_controller.rb +++ b/app/controllers/idv/sessions_controller.rb @@ -9,7 +9,7 @@ def destroy analytics.track_event(Analytics::IDV_START_OVER, **location_params) user_session['idv/doc_auth'] = {} idv_session.clear - user_session.delete(:decrypted_pii) + Pii::Cacher.new(current_user, user_session).delete redirect_to idv_url end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index a7e182d3e2b..61658714e35 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -137,7 +137,7 @@ def store_request def pii_requested_but_locked? sp_session && sp_session_ial > 1 && UserDecorator.new(current_user).identity_verified? && - user_session[:decrypted_pii].blank? + Pii::Cacher.new(current_user, user_session).fetch.blank? end def track_events diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 1f945e261b9..4c05994d723 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -95,7 +95,8 @@ def profile_or_identity_needs_verification_or_decryption? end def identity_needs_decryption? - UserDecorator.new(current_user).identity_verified? && user_session[:decrypted_pii].blank? + UserDecorator.new(current_user).identity_verified? && + Pii::Cacher.new(current_user, user_session).fetch.blank? end def capture_analytics diff --git a/app/controllers/sign_up/completions_controller.rb b/app/controllers/sign_up/completions_controller.rb index 6b55f05b9e1..4bd9975d519 100644 --- a/app/controllers/sign_up/completions_controller.rb +++ b/app/controllers/sign_up/completions_controller.rb @@ -81,7 +81,8 @@ def track_completion_event(last_page) end def pii - JSON.parse(user_session.fetch('decrypted_pii', '{}')).symbolize_keys + pii_string = Pii::Cacher.new(current_user, user_session).fetch_string + JSON.parse(pii_string || '{}').symbolize_keys end end end diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 1458e146f87..a094d1d7b45 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -27,7 +27,8 @@ def update private def capture_password_if_pii_requested_but_locked - return unless current_user.decorate.identity_verified? && user_session[:decrypted_pii].blank? + return unless current_user.decorate.identity_verified? && + Pii::Cacher.new(current_user, user_session).fetch.blank? user_session[:stored_location] = request.url redirect_to capture_password_url end diff --git a/app/services/pii/cacher.rb b/app/services/pii/cacher.rb index a96b681b6e2..afb9858fdbe 100644 --- a/app/services/pii/cacher.rb +++ b/app/services/pii/cacher.rb @@ -19,9 +19,18 @@ def save(user_password, profile = user.active_profile) end def fetch - decrypted_pii = user_session[:decrypted_pii] - return unless decrypted_pii - Pii::Attributes.new_from_json(decrypted_pii) + pii_string = fetch_string + return nil unless pii_string + + Pii::Attributes.new_from_json(pii_string) + end + + def fetch_string + user_session[:decrypted_pii] + end + + def delete + user_session.delete(:decrypted_pii) end private diff --git a/app/services/pii/session_store.rb b/app/services/pii/session_store.rb index 8c11883c2cd..f585395e428 100644 --- a/app/services/pii/session_store.rb +++ b/app/services/pii/session_store.rb @@ -15,7 +15,8 @@ def initialize(session_uuid) def load session = session_accessor.load - Pii::Attributes.new_from_json(session.dig('warden.user.user.session', :decrypted_pii)) + + Pii::Cacher.new(nil, session.dig('warden.user.user.session')).fetch end # @api private diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 89e97729ada..acda9d56987 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -452,6 +452,7 @@ def name_id_version(format_urn) zipcode: '12345', ) end + let(:pii_json) { pii.present? ? pii.to_json : nil } let(:this_authn_request) do ial2_authnrequest = saml_authn_request_url( overrides: { @@ -481,7 +482,7 @@ def name_id_version(format_urn) ) allow(subject).to receive(:attribute_asserter) { asserter } - controller.user_session[:decrypted_pii] = pii + controller.user_session[:decrypted_pii] = pii_json end it 'calls AttributeAsserter#build' do diff --git a/spec/controllers/users/passwords_controller_spec.rb b/spec/controllers/users/passwords_controller_spec.rb index 3547323fee6..d3be5af633d 100644 --- a/spec/controllers/users/passwords_controller_spec.rb +++ b/spec/controllers/users/passwords_controller_spec.rb @@ -112,7 +112,7 @@ end it 'renders form if PII is decrypted' do - controller.user_session[:decrypted_pii] = pii + controller.user_session[:decrypted_pii] = pii.to_json get :edit From 232c2c9e0883bb06f0c46e401116eec93eecf637 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 14 Mar 2022 08:54:10 -0500 Subject: [PATCH 2/3] Update app/controllers/sign_up/completions_controller.rb Co-authored-by: Zach Margolis --- app/controllers/sign_up/completions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/sign_up/completions_controller.rb b/app/controllers/sign_up/completions_controller.rb index 4bd9975d519..74172cf2e49 100644 --- a/app/controllers/sign_up/completions_controller.rb +++ b/app/controllers/sign_up/completions_controller.rb @@ -82,7 +82,7 @@ def track_completion_event(last_page) def pii pii_string = Pii::Cacher.new(current_user, user_session).fetch_string - JSON.parse(pii_string || '{}').symbolize_keys + JSON.parse(pii_string || '{}', symbolize_names: true) end end end From ba75087d88e09f8e5b4c14b08e7dcc8f5d40ab32 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 14 Mar 2022 09:05:23 -0500 Subject: [PATCH 3/3] Add method to allow for checking of existence of PII bundle in session --- app/controllers/application_controller.rb | 2 +- app/controllers/openid_connect/authorization_controller.rb | 2 +- app/controllers/saml_idp_controller.rb | 2 +- app/controllers/users/passwords_controller.rb | 2 +- app/services/pii/cacher.rb | 4 ++++ 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 212e97d297a..cee9beb2e6e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -193,7 +193,7 @@ def fix_broken_personal_key_url flash[:info] = t('account.personal_key.needs_new') - pii_unlocked = Pii::Cacher.new(current_user, user_session).fetch.present? + pii_unlocked = Pii::Cacher.new(current_user, user_session).exists_in_session? if pii_unlocked cacher = Pii::Cacher.new(current_user, user_session) diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 61658714e35..02a796d2dc9 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -137,7 +137,7 @@ def store_request def pii_requested_but_locked? sp_session && sp_session_ial > 1 && UserDecorator.new(current_user).identity_verified? && - Pii::Cacher.new(current_user, user_session).fetch.blank? + !Pii::Cacher.new(current_user, user_session).exists_in_session? end def track_events diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 4c05994d723..b1d24e5bddd 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -96,7 +96,7 @@ def profile_or_identity_needs_verification_or_decryption? def identity_needs_decryption? UserDecorator.new(current_user).identity_verified? && - Pii::Cacher.new(current_user, user_session).fetch.blank? + !Pii::Cacher.new(current_user, user_session).exists_in_session? end def capture_analytics diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index a094d1d7b45..a6116b0698f 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -28,7 +28,7 @@ def update def capture_password_if_pii_requested_but_locked return unless current_user.decorate.identity_verified? && - Pii::Cacher.new(current_user, user_session).fetch.blank? + !Pii::Cacher.new(current_user, user_session).exists_in_session? user_session[:stored_location] = request.url redirect_to capture_password_url end diff --git a/app/services/pii/cacher.rb b/app/services/pii/cacher.rb index afb9858fdbe..14217e84092 100644 --- a/app/services/pii/cacher.rb +++ b/app/services/pii/cacher.rb @@ -29,6 +29,10 @@ def fetch_string user_session[:decrypted_pii] end + def exists_in_session? + fetch_string.present? + end + def delete user_session.delete(:decrypted_pii) end