From ccc2b1873148c0f15b02f74017455764a5c40000 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 12 Feb 2024 14:20:48 -0500 Subject: [PATCH 01/19] Jmax/lg 12260 use authn context resolver to compute legacy sp session values (#10030) Updated StoreSpMetadataInSession to use the new VoT parser (which included backwards support for ACR values). changelog: Upcoming Features,VOT,Handle vector of trust in SP requestt --- .../service_provider_request_proxy.rb | 3 +- app/services/store_sp_metadata_in_session.rb | 80 ++- app/services/vot/legacy_component_values.rb | 2 +- app/services/vot/parser.rb | 5 + config/application.yml.default | 1 + .../authorization_controller_spec.rb | 32 +- spec/controllers/saml_idp_controller_spec.rb | 2 +- .../email_confirmations_controller_spec.rb | 7 +- .../openid_connect_authorize_form_spec.rb | 12 + .../store_sp_metadata_in_session_spec.rb | 530 +++++++++++------- spec/services/vot/parser_spec.rb | 7 + 11 files changed, 453 insertions(+), 228 deletions(-) diff --git a/app/services/service_provider_request_proxy.rb b/app/services/service_provider_request_proxy.rb index f99c21b5947..a7a8f983366 100644 --- a/app/services/service_provider_request_proxy.rb +++ b/app/services/service_provider_request_proxy.rb @@ -36,6 +36,7 @@ def self.find_or_create_by(uuid:) aal: nil, requested_attributes: nil, biometric_comparison_required: false, acr_values: nil, vtr: nil ) + yield(spr) create( uuid: uuid, @@ -59,8 +60,8 @@ def self.create(hash) :aal, :requested_attributes, :biometric_comparison_required, - :acr_values, :vtr, + :acr_values, ) write(obj, uuid) hash_to_spr(obj, uuid) diff --git a/app/services/store_sp_metadata_in_session.rb b/app/services/store_sp_metadata_in_session.rb index c3c0577504e..e42746c95b5 100644 --- a/app/services/store_sp_metadata_in_session.rb +++ b/app/services/store_sp_metadata_in_session.rb @@ -16,6 +16,16 @@ def call(service_provider_request: nil, requested_service_provider: nil) attr_reader :session, :request_id + def parsed_vot + return nil if !sp_request.vtr && !sp_request.acr_values + + @parsed_vot ||= AuthnContextResolver.new( + service_provider: service_provider, + vtr: sp_request.vtr, + acr_values: sp_request.acr_values, + ).resolve + end + def ial_context @ial_context ||= IalContext.new(ial: sp_request.ial, service_provider: service_provider) end @@ -24,38 +34,66 @@ def sp_request @sp_request ||= ServiceProviderRequestProxy.from_uuid(request_id) end + def ial_value + return nil unless parsed_vot + + if parsed_vot&.ialmax? + 0 + elsif parsed_vot&.identity_proofing? + 2 + elsif parsed_vot + 1 + end + end + + def ial2_value + parsed_vot&.identity_proofing? + end + + def ialmax_value + parsed_vot&.ialmax? + end + + def aal_level_requested_value + return nil unless parsed_vot + + if parsed_vot.aal2? + 2 + else + 1 + end + end + + def piv_cac_requested_value + parsed_vot&.hspd12? + end + + def phishing_resistant_value + parsed_vot&.phishing_resistant? + end + + def biometric_comparison_required_value + parsed_vot&.biometric_comparison? || sp_request&.biometric_comparison_required + end + def update_session session[:sp] = { issuer: sp_request.issuer, - ial: ial_context.ial, - ial2: ial_context.ial2_requested?, - ialmax: ial_context.ialmax_requested?, - aal_level_requested: aal_requested, - piv_cac_requested: hspd12_requested, - phishing_resistant_requested: phishing_resistant_requested, request_url: sp_request.url, request_id: sp_request.uuid, requested_attributes: sp_request.requested_attributes, - biometric_comparison_required: sp_request.biometric_comparison_required, + ial: ial_value, + ial2: ial2_value, + ialmax: ialmax_value, + aal_level_requested: aal_level_requested_value, + piv_cac_requested: piv_cac_requested_value, + phishing_resistant_requested: phishing_resistant_value, + biometric_comparison_required: biometric_comparison_required_value, acr_values: sp_request.acr_values, vtr: sp_request.vtr, } end - def aal_requested - Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[sp_request.aal] - end - - def phishing_resistant_requested - sp_request.aal == Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF || - sp_request.aal == Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF - end - - def hspd12_requested - sp_request.aal == Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF || - sp_request.aal == Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF - end - def service_provider return @service_provider if defined?(@service_provider) @service_provider = ServiceProvider.find_by(issuer: sp_request.issuer) diff --git a/app/services/vot/legacy_component_values.rb b/app/services/vot/legacy_component_values.rb index 034edb6d40b..785c70a7bf3 100644 --- a/app/services/vot/legacy_component_values.rb +++ b/app/services/vot/legacy_component_values.rb @@ -67,7 +67,7 @@ module LegacyComponentValues name: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, description: 'Legacy AAL3', implied_component_values: [], - requirements: [:aal2, :hspd12], + requirements: [:aal2, :phishing_resistant], ) AAL3_HSPD12 = ComponentValue.new( name: Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, diff --git a/app/services/vot/parser.rb b/app/services/vot/parser.rb index 3fa71a67a1a..1ac2ed9e8b0 100644 --- a/app/services/vot/parser.rb +++ b/app/services/vot/parser.rb @@ -25,6 +25,11 @@ def parse elsif acr_values.present? map_initial_acr_values_to_component_values end + + if !initial_components + raise ParseException.new('VoT parser called without VoT or ACR values') + end + expand_components_with_initial_components(initial_components) end diff --git a/config/application.yml.default b/config/application.yml.default index fa0507797c2..9b82d3281e7 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -342,6 +342,7 @@ verify_personal_key_max_attempts: 5 version_headers_enabled: false use_dashboard_service_providers: false use_kms: false +use_vot_in_sp_requests: false usps_auth_token_refresh_job_enabled: false usps_confirmation_max_days: 30 usps_ipp_password: '' diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index adaeb2a1ebf..9f91bf0f566 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -1049,20 +1049,22 @@ it 'redirects to SP landing page with the request_id in the params' do stub_analytics expect(@analytics).to receive(:track_event). - with('OpenID Connect: authorization request', - success: true, - client_id: client_id, - prompt: 'select_account', - referer: nil, - allow_prompt_login: true, - errors: {}, - unauthorized_scope: true, - user_fully_authenticated: false, - acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', - code_challenge_present: false, - service_provider_pkce: nil, - scope: 'openid', - vtr: nil) + with( + 'OpenID Connect: authorization request', + success: true, + client_id: client_id, + prompt: 'select_account', + referer: nil, + allow_prompt_login: true, + errors: {}, + unauthorized_scope: true, + user_fully_authenticated: false, + acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', + code_challenge_present: false, + service_provider_pkce: nil, + scope: 'openid', + vtr: nil, + ) action sp_request_id = ServiceProviderRequestProxy.last.uuid @@ -1076,7 +1078,7 @@ sp_request_id = ServiceProviderRequestProxy.last.uuid expect(session[:sp]).to eq( - aal_level_requested: nil, + aal_level_requested: 1, acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, piv_cac_requested: false, phishing_resistant_requested: false, diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index eb92b202d3d..e1572e88723 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -522,7 +522,7 @@ def name_id_version(format_urn) end let(:xmldoc) { SamlResponseDoc.new('controller', 'response_assertion', response) } - let(:aal_level) { 0 } + let(:aal_level) { 1 } let(:ial2_settings) do saml_settings( overrides: { diff --git a/spec/controllers/sign_up/email_confirmations_controller_spec.rb b/spec/controllers/sign_up/email_confirmations_controller_spec.rb index f3dddcaea73..38206991c33 100644 --- a/spec/controllers/sign_up/email_confirmations_controller_spec.rb +++ b/spec/controllers/sign_up/email_confirmations_controller_spec.rb @@ -174,9 +174,14 @@ url: '', uuid: sp_request_uuid, ial: '1', + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, ) create(:email_address, :unconfirmed, confirmation_token:, user: build(:user, email: nil)) - get :create, params: { confirmation_token:, _request_id: request_id_param } + get :create, params: { + confirmation_token:, + _request_id: request_id_param, + acr_values: Vot::LegacyComponentValues::IAL1, + } end context 'with invalid request id' do diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index a019462b577..3b8128759e0 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -33,6 +33,10 @@ let(:verified_within) { nil } let(:biometric_comparison_required) { nil } + before do + allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).and_return(true) + end + describe '#submit' do subject(:result) { form.submit } @@ -145,6 +149,7 @@ context 'with an invalid vtr' do let(:vtr) { ['A1.B2.C3'].to_json } + it 'has errors' do expect(valid?).to eq(false) expect(form.errors[:vtr]). @@ -165,6 +170,7 @@ context 'with no authorized vtr components' do let(:vtr) { ['C1.P1'].to_json } let(:client_id) { 'urn:gov:gsa:openidconnect:test:loa1' } + it 'has errors' do expect(valid?).to eq(false) expect(form.errors[:acr_values]). @@ -245,6 +251,7 @@ context 'when prompt is not given' do let(:prompt) { nil } + it { expect(valid?).to eq(true) } end @@ -282,6 +289,7 @@ context 'when scope is unauthorized and we block unauthorized scopes' do let(:scope) { 'email profile' } + it 'has errors' do allow(IdentityConfig.store).to receive(:unauthorized_scope_enabled).and_return(true) expect(valid?).to eq(false) @@ -292,6 +300,7 @@ context 'when scope is good and we block unauthorized scopes' do let(:scope) { 'email' } + it 'does not have errors' do allow(IdentityConfig.store).to receive(:unauthorized_scope_enabled).and_return(false) expect(valid?).to eq(true) @@ -300,6 +309,7 @@ context 'when scope is unauthorized and we do not block unauthorized scopes' do let(:scope) { 'email profile' } + it 'does not have errors' do allow(IdentityConfig.store).to receive(:unauthorized_scope_enabled).and_return(false) expect(valid?).to eq(true) @@ -660,6 +670,8 @@ end describe '#verified_within' do + let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + context 'the issuer is allowed to use verified_within' do before do allow(IdentityConfig.store).to receive(:allowed_verified_within_providers). diff --git a/spec/services/store_sp_metadata_in_session_spec.rb b/spec/services/store_sp_metadata_in_session_spec.rb index d1486bb4258..4ecbf0e194f 100644 --- a/spec/services/store_sp_metadata_in_session_spec.rb +++ b/spec/services/store_sp_metadata_in_session_spec.rb @@ -2,209 +2,363 @@ RSpec.describe StoreSpMetadataInSession do describe '#call' do + let(:request_id) { SecureRandom.uuid } + let(:app_session) { {} } + let(:instance) do + StoreSpMetadataInSession.new(session: app_session, request_id: request_id) + end + context 'when a ServiceProviderRequestProxy is not found' do - it 'does not set the session[:sp] hash' do - app_session = {} - instance = StoreSpMetadataInSession.new(session: app_session, request_id: 'foo') + let(:request_id) { 'foo' } + it 'does not set the session[:sp] hash' do expect { instance.call }.to_not change(app_session, :keys) end end - context 'when a ServiceProviderRequestProxy is found' do - it 'sets the session[:sp] hash' do - app_session = {} - request_id = SecureRandom.uuid + context 'when a ServiceProviderRequest is found' do + let(:issuer) { 'issuer' } + let(:request_url) { 'http://issuer.gov' } + let(:requested_attributes) { %w[email] } + let(:request_acr) { nil } + let(:request_vtr) { nil } + let(:biometric_comparison_required) { false } + let(:sp_request) do ServiceProviderRequestProxy.find_or_create_by(uuid: request_id) do |sp_request| - sp_request.issuer = 'issuer' - sp_request.ial = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF - sp_request.acr_values = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF - sp_request.url = 'http://issuer.gov' - sp_request.requested_attributes = %w[email] - sp_request.biometric_comparison_required = false - end - instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id) - - app_session_hash = { - issuer: 'issuer', - aal_level_requested: nil, - acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, - piv_cac_requested: false, - phishing_resistant_requested: false, - ial: 1, - ial2: false, - ialmax: false, - request_url: 'http://issuer.gov', - request_id: request_id, - requested_attributes: %w[email], - biometric_comparison_required: false, - vtr: nil, - } - - instance.call - expect(app_session[:sp]).to eq app_session_hash + sp_request.issuer = issuer + sp_request.url = request_url + sp_request.requested_attributes = requested_attributes + sp_request.biometric_comparison_required = biometric_comparison_required + sp_request.acr_values = request_acr + sp_request.vtr = request_vtr + end end - end - context 'when IAL2 and AAL3 are requested' do - it 'sets the session[:sp] hash' do - app_session = {} - request_id = SecureRandom.uuid - ServiceProviderRequestProxy.find_or_create_by(uuid: request_id) do |sp_request| - sp_request.issuer = 'issuer' - sp_request.ial = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF - sp_request.aal = Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF - sp_request.acr_values = [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, - ].join(' ') - sp_request.url = 'http://issuer.gov' - sp_request.requested_attributes = %w[email] - sp_request.biometric_comparison_required = false - end - instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id) - - app_session_hash = { - issuer: 'issuer', - aal_level_requested: 3, - acr_values: [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, - ].join(' '), - piv_cac_requested: false, - phishing_resistant_requested: true, - ial: 2, - ial2: true, - ialmax: false, - request_url: 'http://issuer.gov', - request_id: request_id, - requested_attributes: %w[email], - biometric_comparison_required: false, - vtr: nil, - } - - instance.call - expect(app_session[:sp]).to eq app_session_hash + before do + instance.call(service_provider_request: sp_request) end - end - context 'when IAL2 and phishing-resistant are requested' do - it 'sets the session[:sp] hash' do - app_session = {} - request_id = SecureRandom.uuid - ServiceProviderRequestProxy.find_or_create_by(uuid: request_id) do |sp_request| - sp_request.issuer = 'issuer' - sp_request.ial = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF - sp_request.aal = Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF - sp_request.acr_values = [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, - ].join(' ') - sp_request.url = 'http://issuer.gov' - sp_request.requested_attributes = %w[email] - sp_request.biometric_comparison_required = false - end - instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id) - - app_session_hash = { - issuer: 'issuer', - aal_level_requested: 2, - acr_values: [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, - ].join(' '), - piv_cac_requested: false, - phishing_resistant_requested: true, - ial: 2, - ial2: true, - ialmax: false, - request_url: 'http://issuer.gov', - request_id: request_id, - requested_attributes: %w[email], - biometric_comparison_required: false, - vtr: nil, - } - - instance.call - expect(app_session[:sp]).to eq app_session_hash + context 'IAL1 is requested with ACRs' do + let(:request_acr) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 1, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 1, + ial2: false, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end end - end - context 'when biometric comparison is requested' do - it 'sets the session[:sp] hash' do - app_session = {} - request_id = SecureRandom.uuid - ServiceProviderRequestProxy.find_or_create_by(uuid: request_id) do |sp_request| - sp_request.issuer = 'issuer' - sp_request.ial = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF - sp_request.aal = Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF - sp_request.acr_values = [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, - ].join(' ') - sp_request.url = 'http://issuer.gov' - sp_request.requested_attributes = %w[email] - sp_request.biometric_comparison_required = true - end - instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id) - - app_session_hash = { - issuer: 'issuer', - aal_level_requested: 3, - acr_values: [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, - ].join(' '), - piv_cac_requested: false, - phishing_resistant_requested: true, - ial: 2, - ial2: true, - ialmax: false, - request_url: 'http://issuer.gov', - request_id: request_id, - requested_attributes: %w[email], - biometric_comparison_required: true, - vtr: nil, - } - - instance.call - expect(app_session[:sp]).to eq app_session_hash + context 'when IAL2 and AAL3 are requested with ACRs' do + let(:request_acr) do + [Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF].join(' ') + end + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: true, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end end - end + context 'when IAL2 and phishing-resistant are requested with ACRs' do + let(:request_acr) do + [Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF].join(' ') + end - context 'when a vtr is present' do - it 'sets the session[:sp] hash' do - app_session = {} - request_id = SecureRandom.uuid - ServiceProviderRequestProxy.find_or_create_by(uuid: request_id) do |sp_request| - sp_request.issuer = 'issuer' - sp_request.ial = nil - sp_request.aal = nil - sp_request.vtr = ['C2.P1'] - sp_request.url = 'http://issuer.gov' - sp_request.requested_attributes = %w[email] - sp_request.biometric_comparison_required = false - end - instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id) - - app_session_hash = { - issuer: 'issuer', - aal_level_requested: nil, - acr_values: nil, - piv_cac_requested: false, - phishing_resistant_requested: false, - ial: nil, - ial2: false, - ialmax: nil, - request_url: 'http://issuer.gov', - request_id: request_id, - requested_attributes: %w[email], - biometric_comparison_required: false, - vtr: ['C2.P1'], - } - - instance.call - expect(app_session[:sp]).to eq app_session_hash + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: true, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end + end + + context 'when biometric comparison is requested with ACRs' do + let(:request_acr) do + [Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF].join(' ') + end + let(:biometric_comparison_required) { true } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: true, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end + end + + context 'when MFA is requested using a VTR' do + let(:request_vtr) { ['C1'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 1, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 1, + ial2: false, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end + end + + context 'when MFA and biometric comparison are requested using a VTR' do + context 'using VTR' do + let(:request_vtr) { ['C1.Pb'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end + end + end + + context 'when AAL2 and proofing are requested using a VTR' do + let(:request_vtr) { ['C2.P1'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end + end + + context 'when AAL2 and biometric comparison are requested using a VTR' do + let(:request_vtr) { ['C2.Pb'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end + end + + context 'when phishing resistant ID and proofing are requested using a VTR' do + let(:request_vtr) { ['Ca.P1'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: true, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end + end + + context 'when phishing resistant and ID biometric comparison are requested using a VTR' do + let(:request_vtr) { ['Ca.Pb'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + acr_values: request_acr, + aal_level_requested: 2, + piv_cac_requested: false, + phishing_resistant_requested: true, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end + end + + context 'when PIV/CAC and proofing are requested using a VTR' do + let(:request_vtr) { ['Cb.P1'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: true, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: false, + vtr: request_vtr, + }, + ) + end + end + + context 'when PIV/CAC and biometric comparison are requested using a VTR' do + let(:request_vtr) { ['Cb.Pb'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: true, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end + end + + context 'IAL2 proofing requested with no authentication requirement using a VTR' do + let(:request_vtr) { ['Pb'] } + + it 'sets the session[:sp] hash correctly' do + expect(app_session[:sp]).to eq( + { + issuer: issuer, + aal_level_requested: 2, + acr_values: request_acr, + piv_cac_requested: false, + phishing_resistant_requested: false, + ial: 2, + ial2: true, + ialmax: false, + request_url: request_url, + request_id: request_id, + requested_attributes: requested_attributes, + biometric_comparison_required: true, + vtr: request_vtr, + }, + ) + end end end end diff --git a/spec/services/vot/parser_spec.rb b/spec/services/vot/parser_spec.rb index 662acde156b..40af3ce6986 100644 --- a/spec/services/vot/parser_spec.rb +++ b/spec/services/vot/parser_spec.rb @@ -2,6 +2,13 @@ RSpec.describe Vot::Parser do describe '#parse' do + context 'when neither a VtR nor ACR values are provided' do + it 'raises an error' do + expect { Vot::Parser.new(vector_of_trust: nil, acr_values: nil).parse }. + to raise_error(Vot::Parser::ParseException, 'VoT parser called without VoT or ACR values') + end + end + context 'when a vector is completely expanded' do it 'returns the vector along with requirements' do vector_of_trust = 'C1.C2.Cb' From 6cadf9896c8a1f5956dd3d1c045b0d212d912ea5 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Mon, 12 Feb 2024 14:46:41 -0500 Subject: [PATCH 02/19] LG-12355: store reference in doc auth response extras for failed LN http requests (#10061) * store reference in doc auth response extras for failed LN http requests * changelog: Internal, Document Authentication, LexisNexis invalid responses to log reference value sent in request --- app/services/doc_auth/lexis_nexis/request.rb | 4 +++- spec/services/doc_auth/lexis_nexis/lexis_nexis_client_spec.rb | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/services/doc_auth/lexis_nexis/request.rb b/app/services/doc_auth/lexis_nexis/request.rb index 422385329ed..68ca892ec57 100644 --- a/app/services/doc_auth/lexis_nexis/request.rb +++ b/app/services/doc_auth/lexis_nexis/request.rb @@ -70,6 +70,7 @@ def handle_connection_error(exception:, status_code: nil, status_message: nil) selfie_quality_good: false, vendor_status_code: status_code, vendor_status_message: status_message, + reference: @reference, }.compact, ) end @@ -146,13 +147,14 @@ def hmac_authorization end def settings + @reference = uuid { Type: 'Initiate', Settings: { Mode: request_mode, Locale: config.locale, Venue: 'online', - Reference: uuid, + Reference: @reference, }, } end diff --git a/spec/services/doc_auth/lexis_nexis/lexis_nexis_client_spec.rb b/spec/services/doc_auth/lexis_nexis/lexis_nexis_client_spec.rb index 0f89ba4e0a7..52d13e68842 100644 --- a/spec/services/doc_auth/lexis_nexis/lexis_nexis_client_spec.rb +++ b/spec/services/doc_auth/lexis_nexis/lexis_nexis_client_spec.rb @@ -168,6 +168,7 @@ expect(result.class).to eq(DocAuth::LexisNexis::Responses::TrueIdResponse) expect(result.doc_auth_success?).to eq(false) result_hash = result.to_h + expect(result_hash[:reference]).not_to be_nil expect(result_hash[:selfie_status]).to eq(:fail) expect(result.selfie_live?).to eq(true) expect(result.selfie_quality_good?).to eq(false) @@ -206,6 +207,7 @@ result_hash = result.to_h expect(result_hash[:vendor]).to eq('TrueID') expect(result_hash[:doc_auth_success]).to eq(false) + expect(result_hash[:reference]).not_to be_nil expect(result_hash[:selfie_status]).to eq(:not_processed) expect(result_hash[:vendor_status_code]).to eq(status_code) expect(result_hash[:vendor_status_message]).to eq(status_message) @@ -236,6 +238,7 @@ result_hash = result.to_h expect(result_hash[:vendor]).to eq('TrueID') expect(result_hash[:doc_auth_success]).to eq(false) + expect(result_hash[:reference]).not_to be_nil expect(result_hash[:selfie_status]).to eq(:not_processed) expect(result_hash[:vendor_status_code]).to be_nil expect(result_hash[:vendor_status_message]).to be_nil From eb992e2125dd3857f0e18dcfe55fb069a1051d20 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 12 Feb 2024 16:06:39 -0500 Subject: [PATCH 03/19] Jmax/LG-12134 Spike use absolute timestamps for rate limiter Redis timeouts. (#10009) * Use absolute Redis timestamps instead of offsets. Several rate limiter tests were slightly flaky due to timestamp bobbles between Redis's clock and Ruby's. This change sets all rate limit expirations using the Ruby clock, rather than using offsets from the current (Redis) time. changelog: Internal,Rate Limiting,Use absolute timestamps in Redis for improved consistency. --- app/services/rate_limiter.rb | 7 ++++--- .../idv/steps/request_letter_step_spec.rb | 6 ++++++ spec/services/rate_limiter_spec.rb | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/app/services/rate_limiter.rb b/app/services/rate_limiter.rb index f278b178fe3..67f7f06f7f1 100644 --- a/app/services/rate_limiter.rb +++ b/app/services/rate_limiter.rb @@ -71,18 +71,19 @@ def increment! return if limited? value = nil + now = Time.zone.now REDIS_THROTTLE_POOL.with do |client| value, _success = client.multi do |multi| multi.incr(key) - multi.expire( + multi.expireat( key, - RateLimiter.attempt_window_in_minutes(rate_limit_type).minutes.seconds.to_i, + now + RateLimiter.attempt_window_in_minutes(rate_limit_type).minutes.seconds.to_i, ) end end @redis_attempts = value.to_i - @redis_attempted_at = Time.zone.now + @redis_attempted_at = now attempts end diff --git a/spec/features/idv/steps/request_letter_step_spec.rb b/spec/features/idv/steps/request_letter_step_spec.rb index 17c03d15e20..3279bfbab07 100644 --- a/spec/features/idv/steps/request_letter_step_spec.rb +++ b/spec/features/idv/steps/request_letter_step_spec.rb @@ -32,6 +32,12 @@ context 'the user has sent a letter but not verified an OTP' do let(:user) { user_with_2fa } + before do + # Without this, the check for GPO expiration leaves an expired + # OTP rate limiter laying around. + allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(3) + end + it 'if not rate limited, allow user to resend letter & redirect to letter enqueued step', :js do complete_idv_by_mail_and_sign_out diff --git a/spec/services/rate_limiter_spec.rb b/spec/services/rate_limiter_spec.rb index dae6a7297f9..22a2e920922 100644 --- a/spec/services/rate_limiter_spec.rb +++ b/spec/services/rate_limiter_spec.rb @@ -145,6 +145,21 @@ of(rate_limiter.attempted_at + attempt_window.minutes) end end + + context 'when we are out of sync with Redis' do + it 'expires at the correct time' do + fake_attempt_time = 1.year.ago + travel_to(fake_attempt_time) do + # Redis should immediately delete the rate limiter because + # we're supplying an expiration time which has long since + # passed. + rate_limiter.increment! + end + + new_rate_limiter = RateLimiter.new(target: '1', rate_limit_type: rate_limit_type) + expect(new_rate_limiter.expires_at).to be_nil + end + end end context 'with active rate_limiter' do From 61b9fc19a68cb819088d8fbb2b135336e4f58d6c Mon Sep 17 00:00:00 2001 From: Shannon A <20867088+svalexander@users.noreply.github.com> Date: Mon, 12 Feb 2024 16:23:37 -0500 Subject: [PATCH 04/19] LG-12017 fraud pending deadline passed (#10053) * spec verifies ipp profile is rejected after 30 days in review * changelog: Internal, Threatmetrix, in person spec verifies profile rejected * update spec --- spec/features/idv/in_person_spec.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/features/idv/in_person_spec.rb b/spec/features/idv/in_person_spec.rb index 9b167c6ed4c..57681e3ec7d 100644 --- a/spec/features/idv/in_person_spec.rb +++ b/spec/features/idv/in_person_spec.rb @@ -109,6 +109,34 @@ visit_idp_from_sp_with_ial2(:oidc) expect(page).to have_current_path(idv_in_person_ready_to_verify_path) end + + it 'handles profiles in fraud review after 30 days', allow_browser_log: true do + sign_in_and_2fa_user(user) + begin_in_person_proofing(user) + complete_prepare_step(user) + complete_location_step + complete_state_id_step(user) + + # ssn page + select 'Review', from: :mock_profiling_result + complete_ssn_step(user) + complete_verify_step(user) + complete_phone_step(user) + complete_enter_password_step(user) + acknowledge_and_confirm_personal_key + + profile = InPersonEnrollment.last.profile + profile.deactivate_for_fraud_review + expect(profile.fraud_review_pending_at).to be_truthy + expect(profile.fraud_rejection_at).to eq(nil) + profile.update(fraud_review_pending_at: 31.days.ago) + FraudRejectionDailyJob.new.perform(Time.zone.now) + + # profile is rejected + profile.reload + expect(profile.fraud_review_pending_at).to be(nil) + expect(profile.fraud_rejection_at).to be_truthy + end end it 'works for a happy path', allow_browser_log: true do From f622ba2f3a7906ab9f026ed5fcdc4b910d31d3aa Mon Sep 17 00:00:00 2001 From: Brittany Greaner Date: Mon, 12 Feb 2024 13:45:36 -0800 Subject: [PATCH 05/19] Convert file-base64-cache.js to typescript (#10070) [skip changelog] Co-authored-by: Amir Reavis-Bey Co-authored-by: Andrew Duthie <1779930+aduth@users.noreply.github.com> --- .../context/{file-base64-cache.js => file-base64-cache.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename app/javascript/packages/document-capture/context/{file-base64-cache.js => file-base64-cache.ts} (59%) diff --git a/app/javascript/packages/document-capture/context/file-base64-cache.js b/app/javascript/packages/document-capture/context/file-base64-cache.ts similarity index 59% rename from app/javascript/packages/document-capture/context/file-base64-cache.js rename to app/javascript/packages/document-capture/context/file-base64-cache.ts index 8ad95a50d03..366b141a6bf 100644 --- a/app/javascript/packages/document-capture/context/file-base64-cache.js +++ b/app/javascript/packages/document-capture/context/file-base64-cache.ts @@ -1,6 +1,6 @@ import { createContext } from 'react'; -const FileBase64CacheContext = createContext(/** @type {WeakMap} */ (new WeakMap())); +const FileBase64CacheContext = createContext(new WeakMap()); FileBase64CacheContext.displayName = 'FileBase64CacheContext'; From 44e502d4770d877dd36a4d08cef08de2fab00a08 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Tue, 13 Feb 2024 09:39:26 -0500 Subject: [PATCH 06/19] LG-12370: success_status to check if pii exists (#10077) * document capture success status to check if pii exists [skip changelog] * happy linting --- .../document_capture_session_result.rb | 2 +- .../idv/doc_auth/document_capture_spec.rb | 21 +++++++++++++++++++ ...st_credential_barcode_attention_no_dob.yml | 15 +++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/ial2_test_credential_barcode_attention_no_dob.yml diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index 374a338cce4..33ae3c94e94 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -28,7 +28,7 @@ def selfie_status def success_status # doc_auth_success : including document, attention_with_barcode and id type verification - !!doc_auth_success && selfie_status != :fail + !!doc_auth_success && selfie_status != :fail && !!pii end alias_method :success?, :success_status diff --git a/spec/features/idv/doc_auth/document_capture_spec.rb b/spec/features/idv/doc_auth/document_capture_spec.rb index deccef71931..9141c0d592d 100644 --- a/spec/features/idv/doc_auth/document_capture_spec.rb +++ b/spec/features/idv/doc_auth/document_capture_spec.rb @@ -53,6 +53,27 @@ end end + context 'attention barcode with invalid pii is uploaded', allow_browser_log: true do + it 'try again and page show doc type inline error message' do + attach_images( + Rails.root.join( + 'spec', 'fixtures', + 'ial2_test_credential_barcode_attention_no_dob.yml' + ), + ) + submit_images + + expect(page).to have_content(t('doc_auth.errors.barcode_attention.heading')) + click_idv_continue + + # should show try again + expect(page).to have_current_path(idv_document_capture_path) + attach_images + submit_images + expect(page).to have_current_path(idv_ssn_path) + end + end + context 'rate limits calls to acuant', allow_browser_log: true do let(:fake_attempts_tracker) { IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new } before do diff --git a/spec/fixtures/ial2_test_credential_barcode_attention_no_dob.yml b/spec/fixtures/ial2_test_credential_barcode_attention_no_dob.yml new file mode 100644 index 00000000000..41588f87324 --- /dev/null +++ b/spec/fixtures/ial2_test_credential_barcode_attention_no_dob.yml @@ -0,0 +1,15 @@ +document: + first_name: Jane + last_name: Doe + middle_name: Q + address1: 1800 F Street + address2: Apt 3 + city: Bayside + state: NY + zipcode: '11364' + phone: +1 314-555-1212 + state_id_jurisdiction: 'NY' + state_id_number: 'S59397998' +failed_alerts: + - name: 2D Barcode Read + result: Attention \ No newline at end of file From 436969d2102a6d8bee8bd6e05c48b27f7de58fd3 Mon Sep 17 00:00:00 2001 From: dawei-nava <130466753+dawei-nava@users.noreply.github.com> Date: Tue, 13 Feb 2024 11:24:17 -0500 Subject: [PATCH 07/19] LG-12332: fix mock client to pass when using real image. (#10066) * LG-12332: fix mock client to pass when using real image. changelog: Internal, Doc Auth, Allow test to pass with selfie image. * LG-12332: update test. * LG-12332: strict checking, so it behaves like real use cases. --- .../doc_auth/mock/doc_auth_mock_client.rb | 8 ++++++-- app/services/doc_auth/mock/result_response.rb | 11 ++++++++--- .../doc_auth/mock/doc_auth_mock_client_spec.rb | 18 ++++++++++++++++++ .../doc_auth/mock/result_response_spec.rb | 13 +++++++------ 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/app/services/doc_auth/mock/doc_auth_mock_client.rb b/app/services/doc_auth/mock/doc_auth_mock_client.rb index 82427f1bd49..739a6617fe0 100644 --- a/app/services/doc_auth/mock/doc_auth_mock_client.rb +++ b/app/services/doc_auth/mock/doc_auth_mock_client.rb @@ -75,10 +75,13 @@ def post_images( back_image_response = post_back_image(image: back_image, instance_id: instance_id) return back_image_response unless back_image_response.success? - get_results(instance_id: instance_id) + get_results( + instance_id: instance_id, + selfie_required: liveness_checking_required, + ) end - def get_results(instance_id:) + def get_results(instance_id:, selfie_required: false) return mocked_response_for_method(__method__) if method_mocked?(__method__) error_response = http_error_response(self.class.last_uploaded_back_image, 'result') return error_response if error_response @@ -91,6 +94,7 @@ def get_results(instance_id:) ResultResponse.new( self.class.last_uploaded_back_image, overriden_config, + selfie_required, ) end # rubocop:enable Lint/UnusedMethodArgument diff --git a/app/services/doc_auth/mock/result_response.rb b/app/services/doc_auth/mock/result_response.rb index 4cb188025cc..2a6c12a6805 100644 --- a/app/services/doc_auth/mock/result_response.rb +++ b/app/services/doc_auth/mock/result_response.rb @@ -7,9 +7,10 @@ class ResultResponse < DocAuth::Response attr_reader :uploaded_file, :config - def initialize(uploaded_file, config) + def initialize(uploaded_file, config, selfie_required = false) @uploaded_file = uploaded_file.to_s @config = config + @selfie_required = selfie_required super( success: success?, errors: errors, @@ -138,8 +139,12 @@ def doc_auth_success? end def selfie_status - return :not_processed if portrait_match_results&.dig(:FaceMatchResult).nil? - portrait_match_results[:FaceMatchResult] == 'Pass' ? :success : :fail + if @selfie_required + return :success if portrait_match_results&.dig(:FaceMatchResult).nil? + portrait_match_results[:FaceMatchResult] == 'Pass' ? :success : :fail + else + :not_processed + end end private diff --git a/spec/services/doc_auth/mock/doc_auth_mock_client_spec.rb b/spec/services/doc_auth/mock/doc_auth_mock_client_spec.rb index 0357e5d389a..ec429286b14 100644 --- a/spec/services/doc_auth/mock/doc_auth_mock_client_spec.rb +++ b/spec/services/doc_auth/mock/doc_auth_mock_client_spec.rb @@ -193,6 +193,11 @@ failed_alerts: [] YAML + image_no_setting = <<~YAML + doc_auth_result: Passed + failed_alerts: [] + YAML + it 'sets selfie_check_performed to true' do response = client.post_images( front_image: image, @@ -204,6 +209,19 @@ expect(response.selfie_check_performed?).to be(true) expect(response.extra).to have_key(:portrait_match_results) end + + it 'returns selfie status with default setting' do + response = client.post_images( + front_image: image_no_setting, + back_image: image_no_setting, + liveness_checking_required: liveness_checking_required, + selfie_image: image_no_setting, + ) + + expect(response.selfie_check_performed?).to be(true) + expect(response.selfie_status).to eq(:success) + expect(response.extra).to_not have_key(:portrait_match_results) + end end context 'when a liveness check is not required' do diff --git a/spec/services/doc_auth/mock/result_response_spec.rb b/spec/services/doc_auth/mock/result_response_spec.rb index 10b2ca04f75..227b3c6284a 100644 --- a/spec/services/doc_auth/mock/result_response_spec.rb +++ b/spec/services/doc_auth/mock/result_response_spec.rb @@ -2,7 +2,7 @@ RSpec.describe DocAuth::Mock::ResultResponse do let(:warn_notifier) { instance_double('Proc') } - + let(:selfie_required) { false } subject(:response) do config = DocAuth::Mock::Config.new( dpi_threshold: 290, @@ -10,12 +10,12 @@ glare_threshold: 40, warn_notifier: warn_notifier, ) - described_class.new(input, config) + described_class.new(input, config, selfie_required) end context 'with an image file' do let(:input) { DocAuthImageFixtures.document_front_image } - + let(:selfie_required) { true } it 'returns a successful response with the default PII' do expect(response.success?).to eq(true) expect(response.errors).to eq({}) @@ -23,6 +23,7 @@ expect(response.pii_from_doc). to eq(Idp::Constants::MOCK_IDV_APPLICANT) expect(response.attention_with_barcode?).to eq(false) + expect(response.selfie_status).to eq(:success) end end @@ -659,7 +660,7 @@ failed_alerts: [] YAML end - let(:selfie_check_performed) { true } + let(:selfie_required) { true } it 'returns the expected values' do selfie_results = { @@ -686,7 +687,7 @@ failed_alerts: [] YAML end - let(:selfie_check_performed) { true } + let(:selfie_required) { true } it 'returns the expected values' do selfie_results = { @@ -705,7 +706,7 @@ context 'when a selfie check is not performed' do let(:input) { DocAuthImageFixtures.document_front_image } - let(:selfie_check_performed) { false } + let(:selfie_required) { false } it 'returns the expected values' do expect(response.selfie_check_performed?).to eq(false) From 1374c0f34983195499c1cd680fdbf3d3d6431750 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 13 Feb 2024 09:49:13 -0800 Subject: [PATCH 08/19] Set allow_failure: true for production_deploy (#10079) [skip changelog] --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 84abbaf1ba0..b8ed7a69236 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -532,7 +532,7 @@ stop-review-app: deploy_production: stage: deploy_production - allow_failure: false + allow_failure: true needs: - job: build-review-image resource_group: $CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov From 2502de38264ff4e73d6d18eb582bfebf01bfa8af Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 13 Feb 2024 10:06:54 -0800 Subject: [PATCH 09/19] Update OIG report format (#10054) * Update output format of logs and data for OIG requests (LG-12335) - Before: one directory with 3 files per user - After: 3 files that each contain items by UUID * remove no-longer-needed mkdir, add io flushes for easier tail -f watching * Add phone_id too per request changelog: Internal, Reporting, Update format of OIG export reports --- lib/data_pull.rb | 25 +++- .../create_mfa_configurations_report.rb | 1 + .../local/write_cloudwatch_logs.rb | 23 ++-- lib/data_requests/local/write_user_events.rb | 33 +++-- lib/data_requests/local/write_user_info.rb | 124 +++++++++++------- lib/tasks/data_requests.rake | 36 ++++- spec/fixtures/data_request.json | 1 + spec/lib/data_pull_spec.rb | 28 ++++ .../create_mfa_configurations_report_spec.rb | 1 + .../local/write_cloudwatch_logs_spec.rb | 25 ++-- .../local/write_user_events_spec.rb | 30 +++-- .../local/write_user_info_spec.rb | 76 ++++++++--- 12 files changed, 296 insertions(+), 107 deletions(-) diff --git a/lib/data_pull.rb b/lib/data_pull.rb index dd5b211f565..671efa0c8bb 100644 --- a/lib/data_pull.rb +++ b/lib/data_pull.rb @@ -152,13 +152,36 @@ def run(args:, config:) ActiveRecord::Base.connection.execute('SET statement_timeout = 0') uuids = args - users = uuids.map { |uuid| DataRequests::Deployed::LookupUserByUuid.new(uuid).call }.compact + users, missing_uuids = uuids.map do |uuid| + DataRequests::Deployed::LookupUserByUuid.new(uuid).call || uuid + end.partition { |u| u.is_a?(User) } + shared_device_users = DataRequests::Deployed::LookupSharedDeviceUsers.new(users).call output = shared_device_users.map do |user| DataRequests::Deployed::CreateUserReport.new(user, config.requesting_issuers).call end + if config.include_missing? + output += missing_uuids.map do |uuid| + { + user_id: nil, + login_uuid: nil, + requesting_issuer_uuid: uuid, + email_addresses: [], + mfa_configurations: { + phone_configurations: [], + auth_app_configurations: [], + webauthn_configurations: [], + piv_cac_configurations: [], + backup_code_configurations: [], + }, + user_events: [], + not_found: true, + } + end + end + ScriptBase::Result.new( subtask: 'ig-request', uuids: users.map(&:uuid), diff --git a/lib/data_requests/deployed/create_mfa_configurations_report.rb b/lib/data_requests/deployed/create_mfa_configurations_report.rb index 58b050f02ab..6a2f97e0b69 100644 --- a/lib/data_requests/deployed/create_mfa_configurations_report.rb +++ b/lib/data_requests/deployed/create_mfa_configurations_report.rb @@ -40,6 +40,7 @@ def backup_code_configurations_report def phone_configurations_report user.phone_configurations.map do |phone_configuration| { + id: phone_configuration.id, phone: phone_configuration.phone, created_at: phone_configuration.created_at, confirmed_at: phone_configuration.confirmed_at, diff --git a/lib/data_requests/local/write_cloudwatch_logs.rb b/lib/data_requests/local/write_cloudwatch_logs.rb index c9f5a1939ab..9805edd5877 100644 --- a/lib/data_requests/local/write_cloudwatch_logs.rb +++ b/lib/data_requests/local/write_cloudwatch_logs.rb @@ -4,6 +4,7 @@ module DataRequests module Local class WriteCloudwatchLogs HEADERS = %w[ + uuid timestamp event_name success @@ -14,19 +15,24 @@ class WriteCloudwatchLogs user_agent ].freeze - attr_reader :cloudwatch_results, :output_dir + attr_reader :cloudwatch_results, :requesting_issuer_uuid, :csv - def initialize(cloudwatch_results, output_dir) + def initialize(cloudwatch_results:, requesting_issuer_uuid:, csv:, include_header: false) @cloudwatch_results = cloudwatch_results - @output_dir = output_dir + @requesting_issuer_uuid = requesting_issuer_uuid + @csv = csv + @include_header = include_header + end + + def include_header? + !!@include_header end def call - CSV.open(File.join(output_dir, 'logs.csv'), 'w') do |csv| - csv << HEADERS - cloudwatch_results.each do |row| - csv << build_row(row) - end + csv << HEADERS if include_header? + + cloudwatch_results.each do |row| + csv << build_row(row) end end @@ -60,6 +66,7 @@ def build_row(row) user_agent = data.dig('properties', 'user_agent') [ + requesting_issuer_uuid, timestamp, event_name, success, diff --git a/lib/data_requests/local/write_user_events.rb b/lib/data_requests/local/write_user_events.rb index 034deaf7e1d..f60fafce969 100644 --- a/lib/data_requests/local/write_user_events.rb +++ b/lib/data_requests/local/write_user_events.rb @@ -3,16 +3,21 @@ module DataRequests module Local class WriteUserEvents - attr_reader :user_report, :output_dir, :requesting_issuer_uuid + attr_reader :user_report, :requesting_issuer_uuid, :csv - def initialize(user_report, output_dir, requesting_issuer_uuid) + def initialize(user_report:, requesting_issuer_uuid:, csv:, include_header: false) @user_report = user_report - @output_dir = output_dir + @csv = csv @requesting_issuer_uuid = requesting_issuer_uuid + @include_header = include_header + end + + def include_header? + !!@include_header end def call - CSV.open(File.join(output_dir, 'events.csv'), 'w') do |csv| + if include_header? csv << %w[ uuid event_name @@ -22,17 +27,17 @@ def call user_agent device_cookie ] + end - user_report[:user_events].each do |row| - csv << [requesting_issuer_uuid] + row.values_at( - :event_name, - :date_time, - :ip, - :disavowed_at, - :user_agent, - :device_cookie, - ) - end + user_report[:user_events].each do |row| + csv << [requesting_issuer_uuid] + row.values_at( + :event_name, + :date_time, + :ip, + :disavowed_at, + :user_agent, + :device_cookie, + ) end end end diff --git a/lib/data_requests/local/write_user_info.rb b/lib/data_requests/local/write_user_info.rb index e1f67b23209..389d0a2d230 100644 --- a/lib/data_requests/local/write_user_info.rb +++ b/lib/data_requests/local/write_user_info.rb @@ -3,86 +3,122 @@ module DataRequests module Local class WriteUserInfo - attr_reader :user_report, :output_dir + attr_reader :user_report, :csv - def initialize(user_report, output_dir) + def initialize(user_report:, csv:, include_header: false) @user_report = user_report - @output_dir = output_dir + @csv = csv + @include_header = include_header + end + + def include_header? + !!@include_header end def call + if include_header? + csv << %w[ + uuid + type + value + created_at + confirmed_at + internal_id + ] + end + + write_not_found write_emails write_phone_configurations write_auth_app_configurations write_webauthn_configurations write_piv_cac_configurations write_backup_code_configurations - output_file.close end private - def output_file - @output_file ||= begin - output_path = File.join(output_dir, 'user.csv') - File.open(output_path, 'w') - end + def uuid + user_report[:requesting_issuer_uuid] end - def write_rows_to_csv(rows, *columns) - output_file.puts(columns.join(',')) - - return output_file.puts("No data\n\n") if rows.empty? - - rows.each do |row| - output_file.puts CSV.generate_line(row.values_at(*columns)) + def write_not_found + if user_report[:not_found] + csv << [ + uuid, + 'not found', + ] end - output_file.puts("\n") end def write_auth_app_configurations - output_file.puts('Auth app configurations:') - write_rows_to_csv( - user_report[:mfa_configurations][:auth_app_configurations], - :name, :created_at - ) + user_report[:mfa_configurations][:auth_app_configurations].each do |auth_app_config| + csv << [ + uuid, + 'Auth app configuration', + auth_app_config[:name], + auth_app_config[:created_at], + ] + end end def write_backup_code_configurations - output_file.puts('Backup code configurations:') - write_rows_to_csv( - user_report[:mfa_configurations][:backup_code_configurations], - :created_at, :used_at - ) + user_report[:mfa_configurations][:backup_code_configurations].each do |backup_code_config| + csv << [ + uuid, + 'Backup code configuration', + nil, + backup_code_config[:created_at], + backup_code_config[:used_at], + ] + end end def write_emails - output_file.puts('Emails:') - write_rows_to_csv(user_report[:email_addresses], :email, :created_at, :confirmed_at) + user_report[:email_addresses].each do |email| + csv << [ + uuid, + 'Email', + email[:email], + email[:created_at], + email[:confirmed_at], + ] + end end def write_phone_configurations - output_file.puts('Phone configurations:') - write_rows_to_csv( - user_report[:mfa_configurations][:phone_configurations], - :phone, :created_at, :confirmed_at - ) + user_report[:mfa_configurations][:phone_configurations].each do |phone_config| + csv << [ + uuid, + 'Phone configuration', + phone_config[:phone], + phone_config[:created_at], + phone_config[:confirmed_at], + phone_config[:id], + ] + end end def write_piv_cac_configurations - output_file.puts('PIV/CAC configurations:') - write_rows_to_csv( - user_report[:mfa_configurations][:piv_cac_configurations], - :name, :created_at - ) + user_report[:mfa_configurations][:piv_cac_configurations].each do |piv_cac_config| + csv << [ + uuid, + 'PIV/CAC configuration', + piv_cac_config[:name], + piv_cac_config[:created_at], + ] + end end def write_webauthn_configurations - output_file.puts('WebAuthn configurations:') - write_rows_to_csv( - user_report[:mfa_configurations][:webauthn_configurations], - :name, :created_at - ) + user_report[:mfa_configurations][:webauthn_configurations].each do |webauthn_config| + csv << [ + uuid, + 'WebAuthn configuration', + webauthn_config[:name], + webauthn_config[:created_at], + ] + end end end end diff --git a/lib/tasks/data_requests.rake b/lib/tasks/data_requests.rake index e6ec274e126..038af267649 100644 --- a/lib/tasks/data_requests.rake +++ b/lib/tasks/data_requests.rake @@ -39,25 +39,49 @@ namespace :data_requests do users_report = JSON.parse(File.read(ENV['USERS_REPORT']), symbolize_names: true) output_dir = ENV['OUTPUT_DIR'] - users_report.each do |user_report| + users_csv = CSV.open(File.join(output_dir, 'users.csv'), 'w') + user_events_csv = CSV.open(File.join(output_dir, 'user_events.csv'), 'w') + logs_csv = CSV.open(File.join(output_dir, 'logs.csv'), 'w') + + users_report.each_with_index do |user_report, idx| puts "Processing user: #{user_report[:requesting_issuer_uuid]}" - user_output_dir = File.join(output_dir, user_report[:requesting_issuer_uuid]) - FileUtils.mkdir_p(user_output_dir) - DataRequests::Local::WriteUserInfo.new(user_report, user_output_dir).call + DataRequests::Local::WriteUserInfo.new( + user_report:, + csv: users_csv, + include_header: idx == 0, + ).call + DataRequests::Local::WriteUserEvents.new( - user_report, user_output_dir, user_report[:requesting_issuer_uuid] + user_report:, + requesting_issuer_uuid: user_report[:requesting_issuer_uuid], + csv: user_events_csv, + include_header: idx == 0, ).call cloudwatch_dates = user_report[:user_events].pluck(:date_time).map do |date_time| Time.zone.parse(date_time).to_date end.uniq + cloudwatch_results = DataRequests::Local::FetchCloudwatchLogs.new( user_report[:login_uuid], cloudwatch_dates, ).call - DataRequests::Local::WriteCloudwatchLogs.new(cloudwatch_results, user_output_dir).call + DataRequests::Local::WriteCloudwatchLogs.new( + cloudwatch_results:, + requesting_issuer_uuid: user_report[:requesting_issuer_uuid], + csv: logs_csv, + include_header: idx == 0, + ).call + + users_csv.flush + user_events_csv.flush + logs_csv.flush end + ensure + users_csv&.close + user_events_csv&.close + logs_csv&.close end end diff --git a/spec/fixtures/data_request.json b/spec/fixtures/data_request.json index a24d8f3614c..71a099d0e2b 100644 --- a/spec/fixtures/data_request.json +++ b/spec/fixtures/data_request.json @@ -13,6 +13,7 @@ "mfa_configurations": { "phone_configurations": [ { + "id": 123456, "phone": "+1 555-555-5555", "created_at": "2021-10-21T14:53:08.803Z", "confirmed_at": "2021-10-21T14:53:08.790Z" diff --git a/spec/lib/data_pull_spec.rb b/spec/lib/data_pull_spec.rb index ec4efaf551e..44d28518014 100644 --- a/spec/lib/data_pull_spec.rb +++ b/spec/lib/data_pull_spec.rb @@ -126,6 +126,34 @@ :user_events, ) end + + context 'with a UUID that is not found' do + let(:uuid) { 'abcdef' } + let(:argv) do + ['ig-request', uuid, '--requesting-issuer', service_provider.issuer] + end + + it 'returns an empty hash for that user' do + data_pull.run + + response = JSON.parse(stdout.string, symbolize_names: true) + expect(response.first).to eq( + user_id: nil, + login_uuid: nil, + requesting_issuer_uuid: uuid, + email_addresses: [], + mfa_configurations: { + phone_configurations: [], + auth_app_configurations: [], + webauthn_configurations: [], + piv_cac_configurations: [], + backup_code_configurations: [], + }, + user_events: [], + not_found: true, + ) + end + end end end diff --git a/spec/lib/data_requests/deployed/create_mfa_configurations_report_spec.rb b/spec/lib/data_requests/deployed/create_mfa_configurations_report_spec.rb index d11712ed2d4..6477d692196 100644 --- a/spec/lib/data_requests/deployed/create_mfa_configurations_report_spec.rb +++ b/spec/lib/data_requests/deployed/create_mfa_configurations_report_spec.rb @@ -10,6 +10,7 @@ result = described_class.new(user).call phone_data = result[:phone_configurations] + expect(phone_data.first[:id]).to eq(phone_configuration.id) expect(phone_data.first[:phone]).to eq(phone_configuration.phone) expect(phone_data.first[:created_at]).to be_within(1.second).of( phone_configuration.created_at, diff --git a/spec/lib/data_requests/local/write_cloudwatch_logs_spec.rb b/spec/lib/data_requests/local/write_cloudwatch_logs_spec.rb index eb2de59f2b7..732cb464bb6 100644 --- a/spec/lib/data_requests/local/write_cloudwatch_logs_spec.rb +++ b/spec/lib/data_requests/local/write_cloudwatch_logs_spec.rb @@ -30,23 +30,28 @@ def build_result_row(event_properties = {}) ] end - around do |ex| - Dir.mktmpdir do |dir| - @output_dir = dir - ex.run - end - end + let(:io) { StringIO.new } + let(:csv) { CSV.new(io) } + + let(:requesting_issuer_uuid) { SecureRandom.hex } + let(:include_header) { true } subject(:writer) do - DataRequests::Local::WriteCloudwatchLogs.new(cloudwatch_results, @output_dir) + DataRequests::Local::WriteCloudwatchLogs.new( + cloudwatch_results:, + csv:, + requesting_issuer_uuid:, + include_header: include_header, + ) end describe '#call' do - it 'writes the logs to output_dir/logs.csv' do + it 'writes the logs to the CSV' do writer.call - row = CSV.read(File.join(@output_dir, 'logs.csv'), headers: true).first + row = CSV.parse(io.string, headers: true).first + expect(row['uuid']).to eq(requesting_issuer_uuid) expect(row['timestamp']).to eq(now.iso8601) expect(row['event_name']).to eq('Some Log: Event') expect(row['success']).to eq('true') @@ -83,7 +88,7 @@ def build_result_row(event_properties = {}) it 'unpacks all multi factor ids' do writer.call - csv = CSV.read(File.join(@output_dir, 'logs.csv'), headers: true) + csv = CSV.parse(io.string, headers: true) expect(csv.map { |row| [row['multi_factor_auth_method'], row['multi_factor_id']] }). to eq( diff --git a/spec/lib/data_requests/local/write_user_events_spec.rb b/spec/lib/data_requests/local/write_user_events_spec.rb index 3f5c52a650c..407ce4d554d 100644 --- a/spec/lib/data_requests/local/write_user_events_spec.rb +++ b/spec/lib/data_requests/local/write_user_events_spec.rb @@ -5,18 +5,30 @@ let(:requesting_issuer_uuid) { SecureRandom.uuid } describe '#call' do - it 'writes a file with event information' do - user_report = JSON.parse( + let(:user_report) do + JSON.parse( File.read('spec/fixtures/data_request.json'), symbolize_names: true ).first + end + + let(:io) { StringIO.new } + let(:csv) { CSV.new(io) } + + subject(:writer) do + described_class.new( + user_report:, + requesting_issuer_uuid:, + csv:, + include_header: true, + ) + end + + it 'writes a file with event information' do + writer.call - Dir.mktmpdir do |dir| - described_class.new(user_report, dir, requesting_issuer_uuid).call - events = File.read(File.join(dir, 'events.csv')) - csv = CSV.parse(events, headers: true) - expect(csv.first['uuid']).to eq(requesting_issuer_uuid) - expect(csv.count).to eq(user_report[:user_events].length) - end + parsed = CSV.parse(io.string, headers: true) + expect(parsed.first['uuid']).to eq(requesting_issuer_uuid) + expect(parsed.count).to eq(user_report[:user_events].length) end end end diff --git a/spec/lib/data_requests/local/write_user_info_spec.rb b/spec/lib/data_requests/local/write_user_info_spec.rb index 6fca619b720..13acab62f04 100644 --- a/spec/lib/data_requests/local/write_user_info_spec.rb +++ b/spec/lib/data_requests/local/write_user_info_spec.rb @@ -3,24 +3,70 @@ RSpec.describe DataRequests::Local::WriteUserInfo do describe '#call' do - it 'writes a file with user information' do - user_report = JSON.parse( + let(:io) { StringIO.new } + let(:csv) { CSV.new(io) } + + subject(:instance) do + DataRequests::Local::WriteUserInfo.new( + user_report:, + csv:, + include_header: true, + ) + end + + let(:user_report) do + JSON.parse( File.read('spec/fixtures/data_request.json'), symbolize_names: true ).first + end + let(:uuid) { user_report[:requesting_issuer_uuid] } + + it 'adds user information to the CSV' do + instance.call + + parsed = CSV.parse(io.string, headers: true) + + email_row = parsed.find { |r| r['type'] == 'Email' } + expect(email_row['uuid']).to eq(uuid) + expect(email_row['value']).to eq('test@example.com') + expect(email_row['created_at']).to be_present + expect(email_row['confirmed_at']).to be_present + expect(email_row['internal_id']).to be_nil + + phone_row = parsed.find { |r| r['type'] == 'Phone configuration' } + expect(phone_row['uuid']).to eq(uuid) + expect(phone_row['value']).to eq('+1 555-555-5555') + expect(phone_row['created_at']).to be_present + expect(phone_row['confirmed_at']).to be_present + expect(phone_row['internal_id']).to be_present + end + + context 'with a not_found user' do + let(:uuid) { SecureRandom.hex } + let(:user_report) do + { + user_id: nil, + login_uuid: nil, + requesting_issuer_uuid: uuid, + email_addresses: [], + mfa_configurations: { + phone_configurations: [], + auth_app_configurations: [], + webauthn_configurations: [], + piv_cac_configurations: [], + backup_code_configurations: [], + }, + user_events: [], + not_found: true, + } + end + + it 'writes a not found row' do + instance.call - Dir.mktmpdir do |dir| - described_class.new(user_report, dir).call - user = File.read(File.join(dir, 'user.csv')) - headings = user.split("\n\n").map do |info_group| - info_group.split("\n").first - end - - expect(headings).to include('Emails:') - expect(headings).to include('Phone configurations:') - expect(headings).to include('Auth app configurations:') - expect(headings).to include('WebAuthn configurations:') - expect(headings).to include('PIV/CAC configurations:') - expect(headings).to include('Backup code configurations:') + parsed = CSV.parse(io.string, headers: true) + expect(parsed.first['uuid']).to eq(uuid) + expect(parsed.first['type']).to eq('not found') end end end From 6f889cc95b22a2dd570f9a808c6192766fbfa9da Mon Sep 17 00:00:00 2001 From: "Davida (she/they)" Date: Tue, 13 Feb 2024 15:32:10 -0500 Subject: [PATCH 10/19] LG-11648 MFA method report (#10076) * changelog: Internal, Reporting, Adds an MFA Method on sign-in report --- lib/reporting/mfa_report.rb | 209 ++++++++++++++++++++++++++ spec/lib/reporting/mfa_report_spec.rb | 168 +++++++++++++++++++++ 2 files changed, 377 insertions(+) create mode 100644 lib/reporting/mfa_report.rb create mode 100644 spec/lib/reporting/mfa_report_spec.rb diff --git a/lib/reporting/mfa_report.rb b/lib/reporting/mfa_report.rb new file mode 100644 index 00000000000..e53b6648bbe --- /dev/null +++ b/lib/reporting/mfa_report.rb @@ -0,0 +1,209 @@ +# frozen_string_literal: true + +require 'csv' +begin + require 'reporting/cloudwatch_client' + require 'reporting/cloudwatch_query_quoting' + require 'reporting/command_line_options' +rescue LoadError => e + warn 'could not load paths, try running with "bundle exec rails runner"' + raise e +end + +module Reporting + class MfaReport + include Reporting::CloudwatchQueryQuoting + + attr_reader :issuers, :time_range + EVENT = 'Multi-Factor Authentication' + + module Methods + def self.all_methods + TwoFactorAuthenticatable::AuthMethod.constants.map do |c| + next if c == :PHISHING_RESISTANT_METHODS || c == :REMEMBER_DEVICE + + if c == :PERSONAL_KEY + # The AuthMethod constant defines this as `personal_key` + # but in the events it is `personal-key` + 'personal-key' + else + TwoFactorAuthenticatable::AuthMethod.const_get(c) + end + end.compact + end + end + + # @param [Array] issuers + # @param [Range