From 176c642ca73679cabc5fa1a113bc9b600aa04dcd Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 28 Nov 2023 11:31:49 +0100 Subject: [PATCH] Call Devise RegistrationsController block When a Devise user calls create with a block, that block was swallowed by the block we use to perform our resource information extraction for registration events. Ensure the original block is called. We call it after our analysis in the event we should block for some reason, thus protecting the additional user code in their block. JIRA: - [SCRS-704](https://datadoghq.atlassian.net/browse/SCRS-704) - [APPSEC-12115](https://datadoghq.atlassian.net/browse/APPSEC-12115) - [APPSEC-12770](https://datadoghq.atlassian.net/browse/APPSEC-12770) --- .../patcher/registration_controller_patch.rb | 2 + .../registration_controller_patch_spec.rb | 125 ++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb b/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb index 3433d9a4c4d..1b8fae31266 100644 --- a/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb +++ b/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb @@ -42,6 +42,8 @@ def create **event_information.to_h ) end + + yield resource if block_given? end end end diff --git a/spec/datadog/appsec/contrib/devise/patcher/registration_controller_patch_spec.rb b/spec/datadog/appsec/contrib/devise/patcher/registration_controller_patch_spec.rb index 38ca8a02145..daeb24569cf 100644 --- a/spec/datadog/appsec/contrib/devise/patcher/registration_controller_patch_spec.rb +++ b/spec/datadog/appsec/contrib/devise/patcher/registration_controller_patch_spec.rb @@ -66,6 +66,17 @@ def try(value) expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) expect(controller.create).to eq(true) end + + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + it 'do not tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end end context 'Automated user tracking is disabled' do @@ -77,6 +88,17 @@ def try(value) expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) expect(controller.create).to eq(true) end + + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + it 'do not tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end end context 'AppSec scope is nil ' do @@ -89,6 +111,17 @@ def try(value) expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) expect(controller.create).to eq(true) end + + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + it 'do not tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end end context 'with persisted resource' do @@ -99,6 +132,41 @@ def try(value) context 'with resource ID' do let(:resource) { persited_resource } + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + context 'safe mode' do + let(:mode) { 'safe' } + + it 'tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to receive(:track_signup).with( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: resource.id, + **{} + ) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end + + context 'extended mode' do + let(:mode) { 'extended' } + + it 'tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to receive(:track_signup).with( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: resource.id, + **{ email: 'hello@gmail.com', username: 'John' } + ) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end + end + context 'safe mode' do let(:mode) { 'safe' } @@ -131,6 +199,41 @@ def try(value) context 'without resource ID' do let(:resource) { mock_resource.new(nil, 'hello@gmail.com', 'John', true) } + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + context 'safe mode' do + let(:mode) { 'safe' } + + it 'tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to receive(:track_signup).with( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: nil, + **{} + ) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end + + context 'extended mode' do + let(:mode) { 'extended' } + + it 'tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to receive(:track_signup).with( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: nil, + **{ email: 'hello@gmail.com', username: 'John' } + ) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end + end + context 'safe mode' do let(:mode) { 'safe' } @@ -174,6 +277,17 @@ def try(value) expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) expect(controller.create).to eq(true) end + + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + it 'do not tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end end context 'extended mode' do @@ -183,6 +297,17 @@ def try(value) expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) expect(controller.create).to eq(true) end + + context 'and a block is given' do + let(:canary) { proc { |resource| } } + let(:block) { proc { |resource| canary.call(resource) } } + + it 'do not tracks event' do + expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) + expect(canary).to receive(:call).with(resource) + expect(controller.create(&block)).to eq(true) + end + end end end end