diff --git a/Gemfile b/Gemfile index 3ab468336f6..63e1d635240 100644 --- a/Gemfile +++ b/Gemfile @@ -103,7 +103,7 @@ end group :development, :test do gem 'brakeman', require: false gem 'bullet', '~> 8.0' - gem 'capybara-webmock', git: 'https://github.com/hashrocket/capybara-webmock.git', ref: 'd3f3b7c' + gem 'capybara-webmock', github: '18F/capybara-webmock', branch: 'add-support-for-rack-3' gem 'erb_lint', '~> 0.7.0', require: false gem 'i18n-tasks', '~> 1.0' gem 'knapsack' @@ -136,7 +136,6 @@ group :test do gem 'rack_session_access', '>= 0.2.0' gem 'rack-test', '>= 1.1.0' gem 'rails-controller-testing', '>= 1.0.4' - gem 'rspec-retry' gem 'rspec_junit_formatter' gem 'shoulda-matchers', '~> 4.0', require: false gem 'simple_xlsx_reader', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 6c8e57c4ce0..aa3aec2ad1f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,16 @@ +GIT + remote: https://github.com/18F/capybara-webmock.git + revision: d859e8df7280ff5e2e8efc2ce20205162e83ac09 + branch: add-support-for-rack-3 + specs: + capybara-webmock (0.7.0) + capybara (>= 2.4, < 4) + rack (>= 1.4) + rack-proxy (>= 0.6.0) + rexml (>= 3.2) + selenium-webdriver (>= 4.0) + webrick (>= 1.7) + GIT remote: https://github.com/18F/identity-hostdata.git revision: 67a19c577b8fa9305350cf9cefa572cef4a80310 @@ -57,19 +70,6 @@ GIT mail (>= 2.6.1) simpleidn -GIT - remote: https://github.com/hashrocket/capybara-webmock.git - revision: d3f3b7c8edbeca7b575e74b256ad22df80d2b420 - ref: d3f3b7c - specs: - capybara-webmock (0.6.0) - capybara (>= 2.4, < 4) - rack (>= 1.4) - rack-proxy (>= 0.6.0) - rexml (>= 3.2) - selenium-webdriver (>= 4.0) - webrick (>= 1.7) - GIT remote: https://github.com/rack/rack-attack.git revision: d9fedfae4f7f6409f33857763391f4e18a6d7467 @@ -405,7 +405,7 @@ GEM listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - logger (1.6.5) + logger (1.6.6) lograge (0.11.2) actionpack (>= 4) activesupport (>= 4) @@ -461,7 +461,7 @@ GEM net-ssh (6.1.0) newrelic_rpm (9.7.0) nio4r (2.7.4) - nokogiri (1.18.3) + nokogiri (1.18.5) mini_portile2 (~> 2.8.2) racc (~> 1.4) numbers_and_words (0.11.12) @@ -512,7 +512,7 @@ GEM nio4r (~> 2.0) raabro (1.4.0) racc (1.8.1) - rack (3.0.12) + rack (3.0.14) rack-cors (2.0.2) rack (>= 2.0.0) rack-headers_filter (0.0.1) @@ -522,7 +522,7 @@ GEM rack rack-session (2.0.0) rack (>= 3.0.0) - rack-test (2.1.0) + rack-test (2.2.0) rack (>= 1.3) rack-timeout (0.6.3) rack_session_access (0.2.0) @@ -616,8 +616,6 @@ GEM rspec-expectations (~> 3.13) rspec-mocks (~> 3.13) rspec-support (~> 3.13) - rspec-retry (0.6.2) - rspec-core (> 3.3) rspec-support (3.13.2) rspec_junit_formatter (0.6.0) rspec-core (>= 2, < 4, != 2.12.0) @@ -650,7 +648,7 @@ GEM nokogiri (>= 1.13.10) rexml ruby-statistics (3.0.2) - rubyzip (2.3.2) + rubyzip (2.4.1) safe_target_blank (1.0.2) rails safely_block (0.3.0) @@ -661,7 +659,7 @@ GEM ffi-compiler (>= 1.0, < 2.0) rake (>= 9, < 14) securerandom (0.4.1) - selenium-webdriver (4.27.0) + selenium-webdriver (4.29.1) base64 (~> 0.2) logger (~> 1.4) rexml (~> 3.2, >= 3.2.5) @@ -727,7 +725,7 @@ GEM openssl (>= 2.2, < 3.1) safety_net_attestation (~> 0.4.0) tpm-key_attestation (~> 0.11.0) - webmock (3.24.0) + webmock (3.25.1) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) @@ -850,7 +848,6 @@ DEPENDENCIES rqrcode rspec (~> 3.13.0) rspec-rails (~> 7.0) - rspec-retry rspec_junit_formatter rubocop (~> 1.70.0) rubocop-capybara diff --git a/app/services/encryption/multi_region_kms_profile_migrator.rb b/app/services/encryption/multi_region_kms_profile_migrator.rb new file mode 100644 index 00000000000..31032b2636d --- /dev/null +++ b/app/services/encryption/multi_region_kms_profile_migrator.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Encryption + class MultiRegionKmsProfileMigrator + include ::NewRelic::Agent::MethodTracer + + attr_reader :profile + + def initialize(profile) + @profile = profile + end + + def migrate! + profile.with_lock do + if profile.encrypted_pii.blank? && profile.encrypted_pii_recovery.blank? + raise "Profile##{profile.id} is missing encrypted_pii or encrypted_pii_recovery" + end + + next if profile.encrypted_pii_multi_region.present? && + profile.encrypted_pii_recovery_multi_region.present? + + if profile.encrypted_pii.present? && profile.encrypted_pii_multi_region.blank? + encrypted_pii_multi_region = migrate_ciphertext(profile.encrypted_pii) + profile.update!( + encrypted_pii_multi_region: encrypted_pii_multi_region, + ) + end + if profile.encrypted_pii_recovery.present? && + profile.encrypted_pii_recovery_multi_region.blank? + encrypted_pii_recovery_multi_region = migrate_ciphertext(profile.encrypted_pii_recovery) + profile.update!( + encrypted_pii_recovery_multi_region: encrypted_pii_recovery_multi_region, + ) + end + + profile + end + end + + private + + def migrate_ciphertext(ciphertext_string) + ciphertext = Encryption::Encryptors::PiiEncryptor::Ciphertext.parse_from_string( + ciphertext_string, + ) + + aes_encrypted_data = multi_region_kms_client.decrypt( + ciphertext.encrypted_data, kms_encryption_context + ) + multi_region_kms_encrypted_data = multi_region_kms_client.encrypt( + aes_encrypted_data, kms_encryption_context + ) + Encryption::Encryptors::PiiEncryptor::Ciphertext.new( + multi_region_kms_encrypted_data, + ciphertext.salt, + ciphertext.cost, + ) + end + + def kms_encryption_context + { + 'context' => 'pii-encryption', + 'user_uuid' => profile.user.uuid, + } + end + + def multi_region_kms_client + @multi_region_kms_client ||= KmsClient.new( + kms_key_id: IdentityConfig.store.aws_kms_multi_region_key_id, + ) + end + end +end diff --git a/db/primary_migrate/20250321141653_change_null_constraint_on_sp_return_log.rb b/db/primary_migrate/20250321141653_change_null_constraint_on_sp_return_log.rb new file mode 100644 index 00000000000..34b2a8985ef --- /dev/null +++ b/db/primary_migrate/20250321141653_change_null_constraint_on_sp_return_log.rb @@ -0,0 +1,5 @@ +class ChangeNullConstraintOnSpReturnLog < ActiveRecord::Migration[8.0] + def change + change_column_null(:sp_return_logs, :requested_at, true) + end +end diff --git a/db/schema.rb b/db/schema.rb index ef48c047c73..9d63e03380c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_03_11_164618) do +ActiveRecord::Schema[8.0].define(version: 2025_03_21_141653) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" @@ -581,7 +581,7 @@ end create_table "sp_return_logs", force: :cascade do |t| - t.datetime "requested_at", precision: nil, null: false, comment: "sensitive=false" + t.datetime "requested_at", precision: nil, comment: "sensitive=false" t.string "request_id", null: false, comment: "sensitive=false" t.integer "ial", null: false, comment: "sensitive=false" t.string "issuer", null: false, comment: "sensitive=false" diff --git a/docs/attempts-api/schemas/events/IdentityProofingEvents.yml b/docs/attempts-api/schemas/events/IdentityProofingEvents.yml index 8c77a0873eb..41bd70c7993 100644 --- a/docs/attempts-api/schemas/events/IdentityProofingEvents.yml +++ b/docs/attempts-api/schemas/events/IdentityProofingEvents.yml @@ -1,4 +1,6 @@ properties: + idv-address-submitted: + $ref: './identity-proofing/IdvAddressSubmitted.yml' idv-document-uploaded: $ref: './identity-proofing/IdvDocumentUploaded.yml' idv-document-upload-submitted: diff --git a/docs/attempts-api/schemas/events/identity-proofing/IdvAddressSubmitted.yml b/docs/attempts-api/schemas/events/identity-proofing/IdvAddressSubmitted.yml new file mode 100644 index 00000000000..f47f11085ae --- /dev/null +++ b/docs/attempts-api/schemas/events/identity-proofing/IdvAddressSubmitted.yml @@ -0,0 +1,8 @@ +description: | + User manually submitted an IdV address +allOf: + - $ref: '../shared/EventProperties.yml' + - type: object + properties: + address: + type: string \ No newline at end of file diff --git a/lib/tasks/backfill_profiles.rake b/lib/tasks/backfill_profiles.rake new file mode 100644 index 00000000000..0cdf843891d --- /dev/null +++ b/lib/tasks/backfill_profiles.rake @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +namespace :profiles do + desc 'Backfill the encrypted_pii_multi_region value column.' + + ## + # Usage: + # + # Print pending updates + # bundle exec rake profiles:backfill_encrypted_pii_multi_region + # + # Commit updates + # bundle exec rake profiles:backfill_encrypted_pii_multi_region UPDATE_PROFILES=true + # + task backfill_encrypted_pii_multi_region: :environment do |_task, _args| + profile_limit = ENV['PROFILE_LIMIT'].to_i + statement_timeout_seconds = ENV['STATEMENT_TIMEOUT_SECONDS'].to_i + update_profiles = ENV['UPDATE_PROFILES'] == 'true' + + profiles = Profile.transaction do + quoted_timeout = Profile.connection.quote(statement_timeout_seconds * 1000) + Profile.connection.execute("SET LOCAL statement_timeout = #{quoted_timeout}") + + Profile.where( + <<-SQL, + (encrypted_pii IS NOT NULL AND encrypted_pii_multi_region IS NULL) OR + (encrypted_pii_recovery IS NOT NULL AND encrypted_pii_recovery_multi_region IS NULL) + SQL + ).limit(profile_limit) + end + + Rails.logger.info("#{profiles.count} profiles found") + profiles.each do |profile| + Rails.logger.info(profile.id) + Encryption::MultiRegionKmsProfileMigrator.new(profile).migrate! if update_profiles + end + end +end diff --git a/spec/lib/tasks/backfill_profiles_rake_spec.rb b/spec/lib/tasks/backfill_profiles_rake_spec.rb new file mode 100644 index 00000000000..789d44bc40f --- /dev/null +++ b/spec/lib/tasks/backfill_profiles_rake_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' +require 'rake' + +RSpec.describe 'backfill profiles tasks' do + let(:profile_limit) { '10' } + let(:update_profiles) { nil } + let(:env) do + { + 'PROFILE_LIMIT' => profile_limit, + 'STATEMENT_TIMEOUT_SECONDS' => '100', + 'UPDATE_PROFILES' => update_profiles, + } + end + + before do + Rake.application.rake_require 'tasks/backfill_profiles' + Rake::Task.define_task(:environment) + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].reenable + stub_const('ENV', env) + end + + describe 'dev:backfill_encrypted_pii_multi_region' do + it 'logs data about profiles being migrated' do + profile = create_profile_that_needs_to_be_migrated + create(:profile, :with_pii) + + expect(Rails.logger).to receive(:info).with('1 profiles found') + expect(Rails.logger).to receive(:info).with(profile.id) + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + end + + context 'with update_profiles disabled' do + it 'does not update profiles' do + profile = create_profile_that_needs_to_be_migrated + + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + end + end + + context 'with update_profiles enabled' do + let(:update_profiles) { 'true' } + + it 'does update profiles' do + profile = create_profile_that_needs_to_be_migrated + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + + profile.reload + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to be_present + expect(profile.encrypted_pii_recovery_multi_region).to be_present + end + end + end + + def create_profile_that_needs_to_be_migrated + profile = create(:profile, :with_pii) + profile.update( + encrypted_pii: profile.encrypted_pii_multi_region, + encrypted_pii_multi_region: nil, + encrypted_pii_recovery: profile.encrypted_pii_recovery_multi_region, + encrypted_pii_recovery_multi_region: nil, + ) + profile + end +end diff --git a/spec/services/encryption/kms_client_spec.rb b/spec/services/encryption/kms_client_spec.rb index 4b4a46e94a5..dab6a9771ca 100644 --- a/spec/services/encryption/kms_client_spec.rb +++ b/spec/services/encryption/kms_client_spec.rb @@ -12,29 +12,17 @@ ) # rubocop:disable Layout/LineLength - stub_mapped_aws_kms_client( - [ - { plaintext: 'a' * 3000, ciphertext: 'us-north-1:kms1', key_id: key_id, region: 'us-north-1' }, - { plaintext: 'b' * 3000, ciphertext: 'us-north-1:kms2', key_id: key_id, region: 'us-north-1' }, - { plaintext: 'c' * 3000, ciphertext: 'us-north-1:kms3', key_id: key_id, region: 'us-north-1' }, - ], - ) + if kms_enabled + stub_mapped_aws_kms_client( + [ + { plaintext: 'a' * 3000, ciphertext: 'us-north-1:kms1', key_id: key_id, region: 'us-north-1' }, + { plaintext: 'b' * 3000, ciphertext: 'us-north-1:kms2', key_id: key_id, region: 'us-north-1' }, + { plaintext: 'c' * 3000, ciphertext: 'us-north-1:kms3', key_id: key_id, region: 'us-north-1' }, + ], + ) + end # rubocop:enable Layout/LineLength - encryptor = Encryption::Encryptors::AesEncryptor.new - { - 'a' * 3000 => 'local1', - 'b' * 3000 => 'local2', - 'c' * 3000 => 'local3', - }.each do |plaintext, ciphertext| - allow(encryptor).to receive(:encrypt) - .with(plaintext, local_encryption_key) - .and_return(ciphertext) - allow(encryptor).to receive(:decrypt) - .with(ciphertext, local_encryption_key) - .and_return(plaintext) - end - allow(Encryption::Encryptors::AesEncryptor).to receive(:new).and_return(encryptor) allow(FeatureManagement).to receive(:use_kms?).and_return(kms_enabled) allow(IdentityConfig.store).to receive(:aws_region).and_return(aws_region) allow(IdentityConfig.store).to receive(:aws_kms_key_id).and_return(key_id) @@ -46,14 +34,6 @@ let(:encryption_context) { { 'context' => 'attribute-bundle', 'user_id' => '123-abc-456-def' } } let(:log_timestamp) { Time.utc(2025, 2, 28, 15, 30, 1) } - let(:local_encryption_key) do - OpenSSL::HMAC.digest( - 'sha256', - IdentityConfig.store.password_pepper, - '123-abc-456-defattribute-bundlecontextuser_id', - ) - end - let(:aws_region) { 'us-north-1' } let(:kms_ciphertext) do @@ -64,10 +44,6 @@ ].map { |c| Base64.strict_encode64(c) }.to_json end - let(:local_ciphertext) do - 'LOCc' + %w[local1 local2 local3].map { |c| Base64.strict_encode64(c) }.to_json - end - let(:kms_enabled) { true } describe '#encrypt' do @@ -111,7 +87,7 @@ it 'encrypts with a local key' do result = subject.encrypt(plaintext, encryption_context) - expect(result).to eq(local_ciphertext) + expect(result).to_not include(plaintext) end end @@ -136,8 +112,11 @@ end context 'with a ciphertext encrypted with a local key' do + let(:kms_enabled) { false } + it 'decrypts the ciphertext with a local key' do - result = subject.decrypt(local_ciphertext, encryption_context) + ciphertext = subject.encrypt(plaintext, encryption_context) + result = subject.decrypt(ciphertext, encryption_context) expect(result).to eq(plaintext) end diff --git a/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb b/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb new file mode 100644 index 00000000000..62c503890cb --- /dev/null +++ b/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb @@ -0,0 +1,115 @@ +require 'rails_helper' + +RSpec.describe Encryption::MultiRegionKmsProfileMigrator do + let(:profile) { create(:profile, pii: pii) } + let(:user_password) { profile.user.password } + let(:personal_key) { PersonalKeyGenerator.new(profile.user).normalize(profile.personal_key) } + let(:pii) do + { + dob: '1920-01-01', + ssn: '666-66-1234', + first_name: 'Jane', + last_name: 'Doe', + zipcode: '20001', + } + end + + subject { described_class.new(profile) } + + before do + allow(IdentityConfig.store).to receive(:aws_kms_multi_region_read_enabled).and_return(true) + end + + describe '#migrate!' do + context 'for a user without multi-region ciphertexts' do + it 'migrates the single-region ciphertext and saves it to the profile' do + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + profile.encrypted_pii = pii_encryptor.encrypt(pii.to_json, user_uuid: profile.user.uuid) + recovery_pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + profile.encrypted_pii_recovery = recovery_pii_encryptor.encrypt( + pii.to_json, user_uuid: profile.user.uuid + ) + profile.encrypted_pii_multi_region = nil + profile.encrypted_pii_recovery_multi_region = nil + profile.save + subject.migrate! + + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + + single_region_pii = pii_encryptor.decrypt( + profile.encrypted_pii, + user_uuid: profile.user.uuid, + ) + multi_region_pii = pii_encryptor.decrypt( + profile.encrypted_pii_multi_region, + user_uuid: profile.user.uuid, + ) + + expect(profile.encrypted_pii).to_not be_blank + expect(profile.encrypted_pii_multi_region).to_not be_blank + expect(profile.encrypted_pii).to_not eq(profile.encrypted_pii_multi_region) + expect(single_region_pii).to eq(multi_region_pii) + + pii_recovery_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + + single_region_pii_recovery = pii_recovery_encryptor.decrypt( + profile.encrypted_pii_recovery, + user_uuid: profile.user.uuid, + ) + multi_region_pii_recovery = pii_recovery_encryptor.decrypt( + profile.encrypted_pii_recovery_multi_region, + user_uuid: profile.user.uuid, + ) + + expect(profile.encrypted_pii_recovery).to_not be_blank + expect(profile.encrypted_pii_recovery_multi_region).to_not be_blank + expect(profile.encrypted_pii_recovery).to_not eq( + profile.encrypted_pii_recovery_multi_region, + ) + + expect(single_region_pii_recovery).to eq(multi_region_pii_recovery) + end + end + + context 'for a user with multi-region ciphertexts' do + it 'does not modify the profile record' do + expect do + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + profile.encrypted_pii = pii_encryptor.encrypt(pii.to_json, user_uuid: profile.user.uuid) + recovery_pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + profile.encrypted_pii_recovery = recovery_pii_encryptor.encrypt( + pii.to_json, user_uuid: profile.user.uuid + ) + profile.save + + subject.migrate! + end.to_not change { + profile.attributes.slice( + :encrypted_pii, + :encrypted_pii_multi_region, + :encrypted_pii_recovery, + :encrypted_pii_recovery_multi_region, + ) + } + end + end + + context 'for a user without multi-region or single-region ciphertexts' do + before do + profile.update!( + encrypted_pii: nil, + encrypted_pii_multi_region: nil, + encrypted_pii_recovery: nil, + encrypted_pii_recovery_multi_region: nil, + ) + end + + it 'does not modify the profile record' do + expect { subject.migrate! }.to raise_error( + RuntimeError, + "Profile##{profile.id} is missing encrypted_pii or encrypted_pii_recovery", + ) + end + end + end +end