From 31951f08a0cd4a957e83267bd90c8303bc060ffc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 3 Nov 2021 08:25:23 -0400 Subject: [PATCH 01/15] Remove unused document capture notice text (#5574) **Why**: Because they're never actually presented to the user, who will instead see the message derived from the failing response. --- .../idv/steps/document_capture_step.rb | 7 +------ config/locales/errors/en.yml | 20 ------------------- config/locales/errors/es.yml | 20 ------------------- config/locales/errors/fr.yml | 20 ------------------- 4 files changed, 1 insertion(+), 66 deletions(-) diff --git a/app/services/idv/steps/document_capture_step.rb b/app/services/idv/steps/document_capture_step.rb index d7f44dbc086..e34a5cee952 100644 --- a/app/services/idv/steps/document_capture_step.rb +++ b/app/services/idv/steps/document_capture_step.rb @@ -80,13 +80,8 @@ def post_images def handle_document_verification_failure(response) mark_step_incomplete(:document_capture) - notice = if liveness_checking_enabled? - { notice: I18n.t('errors.doc_auth.document_capture_info_with_selfie_html') } - else - { notice: I18n.t('errors.doc_auth.document_capture_info_html') } - end log_document_error(response) - extra = response.to_h.merge(notice) + extra = response.to_h failure(response.first_error_message, extra) end diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 652069bc862..1d5112ce930 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -23,26 +23,6 @@ en: consent_form: Before you can continue, you must give us permission. Please check the box below and then click continue. document_capture_cancelled: You have cancelled uploading photos of your ID on your phone. - document_capture_info_html: |- -
Please check that:
- - document_capture_info_with_selfie_html: |- -
Please check that:
- document_verification_timeout: The server took too long to respond. Please try again. phone_step_incomplete: You must go to your phone and upload photos of your ID before continuing. We sent you a link with instructions. diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index a72fd8bc0d2..49556910450 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -23,26 +23,6 @@ es: consent_form: Antes de continuar, debe darnos permiso. Marque la casilla a continuación y luego haga clic en continuar. document_capture_cancelled: Ha cancelado la carga de fotos de su identificación en este teléfono. - document_capture_info_html: |- -
Por favor verifique que:
- - document_capture_info_with_selfie_html: |- -
Por favor verifique que:
- document_verification_timeout: El servidor tardó demasiado en responder. Inténtalo de nuevo. phone_step_incomplete: Debe ir a su teléfono y cargar fotos de su identificación antes de continuar. Te enviamos un enlace con instrucciones. diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 06e3e500985..26c7f36455f 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -26,26 +26,6 @@ fr: Veuillez cocher la case ci-dessous puis cliquez sur continuer. document_capture_cancelled: Vous avez annulé le téléchargement de vos photos d'identité sur votre téléphone. - document_capture_info_html: |- -
Veuillez vérifier que:
- - document_capture_info_with_selfie_html: |- -
Veuillez vérifier que:
- document_verification_timeout: Le serveur a mis trop de temps à répondre. Veuillez réessayer. phone_step_incomplete: Vous devez aller sur votre téléphone et télécharger des photos de votre identifiant avant de continuer. Nous vous avons envoyé From eb194cde4a87759fafa56090375172cff30b4c7d Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 3 Nov 2021 09:00:25 -0400 Subject: [PATCH 02/15] LG-4956: translate activemodel error messages (#5560) * LG-4956: translations for possible error messages for activemodel validation * LG-4956: Include additional translations for activemodel validations * LG-4956: normalize yaml --- config/locales/errors/en.yml | 11 +++++++++++ config/locales/errors/es.yml | 11 +++++++++++ config/locales/errors/fr.yml | 11 +++++++++++ 3 files changed, 33 insertions(+) diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 1d5112ce930..f0fc75242d6 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -55,6 +55,7 @@ en: gpo_otp_expired: Your confirmation code has expired. Please request another letter for a new code. improbable_phone: Invalid phone number. Please make sure you enter a valid phone number. + inclusion: is not included in the list invalid_calling_area: Calls to that phone number are not supported. Please try SMS if you have an SMS-capable phone. invalid_phone_number: The phone number entered is not valid. @@ -66,6 +67,7 @@ en: must_have_us_country_code: Invalid phone number. Please make sure you enter a phone number with a U.S. country code. no_pending_profile: No profile is waiting for verification + not_a_number: is not a number otp_failed: Your security code failed to send. password_incorrect: Incorrect password personal_key_incorrect: Incorrect personal key @@ -77,6 +79,12 @@ en: select a different number and try again. pwned_password: The password you entered is not safe. It’s in a list of known passwords exposed in data breaches. + too_long: + one: is too long (maximum is 1 character) + other: is too long (maximum is %{count} characters) + too_short: + one: is too short (minimum is 1 character) + other: is too short (minimum is %{count} characters) try_again: Please try again. unauthorized_authn_context: Unauthorized authentication context unauthorized_nameid_format: Unauthorized nameID format @@ -85,6 +93,9 @@ en: voip_phone: This number is a web-based (VOIP) phone service. Please select a different number and try again weak_password: Your password is not strong enough. %{feedback} + wrong_length: + one: is the wrong length (should be 1 character) + other: is the wrong length (should be %{count} characters) piv_cac_setup: unique_name: That name is already taken. Please choose a different name. registration: diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 49556910450..088a0cfe4fb 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -57,6 +57,7 @@ es: carta para recibir un nuevo código. improbable_phone: Número de teléfono no válido. Asegúrate de introducir un número de teléfono válido. + inclusion: No se incluye en la lista. invalid_calling_area: No se admiten llamadas a ese número de teléfono. Intenta enviar un SMS si tienes un teléfono que permita enviar SMS. invalid_phone_number: El número de teléfono ingresado no está en el formato correcto. @@ -68,6 +69,7 @@ es: must_have_us_country_code: Número de teléfono no válido. Asegúrate de introducir un número de teléfono con un código país de EE. UU. no_pending_profile: Ningún perfil está esperando verificación + not_a_number: no es un número otp_failed: Se produjo un error al enviar el código de seguridad. password_incorrect: La contraseña es incorrecta personal_key_incorrect: La clave personal es incorrecta @@ -80,6 +82,12 @@ es: premium. Por favor, seleccione otro número e inténtelo de nuevo. pwned_password: La contraseña que ingresaste no es segura. Está en una lista de contraseñas conocidas expuestas en violaciones de datos. + too_long: + one: es demasiado largo (el máximo es 1 carácter) + other: es demasiado largo (el máximo es %{count} caracteres) + too_short: + one: es demasiado corto (el mínimo es 1 carácter) + other: es demasiado corto (el mínimo es %{count} caracteres) try_again: Por favor, inténtelo de nuevo. unauthorized_authn_context: Contexto de autenticación no autorizado unauthorized_nameid_format: Formato de ID de nombre no autorizado @@ -89,6 +97,9 @@ es: voip_phone: Este número corresponde a un servicio de telefonía basado en la web (VoIP). Por favor, seleccione un número diferente e inténtelo de nuevo. weak_password: Su contraseña no es suficientemente segura. %{feedback} + wrong_length: + one: es la longitud incorrecta (debería ser de 1 carácter) + other: es la longitud incorrecta (debería ser de %{count} caracteres) piv_cac_setup: unique_name: El nombre ya fue escogido. Por favor, elija un nombre diferente. registration: diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 26c7f36455f..fcef42dc381 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -63,6 +63,7 @@ fr: autre lettre afin d’obtenir un nouveau code. improbable_phone: Numéro de téléphone non valide. Veillez à saisir un numéro de téléphone valide. + inclusion: N'est pas inclus dans la liste invalid_calling_area: Les appels vers ce numéro de téléphone ne sont pas pris en charge. Veuillez essayer par SMS si vous possédez un téléphone disposant de cette fonction. @@ -75,6 +76,7 @@ fr: must_have_us_country_code: Numéro de téléphone non valide. Veuillez vous assurer de saisir un numéro de téléphone avec un code pays des États-Unis. no_pending_profile: Aucun profil en attente de vérification + not_a_number: N'est pas un nombre otp_failed: Échec de l’envoi de votre code de sécurité. password_incorrect: Mot de passe incorrect personal_key_incorrect: Clé personnelle incorrecte @@ -89,6 +91,12 @@ fr: pwned_password: Le mot de passe que vous avez entré n’est pas sécurisé. C’est dans une liste de mots de passe connus exposés dans les violations de données. + too_long: + one: Est trop long (maximum de 1 caractère) + other: est trop long (le maximum est de %{count} caractères) + too_short: + one: est trop court (1 caractère minimum) + other: est trop court (le minimum est de %{count} caractères) try_again: Veuillez réessayer. unauthorized_authn_context: Contexte d’authentification non autorisé unauthorized_nameid_format: Format non autorisé du nom d’identification @@ -98,6 +106,9 @@ fr: voip_phone: Ce numéro est un service téléphonique basé sur le Web (Voix sur IP). Veuillez sélectionner un autre numéro et réessayer weak_password: Votre mot de passe n’est pas assez fort. %{feedback} + wrong_length: + one: n'est pas de la bonne longueur (devrait être de 1 caractère) + other: n'est pas de la bonne longueur (devrait être %{count} caractères) piv_cac_setup: unique_name: Ce nom est déjà pris. Veuillez choisir un autre nom. registration: From 6366a1bd54b30cd59172b63415d294ee9e715213 Mon Sep 17 00:00:00 2001 From: Oren Kanner Date: Thu, 4 Nov 2021 13:34:43 -0400 Subject: [PATCH 03/15] Add configuration to turn off agreements data seeding (#5581) --- config/application.yml.default | 2 ++ db/seeds.rb | 16 ++++++++++------ lib/identity_config.rb | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/config/application.yml.default b/config/application.yml.default index 609b7cb8ec4..9a5ce741eaf 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -196,6 +196,7 @@ rules_of_use_updated_at: '2021-05-21T00:00:00Z' s3_public_reports_enabled: 'false' s3_reports_enabled: 'false' saml_secret_rotation_enabled: 'false' +seed_agreements_data: 'true' service_provider_request_ttl_hours: '24' session_check_delay: '30' session_check_frequency: '30' @@ -345,6 +346,7 @@ production: saml_endpoint_configs: '[]' scrypt_cost: 10000$8$1$ secret_key_base: + seed_agreements_data: 'false' session_encryption_key: skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:dev", "urn:gov:gsa:SAML:2.0.profiles:sp:sso:int"]' sps_over_quota_limit_notify_email_list: '[]' diff --git a/db/seeds.rb b/db/seeds.rb index 8d82bcb00a6..30d3804c03b 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -5,9 +5,13 @@ AgencySeeder.new.run # add partnerships / agreements data, note that the order matters! -Agreements::PartnerAccountStatusSeeder.new.run -Agreements::PartnerAccountSeeder.new.run -Agreements::IaaGtcSeeder.new.run -Agreements::IntegrationStatusSeeder.new.run -Agreements::IntegrationSeeder.new.run -Agreements::IaaOrderSeeder.new.run +if IdentityConfig.store.seed_agreements_data + puts '=== Seeding agreements data ===' # rubocop:disable Rails/Output + + Agreements::PartnerAccountStatusSeeder.new.run + Agreements::PartnerAccountSeeder.new.run + Agreements::IaaGtcSeeder.new.run + Agreements::IntegrationStatusSeeder.new.run + Agreements::IntegrationSeeder.new.run + Agreements::IaaOrderSeeder.new.run +end diff --git a/lib/identity_config.rb b/lib/identity_config.rb index e74026155cb..4203329f3bf 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -282,6 +282,7 @@ def self.build_store(config_map) config.add(:saml_secret_rotation_enabled, type: :boolean) config.add(:scrypt_cost, type: :string) config.add(:secret_key_base, type: :string) + config.add(:seed_agreements_data, type: :boolean) config.add(:service_provider_request_ttl_hours, type: :integer) config.add(:session_check_delay, type: :integer) config.add(:session_check_frequency, type: :integer) From 3d649b5fab3fbaca9cde2d8bf53c2c9f6003ec41 Mon Sep 17 00:00:00 2001 From: Oren Kanner Date: Thu, 4 Nov 2021 13:34:58 -0400 Subject: [PATCH 04/15] Add admin@gsa.gov to the dev:prime rake task (#5580) --- lib/tasks/dev.rake | 2 +- spec/lib/tasks/dev_rake_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 5e4e3760c61..dd663949f53 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -2,7 +2,7 @@ namespace :dev do desc 'Sample data for local development environment' task prime: :environment do pw = 'salty pickles' - %w[test1@test.com test2@test.com].each_with_index do |email, index| + %w[test1@test.com test2@test.com admin@gsa.gov].each_with_index do |email, index| ee = EncryptedAttribute.new_from_decrypted(email) User.find_or_create_by!(email_fingerprint: ee.fingerprint) do |user| setup_user(user, ee: ee, pw: pw, num: index) diff --git a/spec/lib/tasks/dev_rake_spec.rb b/spec/lib/tasks/dev_rake_spec.rb index 5c40b182417..e01a5e8791b 100644 --- a/spec/lib/tasks/dev_rake_spec.rb +++ b/spec/lib/tasks/dev_rake_spec.rb @@ -12,7 +12,7 @@ it 'runs successfully' do Rake::Task['dev:prime'].invoke - expect(User.count).to eq 3 + expect(User.count).to eq 4 end end From e74f464793c5381c36ab280530870d74a0f284e6 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 4 Nov 2021 10:45:36 -0700 Subject: [PATCH 05/15] Pass around Pii::Attributes around instead of a string (LG-5294) (#5579) - Minimize chances of accidental logging by using redacted struct --- app/controllers/users/verify_password_controller.rb | 4 ++-- .../users/verify_personal_key_controller.rb | 7 ++++--- app/forms/verify_personal_key_form.rb | 9 +++------ app/models/profile.rb | 4 ++++ app/services/reactivate_account_session.rb | 9 +++++++-- spec/forms/verify_personal_key_form_spec.rb | 5 ++--- spec/services/reactivate_account_session_spec.rb | 10 +++++----- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/app/controllers/users/verify_password_controller.rb b/app/controllers/users/verify_password_controller.rb index f292c08e1d2..8cec216bf66 100644 --- a/app/controllers/users/verify_password_controller.rb +++ b/app/controllers/users/verify_password_controller.rb @@ -29,9 +29,9 @@ def confirm_personal_key redirect_to root_url end + # @return [Pii::Attributes, nil] def decrypted_pii - pii = reactivate_account_session.decrypted_pii - @_decrypted_pii ||= Pii::Attributes.new_from_json(pii) + @_decrypted_pii ||= reactivate_account_session.decrypted_pii end def handle_success(result) diff --git a/app/controllers/users/verify_personal_key_controller.rb b/app/controllers/users/verify_personal_key_controller.rb index cf7e7d1262b..7ce30a7873e 100644 --- a/app/controllers/users/verify_personal_key_controller.rb +++ b/app/controllers/users/verify_personal_key_controller.rb @@ -28,7 +28,7 @@ def create analytics.track_event(Analytics::PERSONAL_KEY_REACTIVATION_SUBMITTED, result.to_h) if result.success? - handle_success(decrypted_pii_json: personal_key_form.decrypted_pii_json) + handle_success(decrypted_pii: personal_key_form.decrypted_pii) else handle_failure(result) end @@ -61,9 +61,10 @@ def init_account_reactivation reactivate_account_session.start end - def handle_success(decrypted_pii_json:) + # @param [Pii::Attributes] decrypted_pii + def handle_success(decrypted_pii:) analytics.track_event(Analytics::PERSONAL_KEY_REACTIVATION) - reactivate_account_session.store_decrypted_pii(decrypted_pii_json) + reactivate_account_session.store_decrypted_pii(decrypted_pii) redirect_to verify_password_url end diff --git a/app/forms/verify_personal_key_form.rb b/app/forms/verify_personal_key_form.rb index b0b67ced776..4183e934a55 100644 --- a/app/forms/verify_personal_key_form.rb +++ b/app/forms/verify_personal_key_form.rb @@ -22,8 +22,9 @@ def submit FormResponse.new(success: valid?, errors: errors, extra: extra) end - def decrypted_pii_json - decrypted_pii&.to_json + # @return [Pii::Attributes,nil] + def decrypted_pii + @_pii ||= password_reset_profile.recover_pii(personal_key) end private @@ -32,10 +33,6 @@ def password_reset_profile user.decorate.password_reset_profile end - def decrypted_pii - @_pii ||= password_reset_profile.recover_pii(personal_key) - end - def validate_personal_key return check_personal_key if personal_key_decrypts? errors.add :personal_key, :personal_key_incorrect diff --git a/app/models/profile.rb b/app/models/profile.rb index 8d376de9eb1..dc4b91e3761 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -40,12 +40,14 @@ def decrypt_pii(password) Pii::Attributes.new_from_json(decrypted_json) end + # @return [Pii::Attributes] def recover_pii(personal_key) encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) decrypted_recovery_json = encryptor.decrypt(encrypted_pii_recovery, user_uuid: user.uuid) Pii::Attributes.new_from_json(decrypted_recovery_json) end + # @param [Pii::Attributes] pii def encrypt_pii(pii, password) encrypt_ssn_fingerprint(pii) encrypt_compound_pii_fingerprint(pii) @@ -54,6 +56,7 @@ def encrypt_pii(pii, password) encrypt_recovery_pii(pii) end + # @param [Pii::Attributes] pii def encrypt_recovery_pii(pii) personal_key = personal_key_generator.create encryptor = Encryption::Encryptors::PiiEncryptor.new( @@ -63,6 +66,7 @@ def encrypt_recovery_pii(pii) @personal_key = personal_key end + # @param [Pii::Attributes] pii def self.build_compound_pii(pii) values = [ pii.first_name, diff --git a/app/services/reactivate_account_session.rb b/app/services/reactivate_account_session.rb index 756f8e4690c..71504c07bfb 100644 --- a/app/services/reactivate_account_session.rb +++ b/app/services/reactivate_account_session.rb @@ -24,17 +24,22 @@ def suspend session[SESSION_KEY] = generate_session end + # Stores PII as a string in the session + # @param [Pii::Attributes] def store_decrypted_pii(pii) reactivate_account_session[:personal_key] = true - reactivate_account_session[:pii] = pii + reactivate_account_session[:pii] = pii.to_json end def personal_key? reactivate_account_session[:personal_key] end + # Parses string into PII struct + # @return [Pii::Attributes, nil] def decrypted_pii - reactivate_account_session[:pii] + json_str = reactivate_account_session[:pii] + Pii::Attributes.new_from_json(json_str) if json_str end private diff --git a/spec/forms/verify_personal_key_form_spec.rb b/spec/forms/verify_personal_key_form_spec.rb index fc479c991ab..d741bf016d6 100644 --- a/spec/forms/verify_personal_key_form_spec.rb +++ b/spec/forms/verify_personal_key_form_spec.rb @@ -25,9 +25,8 @@ it 'exposes the decrypted_pii as a separate attribute' do form.submit - expect(form.decrypted_pii_json).to be_present - expect(JSON.parse(form.decrypted_pii_json, symbolize_names: true)). - to include(ssn: '123456789') + expect(form.decrypted_pii).to be_present + expect(form.decrypted_pii.ssn).to eq('123456789') end end diff --git a/spec/services/reactivate_account_session_spec.rb b/spec/services/reactivate_account_session_spec.rb index 3ae7e0fe8c3..1559340bcd2 100644 --- a/spec/services/reactivate_account_session_spec.rb +++ b/spec/services/reactivate_account_session_spec.rb @@ -41,14 +41,14 @@ describe '#suspend' do it 'sets the reactivate account object back to its defaults' do - pii = {} + pii = Pii::Attributes.new(first_name: 'Test') @reactivate_account_session.start @reactivate_account_session.store_decrypted_pii(pii) expect(@reactivate_account_session.started?).to be(true) expect(@reactivate_account_session.personal_key?).to be(true) - expect(@reactivate_account_session.decrypted_pii).to be(pii) + expect(@reactivate_account_session.decrypted_pii).to eq(pii) @reactivate_account_session.suspend @@ -60,11 +60,11 @@ describe '#store_decrypted_pii' do it 'stores the supplied object in the session and toggles `personal_key` flag' do - pii = {} + pii = Pii::Attributes.new(first_name: 'Test') @reactivate_account_session.store_decrypted_pii(pii) account_reactivation_obj = user_session[:reactivate_account] expect(account_reactivation_obj[:personal_key]).to be(true) - expect(account_reactivation_obj[:pii]).to eq(pii) + expect(account_reactivation_obj[:pii]).to eq(pii.to_json) end end @@ -85,7 +85,7 @@ end it 'returns the pii stored in the session' do - pii = {} + pii = Pii::Attributes.new(first_name: 'Test') @reactivate_account_session.store_decrypted_pii(pii) expect(@reactivate_account_session.decrypted_pii).to eq(pii) From d87c53129edac584b424b000de604c807646edb0 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 4 Nov 2021 10:55:27 -0700 Subject: [PATCH 06/15] Prepare to drop encrypted_code columns (LG-4427) (#5582) - Make columns nullable so we can stop writing - Remove unique index so we can stop writing --- ...configurations_unique_index_on_code_fingerprint.rb | 11 +++++++++++ ...code_configurations_allow_null_code_fingerprint.rb | 5 +++++ ...p_code_configurations_allow_null_encrypted_code.rb | 5 +++++ db/schema.rb | 7 +++---- 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 db/primary_migrate/20211104171549_drop_backup_code_configurations_unique_index_on_code_fingerprint.rb create mode 100644 db/primary_migrate/20211104172508_change_backup_code_configurations_allow_null_code_fingerprint.rb create mode 100644 db/primary_migrate/20211104173202_change_backup_code_configurations_allow_null_encrypted_code.rb diff --git a/db/primary_migrate/20211104171549_drop_backup_code_configurations_unique_index_on_code_fingerprint.rb b/db/primary_migrate/20211104171549_drop_backup_code_configurations_unique_index_on_code_fingerprint.rb new file mode 100644 index 00000000000..27a90bbc75b --- /dev/null +++ b/db/primary_migrate/20211104171549_drop_backup_code_configurations_unique_index_on_code_fingerprint.rb @@ -0,0 +1,11 @@ +class DropBackupCodeConfigurationsUniqueIndexOnCodeFingerprint < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def change + remove_index :backup_code_configurations, + name: 'index_bcc_on_user_id_code_fingerprint', + column: [:user_id, :code_fingerprint], + unique: true, + algorithm: :concurrently + end +end diff --git a/db/primary_migrate/20211104172508_change_backup_code_configurations_allow_null_code_fingerprint.rb b/db/primary_migrate/20211104172508_change_backup_code_configurations_allow_null_code_fingerprint.rb new file mode 100644 index 00000000000..00851ed4589 --- /dev/null +++ b/db/primary_migrate/20211104172508_change_backup_code_configurations_allow_null_code_fingerprint.rb @@ -0,0 +1,5 @@ +class ChangeBackupCodeConfigurationsAllowNullCodeFingerprint < ActiveRecord::Migration[6.1] + def change + change_column_null :backup_code_configurations, :code_fingerprint, true + end +end diff --git a/db/primary_migrate/20211104173202_change_backup_code_configurations_allow_null_encrypted_code.rb b/db/primary_migrate/20211104173202_change_backup_code_configurations_allow_null_encrypted_code.rb new file mode 100644 index 00000000000..85da152b18f --- /dev/null +++ b/db/primary_migrate/20211104173202_change_backup_code_configurations_allow_null_encrypted_code.rb @@ -0,0 +1,5 @@ +class ChangeBackupCodeConfigurationsAllowNullEncryptedCode < ActiveRecord::Migration[6.1] + def change + change_column_null :backup_code_configurations, :encrypted_code, true + end +end diff --git a/db/schema.rb b/db/schema.rb index c0758e8826c..d6a3044bacc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_10_14_225944) do +ActiveRecord::Schema.define(version: 2021_11_04_173202) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -71,15 +71,14 @@ create_table "backup_code_configurations", force: :cascade do |t| t.integer "user_id", null: false - t.string "code_fingerprint", default: "", null: false - t.string "encrypted_code", default: "", null: false + t.string "code_fingerprint", default: "" + t.string "encrypted_code", default: "" t.datetime "used_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "salted_code_fingerprint" t.string "code_salt" t.string "code_cost" - t.index ["user_id", "code_fingerprint"], name: "index_bcc_on_user_id_code_fingerprint", unique: true t.index ["user_id", "created_at"], name: "index_backup_code_configurations_on_user_id_and_created_at" t.index ["user_id", "salted_code_fingerprint"], name: "index_backup_codes_on_user_id_and_salted_code_fingerprint" end From 70f46e021b291101075f60417f48f87a1c45d488 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 5 Nov 2021 09:03:25 -0400 Subject: [PATCH 07/15] LG-4870: Configure Acuant SDK path for JavaScript WebSDK resources (#5577) * LG-4870: Configure Acuant SDK path for JavaScript WebSDK resources **Why**: Simplify to remove symlinks which exist only to support loading Acuant SDK relatively from subpaths. Instead, configure Acuant to point directly to the canonical source directory. * Revert, support both new and old Acuant static paths **Why**: To avoid issue with mid-deploy session requests --- .../document-capture/context/acuant.jsx | 27 +++++++++++++++++-- config/initializers/secure_headers.rb | 5 +++- .../document-capture/context/acuant-spec.jsx | 10 +++++++ spec/requests/acuant_sdk_spec.rb | 6 +++-- 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/app/javascript/packages/document-capture/context/acuant.jsx b/app/javascript/packages/document-capture/context/acuant.jsx index 7548799fe12..aae41d7c7b8 100644 --- a/app/javascript/packages/document-capture/context/acuant.jsx +++ b/app/javascript/packages/document-capture/context/acuant.jsx @@ -4,6 +4,15 @@ import AnalyticsContext from './analytics'; /** @typedef {import('react').ReactNode} ReactNode */ +/** + * @typedef AcuantConfig + * + * @prop {string=} path Path from which to load SDK service worker. + * + * @see https://github.com/Acuant/JavascriptWebSDKV11/blob/11.4.3/SimpleHTMLApp/webSdk/dist/AcuantJavascriptWebSdk.js#L1025-L1027 + * @see https://github.com/Acuant/JavascriptWebSDKV11/blob/11.4.3/SimpleHTMLApp/webSdk/dist/AcuantJavascriptWebSdk.js#L1049 + */ + /** * @typedef AcuantCamera * @@ -40,8 +49,9 @@ import AnalyticsContext from './analytics'; /** * @typedef AcuantGlobals * - * @prop {()=>void} onAcuantSdkLoaded Acuant initialization callback. - * @prop {AcuantCamera} AcuantCamera Acuant camera API. + * @prop {AcuantConfig=} acuantConfig Acuant configuration. + * @prop {()=>void} onAcuantSdkLoaded Acuant initialization callback. + * @prop {AcuantCamera} AcuantCamera Acuant camera API. * @prop {AcuantJavaScriptWebSDK} AcuantJavascriptWebSdk Acuant web SDK. */ @@ -74,6 +84,15 @@ export const DEFAULT_ACCEPTABLE_GLARE_SCORE = 30; */ export const DEFAULT_ACCEPTABLE_SHARPNESS_SCORE = 30; +/** + * Returns the containing directory of the given file, including a trailing slash. + * + * @param {string} file + * + * @return {string} + */ +export const dirname = (file) => file.split('/').slice(0, -1).concat('').join('/'); + const AcuantContext = createContext({ isReady: false, isAcuantLoaded: false, @@ -175,6 +194,9 @@ function AcuantContextProvider({ ); }; + const originalAcuantConfig = /** @type {AcuantGlobal} */ (window).acuantConfig; + /** @type {AcuantGlobal} */ (window).acuantConfig = { path: dirname(sdkSrc) }; + const script = document.createElement('script'); script.async = true; script.src = sdkSrc; @@ -183,6 +205,7 @@ function AcuantContextProvider({ return () => { /** @type {AcuantGlobal} */ (window).onAcuantSdkLoaded = originalOnAcuantSdkLoaded; + /** @type {AcuantGlobal} */ (window).acuantConfig = originalAcuantConfig; document.body.removeChild(script); }; }, []); diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 5cc94bda25a..4104a24fdd6 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -110,7 +110,10 @@ def call(env) product(%w[doc_auth capture_doc]). map do |locale, flow| File.join('/', *locale, '/verify', flow, worker_js) - end.to_set.freeze + end. + push('/acuant/11.4.3/AcuantImageProcessingWorker.min.js'). + to_set. + freeze config.middleware.insert_before( SecureHeaders::Middleware, diff --git a/spec/javascripts/packages/document-capture/context/acuant-spec.jsx b/spec/javascripts/packages/document-capture/context/acuant-spec.jsx index 0cef17df87b..f4f8e900f16 100644 --- a/spec/javascripts/packages/document-capture/context/acuant-spec.jsx +++ b/spec/javascripts/packages/document-capture/context/acuant-spec.jsx @@ -3,6 +3,7 @@ import { useContext } from 'react'; import { renderHook } from '@testing-library/react-hooks'; import { DeviceContext, AnalyticsContext } from '@18f/identity-document-capture'; import AcuantContext, { + dirname, Provider as AcuantContextProvider, DEFAULT_ACCEPTABLE_GLARE_SCORE, DEFAULT_ACCEPTABLE_SHARPNESS_SCORE, @@ -15,6 +16,15 @@ describe('document-capture/context/acuant', () => { delete window.AcuantCamera; }); + describe('dirname', () => { + it('returns the containing directory with trailing slash', () => { + const file = '/acuant/AcuantJavascriptWebSdk.min.js'; + const result = dirname(file); + + expect(result).to.equal('/acuant/'); + }); + }); + it('provides default context value', () => { const { result } = renderHook(() => useContext(AcuantContext)); diff --git a/spec/requests/acuant_sdk_spec.rb b/spec/requests/acuant_sdk_spec.rb index b3094c3338c..cf2afb5e2bd 100644 --- a/spec/requests/acuant_sdk_spec.rb +++ b/spec/requests/acuant_sdk_spec.rb @@ -6,9 +6,11 @@ # - /en/verify/capture_doc/AcuantImageProcessingWorker.min.js [nil, *I18n.available_locales]. product(%w[doc_auth capture_doc]). - each do |locale, verify_path| + map do |locale, verify_path| base_url = "#{locale && "/#{locale}"}/verify/#{verify_path}" - + end. + push('/acuant/11.4.3'). + each do |base_url| min_js = "#{base_url}/AcuantImageProcessingWorker.min.js" context min_js do before { get min_js } From c5b18d8ae9b9a359943c7bb5df6a0e5abce68f7c Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 5 Nov 2021 10:46:52 -0400 Subject: [PATCH 08/15] Remove unused BassCSS responsive white space (#5585) **Why**: Because none of these classes are currently in use. --- app/assets/stylesheets/_vendor.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/stylesheets/_vendor.scss b/app/assets/stylesheets/_vendor.scss index 1673e6cfa30..48708ea28ed 100644 --- a/app/assets/stylesheets/_vendor.scss +++ b/app/assets/stylesheets/_vendor.scss @@ -13,4 +13,3 @@ @import 'basscss-sass/positions'; @import 'basscss-sass/grid'; @import 'basscss-sass/flex-object'; -@import 'basscss-sass/responsive-white-space'; From 48062672f05b99848f4321fa22b91691cabce3cc Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 8 Nov 2021 09:18:28 -0600 Subject: [PATCH 09/15] Fix error in job log subscriber (#5588) * fix error in job log subscriber * add spec --- lib/identity_job_log_subscriber.rb | 14 ++++++--- spec/lib/identity_job_log_subscriber_spec.rb | 32 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/lib/identity_job_log_subscriber.rb b/lib/identity_job_log_subscriber.rb index 6a18d90af49..a38d99cc7a3 100644 --- a/lib/identity_job_log_subscriber.rb +++ b/lib/identity_job_log_subscriber.rb @@ -84,13 +84,19 @@ def enqueue_retry(event) ex = event.payload[:error] wait_seconds = event.payload[:wait] - json = { - wait_ms: wait.to_i.in_milliseconds, - } + json = default_attributes(event, job).merge( + wait_ms: wait_seconds.to_i.in_milliseconds, + ) json[:exception_class] = ex.class.name if ex - info(json.to_json) + if ex + error(json.to_json) + else + info(json.to_json) + end + + json end def retry_stopped(event) diff --git a/spec/lib/identity_job_log_subscriber_spec.rb b/spec/lib/identity_job_log_subscriber_spec.rb index ff63e85a5f1..7e4efff1643 100644 --- a/spec/lib/identity_job_log_subscriber_spec.rb +++ b/spec/lib/identity_job_log_subscriber_spec.rb @@ -56,4 +56,36 @@ ) }.to raise_error(ArgumentError) end + + describe '#enqueue_retry' do + it 'formats retry message' do + event = double( + 'RetryEvent', + payload: { wait: 1, job: double('Job', job_id: '1', queue_name: 'Default', arguments: []) }, + duration: 1, + name: 'TestEvent', + ) + subscriber = IdentityJobLogSubscriber.new + hash = subscriber.enqueue_retry(event) + expect(hash[:wait_ms]).to eq 1000 + expect(hash[:duration_ms]).to eq 1 + end + + it 'includes exception if there is a failure' do + event = double( + 'RetryEvent', + payload: { + wait: 1, job: double( + 'Job', job_id: '1', queue_name: 'Default', arguments: [] + ), + error: double('Exception') + }, + duration: 1, + name: 'TestEvent', + ) + subscriber = IdentityJobLogSubscriber.new + hash = subscriber.enqueue_retry(event) + expect(hash[:exception_class]).to_not be_nil + end + end end From ac5fdd74bc3b61e5e848ef9428503fd8ecf4a64b Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 8 Nov 2021 10:21:13 -0600 Subject: [PATCH 10/15] update knapsack report (#5591) --- knapsack_rspec_report.json | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index d8ed1dba9f6..3cf4f069cca 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -2,6 +2,9 @@ "spec/blueprints/agreements/agency_blueprint_spec.rb": 0.006590999953914434, "spec/blueprints/agreements/iaa_blueprint_spec.rb": 0.020056000037584454, "spec/blueprints/agreements/partner_account_blueprint_spec.rb": 0.011812999960966408, + "spec/components/accordion_component_spec.rb": 0.010516999987885356, + "spec/components/alert_component_spec.rb": 0.055206999997608364, + "spec/components/base_component_spec.rb": 0.010506999969948083, "spec/config/initializers/ahoy_spec.rb": 0.02662299998337403, "spec/config/initializers/phonelib_spec.rb": 0.026941999967675656, "spec/controllers/account_reset/cancel_controller_spec.rb": 0.5463889999664389, @@ -16,6 +19,7 @@ "spec/controllers/concerns/idv/document_capture_concern_spec.rb": 0.09357199998339638, "spec/controllers/concerns/idv_step_concern_spec.rb": 0.14577200001804158, "spec/controllers/concerns/verify_sp_attributes_concern_spec.rb": 0.43789800000377, + "spec/controllers/country_support_controller_spec.rb": 0.02383299998473376, "spec/controllers/event_disavowal_controller_spec.rb": 0.28618200001074, "spec/controllers/fake_s3_controller_spec.rb": 0.026658000017050654, "spec/controllers/forgot_password_controller_spec.rb": 0.02369400003226474, @@ -40,6 +44,7 @@ "spec/controllers/idv/resend_otp_controller_spec.rb": 0.11073700000997633, "spec/controllers/idv/review_controller_spec.rb": 1.1269719999982044, "spec/controllers/idv/session_errors_controller_spec.rb": 0.13314600003650412, + "spec/controllers/idv/sessions_controller_spec.rb": 0.1530229999916628, "spec/controllers/idv_controller_spec.rb": 0.48395099997287616, "spec/controllers/mfa_confirmation_controller_spec.rb": 0.21629000001121312, "spec/controllers/openid_connect/authorization_controller_spec.rb": 0.6166690000100061, @@ -101,6 +106,8 @@ "spec/controllers/users/verify_personal_key_controller_spec.rb": 0.47911900002509356, "spec/controllers/users/webauthn_setup_controller_spec.rb": 0.3193550000432879, "spec/controllers/users_controller_spec.rb": 0.1636389999766834, + "spec/controllers/vendor_outage_controller_spec.rb": 0.011704000004101545, + "spec/decorators/device_decorator_spec.rb": 0.08465400000568479, "spec/decorators/email_context_spec.rb": 0.05298799998126924, "spec/decorators/event_decorator_spec.rb": 0.08396199997514486, "spec/decorators/mfa_context_spec.rb": 0.8965149999712594, @@ -160,6 +167,7 @@ "spec/features/idv/steps/phone_step_spec.rb": 104.97321700002067, "spec/features/idv/steps/review_step_spec.rb": 37.92662599997129, "spec/features/idv/uak_password_spec.rb": 6.835907000000589, + "spec/features/idv/vendor_outage_spec.rb": 20.898211999970954, "spec/features/legacy_passwords_spec.rb": 2.232516000047326, "spec/features/load_testing/email_sign_up_spec.rb": 0.555809999932535, "spec/features/monitor/create_account_spec.rb": 11.318466000026092, @@ -278,10 +286,13 @@ "spec/forms/user_piv_cac_setup_form_spec.rb": 0.16587099997559562, "spec/forms/user_piv_cac_verification_form_spec.rb": 0.13643799995770678, "spec/forms/verify_account_form_spec.rb": 0.2928119999705814, + "spec/forms/verify_password_form_spec.rb": 0.1348810000345111, + "spec/forms/verify_personal_key_form_spec.rb": 0.4419900000211783, "spec/forms/webauthn_setup_form_spec.rb": 0.09530600003199652, "spec/forms/webauthn_verification_form_spec.rb": 0.038155000016558915, "spec/forms/webauthn_visit_form_spec.rb": 0.025507999991532415, "spec/helpers/application_helper_spec.rb": 0.11493099998915568, + "spec/helpers/asset_helper_spec.rb": 0.006096999975852668, "spec/helpers/aws_s3_helper_spec.rb": 0.06452000001445413, "spec/helpers/go_back_helper_spec.rb": 0.03488400002242997, "spec/helpers/link_helper_spec.rb": 0.011707999976351857, @@ -298,11 +309,13 @@ "spec/jobs/job_helpers/s3_helper_spec.rb": 0.02984699996886775, "spec/jobs/job_helpers/stale_job_helper_spec.rb": 0.011769999982789159, "spec/jobs/job_helpers/timer_spec.rb": 0.006338000006508082, + "spec/jobs/remove_old_throttles_job_spec.rb": 0.07101199997123331, "spec/jobs/reports/agency_invoice_iaa_supplement_report_spec.rb": 0.08734999998705462, "spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb": 0.04119199997512624, "spec/jobs/reports/agency_user_counts_report_spec.rb": 0.019288000010419637, "spec/jobs/reports/base_report_spec.rb": 0.04786900000181049, "spec/jobs/reports/daily_auths_report_spec.rb": 0.03252000000793487, + "spec/jobs/reports/daily_dropoffs_report_spec.rb": 0.4107910000020638, "spec/jobs/reports/deleted_user_accounts_report_spec.rb": 0.19772500003455207, "spec/jobs/reports/gpo_report_spec.rb": 0.1449129999964498, "spec/jobs/reports/iaa_billing_report_spec.rb": 0.09024099999805912, @@ -318,6 +331,7 @@ "spec/jobs/reports/unique_monthly_auths_report_spec.rb": 0.013701999967452139, "spec/jobs/reports/unique_yearly_auths_report_spec.rb": 0.019680999976117164, "spec/jobs/resolution_proofing_job_spec.rb": 0.13581899995915592, + "spec/jobs/risc_delivery_job_spec.rb": 0.18415099999401718, "spec/lib/app_artifacts_spec.rb": 0.025138999975752085, "spec/lib/asset_checker_spec.rb": 0.010629999975208193, "spec/lib/aws/ses_spec.rb": 0.014733999967575073, @@ -327,6 +341,7 @@ "spec/lib/headers_filter_spec.rb": 0.0024090000079013407, "spec/lib/identity_job_log_subscriber_spec.rb": 0.04343299998436123, "spec/lib/linters/localized_validation_mesasge_linter_spec.rb": 0.019807000004220754, + "spec/lib/linters/redirect_back_linter_spec.rb": 0.20296700001927093, "spec/lib/linters/url_options_linter_spec.rb": 0.030101999989710748, "spec/lib/otp_code_generator_spec.rb": 0.006647999980486929, "spec/lib/pinpoint_supported_countries_spec.rb": 0.050749999994877726, @@ -335,6 +350,7 @@ "spec/lib/tasks/partners_rake_spec.rb": 0.5946460000122897, "spec/lib/tasks/rotate_rake_spec.rb": 0.11027800000738353, "spec/lib/utf8_sanitizer_spec.rb": 0.026462000038009137, + "spec/mailers/previews/user_mailer_preview_spec.rb": 0.14119400002527982, "spec/mailers/user_mailer_spec.rb": 1.7547030000132509, "spec/models/account_reset_request_spec.rb": 0.020245000021532178, "spec/models/agency_identity_spec.rb": 0.014334000006783754, @@ -364,6 +380,7 @@ "spec/models/service_provider_identity_spec.rb": 0.3968299999833107, "spec/models/service_provider_spec.rb": 0.049534999998286366, "spec/models/sp_return_log_spec.rb": 0.0029830000130459666, + "spec/models/throttle_spec.rb": 0.4605540000484325, "spec/models/user_spec.rb": 0.6623580000014044, "spec/models/webauthn_configuration_spec.rb": 0.11598300002515316, "spec/policies/backup_code_policy_spec.rb": 0.023431000008713454, @@ -409,11 +426,13 @@ "spec/presenters/two_factor_options_presenter_spec.rb": 0.028107999998610467, "spec/presenters/utc_time_presenter_spec.rb": 0.0032820000196807086, "spec/requests/acuant_sdk_spec.rb": 0.9878460000036284, + "spec/requests/country_support_cors_spec.rb": 0.43752899998798966, "spec/requests/csp_spec.rb": 1.3553689999971539, "spec/requests/headers_spec.rb": 0.4034349999856204, "spec/requests/i18n_spec.rb": 0.26939100003801286, "spec/requests/invalid_encoding_spec.rb": 0.29150900000240654, "spec/requests/invalid_sign_in_params_spec.rb": 0.22254600003361702, + "spec/requests/not_acceptable_spec.rb": 0.28763099998468533, "spec/requests/openid_connect_authorize_spec.rb": 0.7700460000196472, "spec/requests/openid_connect_cors_spec.rb": 0.5003290000022389, "spec/requests/page_not_found_spec.rb": 0.13776100001996383, @@ -453,6 +472,7 @@ "spec/services/attribute_asserter_spec.rb": 1.0788180000381544, "spec/services/backup_code_benchmarker_spec.rb": 0.35868599999230355, "spec/services/backup_code_generator_spec.rb": 1.9309440000215545, + "spec/services/browser_cache_spec.rb": 0.00961700000334531, "spec/services/calendar_service_spec.rb": 0.046334999962709844, "spec/services/completions_decider_spec.rb": 0.03317200002493337, "spec/services/data_requests/create_email_addresses_report_spec.rb": 0.01655099994968623, @@ -533,6 +553,7 @@ "spec/services/event_disavowal/generate_disavowal_token_spec.rb": 0.03131499997107312, "spec/services/event_disavowal/validate_disavowed_event_spec.rb": 0.09553499997127801, "spec/services/expired_license_allower_spec.rb": 0.03267099999357015, + "spec/services/forget_all_browsers_spec.rb": 0.0192370000295341, "spec/services/form_response_spec.rb": 0.14322899997932836, "spec/services/funnel/registration/add_mfa_spec.rb": 0.07139100000495091, "spec/services/funnel/registration/add_password_spec.rb": 0.02401100000133738, @@ -616,6 +637,7 @@ "spec/services/pwned_passwords/lookup_password_spec.rb": 0.008809999970253557, "spec/services/random_phrase_spec.rb": 0.015782999980729073, "spec/services/reactivate_account_session_spec.rb": 0.1225049999775365, + "spec/services/redis_rate_limiter_spec.rb": 0.04739199997857213, "spec/services/remember_device_cookie_spec.rb": 0.12538999994285405, "spec/services/request_password_reset_spec.rb": 2.7663989999564365, "spec/services/reset_user_password_spec.rb": 1.5159979999880306, @@ -638,6 +660,7 @@ "spec/services/throttler/increment_spec.rb": 0.014947000017855316, "spec/services/throttler/is_throttled_spec.rb": 0.015467000019270927, "spec/services/throttler/reset_spec.rb": 0.0079799999948591, + "spec/services/time_service_spec.rb": 0.0035389999975450337, "spec/services/update_user_spec.rb": 0.2406870000413619, "spec/services/uri_service_spec.rb": 0.015483000024687499, "spec/services/user_alerts/alert_user_about_account_verified_spec.rb": 0.4555030000046827, @@ -649,6 +672,7 @@ "spec/services/user_session_context_spec.rb": 0.010798999981489033, "spec/services/usps_in_person_proofer_spec.rb": 0.1974589999881573, "spec/services/uuid_reporter_spec.rb": 0.21517399995354936, + "spec/services/vendor_status_spec.rb": 0.056539000011980534, "spec/services/x509/attribute_spec.rb": 0.0031679999665357172, "spec/services/x509/attributes_spec.rb": 0.013852000003680587, "spec/services/x509/session_store_spec.rb": 0.008479999960400164, @@ -682,6 +706,8 @@ "spec/views/idv/doc_auth/upload.html.erb_spec.rb": 0.018768000009004027, "spec/views/idv/doc_auth/welcome.html.erb_spec.rb": 0.058102000039070845, "spec/views/idv/gpo/index.html.erb_spec.rb": 0.20512100000632927, + "spec/views/idv/otp_delivery_method/new.html.erb_spec.rb": 0.6548910000128672, + "spec/views/idv/phone/new.html.erb_spec.rb": 0.07012300001224503, "spec/views/idv/phone_errors/_warning.html.erb_spec.rb": 0.07010399998398498, "spec/views/idv/phone_errors/failure.html.erb_spec.rb": 0.02501400001347065, "spec/views/idv/phone_errors/jobfail.html.erb_spec.rb": 0.03437200002372265, From 1114f0e14f4589fe47357e94e62f0bd0b02ac49b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 8 Nov 2021 13:39:24 -0800 Subject: [PATCH 11/15] Fix CodeCoverage in CircleCI (#5596) Fixes error in AWS CLI --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 63c6ac677fb..ed9f5687b39 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -191,7 +191,7 @@ jobs: command: | sudo apt-get update sudo apt-get install python3-pip python-dev jq - sudo pip install awscli + sudo pip install awscli --ignore-installed six - run: name: Install Code Climate Test Reporter command: | From 79d1d2e8b56c02c2a95d9db08c27e8904a1353f3 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 8 Nov 2021 13:39:50 -0800 Subject: [PATCH 12/15] Move logging to logfile (#5595) **Why**: Clean up log output in specs --- db/seeds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/seeds.rb b/db/seeds.rb index 30d3804c03b..3d2b700d8f7 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -6,7 +6,7 @@ # add partnerships / agreements data, note that the order matters! if IdentityConfig.store.seed_agreements_data - puts '=== Seeding agreements data ===' # rubocop:disable Rails/Output + Rails.logger.info('=== Seeding agreements data ===') Agreements::PartnerAccountStatusSeeder.new.run Agreements::PartnerAccountSeeder.new.run From b08d237ef44846efbba33f6a3de1d56ab117b645 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 8 Nov 2021 13:40:12 -0800 Subject: [PATCH 13/15] Add unique users per-month to issuer supplement report (LG-5319) (#5592) --- ...agency_invoice_issuer_supplement_report.rb | 13 ++- ...l_monthly_auth_counts_within_iaa_window.rb | 87 +++++++++++++------ ...y_invoice_issuer_supplement_report_spec.rb | 2 + ...thly_auth_counts_within_iaa_window_spec.rb | 85 +++++++++++++----- 4 files changed, 132 insertions(+), 55 deletions(-) diff --git a/app/jobs/reports/agency_invoice_issuer_supplement_report.rb b/app/jobs/reports/agency_invoice_issuer_supplement_report.rb index 42e54b5a88e..700e44a3cb8 100644 --- a/app/jobs/reports/agency_invoice_issuer_supplement_report.rb +++ b/app/jobs/reports/agency_invoice_issuer_supplement_report.rb @@ -11,9 +11,14 @@ class AgencyInvoiceIssuerSupplementReport < BaseReport def perform(_date) raw_results = service_providers.flat_map do |service_provider| - transaction_with_timeout do - Db::MonthlySpAuthCount::TotalMonthlyAuthCountsWithinIaaWindow.call(service_provider) - end.to_a + [:sum, :unique].flat_map do |aggregate| + transaction_with_timeout do + Db::MonthlySpAuthCount::TotalMonthlyAuthCountsWithinIaaWindow.call( + service_provider: service_provider, + aggregate: aggregate, + ) + end.to_a + end end results = combine_by_issuer_month(raw_results) @@ -46,6 +51,8 @@ def combine_by_issuer_month(raw_results) year_month: year_month, ial1_total_auth_count: extract(grouped, 'total_auth_count', ial: 1), ial2_total_auth_count: extract(grouped, 'total_auth_count', ial: 2), + ial1_unique_users: extract(grouped, 'unique_users', ial: 1), + ial2_unique_users: extract(grouped, 'unique_users', ial: 2), } end.values end diff --git a/app/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window.rb b/app/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window.rb index 1c99c6918e7..cba0119c383 100644 --- a/app/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window.rb +++ b/app/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window.rb @@ -9,8 +9,10 @@ module TotalMonthlyAuthCountsWithinIaaWindow module_function + # @param [ServiceProvider] service_provider + # @param [Symbol] aggregate (one of :sum, :unique) # @return [PG::Result,Array] - def call(service_provider) + def call(service_provider:, aggregate:) return [] if !service_provider.iaa_start_date || !service_provider.iaa_end_date iaa_range = (service_provider.iaa_start_date..service_provider.iaa_end_date) @@ -25,35 +27,68 @@ def call(service_provider) # The subqueries create a uniform representation of data: # - full months from monthly_sp_auth_counts # - partial months by aggregating sp_return_logs - # The results are rows with [ial, auth_count, year_month, issuer, iaa, iaa start, iaa end] - union_query = [ - *full_month_subquery(sp: service_provider, full_months: full_months), - *partial_month_subqueries(sp: service_provider, partial_months: partial_months), + # The results are rows with [user_id, ial, auth_count, year_month] + subquery = [ + *full_month_subquery(issuer: issuer, full_months: full_months), + *partial_month_subqueries(issuer: issuer, partial_months: partial_months), ].join(' UNION ALL ') - ActiveRecord::Base.connection.execute(union_query) + select_clause = case aggregate + when :sum + <<~SQL + SUM(billing_month_logs.auth_count)::bigint AS total_auth_count + SQL + when :unique + <<~SQL + COUNT(DISTINCT billing_month_logs.user_id) AS unique_users + SQL + else + raise "unknown aggregate=#{aggregate}" + end + + params = { + iaa_start_date: quote(iaa_range.begin), + iaa_end_date: quote(iaa_range.end), + iaa: quote(service_provider.iaa), + issuer: quote(issuer), + subquery: subquery, + select_clause: select_clause, + } + + sql = format(<<~SQL, params) + WITH subquery AS (%{subquery}) + SELECT + billing_month_logs.year_month + , billing_month_logs.ial + , %{iaa_start_date} AS iaa_start_date + , %{iaa_end_date} AS iaa_end_date + , %{issuer} AS issuer + , %{iaa} AS iaa + , %{select_clause} + FROM + subquery billing_month_logs + GROUP BY + billing_month_logs.year_month + , billing_month_logs.ial + SQL + + ActiveRecord::Base.connection.execute(sql) end # @return [String] - def full_month_subquery(sp:, full_months:) + def full_month_subquery(issuer:, full_months:) return if full_months.blank? params = { - iaa: sp.iaa, - issuer: sp.issuer, - iaa_start_date: sp.iaa_start_date, - iaa_end_date: sp.iaa_end_date, + issuer: issuer, year_months: full_months.map { |r| r.begin.strftime('%Y%m') }, }.transform_values { |value| quote(value) } full_month_subquery = format(<<~SQL, params) SELECT - monthly_sp_auth_counts.year_month + monthly_sp_auth_counts.user_id + , monthly_sp_auth_counts.year_month , monthly_sp_auth_counts.ial - , SUM(monthly_sp_auth_counts.auth_count)::bigint AS total_auth_count - , %{issuer} AS issuer - , %{iaa} AS iaa - , %{iaa_start_date} AS iaa_start_date - , %{iaa_end_date} AS iaa_end_date + , SUM(monthly_sp_auth_counts.auth_count)::bigint AS auth_count FROM monthly_sp_auth_counts WHERE @@ -62,38 +97,34 @@ def full_month_subquery(sp:, full_months:) GROUP BY monthly_sp_auth_counts.year_month , monthly_sp_auth_counts.ial + , monthly_sp_auth_counts.user_id SQL end # @return [Array] - def partial_month_subqueries(sp:, partial_months:) + def partial_month_subqueries(issuer:, partial_months:) partial_months.map do |month_range| params = { - iaa: sp.iaa, - issuer: sp.issuer, - iaa_start_date: sp.iaa_start_date, - iaa_end_date: sp.iaa_end_date, range_start: month_range.begin, range_end: month_range.end, + issuer: issuer, year_month: month_range.begin.strftime('%Y%m'), }.transform_values { |value| quote(value) } format(<<~SQL, params) SELECT - %{year_month} AS year_month + sp_return_logs.user_id + , %{year_month} AS year_month , sp_return_logs.ial , COUNT(sp_return_logs.id) AS auth_count - , %{issuer} AS issuer - , %{iaa} AS iaa - , %{iaa_start_date} AS iaa_start_date - , %{iaa_end_date} AS iaa_end_date FROM sp_return_logs WHERE sp_return_logs.requested_at BETWEEN %{range_start} AND %{range_end} AND sp_return_logs.returned_at IS NOT NULL AND sp_return_logs.issuer = %{issuer} GROUP BY - sp_return_logs.ial + sp_return_logs.user_id + , sp_return_logs.ial SQL end end diff --git a/spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb b/spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb index 55d1e96469a..3c3408ab553 100644 --- a/spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb +++ b/spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb @@ -52,6 +52,8 @@ year_month: '202101', ial1_total_auth_count: 11, ial2_total_auth_count: 22, + ial1_unique_users: 1, + ial2_unique_users: 1, }, ], ) diff --git a/spec/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window_spec.rb b/spec/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window_spec.rb index af6e9059653..d244b7368a7 100644 --- a/spec/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window_spec.rb +++ b/spec/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window_spec.rb @@ -4,8 +4,12 @@ let(:iaa_range) { Date.new(2021, 1, 15)..Date.new(2022, 1, 14) } describe '.call' do + let(:aggregate) { :sum } subject(:result) do - Db::MonthlySpAuthCount::TotalMonthlyAuthCountsWithinIaaWindow.call(service_provider) + Db::MonthlySpAuthCount::TotalMonthlyAuthCountsWithinIaaWindow.call( + service_provider: service_provider, + aggregate: aggregate, + ) end context 'when the SP does not have IAA start/end dates' do @@ -58,29 +62,62 @@ ) end - it 'counts auths across sp_return_logs and monthly_sp_auth_counts' do - rows = [ - { - year_month: partial_month_date.strftime('%Y%m'), - ial: 1, - issuer: service_provider.issuer, - total_auth_count: 2, - iaa: service_provider.iaa, - iaa_start_date: iaa_range.begin.to_s, - iaa_end_date: iaa_range.end.to_s, - }, - { - year_month: full_month_date.strftime('%Y%m'), - ial: 1, - issuer: service_provider.issuer, - total_auth_count: 11, - iaa: service_provider.iaa, - iaa_start_date: iaa_range.begin.to_s, - iaa_end_date: iaa_range.end.to_s, - }, - ] - - expect(result.map(&:symbolize_keys)).to match_array(rows) + context 'with the :sum aggregate' do + let(:aggregate) { :sum } + + it 'counts auths across sp_return_logs and monthly_sp_auth_counts' do + rows = [ + { + year_month: partial_month_date.strftime('%Y%m'), + ial: 1, + issuer: service_provider.issuer, + total_auth_count: 2, + iaa: service_provider.iaa, + iaa_start_date: iaa_range.begin.to_s, + iaa_end_date: iaa_range.end.to_s, + }, + { + year_month: full_month_date.strftime('%Y%m'), + ial: 1, + issuer: service_provider.issuer, + total_auth_count: 11, + iaa: service_provider.iaa, + iaa_start_date: iaa_range.begin.to_s, + iaa_end_date: iaa_range.end.to_s, + }, + ] + + expect(result.map(&:symbolize_keys)).to match_array(rows) + end + end + + context 'when the :unique aggregate' do + let(:aggregate) { :unique } + + it 'counts the distinct users each month' do + rows = [ + { + year_month: partial_month_date.strftime('%Y%m'), + ial: 1, + issuer: service_provider.issuer, + unique_users: 1, + iaa: service_provider.iaa, + iaa_start_date: iaa_range.begin.to_s, + iaa_end_date: iaa_range.end.to_s, + }, + { + year_month: full_month_date.strftime('%Y%m'), + ial: 1, + issuer: service_provider.issuer, + unique_users: 1, + iaa: service_provider.iaa, + iaa_start_date: iaa_range.begin.to_s, + iaa_end_date: iaa_range.end.to_s, + }, + ] + + expect(result.map(&:symbolize_keys)).to match_array(rows) + end end end From 891c6022f860acca8462583deb2b53b49f86a861 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 9 Nov 2021 11:02:33 -0500 Subject: [PATCH 14/15] Support formatted message arguments in console.error test spy (#5597) **Why**: So that it's easier to debug the root cause of a console log, which may use format arguments to describe the callsite. --- spec/javascripts/support/console.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/support/console.js b/spec/javascripts/support/console.js index 31d8bfec843..2bb6cd18817 100644 --- a/spec/javascripts/support/console.js +++ b/spec/javascripts/support/console.js @@ -1,6 +1,7 @@ /* eslint-disable no-console */ import sinon from 'sinon'; +import { format } from 'util'; /** * Chai plugin which adds chainable `logged` method, to be used in combination with @@ -45,8 +46,8 @@ export function chaiConsoleSpy(chai, utils) { export function useConsoleLogSpy() { beforeEach(() => { console.unverifiedCalls = []; - sinon.stub(console, 'error').callsFake((message) => { - console.unverifiedCalls = console.unverifiedCalls.concat(message); + sinon.stub(console, 'error').callsFake((message, ...args) => { + console.unverifiedCalls = console.unverifiedCalls.concat(format(message, ...args)); }); }); From c7ec3ac85eae389a4eff5d55270b9a07bad7ee29 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Tue, 9 Nov 2021 12:44:02 -0600 Subject: [PATCH 15/15] Finer grained funnel tracking (#5589) --- app/services/analytics.rb | 14 ++++++++++++++ .../openid_connect/user_info_controller_spec.rb | 4 ++++ spec/services/analytics_spec.rb | 9 +++++++++ spec/support/fake_request.rb | 4 ++++ 4 files changed, 31 insertions(+) diff --git a/app/services/analytics.rb b/app/services/analytics.rb index d8b2a3b9143..2f9d4409f75 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -14,6 +14,8 @@ def track_event(event, attributes = {}) event_properties: attributes.except(:user_id), new_event: first_event_this_session?, new_session_path: first_path_visit_this_session?, + new_session_success_state: first_success_state_this_session?, + success_state: success_state_token(event), path: request&.path, user_id: attributes[:user_id] || user.uuid, locale: I18n.locale, @@ -38,7 +40,11 @@ def track_event(event, attributes = {}) def update_session_events_and_paths_visited_for_analytics(event) @session[:paths_visited] ||= {} @session[:events] ||= {} + @session[:success_states] ||= {} if request + token = success_state_token(event) + @session[:first_success_state] = !@session[:success_states].key?(token) + @session[:success_states][token] = true @session[:first_path_visit] = !@session[:paths_visited].key?(request.path) @session[:paths_visited][request.path] = true end @@ -50,6 +56,14 @@ def first_path_visit_this_session? @session[:first_path_visit] end + def first_success_state_this_session? + @session[:first_success_state] + end + + def success_state_token(event) + "#{request&.env&.dig('REQUEST_METHOD')}|#{request&.path}|#{event}" + end + def first_event_this_session? @session[:first_event] end diff --git a/spec/controllers/openid_connect/user_info_controller_spec.rb b/spec/controllers/openid_connect/user_info_controller_spec.rb index dff0c40380f..4dcacdc4dda 100644 --- a/spec/controllers/openid_connect/user_info_controller_spec.rb +++ b/spec/controllers/openid_connect/user_info_controller_spec.rb @@ -108,6 +108,10 @@ 'first_path_visit' => true, 'events' => { 'OpenID Connect: bearer token authentication' => true }, 'first_event' => true, + 'first_success_state' => true, + 'success_states' => { + 'POST|/api/openid_connect/userinfo|OpenID Connect: bearer token authentication' => true, + }, } expect(request.session.to_h).to eq(session_hash) end diff --git a/spec/services/analytics_spec.rb b/spec/services/analytics_spec.rb index 6ed71f1790f..2403cdd4129 100644 --- a/spec/services/analytics_spec.rb +++ b/spec/services/analytics_spec.rb @@ -23,6 +23,7 @@ let(:current_user) { build_stubbed(:user, uuid: '123') } let(:request) { FakeRequest.new } let(:path) { 'fake_path' } + let(:success_state) { 'GET|fake_path|Trackable Event' } subject(:analytics) do Analytics.new( @@ -48,6 +49,8 @@ git_sha: IdentityConfig::GIT_SHA, git_branch: IdentityConfig::GIT_BRANCH, new_session_path: true, + new_session_success_state: true, + success_state: success_state, new_event: true, path: path, } @@ -66,6 +69,8 @@ git_sha: IdentityConfig::GIT_SHA, git_branch: IdentityConfig::GIT_BRANCH, new_session_path: nil, + new_session_success_state: nil, + success_state: success_state, new_event: nil, path: path, } @@ -86,6 +91,8 @@ locale: I18n.locale, git_sha: IdentityConfig::GIT_SHA, git_branch: IdentityConfig::GIT_BRANCH, + new_session_success_state: true, + success_state: success_state, new_session_path: true, new_event: true, path: path, @@ -124,6 +131,8 @@ new_session_path: true, new_event: true, path: path, + new_session_success_state: true, + success_state: success_state, } expect(ahoy).to receive(:track). diff --git a/spec/support/fake_request.rb b/spec/support/fake_request.rb index 2a8a6cd96d1..868ae19945a 100644 --- a/spec/support/fake_request.rb +++ b/spec/support/fake_request.rb @@ -24,4 +24,8 @@ def cookies def path 'fake_path' end + + def env + { 'REQUEST_METHOD' => 'GET' } + end end