From e0eacf1887ccb1270b50c87be93fd4a263a2fe8d Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Mon, 9 Dec 2024 16:17:22 -0500 Subject: [PATCH 1/7] Expand on form pattern documentation validation, error handling (#11611) * Expand on form pattern documentation validation, error handling changelog: Internal, Documentation, Expand on form pattern documentation validation, error handling * Fix typo Co-authored-by: Mitchell Henke --------- Co-authored-by: Mitchell Henke --- docs/backend.md | 63 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/docs/backend.md b/docs/backend.md index b476f2fd9e0..89cd714fc6c 100644 --- a/docs/backend.md +++ b/docs/backend.md @@ -62,19 +62,68 @@ Forms should have a `#submit` method that returns a `FormResponse`. - `extra:` is, by convention, a method called `extra_analytics_attributes` that returns a Hash -```ruby -def submit - FormResponse.new( - success: valid?, - errors: errors, - extra: extra_analytics_attributes, - ) +By including `ActiveModel::Model`, you can use any of [Rails' built-in model validation helpers](https://guides.rubyonrails.org/active_record_validations.html#validation-helpers) +or define [custom validation logic](https://guides.rubyonrails.org/active_record_validations.html#custom-methods). +Regardless how you validate, you should use human-readable error messages and associate the error to +the specific form parameter field that it affects, if the form is responsible for validating input +from a page. + +```rb +class NewEmailForm + include ActiveModel::Model + include ActionView::Helpers::TranslationHelper + + validates_presence_of :email, { message: proc { I18n.t('errors.email.blank')} } + validate :validate_banned_email + + def submit(email:) + @email = email + + FormResponse.new(success: valid?, errors:, extra: extra_analytics_attributes) + end + + def validate_banned_email + return if !BannedEmail.find_by(email: @email) + errors.add(:email, :banned, message: t('errors.email.banned')) + end + + # ... end ``` For sensitive properties, or results that are not meant to be logged, add properties to the Form object that get written during `#submit` +### Form Error Handling + +If form validation is unsuccessful, you should inform the user what needs to be done to correct the +issue by one or both of the following: + +- Flash message +- Inline field errors + +For convenience, a `FormResponse` object includes a `first_error_message` method which can be used +if you want to display a single error message, such as in a flash banner. + +```rb +result = @form.submit(**params) +if result.success? + # ... +else + flash.now[:error] = result.first_error_message + render :new +end +``` + +In the view, a [SimpleForm](https://github.com/heartcombo/simple_form) form can be bound to a form +object. By doing so, each error will automatically be shown with the corresponding page input. + +```erb +<%= simple_form_for @form, url: emails_path do |f| %> + <%= render ValidatedFieldComponent.new(form: f, name: :email) %> +<% end > +``` + ### Analytics Analytics events are appended to `log/events.log` and contain information both common information as From a1b4c5687739c080cb1d8c66db01956c87b63792 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Mon, 9 Dec 2024 16:50:00 -0500 Subject: [PATCH 2/7] LG-15135 Add an alert when a user needs to connect to their initiating service provider (#11606) This commit adds a new alert to the account page. This alert appears when a user has completed IdV, but has not connected their account to their SP (i.e. they have not gone back to the SP and done a verified sign in since completing IdV). If a link to the SP is available, a link to go back to the SP is provided. changelog: User-Facing Improvements, Account screen, Add a new link to return to the service provider for verified users who have not connected their account yet. --- app/presenters/account_show_presenter.rb | 15 ++++ .../accounts/_identity_verification.html.erb | 11 +++ config/locales/en.yml | 2 + config/locales/es.yml | 2 + config/locales/fr.yml | 2 + config/locales/zh.yml | 2 + spec/features/idv/sp_follow_up_spec.rb | 7 +- .../presenters/account_show_presenter_spec.rb | 41 ++++++++++ .../_identity_verification.html.erb_spec.rb | 79 +++++++++++++++++++ 9 files changed, 160 insertions(+), 1 deletion(-) diff --git a/app/presenters/account_show_presenter.rb b/app/presenters/account_show_presenter.rb index 7f41a989887..7b81f0936c1 100644 --- a/app/presenters/account_show_presenter.rb +++ b/app/presenters/account_show_presenter.rb @@ -82,6 +82,21 @@ def formatted_legacy_idv_date I18n.l(user.active_profile.created_at, format: :event_date) end + def connect_to_initiating_idv_sp_url + initiating_service_provider = user.active_profile&.initiating_service_provider + return nil if !initiating_service_provider.present? + + SpReturnUrlResolver.new(service_provider: initiating_service_provider).post_idv_follow_up_url + end + + def connected_to_initiating_idv_sp? + initiating_service_provider = user.active_profile&.initiating_service_provider + return false if !initiating_service_provider.present? + + identity = user.identities.find_by(service_provider: initiating_service_provider.issuer) + !!identity&.last_ial2_authenticated_at.present? + end + def show_unphishable_badge? MfaPolicy.new(user).unphishable? end diff --git a/app/views/accounts/_identity_verification.html.erb b/app/views/accounts/_identity_verification.html.erb index 80286e1ec4b..437b698b62c 100644 --- a/app/views/accounts/_identity_verification.html.erb +++ b/app/views/accounts/_identity_verification.html.erb @@ -19,6 +19,17 @@ +<% if @presenter.active_profile? && !@presenter.connected_to_initiating_idv_sp? %> + <%= render AlertComponent.new(type: :warning, class: 'margin-bottom-2') do %> + <%= t('account.index.verification.connect_idv_account.intro') %>
+ <% if @presenter.connect_to_initiating_idv_sp_url.present? %> + <%= link_to(t('account.index.verification.connect_idv_account.cta'), @presenter.connect_to_initiating_idv_sp_url) %> + <% else %> + <%= t('account.index.verification.connect_idv_account.cta') %> + <% end %> + <% end %> +<% end %> + <% if @presenter.active_profile? || @presenter.pending_ipp? || @presenter.pending_gpo? %>

<% if @presenter.active_profile_for_authn_context? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index dddee2c7af2..cde8b2451ec 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -75,6 +75,8 @@ account.index.reactivation.instructions: Your profile was recently deactivated d account.index.reactivation.link: Reactivate your profile now. account.index.sign_in_location_and_ip: From %{ip} (IP address potentially located in %{location}) account.index.unknown_location: unknown location +account.index.verification.connect_idv_account.cta: Sign in to partner agency to access services. +account.index.verification.connect_idv_account.intro: Connect your account to the partner agency. account.index.verification.continue_idv: Continue identity verification account.index.verification.finish_verifying_html: Finish verifying your identity to access %{sp_name}. account.index.verification.finish_verifying_no_sp: Finish the identity verification process to gain access to all %{app_name} partners. diff --git a/config/locales/es.yml b/config/locales/es.yml index 2b9ec65d8af..432d79dfa10 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -75,6 +75,8 @@ account.index.reactivation.instructions: Su perfil fue desactivado recientemente account.index.reactivation.link: Reactive su perfil ahora. account.index.sign_in_location_and_ip: Desde %{ip} (la dirección IP se encuentra posiblemente en %{location}) account.index.unknown_location: ubicación desconocida +account.index.verification.connect_idv_account.cta: Inicie sesión en la agencia asociada para acceder a los servicios. +account.index.verification.connect_idv_account.intro: Conecte su cuenta a la agencia asociada. account.index.verification.continue_idv: Continuar la verificación de identidad account.index.verification.finish_verifying_html: Termine de verificar su identidad para acceder a la %{sp_name}. account.index.verification.finish_verifying_no_sp: Termine el proceso de verificación de identidad para obtener acceso a todos los asociados de %{app_name}. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 1e7ef47f685..c744644e554 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -75,6 +75,8 @@ account.index.reactivation.instructions: Votre profil a été récemment désact account.index.reactivation.link: Réactiver votre profil maintenant. account.index.sign_in_location_and_ip: De %{ip} (adresse IP éventuellement située dans %{location}) account.index.unknown_location: lieu inconnu +account.index.verification.connect_idv_account.cta: Connectez-vous à l’organisme partenaire pour accéder à ses services. +account.index.verification.connect_idv_account.intro: Associer votre compte à l’organisme partenaire. account.index.verification.continue_idv: Poursuivre la vérification d’identité account.index.verification.finish_verifying_html: Terminez la procédure de vérification d’identité pour pouvoir accéder à %{sp_name}. account.index.verification.finish_verifying_no_sp: Terminer la procédure de vérification d’identité pour pouvoir accéder à tous les organismes partenaires de %{app_name}. diff --git a/config/locales/zh.yml b/config/locales/zh.yml index d4727ee6bf5..38951fa9ead 100644 --- a/config/locales/zh.yml +++ b/config/locales/zh.yml @@ -75,6 +75,8 @@ account.index.reactivation.instructions: 你的用户资料因为重设密码最 account.index.reactivation.link: 现在重新激活你的用户资料。 account.index.sign_in_location_and_ip: 从 %{ip}（IP 地址可能位于 %{location}）。 account.index.unknown_location: 未知地点 +account.index.verification.connect_idv_account.cta: 登录合作伙伴机构来获得服务。 +account.index.verification.connect_idv_account.intro: 把你的账户连接到合作伙伴机构。 account.index.verification.continue_idv: 继续身份验证 account.index.verification.finish_verifying_html: 完成身份验证流程来获得访问 %{sp_name} 的权限。 account.index.verification.finish_verifying_no_sp: 完成身份验证流程来获得访问%{app_name} 合作伙伴机构的权限。 diff --git a/spec/features/idv/sp_follow_up_spec.rb b/spec/features/idv/sp_follow_up_spec.rb index a8f5da4e6c7..a6e9f39b3bc 100644 --- a/spec/features/idv/sp_follow_up_spec.rb +++ b/spec/features/idv/sp_follow_up_spec.rb @@ -75,7 +75,7 @@ expect(current_url).to eq(post_idv_follow_up_url) end - scenario 'canceling on the CTA' do + scenario 'canceling on the CTA and visiting from the account page' do post_idv_follow_up_url = 'https://example.com/idv_follow_up' initiating_service_provider = create(:service_provider, post_idv_follow_up_url:) profile = create(:profile, :verify_by_mail_pending, :with_pii, initiating_service_provider:) @@ -101,6 +101,11 @@ click_on t('idv.by_mail.sp_follow_up.go_to_account') expect(current_url).to eq(account_url) + + expect(page).to have_content(t('account.index.verification.connect_idv_account.intro')) + click_on(t('account.index.verification.connect_idv_account.cta')) + + expect(current_url).to eq(post_idv_follow_up_url) end end end diff --git a/spec/presenters/account_show_presenter_spec.rb b/spec/presenters/account_show_presenter_spec.rb index 3688bde40c1..8c909b36dd3 100644 --- a/spec/presenters/account_show_presenter_spec.rb +++ b/spec/presenters/account_show_presenter_spec.rb @@ -395,6 +395,47 @@ end end + describe '#connected_to_initiating_idv_sp?' do + let(:initiating_service_provider) { build(:service_provider) } + let(:user) { create(:user, identities: [identity].compact, profiles: [profile].compact) } + let(:profile) do + build(:profile, :active, initiating_service_provider:) + end + let(:last_ial2_authenticated_at) { 2.days.ago } + let(:identity) do + build( + :service_provider_identity, + service_provider: initiating_service_provider.issuer, + last_ial2_authenticated_at:, + ) + end + + subject(:connected_to_initiating_idv_sp?) { presenter.connected_to_initiating_idv_sp? } + + context 'the user verified without an initiating service provider' do + let(:initiating_service_provider) { nil } + let(:identity) { nil } + + it { expect(connected_to_initiating_idv_sp?).to eq(false) } + end + + context 'the user does not have an identity for the initiating service provider' do + let(:identity) { nil } + + it { expect(connected_to_initiating_idv_sp?).to eq(false) } + end + + context 'the user has signed in to the initiating service provider' do + it { expect(connected_to_initiating_idv_sp?).to eq(true) } + end + + context 'the user has not signed in to the initiating service provider' do + let(:last_ial2_authenticated_at) { nil } + + it { expect(connected_to_initiating_idv_sp?).to eq(false) } + end + end + describe '#header_personalization' do context 'AccountShowPresenter instance has decrypted_pii' do it "returns the user's first name" do diff --git a/spec/views/accounts/_identity_verification.html.erb_spec.rb b/spec/views/accounts/_identity_verification.html.erb_spec.rb index 757d73e8208..310243f1b4f 100644 --- a/spec/views/accounts/_identity_verification.html.erb_spec.rb +++ b/spec/views/accounts/_identity_verification.html.erb_spec.rb @@ -53,6 +53,12 @@ ) end end + + it 'does not render alert to connect to IdV SP' do + expect(rendered).to_not have_content( + strip_tags(t('account.index.verification.connect_idv_account.intro')), + ) + end end context 'with user pending ipp verification' do @@ -82,6 +88,12 @@ ) end end + + it 'does not render alert to connect to IdV SP' do + expect(rendered).to_not have_content( + strip_tags(t('account.index.verification.connect_idv_account.intro')), + ) + end end context 'with partner requesting non-facial match verification' do @@ -561,4 +573,71 @@ end end end + + describe 'connect to SP alert' do + let(:post_idv_follow_up_url) { 'https://example.com/followup' } + let(:initiating_service_provider) do + build( + :service_provider, + friendly_name: initiating_sp_name, + post_idv_follow_up_url:, + return_to_sp_url: nil, + ) + end + let(:initiating_sp_name) { 'Test SP' } + let(:user) { create(:user, identities: [identity].compact, profiles: [profile].compact) } + let(:profile) do + build(:profile, :active, initiating_service_provider:) + end + let(:last_ial2_authenticated_at) { nil } + let(:identity) do + build( + :service_provider_identity, + service_provider: initiating_service_provider.issuer, + last_ial2_authenticated_at:, + ) + end + + context 'with a user who has not connected to their initiating service provider' do + context 'the service provider has a post-idv follow-up url' do + it 'renders an alert to connect to IdV SP with a link' do + expect(rendered).to have_content( + t('account.index.verification.connect_idv_account.intro'), + ) + expect(rendered).to have_link( + t('account.index.verification.connect_idv_account.cta'), + href: post_idv_follow_up_url, + ) + end + end + + context 'the service provider does not have a post-idv follow-up url' do + let(:post_idv_follow_up_url) { nil } + + it 'renders an alert to connect to IdV SP without a link' do + expect(rendered).to have_content( + t('account.index.verification.connect_idv_account.intro'), + ) + expect(rendered).to have_content( + t('account.index.verification.connect_idv_account.cta'), + ) + expect(rendered).to_not have_link( + t('account.index.verification.connect_idv_account.cta'), + ) + end + end + end + + context 'with a user who has connected to their initiating service provider' do + let(:last_ial2_authenticated_at) { 2.days.ago } + + it 'does not render alert to connect to IdV SP' do + expect(rendered).to_not have_content( + strip_tags( + t('account.index.verification.connect_idv_account.intro'), + ), + ) + end + end + end end From d11c417875a13968dc20dc6150be6d185522269e Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Tue, 10 Dec 2024 09:33:25 -0500 Subject: [PATCH 3/7] Add Frontend documentation for Images best practices (#11613) changelog: Internal, Dcoumentation, Add Frontend documentation for Images best practices --- docs/frontend.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/frontend.md b/docs/frontend.md index 3b3a48c0d96..219c2c9d3dc 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -155,6 +155,30 @@ how [`Idv::AnalyticsEventEnhancer`][analytics_events_enhancer.rb] is implemented [data_attributes]: https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes [analytics_events_enhancer.rb]: https://github.com/18F/identity-idp/blob/main/app/services/idv/analytics_events_enhancer.rb +## Image Assets + +When possible, use SVG format for images, as these render at higher quality and with a smaller file +size. Most images in the project are either illustrations or icons, which are ideal for vector image +formats (SVG). + +There are few exceptions to this, such as [images used in emails][email-images] needing to be in a +raster format (PNG) due to lack of SVG support in popular email clients. Logos for relying parties +may also be rendered in formats other than SVG, since these are provided to us by partners. + +Image assets saved in source control should be optimized using a lossless image optimizer before +being committed, to ensure they're served to users at the lowest possible file size. This is +[enforced automatically for SVG images][lint-optimized-assets], but must be done manually for other +image types. Consider using a tool like [Squoosh][squoosh] (web) or [ImageOptim][image-optim] +(macOS) for these other image types. + +Since images, GIFs, and videos are artifacts authored in other tools, there is no need to keep +multiple variants of an asset (e.g., SVG and PNG) in the repository if they are not in use. + +[email-images]: https://github.com/18F/identity-idp/tree/main/app/assets/images/email +[lint-optimized-assets]: https://github.com/18F/identity-idp/blob/a1b4c5687739c080cb1d8c66db01956c87b63792/Makefile#L250-L251 +[squoosh]: https://squoosh.app/ +[imageoptim]: https://imageoptim.com/mac + ## Components ### Design System @@ -237,10 +261,6 @@ For example, consider a **Password Input** component: - A web component would be named `PasswordInputElement` - A web components file would be named `app/javascript/packages/password-input/password-input-element.ts` -#### Graphical Assets - -Web graphic assets like images, GIFs, and videos are artifacts authored in other tools. As such, there is no need to keep multiple variants of an asset (e.g., SVG and PNG) in the repository if they are not in use. - ## Testing ### Stylelint From 5a224c8d50e10b1c6d3472382a9330e1979f24a5 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 10 Dec 2024 09:12:09 -0600 Subject: [PATCH 4/7] Validate identity provider public/private keys (#11612) changelog: Internal, OpenID Connect, Validate identity provider public/private keys Co-authored-by: Zach Margolis --- config/initializers/app_artifacts.rb | 7 ++++ lib/openid_connect_key_validation.rb | 10 +++++ .../lib/openid_connect_key_validation_spec.rb | 41 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 lib/openid_connect_key_validation.rb create mode 100644 spec/lib/openid_connect_key_validation_spec.rb diff --git a/config/initializers/app_artifacts.rb b/config/initializers/app_artifacts.rb index d9486fc409b..2f82b258b3a 100644 --- a/config/initializers/app_artifacts.rb +++ b/config/initializers/app_artifacts.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'app_artifacts' +require 'openid_connect_key_validation' AppArtifacts.setup do |store| # When adding or removing certs, make sure to update the 'saml_endpoint_configs' config @@ -12,3 +13,9 @@ store.add_artifact(:oidc_private_key, '/%s/oidc.key') { |k| OpenSSL::PKey::RSA.new(k) } store.add_artifact(:oidc_public_key, '/%s/oidc.pub') { |k| OpenSSL::PKey::RSA.new(k) } end + +valid = OpenidConnectKeyValidation.valid?( + public_key: AppArtifacts.store.oidc_public_key, + private_key: AppArtifacts.store.oidc_private_key, +) +raise 'OIDC Public/Private Keys do not match' if !valid diff --git a/lib/openid_connect_key_validation.rb b/lib/openid_connect_key_validation.rb new file mode 100644 index 00000000000..53fd5f2c8d3 --- /dev/null +++ b/lib/openid_connect_key_validation.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class OpenidConnectKeyValidation + # @param [private_key] OpenSSL::PKey + # @param [public_key] OpenSSL::PKey + def self.valid?(private_key:, public_key:, data: 'abc123') + signature = private_key.sign('SHA256', data) + public_key.verify('SHA256', signature, data) + end +end diff --git a/spec/lib/openid_connect_key_validation_spec.rb b/spec/lib/openid_connect_key_validation_spec.rb new file mode 100644 index 00000000000..d8f21c99e26 --- /dev/null +++ b/spec/lib/openid_connect_key_validation_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe OpenidConnectKeyValidation do + let(:private_key) { OpenSSL::PKey::RSA.generate(1_024) } + + describe '#valid?' do + it 'returns true for a valid public/private key pair' do + public_key = private_key.public_key + valid = OpenidConnectKeyValidation.valid?( + private_key: private_key, + public_key: public_key, + data: '123', + ) + + expect(valid).to eq(true) + end + + it 'returns false for a invalid pair' do + other_private_key = OpenSSL::PKey::RSA.generate(1_024) + public_key = private_key.public_key + valid = OpenidConnectKeyValidation.valid?( + private_key: other_private_key, + public_key: public_key, + data: '123', + ) + + expect(valid).to eq(false) + end + + it 'raises an error if private key and public key are swapped' do + public_key = private_key.public_key + expect do + OpenidConnectKeyValidation.valid?( + private_key: public_key, + public_key: private_key, + data: '123', + ).to raise_error(RuntimeError.new('private key is needed')) + end + end + end +end From 9b8cf529766cdbd3a8c8333064a3cae2b9509655 Mon Sep 17 00:00:00 2001 From: A Shukla Date: Tue, 10 Dec 2024 09:45:38 -0600 Subject: [PATCH 5/7] Lg-14794 reuse valid capture app url (#11555) * Adding functionality and test for standard controller * Adding logic to hybrid controller, pausing to wait for feature test infastructure * Resolving PR comments, spec test fails due to capture app reset error * Resolving PR comments * Resolving PR comments * Fixing rebase * changelog: Upcoming Features, socure, reuse socure valid urls * Resolving PR comments * Fixing test * Resolving pr comments * Resolving PR additions --- .../socure/document_capture_controller.rb | 8 ++- .../idv/socure/document_capture_controller.rb | 9 +-- .../document_capture_controller_spec.rb | 33 +++++++++++ .../document_capture_controller_spec.rb | 34 +++++++++++ .../doc_auth/socure_document_capture_spec.rb | 56 ++++++++++++++++++- 5 files changed, 132 insertions(+), 8 deletions(-) diff --git a/app/controllers/idv/hybrid_mobile/socure/document_capture_controller.rb b/app/controllers/idv/hybrid_mobile/socure/document_capture_controller.rb index 5fa8e17c763..163c4d438d8 100644 --- a/app/controllers/idv/hybrid_mobile/socure/document_capture_controller.rb +++ b/app/controllers/idv/hybrid_mobile/socure/document_capture_controller.rb @@ -20,6 +20,11 @@ def show Funnel::DocAuth::RegisterStep.new(document_capture_user.id, sp_session[:issuer]). call('hybrid_mobile_socure_document_capture', :view, true) + if document_capture_session.socure_docv_capture_app_url.present? + @url = document_capture_session.socure_docv_capture_app_url + return + end + # document request document_request = DocAuth::Socure::Requests::DocumentRequest.new( redirect_url: idv_hybrid_mobile_socure_document_capture_update_url, @@ -40,9 +45,6 @@ def show return end - document_capture_session = DocumentCaptureSession.find_by( - uuid: document_capture_session_uuid, - ) document_capture_session.socure_docv_transaction_token = document_response.dig( :data, :docvTransactionToken, diff --git a/app/controllers/idv/socure/document_capture_controller.rb b/app/controllers/idv/socure/document_capture_controller.rb index 8beee4f8a92..ed4c8f94820 100644 --- a/app/controllers/idv/socure/document_capture_controller.rb +++ b/app/controllers/idv/socure/document_capture_controller.rb @@ -30,6 +30,11 @@ def show Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). call('socure_document_capture', :view, true) + if document_capture_session.socure_docv_capture_app_url.present? + @url = document_capture_session.socure_docv_capture_app_url + return + end + # document request document_request = DocAuth::Socure::Requests::DocumentRequest.new( redirect_url: idv_socure_document_capture_update_url, @@ -50,10 +55,6 @@ def show return end - document_capture_session = DocumentCaptureSession.find_by( - uuid: document_capture_session_uuid, - ) - document_capture_session.socure_docv_transaction_token = document_response.dig( :data, :docvTransactionToken, diff --git a/spec/controllers/idv/hybrid_mobile/socure/document_capture_controller_spec.rb b/spec/controllers/idv/hybrid_mobile/socure/document_capture_controller_spec.rb index c9cfce2b570..380e2a7853a 100644 --- a/spec/controllers/idv/hybrid_mobile/socure/document_capture_controller_spec.rb +++ b/spec/controllers/idv/hybrid_mobile/socure/document_capture_controller_spec.rb @@ -304,6 +304,39 @@ expect(response).to redirect_to(idv_hybrid_mobile_socure_document_capture_errors_url) end end + context 'reuse of valid capture app urls when appropriate' do + let(:fake_capture_app_url) { 'https://verify.socure.test/fake_capture_app' } + let(:socure_capture_app_url) { 'https://verify.socure.test/' } + let(:docv_transaction_token) { '176dnc45d-2e34-46f3-82217-6f540ae90673' } + let(:response_body) do + { + referenceId: '123ab45d-2e34-46f3-8d17-6f540ae90303', + data: { + eventId: 'zoYgIxEZUbXBoocYAnbb5DrT', + docvTransactionToken: docv_transaction_token, + qrCode: 'data:image/png;base64,iVBO......K5CYII=', + url: socure_capture_app_url, + }, + } + end + + before do + allow(request_class).to receive(:new).and_call_original + allow(I18n).to receive(:locale).and_return(expected_language) + end + + it 'does not create a DocumentRequest when valid capture app exists' do + dcs = create( + :document_capture_session, + uuid: user.id, + socure_docv_capture_app_url: fake_capture_app_url, + ) + allow(DocumentCaptureSession).to receive(:find_by).and_return(dcs) + get(:show) + expect(request_class).not_to have_received(:new) + expect(dcs.socure_docv_capture_app_url).to eq(fake_capture_app_url) + end + end end describe '#update' do diff --git a/spec/controllers/idv/socure/document_capture_controller_spec.rb b/spec/controllers/idv/socure/document_capture_controller_spec.rb index 7d494be9516..1e1f1eef25b 100644 --- a/spec/controllers/idv/socure/document_capture_controller_spec.rb +++ b/spec/controllers/idv/socure/document_capture_controller_spec.rb @@ -313,6 +313,40 @@ expect(response).to redirect_to(idv_socure_document_capture_errors_url) end end + + context 'reuse of valid capture app urls when appropriate' do + let(:fake_capture_app_url) { 'https://verify.socure.test/fake_capture_app' } + let(:socure_capture_app_url) { 'https://verify.socure.test/' } + let(:docv_transaction_token) { '176dnc45d-2e34-46f3-82217-6f540ae90673' } + let(:response_body) do + { + referenceId: '123ab45d-2e34-46f3-8d17-6f540ae90303', + data: { + eventId: 'zoYgIxEZUbXBoocYAnbb5DrT', + docvTransactionToken: docv_transaction_token, + qrCode: 'data:image/png;base64,iVBO......K5CYII=', + url: socure_capture_app_url, + }, + } + end + + before do + allow(request_class).to receive(:new).and_call_original + allow(I18n).to receive(:locale).and_return(expected_language) + end + + it 'does not create a DocumentRequest when valid capture app exists' do + dcs = create( + :document_capture_session, + uuid: user.id, + socure_docv_capture_app_url: fake_capture_app_url, + ) + allow(DocumentCaptureSession).to receive(:find_by).and_return(dcs) + get(:show) + expect(request_class).not_to have_received(:new) + expect(dcs.socure_docv_capture_app_url).to eq(fake_capture_app_url) + end + end end describe '#update' do diff --git a/spec/features/idv/doc_auth/socure_document_capture_spec.rb b/spec/features/idv/doc_auth/socure_document_capture_spec.rb index 7b49d706609..e78605c8546 100644 --- a/spec/features/idv/doc_auth/socure_document_capture_spec.rb +++ b/spec/features/idv/doc_auth/socure_document_capture_spec.rb @@ -91,7 +91,61 @@ end end - context 'network connection errors', allow_browser_log: true do + context 'reuses valid capture app urls when appropriate', allow_browser_log: true do + context 'successfully erases capture app url when flow is complete' do + it 'proceeds to the next page with valid info' do + document_capture_session = DocumentCaptureSession.find_by(user_id: @user.id) + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_socure_document_capture_app_url) + expect(page).to have_current_path(fake_socure_document_capture_app_url) + visit idv_socure_document_capture_path + expect(page).to have_current_path(idv_socure_document_capture_path) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_socure_document_capture_app_url) + socure_docv_upload_documents( + docv_transaction_token: @docv_transaction_token, + ) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url).to be_nil + end + + it 'reuse capture app url when appropriate and creates new when not' do + document_capture_session = DocumentCaptureSession.find_by(user_id: @user.id) + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_socure_document_capture_app_url) + expect(page).to have_current_path(fake_socure_document_capture_app_url) + visit idv_socure_document_capture_path + expect(page).to have_current_path(idv_socure_document_capture_path) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_socure_document_capture_app_url) + fake_capture_app2 = 'https://verify.fake-socure.test/capture2' + document_capture_session.socure_docv_capture_app_url = fake_capture_app2 + document_capture_session.save + socure_docv_send_webhook( + docv_transaction_token: @docv_transaction_token, + event_type: 'DOCUMENT_FRONT_UPLOADED', + ) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_capture_app2) + socure_docv_send_webhook( + docv_transaction_token: @docv_transaction_token, + event_type: 'SESSION_EXPIRED', + ) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url).to be_nil + visit idv_socure_document_capture_path + expect(page).to have_current_path(idv_socure_document_capture_path) + document_capture_session.reload + expect(document_capture_session.socure_docv_capture_app_url). + to eq(fake_socure_document_capture_app_url) + end + end + end + + context 'network connection errors' do context 'getting the capture path' do before do allow_any_instance_of(Faraday::Connection).to receive(:post). From 6759e64a29786ba1db1482ecffdad8fa4774f103 Mon Sep 17 00:00:00 2001 From: "Davida (she/they)" Date: Tue, 10 Dec 2024 15:45:01 -0500 Subject: [PATCH 6/7] Allow no certs if block_encryption is set to 'none' (#11616) * changelog: Bug Fixes, SAML Integration, Adding condition to allow no certs if integration has block_encryption set to none --- app/services/saml_request_validator.rb | 1 + spec/controllers/saml_idp_controller_spec.rb | 21 +++++++++ spec/services/saml_request_validator_spec.rb | 46 +++++++++++++------- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/app/services/saml_request_validator.rb b/app/services/saml_request_validator.rb index 3364854d94e..3baaa584367 100644 --- a/app/services/saml_request_validator.rb +++ b/app/services/saml_request_validator.rb @@ -77,6 +77,7 @@ def registered_cert_exists # if there is no service provider, this error has already been added return if service_provider.blank? return if service_provider.certs.present? + return unless service_provider.encrypt_responses? errors.add( :service_provider, :no_cert_registered, diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index b5ad4de18da..8b587bf2753 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -1365,6 +1365,27 @@ def name_id_version(format_urn) ), ) end + + context 'when service provider has block_encryption set to none' do + before do + service_provider.update!(block_encryption: 'none') + end + + it 'is succesful' do + user = create(:user, :fully_registered) + stub_analytics + + generate_saml_response(user, settings) + + expect(response.body).to_not include(t('errors.messages.no_cert_registered')) + expect(@analytics).to have_logged_event( + 'SAML Auth', + hash_including( + success: true, + ), + ) + end + end end context 'service provider has multiple certs' do diff --git a/spec/services/saml_request_validator_spec.rb b/spec/services/saml_request_validator_spec.rb index c6c3e512a7a..5aac4b99285 100644 --- a/spec/services/saml_request_validator_spec.rb +++ b/spec/services/saml_request_validator_spec.rb @@ -51,24 +51,38 @@ context 'when the sp has no certs registered' do before { sp.update!(certs: nil) } - let(:errors) do - { - service_provider: [t('errors.messages.no_cert_registered')], - } - end - let(:error_details) do - { - service_provider: { - no_cert_registered: true, - }, - } + + context 'when it has block_encryption turned on' do + before { sp.update!(block_encryption: 'aes256-cbc') } + let(:errors) do + { + service_provider: [t('errors.messages.no_cert_registered')], + } + end + let(:error_details) do + { + service_provider: { + no_cert_registered: true, + }, + } + end + + it 'returns an error' do + expect(response.to_h).to include( + errors:, + error_details:, + ) + end end - it 'returns an error' do - expect(response.to_h).to include( - errors:, - error_details:, - ) + context 'when block encryption is not turned on' do + it 'is valid' do + expect(response.to_h).to include( + success: true, + errors: {}, + **extra, + ) + end end end From 3200cf8a3676ea1a63600e6f139c20ea43f37f9f Mon Sep 17 00:00:00 2001 From: Howard <3620291+h-m-m@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:22:17 -0500 Subject: [PATCH 7/7] LG-14273: Ensure the in-person verification profile has an enrollment record (#11315) * Ensure the in-person verifcation profile has an enrollment record In theory, it should never happen that the profile is pending verification and the enrollment record does not exist. changelog: Internal, Automated Testing, Improve test setup for enrolling profiles * fix association for in_person_enrollment pending trait and setup for user with_pending_in_person_enrollment trait * fix setup in individual specs * Add and update traits in profile and users factory * Update spec/factories/in_person_enrollments.rb Co-authored-by: Zach Margolis * remove extraneous setup from profile model spec --------- Co-authored-by: Eileen McFarland <80347702+eileen-nava@users.noreply.github.com> Co-authored-by: Zach Margolis --- .../delete_account_controller_spec.rb | 2 +- .../in_person/ready_to_verify_controller_spec.rb | 1 - .../idv/personal_key_controller_spec.rb | 9 ++------- spec/factories/in_person_enrollments.rb | 7 ++++++- spec/factories/profiles.rb | 15 ++++++++++++++- spec/factories/users.rb | 11 ++++------- spec/models/profile_spec.rb | 3 --- spec/views/accounts/show.html.erb_spec.rb | 8 ++++---- 8 files changed, 31 insertions(+), 25 deletions(-) diff --git a/spec/controllers/account_reset/delete_account_controller_spec.rb b/spec/controllers/account_reset/delete_account_controller_spec.rb index 2d5702daf03..66fbb96a24f 100644 --- a/spec/controllers/account_reset/delete_account_controller_spec.rb +++ b/spec/controllers/account_reset/delete_account_controller_spec.rb @@ -167,7 +167,7 @@ success: true, errors: {}, mfa_method_counts: { phone: 1 }, - profile_idv_level: 'legacy_in_person', + profile_idv_level: 'in_person', identity_verified: true, account_age_in_days: 0, account_confirmed_at: user.confirmed_at, diff --git a/spec/controllers/idv/in_person/ready_to_verify_controller_spec.rb b/spec/controllers/idv/in_person/ready_to_verify_controller_spec.rb index 88f77fd2e31..f475e135858 100644 --- a/spec/controllers/idv/in_person/ready_to_verify_controller_spec.rb +++ b/spec/controllers/idv/in_person/ready_to_verify_controller_spec.rb @@ -44,7 +44,6 @@ context 'with enrollment' do let(:user) { create(:user, :with_pending_in_person_enrollment) } - let(:profile) { create(:profile, :with_pii, user: user) } it 'renders show template' do expect(response).to render_template :show diff --git a/spec/controllers/idv/personal_key_controller_spec.rb b/spec/controllers/idv/personal_key_controller_spec.rb index 6f37ec787c1..5128d3cdd01 100644 --- a/spec/controllers/idv/personal_key_controller_spec.rb +++ b/spec/controllers/idv/personal_key_controller_spec.rb @@ -46,8 +46,6 @@ def assert_personal_key_generated_for_profiles(*profile_pii_pairs) let(:address_verification_mechanism) { 'phone' } - let(:in_person_enrollment) { nil } - let(:idv_session) { subject.idv_session } let(:threatmetrix_review_status) { nil } @@ -524,13 +522,10 @@ def assert_personal_key_generated_for_profiles(*profile_pii_pairs) end context 'with in person profile' do - let!(:in_person_enrollment) do - create(:in_person_enrollment, :pending, user: user).tap do - user.reload_pending_in_person_enrollment - end - end + let!(:profile) { create(:profile, :in_person_verification_pending, user: user) } before do + user.reload_pending_in_person_enrollment allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true) end diff --git a/spec/factories/in_person_enrollments.rb b/spec/factories/in_person_enrollments.rb index 268164e51b2..3678ec43901 100644 --- a/spec/factories/in_person_enrollments.rb +++ b/spec/factories/in_person_enrollments.rb @@ -17,7 +17,12 @@ status { :pending } status_updated_at { Time.zone.now } profile do - association(:profile, :in_person_verification_pending, user: user) + association( + :profile, + :in_person_verification_pending, + user: user, + in_person_enrollment: instance, + ) end end diff --git a/spec/factories/profiles.rb b/spec/factories/profiles.rb index c1483a146d4..c20c42667d2 100644 --- a/spec/factories/profiles.rb +++ b/spec/factories/profiles.rb @@ -37,7 +37,20 @@ trait :in_person_verification_pending do in_person_verification_pending_at { 15.days.ago } - idv_level { :legacy_in_person } + idv_level { :in_person } + in_person_enrollment do + association(:in_person_enrollment, :pending, profile: instance, user:) + end + end + + trait :in_person_verified do + verified_at { Time.zone.now } + activated_at { Time.zone.now } + idv_level { :in_person } + in_person_verification_pending_at { nil } + in_person_enrollment do + association(:in_person_enrollment, :passed, profile: instance, user:) + end end trait :fraud_pending_reason do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 15ed22e6303..893adf1921d 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -221,9 +221,8 @@ end trait :with_pending_in_person_enrollment do - after :build do |user| - profile = create(:profile, :with_pii, :in_person_verification_pending, user: user) - create(:in_person_enrollment, :pending, user: user, profile: profile) + profiles do + [association(:profile, :with_pii, :in_person_verification_pending, user: instance)] end end @@ -270,16 +269,14 @@ confirmed_at { Time.zone.now.round } after :build do |user| - profile = create( + create( :profile, :with_pii, :active, :verified, - :in_person_verification_pending, + :in_person_verified, user: user, ) - create(:in_person_enrollment, :passed, user: user, profile: profile) - profile.in_person_verification_pending_at = nil end end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index 336c6eee4d4..10a5cc1aa64 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -612,9 +612,6 @@ describe '#deactivate_due_to_encryption_error' do context 'when the profile has a "pending" in_person_enrollment' do subject { create(:profile, :in_person_verification_pending, user: user) } - let!(:enrollment) do - create(:in_person_enrollment, user: user, profile: subject, status: :pending) - end before do subject.deactivate_due_to_encryption_error diff --git a/spec/views/accounts/show.html.erb_spec.rb b/spec/views/accounts/show.html.erb_spec.rb index 92aacb09d24..443ba48fa41 100644 --- a/spec/views/accounts/show.html.erb_spec.rb +++ b/spec/views/accounts/show.html.erb_spec.rb @@ -95,7 +95,7 @@ end context 'when current user has ipp pending profile' do - let(:user) { build(:user, :with_pending_in_person_enrollment) } + let(:user) { create(:user, :with_pending_in_person_enrollment) } it 'renders idv partial' do expect(render).to render_template(partial: 'accounts/_identity_verification') @@ -114,7 +114,7 @@ context 'when current user has an in_person_enrollment that was failed' do let(:vtr) { ['Pe'] } let(:sp_name) { 'sinatra-test-app' } - let(:user) { build(:user, :with_pending_in_person_enrollment) } + let(:user) { create(:user, :with_pending_in_person_enrollment) } before do # Make the in_person_enrollment and associated profile failed @@ -132,7 +132,7 @@ context 'when current user has an in_person_enrollment that was cancelled' do let(:vtr) { ['Pe'] } let(:sp_name) { 'sinatra-test-app' } - let(:user) { build(:user, :with_pending_in_person_enrollment) } + let(:user) { create(:user, :with_pending_in_person_enrollment) } before do # Make the in_person_enrollment and associated profile cancelled @@ -150,7 +150,7 @@ context 'when current user has an in_person_enrollment that expired' do let(:vtr) { ['Pe'] } let(:sp_name) { 'sinatra-test-app' } - let(:user) { build(:user, :with_pending_in_person_enrollment) } + let(:user) { create(:user, :with_pending_in_person_enrollment) } before do # Expire the in_person_enrollment and associated profile