From c9ef96189fc997b949f13d1450f3ddb47fa1ed47 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 6 Feb 2024 13:24:47 -0500 Subject: [PATCH 01/16] Swap `#empty?` with `#blank?` and add tests (#10045) The commit that was un-reverted here was problematic because of a `NoMethodError` when calling `empty?` on `nil` here: ```ruby validates :acr_values, presence: true, if: ->(form) { form.vtr.empty? } ``` This commit replaces `#empty?` with `#blank?` which will work on nil. Additionally the `#parse_vtr` method was modified to consistently return `nil` if either a `vtr` param is not provided or the `vtr` param is not enabled. Tests were added to cover this case. [skip changelog] --- app/forms/openid_connect_authorize_form.rb | 55 ++- app/models/federated_protocols/oidc.rb | 8 + app/models/federated_protocols/saml.rb | 8 + app/services/analytics_events.rb | 3 + .../service_provider_request_handler.rb | 2 + app/services/store_sp_metadata_in_session.rb | 2 + config/application.yml.default | 2 + config/locales/openid_connect/en.yml | 1 + config/locales/openid_connect/es.yml | 1 + config/locales/openid_connect/fr.yml | 1 + lib/identity_config.rb | 1 + .../authorization_controller_spec.rb | 26 +- spec/controllers/saml_idp_controller_spec.rb | 16 + .../openid_connect_authorize_form_spec.rb | 437 +++++++++++------- .../store_sp_metadata_in_session_spec.rb | 66 +++ 15 files changed, 461 insertions(+), 168 deletions(-) diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb index 9000ec7c518..bb2c5afb04d 100644 --- a/app/forms/openid_connect_authorize_form.rb +++ b/app/forms/openid_connect_authorize_form.rb @@ -20,6 +20,7 @@ class OpenidConnectAuthorizeForm ATTRS = [ :unauthorized_scope, :acr_values, + :vtr, :scope, :verified_within, :biometric_comparison_required, @@ -37,7 +38,7 @@ class OpenidConnectAuthorizeForm RANDOM_VALUE_MINIMUM_LENGTH = 22 MINIMUM_REPROOF_VERIFIED_WITHIN_DAYS = 30 - validates :acr_values, presence: true + validates :acr_values, presence: true, if: ->(form) { form.vtr.blank? } validates :client_id, presence: true validates :redirect_uri, presence: true validates :scope, presence: true @@ -49,6 +50,7 @@ class OpenidConnectAuthorizeForm validates :code_challenge_method, inclusion: { in: %w[S256] }, if: :code_challenge validate :validate_acr_values + validate :validate_vtr validate :validate_client_id validate :validate_scope validate :validate_unauthorized_scope @@ -59,6 +61,7 @@ class OpenidConnectAuthorizeForm def initialize(params) @acr_values = parse_to_values(params[:acr_values], Saml::Idp::Constants::VALID_AUTHN_CONTEXTS) + @vtr = parse_vtr(params[:vtr]) SIMPLE_ATTRS.each { |key| instance_variable_set(:"@#{key}", params[key]) } @prompt ||= 'select_account' @scope = parse_to_values(params[:scope], scopes) @@ -119,7 +122,13 @@ def ial_context end def ial - Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_IAL[ial_values.sort.max] + if parsed_vector_of_trust&.identity_proofing? + 2 + elsif parsed_vector_of_trust.present? + 1 + else + Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_IAL[ial_values.sort.max] + end end def aal_values @@ -127,7 +136,13 @@ def aal_values end def aal - Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[requested_aal_value] + if parsed_vector_of_trust&.aal2? + 2 + elsif parsed_vector_of_trust.present? + 1 + else + Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[requested_aal_value] + end end def requested_aal_value @@ -163,7 +178,18 @@ def parse_to_values(param_value, possible_values) param_value.split(' ').compact & possible_values end + def parse_vtr(param_value) + return if !IdentityConfig.store.use_vot_in_sp_requests + return if param_value.blank? + + JSON.parse(param_value) + rescue JSON::ParserError + nil + end + def validate_acr_values + return if vtr.present? + if acr_values.empty? errors.add( :acr_values, t('openid_connect.authorization.errors.no_valid_acr_values'), @@ -177,6 +203,15 @@ def validate_acr_values end end + def validate_vtr + return if vtr.blank? + return if parsed_vector_of_trust.present? + errors.add( + :vtr, t('openid_connect.authorization.errors.no_valid_vtr'), + type: :no_valid_vtr + ) + end + # This checks that the SP matches something in the database # OpenidConnect::AuthorizationController#check_sp_active checks that it's currently active def validate_client_id @@ -246,6 +281,7 @@ def extra_analytics_attributes redirect_uri: result_uri, scope: scope&.sort&.join(' '), acr_values: acr_values&.sort&.join(' '), + vtr: vtr, unauthorized_scope: @unauthorized_scope, code_digest: code ? Digest::SHA256.hexdigest(code) : nil, code_challenge_present: code_challenge.present?, @@ -275,6 +311,19 @@ def scopes OpenidConnectAttributeScoper::VALID_IAL1_SCOPES end + def parsed_vector_of_trust + return @parsed_vector_of_trust if defined?(@parsed_vector_of_trust) + return @parsed_vector_of_trust = nil if vtr.blank? + + @parsed_vector_of_trust = begin + if vtr.is_a?(Array) && !vtr.empty? + Vot::Parser.new(vector_of_trust: vtr.first).parse + end + rescue Vot::Parser::ParseException + nil + end + end + def validate_privileges if (ial2_requested? && !ial_context.ial2_service_provider?) || (ial_context.ialmax_requested? && diff --git a/app/models/federated_protocols/oidc.rb b/app/models/federated_protocols/oidc.rb index 33b92251cf3..be1605d9b6d 100644 --- a/app/models/federated_protocols/oidc.rb +++ b/app/models/federated_protocols/oidc.rb @@ -16,6 +16,14 @@ def aal request.aal_values.sort.max end + def acr_values + [aal, ial].compact.join(' ') + end + + def vtr + request.vtr + end + def requested_attributes OpenidConnectAttributeScoper.new(request.scope).requested_attributes end diff --git a/app/models/federated_protocols/saml.rb b/app/models/federated_protocols/saml.rb index ecc0dea6569..7659d466abd 100644 --- a/app/models/federated_protocols/saml.rb +++ b/app/models/federated_protocols/saml.rb @@ -16,6 +16,14 @@ def aal request.requested_aal_authn_context end + def acr_values + [aal, ial].compact.join(' ') + end + + def vtr + nil + end + def requested_attributes @requested_attributes ||= SamlRequestPresenter.new( request: request, service_provider: current_service_provider, diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 7f392be8655..d7e5e29ef14 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3636,12 +3636,14 @@ def openid_connect_bearer_token(success:, ial:, client_id:, errors:, **extra) # @param [String] client_id # @param [String] scope # @param [Array] acr_values + # @param [Array] vtr # @param [Boolean] unauthorized_scope # @param [Boolean] user_fully_authenticated def openid_connect_request_authorization( client_id:, scope:, acr_values:, + vtr:, unauthorized_scope:, user_fully_authenticated:, **extra @@ -3651,6 +3653,7 @@ def openid_connect_request_authorization( client_id: client_id, scope: scope, acr_values: acr_values, + vtr: vtr, unauthorized_scope: unauthorized_scope, user_fully_authenticated: user_fully_authenticated, **extra, diff --git a/app/services/service_provider_request_handler.rb b/app/services/service_provider_request_handler.rb index 089293b8f77..bd62d915a9c 100644 --- a/app/services/service_provider_request_handler.rb +++ b/app/services/service_provider_request_handler.rb @@ -63,6 +63,8 @@ def attributes issuer: protocol.issuer, ial: protocol.ial, aal: protocol.aal, + acr_values: protocol.acr_values, + vtr: protocol.vtr, requested_attributes: protocol.requested_attributes, biometric_comparison_required: protocol.biometric_comparison_required?, uuid: request_id, diff --git a/app/services/store_sp_metadata_in_session.rb b/app/services/store_sp_metadata_in_session.rb index 13c052b2646..c3c0577504e 100644 --- a/app/services/store_sp_metadata_in_session.rb +++ b/app/services/store_sp_metadata_in_session.rb @@ -37,6 +37,8 @@ def update_session request_id: sp_request.uuid, requested_attributes: sp_request.requested_attributes, biometric_comparison_required: sp_request.biometric_comparison_required, + acr_values: sp_request.acr_values, + vtr: sp_request.vtr, } end diff --git a/config/application.yml.default b/config/application.yml.default index e848e7bc7fe..e599da246eb 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -321,6 +321,7 @@ team_ursula_email: '' test_ssn_allowed_list: '' totp_code_interval: 30 unauthorized_scope_enabled: false +use_vot_in_sp_requests: true usps_upload_enabled: false usps_upload_sftp_timeout: 5 valid_authn_contexts: '["http://idmanagement.gov/ns/assurance/loa/1", "http://idmanagement.gov/ns/assurance/loa/3", "http://idmanagement.gov/ns/assurance/ial/1", "http://idmanagement.gov/ns/assurance/ial/2", "http://idmanagement.gov/ns/assurance/ial/0", "http://idmanagement.gov/ns/assurance/ial/2?strict=true", "urn:gov:gsa:ac:classes:sp:PasswordProtectedTransport:duo", "http://idmanagement.gov/ns/assurance/aal/2", "http://idmanagement.gov/ns/assurance/aal/3", "http://idmanagement.gov/ns/assurance/aal/3?hspd12=true","http://idmanagement.gov/ns/assurance/aal/2?phishing_resistant=true","http://idmanagement.gov/ns/assurance/aal/2?hspd12=true"]' @@ -494,6 +495,7 @@ production: state_tracking_enabled: false telephony_adapter: pinpoint use_kms: true + use_vot_in_sp_requests: false usps_auth_token_refresh_job_enabled: true usps_confirmation_max_days: 30 usps_upload_sftp_directory: '' diff --git a/config/locales/openid_connect/en.yml b/config/locales/openid_connect/en.yml index 3c68137adbd..bd5eb70fd1c 100644 --- a/config/locales/openid_connect/en.yml +++ b/config/locales/openid_connect/en.yml @@ -12,6 +12,7 @@ en: no_auth: The acr_values are not authorized no_valid_acr_values: No acceptable acr_values found no_valid_scope: No valid scope values found + no_valid_vtr: No acceptable vots found prompt_invalid: No valid prompt values found redirect_uri_invalid: redirect_uri is invalid redirect_uri_no_match: redirect_uri does not match registered redirect_uri diff --git a/config/locales/openid_connect/es.yml b/config/locales/openid_connect/es.yml index 51ea9ba75b2..7aac8a1747c 100644 --- a/config/locales/openid_connect/es.yml +++ b/config/locales/openid_connect/es.yml @@ -12,6 +12,7 @@ es: no_auth: Los acr_values no están autorizados no_valid_acr_values: ial_valores encontrados no aceptables no_valid_scope: No se han encontrado valores de magnitud válidos + no_valid_vtr: vots encontrados no aceptables prompt_invalid: Prompt no es válido redirect_uri_invalid: Redirect_uri no es válido redirect_uri_no_match: Redirect_uri no coincide con redirect_uri registrado diff --git a/config/locales/openid_connect/fr.yml b/config/locales/openid_connect/fr.yml index 2d8f7eefe24..2ee66ca55c1 100644 --- a/config/locales/openid_connect/fr.yml +++ b/config/locales/openid_connect/fr.yml @@ -12,6 +12,7 @@ fr: no_auth: Les acr_values ne sont pas autorisées no_valid_acr_values: Valeurs acr_values inacceptables trouvées no_valid_scope: Aucune étendue de données valide trouvée + no_valid_vtr: vots encontrados no aceptables prompt_invalid: prompt est non valide redirect_uri_invalid: redirect_uri est non valide redirect_uri_no_match: redirect_uri ne correspond pas au redirect_uri enregistré diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 8d07bde22cf..894ae4ebd83 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -461,6 +461,7 @@ def self.build_store(config_map) config.add(:unauthorized_scope_enabled, type: :boolean) config.add(:use_dashboard_service_providers, type: :boolean) config.add(:use_kms, type: :boolean) + config.add(:use_vot_in_sp_requests, type: :boolean) config.add(:usps_auth_token_refresh_job_enabled, type: :boolean) config.add(:usps_confirmation_max_days, type: :integer) config.add(:usps_ipp_client_id, type: :string) diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index da601abc81e..284024ef17a 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -110,7 +110,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid') + scope: 'openid', + vtr: nil) expect(@analytics).to receive(:track_event). with('OpenID Connect: authorization request handoff', success: true, @@ -257,7 +258,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/2', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid profile') + scope: 'openid profile', + vtr: nil) expect(@analytics).to receive(:track_event). with('OpenID Connect: authorization request handoff', success: true, @@ -495,7 +497,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid profile') + scope: 'openid profile', + vtr: nil) expect(@analytics).to receive(:track_event). with('OpenID Connect: authorization request handoff', success: true, @@ -578,7 +581,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid profile') + scope: 'openid profile', + vtr: nil) expect(@analytics).to receive(:track_event). with('OpenID Connect: authorization request handoff', success: true, @@ -663,7 +667,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid profile') + scope: 'openid profile', + vtr: nil) expect(@analytics).to receive(:track_event). with('OpenID Connect: authorization request handoff', success: true, @@ -865,7 +870,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid') + scope: 'openid', + vtr: nil) expect(@analytics).to_not receive(:track_event).with('sp redirect initiated') action @@ -898,7 +904,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid') + scope: 'openid', + vtr: nil) expect(@analytics).to_not receive(:track_event).with('SP redirect initiated') action @@ -1013,7 +1020,8 @@ acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', code_challenge_present: false, service_provider_pkce: nil, - scope: 'openid') + scope: 'openid', + vtr: nil) action sp_request_id = ServiceProviderRequestProxy.last.uuid @@ -1028,6 +1036,7 @@ expect(session[:sp]).to eq( aal_level_requested: nil, + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, piv_cac_requested: false, phishing_resistant_requested: false, ial: 1, @@ -1038,6 +1047,7 @@ request_url: request.original_url, requested_attributes: %w[], biometric_comparison_required: false, + vtr: nil, ) end diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 511195be937..d07d0858dfa 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -1166,6 +1166,12 @@ def name_id_version(format_urn) end context 'POST to auth correctly stores SP in session' do + let(:acr_values) do + Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF + + ' ' + + Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF + end + before do @user = create(:user, :fully_registered) @saml_request = saml_request(saml_settings) @@ -1181,6 +1187,7 @@ def name_id_version(format_urn) issuer: saml_settings.issuer, aal_level_requested: aal_level, piv_cac_requested: false, + acr_values: acr_values, phishing_resistant_requested: false, ial: 1, ial2: false, @@ -1189,6 +1196,7 @@ def name_id_version(format_urn) request_id: sp_request_id, requested_attributes: ['email'], biometric_comparison_required: false, + vtr: nil, ) end @@ -1201,6 +1209,12 @@ def name_id_version(format_urn) end context 'service provider is valid' do + let(:acr_values) do + Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF + + ' ' + + Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF + end + before do @user = create(:user, :fully_registered) @saml_request = saml_get_auth(saml_settings) @@ -1212,6 +1226,7 @@ def name_id_version(format_urn) expect(session[:sp]).to eq( issuer: saml_settings.issuer, aal_level_requested: aal_level, + acr_values: acr_values, piv_cac_requested: false, phishing_resistant_requested: false, ial: 1, @@ -1221,6 +1236,7 @@ def name_id_version(format_urn) request_id: sp_request_id, requested_attributes: ['email'], biometric_comparison_required: false, + vtr: nil, ) end diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index fafe7188723..a019462b577 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -4,6 +4,7 @@ subject(:form) do OpenidConnectAuthorizeForm.new( acr_values: acr_values, + vtr: vtr, client_id: client_id, nonce: nonce, prompt: prompt, @@ -18,12 +19,8 @@ ) end - let(:acr_values) do - [ - Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, - ].join(' ') - end - + let(:acr_values) { nil } + let(:vtr) { ['C1'].to_json } let(:client_id) { 'urn:gov:gsa:openidconnect:test' } let(:nonce) { SecureRandom.hex } let(:prompt) { 'select_account' } @@ -49,7 +46,8 @@ allow_prompt_login: true, redirect_uri: nil, unauthorized_scope: true, - acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', + acr_values: '', + vtr: JSON.parse(vtr), scope: 'openid', code_digest: nil, code_challenge_present: false, @@ -73,7 +71,8 @@ redirect_uri: "#{redirect_uri}?error=invalid_request&error_description=" \ "Response+type+is+not+included+in+the+list&state=#{state}", unauthorized_scope: true, - acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', + acr_values: '', + vtr: JSON.parse(vtr), scope: 'openid', code_digest: nil, code_challenge_present: false, @@ -93,6 +92,44 @@ expect(result.extra[:redirect_uri]).to be_nil end end + + context 'when use_vot_in_sp_requests flag is false' do + before do + allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).and_return(false) + end + + context 'with only a vtr param' do + let(:vtr) { ['C1.P1'].to_json } + let(:acr_values) { nil } + + it 'is invalid' do + expect(form.vtr).to be_nil + expect(form.valid?).to eq(false) + expect(form.errors[:acr_values]). + to include(t('openid_connect.authorization.errors.no_valid_acr_values')) + expect(form.errors[:vtr]).to be_empty + end + end + + context 'with a vtr and acr_values param' do + let(:vtr) { ['C1.P1'].to_json } + let(:acr_values) { Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF } + + it 'uses the acr_values param and ignores vtr' do + expect(form.vtr).to be_nil + expect(form.valid?).to eq(true) + end + end + + context 'with only an acr_values param' do + let(:vtr) { nil } + let(:acr_values) { Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF } + + it 'uses the acr_values param' do + expect(form.valid?).to eq(true) + end + end + end end describe '#valid?' do @@ -106,8 +143,18 @@ end end + 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]). + to include(t('openid_connect.authorization.errors.no_valid_vtr')) + end + end + context 'with no valid acr_values' do let(:acr_values) { 'abc def' } + let(:vtr) { nil } it 'has errors' do expect(valid?).to eq(false) expect(form.errors[:acr_values]). @@ -115,8 +162,19 @@ end end + 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]). + to include(t('openid_connect.authorization.errors.no_auth')) + end + end + context 'with no authorized acr_values' do let(:acr_values) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF } + let(:vtr) { nil } let(:client_id) { 'urn:gov:gsa:openidconnect:test:loa1' } it 'has errors' do expect(valid?).to eq(false) @@ -127,6 +185,7 @@ context 'with ialmax requested' do let(:acr_values) { Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF } + let(:vtr) { nil } context 'with a service provider not in the allow list' do it 'has errors' do @@ -150,6 +209,7 @@ context 'with aal but not ial requested via acr_values' do let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } + let(:vtr) { nil } it 'has errors' do expect(valid?).to eq(false) expect(form.errors[:acr_values]). @@ -248,7 +308,7 @@ context 'when scope includes profile:verified_at but the sp is only ial1' do let(:client_id) { 'urn:gov:gsa:openidconnect:test:loa1' } - let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + let(:vtr) { ['C1'].to_json } let(:scope) { 'email profile:verified_at' } it 'has errors' do @@ -336,6 +396,7 @@ 'fake_value', ].join(' ') end + let(:vtr) { nil } it 'is parsed into an array of valid ACR values' do expect(form.acr_values).to eq( @@ -348,209 +409,252 @@ end describe '#ial' do - context 'when IAL1 passed' do - let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + context 'with vtr param' do + let(:acr_values) { nil } - it 'returns 1' do - expect(form.ial).to eq(1) + context 'when proofing is requested' do + let(:vtr) { ['C1.P1'].to_json } + + it { expect(form.ial).to eq(2) } end - end - context 'when IAL2 passed' do - let(:acr_values) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF } + context 'when proofing is not requested' do + let(:vtr) { ['C1'].to_json } - it 'returns 2' do - expect(form.ial).to eq(2) + it { expect(form.ial).to eq(1) } end end - context 'when IALMAX passed' do - let(:acr_values) { Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF } + context 'with acr_values param' do + let(:vtr) { nil } - it 'returns 0' do - expect(form.ial).to eq(0) + context 'when IAL1 passed' do + let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + + it 'returns 1' do + expect(form.ial).to eq(1) + end end - end - context 'when LOA1 passed' do - let(:acr_values) { Saml::Idp::Constants::LOA1_AUTHN_CONTEXT_CLASSREF } + context 'when IAL2 passed' do + let(:acr_values) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF } - it 'returns 1' do - expect(form.ial).to eq(1) + it 'returns 2' do + expect(form.ial).to eq(2) + end end - end - context 'when LOA3 passed' do - let(:acr_values) { Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF } + context 'when IALMAX passed' do + let(:acr_values) { Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF } - it 'returns 2' do - expect(form.ial).to eq(2) + it 'returns 0' do + expect(form.ial).to eq(0) + end + end + + context 'when LOA1 passed' do + let(:acr_values) { Saml::Idp::Constants::LOA1_AUTHN_CONTEXT_CLASSREF } + + it 'returns 1' do + expect(form.ial).to eq(1) + end + end + + context 'when LOA3 passed' do + let(:acr_values) { Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF } + + it 'returns 2' do + expect(form.ial).to eq(2) + end end end end describe '#aal' do - context 'when no AAL passed' do - let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + context 'with vtr param' do + let(:acr_values) { nil } - it 'returns 0' do - expect(form.aal).to eq(0) + context 'when AAL2 is requested' do + let(:vtr) { ['C2'].to_json } + + it { expect(form.aal).to eq(2) } end - end - context 'when DEFAULT_AAL passed' do - let(:acr_values) { Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2 is not requested' do + let(:vtr) { ['C1'].to_json } - it 'returns 0' do - expect(form.aal).to eq(0) + it { expect(form.aal).to eq(1) } end end - context 'when AAL2 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF } + context 'with acr_values param' do + let(:vtr) { nil } - it 'returns 2' do - expect(form.aal).to eq(2) + context 'when no AAL passed' do + let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + + it 'returns 0' do + expect(form.aal).to eq(0) + end end - end - context 'when AAL2_PHISHING_RESISTANT passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF } + context 'when DEFAULT_AAL passed' do + let(:acr_values) { Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF } - it 'returns 2' do - expect(form.aal).to eq(2) + it 'returns 0' do + expect(form.aal).to eq(0) + end end - end - context 'when AAL2_HSPD12 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF } - it 'returns 2' do - expect(form.aal).to eq(2) + it 'returns 2' do + expect(form.aal).to eq(2) + end end - end - context 'when AAL3 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2_PHISHING_RESISTANT passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF } - it 'returns 3' do - expect(form.aal).to eq(3) + it 'returns 2' do + expect(form.aal).to eq(2) + end end - end - context 'when AAL3_HSPD12 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF } - it 'returns 3' do - expect(form.aal).to eq(3) + it 'returns 2' do + expect(form.aal).to eq(2) + end + end + + context 'when AAL3 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } + + it 'returns 3' do + expect(form.aal).to eq(3) + end end - end - context 'when IAL and AAL passed' do - aal2 = Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF - ial2 = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF + context 'when AAL3_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF } - let(:acr_values) do - "#{aal2} #{ial2}" + it 'returns 3' do + expect(form.aal).to eq(3) + end end - it 'returns ial and aal' do - expect(form.aal).to eq(2) - expect(form.ial).to eq(2) + context 'when IAL and AAL passed' do + aal2 = Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF + ial2 = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF + + let(:acr_values) do + "#{aal2} #{ial2}" + end + + it 'returns ial and aal' do + expect(form.aal).to eq(2) + expect(form.ial).to eq(2) + end end end end describe '#requested_aal_value' do - context 'when AAL2 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF } + context 'with ACR values' do + let(:vtr) { nil } + context 'when AAL2 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF } - it 'returns AAL2' do - expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF) + it 'returns AAL2' do + expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF) + end end - end - context 'when AAL2_PHISHING_RESISTANT passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2_PHISHING_RESISTANT passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF } - it 'returns AAL2+Phishing Resistant' do - expect(form.requested_aal_value).to eq( - Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, - ) + it 'returns AAL2+Phishing Resistant' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, + ) + end end - end - context 'when AAL2_HSPD12 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF } + context 'when AAL2_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF } - it 'returns AAL2+HSPD12' do - expect(form.requested_aal_value).to eq( - Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, - ) + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end end - end - context 'when AAL3 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } + context 'when AAL3 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } - it 'returns AAL3' do - expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF) + it 'returns AAL3' do + expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF) + end end - end - context 'when AAL3_HSPD12 passed' do - let(:acr_values) { Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF } + context 'when AAL3_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF } - it 'returns AAL3+HSPD12' do - expect(form.requested_aal_value).to eq( - Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, - ) + it 'returns AAL3+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end end - end - context 'when AAL3_HSPD12 and AAL2_HSPD12 passed' do - let(:acr_values) do - [Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF].join(' ') - end + context 'when AAL3_HSPD12 and AAL2_HSPD12 passed' do + let(:acr_values) do + [Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF].join(' ') + end - it 'returns AAL2+HSPD12' do - expect(form.requested_aal_value).to eq( - Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, - ) + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end end - end - context 'when AAL2 and AAL2_PHISHING_RESISTANT passed' do - let(:phishing_resistant) do - Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF - end + context 'when AAL2 and AAL2_PHISHING_RESISTANT passed' do + let(:phishing_resistant) do + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF + end - let(:acr_values) do - "#{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF} - #{phishing_resistant}" - end + let(:acr_values) do + "#{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF} + #{phishing_resistant}" + end - it 'returns AAL2+HSPD12' do - expect(form.requested_aal_value).to eq(phishing_resistant) + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq(phishing_resistant) + end end - end - context 'when AAL2_PHISHING_RESISTANT and AAL2 passed' do - # this is the same as the previous test, just reverse ordered - # AAL values, to ensure it doesn't just take the 2nd AAL. - let(:phishing_resistant) do - Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF - end + context 'when AAL2_PHISHING_RESISTANT and AAL2 passed' do + # this is the same as the previous test, just reverse ordered + # AAL values, to ensure it doesn't just take the 2nd AAL. + let(:phishing_resistant) do + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF + end - let(:acr_values) do - "#{phishing_resistant} - #{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF}" - end + let(:acr_values) do + "#{phishing_resistant} + #{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF}" + end - it 'returns AAL2+HSPD12' do - requested_aal_value = form.requested_aal_value - expect(requested_aal_value).to eq(phishing_resistant) + it 'returns AAL2+HSPD12' do + requested_aal_value = form.requested_aal_value + expect(requested_aal_value).to eq(phishing_resistant) + end end end end @@ -646,29 +750,48 @@ describe '#ial2_requested?' do subject(:ial2_requested?) { form.ial2_requested? } - context 'with ial1' do - let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } - it { expect(ial2_requested?).to eq(false) } - end - context 'with ial2' do - let(:acr_values) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF } - it { expect(ial2_requested?).to eq(true) } - end + context 'with vtr params' do + let(:acr_values) { nil } - context 'with ial1 and ial2' do - let(:acr_values) do - [ - Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - ].join(' ') + context 'when identity proofing is requested' do + let(:vtr) { ['P1'].to_json } + it { expect(ial2_requested?).to eq(true) } + end + + context 'when identity proofing is not requested' do + let(:vtr) { ['C1'].to_json } + it { expect(ial2_requested?).to eq(false) } end - it { expect(ial2_requested?).to eq(true) } end - context 'with a malformed ial' do - let(:acr_values) { 'foobarbaz' } - it { expect(ial2_requested?).to eq(false) } + context 'with acr_values param' do + let(:vtr) { nil } + + context 'with ial1' do + let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } + it { expect(ial2_requested?).to eq(false) } + end + + context 'with ial2' do + let(:acr_values) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF } + it { expect(ial2_requested?).to eq(true) } + end + + context 'with ial1 and ial2' do + let(:acr_values) do + [ + Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, + ].join(' ') + end + it { expect(ial2_requested?).to eq(true) } + end + + context 'with a malformed ial' do + let(:acr_values) { 'foobarbaz' } + it { expect(ial2_requested?).to eq(false) } + end end end diff --git a/spec/services/store_sp_metadata_in_session_spec.rb b/spec/services/store_sp_metadata_in_session_spec.rb index 6504e43d929..d1486bb4258 100644 --- a/spec/services/store_sp_metadata_in_session_spec.rb +++ b/spec/services/store_sp_metadata_in_session_spec.rb @@ -18,6 +18,7 @@ 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 @@ -27,6 +28,7 @@ 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, @@ -36,6 +38,7 @@ request_id: request_id, requested_attributes: %w[email], biometric_comparison_required: false, + vtr: nil, } instance.call @@ -51,6 +54,10 @@ 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 @@ -60,6 +67,10 @@ 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, @@ -69,6 +80,7 @@ request_id: request_id, requested_attributes: %w[email], biometric_comparison_required: false, + vtr: nil, } instance.call @@ -84,6 +96,10 @@ 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 @@ -93,6 +109,10 @@ 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, @@ -102,6 +122,7 @@ request_id: request_id, requested_attributes: %w[email], biometric_comparison_required: false, + vtr: nil, } instance.call @@ -117,6 +138,10 @@ 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 @@ -126,6 +151,10 @@ 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, @@ -135,6 +164,43 @@ 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 + end + 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 From 34c2795d067339995855466e4d511c13517dae32 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 6 Feb 2024 13:35:29 -0500 Subject: [PATCH 02/16] LG-12195 Add a method to get resolved authn context (#10043) * create resolved_authn_context_result in ApplicationController added a method in ApplicationController to get the resolved authn context. We pass in acr_values and vtr values to determine what the resolved context is. * add spec for #resolved_authn_context_result * add changelog changelog: Internal, IdV, Add method in application_controller to get resolved authn context --- app/controllers/application_controller.rb | 8 ++++++++ spec/controllers/application_controller_spec.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ee07e3feb88..67679a40869 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -104,6 +104,14 @@ def sign_out(*args) super end + def resolved_authn_context_result + @resolved_authn_context_result ||= AuthnContextResolver.new( + service_provider: current_sp, + vtr: sp_session[:vtr], + acr_values: sp_session[:acr_values], + ).resolve + end + def context user_session[:context] || UserSessionContext::AUTHENTICATION_CONTEXT end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 67ec6865114..11d7a7cc266 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -458,6 +458,23 @@ def index end end + describe '#resolved_authn_context_result' do + it 'returns a resolved authn context result' do + sp = build(:service_provider, ial: 2) + acr_values = [ + 'http://idmanagement.gov/ns/assurance/aal/1', + ].join(' ') + sp_session = { vtr: nil, acr_values: acr_values } + allow(controller).to receive(:current_sp).and_return(sp) + allow(controller).to receive(:sp_session).and_return(sp_session) + + result = subject.resolved_authn_context_result + + expect(result.aal2?).to eq(true) + expect(result.identity_proofing?).to eq(true) + end + end + describe '#sp_session_request_url_with_updated_params' do controller do def index From 2bad5f32f6fb603bf8ae779d9118584978bcfd9d Mon Sep 17 00:00:00 2001 From: eileen-nava <80347702+eileen-nava@users.noreply.github.com> Date: Tue, 6 Feb 2024 16:19:54 -0500 Subject: [PATCH 03/16] Debug make lint country dialing code (#10047) * fix pinpoint urls to enable make update_pinpoint_supported_countries * changelog: Internal, DocAuth, update pinpoint sms and voice urls to fix make lint_country_dialing_codes --- config/country_dialing_codes.yml | 2 +- config/pinpoint_supported_countries.yml | 9 +++++++-- lib/pinpoint_supported_countries.rb | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/config/country_dialing_codes.yml b/config/country_dialing_codes.yml index 67b75141a74..f4053518a49 100644 --- a/config/country_dialing_codes.yml +++ b/config/country_dialing_codes.yml @@ -989,7 +989,7 @@ SV: supports_voice: false SZ: country_code: '268' - name: Swaziland + name: Eswatini supports_sms: true supports_voice: false TC: diff --git a/config/pinpoint_supported_countries.yml b/config/pinpoint_supported_countries.yml index 710d787a4a2..2b3a3450761 100644 --- a/config/pinpoint_supported_countries.yml +++ b/config/pinpoint_supported_countries.yml @@ -34,6 +34,11 @@ AM: name: Armenia supports_sms: true supports_voice: false +AO: + country_code: '244' + name: Angola + supports_sms: true + supports_voice: false AR: country_code: '54' name: Argentina @@ -748,7 +753,7 @@ PA: country_code: '507' name: Panama supports_sms: true - supports_voice: true + supports_voice: false PE: country_code: '51' name: Peru @@ -906,7 +911,7 @@ SV: supports_voice: true SZ: country_code: '268' - name: Swaziland + name: Eswatini supports_sms: true supports_voice: false TC: diff --git a/lib/pinpoint_supported_countries.rb b/lib/pinpoint_supported_countries.rb index 8f3205041d6..6a9aa8fc6e3 100644 --- a/lib/pinpoint_supported_countries.rb +++ b/lib/pinpoint_supported_countries.rb @@ -6,8 +6,8 @@ # Scrapes HTML tables from Pinpoint help sites to parse out supported countries, and # puts them in a format compatible with country_dialing_codes.yml class PinpointSupportedCountries - PINPOINT_SMS_URL = 'https://docs.aws.amazon.com/pinpoint/latest/userguide/channels-sms-countries.html'.freeze - PINPOINT_VOICE_URL = 'https://docs.aws.amazon.com/pinpoint/latest/userguide/channels-voice-countries.html'.freeze + PINPOINT_SMS_URL = 'https://docs.aws.amazon.com/sms-voice/latest/userguide/phone-numbers-sms-by-country.html'.freeze + PINPOINT_VOICE_URL = 'https://docs.aws.amazon.com/sms-voice/latest/userguide/phone-numbers-voice-support-by-country.html'.freeze # The list of countries where we have our sender ID registered SENDER_ID_COUNTRIES = %w[ From 2a44fbe99690c50555179aaf7a9466108208f381 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 6 Feb 2024 15:46:55 -0600 Subject: [PATCH 04/16] Add support for configuring OIDC redirect by service provider (#10036) changelog: Internal, OpenID Connect, Add support for configuring OIDC redirect by service provider --- .../authorization_controller.rb | 18 ++++++---- .../openid_connect/logout_controller.rb | 19 ++++++---- config/application.yml.default | 1 + lib/identity_config.rb | 4 +++ .../authorization_controller_spec.rb | 36 +++++++++++++++++++ .../openid_connect/logout_controller_spec.rb | 24 +++++++++++++ 6 files changed, 89 insertions(+), 13 deletions(-) diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 7b2ca8d9cba..54ffb9e564f 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -87,6 +87,7 @@ def handle_successful_handoff redirect_user( @authorize_form.success_redirect_uri, + @authorize_form.service_provider.issuer, current_user.uuid, ) @@ -144,7 +145,7 @@ def pre_validate_authorize_form if redirect_uri.nil? render :error else - redirect_user(redirect_uri, current_user&.uuid) + redirect_user(redirect_uri, @authorize_form.service_provider.issuer, current_user&.uuid) end end @@ -203,11 +204,16 @@ def track_events track_billing_events end - def redirect_user(redirect_uri, user_uuid) - redirect_method = IdentityConfig.store.openid_connect_redirect_uuid_override_map.fetch( - user_uuid, - IdentityConfig.store.openid_connect_redirect, - ) + def redirect_user(redirect_uri, issuer, user_uuid) + user_redirect_method_override = + IdentityConfig.store.openid_connect_redirect_uuid_override_map[user_uuid] + + sp_redirect_method_override = + IdentityConfig.store.openid_connect_redirect_issuer_override_map[issuer] + + redirect_method = + user_redirect_method_override || sp_redirect_method_override || + IdentityConfig.store.openid_connect_redirect case redirect_method when 'client_side' diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index a488159319d..660b04a9467 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -31,7 +31,7 @@ def delete analytics.logout_initiated(**result.to_h.except(:redirect_uri)) irs_attempts_api_tracker.logout_initiated(success: result.success?) - redirect_user(redirect_uri, current_user&.uuid) + redirect_user(redirect_uri, @logout_form.service_provider&.issuer, current_user&.uuid) sign_out else render :error @@ -44,11 +44,16 @@ def set_devise_failure_redirect_for_concurrent_session_logout request.env['devise_session_limited_failure_redirect_url'] = request.url end - def redirect_user(redirect_uri, user_uuid) - redirect_method = IdentityConfig.store.openid_connect_redirect_uuid_override_map.fetch( - user_uuid, - IdentityConfig.store.openid_connect_redirect, - ) + def redirect_user(redirect_uri, issuer, user_uuid) + user_redirect_method_override = + IdentityConfig.store.openid_connect_redirect_uuid_override_map[user_uuid] + + sp_redirect_method_override = + IdentityConfig.store.openid_connect_redirect_issuer_override_map[issuer] + + redirect_method = + user_redirect_method_override || sp_redirect_method_override || + IdentityConfig.store.openid_connect_redirect case redirect_method when 'client_side' @@ -115,7 +120,7 @@ def handle_successful_logout_request(result, redirect_uri) sign_out - redirect_user(redirect_uri, current_user&.uuid) + redirect_user(redirect_uri, @logout_form.service_provider&.issuer, current_user&.uuid) end end diff --git a/config/application.yml.default b/config/application.yml.default index e599da246eb..966deeb26fb 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -214,6 +214,7 @@ mx_timeout: 3 openid_connect_redirect: client_side_js openid_connect_content_security_form_action_enabled: false openid_connect_redirect_uuid_override_map: '{}' +openid_connect_redirect_issuer_override_map: '{}' otp_delivery_blocklist_maxretry: 10 otp_valid_for: 10 otp_expiration_warning_seconds: 150 diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 894ae4ebd83..66cbe876517 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -331,6 +331,10 @@ def self.build_store(config_map) :openid_connect_redirect_uuid_override_map, type: :json, ) + config.add( + :openid_connect_redirect_issuer_override_map, + type: :json, + ) config.add(:openid_connect_content_security_form_action_enabled, type: :boolean) config.add(:otp_delivery_blocklist_findtime, type: :integer) config.add(:otp_delivery_blocklist_maxretry, type: :integer) diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 284024ef17a..9cbf7236de1 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -232,6 +232,42 @@ expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) end + it 'respects UUID redirect config when issuer config is also set' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect). + and_return('server_side') + allow(IdentityConfig.store).to receive(:openid_connect_redirect_issuer_override_map). + and_return({ service_provider.issuer => 'client_side' }) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_uuid_override_map). + and_return({ user.uuid => 'client_side_js' }) + + IdentityLinker.new(user, service_provider).link_identity(ial: 3) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + allow(controller).to receive(:pii_requested_but_locked?).and_return(false) + action + + expect(controller).to render_template('openid_connect/shared/redirect_js') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + + it 'respects issuer redirect config if UUID config is not set' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect). + and_return('server_side') + allow(IdentityConfig.store).to receive(:openid_connect_redirect_issuer_override_map). + and_return({ service_provider.issuer => 'client_side_js' }) + + IdentityLinker.new(user, service_provider).link_identity(ial: 3) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + allow(controller).to receive(:pii_requested_but_locked?).and_return(false) + action + + expect(controller).to render_template('openid_connect/shared/redirect_js') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + it 'redirects to the password capture url when pii is locked' do IdentityLinker.new(user, service_provider).link_identity(ial: 3) user.identities.last.update!( diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index f76e45e7ebb..1211dd4140c 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -128,6 +128,30 @@ expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) end + it 'respects UUID redirect config when issuer config is also set' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect). + and_return('server_side') + allow(IdentityConfig.store).to receive(:openid_connect_redirect_issuer_override_map). + and_return({ service_provider.issuer => 'client_side' }) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_uuid_override_map). + and_return({ user.uuid => 'client_side_js' }) + action + + expect(controller).to render_template('openid_connect/shared/redirect_js') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end + + it 'respects issuer redirect config if UUID config is not set' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect). + and_return('server_side') + allow(IdentityConfig.store).to receive(:openid_connect_redirect_issuer_override_map). + and_return({ service_provider.issuer => 'client_side_js' }) + action + + expect(controller).to render_template('openid_connect/shared/redirect_js') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end + it 'tracks events' do stub_analytics expect(@analytics).to receive(:track_event). From d520ea703a7b922e69b6caed45324e9c6a161919 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 6 Feb 2024 15:47:07 -0600 Subject: [PATCH 05/16] Include email_address_id when logging that an email was sent (#10048) changelog: Internal, Logging, Include email_address_id when logging that an email was sent --- app/mailers/user_mailer.rb | 6 +++++- app/services/analytics_events.rb | 4 +++- lib/email_delivery_observer.rb | 8 ++++++-- spec/mailers/user_mailer_spec.rb | 17 +++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 7f858170de6..d6be78e7d4d 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -46,7 +46,11 @@ def validate_user_and_email_address end def add_metadata - message.instance_variable_set(:@_metadata, { user: user, action: action_name }) + message.instance_variable_set( + :@_metadata, { + user: user, email_address: email_address, action: action_name + } + ) end def email_confirmation_instructions(token, request_id:, instructions:) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index d7e5e29ef14..563249ffb27 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -412,11 +412,13 @@ def email_language_visited # Logs after an email is sent # @param [String] action type of email being sent # @param [String, nil] ses_message_id AWS SES Message ID - def email_sent(action:, ses_message_id:, **extra) + # @param [Integer] email_address_id Database identifier for email address record + def email_sent(action:, ses_message_id:, email_address_id:, **extra) track_event( 'Email Sent', action: action, ses_message_id: ses_message_id, + email_address_id: email_address_id, **extra, ) end diff --git a/lib/email_delivery_observer.rb b/lib/email_delivery_observer.rb index eec394150e0..a2376bfc74b 100644 --- a/lib/email_delivery_observer.rb +++ b/lib/email_delivery_observer.rb @@ -2,8 +2,12 @@ class EmailDeliveryObserver def self.delivered_email(mail) metadata = mail.instance_variable_get(:@_metadata) || {} user = metadata[:user] || AnonymousUser.new + email_address_id = metadata[:email_address]&.id action = metadata[:action] - Analytics.new(user: user, request: nil, sp: nil, session: {}). - email_sent(action: action, ses_message_id: mail.header[:ses_message_id]&.value) + Analytics.new(user: user, request: nil, sp: nil, session: {}).email_sent( + action: action, + ses_message_id: mail.header[:ses_message_id]&.value, + email_address_id: email_address_id, + ) end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 69a9484cd80..0051fb5966c 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -531,6 +531,23 @@ def expect_email_body_to_have_help_and_contact_links expect(mail.html_part.body). to have_content(strip_tags(t('user_mailer.please_reset_password.call_to_action'))) end + + it 'logs email metadata to analytics' do + analytics = FakeAnalytics.new + allow(Analytics).to receive(:new).and_return(analytics) + allow(analytics).to receive(:track_event) + + user = create(:user) + email_address = user.email_addresses.first + mail = UserMailer.with(user: user, email_address: email_address).please_reset_password + mail.deliver_now + + expect(analytics). + to have_received(:track_event).with( + 'Email Sent', + action: 'please_reset_password', ses_message_id: nil, email_address_id: email_address.id, + ) + end end describe '#letter_reminder' do From fc8f1b3b31f92f45b085f88c3f15a045169487b4 Mon Sep 17 00:00:00 2001 From: eileen-nava <80347702+eileen-nava@users.noreply.github.com> Date: Tue, 6 Feb 2024 16:50:21 -0500 Subject: [PATCH 06/16] LG-12200: Remove selfie_success field from DocumentCaptureSessionResult (#10035) * remove selfie_success from document_capture_session_result * changelog: Internal, DocAuth, remove selfie_success field from DocumentCaptureSessionResult --- app/services/document_capture_session_result.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index c1bfac2a57c..bed8d68c638 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -11,12 +11,11 @@ :failed_selfie_image_fingerprints, :captured_at, :selfie_check_performed, - :doc_auth_success, :selfie_status, :selfie_success, + :doc_auth_success, :selfie_status, keyword_init: true, allowed_members: [:id, :success, :attention_with_barcode, :failed_front_image_fingerprints, :failed_back_image_fingerprints, :failed_selfie_image_fingerprints, - :captured_at, :selfie_check_performed, :doc_auth_success, :selfie_status, - :selfie_success] + :captured_at, :selfie_check_performed, :doc_auth_success, :selfie_status] ) do include DocAuth::SelfieConcern From a345632cbf7155896102d01b3d529ce6c927af51 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Tue, 6 Feb 2024 17:45:29 -0500 Subject: [PATCH 07/16] convert submission-status component to typescript (#10023) * convert submission-status component to typescript [skip changelog] --------- Co-authored-by: Brittany Greaner Co-authored-by: Charles Ferguson --- .eslintrc | 2 +- .../components/{submission-status.jsx => submission-status.tsx} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename app/javascript/packages/document-capture/components/{submission-status.jsx => submission-status.tsx} (100%) diff --git a/.eslintrc b/.eslintrc index 519066c9cad..dbb99d833cd 100644 --- a/.eslintrc +++ b/.eslintrc @@ -101,7 +101,7 @@ "app/javascript/packages/document-capture/components/status-message.jsx", "app/javascript/packages/document-capture/components/submission-complete.tsx", "app/javascript/packages/document-capture/components/submission-interstitial.jsx", - "app/javascript/packages/document-capture/components/submission-status.jsx", + "app/javascript/packages/document-capture/components/submission-status.tsx", "app/javascript/packages/document-capture/components/submission.jsx", "app/javascript/packages/document-capture/components/suspense-error-boundary.jsx", "app/javascript/packages/document-capture/components/tip-list.tsx", diff --git a/app/javascript/packages/document-capture/components/submission-status.jsx b/app/javascript/packages/document-capture/components/submission-status.tsx similarity index 100% rename from app/javascript/packages/document-capture/components/submission-status.jsx rename to app/javascript/packages/document-capture/components/submission-status.tsx From 258eeeee275beafa6ade4427661b353f3f992de4 Mon Sep 17 00:00:00 2001 From: dawei-nava <130466753+dawei-nava@users.noreply.github.com> Date: Tue, 6 Feb 2024 19:56:27 -0500 Subject: [PATCH 08/16] LG-12039 read auth result (#9990) * LG-12039: use new fields to define success. changelog: Internal, Doc Auth, Use available new fields for success decision * LG-12039: doc_auth_access should it mean overall success? * LG-12039: pull in changes. * LG-12039: pull in changes. * LG-12039: it's questionable to use concern to reduce code complexity. More appropriate to user helper module. * LG-12039: doc_auth_success already checked attention_with_barcode. * LG-12039: refactor, make it more explicit for status check with both old and new approach. * LG-12039: change based on comments. * LG-12039: removing the props. --- .../lexis_nexis/responses/true_id_response.rb | 31 +++++--- .../document_capture_session_result.rb | 7 +- .../idv/capture_doc_status_controller_spec.rb | 1 - .../document_capture_controller_spec.rb | 3 +- .../document_capture_session_result_spec.rb | 74 +++++++++++++++++++ 5 files changed, 102 insertions(+), 14 deletions(-) diff --git a/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb b/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb index f23cd0b65b9..bdfbc98da61 100644 --- a/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb +++ b/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb @@ -32,10 +32,29 @@ def initialize(http_response, config, liveness_checking_enabled = false) ) end + ## returns full check success status, considering all checks: + # vendor (document and selfie if requested) + # document type + # bar code attention def successful_result? (all_passed? || attention_with_barcode?) && id_type_supported? end + # all checks from document perspectives, without considering selfie: + # vendor (document only) + # document_type + # bar code attention + def doc_auth_success? + # really it's everything else excluding selfie + ((transaction_status_passed? && + true_id_product.present? && + product_status_passed? && + doc_auth_result_passed? + ) || + attention_with_barcode? + ) && id_type_supported? + end + def error_messages return {} if successful_result? @@ -77,17 +96,6 @@ def billed? !!doc_auth_result end - def doc_auth_success? - # really it's everything else excluding selfie - ((transaction_status_passed? && - true_id_product.present? && - product_status_passed? && - doc_auth_result_passed? - ) || - attention_with_barcode? - ) && id_type_supported? - end - # @return [:success, :fail, :not_processed] # When selfie result is missing, return :not_processed # Otherwise: @@ -185,6 +193,7 @@ def basic_logging_info } end + # Status of all checks from Vendor perspective def all_passed? transaction_status_passed? && true_id_product.present? && diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index bed8d68c638..a574750983d 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -27,7 +27,12 @@ def selfie_status self[:selfie_status].to_sym end - alias_method :success?, :success + def success_status + # doc_auth_success : including document, attention_with_barcode and id type verification + (doc_auth_success && selfie_status != :fail) || success + end + + alias_method :success?, :success_status alias_method :attention_with_barcode?, :attention_with_barcode alias_method :pii_from_doc, :pii diff --git a/spec/controllers/idv/capture_doc_status_controller_spec.rb b/spec/controllers/idv/capture_doc_status_controller_spec.rb index f4a2885ff31..4ba03135af9 100644 --- a/spec/controllers/idv/capture_doc_status_controller_spec.rb +++ b/spec/controllers/idv/capture_doc_status_controller_spec.rb @@ -2,7 +2,6 @@ RSpec.describe Idv::CaptureDocStatusController do let(:user) { build(:user) } - let(:doc_auth_response) do DocAuth::Response.new( success: true, diff --git a/spec/controllers/idv/hybrid_mobile/document_capture_controller_spec.rb b/spec/controllers/idv/hybrid_mobile/document_capture_controller_spec.rb index 7a6f99e6e67..f1774b9e0f3 100644 --- a/spec/controllers/idv/hybrid_mobile/document_capture_controller_spec.rb +++ b/spec/controllers/idv/hybrid_mobile/document_capture_controller_spec.rb @@ -257,12 +257,13 @@ def stub_document_capture_session_result allow_any_instance_of(DocumentCaptureSession).to receive(:load_result).and_return( DocumentCaptureSessionResult.new( id: 1234, - success: document_capture_session_result_success, pii: { state: 'WA', }, attention_with_barcode: true, captured_at: document_capture_session_result_captured_at, + doc_auth_success: document_capture_session_result_success, + selfie_status: document_capture_session_result_success ? :success : :fail, ), ) end diff --git a/spec/services/document_capture_session_result_spec.rb b/spec/services/document_capture_session_result_spec.rb index b2c1770c464..8ec3cccb915 100644 --- a/spec/services/document_capture_session_result_spec.rb +++ b/spec/services/document_capture_session_result_spec.rb @@ -10,6 +10,8 @@ result = DocumentCaptureSessionResult.new( id: id, success: success, + doc_auth_success: success, + selfie_status: :success, pii: pii, attention_with_barcode: false, ) @@ -20,6 +22,8 @@ expect(loaded_result.success?).to eq(success) expect(loaded_result.pii).to eq(pii.deep_symbolize_keys) expect(loaded_result.attention_with_barcode?).to eq(false) + expect(loaded_result.selfie_status).to eq(:success) + expect(loaded_result.doc_auth_success).to eq(true) end it 'add fingerprint with EncryptedRedisStructStorage' do result = DocumentCaptureSessionResult.new( @@ -47,5 +51,75 @@ expect(result.selfie_status).to be_an_instance_of(Symbol) end end + + describe '#success?' do + it 'reports true when doc_auth_success is true and selfie_status is :not_processed' do + result = DocumentCaptureSessionResult.new( + id: id, + success: false, + pii: pii, + attention_with_barcode: false, + selfie_status: :not_processed, + doc_auth_success: true, + ) + expect(result.success?).to eq(true) + end + it 'reports correctly from success when missing doc_auth_success and selfie_status' do + result = DocumentCaptureSessionResult.new( + id: id, + success: true, + pii: pii, + attention_with_barcode: false, + ) + expect(result.success?).to eq(true) + end + it 'reports failure when selfie_status is :fail' do + result = DocumentCaptureSessionResult.new( + id: id, + success: false, + pii: pii, + attention_with_barcode: false, + selfie_status: :fail, + doc_auth_success: true, + ) + expect(result.success?).to eq(false) + end + + it 'reports failure when doc_auth_success is false' do + result = DocumentCaptureSessionResult.new( + id: id, + success: false, + pii: pii, + attention_with_barcode: false, + selfie_status: :success, + doc_auth_success: false, + ) + expect(result.success?).to eq(false) + end + + describe 'when success field, doc_auth_success, and selfie_status conflict' do + it 'reports correct result' do + result = DocumentCaptureSessionResult.new( + id: id, + success: false, + pii: pii, + attention_with_barcode: false, + selfie_status: :not_processed, + doc_auth_success: true, + ) + expect(result.success?).to eq(true) + + result = DocumentCaptureSessionResult.new( + id: id, + success: true, + pii: pii, + attention_with_barcode: false, + selfie_status: :fail, + doc_auth_success: true, + ) + expect(result.success?).to eq(true) + end + end + end end end From 10c2b65f0fc5ce596b522d85cd8b08d8adc52f5e Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 7 Feb 2024 09:43:18 -0500 Subject: [PATCH 09/16] LG-12249: Log sign-in flow for SP redirect analytics event (#10029) * Log sign-in flow for SP redirect analytics event changelog: Internal, Analytics, Add analytics differentiating sign-in flow for completed sign-in * Document nil-able sign_in_flow parameter * Update additional SAML controller specs --- .../authorization_controller.rb | 1 + app/controllers/saml_idp_controller.rb | 1 + .../sign_up/registrations_controller.rb | 1 + app/controllers/users/sessions_controller.rb | 1 + app/services/analytics_events.rb | 8 ++-- .../authorization_controller_spec.rb | 29 +++++++----- spec/controllers/saml_idp_controller_spec.rb | 44 +++++++++++++------ spec/features/users/sign_up_spec.rb | 16 +++++++ spec/support/shared_examples/sign_in.rb | 21 +++++++++ 9 files changed, 93 insertions(+), 29 deletions(-) diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 54ffb9e564f..feb12eafccd 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -200,6 +200,7 @@ def track_events analytics.sp_redirect_initiated( ial: event_ial_context.ial, billed_ial: event_ial_context.bill_for_ial_1_or_2, + sign_in_flow: session[:sign_in_flow], ) track_billing_events end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index ce5d5190848..5c539f59a86 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -175,6 +175,7 @@ def track_events analytics.sp_redirect_initiated( ial: ial_context.ial, billed_ial: ial_context.bill_for_ial_1_or_2, + sign_in_flow: session[:sign_in_flow], ) track_billing_events end diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 588e8c989bc..1052ed33f7f 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -57,6 +57,7 @@ def process_successful_creation resend_confirmation = params[:user][:resend] session[:email] = @register_user_email_form.email + session[:sign_in_flow] = :create_account redirect_to sign_up_verify_email_url(resend: resend_confirmation) end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 651c8258c8d..68eecf7f08d 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -32,6 +32,7 @@ def new end def create + session[:sign_in_flow] = :sign_in return process_locked_out_session if session_bad_password_count_max_exceeded? return process_locked_out_user if current_user && user_locked_out?(current_user) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 563249ffb27..a588054d5ec 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4432,11 +4432,13 @@ def sp_inactive_visit # Tracks when a user is redirected back to the service provider # @param [Integer] ial # @param [Integer] billed_ial - def sp_redirect_initiated(ial:, billed_ial:, **extra) + # @param [String, nil] sign_in_flow + def sp_redirect_initiated(ial:, billed_ial:, sign_in_flow:, **extra) track_event( 'SP redirect initiated', - ial: ial, - billed_ial: billed_ial, + ial:, + billed_ial:, + sign_in_flow:, **extra, ) end diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 9cbf7236de1..adaeb2a1ebf 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -43,8 +43,10 @@ context 'user is signed in' do let(:user) { create(:user, :fully_registered) } + let(:sign_in_flow) { :sign_in } before do stub_sign_in user + session[:sign_in_flow] = sign_in_flow end context 'with valid params' do @@ -123,6 +125,7 @@ 'SP redirect initiated', ial: 1, billed_ial: 1, + sign_in_flow:, ) IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -307,6 +310,7 @@ 'SP redirect initiated', ial: 2, billed_ial: 2, + sign_in_flow:, ) IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -546,6 +550,7 @@ 'SP redirect initiated', ial: 0, billed_ial: 2, + sign_in_flow:, ) IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -625,12 +630,12 @@ client_id: client_id, user_sp_authorized: true, code_digest: kind_of(String)) - expect(@analytics).to receive(:track_event). - with( - 'SP redirect initiated', - ial: 0, - billed_ial: 1, - ) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: 0, + billed_ial: 1, + sign_in_flow:, + ) IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!( @@ -711,12 +716,12 @@ client_id: client_id, user_sp_authorized: true, code_digest: kind_of(String)) - expect(@analytics).to receive(:track_event). - with( - 'SP redirect initiated', - ial: 0, - billed_ial: 1, - ) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: 0, + billed_ial: 1, + sign_in_flow:, + ) IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!( diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index d07d0858dfa..eb92b202d3d 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -579,9 +579,11 @@ def name_id_version(format_urn) user_session: {}, ) end + let(:sign_in_flow) { :sign_in } before do stub_sign_in(user) + session[:sign_in_flow] = sign_in_flow IdentityLinker.new(user, sp1).link_identity(ial: ial) user.identities.last.update!( verified_attributes: %w[given_name family_name social_security_number address], @@ -652,11 +654,12 @@ def name_id_version(format_urn) request_signed: true, matching_cert_serial: saml_test_sp_cert_serial, }) - expect(@analytics).to receive(:track_event). - with('SP redirect initiated', { - ial: ial, - billed_ial: [ial, 2].min, - }) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: ial, + billed_ial: [ial, 2].min, + sign_in_flow:, + ) allow(controller).to receive(:identity_needs_verification?).and_return(false) saml_get_auth(ial2_settings) @@ -727,9 +730,11 @@ def name_id_version(format_urn) user_session: {}, ) end + let(:sign_in_flow) { :sign_in } before do stub_sign_in(user) + session[:sign_in_flow] = sign_in_flow IdentityLinker.new(user, ServiceProvider.find_by(issuer: sp1_issuer)).link_identity(ial: 2) user.identities.last.update!( verified_attributes: %w[email given_name family_name social_security_number address], @@ -800,8 +805,12 @@ def name_id_version(format_urn) request_signed: true, matching_cert_serial: saml_test_sp_cert_serial, }) - expect(@analytics).to receive(:track_event). - with('SP redirect initiated', { ial: 0, billed_ial: 2 }) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: 0, + billed_ial: 2, + sign_in_flow:, + ) allow(controller).to receive(:identity_needs_verification?).and_return(false) saml_get_auth(ialmax_settings) @@ -2217,6 +2226,7 @@ def stub_requested_attributes user = create(:user, :fully_registered) stub_analytics + session[:sign_in_flow] = :sign_in allow(controller).to receive(:identity_needs_verification?).and_return(false) analytics_hash = { @@ -2243,8 +2253,12 @@ def stub_requested_attributes user_fully_authenticated: true, }) expect(@analytics).to receive(:track_event).with('SAML Auth', analytics_hash) - expect(@analytics).to receive(:track_event). - with('SP redirect initiated', { ial: 1, billed_ial: 1 }) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: 1, + billed_ial: 1, + sign_in_flow: :sign_in, + ) generate_saml_response(user) end @@ -2255,6 +2269,7 @@ def stub_requested_attributes user = create(:user, :fully_registered) stub_analytics + session[:sign_in_flow] = :sign_in allow(controller).to receive(:identity_needs_verification?).and_return(false) allow(controller).to receive(:user_has_pending_profile?).and_return(true) @@ -2282,11 +2297,12 @@ def stub_requested_attributes user_fully_authenticated: true, }) expect(@analytics).to receive(:track_event).with('SAML Auth', analytics_hash) - expect(@analytics).to receive(:track_event). - with('SP redirect initiated', { - ial: 1, - billed_ial: 1, - }) + expect(@analytics).to receive(:track_event).with( + 'SP redirect initiated', + ial: 1, + billed_ial: 1, + sign_in_flow: :sign_in, + ) generate_saml_response(user) end diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 8df50751872..021cb9f5ebe 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -477,6 +477,22 @@ def clipboard_text expect(page).to have_current_path phone_setup_path end + it 'logs expected analytics events for end-to-end sign-up' do + analytics = FakeAnalytics.new + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) + + visit_idp_from_sp_with_ial1(:oidc) + register_user + click_agree_and_continue + + expect(analytics).to have_logged_event( + 'SP redirect initiated', + ial: 1, + billed_ial: 1, + sign_in_flow: 'create_account', + ) + end + describe 'visiting the homepage by clicking the logo image' do context 'on the password confirmation screen' do before do diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index fd875aae6ae..ed3d4761418 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -30,6 +30,27 @@ expect(page).to_not have_content t('devise.failure.timeout') end + + it 'logs expected analytics events' do + user = create(:user, :fully_registered) + analytics = FakeAnalytics.new(user:) + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) + + visit_idp_from_sp_with_ial1(sp) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + click_submit_default if current_path == complete_saml_path + click_agree_and_continue + click_submit_default if current_path == complete_saml_path + + expect(analytics).to have_logged_event( + 'SP redirect initiated', + ial: 1, + billed_ial: 1, + sign_in_flow: 'sign_in', + ) + end end RSpec.shared_examples 'signing in as IAL1 with personal key' do |sp| From c50e219fcaa6d6c7171d68d3d6324c6ef712a0f1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:11:54 -0500 Subject: [PATCH 10/16] Add sideEffects directive to all package.json (#9961) * Add sideEffects to all packages * Enforce sideEffects for all packages changelog: Internal, Performance, Reduce size of JavaScript by removing unreachable code * Mark sideEffects in rails-i18n-webpack-plugin specs * Add guidance for missing required fields * Improve messaging for custom elements sideEffects validation * Update documentation references for sideEffects --- Makefile | 2 +- app/javascript/packages/README.md | 23 +--------- .../packages/address-search/package.json | 3 +- .../packages/analytics/package.json | 5 ++- app/javascript/packages/assets/package.json | 3 +- .../packages/build-sass/package.json | 3 +- .../captcha-submit-button/package.json | 5 ++- .../packages/clipboard-button/package.json | 5 ++- .../packages/components/package.json | 3 +- .../packages/compose-components/package.json | 3 +- app/javascript/packages/config/package.json | 3 +- .../packages/countdown/package.json | 6 ++- app/javascript/packages/device/package.json | 3 +- .../document-capture-polling/package.json | 3 +- .../packages/document-capture/package.json | 3 +- .../packages/eslint-plugin/package.json | 3 +- .../packages/form-steps/package.json | 3 +- app/javascript/packages/i18n/package.json | 3 +- .../manageable-authenticator/package.json | 5 ++- .../packages/masked-text-toggle/package.json | 3 +- .../packages/memorable-date/package.json | 7 ++- app/javascript/packages/modal/package.json | 5 ++- .../packages/normalize-yaml/package.json | 3 +- .../packages/one-time-code-input/package.json | 5 ++- .../password-confirmation/package.json | 5 ++- .../packages/password-strength/package.json | 5 ++- .../packages/password-toggle/package.json | 5 ++- .../packages/phone-input/package.json | 5 ++- .../packages/print-button/package.json | 5 ++- .../rails-i18n-webpack-plugin/package.json | 5 ++- .../packages/react-hooks/package.json | 3 +- .../packages/react-i18n/package.json | 3 +- app/javascript/packages/request/package.json | 3 +- app/javascript/packages/session/package.json | 3 +- .../packages/spinner-button/package.json | 5 ++- .../packages/step-indicator/package.json | 5 ++- .../packages/stylelint-config/package.json | 3 +- .../packages/submit-button/package.json | 5 ++- .../packages/test-helpers/package.json | 3 +- .../packages/time-element/package.json | 5 ++- .../unpolyfill-webpack-plugin/package.json | 3 +- app/javascript/packages/url/package.json | 3 +- .../packages/validated-field/package.json | 5 ++- .../packages/verify-flow/package.json | 3 +- app/javascript/packages/webauthn/package.json | 6 ++- docs/frontend.md | 1 + ...-workspaces.js => validate-workspaces.mjs} | 45 ++++++++++++++++--- 47 files changed, 171 insertions(+), 73 deletions(-) rename scripts/{validate-workspaces.js => validate-workspaces.mjs} (70%) diff --git a/Makefile b/Makefile index 3f37ac5aaea..ee1c06d68c3 100644 --- a/Makefile +++ b/Makefile @@ -117,7 +117,7 @@ lint_yaml: normalize_yaml ## Lints YAML files (! git diff --name-only | grep "^config/.*\.yml$$") || (echo "Error: Run 'make normalize_yaml' to normalize YAML"; exit 1) lint_yarn_workspaces: ## Lints Yarn workspace packages - scripts/validate-workspaces.js + scripts/validate-workspaces.mjs lint_asset_bundle_size: ## Lints JavaScript and CSS compiled bundle size @# This enforces an asset size budget to ensure that download sizes are reasonable and to protect diff --git a/app/javascript/packages/README.md b/app/javascript/packages/README.md index e2d205b0586..a649bd7c8a1 100644 --- a/app/javascript/packages/README.md +++ b/app/javascript/packages/README.md @@ -2,25 +2,4 @@ Packages are independent JavaScript libraries. As much as possible, their behaviors should be reusable and make no assumptions about particular application pages or configuration. Instead, they should be initialized by a [`pack`](../packs) which provides relevant configuration and page element references. -A package should behave much like any other third-party [NPM package](https://www.npmjs.com/), where each folder in this directory represents a single package. These packages are managed using [Yarn workspaces](https://classic.yarnpkg.com/lang/en/docs/workspaces/). - -Each package should include a `package.json` with at least `name`, `version`, and `private` fields: - -- Name should start with `@18f/identity-` and end with the folder name -- Version should be fixed to `1.0.0` -- Packages should be private, since they are not published -- Any `devDependencies` should be defined in the root project directory, not in a package -- Define `dependencies` for any third-party libraries used in a package - - It is not necessary to define `dependencies` for other sibling packages - -**Example:** - -_packages/analytics/package.json_ - -```json -{ - "name": "@18f/identity-analytics", - "version": "1.0.0", - "private": true -} -``` +A package should behave much like any other third-party [NPM package](https://www.npmjs.com/), where each folder in this directory represents a single package. These packages are managed using [Yarn workspaces](https://classic.yarnpkg.com/lang/en/docs/workspaces/). Refer to [Front-end Architecture documentation on Yarn Workspaces](https://github.com/18F/identity-idp/blob/main/docs/frontend.md#yarn-workspaces) for more information. diff --git a/app/javascript/packages/address-search/package.json b/app/javascript/packages/address-search/package.json index 33541701e7d..4ab55f98a1b 100644 --- a/app/javascript/packages/address-search/package.json +++ b/app/javascript/packages/address-search/package.json @@ -32,5 +32,6 @@ "type": "git", "url": "https://github.com/18f/identity-idp.git", "directory": "app/javascript/packages/address-search" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/analytics/package.json b/app/javascript/packages/analytics/package.json index 6ade5aa72c0..e13018b2865 100644 --- a/app/javascript/packages/analytics/package.json +++ b/app/javascript/packages/analytics/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-analytics", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": [ + "./click-observer-element.ts" + ] } diff --git a/app/javascript/packages/assets/package.json b/app/javascript/packages/assets/package.json index e39802f92f9..23ac44a1bbe 100644 --- a/app/javascript/packages/assets/package.json +++ b/app/javascript/packages/assets/package.json @@ -4,5 +4,6 @@ "version": "1.0.0", "peerDependencies": { "webpack": ">=5" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/build-sass/package.json b/app/javascript/packages/build-sass/package.json index 27fb9961242..2941625c096 100644 --- a/app/javascript/packages/build-sass/package.json +++ b/app/javascript/packages/build-sass/package.json @@ -32,5 +32,6 @@ "chokidar": "^3.5.3", "lightningcss": "^1.23.0", "sass-embedded": "^1.70.0" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/captcha-submit-button/package.json b/app/javascript/packages/captcha-submit-button/package.json index dd41558f931..d4f99869900 100644 --- a/app/javascript/packages/captcha-submit-button/package.json +++ b/app/javascript/packages/captcha-submit-button/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-captcha-submit-button", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": [ + "./captcha-submit-button-element.ts" + ] } diff --git a/app/javascript/packages/clipboard-button/package.json b/app/javascript/packages/clipboard-button/package.json index 8b468540391..31a34e15293 100644 --- a/app/javascript/packages/clipboard-button/package.json +++ b/app/javascript/packages/clipboard-button/package.json @@ -4,5 +4,8 @@ "private": true, "dependencies": { "@18f/identity-design-system": "^8.1.2" - } + }, + "sideEffects": [ + "./clipboard-button-element.ts" + ] } diff --git a/app/javascript/packages/components/package.json b/app/javascript/packages/components/package.json index 64ca0dfd35d..b1f4e42250b 100644 --- a/app/javascript/packages/components/package.json +++ b/app/javascript/packages/components/package.json @@ -32,5 +32,6 @@ "type": "git", "url": "https://github.com/18f/identity-idp.git", "directory": "app/javascript/packages/components" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/compose-components/package.json b/app/javascript/packages/compose-components/package.json index 8ec82d11652..29fcd9d2b79 100644 --- a/app/javascript/packages/compose-components/package.json +++ b/app/javascript/packages/compose-components/package.json @@ -4,5 +4,6 @@ "version": "1.0.0", "dependencies": { "react": "^17.0.2" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/config/package.json b/app/javascript/packages/config/package.json index c9af12df0b3..3afb193f3c8 100644 --- a/app/javascript/packages/config/package.json +++ b/app/javascript/packages/config/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-config", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": false } diff --git a/app/javascript/packages/countdown/package.json b/app/javascript/packages/countdown/package.json index 1bc751a6fc5..82526ba1386 100644 --- a/app/javascript/packages/countdown/package.json +++ b/app/javascript/packages/countdown/package.json @@ -1,5 +1,9 @@ { "name": "@18f/identity-countdown", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": [ + "./countdown-alert-element.ts", + "./countdown-element.ts" + ] } diff --git a/app/javascript/packages/device/package.json b/app/javascript/packages/device/package.json index c7d769e402f..0bb8990314e 100644 --- a/app/javascript/packages/device/package.json +++ b/app/javascript/packages/device/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-device", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": false } diff --git a/app/javascript/packages/document-capture-polling/package.json b/app/javascript/packages/document-capture-polling/package.json index 22cd45db4b9..477951beab3 100644 --- a/app/javascript/packages/document-capture-polling/package.json +++ b/app/javascript/packages/document-capture-polling/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-document-capture-polling", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": false } diff --git a/app/javascript/packages/document-capture/package.json b/app/javascript/packages/document-capture/package.json index 76877772573..0c1ca180991 100644 --- a/app/javascript/packages/document-capture/package.json +++ b/app/javascript/packages/document-capture/package.json @@ -6,5 +6,6 @@ "react": "^17.0.2", "react-dom": "^17.0.2", "swr": "^2.0.0" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/eslint-plugin/package.json b/app/javascript/packages/eslint-plugin/package.json index c9f3cdc5f20..1cb9c0e98b9 100644 --- a/app/javascript/packages/eslint-plugin/package.json +++ b/app/javascript/packages/eslint-plugin/package.json @@ -56,5 +56,6 @@ "eslint-plugin-react-hooks": { "optional": true } - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/form-steps/package.json b/app/javascript/packages/form-steps/package.json index 74a60e8e1b2..57dee9a85b8 100644 --- a/app/javascript/packages/form-steps/package.json +++ b/app/javascript/packages/form-steps/package.json @@ -4,5 +4,6 @@ "private": true, "peerDependencies": { "react": "*" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/i18n/package.json b/app/javascript/packages/i18n/package.json index 0b07c95f6f9..2af4ad53947 100644 --- a/app/javascript/packages/i18n/package.json +++ b/app/javascript/packages/i18n/package.json @@ -24,5 +24,6 @@ "type": "git", "url": "https://github.com/18f/identity-idp.git", "directory": "app/javascript/packages/i18n" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/manageable-authenticator/package.json b/app/javascript/packages/manageable-authenticator/package.json index 9d02f272dbe..1c50a094e50 100644 --- a/app/javascript/packages/manageable-authenticator/package.json +++ b/app/javascript/packages/manageable-authenticator/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-manageable-authenticator", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": [ + "./manageable-authenticator-element.ts" + ] } diff --git a/app/javascript/packages/masked-text-toggle/package.json b/app/javascript/packages/masked-text-toggle/package.json index 6c94e97790c..03a7125541a 100644 --- a/app/javascript/packages/masked-text-toggle/package.json +++ b/app/javascript/packages/masked-text-toggle/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-masked-text-toggle", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": false } diff --git a/app/javascript/packages/memorable-date/package.json b/app/javascript/packages/memorable-date/package.json index 9a579ea5612..8f2bf2015a5 100644 --- a/app/javascript/packages/memorable-date/package.json +++ b/app/javascript/packages/memorable-date/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-memorable-date", "private": true, - "version": "1.0.0" -} \ No newline at end of file + "version": "1.0.0", + "sideEffects": [ + "./index.ts" + ] +} diff --git a/app/javascript/packages/modal/package.json b/app/javascript/packages/modal/package.json index e8c917ab013..abef112c26b 100644 --- a/app/javascript/packages/modal/package.json +++ b/app/javascript/packages/modal/package.json @@ -4,5 +4,8 @@ "private": true, "dependencies": { "focus-trap": "^6.7.1" - } + }, + "sideEffects": [ + "./modal-element.ts" + ] } diff --git a/app/javascript/packages/normalize-yaml/package.json b/app/javascript/packages/normalize-yaml/package.json index e9f55bb3c47..c6d186d3aab 100644 --- a/app/javascript/packages/normalize-yaml/package.json +++ b/app/javascript/packages/normalize-yaml/package.json @@ -39,5 +39,6 @@ }, "peerDependencies": { "prettier": ">=3" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/one-time-code-input/package.json b/app/javascript/packages/one-time-code-input/package.json index f35d827a064..dc95624fb4a 100644 --- a/app/javascript/packages/one-time-code-input/package.json +++ b/app/javascript/packages/one-time-code-input/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-one-time-code-input", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": [ + "./one-time-code-input-element.ts" + ] } diff --git a/app/javascript/packages/password-confirmation/package.json b/app/javascript/packages/password-confirmation/package.json index 3e52bad9500..fe13b63efae 100644 --- a/app/javascript/packages/password-confirmation/package.json +++ b/app/javascript/packages/password-confirmation/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-password-confirmation", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": [ + "./password-confirmation-element.ts" + ] } diff --git a/app/javascript/packages/password-strength/package.json b/app/javascript/packages/password-strength/package.json index 0b364ce0299..8e38faea13f 100644 --- a/app/javascript/packages/password-strength/package.json +++ b/app/javascript/packages/password-strength/package.json @@ -4,5 +4,8 @@ "version": "1.0.0", "dependencies": { "zxcvbn": "^4.4.2" - } + }, + "sideEffects": [ + "./password-strength-element.ts" + ] } diff --git a/app/javascript/packages/password-toggle/package.json b/app/javascript/packages/password-toggle/package.json index a3daa28083e..555f2f92b4f 100644 --- a/app/javascript/packages/password-toggle/package.json +++ b/app/javascript/packages/password-toggle/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-password-toggle", "private": true, - "version": "1.0.0" + "version": "1.0.0", + "sideEffects": [ + "./password-toggle-element.ts" + ] } diff --git a/app/javascript/packages/phone-input/package.json b/app/javascript/packages/phone-input/package.json index 56504a875cd..0ee0ba81331 100644 --- a/app/javascript/packages/phone-input/package.json +++ b/app/javascript/packages/phone-input/package.json @@ -5,5 +5,8 @@ "dependencies": { "intl-tel-input": "^17.0.19", "libphonenumber-js": "^1.10.55" - } + }, + "sideEffects": [ + "./index.ts" + ] } diff --git a/app/javascript/packages/print-button/package.json b/app/javascript/packages/print-button/package.json index 72eebb02604..b42b3597fb9 100644 --- a/app/javascript/packages/print-button/package.json +++ b/app/javascript/packages/print-button/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-print-button", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": [ + "./print-button-element.ts" + ] } diff --git a/app/javascript/packages/rails-i18n-webpack-plugin/package.json b/app/javascript/packages/rails-i18n-webpack-plugin/package.json index 948ecbe20af..dbb6f305731 100644 --- a/app/javascript/packages/rails-i18n-webpack-plugin/package.json +++ b/app/javascript/packages/rails-i18n-webpack-plugin/package.json @@ -10,5 +10,8 @@ "peerDependencies": { "sinon": "*", "webpack": ">=5" - } + }, + "sideEffects": [ + "./spec/**" + ] } diff --git a/app/javascript/packages/react-hooks/package.json b/app/javascript/packages/react-hooks/package.json index 0c7f4ca36a9..f014e531868 100644 --- a/app/javascript/packages/react-hooks/package.json +++ b/app/javascript/packages/react-hooks/package.json @@ -4,5 +4,6 @@ "version": "1.0.0", "peerDependencies": { "react": ">=16.8.0" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/react-i18n/package.json b/app/javascript/packages/react-i18n/package.json index 75371b668c6..5622f922eb6 100644 --- a/app/javascript/packages/react-i18n/package.json +++ b/app/javascript/packages/react-i18n/package.json @@ -4,5 +4,6 @@ "version": "1.0.0", "dependencies": { "react": "^17.0.2" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/request/package.json b/app/javascript/packages/request/package.json index 1a949d73073..dbc8dec4c97 100644 --- a/app/javascript/packages/request/package.json +++ b/app/javascript/packages/request/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-request", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": false } diff --git a/app/javascript/packages/session/package.json b/app/javascript/packages/session/package.json index 0005aa6a9b0..8790dfea903 100644 --- a/app/javascript/packages/session/package.json +++ b/app/javascript/packages/session/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-session", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": false } diff --git a/app/javascript/packages/spinner-button/package.json b/app/javascript/packages/spinner-button/package.json index 8d32bb8b494..b608938c6cc 100644 --- a/app/javascript/packages/spinner-button/package.json +++ b/app/javascript/packages/spinner-button/package.json @@ -9,5 +9,8 @@ "react": { "optional": true } - } + }, + "sideEffects": [ + "./spinner-button-element.ts" + ] } diff --git a/app/javascript/packages/step-indicator/package.json b/app/javascript/packages/step-indicator/package.json index c4462f305d6..bb2c3f36406 100644 --- a/app/javascript/packages/step-indicator/package.json +++ b/app/javascript/packages/step-indicator/package.json @@ -9,5 +9,8 @@ "react": { "optional": true } - } + }, + "sideEffects": [ + "./step-indicator-element.ts" + ] } diff --git a/app/javascript/packages/stylelint-config/package.json b/app/javascript/packages/stylelint-config/package.json index c8a634b13c4..62cefac4729 100644 --- a/app/javascript/packages/stylelint-config/package.json +++ b/app/javascript/packages/stylelint-config/package.json @@ -23,5 +23,6 @@ "peerDependencies": { "prettier": ">=3.0.0", "stylelint": ">=15.10.0" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/submit-button/package.json b/app/javascript/packages/submit-button/package.json index a8ce01b62ae..a99425a4a23 100644 --- a/app/javascript/packages/submit-button/package.json +++ b/app/javascript/packages/submit-button/package.json @@ -1,5 +1,8 @@ { "name": "@18f/identity-submit-button", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": [ + "./submit-button-element.ts" + ] } diff --git a/app/javascript/packages/test-helpers/package.json b/app/javascript/packages/test-helpers/package.json index f461143fe62..3c3784aa636 100644 --- a/app/javascript/packages/test-helpers/package.json +++ b/app/javascript/packages/test-helpers/package.json @@ -4,5 +4,6 @@ "version": "1.0.0", "peerDependencies": { "sinon": "*" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/time-element/package.json b/app/javascript/packages/time-element/package.json index 1e3a3231601..830da5124f2 100644 --- a/app/javascript/packages/time-element/package.json +++ b/app/javascript/packages/time-element/package.json @@ -2,5 +2,8 @@ "name": "@18f/identity-time-element", "private": true, "version": "1.0.0", - "type": "module" + "type": "module", + "sideEffects": [ + "./time-element.ts" + ] } diff --git a/app/javascript/packages/unpolyfill-webpack-plugin/package.json b/app/javascript/packages/unpolyfill-webpack-plugin/package.json index 919749936e2..3c6086c75f6 100644 --- a/app/javascript/packages/unpolyfill-webpack-plugin/package.json +++ b/app/javascript/packages/unpolyfill-webpack-plugin/package.json @@ -11,5 +11,6 @@ }, "peerDependencies": { "webpack": ">=5" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/url/package.json b/app/javascript/packages/url/package.json index 38deefef5de..4fbadaf76e8 100644 --- a/app/javascript/packages/url/package.json +++ b/app/javascript/packages/url/package.json @@ -1,5 +1,6 @@ { "name": "@18f/identity-url", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": false } diff --git a/app/javascript/packages/validated-field/package.json b/app/javascript/packages/validated-field/package.json index b7f3f634e5d..56888c3cc41 100644 --- a/app/javascript/packages/validated-field/package.json +++ b/app/javascript/packages/validated-field/package.json @@ -9,5 +9,8 @@ "react": { "optional": true } - } + }, + "sideEffects": [ + "./validated-field-element.ts" + ] } diff --git a/app/javascript/packages/verify-flow/package.json b/app/javascript/packages/verify-flow/package.json index f059daef1dc..2692e3a10ed 100644 --- a/app/javascript/packages/verify-flow/package.json +++ b/app/javascript/packages/verify-flow/package.json @@ -4,5 +4,6 @@ "private": true, "dependencies": { "react": "^17.0.2" - } + }, + "sideEffects": false } diff --git a/app/javascript/packages/webauthn/package.json b/app/javascript/packages/webauthn/package.json index 329b14b4603..ca713cade83 100644 --- a/app/javascript/packages/webauthn/package.json +++ b/app/javascript/packages/webauthn/package.json @@ -1,5 +1,9 @@ { "name": "@18f/identity-webauthn", "version": "1.0.0", - "private": true + "private": true, + "sideEffects": [ + "./webauthn-input-element.ts", + "./webauthn-verify-button-element.ts" + ] } diff --git a/docs/frontend.md b/docs/frontend.md index a1fc13ebe57..29d6b16630a 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -79,6 +79,7 @@ In practice: package is intended to be published to NPM. - ...a value for the `version` field, since it is required. The value value can be anything, and `"1.0.0"` is a good default. + - ...a `sideEffects` value listing files containing any side effects, used for [Webpack's Tree Shaking optimization](https://webpack.js.org/guides/tree-shaking/). - The package should be importable by its bare name, either with an `index.ts` or equivalent [package entrypoints](https://nodejs.org/api/packages.html#package-entry-points) diff --git a/scripts/validate-workspaces.js b/scripts/validate-workspaces.mjs similarity index 70% rename from scripts/validate-workspaces.js rename to scripts/validate-workspaces.mjs index a9a75bc42d1..7774f39efa3 100755 --- a/scripts/validate-workspaces.js +++ b/scripts/validate-workspaces.mjs @@ -1,8 +1,8 @@ #!/usr/bin/env node -const { readFile, stat } = require('fs/promises'); -const { dirname, basename, join } = require('path'); -const { sync: glob } = require('fast-glob'); +import { readFile, stat } from 'node:fs/promises'; +import { dirname, basename, join, resolve, relative } from 'node:path'; +import glob from 'fast-glob'; /** @typedef {[path: string, manifest: Record]} ManifestPair */ /** @typedef {ManifestPair[]} ManifestPairs */ @@ -43,9 +43,16 @@ function checkHaveCommonDependencyVersions(manifests) { */ function checkHaveRequiredFields(manifests) { for (const [path, manifest] of manifests) { - ['name', 'version', 'private'].forEach((field) => { + Object.entries({ + name: `Expected "@18f/identity-${basename(dirname(path))}".`, + version: '"1.0.0" is recommended for private packages.', + private: '`true` is recommended unless the package is intended to be published to NPM.', + sideEffects: + 'This is usually `false`, unless there are files which include side effects, ' + + 'such as custom elements registering to the custom element registry.', + }).forEach(([field, guidance]) => { if (!(field in manifest)) { - throw new Error(`Missing required field ${field} in ${path}`); + throw new Error([`Missing required field ${field} in ${path}`, guidance].join(' ')); } }); } @@ -115,6 +122,31 @@ async function checkHaveDocumentation(manifests) { ); } +/** + * @param {ManifestPairs} manifests + */ +function checkPackageSideEffectsIncludesCustomElements(manifests) { + return Promise.all( + manifests.map(async ([manifestPath, manifest]) => { + const manifestDirectory = dirname(manifestPath); + const customElementPaths = await glob(join(manifestDirectory, '*-element.ts')); + const expectedPaths = customElementPaths.map((path) => resolve(path)); + const actualPaths = Array.from(manifest.sideEffects).map((path) => + resolve(join(manifestDirectory, path)), + ); + const hasExpected = expectedPaths.every((path) => actualPaths.includes(path)); + if (!hasExpected) { + throw new Error( + [ + `Missing expected custom elements in ${manifestPath} sideEffects:`, + customElementPaths.map((path) => `./${relative(manifestDirectory, path)}`).join(', '), + ].join(' '), + ); + } + }), + ); +} + /** * @type {Record void>} */ @@ -126,6 +158,7 @@ const CHECKS = { checkHaveCorrectVersion, checkHaveNoSiblingDependencies, checkHaveDocumentation, + checkPackageSideEffectsIncludesCustomElements, }; /** @@ -137,7 +170,7 @@ const EXCEPTIONS = { checkHaveCorrectPackageName: ['app/javascript/packages/eslint-plugin/package.json'], }; -const manifestPaths = glob('app/javascript/packages/*/package.json'); +const manifestPaths = await glob('app/javascript/packages/*/package.json'); Promise.all(manifestPaths.map(async (path) => [path, await readFile(path, 'utf-8')])) .then((contents) => contents.map(([path, content]) => /** @type {ManifestPair} */ ([path, JSON.parse(content)])), From d011cc9ab9e9723037647062df9ad0ae6ddefde8 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Wed, 7 Feb 2024 10:14:08 -0500 Subject: [PATCH 11/16] resolve merge conflict (#10049) changelog: Internal, Document Authentication, remove selfie_check_performed field from DocumentCaptureSessionResult --- app/services/document_capture_session_result.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index a574750983d..ecf701af1d4 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -10,12 +10,11 @@ :failed_back_image_fingerprints, :failed_selfie_image_fingerprints, :captured_at, - :selfie_check_performed, :doc_auth_success, :selfie_status, keyword_init: true, allowed_members: [:id, :success, :attention_with_barcode, :failed_front_image_fingerprints, :failed_back_image_fingerprints, :failed_selfie_image_fingerprints, - :captured_at, :selfie_check_performed, :doc_auth_success, :selfie_status] + :captured_at, :doc_auth_success, :selfie_status] ) do include DocAuth::SelfieConcern From fbf9f179699eb9dc4c2098bddedca6c3631cbdb8 Mon Sep 17 00:00:00 2001 From: "Luis H. Matos" Date: Wed, 7 Feb 2024 10:33:30 -0600 Subject: [PATCH 12/16] LG-12300 Exclude Deleted Users from the Reuse Report (#10028) * LG-12300 Exclude Deleted Users from the Reuse Report changelog: Internal, Reporting, Exclude deleted users from Reuse Rate Report --- .../reporting/account_reuse_report.rb | 55 ++++-- .../reporting/account_reuse_report_spec.rb | 158 +++++++++++------- 2 files changed, 131 insertions(+), 82 deletions(-) diff --git a/app/services/reporting/account_reuse_report.rb b/app/services/reporting/account_reuse_report.rb index f8c08b7ad65..da5ea4f9c98 100644 --- a/app/services/reporting/account_reuse_report.rb +++ b/app/services/reporting/account_reuse_report.rb @@ -96,6 +96,8 @@ def as_csv self end + # Each EntityReuseSummary (there are two - apps and agencies) contains + # a ReuseDetailSection which is made up of individual ReuseDetailRows ReuseDetailSection = Struct.new(:detail_rows) do def initialize(detail_rows: [ReuseDetailRow.new]) super(detail_rows:) @@ -143,6 +145,10 @@ def organize_results(all_results, idv_results, entity_type) self end + # The Reuse Report has two parts: One for sp(app) reuse and one for agency reuse + # The EntityReuseSummary is the structure for each part, it consists of: + # - A ReuseDetailsSection (made up of ReuseDetailRows) + # - A Summary Row (which holds the data for 2+ Entities) EntityReuseSummary = Struct.new( :details_section, :total_all_users, :total_all_percent, @@ -179,20 +185,25 @@ def update_from_results(results:, total_registered:, total_proofed:) end end - results.each_with_index do |details_section, section_index| - details_section.select { |details| details.num_entities >= 10 }. - reduce do |summary_row, captured_row| - # Delete any rows after the first captured_row (which becomes the summary_row) - details_section.delete(captured_row) if captured_row != summary_row - summary_row.update_details( - num_entities: "10-#{captured_row.num_entities}", - entity_type: summary_row.entity_type, - num_all_users: summary_row.num_all_users + captured_row.num_all_users, - all_percent: summary_row.all_percent + captured_row.all_percent, - num_idv_users: summary_row.num_idv_users + captured_row.num_idv_users, - idv_percent: summary_row.idv_percent + captured_row.idv_percent, - ) - end + # If there are rows that capture data on 10 or more entities, + # they all get condensed into one row here + results.each do |details_section| + # Only condense the rows if there is more than one row in the 10+ range + if details_section.count { |details| details.num_entities >= 10 } > 1 + details_section.select { |details| details.num_entities >= 10 }. + reduce do |condensed_row, captured_row| + # Delete any rows after the first captured_row (which becomes the condensed_row) + details_section.delete(captured_row) if captured_row != condensed_row + condensed_row.update_details( + num_entities: "10-#{captured_row.num_entities}", + entity_type: condensed_row.entity_type, + num_all_users: condensed_row.num_all_users + captured_row.num_all_users, + all_percent: condensed_row.all_percent + captured_row.all_percent, + num_idv_users: condensed_row.num_idv_users + captured_row.num_idv_users, + idv_percent: condensed_row.idv_percent + captured_row.idv_percent, + ) + end + end end self.details_section = results @@ -249,6 +260,8 @@ def sp_reuse_results_all , identities.user_id FROM identities + JOIN + users on users.id = identities.user_id WHERE identities.created_at < %{query_date} GROUP BY @@ -256,7 +269,7 @@ def sp_reuse_results_all ) sps_per_all_users GROUP BY sps_per_all_users.num_apps - HAVING + HAVING sps_per_all_users.num_apps > 1 ORDER BY num_apps ASC @@ -280,6 +293,8 @@ def sp_reuse_results_idv , identities.user_id FROM identities + JOIN + users on users.id = identities.user_id WHERE identities.last_ial2_authenticated_at IS NOT NULL AND @@ -289,7 +304,7 @@ def sp_reuse_results_idv ) sps_per_idv_users GROUP BY sps_per_idv_users.num_apps - HAVING + HAVING sps_per_idv_users.num_apps > 1 ORDER BY num_apps ASC @@ -313,6 +328,8 @@ def agency_reuse_results_all , identities.user_id FROM identities + JOIN + users on users.id = identities.user_id JOIN service_providers sp ON identities.service_provider = sp.issuer JOIN @@ -324,7 +341,7 @@ def agency_reuse_results_all ) agencies_per_user GROUP BY agencies_per_user.num_agencies - HAVING + HAVING agencies_per_user.num_agencies > 1 ORDER BY num_agencies ASC @@ -348,6 +365,8 @@ def agency_reuse_results_idv , identities.user_id FROM identities + JOIN + users on users.id = identities.user_id JOIN service_providers sp ON identities.service_provider = sp.issuer JOIN @@ -361,7 +380,7 @@ def agency_reuse_results_idv ) agencies_per_user GROUP BY agencies_per_user.num_agencies - HAVING + HAVING agencies_per_user.num_agencies > 1 ORDER BY num_agencies ASC diff --git a/spec/services/reporting/account_reuse_report_spec.rb b/spec/services/reporting/account_reuse_report_spec.rb index 4afc8d19e1f..ee1f1ee0cff 100644 --- a/spec/services/reporting/account_reuse_report_spec.rb +++ b/spec/services/reporting/account_reuse_report_spec.rb @@ -151,112 +151,142 @@ # This will give 9 users with 2 agencies for the IDV agency report # This will give 13 users with 2 agencies for the ALL agency report - users_to_query = [ - { id: 1, # 3 apps, 2 agencies + mock_users_to_query = [ + { # 3 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(3), - sp_timestamp: Array.new(3) { in_query } }, - { id: 2, # 3 apps, 2 agencies + sp_timestamp: Array.new(3) { in_query }, + }, + { # 3 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(3), - sp_timestamp: [in_query, in_query, out_of_query] }, - { id: 3, # 3 apps, 2 agencies + sp_timestamp: [in_query, in_query, out_of_query], + }, + { # 3 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(3), - sp_timestamp: [in_query, out_of_query, out_of_query] }, - { id: 4, # 3 apps, 2 agencies + sp_timestamp: [in_query, out_of_query, out_of_query], + }, + { # 3 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(3), - sp_timestamp: [in_query, out_of_query, out_of_query] }, - { id: 5, # 3 apps, 2 agencies + sp_timestamp: [in_query, out_of_query, out_of_query], + }, + { # 3 apps, 2 agencies created_timestamp: out_of_query, sp: all_agency_apps.first(3), - sp_timestamp: Array.new(3) { out_of_query } }, - { id: 6, # 2 apps, 2 agencies + sp_timestamp: Array.new(3) { out_of_query }, + }, + { # 2 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(2), - sp_timestamp: Array.new(2) { in_query } }, - { id: 7, # 2 apps, 1 agency + sp_timestamp: Array.new(2) { in_query }, + }, + { # 2 apps, 1 agency created_timestamp: in_query, sp: agency1_apps.first(2), - sp_timestamp: Array.new(2) { in_query } }, - { id: 8, # 2 apps, 2 agencies + sp_timestamp: Array.new(2) { in_query }, + }, + { # 2 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(2), - sp_timestamp: [in_query, out_of_query] }, - { id: 9, # 2 apps, 1 agency + sp_timestamp: [in_query, out_of_query], + }, + { # 2 apps, 1 agency created_timestamp: in_query, sp: agency1_apps.first(2), - sp_timestamp: [in_query, out_of_query] }, - { id: 10, # 2 apps, 2 agencies + sp_timestamp: [in_query, out_of_query], + }, + { # 2 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(2), - sp_timestamp: Array.new(2) { out_of_query } }, - { id: 11, # 2 apps, 2 agencies + sp_timestamp: Array.new(2) { out_of_query }, + }, + { # 2 apps, 2 agencies created_timestamp: out_of_query, sp: all_agency_apps.first(2), - sp_timestamp: Array.new(2) { out_of_query } }, - { id: 12, + sp_timestamp: Array.new(2) { out_of_query }, + }, + { # 1 app, 1 agency created_timestamp: in_query, sp: [sp_a], - sp_timestamp: [in_query] }, - { id: 13, + sp_timestamp: [in_query], + }, + { # 1 app, 1 agency created_timestamp: in_query, sp: [sp_a], - sp_timestamp: [out_of_query] }, - { id: 14, + sp_timestamp: [out_of_query], + }, + { # 1 app, 1 agency created_timestamp: out_of_query, sp: [sp_a], - sp_timestamp: [out_of_query] }, - { id: 15, # 12 apps, 2 agencies + sp_timestamp: [out_of_query], + }, + { # 12 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps, - sp_timestamp: Array.new(12) { in_query } }, - { id: 16, # 11 apps, 2 agencies + sp_timestamp: Array.new(12) { in_query }, + }, + { # 11 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(11), - sp_timestamp: Array.new(11) { in_query } }, - { id: 17, # 11 apps, 2 agencies + sp_timestamp: Array.new(11) { in_query }, + }, + { # 11 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(11), - sp_timestamp: Array.new(11) { in_query } }, - { id: 18, # 10 apps, 2 agencies + sp_timestamp: Array.new(11) { in_query }, + }, + { # 10 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(10), - sp_timestamp: Array.new(10) { in_query } }, - { id: 19, # 10 apps, 2 agencies + sp_timestamp: Array.new(10) { in_query }, + }, + { # 10 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(10), - sp_timestamp: Array.new(10) { in_query } }, - { id: 20, # 10 apps, 2 agencies + sp_timestamp: Array.new(10) { in_query }, + }, + { # 10 apps, 2 agencies created_timestamp: in_query, sp: all_agency_apps.first(10), - sp_timestamp: Array.new(9) { in_query } + Array.new(1) { out_of_query } }, - + sp_timestamp: Array.new(9) { in_query } + Array.new(1) { out_of_query }, + }, + { # 3 apps, 2 agencies - Last user gets deleted + created_timestamp: in_query, + sp: all_agency_apps.first(3), + sp_timestamp: Array.new(3) { in_query }, + deleted: true, + }, ] - users_to_query.each do |user| - user[:sp].each_with_index do |sp, i| + mock_users_to_query.each do |mock_user| + current_user = create(:user, :fully_registered, registered_at: in_query) + + if mock_user[:deleted] + DeletedUser.create_from_user(current_user) + current_user.destroy! + else + create( + :profile, + :active, + activated_at: in_query, + user: current_user, + ) + end + + mock_user[:sp].each_with_index do |sp, i| ServiceProviderIdentity.create( - user_id: user[:id], + user_id: current_user.id, service_provider: sp, - created_at: user[:created_timestamp], + created_at: mock_user[:created_timestamp], last_ial2_authenticated_at: in_query, - verified_at: user[:sp_timestamp][i], + verified_at: mock_user[:sp_timestamp][i], ) end end - # Create active profiles for total_proofed_identities - # These 20 profiles will yield 10 active profiles in the results - 10.times do - create( - :profile, - :active, - activated_at: in_query, - user: create(:user, :fully_registered, registered_at: in_query), - ) - end + # Create active profiles that are out of query for total_proofed_identities 10.times do create( :profile, @@ -271,13 +301,13 @@ it 'has the correct results' do expected_csv = [ ['Metric', 'Num. all users', '% of accounts', 'Num. IDV users', '% of accounts'], - ['2 apps', 5, 5 / 20.0, 3, 0.3], - ['3 apps', 4, 4 / 20.0, 1, 0.1], - ['9 apps', 0, 0 / 20.0, 1, 0.1], - ['10-12 apps', 6, 6 / 20.0, 5, 0.5], - ['2+ apps', 15, 15 / 20.0, 10, 0.9999999999999999], - ['2 agencies', 13, 13 / 20.0, 9, 0.9], - ['2+ agencies', 13, 13 / 20.0, 9, 0.9], + ['2 apps', 5, 5 / 30.0, 3, 3 / 20.0], + ['3 apps', 4, 4 / 30.0, 1, 1 / 20.0], + ['9 apps', 0, 0 / 30.0, 1, 1 / 20.0], + ['10-12 apps', 6, 6 / 30.0, 5, 5 / 20.0], + ['2+ apps', 15, 15 / 30.0, 10, 0.49999999999999994], + ['2 agencies', 13, 13 / 30.0, 9, 9 / 20.0], + ['2+ agencies', 13, 13 / 30.0, 9, 9 / 20.0], ] aggregate_failures do From f8a01e03a7c4bc1bf400680f34bc3780888e5999 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 7 Feb 2024 09:21:07 -0800 Subject: [PATCH 13/16] LG-12333: Add analytics for password verification after profile reactivation (#10039) * Add analytics for password verification after profile reactivation Log visits and submissions of the verify password form we display after the user reactivates their profile. changelog: Internal, Identity verification, Add analytics for password verification on profile reactivation. * Update verify_password_controller.rb --- .../users/verify_password_controller.rb | 7 ++++++- app/services/analytics_events.rb | 11 ++++++++++ .../users/verify_password_controller_spec.rb | 20 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/verify_password_controller.rb b/app/controllers/users/verify_password_controller.rb index fd006b6a8f7..ffde05879a6 100644 --- a/app/controllers/users/verify_password_controller.rb +++ b/app/controllers/users/verify_password_controller.rb @@ -6,7 +6,9 @@ class VerifyPasswordController < ApplicationController before_action :confirm_password_reset_profile before_action :confirm_personal_key - def new; end + def new + analytics.reactivate_account_verify_password_visited + end def update result = verify_password_form.submit @@ -14,6 +16,9 @@ def update irs_attempts_api_tracker.logged_in_profile_change_reauthentication_submitted( success: result.success?, ) + + analytics.reactivate_account_verify_password_submitted(success: result.success?) + if result.success? handle_success(result) else diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index a588054d5ec..5f9243d6ec3 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4057,6 +4057,17 @@ def reactivate_account_submit track_event('Reactivate Account Submitted') end + # Submission event for the "verify password" page the user sees after entering their personal key. + # @param [Boolean] success Whether the form was submitted successfully. + def reactivate_account_verify_password_submitted(success:, **extra) + track_event(:reactivate_account_verify_password_submitted, success: success, **extra) + end + + # Visit event for the "verify password" page the user sees after entering their personal key. + def reactivate_account_verify_password_visited(**extra) + track_event(:reactivate_account_verify_password_visited, **extra) + end + # Account profile reactivation page visited def reactivate_account_visit track_event('Reactivate Account Visited') diff --git a/spec/controllers/users/verify_password_controller_spec.rb b/spec/controllers/users/verify_password_controller_spec.rb index 33ec58f7c32..b1af63dee0e 100644 --- a/spec/controllers/users/verify_password_controller_spec.rb +++ b/spec/controllers/users/verify_password_controller_spec.rb @@ -7,6 +7,7 @@ let(:user) { create(:user, profiles: profiles, **recovery_hash) } before do + stub_analytics stub_sign_in(user) end @@ -51,6 +52,11 @@ expect(response).to render_template(:new) end + + it 'logs an analytics event' do + get :new + expect(@analytics).to have_logged_event(:reactivate_account_verify_password_visited) + end end describe '#update' do @@ -74,6 +80,13 @@ put :update, params: user_params end + it 'logs an appropriate analytics event' do + expect(@analytics).to have_logged_event( + :reactivate_account_verify_password_submitted, + success: true, + ) + end + it 'tracks the appropriate attempts api events' do expect(@irs_attempts_api_tracker).to have_received( :logged_in_profile_change_reauthentication_submitted, @@ -101,6 +114,13 @@ put :update, params: user_params end + it 'logs an appropriate analytics event' do + expect(@analytics).to have_logged_event( + :reactivate_account_verify_password_submitted, + success: false, + ) + end + it 'tracks the appropriate attempts api event' do expect(@irs_attempts_api_tracker).to have_received( :logged_in_profile_change_reauthentication_submitted, From c205b54dc81608056954679d3ab9e074907855b4 Mon Sep 17 00:00:00 2001 From: Sudheer Dandamudi <134308185+dskgsa@users.noreply.github.com> Date: Wed, 7 Feb 2024 12:49:07 -0500 Subject: [PATCH 14/16] Capture deployment metrics in gitlab pipelines (#10020) * update gitlab ci to track deployments for dora metrics --- .gitlab-ci.yml | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7d2664d1461..84abbaf1ba0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -63,6 +63,7 @@ stages: - after_test - review - scan + - deploy_production workflow: rules: @@ -357,12 +358,7 @@ trigger_devops: - if: $CI_COMMIT_BRANCH == "main" trigger: lg/identity-devops -review-app: - stage: review - allow_failure: true - needs: - - job: build-review-image - resource_group: $CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov +.deploy: image: name: dtzar/helm-kubectl:latest script: @@ -495,6 +491,15 @@ review-app: - echo https://$CI_ENVIRONMENT_SLUG-review-app.pivcac.identitysandbox.gov - echo "Address of Dashboard review app:" - echo https://$CI_ENVIRONMENT_SLUG-review-app-dashboard.review-app.identitysandbox.gov + + +review-app: + stage: review + allow_failure: true + needs: + - job: build-review-image + resource_group: $CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov + extends: .deploy environment: name: review/$CI_COMMIT_REF_NAME url: https://$CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov @@ -525,6 +530,21 @@ stop-review-app: - if: $CI_PIPELINE_SOURCE != "merge_request_event" when: never +deploy_production: + stage: deploy_production + allow_failure: false + needs: + - job: build-review-image + resource_group: $CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov + extends: .deploy + environment: + name: production + deployment_tier: production + url: https://$CI_ENVIRONMENT_SLUG.review-app.identitysandbox.gov + rules: + - if: $CI_COMMIT_BRANCH == "main" && $CI_PIPELINE_SOURCE == "push" + + include: - template: Jobs/SAST.gitlab-ci.yml - template: Jobs/Dependency-Scanning.gitlab-ci.yml From 1aa6a1bfe354c400e0b8b52bdd16f8869f12feec Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 7 Feb 2024 13:31:20 -0500 Subject: [PATCH 15/16] LG-12070: Maintain incoming request for concurrent session logout (#10040) * LG-12070: Maintain incoming request for concurrent session logout changelog: Bug Fixes, Concurrent Session, Maintain authentication request when enforcing concurrent session limit * Add comment clarifying ordering See: https://github.com/18F/identity-idp/pull/10040#discussion_r1479933092 --- .../authorization_controller.rb | 5 +++ app/controllers/saml_idp_controller.rb | 5 ++- .../openid_connect/openid_connect_spec.rb | 31 ++++++++++++++++--- spec/features/saml/saml_logout_spec.rb | 4 --- spec/features/saml/saml_spec.rb | 27 ++++++++++++++++ 5 files changed, 62 insertions(+), 10 deletions(-) diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index feb12eafccd..a1171123193 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -12,6 +12,7 @@ class AuthorizationController < ApplicationController before_action :block_biometric_requests_in_production, only: [:index] before_action :build_authorize_form_from_params, only: [:index] + before_action :set_devise_failure_redirect_for_concurrent_session_logout before_action :pre_validate_authorize_form, only: [:index] before_action :sign_out_if_prompt_param_is_login_and_user_is_signed_in, only: [:index] before_action :store_request, only: [:index] @@ -73,6 +74,10 @@ def redirect_to_reauthenticate redirect_to user_two_factor_authentication_url end + def set_devise_failure_redirect_for_concurrent_session_logout + request.env['devise_session_limited_failure_redirect_url'] = request.url + end + def link_identity_to_service_provider @authorize_form.link_identity_to_service_provider(current_user, session.id) end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 5c539f59a86..e28797867d7 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -2,6 +2,10 @@ require 'saml_idp' class SamlIdpController < ApplicationController + # Ordering is significant, since failure URL must be assigned before any references to the user, + # as the concurrent session timeout occurs as a callback to Warden's `after_set_user` hook. + before_action :set_devise_failure_redirect_for_concurrent_session_logout, only: [:auth, :logout] + include SamlIdp::Controller include SamlIdpAuthConcern include SamlIdpLogoutConcern @@ -17,7 +21,6 @@ class SamlIdpController < ApplicationController skip_before_action :verify_authenticity_token before_action :require_path_year - before_action :set_devise_failure_redirect_for_concurrent_session_logout, only: :logout before_action :handle_banned_user before_action :bump_auth_count, only: :auth before_action :redirect_to_sign_in, only: :auth, unless: :user_signed_in? diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 907d281cf22..c38c511fc60 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -950,12 +950,8 @@ end context 'when signed in with another browser' do - it 'redirects back to the client app after concurrent session logout' do + it 'redirects back to the client app if logging out after concurrent session logout' do user = user_with_2fa - service_provider = ServiceProvider.find_by(issuer: OidcAuthHelper::OIDC_IAL1_ISSUER) - IdentityLinker.new(user, service_provider).link_identity( - verified_attributes: %w[openid email], - ) perform_in_browser(:one) do visit_idp_from_sp_with_ial1(:oidc) @@ -980,6 +976,31 @@ expect(page).to have_content(t('headings.sign_in_without_sp')) end end + + it 'maintains authentication request if logged out by concurrent session logout' do + user = user_with_2fa + + perform_in_browser(:one) do + visit_idp_from_sp_with_ial1(:saml) + sign_in_live_with_2fa(user) + end + + perform_in_browser(:two) do + visit_idp_from_sp_with_ial1(:saml) + sign_in_live_with_2fa(user) + end + + perform_in_browser(:one) do + visit_idp_from_sp_with_ial1(:oidc) + + expect(page).to have_content( + [ + ServiceProvider.find_by(issuer: OidcAuthHelper::OIDC_IAL1_ISSUER).friendly_name, + t('headings.create_account_with_sp.sp_text', app_name: APP_NAME), + ].join(' '), + ) + end + end end end diff --git a/spec/features/saml/saml_logout_spec.rb b/spec/features/saml/saml_logout_spec.rb index d8e000f9365..9d92642d9b0 100644 --- a/spec/features/saml/saml_logout_spec.rb +++ b/spec/features/saml/saml_logout_spec.rb @@ -109,10 +109,6 @@ context 'when signed in with another browser' do it 'redirects to the SP after concurrent session logout' do user = user_with_2fa - service_provider = ServiceProvider.find_by(issuer: SamlAuthHelper::SP_ISSUER) - IdentityLinker.new(user, service_provider).link_identity( - verified_attributes: %w[openid email], - ) perform_in_browser(:one) do visit_idp_from_sp_with_ial1(:saml) diff --git a/spec/features/saml/saml_spec.rb b/spec/features/saml/saml_spec.rb index d00093f67e8..c9d75609158 100644 --- a/spec/features/saml/saml_spec.rb +++ b/spec/features/saml/saml_spec.rb @@ -460,6 +460,33 @@ ), ) end + + context 'when signed in with another browser' do + it 'maintains authentication request if logged out by concurrent session logout' do + user = user_with_2fa + + perform_in_browser(:one) do + visit_idp_from_sp_with_ial1(:oidc) + sign_in_live_with_2fa(user) + end + + perform_in_browser(:two) do + visit_idp_from_sp_with_ial1(:oidc) + sign_in_live_with_2fa(user) + end + + perform_in_browser(:one) do + visit_idp_from_sp_with_ial1(:saml) + + expect(page).to have_content( + [ + ServiceProvider.find_by(issuer: SamlAuthHelper::SP_ISSUER).friendly_name, + t('headings.create_account_with_sp.sp_text', app_name: APP_NAME), + ].join(' '), + ) + end + end + end end end From 22aa3b1a13357be1153d7fa6c50f5b07043b8b3e Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:04:42 -0500 Subject: [PATCH 16/16] Target specific failing files for ESLint React overrides (#10026) changelog: Internal, Automated Testing, Improve accuracy of linting checks for React components --- .eslintrc | 102 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/.eslintrc b/.eslintrc index dbb99d833cd..0c776ec027a 100644 --- a/.eslintrc +++ b/.eslintrc @@ -21,7 +21,8 @@ "selector": "AssignmentExpression[left.property.name='href'][right.type=/(Template)?Literal/]", "message": "Do not assign window.location.href to a string or string template to avoid losing i18n parameters" } - ] + ], + "react-hooks/exhaustive-deps": "error" }, "settings": { "import/internal-regex": "^@18f/identity-" @@ -50,69 +51,80 @@ } }, { - // Turn off react linting rules for most packages/files "files": [ - "spec/**", - "app/javascript/packs/**", - "app/javascript/packages/address-search/**", - "app/javascript/packages/components/**", - "app/javascript/packages/compose-components/**", - "app/javascript/packages/form-steps/**", - "app/javascript/packages/react-hooks/**", - "app/javascript/packages/react-i18n/**", - "app/javascript/packages/spinner-button/**", - "app/javascript/packages/step-indicator/**", - "app/javascript/packages/validated-field/**", - "app/javascript/packages/verify-flow/**", - // In progress: enabling these rules for all files in packages/document-capture - "app/javascript/packages/document-capture/context/**", - "app/javascript/packages/document-capture/higher-order/**", - "app/javascript/packages/document-capture/hooks/**", - // Comment out a file to enable react lint rules for that file only + "app/javascript/packages/address-search/components/address-input.tsx", + "app/javascript/packages/address-search/components/full-address-search-input.tsx", + "app/javascript/packages/components/hooks/use-focus-trap.ts", + "app/javascript/packages/components/hooks/use-toggle-body-class-by-presence.ts", "app/javascript/packages/document-capture/components/acuant-camera.tsx", "app/javascript/packages/document-capture/components/acuant-capture-canvas.jsx", "app/javascript/packages/document-capture/components/acuant-capture.tsx", "app/javascript/packages/document-capture/components/acuant-selfie-camera.tsx", - "app/javascript/packages/document-capture/components/acuant-selfie-capture-canvas.jsx", - "app/javascript/packages/document-capture/components/barcode-attention-warning.tsx", "app/javascript/packages/document-capture/components/callback-on-mount.jsx", - "app/javascript/packages/document-capture/components/document-capture-troubleshooting-options.tsx", "app/javascript/packages/document-capture/components/document-capture-warning.tsx", "app/javascript/packages/document-capture/components/document-capture.tsx", - "app/javascript/packages/document-capture/components/document-side-acuant-capture.jsx", - "app/javascript/packages/document-capture/components/documents-step.jsx", "app/javascript/packages/document-capture/components/file-image.jsx", "app/javascript/packages/document-capture/components/file-input.tsx", - "app/javascript/packages/document-capture/components/hybrid-doc-capture-warning.spec.tsx", - "app/javascript/packages/document-capture/components/hybrid-doc-capture-warning.tsx", - "app/javascript/packages/document-capture/components/in-person-call-to-action.spec.tsx", - "app/javascript/packages/document-capture/components/in-person-call-to-action.tsx", - "app/javascript/packages/document-capture/components/in-person-location-full-address-entry-post-office-search-step.spec.tsx", "app/javascript/packages/document-capture/components/in-person-location-full-address-entry-post-office-search-step.tsx", - "app/javascript/packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx", "app/javascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx", - "app/javascript/packages/document-capture/components/in-person-outage-alert.spec.tsx", - "app/javascript/packages/document-capture/components/in-person-outage-alert.tsx", - "app/javascript/packages/document-capture/components/in-person-prepare-step.spec.tsx", - "app/javascript/packages/document-capture/components/in-person-prepare-step.tsx", "app/javascript/packages/document-capture/components/in-person-switch-back-step.tsx", - "app/javascript/packages/document-capture/components/in-person-troubleshooting-options.tsx", "app/javascript/packages/document-capture/components/review-issues-step.tsx", - "app/javascript/packages/document-capture/components/status-message.jsx", - "app/javascript/packages/document-capture/components/submission-complete.tsx", "app/javascript/packages/document-capture/components/submission-interstitial.jsx", - "app/javascript/packages/document-capture/components/submission-status.tsx", + "app/javascript/packages/document-capture/context/acuant.tsx", + "app/javascript/packages/document-capture/hooks/use-cookie.js", + "app/javascript/packages/form-steps/form-steps.spec.tsx", + "app/javascript/packages/form-steps/form-steps.tsx", + "app/javascript/packages/form-steps/use-history-param.ts", + "app/javascript/packages/react-hooks/use-did-update-effect.ts", + "app/javascript/packages/react-hooks/use-immutable-callback.ts", + "app/javascript/packages/react-hooks/use-object-memo.ts" + ], + "rules": { + "react-hooks/exhaustive-deps": "off" + } + }, + { + "files": [ + "app/javascript/packages/address-search/components/in-person-locations.spec.tsx", + "app/javascript/packages/components/spinner-dots.jsx", + "app/javascript/packages/document-capture/components/acuant-capture.tsx", + "app/javascript/packages/document-capture/components/acuant-selfie-capture-canvas.jsx", + "app/javascript/packages/document-capture/components/document-side-acuant-capture.jsx", + "app/javascript/packages/document-capture/components/file-image.jsx", + "app/javascript/packages/document-capture/components/file-input.tsx", + "app/javascript/packages/document-capture/components/in-person-location-full-address-entry-post-office-search-step.tsx", + "app/javascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx", + "app/javascript/packages/document-capture/components/in-person-prepare-step.tsx", + "app/javascript/packages/document-capture/components/submission-interstitial.jsx", "app/javascript/packages/document-capture/components/submission.jsx", - "app/javascript/packages/document-capture/components/suspense-error-boundary.jsx", - "app/javascript/packages/document-capture/components/tip-list.tsx", - "app/javascript/packages/document-capture/components/unknown-error.tsx", + "spec/javascript/packages/document-capture/context/failed-capture-attempts-spec.jsx", + "spec/javascript/packages/document-capture/hooks/use-async-spec.jsx" + ], + "rules": { + "react/prop-types": "off" + } + }, + { + "files": [ + "app/javascript/packages/components/status-page.spec.tsx", "app/javascript/packages/document-capture/components/warning.tsx" ], "rules": { - "react/prop-types": "off", - "react/display-name": "off", - "react/jsx-key": "off", - "react-hooks/exhaustive-deps": "off", + "react/jsx-key": "off" + } + }, + { + "files": ["app/javascript/packages/document-capture/higher-order/with-props.jsx"], + "rules": { + "react/display-name": "off" + } + }, + { + "files": [ + "app/javascript/packages/form-steps/form-steps.spec.tsx", + "spec/javascript/spec_helper.js" + ], + "rules": { "react-hooks/rules-of-hooks": "off" } }