Merge pull request #2642 from DataDog/appsec-block-user-when-ID-matches

[APPSEC-8112] Appsec block request when user ID matches rules data

Author: GustavoCaso

lib/datadog/appsec/contrib/rack/request_middleware.rb

@@ -2,6 +2,7 @@
 
 require 'json'
 
+require_relative '../../ext'
 require_relative '../../instrumentation/gateway'
 require_relative '../../processor'
 require_relative '../../response'
@@ -38,8 +39,10 @@ def call(env)
 
             add_appsec_tags(processor, active_trace, active_span, env)
 
-            request_return, request_response = Instrumentation.gateway.push('rack.request', request) do
-              @app.call(env)
+            request_return, request_response = catch(::Datadog::AppSec::Ext::INTERRUPT) do
+              Instrumentation.gateway.push('rack.request', request) do
+                @app.call(env)
+              end
             end
 
             if request_response && request_response.any? { |action, _event| action == :block }

lib/datadog/appsec/ext.rb

@@ -0,0 +1,10 @@
+# typed: false
+# frozen_string_literal: true
+
+module Datadog
+  module AppSec
+    module Ext
+      INTERRUPT = :datadog_appsec_interrupt
+    end
+  end
+end

lib/datadog/appsec/monitor/gateway/watcher.rb

@@ -1,6 +1,7 @@
 # typed: false
 # frozen_string_literal: true
 
+require_relative '../../ext'
 require_relative '../../instrumentation/gateway'
 require_relative '../../reactive/operation'
 require_relative '../reactive/set_user'
@@ -48,7 +49,7 @@ def watch_user_id(gateway = Instrumentation.gateway)
                   _result, block = Monitor::Reactive::SetUser.publish(op, user)
                 end
 
-                next [nil, [[:block, event]]] if block
+                throw(Datadog::AppSec::Ext::INTERRUPT, [nil, [:block, event]]) if block
 
                 ret, res = stack.call(user)
 

lib/datadog/kit/identity.rb

@@ -56,6 +56,11 @@ def self.set_user(trace, id:, email: nil, name: nil, session_id: nil, role: nil,
         others.each do |k, v|
           trace.set_tag("usr.#{k}", v) unless v.nil?
         end
+
+        if Datadog.configuration.appsec.enabled
+          user = OpenStruct.new(id: id)
+          ::Datadog::AppSec::Instrumentation.gateway.push('identity.set_user', user)
+        end
       end
       # rubocop:enable Metrics/PerceivedComplexity
       # rubocop:enable Metrics/CyclomaticComplexity

sig/datadog/appsec/ext.rbs

@@ -0,0 +1,7 @@
+module Datadog
+  module AppSec
+    module Ext
+      INTERRUPT: Symbol
+    end
+  end
+end

spec/datadog/appsec/contrib/rack/integration_test_spec.rb

@@ -22,6 +22,7 @@
   let(:appsec_enabled) { true }
   let(:tracing_enabled) { true }
   let(:appsec_ip_denylist) { nil }
+  let(:appsec_user_id_denylist) { nil }
   let(:appsec_ruleset) { :recommended }
 
   let(:crs_942_100) do
@@ -133,11 +134,15 @@
       c.appsec.enabled = appsec_enabled
       c.appsec.instrument :rack
       c.appsec.ip_denylist = appsec_ip_denylist
+      c.appsec.user_id_denylist = appsec_user_id_denylist
       c.appsec.ruleset = appsec_ruleset
     end
   end
 
-  after { Datadog.registry[:rack].reset_configuration! }
+  after do
+    Datadog::AppSec.settings.send(:reset!)
+    Datadog.registry[:rack].reset_configuration!
+  end
 
   context 'for an application' do
     # TODO: also test without Tracing: it should run without trace transport
@@ -293,6 +298,15 @@
               end
             )
           end
+
+          map '/set_user' do
+            run(
+              proc do |_env|
+                Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id')
+                [200, { 'Content-Type' => 'text/html' }, ['OK']]
+              end
+            )
+          end
         end
       end
 
@@ -382,6 +396,26 @@
             it_behaves_like 'a trace with AppSec events'
           end
         end
+
+        context 'with user blocking ID' do
+          let(:url) { '/set_user' }
+
+          it { is_expected.to be_ok }
+
+          it_behaves_like 'a GET 200 span'
+          it_behaves_like 'a trace with AppSec tags'
+          it_behaves_like 'a trace without AppSec events'
+
+          context 'with an event-triggering user ID' do
+            let(:appsec_user_id_denylist) { ['blocked-user-id'] }
+
+            it { is_expected.to be_forbidden }
+
+            it_behaves_like 'a GET 403 span'
+            it_behaves_like 'a trace with AppSec tags'
+            it_behaves_like 'a trace with AppSec events'
+          end
+        end
       end
 
       describe 'POST request' do

spec/datadog/appsec/contrib/rails/integration_test_spec.rb

@@ -32,6 +32,7 @@
   let(:appsec_enabled) { true }
   let(:tracing_enabled) { true }
   let(:appsec_ip_denylist) { nil }
+  let(:appsec_user_id_denylist) { nil }
   let(:appsec_ruleset) { :recommended }
 
   let(:crs_942_100) do
@@ -89,12 +90,18 @@
       c.appsec.enabled = appsec_enabled
       c.appsec.instrument :rails
       c.appsec.ip_denylist = appsec_ip_denylist
+      c.appsec.user_id_denylist = appsec_user_id_denylist
       c.appsec.ruleset = appsec_ruleset
 
       # TODO: test with c.appsec.instrument :rack
     end
   end
 
+  after do
+    Datadog::AppSec.settings.send(:reset!)
+    Datadog.registry[:rails].reset_configuration!
+  end
+
   context 'for an application' do
     include_context 'Rails test application'
 
@@ -118,6 +125,11 @@
           def success
             head :ok
           end
+
+          def set_user
+            Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id')
+            head :ok
+          end
         end
       )
     end
@@ -244,6 +256,7 @@ def success
         {
           '/success' => 'test#success',
           [:post, '/success'] => 'test#success',
+          '/set_user' => 'test#set_user',
         }
       end
 
@@ -347,6 +360,26 @@ def success
           it_behaves_like 'a trace with AppSec tags'
           it_behaves_like 'a trace with AppSec events'
         end
+
+        context 'with user blocking ID' do
+          let(:url) { '/set_user' }
+
+          it { is_expected.to be_ok }
+
+          it_behaves_like 'a GET 200 span'
+          it_behaves_like 'a trace with AppSec tags'
+          it_behaves_like 'a trace without AppSec events'
+
+          context 'with an event-triggering user ID' do
+            let(:appsec_user_id_denylist) { ['blocked-user-id'] }
+
+            it { is_expected.to be_forbidden }
+
+            it_behaves_like 'a GET 403 span'
+            it_behaves_like 'a trace with AppSec tags'
+            it_behaves_like 'a trace with AppSec events'
+          end
+        end
       end
 
       describe 'POST request' do

spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb

@@ -44,6 +44,7 @@
   let(:appsec_enabled) { true }
   let(:tracing_enabled) { true }
   let(:appsec_ip_denylist) { nil }
+  let(:appsec_user_id_denylist) { nil }
   let(:appsec_ruleset) { :recommended }
 
   let(:crs_942_100) do
@@ -101,13 +102,15 @@
       c.appsec.enabled = appsec_enabled
       c.appsec.instrument :sinatra
       c.appsec.ip_denylist = appsec_ip_denylist
+      c.appsec.user_id_denylist = appsec_user_id_denylist
       c.appsec.ruleset = appsec_ruleset
 
       # TODO: test with c.appsec.instrument :rack
     end
   end
 
   after do
+    Datadog::AppSec.settings.send(:reset!)
     Datadog.registry[:rack].reset_configuration!
     Datadog.registry[:sinatra].reset_configuration!
   end
@@ -254,6 +257,11 @@
           post '/success' do
             'ok'
           end
+
+          get '/set_user' do
+            Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id')
+            'ok'
+          end
         end
       end
 
@@ -359,6 +367,26 @@
           it_behaves_like 'a trace with AppSec tags'
           it_behaves_like 'a trace with AppSec events'
         end
+
+        context 'with user blocking ID' do
+          let(:url) { '/set_user' }
+
+          it { is_expected.to be_ok }
+
+          it_behaves_like 'a GET 200 span'
+          it_behaves_like 'a trace with AppSec tags'
+          it_behaves_like 'a trace without AppSec events'
+
+          context 'with an event-triggering user ID' do
+            let(:appsec_user_id_denylist) { ['blocked-user-id'] }
+
+            it { is_expected.to be_forbidden }
+
+            it_behaves_like 'a GET 403 span'
+            it_behaves_like 'a trace with AppSec tags'
+            it_behaves_like 'a trace with AppSec events'
+          end
+        end
       end
 
       describe 'POST request' do

spec/datadog/kit/identity_spec.rb

@@ -230,5 +230,29 @@
         expect { described_class.set_user(trace_op, id: 42, foo: 42) }.to raise_error(TypeError)
       end
     end
+
+    context 'appsec' do
+      after { Datadog.configuration.appsec.send(:reset!) }
+
+      context 'when is enabled' do
+        it 'instruments the user information to appsec' do
+          Datadog.configuration.appsec.enabled = true
+          user = OpenStruct.new(id: '42')
+          expect_any_instance_of(Datadog::AppSec::Instrumentation::Gateway).to receive(:push).with(
+            'identity.set_user',
+            user
+          )
+          described_class.set_user(trace_op, id: '42')
+        end
+      end
+
+      context 'when is disabled' do
+        it 'does not instrument the user information to appsec' do
+          Datadog.configuration.appsec.enabled = false
+          expect_any_instance_of(Datadog::AppSec::Instrumentation::Gateway).to_not receive(:push).with('identity.set_user')
+          described_class.set_user(trace_op, id: '42')
+        end
+      end
+    end
   end
 end