[Security Solution] [Detections] Only check privileges if index param is not empty in rule definition (#89147)

* only check privileges if index param is not empty

* fix jest test i added in previous commit, fixes bug where timestamp field was printed twice in partial failure message

* adds two unit tests to check the error messages written from hasTimestampFields util function

x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts

@@ -14,11 +14,7 @@ import {
   SignalHit,
   WrappedSignalHit,
 } from '../types';
-import {
-  Logger,
-  SavedObject,
-  SavedObjectsFindResponse,
-} from '../../../../../../../../src/core/server';
+import { SavedObject, SavedObjectsFindResponse } from '../../../../../../../../src/core/server';
 import { loggingSystemMock } from '../../../../../../../../src/core/server/mocks';
 import { RuleTypeParams } from '../../types';
 import { IRuleStatusSOAttributes } from '../../rules/types';
@@ -615,7 +611,7 @@ export const exampleFindRuleStatusResponse: (
   saved_objects: mockStatuses.map((obj) => ({ ...obj, score: 1 })),
 });
 
-export const mockLogger: Logger = loggingSystemMock.createLogger();
+export const mockLogger = loggingSystemMock.createLogger();
 
 export const sampleBulkErrorItem = (
   {

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts

@@ -538,6 +538,35 @@ describe('rules_notification_alert_type', () => {
         expect(ruleStatusService.success).toHaveBeenCalled();
       });
 
+      it('should not call checkPrivileges if ML rule', async () => {
+        const ruleAlert = getMlResult();
+        payload = getPayload(ruleAlert, alertServices) as jest.Mocked<RuleExecutorOptions>;
+        jobsSummaryMock.mockResolvedValue([
+          {
+            id: 'some_job_id',
+            jobState: 'started',
+            datafeedState: 'started',
+          },
+        ]);
+        (findMlSignals as jest.Mock).mockResolvedValue({
+          _shards: { failed: 0 },
+          hits: {
+            hits: [{}],
+          },
+        });
+        (bulkCreateMlSignals as jest.Mock).mockResolvedValue({
+          success: true,
+          bulkCreateDuration: 1,
+          createdItemsCount: 1,
+          errors: [],
+        });
+        (checkPrivileges as jest.Mock).mockClear();
+
+        await alert.executor(payload);
+        expect(checkPrivileges).toHaveBeenCalledTimes(0);
+        expect(ruleStatusService.success).toHaveBeenCalled();
+      });
+
       it('should call scheduleActions if signalsCount was greater than 0 and rule has actions defined', async () => {
         const ruleAlert = getMlResult();
         ruleAlert.actions = [

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts

@@ -186,49 +186,52 @@ export const signalRulesAlertType = ({
       // move this collection of lines into a function in utils
       // so that we can use it in create rules route, bulk, etc.
       try {
-        const hasTimestampOverride = timestampOverride != null && !isEmpty(timestampOverride);
-        const [privileges, timestampFieldCaps] = await Promise.all([
-          pipe(
-            { services, version, index },
-            ({ services: svc, version: ver, index: idx }) =>
-              pipe(
-                tryCatch(() => getInputIndex(svc, ver, idx), toError),
-                chain((indices) => tryCatch(() => checkPrivileges(svc, indices), toError))
-              ),
-            toPromise
-          ),
-          services.scopedClusterClient.fieldCaps({
-            index,
-            fields: hasTimestampOverride
-              ? ['@timestamp', timestampOverride as string]
-              : ['@timestamp'],
-            allow_no_indices: false,
-            include_unmapped: true,
-          }),
-        ]);
-
-        wrotePartialFailureStatus = await flow(
-          () =>
-            tryCatch(
-              () => hasReadIndexPrivileges(privileges, logger, buildRuleMessage, ruleStatusService),
-              toError
-            ),
-          chain((wroteStatus) =>
-            tryCatch(
-              () =>
-                hasTimestampFields(
-                  wroteStatus,
-                  hasTimestampOverride ? (timestampOverride as string) : '@timestamp',
-                  timestampFieldCaps,
-                  ruleStatusService,
-                  logger,
-                  buildRuleMessage
+        if (!isEmpty(index)) {
+          const hasTimestampOverride = timestampOverride != null && !isEmpty(timestampOverride);
+          const [privileges, timestampFieldCaps] = await Promise.all([
+            pipe(
+              { services, version, index },
+              ({ services: svc, version: ver, index: idx }) =>
+                pipe(
+                  tryCatch(() => getInputIndex(svc, ver, idx), toError),
+                  chain((indices) => tryCatch(() => checkPrivileges(svc, indices), toError))
                 ),
-              toError
-            )
-          ),
-          toPromise
-        )();
+              toPromise
+            ),
+            services.scopedClusterClient.fieldCaps({
+              index,
+              fields: hasTimestampOverride
+                ? ['@timestamp', timestampOverride as string]
+                : ['@timestamp'],
+              allow_no_indices: false,
+              include_unmapped: true,
+            }),
+          ]);
+
+          wrotePartialFailureStatus = await flow(
+            () =>
+              tryCatch(
+                () =>
+                  hasReadIndexPrivileges(privileges, logger, buildRuleMessage, ruleStatusService),
+                toError
+              ),
+            chain((wroteStatus) =>
+              tryCatch(
+                () =>
+                  hasTimestampFields(
+                    wroteStatus,
+                    hasTimestampOverride ? (timestampOverride as string) : '@timestamp',
+                    timestampFieldCaps,
+                    ruleStatusService,
+                    logger,
+                    buildRuleMessage
+                  ),
+                toError
+              )
+            ),
+            toPromise
+          )();
+        }
       } catch (exc) {
         logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`));
       }

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts

@@ -6,6 +6,7 @@
 
 import moment from 'moment';
 import sinon from 'sinon';
+import { ApiResponse, Context } from '@elastic/elasticsearch/lib/Transport';
 
 import { alertsMock, AlertServicesMock } from '../../../../../alerts/server/mocks';
 import { listMock } from '../../../../../lists/server/mocks';
@@ -28,6 +29,7 @@ import {
   getListsClient,
   getSignalTimeTuples,
   getExceptions,
+  hasTimestampFields,
   wrapBuildingBlocks,
   generateSignalId,
   createErrorsFromShard,
@@ -61,6 +63,14 @@ const buildRuleMessage = buildRuleMessageFactory({
   name: 'fake name',
 });
 
+const ruleStatusServiceMock = {
+  success: jest.fn(),
+  find: jest.fn(),
+  goingToRun: jest.fn(),
+  error: jest.fn(),
+  partialFailure: jest.fn(),
+};
+
 describe('utils', () => {
   const anchor = '2020-01-01T06:06:06.666Z';
   const unix = moment(anchor).valueOf();
@@ -803,6 +813,85 @@ describe('utils', () => {
     });
   });
 
+  describe('hasTimestampFields', () => {
+    test('returns true when missing timestamp override field', async () => {
+      const timestampField = 'event.ingested';
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const timestampFieldCapsResponse: Partial<ApiResponse<Record<string, any>, Context>> = {
+        body: {
+          fields: {
+            [timestampField]: {
+              date: {
+                type: 'date',
+                searchable: true,
+                aggregatable: true,
+                indices: ['myfakeindex-3', 'myfakeindex-4'],
+              },
+              unmapped: {
+                type: 'unmapped',
+                searchable: false,
+                aggregatable: false,
+                indices: ['myfakeindex-1', 'myfakeindex-2'],
+              },
+            },
+          },
+        },
+      };
+      mockLogger.error.mockClear();
+      const res = await hasTimestampFields(
+        false,
+        timestampField,
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        timestampFieldCapsResponse as ApiResponse<Record<string, any>>,
+        ruleStatusServiceMock,
+        mockLogger,
+        buildRuleMessage
+      );
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        'The following indices are missing the timestamp override field "event.ingested": ["myfakeindex-1","myfakeindex-2"] name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
+      );
+      expect(res).toBeTruthy();
+    });
+    test('returns true when missing timestamp field', async () => {
+      const timestampField = '@timestamp';
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const timestampFieldCapsResponse: Partial<ApiResponse<Record<string, any>, Context>> = {
+        body: {
+          fields: {
+            [timestampField]: {
+              date: {
+                type: 'date',
+                searchable: true,
+                aggregatable: true,
+                indices: ['myfakeindex-3', 'myfakeindex-4'],
+              },
+              unmapped: {
+                type: 'unmapped',
+                searchable: false,
+                aggregatable: false,
+                indices: ['myfakeindex-1', 'myfakeindex-2'],
+              },
+            },
+          },
+        },
+      };
+      mockLogger.error.mockClear();
+      const res = await hasTimestampFields(
+        false,
+        timestampField,
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        timestampFieldCapsResponse as ApiResponse<Record<string, any>>,
+        ruleStatusServiceMock,
+        mockLogger,
+        buildRuleMessage
+      );
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        'The following indices are missing the timestamp field "@timestamp": ["myfakeindex-1","myfakeindex-2"] name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
+      );
+      expect(res).toBeTruthy();
+    });
+  });
+
   describe('wrapBuildingBlocks', () => {
     it('should generate a unique id for each building block', () => {
       const wrappedBlocks = wrapBuildingBlocks(

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts

@@ -120,8 +120,10 @@ export const hasTimestampFields = async (
     // if there is a timestamp override and the unmapped array for the timestamp override key is not empty,
     // partial failure
     const errorString = `The following indices are missing the ${
-      timestampField === '@timestamp' ? 'timestamp field "@timestamp"' : 'timestamp override field'
-    } "${timestampField}": ${JSON.stringify(
+      timestampField === '@timestamp'
+        ? 'timestamp field "@timestamp"'
+        : `timestamp override field "${timestampField}"`
+    }: ${JSON.stringify(
       isEmpty(timestampFieldCapsResponse.body.fields)
         ? timestampFieldCapsResponse.body.indices
         : timestampFieldCapsResponse.body.fields[timestampField].unmapped.indices