Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
43 changes: 20 additions & 23 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
73 changes: 73 additions & 0 deletions app/services/encryption/multi_region_kms_profile_migrator.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ChangeNullConstraintOnSpReturnLog < ActiveRecord::Migration[8.0]
def change
change_column_null(:sp_return_logs, :requested_at, true)
end
end
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions docs/attempts-api/schemas/events/IdentityProofingEvents.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
description: |
User manually submitted an IdV address
allOf:
- $ref: '../shared/EventProperties.yml'
- type: object
properties:
address:
type: string
38 changes: 38 additions & 0 deletions lib/tasks/backfill_profiles.rake
Original file line number Diff line number Diff line change
@@ -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
82 changes: 82 additions & 0 deletions spec/lib/tasks/backfill_profiles_rake_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading