diff --git a/Rakefile b/Rakefile index 67cd166a4b6..8d48e577b36 100644 --- a/Rakefile +++ b/Rakefile @@ -194,7 +194,7 @@ namespace :spec do end namespace :appsec do - task all: [:main, :rack, :rails, :sinatra] + task all: [:main, :rack, :rails, :sinatra, :devise] # Datadog AppSec main specs RSpec::Core::RakeTask.new(:main) do |t, args| @@ -209,6 +209,7 @@ namespace :spec do :rack, :sinatra, :rails, + :devise, ].each do |contrib| RSpec::Core::RakeTask.new(contrib) do |t, args| t.pattern = "spec/datadog/appsec/contrib/#{contrib}/**/*_spec.rb" @@ -399,6 +400,7 @@ task :ci do # AppSec contrib specs declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:rack' declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:sinatra' + declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:devise' # AppSec Rails specs declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:appsec:rails' diff --git a/lib/datadog/appsec/contrib/devise/event.rb b/lib/datadog/appsec/contrib/devise/event.rb new file mode 100644 index 00000000000..66223ff703c --- /dev/null +++ b/lib/datadog/appsec/contrib/devise/event.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module Devise + # Class to extract event information from the resource + class Event + UUID_REGEX = /^\h{8}-\h{4}-\h{4}-\h{4}-\h{12}$/.freeze + + SAFE_MODE = 'safe' + EXTENDED_MODE = 'extended' + + attr_reader :user_id + + def initialize(resource, mode) + @resource = resource + @mode = mode + @user_id = nil + @email = nil + @username = nil + + extract if @resource + end + + def to_h + return @event if defined?(@event) + + @event = {} + @event[:email] = @email if @email + @event[:username] = @username if @username + @event + end + + private + + def extract + @user_id = @resource.id + + case @mode + when EXTENDED_MODE + @email = @resource.email + @username = @resource.username + when SAFE_MODE + @user_id = nil unless @user_id && @user_id.to_s =~ UUID_REGEX + else + Datadog.logger.warn( + "Invalid automated user evenst mode: `#{@mode}`. "\ + 'Supported modes are: `safe` and `extended`.' + ) + end + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/devise/patcher.rb b/lib/datadog/appsec/contrib/devise/patcher.rb index 82b21d7afd4..4fddf55e0df 100644 --- a/lib/datadog/appsec/contrib/devise/patcher.rb +++ b/lib/datadog/appsec/contrib/devise/patcher.rb @@ -12,9 +12,6 @@ module Devise module Patcher include Datadog::AppSec::Contrib::Patcher - DISABLED_MODE = 'disabled' - EXTENDED_MODE = 'extended' - module_function def patched? diff --git a/lib/datadog/appsec/contrib/devise/patcher/authenticatable_patch.rb b/lib/datadog/appsec/contrib/devise/patcher/authenticatable_patch.rb index eaba77d0e37..2def7b707af 100644 --- a/lib/datadog/appsec/contrib/devise/patcher/authenticatable_patch.rb +++ b/lib/datadog/appsec/contrib/devise/patcher/authenticatable_patch.rb @@ -2,6 +2,7 @@ require_relative '../tracking' require_relative '../resource' +require_relative '../event' module Datadog module AppSec @@ -10,14 +11,16 @@ module Devise module Patcher # Hook in devise validate method module AuthenticatablePatch - # rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity + # rubocop:disable Metrics/MethodLength def validate(resource, &block) result = super return result unless AppSec.enabled? - return unless Datadog.configuration.appsec.track_user_events.enabled + track_user_events_configuration = Datadog.configuration.appsec.track_user_events - automated_track_user_events_mode = Datadog.configuration.appsec.track_user_events.mode + return result unless track_user_events_configuration.enabled + + automated_track_user_events_mode = track_user_events_configuration.mode appsec_scope = Datadog::AppSec.active_scope @@ -25,64 +28,46 @@ def validate(resource, &block) devise_resource = resource ? Resource.new(resource) : nil - event_information = {} - user_id = nil - - if automated_track_user_events_mode == Patcher::EXTENDED_MODE && devise_resource - resource_email = devise_resource.email - resource_username = devise_resource.username - - event_information[:email] = resource_email if resource_email - event_information[:username] = resource_username if resource_username - end - - user_id = devise_resource.id if devise_resource && devise_resource.id + event_information = Event.new(devise_resource, automated_track_user_events_mode) if result - if user_id - Tracking.track_login_success( - appsec_scope.trace, - appsec_scope.service_entry_span, - user_id: user_id, - **event_information - ) + if event_information.user_id Datadog.logger.debug { 'User Login Event success' } else - Tracking.track_login_success( - appsec_scope.trace, - appsec_scope.service_entry_span, - user_id: nil, - **event_information - ) Datadog.logger.debug { 'User Login Event success, but can\'t extract user ID. Tracking empty event' } end - return result - end - - if devise_resource - Tracking.track_login_failure( + Tracking.track_login_success( appsec_scope.trace, appsec_scope.service_entry_span, - user_id: user_id, - user_exists: true, - **event_information + user_id: event_information.user_id, + **event_information.to_h ) + + return result + end + + user_exists = nil + + if resource + user_exists = true Datadog.logger.debug { 'User Login Event failure users exists' } else - Tracking.track_login_failure( - appsec_scope.trace, - appsec_scope.service_entry_span, - user_id: nil, - user_exists: false, - **event_information - ) + user_exists = false Datadog.logger.debug { 'User Login Event failure user do not exists' } end + Tracking.track_login_failure( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: event_information.user_id, + user_exists: user_exists, + **event_information.to_h + ) + result end - # rubocop:enable Metrics/MethodLength, Metrics/PerceivedComplexity + # rubocop:enable Metrics/MethodLength end end end 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 80aaf5f1ead..3433d9a4c4d 100644 --- a/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb +++ b/lib/datadog/appsec/contrib/devise/patcher/registration_controller_patch.rb @@ -2,6 +2,7 @@ require_relative '../tracking' require_relative '../resource' +require_relative '../event' module Datadog module AppSec @@ -13,9 +14,11 @@ module RegistrationControllerPatch def create return super unless AppSec.enabled? - return unless Datadog.configuration.appsec.track_user_events.enabled + track_user_events_configuration = Datadog.configuration.appsec.track_user_events - automated_track_user_events_mode = Datadog.configuration.appsec.track_user_events.mode + return super unless track_user_events_configuration.enabled + + automated_track_user_events_mode = track_user_events_configuration.mode appsec_scope = Datadog::AppSec.active_scope return super unless appsec_scope @@ -24,34 +27,20 @@ def create if resource.persisted? devise_resource = Resource.new(resource) - event_information = {} - user_id = devise_resource.id if devise_resource && devise_resource.id - - if automated_track_user_events_mode == Patcher::EXTENDED_MODE - resource_email = devise_resource.email - resource_username = devise_resource.username + event_information = Event.new(devise_resource, automated_track_user_events_mode) - event_information[:email] = resource_email if resource_email - event_information[:username] = resource_username if resource_username - end - - if user_id - Tracking.track_signup( - appsec_scope.trace, - appsec_scope.service_entry_span, - user_id: user_id, - **event_information - ) + if event_information.user_id Datadog.logger.debug { 'User Signup Event' } else - Tracking.track_signup( - appsec_scope.trace, - appsec_scope.service_entry_span, - user_id: nil, - **event_information - ) Datadog.logger.warn { 'User Signup Event, but can\'t extract user ID. Tracking empty event' } end + + Tracking.track_signup( + appsec_scope.trace, + appsec_scope.service_entry_span, + user_id: event_information.user_id, + **event_information.to_h + ) end end end diff --git a/lib/datadog/kit/appsec/events.rb b/lib/datadog/kit/appsec/events.rb index b93e1f4dbbf..1f18234d0ac 100644 --- a/lib/datadog/kit/appsec/events.rb +++ b/lib/datadog/kit/appsec/events.rb @@ -110,7 +110,7 @@ def track(event, trace = nil, span = nil, **others) check_trace_span_integrity(trace, span) span.set_tag("appsec.events.#{event}.track", 'true') - span.set_tag("_dd.appsec.appsec.events.#{event}.sdk", 'true') + span.set_tag("_dd.appsec.events.#{event}.sdk", 'true') others.each do |k, v| raise ArgumentError, 'key cannot be :track' if k.to_sym == :track @@ -122,7 +122,7 @@ def track(event, trace = nil, span = nil, **others) else set_trace_and_span_context('track', trace, span) do |active_trace, active_span| active_span.set_tag("appsec.events.#{event}.track", 'true') - active_span.set_tag("_dd.appsec.appsec.events.#{event}.sdk", 'true') + active_span.set_tag("_dd.appsec.events.#{event}.sdk", 'true') others.each do |k, v| raise ArgumentError, 'key cannot be :track' if k.to_sym == :track diff --git a/sig/datadog/appsec/contrib/devise/event.rbs b/sig/datadog/appsec/contrib/devise/event.rbs new file mode 100644 index 00000000000..388cb42c098 --- /dev/null +++ b/sig/datadog/appsec/contrib/devise/event.rbs @@ -0,0 +1,26 @@ +module Datadog + module AppSec + module Contrib + module Devise + class Event + UUID_REGEX: ::Regexp + + SAFE_MODE: String + + EXTENDED_MODE: String + + attr_reader user_id: untyped + + def initialize: (Devise::Resource? resource, String mode) -> void + + + def to_h: () -> Hash[Symbol, untyped] + + private + + def extract: () -> void + end + end + end + end +end diff --git a/spec/datadog/appsec/contrib/devise/event_spec.rb b/spec/datadog/appsec/contrib/devise/event_spec.rb new file mode 100644 index 00000000000..e7b7ae0bb4e --- /dev/null +++ b/spec/datadog/appsec/contrib/devise/event_spec.rb @@ -0,0 +1,92 @@ +require 'datadog/appsec/spec_helper' +require 'datadog/appsec/contrib/devise/resource' +require 'datadog/appsec/contrib/devise/event' + +RSpec.describe Datadog::AppSec::Contrib::Devise::Event do + let(:event) { described_class.new(resource, mode) } + let(:resource) { Datadog::AppSec::Contrib::Devise::Resource.new(object) } + + let(:object_class) do + Class.new do + attr_reader :id, :uuid, :email, :username + + def initialize(id: nil, uuid: nil, email: nil, username: nil) + @id = id + @uuid = uuid + @email = email + @username = username + end + end + end + + context 'without resource' do + let(:resource) { nil } + let(:mode) { 'safe' } + + it do + expect(event.to_h).to eq({}) + end + end + + context 'safe mode' do + let(:mode) { 'safe' } + + context 'with ID but not UUID' do + let(:object) { object_class.new(id: 1234) } + + it do + expect(event.user_id).to be_nil + end + end + + context 'with ID as UUID' do + let(:uuid) { '123e4567-e89b-12d3-a456-426655440000' } + let(:object) { object_class.new(uuid: uuid) } + + it do + expect(event.user_id).to eq(uuid) + end + end + end + + context 'extended mode' do + let(:mode) { 'extended' } + + context 'ID' do + context 'with ID but not UUID' do + let(:object) { object_class.new(id: 1234) } + + it do + expect(event.user_id).to eq(1234) + end + end + + context 'with ID as UUID' do + let(:uuid) { '123e4567-e89b-12d3-a456-426655440000' } + let(:object) { object_class.new(uuid: uuid) } + + it do + expect(event.user_id).to eq(uuid) + end + end + end + + context 'Email and username' do + let(:object) { object_class.new(id: 1234, email: 'foo@test.com', username: 'John') } + + it do + expect(event.to_h).to eq({ email: 'foo@test.com', username: 'John' }) + end + end + end + + context 'invalid mode' do + let(:object) { object_class.new(id: 1234) } + let(:mode) { 'invalid' } + + it do + expect(Datadog.logger).to receive(:warn) + expect(event.to_h).to eq({}) + end + end +end diff --git a/spec/datadog/appsec/contrib/devise/patcher/authenticatable_patch_spec.rb b/spec/datadog/appsec/contrib/devise/patcher/authenticatable_patch_spec.rb index 1da14c4f78c..5c7f346dade 100644 --- a/spec/datadog/appsec/contrib/devise/patcher/authenticatable_patch_spec.rb +++ b/spec/datadog/appsec/contrib/devise/patcher/authenticatable_patch_spec.rb @@ -1,5 +1,6 @@ require 'datadog/appsec/spec_helper' +require 'securerandom' require 'datadog/appsec/contrib/devise/patcher' require 'datadog/appsec/contrib/devise/patcher/authenticatable_patch' @@ -31,21 +32,24 @@ def initialize(id, email, username) end let(:nil_resource) { nil } - let(:resource) { mock_resource.new(1, 'hello@gmail.com', 'John') } + let(:resource) { mock_resource.new(SecureRandom.uuid, 'hello@gmail.com', 'John') } let(:mode) { 'safe' } - let(:automated_track_user_events) { double(automated_track_user_events: mode) } + let(:automated_track_user_events) { double(enabled: track_user_events_enabled, mode: mode) } let(:success_login) { mock_klass.new(true) } let(:failed_login) { mock_klass.new(false) } before do expect(Datadog::AppSec).to receive(:enabled?).and_return(appsec_enabled) - expect(Datadog::AppSec).to receive(:settings).and_return(automated_track_user_events) if appsec_enabled + if appsec_enabled + expect(Datadog.configuration.appsec).to receive(:track_user_events).and_return(automated_track_user_events) - expect(Datadog::AppSec).to receive(:active_scope).and_return(appsec_scope) if appsec_enabled && mode != 'disable' + expect(Datadog::AppSec).to receive(:active_scope).and_return(appsec_scope) if track_user_events_enabled + end end context 'AppSec disabled' do let(:appsec_enabled) { false } + let(:track_user_events_enabled) { false } it 'do not tracks event' do expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_login_success) @@ -55,7 +59,7 @@ def initialize(id, email, username) context 'Automated user tracking is disabled' do let(:appsec_enabled) { true } - let(:mode) { 'disable' } + let(:track_user_events_enabled) { false } it 'do not tracks event' do expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_login_success) @@ -65,6 +69,7 @@ def initialize(id, email, username) context 'AppSec scope is nil ' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:mode) { 'safe' } let(:appsec_scope) { nil } @@ -76,6 +81,7 @@ def initialize(id, email, username) context 'successful login' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:appsec_scope) { instance_double(Datadog::AppSec::Scope, trace: double, service_entry_span: double) } context 'with resource ID' do @@ -143,6 +149,7 @@ def initialize(id, email, username) context 'unsuccessful login' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:appsec_scope) { instance_double(Datadog::AppSec::Scope, trace: double, service_entry_span: double) } context 'with resource' do 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 c390cff994e..38ca8a02145 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 @@ -1,5 +1,6 @@ require 'datadog/appsec/spec_helper' +require 'securerandom' require 'datadog/appsec/contrib/devise/patcher' require 'datadog/appsec/contrib/devise/patcher/registration_controller_patch' @@ -42,21 +43,24 @@ def try(value) end let(:non_persisted_resource) { mock_resource.new(nil, nil, nil, false) } - let(:persited_resource) { mock_resource.new(1, 'hello@gmail.com', 'John', true) } - let(:automated_track_user_events) { double(automated_track_user_events: mode) } + let(:persited_resource) { mock_resource.new(SecureRandom.uuid, 'hello@gmail.com', 'John', true) } + let(:automated_track_user_events) { double(enabled: track_user_events_enabled, mode: mode) } let(:controller) { mock_controller.new(true, resource) } let(:resource) { non_persisted_resource } before do expect(Datadog::AppSec).to receive(:enabled?).and_return(appsec_enabled) - expect(Datadog::AppSec).to receive(:settings).and_return(automated_track_user_events) if appsec_enabled + if appsec_enabled + expect(Datadog.configuration.appsec).to receive(:track_user_events).and_return(automated_track_user_events) - expect(Datadog::AppSec).to receive(:active_scope).and_return(appsec_scope) if appsec_enabled && mode != 'disable' + expect(Datadog::AppSec).to receive(:active_scope).and_return(appsec_scope) if track_user_events_enabled + end end context 'AppSec disabled' do let(:appsec_enabled) { false } + let(:track_user_events_enabled) { false } it 'do not tracks event' do expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) @@ -66,7 +70,8 @@ def try(value) context 'Automated user tracking is disabled' do let(:appsec_enabled) { true } - let(:mode) { 'disable' } + let(:track_user_events_enabled) { false } + let(:mode) { 'safe' } it 'do not tracks event' do expect(Datadog::AppSec::Contrib::Devise::Tracking).to_not receive(:track_signup) @@ -76,6 +81,7 @@ def try(value) context 'AppSec scope is nil ' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:mode) { 'safe' } let(:appsec_scope) { nil } @@ -87,6 +93,7 @@ def try(value) context 'with persisted resource' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:appsec_scope) { instance_double(Datadog::AppSec::Scope, trace: double, service_entry_span: double) } context 'with resource ID' do @@ -156,6 +163,7 @@ def try(value) context 'with non persisted resource' do let(:appsec_enabled) { true } + let(:track_user_events_enabled) { true } let(:appsec_scope) { instance_double(Datadog::AppSec::Scope, trace: double, service_entry_span: double) } let(:resource) { non_persisted_resource } diff --git a/spec/datadog/appsec/contrib/devise/tracking_spec.rb b/spec/datadog/appsec/contrib/devise/tracking_spec.rb index 865290720a1..8df801f2466 100644 --- a/spec/datadog/appsec/contrib/devise/tracking_spec.rb +++ b/spec/datadog/appsec/contrib/devise/tracking_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Datadog::AppSec::Contrib::Devise::Tracking do let(:trace_op) { Datadog::Tracing::TraceOperation.new } - let(:auto_mode) { Datadog.configuration.appsec.automated_track_user_events.to_s } + let(:auto_mode) { Datadog.configuration.appsec.track_user_events.mode.to_s } describe '#track_login_success' do it 'sets event tracking key on trace' do diff --git a/spec/datadog/kit/appsec/events_spec.rb b/spec/datadog/kit/appsec/events_spec.rb index 80e7b6b3c65..4d34b30a169 100644 --- a/spec/datadog/kit/appsec/events_spec.rb +++ b/spec/datadog/kit/appsec/events_spec.rb @@ -52,6 +52,7 @@ trace_op.measure('root') do |span, _trace| described_class.track_login_success(trace_op, user: { id: '42' }) expect(span.tags).to include('appsec.events.users.login.success.track' => 'true') + expect(span.tags).to include('_dd.appsec.events.users.login.success.sdk' => 'true') end end @@ -101,6 +102,7 @@ trace_op.measure('root') do |span, _trace| described_class.track_login_failure(trace_op, user_id: '42', user_exists: true) expect(span.tags).to include('appsec.events.users.login.failure.track' => 'true') + expect(span.tags).to include('_dd.appsec.events.users.login.failure.sdk' => 'true') end end @@ -147,6 +149,7 @@ trace_op.measure('root') do |span, _trace| described_class.track_signup(trace_op, user: { id: '42' }) expect(span.tags).to include('appsec.events.users.signup.track' => 'true') + expect(span.tags).to include('_dd.appsec.events.users.signup.sdk' => 'true') end end @@ -196,6 +199,7 @@ trace_op.measure('root') do |span, _trace| described_class.track('foo', trace_op) expect(span.tags).to include('appsec.events.foo.track' => 'true') + expect(span.tags).to include('_dd.appsec.events.foo.sdk' => 'true') end end