Skip to content

Commit

Permalink
[Infra][ECO] Fix RBAC issue in hosts view (elastic#199841)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
3 people authored and paulinashakirova committed Nov 26, 2024
1 parent 99e3bc0 commit db85cb3
Show file tree
Hide file tree
Showing 19 changed files with 49 additions and 158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
};
Expand Down
11 changes: 2 additions & 9 deletions x-pack/plugins/observability_solution/apm/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { Logger, CoreStart, SavedObjectsClientContract } from '@kbn/core/server';
import { Logger, CoreStart } from '@kbn/core/server';
import {
FleetStartContract,
PostPackagePolicyCreateCallback,
Expand All @@ -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({
Expand Down Expand Up @@ -149,7 +148,7 @@ function onPackagePolicyCreateOrUpdate({
coreStart,
}: {
fleetPluginStart: FleetStartContract;
getApmIndices: (soClient: SavedObjectsClientContract) => Promise<APMIndices>;
getApmIndices: () => Promise<APMIndices>;
coreStart: CoreStart;
}): PutPackagePolicyUpdateCallback & PostPackagePolicyCreateCallback {
return async (packagePolicy) => {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
"requiredPlugins": [
"data"
],
"optionalPlugins": [
"security"
],
"optionalPlugins": [],
"requiredBundles": []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export type {
APMEventESSearchRequest,
APMLogEventESSearchRequest,
DocumentSourcesRequest,
ApmDataAccessPrivilegesCheck,
HostNamesRequest,
GetDocumentTypeParams,
} from './types';
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -38,4 +27,3 @@ export type {
APMEventESSearchRequest,
APMLogEventESSearchRequest,
} from './lib/helpers';
export type { ApmDataAccessPrivilegesCheck };
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]);

Expand Down Expand Up @@ -86,5 +80,5 @@ export const getApmDataAccessClient = ({
};
};

return { hasPrivileges, getServices };
return { getServices };
};
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down
Loading

0 comments on commit db85cb3

Please sign in to comment.