Support [not] relational operator for target_guids on audit events

Built a new validator that allowed target_guids to be either an array or
a hash containing the "not" key mapped to an array

[#174159766]

Co-authored-by: Mona Mohebbi <mmohebbi@pivotal.io>
Co-authored-by: Harsha Nandiwada <hnandiwada@vmware.com>

app/fetchers/event_list_fetcher.rb

@@ -18,7 +18,11 @@ def filter(message, dataset)
         end
 
         if message.requested?(:target_guids)
-          dataset = dataset.where(actee: message.target_guids)
+          dataset = if message.exclude_target_guids?
+                      dataset.exclude(actee: message.target_guids[:not])
+                    else
+                      dataset.where(actee: message.target_guids)
+                    end
         end
 
         if message.requested?(:space_guids)

app/messages/events_list_message.rb

@@ -10,14 +10,22 @@ class EventsListMessage < ListMessage
     ]
 
     validates_with NoAdditionalParamsValidator
+    validates_with TargetGuidsValidator, allow_nil: true
 
     validates :types, array: true, allow_nil: true
-    validates :target_guids, array: true, allow_nil: true
     validates :space_guids, array: true, allow_nil: true
     validates :organization_guids, array: true, allow_nil: true
 
+    def exclude_target_guids?
+      return (target_guids.is_a? Hash) && target_guids[:not].present?
+    end
+
     def self.from_params(params)
-      super(params, %w(types target_guids space_guids organization_guids))
+      if params[:target_guids].is_a? Hash
+        super(params, %w(types space_guids organization_guids), fields: %w(target_guids))
+      else
+        super(params, %w(types target_guids space_guids organization_guids))
+      end
     end
   end
 end

app/messages/validators.rb

@@ -346,4 +346,18 @@ def opinionated_iso_8601(timestamp, record, attribute)
       end
     end
   end
+
+  class TargetGuidsValidator < ActiveModel::Validator
+    def validate(record)
+      if record.target_guids.is_a? Hash
+        if record.target_guids[:not].present?
+          record.errors[:target_guids].concat ['target_guids must be an array'] unless record.target_guids[:not].is_a? Array
+        else
+          record.errors[:target_guids].concat ['target_guids has an invalid operator']
+        end
+      elsif record.target_guids.present?
+        record.errors[:target_guids].concat ['target_guids must be an array'] unless record.target_guids.is_a? Array
+      end
+    end
+  end
 end

spec/request/events_spec.rb

@@ -13,7 +13,7 @@
     let(:app_model) { VCAP::CloudController::AppModel.make(space: space) }
 
     let!(:unscoped_event) { VCAP::CloudController::Event.make(actee: 'dir/key', type: 'blob.remove_orphan', organization_guid: '') }
-    let!(:org_scoped_event) { VCAP::CloudController::Event.make(created_at: Time.now + 100, type: 'audit.organization.create', organization_guid: org.guid) }
+    let!(:org_scoped_event) { VCAP::CloudController::Event.make(created_at: Time.now + 100, type: 'audit.organization.create', actee: org.guid, organization_guid: org.guid) }
     let!(:space_scoped_event) { VCAP::CloudController::Event.make(space_guid: space.guid, organization_guid: org.guid, actee: app_model.guid, type: 'audit.app.restart') }
 
     let(:unscoped_event_json) do
@@ -148,6 +148,28 @@
       end
     end
 
+    context 'filtering out target_guids' do
+      it 'returns filtered events without the specified target guid' do
+        get "/v3/audit_events?target_guids[not]=#{app_model.guid}", nil, admin_header
+
+        expect({
+          resources: parsed_response['resources']
+        }).to match_json_response({
+          resources: [unscoped_event_json, org_scoped_event_json]
+        })
+      end
+
+      it 'works for multiple guids' do
+        get "/v3/audit_events?target_guids[not]=#{app_model.guid},#{org.guid}", nil, admin_header
+
+        expect({
+          resources: parsed_response['resources']
+        }).to match_json_response({
+          resources: [unscoped_event_json]
+        })
+      end
+    end
+
     context 'filtering by space_guid' do
       it 'returns filtered events' do
         get "/v3/audit_events?space_guids=#{space.guid}", nil, admin_header

spec/unit/fetchers/event_list_fetcher_spec.rb

@@ -47,6 +47,16 @@ module VCAP::CloudController
           end
         end
 
+        context 'filtering AGAINST target guid' do
+          let(:filters) do
+            { target_guids: { not: [app_model.guid] } }
+          end
+
+          it 'returns filtered events' do
+            expect(subject).to match_array([unscoped_event, org_scoped_event])
+          end
+        end
+
         context 'filtering by space guid' do
           let(:filters) do
             { space_guids: [space.guid] }

spec/unit/messages/events_list_message_spec.rb

@@ -54,10 +54,34 @@ module VCAP::CloudController
           expect(message.errors[:types]).to include('must be an array')
         end
 
-        it 'validates the target_guids filter' do
-          message = EventsListMessage.from_params({ target_guids: 123 })
-          expect(message).not_to be_valid
-          expect(message.errors[:target_guids]).to include('must be an array')
+        context 'target_guids' do
+          it 'does not allow non-array values' do
+            message = EventsListMessage.from_params({ target_guids: 'not an array' })
+            expect(message).not_to be_valid
+            expect(message.errors_on(:target_guids)).to contain_exactly('target_guids must be an array')
+          end
+
+          it 'is valid for an array' do
+            message = EventsListMessage.from_params({ target_guids: ['guid1', 'guid2'] })
+            expect(message).to be_valid
+          end
+
+          it 'does not allow random operators' do
+            message = EventsListMessage.from_params({ target_guids: { weyman: ['not a number'] } })
+            expect(message).not_to be_valid
+            expect(message.errors_on(:target_guids)).to contain_exactly('target_guids has an invalid operator')
+          end
+
+          it 'allows the not operator' do
+            message = EventsListMessage.from_params({ target_guids: { not: ['guid1'] } })
+            expect(message).to be_valid
+          end
+
+          it 'does not allow non-array values in the "not" field' do
+            message = EventsListMessage.from_params({ target_guids: { not: 'not an array' } })
+            expect(message).not_to be_valid
+            expect(message.errors_on(:target_guids)).to contain_exactly('target_guids must be an array')
+          end
         end
 
         it 'validates the space_guids filter' do
@@ -73,5 +97,25 @@ module VCAP::CloudController
         end
       end
     end
+
+    context 'exclude target guids' do
+      it 'returns false for an array' do
+        message = EventsListMessage.from_params({ target_guids: ['an array'] })
+        expect(message).to be_valid
+        expect(message.exclude_target_guids?).to be false
+      end
+
+      it 'returns false for a hash with the wrong key' do
+        message = EventsListMessage.from_params({ target_guids: { mona: ['an array'] } })
+        expect(message).not_to be_valid
+        expect(message.exclude_target_guids?).to be false
+      end
+
+      it 'returns true for a hash with the right key' do
+        message = EventsListMessage.from_params({ target_guids: { not: ['an array'] } })
+        expect(message).to be_valid
+        expect(message.exclude_target_guids?).to be true
+      end
+    end
   end
 end

spec/unit/messages/list_message_spec.rb

@@ -249,6 +249,13 @@ def self.from_params(params)
         end
       end
 
+      context 'validates the updated_ats filter' do
+        it 'delegates to the TimestampValidator' do
+          message = ListMessage.from_params({ 'updated_ats' => { gte: '2020-06-30T23:49:04Z' } }, [])
+          expect(message).to be_valid
+        end
+      end
+
       context 'validates the updated_ats filter' do
         it 'delegates to the TimestampValidator' do
           message = ListMessage.from_params({ 'updated_ats' => 47 }, [])

spec/unit/messages/validators_spec.rb

@@ -686,5 +686,41 @@ class Relationships < VCAP::CloudController::BaseMessage
         end
       end
     end
+
+    describe 'TargetGuidsValidator' do
+      class TargetGuidsMessage < VCAP::CloudController::BaseMessage
+        register_allowed_keys [:target_guids]
+
+        validates_with TargetGuidsValidator
+      end
+
+      it 'does not allow non-array values' do
+        message = TargetGuidsMessage.new({ target_guids: 'not an array' })
+        expect(message).not_to be_valid
+        expect(message.errors_on(:target_guids)).to contain_exactly('target_guids must be an array')
+      end
+
+      it 'is valid for an array' do
+        message = TargetGuidsMessage.new({ target_guids: ['guid1', 'guid2'] })
+        expect(message).to be_valid
+      end
+
+      it 'does not allow random operators' do
+        message = TargetGuidsMessage.new({ target_guids: { weyman: ['not a number'] } })
+        expect(message).not_to be_valid
+        expect(message.errors_on(:target_guids)).to contain_exactly('target_guids has an invalid operator')
+      end
+
+      it 'allows the not operator' do
+        message = TargetGuidsMessage.new({ target_guids: { not: ['guid1'] } })
+        expect(message).to be_valid
+      end
+
+      it 'does not allow non-array values in the "not" field' do
+        message = TargetGuidsMessage.new({ target_guids: { not: 'not an array' } })
+        expect(message).not_to be_valid
+        expect(message.errors_on(:target_guids)).to contain_exactly('target_guids must be an array')
+      end
+    end
   end
 end