diff --git a/Gemfile b/Gemfile index a50142a77b9..5f57c861658 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,6 @@ gem 'ahoy_matey', '~> 3.0' gem 'autoprefixer-rails', '~> 10.0' gem 'aws-sdk-kms', '~> 1.4' gem 'aws-sdk-ses', '~> 1.6' -gem 'aws-sdk-eventbridge' gem 'base32-crockford' gem 'blueprinter', '~> 0.25.3' gem 'device_detector' diff --git a/Gemfile.lock b/Gemfile.lock index 2c27d0334bc..dce059bf720 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -130,9 +130,6 @@ GEM aws-partitions (~> 1, >= 1.239.0) aws-sigv4 (~> 1.1) jmespath (~> 1.0) - aws-sdk-eventbridge (1.18.0) - aws-sdk-core (~> 3, >= 3.109.0) - aws-sigv4 (~> 1.1) aws-sdk-kms (1.40.0) aws-sdk-core (~> 3, >= 3.109.0) aws-sigv4 (~> 1.1) @@ -704,7 +701,6 @@ DEPENDENCIES ahoy_matey (~> 3.0) autoprefixer-rails (~> 10.0) aws-sdk-cloudwatchlogs - aws-sdk-eventbridge aws-sdk-kms (~> 1.4) aws-sdk-ses (~> 1.6) axe-core-rspec (~> 4.2) diff --git a/app/jobs/risc_delivery_job.rb b/app/jobs/risc_delivery_job.rb new file mode 100644 index 00000000000..e821496b834 --- /dev/null +++ b/app/jobs/risc_delivery_job.rb @@ -0,0 +1,62 @@ +class RiscDeliveryJob < ApplicationJob + queue_as :low + + retry_on Faraday::TimeoutError, Faraday::ConnectionFailed, wait: :exponentially_longer + + def perform( + push_notification_url:, + jwt:, + event_type:, + issuer: + ) + response = faraday.post( + push_notification_url, + jwt, + 'Accept' => 'application/json', + 'Content-Type' => 'application/secevent+jwt', + ) do |req| + req.options.context = { + service_name: inline? ? 'risc_http_push_direct' : 'risc_http_push_async', + } + end + + unless response.success? + Rails.logger.warn( + { + event: 'http_push_error', + transport: inline? ? 'direct' : 'async', + event_type: event_type, + service_provider: issuer, + status: response.status, + }.to_json, + ) + end + rescue Faraday::TimeoutError, Faraday::ConnectionFailed => err + raise err if !inline? + + Rails.logger.warn( + { + event: 'http_push_error', + transport: 'direct', + event_type: event_type, + service_provider: issuer, + error: err.message, + }.to_json, + ) + end + + def faraday + Faraday.new do |f| + f.request :instrumentation, name: 'request_log.faraday' + f.adapter :net_http + f.options.timeout = 3 + f.options.read_timeout = 3 + f.options.open_timeout = 3 + f.options.write_timeout = 3 + end + end + + def inline? + queue_adapter.is_a?(ActiveJob::QueueAdapters::InlineAdapter) + end +end diff --git a/app/services/push_notification/http_push.rb b/app/services/push_notification/http_push.rb index 958b41e09aa..b58918d70ed 100644 --- a/app/services/push_notification/http_push.rb +++ b/app/services/push_notification/http_push.rb @@ -38,72 +38,21 @@ def url_options def deliver_one(service_provider) deliver_local(service_provider) if IdentityConfig.store.risc_notifications_local_enabled - if IdentityConfig.store.risc_notifications_eventbridge_enabled - deliver_eventbridge(service_provider) - else - deliver_direct(service_provider) - end - end - - def deliver_eventbridge(service_provider) - response = eventbridge_client.put_events( - entries: [ - { - time: now, - source: service_provider.issuer, - detail_type: 'notification', - detail: { jwt: jwt(service_provider) }.to_json, - event_bus_name: "#{Identity::Hostdata.env}-risc-notifications", - }, - ], - ) - - if response.failed_entry_count.to_i > 0 - Rails.logger.warn( - { - event: 'http_push_error', - transport: 'eventbridge', - event_type: event.event_type, - service_provider: service_provider.issuer, - error: response.to_s, - }.to_json, + if IdentityConfig.store.risc_notifications_active_job_enabled + RiscDeliveryJob.perform_later( + push_notification_url: service_provider.push_notification_url, + jwt: jwt(service_provider), + event_type: event.event_type, + issuer: service_provider.issuer, ) - end - end - - def deliver_direct(service_provider) - response = faraday.post( - service_provider.push_notification_url, - jwt(service_provider), - 'Accept' => 'application/json', - 'Content-Type' => 'application/secevent+jwt', - ) do |req| - req.options.context = { service_name: 'http_push_direct' } - end - - unless response.success? - Rails.logger.warn( - { - event: 'http_push_error', - transport: 'direct', - event_type: event.event_type, - service_provider: service_provider.issuer, - status: response.status, - }.to_json, + else + RiscDeliveryJob.perform_now( + push_notification_url: service_provider.push_notification_url, + jwt: jwt(service_provider), + event_type: event.event_type, + issuer: service_provider.issuer, ) end - rescue Faraday::TimeoutError, - Faraday::ConnectionFailed, - PushNotification::PushNotificationError => err - Rails.logger.warn( - { - event: 'http_push_error', - transport: 'direct', - event_type: event.event_type, - service_provider: service_provider.issuer, - error: err.message, - }.to_json, - ) end def deliver_local(service_provider) @@ -134,13 +83,6 @@ def jwt_payload(service_provider) } end - def faraday - Faraday.new do |f| - f.request :instrumentation, name: 'request_log.faraday' - f.adapter :net_http - end - end - def agency_uuid(service_provider) AgencyIdentity.find_by( user_id: event.user.id, @@ -151,11 +93,5 @@ def agency_uuid(service_provider) service_provider: service_provider.issuer, )&.uuid end - - def eventbridge_client - @eventbridge_client ||= Aws::EventBridge::Client.new( - region: Identity::Hostdata.aws_region, - ) - end end end diff --git a/app/services/push_notification/push_notification_error.rb b/app/services/push_notification/push_notification_error.rb deleted file mode 100644 index d7f682e1106..00000000000 --- a/app/services/push_notification/push_notification_error.rb +++ /dev/null @@ -1,4 +0,0 @@ -module PushNotification - class PushNotificationError < StandardError - end -end diff --git a/config/application.yml.default b/config/application.yml.default index 15db291b02a..716bfa60632 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -177,7 +177,7 @@ requests_per_ip_track_only_mode: 'false' reset_password_email_max_attempts: '20' reset_password_email_window_in_minutes: '60' risc_notifications_local_enabled: 'false' -risc_notifications_eventbridge_enabled: 'false' +risc_notifications_active_job_enabled: 'false' ruby_workers_enabled: 'true' rules_of_use_horizon_years: '6' rules_of_use_updated_at: '2021-05-21T00:00:00Z' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index cb98ad0df69..6b72fa8919c 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -244,7 +244,7 @@ def self.build_store(config_map) config.add(:reset_password_email_max_attempts, type: :integer) config.add(:reset_password_email_window_in_minutes, type: :integer) config.add(:risc_notifications_local_enabled, type: :boolean) - config.add(:risc_notifications_eventbridge_enabled, type: :boolean) + config.add(:risc_notifications_active_job_enabled, type: :boolean) config.add(:ruby_workers_enabled, type: :boolean) config.add(:rules_of_use_horizon_years, type: :integer) config.add(:rules_of_use_updated_at, type: :timestamp) diff --git a/spec/jobs/risc_delivery_job_spec.rb b/spec/jobs/risc_delivery_job_spec.rb new file mode 100644 index 00000000000..6ab64350d7b --- /dev/null +++ b/spec/jobs/risc_delivery_job_spec.rb @@ -0,0 +1,68 @@ +require 'rails_helper' + +RSpec.describe RiscDeliveryJob do + describe '#perform' do + let(:push_notification_url) { 'https://push.example.gov' } + let(:jwt) { JWT.encode({ foo: 'bar' }, 'a') } + let(:event_type) { PushNotification::IdentifierRecycledEvent::EVENT_TYPE } + let(:issuer) { 'issuer1' } + let(:transport) { 'ruby_worker' } + + let(:job) { RiscDeliveryJob.new } + subject(:perform) do + job.perform( + push_notification_url: push_notification_url, + jwt: jwt, + event_type: event_type, + issuer: issuer, + ) + end + + it 'POSTs the jwt to the given URL' do + req = stub_request(:post, push_notification_url). + with( + body: jwt, + headers: { + 'Content-Type' => 'application/secevent+jwt', + 'Accept' => 'application/json', + }, + ) + + perform + + expect(req).to have_been_requested + end + + context 'network errors' do + before do + stub_request(:post, push_notification_url).to_timeout + end + + context 'when performed inline' do + it 'warns on timeouts' do + expect(Rails.logger).to receive(:warn) do |msg| + payload = JSON.parse(msg, symbolize_names: true) + + expect(payload[:event]).to eq('http_push_error') + expect(payload[:transport]).to eq('direct') + end + + expect { perform }.to_not raise_error + end + end + + context 'when performed in a worker' do + before do + allow(job).to receive(:queue_adapter). + and_return(ActiveJob::QueueAdapters::GoodJobAdapter.new) + end + + it 'raises on timeouts (and retries via ActiveJob)' do + expect(Rails.logger).to_not receive(:warn) + + expect { perform }.to raise_error(Faraday::ConnectionFailed) + end + end + end + end +end diff --git a/spec/services/push_notification/http_push_spec.rb b/spec/services/push_notification/http_push_spec.rb index 45d24e38958..9c6566b129e 100644 --- a/spec/services/push_notification/http_push_spec.rb +++ b/spec/services/push_notification/http_push_spec.rb @@ -22,20 +22,17 @@ ) end let(:now) { Time.zone.now } - let(:risc_notifications_eventbridge_enabled) { false } + let(:risc_notifications_active_job_enabled) { false } let(:push_notifications_enabled) { true } - let(:eventbridge_client) { Aws::EventBridge::Client.new(stub_responses: true) } subject(:http_push) { PushNotification::HttpPush.new(event, now: now) } before do - allow(IdentityConfig.store).to receive(:risc_notifications_eventbridge_enabled). - and_return(risc_notifications_eventbridge_enabled) + allow(IdentityConfig.store).to receive(:risc_notifications_active_job_enabled). + and_return(risc_notifications_active_job_enabled) allow(Identity::Hostdata).to receive(:env).and_return('dev') allow(IdentityConfig.store).to receive(:push_notifications_enabled). and_return(push_notifications_enabled) - - allow(http_push).to receive(:eventbridge_client).and_return(eventbridge_client) end describe '#deliver' do @@ -51,52 +48,24 @@ end end - context 'when the EventBridge is disabled' do - let(:risc_notifications_eventbridge_enabled) { false } - - it 'makes an HTTP post to service providers with a push_notification_url' do - stub_request(:post, sp_with_push_url.push_notification_url). - with do |request| - expect(request.headers['Content-Type']).to eq('application/secevent+jwt') - expect(request.headers['Accept']).to eq('application/json') - - payload, headers = JWT.decode( - request.body, - AppArtifacts.store.oidc_public_key, - true, - algorithm: 'RS256', - ) - - expect(headers['typ']).to eq('secevent+jwt') + context 'when risc_notifications_active_job_enabled is enabled' do + let(:risc_notifications_active_job_enabled) { true } - expect(payload['iss']).to eq(root_url) - expect(payload['iat']).to eq(now.to_i) - expect(payload['exp']).to eq((now + 12.hours).to_i) - expect(payload['aud']).to eq(sp_with_push_url.push_notification_url) - expect(payload['events']).to eq(event.event_type => event.payload.as_json) - end + it 'delivers a notification via background job' do + expect(RiscDeliveryJob).to receive(:perform_later) deliver end end - context 'when the EventBridge is enabled' do - let(:risc_notifications_eventbridge_enabled) { true } - - it 'posts to the EventBridge' do - expect(eventbridge_client).to receive(:put_events).and_wrap_original do |impl, entries:| - expect(entries.size).to eq(1) - entry = entries.first - - expect(entry[:time]).to eq(now) - expect(entry[:source]).to eq(sp_with_push_url.issuer) - expect(entry[:event_bus_name]).to eq('dev-risc-notifications') - expect(entry[:detail_type]).to eq('notification') - - detail_json = JSON.parse(entry[:detail], symbolize_names: true) + it 'makes an HTTP post to service providers with a push_notification_url' do + stub_request(:post, sp_with_push_url.push_notification_url). + with do |request| + expect(request.headers['Content-Type']).to eq('application/secevent+jwt') + expect(request.headers['Accept']).to eq('application/json') payload, headers = JWT.decode( - detail_json[:jwt], + request.body, AppArtifacts.store.oidc_public_key, true, algorithm: 'RS256', @@ -109,28 +78,9 @@ expect(payload['exp']).to eq((now + 12.hours).to_i) expect(payload['aud']).to eq(sp_with_push_url.push_notification_url) expect(payload['events']).to eq(event.event_type => event.payload.as_json) - - impl.call(entries: entries) - end - - deliver - end - - context 'with an error from eventbridge' do - before do - eventbridge_client.stub_responses( - :put_events, - failed_entry_count: 1, - entries: [{ error_code: 'MalformedDetail', error_message: 'Detail is malformed' }], - ) end - it 'logs a warning' do - expect(Rails.logger).to receive(:warn) - - deliver - end - end + deliver end context 'with an event that sends agency-specific iss_sub' do @@ -138,50 +88,20 @@ let(:agency_uuid) { AgencyIdentityLinker.new(sp_with_push_url_identity).link_identity.uuid } - context 'when the EventBridge is disabled' do - let(:risc_notifications_eventbridge_enabled) { false } - - it 'sends the agency-specific uuid' do - stub_request(:post, sp_with_push_url.push_notification_url). - with do |request| - payload, _headers = JWT.decode( - request.body, - AppArtifacts.store.oidc_public_key, - true, - algorithm: 'RS256', - ) - - expect(payload['events'][event.event_type]['subject']['sub']).to eq(agency_uuid) - end - - deliver - end - end - - context 'when the EventBridge is enabled' do - let(:risc_notifications_eventbridge_enabled) { true } - - it 'sends the agency-specific uuid' do - expect(eventbridge_client).to receive(:put_events).and_wrap_original do |impl, entries:| - expect(entries.size).to eq(1) - entry = entries.first - - detail_json = JSON.parse(entry[:detail], symbolize_names: true) - - payload, headers = JWT.decode( - detail_json[:jwt], + it 'sends the agency-specific uuid' do + stub_request(:post, sp_with_push_url.push_notification_url). + with do |request| + payload, _headers = JWT.decode( + request.body, AppArtifacts.store.oidc_public_key, true, algorithm: 'RS256', ) expect(payload['events'][event.event_type]['subject']['sub']).to eq(agency_uuid) - - impl.call(entries: entries) end - deliver - end + deliver end end @@ -238,24 +158,10 @@ RevokeServiceProviderConsent.new(identity).call end - context 'when the EventBridge is disabled' do - let(:risc_notifications_eventbridge_enabled) { false } - - it 'does not notify that SP' do - deliver - - expect(WebMock).not_to have_requested(:get, sp_with_push_url.push_notification_url) - end - end - - context 'when the EventBridge is enabled' do - let(:risc_notifications_eventbridge_enabled) { true } - - it 'does not notify that SP' do - expect(eventbridge_client).to_not receive(:put_events) + it 'does not notify that SP' do + deliver - deliver - end + expect(WebMock).not_to have_requested(:get, sp_with_push_url.push_notification_url) end end end