From 5965bf6a3ebb4010e5771029f4d846df754cb21a Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 25 Aug 2021 14:16:54 -0700 Subject: [PATCH 01/14] Remove Readthis in favor of ActiveSupport::Cache::RedisCacheStore - Also just use plain redis connections elsewhere in the app --- Gemfile | 2 +- Gemfile.lock | 5 +---- app/services/encrypted_redis_struct_storage.rb | 8 ++++---- config/initializers/rack_attack.rb | 5 ++--- spec/models/document_capture_session_spec.rb | 2 +- spec/services/encrypted_redis_struct_storage_spec.rb | 10 ++++------ 6 files changed, 13 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index 0821c9086d8..95c8b3dfe2e 100644 --- a/Gemfile +++ b/Gemfile @@ -24,6 +24,7 @@ gem 'aws-sdk-kms', '~> 1.4' gem 'aws-sdk-ses', '~> 1.6' gem 'base32-crockford' gem 'blueprinter', '~> 0.25.3' +gem 'connection_pool' gem 'device_detector' gem 'devise', '~> 4.8' gem 'dotiw', '>= 4.0.1' @@ -48,7 +49,6 @@ gem 'rack-attack', '>= 6.2.1' gem 'rack-cors', '>= 1.0.5', require: 'rack/cors' gem 'rack-headers_filter' gem 'rack-timeout', require: false -gem 'readthis' gem 'redacted_struct' gem 'redis', '>= 3.2.0' gem 'redis-namespace' diff --git a/Gemfile.lock b/Gemfile.lock index c926002abdf..5242d6be5e1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -491,9 +491,6 @@ GEM rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) - readthis (2.2.0) - connection_pool (~> 2.1) - redis (>= 3.0, < 5.0) redacted_struct (1.1.0) redis (4.3.1) redis-namespace (1.8.1) @@ -716,6 +713,7 @@ DEPENDENCIES bundler-audit capybara-screenshot (>= 1.0.23) capybara-selenium (>= 0.0.6) + connection_pool derailed_benchmarks (~> 1.8) device_detector devise (~> 4.8) @@ -769,7 +767,6 @@ DEPENDENCIES rails-controller-testing (>= 1.0.4) rails-erd (>= 1.6.0) raise-if-root - readthis redacted_struct redis (>= 3.2.0) redis-namespace diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index 787e9f13e50..78b791dc2af 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -19,7 +19,7 @@ module EncryptedRedisStructStorage def load(id, type:) check_for_id_property!(type) - ciphertext = READTHIS_POOL.with { |client| client.read(key(id, type: type)) } + ciphertext = REDIS_POOL.with { |client| client.get(key(id, type: type)) } return nil if ciphertext.blank? json = Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext) @@ -51,11 +51,11 @@ def store(struct, expires_in: 60) payload.transform_values!(&utf_8_encode_strs) - READTHIS_POOL.with do |client| - client.write( + REDIS_POOL.with do |client| + client.setex( key(struct.id, type: struct.class), + expires_in, Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), - expires_in: expires_in, ) end end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 29b24cc92ff..ed4e481f927 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -22,11 +22,10 @@ def headers # Note: The store is only used for throttling and fail2ban filtering; # not blocklisting & safelisting. It must implement .increment and .write - # like ActiveSupport::Cache::Store - cache = Readthis::Cache.new( + cache = ActiveSupport::Cache::RedisCacheStore.new( + url: IdentityConfig.store.redis_throttle_url, expires_in: 2.weeks.to_i, - redis: { url: IdentityConfig.store.redis_throttle_url, driver: :hiredis }, ) Rack::Attack.cache.store = cache diff --git a/spec/models/document_capture_session_spec.rb b/spec/models/document_capture_session_spec.rb index bd8ed617d83..33a07781dc0 100644 --- a/spec/models/document_capture_session_spec.rb +++ b/spec/models/document_capture_session_spec.rb @@ -21,7 +21,7 @@ result_id = record.result_id key = EncryptedRedisStructStorage.key(result_id, type: DocumentCaptureSessionResult) - data = READTHIS_POOL.with { |client| client.read(key) } + data = REDIS_POOL.with { |client| client.get(key) } expect(data).to be_a(String) expect(data).to_not include('Testy') expect(data).to_not include('Testerson') diff --git a/spec/services/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb index 5b9c34ee775..023d089eec3 100644 --- a/spec/services/encrypted_redis_struct_storage_spec.rb +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -119,8 +119,8 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - data = READTHIS_POOL.with do |client| - client.read(EncryptedRedisStructStorage.key(id, type: struct_class)) + data = REDIS_POOL.with do |client| + client.get(EncryptedRedisStructStorage.key(id, type: struct_class)) end expect(data).to be_a(String) @@ -134,10 +134,8 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - ttl = READTHIS_POOL.with do |client| - client.pool.with do |redis| - redis.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) - end + ttl = REDIS_POOL.with do |client| + client.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) end expect(ttl).to be <= 60 From b599c324fbe9068728fffd35598742e5bebd102b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 25 Aug 2021 14:21:09 -0700 Subject: [PATCH 02/14] remove READTHIS_POOL --- config/initializers/redis.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb index 99e902dcc5f..5c3212320e5 100644 --- a/config/initializers/redis.rb +++ b/config/initializers/redis.rb @@ -1,11 +1,3 @@ -READTHIS_POOL = ConnectionPool.new(size: 10) do - # LG-5030: remove Readthis gem - Readthis::Cache.new( - expires_in: IdentityConfig.store.service_provider_request_ttl_hours.hours.to_i, - redis: { url: IdentityConfig.store.redis_url, driver: :hiredis }, - ) -end - REDIS_POOL = ConnectionPool.new(size: 10) do Redis::Namespace.new( 'redis-pool', From 448ba45c052f12451d482e013c731f67c36c1c79 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 25 Aug 2021 14:26:03 -0700 Subject: [PATCH 03/14] WIP --- app/services/service_provider_request_proxy.rb | 14 +++++++++----- spec/rails_helper.rb | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/services/service_provider_request_proxy.rb b/app/services/service_provider_request_proxy.rb index 5fac6e76792..51b05b8daad 100644 --- a/app/services/service_provider_request_proxy.rb +++ b/app/services/service_provider_request_proxy.rb @@ -18,7 +18,7 @@ def self.from_uuid(uuid) def self.delete(request_id) return unless request_id - READTHIS_POOL.with do |client| + REDIS_POOL.with do |client| client.delete(key(request_id)) self.redis_last_uuid = nil if Rails.env.test? end @@ -26,7 +26,7 @@ def self.delete(request_id) def self.find_by(uuid:) return if uuid.blank? - obj = READTHIS_POOL.with { |client| client.read(key(uuid)) } + obj = REDIS_POOL.with { |client| client.get(key(uuid)) } obj ? hash_to_spr(obj, uuid) : nil end @@ -56,8 +56,12 @@ def self.create(hash) end def self.write(obj, uuid) - READTHIS_POOL.with do |client| - client.write(key(uuid), obj) + REDIS_POOL.with do |client| + client.setex( + key(uuid), + IdentityConfig.store.service_provider_request_ttl_hours.hours.to_i, + obj, + ) self.redis_last_uuid = uuid if Rails.env.test? end end @@ -76,7 +80,7 @@ def self.key(uuid) end def self.flush - READTHIS_POOL.with(&:clear) if Rails.env.test? + REDIS_POOL.with(&:flushdb) if Rails.env.test? end def self.hash_to_spr(hash, uuid) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7248f1b68b2..96031fde8a9 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,7 +49,7 @@ Rails.application.load_seed begin - READTHIS_POOL.with { |cache| cache.pool.with(&:info) } + REDIS_POOL.with { |client| client.info } rescue RuntimeError => error puts error puts 'It appears Redis is not running, but it is required for (some) specs to run' From b05549ec410004096ed7a97945d625436855c454 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 25 Aug 2021 15:54:14 -0700 Subject: [PATCH 04/14] Revert "remove READTHIS_POOL" This reverts commit dab2211d99b57def8d434dc30e65500993a2bbb7. --- app/services/service_provider_request_proxy.rb | 14 +++++--------- config/initializers/redis.rb | 8 ++++++++ spec/rails_helper.rb | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/services/service_provider_request_proxy.rb b/app/services/service_provider_request_proxy.rb index 51b05b8daad..5fac6e76792 100644 --- a/app/services/service_provider_request_proxy.rb +++ b/app/services/service_provider_request_proxy.rb @@ -18,7 +18,7 @@ def self.from_uuid(uuid) def self.delete(request_id) return unless request_id - REDIS_POOL.with do |client| + READTHIS_POOL.with do |client| client.delete(key(request_id)) self.redis_last_uuid = nil if Rails.env.test? end @@ -26,7 +26,7 @@ def self.delete(request_id) def self.find_by(uuid:) return if uuid.blank? - obj = REDIS_POOL.with { |client| client.get(key(uuid)) } + obj = READTHIS_POOL.with { |client| client.read(key(uuid)) } obj ? hash_to_spr(obj, uuid) : nil end @@ -56,12 +56,8 @@ def self.create(hash) end def self.write(obj, uuid) - REDIS_POOL.with do |client| - client.setex( - key(uuid), - IdentityConfig.store.service_provider_request_ttl_hours.hours.to_i, - obj, - ) + READTHIS_POOL.with do |client| + client.write(key(uuid), obj) self.redis_last_uuid = uuid if Rails.env.test? end end @@ -80,7 +76,7 @@ def self.key(uuid) end def self.flush - REDIS_POOL.with(&:flushdb) if Rails.env.test? + READTHIS_POOL.with(&:clear) if Rails.env.test? end def self.hash_to_spr(hash, uuid) diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb index 5c3212320e5..99e902dcc5f 100644 --- a/config/initializers/redis.rb +++ b/config/initializers/redis.rb @@ -1,3 +1,11 @@ +READTHIS_POOL = ConnectionPool.new(size: 10) do + # LG-5030: remove Readthis gem + Readthis::Cache.new( + expires_in: IdentityConfig.store.service_provider_request_ttl_hours.hours.to_i, + redis: { url: IdentityConfig.store.redis_url, driver: :hiredis }, + ) +end + REDIS_POOL = ConnectionPool.new(size: 10) do Redis::Namespace.new( 'redis-pool', diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 96031fde8a9..7248f1b68b2 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,7 +49,7 @@ Rails.application.load_seed begin - REDIS_POOL.with { |client| client.info } + READTHIS_POOL.with { |cache| cache.pool.with(&:info) } rescue RuntimeError => error puts error puts 'It appears Redis is not running, but it is required for (some) specs to run' From 488e54fff607b476e9dee0d7035cb1f45c1a78e4 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 25 Aug 2021 15:55:59 -0700 Subject: [PATCH 05/14] wip --- Gemfile | 1 + Gemfile.lock | 4 ++++ app/services/encrypted_redis_struct_storage.rb | 8 ++++---- spec/models/document_capture_session_spec.rb | 2 +- spec/services/encrypted_redis_struct_storage_spec.rb | 10 ++++++---- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Gemfile b/Gemfile index 95c8b3dfe2e..c89b1070322 100644 --- a/Gemfile +++ b/Gemfile @@ -49,6 +49,7 @@ gem 'rack-attack', '>= 6.2.1' gem 'rack-cors', '>= 1.0.5', require: 'rack/cors' gem 'rack-headers_filter' gem 'rack-timeout', require: false +gem 'readthis' gem 'redacted_struct' gem 'redis', '>= 3.2.0' gem 'redis-namespace' diff --git a/Gemfile.lock b/Gemfile.lock index 5242d6be5e1..45e5b5d22f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -491,6 +491,9 @@ GEM rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) + readthis (2.2.0) + connection_pool (~> 2.1) + redis (>= 3.0, < 5.0) redacted_struct (1.1.0) redis (4.3.1) redis-namespace (1.8.1) @@ -767,6 +770,7 @@ DEPENDENCIES rails-controller-testing (>= 1.0.4) rails-erd (>= 1.6.0) raise-if-root + readthis redacted_struct redis (>= 3.2.0) redis-namespace diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index 78b791dc2af..787e9f13e50 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -19,7 +19,7 @@ module EncryptedRedisStructStorage def load(id, type:) check_for_id_property!(type) - ciphertext = REDIS_POOL.with { |client| client.get(key(id, type: type)) } + ciphertext = READTHIS_POOL.with { |client| client.read(key(id, type: type)) } return nil if ciphertext.blank? json = Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext) @@ -51,11 +51,11 @@ def store(struct, expires_in: 60) payload.transform_values!(&utf_8_encode_strs) - REDIS_POOL.with do |client| - client.setex( + READTHIS_POOL.with do |client| + client.write( key(struct.id, type: struct.class), - expires_in, Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), + expires_in: expires_in, ) end end diff --git a/spec/models/document_capture_session_spec.rb b/spec/models/document_capture_session_spec.rb index 33a07781dc0..bd8ed617d83 100644 --- a/spec/models/document_capture_session_spec.rb +++ b/spec/models/document_capture_session_spec.rb @@ -21,7 +21,7 @@ result_id = record.result_id key = EncryptedRedisStructStorage.key(result_id, type: DocumentCaptureSessionResult) - data = REDIS_POOL.with { |client| client.get(key) } + data = READTHIS_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/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb index 023d089eec3..5b9c34ee775 100644 --- a/spec/services/encrypted_redis_struct_storage_spec.rb +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -119,8 +119,8 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - data = REDIS_POOL.with do |client| - client.get(EncryptedRedisStructStorage.key(id, type: struct_class)) + data = READTHIS_POOL.with do |client| + client.read(EncryptedRedisStructStorage.key(id, type: struct_class)) end expect(data).to be_a(String) @@ -134,8 +134,10 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - ttl = REDIS_POOL.with do |client| - client.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) + ttl = READTHIS_POOL.with do |client| + client.pool.with do |redis| + redis.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) + end end expect(ttl).to be <= 60 From fbfe11fc865c2accd5fa1f2d8c7335498a1bfda0 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 15:32:19 -0700 Subject: [PATCH 06/14] namespace new rack-attack cache --- config/initializers/rack_attack.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index ed4e481f927..652415d9a74 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -21,15 +21,13 @@ def headers ### Configure Cache ### # Note: The store is only used for throttling and fail2ban filtering; - # not blocklisting & safelisting. It must implement .increment and .write - - cache = ActiveSupport::Cache::RedisCacheStore.new( + # not blocklisting & safelisting + Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new( + namespace: 'rack-attack', url: IdentityConfig.store.redis_throttle_url, expires_in: 2.weeks.to_i, ) - Rack::Attack.cache.store = cache - ### Configure Safelisting ### # Always allow requests from localhost From ae7f2f40f78a65c729d3ba5909af36efdb6bd54c Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 15:59:21 -0700 Subject: [PATCH 07/14] wip, dual reads + dual writes --- .../encrypted_redis_struct_storage.rb | 15 ++++++----- .../service_provider_request_proxy.rb | 27 ++++++++++++++----- spec/models/document_capture_session_spec.rb | 2 +- spec/rails_helper.rb | 2 +- .../encrypted_redis_struct_storage_spec.rb | 10 +++---- 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/app/services/encrypted_redis_struct_storage.rb b/app/services/encrypted_redis_struct_storage.rb index 787e9f13e50..18324ce51f9 100644 --- a/app/services/encrypted_redis_struct_storage.rb +++ b/app/services/encrypted_redis_struct_storage.rb @@ -19,7 +19,8 @@ module EncryptedRedisStructStorage def load(id, type:) check_for_id_property!(type) - ciphertext = READTHIS_POOL.with { |client| client.read(key(id, type: type)) } + ciphertext = REDIS_POOL.with { |client| client.get(key(id, type: type)) } || + READTHIS_POOL.with { |client| client.read(key(id, type: type)) } return nil if ciphertext.blank? json = Encryption::Encryptors::SessionEncryptor.new.decrypt(ciphertext) @@ -51,12 +52,14 @@ def store(struct, expires_in: 60) payload.transform_values!(&utf_8_encode_strs) + struct_key = key(struct.id, type: struct.class) + ciphertext = Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json) + + REDIS_POOL.with do |client| + client.setex(struct_key, expires_in, ciphertext) + end READTHIS_POOL.with do |client| - client.write( - key(struct.id, type: struct.class), - Encryption::Encryptors::SessionEncryptor.new.encrypt(payload.to_json), - expires_in: expires_in, - ) + client.write(struct_key, ciphertext, expires_in: expires_in) end end diff --git a/app/services/service_provider_request_proxy.rb b/app/services/service_provider_request_proxy.rb index 5fac6e76792..b13217c2d16 100644 --- a/app/services/service_provider_request_proxy.rb +++ b/app/services/service_provider_request_proxy.rb @@ -18,15 +18,18 @@ def self.from_uuid(uuid) def self.delete(request_id) return unless request_id - READTHIS_POOL.with do |client| - client.delete(key(request_id)) - self.redis_last_uuid = nil if Rails.env.test? - end + REDIS_POOL.with { |client| client.del(key(request_id)) } + READTHIS_POOL.with { |client| client.delete(key(request_id)) } + self.redis_last_uuid = nil if Rails.env.test? end def self.find_by(uuid:) return if uuid.blank? - obj = READTHIS_POOL.with { |client| client.read(key(uuid)) } + obj = REDIS_POOL.with do |client| + str = client.get(key(uuid)) + JSON.parse(str, symbolize_names: true) if str + end + obj ||= READTHIS_POOL.with { |client| client.read(key(uuid)) } obj ? hash_to_spr(obj, uuid) : nil end @@ -56,10 +59,17 @@ def self.create(hash) end def self.write(obj, uuid) + REDIS_POOL.with do |client| + client.setex( + key(uuid), + IdentityConfig.store.service_provider_request_ttl_hours.hours.to_i, + obj.to_json, + ) + end READTHIS_POOL.with do |client| client.write(key(uuid), obj) - self.redis_last_uuid = uuid if Rails.env.test? end + self.redis_last_uuid = uuid if Rails.env.test? end def self.create!(hash) @@ -76,7 +86,10 @@ def self.key(uuid) end def self.flush - READTHIS_POOL.with(&:clear) if Rails.env.test? + if Rails.env.test? + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + READTHIS_POOL.with(&:clear) + end end def self.hash_to_spr(hash, uuid) diff --git a/spec/models/document_capture_session_spec.rb b/spec/models/document_capture_session_spec.rb index bd8ed617d83..dd6a656c97f 100644 --- a/spec/models/document_capture_session_spec.rb +++ b/spec/models/document_capture_session_spec.rb @@ -21,7 +21,7 @@ result_id = record.result_id key = EncryptedRedisStructStorage.key(result_id, type: DocumentCaptureSessionResult) - data = READTHIS_POOL.with { |client| client.read(key) } + data = REDIS.with { |client| client.get(key) } expect(data).to be_a(String) expect(data).to_not include('Testy') expect(data).to_not include('Testerson') diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7248f1b68b2..96031fde8a9 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,7 +49,7 @@ Rails.application.load_seed begin - READTHIS_POOL.with { |cache| cache.pool.with(&:info) } + REDIS_POOL.with { |client| client.info } rescue RuntimeError => error puts error puts 'It appears Redis is not running, but it is required for (some) specs to run' diff --git a/spec/services/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb index 5b9c34ee775..62e0d01d576 100644 --- a/spec/services/encrypted_redis_struct_storage_spec.rb +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -119,8 +119,8 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - data = READTHIS_POOL.with do |client| - client.read(EncryptedRedisStructStorage.key(id, type: struct_class)) + data = REDIS_POOL.with do |client| + client.get(EncryptedRedisStructStorage.key(id, type: struct_class)) end expect(data).to be_a(String) @@ -134,10 +134,8 @@ def self.redis_key_prefix struct_class.new(id: id, a: 'value for a', b: 'value for b', c: 'value for c'), ) - ttl = READTHIS_POOL.with do |client| - client.pool.with do |redis| - redis.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) - end + ttl = REDIS_POOL.with do |redis| + redis.ttl(EncryptedRedisStructStorage.key(id, type: struct_class)) end expect(ttl).to be <= 60 From 750e4576421e2f1555884f36cdaa37e3a801b338 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:01:52 -0700 Subject: [PATCH 08/14] remove connection_pool explicit dependency, we get it implicitly through readthis for now :shrug: --- Gemfile | 1 - Gemfile.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/Gemfile b/Gemfile index c89b1070322..0821c9086d8 100644 --- a/Gemfile +++ b/Gemfile @@ -24,7 +24,6 @@ gem 'aws-sdk-kms', '~> 1.4' gem 'aws-sdk-ses', '~> 1.6' gem 'base32-crockford' gem 'blueprinter', '~> 0.25.3' -gem 'connection_pool' gem 'device_detector' gem 'devise', '~> 4.8' gem 'dotiw', '>= 4.0.1' diff --git a/Gemfile.lock b/Gemfile.lock index 45e5b5d22f3..c926002abdf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -716,7 +716,6 @@ DEPENDENCIES bundler-audit capybara-screenshot (>= 1.0.23) capybara-selenium (>= 0.0.6) - connection_pool derailed_benchmarks (~> 1.8) device_detector devise (~> 4.8) From 1567b5a292a6fde585b799a13aa6601ea09446bd Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:04:23 -0700 Subject: [PATCH 09/14] typo --- spec/models/document_capture_session_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/document_capture_session_spec.rb b/spec/models/document_capture_session_spec.rb index dd6a656c97f..33a07781dc0 100644 --- a/spec/models/document_capture_session_spec.rb +++ b/spec/models/document_capture_session_spec.rb @@ -21,7 +21,7 @@ result_id = record.result_id key = EncryptedRedisStructStorage.key(result_id, type: DocumentCaptureSessionResult) - data = REDIS.with { |client| client.get(key) } + data = REDIS_POOL.with { |client| client.get(key) } expect(data).to be_a(String) expect(data).to_not include('Testy') expect(data).to_not include('Testerson') From 8d9aa8e92b7b552922085cffbd405c6f71c69ee8 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:05:11 -0700 Subject: [PATCH 10/14] deprecation warning --- spec/rails_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 96031fde8a9..444a2844d09 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,7 +49,7 @@ Rails.application.load_seed begin - REDIS_POOL.with { |client| client.info } + REDIS_POOL.with { |namespaced| namespaced.client.info } rescue RuntimeError => error puts error puts 'It appears Redis is not running, but it is required for (some) specs to run' From 425f5435fb777d61abc5aaf739f6b9a1051f0931 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:13:03 -0700 Subject: [PATCH 11/14] rubocop --- app/services/service_provider_request_proxy.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/service_provider_request_proxy.rb b/app/services/service_provider_request_proxy.rb index b13217c2d16..2828c8af10e 100644 --- a/app/services/service_provider_request_proxy.rb +++ b/app/services/service_provider_request_proxy.rb @@ -26,9 +26,9 @@ def self.delete(request_id) def self.find_by(uuid:) return if uuid.blank? obj = REDIS_POOL.with do |client| - str = client.get(key(uuid)) - JSON.parse(str, symbolize_names: true) if str - end + str = client.get(key(uuid)) + JSON.parse(str, symbolize_names: true) if str + end obj ||= READTHIS_POOL.with { |client| client.read(key(uuid)) } obj ? hash_to_spr(obj, uuid) : nil end From 53fcede73e86102a30bca7dd2f208d77f78f6bb7 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:16:49 -0700 Subject: [PATCH 12/14] fix before suite --- spec/rails_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 444a2844d09..9cb4908d6ba 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,7 +49,7 @@ Rails.application.load_seed begin - REDIS_POOL.with { |namespaced| namespaced.client.info } + REDIS_POOL.with { |namespaced| namespaced.redis.info } rescue RuntimeError => error puts error puts 'It appears Redis is not running, but it is required for (some) specs to run' From 8fe5e6950e1cbe6e262fe35329d3754e896d8c27 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:26:49 -0700 Subject: [PATCH 13/14] Add specs for migration behavior --- .../encrypted_redis_struct_storage_spec.rb | 23 +++++++++++++++++++ .../service_provider_request_proxy_spec.rb | 17 ++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/spec/services/encrypted_redis_struct_storage_spec.rb b/spec/services/encrypted_redis_struct_storage_spec.rb index 62e0d01d576..07c81c6c103 100644 --- a/spec/services/encrypted_redis_struct_storage_spec.rb +++ b/spec/services/encrypted_redis_struct_storage_spec.rb @@ -56,6 +56,29 @@ def self.redis_key_prefix expect(loaded_result.b).to eq('b') expect(loaded_result.c).to eq('c') end + + context 'with a struct stored by Readthis gem' do + before do + READTHIS_POOL.with do |client| + struct = struct_class.new(id: id, a: 'a', b: 'b', c: 'c') + client.write( + EncryptedRedisStructStorage.key(struct.id, type: struct.class), + Encryption::Encryptors::SessionEncryptor.new.encrypt( + struct.as_json.tap { |s| s.delete('id') }.to_json, + ), + expires_in: 5, + ) + end + end + + it 'loads the value still' do + loaded_result = load_struct + + expect(loaded_result.a).to eq('a') + expect(loaded_result.b).to eq('b') + expect(loaded_result.c).to eq('c') + end + end end context 'with an ordered initializer struct' do diff --git a/spec/services/service_provider_request_proxy_spec.rb b/spec/services/service_provider_request_proxy_spec.rb index 33b0db5b690..28b90fcaa8a 100644 --- a/spec/services/service_provider_request_proxy_spec.rb +++ b/spec/services/service_provider_request_proxy_spec.rb @@ -39,6 +39,23 @@ expect(sp_request.loa).to eq(sp_request.ial) expect(ServiceProviderRequestProxy.from_uuid('123')).to eq sp_request end + + context 'with a value stored by Readthis gem' do + let(:uuid) { SecureRandom.uuid } + + before do + READTHIS_POOL.with do |client| + client.write( + ServiceProviderRequestProxy.key(uuid), + issuer: 'foo' + ) + end + end + + it 'loads the data' do + expect(ServiceProviderRequestProxy.from_uuid(uuid).issuer).to eq('foo') + end + end end context 'when the record does not exist' do From eedf20a4b4b00c9543c95807b7744ef3a3aa3f7b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 27 Aug 2021 16:40:49 -0700 Subject: [PATCH 14/14] lint; --- spec/services/service_provider_request_proxy_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/services/service_provider_request_proxy_spec.rb b/spec/services/service_provider_request_proxy_spec.rb index 28b90fcaa8a..4edb585dd14 100644 --- a/spec/services/service_provider_request_proxy_spec.rb +++ b/spec/services/service_provider_request_proxy_spec.rb @@ -45,10 +45,7 @@ before do READTHIS_POOL.with do |client| - client.write( - ServiceProviderRequestProxy.key(uuid), - issuer: 'foo' - ) + client.write(ServiceProviderRequestProxy.key(uuid), issuer: 'foo') end end