Skip to content

Commit

Permalink
Initial attempt to block request base on user ID
Browse files Browse the repository at this point in the history
  • Loading branch information
GustavoCaso committed Feb 16, 2023
1 parent 3fa663c commit 57d2894
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 20 deletions.
18 changes: 13 additions & 5 deletions lib/datadog/appsec/contrib/rack/gateway/watcher.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# typed: ignore

require_relative '../../../ext'
require_relative '../../../instrumentation/gateway'
require_relative '../../../reactive/operation'
require_relative '../reactive/request'
require_relative '../reactive/request_body'
require_relative '../reactive/response'
require_relative '../reactive/set_user'
require_relative '../../../event'

module Datadog
Expand Down Expand Up @@ -56,7 +58,13 @@ def watch_request(gateway = Instrumentation.gateway)

next [nil, [[:block, event]]] if block

ret, res = stack.call(request)
ret, res = nil

_, catched_request = catch(Datadog::AppSec::Ext::REQUEST_INTERRUPT) do
ret, res = stack.call(request)
end

next [nil, catched_request] if catched_request

if event
res ||= []
Expand Down Expand Up @@ -154,7 +162,7 @@ def watch_request_body(gateway = Instrumentation.gateway)
end

def watch_user_id(gateway = Instrumentation.gateway)
gateway.watch('identity.set_user', :appsec) do |stack, user|
gateway.watch('identity.set_user', :appsec) do |stack, user_id|
block = false
event = nil
waf_context = Datadog::AppSec::Processor.current_context
Expand All @@ -170,7 +178,7 @@ def watch_user_id(gateway = Instrumentation.gateway)
waf_result: result,
trace: trace,
span: span,
user: user,
user_id: user_id,
actions: result.actions
}

Expand All @@ -180,12 +188,12 @@ def watch_user_id(gateway = Instrumentation.gateway)
end
end

_result, block = Rack::Reactive::SetUser.publish(op, user)
_result, block = Rack::Reactive::SetUser.publish(op, user_id)
end

next [nil, [[:block, event]]] if block

ret, res = stack.call(user)
ret, res = stack.call(user_id)

if event
res ||= []
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/appsec/contrib/rack/reactive/set_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ module SetUser
].freeze
private_constant :ADDRESSES

def self.publish(op, user)
def self.publish(op, user_id)
catch(:block) do
op.publish('usr.id', user.id)
op.publish('usr.id', user_id)

nil
end
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/appsec/contrib/rack/request_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'json'

require_relative '../../ext'
require_relative '../../instrumentation/gateway'
require_relative '../../processor'
require_relative '../../response'
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/appsec/contrib/sinatra/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module Sinatra
module Ext
APP = 'sinatra'.freeze
ENV_ENABLED = 'DD_TRACE_SINATRA_ENABLED'.freeze

ROUTE_INTERRUPT = :datadog_appsec_contrib_sinatra_route_interrupt
end
end
Expand Down
10 changes: 10 additions & 0 deletions lib/datadog/appsec/ext.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# typed: false
# frozen_string_literal: true

module Datadog
module AppSec
module Ext
REQUEST_INTERRUPT = :datadog_appsec_request_interrupt
end
end
end
9 changes: 9 additions & 0 deletions lib/datadog/kit/identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module Identity
#
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/AbcSize
def self.set_user(trace, id:, email: nil, name: nil, session_id: nil, role: nil, scope: nil, **others)
raise ArgumentError, 'missing required key: :id' if id.nil?

Expand Down Expand Up @@ -56,9 +57,17 @@ 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::AppSec.settings.enabled
_, appsec_response = AppSec::Instrumentation.gateway.push('identity.set_user', id)
if appsec_response && appsec_response.any? { |action, _event| action == :block }
throw(AppSec::Ext::REQUEST_INTERRUPT, [nil, appsec_response])
end
end
end
# rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/AbcSize
end
end
end
32 changes: 32 additions & 0 deletions spec/datadog/appsec/contrib/rack/integration_test_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@

require 'datadog/tracing'
require 'datadog/appsec'
require 'datadog/kit'

RSpec.describe 'Rack integration tests' do
include Rack::Test::Methods

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
Expand Down Expand Up @@ -133,6 +135,7 @@
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
Expand Down Expand Up @@ -293,6 +296,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

Expand Down Expand Up @@ -382,6 +394,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
Expand Down
26 changes: 13 additions & 13 deletions spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

RSpec.describe Datadog::AppSec::Contrib::Rack::Reactive::SetUser do
let(:operation) { Datadog::AppSec::Reactive::Operation.new('test') }
let(:user) { double(:user, id: 1) }
let(:user_id) { '1' }

describe '.publish' do
it 'propagates request body attributes to the operation' do
expect(operation).to receive(:publish).with('usr.id', 1)
expect(operation).to receive(:publish).with('usr.id', '1')

described_class.publish(operation, user)
described_class.publish(operation, user_id)
end
end

Expand All @@ -33,15 +33,15 @@
it 'does call the waf context with the right arguments' do
expect(operation).to receive(:subscribe).and_call_original

expected_waf_arguments = { 'usr.id' => 1 }
expected_waf_arguments = { 'usr.id' => '1' }

waf_result = double(:waf_result, status: :ok, timeout: false)
expect(waf_context).to receive(:run).with(
expected_waf_arguments,
Datadog::AppSec.settings.waf_timeout
).and_return(waf_result)
described_class.subscribe(operation, waf_context)
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -56,7 +56,7 @@
expect(result).to eq(waf_result)
expect(block).to eq(false)
end
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end

Expand All @@ -69,7 +69,7 @@
expect(result).to eq(waf_result)
expect(block).to eq(true)
end
publish_result, publish_block = described_class.publish(operation, user)
publish_result, publish_block = described_class.publish(operation, user_id)
expect(publish_result).to eq(waf_result)
expect(publish_block).to eq(true)
end
Expand All @@ -82,7 +82,7 @@
waf_result = double(:waf_result, status: :ok, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -94,7 +94,7 @@
waf_result = double(:waf_result, status: :invalid_call, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -106,7 +106,7 @@
waf_result = double(:waf_result, status: :invalid_rule, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -118,7 +118,7 @@
waf_result = double(:waf_result, status: :invalid_flow, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -130,7 +130,7 @@
waf_result = double(:waf_result, status: :no_rule, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand All @@ -142,7 +142,7 @@
waf_result = double(:waf_result, status: :foo, timeout: false)
expect(waf_context).to receive(:run).and_return(waf_result)
expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control
result = described_class.publish(operation, user)
result = described_class.publish(operation, user_id)
expect(result).to be_nil
end
end
Expand Down
28 changes: 28 additions & 0 deletions spec/datadog/appsec/contrib/rails/integration_test_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -89,6 +90,7 @@
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
Expand Down Expand Up @@ -118,6 +120,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
Expand Down Expand Up @@ -244,6 +251,7 @@ def success
{
'/success' => 'test#success',
[:post, '/success'] => 'test#success',
'/set_user' => 'test#set_user',
}
end

Expand Down Expand Up @@ -347,6 +355,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
Expand Down
Loading

0 comments on commit 57d2894

Please sign in to comment.