-
Notifications
You must be signed in to change notification settings - Fork 166
LG-8056 Encrypt document submissions and write them to S3 #7351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f175bac
4424f81
c79ce45
1f0ee92
ad2dee3
51b4003
934fac4
e2f634c
43ce17f
0c6ea3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,13 +11,14 @@ class ApiImageUploadForm | |
| validate :throttle_if_rate_limited | ||
|
|
||
| def initialize(params, service_provider:, analytics: nil, | ||
| uuid_prefix: nil, irs_attempts_api_tracker: nil) | ||
| uuid_prefix: nil, irs_attempts_api_tracker: nil, store_encrypted_images: false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another quick thought (maybe follow-up PR material) should also add this to the DocumentProofingJob in case we ever ship async image upload?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just saw this :jinx:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A note on the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related ticket LG-7390 |
||
| @params = params | ||
| @service_provider = service_provider | ||
| @analytics = analytics | ||
| @readable = {} | ||
| @uuid_prefix = uuid_prefix | ||
| @irs_attempts_api_tracker = irs_attempts_api_tracker | ||
| @store_encrypted_images = store_encrypted_images | ||
| end | ||
|
|
||
| def submit | ||
|
|
@@ -64,8 +65,8 @@ def validate_form | |
|
|
||
| def post_images_to_client | ||
| response = doc_auth_client.post_images( | ||
| front_image: front.read, | ||
| back_image: back.read, | ||
| front_image: front_image_bytes, | ||
| back_image: back_image_bytes, | ||
| image_source: image_source, | ||
| user_uuid: user_uuid, | ||
| uuid_prefix: uuid_prefix, | ||
|
|
@@ -79,6 +80,14 @@ def post_images_to_client | |
| response | ||
| end | ||
|
|
||
| def front_image_bytes | ||
| @front_image_bytes ||= front.read | ||
| end | ||
|
|
||
| def back_image_bytes | ||
| @back_image_bytes ||= back.read | ||
| end | ||
|
|
||
| def validate_pii_from_doc(client_response) | ||
| response = Idv::DocPiiForm.new( | ||
| pii: client_response.pii_from_doc, | ||
|
|
@@ -210,6 +219,7 @@ def update_analytics(client_response) | |
| ).merge(native_camera_ab_test_data), | ||
| ) | ||
| pii_from_doc = client_response.pii_from_doc || {} | ||
| store_encrypted_images_if_required | ||
| irs_attempts_api_tracker.idv_document_upload_submitted( | ||
| success: client_response.success?, | ||
| document_state: pii_from_doc[:state], | ||
|
|
@@ -224,6 +234,23 @@ def update_analytics(client_response) | |
| ) | ||
| end | ||
|
|
||
| def store_encrypted_images_if_required | ||
| return unless store_encrypted_images? | ||
|
|
||
| encrypted_document_storage_writer.encrypt_and_write_document( | ||
| front_image: front_image_bytes, | ||
| back_image: back_image_bytes, | ||
| ) | ||
| end | ||
|
|
||
| def store_encrypted_images? | ||
| @store_encrypted_images | ||
| end | ||
|
|
||
| def encrypted_document_storage_writer | ||
| @encrypted_document_storage_writer ||= EncryptedDocumentStorage::DocumentWriter.new | ||
| end | ||
|
|
||
| def native_camera_ab_test_data | ||
| return {} unless IdentityConfig.store.idv_native_camera_a_b_testing_enabled | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| module EncryptedDocumentStorage | ||
| class DocumentWriter | ||
| def encrypt_and_write_document(front_image:, back_image:) | ||
| key = SecureRandom.bytes(32) | ||
| encrypted_front_image = aes_cipher.encrypt(front_image, key) | ||
| encrypted_back_image = aes_cipher.encrypt(back_image, key) | ||
|
|
||
| front_image_uuid = SecureRandom.uuid | ||
| back_image_uiid = SecureRandom.uuid | ||
|
|
||
| storage.write_image(encrypted_image: encrypted_front_image, name: front_image_uuid) | ||
| storage.write_image(encrypted_image: encrypted_back_image, name: back_image_uiid) | ||
|
|
||
| WriteDocumentResult.new( | ||
| front_uuid: front_image_uuid, | ||
| back_uuid: back_image_uiid, | ||
| front_encryption_key: Base64.strict_encode64(key), | ||
| back_encryption_key: Base64.strict_encode64(key), | ||
| ) | ||
| end | ||
|
|
||
| def storage | ||
| @storage ||= begin | ||
| if Rails.env.production? | ||
jmhooper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| S3Storage.new | ||
| else | ||
| LocalStorage.new | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def aes_cipher | ||
| @aes_cipher ||= Encryption::AesCipher.new | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| module EncryptedDocumentStorage | ||
| class LocalStorage | ||
| def write_image(encrypted_image:, name:) | ||
| FileUtils.mkdir_p(tmp_document_storage_dir) | ||
| filepath = tmp_document_storage_dir.join(name) | ||
| File.write(filepath, encrypted_image) | ||
| end | ||
|
|
||
| def tmp_document_storage_dir | ||
| Rails.root.join('tmp', 'encrypted_doc_storage') | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| module EncryptedDocumentStorage | ||
| class S3Storage | ||
| def write_image(encrypted_image:, name:) | ||
| s3_client.put_object( | ||
| bucket: IdentityConfig.store.encrypted_document_storage_s3_bucket, | ||
| body: encrypted_image, | ||
| key: name, | ||
| ) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def s3_client | ||
| Aws::S3::Client.new( | ||
| http_open_timeout: 5, | ||
| http_read_timeout: 5, | ||
| compute_checksums: false, | ||
| ) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| module EncryptedDocumentStorage | ||
| WriteDocumentResult = Struct.new( | ||
| :front_uuid, | ||
| :back_uuid, | ||
| :front_encryption_key, | ||
| :back_encryption_key, | ||
| keyword_init: true, | ||
| ) | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe EncryptedDocumentStorage::DocumentWriter do | ||
| describe '#encrypt_and_write_document' do | ||
| it 'encrypts the document and writes it to storage' do | ||
| front_image = 'hello, i am the front image' | ||
| back_image = 'hello, i am the back image' | ||
|
|
||
| result = EncryptedDocumentStorage::DocumentWriter.new.encrypt_and_write_document( | ||
| front_image: front_image, | ||
| back_image: back_image, | ||
| ) | ||
jmhooper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| front_file_path = Rails.root.join('tmp', 'encrypted_doc_storage', result.front_uuid) | ||
| back_file_path = Rails.root.join('tmp', 'encrypted_doc_storage', result.back_uuid) | ||
| front_key = Base64.strict_decode64(result.front_encryption_key) | ||
| back_key = Base64.strict_decode64(result.back_encryption_key) | ||
|
|
||
| aes_cipher = Encryption::AesCipher.new | ||
|
|
||
| written_front_image = aes_cipher.decrypt( | ||
| File.read(front_file_path), | ||
| front_key, | ||
| ) | ||
| written_back_image = aes_cipher.decrypt( | ||
| File.read(back_file_path), | ||
| back_key, | ||
| ) | ||
|
|
||
| expect(written_front_image).to eq(front_image) | ||
| expect(written_back_image).to eq(back_image) | ||
| end | ||
| end | ||
|
|
||
| describe '#storage' do | ||
| subject { EncryptedDocumentStorage::DocumentWriter.new } | ||
|
|
||
| context 'in production' do | ||
| it 'is uses S3' do | ||
| allow(Rails.env).to receive(:production?).and_return(true) | ||
|
|
||
| expect(subject.storage).to be_a(EncryptedDocumentStorage::S3Storage) | ||
| end | ||
| end | ||
|
|
||
| context 'outside production' do | ||
| it 'it uses the disk' do | ||
| allow(Rails.env).to receive(:production?).and_return(false) | ||
|
|
||
| expect(subject.storage).to be_a(EncryptedDocumentStorage::LocalStorage) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe EncryptedDocumentStorage::LocalStorage do | ||
| describe '#write_image' do | ||
| it 'writes the document to the disk' do | ||
| encrypted_image = "hello, i'm the encrypted document." | ||
| name = SecureRandom.uuid | ||
|
|
||
| EncryptedDocumentStorage::LocalStorage.new.write_image( | ||
| encrypted_image: encrypted_image, | ||
| name: name, | ||
| ) | ||
|
|
||
| result = File.read( | ||
| Rails.root.join('tmp', 'encrypted_doc_storage', name), | ||
| ) | ||
| expect(result).to eq(encrypted_image) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe EncryptedDocumentStorage::S3Storage do | ||
| describe '#write_image' do | ||
| it 'writes the document to S3' do | ||
| encrypted_image = 'hello, i am the encrypted document.' | ||
| name = '123abc' | ||
|
|
||
| storage = EncryptedDocumentStorage::S3Storage.new | ||
|
|
||
| stubbed_s3_client = Aws::S3::Client.new(stub_responses: true) | ||
| allow(storage).to receive(:s3_client).and_return(stubbed_s3_client) | ||
|
|
||
| expect(stubbed_s3_client).to receive(:put_object).and_call_original | ||
| stubbed_s3_client.stub_responses( | ||
| :put_object, | ||
| ->(context) { | ||
| params = context.params | ||
| expect(params[:bucket]).to eq(IdentityConfig.store.encrypted_document_storage_s3_bucket) | ||
| expect(params[:key]).to eq(name) | ||
| expect(params[:body]).to eq(encrypted_image) | ||
| }, | ||
| ) | ||
|
|
||
| storage.write_image(encrypted_image: encrypted_image, name: name) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not great coverage here. The way I'd like to test this is to look for the reference UUIDs that get sent to the fake attempts API and make sure a file was written there. Since these aren't getting bubbled up to the attempts API yet we'll need to either come up with something else temporarily or hold off.