[Infra][ECO] Fix RBAC issue in hosts view (elastic#199841)

closes [elastic#200151](elastic#200151) ## Summary This PR change the `getApmIndices` function from `apm_data_access` to fetch the information using Kibana's internal user. This was done for 2 reasons: 1 - Plugins using `savedObjects.client` might face a situation where the logged in user doesn't have permission to read saved objects, causing the retrieval of apm indices to fail, which could lead to unexpected exceptions 2 - Elasticsearch is able to determine whether the user has permission to view docs in the index patterns, therefore, it's ok to retrieve the index pattern with Kibana's internal user because ultimately elasticsearch will only return the data the user has access to. ### Infra app permission **Role config:** <img width="500" alt="image" src="https://github.com/user-attachments/assets/ae98a98f-570a-4139-b804-91a8de0c9d1d"> <img width="500" alt="image" src="https://github.com/user-attachments/assets/afe29e7f-ab02-40f4-a86c-aeb016655708"> **Without access to APM indices** <img width="500" alt="image" src="https://github.com/user-attachments/assets/8aa7d4e5-3484-4723-838c-54920e442c08"> <img width="500" alt="image" src="https://github.com/user-attachments/assets/af3ce400-7a45-4313-84c7-5b8170c39bf5"> **With access to APM indices** <img width="500" alt="image" src="https://github.com/user-attachments/assets/1effc137-72a2-4e5b-b2ac-62e685370a21"> <img width="500" alt="image" src="https://github.com/user-attachments/assets/e161f6d9-85a7-4f80-a7d3-7ec0bdc338a3"> ### Admin <img width="500" alt="image" src="https://github.com/user-attachments/assets/d280f0d6-de6c-408f-a080-fa150d237afc"> ### How to test - Follow the steps above - Other areas affected: assistant and profiling --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mikecote · Nov 21, 2024 · 209c667 · 209c667
1 parent 8f8a671
commit 209c667
Show file tree

Hide file tree

Showing 19 changed files with 49 additions and 158 deletions.
diff --git a/x-pack/plugins/observability_solution/apm/server/assistant_functions/index.ts b/x-pack/plugins/observability_solution/apm/server/assistant_functions/index.ts
@@ -72,10 +72,7 @@ export function registerAssistantFunctions({
       ruleDataClient,
       plugins,
       getApmIndices: async () => {
-        const coreContext = await resources.context.core;
-        const apmIndices = await plugins.apmDataAccess.setup.getApmIndices(
-          coreContext.savedObjects.client
-        );
+        const apmIndices = await plugins.apmDataAccess.setup.getApmIndices();
         return apmIndices;
       },
     };

diff --git a/x-pack/plugins/observability_solution/apm/server/plugin.ts b/x-pack/plugins/observability_solution/apm/server/plugin.ts
@@ -16,7 +16,6 @@ import { registerAssistantFunctions } from './assistant_functions';
 import { registerDeprecations } from './deprecations';
 import { APM_FEATURE, registerFeaturesUsage } from './feature';
 import { createApmTelemetry } from './lib/apm_telemetry';
-import { getInternalSavedObjectsClient } from './lib/helpers/get_internal_saved_objects_client';
 import {
   APM_RULE_TYPE_ALERT_CONTEXT,
   apmRuleTypeAlertFieldMap,
@@ -115,21 +114,15 @@ export class APMPlugin
       };
     }) as APMRouteHandlerResources['plugins'];
 
-    const apmIndicesPromise = (async () => {
-      const coreStart = await getCoreStart();
-      const soClient = await getInternalSavedObjectsClient(coreStart);
-      const { getApmIndices } = plugins.apmDataAccess;
-      return getApmIndices(soClient);
-    })();
-
     // This if else block will go away in favour of removing Home Tutorial Integration
     // Ideally we will directly register a custom integration and pass the configs
     // for cloud, onPrem and Serverless so that the actual component can take
     // care of rendering
     if (currentConfig.serverlessOnboarding && plugins.customIntegrations) {
       plugins.customIntegrations?.registerCustomIntegration(apmTutorialCustomIntegration);
     } else {
-      apmIndicesPromise
+      plugins.apmDataAccess
+        .getApmIndices()
         .then((apmIndices) => {
           plugins.home?.tutorials.registerTutorial(
             tutorialProvider({

diff --git a/...plugins/observability_solution/apm/server/routes/apm_routes/register_apm_server_routes.ts b/...plugins/observability_solution/apm/server/routes/apm_routes/register_apm_server_routes.ts
@@ -109,10 +109,7 @@ export function registerRoutes({
         );
 
         const getApmIndices = async () => {
-          const coreContext = await context.core;
-          const apmIndices = await plugins.apmDataAccess.setup.getApmIndices(
-            coreContext.savedObjects.client
-          );
+          const apmIndices = await plugins.apmDataAccess.setup.getApmIndices();
           return apmIndices;
         };
 

diff --git a/...on/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts b/...on/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts
@@ -38,8 +38,7 @@ export const getAlertDetailsContextHandler = (
   return async (requestContext, query) => {
     const resources = {
       getApmIndices: async () => {
-        const coreContext = await requestContext.core;
-        return resourcePlugins.apmDataAccess.setup.getApmIndices(coreContext.savedObjects.client);
+        return resourcePlugins.apmDataAccess.setup.getApmIndices();
       },
       request: requestContext.request,
       params: { query: { _inspect: false } },

diff --git a/...plugins/observability_solution/apm/server/routes/fleet/register_fleet_policy_callbacks.ts b/...plugins/observability_solution/apm/server/routes/fleet/register_fleet_policy_callbacks.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { Logger, CoreStart, SavedObjectsClientContract } from '@kbn/core/server';
+import { Logger, CoreStart } from '@kbn/core/server';
 import {
   FleetStartContract,
   PostPackagePolicyCreateCallback,
@@ -22,7 +22,6 @@ import {
   SOURCE_MAP_API_KEY_PATH,
 } from './get_package_policy_decorators';
 import { createInternalESClient } from '../../lib/helpers/create_es_client/create_internal_es_client';
-import { getInternalSavedObjectsClient } from '../../lib/helpers/get_internal_saved_objects_client';
 import { APMRouteHandlerResources } from '../apm_routes/register_apm_server_routes';
 
 export async function registerFleetPolicyCallbacks({
@@ -149,7 +148,7 @@ function onPackagePolicyCreateOrUpdate({
   coreStart,
 }: {
   fleetPluginStart: FleetStartContract;
-  getApmIndices: (soClient: SavedObjectsClientContract) => Promise<APMIndices>;
+  getApmIndices: () => Promise<APMIndices>;
   coreStart: CoreStart;
 }): PutPackagePolicyUpdateCallback & PostPackagePolicyCreateCallback {
   return async (packagePolicy) => {
@@ -158,8 +157,7 @@ function onPackagePolicyCreateOrUpdate({
     }
 
     const { asInternalUser } = coreStart.elasticsearch.client;
-    const savedObjectsClient = await getInternalSavedObjectsClient(coreStart);
-    const apmIndices = await getApmIndices(savedObjectsClient);
+    const apmIndices = await getApmIndices();
 
     const internalESClient = await createInternalESClient({
       debug: false,

diff --git a/x-pack/plugins/observability_solution/apm_data_access/kibana.jsonc b/x-pack/plugins/observability_solution/apm_data_access/kibana.jsonc
@@ -18,9 +18,7 @@
     "requiredPlugins": [
       "data"
     ],
-    "optionalPlugins": [
-      "security"
-    ],
+    "optionalPlugins": [],
     "requiredBundles": []
   }
 }
diff --git a/x-pack/plugins/observability_solution/apm_data_access/server/index.ts b/x-pack/plugins/observability_solution/apm_data_access/server/index.ts
@@ -91,7 +91,6 @@ export type {
   APMEventESSearchRequest,
   APMLogEventESSearchRequest,
   DocumentSourcesRequest,
-  ApmDataAccessPrivilegesCheck,
   HostNamesRequest,
   GetDocumentTypeParams,
 } from './types';

diff --git a/x-pack/plugins/observability_solution/apm_data_access/server/lib/check_privileges.ts b/x-pack/plugins/observability_solution/apm_data_access/server/lib/check_privileges.ts
diff --git a/x-pack/plugins/observability_solution/apm_data_access/server/plugin.ts b/x-pack/plugins/observability_solution/apm_data_access/server/plugin.ts
@@ -5,32 +5,19 @@
  * 2.0.
  */
 
-import {
-  PluginInitializerContext,
-  CoreSetup,
-  CoreStart,
-  Plugin,
-  SavedObjectsClientContract,
-  Logger,
-} from '@kbn/core/server';
+import { PluginInitializerContext, CoreSetup, CoreStart, Plugin, Logger } from '@kbn/core/server';
 import { APMDataAccessConfig } from '.';
-import {
-  ApmDataAccessPluginSetup,
-  ApmDataAccessPluginStart,
-  ApmDataAccessServerDependencies,
-} from './types';
+import { ApmDataAccessPluginSetup, ApmDataAccessPluginStart } from './types';
 import { migrateLegacyAPMIndicesToSpaceAware } from './saved_objects/migrations/migrate_legacy_apm_indices_to_space_aware';
 import {
   apmIndicesSavedObjectDefinition,
   getApmIndicesSavedObject,
 } from './saved_objects/apm_indices';
 import { getServices } from './services/get_services';
-import { ApmDataAccessPrivilegesCheck, checkPrivileges } from './lib/check_privileges';
 
 export class ApmDataAccessPlugin
   implements Plugin<ApmDataAccessPluginSetup, ApmDataAccessPluginStart>
 {
-  public server?: ApmDataAccessServerDependencies;
   public config: APMDataAccessConfig;
   public logger: Logger;
 
@@ -39,45 +26,34 @@ export class ApmDataAccessPlugin
     this.logger = initContext.logger.get();
   }
 
-  getApmIndices = async (savedObjectsClient: SavedObjectsClientContract) => {
-    const apmIndicesFromSavedObject = await getApmIndicesSavedObject(savedObjectsClient);
-    return { ...this.config.indices, ...apmIndicesFromSavedObject };
-  };
-
   public setup(core: CoreSetup): ApmDataAccessPluginSetup {
     // register saved object
     core.savedObjects.registerType(apmIndicesSavedObjectDefinition);
 
+    const getApmIndices = async () => {
+      const [coreStart] = await core.getStartServices();
+      const soClient = await coreStart.savedObjects.createInternalRepository();
+
+      const apmIndicesFromSavedObject = await getApmIndicesSavedObject(soClient);
+      return { ...this.config.indices, ...apmIndicesFromSavedObject };
+    };
+
     // expose
     return {
       apmIndicesFromConfigFile: this.config.indices,
-      getApmIndices: this.getApmIndices,
+      getApmIndices,
       getServices,
     };
   }
 
-  public start(core: CoreStart, plugins: ApmDataAccessServerDependencies) {
+  public start(core: CoreStart) {
     // TODO: remove in 9.0
     migrateLegacyAPMIndicesToSpaceAware({ coreStart: core, logger: this.logger }).catch((e) => {
       this.logger.error('Failed to run migration making APM indices space aware');
       this.logger.error(e);
     });
 
-    const getApmIndicesWithInternalUserFn = async () => {
-      const soClient = core.savedObjects.createInternalRepository();
-      return this.getApmIndices(soClient);
-    };
-
-    const startServices = {
-      hasPrivileges: ({ request }: Pick<ApmDataAccessPrivilegesCheck, 'request'>) =>
-        checkPrivileges({
-          request,
-          getApmIndices: getApmIndicesWithInternalUserFn,
-          security: plugins.security,
-        }),
-    };
-
-    return { ...startServices };
+    return {};
   }
 
   public stop() {}

diff --git a/x-pack/plugins/observability_solution/apm_data_access/server/types.ts b/x-pack/plugins/observability_solution/apm_data_access/server/types.ts
@@ -5,28 +5,17 @@
  * 2.0.
  */
 
-import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
-import type { SecurityPluginStart } from '@kbn/security-plugin-types-server';
 import type { APMIndices } from '.';
 import { getServices } from './services/get_services';
-import type { ApmDataAccessPrivilegesCheck } from './lib/check_privileges';
 
 export interface ApmDataAccessPluginSetup {
   apmIndicesFromConfigFile: APMIndices;
-  getApmIndices: (soClient: SavedObjectsClientContract) => Promise<APMIndices>;
+  getApmIndices: () => Promise<APMIndices>;
   getServices: typeof getServices;
 }
 
-export interface ApmDataAccessServerDependencies {
-  security?: SecurityPluginStart;
-}
-
-export interface ApmDataAccessPluginStart {
-  hasPrivileges: (params: Pick<ApmDataAccessPrivilegesCheck, 'request'>) => Promise<boolean>;
-}
-export interface ApmDataAccessServerDependencies {
-  security?: SecurityPluginStart;
-}
+// eslint-disable-next-line @typescript-eslint/no-empty-interface
+export interface ApmDataAccessPluginStart {}
 
 export type ApmDataAccessServices = ReturnType<typeof getServices>;
 export type { ApmDataAccessServicesParams } from './services/get_services';
@@ -38,4 +27,3 @@ export type {
   APMEventESSearchRequest,
   APMLogEventESSearchRequest,
 } from './lib/helpers';
-export type { ApmDataAccessPrivilegesCheck };
diff --git a/x-pack/plugins/observability_solution/apm_data_access/tsconfig.json b/x-pack/plugins/observability_solution/apm_data_access/tsconfig.json
@@ -9,7 +9,6 @@
     "@kbn/config-schema",
     "@kbn/core",
     "@kbn/i18n",
-    "@kbn/core-saved-objects-api-server",
     "@kbn/data-plugin",
     "@kbn/inspector-plugin",
     "@kbn/observability-plugin",
@@ -18,8 +17,6 @@
     "@kbn/apm-types",
     "@kbn/core-http-server-mocks",
     "@kbn/apm-utils",
-    "@kbn/core-http-server",
-    "@kbn/security-plugin-types-server",
     "@kbn/utility-types",
     "@kbn/elastic-agent-utils",
     "@kbn/observability-utils-common"

diff --git a/x-pack/plugins/observability_solution/infra/server/lib/helpers/get_apm_data_access_client.ts b/x-pack/plugins/observability_solution/infra/server/lib/helpers/get_apm_data_access_client.ts
@@ -27,23 +27,17 @@ export const getApmDataAccessClient = ({
   context: InfraPluginRequestHandlerContext;
   request: KibanaRequest;
 }) => {
-  const hasPrivileges = async () => {
-    const apmDataAccessStart = await libs.plugins.apmDataAccess.start();
-    return apmDataAccessStart.hasPrivileges({ request });
-  };
-
   const getServices = async () => {
     const apmDataAccess = libs.plugins.apmDataAccess.setup;
 
     const coreContext = await context.core;
 
-    const { savedObjects, uiSettings, elasticsearch } = coreContext;
-    const savedObjectsClient = savedObjects.client;
+    const { uiSettings, elasticsearch } = coreContext;
     const esClient = elasticsearch.client.asCurrentUser;
     const uiSettingsClient = uiSettings.client;
 
     const [apmIndices, includeFrozen] = await Promise.all([
-      apmDataAccess.getApmIndices(savedObjectsClient),
+      apmDataAccess.getApmIndices(),
       uiSettingsClient.get<boolean>(UI_SETTINGS.SEARCH_INCLUDE_FROZEN),
     ]);
 
@@ -86,5 +80,5 @@ export const getApmDataAccessClient = ({
     };
   };
 
-  return { hasPrivileges, getServices };
+  return { getServices };
 };
diff --git a/x-pack/plugins/observability_solution/infra/server/routes/infra/index.ts b/x-pack/plugins/observability_solution/infra/server/routes/infra/index.ts
@@ -41,12 +41,11 @@ export const initInfraAssetRoutes = (libs: InfraBackendLibs) => {
 
       try {
         const apmDataAccessClient = getApmDataAccessClient({ request, libs, context });
-        const hasApmPrivileges = await apmDataAccessClient.hasPrivileges();
 
         const [infraMetricsClient, alertsClient, apmDataAccessServices] = await Promise.all([
           getInfraMetricsClient({ request, libs, context }),
           getInfraAlertsClient({ libs, request }),
-          hasApmPrivileges ? apmDataAccessClient.getServices() : undefined,
+          apmDataAccessClient.getServices(),
         ]);
 
         const hosts = await getHosts({
@@ -97,11 +96,10 @@ export const initInfraAssetRoutes = (libs: InfraBackendLibs) => {
 
       try {
         const apmDataAccessClient = getApmDataAccessClient({ request, libs, context });
-        const hasApmPrivileges = await apmDataAccessClient.hasPrivileges();
 
         const [infraMetricsClient, apmDataAccessServices] = await Promise.all([
           getInfraMetricsClient({ request, libs, context }),
-          hasApmPrivileges ? apmDataAccessClient.getServices() : undefined,
+          apmDataAccessClient.getServices(),
         ]);
 
         const count = await getHostsCount({