From 24fbf448ca44d8d0fc51ee6edb180f63d260578a Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 10:59:56 -0700 Subject: [PATCH 1/7] Create EncryptedRedisStructStorage - A mixin for storing structs in Redis that are encrypted and have an expiration --- .../document_capture_session_result.rb | 63 +--------- .../encrypted_redis_struct_storage.rb | 90 +++++++++++++ ...roofing_document_capture_session_result.rb | 60 +-------- .../encrypted_redis_struct_storage_spec.rb | 118 ++++++++++++++++++ 4 files changed, 215 insertions(+), 116 deletions(-) create mode 100644 app/services/encrypted_redis_struct_storage.rb create mode 100644 spec/services/encrypted_redis_struct_storage_spec.rb diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index 6686a074c3e..0b3b3e469a8 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -1,62 +1,7 @@ -class DocumentCaptureSessionResult - REDIS_KEY_PREFIX = 'dcs:result'.freeze +DocumentCaptureSessionResult = Struct.new(:id, :success, :pii, keyword_init: true) do + include EncryptedRedisStructStorage - attr_reader :id, :success, :pii + configure_encrypted_redis_struct key_prefix: 'dcs:result' - alias success? success - alias pii_from_doc pii - - class << self - def load(id) - ciphertext = REDIS_POOL.with { |client| client.read(key(id)) } - return nil if ciphertext.blank? - decrypt_and_deserialize(id, ciphertext) - end - - def store(id:, success:, pii:) - result = new(id: id, success: success, pii: pii) - REDIS_POOL.with do |client| - client.write(key(id), result.serialize_and_encrypt, expires_in: 60) - end - end - - def key(id) - [REDIS_KEY_PREFIX, id].join(':') - end - - private - - def decrypt_and_deserialize(id, ciphertext) - deserialize( - id, - Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext), - ) - end - - def deserialize(id, json) - data = JSON.parse(json) - new( - id: id, - success: data['success'], - pii: data['pii'], - ) - end - end - - def initialize(id:, success:, pii:) - @id = id - @success = success - @pii = pii - end - - def serialize - { - success: success, - pii: pii, - }.to_json - end - - def serialize_and_encrypt - Encryption::Encryptors::SessionEncryptor.new.encrypt(serialize) - end + alias_method :success?, :success end diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb new file mode 100644 index 00000000000..4509f5d9868 --- /dev/null +++ b/app/services/encrypted_redis_struct_storage.rb @@ -0,0 +1,90 @@ +# Include this mixin in a plain Struct to add ability to store it in redis +# Call +configure_encrypted_redis_struct+ with configuration values +# +# @example +# MyStruct = Struct.new(:id, :a, :b) do +# include EncryptedRedisStructStorage +# configure_encrypted_redis_struct key_prefix: 'myprefix' +# end +# +# MyStruct.store(id: '123', a: 'a', 'b') +# s = MyStruct.load('123') +module EncryptedRedisStructStorage + def self.included(klass) + if klass.members.include?(:id) + klass.extend ClassMethods + else + raise 'EncryptedRedisStructStorage can only be included in classes that have an id key' + end + end + + # Assigns member fields from a hash. That way, it doesn't matter + # if a Struct was created with keyword_init or not (and we can't currently + # reflect on that field) + # @param [Hash] values + def init_fields(values) + values.each do |key, value| + self[key] = value + end + end + + module ClassMethods + attr_reader :expires_in + + def configure_encrypted_redis_struct(key_prefix:, expires_in: 60) + @redis_key_prefix = key_prefix.dup.freeze + @expires_in = expires_in + end + + def load(id) + ciphertext = REDIS_POOL.with { |client| client.read(key(id)) } + return nil if ciphertext.blank? + decrypt_and_deserialize(id, ciphertext) + end + + def store(id:, **rest) + result = new.tap do |struct| + struct.id = id + struct.init_fields(rest) + end + + REDIS_POOL.with do |client| + payload = result.as_json + payload.delete('id') + + + client.write( + key(id), + Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), + expires_in: self.expires_in, + ) + end + end + + def key(id) + [self.redis_key_prefix, id].join(':') + end + + def redis_key_prefix + return @redis_key_prefix if @redis_key_prefix.present? + raise "#{self.name} no redis_key_prefix! make sure to call configure_encrypted_redis_struct" + end + + private + + def decrypt_and_deserialize(id, ciphertext) + deserialize( + id, + Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext), + ) + end + + def deserialize(id, json) + data = JSON.parse(json) + new.tap do |struct| + struct.id = id + struct.init_fields(data) + end + end + end +end diff --git a/app/services/proofing_document_capture_session_result.rb b/app/services/proofing_document_capture_session_result.rb index d5cd829bb32..c4daa9aedd1 100644 --- a/app/services/proofing_document_capture_session_result.rb +++ b/app/services/proofing_document_capture_session_result.rb @@ -1,59 +1,5 @@ -class ProofingDocumentCaptureSessionResult - REDIS_KEY_PREFIX = 'dcs-proofing:result'.freeze +ProofingDocumentCaptureSessionResult = Struct.new(:id, :pii, :result, keyword_init: true) do + include EncryptedRedisStructStorage - attr_reader :id, :pii, :result - - class << self - def load(id) - ciphertext = REDIS_POOL.with { |client| client.read(key(id)) } - return nil if ciphertext.blank? - decrypt_and_deserialize(id, ciphertext) - end - - def store(id:, pii:, result:) - result = new(id: id, pii: pii, result: result) - REDIS_POOL.with do |client| - client.write(key(id), result.serialize_and_encrypt, expires_in: 60) - end - end - - def key(id) - [REDIS_KEY_PREFIX, id].join(':') - end - - private - - def decrypt_and_deserialize(id, ciphertext) - deserialize( - id, - Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext), - ) - end - - def deserialize(id, json) - data = JSON.parse(json) - new( - id: id, - pii: data['pii'], - result: data['result'], - ) - end - end - - def initialize(id:, pii:, result:) - @id = id - @pii = pii - @result = result - end - - def serialize - { - pii: pii, - result: result, - }.to_json - end - - def serialize_and_encrypt - Encryption::Encryptors::SessionEncryptor.new.encrypt(serialize) - end + configure_encrypted_redis_struct key_prefix: 'dcs-proofing:result' end diff --git a/spec/services/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb new file mode 100644 index 00000000000..34c03e5845a --- /dev/null +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -0,0 +1,118 @@ +require 'rails_helper' + +RSpec.describe EncryptedRedisStructStorage do + describe 'including' do + it 'errors when included in a struct that does not have an id property' do + expect do + Struct.new(:not_id) do + include EncryptedRedisStructStorage + end + end.to raise_error(/can only be included in classes that have an id key/) + end + end + + describe '.redis_key_prefix' do + context 'in a struct where the key was configured' do + it 'returns the key prefix' do + klass = Struct.new(:id) do + include EncryptedRedisStructStorage + + configure_encrypted_redis_struct key_prefix: 'abc' + end + + expect(klass.redis_key_prefix).to eq('abc') + end + end + + context 'in a struct where the key was not configured' do + it 'raises an error with instructions to call configure_encrypted_redis_struct' do + klass = Struct.new(:id) do + include EncryptedRedisStructStorage + end + + expect { klass.redis_key_prefix }.to raise_error(/configure_encrypted_redis_struct/) + end + end + end + + let(:id) { SecureRandom.uuid } + let(:example_struct) do + Struct.new(:id, :a, :b, :c, keyword_init: true) do + include EncryptedRedisStructStorage + + configure_encrypted_redis_struct key_prefix: 'example:struct' + end + end + + describe '.key' do + it 'generates a key' do + key = example_struct.key(id) + expect(key).to eq('example:struct:' + id) + end + end + + describe '.store' do + it 'writes encrypted data to redis' do + example_struct.store(id: id, a: 'value for a', b: 'value for b', c: 'value for c') + + data = REDIS_POOL.with { |client| client.read(example_struct.key(id)) } + + expect(data).to be_a(String) + expect(data).to_not include('value for a') + expect(data).to_not include('value for b') + expect(data).to_not include('value for c') + end + + it 'stores the value with a ttl (expiration)' do + example_struct.store(id: id, a: 'value for a', b: 'value for b', c: 'value for c') + + ttl = REDIS_POOL.with do |client| + client.pool.with do |redis| + redis.ttl(example_struct.key(id)) + end + end + + expect(ttl).to be <= 60 + end + end + + describe '.load' do + it 'returns nil if no data exists' do + loaded_result = example_struct.load(SecureRandom.uuid) + + expect(loaded_result).to eq(nil) + end + + context 'with a keyword init struct' do + it 'loads the value out of redis' do + example_struct.store(id: id, a: 'a', b: 'b', c: 'c') + + loaded_result = example_struct.load(id) + + expect(loaded_result.a).to eq('a') + expect(loaded_result.b).to eq('b') + expect(loaded_result.c).to eq('c') + end + end + + context 'with an ordered initializer struct' do + let(:example_struct) do + Struct.new(:id, :d, :e, :f, keyword_init: false) do + include EncryptedRedisStructStorage + + configure_encrypted_redis_struct key_prefix: 'example:struct' + end + end + + it 'loads the value out of redis' do + example_struct.store(id: id, d: 'd', e: 'e', f: 'f') + + loaded_result = example_struct.load(id) + + expect(loaded_result.d).to eq('d') + expect(loaded_result.e).to eq('e') + expect(loaded_result.f).to eq('f') + end + end + end +end From 2a8e33e00dbc7b4a28eff1c01fd2145e0926774e Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 11:07:41 -0700 Subject: [PATCH 2/7] Remove blank line --- app/services/encrypted_redis_struct_storage.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index 4509f5d9868..24611c0ac69 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -52,7 +52,6 @@ def store(id:, **rest) payload = result.as_json payload.delete('id') - client.write( key(id), Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), From e25a09d6664576d80c8837bcb994e02728a7ed4c Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 14:21:44 -0700 Subject: [PATCH 3/7] PR feedback: Refactor EncryptedRedisStructStorage to be a service class instead of a mixin --- app/models/document_capture_session.rb | 34 ++-- .../document_capture_session_result.rb | 8 +- .../encrypted_redis_struct_storage.rb | 120 ++++++------ ...roofing_document_capture_session_result.rb | 8 +- spec/models/document_capture_session_spec.rb | 3 +- .../document_capture_session_result_spec.rb | 34 +--- .../encrypted_redis_struct_storage_spec.rb | 171 ++++++++++-------- ...ng_document_capture_session_result_spec.rb | 24 +++ 8 files changed, 214 insertions(+), 188 deletions(-) create mode 100644 spec/services/proofing_document_capture_session_result_spec.rb diff --git a/app/models/document_capture_session.rb b/app/models/document_capture_session.rb index a27f25ae1a7..080d0246e74 100644 --- a/app/models/document_capture_session.rb +++ b/app/models/document_capture_session.rb @@ -4,36 +4,42 @@ class DocumentCaptureSession < ApplicationRecord belongs_to :user def load_result - DocumentCaptureSessionResult.load(result_id) + EncryptedRedisStructStorage.load(result_id, type: DocumentCaptureSessionResult) end def load_proofing_result - ProofingDocumentCaptureSessionResult.load(result_id) + EncryptedRedisStructStorage.load(result_id, type: ProofingDocumentCaptureSessionResult) end def store_result_from_response(doc_auth_response) - DocumentCaptureSessionResult.store( - id: generate_result_id, - success: doc_auth_response.success?, - pii: doc_auth_response.pii_from_doc, + EncryptedRedisStructStorage.store( + DocumentCaptureSessionResult.new( + id: generate_result_id, + success: doc_auth_response.success?, + pii: doc_auth_response.pii_from_doc, + ), ) save! end def store_proofing_pii_from_doc(pii_from_doc) - ProofingDocumentCaptureSessionResult.store( - id: generate_result_id, - pii: pii_from_doc, - result: nil, + EncryptedRedisStructStorage.store( + ProofingDocumentCaptureSessionResult.new( + id: generate_result_id, + pii: pii_from_doc, + result: nil, + ), ) save! end def store_proofing_result(pii_from_doc, result) - ProofingDocumentCaptureSessionResult.store( - id: result_id, - pii: pii_from_doc, - result: result, + EncryptedRedisStructStorage.store( + ProofingDocumentCaptureSessionResult.new( + id: result_id, + pii: pii_from_doc, + result: result, + ), ) end diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index 0b3b3e469a8..94dd7a447c6 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -1,7 +1,9 @@ -DocumentCaptureSessionResult = Struct.new(:id, :success, :pii, keyword_init: true) do - include EncryptedRedisStructStorage +# frozen_string_literal: true - configure_encrypted_redis_struct key_prefix: 'dcs:result' +DocumentCaptureSessionResult = Struct.new(:id, :success, :pii, keyword_init: true) do + def self.redis_key_prefix + 'dcs:result' + end alias_method :success?, :success end diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index 24611c0ac69..e7ca8e8b6bd 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -1,89 +1,75 @@ -# Include this mixin in a plain Struct to add ability to store it in redis -# Call +configure_encrypted_redis_struct+ with configuration values +# Use this class to store a plain Struct in redis. It will be stored +# encrypted and by default will expire, the struct must have a +redis_key_prefix+ +# class method # # @example # MyStruct = Struct.new(:id, :a, :b) do -# include EncryptedRedisStructStorage -# configure_encrypted_redis_struct key_prefix: 'myprefix' +# def self.redis_key_prefix +# 'mystruct' +# end # end # -# MyStruct.store(id: '123', a: 'a', 'b') -# s = MyStruct.load('123') +# struct = MyStruct.new('id123', 'a', 'b') +# +# EncryptedRedisStructStorage.store(struct) +# s = EncryptedRedisStructStorage.load('id123', type: MyStruct) module EncryptedRedisStructStorage - def self.included(klass) - if klass.members.include?(:id) - klass.extend ClassMethods - else - raise 'EncryptedRedisStructStorage can only be included in classes that have an id key' - end - end + module_function - # Assigns member fields from a hash. That way, it doesn't matter - # if a Struct was created with keyword_init or not (and we can't currently - # reflect on that field) - # @param [Hash] values - def init_fields(values) - values.each do |key, value| - self[key] = value - end - end - - module ClassMethods - attr_reader :expires_in + def load(id, type:) + check_for_id_property!(type) - def configure_encrypted_redis_struct(key_prefix:, expires_in: 60) - @redis_key_prefix = key_prefix.dup.freeze - @expires_in = expires_in - end + ciphertext = REDIS_POOL.with { |client| client.read(key(id, type: type)) } + return nil if ciphertext.blank? - def load(id) - ciphertext = REDIS_POOL.with { |client| client.read(key(id)) } - return nil if ciphertext.blank? - decrypt_and_deserialize(id, ciphertext) + json = Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext) + data = JSON.parse(json).with_indifferent_access + type.new.tap do |struct| + struct.id = id + init_fields(struct: struct, data: data) end + end - def store(id:, **rest) - result = new.tap do |struct| - struct.id = id - struct.init_fields(rest) - end + def store(struct, expires_in: 60) + check_for_id_property!(struct.class) + check_for_empty_id!(struct.id) - REDIS_POOL.with do |client| - payload = result.as_json - payload.delete('id') + payload = struct.as_json + payload.delete('id') - client.write( - key(id), - Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), - expires_in: self.expires_in, - ) - end + REDIS_POOL.with do |client| + client.write( + key(struct.id, type: struct.class), + Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), + expires_in: expires_in, + ) end + end - def key(id) - [self.redis_key_prefix, id].join(':') + def key(id, type:) + if type.respond_to?(:redis_key_prefix) + [type.redis_key_prefix, id].join(':') + else + raise "#{self} expected #{type.name} to have defined class method redis_key_prefix" end + end - def redis_key_prefix - return @redis_key_prefix if @redis_key_prefix.present? - raise "#{self.name} no redis_key_prefix! make sure to call configure_encrypted_redis_struct" + # Assigns member fields from a hash. That way, it doesn't matter + # if a Struct was created with keyword_init or not (and we can't currently + # reflect on that field) + # @param [Hash] data + def init_fields(struct:, data:) + data.each do |key, value| + struct[key] = value end + end - private - - def decrypt_and_deserialize(id, ciphertext) - deserialize( - id, - Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext), - ) - end + def check_for_id_property!(type) + return if type.members.include?(:id) + raise "#{self} expected #{type.name} to have an id property" + end - def deserialize(id, json) - data = JSON.parse(json) - new.tap do |struct| - struct.id = id - struct.init_fields(data) - end - end + def check_for_empty_id!(id) + raise ArgumentError, 'id cannot be empty' if id.blank? end end diff --git a/app/services/proofing_document_capture_session_result.rb b/app/services/proofing_document_capture_session_result.rb index c4daa9aedd1..7fb1dc87cce 100644 --- a/app/services/proofing_document_capture_session_result.rb +++ b/app/services/proofing_document_capture_session_result.rb @@ -1,5 +1,7 @@ -ProofingDocumentCaptureSessionResult = Struct.new(:id, :pii, :result, keyword_init: true) do - include EncryptedRedisStructStorage +# frozen_string_literal: true - configure_encrypted_redis_struct key_prefix: 'dcs-proofing:result' +ProofingDocumentCaptureSessionResult = Struct.new(:id, :pii, :result, keyword_init: true) do + def self.redis_key_prefix + 'dcs-proofing:result' + end end diff --git a/spec/models/document_capture_session_spec.rb b/spec/models/document_capture_session_spec.rb index 1a7f5bddb00..3763afa15ca 100644 --- a/spec/models/document_capture_session_spec.rb +++ b/spec/models/document_capture_session_spec.rb @@ -18,7 +18,8 @@ record.store_result_from_response(doc_auth_response) result_id = record.result_id - data = REDIS_POOL.with { |client| client.read(DocumentCaptureSessionResult.key(result_id)) } + key = EncryptedRedisStructStorage.key(result_id, type: DocumentCaptureSessionResult) + data = REDIS_POOL.with { |client| client.read(key) } expect(data).to be_a(String) expect(data).to_not include('Testy') expect(data).to_not include('Testerson') diff --git a/spec/services/document_capture_session_result_spec.rb b/spec/services/document_capture_session_result_spec.rb index 25cf8e260e5..5a19dd8f338 100644 --- a/spec/services/document_capture_session_result_spec.rb +++ b/spec/services/document_capture_session_result_spec.rb @@ -5,40 +5,16 @@ let(:success) { true } let(:pii) { { 'first_name' => 'Testy', 'last_name' => 'Testerson' } } - describe '.key' do - it 'generates a key' do - key = DocumentCaptureSessionResult.key(id) - expect(key).to eq('dcs:result:' + id) - end - end - - describe '.store' do - it 'writes encrypted data to redis' do - DocumentCaptureSessionResult.store(id: id, success: success, pii: pii) + context 'EncryptedRedisStructStorage' do + it 'works with EncryptedRedisStructStorage' do + result = DocumentCaptureSessionResult.new(id: id, success: success, pii: pii) - data = REDIS_POOL.with { |client| client.read(DocumentCaptureSessionResult.key(id)) } - - expect(data).to be_a(String) - expect(data).to_not include('Testy') - expect(data).to_not include('Testerson') - end - end - - describe '.load' do - it 'reads the unloaded result from the session' do - DocumentCaptureSessionResult.store(id: id, success: success, pii: pii) - - loaded_result = DocumentCaptureSessionResult.load(id) + EncryptedRedisStructStorage.store(result) + loaded_result = EncryptedRedisStructStorage.load(id, type: DocumentCaptureSessionResult) expect(loaded_result.id).to eq(id) expect(loaded_result.success?).to eq(success) expect(loaded_result.pii).to eq(pii) end - - it 'returns nil if no data exists in redis' do - loaded_result = DocumentCaptureSessionResult.load(SecureRandom.uuid) - - expect(loaded_result).to eq(nil) - end end end diff --git a/spec/services/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb index 34c03e5845a..abcf341e912 100644 --- a/spec/services/encrypted_redis_struct_storage_spec.rb +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -1,93 +1,56 @@ require 'rails_helper' RSpec.describe EncryptedRedisStructStorage do - describe 'including' do - it 'errors when included in a struct that does not have an id property' do - expect do - Struct.new(:not_id) do - include EncryptedRedisStructStorage - end - end.to raise_error(/can only be included in classes that have an id key/) - end - end - - describe '.redis_key_prefix' do - context 'in a struct where the key was configured' do - it 'returns the key prefix' do - klass = Struct.new(:id) do - include EncryptedRedisStructStorage - - configure_encrypted_redis_struct key_prefix: 'abc' - end - - expect(klass.redis_key_prefix).to eq('abc') - end - end - - context 'in a struct where the key was not configured' do - it 'raises an error with instructions to call configure_encrypted_redis_struct' do - klass = Struct.new(:id) do - include EncryptedRedisStructStorage - end - - expect { klass.redis_key_prefix }.to raise_error(/configure_encrypted_redis_struct/) - end - end - end - let(:id) { SecureRandom.uuid } - let(:example_struct) do - Struct.new(:id, :a, :b, :c, keyword_init: true) do - include EncryptedRedisStructStorage - configure_encrypted_redis_struct key_prefix: 'example:struct' + let(:struct_class) do + Struct.new(:id, :a, :b, :c, keyword_init: true) do + def self.redis_key_prefix + 'example:prefix' + end end end describe '.key' do - it 'generates a key' do - key = example_struct.key(id) - expect(key).to eq('example:struct:' + id) - end - end - - describe '.store' do - it 'writes encrypted data to redis' do - example_struct.store(id: id, a: 'value for a', b: 'value for b', c: 'value for c') + subject(:key) { EncryptedRedisStructStorage.key(id, type: struct_class) } - data = REDIS_POOL.with { |client| client.read(example_struct.key(id)) } - - expect(data).to be_a(String) - expect(data).to_not include('value for a') - expect(data).to_not include('value for b') - expect(data).to_not include('value for c') + context 'with a struct that has a redis_key_prefix' do + it 'prefixes the id' do + expect(key).to eq("example:prefix:#{id}") + end end - it 'stores the value with a ttl (expiration)' do - example_struct.store(id: id, a: 'value for a', b: 'value for b', c: 'value for c') - - ttl = REDIS_POOL.with do |client| - client.pool.with do |redis| - redis.ttl(example_struct.key(id)) - end + context 'with a struct that does not have a redis_key_prefix' do + let(:struct_class) do + Struct.new(:id, :a, :b, :c) end - expect(ttl).to be <= 60 + it 'raises an error with a message describing what to define' do + expect { key }.to raise_error(/to have defined class method redis_key_prefix/) + end end end describe '.load' do + subject(:load_struct) { EncryptedRedisStructStorage.load(id, type: struct_class) } + it 'returns nil if no data exists' do - loaded_result = example_struct.load(SecureRandom.uuid) + expect(load_struct).to eq(nil) + end - expect(loaded_result).to eq(nil) + context 'with an empty id' do + let(:id) { '' } + + it 'is nil' do + expect(load_struct).to eq(nil) + end end context 'with a keyword init struct' do it 'loads the value out of redis' do - example_struct.store(id: id, a: 'a', b: 'b', c: 'c') + EncryptedRedisStructStorage.store(struct_class.new(id: id, a: 'a', b: 'b', c: 'c')) - loaded_result = example_struct.load(id) + loaded_result = load_struct expect(loaded_result.a).to eq('a') expect(loaded_result.b).to eq('b') @@ -96,23 +59,89 @@ end context 'with an ordered initializer struct' do - let(:example_struct) do + let(:struct_class) do Struct.new(:id, :d, :e, :f, keyword_init: false) do - include EncryptedRedisStructStorage - - configure_encrypted_redis_struct key_prefix: 'example:struct' + def self.redis_key_prefix + 'abcdef' + end end end it 'loads the value out of redis' do - example_struct.store(id: id, d: 'd', e: 'e', f: 'f') + EncryptedRedisStructStorage.store( + struct_class.new(id, 'd', 'e', 'f') + ) - loaded_result = example_struct.load(id) + loaded_result = load_struct expect(loaded_result.d).to eq('d') expect(loaded_result.e).to eq('e') expect(loaded_result.f).to eq('f') end end + + context 'with a struct that does not have a redis_key_prefix' do + let(:struct_class) do + Struct.new(:id, :a, :b, :c) + end + + it 'raises an error with a message describing what to define' do + expect { load_struct }.to raise_error(/to have defined class method redis_key_prefix/) + end + end + end + + describe '.store' do + context 'with a struct that does not have an id method' do + let(:struct_class) do + Struct.new(:a, :b, :c) + end + + it 'throws an error describing the missing method' do + expect do + EncryptedRedisStructStorage.store(struct_class.new(a: 'a', b: 'b')) + end.to raise_error(/to have an id property/) + end + end + + context 'with a struct that has an id' do + context 'with an empty id' do + let(:id) { '' } + + it 'errors' do + expect { EncryptedRedisStructStorage.store(struct_class.new) }. + to raise_error(ArgumentError, 'id cannot be empty') + end + end + + it 'writes encrypted data to redis' do + EncryptedRedisStructStorage.store( + struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), + ) + + data = REDIS_POOL.with do |client| + client.read(EncryptedRedisStructStorage.key(id, type: struct_class)) + end + + expect(data).to be_a(String) + expect(data).to_not include('value for a') + expect(data).to_not include('value for b') + expect(data).to_not include('value for c') + end + + it 'stores the value with a ttl (expiration)' do + EncryptedRedisStructStorage.store( + struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), + ) + + ttl = REDIS_POOL.with do |client| + client.pool.with do |redis| + redis.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) + end + end + + expect(ttl).to be <= 60 + end + end end end diff --git a/spec/services/proofing_document_capture_session_result_spec.rb b/spec/services/proofing_document_capture_session_result_spec.rb new file mode 100644 index 00000000000..85b3619acf3 --- /dev/null +++ b/spec/services/proofing_document_capture_session_result_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe ProofingDocumentCaptureSessionResult do + let(:id) { SecureRandom.uuid } + let(:pii) { { 'first_name' => 'Testy', 'last_name' => 'Testerson' } } + let(:idv_result) { { errors: {}, messages: ['some message'] } } + + context 'EncryptedRedisStructStorage' do + it 'works with EncryptedRedisStructStorage' do + result = ProofingDocumentCaptureSessionResult.new(id: id, pii: pii, result: idv_result) + + EncryptedRedisStructStorage.store(result) + + loaded_result = EncryptedRedisStructStorage.load( + id, type: ProofingDocumentCaptureSessionResult + ) + + expect(loaded_result.id).to eq(id) + expect(loaded_result.pii).to eq(pii) + + expect(loaded_result.result).to eq(idv_result.with_indifferent_access) + end + end +end From 2d5917f087006c4e1c4177d2c59ca967f513de72 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 14:41:26 -0700 Subject: [PATCH 4/7] Looks like we use deep_symbolize_keys, not with_indifferent_access --- app/services/encrypted_redis_struct_storage.rb | 2 +- spec/services/proofing_document_capture_session_result_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index e7ca8e8b6bd..ac8d50fbe42 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -23,7 +23,7 @@ def load(id, type:) return nil if ciphertext.blank? json = Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext) - data = JSON.parse(json).with_indifferent_access + data = JSON.parse(json) type.new.tap do |struct| struct.id = id init_fields(struct: struct, data: data) diff --git a/spec/services/proofing_document_capture_session_result_spec.rb b/spec/services/proofing_document_capture_session_result_spec.rb index 85b3619acf3..f677c0d3850 100644 --- a/spec/services/proofing_document_capture_session_result_spec.rb +++ b/spec/services/proofing_document_capture_session_result_spec.rb @@ -18,7 +18,7 @@ expect(loaded_result.id).to eq(id) expect(loaded_result.pii).to eq(pii) - expect(loaded_result.result).to eq(idv_result.with_indifferent_access) + expect(loaded_result.result.deep_symbolize_keys!).to eq(idv_result) end end end From 61bd95c3f5944555c1455b2b926e00e503b28f7f Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 15:51:16 -0700 Subject: [PATCH 5/7] Maybe fix extract_pii_from_doc --- app/services/idv/steps/back_image_step.rb | 2 +- app/services/idv/steps/doc_auth_base_step.rb | 4 ++-- app/services/idv/steps/document_capture_step.rb | 4 ++-- app/services/idv/steps/link_sent_step.rb | 2 +- app/services/idv/steps/mobile_back_image_step.rb | 2 +- app/services/idv/steps/selfie_step.rb | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/services/idv/steps/back_image_step.rb b/app/services/idv/steps/back_image_step.rb index 55933b06678..fb195c7257e 100644 --- a/app/services/idv/steps/back_image_step.rb +++ b/app/services/idv/steps/back_image_step.rb @@ -25,7 +25,7 @@ def fetch_doc_auth_results_or_redirect_to_selfie if get_results_response.success? mark_step_complete(:selfie) save_proofing_components - extract_pii_from_doc(get_results_response) + extract_pii_from_doc(get_results_response.pii_from_doc) else handle_document_verification_failure(get_results_response) end diff --git a/app/services/idv/steps/doc_auth_base_step.rb b/app/services/idv/steps/doc_auth_base_step.rb index 76eac83a217..7f8e677d50f 100644 --- a/app/services/idv/steps/doc_auth_base_step.rb +++ b/app/services/idv/steps/doc_auth_base_step.rb @@ -63,9 +63,9 @@ def save_proofing_components Db::ProofingComponent::Add.call(user_id, :liveness_check, DocAuth::Client.doc_auth_vendor) end - def extract_pii_from_doc(response) + def extract_pii_from_doc(pii) current_user = User.find(user_id) - flow_session[:pii_from_doc] = response.pii_from_doc.merge( + flow_session[:pii_from_doc] = pii.merge( uuid: current_user.uuid, phone: current_user.phone_configurations.take&.phone, ) diff --git a/app/services/idv/steps/document_capture_step.rb b/app/services/idv/steps/document_capture_step.rb index 0b1ceac85cb..acd7c968553 100644 --- a/app/services/idv/steps/document_capture_step.rb +++ b/app/services/idv/steps/document_capture_step.rb @@ -21,7 +21,7 @@ def post_images_and_handle_result if response.success? save_proofing_components document_capture_session.store_result_from_response(response) - extract_pii_from_doc(response) + extract_pii_from_doc(response.pii_from_doc) response else handle_document_verification_failure(response) @@ -42,7 +42,7 @@ def handle_document_verification_failure(response) def handle_stored_result if stored_result.success? - extract_pii_from_doc(stored_result) + extract_pii_from_doc(stored_result.pii) else extra = { stored_result_present: stored_result.present? } failure(I18n.t('errors.doc_auth.acuant_network_error'), extra) diff --git a/app/services/idv/steps/link_sent_step.rb b/app/services/idv/steps/link_sent_step.rb index 1550c0ce953..99026d10a0e 100644 --- a/app/services/idv/steps/link_sent_step.rb +++ b/app/services/idv/steps/link_sent_step.rb @@ -28,7 +28,7 @@ def fetch_doc_auth_results def handle_document_verification_success(get_results_response) save_proofing_components - extract_pii_from_doc(get_results_response) + extract_pii_from_doc(get_results_response.pii_from_doc) mark_steps_complete end diff --git a/app/services/idv/steps/mobile_back_image_step.rb b/app/services/idv/steps/mobile_back_image_step.rb index bbbbebd1ef7..9ff25ca497d 100644 --- a/app/services/idv/steps/mobile_back_image_step.rb +++ b/app/services/idv/steps/mobile_back_image_step.rb @@ -24,7 +24,7 @@ def fetch_doc_auth_results_or_redirect_to_selfie if get_results_response.success? mark_step_complete(:selfie) save_proofing_components - extract_pii_from_doc(get_results_response) + extract_pii_from_doc(get_results_response.pii_from_doc) else handle_document_verification_failure(get_results_response) end diff --git a/app/services/idv/steps/selfie_step.rb b/app/services/idv/steps/selfie_step.rb index 2afa3a711e1..4741159c0a7 100644 --- a/app/services/idv/steps/selfie_step.rb +++ b/app/services/idv/steps/selfie_step.rb @@ -34,7 +34,7 @@ def handle_successful_selfie_match # DP: handle multiple clients? CaptureDoc::UpdateAcuantToken.call(user_id_from_token, flow_session[:instance_id]) else - extract_pii_from_doc(results_response) + extract_pii_from_doc(results_response.pii_from_doc) end end From 152dd1c3d984b4c1b033b3f3f1283b00bed1cb4b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 16:05:05 -0700 Subject: [PATCH 6/7] Revert "Maybe fix extract_pii_from_doc" This reverts commit 61bd95c3f5944555c1455b2b926e00e503b28f7f. --- app/services/idv/steps/back_image_step.rb | 2 +- app/services/idv/steps/doc_auth_base_step.rb | 4 ++-- app/services/idv/steps/document_capture_step.rb | 4 ++-- app/services/idv/steps/link_sent_step.rb | 2 +- app/services/idv/steps/mobile_back_image_step.rb | 2 +- app/services/idv/steps/selfie_step.rb | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/services/idv/steps/back_image_step.rb b/app/services/idv/steps/back_image_step.rb index fb195c7257e..55933b06678 100644 --- a/app/services/idv/steps/back_image_step.rb +++ b/app/services/idv/steps/back_image_step.rb @@ -25,7 +25,7 @@ def fetch_doc_auth_results_or_redirect_to_selfie if get_results_response.success? mark_step_complete(:selfie) save_proofing_components - extract_pii_from_doc(get_results_response.pii_from_doc) + extract_pii_from_doc(get_results_response) else handle_document_verification_failure(get_results_response) end diff --git a/app/services/idv/steps/doc_auth_base_step.rb b/app/services/idv/steps/doc_auth_base_step.rb index 7f8e677d50f..76eac83a217 100644 --- a/app/services/idv/steps/doc_auth_base_step.rb +++ b/app/services/idv/steps/doc_auth_base_step.rb @@ -63,9 +63,9 @@ def save_proofing_components Db::ProofingComponent::Add.call(user_id, :liveness_check, DocAuth::Client.doc_auth_vendor) end - def extract_pii_from_doc(pii) + def extract_pii_from_doc(response) current_user = User.find(user_id) - flow_session[:pii_from_doc] = pii.merge( + flow_session[:pii_from_doc] = response.pii_from_doc.merge( uuid: current_user.uuid, phone: current_user.phone_configurations.take&.phone, ) diff --git a/app/services/idv/steps/document_capture_step.rb b/app/services/idv/steps/document_capture_step.rb index acd7c968553..0b1ceac85cb 100644 --- a/app/services/idv/steps/document_capture_step.rb +++ b/app/services/idv/steps/document_capture_step.rb @@ -21,7 +21,7 @@ def post_images_and_handle_result if response.success? save_proofing_components document_capture_session.store_result_from_response(response) - extract_pii_from_doc(response.pii_from_doc) + extract_pii_from_doc(response) response else handle_document_verification_failure(response) @@ -42,7 +42,7 @@ def handle_document_verification_failure(response) def handle_stored_result if stored_result.success? - extract_pii_from_doc(stored_result.pii) + extract_pii_from_doc(stored_result) else extra = { stored_result_present: stored_result.present? } failure(I18n.t('errors.doc_auth.acuant_network_error'), extra) diff --git a/app/services/idv/steps/link_sent_step.rb b/app/services/idv/steps/link_sent_step.rb index 99026d10a0e..1550c0ce953 100644 --- a/app/services/idv/steps/link_sent_step.rb +++ b/app/services/idv/steps/link_sent_step.rb @@ -28,7 +28,7 @@ def fetch_doc_auth_results def handle_document_verification_success(get_results_response) save_proofing_components - extract_pii_from_doc(get_results_response.pii_from_doc) + extract_pii_from_doc(get_results_response) mark_steps_complete end diff --git a/app/services/idv/steps/mobile_back_image_step.rb b/app/services/idv/steps/mobile_back_image_step.rb index 9ff25ca497d..bbbbebd1ef7 100644 --- a/app/services/idv/steps/mobile_back_image_step.rb +++ b/app/services/idv/steps/mobile_back_image_step.rb @@ -24,7 +24,7 @@ def fetch_doc_auth_results_or_redirect_to_selfie if get_results_response.success? mark_step_complete(:selfie) save_proofing_components - extract_pii_from_doc(get_results_response.pii_from_doc) + extract_pii_from_doc(get_results_response) else handle_document_verification_failure(get_results_response) end diff --git a/app/services/idv/steps/selfie_step.rb b/app/services/idv/steps/selfie_step.rb index 4741159c0a7..2afa3a711e1 100644 --- a/app/services/idv/steps/selfie_step.rb +++ b/app/services/idv/steps/selfie_step.rb @@ -34,7 +34,7 @@ def handle_successful_selfie_match # DP: handle multiple clients? CaptureDoc::UpdateAcuantToken.call(user_id_from_token, flow_session[:instance_id]) else - extract_pii_from_doc(results_response.pii_from_doc) + extract_pii_from_doc(results_response) end end From 9531d725454b635694884686334abc572b7ec9a7 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 17 Sep 2020 16:05:35 -0700 Subject: [PATCH 7/7] alias pii/pii_from_doc --- app/services/document_capture_session_result.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/document_capture_session_result.rb b/app/services/document_capture_session_result.rb index 94dd7a447c6..fc903f1bae8 100644 --- a/app/services/document_capture_session_result.rb +++ b/app/services/document_capture_session_result.rb @@ -6,4 +6,5 @@ def self.redis_key_prefix end alias_method :success?, :success + alias_method :pii_from_doc, :pii end