[ResponseOps] Exclude RBAC from SIEM for alerts search strategy (#126859)

* Exclude RBAC from siem requests

* PR feedback

* PR feedback

* Fix tests

x-pack/plugins/rule_registry/server/search_strategy/search_strategy.test.ts

@@ -68,6 +68,7 @@ describe('ruleRegistrySearchStrategyProvider()', () => {
   });
 
   let getAuthzFilterSpy: jest.SpyInstance;
+  const searchStrategySearch = jest.fn().mockImplementation(() => of(response));
 
   beforeEach(() => {
     ruleDataService.findIndicesByFeature.mockImplementation(() => {
@@ -80,10 +81,14 @@ describe('ruleRegistrySearchStrategyProvider()', () => {
 
     data.search.getSearchStrategy.mockImplementation(() => {
       return {
-        search: () => of(response),
+        search: searchStrategySearch,
       };
     });
 
+    (data.search.searchAsInternalUser.search as jest.Mock).mockImplementation(() => {
+      return of(response);
+    });
+
     getAuthzFilterSpy = jest
       .spyOn(getAuthzFilterImport, 'getAuthzFilter')
       .mockImplementation(async () => {
@@ -94,7 +99,9 @@ describe('ruleRegistrySearchStrategyProvider()', () => {
   afterEach(() => {
     ruleDataService.findIndicesByFeature.mockClear();
     data.search.getSearchStrategy.mockClear();
+    (data.search.searchAsInternalUser.search as jest.Mock).mockClear();
     getAuthzFilterSpy.mockClear();
+    searchStrategySearch.mockClear();
   });
 
   it('should handle a basic search request', async () => {
@@ -199,4 +206,107 @@ describe('ruleRegistrySearchStrategyProvider()', () => {
       .toPromise();
     expect(result).toBe(EMPTY_RESPONSE);
   });
+
+  it('should not apply rbac filters for siem', async () => {
+    const request: RuleRegistrySearchRequest = {
+      featureIds: [AlertConsumers.SIEM],
+    };
+    const options = {};
+    const deps = {
+      request: {},
+    };
+
+    const strategy = ruleRegistrySearchStrategyProvider(
+      data,
+      ruleDataService,
+      alerting,
+      logger,
+      security,
+      spaces
+    );
+
+    await strategy
+      .search(request, options, deps as unknown as SearchStrategyDependencies)
+      .toPromise();
+    expect(getAuthzFilterSpy).not.toHaveBeenCalled();
+  });
+
+  it('should throw an error if requesting multiple featureIds and one is SIEM', async () => {
+    const request: RuleRegistrySearchRequest = {
+      featureIds: [AlertConsumers.SIEM, AlertConsumers.LOGS],
+    };
+    const options = {};
+    const deps = {
+      request: {},
+    };
+
+    const strategy = ruleRegistrySearchStrategyProvider(
+      data,
+      ruleDataService,
+      alerting,
+      logger,
+      security,
+      spaces
+    );
+
+    let err;
+    try {
+      await strategy
+        .search(request, options, deps as unknown as SearchStrategyDependencies)
+        .toPromise();
+    } catch (e) {
+      err = e;
+    }
+    expect(err).toBeDefined();
+  });
+
+  it('should use internal user when requesting o11y alerts as RBAC is applied', async () => {
+    const request: RuleRegistrySearchRequest = {
+      featureIds: [AlertConsumers.LOGS],
+    };
+    const options = {};
+    const deps = {
+      request: {},
+    };
+
+    const strategy = ruleRegistrySearchStrategyProvider(
+      data,
+      ruleDataService,
+      alerting,
+      logger,
+      security,
+      spaces
+    );
+
+    await strategy
+      .search(request, options, deps as unknown as SearchStrategyDependencies)
+      .toPromise();
+    expect(data.search.searchAsInternalUser.search).toHaveBeenCalled();
+    expect(searchStrategySearch).not.toHaveBeenCalled();
+  });
+
+  it('should use scoped user when requesting siem alerts as RBAC is not applied', async () => {
+    const request: RuleRegistrySearchRequest = {
+      featureIds: [AlertConsumers.SIEM],
+    };
+    const options = {};
+    const deps = {
+      request: {},
+    };
+
+    const strategy = ruleRegistrySearchStrategyProvider(
+      data,
+      ruleDataService,
+      alerting,
+      logger,
+      security,
+      spaces
+    );
+
+    await strategy
+      .search(request, options, deps as unknown as SearchStrategyDependencies)
+      .toPromise();
+    expect(data.search.searchAsInternalUser.search).not.toHaveBeenCalled();
+    expect(searchStrategySearch).toHaveBeenCalled();
+  });
 });

x-pack/plugins/rule_registry/server/search_strategy/search_strategy.ts

@@ -8,7 +8,7 @@ import { map, mergeMap, catchError } from 'rxjs/operators';
 import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 import { Logger } from 'src/core/server';
 import { from, of } from 'rxjs';
-import { isValidFeatureId } from '@kbn/rule-data-utils';
+import { isValidFeatureId, AlertConsumers } from '@kbn/rule-data-utils';
 import { ENHANCED_ES_SEARCH_STRATEGY } from '../../../../../src/plugins/data/common';
 import { ISearchStrategy, PluginStart } from '../../../../../src/plugins/data/server';
 import {
@@ -36,10 +36,22 @@ export const ruleRegistrySearchStrategyProvider = (
   security?: SecurityPluginSetup,
   spaces?: SpacesPluginStart
 ): ISearchStrategy<RuleRegistrySearchRequest, RuleRegistrySearchResponse> => {
-  const es = data.search.getSearchStrategy(ENHANCED_ES_SEARCH_STRATEGY);
-
+  const internalUserEs = data.search.searchAsInternalUser;
+  const requestUserEs = data.search.getSearchStrategy(ENHANCED_ES_SEARCH_STRATEGY);
   return {
     search: (request, options, deps) => {
+      // SIEM uses RBAC fields in their alerts but also utilizes ES DLS which
+      // is different than every other solution so we need to special case
+      // those requests.
+      let siemRequest = false;
+      if (request.featureIds.length === 1 && request.featureIds[0] === AlertConsumers.SIEM) {
+        siemRequest = true;
+      } else if (request.featureIds.includes(AlertConsumers.SIEM)) {
+        throw new Error(
+          'The ruleRegistryAlertsSearchStrategy search strategy is unable to accommodate requests containing multiple feature IDs and one of those IDs is SIEM.'
+        );
+      }
+
       const securityAuditLogger = security?.audit.asScoped(deps.request);
       const getActiveSpace = async () => spaces?.spacesService.getActiveSpace(deps.request);
       const getAsync = async () => {
@@ -47,10 +59,14 @@ export const ruleRegistrySearchStrategyProvider = (
           getActiveSpace(),
           alerting.getAlertingAuthorizationWithRequest(deps.request),
         ]);
-        const authzFilter = (await getAuthzFilter(
-          authorization,
-          ReadOperations.Find
-        )) as estypes.QueryDslQueryContainer;
+        let authzFilter;
+
+        if (!siemRequest) {
+          authzFilter = (await getAuthzFilter(
+            authorization,
+            ReadOperations.Find
+          )) as estypes.QueryDslQueryContainer;
+        }
         return { space, authzFilter };
       };
       return from(getAsync()).pipe(
@@ -93,7 +109,6 @@ export const ruleRegistrySearchStrategyProvider = (
 
           const query = {
             bool: {
-              ...request.query?.bool,
               filter,
             },
           };
@@ -106,7 +121,7 @@ export const ruleRegistrySearchStrategyProvider = (
               query,
             },
           };
-          return es.search({ ...request, params }, options, deps);
+          return (siemRequest ? requestUserEs : internalUserEs).search({ params }, options, deps);
         }),
         map((response) => {
           // Do we have to loop over each hit? Yes.
@@ -141,9 +156,8 @@ export const ruleRegistrySearchStrategyProvider = (
       );
     },
     cancel: async (id, options, deps) => {
-      if (es.cancel) {
-        return es.cancel(id, options, deps);
-      }
+      if (internalUserEs.cancel) internalUserEs.cancel(id, options, deps);
+      if (requestUserEs.cancel) requestUserEs.cancel(id, options, deps);
     },
   };
 };

x-pack/test/common/services/bsearch_secure.ts

@@ -0,0 +1,94 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+// NOTE: This is pretty much a copy/paste from test/common/services/bsearch.ts but with the ability
+// to provide custom auth
+
+import expect from '@kbn/expect';
+import request from 'superagent';
+import type SuperTest from 'supertest';
+import { IEsSearchResponse } from 'src/plugins/data/common';
+import { FtrProviderContext } from '../ftr_provider_context';
+import { RetryService } from '../../../../test/common/services/retry/retry';
+
+const parseBfetchResponse = (resp: request.Response): Array<Record<string, any>> => {
+  return resp.text
+    .trim()
+    .split('\n')
+    .map((item) => JSON.parse(item));
+};
+
+const getSpaceUrlPrefix = (spaceId?: string): string => {
+  return spaceId && spaceId !== 'default' ? `/s/${spaceId}` : ``;
+};
+
+interface SendOptions {
+  supertestWithoutAuth: SuperTest.SuperTest<SuperTest.Test>;
+  auth: { username: string; password: string };
+  options: object;
+  strategy: string;
+  space?: string;
+}
+
+export const BSecureSearchFactory = (retry: RetryService) => ({
+  send: async <T extends IEsSearchResponse>({
+    supertestWithoutAuth,
+    auth,
+    options,
+    strategy,
+    space,
+  }: SendOptions): Promise<T> => {
+    const spaceUrl = getSpaceUrlPrefix(space);
+    const { body } = await retry.try(async () => {
+      const result = await supertestWithoutAuth
+        .post(`${spaceUrl}/internal/search/${strategy}`)
+        .auth(auth.username, auth.password)
+        .set('kbn-xsrf', 'true')
+        .send(options);
+      if (result.status === 500 || result.status === 200) {
+        return result;
+      }
+      throw new Error('try again');
+    });
+
+    if (body.isRunning) {
+      const result = await retry.try(async () => {
+        const resp = await supertestWithoutAuth
+          .post(`${spaceUrl}/internal/bsearch`)
+          .auth(auth.username, auth.password)
+          .set('kbn-xsrf', 'true')
+          .send({
+            batch: [
+              {
+                request: {
+                  id: body.id,
+                  ...options,
+                },
+                options: {
+                  strategy,
+                },
+              },
+            ],
+          })
+          .expect(200);
+        const [parsedResponse] = parseBfetchResponse(resp);
+        expect(parsedResponse.result.isRunning).equal(false);
+        return parsedResponse.result;
+      });
+      return result;
+    } else {
+      return body;
+    }
+  },
+});
+
+export function BSecureSearchProvider({
+  getService,
+}: FtrProviderContext): ReturnType<typeof BSecureSearchFactory> {
+  const retry = getService('retry');
+  return BSecureSearchFactory(retry);
+}

x-pack/test/common/services/index.ts

@@ -9,10 +9,12 @@ import { services as kibanaCommonServices } from '../../../../test/common/servic
 import { services as kibanaApiIntegrationServices } from '../../../../test/api_integration/services';
 
 import { SpacesServiceProvider } from './spaces';
+import { BSecureSearchProvider } from './bsearch_secure';
 
 export const services = {
   ...kibanaCommonServices,
   supertest: kibanaApiIntegrationServices.supertest,
 
   spaces: SpacesServiceProvider,
+  secureBsearch: BSecureSearchProvider,
 };

x-pack/test/rule_registry/common/lib/authentication/roles.ts

@@ -221,6 +221,52 @@ export const observabilityOnlyAllSpacesAll: Role = {
   },
 };
 
+export const logsOnlyAllSpacesAll: Role = {
+  name: 'logs_only_all_spaces_all',
+  privileges: {
+    elasticsearch: {
+      indices: [],
+    },
+    kibana: [
+      {
+        feature: {
+          logs: ['all'],
+        },
+        spaces: ['*'],
+      },
+    ],
+  },
+};
+
+/**
+ * This role exists to test that the alert search strategy allows
+ * users who do not have access to security solutions the ability
+ * to see security solutions alerts. This is because security solutions
+ * does not properly leverage RBAC and we filter out the RBAC when
+ * searching for security solution alerts in the alert search strategy.
+ */
+export const observabilityOnlyAllSpacesAllWithReadESIndices: Role = {
+  name: 'obs_only_all_spaces_all_with_read_es_indices',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        feature: {
+          apm: ['all'],
+        },
+        spaces: ['default', 'space1'],
+      },
+    ],
+  },
+};
+
 export const observabilityOnlyReadSpacesAll: Role = {
   name: 'obs_only_read_all_spaces_all',
   privileges: {
@@ -246,6 +292,7 @@ export const roles = [
   observabilityOnlyAll,
   observabilityOnlyRead,
   observabilityOnlyReadSpacesAll,
+  observabilityOnlyAllSpacesAllWithReadESIndices,
 ];
 
 /**
@@ -416,7 +463,9 @@ export const allRoles = [
   securitySolutionOnlyAllSpacesAll,
   securitySolutionOnlyReadSpacesAll,
   observabilityOnlyAllSpacesAll,
+  logsOnlyAllSpacesAll,
   observabilityOnlyReadSpacesAll,
+  observabilityOnlyAllSpacesAllWithReadESIndices,
   observabilityMinReadAlertsRead,
   observabilityMinReadAlertsReadSpacesAll,
   observabilityMinimalRead,

x-pack/test/rule_registry/common/lib/authentication/users.ts

@@ -15,6 +15,7 @@ import {
   securitySolutionOnlyAllSpacesAll,
   securitySolutionOnlyReadSpacesAll,
   observabilityOnlyAllSpacesAll,
+  logsOnlyAllSpacesAll,
   observabilityOnlyReadSpacesAll,
   // trial license roles
   observabilityMinReadAlertsAll,
@@ -27,6 +28,7 @@ import {
   observabilityOnlyAllSpace2,
   observabilityOnlyReadSpace2,
   observabilityMinReadAlertsAllSpacesAll,
+  observabilityOnlyAllSpacesAllWithReadESIndices,
 } from './roles';
 import { User } from './types';
 
@@ -161,6 +163,18 @@ export const obsOnlySpacesAll: User = {
   roles: [observabilityOnlyAllSpacesAll.name],
 };
 
+export const logsOnlySpacesAll: User = {
+  username: 'logs_only_all_spaces_all',
+  password: 'logs_only_all_spaces_all',
+  roles: [logsOnlyAllSpacesAll.name],
+};
+
+export const obsOnlySpacesAllEsRead: User = {
+  username: 'obs_only_all_spaces_all_es_read',
+  password: 'obs_only_all_spaces_all_es_read',
+  roles: [observabilityOnlyAllSpacesAllWithReadESIndices.name],
+};
+
 export const obsSecSpacesAll: User = {
   username: 'sec_only_all_spaces_all_and_obs_only_all_spaces_all',
   password: 'sec_only_all_spaces_all_and_obs_only_all_spaces_all',
@@ -267,6 +281,7 @@ export const allUsers = [
   secOnlySpacesAll,
   secOnlyReadSpacesAll,
   obsOnlySpacesAll,
+  logsOnlySpacesAll,
   obsSecSpacesAll,
   obsSecReadSpacesAll,
   obsMinReadAlertsRead,
@@ -283,4 +298,5 @@ export const allUsers = [
   obsOnlyReadSpace2,
   obsSecAllSpace2,
   obsSecReadSpace2,
+  obsOnlySpacesAllEsRead,
 ];

x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts

@@ -20,27 +20,38 @@ import {
 } from '../../../../detection_engine_api_integration/utils';
 import { ID } from '../../../../detection_engine_api_integration/security_and_spaces/tests/generating_signals';
 import { QueryCreateSchema } from '../../../../../plugins/security_solution/common/detection_engine/schemas/request';
+import {
+  obsOnlySpacesAllEsRead,
+  obsOnlySpacesAll,
+  logsOnlySpacesAll,
+} from '../../../common/lib/authentication/users';
 
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext) => {
   const esArchiver = getService('esArchiver');
   const supertest = getService('supertest');
-  const bsearch = getService('bsearch');
+  const supertestWithoutAuth = getService('supertestWithoutAuth');
+  // const bsearch = getService('bsearch');
+  const secureBsearch = getService('secureBsearch');
   const log = getService('log');
 
   const SPACE1 = 'space1';
 
   describe('ruleRegistryAlertsSearchStrategy', () => {
     describe('logs', () => {
-      beforeEach(async () => {
+      before(async () => {
         await esArchiver.load('x-pack/test/functional/es_archives/observability/alerts');
       });
-      afterEach(async () => {
+      after(async () => {
         await esArchiver.unload('x-pack/test/functional/es_archives/observability/alerts');
       });
       it('should return alerts from log rules', async () => {
-        const result = await bsearch.send<RuleRegistrySearchResponse>({
-          supertest,
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: logsOnlySpacesAll.username,
+            password: logsOnlySpacesAll.password,
+          },
           options: {
             featureIds: [AlertConsumers.LOGS],
           },
@@ -54,25 +65,13 @@ export default ({ getService }: FtrProviderContext) => {
       });
     });
 
+    // TODO: need xavier's help here
     describe('siem', () => {
-      beforeEach(async () => {
-        await deleteSignalsIndex(supertest, log);
-        await createSignalsIndex(supertest, log);
-      });
-
-      afterEach(async () => {
-        await deleteSignalsIndex(supertest, log);
-        await deleteAllAlerts(supertest, log);
-      });
-
       before(async () => {
+        await createSignalsIndex(supertest, log);
         await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts');
-      });
-      after(async () => {
-        await esArchiver.unload('x-pack/test/functional/es_archives/auditbeat/hosts');
-      });
+        await esArchiver.load('x-pack/test/functional/es_archives/observability/alerts');
 
-      it('should return alerts from siem rules', async () => {
         const rule: QueryCreateSchema = {
           ...getRuleForSignalTesting(['auditbeat-*']),
           query: `_id:${ID}`,
@@ -80,9 +79,22 @@ export default ({ getService }: FtrProviderContext) => {
         const { id: createdId } = await createRule(supertest, log, rule);
         await waitForRuleSuccessOrStatus(supertest, log, createdId);
         await waitForSignalsToBePresent(supertest, log, 1, [createdId]);
+      });
 
-        const result = await bsearch.send<RuleRegistrySearchResponse>({
-          supertest,
+      after(async () => {
+        await deleteSignalsIndex(supertest, log);
+        await deleteAllAlerts(supertest, log);
+        await esArchiver.unload('x-pack/test/functional/es_archives/auditbeat/hosts');
+        await esArchiver.unload('x-pack/test/functional/es_archives/observability/alerts');
+      });
+
+      it('should return alerts from siem rules', async () => {
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: obsOnlySpacesAllEsRead.username,
+            password: obsOnlySpacesAllEsRead.password,
+          },
           options: {
             featureIds: [AlertConsumers.SIEM],
           },
@@ -94,19 +106,45 @@ export default ({ getService }: FtrProviderContext) => {
         );
         expect(consumers.every((consumer) => consumer === AlertConsumers.SIEM));
       });
+
+      it('should throw an error when trying to to search for more than just siem', async () => {
+        type RuleRegistrySearchResponseWithErrors = RuleRegistrySearchResponse & {
+          statusCode: number;
+          message: string;
+        };
+        const result = await secureBsearch.send<RuleRegistrySearchResponseWithErrors>({
+          supertestWithoutAuth,
+          auth: {
+            username: obsOnlySpacesAllEsRead.username,
+            password: obsOnlySpacesAllEsRead.password,
+          },
+          options: {
+            featureIds: [AlertConsumers.SIEM, AlertConsumers.LOGS],
+          },
+          strategy: 'ruleRegistryAlertsSearchStrategy',
+        });
+        expect(result.statusCode).to.be(500);
+        expect(result.message).to.be(
+          `The ruleRegistryAlertsSearchStrategy search strategy is unable to accommodate requests containing multiple feature IDs and one of those IDs is SIEM.`
+        );
+      });
     });
 
     describe('apm', () => {
-      beforeEach(async () => {
+      before(async () => {
         await esArchiver.load('x-pack/test/functional/es_archives/rule_registry/alerts');
       });
-      afterEach(async () => {
+      after(async () => {
         await esArchiver.unload('x-pack/test/functional/es_archives/rule_registry/alerts');
       });
 
       it('should return alerts from apm rules', async () => {
-        const result = await bsearch.send<RuleRegistrySearchResponse>({
-          supertest,
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: obsOnlySpacesAll.username,
+            password: obsOnlySpacesAll.password,
+          },
           options: {
             featureIds: [AlertConsumers.APM],
           },
@@ -123,13 +161,16 @@ export default ({ getService }: FtrProviderContext) => {
 
     describe('empty response', () => {
       it('should return an empty response', async () => {
-        const result = await bsearch.send<RuleRegistrySearchResponse>({
-          supertest,
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: obsOnlySpacesAll.username,
+            password: obsOnlySpacesAll.password,
+          },
           options: {
             featureIds: [],
           },
           strategy: 'ruleRegistryAlertsSearchStrategy',
-          space: SPACE1,
         });
         expect(result.rawResponse).to.eql({});
       });