From a47eab54b5300d415ff9a4d5dbe8abef74de5950 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 16 Jul 2024 13:30:43 -0400 Subject: [PATCH 01/12] LG-13123 Log when `GpoConfirmationUploader` uploads `GpoConfirmations` (#10943) We have observed a number of issues with the `GpoConfirmationUploader`. We are adding more visibility to the job to improve our monitoring and alerting to deal with failures that come up. This commit adds a line to `events.log` when confirmations get uploaded. It includes a count of the confirmations that were uploaded so we can build a metric for that. This new logging is in addition to the records that get written to the `letter_requests_to_usps_ftp_logs` which are used in monthly reports. [skip changelog] --- app/services/analytics_events.rb | 19 +++++++++++++++++++ app/services/gpo_confirmation_uploader.rb | 10 ++++++++++ .../gpo_confirmation_uploader_spec.rb | 18 +++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index e68db8eb358..37b9e38758a 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -670,6 +670,25 @@ def fraud_review_rejected( ) end + # @param [Boolean] success Whether records were successfully uploaded + # @param [String] exception The exception that occured if an exception did occur + # @param [Number] gpo_confirmation_count The number of GPO Confirmation records uploaded + # GPO confirmation records were uploaded for letter sends + def gpo_confirmation_upload( + success:, + exception:, + gpo_confirmation_count:, + **extra + ) + track_event( + :gpo_confirmation_upload, + success: success, + exception: exception, + gpo_confirmation_count: gpo_confirmation_count, + **extra, + ) + end + # @param [Boolean] acuant_sdk_upgrade_a_b_testing_enabled # @param [String] acuant_version # @param [String] flow_path whether the user is in the hybrid or standard flow diff --git a/app/services/gpo_confirmation_uploader.rb b/app/services/gpo_confirmation_uploader.rb index 54422aba58c..4f16d983a6c 100644 --- a/app/services/gpo_confirmation_uploader.rb +++ b/app/services/gpo_confirmation_uploader.rb @@ -12,7 +12,13 @@ def run upload_export(export) LetterRequestsToGpoFtpLog.create(ftp_at: @now, letter_requests_count: confirmations.count) clear_confirmations(confirmations) + analytics.gpo_confirmation_upload( + success: true, exception: nil, gpo_confirmation_count: confirmations.count, + ) rescue StandardError => error + analytics.gpo_confirmation_upload( + success: false, exception: error.to_s, gpo_confirmation_count: 0, + ) NewRelic::Agent.notice_error(error) raise error end @@ -71,4 +77,8 @@ def sftp_config timeout: IdentityConfig.store.usps_upload_sftp_timeout, ] end + + def analytics + Analytics.new(user: AnonymousUser.new, request: nil, session: {}, sp: nil) + end end diff --git a/spec/services/gpo_confirmation_uploader_spec.rb b/spec/services/gpo_confirmation_uploader_spec.rb index 8b95c27fb22..0c32972be0e 100644 --- a/spec/services/gpo_confirmation_uploader_spec.rb +++ b/spec/services/gpo_confirmation_uploader_spec.rb @@ -23,8 +23,11 @@ ] end + let(:job_analytics) { FakeAnalytics.new } + before do allow(IdentityConfig.store).to receive(:usps_upload_enabled).and_return(true) + allow(uploader).to receive(:analytics).and_return(job_analytics) end describe '#generate_export' do @@ -111,13 +114,20 @@ log = logs.first expect(log.ftp_at).to be_present expect(log.letter_requests_count).to eq(1) + expect(job_analytics).to have_logged_event( + :gpo_confirmation_upload, + success: true, + exception: nil, + gpo_confirmation_count: confirmations.count, + ) end end context 'when there is an error' do it 'notifies NewRelic and does not clear confirmations if SFTP fails' do expect(uploader).to receive(:generate_export).with(confirmations).and_return(export) - expect(uploader).to receive(:upload_export).with(export).and_raise(StandardError) + expect(uploader).to receive(:upload_export).with(export). + and_raise(StandardError, 'test error') expect(uploader).not_to receive(:clear_confirmations) expect(NewRelic::Agent).to receive(:notice_error) @@ -125,6 +135,12 @@ expect { subject }.to raise_error expect(GpoConfirmation.count).to eq 1 + expect(job_analytics).to have_logged_event( + :gpo_confirmation_upload, + success: false, + exception: 'test error', + gpo_confirmation_count: 0, + ) end end From a377632ee391c6759ed7bb53b895066489f22347 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 16 Jul 2024 16:03:54 -0400 Subject: [PATCH 02/12] LG-13820 Redirect from request letter controller when letter send is not availble (#10945) This commit fixes a bug where the `RequestLetterController` was not respecting `GpoVerifyByMailPolicy#send_letter_available?`. If that method returned false then links would be hidden but users could still visit the controller directly and request letters. This commit adds a before action to fix the issue and adds tests. [skip changelog] --- .../idv/by_mail/request_letter_controller.rb | 7 +++- .../by_mail/request_letter_controller_spec.rb | 9 +++++ spec/features/idv/gpo_disabled_spec.rb | 35 ++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/by_mail/request_letter_controller.rb b/app/controllers/idv/by_mail/request_letter_controller.rb index b071093cf9c..b7799d71181 100644 --- a/app/controllers/idv/by_mail/request_letter_controller.rb +++ b/app/controllers/idv/by_mail/request_letter_controller.rb @@ -10,6 +10,7 @@ class RequestLetterController < ApplicationController before_action :confirm_mail_not_rate_limited before_action :confirm_step_allowed + before_action :confirm_letter_sends_allowed def index @applicant = idv_session.applicant @@ -33,7 +34,7 @@ def self.step_info action: :index, next_steps: [:enter_password], preconditions: ->(idv_session:, user:) do - idv_session.verify_info_step_complete? || user.gpo_verification_pending_profile? + idv_session.verify_info_step_complete? end, undo_step: ->(idv_session:, user:) { idv_session.address_verification_mechanism = nil }, ) @@ -55,6 +56,10 @@ def confirm_mail_not_rate_limited redirect_to idv_enter_password_url if gpo_verify_by_mail_policy.rate_limited? end + def confirm_letter_sends_allowed + redirect_to idv_enter_password_url if !gpo_verify_by_mail_policy.send_letter_available? + end + def step_indicator_steps if in_person_proofing? Idv::Flows::InPersonFlow::STEP_INDICATOR_STEPS_GPO diff --git a/spec/controllers/idv/by_mail/request_letter_controller_spec.rb b/spec/controllers/idv/by_mail/request_letter_controller_spec.rb index bcee2c46ed0..5ff58bb1acd 100644 --- a/spec/controllers/idv/by_mail/request_letter_controller_spec.rb +++ b/spec/controllers/idv/by_mail/request_letter_controller_spec.rb @@ -63,6 +63,15 @@ expect(response).to redirect_to idv_enter_password_path end + + it 'redirects if the user is not allowed to send mail' do + allow(controller.gpo_verify_by_mail_policy).to receive(:send_letter_available?). + and_return(false) + + get :index + + expect(response).to redirect_to idv_enter_password_path + end end describe '#create' do diff --git a/spec/features/idv/gpo_disabled_spec.rb b/spec/features/idv/gpo_disabled_spec.rb index 322ed785490..37c90e4f829 100644 --- a/spec/features/idv/gpo_disabled_spec.rb +++ b/spec/features/idv/gpo_disabled_spec.rb @@ -17,7 +17,7 @@ Rails.application.reload_routes! end - it 'allows verification without the option to confirm address with usps', js: true do + it 'allows verification without the option to confirm address with usps', :js do user = user_with_2fa start_idv_from_sp complete_idv_steps_before_phone_step(user) @@ -36,4 +36,37 @@ expect(page).to have_current_path(sign_up_completed_path) end end + + context 'with GPO address verification disallowed for biometric comparison' do + before do + allow(IdentityConfig.store).to receive(:no_verify_by_mail_for_biometric_comparison_enabled). + and_return(true) + allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).and_return(true) + end + + it 'does not allow verify by mail with biometric comparison', :js do + user = user_with_2fa + start_idv_from_sp(:oidc, biometric_comparison_required: true) + sign_in_and_2fa_user(user) + complete_all_doc_auth_steps(with_selfie: true) + + # Link to the GPO flow should not be visible + expect(page).to_not have_content(t('idv.troubleshooting.options.verify_by_mail')) + + # Directly visiting the verify my mail path does not allow the user to request a letter + visit idv_request_letter_path + expect(page).to have_current_path(idv_phone_path) + end + + it 'does allow verify by mail without biometric comparison', :js do + user = user_with_2fa + start_idv_from_sp(:oidc, biometric_comparison_required: false) + sign_in_and_2fa_user(user) + complete_all_doc_auth_steps(with_selfie: false) + click_on t('idv.troubleshooting.options.verify_by_mail') + + # The user is allowed to visit the request letter path + expect(page).to have_current_path(idv_request_letter_path) + end + end end From f6cc9943ff8459bd54cbfc5830ab399f85d4c81e Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:16:53 -0400 Subject: [PATCH 03/12] Add spec to ensure mailers don't include SVG (#10949) changelog: Internal, Automated Testing, Add spec to ensure mailers don't include SVG --- spec/mailers/previews/user_mailer_preview_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/mailers/previews/user_mailer_preview_spec.rb b/spec/mailers/previews/user_mailer_preview_spec.rb index f0ae9d81b88..e82fd5c6e38 100644 --- a/spec/mailers/previews/user_mailer_preview_spec.rb +++ b/spec/mailers/previews/user_mailer_preview_spec.rb @@ -4,8 +4,17 @@ RSpec.describe UserMailerPreview do UserMailerPreview.instance_methods(false).each do |mailer_method| describe "##{mailer_method}" do + subject(:mail) { UserMailerPreview.new.public_send(mailer_method) } + it 'generates a preview without blowing up' do - expect { UserMailerPreview.new.public_send(mailer_method).body }.to_not raise_error + expect { mail.body }.to_not raise_error + end + + it 'does not include any svg images' do + # SVGs are typically the preferred format for their high-quality and small file size, but + # they are not well-supported in email clients. Instead, store a rasterized version of the + # image in `app/assets/images/email` for use in mailer content. + expect(mail.html_part.body).not_to have_selector("img[src$='.svg']") end end end From 9ab3079a2ba17550dddb9b068e6ff64bc9ae53f6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:46:51 -0400 Subject: [PATCH 04/12] Split utilities stylesheet to standalone file (#10946) * Split utilities stylesheet to standalone file changelog: Internal, Performance, Split utilities stylesheet to standalone file * Fix typos on utilities stylesheet Co-authored-by: Mitchell Henke --------- Co-authored-by: Mitchell Henke --- Makefile | 2 +- app/assets/stylesheets/application.css.scss | 2 -- app/assets/stylesheets/utilities.css.scss | 3 +++ app/views/layouts/base.html.erb | 3 ++- app/views/layouts/component_preview.html.erb | 3 ++- app/views/saml_idp/shared/saml_post_binding.html.erb | 3 ++- 6 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 app/assets/stylesheets/utilities.css.scss diff --git a/Makefile b/Makefile index eaa8c63d553..e1c37a66d90 100644 --- a/Makefile +++ b/Makefile @@ -134,7 +134,7 @@ lint_asset_bundle_size: ## Lints JavaScript and CSS compiled bundle size @# and you have no options to split that from the common bundles. If you need to increase this @# budget and accept the fact that this will force end-users to endure longer load times, you @# should set the new budget to within a few thousand bytes of the production-compiled size. - find app/assets/builds/application.css -size -185000c | grep . + find app/assets/builds/application.css -size -105000c | grep . find public/packs/application-*.digested.js -size -5000c | grep . lint_migrations: diff --git a/app/assets/stylesheets/application.css.scss b/app/assets/stylesheets/application.css.scss index 6015355f074..86437370173 100644 --- a/app/assets/stylesheets/application.css.scss +++ b/app/assets/stylesheets/application.css.scss @@ -3,5 +3,3 @@ @forward 'uswds'; @forward 'design-system-waiting-room'; @forward 'components'; -@forward 'uswds-utilities'; -@forward 'utilities'; diff --git a/app/assets/stylesheets/utilities.css.scss b/app/assets/stylesheets/utilities.css.scss new file mode 100644 index 00000000000..724dbfd7338 --- /dev/null +++ b/app/assets/stylesheets/utilities.css.scss @@ -0,0 +1,3 @@ +@forward 'uswds-core'; +@forward 'uswds-utilities'; +@forward 'utilities'; diff --git a/app/views/layouts/base.html.erb b/app/views/layouts/base.html.erb index 2ec6e6e4247..648cf73d3a0 100644 --- a/app/views/layouts/base.html.erb +++ b/app/views/layouts/base.html.erb @@ -20,8 +20,9 @@ <% end %> <%= preload_link_tag font_path('public-sans/PublicSans-Bold.woff2') %> <%= preload_link_tag font_path('public-sans/PublicSans-Regular.woff2') %> - <%= render_stylesheet_once_tags %> <%= stylesheet_link_tag 'application', nopush: false %> + <%= render_stylesheet_once_tags %> + <%= stylesheet_link_tag 'utilities', nopush: false %> <%= stylesheet_link_tag 'print', media: :print, preload_links_header: false %> <%= csrf_meta_tags %> diff --git a/app/views/layouts/component_preview.html.erb b/app/views/layouts/component_preview.html.erb index d5662395c30..90344bed5f9 100644 --- a/app/views/layouts/component_preview.html.erb +++ b/app/views/layouts/component_preview.html.erb @@ -2,8 +2,9 @@ Component Preview + <%= stylesheet_link_tag 'application', nopush: false %> <%= render_stylesheet_once_tags %> - <%= stylesheet_link_tag 'application', media: 'all' %> + <%= stylesheet_link_tag 'utilities', nopush: false %> <% if params.dig(:lookbook, :display, :form) == true %> diff --git a/app/views/saml_idp/shared/saml_post_binding.html.erb b/app/views/saml_idp/shared/saml_post_binding.html.erb index ae607743875..faa56cf208b 100644 --- a/app/views/saml_idp/shared/saml_post_binding.html.erb +++ b/app/views/saml_idp/shared/saml_post_binding.html.erb @@ -7,8 +7,9 @@ document.documentElement.classList.replace('no-js', 'js'); <% end %> <%= csrf_meta_tags %> - <%= stylesheet_link_tag 'application', media: 'all' %> + <%= stylesheet_link_tag 'application', nopush: false %> <%= render_stylesheet_once_tags %> + <%= stylesheet_link_tag 'utilities', nopush: false %>
From ea8a6081961d6c373a870dd5fea31efce89fde7e Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 17 Jul 2024 10:33:24 -0400 Subject: [PATCH 05/12] Allow OIDC routes to be localizable (#10941) changelog: Bug Fixes, OpenID Connect, Fix language selection on OpenID Connect logout page --- config/routes.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index cd0391f6206..02c77337a03 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,11 +53,6 @@ post '/api/verify/images' => 'idv/image_uploads#create' post '/api/logger' => 'frontend_log#create' - get '/openid_connect/authorize' => 'openid_connect/authorization#index' - get '/openid_connect/logout' => 'openid_connect/logout#show' - post '/openid_connect/logout' => 'openid_connect/logout#create' - delete '/openid_connect/logout' => 'openid_connect/logout#delete' - get '/robots.txt' => 'robots#index' get '/no_js/detect.css' => 'no_js#index', as: :no_js_detect_css @@ -275,6 +270,11 @@ get '/account/personal_key' => 'accounts/personal_keys#new', as: :create_new_personal_key post '/account/personal_key' => 'accounts/personal_keys#create' + get '/openid_connect/authorize' => 'openid_connect/authorization#index' + get '/openid_connect/logout' => 'openid_connect/logout#show' + post '/openid_connect/logout' => 'openid_connect/logout#create' + delete '/openid_connect/logout' => 'openid_connect/logout#delete' + get '/otp/send' => 'users/two_factor_authentication#send_code' get '/authentication_methods_setup' => 'users/two_factor_authentication_setup#index' From 87c1d60e757d22ccebd1f308451549d4dc684d23 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:25:26 -0400 Subject: [PATCH 06/12] Bump rexml to resolve security advisory (#10953) * Bump rexml to resolve security advisory changelog: Internal, Dependencies, Update dependencies to resolve security advisories * Add rexml as explicit dependency Since we use it in our code, it should be an explicit dependency See: https://github.com/18F/identity-idp/blob/ea8a6081961d6c373a870dd5fea31efce89fde7e/app/services/proofing/aamva/request/verification_request.rb#L60-L102 * Sync AAMVA fixture to expected output Likely a result of https://github.com/ruby/rexml/pull/164 --- Gemfile | 1 + Gemfile.lock | 3 ++- spec/fixtures/proofing/aamva/requests/verification_request.xml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index a645422ddb6..6e20900ec6b 100644 --- a/Gemfile +++ b/Gemfile @@ -66,6 +66,7 @@ gem 'redacted_struct' gem 'redis', '>= 3.2.0' gem 'redis-session-store', github: '18F/redis-session-store', tag: 'v1.0.1-18f' gem 'retries' +gem 'rexml', '~> 3.3' gem 'rotp', '~> 6.3', '>= 6.3.0' gem 'rqrcode' gem 'ruby-progressbar' diff --git a/Gemfile.lock b/Gemfile.lock index 26a02875269..71f99fe949b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -573,7 +573,7 @@ GEM actionpack (>= 5.0) railties (>= 5.0) retries (0.0.5) - rexml (3.3.1) + rexml (3.3.2) strscan rotp (6.3.0) rouge (4.2.0) @@ -832,6 +832,7 @@ DEPENDENCIES redis (>= 3.2.0) redis-session-store! retries + rexml (~> 3.3) rotp (~> 6.3, >= 6.3.0) rqrcode rspec (~> 3.13.0) diff --git a/spec/fixtures/proofing/aamva/requests/verification_request.xml b/spec/fixtures/proofing/aamva/requests/verification_request.xml index 8321a8aba2c..32d5a25010f 100644 --- a/spec/fixtures/proofing/aamva/requests/verification_request.xml +++ b/spec/fixtures/proofing/aamva/requests/verification_request.xml @@ -34,4 +34,4 @@ - + \ No newline at end of file From 88a2a1c166d3598461736abc349c5c7748fabf5a Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 17 Jul 2024 18:51:01 +0000 Subject: [PATCH 07/12] Remove 2023 SAML certificates (#10954) * Remove 2023 SAML certificates changelog: Internal, Maintenance, Remove 2023 SAML certificates * remove 2023 files --- config/artifacts.example/local/saml2023.crt | 21 ------------- .../artifacts.example/local/saml2023.key.enc | 30 ------------------- config/initializers/app_artifacts.rb | 2 -- 3 files changed, 53 deletions(-) delete mode 100644 config/artifacts.example/local/saml2023.crt delete mode 100644 config/artifacts.example/local/saml2023.key.enc diff --git a/config/artifacts.example/local/saml2023.crt b/config/artifacts.example/local/saml2023.crt deleted file mode 100644 index 914a417e994..00000000000 --- a/config/artifacts.example/local/saml2023.crt +++ /dev/null @@ -1,21 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDajCCAlICCQCbZpJCM572hTANBgkqhkiG9w0BAQsFADB3MQswCQYDVQQGEwJV -UzEdMBsGA1UECAwURGlzdHJpY3Qgb2YgQ29sdW1iaWExEzARBgNVBAcMCldhc2hp -bmd0b24xDDAKBgNVBAoMA0dTQTESMBAGA1UECwwJTG9naW4uZ292MRIwEAYDVQQD -DAlsb2dpbi5nb3YwHhcNMjMwNDA0MTYwNzIxWhcNMjQwNDAyMTYwNzIxWjB3MQsw -CQYDVQQGEwJVUzEdMBsGA1UECAwURGlzdHJpY3Qgb2YgQ29sdW1iaWExEzARBgNV -BAcMCldhc2hpbmd0b24xDDAKBgNVBAoMA0dTQTESMBAGA1UECwwJTG9naW4uZ292 -MRIwEAYDVQQDDAlsb2dpbi5nb3YwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQCdToDm3/j0j0PMjca8bc7H0H3FNSTW4l6hpUSywkC/kg2fZ5W6f0hIYYID -TbDYAkpeIiKKE/FDI3TlaQT9+LLe6AbkelLmteS+wMehCPtaBPeRfHKaRNQKsSTk -c5JAf4OWaZKj+F3Fu0e5+dJ2nuYcT2VV7DLoG3KKTw+pcHuXCQZfrPbquyyNbKvo -K4ELVIueQQ5F3EiahP3XchGw+H5FCH5QJPVl57WaCB2gLM8kueELKIzta7roIYHf -GEhdaC71ZYCjGRvsKtAqomNdL2Je67E56dEwJ1fWS242PkSvQTH5vtkYzelE2H9m -V+sPf5lLfc599iLZoTemEe5p6NydAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAH88 -Yl+KbT3NPjrXH7SITWr1wOIemJ8b2/vukz7aS9TiTJnlw6IzsLnD+tiJa1q5CO23 -3/NCaa6piocbD1fC/H7PB6lXu+0ypMwpaStTThpxbpQ6bMUklxKKFyuaX5RpNZn4 -YicYGEnCr7h70+R/01ztgYNOzNdgM6MjHMvEnb8KVkuckuCE1JUoX+LxaE9cxkxX -Auwdct14efBFuyB2HgvWUvqvjN9NDhfS6BG5FgTZWpWJnn7xmjUNUfq1VC4XHQsv -mqoPiDhR1GwB191ZVz7Rq00yysfr7tSUJeWp//5GPZRjSZsrs1wtO6x/tFjngELl -d0/LPNS3OWvaMvvGzgc= ------END CERTIFICATE----- diff --git a/config/artifacts.example/local/saml2023.key.enc b/config/artifacts.example/local/saml2023.key.enc deleted file mode 100644 index c1c1a03fa5d..00000000000 --- a/config/artifacts.example/local/saml2023.key.enc +++ /dev/null @@ -1,30 +0,0 @@ ------BEGIN ENCRYPTED PRIVATE KEY----- -MIIFHzBJBgkqhkiG9w0BBQ0wPDAbBgkqhkiG9w0BBQwwDgQIfgN2KJrydvECAggA -MB0GCWCGSAFlAwQBKgQQEF8wtPY1ps4bJfQ4sV0xeQSCBND598BOsn9cAgqBGUEE -yQgCp+DIpDKFmgwXOiFjjuY1H3vsAFh0lhOoWN2z2UsAWUq8RwWs41UNSajgUllq -wPP3dm+U2BG661AmZEj5+fJ6ckCLX+BZHcv2bjx7cRF8oMv3Z1wlmTvxyARcFlct -b5Rxjl+moDwuBGFUaCg80hL2NZmUSuf/GZ5bj5yHZqGdc/KAXRts6knqLfsBxhrh -uECYUaz11NRN+JCNg8TcQSUJs01QVrhS7MrNhr1WGIwdIYg0AsqHqluzKr4nzjqG -u8vfRV7oraUhNDKrfXfxjGcyy/o4zYnuJO3zBRWPV4Ueesf3efGY3bNyZzoY2BrD -ZlpQwHf7NmANZqHnCAH9dDrk3mcNkfPEnSs/oguhIk05ZHMaZeGR+bIJd1LjxjLY -KOhgw9u+I4WR35GsZRFArwUnHfvAcLTwBbsChMk7ykj16F/rdIknMWiyZX3p31Cm -ZW/tB5j4lm9caA9iPn/d3S/H7O3zG3SSuwQvrwQguImqEegMq2FA6MLT+66R+DU/ -AB/M8WgwIhaLWr136GhGBjJO4Tm21DrNjHzS8NYWcXL8PWdDgagwbTGAL6jLLulr -iCrkEsQoNzX11WiwYgmhrPLKVqophHK4v3dzuY+naMV5GekqhPA+8IfWZfvmizb7 -LVoxb6TKBI0U3p31lhRI9car+Yxa7z5ZSAl0n03TCeZOYvUmKAwj81E23g1XC3Fx -+xB+nrDWQQjvD+ZRXQxqUg58o0qitM9tKsIkh2Qj4IRpATTyTyNO40seGni8AWsU -ftsll8OVWEdHlnOGLNjSPC/uVMYSa8YOcv+xIqErty2Q40yrOsXufX0ompzONNZH -rUbifoY3T8En88m7wCDBQAw38pUdgPniL0VyADdvkc4To5pjkK7B4g7fD+WbjdrD -V7UhU+4wxZkSK9ijaq/4Cj/ZIg5m29qOoCtI/vn2A/DnhLJmomQ8B21xgtUsa/pM -Pi1Z4rz2OLEND1EPuJSm/xijxVGUgYpkOYHoRQmvbpPst9FvXPIXcBdpahbVvqOq -Iv8LcKatTKqTjxyXoOxxUWQ16MYXaOSk8sEbDYqp/YK2N9ItIPiHiYio/Wt+uQM8 -/VbyZGY+e0O9mAGtoWLDuqCziH5HHXjH7sX0mJmrVCc8V7eKewEEYT6jCjvOMG+V -tABCJQXoaOV8Tk5f3rMDVYg+DQaqFfuvOEahj0CLm4xLuEK9mGvNz7HjxgPjRfS1 -CFn93oBKnhJbzeEkNEHWw2sG1/twiOjhSkMs5CbdcpAjm6wNoKE3/e23HbUuFGos -yOb/2mm5oAr/EjBCUSilyNSkIuUTzIKnootLJ4bxHmTHFlpJlYvFBKWT7RIRT21g -LErRAgUHF/f93yNjgEbdzf8lzKfYWBnZK1E2RGbVay3W1OtQQ7Om01CbKd0THt9y -4bwD/rS29+xa4NVImMXmq/aftLLoedaY9TWHz2FOInXhms9vw0wsehmc1aoQ1yrl -DVx+T7raO6YJ8MK385ryjupsZd4J5dYcvHjlkpnZOyvVyzfdNqHvVhtfd765d5rw -GyX2UKPByjECQskxKjeqadjZ8zaAecW/4ujg0wcmdX7l4lfXcdy6+0WiPr3qBwQE -hrcBC92oNEzyorgjdcMEz2RATw== ------END ENCRYPTED PRIVATE KEY----- diff --git a/config/initializers/app_artifacts.rb b/config/initializers/app_artifacts.rb index d9486fc409b..42afaad14f6 100644 --- a/config/initializers/app_artifacts.rb +++ b/config/initializers/app_artifacts.rb @@ -4,8 +4,6 @@ AppArtifacts.setup do |store| # When adding or removing certs, make sure to update the 'saml_endpoint_configs' config - store.add_artifact(:saml_2023_cert, '/%s/saml2023.crt') - store.add_artifact(:saml_2023_key, '/%s/saml2023.key.enc') store.add_artifact(:saml_2024_cert, '/%s/saml2024.crt') store.add_artifact(:saml_2024_key, '/%s/saml2024.key.enc') From ad97bfe12284c3e8887b1a5a6b9bb30717694a53 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:01:47 -0400 Subject: [PATCH 08/12] Align PIV/CAC setup-after-sign-in specs to user behavior (#10939) * Align PIV/CAC setup-after-sign-in specs to user behavior changelog: Internal, Automated Testing, Align PIV/CAC setup-after-sign-in specs to user behavior * Remove unused code for PIV/CAC token processing --- .../piv_cac_setup_from_sign_in_controller.rb | 67 +------------- app/services/analytics_events.rb | 11 ++- config/routes.rb | 2 - .../setup_piv_cac_after_sign_in_spec.rb | 82 +++++++++++++++++ spec/features/users/sign_in_spec.rb | 89 ------------------- spec/support/features/session_helper.rb | 30 ++++++- 6 files changed, 120 insertions(+), 161 deletions(-) create mode 100644 spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb diff --git a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb b/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb index 16c6733047f..99e37c8ad16 100644 --- a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb +++ b/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb @@ -2,85 +2,20 @@ module Users class PivCacSetupFromSignInController < ApplicationController - include TwoFactorAuthenticatableMethods include PivCacConcern - include SecureHeadersConcern include ReauthenticationRequiredConcern before_action :confirm_two_factor_authenticated before_action :confirm_recently_authenticated_2fa - before_action :apply_secure_headers_override, only: :success before_action :set_piv_cac_setup_csp_form_action_uris, only: :prompt def prompt - if params.key?(:token) - process_piv_cac_setup - else - render_prompt - end - end - - def success; end - - def next - redirect_to after_sign_in_path_for(current_user) + analytics.piv_cac_setup_visited(in_account_creation_flow: false) end def decline session.delete(:needs_to_setup_piv_cac_after_sign_in) redirect_to after_sign_in_path_for(current_user) end - - private - - def render_prompt - analytics.piv_cac_setup_visited(in_account_creation_flow: false) - render :prompt - end - - def process_piv_cac_setup - result = user_piv_cac_form.submit - properties = result.to_h.merge(analytics_properties) - analytics.multi_factor_auth_setup(**properties) - if result.success? - process_valid_submission - else - process_invalid_submission - end - end - - def user_piv_cac_form - @user_piv_cac_form ||= UserPivCacSetupForm.new( - user: current_user, - token: params[:token], - nonce: piv_cac_nonce, - name: user_session[:piv_cac_nickname], - ) - end - - def process_invalid_submission - redirect_to login_piv_cac_error_url(error: user_piv_cac_form.error_type) - end - - def process_valid_submission - handle_valid_verification_for_confirmation_context( - auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, - ) - session.delete(:needs_to_setup_piv_cac_after_sign_in) - save_piv_cac_information( - subject: user_piv_cac_form.x509_dn, - issuer: user_piv_cac_form.x509_issuer, - presented: true, - ) - create_user_event(:piv_cac_enabled) - redirect_to login_add_piv_cac_success_url - end - - def analytics_properties - { - in_account_creation_flow: false, - enabled_mfa_methods_count: MfaContext.new(current_user).enabled_mfa_methods_count, - } - end end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 37b9e38758a..c91a7b7747b 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4144,12 +4144,17 @@ def multi_factor_auth_added_phone( # Tracks when the user has added the MFA method piv_cac to their account # @param [Integer] enabled_mfa_methods_count number of registered mfa methods for the user # @param [Boolean] in_account_creation_flow whether user is going through creation flow - def multi_factor_auth_added_piv_cac(enabled_mfa_methods_count:, in_account_creation_flow:, - **extra) + # @param ['piv_cac'] method_name Authentication method added + def multi_factor_auth_added_piv_cac( + enabled_mfa_methods_count:, + in_account_creation_flow:, + method_name: :piv_cac, + **extra + ) track_event( :multi_factor_auth_added_piv_cac, { - method_name: :piv_cac, + method_name:, enabled_mfa_methods_count:, in_account_creation_flow:, **extra, diff --git a/config/routes.rb b/config/routes.rb index 02c77337a03..8ba42da6396 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -137,8 +137,6 @@ get 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#prompt' post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' - get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' - post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' get 'login/piv_cac_recommended' => 'users/piv_cac_recommended#show' post 'login/piv_cac_recommended/add' => 'users/piv_cac_recommended#confirm' post 'login/piv_cac_recommended/skip' => 'users/piv_cac_recommended#skip' diff --git a/spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb b/spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb new file mode 100644 index 00000000000..d41afebdc5c --- /dev/null +++ b/spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' + +RSpec.describe 'Setup PIV/CAC after sign-in' do + include SamlAuthHelper + + scenario 'user opts to not add piv/cac card' do + perform_steps_to_get_to_add_piv_cac_during_sign_up + + click_on t('forms.piv_cac_setup.no_thanks') + + expect(page).to have_current_path(sign_up_completed_path) + end + + context 'without an associated service provider' do + scenario 'user opts to not add piv/cac card' do + perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: nil) + + click_on t('forms.piv_cac_setup.no_thanks') + + expect(page).to have_current_path(account_path) + end + end + + scenario 'user opts to add piv/cac card' do + perform_steps_to_get_to_add_piv_cac_during_sign_up + + fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect + + expect(page).to have_current_path(sign_up_completed_path) + end + + scenario 'user opts to add piv/cac card but gets an error' do + perform_steps_to_get_to_add_piv_cac_during_sign_up + + fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1' + stub_piv_cac_service(error: 'certificate.bad') + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect + + expect(page).to have_current_path(setup_piv_cac_error_path(error: 'certificate.bad')) + end + + scenario 'user opts to add piv/cac card and has piv cac redirect in CSP' do + allow(Identity::Hostdata).to receive(:env).and_return('test') + allow(Identity::Hostdata).to receive(:domain).and_return('example.com') + + perform_steps_to_get_to_add_piv_cac_during_sign_up + + expected_form_action = <<-STR.squish + form-action https://*.pivcac.test.example.com 'self' + http://localhost:7654 https://example.com + STR + + expect(page.response_headers['Content-Security-Policy']). + to(include(expected_form_action)) + end + + def perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: :oidc) + user = create(:user, :fully_registered, :with_phone) + if sp + visit_idp_from_sp_with_ial1(sp) + else + visit new_user_session_path + end + + click_on t('account.login.piv_cac') + stub_piv_cac_service + click_on t('forms.piv_cac_login.submit') + + follow_piv_cac_redirect + expect(page).to have_current_path(login_piv_cac_error_path(error: 'user.not_found')) + click_on t('instructions.mfa.piv_cac.back_to_sign_in') + + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + expect(current_path).to eq login_add_piv_cac_prompt_path + fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1' + end +end diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 20fceb7bd6b..6b5495a85f7 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -32,20 +32,6 @@ to have_link t('devise.failure.not_found_in_database_link_text', href: link_url) end - scenario 'user opts to not add piv/cac card' do - perform_steps_to_get_to_add_piv_cac_during_sign_up - click_on t('forms.piv_cac_setup.no_thanks') - expect(current_path).to eq sign_up_completed_path - end - - context 'without an associated service provider' do - scenario 'user opts to not add piv/cac card' do - perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: nil) - click_on t('forms.piv_cac_setup.no_thanks') - expect(current_path).to eq account_path - end - end - scenario 'user is suspended, gets show please call page after 2fa' do user = create(:user, :fully_registered, :suspended) service_provider = ServiceProvider.find_by(issuer: OidcAuthHelper::OIDC_IAL1_ISSUER) @@ -61,22 +47,6 @@ expect(current_path).to eq(user_please_call_path) end - scenario 'user opts to add piv/cac card' do - perform_steps_to_get_to_add_piv_cac_during_sign_up - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - current_url, - nonce: nonce, - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) - - expect(current_path).to eq login_add_piv_cac_success_path - click_continue - expect(current_path).to eq sign_up_completed_path - end - scenario 'user with old terms of use can accept and continue to IAL1 SP' do user = create( :user, @@ -132,36 +102,6 @@ expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first end - scenario 'user opts to add piv/cac card but gets an error' do - perform_steps_to_get_to_add_piv_cac_during_sign_up - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - current_url, - nonce: nonce, - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - uuid: SecureRandom.uuid, - error: 'certificate.bad', - subject: 'SomeIgnoredSubject', - ) - - expect(page).to have_current_path(login_piv_cac_error_path(error: 'certificate.bad')) - end - - scenario 'user opts to add piv/cac card and has piv cac redirect in CSP' do - allow(Identity::Hostdata).to receive(:env).and_return('test') - allow(Identity::Hostdata).to receive(:domain).and_return('example.com') - - perform_steps_to_get_to_add_piv_cac_during_sign_up - - expected_form_action = <<-STR.squish - form-action https://*.pivcac.test.example.com 'self' - http://localhost:7654 https://example.com - STR - - expect(page.response_headers['Content-Security-Policy']). - to(include(expected_form_action)) - end - scenario 'User with gov/mil email directed to recommended PIV page' do user = create(:user, :with_phone, { email: 'example@example.gov' }) @@ -1095,35 +1035,6 @@ end end - def perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: :oidc) - user = create(:user, :fully_registered, :with_phone) - if sp - visit_idp_from_sp_with_ial1(sp) - else - visit new_user_session_path - end - click_on t('account.login.piv_cac') - allow(FeatureManagement).to receive(:development_and_identity_pki_disabled?).and_return(false) - - stub_piv_cac_service - nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_login.submit'))) - visit_piv_cac_service( - current_url, - nonce: nonce, - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) - - expect(page).to have_current_path(login_piv_cac_error_path(error: 'user.not_found')) - visit new_user_session_path - fill_in_credentials_and_submit(user.email, user.password) - fill_in_code_with_last_phone_otp - click_submit_default - expect(current_path).to eq login_add_piv_cac_prompt_path - fill_in 'name', with: 'Card 1' - end - def with_forgery_protection original_allow_forgery_protection = ActionController::Base.allow_forgery_protection ActionController::Base.allow_forgery_protection = true diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index d8dc562604b..f1b63bfd743 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -559,7 +559,7 @@ def sign_in_via_branded_page(user) click_submit_default end - def stub_piv_cac_service + def stub_piv_cac_service(error: nil) allow(IdentityConfig.store).to receive(:identity_pki_disabled).and_return(false) allow(IdentityConfig.store).to receive(:piv_cac_service_url). and_return('http://piv.example.com/') @@ -570,6 +570,34 @@ def stub_piv_cac_service body: CGI.unescape(request.body.sub(/^token=/, '')), } end + + stub_request(:post, 'piv.example.com'). + with(query: hash_including('nonce', 'redirect_uri')). + to_return do |request| + query = UriService.params(request.uri) + { + status: 302, + headers: { + location: UriService.add_params( + query['redirect_uri'], + token: { + dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', + uuid: SecureRandom.uuid, + subject: 'SomeIgnoredSubject', + nonce: query['nonce'], + error:, + }.compact.to_json, + ), + }, + } + end + end + + def follow_piv_cac_redirect + # RackTest won't do an external redirect to the stubbed PKI service, but we can manually + # submit a request to the `current_url` and get the redirect header. + redirect_url = Faraday.post(current_url).headers['location'] + visit redirect_url end def visit_piv_cac_service(idp_url, token_data) From 1cdb8f2ff85bee669f56de522306bb8be16f6534 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 17 Jul 2024 21:23:58 +0000 Subject: [PATCH 09/12] Revert "Remove 2023 SAML certificates (#10954)" (#10958) This reverts commit 88a2a1c166d3598461736abc349c5c7748fabf5a. --- config/artifacts.example/local/saml2023.crt | 21 +++++++++++++ .../artifacts.example/local/saml2023.key.enc | 30 +++++++++++++++++++ config/initializers/app_artifacts.rb | 2 ++ 3 files changed, 53 insertions(+) create mode 100644 config/artifacts.example/local/saml2023.crt create mode 100644 config/artifacts.example/local/saml2023.key.enc diff --git a/config/artifacts.example/local/saml2023.crt b/config/artifacts.example/local/saml2023.crt new file mode 100644 index 00000000000..914a417e994 --- /dev/null +++ b/config/artifacts.example/local/saml2023.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDajCCAlICCQCbZpJCM572hTANBgkqhkiG9w0BAQsFADB3MQswCQYDVQQGEwJV +UzEdMBsGA1UECAwURGlzdHJpY3Qgb2YgQ29sdW1iaWExEzARBgNVBAcMCldhc2hp +bmd0b24xDDAKBgNVBAoMA0dTQTESMBAGA1UECwwJTG9naW4uZ292MRIwEAYDVQQD +DAlsb2dpbi5nb3YwHhcNMjMwNDA0MTYwNzIxWhcNMjQwNDAyMTYwNzIxWjB3MQsw +CQYDVQQGEwJVUzEdMBsGA1UECAwURGlzdHJpY3Qgb2YgQ29sdW1iaWExEzARBgNV +BAcMCldhc2hpbmd0b24xDDAKBgNVBAoMA0dTQTESMBAGA1UECwwJTG9naW4uZ292 +MRIwEAYDVQQDDAlsb2dpbi5nb3YwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCdToDm3/j0j0PMjca8bc7H0H3FNSTW4l6hpUSywkC/kg2fZ5W6f0hIYYID +TbDYAkpeIiKKE/FDI3TlaQT9+LLe6AbkelLmteS+wMehCPtaBPeRfHKaRNQKsSTk +c5JAf4OWaZKj+F3Fu0e5+dJ2nuYcT2VV7DLoG3KKTw+pcHuXCQZfrPbquyyNbKvo +K4ELVIueQQ5F3EiahP3XchGw+H5FCH5QJPVl57WaCB2gLM8kueELKIzta7roIYHf +GEhdaC71ZYCjGRvsKtAqomNdL2Je67E56dEwJ1fWS242PkSvQTH5vtkYzelE2H9m +V+sPf5lLfc599iLZoTemEe5p6NydAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAH88 +Yl+KbT3NPjrXH7SITWr1wOIemJ8b2/vukz7aS9TiTJnlw6IzsLnD+tiJa1q5CO23 +3/NCaa6piocbD1fC/H7PB6lXu+0ypMwpaStTThpxbpQ6bMUklxKKFyuaX5RpNZn4 +YicYGEnCr7h70+R/01ztgYNOzNdgM6MjHMvEnb8KVkuckuCE1JUoX+LxaE9cxkxX +Auwdct14efBFuyB2HgvWUvqvjN9NDhfS6BG5FgTZWpWJnn7xmjUNUfq1VC4XHQsv +mqoPiDhR1GwB191ZVz7Rq00yysfr7tSUJeWp//5GPZRjSZsrs1wtO6x/tFjngELl +d0/LPNS3OWvaMvvGzgc= +-----END CERTIFICATE----- diff --git a/config/artifacts.example/local/saml2023.key.enc b/config/artifacts.example/local/saml2023.key.enc new file mode 100644 index 00000000000..c1c1a03fa5d --- /dev/null +++ b/config/artifacts.example/local/saml2023.key.enc @@ -0,0 +1,30 @@ +-----BEGIN ENCRYPTED PRIVATE KEY----- +MIIFHzBJBgkqhkiG9w0BBQ0wPDAbBgkqhkiG9w0BBQwwDgQIfgN2KJrydvECAggA +MB0GCWCGSAFlAwQBKgQQEF8wtPY1ps4bJfQ4sV0xeQSCBND598BOsn9cAgqBGUEE +yQgCp+DIpDKFmgwXOiFjjuY1H3vsAFh0lhOoWN2z2UsAWUq8RwWs41UNSajgUllq +wPP3dm+U2BG661AmZEj5+fJ6ckCLX+BZHcv2bjx7cRF8oMv3Z1wlmTvxyARcFlct +b5Rxjl+moDwuBGFUaCg80hL2NZmUSuf/GZ5bj5yHZqGdc/KAXRts6knqLfsBxhrh +uECYUaz11NRN+JCNg8TcQSUJs01QVrhS7MrNhr1WGIwdIYg0AsqHqluzKr4nzjqG +u8vfRV7oraUhNDKrfXfxjGcyy/o4zYnuJO3zBRWPV4Ueesf3efGY3bNyZzoY2BrD +ZlpQwHf7NmANZqHnCAH9dDrk3mcNkfPEnSs/oguhIk05ZHMaZeGR+bIJd1LjxjLY +KOhgw9u+I4WR35GsZRFArwUnHfvAcLTwBbsChMk7ykj16F/rdIknMWiyZX3p31Cm +ZW/tB5j4lm9caA9iPn/d3S/H7O3zG3SSuwQvrwQguImqEegMq2FA6MLT+66R+DU/ +AB/M8WgwIhaLWr136GhGBjJO4Tm21DrNjHzS8NYWcXL8PWdDgagwbTGAL6jLLulr +iCrkEsQoNzX11WiwYgmhrPLKVqophHK4v3dzuY+naMV5GekqhPA+8IfWZfvmizb7 +LVoxb6TKBI0U3p31lhRI9car+Yxa7z5ZSAl0n03TCeZOYvUmKAwj81E23g1XC3Fx ++xB+nrDWQQjvD+ZRXQxqUg58o0qitM9tKsIkh2Qj4IRpATTyTyNO40seGni8AWsU +ftsll8OVWEdHlnOGLNjSPC/uVMYSa8YOcv+xIqErty2Q40yrOsXufX0ompzONNZH +rUbifoY3T8En88m7wCDBQAw38pUdgPniL0VyADdvkc4To5pjkK7B4g7fD+WbjdrD +V7UhU+4wxZkSK9ijaq/4Cj/ZIg5m29qOoCtI/vn2A/DnhLJmomQ8B21xgtUsa/pM +Pi1Z4rz2OLEND1EPuJSm/xijxVGUgYpkOYHoRQmvbpPst9FvXPIXcBdpahbVvqOq +Iv8LcKatTKqTjxyXoOxxUWQ16MYXaOSk8sEbDYqp/YK2N9ItIPiHiYio/Wt+uQM8 +/VbyZGY+e0O9mAGtoWLDuqCziH5HHXjH7sX0mJmrVCc8V7eKewEEYT6jCjvOMG+V +tABCJQXoaOV8Tk5f3rMDVYg+DQaqFfuvOEahj0CLm4xLuEK9mGvNz7HjxgPjRfS1 +CFn93oBKnhJbzeEkNEHWw2sG1/twiOjhSkMs5CbdcpAjm6wNoKE3/e23HbUuFGos +yOb/2mm5oAr/EjBCUSilyNSkIuUTzIKnootLJ4bxHmTHFlpJlYvFBKWT7RIRT21g +LErRAgUHF/f93yNjgEbdzf8lzKfYWBnZK1E2RGbVay3W1OtQQ7Om01CbKd0THt9y +4bwD/rS29+xa4NVImMXmq/aftLLoedaY9TWHz2FOInXhms9vw0wsehmc1aoQ1yrl +DVx+T7raO6YJ8MK385ryjupsZd4J5dYcvHjlkpnZOyvVyzfdNqHvVhtfd765d5rw +GyX2UKPByjECQskxKjeqadjZ8zaAecW/4ujg0wcmdX7l4lfXcdy6+0WiPr3qBwQE +hrcBC92oNEzyorgjdcMEz2RATw== +-----END ENCRYPTED PRIVATE KEY----- diff --git a/config/initializers/app_artifacts.rb b/config/initializers/app_artifacts.rb index 42afaad14f6..d9486fc409b 100644 --- a/config/initializers/app_artifacts.rb +++ b/config/initializers/app_artifacts.rb @@ -4,6 +4,8 @@ AppArtifacts.setup do |store| # When adding or removing certs, make sure to update the 'saml_endpoint_configs' config + store.add_artifact(:saml_2023_cert, '/%s/saml2023.crt') + store.add_artifact(:saml_2023_key, '/%s/saml2023.key.enc') store.add_artifact(:saml_2024_cert, '/%s/saml2024.crt') store.add_artifact(:saml_2024_key, '/%s/saml2024.key.enc') From a64bcaa9c232532434499b52a5e89a92f4c5bd7a Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Thu, 18 Jul 2024 08:13:39 -0400 Subject: [PATCH 10/12] Log new_device with email and password authentication event (#10957) changelog: Internal, Analytics, Log new_device with email and password authentication event --- app/controllers/users/sessions_controller.rb | 1 + app/services/analytics_events.rb | 4 + .../users/sessions_controller_spec.rb | 144 ++++++++++-------- 3 files changed, 85 insertions(+), 64 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 159e784baf2..ffc8d858bc0 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -176,6 +176,7 @@ def track_authentication_attempt(email) bad_password_count: session[:bad_password_count].to_i, sp_request_url_present: sp_session[:request_url].present?, remember_device: remember_device_cookie.present?, + new_device: success ? new_device? : nil, ) end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index c91a7b7747b..bb396286a23 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -404,6 +404,8 @@ def edit_password_visit # @param [String] bad_password_count represents number of prior login failures # @param [Boolean] sp_request_url_present if was an SP request URL in the session # @param [Boolean] remember_device if the remember device cookie was present + # @param [Boolean, nil] new_device Whether the user is authenticating from a new device. Nil if + # there is the attempt was unsuccessful, since it cannot be known whether it's a new device. # Tracks authentication attempts at the email/password screen def email_and_password_auth( success:, @@ -413,6 +415,7 @@ def email_and_password_auth( bad_password_count:, sp_request_url_present:, remember_device:, + new_device:, **extra ) track_event( @@ -424,6 +427,7 @@ def email_and_password_auth( bad_password_count:, sp_request_url_present:, remember_device:, + new_device:, **extra, ) end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c22af06d3bd..ad235ebe656 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -57,6 +57,7 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, + new_device: true, ) end @@ -113,6 +114,24 @@ response end + + it 'tracks as not being from a new device' do + stub_analytics + + response + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', + success: true, + user_id: user.uuid, + user_locked_out: false, + valid_captcha_result: true, + bad_password_count: 0, + sp_request_url_present: false, + remember_device: false, + new_device: false, + ) + end end end @@ -150,7 +169,12 @@ user = create(:user, :fully_registered) stub_analytics - analytics_hash = { + expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original + + post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: false, @@ -158,19 +182,19 @@ bad_password_count: 1, sp_request_url_present: false, remember_device: false, - } - expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } } + new_device: nil, + ) expect(subject.session[:sign_in_flow]).to eq(:sign_in) end it 'tracks the authentication attempt for nonexistent user' do stub_analytics - analytics_hash = { + expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original + + post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: 'anonymous-uuid', user_locked_out: false, @@ -178,13 +202,8 @@ bad_password_count: 1, sp_request_url_present: false, remember_device: false, - } - expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + new_device: nil, + ) end it 'tracks unsuccessful authentication for locked out user' do @@ -195,7 +214,11 @@ ) stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email.upcase, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: true, @@ -203,12 +226,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email.upcase, password: user.password } } + new_device: nil, + ) end it 'tracks unsuccessful authentication for failed reCAPTCHA' do @@ -229,6 +248,7 @@ valid_captcha_result: false, bad_password_count: 0, remember_device: false, + new_device: nil, sp_request_url_present: false, ) end @@ -241,7 +261,10 @@ stub_analytics - analytics_hash = { + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: false, @@ -249,18 +272,18 @@ bad_password_count: 2, sp_request_url_present: false, remember_device: false, - } - - post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + new_device: nil, + ) end it 'tracks the presence of SP request_url in session' do subject.session[:sp] = { request_url: mock_valid_site } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: 'anonymous-uuid', user_locked_out: false, @@ -268,12 +291,8 @@ bad_password_count: 1, sp_request_url_present: true, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + new_device: nil, + ) end context 'IAL1 user' do @@ -431,7 +450,11 @@ ) stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -439,19 +462,12 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - profile_encryption_error = { + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'Profile Encryption: Invalid', error: 'Unable to parse encrypted payload', - } - expect(@analytics).to receive(:track_event). - with('Profile Encryption: Invalid', profile_encryption_error) - - post :create, params: { user: { email: user.email, password: user.password } } - + ) expect(controller.user_session[:encrypted_profiles]).to be_nil expect(profile.reload).to_not be_active end @@ -558,7 +574,11 @@ } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -566,12 +586,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: true, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email, password: user.password } } + new_device: true, + ) end end @@ -584,7 +600,11 @@ } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -592,12 +612,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: true, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email, password: user.password } } + new_device: true, + ) end end From 9e82971ebcdf4883789da15c9a1b0f2e41563653 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Thu, 18 Jul 2024 08:17:18 -0400 Subject: [PATCH 11/12] Normalize newlines in locale data (#10955) changelog: Internal, Localization, Normalize newlines in locale data --- config/locales/en.yml | 6 ++---- config/locales/es.yml | 3 +-- config/locales/fr.yml | 6 ++---- config/locales/zh.yml | 17 +++++------------ 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 9ac290b353d..80fac37d540 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -956,8 +956,7 @@ help_text.requested_attributes.email: Email address help_text.requested_attributes.full_name: Full name help_text.requested_attributes.ial1_consent_reminder_html: You must consent each year to share your information with %{sp}. We’ll share your information with %{sp} to connect your account. help_text.requested_attributes.ial1_intro_html: We’ll share your information with %{sp} to connect your account. -help_text.requested_attributes.ial2_consent_reminder_html: "%{sp} needs to know who you are to connect to your account. You must consent each year to share your verified information with %{sp}. We’ll share this information:" +help_text.requested_attributes.ial2_consent_reminder_html: '%{sp} needs to know who you are to connect to your account. You must consent each year to share your verified information with %{sp}. We’ll share this information:' help_text.requested_attributes.ial2_intro_html: '%{sp} needs to know who you are to connect your account. We’ll share this information with %{sp}:' help_text.requested_attributes.ial2_reverified_consent_info: 'Because you verified your identity again, we need your permission to share this information with %{sp}:' help_text.requested_attributes.phone: Phone number @@ -1603,8 +1602,7 @@ titles.verify_email: Check your email titles.visitors.index: Welcome titles.webauthn_setup: Add your security key two_factor_authentication.aal2_request.phishing_resistant_html: '%{sp_name} requires a high-security authentication method, such as face or touch unlock, a security key or a government employee ID.' -two_factor_authentication.aal2_request.piv_cac_only_html: "%{sp_name} requires your government employee ID, a high-security authentication method." +two_factor_authentication.aal2_request.piv_cac_only_html: '%{sp_name} requires your government employee ID, a high-security authentication method.' two_factor_authentication.account_reset.cancel_link: Cancel your request two_factor_authentication.account_reset.link: deleting your account two_factor_authentication.account_reset.pending: You currently have a pending request to delete your account. It takes %{interval} from the time you made the request to complete the process. Please check back later. diff --git a/config/locales/es.yml b/config/locales/es.yml index 6d0dcb23d9b..9228e8bcca7 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -1614,8 +1614,7 @@ titles.verify_email: Revise su correo electrónico titles.visitors.index: Bienvenido titles.webauthn_setup: Añada su clave de seguridad two_factor_authentication.aal2_request.phishing_resistant_html: '%{sp_name} requiere un método de autenticación de alta seguridad, como desbloqueo facial o táctil, una clave de seguridad o una identificación de empleado de gobierno.' -two_factor_authentication.aal2_request.piv_cac_only_html: "%{sp_name} requiere su identificación de empleado de gobierno, un método de autenticación de alta seguridad." +two_factor_authentication.aal2_request.piv_cac_only_html: '%{sp_name} requiere su identificación de empleado de gobierno, un método de autenticación de alta seguridad.' two_factor_authentication.account_reset.cancel_link: Cancelar su solicitud two_factor_authentication.account_reset.link: eliminando su cuenta two_factor_authentication.account_reset.pending: Actualmente tiene una solicitud pendiente para eliminar su cuenta. Se necesitan %{interval} desde el momento en que realizó la solicitud para completar el proceso. Por favor, vuelva más tarde. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index a5724f1c6b9..b20db82eaf2 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -956,8 +956,7 @@ help_text.requested_attributes.email: Adresse e-mail help_text.requested_attributes.full_name: Nom complet help_text.requested_attributes.ial1_consent_reminder_html: Vous devez consentir chaque année au partage de vos informations avec %{sp}. Nous partagerons vos informations avec %{sp} pour connecter votre compte. help_text.requested_attributes.ial1_intro_html: Nous partagerons vos informations avec %{sp} pour connecter votre compte. -help_text.requested_attributes.ial2_consent_reminder_html: "%{sp} a besoin de savoir qui vous êtes pour se connecter à votre compte. Vous devez consentir chaque année à partager vos informations vérifiées avec %{sp}. Nous partagerons ces informations :" +help_text.requested_attributes.ial2_consent_reminder_html: '%{sp} a besoin de savoir qui vous êtes pour se connecter à votre compte. Vous devez consentir chaque année à partager vos informations vérifiées avec %{sp}. Nous partagerons ces informations :' help_text.requested_attributes.ial2_intro_html: '%{sp} a besoin de savoir qui vous êtes pour connecter votre compte. Nous partagerons ces informations avec %{sp} :' help_text.requested_attributes.ial2_reverified_consent_info: 'Étant donné que vous avez revérifié votre identité, nous avons besoin de votre autorisation pour partager ces informations avec %{sp} :' help_text.requested_attributes.phone: Numéro de téléphone @@ -1603,8 +1602,7 @@ titles.verify_email: Consulter vos e-mails titles.visitors.index: Bienvenue titles.webauthn_setup: Ajouter votre clé de sécurité two_factor_authentication.aal2_request.phishing_resistant_html: '%{sp_name} nécessite une méthode d’authentification de haute sécurité, telle que le déverrouillage facial ou tactile, une clé de sécurité ou une carte d’employé fédéral.' -two_factor_authentication.aal2_request.piv_cac_only_html: "%{sp_name} nécessite votre carte d’employé fédéral, qui est une méthode d’authentification de haute sécurité." +two_factor_authentication.aal2_request.piv_cac_only_html: '%{sp_name} nécessite votre carte d’employé fédéral, qui est une méthode d’authentification de haute sécurité.' two_factor_authentication.account_reset.cancel_link: Annuler votre demande two_factor_authentication.account_reset.link: supprimer votre compte two_factor_authentication.account_reset.pending: Vous avez actuellement une demande en attente pour supprimer votre compte. Il faut compter %{interval} à partir du moment où vous avez fait la demande pour terminer le processus. Veuillez vérifier plus tard. diff --git a/config/locales/zh.yml b/config/locales/zh.yml index f199579ceba..73ccabd61c7 100644 --- a/config/locales/zh.yml +++ b/config/locales/zh.yml @@ -873,9 +873,7 @@ forms.piv_cac_login.submit: 插入 PIV/CAC 卡 forms.piv_cac_mfa.submit: 提供 PIV/CAC 卡 forms.piv_cac_setup.nickname: PIV/CAC 卡昵称 forms.piv_cac_setup.no_thanks: 不用，谢谢。 -forms.piv_cac_setup.piv_cac_intro_html: - "每次你登录时，作为双因素身份证实的一部分，\ - 我们会要你提供你的 PIV/CAC 卡。

点击“添加 PIV/CAC”后，你的浏览器会提示要你的 PIV/CAC 并要你选择一个证书。" +forms.piv_cac_setup.piv_cac_intro_html: '每次你登录时，作为双因素身份证实的一部分，我们会要你提供你的 PIV/CAC 卡。

点击“添加 PIV/CAC”后，你的浏览器会提示要你的 PIV/CAC 并要你选择一个证书。' forms.piv_cac_setup.submit: 添加 PIV/CAC 卡 forms.registration.labels.email: 输入你的电邮地址 forms.registration.labels.email_language: 选择你的电邮语言偏好 @@ -892,9 +890,7 @@ forms.two_factor.try_again: 使用另一个电话号码 forms.validation.required_checkbox: 请在这一框里打勾来继续。 forms.webauthn_platform_setup.continue: 继续 forms.webauthn_platform_setup.info_text: 你设置人脸或触摸解锁后需要再设置另一个身份证实方法。 -forms.webauthn_platform_setup.intro_html: - "

就像解锁你的设备一样来做身份证实，无论是用你的面孔还是指纹、密码或\ - 者其他方法。

如果你使用iCloud Keychain 或 Google Password Manager 这样的密码管理器，管理器可能会要保存一个通行密钥。这会使你能使用那个密码管理器在任何设备上证实身份。%{link}

" +forms.webauthn_platform_setup.intro_html: '

就像解锁你的设备一样来做身份证实，无论是用你的面孔还是指纹、密码或者其他方法。

如果你使用iCloud Keychain 或 Google Password Manager 这样的密码管理器，管理器可能会要保存一个通行密钥。这会使你能使用那个密码管理器在任何设备上证实身份。%{link}

' forms.webauthn_platform_setup.intro_link_text: 了解关于使用不同设备的更多信息。 forms.webauthn_platform_setup.nickname: 设备昵称 forms.webauthn_platform_setup.nickname_hint: 这样如果你添加了更多使用人脸或触摸解锁的设备的话，你就能把它们分辨开来。 @@ -971,8 +967,7 @@ help_text.requested_attributes.email: 电邮地址 help_text.requested_attributes.full_name: 姓名 help_text.requested_attributes.ial1_consent_reminder_html: 你每年都必须授权同意与 %{sp} 分享信息。我们将与 %{sp} 分享你的信息来连接你账户。 help_text.requested_attributes.ial1_intro_html: 我们将与 %{sp} 分享你的信息来连接你账户。 -help_text.requested_attributes.ial2_consent_reminder_html: "%{sp} 需要知道你是谁才能连接你的账户。你每年都必须授权同意与 %{sp} 分享已验证过的你的信息。我们会分享这些信息：" +help_text.requested_attributes.ial2_consent_reminder_html: '%{sp} 需要知道你是谁才能连接你的账户。你每年都必须授权同意与 %{sp} 分享已验证过的你的信息。我们会分享这些信息：' help_text.requested_attributes.ial2_intro_html: '%{sp} 需要知道你是谁才能连接你的账户。我们会与 %{sp} 分享这些信息：' help_text.requested_attributes.ial2_reverified_consent_info: '因为你重新验证了身份，我们需要得到你的许可才能与 %{sp} 分享该信息。' help_text.requested_attributes.phone: 电话号码 @@ -1356,8 +1351,7 @@ instructions.mfa.sms.number_message_html: 我们把带有一次性代码的短 instructions.mfa.voice.number_message_html: 我们给 %{number_html}打了电话告知一次性代码。这一代码 %{expiration} 分钟后会作废。 instructions.mfa.webauthn_platform.learn_more_help: 了解更多有关人脸或触摸解锁的信息 instructions.mfa.webauthn.confirm_webauthn: 提供与你账户相关的安全密钥。 -instructions.mfa.webauthn.confirm_webauthn_platform_html: "

你可以像解锁你的设备一样来做身份证\ - 实，无论是用你的面孔还是指纹、密码或者其他方法。

如果你用一个密码管理器设置了人脸或触摸解锁，则可以从使用那一密码管理器的任何设备进行身份证实。否则的话，使用你设置人脸或触摸解锁的同一设备。

" +instructions.mfa.webauthn.confirm_webauthn_platform_html: '

你可以像解锁你的设备一样来做身份证实，无论是用你的面孔还是指纹、密码或者其他方法。

如果你用一个密码管理器设置了人脸或触摸解锁，则可以从使用那一密码管理器的任何设备进行身份证实。否则的话，使用你设置人脸或触摸解锁的同一设备。

' instructions.mfa.wrong_number: 输入的电话号码不对？ instructions.password.forgot: 不知道你的密码？确认你的电邮地址后重设密码。 instructions.password.help_text: 避免重复使用你其他网上账户（比如银行、电邮和社交媒体）的密码。请勿包括你电邮地址中的单词。 @@ -1865,8 +1859,7 @@ user_mailer.in_person_verified.next_sign_in.without_sp: 接下来请点击按钮 user_mailer.in_person_verified.sign_in: 登录 user_mailer.in_person_verified.subject: 你在 %{app_name} 成功地验证了身份 user_mailer.in_person_verified.warning_contact_us_html: 如果你没有试图亲身验证身份，请登入 重设密码。要报告这件事，联系 %{app_name} 支持 %{app_name}。 -user_mailer.letter_reminder_14_days.body_html: "

%{date_letter_was_se\ - nt} 日你要求了带有验证码的信。

登录%{app_name} 并输入验证码来完成验证你的身份。 %{help_link}.

" +user_mailer.letter_reminder_14_days.body_html: '

%{date_letter_was_sent} 日你要求了带有验证码的信。

登录%{app_name} 并输入验证码来完成验证你的身份。 %{help_link}.

' user_mailer.letter_reminder_14_days.did_not_get_a_letter_html: 如果你没有收到这封信， %{another_letter_link_html}。 user_mailer.letter_reminder_14_days.finish: 完成验证你的身份 user_mailer.letter_reminder_14_days.sign_in_and_request_another_letter: 登录要求再发一封信 From 74d2492c819b30e42c15b85a350411753784fd64 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Thu, 18 Jul 2024 08:17:40 -0400 Subject: [PATCH 12/12] Use improved PKI service test stub consistently (#10942) changelog: Internal, Automated Testing, Use improved PKI service test stub consistently --- spec/features/openid_connect/vtr_spec.rb | 13 ++-- spec/features/saml/vtr_spec.rb | 13 ++-- .../two_factor_authentication/sign_in_spec.rb | 24 ++----- .../features/users/piv_cac_management_spec.rb | 52 +++++--------- spec/features/users/sign_in_spec.rb | 10 +-- spec/support/features/session_helper.rb | 71 +++---------------- 6 files changed, 45 insertions(+), 138 deletions(-) diff --git a/spec/features/openid_connect/vtr_spec.rb b/spec/features/openid_connect/vtr_spec.rb index 21102cfd3e4..f0abcb9c697 100644 --- a/spec/features/openid_connect/vtr_spec.rb +++ b/spec/features/openid_connect/vtr_spec.rb @@ -88,15 +88,10 @@ expect(page).to have_content(t('two_factor_authentication.two_factor_hspd12_choice_intro')) # User must setup PIV/CAC before continuing - visit setup_piv_cac_path - - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) + select_2fa_option('piv_cac') + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect click_agree_and_continue click_on 'Continue' diff --git a/spec/features/saml/vtr_spec.rb b/spec/features/saml/vtr_spec.rb index 50b1d711eac..0a30581b34f 100644 --- a/spec/features/saml/vtr_spec.rb +++ b/spec/features/saml/vtr_spec.rb @@ -116,16 +116,11 @@ expect(page).to have_content(t('two_factor_authentication.two_factor_hspd12_choice_intro')) # User must setup PIV/CAC before continuing - visit setup_piv_cac_path - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) + select_2fa_option('piv_cac') + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect - click_submit_default click_agree_and_continue click_submit_default expect_successful_saml_redirect diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index d635017367b..2118db20cb2 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -405,36 +405,24 @@ def attempt_to_bypass_2fa end scenario 'user uses PIV/CAC as their second factor' do - stub_piv_cac_service - user = user_with_piv_cac sign_in_before_2fa(user) + stub_piv_cac_service(uuid: user.piv_cac_configurations.first.x509_dn_uuid) - nonce = visit_login_two_factor_piv_cac_and_get_nonce + click_on t('forms.piv_cac_mfa.submit') + follow_piv_cac_redirect - visit_piv_cac_service( - login_two_factor_piv_cac_path, - uuid: user.piv_cac_configurations.first.x509_dn_uuid, - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - nonce: nonce, - ) expect(current_path).to eq account_path end scenario 'user uses incorrect PIV/CAC as their second factor' do - stub_piv_cac_service - user = user_with_piv_cac sign_in_before_2fa(user) + stub_piv_cac_service(uuid: Random.uuid) - nonce = visit_login_two_factor_piv_cac_and_get_nonce + click_on t('forms.piv_cac_mfa.submit') + follow_piv_cac_redirect - visit_piv_cac_service( - login_two_factor_piv_cac_path, - uuid: user.piv_cac_configurations.first.x509_dn_uuid + 'X', - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.12345', - nonce: nonce, - ) expect(current_path).to eq login_two_factor_piv_cac_path expect(page).to have_content(t('two_factor_authentication.invalid_piv_cac')) end diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb index 123d268a90d..4951e5709e6 100644 --- a/spec/features/users/piv_cac_management_spec.rb +++ b/spec/features/users/piv_cac_management_spec.rb @@ -9,7 +9,7 @@ allow(Identity::Hostdata).to receive(:env).and_return('test') allow(Identity::Hostdata).to receive(:domain).and_return('example.com') - stub_piv_cac_service + stub_piv_cac_service(uuid:) sign_in_and_2fa_user(user) visit account_two_factor_authentication_path @@ -18,14 +18,9 @@ expect(page.response_headers['Content-Security-Policy'].split(';').map(&:strip)). to(include("form-action https://*.pivcac.test.example.com 'self'")) - nonce = piv_cac_nonce_from_form_action - - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - uuid: uuid, - subject: 'SomeIgnoredSubject', - ) + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect expect(current_path).to eq account_path visit account_two_factor_authentication_path @@ -56,20 +51,15 @@ end scenario 'disallows association of a piv/cac with the same name' do - stub_piv_cac_service + stub_piv_cac_service(uuid:) sign_in_and_2fa_user(user) visit account_two_factor_authentication_path click_link t('account.index.piv_cac_add'), href: setup_piv_cac_url - nonce = piv_cac_nonce_from_form_action - - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - uuid: uuid, - subject: 'SomeIgnoredSubject', - ) + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect expect(current_path).to eq account_path @@ -83,19 +73,16 @@ end scenario 'displays error for piv/cac with no certificate and accepts more error info' do - stub_piv_cac_service + stub_piv_cac_service(error: 'certificate.none') sign_in_and_2fa_user(user) visit account_two_factor_authentication_path click_link t('account.index.piv_cac_add'), href: setup_piv_cac_url - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - error: 'certificate.none', - key_id: 'AB:CD:EF', - ) + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect + expect(current_path).to eq setup_piv_cac_error_path expect(page).to have_link(t('instructions.mfa.piv_cac.try_again'), href: setup_piv_cac_url) expect(page).to have_content( @@ -107,19 +94,16 @@ end scenario 'displays error for expires certificate piv/cac and accepts more error info' do - stub_piv_cac_service + stub_piv_cac_service(error: 'certificate.expired') sign_in_and_2fa_user(user) visit account_two_factor_authentication_path click_link t('account.index.piv_cac_add'), href: setup_piv_cac_url - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - error: 'certificate.expired', - key_id: 'AB:CD:EF', - ) + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect + expect(current_path).to eq setup_piv_cac_error_path expect(page).to have_link( t('instructions.mfa.piv_cac.please_try_again'), diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 6b5495a85f7..2fb3daf39cd 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -132,14 +132,8 @@ allow(FeatureManagement).to receive(:development_and_identity_pki_disabled?).and_return(false) stub_piv_cac_service - nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_login.submit'))) - visit_piv_cac_service( - current_url, - nonce: nonce, - dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) + click_on t('forms.piv_cac_login.submit') + follow_piv_cac_redirect expect(page).to have_current_path(login_piv_cac_error_path(error: 'user.not_found')) visit sign_up_email_path diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index f1b63bfd743..809264ecff6 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -64,21 +64,14 @@ def signin_with_piv(user = user_with_piv_cac) end def signin_with_piv_error(error) - user = user_with_piv_cac visit new_user_session_path click_on t('account.login.piv_cac') allow(FeatureManagement).to receive(:development_and_identity_pki_disabled?).and_return(false) - stub_piv_cac_service - nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_login.submit'))) - visit_piv_cac_service( - current_url, - nonce: nonce, - uuid: user.piv_cac_configurations.first.x509_dn_uuid, - subject: 'SomeIgnoredSubject', - error: error, - ) + stub_piv_cac_service(error:) + click_on t('forms.piv_cac_login.submit') + follow_piv_cac_redirect end def signin_with_bad_piv @@ -93,14 +86,9 @@ def fill_in_piv_cac_credentials_and_submit(user, piv_cac_configurations&.first&.x509_dn_uuid) allow(FeatureManagement).to receive(:development_and_identity_pki_disabled?).and_return(false) - stub_piv_cac_service - nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_login.submit'))) - visit_piv_cac_service( - current_url, - nonce: nonce, - uuid: uuid, - subject: 'SomeIgnoredSubject', - ) + stub_piv_cac_service(uuid:) + click_on t('forms.piv_cac_login.submit') + follow_piv_cac_redirect end def fill_in_bad_piv_cac_credentials_and_submit @@ -537,16 +525,9 @@ def register_user_with_piv_cac(email = 'test@test.com') def set_up_2fa_with_piv_cac stub_piv_cac_service select_2fa_option('piv_cac') - - expect(page).to have_current_path setup_piv_cac_path - - nonce = piv_cac_nonce_from_form_action - visit_piv_cac_service( - setup_piv_cac_url, - nonce: nonce, - uuid: SecureRandom.uuid, - subject: 'SomeIgnoredSubject', - ) + fill_in t('instructions.mfa.piv_cac.step_1'), with: 'Card' + click_on t('forms.piv_cac_setup.submit') + follow_piv_cac_redirect end def skip_second_mfa_prompt @@ -559,7 +540,7 @@ def sign_in_via_branded_page(user) click_submit_default end - def stub_piv_cac_service(error: nil) + def stub_piv_cac_service(error: nil, uuid: Random.uuid) allow(IdentityConfig.store).to receive(:identity_pki_disabled).and_return(false) allow(IdentityConfig.store).to receive(:piv_cac_service_url). and_return('http://piv.example.com/') @@ -582,7 +563,7 @@ def stub_piv_cac_service(error: nil) query['redirect_uri'], token: { dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', - uuid: SecureRandom.uuid, + uuid:, subject: 'SomeIgnoredSubject', nonce: query['nonce'], error:, @@ -600,36 +581,6 @@ def follow_piv_cac_redirect visit redirect_url end - def visit_piv_cac_service(idp_url, token_data) - visit(idp_url + '?token=' + CGI.escape(token_data.to_json)) - end - - def visit_login_two_factor_piv_cac_and_get_nonce - visit login_two_factor_piv_cac_path - get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_mfa.submit'))) - end - - # This is a bit convoluted because we generate a nonce when we visit the - # link. The link provides a redirect to the piv/cac service with the nonce. - # This way, even if JavaScript fetches the link to grab the nonce, a new nonce - # is generated when the user clicks on the link. - def get_piv_cac_nonce_from_link(link) - go_back = current_path - visit link['href'] - nonce = Rack::Utils.parse_nested_query(URI(current_url).query)['nonce'] - visit go_back - nonce - end - - def piv_cac_nonce_from_form_action - go_back = current_path - fill_in 'name', with: 'Card ' + SecureRandom.uuid - click_button t('forms.piv_cac_setup.submit') - nonce = Rack::Utils.parse_nested_query(URI(current_url).query)['nonce'] - visit go_back - nonce - end - def link_identity(user, service_provider, ial = nil) IdentityLinker.new( user,